[bug#35483] gnu: u-boot-tools: Only run full test suite on x86.

* gnu/packages/bootloaders (u-boot-tools)[check]: Remove x86 tests.
  [check-x86]: New phase with x86 tests.
  [patch]: Patch test-imagetools.sh to test binaries to be installed.
---
 gnu/packages/bootloaders.scm | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

I neglected to mention in my initial submission that the test suite has
failed on non-x86 architectures since I started testing u-boot on Guix a
year or so ago...

It blocks working support for veyron-speedy/Asus C201, and that would be
very nice to have fixed for 1.0, of course!

I may need to update the patch to exclude i686-linux from the full test
suite, as it seems with u-boot 2019.04 to also fail with 32-bit x86
making some 64-bit assumptions...

Thanks for the review, if you can!

live well,
  vagrant

On 2019-04-28, Vagrant Cascadian wrote:
> * gnu/packages/bootloaders (u-boot-tools)[check]: Remove x86 tests.
>   [check-x86]: New phase with x86 tests.
>   [patch]: Patch test-imagetools.sh to test binaries to be installed.
> ---
>  gnu/packages/bootloaders.scm | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
> index b4eabaea48..0cd72fb44c 100644
> --- a/gnu/packages/bootloaders.scm
> +++ b/gnu/packages/bootloaders.scm
> @@ -442,6 +442,10 @@ also initializes the boards (RAM etc).")
>                (("def test_ctrl_c")
>                 "@pytest.mark.skip(reason='Guix has problems with SIGINT')
>  def test_ctrl_c"))
> +             ;; Test against the tools being installed rather than tools built
> +             ;; for "sandbox" target.
> +             (substitute* "test/image/test-imagetools.sh"
> +               (("BASEDIR=sandbox") "BASEDIR=."))
>               (for-each (lambda (file)
>                                (substitute* file
>                                    ;; Disable signatures, due to GPL/Openssl
> @@ -484,12 +488,19 @@ def test_ctrl_c"))
>             (delete 'check)
>             (add-after 'install 'check
>               (lambda* (#:key make-flags test-target #:allow-other-keys)
> -               (apply invoke "make" "mrproper" make-flags)
> -               (setenv "SDL_VIDEODRIVER" "dummy")
> -               (setenv "PAGER" "cat")
> -               (apply invoke "make" test-target make-flags)
> -               (symlink "build-sandbox_spl" "sandbox")
> -               (invoke "test/image/test-imagetools.sh"))))))
> +               (invoke "test/image/test-imagetools.sh")))
> +           ;; Only run full test suite on x86 systems, as many tests assume
> +           ;; x86.
> +           ,@(if (string-match "^(x86_64|i686)-linux"
> +                               (or (%current-target-system)
> +                                   (%current-system)))
> +                 '((add-after 'check 'check-x86
> +                     (lambda* (#:key make-flags test-target #:allow-other-keys)
> +                       (apply invoke "make" "mrproper" make-flags)
> +                       (setenv "SDL_VIDEODRIVER" "dummy")
> +                       (setenv "PAGER" "cat")
> +                       (apply invoke "make" test-target make-flags))))
> +                 '()))))
>      (description "U-Boot is a bootloader used mostly for ARM boards.  It
>  also initializes the boards (RAM etc).  This package provides its
>  board-independent tools.")))
> -- 
> 2.20.1

Hi Vagrant,
Hi Ludo,

On Mon, 29 Apr 2019 16:32:40 -0700
Vagrant Cascadian <vagrant@debian.org> wrote:

> I neglected to mention in my initial submission that the test suite has
> failed on non-x86 architectures since I started testing u-boot on Guix a
> year or so ago...
> 
> It blocks working support for veyron-speedy/Asus C201, and that would be
> very nice to have fixed for 1.0, of course!
> 
> I may need to update the patch to exclude i686-linux from the full test
> suite, as it seems with u-boot 2019.04 to also fail with 32-bit x86
> making some 64-bit assumptions...
> 
> Thanks for the review, if you can!

I've reviewed it and found that also on i686 tests are not succeeding.
So I've changed the check to only enable the respective parts of the tests
for x86_64 and pushed it to guix master as
commit 6f5be83cd7158e678602d62c27f26363df3e1649.  Thanks!

I'm not sure whether I should still cherry-pick it for 1.0, so I didn't yet.

> I've reviewed it and found that also on i686 tests are not succeeding.

... or at least they are succeeding with a lot of very serious warnings,
some of which concerning stack alignment for variable arguments and shifts
that are more than the width of the variable slot (which are undefined
on the assembly level and do "funny" things on i686 if you try anyway).

I'm leaving the bug report open since it's one thing if the tests are not
succeeding because the *tests* are broken and another thing if the tests
are not succeeding because the functionality it is testing is broken.
At least on i686 u-boot-tools, the latter seems to be the case often.

So before closing this bug report, let's bring this up upstream and also
let's disable features in the u-boot configuration that are broken but
we don't need anyway.

On 2019-04-30, Danny Milosavljevic wrote:
> On Mon, 29 Apr 2019 16:32:40 -0700
> Vagrant Cascadian <vagrant@debian.org> wrote:
>
>> I neglected to mention in my initial submission that the test suite has
>> failed on non-x86 architectures since I started testing u-boot on Guix a
>> year or so ago...
>> 
>> It blocks working support for veyron-speedy/Asus C201, and that would be
>> very nice to have fixed for 1.0, of course!
>> 
>> I may need to update the patch to exclude i686-linux from the full test
>> suite, as it seems with u-boot 2019.04 to also fail with 32-bit x86
>> making some 64-bit assumptions...
>> 
>> Thanks for the review, if you can!
>
> I've reviewed it and found that also on i686 tests are not succeeding.
> So I've changed the check to only enable the respective parts of the tests
> for x86_64 and pushed it to guix master as
> commit 6f5be83cd7158e678602d62c27f26363df3e1649.  Thanks!
>
> I'm not sure whether I should still cherry-pick it for 1.0, so I didn't yet.

I got confirmation from serveral people on irc to cherry-pick it for
1.0, and with my newfound powers... have pushed it to the version-1.0.0
branch!


live well,
  vagrant

Vagrant Cascadian <vagrant@debian.org> skribis:

> On 2019-04-30, Danny Milosavljevic wrote:

[...]

>> I've reviewed it and found that also on i686 tests are not succeeding.
>> So I've changed the check to only enable the respective parts of the tests
>> for x86_64 and pushed it to guix master as
>> commit 6f5be83cd7158e678602d62c27f26363df3e1649.  Thanks!
>>
>> I'm not sure whether I should still cherry-pick it for 1.0, so I didn't yet.
>
> I got confirmation from serveral people on irc to cherry-pick it for
> 1.0, and with my newfound powers... have pushed it to the version-1.0.0
> branch!

Cool, thank you!

Ludo’.