Only compile activated optional dependencies #1812
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
Dependencies aren't necessarily unique per name as the same dependency can show up multiple times in various dependency kind lists, so it's not quite right to use a hash map anyway.
Development dependencies are traversed during the resolution process, causing them to be returned as the list of dependencies for a package in terms of resolution. The compilation phase would then filter these out depending on the dependency's transitivity, but this was incorrectly accounted for when the dependency showed up multiple times in a few lists. This commit fixes this behavior by updating the filtering logic to ensure that only activated optional dependencies (those whose feature names are listed) are compiled. Closes rust-lang#1805
|
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
bors
added a commit
that referenced
this pull request
Jul 17, 2015
Development dependencies are traversed during the resolution process, causing them to be returned as the list of dependencies for a package in terms of resolution. The compilation phase would then filter these out depending on the dependency's transitivity, but this was incorrectly accounted for when the dependency showed up multiple times in a few lists. This commit fixes this behavior by updating the filtering logic to ensure that only activated optional dependencies (those whose feature names are listed) are compiled. Closes #1805
|
☀️ 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-64 |
| // If the dependency is optional, then we're only activating it | ||
| // if the corresponding feature was activated | ||
| let activated = !d.is_optional() || | ||
| self.resolve.features(pkg.package_id()).map(|f| { |
There was a problem hiding this comment.
this usage seems to imply that features() wants to return a set
There was a problem hiding this comment.
How so? Currently it returns Option<&HashSet>, but you're saying it should return HashSet? Certainly possible, just wanted to avoid the extra work on the calling side for now.
There was a problem hiding this comment.
ah, i incorrectly parsed the map call as being on an iterator. I think HashSet would be slightly cleaner, but it's not as important as I thought.
bors
added a commit
that referenced
this pull request
Jul 15, 2019
Remove unused feature filter. NOTE: Do not merge this lightly. This upended my understanding of how the resolver handles features, and I'm still not sure about it. Remove an unused check that an optional dependency is activated. This was originally added 4 years ago in #1812, during a time when this code iterated over the actual dependencies from `Package.dependencies()`. In #5415 this was refactored so that it gets the `deps` list directly from the Resolver, which AFAIK has already filtered out the features. IIUC, this filtering is done [here](https://github.com/rust-lang/cargo/blame/705009eb3828123cc9dbcf5b28988cc63f60b03b/src/cargo/core/resolver/dep_cache.rs#L270-L272).
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.
Development dependencies are traversed during the resolution process, causing
them to be returned as the list of dependencies for a package in terms of
resolution. The compilation phase would then filter these out depending on the
dependency's transitivity, but this was incorrectly accounted for when the
dependency showed up multiple times in a few lists.
This commit fixes this behavior by updating the filtering logic to ensure that
only activated optional dependencies (those whose feature names are listed) are
compiled.
Closes #1805