Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev only features aren't pruned #60

Closed
Tastaturtaste opened this issue Jan 4, 2024 · 1 comment · Fixed by #61
Closed

Dev only features aren't pruned #60

Tastaturtaste opened this issue Jan 4, 2024 · 1 comment · Fixed by #61
Labels
bug Something isn't working

Comments

@Tastaturtaste
Copy link

Describe the bug

A comment in another issue describing the differing behaviour with context

In the PR linked above, I use cargo tree -i intel-mkl-src --all-features -e "no-dev,features" to see if there is a non-dev-dependency on intel-mkl-src, which has a license I don't want to introduce.

  1. Using resolver = "1" in the workspace intel-mkl-src is present due to feature unification with a dev-dependency
  2. Using resolver = "2" in the workspace it is not present anymore (which is what I want)

However cargo deny --log-level error --all-features --exclude-dev check licenses still complains about intel-mkl-src regardless of the used resolver. I would expect cargo-deny to use the same resolver cargo itself uses.

To reproduce

  1. Clone this
  2. Add resolver = "2" in the workspace Cargo.toml
  3. Run cargo tree -i intel-mkl-src --all-features -e "no-dev,features"
  4. Run cargo deny --log-level error --all-features --exclude-dev check licenses
  5. Observe the difference in resolved dependencies

cargo-deny version

cargo-deny 0.14.3

What OS were you running cargo-deny on?

Windows

Additional context

If you need any more context or information please let me know.

@Tastaturtaste Tastaturtaste added the bug Something isn't working label Jan 4, 2024
@Jake-Shadle Jake-Shadle transferred this issue from EmbarkStudios/cargo-deny Jan 5, 2024
@Jake-Shadle Jake-Shadle changed the title Bug: cargo-deny resolves different dependencies than cargo-tree Dev only features aren't pruned Jan 5, 2024
Jake-Shadle added a commit that referenced this issue Jan 12, 2024
Jake-Shadle added a commit that referenced this issue Jan 12, 2024
Previously the graph was built by going from each root package to each of its dependencies recursively until all packages that were referenced were visited. This mostly worked fine, however had issues, namely #60, that were caused by there being tension between our view of the graph and cargo metadata's. `cargo metadata` presents a "complete" view of a graph, however if a user requested only packages for specific platforms or only of specific kinds, we were still including features that `cargo metadata` had resolved with its complete view of the graph, even if those features were only enabled by an edge that was otherwise pruned.

So now, we just does our own resolution by recursively walking each package + feature combination rooted at the features enabled on each root crate which allows us to accurately prune nodes as we go rather than doing complicated fixup passes or the like
Jake-Shadle added a commit that referenced this issue Jan 12, 2024
* Oops

* Add test for #60

* Refactor graph building

Previously the graph was built by going from each root package to each of its dependencies recursively until all packages that were referenced were visited. This mostly worked fine, however had issues, namely #60, that were caused by there being tension between our view of the graph and cargo metadata's. `cargo metadata` presents a "complete" view of a graph, however if a user requested only packages for specific platforms or only of specific kinds, we were still including features that `cargo metadata` had resolved with its complete view of the graph, even if those features were only enabled by an edge that was otherwise pruned.

So now, we just does our own resolution by recursively walking each package + feature combination rooted at the features enabled on each root crate which allows us to accurately prune nodes as we go rather than doing complicated fixup passes or the like

* Update

* Fix lint

* Don't use path deps

* Normalize paths
@Tastaturtaste
Copy link
Author

Thanks for the quick fix for this issue. I confirmed it works with cargo install cargo-deny (without --locked).
Can you tell me what the expected time frame is until this change is reflected in the cargo-deny-action?

Tastaturtaste added a commit to Tastaturtaste/argmin that referenced this issue Jan 14, 2024
stefan-k pushed a commit to argmin-rs/argmin that referenced this issue Jan 14, 2024
Jake-Shadle added a commit to EmbarkStudios/cargo-deny that referenced this issue Jan 19, 2024
This fixes 2 bugs that were originally opened on this repo but really
belong to the crate used to create crate graphs:

- EmbarkStudios/krates#60
- EmbarkStudios/krates#64

Resolves: #584
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant