Compability for Windows (Spaces in path and maybe quoting problems)

[JanDornbusch]

Using the same project as in #1181 I ran into two startup problems using Win11 with Code and Calva:

Spaces in path

First startup failed as C:\Program could not be executed. I moved Lein and reinstalled Node without spaces in path (C:\dev\lein\ and C:\dev\NodeJS) and this problem was gone and the next one came up, I guess there might be a missing quotation to allow spaces in path to binaries. (As I moved both in one go I am not sure if only Node caused the problem here or both)

Startup problem 2 (maybe quotation too)

Startup fails with

shadow-cljs - running: lein with-profile +dev update-in :dependencies conj [cider/cider-nrepl "0.47.1"] -- run -m shadow.cljs.devtools.cli --npm -d cider/cider-nrepl:0.47.1 watch app test
Error encountered performing task 'update-in' with profile(s): 'base,system,user,provided,dev,dev'
java.lang.RuntimeException: EOF while reading
        at clojure.lang.Util.runtimeException(Util.java:221)
...

opening a powershell within code and executing

lein with-profile +dev update-in :dependencies conj '[cider/cider-nrepl \"0.47.1\"]' -- run -m shadow.cljs.devtools.cli --npm -d cider/cider-nrepl:0.47.1 watch app tapp

does not cause any problems and let me assume there might be some problem on new powershell version of win 11 as my old win 10 computer did not ran into such problems in past? As this problem appeared with all 2.20.x and latest version I have tried it might not be related to any recent change made.

[thheller]

I suspect this is a regression caused by https://github.com/thheller/shadow-cljs/issues/1180.

The code constructing the command to be executed is this: https://github.com/thheller/shadow-cljs/blob/adcf8d7f5cd3df312099d9a0c8cacd9e967cbc0f/src/main/shadow/cljs/npm/cli.cljs#L314

It properly encodes each argument into its own string which is then passed to node spawn.

https://github.com/thheller/shadow-cljs/blob/adcf8d7f5cd3df312099d9a0c8cacd9e967cbc0f/src/main/shadow/cljs/npm/cli.cljs#L93

Guessing that the now added :shell true leads node to "lose" some escaping, or not properly transferring it. Not a clue what to do about this, as shadow-cljs doesn't do any escaping itself here.

[JanDornbusch]

I'll have a look into it tomorrow, it seems only problematic part be the (pr-str dep), maybe in case of windows something like:

;; Escape all " and replace ' by escaped quots to make it maybe compatible?
(format "'%s'" 
        (clojure.string/replace (pr-str dep) #"(?<![\\])['\"]" "\\\""))

!!untested!!!

[thheller]

I suspect that First startup failed as C:\Program could not be executed. is also already part of the problem, as this shouldn't be a problem at all and hasn't been before. I also have node in C:\Program Files\.... and never had an issue with that.

[thheller]

Doesn't seem like the cleanest fix, but appears to work.

[thheller]

Try 2.28.8 of the shadow-cljs npm package.

[JanDornbusch]

And again thank you for take your time and the solution.

The saying "never change a running system" applies really well. If I hadn't had to exchange the computer I would possible not had any of those problems in near future. Even so, all the changes made, make it hard to track down where exactly the problems are located.

I haven't worked with child_process of Node yet but another solution beside the overkill I suggested before or the all double quotation would have been the use of conch or babashka to create shell processes? While I am sure you are having your reasons using the child_process here. I am using conch in some older projects to execute third party binaries and never had any problem with spaces there (just tested some of them on the new computer). Babashka be my choice in newer projects and it's not having those problems as well...

[babashka.process :as process]

(def SOME-BINARY "tools\\whisper\\whisper cpu.exe")

;; Could even do some try-catch here
(process/shell SOME-BINARY "-h")

Maybe these libraries (conch or babashka) can provide a more clean solution or even a function to borrow cleaning up parameters? (In case you wanna touch this case to clean it up in future)

Not sure if I hit the correct line, but it seems they are using a similar solution for arguments with spaces: If argument contains space you can wrap it with ' or \".

[JanDornbusch]

Confirmed to be solved!

[thheller]

Both babashka and conch are Clojure libraries. This doesn't help since the part in question in running in node and is written in CLJS.