Auto merge of #3974 - mcgoo:build_rs_native_path, r=alexcrichton

fix dynamic search path for build scripts

fixes #3957

Author: bors

src/cargo/ops/cargo_rustc/compilation.rs

@@ -100,32 +100,9 @@ impl<'cfg> Compilation<'cfg> {
         let mut search_path = if is_host {
             vec![self.plugins_dylib_path.clone()]
         } else {
-            let mut search_path = vec![];
-
-            // Add -L arguments, after stripping off prefixes like "native="
-            // or "framework=" and filtering out directories *not* inside our
-            // output directory, since they are likely spurious and can cause
-            // clashes with system shared libraries (issue #3366).
-            for dir in self.native_dirs.iter() {
-                let dir = match dir.to_str() {
-                    Some(s) => {
-                        let mut parts = s.splitn(2, '=');
-                        match (parts.next(), parts.next()) {
-                            (Some("native"), Some(path)) |
-                            (Some("crate"), Some(path)) |
-                            (Some("dependency"), Some(path)) |
-                            (Some("framework"), Some(path)) |
-                            (Some("all"), Some(path)) => path.into(),
-                            _ => dir.clone(),
-                        }
-                    }
-                    None => dir.clone(),
-                };
-
-                if dir.starts_with(&self.root_output) {
-                    search_path.push(dir);
-                }
-            }
+            let mut search_path =
+                super::filter_dynamic_search_path(self.native_dirs.iter(),
+                                                  &self.root_output);
             search_path.push(self.root_output.clone());
             search_path.push(self.deps_output.clone());
             search_path

src/cargo/ops/cargo_rustc/context.rs

@@ -336,6 +336,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         self.host.deps()
     }
 
+    /// Return the root of the build output tree
+    pub fn target_root(&self) -> &Path {
+        self.host.dest()
+    }
+
     /// Returns the appropriate output directory for the specified package and
     /// target.
     pub fn out_dir(&mut self, unit: &Unit) -> PathBuf {

src/cargo/ops/cargo_rustc/custom_build.rs

@@ -181,6 +181,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
     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.
     //
@@ -218,7 +220,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
             }
             if let Some(build_scripts) = build_scripts {
                 super::add_plugin_deps(&mut cmd, &build_state,
-                                            &build_scripts)?;
+                                            &build_scripts,
+                                            &root_output)?;
             }
         }
 

src/cargo/ops/cargo_rustc/mod.rs

@@ -297,6 +297,8 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc<Executor>) -> CargoResult<Work
     exec.init(cx);
     let exec = exec.clone();
 
