Skip to content

Commit 4896e9a

Browse files
committed
fix: Switch from pub to public
Since we switched from warn-by-default to allow-by-default, the need to set `pub` on dependencies with an unsupported MSRV is much lower. It'd help with people wanting to enable this now, especially for core packages that are more likely to have lower MSRVs (but also less likely to have dependencies). So if we drop the requirement for allowing `pub` on old MSRVs, we can re-visit using the name `public`.
1 parent a3ecfa6 commit 4896e9a

File tree

1 file changed

+30
-32
lines changed

1 file changed

+30
-32
lines changed

text/3516-public-private-dependencies.md

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ name = "diagnostic"
9191
version = "1.0.0"
9292

9393
[dependencies]
94-
serde = { version = "1", features = ["derive"], pub = true }
95-
serde_json = { version = "1", pub = true }
94+
serde = { version = "1", features = ["derive"], public = true }
95+
serde_json = { version = "1", public = true }
9696
```
97-
For edition migrations, `cargo fix` will look for the warning code and mark those dependencies as `pub`.
97+
For edition migrations, `cargo fix` will look for the warning code and mark those dependencies as `public`.
9898

9999
However, for this example, it was an oversight in exposing `serde_json` in the public API.
100100
Note that removing it from the public API is a semver incompatible change.
@@ -104,7 +104,7 @@ name = "diagnostic"
104104
version = "1.0.0"
105105

106106
[dependencies]
107-
serde = { version = "1", features = ["derive"], pub = true }
107+
serde = { version = "1", features = ["derive"], public = true }
108108
serde_json = "1"
109109
```
110110
```rust
@@ -174,7 +174,7 @@ The main change to the compiler will be to accept a new modifier on the `--exter
174174
supplies which marks it as a private dependency.
175175
The modifier will be called `priv` (e.g. `--extern priv:serde`).
176176
The compiler then emits the lint `external_private_dependency` if it encounters private
177-
dependencies exposed as `pub`.
177+
dependencies exposed as `public`.
178178

179179
`external_private_dependency` will be `allow` by default for pre-2024 editions.
180180
It will be a member of the `rust-2024-compatibility` lint group so that it gets automatically picked up by `cargo fix --edition`.
@@ -189,24 +189,20 @@ This most likely will also be necessary for the more complex relationship of
189189

190190
## cargo
191191

