Skip to content

Commit

Permalink
Fix remote cider-jack-in-clj breaking with cider-enrich-classpath #…
Browse files Browse the repository at this point in the history
…3567

Fixes `cider-jack-in-clj` in tramp buffers throwing:
```
Could not start nREPL server: %s (%S)
bash: /remote/path/to/cider/clojure.sh: No such file or directory\n" "exited abnormally with code 127")
```
in cases where cider is not installed in the same directory on the
remote.

To fix this, we create temporary copy of the enrich-classpath script
named `.cider__<clojure.sh|lein.sh>__<random>` on the remote before
starting the server. The possible locations of the script are, in this
order:
- tramp-tempdir (usually "/tmp")
- clojure-project-dirj
- default-directory

If the script can't be created for any reason, the server is started with
`cider-enrich-classpath` set to nil.

Note: the temporary script will remove itself after use, but stick around when
something goes wrong before the remote process is started.
  • Loading branch information
caadr committed Nov 16, 2023
1 parent ef87c71 commit 44622e5
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 35 deletions.
57 changes: 57 additions & 0 deletions cider-util.el
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,64 @@ Any other value is just returned."
(mapcar #'cider--deep-vector-to-list x)
x))


;;; Files & Directories
(defun cider--ensure-executable (file)
"Try to make FILE executable if it isn't already.
Returns FILE on success, nil on failure."
(with-demoted-errors "Error while trying to make file executable:\n %s"
(when (or (file-executable-p file)
(and (set-file-modes file "u+x")
(file-executable-p file)))
file)))

(defconst cider--temp-name-prefix ".cider__"
"Prefix for marking temporary files created by cider.")

(defun cider--make-temp-name (file)
"Generate a randomized name from FILEs basename.
Tag it with `cider--temp-name-prefix'"
(make-temp-name
(concat cider--temp-name-prefix (file-name-nondirectory file) "__")))

(defun cider--make-nearby-temp-copy (file)
"Create a copy of FILE in the default local or remote tempdir.
Falls back to `clojure-project-dir' or `default-directory'.
The copy is marked with `cider--temp-name-prefix'."
(let* ((default-directory (or (clojure-project-dir) default-directory))
(new-file (file-name-concat (temporary-file-directory)
(cider--make-temp-name file))))
(copy-file file new-file :exists-ok nil nil :keep-permissions)
new-file))

(defun cider--inject-self-delete (bash-file)
"Make BASH-FILE delete itself on exit.
Injects the self-delete script after the first line, assuming it is a
shebang."
(let (;; Don't create any temporary files.
(remote-file-name-inhibit-locks t)
(remote-file-name-inhibit-auto-save-visited t)
(make-backup-files nil)
(auto-save-default nil)
;; Disable version-control check
(vc-handled-backends nil))
(with-temp-buffer
(insert-file-contents bash-file)
;; inject after the first line, assuming it is the shebang
(goto-char (point-min))
(skip-chars-forward "^\n")
(insert "\n")
(insert (format
"trap 'ARG=$?
rm -v %s
echo \"cider: Cleaned up temporary script after use.\"
exit $ARG
' EXIT"
(file-local-name bash-file)))
(write-file bash-file))
bash-file))


;;; Help mode

;; Same as https://github.com/emacs-mirror/emacs/blob/86d083438dba60dc00e9e96414bf7e832720c05a/lisp/help-mode.el#L355
Expand Down
96 changes: 61 additions & 35 deletions cider.el
Original file line number Diff line number Diff line change
Expand Up @@ -411,40 +411,64 @@ without interfering with classloaders."
:package-version '(cider . "1.2.0")
:safe #'booleanp)

(defun cider--get-enrich-classpath-lein-script ()
"Returns the location of enrich-classpath's lein.sh wrapper script."
(when-let ((cider-location (locate-library "cider.el" t)))
(concat (file-name-directory cider-location)
"lein.sh")))

(defun cider--get-enrich-classpath-clojure-cli-script ()
"Returns the location of enrich-classpath's clojure.sh wrapper script."
(when-let ((cider-location (locate-library "cider.el" t)))
(concat (file-name-directory cider-location)
"clojure.sh")))
(defvar cider--enrich-classpath-script-names
'((lein . "lein.sh")
(clojure-cli . "clojure.sh")))

(defun cider--enriched-cmd-p (cmd)
"Test if the shell-quoted CMD contains the name of an enrich-classpath script.
Returns the local path to the script or nil."
(let* ((script-names (map-values cider--enrich-classpath-script-names))
(temp-prefix cider--temp-name-prefix)
(any-name (rx-to-string
`(or (: (or bos "/") (or ,@script-names) (or eos space))
(: ,temp-prefix (or ,@script-names)))))
(script (thread-last
(split-string-shell-command cmd)
(seq-filter (lambda (part) (string-match any-name part)))
(seq-first))))
(when script
(shell-quote-argument script))))

(defun cider--get-enrich-classpath-script (project-type)
"Get or create an executable enrich-classpath script for PROJECT-TYPE.
If `default-directory' is remote, create a copy at
'<remote-tempdir>/.cider__<script-name>__<random>' that deletes itself after
use. The search for '<remote-tempdir>' is handled by tramp and falls back to
`clojure-project-dir' or `default-directory'. Returns nil if anything goes wrong."
(when-let* ((cider-dir (file-name-directory (locate-library "cider.el" t)))
(name (map-elt cider--enrich-classpath-script-names project-type))
(location (concat cider-dir name))
(script (cider--ensure-executable location)))
(if (file-remote-p default-directory)
(with-demoted-errors
"cider: Failed to initialize enrich-classpath on remote."
(thread-first
(cider--make-nearby-temp-copy script)
(cider--ensure-executable)
(cider--inject-self-delete)))
script)))

