From mboxrd@z Thu Jan 1 00:00:00 1970 From: Theodoros Foradis Subject: Re: [PATCH v2 1/1] gnu: Add plantuml. Date: Fri, 28 Oct 2016 14:19:11 +0300 Message-ID: <87d1iku3f4.fsf@openmailbox.org> References: <20161027101329.GD18185@macbook42.flashner.co.il> <20161027111853.4405-1-theodoros.for@openmailbox.org> <20161027111853.4405-2-theodoros.for@openmailbox.org> <87oa25g0ot.fsf@elephly.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:47202) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c05F9-0000tG-J7 for guix-devel@gnu.org; Fri, 28 Oct 2016 07:22:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c05F5-0005eA-73 for guix-devel@gnu.org; Fri, 28 Oct 2016 07:22:39 -0400 Received: from smtp29.openmailbox.org ([62.4.1.63]:60801 helo=smtp14.openmailbox.org) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c05F4-0005dq-T7 for guix-devel@gnu.org; Fri, 28 Oct 2016 07:22:35 -0400 In-reply-to: <87oa25g0ot.fsf@elephly.net> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: guix-devel 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") >> + (("> + (("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=E2=80=99t understand this. Do you only use =E2=80=9Cdefault-buil= d.xml=E2=80=9D to add an > install target? In the previous phase you use the included =E2=80=9Cbu= ild.xml=E2=80=9D. > I find this a little odd and think it would be clearer to just install > the files manually instead of using =E2=80=9Cdefault-build.xml=E2=80=9D= 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=E2=80=99s 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/plantu= ml"))) >> + (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 =E2=80=9C#!/bin/sh=E2=80=9D w= ork? > >> + (assoc-ref inputs "jre") "/bin/java -ja= r " >> + out "/share/java/plantuml.jar \"$@\"\n"= )))) >> + (chmod wrapper #o555))))))) > > Please note that all phases should end with #t or #f.=20 > Will fix. >> + (inputs >> + `(("graphviz" ,graphviz) >> + ("bash" ,bash) >> + ("jre" ,icedtea "out"))) > > =E2=80=9Cout=E2=80=9D 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 =E2=80=9Cusecase=E2=80=9D a word or should it be =E2=80=9Cuse case=E2= =80=9D? > >> +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 =E2=80=9C@code{salt}=E2=80=9D. > Ok. >> +graphical interfaces.") >> + (license license:gpl3+))) > > Could you please send an updated patch? > > ~~ Ricardo Sure, I will send an updated patch later. Regards, --=20 Theodoros Foradis