Figure out what to do with our dynapath 1.0 compatibility problems

[bbatsov]

Long story short:

• We don't support the latest version of dynapath (1.0)
• The version we're using (0.2.5) is going to stop working with Java 10, as it relied on implementation details that are going to be changed

We need to come up with some solution for this soon, otherwise it's going to cause problems for many people.

More details here clojure-emacs/cider-nrepl#482

[tsulej]

Maybe it's related, I've just checked Java 10 and got such warning. nRepl and Cider seem to work.

nREPL server started on port 51299 on host 127.0.0.1 - nrepl://127.0.0.1:51299
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by mranderson048.orchard.v0v1v0_20180321v113210_28.dynapath.v0v2v5.dynapath.defaults$eval7485$fn__7486 to method java.net.URLClassLoader.addURL(java.net.URL)
WARNING: Please consider reporting this to the maintainers of mranderson048.orchard.v0v1v0_20180321v113210_28.dynapath.v0v2v5.dynapath.defaults$eval7485$fn__7486
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
REPL-y 0.3.7, nREPL 0.2.12
Clojure 1.9.0
Java HotSpot(TM) 64-Bit Server VM 10+46

[SevereOverfl0w]

Mirroring the boot approach seems difficult unless we control the start of the JVM very closely. I believe it needs to run prior to clojure.

[bbatsov]

Unfortunately I'm out of other ideas. I'm guessing something must be possible, but I simply don't have the time to do the research for it. The fact that dynapath decided to drop this functionality instead of fixing it leads me to believe probably there's no easy solution, though.

[SevereOverfl0w]

Inside of boot, I think we're okay, as we can use their compatible classloader.

What features does dynapath provide to us?

[bbatsov]

We use it only to add the JDK resources dynamically to the classpath, so afterwards things like source lookup and doc lookup will work for them. See

orchard/src/orchard/java.clj

Line 32 in 3934ea6

(defn add-classpath!

Unfortunately we're using exactly the damn method that the deprecation warnings are about. :-)

[tirkarthi]

A re-implementation of add-classpath (deprecated in Clojure core) that:
is a little more comprehensive than core's add-classpath — it should work as expected in more circumstances, and optionally uses Maven-resolver to add a Maven artifact (and all of its transitive dependencies) to your Clojure runtime's classpath dynamically.

I don't know how viable it is to use pomegranate but quick googling led me to this from a comment on clojuredocs

[SevereOverfl0w]

Pomegranate uses dynapath, https://github.com/cemerick/pomegranate/blob/master/README.md#urlclassloader-modifiability

…

On April 2, 2018 7:18:34 AM GMT+01:00, Xtreak ***@***.***> wrote: > A re-implementation of add-classpath (deprecated in Clojure core) that: > is a little more comprehensive than core's add-classpath — it should work as expected in more circumstances, and optionally uses Maven-resolver to add a Maven artifact (and all of its transitive dependencies) to your Clojure runtime's classpath dynamically. I don't know how viable it is to use pomegranate but quick googling led me to this from a [comment](https://clojuredocs.org/clojure.core/add-classpath#example-59d7d6d8e4b03026fe14ea57) on clojuredocs -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #20 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[SevereOverfl0w]

Pomegranate uses dynapath, it has the same problem. But seems to have some solution. https://github.com/cemerick/pomegranate/blob/master/README.md#urlclassloader-modifiability

…

On April 2, 2018 7:18:34 AM GMT+01:00, Xtreak ***@***.***> wrote: > A re-implementation of add-classpath (deprecated in Clojure core) that: > is a little more comprehensive than core's add-classpath — it should work as expected in more circumstances, and optionally uses Maven-resolver to add a Maven artifact (and all of its transitive dependencies) to your Clojure runtime's classpath dynamically. I don't know how viable it is to use pomegranate but quick googling led me to this from a [comment](https://clojuredocs.org/clojure.core/add-classpath#example-59d7d6d8e4b03026fe14ea57) on clojuredocs -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #20 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[bbatsov]

@SevereOverfl0w Not sure they have a real workaround.

If you are in a situation where you are using pomegranate but do not have a clojure.lang.DynamicClassLoader visible in your classloader hierarchy, you will need to explicitly enable the java.net.URLClassLoader support in dynapath (upon which pomegranate relies for such things).

This sounds like some flag to suppress the new behavior or something. And I don't see it in dynapath's readme.

[SevereOverfl0w]

Why are we not under clojure.lang.DynamicClassLoader when running?

[bbatsov]

No idea to be honest. I think @jeffvalk worked on this originally and we never had to change the code afterwards. Maybe he remembers something about this.

[jeffvalk]

For full Java source support, we need the JDK compiler classes in tools.jar, which is not on the classpath by default. It's also nice to have src.zip, so that the JDK class sources are available to the info middleware (jump-to, class/member docstrings, etc).

Dynapath was (at the time anyway...) the best option for loading these at runtime. And I believe dynapath does use a clojure.lang.DynamicClassLoader internally.

The use of dynapath wasn't very sophisticated; we just need to append those two resources to the classpath.

[bbatsov]

@jeffvalk Looking at the code it seems it's URLClassLoader, unless I'm misunderstanding something. But I guess we need to change this now (and this would also mean we don't need dynapath anymore).

[zcaudate]

I'm personally fine with the reflective warnings. are you sure java 10 is going to ban it completely?

if cider can modify the ns macro https://github.com/clojure/clojure/blob/010864f8ed828f8d261807b7345f1a539c5b20df/src/clj/clojure/core.clj#L5553

to provide a alternate classloader with the additional urls, then it might work.

[zcaudate]

Actually. I realised it's a slightly different problem (I was thinking in-editor evaluation), because ns doesn't have to be evalutated, it's a little easier to go around the dynapath thing.

Just write an alternate load method.

https://github.com/clojure/clojure/blob/010864f8ed828f8d261807b7345f1a539c5b20df/src/clj/clojure/core.clj#L5876-L5893