jack2 fails to build on aarch64

  • Done
  • quality assurance status badge
Details
5 participants
  • Andreas Enge
  • Efraim Flashner
  • Vitaliy Shatrov
  • Mark H Weaver
  • Mike Rosset
Owner
unassigned
Submitted by
Vitaliy Shatrov
Severity
normal

Debbugs page

Vitaliy Shatrov wrote 5 years ago
(name . Д-д-добрые Люди)(address . bug-guix@gnu.org)(name . Mike Rosset)(address . mike.rosset@gmail.com)
87363v7e7j.fsf@disroot.org
Hello Guix.

gst-plugins-good needs jack2. Sites like YouTube need gst-plugins-good.
Some people think they need the video in Web-browsers (lol, mpv rules).

guix 81ea278 is still has no jack2, it's all i know about.

Please fix jack2, someone.
Thankful for Your attention, Vitaliy.
Andreas Enge wrote 5 years ago
(name . Vitaliy Shatrov)(address . guix.vits@disroot.org)
20200906085805.GA2494@jurong
The error message is:
[228/274] Compiling example-clients/simdtests.cpp
...
[243/274] Linking build/example-clients/jack_simdtests
...
ld: example-clients/simdtests.cpp.28.o: undefined reference to symbol '__gxx_personality_v0@@CXXABI_1.3'
ld: /gnu/store/9xmlrmk7vnlzwq5049500r9lrcf6ikqq-gcc-7.5.0-lib/lib/libstdc++.so.6: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
...
Waf: Leaving directory `/tmp/guix-build-jack2-1.9.13.drv-0/jack2-1.9.13/build'
Build failed
-> task in 'jack_simdtests' failed with exit status 1 (run with -v to display more information)
command "python" "waf" "build" failed with status 1

Following the advice and running "python waf build -v", the executed gcc
command is printed:
Build failed
-> task in 'jack_simdtests' failed with exit status 1:
{task 281472893294736: cprogram simdtests.cpp.28.o -> jack_simdtests}
['/gnu/store/ap7hgyv4rjqmhg4a6cb6cypsh3g1f5q4-gcc-7.5.0/bin/gcc', '-Wl,-rpath=/gnu/store/bwmp037cg6gfz31j9q441j800mvk7wva-jack2-1.9.13/lib', 'example-clients/simdtests.cpp.28.o', '-o/tmp/guix-build-jack2-1.9.13.drv-0/jack2-1.9.13/build/example-clients/jack_simdtests', '-Wl,-Bstatic', '-Wl,-Bdynamic', '-Lcommon', '-L/gnu/store/zwrg5jwyj1222vyjdpmppijzhz04sqid-opus-1.3.1/lib', '-L/gnu/store/mpm43myhw7difa8p3a9xnag3zb6pnn4b-dbus-1.12.16/lib', '-ljack', '-lpthread', '-lopus', '-ldb', '-lrt', '-ldl', '-ldbus-1', '-lm']

Andreas
Andreas Enge wrote 5 years ago
(name . Vitaliy Shatrov)(address . guix.vits@disroot.org)
20200906100241.GA4311@jurong
Updating jack2 to 1.9.14 does not solve the problem.

Andreas
Mike Rosset wrote 5 years ago
(name . Andreas Enge)(address . andreas@enge.fr)
87h7sbnhl6.fsf@gmail.com
Andreas Enge <andreas@enge.fr> writes:

Toggle quote (23 lines)
> The error message is:
> [228/274] Compiling example-clients/simdtests.cpp
> ...
> [243/274] Linking build/example-clients/jack_simdtests
> ...
> ld: example-clients/simdtests.cpp.28.o: undefined reference to symbol '__gxx_personality_v0@@CXXABI_1.3'
> ld: /gnu/store/9xmlrmk7vnlzwq5049500r9lrcf6ikqq-gcc-7.5.0-lib/lib/libstdc++.so.6: error adding symbols: DSO missing from command line
> collect2: error: ld returned 1 exit status
> ...
> Waf: Leaving directory `/tmp/guix-build-jack2-1.9.13.drv-0/jack2-1.9.13/build'
> Build failed
> -> task in 'jack_simdtests' failed with exit status 1 (run with -v to display more information)
> command "python" "waf" "build" failed with status 1
>
> Following the advice and running "python waf build -v", the executed gcc
> command is printed:
> Build failed
> -> task in 'jack_simdtests' failed with exit status 1:
> {task 281472893294736: cprogram simdtests.cpp.28.o -> jack_simdtests}
> ['/gnu/store/ap7hgyv4rjqmhg4a6cb6cypsh3g1f5q4-gcc-7.5.0/bin/gcc', '-Wl,-rpath=/gnu/store/bwmp037cg6gfz31j9q441j800mvk7wva-jack2-1.9.13/lib', 'example-clients/simdtests.cpp.28.o', '-o/tmp/guix-build-jack2-1.9.13.drv-0/jack2-1.9.13/build/example-clients/jack_simdtests', '-Wl,-Bstatic', '-Wl,-Bdynamic', '-Lcommon', '-L/gnu/store/zwrg5jwyj1222vyjdpmppijzhz04sqid-opus-1.3.1/lib', '-L/gnu/store/mpm43myhw7difa8p3a9xnag3zb6pnn4b-dbus-1.12.16/lib', '-ljack', '-lpthread', '-lopus', '-ldb', '-lrt', '-ldl', '-ldbus-1', '-lm']
>
> Andreas

undefined reference to symbol '__gxx_personality_v0@@CXXABI_1.3 looks
like a potentially gcc flag order issues. Seems -lstdc++ should be at the
end of the g++ command invocation. I'll see if I'll have time today to look into this more.


Mike
Mike Rosset wrote 4 years ago
[PATCH] gnu: jack-2: Update to 1.9.14.
(address . 43232@debbugs.gnu.org)(name . Mike Rosset)(address . mike.rosset@gmail.com)
20200915042525.4186311-1-mike.rosset@gmail.com
* 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.

This fixes issues that cause jack-2 to not build on system aarh64-linux.
---
gnu/packages/audio.scm | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

Toggle diff (43 lines)
diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm
index 38ee4f8bcc..83c08b718e 100644
--- a/gnu/packages/audio.scm
+++ b/gnu/packages/audio.scm
@@ -2030,7 +2030,7 @@ synchronous execution of all clients, and low latency operation.")
(define-public jack-2
(package (inherit jack-1)
(name "jack2")
- (version "1.9.13")
+ (version "1.9.14")
(source (origin
(method url-fetch)
(uri (string-append "https://github.com/jackaudio/jack2/releases/"
@@ -2039,7 +2039,7 @@ synchronous execution of all clients, and low latency operation.")
(file-name (string-append name "-" version ".tar.gz"))
(sha256
(base32
- "1d1d403jn4366mqig6g8ghr8057b3rn7gs26b5p3rkal34j20qw2"))))
+ "0z11hf55a6mi8h50hfz5wry9pshlwl4mzfwgslghdh40cwv342m2"))))
(build-system waf-build-system)
(arguments
`(#:tests? #f ; no check target
@@ -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))
(add-before 'configure 'set-linkflags
(lambda _
+ ;; Ensure -lstdc++ is the tail of LDFLAGS or the simdtests.cpp
+ ;; will not link with undefined reference to symbol
+ ;; '__gxx_personality_v0@@CXXABI_1.3'
+ (setenv "LDFLAGS" "-lstdc++")
;; Add $libdir to the RUNPATH of all the binaries.
(substitute* "wscript"
((".*CFLAGS.*-Wall.*" m)
--
2.28.0
Efraim Flashner wrote 4 years ago
(name . Mike Rosset)(address . mike.rosset@gmail.com)(address . 43232@debbugs.gnu.org)
20200915062139.GF17272@E5400
On Mon, Sep 14, 2020 at 09:25:25PM -0700, Mike Rosset wrote:
Toggle quote (43 lines)
> * 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.
>
> This fixes issues that cause jack-2 to not build on system aarh64-linux.
> ---
> gnu/packages/audio.scm | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm
> index 38ee4f8bcc..83c08b718e 100644
> --- a/gnu/packages/audio.scm
> +++ b/gnu/packages/audio.scm
> @@ -2030,7 +2030,7 @@ synchronous execution of all clients, and low latency operation.")
> (define-public jack-2
> (package (inherit jack-1)
> (name "jack2")
> - (version "1.9.13")
> + (version "1.9.14")
> (source (origin
> (method url-fetch)
> (uri (string-append "https://github.com/jackaudio/jack2/releases/"
> @@ -2039,7 +2039,7 @@ synchronous execution of all clients, and low latency operation.")
> (file-name (string-append name "-" version ".tar.gz"))
> (sha256
> (base32
> - "1d1d403jn4366mqig6g8ghr8057b3rn7gs26b5p3rkal34j20qw2"))))
> + "0z11hf55a6mi8h50hfz5wry9pshlwl4mzfwgslghdh40cwv342m2"))))
> (build-system waf-build-system)
> (arguments
> `(#:tests? #f ; no check target
> @@ -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.

Toggle quote (16 lines)
> (add-before 'configure 'set-linkflags
> (lambda _
> + ;; Ensure -lstdc++ is the tail of LDFLAGS or the simdtests.cpp
> + ;; will not link with undefined reference to symbol
> + ;; '__gxx_personality_v0@@CXXABI_1.3'
> + (setenv "LDFLAGS" "-lstdc++")
> ;; Add $libdir to the RUNPATH of all the binaries.
> (substitute* "wscript"
> ((".*CFLAGS.*-Wall.*" m)
> --
> 2.28.0
>
>
>
>

--
Efraim Flashner <efraim@flashner.co.il> אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAl9gXXAACgkQQarn3Mo9
g1EocBAAvHedkb2yUnqp4FKFzsElwMVdkg4aylCXjv40FDpM+q+lkjD6fQYH6QnM
BNctLLQQS5gxoo/bFZy/cYSR+mAl0eQtxoO2CeNUq1rDHgZh/3GxCGF5xjMV1OX+
Rbfj6fEdA/PW2BmpmJhokLMl41I0c7kHFeoEeMyMSNenAS2He66Nonmms0w9yWKP
/4Ow/rJ9MhBGhjL0sYAVbP0sNsM4sARrvedGdCEXf//5BuV4HwkoXUsjZvcYKydN
2qdDTTxsldewKg0MRV80ItjD5w+V3lEHr8lK75KZ2IUZvZ9ntyYNMxaouPfhbHIB
rNZ2jC6lVevgl97DJuh9FPEi4CjB2oH6bRnptRiPRwi1Aox69+pMf6AQLj6Fq4ax
kExAOcMOnL72Zsi9HQFNrDN68Pcz+JpWUUhKGo3hbDVPK2rSf3SxOG8OC1t1uxzQ
jFheJzXq8I4/9t+XQzFhx9KmMbG3sD5g4+6m1++Yf2Nh2ln503M6/1GzPGTt50Pp
SlgdFu4u3qOknT8SSW6HoVPROl9umosxcHuerx6lc4/i/X1moYUPyUHNs+gi+Ud+
WOgJ0juBnMY3RlyYk91aPj+icxDXt4fG+1z4y086L8xf5aXRfPL/t5/TRySe85iJ
3vxU2dadgpiRtA/m+0omBiW0J/qNIg4iGxrCCuFONYG0yadWNi4=
=eSJI
-----END PGP SIGNATURE-----


Mike Rosset wrote 4 years ago
(name . Efraim Flashner)(address . efraim@flashner.co.il)(address . 43232@debbugs.gnu.org)
87k0wvxztk.fsf@gmail.com
Efraim Flashner <efraim@flashner.co.il> writes:

Toggle quote (13 lines)
> On Mon, Sep 14, 2020 at 09:25:25PM -0700, Mike Rosset wrote:
>> (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.
>

That's a good idea, in retrospect Andreas did not report having this
issue. Let me do a second in series that removes this hunk.

The second hunk resolves the undefined reference to symbol
'__gxx_personality_v0@@CXXABI_1.3 I think that change is a safe one..

Then I can do some follow up the for loop initialization
errors. Hopefully it's just not me effected by that error. I'll see
about making this a snippet as well.

Mike
Mike Rosset wrote 4 years ago
[PATCH 2/2] gnu: jack2: 'declare-for-int phase no longer required.
(address . 43232@debbugs.gnu.org)(name . Mike Rosset)(address . mike.rosset@gmail.com)
20200915074802.267832-1-mike.rosset@gmail.com
* gnu/packages/audio.scm (jack2): remove 'declare-for-int phase.
[arguements]: phases 'declare-for-int has been removed. since package now
builds.
---
gnu/packages/audio.scm | 6 ------
1 file changed, 6 deletions(-)

Toggle diff (19 lines)
diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm
index 83c08b718e..fb98777290 100644
--- a/gnu/packages/audio.scm
+++ b/gnu/packages/audio.scm
@@ -2047,12 +2047,6 @@ 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))
(add-before 'configure 'set-linkflags
(lambda _
;; Ensure -lstdc++ is the tail of LDFLAGS or the simdtests.cpp
--
2.28.0
Andreas Enge wrote 4 years ago
(name . Mike Rosset)(address . mike.rosset@gmail.com)(address . 43232@debbugs.gnu.org)
20200915080232.GA10884@jurong
Hello Mike,

On Tue, Sep 15, 2020 at 12:48:02AM -0700, Mike Rosset wrote:
Toggle quote (4 lines)
> * gnu/packages/audio.scm (jack2): remove 'declare-for-int phase.
> [arguements]: phases 'declare-for-int has been removed. since package now
> builds.

I am getting a bit lost; it looks like this patch has to be applied on top
of your previous one. But since the previous one is not in master yet, there
will be no point in pushing one patch, then another one that partially
reverts the first one.

Could you please just post a new patch with the one modification you propose,
to be applied on master?

Thanks,

Andreas
Mike Rosset wrote 4 years ago
[PATCH] gnu: jack-2: Update to 1.9.14.
(address . 43232@debbugs.gnu.org)(name . Mike Rosset)(address . mike.rosset@gmail.com)
20200915083323.277828-1-mike.rosset@gmail.com
* gnu/packages/audio.scm (jack-2): Update to 1.9.14.
[arguments]: 'set-linkflags phase now adds -lstdc++ to the LDFLAGS
environment variable.

This fixes an issue that where the linker would fail with "undefined reference
to symbol '__gxx_personality_v0@@CXXABI_1.3" on aarch64-linux system.
---
gnu/packages/audio.scm | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

Toggle diff (35 lines)
diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm
index 38ee4f8bcc..fb98777290 100644
--- a/gnu/packages/audio.scm
+++ b/gnu/packages/audio.scm
@@ -2030,7 +2030,7 @@ synchronous execution of all clients, and low latency operation.")
(define-public jack-2
(package (inherit jack-1)
(name "jack2")
- (version "1.9.13")
+ (version "1.9.14")
(source (origin
(method url-fetch)
(uri (string-append "https://github.com/jackaudio/jack2/releases/"
@@ -2039,7 +2039,7 @@ synchronous execution of all clients, and low latency operation.")
(file-name (string-append name "-" version ".tar.gz"))
(sha256
(base32
- "1d1d403jn4366mqig6g8ghr8057b3rn7gs26b5p3rkal34j20qw2"))))
+ "0z11hf55a6mi8h50hfz5wry9pshlwl4mzfwgslghdh40cwv342m2"))))
(build-system waf-build-system)
(arguments
`(#:tests? #f ; no check target
@@ -2049,6 +2049,10 @@ synchronous execution of all clients, and low latency operation.")
(modify-phases %standard-phases
(add-before 'configure 'set-linkflags
(lambda _
+ ;; Ensure -lstdc++ is the tail of LDFLAGS or the simdtests.cpp
+ ;; will not link with undefined reference to symbol
+ ;; '__gxx_personality_v0@@CXXABI_1.3'
+ (setenv "LDFLAGS" "-lstdc++")
;; Add $libdir to the RUNPATH of all the binaries.
(substitute* "wscript"
((".*CFLAGS.*-Wall.*" m)
--
2.28.0
Mike Rosset wrote 4 years ago
Re: bug#43232: [PATCH 2/2] gnu: jack2: 'declare-for-int phase no longer required.
(name . Andreas Enge)(address . andreas@enge.fr)(address . 43232@debbugs.gnu.org)
87h7rzxwpa.fsf@gmail.com
Andreas Enge <andreas@enge.fr> writes:

Toggle quote (19 lines)
> Hello Mike,
>
> On Tue, Sep 15, 2020 at 12:48:02AM -0700, Mike Rosset wrote:
>> * gnu/packages/audio.scm (jack2): remove 'declare-for-int phase.
>> [arguements]: phases 'declare-for-int has been removed. since package now
>> builds.
>
> I am getting a bit lost; it looks like this patch has to be applied on top
> of your previous one. But since the previous one is not in master yet, there
> will be no point in pushing one patch, then another one that partially
> reverts the first one.
>
> Could you please just post a new patch with the one modification you propose,
> to be applied on master?
>
> Thanks,
>
> Andreas

Apologies, I just wanted to keep a series for posterity which normally
I'm terrible at. I should have mentioned you could just squash them.

I emailed a new first in series that squashed and improved the commit
message.

Hope that helps.

Mike
Andreas Enge wrote 4 years ago
(name . Mike Rosset)(address . mike.rosset@gmail.com)
20200915184824.GA3613@jurong
On Tue, Sep 15, 2020 at 01:37:05AM -0700, Mike Rosset wrote:
Toggle quote (3 lines)
> Apologies, I just wanted to keep a series for posterity which normally
> I'm terrible at. I should have mentioned you could just squash them.

No problem, it is just less work for me to apply just one patch :)

I pushed the commit, thanks a lot! The new jack2 now also compiles on
aarch64, so the original problem is solved.

Andreas
Mike Rosset wrote 4 years ago
(name . Andreas Enge)(address . andreas@enge.fr)
87een2yhoo.fsf@gmail.com
Andreas Enge <andreas@enge.fr> writes:

Toggle quote (6 lines)
> On Tue, Sep 15, 2020 at 01:37:05AM -0700, Mike Rosset wrote:
>> Apologies, I just wanted to keep a series for posterity which normally
>> I'm terrible at. I should have mentioned you could just squash them.
>
> No problem, it is just less work for me to apply just one patch :)

Totally understandable. I'll remember this in the future.

Toggle quote (5 lines)
> I pushed the commit, thanks a lot! The new jack2 now also compiles on
> aarch64, so the original problem is solved.
>
> Andreas

Thanks for reviewing this, much appreciated.

Mike
Mark H Weaver wrote 4 years ago
Re: bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
(address . 43232@debbugs.gnu.org)
87ft7ibwpz.fsf@netris.org
Efraim Flashner <efraim@flashner.co.il> writes:

Toggle quote (5 lines)
> 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.
[...]
Toggle quote (14 lines)
>> @@ -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
Mark H Weaver wrote 4 years ago
(address . 43232@debbugs.gnu.org)
87d02mbwb7.fsf@netris.org
Earlier, I wrote:
Toggle quote (5 lines)
> 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.

Sorry, I made a mistake in the details here, since the pattern applies
only when 'i' is already initialized to 0, but the more general point
still stands, namely that patches are more robust than 'substitute*' for
fixing bugs, and less likely to be misapplied or left forgotten in a
vestigial state after they are no longer needed.

Mark
Mike Rosset wrote 4 years ago
(name . Mark H Weaver)(address . mhw@netris.org)
87bli6ycih.fsf@gmail.com
Mark H Weaver <mhw@netris.org> writes:

Toggle quote (43 lines)
> Efraim Flashner <efraim@flashner.co.il> 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

Hello Mark,

Turns out version 1.9.14 is not effected by this. So the version bump
is good enough. I resubmitted a first in series with just the version
bump and linker change. Which Andreas has merged into master.

Mike
Mike Rosset wrote 4 years ago
(name . Mark H Weaver)(address . mhw@netris.org)
878sday8ra.fsf@gmail.com
Mark H Weaver <mhw@netris.org> writes:

Toggle quote (14 lines)
> Earlier, I wrote:
>> 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.
>
> Sorry, I made a mistake in the details here, since the pattern applies
> only when 'i' is already initialized to 0, but the more general point
> still stands, namely that patches are more robust than 'substitute*' for
> fixing bugs, and less likely to be misapplied or left forgotten in a
> vestigial state after they are no longer needed.
>
> Mark

That change was not quite fitting right with me either.
So I appreciate the review and comments. Thankful it's not needed with
1.9.14 and I'll take your comments into account in the future.

Mike
Mark H Weaver wrote 4 years ago
(name . Mike Rosset)(address . mike.rosset@gmail.com)
87v9geacau.fsf@netris.org
Mike Rosset <mike.rosset@gmail.com> writes:

Toggle quote (10 lines)
> Mark H Weaver <mhw@netris.org> writes:
>
>> [...] patches are more robust than 'substitute*' for
>> fixing bugs, and less likely to be misapplied or left forgotten in a
>> vestigial state after they are no longer needed.
>
> That change was not quite fitting right with me either.
> So I appreciate the review and comments. Thankful it's not needed with
> 1.9.14 and I'll take your comments into account in the future.

Sounds good. Thank you, Mike!

Mark
?
Your comment

This issue is archived.

To comment on this conversation send an email to 43232@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 43232
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help