Skip to content

Add windows support - #194

Closed
abelsromero wants to merge 5 commits into
kscripting:masterfrom
abelsromero:add_windows_support
Closed

abelsromero wants to merge 5 commits into
kscripting:masterfrom
abelsromero:add_windows_support

Conversation

@abelsromero

Copy link
Copy Markdown

This is still WIP, 99% is done, only errors unknown to me are idea IT test.
Just want to share it in case some one has some comment.

Other than that, still missing the Windows CI. About that, I am considering removing the bash script and rewriting it in kotlin so it can be executed from gradle in a specific task, wdyt? maybe in another PR?

@abelsromero

Copy link
Copy Markdown
Author

If someone runs test_suite it in Windows (I found gitbash is nicer that cygwin), you will find some false positives due to difference in treatment of line breaks. But locking closer you see the result is correct.

The only real failures are in custom_interpreters and misc. The first I suspect due not being a real bash, the second still need to check.

........X..........
test #9 "source /c/Users/asalgadr/github/kscript/test/resources/cmd_subst_test.sh" failed:
        expected "command substitution works as well"
        got nothing
1 of 19 script_input_modes tests failed in 96.757s.
..
all 2 environment_tests tests passed in 6.324s.
X.....
test #1 "( resolve_deps log4j:log4j:1.2.14 ) 2>&1 >/dev/null" failed:
        expected "/c/Users/asalgadr/.m2/repository/log4j/log4j/1.2.14/log4j-1.2.14.jar"
        got "C:\Users\asalgadr\.m2\repository\log4j\log4j\1.2.14\log4j-1.2.14.jar"
1 of 6 dependency_lookup tests failed in 9.372s.
......
all 6 annotation_config tests passed in 30.707s.
XX...
test #1 "echo "foo${NL}bar" | kscript -t "stdin.print()"" failed:
        expected "foo
bar"
        got "foo\nbar"
test #2 "echo "foo${NL}bar" | kscript -t "lines.print()"" failed:
        expected "foo
bar"
        got "foo\nbar"
2 of 5 support_api tests failed in 21.754s.
.......XX..
test #8 "rm -f ./package_example && kscript --package test/resources/package_example.kts &>/dev/null && ./package_example 1" failed:
        expected "package_me_args_1_mem_4772593664"
        got nothing
test #9 "rm -f kscriptlet* && cmd=$(kscript --package "println(args.size)" 2>&1 | tail -n1 | cut -f 5 -d " ")  && $cmd three arg uments" failed:
        expected "3"
        got nothing
2 of 11 custom_interpreters tests failed in 61.981s.
........XX
test #9 "tmpDir=$(kscript_nocall --idea test/resources/includes/include_variations.kts | cut -f2 -d" " | xargs echo); cd $tmpDir && gradle build" failed:
        program terminated with code 1 instead of 0
test #10 "tmpDir=$(kscript_nocall --idea test/resources/includes/diamond.kts | cut -f2 -d" " | xargs echo); cd $tmpDir && gradle build" failed:
        program terminated with code 1 instead of 0
2 of 10 misc tests failed in 32.797s.

@holgerbrandl

Copy link
Copy Markdown
Collaborator

Thanks @abelsromero for putting together this nice set of enhancments.

What about WSL support, since kscript would run under linux in a bash-environment, I wonder if the changes in this PR are compatible with it?

I am considering removing the bash script and rewriting it in kotlin so it can be executed from gradle

Clearly CI support for windows would be nice. However, since the majority of users is using bash (since 'kscripts'`s original intent was to be a bash interpreter for kotlin, I actually also consider the test-suite as a a usage set of examples/part of the documentation.

custom_interpreters

Since defined interpreters in the shebang line is really a bash-feature, I'm not sure how this is compatible with the windows shell in general.

Let me know if you need any help.

@abelsromero

Copy link
Copy Markdown
Author

What about WSL support, since kscript would run under linux in a bash-environment, I wonder if the changes in this PR are compatible with it?

Good point! I just checked and found 2 issues that doesn't happen on a Linux.

  • Units complain if .kscript dir does not exists, that should be easy to fix.
  • and some it
.......XX..
test #8 "rm -f ./package_example && kscript --package test/resources/package_example.kts &>/dev/null && ./package_example 1" failed:
        expected "package_me_args_1_mem_4772593664"
        got nothing
test #9 "rm -f kscriptlet* && cmd=$(kscript --package "println(args.size)" 2>&1 | tail -n1 | cut -f 5 -d " ")  && $cmd three arg uments" failed:
        expected "3"
        got nothing
2 of 11 custom_interpreters tests failed in 140.196s.
........XX
test #9 "tmpDir=$(kscript_nocall --idea test/resources/includes/include_variations.kts | cut -f2 -d" " | xargs echo); cd $tmpDir && gradle build" failed:
        program terminated with code 1 instead of 0
test #10 "tmpDir=$(kscript_nocall --idea test/resources/includes/diamond.kts | cut -f2 -d" " | xargs echo); cd $tmpDir && gradle build" failed:
        program terminated with code 1 instead of 0
2 of 10 misc tests failed in 38.011s.

@holgerbrandl

Copy link
Copy Markdown
Collaborator

I would have guessed that at least custom interpreters should run smoothly under WSL. Luckily they at least fail with 1 so it should be rather easy to debug.

@abelsromero

Copy link
Copy Markdown
Author

I got everything working but this, which also fails on a Manjaro VM, but not in Travis. I am really confused.

........X..........
test #9 "source /c/Users/asalgadr/github/kscript/test/resources/cmd_subst_test.sh" failed:
        expected "command substitution works as well"
        got nothing

However, as reported in the other issue, I found that the last update of gradle breaks packaging 😭

@holgerbrandl what do you think should be next steps? fix the gradle issue or merge this?

@abelsromero abelsromero changed the title WIP: Add windows support Add windows support Dec 1, 2018
@abelsromero

Copy link
Copy Markdown
Author

btw, I changed the travis script to install gradle 4.10.2

@holgerbrandl

Copy link
Copy Markdown
Collaborator

You mentioned that the PR is almost ready. Could you let me know when it's complete from your end so that I could start testing it?

@abelsromero

Copy link
Copy Markdown
Author

It is ready now, I found a false positive and fixed it. Tests are not best, but they suffice for now, until I have a better understanding of all the parts I don't dare refactor much.

I've been also checking (very lighlty) travisCI for windows and it includes gitbash, so it would be possible to use the same batch of tests. However, I'd rather add it in another PR unless you consider otherwise.

@holgerbrandl

Copy link
Copy Markdown
Collaborator

I won't merge this PR because kscript seems to work fine now under gitbash after minor modifications to its launcher and because of the new aether based dependency resolver.

Thank you for your understanding and support.

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