rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.86k stars 2.43k forks source link

Mutually exclusive features #2980

Open retep998 opened 8 years ago

retep998 commented 8 years ago

In Windows API there is the notion of a family and partitions. Your choice of family is basically which platform you are targeting and there is a choice between WINAPI_FAMILY_DESKTOP_APP WINAPI_FAMILY_PC_APP WINAPI_FAMILY_PHONE_APP WINAPI_FAMILY_SYSTEM and WINAPI_FAMILY_SERVER. Your choice of family in turn enables certain partitions which enable access to certain functionality. At the moment I cannot use cargo features for this because they are not mutually exclusive, so two downstream dependencies could specify different families which is completely wrong. Even if I use a build script to ensure only one such feature is selected, it would be a nightmare to ensure that every single downstream dependency chooses the same family, threading the choice of family feature through long chains of transitive dependencies that shouldn't have to care about that choice.

What I think would work well here is some sort of feature selection that has to choose exactly one option from a closed set, and that choice is controlled by the root crate. All dependencies that are affected by that choice will then need some way to be informed of the choice so they can adjust which functionality to use depending on that choice.

At the moment the only workaround I see is for the user to pass an environment variable while building the project so winapi's build script can read that to choose the family followed by emitting key=value stuff which downstream dependencies can read via build scripts through the DEP_WINAPI_key=value environment variables to emit cfgs to control their own code.

Also, this can also apply to other choices, such as which version of Windows you want to target, newer versions have more functionality that dependencies might want to be able to take advantage of, but the user might also want them to target an older version instead so it can run on more systems.

Boddlnagg commented 8 years ago

