clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.54k stars 646 forks source link

Unable to jack-in in some cases when `cider-enrich-classpath` is `t` #3581

Closed rrudakov closed 10 months ago

rrudakov commented 10 months ago

Steps to reproduce the problem

  1. Create a simple project with the following files:

deps.edn

{:paths [:clj-paths]

 :deps {}

 :aliases {:clj-paths ["src"]

           :test {:extra-paths ["test"]
                  :extra-deps  {io.github.cognitect-labs/test-runner {:git/tag "v0.5.1" :git/sha "dfb30dd"}}
                  :main-opts   ["-m" "cognitect.test-runner"]
                  :exec-fn     cognitect.test-runner.api/test}}}

src/core.clj (content is not relevant)

(ns core)

(defn foo []
  (println "Hello"))

test/core_test.clj (content is not relevant)

(ns core-test
  (:require
   [clojure.test :refer [deftest is]]))

(deftest foo-test
  (is (= 4 (+ 2 2))))
  1. Try to jack-in.

Expected behavior

I can successfully jack-in without :test alias or with :test alas

Actual behavior

  1. If I jack-in with default aliases (using normal cider-jack-in command), the REPL is running successfully, but my namespaces are not included to the classpath. The following command is generated by clojure.sh (src path is missing):

    /usr/bin/clojure -Sforce -Srepro -J-XX:-OmitStackTraceInFastThrow -J-Dclojure.main.report=stderr -Scp /home/rrudakov/.m2/repository/org/clojure/clojure/1.11.1/clojure-1.11.1.jar:/home/rrudakov/.m2/repository/org/clojure/core.specs.alpha/0.2.62/core.specs.alpha-0.2.62.jar:/home/rrudakov/.m2/repository/org/clojure/spec.alpha/0.3.218/spec.alpha-0.3.218.jar:/home/rrudakov/.m2/repository/cider/cider-nrepl/0.43.3/cider-nrepl-0.43.3.jar:/home/rrudakov/.m2/repository/nrepl/nrepl/1.0.0/nrepl-1.0.0.jar:/home/rrudakov/.m2/repository/refactor-nrepl/refactor-nrepl/3.9.0/refactor-nrepl-3.9.0.jar:/home/rrudakov/.m2/repository/cider/orchard/0.20.0/orchard-0.20.0.jar:/home/rrudakov/.m2/repository/mx/cider/logjam/0.1.1/logjam-0.1.1.jar:/usr/lib/jvm/java-17-temurin/lib/src.zip:/home/rrudakov/.cache/mx.cider/enrich-classpath/1709/1341589140/1730288118.jar:/home/rrudakov/.cache/mx.cider/unzipped-jdk-sources/1709 -J--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED -m nrepl.cmdline --middleware [refactor-nrepl.middleware/wrap-refactor,cider.nrepl/cider-middleware]
  2. If I jack-in with custom :test alias (either by modifying cider-clojure-cli-aliases to ":test" or by using prefix argument and edit start server command replacing -M:cider/nrepl with -M:test:cider/nrepl at the end) REPL won't start. Instead I see in the *Messages* buffer the output of running tests:

    
    error in process sentinel: Could not start nREPL server: WARNING: Implicit use of clojure.main with options is deprecated, use -M

Running tests in #{"test"}

Testing core-test

Ran 1 tests containing 1 assertions. 0 failures, 0 errors. ("finished")


The following command is generated by `clojure.sh` (looks like `cider/nrepl` alias is missing or wrong `main-opts` are picked up):

