Re: [PATCH]: Add IcedTea 6

[Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>]

Ludovic Courtès writes:
>> * I'm not happy with replacing the unpack phase and I don't like to
>>   refer to the OpenJDK source tarball by its full name
>>   "openjdk-6-src-b34-20_jan_2015.tar.xz".  Is there a way around this?
>
> You can just call it whatever you want in the 2nd argument to
> ‘copy-file’, no?

Of course!  I just stopped thinking at some point, I suppose.  (Or it's
a remnant from my initial attempts with IcedTea7 which expects tarballs
of particular names to exist.)

>> From 13490591fe7ad774e8ec95626113138d828366fb Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
>> Date: Tue, 23 Dec 2014 12:32:25 +0100
>> Subject: [PATCH] gnu: Add IcedTea 6
>>
>> * gnu/packages/java.scm: New file.
>> * gnu-system.am (GNU_SYSTEM_MODULES): Add it.
>
> [...]
>
>> +     `(;; There are many failing tests.
>
> You can add that many are known to fail upstream.

Done.

>
>> +        'unpack
>> +        (lambda* (#:key source inputs #:allow-other-keys)
>> +          (let ((target (string-append ,name "-" ,version)))
>> +            (and (zero? (system* "tar" "xvf" source))
>> +                 (zero? (system* "tar" "xvjf" (assoc-ref inputs "ant-bootstrap")))
>> +                 (chdir target))
>> +            (mkdir "openjdk")
>> +            (with-directory-excursion "openjdk"
>> +              (copy-file (assoc-ref inputs "openjdk6-src")
>> +                         "openjdk-6-src-b34-20_jan_2015.tar.xz")
>> +              (system* "tar" "xvf" "openjdk-6-src-b34-20_jan_2015.tar.xz"))))
>
> (zero? (system* ...))

Done.  Now all three steps (unpacking IcedTea, unpacking ant and
unpacking the OpenJDK sources) are checked for non-zero.

>> +           (substitute* "Makefile.in"
>> +             ;; link against libgcj to avoid linker error
>> +             (("-o native-ecj")
>> +              "-lgcj -o native-ecj")
>> +             ;; do not leak information about the build host
>> +             (("DISTRIBUTION_ID=\"\\$\\(DIST_ID\\)\"")
>> +              "DISTRIBUTION_ID=\"\\\"guix\\\"\""))
>
> This could go to ‘snippet’ because it’s independent of the inputs used
> for the build, but do whatever seems most appropriate to you in terms of
> structure.

I made this a snippet.  Other things that would be good as snippets
(such as the patch to remove the timestamps) can only be done after the
build has started, because IcedTea patches the unpacked sources quite a
bit.

>> +              ;; JDK tests
>> +              (with-directory-excursion "openjdk/jdk/test/"
>> +                (substitute* "com/sun/jdi/JdbReadTwiceTest.sh"
>> +                  (("/bin/pwd") (which "pwd")))
>> +                (substitute* "com/sun/jdi/ShellScaffold.sh"
>> +                  (("/bin/kill") (which "kill")))
>> +                (substitute* "start-Xvfb.sh"
>> +                  (("/usr/bin/X11/Xvfb") (which "Xvfb"))
>> +                  (("/usr/bin/nohup")    (which "nohup")))
>> +                (substitute* "javax/security/auth/Subject/doAs/Test.sh"
>> +                  (("/bin/rm") (which "rm")))
>
> Maybe something coarser would work as well:
>
>   (substitute* (find-files "openjdk/jdk/test" ".")
>     (("/usr/bin/([[:graphic:]]+)" _ command)
>      (or (which command) command))
>     (("/bin/([[:graphic:]]+)" _ command)
>      (or (which command) command)))
>     
> WDYT?

I tried this but ran into a couple of problems.  Not all instances of
/bin are bad.  There are many places in the tests where /bin follows a
variable (and these are totally fine).  I also don't want to patch
shebangs twice.  The character class would have to be picked carefully
as well, because commands are sometimes quoted or inside braces etc.

Here's what I used before I gave up ([^[:space:]] instead of
[[:graphic:]] because the latter does not seem to exist and because it
would catch all sorts of unwanted characters if it did):

 (substitute* (find-files "." "\\.(sh|java)$")
   (("[^!](/usr)?/bin/([^[:space:]]+)" _ _ command)
    (or (which command) command)))

To come up with a reliable and maintainable regular expression is a
little messier than I anticipated.  I find it more precise to only fix
those instances that actually cause problems.

>> +            (alist-replace
>> +             'check
>> +             (lambda _
>> +               (and (zero? (system* "make" "check-hotspot"))
>> +                    (zero? (system* "make" "check-langtools"))
>> +                    (zero? (system* "make" "check-jdk"))))
>
> Maybe add a comment that the check- targets always return zero?

I rewrote this to instead check the generated test logs in test/ for
lines starting with "Error:" or "FAILED:".  To disable tests I wrapped
it in an (or #t ...) as the argument #:tests? #f does not disable a
custom check phase.

             'check
             (lambda _
               ;; The "make check-*" targets always return zero, so we need to
               ;; check for errors in the associated log files to determine
               ;; whether any tests have failed.
               (use-modules (ice-9 rdelim))
               (let* ((error-pattern (make-regexp "^(Error|FAILED):.*"))
                      (checker (lambda (port)
                                 (let loop ((_ #t))
                                  (let ((line (read-line port)))
                                    (cond
                                     ((eof-object? line) #t)
                                     ((regexp-exec error-pattern line) #f)
                                     (else (loop #t)))))))
                      (run-test (lambda (test)
                                  (system* "make" test)
                                  (call-with-input-file
                                      (string-append "test/" test ".log")
                                    checker))))
                 (or #t ; skip tests
                     (and (run-test "check-hotspot")
                          (run-test "check-langtools")
                          (run-test "check-jdk")))))

>> +    (synopsis "A harness to build OpenJDK using Free Software build tools")
>
> “Java development kit”?

Okay.

>> +    (description
>> +     "The IcedTea project provides a harness to build the source code from
>> +http://openjdk.java.net using Free Software build tools along with additional
>> +features such as a Free Software plugin and web start implementation and an
>> +LLVM-based JIT.")
>
> No good idea here.  :-)

How about this:

   "The OpenJDK built with the IcedTea build harness."

It doesn't sound great but it actually says what it is.

~~ Ricardo