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

Windows: Enable us to support sync root path > 260 char #10135

Merged
merged 2 commits into from
Oct 7, 2022

Conversation

TheOneRing
Copy link
Member

No description provided.

@TheOneRing TheOneRing requested review from dragotin and a team September 23, 2022 10:04
@TheOneRing TheOneRing marked this pull request as ready for review September 23, 2022 10:04
changelog/unreleased/10135 Outdated Show resolved Hide resolved
Copy link
Contributor

@fmoc fmoc left a comment

Choose a reason for hiding this comment

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

Please see my previous review comment.

Co-authored-by: Fabian Müller <80399010+fmoc@users.noreply.github.com>
@sonarcloud
Copy link

sonarcloud bot commented Oct 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TheOneRing TheOneRing requested a review from fmoc October 6, 2022 07:57
@TheOneRing TheOneRing merged commit 1a98446 into master Oct 7, 2022
@delete-merged-branch delete-merged-branch bot deleted the work/long_pah_db branch October 7, 2022 06:25
@HanaGemela
Copy link
Contributor

HanaGemela commented Nov 10, 2022

Try to sync a file which root path is > 260 char on Windows without VFS. That should work.
With VFS there should be a nice error handling

@HanaGemela
Copy link
Contributor

With VFS follow #10136 to make it work as well

@TheOneRing
Copy link
Member Author

TheOneRing commented Nov 11, 2022

Try to sync a file which root path is > 260 char on Windows without VFS. That should work. With VFS there should be a nice error handling
Not a nice error but an error

@saw-jan
Copy link
Member

saw-jan commented Nov 15, 2022

Try to sync a file which root path is > 260 char on Windows without VFS. That should work. With VFS there should be a nice error handling

@HanaGemela With or without VFS, the file with root path > 260 chars gets synced.
Syncing file:
thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/afile.txt

Steps:

  1. create the above file path in the server (using webUI)
  2. add account to the client (with or without VFS)
  3. wait for sync (sync completes)

Without changing system behavior

  1. ✔️ file which root path is > 260 chars syncs without VFS
  2. ✔️ file which root path is > 260 chars syncs with VFS

System (VM):
Windows: 10 build 1809
Enable Win32 long paths: Disabled
LongPathsEnabled: 0

@TheOneRing
Copy link
Member Author

Same as #10136 (comment) please test a long path to a sync root not a synced file.

@saw-jan
Copy link
Member

saw-jan commented Nov 16, 2022

Same as #10136 (comment) please test a long path to a sync root not a synced file.

With or without the VFS, the long sync root path can be synced without changing system behavior.
Incorrect steps

Screenshot from 2022-11-16 15-11-58
Screenshot from 2022-11-16 15-12-29
Screenshot from 2022-11-16 15-12-43

@saw-jan
Copy link
Member

saw-jan commented Nov 17, 2022

Steps to test:

  1. Create long path > 260 chars in the windows system (use terminal like git bash or https://github.com/PowerShell/PowerShell/releases)
    E.g: C:/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/thisfilehascharacterslongerthantwohundredandsixty/ownCloud
  2. Desktop client: Add account with VFS disabled and sync path pointing to the above path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants