Subject: Re: bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.

From: Mark H Weaver <[email-masked]>
Date: Tue, 15 Sep 2020 16:39:25 -0400

Efraim Flashner <[email-masked]> writes:

> On Mon, Sep 14, 2020 at 09:25:25PM -0700, Mike Rosset wrote:
>> * gnu/packages/audio.scm (jack-2): Update to 1.9.14.
>> [arguments]: new 'declare-for-int phase after unpack that declares 'i in the
>> for initialize statement.  Add -lstdc++ to LDFLAGS 'set-linkflags phase
>> ensures -lstdc++ is at the tail.
[...]
>> @@ -2047,8 +2047,18 @@ synchronous execution of all clients, and low latency operation.")
>>                             "--alsa")
>>         #:phases
>>         (modify-phases %standard-phases
>> +         (add-after 'unpack 'declare-for-int
>> +           (lambda _
>> +             ;; Declare the for loop i incrementer.
>> +             (substitute* "dbus/sigsegv.c"
>> +               (("for\\(i = 0") "for(int i = 0"))
>> +             #t))
>
> Any chance of an upstream bug number or something for this? It seems
> like the type of thing that might be put into a snippet.

I agree that somehow this fix should be in the 'origin', so that this
fix will be in the output of "guix build --source".  However, I'd go
further and suggest that it should be a patch instead of a call to
'substitute*'.

Although patches are larger and a bit more work to create, they are far
more robust.  When this bug is eventually fixed upstream, a patch to fix
it will begin raising an error, alerting us that it's time to remove it.

In contrast, a call to 'substitute*' will silently start doing nothing,
and may easily be forgotten.  To make matters worse, a future version of
jack-2 might add another 'for' loop in that file, matching the same
pattern but where it is important that 'i' _not_ be initialized to 0.
This 'substitute*' call, likely vestigial by that time but long since
forgotten, could start silently introducing a new bug.

What do you think?

      Thanks,
        Mark