[Windows] path to kotlinc not quoted

[ckaag]

Until now I only used kscript in unix and within stuff like WSL, but as I'm currently reevaluating kscript for additional use, I've noticed I can now install it on Windows and use it directly. Sadly, out of the box it will break:

The PC I was installing it on has a pretty usual setup:

• kscript installed via scoop today
• using IntelliJ's builtin Kotlin distribution (per default installed to "C:\Program Files\JetBrains\IntelliJ<..>\plugins\Kotlin\kotlinc\bin\kotlinc.bat" (set in PATH in my case)

In Git Bash the log output to the README's hello world example is as follows (cmd meanwhile flies completely off the handle):

$ ./hello_kscript.kts
[kscript] [ERROR] Compilation of scriplet failed:
[kscript] [ERROR] Command     : 'cmd /c C:[bs]Program Files[bs]JetBrains[bs]IntelliJ IDEA Community Edition 2021.1.1[bs]plugins[bs]Kotlin[bs]kotlinc[bs]bin[bs]kotlinc.bat   -d "C:[bs]Users[bs]chris[bs]AppData[bs]Local[bs]kscript[bs]jar_79d1195f5b541eb5aa1a07f6bec7e715[bs]scriplet.jar" "C:[bs]Users[bs]chris[bs]AppData[bs]Local[bs]kscript[bs]jar_79d1195f5b541eb5aa1a07f6bec7e715[bs]Hello_kscript.kts" "C:[bs]Users[bs]chris[bs]AppData[bs]Local[bs]kscript[bs]jar_79d1195f5b541eb5aa1a07f6bec7e715[bs]Main_Hello_kscript.kt"'
[kscript] [ERROR] Exit Code   : 1
[kscript] [ERROR] Stdout      : ''
[kscript] [ERROR] Stderr      : 'Der Befehl "C:[bs]Program" ist entweder falsch geschrieben oder[nl]konnte nicht gefunden werden.[nl]'
[kscript] [ERROR]

Error message is in german but it comes down to the 'Program Files' directory containing a space in Windows, which breaks the path into two segments.

A quick lookup suggests CommandResolver::resolveKotlinBinary being the culprit - this argument of the command is the only one not being quoted. Escaping would be an alternative (but that would seem inconsistent behavior to me).

Would a PR for this be welcome?

Update/ It seems to be even worse. Directly quoting the path is out, the ShellExecutor.kt in kscript should be probably using something else than cmd /c "<path>" <args>, because the first character being a quote will cause legacy behavior: https://stackoverflow.com/a/356014/1907778

There are multiple ways to deal with this problem, one being to just not use absolute paths, but that's a major refactoring with way too much potential regressions.

The seemingly hackiest but somewhat most isolated and simplest solution seems to be to both put quotes around BUT ALSO prefix it with a single space. It all seems to work from there, and we can isolate this behavior inside CommandResolver only, and only for Windows. A space will disable the unexpected dequoting behavior, and a quoted path can contain as many spaces as you'd want. I will open a PR to show.

[aartiPl]

Thanks a lot for the report and PR - it improves the implementation nicely. I am just wondering if we shouldn't apply this functionality with a comment into the Shell library, as it seems generic for all executions. Please have a look, and let me know if it makes sense to put additional space here: https://github.com/kscripting/shell/blob/92d2856a8d0341a2a2942188294cc12891049153/src/main/kotlin/io/github/kscripting/shell/ShellExecutor.kt#L63

[ckaag]

Thanks a lot for the report and PR - it improves the implementation nicely. I am just wondering if we shouldn't apply this functionality with a comment into the Shell library, as it seems generic for all executions. Please have a look, and let me know if it makes sense to put additional space here: https://github.com/kscripting/shell/blob/92d2856a8d0341a2a2942188294cc12891049153/src/main/kotlin/io/github/kscripting/shell/ShellExecutor.kt#L63

I fully agree, would make sense. The only reason I see against it is that you'd still need to add the quotes within the caller, so only the space would get added inside the shell lib. Still is cleaner, and encapsulates the cmd curiosity where cmd is actually used.

I can create a second PR there if you want.

[aartiPl]

Yes, that would be great! Please also add the comment near the implementation, as this oddity of Windows needs to be explained so that someone will not remove that change by mistake :-) Thanks!

[aartiPl]

Resolved in 4.2.2