Skip to content

Commit 665ce95

Browse files
committed
lib.modules.env: simplify API and move helpers into wlib.env
Response to code review — the previous iteration was over-engineered: too many options, unclear names, helpers scattered across wlib. Changes: - Namespace: wlib.envRef / wlib.renderEnvString / normaliseEnvEntry collapse into wlib.env.{ref, render}. normaliseEnvEntry is now internal. - Rename fallback -> ifUnset. "fallback" was vague; "ifUnset" says what it does: only apply when the caller's env doesn't already have it set. Now uses ${VAR:-} semantics so empty also counts as unset. - Drop prefix / suffix entirely. They were makeWrapper jargon that wasn't self-evident. The same thing is now expressed via a list value and wlib.env.ref: env.PATH.value = [ "/opt/bin" (wlib.env.ref "PATH") ]; - Collapse value + values into a single polymorphic `value` option: a plain string for the literal case, or a list of parts (joined with separator) for everything else. One concept, one name. The module submodule is now just four options: value, separator, ifUnset, unset. List values still merge by concatenation under apply, so composition works the same. lib/modules/env.nix shrinks from 203 to 106 lines; the renderer in lib/default.nix drops from ~180 to ~120. Tests are consolidated from four files to three. https://claude.ai/code/session_01UiEmmBrtkNEstoRemZpnp7
1 parent f648cfa commit 665ce95

11 files changed

Lines changed: 412 additions & 797 deletions

CHANGELOG.md

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,12 @@
1515
were explicitly passing `flagSeparator = " "` to get separate args,
1616
remove it (or change to `null`).
1717

18-
- `env` option type changed from `attrsOf str` to a richer submodule
19-
(see "Added" below). Plain-string and path values keep coercing to
20-
the old behaviour, so `env.FOO = "bar"` is unchanged. Passthru
21-
`wrapPackage` callers now get the structured form on
22-
`passthru.env`; read `passthru.env.<name>.value` instead of
23-
`passthru.env.<name>` if you need the literal. The systemd
24-
integration reads from `config.outputs.staticEnv` instead of
25-
`config.env` and silently drops entries that can't be expressed as
26-
a static literal (prefix/suffix, values, fallback, unset).
18+
- `env` option type changed from `attrsOf str` to a small submodule
19+
with `value` / `separator` / `ifUnset` / `unset`. Plain strings
20+
and `null` keep working via coercion (`env.FOO = "bar"` and
21+
`env.FOO = null` are unchanged). The systemd integration reads
22+
from `config.outputs.staticEnv` instead of `config.env` and drops
23+
any entries it can't express as a literal assignment.
2724

2825
### Added
2926

@@ -32,30 +29,24 @@
3229
- `lib/modules/flags.nix`: flags module with per-flag ordering via
3330
`{ value, order }` submodules. Default order is 1000. Reading
3431
`config.flags` returns clean values (order is transparent).
35-
- `lib/modules/env.nix`: env module with richer per-variable options
36-
for safe composition, modelled on `makeWrapper`'s `--prefix` /
37-
`--suffix` but usable through the NixOS module system.
38-
- `env.<VAR>.value`: literal string (same as `env.<VAR> = "..."`).
39-
- `env.<VAR>.prefix` / `.suffix`: parts to splice around the
40-
existing value of the variable. Empty or unset existing values
41-
drop out cleanly with no stray separators.
42-
- `env.<VAR>.values`: explicit list of parts to join, with
43-
`wlib.envRef "OTHER"` placeholders for runtime env references.
44-
- `env.<VAR>.separator`: join separator (default `:`).
45-
- `env.<VAR>.fallback = true`: only set the variable when it is
46-
not already set in the caller's environment (uses `${VAR+set}`
47-
semantics).
32+
- `lib/modules/env.nix`: env module with per-variable options for
33+
safe composition through the NixOS module system.
34+
- `env.<VAR>.value`: string for a simple literal, or a list of
35+
parts to join with `separator`. List parts can be plain strings
36+
or `wlib.env.ref "NAME"` runtime references. Empty/unset refs
37+
drop out cleanly, so no dangling separators.
38+
- `env.<VAR>.separator`: join separator for a list `value`
39+
(default `:`).
40+
- `env.<VAR>.ifUnset = true`: only apply when the caller's
41+
environment doesn't already have the variable set.
4842
- `env.<VAR>.unset = true`: emit `unset VAR` instead of an
4943
assignment. `env.VAR = null` is sugar for this.
50-
- List-valued entries (`values`, `prefix`, `suffix`) merge by
51-
concatenation when composed via `apply`, so multiple modules can
52-
stack contributions onto the same variable without fighting.
53-
- `wlib.envRef :: name -> envRef`: marker used inside `values` /
54-
`prefix` / `suffix` lists to reference another env variable at
55-
runtime. Dropped cleanly if the referenced variable is unset.
56-
- `wlib.renderEnvString :: env -> str`: pure helper that renders an
57-
`env` attrset into the shell snippet the wrapper uses. Exposed
58-
for testing and downstream composition.
44+
- List `value`s merge by concatenation when composed via `apply`,
45+
so modules stack contributions without fighting over a string.
46+
- `wlib.env.ref NAME`: marker for a runtime env-variable reference
47+
inside `env.<VAR>.value` lists.
48+
- `wlib.env.render`: render an `env` attrset into a shell snippet.
49+
Exposed for tests and downstream composition.
5950
- `outputs.staticEnv`: subset of `env` that resolves to a plain
6051
literal string, used by the systemd integration.
6152
- `wrapper.nix` injects `"$@"` into args at order 1001, controllable

