guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: bug#47615: [PATCH 3/9] gnu: binutils: Adjust test suite on powerpc-l


From: Chris Marusich
Subject: Re: bug#47615: [PATCH 3/9] gnu: binutils: Adjust test suite on powerpc-linux.
Date: Wed, 21 Apr 2021 22:11:48 -0700
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Efraim Flashner <efraim@flashner.co.il> 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

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]