[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[Danny Milosavljevic]

* gnu/packages/bootloaders.scm (dtc)[native-search-paths]: Add PYTHONPATH.
(u-boot-tools)[native-inputs]: Add openssl, python2-coverage, python2-pytest,
sdl.
[arguments]<#:make-flags>: Remove NO_SDL.
[arguments]<#:test-target>: Change to "tests".
[arguments]<#:phases>[patch]: New phase.
[arguments]<#:phases>[check]: Replace and move.
---
 gnu/packages/bootloaders.scm | 49 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 333f30e72..4041f2320 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -33,6 +33,7 @@
   #:use-module (gnu packages disk)
   #:use-module (gnu packages bison)
   #:use-module (gnu packages cdrom)
+  #:use-module (gnu packages check)
   #:use-module (gnu packages cross-base)
   #:use-module (gnu packages disk)
   #:use-module (gnu packages firmware)
@@ -49,6 +50,7 @@
   #:use-module (gnu packages python)
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages tls)
+  #:use-module (gnu packages sdl)
   #:use-module (gnu packages swig)
   #:use-module (gnu packages virtualization)
   #:use-module (gnu packages web)
@@ -317,6 +319,10 @@ menu to select one of the installed operating systems.")
      `(("bison" ,bison)
        ("flex" ,flex)
        ("swig" ,swig)))
+    (native-search-paths
+      (list (search-path-specification
+             (variable "PYTHONPATH")
+             (files '("/lib/python2.7/site-packages")))))
     (inputs
      `(("python-2" ,python-2)))
     (arguments
@@ -350,7 +356,11 @@ tree binary files.  These are board description files used by Linux and BSD.")
     (native-inputs
      `(("bc" ,bc)
        ("dtc" ,dtc)
+       ("openssl" ,openssl)
        ("python-2" ,python-2)
+       ("python2-coverage" ,python2-coverage)
+       ("python2-pytest" ,python2-pytest)
+       ("sdl" ,sdl)
        ("swig" ,swig)))
     (build-system  gnu-build-system)
     (home-page "http://www.denx.de/wiki/U-Boot/")
@@ -364,10 +374,34 @@ also initializes the boards (RAM etc).")
     (inherit u-boot)
     (name "u-boot-tools")
     (arguments
-     `(#:make-flags '("HOSTCC=gcc" "NO_SDL=1")
-       #:test-target "test"
+     `(#:make-flags '("HOSTCC=gcc")
+       #:test-target "tests"
        #:phases
        (modify-phases %standard-phases
+         (add-after 'unpack 'patch
+           (lambda* (#:key inputs #:allow-other-keys)
+             (substitute* "Makefile"
+              (("/bin/pwd") (which "pwd"))
+              (("/bin/false") (which "false")))
+             (substitute* "tools/dtoc/fdt_util.py"
+              (("'cc'") "'gcc'"))
+             (substitute* "test/run"
+              ;; Make it easier to find test failures.
+              (("#!/bin/bash") "#!/bin/bash -x")
+              ;; pytest doesn't find it otherwise.
+              (("test/py/tests/test_ofplatdata.py")
+               "tests/test_ofplatdata.py")
+              ;; This test would require git.
+              (("\\./tools/patman/patman") (which "true"))
+              ;; This test would require internet access.
+              (("\\./tools/buildman/buildman") (which "true")))
+             (substitute* "test/py/tests/test_sandbox_exit.py"
+              (("def test_ctrl_c")
+               "@pytest.mark.skip(reason='Guix has problems with SIGINT')
+def test_ctrl_c"))
+             (substitute* "tools/binman/binman.py"
+              (("100%") "99%")) ; TODO: Find out why that is needed.
+             #t))
          (replace 'configure
            (lambda* (#:key make-flags #:allow-other-keys)
              (call-with-output-file "configs/tools_defconfig"
@@ -395,7 +429,16 @@ also initializes the boards (RAM etc).")
                            "tools/proftool"
                            "tools/fdtgrep"
                            "tools/env/fw_printenv"))
-               #t))))))
+               #t)))
+           (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"))))))
     (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.")))

[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[Ludovic Courtès]

Hey Danny,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/packages/bootloaders.scm (dtc)[native-search-paths]: Add PYTHONPATH.
> (u-boot-tools)[native-inputs]: Add openssl, python2-coverage, python2-pytest,
> sdl.
> [arguments]<#:make-flags>: Remove NO_SDL.
> [arguments]<#:test-target>: Change to "tests".
> [arguments]<#:phases>[patch]: New phase.
> [arguments]<#:phases>[check]: Replace and move.

No need to repeat “[arguments]”.

> +    (native-search-paths
> +      (list (search-path-specification
> +             (variable "PYTHONPATH")
> +             (files '("/lib/python2.7/site-packages")))))

Why is it needed?  That would need a comment.  :-)  Even better if we
can achieve the same effect without defining this search path.

Also no need for the leading slash in ‘files’.

> +           (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"))))))

Please return #t.

Thanks!

Ludo’.

[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

Hi Ludo,

On Sat, 26 May 2018 20:26:20 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Why is it needed?  That would need a comment.  :-)

dtc has Python bindings and we build them.

The u-boot-tools tests use them (nothing else does).

I'll add a comment.

>  Even better if we
> can achieve the same effect without defining this search path.

Sure, I guess.  If you install dtc into your profile it won't work then though.

Also, do you think we should try to extract the Python bindings into its own
package?

> Also no need for the leading slash in ‘files’.

Okay.

> > +           (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"))))))  
> 
> Please return #t.

invoke does :)

We can add another one, though.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[Ludovic Courtès]

Hello Danny!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Sat, 26 May 2018 20:26:20 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> Why is it needed?  That would need a comment.  :-)
>
> dtc has Python bindings and we build them.

But then we do not need a search path definition in dtc itself; the one
in Python is enough.  Or am I missing something?

>>  Even better if we
>> can achieve the same effect without defining this search path.
>
> Sure, I guess.  If you install dtc into your profile it won't work then though.

I think there are two possible situations:

  1. dtc provides Python bindings that are useful to Python
     programmers.  This is addressed by having both Python and dtc in
     the same profile, in which case PYTHONPATH is automatically
     defined.  Nothing special to do.

  2. dtc is itself written in Python and needs to access its own
     Python code.  In that case we should wrap the ‘dtc’ command such
     that PYTHONPATH contains the right thing.

Does that make sense?

> Also, do you think we should try to extract the Python bindings into its own
> package?

Not necessarily, it probably doesn’t take up much space.

>> > +           (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"))))))  
>> 
>> Please return #t.
>
> invoke does :)

Oh, true, sorry!

Thanks,
Ludo’.

[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

Hi Ludo,

On Sun, 27 May 2018 15:10:05 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> > dtc has Python bindings and we build them.  
> 
> But then we do not need a search path definition in dtc itself; the one
> in Python is enough.  Or am I missing something?

There's a libfdt.py, provided by the dtc package, and u-boot-tools
tests use it.

Right now, we're not necessarily installing libfdt.py into python (?) or the
profile or anything.

Like u-boot-tools has somewhere (in its unit tests):

  #!/usr/bin/env python3
  import pylibfdt

And dtc provides

  libfdt.py

which could be useful for Python programmers, too.

>   1. dtc provides Python bindings that are useful to Python
>      programmers.  This is addressed by having both Python and dtc in
>      the same profile, in which case PYTHONPATH is automatically
>      defined.  Nothing special to do.

Yes.  But in this case u-boot-tools uses libfdt.py when building (testing) itself.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[Ludovic Courtès]

Morning!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Sun, 27 May 2018 15:10:05 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> > dtc has Python bindings and we build them.  
>> 
>> But then we do not need a search path definition in dtc itself; the one
>> in Python is enough.  Or am I missing something?
>
> There's a libfdt.py, provided by the dtc package, and u-boot-tools
> tests use it.

OK.  If u-boot-tools has both Python and dtc as inputs, then PYTHONPATH
is correctly defined, isn’t it?

> Right now, we're not necessarily installing libfdt.py into python (?) or the
> profile or anything.

So we should make sure dtc installs libfdt.py in $prefix/lib/python2.7/….

Does that make sense?

Ludo’.

bug#31520: [PATCH] gnu: u-boot-tools: Enable tests.

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Hi Ludo,

On Mon, 28 May 2018 09:57:37 +0200
ludo@gnu.org (Ludovic Courtès) wrote:
> OK.  If u-boot-tools has both Python and dtc as inputs, then PYTHONPATH
> is correctly defined, isn’t it?

Hmm, then I don't understand how native-search-paths works.

I thought native-search-paths would just add $package/$path for the current
package--but it seems guix uses it as a pattern in all packages, not just
the package that specified it.

It indeed works if I remove native-search-paths from dtc, so I've pushed it
without.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[bug#31520] [PATCH] gnu: u-boot-tools: Enable tests.

[Ludovic Courtès]

Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Hmm, then I don't understand how native-search-paths works.

The search path spec is attached to the package that “owns” the given
variables.  So python has a spec for PYTHONPATH, Guile has a spec for
GUILE_LOAD_PATH, and so on.

Then, if you have both python and a python package in the same profile,
the machinery automatically defines PYTHONPATH according to the spec.

> It indeed works if I remove native-search-paths from dtc, so I've pushed it
> without.

Cool, thanks!

Ludo’.