Skip to content

Commit 9d63ae2

Browse files
authored
fix(script): Remove name sanitiztion outside what is strictly required (#16120)
### What does this PR try to resolve? Note that we are currently inconsistent in that `test` gets sanitized but nothing else in `sysroot`. We reviewed what gets sanitized in the Cargo team meeting and found none of these really apply to cargo scripts except conflicting with artifact dirs. For conflicting with artifact dirs, 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. ### How to test and review this PR?
2 parents d80156f + 9b23ede commit 9d63ae2

File tree

2 files changed

+76
-28
lines changed

2 files changed

+76
-28
lines changed

src/cargo/util/toml/embedded.rs

Lines changed: 1 addition & 20 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,25 +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_keyword(&name) {
64-
name.push(placeholder);
65-
} else if restricted_names::is_conflicting_artifact_name(&name) {
66-
// Being an embedded manifest, we always assume it is a `[[bin]]`
67-
name.push(placeholder);
68-
} else if name == "test" {
69-
name.push(placeholder);
70-
} else if restricted_names::is_windows_reserved(&name) {
71-
// Go ahead and be consistent across platforms
72-
name.push(placeholder);
73-
} else {
74-
break;
75-
}
76-
}
77-
78-
name
59+
PackageName::sanitize(name, placeholder).into_inner()
7960
}
8061

8162
#[cfg(test)]

tests/testsuite/script/cargo.rs

Lines changed: 75 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -674,25 +674,92 @@ args: []
674674
}
675675

676676
#[cargo_test(nightly, reason = "-Zscript is unstable")]
677-
fn test_name_is_deps_dir_implicit() {
677+
#[cfg(not(windows))]
678+
fn test_name_is_windows_reserved_name() {
678679
let script = ECHO_SCRIPT;
679-
let p = cargo_test_support::project()
680-
.file("deps.rs", script)
681-
.build();
680+
let p = cargo_test_support::project().file("con", script).build();
682681

683-
p.cargo("-Zscript -v deps.rs")
682+
p.cargo("-Zscript -v ./con")
684683
.masquerade_as_nightly_cargo(&["script"])
685684
.with_stdout_data(str![[r#"
686-
current_exe: [ROOT]/home/.cargo/build/[HASH]/target/debug/deps-[EXE]
685+
current_exe: [ROOT]/home/.cargo/build/[HASH]/target/debug/con[EXE]
687686
arg0: [..]
688687
args: []
689688
690689
"#]])
691690
.with_stderr_data(str![[r#"
692691
[WARNING] `package.edition` is unspecified, defaulting to `2024`
693-
[COMPILING] deps- v0.0.0 ([ROOT]/foo/deps.rs)
692+
[COMPILING] con v0.0.0 ([ROOT]/foo/con)
694693
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
695-
[RUNNING] `[ROOT]/home/.cargo/build/[HASH]/target/debug/deps-[EXE]`
694+
[RUNNING] `[ROOT]/home/.cargo/build/[HASH]/target/debug/con[EXE]`
695+
696+
"#]])
697+
.run();
698+
}
699+
700+
#[cargo_test(nightly, reason = "-Zscript is unstable")]
701+
fn test_name_is_sysroot_package_name() {
702+
let script = ECHO_SCRIPT;
703+
let p = cargo_test_support::project().file("test", script).build();
704+
705+
p.cargo("-Zscript -v ./test")
706+
.masquerade_as_nightly_cargo(&["script"])
707+
.with_stdout_data(str![[r#"
708+
current_exe: [ROOT]/home/.cargo/build/[HASH]/target/debug/test[EXE]
709+
arg0: [..]
710+
args: []
711+
712+
"#]])
713+
.with_stderr_data(str![[r#"
714+
[WARNING] `package.edition` is unspecified, defaulting to `2024`
715+
[COMPILING] test v0.0.0 ([ROOT]/foo/test)
716+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
717+
[RUNNING] `[ROOT]/home/.cargo/build/[HASH]/target/debug/test[EXE]`
718+
719+
"#]])
720+
.run();
721+
}
722+
723+
#[cargo_test(nightly, reason = "-Zscript is unstable")]
724+
fn test_name_is_keyword() {
725+
let script = ECHO_SCRIPT;
726+
let p = cargo_test_support::project().file("self", script).build();
727+
728+
p.cargo("-Zscript -v ./self")
729+
.masquerade_as_nightly_cargo(&["script"])
730+
.with_stdout_data(str![[r#"
731+
current_exe: [ROOT]/home/.cargo/build/[HASH]/target/debug/self[EXE]
732+
arg0: [..]
733+
args: []
734+
735+
"#]])
736+
.with_stderr_data(str![[r#"
737+
[WARNING] `package.edition` is unspecified, defaulting to `2024`
738+
[COMPILING] self v0.0.0 ([ROOT]/foo/self)
739+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
740+
[RUNNING] `[ROOT]/home/.cargo/build/[HASH]/target/debug/self[EXE]`
741+
742+
"#]])
743+
.run();
744+
}
745+
746+
#[cargo_test(nightly, reason = "-Zscript is unstable")]
747+
fn test_name_is_deps_dir_implicit() {
748+
let script = ECHO_SCRIPT;
749+
let p = cargo_test_support::project()
750+
.file("deps.rs", script)
751+
.build();
752+
753+
p.cargo("-Zscript -v deps.rs")
754+
.masquerade_as_nightly_cargo(&["script"])
755+
.with_status(101)
756+
.with_stdout_data(str![""])
757+
.with_stderr_data(str![[r#"
758+
[WARNING] `package.edition` is unspecified, defaulting to `2024`
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
696763
697764
"#]])
698765
.run();

0 commit comments

Comments
 (0)