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.

Sign up for GitHub

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

extract_item: do not delete an existing directory if possible #7866

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

extract_item: do not delete an existing directory if possible #7866

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/borg/archive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,10 +858,13 @@ def same_item(item, st):
st = os.stat(path, follow_symlinks=False)
if continue_extraction and same_item(item, st):
return # done! we already have fully extracted this file in a previous run.
elif stat.S_ISDIR(st.st_mode):
os.rmdir(path)
else:
# remove anything that is not a directory
if not stat.S_ISDIR(st.st_mode):
os.unlink(path)
# only remove a directory if it is conflicting
# preserve existing directories because they might be subvolumes
Copy link
Member

Choose a reason for hiding this comment

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

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.

elif not stat.S_ISDIR(item.mode):
os.rmdir(path)
Comment on lines +866 to +867
Copy link
Member

@ThomasWaldmann ThomasWaldmann Oct 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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").

Copy link
Member

Choose a reason for hiding this comment

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

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.

except UnicodeEncodeError:
raise self.IncompatibleFilesystemEncodingError(path, sys.getfilesystemencoding()) from None
except OSError:
Expand Down