Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Demonstrate an incompatibility with clojure.java.classpath #668

Closed
wants to merge 2 commits into from

Conversation

vemv
Copy link
Member

@vemv vemv commented Feb 29, 2020

Hi,

in a project of mine I found out that the presence of a recent cider-nrepl dependency would make c.j.classpath/classpath-directories return an empty seq.

Were you aware of that?

@vemv
Copy link
Member Author

vemv commented Feb 29, 2020

Repro'd successfully in JDK11 as expected. Let me know what you think.

Thanks!

@vemv vemv changed the title (Hopefully) create a failing test case Demonstrate an incompatibility with clojure.java.classpath Feb 29, 2020
@vemv
Copy link
Member Author

vemv commented Mar 1, 2020

As a workaround, consumers can invoke (classpath/system-classpath) and extract directories out of it, instead of invoking (classpath-directories) directly.

The former appears to be independent of classloader intricacies.

@bbatsov
Copy link
Member

bbatsov commented Mar 1, 2020

Yeah, it just parses an env variable (and it's the only thing that works on Java 9+).

@dpsutton
Copy link
Contributor

dpsutton commented Mar 1, 2020

I am using core.logic and adding the java.classpath dep there. cider-jack-in and classpath-directories is working for me. Are there more repro steps? Perhaps one of the lazily loaded middleware need to be loaded?

;; Connected to nREPL server - nrepl://localhost:39715
;; CIDER 0.25.0snapshot, nREPL 0.6.0
;; Clojure 1.10.1, Java 11.0.6
;;     Docs: (doc function-name)
;;           (find-doc part-of-name)
;;   Source: (source function-name)
;;  Javadoc: (javadoc java-object-or-class)
;;     Exit: <C-c C-q>
;;  Results: Stored in vars *1, *2, *3, an exception in *e;
;;  Startup: /usr/local/bin/clojure -Sdeps '{:deps {nrepl {:mvn/version "0.6.0"} cider/cider-nrepl {:mvn/version "0.24.0"}}}' -m nrepl.cmdline --middleware '["cider.nrepl/cider-middleware"]'
user> (require '[clojure.java.classpath :as cp])
nil
user> (cp/classpath-directories)
(#object[java.io.File 0x336cc394 "src/main/clojure"]
 #object[java.io.File 0x15fd6b0f "src/test/clojure"])

and also with lein in that project:

;; Connected to nREPL server - nrepl://localhost:36745
;; CIDER 0.25.0snapshot, nREPL 0.6.0
;; Clojure 1.10.1, Java 11.0.6
;;     Docs: (doc function-name)
;;           (find-doc part-of-name)
;;   Source: (source function-name)
;;  Javadoc: (javadoc java-object-or-class)
;;     Exit: <C-c C-q>
;;  Results: Stored in vars *1, *2, *3, an exception in *e;
;;  Startup: /home/dan/bin/lein update-in :dependencies conj \[nrepl\ \"0.6.0\"\] -- update-in :plugins conj \[cider/cider-nrepl\ \"0.24.0\"\] -- repl :headless :host localhost
user> (require '[clojure.java.classpath :as cp])
nil
user> (cp/classpath-directories)
(#object[java.io.File 0x38a82b21 "/home/dan/projects/clojure/core.logic/src/test/clojure"]
 #object[java.io.File 0x6f1913c9 "/home/dan/projects/clojure/core.logic/src/main/clojure"]
 #object[java.io.File 0x1307d144 "/home/dan/projects/clojure/core.logic/resources"]
 #object[java.io.File 0x49501a70 "/home/dan/projects/clojure/core.logic/target/classes"])

@dpsutton
Copy link
Contributor

dpsutton commented Mar 1, 2020

user> 
(doseq [[name p] @cider.nrepl/delayed-handlers]
  @p
  (println name ": nil? : " (nil? (seq (cp/classpath-directories)))))
handle-resource : nil? :  false
handle-profile : nil? :  false
handle-inspect : nil? :  false
handle-info : nil? :  false
handle-xref : nil? :  false
handle-version : nil? :  false
handle-trace : nil? :  false
handle-tracker : nil? :  false
handle-slurp : nil? :  false
handle-test : nil? :  false
handle-spec : nil? :  false
handle-apropos : nil? :  false
handle-format : nil? :  false
handle-undef : nil? :  false
handle-complete : nil? :  false
handle-clojuredocs : nil? :  false
handle-classpath : nil? :  false
handle-out : nil? :  false
handle-stacktrace : nil? :  false
handle-refresh : nil? :  false
handle-debug : nil? :  false
handle-content-type : nil? :  false
handle-ns : nil? :  false
handle-enlighten : nil? :  false
handle-macroexpand : nil? :  false
nil
user> (cp/classpath-directories)
(#object[java.io.File 0x11fd2e25 "/home/dan/projects/clojure/core.logic/src/test/clojure"]
 #object[java.io.File 0x584546ff "/home/dan/projects/clojure/core.logic/src/main/clojure"]
 #object[java.io.File 0x22347656 "/home/dan/projects/clojure/core.logic/resources"]
 #object[java.io.File 0x11754050 "/home/dan/projects/clojure/core.logic/target/classes"])
user> 

@vemv
Copy link
Member Author

vemv commented Apr 5, 2021

I rebased this PR against master (for checking if the issue would still be present), although the results are useless right now because master is also red

@bbatsov
Copy link
Member

bbatsov commented Apr 5, 2021

Yeah, the breakage is described here #678

It's something to do with piggieback, but I didn't have time to debug it in details.

vemv added 2 commits April 5, 2021 22:03
It was running pretty early, which might be the reason why this test wasn't failing, as expected.
@vemv
Copy link
Member Author

vemv commented Apr 5, 2021

Now that the master branch is green, I rebased this PR again.

Again it fails for the intended reason.

This would be a good issue to solve. It has an important impact because arbitrary third-party tools (notably tools.namespace (refresh)) can break in face of this - not always in an easily debuggable manner.

For example yesterday someone reported that edge case on Clojurians (Corretto + JDK11 + Ubuntu)

@vemv
Copy link
Member Author

vemv commented Apr 11, 2021

I recreated this more precisely in the actual repo originating the issue clojure-emacs/orchard#112

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants