fix(trim-paths): sanitize absolute source paths in fingerprint by weihanglo · Pull Request #17556 · rust-lang/cargo

weihanglo · 2026-10-07T05:08:30Z

What does this PR try to resolve?

With trim-paths,
rustc only sees remapped source paths.
There is no reason that fingerprint still tracks original paths.

This sanitizes absolute paths in Cargo fingerprint
by applying the same remap rules with trim-paths.

Without this,
moving CARGO_HOME would cause rebuilds,
even the compiled artifacts are byte-identical.

Fixes #10915

How to test and review this PR?

One caveat is that because the "object" scope doesn't remap diagonstics,
if diagnostics were also cached,
a replay may show the "wrong" cached paths.
The "all" scope doesn't have this issue btw.

The cargo_home_moved_trim_paths_object_with_diagnostics test show the caveat.

Also, we might not want this if fingerprint will be obsolete soon in some future blob store design.

🤖 LLM disclosure: I used LLM to update snapshots during the development.

weihanglo · 2026-10-07T15:05:45Z

@ranger-ross what do you think about this special case? Fingerprint may be obsolete in some of the blob store proposals, but if it is still relevant we might want this.

ranger-ross · 2026-10-07T15:48:06Z

The changes here seem reasonable to me.

I probably need to give more thought how fingerprinting will be handled in the blob storage and cache.
Thus far, I've completely kept fingerprint (mostly) unchanged and just let cargo write the fingerprint when a cache entry is loaded as if it were just built. (Mtimed just happen to work out since cached deps mtimes are older and ordered by the previous build, but that isn't going to work for remote cache. There is a TODO in the design doc about this)

I think it would be quite difficult to remove fingerprinting until all build units are fully immutable. Part of my wonders if we should make ImmutableUnits a first class citizen in Cargo so we can remove fingerprinting from those while keeping it for the units that still need it. (And ideally make more things immutable overtime)

rustbot · 2026-10-14T04:05:13Z

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

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

weihanglo

Made it ready, though I am open to just make this under all scope.

View changes since this review

weihanglo · 2026-10-14T04:06:45Z

+[FRESH] bar v0.0.1
+[WARNING] unused variable: `unused`
+ --> [ROOT]/home/.cargo/registry/src/-[..]/bar-0.0.1/src/lib.rs:1:18
+...


Raise the awareness of this:

One caveat is that because the "object" scope doesn't remap diagonstics,
if diagnostics were also cached,
a replay may show the "wrong" cached paths.
The "all" scope doesn't have this issue btw.

Might be worth adding a comment pointing out that .cargo is cached and should actually be .cargo-moved but its a trade off we accepted.

Since its not obvious outside of the context of this PR.

Updated. Thanks.

weihanglo · 2026-10-20T23:00:53Z

r? ranger-ross

Do you want to review this? SHould we go with an FCP? We could also start small only doing this when scope is "all", which doesn't have the cached diagnostic issue.

ranger-ross

The implementation looks good to me.
Given that fingerprints are meant to be Cargo internal, I am completely fine with changing this. If it wasn't for the stale diagnostics, I'd actual even be in favor of not requiring trim paths for this. (I still might be, I am on the fence)

IMO, trim-paths=object feels like a reasonable starting point and can be later adjusted to all or trim-paths=none based on more feedback.

Happy to wait for feedback from others or start an FCP if you feel it is warranted here.

as a sidenote, the way the caching design appears to be going it seems that fingerprints are likely not going to be stored in the cache, making removing absolute paths from fingerprints non-loadbearing for remote caching.

View changes since this review

ranger-ross · 2026-10-21T05:28:24Z

+[FRESH] bar v0.0.1
+[WARNING] unused variable: `unused`
+ --> [ROOT]/home/.cargo/registry/src/-[..]/bar-0.0.1/src/lib.rs:1:18
+...


Might be worth adding a comment pointing out that .cargo is cached and should actually be .cargo-moved but its a trade off we accepted.

Since its not obvious outside of the context of this PR.

weihanglo · 2026-10-21T13:20:47Z

@rfcbot fcp merge T-cargo

See the PR description.

The user-facing change is that after moving CARGO_HOME user might get stale diagnostics.

rust-rfcbot · 2026-10-21T13:21:35Z

@weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

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.

epage · 2026-10-21T16:06:47Z

Seems fine.

Moving CARGO_HOME seems like a rare enough thing to be much of an issue. We already have other cases, like short vs full diagnostics.

With `trim-paths`, rustc only sees remapped source paths. There is no reason that fingerprint still tracks original paths. This sanitizes absolute paths in Cargo fingerprint by applying the same remap rules with trim-paths. Without this, moving `CARGO_HOME` would cause rebuilds, even the compiled artifacts are byte-identical. One caveat is that because the "object" scope doesn't remap diagonstics, if diagnostics were also cached, a replay may show the "wrong" cached paths. The "all" scope doesn't have this issue btw.

rustbot · 2026-10-24T15:26:17Z

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.

rustbot added the A-rebuild-detection Area: rebuild detection and fingerprinting label Oct 7, 2026

weihanglo force-pushed the trim-paths-fingerprint-remap branch from 92836db to 45b355e Compare October 7, 2026 13:34

weihanglo marked this pull request as ready for review October 14, 2026 04:05

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 14, 2026

rustbot assigned epage Oct 14, 2026

weihanglo commented Oct 14, 2026

View reviewed changes

weihanglo mentioned this pull request Oct 14, 2026

Moving CARGO_HOME invalidates target caches #10915

Open

rustbot assigned ranger-ross and unassigned epage Oct 20, 2026

ranger-ross approved these changes Oct 21, 2026

View reviewed changes

weihanglo force-pushed the trim-paths-fingerprint-remap branch from 45b355e to c54cd1c Compare October 21, 2026 13:10

This comment has been minimized.

Sign in to view

weihanglo added the T-cargo Team: Cargo label Oct 21, 2026

rust-lang deleted a comment from rust-rfcbot Oct 21, 2026

rust-rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Oct 21, 2026

weihanglo force-pushed the trim-paths-fingerprint-remap branch from c54cd1c to 877dd8d Compare October 24, 2026 12:17

This comment has been minimized.

Sign in to view

weihanglo added 3 commits October 24, 2026 11:26

test(trim-paths): show moving CARGO_HOME rebuilds deps

c8a1f91

refactor(trim-paths): extract remap pairs iterator

a8baaac

weihanglo force-pushed the trim-paths-fingerprint-remap branch from 877dd8d to 527cffa Compare October 24, 2026 15:26

Conversation

weihanglo commented Oct 7, 2026

What does this PR try to resolve?

How to test and review this PR?

Uh oh!

weihanglo commented Oct 7, 2026

Uh oh!

ranger-ross commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 14, 2026

Uh oh!

weihanglo left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weihanglo Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

ranger-ross Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

weihanglo Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

weihanglo commented Oct 20, 2026

Uh oh!

ranger-ross left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranger-ross Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

weihanglo commented Oct 21, 2026

Uh oh!

rust-rfcbot commented Oct 21, 2026 • edited by epage Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

epage commented Oct 21, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rustbot commented Oct 24, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ranger-ross commented Oct 7, 2026 •

edited

Loading

weihanglo left a comment •

edited by rustbot

Loading

ranger-ross left a comment •

edited by rustbot

Loading

rust-rfcbot commented Oct 21, 2026 •

edited by epage

Loading