Unverified Commit 113dbd79 authored by r-vdp's avatar r-vdp
Browse files

nixos/activation: improve preSwitchChecks 

A couple of improvements:

1. Avoid the generally discouraged apply argument to options, as it has
   quite weird semantics
2. Avoid issues when a user calls a preSwitchCheck `script`, which
   would've been silently overridden by the existing implementation.
   Reliance on a special attribute name like that is bound to lead to a
   very-hard-to-debug problem for someone at some point
3. Use writeShellApplication so that the preSwitchChecks are checked by
   shellcheck and and so that they run with basic bash guardrails
4. Fix shellcheck issue (testing the value of $?)
5. Add a positive preSwitchCheck to the nixos test, to make sure that
   that works as intended
parent 864f89f9

nixos/modules/system/activation/pre-switch-check.nix

+29 −22
Original line number Diff line number Diff line 
{ lib, pkgs, ... }:

{

  lib,

  config,

  pkgs,

  ...

}:

let

  preSwitchCheckScript =

    set:

    lib.concatLines (

  preSwitchCheckScript = lib.concatLines (

    lib.mapAttrsToList (name: text: ''

      # pre-switch check ${name}

        (

      if ! (

        ${text}

        )

        if [[ $? != 0 ]]; then

      ); then

        echo "Pre-switch check '${name}' failed"

        exit 1

      fi

      '') set

    '') config.system.preSwitchChecks

  );

in

{

  options.system.preSwitchChecksScript = lib.mkOption {

    type = lib.types.pathInStore;

    internal = true;

    readOnly = true;

    default = lib.getExe (

      pkgs.writeShellApplication {

        name = "pre-switch-checks";

        text = preSwitchCheckScript;

      }

    );

  };



  options.system.preSwitchChecks = lib.mkOption {

    default = { };

    example = lib.literalExpression ''
@@ -33,12 +47,5 @@ in 
    '';



    type = lib.types.attrsOf lib.types.str;



    apply =

      set:

      set

      // {

        script = pkgs.writeShellScript "pre-switch-checks" (preSwitchCheckScript set);

      };

  };

}

nixos/modules/system/activation/switchable-system.nix

+2 −2
Original line number Diff line number Diff line
@@ -61,7 +61,7 @@ in 
          --subst-var-by coreutils "${pkgs.coreutils}" \

          --subst-var-by distroId ${lib.escapeShellArg config.system.nixos.distroId} \

          --subst-var-by installBootLoader ${lib.escapeShellArg config.system.build.installBootLoader} \

          --subst-var-by preSwitchCheck ${lib.escapeShellArg config.system.preSwitchChecks.script} \

          --subst-var-by preSwitchCheck ${lib.escapeShellArg config.system.preSwitchChecksScript} \

          --subst-var-by localeArchive "${config.i18n.glibcLocales}/lib/locale/locale-archive" \

          --subst-var-by perl "${perlWrapped}" \

          --subst-var-by shell "${pkgs.bash}/bin/sh" \
@@ -94,7 +94,7 @@ in 
            --set TOPLEVEL ''${!toplevelVar} \

            --set DISTRO_ID ${lib.escapeShellArg config.system.nixos.distroId} \

            --set INSTALL_BOOTLOADER ${lib.escapeShellArg config.system.build.installBootLoader} \

            --set PRE_SWITCH_CHECK ${lib.escapeShellArg config.system.preSwitchChecks.script} \

            --set PRE_SWITCH_CHECK ${lib.escapeShellArg config.system.preSwitchChecksScript} \

            --set LOCALE_ARCHIVE ${config.i18n.glibcLocales}/lib/locale/locale-archive \

            --set SYSTEMD ${config.systemd.package}

        )

nixos/modules/system/activation/top-level.nix

+1 −1
Original line number Diff line number Diff line
@@ -343,7 +343,7 @@ in 
      perl = pkgs.perl.withPackages (p: with p; [ ConfigIniFiles FileSlurp ]);

      # End if legacy environment variables



      preSwitchCheck = config.system.preSwitchChecks.script;

      preSwitchCheck = config.system.preSwitchChecksScript;



      # Not actually used in the builder. `passedChecks` is just here to create

      # the build dependencies. Checks are similar to build dependencies in the

nixos/tests/switch-test.nix

+4 −0
Original line number Diff line number Diff line
@@ -611,6 +611,10 @@ in { 
    other = {

      system.switch.enable = true;

      users.mutableUsers = true;

      system.preSwitchChecks.succeeds = ''

        echo this will succeed

        true

      '';

      specialisation.failingCheck.configuration.system.preSwitchChecks.failEveryTime = ''

        echo this will fail

        false