Auto merge of #1443 - alexcrichton:hyphens-to-underscores, r=brson

This change allows *packages* to have hyphens in them, but they are always
translated to underscores when translated to a crate name. This means that all
crates are compiled with a `--crate-name` that has no hyphens (as well as
`--extern` directives having no hyphens).

Binaries and examples, however, are allowed to contain
hyphens in their name. The "crate name" will still have an underscore, but the
output will be in the same dasherized name.

Explicitly named targets are not allowed to have hyphens in them as well.

src/cargo/core/manifest.rs

@@ -322,6 +322,7 @@ impl Target {
     }
 
     pub fn name(&self) -> &str { &self.name }
+    pub fn crate_name(&self) -> String { self.name.replace("-", "_") }
     pub fn src_path(&self) -> &Path { &self.src_path }
     pub fn metadata(&self) -> Option<&Metadata> { self.metadata.as_ref() }
     pub fn kind(&self) -> &TargetKind { &self.kind }
@@ -332,6 +333,10 @@ impl Target {
     pub fn for_host(&self) -> bool { self.for_host }
     pub fn benched(&self) -> bool { self.benched }
 
+    pub fn allows_underscores(&self) -> bool {
+        self.is_bin() || self.is_example() || self.is_custom_build()
+    }
+
     pub fn is_lib(&self) -> bool {
         match self.kind {
             TargetKind::Lib(_) => true,

src/cargo/ops/cargo_rustc/context.rs

@@ -282,9 +282,10 @@ impl<'a, 'b: 'a> Context<'a, 'b> {
     /// Returns the file stem for a given target/profile combo
     pub fn file_stem(&self, target: &Target, profile: &Profile) -> String {
         match self.target_metadata(target, profile) {
-            Some(ref metadata) => format!("{}{}", target.name(),
+            Some(ref metadata) => format!("{}{}", target.crate_name(),
                                           metadata.extra_filename),
-            None => target.name().to_string(),
+            None if target.allows_underscores() => target.name().to_string(),
+            None => target.crate_name().to_string(),
         }
     }
 

src/cargo/ops/cargo_rustc/custom_build.rs

@@ -46,9 +46,7 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform,
     };
 
     // Building the command to execute
-    let profile = cx.build_script_profile(pkg.package_id());
-    let to_exec = try!(cx.target_filenames(target, profile))[0].clone();
-    let to_exec = script_output.join(&to_exec);
+    let to_exec = script_output.join(target.name());
 
     // Start preparing the process to execute, starting out with some
     // environment variables. Note that the profile-related environment

src/cargo/ops/cargo_rustc/mod.rs

@@ -328,9 +328,15 @@ fn rustc(package: &Package, target: &Target, profile: &Profile,
         let pass_l_flag = target.is_lib() || !package.targets().iter().any(|t| {
             t.is_lib()
         });
+        let do_rename = target.allows_underscores() && !profile.test;
+        let real_name = target.name().to_string();
+        let crate_name = target.crate_name();
 
-        let rustc_dep_info_loc = root.join(&cx.file_stem(target, profile))
-                                     .with_extension("d");
+        let rustc_dep_info_loc = if do_rename {
+            root.join(&crate_name)
+        } else {
+            root.join(&cx.file_stem(target, profile))
+        }.with_extension("d");
         let dep_info_loc = fingerprint::dep_info_loc(cx, package, target,
                                                      profile, kind);
         let cwd = cx.config.cwd().to_path_buf();
@@ -363,7 +369,20 @@ fn rustc(package: &Package, target: &Target, profile: &Profile,
                 human(format!("Could not compile `{}`.", name))
             }));
 
-            try!(fs::rename(&rustc_dep_info_loc, &dep_info_loc));
+            if do_rename && real_name != crate_name {
+                let dst = root.join(&filenames[0]);
+                let src = dst.with_file_name(dst.file_name().unwrap()
+                                                .to_str().unwrap()
+                                                .replace(&real_name, &crate_name));
+                try!(fs::rename(&src, &dst).chain_error(|| {
+                    human(format!("could not rename crate {:?}", src))
+                }));
+            }
+
+            try!(fs::rename(&rustc_dep_info_loc, &dep_info_loc).chain_error(|| {
+                human(format!("could not rename dep info: {:?}",
+                              rustc_dep_info_loc))
+            }));
             try!(fingerprint::append_current_dir(&dep_info_loc, &cwd));
 
             Ok(())
@@ -482,7 +501,7 @@ fn rustdoc(package: &Package, target: &Target, profile: &Profile,
     rustdoc.arg(&root_path(cx, package, target))
            .cwd(cx.config.cwd())
            .arg("-o").arg(&cx_root)
-           .arg("--crate-name").arg(target.name());
+           .arg("--crate-name").arg(&target.crate_name());
 
     match cx.resolve.features(package.package_id()) {
         Some(features) => {
@@ -565,7 +584,7 @@ fn build_base_args(cx: &Context,
     // TODO: Handle errors in converting paths into args
     cmd.arg(&root_path(cx, pkg, target));
 
-    cmd.arg("--crate-name").arg(target.name());
+    cmd.arg("--crate-name").arg(&target.crate_name());
 
     for crate_type in crate_types.iter() {
         cmd.arg("--crate-type").arg(crate_type);
@@ -696,7 +715,7 @@ fn build_deps_args(cmd: &mut CommandPrototype,
         for filename in try!(cx.target_filenames(target, profile)).iter() {
             if filename.ends_with(".a") { continue }
             let mut v = OsString::new();
-            v.push(target.name());
+            v.push(&target.crate_name());
             v.push("=");
             v.push(layout.root());
             v.push(&path::MAIN_SEPARATOR.to_string());

src/cargo/util/toml.rs

@@ -395,16 +395,17 @@ impl TomlManifest {
                     ManyOrOne::Many(..) => used_deprecated_lib = true,
                     _ => {}
                 }
-                libs.as_slice().iter().map(|t| {
-                    if layout.lib.is_some() && t.path.is_none() {
+                try!(libs.as_slice().iter().map(|t| {
+                    try!(validate_target_name(t));
+                    Ok(if layout.lib.is_some() && t.path.is_none() {
                         TomlTarget {
                             path: layout.lib.as_ref().map(|p| PathValue::Path(p.clone())),
                             .. t.clone()
                         }
                     } else {
                         t.clone()
-                    }
-                }).collect()
+                    })
+                }).collect::<CargoResult<Vec<_>>>())
             }
             None => inferred_lib_target(&project.name, layout),
         };
@@ -413,34 +414,48 @@ impl TomlManifest {
             Some(ref bins) => {
                 let bin = layout.main();
 
-                bins.iter().map(|t| {
-                    if bin.is_some() && t.path.is_none() {
+                try!(bins.iter().map(|t| {
+                    try!(validate_target_name(t));
+                    Ok(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()
+                    })
+                }).collect::<CargoResult<Vec<_>>>())
             }
             None => inferred_bin_targets(&project.name, layout)
         };
 
         let examples = match self.example {
-            Some(ref examples) => examples.clone(),
+            Some(ref examples) => {
+                for example in examples {
+                    try!(validate_target_name(example));
+                }
+                examples.clone()
+            }
             None => inferred_example_targets(layout),
         };
 
         let tests = match self.test {
-            Some(ref tests) => tests.clone(),
+            Some(ref tests) => {
+                for test in tests {
+                    try!(validate_target_name(test));
+                }
+                tests.clone()
+            }
             None => inferred_test_targets(layout),
         };
 
         let benches = if self.bench.is_none() || self.bench.as_ref().unwrap().is_empty() {
             inferred_bench_targets(layout)
         } else {
-            self.bench.as_ref().unwrap().iter().map(|t| t.clone()).collect()
+            try!(self.bench.as_ref().unwrap().iter().map(|t| {
+                try!(validate_target_name(t));
+                Ok(t.clone())
+            }).collect::<CargoResult<Vec<_>>>())
         };
 
         // processing the custom build script
@@ -529,6 +544,15 @@ impl TomlManifest {
     }
 }
 
+fn validate_target_name(target: &TomlTarget) -> CargoResult<()> {
+    if target.name.contains("-") {
+        Err(human(format!("target names cannot contain hyphens: {}",
+                          target.name)))
+    } else {
+        Ok(())
+    }
+}
+
 fn process_dependencies<F>(cx: &mut Context,
                            new_deps: Option<&HashMap<String, TomlDependency>>,
                            mut f: F) -> CargoResult<()>

tests/test_cargo_compile.rs

@@ -1653,3 +1653,38 @@ test!(cyclic_deps_rejected {
 cyclic package dependency: package `foo v0.0.1 ([..])` depends on itself
 "));
 });
+
+test!(dashes_to_underscores {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo-bar"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/main.rs", "extern crate foo_bar; fn main() {}");
+
+    assert_that(p.cargo_process("build").arg("-v"),
+                execs().with_status(0));
+    assert_that(&p.root().join("target/debug/foo-bar"),
+                existing_file());
+});
+
+test!(dashes_in_crate_name_bad {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [lib]
+            name = "foo-bar"
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/main.rs", "extern crate foo_bar; fn main() {}");
+
+    assert_that(p.cargo_process("build").arg("-v"),
+                execs().with_status(101));
+});

tests/test_cargo_compile_custom_build.rs

@@ -33,7 +33,7 @@ test!(custom_build_script_failed {
                 execs().with_status(101)
                        .with_stdout(format!("\
 {compiling} foo v0.5.0 ({url})
-{running} `rustc build.rs --crate-name build-script-build --crate-type bin [..]`
+{running} `rustc build.rs --crate-name build_script_build --crate-type bin [..]`
 {running} `[..]build-script-build[..]`
 ",
 url = p.url(), compiling = COMPILING, running = RUNNING))
@@ -760,7 +760,7 @@ test!(build_cmd_with_a_build_cmd {
     --out-dir [..]target[..]deps --emit=dep-info,link \
     -L [..]target[..]deps -L [..]target[..]deps`
 {compiling} foo v0.5.0 (file://[..])
-{running} `rustc build.rs --crate-name build-script-build --crate-type bin \
+{running} `rustc build.rs --crate-name build_script_build --crate-type bin \
     -C prefer-dynamic -g \
     --out-dir [..]build[..]foo-[..] --emit=dep-info,link \
     -L [..]target[..]debug -L [..]target[..]deps \