auto merge of #14000 : pnkfelix/rust/fsk-fix-issue13732, r=alexcrichton

Fix #13732.

This is a revised, much less hacky form of PR #13753

The changes here:

 * add instrumentation to aid debugging of linkage errors,
 * fine tune some things in the Makefile where we are telling binaries to use a host-oriented path for finding dynamic libraries, when it should be feeding the binaries a target-oriented path for dynamic libraries.
 * pass along the current stage number to run-make tests, and
 * skip certain tests when running atop stage1.

Fix #13746 as well.

Author: bors

mk/main.mk

@@ -349,18 +349,45 @@ CFGFLAG$(1)_T_$(2)_H_$(3) = stage$(1)
 endef
 
 # Same macro/variables as above, but defined in a separate loop so it can use
-# all the varibles above for all archs. The RPATH_VAR setup sometimes needs to
+# all the variables above for all archs. The RPATH_VAR setup sometimes needs to
 # reach across triples to get things in order.
+#
+# Defines (with the standard $(1)_T_$(2)_H_$(3) suffix):
+# * `LD_LIBRARY_PATH_ENV_NAME`: the name for the key to use in the OS
+#   environment to access or extend the lookup path for dynamic
+#   libraries.  Note on Windows, that key is `$PATH`, and thus not
+#   only conflates programs with dynamic libraries, but also often
+#   contains spaces which confuse make.
+# * `LD_LIBRARY_PATH_ENV_HOSTDIR`: the entry to add to lookup path for the host
+# * `LD_LIBRARY_PATH_ENV_TARGETDIR`: the entry to add to lookup path for target
+# 
+# Below that, HOST_RPATH_VAR and TARGET_RPATH_VAR are defined in terms of the
+# above settings.
+# 
 define SREQ_CMDS
 
 ifeq ($$(OSTYPE_$(3)),apple-darwin)
-  RPATH_VAR$(1)_T_$(2)_H_$(3) := \
-      DYLD_LIBRARY_PATH="$$$$DYLD_LIBRARY_PATH:$$(CURDIR)/$$(HLIB$(1)_H_$(3))"
+  LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3) := DYLD_LIBRARY_PATH
+else
+ifeq ($$(CFG_WINDOWSY_$(2)),1)
+  LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3) := PATH
 else
-  RPATH_VAR$(1)_T_$(2)_H_$(3) := \
-      LD_LIBRARY_PATH="$$$$LD_LIBRARY_PATH:$$(CURDIR)/$$(HLIB$(1)_H_$(3))"
+  LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3) := LD_LIBRARY_PATH
+endif
 endif
 
+LD_LIBRARY_PATH_ENV_HOSTDIR$(1)_T_$(2)_H_$(3) := \
+    $$(CURDIR)/$$(HLIB$(1)_H_$(3))
+LD_LIBRARY_PATH_ENV_TARGETDIR$(1)_T_$(2)_H_$(3) := \
+    $$(CURDIR)/$$(TLIB1_T_$(2)_H_$(CFG_BUILD))
+
+HOST_RPATH_VAR$(1)_T_$(2)_H_$(3) := \
+  $$(LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3))=$$$$$$(LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3)):$$(LD_LIBRARY_PATH_ENV_HOSTDIR$(1)_T_$(2)_H_$(3))
+TARGET_RPATH_VAR$(1)_T_$(2)_H_$(3) := \
+  $$(LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3))=$$$$$$(LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3)):$$(LD_LIBRARY_PATH_ENV_TARGETDIR$(1)_T_$(2)_H_$(3))
+
+RPATH_VAR$(1)_T_$(2)_H_$(3) := $$(HOST_RPATH_VAR$(1)_T_$(2)_H_$(3))
+
 # Pass --cfg stage0 only for the build->host part of stage0;
 # if you're building a cross config, the host->* parts are
 # effectively stage1, since it uses the just-built stage0.
@@ -376,13 +403,7 @@ ifeq ($(1),0)
 ifneq ($(strip $(CFG_BUILD)),$(strip $(3)))
 CFGFLAG$(1)_T_$(2)_H_$(3) = stage1
 
