From f0c0a6aaeea727d9bc8526e6a415b6ce9e4e0c26 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 13 Dec 2017 20:54:01 -0800 Subject: [PATCH] Fix renaming a project using build scripts This commit fixes an issue in Cargo where if a project's folder was renamed but it also contained a build script the project could break. Cargo would continue to use the previous `rustc-link-search` arguments to configure env vars like `LD_LIBRARY_PATH` but the literal values from the previous compilation would be stale as the directories would no longer be there. To fix this when parsing the build script output we now retain a log of the previous output directory of a build script invocation as well as the current output, tweaking paths as appropriate if they were contained in the output folder. Closes #4053 --- src/cargo/ops/cargo_rustc/custom_build.rs | 68 ++++++++++---- src/cargo/ops/cargo_rustc/mod.rs | 4 +- tests/build-script.rs | 105 ++++++++++++++++++++++ 3 files changed, 159 insertions(+), 18 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index ee51b9b3bd0..0ef0fa21882 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -7,7 +7,7 @@ use std::sync::{Mutex, Arc}; use core::PackageId; use util::{Freshness, Cfg}; use util::errors::{CargoResult, CargoResultExt, CargoError}; -use util::{internal, profile, paths}; +use util::{self, internal, profile, paths}; use util::machine_message; use super::job::Work; @@ -27,7 +27,7 @@ pub struct BuildOutput { /// Metadata to pass to the immediate dependencies pub metadata: Vec<(String, String)>, /// Paths to trigger a rerun of this build script. - pub rerun_if_changed: Vec, + pub rerun_if_changed: Vec, /// Environment variables which, when changed, will cause a rebuild. pub rerun_if_env_changed: Vec, /// Warnings generated by this build, @@ -65,7 +65,7 @@ pub struct BuildScripts { pub struct BuildDeps { pub build_script_output: PathBuf, - pub rerun_if_changed: Vec, + pub rerun_if_changed: Vec, pub rerun_if_env_changed: Vec, } @@ -178,29 +178,37 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) let pkg_name = unit.pkg.to_string(); let build_state = Arc::clone(&cx.build_state); let id = unit.pkg.package_id().clone(); - let (output_file, err_file) = { + let (output_file, err_file, root_output_file) = { let build_output_parent = build_output.parent().unwrap(); let output_file = build_output_parent.join("output"); let err_file = build_output_parent.join("stderr"); - (output_file, err_file) + let root_output_file = build_output_parent.join("root-output"); + (output_file, err_file, root_output_file) }; + let root_output = cx.target_root().to_path_buf(); let all = (id.clone(), pkg_name.clone(), Arc::clone(&build_state), - output_file.clone()); + output_file.clone(), root_output.clone()); let build_scripts = super::load_build_deps(cx, unit); let kind = unit.kind; let json_messages = cx.build_config.json_messages; // Check to see if the build script has already run, and if it has keep // track of whether it has told us about some explicit dependencies - let prev_output = BuildOutput::parse_file(&output_file, &pkg_name).ok(); + let prev_root_output = paths::read_bytes(&root_output_file) + .and_then(|bytes| util::bytes2path(&bytes)) + .unwrap_or(cmd.get_cwd().unwrap().to_path_buf()); + let prev_output = BuildOutput::parse_file( + &output_file, + &pkg_name, + &prev_root_output, + &root_output, + ).ok(); let deps = BuildDeps::new(&output_file, prev_output.as_ref()); cx.build_explicit_deps.insert(*unit, deps); fs::create_dir_all(&script_output)?; fs::create_dir_all(&build_output)?; - let root_output = cx.target_root().to_path_buf(); - // Prepare the unit of "dirty work" which will actually run the custom build // command. // @@ -266,7 +274,13 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // well. paths::write(&output_file, &output.stdout)?; paths::write(&err_file, &output.stderr)?; - let parsed_output = BuildOutput::parse(&output.stdout, &pkg_name)?; + paths::write(&root_output_file, &util::path2bytes(&root_output)?)?; + let parsed_output = BuildOutput::parse( + &output.stdout, + &pkg_name, + &root_output, + &root_output, + )?; if json_messages { let library_paths = parsed_output.library_paths.iter().map(|l| { @@ -289,10 +303,17 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) // itself to run when we actually end up just discarding what we calculated // above. let fresh = Work::new(move |_tx| { - let (id, pkg_name, build_state, output_file) = all; + let (id, pkg_name, build_state, output_file, root_output) = all; let output = match prev_output { Some(output) => output, - None => BuildOutput::parse_file(&output_file, &pkg_name)?, + None => { + BuildOutput::parse_file( + &output_file, + &pkg_name, + &prev_root_output, + &root_output, + )? + } }; build_state.insert(id, kind, output); Ok(()) @@ -321,14 +342,20 @@ impl BuildState { } impl BuildOutput { - pub fn parse_file(path: &Path, pkg_name: &str) -> CargoResult { + pub fn parse_file(path: &Path, + pkg_name: &str, + root_output_when_generated: &Path, + root_output: &Path) -> CargoResult { let contents = paths::read_bytes(path)?; - BuildOutput::parse(&contents, pkg_name) + BuildOutput::parse(&contents, pkg_name, root_output_when_generated, root_output) } // Parses the output of a script. // The `pkg_name` is used for error messages. - pub fn parse(input: &[u8], pkg_name: &str) -> CargoResult { + pub fn parse(input: &[u8], + pkg_name: &str, + root_output_when_generated: &Path, + root_output: &Path) -> CargoResult { let mut library_paths = Vec::new(); let mut library_links = Vec::new(); let mut cfgs = Vec::new(); @@ -364,6 +391,13 @@ impl BuildOutput { _ => bail!("Wrong output in {}: `{}`", whence, line), }; + let path = |val: &str| { + match Path::new(val).strip_prefix(root_output_when_generated) { + Ok(path) => root_output.join(path), + Err(_) => PathBuf::from(val), + } + }; + match key { "rustc-flags" => { let (paths, links) = @@ -372,11 +406,11 @@ impl BuildOutput { library_paths.extend(paths.into_iter()); } "rustc-link-lib" => library_links.push(value.to_string()), - "rustc-link-search" => library_paths.push(PathBuf::from(value)), + "rustc-link-search" => library_paths.push(path(value)), "rustc-cfg" => cfgs.push(value.to_string()), "rustc-env" => env.push(BuildOutput::parse_rustc_env(value, &whence)?), "warning" => warnings.push(value.to_string()), - "rerun-if-changed" => rerun_if_changed.push(value.to_string()), + "rerun-if-changed" => rerun_if_changed.push(path(value)), "rerun-if-env-changed" => rerun_if_env_changed.push(value.to_string()), _ => metadata.push((key.to_string(), value.to_string())), } diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 0d4aac38a94..6664ab6bcdf 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -551,7 +551,9 @@ fn link_targets<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, #[cfg(windows)] use std::os::windows::fs::symlink_dir as symlink; - symlink(src, dst) + let dst_dir = dst.parent().unwrap(); + assert!(src.starts_with(dst_dir)); + symlink(src.strip_prefix(dst_dir).unwrap(), dst) } else { fs::hard_link(src, dst) }; diff --git a/tests/build-script.rs b/tests/build-script.rs index 145db690277..32ff94e9f98 100644 --- a/tests/build-script.rs +++ b/tests/build-script.rs @@ -1,8 +1,10 @@ extern crate cargotest; extern crate hamcrest; +use std::env; use std::fs::{self, File}; use std::io::prelude::*; +use std::path::PathBuf; use cargotest::{rustc_host, sleep_ms}; use cargotest::support::{project, execs}; @@ -2794,3 +2796,106 @@ package `a v0.5.0 (file://[..])` also links to native library `a` ")); } + +#[test] +fn rename_with_link_search_path() { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.5.0" + authors = [] + + [lib] + crate-type = ["cdylib"] + "#) + .file("src/lib.rs", " + #[no_mangle] + pub extern fn cargo_test_foo() {} + "); + let p = p.build(); + + assert_that(p.cargo("build"), execs().with_status(0)); + + let p2 = project("bar") + .file("Cargo.toml", r#" + [project] + name = "bar" + version = "0.5.0" + authors = [] + "#) + .file("build.rs", r#" + use std::env; + use std::fs; + use std::path::PathBuf; + + fn main() { + // Move the `libfoo.so` from the root of our project into the + // build directory. This way Cargo should automatically manage + // `LD_LIBRARY_PATH` and such. + let root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()); + let file = format!("{}foo{}", env::consts::DLL_PREFIX, env::consts::DLL_SUFFIX); + let src = root.join(&file); + + let dst_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap()); + let dst = dst_dir.join(&file); + + fs::copy(&src, &dst).unwrap(); + // handle windows, like below + drop(fs::copy(root.join("foo.dll.lib"), dst_dir.join("foo.dll.lib"))); + + println!("cargo:rerun-if-changed=build.rs"); + if cfg!(target_env = "msvc") { + println!("cargo:rustc-link-lib=foo.dll"); + } else { + println!("cargo:rustc-link-lib=foo"); + } + println!("cargo:rustc-link-search={}", + dst.parent().unwrap().display()); + } + "#) + .file("src/main.rs", r#" + extern { + #[link_name = "cargo_test_foo"] + fn foo(); + } + + fn main() { + unsafe { foo(); } + } + "#); + let p2 = p2.build(); + + // Move the output `libfoo.so` into the directory of `p2`, and then delete + // the `p` project. On OSX the `libfoo.dylib` artifact references the + // original path in `p` so we want to make sure that it can't find it (hence + // the deletion). + let root = PathBuf::from(p.root()); + let root = root.join("target").join("debug").join("deps"); + let file = format!("{}foo{}", env::consts::DLL_PREFIX, env::consts::DLL_SUFFIX); + let src = root.join(&file); + + let dst = p2.root().join(&file); + + fs::copy(&src, &dst).unwrap(); + // copy the import library for windows, if it exists + drop(fs::copy(&root.join("foo.dll.lib"), p2.root().join("foo.dll.lib"))); + fs::remove_dir_all(p.root()).unwrap(); + + // Everything should work the first time + assert_that(p2.cargo("run"), + execs().with_status(0)); + + // Now rename the root directory and rerun `cargo run`. Not only should we + // not build anything but we also shouldn't crash. + let mut new = p2.root(); + new.pop(); + new.push("bar2"); + fs::rename(p2.root(), &new).unwrap(); + assert_that(p2.cargo("run").cwd(&new), + execs().with_status(0) + .with_stderr("\ +[FINISHED] [..] +[RUNNING] [..] +")); +}