-
Notifications
You must be signed in to change notification settings - Fork 987
Set RUSTUP_TOOLCHAIN_SOURCE
#4518
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,8 @@ | |
| - `RUSTUP_CONCURRENT_DOWNLOADS` *unstable* (default: the number of components to download). Controls the number of | ||
| downloads made concurrently. | ||
|
|
||
| - `RUSTUP_TOOLCHAIN_SOURCE` *unstable*. Set by rustup to tell proxied tools how `RUSTUP_TOOLCHAIN` was determined. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK, rust-analyzer (and some other tools) overrides My questions are:
Note that this is probably not going to affect general user cases, and Footnotes
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Could your question be restated as, "What are the definitions of the five I would argue they are the same as in the "Overrides" section of the Rustup book. For the sake of your examples, the toolchain would not be specified on the command line,1 so the toolchain should be determined by
My non-authoritative opinion is "no", tools other that rustup should not set I am reminded of a comment @epage made in rust-lang/cargo#15099 (comment):
If non-rustup tools are allowed to set Should we expand the documentation to say that non-rustup tools should not set Footnotes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Given the current definition of I guess my question could be rephrased as: “If a tool caches the rustup-resolved toolchain path for optimization, should it also propagate the original I don’t think we need to overthink this, since rust-analyzer users likely won’t be affected. That said, we should keep in mind that some “rustup” wrappers might produce unexpected or less precise diagnostics if Cargo starts treating Anyway, I agree that we may want to make this clearer in the documentation, especially which environments are meant for reading and which are for writing.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It seems reasonable to answer this with yes. I wonder if we can test this whole thing before we release rustup with this code? I guess it would need some of the relevant Cargo changes (not sure if anyone is working on those already) and then pulling in both as snapshot builds.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO I agreed to include this patch based on the premise that this is a) a private contract between us and cargo; b) a best-effort optimization to communicate with the latter why a toolchain is activated. As such, I am wondering what will the problems caused by overriding that env var be if you are already bypassing rustup's toolchain resolution. So "yes" to the question above, since the results are purely cargo-oriented.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@djc Just to answer your question: rust-lang/cargo#16131 |
||
|
|
||
| [directive syntax]: https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html#directives | ||
| [dc]: https://docs.docker.com/storage/storagedriver/overlayfs-driver/#modifying-files-or-directories | ||
| [override]: overrides.md | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.