From 1ea32d4f698230e2c1e765d857c44b17daa377b4 Mon Sep 17 00:00:00 2001 From: zimbatm Date: Sun, 1 Sep 2024 12:06:30 +0200 Subject: [PATCH] nixos/grub: fix value precendence with optional -> mkIf When using `lib.optionals`, the return value of both branches of the condition get set as a value to the option. When using `lib.mkIf`, only the positive condition gets set as a value to the option. This small distinction is important when dealing with precedence. For example here, we wanted to set a boot.grub.devices default value with lib.mkDefault, and that was getting overridden with the empty value of `lib.optional (cfg.device != "") cfg.device`. See https://github.com/nix-community/srvos/pull/491#discussion_r1738827651 The general conclusion is that using `lib.mkIf` is preferable to `lib.optional` or `lib.optionals` when setting values in the NixOS module system. --- nixos/modules/system/boot/loader/grub/grub.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nixos/modules/system/boot/loader/grub/grub.nix b/nixos/modules/system/boot/loader/grub/grub.nix index 9c36651d6874..81f94e8639d9 100644 --- a/nixos/modules/system/boot/loader/grub/grub.nix +++ b/nixos/modules/system/boot/loader/grub/grub.nix @@ -712,9 +712,9 @@ in (mkIf cfg.enable { - boot.loader.grub.devices = optional (cfg.device != "") cfg.device; + boot.loader.grub.devices = mkIf (cfg.device != "") [ cfg.device ]; - boot.loader.grub.mirroredBoots = optionals (cfg.devices != [ ]) [ + boot.loader.grub.mirroredBoots = mkIf (cfg.devices != [ ]) [ { path = "/boot"; inherit (cfg) devices; inherit (efi) efiSysMountPoint; } ]; @@ -752,7 +752,7 @@ in # set at once. system.boot.loader.id = "grub"; - environment.systemPackages = optional (grub != null) grub; + environment.systemPackages = mkIf (grub != null) [ grub ]; boot.loader.grub.extraPrepareConfig = concatStrings (mapAttrsToList (n: v: ''