From d4fa480262a994dadaeca1eb4ea379157d6ba91b Mon Sep 17 00:00:00 2001 From: Jonathan Thiessen Date: Mon, 24 Mar 2025 10:26:29 -0700 Subject: [PATCH 1/4] Add overlapping (consistent) flake cache insert test * Additionally, update `insert`'s input type hint to support None values (as they are already selectable and (one shot) insertable). This is necessary to appease the linter wrt the added test. --- pkgs/clan-cli/clan_cli/flake.py | 4 +++- pkgs/clan-cli/tests/test_flake_caching.py | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/pkgs/clan-cli/clan_cli/flake.py b/pkgs/clan-cli/clan_cli/flake.py index 39614c378..6dbb9910f 100644 --- a/pkgs/clan-cli/clan_cli/flake.py +++ b/pkgs/clan-cli/clan_cli/flake.py @@ -154,7 +154,9 @@ class FlakeCacheEntry: self.value = value def insert( - self, value: str | float | dict[str, Any] | list[Any], selectors: list[Selector] + self, + value: str | float | dict[str, Any] | list[Any] | None, + selectors: list[Selector], ) -> None: selector: Selector if selectors == []: diff --git a/pkgs/clan-cli/tests/test_flake_caching.py b/pkgs/clan-cli/tests/test_flake_caching.py index 56c04e56c..858bcedc3 100644 --- a/pkgs/clan-cli/tests/test_flake_caching.py +++ b/pkgs/clan-cli/tests/test_flake_caching.py @@ -17,6 +17,14 @@ def test_select() -> None: assert not test_cache.is_cached(["x", "z", 1]) +def test_insert() -> None: + test_cache = FlakeCacheEntry({}, []) + # Inserting the same thing twice should succeed + test_cache.insert(None, ["nix"]) + test_cache.insert(None, ["nix"]) + assert test_cache.select(["nix"]) is None + + def test_out_path() -> None: testdict = {"x": {"y": [123, 345, 456], "z": "/nix/store/bla"}} test_cache = FlakeCacheEntry(testdict, []) From 3c0c2ce9d6fd010fdba50d6520344a9c09b8f20e Mon Sep 17 00:00:00 2001 From: Jonathan Thiessen Date: Mon, 24 Mar 2025 09:57:06 -0700 Subject: [PATCH 2/4] Fix cached None support in FlakeCacheEntry Previously, you could cache None values; however, insertion wasn't idempotent/identical reinsertion would lead to errors due to missing None checks. --- pkgs/clan-cli/clan_cli/flake.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkgs/clan-cli/clan_cli/flake.py b/pkgs/clan-cli/clan_cli/flake.py index 6dbb9910f..e23d012b4 100644 --- a/pkgs/clan-cli/clan_cli/flake.py +++ b/pkgs/clan-cli/clan_cli/flake.py @@ -246,6 +246,12 @@ class FlakeCacheEntry: if self.value != value: msg = "value mismatch in cache, something is fishy" raise TypeError(msg) + + elif value is None: + if self.value is not None: + msg = "value mismatch in cache, something is fishy" + raise TypeError(msg) + else: msg = f"Cannot insert value of type {type(value)} into cache" raise TypeError(msg) From ea7cfc350a6e6d0e0e3cb522051726e1524f11cc Mon Sep 17 00:00:00 2001 From: Jonathan Thiessen Date: Mon, 24 Mar 2025 11:22:20 -0700 Subject: [PATCH 3/4] Add dependent vars generator dynamic validation test --- pkgs/clan-cli/tests/test_vars.py | 72 ++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/pkgs/clan-cli/tests/test_vars.py b/pkgs/clan-cli/tests/test_vars.py index 12123e8ab..f084b7af6 100644 --- a/pkgs/clan-cli/tests/test_vars.py +++ b/pkgs/clan-cli/tests/test_vars.py @@ -919,3 +919,75 @@ def test_invalidation( str(machine.flake.path), machine.name, "my_generator/my_value" ).printable_value assert value2 == value2_new + + +@pytest.mark.with_core +def test_dynamic_invalidation( + monkeypatch: pytest.MonkeyPatch, + flake: ClanFlake, +) -> None: + gen_prefix = "config.clan.core.vars.generators" + + machine = Machine(name="my_machine", flake=Flake(str(flake.path))) + + config = flake.machines[machine.name] + config["nixpkgs"]["hostPlatform"] = "x86_64-linux" + + my_generator = config["clan"]["core"]["vars"]["generators"]["my_generator"] + my_generator["files"]["my_value"]["secret"] = False + my_generator["script"] = "echo -n $RANDOM > $out/my_value" + + dependent_generator = config["clan"]["core"]["vars"]["generators"][ + "dependent_generator" + ] + dependent_generator["files"]["my_value"]["secret"] = False + dependent_generator["dependencies"] = ["my_generator"] + dependent_generator["script"] = "echo -n $RANDOM > $out/my_value" + + flake.refresh() + + # this is an abuse + custom_nix = flake.path / "machines" / machine.name / "hardware-configuration.nix" + custom_nix.write_text(""" + { config, ... }: let + p = config.clan.core.vars.generators.my_generator.files.my_value.path; + in { + clan.core.vars.generators.dependent_generator.validation = if builtins.pathExists p then builtins.readFile p else null; + } + """) + + flake.refresh() + machine.flush_caches() + monkeypatch.chdir(flake.path) + + # before generating, dependent generator validation should be empty; see bogus hardware-configuration.nix above + # we have to avoid `*.files.value` in this initial select because the generators haven't been run yet + generators_0 = machine.eval_nix(f"{gen_prefix}.*.{{validationHash}}") + assert generators_0["dependent_generator"]["validationHash"] is None + + # generate both my_generator and (the dependent) dependent_generator + cli.run(["vars", "generate", "--flake", str(flake.path), machine.name]) + machine.flush_caches() + + # after generating once, dependent generator validation should be set + generators_1 = machine.eval_nix(gen_prefix) + assert generators_1["dependent_generator"]["validationHash"] is not None + + # after generating once, neither generator should want to run again because `clan vars generate` should have re-evaluated the dependent generator's validationHash after executing the parent generator but before executing the dependent generator + # this ensures that validation can depend on parent generators while still only requiring a single pass + cli.run(["vars", "generate", "--flake", str(flake.path), machine.name]) + machine.flush_caches() + + generators_2 = machine.eval_nix(gen_prefix) + assert ( + generators_1["dependent_generator"]["validationHash"] + == generators_2["dependent_generator"]["validationHash"] + ) + assert ( + generators_1["my_generator"]["files"]["my_value"]["value"] + == generators_2["my_generator"]["files"]["my_value"]["value"] + ) + assert ( + generators_1["dependent_generator"]["files"]["my_value"]["value"] + == generators_2["dependent_generator"]["files"]["my_value"]["value"] + ) From 89379f103a87c4e63828f4966977a2c242e8da04 Mon Sep 17 00:00:00 2001 From: Jonathan Thiessen Date: Mon, 24 Mar 2025 11:24:46 -0700 Subject: [PATCH 4/4] Make `Generator`'s `validation` dynamic * Switch `Generator`'s `validation` from a regular property to an `@property` annotated method backed by `Machine`'s `eval_nix()`. * Ensure that `Machine`'s flake cache is flushed after each effectful generator execution (rather than only after all generators have been executed). --- pkgs/clan-cli/clan_cli/vars/generate.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/vars/generate.py b/pkgs/clan-cli/clan_cli/vars/generate.py index 57ea6607d..c2fc311b1 100644 --- a/pkgs/clan-cli/clan_cli/vars/generate.py +++ b/pkgs/clan-cli/clan_cli/vars/generate.py @@ -39,7 +39,6 @@ class Generator: name: str files: list[Var] = field(default_factory=list) share: bool = False - validation: str | None = None prompts: list[Prompt] = field(default_factory=list) dependencies: list[str] = field(default_factory=list) @@ -62,7 +61,6 @@ class Generator: name=data["name"], share=data["share"], files=[Var.from_json(data["name"], f) for f in data["files"].values()], - validation=data["validationHash"], dependencies=data["dependencies"], migrate_fact=data["migrateFact"], prompts=[Prompt.from_json(p) for p in data["prompts"].values()], @@ -76,6 +74,13 @@ class Generator: ) return final_script + @property + def validation(self) -> str | None: + assert self._machine is not None + return self._machine.eval_nix( + f'config.clan.core.vars.generators."{self.name}".validationHash' + ) + def bubblewrap_cmd(generator: str, tmpdir: Path) -> list[str]: test_store = nix_test_store() @@ -253,6 +258,8 @@ def execute_generator( machine.flake_dir, f"Update vars via generator {generator.name} for machine {machine.name}", ) + if len(files_to_commit) > 0: + machine.flush_caches() def _ask_prompts( @@ -456,8 +463,6 @@ def generate_vars_for_machine( public_vars_store=machine.public_vars_store, prompt_values=_ask_prompts(generator), ) - # flush caches to make sure the new secrets are available in evaluation - machine.flush_caches() return True