Add an explicit one-filesystem policy to backups (#18)
* Add an explicit one-filesystem policy to backups scantree() gains an opt-in one_filesystem flag: when a subdirectory's device differs from the walk root's (a mount point), its contents are excluded from the walk. The mount-point directory itself is still yielded, so it appears as an empty directory in the backup rather than vanishing silently - matching rsync's --one-file-system behavior. Wired into rsync()'s two source-tree walks so backups never silently pull in content from a bind mount or other filesystem mounted under the source tree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Fix one-filesystem policy: symlinked root and external rsync parity - scantree() computed the walk root's device with os.lstat(), but scandir() follows the root path if it's a symlink. A symlinked root would compute the symlink's own device instead of the target directory's, making every real subdirectory look cross-device and get its contents wrongly excluded. Use os.stat() for the root only. - rsync_ext() (the external-rsync backend) didn't pass --one-file-system, so backups using external_rsync=True still crossed mount points despite the pure-Python path now refusing to. Addresses GitHub Copilot review feedback on PR #18. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
4 files changed
+109
-5
No files matched your search
+1
-1
@@ -140,7 +140,7 @@ Acceptance criteria:
|
||||
|
||||
- [ ] Document behavior for ACLs, xattrs, flags, sparse files, source hardlinks,
|
||||
ownership, permissions, and timestamps.
|
||||
- [ ] Add an explicit one-filesystem policy.
|
||||
- [x] Add an explicit one-filesystem policy.
|
||||
- [ ] Make Python and external implementations provide equivalent guarantees or
|
||||
document every difference.
|
||||
- [ ] Test supported behavior on Linux and macOS.
|
||||
|
||||
+28
-4
@@ -181,6 +181,7 @@ def rsync_ext(src, dst, dry_run=False) -> Iterable[Tuple[str, Actions, str]]:
|
||||
# rsync_args.append("--inplace")
|
||||
rsync_args.append("--whole-file")
|
||||
rsync_args.append("--human-readable")
|
||||
rsync_args.append("--one-file-system")
|
||||
rsync_args.append("--delete-during")
|
||||
rsync_args.append("--itemize-changes")
|
||||
rsync_args.append(f"{src}/")
|
||||
@@ -219,17 +220,38 @@ def rsync_ext(src, dst, dry_run=False) -> Iterable[Tuple[str, Actions, str]]:
|
||||
process.wait()
|
||||
|
||||
|
||||
def scantree(path, dir_first=True) -> Iterable[os.DirEntry]:
|
||||
def scantree(path, dir_first=True,
|
||||
one_filesystem=False, _root_dev=None) -> Iterable[os.DirEntry]:
|
||||
"""
|
||||
Recursively yield DirEntry objects (dir/file/symlink) for given directory.
|
||||
|
||||
:param one_filesystem: if True, don't descend into a subdirectory that
|
||||
lives on a different device than `path` (a mount point). The
|
||||
directory itself is still yielded, so it appears as an empty
|
||||
directory in the walk - only its contents are excluded. Matches
|
||||
rsync's --one-file-system behavior.
|
||||
"""
|
||||
if one_filesystem and _root_dev is None:
|
||||
# os.stat(), not os.lstat(): scandir() follows `path` if it's a
|
||||
# symlink, so the root device must be the resolved directory's,
|
||||
# not the symlink's own.
|
||||
_root_dev = os.stat(path).st_dev
|
||||
|
||||
entry: os.DirEntry
|
||||
with os.scandir(path) as scan_it:
|
||||
for entry in scan_it:
|
||||
if entry.is_dir(follow_symlinks=False):
|
||||
if (one_filesystem
|
||||
and os.lstat(entry.path).st_dev != _root_dev):
|
||||
_lg.warning(
|
||||
"Scantree, excluding mount point contents: %s",
|
||||
entry.path)
|
||||
yield entry
|
||||
continue
|
||||
if dir_first:
|
||||
yield entry
|
||||
yield from scantree(entry.path, dir_first)
|
||||
yield from scantree(entry.path, dir_first,
|
||||
one_filesystem, _root_dev)
|
||||
if not dir_first:
|
||||
yield entry
|
||||
else:
|
||||
@@ -445,7 +467,8 @@ def rsync(src_dir,
|
||||
|
||||
# Create source map {rel_path: dir_entry}
|
||||
src_files_map = {
|
||||
ent.path[len(src_root_abs) + 1:]: ent for ent in scantree(src_root_abs)
|
||||
ent.path[len(src_root_abs) + 1:]: ent
|
||||
for ent in scantree(src_root_abs, one_filesystem=True)
|
||||
}
|
||||
|
||||
# process dst tree
|
||||
@@ -613,7 +636,8 @@ def rsync(src_dir,
|
||||
yield rel_path, Actions.ERROR, str(exc)
|
||||
|
||||
# restore dir mtimes in dst, updated by updating files
|
||||
for src_entry in scantree(src_root_abs, dir_first=True):
|
||||
for src_entry in scantree(src_root_abs, dir_first=True,
|
||||
one_filesystem=True):
|
||||
if not src_entry.is_dir(follow_symlinks=False):
|
||||
continue
|
||||
rel_path = src_entry.path[len(src_root_abs) + 1:]
|
||||
|
||||
@@ -521,6 +521,85 @@ class TestScantree:
|
||||
sub1_idx = names.index("sub1.txt")
|
||||
assert dir1_idx < sub1_idx
|
||||
|
||||
def test_one_filesystem_default_crosses_mount_points(self, tmp_path,
|
||||
monkeypatch):
|
||||
"""Without one_filesystem, a simulated mount point is descended."""
|
||||
mount_dir = tmp_path / "mount"
|
||||
mount_dir.mkdir()
|
||||
(mount_dir / "other_fs_file.txt").write_text("content")
|
||||
|
||||
real_lstat = os.lstat
|
||||
|
||||
def fake_lstat(path, *args, **kwargs):
|
||||
st = real_lstat(path, *args, **kwargs)
|
||||
if os.path.abspath(path) == str(mount_dir):
|
||||
fields = list(st)
|
||||
fields[2] = st.st_dev + 1 # st_dev
|
||||
st = os.stat_result(fields)
|
||||
return st
|
||||
|
||||
monkeypatch.setattr(os, "lstat", fake_lstat)
|
||||
|
||||
entries = list(fs.scantree(str(tmp_path)))
|
||||
names = [os.path.basename(e.path) for e in entries]
|
||||
assert "other_fs_file.txt" in names
|
||||
|
||||
def test_one_filesystem_excludes_mount_point_contents(self, tmp_path,
|
||||
monkeypatch):
|
||||
"""With one_filesystem, a mount point's contents are excluded."""
|
||||
mount_dir = tmp_path / "mount"
|
||||
mount_dir.mkdir()
|
||||
(mount_dir / "other_fs_file.txt").write_text("content")
|
||||
|
||||
real_lstat = os.lstat
|
||||
|
||||
def fake_lstat(path, *args, **kwargs):
|
||||
st = real_lstat(path, *args, **kwargs)
|
||||
if os.path.abspath(path) == str(mount_dir):
|
||||
fields = list(st)
|
||||
fields[2] = st.st_dev + 1 # st_dev
|
||||
st = os.stat_result(fields)
|
||||
return st
|
||||
|
||||
monkeypatch.setattr(os, "lstat", fake_lstat)
|
||||
|
||||
entries = list(fs.scantree(str(tmp_path), one_filesystem=True))
|
||||
names = [os.path.basename(e.path) for e in entries]
|
||||
|
||||
# the mount point directory itself is still present...
|
||||
assert "mount" in names
|
||||
# ...but its contents are excluded
|
||||
assert "other_fs_file.txt" not in names
|
||||
|
||||
def test_one_filesystem_root_via_symlink(self, tmp_path):
|
||||
"""A symlinked walk root must use the target's device, not its
|
||||
own - scandir() follows the symlink to list real subdirectories,
|
||||
which must not appear to be on a different device than the root."""
|
||||
real_root = tmp_path / "real_root"
|
||||
real_root.mkdir()
|
||||
subdir = real_root / "dir1"
|
||||
subdir.mkdir()
|
||||
(subdir / "file.txt").write_text("content")
|
||||
|
||||
link_root = tmp_path / "link_root"
|
||||
link_root.symlink_to(real_root)
|
||||
|
||||
entries = list(fs.scantree(str(link_root), one_filesystem=True))
|
||||
names = [os.path.basename(e.path) for e in entries]
|
||||
assert "dir1" in names
|
||||
assert "file.txt" in names
|
||||
|
||||
def test_one_filesystem_same_device_unaffected(self, tmp_path):
|
||||
"""With one_filesystem, a same-device tree is fully scanned."""
|
||||
subdir = tmp_path / "dir1"
|
||||
subdir.mkdir()
|
||||
(subdir / "file.txt").write_text("content")
|
||||
|
||||
entries = list(fs.scantree(str(tmp_path), one_filesystem=True))
|
||||
names = [os.path.basename(e.path) for e in entries]
|
||||
assert "dir1" in names
|
||||
assert "file.txt" in names
|
||||
|
||||
|
||||
class TestRmDirentry:
|
||||
"""Test suite for rm_direntry function."""
|
||||
|
||||
@@ -474,6 +474,7 @@ class TestRsyncExt:
|
||||
assert "--human-readable" in args
|
||||
assert "--delete-during" in args
|
||||
assert "--itemize-changes" in args
|
||||
assert "--one-file-system" in args
|
||||
assert args[-2] == f"{src}/"
|
||||
assert args[-1] == str(dst)
|
||||
|
||||
|
||||
Reference in new issue
Block a user