(defun cider--jack-in-resolve-command-enrich (project-type)
"Conditionally wrap the command for PROJECT-TYPE with an enrich-classpath script.
Resolves to the non-wrapped `cider-jack-in-command' if `cider-enrich-classpath' is nil or the
wrapper-script can't be initialized."
(when-let ((command (cider--resolve-command (cider-jack-in-command project-type))))
(if-let ((wrapper-script (and cider-enrich-classpath
(not (eq system-type 'windows-nt))
(cider--get-enrich-classpath-script project-type))))
(concat "bash "
(shell-quote-argument (file-local-name wrapper-script)) " "
command)
command)))

(defun cider-jack-in-resolve-command (project-type)
"Determine the resolved file path to `cider-jack-in-command'.
Throws an error if PROJECT-TYPE is unknown."
(pcase project-type
('lein (let ((r (cider--resolve-command cider-lein-command)))
(if (and cider-enrich-classpath
(not (eq system-type 'windows-nt))
(executable-find (cider--get-enrich-classpath-lein-script)))
(concat "bash " ;; don't assume lein.sh is executable - MELPA might change that
(cider--get-enrich-classpath-lein-script)
" "
r)
r)))
('lein (cider--jack-in-resolve-command-enrich 'lein))
('boot (cider--resolve-command cider-boot-command))
('clojure-cli (if (and cider-enrich-classpath
(not (eq system-type 'windows-nt))
(executable-find (cider--get-enrich-classpath-clojure-cli-script)))
(concat "bash " ;; don't assume clojure.sh is executable - MELPA might change that
(cider--get-enrich-classpath-clojure-cli-script)
" "
(cider--resolve-command cider-clojure-cli-command))
(cider--resolve-command cider-clojure-cli-command)))
('clojure-cli (cider--jack-in-resolve-command-enrich 'clojure-cli))
('babashka (cider--resolve-command cider-babashka-command))
;; here we have to account for the possibility that the command is either
;; "npx shadow-cljs" or just "shadow-cljs"
Expand Down Expand Up @@ -1661,7 +1685,11 @@ PARAMS is a plist with the following keys (non-exhaustive list)
(command-resolved (cider-jack-in-resolve-command project-type))
;; TODO: global-options are deprecated and should be removed in CIDER 2.0
(command-global-opts (cider-jack-in-global-options project-type))
(command-params (cider-jack-in-params project-type)))
(command-params (cider-jack-in-params project-type))
;; ignore `cider-enrich-classpath' if the jack-in-command does not include
;; the necessary wrapper script at this point
(cider-enrich-classpath (and cider-enrich-classpath
(cider--enriched-cmd-p command-resolved))))
(if command-resolved
(with-current-buffer (or (plist-get params :--context-buffer)
(current-buffer))
Expand Down Expand Up @@ -2114,13 +2142,11 @@ M-2 \\[cider-jack-in-universal]."
(cider-jack-in-clj arg))))


;; TODO: Implement a check for command presence over tramp
(defun cider--resolve-command (command)
"Find COMMAND in exec path (see variable `exec-path').
Return nil if not found. In case `default-directory' is non-local we
assume the command is available."
(when-let* ((command (or (and (file-remote-p default-directory) command)
(executable-find command)
"Test if COMMAND exists, is executable and shell-quote it.
Return nil otherwise. When `default-directory' is remote, the check is
performed by tramp."
(when-let* ((command (or (executable-find command :remote)
(executable-find (concat command ".bat")))))
(shell-quote-argument command)))

Expand Down
31 changes: 31 additions & 0 deletions test/cider-tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,37 @@
:and-return-value '())
(expect (cider-project-type) :to-equal cider-jack-in-default))))

(describe "cider--enriched-cmd-p"
(describe "when cmd does not contain the path to an enrich-classpath script"
(it "returns nil"
(expect (cider--enriched-cmd-p "/usr/bin/lein")
:to-equal nil)
(expect (cider--enriched-cmd-p "bash /usr/bin/lein")
:to-equal nil)))
(describe "for different path + script-name combinations"
:var* ((paths `("/simple/path/"
,(shell-quote-argument "/tmp/path/ with spaces/")
,(shell-quote-argument "/ssh:!slightly@cra --zy!path #enrich me/")))
(simple-names (map-values cider--enrich-classpath-script-names))
(tmp-names (mapcar (lambda (s) (cider--make-temp-name s)) simple-names))
(all-names (seq-concatenate 'list simple-names tmp-names)))
(cl-loop
for path in paths do
(cl-loop
for name in all-names do
(describe (format "cider--enriched-cmd-p with script: %s" (concat path name))
:var ((script (concat path name)))
(it "returns the script path in basic cases "
(expect (cider--enriched-cmd-p (concat "bash " script " /usr/bin/lein"))
:to-equal script)
(expect (cider--enriched-cmd-p (concat "TEST=1 bash " script " /usr/bin/env lein"))
:to-equal script))
(it "handles a fully constructed jack-in-cmd."
;; TODO is it worth generating this?
(let ((cmd (concat "bash " script " /usr/local/bin/lein update-in :dependencies conj \[nrepl/nrepl\ \"1.0.0\"\] -- update-in :plugins conj \[cider/cider-nrepl\ \"0.43.0\"\] -- update-in :plugins conj \[mx.cider/lein-enrich-classpath\ \"1.18.2\"\] -- update-in :middleware conj cider.enrich-classpath.plugin-v2/middleware -- repl :headless :host localhost")))
(expect (cider--enriched-cmd-p cmd)
:to-equal script))))))))

;;; cider-jack-in tests
(describe "cider--gradle-dependency-notation"
(it "returns a GAV when given a two-element list"
Expand Down

0 comments on commit 44622e5

Please sign in to comment.