fix(xcf_stack): run last, configurable timeout, tolerate GIMP exit hang
Defer xcf_stack to the final pipeline step via runs_last, expose per-image timeout (default 120s), accept XCF output when gimp-console hangs on quit, and discover .xcf outputs for resume. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -280,4 +280,7 @@ class PipelineRunner:
|
|||||||
if len(ordered_ids) != len(self.steps):
|
if len(ordered_ids) != len(self.steps):
|
||||||
raise CycleError("Pipeline contains a cycle in step dependencies")
|
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
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ class BaseModule(ABC):
|
|||||||
|
|
||||||
name: ClassVar[str]
|
name: ClassVar[str]
|
||||||
description: ClassVar[str] = ""
|
description: ClassVar[str] = ""
|
||||||
|
runs_last: ClassVar[bool] = False
|
||||||
supported_input_formats: ClassVar[tuple[str, ...]] = (
|
supported_input_formats: ClassVar[tuple[str, ...]] = (
|
||||||
".jpg",
|
".jpg",
|
||||||
".jpeg",
|
".jpeg",
|
||||||
|
|||||||
@@ -13,11 +13,11 @@ from imagepipeline.utils.gimp import stack_images_to_xcf
|
|||||||
@register
|
@register
|
||||||
class XcfStackModule(SubprocessModule):
|
class XcfStackModule(SubprocessModule):
|
||||||
name = "xcf_stack"
|
name = "xcf_stack"
|
||||||
|
runs_last = True
|
||||||
description = (
|
description = (
|
||||||
"Stack all prior pipeline step outputs as GIMP layers into one XCF per image"
|
"Stack all prior pipeline step outputs as GIMP layers into one XCF per image"
|
||||||
)
|
)
|
||||||
command_candidates = ("gimp-console", "gimp")
|
command_candidates = ("gimp-console", "gimp")
|
||||||
default_timeout = 600.0
|
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def expected_output_filenames(
|
def expected_output_filenames(
|
||||||
@@ -42,11 +42,17 @@ class XcfStackModule(SubprocessModule):
|
|||||||
default=False,
|
default=False,
|
||||||
help="Skip missing step outputs instead of failing",
|
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:
|
def run(self, ctx: ModuleContext) -> None:
|
||||||
include_input = ctx.params["include_input"]
|
include_input = ctx.params["include_input"]
|
||||||
skip_missing = ctx.params["skip_missing"]
|
skip_missing = ctx.params["skip_missing"]
|
||||||
|
timeout = ctx.params["timeout"]
|
||||||
|
|
||||||
layer_sources: list[tuple[str, Path]] = []
|
layer_sources: list[tuple[str, Path]] = []
|
||||||
if include_input:
|
if include_input:
|
||||||
@@ -86,4 +92,9 @@ class XcfStackModule(SubprocessModule):
|
|||||||
)
|
)
|
||||||
|
|
||||||
dst = ctx.output_dir / f"{stem}.xcf"
|
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"
|
||||||
|
)
|
||||||
|
|||||||
+27
-12
@@ -69,21 +69,39 @@ def _run_gimp_batch(
|
|||||||
*,
|
*,
|
||||||
timeout: float | None,
|
timeout: float | None,
|
||||||
env: dict[str, str],
|
env: dict[str, str],
|
||||||
|
outfile: Path | None = None,
|
||||||
) -> subprocess.CompletedProcess[str]:
|
) -> 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:
|
try:
|
||||||
return subprocess.run(
|
stdout, stderr = process.communicate(timeout=timeout)
|
||||||
cmd,
|
|
||||||
check=False,
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
timeout=timeout,
|
|
||||||
env=env,
|
|
||||||
)
|
|
||||||
except subprocess.TimeoutExpired as exc:
|
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(
|
raise RuntimeError(
|
||||||
f"Command timed out after {timeout}s: {' '.join(cmd)}"
|
f"Command timed out after {timeout}s: {' '.join(cmd)}"
|
||||||
) from exc
|
) 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(
|
def stack_images_to_xcf(
|
||||||
layers: list[tuple[str, Path]],
|
layers: list[tuple[str, Path]],
|
||||||
@@ -134,10 +152,7 @@ def stack_images_to_xcf(
|
|||||||
"--batch",
|
"--batch",
|
||||||
f"(load {_scheme_string(str(script_path))})",
|
f"(load {_scheme_string(str(script_path))})",
|
||||||
]
|
]
|
||||||
try:
|
result = _run_gimp_batch(cmd, timeout=timeout, env=env, outfile=outfile)
|
||||||
result = _run_gimp_batch(cmd, timeout=timeout, env=env)
|
|
||||||
except RuntimeError:
|
|
||||||
raise
|
|
||||||
if not outfile.is_file():
|
if not outfile.is_file():
|
||||||
stderr = (result.stderr or "").strip()
|
stderr = (result.stderr or "").strip()
|
||||||
stdout = (result.stdout or "").strip()
|
stdout = (result.stdout or "").strip()
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import shutil
|
import shutil
|
||||||
|
import subprocess
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
@@ -40,6 +41,38 @@ class TestModuleRegistration:
|
|||||||
def test_get_module_returns_xcf_stack_class(self) -> None:
|
def test_get_module_returns_xcf_stack_class(self) -> None:
|
||||||
assert get_module("xcf_stack") is XcfStackModule
|
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:
|
class TestExpectedOutputFilenames:
|
||||||
def test_returns_xcf_for_jpg_input(self) -> None:
|
def test_returns_xcf_for_jpg_input(self) -> None:
|
||||||
@@ -50,6 +83,14 @@ class TestExpectedOutputFilenames:
|
|||||||
)
|
)
|
||||||
assert names == ["photo.xcf"]
|
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]:
|
def _make_stack_fixture(tmp_path: Path) -> dict[str, Path]:
|
||||||
root = tmp_path
|
root = tmp_path
|
||||||
@@ -185,3 +226,30 @@ class TestStackImagesToXcfIntegration:
|
|||||||
names = outfile.read_bytes()
|
names = outfile.read_bytes()
|
||||||
assert b"bottom" in names
|
assert b"bottom" in names
|
||||||
assert b"top" 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"
|
||||||
|
|||||||
Reference in New Issue
Block a user