search

pubgrub-rs / advanced_dependency_providers

experiment with the DependencyProvider trait
1 stars 0 forks source link

"features" #1

Closed Eh2406 closed 3 years ago

Eh2406 commented 3 years ago

"features" (in cargo) are a form of build flag that can enable optional dependencies. They were added to the dart implementation but where never exposed in the user interface.

This issue is to split off and continue the conversation from https://github.com/pubgrub-rs/pubgrub/issues/39

Eh2406 commented 3 years ago

To summarize the comments from https://github.com/pubgrub-rs/pubgrub/issues/39 so far:

mpizenberg: https://github.com/pubgrub-rs/pubgrub/issues/39#issuecomment-710786729

features

It seems that features effectively change the dependencies of a given package.

Let's start with a few thoughts to imagine if this could be done without changing anything in pubgrub. We could consider that every feature is actually a different package. The package "a" is different from the package "a-feat" since it has a different set of dependencies. So for direct dependencies, there is no issue. Now the question is how is this dealt with in cargo when multiple packages in the dependency tree are asking for "a" and "a-feat"? What should happen if they are required at the same version ("a:1", "a-feat:1") or at a different version ("a:1", "a-feat:1.2"). The obvious thing of considering "a" and "a-feat" independent packages does not care. But it means that cargo needs to build them with different names. Is it what Cargo currently does?

aleksator: https://github.com/pubgrub-rs/pubgrub/issues/39#issuecomment-710797433

features

Now the question is how is this dealt with in cargo when multiple packages in the dependency tree are asking for "a" and "a-feat"? What should happen if they are required at the same version ("a:1", "a-feat:1")

Everyone would be given "a-feat" even when they asked for just "a". That's the whole point of features: enabling additional functionality without impacting those who don't need it. That's why features should only be additive.

or at a different version ("a:1", "a-feat:1.2").

I'm not sure if cargo will produce exactly that ("a:1", "a-feat:1.2") or would do not optimal ("a-feat:1", "a-feat:1.2"), i.e. enable features for everyone.

Eh2406 commented 3 years ago

or at a different version ("a:1", "a-feat:1.2").

I'm not sure if cargo will produce exactly that ("a:1", "a-feat:1.2") or would do not optimal ("a-feat:1", "a-feat:1.2"), i.e. enable features for everyone.

If the example is "1" and "1.2" they are semver compatible so "a" will only be built once it will be "a:1.2" and it will have "feat" enabled. Now if the example is "1" and "2" then they are not semver compatible so both will be built and only one will have "feat" enabled.

So I think this can be modeled correctly by:

I think this will work correctly.

(BTW, checked some notes from when the blog post came out and from talking to Natalie a year and a half ago, and this is way more elegant than what I was thinking then.)

I still think that Natalie decided to work these things into the the Dart implementation and so we should check if she had a good reason. It may be that for separation of concerns reasons we don't want to copy her decision, but I would like to understand it.

mpizenberg commented 3 years ago

This has been implemented in #7