Auto merge of #6772 - Aaron1011:feature/final-pub-priv, r=ehuss

Implement the 'frontend' of public-private dependencies

This is part of rust-lang/rust#44663

This implements the 'frontend' portion of [RFC 1977](https://github.com/rust-lang/rfcs/blob/master/text/1977-public-private-dependencies.md). Once PRs rust-lang/rust#59335 and rust-lang/crates.io#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.

Author: bors

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 {

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<Fingerprint>,
 }
 
@@ -420,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)
     }
 }
 
@@ -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()
@@ -845,11 +848,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));
         }
@@ -895,6 +900,7 @@ impl DepFingerprint {
     ) -> CargoResult<DepFingerprint> {
         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
@@ -915,6 +921,7 @@ impl DepFingerprint {
         Ok(DepFingerprint {
             pkg_id,
             name,
+            public,
             fingerprint,
         })
     }

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,22 +967,32 @@ 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>(
         cmd: &mut ProcessBuilder,
         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() {
@@ -995,7 +1006,17 @@ 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);
+
+            if current.pkg.manifest().features().require(Feature::public_dependency()).is_ok() &&
+                !bcx.is_public_dependency(current, dep)  {
+
+                    cmd.arg("--extern-private");
+                    *need_unstable_opts = true;
+            } else {
+                cmd.arg("--extern");
+            }
+
+            cmd.arg(&v);
         }
         Ok(())
     }

src/cargo/core/dependency.rs

@@ -301,6 +301,10 @@ impl Dependency {
 
     /// Sets whether the dependency is public.
     pub fn set_public(&mut self, public: bool) -> &mut Dependency {
+        if public {
+            // Setting 'public' only makes sense for normal dependencies
+            assert_eq!(self.kind(), Kind::Normal);
+        }
         Rc::make_mut(&mut self.inner).public = public;
         self
     }

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,
     }
 }
 

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<PackageId, Option<String>>,
     metadata: Metadata,
     unused_patches: Vec<PackageId>,
+    // A map from packages to a set of their public dependencies
+    public_dependencies: HashMap<PackageId, HashSet<PackageId>>,
 }
 
 impl Resolve {
@@ -41,6 +44,21 @@ impl Resolve {
         unused_patches: Vec<PackageId>,
     ) -> 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<PackageId> = deps.iter().find(|d| d.kind() == Kind::Normal).and_then(|d| {
+                    if d.is_public() {
+                        Some(dep_package.clone())
+                    } else {
+                        None
+                    }
+                });
+                id_opt
+            }).collect::<HashSet<PackageId>>();
+
+            (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();

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,17 +541,21 @@ 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.
     /// 2. All workspace members agree on this one root as the root.
     /// 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 {

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<dyn Executor> = 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,
         },

src/cargo/ops/resolve.rs

@@ -3,6 +3,7 @@ use std::rc::Rc;
 
 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};
@@ -331,7 +332,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 {

src/cargo/sources/registry/mod.rs

@@ -285,6 +285,7 @@ struct RegistryDependency<'a> {
     kind: Option<Cow<'a, str>>,
     registry: Option<Cow<'a, str>>,
     package: Option<Cow<'a, str>>,
+    public: Option<bool>
 }
 
 impl<'a> RegistryDependency<'a> {
@@ -300,6 +301,7 @@ impl<'a> RegistryDependency<'a> {
             kind,
             registry,
             package,
+            public
         } = self;
 
         let id = if let Some(registry) = &registry {
@@ -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)
     }

src/cargo/util/toml/mod.rs

@@ -239,6 +239,7 @@ pub struct DetailedTomlDependency {
     #[serde(rename = "default_features")]
     default_features2: Option<bool>,
     package: Option<String>,
+    public: Option<bool>
 }
 
 #[derive(Debug, Deserialize, Serialize)]
@@ -1461,6 +1462,16 @@ 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())?;
+
+            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)
     }
 }

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
+```

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;