Re: [PATCH]: Add Ant.

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

Andreas Enge writes:

> indeed, I was a bit confused since I remembered discussions about ANT_HOME,
> which led me to believe that it had made it into the distribution. Please
> do not hesitate to send out a ping if you do not get a reaction to a patch!

The priority of Ant dropped quickly when I discovered that I don't
actually have any use for it yet (even building IcedTea was only
motivated by my desire to build R), so I didn't mind to take a break
from anything Java-related :)

> On Mon, Feb 09, 2015 at 03:51:40PM +0100, Ricardo Wurmus wrote:
>> +(define-public ant-minimal
>
> If I understood correctly, you saw this essentially as a bootstrap-ant.
> So once you add the "real" one, you may wish to hide this one with
> "define" instead of "define-public".

Correct.  As I don't have any programmes in my queue that require Ant I
don't really know how to proceed.  We'd probably want some kind of
ant-build-system that unleashes ant and lets it churn through build.xml
or so --- I don't know if ant-minimal would be sufficient for that and
if we would want to only use it internally.

I'd prefer to leave it public for now because it's the only variant of
Ant we have at the moment.  I encourage anyone more versed in Java stuff
to continue where I left off and provide a more featured variant of Ant.

>> +     `(#:tests? #f ; Tests require hamcrest-core, which needs Ant to build.
>> +       #:phases
>> +       (alist-cons-after
>> +        'unpack 'remove-scripts
>> +        ;; Remove bat / cmd scripts for DOS as well as the antRun and runant
>> +        ;; wrappers.
>> +        (lambda _
>> +          (for-each delete-file
>> +                    (find-files "src/script"
>> +                                "(.*\\.(bat|cmd)|runant.*|antRun.*)")))
>
> Is this needed to prevent installation?

Yes.  The build instructions state that everything in src/script is to
be copied to the target directory.  Since we don't need them I don't see
harm in removing them before the build starts.

>
>> +           ;; disable tests to avoid dependecy on hamcrest-core
>
> Typo, missing "n" in "depende_n_cy".

Oh, will fix it.

>> +           (substitute* "build.xml"
>> +             (("depends=\"jars,test-jar\"") "depends=\"jars\""))
>
> How does this relate do disabling tests above? Are both needed?

This target depends on the "test-jar", which checks for JUnit to be
present.  If JUnit is present (it is because it comes bundled with ant)
the tests are run --- and then they fail because hamcrest-core is
nowhere to be found.  So the least invasive change I could think of was
to remove the dependency on the "test-jar" target.

>> +           (system* "bash" "bootstrap.sh"
>> +                    (string-append "-Ddist.dir="
>> +                                   (assoc-ref %outputs "out"))))
>
> Here you may wish to check the return value with "zero?" as this is
> the last command in the phase.

Okay.

> Then, please push! This is indeed very useful for me.

I'll update this on Monday and push then.  Thanks for the review!

~~ Ricardo