Allow CIDER to load its own middleware via the sideloader

[plexus]

nREPL now has dynamic insertion of middleware, as well a sideloader for requesting resources from the client. CIDER has basic support for sideloading, so it would seem that all the pieces are there to be able to take a vanilla nREPL connection, and upgrade it to a fully CIDER-compliant connection.

After initial testing it seems this isn't quite there yet, there are small tweaks needed in CIDER, cider-nrepl, and nREPL, so this is a meta-issue to keep track of the overall goal and progress.

• [x] https://github.com/clojure-emacs/cider/pull/3038
• [x] https://github.com/clojure-emacs/cider/pull/3039
• [x] https://github.com/nrepl/nrepl/pull/243
• [x] https://github.com/clojure-emacs/cider-nrepl/pull/705
• [x] https://github.com/clojure-emacs/cider/pull/3041
• [x] https://github.com/nrepl/nrepl/pull/244
• [x] https://github.com/clojure-emacs/cider/pull/3044
• [ ] https://github.com/nrepl/nrepl/pull/246

Related

• https://github.com/clojure-emacs/cider/issues/3049

How to try it out

Start a vanilla nREPL server:

clojure -Sdeps '{:deps {nrepl/nrepl {:mvn/version "0.9.0-beta1"}}}' -M: -m nrepl.cmdline --port 1234

Connect to it from Emacs:

