Revert "nixos/modular-services: add portable process.environment"

doc/build-helpers/testers.chapter.md

@@ -741,7 +741,7 @@ Notable attributes:
 
 Compliance suite for [modular service](https://nixos.org/manual/nixos/unstable/#modular-services) integrations.
 
-Tests that a service manager integration correctly handles the portable modular services contract: `process.argv`, `process.environment` (including `null` values that unset a variable), sub-services, assertions, and warnings.
+Tests that a service manager integration correctly handles the portable modular services contract: `process.argv`, sub-services, assertions, and warnings.
 
 ### Return value {#tester-modularServiceCompliance-return}
 

lib/services/service.nix

@@ -70,26 +70,6 @@ in
           Command used for reloading in the underlying service manager to reload.
         '';
       };
-
-      environment = lib.mkOption {
-        type = types.lazyAttrsOf (
-          types.nullOr (types.coercedTo (types.either types.path types.package) (x: "${x}") types.str)
-        );
-        default = { };
-        example = lib.literalExpression ''{ FOO = "bar"; PATH = null; }'';
-        description = ''
-          Environment variables passed verbatim to the service process by the
-          service manager. Entries set to `null` actively unset the variable
-          before the process starts -- backends without native unset support use
-          a wrapper (e.g. `execline`'s `unexport`) so the variable is absent
-          even when the service manager or a backend-specific override would
-          otherwise supply it.
-
-          Values appear in the rendered unit and may be world-readable. For
-          secrets, use a backend-specific mechanism such as
-          `systemd.service.serviceConfig.EnvironmentFile`.
-        '';
-      };
     };
 
     notificationProtocol = mkOption {

lib/services/test.nix

@@ -48,10 +48,6 @@ let
             (dummyPkg "cowsay.sh")
             "world"
           ];
-          environment = {
-            FOO = "bar";
-            DROPPED = null;
-          };
         };
       };
       service3 = {
@@ -114,7 +110,6 @@ let
                 "/usr/bin/echo"
                 "hello"
               ];
-              environment = { };
             };
             services = { };
             assertions = [
@@ -133,10 +128,6 @@ let
                 "${dummyPkg "cowsay.sh"}"
                 "world"
               ];
-              environment = {
-                FOO = "bar";
-                DROPPED = null;
-              };
             };
             services = { };
             assertions = [ ];
