Skip to content

Commit

Permalink
Fix custom build with links dependency causing issues.
Browse files Browse the repository at this point in the history
- Revert previous change that caused infinite recursion (run-custom-build
  depending on itself).
- Avoid poisoning the the `deps` map with incorrect entries with ProfileFor
  set incorrectly.
  • Loading branch information
ehuss committed Jul 12, 2018
1 parent a5617df commit df8118d
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 10 deletions.
24 changes: 15 additions & 9 deletions src/cargo/core/compiler/context/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn deps_of<'a, 'b, 'cfg>(
// requested unit's settings are the same as `Any`, `CustomBuild` can't
// affect anything else in the hierarchy.
if !deps.contains_key(unit) {
let unit_deps = compute_deps(unit, bcx, deps, profile_for)?;
let unit_deps = compute_deps(unit, bcx, profile_for)?;
let to_insert: Vec<_> = unit_deps.iter().map(|&(unit, _)| unit).collect();
deps.insert(*unit, to_insert);
for (unit, profile_for) in unit_deps {
Expand All @@ -75,11 +75,10 @@ fn deps_of<'a, 'b, 'cfg>(
fn compute_deps<'a, 'b, 'cfg>(
unit: &Unit<'a>,
bcx: &BuildContext<'a, 'cfg>,
deps: &'b mut HashMap<Unit<'a>, Vec<Unit<'a>>>,
profile_for: ProfileFor,
) -> CargoResult<Vec<(Unit<'a>, ProfileFor)>> {
if unit.mode.is_run_custom_build() {
return compute_deps_custom_build(unit, bcx, deps);
return compute_deps_custom_build(unit, bcx);
} else if unit.mode.is_doc() && !unit.mode.is_any_test() {
// Note: This does not include Doctest.
return compute_deps_doc(unit, bcx);
Expand Down Expand Up @@ -192,20 +191,27 @@ fn compute_deps<'a, 'b, 'cfg>(
fn compute_deps_custom_build<'a, 'cfg>(
unit: &Unit<'a>,
bcx: &BuildContext<'a, 'cfg>,
deps: &mut HashMap<Unit<'a>, Vec<Unit<'a>>>,
) -> CargoResult<Vec<(Unit<'a>, ProfileFor)>> {
// When not overridden, then the dependencies to run a build script are:
//
// 1. Compiling the build script itself
// 2. For each immediate dependency of our package which has a `links`
// key, the execution of that build script.
let deps = deps
let not_custom_build = unit.pkg
.targets()
.iter()
.find(|(key, _deps)| key.pkg == unit.pkg && !key.target.is_custom_build())
.expect("can't find package deps")
.1;
.find(|t| !t.is_custom_build())
.unwrap();
let tmp = Unit {
pkg: unit.pkg,
target: not_custom_build,
profile: unit.profile,
kind: unit.kind,
mode: CompileMode::Build,
};
let deps = compute_deps(&tmp, bcx, ProfileFor::Any)?;
Ok(deps.iter()
.filter_map(|unit| {
.filter_map(|(unit, _)| {
if !unit.target.linkable() || unit.pkg.manifest().links().is_none() {
return None;
}
Expand Down
5 changes: 4 additions & 1 deletion tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3080,9 +3080,12 @@ fn panic_abort_with_build_scripts() {
execs().with_status(0),
);

p.root().join("target").rm_rf();

assert_that(
p.cargo("test --release"),
p.cargo("test --release -v"),
execs().with_status(0)
.with_stderr_does_not_contain("[..]panic[..]")
);
}

Expand Down
25 changes: 25 additions & 0 deletions tests/testsuite/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4138,3 +4138,28 @@ fn json_artifact_includes_test_flag() {
),
);
}

#[test]
fn test_build_script_links() {
let p = project("foo")
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
links = 'something'
[lib]
test = false
"#,
)
.file("build.rs", "fn main() {}")
.file("src/lib.rs", "")
.build();

assert_that(
p.cargo("test --no-run"),
execs().with_status(0),
);
}

0 comments on commit df8118d

Please sign in to comment.