Skip to content

Commit

Permalink
fix(toml): Disallow inheriting of dependency public status
Browse files Browse the repository at this point in the history
This is a step towards #44663.  When discussing inheriting this field
for #13046, we realized that we should probably start by disallowing
inheritance.  We can always add it later.  imo the principle of what should
be inherited is what is truely common among dependencies.  For example,
we don't allow removing features.  Public should not be universally
applied and likely should be explicit so its not over-done, especially
since we can't (atm) lint for when a public dependency could be
non-public.

This reverts parts of #12817
  • Loading branch information
epage committed Dec 5, 2023
1 parent 123289b commit 00557a2
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 6 deletions.
8 changes: 3 additions & 5 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ fn read_manifest_from_str(
if dep.is_optional() {
bail!("{name} is optional, but workspace dependencies cannot be optional",);
}
if dep.is_public() {
bail!("{name} is public, but workspace dependencies cannot be public",);
}
}
}
return if manifest.project.is_some() || manifest.package.is_some() {
Expand Down Expand Up @@ -1664,11 +1667,6 @@ fn inner_dependency_inherit_with<'a>(
}
_ => {}
}
// Inherit the workspace configuration for `public` unless
// it's explicitly specified for this dependency.
if let Some(public) = dependency.public {
d.public = Some(public);
}
d.features = match (d.features.clone(), dependency.features.clone()) {
(Some(dep_feat), Some(inherit_feat)) => Some(
dep_feat
Expand Down
7 changes: 7 additions & 0 deletions src/cargo/util_schemas/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,13 @@ impl TomlDependency {
}
}

pub fn is_public(&self) -> bool {
match self {
TomlDependency::Detailed(d) => d.public.unwrap_or(false),
TomlDependency::Simple(..) => false,
}
}

pub fn unused_keys(&self) -> Vec<String> {
match self {
TomlDependency::Simple(_) => vec![],
Expand Down
3 changes: 3 additions & 0 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ my_dep = { version = "1.2.3", public = true }
private_dep = "2.0.0" # Will be 'private' by default
```

Documentation updates:
- For workspace's "The `dependencies` table" section, include `public` as an unsupported field for `workspace.dependencies`

## msrv-policy
- [#9930](https://github.com/rust-lang/cargo/issues/9930) (MSRV-aware resolver)
- [#10653](https://github.com/rust-lang/cargo/issues/10653) (MSRV-aware cargo-add)
Expand Down
11 changes: 10 additions & 1 deletion tests/testsuite/pub_priv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ Caused by:
}

#[cargo_test(nightly, reason = "exported_private_dependencies lint is unstable")]
fn workspace_dep_made_public() {
fn workspace_pub_disallowed() {
Package::new("foo1", "0.1.0")
.file("src/lib.rs", "pub struct FromFoo;")
.publish();
Expand Down Expand Up @@ -244,5 +244,14 @@ fn workspace_dep_made_public() {

p.cargo("check")
.masquerade_as_nightly_cargo(&["public-dependency"])
.with_status(101)
.with_stderr(
"\
error: failed to parse manifest at `[CWD]/Cargo.toml`
Caused by:
foo2 is public, but workspace dependencies cannot be public
",
)
.run()
}

0 comments on commit 00557a2

Please sign in to comment.