create: do not archive a root inside an earlier directory root twice, fixes #10393

src/borg/archive.py

@@ -1007,6 +1007,10 @@ def same_item(item, st):
         # overwrite files outside the extraction directory (e.g. /etc/passwd).
         self.safe_dirs.discard(path)  # path is about to be (re)created; never trust a stale entry for it
         self._check_safe_parent(item.path)
+        if "hlid" in item and hlm.retrieve(id=item.hlid) == path:
+            # duplicate item: path was extracted already and is the link target of its hard link group,
+            # removing it would lose the file (and hard linking it to itself can not work).
+            return
         # Attempt to remove existing files, ignore errors on failure
         try:
             st = os.stat(path, follow_symlinks=False)

src/borg/archiver/create_cmd.py

@@ -62,6 +62,21 @@ def stat_root(path):
         raise BackupBrokenSymlinkError("stat", "broken symlink, skipping it") from None
 
 
+def skip_key(path, st):
+    """
+    Return the key that identifies the fs object at *path* (with stat result *st*) in the skip_inodes set.
+
+    A directory is identified by its inode alone: borg does not support hard links to directories, so an
+    identical directory inode is the same directory, even if it is reached via a different path.
+
+    Any other fs object is identified by its inode and its path: hard links are different paths pointing
+    to the same inode and each of them must be archived, only the very same path must not be archived twice.
+    """
+    if stat.S_ISDIR(st.st_mode):
+        return st.st_ino, st.st_dev
+    return st.st_ino, st.st_dev, path
+
+
 class CreateMixIn:
     @with_repository()
     def do_create(self, args, repository, manifest):
@@ -235,6 +250,8 @@ def create_inner(archive, cache, fso):
                         raise CommandError(f"Command {args.paths[0]!r} exited with status {rc}")
             else:
                 paths = list(args.pattern_roots) + list(args.paths)
+                # a root can be inside a directory root given before it, _rec_walk needs to know the roots.
+                root_paths = {posixpath.normpath(path) for path in paths}
                 for path in paths:
                     if path == "":  # issue #5637
                         self.print_warning("An empty string was given as PATH, ignoring.")
@@ -277,15 +294,17 @@ def create_inner(archive, cache, fso):
                             exclude_if_present=args.exclude_if_present,
                             keep_exclude_tags=args.keep_exclude_tags,
                             skip_inodes=skip_inodes,
+                            root_paths=root_paths,
                             restrict_dev=restrict_dev,
                             read_special=args.read_special,
                             dry_run=dry_run,
                             strip_prefix=strip_prefix,
                             follow_symlink=followed,
                         )
                         # if we get back here, we've finished recursing into <path>,
-                        # we do not ever want to get back in there (even if path is given twice as recursion root)
-                        skip_inodes.add((st.st_ino, st.st_dev))
+                        # we do not ever want to get back in there (even if path is given twice as recursion root).
+                        # other hard links of a non-directory <path> are not skipped, see skip_key.
+                        skip_inodes.add(skip_key(path, st))
                     except BackupError as e:
                         # this comes from os.stat, self._rec_walk has own exception handler
                         self.print_warning_instance(BackupWarning(path, e))