README.md

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -93,61 +93,42 @@ wrappers.lib.wrapPackage {
9393

9494
### Environment Variables
9595

96-
Environment variables can be set in three forms, with the richer form
97-
available both via `wrapPackage` and through the `env.<NAME>` option
98-
of wrapper modules.
96+
Each `env.<NAME>` entry is either a plain string, `null` (to unset),
97+
or a small submodule:
9998

10099
```nix
101100
{
102101
env = {
103-
# 1. Plain literal string (same as before):
102+
# Literal:
104103
FOO = "bar";
105104
106-
# 2. Null to explicitly unset a variable inherited from the
107-
# caller's environment:
105+
# Unset a variable inherited from the caller:
108106
NOISY_OLD_VAR = null;
109107
110-
# 3. Structured form with the following fields:
111-
#
112-
# - value: literal string
113-
# - prefix / suffix: parts to splice around the existing value
114-
# of the variable (like `makeWrapper --prefix/--suffix`).
115-
# Empty or unset existing values drop out cleanly.
116-
# - values: explicit list of parts, joined with `separator`.
117-
# Use `wlib.envRef "OTHER"` to reference another variable
118-
# at runtime.
119-
# - separator: join separator (default ":")
120-
# - fallback: only set if the variable isn't already set
121-
# - unset: emit `unset VAR` (takes precedence)
122-
123-
# Prepend to PATH, keeping the caller's existing entries:
124-
PATH.prefix = [ "/opt/bin" ];
125-
126-
# Append to XDG_DATA_DIRS:
127-
XDG_DATA_DIRS.suffix = [ "/opt/share" ];
128-
129-
# Build LD_LIBRARY_PATH with the existing value somewhere in the
130-
# middle, not just at the edges:
131-
LD_LIBRARY_PATH.values = [
132-
"/opt/lib"
133-
(wrappers.lib.envRef "LD_LIBRARY_PATH")
134-
"/other/lib"
135-
];
136-
137-
# Only set EDITOR if the user hasn't picked one already:
138-
EDITOR = {
139-
value = "vim";
140-
fallback = true;
141-
};
108+
# Prepend to PATH using a list and `wlib.env.ref`. The ref
109+
# expands to the caller's existing PATH at runtime; if it's
110+
# unset or empty, the ref drops out and no stray separators
111+
# are left behind.
112+
PATH.value = [ "/opt/bin" (wrappers.lib.env.ref "PATH") ];
113+
114+
# Only set EDITOR if the caller hasn't already picked one.
115+
EDITOR = { value = "vim"; ifUnset = true; };
142116
};
143117
}
144118
```
145119

146-
`prefix`, `suffix` and `values` are lists, so they compose via module
147-
merging: multiple modules (or multiple `apply` calls) contributing to
148-
the same variable stack their contributions instead of fighting over
149-
a single string. Empty parts are filtered at runtime, so unset env
150-
references never leave behind dangling separators.
120+
Submodule options:
121+
- `value`: literal string *or* a list of parts joined with
122+
`separator`. List parts can be plain strings or
123+
`wlib.env.ref "NAME"` runtime references.
124+
- `separator`: join separator for list values (default `:`).
125+
- `ifUnset`: only apply when the caller's environment doesn't
126+
already have the variable set.
127+
- `unset`: unset the variable (takes precedence over `value`).
128+
129+
List `value`s merge by concatenation when composed via `apply`, so
130+
multiple modules stack contributions onto the same variable without
131+
fighting over a single string.
151132

152133
### Creating Custom Wrapper Modules
153134

checks/env-fallback-unset.nix

Lines changed: 0 additions & 63 deletions
This file was deleted.

checks/env-if-unset.nix

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
{
2+
pkgs,
3+
self,
4+
}:
5+
6+
# `ifUnset` (only set if caller hasn't) and explicit unset.
7+
let
8+
wlib = self.lib;
9+
10+
showEnv = pkgs.writeShellScriptBin "show-env" ''
11+
printf 'EDITOR=%s\n' "''${EDITOR-<unset>}"
12+
printf 'BLOAT=%s\n' "''${BLOAT-<unset>}"
13+
'';
14+
15+
wrapped =
16+
(wlib.wrapModule (
17+
{ config, ... }:
18+
{
19+
config.package = showEnv;
20+
config.env.EDITOR = {
21+
value = "vim";
22+
ifUnset = true;
23+
};
24+
# `null` sugar for unset.
25+
config.env.BLOAT = null;
26+
}
27+
).apply { pkgs = pkgs; }).wrapper;
28+
in
29+
pkgs.runCommand "env-if-unset-test" { } ''
30+
set -eu
31+
script="${wrapped}/bin/show-env"
32+
33+
# ifUnset applied when EDITOR is unset.
34+
r=$(unset EDITOR && "$script" | grep '^EDITOR=' | cut -d= -f2-)
35+
[ "$r" = "vim" ] || { echo "FAIL: ifUnset unset: '$r'"; cat "$script"; exit 1; }
36+
echo "PASS: ifUnset applies when unset"
37+
38+
# ifUnset yields to an existing value.
39+
r=$(EDITOR=nano "$script" | grep '^EDITOR=' | cut -d= -f2-)
40+
[ "$r" = "nano" ] || { echo "FAIL: ifUnset overrode existing: '$r'"; exit 1; }
41+
echo "PASS: ifUnset preserves existing"
42+
43+
# Explicit unset beats caller env.
44+
r=$(BLOAT=garbage "$script" | grep '^BLOAT=' | cut -d= -f2-)
45+
[ "$r" = "<unset>" ] || { echo "FAIL: unset: '$r'"; cat "$script"; exit 1; }
46+
echo "PASS: unset overrides caller env"
47+
48+
touch $out
49+
''

checks/env-list-value.nix

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
{
2+
pkgs,
3+
self,
4+
}:
5+
6+
# End-to-end test for list-valued `env.<VAR>.value` with
7+
# `wlib.env.ref` to prepend to an existing variable. Also exercises
8+
# composition via `apply`: lists merge by concatenation.
9+
let
10+
wlib = self.lib;
11+
12+
showVar = pkgs.writeShellScriptBin "show-var" ''
13+
printf 'TEST_VAR=%s\n' "''${TEST_VAR-<unset>}"
14+
'';
15+
16+
base = wlib.wrapModule (
17+
{ config, ... }:
18+
{
19+
config.package = showVar;
20+
config.env.TEST_VAR.value = [
21+
"/base-front"
22+
(wlib.env.ref "TEST_VAR")
23+
"/base-back"
24+
];
25+
}
26+
);
27+
28+
extended = (base.apply { pkgs = pkgs; }).apply {
29+
# List merging via the module system: apply concatenates.
30+
env.TEST_VAR.value = [ "/extra" ];
31+
};
32+
33+
wrapped = extended.wrapper;
34+
in
35+
pkgs.runCommand "env-list-value-test" { } ''
36+
set -eu
37+
script="${wrapped}/bin/show-var"
38+
39+
# Case 1: TEST_VAR unset — envRef drops out, no stray colons.
40+
r1=$(unset TEST_VAR && "$script" | grep '^TEST_VAR=' | cut -d= -f2-)
41+
case "$r1" in
42+
*::*)
43+
echo "FAIL: unset case has stray separator: '$r1'"
44+
cat "$script"; exit 1 ;;
45+
:*|*:)
46+
echo "FAIL: unset case has leading/trailing colon: '$r1'" ; exit 1 ;;
47+
esac
48+
case "$r1" in
49+
*/base-front*) ;;
50+
*) echo "FAIL: base-front missing: '$r1'"; exit 1 ;;
51+
esac
52+
case "$r1" in
53+
*/base-back*) ;;
54+
*) echo "FAIL: base-back missing: '$r1'"; exit 1 ;;
55+
esac
56+
case "$r1" in
57+
*/extra*) ;;
58+
*) echo "FAIL: extra missing (list merge via apply): '$r1'"; exit 1 ;;
59+
esac
60+
echo "PASS: unset case: $r1"
61+
62+
# Case 2: TEST_VAR=/mid — envRef expands in place.
63+
r2=$(TEST_VAR=/mid "$script" | grep '^TEST_VAR=' | cut -d= -f2-)
64+
case "$r2" in
65+
*/mid*) ;;
66+
*) echo "FAIL: existing value lost: '$r2'"; exit 1 ;;
67+
esac
68+
echo "PASS: set case: $r2"
69+
70+
touch $out
71+
''

0 commit comments

Comments
 (0)