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

fix: bypass Qt's QFile::encodeName() in csync #12039

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Jan 23, 2025

ToDo

  • extract commits as individual reasonable PRs
  • fix failing windows test @erikjv
  • implement test case for nfd/nfc encoded mkpath
  • implement test case for nfd/nfc encoded rename

@erikjv erikjv force-pushed the fix/no-normalization-macos branch from 849a1ca to 1b2d2ca Compare January 23, 2025 16:56
@DeepDiver1975 DeepDiver1975 force-pushed the fix/no-normalization-macos branch 3 times, most recently from a70dda3 to 980bd4a Compare January 29, 2025 16:59
@erikjv
Copy link
Collaborator

erikjv commented Jan 30, 2025

I moved "Clarify propagator job descriptions" out as a seperate PR: #12050 . We can merge the fromDisk fix into the commit where it is implemented. But other than that, I think we should keep this as 1 PR.

@erikjv erikjv force-pushed the fix/no-normalization-macos branch 2 times, most recently from a74d10d to 44b4a8c Compare January 30, 2025 17:12

// Test that when a file/directory name on the remote is encoded in NFC, the local name is encoded
// in the same way, and that a subsequent sync does not change anything. And the same for NFD.
void testNameEncoding()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikjv Please document what exactly this test is about. Also thinking about that the function name testNameEncoding() is not giving that much of an explanation. THX

@DeepDiver1975 DeepDiver1975 force-pushed the fix/no-normalization-macos branch from 44b4a8c to a609ad5 Compare January 31, 2025 07:52
@erikjv erikjv force-pushed the fix/no-normalization-macos branch from a609ad5 to ca99d6b Compare January 31, 2025 12:42
@erikjv erikjv force-pushed the fix/no-normalization-macos branch 2 times, most recently from ab3e219 to 4504d25 Compare February 3, 2025 15:29
erikjv and others added 5 commits February 3, 2025 18:25
To prevent `QFile::rename` doing normalization changes to the file name.
Check that a file/directory name with NFC encoding on the server ends up
with the same encoding on the client, and that a subsequent
discovery+sync will not upload differently encoded files. Same for an
NFD encoded file/directory name.
@erikjv erikjv force-pushed the fix/no-normalization-macos branch from 4504d25 to f978da9 Compare February 3, 2025 17:27
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