diff --git a/cli/localci/core/queue.py b/cli/localci/core/queue.py index ba39b6f..9477dbe 100644 --- a/cli/localci/core/queue.py +++ b/cli/localci/core/queue.py @@ -105,24 +105,35 @@ def add_job(self, job_id: str, needs: list[str]) -> None: self._reverse[dep].append(job_id) def resolve(self) -> list[str]: + """Return a topological order of registered jobs. + + Dependencies listed in ``needs`` but never passed to :meth:`add_job` + are still visited with an empty edge list and included in the order. + Callers (e.g. ``Workflow.dependency_order``) must register a complete + graph. + """ visited: set[str] = set() temp_visited: set[str] = set() order: list[str] = [] - def visit(jid: str) -> None: + def visit(jid: str, path: list[str]) -> None: if jid in temp_visited: - raise CyclicDependencyError(jid) + idx = path.index(jid) + cycle = path[idx:] + [jid] + raise CyclicDependencyError(cycle) if jid in visited: return temp_visited.add(jid) + path.append(jid) for dep in self._graph.get(jid, []): - visit(dep) + visit(dep, path) + path.pop() temp_visited.discard(jid) visited.add(jid) order.append(jid) for jid in self._graph: - visit(jid) + visit(jid, []) return order def get_dependencies(self, job_id: str) -> list[str]: diff --git a/cli/localci/core/workflow.py b/cli/localci/core/workflow.py index df5198d..4291dc3 100644 --- a/cli/localci/core/workflow.py +++ b/cli/localci/core/workflow.py @@ -15,6 +15,7 @@ from pathlib import Path from typing import Any, Optional +from localci.core.queue import DependencyResolver from localci.errors import ( LocalCIError, MissingFieldError, @@ -302,30 +303,15 @@ def platform_summary(self) -> dict[Platform, int]: def dependency_order(self) -> list[str]: """Topological sort of jobs by dependencies.""" - visited: set[str] = set() - in_progress: set[str] = set() - order: list[str] = [] - - def visit(job_id: str) -> None: - if job_id in visited: - return - if job_id not in self.jobs: - logger.warning("Dependency '%s' not found in jobs", job_id) - return - if job_id in in_progress: - logger.warning("Circular dependency detected involving '%s'", job_id) - return - in_progress.add(job_id) - visited.add(job_id) - for dep in self.jobs[job_id].needs: - visit(dep) - in_progress.discard(job_id) - order.append(job_id) - - for job_id in self.jobs: - visit(job_id) - - return order + resolver = DependencyResolver() + job_ids = set(self.jobs) + for job_id, job in self.jobs.items(): + needs = [d for d in job.needs if d in job_ids] + for dep in job.needs: + if dep not in job_ids: + logger.warning("Dependency '%s' not found in jobs", dep) + resolver.add_job(job_id, needs) + return resolver.resolve() # ===================================================================== diff --git a/cli/localci/errors.py b/cli/localci/errors.py index c0b0b05..455899e 100644 --- a/cli/localci/errors.py +++ b/cli/localci/errors.py @@ -188,11 +188,27 @@ def __init__(self, entry: dict[str, Any], detail: str) -> None: class CyclicDependencyError(WorkflowError): - """Circular dependency detected in job graph.""" + """Circular dependency detected in job graph. - def __init__(self, job_id: str) -> None: - super().__init__(f"Cyclic dependency detected involving job: {job_id}") - self.job_id = job_id + Attributes + ---------- + cycle: + Ordered job IDs forming the loop; first and last elements are equal + (e.g. ``["a", "b", "a"]``). + job_id: + First job in the cycle, kept for backward compatibility (``cycle[0]``). + """ + + def __init__(self, cycle: list[str]) -> None: + if not isinstance(cycle, list): + raise TypeError( + f"cycle must be a list[str], got {type(cycle).__name__!r}; " + "pass an ordered path such as ['a', 'b', 'a'], not a single job_id string" + ) + self.cycle = list(cycle) + self.job_id = self.cycle[0] if self.cycle else "" + path = " -> ".join(self.cycle) + super().__init__(f"Cyclic dependency detected: {path}") # --------------------------------------------------------------------------- diff --git a/cli/tests/test_errors.py b/cli/tests/test_errors.py index ab51954..dabe955 100644 --- a/cli/tests/test_errors.py +++ b/cli/tests/test_errors.py @@ -168,6 +168,18 @@ def test_workflow_parse_error_attributes(self, tmp_path): assert "unexpected key" in str(exc) assert str(p) in str(exc) + def test_cyclic_dependency_error_attributes(self): + cycle = ["a", "b", "a"] + exc = CyclicDependencyError(cycle) + assert exc.cycle == cycle + assert exc.job_id == "a" + assert "a -> b -> a" in str(exc) + assert isinstance(exc, WorkflowError) + + def test_cyclic_dependency_error_rejects_string(self): + with pytest.raises(TypeError, match="cycle must be a list"): + CyclicDependencyError("ab") + # --------------------------------------------------------------------------- # Backward-compatibility: caught by built-in types diff --git a/cli/tests/test_queue.py b/cli/tests/test_queue.py index 34bafbc..b598247 100644 --- a/cli/tests/test_queue.py +++ b/cli/tests/test_queue.py @@ -118,8 +118,13 @@ def test_cyclic_dependency(self): resolver = DependencyResolver() resolver.add_job("a", ["b"]) resolver.add_job("b", ["a"]) - with pytest.raises(CyclicDependencyError): + with pytest.raises(CyclicDependencyError) as exc_info: resolver.resolve() + exc = exc_info.value + assert exc.cycle == ["a", "b", "a"] + assert exc.cycle[0] == exc.cycle[-1] + assert exc.job_id == "a" + assert "a -> b -> a" in str(exc) def test_all_dependencies_met(self): resolver = DependencyResolver() diff --git a/cli/tests/test_workflow.py b/cli/tests/test_workflow.py index 4ed9de2..f9b90c7 100644 --- a/cli/tests/test_workflow.py +++ b/cli/tests/test_workflow.py @@ -11,6 +11,7 @@ import pytest +from localci.errors import CyclicDependencyError from localci.core.workflow import ( BuildSystem, BuildVariant, @@ -350,6 +351,34 @@ def test_changelog_needs_build(self, sample_workflow): changelog = sample_workflow.jobs["changelog"] assert "build" in changelog.needs + def test_cyclic_dependency_raises(self, tmp_path): + workflow = Workflow( + name="cyclic", + file_path=tmp_path / "cyclic.yml", + events=["push"], + jobs={ + "a": Job( + id="a", + name="A", + runs_on="ubuntu-latest", + needs=["b"], + ), + "b": Job( + id="b", + name="B", + runs_on="ubuntu-latest", + needs=["a"], + ), + }, + ) + with pytest.raises(CyclicDependencyError) as exc_info: + workflow.dependency_order() + exc = exc_info.value + assert exc.cycle == ["a", "b", "a"] + assert exc.cycle[0] == exc.cycle[-1] + assert exc.job_id == "a" + assert "a -> b -> a" in str(exc) + # ===================================================================== # Container info