Closed danielcompton closed 8 years ago
Thanks for your work on this. I haven't had much time but will have a look this weekend.
I realise now that this patch can't be merged until https://github.com/technomancy/leiningen/pull/2032 is closed. Leiningen makes an über jar which bundles all of its dependencies and it comes first on the Classpath so there's no way for us to get access to tools.reader until it's removed from Leiningen.
Hey this is now fixed in Leiningen 2.6.0 (unreleased), and I can verify that this works for Clojure and ClojureScript projects now.
Hey, have you had a chance to look at this change?
Hi. When I tried it out on hiccup, I got an error:
$ cd hiccup
$ lein ns-dep-graph :clj
Exception while parsing ns decl: #error {
:cause Wrong number of args (2) passed to: reader/read
:via
[{:type clojure.lang.ArityException
:message Wrong number of args (2) passed to: reader/read
:at [clojure.lang.AFn throwArity AFn.java 429]}]
:trace
[[clojure.lang.AFn throwArity AFn.java 429]
[clojure.lang.AFn invoke AFn.java 36]
[clojure.tools.namespace.parse$read_ns_decl invoke parse.cljc 55]
[leiningen.ns_dep_graph$read_file_ns_decl invoke ns_dep_graph.clj 21]
[leiningen.ns_dep_graph$read_file_ns_decl invoke ns_dep_graph.clj 18]
[clojure.core$comp$fn__4495 invoke core.clj 2438]
[clojure.core$map$fn__4553 invoke core.clj 2624]
[clojure.lang.LazySeq sval LazySeq.java 40]
[clojure.lang.LazySeq seq LazySeq.java 49]
[clojure.lang.RT seq RT.java 507]
[clojure.core$seq__4128 invoke core.clj 137]
[clojure.core$reduce1 invoke core.clj 903]
[clojure.core$set invoke core.clj 3960]
[leiningen.ns_dep_graph$ns_dep_graph doInvoke ns_dep_graph.clj 37]
[clojure.lang.RestFn invoke RestFn.java 423]
[clojure.lang.Var invoke Var.java 383]
[clojure.lang.AFn applyToHelper AFn.java 156]
[clojure.lang.Var applyTo Var.java 700]
[clojure.core$apply invoke core.clj 632]
[leiningen.core.main$partial_task$fn__6030 doInvoke main.clj 261]
[clojure.lang.RestFn applyTo RestFn.java 139]
[clojure.lang.AFunction$1 doInvoke AFunction.java 29]
[clojure.lang.RestFn applyTo RestFn.java 137]
[clojure.core$apply invoke core.clj 632]
[leiningen.core.main$apply_task invoke main.clj 311]
[leiningen.core.main$resolve_and_apply invoke main.clj 317]
[leiningen.core.main$_main$fn__6096 invoke main.clj 390]
[leiningen.core.main$_main doInvoke main.clj 383]
[clojure.lang.RestFn invoke RestFn.java 421]
[clojure.lang.Var invoke Var.java 383]
[clojure.lang.AFn applyToHelper AFn.java 156]
[clojure.lang.Var applyTo Var.java 700]
[clojure.core$apply invoke core.clj 630]
[clojure.main$main_opt invoke main.clj 316]
[clojure.main$main doInvoke main.clj 421]
[clojure.lang.RestFn invoke RestFn.java 457]
[clojure.lang.Var invoke Var.java 394]
[clojure.lang.AFn applyToHelper AFn.java 165]
[clojure.lang.Var applyTo Var.java 700]
[clojure.main main main.java 37]]}
(...)
Does this work for you? Am I doing something wrong?
It will only work on Lein 2.6.0 and up, and I don't think it's been released yet. You can build it from source, or you could wait until it's released, no biggy.
OK, thanks. I will keep an eye on Leiningen and try merging when 2.6.0 is released.
lein 2.6.1 is out now, it should be fine to merge?
My pull request was merged, so it is not okay to merge as is. Conflicts will have to be resolved. When that is done, it also needs to be re-built so we can use it on projects.
I've redone this patch, and the patch applies now.
@hilverd can you cut a new release with this commit in?
Done. Sorry for the delay :disappointed:.
This required an upgrade to tools.namespace too. The new version of tools.namespace requires tools.reader 0.10.0, but when I was testing this there was another version of tools.reader coming from somewhere which was conflicting. I've opened a ticket against Leiningen to upgrade their clj-http dependency as I suspect it could be this one.
I've vendored read-file-ns-decl so that it prints out exceptions that get thrown. I'm not 100% sure if this is the right option, but I don't like how it swallows all exceptions.
Fixes #1