Subject: [bug#29061] Java patches

From: Julien Lepiller <[email-masked]>
Date: Mon, 30 Oct 2017 16:23:32 +0100

Le 30 octobre 2017 15:55:36 GMT+01:00, Leo Famulari <[email-masked]> a écrit :
>On Sun, Oct 29, 2017 at 09:54:38PM +0100, Julien Lepiller wrote:
>> On Sun, 29 Oct 2017 21:47:09 +0100
>> Julien Lepiller <[email-masked]> wrote:
>> 
>> > Hi, here are 22 new java patches that get us a bit closer to maven
>:)
>> > 
>> > 
>> > 
>> 
>> Somehow I can't use git send-email, so here are the patches.
>
>Thanks for all this!
>
>This is a cursory review. I trust that, in general, these packages are
>the latest upstream versions (or there is a code comment explaining why
>not), the licenses are free and correctly listed, and that everything
>seems to work.
>
>Beyond that, I think your work on Java packaging falls under the commit
>policy mentioned in HACKING, about "allowing individuals to commit
>directly on non-controversial changes on parts they’re familiar with":
>
>https://git.savannah.gnu.org/cgit/guix.git/tree/HACKING?id=faffd821f35dc34944226a992033df5a4be60cb1#n63
>
>> From 10c1801922abbc82e48360b22d5d8681449715b9 Mon Sep 17 00:00:00
>2001
>> From: Julien Lepiller <[email-masked]>
>> Date: Tue, 24 Oct 2017 22:05:04 +0200
>> Subject: [PATCH 01/22] gnu: Add java-bsh.
>> 
>> * gnu/packages/java.scm (java-bsh): New variable.
>
>> From f02f8c081e1627180e9f78393c3ab1fd8ad8b60b Mon Sep 17 00:00:00
>2001
>> From: Julien Lepiller <[email-masked]>
>> Date: Tue, 24 Oct 2017 22:05:04 +0200
>> Subject: [PATCH 02/22] gnu: Add java-bsh.
>> 
>> * gnu/packages/java.scm (java-bsh): New variable.
>
>These two patches can be squashed into one, right?
>
>> From 016af8aad804f488e530ae680855f175e8d8cfd1 Mon Sep 17 00:00:00
>2001
>> From: Julien Lepiller <[email-masked]>
>> Date: Tue, 24 Oct 2017 22:08:38 +0200
>> Subject: [PATCH 03/22] gnu: Add java-jmock.
>> 
>> * gnu/packages/java.scm (java-jmock): New variable.
>
>This patch looks a bit weird, as if there are two copies of the package
>or something.
>
>> From ff6b73b44424949796b18689e4c5aba156573b41 Mon Sep 17 00:00:00
>2001
>> From: Julien Lepiller <[email-masked]>
>> Date: Tue, 24 Oct 2017 22:30:48 +0200
>> Subject: [PATCH 10/22] gnu: Add java-mvel2.
>> 
>> * gnu/packages/java.scm (java-mvel2): New variable.
>
>This patch also adds java-jnacl. It should be added in its own patch,
>right?
>
>> From 3ea3acbd6cc141687107b1ad2a14bad7cefb9ebe Mon Sep 17 00:00:00
>2001
>> From: Julien Lepiller <[email-masked]>
>> Date: Tue, 24 Oct 2017 22:36:11 +0200
>> Subject: [PATCH 12/22] gnu: Add java-bouncycastle-bcprov.
>> 
>> * gnu/packages/java.scm (java-bouncycastle-bcprov): New variable.
>
>[...]
>
>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-before 'configure 'unzip-src
>> +           (lambda _
>> +             (mkdir-p "src")
>> +             (with-directory-excursion "src"
>> +               (zero? (system* "unzip" "../src.zip"))))))))
>
>Not a blocker for this patch series — I notice this custom unzip
>phase in a few packages. Should it be added to ant-build-system?
>
>> From d300f5ec70007f7554f14031b2af2def5ad3f439 Mon Sep 17 00:00:00
>2001
>> From: Julien Lepiller <[email-masked]>
>> Date: Wed, 25 Oct 2017 23:05:10 +0200
>> Subject: [PATCH 16/22] gnu: Add java-powermock-reflect.
>> 
>> * gnu/packages/java.scm (java-powermock-reflect): New variable.
>> ---
>>  gnu/local.mk                                       |   1 +
>>  gnu/packages/java.scm                              |  38 +++++
>>  .../patches/java-powermock-fix-java-files.patch    | 178
>+++++++++++++++++++++
>
>Please mention the new patch file and its addition to gnu/local.mk in
>the commit message.
>
>Also, do you think the patch should be submitted upstream?

I think I messed up these patches... I use git rebase a lot to fix things, but sometimes it does weird things. The result of applying these patches is working though.

I don't think the unzip phase has anything to do with ant-build-system. In this case, the "source" archive contains other archives (source, bin) and only one is interesting for our purpose, but its name is probably difficult to guess.

Re the patch, I'm familiar enough with java to produce it, but not to understand why it's needed. I will report compiling issues upstream.

Thanks for the review, I will fix the issues you mentionned and push when I have time.