From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Vong Subject: Re: [PATCH] gnu: Add clojure. Date: Tue, 26 Jul 2016 20:45:03 +0800 Message-ID: <87h9bc36wg.fsf@gmail.com> References: <87r3b7gc4d.fsf@gmail.com> <8737ndiln5.fsf@elephly.net> <87h9bs4amm.fsf@gmail.com> <87twfe7n6t.fsf@elephly.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:37739) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS1ja-0005M5-PB for guix-devel@gnu.org; Tue, 26 Jul 2016 08:45:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bS1jW-0001BD-F1 for guix-devel@gnu.org; Tue, 26 Jul 2016 08:45:17 -0400 Received: from mail-pa0-x243.google.com ([2607:f8b0:400e:c03::243]:36271) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS1jW-0001B0-1t for guix-devel@gnu.org; Tue, 26 Jul 2016 08:45:14 -0400 Received: by mail-pa0-x243.google.com with SMTP id ez1so13277551pab.3 for ; Tue, 26 Jul 2016 05:45:13 -0700 (PDT) In-Reply-To: <87twfe7n6t.fsf@elephly.net> (Ricardo Wurmus's message of "Sun, 24 Jul 2016 23:15:18 +0200") 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: Ricardo Wurmus Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Ricardo, Thanks for the review again, the package definition is now simplier. Ricardo Wurmus writes: > Hi Alex, > > Usually, we will split bundled libraries. For bundled =E2=80=9Cjar=E2=80= =9D archives > this is necessary in any case as a =E2=80=9Cjar=E2=80=9D file is a binary. > > If the libraries are bundled in source form (not as =E2=80=9Cjar=E2=80=9D= 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=E2=80=99t very easy as we lac= k a > bootstrapped set of core libraries, but you might be able to use the > =E2=80=9Cant-build-system=E2=80=9D to get closer to that goal. I also ha= ve a couple of > packages for Java core libraries that haven=E2=80=99t yet been pushed. I= f you > intend to work on this I can share my current work in progress. > Yes, the ASM library is included as source (not jar) and is one majar version behind upstream (4 vs 5). Also, this SO question says it is indeed a fork (https://stackoverflow.com/questions/21642115/how-does-the-clojure-com= piler-generates-jvm-bytecode). > 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=E2=80=99t need this. The =E2=80=9Cant-build-system=E2=80=9D allo= ws you to override the > name of the =E2=80=9Cjar=E2=80=9D archive. You can change the name to = =E2=80=9C(string-append > "clojure-" ,version ".jar")=E2=80=9D there without needing to override the > install phase. > Actually, build.xml does not provide any install target, so we have to roll our own. I should have made this clear. I've added a comment to clarify this point. >> + (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-na= me >> + 1 ".ht= ml") >> + "--verbose" >> + "--from" "markdown_gith= ub" >> + "--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 =E2=80=9Cpandoc=E2=80=9D to the inputs only for building the= documentation > please reconsider this decision. The closure of the =E2=80=9Cpandoc=E2= =80=9D package is > *massive* as it depends on countless Haskell packages. You would make > the =E2=80=9Cclojure=E2=80=9D package dependent on both Java (which is la= rge) and an > even larger set of packages consisting of GHC and numerous packages. > > Couldn=E2=80=99t you just install the markdown files as they are? > Sure, we could just install the markdown files as it. Though I am curious to know what do you mean the closure is massive. Isn't pandoc only needed at build-time, so the user doesn't need to download the ghc-pandoc substitute? Also, I realize I over look the `javadoc' target, which builds documentations in addition to those markdown file. So, I change the target to the following: ;;; The javadoc target is not built by default. (add-after 'build 'build-doc (lambda _ (system* "ant" "javadoc"))) >> + (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=E2=80= =99d much > prefer to have the original plain text files. > With the new build-doc target, we now only need to copy `doc/' to `share/doc/' `target/javadoc/' to `share/doc/javadoc/' and other top-level markdown/html files to `doc/', another simplification. > What do you think? > Finally, I want to ask do I need to sign my commit? I sign my commit and do a `magit-format-patch', but it seems the patch does not contain info of the signature. > ~~ Ricardo Thanks, Alex --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-fix-gcj-build.patch >From b4094a8acf9f7b41085f5c3aa0d548638ea8cb48 Mon Sep 17 00:00:00 2001 From: Alex Vong Date: Tue, 22 Dec 2015 01:06:33 +0800 Subject: [PATCH] fix gcj build --- build.xml | 29 ++++++++++++++++++++++------- src/jvm/clojure/lang/Compiler.java | 26 +++++++++++++------------- src/jvm/clojure/lang/Numbers.java | 35 ++++++++++++++++++++++++----------- src/jvm/clojure/lang/Ratio.java | 22 ++++++++++++++++++++-- 4 files changed, 79 insertions(+), 33 deletions(-) diff --git a/build.xml b/build.xml index cae30b2..2427d68 100644 --- a/build.xml +++ b/build.xml @@ -40,8 +40,10 @@ + includeAntRuntime="false" compiler="gcj" + debug="true" source="1.6" target="1.6"> + + + @@ -88,13 +92,18 @@ description="Compile the subset of tests that require compilation." unless="maven.test.skip"> - + + + Direct linking = ${directlinking} + jvm="gij" + fork="true"> + @@ -110,7 +119,10 @@ description="Run clojure tests without recompiling clojure." depends="compile-tests" unless="maven.test.skip"> - + + @@ -129,7 +141,10 @@ description="Run test generative tests without recompiling clojure." depends="compile-tests" unless="maven.test.skip"> - + + diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java index 6066825..d40099b 100644 --- a/src/jvm/clojure/lang/Compiler.java +++ b/src/jvm/clojure/lang/Compiler.java @@ -5347,19 +5347,19 @@ public static class FnMethod extends ObjMethod{ registerLocal(p, null, new MethodParamExpr(pc), true) : registerLocal(p, state == PSTATE.REST ? ISEQ : tagOf(p), null, true); argLocals = argLocals.cons(lb); - switch(state) - { - case REQ: - method.reqParms = method.reqParms.cons(lb); - break; - case REST: - method.restParm = lb; - state = PSTATE.DONE; - break; - - default: - throw Util.runtimeException("Unexpected parameter"); - } + if (state == PSTATE.REQ) + { + method.reqParms = method.reqParms.cons(lb); + } + else if (state == PSTATE.REST) + { + method.restParm = lb; + state = PSTATE.DONE; + } + else + { + throw Util.runtimeException("Unexpected parameter"); + } } } if(method.reqParms.count() > MAX_POSITIONAL_ARITY) diff --git a/src/jvm/clojure/lang/Numbers.java b/src/jvm/clojure/lang/Numbers.java index 623743b..9b828de 100644 --- a/src/jvm/clojure/lang/Numbers.java +++ b/src/jvm/clojure/lang/Numbers.java @@ -15,6 +15,7 @@ package clojure.lang; import java.math.BigInteger; import java.math.BigDecimal; import java.math.MathContext; +import java.math.RoundingMode; public class Numbers{ @@ -941,23 +942,35 @@ final static class BigDecimalOps extends OpsP{ public Number divide(Number x, Number y){ MathContext mc = (MathContext) MATH_CONTEXT.deref(); - return mc == null - ? toBigDecimal(x).divide(toBigDecimal(y)) - : toBigDecimal(x).divide(toBigDecimal(y), mc); + RoundingMode mode = mc.getRoundingMode(); + int vals[] = + { + BigDecimal.ROUND_UP, + BigDecimal.ROUND_DOWN, + BigDecimal.ROUND_CEILING, + BigDecimal.ROUND_FLOOR, + BigDecimal.ROUND_HALF_UP, + BigDecimal.ROUND_HALF_DOWN, + BigDecimal.ROUND_HALF_EVEN, + BigDecimal.ROUND_UNNECESSARY + }; + int i; + + for (i = 0; i < vals.length; ++i) + if (mode == RoundingMode.valueOf(vals[i])) + return mc == null + ? toBigDecimal(x).divide(toBigDecimal(y)) + : toBigDecimal(x).divide(toBigDecimal(y), vals[i]); + + throw new IllegalArgumentException("invalid rounding mode"); } public Number quotient(Number x, Number y){ - MathContext mc = (MathContext) MATH_CONTEXT.deref(); - return mc == null - ? toBigDecimal(x).divideToIntegralValue(toBigDecimal(y)) - : toBigDecimal(x).divideToIntegralValue(toBigDecimal(y), mc); + return toBigDecimal(x).divideToIntegralValue(toBigDecimal(y)); } public Number remainder(Number x, Number y){ - MathContext mc = (MathContext) MATH_CONTEXT.deref(); - return mc == null - ? toBigDecimal(x).remainder(toBigDecimal(y)) - : toBigDecimal(x).remainder(toBigDecimal(y), mc); + return toBigDecimal(x).remainder(toBigDecimal(y)); } public boolean equiv(Number x, Number y){ diff --git a/src/jvm/clojure/lang/Ratio.java b/src/jvm/clojure/lang/Ratio.java index 6c7a9bb..c7d2ff5 100644 --- a/src/jvm/clojure/lang/Ratio.java +++ b/src/jvm/clojure/lang/Ratio.java @@ -15,6 +15,7 @@ package clojure.lang; import java.math.BigInteger; import java.math.BigDecimal; import java.math.MathContext; +import java.math.RoundingMode; public class Ratio extends Number implements Comparable{ final public BigInteger numerator; @@ -63,8 +64,25 @@ public BigDecimal decimalValue(){ public BigDecimal decimalValue(MathContext mc){ BigDecimal numerator = new BigDecimal(this.numerator); BigDecimal denominator = new BigDecimal(this.denominator); - - return numerator.divide(denominator, mc); + RoundingMode mode = mc.getRoundingMode(); + int vals[] = + { + BigDecimal.ROUND_UP, + BigDecimal.ROUND_DOWN, + BigDecimal.ROUND_CEILING, + BigDecimal.ROUND_FLOOR, + BigDecimal.ROUND_HALF_UP, + BigDecimal.ROUND_HALF_DOWN, + BigDecimal.ROUND_HALF_EVEN, + BigDecimal.ROUND_UNNECESSARY + }; + int i; + + for (i = 0; i < vals.length; ++i) + if (mode == RoundingMode.valueOf(vals[i])) + return numerator.divide(denominator, vals[i]); + + throw new IllegalArgumentException("invalid rounding mode"); } public BigInteger bigIntegerValue(){ -- 2.6.3 --=-=-=--