Fix cargo tree -p output with feature-unification=workspace#16584
Fix cargo tree -p output with feature-unification=workspace#16584terror wants to merge 4 commits intorust-lang:masterfrom
cargo tree -p output with feature-unification=workspace#16584Conversation
rustbot
added
Command-tree
S-waiting-on-review
terror
changed the title
cargo tree -p to respect package filter with feature-unification=workspacecargo tree -p output with feature-unification=workspace
terror
force-pushed
the
tree-workspace-output
branch
from
84de652 to
d526e1e
Compare
|
|
||
| entry_ids | ||
| .into_iter() | ||
| .filter(|id| requested_ids.contains(id)) |
There was a problem hiding this comment.
Something I need to dig into is why this filtering happens rather than just returning requested_ids
src/cargo/ops/tree/mod.rs
Outdated
| let root_specs = if opts.invert.is_empty() { | ||
| specs | ||
| let root_ids = if opts.invert.is_empty() { | ||
| let entry_ids = ws_resolve.targeted_resolve.specs_to_ids(&specs)?; |
There was a problem hiding this comment.
Is this a bug with the specs that is returned? Are there other places that are negatively impacted by this behavior?
There was a problem hiding this comment.
Afaict, this appears specific to tree root selection; resolve_ws_with_opts behavior is intentional for workspace feature resolution.
There was a problem hiding this comment.
Could you go into detail on why you feel that way? Feature resolution has already happened by the end of the function. Why would a caller need specs unrelated to their use?
There was a problem hiding this comment.
Apologies if I'm not making myself clear enough, I'm still getting up to speed with this area of the codebase 😅
My understanding here is that in this function specs is the user request, while each SpecsAndResolvedFeatures entry can have a different specs scope depending on feature-unification mode. I renamed that loop binding to entry_specs to make that distinction explicit.
We'll then build the graph with the feature resolution scope for that entry, but roots should still respect the users choice of packages (i.e. -p), so I compute requested_ids from outer specs and intersect with entry_ids from entry_specs.
So in the end resolve_ws_with_opts behavior is intentional here, with the core issue being how tree interpreted it.
There was a problem hiding this comment.
That is talking about cargo tree but not compilation.
There was a problem hiding this comment.
Right, because this PR (and underlying issue) is scoped to cargo tree. Compilation is doing something similar however.
What I'm trying to establish as common understanding here is that cargo tree used specs derived from resolve_ws_with_opts as display roots, which works in some modes, but with feature-unification=workspace those specs are intentionally broader (workspace-wide), so it over-printed roots (the issue at hand here).
There was a problem hiding this comment.
I understand. What I'm looking at with this is where is the correct place to resolve this problem. Is resolve_ws_with_opts upholding its promises as an abstraction? If it is, then yes, this is on cargo trees side. If not, then we should fix resolve_ws_with_opts. From my high level inspection, this looks like resolve_ws_with_opts is not behaving correctly and cargo tree is correctly doing what it is told. If that is not the case, I want to understand why.
terror
force-pushed
the
tree-workspace-output
branch
from
d526e1e to
e1613f8
Compare
4 participants
Resolves #16583
When using
feature-unification=workspace, runningcargo tree -p <package>incorrectly displayed trees for all workspace members instead of just the requested package.