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

Allow requiring "at least one feature" #3347

Closed
Binary file added resources/0000-at-least-one-feature.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
297 changes: 297 additions & 0 deletions text/0000-at-least-one-feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,297 @@
- Feature Name: `at-least-one-feature`
- Start Date: 2022-11-11
- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000)
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)

# Summary
[summary]: #summary

Allow packages to require that dependencies on them must specify at least one feature (the `default` feature counts).
This avoids backwards compatibility problems with `default-features = false`.

# Motivation
[motivation]: #motivation

A major use-case of Cargo features to take previously mandatory functionality and make it optional.
This is usual done in order to make the code more portable than it was previously, while not breaking existing consumers of the library.
Consider this example which shoes both work works and what doesn't:

1. Library has no features.

2. A `foo` feature is added, gating functionality that already existed.
It is on by default.
`no-default-features = true` can be used in which some dependencies only needed for `foo` can be avoided.
Yay!

3. A `bar` feature is added, gating functionality that already existed.

- Suppose it is off by default.
Oh no!
All Existing use-cases break because the functionality that depends on `bar` goes away.

- Suppose it is on by default, or depended-upon by `bar`.
Oh no!
Existing `no-default-features = true` are now broken!
They want a feature set of `{bar}` which would correspond to the old `{}`, but there is no way to arrange it.

In step two, we could "ret-con" the empty feature set with the `default` feature.
But this is a trick that can only be pulled once.
The second time around, we already have a default feature; we are out of luck.

The previous attempts attempted to make new default features, or migrate the requested feature sets.
But that is complex.
There is exactly a simpler solution: simply require that *some* feature always be depended-upon.

To see why this works, it helps to first see that step 2 was *already* broken.
Here's the thing, even though there previously were not any features, that *doesn't* mean there were not any `no-default-features = true` users!
Sure, it wouldn't do anything for crate with new features, but one can still use it.
Then when just `bar` is added, we already have a problem, because the `default` feature will no "catch" all the existing users ---
the mischievous users that were already using `no-default-features = true` will have their code broken!

This brings us to the heart of the problem.
So long as users are depending on "something", we can be careful to make sure those features keep their meaning.
But when users are depending on nothing at all with `no-default-features = true` and an empty feature set, we have nothing to "hook into".
The simple solution is just to rule out that problem entirely!

# Guide-level explanation
[guide-level-explanation]: #guide-level-explanation

Packages with
```toml
[package]
empty-features = false
```
are easier to maintain!
You don't need to worry about the `no-default-features = true, features = []` case anymore.
You can be sure that all consumers must dependent on either `default` or a regular named feature.

Whenever you want to make existing functionality more conditional, simply extend the set of features the code relies on with a new feature, and ensure existing features depend on that feature.

For example:
```rust
fn my_fun_that_allocates() { .. }

#[cfg(all(feature = "foo", feature = "bar"))]
fn my_weird_fun_that_allocates() { .. }
```
becomes:
```rust
#[cfg(feature = "baz")]
fn my_fun_that_allocates() { .. }

#[cfg(all(feature = "foo", feature = "bar", feature = "baz"))]
fn my_weird_fun_that_allocates() { .. }
```

And the corresponding `Cargo.toml`:
```toml
[features]
default = ["foo"]
bar = ["foo"]
```
becomes:
```toml
[features]
default = ["foo", "baz"]
foo = ["baz"]
bar = ["foo"] # no need to add "baz" because "bar" picks it up
```

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

Depending on a
```toml
[package]
empty-features = false
```
crate with an empty feature set is disallowed and invalidates the solution.
The `default` feature counts as a member of that set when `default-features = true`.

The solver shall avoid such solutions (so as not break old versions of libraries without `at-least-one-feature` being "discoverable").
Copy link
Member

Choose a reason for hiding this comment

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

How? Is it supposed to just pick a feature and enable that?

