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

Create placeholder while dehydrating if needed #5890

Merged

Conversation

m7913d
Copy link
Contributor

@m7913d m7913d commented Jul 15, 2023

When replacing an OnlineOnly file by another one, the file maintains it OnlineOnly pin state, but it is converted to a regular file. So, the dehydration should convert the regular file to a (dehydrated) placeholder instead of trying to update the (non-existing) placeholder.

Closes #4274

The same problem is also discussed in the following threads:

Steps to reproduce the the issue:
- Create file "a"
- Create folder "A"
- Copy file "a" to "A"
- Let the client sync the local changes to the server.
- Choose "Free up space" for "A/a" in Windows explorer
- Copy file "a" again to "A" (and let Windows replace the file)
- "Can't update non existing placeholder info" error is shown and "A/a" will indefinitely show the syncing icon.

@mgallien mgallien self-requested a review July 17, 2023 09:43
@m7913d m7913d force-pushed the fixCantUpdateNonExistingPlaceholder branch from 4034de7 to 376a318 Compare July 17, 2023 09:44
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

thanks
see my comments

src/libsync/vfs/cfapi/cfapiwrapper.cpp Outdated Show resolved Hide resolved
src/libsync/vfs/cfapi/cfapiwrapper.cpp Outdated Show resolved Hide resolved
@m7913d m7913d force-pushed the fixCantUpdateNonExistingPlaceholder branch 2 times, most recently from c25309a to 68fbea0 Compare July 17, 2023 09:56
@m7913d m7913d requested a review from mgallien July 17, 2023 09:56
@mgallien mgallien force-pushed the fixCantUpdateNonExistingPlaceholder branch from 68fbea0 to 96be7a3 Compare July 18, 2023 12:34
@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #5890 (9256417) into master (759c2a2) will decrease coverage by 0.03%.
The diff coverage is 66.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5890      +/-   ##
==========================================
- Coverage   60.08%   60.06%   -0.03%     
==========================================
  Files         145      145              
  Lines       18756    18759       +3     
==========================================
- Hits        11270    11268       -2     
- Misses       7486     7491       +5     
Impacted Files Coverage Δ
src/libsync/vfs/cfapi/cfapiwrapper.cpp 72.80% <66.66%> (-0.27%) ⬇️

... and 1 file with indirect coverage changes

When replacing an OnlineOnly file by another one, the file maintains it
OnlineOnly pin state, but it is converted to a regular file. So, the
dehydration should convert the regular file to a (dehydrated)
placeholder instead of trying to update the (non-existing) placeholder.

Closes nextcloud#4274

Signed-off-by: Dries Mys <[email protected]>
@mgallien mgallien force-pushed the fixCantUpdateNonExistingPlaceholder branch from 96be7a3 to 9256417 Compare July 20, 2023 13:07
@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5890-9256417612e36df64a96fff2ffe97e6f8a8fa8f7-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien
Copy link
Collaborator

/backport to stable-3.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Can't update non existing placeholder info" multiple messages
3 participants