From d325edd6272a18fa5e7f2e5b5c611badeb58e4a2 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Wed, 27 Nov 2024 19:01:57 +0000 Subject: [PATCH 01/41] nixos-rebuild-ng: introduce models.BuildingAttr --- .../src/nixos_rebuild/__init__.py | 22 ++++++++----------- .../src/nixos_rebuild/models.py | 12 ++++++++++ .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 15 +++++-------- .../nixos-rebuild-ng/src/tests/test_models.py | 13 +++++++++++ .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 16 +++++--------- 5 files changed, 45 insertions(+), 33 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 25519dac6240..a83718c89587 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 @@ -9,7 +9,7 @@ from tempfile import TemporaryDirectory from typing import assert_never from . import nix -from .models import Action, Flake, NRError, Profile +from .models import Action, BuildAttr, Flake, NRError, Profile from .process import Remote, cleanup_ssh from .utils import info @@ -170,6 +170,7 @@ def execute(argv: list[str]) -> None: profile = Profile.from_name(args.profile_name) target_host = Remote.from_arg(args.target_host, args.ask_sudo_password, tmpdir_path) + build_attr = BuildAttr.from_arg(args.attr, args.file) flake = Flake.from_arg(args.flake, target_host) if args.upgrade or args.upgrade_all: @@ -204,8 +205,7 @@ def execute(argv: list[str]) -> None: else: path_to_config = nix.nixos_build( "system", - args.attr, - args.file, + build_attr, no_out_link=True, **build_flags, ) @@ -244,8 +244,7 @@ def execute(argv: list[str]) -> None: else: path_to_config = nix.nixos_build( "system", - args.attr, - args.file, + build_attr, dry_run=dry_run, **build_flags, ) @@ -269,8 +268,7 @@ def execute(argv: list[str]) -> None: else: path_to_config = nix.nixos_build( attr, - args.attr, - args.file, + build_attr, **build_flags, ) vm_path = next(path_to_config.glob("bin/run-*-vm"), "./result/bin/run-*-vm") @@ -307,12 +305,10 @@ def execute(argv: list[str]) -> None: ) print(table) case Action.REPL: - # For now just redirect it to `nixos-rebuild` instead of - # duplicating the code - os.execv( - "@nixos_rebuild@", - argv, - ) + if flake: + nix.repl_flake("toplevel", flake, **flake_build_flags) + else: + nix.repl("system", build_attr, **build_flags) case _: assert_never(action) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py index 51cee3b7c517..59d99230061c 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py @@ -45,6 +45,18 @@ class Action(Enum): return [a.value for a in Action] +@dataclass(frozen=True) +class BuildAttr: + path: Path + attr: str | None + + @classmethod + def from_arg(cls, attr: str | None, file: str | None) -> Self | None: + if not (attr or file): + return None + return cls(Path(file or "default.nix"), attr) + + @dataclass(frozen=True) class Flake: path: Path 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 f0c328cff4c0..4009996609c7 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 @@ -6,6 +6,7 @@ from typing import Final from .models import ( Action, + BuildAttr, Flake, Generation, GenerationJson, @@ -235,24 +236,20 @@ def list_generations(profile: Profile) -> list[GenerationJson]: def nixos_build( attr: str, - pre_attr: str | None, - file: str | None, + build_attr: BuildAttr | None, **nix_flags: Args, ) -> Path: """Build NixOS attribute using classic Nix. - It will by default build `` with `attr`, however it - optionally supports building from an external file and custom attributes - paths. - Returns the built attribute as path. """ - if pre_attr or file: + run_args: list[str | Path] + if build_attr: run_args = [ "nix-build", - file or "default.nix", + build_attr.path, "--attr", - f"{'.'.join(x for x in [pre_attr, attr] if x)}", + f"{'.'.join(x for x in [build_attr.attr, attr] if x)}", ] else: run_args = ["nix-build", "", "--attr", attr] diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py index f30ef4059310..a2646eaec416 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py @@ -9,6 +9,19 @@ import nixos_rebuild.models as m from .helpers import get_qualified_name +def test_building_attr_from_arg() -> None: + assert m.BuildAttr.from_arg(None, None) is None + assert m.BuildAttr.from_arg("attr", None) == m.BuildAttr( + Path("default.nix"), "attr" + ) + assert m.BuildAttr.from_arg("attr", "file.nix") == m.BuildAttr( + Path("file.nix"), "attr" + ) + assert m.BuildAttr.from_arg(None, "file.nix") == m.BuildAttr( + Path("file.nix"), None + ) + + def test_flake_parse() -> None: assert m.Flake.parse("/path/to/flake#attr") == m.Flake( Path("/path/to/flake"), "nixosConfigurations.attr" 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 8234f0ff98f9..0d6079e8eb5b 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 @@ -229,27 +229,21 @@ def test_nixos_build_flake(mock_run: Any) -> None: return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), ) def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: - assert n.nixos_build("attr", None, None, nix_flag="foo") == Path("/path/to/file") + assert n.nixos_build("attr", None, nix_flag="foo") == Path("/path/to/file") mock_run.assert_called_with( ["nix-build", "", "--attr", "attr", "--nix-flag", "foo"], stdout=PIPE, ) - n.nixos_build("attr", "preAttr", "file") + n.nixos_build("attr", m.BuildAttr(Path("file"), "preAttr")) mock_run.assert_called_with( - ["nix-build", "file", "--attr", "preAttr.attr"], + ["nix-build", Path("file"), "--attr", "preAttr.attr"], stdout=PIPE, ) - n.nixos_build("attr", None, "file", no_out_link=True) + n.nixos_build("attr", m.BuildAttr(Path("file"), None)) mock_run.assert_called_with( - ["nix-build", "file", "--attr", "attr", "--no-out-link"], - stdout=PIPE, - ) - - n.nixos_build("attr", "preAttr", None, no_out_link=False, keep_going=True) - mock_run.assert_called_with( - ["nix-build", "default.nix", "--attr", "preAttr.attr", "--keep-going"], + ["nix-build", Path("file"), "--attr", "attr"], stdout=PIPE, ) From 88b4eb3aeb0f24a1055dd83bd977b19175cd9d88 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Wed, 27 Nov 2024 19:07:16 +0000 Subject: [PATCH 02/41] nixos-rebuild-ng: add repl --- pkgs/by-name/ni/nixos-rebuild-ng/README.md | 2 +- pkgs/by-name/ni/nixos-rebuild-ng/package.nix | 5 --- .../src/nixos_rebuild/__init__.py | 1 - .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 38 ++++++++++++++++++ .../src/nixos_rebuild/repl.template.nix | 40 +++++++++++++++++++ .../ni/nixos-rebuild-ng/src/pyproject.toml | 3 ++ .../nixos-rebuild-ng/src/tests/test_models.py | 4 +- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 19 +++++++++ 8 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.template.nix diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/README.md b/pkgs/by-name/ni/nixos-rebuild-ng/README.md index a6199e873060..f34c3d4996ef 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/README.md +++ b/pkgs/by-name/ni/nixos-rebuild-ng/README.md @@ -126,7 +126,7 @@ ruff format . - [ ] Change module system to allow easier opt-in, like `system.switch.enableNg` for `switch-to-configuration-ng` - [ ] Improve documentation -- [ ] `nixos-rebuild repl` (calling old `nixos-rebuild` for now) +- [x] `nixos-rebuild repl` - [ ] `nix` build/bootstrap - [ ] Generate tab completion via [`shtab`](https://docs.iterative.ai/shtab/) - [x] Reduce build closure diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/package.nix b/pkgs/by-name/ni/nixos-rebuild-ng/package.nix index b111c7cb108a..4ef9d29807f5 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/package.nix +++ b/pkgs/by-name/ni/nixos-rebuild-ng/package.nix @@ -39,11 +39,6 @@ python3Packages.buildPythonApplication rec { nix ]; - preBuild = '' - substituteInPlace nixos_rebuild/__init__.py \ - --subst-var-by nixos_rebuild ${lib.getExe nixos-rebuild} - ''; - postInstall = '' installManPage ${nixos-rebuild}/share/man/man8/nixos-rebuild.8 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 a83718c89587..f57991800404 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,7 +1,6 @@ import argparse import atexit import json -import os import sys from pathlib import Path from subprocess import run 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 4009996609c7..aeaea3bf03c9 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 @@ -1,6 +1,8 @@ import os from datetime import datetime +from importlib.resources import files from pathlib import Path +from string import Template from subprocess import PIPE, CalledProcessError from typing import Final @@ -18,6 +20,7 @@ from .process import run_wrapper from .utils import Args, dict_to_flags, info FLAKE_FLAGS: Final = ["--extra-experimental-features", "nix-command flakes"] +FLAKE_REPL_TEMPLATE: Final = "repl.template.nix" def copy_closure( @@ -279,6 +282,41 @@ def nixos_build_flake( return Path(r.stdout.strip()) +def repl(attr: str, build_attr: BuildAttr | None, **nix_flags: Args) -> None: + run_args: list[str | Path] = ["nix", "repl", "--file"] + if build_attr: + run_args.append(build_attr.path) + if build_attr.attr: + run_args.append(build_attr.attr) + run_wrapper([*run_args, *dict_to_flags(nix_flags)]) + else: + run_wrapper([*run_args, "", *dict_to_flags(nix_flags)]) + + +def repl_flake(attr: str, flake: Flake, **flake_flags: Args) -> None: + expr = Template( + files(__package__).joinpath(FLAKE_REPL_TEMPLATE).read_text() + ).substitute( + flake_path=flake.path, + flake_attr=flake.attr, + bold="\033[1m", + blue="\033[34;1m", + attention="\033[35;1m", + reset="\033[0m", + ) + run_wrapper( + [ + "nix", + *FLAKE_FLAGS, + "repl", + "--impure", + "--expr", + expr, + *dict_to_flags(flake_flags), + ] + ) + + def rollback(profile: Profile, target_host: Remote | None, sudo: bool) -> Path: "Rollback Nix profile, like one created by `nixos-rebuild switch`." run_wrapper( diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.template.nix b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.template.nix new file mode 100644 index 000000000000..0fffd6ecc746 --- /dev/null +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.template.nix @@ -0,0 +1,40 @@ +let + flake = builtins.getFlake ''${flake_path}''; + configuration = flake.${flake_attr}; + motd = '' + $${"\n"} + Hello and welcome to the NixOS configuration + ${flake_attr} + in ${flake_path} + + The following is loaded into nix repl's scope: + + - ${blue}config${reset} All option values + - ${blue}options${reset} Option data and metadata + - ${blue}pkgs${reset} Nixpkgs package set + - ${blue}lib${reset} Nixpkgs library functions + - other module arguments + + - ${blue}flake${reset} Flake outputs, inputs and source info of ${flake_path} + + Use tab completion to browse around ${blue}config${reset}. + + Use ${bold}:r${reset} to ${bold}reload${reset} everything after making a change in the flake. + (assuming ${flake_path} is a mutable flake ref) + + See ${bold}:?${reset} for more repl commands. + + ${attention}warning:${reset} nixos-rebuild repl does not currently enforce pure evaluation. + ''; + scope = + assert configuration._type or null == ''configuration''; + assert configuration.class or ''nixos'' == ''nixos''; + configuration._module.args + // configuration._module.specialArgs + // { + inherit (configuration) config options; + lib = configuration.lib or configuration.pkgs.lib; + inherit flake; + }; +in +builtins.seq scope builtins.trace motd scope diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml b/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml index f26b3cba7056..0dc662814c33 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml @@ -9,6 +9,9 @@ version = "0.0.0" [project.scripts] nixos-rebuild = "nixos_rebuild:main" +[tool.setuptools.package-data] +nixos_rebuild = ["*.template.nix"] + [tool.mypy] # `--strict` config, but explicit options to avoid breaking build when mypy is # updated diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py index a2646eaec416..1a604ad53164 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py @@ -17,9 +17,7 @@ def test_building_attr_from_arg() -> None: assert m.BuildAttr.from_arg("attr", "file.nix") == m.BuildAttr( Path("file.nix"), "attr" ) - assert m.BuildAttr.from_arg(None, "file.nix") == m.BuildAttr( - Path("file.nix"), None - ) + assert m.BuildAttr.from_arg(None, "file.nix") == m.BuildAttr(Path("file.nix"), None) def test_flake_parse() -> None: 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 0d6079e8eb5b..92411a0f5488 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 @@ -248,6 +248,25 @@ def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: ) +@patch(get_qualified_name(n.run_wrapper, n), autospec=True) +def test_repl(mock_run: Any) -> None: + n.repl("attr", None, nix_flag=True) + mock_run.assert_called_with( + ["nix", "repl", "--file", "", "--nix-flag"] + ) + + n.repl("attr", m.BuildAttr(Path("file.nix"), "myAttr")) + mock_run.assert_called_with(["nix", "repl", "--file", Path("file.nix"), "myAttr"]) + + +@patch(get_qualified_name(n.run_wrapper, n), autospec=True) +def test_repl_flake(mock_run: Any) -> None: + n.repl_flake("attr", m.Flake(Path("flake.nix"), "myAttr"), nix_flag=True) + # This method would be really annoying to test, and it is not that important + # So just check that we are at least calling it + assert mock_run.called + + @patch(get_qualified_name(n.run_wrapper, n), autospec=True) def test_rollback(mock_run: Any, tmp_path: Path) -> None: path = tmp_path / "test" From 0774b36546976a2a0454f51bec02d1b695c5f9dd Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Wed, 27 Nov 2024 23:14:22 +0000 Subject: [PATCH 03/41] nixos-rebuild-ng: move default path logic to BuildAttr --- .../src/nixos_rebuild/models.py | 6 ++-- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 34 +++++++------------ .../nixos-rebuild-ng/src/tests/test_models.py | 4 +-- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 12 +++---- 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py index 59d99230061c..3039a3eafc7f 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py @@ -47,13 +47,13 @@ class Action(Enum): @dataclass(frozen=True) class BuildAttr: - path: Path + path: str | Path attr: str | None @classmethod - def from_arg(cls, attr: str | None, file: str | None) -> Self | None: + def from_arg(cls, attr: str | None, file: str | None) -> Self: if not (attr or file): - return None + return cls("", None) return cls(Path(file or "default.nix"), attr) 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 aeaea3bf03c9..f56044451f02 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 @@ -239,24 +239,20 @@ def list_generations(profile: Profile) -> list[GenerationJson]: def nixos_build( attr: str, - build_attr: BuildAttr | None, + build_attr: BuildAttr, **nix_flags: Args, ) -> Path: """Build NixOS attribute using classic Nix. Returns the built attribute as path. """ - run_args: list[str | Path] - if build_attr: - run_args = [ - "nix-build", - build_attr.path, - "--attr", - f"{'.'.join(x for x in [build_attr.attr, attr] if x)}", - ] - else: - run_args = ["nix-build", "", "--attr", attr] - run_args += dict_to_flags(nix_flags) + run_args = [ + "nix-build", + build_attr.path, + "--attr", + f"{'.'.join(x for x in [build_attr.attr, attr] if x)}", + *dict_to_flags(nix_flags), + ] r = run_wrapper(run_args, stdout=PIPE) return Path(r.stdout.strip()) @@ -282,15 +278,11 @@ def nixos_build_flake( return Path(r.stdout.strip()) -def repl(attr: str, build_attr: BuildAttr | None, **nix_flags: Args) -> None: - run_args: list[str | Path] = ["nix", "repl", "--file"] - if build_attr: - run_args.append(build_attr.path) - if build_attr.attr: - run_args.append(build_attr.attr) - run_wrapper([*run_args, *dict_to_flags(nix_flags)]) - else: - run_wrapper([*run_args, "", *dict_to_flags(nix_flags)]) +def repl(attr: str, build_attr: BuildAttr, **nix_flags: Args) -> None: + run_args = ["nix", "repl", "--file", build_attr.path] + if build_attr.attr: + run_args.append(build_attr.attr) + run_wrapper([*run_args, *dict_to_flags(nix_flags)]) def repl_flake(attr: str, flake: Flake, **flake_flags: Args) -> None: diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py index 1a604ad53164..31920e97a25c 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py @@ -9,8 +9,8 @@ import nixos_rebuild.models as m from .helpers import get_qualified_name -def test_building_attr_from_arg() -> None: - assert m.BuildAttr.from_arg(None, None) is None +def test_build_attr_from_arg() -> None: + assert m.BuildAttr.from_arg(None, None) == m.BuildAttr("", None) assert m.BuildAttr.from_arg("attr", None) == m.BuildAttr( Path("default.nix"), "attr" ) 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 92411a0f5488..e47869867a7e 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 @@ -229,7 +229,9 @@ def test_nixos_build_flake(mock_run: Any) -> None: return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), ) def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: - assert n.nixos_build("attr", None, nix_flag="foo") == Path("/path/to/file") + assert n.nixos_build( + "attr", m.BuildAttr("", None), nix_flag="foo" + ) == Path("/path/to/file") mock_run.assert_called_with( ["nix-build", "", "--attr", "attr", "--nix-flag", "foo"], stdout=PIPE, @@ -241,16 +243,10 @@ def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: stdout=PIPE, ) - n.nixos_build("attr", m.BuildAttr(Path("file"), None)) - mock_run.assert_called_with( - ["nix-build", Path("file"), "--attr", "attr"], - stdout=PIPE, - ) - @patch(get_qualified_name(n.run_wrapper, n), autospec=True) def test_repl(mock_run: Any) -> None: - n.repl("attr", None, nix_flag=True) + n.repl("attr", m.BuildAttr("", None), nix_flag=True) mock_run.assert_called_with( ["nix", "repl", "--file", "", "--nix-flag"] ) From 5cc71a346a16620c721ac08be73efe1361a60c97 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Wed, 27 Nov 2024 23:51:27 +0000 Subject: [PATCH 04/41] nixos-rebuild-ng: Profile.from_name -> Profile.from_arg --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 2 +- .../by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py | 2 +- pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py | 6 +++--- 3 files changed, 5 insertions(+), 5 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 f57991800404..e37f50ef7611 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 @@ -167,7 +167,7 @@ def execute(argv: list[str]) -> None: tmpdir_path = Path(tmpdir.name) atexit.register(cleanup_ssh, tmpdir_path) - profile = Profile.from_name(args.profile_name) + profile = Profile.from_arg(args.profile_name) target_host = Remote.from_arg(args.target_host, args.ask_sudo_password, tmpdir_path) build_attr = BuildAttr.from_arg(args.attr, args.file) flake = Flake.from_arg(args.flake, target_host) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py index 3039a3eafc7f..235ce9424e3a 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py @@ -137,7 +137,7 @@ class Profile: path: Path @classmethod - def from_name(cls, name: str = "system") -> Self: + def from_arg(cls, name: str) -> Self: match name: case "system": return cls(name, Path("/nix/var/nix/profiles/system")) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py index 31920e97a25c..baeb5977b84f 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py @@ -111,13 +111,13 @@ def test_flake_from_arg(mock_node: Any) -> None: @patch(get_qualified_name(m.Path.mkdir, m), autospec=True) -def test_profile_from_name(mock_mkdir: Any) -> None: - assert m.Profile.from_name("system") == m.Profile( +def test_profile_from_arg(mock_mkdir: Any) -> None: + assert m.Profile.from_arg("system") == m.Profile( "system", Path("/nix/var/nix/profiles/system"), ) - assert m.Profile.from_name("something") == m.Profile( + assert m.Profile.from_arg("something") == m.Profile( "something", Path("/nix/var/nix/profiles/system-profiles/something"), ) From 2db09d7f77305e4b6283cf2adb3c7151ecc7b570 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 10:20:41 +0000 Subject: [PATCH 05/41] nixos-rebuild-ng: configure logging --- .../src/nixos_rebuild/__init__.py | 24 ++++++++++++------- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 15 ++++++++---- .../src/nixos_rebuild/process.py | 15 +++++++----- .../src/nixos_rebuild/utils.py | 20 ++++++++++++---- .../nixos-rebuild-ng/src/tests/test_main.py | 5 ++-- 5 files changed, 53 insertions(+), 26 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 e37f50ef7611..51f2c79e7702 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,6 +1,7 @@ import argparse import atexit import json +import logging import sys from pathlib import Path from subprocess import run @@ -10,9 +11,10 @@ from typing import assert_never from . import nix from .models import Action, BuildAttr, Flake, NRError, Profile from .process import Remote, cleanup_ssh -from .utils import info +from .utils import LogFormatter -VERBOSE = 0 +logger = logging.getLogger() +logger.setLevel(logging.INFO) def get_parser() -> tuple[argparse.ArgumentParser, dict[str, argparse.ArgumentParser]]: @@ -103,11 +105,11 @@ def parse_args( } def parser_warn(msg: str) -> None: - info(f"{parser.prog}: warning: {msg}") + print(f"{parser.prog}: warning: {msg}", file=sys.stderr) - global VERBOSE # This flag affects both nix and this script - VERBOSE = args.verbose + if args.verbose: + logger.setLevel(logging.DEBUG) # https://github.com/NixOS/nixpkgs/blob/master/pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh#L56 if args.action == Action.DRY_RUN.value: @@ -190,7 +192,7 @@ def execute(argv: list[str]) -> None: match action: case Action.SWITCH | Action.BOOT: - info("building the system configuration...") + logger.info("building the system configuration...") if args.rollback: path_to_config = nix.rollback(profile, target_host, sudo=args.sudo) else: @@ -219,7 +221,7 @@ def execute(argv: list[str]) -> None: install_bootloader=args.install_bootloader, ) case Action.TEST | Action.BUILD | Action.DRY_BUILD | Action.DRY_ACTIVATE: - info("building the system configuration...") + logger.info("building the system configuration...") dry_run = action == Action.DRY_BUILD if args.rollback: if action not in (Action.TEST, Action.BUILD): @@ -256,7 +258,7 @@ def execute(argv: list[str]) -> None: specialisation=args.specialisation, ) case Action.BUILD_VM | Action.BUILD_VM_WITH_BOOTLOADER: - info("building the system configuration...") + logger.info("building the system configuration...") attr = "vm" if action == Action.BUILD_VM else "vmWithBootLoader" if flake: path_to_config = nix.nixos_build_flake( @@ -313,10 +315,14 @@ def execute(argv: list[str]) -> None: def main() -> None: + ch = logging.StreamHandler() + ch.setFormatter(LogFormatter("%(levelname)s: %(message)s")) + logger.addHandler(ch) + try: execute(sys.argv) except (Exception, KeyboardInterrupt) as ex: - if VERBOSE: + if logger.level == logging.DEBUG: raise ex else: sys.exit(str(ex)) 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 f56044451f02..ab34b0896ffc 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 @@ -1,3 +1,4 @@ +import logging import os from datetime import datetime from importlib.resources import files @@ -17,10 +18,11 @@ from .models import ( Remote, ) from .process import run_wrapper -from .utils import Args, dict_to_flags, info +from .utils import Args, dict_to_flags FLAKE_FLAGS: Final = ["--extra-experimental-features", "nix-command flakes"] FLAKE_REPL_TEMPLATE: Final = "repl.template.nix" +logger = logging.getLogger(__name__) def copy_closure( @@ -107,7 +109,7 @@ def get_nixpkgs_rev(nixpkgs_path: Path | None) -> str | None: ) except FileNotFoundError: # Git is not included in the closure so we need to check - info(f"warning: Git not found; cannot figure out revision of '{nixpkgs_path}'") + logger.warning(f"Git not found; cannot figure out revision of '{nixpkgs_path}'") return None if rev := r.stdout.strip(): @@ -203,13 +205,15 @@ def list_generations(profile: Profile) -> list[GenerationJson]: ) try: nixos_version = (generation_path / "nixos-version").read_text().strip() - except IOError: + except IOError as ex: + logger.debug("could not get nixos-version: %s", ex) nixos_version = "Unknown" try: kernel_version = next( (generation_path / "kernel-modules/lib/modules").iterdir() ).name - except IOError: + except IOError as ex: + logger.debug("could not get kernel version: %s", ex) kernel_version = "Unknown" specialisations = [ s.name for s in (generation_path / "specialisation").glob("*") if s.is_dir() @@ -219,7 +223,8 @@ def list_generations(profile: Profile) -> list[GenerationJson]: [generation_path / "sw/bin/nixos-version", "--configuration-revision"], capture_output=True, ).stdout.strip() - except (CalledProcessError, IOError): + except (CalledProcessError, IOError) as ex: + logger.debug("could not get configuration revision: %s", ex) configuration_revision = "Unknown" result.append( 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 db470ee5753c..91a9f04593c8 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,5 +1,6 @@ from __future__ import annotations +import logging import os import subprocess from dataclasses import dataclass @@ -7,7 +8,7 @@ from getpass import getpass from pathlib import Path from typing import Self, Sequence, TypedDict, Unpack -from .utils import info +logger = logging.getLogger(__name__) @dataclass(frozen=True) @@ -46,13 +47,13 @@ class Remote: def _validate_opts(opts: list[str], ask_sudo_password: bool | None) -> None: for o in opts: if o in ["-t", "-tt", "RequestTTY=yes", "RequestTTY=force"]: - info( - f"warning: detected option '{o}' in NIX_SSHOPTS. SSH's TTY " - + "may cause issues, it is recommended to remove this option" + logger.warning( + f"detected option '{o}' in NIX_SSHOPTS. SSH's TTY may " + + "cause issues, it is recommended to remove this option" ) if not ask_sudo_password: - info( - "If you want to prompt for sudo password use " + logger.warning( + "if you want to prompt for sudo password use " + "'--ask-sudo-password' option instead" ) @@ -99,6 +100,8 @@ def run_wrapper( if sudo: args = ["sudo", *args] + logger.debug("calling run with args=%s, extra_env=%s", args, extra_env) + return subprocess.run( args, check=check, diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py index 8d4f128075d4..dc8d899ab056 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py @@ -1,11 +1,23 @@ -import sys -from functools import partial -from typing import TypeAlias +import logging +from typing import TypeAlias, override -info = partial(print, file=sys.stderr) Args: TypeAlias = bool | str | list[str] | int | None +class LogFormatter(logging.Formatter): + @override + def format(self, record: logging.LogRecord) -> str: + record.levelname = record.levelname.lower() + match record.levelno: + case logging.INFO: + self._style._fmt = "%(message)s" + case logging.DEBUG: + self._style._fmt = "%(levelname)s: %(name)s: %(message)s" + case _: + self._style._fmt = "%(levelname)s: %(message)s" + return super().format(record) + + def dict_to_flags(d: dict[str, Args]) -> list[str]: flags = [] for key, value in d.items(): 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 e3bb1ba8f6e7..8bb624a44c30 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 @@ -1,3 +1,4 @@ +import logging import textwrap from pathlib import Path from subprocess import PIPE, CompletedProcess @@ -43,7 +44,7 @@ def test_parse_args() -> None: "bar", ] ) - assert nr.VERBOSE == 0 + assert nr.logger.level == logging.INFO assert r1.flake == "/etc/nixos" assert r1.install_bootloader is True assert r1.install_grub is True @@ -65,7 +66,7 @@ def test_parse_args() -> None: "-vvv", ] ) - assert nr.VERBOSE == 3 + assert nr.logger.level == logging.DEBUG assert r2.verbose == 3 assert r2.flake is False assert r2.action == "dry-build" From 1e34a97f9f793110ecfc9f5f08a9e86961ad0d57 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 13:12:47 +0000 Subject: [PATCH 06/41] nixos-rebuild-ng: add message to help if the user forgot --ask-sudo-password --- .../src/nixos_rebuild/process.py | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 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 91a9f04593c8..a64905d7d3bc 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 @@ -102,14 +102,22 @@ def run_wrapper( logger.debug("calling run with args=%s, extra_env=%s", args, extra_env) - return subprocess.run( - args, - check=check, - env=env, - input=input, - # Hope nobody is using NixOS with non-UTF8 encodings, but "surrogateescape" - # should still work in those systems. - text=True, - errors="surrogateescape", - **kwargs, - ) + try: + return subprocess.run( + args, + check=check, + env=env, + input=input, + # Hope nobody is using NixOS with non-UTF8 encodings, but "surrogateescape" + # should still work in those systems. + text=True, + errors="surrogateescape", + **kwargs, + ) + except subprocess.CalledProcessError as ex: + if sudo and remote: + logger.error( + "while running command with remote sudo, did you forget to use " + + "--ask-sudo-password?" + ) + raise ex From 7a01349f792840731bbc6e410c118b857dd01e87 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 13:28:46 +0000 Subject: [PATCH 07/41] nixos-rebuild-ng: add TRY201 check for ruff --- .../by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 2 +- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 4 ++-- pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml | 2 ++ 3 files changed, 5 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 51f2c79e7702..90572f51b1db 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 @@ -323,6 +323,6 @@ def main() -> None: execute(sys.argv) except (Exception, KeyboardInterrupt) as ex: if logger.level == logging.DEBUG: - raise ex + raise else: sys.exit(str(ex)) 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 a64905d7d3bc..2770a5a7d969 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 @@ -114,10 +114,10 @@ def run_wrapper( errors="surrogateescape", **kwargs, ) - except subprocess.CalledProcessError as ex: + except subprocess.CalledProcessError: if sudo and remote: logger.error( "while running command with remote sudo, did you forget to use " + "--ask-sudo-password?" ) - raise ex + raise diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml b/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml index 0dc662814c33..d773e8888923 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/pyproject.toml @@ -43,6 +43,8 @@ extend-select = [ "I", # require `check` argument for `subprocess.run` "PLW1510", + # check for needless exception names in raise statements + "TRY201", ] [tool.ruff.lint.per-file-ignores] From 3a0c0975a8b623129c7d1132934e4f3e35f1ce67 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 15:23:23 +0000 Subject: [PATCH 08/41] nixos-rebuild-ng: remove unnecessary "from __future__ import annotations" --- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py | 2 -- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 2 -- pkgs/by-name/ni/nixos-rebuild-ng/src/tests/helpers.py | 2 -- 3 files changed, 6 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py index 235ce9424e3a..ffd8c5bde454 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py @@ -1,5 +1,3 @@ -from __future__ import annotations - import platform import re import subprocess 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 2770a5a7d969..945a1154ee1b 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,5 +1,3 @@ -from __future__ import annotations - import logging import os import subprocess diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/helpers.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/helpers.py index 0474c6671edd..77ddad740865 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/helpers.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/helpers.py @@ -1,5 +1,3 @@ -from __future__ import annotations - from types import ModuleType from typing import Any, Callable From 73567536e149a08acac4204aceae4ff4843a6037 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 15:32:28 +0000 Subject: [PATCH 09/41] nixos-rebuild-ng: add from_host in nix.copy_closure --- .../src/nixos_rebuild/__init__.py | 4 ++-- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 11 ++++++--- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 23 ++++++++++++++++--- 3 files changed, 30 insertions(+), 8 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 90572f51b1db..b53d59f037f7 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 @@ -210,7 +210,7 @@ def execute(argv: list[str]) -> None: no_out_link=True, **build_flags, ) - nix.copy_closure(path_to_config, target_host, **copy_flags) + nix.copy_closure(path_to_config, target_host, None, **copy_flags) nix.set_profile(profile, path_to_config, target_host, sudo=args.sudo) nix.switch_to_configuration( path_to_config, @@ -253,7 +253,7 @@ def execute(argv: list[str]) -> None: nix.switch_to_configuration( path_to_config, action, - target_host, + target_host=None, sudo=args.sudo, specialisation=args.specialisation, ) 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 ab34b0896ffc..3921629edd1e 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 @@ -27,10 +27,14 @@ logger = logging.getLogger(__name__) def copy_closure( closure: Path, - target_host: Remote | None, + to_host: Remote | None, + from_host: Remote | None = None, **copy_flags: Args, ) -> None: - host = target_host + """Copy a nix closure to or from host to localhost. + + Also supports copying a closure from a remote to another remote.""" + host = to_host or from_host if not host: return @@ -38,11 +42,12 @@ def copy_closure( [ "nix-copy-closure", *dict_to_flags(copy_flags), - "--to", + "--to" if to_host else "--from", host.host, closure, ], extra_env={"NIX_SSHOPTS": " ".join(host.opts)}, + remote=from_host if to_host else None, ) 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 e47869867a7e..3d748732775b 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 @@ -18,11 +18,28 @@ def test_copy_closure(mock_run: Any) -> None: n.copy_closure(closure, None) mock_run.assert_not_called() - target_host = m.Remote("user@host", ["--ssh", "opt"], None) + target_host = m.Remote("user@target.host", ["--ssh", "target-opt"], None) + build_host = m.Remote("user@build.host", ["--ssh", "build-opt"], None) + n.copy_closure(closure, target_host) mock_run.assert_called_with( - ["nix-copy-closure", "--to", "user@host", closure], - extra_env={"NIX_SSHOPTS": "--ssh opt"}, + ["nix-copy-closure", "--to", "user@target.host", closure], + extra_env={"NIX_SSHOPTS": "--ssh target-opt"}, + remote=None, + ) + + n.copy_closure(closure, None, build_host) + mock_run.assert_called_with( + ["nix-copy-closure", "--from", "user@build.host", closure], + extra_env={"NIX_SSHOPTS": "--ssh build-opt"}, + remote=None, + ) + + n.copy_closure(closure, target_host, build_host) + mock_run.assert_called_with( + ["nix-copy-closure", "--to", "user@target.host", closure], + extra_env={"NIX_SSHOPTS": "--ssh target-opt"}, + remote=build_host, ) From 02b943d57ff972d21df41acca540e7bee7f0d9f5 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 15:42:27 +0000 Subject: [PATCH 10/41] nixos-rebuild-ng: use find_file in edit --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 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 3921629edd1e..b129a65d1cf3 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 @@ -69,13 +69,7 @@ def edit(flake: Flake | None, **flake_flags: Args) -> None: if flake_flags: raise NRError("'edit' does not support extra Nix flags") nixos_config = Path( - os.getenv("NIXOS_CONFIG") - or run_wrapper( - ["nix-instantiate", "--find-file", "nixos-config"], - stdout=PIPE, - check=False, - ).stdout.strip() - or "/etc/nixos/default.nix" + os.getenv("NIXOS_CONFIG") or find_file("nixos-config") or "/etc/nixos" ) if nixos_config.is_dir(): nixos_config /= "default.nix" @@ -87,7 +81,7 @@ def edit(flake: Flake | None, **flake_flags: Args) -> None: def find_file(file: str, **nix_flags: Args) -> Path | None: - "Find classic Nixpkgs location." + "Find classic Nix file location." r = run_wrapper( ["nix-instantiate", "--find-file", file, *dict_to_flags(nix_flags)], stdout=PIPE, From f7266986d371f814f9f5ab16b228d242aa05e51e Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 16:27:38 +0000 Subject: [PATCH 11/41] nixos-rebuild-ng: implement --build-host --- .../src/nixos_rebuild/__init__.py | 71 ++++++++---- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 66 ++++++++++- .../src/nixos_rebuild/process.py | 7 +- .../nixos-rebuild-ng/src/tests/test_main.py | 99 ++++++++++++++++- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 103 +++++++++++++++++- 5 files changed, 317 insertions(+), 29 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 b53d59f037f7..cff4f4f7c945 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 @@ -87,7 +87,7 @@ def get_parser() -> tuple[argparse.ArgumentParser, dict[str, argparse.ArgumentPa main_parser.add_argument("--ask-sudo-password", action="store_true") main_parser.add_argument("--use-remote-sudo", action="store_true") # deprecated main_parser.add_argument("--no-ssh-tty", action="store_true") # deprecated - # parser.add_argument("--build-host") # TODO + main_parser.add_argument("--build-host") main_parser.add_argument("--target-host") main_parser.add_argument("action", choices=Action.values(), nargs="?") @@ -135,15 +135,11 @@ def parse_args( if args.action == Action.EDIT.value and (args.file or args.attr): parser.error("--file and --attr are not supported with 'edit'") - if args.target_host and args.action not in ( + if (args.target_host or args.build_host) and args.action not in ( Action.SWITCH.value, Action.BOOT.value, - Action.TEST.value, - Action.BUILD.value, - Action.DRY_BUILD.value, - Action.DRY_ACTIVATE.value, ): - parser.error(f"--target-host is not supported with '{args.action}'") + parser.error(f"--target-host/--build-host is not supported with '{args.action}'") if args.flake and (args.file or args.attr): parser.error("--flake cannot be used with --file or --attr") @@ -170,6 +166,7 @@ def execute(argv: list[str]) -> None: atexit.register(cleanup_ssh, tmpdir_path) profile = Profile.from_arg(args.profile_name) + build_host = Remote.from_arg(args.build_host, False, tmpdir_path) target_host = Remote.from_arg(args.target_host, args.ask_sudo_password, tmpdir_path) build_attr = BuildAttr.from_arg(args.attr, args.file) flake = Flake.from_arg(args.flake, target_host) @@ -197,25 +194,55 @@ def execute(argv: list[str]) -> None: path_to_config = nix.rollback(profile, target_host, sudo=args.sudo) else: if flake: - path_to_config = nix.nixos_build_flake( - "toplevel", - flake, - no_link=True, - **flake_build_flags, - ) + if build_host: + path_to_config = nix.nixos_remote_build_flake( + "toplevel", + flake, + build_host, + flake_build_flags=flake_build_flags, + copy_flags=copy_flags, + build_flags=build_flags, + ) + else: + path_to_config = nix.nixos_build_flake( + "toplevel", + flake, + no_link=True, + **flake_build_flags, + ) else: - path_to_config = nix.nixos_build( - "system", - build_attr, - no_out_link=True, - **build_flags, - ) - nix.copy_closure(path_to_config, target_host, None, **copy_flags) - nix.set_profile(profile, path_to_config, target_host, sudo=args.sudo) + if build_host: + path_to_config = nix.nixos_remote_build( + "system", + build_attr, + build_host, + instantiate_flags=common_flags, + copy_flags=copy_flags, + build_flags=build_flags, + ) + else: + path_to_config = nix.nixos_build( + "system", + build_attr, + no_out_link=True, + **build_flags, + ) + nix.copy_closure( + path_to_config, + to_host=target_host, + from_host=None, + **copy_flags, + ) + nix.set_profile( + profile, + path_to_config, + target_host=target_host, + sudo=args.sudo, + ) nix.switch_to_configuration( path_to_config, action, - target_host, + target_host=target_host, sudo=args.sudo, specialisation=args.specialisation, install_bootloader=args.install_bootloader, 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 b129a65d1cf3..2f86bf02faa6 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 @@ -244,7 +244,7 @@ def list_generations(profile: Profile) -> list[GenerationJson]: def nixos_build( attr: str, build_attr: BuildAttr, - **nix_flags: Args, + **build_flags: Args, ) -> Path: """Build NixOS attribute using classic Nix. @@ -255,7 +255,7 @@ def nixos_build( build_attr.path, "--attr", f"{'.'.join(x for x in [build_attr.attr, attr] if x)}", - *dict_to_flags(nix_flags), + *dict_to_flags(build_flags), ] r = run_wrapper(run_args, stdout=PIPE) return Path(r.stdout.strip()) @@ -264,7 +264,7 @@ def nixos_build( def nixos_build_flake( attr: str, flake: Flake, - **flake_flags: Args, + **flake_build_flags: Args, ) -> Path: """Build NixOS attribute using Flakes. @@ -276,12 +276,70 @@ def nixos_build_flake( "build", "--print-out-paths", f"{flake}.config.system.build.{attr}", - *dict_to_flags(flake_flags), + *dict_to_flags(flake_build_flags), ] r = run_wrapper(run_args, stdout=PIPE) return Path(r.stdout.strip()) +def nixos_remote_build( + attr: str, + build_attr: BuildAttr, + build_host: Remote | None, + build_flags: dict[str, Args] | None = None, + instantiate_flags: dict[str, Args] | None = None, + copy_flags: dict[str, Args] | None = None, +) -> Path: + r = run_wrapper( + [ + "nix-instantiate", + "--raw", + build_attr.path, + "--attr", + f"{'.'.join(x for x in [build_attr.attr, attr] if x)}", + *dict_to_flags(instantiate_flags or {}), + ], + stdout=PIPE, + ) + drv = Path(r.stdout.strip()) + copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) + r = run_wrapper( + ["nix-store", "--realise", drv, *dict_to_flags(build_flags or {})], + remote=build_host, + stdout=PIPE, + ) + return Path(r.stdout.strip()) + + +def nixos_remote_build_flake( + attr: str, + flake: Flake, + build_host: Remote, + flake_build_flags: dict[str, Args] | None = None, + copy_flags: dict[str, Args] | None = None, + build_flags: dict[str, Args] | None = None, +) -> Path: + r = run_wrapper( + [ + "nix", + *FLAKE_FLAGS, + "eval", + "--raw", + f"{flake}.config.system.build.{attr}.drvPath", + *dict_to_flags(flake_build_flags or {}), + ], + stdout=PIPE, + ) + drv = Path(r.stdout.strip()) + copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) + r = run_wrapper( + ["nix-store", "--realise", drv, *dict_to_flags(build_flags or {})], + remote=build_host, + stdout=PIPE, + ) + return Path(r.stdout.strip()) + + def repl(attr: str, build_attr: BuildAttr, **nix_flags: Args) -> None: run_args = ["nix", "repl", "--file", build_attr.path] if build_attr.attr: 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 945a1154ee1b..ed51f1e87937 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 @@ -98,7 +98,12 @@ def run_wrapper( if sudo: args = ["sudo", *args] - logger.debug("calling run with args=%s, extra_env=%s", args, extra_env) + logger.debug( + "calling run with args=%s, kwargs=%s, extra_env=%s", + args, + kwargs, + extra_env, + ) try: return subprocess.run( 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 8bb624a44c30..020fbe0a8cd3 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 @@ -218,7 +218,7 @@ def test_execute_nix_switch_flake(mock_run: Any, tmp_path: Path) -> None: @patch.dict(nr.process.os.environ, {}, clear=True) @patch(get_qualified_name(nr.process.subprocess.run), autospec=True) @patch(get_qualified_name(nr.TemporaryDirectory, nr)) # can't autospec -def test_execute_nix_switch_flake_remote( +def test_execute_nix_switch_flake_target_host( mock_tmpdir: Any, mock_run: Any, tmp_path: Path, @@ -316,6 +316,103 @@ def test_execute_nix_switch_flake_remote( ) +@patch.dict(nr.process.os.environ, {}, clear=True) +@patch(get_qualified_name(nr.process.subprocess.run), autospec=True) +@patch(get_qualified_name(nr.TemporaryDirectory, nr)) # can't autospec +def test_execute_nix_switch_flake_build_host( + mock_tmpdir: Any, + 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)), + CompletedProcess([], 0), + CompletedProcess([], 0, str(config_path)), + # set_profile + CompletedProcess([], 0), + # copy_closure + CompletedProcess([], 0), + # switch_to_configuration + CompletedProcess([], 0), + ] + mock_tmpdir.return_value.name = "/tmp/test" + + nr.execute( + [ + "nixos-rebuild", + "switch", + "--flake", + "/path/to/config#hostname", + "--use-remote-sudo", + "--build-host", + "user@localhost", + ] + ) + + assert mock_run.call_count == 5 + mock_run.assert_has_calls( + [ + call( + [ + "nix", + "--extra-experimental-features", + "nix-command flakes", + "eval", + "--raw", + "/path/to/config#nixosConfigurations.hostname.config.system.build.toplevel.drvPath", + ], + check=True, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ), + call( + ["nix-copy-closure", "--to", "user@localhost", config_path], + check=True, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "ssh", + "-o", + "ControlMaster=auto", + "-o", + "ControlPath=/tmp/test/ssh-%n", + "-o", + "ControlPersist=60", + "user@localhost", + "--", + "nix-store", + "--realise", + config_path, + ], + check=True, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ), + call( + [ + "sudo", + "nix-env", + "-p", + Path("/nix/var/nix/profiles/system"), + "--set", + config_path, + ], + check=True, + **DEFAULT_RUN_KWARGS, + ), + call( + ["sudo", config_path / "bin/switch-to-configuration", "switch"], + check=True, + **DEFAULT_RUN_KWARGS, + ), + ] + ) + + @patch(get_qualified_name(nr.process.subprocess.run), autospec=True) def test_execute_switch_rollback(mock_run: Any, tmp_path: Path) -> None: nixpkgs_path = tmp_path / "nixpkgs" 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 3d748732775b..c10aa7c28c5f 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 @@ -254,13 +254,114 @@ def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: stdout=PIPE, ) - n.nixos_build("attr", m.BuildAttr(Path("file"), "preAttr")) + assert n.nixos_build("attr", m.BuildAttr(Path("file"), "preAttr")) == Path( + "/path/to/file" + ) mock_run.assert_called_with( ["nix-build", Path("file"), "--attr", "preAttr.attr"], stdout=PIPE, ) +@patch( + get_qualified_name(n.run_wrapper, n), + autospec=True, + return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), +) +def test_nixos_remote_build_flake(mock_run: Any) -> None: + flake = m.Flake.parse(".#hostname") + build_host = m.Remote("user@host", ["--ssh", "opts"], None) + + assert n.nixos_remote_build_flake( + "toplevel", + flake, + build_host, + flake_build_flags={"flake": True}, + copy_flags={"copy": True}, + build_flags={"build": True}, + ) == Path("/path/to/file") + mock_run.assert_has_calls( + [ + call( + [ + "nix", + "--extra-experimental-features", + "nix-command flakes", + "eval", + "--raw", + ".#nixosConfigurations.hostname.config.system.build.toplevel.drvPath", + "--flake", + ], + stdout=PIPE, + ), + call( + [ + "nix-copy-closure", + "--copy", + "--to", + "user@host", + Path("/path/to/file"), + ], + extra_env={"NIX_SSHOPTS": "--ssh opts"}, + remote=None, + ), + call( + ["nix-store", "--realise", Path("/path/to/file"), "--build"], + remote=build_host, + stdout=PIPE, + ), + ] + ) + + +@patch( + get_qualified_name(n.run_wrapper, n), + autospec=True, + return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), +) +def test_nixos_remote_build(mock_run: Any, monkeypatch: Any) -> None: + build_host = m.Remote("user@host", ["--ssh", "opts"], None) + assert n.nixos_remote_build( + "attr", + m.BuildAttr("", None), + build_host, + build_flags={"build": True}, + instantiate_flags={"inst": True}, + copy_flags={"copy": True}, + ) == Path("/path/to/file") + mock_run.assert_has_calls( + [ + call( + [ + "nix-instantiate", + "--raw", + "", + "--attr", + "attr", + "--inst", + ], + stdout=PIPE, + ), + call( + [ + "nix-copy-closure", + "--copy", + "--to", + "user@host", + Path("/path/to/file"), + ], + extra_env={"NIX_SSHOPTS": "--ssh opts"}, + remote=None, + ), + call( + ["nix-store", "--realise", Path("/path/to/file"), "--build"], + remote=build_host, + stdout=PIPE, + ), + ] + ) + + @patch(get_qualified_name(n.run_wrapper, n), autospec=True) def test_repl(mock_run: Any) -> None: n.repl("attr", m.BuildAttr("", None), nix_flag=True) From 8a4105cfd7a6f58bf88a5f9e71dd1754f0996b36 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 18:08:43 +0000 Subject: [PATCH 12/41] nixos-rebuild-ng: update README.md --- pkgs/by-name/ni/nixos-rebuild-ng/README.md | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/README.md b/pkgs/by-name/ni/nixos-rebuild-ng/README.md index f34c3d4996ef..c52d1c06d920 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/README.md +++ b/pkgs/by-name/ni/nixos-rebuild-ng/README.md @@ -97,18 +97,8 @@ ruff format . version (the one that will be switched) and re-exec to it instead. This means that in case of bugs in `nixos-rebuild-ng`, the only way that you will get them fixed is **after** you switch to a new version -- `nix` bootstrap is also **not** implemented yet, so this means that you will - eval with an old version of Nix instead of a newer one. This is unlikely to - cause issues, because the build will happen in the daemon anyway (that is - only changed after the switch), and unless you are using bleeding edge `nix` - features you will probably have zero problems here. You can basically think - that using `nixos-rebuild-ng` is similar to running `nixos-rebuild --fast` - right now - Ignore any performance advantages of the rewrite right now, because of the 2 caveats above -- `--target-host` and `--build-host` are not implemented yet and this is - probably the thing that will be most difficult to implement. Help here is - welcome - Bugs in the profile manipulation can cause corruption of your profile that may be difficult to fix, so right now I only recommend using `nixos-rebuild-ng` if you are testing in a VM or in a filesystem with @@ -118,7 +108,7 @@ ruff format . ## TODO -- [ ] Remote host/builders (via SSH) +- [x] Remote host/builders (via SSH) - [x] Improve nix arguments handling (e.g.: `nixFlags` vs `copyFlags` in the old `nixos-rebuild`) - [ ] `_NIXOS_REBUILD_EXEC` @@ -127,11 +117,20 @@ ruff format . `system.switch.enableNg` for `switch-to-configuration-ng` - [ ] Improve documentation - [x] `nixos-rebuild repl` -- [ ] `nix` build/bootstrap - [ ] Generate tab completion via [`shtab`](https://docs.iterative.ai/shtab/) - [x] Reduce build closure ## TODON'T -- Reimplement `systemd-run` logic (will be moved to the new - [`apply`](https://github.com/NixOS/nixpkgs/pull/344407) script) +- Reimplement `systemd-run` logic: will be moved to the new + [`apply`](https://github.com/NixOS/nixpkgs/pull/344407) script +- Nix bootstrap: it is only used for non-Flake paths and it is basically + useless nowadays. It was created at a time when Nix was changing frequently + and there was a need to bootstrap a new version of Nix before evaluating the + configuration (otherwise the new Nixpkgs version may have code that is only + compatible with a newer version of Nix). Nixpkgs now has a policy to be + compatible with Nix 2.3, and even if this is bumped as long we don't do + drastic minimum version changes this should not be an issue. Also, the daemon + itself always run with the previous version since even we can replace Nix in + `PATH` (so Nix client), but we can't replace the daemon without switching to + a new version. From 34cda442082f6ac50ef3196149dd5e8343789e59 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 19:57:03 +0000 Subject: [PATCH 13/41] nixos-rebuild-ng: use `nix build` for remote builds in Flakes, fix remote args --- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 9 +++++++- .../src/nixos_rebuild/process.py | 12 ++++++++++- .../nixos-rebuild-ng/src/tests/test_main.py | 21 ++++--------------- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 10 ++++++++- .../src/tests/test_process.py | 13 +++--------- 5 files changed, 35 insertions(+), 30 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 2f86bf02faa6..11d13502e13c 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 @@ -333,7 +333,14 @@ def nixos_remote_build_flake( drv = Path(r.stdout.strip()) copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) r = run_wrapper( - ["nix-store", "--realise", drv, *dict_to_flags(build_flags or {})], + [ + "nix", + *FLAKE_FLAGS, + "build", + f"{drv}^*", + "--print-out-paths", + *dict_to_flags(build_flags or {}), + ], remote=build_host, stdout=PIPE, ) 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 ed51f1e87937..05d2840529a1 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,5 +1,6 @@ import logging import os +import shlex import subprocess from dataclasses import dataclass from getpass import getpass @@ -91,7 +92,16 @@ def run_wrapper( input = remote.sudo_password + "\n" else: args = ["sudo", *args] - args = ["ssh", *remote.opts, remote.host, "--", *args] + args = [ + "ssh", + *remote.opts, + remote.host, + "--", + # sadly SSH just join all remaining parameters, expanding glob and + # ignoring quotes + # so we need to use shlex.join() to safely join the arguments + shlex.join(str(a) for a in args), + ] else: if extra_env: env = os.environ | extra_env 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 020fbe0a8cd3..70fc0e7045ea 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 @@ -282,12 +282,7 @@ def test_execute_nix_switch_flake_target_host( "ControlPersist=60", "user@localhost", "--", - "sudo", - "nix-env", - "-p", - Path("/nix/var/nix/profiles/system"), - "--set", - config_path, + f"sudo nix-env -p /nix/var/nix/profiles/system --set {config_path}", ], check=True, **DEFAULT_RUN_KWARGS, @@ -303,11 +298,7 @@ def test_execute_nix_switch_flake_target_host( "ControlPersist=60", "user@localhost", "--", - "sudo", - "env", - "NIXOS_INSTALL_BOOTLOADER=0", - config_path / "bin/switch-to-configuration", - "switch", + f"sudo env NIXOS_INSTALL_BOOTLOADER=0 {config_path / 'bin/switch-to-configuration'} switch", ], check=True, **DEFAULT_RUN_KWARGS, @@ -346,7 +337,6 @@ def test_execute_nix_switch_flake_build_host( "switch", "--flake", "/path/to/config#hostname", - "--use-remote-sudo", "--build-host", "user@localhost", ] @@ -384,9 +374,7 @@ def test_execute_nix_switch_flake_build_host( "ControlPersist=60", "user@localhost", "--", - "nix-store", - "--realise", - config_path, + f"nix --extra-experimental-features 'nix-command flakes' build '{config_path}^*' --print-out-paths", ], check=True, stdout=PIPE, @@ -394,7 +382,6 @@ def test_execute_nix_switch_flake_build_host( ), call( [ - "sudo", "nix-env", "-p", Path("/nix/var/nix/profiles/system"), @@ -405,7 +392,7 @@ def test_execute_nix_switch_flake_build_host( **DEFAULT_RUN_KWARGS, ), call( - ["sudo", config_path / "bin/switch-to-configuration", "switch"], + [config_path / "bin/switch-to-configuration", "switch"], check=True, **DEFAULT_RUN_KWARGS, ), 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 c10aa7c28c5f..5a5ee01cb974 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 @@ -306,7 +306,15 @@ def test_nixos_remote_build_flake(mock_run: Any) -> None: remote=None, ), call( - ["nix-store", "--realise", Path("/path/to/file"), "--build"], + [ + "nix", + "--extra-experimental-features", + "nix-command flakes", + "build", + "/path/to/file^*", + "--print-out-paths", + "--build", + ], remote=build_host, stdout=PIPE, ), diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py index 6987aac9283b..57d3865bcef4 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py @@ -40,12 +40,12 @@ def test_run(mock_run: Any) -> None: ) p.run_wrapper( - ["test", "--with", "flags"], + ["test", "--with", "some flags"], check=True, remote=m.Remote("user@localhost", ["--ssh", "opt"], "password"), ) mock_run.assert_called_with( - ["ssh", "--ssh", "opt", "user@localhost", "--", "test", "--with", "flags"], + ["ssh", "--ssh", "opt", "user@localhost", "--", "test --with 'some flags'"], check=True, text=True, errors="surrogateescape", @@ -67,14 +67,7 @@ def test_run(mock_run: Any) -> None: "opt", "user@localhost", "--", - "sudo", - "--prompt=", - "--stdin", - "env", - "FOO=bar", - "test", - "--with", - "flags", + "sudo --prompt= --stdin env FOO=bar test --with flags", ], check=True, env=None, From 7d58c66881eba7fdc4701664b6393c4bf035d585 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 20:02:53 +0000 Subject: [PATCH 14/41] nixos-rebuild-ng: fix --build-host --- .../nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 2 +- .../ni/nixos-rebuild-ng/src/tests/test_main.py | 12 +++++++++++- 2 files changed, 12 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 cff4f4f7c945..ad4b757878dd 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 @@ -230,7 +230,7 @@ def execute(argv: list[str]) -> None: nix.copy_closure( path_to_config, to_host=target_host, - from_host=None, + from_host=build_host, **copy_flags, ) nix.set_profile( 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 70fc0e7045ea..eeecf9445978 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 @@ -342,7 +342,7 @@ def test_execute_nix_switch_flake_build_host( ] ) - assert mock_run.call_count == 5 + assert mock_run.call_count == 6 mock_run.assert_has_calls( [ call( @@ -380,6 +380,16 @@ def test_execute_nix_switch_flake_build_host( stdout=PIPE, **DEFAULT_RUN_KWARGS, ), + call( + [ + "nix-copy-closure", + "--from", + "user@localhost", + config_path, + ], + check=True, + **DEFAULT_RUN_KWARGS, + ), call( [ "nix-env", From 2ac1f78a110332353d1e5b6020c608830d4358bf Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Thu, 28 Nov 2024 20:07:35 +0000 Subject: [PATCH 15/41] nixos-rebuild-ng: validate NIX_SSHOPTS only once --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 8 ++++++-- .../ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 4 +++- 2 files changed, 9 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 ad4b757878dd..99391ecf0c98 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 @@ -139,7 +139,9 @@ def parse_args( Action.SWITCH.value, Action.BOOT.value, ): - parser.error(f"--target-host/--build-host is not supported with '{args.action}'") + parser.error( + f"--target-host/--build-host is not supported with '{args.action}'" + ) if args.flake and (args.file or args.attr): parser.error("--flake cannot be used with --file or --attr") @@ -166,7 +168,9 @@ def execute(argv: list[str]) -> None: atexit.register(cleanup_ssh, tmpdir_path) profile = Profile.from_arg(args.profile_name) - build_host = Remote.from_arg(args.build_host, False, tmpdir_path) + build_host = Remote.from_arg( + args.build_host, False, tmpdir_path, validate_opts=False + ) target_host = Remote.from_arg(args.target_host, args.ask_sudo_password, tmpdir_path) build_attr = BuildAttr.from_arg(args.attr, args.file) flake = Flake.from_arg(args.flake, target_host) 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 05d2840529a1..6fa5be980c16 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 @@ -22,12 +22,14 @@ class Remote: host: str | None, ask_sudo_password: bool | None, tmp_dir: Path, + validate_opts: bool = True, ) -> Self | None: if not host: return None opts = os.getenv("NIX_SSHOPTS", "").split() - cls._validate_opts(opts, ask_sudo_password) + if validate_opts: + cls._validate_opts(opts, ask_sudo_password) opts += [ # SSH ControlMaster flags, allow for faster re-connection "-o", From 29e9b42022b6cf20b2ca7bc0b6b841c51428fc96 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 10:14:43 +0000 Subject: [PATCH 16/41] nixos-rebuild-ng: fix --build-host and --target-host case --- .../by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py | 9 ++++++++- pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py | 5 +++-- 2 files changed, 11 insertions(+), 3 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 11d13502e13c..317c04467a5d 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 @@ -46,7 +46,14 @@ def copy_closure( host.host, closure, ], - extra_env={"NIX_SSHOPTS": " ".join(host.opts)}, + extra_env={ + # for the remote to remote case, we can't use host.opts because it + # includes the ControlPane opts that will not work in the remote, + # because the temporary directory that we created will not exist + "NIX_SSHOPTS": os.environ.get("NIX_SSHOPTS", "") + if from_host and to_host + else " ".join(host.opts) + }, remote=from_host if to_host else None, ) 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 5a5ee01cb974..712bd09b8704 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 @@ -13,7 +13,7 @@ from .helpers import get_qualified_name @patch(get_qualified_name(n.run_wrapper, n), autospec=True) -def test_copy_closure(mock_run: Any) -> None: +def test_copy_closure(mock_run: Any, monkeypatch: Any) -> None: closure = Path("/path/to/closure") n.copy_closure(closure, None) mock_run.assert_not_called() @@ -35,11 +35,12 @@ def test_copy_closure(mock_run: Any) -> None: remote=None, ) + monkeypatch.setenv("NIX_SSHOPTS", "--ssh build-target-opt") n.copy_closure(closure, target_host, build_host) mock_run.assert_called_with( ["nix-copy-closure", "--to", "user@target.host", closure], - extra_env={"NIX_SSHOPTS": "--ssh target-opt"}, remote=build_host, + extra_env={"NIX_SSHOPTS": "--ssh build-target-opt"}, ) From c859df048f77690fdf8e1fab5c45513423596af8 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 11:07:44 +0000 Subject: [PATCH 17/41] nixos-rebuild-ng: refactor classic Nix to simplify logic --- .../src/nixos_rebuild/__init__.py | 6 +++--- .../ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py | 4 ++-- .../ni/nixos-rebuild-ng/src/tests/test_main.py | 2 +- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 15 +++++++++++---- 4 files changed, 17 insertions(+), 10 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 99391ecf0c98..0ee3c6db0fd7 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 @@ -217,7 +217,7 @@ def execute(argv: list[str]) -> None: else: if build_host: path_to_config = nix.nixos_remote_build( - "system", + "toplevel", build_attr, build_host, instantiate_flags=common_flags, @@ -226,7 +226,7 @@ def execute(argv: list[str]) -> None: ) else: path_to_config = nix.nixos_build( - "system", + "toplevel", build_attr, no_out_link=True, **build_flags, @@ -275,7 +275,7 @@ def execute(argv: list[str]) -> None: ) else: path_to_config = nix.nixos_build( - "system", + "toplevel", build_attr, dry_run=dry_run, **build_flags, 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 317c04467a5d..a48dc18d9bf2 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 @@ -261,7 +261,7 @@ def nixos_build( "nix-build", build_attr.path, "--attr", - f"{'.'.join(x for x in [build_attr.attr, attr] if x)}", + f"{build_attr.attr + '.' if build_attr.attr else ''}config.system.build.{attr}", *dict_to_flags(build_flags), ] r = run_wrapper(run_args, stdout=PIPE) @@ -303,7 +303,7 @@ def nixos_remote_build( "--raw", build_attr.path, "--attr", - f"{'.'.join(x for x in [build_attr.attr, attr] if x)}", + f"{build_attr.attr + '.' if build_attr.attr else ''}config.system.build.{attr}", *dict_to_flags(instantiate_flags or {}), ], stdout=PIPE, 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 eeecf9445978..b5c93a2dd217 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 @@ -122,7 +122,7 @@ def test_execute_nix_boot(mock_run: Any, tmp_path: Path) -> None: "nix-build", "", "--attr", - "system", + "config.system.build.toplevel", "--no-out-link", "-vvv", ], 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 712bd09b8704..dce98cf6b939 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 @@ -251,7 +251,14 @@ def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: "attr", m.BuildAttr("", None), nix_flag="foo" ) == Path("/path/to/file") mock_run.assert_called_with( - ["nix-build", "", "--attr", "attr", "--nix-flag", "foo"], + [ + "nix-build", + "", + "--attr", + "config.system.build.attr", + "--nix-flag", + "foo", + ], stdout=PIPE, ) @@ -259,7 +266,7 @@ def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: "/path/to/file" ) mock_run.assert_called_with( - ["nix-build", Path("file"), "--attr", "preAttr.attr"], + ["nix-build", Path("file"), "--attr", "preAttr.config.system.build.attr"], stdout=PIPE, ) @@ -332,7 +339,7 @@ def test_nixos_remote_build(mock_run: Any, monkeypatch: Any) -> None: build_host = m.Remote("user@host", ["--ssh", "opts"], None) assert n.nixos_remote_build( "attr", - m.BuildAttr("", None), + m.BuildAttr("", "preAttr"), build_host, build_flags={"build": True}, instantiate_flags={"inst": True}, @@ -346,7 +353,7 @@ def test_nixos_remote_build(mock_run: Any, monkeypatch: Any) -> None: "--raw", "", "--attr", - "attr", + "preAttr.config.system.build.attr", "--inst", ], stdout=PIPE, From 359d3415359066c2200013cde0ee672c8ac7a569 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 11:22:00 +0000 Subject: [PATCH 18/41] nixos-rebuild-ng: add {BuildAttr,Flake}.to_attr() --- .../src/nixos_rebuild/models.py | 6 ++++++ .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 8 ++++---- .../nixos-rebuild-ng/src/tests/test_models.py | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py index ffd8c5bde454..47ce0f5ca91d 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/models.py @@ -48,6 +48,9 @@ class BuildAttr: path: str | Path attr: str | None + def to_attr(self, *attrs: str) -> str: + return f"{self.attr + '.' if self.attr else ''}{".".join(attrs)}" + @classmethod def from_arg(cls, attr: str | None, file: str | None) -> Self: if not (attr or file): @@ -61,6 +64,9 @@ class Flake: attr: str _re: ClassVar = re.compile(r"^(?P[^\#]*)\#?(?P[^\#\"]*)$") + def to_attr(self, *attrs: str) -> str: + return f"{self}.{".".join(attrs)}" + @override def __str__(self) -> str: return f"{self.path}#{self.attr}" 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 a48dc18d9bf2..7a42645d904b 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 @@ -261,7 +261,7 @@ def nixos_build( "nix-build", build_attr.path, "--attr", - f"{build_attr.attr + '.' if build_attr.attr else ''}config.system.build.{attr}", + build_attr.to_attr("config", "system", "build", attr), *dict_to_flags(build_flags), ] r = run_wrapper(run_args, stdout=PIPE) @@ -282,7 +282,7 @@ def nixos_build_flake( *FLAKE_FLAGS, "build", "--print-out-paths", - f"{flake}.config.system.build.{attr}", + flake.to_attr("config", "system", "build", attr), *dict_to_flags(flake_build_flags), ] r = run_wrapper(run_args, stdout=PIPE) @@ -303,7 +303,7 @@ def nixos_remote_build( "--raw", build_attr.path, "--attr", - f"{build_attr.attr + '.' if build_attr.attr else ''}config.system.build.{attr}", + build_attr.to_attr("config", "system", "build", attr), *dict_to_flags(instantiate_flags or {}), ], stdout=PIPE, @@ -332,7 +332,7 @@ def nixos_remote_build_flake( *FLAKE_FLAGS, "eval", "--raw", - f"{flake}.config.system.build.{attr}.drvPath", + flake.to_attr("config", "system", "build", attr, "drvPath"), *dict_to_flags(flake_build_flags or {}), ], stdout=PIPE, diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py index baeb5977b84f..4da2d6ed7356 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_models.py @@ -20,6 +20,16 @@ def test_build_attr_from_arg() -> None: assert m.BuildAttr.from_arg(None, "file.nix") == m.BuildAttr(Path("file.nix"), None) +def test_build_attr_to_attr() -> None: + assert ( + m.BuildAttr("", None).to_attr("attr1", "attr2") == "attr1.attr2" + ) + assert ( + m.BuildAttr("", "preAttr").to_attr("attr1", "attr2") + == "preAttr.attr1.attr2" + ) + + def test_flake_parse() -> None: assert m.Flake.parse("/path/to/flake#attr") == m.Flake( Path("/path/to/flake"), "nixosConfigurations.attr" @@ -35,6 +45,15 @@ def test_flake_parse() -> None: assert m.Flake.parse(".") == m.Flake(Path("."), "nixosConfigurations.default") +def test_flake_to_attr() -> None: + assert ( + m.Flake(Path("/path/to/flake"), "nixosConfigurations.preAttr").to_attr( + "attr1", "attr2" + ) + == "/path/to/flake#nixosConfigurations.preAttr.attr1.attr2" + ) + + @patch(get_qualified_name(platform.node), autospec=True) def test_flake_from_arg(mock_node: Any) -> None: mock_node.return_value = "hostname" From 3cadcd1653e49a2b9a96ebb4a79bef513cfbe248 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 11:36:39 +0000 Subject: [PATCH 19/41] nixos-rebuild-ng: make build functions more generic --- .../src/nixos_rebuild/__init__.py | 34 +- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 212 +++++------ .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 334 +++++++++--------- 3 files changed, 291 insertions(+), 289 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 0ee3c6db0fd7..ec10db85fcc5 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 @@ -194,13 +194,14 @@ def execute(argv: list[str]) -> None: match action: case Action.SWITCH | Action.BOOT: logger.info("building the system configuration...") + attr = "config.system.build.toplevel" if args.rollback: path_to_config = nix.rollback(profile, target_host, sudo=args.sudo) else: if flake: if build_host: - path_to_config = nix.nixos_remote_build_flake( - "toplevel", + path_to_config = nix.remote_build_flake( + attr, flake, build_host, flake_build_flags=flake_build_flags, @@ -208,16 +209,16 @@ def execute(argv: list[str]) -> None: build_flags=build_flags, ) else: - path_to_config = nix.nixos_build_flake( - "toplevel", + path_to_config = nix.build_flake( + attr, flake, no_link=True, **flake_build_flags, ) else: if build_host: - path_to_config = nix.nixos_remote_build( - "toplevel", + path_to_config = nix.remote_build( + attr, build_attr, build_host, instantiate_flags=common_flags, @@ -225,8 +226,8 @@ def execute(argv: list[str]) -> None: build_flags=build_flags, ) else: - path_to_config = nix.nixos_build( - "toplevel", + path_to_config = nix.build( + attr, build_attr, no_out_link=True, **build_flags, @@ -253,6 +254,7 @@ def execute(argv: list[str]) -> None: ) case Action.TEST | Action.BUILD | Action.DRY_BUILD | Action.DRY_ACTIVATE: logger.info("building the system configuration...") + attr = "config.system.build.toplevel" dry_run = action == Action.DRY_BUILD if args.rollback: if action not in (Action.TEST, Action.BUILD): @@ -267,15 +269,15 @@ def execute(argv: list[str]) -> None: else: raise NRError("could not find previous generation") elif flake: - path_to_config = nix.nixos_build_flake( - "toplevel", + path_to_config = nix.build_flake( + attr, flake, dry_run=dry_run, **flake_build_flags, ) else: - path_to_config = nix.nixos_build( - "toplevel", + path_to_config = nix.build( + attr, build_attr, dry_run=dry_run, **build_flags, @@ -292,14 +294,14 @@ def execute(argv: list[str]) -> None: logger.info("building the system configuration...") attr = "vm" if action == Action.BUILD_VM else "vmWithBootLoader" if flake: - path_to_config = nix.nixos_build_flake( - attr, + path_to_config = nix.build_flake( + f"config.system.build.{attr}", flake, **flake_build_flags, ) else: - path_to_config = nix.nixos_build( - attr, + path_to_config = nix.build( + f"config.system.build.{attr}", build_attr, **build_flags, ) 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 7a42645d904b..ccf927a5e59f 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 @@ -25,6 +25,112 @@ FLAKE_REPL_TEMPLATE: Final = "repl.template.nix" logger = logging.getLogger(__name__) +def build( + attr: str, + build_attr: BuildAttr, + **build_flags: Args, +) -> Path: + """Build NixOS attribute using classic Nix. + + Returns the built attribute as path. + """ + run_args = [ + "nix-build", + build_attr.path, + "--attr", + build_attr.to_attr(attr), + *dict_to_flags(build_flags), + ] + r = run_wrapper(run_args, stdout=PIPE) + return Path(r.stdout.strip()) + + +def build_flake( + attr: str, + flake: Flake, + **flake_build_flags: Args, +) -> Path: + """Build NixOS attribute using Flakes. + + Returns the built attribute as path. + """ + run_args = [ + "nix", + *FLAKE_FLAGS, + "build", + "--print-out-paths", + flake.to_attr(attr), + *dict_to_flags(flake_build_flags), + ] + r = run_wrapper(run_args, stdout=PIPE) + return Path(r.stdout.strip()) + + +def remote_build( + attr: str, + build_attr: BuildAttr, + build_host: Remote | None, + build_flags: dict[str, Args] | None = None, + instantiate_flags: dict[str, Args] | None = None, + copy_flags: dict[str, Args] | None = None, +) -> Path: + r = run_wrapper( + [ + "nix-instantiate", + "--raw", + build_attr.path, + "--attr", + build_attr.to_attr(attr), + *dict_to_flags(instantiate_flags or {}), + ], + stdout=PIPE, + ) + drv = Path(r.stdout.strip()) + copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) + r = run_wrapper( + ["nix-store", "--realise", drv, *dict_to_flags(build_flags or {})], + remote=build_host, + stdout=PIPE, + ) + return Path(r.stdout.strip()) + + +def remote_build_flake( + attr: str, + flake: Flake, + build_host: Remote, + flake_build_flags: dict[str, Args] | None = None, + copy_flags: dict[str, Args] | None = None, + build_flags: dict[str, Args] | None = None, +) -> Path: + r = run_wrapper( + [ + "nix", + *FLAKE_FLAGS, + "eval", + "--raw", + flake.to_attr(attr, "drvPath"), + *dict_to_flags(flake_build_flags or {}), + ], + stdout=PIPE, + ) + drv = Path(r.stdout.strip()) + copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) + r = run_wrapper( + [ + "nix", + *FLAKE_FLAGS, + "build", + f"{drv}^*", + "--print-out-paths", + *dict_to_flags(build_flags or {}), + ], + remote=build_host, + stdout=PIPE, + ) + return Path(r.stdout.strip()) + + def copy_closure( closure: Path, to_host: Remote | None, @@ -248,112 +354,6 @@ def list_generations(profile: Profile) -> list[GenerationJson]: return result -def nixos_build( - attr: str, - build_attr: BuildAttr, - **build_flags: Args, -) -> Path: - """Build NixOS attribute using classic Nix. - - Returns the built attribute as path. - """ - run_args = [ - "nix-build", - build_attr.path, - "--attr", - build_attr.to_attr("config", "system", "build", attr), - *dict_to_flags(build_flags), - ] - r = run_wrapper(run_args, stdout=PIPE) - return Path(r.stdout.strip()) - - -def nixos_build_flake( - attr: str, - flake: Flake, - **flake_build_flags: Args, -) -> Path: - """Build NixOS attribute using Flakes. - - Returns the built attribute as path. - """ - run_args = [ - "nix", - *FLAKE_FLAGS, - "build", - "--print-out-paths", - flake.to_attr("config", "system", "build", attr), - *dict_to_flags(flake_build_flags), - ] - r = run_wrapper(run_args, stdout=PIPE) - return Path(r.stdout.strip()) - - -def nixos_remote_build( - attr: str, - build_attr: BuildAttr, - build_host: Remote | None, - build_flags: dict[str, Args] | None = None, - instantiate_flags: dict[str, Args] | None = None, - copy_flags: dict[str, Args] | None = None, -) -> Path: - r = run_wrapper( - [ - "nix-instantiate", - "--raw", - build_attr.path, - "--attr", - build_attr.to_attr("config", "system", "build", attr), - *dict_to_flags(instantiate_flags or {}), - ], - stdout=PIPE, - ) - drv = Path(r.stdout.strip()) - copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) - r = run_wrapper( - ["nix-store", "--realise", drv, *dict_to_flags(build_flags or {})], - remote=build_host, - stdout=PIPE, - ) - return Path(r.stdout.strip()) - - -def nixos_remote_build_flake( - attr: str, - flake: Flake, - build_host: Remote, - flake_build_flags: dict[str, Args] | None = None, - copy_flags: dict[str, Args] | None = None, - build_flags: dict[str, Args] | None = None, -) -> Path: - r = run_wrapper( - [ - "nix", - *FLAKE_FLAGS, - "eval", - "--raw", - flake.to_attr("config", "system", "build", attr, "drvPath"), - *dict_to_flags(flake_build_flags or {}), - ], - stdout=PIPE, - ) - drv = Path(r.stdout.strip()) - copy_closure(drv, to_host=build_host, from_host=None, **(copy_flags or {})) - r = run_wrapper( - [ - "nix", - *FLAKE_FLAGS, - "build", - f"{drv}^*", - "--print-out-paths", - *dict_to_flags(build_flags or {}), - ], - remote=build_host, - stdout=PIPE, - ) - return Path(r.stdout.strip()) - - def repl(attr: str, build_attr: BuildAttr, **nix_flags: Args) -> None: run_args = ["nix", "repl", "--file", build_attr.path] if build_attr.attr: 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 dce98cf6b939..317821d5fea0 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 @@ -12,6 +12,173 @@ import nixos_rebuild.nix as n from .helpers import get_qualified_name +@patch( + get_qualified_name(n.run_wrapper, n), + autospec=True, + return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), +) +def test_build(mock_run: Any, monkeypatch: Any) -> None: + assert n.build( + "config.system.build.attr", m.BuildAttr("", None), nix_flag="foo" + ) == Path("/path/to/file") + mock_run.assert_called_with( + [ + "nix-build", + "", + "--attr", + "config.system.build.attr", + "--nix-flag", + "foo", + ], + stdout=PIPE, + ) + + assert n.build( + "config.system.build.attr", m.BuildAttr(Path("file"), "preAttr") + ) == Path("/path/to/file") + mock_run.assert_called_with( + ["nix-build", Path("file"), "--attr", "preAttr.config.system.build.attr"], + stdout=PIPE, + ) + + +@patch( + get_qualified_name(n.run_wrapper, n), + autospec=True, + return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), +) +def test_build_flake(mock_run: Any) -> None: + flake = m.Flake.parse(".#hostname") + + assert n.build_flake( + "config.system.build.toplevel", + flake, + no_link=True, + nix_flag="foo", + ) == Path("/path/to/file") + mock_run.assert_called_with( + [ + "nix", + "--extra-experimental-features", + "nix-command flakes", + "build", + "--print-out-paths", + ".#nixosConfigurations.hostname.config.system.build.toplevel", + "--no-link", + "--nix-flag", + "foo", + ], + stdout=PIPE, + ) + + +@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: + build_host = m.Remote("user@host", ["--ssh", "opts"], None) + assert n.remote_build( + "config.system.build.toplevel", + m.BuildAttr("", "preAttr"), + build_host, + build_flags={"build": True}, + instantiate_flags={"inst": True}, + copy_flags={"copy": True}, + ) == Path("/path/to/file") + mock_run.assert_has_calls( + [ + call( + [ + "nix-instantiate", + "--raw", + "", + "--attr", + "preAttr.config.system.build.toplevel", + "--inst", + ], + stdout=PIPE, + ), + call( + [ + "nix-copy-closure", + "--copy", + "--to", + "user@host", + Path("/path/to/file"), + ], + extra_env={"NIX_SSHOPTS": "--ssh opts"}, + remote=None, + ), + call( + ["nix-store", "--realise", Path("/path/to/file"), "--build"], + remote=build_host, + stdout=PIPE, + ), + ] + ) + + +@patch( + get_qualified_name(n.run_wrapper, n), + autospec=True, + return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), +) +def test_remote_build_flake(mock_run: Any) -> None: + flake = m.Flake.parse(".#hostname") + build_host = m.Remote("user@host", ["--ssh", "opts"], None) + + assert n.remote_build_flake( + "config.system.build.toplevel", + flake, + build_host, + flake_build_flags={"flake": True}, + copy_flags={"copy": True}, + build_flags={"build": True}, + ) == Path("/path/to/file") + mock_run.assert_has_calls( + [ + call( + [ + "nix", + "--extra-experimental-features", + "nix-command flakes", + "eval", + "--raw", + ".#nixosConfigurations.hostname.config.system.build.toplevel.drvPath", + "--flake", + ], + stdout=PIPE, + ), + call( + [ + "nix-copy-closure", + "--copy", + "--to", + "user@host", + Path("/path/to/file"), + ], + extra_env={"NIX_SSHOPTS": "--ssh opts"}, + remote=None, + ), + call( + [ + "nix", + "--extra-experimental-features", + "nix-command flakes", + "build", + "/path/to/file^*", + "--print-out-paths", + "--build", + ], + remote=build_host, + stdout=PIPE, + ), + ] + ) + + @patch(get_qualified_name(n.run_wrapper, n), autospec=True) def test_copy_closure(mock_run: Any, monkeypatch: Any) -> None: closure = Path("/path/to/closure") @@ -211,173 +378,6 @@ def test_list_generations(mock_get_generations: Any, tmp_path: Path) -> None: ] -@patch( - get_qualified_name(n.run_wrapper, n), - autospec=True, - return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), -) -def test_nixos_build_flake(mock_run: Any) -> None: - flake = m.Flake.parse(".#hostname") - - assert n.nixos_build_flake( - "toplevel", - flake, - no_link=True, - nix_flag="foo", - ) == Path("/path/to/file") - mock_run.assert_called_with( - [ - "nix", - "--extra-experimental-features", - "nix-command flakes", - "build", - "--print-out-paths", - ".#nixosConfigurations.hostname.config.system.build.toplevel", - "--no-link", - "--nix-flag", - "foo", - ], - stdout=PIPE, - ) - - -@patch( - get_qualified_name(n.run_wrapper, n), - autospec=True, - return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), -) -def test_nixos_build(mock_run: Any, monkeypatch: Any) -> None: - assert n.nixos_build( - "attr", m.BuildAttr("", None), nix_flag="foo" - ) == Path("/path/to/file") - mock_run.assert_called_with( - [ - "nix-build", - "", - "--attr", - "config.system.build.attr", - "--nix-flag", - "foo", - ], - stdout=PIPE, - ) - - assert n.nixos_build("attr", m.BuildAttr(Path("file"), "preAttr")) == Path( - "/path/to/file" - ) - mock_run.assert_called_with( - ["nix-build", Path("file"), "--attr", "preAttr.config.system.build.attr"], - stdout=PIPE, - ) - - -@patch( - get_qualified_name(n.run_wrapper, n), - autospec=True, - return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), -) -def test_nixos_remote_build_flake(mock_run: Any) -> None: - flake = m.Flake.parse(".#hostname") - build_host = m.Remote("user@host", ["--ssh", "opts"], None) - - assert n.nixos_remote_build_flake( - "toplevel", - flake, - build_host, - flake_build_flags={"flake": True}, - copy_flags={"copy": True}, - build_flags={"build": True}, - ) == Path("/path/to/file") - mock_run.assert_has_calls( - [ - call( - [ - "nix", - "--extra-experimental-features", - "nix-command flakes", - "eval", - "--raw", - ".#nixosConfigurations.hostname.config.system.build.toplevel.drvPath", - "--flake", - ], - stdout=PIPE, - ), - call( - [ - "nix-copy-closure", - "--copy", - "--to", - "user@host", - Path("/path/to/file"), - ], - extra_env={"NIX_SSHOPTS": "--ssh opts"}, - remote=None, - ), - call( - [ - "nix", - "--extra-experimental-features", - "nix-command flakes", - "build", - "/path/to/file^*", - "--print-out-paths", - "--build", - ], - remote=build_host, - stdout=PIPE, - ), - ] - ) - - -@patch( - get_qualified_name(n.run_wrapper, n), - autospec=True, - return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), -) -def test_nixos_remote_build(mock_run: Any, monkeypatch: Any) -> None: - build_host = m.Remote("user@host", ["--ssh", "opts"], None) - assert n.nixos_remote_build( - "attr", - m.BuildAttr("", "preAttr"), - build_host, - build_flags={"build": True}, - instantiate_flags={"inst": True}, - copy_flags={"copy": True}, - ) == Path("/path/to/file") - mock_run.assert_has_calls( - [ - call( - [ - "nix-instantiate", - "--raw", - "", - "--attr", - "preAttr.config.system.build.attr", - "--inst", - ], - stdout=PIPE, - ), - call( - [ - "nix-copy-closure", - "--copy", - "--to", - "user@host", - Path("/path/to/file"), - ], - extra_env={"NIX_SSHOPTS": "--ssh opts"}, - remote=None, - ), - call( - ["nix-store", "--realise", Path("/path/to/file"), "--build"], - remote=build_host, - stdout=PIPE, - ), - ] - ) - - @patch(get_qualified_name(n.run_wrapper, n), autospec=True) def test_repl(mock_run: Any) -> None: n.repl("attr", m.BuildAttr("", None), nix_flag=True) From 3fd384af80c6be1c7159acbacb4ccefce3b1c1e6 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 12:49:09 +0000 Subject: [PATCH 20/41] nixos-rebuild-ng: implement _NIXOS_REBUILD_REEXEC --- pkgs/by-name/ni/nixos-rebuild-ng/README.md | 10 +------ .../src/nixos_rebuild/__init__.py | 30 ++++++++++++++++--- .../nixos-rebuild-ng/src/tests/test_main.py | 17 +++++------ 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/README.md b/pkgs/by-name/ni/nixos-rebuild-ng/README.md index c52d1c06d920..9a565de2dd18 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/README.md +++ b/pkgs/by-name/ni/nixos-rebuild-ng/README.md @@ -91,14 +91,6 @@ ruff format . conflicting with the current `nixos-rebuild`. This means you can keep both in your system at the same time, but it also means that a few things like bash completion are broken right now (since it looks at `nixos-rebuild` binary) -- `_NIXOS_REBUILD_EXEC` is **not** implemented yet, so different from - `nixos-rebuild`, this will use the current version of `nixos-rebuild-ng` in - your `PATH` to build/set profile/switch, while `nixos-rebuild` builds the new - version (the one that will be switched) and re-exec to it instead. This means - that in case of bugs in `nixos-rebuild-ng`, the only way that you will get - them fixed is **after** you switch to a new version -- Ignore any performance advantages of the rewrite right now, because of the 2 - caveats above - Bugs in the profile manipulation can cause corruption of your profile that may be difficult to fix, so right now I only recommend using `nixos-rebuild-ng` if you are testing in a VM or in a filesystem with @@ -111,7 +103,7 @@ ruff format . - [x] Remote host/builders (via SSH) - [x] Improve nix arguments handling (e.g.: `nixFlags` vs `copyFlags` in the old `nixos-rebuild`) -- [ ] `_NIXOS_REBUILD_EXEC` +- [x] `_NIXOS_REBUILD_REEXEC` - [ ] Port `nixos-rebuild.passthru.tests` - [ ] Change module system to allow easier opt-in, like `system.switch.enableNg` for `switch-to-configuration-ng` 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 ec10db85fcc5..71e630262608 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 @@ -2,6 +2,7 @@ import argparse import atexit import json import logging +import os import sys from pathlib import Path from subprocess import run @@ -87,6 +88,7 @@ def get_parser() -> tuple[argparse.ArgumentParser, dict[str, argparse.ArgumentPa main_parser.add_argument("--ask-sudo-password", action="store_true") main_parser.add_argument("--use-remote-sudo", action="store_true") # deprecated main_parser.add_argument("--no-ssh-tty", action="store_true") # deprecated + main_parser.add_argument("--fast", action="store_true") main_parser.add_argument("--build-host") main_parser.add_argument("--target-host") main_parser.add_argument("action", choices=Action.values(), nargs="?") @@ -174,10 +176,6 @@ def execute(argv: list[str]) -> None: target_host = Remote.from_arg(args.target_host, args.ask_sudo_password, tmpdir_path) build_attr = BuildAttr.from_arg(args.attr, args.file) flake = Flake.from_arg(args.flake, target_host) - - if args.upgrade or args.upgrade_all: - nix.upgrade_channels(bool(args.upgrade_all)) - action = Action(args.action) # Only run shell scripts from the Nixpkgs tree if the action is # "switch", "boot", or "test". With other actions (such as "build"), @@ -185,6 +183,30 @@ def execute(argv: list[str]) -> None: # executed, so it's safe to run nixos-rebuild against a potentially # untrusted tree. can_run = action in (Action.SWITCH, Action.BOOT, Action.TEST) + + # Re-exec to a newer version of the script before building to ensure we get + # the latest fixes + if can_run and not args.fast and not os.environ.get("_NIXOS_REBUILD_REEXEC"): + if flake: + drv = nix.build_flake("pkgs.nixos-rebuild-ng", flake, **flake_build_flags) + else: + drv = nix.build("pkgs.nixos-rebuild-ng", build_attr, **build_flags) + new = drv / "bin/nixos-rebuild-ng" + current = Path(argv[0]) + # Disable re-exec during development + if current.name != "__main__.py" and new != current: + logging.debug( + "detected newer version of script, re-exec'ing, current=%s, new=%s", + argv[0], + new, + ) + cleanup_ssh(tmpdir_path) + tmpdir.cleanup() + os.execve(new, argv, os.environ | {"_NIXOS_REBUILD_REEXEC": "1"}) + + if args.upgrade or args.upgrade_all: + nix.upgrade_channels(bool(args.upgrade_all)) + if can_run and not flake: nixpkgs_path = nix.find_file("nixpkgs", **build_flags) rev = nix.get_nixpkgs_rev(nixpkgs_path) 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 b5c93a2dd217..d647f78460b3 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 @@ -95,7 +95,7 @@ def test_execute_nix_boot(mock_run: Any, tmp_path: Path) -> None: CompletedProcess([], 0), ] - nr.execute(["nixos-rebuild", "boot", "--no-flake", "-vvv"]) + nr.execute(["nixos-rebuild", "boot", "--no-flake", "-vvv", "--fast"]) assert mock_run.call_count == 6 mock_run.assert_has_calls( @@ -173,6 +173,7 @@ def test_execute_nix_switch_flake(mock_run: Any, tmp_path: Path) -> None: "--install-bootloader", "--sudo", "--verbose", + "--fast", ] ) @@ -246,6 +247,7 @@ def test_execute_nix_switch_flake_target_host( "--use-remote-sudo", "--target-host", "user@localhost", + "--fast", ] ) @@ -339,6 +341,7 @@ def test_execute_nix_switch_flake_build_host( "/path/to/config#hostname", "--build-host", "user@localhost", + "--fast", ] ) @@ -415,7 +418,9 @@ def test_execute_switch_rollback(mock_run: Any, tmp_path: Path) -> None: nixpkgs_path = tmp_path / "nixpkgs" nixpkgs_path.touch() - nr.execute(["nixos-rebuild", "switch", "--rollback", "--install-bootloader"]) + nr.execute( + ["nixos-rebuild", "switch", "--rollback", "--install-bootloader", "--fast"] + ) assert mock_run.call_count >= 2 # ignoring update_nixpkgs_rev calls @@ -467,13 +472,7 @@ def test_execute_test_rollback( ] nr.execute( - [ - "nixos-rebuild", - "test", - "--rollback", - "--profile-name", - "foo", - ] + ["nixos-rebuild", "test", "--rollback", "--profile-name", "foo", "--fast"] ) assert mock_run.call_count == 2 From 776c21be0fc89962e04a0b44dd65ec2ef6d5adc5 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 13:06:51 +0000 Subject: [PATCH 21/41] nixos-rebuild-ng: fix cleanup_ssh --- .../src/nixos_rebuild/process.py | 4 +++- .../src/tests/test_process.py | 24 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 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 6fa5be980c16..1c118d48b850 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 @@ -69,7 +69,9 @@ class RunKwargs(TypedDict, total=False): def cleanup_ssh(tmp_dir: Path) -> None: "Close SSH ControlMaster connection." for ctrl in tmp_dir.glob("ssh-*"): - subprocess.run(["ssh", "-o", f"ControlPath={ctrl}", "exit"], check=False) + subprocess.run( + ["ssh", "-o", f"ControlPath={ctrl}", "-O", "exit", "dummyhost"], check=False + ) def run_wrapper( diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py index 57d3865bcef4..998ad5dedc83 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py @@ -1,6 +1,6 @@ from pathlib import Path from typing import Any -from unittest.mock import patch +from unittest.mock import call, patch import nixos_rebuild.models as m import nixos_rebuild.process as p @@ -8,6 +8,28 @@ import nixos_rebuild.process as p from .helpers import get_qualified_name +@patch(get_qualified_name(p.subprocess.run)) +def test_cleanup_ssh(mock_run: Any, tmp_path: Path) -> None: + (tmp_path / "ssh-conn").touch() + + p.cleanup_ssh(tmp_path) + mock_run.assert_has_calls( + [ + call( + [ + "ssh", + "-o", + f"ControlPath={tmp_path}/ssh-conn", + "-O", + "exit", + "dummyhost", + ], + check=False, + ) + ] + ) + + @patch(get_qualified_name(p.subprocess.run)) def test_run(mock_run: Any) -> None: p.run_wrapper(["test", "--with", "flags"], check=True) From fed6778da3a41b30973b054ada0daf9d2b1d334a Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 20:04:38 +0000 Subject: [PATCH 22/41] nixos-rebuild-ng: move temporary directory to process --- .../src/nixos_rebuild/__init__.py | 15 ++-- .../src/nixos_rebuild/process.py | 28 ++++---- .../nixos-rebuild-ng/src/tests/test_main.py | 31 ++------- .../src/tests/test_process.py | 68 +++++-------------- 4 files changed, 45 insertions(+), 97 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 71e630262608..f0bbabdb66ab 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 @@ -6,7 +6,6 @@ import os import sys from pathlib import Path from subprocess import run -from tempfile import TemporaryDirectory from typing import assert_never from . import nix @@ -164,16 +163,11 @@ def execute(argv: list[str]) -> None: flake_build_flags = common_build_flags | vars(args_groups["flake_build_flags"]) copy_flags = common_flags | vars(args_groups["copy_flags"]) - # Will be cleaned up on exit automatically. - tmpdir = TemporaryDirectory(prefix="nixos-rebuild.") - tmpdir_path = Path(tmpdir.name) - atexit.register(cleanup_ssh, tmpdir_path) + atexit.register(cleanup_ssh) profile = Profile.from_arg(args.profile_name) - build_host = Remote.from_arg( - args.build_host, False, tmpdir_path, validate_opts=False - ) - target_host = Remote.from_arg(args.target_host, args.ask_sudo_password, tmpdir_path) + build_host = Remote.from_arg(args.build_host, False, validate_opts=False) + target_host = Remote.from_arg(args.target_host, args.ask_sudo_password) build_attr = BuildAttr.from_arg(args.attr, args.file) flake = Flake.from_arg(args.flake, target_host) action = Action(args.action) @@ -200,8 +194,7 @@ def execute(argv: list[str]) -> None: argv[0], new, ) - cleanup_ssh(tmpdir_path) - tmpdir.cleanup() + cleanup_ssh() os.execve(new, argv, os.environ | {"_NIXOS_REBUILD_REEXEC": "1"}) if args.upgrade or args.upgrade_all: 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 1c118d48b850..54200f14d85a 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,10 +5,22 @@ 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 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"}", + "-o", + "ControlPersist=60", +] + @dataclass(frozen=True) class Remote: @@ -21,7 +33,6 @@ class Remote: cls, host: str | None, ask_sudo_password: bool | None, - tmp_dir: Path, validate_opts: bool = True, ) -> Self | None: if not host: @@ -30,15 +41,6 @@ class Remote: opts = os.getenv("NIX_SSHOPTS", "").split() if validate_opts: cls._validate_opts(opts, ask_sudo_password) - opts += [ - # SSH ControlMaster flags, allow for faster re-connection - "-o", - "ControlMaster=auto", - "-o", - f"ControlPath={tmp_dir / "ssh-%n"}", - "-o", - "ControlPersist=60", - ] sudo_password = None if ask_sudo_password: sudo_password = getpass(f"[sudo] password for {host}: ") @@ -66,12 +68,13 @@ class RunKwargs(TypedDict, total=False): stdout: int | None -def cleanup_ssh(tmp_dir: Path) -> None: +def cleanup_ssh() -> None: "Close SSH ControlMaster connection." - for ctrl in tmp_dir.glob("ssh-*"): + for ctrl in TMPDIR_PATH.glob("ssh-*"): subprocess.run( ["ssh", "-o", f"ControlPath={ctrl}", "-O", "exit", "dummyhost"], check=False ) + TMPDIR.cleanup() def run_wrapper( @@ -99,6 +102,7 @@ def run_wrapper( args = [ "ssh", *remote.opts, + *SSH_DEFAULT_OPTS, remote.host, "--", # sadly SSH just join all remaining parameters, expanding glob and 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 d647f78460b3..97ae0072a829 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 @@ -218,9 +218,9 @@ def test_execute_nix_switch_flake(mock_run: Any, tmp_path: Path) -> None: @patch.dict(nr.process.os.environ, {}, clear=True) @patch(get_qualified_name(nr.process.subprocess.run), autospec=True) -@patch(get_qualified_name(nr.TemporaryDirectory, nr)) # can't autospec +@patch(get_qualified_name(nr.cleanup_ssh, nr), autospec=True) def test_execute_nix_switch_flake_target_host( - mock_tmpdir: Any, + mock_cleanup_ssh: Any, mock_run: Any, tmp_path: Path, ) -> None: @@ -236,7 +236,6 @@ def test_execute_nix_switch_flake_target_host( # switch_to_configuration CompletedProcess([], 0), ] - mock_tmpdir.return_value.name = "/tmp/test" nr.execute( [ @@ -276,12 +275,7 @@ def test_execute_nix_switch_flake_target_host( call( [ "ssh", - "-o", - "ControlMaster=auto", - "-o", - "ControlPath=/tmp/test/ssh-%n", - "-o", - "ControlPersist=60", + *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", f"sudo nix-env -p /nix/var/nix/profiles/system --set {config_path}", @@ -292,12 +286,7 @@ def test_execute_nix_switch_flake_target_host( call( [ "ssh", - "-o", - "ControlMaster=auto", - "-o", - "ControlPath=/tmp/test/ssh-%n", - "-o", - "ControlPersist=60", + *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", f"sudo env NIXOS_INSTALL_BOOTLOADER=0 {config_path / 'bin/switch-to-configuration'} switch", @@ -311,9 +300,9 @@ def test_execute_nix_switch_flake_target_host( @patch.dict(nr.process.os.environ, {}, clear=True) @patch(get_qualified_name(nr.process.subprocess.run), autospec=True) -@patch(get_qualified_name(nr.TemporaryDirectory, nr)) # can't autospec +@patch(get_qualified_name(nr.cleanup_ssh, nr), autospec=True) def test_execute_nix_switch_flake_build_host( - mock_tmpdir: Any, + mock_cleanup_ssh: Any, mock_run: Any, tmp_path: Path, ) -> None: @@ -331,7 +320,6 @@ def test_execute_nix_switch_flake_build_host( # switch_to_configuration CompletedProcess([], 0), ] - mock_tmpdir.return_value.name = "/tmp/test" nr.execute( [ @@ -369,12 +357,7 @@ def test_execute_nix_switch_flake_build_host( call( [ "ssh", - "-o", - "ControlMaster=auto", - "-o", - "ControlPath=/tmp/test/ssh-%n", - "-o", - "ControlPersist=60", + *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", f"nix --extra-experimental-features 'nix-command flakes' build '{config_path}^*' --print-out-paths", diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py index 998ad5dedc83..53ed6824e39f 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py @@ -1,6 +1,5 @@ -from pathlib import Path from typing import Any -from unittest.mock import call, patch +from unittest.mock import patch import nixos_rebuild.models as m import nixos_rebuild.process as p @@ -8,29 +7,7 @@ import nixos_rebuild.process as p from .helpers import get_qualified_name -@patch(get_qualified_name(p.subprocess.run)) -def test_cleanup_ssh(mock_run: Any, tmp_path: Path) -> None: - (tmp_path / "ssh-conn").touch() - - p.cleanup_ssh(tmp_path) - mock_run.assert_has_calls( - [ - call( - [ - "ssh", - "-o", - f"ControlPath={tmp_path}/ssh-conn", - "-O", - "exit", - "dummyhost", - ], - check=False, - ) - ] - ) - - -@patch(get_qualified_name(p.subprocess.run)) +@patch(get_qualified_name(p.subprocess.run), autospec=True) def test_run(mock_run: Any) -> None: p.run_wrapper(["test", "--with", "flags"], check=True) mock_run.assert_called_with( @@ -67,7 +44,15 @@ def test_run(mock_run: Any) -> None: remote=m.Remote("user@localhost", ["--ssh", "opt"], "password"), ) mock_run.assert_called_with( - ["ssh", "--ssh", "opt", "user@localhost", "--", "test --with 'some flags'"], + [ + "ssh", + "--ssh", + "opt", + *p.SSH_DEFAULT_OPTS, + "user@localhost", + "--", + "test --with 'some flags'", + ], check=True, text=True, errors="surrogateescape", @@ -87,6 +72,7 @@ def test_run(mock_run: Any) -> None: "ssh", "--ssh", "opt", + *p.SSH_DEFAULT_OPTS, "user@localhost", "--", "sudo --prompt= --stdin env FOO=bar test --with flags", @@ -99,38 +85,20 @@ def test_run(mock_run: Any) -> None: ) -def test_remote_from_name(monkeypatch: Any, tmpdir: Path) -> None: +def test_remote_from_name(monkeypatch: Any) -> None: monkeypatch.setenv("NIX_SSHOPTS", "") - assert m.Remote.from_arg("user@localhost", None, tmpdir) == m.Remote( + assert m.Remote.from_arg("user@localhost", None, False) == m.Remote( "user@localhost", - opts=[ - "-o", - "ControlMaster=auto", - "-o", - f"ControlPath={tmpdir / "ssh-%n"}", - "-o", - "ControlPersist=60", - ], + opts=[], sudo_password=None, ) # get_qualified_name doesn't work because getpass is aliased to another # function with patch(f"{p.__name__}.getpass", return_value="password"): - monkeypatch.setenv("NIX_SSHOPTS", "-f foo -b bar") - assert m.Remote.from_arg("user@localhost", True, tmpdir) == m.Remote( + monkeypatch.setenv("NIX_SSHOPTS", "-f foo -b bar -t") + assert m.Remote.from_arg("user@localhost", True, True) == m.Remote( "user@localhost", - opts=[ - "-f", - "foo", - "-b", - "bar", - "-o", - "ControlMaster=auto", - "-o", - f"ControlPath={tmpdir / "ssh-%n"}", - "-o", - "ControlPersist=60", - ], + opts=["-f", "foo", "-b", "bar", "-t"], sudo_password="password", ) From 6c3ba91ce46f66c455aa895e8e249bf5ee2a7a3f Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 20:18:42 +0000 Subject: [PATCH 23/41] nixos-rebuild-ng: rename template file to not trigger CI --- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py | 2 +- .../src/nixos_rebuild/{repl.template.nix => repl.nix.template} | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) rename pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/{repl.template.nix => repl.nix.template} (98%) 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 ccf927a5e59f..5247a2991b1b 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 @@ -21,7 +21,7 @@ from .process import run_wrapper from .utils import Args, dict_to_flags FLAKE_FLAGS: Final = ["--extra-experimental-features", "nix-command flakes"] -FLAKE_REPL_TEMPLATE: Final = "repl.template.nix" +FLAKE_REPL_TEMPLATE: Final = "repl.nix.template" logger = logging.getLogger(__name__) diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.template.nix b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.nix.template similarity index 98% rename from pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.template.nix rename to pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.nix.template index 0fffd6ecc746..17db87423711 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.template.nix +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/repl.nix.template @@ -1,3 +1,4 @@ +# vim: set syntax=nix: let flake = builtins.getFlake ''${flake_path}''; configuration = flake.${flake_attr}; From bebec2668b43941ec833c81c57e519f328c57862 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 20:29:26 +0000 Subject: [PATCH 24/41] nixos-rebuild-ng: add missing flags from nixos-rebuild-ng --- .../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 f0bbabdb66ab..e7124f93001b 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 @@ -53,7 +53,9 @@ def get_parser() -> tuple[argparse.ArgumentParser, dict[str, argparse.ArgumentPa classic_build_flags.add_argument("--no-build-output", "-Q", action="store_true") copy_flags = argparse.ArgumentParser(add_help=False) - copy_flags.add_argument("--use-substitutes", "-s", action="store_true") + copy_flags.add_argument( + "--use-substitutes", "--substitute-on-destination", "-s", action="store_true" + ) sub_parsers = { "common_flags": common_flags, @@ -90,6 +92,7 @@ def get_parser() -> tuple[argparse.ArgumentParser, dict[str, argparse.ArgumentPa main_parser.add_argument("--fast", action="store_true") main_parser.add_argument("--build-host") main_parser.add_argument("--target-host") + main_parser.add_argument("--no-build-nix", action="store_true") # deprecated main_parser.add_argument("action", choices=Action.values(), nargs="?") return main_parser, sub_parsers @@ -133,6 +136,10 @@ def parse_args( if args.no_ssh_tty: parser_warn("--no-ssh-tty deprecated, SSH's TTY is never used anymore") + # TODO: use deprecated=True in Python >=3.13 + if args.no_build_nix: + parser_warn("--no-build-nix deprecated, we do not build nix anymore") + if args.action == Action.EDIT.value and (args.file or args.attr): parser.error("--file and --attr are not supported with 'edit'") From cfe42fba1c5c0f8dde09946d122ee02f4d67f997 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 20:49:44 +0000 Subject: [PATCH 25/41] nixos-rebuild-ng: do not fail if re-exec fails --- .../src/nixos_rebuild/__init__.py | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 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 e7124f93001b..d51a3c273c33 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 @@ -5,7 +5,7 @@ import logging import os import sys from pathlib import Path -from subprocess import run +from subprocess import CalledProcessError, run from typing import assert_never from . import nix @@ -185,27 +185,32 @@ def execute(argv: list[str]) -> None: # untrusted tree. can_run = action in (Action.SWITCH, Action.BOOT, Action.TEST) + if args.upgrade or args.upgrade_all: + nix.upgrade_channels(bool(args.upgrade_all)) + # Re-exec to a newer version of the script before building to ensure we get # the latest fixes if can_run and not args.fast and not os.environ.get("_NIXOS_REBUILD_REEXEC"): - if flake: - drv = nix.build_flake("pkgs.nixos-rebuild-ng", flake, **flake_build_flags) - else: - drv = nix.build("pkgs.nixos-rebuild-ng", build_attr, **build_flags) - new = drv / "bin/nixos-rebuild-ng" - current = Path(argv[0]) - # Disable re-exec during development - if current.name != "__main__.py" and new != current: - logging.debug( - "detected newer version of script, re-exec'ing, current=%s, new=%s", - argv[0], - new, - ) - cleanup_ssh() - os.execve(new, argv, os.environ | {"_NIXOS_REBUILD_REEXEC": "1"}) - - if args.upgrade or args.upgrade_all: - nix.upgrade_channels(bool(args.upgrade_all)) + try: + if flake: + drv = nix.build_flake( + "pkgs.nixos-rebuild-ng", flake, **flake_build_flags + ) + else: + drv = nix.build("pkgs.nixos-rebuild-ng", build_attr, **build_flags) + new = drv / "bin/nixos-rebuild-ng" + current = Path(argv[0]) + # Disable re-exec during development + if current.name != "__main__.py" and new != current: + logging.debug( + "detected newer version of script, re-exec'ing, current=%s, new=%s", + argv[0], + new, + ) + cleanup_ssh() + os.execve(new, argv, os.environ | {"_NIXOS_REBUILD_REEXEC": "1"}) + except CalledProcessError: + logger.warning("could not find a newer version of nixos-rebuild") if can_run and not flake: nixpkgs_path = nix.find_file("nixpkgs", **build_flags) From f72572c1470019513b4bf5a5d31ee8640dd5b99e Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 20:57:28 +0000 Subject: [PATCH 26/41] nixos-rebuild-ng: ignore non-directories in upgrade_channels --- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py | 2 +- pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_nix.py | 3 ++- 2 files changed, 3 insertions(+), 2 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 5247a2991b1b..7cc606561404 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 @@ -471,7 +471,7 @@ def upgrade_channels(all: bool = False) -> None: that has a `.update-on-nixos-rebuild` file) or all. """ for channel_path in Path("/nix/var/nix/profiles/per-user/root/channels/").glob("*"): - if ( + if channel_path.is_dir() and ( all or channel_path.name == "nixos" or (channel_path / ".update-on-nixos-rebuild").exists() 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 317821d5fea0..093cba93ef36 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 @@ -561,7 +561,8 @@ def test_switch_to_configuration(mock_run: Any, monkeypatch: Any) -> None: Path("/nix/var/nix/profiles/per-user/root/channels/home-manager"), ], ) -def test_upgrade_channels(mock_glob: Any) -> None: +@patch(get_qualified_name(n.Path.is_dir, n), autospec=True, return_value=True) +def test_upgrade_channels(mock_is_dir: Any, mock_glob: Any) -> None: with patch(get_qualified_name(n.run_wrapper, n), autospec=True) as mock_run: n.upgrade_channels(False) mock_run.assert_called_with(["nix-channel", "--update", "nixos"], check=False) From c50144ab79b9a5cbeea195f032be2f8bb5a73147 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Fri, 29 Nov 2024 23:42:34 +0000 Subject: [PATCH 27/41] nixos-rebuild-ng: move reexec earlier --- .../src/nixos_rebuild/__init__.py | 82 ++++++++++++------- 1 file changed, 52 insertions(+), 30 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 d51a3c273c33..a6153ae974f8 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 @@ -11,7 +11,7 @@ from typing import assert_never from . import nix from .models import Action, BuildAttr, Flake, NRError, Profile from .process import Remote, cleanup_ssh -from .utils import LogFormatter +from .utils import Args, LogFormatter logger = logging.getLogger() logger.setLevel(logging.INFO) @@ -161,22 +161,60 @@ def parse_args( return args, args_groups +def reexec( + argv: list[str], + args: argparse.Namespace, + build_flags: dict[str, Args], + flake_build_flags: dict[str, Args], +) -> None: + drv = None + try: + # Need to set target_host=None, to avoid connecting to remote + if flake := Flake.from_arg(args.flake, None): + drv = nix.build_flake( + "pkgs.nixos-rebuild-ng", + flake, + **flake_build_flags, + no_link=True, + ) + else: + drv = nix.build( + "pkgs.nixos-rebuild-ng", + BuildAttr.from_arg(args.attr, args.file), + **build_flags, + no_out_link=True, + ) + except CalledProcessError: + logger.warning("could not find a newer version of nixos-rebuild") + + if drv: + new = drv / "bin/nixos-rebuild-ng" + current = Path(argv[0]) + # Disable re-exec during development + if current.name != "__main__.py" and new != current: + logging.debug( + "detected newer version of script, re-exec'ing, current=%s, new=%s", + argv[0], + new, + ) + cleanup_ssh() + os.execve(new, argv, os.environ | {"_NIXOS_REBUILD_REEXEC": "1"}) + + def execute(argv: list[str]) -> None: args, args_groups = parse_args(argv) + 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"]) flake_build_flags = common_build_flags | vars(args_groups["flake_build_flags"]) copy_flags = common_flags | vars(args_groups["copy_flags"]) - atexit.register(cleanup_ssh) + if args.upgrade or args.upgrade_all: + nix.upgrade_channels(bool(args.upgrade_all)) - profile = Profile.from_arg(args.profile_name) - build_host = Remote.from_arg(args.build_host, False, validate_opts=False) - target_host = Remote.from_arg(args.target_host, args.ask_sudo_password) - build_attr = BuildAttr.from_arg(args.attr, args.file) - flake = Flake.from_arg(args.flake, target_host) action = Action(args.action) # Only run shell scripts from the Nixpkgs tree if the action is # "switch", "boot", or "test". With other actions (such as "build"), @@ -185,32 +223,16 @@ def execute(argv: list[str]) -> None: # untrusted tree. can_run = action in (Action.SWITCH, Action.BOOT, Action.TEST) - if args.upgrade or args.upgrade_all: - nix.upgrade_channels(bool(args.upgrade_all)) - # Re-exec to a newer version of the script before building to ensure we get # the latest fixes if can_run and not args.fast and not os.environ.get("_NIXOS_REBUILD_REEXEC"): - try: - if flake: - drv = nix.build_flake( - "pkgs.nixos-rebuild-ng", flake, **flake_build_flags - ) - else: - drv = nix.build("pkgs.nixos-rebuild-ng", build_attr, **build_flags) - new = drv / "bin/nixos-rebuild-ng" - current = Path(argv[0]) - # Disable re-exec during development - if current.name != "__main__.py" and new != current: - logging.debug( - "detected newer version of script, re-exec'ing, current=%s, new=%s", - argv[0], - new, - ) - cleanup_ssh() - os.execve(new, argv, os.environ | {"_NIXOS_REBUILD_REEXEC": "1"}) - except CalledProcessError: - logger.warning("could not find a newer version of nixos-rebuild") + reexec(argv, args, build_flags, flake_build_flags) + + profile = Profile.from_arg(args.profile_name) + target_host = Remote.from_arg(args.target_host, args.ask_sudo_password) + build_host = Remote.from_arg(args.build_host, False, validate_opts=False) + build_attr = BuildAttr.from_arg(args.attr, args.file) + flake = Flake.from_arg(args.flake, target_host) if can_run and not flake: nixpkgs_path = nix.find_file("nixpkgs", **build_flags) From b118371ebb1fe22a0723f72832b9062dc1597deb Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sat, 30 Nov 2024 17:45:30 +0000 Subject: [PATCH 28/41] nixos-rebuild-ng: add SSH_DEFAULT_OPTS to copy-closure --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/nix.py | 14 ++++++++------ .../src/nixos_rebuild/process.py | 2 +- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 16 ++++++++++------ 3 files changed, 19 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 7cc606561404..e4e1c5663fcb 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 @@ -1,5 +1,6 @@ import logging import os +import shlex from datetime import datetime from importlib.resources import files from pathlib import Path @@ -17,7 +18,7 @@ from .models import ( Profile, Remote, ) -from .process import run_wrapper +from .process import SSH_DEFAULT_OPTS, run_wrapper from .utils import Args, dict_to_flags FLAKE_FLAGS: Final = ["--extra-experimental-features", "nix-command flakes"] @@ -153,12 +154,13 @@ def copy_closure( closure, ], extra_env={ - # for the remote to remote case, we can't use host.opts because it - # includes the ControlPane opts that will not work in the remote, - # because the temporary directory that we created will not exist - "NIX_SSHOPTS": os.environ.get("NIX_SSHOPTS", "") + # for the remote to remote case, we can't add SSH_DEFAULT_OPTS to + # host.opts because it includes the ControlPane opts that will not + # work in the remote, because the temporary directory that we + # created will not exist + "NIX_SSHOPTS": shlex.join(host.opts) if from_host and to_host - else " ".join(host.opts) + else shlex.join(SSH_DEFAULT_OPTS + host.opts) }, remote=from_host if to_host else None, ) 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 54200f14d85a..a9e8d7dff295 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 @@ -38,7 +38,7 @@ class Remote: if not host: return None - opts = os.getenv("NIX_SSHOPTS", "").split() + opts = shlex.split(os.getenv("NIX_SSHOPTS", "")) if validate_opts: cls._validate_opts(opts, ask_sudo_password) sudo_password = None 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 093cba93ef36..0a98270490f9 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 @@ -8,6 +8,7 @@ import pytest import nixos_rebuild.models as m import nixos_rebuild.nix as n +import nixos_rebuild.process as p from .helpers import get_qualified_name @@ -108,7 +109,9 @@ def test_remote_build(mock_run: Any, monkeypatch: Any) -> None: "user@host", Path("/path/to/file"), ], - extra_env={"NIX_SSHOPTS": "--ssh opts"}, + extra_env={ + "NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh opts"]) + }, remote=None, ), call( @@ -159,7 +162,9 @@ def test_remote_build_flake(mock_run: Any) -> None: "user@host", Path("/path/to/file"), ], - extra_env={"NIX_SSHOPTS": "--ssh opts"}, + extra_env={ + "NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh opts"]) + }, remote=None, ), call( @@ -191,23 +196,22 @@ def test_copy_closure(mock_run: Any, monkeypatch: Any) -> None: n.copy_closure(closure, target_host) mock_run.assert_called_with( ["nix-copy-closure", "--to", "user@target.host", closure], - extra_env={"NIX_SSHOPTS": "--ssh target-opt"}, + extra_env={"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh target-opt"])}, remote=None, ) n.copy_closure(closure, None, build_host) mock_run.assert_called_with( ["nix-copy-closure", "--from", "user@build.host", closure], - extra_env={"NIX_SSHOPTS": "--ssh build-opt"}, + extra_env={"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh build-opt"])}, remote=None, ) - monkeypatch.setenv("NIX_SSHOPTS", "--ssh build-target-opt") n.copy_closure(closure, target_host, build_host) mock_run.assert_called_with( ["nix-copy-closure", "--to", "user@target.host", closure], remote=build_host, - extra_env={"NIX_SSHOPTS": "--ssh build-target-opt"}, + extra_env={"NIX_SSHOPTS": "--ssh target-opt"}, ) From cab7882bf5215adc8b0187bcbd1c9454f68e493a Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sat, 30 Nov 2024 17:46:23 +0000 Subject: [PATCH 29/41] nixos-rebuild-ng: move check for missing action argument earlier --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 a6153ae974f8..92394197110a 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 @@ -122,6 +122,10 @@ def parse_args( if args.ask_sudo_password: args.sudo = True + if args.help or args.action is None: + r = run(["man", "8", "nixos-rebuild"], check=False) + parser.exit(r.returncode) + # TODO: use deprecated=True in Python >=3.13 if args.install_grub: parser_warn("--install-grub deprecated, use --install-bootloader instead") @@ -154,10 +158,6 @@ def parse_args( if args.flake and (args.file or args.attr): parser.error("--flake cannot be used with --file or --attr") - if args.help or args.action is None: - r = run(["man", "8", "nixos-rebuild"], check=False) - parser.exit(r.returncode) - return args, args_groups From a61aede3688d97cd8dd47f311c7e898cc006d8f7 Mon Sep 17 00:00:00 2001 From: r-vdp Date: Sat, 30 Nov 2024 17:57:06 +0100 Subject: [PATCH 30/41] nixos-rebuild-ng: make sure to copy the new closure when doing test or build --- pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 3 +++ 1 file changed, 3 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 92394197110a..3371424ea214 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 @@ -331,6 +331,9 @@ def execute(argv: list[str]) -> None: dry_run=dry_run, **build_flags, ) + # If we are rolling back, the generation that we roll back to, should already be present on the remote + if not args.rollback: + nix.copy_closure(path_to_config, target_host, **copy_flags) if action in (Action.TEST, Action.DRY_ACTIVATE): nix.switch_to_configuration( path_to_config, From 4431a47fcb4a2a97cc40e5e5e931a1d06fd2d08f Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sat, 30 Nov 2024 18:04:31 +0000 Subject: [PATCH 31/41] nixos-rebuild-ng: add --build-host/--target-host to TEST/BUILD/DRY_BUILD/DRY_ACTIVATE --- .../src/nixos_rebuild/__init__.py | 60 ++++++++++++++----- 1 file changed, 45 insertions(+), 15 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 3371424ea214..25f6889314ad 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 @@ -150,6 +150,10 @@ def parse_args( if (args.target_host or args.build_host) and args.action not in ( Action.SWITCH.value, Action.BOOT.value, + Action.TEST.value, + Action.BUILD.value, + Action.DRY_BUILD.value, + Action.DRY_ACTIVATE.value, ): parser.error( f"--target-host/--build-host is not supported with '{args.action}'" @@ -318,27 +322,53 @@ def execute(argv: list[str]) -> None: else: raise NRError("could not find previous generation") elif flake: - path_to_config = nix.build_flake( - attr, - flake, - dry_run=dry_run, - **flake_build_flags, - ) + if build_host: + path_to_config = nix.remote_build_flake( + attr, + flake, + build_host, + flake_build_flags=flake_build_flags, + copy_flags=copy_flags, + build_flags=build_flags, + ) + else: + path_to_config = nix.build_flake( + attr, + flake, + dry_run=dry_run, + **flake_build_flags, + ) else: - path_to_config = nix.build( - attr, - build_attr, - dry_run=dry_run, - **build_flags, - ) - # If we are rolling back, the generation that we roll back to, should already be present on the remote + if build_host: + path_to_config = nix.remote_build( + attr, + build_attr, + build_host, + instantiate_flags=common_flags, + copy_flags=copy_flags, + build_flags=build_flags, + ) + else: + path_to_config = nix.build( + attr, + build_attr, + dry_run=dry_run, + **build_flags, + ) + # If we are rolling back, the generation that we roll back to, + # should already be present on the remote if not args.rollback: - nix.copy_closure(path_to_config, target_host, **copy_flags) + nix.copy_closure( + path_to_config, + to_host=target_host, + from_host=build_host, + **copy_flags, + ) if action in (Action.TEST, Action.DRY_ACTIVATE): nix.switch_to_configuration( path_to_config, action, - target_host=None, + target_host=target_host, sudo=args.sudo, specialisation=args.specialisation, ) From 4bc3ac552d0dc20d80333f329bac840ce0445a11 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sat, 30 Nov 2024 18:28:36 +0000 Subject: [PATCH 32/41] nixos-rebuild-ng: merge actions --- .../src/nixos_rebuild/__init__.py | 104 ++++++------------ 1 file changed, 32 insertions(+), 72 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 25f6889314ad..14236c289709 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 @@ -245,11 +245,35 @@ def execute(argv: list[str]) -> None: (nixpkgs_path / ".version-suffix").write_text(rev) match action: - case Action.SWITCH | Action.BOOT: + case ( + Action.SWITCH + | Action.BOOT + | Action.TEST + | Action.BUILD + | Action.DRY_BUILD + | Action.DRY_ACTIVATE + ): logger.info("building the system configuration...") + attr = "config.system.build.toplevel" + dry_run = action == Action.DRY_BUILD + no_link = action in (Action.SWITCH, Action.BOOT) + if args.rollback: - path_to_config = nix.rollback(profile, target_host, sudo=args.sudo) + if action in (Action.SWITCH, Action.BOOT): + path_to_config = nix.rollback(profile, target_host, sudo=args.sudo) + elif action in (Action.TEST, Action.BUILD): + maybe_path_to_config = nix.rollback_temporary_profile( + profile, + target_host, + sudo=args.sudo, + ) + if maybe_path_to_config: # kinda silly but this makes mypy happy + path_to_config = maybe_path_to_config + else: + raise NRError("could not find previous generation") + else: + raise NRError(f"--rollback is incompatible with '{action}'") else: if flake: if build_host: @@ -265,7 +289,8 @@ def execute(argv: list[str]) -> None: path_to_config = nix.build_flake( attr, flake, - no_link=True, + no_link=no_link, + dry_run=dry_run, **flake_build_flags, ) else: @@ -282,7 +307,8 @@ def execute(argv: list[str]) -> None: path_to_config = nix.build( attr, build_attr, - no_out_link=True, + no_out_link=no_link, + dry_run=dry_run, **build_flags, ) nix.copy_closure( @@ -297,80 +323,14 @@ def execute(argv: list[str]) -> None: target_host=target_host, sudo=args.sudo, ) - nix.switch_to_configuration( - path_to_config, - action, - target_host=target_host, - sudo=args.sudo, - specialisation=args.specialisation, - install_bootloader=args.install_bootloader, - ) - case Action.TEST | Action.BUILD | Action.DRY_BUILD | Action.DRY_ACTIVATE: - logger.info("building the system configuration...") - attr = "config.system.build.toplevel" - dry_run = action == Action.DRY_BUILD - if args.rollback: - if action not in (Action.TEST, Action.BUILD): - raise NRError(f"--rollback is incompatible with '{action}'") - maybe_path_to_config = nix.rollback_temporary_profile( - profile, - target_host, - sudo=args.sudo, - ) - if maybe_path_to_config: # kinda silly but this makes mypy happy - path_to_config = maybe_path_to_config - else: - raise NRError("could not find previous generation") - elif flake: - if build_host: - path_to_config = nix.remote_build_flake( - attr, - flake, - build_host, - flake_build_flags=flake_build_flags, - copy_flags=copy_flags, - build_flags=build_flags, - ) - else: - path_to_config = nix.build_flake( - attr, - flake, - dry_run=dry_run, - **flake_build_flags, - ) - else: - if build_host: - path_to_config = nix.remote_build( - attr, - build_attr, - build_host, - instantiate_flags=common_flags, - copy_flags=copy_flags, - build_flags=build_flags, - ) - else: - path_to_config = nix.build( - attr, - build_attr, - dry_run=dry_run, - **build_flags, - ) - # If we are rolling back, the generation that we roll back to, - # should already be present on the remote - if not args.rollback: - nix.copy_closure( - path_to_config, - to_host=target_host, - from_host=build_host, - **copy_flags, - ) - if action in (Action.TEST, Action.DRY_ACTIVATE): + if action in (Action.SWITCH, Action.BOOT, Action.TEST, Action.DRY_ACTIVATE): nix.switch_to_configuration( path_to_config, action, target_host=target_host, sudo=args.sudo, specialisation=args.specialisation, + install_bootloader=args.install_bootloader, ) case Action.BUILD_VM | Action.BUILD_VM_WITH_BOOTLOADER: logger.info("building the system configuration...") From 6ea9eae476d5f325275aa7b604cf3bccf883c576 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sat, 30 Nov 2024 20:19:32 +0000 Subject: [PATCH 33/41] nixos-rebuild-ng: avoid usage of implementation details in LogFormatter --- .../src/nixos_rebuild/__init__.py | 2 +- .../nixos-rebuild-ng/src/nixos_rebuild/utils.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 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 14236c289709..5b90caf545a2 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 @@ -391,7 +391,7 @@ def execute(argv: list[str]) -> None: def main() -> None: ch = logging.StreamHandler() - ch.setFormatter(LogFormatter("%(levelname)s: %(message)s")) + ch.setFormatter(LogFormatter()) logger.addHandler(ch) try: diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py index dc8d899ab056..cd89435f2d4f 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/nixos_rebuild/utils.py @@ -5,17 +5,17 @@ Args: TypeAlias = bool | str | list[str] | int | None class LogFormatter(logging.Formatter): + formatters = { + logging.INFO: logging.Formatter("%(message)s"), + logging.DEBUG: logging.Formatter("%(levelname)s: %(name)s: %(message)s"), + "DEFAULT": logging.Formatter("%(levelname)s: %(message)s"), + } + @override def format(self, record: logging.LogRecord) -> str: record.levelname = record.levelname.lower() - match record.levelno: - case logging.INFO: - self._style._fmt = "%(message)s" - case logging.DEBUG: - self._style._fmt = "%(levelname)s: %(name)s: %(message)s" - case _: - self._style._fmt = "%(levelname)s: %(message)s" - return super().format(record) + formatter = self.formatters.get(record.levelno, self.formatters["DEFAULT"]) + return formatter.format(record) def dict_to_flags(d: dict[str, Args]) -> list[str]: From c4902dad75f98479b5ca13be0d6a5abfc6d3f22a Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sun, 1 Dec 2024 12:15:54 +0000 Subject: [PATCH 34/41] nixos-rebuild-ng: use raw NIX_SSHOPTS in copy_closure --- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 14 +++++++------- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 18 +++++++++++------- 2 files changed, 18 insertions(+), 14 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 e4e1c5663fcb..7c9338b3ef92 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 @@ -1,6 +1,5 @@ import logging import os -import shlex from datetime import datetime from importlib.resources import files from pathlib import Path @@ -145,6 +144,7 @@ def copy_closure( if not host: return + sshopts = os.getenv("NIX_SSHOPTS", "") run_wrapper( [ "nix-copy-closure", @@ -154,13 +154,13 @@ def copy_closure( closure, ], extra_env={ - # for the remote to remote case, we can't add SSH_DEFAULT_OPTS to - # host.opts because it includes the ControlPane opts that will not - # work in the remote, because the temporary directory that we - # created will not exist - "NIX_SSHOPTS": shlex.join(host.opts) + # Using raw NIX_SSHOPTS here to avoid messing up with the passed + # parameters, and we do not add the SSH_DEFAULT_OPTS in the remote + # to remote case, otherwise it will fail because of ControlPath + # will not exist in remote + "NIX_SSHOPTS": sshopts if from_host and to_host - else shlex.join(SSH_DEFAULT_OPTS + host.opts) + else " ".join(filter(lambda x: x, [*SSH_DEFAULT_OPTS, sshopts])) }, remote=from_host if to_host else None, ) 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 0a98270490f9..6a34f98bd066 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 @@ -79,7 +79,8 @@ def test_build_flake(mock_run: Any) -> None: return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), ) def test_remote_build(mock_run: Any, monkeypatch: Any) -> None: - build_host = m.Remote("user@host", ["--ssh", "opts"], None) + build_host = m.Remote("user@host", [], None) + monkeypatch.setenv("NIX_SSHOPTS", "--ssh opts") assert n.remote_build( "config.system.build.toplevel", m.BuildAttr("", "preAttr"), @@ -128,9 +129,10 @@ def test_remote_build(mock_run: Any, monkeypatch: Any) -> None: autospec=True, return_value=CompletedProcess([], 0, stdout=" \n/path/to/file\n "), ) -def test_remote_build_flake(mock_run: Any) -> None: +def test_remote_build_flake(mock_run: Any, monkeypatch: Any) -> None: flake = m.Flake.parse(".#hostname") - build_host = m.Remote("user@host", ["--ssh", "opts"], None) + build_host = m.Remote("user@host", [], None) + monkeypatch.setenv("NIX_SSHOPTS", "--ssh opts") assert n.remote_build_flake( "config.system.build.toplevel", @@ -190,16 +192,17 @@ def test_copy_closure(mock_run: Any, monkeypatch: Any) -> None: n.copy_closure(closure, None) mock_run.assert_not_called() - target_host = m.Remote("user@target.host", ["--ssh", "target-opt"], None) - build_host = m.Remote("user@build.host", ["--ssh", "build-opt"], None) + target_host = m.Remote("user@target.host", [], None) + build_host = m.Remote("user@build.host", [], None) n.copy_closure(closure, target_host) mock_run.assert_called_with( ["nix-copy-closure", "--to", "user@target.host", closure], - extra_env={"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS + ["--ssh target-opt"])}, + extra_env={"NIX_SSHOPTS": " ".join(p.SSH_DEFAULT_OPTS)}, remote=None, ) + monkeypatch.setenv("NIX_SSHOPTS", "--ssh build-opt") n.copy_closure(closure, None, build_host) mock_run.assert_called_with( ["nix-copy-closure", "--from", "user@build.host", closure], @@ -207,11 +210,12 @@ def test_copy_closure(mock_run: Any, monkeypatch: Any) -> None: remote=None, ) + monkeypatch.setenv("NIX_SSHOPTS", "--ssh build-target-opt") n.copy_closure(closure, target_host, build_host) mock_run.assert_called_with( ["nix-copy-closure", "--to", "user@target.host", closure], remote=build_host, - extra_env={"NIX_SSHOPTS": "--ssh target-opt"}, + extra_env={"NIX_SSHOPTS": "--ssh build-target-opt"}, ) From 752c092c47a8ed8d10dcfe4f1e24ab963cc59ae8 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sun, 1 Dec 2024 12:47:20 +0000 Subject: [PATCH 35/41] nixos-rebuild-ng: use shlex.quote instead of join in run_wrapper --- .../src/nixos_rebuild/process.py | 9 +++++---- .../nixos-rebuild-ng/src/tests/test_main.py | 20 ++++++++++++++++--- .../src/tests/test_process.py | 13 ++++++++++-- 3 files changed, 33 insertions(+), 9 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 a9e8d7dff295..1232275d23c3 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 @@ -105,10 +105,11 @@ def run_wrapper( *SSH_DEFAULT_OPTS, remote.host, "--", - # sadly SSH just join all remaining parameters, expanding glob and - # ignoring quotes - # so we need to use shlex.join() to safely join the arguments - shlex.join(str(a) for a in args), + # SSH will join the parameters here and pass it to the shell, so we + # need to quote it to avoid issues. + # We can't use `shlex.join`, otherwise we will hit MAX_ARG_STRLEN + # limits when the command becomes too big. + *[shlex.quote(str(a)) for a in args], ] else: if extra_env: 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 97ae0072a829..9337fda44dc0 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 @@ -278,7 +278,12 @@ def test_execute_nix_switch_flake_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", - f"sudo nix-env -p /nix/var/nix/profiles/system --set {config_path}", + "sudo", + "nix-env", + "-p", + "/nix/var/nix/profiles/system", + "--set", + str(config_path), ], check=True, **DEFAULT_RUN_KWARGS, @@ -289,7 +294,11 @@ def test_execute_nix_switch_flake_target_host( *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", - f"sudo env NIXOS_INSTALL_BOOTLOADER=0 {config_path / 'bin/switch-to-configuration'} switch", + "sudo", + "env", + "NIXOS_INSTALL_BOOTLOADER=0", + f"{config_path / 'bin/switch-to-configuration'}", + "switch", ], check=True, **DEFAULT_RUN_KWARGS, @@ -360,7 +369,12 @@ def test_execute_nix_switch_flake_build_host( *nr.process.SSH_DEFAULT_OPTS, "user@localhost", "--", - f"nix --extra-experimental-features 'nix-command flakes' build '{config_path}^*' --print-out-paths", + "nix", + "--extra-experimental-features", + "'nix-command flakes'", + "build", + f"'{config_path}^*'", + "--print-out-paths", ], check=True, stdout=PIPE, diff --git a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py index 53ed6824e39f..6c2ca2289245 100644 --- a/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py +++ b/pkgs/by-name/ni/nixos-rebuild-ng/src/tests/test_process.py @@ -51,7 +51,9 @@ def test_run(mock_run: Any) -> None: *p.SSH_DEFAULT_OPTS, "user@localhost", "--", - "test --with 'some flags'", + "test", + "--with", + "'some flags'", ], check=True, text=True, @@ -75,7 +77,14 @@ def test_run(mock_run: Any) -> None: *p.SSH_DEFAULT_OPTS, "user@localhost", "--", - "sudo --prompt= --stdin env FOO=bar test --with flags", + "sudo", + "--prompt=", + "--stdin", + "env", + "FOO=bar", + "test", + "--with", + "flags", ], check=True, env=None, From d704aae2cff68bb6dd409c7467dca78f956d713a Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sun, 1 Dec 2024 13:10:47 +0000 Subject: [PATCH 36/41] nixos-rebuild-ng: add logging for captured output values --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 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 1232275d23c3..946cc70f6a69 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 @@ -118,14 +118,14 @@ def run_wrapper( args = ["sudo", *args] logger.debug( - "calling run with args=%s, kwargs=%s, extra_env=%s", + "calling run with args=%r, kwargs=%r, extra_env=%r", args, kwargs, extra_env, ) try: - return subprocess.run( + r = subprocess.run( args, check=check, env=env, @@ -136,6 +136,11 @@ def run_wrapper( errors="surrogateescape", **kwargs, ) + + if kwargs.get("capture_output") or kwargs.get("stderr") or kwargs.get("stdout"): + logger.debug("captured output stdout=%r, stderr=%r", r.stdout, r.stderr) + + return r except subprocess.CalledProcessError: if sudo and remote: logger.error( From fb5178c3c512b3f29d35548b62cce5b74e4651c1 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sun, 1 Dec 2024 13:54:53 +0000 Subject: [PATCH 37/41] nixos-rebuild-ng: set capture_output=True to cleanup_ssh --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/process.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 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 946cc70f6a69..07d01c6f121b 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 @@ -71,8 +71,10 @@ class RunKwargs(TypedDict, total=False): def cleanup_ssh() -> None: "Close SSH ControlMaster connection." for ctrl in TMPDIR_PATH.glob("ssh-*"): - subprocess.run( - ["ssh", "-o", f"ControlPath={ctrl}", "-O", "exit", "dummyhost"], check=False + run_wrapper( + ["ssh", "-o", f"ControlPath={ctrl}", "-O", "exit", "dummyhost"], + check=False, + capture_output=True, ) TMPDIR.cleanup() From d34056b2189d60118b5914db0d5d349efd25a3e0 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Sun, 1 Dec 2024 20:21:58 +0000 Subject: [PATCH 38/41] nixos-rebuild-ng: disable _NIXOS_REBUILD_REEXEC for now --- .../ni/nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 7 ++++++- 1 file changed, 6 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 5b90caf545a2..043aa9a3e157 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 @@ -229,7 +229,12 @@ def execute(argv: list[str]) -> None: # Re-exec to a newer version of the script before building to ensure we get # the latest fixes - if can_run and not args.fast and not os.environ.get("_NIXOS_REBUILD_REEXEC"): + if ( + False # disabled until we introduce `config.system.build.nixos-rebuild-ng` + and can_run + and not args.fast + and not os.environ.get("_NIXOS_REBUILD_REEXEC") + ): reexec(argv, args, build_flags, flake_build_flags) profile = Profile.from_arg(args.profile_name) From debea81ba7f3b06c34edfeb5b2bec0907d912d66 Mon Sep 17 00:00:00 2001 From: r-vdp Date: Tue, 3 Dec 2024 12:19:10 +0000 Subject: [PATCH 39/41] nixos-rebuild-ng: don't try to register the profile when doing build or test --- .../nixos-rebuild-ng/src/nixos_rebuild/__init__.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 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 043aa9a3e157..d41a0fcbd1bf 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 @@ -322,12 +322,13 @@ def execute(argv: list[str]) -> None: from_host=build_host, **copy_flags, ) - nix.set_profile( - profile, - path_to_config, - target_host=target_host, - sudo=args.sudo, - ) + if action in (Action.SWITCH, Action.BOOT): + nix.set_profile( + profile, + path_to_config, + target_host=target_host, + sudo=args.sudo, + ) if action in (Action.SWITCH, Action.BOOT, Action.TEST, Action.DRY_ACTIVATE): nix.switch_to_configuration( path_to_config, From c6e9bd02ca1197289185ef9df5fc9b295352965f Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Tue, 3 Dec 2024 12:54:13 +0000 Subject: [PATCH 40/41] nixos-rebuild-ng: add test to `nixos-rebuild build` --- .../nixos-rebuild-ng/src/tests/test_main.py | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) 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 9337fda44dc0..913103985f66 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 @@ -445,6 +445,35 @@ def test_execute_switch_rollback(mock_run: Any, tmp_path: Path) -> None: ) +@patch(get_qualified_name(nr.process.subprocess.run), autospec=True) +def test_execute_build(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)), + ] + + nr.execute(["nixos-rebuild", "build", "--no-flake", "--fast"]) + + assert mock_run.call_count == 1 + mock_run.assert_has_calls( + [ + call( + [ + "nix-build", + "", + "--attr", + "config.system.build.toplevel", + ], + check=True, + stdout=PIPE, + **DEFAULT_RUN_KWARGS, + ) + ] + ) + + @patch(get_qualified_name(nr.process.subprocess.run), autospec=True) @patch(get_qualified_name(nr.nix.Path.exists, nr.nix), autospec=True, return_value=True) @patch(get_qualified_name(nr.nix.Path.mkdir, nr.nix), autospec=True) From 98e9372c1c7f3cf9fc76715ea6904bd797dc33d1 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Tue, 3 Dec 2024 12:53:04 +0000 Subject: [PATCH 41/41] nixos-rebuild: refactor if-else in match --- .../src/nixos_rebuild/__init__.py | 86 ++++++++++--------- 1 file changed, 45 insertions(+), 41 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 d41a0fcbd1bf..c7dbf8acb350 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 @@ -263,11 +263,12 @@ def execute(argv: list[str]) -> None: attr = "config.system.build.toplevel" dry_run = action == Action.DRY_BUILD no_link = action in (Action.SWITCH, Action.BOOT) + rollback = bool(args.rollback) - if args.rollback: - if action in (Action.SWITCH, Action.BOOT): + match (action, rollback, build_host, flake): + case (Action.SWITCH | Action.BOOT, True, _, _): path_to_config = nix.rollback(profile, target_host, sudo=args.sudo) - elif action in (Action.TEST, Action.BUILD): + case (Action.TEST | Action.BUILD, True, _, _): maybe_path_to_config = nix.rollback_temporary_profile( profile, target_host, @@ -277,45 +278,48 @@ def execute(argv: list[str]) -> None: path_to_config = maybe_path_to_config else: raise NRError("could not find previous generation") - else: + case (_, True, _, _): raise NRError(f"--rollback is incompatible with '{action}'") - else: - if flake: - if build_host: - path_to_config = nix.remote_build_flake( - attr, - flake, - build_host, - flake_build_flags=flake_build_flags, - copy_flags=copy_flags, - build_flags=build_flags, - ) - else: - path_to_config = nix.build_flake( - attr, - flake, - no_link=no_link, - dry_run=dry_run, - **flake_build_flags, - ) - else: - if build_host: - path_to_config = nix.remote_build( - attr, - build_attr, - build_host, - instantiate_flags=common_flags, - copy_flags=copy_flags, - build_flags=build_flags, - ) - else: - path_to_config = nix.build( - attr, - build_attr, - no_out_link=no_link, - dry_run=dry_run, - **build_flags, - ) + case (_, False, Remote(_), Flake(_)): + path_to_config = nix.remote_build_flake( + attr, + flake, + build_host, + flake_build_flags=flake_build_flags, + copy_flags=copy_flags, + build_flags=build_flags, + ) + case (_, False, None, Flake(_)): + path_to_config = nix.build_flake( + attr, + flake, + no_link=no_link, + dry_run=dry_run, + **flake_build_flags, + ) + case (_, False, Remote(_), None): + path_to_config = nix.remote_build( + attr, + build_attr, + build_host, + instantiate_flags=common_flags, + copy_flags=copy_flags, + build_flags=build_flags, + ) + case (_, False, None, None): + path_to_config = nix.build( + attr, + build_attr, + no_out_link=no_link, + dry_run=dry_run, + **build_flags, + ) + case m: + # should never happen, but mypy is not smart enough to + # handle this with assert_never + raise NRError(f"invalid match for build: {m}") + + if not rollback: nix.copy_closure( path_to_config, to_host=target_host,