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

Add option to pass environment variables #94387

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions compiler/rustc_builtin_macros/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ pub fn expand_option_env<'cx>(
};

let sp = cx.with_def_site_ctxt(sp);
let value = env::var(var.as_str()).ok().as_deref().map(Symbol::intern);

let injected_value =
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));
Copy link
Member

@bjorn3 bjorn3 Jun 23, 2022

Choose a reason for hiding this comment

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

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.

let e = match value {
None => {
Expand Down Expand Up @@ -78,7 +83,11 @@ pub fn expand_env<'cx>(
}

let sp = cx.with_def_site_ctxt(sp);
let value = env::var(var.as_str()).ok().as_deref().map(Symbol::intern);

let key = var.as_str();
let injected_value = cx.sess.opts.injected_env_vars.get(key).map(|s| Symbol::intern(s));
let value = injected_value.or_else(|| env::var(key).ok().as_deref().map(Symbol::intern));

cx.sess.parse_sess.env_depinfo.borrow_mut().insert((var, value));
let e = match value {
None => {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_expand/src/proc_macro_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,10 @@ impl server::Types for Rustc<'_, '_> {
}

impl server::FreeFunctions for Rustc<'_, '_> {
fn injected_env_var(&mut self, var: &str) -> Option<String> {
self.ecx.sess.opts.injected_env_vars.get(var).cloned()
}

fn track_env_var(&mut self, var: &str, value: Option<&str>) {
self.sess()
.env_depinfo
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,7 @@ impl Default for Options {
json_future_incompat: false,
pretty: None,
working_dir: RealFileName::LocalPath(std::env::current_dir().unwrap()),
injected_env_vars: Default::default(),
}
}
}
Expand Down Expand Up @@ -1429,6 +1430,7 @@ pub fn rustc_optgroups() -> Vec<RustcOptGroup> {
"Remap source names in all output (compiler messages and output files)",
"FROM=TO",
),
opt::multi("", "env", "Inject an environment variable", "VAR=VALUE"),
]);
opts
}
Expand Down Expand Up @@ -2197,6 +2199,23 @@ fn parse_remap_path_prefix(
mapping
}

fn parse_injected_env_vars(
matches: &getopts::Matches,
error_format: ErrorOutputType,
) -> FxHashMap<String, String> {
let mut vars = FxHashMap::default();

for arg in matches.opt_strs("env") {
if let Some((name, val)) = arg.split_once('=') {
vars.insert(name.to_string(), val.to_string());
} else {
early_error(error_format, &format!("`--env`: specify value for variable `{}`", arg));
}
}

vars
}

// JUSTIFICATION: before wrapper fn is available
#[cfg_attr(not(bootstrap), allow(rustc::bad_opt_access))]
pub fn build_session_options(matches: &getopts::Matches) -> Options {
Expand Down Expand Up @@ -2470,6 +2489,8 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
RealFileName::LocalPath(path)
};

let injected_env_vars = parse_injected_env_vars(matches, error_format);

Options {
assert_incr_state,
crate_types,
Expand Down Expand Up @@ -2506,6 +2527,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
json_future_incompat,
pretty,
working_dir,
injected_env_vars,
}
}

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use rustc_target::spec::{
RelocModel, RelroLevel, SplitDebuginfo, StackProtector, TargetTriple, TlsModel,
};

use rustc_data_structures::fx::FxHashMap;

use rustc_feature::UnstableFeatures;
use rustc_span::edition::Edition;
use rustc_span::RealFileName;
Expand Down Expand Up @@ -210,6 +212,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],
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

}
);

