reproduce infinite recursion with runNixOSTest

[steveej]

this reproduces the strange behavior uncovered by #55.

i think it's a bug.

cc @phaer

[phaer]

Thank you very much for the reproducer!

I don't think this is related to #50, as it's also reproducible if i drop that commit from my local branch. I'll look into it a bit further after lunch, but the issue looks like it be in our checks loader?
We get the following where we would except a derivation:

nix-repl> :lf .
Added 25 variables.

nix-repl> :p checks.x86_64-linux.nixos-test
{
  path = "/nix/store/hz5qd5f970m5ly8xi2bwjvqp639w24v1-source/checks/nixos-test.nix";
  type = "regular";
}

[steveej]

I don't think this is related to #50, as it's also reproducible if i drop that commit from my local branch.

i'm curious which commit you dropped as i branched off of main for this PR.

We get the following where we would except a derivation:

nix-repl> :p checks.x86_64-linux.nixos-test
{
  path = "/nix/store/hz5qd5f970m5ly8xi2bwjvqp639w24v1-source/checks/nixos-test.nix";
  type = "regular";
}

this is a symptom of the bug that is supposedly fixed in #55. if you merge that branch into here then you'd get the infinite recursion.

or, on this branch, you can reproduce it by building the check that is exposed via the package passthru:

nix-repl> :p checks.x86_64-linux.pkgs-default-repro
error:
       … while evaluating the attribute 'drvPath'
         at /nix/store/cb1gs888vfqxawvc65q1dk6jzbayh3wz-source/lib/customisation.nix:365:7:
          364|     in commonAttrs // {
          365|       drvPath = assert condition; drv.drvPath;
             |       ^
          366|       outPath = assert condition; drv.outPath;

       … while calling the 'derivationStrict' builtin
         at <nix/derivation-internal.nix>:34:12:
           33|
           34|   strict = derivationStrict drvAttrs;
             |            ^
           35|

       … while evaluating the option `nodes.machine._module.freeformType':

       … while evaluating the module argument `pkgs' in ":anon-1619:anon-1:anon-1":

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: infinite recursion encountered
       at /nix/store/cb1gs888vfqxawvc65q1dk6jzbayh3wz-source/lib/modules.nix:508:28:
          507|         builtins.addErrorContext (context name)
          508|           (args.${name} or config._module.args.${name})
             |                            ^
          509|       ) (lib.functionArgs f);

[steveej]

very curious find in db234a5. the usage of pkgs.lib in the nixos module that's used by the nixos test causes an infinite recursion. whereas using lib works.

[phaer]

i'm curious which commit you dropped as i branched off of main for this PR.

I had dropped 3da8b35, as I thought you had originally pinged me because it might be related to #50. On a second reading, you didn't really suggest that.

[phaer]

I suspect the root cause is that we don't pass specialArgs to the nixos eval in runNixosTest. EDIT: this was wrong

[steveej]

I suspect the root cause is that we don't pass specialArgs to the nixos eval in runNixosTest.

there may be missing arguments indeed because of that. however, to me that doesn't explain why the pkgs.lib.mkIf call causes an infinite recursion. i just minimized this PR to focus on just this aspect.

[phaer]

This took me a while to understand, as I followed a few false leads, but I think in the end it's rather simple.

Minimal example (in a repl with nixpkgs loaded)

• pkgs.nixos ({pkgs, lib, ...}: {config = pkgs.lib.mkIf true {};}) => infinite recursion
• pkgs.nixos ({pkgs, lib, ...}: {config = lib.mkIf true {};} ) => works
• pkgs.nixos ({pkgs, lib, ...}: {config.environment.systemPackages = pkgs.lib.mkIf true [];}) => works

Thinking about it, it makes sense to me as pkgs in a nixos evaluation can be set/overriden via config._module.args. So nix needs to evaluate (the top-level of) config before it knows what the final value of pkgs can be. With that in mind, it seems clear how the first example must lead to infinite recursion, I think?

[phaer]

The problem in the new reproducer has a similar root-cause in how ._module.args is handled. As it's a part of config it's only usable after imports have been resolved.

So the problem here is that you are trying to import a module from flake while flake is passed via _module.args, but in order to know the final value for that all imports have to be resolved first -> infinite recursion.

So in your example you could just remove flake from your module args and use rely on the flake arg you get from the top-level package function.

If you need to refer from modules in other files, it might be easiest to use the wrapping-function around modules added in #50.

If you need the flake as a module arg in all your modules, I think you would need an entrypoint for nixosTest which allows you to pass specialArgs, which doesn't have the same limitations as _module.args has w.r.t. to imports.

[steveej]

thank you for pointing out the right lead!

i've got to a point where i got my use-case working, and i demonstrated it in this PR.

i believe i did find a minor bug along the way: perSystem isn't found in injectPublisherArgs. i'm going to open an issue or PR for this separately.

So in your example you could just remove flake from your module args and use rely on the flake arg you get from the top-level package function.

this works for inline modules, however the composable solution is to be able to reuse modules in modules/nixos, and have them access the blueprint arguments. this works with the current code. it would be beneficial for blueprint users to document how to write nixos modules that use the plubisherArgs, and thereby have access to blueprint specific args. see this exmaple: https://github.com/numtide/blueprint/pull/56/files#diff-8726adc3e0e1d16ba6b6890a9f254d89e48083c898eff54113d6c477e19a5135. the nice thing is that they are still reusable via the flake.nixosModules output transparently.