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

Breaking Change: don't try to normalize unicode in collision testing. #25

Merged
merged 2 commits into from
Jul 24, 2023

Conversation

nhz2
Copy link
Member

@nhz2 nhz2 commented Jul 18, 2023

Initially, I tried to check if two entries had the same name in a case-insensitive way using lowercase.

The function I should have used was Unicode.normalize

However, normalize fails on invalid UTF-8, and it is not clear that normalize is doing the same kind of normalization Windows does. In addition, on Windows it is possible to make a specific directory case sensitive.
https://learn.microsoft.com/en-us/windows/wsl/case-sensitivity#change-the-case-sensitivity-of-files-and-directories

Because of these complications, I am no longer attempting to normalize or lowercase entry names when checking for collisions.

I also am now checking if a new file is an existing implicit or explicit directory, and if a new implicit or explicit directory is an existing file.

This means you will now get an error if you try to create "a.txt/b.txt" and "a.txt" in the same archive.

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.05 🎉

Comparison is base (ea9ef99) 98.04% compared to head (cb3e3a2) 98.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
+ Coverage   98.04%   98.10%   +0.05%     
==========================================
  Files           6        6              
  Lines         820      843      +23     
==========================================
+ Hits          804      827      +23     
  Misses         16       16              
Impacted Files Coverage Δ
src/types.jl 100.00% <ø> (ø)
src/filename-checks.jl 100.00% <100.00%> (ø)
src/writer.jl 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@nhz2 nhz2 changed the title Breaking Change: use case sensitive name collision testing. Breaking Change: don't try to normalize unicode in collision testing. Jul 24, 2023
@nhz2 nhz2 merged commit b17880f into main Jul 24, 2023
5 checks passed
@nhz2 nhz2 deleted the fix-filename-check branch July 24, 2023 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants