From 8a883f962b31f1b8959eac0e77404915bfce70ee Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sat, 9 Sep 2023 19:09:30 -0400 Subject: [PATCH 1/6] Add initial feature-visibility RFC split from feature-metadata --- text/0000-feature-visibility.md | 160 ++++++++++++++++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 text/0000-feature-visibility.md diff --git a/text/0000-feature-visibility.md b/text/0000-feature-visibility.md new file mode 100644 index 00000000000..a2392da0d2a --- /dev/null +++ b/text/0000-feature-visibility.md @@ -0,0 +1,160 @@ +- Feature Name: feature-metadata +- Start Date: 2023-09-08 +- RFC PR: [rust-lang/rfcs#3416](https://github.com/rust-lang/rfcs/pull/3416) +- Rust Issue: + [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) + +# Summary + +[summary]: #summary + +This RFC describes a new key under `features` in `Cargo.toml` to indicate that a +feature is private. + +Please see the parent meta RFC for background information: [`feature-metadata`]. + +# Motivation + +[motivation]: #motivation + +WIP + +No way to hide unstable API such as testing-only features or internally enabled +features. + +# Guide-level explanation + +[guide-level-explanation]: #guide-level-explanation + +There will be a new flag allowed within `[features]`: `public`. This is boolean +flag defaulting to `true` that indicates whether or not downstream crates should +be allowed to use this feature. + +```toml +[features] +foo = { enables = [], public = false} +``` + +Attempting to use a private feature on a downstream crate will result in +messages like the following: + +``` +error: feature `baz` on crate `mycrate` is private and cannot be used by + downstream crates +``` + +# Reference-level explanation + +[reference-level-explanation]: #reference-level-explanation + +`public` is a boolean value that defaults to `true`. It can be thought of as +`pub` in Rust source files, with the exception of being true by default. If set +to `false`, Cargo should forbid its use with an error message on any downstream +crates. + +The default `true` is not consistent with [`public_private_dependencies`] or +Rust's `pub`, but is a reasonable default to be consistent with the current +behavior. This means that either `feature = []` or +`feature = { "enables" = [] }` will result in the same configuration. + +The name `public` was chosen in favor of `pub` to be consistent with the +[`public_private_dependencies`] RFC, and to match the existing style of using +non-truncated words as keys. + +In general, marking a feature `public = false` should make tooling treat the +feature as non-public API. This is described as the following: + +- The feature is always usable within the same crate: + - Enablement by other features, e.g. + `foo = { enables = [some-private-feature] }`, is allowed + - Using the feature in integration tests is allowed + - Using the feature in benchmarks is allowed +- The feature should not be accepted by `cargo add --features` +- The feature should not be reported from `cargo add`'s feature output report +- Once `rustdoc` is able to consume feature metadata, `rustdoc` should not + document these features unless `--document-private-items` is specified +- A future tool like `cargo info` shouldn't display information about these + features +- Explicitly specifying the feature via `--features somecrate/private-feature` + will allow enabling a private feature that would otherwise be forbidden + +Attempting to use a private feature in any of the forbidden cases should result +in an error. Exact details of how features work will likely be refined during +implementation and experimentation. + +Two sample use cases for `public = false` include: + +- `docs.rs` having a way to know which features should be hidden +- Features that are included in feature chains (feature `a` enables feature `b`) + but not meant for public consumption could be marked not public + +This feature requires adjustments to the index for full support. This RFC +proposes that it would be acceptable for the first implementation to simply +strip private features from the manifest; this meanss that there will be no way +to `cfg` based on these features. + +Full support does not need to happen immediately, since it will require this +information be present in the index. [Index changes] describes how this can take +place. + +# Drawbacks + +[drawbacks]: #drawbacks + +- Added complexity to Cargo. Parsing is trivial, but exact implementation + details do add test surface area +- Added Cargo arguments if escape hatches for `public` are created +- `public` uses may not be common enough to be worth including + +# Rationale and alternatives + +[rationale-and-alternatives]: #rationale-and-alternatives + +TODO + +# Prior art + +[prior-art]: #prior-art + +- `docs.rs` displays a limited feature table. Features that start with `_` are + hidden from this table. +- Ivy has a [visibility attribute] for its configuration (mentioned in + [cargo #10882]) +- Discussion on stable/unstable/nightly-only features + + +# Unresolved questions + +[unresolved-questions]: #unresolved-questions + +- Are the semantics of `public` proposed in this RFC suitable? Should private + features be usable in examples or integration tests without a `--features` + argument? +- Should there be a simple `--allow-private-features` flag that allows using all + features, such as for crater runs? This can be decided during implementation. +- Does `public` need to be in the index? + +# Future possibilities + +[future-possibilities]: #future-possibilities + +- A `stable` field can be set false to indicate API-unstable or nightly-only + features (somethign such as `stable = 3.2` could be used to indicate when a + feature was stabilized). See also: + +- A `rust-version` field that could indicate e.g. `rust-version = "nightly"` or + `rust-version = "1.65"` to specify a MSRV for that feature. See: + +- The `public` option could be used to allow optional dev dependencies. See: + + +[cargo #12335]: https://github.com/rust-lang/cargo/issues/12235 +[cargo #10882]: https://github.com/rust-lang/cargo/issues/10882 +[`cargo-info`]: https://github.com/rust-lang/cargo/issues/948 +[`deprecated`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-deprecated-attribute +[`deprecated-suggestions`]: https://github.com/rust-lang/rust/issues/94785 +[discussion on since]: https://github.com/rust-lang/rfcs/pull/3416#discussion_r1172895497 +[`public_private_dependencies`]: https://rust-lang.github.io/rfcs/1977-public-private-dependencies.html +[`rustdoc-cargo-configuration`]: https://github.com/rust-lang/rfcs/pull/3421 +[`tokio`]: https://docs.rs/crate/tokio/latest/features +[visibility attribute]: https://ant.apache.org/ivy/history/latest-milestone/ivyfile/conf.html From 04ee34d0d10cd05cb9344c921059e70d75035276 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Sat, 9 Sep 2023 19:13:15 -0400 Subject: [PATCH 2/6] Update RFC number --- text/{0000-feature-visibility.md => 3487-feature-visibility.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename text/{0000-feature-visibility.md => 3487-feature-visibility.md} (98%) diff --git a/text/0000-feature-visibility.md b/text/3487-feature-visibility.md similarity index 98% rename from text/0000-feature-visibility.md rename to text/3487-feature-visibility.md index a2392da0d2a..7869730c9e5 100644 --- a/text/0000-feature-visibility.md +++ b/text/3487-feature-visibility.md @@ -1,6 +1,6 @@ - Feature Name: feature-metadata - Start Date: 2023-09-08 -- RFC PR: [rust-lang/rfcs#3416](https://github.com/rust-lang/rfcs/pull/3416) +- RFC PR: [rust-lang/rfcs#3487](https://github.com/rust-lang/rfcs/pull/3487) - Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) From 7247b2ce86c9302b50e9afe469b372264ac15219 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 9 Sep 2023 21:00:29 -0500 Subject: [PATCH 3/6] Misc cleanup These are meant to apply simple feedback that wouldn't change the author's intent. --- text/3487-feature-visibility.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/text/3487-feature-visibility.md b/text/3487-feature-visibility.md index 7869730c9e5..c0887c58ba7 100644 --- a/text/3487-feature-visibility.md +++ b/text/3487-feature-visibility.md @@ -65,18 +65,19 @@ In general, marking a feature `public = false` should make tooling treat the feature as non-public API. This is described as the following: - The feature is always usable within the same crate: - - Enablement by other features, e.g. + - Enabled by other features, e.g. `foo = { enables = [some-private-feature] }`, is allowed - - Using the feature in integration tests is allowed - - Using the feature in benchmarks is allowed + - Referenced in `[[bench]]` and `[[test]]` target `required-features` + - Using the feature on the command-line is allowed +- Users may explicitly specifying the private features for their dependencies + on the command-line (e.g. `--features somecrate/private-feature`) which would + otherwise be forbidden - The feature should not be accepted by `cargo add --features` - The feature should not be reported from `cargo add`'s feature output report + - A future tool like `cargo info` shouldn't display information about these + features - Once `rustdoc` is able to consume feature metadata, `rustdoc` should not document these features unless `--document-private-items` is specified -- A future tool like `cargo info` shouldn't display information about these - features -- Explicitly specifying the feature via `--features somecrate/private-feature` - will allow enabling a private feature that would otherwise be forbidden Attempting to use a private feature in any of the forbidden cases should result in an error. Exact details of how features work will likely be refined during @@ -116,8 +117,7 @@ TODO [prior-art]: #prior-art -- `docs.rs` displays a limited feature table. Features that start with `_` are - hidden from this table. +- `docs.rs` treats features with a leading `_` as private / hidden - Ivy has a [visibility attribute] for its configuration (mentioned in [cargo #10882]) - Discussion on stable/unstable/nightly-only features From 50fba0d19b592cdcc3c8bd76660ba82352518b2e Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 15 Sep 2023 02:45:23 -0400 Subject: [PATCH 4/6] Add motivation and one alternative. Remove unused items, fix typos --- text/3487-feature-visibility.md | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/text/3487-feature-visibility.md b/text/3487-feature-visibility.md index c0887c58ba7..370c5b4ae7b 100644 --- a/text/3487-feature-visibility.md +++ b/text/3487-feature-visibility.md @@ -17,10 +17,19 @@ Please see the parent meta RFC for background information: [`feature-metadata`]. [motivation]: #motivation -WIP +Cargo feature gates are one of the main means to support optional build +configuration in Rust crates. Often these are for configuration options that a +library user may want, but another common use case is hiding API that shouldn't +be available to downstream users. Examples include: -No way to hide unstable API such as testing-only features or internally enabled -features. +- Debugging, benchmarking or test-related features that expose unstable internal + API +- Intermediate features that are enabled by user-facing features but not meant + to be used on their own + +A way to hide these features from user-facing configuration will make options +easier to understand and lowers the chance of library users accidentally using +unstable internal PAI. # Guide-level explanation @@ -91,7 +100,7 @@ Two sample use cases for `public = false` include: This feature requires adjustments to the index for full support. This RFC proposes that it would be acceptable for the first implementation to simply -strip private features from the manifest; this meanss that there will be no way +strip private features from the manifest; this means that there will be no way to `cfg` based on these features. Full support does not need to happen immediately, since it will require this @@ -111,7 +120,10 @@ place. [rationale-and-alternatives]: #rationale-and-alternatives -TODO +- Currently, `docs.rs` will hide features from its autogenerated feature list + if they start with a leading underscore. This convention would work here, but + it would not be consistent with the Rust language (leading underscores indicate + unused variables, lang items are used to indicate visibility) # Prior art @@ -130,8 +142,6 @@ TODO - Are the semantics of `public` proposed in this RFC suitable? Should private features be usable in examples or integration tests without a `--features` argument? -- Should there be a simple `--allow-private-features` flag that allows using all - features, such as for crater runs? This can be decided during implementation. - Does `public` need to be in the index? # Future possibilities @@ -139,12 +149,9 @@ TODO [future-possibilities]: #future-possibilities - A `stable` field can be set false to indicate API-unstable or nightly-only - features (somethign such as `stable = 3.2` could be used to indicate when a + features (something such as `stable = 3.2` could be used to indicate when a feature was stabilized). See also: -- A `rust-version` field that could indicate e.g. `rust-version = "nightly"` or - `rust-version = "1.65"` to specify a MSRV for that feature. See: - - The `public` option could be used to allow optional dev dependencies. See: From 187afcf4f1c36ab8202174c1e6fe740c23bb2c88 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 15 Sep 2023 03:04:28 -0400 Subject: [PATCH 5/6] Clarify index changes, add cfg diagnostic drawback --- text/3487-feature-visibility.md | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/text/3487-feature-visibility.md b/text/3487-feature-visibility.md index 370c5b4ae7b..7878147bf4d 100644 --- a/text/3487-feature-visibility.md +++ b/text/3487-feature-visibility.md @@ -25,11 +25,11 @@ be available to downstream users. Examples include: - Debugging, benchmarking or test-related features that expose unstable internal API - Intermediate features that are enabled by user-facing features but not meant - to be used on their own + to be used on their own (e.g. a feature enabling dependency features) A way to hide these features from user-facing configuration will make options easier to understand and lowers the chance of library users accidentally using -unstable internal PAI. +unstable internal API. # Guide-level explanation @@ -92,20 +92,16 @@ Attempting to use a private feature in any of the forbidden cases should result in an error. Exact details of how features work will likely be refined during implementation and experimentation. -Two sample use cases for `public = false` include: - -- `docs.rs` having a way to know which features should be hidden -- Features that are included in feature chains (feature `a` enables feature `b`) - but not meant for public consumption could be marked not public - This feature requires adjustments to the index for full support. This RFC proposes that it would be acceptable for the first implementation to simply strip private features from the manifest; this means that there will be no way to `cfg` based on these features. Full support does not need to happen immediately, since it will require this -information be present in the index. [Index changes] describes how this can take -place. +information be present in the index. The [`feature-deprecation`] RFC describes +a way to add attributes to features in a forward-compatible way under a +`features3` key, which would be suitible for any additional information needed +here. # Drawbacks @@ -114,7 +110,8 @@ place. - Added complexity to Cargo. Parsing is trivial, but exact implementation details do add test surface area - Added Cargo arguments if escape hatches for `public` are created -- `public` uses may not be common enough to be worth including +- This adds confusion to the `cfg` diagnostics introduced in + # Rationale and alternatives @@ -165,3 +162,4 @@ place. [`rustdoc-cargo-configuration`]: https://github.com/rust-lang/rfcs/pull/3421 [`tokio`]: https://docs.rs/crate/tokio/latest/features [visibility attribute]: https://ant.apache.org/ivy/history/latest-milestone/ivyfile/conf.html +[`feature-deprecation`]: https://github.com/rust-lang/rfcs/pull/3486 \ No newline at end of file From 778bdd7f0c67052c84cfda0000f96714228213ec Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Fri, 15 Sep 2023 17:01:32 -0400 Subject: [PATCH 6/6] Update text/3487-feature-visibility.md Co-authored-by: Weihang Lo --- text/3487-feature-visibility.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/3487-feature-visibility.md b/text/3487-feature-visibility.md index 7878147bf4d..d57c243df02 100644 --- a/text/3487-feature-visibility.md +++ b/text/3487-feature-visibility.md @@ -17,7 +17,7 @@ Please see the parent meta RFC for background information: [`feature-metadata`]. [motivation]: #motivation -Cargo feature gates are one of the main means to support optional build +[Cargo features](link to features doc) are one of the main means to support conditional build and optional dependency configuration in Rust crates. Often these are for configuration options that a library user may want, but another common use case is hiding API that shouldn't be available to downstream users. Examples include: