-
Notifications
You must be signed in to change notification settings - Fork 50
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
Issue [#48] #58
base: master
Are you sure you want to change the base?
Issue [#48] #58
Conversation
oops, didn't run all the tests - figured out how with https://github.com/mojohaus/appassembler/blob/master/.travis.yml ... fixing |
…st and has a chance for its values to take precedence
any chance this can get merged? |
@@ -58,7 +58,7 @@ t.checkExistenceAndContentOfAFile(new File( fileBinFolder, "booter-windows/bin/b | |||
'@REM ******************************************', | |||
'@REM --- This is my own license header file ---', | |||
'@REM ******************************************', | |||
'%JAVACMD% %JAVA_OPTS% -classpath %CLASSPATH% -Dapp.name="booterLicenseHeaderTest" -Dapp.repo="%REPO%" -Dapp.home="%BASEDIR%" -Dbasedir="%BASEDIR%" org.codehaus.mojo.appassembler.booter.AppassemblerBooter %CMD_LINE_ARGS%', | |||
'%JAVACMD% %JAVA_OPTS% -classpath %CLASSPATH% -Dapp.name="booterLicenseHeaderTest" -Dapp.repo="%REPO%" -Dapp.home="%BASEDIR%" -Dbasedir="%BASEDIR%" org.codehaus.mojo.appassembler.booter.AppassemblerBooter %CMD_LINE_ARGS%', |
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.
Special reason why this has been changed? also Line 39 ?
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.
i must've been confused - removed those changes.
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.
oops - de ja vu :) #58 (comment) - some of these mvn --show-version --errors --batch-mode -Prun-its clean verify
tests failed
[INFO] Building: booterLicenseHeaderTest/pom.xml
[INFO] run script verify.groovy
[INFO] ..FAILED (2.3 s)
[INFO] The post-build script did not succeed. The expected content 'exec "$JAVACMD" $JAVA_OPTS \' in the file '/Users/nezda/code/appassembler.git/appassembler-maven-plugin/target/it/booterLicenseHeaderTest/target/generated-resources/appassembler/booter-unix/bin/booterLicenseHeaderTest' couldn't be found.
[INFO] Building: programLicenseHeaderTest/pom.xml
[INFO] run script verify.groovy
[INFO] ..FAILED (2.2 s)
[INFO] The post-build script did not succeed. The expected content 'exec "$JAVACMD" $JAVA_OPTS \' in the file '/Users/nezda/code/appassembler.git/appassembler-maven-plugin/target/it/programLicenseHeaderTest/target/appassembler/bin/programLicenseHeader-test' couldn't be found.
reverting that last change
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.
The travis test failure appears to be because they no longer support Java 7
https://travis-ci.org/github/mojohaus/appassembler/jobs/679529347
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.
What can I do to get this merged?
This reverts commit b8058f7
suppose I gotta make the tests pass again 😎... |
[#48] just swap
$JAVA_OPTS
and@EXTRA_JVM_ARGUMENTS@
so env is last and has a chance for its values to take precedence(I tested this works for me using
<unixScriptTemplate>
configuration option suggested in http://stackoverflow.com/a/27608001/689119 to override-Xmx
with$JAVA_OPTS
env)