crates_io_tarball: Disallow paths differing only by case

[Turbo87]

Resolves #8410

This implementation uses .to_ascii_lowercase() on the Path to compare the paths. In other words: this may or may not be sufficient depending on the filesystem and the usage of non-ASCII characters in the paths. It seems to cover the majority of the existing cases though, so it's at least a step in the right direction.

In the future we could even consider rejecting non-ASCII paths, in which case this implementation would probably be sufficient.

/cc @kornelski

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.65%. Comparing base (3dc1848) to head (19dee4e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8788      +/-   ##
==========================================
- Coverage   88.66%   88.65%   -0.02%     
==========================================
  Files         276      276              
  Lines       27605    27613       +8     
==========================================
+ Hits        24476    24480       +4     
- Misses       3129     3133       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[LawnGnome]

LGTM.

I think we'll eventually have to grapple with the terrible questions of "so, what, if any, encoding do we expect paths in crate files to be in", and "is uniqueness actually defined by the NF[CD] canonicalisations of path names after case folding, if the answer to the previous question involves Unicode in some way", but let's not let perfect be the enemy of good here.

[zimond]

after this, I cannot publish my crate with the following error:

the remote server responded with an error (status 400 Bad Request): uploaded tarball contains more than one file with the same path xxx/README.md

I have to delete the readme file to upload a new version, which is weird. The bundled artifact .crate file contains only one readme file and I don't know where this error comes from.

[Turbo87]

for what crate is this? can you cargo package and send us the file to take a look?

[zimond]

@Turbo87 it's https://crates.io/crates/fontkit
fontkit-0.6.0-beta.5.crate.zip

[eth3lbert]

The doc.rs source page indicates that there are two README.md files.
https://docs.rs/crate/fontkit/0.5.2/source/

[zimond]

Now I'm really confused.

[zimond]

Could it be that cargo is generating a README.md for me? link

[zimond]

Ok so with readme = "Readme.md" in Cargo.toml, I successfully uploaded a new version 0.6.0-beta.6 to crates.io.

[Turbo87]

hmm I'm not aware of cargo having such behavior. @rust-lang/cargo ?

[epage]

This is likely rust-lang/cargo#13722 which comes from our logic to ensure a README or a LICENSE from another location in the repo is pulled into the .crate file

https://github.com/rust-lang/cargo/blob/10b7d384352f6d9a39f609de44476f815fc792a2/src/cargo/ops/cargo_package.rs#L330-L338

[epage]

btw this seems like this would have been a good change to coordinate with the Cargo team on as this is interrelated to Cargo's behavior and choices Cargo makes. If nothing else, we should probably be consistent on our case sensitivity logic.

[ehuss]

Filed rust-lang/cargo#14020 about the readme specifically.

[ehuss]

@Turbo87 I would suggest reverting this until things are fixed, as I suspect this will impact a large number of users.

I would also suggest doing a scan of existing crates to see how common it is. This may trigger for all sorts of reasons which we don't know about.

[LawnGnome]

I've just deployed crates.io with this reverted.

Let's figure out what the right behaviour should be here and then get cargo and crates.io on the same page.