Ricardo Wurmus writes:
> Theodoros Foradis <theodoros....@openmailbox.org> writes:
>
>>>> +(define-public plantuml
>
> […]
>
>>>> + (modify-phases %standard-phases
>>>> + (add-before 'build 'delete-extra-from-cp
>
> BTW: the phase name is a little hard to understand. We don’t mind
> slightly longer phase names if that improves readability.
>
I'll change that to delete-extra-from-classpath.
>>>> + (lambda _
>>>> + (substitute* "build.xml"
>>>> + (("1.6") "1.7")
>>>> + (("<attribute name=\"Class-Path\"") "<!--")
>>>> + (("j2v8_macosx_x86_64-3.1.7.jar\" />")
>>>> "-->"))
>
> Another thing I forgot: is this jar bundled? If so, it should be
> removed in a snippet.
>
The jar is not bundled. With those substitutions, I comment out all the
extra jars (not included anyway) from the classpath.
>>>> + #t))
>>>> + (add-before 'install 'gen-install
>>>> + (lambda* (#:key outputs #:allow-other-keys)
>>>> + (mkdir-p "build/jar")
>>>> + (system* "mv" "plantuml.jar" "build/jar")
>>>> + ((@@ (guix build ant-build-system) default-build.xml)
>>>> + "plantuml.jar"
>>>> + (string-append (assoc-ref outputs "out")
>>>> + "/share/java"))))
>>>
>>> I don’t understand this. Do you only use “default-build.xml” to add an
>>> install target? In the previous phase you use the included “build.xml”.
>>> I find this a little odd and think it would be clearer to just install
>>> the files manually instead of using “default-build.xml” here.
>>
Right, I should install manually instead. Generating the
default-build.xml is unneeded.
>> The build.xml that our ant-build-system generates, does not generate a
>> correct manifest attribute with the Main-Class, so the produced jar file
>> cannot be run. Instead of generating the required text manually, I use
>> the default build.xml for the build phase.
>>
>> The default build.xml does not include an install phase, so I generate
>> it after compilation, with our ant-build-system.
>>
>> Feedback is welcome, if there is a better way to do this, before I
>> reformat the patch.
>
> (I’m a little confused. When you write “default build.xml” you mean the
> included “build.xml”, not the one generated by the procedure
> “default-build.xml”, right?)
Right.
>
> Should the “default-build.xml” procedure be changed to (conditionally)
> add the “Main-Class” attribute?
>
I am not very knowledgeable about the ant-build-system, but I think it
would be a useful addition, as runnable jars need that "Main-Class"
attibute.
> In this case I think using “default-build.xml” just for the install
> phase is a little over the top. All it does is copy the jars to the
> target directory:
>
> (target (@ (name "install"))
> (copy (@ (todir "${dist.dir}"))
> (fileset (@ (dir "${jar.dir}"))
> (include (@ (name "**/*.jar"))))))
>
> That’s something you could do with Scheme directly:
>
> (replace 'install
> (lambda* (#:key outputs #:allow-other-keys)
> (for-each (lambda … install-file …)
> (find-files … "\\.jar$"))
> #t))
>
> I find that a lot clearer than accessing a private procedure from
> “ant-build-system”.
>
Thanks for the hint. I'll skip the "default-build.xml" and do it with
Scheme directly.
>>>
>>>> + (add-after 'install 'make-wrapper
>>>> + (lambda* (#:key inputs outputs #:allow-other-keys)
>>>
>>> Please check the indentation for all phases. That’s too far to the
>>> right.
>>>
>>
>> I will fix that. This is the default indentation emacs does with this,
>> so I forgot to fix it manually.
>
> Actually, we have a “.dir-locals.el” file that Emacs should respect.
> Using the settings in that file will tell Emacs to indent these things
> correctly. (Many of us here are using Emacs and we don’t fix
> indentation manually.)
>
> ~~ Ricardo
Thanks for letting me know. I thought I had been using that
".dir-locals.el", and that this indentation wasn't included, but I should
have made some mistake.
Regards,
--
Theodoros Foradis
