deftemplate barfs in the latest version of Clojure master branch

20 views
Skip to first unread message

Tom Faulhaber

unread,
Dec 1, 2009, 2:14:10 AM12/1/09
to Enlive
Hi!

I just synced to the latest master and deftemplate has been broken. It
appears that Miekel added code to zip to make it more strict and that
is causing probs in enlive.

Here's some sample code:

enl_test.clj:
----------------

(ns enl-test (:use net.cgrand.enlive-html))
(deftemplate page "layout.html" [])


layout.html:
----------------

<html xmlns="http://www.w3.org/1999/xhtml">
</html>


(Note that both files are trimmed down from much larger one's, so the
problem is not due to their minimal nature.)

When I load enl_test.clj into clojure, I get:

tom@renoir:~/src/clj/play/enl-test$ clojure enl_test.clj
Exception in thread "main" java.lang.reflect.InvocationTargetException
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke
(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke
(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at jline.ConsoleRunner.main(ConsoleRunner.java:71)
Caused by: java.lang.Exception: called children on a leaf node
(enl_test.clj:3)
at clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:2780)
at clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:2774)
at clojure.lang.Compiler$DefExpr.eval(Compiler.java:302)
at clojure.lang.Compiler.eval(Compiler.java:4647)
at clojure.lang.Compiler.load(Compiler.java:4972)
at clojure.lang.Compiler.loadFile(Compiler.java:4939)
at clojure.main$load_script__7359.invoke(main.clj:211)
at clojure.main$script_opt__7396.invoke(main.clj:263)
at clojure.main$main__7420.doInvoke(main.clj:338)
at clojure.lang.RestFn.invoke(RestFn.java:413)
at clojure.lang.Var.invoke(Var.java:359)
at clojure.lang.AFn.applyToHelper(AFn.java:173)
at clojure.lang.Var.applyTo(Var.java:476)
at clojure.main.main(main.java:37)
... 5 more
Caused by: java.lang.Exception: called children on a leaf node
at clojure.zip$children__7636.invoke(zip.clj:70)
at clojure.zip$down__7651.invoke(zip.clj:100)
at net.cgrand.insertion_point$insertion_point__3.invoke
(insertion_point.clj:27)
at net.cgrand.xml$parse__150.invoke(xml.clj:91)
at net.cgrand.enlive_html$load_html_resource__236$fn__238.invoke
(enlive_html.clj:35)
at net.cgrand.enlive_html$load_html_resource__236.invoke
(enlive_html.clj:34)
at net.cgrand.enlive_html$fn__260.invoke(enlive_html.clj:67)
at clojure.lang.MultiFn.invoke(MultiFn.java:161)
at net.cgrand.enlive_html$html_resource__248.invoke(enlive_html.clj:
50)
at enl_test$fn__9.invoke(enl_test.clj:3)
at clojure.lang.AFn.applyToHelper(AFn.java:171)
at clojure.lang.AFn.applyTo(AFn.java:164)
at clojure.lang.Compiler$InvokeExpr.eval(Compiler.java:2775)
... 18 more



git bisect confirms what I guessed from looking at the commit log: the
commit that broke things was:

commit 490e2f499ffdfaecc6e52805ded285f655b3020d
Author: Meikel Brandmeyer <m...@kotka.de>
Date: Tue Jun 23 23:20:22 2009 +0200

zip/children now checks whether the node is a branch

Signed-off-by: Rich Hickey <richh...@gmail.com>

I started to look at the code, but decided to put it into more expert
hands. I am running the latest enlive code, but I don't think it much
matters since before I began to see the bug I was about six months
behind.

Let me know what you find or if I can do any more field work,

Tom

Christophe Grand

unread,
Dec 1, 2009, 4:53:29 AM12/1/09
to enliv...@googlegroups.com
On Tue, Dec 1, 2009 at 8:14 AM, Tom Faulhaber <tomfau...@gmail.com> wrote:
I just synced to the latest master and deftemplate has been broken. It
appears that Miekel added code to zip to make it more strict and that
is causing probs in enlive.

Fixed, thanks for the report. This bug remained unnoticed because children was lenient.

