cargo icon indicating copy to clipboard operation
cargo copied to clipboard

fix: update `resolve_all_features()` to filter pkg deps

Open Saplyn opened this issue 2 months ago • 3 comments

What does this PR try to resolve?

Before, resolve_all_features() isn't filtering dependencies, as it uses the set returned by Resolve::deps() directly which is mostly unfiltered. This PR fixes it by having the set goes through PackageSet::filter_deps()'s filter.

Note that this patch changed resolve_all_features()'s function signature as well as made PackageSet::filter_deps() public.

Close #15399

How to test and review this PR?

As described in #15399, cargo test --frozen should not panic (doesn't try to download unneeded deps) when does the following:

git clone https://github.com/hickory-dns/hickory-dns
cd hickory-dns
git checkout v0.25.1
mkdir .cargo_home
CARGO_HOME=.cargo_home cargo fetch --locked --target=x86_64-unknown-linux-gnu
CARGO_HOME=.cargo_home cargo test --frozen -p hickory-resolver

More discussion on Zulip chat.

Saplyn avatar Nov 07 '25 03:11 Saplyn

Not sure I'm in a good place to give feedback on the change itself at this time but wanted to highlight PR expectations (https://doc.crates.io/contrib/process/working-on-cargo.html#submitting-a-pull-request)

  • A test will be needed for this
  • It is really helpful if you create a test in its own commit that shows the panic and then put this commit on top that removes the panic. Then the diff between these two commits shows how behavior changed. This allows you to verify the test is testing the right thing and gives a good demonstration of the fix

epage avatar Nov 07 '25 22:11 epage

Thanks! I'll look into how to write a test for this pr and try implementing it before making it ready for review

Saplyn avatar Nov 08 '25 06:11 Saplyn

r? @epage

rustbot has assigned @epage. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot avatar Nov 14 '25 08:11 rustbot