```text
/usr/bin/clojure -Sforce -Srepro -J-XX:-OmitStackTraceInFastThrow -J-Dclojure.main.report=stderr -Scp test:/home/rrudakov/.m2/repository/org/clojure/clojure/1.11.1/clojure-1.11.1.jar:/home/rrudakov/.m2/repository/org/clojure/core.specs.alpha/0.2.62/core.specs.alpha-0.2.62.jar:/home/rrudakov/.m2/repository/org/clojure/java.classpath/1.0.0/java.classpath-1.0.0.jar:/home/rrudakov/.m2/repository/org/clojure/spec.alpha/0.3.218/spec.alpha-0.3.218.jar:/home/rrudakov/.m2/repository/org/clojure/tools.cli/1.0.206/tools.cli-1.0.206.jar:/home/rrudakov/.m2/repository/org/clojure/tools.namespace/1.3.0/tools.namespace-1.3.0.jar:/home/rrudakov/.m2/repository/org/clojure/tools.reader/1.3.6/tools.reader-1.3.6.jar:/home/rrudakov/.m2/repository/cider/cider-nrepl/0.43.3/cider-nrepl-0.43.3.jar:/home/rrudakov/.m2/repository/nrepl/nrepl/1.0.0/nrepl-1.0.0.jar:/home/rrudakov/.m2/repository/refactor-nrepl/refactor-nrepl/3.9.0/refactor-nrepl-3.9.0.jar:/home/rrudakov/.m2/repository/cider/orchard/0.20.0/orchard-0.20.0.jar:/home/rrudakov/.gitlibs/libs/io.github.cognitect-labs/test-runner/dfb30dd6605cb6c0efc275e1df1736f6e90d4d73/src:/home/rrudakov/.m2/repository/mx/cider/logjam/0.1.1/logjam-0.1.1.jar:/usr/lib/jvm/java-17-temurin/lib/src.zip:/home/rrudakov/.cache/mx.cider/enrich-classpath/1709/1323891204/1730288118.jar:/home/rrudakov/.cache/mx.cider/unzipped-jdk-sources/1709 -J--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED -m cognitect.test-runner

Environment & Version information

CIDER version information

;; CIDER 1.12.0-snapshot (package: 20231112.518), nREPL 1.0.0
;; Clojure 1.11.1, Java 17.0.9

Lein / Clojure CLI version

Clojure CLI 1.11.1

Emacs version

GNU Emacs 29.1.90 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars) of 2023-10-23

Operating system

6.6.1-arch1-1 GNU/Linux

JDK distribution

openjdk version "17.0.9" 2023-10-17
OpenJDK Runtime Environment Temurin-17.0.9+9 (build 17.0.9+9)
OpenJDK 64-Bit Server VM Temurin-17.0.9+9 (build 17.0.9+9, mixed mode, sharing)
vemv commented 10 months ago

I had never seen :paths [:clj-paths] syntax I believe - could you comment on it?

rrudakov commented 10 months ago

https://clojure.org/reference/deps_and_cli#_paths it's officially supported in deps.edn

vemv commented 10 months ago

Thanks!

I'll check it around tomorrow.

Feel free to verify if the problem goes away using the traditional syntax (Enrich certainly honors provided aliases)

rrudakov commented 10 months ago

Yes, if replace aliases with normal strings, it fixes the first problem. But second problem with :test alias still exists.

vemv commented 10 months ago

Problem 2 is not unique to Enrich, IME - it's very common in tools.deps projects to unintentionally run the tests.

Over the years (when Enrich wasn't on the radar) I always suggested to split your deps.edn into :test and :test-runner. That way one can e.g. pick an alternative runner.

In general, the best practice is to keep aliases fine-grained - more so than the average Lein project, for instance.

Otherwise I'd be hesitant about 'fixing' this - Enrich supports custom main programs so here, most likely it's doing what you're telling it to do.

rrudakov commented 10 months ago

Hm, but why I can successfully jack-in with :test alias enabled when cider-enrich-classpath is nil? I would expect it to fail as well then?

vemv commented 10 months ago

Enrich supports a broader set of use cases e.g. running it from the terminal, where it's more common to specify a main program (e.g. for specifying a repl - there are a few possible choices).

We may be able to hack something, but I think that nudging users into best practices also is a good idea.

rrudakov commented 10 months ago

OK. I've been using :test alias with main-opts for a while without any issues. What's the best way to split it?

{:test {:extra-paths ["test"]
                  :extra-deps  {io.github.cognitect-labs/test-runner {:git/tag "v0.5.1" :git/sha "dfb30dd"}}}

           :test-runner {:main-opts ["-m" "cognitect.test-runner"]
                         :exec-fn   cognitect.test-runner.api/test}}

If I split it like this I won't be able to run :test-runner without specirying :test additionaly (because of dependencies).

Or should I duplicate all dependencies in both :test and :test-runner aliases? Maybe you have some good article on the topic?

vemv commented 10 months ago

Thanks for the willingness!

I'd suggest to

This has the advantages of:

Maybe you have some good article on the topic?

Not really but probably it's easy enough to see how this is a design that favors composition - often valued in Clojure.

vemv commented 10 months ago

The latest Enrich supports the paths-as-aliases feature.

The issue related to :main-opts will remain as-is for the time being. I'm reasonably confident that we can foster the described, better pattern.

Cheers - V

rrudakov commented 10 months ago

Good enough for me :) Thank you!