rust
rust copied to clipboard
rust-lang
Add option to pass environment variables
Closes #80792. Adds the unstable --env VAR=VALUE option, which injects environment variables into the program, allowing them to be read by the env! and option_env! macros.
r? @estebank
(rust-highfive has picked a reviewer for you, use r? to override)
Huh, I completely forgot about proc macros. Looks like I'll need to figure that out too.
That one may be hard to solve "correctly" as environment variables are process global and it is possible to run multiple rustc instances in the same process when using an unstable api.
We should be able to solve it correctly if the proc_macro is using https://doc.rust-lang.org/nightly/proc_macro/tracked_env/fn.var.html, right? That API is unstable right now, but it's a start, and stabilizing it might not be that hard (this could be an additional piece of motivation).
Indeed. It may take a while for proc macros to migrate to it though. (Unless we clear the process env for proc macros when using the rustc binary)
by reading the comments, it seems there's still a bit of work to do before another round of review, so I'll flip the switch (but feel free to re-request a review if it's the case)
@rustbot author
:umbrella: The latest upstream changes (presumably #95697) made this pull request unmergeable. Please resolve the merge conflicts.
Triage: @DrMeepster - can you please post the status of this PR? Thank you
Got side-tracked by other work. I should have it updated in around a few days.
The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Compiling rustc-std-workspace-std v1.99.0 (/checkout/library/rustc-std-workspace-std)
Compiling proc_macro v0.0.0 (/checkout/library/proc_macro)
Compiling unicode-width v0.1.8
Compiling getopts v0.2.21
error[E0277]: expected a `FnOnce<()>` closure, found `Result<String, VarError>`
--> library/proc_macro/src/lib.rs:1293:48
|
1293 | let value = injected_value.map_or_else(env_value, Ok);
| ----------- ^^^^^^^^^ expected an `FnOnce<()>` closure, found `Result<String, VarError>`
| required by a bound introduced by this call
|
|
= help: the trait `FnOnce<()>` is not implemented for `Result<String, VarError>`
= note: wrap the `Result<String, VarError>` in a closure with no arguments: `|| { /* code */ }`
note: required by a bound in `Option::<T>::map_or_else`
|
|
996 | D: ~const FnOnce() -> U,
| ^^^^^^^^^^^^^^^^^^^^ required by this bound in `Option::<T>::map_or_else`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `proc_macro` due to previous error
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:00:35
Alright, proc_macro::tracked_env::var now works with this option.
@rustbot ready
Having environment variables differ for proc_macros between std::env and tracked_env seems ... pretty bad. Is there any way we can make this consistent? I worry this is going to lead to weird bugs in the ecosystem.
Having environment variables differ for proc_macros between std::env and tracked_env seems ... pretty bad
Hmm, what I would really love is for all accesses to env variables to be tracked - can we kill two birds with one stone and have std::env use tracked_env internally when it detects that it's running as a proc macro somehow? That would also let us land tracked_env without an FCP for the new API.
Is there any way we can make this consistent?
Env vars are global to the process (and not the compilation session) and as recent discussion has shown can't be written after any thread has been spawned (as is the case for rustc). Maybe we could prevent usage of std::env::var entirely for a given thread using an unstable hook in libstd. Rust-analyzer can't depend on this, so having an unstable hook to override env vars for specific threads is not really an option.
Rust-analyzer can't depend on this, so having an unstable hook to override env vars for specific threads is not really an option.
I don't follow, sorry - the hook idea sounds great to me, why would it break rust-analyzer?
Rust-analyzer is compiled with stable rustc. This means it can't depend on an unstable hook in libstd. If the hook is used in any other way than to prevent std::env::var from being called in the first place, proc macros will keep using std::env::var without any hooks when loaded in rust-analyzer. If on the other hand std::env::var panics, proc macros will be steered towards tracked_env::var which rust-analyzer can easily intercept.
I mean, that's true and gets everything working in the short term (if you don't count the breakage it causes before people update their macros). But it means proc macros have to change, and we end up with proc macros looking even more special. I think using the hook is a better end goal, even if it's not clear how to resolve the stability concerns - I don't think we should prioritize the RA implementation above the language itself.
Hooks will misbehave for background threads spawned by proc macros (there is no way to associate said background threads with the right rustc session), while tracked_env::var will panic if it isn't called on the main thread of the proc macro without exceptions.
IMO the fix for that is for rustc_sesssion to stop using thread locals ... Isn't there already some solution for this when parallel_compiler is enabled?
IMO the fix for that is for rustc_sesssion to stop using thread locals
What else can it do to support multiple rustc sessions in the same process?
Isn't there already some solution for this when parallel_compiler is enabled?
For parallel_compiler rustc-rayon is responsible for spawning new threads. This fork of rayon allows setting thread local variables right after they are spawned by rayon to copy the main rustc thread values. Using std::thread::spawn manually will misbehave too.
What else can it do to support multiple rustc sessions in the same process?
Why do we want to support this?
also though, why do we have code accessing global (currently thread-local) variables? AFAIK this is mostly used for Debug impls ... can we make those implement a custom trait instead?
I know all these suggestions are a lot of work, but I would rather do a lot of work improving our compiler implementation than make the language harder to use.
Why do we want to support this?
In the past RLS did this. I also got a use case for it: To implement hot code swapping in cg_clif by invoking rustc and passing a special CodegenBackend value that replaces functions in the specified JITModule (and thus requires every rustc invocation to be in the same process). This hot code swapping support is on an old side branch, but I want to rebase it someday.
also though, why do we have code accessing global (currently thread-local) variables? AFAIK this is mostly used for Debug impls ... can we make those implement a custom trait instead?
That is going to be a lot of work to replace pretty much every Debug impl with an impl of this trait. It would mean losing the ability to do for example bug!("some error with {:?}", the_error) or dbg!(ty) as those require Debug to be implemented. And finally the query system itself requires a thread local variable to keep track of the queries the current thread is evaluation.
:umbrella: The latest upstream changes (presumably #99816) made this pull request unmergeable. Please resolve the merge conflicts.
Hey! It looks like you've submitted a new PR for the library teams!
If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.
Examples of T-libs-api changes:
- Stabilizing library features
- Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
- Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
- Changing public documentation in ways that create new stability guarantees
- Changing observable runtime behavior of library APIs
Some changes occurred in library/proc_macro/src/bridge
cc @rust-lang/wg-rls-2
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
error[E0046]: not all trait items implemented, missing: `injected_env_var`
--> crates/proc-macro-srv/src/abis/abi_sysroot/ra_server.rs:74:1
|
74 | impl server::FreeFunctions for RustAnalyzer {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `injected_env_var` in implementation
|
= help: implement the missing item: `fn injected_env_var(&mut self, _: &str) -> Option<std::string::String> { todo!() }`
Checking hir-def v0.0.0 (/checkout/src/tools/rust-analyzer/crates/hir-def)
For more information about this error, try `rustc --explain E0046`.
error: could not compile `proc-macro-srv` due to previous error
warning: build failed, waiting for other jobs to finish...
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking hir-def v0.0.0 (/checkout/src/tools/rust-analyzer/crates/hir-def)
error: unused variable: `var`
--> crates/proc-macro-srv/src/abis/abi_sysroot/ra_server.rs:75:36
|
75 | fn injected_env_var(&mut self, var: &str) -> Option<String> {
| ^^^ help: if this is intentional, prefix it with an underscore: `_var`
|
= note: `-D unused-variables` implied by `-D warnings`
error: could not compile `proc-macro-srv` due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `proc-macro-srv` due to previous error
Build completed unsuccessfully in 0:03:23
That is going to be a lot of work
We'll need to do that work anyway to support parallel_compiler, though, right?
That is going to be a lot of work
We'll need to do that work anyway to support parallel_compiler, though, right?
All threads of a compilation session inherit the tls variables thanks to our fork of rayon having support for a hook that runs just after spawning a new worker thread and before running any jobs on it. This means that all Debug impls should already work with parallel_rustc.
I think this needs an MCP. @DrMeepster can you open an issue on rust-lang/compiler-team?