Upon connect, namespaces are reloaded in wrong order

[wagjo]

When building AST, refactor-nrepl (re)loads namespaces, albeit in order that may cause defprotocols to be re-evaluated after deftypes using that protocols, that are defined in some other namespaces.

When that happens, calling protocol methods on those deftypes results in No implementation of method errors.

[expez]

Are you sure this is it? Lately I've noticed the No implementation of method errors while working on a single file too. This would happen while making changes and reloading the file, with C-c C-k in CIDER.

In any event, this is tools.analyzer.jvm doing its thing. The only thing I can think of to fix this is to call cider-refresh for you, to reload all code in dependency order, but I'm not sure you'd want that.

[wagjo]

Upon cider-connect namespaces are reloaded, as I can see reflection warnings being print again in console.

I must manually reload namespace C-c C-k where my deftypes are in order to make it work. When I do not use refactor-nrepl, I don't get that error, so it's probably not me breaking my code :)

cider-refresh does not help, nor it seems to reload any namespaces.

I'm using boot, spacemacs 0.104.x with 0.10.0snapshot cider and 2.0.0-snapshot refector-nrepl.

Does tools.analyzer process all namespaces in project directory or just those in opened buffers?

[expez]

Does tools.analyzer process all namespaces in project directory or just those in opened buffers

When the repl is started we eagerly build a cache of ASTs using tools.analyzer, for every file in the project. It takes some time to build these ASTs so we do this to make any future requests a bit snappier. This is what's happening when you're experiencing that your 'namespaces are reloaded'.

It just occurred to me that cider-refresh won't do anything, because it only acts on files that have been modified. If you give it a prefix it will unconditionally reload every file.

[wagjo]

How about running tools.analyzer in a separate classloader? You can use something like boot's pods for that https://github.com/boot-clj/boot/wiki/Pods

[benedekfazekas]

we've been there and failed. see https://github.com/clojure-emacs/refactor-nrepl/tree/classloader-isolation for reference. we went to source inlining instead (to at least solve the dependency clash problems) which now cider-nrepl uses too

does not mean we would not jubilate over a PR towards that direction ;)

[xeqi]

I ran into this and made a reproduction case at https://github.com/xeqi/nrepl-error. In it the user namespace is analyzed/loaded first by build-ast. Then the protocol namespace is analyzed/loaded. This makes the reify in the user namespace point to the old version of the protocol.

I believe this error could be prevented by analyzing the files in namespace dependency order. It looks like clojure.tools.namespace.dir, clojure.tools.namespace.tracker, and some code based on clojure.tools.namespace.reload could be used to determine the file order. Would you be interested in a PR that attempts this?

[expez]

Would you be interested in a PR that attempts this?

Yes. I've been meaning to fix this myself--I just haven't had the time. There's already a "tracker" ns here, although it doesn't do quite what you want.

[enaeher]

As a data point, I've run into what I believe is the same problem with Clara where rules (which refer to record types) sometimes fail to match any facts (expressed as records) even when they should. I haven't figured out the exact mechanism of action yet but I've confirmed that it only happens after connecting with refactor-nrepl loaded, and it stops happening after a cider-refresh.

[benedekfazekas]

@wagjo @enaeher pls retest your cases with latest 2.0.0-SNAPSHOT @xeqi I run your example project with the latest, it returned nil as expected.

[enaeher]

@benedekfazekas with the latest snapshot I no longer experience this problem--thank you!

[benedekfazekas]

no worries. if others in this issue experience the same this might be a close ;)

[wagjo]

I can confirm that namespaces are now reloaded in correct order. Note that classloader isolation would still be the best solution, as global vars like stuart sierra's components contain implementations of protocols that refactor-nrepl incidentally redefines.

[benedekfazekas]

@wagjo thx. time to close this I guess.

classloader isolation

noted. PR is still very welcome ;)