Try to better handle restricted crate names. #7959

ehuss · 2020-03-03T00:25:39Z

This attempts to improve handling of restricted crate names, particularly for cargo new and cargo init. Hopefully the code is straightforward to follow, but in summary the changes are:

General changes

Add more details to the error messages about why a name is not allowed, and what is allowed.
Change the valid package name check to be restricted to Unicode XID. This brings it in line with non_ascii_idents support in rustc. For the most part, this is pretty much the same as before. Note: this is used for the package name, and registry names. The differences are:
- Package names cannot start with numbers. Previously this was only rejected in cargo new. crates.io also rejects numbers. Numbers are also not valid crate names.
- Package names cannot start with dash -. This is a somewhat arbitrary change, but seems like it would stem problems. crates.io also rejects this.
- Package names cannot start with these characters that were previously allowed: https://gist.github.com/ehuss/804a797950001b5226e1264b6f65211f#file-not_start_but_alphanumeric-txt
  - Most of these are wacky numbers or other strange things.
- Package names cannot contain these characters that were previously allowed: https://gist.github.com/ehuss/804a797950001b5226e1264b6f65211f#file-not_continue_but_alphanumeric-txt
  - These are mostly odd things that for whatever reason the Unicode people decided not to include. It seems unlikely to me that someone would want to use one of these.
Display a warning on Windows if a Cargo target is a special Windows filename. The build error tends to be hard to understand, so the hope is the warning will make it evident.
cargo package/publish: Warn if a special Windows name is in the package.

cargo new/init specific changes

Update keyword list to 2018 edition.
Add warning if creating a library that has one of the conflicting names (deps/examples/build/incremental).
Warn about conflicting std names (core/std/alloc/proc-macro).
Windows reserved names: Rejected on windows, warned on other platforms.
Warn about non-ASCII package names.
Only print the --name suggestion for cargo init. I found the suggestion confusing, and I feel like it doesn't really make sense for cargo new (since it would only affect the directory name).

ehuss · 2020-03-03T00:25:54Z

I know making some of these changes to errors might be a little aggressive, so I'd be happy to discuss dialing it back. Particularly the package name starting with numbers.

alexcrichton

This does feel a bit aggressive but I think it's also fine to try this out and see what the warnings look like. I left a few inline comments but overall looks great, thanks!

alexcrichton · 2020-03-03T15:13:55Z

src/cargo/util/toml/targets.rs

+                        "{} target `{}` is a reserved Windows filename, \
+                        this target will not work on Windows platforms",
+                        target_kind_human, name
+                    ));


Hm can you clarify this a bit for me? How come we'd only issue this warning on windows, and how comit it applies to all targets? It should be ok to have a library called con, right? I don't think that creates any of the bad file names?

My thinking for only showing on Windows is: if someone wants to create an executable called con, and they never intend for it to be used on Windows, then that would create a warning that they cannot silence. Maybe if we had the "compatibility" table we discussed at the last meeting, we could warn on all platforms and then allow the warning to be silenced by specifying that it is incompatible with windows?

FWIW, I scanned all of crates.io, and didn't find any targets with a reserved name. Part of this came up with a user on Discord who had a package named prune with a binary named prn, and was confused when their Windows CI failed. I don't think they've published to crates.io, though.

As for libraries, in some situations rustc creates filenames like con.tr1cof326ip8vwg.rcgu.o which fail on windows. It also seems like it would just be a risky thing to do, and I didn't want to put in effort and complexity to handle that edge case, which is pretty far on the edge.

Oh for sure binaries make sense here, I was just confused about the library part. The point about crate names showing up in object files makes sense though!

alexcrichton · 2020-03-03T15:15:34Z

src/cargo/util/restricted_names.rs

