diff --git a/imagepipeline/core/runner.py b/imagepipeline/core/runner.py index f3f07e5..62c40ae 100644 --- a/imagepipeline/core/runner.py +++ b/imagepipeline/core/runner.py @@ -280,4 +280,7 @@ class PipelineRunner: if len(ordered_ids) != len(self.steps): raise CycleError("Pipeline contains a cycle in step dependencies") - return [step_by_id[step_id] for step_id in ordered_ids] + ordered = [step_by_id[step_id] for step_id in ordered_ids] + deferred = [step for step in ordered if step.module.runs_last] + regular = [step for step in ordered if not step.module.runs_last] + return regular + deferred diff --git a/imagepipeline/modules/base.py b/imagepipeline/modules/base.py index c4527e8..a083cdb 100644 --- a/imagepipeline/modules/base.py +++ b/imagepipeline/modules/base.py @@ -14,6 +14,7 @@ class BaseModule(ABC): name: ClassVar[str] description: ClassVar[str] = "" + runs_last: ClassVar[bool] = False supported_input_formats: ClassVar[tuple[str, ...]] = ( ".jpg", ".jpeg", diff --git a/imagepipeline/modules/xcf_stack.py b/imagepipeline/modules/xcf_stack.py index e0b9f59..c4efdda 100644 --- a/imagepipeline/modules/xcf_stack.py +++ b/imagepipeline/modules/xcf_stack.py @@ -13,11 +13,11 @@ from imagepipeline.utils.gimp import stack_images_to_xcf @register class XcfStackModule(SubprocessModule): name = "xcf_stack" + runs_last = True description = ( "Stack all prior pipeline step outputs as GIMP layers into one XCF per image" ) command_candidates = ("gimp-console", "gimp") - default_timeout = 600.0 @classmethod def expected_output_filenames( @@ -42,11 +42,17 @@ class XcfStackModule(SubprocessModule): default=False, help="Skip missing step outputs instead of failing", ), + "timeout": Param( + "float", + default=120.0, + help="Seconds to wait for gimp-console per image (default 120)", + ), } def run(self, ctx: ModuleContext) -> None: include_input = ctx.params["include_input"] skip_missing = ctx.params["skip_missing"] + timeout = ctx.params["timeout"] layer_sources: list[tuple[str, Path]] = [] if include_input: @@ -86,4 +92,9 @@ class XcfStackModule(SubprocessModule): ) dst = ctx.output_dir / f"{stem}.xcf" - stack_images_to_xcf(layers, dst, timeout=self.default_timeout) + stack_images_to_xcf(layers, dst, timeout=timeout) + + def list_output_images(self, ctx: ModuleContext) -> list[Path]: + return sorted( + p for p in ctx.output_dir.iterdir() if p.is_file() and p.suffix.lower() == ".xcf" + ) diff --git a/imagepipeline/utils/gimp.py b/imagepipeline/utils/gimp.py index c1837fe..402d0ed 100644 --- a/imagepipeline/utils/gimp.py +++ b/imagepipeline/utils/gimp.py @@ -69,21 +69,39 @@ def _run_gimp_batch( *, timeout: float | None, env: dict[str, str], + outfile: Path | None = None, ) -> subprocess.CompletedProcess[str]: + """Run GIMP batch; tolerate exit hang when ``outfile`` was already written.""" + process = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + env=env, + ) try: - return subprocess.run( - cmd, - check=False, - capture_output=True, - text=True, - timeout=timeout, - env=env, - ) + stdout, stderr = process.communicate(timeout=timeout) except subprocess.TimeoutExpired as exc: + process.kill() + stdout, stderr = process.communicate() + if outfile is not None and outfile.is_file() and outfile.stat().st_size > 0: + return subprocess.CompletedProcess( + cmd, + returncode=0, + stdout=stdout or "", + stderr=stderr or "", + ) raise RuntimeError( f"Command timed out after {timeout}s: {' '.join(cmd)}" ) from exc + return subprocess.CompletedProcess( + cmd, + returncode=process.returncode if process.returncode is not None else -1, + stdout=stdout or "", + stderr=stderr or "", + ) + def stack_images_to_xcf( layers: list[tuple[str, Path]], @@ -134,10 +152,7 @@ def stack_images_to_xcf( "--batch", f"(load {_scheme_string(str(script_path))})", ] - try: - result = _run_gimp_batch(cmd, timeout=timeout, env=env) - except RuntimeError: - raise + result = _run_gimp_batch(cmd, timeout=timeout, env=env, outfile=outfile) if not outfile.is_file(): stderr = (result.stderr or "").strip() stdout = (result.stdout or "").strip() diff --git a/tests/test_xcf_stack.py b/tests/test_xcf_stack.py index d9b3ac5..29a3dd7 100644 --- a/tests/test_xcf_stack.py +++ b/tests/test_xcf_stack.py @@ -1,6 +1,7 @@ from __future__ import annotations import shutil +import subprocess from pathlib import Path from unittest.mock import patch @@ -40,6 +41,38 @@ class TestModuleRegistration: def test_get_module_returns_xcf_stack_class(self) -> None: assert get_module("xcf_stack") is XcfStackModule + def test_xcf_stack_runs_last(self) -> None: + from imagepipeline.core.runner import PipelineRunner + from imagepipeline.core.step import StepDefinition + from imagepipeline.modules.imagemagick_grayscale import ImageMagickGrayscale + + steps = [ + StepDefinition( + step_id="xcf_stack_01", + module_name="xcf_stack", + module=XcfStackModule, + input_refs=["input"], + params={}, + output_dir_name="xcf_stack_01", + ), + StepDefinition( + step_id="imagemagick_grayscale_01", + module_name="imagemagick_grayscale", + module=ImageMagickGrayscale, + input_refs=["input"], + params={}, + output_dir_name="imagemagick_grayscale_01", + ), + ] + runner = PipelineRunner( + name="order", + input_dir=Path("/tmp/unused"), + output_base=Path("/tmp/unused"), + steps=steps, + ) + ordered = runner._topological_sort() + assert ordered[-1].module_name == "xcf_stack" + class TestExpectedOutputFilenames: def test_returns_xcf_for_jpg_input(self) -> None: @@ -50,6 +83,14 @@ class TestExpectedOutputFilenames: ) assert names == ["photo.xcf"] + def test_default_timeout(self) -> None: + params = XcfStackModule.validate_module_params({}) + assert params["timeout"] == 120.0 + + def test_custom_timeout(self) -> None: + params = XcfStackModule.validate_module_params({"timeout": 45}) + assert params["timeout"] == 45.0 + def _make_stack_fixture(tmp_path: Path) -> dict[str, Path]: root = tmp_path @@ -185,3 +226,30 @@ class TestStackImagesToXcfIntegration: names = outfile.read_bytes() assert b"bottom" in names assert b"top" in names + + +class TestGimpTimeoutHandling: + def test_accepts_outfile_when_gimp_hangs_on_exit(self, tmp_path: Path) -> None: + from unittest.mock import MagicMock, patch + + from imagepipeline.utils.gimp import stack_images_to_xcf + + layer = tmp_path / "layer.png" + layer.write_bytes(b"png") + outfile = tmp_path / "stack.xcf" + outfile.write_bytes(b"xcf-data") + + def fake_popen(*_args, **_kwargs): + process = MagicMock() + process.communicate.side_effect = [ + subprocess.TimeoutExpired(cmd="gimp", timeout=1), + ("", ""), + ] + process.kill = MagicMock() + return process + + with patch("imagepipeline.utils.gimp.subprocess.Popen", side_effect=fake_popen): + with patch("imagepipeline.utils.gimp.require_gimp", return_value="gimp-console"): + stack_images_to_xcf([("layer", layer)], outfile, timeout=1.0) + + assert outfile.read_bytes() == b"xcf-data"