I started to look at the code, but decided to put it into more expert
hands. I am running the latest enlive code, but I don't think it much
matters since before I began to see the bug I was about six months
behind.

Meikel's patch -- while authored 6 months ago -- only got applied last week, see http://github.com/richhickey/clojure/commit/490e2f499ffdfaecc6e52805ded285f655b3020d

Let me know what you find or if I can do any more field work,

Your report was very precise. Thanks again.

Christophe

Tom Faulhaber

unread,
Dec 3, 2009, 12:54:26 AM12/3/09
to Enlive
Thanks, Christophe. That did the trick and now autodoc is back on the
road.

In a DVCS it's hard to understand the exact meaning of a "commit
date." Interesting.

Hey, do you have any performance tips for working with enlive? I
haven't focused to much on speed yet, but I need to and I'd love to
get a running start.

Tom

Christophe Grand

unread,
Dec 3, 2009, 6:45:02 AM12/3/09
to enliv...@googlegroups.com
On Thu, Dec 3, 2009 at 6:54 AM, Tom Faulhaber <tomfau...@gmail.com> wrote:
Thanks, Christophe. That did the trick and now autodoc is back on the
road.

Are you evaluating enlive for autodoc?

Hey, do you have any performance tips for working with enlive? I
haven't focused to much on speed yet, but I need to and I'd love to
get a running start.

A common error is to read sources (html) at each rendering. If you have only deftemplate or defsnippet don't worry.

Enlive could be made faster (eg selection could fail fast) but since no one complains I have no incentive :-)
If it's slow for you, complain!

Christophe

Tom Faulhaber

unread,
Dec 3, 2009, 2:18:44 PM12/3/09
to Enlive
Indeed, enlive is an integral part of autodoc. Has been since the
google code -> github switch.

As I said, I haven't done any performance analysis yet, so I don't
even know the degree to which enlive is a bottleneck or other things
are. To this point, autodoc has run only on my system and it hasn't
really mattered if it takes 5 minutes to build the contrib docs, cause
it's running unattended (and no one's *that* cutting edge :-)).

But now I'm looking at folding it into leiningen and so folks will be
running it as part of their build cycle. This creates a different set
of expectations.

I'll keep you posted if I see any bottlenecks here. I've definitely
had a fun time building templates with enlive.

Thanks,

Tom

Christophe Grand

unread,
Dec 3, 2009, 3:10:44 PM12/3/09
to enliv...@googlegroups.com
On Thu, Dec 3, 2009 at 8:18 PM, Tom Faulhaber <tomfau...@gmail.com> wrote:
Indeed, enlive is an integral part of autodoc. Has been since the
google code -> github switch.

happy to hear that!

As I said, I haven't done any performance analysis yet, so I don't
even know the degree to which enlive is a bottleneck or other things
are. To this point, autodoc has run only on my system and it hasn't
really mattered if it takes 5 minutes to build the contrib docs, cause
it's running unattended (and no one's *that* cutting edge :-)).

Okay, I quickly browsed your code and I think deffragment is problematic: each call to a fragment parses the source of the fragment.
Why did you roll your own deffragment instead of defsnippet?
I'll look at it more closely tomorrow.

Christophe

Christophe Grand

unread,
Dec 4, 2009, 4:38:05 AM12/4/09
to enliv...@googlegroups.com
Hi Tom,

Here is a patch which removes repeated sources loading and roundtrips through xml serialization. On my box it saves a handful of seconds when run on Clojure (from 48 to 42s) but if I comment the call to make-index-json, the build time drops to 14s.

On the Enlive part I wonder if your template sources need to be that fragmented. Do you want me to look at it?

Christophe
--
Professional: http://cgrand.net/ (fr)
On Clojure: http://clj-me.cgrand.net/ (en)
enlive.patch

Christophe Grand

unread,
Dec 4, 2009, 4:48:18 AM12/4/09
to enliv...@googlegroups.com
I forgot to say that I removed the divs in which you wrapped your fragments. Are they needed?

Tom Faulhaber

unread,
Dec 6, 2009, 1:53:13 AM12/6/09
to Enlive
Hey Christophe,

