Subject: Re: [bug#28663] [PATCH 01/22] guix: ant-build-system: Add main-class support.

From: julien lepiller <[email-masked]>
Date: Tue, 03 Oct 2017 11:50:18 +0200

Le 2017-10-03 11:09, Ricardo Wurmus a écrit :
> [email-masked] writes:
> 
>> From: Julien Lepiller <[email-masked]>
>> 
>> * guix/build-system/ant.scm: New #:main-class argument
>> * guix/build/ant-build-system.scm: Generate a manifest file with
>> additional properties.
>> ---
> […]
>> +                 (target (@ (name "manifest"))
>> +                         (mkdir (@ (dir "${manifest.dir}")))
>> +                         (echo (@ (file "${manifest.file}")
>> +                                  (message ,(string-append
>> +                                              (if main-class
>> +                                                (string-append
>> +                                                  "Main-Class: " 
>> main-class
>> +                                                  
>> "${line.separator}")
>> +                                                "")
>> +                                              "")))))
>> +
>>                   (target (@ (name "compile"))
>>                           (mkdir (@ (dir "${classes.dir}")))
>>                           (javac (@ (includeantruntime "false")
>> @@ -97,10 +112,11 @@
>>                                                      (include (@ (name 
>> "**/*Test.java" )))))))
>> 
>>                   (target (@ (name "jar")
>> -                            (depends "compile"))
>> +                            (depends "compile, manifest"))
>>                           (mkdir (@ (dir "${jar.dir}")))
>>                           (exec (@ (executable "jar"))
>> -                               (arg (@ (line ,(string-append "-cf 
>> ${jar.dir}/" jar-name
>> +                               (arg (@ (line ,(string-append "-cmf 
>> ${manifest.file} "
>> +                                                             
>> "${jar.dir}/" jar-name
>>                                                               " -C 
>> ${classes.dir} ."))))))
> 
> This is good, thank you.  Could you please also document this in the
> manual in section “Build Systems”?

Is something like this OK? I will divide that paragraph in the two 
commits of course.

+The @code{#:main-class} parameter can be used with the minimal ant 
build
+file to specify the main class of the resulting jar.  This makes the 
jar
+file executable. The @code{#:test-include} parameter can be used to 
specify the
+list of junit tests to run. It defaults to @code{(list 
"**/*Test.java")}.
+The @code{#:test-exclude} can be used to disable some tests. It 
defaults to
+@code{(list "**/Abstract*.java")}, because abstract classes cannot be 
run as
+tests.

> 
> One question remains, though: will this affect the timestamps inside 
> the
> jar file?  If so, can we reset the timestamp to ensure reproducibility?

I think it's taken care of by the strip-jar-timestamps phase, but I will 
check.

I also have a side-question:

Some of the maven packages use three or four different generators, and 
some dependencies (such as java-eclipse-sisu-plexus) have a lot of 
runtime dependencies that are not discovered by java. The issue here is 
that including java-eclipse-sisu-plexus in the inputs field of a package 
is enough to build it, but not to run it. We could use propagated-inputs 
for that, as in the python world, but it doesn't feel right. This issue 
is also present for test dependencies where I would get a lot of runtime 
error because java cannot find a dependency of a dependency.

I see that there is a Class-Path entry that can be added to the 
MANIFEST.MF file, and that could be a way java could find the 
dependencies of a package.

The jar file is actually a zip archive. Would the grafter be able to 
graft it?

Do you know whether Class-Path is transitive?

Is there a way to programmatically filter inputs from native- and 
propagated- inputs? Or should we ask the packager to give a list of 
dependencies in a build argument, effectively duplicating some code?

I had this issue with the antlr tools too, but I found a solution: I 
added a shell wrapper that uses the correct classpath, but it only works 
because antlr is not a library.

Thank for your review and sorry for the unexpected questions.