-ifeq ($$(OSTYPE_$(3)),apple-darwin)
-  RPATH_VAR$(1)_T_$(2)_H_$(3) := \
-      DYLD_LIBRARY_PATH="$$$$DYLD_LIBRARY_PATH:$$(CURDIR)/$$(TLIB1_T_$(2)_H_$(CFG_BUILD))"
-else
-  RPATH_VAR$(1)_T_$(2)_H_$(3) := \
-      LD_LIBRARY_PATH="$$$$LD_LIBRARY_PATH:$$(CURDIR)/$$(TLIB1_T_$(2)_H_$(CFG_BUILD))"
-endif
+RPATH_VAR$(1)_T_$(2)_H_$(3) := $$(TARGET_RPATH_VAR$(1)_T_$(2)_H_$(3))
 endif
 endif
 

mk/tests.mk

@@ -793,8 +793,27 @@ else
 CRATEDOCTESTDEP_$(1)_$(2)_$(3)_$(4) = $$(RSINPUTS_$(4))
 endif
 
+# (Issues #13732, #13983, #14000) The doc for the regex crate includes
+# uses of the `regex!` macro from the regex_macros crate.  There is
+# normally a dependence injected that makes the target's regex depend
+# upon the host's regex_macros (see #13845), but that dependency
+# injection is currently skipped for stage1 as a special case.
+#
+# Therefore, as a further special case, this conditional skips
+# attempting to run the doc tests for the regex crate atop stage1,
+# (since there is no regex_macros crate for the stage1 rustc to load).
+#
+# (Another approach for solving this would be to inject the desired
+# dependence for stage1 as well, by setting things up to generate a
+# regex_macros crate that was compatible with the stage1 rustc and
+# thus re-enable our ability to run this test.)
+ifeq (stage$(1)-crate-$(4),stage1-crate-regex)
+check-stage$(1)-T-$(2)-H-$(3)-doc-crate-$(4)-exec:
+	@$$(call E, skipping doc-crate-$(4) as it uses macros and cannot run at stage$(1))
+else
 check-stage$(1)-T-$(2)-H-$(3)-doc-crate-$(4)-exec: \
 	$$(call TEST_OK_FILE,$(1),$(2),$(3),doc-crate-$(4))
+endif
 
 ifeq ($(2),$$(CFG_BUILD))
 $$(call TEST_OK_FILE,$(1),$(2),$(3),doc-crate-$(4)): $$(CRATEDOCTESTDEP_$(1)_$(2)_$(3)_$(4))
@@ -951,7 +970,10 @@ $(3)/test/run-make/%-$(1)-T-$(2)-H-$(3).ok: \
 	    "$$(CC_$(3)) $$(CFG_GCCISH_CFLAGS_$(3))" \
 	    $$(HBIN$(1)_H_$(3))/rustdoc$$(X_$(3)) \
 	    "$$(TESTNAME)" \
-	    "$$(RPATH_VAR$(1)_T_$(2)_H_$(3))"
+	    $$(LD_LIBRARY_PATH_ENV_NAME$(1)_T_$(2)_H_$(3)) \
+	    "$$(LD_LIBRARY_PATH_ENV_HOSTDIR$(1)_T_$(2)_H_$(3))" \
+	    "$$(LD_LIBRARY_PATH_ENV_TARGETDIR$(1)_T_$(2)_H_$(3))" \
+	    $(1)
 	@touch $$@
 else
 # FIXME #11094 - The above rule doesn't work right for multiple targets

src/compiletest/procsrv.rs

@@ -11,54 +11,31 @@
 use std::os;
 use std::str;
 use std::io::process::{ProcessExit, Command, Process, ProcessOutput};
+use std::unstable::dynamic_lib::DynamicLibrary;
 
