From 87f1a4b217630bd663dbf891fc970c1bc8b192d6 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 20 Mar 2019 19:25:59 -0400 Subject: [PATCH 01/11] Implement the 'frontend' of public-private dependencies This is part of https://github.com/rust-lang/rust/issues/44663 This implements the 'frontend' portion of RFC 1977. Once PRs https://github.com/rust-lang/rust/pull/59335 and https://github.com/rust-lang/crates.io/pull/1685 are merged, it will be possible to test the full public-private dependency feature: marking a dependency a public, seeing exported_private_dependencies warnings from rustc, and seeing pub-dep-reachability errors from Cargo. Everything in this commit should be fully backwards-compatible - users who don't enable the 'public-dependency' cargo feature won't notice any changes. Note that this commit does *not* implement the remaining two features of the RFC: * Choosing smallest versions when 'cargo publish' is run * Turning exported_private_dependencies warnings into hard errors when 'cargo publish' is run The former is a major change to Cargo's behavior, and should be done in a separate PR with some kind of rollout plan. The latter is described by the RFC as being enabled at 'some point in the future'. This can be done via a follow-up PR. --- src/cargo/core/compiler/build_context/mod.rs | 5 + src/cargo/core/compiler/mod.rs | 20 ++- src/cargo/core/features.rs | 3 + src/cargo/core/resolver/resolve.rs | 25 +++ src/cargo/core/workspace.rs | 13 +- src/cargo/ops/cargo_package.rs | 11 +- src/cargo/ops/resolve.rs | 3 +- src/cargo/sources/registry/mod.rs | 8 +- src/cargo/util/toml/mod.rs | 6 + tests/testsuite/main.rs | 1 + tests/testsuite/pub_priv.rs | 180 +++++++++++++++++++ 11 files changed, 267 insertions(+), 8 deletions(-) create mode 100644 tests/testsuite/pub_priv.rs diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 858f7ae9074..9b55fedb88c 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -93,6 +93,11 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { .extern_crate_name(unit.pkg.package_id(), dep.pkg.package_id(), dep.target) } + pub fn is_public_dependency(&self, unit: &Unit<'a>, dep: &Unit<'a>) -> bool { + self.resolve + .is_public_dep(unit.pkg.package_id(), dep.pkg.package_id()) + } + /// Whether a dependency should be compiled for the host or target platform, /// specified by `Kind`. pub fn dep_platform_activated(&self, dep: &Dependency, kind: Kind) -> bool { diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 9c9f6533b92..249d2000378 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -23,6 +23,7 @@ use log::debug; use same_file::is_same_file; use serde::Serialize; +use crate::core::Feature; pub use self::build_config::{BuildConfig, CompileMode, MessageFormat}; pub use self::build_context::{BuildContext, FileFlavor, TargetConfig, TargetInfo}; use self::build_plan::BuildPlan; @@ -966,15 +967,24 @@ fn build_deps_args<'a, 'cfg>( } } + let mut unstable_opts = false; + for dep in dep_targets { if dep.mode.is_run_custom_build() { cmd.env("OUT_DIR", &cx.files().build_script_out_dir(&dep)); } if dep.target.linkable() && !dep.mode.is_doc() { - link_to(cmd, cx, unit, &dep)?; + link_to(cmd, cx, unit, &dep, &mut unstable_opts)?; } } + // This will only be set if we're already usign a feature + // requiring nightly rust + if unstable_opts { + cmd.arg("-Z").arg("unstable-options"); + } + + return Ok(()); fn link_to<'a, 'cfg>( @@ -982,6 +992,7 @@ fn build_deps_args<'a, 'cfg>( cx: &mut Context<'a, 'cfg>, current: &Unit<'a>, dep: &Unit<'a>, + need_unstable_opts: &mut bool ) -> CargoResult<()> { let bcx = cx.bcx; for output in cx.outputs(dep)?.iter() { @@ -996,6 +1007,13 @@ fn build_deps_args<'a, 'cfg>( v.push(&path::MAIN_SEPARATOR.to_string()); v.push(&output.path.file_name().unwrap()); cmd.arg("--extern").arg(&v); + + if current.pkg.manifest().features().require(Feature::public_dependency()).is_ok() { + if !bcx.is_public_dependency(current, dep) { + cmd.arg("--extern-private").arg(&v); + *need_unstable_opts = true + } + } } Ok(()) } diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 59787c3c3e8..3d0ae9b1759 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -199,6 +199,9 @@ features! { // Declarative build scripts. [unstable] metabuild: bool, + + // Specifying the 'public' attribute on dependencies + [unstable] public_dependency: bool, } } diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 04261d1f398..a2acb32c755 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -6,6 +6,7 @@ use std::iter::FromIterator; use url::Url; use crate::core::{Dependency, PackageId, PackageIdSpec, Summary, Target}; +use crate::core::dependency::Kind; use crate::util::errors::CargoResult; use crate::util::Graph; @@ -29,6 +30,8 @@ pub struct Resolve { checksums: HashMap>, metadata: Metadata, unused_patches: Vec, + // A map from packages to a set of their public dependencies + public_dependencies: HashMap>, } impl Resolve { @@ -41,6 +44,21 @@ impl Resolve { unused_patches: Vec, ) -> Resolve { let reverse_replacements = replacements.iter().map(|(&p, &r)| (r, p)).collect(); + let public_dependencies = graph.iter().map(|p| { + let public_deps = graph.edges(p).flat_map(|(dep_package, deps)| { + let id_opt: Option = deps.iter().find(|d| d.kind() == Kind::Normal).and_then(|d| { + if d.is_public() { + Some(dep_package.clone()) + } else { + None + } + }); + id_opt + }).collect::>(); + + (p.clone(), public_deps) + }).collect(); + Resolve { graph, replacements, @@ -50,6 +68,7 @@ impl Resolve { unused_patches, empty_features: HashSet::new(), reverse_replacements, + public_dependencies } } @@ -197,6 +216,12 @@ unable to verify that `{0}` is the same as when the lockfile was generated self.features.get(&pkg).unwrap_or(&self.empty_features) } + pub fn is_public_dep(&self, pkg: PackageId, dep: PackageId) -> bool { + self.public_dependencies.get(&pkg) + .map(|public_deps| public_deps.contains(&dep)) + .unwrap_or_else(|| panic!("Unknown dependency {:?} for package {:?}", dep, pkg)) + } + pub fn features_sorted(&self, pkg: PackageId) -> Vec<&str> { let mut v = Vec::from_iter(self.features(pkg).iter().map(|s| s.as_ref())); v.sort_unstable(); diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7d71afde402..304e6f86fa7 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -8,6 +8,7 @@ use glob::glob; use log::debug; use url::Url; +use crate::core::features::Features; use crate::core::profiles::Profiles; use crate::core::registry::PackageRegistry; use crate::core::{Dependency, PackageIdSpec, PackageId}; @@ -540,6 +541,13 @@ impl<'cfg> Workspace<'cfg> { Ok(()) } + pub fn features(&self) -> &Features { + match self.root_maybe() { + MaybePackage::Package(p) => p.manifest().features(), + MaybePackage::Virtual(vm) => vm.features(), + } + } + /// Validates a workspace, ensuring that a number of invariants are upheld: /// /// 1. A workspace only has one root. @@ -547,10 +555,7 @@ impl<'cfg> Workspace<'cfg> { /// 3. The current crate is a member of this workspace. fn validate(&mut self) -> CargoResult<()> { // Validate config profiles only once per workspace. - let features = match self.root_maybe() { - MaybePackage::Package(p) => p.manifest().features(), - MaybePackage::Virtual(vm) => vm.features(), - }; + let features = self.features(); let mut warnings = Vec::new(); self.config.profiles()?.validate(features, &mut warnings)?; for warning in warnings { diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index edb7aca9cbd..f38f21c8dcb 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -18,6 +18,7 @@ use crate::core::resolver::Method; use crate::core::{ Package, PackageId, PackageIdSpec, PackageSet, Resolve, Source, SourceId, Verbosity, Workspace, }; +use crate::core::Feature; use crate::ops; use crate::sources::PathSource; use crate::util::errors::{CargoResult, CargoResultExt}; @@ -590,6 +591,14 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car let pkg_fingerprint = hash_all(&dst)?; let ws = Workspace::ephemeral(new_pkg, config, None, true)?; + let rustc_args = if pkg.manifest().features().require(Feature::public_dependency()).is_ok() { + // FIXME: Turn this on at some point in the future + //Some(vec!["-D exported_private_dependencies".to_string()]) + None + } else { + None + }; + let exec: Arc = Arc::new(DefaultExecutor); ops::compile_ws( &ws, @@ -604,7 +613,7 @@ fn run_verify(ws: &Workspace<'_>, tar: &FileLock, opts: &PackageOpts<'_>) -> Car required_features_filterable: true, }, target_rustdoc_args: None, - target_rustc_args: None, + target_rustc_args: rustc_args, local_rustdoc_args: None, export_dir: None, }, diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 48acfeaab35..ac6759c8a06 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -2,6 +2,7 @@ use std::collections::HashSet; use log::{debug, trace}; +use crate::core::Feature; use crate::core::registry::PackageRegistry; use crate::core::resolver::{self, Method, Resolve}; use crate::core::{PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace}; @@ -330,7 +331,7 @@ pub fn resolve_with_previous<'cfg>( registry, &try_to_use, Some(ws.config()), - false, // TODO: use "public and private dependencies" feature flag + ws.features().require(Feature::public_dependency()).is_ok(), )?; resolved.register_used_patches(registry.patches()); if register_patches { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index a2c6d4f9ce7..e4c2d027474 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -285,6 +285,7 @@ struct RegistryDependency<'a> { kind: Option>, registry: Option>, package: Option>, + public: Option } impl<'a> RegistryDependency<'a> { @@ -300,6 +301,7 @@ impl<'a> RegistryDependency<'a> { kind, registry, package, + public } = self; let id = if let Some(registry) = ®istry { @@ -324,6 +326,9 @@ impl<'a> RegistryDependency<'a> { None => None, }; + // All dependencies are private by default + let public = public.unwrap_or(false); + // Unfortunately older versions of cargo and/or the registry ended up // publishing lots of entries where the features array contained the // empty feature, "", inside. This confuses the resolution process much @@ -341,7 +346,8 @@ impl<'a> RegistryDependency<'a> { .set_default_features(default_features) .set_features(features) .set_platform(platform) - .set_kind(kind); + .set_kind(kind) + .set_public(public); Ok(dep) } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index be2baca7ae3..4e83cfe99e9 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -239,6 +239,7 @@ pub struct DetailedTomlDependency { #[serde(rename = "default_features")] default_features2: Option, package: Option, + public: Option } #[derive(Debug, Deserialize, Serialize)] @@ -1461,6 +1462,11 @@ impl DetailedTomlDependency { cx.features.require(Feature::rename_dependency())?; dep.set_explicit_name_in_toml(name_in_toml); } + + if let Some(p) = self.public { + cx.features.require(Feature::public_dependency())?; + dep.set_public(p); + } Ok(dep) } } diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index ddc922ce894..74c767a150c 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -67,6 +67,7 @@ mod profile_config; mod profile_overrides; mod profile_targets; mod profiles; +mod pub_priv; mod publish; mod publish_lockfile; mod read_manifest; diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs new file mode 100644 index 00000000000..742d4127295 --- /dev/null +++ b/tests/testsuite/pub_priv.rs @@ -0,0 +1,180 @@ +use crate::support::registry::Package; +use crate::support::{project, is_nightly}; + +#[test] +fn exported_priv_warning() { + if !is_nightly() { + return; + } + Package::new("priv_dep", "0.1.0") + .file( + "src/lib.rs", + "pub struct FromPriv;" + ) + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["public-dependency"] + + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + priv_dep = "0.1.0" + "#, + ) + .file( + "src/lib.rs", + " + extern crate priv_dep; + pub fn use_priv(_: priv_dep::FromPriv) {} + ", + ) + .build(); + + p.cargo("build --message-format=short") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] priv_dep v0.1.0 ([..]) +[COMPILING] priv_dep v0.1.0 +[COMPILING] foo v0.0.1 ([CWD]) +src/lib.rs:3:13: warning: type `priv_dep::FromPriv` from private dependency 'priv_dep' in public interface +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +" + ) + .run() +} + +#[test] +fn exported_pub_dep() { + if !is_nightly() { + return; + } + Package::new("pub_dep", "0.1.0") + .file( + "src/lib.rs", + "pub struct FromPub;" + ) + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["public-dependency"] + + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + pub_dep = {version = "0.1.0", public = true} + "#, + ) + .file( + "src/lib.rs", + " + extern crate pub_dep; + pub fn use_pub(_: pub_dep::FromPub) {} + ", + ) + .build(); + + p.cargo("build --message-format=short") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] pub_dep v0.1.0 ([..]) +[COMPILING] pub_dep v0.1.0 +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +" + ) + .run() + +} + +#[test] +pub fn requires_nightly_cargo() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["public-dependency"] + "#, + ) + .file( + "src/lib.rs", + "" + ) + .build(); + + p.cargo("build --message-format=short") + .with_status(101) + .with_stderr( + "\ +error: failed to parse manifest at `[..]` + +Caused by: + the cargo feature `public-dependency` requires a nightly version of Cargo, but this is the `stable` channel +" + ) + .run() +} + +#[test] +fn requires_feature() { + + + Package::new("pub_dep", "0.1.0") + .file( + "src/lib.rs", + "" + ) + .publish(); + + + let p = project() + .file( + "Cargo.toml", + r#" + + [package] + name = "foo" + version = "0.0.1" + + [dependencies] + pub_dep = { version = "0.1.0", public = true } + "#, + ) + .file( + "src/lib.rs", + "" + ) + .build(); + + p.cargo("build --message-format=short") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +error: failed to parse manifest at `[..]` + +Caused by: + feature `public-dependency` is required + +consider adding `cargo-features = [\"public-dependency\"]` to the manifest +" + ) + .run() + +} From be9ae5e547a83b3348406c7d73dcbd56cde82e63 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 14 Apr 2019 16:22:40 -0400 Subject: [PATCH 02/11] Fix error message on stable --- tests/testsuite/pub_priv.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs index 742d4127295..7d31576ced5 100644 --- a/tests/testsuite/pub_priv.rs +++ b/tests/testsuite/pub_priv.rs @@ -126,6 +126,7 @@ error: failed to parse manifest at `[..]` Caused by: the cargo feature `public-dependency` requires a nightly version of Cargo, but this is the `stable` channel +See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels. " ) .run() From 715d6ace0275f9577ec453d08fdaaf27d243ef68 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 23 Apr 2019 00:35:34 -0400 Subject: [PATCH 03/11] Update unstable reference --- src/doc/src/reference/unstable.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 9a740d75881..b78cb5c7d95 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -264,3 +264,20 @@ conflicting binaries from another package. Additionally, a new flag `--no-track` is available to prevent `cargo install` from writing tracking information in `$CARGO_HOME` about which packages are installed. + +### public-dependency +* Tracking Issue: [#44663](https://github.com/rust-lang/rust/issues/44663) + +The 'public-dependency' features allows marking dependencies as 'public' +or 'private'. When this feature is enabled, additional information is passed to rustc to allow +the 'exported_private_dependencies' lint to function properly. + +This requires the appropriate key to be set in `cargo-features`: + +```toml +cargo-features = ["public-dependency"] + +[dependencies] +my_dep = { version = "1.2.3", public = true } +private_dep = "2.0.0" # Will be 'private' by default +``` From 8185564a53e28913617418f2c8a2b59f7422a0b2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 23 Apr 2019 00:46:05 -0400 Subject: [PATCH 04/11] Track 'public/private' depenendecy status in fingerprint --- src/cargo/core/compiler/fingerprint.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index e7a2b378542..b5832903672 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -315,11 +315,13 @@ pub fn prepare_target<'a, 'cfg>( /// A compilation unit dependency has a fingerprint that is comprised of: /// * its package ID /// * its extern crate name +/// * its public/private status /// * its calculated fingerprint for the dependency #[derive(Clone)] struct DepFingerprint { pkg_id: u64, name: String, + public: bool, fingerprint: Arc, } @@ -429,10 +431,11 @@ impl<'de> Deserialize<'de> for DepFingerprint { where D: de::Deserializer<'de>, { - let (pkg_id, name, hash) = <(u64, String, u64)>::deserialize(d)?; + let (pkg_id, name, public, hash) = <(u64, String, bool, u64)>::deserialize(d)?; Ok(DepFingerprint { pkg_id, name, + public, fingerprint: Arc::new(Fingerprint { memoized_hash: Mutex::new(Some(hash)), ..Fingerprint::new() @@ -850,11 +853,13 @@ impl hash::Hash for Fingerprint { for DepFingerprint { pkg_id, name, + public, fingerprint, } in deps { pkg_id.hash(h); name.hash(h); + public.hash(h); // use memoized dep hashes to avoid exponential blowup h.write_u64(Fingerprint::hash(fingerprint)); } @@ -900,6 +905,7 @@ impl DepFingerprint { ) -> CargoResult { let fingerprint = calculate(cx, dep)?; let name = cx.bcx.extern_crate_name(parent, dep)?; + let public = cx.bcx.is_public_dependency(parent, dep); // We need to be careful about what we hash here. We have a goal of // supporting renaming a project directory and not rebuilding @@ -920,6 +926,7 @@ impl DepFingerprint { Ok(DepFingerprint { pkg_id, name, + public, fingerprint, }) } From 8a45e5c0b01ae716292de879ccc39cb07d72b798 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 23 Apr 2019 00:47:31 -0400 Subject: [PATCH 05/11] Format pub_priv test --- tests/testsuite/pub_priv.rs | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs index 7d31576ced5..32d688ccf06 100644 --- a/tests/testsuite/pub_priv.rs +++ b/tests/testsuite/pub_priv.rs @@ -1,5 +1,5 @@ use crate::support::registry::Package; -use crate::support::{project, is_nightly}; +use crate::support::{is_nightly, project}; #[test] fn exported_priv_warning() { @@ -7,10 +7,7 @@ fn exported_priv_warning() { return; } Package::new("priv_dep", "0.1.0") - .file( - "src/lib.rs", - "pub struct FromPriv;" - ) + .file("src/lib.rs", "pub struct FromPriv;") .publish(); let p = project() @@ -58,10 +55,7 @@ fn exported_pub_dep() { return; } Package::new("pub_dep", "0.1.0") - .file( - "src/lib.rs", - "pub struct FromPub;" - ) + .file("src/lib.rs", "pub struct FromPub;") .publish(); let p = project() @@ -97,10 +91,9 @@ fn exported_pub_dep() { [COMPILING] pub_dep v0.1.0 [COMPILING] foo v0.0.1 ([CWD]) [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] -" +", ) .run() - } #[test] @@ -112,10 +105,7 @@ pub fn requires_nightly_cargo() { cargo-features = ["public-dependency"] "#, ) - .file( - "src/lib.rs", - "" - ) + .file("src/lib.rs", "") .build(); p.cargo("build --message-format=short") @@ -134,16 +124,10 @@ See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more inform #[test] fn requires_feature() { - - Package::new("pub_dep", "0.1.0") - .file( - "src/lib.rs", - "" - ) + .file("src/lib.rs", "") .publish(); - let p = project() .file( "Cargo.toml", @@ -157,10 +141,7 @@ fn requires_feature() { pub_dep = { version = "0.1.0", public = true } "#, ) - .file( - "src/lib.rs", - "" - ) + .file("src/lib.rs", "") .build(); p.cargo("build --message-format=short") @@ -174,8 +155,7 @@ Caused by: feature `public-dependency` is required consider adding `cargo-features = [\"public-dependency\"]` to the manifest -" +", ) .run() - } From e578fc1835d8259e700b38fb2e07f31bb731d749 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 23 Apr 2019 01:40:39 -0400 Subject: [PATCH 06/11] Fix serializing fingerprint --- src/cargo/core/compiler/fingerprint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index b5832903672..c504ee68e67 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -422,7 +422,7 @@ impl Serialize for DepFingerprint { where S: ser::Serializer, { - (&self.pkg_id, &self.name, &self.fingerprint.hash()).serialize(ser) + (&self.pkg_id, &self.name, &self.public, &self.fingerprint.hash()).serialize(ser) } } From a22a7dbc382dc4fa408dae2f55ef6b1cc0e2fc1f Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 23 Apr 2019 15:48:55 -0400 Subject: [PATCH 07/11] Pass --extern-private or --extern, but not both --- src/cargo/core/compiler/mod.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 249d2000378..fe80d1fc758 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -1006,14 +1006,24 @@ fn build_deps_args<'a, 'cfg>( v.push(cx.files().out_dir(dep)); v.push(&path::MAIN_SEPARATOR.to_string()); v.push(&output.path.file_name().unwrap()); - cmd.arg("--extern").arg(&v); + + let mut private = false; if current.pkg.manifest().features().require(Feature::public_dependency()).is_ok() { if !bcx.is_public_dependency(current, dep) { - cmd.arg("--extern-private").arg(&v); - *need_unstable_opts = true + private = true; } } + + if private { + cmd.arg("--extern-private"); + } else { + cmd.arg("--extern"); + } + + cmd.arg(&v); + *need_unstable_opts |= private; + } Ok(()) } From 8c8824fad86c6f923556f3c8a23bfc742dda361d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 25 Apr 2019 22:29:02 -0400 Subject: [PATCH 08/11] Remove 'private' flag --- src/cargo/core/compiler/mod.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index fe80d1fc758..e2107b8a97c 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -1007,23 +1007,16 @@ fn build_deps_args<'a, 'cfg>( v.push(&path::MAIN_SEPARATOR.to_string()); v.push(&output.path.file_name().unwrap()); - let mut private = false; + if current.pkg.manifest().features().require(Feature::public_dependency()).is_ok() && + !bcx.is_public_dependency(current, dep) { - if current.pkg.manifest().features().require(Feature::public_dependency()).is_ok() { - if !bcx.is_public_dependency(current, dep) { - private = true; - } - } - - if private { - cmd.arg("--extern-private"); + cmd.arg("--extern-private"); + *need_unstable_opts = true; } else { cmd.arg("--extern"); } cmd.arg(&v); - *need_unstable_opts |= private; - } Ok(()) } From df4d2095ac7fce5cfd68eebf45caf3317bc3d71b Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 25 Apr 2019 22:34:18 -0400 Subject: [PATCH 09/11] Prevent 'public' specifier from being used on non-Normal dependencies --- src/cargo/core/dependency.rs | 2 ++ src/cargo/util/toml/mod.rs | 5 ++++ tests/testsuite/pub_priv.rs | 44 ++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index b779d77c693..36cbcfced48 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -301,6 +301,8 @@ impl Dependency { /// Sets whether the dependency is public. pub fn set_public(&mut self, public: bool) -> &mut Dependency { + // Setting 'public' only makes sense for normal dependencies + assert_eq!(self.kind(), Kind::Normal); Rc::make_mut(&mut self.inner).public = public; self } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 4e83cfe99e9..10311359f62 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1465,6 +1465,11 @@ impl DetailedTomlDependency { if let Some(p) = self.public { cx.features.require(Feature::public_dependency())?; + + if dep.kind() != Kind::Normal { + bail!("'public' specifier can only be used on regular dependencies, not {:?} dependencies", dep.kind()); + } + dep.set_public(p); } Ok(dep) diff --git a/tests/testsuite/pub_priv.rs b/tests/testsuite/pub_priv.rs index 32d688ccf06..894f7775661 100644 --- a/tests/testsuite/pub_priv.rs +++ b/tests/testsuite/pub_priv.rs @@ -159,3 +159,47 @@ consider adding `cargo-features = [\"public-dependency\"]` to the manifest ) .run() } + + +#[test] +fn pub_dev_dependency() { + Package::new("pub_dep", "0.1.0") + .file("src/lib.rs", "pub struct FromPub;") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["public-dependency"] + + [package] + name = "foo" + version = "0.0.1" + + [dev-dependencies] + pub_dep = {version = "0.1.0", public = true} + "#, + ) + .file( + "src/lib.rs", + " + extern crate pub_dep; + pub fn use_pub(_: pub_dep::FromPub) {} + ", + ) + .build(); + + p.cargo("build --message-format=short") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +error: failed to parse manifest at `[..]` + +Caused by: + 'public' specifier can only be used on regular dependencies, not Development dependencies +", + ) + .run() +} From 2de44c47ce6f25ec38847ee34d676903f66682dd Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 25 Apr 2019 23:24:32 -0400 Subject: [PATCH 10/11] Relax assertion --- src/cargo/core/dependency.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 36cbcfced48..0ac6716ea63 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -301,8 +301,10 @@ impl Dependency { /// Sets whether the dependency is public. pub fn set_public(&mut self, public: bool) -> &mut Dependency { - // Setting 'public' only makes sense for normal dependencies - assert_eq!(self.kind(), Kind::Normal); + if !public { + // Setting 'private' only makes sense for normal dependencies + assert_eq!(self.kind(), Kind::Normal); + } Rc::make_mut(&mut self.inner).public = public; self } From f4aac94f12b293d51064babdefd1c8c3e3d593b0 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 26 Apr 2019 00:07:04 -0400 Subject: [PATCH 11/11] Properly fix assertion and tests --- src/cargo/core/dependency.rs | 4 ++-- tests/testsuite/support/resolver.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 0ac6716ea63..f1a87e0a6f9 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -301,8 +301,8 @@ impl Dependency { /// Sets whether the dependency is public. pub fn set_public(&mut self, public: bool) -> &mut Dependency { - if !public { - // Setting 'private' only makes sense for normal dependencies + if public { + // Setting 'public' only makes sense for normal dependencies assert_eq!(self.kind(), Kind::Normal); } Rc::make_mut(&mut self.inner).public = public; diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs index 84bbf3d8317..fd5dc22db02 100644 --- a/tests/testsuite/support/resolver.rs +++ b/tests/testsuite/support/resolver.rs @@ -348,8 +348,8 @@ fn meta_test_deep_pretty_print_registry() { pkg!(("baz", "1.0.1")), pkg!(("cat", "1.0.2") => [dep_req_kind("other", "2", Kind::Build, false)]), pkg!(("cat", "1.0.3") => [dep_req_kind("other", "2", Kind::Development, false)]), - pkg!(("cat", "1.0.4") => [dep_req_kind("other", "2", Kind::Build, true)]), - pkg!(("cat", "1.0.5") => [dep_req_kind("other", "2", Kind::Development, true)]), + pkg!(("cat", "1.0.4") => [dep_req_kind("other", "2", Kind::Build, false)]), + pkg!(("cat", "1.0.5") => [dep_req_kind("other", "2", Kind::Development, false)]), pkg!(("dep_req", "1.0.0")), pkg!(("dep_req", "2.0.0")), ]) @@ -363,8 +363,8 @@ fn meta_test_deep_pretty_print_registry() { pkg!((\"baz\", \"1.0.1\")),\ pkg!((\"cat\", \"1.0.2\") => [dep_req_kind(\"other\", \"^2\", Kind::Build, false),]),\ pkg!((\"cat\", \"1.0.3\") => [dep_req_kind(\"other\", \"^2\", Kind::Development, false),]),\ - pkg!((\"cat\", \"1.0.4\") => [dep_req_kind(\"other\", \"^2\", Kind::Build, true),]),\ - pkg!((\"cat\", \"1.0.5\") => [dep_req_kind(\"other\", \"^2\", Kind::Development, true),]),\ + pkg!((\"cat\", \"1.0.4\") => [dep_req_kind(\"other\", \"^2\", Kind::Build, false),]),\ + pkg!((\"cat\", \"1.0.5\") => [dep_req_kind(\"other\", \"^2\", Kind::Development, false),]),\ pkg!((\"dep_req\", \"1.0.0\")),\ pkg!((\"dep_req\", \"2.0.0\")),]" )