Improve CI caching by skipping mtime checks for paths in $CARGO_HOME #11613
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
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon. Please see the contribution instructions for more information. |
rustbot
added
the
S-waiting-on-review
arriven
force-pushed
the
fix/11083-rerun-if-changed-published-sources-dir
branch
from
53e1129 to
111a4a6
Compare
epage
reviewed
Jan 23, 2023
epage
reviewed
Jan 23, 2023
Comment on lines
+4874
to
+4880
| [COMPILING] mylib-sys [..] | ||
| [RUNNING] `rustc --crate-name build_script_build [..] | ||
| [RUNNING] `[..]build-script-build[..]` | ||
| [RUNNING] `rustc --crate-name mylib_sys [..] | ||
| [CHECKING] foo [..] | ||
| [RUNNING] `rustc --crate-name foo [..] | ||
| [FINISHED] [..]", |
There was a problem hiding this comment.
I believe the test looks good but one way to test the test is if you have one commit with the test showing the old behavior and then the commit with the fix also updates the test.
Not required but something you can do to be extra sure and communicate that to reviewers.
epage
reviewed
Jan 23, 2023
arriven
force-pushed
the
fix/11083-rerun-if-changed-published-sources-dir
branch
from
111a4a6 to
9e01c8b
Compare
epage
changed the title
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
bors
pushed a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 25, 2023
11 commits in 985d561f0bb9b76ec043a2b12511790ec7a2b954..3c5af6bed9a1a243a693e8e22ee2486bd5b82a6c 2023-01-20 14:39:28 +0000 to 2023-01-24 15:48:15 +0000 - Add a note about verifying your email address on crates.io (rust-lang/cargo#11620) - Improve CI caching by skipping mtime checks for paths in $CARGO_HOME (rust-lang/cargo#11613) - test: Update for clap 4.1.3 (rust-lang/cargo#11619) - Fix unused attribute on Windows. (rust-lang/cargo#11614) - [Doc]: Added links to the `Target` section of the glossary for occurences of `target triple` (rust-lang/cargo#11603) - feat: stabilize auto fix note (rust-lang/cargo#11558) - Clarify the difference between CARGO_CRATE_NAME and CARGO_PKG_NAME (rust-lang/cargo#11576) - Temporarily pin libgit2-sys. (rust-lang/cargo#11609) - Disable network SSH tests on windows. (rust-lang/cargo#11610) - fix(toml): Add `default-features` to `TomlWorkspaceDependency` (rust-lang/cargo#11409) - doc(contrib): remove rls in release process (rust-lang/cargo#11601)
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jan 25, 2023
Update cargo 11 commits in 985d561f0bb9b76ec043a2b12511790ec7a2b954..3c5af6bed9a1a243a693e8e22ee2486bd5b82a6c 2023-01-20 14:39:28 +0000 to 2023-01-24 15:48:15 +0000 - Add a note about verifying your email address on crates.io (rust-lang/cargo#11620) - Improve CI caching by skipping mtime checks for paths in $CARGO_HOME (rust-lang/cargo#11613) - test: Update for clap 4.1.3 (rust-lang/cargo#11619) - Fix unused attribute on Windows. (rust-lang/cargo#11614) - [Doc]: Added links to the `Target` section of the glossary for occurences of `target triple` (rust-lang/cargo#11603) - feat: stabilize auto fix note (rust-lang/cargo#11558) - Clarify the difference between CARGO_CRATE_NAME and CARGO_PKG_NAME (rust-lang/cargo#11576) - Temporarily pin libgit2-sys. (rust-lang/cargo#11609) - Disable network SSH tests on windows. (rust-lang/cargo#11610) - fix(toml): Add `default-features` to `TomlWorkspaceDependency` (rust-lang/cargo#11409) - doc(contrib): remove rls in release process (rust-lang/cargo#11601) r? `@ghost`
|
Perhaps an unintended effect of this change is that $CARGO_HOME/bin is also ignored, and that can be pretty surprising. See aya-rs/aya#668 for the nasty workaround that was required. |
|
Yep. The proposed fix is here #12090 (comment). Just no one yet picked it up. |
|
Looks like #12369 is working on this. |
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.
Skip mtime checks for paths pointing into
$CARGO_HOMEto avoid rebuilds when only caching $CARGO_HOME/registry/{index, cache} and $CARGO_HOME/git/db and some of the dependencies havererun-if-changed=directoryin theirbuild.rsI considered skipping mtime checking only on
$CARGO_HOME/registry/srcbut it looks like other functionality (like downloading a newer version of dependency) is unaffected by this and this way we also cover the same issue with git based dependencies (except the part wherecargois forced to re-fetch submodules if$CARGO_HOME/git/checkoutsis missing) and it is more in line with the discussion in #9455Fix #11083
Credit @weihanglo for the test (I did add a case of checking that dependency update still triggers a rebuild but they are the original author of the rest of the test)