Auto merge of #12783 - dvdhrm:pr/libname2, r=ehuss

cargo: prevent dashes in lib.name

The TOML parser of Cargo currently refuses `lib.name` entries that contain dashes. Unfortunately, it uses the package-name as default if no explicit `lib.name` entry is specified. This package-name, however, can contain dashes.

Cargo documentation states that the package name is converted first, yet this was never implemented by the code-base.

Fix this inconsistency and convert the package name to a suitable crate-name first.

This fixes #12780. It is an alternative to #12640.

src/cargo/core/compiler/artifact.rs

@@ -29,15 +29,39 @@ pub fn get_env(
             let path = artifact_path.parent().expect("parent dir for artifacts");
             env.insert(var, path.to_owned().into());
 
-            let var = format!(
+            let var_file = format!(
                 "CARGO_{}_FILE_{}_{}",
                 artifact_type_upper,
                 dep_name_upper,
                 unit_dep.unit.target.name()
             );
-            env.insert(var, artifact_path.to_owned().into());
 
-            if unit_dep.unit.target.name() == dep_name.as_str() {
+            // In older releases, lib-targets defaulted to the name of the package. Newer releases
+            // use the same name as default, but with dashes replaced. Hence, if the name of the
+            // target was inferred by Cargo, we also set the env-var with the unconverted name for
+            // backwards compatibility.
+            let need_compat = unit_dep.unit.target.is_lib() && unit_dep.unit.target.name_inferred();
+            if need_compat {
+                let var_compat = format!(
+                    "CARGO_{}_FILE_{}_{}",
+                    artifact_type_upper,
+                    dep_name_upper,
+                    unit_dep.unit.pkg.name(),
+                );
+                if var_compat != var_file {
+                    env.insert(var_compat, artifact_path.to_owned().into());
+                }
+            }
+
+            env.insert(var_file, artifact_path.to_owned().into());
+
+            // If the name of the target matches the name of the dependency, we strip the
+            // repetition and provide the simpler env-var as well.
+            // For backwards-compatibility of inferred names, we compare against the name of the
+            // package as well, since that used to be the default for library targets.
+            if unit_dep.unit.target.name() == dep_name.as_str()
+                || (need_compat && unit_dep.unit.pkg.name() == dep_name.as_str())
+            {
                 let var = format!("CARGO_{}_FILE_{}", artifact_type_upper, dep_name_upper,);
                 env.insert(var, artifact_path.to_owned().into());
             }

src/cargo/core/manifest.rs

@@ -213,6 +213,8 @@ pub struct Target {
 struct TargetInner {
     kind: TargetKind,
     name: String,
+    // Whether the name was inferred by Cargo, or explicitly given.
+    name_inferred: bool,
     // Note that `bin_name` is used for the cargo-feature `different_binary_name`
     bin_name: Option<String>,
     // Note that the `src_path` here is excluded from the `Hash` implementation
@@ -372,6 +374,7 @@ compact_debug! {
             [debug_the_fields(
                 kind
                 name
+                name_inferred
                 bin_name
                 src_path
                 required_features
@@ -679,6 +682,7 @@ impl Target {
             inner: Arc::new(TargetInner {
                 kind: TargetKind::Bin,
                 name: String::new(),
+                name_inferred: false,
                 bin_name: None,
                 src_path,
                 required_features: None,
@@ -812,6 +816,9 @@ impl Target {
     pub fn name(&self) -> &str {
         &self.inner.name
     }
+    pub fn name_inferred(&self) -> bool {
+        self.inner.name_inferred
+    }
     pub fn crate_name(&self) -> String {
         self.name().replace("-", "_")
     }
@@ -986,6 +993,10 @@ impl Target {
         Arc::make_mut(&mut self.inner).name = name.to_string();
         self
     }
+    pub fn set_name_inferred(&mut self, inferred: bool) -> &mut Target {
+        Arc::make_mut(&mut self.inner).name_inferred = inferred;
+        self
+    }
     pub fn set_binary_name(&mut self, bin_name: Option<String>) -> &mut Target {
         Arc::make_mut(&mut self.inner).bin_name = bin_name;
         self

src/cargo/util/toml/targets.rs

@@ -156,18 +156,20 @@ fn clean_lib(
     let lib = match toml_lib {
         Some(lib) => {
             if let Some(ref name) = lib.name {
-                // XXX: other code paths dodge this validation
                 if name.contains('-') {
                     anyhow::bail!("library target names cannot contain hyphens: {}", name)
                 }
             }
             Some(TomlTarget {
-                name: lib.name.clone().or_else(|| Some(package_name.to_owned())),
+                name: lib
+                    .name
+                    .clone()
+                    .or_else(|| Some(package_name.replace("-", "_"))),
                 ..lib.clone()
             })
         }
         None => inferred.as_ref().map(|lib| TomlTarget {
-            name: Some(package_name.to_string()),
+            name: Some(package_name.replace("-", "_")),
             path: Some(PathValue(lib.clone())),
             ..TomlTarget::new()
         }),
@@ -262,6 +264,7 @@ fn clean_lib(
 
     let mut target = Target::lib_target(name_or_panic(lib), crate_types, path, edition);
     configure(lib, &mut target)?;
+    target.set_name_inferred(toml_lib.map_or(true, |v| v.name.is_none()));
     Ok(Some(target))
 }
 

tests/testsuite/artifact_dep.rs

@@ -841,8 +841,10 @@ fn lib_with_selected_dashed_bin_artifact_and_lib_true() {
                 let _b = include_bytes!(env!("CARGO_BIN_FILE_BAR_BAZ_baz-suffix"));
                 let _b = include_bytes!(env!("CARGO_STATICLIB_FILE_BAR_BAZ"));
                 let _b = include_bytes!(env!("CARGO_STATICLIB_FILE_BAR_BAZ_bar-baz"));
+                let _b = include_bytes!(env!("CARGO_STATICLIB_FILE_BAR_BAZ_bar_baz"));
                 let _b = include_bytes!(env!("CARGO_CDYLIB_FILE_BAR_BAZ"));
                 let _b = include_bytes!(env!("CARGO_CDYLIB_FILE_BAR_BAZ_bar-baz"));
+                let _b = include_bytes!(env!("CARGO_CDYLIB_FILE_BAR_BAZ_bar_baz"));
             }
         "#,
         )

tests/testsuite/collisions.rs

@@ -550,7 +550,7 @@ fn collision_with_root() {
 [DOWNLOADING] crates ...
 [DOWNLOADED] foo-macro v1.0.0 [..]
 warning: output filename collision.
-The lib target `foo-macro` in package `foo-macro v1.0.0` has the same output filename as the lib target `foo-macro` in package `foo-macro v1.0.0 [..]`.
+The lib target `foo_macro` in package `foo-macro v1.0.0` has the same output filename as the lib target `foo_macro` in package `foo-macro v1.0.0 [..]`.
 Colliding filename is: [CWD]/target/doc/foo_macro/index.html
 The targets should have unique names.
 This is a known bug where multiple crates with the same name use

tests/testsuite/doc.rs

@@ -2203,7 +2203,7 @@ fn doc_test_in_workspace() {
         )
         .build();
     p.cargo("test --doc -vv")
-        .with_stderr_contains("[DOCTEST] crate-a")
+        .with_stderr_contains("[DOCTEST] crate_a")
         .with_stdout_contains(
             "
 running 1 test
@@ -2212,7 +2212,7 @@ test crate-a/src/lib.rs - (line 1) ... ok
 test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..]
 ",
         )
-        .with_stderr_contains("[DOCTEST] crate-b")
+        .with_stderr_contains("[DOCTEST] crate_b")
         .with_stdout_contains(
             "
 running 1 test

tests/testsuite/metadata.rs

@@ -1622,7 +1622,7 @@ fn workspace_metadata_with_dependencies_and_resolve() {
                       "kind": [
                         "lib"
                       ],
-                      "name": "non-artifact",
+                      "name": "non_artifact",
                       "src_path": "[..]/foo/non-artifact/src/lib.rs",
                       "test": true
                     }
@@ -3114,7 +3114,7 @@ fn filter_platform() {
           "crate_types": [
             "lib"
           ],
-          "name": "alt-dep",
+          "name": "alt_dep",
           "src_path": "[..]/alt-dep-0.0.1/src/lib.rs",
           "edition": "2015",
           "test": true,
@@ -3158,7 +3158,7 @@ fn filter_platform() {
           "crate_types": [
             "lib"
           ],
-          "name": "cfg-dep",
+          "name": "cfg_dep",
           "src_path": "[..]/cfg-dep-0.0.1/src/lib.rs",
           "edition": "2015",
           "test": true,
@@ -3202,7 +3202,7 @@ fn filter_platform() {
           "crate_types": [
             "lib"
           ],
-          "name": "host-dep",
+          "name": "host_dep",
           "src_path": "[..]/host-dep-0.0.1/src/lib.rs",
           "edition": "2015",
           "test": true,
@@ -3246,7 +3246,7 @@ fn filter_platform() {
           "crate_types": [
             "lib"
           ],
-          "name": "normal-dep",
+          "name": "normal_dep",
           "src_path": "[..]/normal-dep-0.0.1/src/lib.rs",
           "edition": "2015",
           "test": true,

tests/testsuite/test.rs

@@ -4864,9 +4864,9 @@ fn test_workspaces_cwd() {
         .build();
 
     p.cargo("test --workspace --all")
-        .with_stderr_contains("[DOCTEST] root-crate")
-        .with_stderr_contains("[DOCTEST] nested-crate")
-        .with_stderr_contains("[DOCTEST] deep-crate")
+        .with_stderr_contains("[DOCTEST] root_crate")
+        .with_stderr_contains("[DOCTEST] nested_crate")
+        .with_stderr_contains("[DOCTEST] deep_crate")
         .with_stdout_contains("test test_unit_root_cwd ... ok")
         .with_stdout_contains("test test_unit_nested_cwd ... ok")
         .with_stdout_contains("test test_unit_deep_cwd ... ok")
@@ -4876,33 +4876,33 @@ fn test_workspaces_cwd() {
         .run();
 
     p.cargo("test -p root-crate --all")
-        .with_stderr_contains("[DOCTEST] root-crate")
+        .with_stderr_contains("[DOCTEST] root_crate")
         .with_stdout_contains("test test_unit_root_cwd ... ok")
         .with_stdout_contains("test test_integration_root_cwd ... ok")
         .run();
 
     p.cargo("test -p nested-crate --all")
-        .with_stderr_contains("[DOCTEST] nested-crate")
+        .with_stderr_contains("[DOCTEST] nested_crate")
         .with_stdout_contains("test test_unit_nested_cwd ... ok")
         .with_stdout_contains("test test_integration_nested_cwd ... ok")
         .run();
 
     p.cargo("test -p deep-crate --all")
-        .with_stderr_contains("[DOCTEST] deep-crate")
+        .with_stderr_contains("[DOCTEST] deep_crate")
         .with_stdout_contains("test test_unit_deep_cwd ... ok")
         .with_stdout_contains("test test_integration_deep_cwd ... ok")
         .run();
 
     p.cargo("test --all")
         .cwd("nested-crate")
-        .with_stderr_contains("[DOCTEST] nested-crate")
+        .with_stderr_contains("[DOCTEST] nested_crate")
         .with_stdout_contains("test test_unit_nested_cwd ... ok")
         .with_stdout_contains("test test_integration_nested_cwd ... ok")
         .run();
 
     p.cargo("test --all")
         .cwd("very/deeply/nested/deep-crate")
-        .with_stderr_contains("[DOCTEST] deep-crate")
+        .with_stderr_contains("[DOCTEST] deep_crate")
         .with_stdout_contains("test test_unit_deep_cwd ... ok")
         .with_stdout_contains("test test_integration_deep_cwd ... ok")
         .run();