Auto merge of #4616 - kennytm:fix-4490, r=alexcrichton

Uplift *.dSYM

Fixed #4490.

The solution is based on #4570. Simply adding `.dSYM` into `add_target_specific_suffixes` will cause cargo trying to actually run that `.dSYM` folder, so I've upgraded the `linkable` boolean into a 3-value enum `TargetFileType`, to tell `cargo run` and `cargo test` to avoid these debug symbol files.

(I haven't checked if it can solve #4056, don't wanna mess with Spotlight 😝)

Author: bors

src/cargo/ops/cargo_rustc/context.rs

@@ -60,6 +60,17 @@ pub struct Unit<'a> {
     pub kind: Kind,
 }
 
+/// Type of each file generated by a Unit.
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+pub enum TargetFileType {
+    /// Not a special file type.
+    Normal,
+    /// It is something you can link against (e.g. a library)
+    Linkable,
+    /// It is a piece of external debug information (e.g. *.dSYM and *.pdb)
+    DebugInfo,
+}
+
 /// The build context, containing all information about a build task
 pub struct Context<'a, 'cfg: 'a> {
     /// The workspace the build is for
@@ -96,8 +107,8 @@ pub struct Context<'a, 'cfg: 'a> {
     ///  - File name that will be produced by the build process (in `deps`)
     ///  - If it should be linked into `target`, and what it should be called (e.g. without
     ///    metadata).
-    ///  - Whether it is something you can link against (e.g. a library)
-    target_filenames: HashMap<Unit<'a>, Arc<Vec<(PathBuf, Option<PathBuf>, bool)>>>,
+    ///  - Type of the file (library / debug symbol / else)
+    target_filenames: HashMap<Unit<'a>, Arc<Vec<(PathBuf, Option<PathBuf>, TargetFileType)>>>,
     target_metadatas: HashMap<Unit<'a>, Option<Metadata>>,
 }
 
