-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Stabilize --env-set
option
#119926
Stabilize --env-set
option
#119926
Conversation
Can you link to some discussion confirming that ninja has integrated this flag experimentally and that it works properly? We definitely need confirmation for that first before stabilizing anything. |
ninja? Also we can postpone approving this PR. No hurry here. |
I thought this flag was added for ninja |
Ah I see. No idea. "meson devs" so not sure for what they use it exactly. |
Marking it as waiting on author, you should prepare a stabilization report mentioning the use case again and pointing at build tools that tried to use this successfully. |
for us —set-env alone is a massive improvement, and the main thing we need. We can use the others, but we really need the set command |
@dcbaker have you been able to integrate it and ensure that it solves all your use cases correctly? |
This would be at least useful for meson, which needs something like this because of ninja's lack of support for environment variables. Other ninja-based build systems should benefit from it in the same way. |
I had planned on having a working prototype this weekend, but instead had no power or internet. Hopefully in the next day or so |
Meson with this PR: mesonbuild/meson#10030 demonstrates that Edit: CI failures on that branch are expected, we don't have a nightly compiler in any of the images |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation LGTM, some working changes in the documentation.
Could you write a brief stabilisation report describing what is being stabilised and then I'll start an MCP? |
Thanks for the review @davidtwco! I added the stabilization report in the first comment of this PR. |
@rfcbot fcp merge See stabilization report. |
Team member @davidtwco has proposed to merge this. The next step is review by the rest of the tagged team members:
Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Besides None of those are tracked in depinfo. I wonder how should all of them interact with @rfcbot concern other-rustc-vars |
I have a hard time understanding how this functionality can be useful in the form that only affects the results of the I also have a hard time wrapping my head around why the behaviour of falling back to regular environment variables would be desirable. It seems to me that once you care enough to specify |
☔ The latest upstream changes (presumably #120881) made this pull request unmergeable. Please resolve the merge conflicts. |
If I want to set an environment variable to an empty value, should we allow |
After a discussion with the author, they have preferred this to be closed as they don't have a usecase or motivation for completing it. |
Fixes #118372.
Fixes #80792.
As discussed with meson devs, the option by itself is good enough to be used, so there isn't really a need to wait for rust-lang/rfcs#2794 to stabilize
--env-set
.Summary
This PR stabilizes the
--env-set
option. This option flag allows specification of environment variables at compile time for theenv!
andoption_env!
macros andtracked_env::var
function from theproc_macro
crate.This information will be stored in the dep-info files. For more information about dep-info files, take a look here.
When retrieving an environment variable, the value specified by
--env-set
will takeprecedence. For example, if you want have
PATH=env
in your environment and pass:Then you will have:
Crates will be fully re-compiled if
--env-set
arguments are changed.Tracking issue is #118372 (it contains le list of PRs for implementing it).
Motivation
External tools like
ninja
ormeson
need to be able to specify environment variables through the command line to override current process environment. It also allows to force a recompilation if an environment variable passed with this flag is modified.Use of this feature
It is currently used in mesonbuild/meson#10030.
Tests
List of tests for this option flag:
tests/ui/extenv/extenv-env-overload.rs
tests/ui/extenv/extenv-env.rs
tests/ui/extenv/extenv-not-env.rs
tests/ui/feature-gates/env-flag.rs
tests/ui/proc-macro/auxiliary/env.rs
tests/ui/proc-macro/env.rs
r? @davidtwco