diff --git a/src/pkgmgr/actions/install/__init__.py b/src/pkgmgr/actions/install/__init__.py index b7b8373..5fae16e 100644 --- a/src/pkgmgr/actions/install/__init__.py +++ b/src/pkgmgr/actions/install/__init__.py @@ -38,13 +38,14 @@ from pkgmgr.core.repository.verify import verify_repository Repository = dict[str, Any] INSTALLERS = [ + MakefileInstaller(), ArchPkgbuildInstaller(), DebianControlInstaller(), RpmSpecInstaller(), NixFlakeInstaller(), PythonInstaller(), - MakefileInstaller(), ] +"""Preference order: the pipeline runs the first supported hook and stops.""" def _ensure_repo_dir( diff --git a/src/pkgmgr/actions/install/pipeline.py b/src/pkgmgr/actions/install/pipeline.py index 0d5e3e9..3028f22 100644 --- a/src/pkgmgr/actions/install/pipeline.py +++ b/src/pkgmgr/actions/install/pipeline.py @@ -81,13 +81,15 @@ class InstallationPipeline: else: repo.pop("command", None) - provided_capabilities: set[str] = set() + attempted: list[str] = [] for installer in self._installers: layer_name = getattr(installer, "layer", None) if layer_name is None: - self._run_installer(installer, ctx, identifier, repo_dir, quiet) + if self._run_installer(installer, ctx, identifier, repo_dir, quiet): + return + attempted.append(installer.__class__.__name__) continue try: @@ -120,15 +122,6 @@ class InstallationPipeline: if not installer.supports(ctx): continue - caps = installer.discover_capabilities(ctx) - if caps and caps.issubset(provided_capabilities): - if not quiet: - print( - f"Skipping installer {installer.__class__.__name__} " - f"for {identifier} – capabilities {caps} already provided." - ) - continue - if not quiet: if ( ctx.force_update @@ -142,13 +135,17 @@ class InstallationPipeline: else: print( f"[pkgmgr] Running installer {installer.__class__.__name__} " - f"for {identifier} in '{repo_dir}' " - f"(new capabilities: {caps or set()})..." + f"for {identifier} in '{repo_dir}'..." ) - self._run_installer(installer, ctx, identifier, repo_dir, quiet) - - provided_capabilities.update(caps) + if not self._run_installer(installer, ctx, identifier, repo_dir, quiet): + attempted.append(installer.__class__.__name__) + if not quiet: + print( + f"[pkgmgr] {installer.__class__.__name__} installed nothing " + f"for {identifier}; falling back to the next hook." + ) + continue new_state = resolver.resolve() if new_state.command: @@ -165,6 +162,13 @@ class InstallationPipeline: repo.pop("command", None) state = new_state + return + + if attempted: + raise SystemExit( + f"every installation hook failed for {identifier}: " + f"{', '.join(attempted)}" + ) @staticmethod def _run_installer( @@ -173,9 +177,11 @@ class InstallationPipeline: identifier: str, repo_dir: str, quiet: bool, - ) -> None: + ) -> bool: + """Run one hook. Returns whether it installed anything.""" try: installer.run(ctx) + return True except SystemExit as exc: exit_code = exc.code if isinstance(exc.code, int) else str(exc.code) print( @@ -193,4 +199,4 @@ class InstallationPipeline: f" pkgmgr install {identifier} " "--clone-mode shallow --no-verification" ) - raise + return False diff --git a/tests/integration/test_recursive_capabilities.py b/tests/integration/test_recursive_capabilities.py index 45f8bbb..59d64b9 100644 --- a/tests/integration/test_recursive_capabilities.py +++ b/tests/integration/test_recursive_capabilities.py @@ -1,9 +1,5 @@ """ -Integration tests for recursive capability resolution and installer shadowing. - -These tests verify that, given different repository layouts (Makefile, pyproject, -flake.nix, PKGBUILD), only the expected installers are executed based on the -capabilities provided by higher layers. +Integration tests for installer selection across repository layouts. Layer order (strongest → weakest): @@ -129,10 +125,8 @@ class TestRecursiveCapabilitiesIntegration(unittest.TestCase): "With only a Makefile, the MakefileInstaller should run exactly once.", ) - def test_python_and_makefile_both_run_when_caps_disjoint(self) -> None: - """ - If Python and Makefile have disjoint capabilities, both installers run. - """ + def test_only_the_first_supported_hook_runs(self) -> None: + """A repository with two usable hooks is installed by the first one.""" repo_dir = self._new_repo() # pyproject.toml without any explicit "make install" hint @@ -153,9 +147,8 @@ class TestRecursiveCapabilitiesIntegration(unittest.TestCase): self.assertEqual( called, - ["python", "makefile"], - "PythonInstaller and MakefileInstaller should both run when their " - "capabilities are disjoint.", + ["python"], + "Only the first supported hook may install the repository.", ) def test_python_shadows_makefile_when_pyproject_mentions_make_install(self) -> None: diff --git a/tests/unit/pkgmgr/actions/install/test_pipeline.py b/tests/unit/pkgmgr/actions/install/test_pipeline.py index 717dc94..b9c8b58 100644 --- a/tests/unit/pkgmgr/actions/install/test_pipeline.py +++ b/tests/unit/pkgmgr/actions/install/test_pipeline.py @@ -21,11 +21,13 @@ class DummyInstaller(BaseInstaller): layer: str | None = None, supports_result: bool = True, capabilities: set[str] | None = None, + fails: bool = False, ) -> None: self._name = name self.layer = layer # type: ignore[assignment] self._supports_result = supports_result self._capabilities = capabilities or set() + self._fails = fails self.ran = False def supports(self, ctx: RepoContext) -> bool: # type: ignore[override] @@ -33,6 +35,8 @@ class DummyInstaller(BaseInstaller): def run(self, ctx: RepoContext) -> None: # type: ignore[override] self.ran = True + if self._fails: + raise SystemExit(2) def discover_capabilities(self, ctx: RepoContext) -> set[str]: # type: ignore[override] return set(self._capabilities) @@ -117,15 +121,12 @@ class TestInstallationPipeline(unittest.TestCase): @patch("pkgmgr.actions.install.pipeline.create_ink") @patch("pkgmgr.actions.install.pipeline.resolve_command_for_repo") - def test_capabilities_prevent_duplicate_installers( + def test_only_one_installation_hook_runs( self, mock_resolve_command_for_repo: MagicMock, mock_create_ink: MagicMock, ) -> None: - """ - If one installer has already provided a set of capabilities, - a second installer advertising the same capabilities should be skipped. - """ + """A repository is installed once, by the first hook that supports it.""" mock_resolve_command_for_repo.return_value = None # no CLI initially ctx = _minimal_context() @@ -148,9 +149,103 @@ class TestInstallationPipeline(unittest.TestCase): self.assertTrue(first.ran, "First installer should run.") self.assertFalse( second.ran, - "Second installer with identical capabilities must be skipped.", + "A second installation hook must not run after the first one did.", ) + @patch("pkgmgr.actions.install.pipeline.create_ink") + @patch("pkgmgr.actions.install.pipeline.resolve_command_for_repo") + def test_a_differing_second_hook_is_not_run_either( + self, + mock_resolve_command_for_repo: MagicMock, + mock_create_ink: MagicMock, + ) -> None: + """The rule is one hook, not one hook per capability.""" + mock_resolve_command_for_repo.return_value = None + + ctx = _minimal_context() + first = DummyInstaller( + "python-installer", + layer=CliLayer.PYTHON.value, + supports_result=True, + capabilities={"python-runtime"}, + ) + second = DummyInstaller( + "makefile-installer", + layer=CliLayer.MAKEFILE.value, + supports_result=True, + capabilities={"make-install"}, + ) + + pipeline = InstallationPipeline([first, second]) + pipeline.run(ctx) + + self.assertTrue(first.ran, "The first supported hook should run.") + self.assertFalse( + second.ran, + "A hook advertising a different capability must not run either: " + "the project is already installed.", + ) + + @patch("pkgmgr.actions.install.pipeline.create_ink") + @patch("pkgmgr.actions.install.pipeline.resolve_command_for_repo") + def test_a_failed_hook_falls_back_to_the_next( + self, + mock_resolve_command_for_repo: MagicMock, + mock_create_ink: MagicMock, + ) -> None: + """A hook that fails installed nothing, so the next one gets its turn.""" + mock_resolve_command_for_repo.return_value = None + + ctx = _minimal_context() + broken = DummyInstaller( + "makefile-installer", + layer=CliLayer.MAKEFILE.value, + supports_result=True, + fails=True, + ) + working = DummyInstaller( + "python-installer", + layer=CliLayer.PYTHON.value, + supports_result=True, + ) + + pipeline = InstallationPipeline([broken, working]) + pipeline.run(ctx) + + self.assertTrue(broken.ran, "The preferred hook should be tried first.") + self.assertTrue( + working.ran, + "The next hook must run when the preferred one installed nothing.", + ) + + @patch("pkgmgr.actions.install.pipeline.create_ink") + @patch("pkgmgr.actions.install.pipeline.resolve_command_for_repo") + def test_a_repository_whose_hooks_all_fail_is_reported( + self, + mock_resolve_command_for_repo: MagicMock, + mock_create_ink: MagicMock, + ) -> None: + """Falling back must not turn a broken repository into a silent pass.""" + mock_resolve_command_for_repo.return_value = None + + ctx = _minimal_context() + first = DummyInstaller( + "makefile-installer", + layer=CliLayer.MAKEFILE.value, + supports_result=True, + fails=True, + ) + second = DummyInstaller( + "python-installer", + layer=CliLayer.PYTHON.value, + supports_result=True, + fails=True, + ) + + pipeline = InstallationPipeline([first, second]) + with self.assertRaises(SystemExit): + pipeline.run(ctx) + if __name__ == "__main__": unittest.main()