From 2b224f0e3c08e45ac08a5137142bb0bbd307e3f9 Mon Sep 17 00:00:00 2001 From: r-vdp Date: Thu, 30 Nov 2023 10:54:09 +0100 Subject: [PATCH] nixos/systemd: allow using writeShellApplication for systemd unit scripts --- .../manual/release-notes/rl-2411.section.md | 9 ++++ nixos/lib/systemd-lib.nix | 29 +++++++++--- nixos/lib/systemd-unit-options.nix | 45 ++++++++++++++++--- nixos/modules/system/boot/systemd.nix | 2 + 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2411.section.md b/nixos/doc/manual/release-notes/rl-2411.section.md index b3b2af8d0778..d50dd9d2c7fb 100644 --- a/nixos/doc/manual/release-notes/rl-2411.section.md +++ b/nixos/doc/manual/release-notes/rl-2411.section.md @@ -62,6 +62,15 @@ - The OCaml-based Xen Store can now be configured using [`virtualisation.xen.store.settings`](#opt-virtualisation.xen.store.settings). - The `virtualisation.xen.bridge` options have been deprecated in this release cycle. Users who need network bridges are encouraged to set up their own networking configurations. +- A new option [`systemd.enableStrictShellChecks`](#opt-systemd.enableStrictShellChecks) has been added. When enabled, all systemd scripts generated by NixOS will + be checked with [shellcheck](https://www.shellcheck.net) and any errors or warnings will cause the build to fail. + This affects all scripts that have been created through the `script`, `reload`, `preStart`, `postStart`, `preStop` and `postStop` options for systemd services. + This does not affect commandlines passed directly to `ExecStart`, `ExecReload`, `ExecStartPre`, `ExecStartPost`, `ExecStop` or `ExecStopPost`. + It therefore also does not affect systemd units that are coming from packages and that are not defined through the NixOS config. + This option is disabled by default, and although some services have already been fixed, it is still likely that you will encounter build failures when enabling this. + We encourage people to enable this option when they are willing and able to submit fixes for potential build failures to nixpkgs. + The option can also be enabled or disabled for individual services using the `enableStrictShellChecks` option on the service itself, which will take precedence over the global setting. + ## New Modules {#sec-release-24.11-new-modules} - [TaskChampion Sync-Server](https://github.com/GothenburgBitFactory/taskchampion-sync-server), a [Taskwarrior 3](https://taskwarrior.org/docs/upgrade-3/) sync server, replacing Taskwarrior 2's sync server named [`taskserver`](https://github.com/GothenburgBitFactory/taskserver). diff --git a/nixos/lib/systemd-lib.nix b/nixos/lib/systemd-lib.nix index fedd85f09b80..c5d64c3bd4f6 100644 --- a/nixos/lib/systemd-lib.nix +++ b/nixos/lib/systemd-lib.nix @@ -386,18 +386,27 @@ in rec { ''} ''; # */ - makeJobScript = name: text: + makeJobScript = { name, text, enableStrictShellChecks }: let scriptName = replaceStrings [ "\\" "@" ] [ "-" "_" ] (shellEscape name); - out = (pkgs.writeShellScriptBin scriptName '' - set -e - ${text} - '').overrideAttrs (_: { + out = ( + if ! enableStrictShellChecks then + pkgs.writeShellScriptBin scriptName '' + set -e + + ${text} + '' + else + pkgs.writeShellApplication { + name = scriptName; + inherit text; + } + ).overrideAttrs (_: { # The derivation name is different from the script file name # to keep the script file name short to avoid cluttering logs. name = "unit-script-${scriptName}"; }); - in "${out}/bin/${scriptName}"; + in lib.getExe out; unitConfig = { config, name, options, ... }: { config = { @@ -448,10 +457,16 @@ in rec { }; }; - serviceConfig = { name, config, ... }: { + serviceConfig = + let + nixosConfig = config; + in + { name, lib, config, ... }: { config = { name = "${name}.service"; environment.PATH = mkIf (config.path != []) "${makeBinPath config.path}:${makeSearchPathOutput "bin" "sbin" config.path}"; + + enableStrictShellChecks = lib.mkOptionDefault nixosConfig.systemd.enableStrictShellChecks; }; }; diff --git a/nixos/lib/systemd-unit-options.nix b/nixos/lib/systemd-unit-options.nix index 160f2bf9483a..b02a11ef33a6 100644 --- a/nixos/lib/systemd-unit-options.nix +++ b/nixos/lib/systemd-unit-options.nix @@ -17,6 +17,7 @@ let concatMap filterOverrides isList + literalExpression mergeEqualOption mkIf mkMerge @@ -357,6 +358,14 @@ in rec { ''; }; + enableStrictShellChecks = mkOption { + type = types.bool; + description = "Enable running shellcheck on the generated scripts for this unit."; + # The default gets set in systemd-lib.nix because we don't have access to + # the full NixOS config here. + defaultText = literalExpression "config.systemd.enableStrictShellChecks"; + }; + script = mkOption { type = types.lines; default = ""; @@ -428,27 +437,51 @@ in rec { config = mkMerge [ (mkIf (config.preStart != "") rec { - jobScripts = makeJobScript "${name}-pre-start" config.preStart; + jobScripts = makeJobScript { + name = "${name}-pre-start"; + text = config.preStart; + inherit (config) enableStrictShellChecks; + }; serviceConfig.ExecStartPre = [ jobScripts ]; }) (mkIf (config.script != "") rec { - jobScripts = makeJobScript "${name}-start" config.script; + jobScripts = makeJobScript { + name = "${name}-start"; + text = config.script; + inherit (config) enableStrictShellChecks; + }; serviceConfig.ExecStart = jobScripts + " " + config.scriptArgs; }) (mkIf (config.postStart != "") rec { - jobScripts = (makeJobScript "${name}-post-start" config.postStart); + jobScripts = makeJobScript { + name = "${name}-post-start"; + text = config.postStart; + inherit (config) enableStrictShellChecks; + }; serviceConfig.ExecStartPost = [ jobScripts ]; }) (mkIf (config.reload != "") rec { - jobScripts = makeJobScript "${name}-reload" config.reload; + jobScripts = makeJobScript { + name = "${name}-reload"; + text = config.reload; + inherit (config) enableStrictShellChecks; + }; serviceConfig.ExecReload = jobScripts; }) (mkIf (config.preStop != "") rec { - jobScripts = makeJobScript "${name}-pre-stop" config.preStop; + jobScripts = makeJobScript { + name = "${name}-pre-stop"; + text = config.preStop; + inherit (config) enableStrictShellChecks; + }; serviceConfig.ExecStop = jobScripts; }) (mkIf (config.postStop != "") rec { - jobScripts = makeJobScript "${name}-post-stop" config.postStop; + jobScripts = makeJobScript { + name = "${name}-post-stop"; + text = config.postStop; + inherit (config) enableStrictShellChecks; + }; serviceConfig.ExecStopPost = jobScripts; }) ]; diff --git a/nixos/modules/system/boot/systemd.nix b/nixos/modules/system/boot/systemd.nix index 98ff9b0d88d9..6c70f50a6073 100644 --- a/nixos/modules/system/boot/systemd.nix +++ b/nixos/modules/system/boot/systemd.nix @@ -197,6 +197,8 @@ in package = mkPackageOption pkgs "systemd" {}; + enableStrictShellChecks = mkEnableOption "running shellcheck on the generated scripts for systemd units."; + units = mkOption { description = "Definition of systemd units; see {manpage}`systemd.unit(5)`."; default = {};