+    let root_output = cx.target_root().to_path_buf();
+
     return Ok(Work::new(move |state| {
         // Only at runtime have we discovered what the extra -L and -l
         // arguments are for native libraries, so we process those here. We
@@ -307,7 +309,8 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc<Executor>) -> CargoResult<Work
             let build_state = build_state.outputs.lock().unwrap();
             add_native_deps(&mut rustc, &build_state, &build_deps,
                                  pass_l_flag, &current_id)?;
-            add_plugin_deps(&mut rustc, &build_state, &build_deps)?;
+            add_plugin_deps(&mut rustc, &build_state, &build_deps,
+                                 &root_output)?;
         }
 
         // FIXME(rust-lang/rust#18913): we probably shouldn't have to do
@@ -492,7 +495,8 @@ fn load_build_deps(cx: &Context, unit: &Unit) -> Option<Arc<BuildScripts>> {
 // execute.
 fn add_plugin_deps(rustc: &mut ProcessBuilder,
                    build_state: &BuildMap,
-                   build_scripts: &BuildScripts)
+                   build_scripts: &BuildScripts,
+                   root_output: &PathBuf)
                    -> CargoResult<()> {
     let var = util::dylib_path_envvar();
     let search_path = rustc.get_env(var).unwrap_or(OsString::new());
@@ -502,15 +506,47 @@ fn add_plugin_deps(rustc: &mut ProcessBuilder,
         let output = build_state.get(&key).chain_error(|| {
             internal(format!("couldn't find libs for plugin dep {}", id))
         })?;
-        for path in output.library_paths.iter() {
-            search_path.push(path.clone());
-        }
+        search_path.append(&mut filter_dynamic_search_path(output.library_paths.iter(),
+                                                           root_output));
     }
     let search_path = join_paths(&search_path, var)?;
     rustc.env(var, &search_path);
     Ok(())
 }
 
+// Determine paths to add to the dynamic search path from -L entries
+//
+// Strip off prefixes like "native=" or "framework=" and filter out directories
+// *not* inside our output directory since they are likely spurious and can cause
+// clashes with system shared libraries (issue #3366).
+fn filter_dynamic_search_path<'a, I>(paths :I, root_output: &PathBuf) -> Vec<PathBuf>
+        where I: Iterator<Item=&'a PathBuf> {
+    let mut search_path = vec![];
+    for dir in paths {
+        let dir = match dir.to_str() {
+            Some(s) => {
+                let mut parts = s.splitn(2, '=');
+                match (parts.next(), parts.next()) {
+                    (Some("native"), Some(path)) |
+                    (Some("crate"), Some(path)) |
+                    (Some("dependency"), Some(path)) |
+                    (Some("framework"), Some(path)) |
+                    (Some("all"), Some(path)) => path.into(),
+                    _ => dir.clone(),
+                }
+            }
+            None => dir.clone(),
+        };
+        if dir.starts_with(&root_output) {
+            search_path.push(dir);
+        } else {
+            debug!("Not including path {} in runtime library search path because it is \
+                    outside target root {}", dir.display(), root_output.display());
+        }
+    }
+    search_path
+}
+
 fn prepare_rustc(cx: &mut Context,
                  crate_types: Vec<&str>,
                  unit: &Unit) -> CargoResult<ProcessBuilder> {

tests/build-script.rs

@@ -1131,7 +1131,15 @@ fn test_dev_dep_build_script() {
 
 #[test]
 fn build_script_with_dynamic_native_dependency() {
-    let build = project("builder")
+
+    let workspace = project("ws")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["builder", "foo"]
+        "#);
+    workspace.build();
+
+    let build = project("ws/builder")
         .file("Cargo.toml", r#"
             [package]
             name = "builder"
@@ -1147,11 +1155,9 @@ fn build_script_with_dynamic_native_dependency() {
             #[no_mangle]
             pub extern fn foo() {}
         "#);
-    assert_that(build.cargo_process("build").arg("-v")
-                .env("RUST_LOG", "cargo::ops::cargo_rustc"),
-                execs().with_status(0));
+    build.build();
 
-    let foo = project("foo")
+    let foo = project("ws/foo")
         .file("Cargo.toml", r#"
             [package]
             name = "foo"
@@ -1180,7 +1186,7 @@ fn build_script_with_dynamic_native_dependency() {
 
             fn main() {
                 let src = PathBuf::from(env::var("SRC").unwrap());
-                println!("cargo:rustc-link-search={}/target/debug/deps",
+                println!("cargo:rustc-link-search=native={}/target/debug/deps",
                          src.display());
             }
         "#)
@@ -1192,8 +1198,13 @@ fn build_script_with_dynamic_native_dependency() {
                 unsafe { foo() }
             }
         "#);
+    foo.build();
+
+    assert_that(build.cargo("build").arg("-v")
+                .env("RUST_LOG", "cargo::ops::cargo_rustc"),
+                execs().with_status(0));
 
-    assert_that(foo.cargo_process("build").arg("-v").env("SRC", build.root())
+    assert_that(foo.cargo("build").arg("-v").env("SRC", build.root())
                 .env("RUST_LOG", "cargo::ops::cargo_rustc"),
                 execs().with_status(0));
 }

tests/plugins.rs

@@ -90,7 +90,14 @@ fn plugin_to_the_max() {
 fn plugin_with_dynamic_native_dependency() {
     if !is_nightly() { return }
 
-    let build = project("builder")
+    let workspace = project("ws")
+        .file("Cargo.toml", r#"
+            [workspace]
+            members = ["builder", "foo"]
+        "#);
+    workspace.build();
+
+    let build = project("ws/builder")
         .file("Cargo.toml", r#"
             [package]
             name = "builder"
@@ -105,16 +112,9 @@ fn plugin_with_dynamic_native_dependency() {
             #[no_mangle]
             pub extern fn foo() {}
         "#);
-    assert_that(build.cargo_process("build"),
-                execs().with_status(0));
-    let src = build.root().join("target/debug");
-    let lib = fs::read_dir(&src).unwrap().map(|s| s.unwrap().path()).find(|lib| {
-        let lib = lib.file_name().unwrap().to_str().unwrap();
-        lib.starts_with(env::consts::DLL_PREFIX) &&
-            lib.ends_with(env::consts::DLL_SUFFIX)
-    }).unwrap();
+    build.build();
 
-    let foo = project("foo")
+    let foo = project("ws/foo")
         .file("Cargo.toml", r#"
             [package]
             name = "foo"
@@ -165,8 +165,19 @@ fn plugin_with_dynamic_native_dependency() {
                 unsafe { foo() }
             }
         "#);
+    foo.build();
+
+    assert_that(build.cargo("build"),
+                execs().with_status(0));
+
+    let src = workspace.root().join("target/debug");
+    let lib = fs::read_dir(&src).unwrap().map(|s| s.unwrap().path()).find(|lib| {
+        let lib = lib.file_name().unwrap().to_str().unwrap();
+        lib.starts_with(env::consts::DLL_PREFIX) &&
+            lib.ends_with(env::consts::DLL_SUFFIX)
+    }).unwrap();
 
-    assert_that(foo.cargo_process("build").env("SRC", &lib).arg("-v"),
+    assert_that(foo.cargo("build").env("SRC", &lib).arg("-v"),
                 execs().with_status(0));
 }