+pub fn is_windows_reserved(name: &str) -> bool {
+    [
+        "con", "prn", "aux", "nul", "com1", "com2", "com3", "com4", "com5", "com6", "com7", "com8",
+        "com9", "lpt1", "lpt2", "lpt3", "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9",


So just to clarify, lpt9.foobarbazhello is an invalid filename on Windows?

All of these are invalid as the filestem of a file anywhere on Windows?

Correct on the extension.

As for "anywhere", that's a bit of a stretch. It is possible to create these files, like with cygwin tools which use different APIs, but those files end up causing problems with the rest of the Windows ecosystem.

Ah ok, thanks for clarifying! I agree yeah that I've seen these "work" in some contexts but it gets really weird when only some contexts have them work and others don't.

alexcrichton · 2020-03-04T15:37:28Z

@bors: r+

bors · 2020-03-04T15:37:30Z

📌 Commit 2a874aa has been approved by alexcrichton

bors · 2020-03-04T15:37:37Z

⌛ Testing commit 2a874aa with merge 5fe8ab5...

bors · 2020-03-04T16:02:15Z

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing 5fe8ab5 to master...

Update cargo Update cargo 21 commits in bda50510d1daf6e9c53ad6ccf603da6e0fa8103f..7019b3ed3d539db7429d10a343b69be8c426b576 2020-03-02 18:05:34 +0000 to 2020-03-17 21:02:00 +0000 - Run through clippy (rust-lang/cargo#8015) - Fix config profiles using "dev" in `cargo test`. (rust-lang/cargo#8012) - Run CI on all PRs. (rust-lang/cargo#8011) - Add unit-graph JSON output. (rust-lang/cargo#7977) - Split workspace/validate() into multiple functions (rust-lang/cargo#8008) - Use Option::as_deref (rust-lang/cargo#8005) - De-duplicate edges (rust-lang/cargo#7993) - Revert "Disable preserving mtimes on archives" (rust-lang/cargo#7935) - Close the front door for clippy but open the back (rust-lang/cargo#7533) - Fix CHANGELOG.md typos (rust-lang/cargo#7999) - Update changelog note about crate-versions flag. (rust-lang/cargo#7998) - Bump to 0.45.0, update changelog (rust-lang/cargo#7997) - Bump libgit2 dependencies (rust-lang/cargo#7996) - Avoid buffering large amounts of rustc output. (rust-lang/cargo#7838) - Add "Updating" status for git submodules. (rust-lang/cargo#7989) - WorkspaceResolve: Use descriptive lifetime label. (rust-lang/cargo#7990) - Support old html anchors in manifest chapter. (rust-lang/cargo#7983) - Don't create hardlink for library test and integrations tests, fixing rust-lang/cargo#7960 (rust-lang/cargo#7965) - Partially revert change to filter debug_assertions. (rust-lang/cargo#7970) - Try to better handle restricted crate names. (rust-lang/cargo#7959) - Fix bug with new feature resolver and required-features. (rust-lang/cargo#7962)

Refs: rust-lang/cargo#7959

63: Add a prefix to each package names to adapt to rust-lang/cargo#7959 r=qryxip a=qryxip Fixes #62. bors r+ Co-authored-by: Ryo Yamashita <qryxip@gmail.com>

Add --name suggestion for cargo new Resolves #8613 Since `check_name` have already got a parameter to show name help, I reuse the logic and sync the behavior between `cargo init` and `cargo new`. The divergence seems to be intentionally made in #7959: _...Only print the --name suggestion for `cargo init`._ Feel free to discuss.

ehuss added 3 commits March 2, 2020 16:04

Try to better handle restricted crate names.

95008f9

Warn on Windows about reserved target names.

15ac82b

Warn when packaging files with Windows special names.

2a874aa

rust-highfive assigned alexcrichton Mar 3, 2020

alexcrichton reviewed Mar 3, 2020

View reviewed changes

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 4, 2020

bors merged commit 5fe8ab5 into rust-lang:master Mar 4, 2020

ehuss mentioned this pull request Mar 11, 2020

Update cargo rust-lang/rust#69907

Merged

ehuss mentioned this pull request Apr 14, 2020

Cargo should check for windows-reserved filenames when extracting a package. #8055

Closed

This was referenced Apr 20, 2020

Implement Plasma effect rust-rosetta/rust-rosetta#704

Merged

CI failure due to invalid crate names rust-rosetta/rust-rosetta#705

Closed

ehuss mentioned this pull request Apr 26, 2020

Cargo warning when you try to publish a crate that contains files/folders with windows reserved filenames #8163

Closed

klemens mentioned this pull request Jun 7, 2020

Not building on 1.44.0 klemens/2i-emulator#9

Closed

taiki-e mentioned this pull request Jun 11, 2020

Fix invalid package names rust-lang/async-book#94

Merged

taiki-e added a commit to rust-lang/async-book that referenced this pull request Jun 11, 2020

Fix invalid package names

8121c2e

Refs: rust-lang/cargo#7959

huangjj27 pushed a commit to huangjj27/async-book that referenced this pull request Aug 1, 2020

Fix invalid package names

696bc31

Refs: rust-lang/cargo#7959

weihanglo mentioned this pull request Sep 4, 2020

Add --name suggestion for cargo new #8675

Merged

qryxip mentioned this pull request Sep 6, 2020

v0.5 以降、Codeforces のコンテストで cargo compete new がエラーとなる qryxip/cargo-compete#62

Closed

qryxip added a commit to qryxip/cargo-compete that referenced this pull request Sep 6, 2020

Add a prefix to each package names to adapt to rust-lang/cargo#7959

e45143d

bors bot added a commit to qryxip/cargo-compete that referenced this pull request Sep 6, 2020

Merge #63

370a6bc

63: Add a prefix to each package names to adapt to rust-lang/cargo#7959 r=qryxip a=qryxip Fixes #62. bors r+ Co-authored-by: Ryo Yamashita <qryxip@gmail.com>

ehuss added this to the 1.44.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to better handle restricted crate names. #7959

Try to better handle restricted crate names. #7959

ehuss commented Mar 3, 2020

ehuss commented Mar 3, 2020

alexcrichton left a comment

alexcrichton Mar 3, 2020

ehuss Mar 3, 2020

alexcrichton Mar 4, 2020

alexcrichton Mar 3, 2020

ehuss Mar 3, 2020

alexcrichton Mar 4, 2020

alexcrichton commented Mar 4, 2020

bors commented Mar 4, 2020

bors commented Mar 4, 2020

bors commented Mar 4, 2020

Try to better handle restricted crate names. #7959

Try to better handle restricted crate names. #7959

Conversation

ehuss commented Mar 3, 2020

ehuss commented Mar 3, 2020

alexcrichton left a comment

Choose a reason for hiding this comment

alexcrichton Mar 3, 2020

Choose a reason for hiding this comment

ehuss Mar 3, 2020

Choose a reason for hiding this comment

alexcrichton Mar 4, 2020

Choose a reason for hiding this comment

alexcrichton Mar 3, 2020

Choose a reason for hiding this comment

ehuss Mar 3, 2020

Choose a reason for hiding this comment

alexcrichton Mar 4, 2020

Choose a reason for hiding this comment

alexcrichton commented Mar 4, 2020

bors commented Mar 4, 2020

bors commented Mar 4, 2020

bors commented Mar 4, 2020