[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set
From: |
Ludovic Courtès |
Subject: |
[bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u |
Date: |
Wed, 11 Oct 2023 19:06:32 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
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’.
- [bug#64359] [PATCH] [RFC] --search-paths: emit code compatible with set -u,
Ludovic Courtès <=