-
Notifications
You must be signed in to change notification settings - Fork 70
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 args parsing on Windows #609
Fix args parsing on Windows #609
Conversation
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, the code you added to getBuildArgs()
needs to be unit tested.
Please refactor it to a separate method that you can varify in a Spock unit test.
native-maven-plugin/src/main/java/org/graalvm/buildtools/maven/AbstractNativeImageMojo.java
Outdated
Show resolved
Hide resolved
native-maven-plugin/src/main/java/org/graalvm/buildtools/maven/AbstractNativeImageMojo.java
Outdated
Show resolved
Hide resolved
aa39c68
to
1e5a91a
Compare
1e5a91a
to
5a86516
Compare
a0b1df8
to
5a86516
Compare
|
||
abstract class AbstractGraalVMMavenFunctionalTest extends Specification { | ||
@TempDir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you have removed @TempDir
annotation and then proceeded with deleting files manually in the cleanup function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my local when I work with Tempdir all tests are passing, because windows avoids tempDir path whitespace with a certain encoding (C:\Users\Lahoucine~\AppData\Temp) so no parsing errors will result if I have withespaces in my path.
...c/testFixtures/groovy/org/graalvm/buildtools/maven/AbstractGraalVMMavenFunctionalTest.groovy
Outdated
Show resolved
Hide resolved
native-maven-plugin/src/main/java/org/graalvm/buildtools/maven/AbstractNativeImageMojo.java
Outdated
Show resolved
Hide resolved
The plugin introduced some "windows fix"[1] that causes current arg parsing to fail. Solution seems to be to split each arg in own line, but that introduced another set of challenges. [1] graalvm/native-build-tools#609
…arsing to fail. Solution seems to be to split each arg in own line, but that introduced another set of challenges. [1] graalvm/native-build-tools#609 Supersedes apache#1135
The plugin introduced some "windows fix"[1] that causes current arg parsing to fail. Solution seems to be to split each arg in own line, but that introduced another set of challenges. [1] graalvm/native-build-tools#609 Supersedes #1135
The plugin introduced some "windows fix"[1] that causes current arg parsing to fail. Solution seems to be to split each arg in own line, but that introduced another set of challenges. [1] graalvm/native-build-tools#609 Supersedes #1133
Fixes https://github.com/micronaut-projects/micronaut-maven-plugin#1098