Skip to content

Conversation

epage
Copy link
Contributor

@epage epage commented Jul 11, 2023

What does this PR try to resolve?

The sanitization logic uses a placeholder for the first character that isn't valid in the first character position. #12329 took the approach of always using _ which has the problem of mixing separators if the user used - or we had other placeholders to insert. Instead, this takes the approach of stripping the leading invalid characters and using a placeholder name if nothing is left.

Fixes #12330

How should we test and review this PR?

Per-commit. The first adds tests so the change in behavior can be observed over each additional commit.

Additional information

I was also hoping to make the binary name not use placeholders by setting bin.name to file_stem but then I got

   Compiling s-h-w-c- v0.0.0 (/home/epage/src/personal/cargo/target/tmp/cit/t133/foo)
error: invalid character `'.'` in crate name: `s_h.w§c!`

error: invalid character `'§'` in crate name: `s_h.w§c!`

error: invalid character `'!'` in crate name: `s_h.w§c!`

error: could not compile `s-h-w-c-` (bin "s-h.w§c!") due to 3 previous errors

I decided to not get into what are or aren't valid characters according to rustc.

@rustbot
Copy link
Collaborator

rustbot commented Jul 11, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 11, 2023
@epage epage changed the title fix(embedded): Generate valid package names fix(embedded): Always generate valid package names Jul 11, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. Thanks!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2023

📌 Commit 7d4e39c has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 11, 2023
@bors
Copy link
Contributor

bors commented Jul 11, 2023

⌛ Testing commit 7d4e39c with merge 694a579...

@bors
Copy link
Contributor

bors commented Jul 11, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 694a579 to master...

@bors bors merged commit 694a579 into rust-lang:master Jul 11, 2023
@epage epage deleted the sanitize branch July 12, 2023 13:51
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 12, 2023
Update cargo

10 commits in 45782b6b8afd1da042d45c2daeec9c0744f72cc7..694a579566a9a1482b20aff8a68f0e4edd99bd28
2023-07-05 16:54:51 +0000 to 2023-07-11 22:28:29 +0000
- fix(embedded): Always generate valid package names (rust-lang/cargo#12349)
- fix(embedded): Error on unsupported commands (rust-lang/cargo#12350)
- chore(ci): Automatically test new packages by using `--workspace` (rust-lang/cargo#12342)
- contrib docs: Add some more detail about how publishing works (rust-lang/cargo#12344)
- docs: Put cargo-add change under nightly (rust-lang/cargo#12343)
- Minor: Use "number" instead of "digit" when explaining Cargo's use of semver (rust-lang/cargo#12340)
- Update criterion (rust-lang/cargo#12338)
- Add profile strip to config docs. (rust-lang/cargo#12337)
- update re: multiple versions that differ only in the metadata tag (rust-lang/cargo#12335)
- doc: state `PackageId`/`SourceId` string is opaque (rust-lang/cargo#12313)

r? `@ghost`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 13, 2023
Update cargo

10 commits in 45782b6b8afd1da042d45c2daeec9c0744f72cc7..694a579566a9a1482b20aff8a68f0e4edd99bd28
2023-07-05 16:54:51 +0000 to 2023-07-11 22:28:29 +0000
- fix(embedded): Always generate valid package names (rust-lang/cargo#12349)
- fix(embedded): Error on unsupported commands (rust-lang/cargo#12350)
- chore(ci): Automatically test new packages by using `--workspace` (rust-lang/cargo#12342)
- contrib docs: Add some more detail about how publishing works (rust-lang/cargo#12344)
- docs: Put cargo-add change under nightly (rust-lang/cargo#12343)
- Minor: Use "number" instead of "digit" when explaining Cargo's use of semver (rust-lang/cargo#12340)
- Update criterion (rust-lang/cargo#12338)
- Add profile strip to config docs. (rust-lang/cargo#12337)
- update re: multiple versions that differ only in the metadata tag (rust-lang/cargo#12335)
- doc: state `PackageId`/`SourceId` string is opaque (rust-lang/cargo#12313)

r? ``@ghost``
@ehuss ehuss added this to the 1.73.0 milestone Jul 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo -Zscript uses invalid package names when the file name starts with a digit
5 participants