Add unstable support for outputting file checksums for use in cargo by Xaeroxe · Pull Request #126930 · rust-lang/rust (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation88 Commits12 Checks6 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
Adds an unstable option that appends file checksums and expected lengths to the end of the dep-info file such that cargo can read and use these values as an alternative to file mtimes.
This PR powers the changes made in this cargo PR rust-lang/cargo#14137
Here's the tracking issue for the cargo feature rust-lang/cargo#14136.
r? @fmease
rustbot has assigned @fmease.
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
Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
This was referenced
Jun 25, 2024
Given that this PR adds a new concept and a new flag it requires a minimum of discussion within the team.
Could you start a T-compiler Major Change Proposol following the guide.
These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.
Status: Blocked on something else such as an RFC or other implementation work.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
The MCP has been accepted
@rustbot ready
Status: Awaiting review from the assignee but also interested parties.
and removed S-blocked
Status: Blocked on something else such as an RFC or other implementation work.
labels
Area: The testsuite used to check the correctness of rustc
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
labels
The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.
Correct, blake3 has been added as it was mentioned during the MCP review as being desirable.
This comment was marked as outdated.
We had some LLVM BOLT issues with the blake3 crate in #127754, a couple of days ago, on a simple try build.
Let's try this PR, to see if is also has those compilation issues.
@bors try (@Xaeroxe could you fix the conflicts please)
This comment was marked as outdated.
This comment has been minimized.
The rollup hasn't actually started, so it's actually quite cost-free to just
@bors r-
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
The Cargo PR has already been adjusted to this new behavior.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked that 58c5ac4 works with the latest commit rust-lang/cargo@c755949 in rust-lang/cargo#14137.
I would like to r+ this, but I am not in t-compiler so probably shouldn't do that.
It would be appreciated if @chenyukang or someone on t-compiler could take another look. It should be pretty much the same as before except writing # checksum comments to separate lines (b48c5f1)
📌 Commit 58c5ac4 has been approved by chenyukang
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
bors added a commit to rust-lang-ci/rust that referenced this pull request
…kingjubilee
Rollup of 3 pull requests
Successful merges:
- rust-lang#126930 (Add unstable support for outputting file checksums for use in cargo)
- rust-lang#130725 (Parser: better error messages for
@in struct patterns) - rust-lang#131166 (Fix
target_vendorforaarch64-nintendo-switch-freestanding)
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#126930 - Xaeroxe:file-checksum-hint, r=chenyukang
Add unstable support for outputting file checksums for use in cargo
Adds an unstable option that appends file checksums and expected lengths to the end of the dep-info file such that cargo can read and use these values as an alternative to file mtimes.
This PR powers the changes made in this cargo PR rust-lang/cargo#14137
Here's the tracking issue for the cargo feature rust-lang/cargo#14136.
Can PR description be updated with actual changes in that PR? Currently is slightly misleading.
Comment on lines +1706 to +1707
| checksum_hash_algorithm: Option = (None, parse_cargo_src_file_hash, [TRACKED], |
|---|
| "hash algorithm of source files used to check freshness in cargo (`blake3` or `sha256`)"), |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happened if md5 passed here? SourceFileHashAlgorithm allows it.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then md5 checksums will be emitted. This doesn’t make any guarantees about whether the tooling surrounding rustc can actually ingest those. It’s up to the tooling to understand what it can ingest. An earlier version of this PR did forbid things like md5 but I was instructed to not build in that kind of restriction by prior reviewers.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, at least help message mentions only blake3 and sha256, while actually accepting something else too.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm… still seeing "cargo" mentioned here and elsewhere. Not really critical but would love to see we make the option more general than a Cargo-specific compiler flag (though in fact it was made for Cargo 😆)
Can PR description be updated with actual changes in that PR? Currently is slightly misleading.
How so? I just went back and read it and it seems correct to me. What’s misleading about it?
PR also adds option for using blake3 source file hashing (or not? I'm not clearly understand that, the same enum used for source file hash for debug and for new cargo feature)
blake3 can’t be used in debug data as no debug format supports it, but it can be used in dep-info files. If you try to use blake3 here then you simply won’t get a hash in your debug files.
bors added a commit to rust-lang/cargo that referenced this pull request
initial version of checksum based freshness
Implementation for #14136 and resolves #6529
This PR implements the use of checksums in cargo fingerprints as an alternative to using mtimes. This is most useful on systems with poor mtime implementations.
This has a dependency on rust-lang/rust#126930. It's expected this will increase the time it takes to declare a build to be fresh. Still this loss in performance may be preferable to the issues the ecosystem has had with the use of mtimes for determining freshness.
bors added a commit to rust-lang/cargo that referenced this pull request
initial version of checksum based freshness
Implementation for #14136 and resolves #6529
This PR implements the use of checksums in cargo fingerprints as an alternative to using mtimes. This is most useful on systems with poor mtime implementations.
This has a dependency on rust-lang/rust#126930. It's expected this will increase the time it takes to declare a build to be fresh. Still this loss in performance may be preferable to the issues the ecosystem has had with the use of mtimes for determining freshness.
bors pushed a commit to rust-lang-ci/rust that referenced this pull request
This also adds three license exceptions to Cargo.
- arrayref — BSD-2-Clause
- blake3 — CC0-1.0 OR Apache-2.0 OR Apache-2.0 WITH LLVM-exception
- constant_time_eq — CC0-1.0 OR MIT-0 OR Apache-2.0
These exceptions were added to rustc in rust-lang#126930, so should be fine for Cargo as well.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Update cargo
8 commits in ad074abe3a18ce8444c06f962ceecfd056acfc73..15fbd2f607d4defc87053b8b76bf5038f2483cf4 2024-10-04 18🔞15 +0000 to 2024-10-08 21:08:11 +0000
- initial version of checksum based freshness (rust-lang/cargo#14137)
- feat: Add custom completer for completing registry name (rust-lang/cargo#14656)
- Document build-plan as being deprecated (rust-lang/cargo#14657)
- fix(complete): Don't complete files for any value (rust-lang/cargo#14653)
- Add more SAT resolver tests (rust-lang/cargo#14614)
- fix: avoid inserting duplicate
dylib_path_envvarwhen callingcargo runrecursively (rust-lang/cargo#14464) - chore(deps): bump gix-path from 0.10.9 to 0.10.11 (rust-lang/cargo#14489)
- improve error reporting when feature not found in
activated_features(rust-lang/cargo#14647)
This also adds three license exceptions to Cargo.
- arrayref — BSD-2-Clause
- blake3 — CC0-1.0 OR Apache-2.0 OR Apache-2.0 WITH LLVM-exception
- constant_time_eq — CC0-1.0 OR MIT-0 OR Apache-2.0
These exceptions were added to rustc in rust-lang#126930, so should be fine for Cargo as well.
rust-cloud-vms bot pushed a commit to liwagu/rust that referenced this pull request
This also adds three license exceptions to Cargo.
- arrayref — BSD-2-Clause
- blake3 — CC0-1.0 OR Apache-2.0 OR Apache-2.0 WITH LLVM-exception
- constant_time_eq — CC0-1.0 OR MIT-0 OR Apache-2.0
These exceptions were added to rustc in rust-lang#126930, so should be fine for Cargo as well.
Labels
Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html)
Area: port run-make Makefiles to rmake.rs
Area: The testsuite used to check the correctness of rustc
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Relevant to the compiler team, which will review and decide on the PR/issue.