all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Ricardo Wurmus <rekado@elephly.net>
To: Alex Vong <alexvong1995@gmail.com>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH] gnu: Add clojure.
Date: Sun, 24 Jul 2016 23:15:18 +0200	[thread overview]
Message-ID: <87twfe7n6t.fsf@elephly.net> (raw)
In-Reply-To: <87h9bs4amm.fsf@gmail.com>


Hi Alex,

> I see. So the general policy here is to be lazy and packaged by need :)
> This time only version 1.8 is added. Does this apply to splitting
> bundled libraries as well? I see clojure has some java libraries bundled
> such as ASM. Currently, I suppose no other packages depend on it.

Usually, we will split bundled libraries.  For bundled “jar” archives
this is necessary in any case as a “jar” file is a binary.

If the libraries are bundled in source form (not as “jar” archives) and
if they are closely tied to clojure (or if they were forked from
upstream libraries to better fit clojure), we could make an exception.

Packaging Java libraries for Guix still isn’t very easy as we lack a
bootstrapped set of core libraries, but you might be able to use the
“ant-build-system” to get closer to that goal.  I also have a couple of
packages for Java core libraries that haven’t yet been pushed.  If you
intend to work on this I can share my current work in progress.

>> I see that you added a very large patch to make this work.  In Guix we
>> usually avoid patching upstream software unless it’s absolutely
>> necessary.  Have you thought about submitting your patch upstream to
>> enable compilation with GCJ?  I think we should not add extensive
>> patches like that unless they are considered by the upstream developers.
>>
>> It’s also not so pretty that you have to call “gcj” and “g++” in a
>> somewhat complicated build phase “build-native”.  If you can get
>> upstream to accept your patches to build with GCJ maybe you can slip in
>> a patch to add a new Makefile target as well?  This would greatly
>> simplify the build phases.
>>
> I see clojure is written only in java and clojure. I will ask for
> upstream advice on this one.

Thank you.  Please let us know when a decision has been reached.

Here are some more comments about the patch you sent:

> +           (replace 'install
> +             (lambda* (#:key outputs #:allow-other-keys)
> +               (let ((java-dir (string-append (assoc-ref outputs "out")
> +                                              "/share/java/")))
> +                 ;; Do not install clojure.jar to avoid collisions.
> +                 (install-file (string-append "clojure-" ,version ".jar")
> +                               java-dir)
> +                 #t)))

You don’t need this.  The “ant-build-system” allows you to override the
name of the “jar” archive.  You can change the name to “(string-append
"clojure-" ,version ".jar")” there without needing to override the
install phase.

> +           (add-after 'build 'build-doc
> +             (lambda _
> +               (let* ((markdown-regex "(.*)\\.(md|markdown|txt)")
> +                      (gsub regexp-substitute/global)
> +                      (markdown->html (lambda (src-name)
> +                                        (zero? (system*
> +                                                "pandoc"
> +                                                "--output" (gsub #f
> +                                                                 markdown-regex
> +                                                                 src-name
> +                                                                 1 ".html")
> +                                                "--verbose"
> +                                                "--from" "markdown_github"
> +                                                "--to" "html"
> +                                                src-name)))))
> +                 (every markdown->html
> +                        (find-files "./" markdown-regex)))))

Why is this needed?  Is there no target for building the documentation?
If you added “pandoc” to the inputs only for building the documentation
please reconsider this decision.  The closure of the “pandoc” package is
*massive* as it depends on countless Haskell packages.  You would make
the “clojure” package dependent on both Java (which is large) and an
even larger set of packages consisting of GHC and numerous packages.

Couldn’t you just install the markdown files as they are?

> +           (add-after 'install 'install-doc
> +             (lambda* (#:key outputs #:allow-other-keys)
> +               (let ((doc-dir (string-append (assoc-ref outputs "out")
> +                                             "/share/doc/clojure-"
> +                                             ,version "/"))
> +                     (copy-file-to-dir (lambda (file dir)
> +                                         (copy-file file
> +                                                    (string-append dir
> +                                                                   file)))))
> +                 (for-each delete-file
> +                           (find-files "doc/clojure/"
> +                                       ".*\\.(md|markdown|txt)"))
> +                 (copy-recursively "doc/clojure/" doc-dir)
> +                 (for-each (cut copy-file-to-dir <> doc-dir)
> +                           (filter (cut string-match ".*\\.(html|txt)" <>)
> +                                   (scandir "./")))
> +                 #t))))))

Similar comments here.  Why delete the markdown documentation?  I’d much
prefer to have the original plain text files.

What do you think?

~~ Ricardo

  reply	other threads:[~2016-07-24 21:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-06 12:54 [PATCH] gnu: Add clojure Alex Vong
2016-07-13 15:49 ` Ricardo Wurmus
2016-07-14 13:22   ` Alex Vong
2016-07-24 21:15     ` Ricardo Wurmus [this message]
2016-07-26 12:45       ` Alex Vong
2016-07-26 20:00         ` Ricardo Wurmus
2016-07-27  6:47           ` Alex Vong
2016-08-15 12:20             ` Ricardo Wurmus
2016-08-16 13:28               ` Alex Vong
2016-08-16 18:45                 ` Ricardo Wurmus
2016-08-16 18:56                   ` Pjotr Prins
  -- strict thread matches above, loose matches on Subject: below --
2016-02-24 20:46 Federico Beffa
2016-02-26 11:56 ` Alex Vong
2016-02-27  8:27   ` Federico Beffa
2016-02-27 11:51   ` Ricardo Wurmus
2016-02-24  5:03 Alex Vong
2016-02-24 11:33 ` Ricardo Wurmus
2016-02-24 15:45   ` Alex Vong

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=87twfe7n6t.fsf@elephly.net \
    --to=rekado@elephly.net \
    --cc=alexvong1995@gmail.com \
    --cc=guix-devel@gnu.org \
    /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.