@@ -145,7 +136,6 @@ let
           service3 = {
             process = {
               argv = [ "/bin/false" ];
-              environment = { };
             };
             services.exclacow = {
               process = {
@@ -153,7 +143,6 @@ let
                   "${dummyPkg "cowsay-ng"}/bin/cowsay"
                   "!"
                 ];
-                environment = { };
               };
               services = { };
               assertions = [

nixos/doc/manual/default.nix

@@ -152,16 +152,7 @@ let
   };
 
   systemdServiceOptions = buildPackages.nixosOptionsDoc {
-    inherit
-      (evalModules {
-        modules = [
-          (modules.importApply ../../modules/system/service/systemd/service.nix {
-            pkgs = throw "nixos docs / systemdServiceOptions: Do not reference pkgs in docs";
-          })
-        ];
-      })
-      options
-      ;
+    inherit (evalModules { modules = [ ../../modules/system/service/systemd/service.nix ]; }) options;
     # TODO: filter out options that are not systemd-specific, maybe also change option prefix to just `service-opt-`?
     inherit revision warningsAreErrors;
     transformOptions =

nixos/modules/system/service/systemd/service.nix

@@ -1,9 +1,3 @@
-# Non-module arguments
-# These are separate from the module arguments to avoid implicit dependencies.
-# This makes service modules self-contained, allowing mixing of Nixpkgs versions.
-{ pkgs }:
-
-# The module
 {
   lib,
   config,
@@ -98,11 +92,6 @@ in
         to prevent systemd substitution. Set this option explicitly to enable
         systemd's substitution features.
 
-        When {option}`process.environment` contains keys set to `null`, the default
-        is automatically prefixed with `unexport KEY` invocations (from
-        `pkgs.execline`) so those variables are unset before the process starts,
-        regardless of what `Environment=` or inherited environment supplies.
-
         To extend {option}`process.argv` with systemd specifiers, you can append
         to the escaped arguments:
 
@@ -120,19 +109,8 @@ in
         for available specifiers like `%n`, `%i`, `%t`.
       '';
       type = types.str;
-      default =
-        let
-          nullEnvKeys = lib.attrNames (lib.filterAttrs (_: v: v == null) config.process.environment);
-        in
-        if nullEnvKeys == [ ] then
-          config.systemd.lib.escapeSystemdExecArgs config.process.argv
-        else
-          lib.concatMapStringsSep " " (
-            k: "${escapeSystemdExecArg "${pkgs.execline}/bin/unexport"} ${escapeSystemdExecArg k}"
-          ) nullEnvKeys
-          + " "
-          + config.systemd.lib.escapeSystemdExecArgs config.process.argv;
-      defaultText = lib.literalMD "The escaped `process.argv`, prefixed with `\"unexport\" \"KEY\"` (from `pkgs.execline`) for each key in `process.environment` set to `null`.";
+      default = config.systemd.lib.escapeSystemdExecArgs config.process.argv;
+      defaultText = lib.literalExpression "config.systemd.lib.escapeSystemdExecArgs config.process.argv";
     };
 
     systemd.mainExecReload = mkOption {
@@ -209,7 +187,7 @@ in
         types.submoduleWith {
           class = "service";
           modules = [
-            (lib.modules.importApply ./service.nix { inherit pkgs; })
+            ./service.nix
           ];
           specialArgs = {
             inherit systemdPackage;
@@ -229,9 +207,6 @@ in
     systemd.services."" = {
       # TODO description;
       wantedBy = lib.mkDefault [ "multi-user.target" ];
-      environment = lib.mapAttrs (_: lib.mkDefault) (
-        lib.filterAttrs (_: v: v != null) config.process.environment
-      );
       serviceConfig = {
         ExecReload = config.systemd.mainExecReload;
         Type = lib.mkDefault (if config.notificationProtocol.systemd then "notify" else "simple");

nixos/modules/system/service/systemd/system.nix

@@ -63,7 +63,7 @@ let
   modularServiceConfiguration = portable-lib.configure {
     serviceManagerPkgs = pkgs;
     extraRootModules = [
-      (lib.modules.importApply ./service.nix { inherit pkgs; })
+      ./service.nix
       ./config-data-path.nix
     ];
     extraRootSpecialArgs = {

nixos/modules/system/service/systemd/test.nix

@@ -76,40 +76,6 @@ let
         };
       };
 
-      # Test that `process.environment` becomes `Environment=` entries on the unit,
-      # that null values are dropped from `Environment=` and wrapped with unexport
-      # in `ExecStart`.
-      system.services.envvars = {
-        process = {
-          argv = [ hello' ];
-          environment = {
-            FOO = "bar";
-            BAZ = "qux";
-            DROPPED = null;
-          };
-        };
-      };
-
-      # Test that an explicit `systemd.service.environment` override wins over
-      # the portable default produced by `process.environment`.
-      system.services.envvars-override = {
-        process = {
-          argv = [ hello' ];
-          environment.FOO = "from-process";
-        };
-        systemd.service.environment.FOO = "from-systemd";
-      };
-
-      # Test that `process.environment` `null` unsets via wrapper even when the
-      # systemd layer sets the same key (true unset, not just "skip setting").
-      system.services.envvars-unset = {
-        process = {
-          argv = [ hello' ];
-          environment.FOO = null;
-        };
-        systemd.service.environment.FOO = "leaked";
-      };
-
       # Test extending process.argv with systemd specifiers
       system.services.argv-extended =
         { config, ... }:
@@ -171,22 +137,6 @@ runCommand "test-modular-service-systemd-units"
       # The base command should be escaped ($1 -> $$1, m%n -> m%%n), but the appended --systemd-unit %n should not be
       grep -F 'ExecStart="${hello}/bin/hello" "--greeting" "Fun $$1 fact, remainder is often expressed as m%%n" --systemd-unit %n' ${toplevel}/etc/systemd/system/argv-extended.service >/dev/null
 
-      # process.environment becomes Environment= entries; null values are dropped
-      # from Environment= and wrapped with unexport in ExecStart.
-      grep -F 'Environment="FOO=bar"' ${toplevel}/etc/systemd/system/envvars.service >/dev/null
-      grep -F 'Environment="BAZ=qux"' ${toplevel}/etc/systemd/system/envvars.service >/dev/null
-      ! grep -F 'Environment=.*DROPPED' ${toplevel}/etc/systemd/system/envvars.service
-      grep 'ExecStart=.*unexport.*DROPPED' ${toplevel}/etc/systemd/system/envvars.service >/dev/null
-
-      # systemd.service.environment override wins over process.environment.
-      grep -F 'Environment="FOO=from-systemd"' ${toplevel}/etc/systemd/system/envvars-override.service >/dev/null
-      ! grep -F 'FOO=from-process' ${toplevel}/etc/systemd/system/envvars-override.service
-
-      # process.environment null uses unexport wrapper for true unset, even when
-      # the systemd layer has an Environment= entry for the same key.
-      grep -F 'Environment="FOO=leaked"' ${toplevel}/etc/systemd/system/envvars-unset.service >/dev/null
-      grep 'ExecStart=.*unexport.*FOO' ${toplevel}/etc/systemd/system/envvars-unset.service >/dev/null
-
       [[ ! -e ${toplevel}/etc/systemd/system/foo.socket ]]
       [[ ! -e ${toplevel}/etc/systemd/system/bar.socket ]]
       [[ ! -e ${toplevel}/etc/systemd/system/bar-db.socket ]]

pkgs/build-support/testers/modular-service-compliance.nix

@@ -29,10 +29,6 @@ let
     services = {
       svc = {
         process.argv = [ "${coreutils}/bin/true" ];
-        process.environment = {
-          FOO = "bar";
-          DROPPED = null;
-        };
         assertions = [
           {
             assertion = true;
@@ -74,19 +70,6 @@ let
         expected = [ "${coreutils}/bin/true" ];
       };
 
-      # A set environment variable round-trips through process.environment.
-      testProcessEnvironment = {
-        expr = c.process.environment.FOO;
-        expected = "bar";
-      };
-
-      # A null environment variable is preserved as null (unset request),
-      # rather than coerced to a string or dropped from the attrset.
-      testProcessEnvironmentNull = {
-        expr = c.process.environment.DROPPED;
-        expected = null;
-      };
-
       testAssertions = {
         expr = lib.elem {
           assertion = true;
@@ -203,9 +186,6 @@ let
     mkdir -p "$dir"
     echo "$$" > "$dir/pid"
     printf '%s\n' "$@" > "$dir/args"
-    # Record the process's own environment as received from the service
-    # manager (NUL-delimited, as the kernel stores it).
-    "${coreutils}/bin/cat" "/proc/$$/environ" > "$dir/environ"
     exec "${coreutils}/bin/sleep" infinity
   '';
 
@@ -258,31 +238,6 @@ let
         || { echo "${id}: expected arg ${lib.escapeShellArg arg} not found"; cat "${sharedDir}/${id}/args"; exit 1; }
     '') expectedArgs;
 
-  /**
-    Shell snippet: assert that the service's recorded environment contains
-    each `present` entry (an exact `KEY=value` string) and contains no
-    variable named in `absent`.
-  */
-  checkEnv =
-    id:
-    {
-      present ? [ ],
-      absent ? [ ],
-    }:
-    ''
-      # The recorded environ is NUL-delimited; render one entry per line.
-      tr '\0' '\n' < "${sharedDir}/${id}/environ" > "${sharedDir}/${id}/environ.lines"
-    ''
-    + lib.concatMapStrings (entry: ''
-      grep -qxF -- ${lib.escapeShellArg entry} "${sharedDir}/${id}/environ.lines" \
-        || { echo "${id}: expected env ${lib.escapeShellArg entry} not found"; cat "${sharedDir}/${id}/environ.lines"; exit 1; }
-    '') present
-    + lib.concatMapStrings (key: ''
-      if grep -qE ${lib.escapeShellArg "^${key}="} "${sharedDir}/${id}/environ.lines"; then
-        echo "${id}: env variable ${lib.escapeShellArg key} should be unset"; exit 1
-      fi
-    '') absent;
-
   mkTestScript =
     name: text:
     lib.getExe (writeShellApplication {
@@ -375,24 +330,6 @@ in
     );
   };
 
-  environment = mkTest {
-    name = "${namePrefix}-environment";
-    services.test = {
-      process.argv = mkArgv "env" [ ];
-      process.environment = {
-        FOO = "bar";
-        DROPPED = null;
-      };
-    };
-    testExe = mkTestScript "environment" (
-      waitAndCheck "env" [ ]
-      + checkEnv "env" {
-        present = [ "FOO=bar" ];
-        absent = [ "DROPPED" ];
-      }
-    );
-  };
-
   sub-services = mkTest {
     name = "${namePrefix}-sub-services";
     services.a = {