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

Remote cider-jack-in breaks with cider-enrich-classpath: No such file or directory #3567

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,7 @@

### Changes

- [#3567](https://github.com/clojure-emacs/cider/pull/3567): Fix remote `cider-jack-in-clj` breaking with `cider-enrich-classpath` when `clojure.sh|lein.sh` are not available on the remote.
- CIDER [Inspector](https://docs.cider.mx/cider/debugging/inspector.html): display Java class/method/field info when available.
- This info is available when [enrich-classpath](https://docs.cider.mx/cider/config/basic_config.html#use-enrich-classpath) is active.
- [#3495](https://github.com/clojure-emacs/cider/issues/3495): possibly display error overlays on [`cider-load-buffer`](https://docs.cider.mx/cider/usage/code_evaluation.html#basic-evaluation).
Expand Down
56 changes: 56 additions & 0 deletions cider-util.el
Expand Up @@ -515,7 +515,63 @@ 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)
(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
90 changes: 55 additions & 35 deletions cider.el
Expand Up @@ -411,40 +411,58 @@ 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."
(let* ((script-names (map-values cider--enrich-classpath-script-names))
(temp-prefix cider--temp-name-prefix)
(any-name (rx-to-string
`(or (: (or ,@script-names) (or eos space))
(: ,temp-prefix (or ,@script-names))))))
(string-match any-name cmd)))

(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:\n %s"
(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 +1679,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 +2136,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
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/"
,"/tmp/path/ with spaces/"
,"/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" (shell-quote-argument (concat path name)))
:var ((script (shell-quote-argument (concat path name))))
(it "is true in basic cases "
(expect (cider--enriched-cmd-p (concat "bash " script " /usr/bin/lein"))
:to-be-truthy)
(expect (cider--enriched-cmd-p (concat "TEST=1 bash " script " /usr/bin/env lein"))
:to-be-truthy))
(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-be-truthy))))))))

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