From 927d182d908eee6648e4e0b237a78f3f31cd697d Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 11:12:38 +0000 Subject: [PATCH 01/10] nixos-rebuild-ng: move TMPDIR to its own file --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 5 ++++- .../ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 11 ++++------- .../ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py | 5 +++++ 3 files changed, 13 insertions(+), 8 deletions(-) create mode 100644 pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py index 50c4d9ba787f..ab406e7e0d87 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py @@ -8,7 +8,7 @@ from pathlib import Path from subprocess import CalledProcessError, run from typing import assert_never -from . import nix +from . import nix, tmpdir from .constants import EXECUTABLE, WITH_NIX_2_18, WITH_REEXEC, WITH_SHELL_FILES from .models import Action, BuildAttr, Flake, NRError, Profile from .process import Remote, cleanup_ssh @@ -280,7 +280,10 @@ def reexec( argv[0], new, ) + # Manually call clean-up functions since os.execve() will replace + # the process immediately cleanup_ssh() + tmpdir.TMPDIR.cleanup() os.execve(new, argv, os.environ | {"_NIXOS_REBUILD_REEXEC": "1"}) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py index 10666b47d657..337fae18b2ff 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py @@ -4,19 +4,17 @@ import shlex import subprocess from dataclasses import dataclass from getpass import getpass -from pathlib import Path -from tempfile import TemporaryDirectory from typing import Self, Sequence, TypedDict, Unpack +from . import tmpdir + logger = logging.getLogger(__name__) -TMPDIR = TemporaryDirectory(prefix="nixos-rebuild.") -TMPDIR_PATH = Path(TMPDIR.name) SSH_DEFAULT_OPTS = [ "-o", "ControlMaster=auto", "-o", - f"ControlPath={TMPDIR_PATH / "ssh-%n"}", + f"ControlPath={tmpdir.TMPDIR_PATH / "ssh-%n"}", "-o", "ControlPersist=60", ] @@ -70,13 +68,12 @@ class RunKwargs(TypedDict, total=False): def cleanup_ssh() -> None: "Close SSH ControlMaster connection." - for ctrl in TMPDIR_PATH.glob("ssh-*"): + for ctrl in tmpdir.TMPDIR_PATH.glob("ssh-*"): run_wrapper( ["ssh", "-o", f"ControlPath={ctrl}", "-O", "exit", "dummyhost"], check=False, capture_output=True, ) - TMPDIR.cleanup() def run_wrapper( diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py new file mode 100644 index 000000000000..ca71a6ddd8c3 --- /dev/null +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py @@ -0,0 +1,5 @@ +from pathlib import Path +from tempfile import TemporaryDirectory + +TMPDIR = TemporaryDirectory(prefix="nixos-rebuild.") +TMPDIR_PATH = Path(TMPDIR.name) From 3c6acbe080d9f78681a70092a218aaba7b86508e Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 12:31:15 +0000 Subject: [PATCH 02/10] 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 c1f2e5d5c711..c26a771c26f8 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 pathlib import Path 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 34f47029c103..6245aa5cd196 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 typing import Any 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), ] ) From aae4abe6c1da5f484de531f2c41afea2a751ff7e Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 12:53:08 +0000 Subject: [PATCH 03/10] nixos-rebuild-ng: convert side_effects mocks to function --- .../nixos-rebuild-ng/src/tests/test_main.py | 135 ++++++++++-------- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 10 +- 2 files changed, 80 insertions(+), 65 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py index 39a50ecc405a..b372a3e8f9a1 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py @@ -82,18 +82,20 @@ def test_execute_nix_boot(mock_run: Any, tmp_path: Path) -> None: nixpkgs_path.mkdir() config_path = tmp_path / "test" config_path.touch() - mock_run.side_effect = [ - # update_nixpkgs_rev - CompletedProcess([], 0, str(nixpkgs_path)), - CompletedProcess([], 0, "nixpkgs-rev"), - CompletedProcess([], 0), - # nixos_build - CompletedProcess([], 0, str(config_path)), - # set_profile - CompletedProcess([], 0), - # switch_to_configuration - CompletedProcess([], 0), - ] + + def run_wrapper_side_effect( + args: list[str], **kwargs: Any + ) -> CompletedProcess[str]: + if args[0] == "nix-instantiate": + return CompletedProcess([], 0, str(nixpkgs_path)) + elif args[0] == "git" and "rev-parse" in args: + return CompletedProcess([], 0, "nixpkgs-rev") + elif args[0] == "nix-build": + return CompletedProcess([], 0, str(config_path)) + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_wrapper_side_effect nr.execute(["nixos-rebuild", "boot", "--no-flake", "-vvv", "--fast"]) @@ -155,14 +157,16 @@ def test_execute_nix_boot(mock_run: Any, tmp_path: Path) -> None: def test_execute_nix_switch_flake(mock_run: Any, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() - mock_run.side_effect = [ - # nixos_build_flake - CompletedProcess([], 0, str(config_path)), - # set_profile - CompletedProcess([], 0), - # switch_to_configuration - CompletedProcess([], 0), - ] + + def run_wrapper_side_effect( + args: list[str], **kwargs: Any + ) -> CompletedProcess[str]: + if args[0] == "nix": + return CompletedProcess([], 0, str(config_path)) + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_wrapper_side_effect nr.execute( [ @@ -226,16 +230,16 @@ def test_execute_nix_switch_flake_target_host( ) -> None: config_path = tmp_path / "test" config_path.touch() - mock_run.side_effect = [ - # nixos_build_flake - CompletedProcess([], 0, str(config_path)), - # set_profile - CompletedProcess([], 0), - # copy_closure - CompletedProcess([], 0), - # switch_to_configuration - CompletedProcess([], 0), - ] + + def run_wrapper_side_effect( + args: list[str], **kwargs: Any + ) -> CompletedProcess[str]: + if args[0] == "nix": + return CompletedProcess([], 0, str(config_path)) + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_wrapper_side_effect nr.execute( [ @@ -317,18 +321,18 @@ def test_execute_nix_switch_flake_build_host( ) -> None: config_path = tmp_path / "test" config_path.touch() - mock_run.side_effect = [ - # nixos_build_flake - CompletedProcess([], 0, str(config_path)), - CompletedProcess([], 0), - CompletedProcess([], 0, str(config_path)), - # set_profile - CompletedProcess([], 0), - # copy_closure - CompletedProcess([], 0), - # switch_to_configuration - CompletedProcess([], 0), - ] + + def run_wrapper_side_effect( + args: list[str], **kwargs: Any + ) -> CompletedProcess[str]: + if args[0] == "nix" and "eval" in args: + return CompletedProcess([], 0, str(config_path)) + if args[0] == "ssh" and "nix" in args: + return CompletedProcess([], 0, str(config_path)) + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_wrapper_side_effect nr.execute( [ @@ -478,12 +482,16 @@ def test_execute_build(mock_run: Any, tmp_path: Path) -> None: def test_execute_test_flake(mock_run: Any, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() - mock_run.side_effect = [ - # nixos_build_flake - CompletedProcess([], 0, str(config_path)), - # switch_to_configuration - CompletedProcess([], 0), - ] + + def run_wrapper_side_effect( + args: list[str], **kwargs: Any + ) -> CompletedProcess[str]: + if args[0] == "nix": + return CompletedProcess([], 0, str(config_path)) + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_wrapper_side_effect nr.execute( ["nixos-rebuild", "test", "--flake", "github:user/repo#hostname", "--fast"] @@ -522,20 +530,23 @@ def test_execute_test_rollback( mock_path_exists: Any, mock_run: Any, ) -> None: - mock_run.side_effect = [ - # rollback_temporary_profile - CompletedProcess( - [], - 0, - stdout=textwrap.dedent("""\ - 2082 2024-11-07 22:58:56 - 2083 2024-11-07 22:59:41 - 2084 2024-11-07 23:54:17 (current) - """), - ), - # switch_to_configuration - CompletedProcess([], 0), - ] + def run_wrapper_side_effect( + args: list[str], **kwargs: Any + ) -> CompletedProcess[str]: + if args[0] == "nix-env": + return CompletedProcess( + [], + 0, + stdout=textwrap.dedent("""\ + 2082 2024-11-07 22:58:56 + 2083 2024-11-07 22:59:41 + 2084 2024-11-07 23:54:17 (current) + """), + ) + else: + return CompletedProcess([], 0) + + mock_run.side_effect = run_wrapper_side_effect nr.execute( ["nixos-rebuild", "test", "--rollback", "--profile-name", "foo", "--fast"] 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 6245aa5cd196..b2345d191a95 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 @@ -1,11 +1,11 @@ import textwrap +import uuid from pathlib import Path from subprocess import PIPE, CompletedProcess from typing import Any from unittest.mock import ANY, call, patch import pytest -import uuid import nixos_rebuild.models as m import nixos_rebuild.nix as n @@ -80,13 +80,17 @@ 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 + def run_wrapper_side_effect( + args: list[str], **kwargs: Any + ) -> CompletedProcess[str]: 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 ") + return CompletedProcess( + [], 0, stdout=" \n/tmp/tmpdir/00000000000000000000000000000000\n " + ) elif args[0] == "readlink": return CompletedProcess([], 0, stdout=" \n/path/to/config\n ") else: From 3d8b0daa1ae49a7cf93a09273a7dbad3024ce256 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 18:53:05 +0000 Subject: [PATCH 04/10] nixos-rebuild-ng: move atexit.register(cleanup_ssh) to process --- .../by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 3 --- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py index ab406e7e0d87..de851516d3bf 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py @@ -1,5 +1,4 @@ import argparse -import atexit import json import logging import os @@ -293,8 +292,6 @@ def execute(argv: list[str]) -> None: if not WITH_NIX_2_18: logger.warning("you're using Nix <2.18, some features will not work correctly") - atexit.register(cleanup_ssh) - common_flags = vars(args_groups["common_flags"]) common_build_flags = common_flags | vars(args_groups["common_build_flags"]) build_flags = common_build_flags | vars(args_groups["classic_build_flags"]) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py index 337fae18b2ff..f7c83f714937 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py @@ -1,3 +1,4 @@ +import atexit import logging import os import shlex @@ -76,6 +77,9 @@ def cleanup_ssh() -> None: ) +atexit.register(cleanup_ssh) + + def run_wrapper( args: Sequence[str | bytes | os.PathLike[str] | os.PathLike[bytes]], *, From 8c835bfe2183a2d647856d80abe427279c07d4b2 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 20:06:42 +0000 Subject: [PATCH 05/10] nixos-rebuild-ng: add help for --verbose --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py index de851516d3bf..979f83b3a4d0 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py @@ -19,7 +19,14 @@ logger.setLevel(logging.INFO) def get_parser() -> tuple[argparse.ArgumentParser, dict[str, argparse.ArgumentParser]]: common_flags = argparse.ArgumentParser(add_help=False) - common_flags.add_argument("--verbose", "-v", action="count", dest="v", default=0) + common_flags.add_argument( + "--verbose", + "-v", + action="count", + dest="v", + default=0, + help="Enable verbose logging (includes nix)", + ) common_flags.add_argument("--max-jobs", "-j") common_flags.add_argument("--cores") common_flags.add_argument("--log-format") From b91be5b0e07ab9190b8c8def179694ab6f84c438 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 22:05:46 +0000 Subject: [PATCH 06/10] nixos-rebuild-ng: exit with the error code of the process when CalledProcessError --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py index 979f83b3a4d0..f99dadd1d319 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py @@ -485,6 +485,15 @@ def main() -> None: try: execute(sys.argv) + except CalledProcessError as ex: + if logger.level == logging.DEBUG: + import traceback + + traceback.print_exc() + else: + print(str(ex), file=sys.stderr) + # Exit with the error code of the process that failed + sys.exit(ex.returncode) except (Exception, KeyboardInterrupt) as ex: if logger.level == logging.DEBUG: raise From ed7136fee403ffb559bb57e2b2df9301d0e99bf3 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 16 Dec 2024 23:06:31 +0000 Subject: [PATCH 07/10] nixos-rebuild-ng: add Final annotations for some constants --- .../by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 4 ++-- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py index f7c83f714937..54c8b71036e9 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py @@ -5,13 +5,13 @@ import shlex import subprocess from dataclasses import dataclass from getpass import getpass -from typing import Self, Sequence, TypedDict, Unpack +from typing import Final, Self, Sequence, TypedDict, Unpack from . import tmpdir logger = logging.getLogger(__name__) -SSH_DEFAULT_OPTS = [ +SSH_DEFAULT_OPTS: Final = [ "-o", "ControlMaster=auto", "-o", diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py index ca71a6ddd8c3..521af84ce464 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/tmpdir.py @@ -1,5 +1,6 @@ from pathlib import Path from tempfile import TemporaryDirectory +from typing import Final -TMPDIR = TemporaryDirectory(prefix="nixos-rebuild.") -TMPDIR_PATH = Path(TMPDIR.name) +TMPDIR: Final = TemporaryDirectory(prefix="nixos-rebuild.") +TMPDIR_PATH: Final = Path(TMPDIR.name) From c2dc57aec41513ffe106f2b5bf9dd1b880f197fa Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Tue, 17 Dec 2024 17:22:47 +0000 Subject: [PATCH 08/10] nixos-rebuild-ng: tweak reexec warning message --- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py index f99dadd1d319..8727f77df96a 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py @@ -275,7 +275,7 @@ def reexec( build_attr = BuildAttr.from_arg(args.attr, args.file) drv = nix.build(attr, build_attr, **build_flags, no_out_link=True) except CalledProcessError: - logger.warning("could not find a newer version of nixos-rebuild") + logger.warning("could not build a newer version of nixos-rebuild") if drv: new = drv / f"bin/{EXECUTABLE}" From e20247b0ecb147701613e8ba2e2187bf2ddad846 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Tue, 17 Dec 2024 17:50:03 +0000 Subject: [PATCH 09/10] nixos-rebuild-ng: do not ignore target_host in reexec --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py index 8727f77df96a..2faa0771f870 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py @@ -268,8 +268,9 @@ def reexec( drv = None attr = "config.system.build.nixos-rebuild" try: - # Need to set target_host=None, to avoid connecting to remote - if flake := Flake.from_arg(args.flake, None): + # Parsing the args here but ignore ask_sudo_password since it is not + # needed and we would end up asking sudo password twice + if flake := Flake.from_arg(args.flake, Remote.from_arg(args.target_host, None)): drv = nix.build_flake(attr, flake, **flake_build_flags, no_link=True) else: build_attr = BuildAttr.from_arg(args.attr, args.file) From e736564cdf3a6f4123f698da2cca06fc90e75df4 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Tue, 17 Dec 2024 20:15:34 +0000 Subject: [PATCH 10/10] nixos-rebuild-ng: improve test for test_remote_build --- .../nixos-rebuild-ng/src/tests/test_main.py | 36 +++++++------------ .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 12 +++---- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py index b372a3e8f9a1..23fd55283fce 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_main.py @@ -83,9 +83,7 @@ def test_execute_nix_boot(mock_run: Any, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() - def run_wrapper_side_effect( - args: list[str], **kwargs: Any - ) -> CompletedProcess[str]: + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix-instantiate": return CompletedProcess([], 0, str(nixpkgs_path)) elif args[0] == "git" and "rev-parse" in args: @@ -95,7 +93,7 @@ def test_execute_nix_boot(mock_run: Any, tmp_path: Path) -> None: else: return CompletedProcess([], 0) - mock_run.side_effect = run_wrapper_side_effect + mock_run.side_effect = run_side_effect nr.execute(["nixos-rebuild", "boot", "--no-flake", "-vvv", "--fast"]) @@ -158,15 +156,13 @@ def test_execute_nix_switch_flake(mock_run: Any, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() - def run_wrapper_side_effect( - args: list[str], **kwargs: Any - ) -> CompletedProcess[str]: + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix": return CompletedProcess([], 0, str(config_path)) else: return CompletedProcess([], 0) - mock_run.side_effect = run_wrapper_side_effect + mock_run.side_effect = run_side_effect nr.execute( [ @@ -231,15 +227,13 @@ def test_execute_nix_switch_flake_target_host( config_path = tmp_path / "test" config_path.touch() - def run_wrapper_side_effect( - args: list[str], **kwargs: Any - ) -> CompletedProcess[str]: + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix": return CompletedProcess([], 0, str(config_path)) else: return CompletedProcess([], 0) - mock_run.side_effect = run_wrapper_side_effect + mock_run.side_effect = run_side_effect nr.execute( [ @@ -322,9 +316,7 @@ def test_execute_nix_switch_flake_build_host( config_path = tmp_path / "test" config_path.touch() - def run_wrapper_side_effect( - args: list[str], **kwargs: Any - ) -> CompletedProcess[str]: + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix" and "eval" in args: return CompletedProcess([], 0, str(config_path)) if args[0] == "ssh" and "nix" in args: @@ -332,7 +324,7 @@ def test_execute_nix_switch_flake_build_host( else: return CompletedProcess([], 0) - mock_run.side_effect = run_wrapper_side_effect + mock_run.side_effect = run_side_effect nr.execute( [ @@ -483,15 +475,13 @@ def test_execute_test_flake(mock_run: Any, tmp_path: Path) -> None: config_path = tmp_path / "test" config_path.touch() - def run_wrapper_side_effect( - args: list[str], **kwargs: Any - ) -> CompletedProcess[str]: + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix": return CompletedProcess([], 0, str(config_path)) else: return CompletedProcess([], 0) - mock_run.side_effect = run_wrapper_side_effect + mock_run.side_effect = run_side_effect nr.execute( ["nixos-rebuild", "test", "--flake", "github:user/repo#hostname", "--fast"] @@ -530,9 +520,7 @@ def test_execute_test_rollback( mock_path_exists: Any, mock_run: Any, ) -> None: - def run_wrapper_side_effect( - args: list[str], **kwargs: Any - ) -> CompletedProcess[str]: + def run_side_effect(args: list[str], **kwargs: Any) -> CompletedProcess[str]: if args[0] == "nix-env": return CompletedProcess( [], @@ -546,7 +534,7 @@ def test_execute_test_rollback( else: return CompletedProcess([], 0) - mock_run.side_effect = run_wrapper_side_effect + mock_run.side_effect = run_side_effect nr.execute( ["nixos-rebuild", "test", "--rollback", "--profile-name", "foo", "--fast"] 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 b2345d191a95..c2bef1fffe97 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 @@ -88,16 +88,14 @@ def test_remote_build(mock_uuid4: Any, mock_run: Any, monkeypatch: Any) -> None: 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 " - ) + return CompletedProcess([], 0, stdout=" \n/tmp/tmpdir/config\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) + mock_uuid4.side_effect = [uuid.UUID(int=1), uuid.UUID(int=2)] assert n.remote_build( "config.system.build.toplevel", @@ -117,7 +115,7 @@ def test_remote_build(mock_uuid4: Any, mock_run: Any, monkeypatch: Any) -> None: "--attr", "preAttr.config.system.build.toplevel", "--add-root", - n.tmpdir.TMPDIR_PATH / "00000000000000000000000000000000", + n.tmpdir.TMPDIR_PATH / "00000000000000000000000000000001", "--inst", ], stdout=PIPE, @@ -145,14 +143,14 @@ def test_remote_build(mock_uuid4: Any, mock_run: Any, monkeypatch: Any) -> None: "--realise", Path("/path/to/file"), "--add-root", - Path("/tmp/tmpdir/00000000000000000000000000000000"), + Path("/tmp/tmpdir/00000000000000000000000000000002"), "--build", ], remote=build_host, stdout=PIPE, ), call( - ["readlink", "-f", "/tmp/tmpdir/00000000000000000000000000000000"], + ["readlink", "-f", "/tmp/tmpdir/config"], remote=build_host, stdout=PIPE, ),