From 3c6acbe080d9f78681a70092a218aaba7b86508e Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 12:31:15 +0000 Subject: [PATCH] nixos-rebuild-ng: add --add-root in nix.remote_build Fix: #365225 --- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 38 ++++++++++++-- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 51 ++++++++++++++++--- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py index c1f2e5d5c711b..c26a771c26f83 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py @@ -7,7 +7,9 @@ from string import Template from subprocess import PIPE, CalledProcessError from typing import Final +from uuid import uuid4 +from . import tmpdir from .constants import WITH_NIX_2_18 from .models import ( Action, @@ -76,24 +78,50 @@ def remote_build( instantiate_flags: dict[str, Args] | None = None, copy_flags: dict[str, Args] | None = None, ) -> Path: + # We need to use `--add-root` otherwise Nix will print this warning: + # > warning: you did not specify '--add-root'; the result might be removed + # > by the garbage collector r = run_wrapper( [ "nix-instantiate", build_attr.path, "--attr", build_attr.to_attr(attr), + "--add-root", + tmpdir.TMPDIR_PATH / uuid4().hex, *dict_to_flags(instantiate_flags or {}), ], stdout=PIPE, ) - drv = Path(r.stdout.strip()) + drv = Path(r.stdout.strip()).resolve() copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) + + # Need a temporary directory in remote to use in `nix-store --add-root` r = run_wrapper( - ["nix-store", "--realise", drv, *dict_to_flags(build_flags or {})], - remote=build_host, - stdout=PIPE, + ["mktemp", "-d", "-t", "nixos-rebuild.XXXXX"], remote=build_host, stdout=PIPE ) - return Path(r.stdout.strip()) + remote_tmpdir = Path(r.stdout.strip()) + try: + r = run_wrapper( + [ + "nix-store", + "--realise", + drv, + "--add-root", + remote_tmpdir / uuid4().hex, + *dict_to_flags(build_flags or {}), + ], + remote=build_host, + stdout=PIPE, + ) + # When you use `--add-root`, `nix-store` returns the root and not the + # path inside Nix store + r = run_wrapper( + ["readlink", "-f", r.stdout.strip()], remote=build_host, stdout=PIPE + ) + return Path(r.stdout.strip()) + finally: + run_wrapper(["rm", "-rf", remote_tmpdir], remote=build_host, check=False) def remote_build_flake( diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py index 34f47029c1035..6245aa5cd1961 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py @@ -5,6 +5,7 @@ from unittest.mock import ANY, call, patch import pytest +import uuid import nixos_rebuild.models as m import nixos_rebuild.nix as n @@ -73,14 +74,27 @@ def test_build_flake(mock_run: Any) -> None: ) -@patch( - get_qualified_name(n.run_wrapper, n), - autospec=True, - return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), -) -def test_remote_build(mock_run: Any, monkeypatch: Any) -> None: +@patch(get_qualified_name(n.run_wrapper, n), autospec=True) +@patch(get_qualified_name(n.uuid4, n), autospec=True) +def test_remote_build(mock_uuid4: Any, mock_run: Any, monkeypatch: Any) -> None: build_host = m.Remote("user@host", [], None) monkeypatch.setenv("NIX_SSHOPTS", "--ssh opts") + + def run_wrapper_side_effect(args, **kwargs): # type: ignore + if args[0] == "nix-instantiate": + return CompletedProcess([], 0, stdout=" \n/path/to/file\n ") + elif args[0] == "mktemp": + return CompletedProcess([], 0, stdout=" \n/tmp/tmpdir\n ") + elif args[0] == "nix-store": + return CompletedProcess([], 0, stdout=" \n/tmp/tmpdir/00000000000000000000000000000000\n ") + elif args[0] == "readlink": + return CompletedProcess([], 0, stdout=" \n/path/to/config\n ") + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_wrapper_side_effect + mock_uuid4.return_value = uuid.UUID(int=0) + assert n.remote_build( "config.system.build.toplevel", m.BuildAttr("", "preAttr"), @@ -88,7 +102,8 @@ def test_remote_build(mock_run: Any, monkeypatch: Any) -> None: build_flags={"build": True}, instantiate_flags={"inst": True}, copy_flags={"copy": True}, - ) == Path("/path/to/file") + ) == Path("/path/to/config") + mock_run.assert_has_calls( [ call( @@ -97,6 +112,8 @@ def test_remote_build(mock_run: Any, monkeypatch: Any) -> None: "", "--attr", "preAttr.config.system.build.toplevel", + "--add-root", + n.tmpdir.TMPDIR_PATH / "00000000000000000000000000000000", "--inst", ], stdout=PIPE, @@ -114,10 +131,28 @@ def test_remote_build(mock_run: Any, monkeypatch: Any) -> None: }, ), call( - ["nix-store", "--realise", Path("/path/to/file"), "--build"], + ["mktemp", "-d", "-t", "nixos-rebuild.XXXXX"], + remote=build_host, + stdout=PIPE, + ), + call( + [ + "nix-store", + "--realise", + Path("/path/to/file"), + "--add-root", + Path("/tmp/tmpdir/00000000000000000000000000000000"), + "--build", + ], + remote=build_host, + stdout=PIPE, + ), + call( + ["readlink", "-f", "/tmp/tmpdir/00000000000000000000000000000000"], remote=build_host, stdout=PIPE, ), + call(["rm", "-rf", Path("/tmp/tmpdir")], remote=build_host, check=False), ] )