192-
A new dependency field, `pub = <bool>` will be added that defaults to `false`.
192+
A new dependency field, `public = <bool>` will be added that defaults to `false`.
193193
This field can be specified in `workspace.dependencies` and be overridden when `workspace = true` is in a dependency.
194194
When building a `lib`, Cargo will use the `priv` modifier with `--extern` for all private dependencies.
195-
What is private is what is left after recursively walking public dependencies (`pub = true`).
195+
What is private is what is left after recursively walking public dependencies (`public = true`).
196196
For other [`crate-type`s](https://doc.rust-lang.org/cargo/reference/cargo-targets.html#the-crate-type-field) (e.g. `bin`),
197-
we'll tell rustc that all dependencies are public to reduce noise from inaccessible `pub` items.
198-
199-
Old cargo versions will emit a warning when this key is encountered but otherwise continue,
200-
even if the feature is present but unstable.
201-
While it is unstable, `cargo publish` will strip the field.
197+
we'll tell rustc that all dependencies are public to reduce noise from inaccessible `public` items.
202198

203199
Cargo will not force a `rust-version` bump when using this feature as someone
204-
building with an old version of cargo depending on packages that set `pub =
200+
building with an old version of cargo depending on packages that set `public =
205201
true` will not start to fail when upgrading to new versions of cargo.
206202

207-
`cargo add` will gain `--pub <bool>` flags to control this field.
203+
`cargo add` will gain `--public <bool>` flags to control this field.
208204
When adding a dependency today, the version requirement is reused from other dependency tables within your manifest.
209-
With this RFC, that will be extended to also checking your dependencies for any `pub` dependencies, and reusing their version requirement.
205+
With this RFC, that will be extended to also checking your dependencies for any `public` dependencies, and reusing their version requirement.
210206
This would be most easily done by having the field in the Index but `cargo add` could also read the `.crate` files as a fallback.
211207

212208
## crates.io
@@ -233,33 +229,35 @@ This doesn't cover the case where a dependency is public only if a feature is en
233229
The warning/error is emitted even when a `pub` item isn't accessible.
234230
We at least reduced the impact of this by not marking dependencies as private for `crate-type=["bin"]`.
235231

236-
You can't definitively lint when a `pub = true` is unused since it may depend on which platform or features.
232+
You can't definitively lint when a `public = true` is unused since it may depend on which platform or features.
237233

238234
# Rationale and alternatives
239235
[rationale-and-alternatives]: #rationale-and-alternatives
240236

241237
## Misc
242238

243-
- `Cargo.toml`: instead of `pub` (named after the [Rust keyword](https://doc.rust-lang.org/reference/visibility-and-privacy.html), we could name the field `public` (like [RFC 1977]) or name the field `visibility = "<public|private>"`
244-
- The parallel with Rust seemed worth pursuing
239+
- `Cargo.toml`: instead of `public` (like [RFC 1977]), we could name the field `pub` (named after the [Rust keyword](https://doc.rust-lang.org/reference/visibility-and-privacy.html) or name the field `visibility = "<public|private>"`
240+
- `pub` has a nice parallel with Rust
241+
- `pub`: Cargo doesn't use abbreviations as much as Rust (though some are used)
245242
- `pub` could be seen as ambiguous with `publish`
243+
- `public` already is reserved and requires a `cargo_features`, meaning using it requires an MSRV bump
246244
- While `visibility` would offer greater flexibility, it is unclear if we need that flexibility and if the friction of any feature leveraging it would be worth it
247-
- In the end, we went with `pub` because `public` would prevent our "no MSRV bump" approach because cargo versions exist with `public` being reserved
248245
- `rustc`: Instead of `allow` by default for pre-2024 editions, we could warn by default
249246
- More people would get the benefit of the feature now
250247
- However, this would be extremely noisy and likely make people grumpy
251-
- `Cargo.toml`: Instead of `pub = false` being the default and changing the warning level on an edition boundary, we could instead start with `pub = true` and change the default on an edition boundary.
252-
- This would require `cargo fix` marking all dependencies as `pub = true`, while using the warning means we can limit it to only those dependencies that need it.
253-
- `Cargo.toml`: Instead of `pub = false` being the default, we could have a "unchecked" / "unset" state
254-
- This would require `cargo fix` marking all dependencies as `pub = true`, while using the warning means we can limit it to only those dependencies that need it.
255-
- `Cargo.toml`: In the long term, we decided on the default being `pub = false` as that is the common case and gives us more information than supporting a `pub = "unchecked"` and having that be the long term solution.
256-
- `cargo add`: instead of `--pub <bool>` it could be `--pub` / `--no-pub` like `--optional` or `--public` / `--private`
257-
- `cargo add`: when adding a dependency, we could automatically add all of its `pub` dependencies.
258-
- This was passed up as being too noisy, especially when dealing with facade crates, those that fully re-export their `pub = true` dependency
259-
- We leave whether `pub` is in the Index as unspecified
248+
- If we did this, we'd likely want to not require an MSRV bump so people can immediately silence the warning which would require using a key besides `public` (since its already reserved) and treating the field as an unused key when the `-Z` isn't enabled.
249+
- `Cargo.toml`: Instead of `public = false` being the default and changing the warning level on an edition boundary, we could instead start with `public = true` and change the default on an edition boundary.
250+
- This would require `cargo fix` marking all dependencies as `public = true`, while using the warning means we can limit it to only those dependencies that need it.
251+
- `Cargo.toml`: Instead of `public = false` being the default, we could have a "unchecked" / "unset" state
252+
- This would require `cargo fix` marking all dependencies as `public = true`, while using the warning means we can limit it to only those dependencies that need it.
253+
- `Cargo.toml`: In the long term, we decided on the default being `public = false` as that is the common case and gives us more information than supporting a `public = "unchecked"` and having that be the long term solution.
254+
- `cargo add`: instead of `--public <bool>` it could be `--public` / `--no-public` like `--optional` or `--public` / `--private`
255+
- `cargo add`: when adding a dependency, we could automatically add all of its `public` dependencies.
256+
- This was passed up as being too noisy, especially when dealing with facade crates, those that fully re-export their `public = true` dependency
257+
- We leave whether `public` is in the Index as unspecified
260258
- It isn't strictly needed now
261259
- It would make `cargo add` easier
262-
- If we rely on `pub` in the resolver, we might need it but we can always backfill it
260+
- If we rely on `public` in the resolver, we might need it but we can always backfill it
263261
- Parts of the implementation are already there from the original RFC
264262

265263
## Minimal version resolution
@@ -273,7 +271,7 @@ This should be handled independent of this RFC.
273271
This is deferred to [Future possibilities](#future-possibilities)
274272
- This has been the main hang-up for stabilization over the last 6 years since the RFC was approved
275273
- For more on the complexity involved, see the thread starting at [this comment](https://github.com/rust-lang/rust/issues/44663#issuecomment-881965668)
276-
- More thought is needed as we found that making a dependency `pub = true` can be a breaking change if the caller also depends on it but with a different semver incompatible version
274+
- More thought is needed as we found that making a dependency `public = true` can be a breaking change if the caller also depends on it but with a different semver incompatible version
277275
- More thought is needed on what happens if you have multiple versions of a package that are public (via renaming like `tokio_03` and `tokio_1`)
278276

279277
Related problems potentially blocked on this
@@ -347,7 +345,7 @@ How this will work:
347345
If we want to go this route, some hurdles to overcome include:
348346
- Difficulties in working with cargo's resolver as this has been the main hang-up for stabilization over the last 6 years since the [RFC 1977] was approved
349347
- For more on the complexity involved, see the thread starting at [this comment](https://github.com/rust-lang/rust/issues/44663#issuecomment-881965668)
350-
- More thought is needed as we found that making a dependency `pub = true` can be a breaking change if the caller also depends on it but with a different semver incompatible version
348+
- More thought is needed as we found that making a dependency `public = true` can be a breaking change if the caller also depends on it but with a different semver incompatible version
351349
- More thought is needed on what happens if you have multiple versions of a package that are public (via renaming like `tokio_03` and `tokio_1`)
352350

353351
### Caller-declared relations
@@ -378,7 +376,7 @@ version should resolve to (clap 3.4 vs clap 4.0), then it is an error.
378376

379377
Compared to the resolver doing this implicitly
380378
- It is unclear if this would be any more difficult to implement in the resolver
381-
- Changing a dependency from `pub = false` to `pub = true` is backwards compatible because it has no affect on existing callers.
379+
- Changing a dependency from `public = false` to `public = true` is backwards compatible because it has no affect on existing callers.
382380
- It is unclear how this would handle multiple versions of a package that are public
383381

384382
The downside is it feels like the declaration is backwards.

0 commit comments

Comments
 (0)