adzerk-oss / boot-cljs-repl

Boot task providing a REPL for ClojureScript development.
Eclipse Public License 1.0
72 stars 28 forks source link

New weasel #18

Closed piranha closed 9 years ago

piranha commented 9 years ago

Now it will work with modern CLJS.

martinklepsch commented 9 years ago

I tried this branch in a fresh tenzing project (lein new tenzing repl-test +reagent) and couldn't get it to work.

java.io.FileNotFoundException: Could not locate cljs/repl__init.class or cljs/repl.clj on classpath: , compiling:(cemerick/piggieback.clj:1:1)

When running with -vv I get a list of REPL related dependencies:

Loaded REPL dependencies: ([org.clojure/tools.nrepl "0.2.8" :exclusions [[org.clojure/clojure]]] [weasel "0.7.0"] [com.cemerick/piggieback "0.2.1"] [org.clojure/clojurescript "0.
0-3269"])

Using [adzerk/boot-cljs "0.0-2814-4" :scope "test"], maybe that is too old, so I'll try another one. But since I pinned Clojurescript itself to 3269 I wouldn't expect this to change anything.

martinklepsch commented 9 years ago

No luck with [adzerk/boot-cljs "0.0-3269-2" :scope "test"] either.

tomjakubowski commented 9 years ago

I believe newish ClojureScript versions depend on the Clojure 1.7.0 betas/RCs. Weasel itself lists a dependency of 1.7.0-RC1, and I had similar errors if I recall in the example project, before I bumped the Clojure version up from the 1.6 series.

martinklepsch commented 9 years ago

@tomjakubowski thanks, you're right of course! Fixed that. Now I'm getting another exception though:

clojure.lang.Compiler$CompilerException: java.lang.IllegalStateException: var: #'clojure.tools.nrepl.middleware.interruptible-eval/queue-eval is not public, compiling:(cemerick/p
iggieback.clj:240:3)

This is caused by [org.clojure/tools.nrepl "0.2.8"] being loaded. In [org.clojure/tools.nrepl "0.2.10"] this var is public.

piranha commented 9 years ago

I guess I'll just add those two as dependencies then?

martinklepsch commented 9 years ago

tools.nrepl should come in transitively via piggieback. Not sure why I'm getting an old version in my case but I'd guess that could be totally a problem on my side.

martinklepsch commented 9 years ago

@piranha also I think that all of these deps are loaded in a pod so I'm not sure if it's the correct thing to do just adding them as transitive dependencies.

piranha commented 9 years ago

Oh, that may be. Ok, not sure what to do, maybe I should change readme saying 'check for those'?

martinklepsch commented 9 years ago

I think the task should check if the pod's dependencies are sufficient, there's some related stuff in place already but nothing that "fails" in case they're not. Maybe that's for another day though(?)

Deraen commented 9 years ago

Boot-cljs has some magic to try to update Clojure dependency, but it's not working very well. I'm probably going to remove the magic and add a warning which tells the user to add the proper dependency to the project.

Deraen commented 9 years ago

If I'm not mistaken, the repl stuff which is causing the error is not running in pod.

Problem might be that repl dependencies are appended to dependencies vector which means that if any previous dependency in the vector has transitive dependency to tools.nrepl, that version is used instead.

Prepending our dependencies should force our versions to be used. Not sure if will cause other problems.

Deraen commented 9 years ago

Hmh, doesn't seem to help. It's probably impossible to update the dependency version if is is already loaded, e.g. by any library in projects dependencies.

Making org.clojure/tools.nrepl "0.2.10" a dependency (no provided scope) of boot-cljs would fix this. I'm not sure if there are alternatives?

I would also think about moving piggieback and weasel to normal dependencies, instead of adding them using set-env.

piranha commented 9 years ago

Like that?

Deraen commented 9 years ago

Yeah. Does @micha have comments about this?

Now I'm getting another error:

clojure.lang.ArityException: Wrong number of args (2) passed to: reader/read
Deraen commented 9 years ago

E.g. ring-core depends on old tools.reader.

Deraen commented 9 years ago

I got this working by adding tools.reader 0.9.2 dependency to my project.

piranha commented 9 years ago

Do you think I should add it as a dependency as well?

Deraen commented 9 years ago

I don't know. It's ClojureScript compiler which depends on that, not piggieback or weasel.

Deraen commented 9 years ago

Adding ClojureScript dependency to the project also fixes the error.

piranha commented 9 years ago

Like, instead of adding it in a pod? That actually make sense, boot-cljs-repl won't work without ClojureScript anyway...

Deraen commented 9 years ago

I'm not sure that it would be good idea to mess with dependencies (outside of pods) in plugin. Perhaps we should instead tell the user to add the desired ClojureScript dependency to the project.

Might work well with boot-cljs in future too, I'm thinking of making it not follow cljs versions and user would select the cljs version by adding the dependency to the project.

piranha commented 9 years ago

Ok, I'll switch this to asserts tomorrow then.

piranha commented 9 years ago

Done!

piranha commented 9 years ago

Ok, so this stuff works, but I'm not exactly happy with version comparison - it's a bit fragile (like, it will not parse "-SNAPSHOT" stuff). Any thoughts about depending on https://github.com/grimradical/clj-semver here?

martinklepsch commented 9 years ago

Funky:

WARNING: org.clojure/tools.nrepl version null is older than required 0.2.10

How do you specify the dependency on tools.nrepl? I have this:

(reset! boot.repl/*default-dependencies*
        '[[cider/cider-nrepl "0.9.0-SNAPSHOT"]
          [org.clojure/tools.nrepl "0.2.10" :exclusions [[org.clojure/clojure]]]])

but it's not found in the list of dependencies that is retrieved in warn-deps-versions using

(map :dep (pod/resolve-dependencies (b/get-env)))
martinklepsch commented 9 years ago

If I specify the dependency directly in my build.boot ([org.clojure/tools.nrepl "0.2.10" :exclusions [[org.clojure/clojure]]] ) everything is working fine, even all the stuff that usually broke things. Amazing 😍

EDIT Maybe the check could print a different warning if a dependency could not be found at all. Also — should boot-cljs-repl add the nrepl dependency if it's missing? Where should it come from by default? I assume that *default-dependencies* was the wrong place.