[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] gnu: Add plantuml.
From: |
Theodoros Foradis |
Subject: |
Re: [PATCH v2 1/1] gnu: Add plantuml. |
Date: |
Fri, 28 Oct 2016 14:19:11 +0300 |
User-agent: |
mu4e 0.9.17; emacs 25.1.1 |
Hello Ricardo,
> Hi Theodoros,
>
> thanks for the patch!
>
>> +(define-public plantuml
>> + (package
>> + (name "plantuml")
>> + (version "8048")
>> + (source (origin
>> + (method url-fetch)
>> + (uri (string-append
>> + "mirror://sourceforge/plantuml/plantuml-"
>> + version ".tar.gz"))
>> + (sha256
>> + (base32
>> + "1vipxd6p7isb1k1qqh4hrpfcj27hx1nll2yp0rfwpvps1w2d936i"))))
>> + (build-system ant-build-system)
>> + (arguments
>> + `(#:tests? #f ; no tests
>> + #:build-target "dist"
>> + #:phases
>> + (modify-phases %standard-phases
>> + (add-before 'build 'delete-extra-from-cp
>> + (lambda _
>> + (substitute* "build.xml"
>> + (("1.6") "1.7")
>> + (("<attribute name=\"Class-Path\"") "<!--")
>> + (("j2v8_macosx_x86_64-3.1.7.jar\" />") "-->"))
>> + #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.
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.
>
>> + (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.
>> + (let* ((out (assoc-ref outputs "out"))
>> + (wrapper (string-append out "/bin/plantuml")))
>> + (mkdir-p (string-append out "/bin"))
>> + (with-output-to-file wrapper
>> + (lambda _
>> + (display
>> + (string-append
>> + "#!" (assoc-ref inputs "bash")
>> "/bin/sh\n\n"
>
Works for me, so I'll change it.
> You might not need bash here. Would just “#!/bin/sh” work?
>
>> + (assoc-ref inputs "jre") "/bin/java -jar "
>> + out "/share/java/plantuml.jar
>> \"address@hidden"\n"))))
>> + (chmod wrapper #o555)))))))
>
> Please note that all phases should end with #t or #f.
>
Will fix.
>> + (inputs
>> + `(("graphviz" ,graphviz)
>> + ("bash" ,bash)
>> + ("jre" ,icedtea "out")))
>
> “out” is the default so you can just leave it off.
>
>> + (home-page "http://plantuml.com/")
>> + (synopsis "Draw UML diagrams from simple textual description")
>> + (description
>> + "Plantuml is a tool to generate sequence, usecase, class,
>> activity,
>
> Is “usecase” a word or should it be “use case”?
>
>> +component, state, deployment and object UML diagrams, using a simple and
>> +human readable text description. Contains salt, a tool that can
>> design simple
>
> Please use address@hidden
>
Ok.
>> +graphical interfaces.")
>> + (license license:gpl3+)))
>
> Could you please send an updated patch?
>
> ~~ Ricardo
Sure, I will send an updated patch later.
Regards,
--
Theodoros Foradis
Re: [PATCH] gnu: Add plantuml., Marius Bakke, 2016/10/27