-#[cfg(target_os = "win32")]
 fn target_env(lib_path: &str, prog: &str) -> Vec<(StrBuf, StrBuf)> {
-    let env = os::env();
+    let prog = if cfg!(windows) {prog.slice_to(prog.len() - 4)} else {prog};
+    let aux_path = prog + ".libaux";
 
-    // Make sure we include the aux directory in the path
-    assert!(prog.ends_with(".exe"));
-    let aux_path = prog.slice(0u, prog.len() - 4u).to_owned() + ".libaux";
+    // Need to be sure to put both the lib_path and the aux path in the dylib
+    // search path for the child.
+    let mut path = DynamicLibrary::search_path();
+    path.insert(0, Path::new(aux_path));
+    path.insert(0, Path::new(lib_path));
 
-    let mut new_env: Vec<_> = env.move_iter().map(|(k, v)| {
-        let new_v = if "PATH" == k {
-            format_strbuf!("{};{};{}", v, lib_path, aux_path)
-        } else {
-            v.to_strbuf()
-        };
-        (k.to_strbuf(), new_v)
-    }).collect();
-    if prog.ends_with("rustc.exe") {
-        new_env.push(("RUST_THREADS".to_strbuf(), "1".to_strbuf()));
+    // Remove the previous dylib search path var
+    let var = DynamicLibrary::envvar();
+    let mut env: Vec<(StrBuf,StrBuf)> =
+        os::env().move_iter().map(|(a,b)|(a.to_strbuf(), b.to_strbuf())).collect();
+    match env.iter().position(|&(ref k, _)| k.as_slice() == var) {
+        Some(i) => { env.remove(i); }
+        None => {}
     }
-    return new_env;
-}
 
-#[cfg(target_os = "linux")]
-#[cfg(target_os = "macos")]
-#[cfg(target_os = "freebsd")]
-fn target_env(lib_path: &str, prog: &str) -> Vec<(StrBuf,StrBuf)> {
-    // Make sure we include the aux directory in the path
-    let aux_path = prog + ".libaux";
-
-    let mut env: Vec<(StrBuf,StrBuf)> =
-        os::env().move_iter()
-                 .map(|(ref k, ref v)| (k.to_strbuf(), v.to_strbuf()))
-                 .collect();
-    let var = if cfg!(target_os = "macos") {
-        "DYLD_LIBRARY_PATH"
-    } else {
-        "LD_LIBRARY_PATH"
-    };
-    let prev = match env.iter().position(|&(ref k, _)| k.as_slice() == var) {
-        Some(i) => env.remove(i).unwrap().val1(),
-        None => "".to_strbuf(),
-    };
-    env.push((var.to_strbuf(), if prev.is_empty() {
-        format_strbuf!("{}:{}", lib_path, aux_path)
-    } else {
-        format_strbuf!("{}:{}:{}", lib_path, aux_path, prev)
-    }));
+    // Add the new dylib search path var
+    let newpath = DynamicLibrary::create_path(path.as_slice());
+    env.push((var.to_strbuf(),
+              str::from_utf8(newpath.as_slice()).unwrap().to_strbuf()));
     return env;
 }
 

src/etc/maketest.py

@@ -30,20 +30,21 @@ def putenv(name, value):
         value = normalize_path(value)
     os.putenv(name, value)
 
+def convert_path_spec(name, value):
+    if os.name == 'nt' and name != 'PATH':
+        value = ":".join(normalize_path(v) for v in value.split(";"))
+    return value
 
 make = sys.argv[2]
 putenv('RUSTC', os.path.abspath(sys.argv[3]))
 putenv('TMPDIR', os.path.abspath(sys.argv[4]))
 putenv('CC', sys.argv[5])
 putenv('RUSTDOC', os.path.abspath(sys.argv[6]))
 filt = sys.argv[7]
-ldpath = sys.argv[8]
-if ldpath != '':
-    name = ldpath.split('=')[0]
-    value = ldpath.split('=')[1]
-    if os.name == 'nt' and name != 'PATH':
-        value = ":".join(normalize_path(v) for v in value.split(";"))
-    os.putenv(name, value)
+putenv('LD_LIB_PATH_ENVVAR', sys.argv[8]);
+putenv('HOST_RPATH_DIR', os.path.abspath(sys.argv[9]));
+putenv('TARGET_RPATH_DIR', os.path.abspath(sys.argv[10]));
+putenv('RUST_BUILD_STAGE', sys.argv[11])
 
 if not filt in sys.argv[1]:
     sys.exit(0)

src/librustc/metadata/creader.rs

@@ -17,7 +17,7 @@ use back::svh::Svh;
 use driver::session::Session;
 use driver::{driver, config};
 use metadata::cstore;
-use metadata::cstore::CStore;
+use metadata::cstore::{CStore, CrateSource};
 use metadata::decoder;
 use metadata::loader;
 use metadata::loader::CratePaths;
@@ -68,10 +68,15 @@ impl<'a> visit::Visitor<()> for Env<'a> {
 
 fn dump_crates(cstore: &CStore) {
     debug!("resolved crates:");
-    cstore.iter_crate_data(|_, data| {
+    cstore.iter_crate_data_origins(|_, data, opt_source| {
         debug!("crate_id: {}", data.crate_id());
         debug!("  cnum: {}", data.cnum);
         debug!("  hash: {}", data.hash());
+        opt_source.map(|cs| {
+            let CrateSource { dylib, rlib, cnum: _ } = cs;
+            dylib.map(|dl| debug!("  dylib: {}", dl.display()));
+            rlib.map(|rl|  debug!("   rlib: {}", rl.display()));
+        });
     })
 }
 

src/librustc/metadata/cstore.rs

@@ -114,6 +114,17 @@ impl CStore {
         }
     }
 
+    /// Like `iter_crate_data`, but passes source paths (if available) as well.
+    pub fn iter_crate_data_origins(&self, i: |ast::CrateNum,
+                                              &crate_metadata,
+                                              Option<CrateSource>|) {
+        for (&k, v) in self.metas.borrow().iter() {
+            let origin = self.get_used_crate_source(k);
+            origin.as_ref().map(|cs| { assert!(k == cs.cnum); });
+            i(k, &**v, origin);
+        }
+    }
+
     pub fn add_used_crate_source(&self, src: CrateSource) {
         let mut used_crate_sources = self.used_crate_sources.borrow_mut();
         if !used_crate_sources.contains(&src) {

src/librustc/metadata/filesearch.rs

@@ -136,7 +136,7 @@ impl<'a> FileSearch<'a> {
 
     pub fn add_dylib_search_paths(&self) {
         self.for_each_lib_search_path(|lib_search_path| {
-            DynamicLibrary::add_search_path(lib_search_path);
+            DynamicLibrary::prepend_search_path(lib_search_path);
             FileDoesntMatch
         })
     }

src/librustdoc/lib.rs

@@ -284,7 +284,7 @@ fn rust_input(cratefile: &str, matches: &getopts::Matches) -> Output {
         core::run_core(libs.move_iter().map(|x| x.clone()).collect(),
                        cfgs.move_iter().map(|x| x.to_strbuf()).collect(),
                        &cr)
-    }).unwrap();
+    }).map_err(|boxed_any|format!("{:?}", boxed_any)).unwrap();
     info!("finished with rustc");
     analysiskey.replace(Some(analysis));
 

src/librustdoc/test.rs

@@ -15,6 +15,7 @@ use std::io::{Command, TempDir};
 use std::os;
 use std::str;
 use std::strbuf::StrBuf;
+use std::unstable::dynamic_lib::DynamicLibrary;
 
 use collections::{HashSet, HashMap};
 use testing;
@@ -150,12 +151,37 @@ fn runtest(test: &str, cratename: &str, libs: HashSet<Path>, should_fail: bool,
     let outdir = TempDir::new("rustdoctest").expect("rustdoc needs a tempdir");
     let out = Some(outdir.path().clone());
     let cfg = config::build_configuration(&sess);
+    let libdir = sess.target_filesearch().get_lib_path();
     driver::compile_input(sess, cfg, &input, &out, &None);
 
     if no_run { return }
 
     // Run the code!
-    match Command::new(outdir.path().join("rust_out")).output() {
+    //
+    // We're careful to prepend the *target* dylib search path to the child's
+    // environment to ensure that the target loads the right libraries at
+    // runtime. It would be a sad day if the *host* libraries were loaded as a
+    // mistake.
+    let exe = outdir.path().join("rust_out");
+    let env = {
+        let mut path = DynamicLibrary::search_path();
+        path.insert(0, libdir.clone());
+
+        // Remove the previous dylib search path var
+        let var = DynamicLibrary::envvar();
+        let mut env: Vec<(~str,~str)> = os::env().move_iter().collect();
+        match env.iter().position(|&(ref k, _)| k.as_slice() == var) {
+            Some(i) => { env.remove(i); }
+            None => {}
+        };
+
+        // Add the new dylib search path var
+        let newpath = DynamicLibrary::create_path(path.as_slice());
+        env.push((var.to_owned(),
+                  str::from_utf8(newpath.as_slice()).unwrap().to_owned()));
+        env
+    };
+    match Command::new(exe).env(env.as_slice()).output() {
         Err(e) => fail!("couldn't run the test: {}{}", e,
                         if e.kind == io::PermissionDenied {
                             " - maybe your tempdir is mounted with noexec?"