Include build script execution in the fingerprint. #6720
Merged
+170
−38
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
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
|
@bors: r+ Thanks again for diagnosing this! |
|
📌 Commit 035913f has been approved by |
|
⌛ Testing commit 035913f with merge 57d2cac8444f7352dc0667131d343953d5c0514f... |
|
💔 Test failed - checks-travis |
|
@bors retry |
|
⌛ Testing commit 035913f with merge 3a13c482526b97c8826263b8a8b56e98fbc3fd7c... |
|
💔 Test failed - checks-travis |
|
@bors retry |
bors
added a commit
that referenced
this pull request
Mar 6, 2019
Include build script execution in the fingerprint. This adds information about the execution of a build script to the fingerprint. Previously, no information was included, and cargo relied on dirty propagation in `JobQueue` to trigger recompiles. However, if two separate targets are built via separate commands (such as `cargo build` then `cargo test`), the second command did not know that the build script was updated, and thus was incorrectly treated as "fresh". This works by including the timestamp of the last time the build script was ran in the fingerprint. For overridden build scripts, it includes the replaced output. Fixes #4979
|
☀️ Test successful - checks-travis, status-appveyor |
bors
added a commit
to rust-lang/rust
that referenced
this pull request
Mar 9, 2019
Update cargo, rls, books ## cargo 10 commits in 5c6aa46e6f28661270979696e7b4c2f0dff8628f..95b45eca19ac785263fed98ecefe540bb47337ac 2019-02-22 19:32:35 +0000 to 2019-03-06 19:24:30 +0000 - Relax some rustdoc tests. (rust-lang/cargo#6721) - Include build script execution in the fingerprint. (rust-lang/cargo#6720) - part of the infrastructure for public & private dependencies in the resolver (rust-lang/cargo#6653) - Bump to 0.36.0 (rust-lang/cargo#6718) - Some test/bench-related tweaks (rust-lang/cargo#6707) - Fix links to the permanent home of the edition guide. (rust-lang/cargo#6703) - HTTPS all the things (rust-lang/cargo#6614) - Cargo test quicker by not building untested examples when filtered (rust-lang/cargo#6683) - Link from ARCHITECTURE.md to docs.rs and github (rust-lang/cargo#6695) - Update how to install rustfmt (rust-lang/cargo#6696) ## rls 9 commits in 0d6f53e1a4adbaf7d83cdc0cb54720203fcb522e..6a1b5a9cfda2ae19372e0613e76ebefba36edcf5 2019-02-14 07:52:15 +0000 to 2019-03-04 20:24:45 +0000 - Update cargo and clippy. (rust-lang/rls#1388) - catch up rust-lang/rust PR#58321 (rust-lang/rls#1384) - Apply Clippy fixes (rust-lang/rls#1327) - Various cosmetic improvements (rust-lang/rls#1326) - Make test `RlsHandle` transport-agnostic (rust-lang/rls#1317) - Translate remaining tests (rust-lang/rls#1320) - Remove unnecessary #![feature]s (rust-lang/rls#1323) - Update Clippy (rust-lang/rls#1319) - Update Clippy (rust-lang/rls#1315) cc @Xanewok ## Books See #58936 for details.
This was referenced Mar 10, 2019
bors
added a commit
that referenced
this pull request
Mar 11, 2019
Fingerprint build script deps only for path packages. #6720 introduced some protection that if there is a build script, and two commands are used (such as `cargo build` then `cargo test`), the second command would correctly get rebuilt. However, the way it was implemented relies on mtimes working correctly. A common use case is to cache built dependencies in Docker, and Docker zeros the nanoseconds from mtime when the image is saved. This caused all packages that had build scripts to get rebuilt when the Docker image runs. The solution here is to only use the #6720 protection for local (path) packages. This runs under the assumption that mtimes need to work for those anyways. The consequence is that the scenario in #6720 will no longer work for detecting changes in registry dependencies with build scripts. Fixing that final edge case is nontrivial. Since it is unlikely to happen often, I figure this workaround should be sufficient for now. cc rust-lang/rust-playground#469 cc rust-lang/rust#59061
This was referenced Mar 21, 2019
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.
This adds information about the execution of a build script to the fingerprint. Previously, no information was included, and cargo relied on dirty propagation in
JobQueueto trigger recompiles. However, if two separate targets are built via separate commands (such ascargo buildthencargo test), the second command did not know that the build script was updated, and thus was incorrectly treated as "fresh".This works by including the timestamp of the last time the build script was ran in the fingerprint. For overridden build scripts, it includes the replaced output.
Fixes #4979