I am also confused by the paragraph on why adding empty-features = false is not a breaking change, which is probably related to my confusion about this sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just meant if no feature is enable than the plan is invalid, and it must throw it out. (If all plans don't enable any feature for a empty-features = false crate, then no plans are valid)

Is it supposed to just pick a feature and enable that?

Hehe I didn't even think of that. I suppose it is quite easy to take any such invalid plan and make it valid in this way, but yeah, ew.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I get it, so the reason this is semver compatible to add is that cargo will refuse to use the newer version because that would violate this condition -- so it is stuck with the old version?

That makes sense, but could be explained in a bit more detail to make it more clear.


## Provisional Theory

We can begin to formalize library compatibility with something like this
[commutative diagram](https://en.wikipedia.org/wiki/Commutative_diagram):
![commutative diagram](../resources/0000-at-least-one-feature.png)
[(generated from here)](https://q.uiver.app/?q=WzAsNCxbMCwwLCJcXG1hdGhjYWx7UF/PiX0oXFxtYXRocm17RmVhdHVyZXN9X3tcXG1hdGhybXtPbGR9fSkiXSxbMSwwLCJcXG1hdGhjYWx7UF/PiX0oXFxtYXRocm17RmVhdHVyZXN9X3tcXG1hdGhybXtOZXd9fSkiXSxbMCwxLCJcXG1hdGhybXtSdXN0fSJdLFsxLDEsIlxcbWF0aHJte1J1c3R9Il0sWzAsMSwiXFxtYXRocm17c2FtZVxcIG5hbWVzfSIsMCx7InN0eWxlIjp7InRhaWwiOnsibmFtZSI6Imhvb2siLCJzaWRlIjoiYm90dG9tIn19fV0sWzAsMiwiXFwjW1xcbWF0aHR0e2NmZ30oLi4uKV1fXFxtYXRocm17T2xkfSIsMl0sWzEsMywiXFwjW1xcbWF0aHR0e2NmZ30oLi4uKV1fXFxtYXRocm17TmV3fSJdLFszLDIsIlxcbWF0aHJte2BgdXBjYXN0XCJcXCBsaWJyYXJ5XFwgaW50ZXJmYWNlc30iXV0=)

- The old and new features form a partial order

- P_ω takes those partial orders to the partial order of their downsets.
(That is, sets of features with the implied features from feature dependencies "filled in", ordered by inclusion.)

- "same names" maps the old downsets to the new downsets, filling in any newly implied features as needed.

- `#[cfg(...)]` is the mapping of feature sets to exposed library interfaces

- "'upcast' library interfaces" forgets whatever unrelated new stuff was added in the new library version

The idea is that going from the old features directly to the old interfaces, or going the "long way" from old features to new features to new interfaces to old interfaces should yield the same result.

For the more part, features can be "interspersed" anywhere the old feature partial order to make the new feature partial order.
However, this is an exception!
The old empty downset becomes the new empty downset, which means nothing can be added below it.
This is the `default-features = false` gotcha!

When we disallow the empty feature set, we are replacing P_ω with the "free join-semilattice" construction.
We are enriching features with the ∨ binary operator but no ⊥ identity element.
There is no empty downset becomes empty downset constraint, and thus we are free to add new features below all the others all we want.

# Compatibility of `empty-features = false` itself

At first, it seems like adding `empty-features = false` to a new version of a crate is a breaking change.
But, I don't think it is.
We have to step back a bit: a breaking change is one that if it *didn't* come with an increasing major version, would result in valid Cargo plans that do not build.
Yes, adding `empty-features = false` to a newer version of a crate will result in depending (not dependent) crate that do not specify a feature for that dependency being unable to upgrade.
But that's it!
Those crates will *not* have their dependencies upgraded incorrectly, resulting in a build failure, and thus there is no problem.

(This is very much analogous to the situation with minimum Rust versions.
Updating the minimum Rust version in a new version of a crate is not a breaking change.
Some users with old versions will be unable to use the new version, but no one will be falsely upgraded resulting in a build failure.

Indeed, that point of being able to express the minimum Rust version is precisely to *avoid* bumping the minimum supported version being a breaking change.
Before, Cargo was unaware of this dependency on the compiler, and so raising the implicit lower bound had to be a breaking change.
Now, Cargo is aware of the dependency and it is no longer a breaking change.

Putting it altogether, in general we can say that bumping major versions is a tool of last result to account for issues Cargo can't directly know about.
If Cargo can be directly made aware of the issue, then there is no "residual breakage" requiring a major version bump.)

# Drawbacks
[drawbacks]: #drawbacks

## Bad names over time

A downside of this proposal is that one has to resort to increasingly bad names.
Take [seanmonstar/warp#968](https://github.com/seanmonstar/warp/issues/968) as an example.
Warp currently ships HTTP2 support by default. This means that in order to be able to ship a version without http2, _all current features must depend on HTTP2_.
This means creating new `websocket-without-http2`, `tls-without-http2`, `compression-without-http2`, `compression-brotli-without-http2`, `compression-gzip-without-http2` features in order for other crates to depend on them without pulling in http2 support.

Note that this is only a problem for new features that existing features or orthogonal too.
In many cases, existing features will naturally (not just for back compat) depend on the new features, and then there is no need or possibility to introduce `...-without-...` siblings to those features.

Choose a reason for hiding this comment

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

But in many cases existing features won't naturally depend on the new features. I don't know for sure which would actually be more prevalent, but my intuition is that even if that is true for some features, it is probably pretty rare that all existing features will depend on the new feature, so you will end up with at least some -without- siblings, whose existence is necessary only for backwards compatibility with packages that only included that one feature.


My sense that while that is a downside, it is better to first walk that run.
Do this and the problem is solved.
We can add feature migrations later once there is more demand.
See the Alternatives section for more details.

# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives
epage marked this conversation as resolved.
Show resolved Hide resolved

## Empty feature set is empty library.

An alternative is not to ban the empty feature set, but ensure it always translates to the empty library.
I.e. to require that *all* items must be dependent upon *some* feature; everything needs a `cfg`.
Comment on lines +183 to +186
Copy link
Member

Choose a reason for hiding this comment

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

This is something a crate could already do, right? So in principle we don't need a new cargo feature to make this possible. The problem is that doing this to an existing crate requires one semver bump to make the no-feature-lib empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, right. I suppose making the cargo init crate have a default feature and #![cfg(feature = "default")] might do the trick.


Returning to our half-worked-out theory, instead of banning a notion of a ⊥ empty feature set that must be preserved from the old library to the new (removing a requirement), we are adding a *new* requirement that the ⊥ feature set must map to the ⊥ Rust interface.
We still have the restriction that new features can be added below, but this restriction is no longer a problem:
there is no point of adding such a new minimal feature because there is nothing left to `cfg`-out!

This solution is more mathematically elegant, but it seems harder to implement.
It is unclear how Cargo could require the Rust code to obey this property without new infra like the portability lint.

Finally, in a sense these variations are actually isomorphic.
If no one can request the empty feature set, no one can observe what library interface that corresponds to.
There is no reason to conclude that it *isn't* the empty library.

## RFC [#3146](https://github.com/rust-lang/rfcs/pull/3146)

This more more expensive solution aimed to solve the "running out of good names" problem this has.

Returning to our commutative diagram, instead of

- removing the ⊥ feature downset (this proposal)

- mandating the ⊥ feature downset correspond to ⊥ the empty library interface (first alternative, extra rules on the vertical mappings in diagrams)

We can instead

- allow customizing the mapping from old feature names to new feature names (customizing top horizontal mapping)

This works and allows giving things the "right" names" every time, but is much work work to implement.

One [point brought up in the comments](https://github.com/rust-lang/rfcs/pull/3146#issuecomment-1273473456) is that punning on the lower version bound to get the feature namespace is a bit tricky.
In particular, it makes manually adjusting versions bounds risky because without consulting package definitions it is unclear how the feature list will be interpretted.
I think that is compelling reason not to do that, but instead version the feature namespace separately.

Overal, I think it might make sense to do some sort of "name indirection" along these lines (but addressing the above problem), but it is better to start with this RFC before getting to that.
My thinking about this is basically "walk before run".
Today, that would be lots of work for an obscure problem, and a solution that downstream crates need to know about because it affects how dependencies are written.
However if we do this first, "opt-out features" are poised to become much more popular now that they are no longer a compatibility hazard.
We can evaluate both that new popularity, and the incidence of the "bad name" problem in *practice*, and use that to justify or not justify a more heavyweight solutions.

## RFC [#3283](https://github.com/rust-lang/rfcs/pull/3283)

This proposal adds a new notion of a "feature base", which aims to essential provide more `default-features = false` options rather than just one forever.
`default-features = false` is ret-conned as the first feature base.

My view is that that RFC is pretty close to being this one in disguise.

Choose a reason for hiding this comment

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

To me this RFC seems like just a special case of RFC 3283. Specifically, the case where the set of base features includes all available features. And because it is just one special case, it has problems that 3283 didn't have, notably the problems described in "bad names over time".

Personally, I'd rather see a more general solution where you specify multiple sets of features where at least one must be included, so you could for example say you need at least one of "http1" and "http2" and at least one of a set of features for TLS support (rustls, native-tls, etc.).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I don't think this is special case to that. Consider if default and default-features = <bool> and all that didn't exist. This would still be solution.

It is probably best to imagine 3 parts:

  1. This to fix the original problem.
  2. Some deprecation cycle with editions to get rid of default-features = <bool>
  3. Some namespacing / indirection so we can use good names forever.

Each builds on the last, but the former steps stand alone.

Remember Rust itself has the same name issue! If you end up making a new version function which deserves the name more than the old version and don't want to make a breaking change...to bad. If it's not a show-stopper there, why should it be here?

so you could for example say you need at least one

Disjunctive ("or", not "and") dependencies are completely unrelated to what is being discussed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we try to solve the problem and deal with various back compact issues at the same (not library, but Cargo itself back compat), I think we'll just melt our brains.

First why should agree on an ideal design*, second we should try to figure out a migration plan to get there, third we should adjust the destination of it's not possible.

Choose a reason for hiding this comment

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

Hmm I don't think this is special case to that. Consider if default and default-features = and all that didn't exist. This would still be solution.

Unless I am missing some subtle distinction, having empty-features = false with this RFC is effectively equivalent to RFC 3283, where every feature in the crate is included in the feature-bases. Sure, the syntax is a little different because in 3283 you would need to put one of the features in the base attribute rather than features attribute, but that is a very surface-level detail. In both cases, you have to specify at least one feature. However, RFC 3283 is strictly more flexible, because rather than always requiring you to require one of any feature in the crate. You can limit the requirement to a subset of the features. I don't see any argument in this RFC on why having a boolean to require "at least one feature" is a superior option to having a set of features, and requiring at least feature from that set (or possibly multiple such sets).

Remember Rust itself has the same name issue! If you end up making a new version function which deserves the name more than the old version and don't want to make a breaking change...to [sic] bad.

It's not quite the same. It isn't just that you can't use the name of an existing feature for a new feature. It is that if you add a new feature that was previously part of the "base", then it can potentially double the number of features that you need. If you do this multiple times, it can create quite mess, and you'll end with things like foo-without-bar-without-baz-without-soup.

A "feature base" is just a feature.
The fact that there is always one "feature base" is just another way of accomplishing the "at least one feature"
restriction.

The main difference is that `default-features = false` is ret-conned as a feature to allow for existing users.
That can be thought of from the perspective of this RFC as a "one-off" feature migration to get reverse dependencies predating the use of `empty-features = false` on to a feature so they retroactively abide by the "at least one feature" rule.

## "Negative features"

I take a pretty dim view of this.
epage marked this conversation as resolved.
Show resolved Hide resolved
It is easy to say that one wants negative features; it is harder to actually give a proper semantics for them.
As far as I know, additive features are the only reasonable core semantics, so we have no choice but to "pre-process" away negative features before they ruin our core model.

Here is an example of such a pre-processing step.
Recall from our preliminary theory section that feature sets are feature *downsets*.
Whatever the user writes, we must complete it with the missing features implied by what they wrote.
That means we have this process:

> Syntactic feature set
>
> --- fill in missing features --->
>
> Semantic feature downset

We can add a middle step to account for "negative features"

> Syntactic feature "dual set"
>
> --- negate negative features: add if they were not included, remove if they were --->
>
> Strictly positive feature set
>
> --- fill in missing features --->
>
> Semantic feature downset

But not the ramifications of this: the final "fill in missing features" step may well "add back" one of the features that was removed!

I think this will be confusing to users, to say the least.
Moreover it is crucial to understand the underlying additive feature model to become proficient with Cargo, and the more we "disguise" it with superficial negative features, the harder it will be for users to understand that model.
The syntax *impedes* learning.

I think this is an awkward middle ground between this (dirt easy) and full feature migrations.
Only full feature migrations provide the "ideal" naming of feature so users new to a library can be blissfully ignorant of whatever features previous versions of the library supported.

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

Nothing direct to my knowledge.
epage marked this conversation as resolved.
Show resolved Hide resolved
In Haskell we often implement a lot of "non-empty" data structures because their sneaky semantics are useful.
So in some sense the Math is newly-applied here, but not newly applied to programming in general.

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

It would be nice to completely work out the theory.

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

The `default-features = false` syntax is clunky.
A new edition could say that an explicit feature list always means `default-features = []`, but that `default` can be used in feature lists.
With this change, "must depend on one feature, including possibly the default feature" becomes easier to explain:

- `[]` disallowed
- `["default"]` allowed
- `["foo"]` allowed