Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo -Zscript uses invalid package names when the file name starts with a digit #12330

Closed
figsoda opened this issue Jul 5, 2023 · 7 comments · Fixed by #12349
Closed

cargo -Zscript uses invalid package names when the file name starts with a digit #12330

figsoda opened this issue Jul 5, 2023 · 7 comments · Fixed by #12349
Labels
C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-triage Status: This issue is waiting on initial triage. Z-script Nightly: cargo script

Comments

@figsoda
Copy link

figsoda commented Jul 5, 2023

Problem

When the file name starts with a digit, e.g. 0.rs, cargo -Zscript 0.rs defaults the package name to -0 and fails

Steps

echo 'fn main() {}' > 0.rs
cargo -Zscript 0.rs
warning: `package.edition` is unspecifiead, defaulting to `2021`
error: failed to parse manifest at `<...>/0.rs`

Caused by:
  invalid character `-` in package name: `-0`, the first character must be a Unicode XID start character (most letters or `_`)

Possible Solution(s)

When the first character of the file name is invalid, insert _ instead of -, which makes the package name valid. So for 0.rs, the package name would default to _0. This is #12329's approach

Notes

Related: #12207, #12329 (comment)

Version

cargo 1.72.0-nightly (5b377cece 2023-06-30)
release: 1.72.0-nightly
commit-hash: 5b377cece0e0dd0af686cf53ce4637d5d85c2a10
commit-date: 2023-06-30
host: x86_64-unknown-linux-gnu
libgit2: 1.6.4 (sys:0.17.2 vendored)
libcurl: 8.1.2-DEV (sys:0.4.63+curl-8.1.2 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: NixOS 23.11.0 [64-bit]
@figsoda figsoda added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jul 5, 2023
@epage
Copy link
Contributor

epage commented Jul 5, 2023

I'm curious, was 0.rs the scenario you ran into this or something else?

My primary concern with unilaterally applying _ is if the file name is one where we'd also apply - elsewhere as mixing _ and - looks a bit odd, see b116864 from #12255.

The question is whether there is a viable alternative.

Purely for brainstorming purposes

  • We add an arbitrary letter or word
  • We strop the leading invalid digit. If nothing is left, we
    • Use an arbitrary letter or word
    • Try turning the invalid item into a word (best effort)
  • We go ahead and error unlike all of the other sanitation effort to avoid errors.
  • We instead always validate instead of sanitize

@epage epage added Z-script Nightly: cargo script S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. labels Jul 5, 2023
@figsoda
Copy link
Author

figsoda commented Jul 5, 2023

I'm curious, was 0.rs the scenario you ran into this or something else?

Yes. I set up my editor (neovim) to create temporary files using vim.fn.tempname(), which returns non-negative integers, and makes the temporary file names start with a digit.

We add an arbitrary word

Out of the ideas you listed, this is the best option in my opinion (tied with my initial approach)

We instead always validate instead of sanitize

I'm not sure what this means, does it mean that cargo -Zscript 0.rs will fail due to 0 being an invalid package name (instead of -0)?

@figsoda
Copy link
Author

figsoda commented Jul 5, 2023

We can also unconditionally add an arbitrary word, so both 0.rs and foo.rs would become e.g. script-0 and script-foo

@epage
Copy link
Contributor

epage commented Jul 5, 2023

Yes. I set up my editor (neovim) to create temporary files using vim.fn.tempname(), which returns non-negative integers, and makes the temporary file names start with a digit.

So not necessarily 0.rs but 980980809.rs?

We instead always validate instead of sanitize

I'm not sure what this means, does it mean that cargo -Zscript 0.rs will fail due to 0 being an invalid package name (instead of -0)?

Yes

We can also unconditionally add an arbitrary word, so both 0.rs and foo.rs would become e.g. script-0 and script-foo

  • For users with files with more meaningful names, this would be a regression in user messages
  • For users wanting to build binaries or publish packages from their script, this would be a blocker
  • We are looking to allow these packages as workspace members, so cargo test -p <name> should work rather than cargo test -p script-<name>

Most likely we'd also make the prefix be "package". "script" is unlikely to be in any of the official naming as this isn't really a script though it behaves somewhat like people are used to using scripts.

@figsoda
Copy link
Author

figsoda commented Jul 5, 2023

So not necessarily 0.rs but 980980809.rs?

Yes

@figsoda
Copy link
Author

figsoda commented Jul 5, 2023

I suggest we go with #12329 for now, considering that it is a partial revert of #12255 which caused a regression

@epage
Copy link
Contributor

epage commented Jul 5, 2023

The behavior pre-#12255 was fairly arbitrary so reverting some of it isn't going to any better state but going to some state. Seeing as this is an unstable feature and this isn't the biggest of concerns for resolving for stabilization, I don't think we need to rush a specific answer but lets figure out a proposal of what it should be and document it so we reduce the number of caveats, rather than just transferring this caveat into a different one.

At the moment, I lean towards stripping invalid package-name start chars until we find a valid one. If we don't find any valid ones, we substitute in package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-triage Status: This issue is waiting on initial triage. Z-script Nightly: cargo script
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants