Clojure CLI command parameters order incorrect

[practicalli-johnny]

EDIT: cider-clojure-cli-aliases should be used instead of cider-clojure-cli-global-options and execution flags should not be included in the value, only the alias name (keyword, e.g. ":env/test") or names (keword chain, e.g. ":env/test:lib/reloaded")

The order of the arguments to the Clojure CLI tools command does not follow the documentation. It seems the -A flag (being deprecated) did allow the command to work with arguments out of sequence, however the -M does not.

Suggest updating the generation of the clojure command line to follow the documented order of arguments
clojure [clj-opt*] -M[:aliases] [init-opt*] [main-opt] [arg*]

Another approach would be to document a variable that can be used in the .dir-locals.el to disable the -Sdeps argument completely and require the Clojure CLI tools alias to provide the required libraries as an alias.

Expected behavior

Use a .dir-locals.el file to set the cider-clojure-cli-global-options to include an alias using the -M flag of the clojure command

((clojure-mode . ((cider-preferred-build-tool . "clojure-cli")
                           (cider-clojure-cli-global-options . "-M:env/test"))))

Open a Clojure project that is configured with deps.edn and run cider-jack-in and a REPL process should start using Clojure CLI tools, which cider then connects to.

The command line used by cider-jack-in-clj should place the -M:env/test argument after the -Sdeps argument and before the -m argument on the clojure command line

/usr/local/bin/clojure -Sdeps '{:deps {nrepl {:mvn/version "0.8.2"} refactor-nrepl {:mvn/version "2.5.0"} cider/cider-nrepl {:mvn/version "0.25.3"}}}' -M:env/test -m nrepl.cmdline --middleware '["refactor-nrepl.middleware/wrap-refactor", "cider.nrepl/cider-middleware"]'

Actual behavior

An error occurs and the REPL does not start

error in process sentinel: Could not start nREPL server: Execution error (FileNotFoundException) at java.io.FileInputStream/open0 (FileInputStream.java:-2).
-Sdeps (No such file or directory)

The clojure command line puts the -M:env/test argument before the -Sdeps argument, causing the error.

/usr/local/bin/clojure -M:env/test -Sdeps '{:deps {nrepl {:mvn/version "0.8.2"} refactor-nrepl {:mvn/version "2.5.0"} cider/cider-nrepl {:mvn/version "0.25.3"}}}' -m nrepl.cmdline --middleware '["refactor-nrepl.middleware/wrap-refactor", "cider.nrepl/cider-middleware"]'

Steps to reproduce the problem

Install Clojure CLI tools, version 1.10.1.679 or greater, following the Linux or Homebrew instructions on https://clojure.org/guides/getting_started#_clojure_installer_and_cli_tools

Clone this example Clojure project that demonstrates the issue, with an -M:env/test argument set in the .dir-locals.el file.
https://github.com/practicalli/four-clojure

Open the project in Emacs and call cider-jack-in-clj

The error should appear in the mini-buffer and in the messages buffer

Environment & Version information

CIDER version information

From cider-version:
CIDER 1.0.0snapshot (package: 20200930.1242)

From cider-display-connection-info:

CLJ four-clojure@localhost:46803 (Java 11.0.8, Clojure 1.10.1, nREPL 0.8.2)

Emacs version

GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0) of 2020-09-19

Operating system

Ubuntu Linux 20.04

[iarenaza]

@practicalli This is where the part of the command line that deals with global options, dependencies and other arguments to the Clojure CLI is built: https://github.com/clojure-emacs/cider/blob/master/cider.el#L565-L577

If my reading of CIDER code is correct, cider-clojure-cli-global-options ends up verbatim in global-opts in that function.

params is the result of evaluating cider-clojure-cli-parameters using format (thus the -m nrepl.cmdline --middleware '["refactor-nrepl.middleware/wrap-refactor", "cider.nrepl/cider-middleware"]' part using default configuration settings for a CLJ-only jack-in)

And dependencies is the consing of cider-jack-in-dependencies and cider-jack-in-lein-plugins (thus the -Sdeps '{:deps {nrepl {:mvn/version "0.8.2"} refactor-nrepl {:mvn/version "2.5.0"} cider/cider-nrepl {:mvn/version "0.25.3"}}}' part using default configuration settings for a CLJ-only jack-in).

So if my analysis is correct, this patch should fix the issue:

diff --git a/cider.el b/cider.el
index 6c134a15..af680b58 100644
--- a/cider.el
+++ b/cider.el
@@ -567,13 +567,13 @@ removed, LEIN-PLUGINS, and finally PARAMS."
 Does so by concatenating GLOBAL-OPTS, DEPENDENCIES finally PARAMS."
   (let ((dependencies (append dependencies cider-jack-in-lein-plugins)))
     (concat
-     global-opts
-     (unless (seq-empty-p global-opts) " ")
      "-Sdeps '{:deps {"
      (mapconcat #'identity
                 (seq-map (lambda (dep) (format "%s {:mvn/version \"%s\"}" (car dep) (cadr dep))) dependencies)
                 " ")
      "}}' "
+     global-opts
+     (unless (seq-empty-p global-opts) " ")
      params)))

 (defun cider-shadow-cljs-jack-in-dependencies (global-opts params dependencies)

Could you test it and confirm it does fix the issue?

Edit: I don't use Clojure CLI tools, so I don't know if the new arguments order would work in previous versions of the tool. If it doesn't, this would be a breaking change for people using previous versions of it.

[practicalli-johnny]

Edit: its working... yay...

It would seem that shadow-cljs dependences function directly below the also using the incorrect order, but I may leave that to another pull request as I haven not been using shadow recently.

[bbatsov]

The notion of global opts came from Leiningen and frankly I'm not sure it applies well to other build tools. Originally I just copy pasted the code for Lein, without giving that much thought as I never used tools.deps for anything meaningful. Perhaps for it we can remove this altogether and just have people update a single configuration param?

[bbatsov]

To be more precise - I don't there's much value to have both global-opts and params they seem more or less the same in clojure-cli. You probably know more about this than me, though. :-)

[practicalli-johnny]

The #2917 PR fixes the issue for those using Clojure CLI tools 1.10.1.697 or above (the default for new installs). If that could be merged then happy to work on a longer term solution.

global-opts could be changed to aliases to be more meaningful, although then it would also make sense to use cider-clojure-cli-aliases and depreciate the cider-clojure-cli-global-options variable eventually

It would be possible to replace dependencies and params entirely with an alias. In practicalli/clojure-deps-edn I already has the following aliases that should work. Although this relies on having this kind of alias in a project deps.edn or user level deps.edn file (~/.clojure/deps.edn)

  ;; Run a REPL using nREPL server for access by cider-connect-clj
  ;; clojure -M:middleware/cider-clj
  :middleware/cider-clj
  {:extra-deps {nrepl/nrepl       {:mvn/version "0.8.2"}
                cider/cider-nrepl {:mvn/version "0.25.3"}}
   :main-opts  ["-m" "nrepl.cmdline"
                "--middleware" "[cider.nrepl/cider-middleware]"]}

  ;; As above, but including clj-refactor
  :middleware/cider-clj-refactor
  {:extra-deps {nrepl/nrepl       {:mvn/version "0.8.2"}
                refactor-nrepl    {:mvn/version "2.5.0"}
                cider/cider-nrepl {:mvn/version "0.25.3"}}
   :main-opts  ["-m" "nrepl.cmdline"
                "--middleware" "[refactor-nrepl.middleware/wrap-refactor,cider.nrepl/cider-middleware]"]}

Using just an alias would give more flexibility on how to run specific projects, simply by using different aliases in the .dir-locals.el file for each project.

As an aside, I noticed that some of the library names are not qualified in the cider code, so I can fix this at some point too.

Now I understand the code a bit better, I will experiment and see what seems to work well.

[bbatsov]

global-opts could be changed to aliases to be more meaningful, although then it would also make sense to use cider-clojure-cli-aliases and depreciate the cider-clojure-cli-global-options variable eventually

In the early days of CIDER there were some more granular settings (e.g. Lein had one for the profile(s) to run), but eventually I removed those as I felt they added little value and added a lot of complexity (especially if you want expose them somewhere in the UI for interactive editing). That's the reason why I mentioned that probably we can consolidate the params further down the road.