I just had this idea, but haven't thought it through yet: To get mutually exclusive features couldn't you just add some piece of code to the crate that is gated on the features that should not be used together and make rustc output some error message that actually points to the problem (do we have something like #error in C, probably a macro?).

This could be considered an ugly workaround, but maybe it allows the greatest flexibility.

retep998 commented 8 years ago

@Boddlnagg Suppose the user is working on their project apple which depends on banana which in turn depends on cat which in turn depends on winapi. Now cat is capable of supporting two different winapi families, so how does it deal with this? Suppose it decides to provide two features for each of these options so the user can decide, but wait, the user is not the one depending on cat, it is banana. So banana is now responsible for re-exporting those features, even though it does not use winapi directly, and should not care at all which winapi family is being used. But now realize that winapi has a ton of reverse dependencies and the poor user's apple project ends up depending on a whole messy dependency graph with winapi up many of those branches. Do you really expect every single crate in between apple and winapi to diligently re-export features so that the user can choose which family they want?

Right now the way features work is that foo specifies which features it requires from bar and the user only has any say in this if foo provides its own features to control this, and every single transitive dependency downstream re-exports these features. What I really think we need instead is a way for foo to specify which features it could use from bar, but not necessarily require. Then the user can choose which of those features it wants and foo will be notified in some manner of which features it actually got so it can adjust its code appropriately. Imagine something like hyper where right now if you want to disable the ssl feature (due to that annoying openssl dependency) you have to hope every single crate which depends on hyper, even transitively, re-exports the option to not depend on the ssl feature. If instead each of the crates that directly depends on hyper said it merely could use the ssl feature if it was available, then the user could choose whether or not to enable ssl from a single point and all the reverse dependencies of hyper would be aware of this and adjust accordingly. Without a system like this, I don't think it would be possible to sanely have mutually exclusive features.

mat8913 commented 7 years ago

Maybe allow specifying feature conflicts in Cargo.toml, such as:

[dependencies.awesome]
version = "1.3.5"
features = ["!secure-password", "civet"]

Which means this package depends on "awesome" but requires the "secure-password" feature to be disabled.

Also,

[features]
go-faster = []
secure-password = ["!go-faster", "bcrypt"]

means that this package provides features "go-faster" and "secure-password". But if "secure-password" is enabled, "go-faster" must be disabled.

luser commented 6 years ago

I've run into this in practice before with the flate2 crate, which has features to select between using zlib and miniz as the implementation. It's an either-or choice, but nothing in cargo actually ensures that.

Your use case here seems like it's not well-served by features at all, honestly. cargo features as-implemented are allowed to be required by arbitrary crates in the dependency graph, so you'd have to have some way to completely override that. It's more similar to openssl-sys's OPENSSL_STATIC option, which is processed by its build script from the environment, or -Ctarget-feature=+crt-static. That is, it seems like a configuration option that can only be chosen by the crate at the top of the dependency tree which is producing a native binary output.

stale[bot] commented 6 years ago

As there hasn't been any activity here in over 6 months I've marked this as stale and if no further activity happens for 7 days I will close it.

I'm a bot so this may be in error! If this issue should remain open, could someone (the author, a team member, or any interested party) please comment to that effect?

The team would be especially grateful if such a comment included details such as:

Thank you for contributing!

(The cargo team is currently evaluating the use of Stale bot, and using #6035 as the tracking issue to gather feedback.)

If you're reading this comment from the distant future, fear not if this was closed automatically. If you believe it's still an issue please leave a comment and a team member can reopen this issue. Opening a new issue is also acceptable!

retep998 commented 6 years ago

Is this still relevant?

Yes.

If so, what is blocking it?

Someone needs to come up with a good design for a better feature system that supports mutually exclusive features among other things.

Is it known what could be done to help move this forward?

Convince someone to work on such a design.

thomcc commented 6 years ago

Someone needs to come up with a good design for a better feature system that supports mutually exclusive features among other things.

What's wrong with the design suggested by @mat8913? https://github.com/rust-lang/cargo/issues/2980#issuecomment-270554758

It doesn't cover "among other things", but seems to address the issue while being flexible. (Some consideration would need to be given for the --all-features flag, but an error is seems sufficient)

TheDan64 commented 5 years ago

In inkwell, we currently support 9 different versions of LLVM. This is represented by feature flags which correspond to different versions of our dependency, the llvm-sys crate. We enforce that they are used exclusively though some macro trickery. But cargo has no understanding of this, and so cannot enforce it appropriately. So currently we have multiple branches which just have the slight difference of changing the llvm-sys version number, and version feature flag.

Mutually exclusive feature flags would allow us to deploy (also to crates.io) one single release where you specify a feature flag, and cargo builds a crate graph that is able to use different dependency versions depending on the flag.

Some previous discussions: https://github.com/rust-lang/cargo/issues/5653#issuecomment-430489303 https://internals.rust-lang.org/t/mutually-exclusive-feature-flags/8601

What's wrong with the design suggested by @mat8913? #2980 (comment)

The second example in particular can end up being pretty verbose I think. As mentioned previously, in inkwell we have 9 features that we use exclusively for different LLVM versions. So it would look something like this:

[features]
llvm3-6 = ["!llvm3-7", "!llvm3-8", "!llvm3-9", ..., "!llvm8-0"]
llvm3-7 = ["!llvm3-6", "!llvm3-8", "!llvm3-9", ..., "!llvm8-0"]
...
llvm8-0 = ["!llvm3-6", "!llvm3-7", "!llvm3-8", ..., "!llvm7-0"]

I suppose this could be simplified a bit, ie if 3-6 has 3-7 excluded, then 3-7 doesn't need to list 3-6 explicitly. But this seems to be too implicit and potentially a maintenance headache.

It would be clearer to collect exclusions into named sets like so:

[mutually_exclusive_features]
llvm_versions = ["llvm3-6", "llvm3-7", "llvm3-8", ..., "llvm8-0"]

That way, we can keep verbosity down and the compiler can reference the set name in error messages. IE "llvm3-6 and llvm3-7 features cannot be combined as defined in the llvm_versions exclusion set"

To reformat @mat8913's examples:

[features]
go-faster = []
secure-password = ["bcrypt"]

[mutually_exclusive_features]
speed_or_security = ["go-faster", "secure-password"]

[dependencies.awesome]
version = "1.3.5"
features = ["civet"]
mutually_exclusive_features = ["secure-password"]

Note: Instead of mutually_exclusive_features we could just call it mutex_features for brevity since mutex is commonly known to mean "mutually exclusive"

Some consideration would need to be given for the --all-features flag, but an error is seems sufficient

Agreed. --all-features flag should straight up error when mutually exclusive features are specified

seanyoung commented 4 years ago

Another way of doing this is to provide "values" for features.

[features]
llvm="9"
flate2="zlib"
winapi_family="desktop_app"

Features could default to true or 1.

GopherJ commented 4 years ago

hello guys, I've been waiting this for more than one year, any progress? It's open at: Aug 9, 2016 ~

tuxxy commented 4 years ago

Still waiting on this, it would be very nice to have. :)

tesuji commented 4 years ago

Disclaimer: I don't exactly know how to use below build systems, but here what I found on the topic in them:

TheDan64 commented 4 years ago

Would anyone on the cargo team be willing to mentor an implementation for this? I took a shot at it myself but didn't get much farther than adding an experimental cargo feature

Eh2406 commented 4 years ago

Thanks for offering to help. There are many levels of things to work on for this. Eventually it will probably need an RFC. But given its complexity IMO the implementation should probably come first.

There is work to be done on adding syntax and hammering out the exact semantics. Do we just add a negative feature "!foo" or do we add a new "capability" structure? Then there is figuring out how this is stored in the index, and making sure old cargo do something reasonable. We don't want another misread the index cve. There are probably other things that need to be worked on that I have not thought of.

The part I can provide mentorship on is the dependency resolver. Someone may object that "doesn't this mean that determining witch dependencies have compatible feature requests will now be an SAT problem?" and the answer is Yes. But that is not such a big deal as witch dependencies have compatible dependencies is already an SAT problem. (There are several ways to express mutually exclusive features in terms of dependencies. To take the example from the OP, have a crate for each WINAPI_FAMILY_* each of witch has one published version and has the link attribute set to "WINAPI_FAMILY", then have winapi optionally depend on all of those crates. 🎉 only one of the optionals dependencies will be active.)

So adding support for this to the dependency resolver may be "just":

Ekleog commented 4 years ago

I think one of the things that has been mentioned in other places as well as above was, that, to avoid misuse of negative features that'd lead to actually-bad SAT problems, the mutually exclusive features should be possible to set only from the top-level binary crate configuration.

With this in mind, mutually-exclusive features probably don't require any change to the solver other than refusing to set them from library crates, and having some syntax to set them properly in the binary crates. Now, I'm saying this without ever having had a look at cargo's feature solver, so I may be completely wrong :)

jonhoo commented 4 years ago

To contribute my small part to this conversation, I've recently come across a case where the right outcome seems like it'd be to compile two independent copies of a crate if mutually exclusive features are used. Essentially, treat "exclusive" features are constructing a unique version of the crate in question that is distinct from the same version of the crate without the feature. In theory (though maybe not in practice), cargo should be able to compile multiple instances of one crate, since it already allows building multiple major versions of the same crate. Could we perhaps re-use that logic here?

retep998 commented 4 years ago

At least in my view of what mutually exclusive features are used for, building a crate twice with different mutually exclusive features seems not particularly useful. Typically the most common sort of mutually exclusive option is things like choosing 1. which backend or implementation to use for some crate or 2. how to link to some library. Both of those situations aren't decisions that their direct reverse dependencies typically care about, but it does matter to the top level binary crate and the user building it. Therefore what I think the solution should be is adding support for features that are chosen by the top level crate and not the direct reverse dependencies and allowing mutually exclusive features as part of them. As well there should probably be some way for crates to learn what features are enabled in their dependencies so they can automatically adapt to the situation.

jonhoo commented 4 years ago

Possibly, though I think both use-cases are common. In the example I linked above for example, the feature enables a different resizing strategy for hash tables. You could imagine that one "part" of an application needs the performance characteristics of that alternative strategy, but another "part" would use prohibitively much memory if that alternative strategy is used. At the moment, users can only opt into the feature on a global basis, if any part of their dependency tree uses the feature, which seems unfortunate.

ctron commented 4 years ago

Why not let features provide some kind of functionality/capability/tag? If a feature does, it must be the only one in that category:

The following is just an example:

[features]
foo = []
bar = []

[features.foo]
provides = ["impl"]

[features.bar]
provides = ["impl"]

Only one provides=impl must be active at a time. Everything else would be a build failure.

This could be done on a global, or local level:

[features]
foo = []
bar = []

[features.foo]
provides = ["impl"]
provides-globally = ["global-impl"]

[features.bar]
provides = ["impl"]
provides-globally = ["global-impl"]

In the latter example, it would be possible for a different crate in the build tree to provide impl", while only one crate in the build tree may provide global-impl.

thomcc commented 4 years ago

That's not a bad proposal, but I think it would need an RFC to hash out all the details and gain consensus before it's really implementable.

ctron commented 4 years ago

That's not a bad proposal, but I think it would need an RFC to hash out all the details and gain consensus before it's really implementable.

Yes, I think this should be though through a bit more. I can prepare an RFC in a fork of the https://github.com/rust-lang/rfcs repository, for previewing, if that helps.

tapeinosyne commented 4 years ago

I can prepare an RFC in a fork of the https://github.com/rust-lang/rfcs repository, for previewing, if that helps.

Once you have a rough draft, you may also want to consider opening a discussion over at the Language Internals forum. Thank you for being willing to work on this.

ctron commented 4 years ago

@thomcc @tapeinosyne I started to work on an RFC. It is a work in progress, and still has some "to be written" markers. Thus I think it is too early to share it with the forum. I would welcome some early feedback though and will continue on this in the next few days.

Link: https://github.com/ctron/rfcs/tree/feature/cargo_feature_caps_1

ctron commented 4 years ago

I made some more progress on the RFC. I think it is a rough enough draft to post it to the forum now.

zesterer commented 4 years ago

@jonhoo I have a very similar use-case.

For spin we have an optional std features that does thread yielding instead of spinning to prevent priority inversion. The problem with this is that crates that that require spinning and cannot correctly perform yielding may have this feature arbitrarily enabled by other instances of the crate in the dependency tree. In this case, it makes the most sense for different instances of the crate with different feature sets to be compiled into the final binary.

Kixunil commented 4 years ago

@jonhoo it seems to me that generics are a more appropriate solution for such scenario although admittedly more tedious to implement. However, generics can offer much better outcome - they can still share some reasonable code such as error types.

If this happens often and there's a specific problem with generics I'd suggest to improve generics instead.

Bathtor commented 4 years ago

If this happens often and there's a specific problem with generics I'd suggest to improve generics instead.

I think the only proposal that would be of sufficient conciseness is module level generics.

I agree that most likely we will not need both mutually exclusive features and module level generics. But at least one of them is necessary to preserve some sanity for library developers when offering implementation options that are very orthogonal to other code, since adding a generic flag to almost every single type in a module is somewhat maddening.

Kixunil commented 4 years ago

I think the only proposal that would be of sufficient conciseness is module level generics.

I agree, note the author of that pre-RFC. ;)

