From f648cfac406b00d0ed14a48d2ba630ffffd71996 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Apr 2026 10:54:05 +0000 Subject: [PATCH 1/4] lib.modules.env: structured env with prefix/suffix/values/fallback Resolves the composability gap from #55 ("Prepend ENV variables") and adds handling for the related cases that came up during that discussion. The old env option was attrsOf str, which forced users into bash parameter-expansion gymnastics whenever they wanted to prefix PATH, provide a fallback default for EDITOR, or unset an inherited variable. The new env option is a submodule per entry with: - value / values: literal or list-of-parts - prefix / suffix: splice around the existing value, makeWrapper style - separator: join separator (default ":") - fallback: only set when the variable isn't already set - unset: emit `unset VAR` List-valued fields merge by concatenation, so modules composing via apply stack contributions cleanly instead of fighting over a single string. Empty/unset env references drop out at runtime via a shared shell helper (_wrapper_env_join), so no dangling separators. Plain strings and null keep working via coercedTo, so existing env.FOO = "bar" / env.FOO = null usage is unchanged. wlib.envRef produces runtime references for the values list; wlib.renderEnvString is exposed for tests and downstream tooling. The systemd integration reads from the new outputs.staticEnv output, which only exposes entries that resolve to a plain literal. https://claude.ai/code/session_01UiEmmBrtkNEstoRemZpnp7 --- CHANGELOG.md | 36 ++++++ README.md | 58 ++++++++++ checks/env-fallback-unset.nix | 63 +++++++++++ checks/env-prefix-suffix.nix | 155 ++++++++++++++++++++++++++ checks/env-render-string.nix | 136 +++++++++++++++++++++++ checks/env-values-envref.nix | 63 +++++++++++ lib/default.nix | 198 +++++++++++++++++++++++++++++++-- lib/modules/command.nix | 8 +- lib/modules/env.nix | 202 ++++++++++++++++++++++++++++++++++ lib/modules/systemd.nix | 2 +- 10 files changed, 904 insertions(+), 17 deletions(-) create mode 100644 checks/env-fallback-unset.nix create mode 100644 checks/env-prefix-suffix.nix create mode 100644 checks/env-render-string.nix create mode 100644 checks/env-values-envref.nix create mode 100644 lib/modules/env.nix diff --git a/CHANGELOG.md b/CHANGELOG.md index f3b67ef..1cf1b23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,16 @@ were explicitly passing `flagSeparator = " "` to get separate args, remove it (or change to `null`). +- `env` option type changed from `attrsOf str` to a richer submodule + (see "Added" below). Plain-string and path values keep coercing to + the old behaviour, so `env.FOO = "bar"` is unchanged. Passthru + `wrapPackage` callers now get the structured form on + `passthru.env`; read `passthru.env..value` instead of + `passthru.env.` if you need the literal. The systemd + integration reads from `config.outputs.staticEnv` instead of + `config.env` and silently drops entries that can't be expressed as + a static literal (prefix/suffix, values, fallback, unset). + ### Added - `lib/modules/command.nix`: base module with shared command spec @@ -22,6 +32,32 @@ - `lib/modules/flags.nix`: flags module with per-flag ordering via `{ value, order }` submodules. Default order is 1000. Reading `config.flags` returns clean values (order is transparent). +- `lib/modules/env.nix`: env module with richer per-variable options + for safe composition, modelled on `makeWrapper`'s `--prefix` / + `--suffix` but usable through the NixOS module system. + - `env..value`: literal string (same as `env. = "..."`). + - `env..prefix` / `.suffix`: parts to splice around the + existing value of the variable. Empty or unset existing values + drop out cleanly with no stray separators. + - `env..values`: explicit list of parts to join, with + `wlib.envRef "OTHER"` placeholders for runtime env references. + - `env..separator`: join separator (default `:`). + - `env..fallback = true`: only set the variable when it is + not already set in the caller's environment (uses `${VAR+set}` + semantics). + - `env..unset = true`: emit `unset VAR` instead of an + assignment. `env.VAR = null` is sugar for this. + - List-valued entries (`values`, `prefix`, `suffix`) merge by + concatenation when composed via `apply`, so multiple modules can + stack contributions onto the same variable without fighting. +- `wlib.envRef :: name -> envRef`: marker used inside `values` / + `prefix` / `suffix` lists to reference another env variable at + runtime. Dropped cleanly if the referenced variable is unset. +- `wlib.renderEnvString :: env -> str`: pure helper that renders an + `env` attrset into the shell snippet the wrapper uses. Exposed + for testing and downstream composition. +- `outputs.staticEnv`: subset of `env` that resolves to a plain + literal string, used by the systemd integration. - `wrapper.nix` injects `"$@"` into args at order 1001, controllable via the ordering system. - `outputs.wrapper` as the canonical output path (config.wrapper is diff --git a/README.md b/README.md index bde5a53..eb6454e 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,64 @@ wrappers.lib.wrapPackage { } ``` +### Environment Variables + +Environment variables can be set in three forms, with the richer form +available both via `wrapPackage` and through the `env.` option +of wrapper modules. + +```nix +{ + env = { + # 1. Plain literal string (same as before): + FOO = "bar"; + + # 2. Null to explicitly unset a variable inherited from the + # caller's environment: + NOISY_OLD_VAR = null; + + # 3. Structured form with the following fields: + # + # - value: literal string + # - prefix / suffix: parts to splice around the existing value + # of the variable (like `makeWrapper --prefix/--suffix`). + # Empty or unset existing values drop out cleanly. + # - values: explicit list of parts, joined with `separator`. + # Use `wlib.envRef "OTHER"` to reference another variable + # at runtime. + # - separator: join separator (default ":") + # - fallback: only set if the variable isn't already set + # - unset: emit `unset VAR` (takes precedence) + + # Prepend to PATH, keeping the caller's existing entries: + PATH.prefix = [ "/opt/bin" ]; + + # Append to XDG_DATA_DIRS: + XDG_DATA_DIRS.suffix = [ "/opt/share" ]; + + # Build LD_LIBRARY_PATH with the existing value somewhere in the + # middle, not just at the edges: + LD_LIBRARY_PATH.values = [ + "/opt/lib" + (wrappers.lib.envRef "LD_LIBRARY_PATH") + "/other/lib" + ]; + + # Only set EDITOR if the user hasn't picked one already: + EDITOR = { + value = "vim"; + fallback = true; + }; + }; +} +``` + +`prefix`, `suffix` and `values` are lists, so they compose via module +merging: multiple modules (or multiple `apply` calls) contributing to +the same variable stack their contributions instead of fighting over +a single string. Empty parts are filtered at runtime, so unset env +references never leave behind dangling separators. + ### Creating Custom Wrapper Modules ```nix diff --git a/checks/env-fallback-unset.nix b/checks/env-fallback-unset.nix new file mode 100644 index 0000000..8b597a6 --- /dev/null +++ b/checks/env-fallback-unset.nix @@ -0,0 +1,63 @@ +{ + pkgs, + self, +}: + +# Fallback (only-set-if-unset) and explicit unset handling. +let + wlib = self.lib; + + showEnv = pkgs.writeShellScriptBin "show-env" '' + printf 'EDITOR=%s\n' "''${EDITOR-}" + printf 'BLOAT=%s\n' "''${BLOAT-}" + ''; + + wrapped = + (wlib.wrapModule ( + { config, ... }: + { + config.package = showEnv; + config.env.EDITOR = { + value = "vim"; + fallback = true; + }; + # `null` is sugar for `unset = true`, exercise both forms. + config.env.BLOAT = null; + } + ).apply { pkgs = pkgs; }).wrapper; +in +pkgs.runCommand "env-fallback-unset-test" { } '' + set -eu + script="${wrapped}/bin/show-env" + + # Fallback: EDITOR unset → set to vim. + editor_unset=$(unset EDITOR && "$script" | grep '^EDITOR=' | cut -d= -f2-) + if [ "$editor_unset" = "vim" ]; then + echo "PASS: fallback applied when unset" + else + echo "FAIL: fallback unset case: '$editor_unset'" + cat "$script" + exit 1 + fi + + # Fallback: EDITOR already set → preserved. + editor_set=$(EDITOR=nano "$script" | grep '^EDITOR=' | cut -d= -f2-) + if [ "$editor_set" = "nano" ]; then + echo "PASS: fallback preserved existing value" + else + echo "FAIL: fallback preserved case: '$editor_set'" + exit 1 + fi + + # Unset: BLOAT should be unset even if caller exports it. + bloat_result=$(BLOAT=garbage "$script" | grep '^BLOAT=' | cut -d= -f2-) + if [ "$bloat_result" = "" ]; then + echo "PASS: unset overrides caller env" + else + echo "FAIL: unset case: '$bloat_result'" + cat "$script" + exit 1 + fi + + touch $out +'' diff --git a/checks/env-prefix-suffix.nix b/checks/env-prefix-suffix.nix new file mode 100644 index 0000000..acb4b9e --- /dev/null +++ b/checks/env-prefix-suffix.nix @@ -0,0 +1,155 @@ +{ + pkgs, + self, +}: + +# Prefix/suffix composition for env variables. +# +# Goals: +# 1. A `prefix`/`suffix` entry splices around the existing value of +# the variable and leaves no dangling separator when that variable +# is unset or empty. +# 2. Multiple modules contributing to the same variable via `apply` +# compose via list concatenation, not via string overwriting. +# +# We use a custom variable name (WRAPPER_TEST_VAR) instead of PATH so +# that unsetting it inside a subshell doesn't break the shell itself. +let + wlib = self.lib; + + showVar = pkgs.writeShellScriptBin "show-var" '' + printf 'WRAPPER_TEST_VAR=%s\n' "''${WRAPPER_TEST_VAR-}" + ''; + + base = wlib.wrapModule ( + { config, ... }: + { + config.package = showVar; + config.env.WRAPPER_TEST_VAR.prefix = [ "/base-pre" ]; + config.env.WRAPPER_TEST_VAR.suffix = [ "/base-post" ]; + } + ); + + extended = (base.apply { pkgs = pkgs; }).apply { + env.WRAPPER_TEST_VAR.prefix = [ "/extra-pre" ]; + env.WRAPPER_TEST_VAR.suffix = [ "/extra-post" ]; + }; + + wrapped = extended.wrapper; +in +pkgs.runCommand "env-prefix-suffix-test" { } '' + set -eu + script="${wrapped}/bin/show-var" + if [ ! -f "$script" ]; then + echo "FAIL: wrapper script not found" + exit 1 + fi + + # Case 1: WRAPPER_TEST_VAR unset — prefix and suffix join with no + # dangling separators, no stray reference to the empty existing + # value. + unset WRAPPER_TEST_VAR + result_unset=$("$script" | grep '^WRAPPER_TEST_VAR=' | cut -d= -f2-) + case "$result_unset" in + *:*:*) + # good, at least three parts (two prefixes + two suffixes all + # joined together — order is list-concatenation order) + ;; + *) + echo "FAIL: unset case collapsed too aggressively: '$result_unset'" + cat "$script" + exit 1 + ;; + esac + case "$result_unset" in + *::*) + echo "FAIL: unset case has double colon (stray separator): '$result_unset'" + cat "$script" + exit 1 + ;; + :*|*:) + echo "FAIL: unset case has leading/trailing colon: '$result_unset'" + exit 1 + ;; + *) + echo "PASS: unset case has no dangling separators: $result_unset" + ;; + esac + case "$result_unset" in + */base-pre*) ;; + *) + echo "FAIL: base prefix missing: '$result_unset'" + exit 1 + ;; + esac + case "$result_unset" in + */extra-pre*) ;; + *) + echo "FAIL: extra prefix missing (apply list merge?): '$result_unset'" + exit 1 + ;; + esac + case "$result_unset" in + */base-post*) ;; + *) + echo "FAIL: base suffix missing: '$result_unset'" + exit 1 + ;; + esac + case "$result_unset" in + */extra-post*) ;; + *) + echo "FAIL: extra suffix missing (apply list merge?): '$result_unset'" + exit 1 + ;; + esac + + # Case 2: WRAPPER_TEST_VAR set — prefixes appear before, suffixes + # after, and the existing value survives in the middle. + export WRAPPER_TEST_VAR=/user-value + result_set=$("$script" | grep '^WRAPPER_TEST_VAR=' | cut -d= -f2-) + case "$result_set" in + */user-value*) + echo "PASS: existing value preserved: $result_set" + ;; + *) + echo "FAIL: existing value lost: '$result_set'" + exit 1 + ;; + esac + # Prefixes must appear before the existing middle value. + prefix_part="''${result_set%%/user-value*}" + case "$prefix_part" in + */base-pre*) ;; + *) + echo "FAIL: base prefix not before existing value: '$result_set'" + exit 1 + ;; + esac + # Suffixes must appear after. + suffix_part="''${result_set##*/user-value}" + case "$suffix_part" in + */base-post*) ;; + *) + echo "FAIL: base suffix not after existing value: '$result_set'" + exit 1 + ;; + esac + + # Case 3: WRAPPER_TEST_VAR set to the empty string. Empty is filtered + # the same as unset so the middle drops out cleanly. + export WRAPPER_TEST_VAR="" + result_empty=$("$script" | grep '^WRAPPER_TEST_VAR=' | cut -d= -f2-) + case "$result_empty" in + *::*) + echo "FAIL: empty existing value left a double colon: '$result_empty'" + exit 1 + ;; + *) + echo "PASS: empty existing value collapsed cleanly: $result_empty" + ;; + esac + + echo "SUCCESS: env prefix/suffix composition works" + touch $out +'' diff --git a/checks/env-render-string.nix b/checks/env-render-string.nix new file mode 100644 index 0000000..9b7933a --- /dev/null +++ b/checks/env-render-string.nix @@ -0,0 +1,136 @@ +{ + pkgs, + self, +}: + +# Pure rendering tests for `wlib.renderEnvString`. These don't build +# a wrapper, they just assert the generated shell snippet is what we +# expect. Keeps the feedback loop tight when iterating on escaping. +let + lib = pkgs.lib; + wlib = self.lib; + + cases = [ + { + name = "empty"; + input = { }; + expected = ""; + } + { + name = "simple-literal"; + input = { + FOO = "bar"; + }; + expected = '' + export FOO="bar" + ''; + } + { + name = "null-is-unset"; + input = { + FOO = null; + }; + expected = '' + unset FOO + ''; + } + { + name = "explicit-unset"; + input = { + FOO = { + unset = true; + }; + }; + expected = '' + unset FOO + ''; + } + { + name = "fallback"; + input = { + EDITOR = { + value = "vim"; + fallback = true; + }; + }; + expected = '' + if [ -z "''${EDITOR+set}" ]; then + export EDITOR="vim" + fi + ''; + } + { + name = "escaped-literal"; + input = { + # Double quotes and backslashes survive unmangled inside the + # double-quoted export. + MSG = ''He said "hi" and went \home''; + }; + expected = '' + export MSG="He said \"hi\" and went \\home" + ''; + } + ]; + + runCase = + { name, input, expected }: + let + actual = wlib.renderEnvString input; + in + if actual == expected then + "PASS: ${name}" + else + throw '' + FAIL: ${name} + expected: + ${expected} + --- + actual: + ${actual} + ''; + + results = map runCase cases; + + # Structural assertions that check _prefixes/_contents_ of the + # generated snippet rather than exact bytes, because the join + # helper injects a bunch of shell glue that is annoying to match + # exactly. + prefixInput = { + PATH.prefix = [ "/opt/bin" ]; + }; + prefixOut = wlib.renderEnvString prefixInput; + prefixAsserts = + if lib.hasInfix "_wrapper_env_join" prefixOut && lib.hasInfix ''"/opt/bin"'' prefixOut then + "PASS: prefix case includes join helper and literal" + else + throw "FAIL: prefix case missing expected content:\n${prefixOut}"; + + valuesInput = { + FOO.values = [ + "A" + (wlib.envRef "BAR") + "C" + ]; + }; + valuesOut = wlib.renderEnvString valuesInput; + valuesAsserts = + if + lib.hasInfix ''"A"'' valuesOut + && lib.hasInfix ''"''${BAR-}"'' valuesOut + && lib.hasInfix ''"C"'' valuesOut + then + "PASS: values case renders literals and envRef" + else + throw "FAIL: values case missing expected content:\n${valuesOut}"; + + allResults = results ++ [ + prefixAsserts + valuesAsserts + ]; +in +pkgs.runCommand "env-render-string-test" { } '' + cat <<'EOF' + ${lib.concatStringsSep "\n" allResults} + EOF + touch $out +'' diff --git a/checks/env-values-envref.nix b/checks/env-values-envref.nix new file mode 100644 index 0000000..12748a5 --- /dev/null +++ b/checks/env-values-envref.nix @@ -0,0 +1,63 @@ +{ + pkgs, + self, +}: + +# `values` list with explicit envRef placeholders for maximum +# flexibility: put the existing value anywhere in the resulting +# string. +let + wlib = self.lib; + + showEnv = pkgs.writeShellScriptBin "show-env" '' + printf 'LD_LIBRARY_PATH=%s\n' "''${LD_LIBRARY_PATH-}" + ''; + + wrapped = + (wlib.wrapModule ( + { config, ... }: + { + config.package = showEnv; + config.env.LD_LIBRARY_PATH.values = [ + "/opt/lib" + (wlib.envRef "LD_LIBRARY_PATH") + "/other/lib" + ]; + } + ).apply { pkgs = pkgs; }).wrapper; +in +pkgs.runCommand "env-values-envref-test" { } '' + set -eu + script="${wrapped}/bin/show-env" + + # Unset case: inner envRef drops out, surrounding values joined. + unset_result=$(unset LD_LIBRARY_PATH && "$script" | grep '^LD_LIBRARY_PATH=' | cut -d= -f2-) + if [ "$unset_result" = "/opt/lib:/other/lib" ]; then + echo "PASS: unset case: $unset_result" + else + echo "FAIL: unset case: '$unset_result' (expected /opt/lib:/other/lib)" + cat "$script" + exit 1 + fi + + # Set case: envRef expands in place, preserving order. + set_result=$(LD_LIBRARY_PATH=/user/lib "$script" | grep '^LD_LIBRARY_PATH=' | cut -d= -f2-) + if [ "$set_result" = "/opt/lib:/user/lib:/other/lib" ]; then + echo "PASS: set case: $set_result" + else + echo "FAIL: set case: '$set_result' (expected /opt/lib:/user/lib:/other/lib)" + exit 1 + fi + + # Empty-string case: an empty existing value is treated like unset + # so we don't leave stray separators. + empty_result=$(LD_LIBRARY_PATH="" "$script" | grep '^LD_LIBRARY_PATH=' | cut -d= -f2-) + if [ "$empty_result" = "/opt/lib:/other/lib" ]; then + echo "PASS: empty-string case: $empty_result" + else + echo "FAIL: empty-string case: '$empty_result' (expected /opt/lib:/other/lib)" + exit 1 + fi + + touch $out +'' diff --git a/lib/default.nix b/lib/default.nix index 2957575..1788ad5 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -123,6 +123,190 @@ let in ''"${escaped}"''; + /** + Mark an entry inside an `env..values` / `prefix` / `suffix` + list as a reference to another environment variable, expanded at + runtime by the wrapper rather than treated as a literal string. + + # Arguments + + - `name`: the name of the environment variable to reference. + + # Example + + ```nix + env.LD_LIBRARY_PATH.values = [ + "/opt/lib" + (wlib.envRef "LD_LIBRARY_PATH") + "/other/lib" + ]; + ``` + + Empty/unset references drop out at runtime, so there are no + stray separators if the referenced variable isn't set. + */ + envRef = name: { + _type = "envRef"; + inherit name; + default = ""; + }; + + /** + Normalise an `env` attribute set (as accepted by `wrapPackage` or + the `env` module option) into the canonical per-entry form with + all fields populated. Accepts: + + - a plain string (becomes `{ value = ; }`) + - `null` (becomes `{ unset = true; }`) + - a partial attrset with any subset of the structured options + */ + normaliseEnvEntry = + entry: + let + defaults = { + value = null; + values = [ ]; + prefix = [ ]; + suffix = [ ]; + separator = ":"; + fallback = false; + unset = false; + }; + raw = + if entry == null then + { unset = true; } + else if builtins.isString entry then + { value = entry; } + else if builtins.isAttrs entry then + entry + else + throw "wrappers: unsupported env entry of type ${builtins.typeOf entry}"; + in + defaults // raw; + + /** + Render an `env` attribute set into a shell snippet that sets the + corresponding environment variables when sourced/evaluated. Used + internally by `wrapPackage` but exposed so downstream callers can + render env snippets themselves (e.g. for testing or composition). + + Returns the empty string when there is nothing to emit. + */ + renderEnvString = + env: + let + shellEscape = + s: + let + escaped = lib.replaceStrings [ ''\'' ''"'' ] [ ''\\'' ''\"'' ] (toString s); + in + ''"${escaped}"''; + + renderPart = + part: + if builtins.isString part then + shellEscape part + else if (part._type or null) == "envRef" then + let + defaultEscaped = lib.replaceStrings [ ''\'' ''"'' ] [ ''\\'' ''\"'' ] ( + part.default or "" + ); + in + ''"''${${part.name}-${defaultEscaped}}"'' + else + throw "wrappers: invalid env value part: ${lib.generators.toPretty { } part}"; + + effectiveParts = + name: entry: + let + middle = + if entry.values != [ ] then + (lib.optional (entry.value != null) entry.value) ++ entry.values + else if entry.value != null then + [ entry.value ] + else if entry.prefix != [ ] || entry.suffix != [ ] then + [ + { + _type = "envRef"; + inherit name; + default = ""; + } + ] + else + [ ]; + in + entry.prefix ++ middle ++ entry.suffix; + + entries = lib.mapAttrs (_: normaliseEnvEntry) env; + + rendered = lib.mapAttrsToList ( + name: entry: + if entry.unset then + { + needsJoin = false; + body = "unset ${name}"; + } + else + let + parts = effectiveParts name entry; + isTrivialLiteral = + lib.length parts == 1 + && builtins.isString (lib.head parts) + && entry.values == [ ] + && entry.prefix == [ ] + && entry.suffix == [ ]; + body = + if parts == [ ] then + null + else if isTrivialLiteral then + "export ${name}=${shellEscape (lib.head parts)}" + else + let + renderedParts = map renderPart parts; + sepArg = shellEscape entry.separator; + in + "export ${name}=\"$(_wrapper_env_join ${sepArg} ${lib.concatStringsSep " " renderedParts})\""; + wrapped = + if body == null then + null + else if entry.fallback then + '' + if [ -z "''${${name}+set}" ]; then + ${body} + fi'' + else + body; + in + { + needsJoin = !isTrivialLiteral && body != null; + body = wrapped; + } + ) entries; + + nonEmpty = lib.filter (r: r.body != null) rendered; + needsJoin = lib.any (r: r.needsJoin) nonEmpty; + + joinHelper = '' + _wrapper_env_join() { + local _sep="$1" + shift + local _out="" + local _p + for _p in "$@"; do + [ -n "$_p" ] || continue + _out="''${_out:+$_out$_sep}$_p" + done + printf '%s' "$_out" + }''; + in + if nonEmpty == [ ] then + "" + else + lib.concatStringsSep "\n" ( + (lib.optional needsJoin joinHelper) ++ (map (r: r.body) nonEmpty) + ) + + "\n"; + /** A collection of types for wrapper modules. For now this only contains a file type. @@ -253,7 +437,7 @@ let inherit modules class specialArgs; }; - modules = lib.genAttrs [ "package" "flags" "command" "wrapper" "meta" "systemd" ] ( + modules = lib.genAttrs [ "package" "flags" "env" "command" "wrapper" "meta" "systemd" ] ( name: import ./modules/${name}.nix ); @@ -482,14 +666,7 @@ let inherit (pkgs) lndir; # Generate environment variable exports - envString = - if env == { } then - "" - else - lib.concatStringsSep "\n" ( - lib.mapAttrsToList (name: value: ''export ${name}="${toString value}"'') env - ) - + "\n"; + envString = wrapperLib.renderEnvString env; # Generate flag arguments with proper line breaks and indentation flagsString = @@ -698,6 +875,9 @@ let escapeShellArgWithEnv generateArgsFromFlags flagToArgs + envRef + renderEnvString + normaliseEnvEntry ; }; in diff --git a/lib/modules/command.nix b/lib/modules/command.nix index 9949432..9c8134a 100644 --- a/lib/modules/command.nix +++ b/lib/modules/command.nix @@ -9,6 +9,7 @@ imports = [ wlib.modules.package wlib.modules.flags + wlib.modules.env ]; options.args = lib.mkOption { type = lib.types.listOf lib.types.str; @@ -28,13 +29,6 @@ These packages will be added to the wrapper's runtime dependencies, ensuring they are available when the wrapped program is executed. ''; }; - options.env = lib.mkOption { - type = lib.types.attrsOf lib.types.str; - default = { }; - description = '' - Environment variables to set in the wrapper. - ''; - }; options.preHook = lib.mkOption { type = lib.types.str; default = ""; diff --git a/lib/modules/env.nix b/lib/modules/env.nix new file mode 100644 index 0000000..6e2e11d --- /dev/null +++ b/lib/modules/env.nix @@ -0,0 +1,202 @@ +{ + lib, + wlib, + config, + ... +}: +let + # A part of an env value list: either a literal string or a reference + # to another environment variable (produced by `wlib.envRef`). We + # use a custom type with a runtime check so we don't run into + # type-merging quirks between `lib.types.str` and submodules. + envPartType = lib.mkOptionType { + name = "envValuePart"; + description = "environment variable value part (string or wlib.envRef)"; + check = + v: + builtins.isString v + || ( + builtins.isAttrs v + && v ? _type + && v._type == "envRef" + && v ? name + && builtins.isString v.name + ); + merge = lib.options.mergeEqualOption; + }; + + # A single env entry. The submodule is intentionally permissive: any + # combination of `value`/`values`/`prefix`/`suffix` is allowed, the + # renderer composes them in a defined order: + # + # prefix ++ (values OR [value] OR [envRef self]) ++ suffix + # + # Empty parts are filtered at runtime, so unset/empty env refs drop + # out without leaving stray separators. + envEntrySubmodule = lib.types.submodule { + options = { + value = lib.mkOption { + type = lib.types.nullOr ( + lib.types.coercedTo lib.types.path toString lib.types.str + ); + default = null; + description = '' + Literal string value for the variable. Setting a plain + string via `env.VAR = "..."` coerces to this field. Nix + paths are accepted and stringified automatically, matching + the behaviour of the old `attrsOf str` type. + ''; + }; + values = lib.mkOption { + type = lib.types.listOf envPartType; + default = [ ]; + description = '' + Explicit list of parts to join with `separator`. Parts may + be literal strings or env references produced by + `wlib.envRef "NAME"`. Empty parts are skipped at runtime, + so unset references don't leave dangling separators. + + If both `value` and `values` are set, `value` is spliced + into the middle of the resulting list (after `prefix` and + before `values`). This makes composition via `apply` + predictable: each module contributes more parts to the + list rather than fighting over a single string. + ''; + }; + prefix = lib.mkOption { + type = lib.types.listOf envPartType; + default = [ ]; + description = '' + Parts to prepend to the existing value of the variable. + Equivalent to `makeWrapper`'s `--prefix VAR SEP VAL`. The + existing value is implicitly referenced; if it is unset or + empty, it drops out cleanly. + ''; + }; + suffix = lib.mkOption { + type = lib.types.listOf envPartType; + default = [ ]; + description = '' + Parts to append to the existing value of the variable. + Equivalent to `makeWrapper`'s `--suffix VAR SEP VAL`. + ''; + }; + separator = lib.mkOption { + type = lib.types.str; + default = ":"; + description = '' + Separator used when joining list-valued entries. Defaults + to `:`, which is the conventional separator for PATH-like + variables. + ''; + }; + fallback = lib.mkOption { + type = lib.types.bool; + default = false; + description = '' + Only set the variable when it is not already set in the + environment. Uses `$\{VAR+set}` semantics: an empty but + set variable is left alone. Useful for "default" values + like `EDITOR`, `PAGER`, etc. + ''; + }; + unset = lib.mkOption { + type = lib.types.bool; + default = false; + description = '' + Unset the variable instead of assigning it. Takes + precedence over all other options on this entry. + ''; + }; + }; + }; + + # Type that users set `env` to. Plain strings, paths and `null` + # coerce into the submodule form so that existing + # `env.FOO = "bar"` keeps working (paths also accepted and + # stringified) and `env.FOO = null` is sugar for `{ unset = true; }`. + envValueType = lib.types.coercedTo ( + lib.types.nullOr (lib.types.either lib.types.str lib.types.path) + ) (v: if v == null then { unset = true; } else { value = toString v; }) envEntrySubmodule; +in +{ + _file = "lib/modules/env.nix"; + + options.env = lib.mkOption { + type = lib.types.attrsOf envValueType; + default = { }; + example = lib.literalExpression '' + { + # Simple literal. + FOO = "bar"; + + # Prepend to PATH, keeping the user's existing entries. + PATH.prefix = [ "/opt/bin" ]; + + # Build an XDG_DATA_DIRS-like value with an explicit list. + XDG_DATA_DIRS.values = [ + "/opt/share" + (wlib.envRef "XDG_DATA_DIRS") + "/usr/local/share" + ]; + + # Only set EDITOR when the user hasn't already picked one. + EDITOR = { value = "vim"; fallback = true; }; + + # Explicitly unset a variable. + OLD_VAR = null; + } + ''; + description = '' + Environment variables to set in the wrapper. + + Each entry accepts either a plain string (literal value), + `null` (unset the variable), or a structured attribute set with + the following fields: + + - `value`: literal string value. + - `values`: list of parts joined with `separator`. Parts may + be plain strings or env references produced by + `wlib.envRef "NAME"`. + - `prefix` / `suffix`: parts to splice around the existing + value of the variable. Empty or unset references drop out + cleanly, so no dangling separators are left behind. + - `separator`: join separator (default `:`). + - `fallback`: if true, only set the variable when it is not + already present in the caller's environment. + - `unset`: if true, emit `unset VAR` instead of an assignment. + + The plain-string and `null` forms coerce into this submodule, + so existing `env.FOO = "bar"` usage keeps working unchanged. + ''; + }; + + # Subset of `env` that resolves to a static literal string with no + # runtime composition. Used by the systemd integration, which cannot + # express bash-style prefix/suffix assignments natively. Entries + # using `prefix`, `suffix`, `values`, `fallback` or `unset` are + # omitted and must be set via `systemd.environment` directly if they + # need to reach the service file. + options.outputs.staticEnv = lib.mkOption { + type = lib.types.attrsOf lib.types.str; + internal = true; + readOnly = true; + description = '' + Subset of `env` that resolves to a plain literal string. Used + by integrations like systemd that cannot express runtime env + composition. Complex entries are silently dropped; set them via + the integration's own environment option if you need them. + ''; + default = lib.mapAttrs (_: entry: entry.value) ( + lib.filterAttrs ( + _: entry: + !entry.unset + && !entry.fallback + && entry.value != null + && entry.values == [ ] + && entry.prefix == [ ] + && entry.suffix == [ ] + ) config.env + ); + }; +} diff --git a/lib/modules/systemd.nix b/lib/modules/systemd.nix index 1993ca9..b553d9b 100644 --- a/lib/modules/systemd.nix +++ b/lib/modules/systemd.nix @@ -115,7 +115,7 @@ in in lib.concatStringsSep " " ([ config.exePath ] ++ map escapeForSystemd config.args) ); - environment = lib.mkDefault config.env; + environment = lib.mkDefault config.outputs.staticEnv; path = lib.mkDefault config.extraPackages; preStart = lib.mkIf (config.preHook != "") (lib.mkDefault config.preHook); postStop = lib.mkIf (config.postHook != "") (lib.mkDefault config.postHook); From 665ce952ea5bb671104c7793d1a443a1e67dd08a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Apr 2026 11:40:58 +0000 Subject: [PATCH 2/4] lib.modules.env: simplify API and move helpers into wlib.env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 53 +++---- README.md | 67 ++++----- checks/env-fallback-unset.nix | 63 -------- checks/env-if-unset.nix | 49 +++++++ checks/env-list-value.nix | 71 +++++++++ checks/env-prefix-suffix.nix | 155 -------------------- checks/env-render-string.nix | 136 ------------------ checks/env-render.nix | 103 +++++++++++++ checks/env-values-envref.nix | 63 -------- lib/default.nix | 263 +++++++++++++--------------------- lib/modules/env.nix | 186 ++++++------------------ 11 files changed, 412 insertions(+), 797 deletions(-) delete mode 100644 checks/env-fallback-unset.nix create mode 100644 checks/env-if-unset.nix create mode 100644 checks/env-list-value.nix delete mode 100644 checks/env-prefix-suffix.nix delete mode 100644 checks/env-render-string.nix create mode 100644 checks/env-render.nix delete mode 100644 checks/env-values-envref.nix diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cf1b23..8952580 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,15 +15,12 @@ were explicitly passing `flagSeparator = " "` to get separate args, remove it (or change to `null`). -- `env` option type changed from `attrsOf str` to a richer submodule - (see "Added" below). Plain-string and path values keep coercing to - the old behaviour, so `env.FOO = "bar"` is unchanged. Passthru - `wrapPackage` callers now get the structured form on - `passthru.env`; read `passthru.env..value` instead of - `passthru.env.` if you need the literal. The systemd - integration reads from `config.outputs.staticEnv` instead of - `config.env` and silently drops entries that can't be expressed as - a static literal (prefix/suffix, values, fallback, unset). +- `env` option type changed from `attrsOf str` to a small submodule + with `value` / `separator` / `ifUnset` / `unset`. Plain strings + and `null` keep working via coercion (`env.FOO = "bar"` and + `env.FOO = null` are unchanged). The systemd integration reads + from `config.outputs.staticEnv` instead of `config.env` and drops + any entries it can't express as a literal assignment. ### Added @@ -32,30 +29,24 @@ - `lib/modules/flags.nix`: flags module with per-flag ordering via `{ value, order }` submodules. Default order is 1000. Reading `config.flags` returns clean values (order is transparent). -- `lib/modules/env.nix`: env module with richer per-variable options - for safe composition, modelled on `makeWrapper`'s `--prefix` / - `--suffix` but usable through the NixOS module system. - - `env..value`: literal string (same as `env. = "..."`). - - `env..prefix` / `.suffix`: parts to splice around the - existing value of the variable. Empty or unset existing values - drop out cleanly with no stray separators. - - `env..values`: explicit list of parts to join, with - `wlib.envRef "OTHER"` placeholders for runtime env references. - - `env..separator`: join separator (default `:`). - - `env..fallback = true`: only set the variable when it is - not already set in the caller's environment (uses `${VAR+set}` - semantics). +- `lib/modules/env.nix`: env module with per-variable options for + safe composition through the NixOS module system. + - `env..value`: string for a simple literal, or a list of + parts to join with `separator`. List parts can be plain strings + or `wlib.env.ref "NAME"` runtime references. Empty/unset refs + drop out cleanly, so no dangling separators. + - `env..separator`: join separator for a list `value` + (default `:`). + - `env..ifUnset = true`: only apply when the caller's + environment doesn't already have the variable set. - `env..unset = true`: emit `unset VAR` instead of an assignment. `env.VAR = null` is sugar for this. - - List-valued entries (`values`, `prefix`, `suffix`) merge by - concatenation when composed via `apply`, so multiple modules can - stack contributions onto the same variable without fighting. -- `wlib.envRef :: name -> envRef`: marker used inside `values` / - `prefix` / `suffix` lists to reference another env variable at - runtime. Dropped cleanly if the referenced variable is unset. -- `wlib.renderEnvString :: env -> str`: pure helper that renders an - `env` attrset into the shell snippet the wrapper uses. Exposed - for testing and downstream composition. + - List `value`s merge by concatenation when composed via `apply`, + so modules stack contributions without fighting over a string. +- `wlib.env.ref NAME`: marker for a runtime env-variable reference + inside `env..value` lists. +- `wlib.env.render`: render an `env` attrset into a shell snippet. + Exposed for tests and downstream composition. - `outputs.staticEnv`: subset of `env` that resolves to a plain literal string, used by the systemd integration. - `wrapper.nix` injects `"$@"` into args at order 1001, controllable diff --git a/README.md b/README.md index eb6454e..bb452eb 100644 --- a/README.md +++ b/README.md @@ -93,61 +93,42 @@ wrappers.lib.wrapPackage { ### Environment Variables -Environment variables can be set in three forms, with the richer form -available both via `wrapPackage` and through the `env.` option -of wrapper modules. +Each `env.` entry is either a plain string, `null` (to unset), +or a small submodule: ```nix { env = { - # 1. Plain literal string (same as before): + # Literal: FOO = "bar"; - # 2. Null to explicitly unset a variable inherited from the - # caller's environment: + # Unset a variable inherited from the caller: NOISY_OLD_VAR = null; - # 3. Structured form with the following fields: - # - # - value: literal string - # - prefix / suffix: parts to splice around the existing value - # of the variable (like `makeWrapper --prefix/--suffix`). - # Empty or unset existing values drop out cleanly. - # - values: explicit list of parts, joined with `separator`. - # Use `wlib.envRef "OTHER"` to reference another variable - # at runtime. - # - separator: join separator (default ":") - # - fallback: only set if the variable isn't already set - # - unset: emit `unset VAR` (takes precedence) - - # Prepend to PATH, keeping the caller's existing entries: - PATH.prefix = [ "/opt/bin" ]; - - # Append to XDG_DATA_DIRS: - XDG_DATA_DIRS.suffix = [ "/opt/share" ]; - - # Build LD_LIBRARY_PATH with the existing value somewhere in the - # middle, not just at the edges: - LD_LIBRARY_PATH.values = [ - "/opt/lib" - (wrappers.lib.envRef "LD_LIBRARY_PATH") - "/other/lib" - ]; - - # Only set EDITOR if the user hasn't picked one already: - EDITOR = { - value = "vim"; - fallback = true; - }; + # Prepend to PATH using a list and `wlib.env.ref`. The ref + # expands to the caller's existing PATH at runtime; if it's + # unset or empty, the ref drops out and no stray separators + # are left behind. + PATH.value = [ "/opt/bin" (wrappers.lib.env.ref "PATH") ]; + + # Only set EDITOR if the caller hasn't already picked one. + EDITOR = { value = "vim"; ifUnset = true; }; }; } ``` -`prefix`, `suffix` and `values` are lists, so they compose via module -merging: multiple modules (or multiple `apply` calls) contributing to -the same variable stack their contributions instead of fighting over -a single string. Empty parts are filtered at runtime, so unset env -references never leave behind dangling separators. +Submodule options: +- `value`: literal string *or* a list of parts joined with + `separator`. List parts can be plain strings or + `wlib.env.ref "NAME"` runtime references. +- `separator`: join separator for list values (default `:`). +- `ifUnset`: only apply when the caller's environment doesn't + already have the variable set. +- `unset`: unset the variable (takes precedence over `value`). + +List `value`s merge by concatenation when composed via `apply`, so +multiple modules stack contributions onto the same variable without +fighting over a single string. ### Creating Custom Wrapper Modules diff --git a/checks/env-fallback-unset.nix b/checks/env-fallback-unset.nix deleted file mode 100644 index 8b597a6..0000000 --- a/checks/env-fallback-unset.nix +++ /dev/null @@ -1,63 +0,0 @@ -{ - pkgs, - self, -}: - -# Fallback (only-set-if-unset) and explicit unset handling. -let - wlib = self.lib; - - showEnv = pkgs.writeShellScriptBin "show-env" '' - printf 'EDITOR=%s\n' "''${EDITOR-}" - printf 'BLOAT=%s\n' "''${BLOAT-}" - ''; - - wrapped = - (wlib.wrapModule ( - { config, ... }: - { - config.package = showEnv; - config.env.EDITOR = { - value = "vim"; - fallback = true; - }; - # `null` is sugar for `unset = true`, exercise both forms. - config.env.BLOAT = null; - } - ).apply { pkgs = pkgs; }).wrapper; -in -pkgs.runCommand "env-fallback-unset-test" { } '' - set -eu - script="${wrapped}/bin/show-env" - - # Fallback: EDITOR unset → set to vim. - editor_unset=$(unset EDITOR && "$script" | grep '^EDITOR=' | cut -d= -f2-) - if [ "$editor_unset" = "vim" ]; then - echo "PASS: fallback applied when unset" - else - echo "FAIL: fallback unset case: '$editor_unset'" - cat "$script" - exit 1 - fi - - # Fallback: EDITOR already set → preserved. - editor_set=$(EDITOR=nano "$script" | grep '^EDITOR=' | cut -d= -f2-) - if [ "$editor_set" = "nano" ]; then - echo "PASS: fallback preserved existing value" - else - echo "FAIL: fallback preserved case: '$editor_set'" - exit 1 - fi - - # Unset: BLOAT should be unset even if caller exports it. - bloat_result=$(BLOAT=garbage "$script" | grep '^BLOAT=' | cut -d= -f2-) - if [ "$bloat_result" = "" ]; then - echo "PASS: unset overrides caller env" - else - echo "FAIL: unset case: '$bloat_result'" - cat "$script" - exit 1 - fi - - touch $out -'' diff --git a/checks/env-if-unset.nix b/checks/env-if-unset.nix new file mode 100644 index 0000000..74178a8 --- /dev/null +++ b/checks/env-if-unset.nix @@ -0,0 +1,49 @@ +{ + pkgs, + self, +}: + +# `ifUnset` (only set if caller hasn't) and explicit unset. +let + wlib = self.lib; + + showEnv = pkgs.writeShellScriptBin "show-env" '' + printf 'EDITOR=%s\n' "''${EDITOR-}" + printf 'BLOAT=%s\n' "''${BLOAT-}" + ''; + + wrapped = + (wlib.wrapModule ( + { config, ... }: + { + config.package = showEnv; + config.env.EDITOR = { + value = "vim"; + ifUnset = true; + }; + # `null` sugar for unset. + config.env.BLOAT = null; + } + ).apply { pkgs = pkgs; }).wrapper; +in +pkgs.runCommand "env-if-unset-test" { } '' + set -eu + script="${wrapped}/bin/show-env" + + # ifUnset applied when EDITOR is unset. + r=$(unset EDITOR && "$script" | grep '^EDITOR=' | cut -d= -f2-) + [ "$r" = "vim" ] || { echo "FAIL: ifUnset unset: '$r'"; cat "$script"; exit 1; } + echo "PASS: ifUnset applies when unset" + + # ifUnset yields to an existing value. + r=$(EDITOR=nano "$script" | grep '^EDITOR=' | cut -d= -f2-) + [ "$r" = "nano" ] || { echo "FAIL: ifUnset overrode existing: '$r'"; exit 1; } + echo "PASS: ifUnset preserves existing" + + # Explicit unset beats caller env. + r=$(BLOAT=garbage "$script" | grep '^BLOAT=' | cut -d= -f2-) + [ "$r" = "" ] || { echo "FAIL: unset: '$r'"; cat "$script"; exit 1; } + echo "PASS: unset overrides caller env" + + touch $out +'' diff --git a/checks/env-list-value.nix b/checks/env-list-value.nix new file mode 100644 index 0000000..470bfe5 --- /dev/null +++ b/checks/env-list-value.nix @@ -0,0 +1,71 @@ +{ + pkgs, + self, +}: + +# End-to-end test for list-valued `env..value` with +# `wlib.env.ref` to prepend to an existing variable. Also exercises +# composition via `apply`: lists merge by concatenation. +let + wlib = self.lib; + + showVar = pkgs.writeShellScriptBin "show-var" '' + printf 'TEST_VAR=%s\n' "''${TEST_VAR-}" + ''; + + base = wlib.wrapModule ( + { config, ... }: + { + config.package = showVar; + config.env.TEST_VAR.value = [ + "/base-front" + (wlib.env.ref "TEST_VAR") + "/base-back" + ]; + } + ); + + extended = (base.apply { pkgs = pkgs; }).apply { + # List merging via the module system: apply concatenates. + env.TEST_VAR.value = [ "/extra" ]; + }; + + wrapped = extended.wrapper; +in +pkgs.runCommand "env-list-value-test" { } '' + set -eu + script="${wrapped}/bin/show-var" + + # Case 1: TEST_VAR unset — envRef drops out, no stray colons. + r1=$(unset TEST_VAR && "$script" | grep '^TEST_VAR=' | cut -d= -f2-) + case "$r1" in + *::*) + echo "FAIL: unset case has stray separator: '$r1'" + cat "$script"; exit 1 ;; + :*|*:) + echo "FAIL: unset case has leading/trailing colon: '$r1'" ; exit 1 ;; + esac + case "$r1" in + */base-front*) ;; + *) echo "FAIL: base-front missing: '$r1'"; exit 1 ;; + esac + case "$r1" in + */base-back*) ;; + *) echo "FAIL: base-back missing: '$r1'"; exit 1 ;; + esac + case "$r1" in + */extra*) ;; + *) echo "FAIL: extra missing (list merge via apply): '$r1'"; exit 1 ;; + esac + echo "PASS: unset case: $r1" + + # Case 2: TEST_VAR=/mid — envRef expands in place. + r2=$(TEST_VAR=/mid "$script" | grep '^TEST_VAR=' | cut -d= -f2-) + case "$r2" in + */mid*) ;; + *) echo "FAIL: existing value lost: '$r2'"; exit 1 ;; + esac + echo "PASS: set case: $r2" + + touch $out +'' diff --git a/checks/env-prefix-suffix.nix b/checks/env-prefix-suffix.nix deleted file mode 100644 index acb4b9e..0000000 --- a/checks/env-prefix-suffix.nix +++ /dev/null @@ -1,155 +0,0 @@ -{ - pkgs, - self, -}: - -# Prefix/suffix composition for env variables. -# -# Goals: -# 1. A `prefix`/`suffix` entry splices around the existing value of -# the variable and leaves no dangling separator when that variable -# is unset or empty. -# 2. Multiple modules contributing to the same variable via `apply` -# compose via list concatenation, not via string overwriting. -# -# We use a custom variable name (WRAPPER_TEST_VAR) instead of PATH so -# that unsetting it inside a subshell doesn't break the shell itself. -let - wlib = self.lib; - - showVar = pkgs.writeShellScriptBin "show-var" '' - printf 'WRAPPER_TEST_VAR=%s\n' "''${WRAPPER_TEST_VAR-}" - ''; - - base = wlib.wrapModule ( - { config, ... }: - { - config.package = showVar; - config.env.WRAPPER_TEST_VAR.prefix = [ "/base-pre" ]; - config.env.WRAPPER_TEST_VAR.suffix = [ "/base-post" ]; - } - ); - - extended = (base.apply { pkgs = pkgs; }).apply { - env.WRAPPER_TEST_VAR.prefix = [ "/extra-pre" ]; - env.WRAPPER_TEST_VAR.suffix = [ "/extra-post" ]; - }; - - wrapped = extended.wrapper; -in -pkgs.runCommand "env-prefix-suffix-test" { } '' - set -eu - script="${wrapped}/bin/show-var" - if [ ! -f "$script" ]; then - echo "FAIL: wrapper script not found" - exit 1 - fi - - # Case 1: WRAPPER_TEST_VAR unset — prefix and suffix join with no - # dangling separators, no stray reference to the empty existing - # value. - unset WRAPPER_TEST_VAR - result_unset=$("$script" | grep '^WRAPPER_TEST_VAR=' | cut -d= -f2-) - case "$result_unset" in - *:*:*) - # good, at least three parts (two prefixes + two suffixes all - # joined together — order is list-concatenation order) - ;; - *) - echo "FAIL: unset case collapsed too aggressively: '$result_unset'" - cat "$script" - exit 1 - ;; - esac - case "$result_unset" in - *::*) - echo "FAIL: unset case has double colon (stray separator): '$result_unset'" - cat "$script" - exit 1 - ;; - :*|*:) - echo "FAIL: unset case has leading/trailing colon: '$result_unset'" - exit 1 - ;; - *) - echo "PASS: unset case has no dangling separators: $result_unset" - ;; - esac - case "$result_unset" in - */base-pre*) ;; - *) - echo "FAIL: base prefix missing: '$result_unset'" - exit 1 - ;; - esac - case "$result_unset" in - */extra-pre*) ;; - *) - echo "FAIL: extra prefix missing (apply list merge?): '$result_unset'" - exit 1 - ;; - esac - case "$result_unset" in - */base-post*) ;; - *) - echo "FAIL: base suffix missing: '$result_unset'" - exit 1 - ;; - esac - case "$result_unset" in - */extra-post*) ;; - *) - echo "FAIL: extra suffix missing (apply list merge?): '$result_unset'" - exit 1 - ;; - esac - - # Case 2: WRAPPER_TEST_VAR set — prefixes appear before, suffixes - # after, and the existing value survives in the middle. - export WRAPPER_TEST_VAR=/user-value - result_set=$("$script" | grep '^WRAPPER_TEST_VAR=' | cut -d= -f2-) - case "$result_set" in - */user-value*) - echo "PASS: existing value preserved: $result_set" - ;; - *) - echo "FAIL: existing value lost: '$result_set'" - exit 1 - ;; - esac - # Prefixes must appear before the existing middle value. - prefix_part="''${result_set%%/user-value*}" - case "$prefix_part" in - */base-pre*) ;; - *) - echo "FAIL: base prefix not before existing value: '$result_set'" - exit 1 - ;; - esac - # Suffixes must appear after. - suffix_part="''${result_set##*/user-value}" - case "$suffix_part" in - */base-post*) ;; - *) - echo "FAIL: base suffix not after existing value: '$result_set'" - exit 1 - ;; - esac - - # Case 3: WRAPPER_TEST_VAR set to the empty string. Empty is filtered - # the same as unset so the middle drops out cleanly. - export WRAPPER_TEST_VAR="" - result_empty=$("$script" | grep '^WRAPPER_TEST_VAR=' | cut -d= -f2-) - case "$result_empty" in - *::*) - echo "FAIL: empty existing value left a double colon: '$result_empty'" - exit 1 - ;; - *) - echo "PASS: empty existing value collapsed cleanly: $result_empty" - ;; - esac - - echo "SUCCESS: env prefix/suffix composition works" - touch $out -'' diff --git a/checks/env-render-string.nix b/checks/env-render-string.nix deleted file mode 100644 index 9b7933a..0000000 --- a/checks/env-render-string.nix +++ /dev/null @@ -1,136 +0,0 @@ -{ - pkgs, - self, -}: - -# Pure rendering tests for `wlib.renderEnvString`. These don't build -# a wrapper, they just assert the generated shell snippet is what we -# expect. Keeps the feedback loop tight when iterating on escaping. -let - lib = pkgs.lib; - wlib = self.lib; - - cases = [ - { - name = "empty"; - input = { }; - expected = ""; - } - { - name = "simple-literal"; - input = { - FOO = "bar"; - }; - expected = '' - export FOO="bar" - ''; - } - { - name = "null-is-unset"; - input = { - FOO = null; - }; - expected = '' - unset FOO - ''; - } - { - name = "explicit-unset"; - input = { - FOO = { - unset = true; - }; - }; - expected = '' - unset FOO - ''; - } - { - name = "fallback"; - input = { - EDITOR = { - value = "vim"; - fallback = true; - }; - }; - expected = '' - if [ -z "''${EDITOR+set}" ]; then - export EDITOR="vim" - fi - ''; - } - { - name = "escaped-literal"; - input = { - # Double quotes and backslashes survive unmangled inside the - # double-quoted export. - MSG = ''He said "hi" and went \home''; - }; - expected = '' - export MSG="He said \"hi\" and went \\home" - ''; - } - ]; - - runCase = - { name, input, expected }: - let - actual = wlib.renderEnvString input; - in - if actual == expected then - "PASS: ${name}" - else - throw '' - FAIL: ${name} - expected: - ${expected} - --- - actual: - ${actual} - ''; - - results = map runCase cases; - - # Structural assertions that check _prefixes/_contents_ of the - # generated snippet rather than exact bytes, because the join - # helper injects a bunch of shell glue that is annoying to match - # exactly. - prefixInput = { - PATH.prefix = [ "/opt/bin" ]; - }; - prefixOut = wlib.renderEnvString prefixInput; - prefixAsserts = - if lib.hasInfix "_wrapper_env_join" prefixOut && lib.hasInfix ''"/opt/bin"'' prefixOut then - "PASS: prefix case includes join helper and literal" - else - throw "FAIL: prefix case missing expected content:\n${prefixOut}"; - - valuesInput = { - FOO.values = [ - "A" - (wlib.envRef "BAR") - "C" - ]; - }; - valuesOut = wlib.renderEnvString valuesInput; - valuesAsserts = - if - lib.hasInfix ''"A"'' valuesOut - && lib.hasInfix ''"''${BAR-}"'' valuesOut - && lib.hasInfix ''"C"'' valuesOut - then - "PASS: values case renders literals and envRef" - else - throw "FAIL: values case missing expected content:\n${valuesOut}"; - - allResults = results ++ [ - prefixAsserts - valuesAsserts - ]; -in -pkgs.runCommand "env-render-string-test" { } '' - cat <<'EOF' - ${lib.concatStringsSep "\n" allResults} - EOF - touch $out -'' diff --git a/checks/env-render.nix b/checks/env-render.nix new file mode 100644 index 0000000..c07d1e1 --- /dev/null +++ b/checks/env-render.nix @@ -0,0 +1,103 @@ +{ + pkgs, + self, +}: + +# Pure tests for `wlib.env.render`. No wrapper build, just string +# comparison so iteration on escaping stays fast. +let + lib = pkgs.lib; + wlib = self.lib; + + cases = [ + { + name = "empty"; + input = { }; + expected = ""; + } + { + name = "literal"; + input.FOO = "bar"; + expected = '' + export FOO="bar" + ''; + } + { + name = "null-unsets"; + input.FOO = null; + expected = '' + unset FOO + ''; + } + { + name = "explicit-unset"; + input.FOO.unset = true; + expected = '' + unset FOO + ''; + } + { + name = "if-unset"; + input.EDITOR = { + value = "vim"; + ifUnset = true; + }; + expected = '' + if [ -z "''${EDITOR:-}" ]; then + export EDITOR="vim" + fi + ''; + } + { + name = "escaped"; + input.MSG = ''He said "hi" and went \home''; + expected = '' + export MSG="He said \"hi\" and went \\home" + ''; + } + ]; + + run = + { name, input, expected }: + let + actual = wlib.env.render input; + in + if actual == expected then + "PASS: ${name}" + else + throw '' + FAIL: ${name} + expected: ${builtins.toJSON expected} + actual: ${builtins.toJSON actual} + ''; + + results = map run cases; + + # Structural checks for list values: the join helper and all + # literal parts must show up somewhere in the rendered snippet. + listOut = wlib.env.render { + PATH.value = [ + "/opt/bin" + (wlib.env.ref "PATH") + "/extra/bin" + ]; + }; + listAssert = + if + lib.hasInfix "_wrapper_env_join" listOut + && lib.hasInfix ''"/opt/bin"'' listOut + && lib.hasInfix ''"''${PATH-}"'' listOut + && lib.hasInfix ''"/extra/bin"'' listOut + then + "PASS: list-value renders helper + literals + envRef" + else + throw "FAIL: list-value rendering:\n${listOut}"; + + all = results ++ [ listAssert ]; +in +pkgs.runCommand "env-render-test" { } '' + cat <<'EOF' + ${lib.concatStringsSep "\n" all} + EOF + touch $out +'' diff --git a/checks/env-values-envref.nix b/checks/env-values-envref.nix deleted file mode 100644 index 12748a5..0000000 --- a/checks/env-values-envref.nix +++ /dev/null @@ -1,63 +0,0 @@ -{ - pkgs, - self, -}: - -# `values` list with explicit envRef placeholders for maximum -# flexibility: put the existing value anywhere in the resulting -# string. -let - wlib = self.lib; - - showEnv = pkgs.writeShellScriptBin "show-env" '' - printf 'LD_LIBRARY_PATH=%s\n' "''${LD_LIBRARY_PATH-}" - ''; - - wrapped = - (wlib.wrapModule ( - { config, ... }: - { - config.package = showEnv; - config.env.LD_LIBRARY_PATH.values = [ - "/opt/lib" - (wlib.envRef "LD_LIBRARY_PATH") - "/other/lib" - ]; - } - ).apply { pkgs = pkgs; }).wrapper; -in -pkgs.runCommand "env-values-envref-test" { } '' - set -eu - script="${wrapped}/bin/show-env" - - # Unset case: inner envRef drops out, surrounding values joined. - unset_result=$(unset LD_LIBRARY_PATH && "$script" | grep '^LD_LIBRARY_PATH=' | cut -d= -f2-) - if [ "$unset_result" = "/opt/lib:/other/lib" ]; then - echo "PASS: unset case: $unset_result" - else - echo "FAIL: unset case: '$unset_result' (expected /opt/lib:/other/lib)" - cat "$script" - exit 1 - fi - - # Set case: envRef expands in place, preserving order. - set_result=$(LD_LIBRARY_PATH=/user/lib "$script" | grep '^LD_LIBRARY_PATH=' | cut -d= -f2-) - if [ "$set_result" = "/opt/lib:/user/lib:/other/lib" ]; then - echo "PASS: set case: $set_result" - else - echo "FAIL: set case: '$set_result' (expected /opt/lib:/user/lib:/other/lib)" - exit 1 - fi - - # Empty-string case: an empty existing value is treated like unset - # so we don't leave stray separators. - empty_result=$(LD_LIBRARY_PATH="" "$script" | grep '^LD_LIBRARY_PATH=' | cut -d= -f2-) - if [ "$empty_result" = "/opt/lib:/other/lib" ]; then - echo "PASS: empty-string case: $empty_result" - else - echo "FAIL: empty-string case: '$empty_result' (expected /opt/lib:/other/lib)" - exit 1 - fi - - touch $out -'' diff --git a/lib/default.nix b/lib/default.nix index 1788ad5..cd50e30 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -124,188 +124,123 @@ let ''"${escaped}"''; /** - Mark an entry inside an `env..values` / `prefix` / `suffix` - list as a reference to another environment variable, expanded at - runtime by the wrapper rather than treated as a literal string. + Environment variable helpers. - # Arguments - - - `name`: the name of the environment variable to reference. + - `env.ref NAME`: marker placed inside an `env..value` list + to reference another variable at runtime. Empty/unset refs + drop out cleanly via a runtime join helper, so there are no + dangling separators. - # Example + - `env.render`: render an `env` attrset into a shell snippet. + Used by `wrapPackage` and exposed for tests. - ```nix - env.LD_LIBRARY_PATH.values = [ - "/opt/lib" - (wlib.envRef "LD_LIBRARY_PATH") - "/other/lib" - ]; - ``` + `env.render` accepts the same shapes as the `env` module option: - Empty/unset references drop out at runtime, so there are no - stray separators if the referenced variable isn't set. + - `env.FOO = "bar"` literal + - `env.FOO = null` unset + - `env.FOO.value = "bar"` literal (explicit) + - `env.FOO.value = [ "a" "b" ]` list, joined with `separator` + - `env.FOO.value = [ "/opt/bin" (env.ref "PATH") ]` prepend + - `env.FOO.ifUnset = true` only set when caller hasn't + - `env.FOO.unset = true` unset (takes precedence) */ - envRef = name: { - _type = "envRef"; - inherit name; - default = ""; - }; - - /** - Normalise an `env` attribute set (as accepted by `wrapPackage` or - the `env` module option) into the canonical per-entry form with - all fields populated. Accepts: - - - a plain string (becomes `{ value = ; }`) - - `null` (becomes `{ unset = true; }`) - - a partial attrset with any subset of the structured options - */ - normaliseEnvEntry = - entry: + env = let - defaults = { - value = null; - values = [ ]; - prefix = [ ]; - suffix = [ ]; - separator = ":"; - fallback = false; - unset = false; - }; - raw = - if entry == null then + esc = + s: ''"${lib.replaceStrings [ ''\'' ''"'' ] [ ''\\'' ''\"'' ] (toString s)}"''; + + norm = + e: + if e == null then { unset = true; } - else if builtins.isString entry then - { value = entry; } - else if builtins.isAttrs entry then - entry + else if builtins.isString e || builtins.isList e then + { value = e; } else - throw "wrappers: unsupported env entry of type ${builtins.typeOf entry}"; - in - defaults // raw; - - /** - Render an `env` attribute set into a shell snippet that sets the - corresponding environment variables when sourced/evaluated. Used - internally by `wrapPackage` but exposed so downstream callers can - render env snippets themselves (e.g. for testing or composition). - - Returns the empty string when there is nothing to emit. - */ - renderEnvString = - env: - let - shellEscape = - s: - let - escaped = lib.replaceStrings [ ''\'' ''"'' ] [ ''\\'' ''\"'' ] (toString s); - in - ''"${escaped}"''; + e; renderPart = - part: - if builtins.isString part then - shellEscape part - else if (part._type or null) == "envRef" then - let - defaultEscaped = lib.replaceStrings [ ''\'' ''"'' ] [ ''\\'' ''\"'' ] ( - part.default or "" - ); - in - ''"''${${part.name}-${defaultEscaped}}"'' + p: + if builtins.isString p then + esc p + else if (p._type or null) == "envRef" then + ''"''${${p.name}-}"'' else - throw "wrappers: invalid env value part: ${lib.generators.toPretty { } part}"; + throw "wlib.env.render: invalid part ${lib.generators.toPretty { } p}"; - effectiveParts = - name: entry: + line = + name: raw: let - middle = - if entry.values != [ ] then - (lib.optional (entry.value != null) entry.value) ++ entry.values - else if entry.value != null then - [ entry.value ] - else if entry.prefix != [ ] || entry.suffix != [ ] then - [ - { - _type = "envRef"; - inherit name; - default = ""; - } - ] + e = norm raw; + unset = e.unset or false; + ifUnset = e.ifUnset or false; + val = e.value or null; + parts = + if val == null then + [ ] + else if builtins.isList val then + val + else + [ val ]; + simple = lib.length parts == 1 && builtins.isString (lib.head parts); + sep = e.separator or ":"; + body = + if unset then + "unset ${name}" + else if parts == [ ] then + null + else if simple then + "export ${name}=${esc (lib.head parts)}" else - [ ]; + "export ${name}=\"$(_wrapper_env_join ${esc sep} ${ + lib.concatStringsSep " " (map renderPart parts) + })\""; + wrapped = + if body == null then + null + else if ifUnset && !unset then + '' + if [ -z "''${${name}:-}" ]; then + ${body} + fi'' + else + body; in - entry.prefix ++ middle ++ entry.suffix; - - entries = lib.mapAttrs (_: normaliseEnvEntry) env; - - rendered = lib.mapAttrsToList ( - name: entry: - if entry.unset then - { - needsJoin = false; - body = "unset ${name}"; - } - else - let - parts = effectiveParts name entry; - isTrivialLiteral = - lib.length parts == 1 - && builtins.isString (lib.head parts) - && entry.values == [ ] - && entry.prefix == [ ] - && entry.suffix == [ ]; - body = - if parts == [ ] then - null - else if isTrivialLiteral then - "export ${name}=${shellEscape (lib.head parts)}" - else - let - renderedParts = map renderPart parts; - sepArg = shellEscape entry.separator; - in - "export ${name}=\"$(_wrapper_env_join ${sepArg} ${lib.concatStringsSep " " renderedParts})\""; - wrapped = - if body == null then - null - else if entry.fallback then - '' - if [ -z "''${${name}+set}" ]; then - ${body} - fi'' - else - body; - in - { - needsJoin = !isTrivialLiteral && body != null; - body = wrapped; - } - ) entries; - - nonEmpty = lib.filter (r: r.body != null) rendered; - needsJoin = lib.any (r: r.needsJoin) nonEmpty; + { + join = !unset && !simple && wrapped != null; + text = wrapped; + }; - joinHelper = '' + helper = '' _wrapper_env_join() { - local _sep="$1" + local sep="$1" out="" p shift - local _out="" - local _p - for _p in "$@"; do - [ -n "$_p" ] || continue - _out="''${_out:+$_out$_sep}$_p" + for p in "$@"; do + [ -n "$p" ] || continue + out="''${out:+$out$sep}$p" done - printf '%s' "$_out" + printf '%s' "$out" }''; in - if nonEmpty == [ ] then - "" - else - lib.concatStringsSep "\n" ( - (lib.optional needsJoin joinHelper) ++ (map (r: r.body) nonEmpty) - ) - + "\n"; + { + ref = name: { + _type = "envRef"; + inherit name; + }; + + render = + raw: + let + lines = lib.filter (l: l.text != null) (lib.mapAttrsToList line raw); + needsJoin = lib.any (l: l.join) lines; + in + if lines == [ ] then + "" + else + lib.concatStringsSep "\n" ( + lib.optional needsJoin helper ++ map (l: l.text) lines + ) + + "\n"; + }; /** A collection of types for wrapper modules. @@ -666,7 +601,7 @@ let inherit (pkgs) lndir; # Generate environment variable exports - envString = wrapperLib.renderEnvString env; + envString = wrapperLib.env.render env; # Generate flag arguments with proper line breaks and indentation flagsString = @@ -875,9 +810,7 @@ let escapeShellArgWithEnv generateArgsFromFlags flagToArgs - envRef - renderEnvString - normaliseEnvEntry + env ; }; in diff --git a/lib/modules/env.nix b/lib/modules/env.nix index 6e2e11d..38f41d9 100644 --- a/lib/modules/env.nix +++ b/lib/modules/env.nix @@ -5,197 +5,101 @@ ... }: let - # A part of an env value list: either a literal string or a reference - # to another environment variable (produced by `wlib.envRef`). We - # use a custom type with a runtime check so we don't run into - # type-merging quirks between `lib.types.str` and submodules. - envPartType = lib.mkOptionType { - name = "envValuePart"; - description = "environment variable value part (string or wlib.envRef)"; + # A part of an `env..value` list: a literal string or a + # runtime reference produced by `wlib.env.ref`. + envPart = lib.mkOptionType { + name = "envPart"; + description = "string or wlib.env.ref"; check = - v: - builtins.isString v - || ( - builtins.isAttrs v - && v ? _type - && v._type == "envRef" - && v ? name - && builtins.isString v.name - ); + v: builtins.isString v || (builtins.isAttrs v && (v._type or null) == "envRef"); merge = lib.options.mergeEqualOption; }; - # A single env entry. The submodule is intentionally permissive: any - # combination of `value`/`values`/`prefix`/`suffix` is allowed, the - # renderer composes them in a defined order: - # - # prefix ++ (values OR [value] OR [envRef self]) ++ suffix - # - # Empty parts are filtered at runtime, so unset/empty env refs drop - # out without leaving stray separators. - envEntrySubmodule = lib.types.submodule { + valueType = lib.types.either lib.types.str (lib.types.listOf envPart); + + entry = lib.types.submodule { options = { value = lib.mkOption { - type = lib.types.nullOr ( - lib.types.coercedTo lib.types.path toString lib.types.str - ); + type = lib.types.nullOr valueType; default = null; description = '' - Literal string value for the variable. Setting a plain - string via `env.VAR = "..."` coerces to this field. Nix - paths are accepted and stringified automatically, matching - the behaviour of the old `attrsOf str` type. - ''; - }; - values = lib.mkOption { - type = lib.types.listOf envPartType; - default = [ ]; - description = '' - Explicit list of parts to join with `separator`. Parts may - be literal strings or env references produced by - `wlib.envRef "NAME"`. Empty parts are skipped at runtime, - so unset references don't leave dangling separators. - - If both `value` and `values` are set, `value` is spliced - into the middle of the resulting list (after `prefix` and - before `values`). This makes composition via `apply` - predictable: each module contributes more parts to the - list rather than fighting over a single string. - ''; - }; - prefix = lib.mkOption { - type = lib.types.listOf envPartType; - default = [ ]; - description = '' - Parts to prepend to the existing value of the variable. - Equivalent to `makeWrapper`'s `--prefix VAR SEP VAL`. The - existing value is implicitly referenced; if it is unset or - empty, it drops out cleanly. - ''; - }; - suffix = lib.mkOption { - type = lib.types.listOf envPartType; - default = [ ]; - description = '' - Parts to append to the existing value of the variable. - Equivalent to `makeWrapper`'s `--suffix VAR SEP VAL`. + What to set the variable to. Accepts either a single + string (literal), or a list of parts joined with + `separator`. List parts can be plain strings or + `wlib.env.ref "NAME"` runtime references; empty parts + (e.g. unset refs) are filtered at runtime so no dangling + separators are left behind. ''; }; separator = lib.mkOption { type = lib.types.str; default = ":"; - description = '' - Separator used when joining list-valued entries. Defaults - to `:`, which is the conventional separator for PATH-like - variables. - ''; + description = "Separator used when joining a list-valued `value`."; }; - fallback = lib.mkOption { + ifUnset = lib.mkOption { type = lib.types.bool; default = false; description = '' - Only set the variable when it is not already set in the - environment. Uses `$\{VAR+set}` semantics: an empty but - set variable is left alone. Useful for "default" values - like `EDITOR`, `PAGER`, etc. + Only apply this entry when the variable is unset (or + empty) in the caller's environment. Useful for defaults + like `EDITOR = "vim"`. ''; }; unset = lib.mkOption { type = lib.types.bool; default = false; - description = '' - Unset the variable instead of assigning it. Takes - precedence over all other options on this entry. - ''; + description = "Unset the variable (takes precedence over `value`)."; }; }; }; - # Type that users set `env` to. Plain strings, paths and `null` - # coerce into the submodule form so that existing - # `env.FOO = "bar"` keeps working (paths also accepted and - # stringified) and `env.FOO = null` is sugar for `{ unset = true; }`. - envValueType = lib.types.coercedTo ( + envValue = lib.types.coercedTo ( lib.types.nullOr (lib.types.either lib.types.str lib.types.path) - ) (v: if v == null then { unset = true; } else { value = toString v; }) envEntrySubmodule; + ) (v: if v == null then { unset = true; } else { value = toString v; }) entry; in { _file = "lib/modules/env.nix"; options.env = lib.mkOption { - type = lib.types.attrsOf envValueType; + type = lib.types.attrsOf envValue; default = { }; example = lib.literalExpression '' { - # Simple literal. - FOO = "bar"; - - # Prepend to PATH, keeping the user's existing entries. - PATH.prefix = [ "/opt/bin" ]; - - # Build an XDG_DATA_DIRS-like value with an explicit list. - XDG_DATA_DIRS.values = [ - "/opt/share" - (wlib.envRef "XDG_DATA_DIRS") - "/usr/local/share" - ]; - - # Only set EDITOR when the user hasn't already picked one. - EDITOR = { value = "vim"; fallback = true; }; - - # Explicitly unset a variable. - OLD_VAR = null; + FOO = "bar"; # literal + BLOAT = null; # unset + PATH.value = [ "/opt/bin" (wlib.env.ref "PATH") ]; # prepend + EDITOR = { value = "vim"; ifUnset = true; }; # default } ''; description = '' Environment variables to set in the wrapper. - Each entry accepts either a plain string (literal value), - `null` (unset the variable), or a structured attribute set with - the following fields: + Each entry accepts: + - a plain string: literal value + - `null`: unset the variable + - a submodule with `value`/`separator`/`ifUnset`/`unset` - - `value`: literal string value. - - `values`: list of parts joined with `separator`. Parts may - be plain strings or env references produced by - `wlib.envRef "NAME"`. - - `prefix` / `suffix`: parts to splice around the existing - value of the variable. Empty or unset references drop out - cleanly, so no dangling separators are left behind. - - `separator`: join separator (default `:`). - - `fallback`: if true, only set the variable when it is not - already present in the caller's environment. - - `unset`: if true, emit `unset VAR` instead of an assignment. + To prepend/append to an existing variable, pass `value` as a + list and include `wlib.env.ref "NAME"` at the point where the + existing value should appear: - The plain-string and `null` forms coerce into this submodule, - so existing `env.FOO = "bar"` usage keeps working unchanged. + env.PATH.value = [ "/opt/bin" (wlib.env.ref "PATH") ]; ''; }; - # Subset of `env` that resolves to a static literal string with no - # runtime composition. Used by the systemd integration, which cannot - # express bash-style prefix/suffix assignments natively. Entries - # using `prefix`, `suffix`, `values`, `fallback` or `unset` are - # omitted and must be set via `systemd.environment` directly if they - # need to reach the service file. + # Subset of `env` that resolves to a plain literal string. Used by + # the systemd integration, which cannot express runtime env + # composition. Complex entries are silently dropped — set them via + # `systemd.environment` directly if you need them. options.outputs.staticEnv = lib.mkOption { type = lib.types.attrsOf lib.types.str; internal = true; readOnly = true; - description = '' - Subset of `env` that resolves to a plain literal string. Used - by integrations like systemd that cannot express runtime env - composition. Complex entries are silently dropped; set them via - the integration's own environment option if you need them. - ''; - default = lib.mapAttrs (_: entry: entry.value) ( + description = "Plain literal `env` entries, for integrations like systemd."; + default = lib.mapAttrs (_: e: e.value) ( lib.filterAttrs ( - _: entry: - !entry.unset - && !entry.fallback - && entry.value != null - && entry.values == [ ] - && entry.prefix == [ ] - && entry.suffix == [ ] + _: e: + !e.unset && !e.ifUnset && e.value != null && !(builtins.isList e.value) ) config.env ); }; From 1f15cdc53dd4626a300812d82d19fc748a893069 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Apr 2026 12:07:57 +0000 Subject: [PATCH 3/4] lib.modules.env: drop unset option, point users at preHook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unset option turned out clunky: combining it with value/ifUnset had silent precedence, and overriding an inherited unset in a later apply required mkForce gymnastics. Unsetting a variable isn't a composable declarative concept, it's a one-line shell command — preHook is right there. Changes: - Remove `env..unset` from the submodule. - Remove the `null` top-level coercion. `env.FOO = null` is now a type error instead of silent unset. - Simplify the renderer: no more unset branch, no more precedence ordering between unset/ifUnset/value. The schema is now three options: value, separator, ifUnset. For unsetting, document `preHook = "unset LD_PRELOAD";` in the README. https://claude.ai/code/session_01UiEmmBrtkNEstoRemZpnp7 --- CHANGELOG.md | 13 ++++++------- README.md | 16 +++++++++------- checks/env-if-unset.nix | 16 +++++++--------- checks/env-render.nix | 14 -------------- lib/default.nix | 21 ++++++--------------- lib/modules/env.nix | 22 ++++++++-------------- 6 files changed, 36 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8952580..85cff65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,11 +16,12 @@ remove it (or change to `null`). - `env` option type changed from `attrsOf str` to a small submodule - with `value` / `separator` / `ifUnset` / `unset`. Plain strings - and `null` keep working via coercion (`env.FOO = "bar"` and - `env.FOO = null` are unchanged). The systemd integration reads - from `config.outputs.staticEnv` instead of `config.env` and drops - any entries it can't express as a literal assignment. + with `value` / `separator` / `ifUnset`. Plain strings keep working + via coercion (`env.FOO = "bar"` is unchanged). To unset a + variable, put `unset VAR` in `preHook` — the module system + doesn't model it as a declarative option. The systemd integration + reads from `config.outputs.staticEnv` instead of `config.env` and + drops any entries it can't express as a literal assignment. ### Added @@ -39,8 +40,6 @@ (default `:`). - `env..ifUnset = true`: only apply when the caller's environment doesn't already have the variable set. - - `env..unset = true`: emit `unset VAR` instead of an - assignment. `env.VAR = null` is sugar for this. - List `value`s merge by concatenation when composed via `apply`, so modules stack contributions without fighting over a string. - `wlib.env.ref NAME`: marker for a runtime env-variable reference diff --git a/README.md b/README.md index bb452eb..e2aafb3 100644 --- a/README.md +++ b/README.md @@ -93,8 +93,8 @@ wrappers.lib.wrapPackage { ### Environment Variables -Each `env.` entry is either a plain string, `null` (to unset), -or a small submodule: +Each `env.` entry is either a plain string or a small +submodule: ```nix { @@ -102,9 +102,6 @@ or a small submodule: # Literal: FOO = "bar"; - # Unset a variable inherited from the caller: - NOISY_OLD_VAR = null; - # Prepend to PATH using a list and `wlib.env.ref`. The ref # expands to the caller's existing PATH at runtime; if it's # unset or empty, the ref drops out and no stray separators @@ -114,6 +111,12 @@ or a small submodule: # Only set EDITOR if the caller hasn't already picked one. EDITOR = { value = "vim"; ifUnset = true; }; }; + + # To unset a variable inherited from the caller, drop a line in + # preHook — the env option is for declarative assignments only: + preHook = '' + unset LD_PRELOAD + ''; } ``` @@ -123,8 +126,7 @@ Submodule options: `wlib.env.ref "NAME"` runtime references. - `separator`: join separator for list values (default `:`). - `ifUnset`: only apply when the caller's environment doesn't - already have the variable set. -- `unset`: unset the variable (takes precedence over `value`). + already have the variable set (empty counts as unset). List `value`s merge by concatenation when composed via `apply`, so multiple modules stack contributions onto the same variable without diff --git a/checks/env-if-unset.nix b/checks/env-if-unset.nix index 74178a8..e5e1faf 100644 --- a/checks/env-if-unset.nix +++ b/checks/env-if-unset.nix @@ -3,13 +3,13 @@ self, }: -# `ifUnset` (only set if caller hasn't) and explicit unset. +# `ifUnset`: only apply when the caller's env doesn't have the +# variable set (or has it empty). let wlib = self.lib; showEnv = pkgs.writeShellScriptBin "show-env" '' printf 'EDITOR=%s\n' "''${EDITOR-}" - printf 'BLOAT=%s\n' "''${BLOAT-}" ''; wrapped = @@ -21,8 +21,6 @@ let value = "vim"; ifUnset = true; }; - # `null` sugar for unset. - config.env.BLOAT = null; } ).apply { pkgs = pkgs; }).wrapper; in @@ -30,7 +28,7 @@ pkgs.runCommand "env-if-unset-test" { } '' set -eu script="${wrapped}/bin/show-env" - # ifUnset applied when EDITOR is unset. + # ifUnset applies when EDITOR is unset. r=$(unset EDITOR && "$script" | grep '^EDITOR=' | cut -d= -f2-) [ "$r" = "vim" ] || { echo "FAIL: ifUnset unset: '$r'"; cat "$script"; exit 1; } echo "PASS: ifUnset applies when unset" @@ -40,10 +38,10 @@ pkgs.runCommand "env-if-unset-test" { } '' [ "$r" = "nano" ] || { echo "FAIL: ifUnset overrode existing: '$r'"; exit 1; } echo "PASS: ifUnset preserves existing" - # Explicit unset beats caller env. - r=$(BLOAT=garbage "$script" | grep '^BLOAT=' | cut -d= -f2-) - [ "$r" = "" ] || { echo "FAIL: unset: '$r'"; cat "$script"; exit 1; } - echo "PASS: unset overrides caller env" + # Empty counts as unset. + r=$(EDITOR="" "$script" | grep '^EDITOR=' | cut -d= -f2-) + [ "$r" = "vim" ] || { echo "FAIL: ifUnset empty: '$r'"; exit 1; } + echo "PASS: ifUnset treats empty as unset" touch $out '' diff --git a/checks/env-render.nix b/checks/env-render.nix index c07d1e1..d8cbeea 100644 --- a/checks/env-render.nix +++ b/checks/env-render.nix @@ -22,20 +22,6 @@ let export FOO="bar" ''; } - { - name = "null-unsets"; - input.FOO = null; - expected = '' - unset FOO - ''; - } - { - name = "explicit-unset"; - input.FOO.unset = true; - expected = '' - unset FOO - ''; - } { name = "if-unset"; input.EDITOR = { diff --git a/lib/default.nix b/lib/default.nix index cd50e30..0b6ad71 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -137,12 +137,12 @@ let `env.render` accepts the same shapes as the `env` module option: - `env.FOO = "bar"` literal - - `env.FOO = null` unset - `env.FOO.value = "bar"` literal (explicit) - `env.FOO.value = [ "a" "b" ]` list, joined with `separator` - `env.FOO.value = [ "/opt/bin" (env.ref "PATH") ]` prepend - `env.FOO.ifUnset = true` only set when caller hasn't - - `env.FOO.unset = true` unset (takes precedence) + + To unset a variable, put `unset VAR` in `preHook` instead. */ env = let @@ -151,12 +151,7 @@ let norm = e: - if e == null then - { unset = true; } - else if builtins.isString e || builtins.isList e then - { value = e; } - else - e; + if builtins.isString e || builtins.isList e then { value = e; } else e; renderPart = p: @@ -171,8 +166,6 @@ let name: raw: let e = norm raw; - unset = e.unset or false; - ifUnset = e.ifUnset or false; val = e.value or null; parts = if val == null then @@ -184,9 +177,7 @@ let simple = lib.length parts == 1 && builtins.isString (lib.head parts); sep = e.separator or ":"; body = - if unset then - "unset ${name}" - else if parts == [ ] then + if parts == [ ] then null else if simple then "export ${name}=${esc (lib.head parts)}" @@ -197,7 +188,7 @@ let wrapped = if body == null then null - else if ifUnset && !unset then + else if e.ifUnset or false then '' if [ -z "''${${name}:-}" ]; then ${body} @@ -206,7 +197,7 @@ let body; in { - join = !unset && !simple && wrapped != null; + join = !simple && wrapped != null; text = wrapped; }; diff --git a/lib/modules/env.nix b/lib/modules/env.nix index 38f41d9..dae349c 100644 --- a/lib/modules/env.nix +++ b/lib/modules/env.nix @@ -45,17 +45,12 @@ let like `EDITOR = "vim"`. ''; }; - unset = lib.mkOption { - type = lib.types.bool; - default = false; - description = "Unset the variable (takes precedence over `value`)."; - }; }; }; envValue = lib.types.coercedTo ( - lib.types.nullOr (lib.types.either lib.types.str lib.types.path) - ) (v: if v == null then { unset = true; } else { value = toString v; }) entry; + lib.types.either lib.types.str lib.types.path + ) (v: { value = toString v; }) entry; in { _file = "lib/modules/env.nix"; @@ -66,7 +61,6 @@ in example = lib.literalExpression '' { FOO = "bar"; # literal - BLOAT = null; # unset PATH.value = [ "/opt/bin" (wlib.env.ref "PATH") ]; # prepend EDITOR = { value = "vim"; ifUnset = true; }; # default } @@ -74,10 +68,11 @@ in description = '' Environment variables to set in the wrapper. - Each entry accepts: - - a plain string: literal value - - `null`: unset the variable - - a submodule with `value`/`separator`/`ifUnset`/`unset` + Each entry accepts a plain string (literal) or a submodule + with `value` / `separator` / `ifUnset`. Unset a variable by + putting `unset VAR` in `preHook` — the wrapper already runs + arbitrary shell there and that's the cleanest way to scrub + inherited env. To prepend/append to an existing variable, pass `value` as a list and include `wlib.env.ref "NAME"` at the point where the @@ -98,8 +93,7 @@ in description = "Plain literal `env` entries, for integrations like systemd."; default = lib.mapAttrs (_: e: e.value) ( lib.filterAttrs ( - _: e: - !e.unset && !e.ifUnset && e.value != null && !(builtins.isList e.value) + _: e: !e.ifUnset && e.value != null && !(builtins.isList e.value) ) config.env ); }; From c6731fbbeb69d0ed1caca6dfaff87a6a2960950d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 8 Apr 2026 12:47:06 +0000 Subject: [PATCH 4/4] lib.modules.env: canonicalize value to always-list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Polymorphic `value` (string OR list) meant consumers inspecting another wrapper's env had to safeguard against both types. Now `value` is internally always a list and plain strings coerce to singletons at write time — writing stays ergonomic, reading is uniform. env.EDITOR = "vim"; # coerces to [ "vim" ] a.env.EDITOR.value # => [ "vim" ] (always a list) # Literal-read pattern for consumers: lib.concatStringsSep a.env.EDITOR.separator a.env.EDITOR.value outputs.staticEnv now joins literal-only list values with `separator` instead of rejecting them outright, so `env.FOO = "bar"` still reaches systemd as a plain literal. New checks/env-read.nix exercises the round-trip invariant. https://claude.ai/code/session_01UiEmmBrtkNEstoRemZpnp7 --- CHANGELOG.md | 15 ++++---- README.md | 12 +++++-- checks/env-read.nix | 87 +++++++++++++++++++++++++++++++++++++++++++++ lib/modules/env.nix | 27 +++++++++----- 4 files changed, 123 insertions(+), 18 deletions(-) create mode 100644 checks/env-read.nix diff --git a/CHANGELOG.md b/CHANGELOG.md index 85cff65..8e1cbbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,16 +32,19 @@ `config.flags` returns clean values (order is transparent). - `lib/modules/env.nix`: env module with per-variable options for safe composition through the NixOS module system. - - `env..value`: string for a simple literal, or a list of - parts to join with `separator`. List parts can be plain strings - or `wlib.env.ref "NAME"` runtime references. Empty/unset refs - drop out cleanly, so no dangling separators. - - `env..separator`: join separator for a list `value` - (default `:`). + - `env..value`: always a list of parts joined with + `separator`. A plain string coerces to a singleton list, so + `env.FOO = "bar"` works, but reading back always gives a list. + Parts can be plain strings or `wlib.env.ref "NAME"` runtime + references. Empty/unset refs drop out cleanly, so no dangling + separators. + - `env..separator`: join separator for `value` (default `:`). - `env..ifUnset = true`: only apply when the caller's environment doesn't already have the variable set. - List `value`s merge by concatenation when composed via `apply`, so modules stack contributions without fighting over a string. + - To read another wrapper's literal entry as a string, use + `lib.concatStringsSep entry.separator entry.value`. - `wlib.env.ref NAME`: marker for a runtime env-variable reference inside `env..value` lists. - `wlib.env.render`: render an `env` attrset into a shell snippet. diff --git a/README.md b/README.md index e2aafb3..6f65bf4 100644 --- a/README.md +++ b/README.md @@ -121,13 +121,19 @@ submodule: ``` Submodule options: -- `value`: literal string *or* a list of parts joined with - `separator`. List parts can be plain strings or +- `value`: a list of parts joined with `separator`. Plain strings + coerce to singleton lists, so `env.FOO = "bar"` works, but + reading back always gives a list. Parts can be plain strings or `wlib.env.ref "NAME"` runtime references. -- `separator`: join separator for list values (default `:`). +- `separator`: join separator (default `:`). - `ifUnset`: only apply when the caller's environment doesn't already have the variable set (empty counts as unset). +`value` is always a list when read back, so consumers inspecting +another wrapper's config don't need to safeguard against two +types. To read a literal entry as a joined string, use +`lib.concatStringsSep entry.separator entry.value`. + List `value`s merge by concatenation when composed via `apply`, so multiple modules stack contributions onto the same variable without fighting over a single string. diff --git a/checks/env-read.nix b/checks/env-read.nix new file mode 100644 index 0000000..df0d8e1 --- /dev/null +++ b/checks/env-read.nix @@ -0,0 +1,87 @@ +{ + pkgs, + self, +}: + +# Verify that `env..value` is always a list when read from +# another wrapper's config, regardless of whether the user wrote +# the input as a string or a list. This is the invariant that lets +# consumers inspect env entries without safeguarding against two +# types. +let + lib = pkgs.lib; + wlib = self.lib; + + producer = wlib.wrapModule ( + { config, ... }: + { + config.package = pkgs.hello; + # Plain string input. + config.env.EDITOR = "vim"; + # Already-a-list input. + config.env.PATH.value = [ + "/opt/bin" + (wlib.env.ref "PATH") + ]; + } + ); + + applied = producer.apply { inherit pkgs; }; + + editorValue = applied.env.EDITOR.value; + pathValue = applied.env.PATH.value; + + # Literal read helper: joins the parts with separator. Works as + # long as all parts are plain strings (no envRefs). + readLiteral = e: lib.concatStringsSep e.separator e.value; + + editorString = readLiteral applied.env.EDITOR; + + checks = [ + ( + if builtins.isList editorValue then + "PASS: string input is a list when read back" + else + throw "FAIL: editor value is ${builtins.typeOf editorValue}, expected list" + ) + ( + if editorValue == [ "vim" ] then + "PASS: string coerced to singleton [ \"vim\" ]" + else + throw "FAIL: editor value = ${builtins.toJSON editorValue}" + ) + ( + if editorString == "vim" then + "PASS: readLiteral round-trips the string" + else + throw "FAIL: readLiteral gave ${editorString}" + ) + ( + if builtins.isList pathValue && lib.length pathValue == 2 then + "PASS: list input preserved as list" + else + throw "FAIL: path value = ${builtins.toJSON pathValue}" + ) + ( + if builtins.isString (lib.head pathValue) then + "PASS: string parts stay strings" + else + throw "FAIL: path first part = ${builtins.toJSON (lib.head pathValue)}" + ) + ( + let + second = lib.elemAt pathValue 1; + in + if builtins.isAttrs second && second._type or null == "envRef" && second.name == "PATH" then + "PASS: envRef round-trips" + else + throw "FAIL: path second part = ${builtins.toJSON second}" + ) + ]; +in +pkgs.runCommand "env-read-test" { } '' + cat <<'EOF' + ${lib.concatStringsSep "\n" checks} + EOF + touch $out +'' diff --git a/lib/modules/env.nix b/lib/modules/env.nix index dae349c..2ad13d2 100644 --- a/lib/modules/env.nix +++ b/lib/modules/env.nix @@ -15,7 +15,7 @@ let merge = lib.options.mergeEqualOption; }; - valueType = lib.types.either lib.types.str (lib.types.listOf envPart); + valueType = lib.types.coercedTo lib.types.str (v: [ v ]) (lib.types.listOf envPart); entry = lib.types.submodule { options = { @@ -23,12 +23,17 @@ let type = lib.types.nullOr valueType; default = null; description = '' - What to set the variable to. Accepts either a single - string (literal), or a list of parts joined with - `separator`. List parts can be plain strings or - `wlib.env.ref "NAME"` runtime references; empty parts - (e.g. unset refs) are filtered at runtime so no dangling - separators are left behind. + Parts to join with `separator`. Accepts a plain string + (coerced to a singleton list) or a list of parts. List + parts can be plain strings or `wlib.env.ref "NAME"` + runtime references; empty parts are filtered at runtime + so unset refs don't leave dangling separators. + + This option is always a list when read back. To read a + known-literal entry as a string from another wrapper's + config, use `lib.head entry.value` (for a singleton) or + `lib.concatStringsSep entry.separator entry.value` (for + multiple literal parts). ''; }; separator = lib.mkOption { @@ -91,9 +96,13 @@ in internal = true; readOnly = true; description = "Plain literal `env` entries, for integrations like systemd."; - default = lib.mapAttrs (_: e: e.value) ( + default = lib.mapAttrs (_: e: lib.concatStringsSep e.separator e.value) ( lib.filterAttrs ( - _: e: !e.ifUnset && e.value != null && !(builtins.isList e.value) + _: e: + !e.ifUnset + && e.value != null + && e.value != [ ] + && lib.all builtins.isString e.value ) config.env ); };