cargo icon indicating copy to clipboard operation
cargo copied to clipboard

initial version of checksum based freshness

Open Xaeroxe opened this issue 1 year ago • 3 comments

Implementation for https://github.com/rust-lang/cargo/issues/14136 and resolves https://github.com/rust-lang/cargo/issues/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 https://github.com/rust-lang/rust/pull/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.

Xaeroxe avatar Jun 25 '24 05:06 Xaeroxe

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 avatar Jun 25 '24 05:06 rustbot

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

bors avatar Jul 26 '24 22:07 bors

Merge conflicts resolved.

Xaeroxe avatar Jul 26 '24 22:07 Xaeroxe

:umbrella: The latest upstream changes (presumably #14493) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Sep 10 '24 14:09 bors

:umbrella: The latest upstream changes (presumably #14590) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Sep 25 '24 01:09 bors

@bors try

Let's see how it really goes.

weihanglo avatar Oct 04 '24 12:10 weihanglo

:hourglass: Trying commit 4ddd017694a0415a7c91598153e642e3c2b26321 with merge 5b99bb7bc4645e2f33e933da4a809e516c5a3d45...

bors avatar Oct 04 '24 12:10 bors

:broken_heart: Test failed - checks-actions

bors avatar Oct 04 '24 12:10 bors

All tests are passed. I will do another round of review today.

weihanglo avatar Oct 04 '24 13:10 weihanglo

@weihanglo All comments addressed in 545599890421b6b10aa40b422d40ed06b6856898

Also the PR has been rebased to remove some of the rejected designs for this feature.

Xaeroxe avatar Oct 08 '24 04:10 Xaeroxe

@bors r? @weihanglo

Xaeroxe avatar Oct 08 '24 04:10 Xaeroxe

Could not assign reviewer from: weihanglo. User(s) weihanglo are either the PR author, already assigned, or on vacation, and there are no other candidates. Use r? to specify someone else to assign.

rustbot avatar Oct 08 '24 04:10 rustbot

@bors r+

weihanglo avatar Oct 08 '24 21:10 weihanglo

:pushpin: Commit cf893c1695d96d4bddf5107b832df612d22b4612 has been approved by weihanglo

It is now in the queue for this repository.

bors avatar Oct 08 '24 21:10 bors

:hourglass: Testing commit cf893c1695d96d4bddf5107b832df612d22b4612 with merge 15fbd2f607d4defc87053b8b76bf5038f2483cf4...

bors avatar Oct 08 '24 21:10 bors

:sunny: Test successful - checks-actions Approved by: weihanglo Pushing 15fbd2f607d4defc87053b8b76bf5038f2483cf4 to master...

bors avatar Oct 08 '24 21:10 bors

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

jonhoo avatar Oct 19 '24 09:10 jonhoo

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 https://github.com/rust-lang/cargo/issues/14136), maybe we could celebrate a bit later when we resolve that? :)

weihanglo avatar Oct 19 '24 16:10 weihanglo

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

weihanglo avatar Oct 23 '24 14:10 weihanglo