lynaghk / cljx

Write a portable codebase targeting Clojure/ClojureScript
Other
398 stars 34 forks source link

Leiningen 2.5.1 incompatibility of :plugins #66

Open lgrapenthin opened 9 years ago

lgrapenthin commented 9 years ago

The middleware is not wrapped with leiningen 2.5.1.

Idk whether its cljx or leiningens "fault". Just wanted to report.

cemerick commented 9 years ago

Confirmed. Example interaction using 2.5.1:

user=> #+clj 5

RuntimeException No reader function for tag +clj  clojure.lang.LispReader$CtorReader.readTagged (LispReader.java:1180)

Same version of repl-y and nREPL as in leiningen 2.5.0, which works as expected.

It would be very helpful if someone could bisect the changes to leiningen between 2.5.0 and 2.5.1.

lbradstreet commented 9 years ago

I've been meaning to experiment with some clojure bisect patterns / kung fu, and also use cljx, so I might give this a go. I'm also having a lot of trouble with uberjar'ing stuff so maybe I can give that a go at the same time.

sxren commented 9 years ago
leiningen (master=)$ git bisect start
leiningen (master|BISECTING=)$ git bisect good c78a8110a8f4db2e84c0daf09ed1145a8aa18c30
leiningen (master|BISECTING=)$ git bisect bad b630fa37b8b408c16ca86fdc5784e09dc889a612
Bisecting: 48 revisions left to test after this (roughly 6 steps)
[e7a33e7d1cc25c2cced4610cfbfd07261ec05ae3] Bump stencil to 0.3.5.
leiningen ((e7a33e7...)|BISECTING)$ git bisect bad
Bisecting: 21 revisions left to test after this (roughly 5 steps)
[bc69a1212db93182149abf6dd9ad0e05ce766a31] Merge pull request #1700 from circlespainter/circlespainter-manifest-sections
leiningen ((bc69a12...)|BISECTING)$ git bisect good
Bisecting: 10 revisions left to test after this (roughly 4 steps)
[96f24b30580e4cae5f9a4aaad84185eb18fc1a3f] Update NEWS and clarify uses of GPG.
leiningen ((96f24b3...)|BISECTING)$ git bisect bad
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[caa28570df4e87619e8b4a5f07280f242a9cbf43] Revert "Grant non-project profile access to with-profile"
leiningen ((caa2857...)|BISECTING)$ git bisect good
Bisecting: 2 revisions left to test after this (roughly 2 steps)
[e946ad112fb85c8df7cee854b7bff005ba4428ad] Allow multiple repl task profiles
leiningen ((e946ad1...)|BISECTING)$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 1 step)
[7baeb2bce512105f0cbc12744275fbbacdc995c2] Add profiles-with-matching-meta
leiningen ((7baeb2b...)|BISECTING)$ git bisect good
e946ad112fb85c8df7cee854b7bff005ba4428ad is the first bad commit
commit e946ad112fb85c8df7cee854b7bff005ba4428ad
Author: Hugo Duncan <hugo@hugoduncan.org>
Date:   Sun Sep 21 10:19:04 2014 -0400

    Allow multiple repl task profiles

    The :repl profile does not compose well when specified in both
    project.clj and profiles.clj files.

    Merge all profiles with ^:repl metadata in the repl task.  This allows
    multiple profiles to be applied for the repl task.

:040000 040000 5bedfcae2af2622a6da14816187913e4e8d79774 69348a9d1837d9991905f25f72cf28541b965cd2 M      leiningen-core
:040000 040000 6fecf8f050ce91ccb33a7136dc9fecffe675e057 ba16c14939f3462e12aec1182663665ac3efcb57 M      src
lbradstreet commented 9 years ago

Nice job

cemerick commented 9 years ago

My "solution" to #47 was to only add cljx as a dependency when the REPL was being run. This worked before because :included-profiles in project metadata included a :repl profile when the REPL was being run. This changed in 2.5.1 with technomancy/leiningen#1704. As far as I can tell, there is no way for plugin middleware to know that the REPL is being invoked, and (as discussed in #47 and elsewhere) there's no way for a plugin to affect merged profiles or to provide one of its own.

So, I think we're left with a couple options:

  1. Hook leiningen.repl/repl, and attempt to add the necessary dependency and REPL configuration there.
  2. Eliminate the middleware entirely, and fall back to providing a profile that all users must add to their :dev profile (or whatever) explicitly (@weavejester's original suggestion on #47).

The latter is probably "more correct" and probably less likely to break in the future, but is an unfortunate concession away from user ease. I'll accept a patch implementing either.