cider
cider copied to clipboard
Remote `cider-jack-in` breaks with `cider-enrich-classpath`: `No such file or directory`
This is an attempt to fix enrich-classpath erroring out, when default-directory is remote, by creating a copy of the lein.sh|clojure.sh script on the remote before jacking in.
The core of it is happening here.
Conversation from the original Issue below.
- [x] The commits are consistent with our contribution guidelines
- [x] You've added (very few) tests to cover your change(s)
- [x] All tests are passing (
eldev test) - [x] All code passes the linter (
eldev lint) which is based onelisp-lintand includes- byte-compilation,
checkdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.
- byte-compilation,
- [x] You've updated the changelog (if adding/changing user-visible functionality)
- [-] You've updated the user manual (if adding/changing user-visible functionality)
Expected behavior
With point in a project-file visited via ssh/tramp and cider-enrich-classpath = t,
cider-jack-in-clj creates a remote repl and connects to it.
Actual behavior
nrepl-server-sentinel throws:
error("Could not start nREPL server: %s (%S)" "bash: /home/**/cider/clojure.sh: No such file or directory\n" "exited abnormally with code 127")
Reason
Cider is trying to use the local path (from cider--get-enrich-classpath-clojure-cli-script) on the remote, where it does not exist.
Possible Workaround
Copy the script to the remote before starting the server.
Hi @adrech !
I'm happy to hear you're using Enrich. The proposed branch would LGTM after the following feedback:
- Extract the files to cider-util.el instead
- cider.el has grown pretty large
- If it's at hand, prefer
mktemp -d -t cider.XXXXXXto/tmp - Detect if the copying failed
- If so, ~setq-local~ bind
cider-enrich-classpath niland proceed - Would appreciate if you could pretend (hardcode) that this failed while QAing this, to verify that the fallback works as intended.
- If so, ~setq-local~ bind
Thanks - V
Let me know if you'd like to target this during the week, matching our fortcoming major release.
Else of course, it's no issue - we could also shortcirtuit Enrich for the time being for the tramp code path.
Will be on it! I spent the weekend learning more about tramp, shells, ssh & docker and sanitized how the tramp-sample-project sets up PATH, so I have something reliable to test against. If you'd like, I can include that or create a separate PR.
Whats the target date?
That's awesome to hear!
Feel free to include that work as you wish. Perhaps separate PR since most likely we'll end up squashing stuff, so more commits is better.
Whats the target date?
Flexible - do not feel pressured, we can also simply cut bugfix releases later.
- If it's at hand, prefer
mktemp -d -t cider.XXXXXXto/tmp
This should essentially do the same thing in most cases:
(make-nearby-temp-file "cider." :dir) ;; => /ssh:root@localhost#8022:/tmp/cider.OrkB00
In this draft I make the copy directly at <remote-tempdir>/.cider__<script-name>__XXXXXX, because cleanup seemed simpler without a directory. The way it works right now, the file could end up in default-directory - when tramp does not find an accessible tempdir on the remote - and I didn't want the files to pile up in the project dir. There are still some holes in the cleanup process: when something goes wrong before the script is executed it will stay. Making the script delete itself is probably not the best approach, but I like that it is "self-contained" and it was fun playing around :)
- If so, ~setq-local~ bind
cider-enrich-classpath niland proceed
This is happening in cider--update-jack-in-cmd by checking if the command contains one of the scripts names. To keep that simple but reliable, I prefixed the files with enriched_[lein|clojure].sh.
So far, this draft works as expected with a minimal init.el on my pi and in clojure / lein docker containers, but it'll need more thorough testing, considering the moving parts.
EDIT: did some manual testing with a renamed /tmp and a read-only project dir and cider is falling back correctly.
Let me know if I'm moving in the right direction and I'll split it up, add tests, docstrings.
Happy to see these iterations 🙌
Please don't rename lein.sh → enrich_lein.sh as it makes some UIs more verbose, and it also would require a PR against https://github.com/melpa/melpa which is extra friction.
These files are also "APIs" that users can reference from .zshrc.
Cheers - V
Aye, of course, reverted the filenames.
There's a potential problem (with handling ssh remotes in general):
All of this assumes, that tramp and the remote PATH are set up to work together. It will fail early in the process otherwise.
This would happen e.g. when one connects via ssh + relies on a PATH setup in the remotes .bashrc or .profile + uses default tramp. More specifically, it they don't have this line in their tramp config, which makes tramp grab the env from a login shell:
(add-to-list 'tramp-remote-path 'tramp-own-remote-path)
For more background, see: (info "(tramp) Remote progams") and the ramblings at
https://github.com/clojure-emacs/cider/blob/92c9694161fa7638b5e9d05bb797ffefde613237/dev/tramp-sample-project/Dockerfile.template#L4
A couple of ways to approach this:
- rebinding
tramp-remote-pathsomewhere high up in the callchain (only if it is untouched) - informing the user that they might have this problem and how to fix it, when resolving a remote command fails
- reverting to the old behaviour of
cider--resolve-command, that would just no-op on remotes and let the user run into an error message likejava not found...when the server tries to start, which would give them an idea that something is wrong with their PATH setup.