Auto merge of #4006 - mcgoo:cargo_test_dylib, r=alexcrichton

fix `cargo test` of dylib projects for end user runs too

Fixes running `cargo test` and `cargo test --target <target>` for dylib projects.

Moves the logic just landed in #3996 into cargo itself, so cargo sets the dylib path for anyone running `cargo test` or `cargo run`. Current master sets the dylib path only for  `cargo test` on cargo itself.

This PR pins to rustup 1.2.0 for the purposes of testing. If rust-lang/rustup#1093 ends up working out, then this PR would only be important for non-rustup users and people doing cross testing, `cargo test --target <target>`.

Arguably https://github.com/mcgoo/cargo/blob/ed273851f8bc76f726eda4a2e2a7bb470c3718bc/src/cargo/ops/cargo_rustc/context.rs#L249-L253 should point to lib/rustlib/\<host triple\>/lib instead of sysroot/lib, because I think if the libs are different, you will never be able to compile a working plugin anyway, and for the host=target case you get the  lib/rustlib/\<host triple\>/lib anyhow. Is there ever a case where the lib/rustlib/\<host triple\>/lib and sysroot/lib versions of the libs would be expected to differ?

This is not a huge deal for me one way or the other - it doesn't impact my workflow at all. I nearly dropped it when I saw @alexcrichton had made it all work in 3996, but I think it's worth doing because it removes a surprise. It certainly would have saved me a couple of days of confusion. Either way, thanks for looking it over.

Author: bors

src/cargo/ops/cargo_rustc/compilation.rs

@@ -35,6 +35,12 @@ pub struct Compilation<'cfg> {
     /// which have dynamic dependencies.
     pub plugins_dylib_path: PathBuf,
 
+    /// The path to rustc's own libstd
+    pub host_dylib_path: Option<PathBuf>,
+
+    /// The path to libstd for the target
+    pub target_dylib_path: Option<PathBuf>,
+
     /// Extra environment variables that were passed to compilations and should
     /// be passed to future invocations of programs.
     pub extra_env: HashMap<PackageId, Vec<(String, String)>>,
