Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve build dependencies independently [WIP] #7761

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,12 @@ fn compute_deps<'a, 'cfg>(

let bcx = state.bcx;
let id = unit.pkg.package_id();
let deps = state.resolve().deps(id).filter(|&(_id, deps)| {
let deps = if unit.target.is_custom_build() {
state.resolve().build_graph(id).map(|x| x.deps(id)).into_iter().flatten()
} else {
Some(state.resolve().deps(id)).into_iter().flatten()
};
let deps = deps.filter(|&(_id, deps)| {
assert!(!deps.is_empty());
deps.iter().any(|dep| {
// If this target is a build command, then we only want build
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,12 @@ impl Manifest {
}
}

pub fn run_dependencies(&self) -> impl Iterator<Item=&Dependency> {
self.summary.run_dependencies()
}
pub fn build_dependencies(&self) -> impl Iterator<Item=&Dependency> {
self.summary.build_dependencies()
}
pub fn dependencies(&self) -> &[Dependency] {
self.summary.dependencies()
}
Expand Down
10 changes: 9 additions & 1 deletion src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,15 @@ impl Package {
}
}

/// Gets the manifest dependencies.
/// Gets the manifest runtime (i.e. normal and development) dependencies.
pub fn run_dependencies(&self) -> impl Iterator<Item=&Dependency> {
self.manifest.run_dependencies()
}
/// Gets the manifest build dependencies.
pub fn build_dependencies(&self) -> impl Iterator<Item=&Dependency> {
self.manifest.build_dependencies()
}
/// Gets all dependencies.
pub fn dependencies(&self) -> &[Dependency] {
self.manifest.dependencies()
}
Expand Down
8 changes: 8 additions & 0 deletions src/cargo/core/resolver/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ pub struct Context {
/// a way to look up for a package in activations what packages required it
/// and all of the exact deps that it fulfilled.
pub parents: Graph<PackageId, Rc<Vec<Dependency>>>,

/// full resolution for build dependencies
pub build_graphs: im_rc::HashMap<PackageId, Resolve>,
}

/// When backtracking it can be useful to know how far back to go.
Expand Down Expand Up @@ -93,6 +96,7 @@ impl Context {
},
parents: Graph::new(),
activations: im_rc::HashMap::new(),
build_graphs: im_rc::HashMap::new(),
}
}

Expand Down Expand Up @@ -269,6 +273,10 @@ impl Context {
}
graph
}

pub fn build_graphs(&self) -> HashMap<PackageId, Resolve> {
self.build_graphs.iter().cloned().collect()
}
}

impl Graph<PackageId, Rc<Vec<Dependency>>> {
Expand Down
14 changes: 12 additions & 2 deletions src/cargo/core/resolver/dep_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ impl<'a> RegistryQueryer<'a> {
parent: Option<PackageId>,
candidate: &Summary,
opts: &ResolveOpts,
is_build: bool,
) -> ActivateResult<Rc<(HashSet<InternedString>, Rc<Vec<DepInfo>>)>> {
// if we have calculated a result before, then we can just return it,
// as it is a "pure" query of its arguments.
Expand All @@ -213,7 +214,7 @@ impl<'a> RegistryQueryer<'a> {
// First, figure out our set of dependencies based on the requested set
// of features. This also calculates what features we're going to enable
// for our own dependencies.
let (used_features, deps) = resolve_features(parent, candidate, opts)?;
let (used_features, deps) = resolve_features(parent, candidate, opts, is_build)?;

// Next, transform all dependencies into a list of possible candidates
// which can satisfy that dependency.
Expand Down Expand Up @@ -248,10 +249,19 @@ pub fn resolve_features<'b>(
parent: Option<PackageId>,
s: &'b Summary,
opts: &'b ResolveOpts,
is_build: bool,
) -> ActivateResult<(HashSet<InternedString>, Vec<(Dependency, FeaturesSet)>)> {
// First, filter by dev-dependencies.
let deps = s.dependencies();
let deps = deps.iter().filter(|d| d.is_transitive() || opts.dev_deps);
let deps = deps.iter().filter(|d| {
use crate::core::dependency::Kind;
match (d.kind(), is_build) {
(Kind::Build, _) => is_build,
(_, true) => false,
(Kind::Normal, false) => true,
(Kind::Development, false) => opts.dev_deps,
}
});

let reqs = build_requirements(s, opts)?;
let mut ret = Vec::new();
Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ impl EncodableResolve {
checksums,
metadata,
unused_patches,
HashMap::new(), // TODO how to serialize??
version,
))
}
Expand Down
40 changes: 34 additions & 6 deletions src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,27 @@ pub fn resolve(
config: Option<&Config>,
check_public_visible_dependencies: bool,
) -> CargoResult<Resolve> {
let cx = Context::new(check_public_visible_dependencies);
let _p = profile::start("resolving");
let minimal_versions = match config {
Some(config) => config.cli_unstable().minimal_versions,
None => false,
};
let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions);
let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
let resolve = resolve_deps(summaries, &mut registry, config, check_public_visible_dependencies, false)?;
trace!("resolved: {:?}", resolve);

Ok(resolve)
}

fn resolve_deps(
summaries: &[(Summary, ResolveOpts)],
registry: &mut RegistryQueryer<'_>,
config: Option<&Config>,
check_public_visible_dependencies: bool,
is_build: bool,
) -> CargoResult<Resolve> {
let cx = Context::new(check_public_visible_dependencies);
let cx = activate_deps_loop(cx, registry, summaries, config, is_build)?;

let mut cksums = HashMap::new();
for (summary, _) in cx.activations.values() {
Expand All @@ -151,12 +164,12 @@ pub fn resolve(
cksums,
BTreeMap::new(),
Vec::new(),
cx.build_graphs(),
ResolveVersion::default_for_new_lockfiles(),
);

check_cycles(&resolve)?;
check_duplicate_pkgs_in_lockfile(&resolve)?;
trace!("resolved: {:?}", resolve);

Ok(resolve)
}
Expand All @@ -171,6 +184,7 @@ fn activate_deps_loop(
registry: &mut RegistryQueryer<'_>,
summaries: &[(Summary, ResolveOpts)],
config: Option<&Config>,
is_build: bool,
) -> CargoResult<Context> {
let mut backtrack_stack = Vec::new();
let mut remaining_deps = RemainingDeps::new();
Expand All @@ -182,7 +196,7 @@ fn activate_deps_loop(
// Activate all the initial summaries to kick off some work.
for &(ref summary, ref opts) in summaries {
debug!("initial activation: {}", summary.package_id());
let res = activate(&mut cx, registry, None, summary.clone(), opts.clone());
let res = activate(&mut cx, registry, None, summary.clone(), opts.clone(), config.clone(), is_build);
match res {
Ok(Some((frame, _))) => remaining_deps.push(frame),
Ok(None) => (),
Expand Down Expand Up @@ -379,7 +393,7 @@ fn activate_deps_loop(
dep.package_name(),
candidate.version()
);
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, opts);
let res = activate(&mut cx, registry, Some((&parent, &dep)), candidate, opts, config, false);

let successfully_activated = match res {
// Success! We've now activated our `candidate` in our context
Expand Down Expand Up @@ -592,6 +606,8 @@ fn activate(
parent: Option<(&Summary, &Dependency)>,
candidate: Summary,
opts: ResolveOpts,
config: Option<&Config>,
is_build: bool,
) -> ActivateResult<Option<(DepsFrame, Duration)>> {
let candidate_pid = candidate.package_id();
cx.age += 1;
Expand All @@ -616,6 +632,18 @@ fn activate(

let activated = cx.flag_activated(&candidate, &opts, parent)?;

if !is_build && candidate.build_dependencies().count() > 0 {
// resolve build dependencies
let build_graph = resolve_deps(
&[(candidate.clone(), opts.clone())],
registry,
config,
cx.public_dependency.is_some(),
true,
)?;
cx.build_graphs.insert(candidate_pid, build_graph);
}

let candidate = match registry.replacement_summary(candidate_pid) {
Some(replace) => {
// Note the `None` for parent here since `[replace]` is a bit wonky
Expand Down Expand Up @@ -644,7 +672,7 @@ fn activate(

let now = Instant::now();
let (used_features, deps) =
&*registry.build_deps(parent.map(|p| p.0.package_id()), &candidate, &opts)?;
&*registry.build_deps(parent.map(|p| p.0.package_id()), &candidate, &opts, is_build)?;

// Record what list of features is active for this package.
if !used_features.is_empty() {
Expand Down
11 changes: 10 additions & 1 deletion src/cargo/core/resolver/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use super::encode::Metadata;
///
/// Each instance of `Resolve` also understands the full set of features used
/// for each package.
#[derive(Clone)]
pub struct Resolve {
/// A graph, whose vertices are packages and edges are dependency specifications
/// from `Cargo.toml`. We need a `Vec` here because the same package
Expand Down Expand Up @@ -47,6 +48,8 @@ pub struct Resolve {
unused_patches: Vec<PackageId>,
/// A map from packages to a set of their public dependencies
public_dependencies: HashMap<PackageId, HashSet<PackageId>>,
/// A map from packages to their build dependency graphs
build_graphs: HashMap<PackageId, Resolve>,
/// Version of the `Cargo.lock` format, see
/// `cargo::core::resolver::encode` for more.
version: ResolveVersion,
Expand Down Expand Up @@ -77,6 +80,7 @@ impl Resolve {
checksums: HashMap<PackageId, Option<String>>,
metadata: Metadata,
unused_patches: Vec<PackageId>,
build_graphs: HashMap<PackageId, Resolve>,
version: ResolveVersion,
) -> Resolve {
let reverse_replacements = replacements.iter().map(|(&p, &r)| (r, p)).collect();
Expand Down Expand Up @@ -106,6 +110,7 @@ impl Resolve {
empty_features: HashSet::new(),
reverse_replacements,
public_dependencies,
build_graphs,
version,
}
}
Expand Down Expand Up @@ -277,6 +282,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated
.map(|(id, deps)| (*id, deps.as_slice()))
}

pub fn build_graph(&self, pkg: PackageId) -> Option<&Resolve> {
self.build_graphs.get(&pkg)
}

pub fn replacement(&self, pkg: PackageId) -> Option<PackageId> {
self.replacements.get(&pkg).cloned()
}
Expand Down Expand Up @@ -388,7 +397,7 @@ impl PartialEq for Resolve {
compare! {
// fields to compare
graph replacements reverse_replacements empty_features features
checksums metadata unused_patches public_dependencies
checksums metadata unused_patches public_dependencies build_graphs
|
// fields to ignore
version
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ impl Summary {
pub fn source_id(&self) -> SourceId {
self.package_id().source_id()
}
pub fn run_dependencies(&self) -> impl Iterator<Item=&Dependency> {
self.inner.dependencies.iter().filter(|dep| !dep.is_build())
}
pub fn build_dependencies(&self) -> impl Iterator<Item=&Dependency> {
self.inner.dependencies.iter().filter(|dep| dep.is_build())
}
pub fn dependencies(&self) -> &[Dependency] {
&self.inner.dependencies
}
Expand Down
76 changes: 76 additions & 0 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3958,3 +3958,79 @@ Caused by:
.run();
fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).unwrap();
}

#[cargo_test]
fn links_duplicated_across_build_deps() {
let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.5.0"
authors = []

[dependencies.a]
path = "a"
[dependencies.b]
path = "b"
"#,
)
.file("src/lib.rs", "")
.file(
"a/Cargo.toml",
r#"
[project]
name = "a"
version = "0.5.0"
authors = []

[build-dependencies.bar-sys]
path = "../bar-sys/1"
"#,
)
.file("a/src/lib.rs", "")
.file(
"b/Cargo.toml",
r#"
[project]
name = "b"
version = "0.5.0"
authors = []

[build-dependencies.bar-sys]
path = "../bar-sys/2"
"#,
)
.file("b/src/lib.rs", "")
.file(
"bar-sys/1/Cargo.toml",
r#"
[project]
name = "bar-sys"
version = "0.1.0"
authors = []
links = "bar"
build = "build.rs"
"#,
)
.file("bar-sys/1/src/lib.rs", "")
.file("bar-sys/1/build.rs", "")
.file(
"bar-sys/2/Cargo.toml",
r#"
[project]
name = "bar-sys"
version = "0.2.0"
authors = []
links = "bar"
build = "build.rs"
"#,
)
.file("bar-sys/2/src/lib.rs", "")
.file("bar-sys/2/build.rs", "")
.build();

p.cargo("build")
.run();
}