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 for native-image to work in Windows #1312

Merged
merged 5 commits into from
Mar 5, 2020

Conversation

sshark
Copy link
Contributor

@sshark sshark commented Feb 29, 2020

  • The final native-image executable cannot be located in the file
    native-image.cmd unless the full path to native-image.cmd is
    given. Therefore, a new parameter is provided to the user to
    specify the native-image.cmd location.
  • Picks the right CLASSPATH separator according to the OS
    instead of hardcoded to colon. Using Colon will fail in Windows
    build.
  • Tested against GraalVM 20.0.0 and VS 2019.

* The final native-image executable cannot be located in the file
  native-image.cmd unless the full path to native-image.cmd is
  given. Therefore, a new parameter is provided to the user to
  specify the native-image.cmd location.
* Picks the right CLASSPATH separator according to the OS
  instead of hardcoded to colon. Using Colon will fail in Windows
  build.
* Tested against GraalVM 20.0.0 and VS 2019.
@lightbend-cla-validator

Hi @sshark,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement:

http://www.lightbend.com/contribute/cla

@muuki88 muuki88 added windows graalvm GraalVM releated issues labels Mar 2, 2020
muuki88
muuki88 previously approved these changes Mar 2, 2020
Copy link
Contributor

@muuki88 muuki88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the very well done pull request ❤️

A small remark that would be nice if you could fix it 😄

Copy link
Contributor

@muuki88 muuki88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix ❤️ I'll merge this as soon as the checks are green.

This might be too much to ask, but since windows users are rare 😄
If you have time and interest in helping to fix the windows build that would be awesome.
However don't feel pressured or anything. I just wanted to ask 😃

The appveyor build started to fail for no reasons a month ago. It started with #1299 which didn't change any windows related stuff.
Example: https://ci.appveyor.com/project/muuki88/sbt-native-packager/builds/31212944

[info] [error] > call set _PARAM1=%1 
[info] [error] error < > was unexpected at this time.
[info] [error] 
[info] [error] >   set "_TEST_PARAM=\"-Dtest.hoge=\[]!< >%\""
[info] [error] Total time: 13 s, completed Mar 3, 2020 4:15:24 PM
[error] x windows / test-bat-template 
[error]  Cause of test exception: {line 4}  Command failed: checkScript failed
Running windows / test-custom-main
[error] java.lang.RuntimeException: Failed tests:
[error] 	windows / test-bat-template
[error] 

I can't make any sense from the error message. So if you can and have an idea how to fix this, I would be suuuuper happy 😍

@sshark
Copy link
Contributor Author

sshark commented Mar 4, 2020

Let me take a shot at it over the weekend and see if I can make sense out of those error messages. I am not a Windows developer either. For me, Windows is just a means to an end 🤐 😃

@sshark sshark closed this Mar 4, 2020
@sshark sshark reopened this Mar 4, 2020
@muuki88 muuki88 merged commit 2d85875 into sbt:master Mar 5, 2020
@muuki88
Copy link
Contributor

muuki88 commented Mar 5, 2020

Let me take a shot at it over the weekend and see if I can make sense out of those error messages. I am not a Windows developer either. For me, Windows is just a means to an end

Even more thanks then 😍

@sshark sshark deleted the fix-native-image-windows branch March 8, 2020 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graalvm GraalVM releated issues windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants