Re: [bug#47615] [PATCH 3/9] gnu: binutils: Adjust test suite on powerpc-linux.
On Wed, Apr 21, 2021 at 10:11:48PM -0700, Chris Marusich wrote: > Efraim Flashner writes: > > >> If it's a test failure, has the issue been reported upstream? > > > > I'll check to see if I can find something upstream. If not I'll report > > it. FWIW Debian disables the test suite for powerpc. > > https://sources.debian.org/src/binutils/2.36.1-6/debian/rules/#L537 > > How significant is the failure, anyway? Are we likely to encounter > trouble down the line on powerpc if we don't resolve this? I have no > idea, honestly, so I'm asking because I just don't know. I submitted the bug upstream. It turns out it was a bug in the test suite which only showed up on big-endian systems. They've fixed it in libiberty (in GCC) and I added the patch to binutils on core-updates. Tests pass on powerpc-linux and x86_64-linux. It also made this commit much much smaller :) -- 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
Re: bug#47615: [PATCH 3/9] gnu: binutils: Adjust test suite on powerpc-linux.
Efraim Flashner writes: >> If it's a test failure, has the issue been reported upstream? > > I'll check to see if I can find something upstream. If not I'll report > it. FWIW Debian disables the test suite for powerpc. > https://sources.debian.org/src/binutils/2.36.1-6/debian/rules/#L537 How significant is the failure, anyway? Are we likely to encounter trouble down the line on powerpc if we don't resolve this? I have no idea, honestly, so I'm asking because I just don't know. I looked at the rules file you linked. Are you sure it disables the test for powerpc? It reads: with_check := yes ifneq (,$(findstring nocheck,$(DEB_BUILD_OPTIONS))) # override buildd admins to run the testsuite anyway ... ifeq (,$(filter $(DEB_HOST_ARCH), m68k powerpc sh4 sparc64)) with_check := disabled through DEB_BUILD_OPTIONS endif endif #with_check := disabled for this upload I'm not sure what the values for DEB_HOST_ARCH or DEB_BUILD_OPTIONS would be here. However, it seems to be setting with_check to disabled if and only if (1) nocheck shows up in DEB_BUILD_OPTIONS and (2) DEB_HOST_ARCH is not one of m68k, powerpc, sh4, or sparc64. In other words, it seems to enable the tests unless nocheck is in DEB_BUILD_OPTIONS, in which case it STILL enables the tests provided that DEB_HOST_ARCH is one of m68k, powerpc, sh4, or sparc64. I'm not very familiar with Debian rules files, so perhaps I'm mistaken. However, I think this means that when DEB_BUILD_OPTIONS doesn't contain "nocheck" (presumably it doesn't usually?), the tests will be run for every platform. In any case, reporting this upstream seems like the right thing to do. > I like the way you've done it (and the comment). The fewer places with > copy/pasted code the better. I'm building it again now to see about the > tests. With your changes I only need to add the phase in (gnu packages > base). I think the comment is honestly more than necessary, but if you find it helpful, perhaps someone in the future might, too. It describes a possibly surprising behavior of substitute-keyword-arguments that can happen any time you invoke it. I just wanted to call it out specifically for the purpose of this patch review. -- Chris signature.asc Description: PGP signature
Re: bug#47615: [PATCH 3/9] gnu: binutils: Adjust test suite on powerpc-linux.
Chris Marusich writes:
> (define binutils-boot0
> (package
> (inherit binutils)
> (source (bootstrap-origin (package-source binutils)))
> (name "binutils-cross-boot0")
> (arguments
> `(#:guile ,%bootstrap-guile
>#:implicit-inputs? #f
>
>#:modules ((guix build gnu-build-system)
> (guix build utils)
> (ice-9 ftw)); for 'scandir'
>
>,@(substitute-keyword-arguments (package-arguments binutils)
>((#:configure-flags cf)
> `(cons ,(string-append "--target=" (boot-triplet))
>,cf))
>;; The presence of '%standard-phases as the default value here is
>;; important. It ensures that even when (package-argument
>;; binutils) does not already contain the #:phases keyword
>;; argument, the substitution will occur. If you omit a default
>;; value and (package-arguments binutils) does not contain the
>;; #:phases keyword argument (e.g., on an x86_64-linux system),
>;; then the substitution will not occur, and no phases at all will
>;; be added.
>((#:phases phases '%standard-phases)
> `(modify-phases ,phases
>,@(if (string=? (%current-system) "powerpc-linux")
> '((add-after 'unpack 'disable-rust-libiberty-test
> (lambda _
>(substitute* "libiberty/testsuite/Makefile.in"
> ((" check-rust-demangle ") ""))
>#t)))
> '())
>(add-after 'install 'add-symlinks
> (lambda* (#:key outputs #:allow-other-keys)
>;; The cross-gcc invokes 'as', 'ld', etc, without the
>;; triplet prefix, so add symlinks.
>(let ((out (assoc-ref outputs "out"))
> (triplet-prefix (string-append ,(boot-triplet) "-")))
> (define (has-triplet-prefix? name)
>(string-prefix? triplet-prefix name))
> (define (remove-triplet-prefix name)
>(substring name (string-length triplet-prefix)))
> (with-directory-excursion (string-append out "/bin")
>(for-each
> (lambda (name)
> (symlink name (remove-triplet-prefix name)))
> (scandir "." has-triplet-prefix?)))
>
> (inputs (%boot0-inputs
Sorry, I meant to write this instead:
(define binutils-boot0
(package
(inherit binutils)
(source (bootstrap-origin (package-source binutils)))
(name "binutils-cross-boot0")
(arguments
`(#:guile ,%bootstrap-guile
#:implicit-inputs? #f
#:modules ((guix build gnu-build-system)
(guix build utils)
(ice-9 ftw)); for 'scandir'
,@(substitute-keyword-arguments (package-arguments binutils)
((#:configure-flags cf)
`(cons ,(string-append "--target=" (boot-triplet))
,cf))
;; The presence of '%standard-phases as the default value here is
;; important. It ensures that even when (package-argument
;; binutils) does not already contain the #:phases keyword
;; argument, the substitution will occur. If you omit a default
;; value and (package-arguments binutils) does not contain the
;; #:phases keyword argument (e.g., on an x86_64-linux system),
;; then the substitution will not occur, and no phases at all will
;; be added.
((#:phases phases '%standard-phases)
`(modify-phases ,phases
(add-after 'install 'add-symlinks
(lambda* (#:key outputs #:allow-other-keys)
;; The cross-gcc invokes 'as', 'ld', etc, without the
;; triplet prefix, so add symlinks.
(let ((out (assoc-ref outputs "out"))
(triplet-prefix (string-append ,(boot-triplet) "-")))
(define (has-triplet-prefix? name)
(string-prefix? triplet-prefix name))
(define (remove-triplet-prefix name)
(substring name (string-length triplet-prefix)))
(with-directory-excursion (string-append out "/bin")
(for-each
(lambda (name)
(symlink name (remove-triplet-prefix name)))
(scandir "." has-triplet-prefix?)))
(inputs (%boot0-inputs
The point is that we can probably "inherit" the phases, so we wouldn't
have to repeat ourselves.
--
Chris
signature.asc
Description: PGP signature
[PATCH 3/9] gnu: binutils: Adjust test suite on powerpc-linux.
* gnu/packages/base.scm (binutils)[arguments]: Add phase on
powerpc-linux to adjust the test suite.
* gnu/packages/commencement.scm (binutils-boot0)[arguments]: Move custom
phases after inherited arguments. Add phase on powerpc-linux to adjust
the test suite.
---
gnu/packages/base.scm | 11 ++-
gnu/packages/commencement.scm | 21 -
2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index dbb7c619fe..b9fc0a6e29 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -531,7 +531,16 @@ change. GNU make offers many powerful extensions over the
standard utility.")
;; Make sure 'ar' and 'ranlib' produce archives in a
;; deterministic fashion.
- "--enable-deterministic-archives")))
+ "--enable-deterministic-archives")
+ ,@(if (string=? (%current-system) "powerpc-linux")
+ `(#:phases
+(modify-phases %standard-phases
+ (add-after 'unpack 'disable-rust-libiberty-test
+(lambda _
+ (substitute* "libiberty/testsuite/Makefile.in"
+((" check-rust-demangle ") ""))
+ #t
+ '(
(synopsis "Binary utilities: bfd gas gprof ld")
(description
diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 7c39a84008..f707a01d30 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -2653,7 +2653,22 @@ exec " gcc "/bin/" program
#:modules ((guix build gnu-build-system)
(guix build utils)
(ice-9 ftw)); for 'scandir'
+
+ ;; #:phases gets modified for powerpc-linux in binutils,
+ ;; so #:phases here needs to be after the inherited one.
+ ,@(substitute-keyword-arguments (package-arguments binutils)
+ ((#:configure-flags cf)
+`(cons ,(string-append "--target=" (boot-triplet))
+ ,cf)))
+
#:phases (modify-phases %standard-phases
+ ,@(if (string=? (%current-system) "powerpc-linux")
+ '((add-after 'unpack 'disable-rust-libiberty-test
+ (lambda _
+(substitute* "libiberty/testsuite/Makefile.in"
+ ((" check-rust-demangle ") ""))
+#t)))
+ '())
(add-after 'install 'add-symlinks
(lambda* (#:key outputs #:allow-other-keys)
;; The cross-gcc invokes 'as', 'ld', etc, without the
@@ -2667,12 +2682,8 @@ exec " gcc "/bin/" program
(with-directory-excursion (string-append out "/bin")
(for-each (lambda (name)
(symlink name (remove-triplet-prefix
name)))
-(scandir "." has-triplet-prefix?)))
+(scandir "." has-triplet-prefix?)
- ,@(substitute-keyword-arguments (package-arguments binutils)
- ((#:configure-flags cf)
-`(cons ,(string-append "--target=" (boot-triplet))
- ,cf)
(inputs (%boot0-inputs
(define libstdc++-boot0
--
2.31.1
