Auto merge of #4724 - pornel:sortpaths, r=alexcrichton

Sort native library paths for deterministic builds

Fixes #3800 by using a `BTreeSet`, which guarantees a deterministic iteration order.

Since the order was previously arbitrary, a sorted order is just as good. The list is so small, that any performance difference between `BTreeSet` and `HashSet` is negligible.

Author: bors

src/cargo/ops/cargo_rustc/compilation.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::{HashMap, HashSet, BTreeSet};
 use std::ffi::OsStr;
 use std::path::PathBuf;
 use semver::Version;
@@ -22,8 +22,10 @@ pub struct Compilation<'cfg> {
     ///
     /// This is currently used to drive some entries which are added to the
     /// LD_LIBRARY_PATH as appropriate.
+    ///
+    /// The order should be deterministic.
     // TODO: deprecated, remove
-    pub native_dirs: HashSet<PathBuf>,
+    pub native_dirs: BTreeSet<PathBuf>,
 
     /// Root output directory (for the local package's artifacts)
     pub root_output: PathBuf,
@@ -64,7 +66,7 @@ impl<'cfg> Compilation<'cfg> {
     pub fn new(config: &'cfg Config) -> Compilation<'cfg> {
         Compilation {
             libraries: HashMap::new(),
-            native_dirs: HashSet::new(),  // TODO: deprecated, remove
+            native_dirs: BTreeSet::new(),  // TODO: deprecated, remove
             root_output: PathBuf::from("/"),
             deps_output: PathBuf::from("/"),
             host_deps_output: PathBuf::from("/"),

tests/run.rs

@@ -784,6 +784,49 @@ fn run_with_library_paths() {
     assert_that(p.cargo("run"), execs().with_status(0));
 }
 
+#[test]
+fn library_paths_sorted_alphabetically() {
+    let p = project("foo");
+
+    let mut dir1 = p.target_debug_dir();
+    dir1.push("zzzzzzz");
+
+    let mut dir2 = p.target_debug_dir();
+    dir2.push("BBBBBBB");
+
+    let mut dir3 = p.target_debug_dir();
+    dir3.push("aaaaaaa");
+
+    let p = p
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            build = "build.rs"
+        "#)
+        .file("build.rs", &format!(r##"
+            fn main() {{
+                println!(r#"cargo:rustc-link-search=native={}"#);
+                println!(r#"cargo:rustc-link-search=native={}"#);
+                println!(r#"cargo:rustc-link-search=native={}"#);
+            }}
+        "##, dir1.display(), dir2.display(), dir3.display()))
+        .file("src/main.rs", &format!(r##"
+            fn main() {{
+                let search_path = std::env::var_os("{}").unwrap();
+                let paths = std::env::split_paths(&search_path).collect::<Vec<_>>();
+                // ASCII case-sensitive sort
+                assert_eq!("BBBBBBB", paths[0].file_name().unwrap().to_string_lossy());
+                assert_eq!("aaaaaaa", paths[1].file_name().unwrap().to_string_lossy());
+                assert_eq!("zzzzzzz", paths[2].file_name().unwrap().to_string_lossy());
+            }}
+        "##, dylib_path_envvar()))
+        .build();
+
+    assert_that(p.cargo("run"), execs().with_status(0));
+}
+
 #[test]
 fn fail_no_extra_verbose() {
     let p = project("foo")