Open lread opened 2 years ago
Hey, thanks for reporting this. I'm not sure what the intention is of (-> [] .getClass .getClassLoader)
, maybe @skuro remembers.
If we simply want to scan for data_reader files then I see no reasing why we can't use the currentContextClassloader
. If for some reason we really want the system app classloader, then there's (ClassLoader/getSystemClassLoader)
for that.
So yes a PR would be very appreciated. Try replacing (-> [] (.getClass) (.getClassLoader))
with (.getCurrenctContextClassloader (Thread/currentThread))
. I see no reason why that wouldn't work. The only potential downside is that depending on which specific combo of tools you are using you could get quite a few different resutls from that call, but pretty much any compliant classloader should be good enough here. A similar change could also be made in lein-cloverage.
Thanks for the reply @plexus!
I can't say I understand the change nor why it doesn't work when :eval-in leiningen
is active.
But... maybe that does not matter too much.
This change was trying to make sure data readers were found and enabled.
I could check that this is the case with (.getCurrenctContextClassloader (Thread/currentThread))
regardless of lein :eval-in
values.
Hello @lread ,
My lein is of version Leiningen 2.9.8
, and I have tried several different java, including Java.net, Microsoft, Temurin, Zulu.
nil
returned by (-> [] (.getClass) (.getClassLoader))
It seems that the issues resolved in newer leiningen version.
Hi @humorless, thanks for taking a look! :heart:
It seems that the code
(-> [] (.getClass) (.getClassLoader))
is the same like here.
Yes, this is what I noticed too and reported in the original issue. But both @plexus and I did not exactly understand why.
I'm curious, Is this issue closed because it is fixed? Or because it is not considered an issue? Or because it is stale and there wasn't much interest in fixing?
Hi @humorless, thanks for taking a look! ❤️
It seems that the code
(-> [] (.getClass) (.getClassLoader))
is the same like here.
Yes, this is what I noticed too and reported in the original issue. But both @plexus and I did not exactly understand why.
I'm curious, Is this issue closed because it is fixed? Or because it is not considered an issue? Or because it is stale and there wasn't much interest in fixing?
Hello @lread,
I have replied this issue, and I previously thought we get to good enough answer. (It seems that I was wrong.) I reopen it now.
I made my judgement by two facts which I believe. (But, you know, sometimes, I make mistakes.)
(-> [] (.getClass) (.getClassLoader))
is a correct way to get ClassLoader. I have done certain search in the Internet. (-> [] (.getClass) (.getClassLoader)) => nil
in Leiningen repl.Therefore, I guessed probably the issue disappears in the newer version of Leiningen.
Cool, thanks for following up!
We may actually want to close this issue if we can't reproduce it in the latest version of Leningen. @lread, have you run into this issue recently? Do you happen to know which version of Leiningen you were using?
I suppose another reason to keep this open is the (-> [] (.getClass) (.getClassLoader))
code. Although, based on Laurence's investigation, it's not clear why this wouldn't work. I almost wonder if it's an issue in Leiningen (perhaps fixed)?
JavaDocs for Class.getClassLoader
say:
Every Class object contains a reference to the ClassLoader that defined it.
Okay, I did the obvious thing and tried lein repl
on my machine. It failed on Leiningen 2.9.1 on Ubuntu on Java 13 with Clojure 1.10.1. However, it succeeded on a project running Clojure 1.10.3 and when I changed it to run Clojure 1.10.2. I thought it was the Clojrue version but then I tried it in the same project with Clojure 1.10.1 and it worked there.
I did find this on Clojuredocs:
`clojure
;; WARNING: If the examples above don't seem to return the expected results, ;; be aware that tools like Leiningen[https://leiningen.org/] or ;; nRepl[https://github.com/nrepl/nrepl] alter the context class loader. ;; Please use a vanilla REPL like ;; clojure.deps[https://clojure.org/guides/getting_started] instead. `
I wonder if nREPL is really to blame. They did fix an issue with the context class loader in 0.6.0. I can't test earlier versions of nREPL since manually setting an older version of nREPL in the project.clj
file crashes.
Symptom
I was upgrading some pretty dated koacha deps in MrAnderson. It was using version
0.0-32
of the kaocha-cloverage plugin.After upgrading to the current kaocha versions, I encoutered NullPointerExceptions:
Exploration
I had a peek a the plugin code that throws and saw: https://github.com/lambdaisland/kaocha-cloverage/blob/14f63cd298fdfa2fdbb60108472b3a533a376aa1/src/kaocha/plugin/cloverage.clj#L142-L146
If I try this in code in a lein repl I can reproduce the
nil
:If I do this from a clojure REPL all is good:
I noticed that this change came from lein-cloverage and found a similar issue raised over there.
I then noticed MrAnderson is using
:eval-in :leiningen
in itsproject.clj
.Workaround
In MrAnderson's
project.clj
:kaocha
profile, I set:eval-in :sub-process
.Next Steps
Please feel free to close this issue if it is expected behaviour.
If it does represent an issue, I am happy to help in any way I can.