From d88b9464b06021f35e35c30f880a989af58b7f92 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 19 Aug 2025 22:22:12 +0200 Subject: [PATCH] system.services: Remove ambiguous, redundant pkgs module argument Primary reasons: remove implicit dependencies and force uniformity. See nixos/modules/system/service/README.md for detailed rationale. --- nixos/README-modular-services.md | 6 +- .../manual/development/modular-services.md | 4 +- nixos/modules/system/service/README.md | 90 +++++++++++++++++++ .../system/service/portable/config-data.nix | 5 +- .../system/service/portable/service.nix | 1 - .../modules/system/service/systemd/system.nix | 11 +-- .../python-http-server.nix | 20 +---- nixos/tests/modular-service-etc/test.nix | 30 ++++++- nixos/tests/php/fpm-modular.nix | 2 + pkgs/by-name/gh/ghostunnel/package.nix | 8 +- pkgs/by-name/gh/ghostunnel/service.nix | 10 ++- pkgs/development/interpreters/php/generic.nix | 8 +- pkgs/development/interpreters/php/service.nix | 20 +++-- 13 files changed, 174 insertions(+), 41 deletions(-) diff --git a/nixos/README-modular-services.md b/nixos/README-modular-services.md index 6bb5c83626f5..f6e933173b2c 100644 --- a/nixos/README-modular-services.md +++ b/nixos/README-modular-services.md @@ -51,6 +51,10 @@ A [`_class`](https://nixos.org/manual/nixpkgs/unstable/#module-system-lib-evalMo Provide it as the first attribute in the module: ```nix +# Non-module dependencies (`importApply`) +{ writeScript, runtimeShell }: + +# Service module { lib, config, ... }: { _class = "service"; @@ -87,7 +91,7 @@ stdenv.mkDerivation (finalAttrs: { passthru = { services = { default = { - imports = [ ./service.nix ]; + imports = [ (lib.modules.importApply ./service.nix { inherit pkgs; }) ]; example.package = finalAttrs.finalPackage; # ... }; diff --git a/nixos/doc/manual/development/modular-services.md b/nixos/doc/manual/development/modular-services.md index 963da8f32b0b..c616b51fa933 100644 --- a/nixos/doc/manual/development/modular-services.md +++ b/nixos/doc/manual/development/modular-services.md @@ -85,7 +85,9 @@ Moving their logic into separate Nix files may still be beneficial for the effic ## Writing and Reviewing a Modular Service {#modular-service-review} -Refer to the contributor documentation in [`nixos/README-modular-services.md`](https://github.com/NixOS/nixpkgs/blob/master/nixos/README-modular-services.md). +A typical service module consists of the following: + +For more details, refer to the contributor documentation in [`nixos/README-modular-services.md`](https://github.com/NixOS/nixpkgs/blob/master/nixos/README-modular-services.md). ## Portable Service Options {#modular-service-options-portable} diff --git a/nixos/modules/system/service/README.md b/nixos/modules/system/service/README.md index 667abb511f52..fa4d0204611e 100644 --- a/nixos/modules/system/service/README.md +++ b/nixos/modules/system/service/README.md @@ -53,3 +53,93 @@ Many services implement automatic reloading or reloading on e.g. `SIGUSR1`, but - **Simple attribute structure**: Unlike `environment.etc`, `configData` uses a simpler structure with just `enable`, `name`, `text`, `source`, and `path` attributes. Complex ownership options were omitted for simplicity and portability. Per-service user creation is still TBD. + +## No `pkgs` module argument + +The modular service infrastructure avoids exposing `pkgs` as a module argument to service modules. Instead, derivations and builder functions are provided through lexical closure, making dependency relationships explicit and avoiding uncertainty about where dependencies come from. + +### Benefits + +- **Explicit dependencies**: Services declare what they need rather than implicitly depending on `pkgs` +- **No interference**: Service modules can be reused in different contexts without assuming a specific `pkgs` instance. An unexpected `pkgs` version is not a failure mode anymore. +- **Clarity**: With fewer ways to do things, there's no ambiguity about where dependencies come from (from the module, not the OS or service manager) + +### Implementation + +- **Portable layer**: Service modules in `portable/` do not receive `pkgs` as a module argument. Any required derivations must be provided by the caller. + +- **Systemd integration**: The `systemd/system.nix` module imports `config-data.nix` as a function, providing `pkgs` in lexical closure: + ```nix + (import ../portable/config-data.nix { inherit pkgs; }) + ``` + +- **Service modules**: + 1. Should explicitly declare their package dependencies as options rather than using `pkgs` defaults: + ```nix + { + # Bad: uses pkgs module argument + foo.package = mkOption { + default = pkgs.python3; + # ... + }; + } + ``` + + ```nix + { + # Good: caller provides the package + foo.package = mkOption { + type = types.package; + description = "Python package to use"; + defaultText = lib.literalMD "The package that provided this module."; + }; + } + ``` + + 2. `passthru.services` can still provide a complete module using the package's lexical scope, making the module truly self-contained: + + **Package (`package.nix`):** + ```nix + { + lib, + writeScript, + runtimeShell, + # ... other dependencies + }: + stdenv.mkDerivation (finalAttrs: { + # ... package definition + + passthru.services.default = { + imports = [ + (lib.modules.importApply ./service.nix { + inherit writeScript runtimeShell; + }) + ]; + someService.package = finalAttrs.finalPackage; + }; + }) + ``` + + **Service module (`service.nix`):** + ```nix + # Non-module dependencies (importApply) + { writeScript, runtimeShell }: + + # Service module + { + lib, + config, + options, + ... + }: + { + # Service definition using writeScript, runtimeShell from lexical scope + process.argv = [ + (writeScript "wrapper" '' + #!${runtimeShell} + # ... wrapper logic + '') + # ... other args + ]; + } + ``` diff --git a/nixos/modules/system/service/portable/config-data.nix b/nixos/modules/system/service/portable/config-data.nix index 84c4d53452a9..63f570140b55 100644 --- a/nixos/modules/system/service/portable/config-data.nix +++ b/nixos/modules/system/service/portable/config-data.nix @@ -1,10 +1,13 @@ # Tests in: ../../../../tests/modular-service-etc/test.nix + +# Non-modular context provided by the modular services integration. +{ pkgs }: + # Configuration data support for portable services # This module provides configData for services, enabling configuration reloading # without terminating and restarting the service process. { lib, - pkgs, ... }: let diff --git a/nixos/modules/system/service/portable/service.nix b/nixos/modules/system/service/portable/service.nix index 339a99d7a856..9b9bce4082f2 100644 --- a/nixos/modules/system/service/portable/service.nix +++ b/nixos/modules/system/service/portable/service.nix @@ -11,7 +11,6 @@ in _class = "service"; imports = [ ../../../misc/assertions.nix - ./config-data.nix ]; options = { services = mkOption { diff --git a/nixos/modules/system/service/systemd/system.nix b/nixos/modules/system/service/systemd/system.nix index 139eb8398751..978c59222e9c 100644 --- a/nixos/modules/system/service/systemd/system.nix +++ b/nixos/modules/system/service/systemd/system.nix @@ -73,16 +73,16 @@ in modules = [ ./service.nix ./config-data-path.nix + (lib.modules.importApply ../portable/config-data.nix { inherit pkgs; }) - # TODO: Consider removing pkgs. Service modules can provide their own - # dependencies. { # Extend portable services option options.services = lib.mkOption { type = types.attrsOf ( types.submoduleWith { - specialArgs.pkgs = pkgs; - modules = [ ]; + modules = [ + (lib.modules.importApply ../portable/config-data.nix { inherit pkgs; }) + ]; } ); }; @@ -90,9 +90,6 @@ in ]; specialArgs = { # perhaps: features."systemd" = { }; - # TODO: Consider removing pkgs. Service modules can provide their own - # dependencies. - inherit pkgs; systemdPackage = config.systemd.package; }; } diff --git a/nixos/tests/modular-service-etc/python-http-server.nix b/nixos/tests/modular-service-etc/python-http-server.nix index 539354c66de1..72315831ba94 100644 --- a/nixos/tests/modular-service-etc/python-http-server.nix +++ b/nixos/tests/modular-service-etc/python-http-server.nix @@ -3,7 +3,6 @@ { config, lib, - pkgs, ... }: let @@ -16,7 +15,6 @@ in python-http-server = { package = mkOption { type = types.package; - default = pkgs.python3; description = "Python package to use for the web server"; }; @@ -46,21 +44,9 @@ in ]; configData = { - # This should probably just be {} if we were to put this module in production. - "webroot" = lib.mkDefault { - source = pkgs.runCommand "default-webroot" { } '' - mkdir -p $out - cat > $out/index.html << 'EOF' - - - Python Web Server - -

Welcome to the Python Web Server

-

Serving from port ${toString config.python-http-server.port}

- - - EOF - ''; + "webroot" = { + # Enable only if directory is set to use this path + enable = lib.mkDefault (config.python-http-server.directory == config.configData."webroot".path); }; }; }; diff --git a/nixos/tests/modular-service-etc/test.nix b/nixos/tests/modular-service-etc/test.nix index 7eee2f332a75..66e682871194 100644 --- a/nixos/tests/modular-service-etc/test.nix +++ b/nixos/tests/modular-service-etc/test.nix @@ -12,18 +12,44 @@ nodes = { server = { pkgs, ... }: + let + # Normally the package services.default attribute combines this, but we + # don't have that, because this is not a production service. Should it be? + python-http-server = { + imports = [ ./python-http-server.nix ]; + python-http-server.package = pkgs.python3; + }; + in { system.services.webserver = { # The python web server is simple enough that it doesn't need a reload signal. # Other services may need to receive a signal in order to re-read what's in `configData`. - imports = [ ./python-http-server.nix ]; + imports = [ python-http-server ]; python-http-server = { port = 8080; }; + configData = { + "webroot" = { + source = pkgs.runCommand "webroot" { } '' + mkdir -p $out + cat > $out/index.html << 'EOF' + + + Python Web Server + +

Welcome to the Python Web Server

+

Serving from port 8080

+ + + EOF + ''; + }; + }; + # Add a sub-service services.api = { - imports = [ ./python-http-server.nix ]; + imports = [ python-http-server ]; python-http-server = { port = 8081; }; diff --git a/nixos/tests/php/fpm-modular.nix b/nixos/tests/php/fpm-modular.nix index cd95bbd7e098..360e664963cf 100644 --- a/nixos/tests/php/fpm-modular.nix +++ b/nixos/tests/php/fpm-modular.nix @@ -1,3 +1,5 @@ +# Run with: +# nix-build -A nixosTests.php.fpm-modular { lib, php, ... }: { name = "php-${php.version}-fpm-modular-nginx-test"; diff --git a/pkgs/by-name/gh/ghostunnel/package.nix b/pkgs/by-name/gh/ghostunnel/package.nix index a057dd263d39..ed4cc9f9c7a5 100644 --- a/pkgs/by-name/gh/ghostunnel/package.nix +++ b/pkgs/by-name/gh/ghostunnel/package.nix @@ -7,6 +7,8 @@ ghostunnel, apple-sdk_12, darwinMinVersionHook, + writeScript, + runtimeShell, }: buildGoModule rec { @@ -41,7 +43,11 @@ buildGoModule rec { }; passthru.services.default = { - imports = [ ./service.nix ]; + imports = [ + (lib.modules.importApply ./service.nix { + inherit writeScript runtimeShell; + }) + ]; ghostunnel.package = ghostunnel; # FIXME: finalAttrs.finalPackage }; diff --git a/pkgs/by-name/gh/ghostunnel/service.nix b/pkgs/by-name/gh/ghostunnel/service.nix index 485ce30fc8e0..4fb7db2c3150 100644 --- a/pkgs/by-name/gh/ghostunnel/service.nix +++ b/pkgs/by-name/gh/ghostunnel/service.nix @@ -1,8 +1,11 @@ +# Non-module dependencies (`importApply`) +{ writeScript, runtimeShell }: + +# Service module { lib, config, options, - pkgs, ... }: let @@ -25,6 +28,7 @@ in ghostunnel = { package = mkOption { description = "Package to use for ghostunnel"; + defaultText = "The ghostunnel package that provided this module."; type = types.package; }; @@ -191,8 +195,8 @@ in cfg.cacert ]) ( - pkgs.writeScript "load-credentials" '' - #!${pkgs.runtimeShell} + writeScript "load-credentials" '' + #!${runtimeShell} exec $@ ${ concatStringsSep " " ( optional (cfg.keystore != null) "--keystore=$CREDENTIALS_DIRECTORY/keystore" diff --git a/pkgs/development/interpreters/php/generic.nix b/pkgs/development/interpreters/php/generic.nix index 22e8b68e8b05..fd95f37dba32 100644 --- a/pkgs/development/interpreters/php/generic.nix +++ b/pkgs/development/interpreters/php/generic.nix @@ -32,6 +32,8 @@ let common-updater-scripts, curl, jq, + coreutils, + formats, version, phpSrc ? null, @@ -390,7 +392,11 @@ let inherit ztsSupport; services.default = { - imports = [ ./service.nix ]; + imports = [ + (lib.modules.importApply ./service.nix { + inherit formats coreutils; + }) + ]; php-fpm.package = lib.mkDefault finalAttrs.finalPackage; }; }; diff --git a/pkgs/development/interpreters/php/service.nix b/pkgs/development/interpreters/php/service.nix index 50fcbb649800..06b1247c5e20 100644 --- a/pkgs/development/interpreters/php/service.nix +++ b/pkgs/development/interpreters/php/service.nix @@ -1,13 +1,18 @@ +# Tests in: nixos/tests/php/fpm-modular.nix + +# Non-module dependencies (importApply) +{ formats, coreutils }: + +# Service module { options, config, - pkgs, lib, ... }: let cfg = config.php-fpm; - format = pkgs.formats.iniWithGlobalSection { }; + format = formats.iniWithGlobalSection { }; configFile = format.generate "php-fpm.conf" { globalSection = lib.filterAttrs (_: v: !lib.isAttrs v) cfg.settings; sections = lib.filterAttrs (_: lib.isAttrs) cfg.settings; @@ -76,8 +81,11 @@ in _class = "service"; options.php-fpm = { - package = lib.mkPackageOption pkgs "php" { - example = '' + package = lib.mkOption { + type = lib.types.package; + description = "PHP package to use for php-fpm"; + defaultText = lib.literalMD ''The PHP package that provided this module.''; + example = lib.literalExpression '' php.buildEnv { extensions = { all, ... }: @@ -163,7 +171,7 @@ in serviceConfig = { Type = "notify"; - ExecReload = "${pkgs.coreutils}/bin/kill -USR2 $MAINPID"; + ExecReload = "${coreutils}/bin/kill -USR2 $MAINPID"; RuntimeDirectory = "php-fpm"; RuntimeDirectoryPreserve = true; Restart = "always"; @@ -175,7 +183,7 @@ in finit.service = { conditions = [ "service/syslogd/ready" ]; - reload = "${pkgs.coreutils}/bin/kill -USR2 $MAINPID"; + reload = "${coreutils}/bin/kill -USR2 $MAINPID"; notify = "systemd"; }; };