Add option to pass environment variables #94387

beepster4096 · 2022-02-26T00:43:34Z

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.

rust-highfive · 2022-02-26T00:43:37Z

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

bjorn3 · 2022-02-27T09:28:47Z

It seems like this doesn't affect proc macros in any way.

beepster4096 · 2022-02-27T11:16:54Z

Huh, I completely forgot about proc macros. Looks like I'll need to figure that out too.

bjorn3 · 2022-02-27T11:27:15Z

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.

Mark-Simulacrum · 2022-02-27T13:29:43Z

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).

bjorn3 · 2022-02-27T14:09:47Z

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)

apiraino · 2022-03-30T20:07:38Z

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

bors · 2022-04-09T16:04:18Z

☔ The latest upstream changes (presumably #95697) made this pull request unmergeable. Please resolve the merge conflicts.

JohnCSimon · 2022-04-23T07:22:25Z

Triage:
@drmeepster - can you please post the status of this PR? Thank you

beepster4096 · 2022-04-24T06:25:41Z

Got side-tracked by other work. I should have it updated in around a few days.

beepster4096 · 2022-04-27T04:04:59Z

Alright, proc_macro::tracked_env::var now works with this option.

@rustbot ready

src/test/run-make/env-dep-info/Makefile

wesleywiser · 2022-06-23T14:14:22Z

compiler/rustc_session/src/options.rs

@@ -230,6 +232,9 @@ top_level_options!(

        /// The (potentially remapped) working directory
        working_dir: RealFileName [TRACKED],
+
+        /// Overridden env vars used for `env!` and `option_env!`
+        injected_env_vars: FxHashMap<String, String> [UNTRACKED],


I think this needs to be [TRACKED] as changing the provided env vars should cause code which uses them to be recompiled.

Shouldn't the tracking be based on the actual use of the env vars? If an env var is specified but the crate never references it, changing it shouldn't require a crate recompile.

jyn514 · 2022-06-23T14:16:13Z

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.

bjorn3 · 2022-06-23T14:17:56Z

compiler/rustc_builtin_macros/src/env.rs

+        cx.sess.opts.injected_env_vars.get(var.as_str()).map(|s| Symbol::intern(s));
+    let value =
+        injected_value.or_else(|| env::var(var.as_str()).ok().as_deref().map(Symbol::intern));
+
    cx.sess.parse_sess.env_depinfo.borrow_mut().insert((var, value));


I think this should be skipped in case --env mentions this env var. Otherwise changes to the actual env vars will cause a recompilation, even if they don't affect compilation at all.

bjorn3 · 2022-06-23T14:20:02Z

library/proc_macro/src/lib.rs

@@ -1287,7 +1287,10 @@ pub mod tracked_env {
    #[unstable(feature = "proc_macro_tracked_env", issue = "74690")]
    pub fn var<K: AsRef<OsStr> + AsRef<str>>(key: K) -> Result<String, VarError> {
        let key: &str = key.as_ref();
-        let value = env::var(key);
+        let injected_value = crate::bridge::client::FreeFunctions::injected_env_var(key);
+        let env_value = env::var(key);


I think env::var should only be called if injected_env_var returned None.

In my take on this - https://github.com/jsgf/rust/tree/env-sandbox - I just initialized the logical environment (equiv of injected vars) with all the env vars, and never used env::var after initializing it. I think it's a more consistent approach. In particular, it allows you to logically delete vars from the environment.

jyn514 · 2022-06-23T14:23:15Z

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.

bjorn3 · 2022-06-23T14:24:16Z

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.

jyn514 · 2022-06-23T14:29:47Z

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?

bjorn3 · 2022-06-23T17:01:02Z

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.

bors · 2022-07-27T21:38:51Z

☔ The latest upstream changes (presumably #99816) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2022-08-01T07:51:20Z

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

jyn514 · 2022-08-01T13:35:29Z

That is going to be a lot of work

We'll need to do that work anyway to support parallel_compiler, though, right?

bjorn3 · 2022-08-02T20:23:15Z

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.

estebank · 2022-08-05T13:30:06Z

r? @jyn514

jyn514 · 2022-08-06T23:13:55Z

I think this needs an MCP. @drmeepster can you open an issue on rust-lang/compiler-team?

JohnCSimon · 2022-09-11T05:53:46Z

@drmeepster

jyn514 commented on Aug 6
I think this needs an MCP. @drmeepster can you open an issue on rust-lang/compiler-team?

If you have created this already, please link it. Thank you.

jyn514 · 2022-10-01T21:05:26Z

I'm going to close this for now. Feel free to re-open whenever you have time for an MCP.

rust-highfive assigned estebank Feb 26, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 26, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 26, 2022

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2022

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 30, 2022

beepster4096 force-pushed the inject_env branch from 090788f to 7f4eb38 Compare April 27, 2022 02:34

This comment has been minimized.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 27, 2022

bjorn3 reviewed Apr 27, 2022

View reviewed changes

src/test/run-make/env-dep-info/Makefile Outdated Show resolved Hide resolved

wesleywiser reviewed Jun 23, 2022

View reviewed changes

bjorn3 reviewed Jun 23, 2022

View reviewed changes

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2022

beepster4096 added 5 commits August 1, 2022 00:50

add --env option

f64a0b3

add support for proc macros

53a64d2

add --env to env-dep-info test

34d71d9

fix the test

5fad881

fix test

223942d

beepster4096 force-pushed the inject_env branch from 114f0f0 to 223942d Compare August 1, 2022 07:51

This comment has been minimized.

Sign in to view

add method to rust-analyzer macro server

8a9edfd

beepster4096 force-pushed the inject_env branch from c405b41 to 8a9edfd Compare August 1, 2022 08:21

rust-highfive assigned jyn514 and unassigned estebank Aug 5, 2022

jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2022

JohnCSimon added S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. and removed S-waiting-on-ACP Status: PR has an ACP and is waiting for the ACP to complete. labels Sep 11, 2022

jyn514 closed this Oct 1, 2022

jyn514 added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Oct 1, 2022

GuillaumeGomez mentioned this pull request Jul 13, 2023

Add option to pass environment variables rust-lang/compiler-team#653

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to pass environment variables #94387

Add option to pass environment variables #94387

beepster4096 commented Feb 26, 2022

rust-highfive commented Feb 26, 2022

bjorn3 commented Feb 27, 2022

beepster4096 commented Feb 27, 2022

bjorn3 commented Feb 27, 2022

Mark-Simulacrum commented Feb 27, 2022

bjorn3 commented Feb 27, 2022 •

edited

Loading

apiraino commented Mar 30, 2022

bors commented Apr 9, 2022

JohnCSimon commented Apr 23, 2022

beepster4096 commented Apr 24, 2022

This comment has been minimized.

beepster4096 commented Apr 27, 2022

wesleywiser Jun 23, 2022

jsgf Oct 2, 2023

jyn514 commented Jun 23, 2022

bjorn3 Jun 23, 2022 •

edited

Loading

bjorn3 Jun 23, 2022 •

edited

Loading

jsgf Oct 2, 2023 •

edited

Loading

jyn514 commented Jun 23, 2022 •

edited

Loading

bjorn3 commented Jun 23, 2022

jyn514 commented Jun 23, 2022

bjorn3 commented Jun 23, 2022

bors commented Jul 27, 2022

rustbot commented Aug 1, 2022

This comment has been minimized.

This comment has been minimized.

jyn514 commented Aug 1, 2022

bjorn3 commented Aug 2, 2022

estebank commented Aug 5, 2022

jyn514 commented Aug 6, 2022

JohnCSimon commented Sep 11, 2022

jyn514 commented Oct 1, 2022

Add option to pass environment variables #94387

Add option to pass environment variables #94387

Conversation

beepster4096 commented Feb 26, 2022

rust-highfive commented Feb 26, 2022

bjorn3 commented Feb 27, 2022

beepster4096 commented Feb 27, 2022

bjorn3 commented Feb 27, 2022

Mark-Simulacrum commented Feb 27, 2022

bjorn3 commented Feb 27, 2022 • edited Loading

apiraino commented Mar 30, 2022

bors commented Apr 9, 2022

JohnCSimon commented Apr 23, 2022

beepster4096 commented Apr 24, 2022

This comment has been minimized.

beepster4096 commented Apr 27, 2022

wesleywiser Jun 23, 2022

Choose a reason for hiding this comment

jsgf Oct 2, 2023

Choose a reason for hiding this comment

jyn514 commented Jun 23, 2022

bjorn3 Jun 23, 2022 • edited Loading

Choose a reason for hiding this comment

bjorn3 Jun 23, 2022 • edited Loading

Choose a reason for hiding this comment

jsgf Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

jyn514 commented Jun 23, 2022 • edited Loading

bjorn3 commented Jun 23, 2022

jyn514 commented Jun 23, 2022

bjorn3 commented Jun 23, 2022

bors commented Jul 27, 2022

rustbot commented Aug 1, 2022

This comment has been minimized.

This comment has been minimized.

jyn514 commented Aug 1, 2022

bjorn3 commented Aug 2, 2022

estebank commented Aug 5, 2022

jyn514 commented Aug 6, 2022

JohnCSimon commented Sep 11, 2022

jyn514 commented Oct 1, 2022

bjorn3 commented Feb 27, 2022 •

edited

Loading

bjorn3 Jun 23, 2022 •

edited

Loading

bjorn3 Jun 23, 2022 •

edited

Loading

jsgf Oct 2, 2023 •

edited

Loading

jyn514 commented Jun 23, 2022 •

edited

Loading