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
This commit is contained in:
Kevin Veen-Birkenbach
2025-12-06 15:47:31 +01:00
parent aae852995e
commit 96a0409dbb
2 changed files with 89 additions and 9 deletions

View File

@@ -6,6 +6,12 @@ Installer for Nix flakes.
If a repository contains flake.nix and the 'nix' command is available, this If a repository contains flake.nix and the 'nix' command is available, this
installer will try to install profile outputs from the flake. 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 import os
@@ -20,6 +26,7 @@ class NixFlakeInstaller(BaseInstaller):
"""Install Nix flake profiles for repositories that define flake.nix.""" """Install Nix flake profiles for repositories that define flake.nix."""
FLAKE_FILE = "flake.nix" FLAKE_FILE = "flake.nix"
PROFILE_NAME = "package-manager"
def supports(self, ctx: RepoContext) -> bool: def supports(self, ctx: RepoContext) -> bool:
""" """
@@ -32,11 +39,39 @@ class NixFlakeInstaller(BaseInstaller):
flake_path = os.path.join(ctx.repo_dir, self.FLAKE_FILE) flake_path = os.path.join(ctx.repo_dir, self.FLAKE_FILE)
return os.path.exists(flake_path) 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: def run(self, ctx: RepoContext) -> None:
""" """
Install Nix flake profile outputs (pkgmgr, default). Install Nix flake profile outputs (pkgmgr, default).
Any failure in `nix profile install` is treated as fatal (SystemExit). 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) flake_path = os.path.join(ctx.repo_dir, self.FLAKE_FILE)
if not os.path.exists(flake_path): 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.") print("Warning: flake.nix found but 'nix' command not available. Skipping flake setup.")
return 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"): for output in ("pkgmgr", "default"):
cmd = f"nix profile install {ctx.repo_dir}#{output}" cmd = f"nix profile install {ctx.repo_dir}#{output}"
try: try:

View File

@@ -40,7 +40,17 @@ class TestNixFlakeInstaller(unittest.TestCase):
@patch("os.path.exists", return_value=True) @patch("os.path.exists", return_value=True)
@patch("shutil.which", return_value="/usr/bin/nix") @patch("shutil.which", return_value="/usr/bin/nix")
@mock.patch("pkgmgr.installers.nix_flake.run_command") @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 = [] cmds = []
def side_effect(cmd, cwd=None, preview=False, *args, **kwargs): def side_effect(cmd, cwd=None, preview=False, *args, **kwargs):
@@ -51,14 +61,45 @@ class TestNixFlakeInstaller(unittest.TestCase):
self.installer.run(self.ctx) self.installer.run(self.ctx)
self.assertIn( remove_cmd = f"nix profile remove {self.installer.PROFILE_NAME} || true"
f"nix profile install {self.ctx.repo_dir}#pkgmgr", install_pkgmgr_cmd = f"nix profile install {self.ctx.repo_dir}#pkgmgr"
cmds, install_default_cmd = f"nix profile install {self.ctx.repo_dir}#default"
)
self.assertIn( # Mindestens diese drei Kommandos müssen aufgerufen worden sein
f"nix profile install {self.ctx.repo_dir}#default", self.assertIn(remove_cmd, cmds)
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__": if __name__ == "__main__":
unittest.main() unittest.main()