From 8e5ac55fb1dea52932f38bbc4502133a4dcc2a09 Mon Sep 17 00:00:00 2001 From: phaer Date: Tue, 1 Apr 2025 11:52:18 +0200 Subject: [PATCH 1/2] nixos-rebuild: don't eval closure before validating variant Because we want to be able to list variants even if one of them might not eval correctly. The eager evaluation was caused by us querying for the resulting image file name to early and is fixed by calling nix-instantiate/nix eval twice now, once for the variants, once for the image file name. fixes #394626 --- .../linux/nixos-rebuild/nixos-rebuild.sh | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh b/pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh index c4766a7e685d..6c61b3c7a503 100755 --- a/pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh +++ b/pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh @@ -835,28 +835,50 @@ if [ -z "$rollback" ]; then "let value = import \"$(realpath $buildFile)\"; set = if builtins.isFunction value then value {} else value; - in builtins.mapAttrs (n: v: v.passthru.filePath) set.${attr:+$attr.}config.system.build.images" \ + in builtins.attrNames set.${attr:+$attr.}config.system.build.images" \ "${extraBuildFlags[@]}" )" elif [[ -z $flake ]]; then variants="$( runCmd nix-instantiate --eval --strict --json --expr \ - "with import {}; builtins.mapAttrs (n: v: v.passthru.filePath) config.system.build.images" \ + "with import {}; builtins.attrNames config.system.build.images" \ "${extraBuildFlags[@]}" )" else variants="$( runCmd nix "${flakeFlags[@]}" eval --json \ "$flake#$flakeAttr.config.system.build.images" \ - --apply "builtins.mapAttrs (n: v: v.passthru.filePath)" "${evalArgs[@]}" "${extraBuildFlags[@]}" + --apply "builtins.attrNames" "${evalArgs[@]}" "${extraBuildFlags[@]}" )" fi - if ! echo "$variants" | jq -e --arg variant "$imageVariant" "keys | any(. == \$variant)" > /dev/null; then + if ! echo "$variants" | jq -e --arg variant "$imageVariant" "any(. == \$variant)" > /dev/null; then echo -e "Please specify one of the following supported image variants via --image-variant:\n" >&2 - echo "$variants" | jq -r '. | keys | join ("\n")' + echo "$variants" | jq -r 'join ("\n")' exit 1 fi - imageName="$(echo "$variants" | jq -r --arg variant "$imageVariant" ".[\$variant]")" + + if [[ -z $buildingAttribute ]]; then + imageName="$( + runCmd nix-instantiate --eval --strict --json --expr \ + "let + value = import \"$(realpath $buildFile)\"; + set = if builtins.isFunction value then value {} else value; + in set.${attr:+$attr.}config.system.build.images.$imageVariant.v.passthru.filePath" \ + "${extraBuildFlags[@]}" + )" + elif [[ -z $flake ]]; then + imageName="$( + runCmd nix-instantiate --eval --strict --json --expr \ + "with import {}; config.system.build.images.$imageVariant.passthru.filePath" \ + "${extraBuildFlags[@]}" + )" + else + imageName="$( + runCmd nix "${flakeFlags[@]}" eval --json \ + "$flake#$flakeAttr.config.system.build.images.$imageVariant.passthru.filePath" \ + "${evalArgs[@]}" "${extraBuildFlags[@]}" + )" + fi if [[ -z $buildingAttribute ]]; then pathToConfig="$(nixBuild $buildFile -A "${attr:+$attr.}config.system.build.images.${imageVariant}" "${extraBuildFlags[@]}")" From ab4f5ac2f1783a2b60d5e691e7abacbf1513505f Mon Sep 17 00:00:00 2001 From: phaer Date: Tue, 1 Apr 2025 13:17:34 +0200 Subject: [PATCH 2/2] nixos-rebuild-ng: don't eval closure before validating variant Because we want to be able to list variants even if one of them might not eval correctly. The eager evaluation was caused by us querying for the resulting image file name to early and is fixed by calling nix-instantiate/nix eval twice now, once for the variants, once for the image file name. fixes #394626 --- .../src/nixos_rebuild/__init__.py | 16 +++++- .../src/nixos_rebuild/models.py | 2 +- .../nixos-rebuild-ng/src/nixos_rebuild/nix.py | 57 ++++++++++++++++++- .../nixos-rebuild-ng/src/tests/test_main.py | 22 ++++--- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 6 +- 5 files changed, 87 insertions(+), 16 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 7709f1af8742..173462767b05 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 @@ -396,7 +396,7 @@ def execute(argv: list[str]) -> None: raise NRError( "please specify one of the following " + "supported image variants via --image-variant:\n" - + "\n".join(f"- {v}" for v in variants.keys()) + + "\n".join(f"- {v}" for v in variants) ) match action: @@ -518,7 +518,19 @@ def execute(argv: list[str]) -> None: "Done. The virtual machine can be started by running", vm_path ) case Action.BUILD_IMAGE: - disk_path = path_to_config / variants[args.image_variant] + if flake: + image_name = nix.get_build_image_name_flake( + flake, + args.image_variant, + eval_flags=flake_common_flags, + ) + else: + image_name = nix.get_build_image_name( + build_attr, + args.image_variant, + instantiate_flags=flake_common_flags, + ) + disk_path = path_to_config / image_name print_result("Done. The disk image can be found in", disk_path) case Action.EDIT: 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 a18578a481d6..3fe47acc1bb6 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 @@ -8,7 +8,7 @@ from typing import Any, Callable, ClassVar, Self, TypedDict, override from .process import Remote, run_wrapper -type ImageVariants = dict[str, str] +type ImageVariants = list[str] class NRError(Exception): 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 c7b2010229ab..a8fed7d8b9de 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 @@ -266,6 +266,59 @@ def find_file(file: str, nix_flags: Args | None = None) -> Path | None: return Path(r.stdout.strip()) +def get_build_image_name( + build_attr: BuildAttr, + image_variant: str, + instantiate_flags: Args | None = None, +) -> str: + path = ( + f'"{build_attr.path.resolve()}"' + if isinstance(build_attr.path, Path) + else build_attr.path + ) + r = run_wrapper( + [ + "nix-instantiate", + "--eval", + "--strict", + "--json", + "--expr", + textwrap.dedent(f""" + let + value = import {path}; + set = if builtins.isFunction value then value {{}} else value; + in + set.{build_attr.to_attr("config.system.build.images", image_variant, "passthru", "filePath")} + """), + *dict_to_flags(instantiate_flags), + ], + stdout=PIPE, + ) + j: str = json.loads(r.stdout.strip()) + return j + + +def get_build_image_name_flake( + flake: Flake, + image_variant: str, + eval_flags: Args | None = None, +) -> str: + r = run_wrapper( + [ + "nix", + "eval", + "--json", + flake.to_attr( + "config.system.build.images", image_variant, "passthru", "filePath" + ), + *dict_to_flags(eval_flags), + ], + stdout=PIPE, + ) + j: str = json.loads(r.stdout.strip()) + return j + + def get_build_image_variants( build_attr: BuildAttr, instantiate_flags: Args | None = None, @@ -287,7 +340,7 @@ def get_build_image_variants( value = import {path}; set = if builtins.isFunction value then value {{}} else value; in - builtins.mapAttrs (n: v: v.passthru.filePath) set.{build_attr.to_attr("config.system.build.images")} + builtins.attrNames set.{build_attr.to_attr("config.system.build.images")} """), *dict_to_flags(instantiate_flags), ], @@ -308,7 +361,7 @@ def get_build_image_variants_flake( "--json", flake.to_attr("config.system.build.images"), "--apply", - "builtins.mapAttrs (n: v: v.passthru.filePath)", + "builtins.attrNames", *dict_to_flags(eval_flags), ], 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 2e9363e2869a..35306fc7e30c 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 @@ -347,12 +347,7 @@ def test_execute_nix_build_image_flake(mock_run: Mock, tmp_path: Path) -> None: return CompletedProcess( [], 0, - """ - { - "azure": "nixos-image-azure-25.05.20250102.6df2492-x86_64-linux.vhd", - "vmware": "nixos-image-vmware-25.05.20250102.6df2492-x86_64-linux.vmdk" - } - """, + '"nixos-image-azure-25.05.20250102.6df2492-x86_64-linux.vhd"', ) elif args[0] == "nix": return CompletedProcess([], 0, str(config_path)) @@ -372,7 +367,7 @@ def test_execute_nix_build_image_flake(mock_run: Mock, tmp_path: Path) -> None: ] ) - assert mock_run.call_count == 2 + assert mock_run.call_count == 3 mock_run.assert_has_calls( [ call( @@ -382,7 +377,7 @@ def test_execute_nix_build_image_flake(mock_run: Mock, tmp_path: Path) -> None: "--json", "/path/to/config#nixosConfigurations.hostname.config.system.build.images", "--apply", - "builtins.mapAttrs (n: v: v.passthru.filePath)", + "builtins.attrNames", ], check=True, stdout=PIPE, @@ -401,6 +396,17 @@ def test_execute_nix_build_image_flake(mock_run: Mock, tmp_path: Path) -> None: stdout=PIPE, **DEFAULT_RUN_KWARGS, ), + call( + [ + "nix", + "eval", + "--json", + "/path/to/config#nixosConfigurations.hostname.config.system.build.images.azure.passthru.filePath", + ], + check=True, + stdout=PIPE, + **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 af8c0bdfd6a7..6f615872ac43 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 @@ -353,7 +353,7 @@ def test_get_build_image_variants(mock_run: Mock, tmp_path: Path) -> None: value = import ; set = if builtins.isFunction value then value {} else value; in - builtins.mapAttrs (n: v: v.passthru.filePath) set.config.system.build.images + builtins.attrNames set.config.system.build.images """), ], stdout=PIPE, @@ -376,7 +376,7 @@ def test_get_build_image_variants(mock_run: Mock, tmp_path: Path) -> None: value = import "{tmp_path}"; set = if builtins.isFunction value then value {{}} else value; in - builtins.mapAttrs (n: v: v.passthru.filePath) set.preAttr.config.system.build.images + builtins.attrNames set.preAttr.config.system.build.images """), "--inst-flag", ], @@ -411,7 +411,7 @@ def test_get_build_image_variants_flake(mock_run: Mock) -> None: "--json", "flake.nix#myAttr.config.system.build.images", "--apply", - "builtins.mapAttrs (n: v: v.passthru.filePath)", + "builtins.attrNames", "--eval-flag", ], stdout=PIPE,