From 304cac2d6b0e2089b82d22d9a83e3e3e4c882901 Mon Sep 17 00:00:00 2001 From: Vassili Tchersky Date: Thu, 30 Jul 2026 22:55:58 +0000 Subject: [PATCH 1/4] tests,cmdline: cache get_env_flags and has_features Also, some FreeBSD compat for the bind mounts and remove a stray ANSI reset code from --version so that has_features() parses them properly. --- docs/testing.rst | 2 +- lib/cmdline.c | 6 +- tests/test_types/test_duplicate.py | 4 +- tests/utils.py | 119 ++++++++++++++++------------- 4 files changed, 74 insertions(+), 57 deletions(-) diff --git a/docs/testing.rst b/docs/testing.rst index e8c6f9b3..88cfa662 100644 --- a/docs/testing.rst +++ b/docs/testing.rst @@ -5,7 +5,7 @@ Testsuite complete yet (and probably never will), but it's already a valuable boost of confidence in ``rmlint's`` correctness. -The tests are based on ``pytest`` and are written in ``python>=3.6``. +The tests are based on ``pytest`` and are written in ``python>=3.9``. Every testcase just runs the (previously built) ``rmlint`` binary a and parses its json output. So they are technically blackbox-tests. diff --git a/lib/cmdline.c b/lib/cmdline.c index 45f5a127..875d0d86 100644 --- a/lib/cmdline.c +++ b/lib/cmdline.c @@ -76,13 +76,13 @@ static void rm_cmd_show_version(void) { fprintf(stderr, " %c%s", (features[i].enabled) ? '+' : '-', features[i].name); } - fprintf(stderr, RESET "\n\n"); + fputs("\n\n", stderr); fprintf(stderr, _("rmlint was written by Christopher Pahl and Daniel " " Thomas.")); - fprintf(stderr, "\n"); + fputc('\n', stderr); fprintf(stderr, _("The code at https://github.com/sahib/rmlint is licensed under the " "terms of the GPLv3.")); - fprintf(stderr, "\n"); + fputc('\n', stderr); exit(0); } diff --git a/tests/test_types/test_duplicate.py b/tests/test_types/test_duplicate.py index dbd6f6e9..a67b8a1c 100644 --- a/tests/test_types/test_duplicate.py +++ b/tests/test_types/test_duplicate.py @@ -1,4 +1,4 @@ -from tests.utils import create_dirs, create_file, create_link, get_testdir, run_rmlint, use_valgrind +from tests.utils import create_dirs, create_file, create_link, get_env_flag, get_testdir, run_rmlint def test_small_diffs(): @@ -8,7 +8,7 @@ def create_data(length, flips=()): data[flip] = '1' return ''.join(data) - if use_valgrind(): + if get_env_flag('use_valgrind'): size = 32 else: # Takes horribly long elsewhise diff --git a/tests/utils.py b/tests/utils.py index 27a1e278..c005331e 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1,17 +1,21 @@ """Utilities""" import contextlib import json +import logging import os import pprint import re import shlex +import shutil import struct import subprocess import sys import tempfile import time +from functools import cache import psutil +import pytest import xattr # TESTDIR_BASE holds every test directory. It is not created automatically. @@ -39,59 +43,72 @@ def get_testdir(): return _TESTDIR +# XXX: metrocrc* used to be gated behind inexistent 'sse4' feature. CKSUM_TYPES = [ 'murmur', - 'metro', - 'metro256', + 'metro', 'metro256', + # 'metrocrc', 'metrocrc256' 'md5', 'sha1', - 'sha256', - 'sha512', - 'sha3-256', - 'sha3-384', - 'sha3-512', - 'blake2s', - 'blake2b', - 'blake2sp', - 'blake2bp', - 'blake3', - 'blake3_512', + 'sha256', 'sha512', + 'sha3-256', 'sha3-384', 'sha3-512', + 'blake2s', 'blake2b', 'blake2sp', 'blake2bp', + 'blake3', 'blake3_512', 'xxhash', - 'highway64', - 'highway128', - 'highway256', - #'cumulative', - #'ext', + 'highway64', 'highway128', 'highway256', + # 'cumulative', 'ext', 'paranoid', ] -def get_env_flag(name): +@cache +def get_env_flag(name: str) -> bool: + env_name = f'RM_TS_{name.upper()}' try: - return int(os.environ.get(name) or 0) + return bool(int(os.environ.get(env_name, 0))) except ValueError: - print(f'{name} should be an integer.') - return 0 + logging.warning("%s should be an integer; assuming 0.", env_name) + return False -_USE_VALGRIND = get_env_flag('RM_TS_USE_VALGRIND') -_PRINT_CMD = get_env_flag('RM_TS_PRINT_CMD') -_SLEEP = get_env_flag('RM_TS_SLEEP') -_FEATURES = subprocess.check_output( - [RMLINT_BINARY, '--version'], stderr=subprocess.STDOUT).decode('utf-8') +@cache +def features() -> dict[str, bool]: + version = subprocess.run( + (RMLINT_BINARY, '--version'), + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + check=True, + text=True, + ).stderr + match = re.search(r'^compiled with:\s*(.+)$', version, re.MULTILINE) + if not match: + raise RuntimeError(f"could not extract features from: \n{version}") -def use_valgrind(): - return _USE_VALGRIND + result = {} + for token in match.group(1).split(): + sign, name = token[0], token[1:] + if sign not in "+-": + raise RuntimeError(f"unexpected feature token {token!r}") + result[name] = sign == '+' + return result -def has_feature(feature): - return '+' + feature in _FEATURES +def has_feature(feature: str) -> bool: + try: + return features()[feature] + except KeyError: + raise LookupError( + f"{feature!r} is not a known rmlint feature " + f"(known: {sorted(features())})" + ) from None -if has_feature('sse4'): - CKSUM_TYPES.append('metrocrc') - CKSUM_TYPES.append('metrocrc256') +@cache +def get_bash() -> str: + if bash_path := shutil.which("bash"): + return bash_path + raise RuntimeError('bash not found.') def runs_as_root(): @@ -142,15 +159,15 @@ def run_rmlint_once(*args, else: target_dir = "" - if use_valgrind(): + if get_env_flag('use_valgrind'): env = { 'G_DEBUG': 'gc-friendly', 'G_SLICE': 'always-malloc' } cmd = ['valgrind', '--error-exitcode=1', '-q'] - if get_env_flag('RM_TS_CHECK_LEAKS') and not has_known_leak(*args): + if get_env_flag('check_leaks') and not has_known_leak(*args): cmd += ('--leak-check=full', '--show-leak-kinds=definite', '--errors-for-leak-kinds=definite') - elif get_env_flag('RM_TS_USE_GDB'): + elif get_env_flag('use_gdb'): env, cmd = {}, ['gdb', '-batch', '--silent', '-ex=run', '-ex=thread apply all bt', '-ex=quit', '--args'] else: env, cmd = {}, [] @@ -181,7 +198,7 @@ def run_rmlint_once(*args, } if use_shell: - run_args['executable'] = "bash" + run_args['executable'] = get_bash() if uses_py_formatter: # The py formatter writes its JSON document to `.rmlint.json` in @@ -190,17 +207,17 @@ def run_rmlint_once(*args, with contextlib.suppress(FileNotFoundError): os.unlink(os.path.join(get_testdir(), '.rmlint.json')) - if _PRINT_CMD: + if get_env_flag('print_cmd'): print(f"running{' in shell' if use_shell else ''} from `{get_testdir()}`: {' '.join(cmd)}") - if _SLEEP: + if get_env_flag('sleep'): print('Waiting for 1000 seconds.') time.sleep(1000) result = subprocess.run(' '.join(cmd) if use_shell else cmd, **run_args) sys.stdout.buffer.write(result.stderr) - if get_env_flag('RM_TS_USE_GDB'): + if get_env_flag('use_gdb'): sys.stdout.buffer.write(b"\n==> START OF GDB OUTPUT <==\n") sys.stdout.buffer.write(result.stdout) sys.stdout.buffer.write(b"==> END OF GDB OUTPUT <==\n") @@ -289,7 +306,7 @@ def run_rmlint_pedantic(*args, **kwargs): '--algorithm=paranoid --limit-mem 1M' ] - + # XXX: 'paranoid' is in CKSUM_TYPES for cksum_type in CKSUM_TYPES: options.append('--algorithm=' + cksum_type) @@ -312,7 +329,7 @@ def run_rmlint_pedantic(*args, **kwargs): # We cannot compare checksum in all cases. # XXX: algorithm options must be grouped at the end of the options list. # TODO: end-to-end tests of algorithms - compare_checksum = not any((option.startswith('--algorithm='), + compare_checksum = not any((option.startswith('--algorithm='), option.startswith('-P'), option.startswith('-p'))) if (data_skip and 'directly_return_output' not in kwargs @@ -419,17 +436,17 @@ def create_special_fs(name, fs_type='ext4'): @contextlib.contextmanager def bind_mount_a_b(mnt_root): mnt_dir = os.path.join(mnt_root, 'a/b') - subprocess.call( - f'mount --rbind {mnt_root} {mnt_dir}', - shell=True - ) + if sys.platform.startswith("linux"): + subprocess.call(('mount', '--bind', mnt_root, mnt_dir)) + elif sys.platform.startswith("freebsd"): + pytest.xfail("https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=297174") + subprocess.call(('mount', '-t', 'nullfs', mnt_root, mnt_dir)) + else: + pytest.skip(f"bind_mount: {sys.platform} not implemented/supported") try: yield finally: - subprocess.call( - f'umount {mnt_dir}', - shell=True - ) + subprocess.call(('umount', mnt_dir)) def must_read_xattr(path): From 652d9a15f8f9d3da195c9c16ecdad8006489eef6 Mon Sep 17 00:00:00 2001 From: Vassili Tchersky Date: Thu, 30 Jul 2026 22:58:38 +0000 Subject: [PATCH 2/4] tests: a bit of platform agnosticism (FreeBSD compat) Use standard cc, not gcc; use pathconf. Use FreeBSD usertools where needed. Abort configure if pkg-config is not available. --- SConstruct | 2 +- site_scons/rm_build_checks.py | 9 +++-- tests/test_formatters/test_sh.py | 5 +-- tests/test_mains/test_is_reflink.py | 5 +++ .../test_robustness/test_manylongpathfiles.py | 25 ++++++++----- tests/test_types/test_baduids.py | 36 ++++++++++++++----- tests/test_types/test_nonstripped.py | 2 +- 7 files changed, 60 insertions(+), 24 deletions(-) diff --git a/SConstruct b/SConstruct index f8976a96..7d47cfeb 100755 --- a/SConstruct +++ b/SConstruct @@ -192,7 +192,7 @@ else: # check _mm_crc32_u64 (SSE4.2) support: conf.check_mm_crc32_u64() -if IS_CLANG := any(cc in os.path.basename(conf.env['CC']) for cc in ('clang', 'include-what-you-use')): +if IS_CLANG := conf.CheckDeclaration("__clang__"): conf.env.Append(CCFLAGS=['-fcolor-diagnostics']) # Colored warnings conf.env.Append(CCFLAGS=['-Qunused-arguments']) # Hide wrong messages conf.env.Append(CCFLAGS=['-Wno-bad-function-cast']) diff --git a/site_scons/rm_build_checks.py b/site_scons/rm_build_checks.py index b6c6e4af..27edff9f 100644 --- a/site_scons/rm_build_checks.py +++ b/site_scons/rm_build_checks.py @@ -15,9 +15,12 @@ def check_pkgconfig(context, version): context.Message('Checking for pkg-config... ') command = PKG_CONFIG + ' --atleast-pkgconfig-version=' + version - ret = context.TryAction(command)[0] - context.Result(ret) - return ret + rc, _ = context.TryAction(command) + if not rc: + print("Error: pkg-config not found (or too old).") + Exit(1) + context.Result(rc) + return rc def check_pkg(context, name, varname, required=True): diff --git a/tests/test_formatters/test_sh.py b/tests/test_formatters/test_sh.py index d3ad4c33..ba37fb70 100644 --- a/tests/test_formatters/test_sh.py +++ b/tests/test_formatters/test_sh.py @@ -15,8 +15,9 @@ def run_shell_script(shell, sh_path, *args): return subprocess.check_output( [shell_path, sh_path] + list(args), - shell=False - ).decode("utf-8") + shell=False, + text=True + ) def filter_part_of_directory(data): diff --git a/tests/test_mains/test_is_reflink.py b/tests/test_mains/test_is_reflink.py index 0e5751e1..64827d9a 100644 --- a/tests/test_mains/test_is_reflink.py +++ b/tests/test_mains/test_is_reflink.py @@ -9,10 +9,15 @@ create_file, create_link, get_testdir, + has_feature, run_rmlint, run_rmlint_once, ) +if not has_feature('fiemap'): + pytest.skip("rmlint was compiled without fiemap support", + allow_module_level=True) + def check_is_reflink_status(status_code, *paths): with assert_exit_code(status_code): diff --git a/tests/test_robustness/test_manylongpathfiles.py b/tests/test_robustness/test_manylongpathfiles.py index 1f782201..c9da508f 100644 --- a/tests/test_robustness/test_manylongpathfiles.py +++ b/tests/test_robustness/test_manylongpathfiles.py @@ -1,25 +1,32 @@ +import os + import pytest -from tests.utils import create_dirs, create_file, run_rmlint +from tests.utils import create_dirs, create_file, get_testdir, run_rmlint @pytest.mark.slow def test_manylongpathfiles(): + path_max = os.pathconf(get_testdir(), "PC_PATH_MAX") + path_max = path_max if 0 < path_max <= 1024 else 1024 + prefix = os.path.abspath(get_testdir()) + os.sep + budget = path_max - len(prefix) - 12 + + # four equally-sized path components, up to min(1024, PATH_MAX) + component_len = (budget - 4) // 4 + component = "l" * component_len + longpath = (component + "/") * 4 - #create ~1000 character path, 4 dirs deep - longpath = ("long" * (1000//4//4) + "/") * 4 create_dirs(longpath) - # create heaps of identical files: numfiles = 1024 * 32 + 1 for i in range(numfiles): - create_file('xxx', longpath + 'file' + str(i).zfill(7)) + create_file("xxx", longpath + f"file{i:07d}") - # create heaps of identical pairs: numpairs = 1024 * 32 + 1 for i in range(numpairs): - create_file(str(i), longpath + 'a' + str(i).zfill(7)) - create_file(str(i), longpath + 'b' + str(i).zfill(7)) + create_file(str(i), longpath + f"a{i:07d}") + create_file(str(i), longpath + f"b{i:07d}") - _, *data, _ = run_rmlint('') + _, *data, _ = run_rmlint("") assert len(data) == numfiles + numpairs * 2 diff --git a/tests/test_types/test_baduids.py b/tests/test_types/test_baduids.py index cebc017a..573a6e4b 100644 --- a/tests/test_types/test_baduids.py +++ b/tests/test_types/test_baduids.py @@ -1,10 +1,36 @@ import subprocess +import sys + +import pytest from tests.utils import create_file, get_testdir, run_rmlint, runs_as_root RMLINT_DUMMY_GROUP = '__rmlint_dummy_group' RMLINT_DUMMY_USER = '__rmlint_dummy_user' +if sys.platform.startswith('linux'): + ADD_ID_CMDS = ( + 'groupadd {g}', + 'useradd -M -N {u}', + ) + DEL_ID_CMDS = ( + 'userdel -r {u}', + 'groupdel {g}', + ) +elif sys.platform.startswith('freebsd'): + ADD_ID_CMDS = ( + 'pw groupadd -n {g}', + 'pw useradd -n {u}', + ) + DEL_ID_CMDS = ( + 'pw userdel -n {u}', + 'pw groupdel -n {g}', + ) +else: + ADD_ID_CMDS = DEL_ID_CMDS = None + pytest.skip(f"uid/gid: {sys.platform} not implemented/supported", + allow_module_level=True) + def exec_cmds(cmds): for cmd in cmds: @@ -24,10 +50,7 @@ def test_bad_ids(): if not runs_as_root(): return - exec_cmds([ - 'groupadd {g}', - 'useradd -M -N {u}', - ]) + exec_cmds(ADD_ID_CMDS) try: create_file('x', '1_bad_uid') @@ -40,10 +63,7 @@ def test_bad_ids(): 'chown {u}:{g} {t}/3_bad_gid_and_uid' ]) finally: - exec_cmds([ - 'userdel -r {u}', - 'groupdel {g}' - ]) + exec_cmds(DEL_ID_CMDS) _, *data, footer = run_rmlint('-S a') diff --git a/tests/test_types/test_nonstripped.py b/tests/test_types/test_nonstripped.py index 24073644..c62cfc07 100644 --- a/tests/test_types/test_nonstripped.py +++ b/tests/test_types/test_nonstripped.py @@ -23,7 +23,7 @@ def create_binary(path, stripped=False): full_path = os.path.join(get_testdir(), path) command = '{cc} -o {path} {option} -std=c99 -xc -'.format( - cc=os.environ.get('CC', 'gcc'), path=full_path, option='-s' if stripped else '-ggdb3', + cc=os.environ.get('CC', 'cc'), path=full_path, option='-s' if stripped else '-g3', ) subprocess.run(command, input=SOURCE, shell=True, text=True, check=True) From 6a39d7487429aaa15558722d995ed261500ceae3 Mon Sep 17 00:00:00 2001 From: Vassili Tchersky Date: Thu, 30 Jul 2026 23:22:00 +0000 Subject: [PATCH 3/4] ci: test on FreeBSD --- .github/workflows/freebsd.yml | 29 +++++++++++++++++++++++++++++ ci/freebsd/build-and-test.sh | 14 ++++++++++++++ ci/freebsd/prepare.sh | 21 +++++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 .github/workflows/freebsd.yml create mode 100755 ci/freebsd/build-and-test.sh create mode 100755 ci/freebsd/prepare.sh diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml new file mode 100644 index 00000000..d616fef8 --- /dev/null +++ b/.github/workflows/freebsd.yml @@ -0,0 +1,29 @@ +name: FreeBSD + +on: + pull_request: + types: + - opened + - synchronize + - reopened + - ready_for_review + push: + branches: [master] + workflow_dispatch: + +jobs: + test: + name: FreeBSD + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - name: Build and test + uses: vmactions/freebsd-vm@v1 + with: + mem: 12288 + copyback: false + cache-after-prepare: true + prepare: | + ./ci/freebsd/prepare.sh + run: | + ./ci/freebsd/build-and-test.sh diff --git a/ci/freebsd/build-and-test.sh b/ci/freebsd/build-and-test.sh new file mode 100755 index 00000000..80557b4a --- /dev/null +++ b/ci/freebsd/build-and-test.sh @@ -0,0 +1,14 @@ +#!/bin/sh +set -eu + +cd "$(dirname "$0")/../.." +echo "==> $(freebsd-version) $(uname -m), $(sysctl -n hw.ncpu) cpus, $(pwd)" + +scons_ARGS="VERBOSE=1 DEBUG=1 O=release" +scons config $scons_ARGS +scons $scons_ARGS + +mount -t tmpfs none /rt +RM_TS_DIR=/rt pytest -m "not slow" + +echo "==> tests passed" diff --git a/ci/freebsd/prepare.sh b/ci/freebsd/prepare.sh new file mode 100755 index 00000000..89dff70a --- /dev/null +++ b/ci/freebsd/prepare.sh @@ -0,0 +1,21 @@ +#!/bin/sh +set -eu + +cd "$(dirname "$0")/../.." +export ASSUME_ALWAYS_YES=yes + +PACKAGES="rsync git py312-sphinx" +PACKAGES_BUILD="scons-py312 pkgconf glib json-glib libblkid gettext py312-py-cpuinfo" +PACKAGES_TEST="bash dash py312-pip" + +echo "==> pkg install: $PACKAGES" + +pkg install -y $PACKAGES +pkg install -y $PACKAGES_BUILD +pkg install -y $PACKAGES_TEST +pip install -r tests/requirements.txt +pip install -r docs/requirements.txt + +mkdir /rt + +echo "==> prepare done" From e117e3e79bcd70becd67ea2c21f67d7c387d3acf Mon Sep 17 00:00:00 2001 From: Vassili Tchersky Date: Fri, 31 Jul 2026 12:22:12 +0000 Subject: [PATCH 4/4] utilities: fix CPP comments On FreeBSD: HAVE_FIEMAP=0 but RM_MOUNTTABLE_IS_USABLE=1 --- lib/utilities.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/utilities.c b/lib/utilities.c index 2efbd460..973e5589 100644 --- a/lib/utilities.c +++ b/lib/utilities.c @@ -689,7 +689,7 @@ static gchar rm_mounts_is_rotational_blockdev(const char *dev) { fclose(sys_fdes); #else (void)dev; -#endif +#endif /* HAVE_SYSBLOCK */ return is_rotational; } @@ -1012,7 +1012,7 @@ void rm_mounts_table_destroy(RmMountTable *self) { g_slice_free(RmMountTable, self); } -#else /* probably FreeBSD */ +#else RmMountTable *rm_mounts_table_new(_UNUSED bool force_fiemap) { return NULL; @@ -1105,7 +1105,7 @@ dev_t rm_mounts_get_disk_id(RmMountTable *self, _UNUSED dev_t dev, (void)dev; (void)path; return 0; -#endif +#endif /* RM_MOUNTTABLE_IS_USABLE */ } dev_t rm_mounts_get_disk_id_by_path(RmMountTable *self, const char *path) { @@ -1290,7 +1290,7 @@ RmOff rm_offset_get_from_path(const char *path, RmOff file_offset, return result; } -#else /* Probably FreeBSD */ +#else /* FreeBSD */ RmOff rm_offset_get_from_fd(_UNUSED int fd, _UNUSED RmOff file_offset, _UNUSED RmOff *file_offset_next, _UNUSED RmOff *logical_offset, @@ -1303,7 +1303,7 @@ RmOff rm_offset_get_from_path(_UNUSED const char *path, _UNUSED RmOff file_offse return 0; } -#endif +#endif /* HAVE_FIEMAP */ static gboolean rm_util_is_path_double(const char *path1, const char *path2) { const char *basename1 = rm_util_basename(path1);