A visitor API for exploring a package's features and their dependencies

[regexident]

An exploration for how Metadata could provide a flexible API for processing a package's feature dependency graph.

Resolves https://github.com/oli-obk/cargo_metadata/issues/279

[regexident]

Personally, I'm unfortunately not a fan of this API. I find it very confusingly structured and documented, and it isn't clear to me that the flexibility a visitor offers compared to other approaches is valuable for many use cases.

I can imagine two modes where feature information is useful:

• Which things are directly enabled by which features? This is analogous to "what's written in the Cargo.toml next to each feature."
• What are all the things enabled by each feature? This is analogous to "if I enable feature X, what do I actually get."

Obviously, a visitor can cover both modes and everything in between. But it isn't clear to me that there is any use case that lies in between. So I can't justify the complexity of either maintaining this API, nor of users learning how to use it.

For my 2 cents, I'd prefer an API that offered functionality for each of the two modes, and was designed to be simple and easy to understand.

I'd 100% agree if such a Visitor API would be the only API provided. For common use-cases I would expect the crate to provide batteries-included "collectors", like the TransitiveFeatureCollector that I use in the tests.

Going with a visitor API doesn't have to mean "difficult to use". Instead it allows you to neatly compartmentalize the logic in a "walker" and avoid diluting the API of Metadata with methods and options for all the different usage needs.

Also speaking from an implementation point of view: migrating to a visitor made the implementation of the feature graph traversal logic much simpler and cleaner. And by being driven from outside having a visitor also makes the implementation less likely to accumulate all kinds of intricate control logic (to covered different use cases, etc) in the future.

[obi1kenobi]

Pulling your comment out of the thread for broader visibility:

But whether or not this method should fail on missing packages depends on the use-case, I think. With a visitor this decision is totally up to you, without bloating the crate's API with options.

Similarly one may want to be able to pick between different collection modes: recursive (i.e. transitive) and non-recursive (i.e. immediate) feature dependencies. Again, a visitor API leaves this up to the user, without bloating the crate's API with options.

I don't find this argument convincing, personally. Leaving more things up to the user is not always a good choice.

cargo_metadata already has low-level functionality that supports all the possible special cases and leaves maximum choice up to the users. I don't think it's worth adding a new and very complex layer that also tries to be super generic.

I think what's missing is a simple way to answer the most common questions:

• What is directly enabled by this feature?
• What are all things transitively enabled by this feature?

Something a user could easily find, quickly understand, and immediately use in their own code. A visitor API in my book accomplishes none of that.

I'm not the cargo_metadata maintainer so the final call isn't mine to make. But as a user of cargo_metadata, I wouldn't switch from what I currently have to a visitor-based API. It would make everything more complex for me to understand and maintain, not less.

[regexident]

I'm not the cargo_metadata maintainer so the final call isn't mine to make. But as a user of cargo_metadata, I wouldn't switch from what I currently have to a visitor-based API. It would make everything more complex for me to understand and maintain, not less.

And I wouldn't expect you to. For the 80% I'd expect convenient methods to exist. The for other 20% a visitor API is preferable.

One such use case being mine (the one you reminded me of, when you brought up your use case): I'd need to be able to selectively block certain downstream features from (it and its dependencies) being collected. With a method that only returns the immediate dependencies I'd be out of luck as I would have to write the recursive traversal logic myself, which is arguably more difficult (to do right) than implementing a visitor. And a one-off method for transitive feature dependencies wouldn't be of much use either.

[oli-obk]

I also feel like the visitor is overkill, but until we get generators, I can also see how an iterator based version may be annoying to implement.

I'd be fine adding this under an unstable_visitor feature gate, and using it to implement a higher level API (along with giving you the ability to play with it directly), but I'm hesitant to support a visitor API forever.