Re: [PATCH] Add IcedTea 7.

[ludo@gnu.org (Ludovic Courtès)]

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

> IcedTea 7 is served!

\o/

> The version numbers may be a little confusing.  We are using the IcedTea
> 2.5.5 framework and the OpenJDK 7 sources to build a "sanitised" version
> of the OpenJDK 7.  Since it's not exactly the OpenJDK that is built but
> rather a cleaned up, liberated version, with IcedTea patches applied the
> package name is "icedtea" rather than "openjdk".

Sure.

> The tests do not pass, unfortunately.  There are *some* failures that
> are seen in upstream builds, but our number of test failures is larger
> than that.  Some failures are probably related to the fact that I have
> not added Xfvb to the native inputs for some tests; others are very
> difficult for me to analyse and understand.

Fair enough.

> From 28581336e8bb9d0c77acdaf483addcbb242381d7 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
> Date: Mon, 20 Apr 2015 12:55:20 +0200
> Subject: [PATCH 1/2] gnu: icedtea6: Split test fixing phases.
>
> * gnu/packages/java.scm (icedtea6)[arguments]: Split phase 'fix-tests into
>   three phases 'fix-test-framework, 'fix-hotspot-tests, and 'fix-jdk-tests.

OK!

> From 2532230b43b4164fee363562534929e31f725428 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
> Date: Tue, 24 Mar 2015 12:09:29 +0100
> Subject: [PATCH 2/2] gnu: Add icedtea7.
>
> * gnu/packages/java.scm (icedtea7): New variable.

[...]

> +         #:locale "C"

Could you add a comment explaining why?

> +         ,@(substitute-keyword-arguments `(#:modules ((guix build gnu-build-system)
> +                                                      (guix build utils)
> +                                                      (srfi srfi-1)
> +                                                      (srfi srfi-26))
> +                                                     ,@(package-arguments icedtea6))

The ,@ should be aligned with #:modules (unfortunately Emacs fails to do
that by default.)

> +             ((#:phases phases)
> +              `(alist-replace

This is typically a case where ‘modify-phases’ would be more readable,
if you feel like changing it.

Otherwise OK.

Thank you for the hard work!

Ludo’.