-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
nixos: systemd: split off helper functions into systemd-lib #164317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ with lib; | |
| let | ||
| cfg = config.systemd; | ||
| lndir = "${pkgs.buildPackages.xorg.lndir}/bin/lndir"; | ||
| systemd = cfg.package; | ||
| in rec { | ||
|
|
||
| shellEscape = s: (replaceChars [ "\\" ] [ "\\\\" ] s); | ||
|
|
@@ -235,4 +236,205 @@ in rec { | |
| ''} | ||
| ''; # */ | ||
|
|
||
| makeJobScript = name: text: | ||
| let | ||
| scriptName = replaceChars [ "\\" "@" ] [ "-" "_" ] (shellEscape name); | ||
| out = (pkgs.writeShellScriptBin scriptName '' | ||
| set -e | ||
| ${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}"; | ||
|
|
||
| unitConfig = { config, options, ... }: { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a module. If you put it in a file, it will be more recognizable in error messages.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fully agree with your comments. However, this PR was made to have make it easier to merge. It is the minimal amount of changes that I need to reuse systemd options for systemd in initramfs. This PR was split off from #164016. It was suggested to me to keep the PRs as small as possible to avoid long review a long review process and avoid it going out-of-date quickly. I think that is indeed the right way to go, so I'm going to keep creating smallish PRs to move towards the goal of having a cleaner systemd module as well as reusing parts in systemd.user and initramfs. For your interest, I have also tried to create individual 'submodules' for each part of the common systemd options. See bobvanderlinden@e5ad5b3. I'm also very interested in feedback on this one... though I do know it will be a long while before that can even be considered for reviewing.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'd try to avoid functions to modules; instead turning the parameters into internal options.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The internal options would be ideal. I started out with |
||
| config = { | ||
| unitConfig = | ||
| optionalAttrs (config.requires != []) | ||
| { Requires = toString config.requires; } | ||
| // optionalAttrs (config.wants != []) | ||
| { Wants = toString config.wants; } | ||
| // optionalAttrs (config.after != []) | ||
| { After = toString config.after; } | ||
| // optionalAttrs (config.before != []) | ||
| { Before = toString config.before; } | ||
| // optionalAttrs (config.bindsTo != []) | ||
| { BindsTo = toString config.bindsTo; } | ||
| // optionalAttrs (config.partOf != []) | ||
| { PartOf = toString config.partOf; } | ||
| // optionalAttrs (config.conflicts != []) | ||
| { Conflicts = toString config.conflicts; } | ||
| // optionalAttrs (config.requisite != []) | ||
| { Requisite = toString config.requisite; } | ||
| // optionalAttrs (config.restartTriggers != []) | ||
| { X-Restart-Triggers = toString config.restartTriggers; } | ||
| // optionalAttrs (config.reloadTriggers != []) | ||
| { X-Reload-Triggers = toString config.reloadTriggers; } | ||
| // optionalAttrs (config.description != "") { | ||
| Description = config.description; } | ||
| // optionalAttrs (config.documentation != []) { | ||
| Documentation = toString config.documentation; } | ||
| // optionalAttrs (config.onFailure != []) { | ||
| OnFailure = toString config.onFailure; } | ||
| // optionalAttrs (options.startLimitIntervalSec.isDefined) { | ||
| StartLimitIntervalSec = toString config.startLimitIntervalSec; | ||
| } // optionalAttrs (options.startLimitBurst.isDefined) { | ||
| StartLimitBurst = toString config.startLimitBurst; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
||
| serviceConfig = { name, config, ... }: { | ||
| config = mkMerge | ||
| [ { # Default path for systemd services. Should be quite minimal. | ||
| path = mkAfter | ||
| [ pkgs.coreutils | ||
| pkgs.findutils | ||
| pkgs.gnugrep | ||
| pkgs.gnused | ||
| systemd | ||
| ]; | ||
| environment.PATH = "${makeBinPath config.path}:${makeSearchPathOutput "bin" "sbin" config.path}"; | ||
| } | ||
| (mkIf (config.preStart != "") | ||
| { serviceConfig.ExecStartPre = | ||
| [ (makeJobScript "${name}-pre-start" config.preStart) ]; | ||
| }) | ||
| (mkIf (config.script != "") | ||
| { serviceConfig.ExecStart = | ||
| makeJobScript "${name}-start" config.script + " " + config.scriptArgs; | ||
| }) | ||
| (mkIf (config.postStart != "") | ||
| { serviceConfig.ExecStartPost = | ||
| [ (makeJobScript "${name}-post-start" config.postStart) ]; | ||
| }) | ||
| (mkIf (config.reload != "") | ||
| { serviceConfig.ExecReload = | ||
| makeJobScript "${name}-reload" config.reload; | ||
| }) | ||
| (mkIf (config.preStop != "") | ||
| { serviceConfig.ExecStop = | ||
| makeJobScript "${name}-pre-stop" config.preStop; | ||
| }) | ||
| (mkIf (config.postStop != "") | ||
| { serviceConfig.ExecStopPost = | ||
| makeJobScript "${name}-post-stop" config.postStop; | ||
| }) | ||
| ]; | ||
| }; | ||
|
|
||
| mountConfig = { config, ... }: { | ||
| config = { | ||
| mountConfig = | ||
| { What = config.what; | ||
| Where = config.where; | ||
| } // optionalAttrs (config.type != "") { | ||
| Type = config.type; | ||
| } // optionalAttrs (config.options != "") { | ||
| Options = config.options; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
||
| automountConfig = { config, ... }: { | ||
| config = { | ||
| automountConfig = | ||
| { Where = config.where; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
||
| commonUnitText = def: '' | ||
| [Unit] | ||
| ${attrsToSection def.unitConfig} | ||
| ''; | ||
|
|
||
| targetToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = | ||
| '' | ||
| [Unit] | ||
| ${attrsToSection def.unitConfig} | ||
| ''; | ||
| }; | ||
|
|
||
| serviceToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = commonUnitText def + | ||
| '' | ||
| [Service] | ||
| ${let env = cfg.globalEnvironment // def.environment; | ||
| in concatMapStrings (n: | ||
| let s = optionalString (env.${n} != null) | ||
| "Environment=${builtins.toJSON "${n}=${env.${n}}"}\n"; | ||
| # systemd max line length is now 1MiB | ||
| # https://github.com/systemd/systemd/commit/e6dde451a51dc5aaa7f4d98d39b8fe735f73d2af | ||
| in if stringLength s >= 1048576 then throw "The value of the environment variable ‘${n}’ in systemd service ‘${name}.service’ is too long." else s) (attrNames env)} | ||
| ${if def.reloadIfChanged then '' | ||
| X-ReloadIfChanged=true | ||
| '' else if !def.restartIfChanged then '' | ||
| X-RestartIfChanged=false | ||
| '' else ""} | ||
| ${optionalString (!def.stopIfChanged) "X-StopIfChanged=false"} | ||
| ${attrsToSection def.serviceConfig} | ||
| ''; | ||
| }; | ||
|
|
||
| socketToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = commonUnitText def + | ||
| '' | ||
| [Socket] | ||
| ${attrsToSection def.socketConfig} | ||
| ${concatStringsSep "\n" (map (s: "ListenStream=${s}") def.listenStreams)} | ||
| ${concatStringsSep "\n" (map (s: "ListenDatagram=${s}") def.listenDatagrams)} | ||
| ''; | ||
| }; | ||
|
|
||
| timerToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = commonUnitText def + | ||
| '' | ||
| [Timer] | ||
| ${attrsToSection def.timerConfig} | ||
| ''; | ||
| }; | ||
|
|
||
| pathToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = commonUnitText def + | ||
| '' | ||
| [Path] | ||
| ${attrsToSection def.pathConfig} | ||
| ''; | ||
| }; | ||
|
|
||
| mountToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = commonUnitText def + | ||
| '' | ||
| [Mount] | ||
| ${attrsToSection def.mountConfig} | ||
| ''; | ||
| }; | ||
|
|
||
| automountToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = commonUnitText def + | ||
| '' | ||
| [Automount] | ||
| ${attrsToSection def.automountConfig} | ||
| ''; | ||
| }; | ||
|
|
||
| sliceToUnit = name: def: | ||
| { inherit (def) aliases wantedBy requiredBy enable; | ||
| text = commonUnitText def + | ||
| '' | ||
| [Slice] | ||
| ${attrsToSection def.sliceConfig} | ||
| ''; | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function can eventually be removed when replaced by #154154
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only used in one module, so you could put it in a
letbinding there. (See the other comment about extracting to files)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to reuse these functions (or something better that can be reused). Since I'd like to move forward with systemd in initramfs I wanted to make the minimal amount of changes while still allowing me to reuse functionality.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at #154154. It looks nice, but it hasn't had activity since januari 7th. I can't wait for that one to be worked on before I can do this PR, which is needed to work on an alternative to #120015 that will be easier to review/merge. #120015 is also stalled, probably because of hard to review/merge and maybe lack of time/motivation.