@@ -57,6 +63,8 @@ impl<'cfg> Compilation<'cfg> {
             root_output: PathBuf::from("/"),
             deps_output: PathBuf::from("/"),
             plugins_dylib_path: PathBuf::from("/"),
+            host_dylib_path: None,
+            target_dylib_path: None,
             tests: Vec::new(),
             binaries: Vec::new(),
             extra_env: HashMap::new(),
@@ -98,13 +106,16 @@ impl<'cfg> Compilation<'cfg> {
                 -> CargoResult<ProcessBuilder> {
 
         let mut search_path = if is_host {
-            vec![self.plugins_dylib_path.clone()]
+            let mut search_path = vec![self.plugins_dylib_path.clone()];
+            search_path.push(self.host_dylib_path.iter().collect());
+            search_path
         } else {
             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.push(self.target_dylib_path.iter().collect());
             search_path
         };
 

src/cargo/ops/cargo_rustc/context.rs

@@ -206,11 +206,12 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         }
 
         let mut with_cfg = process.clone();
+        with_cfg.arg("--print=sysroot");
         with_cfg.arg("--print=cfg");
 
-        let mut has_cfg = true;
+        let mut has_cfg_and_sysroot = true;
         let output = with_cfg.exec_with_output().or_else(|_| {
-            has_cfg = false;
+            has_cfg_and_sysroot = false;
             process.exec_with_output()
         }).chain_error(|| {
             human(format!("failed to run `rustc` to learn about \
@@ -247,7 +248,30 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             map.insert(crate_type.to_string(), Some((prefix.to_string(), suffix.to_string())));
         }
 
-        let cfg = if has_cfg {
+        if has_cfg_and_sysroot {
+            let line = match lines.next() {
+                Some(line) => line,
+                None => bail!("output of --print=sysroot missing when learning about \
+                               target-specific information from rustc"),
+            };
+            let mut rustlib = PathBuf::from(line);
+            if kind == Kind::Host {
+                if cfg!(windows) {
+                    rustlib.push("bin");
+                } else {
+                    rustlib.push("lib");
+                }
+                self.compilation.host_dylib_path = Some(rustlib);
+            } else {
+                rustlib.push("lib");
+                rustlib.push("rustlib");
+                rustlib.push(self.target_triple());
+                rustlib.push("lib");
+                self.compilation.target_dylib_path = Some(rustlib);
+            }
+        }
+
+        let cfg = if has_cfg_and_sysroot {
             Some(try!(lines.map(Cfg::from_str).collect()))
         } else {
             None

tests/cargotest/lib.rs

@@ -20,10 +20,9 @@ extern crate url;
 
 use std::ffi::OsStr;
 use std::time::Duration;
-use std::path::{Path, PathBuf};
 
 use cargo::util::Rustc;
-use cargo::util::paths;
+use std::path::PathBuf;
 
 pub mod support;
 pub mod install;
@@ -67,25 +66,6 @@ fn _process(t: &OsStr) -> cargo::util::ProcessBuilder {
      .env_remove("GIT_COMMITTER_EMAIL")
      .env_remove("CARGO_TARGET_DIR")     // we assume 'target'
      .env_remove("MSYSTEM");             // assume cmd.exe everywhere on windows
-
-    // We'll need dynamic libraries at some point in this test suite, so ensure
-    // that the rustc libdir is somewhere in LD_LIBRARY_PATH as appropriate.
-    let mut rustc = RUSTC.with(|r| r.process());
-    let output = rustc.arg("--print").arg("sysroot").exec_with_output().unwrap();
-    let libdir = String::from_utf8(output.stdout).unwrap();
-    let libdir = Path::new(libdir.trim());
-    let libdir = if cfg!(windows) {
-        libdir.join("bin")
-    } else {
-        libdir.join("lib")
-    };
-    let mut paths = paths::dylib_path();
-    println!("libdir: {:?}", libdir);
-    if !paths.contains(&libdir) {
-        paths.push(libdir);
-        p.env(paths::dylib_path_envvar(),
-              paths::join_paths(&paths, paths::dylib_path_envvar()).unwrap());
-    }
     return p
 }
 

tests/cross-compile.rs

@@ -1053,3 +1053,79 @@ fn platform_specific_variables_reflected_in_build_scripts() {
     assert_that(p.cargo("build").arg("-v").arg("--target").arg(&target),
                 execs().with_status(0));
 }
+
+#[test]
+fn cross_test_dylib() {
+    if disabled() { return }
+
+    let target = alternate();
+
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [lib]
+            name = "foo"
+            crate_type = ["dylib"]
+
+            [dependencies.bar]
+            path = "bar"
+        "#)
+        .file("src/lib.rs", r#"
+            extern crate bar as the_bar;
+
+            pub fn bar() { the_bar::baz(); }
+
+            #[test]
+            fn foo() { bar(); }
+        "#)
+        .file("tests/test.rs", r#"
+            extern crate foo as the_foo;
+
+            #[test]
+            fn foo() { the_foo::bar(); }
+        "#)
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.0.1"
+            authors = []
+
+            [lib]
+            name = "bar"
+            crate_type = ["dylib"]
+        "#)
+        .file("bar/src/lib.rs", &format!(r#"
+             use std::env;
+             pub fn baz() {{
+                assert_eq!(env::consts::ARCH, "{}");
+            }}
+        "#, alternate_arch()));
+
+    assert_that(p.cargo_process("test").arg("--target").arg(&target),
+                execs().with_status(0)
+                       .with_stderr(&format!("\
+[COMPILING] bar v0.0.1 ({dir}/bar)
+[COMPILING] foo v0.0.1 ({dir})
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
+[RUNNING] target[/]{arch}[/]debug[/]deps[/]foo-[..][EXE]
+[RUNNING] target[/]{arch}[/]debug[/]deps[/]test-[..][EXE]",
+                        dir = p.url(), arch = alternate()))
+                       .with_stdout("
+running 1 test
+test foo ... ok
+
+test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured
+
+
+running 1 test
+test foo ... ok
+
+test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured
+
+"));
+
+}