[PATCH] GCJ: run libjava tests.

[PATCH] GCJ: run libjava tests.

[Ricardo Wurmus]

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

Hi Guix,

GCJ is likely broken on MIPS and ARM (as can be seen in the way Ant
fails to build on these platforms), but the package builds just fine.
That’s probably because we are not running the libjava tests, which
require dejagnu.

The attached patch adds dejagnu to the native-inputs and replaces the
check phase to build and run the libjava tests.

This patch will likely cause the GCJ package to fail on MIPS and ARM,
but that’s expected and helpful, as it should give us a hint how to make
it work on these platforms.

~~ Ricardo


[-- Attachment #2: 0001-gnu-gcj-Run-libjava-tests.patch --]
[-- Type: text/x-patch, Size: 2138 bytes --]

From b31be9b6abe66db1620214ad89b8d81342ac2ccd Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
Date: Thu, 22 Oct 2015 13:02:37 +0200
Subject: [PATCH] gnu: gcj: Run libjava tests.

* gnu/packages/gcc.scm (gcj)[native-inputs]: Add dejagnu.
  [arguments]: Replace check phase to run "check-target-libjava" target.
---
 gnu/packages/gcc.scm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index 7e4f18b..b213be9 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014, 2015 Mark H Weaver <mhw@netris.org>
-;;; Copyright © 2014 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2014, 2015 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2015 Andreas Enge <andreas@enge.fr>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -27,6 +27,7 @@
   #:use-module (gnu packages compression)
   #:use-module (gnu packages multiprecision)
   #:use-module (gnu packages texinfo)
+  #:use-module (gnu packages dejagnu)
   #:use-module (gnu packages doxygen)
   #:use-module (gnu packages xml)
   #:use-module (gnu packages docbook)
@@ -460,6 +461,9 @@ using compilers other than GCC."
        ("javac.in" ,javac.in)
        ("ecj-bootstrap" ,ecj-bootstrap)
        ,@(package-inputs gcc)))
