Skip to content

Commit

Permalink
Fix panic with plugins and build commands
Browse files Browse the repository at this point in the history
This fixes a bug in cargo where target executables and libraries would be linked
to plugin native dependencies (not wanted!).

Closes rust-lang#885
  • Loading branch information
alexcrichton committed Nov 17, 2014
1 parent 0202646 commit 71a89ef
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 22 deletions.
4 changes: 4 additions & 0 deletions src/cargo/core/package.rs
Expand Up @@ -110,6 +110,10 @@ impl Package {
pub fn get_absolute_target_dir(&self) -> Path {
self.get_root().join(self.get_target_dir())
}

pub fn has_custom_build(&self) -> bool {
self.get_targets().iter().any(|t| t.get_profile().is_custom_build())
}
}

impl Show for Package {
Expand Down
17 changes: 17 additions & 0 deletions src/cargo/ops/cargo_rustc/context.rs
Expand Up @@ -326,4 +326,21 @@ impl PlatformRequirement {
_ => PlatformPluginAndTarget,
}
}

pub fn includes(self, kind: Kind) -> bool {
match (self, kind) {
(PlatformPluginAndTarget, _) |
(PlatformTarget, KindTarget) |
(PlatformPlugin, KindHost) => true,
_ => false,
}
}

pub fn each_kind(self, f: |Kind|) {
match self {
PlatformTarget => f(KindTarget),
PlatformPlugin => f(KindHost),
PlatformPluginAndTarget => { f(KindTarget); f(KindHost); }
}
}
}
46 changes: 24 additions & 22 deletions src/cargo/ops/cargo_rustc/mod.rs
Expand Up @@ -390,30 +390,38 @@ fn rustc(package: &Package, target: &Target,
let build_state = cx.build_state.clone();
let mut native_lib_deps = HashSet::new();
let current_id = package.get_package_id().clone();
let has_custom_build = package.get_targets().iter().any(|t| {
t.get_profile().is_custom_build()
});

if has_custom_build && !target.get_profile().is_custom_build() {
if package.has_custom_build() && !target.get_profile().is_custom_build() {
native_lib_deps.insert(current_id.clone());
}
// Visit dependencies transitively to figure out what our native
// dependencies are (for -L and -l flags).
for &(pkg, _) in cx.dep_targets(package, target).iter() {
each_dep(pkg, cx, |dep| {
let has_custom_build = dep.get_targets().iter().any(|t| {
t.get_profile().is_custom_build()
});
if has_custom_build {
native_lib_deps.insert(dep.get_package_id().clone());
//
// Be sure to only look at targets of the same Kind, however, as we
// don't want to include native libs of plugins for targets for example.
fn visit<'a>(cx: &'a Context, pkg: &'a Package, target: &Target,
kind: Kind,
visiting: &mut HashSet<&'a PackageId>,
libs: &mut HashSet<PackageId>) {
for &(pkg, target) in cx.dep_targets(pkg, target).iter() {
let req = cx.get_requirement(pkg, target);
if !req.includes(kind) { continue }
if !visiting.insert(pkg.get_package_id()) { continue }

if pkg.has_custom_build() {
libs.insert(pkg.get_package_id().clone());
}
});
visit(cx, pkg, target, kind, visiting, libs);
visiting.remove(&pkg.get_package_id());
}
}
visit(cx, package, target, kind,
&mut HashSet::new(), &mut native_lib_deps);
let mut native_lib_deps = native_lib_deps.into_iter().collect::<Vec<_>>();
native_lib_deps.sort();

// If we are a binary and the package also contains a library, then we don't
// pass the `-l` flags.
// If we are a binary and the package also contains a library, then we
// don't pass the `-l` flags.
let pass_l_flag = target.is_lib() || !package.get_targets().iter().any(|t| {
t.is_lib()
});
Expand Down Expand Up @@ -503,10 +511,7 @@ fn rustdoc(package: &Package, target: &Target,

let mut rustdoc = try!(build_deps_args(rustdoc, target, package, cx, kind));

let has_build_cmd = package.get_targets().iter().any(|t| {
t.get_profile().is_custom_build()
});
rustdoc = rustdoc.env("OUT_DIR", if has_build_cmd {
rustdoc = rustdoc.env("OUT_DIR", if package.has_custom_build() {
Some(cx.layout(package, kind).build_out(package))
} else {
None
Expand Down Expand Up @@ -663,10 +668,7 @@ fn build_deps_args(mut cmd: ProcessBuilder, target: &Target, package: &Package,
cmd = cmd.arg("-L").arg(layout.root());
cmd = cmd.arg("-L").arg(layout.deps());

let has_build_cmd = package.get_targets().iter().any(|t| {
t.get_profile().is_custom_build()
});
cmd = cmd.env("OUT_DIR", if has_build_cmd {
cmd = cmd.env("OUT_DIR", if package.has_custom_build() {
Some(layout.build_out(package))
} else {
None
Expand Down
42 changes: 42 additions & 0 deletions tests/test_cargo_compile_custom_build.rs
Expand Up @@ -897,3 +897,45 @@ test!(shared_dep_with_a_build_script {
assert_that(p.cargo_process("build"),
execs().with_status(0));
})

test!(transitive_dep_host {
let p = project("foo")
.file("Cargo.toml", r#"
[project]
name = "foo"
version = "0.5.0"
authors = []
build = "build.rs"
[build-dependencies.b]
path = "b"
"#)
.file("src/lib.rs", "")
.file("build.rs", "fn main() {}")
.file("a/Cargo.toml", r#"
[package]
name = "a"
version = "0.5.0"
authors = []
links = "foo"
build = "build.rs"
"#)
.file("a/build.rs", "fn main() {}")
.file("a/src/lib.rs", "")
.file("b/Cargo.toml", r#"
[package]
name = "b"
version = "0.5.0"
authors = []
[lib]
name = "b"
plugin = true
[dependencies.a]
path = "../a"
"#)
.file("b/src/lib.rs", "");
assert_that(p.cargo_process("build"),
execs().with_status(0));
})

1 comment on commit 71a89ef

@brson
Copy link

@brson brson commented on 71a89ef Nov 18, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

Please sign in to comment.