cider icon indicating copy to clipboard operation
cider copied to clipboard

`sesman-start` should work regardless of the user shell

Open crinklywrappr opened this issue 3 years ago • 5 comments

Expected behavior

sesman-start should work regardless of the user shell

Actual behavior

Cider builds a string like this which has a lot of bash-specific escape characters. This probably works in many shells, but fails in others. Elvish, for example:

image

Steps to reproduce the problem

I am running nixos. To do this in nixos is very easy. Just set the user shell in the system config.

/etc/nixos/configuration.nix

users.users.<username>.shell = pkgs.elvish;

apply the change w/ sudo nixos-rebuild switch, reboot and attempt to start a cider environment in emacs. SPC m ' in spacemacs.

Environment & Version information

CIDER version information

;; CIDER 1.2.0snapshot (package: 20211021.545), nREPL 0.9.0-beta3
;; Clojure 1.10.1, Java 17.0.1

Lein/Boot version

Leiningen 2.9.7 on Java 17.0.1 OpenJDK 64-Bit Server VM

Emacs version

27.2

Operating system

nixos 21.11

crinklywrappr avatar Jan 03 '22 20:01 crinklywrappr

I am assuming that the Elvish shell does not support the Posix utility syntax (guildeline 10) that says the -- characters indicate the end of the command options and the start of the positional arguments

As far as I remember, Leiningen requires the -- to indicate arguments passed into the process that Leiningen runs, separating them from the lein command options

Do you know what syntax is required to support that Leiningen approach Elvish shell? How would you run Leiningen with CIDER and nREPL dependencies on the Evilsh shell command line (without hard-coding dependencies and making jack-in superfluous) ?

Does Elvish shell run successfully when using Clojure CLI rather than Leiningen? I havent seen the use of -- in Clojure CLI so perhaps that is another option for Elvish shell users.

practicalli-johnny avatar Jan 04 '22 11:01 practicalli-johnny

Good question. I am certain that the problem is with the brackets. It treats eg \[ as an invalid escape sequence. Here is a valid line which uses no escape sequences and works in bash AND elvish

lein update-in :dependencies conj '[nrepl/nrepl "0.9.0"]' -- update-in :plugins conj '[cider/cider-nrepl "0.27.2"]' -- repl :headless :host localhost

crinklywrappr avatar Jan 04 '22 15:01 crinklywrappr

Accounting for every possible shell type will complicate a lot the jack-in code, that's why I'm not eager to go in this direction. I'd be happy to accept small changes that address the support for some shell, provided someone is willing to implement those.

bbatsov avatar Jan 05 '22 08:01 bbatsov

Is it possible to execute leiningen using the one shell you support?

eg bash -e lein ...

crinklywrappr avatar Jan 05 '22 08:01 crinklywrappr

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

stale[bot] avatar Apr 17 '22 00:04 stale[bot]

Is it possible to execute leiningen using the one shell you support?

This probably wouldn't work because bash -e lein <a very long command string with bash-escaped characters> would still be invalid Elvish.

We could do something like dynamically persisting the command to a file, adding #!/usr/bin/env bash on top, then running that one-off file.

That might be a reasonably easy PR that we would welcome.

In the meantime, cider-connect always should be a workable alternative.

Cheers - V

vemv avatar Aug 25 '23 04:08 vemv

We could do something like dynamically persisting the command to a file, adding #!/usr/bin/env bash on top, then running that one-off file.

Humm, but that wouldn't work in Windows (when not using WSL, but the regular shell / Powershell), would it?

iarenaza avatar Sep 03 '23 10:09 iarenaza

I don't know, we're getting in a quite hypothetical scenario there (supporting Elvish but running on Windows but not on WSL) 😄

Probably if we had a solution in the lines of what I suggested, we could always tweak it driven by specific needs.

For instance we could emit either a .sh or a powershell script, depending on the host machine.

But someone would have to make the first move creating a PR and trying it locally for a while - we generally don't like to complicate things unless there's a pressing need.

vemv avatar Sep 03 '23 11:09 vemv

Sorry, my bad. I didn't want to imply that CIDER should support running all combinations of all possible shells on all possible operating systems with all execution environments. I just wanted to point out that if we want to only rely on bash (and its quoting syntax, etc.) to run the command line that CIDER builds, as a way to avoid having that "combinatorial explosion" of shell/operating systems/execution environment options, that would not cover all of the currently supported cases.

Because, in the general case, you can't expect bash to be available in Windows (unless you are using WSL, but in that case you are not "actually" using Windows, but Linux). I guess that is why Clojure itself (clj et al.) uses Powershell when runnin on Windows[1].

Not that I expect that many people to be using Clojure and CIDER on Windows (outside WSL, that is). But I just wanted to point out that, as you say, poweshell would be involved in the Widows case.

[1] https://github.com/clojure/tools.deps.alpha/wiki/clj-on-Windows

iarenaza avatar Sep 03 '23 12:09 iarenaza

In any case, I fully agree with you. No need to complicate things unless there is a pressing need. In our case, we launch leiningen from outside CIDER, because we run leiningen inside a container, with a lot of container specific config that we know beforehand hand, and hand-craft the specific lein command line that is suitable for that case. And we simply cider-connect to it from the host.

iarenaza avatar Sep 03 '23 12:09 iarenaza

Thanks!

Luckily we appear to be on the same page. AFAIK, cider has essentially two code paths: Unix and Windows. There is special handling for powershell quoting: https://github.com/clojure-emacs/cider/blob/5d87a4d2990cc17e9a1ba88ea9063b3278796fad/cider.el#L801

...so that pattern could be extended to detect Elvish, and use a dedicated function instead of shell-quote-argument.

Just worth noting, that change would have to come from an external PR.

vemv avatar Sep 03 '23 14:09 vemv