all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Julien Lepiller <julien@lepiller.eu>
To: Frank Pursel <frank.pursel@gmail.com>, 60976@debbugs.gnu.org
Subject: [bug#60976] [PATCH v3 3/4] gnu: Add ditaa
Date: Sun, 26 Feb 2023 09:19:25 +0100	[thread overview]
Message-ID: <5B15B2D7-0E82-4D0C-A188-7086E45BDB8C@lepiller.eu> (raw)
In-Reply-To: <51d9aae0d048aebfccc46b5363271dc4f2bc64be.1677369969.git.frank.pursel@gmail.com>

Hi,

Thanks again for the patch series. Comments below. Could you adress these and the others, and send a v2 for this patch series?

I think "guix lint" would have caught most of these ;)

Well, first I'm not sure this requires a new file. Maybe java-graphics.scm would be a better place? If you add a new file anyway, you need to add it to the list in gnu/local.mk too and add it to the commit log.

Le 26 janvier 2023 18:01:17 GMT+01:00, Frank Pursel <frank.pursel@gmail.com> a écrit :
>---
> gnu/packages/ditaa.scm | 145 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 145 insertions(+)
> create mode 100644 gnu/packages/ditaa.scm
>
>diff --git a/gnu/packages/ditaa.scm b/gnu/packages/ditaa.scm
>new file mode 100644
>index 0000000000..e1f063e179
>--- /dev/null
>+++ b/gnu/packages/ditaa.scm
>@@ -0,0 +1,145 @@
>+;;; GNU Guix --- Functional package management for GNU
>+;;;
>+;;; 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/>.
>+;;;
>+;;; Copyright © 2023 Frank Pursel <frank.pursel@gmail.com>
>+;;;
>+
>+(define-module (gnu packages ditaa)
>+  #:use-module (gnu packages)
>+  #:use-module (gnu packages bash)
>+  #:use-module (gnu packages batik)
>+  #:use-module (gnu packages java)
>+  #:use-module (gnu packages java-xml)
>+  #:use-module (gnu packages xml)
>+  #:use-module (guix build-system ant)
>+  #:use-module (guix build utils)
>+  #:use-module (guix download)
>+  #:use-module (guix git-download)
>+  #:use-module ((guix licenses) #:prefix license:)
>+  #:use-module (guix packages)
>+  #:use-module (guix modules)
>+ )

This parenthesis feels lonely :)

