files: pass find -printf \0 as an escape, not a Python NUL (#716)
Some checks are pending
CodeQL / Analyze (push) Waiting to run
Controller Tests / controller-tests (push) Waiting to run
Jest Tests / jest-tests (push) Waiting to run

* files: pass find -printf \\0 as an escape, not a Python NUL

'%h\\0%f\\0%T@\\0' in a Python string is an embedded null byte.  subprocess
cannot pass that as an argument, so every GNU-find refresh on Linux CI
raised ValueError.  find wants the two-character escape \\0 in the format
string and emits NULs itself.

* files: serialize stem backfill so a cancelled compare cannot lock SQLite

compare_file_groups runs ensure_file_group_stems in a thread.  Cancelling
the task does not stop that thread, and a second compare then writes
wrolpi_kv on the same connection (tests share one session).  A lock
plus a version re-check makes the second call a no-op.
This commit is contained in:
lrnselfreliance 2026-09-18 17:31:18 -06:00 • committed by GitHub
parent 9352839106
commit 3a6cf1fa3a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 50 additions and 16 deletions

View file

@ -11,6 +11,7 @@ import re
import shutil
import subprocess
import tarfile
import threading
import time
import urllib.parse
import zipfile
@ -493,6 +494,9 @@ _HARDCODED_SUFFIXES = frozenset(
# before any compare when this version does not match the value in wrolpi_kv.
STEM_ALGORITHM_VERSION = 1
STEM_ALGORITHM_VERSION_KEY = 'stem_algorithm_version'
# Cancelled compare_file_groups leaves ensure_file_group_stems running in a thread.
# Tests (and overlapping refreshes) share one SQLite writer; serialize the backfill.
_stem_backfill_lock = threading.Lock()
PART_PARSER = re.compile(r'(.+?)(\.f[\d]{2,3})?(\.info)?(\.\w{3,4})(\.part)', re.IGNORECASE)
@ -850,22 +854,26 @@ def ensure_file_group_stems():
After a successful backfill the kv version matches, and later refreshes skip the table scan.
Upsert, from_paths, move, and reorganize must keep stem current.
A lock serializes callers: cancelling compare_file_groups does not stop the executor
thread, and a second compare would otherwise write wrolpi_kv concurrently (SQLITE_BUSY).
"""
stored = get_stem_algorithm_version()
if stored == STEM_ALGORITHM_VERSION:
return
with get_db_curs() as curs:
curs.execute('SELECT id, primary_path FROM file_group')
rows = list(curs.fetchall())
if rows:
updates = []
for row in rows:
stem, _ = split_path_stem_and_suffix(row['primary_path'])
updates.append((stem, row['id']))
with get_db_curs(commit=True) as curs:
for chunk in chunks(updates, 500):
curs.executemany('UPDATE file_group SET stem = ? WHERE id = ?', chunk)
set_stem_algorithm_version(STEM_ALGORITHM_VERSION)
with _stem_backfill_lock:
stored = get_stem_algorithm_version()
if stored == STEM_ALGORITHM_VERSION:
return
with get_db_curs() as curs:
curs.execute('SELECT id, primary_path FROM file_group')
rows = list(curs.fetchall())
if rows:
updates = []
for row in rows:
stem, _ = split_path_stem_and_suffix(row['primary_path'])
updates.append((stem, row['id']))
with get_db_curs(commit=True) as curs:
for chunk in chunks(updates, 500):
curs.executemany('UPDATE file_group SET stem = ? WHERE id = ?', chunk)
set_stem_algorithm_version(STEM_ALGORITHM_VERSION)
async def apply_indexers(progress_callback: Callable[[int, int], None] = None):

View file

@ -1,5 +1,6 @@
"""Tests for the file comparison worker."""
import asyncio
import os
from unittest import mock
import pytest
@ -7,6 +8,7 @@ from sqlalchemy import text
from wrolpi.conftest import await_switches
from wrolpi.dates import from_timestamp
from wrolpi.vars import IS_MACOS
from wrolpi.files.models import FileGroup
from wrolpi.files.worker import (
compare_file_groups,
@ -20,6 +22,8 @@ from wrolpi.files.worker import (
_split_nul_field_triples,
_entry_from_printf_fields,
_normalize_roots,
GNU_FIND_PRINTF,
_stream_gnu_find_entries,
)
@ -1803,6 +1807,23 @@ async def test_refresh_two_directories_does_not_delete_third(
'refreshing A and B deleted a stale FileGroup in C'
def test_gnu_find_printf_is_safe_subprocess_arg():
"""find -printf format must not contain a real NUL; that raises ValueError on exec."""
assert '\x00' not in GNU_FIND_PRINTF
os.fsencode(GNU_FIND_PRINTF)
@pytest.mark.skipif(IS_MACOS, reason='GNU find -printf')
@pytest.mark.asyncio
async def test_gnu_find_printf_emits_nul_records(test_directory, make_files_structure):
"""Linux/CI: GNU find -printf with \\0 escapes must actually stream files."""
make_files_structure(['docs/hello.txt'])
find_args = [str(test_directory), '-type', 'f', '-not', '-path', '*/.*']
entries = [e async for e in _stream_gnu_find_entries(find_args)]
names = {filename for _directory, filename, _stem, _mtime in entries}
assert 'hello.txt' in names
def test_split_nul_field_triples_allows_tab_and_newline_in_filename():
"""GNU find -printf uses NUL delimiters so a tab/newline in the name is not a field break."""
data = (

View file

@ -51,6 +51,10 @@ QUEUE_STALL_SECONDS = 30
# 0.5s accommodates SD card / FAT32 / exFAT timestamp granularity on Pi 4.
MTIME_TOLERANCE_SECONDS = 0.5
# GNU find -printf format. `\0` here is the two-character find escape, NOT a Python
# NUL: subprocess arguments cannot contain embedded null bytes.
GNU_FIND_PRINTF = r'%h\0%f\0%T@\0'
__all__ = [
'FileGroupDiff',
'FileComparisonResult',
@ -59,6 +63,7 @@ __all__ = [
'compare_file_groups',
'count_files',
'file_worker',
'GNU_FIND_PRINTF',
]
@ -420,7 +425,7 @@ def _entry_from_printf_fields(directory_b: bytes, filename_b: bytes, mtime_b: by
async def _stream_gnu_find_entries(find_args: list[str]) -> AsyncGenerator[Tuple[str, str, str, float], None]:
"""Stream (directory, filename, stem, mtime) using GNU find -printf with NUL delimiters."""
proc = await asyncio.create_subprocess_exec(
'find', *find_args, '-printf', '%h\0%f\0%T@\0',
'find', *find_args, '-printf', GNU_FIND_PRINTF,
stdout=asyncio.subprocess.PIPE,
)
buf = b''