@@ -649,7 +660,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
     ///  - link_dst: Optional file to link/copy the result to (without metadata suffix)
     ///  - linkable: Whether possible to link against file (eg it's a library)
     pub fn target_filenames(&mut self, unit: &Unit<'a>)
-                            -> CargoResult<Arc<Vec<(PathBuf, Option<PathBuf>, bool)>>> {
+                            -> CargoResult<Arc<Vec<(PathBuf, Option<PathBuf>, TargetFileType)>>> {
         if let Some(cache) = self.target_filenames.get(unit) {
             return Ok(Arc::clone(cache))
         }
@@ -662,7 +673,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
     }
 
     fn calc_target_filenames(&mut self, unit: &Unit<'a>)
-                            -> CargoResult<Arc<Vec<(PathBuf, Option<PathBuf>, bool)>>> {
+                            -> CargoResult<Arc<Vec<(PathBuf, Option<PathBuf>, TargetFileType)>>> {
         let out_dir = self.out_dir(unit);
         let stem = self.file_stem(unit);
         let link_stem = self.link_stem(unit);
@@ -680,9 +691,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                 let link_dst = link_stem.clone().map(|(ld, ls)| {
                     ld.join(format!("lib{}.rmeta", ls))
                 });
-                ret.push((filename, link_dst, true));
+                ret.push((filename, link_dst, TargetFileType::Linkable));
             } else {
-                let mut add = |crate_type: &str, linkable: bool| -> CargoResult<()> {
+                let mut add = |crate_type: &str, file_type: TargetFileType| -> CargoResult<()> {
                     let crate_type = if crate_type == "lib" {"rlib"} else {crate_type};
                     let mut crate_types = info.crate_types.borrow_mut();
                     let entry = crate_types.entry(crate_type.to_string());
@@ -698,10 +709,11 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                             let suffixes = add_target_specific_suffixes(
                                 &self.target_triple(),
                                 &crate_type,
+                                unit.target.kind(),
                                 suffix,
-                                linkable,
+                                file_type,
                             );
-                            for (suffix, linkable, should_replace_hyphens) in suffixes {
+                            for (suffix, file_type, should_replace_hyphens) in suffixes {
                                 // wasm bin target will generate two files in deps such as
                                 // "web-stuff.js" and "web_stuff.wasm". Note the different usages of
                                 // "-" and "_". should_replace_hyphens is a flag to indicate that
@@ -717,7 +729,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                                 let link_dst = link_stem.clone().map(|(ld, ls)| {
                                     ld.join(format!("{}{}{}", prefix, conv(ls), suffix))
                                 });
-                                ret.push((filename, link_dst, linkable));
+                                ret.push((filename, link_dst, file_type));
                             }
                             Ok(())
                         }
@@ -735,17 +747,21 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                     TargetKind::ExampleBin |
                     TargetKind::Bench |
                     TargetKind::Test => {
-                        add("bin", false)?;
+                        add("bin", TargetFileType::Normal)?;
                     }
                     TargetKind::Lib(..) |
                     TargetKind::ExampleLib(..)
                     if unit.profile.test => {
-                        add("bin", false)?;
+                        add("bin", TargetFileType::Normal)?;
                     }
                     TargetKind::ExampleLib(ref kinds) |
                     TargetKind::Lib(ref kinds) => {
                         for kind in kinds {
-                            add(kind.crate_type(), kind.linkable())?;
+                            add(kind.crate_type(), if kind.linkable() {
+                                TargetFileType::Linkable
+                            } else {
+                                TargetFileType::Normal
+                            })?;
                         }
                     }
                 }
@@ -1257,30 +1273,40 @@ fn parse_crate_type(
 }
 
 // (not a rustdoc)
-// Return a list of 3-tuples (suffix, linkable, should_replace_hyphens).
+// Return a list of 3-tuples (suffix, file_type, should_replace_hyphens).
 //
 // should_replace_hyphens will be used by the caller to replace "-" with "_"
 // in a bin_stem. See the caller side (calc_target_filenames()) for details.
 fn add_target_specific_suffixes(
     target_triple: &str,
     crate_type: &str,
+    target_kind: &TargetKind,
     suffix: &str,
-    linkable: bool,
-) -> Vec<(String, bool, bool)> {
-    let mut ret = vec![(suffix.to_string(), linkable, false)];
+    file_type: TargetFileType,
+) -> Vec<(String, TargetFileType, bool)> {
+    let mut ret = vec![(suffix.to_string(), file_type, false)];
 
     // rust-lang/cargo#4500
     if target_triple.ends_with("pc-windows-msvc") && crate_type.ends_with("dylib") &&
         suffix == ".dll"
     {
-        ret.push((".dll.lib".to_string(), false, false));
+        ret.push((".dll.lib".to_string(), TargetFileType::Normal, false));
     }
 
     // rust-lang/cargo#4535
     if target_triple.starts_with("wasm32-") && crate_type == "bin" &&
         suffix == ".js"
     {
-        ret.push((".wasm".to_string(), false, true));
+        ret.push((".wasm".to_string(), TargetFileType::Normal, true));
+    }
+
+    // rust-lang/cargo#4490
+    //  - only uplift *.dSYM for binaries.
+    //    tests are run directly from target/debug/deps/
+    //    and examples are inside target/debug/examples/ which already have *.dSYM next to them
+    //    so no need to do anything.
+    if target_triple.contains("-apple-") && *target_kind == TargetKind::Bin {
+        ret.push((".dSYM".to_string(), TargetFileType::DebugInfo, false));
     }
 
     ret

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -18,7 +18,7 @@ use util::errors::{CargoResult, CargoResultExt};
 use util::paths;
 
 use super::job::Work;
-use super::context::{Context, Unit};
+use super::context::{Context, Unit, TargetFileType};
 use super::custom_build::BuildDeps;
 
 /// A tuple result of the `prepare_foo` functions in this module.
@@ -87,7 +87,10 @@ pub fn prepare_target<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
         missing_outputs = !root.join(unit.target.crate_name())
                                .join("index.html").exists();
     } else {
-        for &(ref src, ref link_dst, _) in cx.target_filenames(unit)?.iter() {
+        for &(ref src, ref link_dst, file_type) in cx.target_filenames(unit)?.iter() {
+            if file_type == TargetFileType::DebugInfo {
+                continue;
+            }
             missing_outputs |= !src.exists();
             if let Some(ref link_dst) = *link_dst {
                 missing_outputs |= !link_dst.exists();

src/cargo/ops/cargo_rustc/mod.rs

@@ -23,7 +23,7 @@ use self::job_queue::JobQueue;
 use self::output_depinfo::output_depinfo;
 
 pub use self::compilation::Compilation;
-pub use self::context::{Context, Unit};
+pub use self::context::{Context, Unit, TargetFileType};
 pub use self::custom_build::{BuildOutput, BuildMap, BuildScripts};
 pub use self::layout::is_bad_artifact_name;
 
@@ -179,7 +179,11 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
     queue.execute(&mut cx)?;
 
     for unit in units.iter() {
-        for &(ref dst, ref link_dst, _) in cx.target_filenames(unit)?.iter() {
+        for &(ref dst, ref link_dst, file_type) in cx.target_filenames(unit)?.iter() {
+            if file_type == TargetFileType::DebugInfo {
+                continue;
+            }
+
             let bindst = match *link_dst {
                 Some(ref link_dst) => link_dst,
                 None => dst,
@@ -505,7 +509,7 @@ fn link_targets<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
         // above. This means that `cargo build` will produce binaries in
         // `target/debug` which one probably expects.
         let mut destinations = vec![];
-        for &(ref src, ref link_dst, _linkable) in filenames.iter() {
+        for &(ref src, ref link_dst, _file_type) in filenames.iter() {
             // This may have been a `cargo rustc` command which changes the
             // output, so the source may not actually exist.
             if !src.exists() {
@@ -887,8 +891,8 @@ fn build_deps_args<'a, 'cfg>(cmd: &mut ProcessBuilder,
     fn link_to<'a, 'cfg>(cmd: &mut ProcessBuilder,
                          cx: &mut Context<'a, 'cfg>,
                          unit: &Unit<'a>) -> CargoResult<()> {
-        for &(ref dst, _, ref linkable) in cx.target_filenames(unit)?.iter() {
-            if !*linkable {
+        for &(ref dst, _, file_type) in cx.target_filenames(unit)?.iter() {
+            if file_type != TargetFileType::Linkable {
                 continue
             }
             let mut v = OsString::new();

tests/build.rs

@@ -15,7 +15,7 @@ use cargotest::support::paths::{CargoPathExt,root};
 use cargotest::support::{ProjectBuilder};
 use cargotest::support::{project, execs, main_file, basic_bin_manifest};
 use cargotest::support::registry::Package;
-use hamcrest::{assert_that, existing_file, is_not};
+use hamcrest::{assert_that, existing_file, existing_dir, is_not};
 use tempdir::TempDir;
 
 #[test]
@@ -2718,6 +2718,9 @@ fn compiler_json_error_format() {
             version = "0.5.0"
             authors = ["[email protected]"]
 
+            [profile.dev]
+            debug = false   # prevent the *.dSYM from affecting the test result
+
             [dependencies.bar]
             path = "bar"
         "#)
@@ -2751,7 +2754,7 @@ fn compiler_json_error_format() {
         "reason":"compiler-artifact",
         "profile": {
             "debug_assertions": true,
-            "debuginfo": 2,
+            "debuginfo": null,
             "opt_level": "0",
             "overflow_checks": true,
             "test": false
@@ -2791,7 +2794,7 @@ fn compiler_json_error_format() {
         },
         "profile": {
             "debug_assertions": true,
-            "debuginfo": 2,
+            "debuginfo": null,
             "opt_level": "0",
             "overflow_checks": true,
             "test": false
@@ -2811,7 +2814,7 @@ fn compiler_json_error_format() {
         "reason":"compiler-artifact",
         "profile": {
             "debug_assertions": true,
-            "debuginfo": 2,
+            "debuginfo": null,
             "opt_level": "0",
             "overflow_checks": true,
             "test": false
@@ -2839,7 +2842,7 @@ fn compiler_json_error_format() {
         },
         "profile": {
             "debug_assertions": true,
-            "debuginfo": 2,
+            "debuginfo": null,
             "opt_level": "0",
             "overflow_checks": true,
             "test": false
@@ -2871,7 +2874,7 @@ fn message_format_json_forward_stderr() {
         .file("src/main.rs", "fn main() { let unused = 0; }")
         .build();
 
-    assert_that(p.cargo("rustc").arg("--bin").arg("foo")
+    assert_that(p.cargo("rustc").arg("--release").arg("--bin").arg("foo")
                 .arg("--message-format").arg("JSON"),
                 execs().with_status(0)
                 .with_json(r#"
@@ -2897,10 +2900,10 @@ fn message_format_json_forward_stderr() {
             "src_path":"[..]"
         },
         "profile":{
-            "debug_assertions":true,
-            "debuginfo":2,
-            "opt_level":"0",
-            "overflow_checks": true,
+            "debug_assertions":false,
+            "debuginfo":null,
+            "opt_level":"3",
+            "overflow_checks": false,
             "test":false
         },
         "features":[],
@@ -3874,3 +3877,28 @@ fn building_a_dependent_crate_witout_bin_should_fail() {
                     "[..]can't find `a_bin` bin, specify bin.path"
                 ));
 }
+
+#[cfg(any(target_os = "macos", target_os = "ios"))]
+#[test]
+fn uplift_dsym_of_bin_on_mac() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.1.0"
+        "#)
+        .file("src/main.rs", "fn main() { panic!(); }")
+        .file("src/bin/b.rs", "fn main() { panic!(); }")
+        .file("examples/c.rs", "fn main() { panic!(); }")
+        .file("tests/d.rs", "fn main() { panic!(); }")
+        .build();
+
+    assert_that(
+        p.cargo("build").arg("--bins").arg("--examples").arg("--tests"),
+        execs().with_status(0)
+    );
+    assert_that(&p.bin("foo.dSYM"), existing_dir());
+    assert_that(&p.bin("b.dSYM"), existing_dir());
+    assert_that(&p.bin("c.dSYM"), is_not(existing_dir()));
+    assert_that(&p.bin("d.dSYM"), is_not(existing_dir()));
+}