auto merge of #12398 : alexcrichton/rust/rlibs-and-dylibs-2, r=cmr

The new methodology can be found in the re-worded comment, but the gist of it is
that -C prefer-dynamic doesn't turn off static linkage. The error messages
should also be a little more sane now.

Closes #12133

mk/tests.mk

@@ -537,10 +537,6 @@ TEST_SREQ$(1)_T_$(2)_H_$(3) = \
 # remove directive, if present, from CFG_RUSTC_FLAGS (issue #7898).
 CTEST_RUSTC_FLAGS := $$(subst --cfg ndebug,,$$(CFG_RUSTC_FLAGS))
 
-# There's no need our entire test suite to take up gigabytes of space on disk
-# including copies of libstd/libextra all over the place
-CTEST_RUSTC_FLAGS := $$(CTEST_RUSTC_FLAGS) -C prefer-dynamic
-
 # The tests can not be optimized while the rest of the compiler is optimized, so
 # filter out the optimization (if any) from rustc and then figure out if we need
 # to be optimized

src/compiletest/header.rs

@@ -32,6 +32,8 @@ pub struct TestProps {
     force_host: bool,
     // Check stdout for error-pattern output as well as stderr
     check_stdout: bool,
+    // Don't force a --crate-type=dylib flag on the command line
+    no_prefer_dynamic: bool,
 }
 
 // Load any test directives embedded in the file
