[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: |
Sat, 14 Oct 2023 19:04:17 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Hi Zack,
"Zack Weinberg" <zack@owlfolio.org> skribis:
>>> +++ 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?
>
> It's not strictly necessary to fix the bug, no. I made this change because
> it's the only other appearance of 'export VAR="additional-value${VAR:+:}$VAR"'
> in the guix source code and I thought it would be better to change both of
> them the same way. If only so that years from now someone doesn't waste any
> time wondering why they're not quite the same and whether it matters.
>
> Why is it a full-rebuild change? As you point out, it should not actually
> change anything?
It’s a full-rebuild change because every single package depends on (guix
build utils). When we change it, we have to rebuild literally every
package.
>>> --- a/tests/guix-environment.sh
>>> +++ b/tests/guix-environment.sh
>> You can remove this change and keep only the ‘tests/guix-shell.sh’ part.
>
> I know "guix environment" is obsolete, but isn't it appropriate to test it
> thoroughly for as long as it still exists? (and again, years from now someone
> might waste time wondering why this is only tested for "guix shell")
No, I think it’s unnecessary because the two share the same code.
Eventually we’ll merge the two tests (and remove ‘guix environment’,
someday).
Could you send an updated patch?
Thanks,
Ludo’.