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

Document (and perhaps change) interaction between rust-toolchain.toml and other override sources for components and targets #3483

Open
kpreid opened this issue Sep 19, 2023 · 6 comments · May be fixed by #3492
Assignees
Milestone

Comments

@kpreid
Copy link

kpreid commented Sep 19, 2023

Problem you are trying to solve

It occurred to me that, even though I am using stable Rust, I could use a rust-toolchain.toml file to declare the targets and components my project requires, making it more convenient to use. So, I did this, and also replaced the part of my CI configuration which ran explicit rustup commands with rustup override set "${{ matrix.toolchain }}".

This then failed, revealing to me that rustup override set appears to cause the rust-toolchain.toml file to be completely ignored, even though the file can specify targets and components which rustup override set cannot.

Solution you'd like

  1. Explicitly document the (non-)inheritance behavior at https://rust-lang.github.io/rustup/overrides.html.
  2. If feasible, add a means to request “override the toolchain but install the same targets and components as would otherwise have been installed" (or make that the default behavior if stability policy allows).

Notes

The workaround I've found to get the effect I wanted is

sed -i "s/stable/${{ matrix.toolchain }}/" rust-toolchain.toml
rustup show

which isn't elegant and leaves the working tree modified.

@kpreid kpreid changed the title Document (and perhaps change) override priority behavior for components and targets Document (and perhaps change) interaction between rust-toolchain.toml and other override sources for components and targets Sep 19, 2023
@rami3l
Copy link
Member

rami3l commented Sep 20, 2023

Hello!

  1. Explicitly document the (non-)inheritance behavior at https://rust-lang.github.io/rustup/overrides.html.

The current doc says "The toolchain is chosen in the order listed above..." so I'd expect rustup override to take precedence over rust-toolchain.toml (so indeed the targets and components won't be read anyway). What could possibly be changed to make this suit your expectations even better?

  1. If feasible, add a means to request “override the toolchain but install the same targets and components as would otherwise have been installed" (or make that the default behavior if stability policy allows).

Notes

The workaround I've found to get the effect I wanted is

sed -i "s/stable/${{ matrix.toolchain }}/" rust-toolchain.toml
rustup show

I agree that leaving the directory dirty is not elegant.
However, rust-toolchain.toml is exactly for pinning down a specific rust version to be used in the project (to prevent breakages from, for example, the stabilization of a new nightly feature).
Thus, you generally cannot assume the toolchain that your collaborator is using as long as it is compatible with your codebase. If you are doing this only in the CI to check for nightly compatibility or something similar, your "hack" might also be the best thing to do if I were you.

In order to prevent an XY problem and to better understand your usecase, may I ask what exactly did you want to achieve with this rust-toolchain.toml file, and why did you want to change it in the CI?

Finally, thanks a lot for your report!

@kpreid
Copy link
Author

kpreid commented Sep 20, 2023

The current doc says "The toolchain is chosen in the order listed above..." so I'd expect rustup override to take precedence over rust-toolchain.toml (so indeed the targets and components won't be read anyway). What could possibly be changed to make this suit your expectations even better?

Most configurations of this kind (e.g. .cargo/config.toml) apply on a per-setting basis, not “if this data source exists, the lower-priority ones are completely ignored”. The document doesn't address this interaction at all, so a plausible assumption is that it behaves in this typical way.

Additionally, the rust-toolchain.toml file is the only one that can specify targets and components, so it simply isn't possible to create a higher-priority override while still specifying them, so (given that it doesn't work per-setting) there is a gap in functionality.

Practically, with knowledge of history, the document reads like “nobody adjusted it to explain this interaction after the support for specifying more than a channel name in the file was added”. Hence I experimented to find out which way it went, and was disappointed with the answer.

Regarding changing the documentation, all I want is a sentence, in the introduction or the section on the rust-toolchain.toml file, that describes what happens to targets and components when something higher-priority than the file applies. But of course, I'd also like it to work in the way I hoped it did.

In order to prevent an XY problem and to better understand your usecase, may I ask what exactly did you want to achieve with this rust-toolchain.toml file,

The previous situation:

  • Project has no rust-toolchain.toml file.
  • Project's build process involves compiling to wasm32-unknown-unknown.
  • (And in the future, I might want to test the build against a known no_std target.)
  • CI runs rustup commands to install necessary files.

It occurred to me that creating a rust-toolchain.toml would benefit people who clone the repository and build, because they would get wasm32-unknown-unknown installed automatically rather than not. Generally, it seems to me desirable to place as much information about the project requirements as possible in configuration files that apply to CI and non-CI situations, rather than only CI — and to use declarative over imperative mechanisms (installation on demand rather than as a necessary preparation step).

I don't actually want to pin the toolchain channel at all — my project uses stable Rust and “latest stable” is of course the default assumption. But the place for specifying targets and components is also the place for overriding the channel.

and why did you want to change it in the CI?

My CI builds my project with multiple Rust toolchains so that I can test nightly and beta, because I have found this to be an effective way to discover new Rust bugs and report them before they reach stable.

So, the thing I want here is to be able to separately

  • get auto-installed targets, and
  • override the toolchain

without either of these two configurations defeating each other. And I think that this could be done very straightforwardly by allowing the rest of the file configuration to apply even if the toolchain has been overridden; I don't see how this would ever be undesirable. I recognize that I'm not using this feature as originally imagined; what I'm proposing is that it be slightly adjusted to be useful in more scenarios.

@rami3l
Copy link
Member

rami3l commented Sep 21, 2023

@kpreid Thanks for your detailed explanation!

cargo explicitly mentions a hierarchical config system in its docs so it is indeed expected to perform the fallback that you wish to observe. I see that in your particular use case, the current behavior of the rustup toolchain override config system is disappointing, and I would be glad to make it hierarchical as well.

@rbtcollins will this potential change break our compatibility promises?

@rami3l
Copy link
Member

rami3l commented Sep 23, 2023

Claiming this issue on @rbtcollins' approval of implementing this idea.

@rami3l rami3l linked a pull request Sep 25, 2023 that will close this issue
5 tasks
@rami3l rami3l self-assigned this Oct 5, 2023
rami3l added a commit to rami3l/rustup that referenced this issue Oct 10, 2023
@rami3l
Copy link
Member

rami3l commented Oct 10, 2023

Possibly related: rust-lang/cargo#11957

@polarathene
Copy link

Just for awareness, I ran into an issue with a minimal profile configured in rustup, but when a project had a rust-toolchain.toml file, the missing toolchain was installed with the default profile, bringing in excess components including over 600MB of HTML docs: #3805

That's probably worth documenting if intentional. Otherwise hopefully something that'll be addressed as a bug fix 😅

@rami3l rami3l added this to the 1.28.0 milestone May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants