Auto merge of #4818 - alexcrichton:rename-link-paths, r=matklad

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

@@ -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<String>,
+    pub rerun_if_changed: Vec<PathBuf>,
     /// Environment variables which, when changed, will cause a rebuild.
     pub rerun_if_env_changed: Vec<String>,
     /// 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<String>,
+    pub rerun_if_changed: Vec<PathBuf>,
     pub rerun_if_env_changed: Vec<String>,
 }
 
@@ -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<BuildOutput> {
+    pub fn parse_file(path: &Path,
+                      pkg_name: &str,
+                      root_output_when_generated: &Path,
+                      root_output: &Path) -> CargoResult<BuildOutput> {
         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<BuildOutput> {
+    pub fn parse(input: &[u8],
+                 pkg_name: &str,
+                 root_output_when_generated: &Path,
+                 root_output: &Path) -> CargoResult<BuildOutput> {
         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())),
             }

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)
             };

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] [..]
+"));
+}