guix-patches
[Top][All Lists]
Advanced

[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’.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]