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

Tracking Issue for checksum freshness #14136

Open
2 tasks done
Xaeroxe opened this issue Jun 25, 2024 · 5 comments
Open
2 tasks done

Tracking Issue for checksum freshness #14136

Xaeroxe opened this issue Jun 25, 2024 · 5 comments
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-tracking-issue Category: A tracking issue for something unstable. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. Z-checksum-freshness Nightly: rebuild detection on file checksum instead of mtime

Comments

@Xaeroxe
Copy link
Contributor

Xaeroxe commented Jun 25, 2024 •
edited by weihanglo
Loading

Summary

Original issue: #6529
Implementation: #14137
Rustc implementation: rust-lang/rust#126930
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#checksum-freshness

Add the ability to use content checksums rather than filesystem mtimes to determine if a crate should be recompiled.

Unresolved Issues

Future Extensions

No response

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Tasks
Beta Give feedback
  1. initial version of checksum based freshness #14137
    A-build-execution A-dep-info A-rebuild-detection A-unstable S-waiting-on-bors Z-checksum-freshness
    weihanglo
  2. Add unstable support for outputting file checksums for use in cargo rust#126930
    A-query-system A-run-make A-testsuite S-waiting-on-bors T-bootstrap T-compiler
    chenyukang
@Xaeroxe Xaeroxe added the C-tracking-issue Category: A tracking issue for something unstable. label Jun 25, 2024
This was referenced Jun 25, 2024
Merged
Merged
Closed
Closed
@epage epage added the A-rebuild-detection Area: rebuild detection and fingerprinting label Jul 8, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue 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.
ijc added a commit to ijc/earthly-lib that referenced this issue Aug 16, 2024
@ijc
rust: Add option not to cache path = cargo dependencies
2d37277 
Due to Earthly's layer cache code added with `COPY` (even with `--keep-ts`) can
end up with timestamps (`mtime`) corresponding to the point of creation of the
cache entry, not the current time.

However on a following build the `target` mount cache may contain builds from
other branches, with different code for those dependencies, which have a newer
`mtime`. In this case `cargo` will think it can use the cached dependency
instead of rebuilding (because the code appears older than the cached entry
under `target`).

This only affects `path` dependencies since other dependencies are versioned
(either from crates.io or from git dependencies) so cargo's cache key will
differ.

This should become unnecessary with rust-lang/cargo#14136
ijc added a commit to ijc/earthly-lib that referenced this issue Aug 16, 2024
@ijc
rust: Add option not to cache path = cargo dependencies
d9c437e 
Due to Earthly's layer cache code added with `COPY` (even with `--keep-ts`) can
end up with timestamps (`mtime`) corresponding to the point of creation of the
cache entry, not the current time.

However on a following build the `target` mount cache may contain builds from
other branches, with different code for those dependencies, which have a newer
`mtime`. In this case `cargo` will think it can use the cached dependency
instead of rebuilding (because the code appears older than the cached entry
under `target`).

This only affects `path` dependencies since other dependencies are versioned
(either from crates.io or from git dependencies) so cargo's cache key will
differ.

This should become unnecessary with rust-lang/cargo#14136
xv-ian-c pushed a commit to ijc/earthly-lib that referenced this issue Aug 19, 2024
@ijc
rust: Replace keep_fingerprints logic with unconditional cargo clean
d24d25c 
Due to Earthly's layer cache code added with `COPY` (even with `--keep-ts`) can
end up with timestamps (`mtime`) corresponding to the point of creation of the
cache entry, not the current time.

However on a following build the `target` mount cache may contain builds from
other branches, with different code for those dependencies, which have a newer
`mtime`. In this case `cargo` will think it can use the cached dependency
instead of rebuilding (because the code appears older than the cached entry
under `target`).

Avoid this by using `cargo clean` to remove the build artifacts for any local
crate. This should become unnecessary with rust-lang/cargo#14136

This replaces the old behaviour of removing the fingerprints directory. Using
`cargo clean` uses a proper cargo API rather than relying on implementation
details like where the fingerprints live and what the consequence removing them
is. It may also keep the cached data smaller since it removes the build
artifacts which will likely never be reused due to the lack of fingerprint.

Note that the previous fingerprint cleaning was subject to a race where a
different parallel build could reintroduce some fingerprints between `DO
+REMOVE_SOURCE_FINGERPRINTS` and the `RUN ... cargo $args`. For that reason the
calls to `cargo clean` here are made within the same `RUN` command so that the
target cache remains locked.

By switching to `cargo metadata` the requirement for `tomljson` is removed.
@ijc ijc mentioned this issue Aug 19, 2024
Open
@gustavovalverde gustavovalverde mentioned this issue Sep 2, 2024
Open
@workingjubilee workingjubilee mentioned this issue Sep 3, 2024
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 2, 2024
@matthiaskrgr
Rollup merge of rust-lang#126930 - Xaeroxe:file-checksum-hint, r=chen…
9030b26 
…yukang

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.
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Oct 3, 2024
@workingjubilee
Rollup merge of rust-lang#126930 - Xaeroxe:file-checksum-hint, r=chen…
44f6275 
…yukang

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.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 3, 2024
@rust-timer
Unrolled build for rust-lang#126930
f6faf34 
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.
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Oct 4, 2024
@workingjubilee
Rollup merge of #126930 - Xaeroxe:file-checksum-hint, r=chenyukang
f4fb99b 
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 added a commit that referenced this issue Oct 4, 2024
@bors
Auto merge of #14137 - Xaeroxe:checksum-freshness, r=<try>
5b99bb7 
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 that referenced this issue Oct 8, 2024
@bors
Auto merge of #14137 - Xaeroxe:checksum-freshness, r=weihanglo
15fbd2f 
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.
@weihanglo weihanglo added the S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. label Oct 10, 2024
@weihanglo

This comment has been minimized.

Sign in to view
@weihanglo

This comment has been minimized.

Sign in to view
@Xaeroxe

This comment has been minimized.

Sign in to view
@epage

This comment has been minimized.

Sign in to view
@weihanglo weihanglo mentioned this issue Oct 23, 2024
Open
@weihanglo weihanglo added the Z-checksum-freshness Nightly: rebuild detection on file checksum instead of mtime label Oct 23, 2024
@weihanglo
Copy link
Member

Moved the performance discussion to #14722, and left this for tracking overall progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-tracking-issue Category: A tracking issue for something unstable. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. Z-checksum-freshness Nightly: rebuild detection on file checksum instead of mtime
Projects
Cargo Roadmap
Status: No status
Milestone
No milestone
Development

No branches or pull requests
3 participants
@epage @Xaeroxe @weihanglo