From 4b0d1225f5e93e794800306d0785473e20129534 Mon Sep 17 00:00:00 2001 From: Will Fancher Date: Tue, 13 May 2025 18:27:14 -0400 Subject: [PATCH 1/2] lib/asserts: Factor out NixOS's toplevel assertion / warning logic. --- lib/asserts.nix | 68 +++++++++++++++++++ nixos/modules/system/activation/top-level.nix | 9 +-- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/lib/asserts.nix b/lib/asserts.nix index b22252a7654b..41908d00a812 100644 --- a/lib/asserts.nix +++ b/lib/asserts.nix @@ -1,5 +1,16 @@ { lib }: +let + inherit (lib.strings) + concatStringsSep + ; + inherit (lib.lists) + filter + ; + inherit (lib.trivial) + showWarnings + ; +in rec { /** @@ -131,4 +142,61 @@ rec { "each element in ${name} must be one of ${lib.generators.toPretty { } xs}, but is: ${ lib.generators.toPretty { } vals }"; + + /** + Wrap a value with logic that throws an error when assertions + fail and emits any warnings. + + # Inputs + + `assertions` + + : A list of assertions. If any of their `assertion` attrs is `false`, their `message` attrs will be emitted in a `throw`. + + `warnings` + + : A list of strings to emit as warnings. This function does no filtering on this list. + + `val` + + : A value to return, wrapped in `warn`, if a `throw` is not necessary. + + # Type + + ``` + checkAssertWarn :: [ { assertion :: Bool; message :: String } ] -> [ String ] -> Any -> Any + ``` + + # Examples + :::{.example} + ## `lib.asserts.checkAssertWarn` usage example + ```nix + checkAssertWarn + [ { assertion = false; message = "Will fail"; } ] + [ ] + null + stderr> error: + stderr> Failed assertions: + stderr> - Will fail + + checkAssertWarn + [ { assertion = true; message = "Will not fail"; } ] + [ "Will warn" ] + null + stderr> evaluation warning: Will warn + null + ``` + + ::: + */ + checkAssertWarn = + assertions: warnings: val: + let + failedAssertions = map (x: x.message) (filter (x: !x.assertion) assertions); + in + if failedAssertions != [ ] then + throw "\nFailed assertions:\n${concatStringsSep "\n" (map (x: "- ${x}") failedAssertions)}" + else + showWarnings warnings val; + } diff --git a/nixos/modules/system/activation/top-level.nix b/nixos/modules/system/activation/top-level.nix index 75738bc0f5db..56e97d3fdea9 100644 --- a/nixos/modules/system/activation/top-level.nix +++ b/nixos/modules/system/activation/top-level.nix @@ -77,14 +77,7 @@ let ); # Handle assertions and warnings - - failedAssertions = map (x: x.message) (filter (x: !x.assertion) config.assertions); - - baseSystemAssertWarn = - if failedAssertions != [ ] then - throw "\nFailed assertions:\n${concatStringsSep "\n" (map (x: "- ${x}") failedAssertions)}" - else - showWarnings config.warnings baseSystem; + baseSystemAssertWarn = lib.asserts.checkAssertWarn config.assertions config.warnings baseSystem; # Replace runtime dependencies system = From 26ccfb7a8cac9719fc4c96adb8fec8d565409543 Mon Sep 17 00:00:00 2001 From: Will Fancher Date: Tue, 13 May 2025 18:27:14 -0400 Subject: [PATCH 2/2] nixos/image/repart: Use own assertions / warnings. 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. --- nixos/modules/image/repart.nix | 132 +++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 55 deletions(-) diff --git a/nixos/modules/image/repart.nix b/nixos/modules/image/repart.nix index 891bfeb30449..20ffaba4e7cc 100644 --- a/nixos/modules/image/repart.nix +++ b/nixos/modules/image/repart.nix @@ -3,6 +3,7 @@ { config, + options, pkgs, lib, utils, @@ -258,50 +259,29 @@ in ''; }; + assertions = lib.mkOption { + type = options.assertions.type; + default = [ ]; + internal = true; + visible = false; + description = '' + Assertions only evaluated by the repart image, not by the system toplevel. + ''; + }; + + warnings = lib.mkOption { + type = options.warnings.type; + default = [ ]; + internal = true; + visible = false; + description = '' + Warnings only evaluated by the repart image, not by the system toplevel. + ''; + }; + }; config = { - assertions = lib.mapAttrsToList ( - fileName: partitionConfig: - let - inherit (partitionConfig) repartConfig; - labelLength = builtins.stringLength repartConfig.Label; - in - { - assertion = repartConfig ? Label -> GPTMaxLabelLength >= labelLength; - message = '' - The partition label '${repartConfig.Label}' - defined for '${fileName}' is ${toString labelLength} characters long, - but the maximum label length supported by UEFI is ${toString GPTMaxLabelLength}. - ''; - } - ) cfg.partitions; - - warnings = lib.filter (v: v != null) ( - lib.mapAttrsToList ( - fileName: partitionConfig: - let - inherit (partitionConfig) repartConfig; - suggestedMaxLabelLength = GPTMaxLabelLength - 2; - labelLength = builtins.stringLength repartConfig.Label; - in - if (repartConfig ? Label && labelLength >= suggestedMaxLabelLength) then - '' - The partition label '${repartConfig.Label}' - defined for '${fileName}' is ${toString labelLength} characters long. - The suggested maximum label length is ${toString suggestedMaxLabelLength}. - - If you use sytemd-sysupdate style A/B updates, this might - not leave enough space to increment the version number included in - the label in a future release. For example, if your label is - ${toString GPTMaxLabelLength} characters long (the maximum enforced by UEFI) and - you're at version 9, you cannot increment this to 10. - '' - else - null - ) cfg.partitions - ); - image.baseName = let version = config.image.repart.version; @@ -352,6 +332,47 @@ in }; finalPartitions = lib.mapAttrs addClosure cfg.partitions; + + assertions = lib.mapAttrsToList ( + fileName: partitionConfig: + let + inherit (partitionConfig) repartConfig; + labelLength = builtins.stringLength repartConfig.Label; + in + { + assertion = repartConfig ? Label -> GPTMaxLabelLength >= labelLength; + message = '' + The partition label '${repartConfig.Label}' + defined for '${fileName}' is ${toString labelLength} characters long, + but the maximum label length supported by UEFI is ${toString GPTMaxLabelLength}. + ''; + } + ) cfg.partitions; + + warnings = lib.filter (v: v != null) ( + lib.mapAttrsToList ( + fileName: partitionConfig: + let + inherit (partitionConfig) repartConfig; + suggestedMaxLabelLength = GPTMaxLabelLength - 2; + labelLength = builtins.stringLength repartConfig.Label; + in + if (repartConfig ? Label && labelLength >= suggestedMaxLabelLength) then + '' + The partition label '${repartConfig.Label}' + defined for '${fileName}' is ${toString labelLength} characters long. + The suggested maximum label length is ${toString suggestedMaxLabelLength}. + + If you use sytemd-sysupdate style A/B updates, this might + not leave enough space to increment the version number included in + the label in a future release. For example, if your label is + ${toString GPTMaxLabelLength} characters long (the maximum enforced by UEFI) and + you're at version 9, you cannot increment this to 10. + '' + else + null + ) cfg.partitions + ); }; system.build.image = @@ -367,21 +388,22 @@ in ); mkfsEnv = mkfsOptionsToEnv cfg.mkfsOptions; + val = pkgs.callPackage ./repart-image.nix { + systemd = cfg.package; + imageFileBasename = config.image.baseName; + inherit (cfg) + name + version + compression + split + seed + sectorSize + finalPartitions + ; + inherit fileSystems definitionsDirectory mkfsEnv; + }; in - pkgs.callPackage ./repart-image.nix { - systemd = cfg.package; - imageFileBasename = config.image.baseName; - inherit (cfg) - name - version - compression - split - seed - sectorSize - finalPartitions - ; - inherit fileSystems definitionsDirectory mkfsEnv; - }; + lib.asserts.checkAssertWarn cfg.assertions cfg.warnings val; }; meta.maintainers = with lib.maintainers; [