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 args parsing on Windows #609

Merged
merged 11 commits into from
Sep 4, 2024

Conversation

houcine7
Copy link
Member

@houcine7 houcine7 commented Jul 9, 2024

  • When your home folder contains white spaces the write-file-args generates incorrect results as this example
...
\\QC:\\Users\\Lahoucine
EL
ADDALI\\.m2\\repository\\io\\netty\\netty-transport\\4.1.108.Final\\netty-transport-4.1.108.Final.jar\\E
^/META-INF/native-image/
--exclude-config
\\QC:\\Users\\Lahoucine
EL
ADDALI\\.m2\\repository\\io\\netty\\netty-codec-http\\4.1.108.Final\\netty-codec-http-4.1.108.Final.jar\\E
^/META-INF/native-image/
-cp
"C:\\Users\\Lahoucine 
EL 
ADDALI\\Desktop\\outdir\\target/java-application-with-custom-packaging-0.1.jar"
  • this pull request is aimed to address this issue.

Fixes https://github.com/micronaut-projects/micronaut-maven-plugin#1098

Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the 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.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Jul 9, 2024
@houcine7 houcine7 changed the title Fixes_micronaut-projects/micronaut-maven-plugin#1098 Fixes micronaut-projects/micronaut-maven-plugin#1098 Jul 9, 2024
@alvarosanchez alvarosanchez changed the title Fixes micronaut-projects/micronaut-maven-plugin#1098 Fix args parsing on Windows Jul 29, 2024
Copy link
Member

@alvarosanchez alvarosanchez left a 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.

@houcine7 houcine7 requested a review from alvarosanchez August 6, 2024 13:50
@houcine7 houcine7 force-pushed the fix-path-whitespaces-args branch from aa39c68 to 1e5a91a Compare August 7, 2024 12:03
@houcine7 houcine7 force-pushed the fix-path-whitespaces-args branch from 1e5a91a to 5a86516 Compare August 8, 2024 14:09
@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Aug 8, 2024
@houcine7 houcine7 force-pushed the fix-path-whitespaces-args branch from a0b1df8 to 5a86516 Compare August 8, 2024 14:45

abstract class AbstractGraalVMMavenFunctionalTest extends Specification {
@TempDir
Copy link
Collaborator

@dnestoro dnestoro Aug 9, 2024

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?

Copy link
Member Author

@houcine7 houcine7 Aug 9, 2024

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.

@alvarosanchez alvarosanchez merged commit 8536ecd into graalvm:master Sep 4, 2024
18 checks passed
cstamas added a commit to cstamas/maven-mvnd that referenced this pull request Sep 20, 2024
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
cstamas added a commit to cstamas/maven-mvnd that referenced this pull request Sep 20, 2024
…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
cstamas added a commit to apache/maven-mvnd that referenced this pull request Oct 9, 2024
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
cstamas added a commit to apache/maven-mvnd that referenced this pull request Oct 9, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants