Skip to content

Commit

Permalink
Auto merge of #13125 - epage:public, r=weihanglo
Browse files Browse the repository at this point in the history
fix(toml): Disallow inheriting of dependency public status

### What does this PR try to resolve?

This is a step towards rust-lang/rust#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.

### How should we test and review this PR?

### Additional information

This reverts parts of #12817
  • Loading branch information
bors committed Dec 6, 2023
2 parents 862e53a + 00557a2 commit 000d279
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 10 deletions.
13 changes: 4 additions & 9 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ fn read_manifest_from_str(
{
for (name, dep) in deps {
if dep.is_optional() {
bail!(
"{} is optional, but workspace dependencies cannot be optional",
name
);
bail!("{name} is optional, but workspace dependencies cannot be optional",);
}
if dep.is_public() {
bail!("{name} is public, but workspace dependencies cannot be public",);
}
}
}
Expand Down Expand Up @@ -1667,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 000d279

Please sign in to comment.