Add more metadata to rustc_fingerprint
#14761
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, `rustc_fingerprint` was only using the path and mtime for each executable, but this is not always sufficient. For example, Fedora will [clamp mtimes] to help reproducible builds, so Rust 1.82 looks like `/usr/bin/rustc` and `2024-10-17 00:00:00` across all builds in Fedora 39 through 42 (rawhide), even though they are different in their full version strings, LLVM, etc. [clamp mtimes]: https://fedoraproject.org/wiki/Changes/ReproducibleBuildsClampMtimes If the target directory (including `.rustc_info.json`) is shared across such systems, the fingerprint wouldn't notice the difference, and cargo would try to use artifacts that aren't actually compatible, like: ``` error[E0514]: found crate `autocfg` compiled by an incompatible version of rustc --> build.rs:2:14 | 2 | let ac = autocfg::new(); | ^^^^^^^ | = note: the following crate versions were found: crate `autocfg` compiled by rustc 1.82.0 (f6e511eec 2024-10-15) (Fedora 1.82.0-1.fc42): [...]/target/debug/deps/libautocfg-589c41db1eea6297.rlib = help: please recompile that crate using this compiler (rustc 1.82.0 (f6e511eec 2024-10-15) (Fedora 1.82.0-1.fc40)) (consider running `cargo clean` first) ``` We can improve this situation by adding the file length, although that could also happen to be the same, and the creation date that will match when the file was installed, though not all filesystems support that. All of this comes from a single metadata call, so it shouldn't have any noticeable slowdown that would hurt the caching effort.
rustbot
added
A-cache-messages
|
It would be more robust if we hash the file contents, or even just its |
|
Thanks for doing this! Interesting timing as we recently merged unstable support for switching from mtimes to checksums for determining when to rebuild, see https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#checksum-freshness Performance does seem to be an issue in that case, so I can understand being cautious here as well. This is also not as "active" as a case for changes as source. This seems fine as an incremental improvement that doesn't restrict us on what we do in the future. @bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 2, 2024
Update cargo 18 commits in e75214ea4936d2f2c909a71a1237042cc0e14b07..0310497822a7a673a330a5dd068b7aaa579a265e 2024-10-25 16:34:32 +0000 to 2024-11-01 19:27:56 +0000 - Add more metadata to `rustc_fingerprint` (rust-lang/cargo#14761) - test(rustfix): switch to a simpler case for dedup-suggestions (rust-lang/cargo#14765) - chore(deps): update rust crate security-framework to v3 (rust-lang/cargo#14766) - chore(deps): update rust crate gix to 0.67.0 (rust-lang/cargo#14762) - fix(util): Respect all `..`s in `normalize_path` (rust-lang/cargo#14750) - test(doc): Resolve flaky test (rust-lang/cargo#14760) - refactor(test): Remove dead 'expect_stdout_contains_n' check (rust-lang/cargo#14759) - add unstable -Zroot-dir flag to configure the path from which rustc should be invoked (rust-lang/cargo#14752) - docs(resolver): Further v3 prep (rust-lang/cargo#14753) - fix: track version in fingerprint dep-info files (rust-lang/cargo#14751) - test: Remove unused msrv-policy (rust-lang/cargo#14748) - download targeted transitive deps of with artifact deps' target platform (rust-lang/cargo#14723) - Remove requirement for --target when invoking Cargo with -Zbuild-std (rust-lang/cargo#14317) - docs(fingerprint): document the encoding of Cargo's depinfo (rust-lang/cargo#14745) - Allow build scripts to report error messages through `cargo::error` (rust-lang/cargo#14743) - fix(publish): Downgrade version-exists error to warning on dry-run (rust-lang/cargo#14742) - fix: clean up for deprecated and removed commands (rust-lang/cargo#14739) - Deprecate `cargo verify-project` (rust-lang/cargo#14736)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously,
rustc_fingerprintwas only using the path and mtime for each executable, but this is not always sufficient. For example, Fedora will clamp mtimes to help reproducible builds, so Rust 1.82 looks like/usr/bin/rustcand2024-10-17 00:00:00across all builds in Fedora 39 through 42 (rawhide), even though they are different in their full version strings, LLVM, etc.If the target directory (including
.rustc_info.json) is shared across such systems, the fingerprint wouldn't notice the difference, and cargo would try to use artifacts that aren't actually compatible, like:We can improve this situation by adding the file length, although that could also happen to be the same, and the creation date that will match when the file was installed, though not all filesystems support that. All of this comes from a single metadata call, so it shouldn't have any noticeable slowdown that would hurt the caching effort.