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

Case insensitive file name matching when packaging files #13885

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

torhovland
Copy link
Contributor

@torhovland torhovland commented May 8, 2024

Fixes #13722.

Handles the readme and license files. The Cargo.lock file is trickier. There are a bunch of places in the code where the existence of Cargo.lock (case sensitive) is checked, including when packaging.

I think it should be OK to keep Cargo.lock out of this. While the readme and license files are typically created by the user, the Cargo.lock file is typically created by Cargo, hence casing should be consistent anyway.

This PR also doesn't handle a TARGET dir. I can look into that if desired.

#13722 (comment)

@rustbot
Copy link
Collaborator

rustbot commented May 8, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added Command-package S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 8, 2024
@epage
Copy link
Contributor

epage commented May 8, 2024

Note that #13722 is not marked as S-Accepted. I don't think we've come to a conclusion on if it should be fixed and what the right constraints are.

@torhovland
Copy link
Contributor Author

I am now noticing that Cargo uses the unicase crate for case insensitive comparisons elsewhere.

use unicase::Ascii as UncasedAscii;

This PR should do that rather than explicitly converting to lowercase and comparing.

@weihanglo weihanglo added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 9, 2024
@weihanglo
Copy link
Member

Thanks for the PR. Marked it as draft for now to reflect the fact that it's still in discussion.

@weihanglo weihanglo marked this pull request as draft May 9, 2024 15:28
@bors
Copy link
Contributor

bors commented Jun 3, 2024

☔ The latest upstream changes (presumably #14004) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Command-package S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo packages duplicate files on case-insensitive file systems
5 participants