extract: do not delete existing directory if possible, fixes #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 #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.

src/borg/archive.py

@@ -831,10 +831,16 @@ Duration: {0.duration}
             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:
+            if not stat.S_ISDIR(st.st_mode):
                 os.unlink(path)
+            elif stat.S_ISDIR(item.mode):
+                # if we have an existing directory and we want to extract a directory,
+                # we just use the existing one and do not remove it.
+                # This fixes the issue that the existing directory might be a BTRFS subvolume.
+                # If we removed it, we would lose the subvolume, see #4233.
+                pass
+            else:
+                os.rmdir(path)  # only works for empty directories
         except UnicodeEncodeError:
             raise self.IncompatibleFilesystemEncodingError(path, sys.getfilesystemencoding()) from None
         except OSError:
@@ -883,6 +889,12 @@ Duration: {0.duration}
                 if not os.path.exists(path):
                     os.mkdir(path)
                 if restore_attrs:
+                    # note: if we did not create the directory freshly, existing attributes
+                    # might get mixed up with the archived attributes. this is acceptable,
+                    # considering we usually extract into an empty base directory.
+                    # when continuing an extraction, the existing attributes and the archived
+                    # attributes should be identical anyway.
+                    # Also, we want to avoid #4223 (losing btrfs subvolumes).
                     self.restore_attrs(path, item)
             elif stat.S_ISLNK(mode):
                 make_parent(path)

src/borg/testsuite/archiver/extract_cmd_test.py

@@ -791,3 +791,20 @@ def test_extract_file_with_missing_chunk(archivers, request):
     output = cmd(archiver, "extract", "archive")
     # TODO: this is a bit dirty still: no warning/error rc, no filename output for the damaged file.
     assert f"repository object {bin_to_hex(chunk.id)} missing, returning {chunk.size} zero bytes." in output
+
+
+def test_extract_existing_directory(archivers, request):
+    # if we extract a directory and there is already a directory at that location,
+    # we should just use the existing directory and not remove/recreate it.
+    archiver = request.getfixturevalue(archivers)
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    os.mkdir("input/dir")
+    cmd(archiver, "create", "test", "input")
+    with changedir("output"):
+        # create pre-existing directory:
+        os.makedirs("input/dir", exist_ok=True)
+        st1 = os.stat("input/dir")
+        # extract
+        cmd(archiver, "extract", "test")
+        st2 = os.stat("input/dir")
+    assert st1.st_ino == st2.st_ino