bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
Mike Rosset writes: > Mark H Weaver 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
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
Mark H Weaver writes: > 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
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
Mark H Weaver writes:
> Efraim Flashner 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
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
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
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
Efraim Flashner 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
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
* 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(-)
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
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
Efraim Flashner writes:
> 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
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
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.
>
> 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.
> (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 אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted
signature.asc
Description: PGP signature
bug#43232: [PATCH] gnu: jack-2: Update to 1.9.14.
* 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))
(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