Wow, thanks for all the tuning. I'll have to walk through each thing
in detail and make sure I understand it completely.

The reason I have so many fragments is purely my convenience - I think
of them as kind of small subroutines of XML that can be managed
independently. Also, someone using autodoc on a project may want to
change one, but not the others to get some particular tweak.

The reason I created deffragment instead of defsnippet is because I
thought defsnippet didn't do exactly what I wanted. It looks like
you're showing me how to use defsnippet to accomplish what I want. I
haven't tried to wrap my brain around exactly what's going on there.

Of course, if the JSON output is the real problem, then I need to talk
to the pretty print guy about speeding up his JSON writer. That's kind
of ironic, but should serve as a motivator to get me back to getting
pprint to run faster. :-)

Thanks. I'm sure I'll be hitting you up with more questions, etc. as I
dig in deeper.

Tom

On Dec 4, 1:48 am, Christophe Grand <christo...@cgrand.net> wrote:
> I forgot to say that I removed the divs in which you wrapped your fragments.
> Are they needed?
>
> On Fri, Dec 4, 2009 at 10:38 AM, Christophe Grand <christo...@cgrand.net>wrote:
>
>
>
> > Hi Tom,
>
> > Here is a patch which removes repeated sources loading and roundtrips
> > through xml serialization. On my box it saves a handful of seconds when run
> > on Clojure (from 48 to 42s) but if I comment the call to make-index-json,
> > the build time drops to 14s.
>
> > On the Enlive part I wonder if your template sources need to be that
> > fragmented. Do you want me to look at it?
>
> > Christophe
>
> > On Thu, Dec 3, 2009 at 9:10 PM, Christophe Grand <christo...@cgrand.net>wrote:

Tom Faulhaber

unread,
Dec 18, 2009, 2:03:26 AM12/18/09
to Enlive
Hi Christophe,

I finally got the time to look at and understand your suggestions and
now I see why you're recommending them.

The one issue that came up, that I hadn't thought through fully
before, is that I am defining which file to use as the template at run
time (so that individual projects can optionally have their own. This
is doesn't work with deftemplate/defsnippet which define things at
read time (at least not without some tricky gams about making sure
that reading happens at the right time).

To get around this, I created my own version of deftemplate and a new
version of deftemplate that do the htnl-resource evaluation at runtime
and memoize the result. Otherwise I basically applied the patch you
gave me. Thanks!

If you want to comment on any of it, I'd certainly appreciate it.

Thanks,

Tom

Christophe Grand

unread,
Dec 18, 2009, 3:40:53 AM12/18/09
to enliv...@googlegroups.com
Hi Tom,

On Fri, Dec 18, 2009 at 8:03 AM, Tom Faulhaber <tomfau...@gmail.com> wrote:
I finally got the time to look at and understand your suggestions and
now I see why you're recommending them.

Great!
 
The one issue that came up, that I hadn't thought through fully
before, is that I am defining which file to use as the template at run
time (so that individual projects can optionally have their own. This
is doesn't work with deftemplate/defsnippet which define things at
read time (at least not without some tricky gams about making sure
that reading happens at the right time).

How do you expect projects to declare custom templates? (I need to understand this to be able to expose alternative choices.)
 
Christophe

Tom Faulhaber

unread,
Dec 18, 2009, 12:47:53 PM12/18/09
to Enlive
> How do you expect projects to declare custom templates? (I need to
> understand this to be able to expose alternative choices.)

Right now, the program is invoked with a parameter directory argument
which includes a params.clj file to choose options and point to the
source and output directories, a templates/ subdirectory and
instructions (in the form of a build.xml) for doing any build on the
target project.

At run time, when we want to grab a template, say layout.html, we look
in the passed in param/templates directory to see if it has a
layout.html and, if so, we use that one. Otherwise, we take the
layout.html file from the autodoc source directory.

This will probably change a little as I make autodoc into more of a
library so it can be invoked from tools like leiningen. In particular,
I'll probably turn the default templates into resources in the jar
file so I can get everything packaged into a single jar.

Tom

Reply all
Reply to author
Forward
0 new messages