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

Previous Next

Full log

Message #14 received at 64359 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: "Zack Weinberg" <zack <at> owlfolio.org>
Cc: 64359 <at> debbugs.gnu.org
Subject: Re: [bug#64359] [PATCH] [RFC] --search-paths: emit code compatible
 with set -u
Date: Sat, 14 Oct 2023 19:04:17 +0200

Hi Zack,

"Zack Weinberg" <zack <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.