From: Andrew Tridgell Date: Thu, 21 May 2026 04:29:29 +0000 (+1000) Subject: testsuite: restore non-Linux xattr/fake-super coverage X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4b862306e537084abc8e5bfe18014817288a4305;p=thirdparty%2Frsync.git testsuite: restore non-Linux xattr/fake-super coverage The Python rewrite had gated the xattr / fake-super tests (xattrs, xattrs-hlink, chown-fake, devices-fake) to Linux because it used the Linux-only os.*xattr. Restore them on macOS, FreeBSD, Cygwin and Solaris via a per-OS xattr surface in rsyncfns.py (xattrs_supported / xattr_set / xattr_dump): * Linux -- os.*xattr * macOS -- xattr * FreeBSD -- setextattr / lsextattr / getextattr * Cygwin -- getfattr / setfattr (from the `attr` package; CPython on Cygwin has no os.*xattr) * Solaris -- runat(1), with the script on stdin and the attr name/value passed via the environment (the runat -c form mangles args) Test attribute names are logical; the "user." namespace prefix is added only on the Linux-style platforms (Linux, Cygwin). RSYNC_PREFIX/RUSR vary per OS (macOS and Solaris use rsync.nonuser to avoid rsync's reserved rsync.* space). The macOS and Cygwin workflows no longer skip these tests; the FreeBSD/Solaris jobs use IGNORE skip-checking so need no change. Verified on real Linux, macOS, FreeBSD, Cygwin and Solaris hosts. Co-Authored-By: Claude Opus 4.7 (1M context) --- diff --git a/.github/workflows/cygwin-build.yml b/.github/workflows/cygwin-build.yml index 268a4792..946166fa 100644 --- a/.github/workflows/cygwin-build.yml +++ b/.github/workflows/cygwin-build.yml @@ -39,11 +39,12 @@ jobs: - name: info run: bash -c '/usr/local/bin/rsync --version' - name: check - # The fake-super / xattr tests (chown-fake, devices-fake, xattrs, - # xattrs-hlink) skip on Cygwin: the Python rewrite currently does the - # xattr operations via os.setxattr, which CPython only provides on - # Linux. (Restoring non-Linux xattr coverage is a follow-up.) - run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls,bare-do-open-symlink-race,chdir-symlink-race,chown-fake,chown,daemon-chroot-acl,devices-fake,devices,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,sender-flist-symlink-leak,simd-checksum,symlink-dirlink-basis,xattrs-hlink,xattrs make check' + # chown-fake / devices-fake / xattrs / xattrs-hlink now RUN on Cygwin + # (rsyncfns.py drives xattrs via getfattr/setfattr from the `attr` + # package installed above), verified on a real Cygwin host. The real + # chown/devices tests still skip (need root/mknod), as do the + # RESOLVE_BENEATH symlink-race tests. + run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls,bare-do-open-symlink-race,chdir-symlink-race,chown,daemon-chroot-acl,devices,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,sender-flist-symlink-leak,simd-checksum,symlink-dirlink-basis make check' - name: check (TCP daemon transport) # Second run with daemon tests over a real loopback rsyncd; the default # 'make check' above uses the secure stdio-pipe transport. diff --git a/.github/workflows/macos-build.yml b/.github/workflows/macos-build.yml index 19ef8a75..f019e6c4 100644 --- a/.github/workflows/macos-build.yml +++ b/.github/workflows/macos-build.yml @@ -41,7 +41,10 @@ jobs: - name: info run: rsync --version - name: check - run: sudo RSYNC_EXPECT_SKIPPED=acls-default,chmod-temp-dir,chown-fake,daemon-chroot-acl,devices-fake,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,simd-checksum,xattrs-hlink,xattrs make check + # chown-fake / devices-fake / xattrs / xattrs-hlink now RUN on macOS + # (rsyncfns.py drives xattrs via the `xattr` command), verified on a + # real macOS host, so they're no longer in the skip set. + run: sudo RSYNC_EXPECT_SKIPPED=acls-default,chmod-temp-dir,daemon-chroot-acl,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,simd-checksum make check - name: check (TCP daemon transport) # Second run with daemon tests over a real loopback rsyncd; the default # 'make check' above uses the secure stdio-pipe transport. diff --git a/testsuite/chown_test.py b/testsuite/chown_test.py index 80559820..116b97ef 100644 --- a/testsuite/chown_test.py +++ b/testsuite/chown_test.py @@ -8,15 +8,12 @@ # tests --fake-super. import os -import platform -import shutil -import subprocess import sys import rsyncfns from rsyncfns import ( - FROMDIR, TODIR, - checkit, run_rsync, test_fail, test_skipped, + FROMDIR, RSYNC_PREFIX, TODIR, + checkit, test_skipped, xattr_set, xattrs_supported, ) @@ -26,28 +23,20 @@ script_name = os.path.basename(sys.argv[0] if sys.argv[0] else __file__) fake_variant = 'fake' in script_name if fake_variant: - # --fake-super needs xattrs support. - vv = run_rsync('-VV', check=True, capture_output=True) - if '"xattrs": true' not in vv.stdout: + # --fake-super needs xattrs support (and a way to set them here). + if not xattrs_supported(): test_skipped("Rsync needs xattrs for fake device tests") # Augment the RSYNC command and TLS_ARGS so checkit's listing path # treats the xattr-encoded ownership as the file's real ownership. rsyncfns.RSYNC = rsyncfns.RSYNC + ' --fake-super' rsyncfns.TLS_ARGS = (rsyncfns.TLS_ARGS + ' --fake-super').strip() - if platform.system() != 'Linux': - test_skipped( - f"fake chown emulation not implemented for {platform.system()}" - ) - def chown_or_fake(path, uid, gid): - # On Linux, store ownership in the user.rsync.%stat xattr -- the - # format rsync's --fake-super expects. - stat = os.stat(path) - mode = stat.st_mode + # Store ownership in rsync's fake-super "%stat" xattr -- the name + # (RSYNC_PREFIX) and namespace vary by OS; xattr_set handles that. # %stat format: "MODE DEV_MAJOR,DEV_MINOR UID:GID" - value = f"{mode:o} 0,0 {uid}:{gid}".encode() - os.setxattr(str(path), b'user.rsync.%stat', value) + mode = os.stat(path).st_mode + xattr_set(f'{RSYNC_PREFIX}.%stat', f"{mode:o} 0,0 {uid}:{gid}", path) return True else: rsyncfns.RSYNC = rsyncfns.RSYNC + ' --super' diff --git a/testsuite/devices_test.py b/testsuite/devices_test.py index b6af3eb9..28734f52 100644 --- a/testsuite/devices_test.py +++ b/testsuite/devices_test.py @@ -7,17 +7,15 @@ # user.rsync.%stat xattr instead of mknod-ing real devices. import os -import platform -import shutil import subprocess import sys import rsyncfns from rsyncfns import ( - CHKDIR, CHKFILE, FROMDIR, OUTFILE, TMPDIR, TODIR, + CHKDIR, CHKFILE, FROMDIR, OUTFILE, RSYNC_PREFIX, TMPDIR, TODIR, all_plus, allspace, dots, checkdiff, hands_setup, makepath, rsync_ls_lR, run_rsync, - test_fail, test_skipped, v_filt, + test_fail, test_skipped, v_filt, xattr_set, xattrs_supported, ) @@ -25,32 +23,25 @@ script_name = os.path.basename(sys.argv[0] if sys.argv[0] else __file__) fake_variant = 'fake' in script_name if fake_variant: - vv = run_rsync('-VV', check=True, capture_output=True) - if '"xattrs": true' not in vv.stdout: + if not xattrs_supported(): test_skipped("Rsync needs xattrs for fake device tests") rsyncfns.RSYNC = rsyncfns.RSYNC + ' --fake-super' rsyncfns.TLS_ARGS = (rsyncfns.TLS_ARGS + ' --fake-super').strip() - if platform.system() != 'Linux': - test_skipped( - f"fake device emulation not implemented for {platform.system()}" - ) - def make_special(path, kind: str, major: int = 0, minor: int = 0) -> bool: - """Pretend to mknod `path` as kind {'p','c','b'} via an xattr. - - Returns True on success, False if the FS rejects the xattr (so the - caller can skip). + """Pretend to mknod `path` as kind {'p','c','b'} via rsync's + fake-super "%stat" xattr (name/namespace handled per-OS by + xattr_set). Returns True on success, False if the FS rejects it. """ mode = {'p': 0o10644, 'c': 0o20644, 'b': 0o60644}[kind] try: with open(path, 'w'): pass - value = f"{mode:o} {major},{minor} 0:0".encode() - os.setxattr(str(path), b'user.rsync.%stat', value) + xattr_set(f'{RSYNC_PREFIX}.%stat', + f"{mode:o} {major},{minor} 0:0", path) return True - except OSError: + except (OSError, subprocess.CalledProcessError): return False else: my_uid = os.getuid() diff --git a/testsuite/rsyncfns.py b/testsuite/rsyncfns.py index 375d0d4a..5b42654e 100644 --- a/testsuite/rsyncfns.py +++ b/testsuite/rsyncfns.py @@ -20,6 +20,7 @@ from __future__ import annotations import atexit import fcntl import os +import platform import shlex import shutil import socket as _socket @@ -511,6 +512,128 @@ def rsync_getgroups() -> list: return out.split() +# --- extended attributes (per-OS surface) ---------------------------------- +# Mirrors the per-OS xset/xls/RSYNC_PREFIX/RUSR logic from the old +# testsuite/rsync.fns + xattrs.test so the xattr / fake-super tests run on +# Linux, macOS and FreeBSD (not just Linux). Test attributes use literal +# names ("user.foo" etc., exactly as the shell did on every platform); only +# rsync's own fake-super attribute name (RSYNC_PREFIX, used for the +# "%stat" attr) and the special "equal" attr (RUSR) vary by OS. + +_SYSTEM = platform.system() + +# Cygwin reports "CYGWIN_NT-10.0-..." and uses Linux-style user.* xattrs +# (rsync builds there with HAVE_LINUX_XATTRS), but CPython on Cygwin lacks +# os.*xattr, so we drive the getfattr/setfattr CLIs there instead. +_CYGWIN = _SYSTEM.startswith('CYGWIN') + +# Platforms whose user xattrs live in the "user." namespace encoded in the +# attribute name (Linux and Cygwin). macOS/FreeBSD carry the namespace out +# of band and a literal "user." prefix is actually rejected there. +_LINUX_NS = _SYSTEM == 'Linux' or _CYGWIN + +# Test attribute names are LOGICAL (un-prefixed, e.g. "foo", "rsync.%stat"); +# _xattr_full() adds the "user." prefix on the Linux-namespace platforms. +# RSYNC_PREFIX is the logical name of rsync's own fake-super attr ("rsync" +# -> "rsync.%stat", and "user.rsync.%stat" on Linux/Cygwin). RUSR is the +# prefix for the test's "equal" attr; macOS and Solaris use "rsync.nonuser" +# to stay clear of rsync's reserved "rsync.*" space. +RSYNC_PREFIX = 'rsync' +RUSR = 'rsync.nonuser' if _SYSTEM in ('Darwin', 'SunOS') else 'rsync' + + +def _xattr_full(name: str) -> str: + """Map a logical user-xattr name to the on-disk name for this OS.""" + return ('user.' + name) if _LINUX_NS else name + + +def xattrs_supported() -> bool: + """True if this rsync was built with xattr support AND this platform has + a way for the tests to set/list user xattrs.""" + vv = run_rsync('-VV', check=True, capture_output=True).stdout + if '"xattrs": true' not in vv: + return False + if _SYSTEM == 'Linux': + return hasattr(os, 'setxattr') + if _CYGWIN: + return shutil.which('setfattr') is not None + if _SYSTEM == 'Darwin': + return shutil.which('xattr') is not None + if _SYSTEM == 'FreeBSD': + return shutil.which('setextattr') is not None + if _SYSTEM == 'SunOS': + return shutil.which('runat') is not None + return False # NetBSD/etc.: not yet ported + + +def xattr_set(name: str, value: str, *paths) -> 'None': + """Set the user-namespace xattr `name` (logical) = `value` on each path.""" + full = _xattr_full(name) + for p in paths: + p = str(p) + if _SYSTEM == 'Linux': + os.setxattr(p, full.encode(), value.encode()) + elif _CYGWIN: + subprocess.run(['setfattr', '-n', full, '-v', value, p], + check=True) + elif _SYSTEM == 'Darwin': + subprocess.run(['xattr', '-w', full, value, p], check=True) + elif _SYSTEM == 'FreeBSD': + subprocess.run(['setextattr', '-h', 'user', full, value, p], + check=True) + elif _SYSTEM == 'SunOS': + # Solaris extended attributes are a per-file namespace; runat + # cd's into it and runs a shell that reads the script on stdin + # (the -c form mangles args). Pass name/value via the environment + # to dodge quoting; printf writes the value with no trailing + # newline, matching the byte-exact value other platforms store. + subprocess.run( + ['runat', p, '/bin/sh'], + input='printf %s "$XVAL" > "$XNAME"\n', text=True, + env={**os.environ, 'XNAME': full, 'XVAL': value}, check=True) + else: + raise NotImplementedError(f"xattr_set on {_SYSTEM}") + + +def xattr_dump(*paths) -> str: + """Return a deterministic name=value dump of the user xattrs on `paths`, + for comparing a source tree against its rsync'd copy. The format only + needs to be self-consistent on a given OS (we never compare across OSes), + mirroring the per-OS xls() in the old xattrs.test.""" + if _SYSTEM == 'Linux' or _CYGWIN: + return subprocess.check_output( + ['getfattr', '-d', *(str(p) for p in paths)], text=True) + if _SYSTEM == 'Darwin': + out = [] + for p in paths: + t = subprocess.check_output(['xattr', '-l', str(p)], text=True) + out.append('\n'.join(ln.lstrip(' \t') for ln in t.splitlines())) + out.append('\n') + return ''.join(out) + if _SYSTEM == 'FreeBSD': + out = [] + for p in paths: + names = subprocess.check_output( + ['lsextattr', '-q', '-h', 'user', str(p)], text=True).split() + for n in sorted(names): + out.append(subprocess.check_output( + ['getextattr', '-h', 'user', n, str(p)], text=True)) + return ''.join(out) + if _SYSTEM == 'SunOS': + # List the file's extended-attribute namespace via runat (script on + # stdin), skipping the always-present SUNWattr_* system attrs, and + # dump name=value (sorted glob order; $(cat) drops a trailing newline). + script = ('for x in *; do case "$x" in SUNWattr_*) continue;; esac; ' + 'printf "%s=%s\\n" "$x" "$(cat "$x")"; done\n') + out = [] + for p in paths: + out.append(subprocess.run( + ['runat', str(p), '/bin/sh'], input=script, + capture_output=True, text=True, check=True).stdout) + return ''.join(out) + raise NotImplementedError(f"xattr_dump on {_SYSTEM}") + + def runtest(label: str, fn, *args, **kwargs): """Run a sub-test step with an echoed label, like rsync.fns runtest. diff --git a/testsuite/xattrs_test.py b/testsuite/xattrs_test.py index a2cfc5e6..18e317f5 100644 --- a/testsuite/xattrs_test.py +++ b/testsuite/xattrs_test.py @@ -8,45 +8,19 @@ # survive alongside xattrs. import os -import platform import subprocess import sys from rsyncfns import ( - CHKDIR, FROMDIR, SCRATCHDIR, TMPDIR, TODIR, TOOLDIR, + CHKDIR, FROMDIR, RSYNC_PREFIX, RUSR, SCRATCHDIR, TMPDIR, TODIR, TOOLDIR, checkit, cp_touch, makepath, run_rsync, test_fail, test_skipped, + xattr_set as xset, xattr_dump, xattrs_supported, ) -vv = run_rsync('-VV', check=True, capture_output=True) -if '"xattrs": true' not in vv.stdout: - test_skipped("Rsync is configured without xattr support") - -if platform.system() != 'Linux': - test_skipped(f"xattr surface not implemented for {platform.system()}") - -# Per-OS xattr surfaces -- Linux only here (other platforms test_skipped'd -# above). RSYNC_PREFIX is the name-prefix rsync itself looks for; RUSR is -# the prefix the test uses for "%stat"-style faux-attributes (must match -# how --fake-super stores them). -RSYNC_PREFIX = 'user.rsync' -RUSR = 'user.rsync' - - -def xset(name: str, value: str, *paths): - """Set the named xattr to `value` on each of `paths`.""" - val = value.encode() - for p in paths: - try: - os.setxattr(str(p), name.encode(), val) - except OSError as e: - raise OSError(f"setxattr {name}={value} on {p}: {e}") - - -def xls(*paths) -> str: - """Mirror `getfattr -d` -- a per-path dump of name=value lines.""" - return subprocess.check_output(['getfattr', '-d', *(str(p) for p in paths)], - text=True) +if not xattrs_supported(): + test_skipped("Rsync is configured without xattr support (or no xattr " + "tooling on this platform)") script_name = os.path.basename(sys.argv[0] if sys.argv[0] else __file__) @@ -84,47 +58,47 @@ uid_gid = f"{m.group(1)}:{m.group(2)}" os.chdir(FROMDIR) try: - xset('user.foo', 'foo', 'file0') + xset('foo', 'foo', 'file0') except OSError: test_skipped("Unable to set an xattr") -xset('user.bar', 'bar', 'file0') +xset('bar', 'bar', 'file0') -xset('user.short', 'this is short', 'file1') -xset('user.long', +xset('short', 'this is short', 'file1') +xset('long', 'this is a long attribute that will be truncated in the initial data send', 'file1') -xset('user.good', 'this is good', 'file1') -xset('user.nice', 'this is nice', 'file1') +xset('good', 'this is good', 'file1') +xset('nice', 'this is nice', 'file1') -xset('user.foo', 'foo', 'file2') -xset('user.bar', 'bar', 'file2') -xset('user.long', +xset('foo', 'foo', 'file2') +xset('bar', 'bar', 'file2') +xset('long', 'a long attribute for our new file that tests to ensure that this works', 'file2') -xset('user.dir1', 'need to test directory xattrs too', 'foo') -xset('user.dir2', 'another xattr', 'foo') -xset('user.dir3', 'this is one last one for the moment', 'foo') +xset('dir1', 'need to test directory xattrs too', 'foo') +xset('dir2', 'another xattr', 'foo') +xset('dir3', 'this is one last one for the moment', 'foo') -xset('user.dir4', 'another dir test', 'foo/bar') -xset('user.dir5', 'one last one', 'foo/bar') +xset('dir4', 'another dir test', 'foo/bar') +xset('dir5', 'one last one', 'foo/bar') -xset('user.foo', 'new foo', 'foo/file3', 'foo/bar/file5') -xset('user.bar', 'new bar', 'foo/file3', 'foo/bar/file5') -xset('user.long', +xset('foo', 'new foo', 'foo/file3', 'foo/bar/file5') +xset('bar', 'new bar', 'foo/file3', 'foo/bar/file5') +xset('long', 'this is also a long attribute that will be truncated in the initial data send', 'foo/file3', 'foo/bar/file5') xset(f'{RUSR}.equal', 'this long attribute should remain the same and not need to be transferred', 'foo/file3', 'foo/bar/file5') -xset('user.dir0', 'old extra value', CHKDIR / 'foo') -xset('user.dir1', 'old dir value', CHKDIR / 'foo') +xset('dir0', 'old extra value', CHKDIR / 'foo') +xset('dir1', 'old dir value', CHKDIR / 'foo') -xset('user.short', 'old short', CHKDIR / 'file1') -xset('user.extra', 'remove me', CHKDIR / 'file1') +xset('short', 'old short', CHKDIR / 'file1') +xset('extra', 'remove me', CHKDIR / 'file1') -xset('user.foo', 'old foo', CHKDIR / 'foo' / 'file3') +xset('foo', 'old foo', CHKDIR / 'foo' / 'file3') xset(f'{RUSR}.equal', 'this long attribute should remain the same and not need to be transferred', CHKDIR / 'foo' / 'file3') @@ -144,8 +118,7 @@ else: def _save_xattrs(paths, dest_file): """Snapshot the xattrs of `paths` (relative to cwd) into dest_file.""" - out = subprocess.check_output(['getfattr', '-d', *paths], text=True) - dest_file.write_text(out) + dest_file.write_text(xattr_dump(*paths)) _save_xattrs(dirs + files, SCRATCHDIR / 'xattrs.txt') @@ -156,7 +129,7 @@ XFILT = ['-f-x_system.*', '-f-x_security.*'] checkit(['-avX', *XFILT, *dashH, '--super', '.', f'{CHKDIR}/'], FROMDIR, CHKDIR) os.chdir(CHKDIR) -got = subprocess.check_output(['getfattr', '-d', *(dirs + files)], text=True) +got = xattr_dump(*(dirs + files)) expected = (SCRATCHDIR / 'xattrs.txt').read_text() if got != expected: from difflib import unified_diff @@ -178,7 +151,7 @@ checkit(['-aiX', *XFILT, *dashH, '--super', f'{altDest}=../chk', '.', '../to'], FROMDIR, TODIR) os.chdir(TODIR) -got = subprocess.check_output(['getfattr', '-d', *(dirs + files)], text=True) +got = xattr_dump(*(dirs + files)) if got != expected: test_fail("xattr listing differs after --copy-dest / --link-dest copy") @@ -190,7 +163,7 @@ os.chdir(FROMDIR) import shutil shutil.rmtree(TODIR, ignore_errors=True) -xset('user.nice', 'this is nice, but different', 'file1') +xset('nice', 'this is nice, but different', 'file1') _save_xattrs(dirs + files, SCRATCHDIR / 'xattrs.txt') @@ -198,7 +171,7 @@ checkit(['-aiX', *XFILT, *dashH, '--fake-super', '--link-dest=../chk', '.', '../ CHKDIR, TODIR) os.chdir(TODIR) -got = subprocess.check_output(['getfattr', '-d', *(dirs + files)], text=True) +got = xattr_dump(*(dirs + files)) expected = (SCRATCHDIR / 'xattrs.txt').read_text() if got != expected: test_fail("xattr listing differs after --fake-super --link-dest copy") @@ -232,7 +205,7 @@ os.chmod('.', 0o700) for p in dirs + files: os.chmod(p, os.stat(p).st_mode & ~0o077) -xset('user.nice', 'this is nice, but different', 'file1') +xset('nice', 'this is nice, but different', 'file1') xset(f'{RSYNC_PREFIX}.%stat', f'40000 0,0 {uid_gid}', *dirs) xset(f'{RSYNC_PREFIX}.%stat', f'100000 0,0 {uid_gid}', *files) @@ -247,7 +220,7 @@ checkit(['-aiX', *XFILT, *dashH, '--fake-super', '--chmod=a=', '.', '../to'], CHKDIR, TODIR) os.chdir(TODIR) -got = subprocess.check_output(['getfattr', '-d', *(dirs + files)], text=True) +got = xattr_dump(*(dirs + files)) expected = (SCRATCHDIR / 'xattrs.txt').read_text() if got != expected: test_fail("xattr listing differs after --fake-super --chmod=a= copy") @@ -272,7 +245,7 @@ if dashH: (lnkdir / 'extra-link').unlink() os.chdir(TODIR) -got = subprocess.check_output(['getfattr', '-d', 'file1', 'file2'], text=True) +got = xattr_dump('file1', 'file2') expected = (SCRATCHDIR / 'xattrs.txt').read_text() if got != expected: test_fail("xattr listing differs after --link-dest=../lnk copy") @@ -287,6 +260,6 @@ run_rsync('-aX', '.', '../chk/') checkit(['-aiiX', *XFILT, '.', '../to'], CHKDIR, TODIR) os.chdir(TODIR) -got = subprocess.check_output(['getfattr', '-d', 'file1', 'file2'], text=True) +got = xattr_dump('file1', 'file2') if got != expected: test_fail("xattr listing differs after the final round")