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

Make jump-to-definition work in projects needing cider-path-translations #3414

Merged
merged 4 commits into from
Aug 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- [#3393](https://github.com/clojure-emacs/cider/issues/3393): Recompute namespace info on each shadow-cljs recompilation or evaluation.
- Recompute namespace info on each fighweel-main recompilation.
- [#3250](https://github.com/clojure-emacs/cider/issues/3250): don't lose the CIDER session over TRAMP files.
- [#3413](https://github.com/clojure-emacs/cider/issues/3413): Make jump-to-definition work in projects needing `cider-path-translations` (i.e. Dockerized projects).
- Fix the `xref-find-definitions` CIDER backend to return correct filenames.
- Fix the `cider-xref-fn-deps` buttons to direct to the right file.
- Make TRAMP functionality work when using non-standard ports.
Expand Down
45 changes: 33 additions & 12 deletions cider-common.el
Original file line number Diff line number Diff line change
Expand Up @@ -286,22 +286,38 @@ in the container, the alist would be `((\"/src\" \"~/projects/foo/src\"))."
:group 'cider
:package-version '(cider . "0.23.0"))

(defun cider--translate-path (path direction)
"Attempt to translate the PATH in the given DIRECTION.
(defun cider--translate-path (path direction &optional return-all)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me why we can have multiple mappings for the same path? I never used TRAMP, so I don't remember very well how this was supposed to work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we don't expect that to be the case, however I'm using the new cider--all-path-translations function (it has a comment justifying its existence) which gets the buffer-local cider-path-translations value from all buffers.

I imagine it's possible that some unrelated buffer's cider-path-translations can unexpectedly work for a given path.

So by specifying return-all, we get all possible matches - good and bad ones. Later, in the 'friendly session' computation, we compare them against the classpath. The bad ones won't matter - they won't be found in the classpath, so nothing is changed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems overly complicated to me, but given that I don't use or care about this functionality I'll assume you know what you're doing. :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish it could be less complicated. 😄

"Attempt to translate the PATH in the given DIRECTION, optionally RETURN-ALL.
Looks at `cider-path-translations' for (container . host) alist of path
prefixes and translates PATH from container to host or vice-versa depending on
whether DIRECTION is 'from-nrepl or 'to-nrepl."
(seq-let [from-fn to-fn path-fn] (cond ((eq direction 'from-nrepl) '(car cdr identity))
((eq direction 'to-nrepl) '(cdr car expand-file-name)))
(let ((path (funcall path-fn path)))
(seq-some (lambda (translation)
(let ((prefix (file-name-as-directory (expand-file-name (funcall from-fn translation)))))
(when (string-prefix-p prefix path)
(replace-regexp-in-string (format "^%s" (regexp-quote prefix))
(file-name-as-directory
(expand-file-name (funcall to-fn translation)))
path))))
cider-path-translations))))
(let ((f (lambda (translation)
(let ((path (funcall path-fn path))
(prefix (file-name-as-directory (expand-file-name (funcall from-fn translation)))))
(when (string-prefix-p prefix path)
(replace-regexp-in-string (format "^%s" (regexp-quote prefix))
(file-name-as-directory
(expand-file-name (funcall to-fn translation)))
path))))))
(if return-all
(seq-filter #'identity (mapcar f cider-path-translations))
(seq-some f cider-path-translations)))))

(defun cider--all-path-translations ()
"Returns `cider-path-translations' if non-empty, else seeks a present value."
(or cider-path-translations
;; cider-path-translations often is defined as a directory-local variable,
;; so after jumping to a .jar file, its value can be lost,
;; so we have to figure out a possible translation:
(thread-last (buffer-list)
(seq-map (lambda (buffer)
(buffer-local-value 'cider-path-translations buffer)))
(seq-filter #'identity)
(seq-uniq)
(apply #'append)
(seq-uniq))))

(defun cider--translate-path-from-nrepl (path)
"Attempt to translate the nREPL PATH to a local path."
Expand Down Expand Up @@ -336,7 +352,12 @@ If no local or remote file exists, return nil."
((and tramp-path (file-exists-p tramp-path))
tramp-path)
((and local-path (file-exists-p local-path))
local-path))))
local-path)
(t
(when-let* ((cider-path-translations (cider--all-path-translations)))
(thread-last (cider--translate-path local-path 'from-nrepl :return-all)
(seq-filter #'file-exists-p)
car))))))

(declare-function archive-extract "arc-mode")
(declare-function archive-zip-extract "arc-mode")
Expand Down
33 changes: 30 additions & 3 deletions cider-connection.el
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,9 @@ REPL defaults to the current REPL."
(sesman-more-recent-p (cdr session1) (cdr session2)))

(declare-function cider-classpath-entries "cider-client")
(cl-defmethod sesman-friendly-session-p ((_system (eql CIDER)) session)
"Check if SESSION is a friendly session."

(defun cider--sesman-friendly-session-p (session &optional debug)
"Check if SESSION is a friendly session, DEBUG optionally."
(setcdr session (seq-filter #'buffer-live-p (cdr session)))
(when-let* ((repl (cadr session))
(proc (get-buffer-process repl))
Expand Down Expand Up @@ -644,7 +645,33 @@ REPL defaults to the current REPL."
(or (seq-find (lambda (path) (string-prefix-p path file))
classpath)
(seq-find (lambda (path) (string-prefix-p path file))
classpath-roots))))))
classpath-roots)
(when-let* ((cider-path-translations (cider--all-path-translations))
(translated (cider--translate-path file 'to-nrepl :return-all)))
(seq-find (lambda (translated-path)
(or (seq-find (lambda (path)
(string-prefix-p path translated-path))
classpath)
(seq-find (lambda (path)
(string-prefix-p path translated-path))
classpath-roots)))
translated))
(when debug
(list file "was not determined to belong to classpath:" classpath "or classpath-roots:" classpath-roots)))))))

(defun cider-debug-sesman-friendly-session-p ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this named as a predicate?

You might want to mention it somewhere in the troubleshooting/sesman docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this named as a predicate?

Funny you ask, I intended it to read debug(sesman-friendly-session-p) i.e. the -p is for the thing being debugged.

Alternative welcome.

Good call on adding it to the manual!

"`message's debugging information relative to friendly sessions.

This is useful for when one sees 'No linked CIDER sessions'
in an unexpected place."
(interactive)
(message (prin1-to-string (mapcar (lambda (session)
(cider--sesman-friendly-session-p session t))
(sesman--all-system-sessions 'CIDER)))))

(cl-defmethod sesman-friendly-session-p ((_system (eql CIDER)) session)
"Check if SESSION is a friendly session."
(cider--sesman-friendly-session-p session))

(defvar cider-sesman-browser-map
(let ((map (make-sparse-keymap)))
Expand Down
4 changes: 4 additions & 0 deletions dev/docker-sample-project/.dir-locals.el
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
((nil . ((eval . (customize-set-variable 'cider-path-translations
(list
(cons "/src" (clojure-project-dir))
(cons "/root/.m2" (concat (getenv "HOME") "/.m2"))))))))
9 changes: 9 additions & 0 deletions dev/docker-sample-project/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM clojure:temurin-17-lein-bullseye
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This is a different Dockerfile from that of #3412)

ENV DEBIAN_FRONTEND=noninteractive
ENV NREPL_PORT=7888
WORKDIR /root/app
COPY . /root/app
RUN lein deps
EXPOSE 7888
RUN lein classpath
CMD ["lein", "repl", ":headless", ":host", "0.0.0.0", ":port", "7888"]
5 changes: 5 additions & 0 deletions dev/docker-sample-project/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
build:
DOCKER_BUILDKIT=0 docker build --no-cache -t cider-docker-dev .

run: build
docker run -v $$PWD/src:/app/src -p 7888:7888 cider-docker-dev
14 changes: 14 additions & 0 deletions dev/docker-sample-project/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
This project spins up a Clojure project within a Docker image.

The Docker image exposes an nREPL server.

This way, for development purposes, we can exercise CIDER's Docker-related capabilities.

To get started:

* From a terminal tab, run `make run` to run the Docker image
* Note that it has a volume mapping for `src`, so any local changes will be visible in the Docker image.
* Also note that the root of this subproject has a .dir-locals.el setting up `cider-path-translations`.
* `M-x cider-connect-clj`, choose localhost, 7888
* `M-x cider-load-buffer` the foo.clj namespace.
* From now on, you can `M-.` (jump to definition) recursively, starting from `clj-http.client`.
5 changes: 5 additions & 0 deletions dev/docker-sample-project/project.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
(defproject cider-docker-dev "0"
:dependencies [[org.clojure/clojure "1.11.1"]
[clj-http "3.12.3"]]
:source-paths ["src"]
:plugins [[cider/cider-nrepl "0.35.1"]])
2 changes: 2 additions & 0 deletions dev/docker-sample-project/src/bar.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
(ns bar
(:require [foo]))
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
# Use an official Clojure runtime as a parent image
FROM clojure:temurin-17-lein-bullseye

# Set environment variables to non-interactive (this prevents some prompts)
ENV DEBIAN_FRONTEND=noninteractive

# Define environment variable for nREPL port
ENV NREPL_PORT=7888

RUN apt-get update \
Expand All @@ -21,7 +16,6 @@ RUN locale-gen en_US.UTF-8
RUN locale-gen en en_US en_US.UTF-8
RUN dpkg-reconfigure locales

# Set root password
RUN echo 'root:cider' | chpasswd

RUN sed -i 's/^#* *PasswordAuthentication .*/PasswordAuthentication yes/' /etc/ssh/sshd_config
Expand All @@ -31,31 +25,20 @@ RUN sed -i 's/^#* *ChallengeResponseAuthentication .*/ChallengeResponseAuthentic
# SSH login fix. Otherwise user is kicked off after login
RUN sed 's@session\s*required\s*pam_loginuid.so@session optional pam_loginuid.so@g' -i /etc/pam.d/sshd

# Expose SSH port
EXPOSE 22

CMD ["/usr/sbin/sshd", "-D"]

# Set the working directory in the docker image
WORKDIR /usr/src/app

# Copy the current directory contents into the directory at /usr/src/app in the image
COPY . /usr/src/app

# Install any needed packages specified in project.clj
RUN lein deps

# Forward all relevant env vars for ssh sessions
RUN echo "export JAVA_HOME=${JAVA_HOME}" >> /root/.bashrc
RUN echo "export LEIN_HOME=${LEIN_HOME}" >> /root/.bashrc
RUN echo "export LEIN_JAVA_CMD=${LEIN_JAVA_CMD}" >> /root/.bashrc
RUN echo "export LEIN_JVM_OPTS=${LEIN_JVM_OPTS}" >> /root/.bashrc
RUN echo "export LEIN_ROOT=${LEIN_ROOT}" >> /root/.bashrc
RUN echo "export NREPL_PORT=${NREPL_PORT}" >> /root/.bashrc
RUN echo "export PATH=${PATH}" >> /root/.bashrc

# Make port 7888 available to the world outside this container (nREPL default port)
#EXPOSE 7888

# Run lein repl when the container launches, on port defined by NREPL_PORT
#CMD ["lein", "repl", ":headless", ":host", "0.0.0.0", ":port", "7888"]
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
:dependencies [[org.clojure/clojure "1.11.1"]
[clj-http "3.12.3"]]
:source-paths ["src"]
:plugins [[cider/cider-nrepl "0.35.0"]])
:plugins [[cider/cider-nrepl "0.35.1"]])
3 changes: 3 additions & 0 deletions dev/tramp-sample-project/src/foo.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(ns foo
(:require
[clj-http.client :as client]))
15 changes: 12 additions & 3 deletions doc/modules/ROOT/pages/config/basic_config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,10 @@ To prefer local resources to remote resources (tramp) when both are available:

== Translate File Paths

If you wish to translate file paths from your running instance you may use the
`cider-path-translations` defcustom to do so. For instance, suppose your app is
running in a docker container with your source directories mounted there. The
If you are running Clojure within a Docker image, or doing something similar (i.e. you're `cider-connect`ing to a process,
and there's a directory mapping for your source paths), you typically need to set `cider-path-translations`
for jump-to-definition to properly work. For instance, suppose your app is
running in a docker container with your source directories mounted there as volumes. The
navigation paths you'd get from nREPL will be relative to the source in the
docker container rather than the correct path on your host machine. You can add
translation mappings easily by setting the following (typically in `.dir-locals.el`):
Expand Down Expand Up @@ -237,6 +238,14 @@ different configurations.
In this example, the path `/src` will be translated to the correct path of your
Clojure project on the host machine. And `/root/.m2` to the host's `~/.m2` folder.

You need to run `lein deps` (or `clojure -P`, etc) in the host machine in order for
navigation to fully work, at least once, and then, preferably, every time your Maven dependencies change.
This allows the `.m2` part of `cider-path-translations` to be actually useful.

If you can't or won't do that, you can use TRAMP capabilities (which CIDER supports) instead of
setting up `cider-path-translations`. For that, you'd typically need to set up a SSH daemon
within your Docker image.

== Filter out namespaces in certain namespace-related commands

You can hide all nREPL middleware details from `cider-browse-ns*` and `cider-apropos*`
Expand Down
14 changes: 14 additions & 0 deletions doc/modules/ROOT/pages/troubleshooting.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -352,3 +352,17 @@ You can disable this Paredit behaviour by adding the following to your `init.el`
----
(define-key paredit-mode-map (kbd "RET") nil)
----

=== Interactions fail with `No linked CIDER sessions`

If any interactive feature is being shortcircuited for you with the message `No linked CIDER sessions`,
that's due to one of the following reasons:

* You're evaluating code in a buffer from a project that hasn't started a repl
* You can fix this by switching instead to a project that has.
* You can also, simply, start a repl in the current project.
* There's a bug in the CIDER/Sesman integration
* Session linking generally works by determining whether the current buffer is related to the classpath of some REPL.
* You can obtain debug info echoed to the `*messages*` buffer by running `M-x cider-debug-sesman-friendly-session-p` on the problematic buffer.
* By reading it, you might be able to determine why CIDER failed to see the relationship between `(buffer-filename)` and the classpath.
* Feel free to created a detailed GitHub issue including this information.
Loading