diff --git a/pkgmgr/installers/makefile.py b/pkgmgr/installers/makefile.py index 89e3e5f..80b78ff 100644 --- a/pkgmgr/installers/makefile.py +++ b/pkgmgr/installers/makefile.py @@ -2,13 +2,15 @@ # -*- coding: utf-8 -*- """ -Installer that triggers `make install` if a Makefile is present. +Installer that triggers `make install` if a Makefile is present and +the Makefile actually defines an 'install' target. This is useful for repositories that expose a standard Makefile-based installation step. """ import os +import re from pkgmgr.context import RepoContext from pkgmgr.installers.base import BaseInstaller @@ -16,7 +18,7 @@ from pkgmgr.run_command import run_command class MakefileInstaller(BaseInstaller): - """Run `make install` if a Makefile exists in the repository.""" + """Run `make install` if a Makefile with an 'install' target exists.""" # Logical layer name, used by capability matchers. layer = "makefile" @@ -24,15 +26,68 @@ class MakefileInstaller(BaseInstaller): MAKEFILE_NAME = "Makefile" def supports(self, ctx: RepoContext) -> bool: + """Return True if a Makefile exists in the repository directory.""" makefile_path = os.path.join(ctx.repo_dir, self.MAKEFILE_NAME) return os.path.exists(makefile_path) + def _has_install_target(self, makefile_path: str) -> bool: + """ + Check whether the Makefile defines an 'install' target. + + We treat the presence of a real install target as either: + - a line starting with 'install:' (optionally preceded by whitespace), or + - a .PHONY line that lists 'install' as one of the targets. + """ + try: + with open(makefile_path, "r", encoding="utf-8", errors="ignore") as f: + content = f.read() + except OSError: + # If we cannot read the Makefile for some reason, assume no target. + return False + + # install: ... + if re.search(r"^\s*install\s*:", content, flags=re.MULTILINE): + return True + + # .PHONY: ... install ... + if re.search(r"^\s*\.PHONY\s*:\s*.*\binstall\b", content, flags=re.MULTILINE): + return True + + return False + def run(self, ctx: RepoContext) -> None: """ - Execute `make install` in the repository directory. + Execute `make install` in the repository directory, but only if an + 'install' target is actually defined in the Makefile. Any failure in `make install` is treated as a fatal error and will propagate as SystemExit from run_command(). """ + makefile_path = os.path.join(ctx.repo_dir, self.MAKEFILE_NAME) + + if not os.path.exists(makefile_path): + # Should normally not happen if supports() was checked before, + # but keep this guard for robustness. + if not ctx.quiet: + print( + f"[pkgmgr] Makefile '{makefile_path}' not found, " + "skipping make install." + ) + return + + if not self._has_install_target(makefile_path): + if not ctx.quiet: + print( + "[pkgmgr] Skipping Makefile install: no 'install' target " + f"found in {makefile_path}." + ) + return + + if not ctx.quiet: + print( + f"[pkgmgr] Running 'make install' in {ctx.repo_dir} " + "(install target detected in Makefile)." + ) + cmd = "make install" run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) diff --git a/tests/unit/pkgmgr/installers/test_makefile_installer.py b/tests/unit/pkgmgr/installers/test_makefile_installer.py index fbf47fa..b30df1d 100644 --- a/tests/unit/pkgmgr/installers/test_makefile_installer.py +++ b/tests/unit/pkgmgr/installers/test_makefile_installer.py @@ -2,7 +2,7 @@ import os import unittest -from unittest.mock import patch +from unittest.mock import patch, mock_open from pkgmgr.context import RepoContext from pkgmgr.installers.makefile import MakefileInstaller @@ -36,9 +36,25 @@ class TestMakefileInstaller(unittest.TestCase): self.assertFalse(self.installer.supports(self.ctx)) @patch("pkgmgr.installers.makefile.run_command") + @patch( + "builtins.open", + new_callable=mock_open, + read_data="install:\n\t@echo 'installing'\n", + ) @patch("os.path.exists", return_value=True) - def test_run_executes_make_install(self, mock_exists, mock_run_command): + def test_run_executes_make_install_when_target_present( + self, mock_exists, mock_file, mock_run_command + ): self.installer.run(self.ctx) + + # Ensure we checked the Makefile and then called make install. + mock_file.assert_called_once_with( + os.path.join(self.ctx.repo_dir, "Makefile"), + "r", + encoding="utf-8", + errors="ignore", + ) + cmd = mock_run_command.call_args[0][0] self.assertEqual(cmd, "make install") self.assertEqual( @@ -46,6 +62,27 @@ class TestMakefileInstaller(unittest.TestCase): self.ctx.repo_dir, ) + @patch("pkgmgr.installers.makefile.run_command") + @patch( + "builtins.open", + new_callable=mock_open, + read_data="all:\n\t@echo 'nothing here'\n", + ) + @patch("os.path.exists", return_value=True) + def test_run_skips_when_no_install_target( + self, mock_exists, mock_file, mock_run_command + ): + self.installer.run(self.ctx) + + # We should have read the Makefile, but not called run_command(). + mock_file.assert_called_once_with( + os.path.join(self.ctx.repo_dir, "Makefile"), + "r", + encoding="utf-8", + errors="ignore", + ) + mock_run_command.assert_not_called() + if __name__ == "__main__": unittest.main()