Fixes several correctness bugs primarily around modular services' recent
reload/notification options (#535695) and adds compliance coverage to guard them.
Fixes:
- `lib/services/service.nix`:
- the reload-conflict assertion had inverted polarity, so it fired on the
default configuration
- the `mkIf` guard on `process.reloadCommand` had a misplaced paren, applying
`!= null` to the `mkIf` result rather than to the condition
- `process.reloadSignal` derives `process.reloadCommand`, so the assertion
could not check `reloadCommand != null` -- that fired on every signal-only
service. The command is now derived at `mkDefault` priority and the
assertion is gated on `options.process.reloadCommand.highestPrio`, firing
only when the user also set `reloadCommand` explicitly.
- change `notificationProtocol` to a sub-module type
- `nixos/modules/system/service/systemd/service.nix`:
- `systemd.mainExecReload`'s default ran `escapeSystemdExecArgs` (a list
escaper) on the `nullOr str` `process.reloadCommand`; this threw
`expected a list but found a string` and would have mangled `$MAINPID`. It
now uses `process.reloadCommand` verbatim.
- the `Type` default read a non-existent
`config.serviceManager.notificationProtocol` instead of
`config.notificationProtocol`.
Tests:
Extend the modular-service compliance suite to guard the above:
- Portable (manager-agnostic) eval assertions: `reloadSignal` derives
`reloadCommand`, the conflict assertion does not fire on signal-only services
but does when both are set explicitly, and `notificationProtocol.systemd`/`.s6`
default to `false`.
- systemd-specific eval assertions: `serviceConfig.Type` (simple/notify) and
`serviceConfig.ExecReload` are asserted on the resolved host units. This
directly guards the `mainExecReload` fix, which threw before it.
- Runtime reload compliance test: a nested reloadable sub-service is started and
reloaded, asserting the service observed the reload (recorded a SIGHUP marker).
`callReload` receives the service's name path (the list of names from the
top-level service down to the target sub-service); each integration joins it
per its own unit-naming convention (NixOS dash-joins to the systemd unit name,
e.g. `reload-inner.service`). Keeping it a path list rather than a read-only
submodule option keeps the suite manager-agnostic.
- `doc/build-helpers/testers.chapter.md`: document `callReload`.
Follow-up to #535695.
Signed-off-by: cinereal <cinereal@riseup.net>
Assisted-by: Claude:claude-opus-4-8
Adds modular service option `process.environment` to pass an attrset of
env vars to the service manager.
`null` values actively unset the variable before the process starts.
Values are `coercedTo (either path package) str` via interpolation,
mirroring `pathOrStr`, so paths and packages render to store-path strings
with string context preserved. The type is `lazyAttrsOf`, allowing one
entry to reference another (recursive env definitions).
The systemd backend unsets entries using `unexport` in `ExecStart`,
so the variable is absent even when `Environment=` or
the inherited environment would otherwise supply it.
The systemd backend lifts non-null entries onto the primary unit wrapped
per-key with `lib.mkDefault` so they merge with the existing priority-100
`environment.PATH` binding in `nixos/lib/systemd-lib.nix` while still letting
explicit `systemd.service.environment.<k>` overrides win.
The systemd extra-root modules are loaded via `importApply`, closing `pkgs`
over `systemd/service.nix` as a non-module argument (matching the portable
`lib/services/service.nix` convention) instead of passing a redundant `pkgs`
specialArg. The docs eval threads `pkgs = throw` accordingly.
Portable coverage lives in `testers.modularServiceCompliance`: an eval-level
check that a set value round-trips and a `null` value is preserved, plus an
integration test that records the service's own `/proc/$$/environ` and asserts
the set variable is present and the null variable is absent. The
systemd-specific grep assertions in `systemd/test.nix` cover how systemd
achieves this (`Environment=` rendering, null filtering, the `unexport`
wrapper, and override precedence).
Assisted-by: Claude:claude-opus-4-8
Every in-tree union/intersection use has exactly two members, and the
binary form verifies without allocating a closure or calling the any/all
primop per check. check-meta.nix switches to either/both; union and
intersection are kept for potential future use.
Assisted-by: Claude Code (Claude Fable 5)
throwIfNot sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
throwIfNot sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
throwIfNot sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
throwIfNot sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
throwIf sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
throwIf sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
throwIf sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
assertMsg sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
assertMsg sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
assertMsg sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
assertMsg sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
assertMsg sends our error message through a function call, even if the
error condition doesn't trigger. This requires a lot of thunk allocation
that can be easily avoided.
