This repository has been archived by the owner on Sep 28, 2022. It is now read-only.
[ready to review] v2 feature resolver #126
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
This ended up being really straightforward! I discovered a couple of behavior differences around the presence of build scripts -- it would be interesting to report this upstream. Upstream cargo had some fixes that I ended up having to pull in, hence the git dependency.
Also include a failed attempt to allow setting a custom home dir (sorry!)
sunshowers
changed the title
sunshowers
commented
May 2, 2020
| @@ -351,6 +351,98 @@ impl<'a> CargoSetBuildState<'a> { | |||
| } | |||
| } | |||
|
|
|||
| fn new_v2(self, query: FeatureQuery<'_>) -> CargoSet { | |||
There was a problem hiding this comment.
The core algorithm starts here. Note the XXX marks about whether some cases are bugs in upstream cargo.
sunshowers
force-pushed
the
cargosim-v2
branch
14 times, most recently
from
73961df
to
51e26b1
Compare
sunshowers
commented
May 3, 2020
| pub fn triple_strategy() -> impl Strategy<Value = Option<&'static str>> { | ||
| // Filter out Apple platforms because rustc requires the Apple SDKs to be set up for them. | ||
| let platform_strategy = Platform::filtered_strategy( | ||
| |triple| !triple.contains("-apple-"), |
bors
added a commit
to rust-lang/cargo
that referenced
this pull request
May 5, 2020
features: allow activated_features_unverified to communicate not-present Hi there! :) I'm currently writing [`cargo-guppy`](https://github.com/facebookincubator/cargo-guppy), a toolkit for analyzing Rust dependency graphs. As part of that I'm writing some tests to compare guppy to `cargo` (see facebookarchive/cargo-guppy#126), to ensure that it produces the same results for the subset of analyses `cargo` can do. While writing tests I found it useful to distinguish between packages that weren't present at all and packages that were activated but with no features. This commit adds that functionality to the `_unverified` method. (BTW, of possible interest to @ehuss, I've also found some interesting behavior differences between the v1 and v2 resolvers. Will file issues for them soon!)
This works by: * setting up a real, fairly complex fixture * generating random queries using proptest * running them through cargo and guppy * ensuring that they produce the same results Not only does this give us high confidence that cargo and guppy produce the same results, it also gets us most of the way to facebookarchive#83. Most of the work in this commit is simply setting up all the necessary test infrastructure. We piggy-back on existing proptest infrastructure as much as possible.
Do this in a separate job to allow for greater parallelization.
Make proptests more interesting by generating random platforms for tests.
metajack
approved these changes
May 5, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Includes:
Of particular interest are two points of note between the V1 and V2 resolvers that I thought were surprising:
I'll file issues against the cargo repo soon.