Add environment variable query #130883

madsmtm · 2024-09-26T15:10:05Z

Generally, rustc prefers command-line arguments, but in some cases, an environment variable really is the most sensible option. We should make sure that this works properly with the compiler's change-tracking mechanisms, such that changing the relevant environment variable causes a rebuild.

This PR is a first step forwards in doing that.

Part of the work needed to do #118204, see #129342 for some discussion.

r? @petrochenkov

compiler/rustc_interface/src/passes.rs

madsmtm · 2024-09-26T15:11:37Z

compiler/rustc_interface/src/passes.rs

@@ -336,6 +337,20 @@ fn early_lint_checks(tcx: TyCtxt<'_>, (): ()) {
    )
 }

+fn env_var(tcx: TyCtxt<'_>, key: Symbol) -> Option<Symbol> {
+    let var = match std::env::var(key.as_str()) {


Is there something similar to Clippy's disallowed-methods that I could use to lint against usage of std::env::var in the compiler, and direct users towards the query instead?

See also rust-lang/cargo#11588.

There's some rustc-internal lints in https://github.com/rust-lang/rust/blob/master/compiler/rustc_lint/src/internal.rs

compiler/rustc_interface/src/passes.rs

compiler/rustc_borrowck/src/nll.rs

compiler/rustc_middle/src/query/mod.rs

…g, r=petrochenkov Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa` Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc. We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion. The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session. Tested with the same commands as in rust-lang#130435. r? `@petrochenkov`

…g, r=petrochenkov Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa` Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc. We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion. The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session. Tested with the same commands as in rust-lang#130435. r? ```@petrochenkov```

…g, r=petrochenkov Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa` Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc. We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion. The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session. Tested with the same commands as in rust-lang#130435. r? ````@petrochenkov````

…g, r=petrochenkov Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa` Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc. We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion. The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session. Tested with the same commands as in rust-lang#130435. r? `````@petrochenkov`````

…g, r=petrochenkov Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa` Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc. We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion. The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session. Tested with the same commands as in rust-lang#130435. r? ``````@petrochenkov``````

Rollup merge of rust-lang#131037 - madsmtm:move-llvm-target-versioning, r=petrochenkov Move versioned Apple LLVM targets from `rustc_target` to `rustc_codegen_ssa` Fully specified LLVM targets contain the OS version on macOS/iOS/tvOS/watchOS/visionOS, and this version depends on the deployment target environment variables like `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET` etc. We would like to move this to later in the compilation pipeline, both because it feels impure to access environment variables when fetching target information, but mostly because we need access to more information from rust-lang#130883 to do rust-lang#118204. See also rust-lang#129342 (comment) for some discussion. The first and second commit does the actual refactor, it should be a non-functional change, the third commit adds diagnostics for invalid deployment targets, which are now possible to do because we have access to the session. Tested with the same commands as in rust-lang#130435. r? ``````@petrochenkov``````

madsmtm · 2024-11-16T18:23:48Z

@rustbot ready

Note that I've also added a test that env!/option_env! works with incremental compilation, I felt it was relevant to this PR, but feel free to tell me to do it in a different one.

petrochenkov · 2025-02-19T16:20:07Z

Still waiting on the author to address the review.
@madsmtm Do you plan to return to this?

madsmtm · 2025-02-19T17:01:12Z

Yeah, I think I got a bit discouraged by #130883 (comment), so I feared that we wouldn't be able to land it because of disagreements. But I seem to have misremembered, so thanks for the ping, I'll try to pick it up one of the coming days.

This currently works because it's part of expansion, and that isn't yet tracked by the query system. But we want to ensure it continues working, even if that is changed.

madsmtm · 2025-03-26T13:53:11Z

Finally got around to rebasing and addressing your review comments, sorry for the long wait.

@rustbot ready

compiler/rustc_borrowck/src/nll.rs

compiler/rustc_interface/src/passes.rs

petrochenkov · 2025-03-26T14:39:02Z

r=me with the remaining nits addressed.
@rustbot author

Along with `TyCtx::env_var` helper. These can be used to track environment variable accesses in the query system. Since `TyCtx::env_var_os` uses `OsStr`, this commit also adds the necessary trait implementations for that to work.

This won't work with Cargo's change tracking, but it should work with incremental.

madsmtm · 2025-03-26T14:49:27Z

Done!

@rustbot ready

petrochenkov · 2025-03-26T14:50:25Z

Thanks!
@bors r+

bors · 2025-03-26T14:50:29Z

📌 Commit 632ce38 has been approved by petrochenkov

It is now in the queue for this repository.

Add environment variable query Generally, `rustc` prefers command-line arguments, but in some cases, an environment variable really is the most sensible option. We should make sure that this works properly with the compiler's change-tracking mechanisms, such that changing the relevant environment variable causes a rebuild. This PR is a first step forwards in doing that. Part of the work needed to do rust-lang#118204, see rust-lang#129342 for some discussion. r? `@petrochenkov`

Rollup of 9 pull requests Successful merges: - rust-lang#130883 (Add environment variable query) - rust-lang#138672 (Avoiding calling queries when collecting active queries) - rust-lang#138702 (Allow spawning threads after TLS destruction) - rust-lang#138935 (Update wg-prio triagebot config) - rust-lang#138946 (Un-bury chapters from the chapter list in rustc book) - rust-lang#138964 (Implement lint against using Interner and InferCtxtLike in random compiler crates) - rust-lang#138977 (Don't deaggregate InvocationParent just to reaggregate it again) - rust-lang#138980 (Collect items referenced from var_debug_info) - rust-lang#138985 (Use the correct binder scope for elided lifetimes in assoc consts) r? `@ghost` `@rustbot` modify labels: rollup

Add environment variable query Generally, `rustc` prefers command-line arguments, but in some cases, an environment variable really is the most sensible option. We should make sure that this works properly with the compiler's change-tracking mechanisms, such that changing the relevant environment variable causes a rebuild. This PR is a first step forwards in doing that. Part of the work needed to do rust-lang#118204, see rust-lang#129342 for some discussion. r? ``@petrochenkov``

Rollup of 10 pull requests Successful merges: - rust-lang#130883 (Add environment variable query) - rust-lang#138624 (Add mipsel maintainer) - rust-lang#138672 (Avoiding calling queries when collecting active queries) - rust-lang#138935 (Update wg-prio triagebot config) - rust-lang#138946 (Un-bury chapters from the chapter list in rustc book) - rust-lang#138964 (Implement lint against using Interner and InferCtxtLike in random compiler crates) - rust-lang#138977 (Don't deaggregate InvocationParent just to reaggregate it again) - rust-lang#138980 (Collect items referenced from var_debug_info) - rust-lang#138985 (Use the correct binder scope for elided lifetimes in assoc consts) - rust-lang#138987 (Always emit `native-static-libs` note, even if it is empty) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned petrochenkov Sep 26, 2024

rustbot added S-waiting-on-review T-compiler labels Sep 26, 2024

madsmtm commented Sep 26, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

madsmtm force-pushed the env-var-query branch from 506b0df to af26a40 Compare September 26, 2024 19:43

This comment has been minimized.

Sign in to view

madsmtm force-pushed the env-var-query branch 2 times, most recently from dee9b61 to 0ef9570 Compare September 27, 2024 14:06

This comment has been minimized.

Sign in to view

madsmtm force-pushed the env-var-query branch from 0ef9570 to 455e468 Compare September 27, 2024 15:56

This comment has been minimized.

Sign in to view

madsmtm force-pushed the env-var-query branch from 455e468 to 46276c8 Compare September 27, 2024 16:10

madsmtm commented Sep 27, 2024

View reviewed changes

compiler/rustc_interface/src/passes.rs Outdated Show resolved Hide resolved

madsmtm mentioned this pull request Sep 29, 2024

Move versioned Apple LLVM targets from rustc_target to rustc_codegen_ssa #131037

Merged

petrochenkov reviewed Oct 22, 2024

View reviewed changes

compiler/rustc_borrowck/src/nll.rs Outdated Show resolved Hide resolved

compiler/rustc_middle/src/query/mod.rs Outdated Show resolved Hide resolved

compiler/rustc_middle/src/query/mod.rs Show resolved Hide resolved

petrochenkov added S-waiting-on-author and removed S-waiting-on-review labels Oct 22, 2024

This comment was marked as resolved.

Sign in to view

petrochenkov mentioned this pull request Nov 10, 2024

Rebuild on changes to the deployment target when compiling Apple targets #129342

Open

madsmtm force-pushed the env-var-query branch from 46276c8 to 916e404 Compare November 16, 2024 18:15

rustbot added S-waiting-on-review and removed S-waiting-on-author labels Nov 16, 2024

petrochenkov added S-waiting-on-author and removed S-waiting-on-review labels Dec 1, 2024

This comment was marked as resolved.

Sign in to view

Test that env! works with incremental compilation

c772573

This currently works because it's part of expansion, and that isn't yet tracked by the query system. But we want to ensure it continues working, even if that is changed.

madsmtm force-pushed the env-var-query branch from 60f18ba to 8b0095a Compare March 26, 2025 13:52

rustbot added S-waiting-on-review and removed S-waiting-on-author labels Mar 26, 2025

petrochenkov reviewed Mar 26, 2025

View reviewed changes

compiler/rustc_borrowck/src/nll.rs Outdated Show resolved Hide resolved

compiler/rustc_interface/src/passes.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Mar 26, 2025

madsmtm added 2 commits March 26, 2025 15:46

Add TyCtx::env_var_os

17db054

Along with `TyCtx::env_var` helper. These can be used to track environment variable accesses in the query system. Since `TyCtx::env_var_os` uses `OsStr`, this commit also adds the necessary trait implementations for that to work.

Add environment variable tracking in places where it was convenient

632ce38

This won't work with Cargo's change tracking, but it should work with incremental.

madsmtm force-pushed the env-var-query branch from 8b0095a to 632ce38 Compare March 26, 2025 14:47

rustbot added S-waiting-on-review and removed S-waiting-on-author labels Mar 26, 2025

bors added S-waiting-on-bors and removed S-waiting-on-review labels Mar 26, 2025

TaKO8Ki mentioned this pull request Mar 26, 2025

Rollup of 9 pull requests #138994

Closed

Zalathar mentioned this pull request Mar 27, 2025

Rollup of 10 pull requests #139012

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add environment variable query #130883

Add environment variable query #130883

madsmtm commented Sep 26, 2024

madsmtm Sep 26, 2024

clubby789 Sep 28, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment was marked as resolved.

madsmtm commented Nov 16, 2024 •

edited

Loading

This comment was marked as resolved.

petrochenkov commented Feb 19, 2025

madsmtm commented Feb 19, 2025

madsmtm commented Mar 26, 2025 •

edited

Loading

petrochenkov commented Mar 26, 2025

madsmtm commented Mar 26, 2025

petrochenkov commented Mar 26, 2025

bors commented Mar 26, 2025

Add environment variable query #130883

Are you sure you want to change the base?

Add environment variable query #130883

Conversation

madsmtm commented Sep 26, 2024

madsmtm Sep 26, 2024

Choose a reason for hiding this comment

clubby789 Sep 28, 2024

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment was marked as resolved.

madsmtm commented Nov 16, 2024 • edited Loading

This comment was marked as resolved.

petrochenkov commented Feb 19, 2025

madsmtm commented Feb 19, 2025

madsmtm commented Mar 26, 2025 • edited Loading

petrochenkov commented Mar 26, 2025

madsmtm commented Mar 26, 2025

petrochenkov commented Mar 26, 2025

bors commented Mar 26, 2025

madsmtm commented Nov 16, 2024 •

edited

Loading

madsmtm commented Mar 26, 2025 •

edited

Loading