Skip to content

Commit 4bbfc70

Browse files
committed
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
2 parents 510cf5e + f0c0a6a commit 4bbfc70

File tree

3 files changed

+159
-18
lines changed

3 files changed

+159
-18
lines changed

src/cargo/ops/cargo_rustc/custom_build.rs

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::sync::{Mutex, Arc};
77
use core::PackageId;
88
use util::{Freshness, Cfg};
99
use util::errors::{CargoResult, CargoResultExt, CargoError};
10-
use util::{internal, profile, paths};
10+
use util::{self, internal, profile, paths};
1111
use util::machine_message;
1212

1313
use super::job::Work;
@@ -27,7 +27,7 @@ pub struct BuildOutput {
2727
/// Metadata to pass to the immediate dependencies
2828
pub metadata: Vec<(String, String)>,
2929
/// Paths to trigger a rerun of this build script.
30-
pub rerun_if_changed: Vec<String>,
30+
pub rerun_if_changed: Vec<PathBuf>,
3131
/// Environment variables which, when changed, will cause a rebuild.
3232
pub rerun_if_env_changed: Vec<String>,
3333
/// Warnings generated by this build,
@@ -65,7 +65,7 @@ pub struct BuildScripts {
6565

6666
pub struct BuildDeps {
6767
pub build_script_output: PathBuf,
68-
pub rerun_if_changed: Vec<String>,
68+
pub rerun_if_changed: Vec<PathBuf>,
6969
pub rerun_if_env_changed: Vec<String>,
7070
}
7171

@@ -178,29 +178,37 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
178178
let pkg_name = unit.pkg.to_string();
179179
let build_state = Arc::clone(&cx.build_state);
180180
let id = unit.pkg.package_id().clone();
181-
let (output_file, err_file) = {
181+
let (output_file, err_file, root_output_file) = {
182182
let build_output_parent = build_output.parent().unwrap();
183183
let output_file = build_output_parent.join("output");
184184
let err_file = build_output_parent.join("stderr");
185-
(output_file, err_file)
185+
let root_output_file = build_output_parent.join("root-output");
186+
(output_file, err_file, root_output_file)
186187
};
188+
let root_output = cx.target_root().to_path_buf();
187189
let all = (id.clone(), pkg_name.clone(), Arc::clone(&build_state),
188-
output_file.clone());
190+
output_file.clone(), root_output.clone());
189191
let build_scripts = super::load_build_deps(cx, unit);
190192
let kind = unit.kind;
191193
let json_messages = cx.build_config.json_messages;
192194

193195
// Check to see if the build script has already run, and if it has keep
194196
// track of whether it has told us about some explicit dependencies
195-
let prev_output = BuildOutput::parse_file(&output_file, &pkg_name).ok();
197+
let prev_root_output = paths::read_bytes(&root_output_file)
198+
.and_then(|bytes| util::bytes2path(&bytes))
199+
.unwrap_or(cmd.get_cwd().unwrap().to_path_buf());
200+
let prev_output = BuildOutput::parse_file(
201+
&output_file,
202+
&pkg_name,
203+
&prev_root_output,
204+
&root_output,
205+
).ok();
196206
let deps = BuildDeps::new(&output_file, prev_output.as_ref());
197207
cx.build_explicit_deps.insert(*unit, deps);
198208

199209
fs::create_dir_all(&script_output)?;
200210
fs::create_dir_all(&build_output)?;
201211

202-
let root_output = cx.target_root().to_path_buf();
203-
204212
// Prepare the unit of "dirty work" which will actually run the custom build
205213
// command.
206214
//
@@ -266,7 +274,13 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
266274
// well.
267275
paths::write(&output_file, &output.stdout)?;
268276
paths::write(&err_file, &output.stderr)?;
269-
let parsed_output = BuildOutput::parse(&output.stdout, &pkg_name)?;
277+
paths::write(&root_output_file, &util::path2bytes(&root_output)?)?;
278+
let parsed_output = BuildOutput::parse(
279+
&output.stdout,
280+
&pkg_name,
281+
&root_output,
282+
&root_output,
283+
)?;
270284

271285
if json_messages {
272286
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>)
289303
// itself to run when we actually end up just discarding what we calculated
290304
// above.
291305
let fresh = Work::new(move |_tx| {
292-
let (id, pkg_name, build_state, output_file) = all;
306+
let (id, pkg_name, build_state, output_file, root_output) = all;
293307
let output = match prev_output {
294308
Some(output) => output,
295-
None => BuildOutput::parse_file(&output_file, &pkg_name)?,
309+
None => {
310+
BuildOutput::parse_file(
311+
&output_file,
312+
&pkg_name,
313+
&prev_root_output,
314+
&root_output,
315+
)?
316+
}
296317
};
297318
build_state.insert(id, kind, output);
298319
Ok(())
@@ -321,14 +342,20 @@ impl BuildState {
321342
}
322343

323344
impl BuildOutput {
324-
pub fn parse_file(path: &Path, pkg_name: &str) -> CargoResult<BuildOutput> {
345+
pub fn parse_file(path: &Path,
346+
pkg_name: &str,
347+
root_output_when_generated: &Path,
348+
root_output: &Path) -> CargoResult<BuildOutput> {
325349
let contents = paths::read_bytes(path)?;
326-
BuildOutput::parse(&contents, pkg_name)
350+
BuildOutput::parse(&contents, pkg_name, root_output_when_generated, root_output)
327351
}
328352

329353
// Parses the output of a script.
330354
// The `pkg_name` is used for error messages.
331-
pub fn parse(input: &[u8], pkg_name: &str) -> CargoResult<BuildOutput> {
355+
pub fn parse(input: &[u8],
356+
pkg_name: &str,
357+
root_output_when_generated: &Path,
358+
root_output: &Path) -> CargoResult<BuildOutput> {
332359
let mut library_paths = Vec::new();
333360
let mut library_links = Vec::new();
334361
let mut cfgs = Vec::new();
@@ -364,6 +391,13 @@ impl BuildOutput {
364391
_ => bail!("Wrong output in {}: `{}`", whence, line),
365392
};
366393

394+
let path = |val: &str| {
395+
match Path::new(val).strip_prefix(root_output_when_generated) {
396+
Ok(path) => root_output.join(path),
397+
Err(_) => PathBuf::from(val),
398+
}
399+
};
400+
367401
match key {
368402
"rustc-flags" => {
369403
let (paths, links) =
@@ -372,11 +406,11 @@ impl BuildOutput {
372406
library_paths.extend(paths.into_iter());
373407
}
374408
"rustc-link-lib" => library_links.push(value.to_string()),
375-
"rustc-link-search" => library_paths.push(PathBuf::from(value)),
409+
"rustc-link-search" => library_paths.push(path(value)),
376410
"rustc-cfg" => cfgs.push(value.to_string()),
377411
"rustc-env" => env.push(BuildOutput::parse_rustc_env(value, &whence)?),
378412
"warning" => warnings.push(value.to_string()),
379-
"rerun-if-changed" => rerun_if_changed.push(value.to_string()),
413+
"rerun-if-changed" => rerun_if_changed.push(path(value)),
380414
"rerun-if-env-changed" => rerun_if_env_changed.push(value.to_string()),
381415
_ => metadata.push((key.to_string(), value.to_string())),
382416
}

src/cargo/ops/cargo_rustc/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,9 @@ fn link_targets<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
551551
#[cfg(windows)]
552552
use std::os::windows::fs::symlink_dir as symlink;
553553

554-
symlink(src, dst)
554+
let dst_dir = dst.parent().unwrap();
555+
assert!(src.starts_with(dst_dir));
556+
symlink(src.strip_prefix(dst_dir).unwrap(), dst)
555557
} else {
556558
fs::hard_link(src, dst)
557559
};

tests/build-script.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
extern crate cargotest;
22
extern crate hamcrest;
33

4+
use std::env;
45
use std::fs::{self, File};
56
use std::io::prelude::*;
7+
use std::path::PathBuf;
68

79
use cargotest::{rustc_host, sleep_ms};
810
use cargotest::support::{project, execs};
@@ -2794,3 +2796,106 @@ package `a v0.5.0 (file://[..])`
27942796
also links to native library `a`
27952797
"));
27962798
}
2799+
2800+
#[test]
2801+
fn rename_with_link_search_path() {
2802+
let p = project("foo")
2803+
.file("Cargo.toml", r#"
2804+
[project]
2805+
name = "foo"
2806+
version = "0.5.0"
2807+
authors = []
2808+
2809+
[lib]
2810+
crate-type = ["cdylib"]
2811+
"#)
2812+
.file("src/lib.rs", "
2813+
#[no_mangle]
2814+
pub extern fn cargo_test_foo() {}
2815+
");
2816+
let p = p.build();
2817+
2818+
assert_that(p.cargo("build"), execs().with_status(0));
2819+
2820+
let p2 = project("bar")
2821+
.file("Cargo.toml", r#"
2822+
[project]
2823+
name = "bar"
2824+
version = "0.5.0"
2825+
authors = []
2826+
"#)
2827+
.file("build.rs", r#"
2828+
use std::env;
2829+
use std::fs;
2830+
use std::path::PathBuf;
2831+
2832+
fn main() {
2833+
// Move the `libfoo.so` from the root of our project into the
2834+
// build directory. This way Cargo should automatically manage
2835+
// `LD_LIBRARY_PATH` and such.
2836+
let root = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap());
2837+
let file = format!("{}foo{}", env::consts::DLL_PREFIX, env::consts::DLL_SUFFIX);
2838+
let src = root.join(&file);
2839+
2840+
let dst_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());
2841+
let dst = dst_dir.join(&file);
2842+
2843+
fs::copy(&src, &dst).unwrap();
2844+
// handle windows, like below
2845+
drop(fs::copy(root.join("foo.dll.lib"), dst_dir.join("foo.dll.lib")));
2846+
2847+
println!("cargo:rerun-if-changed=build.rs");
2848+
if cfg!(target_env = "msvc") {
2849+
println!("cargo:rustc-link-lib=foo.dll");
2850+
} else {
2851+
println!("cargo:rustc-link-lib=foo");
2852+
}
2853+
println!("cargo:rustc-link-search={}",
2854+
dst.parent().unwrap().display());
2855+
}
2856+
"#)
2857+
.file("src/main.rs", r#"
2858+
extern {
2859+
#[link_name = "cargo_test_foo"]
2860+
fn foo();
2861+
}
2862+
2863+
fn main() {
2864+
unsafe { foo(); }
2865+
}
2866+
"#);
2867+
let p2 = p2.build();
2868+
2869+
// Move the output `libfoo.so` into the directory of `p2`, and then delete
2870+
// the `p` project. On OSX the `libfoo.dylib` artifact references the
2871+
// original path in `p` so we want to make sure that it can't find it (hence
2872+
// the deletion).
2873+
let root = PathBuf::from(p.root());
2874+
let root = root.join("target").join("debug").join("deps");
2875+
let file = format!("{}foo{}", env::consts::DLL_PREFIX, env::consts::DLL_SUFFIX);
2876+
let src = root.join(&file);
2877+
2878+
let dst = p2.root().join(&file);
2879+
2880+
fs::copy(&src, &dst).unwrap();
2881+
// copy the import library for windows, if it exists
2882+
drop(fs::copy(&root.join("foo.dll.lib"), p2.root().join("foo.dll.lib")));
2883+
fs::remove_dir_all(p.root()).unwrap();
2884+
2885+
// Everything should work the first time
2886+
assert_that(p2.cargo("run"),
2887+
execs().with_status(0));
2888+
2889+
// Now rename the root directory and rerun `cargo run`. Not only should we
2890+
// not build anything but we also shouldn't crash.
2891+
let mut new = p2.root();
2892+
new.pop();
2893+
new.push("bar2");
2894+
fs::rename(p2.root(), &new).unwrap();
2895+
assert_that(p2.cargo("run").cwd(&new),
2896+
execs().with_status(0)
2897+
.with_stderr("\
2898+
[FINISHED] [..]
2899+
[RUNNING] [..]
2900+
"));
2901+
}

0 commit comments

Comments
 (0)