Expand Down
1 change: 1 addition & 0 deletions library/proc_macro/src/bridge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ macro_rules! with_api {
$m! {
FreeFunctions {
fn drop($self: $S::FreeFunctions);
fn injected_env_var(var: &str) -> Option<String>;
fn track_env_var(var: &str, value: Option<&str>);
fn track_path(path: &str);
fn literal_from_str(s: &str) -> Result<Literal<$S::Span, $S::Symbol>, ()>;
Expand Down
5 changes: 4 additions & 1 deletion library/proc_macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1505,7 +1505,10 @@ pub mod tracked_env {
#[unstable(feature = "proc_macro_tracked_env", issue = "99515")]
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);
Copy link
Member

@bjorn3 bjorn3 Jun 23, 2022

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@jsgf jsgf Oct 2, 2023

Choose a reason for hiding this comment

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

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.


let value = injected_value.map_or(env_value, Ok);
crate::bridge::client::FreeFunctions::track_env_var(key, value.as_deref().ok());
value
}
Expand Down
8 changes: 6 additions & 2 deletions src/test/run-make/env-dep-info/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,17 @@ ADDITIONAL_ARGS := $(RUSTFLAGS)
endif

all:
EXISTING_ENV=1 EXISTING_OPT_ENV=1 $(RUSTC) --emit dep-info main.rs
EXISTING_ENV=1 EXISTING_OPT_ENV=1 OVERRIDEN_ENV=0 $(RUSTC) --emit dep-info -Zunstable-options --env INJECTED_ENV=1 --env OVERRIDEN_ENV=1 main.rs
$(CGREP) "# env-dep:EXISTING_ENV=1" < $(TMPDIR)/main.d
$(CGREP) "# env-dep:EXISTING_OPT_ENV=1" < $(TMPDIR)/main.d
$(CGREP) "# env-dep:NONEXISTENT_OPT_ENV" < $(TMPDIR)/main.d
$(CGREP) "# env-dep:ESCAPE\nESCAPE\\" < $(TMPDIR)/main.d
$(CGREP) "# env-dep:INJECTED_ENV=1" < $(TMPDIR)/main.d
$(CGREP) "# env-dep:OVERRIDEN_ENV=1" < $(TMPDIR)/main.d
# Proc macro
$(BARE_RUSTC) $(ADDITIONAL_ARGS) --out-dir $(TMPDIR) macro_def.rs
EXISTING_PROC_MACRO_ENV=1 $(RUSTC) --emit dep-info macro_use.rs
EXISTING_PROC_MACRO_ENV=1 OVERRIDEN_PROC_MACRO_ENV=0 $(RUSTC) --emit dep-info -Zunstable-options --env INJECTED_PROC_MACRO_ENV=1 --env OVERRIDEN_PROC_MACRO_ENV=1 macro_use.rs
$(CGREP) "# env-dep:EXISTING_PROC_MACRO_ENV=1" < $(TMPDIR)/macro_use.d
$(CGREP) "# env-dep:NONEXISTENT_PROC_MACEO_ENV" < $(TMPDIR)/macro_use.d
$(CGREP) "# env-dep:INJECTED_PROC_MACRO_ENV=1" < $(TMPDIR)/macro_use.d
$(CGREP) "# env-dep:OVERRIDEN_PROC_MACRO_ENV=1" < $(TMPDIR)/macro_use.d
2 changes: 2 additions & 0 deletions src/test/run-make/env-dep-info/macro_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ use proc_macro::*;
pub fn access_env_vars(_: TokenStream) -> TokenStream {
let _ = tracked_env::var("EXISTING_PROC_MACRO_ENV");
let _ = tracked_env::var("NONEXISTENT_PROC_MACEO_ENV");
let _ = tracked_env::var("INJECTED_PROC_MACRO_ENV");
let _ = tracked_env::var("OVERRIDEN_PROC_MACRO_ENV");
TokenStream::new()
}
2 changes: 2 additions & 0 deletions src/test/run-make/env-dep-info/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ fn main() {
option_env!("EXISTING_OPT_ENV");
option_env!("NONEXISTENT_OPT_ENV");
option_env!("ESCAPE\nESCAPE\\");
env!("INJECTED_ENV");
env!("OVERRIDEN_ENV");
}
8 changes: 8 additions & 0 deletions src/test/ui/extenv/extenv-injected.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// run-pass
// rustc_env: overridden=no
// compile-flags: -Z unstable-options --env x=y --env overridden=yes

fn main() {
assert_eq!(env!("x"), "y");
assert_eq!(env!("overridden"), "yes");
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ impl server::Types for RustAnalyzer {
}

impl server::FreeFunctions for RustAnalyzer {
fn injected_env_var(&mut self, _var: &str) -> Option<String> {
None
}

fn track_env_var(&mut self, _var: &str, _value: Option<&str>) {
// FIXME: track env var accesses
// https://github.com/rust-lang/rust/pull/71858
Expand Down