@@ -45,6 +47,7 @@ pub fn load_props(testfile: &Path) -> TestProps {
     let mut check_lines = ~[];
     let mut force_host = false;
     let mut check_stdout = false;
+    let mut no_prefer_dynamic = false;
     iter_header(testfile, |ln| {
         match parse_error_pattern(ln) {
           Some(ep) => error_patterns.push(ep),
@@ -67,6 +70,10 @@ pub fn load_props(testfile: &Path) -> TestProps {
             check_stdout = parse_check_stdout(ln);
         }
 
+        if !no_prefer_dynamic {
+            no_prefer_dynamic = parse_no_prefer_dynamic(ln);
+        }
+
         match parse_aux_build(ln) {
             Some(ab) => { aux_builds.push(ab); }
             None => {}
@@ -99,6 +106,7 @@ pub fn load_props(testfile: &Path) -> TestProps {
         check_lines: check_lines,
         force_host: force_host,
         check_stdout: check_stdout,
+        no_prefer_dynamic: no_prefer_dynamic,
     };
 }
 
@@ -167,6 +175,10 @@ fn parse_check_stdout(line: &str) -> bool {
     parse_name_directive(line, "check-stdout")
 }
 
+fn parse_no_prefer_dynamic(line: &str) -> bool {
+    parse_name_directive(line, "no-prefer-dynamic")
+}
+
 fn parse_exec_env(line: &str) -> Option<(~str, ~str)> {
     parse_name_value_directive(line, ~"exec-env").map(|nv| {
         // nv is either FOO or FOO=BAR

src/compiletest/runtest.rs

@@ -704,9 +704,13 @@ fn compose_and_run_compiler(
     for rel_ab in props.aux_builds.iter() {
         let abs_ab = config.aux_base.join(rel_ab.as_slice());
         let aux_props = load_props(&abs_ab);
+        let crate_type = if aux_props.no_prefer_dynamic {
+            ~[]
+        } else {
+            ~[~"--crate-type=dylib"]
+        };
         let aux_args =
-            make_compile_args(config, &aux_props, ~[~"--crate-type=dylib"]
-                                                  + extra_link_args,
+            make_compile_args(config, &aux_props, crate_type + extra_link_args,
                               |a,b| {
                                   let f = make_lib_name(a, b, testfile);
                                   ThisDirectory(f.dir_path())
@@ -770,6 +774,10 @@ fn make_compile_args(config: &config,
                      ~"-L", config.build_base.as_str().unwrap().to_owned(),
                      ~"--target=" + target]
         + extras;
+    if !props.no_prefer_dynamic {
+        args.push(~"-C");
+        args.push(~"prefer-dynamic");
+    }
     let path = match xform_file {
         ThisFile(path) => { args.push(~"-o"); path }
         ThisDirectory(path) => { args.push(~"--out-dir"); path }

src/librustc/back/link.rs

@@ -1220,6 +1220,74 @@ fn add_local_native_libraries(args: &mut ~[~str], sess: Session) {
 // the intermediate rlib version)
 fn add_upstream_rust_crates(args: &mut ~[~str], sess: Session,
                             dylib: bool, tmpdir: &Path) {
+
+    // As a limitation of the current implementation, we require that everything
+    // must be static or everything must be dynamic. The reasons for this are a
+    // little subtle, but as with staticlibs and rlibs, the goal is to prevent
+    // duplicate copies of the same library showing up. For example, a static
+    // immediate dependency might show up as an upstream dynamic dependency and
+    // we currently have no way of knowing that. We know that all dynamic
+    // libraries require dynamic dependencies (see above), so it's satisfactory
+    // to include either all static libraries or all dynamic libraries.
+    //
+    // With this limitation, we expose a compiler default linkage type and an
+    // option to reverse that preference. The current behavior looks like:
+    //
+    // * If a dylib is being created, upstream dependencies must be dylibs
+    // * If nothing else is specified, static linking is preferred
+    // * If the -C prefer-dynamic flag is given, dynamic linking is preferred
+    // * If one form of linking fails, the second is also attempted
+    // * If both forms fail, then we emit an error message
+
+    let dynamic = get_deps(sess.cstore, cstore::RequireDynamic);
+    let statik = get_deps(sess.cstore, cstore::RequireStatic);
+    match (dynamic, statik, sess.opts.cg.prefer_dynamic, dylib) {
+        (_, Some(deps), false, false) => {
+            add_static_crates(args, sess, tmpdir, deps)
+        }
+
+        (None, Some(deps), true, false) => {
+            // If you opted in to dynamic linking and we decided to emit a
+            // static output, you should probably be notified of such an event!
+            sess.warn("dynamic linking was preferred, but dependencies \
+                       could not all be found in an dylib format.");
+            sess.warn("linking statically instead, using rlibs");
+            add_static_crates(args, sess, tmpdir, deps)
+        }
+
+        (Some(deps), _, _, _) => add_dynamic_crates(args, sess, deps),
+
+        (None, _, _, true) => {
+            sess.err("dylib output requested, but some depenencies could not \
+                      be found in the dylib format");
+            let deps = sess.cstore.get_used_crates(cstore::RequireDynamic);
+            for (cnum, path) in deps.move_iter() {
+                if path.is_some() { continue }
+                let name = sess.cstore.get_crate_data(cnum).name.clone();
+                sess.note(format!("dylib not found: {}", name));
+            }
+        }
+
+        (None, None, pref, false) => {
+            let (pref, name) = if pref {
+                sess.err("dynamic linking is preferred, but dependencies were \
+                          not found in either dylib or rlib format");
+                (cstore::RequireDynamic, "dylib")
+            } else {
+                sess.err("dependencies were not all found in either dylib or \
+                          rlib format");
+                (cstore::RequireStatic, "rlib")
+            };
+            sess.note(format!("dependencies not found in the `{}` format",
+                              name));
+            for (cnum, path) in sess.cstore.get_used_crates(pref).move_iter() {
+                if path.is_some() { continue }
+                let name = sess.cstore.get_crate_data(cnum).name.clone();
+                sess.note(name);
+            }
+        }
+    }
+
     // Converts a library file-stem into a cc -l argument
     fn unlib(config: @session::Config, stem: &str) -> ~str {
         if stem.starts_with("lib") &&
@@ -1230,96 +1298,82 @@ fn add_upstream_rust_crates(args: &mut ~[~str], sess: Session,
         }
     }
 
-    let cstore = sess.cstore;
-    if !dylib && !sess.opts.cg.prefer_dynamic {
-        // With an executable, things get a little interesting. As a limitation
-        // of the current implementation, we require that everything must be
-        // static or everything must be dynamic. The reasons for this are a
-        // little subtle, but as with the above two cases, the goal is to
-        // prevent duplicate copies of the same library showing up. For example,
-        // a static immediate dependency might show up as an upstream dynamic
-        // dependency and we currently have no way of knowing that. We know that
-        // all dynamic libraries require dynamic dependencies (see above), so
-        // it's satisfactory to include either all static libraries or all
-        // dynamic libraries.
-        let crates = cstore.get_used_crates(cstore::RequireStatic);
+    // Attempts to find all dependencies with a certain linkage preference,
+    // returning `None` if not all libraries could be found with that
+    // preference.
+    fn get_deps(cstore: &cstore::CStore,  preference: cstore::LinkagePreference)
+            -> Option<~[(ast::CrateNum, Path)]>
+    {
+        let crates = cstore.get_used_crates(preference);
         if crates.iter().all(|&(_, ref p)| p.is_some()) {
-            for (cnum, path) in crates.move_iter() {
-                let cratepath = path.unwrap();
-
-                // When performing LTO on an executable output, all of the
-                // bytecode from the upstream libraries has already been
-                // included in our object file output. We need to modify all of
-                // the upstream archives to remove their corresponding object
-                // file to make sure we don't pull the same code in twice.
-                //
-                // We must continue to link to the upstream archives to be sure
-                // to pull in native static dependencies. As the final caveat,
-                // on linux it is apparently illegal to link to a blank archive,
-                // so if an archive no longer has any object files in it after
-                // we remove `lib.o`, then don't link against it at all.
-                //
-                // If we're not doing LTO, then our job is simply to just link
-                // against the archive.
-                if sess.lto() {
-                    let name = sess.cstore.get_crate_data(cnum).name.clone();
-                    time(sess.time_passes(), format!("altering {}.rlib", name),
-                         (), |()| {
-                        let dst = tmpdir.join(cratepath.filename().unwrap());
-                        match fs::copy(&cratepath, &dst) {
-                            Ok(..) => {}
-                            Err(e) => {
-                                sess.err(format!("failed to copy {} to {}: {}",
-                                                 cratepath.display(),
-                                                 dst.display(),
-                                                 e));
-                                sess.abort_if_errors();
-                            }
-                        }
-                        let dst_str = dst.as_str().unwrap().to_owned();
-                        let mut archive = Archive::open(sess, dst);
-                        archive.remove_file(format!("{}.o", name));
-                        let files = archive.files();
-                        if files.iter().any(|s| s.ends_with(".o")) {
-                            args.push(dst_str);
+            Some(crates.move_iter().map(|(a, b)| (a, b.unwrap())).collect())
+        } else {
+            None
+        }
+    }
+
+    // Adds the static "rlib" versions of all crates to the command line.
+    fn add_static_crates(args: &mut ~[~str], sess: Session, tmpdir: &Path,
+                         crates: ~[(ast::CrateNum, Path)]) {
+        for (cnum, cratepath) in crates.move_iter() {
+            // When performing LTO on an executable output, all of the
+            // bytecode from the upstream libraries has already been
+            // included in our object file output. We need to modify all of
+            // the upstream archives to remove their corresponding object
+            // file to make sure we don't pull the same code in twice.
+            //
+            // We must continue to link to the upstream archives to be sure
+            // to pull in native static dependencies. As the final caveat,
+            // on linux it is apparently illegal to link to a blank archive,
+            // so if an archive no longer has any object files in it after
+            // we remove `lib.o`, then don't link against it at all.
+            //
+            // If we're not doing LTO, then our job is simply to just link
+            // against the archive.
+            if sess.lto() {
+                let name = sess.cstore.get_crate_data(cnum).name.clone();
+                time(sess.time_passes(), format!("altering {}.rlib", name),
+                     (), |()| {
+                    let dst = tmpdir.join(cratepath.filename().unwrap());
+                    match fs::copy(&cratepath, &dst) {
+                        Ok(..) => {}
+                        Err(e) => {
+                            sess.err(format!("failed to copy {} to {}: {}",
+                                             cratepath.display(),
+                                             dst.display(),
+                                             e));
+                            sess.abort_if_errors();
                         }
-                    });
-                } else {
-                    args.push(cratepath.as_str().unwrap().to_owned());
-                }
+                    }
+                    let dst_str = dst.as_str().unwrap().to_owned();
+                    let mut archive = Archive::open(sess, dst);
+                    archive.remove_file(format!("{}.o", name));
+                    let files = archive.files();
+                    if files.iter().any(|s| s.ends_with(".o")) {
+                        args.push(dst_str);
+                    }
+                });
+            } else {
+                args.push(cratepath.as_str().unwrap().to_owned());
             }
-            return;
         }
     }
 
-    // If we're performing LTO, then it should have been previously required
-    // that all upstream rust dependencies were available in an rlib format.
-    assert!(!sess.lto());
-
-    // This is a fallback of three different  cases of linking:
-    //
-    // * When creating a dynamic library, all inputs are required to be dynamic
-    //   as well
-    // * If an executable is created with a preference on dynamic linking, then
-    //   this case is the fallback
-    // * If an executable is being created, and one of the inputs is missing as
-    //   a static library, then this is the fallback case.
-    let crates = cstore.get_used_crates(cstore::RequireDynamic);
-    for &(cnum, ref path) in crates.iter() {
-        let cratepath = match *path {
-            Some(ref p) => p.clone(),
-            None => {
-                sess.err(format!("could not find dynamic library for: `{}`",
-                                 sess.cstore.get_crate_data(cnum).name));
-                return
-            }
-        };
-        // Just need to tell the linker about where the library lives and what
-        // its name is
-        let dir = cratepath.dirname_str().unwrap();
-        if !dir.is_empty() { args.push("-L" + dir); }
-        let libarg = unlib(sess.targ_cfg, cratepath.filestem_str().unwrap());
-        args.push("-l" + libarg);
+    // Same thing as above, but for dynamic crates instead of static crates.
+    fn add_dynamic_crates(args: &mut ~[~str], sess: Session,
+                          crates: ~[(ast::CrateNum, Path)]) {
+        // If we're performing LTO, then it should have been previously required
+        // that all upstream rust dependencies were available in an rlib format.
+        assert!(!sess.lto());
+
+        for (_, cratepath) in crates.move_iter() {
+            // Just need to tell the linker about where the library lives and
+            // what its name is
+            let dir = cratepath.dirname_str().unwrap();
+            if !dir.is_empty() { args.push("-L" + dir); }
+            let libarg = unlib(sess.targ_cfg, cratepath.filestem_str().unwrap());
+            args.push("-l" + libarg);
+        }
     }
 }
 

src/test/auxiliary/issue-12133-dylib.rs

@@ -0,0 +1,13 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// no-prefer-dynamic
+
+#[crate_type = "dylib"];

src/test/auxiliary/issue-12133-rlib.rs

@@ -0,0 +1,13 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// no-prefer-dynamic
+
+#[crate_type = "rlib"];

src/test/compile-fail/issue-12133-1.rs

@@ -0,0 +1,19 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// aux-build:issue-12133-rlib.rs
+// aux-build:issue-12133-dylib.rs
+
+// error-pattern: dynamic linking is preferred, but dependencies were not found
+
+extern crate a = "issue-12133-rlib";
+extern crate b = "issue-12133-dylib";
+
+fn main() {}

src/test/compile-fail/issue-12133-2.rs

@@ -0,0 +1,20 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// aux-build:issue-12133-rlib.rs
+// aux-build:issue-12133-dylib.rs
+// no-prefer-dynamic
+
+// error-pattern: dependencies were not all found in either dylib or rlib format
+
+extern crate a = "issue-12133-rlib";
+extern crate b = "issue-12133-dylib";
+
+fn main() {}