crates_io_tarball: Disallow paths differing only by case #8788
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
There was a problem hiding this comment.
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.
|
after this, I cannot publish my crate with the following error: I have to delete the readme file to upload a new version, which is weird. The bundled artifact |
|
for what crate is this? can you |
|
The doc.rs source page indicates that there are two |
|
Now I'm really confused. |
|
Could it be that |
|
Ok so with |
|
hmm I'm not aware of cargo having such behavior. @rust-lang/cargo ? |
|
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 |
|
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. |
|
Filed rust-lang/cargo#14020 about the readme specifically. |
|
@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. |
…)" This reverts commit e476d90.
|
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. |
Resolves #8410
This implementation uses
.to_ascii_lowercase()on thePathto 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