diff mbox

[v2,1/1] gnu: Add plantuml.

Message ID 20161027111853.4405-2-theodoros.for@openmailbox.org
State New
Headers show

Commit Message

Theodoros Foradis Oct. 27, 2016, 11:18 a.m. UTC
* gnu/packages/uml.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
---
 gnu/local.mk         |  1 +
 gnu/packages/uml.scm | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 gnu/packages/uml.scm

Comments

Ricardo Wurmus Oct. 27, 2016, 5:29 p.m. UTC | #1
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.

> +         (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.

> +                      (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"

You might not need bash here.  Would just “#!/bin/sh” work?

> +                              (assoc-ref inputs "jre") "/bin/java -jar "
> +                              out "/share/java/plantuml.jar \"$@\"\n"))))
> +                        (chmod wrapper #o555)))))))

Please note that all phases should end with #t or #f. 

> +    (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 “@code{salt}”.

> +graphical interfaces.")
> +    (license license:gpl3+)))

Could you please send an updated patch?

~~ Ricardo
Theodoros Foradis Oct. 28, 2016, 11:19 a.m. UTC | #2
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 \"$@\"\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 “@code{salt}”.
>

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 Oct. 28, 2016, 7:12 p.m. UTC | #3
>> +    (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”?
>

It is a word. I reply with the changes you suggested, with no change in the 
build procedure.

Theodoros Foradis (1):
      gnu: Add plantuml.

 gnu/local.mk         |  1 +
 gnu/packages/uml.scm | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+)
Ricardo Wurmus Oct. 29, 2016, 9:46 p.m. UTC | #4
Theodoros Foradis <theodoros.for@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.

>>> +                     (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.

>>> +                       #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.

(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?)

Should the “default-build.xml” procedure be changed to (conditionally)
add the “Main-Class” attribute?

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”.

>>
>>> +         (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
Theodoros Foradis Nov. 2, 2016, 12:07 a.m. UTC | #5
Ricardo Wurmus writes:

> Theodoros Foradis <theodoros.for@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,
diff mbox

Patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 0d400e9..595a5bd 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -357,6 +357,7 @@  GNU_SYSTEM_MODULES =				\
   %D%/packages/tmux.scm				\
   %D%/packages/tor.scm				\
   %D%/packages/tv.scm				\
+  %D%/packages/uml.scm				\
   %D%/packages/unrtf.scm			\
   %D%/packages/upnp.scm				\
   %D%/packages/uucp.scm				\
diff --git a/gnu/packages/uml.scm b/gnu/packages/uml.scm
new file mode 100644
index 0000000..7dbcd20
--- /dev/null
+++ b/gnu/packages/uml.scm
@@ -0,0 +1,86 @@ 
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Theodoros Foradis <theodoros.for@openmailbox.org>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu packages uml)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (guix packages)
+  #:use-module (guix download)
+  #:use-module (guix utils)
+  #:use-module (guix build-system ant)
+  #:use-module (gnu packages bash)
+  #:use-module (gnu packages graphviz)
+  #:use-module (gnu packages java))
+
+(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"))))
+         (add-after 'install 'make-wrapper
+                    (lambda* (#:key inputs outputs #:allow-other-keys)
+                      (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"
+                              (assoc-ref inputs "jre") "/bin/java -jar "
+                              out "/share/java/plantuml.jar \"$@\"\n"))))
+                        (chmod wrapper #o555)))))))
+    (inputs
+     `(("graphviz" ,graphviz)
+       ("bash" ,bash)
+       ("jre" ,icedtea "out")))
+    (home-page "http://plantuml.com/")
+    (synopsis "Draw UML diagrams from simple textual description")
+    (description
+     "Plantuml is a tool to generate sequence, usecase, class, activity,
+component, state, deployment and object UML diagrams, using a simple and
+human readable text description.  Contains salt, a tool that can design simple
+graphical interfaces.")
+    (license license:gpl3+)))