From 96a0409dbb2b690438f505bc9a126a9c8e86eb24 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Sat, 6 Dec 2025 15:47:31 +0100 Subject: [PATCH] Enhance Nix flake installer to remove old profile entries before installation and update unit tests accordingly. Includes: - Added best-effort removal of existing 'package-manager' Nix profile entry. - Updated install logic to avoid file conflicts. - Extended unit tests to verify removal behavior and SystemExit-safe handling. Conversation reference: https://chatgpt.com/share/69332bc4-a128-800f-a69c-fdc24c4cc7fe --- pkgmgr/installers/nix_flake.py | 41 ++++++++++++- .../unit/pkgmgr/installers/test_nix_flake.py | 57 ++++++++++++++++--- 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/pkgmgr/installers/nix_flake.py b/pkgmgr/installers/nix_flake.py index 3b265b1..2717e21 100644 --- a/pkgmgr/installers/nix_flake.py +++ b/pkgmgr/installers/nix_flake.py @@ -6,6 +6,12 @@ Installer for Nix flakes. If a repository contains flake.nix and the 'nix' command is available, this installer will try to install profile outputs from the flake. + +Behavior: + - If flake.nix is present and `nix` exists on PATH: + * First remove any existing `package-manager` profile entry (best-effort). + * Then install the flake outputs (pkgmgr, default) via `nix profile install`. + - Any failure in `nix profile install` is treated as fatal (SystemExit). """ import os @@ -20,6 +26,7 @@ class NixFlakeInstaller(BaseInstaller): """Install Nix flake profiles for repositories that define flake.nix.""" FLAKE_FILE = "flake.nix" + PROFILE_NAME = "package-manager" def supports(self, ctx: RepoContext) -> bool: """ @@ -32,11 +39,39 @@ class NixFlakeInstaller(BaseInstaller): flake_path = os.path.join(ctx.repo_dir, self.FLAKE_FILE) return os.path.exists(flake_path) + def _ensure_old_profile_removed(self, ctx: RepoContext) -> None: + """ + Best-effort removal of an existing profile entry. + + This handles the "already provides the following file" conflict by + removing previous `package-manager` installations before we install + the new one. + + Any error in `nix profile remove` is intentionally ignored, because + a missing profile entry is not a fatal condition. + """ + if shutil.which("nix") is None: + return + + # We do NOT use run_command here, because we explicitly want to ignore + # the failure of `nix profile remove` (e.g. entry not present). + # Using `|| true` makes this idempotent. + cmd = f"nix profile remove {self.PROFILE_NAME} || true" + # This will still respect preview mode inside run_command. + try: + run_command(cmd, cwd=ctx.repo_dir, preview=ctx.preview) + except SystemExit: + # Ignore any error here: if the profile entry does not exist, + # that's fine and not a fatal condition. + pass + def run(self, ctx: RepoContext) -> None: """ Install Nix flake profile outputs (pkgmgr, default). Any failure in `nix profile install` is treated as fatal (SystemExit). + The "already installed / file conflict" situation is avoided by + removing the existing profile entry beforehand. """ flake_path = os.path.join(ctx.repo_dir, self.FLAKE_FILE) if not os.path.exists(flake_path): @@ -46,7 +81,11 @@ class NixFlakeInstaller(BaseInstaller): print("Warning: flake.nix found but 'nix' command not available. Skipping flake setup.") return - print("Nix flake detected, attempting to install profile output...") + print("Nix flake detected, attempting to install profile outputs...") + + # Handle the "already installed" case up-front: + self._ensure_old_profile_removed(ctx) + for output in ("pkgmgr", "default"): cmd = f"nix profile install {ctx.repo_dir}#{output}" try: diff --git a/tests/unit/pkgmgr/installers/test_nix_flake.py b/tests/unit/pkgmgr/installers/test_nix_flake.py index ec47e23..035c1f3 100644 --- a/tests/unit/pkgmgr/installers/test_nix_flake.py +++ b/tests/unit/pkgmgr/installers/test_nix_flake.py @@ -40,7 +40,17 @@ class TestNixFlakeInstaller(unittest.TestCase): @patch("os.path.exists", return_value=True) @patch("shutil.which", return_value="/usr/bin/nix") @mock.patch("pkgmgr.installers.nix_flake.run_command") - def test_run_tries_pkgmgr_then_default(self, mock_run_command, mock_which, mock_exists): + def test_run_removes_old_profile_and_installs_outputs( + self, + mock_run_command, + mock_which, + mock_exists, + ): + """ + Ensure that run(): + - first tries to remove the old 'package-manager' profile entry + - then installs both 'pkgmgr' and 'default' outputs. + """ cmds = [] def side_effect(cmd, cwd=None, preview=False, *args, **kwargs): @@ -51,14 +61,45 @@ class TestNixFlakeInstaller(unittest.TestCase): self.installer.run(self.ctx) - self.assertIn( - f"nix profile install {self.ctx.repo_dir}#pkgmgr", - cmds, - ) - self.assertIn( - f"nix profile install {self.ctx.repo_dir}#default", - cmds, + remove_cmd = f"nix profile remove {self.installer.PROFILE_NAME} || true" + install_pkgmgr_cmd = f"nix profile install {self.ctx.repo_dir}#pkgmgr" + install_default_cmd = f"nix profile install {self.ctx.repo_dir}#default" + + # Mindestens diese drei Kommandos müssen aufgerufen worden sein + self.assertIn(remove_cmd, cmds) + self.assertIn(install_pkgmgr_cmd, cmds) + self.assertIn(install_default_cmd, cmds) + + # Optional: sicherstellen, dass der remove-Aufruf zuerst kam + self.assertEqual(cmds[0], remove_cmd) + + @patch("shutil.which", return_value="/usr/bin/nix") + @mock.patch("pkgmgr.installers.nix_flake.run_command") + def test_ensure_old_profile_removed_ignores_systemexit( + self, + mock_run_command, + mock_which, + ): + """ + _ensure_old_profile_removed() must not propagate SystemExit, even if + 'nix profile remove' fails (e.g. profile entry does not exist). + """ + + def side_effect(cmd, cwd=None, preview=False, *args, **kwargs): + raise SystemExit(1) + + mock_run_command.side_effect = side_effect + + # Should not raise, SystemExit is swallowed internally. + self.installer._ensure_old_profile_removed(self.ctx) + + remove_cmd = f"nix profile remove {self.installer.PROFILE_NAME} || true" + mock_run_command.assert_called_with( + remove_cmd, + cwd=self.ctx.repo_dir, + preview=self.ctx.preview, ) + if __name__ == "__main__": unittest.main()