From 76c618ecddf2b7bafc5805282972fa1036717211 Mon Sep 17 00:00:00 2001 From: Thiago Kenji Okada Date: Mon, 22 Sep 2025 11:10:18 +0100 Subject: [PATCH] nixos-rebuild-ng: do not parse the path part from Flake as a Path When I first wrote the Flake code for `nixos-rebuild-ng`, I set the Flake parser to normalize the `foo` part from `foo#host` as a Python's `pathlib.Path` if this looked like a path. The idea originally was to improve type-safety and make it easier to compose the path later if needed. But, in retrospect this is the wrong thing to do here because there are so many things that `nix` can do in those cases: - `foo` can be either a directory or a Flake registry - #433726 So let's stop being smart here and just assume a plain string. We don't really get a lot of type-safety or any of the nice things pathlib gives to us since there isn't any path manipulation going on. Let `nix` itself handle any translation here. Fix: #433726. --- .../src/nixos_rebuild/models.py | 13 ++------ .../nixos-rebuild-ng/src/tests/test_models.py | 31 +++++++++---------- .../ni/nixos-rebuild-ng/src/tests/test_nix.py | 4 +-- 3 files changed, 20 insertions(+), 28 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 d804a1c07ad6..19484f7e36e1 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 @@ -77,7 +77,7 @@ def _get_hostname(target_host: Remote | None) -> str | None: @dataclass(frozen=True) class Flake: - path: Path | str + path: str attr: str _re: ClassVar = re.compile(r"^(?P[^\#]*)\#?(?P[^\#\"]*)$") @@ -86,11 +86,7 @@ class Flake: @override def __str__(self) -> str: - if isinstance(self.path, Path): - # https://github.com/NixOS/nixpkgs/issues/433726 - return f"{self.path.absolute()}#{self.attr}" - else: - return f"{self.path}#{self.attr}" + return f"{self.path}#{self.attr}" @classmethod def parse(cls, flake_str: str, target_host: Remote | None = None) -> Self: @@ -101,10 +97,7 @@ class Flake: f'nixosConfigurations."{attr or _get_hostname(target_host) or "default"}"' ) path = m.group("path") - if ":" in path: - return cls(path, nixos_attr) - else: - return cls(Path(path), nixos_attr) + return cls(path, nixos_attr) @classmethod def from_arg(cls, flake_arg: Any, target_host: Remote | None) -> Self | None: # noqa: ANN401 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 0663d73ef538..4b7f16570c5e 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 @@ -33,10 +33,10 @@ def test_build_attr_to_attr() -> None: @patch("platform.node", autospec=True, return_value=None) def test_flake_parse(mock_node: Mock, tmpdir: Path, monkeypatch: MonkeyPatch) -> None: assert m.Flake.parse("/path/to/flake#attr") == m.Flake( - Path("/path/to/flake"), 'nixosConfigurations."attr"' + "/path/to/flake", 'nixosConfigurations."attr"' ) assert m.Flake.parse("/path/ to /flake") == m.Flake( - Path("/path/ to /flake"), 'nixosConfigurations."default"' + "/path/ to /flake", 'nixosConfigurations."default"' ) with patch( get_qualified_name(m.run_wrapper, m), @@ -45,11 +45,11 @@ def test_flake_parse(mock_node: Mock, tmpdir: Path, monkeypatch: MonkeyPatch) -> ): target_host = m.Remote("target@remote", [], None) assert m.Flake.parse("/path/to/flake", target_host) == m.Flake( - Path("/path/to/flake"), 'nixosConfigurations."remote"' + "/path/to/flake", 'nixosConfigurations."remote"' ) - assert m.Flake.parse(".#attr") == m.Flake(Path("."), 'nixosConfigurations."attr"') - assert m.Flake.parse("#attr") == m.Flake(Path("."), 'nixosConfigurations."attr"') - assert m.Flake.parse(".") == m.Flake(Path("."), 'nixosConfigurations."default"') + assert m.Flake.parse(".#attr") == m.Flake(".", 'nixosConfigurations."attr"') + assert m.Flake.parse("#attr") == m.Flake("", 'nixosConfigurations."attr"') + assert m.Flake.parse(".") == m.Flake(".", 'nixosConfigurations."default"') assert m.Flake.parse("path:/to/flake#attr") == m.Flake( "path:/to/flake", 'nixosConfigurations."attr"' ) @@ -64,7 +64,7 @@ def test_flake_parse(mock_node: Mock, tmpdir: Path, monkeypatch: MonkeyPatch) -> def test_flake_to_attr() -> None: assert ( - m.Flake(Path("/path/to/flake"), "nixosConfigurations.preAttr").to_attr( + m.Flake("/path/to/flake", "nixosConfigurations.preAttr").to_attr( "attr1", "attr2" ) == "/path/to/flake#nixosConfigurations.preAttr.attr1.attr2" @@ -73,10 +73,9 @@ def test_flake_to_attr() -> None: def test_flake__str__(monkeypatch: MonkeyPatch, tmpdir: Path) -> None: assert str(m.Flake("github:nixos/nixpkgs", "attr")) == "github:nixos/nixpkgs#attr" - assert str(m.Flake(Path("/etc/nixos"), "attr")) == "/etc/nixos#attr" - with monkeypatch.context() as patch_context: - patch_context.chdir(tmpdir) - assert str(m.Flake(Path("."), "attr")) == f"{tmpdir}#attr" + assert str(m.Flake("/etc/nixos", "attr")) == "/etc/nixos#attr" + assert str(m.Flake(".", "attr")) == ".#attr" + assert str(m.Flake("", "attr")) == "#attr" @patch("platform.node", autospec=True) @@ -87,7 +86,7 @@ def test_flake_from_arg( # Flake string assert m.Flake.from_arg("/path/to/flake#attr", None) == m.Flake( - Path("/path/to/flake"), 'nixosConfigurations."attr"' + "/path/to/flake", 'nixosConfigurations."attr"' ) # False @@ -97,7 +96,7 @@ def test_flake_from_arg( with monkeypatch.context() as patch_context: patch_context.chdir(tmpdir) assert m.Flake.from_arg(True, None) == m.Flake( - Path("."), 'nixosConfigurations."hostname"' + ".", 'nixosConfigurations."hostname"' ) # None when we do not have /etc/nixos/flake.nix @@ -122,7 +121,7 @@ def test_flake_from_arg( ), ): assert m.Flake.from_arg(None, None) == m.Flake( - Path("/etc/nixos"), 'nixosConfigurations."hostname"' + "/etc/nixos", 'nixosConfigurations."hostname"' ) with ( @@ -138,7 +137,7 @@ def test_flake_from_arg( ), ): assert m.Flake.from_arg(None, None) == m.Flake( - Path("/path/to"), 'nixosConfigurations."hostname"' + "/path/to", 'nixosConfigurations."hostname"' ) with ( @@ -149,7 +148,7 @@ def test_flake_from_arg( ), ): assert m.Flake.from_arg("/path/to", m.Remote("user@host", [], None)) == m.Flake( - Path("/path/to"), 'nixosConfigurations."remote-hostname"' + "/path/to", 'nixosConfigurations."remote-hostname"' ) 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 a61004a60d6a..6cc525410fe4 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 @@ -385,7 +385,7 @@ def test_get_build_image_variants(mock_run: Mock, tmp_path: Path) -> None: ), ) def test_get_build_image_variants_flake(mock_run: Mock) -> None: - flake = m.Flake(Path("/flake.nix"), "myAttr") + flake = m.Flake("/flake.nix", "myAttr") assert n.get_build_image_variants_flake(flake, {"eval_flag": True}) == { "azure": "nixos-image-azure-25.05.20250102.6df2492-x86_64-linux.vhd", "vmware": "nixos-image-vmware-25.05.20250102.6df2492-x86_64-linux.vmdk", @@ -574,7 +574,7 @@ def test_repl(mock_run: Mock) -> None: @patch(get_qualified_name(n.run_wrapper, n), autospec=True) def test_repl_flake(mock_run: Mock) -> None: - n.repl_flake(m.Flake(Path("flake.nix"), "myAttr"), {"nix_flag": True}) + n.repl_flake(m.Flake("flake.nix", "myAttr"), {"nix_flag": True}) # See nixos-rebuild-ng.tests.repl for a better test, # this is mostly for sanity check assert mock_run.call_count == 1