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

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](https://github.com/rust-lang/rfcs/blob/master/text/1977-public-private-dependencies.md). 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.
This commit is contained in:
bors 2019-04-26 22:47:59 +00:00
commit 7b13469ee9
15 changed files with 335 additions and 15 deletions

View file

@ -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 {

View file

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

View file

@ -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() {
@ -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(())
}

View file

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

View file

@ -199,6 +199,9 @@ features! {
// Declarative build scripts.
[unstable] metabuild: bool,
// Specifying the 'public' attribute on dependencies
[unstable] public_dependency: bool,
}
}

View file

@ -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();

View file

@ -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 {

View file

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

View file

@ -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 {

View file

@ -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)
}

View file

@ -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)
}
}

View file

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

View file

@ -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;

205
tests/testsuite/pub_priv.rs Normal file
View file

@ -0,0 +1,205 @@
use crate::support::registry::Package;
use crate::support::{is_nightly, project};
#[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
See https://doc.rust-lang.org/book/appendix-07-nightly-rust.html for more information about Rust release channels.
"
)
.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()
}
#[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()
}

View file

@ -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\")),]"
)