Handle rerun-if-changed directives in dependencies #2279
Merged
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? @brson |
There was a failure mode of the handling of the rerun-if-changed directive where it would rerun the build script twice before hitting a steady state of actually processing the directives. The order of events that led to this were: 1. A project was built from a clean directory. Cargo recorded a fingerprint indicating this (for the build script), but the fingerprint indicated that the build script was a normal build script (no manually specified inputs) because Cargo had no prior knowledge. 2. A project was then rebuilt from the same directory. Cargo's new fingerprint for the build script now indicates that there is a custom list of dependencies, but the previous fingerprint indicates there wasn't, so the mismatch causes another rebuild. 3. All future rebuilds will agree that there are custom lists both before and after, so the directives are processed as one would expect. This commit does a bit of refactoring in the fingerprint module to fix this situation. The recorded fingerprint in step (1) is now recorded as a "custom dependencies are specified" fingerprint if, after the build script is run, custom dependencies were specified. Closes rust-lang#2267
alexcrichton
force-pushed
the
rerun-if-changed-rust-file
branch
from
591e339
to
7dcedd8
Compare
|
I don't understand this code. I'm just approving it. @bors r+ |
|
📌 Commit 7dcedd8 has been approved by |
bors
added a commit
that referenced
this pull request
Jan 15, 2016
There was a failure mode of the handling of the rerun-if-changed directive where it would rerun the build script twice before hitting a steady state of actually processing the directives. The order of events that led to this were: 1. A project was built from a clean directory. Cargo recorded a fingerprint indicating this (for the build script), but the fingerprint indicated that the build script was a normal build script (no manually specified inputs) because Cargo had no prior knowledge. 2. A project was then rebuilt from the same directory. Cargo's new fingerprint for the build script now indicates that there is a custom list of dependencies, but the previous fingerprint indicates there wasn't, so the mismatch causes another rebuild. 3. All future rebuilds will agree that there are custom lists both before and after, so the directives are processed as one would expect. This commit does a bit of refactoring in the fingerprint module to fix this situation. The recorded fingerprint in step (1) is now recorded as a "custom dependencies are specified" fingerprint if, after the build script is run, custom dependencies were specified. Closes #2267
|
☀️ Test successful - cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64 |
This was referenced Mar 1, 2016
bors
added a commit
that referenced
this pull request
Mar 4, 2016
All crates being compiled by Cargo are identified by a unique `PackageId` instance. This ID incorporates information such as the name, version, and source from where the crate came from. Package ids are allowed to have path sources to depend on local crates on the filesystem. The package id itself encodes the path of where the crate came from. Historically, however, the "path source" from where these packages are learned had some interesting logic. Specifically one specific source would be able to return many packages within. In other words, a path source would recursively walk crate definitions and the filesystem attempting to find crates. Each crate returned from a source has the same source id, so consequently all packages from one source path would have the same source path id. This in turn leads to confusing an surprising behavior, for example: * When crates are compiled the status message indicates the path of the crate root, not the crate being compiled * When viewed from two different locations (e.g. two different crate roots) the same package would have two different source ids because the id is based on the root location. This hash mismatch has been [papered over](#1697) in the past to try to fix some spurious recompiles, but it unfortunately [leaked back in](#2279). This is clearly indicative of the "hack" being inappropriate so instead these commits fix the root of the problem. --- In short, these commits ensure that the package id for a package defined locally has a path that points precisely at that package. This was a relatively invasive change and had ramifications on a few specific portions which now require a bit of extra code to support. The fundamental change here was to change `PathSource` to be non-recursive by default in terms of what packages it thinks it contains. There are still two recursive use cases, git repositories and path overrides, which are used for backwards compatibility. This meant, however, that the packaging step for crate no longer has knowledge of other crates in a repository to filter out files from. Some specific logic was added to assist in discovering a git repository as well as filtering out sibling packages. Another ramification of this patch, however, is that special care needs to be taken when decoding a lockfile. We now need all path dependencies in the lockfile to point precisely at where the path dependency came from, and this information is not encoded in the lock file. The decoding support was altered to do a simple probe of the filesystem to recursively walk path dependencies to ensure that we can match up packages in a lock file to where they're found on the filesystem. Overall, however, this commit closes #1697 and also addresses servo/servo#9794 where this issue was originally reported.
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.
There was a failure mode of the handling of the rerun-if-changed directive where
it would rerun the build script twice before hitting a steady state of actually
processing the directives. The order of events that led to this were:
indicating this (for the build script), but the fingerprint indicated that
the build script was a normal build script (no manually specified inputs)
because Cargo had no prior knowledge.
for the build script now indicates that there is a custom list of
dependencies, but the previous fingerprint indicates there wasn't, so the
mismatch causes another rebuild.
after, so the directives are processed as one would expect.
This commit does a bit of refactoring in the fingerprint module to fix this
situation. The recorded fingerprint in step (1) is now recorded as a "custom
dependencies are specified" fingerprint if, after the build script is run,
custom dependencies were specified.
Closes #2267