IMO generics should be used for algorithm tuning while mutually-exclusive features would make sense for choosing e.g. library linking. One property of mutually-exclusive features that should be present is being able to set them from the binary crate without intermediate dependencies having to reexport it.

In other words, if A depends on B which depends on C, which has a mutually exclusive features foo and bar, then A must be able to set these features without B having to write any supporting code at all. B should completely ignore that C even has mutually exclusive dependencies. Possibly with the exception of tests in B.

photino commented 3 years ago

Any progress? It is really helpful to address the problem of selecting an async runtime for a lib.

FallenWarrior2k commented 3 years ago

Of course, this would be a very useful thing to have, but for your case, if you already have the code for the different options or are planning to write it, why make it mutually exclusive? What is the argument against making both available conditionally? Unless you're using library-global state, it shouldn't really make a difference, right? Or am I missing something obvious here?

-------- Original Message -------- On Sep 30, 2021, 05:26, Zan Pan wrote:

Any progress? It is really helpful to address the problem of selecting an async runtime for a lib.

photino commented 3 years ago

If we have an ORM lib based on sqlx, which provides both async-std-rt and tokio-rt features. Since only one of the runtimes should be used, they are mutually exclusive. How can we map these features to sqlx's features runtime-async-std-native-tls and runtime-tokio-native-tls?

