Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jan 24, 2023

Conversation

goto1134
Copy link
Contributor

@goto1134 goto1134 commented Jan 21, 2023

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
Copy link
Collaborator

aartiPl commented Jan 23, 2023

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
Copy link
Contributor Author

goto1134 commented Jan 23, 2023

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 aartiPl merged commit 18d733f into kscripting:master Jan 24, 2023
@aartiPl
Copy link
Collaborator

aartiPl commented Jan 24, 2023

@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
Copy link
Contributor Author

goto1134 commented Jan 24, 2023

Actually we merged kscript to scoop recently ScoopInstaller/Extras#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
Copy link
Collaborator

aartiPl commented Jan 24, 2023

That sounds cool! I have created an issue for that:
#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
Copy link
Contributor Author

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
Copy link
Collaborator

aartiPl commented Jan 24, 2023

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.

@goto1134 goto1134 deleted the fix-calling-kscript-by-symlink branch January 25, 2023 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants