Auto merge of #6328 - ehuss:rename-cross-build-output, r=alexcrichton

Fix renaming directory project using build scripts with cross-compiling.

The rename-protection introduced in #4818 checks for paths with a prefix of `/…/target/debug`, but this does not work for paths used during cross-compiling.

This change checks for paths with the full `OUT_DIR` prefix, and the `build/PKG/root-output` file now includes that full `OUT_DIR` instead of `/…/target/debug`.

This also includes a few other changes:
- Support fixing KIND=PATH style paths.
- Support fixing paths in metadata (like `cargo:root=…` or `cargo:include=…` which are common).
- Adds a "version" value to the metadata hash to ensure that cargo doesn't get confused with the new `root-output` file format.

Fixes #6177

src/cargo/core/compiler/context/compilation_files.rs

@@ -402,6 +402,15 @@ fn compute_metadata<'a, 'cfg>(
 
     let mut hasher = SipHasher::new_with_keys(0, 0);
 
+    // This is a generic version number that can be changed to make
+    // backwards-incompatible changes to any file structures in the output
+    // directory. For example, the fingerprint files or the build-script
+    // output files. Normally cargo updates ship with rustc updates which will
+    // cause a new hash due to the rustc version changing, but this allows
+    // cargo to be extra careful to deal with different versions of cargo that
+    // use the same rustc version.
+    1.hash(&mut hasher);
+
     // Unique metadata per (name, source, version) triple. This'll allow us
     // to pull crates from anywhere w/o worrying about conflicts
     unit.pkg

src/cargo/core/compiler/custom_build.rs

@@ -28,6 +28,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.
+    /// May be absolute or relative paths (relative to package root).
     pub rerun_if_changed: Vec<PathBuf>,
     /// Environment variables which, when changed, will cause a rebuild.
     pub rerun_if_env_changed: Vec<String>,
@@ -129,8 +130,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
         .iter()
         .find(|d| !d.mode.is_run_custom_build() && d.target.is_custom_build())
         .expect("running a script not depending on an actual script");
-    let script_output = cx.files().build_script_dir(build_script_unit);
-    let build_output = cx.files().build_script_out_dir(unit);
+    let script_dir = cx.files().build_script_dir(build_script_unit);
+    let script_out_dir = cx.files().build_script_out_dir(unit);
     let build_plan = bcx.build_config.build_plan;
     let invocation_name = unit.buildkey();
 
@@ -139,7 +140,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
     }
 
     // Building the command to execute
-    let to_exec = script_output.join(unit.target.name());
+    let to_exec = script_dir.join(unit.target.name());
 
     // Start preparing the process to execute, starting out with some
     // environment variables. Note that the profile-related environment
@@ -151,7 +152,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
     let to_exec = to_exec.into_os_string();
     let mut cmd = cx.compilation.host_process(to_exec, unit.pkg)?;
     let debug = unit.profile.debuginfo.unwrap_or(0) != 0;
-    cmd.env("OUT_DIR", &build_output)
+    cmd.env("OUT_DIR", &script_out_dir)
         .env("CARGO_MANIFEST_DIR", unit.pkg.root())
         .env("NUM_JOBS", &bcx.jobs().to_string())
         .env(
@@ -241,19 +242,19 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
     let build_state = Arc::clone(&cx.build_state);
     let id = unit.pkg.package_id().clone();
     let (output_file, err_file, root_output_file) = {
-        let build_output_parent = build_output.parent().unwrap();
+        let build_output_parent = script_out_dir.parent().unwrap();
         let output_file = build_output_parent.join("output");
         let err_file = build_output_parent.join("stderr");
         let root_output_file = build_output_parent.join("root-output");
         (output_file, err_file, root_output_file)
     };
-    let root_output = cx.files().target_root().to_path_buf();
+    let host_target_root = cx.files().target_root().to_path_buf();
     let all = (
         id.clone(),
         pkg_name.clone(),
         Arc::clone(&build_state),
         output_file.clone(),
-        root_output.clone(),
+        script_out_dir.clone(),
     );
     let build_scripts = super::load_build_deps(cx, unit);
     let kind = unit.kind;
@@ -262,16 +263,17 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
 
     // 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_root_output = paths::read_bytes(&root_output_file)
+    let prev_script_out_dir = paths::read_bytes(&root_output_file)
         .and_then(|bytes| util::bytes2path(&bytes))
-        .unwrap_or_else(|_| cmd.get_cwd().unwrap().to_path_buf());
+        .unwrap_or_else(|_| script_out_dir.clone());
+
     let prev_output =
-        BuildOutput::parse_file(&output_file, &pkg_name, &prev_root_output, &root_output).ok();
+        BuildOutput::parse_file(&output_file, &pkg_name, &prev_script_out_dir, &script_out_dir).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)?;
+    fs::create_dir_all(&script_dir)?;
+    fs::create_dir_all(&script_out_dir)?;
 
     // Prepare the unit of "dirty work" which will actually run the custom build
     // command.
@@ -283,8 +285,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
         //
         // If we have an old build directory, then just move it into place,
         // otherwise create it!
-        if fs::metadata(&build_output).is_err() {
-            fs::create_dir(&build_output).chain_err(|| {
+        if fs::metadata(&script_out_dir).is_err() {
+            fs::create_dir(&script_out_dir).chain_err(|| {
                 internal(
                     "failed to create script output directory for \
                      build command",
@@ -316,7 +318,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
                 }
             }
             if let Some(build_scripts) = build_scripts {
-                super::add_plugin_deps(&mut cmd, &build_state, &build_scripts, &root_output)?;
+                super::add_plugin_deps(&mut cmd, &build_state, &build_scripts, &host_target_root)?;
             }
         }
 
@@ -348,9 +350,9 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
             // well.
             paths::write(&output_file, &output.stdout)?;
             paths::write(&err_file, &output.stderr)?;
-            paths::write(&root_output_file, util::path2bytes(&root_output)?)?;
+            paths::write(&root_output_file, util::path2bytes(&script_out_dir)?)?;
             let parsed_output =
-                BuildOutput::parse(&output.stdout, &pkg_name, &root_output, &root_output)?;
+                BuildOutput::parse(&output.stdout, &pkg_name, &script_out_dir, &script_out_dir)?;
 
             if json_messages {
                 emit_build_output(&parsed_output, &id);
@@ -364,11 +366,11 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes
     // 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, root_output) = all;
+        let (id, pkg_name, build_state, output_file, script_out_dir) = all;
         let output = match prev_output {
             Some(output) => output,
             None => {
-                BuildOutput::parse_file(&output_file, &pkg_name, &prev_root_output, &root_output)?
+                BuildOutput::parse_file(&output_file, &pkg_name, &prev_script_out_dir, &script_out_dir)?
             }
         };
 
@@ -406,11 +408,11 @@ impl BuildOutput {
     pub fn parse_file(
         path: &Path,
         pkg_name: &str,
-        root_output_when_generated: &Path,
-        root_output: &Path,
+        script_out_dir_when_generated: &Path,
+        script_out_dir: &Path,
     ) -> CargoResult<BuildOutput> {
         let contents = paths::read_bytes(path)?;
-        BuildOutput::parse(&contents, pkg_name, root_output_when_generated, root_output)
+        BuildOutput::parse(&contents, pkg_name, script_out_dir_when_generated, script_out_dir)
     }
 
     // Parses the output of a script.
@@ -418,8 +420,8 @@ impl BuildOutput {
     pub fn parse(
         input: &[u8],
         pkg_name: &str,
-        root_output_when_generated: &Path,
-        root_output: &Path,
+        script_out_dir_when_generated: &Path,
+        script_out_dir: &Path,
     ) -> CargoResult<BuildOutput> {
         let mut library_paths = Vec::new();
         let mut library_links = Vec::new();
@@ -456,23 +458,24 @@ 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),
-            };
+            // This will rewrite paths if the target directory has been moved.
+            let value = value.replace(
+                script_out_dir_when_generated.to_str().unwrap(),
+                script_out_dir.to_str().unwrap(),
+            );
 
             match key {
                 "rustc-flags" => {
-                    let (paths, links) = BuildOutput::parse_rustc_flags(value, &whence)?;
+                    let (paths, links) = BuildOutput::parse_rustc_flags(&value, &whence)?;
                     library_links.extend(links.into_iter());
                     library_paths.extend(paths.into_iter());
                 }
                 "rustc-link-lib" => library_links.push(value.to_string()),
-                "rustc-link-search" => library_paths.push(path(value)),
+                "rustc-link-search" => library_paths.push(PathBuf::from(value)),
                 "rustc-cfg" => cfgs.push(value.to_string()),
-                "rustc-env" => env.push(BuildOutput::parse_rustc_env(value, &whence)?),
+                "rustc-env" => env.push(BuildOutput::parse_rustc_env(&value, &whence)?),
                 "warning" => warnings.push(value.to_string()),
-                "rerun-if-changed" => rerun_if_changed.push(path(value)),
+                "rerun-if-changed" => rerun_if_changed.push(PathBuf::from(value)),
                 "rerun-if-env-changed" => rerun_if_env_changed.push(value.to_string()),
                 _ => metadata.push((key.to_string(), value.to_string())),
             }

tests/testsuite/build_script.rs

@@ -3191,6 +3191,24 @@ failed to select a version for `a` which could resolve this conflict
 
 #[test]
 fn rename_with_link_search_path() {
+    _rename_with_link_search_path(false);
+}
+
+#[test]
+fn rename_with_link_search_path_cross() {
+    if cross_compile::disabled() {
+        return;
+    }
+
+    _rename_with_link_search_path(true);
+}
+
+fn _rename_with_link_search_path(cross: bool) {
+    let target_arg = if cross {
+        format!(" --target={}", cross_compile::alternate())
+    } else {
+        "".to_string()
+    };
     let p = project()
         .file(
             "Cargo.toml",
@@ -3209,7 +3227,7 @@ fn rename_with_link_search_path() {
         );
     let p = p.build();
 
-    p.cargo("build").run();
+    p.cargo(&format!("build{}", target_arg)).run();
 
     let p2 = project()
         .at("bar")
@@ -3242,7 +3260,7 @@ fn rename_with_link_search_path() {
                 } else {
                     println!("cargo:rustc-link-lib=foo");
                 }
-                println!("cargo:rustc-link-search={}",
+                println!("cargo:rustc-link-search=all={}",
                          dst.parent().unwrap().display());
             }
         "#,
@@ -3265,7 +3283,11 @@ fn rename_with_link_search_path() {
     // 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 = p.root().join("target").join("debug").join("deps");
+    let root = if cross {
+        p.root().join("target").join(cross_compile::alternate()).join("debug").join("deps")
+    } else {
+        p.root().join("target").join("debug").join("deps")
+    };
     let file = format!("{}foo{}", env::consts::DLL_PREFIX, env::consts::DLL_SUFFIX);
     let src = root.join(&file);
 
@@ -3280,7 +3302,7 @@ fn rename_with_link_search_path() {
     remove_dir_all(p.root()).unwrap();
 
     // Everything should work the first time
-    p2.cargo("run").run();
+    p2.cargo(&format!("run{}", target_arg)).run();
 
     // Now rename the root directory and rerun `cargo run`. Not only should we
     // not build anything but we also shouldn't crash.
@@ -3309,7 +3331,7 @@ fn rename_with_link_search_path() {
         thread::sleep(Duration::from_millis(100));
     }
 
-    p2.cargo("run")
+    p2.cargo(&format!("run{}", target_arg))
         .cwd(&new)
         .with_stderr(
             "\