[bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u

["Ludovic Courtès" <ludo@gnu.org>]

Hi Zack,

Zack Weinberg <zack@owlfolio.org> skribis:

> The shell script fragment emitted by the --search-paths option to
> ‘guix shell’, etc., uses this construct to prepend a directory to
> a search-path environment variable:
>
>     export VAR="/gnu/store/...${VAR:+:}$VAR"
>
> If this is evaluated by a shell in set -u mode, and VAR was previously
> unset, it will error out:
>
>     $ bash -c '
>         set -u
>         unset PKG_CONFIG_PATH
>         export PKG_CONFIG_PATH="/example/lib/pkgconfig${PKG_CONFIG_PATH:+:}$PKG_CONFIG_PATH"
>         echo $PKG_CONFIG_PATH
>     '
>     bash: line 4: PKG_CONFIG_PATH: unbound variable
>
> This happens in real life, for instance, if direnv[1] is being used in its
> “strict_env” mode[2], which the documentation says will become the default
> in a future release.
>
> This patch makes the code emitted by --search-paths compatible with set -u,
> by changing each use of bare `$VARIABLE` to `${VARIABLE:-}`, e.g.
>
>     $ bash -c '
>         set -u
>         unset PKG_CONFIG_PATH
>         export PKG_CONFIG_PATH="/example/lib/pkgconfig${PKG_CONFIG_PATH:+:}${PKG_CONFIG_PATH:-}"
>         echo $PKG_CONFIG_PATH'
>     /example/lib/pkgconfig

This change makes a lot of sense to me.


[...]

> +++ b/guix/build/utils.scm
> @@ -1384,19 +1384,19 @@ (define (export-variable lst)
>         (format #f "export ~a=\"~a\""
>                 var (string-join rest sep)))
>        ((var sep 'prefix rest)
> -       (format #f "export ~a=\"~a${~a:+~a}$~a\""
> +       (format #f "export ~a=\"~a${~a:+~a}${~a:-}\""
>                 var (string-join rest sep) var sep var))

This part is a full-rebuild change, so it’d have to wait.  However, it’s
within ‘wrap-program’; the script generated by ‘wrap-program’ does *not*
use ‘set -u’, so I think this change is unnecessary.  Am I right?

> +++ b/guix/search-paths.scm
> @@ -225,10 +225,10 @@ (define* (environment-variable-definition variable value
>      ('exact
>       (format #f "export ~a=\"~a\"" variable value))
>      ('prefix
> -     (format #f "export ~a=\"~a${~a:+~a}$~a\""
> +     (format #f "export ~a=\"~a${~a:+~a}${~a:-}\""
>               variable value variable separator variable))
>      ('suffix
> -     (format #f "export ~a=\"$~a${~a:+~a}~a\""
> +     (format #f "export ~a=\"${~a:-}${~a:+~a}~a\""
>               variable variable variable separator value))))

LGTM.

> --- a/tests/guix-environment.sh
> +++ b/tests/guix-environment.sh

You can remove this change and keep only the ‘tests/guix-shell.sh’ part.

Could you send an updated patch?

Thanks, and apologies for the long delay!

Ludo’.