-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cargo target features #3374
Cargo target features #3374
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,212 @@ | ||
- Feature Name: `cargo_target_features` | ||
- Start Date: 2023-01-20 | ||
- RFC PR: [rust-lang/rfcs#3374](https://github.com/rust-lang/rfcs/pull/3374) | ||
- Tracking Issue: [rust-lang/cargo#0000](https://github.com/rust-lang/cargo/issues/0000) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
This adds a new `enable-features` field to [Cargo Targets](https://doc.rust-lang.org/cargo/reference/cargo-targets.html#configuring-a-target) which forces a [feature](https://doc.rust-lang.org/cargo/reference/features.html) to be enabled if the target is being built. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
There are several situations where a non-library target must have a particular feature enabled for it to work correctly. | ||
Although it is possible to manually enable these features on the command-line, this can make it awkward to use. | ||
This RFC adds a mechanism to make it easier to work with these targets. | ||
|
||
Some example use cases are: | ||
|
||
1. Binaries that need additional dependencies like command-line processing libraries, or logging functionality. | ||
2. Examples illustrating how to use a library with a specific feature enabled. | ||
3. Benchmarks which require a separate benchmarking library, but you don't want to pay the cost of building that library when not running benchmarks. | ||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
The `enable-features` field can be added to a target in `Cargo.toml` to specify features or dependencies that will be automatically enabled whenever the target is being built. | ||
For example: | ||
|
||
```toml | ||
[package] | ||
name = "myproject" | ||
version = "0.1.0" | ||
edition = "2021" | ||
|
||
[[example]] | ||
name = "fetch-http" | ||
enable-features = ["networking"] | ||
|
||
[[bin]] | ||
name = "myproject" | ||
enable-features = ["dep:clap"] | ||
|
||
[dependencies] | ||
clap = { version = "4.0.26", optional = true } | ||
|
||
[features] | ||
networking = [] | ||
``` | ||
|
||
When using `myproject` as a library dependency, by default the `networking` feature and `clap` will not be enabled. | ||
When building the binary as in `cargo install myproject` or `cargo build`, the `clap` dependency will automatically be enabled allowing the binary to be built correctly. | ||
Similarly, `cargo build --example fetch-http` will enable the `networking` feature so that the example can be built. | ||
Comment on lines
+51
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I feel like an important part of this RFC is non-obvious unless a person is very familiar with these terms and fully thinks through the implications of those terms. Going into this RFC, my care about was
However, this RFC also introduces
This is in contrast to My primary concern with this is it will make it impossible to
The assumption is this covers enough feature combinations to feel confident that it will build. There are combinations of features that might break but the probability is low enough to not justify extra CI time. With From watching cargo's issue tracker and other forums, I feel like feature unification is one of the biggest hurdles to features in cargo today and I feel that this design compounds that problem to being unmanageable. Under the alternatives is listed:
As I said, this is what I expected going into this RFC. For The one use case I can think of that wouldn't be served by this is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What's stopping us from default enabling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had considered including that but vaguely remember a conversation with concerns for a similar idea ("just make One problem is there are two different workflows
This also makes That isn't to say I'm against it but that we'd need to do weigh it carefully and maybe find ways to help with these problems. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Those are very valid and strong concerns. Part of the drawbacks mentions
IIUC, this is suggesting to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This doesn't seem prohibitive. The tests could have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I have not found this to be true up till now in feature-heavy crates. I've taken two approaches in the past: feature-gate tests appropriately and note in the readme that you need to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps my comment wasn't clear, but for me
This isn't intended for just feature-heavy situations. The primary use case and motivation for this is target-specific dependencies, where you might not have any features. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And FWIW, it may be the solution is to instead lean more heavily on workspaces, and making them easier to use. That is what I heavily lean towards as an alternative to this, particularly since the amount of complexity ended up being far greater than I was expecting. I'm not sure what might be the solution there, but it would at least be helpful for all workspace users if workspaces were easier to use. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think this is one of the problems with the RFC. It is taking a very specific use case / workflow (bin/lib crates existing in the same package) and making a general feature for it. For example, test, and bench targets will more likely run into this problem due to being in feature-heavy crates (like clap) for which this solution is a major step back yet is being advertised as solving that problem.
imo potential steps forward are:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the cargo-install use case, another solution is "bin dependencies" or dependencies that activate for bins (#2887). |
||
|
||
This field can be specified for any Cargo Target except the `[lib]` target. | ||
|
||
> **Note**: Because features and dependencies are package-wide, using `enable-features` does not narrow the scope of the features or dependencies to the specific target. | ||
> The features and dependencies will be enabled for all targets that have been asked to be built. | ||
|
||
# Implementation Details | ||
|
||
## Feature resolver and selection | ||
|
||
Before doing dependency and feature resolution, Cargo will need to determine which features are being force-enabled by the targets selected on the command-line. | ||
These additional features will be combined with any features selected on the command-line before calling the resolver. | ||
|
||
Unfortunately this selection of targets is quite complex. | ||
This may require some potentially invasive refactoring, as Cargo currently chooses the targets to select after resolution is done (including a separate phase for implicit binaries used for integration tests). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quoting your question from #3020 (comment):
How does the feature resolver solve this problem? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for bringing that up! I had forgotten about it, and have added it to the RFC. It should likely iterate until no new requirements can be satisfied. |
||
|
||
## Hidden `dep:` dependencies | ||
|
||
`enable-features` should allow the use of `dep:` to enable an optional dependency. | ||
The use of `dep:` should behave the same as-if it was specified in the `[features]` table. | ||
That is, it should suppress the creation of the implicit feature of the same name. | ||
This may require some challenging changes to the way the features table is handled, | ||
as the corresponding code has no knowledge about the manifest. | ||
|
||
For the purpose of generating the lock file, the dependency resolver will need to assume that all optional dependencies of the workspace packages are enabled. | ||
Currently this is implicitly done by assuming `--all-features` is passed. | ||
However, if a `enable-features` field specifies a `dep:` dependency, and nothing else enables that dependency, then the resolver will not be able to see that it is enabled. | ||
This may require some changes to differentiate between the use of explicit and implicit `--all-features`. | ||
|
||
## Artifact dependencies | ||
|
||
When depending on a binary [artifact dependency](https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#artifact-dependencies), Cargo should enable any `enable-features` features of that binary. | ||
This may require plumbing that information into the index so that the dependency and feature resolvers can determine that field is being set. | ||
This RFC does not propose a specific plan here, but that it should eventually be supported. | ||
|
||
## Relationship with `required-features` | ||
|
||
[`required-features`](https://doc.rust-lang.org/cargo/reference/cargo-targets.html#the-required-features-field) is a very similar field to `enable-features`, but works in a different way. | ||
It essentially says that the target will not be built if the feature is not already enabled. | ||
|
||
There are legitimate use cases where `required-features` is more appropriate than `enable-features`. | ||
For example, there may be examples or tests that only work with a specific feature enabled. | ||
However, the project may not want to make those examples or tests automatically build when running `cargo test`. | ||
The feature may be expensive to build, or may require specific components on the system to be installed. | ||
In these situations, `required-features` may be the appropriate way to conditionally include a target. | ||
|
||
Documentation will need to emphasize the difference between these seemingly similar options. | ||
|
||
### `required-features` unlock | ||
|
||
There may be situations where an `enable-features` field may enable features that were listed in `required-features` of another target. | ||
The resolver should iterate, enabling features unlocked by newly added targets now satisfying a `required-features` field. | ||
|
||
For example: | ||
|
||
```toml | ||
[[bin]] | ||
name = "foo" | ||
enable-features = ["a"] | ||
|
||
[[bin]] | ||
name = "bar" | ||
required-features = ["a"] | ||
enable-features = ["b"] | ||
|
||
[[bin]] | ||
name = "baz" | ||
required-features = ["b"] | ||
``` | ||
|
||
If this is built with no explicit feature flags, then `foo` will enable feature `a`. | ||
Enabling feature `a` now satisfies `bar`'s required features, and thus it is added to the set of enabled targets, which in turn enables feature `b`. | ||
Now that feature `b` is enabled, `baz`'s required features are also now satisfied, so it is also enabled. | ||
|
||
Thus, all three targets will be built, and features `a` and `b` are enabled. | ||
|
||
This will likely require iterating over the targets until no additional targets can be added. | ||
|
||
## Other cargo command behavior | ||
|
||
[`cargo metadata`](https://doc.rust-lang.org/cargo/commands/cargo-metadata.html) and [`cargo tree`](https://doc.rust-lang.org/cargo/commands/cargo-tree.html) will behave as-if they are ignoring the `enable-features` fields. | ||
These commands do not have a concept of targets being built, nor do they have any options for selecting them. | ||
|
||
When using those commands with `--all-features`, any hidden `dep:` dependencies that are only enabled via `enable-features` will be included. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
* This adds additional complexity to `Cargo.toml` potentially making it more difficult to understand. | ||
* Users may be easily confused between the difference of `enable-features` and `required-features`. | ||
Hopefully clear and explicit documentation may help avoid some of that confusion. | ||
The error messages with `required-features` may also be extended to mention `enable-features` as an alternative (as well as other situations like `cargo install` failing to find a binary). | ||
* It may not be clear that features are unified across all targets. | ||
This may particularly come into play where it may not be clear that an optional dependency suddenly becomes *available* to all the other targets when the `enable-features` causes it to be included. | ||
A similar situation arises with dev-dependencies, where a user may get confused when referencing a dev-dependency outside of a `#[cfg(test)]` block or module, which causes an error. | ||
* This may not have the same clarity as explicit dependencies as proposed in [RFC 2887](https://github.com/rust-lang/rfcs/pull/2887). | ||
* There may be increased confusion and complexity regarding the interaction of various cargo commands and this feature (such as `cargo tree` mentioned above which has no concept of cargo target selection). | ||
* There may be confusion about the interaction with `--no-default-features`. | ||
`--no-default-features` will continue to only affect the `default` feature. | ||
Features enabled by `enable-features` will be enabled even with `--no-default-features`. | ||
This may be confusing and should be mentioned in the documentation. | ||
* There are some alternatives to avoiding a target from being built, such as not including it in the CLI options, using `required-features` instead, or disabling it with fields such as `test=false`. | ||
However, not all of these options may be satisfying. | ||
* This may add significant complexity to Cargo's implementation. | ||
|
||
# Rationale and alternatives | ||
[rationale-and-alternatives]: #rationale-and-alternatives | ||
|
||
## Change the `required-features` behavior | ||
|
||
* `required-features` could be changed to behave the same as `enable-features` described in this RFC (possibly over an Edition). | ||
However, as outlined in the [Relationship with `required-features`](#relationship-with-required-features) section, there are some use cases where the present behavior of `required-features` is desirable. | ||
This could also lead to a breaking change for some projects if it started building targets that were previously not included. | ||
* Instead of adding a separate field that lists features, a `force-enable-features = true` field could be added to change the behavior of `required-features` to have the behavior explained in this RFC. | ||
That might be less confusing, but would prevent the ability to have both behaviors at the same time. | ||
Comment on lines
+167
to
+168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you speak to when it is desirable to have both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would expect it to be rare. A possible example would be: [[example]]
name = "db"
enable-features = ["dep:clap"]
required-features = ["sql"] Here the example would only be built if I think a con for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I quite like the distinction between "enable this target if X features are enabled" in addition to "if this target is enabled, build it with Y additional features". If I'm understanding correctly, In the embedded world, this would be very useful to enable building variant targets of e.g. firmware based on chip class/manufacturer. [[bin]]
name = "stm32f4"
enable-features = ["stm32f4"]
required-features = ["stm32"]
[[bin]]
name = "stm32f7"
enable-features = ["stm32f7"]
required-features = ["stm32"] Which would build both the |
||
It would also require entering two lines (instead of one) in `Cargo.toml` to get the behavior that most users will likely want. | ||
* Only the situation where `required-features` generates an error could be changed to implicitly enable the missing features. | ||
This would likely make `required-features` less annoying to work with, but doesn't help for use cases like running `cargo test` where you have specific tests or examples that you want to be automatically included (where `required-features` simply makes them silently excluded). | ||
* A new CLI argument could be added to change the behavior of `required-features` to behave the same as `enable-features`, avoiding the need to add `enable-features`. | ||
This is viable, but the goal of this RFC is to make it as easy as possible to work with the cargo targets without requiring additional CLI arguments. | ||
|
||
## Alternate workflows | ||
|
||
* Instead of using `enable-features`, developers can be diligent in passing the appropriate `--features` options on the command-line when building their projects, possibly using `required-features` to ensure they only get built in the correct scenarios. | ||
The intent of this RFC is to make that process easier and more seamless. | ||
Comment on lines
+177
to
+178
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the UX should be called out for this alternative. atm commands like $ cargo run --example git-derive
error: target `git-derive` in package `clap` requires the features: `derive`
Consider enabling them by passing, e.g., `--features="derive"` The gap is with $ cargo install pulldown-cmark --no-default-features
Updating crates.io index
Installing pulldown-cmark v0.9.2
Compiling version_check v0.9.4
Compiling memchr v2.5.0
Compiling pulldown-cmark v0.9.2
Compiling bitflags v1.3.2
Compiling unicase v2.6.0
Finished release [optimized] target(s) in 1.86s
warning: none of the package's binaries are available for install using the selected features I've created rust-lang/cargo#11617 for us to track this |
||
The current UX for using `required-features` can be confusing when the required features are not specified. | ||
For example, `cargo install` fails to inform why it fails (see [#11617](https://github.com/rust-lang/cargo/issues/11617)). | ||
* Users can set up [aliases](https://doc.rust-lang.org/cargo/reference/config.html#alias) which pass in the feature flags they want to enable. | ||
This can help with a development workflow, but requires more documentation and education, and doesn't help with some commands like a remote `cargo install`. | ||
* Developers can organize their project in a [Cargo Workspace](https://doc.rust-lang.org/cargo/reference/workspaces.html) instead of using multiple targets within a single package. | ||
This allows customizing dependencies and other settings within each package. | ||
Workspaces can add some more overhead for managing multiple packages, but offer a lot more flexibility. | ||
Instead of implementing this RFC, we could put more work into making workspaces easier to use, which could benefit a larger audience. | ||
However, it is not clear if it is feasible to make workspaces work as effortlessly compared to targets. | ||
Also, there is likely more work involved to get workspaces on the same level. | ||
Comment on lines
+183
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I am not mistaken, this works for binaries, but not for examples. This gap is indeed an issue of its own. It leads projects with complex examples (especially ones where the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you say more about what you feel like you lose when the distinction of an "example" is lost? I understand that instead of running There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is definitely a perception element. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also - as far as I'm aware - installing a crate with multiple |
||
|
||
## Alternate designs | ||
|
||
* [RFC 2887](https://github.com/rust-lang/rfcs/pull/2887) proposes being able to add dependency tables directly in a target definition. | ||
It is intended that this RFC using `enable-features` will hopefully be easier to implement, make it easier to reuse a dependency declaration across multiple targets, and allow working with features that are not related to dependencies. | ||
* [RFC 3020](https://github.com/rust-lang/rfcs/pull/3020) proposes an enhancement similar to this RFC. | ||
|
||
## Other alternative considerations | ||
|
||
* [cargo#1982](https://github.com/rust-lang/cargo/issues/1982) is the primary issue requesting the ability to set per-target dependencies, and contains some discussion of the desired use cases. | ||
* Other names may be considered for the field `enable-features`, such as `forced-features`, `force-enable-features`, etc. | ||
* A flag could be added to the target definition to indicate that default features should be disabled when the target is being built. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not exactly an alternative but an extension on top of the RFC. I think this should be actually be inside this RFC itself, but if not I will create a quick follow up RFC since IMHO, there are good use cases for this. The flag name could be: disable-default-features = true |
||
This would allow having different default features when running a command such as `cargo install` versus using the package's library as a dependency. | ||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
> NOTE: These could use vetting by people more familiar with these tools. | ||
> More examples are welcome. | ||
|
||
* Swift has the ability to specify dependencies within a target definition (see [SE-0226](https://github.com/apple/swift-evolution/blob/main/proposals/0226-package-manager-target-based-dep-resolution.md) and [Target Dependency](https://docs.swift.org/package-manager/PackageDescription/PackageDescription.html#target-dependency)). | ||
These can also specify explicit dependencies on artifacts of other targets within the package. | ||
* Go can specify dependencies directly in the source of a module. | ||
* Many other tools do not have a similar concept as Cargo targets, or if they have something similar, they do not have a way to specify dependencies or other settings per target. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #3020, I have the following line:
And there were some concerns about that as described in #3020 (comment)
The original motivation for having that line is to allow the library and binary to have separate dependencies. There are use cases where the user use the same
cool
name for a library and wants to have a cli tool with the samecool
name but with different dependencies. (ex: cli tool only does project generation to use the library). In this scenario, when the end user runscargo install cool
, the unneeded dependencies present indefault
are also built.To solve this issue, I would propose what I had been thinking last on #3020.
If I were to run
cargo install cool --bin foo
, default features will not be activated:If I were to run
cargo install cool
(installing both binaries), the default features will be activated because ofbar
target:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion! I have added it as a possible extension to consider.