From 1d6327573ceab03da0a11e765e34e8008d423ee2 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 24 Oct 2026 09:32:01 -0500 Subject: [PATCH] fix: pin companion releases and expose engine failure diagnostics --- .github/workflows/ci.yml | 2 +- CLAUDE.md | 5 +- README.md | 12 +++- docs/developer-guide.md | 5 +- packages/core/README.md | 14 ++++- packages/core/hatch_build.py | 15 +++++ packages/core/pyproject.toml | 18 ++---- .../core/src/python_redlines/__about__.py | 2 +- packages/core/src/python_redlines/__init__.py | 2 + packages/core/src/python_redlines/engines.py | 25 +++++++- tests/test_engine_errors.py | 59 +++++++++++++++++++ tests/test_package_metadata.py | 52 ++++++++++++++++ 12 files changed, 189 insertions(+), 22 deletions(-) create mode 100644 packages/core/hatch_build.py create mode 100644 tests/test_engine_errors.py create mode 100644 tests/test_package_metadata.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aed54e8..8229f46 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,7 +47,7 @@ jobs: run: python build_differ.py ${{ steps.rid.outputs.rid }} - name: Install packages (editable) - run: pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest + run: pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest hatchling - name: Run tests run: python -m pytest tests/ -v diff --git a/CLAUDE.md b/CLAUDE.md index e16f840..70a7c44 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ python build_differ.py linux-x64 python build_differ.py --all # Install all three packages editable for development -pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest +pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest hatchling # Run tests (from repo root) python -m pytest tests/ @@ -122,7 +122,8 @@ python -m build --wheel packages/docxodus # needs an archive in _binaries/ `packages/core/src/python_redlines/__about__.py` is the single source of truth. The two binary packages read it via `[tool.hatch.version] path = "../core/..."`, -so all three always share one version. Bump only that file. +so all three always share one version. The core's `hatch_build.py` metadata hook +generates exact companion dependency pins from the same version. Bump only that file. ## Testing Notes diff --git a/README.md b/README.md index dec1b12..8baf02f 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,16 @@ selects the wheel matching your platform automatically. Instantiating an engine companion package is not installed raises `EngineNotInstalledError` telling you which extra to install. +From 1.0.1 onward, extras pin each engine companion to the wrapper's exact release +version. Older published releases still have unconstrained extras: if you need +0.3.0, install both `python-redlines[docxodus]==0.3.0` and +`python-redlines-docxodus==0.3.0` explicitly. + +If the engine exits unsuccessfully, `run_redline` raises `EngineExecutionError` +(a `subprocess.CalledProcessError` subclass). Its message includes the engine's +captured stderr and stdout; existing handlers can still access `returncode`, +`cmd`, `stdout`/`output`, and `stderr`. + ### Use the Library See the [Quick Start](#quick-start) above for a minimal example, or the @@ -404,7 +414,7 @@ git submodule update --init --recursive python build_differ.py linux-x64 # Install all three packages editable -pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest +pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest hatchling ``` ### Commands diff --git a/docs/developer-guide.md b/docs/developer-guide.md index 9688807..f68e97c 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -36,7 +36,7 @@ git submodule update --init --recursive python build_differ.py linux-x64 # or win-x64 / osx-arm64 / ... # Install all three packages editable, plus pytest -pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest +pip install -e packages/core -e packages/ooxmlpowertools -e packages/docxodus pytest hatchling ``` ## Building the C# binaries @@ -94,4 +94,5 @@ current platform. per-platform engine wheels across three OS runners, builds the core sdist + wheel, and publishes all three packages to PyPI. Bump the version in `packages/core/src/python_redlines/__about__.py` only — the binary packages read it -from there, so all three always release in lockstep. +from there, so all three always release in lockstep. The core metadata hook also +pins every engine extra to that version; no dependency pins need manual updates. diff --git a/packages/core/README.md b/packages/core/README.md index d2f1771..bb08a40 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -28,8 +28,18 @@ redline_bytes, stdout, stderr = engine.run_redline( ) ``` -`DocxodusEngine` accepts `engine="wmlcomparer"` (default) or `engine="docxdiff"` to select the -comparison algorithm. See the [project README](https://github.com/JSv4/Python-Redlines#choosing-an-engine). +`DocxodusEngine` runs DocxDiff. The `engine` selector was removed in 1.0.0 along +with WmlComparer. See the [project README](https://github.com/JSv4/Python-Redlines#comparison-settings-docxodusengine-only) +for supported settings. + +From 1.0.1 onward, each extra pins its companion package to the same release +version as the wrapper. For older releases, pin both packages explicitly (for +example, `python-redlines[docxodus]==0.3.0` and +`python-redlines-docxodus==0.3.0`). Existing published metadata is unchanged. + +An unsuccessful engine exit raises `EngineExecutionError`, a subclass of +`subprocess.CalledProcessError`. Its message includes the captured stderr and +stdout; `returncode`, `cmd`, `stdout`/`output`, and `stderr` remain available. If an engine's companion package is not installed, instantiating the engine raises `EngineNotInstalledError` with the `pip install` command to fix it. diff --git a/packages/core/hatch_build.py b/packages/core/hatch_build.py new file mode 100644 index 0000000..f63c46b --- /dev/null +++ b/packages/core/hatch_build.py @@ -0,0 +1,15 @@ +"""Keep optional engine dependencies in lockstep with the core release.""" +from hatchling.metadata.plugin.interface import MetadataHookInterface + + +class CompanionMetadataHook(MetadataHookInterface): + def update(self, metadata): + version = metadata["version"] + ooxmlpowertools = f"python-redlines-ooxmlpowertools=={version}" + docxodus = f"python-redlines-docxodus=={version}" + # Keep the deprecated engine in both its own extra and `all`. + metadata["optional-dependencies"] = { + "ooxmlpowertools": [ooxmlpowertools], + "docxodus": [docxodus], + "all": [ooxmlpowertools, docxodus], + } diff --git a/packages/core/pyproject.toml b/packages/core/pyproject.toml index dcc7efc..9cabd44 100644 --- a/packages/core/pyproject.toml +++ b/packages/core/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "python-redlines" -dynamic = ["version"] +dynamic = ["version", "optional-dependencies"] description = "Open-source DOCX comparison tool to generate native Word tracked changes (redlines) in Python without MS Word dependencies." readme = "README.md" requires-python = ">=3.9" @@ -40,17 +40,6 @@ dependencies = [ "platformdirs>=3.0", ] -# Engine binaries ship in separate companion packages. Install an engine via -# its extra, e.g. `pip install python-redlines[docxodus]`. All three packages -# are released together from the same repository on each tagged release. -[project.optional-dependencies] -# Deprecated: wraps the archived Open-XML-PowerTools WmlComparer. Kept so that -# anyone who needs that algorithm's output has somewhere to stand; `all` keeps -# installing it so the extra does not change meaning under existing pins. -ooxmlpowertools = ["python-redlines-ooxmlpowertools"] -docxodus = ["python-redlines-docxodus"] -all = ["python-redlines-ooxmlpowertools", "python-redlines-docxodus"] - [project.urls] Homepage = "https://github.com/JSv4/Python-Redlines" Documentation = "https://jsv4.github.io/Python-Redlines/" @@ -61,5 +50,10 @@ Source = "https://github.com/JSv4/Python-Redlines" [tool.hatch.version] path = "src/python_redlines/__about__.py" +# All three packages release together. Generate exact companion pins from the +# same version source, including in editable installs and source distributions. +[tool.hatch.metadata.hooks.custom] +path = "hatch_build.py" + [tool.hatch.build.targets.wheel] packages = ["src/python_redlines"] diff --git a/packages/core/src/python_redlines/__about__.py b/packages/core/src/python_redlines/__about__.py index f58919b..7452bc7 100644 --- a/packages/core/src/python_redlines/__about__.py +++ b/packages/core/src/python_redlines/__about__.py @@ -1,4 +1,4 @@ # SPDX-FileCopyrightText: 2024-present U.N. Owen # # SPDX-License-Identifier: MIT -__version__ = "1.0.0" +__version__ = "1.0.1" diff --git a/packages/core/src/python_redlines/__init__.py b/packages/core/src/python_redlines/__init__.py index 56014f6..d3991e2 100644 --- a/packages/core/src/python_redlines/__init__.py +++ b/packages/core/src/python_redlines/__init__.py @@ -6,6 +6,7 @@ from .engines import ( BaseEngine, DocxodusEngine, + EngineExecutionError, EngineNotInstalledError, XmlPowerToolsEngine, ) @@ -14,6 +15,7 @@ "BaseEngine", "XmlPowerToolsEngine", "DocxodusEngine", + "EngineExecutionError", "EngineNotInstalledError", "__version__", ] diff --git a/packages/core/src/python_redlines/engines.py b/packages/core/src/python_redlines/engines.py index 3cf3cfe..89651b3 100644 --- a/packages/core/src/python_redlines/engines.py +++ b/packages/core/src/python_redlines/engines.py @@ -22,6 +22,21 @@ class EngineNotInstalledError(ImportError): """Raised when an engine is used but its binary package is not installed.""" +class EngineExecutionError(subprocess.CalledProcessError): + """An engine exited unsuccessfully; its diagnostics appear in the message. + + Retains CalledProcessError's returncode, cmd, stdout/output and stderr so + existing subprocess.CalledProcessError handlers keep working. + """ + + def __str__(self): + message = super().__str__() + for label, output in (("stderr", self.stderr), ("stdout", self.stdout)): + if output and output.strip(): + message += f"\n{label}:\n{output.rstrip()}" + return message + + def _detect_rid() -> str: """Return the .NET-style runtime identifier for the current platform.""" os_name = platform.system().lower() @@ -142,6 +157,9 @@ def run_redline(self, author_tag: str, original: Union[str, bytes, Path], modifi A path the caller supplies is never deleted; only scratch files this method creates are cleaned up. + A nonzero engine exit raises EngineExecutionError (a CalledProcessError + subclass) with captured stderr and stdout included in its message. + Additional keyword arguments are passed to _build_command() for engine-specific options. DocxodusEngine supports: case_insensitive, detect_moves, move_similarity_threshold, move_minimum_word_count, detect_format_changes, @@ -163,7 +181,12 @@ def run_redline(self, author_tag: str, original: Union[str, bytes, Path], modifi command = self._build_command(author_tag, original_path, modified_path, target_path, **kwargs) # Capture stdout and stderr - result = subprocess.run(command, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + try: + result = subprocess.run(command, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + except subprocess.CalledProcessError as exc: + raise EngineExecutionError( + exc.returncode, exc.cmd, output=exc.output, stderr=exc.stderr, + ) from exc stdout_output = result.stdout if isinstance(result.stdout, str) and len(result.stdout) > 0 else None stderr_output = result.stderr if isinstance(result.stderr, str) and len(result.stderr) > 0 else None diff --git a/tests/test_engine_errors.py b/tests/test_engine_errors.py new file mode 100644 index 0000000..834193b --- /dev/null +++ b/tests/test_engine_errors.py @@ -0,0 +1,59 @@ +"""Engine failures must expose diagnostics without breaking subprocess callers.""" +import subprocess +import sys +import tempfile + +import pytest + +from python_redlines import BaseEngine + + +@pytest.mark.parametrize("stdout,stderr", [ + ("Usage: redline ...\n", "Error: Unknown flag: --engine=wmlcomparer\n"), + ("The engine explains the failure on stdout\n", ""), + ("", "The engine explains the failure on stderr\n"), + ("", ""), +]) +@pytest.mark.parametrize("use_paths", [False, True]) +def test_failed_engine_exposes_diagnostics_and_cleans_scratch( + tmp_path, monkeypatch, stdout, stderr, use_paths, +): + from python_redlines import EngineExecutionError + + class FailingEngine(BaseEngine): + def _resolve_binary(self): + return sys.executable + + def _build_command(self, *args, **kwargs): + script = ( + f"import sys; sys.stdout.write({stdout!r}); " + f"sys.stderr.write({stderr!r}); sys.exit(2)" + ) + return [self.extracted_binaries_path, "-c", script] + + scratch = tmp_path / "scratch" + scratch.mkdir() + monkeypatch.setattr(tempfile, "tempdir", str(scratch)) + original = tmp_path / "original.docx" + modified = tmp_path / "modified.docx" + original.write_bytes(b"original") + modified.write_bytes(b"modified") + inputs = (original, modified) if use_paths else (b"original", b"modified") + + with pytest.raises(subprocess.CalledProcessError) as caught: + FailingEngine().run_redline("Author", *inputs) + + error = caught.value + assert isinstance(error, EngineExecutionError) + assert error.returncode == 2 + assert error.cmd[:2] == [sys.executable, "-c"] + assert error.stdout == error.output == stdout + assert error.stderr == stderr + assert isinstance(error.__cause__, subprocess.CalledProcessError) + assert "exit status 2" in str(error) + for label, diagnostic in (("stdout", stdout), ("stderr", stderr)): + if diagnostic: + assert f"{label}:\n{diagnostic.rstrip()}" in str(error) + assert not list(scratch.iterdir()) + assert original.read_bytes() == b"original" + assert modified.read_bytes() == b"modified" diff --git a/tests/test_package_metadata.py b/tests/test_package_metadata.py new file mode 100644 index 0000000..866ac49 --- /dev/null +++ b/tests/test_package_metadata.py @@ -0,0 +1,52 @@ +"""Check the shipped dependency metadata, including after a future version bump.""" +from email.parser import BytesParser +from pathlib import Path +import shutil +import tarfile +import zipfile + +from hatchling.builders.sdist import SdistBuilder +from hatchling.builders.wheel import WheelBuilder +from packaging.requirements import Requirement +import pytest + + +@pytest.mark.parametrize("version_override", [None, "9.8.7"]) +def test_core_distributions_pin_companions(tmp_path, version_override): + project = tmp_path / "core" + shutil.copytree( + Path(__file__).resolve().parents[1] / "packages" / "core", project, + ignore=shutil.ignore_patterns("dist", "__pycache__", "*.egg-info"), + ) + if version_override: + (project / "src/python_redlines/__about__.py").write_text( + f'__version__ = "{version_override}"\n', encoding="utf-8", + ) + output = tmp_path / "dist" + output.mkdir() + wheel = next(WheelBuilder(str(project)).build(directory=str(output))) + sdist = next(SdistBuilder(str(project)).build(directory=str(output))) + with zipfile.ZipFile(wheel) as archive: + metadata_name = next(n for n in archive.namelist() if n.endswith(".dist-info/METADATA")) + wheel_metadata = archive.read(metadata_name) + with tarfile.open(sdist) as archive: + assert any(n.endswith("/hatch_build.py") for n in archive.getnames()) + metadata_name = next(n for n in archive.getnames() if n.endswith("/PKG-INFO")) + sdist_metadata = archive.extractfile(metadata_name).read() + + for raw_metadata in (wheel_metadata, sdist_metadata): + metadata = BytesParser().parsebytes(raw_metadata) + version = metadata["Version"] + if version_override: + assert version == version_override + requirements = [Requirement(r) for r in metadata.get_all("Requires-Dist", [])] + companions = [r for r in requirements if r.name.startswith("python-redlines-")] + for extra, names in { + "": set(), + "docxodus": {"python-redlines-docxodus"}, + "ooxmlpowertools": {"python-redlines-ooxmlpowertools"}, + "all": {"python-redlines-docxodus", "python-redlines-ooxmlpowertools"}, + }.items(): + selected = [r for r in companions if r.marker is None or r.marker.evaluate({"extra": extra})] + assert {r.name for r in selected} == names + assert all(str(r.specifier) == f"=={version}" for r in selected)