bhauman / rebel-readline

Terminal readline library for Clojure dialects
Eclipse Public License 1.0
684 stars 37 forks source link

Consider using MrAnderson to inline dependencies #189

Open benedekfazekas opened 5 years ago

benedekfazekas commented 5 years ago

Rebel-readline was on my list of things to check out and when I've got to it I realised it might benefit from using MrAnderson to inline some dependencies. That should mitigate the problem described in design priorities of the intro file:

keep dependency tree very simple and shallow. In order for a library like this to be adopted widely across the Clojure tooling system it needs to not bring extraneous dependencies. This means the core library should have as few dependencies as possible. When dependencies are required the transient dependencies again should be few to none.

and help fixing #15 and maybe #94, #162 too.

I played with both rebel-readline and rebel-readline-cljs a bit. Seems to me that inlining cljfmt, compliment and cljs-tooling for rebel-readline-cljs working OK. Although MrAnderson can prefix java classes as well I would not touch the jline stuff for now.

On downside the testing/building/releasing gets a bit more complex eg. the build need to make sure that everything is working fine with inlined deps.

Happy to put together a PR for this. I could add some Makefiles and the like as well to support the build -- I would very likely use https://github.com/clojure-emacs/cider-nrepl and https://github.com/clojure-emacs/refactor-nrepl Makefiles as templates.

Thanks for all your open source work btw, really appreciated.

dharrigan commented 5 years ago

For me, this would break my usage of rebel-readline.

I use Protobuf, but an older version (3.5.1). It's unfortunate that cljfmt brings in the closure compiler which also brings in Protobuf, causing a massive headache in terms of classpath incompatibilities.

clojure -A:prepl:rebel -Stree 

com.bhauman/rebel-readline 0.1.4
  org.jline/jline-terminal-jansi 3.5.1
    org.fusesource.jansi/jansi 1.16
  cljfmt/cljfmt 0.5.7
    rewrite-clj/rewrite-clj 0.5.2
    org.clojure/tools.reader 1.0.0-alpha4
    rewrite-cljs/rewrite-cljs 0.4.3
      org.clojure/clojurescript 1.7.228
        org.clojure/data.json 0.2.6
        org.clojure/google-closure-library 0.0-20151016-61277aea
          org.clojure/google-closure-library-third-party 0.0-20151016-61277aea
        org.mozilla/rhino 1.7R5
        com.google.javascript/closure-compiler v20151216

Inlining this would break 100% the use of rebel-readline for me. Thankfully, at the moment, I'm able to exclude the Closure compiler stuff like this:

  :rebel {:extra-deps {com.bhauman/rebel-readline {:mvn/version "RELEASE" :exclusions [org.clojure/google-closure-library rewrite-cljs/rewrite-cljs]}}

resulting in:

clojure -A:prepl:rebel -Stree 

org.clojure/clojure 1.10.1
  org.clojure/core.specs.alpha 0.2.44
  org.clojure/spec.alpha 0.2.176
eftest/eftest 0.5.8
  mvxcvi/puget 1.1.1
    fipp/fipp 0.6.17
      org.clojure/core.rrb-vector 0.0.14
    mvxcvi/arrangement 1.2.0
  io.aviso/pretty 0.1.34
  progrock/progrock 0.1.2
com.bhauman/rebel-readline 0.1.4
  org.jline/jline-terminal-jansi 3.5.1
    org.fusesource.jansi/jansi 1.16
  cljfmt/cljfmt 0.5.7
    rewrite-clj/rewrite-clj 0.5.2
    org.clojure/tools.reader 1.0.0-alpha4
  compliment/compliment 0.3.6
  org.jline/jline-terminal 3.5.1
  org.jline/jline-reader 3.5.1
dev-local/dev-local /home/david/.env/clojure/libs/dev.local
  org.clojure/tools.namespace 0.3.0
    org.clojure/java.classpath 0.2.3
  yogthos/config 1.1.4
    org.clojure/tools.logging 0.4.0
  mount/mount 0.1.16
org.clojure/test.check 0.10.0-RC1

So, for me, inlining would be a show-stopper in my use of rebel-readline (which I like quite a lot).

benedekfazekas commented 5 years ago

maybe you missing a piece of puzzle here (or maybe I do) but MrAnderson not only inlines it also changes ns names and their references (aka inlines and shadows dependencies). so you would not have a clash on the cp with MrAnderson inlined deps. hope this makes sense. more details in the MrAnderson readme or you can have a look on the innards of cider-nrepl artifact

dharrigan commented 5 years ago

Ah, I see, okay that's fine then :)