extract_item: do not delete an existing directory if possible #7866
Conversation
|
Please rebase onto current master to fix the mypy issue. |
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.
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## master #7866 +/- ##
==========================================
- Coverage 83.33% 83.12% -0.22%
==========================================
Files 66 66
Lines 11853 11826 -27
Branches 2149 1866 -283
==========================================
- Hits 9878 9830 -48
- Misses 1393 1407 +14
- Partials 582 589 +7
|
| elif not stat.S_ISDIR(item.mode): | ||
| os.rmdir(path) |
There was a problem hiding this comment.
It's rather unclear still whether the resulting directory after extraction will be correct (== as in the archive) in all cases:
- simple attrs
- xattrs
- acls
- (bsd/fs)flags
The existing directory could have some metadata set already, but the resulting directory must be exactly what we have in the archived item, not a mix up of fs item and archived item.
There was a problem hiding this comment.
The code that makes sure about this could be also useful later if we implement extracting into a non-empty directory (for more than the simplest cases, like the already implemented "continue an extraction").
There was a problem hiding this comment.
hmm, thinking about it...
we currently more or less require extracting into an empty dir (exception: that "continue extraction" feature). we could also require that if there is any pre-existing directory inside the extraction directory, it must not have any special attrs set.
| os.unlink(path) | ||
| # only remove a directory if it is conflicting | ||
| # preserve existing directories because they might be subvolumes |
There was a problem hiding this comment.
is subvolumes a feature implemented by other fs than btrfs?
if not, maybe we better call it "btrfs subvolumes" here, so it is more clear.
|
A small test with an existing fs dir and then extracting "over" that would be good. |
|
@intelfx can you add tests? |
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.
I'm also interested in this feature being backported to Borg 1.2. I have a patch that applies on top of
1.2-maint.