Repository navigation
fix: exclude abspath of path deps from -Cmetadata hash - #17624
Conversation
rustbot
added
the
A-layout
8ab48d0 to
043aba3
Compare
|
r? ranger-ross What do you think? |
rustbot
added
the
S-waiting-on-review
043aba3 to
e367eb4
Compare
|
I am trying to understand what the risks of this change are. So the risk is that we no longer track absolute file paths outside of the workspace. So if that file changes we could end up with 2 rlibs with the same symbol mangling and rustc would not be happy. I am assuming it would error. I guess we could add a level of safety by hashing the file contents, but that would probably become a rabbit hole of its own. (if that file pulled in more source files or if it contained absolute paths) |
It depends on whether there is a rebuild triggered. If yes, and rustc would error. For transitive deps from But anyway we have absolute path in |
|
The future risk if we ever trim abspath from And here are what may happen:
If the blob store / |
| # See https://github.com/rust-lang/rust/pull/158873 | ||
| [target.'cfg(target_env = "msvc")'] | ||
| rustflags = ["-Clink-arg=/Brepro"] | ||
| # On Windows GNU the PE format insert a timestamp by default | ||
| # so that we need to opt-out. | ||
| # See https://sourceware.org/binutils/docs/ld.html#index-_002d_002dinsert_002dtimestamp | ||
| [target.'cfg(all(target_os = "windows", target_env = "gnu"))'] | ||
| rustflags = ["-Clink-arg=-Wl,--no-insert-timestamp"] |
There was a problem hiding this comment.
I am surprised (but not too surprised) that we need to that much to achieve reproducibility on Windows targets
|
Some relevant discussion: #t-cargo/build-script > Cross-platform build reproducibility and build scripts @ 💬 |
they will diverge later
`-Cmetadata` is for rustc to uniquely identify different crates in a crate graph. Without `trim-paths`, the binaries embeds absolute paths anyway. Dropping the path from `-Cmetadata` doesn't make any difference. Now we `trim-paths` we can aggressively drop those absolute paths, so that binaries are now more reproducible.
e367eb4 to
20319b9
Compare
|
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
removed
the
S-waiting-on-review
Update cargo submodule 7 commits in cad07cf8e597397fff5d0f6cb79c9f870c0b1dd2..31d3246e0e8d3a80d4a90b54aa48ff180adb408b 2026-10-23 22:04:58 +0000 to 2026-10-27 13:54:39 +0000 - Warn about conflicting proc-macro manifest settings (rust-lang/cargo#17641) - docs: use latest `resolver` and `edition` in examples (rust-lang/cargo#17644) - docs(hints): write mostly-unused docs as end-user documentation (rust-lang/cargo#17549) - refactor(help): embed man pages without a tarball (rust-lang/cargo#17640) - refactor(cargo-test-support): remove build.rs (rust-lang/cargo#17638) - fix: exclude abspath of path deps from `-Cmetadata` hash (rust-lang/cargo#17624) - use build-rs for buildscripts (rust-lang/cargo#17637) r? ghost
Update cargo submodule 7 commits in cad07cf8e597397fff5d0f6cb79c9f870c0b1dd2..31d3246e0e8d3a80d4a90b54aa48ff180adb408b 2026-10-23 22:04:58 +0000 to 2026-10-27 13:54:39 +0000 - Warn about conflicting proc-macro manifest settings (rust-lang/cargo#17641) - docs: use latest `resolver` and `edition` in examples (rust-lang/cargo#17644) - docs(hints): write mostly-unused docs as end-user documentation (rust-lang/cargo#17549) - refactor(help): embed man pages without a tarball (rust-lang/cargo#17640) - refactor(cargo-test-support): remove build.rs (rust-lang/cargo#17638) - fix: exclude abspath of path deps from `-Cmetadata` hash (rust-lang/cargo#17624) - use build-rs for buildscripts (rust-lang/cargo#17637) r? ghost
What does this PR try to resolve?
-Cmetadatais for rustc to uniquely identify different crates in acrate graph. Without
trim-paths, the binaries embeds absolute pathsanyway. Dropping the path from
-Cmetadatadoesn't make any difference.Now we
trim-pathswe can aggressively drop those absolute paths,so that binaries are now more reproducible.
Notes to reviewers
See #13586 (comment).
And I believe if this got merged it fixes #13586 anyway.
This doesn't really help caching (yet),
as
unit_idhash still hold those absolute paths (and may need checksum-freshness to make it work)🤖 LLM involvement: Tests were initially generated though LLM did pretty bad at parameterizing those so I rewrote quite a bit.