@@ -585,6 +604,7 @@ def _rec_walk(
         exclude_if_present,
         keep_exclude_tags,
         skip_inodes,
+        root_paths,
         restrict_dev,
         read_special,
         dry_run,
@@ -624,7 +644,7 @@ def _rec_walk(
                 if not stat.S_ISDIR(st.st_mode):
                     return
 
-            if (st.st_ino, st.st_dev) in skip_inodes:
+            if skip_key(path, st) in skip_inodes:
                 return
             # if restrict_dev is given, we do not want to recurse into a new filesystem,
             # but we WILL save the mountpoint directory (or more precise: the root
@@ -692,6 +712,7 @@ def _rec_walk(
                                             exclude_if_present=exclude_if_present,
                                             keep_exclude_tags=keep_exclude_tags,
                                             skip_inodes=skip_inodes,
+                                            root_paths=root_paths,
                                             restrict_dev=restrict_dev,
                                             read_special=read_special,
                                             dry_run=dry_run,
@@ -726,12 +747,18 @@ def _rec_walk(
                                 exclude_if_present=exclude_if_present,
                                 keep_exclude_tags=keep_exclude_tags,
                                 skip_inodes=skip_inodes,
+                                root_paths=root_paths,
                                 restrict_dev=restrict_dev,
                                 read_special=read_special,
                                 dry_run=dry_run,
                                 strip_prefix=strip_prefix,
                             )
 
+            if path in root_paths and (recurse or not stat.S_ISDIR(st.st_mode)):
+                # we are done with <path>, which also is a recursion root: do not process it again as a root.
+                # a directory we did not recurse into (other filesystem) is still processed as a root.
+                skip_inodes.add(skip_key(path, st))
+
         except BackupError as e:
             self.print_warning_instance(BackupWarning(path, e))
             status = "E"
@@ -772,8 +799,11 @@ def build_parser_create(self, subparsers, common_parser, mid_common_parser):
         but let borg find it while recursing (symlinks found that way are never followed).
         A recursion root that is a symlink with a non-existing target is skipped with a warning.
 
-        If you give both a symlink and its target as recursion roots, borg archives the fs
-        objects only once, under the path given first (like for any other root given twice).
+        If you give the same recursion root twice, borg archives it only once. That also
+        applies if you give both a symlink to a directory and that directory as recursion
+        roots: borg archives the fs objects only once, under the path given first.
+        Different paths pointing to the same non-directory fs object (hard links, or a symlink
+        and the file it points to) are all archived if you give them as recursion roots.
 
         When specifying '-' as a path, borg will read data from standard input and create a
         file named 'stdin' in the created archive from that data. In some cases, it is more

src/borg/testsuite/archiver/create_cmd_test.py

@@ -216,6 +216,104 @@ def test_create_duplicate_root(archivers, request):
     assert sorted(paths) == ["input", "input/a", "input/a/hardlink", "input/b", "input/b/hardlink"]
 
 
+def _create_hardlinked_files(archiver):
+    create_regular_file(archiver.input_path, "file1", contents=b"123456")
+    for name in "file2", "file3":
+        os.link(os.path.join(archiver.input_path, "file1"), os.path.join(archiver.input_path, name))
+
+
+def _list_items(archiver, name):
+    archive_list = cmd(archiver, "list", name, "--json-lines", "--format={path}{hlid}")
+    return [json.loads(line) for line in archive_list.split("\n") if line]
+
+
+@requires_hardlinks
+def test_create_hardlinked_roots(archivers, request):
+    # recursion roots that are hard links of each other are different paths and must all be archived,
+    # they are not the same root given twice (#5603).
+    archiver = request.getfixturevalue(archivers)
+    _create_hardlinked_files(archiver)
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input/file1", "input/file2", "input/file3")
+    items = _list_items(archiver, "test")
+    assert [item["path"] for item in items] == ["input/file1", "input/file2", "input/file3"]
+    hlids = {item["hlid"] for item in items}
+    assert len(hlids) == 1 and hlids != {""}  # one hard link group
+    with changedir("output"):
+        cmd(archiver, "extract", "test")
+        sts = [os.stat(f"input/{name}") for name in ("file1", "file2", "file3")]
+        assert {st.st_ino for st in sts} == {sts[0].st_ino}
+        assert all(st.st_nlink == 3 for st in sts)
+        with open("input/file3", "rb") as f:
+            assert f.read() == b"123456"
+
+
+@requires_hardlinks
+def test_create_hardlinked_root_and_parent_dir(archivers, request):
+    # a file root given before its parent directory must not hide its hard links when recursing into
+    # the directory, and the file itself must be archived only once.
+    archiver = request.getfixturevalue(archivers)
+    _create_hardlinked_files(archiver)
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input/file1", "input")
+    paths = [item["path"] for item in _list_items(archiver, "test")]
+    assert sorted(paths) == ["input", "input/file1", "input/file2", "input/file3"]
+    with changedir("output"):
+        cmd(archiver, "extract", "test")
+        assert all(os.stat(f"input/{name}").st_nlink == 3 for name in ("file1", "file2", "file3"))
+
+
+@requires_hardlinks
+def test_create_duplicate_file_root(archivers, request):
+    # the very same file given twice as a recursion root (also with a different spelling of the path)
+    # is archived only once, like a directory given twice (#5603).
+    archiver = request.getfixturevalue(archivers)
+    _create_hardlinked_files(archiver)
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input/file1", "input/file1", "./input/../input/file1", "input/file2")
+    paths = [item["path"] for item in _list_items(archiver, "test")]
+    assert paths == ["input/file1", "input/file2"]
+
+
+@requires_hardlinks
+def test_create_dir_root_and_roots_inside_it(archivers, request):
+    # roots that were already archived while recursing into a directory root given before them must
+    # not be archived again. a duplicate hard link item made borg extract delete the file (#10393).
+    archiver = request.getfixturevalue(archivers)
+    _create_hardlinked_files(archiver)
+    create_regular_file(archiver.input_path, "dir/file", contents=b"abc")
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input", "input/file1", "input/file2", "./input/file3", "input/dir")
+    paths = [item["path"] for item in _list_items(archiver, "test")]
+    assert sorted(paths) == ["input", "input/dir", "input/dir/file", "input/file1", "input/file2", "input/file3"]
+    with changedir("output"):
+        cmd(archiver, "extract", "test")
+        assert all(os.stat(f"input/{name}").st_nlink == 3 for name in ("file1", "file2", "file3"))
+
+
+def test_create_dir_root_and_not_archived_roots_inside_it(archivers, request):
+    # roots inside a directory root given before them, which were not archived while recursing into
+    # that directory, because it did not recurse into their parent directory.
+    archiver = request.getfixturevalue(archivers)
+    create_regular_file(archiver.input_path, "file", contents=b"abc")
+    create_regular_file(archiver.input_path, "tagged/.nobackup")
+    create_regular_file(archiver.input_path, "tagged/file", contents=b"abc")
+    create_regular_file(archiver.input_path, "tagged/dir/file", contents=b"abc")
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(
+        archiver,
+        "create",
+        "--exclude-if-present=.nobackup",
+        "test",
+        "input",
+        "input/tagged/file",
+        "input/tagged/dir",
+        "input/file",
+    )
+    paths = [item["path"] for item in _list_items(archiver, "test")]
+    assert sorted(paths) == ["input", "input/file", "input/tagged/dir", "input/tagged/dir/file", "input/tagged/file"]
+
+
 def test_create_unreadable_parent(archiver):
     parent_dir = os.path.join(archiver.input_path, "parent")
     root_dir = os.path.join(archiver.input_path, "parent", "root")
@@ -1283,8 +1381,8 @@ def test_create_symlink_below_root_not_followed(archivers, request):
 
 @pytest.mark.skipif(not are_symlinks_supported(), reason="symlinks not supported")
 def test_create_symlink_root_and_target(archivers, request):
-    # a followed symlink root and its target are the same fs objects, so they are archived
-    # only once, under the path given first (like any other recursion root given twice).
+    # a followed symlink root and its target directory are the same directory, so its contents are
+    # archived only once, under the path given first (like any other directory given twice).
     archiver = request.getfixturevalue(archivers)
     create_regular_file(archiver.input_path, "target/file", contents=b"content")
     os.symlink("target", os.path.join(archiver.input_path, "link"))
@@ -1295,6 +1393,19 @@ def test_create_symlink_root_and_target(archivers, request):
     assert "input/target/file" not in output
 
 
+@pytest.mark.skipif(not are_symlinks_supported(), reason="symlinks not supported")
+def test_create_symlink_root_and_target_file(archivers, request):
+    # a followed symlink root and its target file are different paths, so both are archived.
+    archiver = request.getfixturevalue(archivers)
+    create_regular_file(archiver.input_path, "target", contents=b"content")
+    os.symlink("target", os.path.join(archiver.input_path, "link"))
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "test", "input/link", "input/target")
+    archive_list = cmd(archiver, "list", "test", "--json-lines")
+    items = [json.loads(line) for line in archive_list.split("\n") if line]
+    assert [(item["path"], item["type"]) for item in items] == [("input/link", "-"), ("input/target", "-")]
+
+
 @pytest.mark.skipif(not are_symlinks_supported(), reason="symlinks not supported")
 def test_create_symlink_root_broken(archivers, request):
     # a recursion root that is a symlink with a non-existing target is skipped with a warning

src/borg/testsuite/archiver/extract_cmd_test.py

@@ -549,6 +549,24 @@ def test_extract_hardlinks_twice(archivers, request):
         assert os.stat("input/b/hardlink").st_nlink == 2
 
 
+@requires_hardlinks
+def test_extract_duplicate_hardlink_item(archivers, request):
+    # an archive can have the same hard link item twice (here: same path given twice via stdin).
+    # extracting the duplicate must not remove the file that is the link target of its hard link group (#10393).
+    archiver = request.getfixturevalue(archivers)
+    create_regular_file(archiver.input_path, "file1", contents=b"123456")
+    os.link(os.path.join(archiver.input_path, "file1"), os.path.join(archiver.input_path, "file2"))
+    cmd(archiver, "repo-create", RK_ENCRYPTION)
+    cmd(archiver, "create", "--paths-from-stdin", "test", input=b"input/file1\ninput/file1\ninput/file2")
+    assert cmd(archiver, "list", "test", "--format={path}{NL}").splitlines() == ["input/file1"] * 2 + ["input/file2"]
+    with changedir("output"):
+        cmd(archiver, "extract", "test")
+        assert os.stat("input/file1").st_nlink == 2
+        assert os.stat("input/file2").st_nlink == 2
+        with open("input/file1", "rb") as f:
+            assert f.read() == b"123456"
+
+
 def test_extract_include_exclude(archivers, request):
     archiver = request.getfixturevalue(archivers)
     cmd(archiver, "repo-create", RK_ENCRYPTION)