feat: add optional dep-kind parameter

[piotr-sk]

Adds an optional --dep-kinds flag allowing to filtering out dev or build dependencies similarly to the edges option in cargo tree (https://doc.rust-lang.org/cargo/commands/cargo-tree.html)

Possible values are:

• all (default)
• normal
• build
• dev
• no-normal
• no-build
• no-dev

The goal was to modify as little as possible the existing code base and to add the additional filtering only if the optional parameter is set.
Ideally this filter will be added to the cargo metadata, so that the commit can be reworked to apply the filter directly in function add_packages_for_platform()

Closes #101

[piotr-sk]

@cgwalters it turned out to be more complicated that I initially thought, but here is a solution with hopefully minimal impact to everyone not needing the additional filtering.

Let me know what you think.

[cgwalters]

I very superficially skimmed this and it seems sane but I'd like to cut a new release first before merging and then take this one for the next cycle if that's OK.

[piotr-sk]

OK provided we can agree on a reasonable timeline for your next cycle 😉

The change is fully backwards compatible and nothing additional is computed or filtered until the dep-kinds flag is set via command line or toml file.

Some background to the last commit: the only dev dependency of this crate once_cell becomes a normal dependency after release 0.5.15 because the update of tempfile to v3.13.0 pulls it in. See the cargo tree output after the update:

$ cargo tree --package once_cell --invert
once_cell v1.20.2
├── openssl v0.10.68
│   └── cargo-vendor-filterer v0.5.14
├── serial_test v3.1.1
│   [dev-dependencies]
│   └── cargo-vendor-filterer v0.5.14
└── tempfile v3.13.0
    └── cargo-vendor-filterer v0.5.14
[dev-dependencies]
└── cargo-vendor-filterer v0.5.14

To ensure the logic is tested I pull in a serial_test dev dependency and use it marking some tests for parallel execution (i.e. no slow-down to testing).

Should bringing this dev-dependency for testing purposes be a problem, I can remove problematic tests or use a different crate.

[cgwalters]

The serial_test seems fine to me, skimming https://crates.io/crates/serial_test/reverse_dependencies it's pulled in by rustix e.g.

Still without looking deep at the code yet...it looks like the tracking issue you filed against cargo got put in their (looong) backlog, which is totally understandable.

The bar is relatively lower for landing features here 😄 (which is both good and bad). My biggest concern is:

• cargo tree certainly looks like it was mainly created for human consumption...yes, with --quiet at least the Unicode line drawing characters are omitted...but still, I'd be a bit happier if it had a --json that was supported with cargo_metadata equivalent.

Subconcerns:

• Relatively minor bikeshed: What if cargo vendor decides to accept --dep-kinds but wants to change some of the possible values? So far there's only a bit of precedent in this project for CLI options accepted only by this tool but not cargo vendor. I wonder if we should make it something a bit more "namespaced" like --filter-dep-kinds or something and save the --dep-kinds for cargo vendor (and if it gets accepted there we make it an alias?)
• This is effectively a new algorithm for computing dependencies that only shares high level logic with the cargo metadata path...I can't think of a concrete problem but I'd be a bit more comfortable if we e.g. always used cargo tree perhaps so there was only one dependency-parsing path? (Yes, more invasive...maybe in CI we could assert that the two paths compute the same results?)

[piotr-sk]

Thanks for the review.

I agree with your main point that cargo tree is intended for people and not machines.

Initially, I started by changing the add_packages_for_platform() to use cargo tree instead of cargo metadata, but I do not think it would be a good solution in the long run for the reasons you've mentioned above.
Moreover, when cargo evolves and rust-lang/cargo#10718 gets implemented, we should be able to use cargo metadata only.
To prepare for this, I propose to keep the usage of cargo tree in a separate module and only for people who explicitly set this parameter to anything else than the default, so it can be more easily removed afterwards.

I like your idea of avoiding a potential conflict with the cargo metadata CLI parameter name.
When I was updating the documentation, the name of --keep-dep-kinds seemed a more straightforward solution.
What do you think?

[cgwalters]

Just one doc change that needs doing and a nit, otherwise looks good to me

[cgwalters]

Ah yeah now we need to drop the unnecessary vec![] which can just be []

[cgwalters]

To prepare for this, I propose to keep the usage of cargo tree in a separate module and only for people who explicitly set this parameter to anything else than the default, so it can be more easily removed afterwards.

SGTM.

When I was updating the documentation, the name of --keep-dep-kinds seemed a more straightforward solution.
What do you think?

This is also fine by me. It seems somewhat unlikely that cargo would add the option with the same name but different functionality in the end too.

[cgwalters]

Thanks for your contribution!

OK now I think we should get #107 in after the submitter comes back with fixes and then I think we can do a new release later this next week.

It's probably about time to submit that next release to various Rust news channels like reddit.com/r/rust perhaps? Probably everyone who needs this project has already found it but still it's important to talk about what we're doing sometimes.

[piotr-sk]

Thanks for your detailed review and I agree with the proposed next steps.

I think it makes sense to mention this project in relevant open cargo issues – this is how we found your project as a solution to a not-yet-solved point there.