(setq
 ;; Make sure we can inspect the nREPL traffic
 nrepl-log-messages t
 ;; Prevent the nrepl pprint middleware from looking for a print function that
 ;; isn't (yet) there. Not necessary but prevents some noise.
 cider-print-fn nil
 ;; Use the cider-nrepl jar that I `make install'ed locally
 ;; we need the classloader-related fixes in the deferred middleware
 cider-dynload-cider-nrepl-version "0.27.0-SNAPSHOT"
 ;; Make sure caching doesn't bite us
 cider-jar-content-cache (make-hash-table :test #'equal))

;; Start the connection
(cider-nrepl-connect
 '( :project-dir "/home/arne/clj-projects/timezoner"
    :host "localhost"
    :port 1234
    :repl-init-function nil
    :session-name nil
    :repl-type clj))

;; Go to a clojure or cider repl buffer and do
;; M-x cider-upgrade-nrepl-connection

[plexus]

With the above changes to cider/cider-nrepl/nrepl I can load the middleware, and invoke a cider-version. Not everything is working yet (still trying things out and investigating) but this is a promising start.

[plexus]

Question regarding the sideloader design for @shen-tian. It seems like there's no way to distinguish between "file not found" and "empty file"? Maybe not the most common edge case but still seems significant...

[bbatsov]

While I don't remember the details as well as Shen, I'm assuming that's just something that came from the early sideloader prototype that was created by @cgrand. Perhaps the reasoning was that an empty file and a missing file are more or less the same as far as the sideloader is concerned. We can probably make the responses more granular if that'd help somehow.

[plexus]

With the last two PRs this actually seems to pretty much work as intended. It's a little slow (so might be worth considering caching responses, since currently every clj file gets served twice), and I didn't try every middleware (the deferred loading means there might still be errors lurking there), but all in all this is quite promising, although you still need a bit of low-level invocations to make it work. Maybe it's time to think about the ergonomics of this. Is the end goal to bundle cider-nrepl (and orchard) with cider? I think it would be preferable to download the jars from clojars, and either adapt our resource loading to deal with the zip files, or just unzip them in a cache location. So we'll need some logic that does that. and then I guess ultimately you would have something like M-x cider-upgrade-nrepl or even (setq cider-auto-upgrade-nrepl t) that detects if the middleware is there or not.

When running directly from source I also had to edit cider.nrepl.version because it references project.clj in a way that doesn't resolve, but I think when we look it up in the jar that shouldn't be an issue.

Looping in @mk at this point

[bbatsov]

Is the end goal to bundle cider-nrepl (and orchard) with cider? I think it would be preferable to download the jars from clojars, and either adapt our resource loading to deal with the zip files, or just unzip them in a cache location. So we'll need some logic that does that. and then I guess ultimately you would have something like M-x cider-upgrade-nrepl or even (setq cider-auto-upgrade-nrepl t) that detects if the middleware is there or not.

Yep, that's the end goal. Or at least one of them - I envision that every client will use sideloading for their deps (e.g. clj-refactor, sayid, etc). The ergonomics definitely have to be improved and that's the main reason I started to work on this and I dropped it back in the day - I couldn't figure out how to properly approach this. E.g. I was thinking of bundling CIDER's deps with the Emacs package and have it sideload them from some lib folder, but this required developing some mapping between the jars and the classes in them (if you wanted to to unzip them on the demand, that is). Also I didn't feel very comfortable committing big binary blobs to the git repo.

I'd be fine with fetching deps from Clojars, although this might slow down the first jack-in/connect. I was also thinking there some be some configurable list of folders, from which you can sideload resources. (useful for user extensions based on eval)

Anyways, I'm totally open to ideas how to make this optimal.

[plexus]

in any case I think it should be opt-in, the current jack-in approach works fine and has benefits over the sideloading (speed, having resources actually on the classpath).

[mk]

@plexus great to see this being this far along already, thank you! 👏👏👏

Regarding fetching deps I'm wondering if there's a way to delegate this to another tool. Could downloading the jars could be handled by shelling out to clojure maybe? Similar to what jack-in does but appending -P to not actually start anything but just download the deps?

[bbatsov]

You have to keep in mind that when someone is doing cider-connect we still want to inject cider-nrepl, but it's not clear whether the user has something like lein or clj available. Unix utilities like curl and wget are unlikely to available on Windows. Generally, it's best to stick to portable Elisp implementation, although I'm not sure if we can handle unzipping in this way. A quick search that's probably not possible. Perhaps it's fine to reach for Unix utils at first and figure out some Windows support down the road...

[mk]

You have to keep in mind that when someone is doing cider-connect we still want to inject cider-nrepl, but it's not clear whether the user has something like lein or clj available.

I was thinking that fetching the deps could work similarly to jack-in, where cider is also delegating this to another tool (clj, lein, shadow, boot…). Requiring e.g. clj to be available when opting into this feature via cider-auto-upgrade-nrepl might be reasonable? Down the road more tools can be supported. An easy to support alternative on windows to clj is the api-compatible bb clojure.

The upside I would see is that it would put the jars in the same places as when using jack-in.

[bbatsov]

Requiring e.g. clj to be available when opting into this feature via cider-auto-upgrade-nrepl might be reasonable?

In theory it is, but people like me, who are heavy Lein users normally don't install clj at all. If I wasn't working on CIDER I'd probably never have installed it. (I actually installed it a couple of days ago when I needed to debug some clj-specific issue) My point is simply that replacing one set of requirements (go add some middleware manually) with another set of requirements (go install some specific tool) is not the ideal outcome, at least for me. Sure, this would make it more convenient for us to implement the injection of deps, but it's going to result in subpar experience for some people. I think it'd be fine to rely on tooling for jack-in if we decide to leverage sideloading there over the current approach of just crafting a magic command that adds the deps. (because this already assumes some project using some build tool) While this works reasonable well for Clojure tools, it never worked with Gradle and Maven, and a truly portable solution based on sideloading would be a nice improvement.

Anyways, I've always been a believer in incremental progress, so I don't think we need to have everything super polished from the start. I'm not opposed to shelling out, I just think we should aim for utilities that are most likely to be around (e.g. generic Unix tools). In general I think we should aim for whatever requires the least amount of setup effort for the end users.

[plexus]

Here are some quick PoC experiments: https://gist.github.com/720798947e999a2142390a702a9ebe0b

• shelling out to a suitable tool for fetching the jar and returning a classpath

We have a lot of options here these days actually, some cross-platform (bb, deps) or platform-independent (deps.clj as standalone jar), and you could even throw in lein classpath as a fallback.

However these are all a bit unsatisfactory because they add stuff to classpath we don't want (src, clojure, spec.alpha), and lein classpath requires a project.clj even when you just provide the dependencies on the command line.

• alternatively: do a straightforward fetch-and-cache-for-later of the cider-nrepl.jar straight off clojars

Since this jar is fully self contained (dependencies are inlined) this seems like the pragmatic solution here. It uses portable emacs lisp, and adds a one-time cost of downloading 2.6M. For me the first invocation was 0.86s, and after that it's just a check to see if the file exists.

You could go a step further and check first if the file is perhaps already present in ~/.m2, so you don't have to download it. I don't think we should write to ~/.m2 since I'm not sure if other tools get confused if the jar is there but not the pom or sha files.

• Use archive-mode functions for dealing with the jar

Under the hood this uses elisp for listing the contents, but then shells out to extract a file. Still it seems this code is fairly portable, detecting the most appropriate zip tool and dealing with platform differences.

Reading the cider/nrepl.clj out of the jar takes something like 0.026s for me, which to be fair is already significant, but will probably still be dwarfed by the overhead of the rest of the sideloading (network, base64 en/decoding). We could cache the contents of the jar, that would shave off 70~80% of that.

So I think my next target should be

M-x cider-upgrade-nrepl-connection

which

• starts the sideloader
• downloads cider-nrepl if it's not there yet
• add cider-nrepl.jar to the sideloader paths
• inserts the middleware

This should give users something that works with minimal setup or extra dependencies, without affecting any existing functionality or workflows. And people can combine some of those pieces themselves if they want to change e.g. where the jar comes from.

I don't think the goal should be to make this behavior the default, we have a system that works well, and there are some benefits to adding the middleware directly to the server process. I would also hate to break existing workflows, I know how frustrating that can be. But we can make it easy to use and easy to find for people that can't or won't add the middleware directly to the server. Once this is working well for instance we can add a message to the current warning if cider-nrepl isn't found that people can upgrade their connection.

[bbatsov]

I love the proposed plan - it's definitely the one of least resistance. We can also add some defcustom like cider-auto-upgrade-nrepl-connection to do the connection upgrade automatically (nil by default) when needed.

[plexus]

I'm continuing to dogfood this, and have some more changes to nrepl incoming, including the caching we talked about.

What I'm seeing in real-world usage though is that this slows down loading of any code to a point that will be unacceptable for most people. The problem is that any resource that isn't found through the regular classloader will get tried via the sideloader. This doesn't sound too bad, until you consider that when you (require 'foo.bar) it will first look for

• resource foo/bar__init.class
• class foo.bar__init

So that's already 2 network round-trips for every clj file. If it's a cljc file it's 3. ClojureScript is better, but it will still do a needless round-trip when it's a cljc file.

So I think in practice we're going to have to either stop the sideloader again when we're done with it. Kind of problematic too with all the deferred loading. Or we need to have some kind of filter mechanism so we can tell it to only contact the client for certain resources.

[plexus]

After more internal discussion we are going to focus currently on testing the add-middleware functionality without the sideloader, by using lambdaisland.classpath to load cider-nrepl dynamically. Most of the challenges and issues left seem to be related to the sideloader.

I'm not really convinced that the sideloader is a good fit for this particular scenario, given that we have increasingly good options for dynamically loading jar dependencies, which are faster and more reliable. A more useful use case would be doing development over the network but being able to (require ...) files from your local development files. This could potentially be something I'll experiment with with witchcraft-plugin.

[bbatsov]

Whatever works best for you. For me the main appeal of the sideloader is for clients to be able to quickly customize their nREPL server without the need to use any external libraries, but obviously it's not the only way to inject some dependency dynamically. The idea to piggyback the add-middleware stuff on top of it came later - at first I was mostly thinking of using to deliver libraries that'd be used via eval from the clients. I'll have to check out lambdaisland.classpath, as I'm not familiar with the project.

So I think in practice we're going to have to either stop the sideloader again when we're done with it.

Yeah, that makes perfect sense, especially in the context of commands like cider-upgrade-connection.

Kind of problematic too with all the deferred loading.

What's the issue there?

Or we need to have some kind of filter mechanism so we can tell it to only contact the client for certain resources.

I've also been thinking about this - not really a filter, but more of a list of namespace prefixes that will be invoking the sideloader. I think this will mostly solve the first problem that you mentioned.

[shen-tian]

Question regarding the sideloader design for @shen-tian. It seems like there's no way to distinguish between "file not found" and "empty file"? Maybe not the most common edge case but still seems significant...

I don't remember any intentional design decision around this, so it's likely a carry-over from the unrepl design which Christophe authored. (Though I should have thought through this a bit more before adding it to nREPL). If we think this should change, let's do it before there's much use of this in the wild.

[andreyorst]

I've tried this feature, as per the ticket description with nrepl-0.9.0-beta5 and I do get exceptions during the addition of middleware. Wanted to ask if this still stands correct:

;; we need the classloader-related fixes in the deferred middleware

I've also tried with cider-dynload-cider-nrepl-version "0.27.2" but wasn't available to get working things like an interactive debugger.

Is this ticket is the right place to submit these kinds of errors, or should I create a separate one?

[bbatsov]

@andreyorst I think a separate ticket linking to this one would be best.