Auto merge of rust-lang#3609 - jmatraszek:build_proper_binary, r=alexcrichton

Fix building multiple binaries that do not have path spacified in Cargo.toml

When multiple binaries are specified in Cargo.toml, the binaries that do not have `path` specified are build from `src/main.rs`. Discovered here: rust-lang-nursery/thanks#40 (comment).

This was caused by setting for a binary a main layout here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L478, which caused `normalize` to not fallback to default binary path here https://github.com/rust-lang/cargo/blob/master/src/cargo/util/toml.rs#L1149 (as `bin.path` was always `Some("/path/to/main.rs")`.

Added a test and fixed this by not using `layout.main()`, so right now for bins without `path` specified we fallback to default path inferred from bin's name (e.g. `src/bin/foo.rs`), test if the file exists and only if it doesn't -- fallback to `src/main.rs`.

I do not have any knowledge about Cargo's design, so I am not sure if this is the proper place to test for file existence.

Author: bors

src/cargo/util/toml.rs

@@ -64,15 +64,6 @@ impl Layout {
             benches: benches,
         }
     }
-
-    fn main(&self) -> Option<&PathBuf> {
-        self.bins.iter().find(|p| {
-            match p.file_name().and_then(|s| s.to_str()) {
-                Some(s) => s == "main.rs",
-                None => false
-            }
-        })
-    }
 }
 
 fn try_add_file(files: &mut Vec<PathBuf>, file: PathBuf) {
@@ -475,22 +466,10 @@ impl TomlManifest {
 
         let bins = match self.bin {
             Some(ref bins) => {
-                let bin = layout.main();
-
                 for target in bins {
                     target.validate_binary_name()?;
-                }
-
-                bins.iter().map(|t| {
-                    if bin.is_some() && t.path.is_none() {
-                        TomlTarget {
-                            path: bin.as_ref().map(|&p| PathValue::Path(p.clone())),
-                            .. t.clone()
-                        }
-                    } else {
-                        t.clone()
-                    }
-                }).collect()
+                };
+                bins.clone()
             }
             None => inferred_bin_targets(&project.name, layout)
         };
@@ -1147,7 +1126,11 @@ fn normalize(package_root: &Path,
                        default: &mut FnMut(&TomlBinTarget) -> PathBuf| {
         for bin in bins.iter() {
             let path = bin.path.clone().unwrap_or_else(|| {
-                PathValue::Path(default(bin))
+                let default_bin_path = PathValue::Path(default(bin));
+                match package_root.join(default_bin_path.to_path()).exists() {
+                    true => default_bin_path, // inferred from bin's name
+                    false => PathValue::Path(Path::new("src").join("main.rs"))
+                }
             });
             let mut target = Target::bin_target(&bin.name(), package_root.join(path.to_path()));
             configure(bin, &mut target);

tests/build.rs

@@ -2769,3 +2769,30 @@ fn build_all_member_dependency_same_name() {
                        [..] Finished dev [unoptimized + debuginfo] target(s) in [..]\n"));
 }
 
+#[test]
+fn run_proper_binary() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            authors = []
+            version = "0.0.0"
+            [[bin]]
+            name = "main"
+            [[bin]]
+            name = "other"
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/bin/main.rs", r#"
+            fn main() {
+                panic!("This should never be run.");
+            }
+        "#)
+        .file("src/bin/other.rs", r#"
+            fn main() {
+            }
+        "#);
+
+    assert_that(p.cargo_process("run").arg("--bin").arg("other"),
+                execs().with_status(0));
+}

tests/metadata.rs

@@ -8,6 +8,7 @@ use cargotest::support::{project, execs, basic_bin_manifest, basic_lib_manifest,
 #[test]
 fn cargo_metadata_simple() {
     let p = project("foo")
+            .file("src/foo.rs", "")
             .file("Cargo.toml", &basic_bin_manifest("foo"));
 
     assert_that(p.cargo_process("metadata"), execs().with_json(r#"
@@ -52,6 +53,7 @@ fn cargo_metadata_simple() {
 #[test]
 fn cargo_metadata_with_deps_and_version() {
     let p = project("foo")
+        .file("src/foo.rs", "")
         .file("Cargo.toml", r#"
             [project]
             name = "foo"