extract: do not delete existing directory if possible, fixes #4233 #9288
+78
−25
Conversation
This file contains hidden or 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
Member
Author
|
TODO: Maybe That would avoid any potential issues with calling restore_attrs on directories that already have metadata. Also it would speed up continuing an extraction. |
ThomasWaldmann
force-pushed
the
directory-extraction-master
branch
from
03f0347 to
fdfb856
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9288 +/- ##
==========================================
- Coverage 75.89% 75.89% -0.01%
==========================================
Files 86 86
Lines 14761 14767 +6
Branches 2198 2200 +2
==========================================
+ Hits 11203 11207 +4
- Misses 2884 2885 +1
- Partials 674 675 +1 ☔ View full report in Codecov by Sentry. |
ThomasWaldmann
force-pushed
the
directory-extraction-master
branch
from
8db7b98 to
73a5241
Compare
…up#4233 A pre-existing directory might be a btrfs subvolume that was created by the user ahead of time when restoring several nested subvolumes from a single archive. If the archive item to be extracted is a directory and there is already a directory at the destination path, do not remove (and recreate) it, but just use it. That way, btrfs subvolumes (which look like directories) are not deleted. Fix originally contributed by @intelfx in borgbackup#7866, but needed more work, so I thought more about the implications and added a test. Note: In the past, we first removed (empty) directories, then created a fresh one, then called restore_attrs for that. That produced correct metadata, but only for the case of an EMPTY exisiting directory. If the existing directory was not empty, the simply os.rmdir we tried did not work anyway and did not remove the existing directory. Usually we extract to an empty base directory, thus encountering this edge case is mostly limited to continuing a previous extraction. In that case, calling restore_attrs again on a directory that already has existing attrs should be harmless, because they are identical.
if an already existing fs directory has the correct (as archived) mtime, we have already extracted it in a previous borg extract run and we do not need and should not call restore_attrs for it again. if the directory exists, but does not have the correct mtime, restore_attrs will be called and its attributes will be extracted (and mtime set to correct value).
ThomasWaldmann
force-pushed
the
directory-extraction-master
branch
from
73a5241 to
9a43efa
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
A pre-existing directory might be a btrfs subvolume that was created by the user ahead of time when restoring several nested subvolumes from a single archive.
If the archive item to be extracted is a directory and there is already a directory at the destination path, do not remove (and recreate) it, but just use it.
That way, btrfs subvolumes (which look like directories) are not deleted.
Fix originally contributed by @intelfx in #7866, but needed more work, so I thought more about the implications and added a test.
Note:
In the past, we first removed (empty) directories, then created a fresh one, then called restore_attrs for that. That produced correct metadata, but only for the case of an EMPTY exisiting directory. If the existing directory was not empty, the simply os.rmdir we tried did not work anyway and did not remove the existing directory.
Usually we extract to an empty base directory, thus encountering this edge case is mostly limited to continuing a previous extraction. In that case, calling restore_attrs again on a directory that already has existing attrs should be harmless, because they are identical.