>+
>+(define-public ditaa
>+  (package
>+    (name "ditaa")
>+    (version "0.11.0")
>+    (source (origin
>+              (method git-fetch)
>+              (uri (git-reference
>+                    (url "https://github.com/stathissideris/ditaa")
>+                    (commit (string-append "v" version))))
>+              (file-name (git-file-name name version))
>+              (sha256
>+               (base32
>+                "1y3g17wp1wvb05m56pp25avww2njpgh0gk0jsbsf25scj7hyyj26"))
>+              (modules '((guix build utils)))
>+              (snippet
>+               '(begin
>+                  (format #t "~%~a~%" "Finding and removing sourced jars.")

Nitpicking, but "embedded" might be a better term, wdyt?

>+                  (for-each
>+                   (lambda (jarf)
>+                     (delete-file jarf)
>+                     (format #t "Deleted: ~a~%" jarf))
>+                   (find-files "." "\\.jar$"))))))
>+    (build-system ant-build-system)
>+    (inputs (list bash-minimal))
>+    (native-inputs (list java-libbatik java-commons-cli java-w3c-svg
>+                         java-jericho-html `(,icedtea "jdk") java-junit))

You shouldn't need icedtea in native-inputs, it's already added by the ant-build-system. Maybe you need the default output instead? To get a "java" binary from a small package instead of depending on the whole jdk at runtime?

>+    (arguments
>+     `(#:build-target "release-all"
>+       #:phases
>+       (modify-phases %standard-phases
>+           ;; Ant's buildfile and build tree need to be modified
>+           ;; to provide access to the guix builds of the
>+           ;; batik and the java-commons-cli
>+           ;; jar files.  Also some of the source requires java7.
>+           (add-before 'build 'build-prep
>+             (lambda* (#:key inputs outputs #:allow-other-keys)
>+               (let* ((batik-jar (search-input-file inputs
>+                                                    "share/java/batik.jar"))
>+                      (commons-cli-jar (search-input-file inputs
>+                                                          "lib/m2/commons-cli/commons-cli/1.4/commons-cli-1.4.jar")))
>+                 (mkdir-p "lib")
>+                 (copy-file batik-jar "./lib/batik.jar")
>+                 (copy-file commons-cli-jar "./lib/commons-cli.jar"))
>+               (with-directory-excursion "build"
>+                 (substitute* "release.xml"
>+                   (("source=\"1.6\"")
>+                    "source=\"7\"")
>+                   (("<file name=\"commons-cli-1.2.jar\"/>")
>+                    (string-append "<file name=\"commons-cli.jar\"/>"
>+                                   "\n" "<file name=\"batik.jar\"/>"))))
>+               #t))

Again, no need to end with #t.

>+           (replace 'build
>+             (lambda* _
>+               (setenv "ANT_OPTS"
>+                       (string-append "-Dversion.string="
>+                                      ,version))
>+               (with-directory-excursion "build"
>+                 (invoke "ant" "-f" "release.xml" "release-jar")) #t))

Same here.

>+           (replace 'check
>+             (lambda* (#:key tests? #:allow-other-keys)
>+               (if tests?
>+                   (begin
>+                     (setenv "ANT_OPTS"
>+                             (string-append "-Dversion.string="
>+                                            ,version))
>+                     (mkdir-p "tests/testlib")
>+                     (with-directory-excursion "build"
>+                       (invoke "ant" "-f" "release.xml"
>+                               "generate-test-images")
>+                       (invoke "ant" "test"))) #f)))

And here.

>+           (replace 'install
>+             (lambda* (#:key inputs outputs #:allow-other-keys)
>+               (let* ((out (assoc-ref outputs "out"))
>+                      (lib (string-append out "/lib"))
>+                      (bin (string-append out "/bin"))
>+                      (bash (search-input-file inputs "bin/bash"))
>+                      (java (search-input-file inputs "bin/java"))
>+                      (jre (search-input-directory inputs "jre"))
>+                      (ditaa (string-append out "/bin/ditaa"))
>+                      (jar-name (string-append ,name
>+                                               ,version ".jar")))
>+                 (with-directory-excursion "releases"
>+                   (install-file jar-name lib))
>+                 (mkdir-p bin)
>+                 (with-output-to-file ditaa
>+                   (lambda _
>+                     (format #t "#!~a~%JAVA_HOME=~a ~a -jar ~a/~a $@~%"
>+                             bash jre java lib jar-name)))
>+                 (chmod ditaa #o755)) #t))
>+           (add-after 'install 'install-docs
>+             (lambda* (#:key outputs #:allow-other-keys)
>+               (let ((doc (string-append (assoc-ref outputs "out")
>+                                         "/share/doc/")))
>+                 (for-each (lambda (filen)
>+                             (install-file filen doc))
>+                           (find-files "." ".*README\\.md"))) #t)))))

No need for #t.

>+    (home-page "https://github.com/stathissideris/ditaa")
>+    (synopsis "Create graphics from ascii art")
>+    (description
>+     "ditaa is a small command-line utility 
>+that converts diagrams drawn using ascii art 
>+('drawings' that contain characters that resemble lines like | / - ), 
>+into proper bitmap graphics.")

Maybe @samp{|}, @samp{/}, …

>+    (license license:lgpl3)))
>+
>+
>+
>+
>+

And remove these additional empty lines.




  reply	other threads:[~2023-02-26  8:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1351eea169a28d30040826b14079fb49de3c98b8.1677369969.git.frank.pursel@gmail.com>
2023-01-26 16:53 ` [bug#60976] [PATCH v3 2/4] gnu: Add java-libbatik Frank Pursel
2023-02-26  7:59   ` Julien Lepiller
2023-01-26 17:01 ` [bug#60976] [PATCH v3 3/4] gnu: Add ditaa Frank Pursel
2023-02-26  8:19   ` Julien Lepiller [this message]
2023-02-23 21:00 ` [bug#60976] [PATCH v3 4/4] java-jericho-html: Using bzr-fetch and complete testing Frank Pursel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5B15B2D7-0E82-4D0C-A188-7086E45BDB8C@lepiller.eu \
    --to=julien@lepiller.eu \
    --cc=60976@debbugs.gnu.org \
    --cc=frank.pursel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.