extract: do not delete existing directory if possible, fixes #4233#9288
Open
ThomasWaldmann wants to merge 2 commits intoborgbackup:masterfrom
Open
extract: do not delete existing directory if possible, fixes #4233#9288ThomasWaldmann wants to merge 2 commits intoborgbackup:masterfrom
ThomasWaldmann wants to merge 2 commits intoborgbackup:masterfrom
Conversation
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. |
…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.
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.83% 75.83% -0.01%
==========================================
Files 86 86
Lines 14747 14753 +6
Branches 2194 2196 +2
==========================================
+ Hits 11184 11188 +4
- Misses 2890 2891 +1
- Partials 673 674 +1 ☔ View full report in Codecov by Sentry. |
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).
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
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.