kscripting / kscript

Scripting enhancements for Kotlin
MIT License
2.07k stars 124 forks source link

Fix calling kscript.bat by symlink and PowerShell compatibility #390

Closed goto1134 closed 1 year ago

goto1134 commented 1 year ago

The scenario:

  1. Unpack kscript files to a folder which is not in the PATH
  2. Make a symlink to kscript.bat
  3. Run kscript "println('1')" in cmd or kscript """println('1')""" in PowerShell.

You will get the following error:

Error: Could not find or load main class io.github.kscripting.kscript.KscriptKt
Caused by: java.lang.ClassNotFoundException: io.github.kscripting.kscript.KscriptKt

The reason is that kscript.bat strictly depends on kscript.bat being in your PATH. It uses where (where.exe from System32) to locate the kscript.bat in the PATH.

Another problem with this approach is that when executed in PowerShell, where becomes an alias to the Where-Object PowerShell function, which returns an empty string.

The provided solution both makes symlinks work and fixes compatibility with PowerShell.

aartiPl commented 1 year ago

Hi! First of all, thanks for providing the PR. I am happy to merge it. My only concerns are connected with the options 's' and 'i', which were available in

for %%i in (%~sf0) do set _BIN_DIR=%_BIN_DIR%%%~dpsi

's' is for short file names - I guess it should stay, but I am not sure about 'i'. Have you been able to check those options?

goto1134 commented 1 year ago

s is for short names (https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#short-vs-long-names) i is for the loop variable

As I understand the script

for %%i in (%~sf0) do set _BIN_DIR=%_BIN_DIR%%%~dpsi

can be translated to kotlin-like pseudocode this way:

var _BIN_DIR = ""
for (i in listOf(getShortCurrentScriptExecutableFileName())) {
  _BIN_DIR = getShortParentDirectoryPath(i);
}

The whole loop is equivalent to set _BIN_DIR=%~dps0. I will make a change to add s to the expression. But I don't know if this change affects the script result anyhow.

aartiPl commented 1 year ago

@goto1134 - I have merged the PR.

If you would like to work on something regarding Windows compatibility, I will gladly welcome the patches. For example distribution package for windows is needed: scoop or choco (mostly investigation first). Other than that, more tests in all areas are always welcome.

Thanks a lot for your contribution!

goto1134 commented 1 year ago

Actually we merged kscript to scoop recently https://github.com/ScoopInstaller/Extras/pull/9353. The reason I came here is that the scoop packaged kscript does not work without this fix.

Maybe I could help with choco too

aartiPl commented 1 year ago

That sounds cool! I have created an issue for that: https://github.com/kscripting/kscript/issues/391

It would be great if you could document in a simple way how to start and, later, how to do releases. Then I will be able to create release scripts. I have no idea what should be done, so you can probably help also in other ways. :-)

goto1134 commented 1 year ago

Actually you don't need to do anything specific. Just stick to the current GitHub release format and scoop should automatically update the package version once in a while.

If you are interested in how scoop works, you should check its wiki.

aartiPl commented 1 year ago

Great! I am working on the release scripts based on kscript, but they need to be more stable, to ask you for help with them. For now, I will reference you in the ticket and add a link to the wiki. You are welcome to add additional comments to the ticket.