nixos/image/repart: Use own assertions / warnings.#406940
nixos/image/repart: Use own assertions / warnings.#406940arianvp merged 2 commits intoNixOS:masterfrom
Conversation
It was easy to accidentally trigger infinite recursion if you depended on `toplevel` in any way before. For instance, if you used `CopyBlocks` with an image containing `toplevel`. This was because `toplevel`'s assertion / warning logic has to be evaluated, but that means evaluating `image.repart`'s assertions / warnings, which requires evaluating the `repartConfig` attrsets to check for malformed `Label`s. That causes the module system to type check *all* `repartConfig` keys, even though most of them aren't used in the assertions / warnings. So evaluating `system.build.image` evaluates `repartConfig.CopyBlocks`, which evaluates `toplevel`, which evaluates assertions / warnings, which evaluates `repartConfig.CopyBlocks` to type check it. Infinite loop. Even ignoring this recursion problem, it's still better for the repart module to have its own assertions / warnings options. You don't have to use `toplevel` in a repart image, so its assertions / warnings would have been ignored in that case anyway. This way they're *always* checked when you build an image.
WilliButz
left a comment
There was a problem hiding this comment.
You don't have
to usetoplevelin a repart image, so its assertions / warnings
would have been ignored in that case anyway. This way they're always
checked when you build an image.
I agree, this is very nice!
|
A similar solution was previously discussed. I believe this is worth looking into again, particularly now, as we can "avoid" adding a It feels unfortunate that we're doing this right before we have the means, ... to implement properly: ... but we have to see users' reality for what it is, and |
|
I'm fine with reverting this and cooking the library functions first. There is no time pressure here. At least not from my side. |
|
Thanks. |
This fixes some infinite recursion problems that are easy to trigger with the repart image builder. See commit message for details.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.