refactor: move sysroot lookup to GlobalContext by adamgemmell · Pull Request #17276 · rust-lang/cargo

adamgemmell · 2026-07-30T14:25:00Z

What does this PR try to resolve?

I split this out from a refactor of #16675, as part of work on build-std=always. In this PR, and in other build-std work I have in progress, it's useful to know the sysroot path earlier than the compiler/ stage of Cargo. Because there's one rustc instance per Cargo invocation, and because the sysroot path remains fixed per rustc and isn't dependent on the target, this should be safe to move out of TargetInfo.

How to test and review this PR?

The first commit shows the logic being moved, and the second commit shows updating all the users of this logic.

The third removes the actual --print lookup in TargetInfo, which necessitated some test changes and a refactor to ensure the rustc info cache can be written to.

Getting the sysroot is already heavily tested in Cargo, hence why I didn't add a new test for just this.

rustbot · 2026-07-30T14:25:06Z

Thanks for the pull request, and welcome! The Rust Project is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @epage, @weihanglo
@epage, @weihanglo expanded to epage, weihanglo
Random selection from epage, weihanglo

The move to passing in a Rustc rather than a Workspace was necessary as a 2nd Rustc is unable to update the rustc info cache.

rustbot · 2026-08-03T16:37:40Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Update cargo submodule 21 commits in 7c83d4cc0953b81d823e47d640c64da9b8bd4fac..c79e8f89441b3e73d6d65d125c0c745792808c74 2026-07-29 21:34:53 +0000 to 2026-08-04 19:17:33 +0000 - fix(diag): Ensure diagnostic titles work without snippets (rust-lang/cargo#17304) - refactor: Remove unnecessary mut in sources (rust-lang/cargo#17305) - feat(trim-paths): emit unremap files for final artifacts (rust-lang/cargo#17303) - fix: prevent panic when `package.build` is empty (rust-lang/cargo#17268) - Add a suggestion when adding `[lints]` to a workspace to use `[workspace.lints]` instead (rust-lang/cargo#17300) - chore(deps): update embarkstudios/cargo-deny-action action to v2.1.1 (rust-lang/cargo#17291) - refactor: move sysroot lookup to GlobalContext (rust-lang/cargo#17276) - fix(trim-paths): unambiguous and reversible remap rules (rust-lang/cargo#17302) - Avoid parsing unchanged lockfiles (rust-lang/cargo#17301) - Remove unnecessary to_path_buf (rust-lang/cargo#17295) - chore(deps): update cargo-semver-checks to v0.50.0 (rust-lang/cargo#17297) - chore(deps): update actions/checkout action to v6.1.0 (rust-lang/cargo#17290) - Remove unnecessary return at end of functions (rust-lang/cargo#17292) - make __CARGO_TEST_FORCE_ARGFILE available in distributed builds (rust-lang/cargo#17293) - Fix manual_readme lint for lower-priority README files (rust-lang/cargo#17208) - fix(git): make checkout names independent of git config (rust-lang/cargo#17289) - fix(diag): Rename redundant_readme to manual_readme (rust-lang/cargo#17288) - Remove redundant double call .to_string() (rust-lang/cargo#17286) - fix(completions): complete paths for cargo run arguments (rust-lang/cargo#17284) - test(git): exercise multi git revision lockfile (rust-lang/cargo#17279) - add context to lints documentation (rust-lang/cargo#17273) r? ghost

Kobzol · 2026-08-05T14:38:57Z

This broke rustc-perf, because cargo now queries rustc for its sysroot, and that's an additional rustc invocation that rustc-perf didn't expect.

I can work around that in rustc-perf easily, but I wanted to ask whether Cargo will now call rustc --print=sysroot unconditionally for every compilation? I wonder if it would be possible to instead just add --print=sysroot to the existing rustc invocation that queries --print=file-names etc.?

Kobzol

Ah, I see, what I suggested is actually what happened before this PR. In that case, this PR sort of pessimizes the case where Cargo does actually get to the compilation stage, because then there will be one extra rustc invocation.

Kobzol · 2026-08-05T14:41:27Z

    ///
    /// Search `Tricky` to learn why querying `rustc` several times is needed.
+    ///
+    /// When a Workspace is provided,


Probably an unfinished comment? (noticed while reading the PR).

💩

I have another small refactor PR which I'll likely ship, and I'll remove this comment in that

adamgemmell · 2026-08-05T14:58:06Z

This broke rustc-perf, because cargo now queries rustc for its sysroot, and that's an additional rustc invocation that rustc-perf didn't expect.

I can work around that in rustc-perf easily, but I wanted to ask whether Cargo will now call rustc --print=sysroot unconditionally for every compilation? I wonder if it would be possible to instead just add --print=sysroot to the existing rustc invocation that queries --print=file-names etc.?

Yes it will do. It might be possible to remove this extra invocation (or at least limit it to build-std) depending on how my two large patches in flight land, but as it stands it would be more complex. Note that cargo does cache rustc output in target/.rustc_info.json. I can revisit this with perf in mind once the patches that use it earlier in Cargo's pipeline are merged.

Kobzol · 2026-08-05T15:02:58Z

Oh, I thought that it is cached only in memory. If it is cached on disk, then that is better. But it doesn't seem to work like I would expect it to then, because in rustc-perf we do essentially this:

cargo check
touch <some Rust file>
cargo check

And the --print=sysroot call seems to be performed also during the second cargo check invocation.

weihanglo · 2026-08-05T15:05:28Z

It might be possible to remove this extra invocation (or at least limit it to build-std)

Can we track this somewhere in any of the tracking issue? This to me is a requirement for stabilization, unless it is too messy to implement.

adamgemmell · 2026-08-05T15:11:58Z

Sure, I've added it to the bottom of our task breakdown on hackmd

adamgemmell · 2026-08-05T15:26:02Z

Oh, I thought that it is cached only in memory. If it is cached on disk, then that is better. But it doesn't seem to work like I would expect it to then, because in rustc-perf we do essentially this:
cargo check
touch <some Rust file>
cargo check
And the --print=sysroot call seems to be performed also during the second cargo check invocation.

I can't reproduce this with my local cargo build, I can see the sysroot in the json cache and I can see a "rustc info cache hit" with CARGO_LOG=debug.

As for perf, I think the impact here might actually be less than we though due to a separate issue - with debug logging on you can see lots of "rustc info cache miss" messages and each one invokes rustc -vV. This is because GlobalContext::load_global_rustc creates a new Rustc on every call, which invokes that on every construction.

To fix that maybe we could cache the Rustc and just have callers use it by reference

Kobzol · 2026-08-05T16:51:14Z

Ok, I found the culprit. When Cargo hashes the executed command for hashing, it also hashes CARGO_RUSTC_CURRENT_DIR. And what rustc-perf does it prepare the target directory in dir A, and then copy it to dir B, and then run cargo rustc on the leaf crate. And since CARGO_RUSTC_CURRENT_DIR is different in both cases, the hash doesn't match.

I think that this environment variable is set by Cargo's config.toml file? I was quite confused by where is it coming from, because I thought that it was removed (#14798).

That explains why specifically the cargo benchmark was broken, and nothing else. If I remove the environment variable, the command execution is properly cached.

Noting: this is not a big deal regarding this PR, it looks like caching works, unless you literally compile Cargo and move the target directory in-between builds xD

Update cargo submodule 21 commits in 7c83d4cc0953b81d823e47d640c64da9b8bd4fac..c79e8f89441b3e73d6d65d125c0c745792808c74 2026-07-29 21:34:53 +0000 to 2026-08-04 19:17:33 +0000 - fix(diag): Ensure diagnostic titles work without snippets (rust-lang/cargo#17304) - refactor: Remove unnecessary mut in sources (rust-lang/cargo#17305) - feat(trim-paths): emit unremap files for final artifacts (rust-lang/cargo#17303) - fix: prevent panic when `package.build` is empty (rust-lang/cargo#17268) - Add a suggestion when adding `[lints]` to a workspace to use `[workspace.lints]` instead (rust-lang/cargo#17300) - chore(deps): update embarkstudios/cargo-deny-action action to v2.1.1 (rust-lang/cargo#17291) - refactor: move sysroot lookup to GlobalContext (rust-lang/cargo#17276) - fix(trim-paths): unambiguous and reversible remap rules (rust-lang/cargo#17302) - Avoid parsing unchanged lockfiles (rust-lang/cargo#17301) - Remove unnecessary to_path_buf (rust-lang/cargo#17295) - chore(deps): update cargo-semver-checks to v0.50.0 (rust-lang/cargo#17297) - chore(deps): update actions/checkout action to v6.1.0 (rust-lang/cargo#17290) - Remove unnecessary return at end of functions (rust-lang/cargo#17292) - make __CARGO_TEST_FORCE_ARGFILE available in distributed builds (rust-lang/cargo#17293) - Fix manual_readme lint for lower-priority README files (rust-lang/cargo#17208) - fix(git): make checkout names independent of git config (rust-lang/cargo#17289) - fix(diag): Rename redundant_readme to manual_readme (rust-lang/cargo#17288) - Remove redundant double call .to_string() (rust-lang/cargo#17286) - fix(completions): complete paths for cargo run arguments (rust-lang/cargo#17284) - test(git): exercise multi git revision lockfile (rust-lang/cargo#17279) - add context to lints documentation (rust-lang/cargo#17273) r? ghost

weihanglo · 2026-08-06T13:30:48Z

I think that this environment variable is set by Cargo's config.toml file? I was quite confused by where is it coming from, because I thought that it was removed (#14798).

CARGO_RUSTC_CURRENT_DIR became a protocol from snapbox https://github.com/assert-rs/snapbox/blob/f85fb904b6c08504d4c31b7b58bcb3b6e8a87018/crates/snapbox/src/macros.rs#L90-L114 Some more context: #14869.

Kobzol · 2026-08-06T14:43:17Z

I see. Automatically setting environment variables seems quite hacky :/ But I worked around it in rustc-perf, so shouldn't matter.

adamgemmell · 2026-08-06T16:27:03Z

To fix that maybe we could cache the Rustc and just have callers use it by reference

I have a WIP for this that should cut down a bunch of unnecessary rustc calls but it's gotten a little complex, so I'll revisit it later.

rustbot assigned weihanglo Jul 30, 2026

epage reviewed Jul 30, 2026

View reviewed changes

Comment thread src/context/mod.rs Outdated

epage reviewed Jul 30, 2026

View reviewed changes

Comment thread src/util/rustc.rs

epage reviewed Jul 30, 2026

View reviewed changes

Comment thread src/compiler/build_context/target_info.rs

This comment has been minimized.

Sign in to view

adamgemmell added 3 commits August 3, 2026 17:29

refactor: move the sysroot path to the GlobalContext

936fb92

refactor: update all users of the sysroot path

9f3c326

test(cfg): use the gctx sysroot when building TargetInfo

4c20b84

The move to passing in a Rustc rather than a Workspace was necessary as a 2nd Rustc is unable to update the rustc info cache.

adamgemmell force-pushed the dev/adagem01/sysroot-path branch from 39c8232 to 4c20b84 Compare August 3, 2026 16:37

epage reviewed Aug 3, 2026

View reviewed changes

Comment thread src/context/mod.rs

epage reviewed Aug 3, 2026

View reviewed changes

Comment thread src/util/rustc.rs

epage approved these changes Aug 3, 2026

View reviewed changes

epage added this pull request to the merge queue Aug 3, 2026

Merged via the queue into rust-lang:master with commit fee70b3 Aug 3, 2026
29 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 3, 2026

adamgemmell deleted the dev/adagem01/sysroot-path branch August 4, 2026 08:50

weihanglo mentioned this pull request Aug 4, 2026

Update cargo submodule rust-lang/rust#160527

Merged

Kobzol reviewed Aug 5, 2026

View reviewed changes

Kobzol mentioned this pull request Aug 5, 2026

Add support for bare rustc invocation with --print-sysroot rust-lang/rustc-perf#2520

Merged

rustbot added this to the 1.99.0 milestone Aug 6, 2026

Uh oh!

Uh oh!

Conversation

adamgemmell commented Jul 30, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How to test and review this PR?

Uh oh!

rustbot commented Jul 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Aug 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kobzol commented Aug 5, 2026

Uh oh!

Kobzol left a comment

Choose a reason for hiding this comment

Uh oh!

Kobzol Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

adamgemmell Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

adamgemmell commented Aug 5, 2026

Uh oh!

Kobzol commented Aug 5, 2026

Uh oh!

weihanglo commented Aug 5, 2026

Uh oh!

adamgemmell commented Aug 5, 2026

Uh oh!

adamgemmell commented Aug 5, 2026

Uh oh!

Kobzol commented Aug 5, 2026

Uh oh!

weihanglo commented Aug 6, 2026

Uh oh!

Kobzol commented Aug 6, 2026

Uh oh!

adamgemmell commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adamgemmell commented Jul 30, 2026 •

edited by rustbot

Loading