diff --git a/.github/workflows/artifacts.yml b/.github/workflows/artifacts.yml index 87e08fe..a4470b6 100644 --- a/.github/workflows/artifacts.yml +++ b/.github/workflows/artifacts.yml @@ -70,11 +70,18 @@ jobs: printf '{"a": 1, "b": [2, 4]}' > "$RUNNER_TEMP/to.json" exit_status=0 bindist/dist/graphtage --no-status --format yaml \ - "$RUNNER_TEMP/from.json" "$RUNNER_TEMP/to.json" || exit_status=$? + "$RUNNER_TEMP/from.json" "$RUNNER_TEMP/to.json" 2> "$RUNNER_TEMP/stderr.txt" || exit_status=$? if [ "$exit_status" -ne 1 ]; then echo "Expected exit status 1 (differences found), got $exit_status" >&2 exit 1 fi + # A frozen binary can produce a correct diff and still write a traceback, if something it imports + # re-executes sys.executable -- which is the binary itself. `--no-status` promises an empty stderr. + if [ -s "$RUNNER_TEMP/stderr.txt" ]; then + echo "Expected --no-status to leave stderr empty. It wrote:" >&2 + cat "$RUNNER_TEMP/stderr.txt" >&2 + exit 1 + fi # `gh release upload` can only add assets to a release that already exists, so a stray tag # cannot publish a release of its own. It uploads to a draft just as well as to a published diff --git a/graphtage/progress.py b/graphtage/progress.py index 245a053..49ec2ab 100644 --- a/graphtage/progress.py +++ b/graphtage/progress.py @@ -2,12 +2,19 @@ import io import sys +import threading from collections.abc import Iterable, Iterator from types import TracebackType from typing import IO, AnyStr, TextIO from tqdm import tqdm, trange +# Graphtage draws its progress bars from a single process, so tqdm's default multiprocessing lock guards nothing. +# Building that lock registers a semaphore with multiprocessing.resource_tracker, which starts a helper process by +# re-executing sys.executable with the interpreter's own flags. Under PyInstaller sys.executable is the Graphtage +# binary, so the helper re-runs Graphtage with flags that belong to a Python interpreter. +tqdm.set_lock(threading.RLock()) + class StatusWriter(IO[str]): """A writer compatible with the :class:`graphtage.printer.Writer` protocol that can print status. diff --git a/test/test_progress.py b/test/test_progress.py index ff9f84d..dc53bac 100644 --- a/test/test_progress.py +++ b/test/test_progress.py @@ -17,6 +17,19 @@ PROGRESS_BAR = b"Diffing:" +HELPER_PROCESS_PROBE = """ +import multiprocessing.resource_tracker as resource_tracker + +from graphtage.progress import StatusWriter + +writer = StatusWriter(quiet=False) +with writer.tqdm(desc="probe", total=1, leave=False) as bar: + bar.update(1) +writer.flush(final=True) +print(resource_tracker._resource_tracker._pid) +""" +"""Draws one progress bar the way :mod:`graphtage.__main__` does, then reports the resource tracker's process ID.""" + def run_graphtage(*args: str) -> tuple[bytes, bytes]: """Runs the command line in a subprocess and returns what it wrote to stdout and to stderr. @@ -84,3 +97,25 @@ def test_replacement_printer_reaches_every_progress_bar(self): ) finally: printer.set_default_printer(original) + + def test_drawing_a_progress_bar_starts_no_helper_process(self): + """Drawing a progress bar must not start a :mod:`multiprocessing.resource_tracker` helper process. + + tqdm builds a :class:`multiprocessing.RLock` for its default write lock. Registering that lock's semaphore + starts the resource tracker, which re-executes :attr:`sys.executable` with the interpreter's own flags. Under + PyInstaller :attr:`sys.executable` is the Graphtage binary, so the helper re-ran Graphtage with ``-B -S -I`` + and every diff through the macOS binary printed a traceback to stderr. Graphtage never uses multiprocessing, + so it installs a threading lock instead. + + The sibling tests here cannot catch this: from a source checkout the helper is a real interpreter and exits + quietly, so stderr stays empty either way. This asserts on the mechanism, in a subprocess because the tracker + is process-wide and starts at most once. + + """ + result = subprocess.run([sys.executable, "-c", HELPER_PROCESS_PROBE], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual( + "None", + result.stdout.strip(), + "the resource tracker was started, so tqdm most likely built a multiprocessing lock", + )