#44
- Created on2009-11-08 23:25
- Reported by
- StatusFixed
- PriorityHigh (2)
- Assigned to
- Milestone-
- Plan Level-
- Due Date-
AOT compilation of clojure-contrib.jar pre-sets logging implementation
Using ant to build clojure-contrib.jar causes clojure.contrib.logging to be AOT-compiled.
This means that the classpath at contrib build time affects which logging implementation is used, not the classpath in use when the code using c.c.l is compiled.
Using a standard pre-built clojure-contrib.jar one can never use anything but java.util.logging! That's bad for anyone who uses released snapshots, or who uses different log implementations on different projects, but doesn't want to maintain different builds of clojure-contrib.jar.
Furthermore, someone building clojure-contrib.jar with, say, log4j in their classpath will produce a jar in which c.c.l will fail when log4j is unavailable:
c.c.l should be adjusted so that the logging implementation is not discovered until runtime, or otherwise be removed from the AOT-compiled set.
This means that the classpath at contrib build time affects which logging implementation is used, not the classpath in use when the code using c.c.l is compiled.
Using a standard pre-built clojure-contrib.jar one can never use anything but java.util.logging! That's bad for anyone who uses released snapshots, or who uses different log implementations on different projects, but doesn't want to maintain different builds of clojure-contrib.jar.
Furthermore, someone building clojure-contrib.jar with, say, log4j in their classpath will produce a jar in which c.c.l will fail when log4j is unavailable:
user=> (use 'clojure.contrib.logging)
java.lang.NoClassDefFoundError: org/apache/log4j/Level (NO_SOURCE_FILE:0)c.c.l should be adjusted so that the logging implementation is not discovered until runtime, or otherwise be removed from the AOT-compiled set.
Leave a comment
This simple diff fixes things, albeit at the cost of runtime work.
diff --git a/build.xml b/build.xml
index e72d36e..fa1161c 100644
--- a/build.xml
+++ b/build.xml
@@ -118,6 +118,7 @@
<exclude name="**/repl_utils/javadoc.clj"/>
<exclude name="clojure/contrib/load_all.clj"/>
<exclude name="**/tests.clj"/>
+ <exclude name="**/logging.clj"/>
</fileset>
<chainedmapper>
<packagemapper from="${src}/*.clj" to="*" />
on 2009-11-10 11:08 *
By Chas Emerick
That's not really a solution, especially for those of us who can't (or don't want to) ship .clj files.
Would folks be amenable to having each logging system setup fn return a map of implementation fns, which can sit in a top-level delay? This would push selection of a logging system out of compile-time, regardless of AOT. Adds the cost of a deref to each logging call, but I can certainly live with that.
Would folks be amenable to having each logging system setup fn return a map of implementation fns, which can sit in a top-level delay? This would push selection of a logging system out of compile-time, regardless of AOT. Adds the cost of a deref to each logging call, but I can certainly live with that.
on 2009-11-11 01:02 *
By Chas Emerick
I don't think what I suggested this morning would require much. If Alex et al. are amenable to it, I'd be happy to produce a patch when I have some spare moments.
on 2009-12-19 22:35 *
By mattrevelle
Assigned to changed from ataggart to mattrevelle
Status changed from New to Accepted
I'm going to go ahead and write up a patch for Chas' proposition. Alex, if you already have something in mind then feel free to kick me off the ticket.
on 2009-12-20 21:36 *
By mattrevelle
Attachment 0001-Delayed-detection-of-logging-implementation.-See-44.patch added
Patch attached above. Feedback very welcome.
I must say, I'm a bit put off by the notion that we should have to discard perfectly useful aspects of Clojure to suit some peoples reaction to whether a JAR contains some .clj files vs. exclusively .class files. AOT should be an option for code, not a mandate that infects all code in contrib.
That said, I'm still going to go over the proposed changes.
That said, I'm still going to go over the proposed changes.
on 2010-01-04 17:18 *
By technomancy
I recommend turning off AOT until Alex gets a chance to go over this patch in more detail.
on 2010-01-04 18:48 *
By Steve Gilardi
if nobody objects, I plan to make Phil's suggested minimal workaround patch "change build.xml to turn off aot for this one file" today.
on 2010-01-04 19:08 *
By mattrevelle
Sounds good.
on 2010-01-05 15:25 *
By Steve Gilardi
Checked in to the master branch: change to build.xml so it doesn't compile logging.clj. (suggested by Richard Newman, Phil Hagelberg).
Do we need this interim change on the 1.1.x branch? Re: 1.1.x, Stuart mentioned "assume all library code is frozen on these branches". I don't plan to push this change to the 1.1.x branch without more discussion.
Do we need this interim change on the 1.1.x branch? Re: 1.1.x, Stuart mentioned "assume all library code is frozen on these branches". I don't plan to push this change to the 1.1.x branch without more discussion.
on 2010-01-07 19:18 *
By stuartsierra
Added to 1.1.x branch, included in 1.1.0-RC2
on 2010-01-07 19:18 *
By stuartsierra
That is, the temporary fix (not compiling logging.clj) is in 1.1.x and 1.1.0-RC2
on 2010-01-12 23:40 *
By timothypratley
The issue remains in clojure-contrib-1.1.0-master-20100111.160148-18.jar (do RC live elsewhere than http://build.clojure.org/snapshots ?)
Might it be possible to have the best of both worlds by having a dynamic binding of the macros? By this I mean the log macros can be rebound when the lib is used, so macro expansions after loading the lib will use the current environment. This would preclude AOT libs from using logging though, as the macro would be different in the lib compile environment and the user compile environment.
It seems the most robust solution would be to allow the choice of either fully dynamic log binding (might be useful for libs), or macro expanded logging... which would require two namespaces: clojure.contrib.logging and clojure.contrib.logging-dynamic or something like that.
Might it be possible to have the best of both worlds by having a dynamic binding of the macros? By this I mean the log macros can be rebound when the lib is used, so macro expansions after loading the lib will use the current environment. This would preclude AOT libs from using logging though, as the macro would be different in the lib compile environment and the user compile environment.
It seems the most robust solution would be to allow the choice of either fully dynamic log binding (might be useful for libs), or macro expanded logging... which would require two namespaces: clojure.contrib.logging and clojure.contrib.logging-dynamic or something like that.
file:asSL5Ea1qr34B3eJe5aVNr: patch to handle aotc issues
I've taken Matt's idea regarding delays and made a less invasive change.
The patch also contains some switches that can be set during compilation to obviate certain performance penalties introduced in order to handle AOTC.
See the note at the bottom of the ns docs for more info.
The patch also contains some switches that can be set during compilation to obviate certain performance penalties introduced in order to handle AOTC.
See the note at the bottom of the ns docs for more info.
file:b3wOBUbi0r37IeeJe5aVNr: Final (I hope) patch attempt
Applied in commit 655060b3f265026ef3b45b44f5ab22d8897b3034