From 00557a2a772ba8ae517a8735319eed88ed280709 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 27 Nov 2023 10:23:29 -0600 Subject: [PATCH] fix(toml): Disallow inheriting of dependency `public` status 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 --- src/cargo/util/toml/mod.rs | 8 +++----- src/cargo/util_schemas/manifest.rs | 7 +++++++ src/doc/src/reference/unstable.md | 3 +++ tests/testsuite/pub_priv.rs | 11 ++++++++++- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index a7954676800..130584b0fd0 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -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() { @@ -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 diff --git a/src/cargo/util_schemas/manifest.rs b/src/cargo/util_schemas/manifest.rs index 90cc9a84499..588dca678cf 100644 --- a/src/cargo/util_schemas/manifest.rs +++ b/src/cargo/util_schemas/manifest.rs @@ -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 { match self { TomlDependency::Simple(_) => vec![], diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index b855ec1c3df..813276cb6ef 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -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) diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs index b2160e0fa11..f075f1eab02 100644 --- a/tests/testsuite/pub_priv.rs +++ b/tests/testsuite/pub_priv.rs @@ -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(); @@ -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() }