Eric Bavier <ericbav...@openmailbox.org> writes:
> On 2016-04-22 09:16, Ricardo Wurmus wrote:
>> Hi Guix,
>>
>> here’s a first batch of patches for Java libraries. Many Java packages
>> depend on JUnit, so that’s where I started. “hamcrest-core” is just a
>> small part of the whole hamcrest library, but it’s enough to build
>> JUnit.
> [...]
>> + (add-before 'configure 'patch-build.xml
>> + (lambda _
>> + (substitute* "build.xml"
>> + (("unit-test, ") "")
>> + (("\\$\\{build.timestamp\\}") "guix"))
>
> Is using "guix" here as a timestamp safe? Will nothing read the
> manifest and expect an actualy timestamp? (I've become unfamiliar with
> most of java-land lately).
hamcrest-core is the only package I’ve encountered so far that adds a
timestamp to the manifest. Anything can be put inside manifests, so I
don’t think it matters much. We could replace the timestamp with the
commit date of the package expression, just so that we have an actual
date (and one that isn’t 30+ years in the past).
>> + ;; Java's "getMethods()" returns methods in an unpredictable
>> order.
>> + ;; To make the output of the generated code deterministic we
>> must
>> + ;; sort the array of methods.
>> + (add-after 'unpack 'make-method-order-deterministic
>
> Should we patch our java package instead? Perhaps that can be saved as
> a future exercise.
The official documentation states that the order of methods returned is
undefined. I wouldn’t like to force sorting on all users of the
reflection API. In this case it’s important as the method names are
written to a file in whatever order they are returned. In general the
order is of no importance.
>> + (snippet
>> + '(begin
>> + ;; Delete bundled jar archives.
>> + (delete-file-recursively "lib")
>> + #t))))
>
> Is this very common in java packages?
I’d say it’s somewhat more common than in packages written in other
languages, but I wouldn’t yet feel comfortable doing this by default for
all Java packages.
> Otherwise LGTM.
Thanks for the review!
~~ Ricardo
