Skip to content
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

RFC 3283: Backward compatible default features #3283

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 144 additions & 0 deletions text/3283-backward-compatible-default-features.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
- Feature Name: `backward_compatible_default_features`
- Start Date: 2022-06-23
- RFC PR: [rust-lang/rfcs#3283](https://github.com/rust-lang/rfcs/pull/3283)
- Rust Issue: None

# Summary
[summary]: #summary

Add feature bases as an alternative to `default-features = false` that allows
adding new default-enabled features for existing functionality.

# Motivation
[motivation]: #motivation

Currently, crates cannot add new default-enabled features for existing
functionality. This is a bad thing, because it means that crates cannot allow
users to opt out of more features, because adding new default-enabled features
for these would break existing users with `default-features = false`.

To fix this, I propose we add the notion of a feature `base`. Users of crates
have to select at least one of its base features. This means that in order to
make another part of a crate optional, people can simply add a new optional
feature and add it to all existing feature bases. Then they can create another
feature base that excludes this new optional feature. The syntax of
`default-features = false` will get replaced by `base = "<feature base>"`, with
the default being `base = "default"`.

In order to introduce this in a way that works with the existing
`default-features = false` syntax, `default-features = false` will imply a
feature base, `no-default-features`.
Copy link
Contributor

@teor2345 teor2345 Nov 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the name was changed elsewhere, but not here?

Suggested change
feature base, `no-default-features`.
feature base, `default-features-false`.


For example, the warp crate enables HTTP2 by default and would like to make
that optional. This was done in a PR, but the backward incompatibility was
noticed (https://github.com/seanmonstar/warp/issues/913) and the PR was
reverted. With this feature proposal, they could instead add a base called
`no-default-features = ["http2"]`, so old crates depending on warp still get
HTTP2 support. People can then opt out of the HTTP2 dependency by specifying a
feature base like `base = "minimal_v1"`.

# Guide-level explanation
[guide-level-explanation]: #guide-level-explanation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will the cargo user that currently uses --features, --all-features, and most importantly --no-default-features interact with this new feature?

I'm assuming cargo will need a --feature-base flag.


When wanting to give users of your crate the possibility to opt out of parts of
the functionality, you can turn to feature bases: Say you have a dependency on
`large_crate`:

```toml
[dependencies]
large_crate = "3.4"
```

You want to make that dependency optional, but since this would take away parts
of the functionality of your crate as well, existing users must continue to use
your crate with that dependency. The solution is to create a new feature base:

```toml
[package]
feature-bases = ["minimal_v1"] # "default" is automatically a feature base

[dependencies]
large_crate = { version = "3.4", optional = true }

[features]
default = ["large_crate"]
minimal_v1 = []
```

Users of your crate can now opt out of the large crate by specifying the
following in their `Cargo.toml`:

```toml
[dependencies]
your_crate = { version = "5.7", base = "minimal_v1" }
```
Comment on lines +72 to +74
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the impact on cargo add?

cargo add is about to be released on stable and with new manifest features, we should consider if they should be exposed in cargo add and how.


## Migration from default-features
[migration-from-default-features]: #migration-from-default-features

The `default-features = false` feature allowed to do the same migration, but
only once per major version of a crate. To be backward-compatible with that
approach, `default-features = false` implies the feature base called
`default-features-false`.

# Reference-level explanation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the impact to the Index?

For example, see the explanation of index changes for weak and namespaced features

My main concern is that if we have a disruptive index change that we provide a better experience than we did with weak and namespaced features, see rust-lang/cargo#10623

[reference-level-explanation]: #reference-level-explanation

A new key `package.feature-bases` is introduced. This key specifies a list of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A draw back to this is the bases are free form and we can't do tooling to migrate users. Maybe a more concrete syntax with at least a version field would help.

If we also restricted the names to go with the versions, that might help with

  • Developers providing a consistent set of bases (minimal and default)
  • Users discovering the base they need since crates won't have different conventions
  • Discourages using bases as feature groups (if we'd want to do that)
    • If people want versioned feature groups then that might suggest we need general feature versioning and not a new "base" concept

features that, augmented with `default` and `default-features-false`, are
Copy link
Contributor

@epage epage Jun 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies new semantics to a feature name (default-features-false) that was not previously reserved. What, if any, backwards compatibility concerns do we have about this?

eligible as to be specified in the `base` key of a dependency.

A new key `base` is introduced for dependencies, its value is a string. It is
well-formed if it is in the above-mentioned set of possible bases.

The keys `default-features` and `base` on dependencies are mutually exclusive;
`default-features = false` implies `base = "default-features-false"`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A caution with this is there will no longer be a guaranteed way of getting the minimal set of features. Users will have to know what all is happening within the bases to get a minimal set.

Copy link

@ssokolow ssokolow Jul 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...which could cause Public Relations issues, given that you already see people complaining about build times and insufficient tooling for help identifying what to turn off because you don't need it in order to reduce build times.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insufficient tooling for help identifying what to turn off because you don't need it in order to reduce build times

One small step for improving this is cargo add raises the visibility of what features are enabled by default.


# Drawbacks
[drawbacks]: #drawbacks

This specifies one way to solve this problem. It is kind of clear that this
problem should be solved, the only question is whether this is the right or
best solution to the problem.

# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

There is another proposal to fix this problem in [RFC 3146: Cargo feature
migrations](https://github.com/rust-lang/rfcs/pull/3146).

One upside of this over RFC 3146 is that

Not doing this lets problems such as the one in
https://github.com/rust-lang/rfcs/pull/3140#issuecomment-862020840 continue
happening, not allowing crate authors (or even the standard library) to create
new default-enabled features for existing functionality:

> The general concept of `infallible_allocation` seems sensible. However, I
> want to point out a forward-compatibility concern with the proposed way of
> handling this in cargo, which has also applied to previous proposals that
> suggest the same thing:

> If crates start declaring dependencies on `std` with `default-features =
> false`, they'll break if we introduce a new enabled-by-default feature in the
> future. We'd likely find that we could never add a new default feature.

> I think, instead, we'd need to have a syntax to say "default minus
> `infallible_allocations`", which would be forwards-compatible.

# Prior art
[prior-art]: #prior-art

Slint has done a hack that is similar to this proposal:
https://slint-ui.com/blog/rust-adding-default-cargo-feature.html.

# Unresolved questions
[unresolved-questions]: #unresolved-questions

- Should feature bases instead be treated like normal features entirely, with
the only difference being that cargo enforces to use at least one of them?

# Future possibilities
[future-possibilities]: #future-possibilities

No future possibilities were thought of.