Skip to content

fix(rustup-init/sh): prevent passing --default-host twice#4756

Merged
rami3l merged 1 commit intorust-lang:mainfrom
rami3l:fix/rustup-init-cygwin-host-override
Mar 16, 2026
Merged

fix(rustup-init/sh): prevent passing --default-host twice#4756
rami3l merged 1 commit intorust-lang:mainfrom
rami3l:fix/rustup-init-cygwin-host-override

Conversation

@rami3l
Copy link
Member

@rami3l rami3l commented Mar 16, 2026

Follow-up of #4497, closes #4755.

My apologies for having misled you @cachebag on this one (I mistakenly thought rustup followed the GNU coreutils conventions, but should we adopt just that in rustup the binary instead?) 🙇

@abr-egn Please feel free to test the patch locally if you have time 🙏

Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

I think an argument could be made that rustup should just use the last value passed in. I don't think this is clear cut though and could do with some more discussion. For example, rustc only allows one --target (is that a mistake? is --default-host different? I don't know).

View changes since this review

@rami3l
Copy link
Member Author

rami3l commented Mar 16, 2026

I think an argument could be made that rustup should just use the last value passed in.

@ChrisDenton Yeah that's how I did the review 😅 clearly confused rustup with something else; but we could easily revert this if we are going that way.

I'd say I'll merge upon @abr-egn's approval.

Copy link

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

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

This does indeed fix it, thanks for the quick update!

View changes since this review

@rami3l rami3l added this pull request to the merge queue Mar 16, 2026
Merged via the queue into rust-lang:main with commit 4b2c091 Mar 16, 2026
29 checks passed
@rami3l rami3l deleted the fix/rustup-init-cygwin-host-override branch March 16, 2026 20:39
@cachebag
Copy link
Contributor

cachebag commented Mar 16, 2026

should we adopt just that in rustup the binary instead?

@rami3l most CLI tools follow this convention of "last flag wins", as @ChrisDenton notes.

we could just take a 'belt and suspenders' approach to this; #[arg(long, overrides_with = "default_host")] in the RustupInit struct and then your fix here is perfect- now the script is covered and so is the binary. I can open a PR for this if you agree.

For example, rustc only allows one --target (is that a mistake? is --default-host different? I don't know).

@ChrisDenton
It's my understanding that rustc disallowing multiple --target is arguably a design choice specific to compilation, where "which target?" is a critical single-answer question. I feel that --default-host in rustup is a configuration preference, and "last wins" is more user-friendly IMHO.

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.

rustup-init: --default-host is broken under cygwin

4 participants