+    (native-inputs
+     `(("dejagnu" ,dejagnu)
+       ,@(package-native-inputs gcc)))
     ;; Suppress the separate "lib" output, because otherwise the
     ;; "lib" and "out" outputs would refer to each other, creating
     ;; a cyclic dependency.  <http://debbugs.gnu.org/18101>
@@ -482,6 +486,8 @@ using compilers other than GCC."
                        ,flags))))
        ((#:phases phases)
         `(modify-phases ,phases
+           (replace 'check
+            (lambda _ (zero? (system* "make" "check-target-libjava"))))
            (add-after
             'unpack 'add-lib-output-to-rpath
             (lambda _
-- 
2.1.0

Re: [PATCH] GCJ: run libjava tests.

[Andreas Enge]

Hello,

On Thu, Oct 22, 2015 at 01:07:25PM +0200, Ricardo Wurmus wrote:
>         ((#:phases phases)
>          `(modify-phases ,phases
> +           (replace 'check
> +            (lambda _ (zero? (system* "make" "check-target-libjava"))))

Could this be replaced by
   #:test-target "check-target-libjava"
? Apart from that, your patch looks very useful.

Andreas

Re: [PATCH] GCJ: run libjava tests.

[Ricardo Wurmus]

Andreas Enge <andreas@enge.fr> writes:

> On Thu, Oct 22, 2015 at 01:07:25PM +0200, Ricardo Wurmus wrote:
>>         ((#:phases phases)
>>          `(modify-phases ,phases
>> +           (replace 'check
>> +            (lambda _ (zero? (system* "make" "check-target-libjava"))))
>
> Could this be replaced by
>    #:test-target "check-target-libjava"
> ? Apart from that, your patch looks very useful.

Ah, good idea.  I’m trying to build GCJ right now by adding this

    ((#:tests? _) #t)
    ((#:test-target _) "check-target-libjava")

to the ‘substitute-keyword-arguments’ expression instead of replacing
the “check” phase.  If this works I’ll push an updated patch.

Thanks!

~~ Ricardo

Re: [PATCH] GCJ: run libjava tests.

[Ricardo Wurmus]

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:

> Andreas Enge <andreas@enge.fr> writes:
>
>> On Thu, Oct 22, 2015 at 01:07:25PM +0200, Ricardo Wurmus wrote:
>>>         ((#:phases phases)
>>>          `(modify-phases ,phases
>>> +           (replace 'check
>>> +            (lambda _ (zero? (system* "make" "check-target-libjava"))))
>>
>> Could this be replaced by
>>    #:test-target "check-target-libjava"
>> ? Apart from that, your patch looks very useful.
>
> Ah, good idea.  I’m trying to build GCJ right now by adding this
>
>     ((#:tests? _) #t)
>     ((#:test-target _) "check-target-libjava")
>
> to the ‘substitute-keyword-arguments’ expression instead of replacing
> the “check” phase.  If this works I’ll push an updated patch.

Sadly, this doesn’t work.  When tests are re-enabled *all* tests are
run, including tests for g++, no matter what I set as a test target.

I don’t know why.  Do you have any ideas?

~~ Ricardo

Re: [PATCH] GCJ: run libjava tests.

[Andreas Enge]

On Thu, Oct 22, 2015 at 04:58:10PM +0200, Ricardo Wurmus wrote:
> I don’t know why.  Do you have any ideas?

No idea. Your initial patch
+           (replace 'check
+            (lambda _ (zero? (system* "make" "check-target-libjava"))))
looks a lot like the check build phase of gnu-build-system:
      (zero? (apply system* "make" test-target
                    `(,@(if parallel-tests?
                            `("-j" ,(number->string (parallel-job-count)))
                            '())
                      ,@make-flags)))
Besides the parallelism, the only difference could be in the make-flags.

Andreas

Re: [PATCH] GCJ: run libjava tests.

[Ludovic Courtès]

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:
>
>> Andreas Enge <andreas@enge.fr> writes:
>>
>>> On Thu, Oct 22, 2015 at 01:07:25PM +0200, Ricardo Wurmus wrote:
>>>>         ((#:phases phases)
>>>>          `(modify-phases ,phases
>>>> +           (replace 'check
>>>> +            (lambda _ (zero? (system* "make" "check-target-libjava"))))
>>>
>>> Could this be replaced by
>>>    #:test-target "check-target-libjava"
>>> ? Apart from that, your patch looks very useful.
>>
>> Ah, good idea.  I’m trying to build GCJ right now by adding this
>>
>>     ((#:tests? _) #t)
>>     ((#:test-target _) "check-target-libjava")
>>
>> to the ‘substitute-keyword-arguments’ expression instead of replacing
>> the “check” phase.  If this works I’ll push an updated patch.
>
> Sadly, this doesn’t work.  When tests are re-enabled *all* tests are
> run, including tests for g++, no matter what I set as a test target.
>
> I don’t know why.  Do you have any ideas?

‘substitute-keyword-arguments’ only works if the given keyword appears
in the input:

  (substitute-keyword-arguments '()
    ((#:foo _) 'bar))

  => ()

It might be what’s happening here?

Ludo’.

Re: [PATCH] GCJ: run libjava tests.

[Ricardo Wurmus]

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


Ludovic Courtès <ludo@gnu.org> writes:

> ‘substitute-keyword-arguments’ only works if the given keyword appears
> in the input:
>
>   (substitute-keyword-arguments '()
>     ((#:foo _) 'bar))
>
>   => ()
>
> It might be what’s happening here?

You are right, this exactly what happened here.

Attached is a new patch that pulls the ‘#:test-target’ line out of the
‘substitute-keyword-arguments’ expression.  The indentation of many
lines changes because of this.  ‘git diff -w’ shows that the actual
change is pretty small.


[-- Attachment #2: 0001-gnu-gcj-Run-libjava-tests.patch --]
[-- Type: text/x-patch, Size: 8624 bytes --]

From 95609455bbdaa43699a66efe9a5508c06e0bf9ad Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
Date: Thu, 22 Oct 2015 13:02:37 +0200
Subject: [PATCH] gnu: gcj: Run libjava tests.

* gnu/packages/gcc.scm (gcj)[native-inputs]: Add dejagnu.
  [arguments]: Replace check phase to run "check-target-libjava" target.
---
 gnu/packages/gcc.scm | 138 +++++++++++++++++++++++++++------------------------
 1 file changed, 72 insertions(+), 66 deletions(-)

diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index 7e4f18b..5d07255 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014, 2015 Mark H Weaver <mhw@netris.org>
-;;; Copyright © 2014 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2014, 2015 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2015 Andreas Enge <andreas@enge.fr>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -27,6 +27,7 @@
   #:use-module (gnu packages compression)
   #:use-module (gnu packages multiprecision)
   #:use-module (gnu packages texinfo)
+  #:use-module (gnu packages dejagnu)
   #:use-module (gnu packages doxygen)
   #:use-module (gnu packages xml)
   #:use-module (gnu packages docbook)
@@ -460,77 +461,82 @@ using compilers other than GCC."
        ("javac.in" ,javac.in)
        ("ecj-bootstrap" ,ecj-bootstrap)
        ,@(package-inputs gcc)))
+    (native-inputs
+     `(("dejagnu" ,dejagnu)
+       ,@(package-native-inputs gcc)))
     ;; Suppress the separate "lib" output, because otherwise the
     ;; "lib" and "out" outputs would refer to each other, creating
     ;; a cyclic dependency.  <http://debbugs.gnu.org/18101>
     (outputs
      (delete "lib" (package-outputs gcc)))
     (arguments
-     (substitute-keyword-arguments `(#:modules ((guix build gnu-build-system)
-                                                (guix build utils)
-                                                (ice-9 regex)
-                                                (srfi srfi-1)
-                                                (srfi srfi-26))
-                                               ,@(package-arguments gcc))
-       ((#:configure-flags flags)
-        `(let ((ecj (assoc-ref %build-inputs "ecj-bootstrap")))
-           `("--enable-java-home"
-             "--enable-gjdoc"
-             ,(string-append "--with-ecj-jar=" ecj)
-             "--enable-languages=java"
-             ,@(remove (cut string-match "--enable-languages.*" <>)
-                       ,flags))))
-       ((#:phases phases)
-        `(modify-phases ,phases
-           (add-after
-            'unpack 'add-lib-output-to-rpath
-            (lambda _
-              (substitute* "libjava/Makefile.in"
-                (("libgcj_bc_dummy_LINK = .* -shared" line)
-                 (string-append line " -Wl,-rpath=$(libdir)"))
-                (("libgcj(_bc)?_la_LDFLAGS =" ldflags _)
-                 (string-append ldflags " -Wl,-rpath=$(libdir)")))))
-           (add-after
-            'install 'install-javac-and-javap-wrappers
-            (lambda _
-              (let* ((javac  (assoc-ref %build-inputs "javac.in"))
-                     (ecj    (assoc-ref %build-inputs "ecj-bootstrap"))
-                     (gcj    (assoc-ref %outputs "out"))
-                     (gcjbin (string-append gcj "/bin/"))
-                     (jvm    (string-append gcj "/lib/jvm/"))
-                     (target (string-append jvm "/bin/javac")))
-
-                (symlink (string-append gcjbin "jcf-dump")
-                         (string-append jvm "/bin/javap"))
-
-                (copy-file ecj (string-append gcj "/share/java/ecj.jar"))
-
-                ;; Create javac wrapper from the template javac.in by
-                ;; replacing the @VARIABLES@ with paths.
-                (copy-file javac target)
-                (patch-shebang target)
-                (substitute* target
-                  (("@JAVA@")
-                   (string-append jvm "/bin/java"))
-                  (("@ECJ_JAR@")
-                   (string-append gcj "/share/java/ecj.jar"))
-                  (("@RT_JAR@")
-                   (string-append jvm "/jre/lib/rt.jar"))
-                  (("@TOOLS_JAR@")
-                   (string-append jvm "/lib/tools.jar")))
-                (chmod target #o755)
-                #t)))
-           (add-after
-            'install 'remove-broken-or-conflicting-files
-            (lambda _
-              (let ((out (assoc-ref %outputs "out")))
-                (for-each
-                 delete-file
-                 (append (find-files (string-append out "/lib/jvm/jre/lib")
-                                     "libjawt.so")
-                         (find-files (string-append out "/bin")
-                                     ".*(c\\+\\+|cpp|g\\+\\+|gcc.*)"))))
-              #t))))))))
+     `(#:test-target "check-target-libjava"
+       ,@(substitute-keyword-arguments `(#:modules ((guix build gnu-build-system)
+                                                    (guix build utils)
+                                                    (ice-9 regex)
+                                                    (srfi srfi-1)
+                                                    (srfi srfi-26))
+                                                   ,@(package-arguments gcc))
+           ((#:tests? _) #t)
+           ((#:configure-flags flags)
+            `(let ((ecj (assoc-ref %build-inputs "ecj-bootstrap")))
+               `("--enable-java-home"
+                 "--enable-gjdoc"
+                 ,(string-append "--with-ecj-jar=" ecj)
+                 "--enable-languages=java"
+                 ,@(remove (cut string-match "--enable-languages.*" <>)
+                           ,flags))))
+           ((#:phases phases)
+            `(modify-phases ,phases
+               (add-after
+                'unpack 'add-lib-output-to-rpath
+                (lambda _
+                  (substitute* "libjava/Makefile.in"
+                    (("libgcj_bc_dummy_LINK = .* -shared" line)
+                     (string-append line " -Wl,-rpath=$(libdir)"))
+                    (("libgcj(_bc)?_la_LDFLAGS =" ldflags _)
+                     (string-append ldflags " -Wl,-rpath=$(libdir)")))))
+               (add-after
+                'install 'install-javac-and-javap-wrappers
+                (lambda _
+                  (let* ((javac  (assoc-ref %build-inputs "javac.in"))
+                         (ecj    (assoc-ref %build-inputs "ecj-bootstrap"))
+                         (gcj    (assoc-ref %outputs "out"))
+                         (gcjbin (string-append gcj "/bin/"))
+                         (jvm    (string-append gcj "/lib/jvm/"))
+                         (target (string-append jvm "/bin/javac")))
+
+                    (symlink (string-append gcjbin "jcf-dump")
+                             (string-append jvm "/bin/javap"))
+
+                    (copy-file ecj (string-append gcj "/share/java/ecj.jar"))
+
+                    ;; Create javac wrapper from the template javac.in by
+                    ;; replacing the @VARIABLES@ with paths.
+                    (copy-file javac target)
+                    (patch-shebang target)
+                    (substitute* target
+                      (("@JAVA@")
+                       (string-append jvm "/bin/java"))
+                      (("@ECJ_JAR@")
+                       (string-append gcj "/share/java/ecj.jar"))
+                      (("@RT_JAR@")
+                       (string-append jvm "/jre/lib/rt.jar"))
+                      (("@TOOLS_JAR@")
+                       (string-append jvm "/lib/tools.jar")))
+                    (chmod target #o755)
+                    #t)))
+               (add-after
+                'install 'remove-broken-or-conflicting-files
+                (lambda _
+                  (let ((out (assoc-ref %outputs "out")))
+                    (for-each
+                     delete-file
+                     (append (find-files (string-append out "/lib/jvm/jre/lib")
+                                         "libjawt.so")
+                             (find-files (string-append out "/bin")
+                                         ".*(c\\+\\+|cpp|g\\+\\+|gcc.*)"))))
+                  #t)))))))))
 
 (define ecj-bootstrap
   (origin
-- 
2.1.0

Re: [PATCH] GCJ: run libjava tests.

[Ludovic Courtès]

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> Attached is a new patch that pulls the ‘#:test-target’ line out of the
> ‘substitute-keyword-arguments’ expression.  The indentation of many
> lines changes because of this.  ‘git diff -w’ shows that the actual
> change is pretty small.

[...]

> +     `(#:test-target "check-target-libjava"
> +       ,@(substitute-keyword-arguments `(#:modules ((guix build gnu-build-system)
> +                                                    (guix build utils)
> +                                                    (ice-9 regex)
> +                                                    (srfi srfi-1)
> +                                                    (srfi srfi-26))
> +                                                   ,@(package-arguments gcc))

#:test-target could even go after #:modules in the argument to
‘substitute-keyword-arguments’, for consistency.

Otherwise LGTM.

Thanks!

Ludo’.

Re: [PATCH] GCJ: run libjava tests.

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:
>
>> Attached is a new patch that pulls the ‘#:test-target’ line out of the
>> ‘substitute-keyword-arguments’ expression.  The indentation of many
>> lines changes because of this.  ‘git diff -w’ shows that the actual
>> change is pretty small.
>
> [...]
>
>> +     `(#:test-target "check-target-libjava"
>> +       ,@(substitute-keyword-arguments `(#:modules ((guix build gnu-build-system)
>> +                                                    (guix build utils)
>> +                                                    (ice-9 regex)
>> +                                                    (srfi srfi-1)
>> +                                                    (srfi srfi-26))
>> +                                                   ,@(package-arguments gcc))
>
> #:test-target could even go after #:modules in the argument to
> ‘substitute-keyword-arguments’, for consistency.

Ah, yes, that’s much prettier.  I’ll push with this change.  Thanks!

~~ Ricardo