-
Notifications
You must be signed in to change notification settings - Fork 977
fix(install): default to GNU host in Cygwin/MSYS/MinGW environments (#4421) #4497
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much :)
However, 4221 is not an issue. Maybe you've got the number wrong?
rustup-init.sh
Outdated
esac | ||
done | ||
if [ "$_has_default_host_arg" = false ]; then | ||
set -- "$@" --default-host x86_64-pc-windows-gnu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider the possibility of aarch64-pc-windows-gnullvm
as well? It's a tier 2 target for now.
cc @ChrisDenton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... for arm we should probably not touch the default for now, at least until support for gnu on the target is more mainstream. The gnullvm target was only fairly recently promoted to having host tooling and iirc requires installing llvm tools (instead of gnu ones). We could revisit this in the future.
rustup-init.sh
Outdated
ignore "$_file" "$@" < /dev/tty | ||
else | ||
# Default to GNU toolchain under Cygwin/MSYS/MinGW environments | ||
case "$(uname -s)" in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should move whatever default host detection logic you may have into the get_architecture()
function, otherwise it might break the RUSTUP_INIT_SH_PRINT=arch
functionality.
192531b
to
a051ee1
Compare
@rami3l |
950e61d
to
072ba3f
Compare
072ba3f
to
f3ccd26
Compare
Addresses first mentioned issue in #4421, modifying the install script to default to
x86_64-pc-windows-gnu
under Cygwin/MSYS when no --default-host is provided.The second issue requires changes in the compiled rustup-init binary (src/), which is out of scope for now