Skip to content

Commit 9b23ede

Browse files
committed
fix(script): Error, rather than sanitize, artfact dir names
Making this an error now gives us the best compatibility story. A user can workaround this by overriding `package.name`. Our paths forward include: - Leave it as-is or at least improve the error message for this case - Make it so we don't need the error message - Add back sanitizating the name Ideally, we make it so we don't need the error message. The ground work was laid out for this in #16086. The next step is to move the conflict error from manifest parsing to compilation so we can check whether target-dir and build-dir overlap to error. There are unknowns with this, including whether the usability is good enough for making this error conditional on the target-dir and build-dir overlapping. We may even want to wait until we change the default build-dir.
1 parent b957ec8 commit 9b23ede

File tree

2 files changed

+7
-22
lines changed

2 files changed

+7
-22
lines changed

src/cargo/util/toml/embedded.rs

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use cargo_util_schemas::manifest::PackageName;
22

33
use crate::util::frontmatter::FrontmatterError;
44
use crate::util::frontmatter::ScriptSource;
5-
use crate::util::restricted_names;
65

76
pub(super) fn expand_manifest(content: &str) -> Result<String, FrontmatterError> {
87
let source = ScriptSource::parse(content)?;
@@ -57,18 +56,7 @@ pub fn sanitize_name(name: &str) -> String {
5756
'-'
5857
};
5958

60-
let mut name = PackageName::sanitize(name, placeholder).into_inner();
61-
62-
loop {
63-
if restricted_names::is_conflicting_artifact_name(&name) {
64-
// Being an embedded manifest, we always assume it is a `[[bin]]`
65-
name.push(placeholder);
66-
} else {
67-
break;
68-
}
69-
}
70-
71-
name
59+
PackageName::sanitize(name, placeholder).into_inner()
7260
}
7361

7462
#[cfg(test)]

tests/testsuite/script/cargo.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -752,17 +752,14 @@ fn test_name_is_deps_dir_implicit() {
752752

753753
p.cargo("-Zscript -v deps.rs")
754754
.masquerade_as_nightly_cargo(&["script"])
755-
.with_stdout_data(str![[r#"
756-
current_exe: [ROOT]/home/.cargo/build/[HASH]/target/debug/deps-[EXE]
757-
arg0: [..]
758-
args: []
759-
760-
"#]])
755+
.with_status(101)
756+
.with_stdout_data(str![""])
761757
.with_stderr_data(str![[r#"
762758
[WARNING] `package.edition` is unspecified, defaulting to `2024`
763-
[COMPILING] deps- v0.0.0 ([ROOT]/foo/deps.rs)
764-
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
765-
[RUNNING] `[ROOT]/home/.cargo/build/[HASH]/target/debug/deps-[EXE]`
759+
[ERROR] failed to parse manifest at `[ROOT]/foo/deps.rs`
760+
761+
Caused by:
762+
the binary target name `deps` is forbidden, it conflicts with cargo's build directory names
766763
767764
"#]])
768765
.run();

0 commit comments

Comments
 (0)