Kixunil commented 3 years ago

For the time being one can use cfg(my_custom_feature) instead of cfg(feature = "my_feature") to enforce that libraries don't depend on particular feature. The binary can then specify feature via command line, which is not as great as having native support but better than nothing.

FallenWarrior2k commented 3 years ago

@photino Unfortunately, sqlx requires a single runtime to be selected through features. If you want to implement a similar requirement for your crate, you can probably take inspiration from this file.

Though, since sqlx is compiled before your crate, I doubt your checks could ever be hit, as the compiler error in sqlx-rt would stop compilation before your crate even gets to compile to be able to emit its own errors.

alex commented 2 years ago

Another motivating use case here is that in the absence of a way for cargo to know that two features are mutually exclusive, there's no way to express the multiple-dependency-versions pattern (e.g. https://github.com/TrueLayer/reqwest-middleware/blob/main/reqwest-tracing/Cargo.toml) with a package that specifies the links field in Cargo.toml.

epage commented 1 year ago

Discussion Summary

Use cases

Other discussions

Related prior art

Proposals

Conflicting features

https://github.com/rust-lang/cargo/issues/2980#issuecomment-270554758

[dependencies.awesome]
version = "1.3.5"
features = ["!secure-password", "civet"]

Feature values

https://github.com/rust-lang/cargo/issues/2980#issuecomment-587584817

