Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows: detect architecture on website, update to matching arch (#1353) #1354

Merged
merged 1 commit into from
Feb 13, 2018

Conversation

steffengy
Copy link
Contributor

Updates on windows will now switch over to host appropriate versions,
ensuring the best user experience (e.g. x86_64 builds on x64).

The website can distinguish between Win64/Win32 builds now and
a new platform can be introduced more easily.

r? @alexcrichton

@steffengy
Copy link
Contributor Author

Notably, this shouldn't be merged without a test of rustup itself yet, just first feedback & an initial CI check.

@alexcrichton
Copy link
Member

This looks good to me, thanks! What sort of testing did you have in mind for this?

@steffengy
Copy link
Contributor Author

@alexcrichton
Building rustup locally as i686 and x86_64 with 1.09 as version and testing if it downloads the right 1.10 executable when updating. Other than that I think this should be fine?

@alexcrichton
Copy link
Member

I can confirm that locally after building the i686-pc-windows-msvc version it switched to x86_64-pc-windows-msvc

// and also works around if the website messed up the detection.
// If someone really wants to use another version, he still can enforce
// that using the environment variable RUSTUP_OVERRIDE_HOST_TRIPLE.
#[cfg(windows)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ended up causing a build warning on windows, could if cfg!(windows) be used instead?

…t-lang#1353)

Updates on windows will now switch over to host appropriate versions,
ensuring the best user experience (e.g. x86_64 builds on x64).

The website can distinguish between Win64/Win32 builds now and
a new platform can be introduced more easily.
@steffengy
Copy link
Contributor Author

@alexcrichton
Changed that match to an if and tested using x86_64-pc-windows-msvc and i686-pc-windows-gnu (UGH I hate MSYS2) and it also works.

So good to go from my side.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 13, 2018

📌 Commit 0068f09 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Feb 13, 2018

⌛ Testing commit 0068f09 with merge 35bfc26...

bors added a commit that referenced this pull request Feb 13, 2018
windows: detect architecture on website, update to matching arch (#1353)

Updates on windows will now switch over to host appropriate versions,
ensuring the best user experience (e.g. x86_64 builds on x64).

The website can distinguish between Win64/Win32 builds now and
a new platform can be introduced more easily.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Feb 13, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 35bfc26 to master...

@bors bors merged commit 0068f09 into rust-lang:master Feb 13, 2018
AJ-Ianozi pushed a commit to AJ-Ianozi/getada-download that referenced this pull request Mar 9, 2024
windows: detect architecture on website, update to matching arch (rust-lang#1353)

Updates on windows will now switch over to host appropriate versions,
ensuring the best user experience (e.g. x86_64 builds on x64).

The website can distinguish between Win64/Win32 builds now and
a new platform can be introduced more easily.

r? @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants