Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

initial version of checksum based freshness #14137

Merged
merged 13 commits into from
Oct 8, 2024
Merged

initial version of checksum based freshness #14137

merged 13 commits into from
Oct 8, 2024

Conversation

Xaeroxe
Copy link
Contributor

@Xaeroxe Xaeroxe commented Jun 25, 2024 •
edited
Loading

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.

@rustbot
Copy link
Collaborator

rustbot commented Jun 25, 2024

Thanks for the pull request, and welcome! The Rust team 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

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler A-cli Area: Command-line interface, option parsing, etc. A-configuration Area: cargo config files and env vars A-rebuild-detection Area: rebuild detection and fingerprinting A-unstable Area: nightly unstable support S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 25, 2024
This was referenced Jun 25, 2024
Open
Merged
@Xaeroxe Xaeroxe force-pushed the checksum-freshness branch from 320f73c to 310cd79 Compare June 25, 2024 05:37
tgross35
tgross35 reviewed Jun 25, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@Xaeroxe Xaeroxe force-pushed the checksum-freshness branch from cce62ba to 59441b6 Compare June 26, 2024 05:55
@Xaeroxe Xaeroxe mentioned this pull request Jul 1, 2024
Closed
3 tasks
@rustbot rustbot added the A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. label Jul 13, 2024
@Xaeroxe Xaeroxe force-pushed the checksum-freshness branch from 27e2a18 to c83be55 Compare July 13, 2024 14:58
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@bors
Auto merge of rust-lang#126930 - Xaeroxe:file-checksum-hint, r=<try>
c5ac80a 
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.
@bors
Copy link
Contributor

bors commented Jul 26, 2024

☔ The latest upstream changes (presumably #13947) made this pull request unmergeable. Please resolve the merge conflicts.

@Xaeroxe Xaeroxe force-pushed the checksum-freshness branch from c83be55 to b8c21fa Compare July 26, 2024 22:23
@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Jul 26, 2024

Merge conflicts resolved.

epage
epage reviewed Aug 1, 2024
View reviewed changes
src/cargo/util/context/mod.rs Outdated Show resolved Hide resolved
weihanglo
weihanglo approved these changes Oct 8, 2024
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just rearrange commits a bit, so we can remove unnecessary commit like 2dafe11.

Thank you for the hard work! I am going to merge this when the CI is green.

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 8, 2024

📌 Commit cf893c1 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Oct 8, 2024
@bors
Copy link
Contributor

bors commented Oct 8, 2024

⌛ Testing commit cf893c1 with merge 15fbd2f...

@bors
Copy link
Contributor

bors commented Oct 8, 2024

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 15fbd2f to master...

@bors bors merged commit 15fbd2f into rust-lang:master Oct 8, 2024
24 checks passed
This was referenced Oct 8, 2024
Closed
Open
Closed
Merged
Open
Open
@weihanglo weihanglo mentioned this pull request Oct 9, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 9, 2024
@bors
Auto merge of rust-lang#131421 - weihanglo:update-cargo, r=weihanglo
883f9a2 
Update cargo

8 commits in ad074abe3a18ce8444c06f962ceecfd056acfc73..15fbd2f607d4defc87053b8b76bf5038f2483cf4
2024-10-04 18: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_envvar` when calling `cargo run` recursively (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.
@Xaeroxe Xaeroxe deleted the checksum-freshness branch October 9, 2024 01:59
@rustbot rustbot added this to the 1.83.0 milestone Oct 9, 2024
@weihanglo weihanglo mentioned this pull request Oct 10, 2024
Merged
@jonhoo
Copy link
Contributor

jonhoo commented Oct 19, 2024

Is there a way we can tag this for mention in the release notes? May be worth giving a bit of celebration to :)

@weihanglo
Copy link
Member

It has been listed in changelog of 1.83 already, under the nightly section! https://github.com/rust-lang/cargo/blob/master/CHANGELOG.md#cargo-183-2024-11-28

Though we haven't figured out the build script part (see #14136), maybe we could celebrate a bit later when we resolve that? :)

@weihanglo weihanglo added Z-checksum-freshness Nightly: rebuild detection on file checksum instead of mtime and removed A-configuration Area: cargo config files and env vars A-testing-cargo-itself Area: cargo's tests A-cli Area: Command-line interface, option parsing, etc. A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. A-documenting-cargo-itself Area: Cargo's documentation labels Oct 23, 2024
@weihanglo
Copy link
Member

If anyone is interested in helping this go stabilized, please share your benchmark results for -Zchecksum-freshness on this Zulip topic, or in #14722.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@tgross35 tgross35 tgross35 left review comments

@ReillyBrogan ReillyBrogan ReillyBrogan left review comments

@arlosi arlosi arlosi left review comments

@epage epage epage left review comments

@weihanglo weihanglo weihanglo approved these changes

Assignees

@weihanglo weihanglo

Labels
A-build-execution Area: anything dealing with executing the compiler A-dep-info Area: dep-info, .d files A-rebuild-detection Area: rebuild detection and fingerprinting A-unstable Area: nightly unstable support S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. Z-checksum-freshness Nightly: rebuild detection on file checksum instead of mtime
Projects
None yet
Milestone
1.83.0
Development

Successfully merging this pull request may close these issues.

(Option to) Fingerprint by file contents instead of mtime
10 participants
@Xaeroxe @rustbot @bors @weihanglo @jonhoo @epage @arlosi @ReillyBrogan @tgross35 @bjorn3