[features]
llvm="9"
flate2="zlib"
winapi_family="desktop_app"

Provides

https://github.com/rust-lang/cargo/issues/2980#issuecomment-700687022

[features]
foo = []
bar = []

[features.foo]
provides = ["impl"]

[features.bar]
provides = ["impl"]

Only one provides=impl must be active at a time. Everything else would be a build failure.

See also https://github.com/ctron/rfcs/blob/feature/cargo_feature_caps_1/text/0000-cargo-mutex-features.md

epage commented 1 year ago

Did a recent brainstorming discussion on this.

A key aspect we narrowed in on is that most of the use cases are really meant to be decided by the final binary and it is not ergonomic to bubble these up to the final binary. I've experienced this myself where cargo release had to add expose a vendored-openssl feature because a dependency used git2 with openssl and users of the binary need to control vendoring (or not). See also #9094.

We had the idea of "global features".

Care abouts

High level

Questions

... I feel like there were more points but I'm not finding it in my notes.

Kixunil commented 1 year ago

For vendoring/system libs choice, I think the use case deserves a first-class feature. This is partially solved by being able to override build scripts. The missing part is that build scripts may need to also generate code (bindgen), so disabling them entirely doesn't help.

Caellian commented 1 year ago

Keep in mind that currently feature values are Vec<InternedString>, so feature values approach would prevent feature dependencies from being used AFAICT and be prone to user error:

my_feature = "dep:bytemuck" # unintended wrong use
my_feature = ["dep:bytemuck"] # what was intended

And this would effectively be the same as passing rustc-cfg from a build script except maybe being more trustworthy?

I think provides covers most current and future use cases assuming feature values would be:

#[derive(Default)] // default is the current `[]`
struct FeatureDetails {
  /// list of crate/feature dependencies
  dependencies: Vec<FeatureValue>,
  /// Name for mutual exclusion
  provides: Option<InternedString>,

  // ... future details ...

  /// List of features and dependencies disabled when this feature is active
  // disable: Vec<FeatureValue>,
}

And constructor could take arguments currently passed to build_feature_map in a form of fn new(deps: impl AsRef<Dependencies>, value: toml::value::Value) and then match for backwards compatibility.

epage commented 1 year ago

I've created a Pre-RFC for mutually-exclusive, global features.