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
Handle case only file name updates on case insensitive file systems #18865
Merged
justinvelluppillai
merged 5 commits into
next_release
from
m-18724-case-insensitive-file-delete
Mar 8, 2022
Merged
Handle case only file name updates on case insensitive file systems #18865
justinvelluppillai
merged 5 commits into
next_release
from
m-18724-case-insensitive-file-delete
Mar 8, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bx80
added
Bug
For errors / faults / flaws / inconsistencies etc.
Needs Review
PRs that need a code review
labels
Mar 1, 2022
justinvelluppillai
approved these changes
Mar 1, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and tests work (using the case insensitive filesystem path).
@tsteur do you maybe want to take a look at this solution?
Co-authored-by: Justin Velluppillai <justin@innocraft.com>
Co-authored-by: Justin Velluppillai <justin@innocraft.com>
justinvelluppillai
added a commit
that referenced
this pull request
Mar 8, 2022
* Update version to 4.8.0-rc1 * Fix possible warning in Url::isValidHost (#18887) * Handle case only file name updates on case insensitive file systems (#18865) * Added unit test for case-sensitive unlink * Use case-insensitive comparison in directoryDiff() if a case-insensitive filesystem is detected * Fix misleading method name * Update tests/PHPUnit/Unit/FilesystemTest.php Co-authored-by: Justin Velluppillai <justin@innocraft.com> * Update tests/PHPUnit/Unit/FilesystemTest.php Co-authored-by: Justin Velluppillai <justin@innocraft.com> Co-authored-by: Justin Velluppillai <justin@innocraft.com> * Update version to 4.8.0 (#18893) Co-authored-by: Stefan Giehl <stefan@matomo.org> Co-authored-by: Ben Burgess <88810029+bx80@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Fixes #18724
Background
During an update, any files that already exist in the installation destination but no longer exist in the update package will be identified and removed.
The process is:
This causes a problem when a file is renamed with only a change in case, eg.
Myfile
toMyFile
.For case sensitive systems this works:
MyFile
is copied without interfering with the oldMyfile
Myfile
is identified as not being in the source package and noted for deletion.Myfile
is correctly unlinked without interfering with the newMyFile
.For case insensitive system this fails:
MyFile
is copied and overwrites the oldMyfile
as the name is treated as the same.Myfile
is identified as not being in the source package and noted for deletion.MyFile
is incorrectly deleted as the name is treated as the same, this results in a missing file.Fix
To fix this a new method has been added which checks if the file system is case insensitive by writing a file with one case and checking if it exists using another case. If the file is reported to exist under a differently cased filename then the file system is assumed to be case insensitive.
When checking for files which exist in the installation but not in the source package and a case insensitive file system is detected then the file name comparison will be performed in a case insensitive manner, this results in the following behaviour:
MyFile
is copied and overwrites the oldMyfile
as the name is treated as the same.MyFile
in the source package is compared case insensitively with the existingMyfile
and considered to exist in the source package, so it is not noted for deletion.Myfile
and as it has been overwritten with the newMyFile
, resulting in the new file in the correct place.Review