rust
/
cargo
mirror of https://github.com/rust-lang/cargo
0
0
Fork 0
Code Issues Releases Wiki Activity

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.
This commit is contained in:
Aaron Hill 2019-03-20 19:25:59 -04:00
parent c9330feb48
commit 87f1a4b217
No known key found for this signature in database
GPG Key ID: B4087E510E98B164
11 changed files with 267 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/cargo/core/compiler/build_context/mod.rs
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 {

20
src/cargo/core/compiler/mod.rs
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() {
@ -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(())
}

3
src/cargo/core/features.rs
View File

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

25
src/cargo/core/resolver/resolve.rs
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();

13
src/cargo/core/workspace.rs
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 {

11
src/cargo/ops/cargo_package.rs
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,
},

3
src/cargo/ops/resolve.rs
View File

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

8
src/cargo/sources/registry/mod.rs
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)
}

6
src/cargo/util/toml/mod.rs
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,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)
}
}

1
tests/testsuite/main.rs
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;

180
tests/testsuite/pub_priv.rs Normal file
View File

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