Skip to content

Strip new lines in argLine and debugArgLine parameters #77

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

Merged

Conversation

veysiertekin
Copy link
Contributor

@veysiertekin veysiertekin commented Aug 8, 2020

Problem

Formatters sometime causes new lines in pom.xml files:

        <argLine>-Xms4g -Xmx4g -XX:MaxPermSize=512m -XX:MaxDirectMemorySize=10g
          -Dlog4j2.configurationFile=log4j2-fatal-only.xml
        </argLine>

This result in failures at java command execution:

[INFO] --- scalatest-maven-plugin:1.0:test (test) @ test-project ---
Usage: java [-options] class [args...]
           (to execute a class)
   or  java [-options] -jar jarfile [args...]
           (to execute a jar file)
where options include:
    -d32	  use a 32-bit data model if available
    -d64	  use a 64-bit data model if available
    -server	  to select the "server" VM
                  The default VM is server,
                  because you are running on a server-class machine.


    -cp <class search path of directories and zip/jar files>
    -classpath <class search path of directories and zip/jar files>
                  A : separated list of directories, JAR archives,
                  and ZIP archives to search for class files.
    -D<name>=<value>
                  set a system property
    -verbose:[class|gc|jni]
                  enable verbose output
    -version      print product version and exit
    -version:<value>
                  Warning: this feature is deprecated and will be removed
                  in a future release.
                  require the specified version to run
    -showversion  print product version and continue
    -jre-restrict-search | -no-jre-restrict-search
                  Warning: this feature is deprecated and will be removed
                  in a future release.
                  include/exclude user private JREs in the version search
    -? -help      print this help message
    -X            print help on non-standard options
    -ea[:<packagename>...|:<classname>]
    -enableassertions[:<packagename>...|:<classname>]
                  enable assertions with specified granularity
    -da[:<packagename>...|:<classname>]
    -disableassertions[:<packagename>...|:<classname>]
                  disable assertions with specified granularity
    -esa | -enablesystemassertions
                  enable system assertions
    -dsa | -disablesystemassertions
                  disable system assertions
    -agentlib:<libname>[=<options>]
                  load native agent library <libname>, e.g. -agentlib:hprof
                  see also, -agentlib:jdwp=help and -agentlib:hprof=help
    -agentpath:<pathname>[=<options>]
                  load native agent library by full pathname
    -javaagent:<jarpath>[=<options>]
                  load Java programming language agent, see java.lang.instrument
    -splash:<imagepath>
                  show splash screen with specified image
See http://www.oracle.com/technetwork/java/javase/documentation/index.html for more details.
/bin/sh: line 1: -Dlog4j2.configurationFile=log4j2-fatal-only.xml: command not found
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  20.805 s
[INFO] Finished at: 2020-08-09T02:39:43+03:00

Solution

This PR replaces platform-agnostic new line delimiters ("\r", "\n", "\r\n") in argLine & debugArgLine with ordinary spaces (" ") to allow multiline JVM argument lists.

Reference

maven-surefire-plugin also have a fix for this:

https://github.com/apache/maven-surefire/blob/eb48f1b59ca5ccf6954ef33ecab03dbaf93214cd/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/DefaultForkConfiguration.java#L246-L249

@cla-bot
Copy link

cla-bot bot commented Aug 8, 2020

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: veysi.ertekin.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@veysiertekin veysiertekin force-pushed the bugfix/fix_multiline_jvm_argument branch from 705c2bd to d98bc21 Compare August 8, 2020 23:49
@cla-bot
Copy link

cla-bot bot commented Aug 8, 2020

Hi @veysiertekin, we require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please access https://www.artima.com/cla/choose-type to sign our Contributor License Agreement. Your effort is highly appreciated. Thank you.

@artimasites
Copy link

@cla-bot[bot] check

@cla-bot cla-bot bot added the cla-signed label Aug 8, 2020
@cla-bot
Copy link

cla-bot bot commented Aug 8, 2020

The cla-bot has been summoned, and re-checked this pull request!

@veysiertekin veysiertekin force-pushed the bugfix/fix_multiline_jvm_argument branch from d98bc21 to d5c7b1f Compare August 9, 2020 09:27
Copy link
Collaborator

@katrinsharp katrinsharp left a comment

Choose a reason for hiding this comment

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

Could you please add a test with a command line arguments without new lines, i.e.
"-Xms4g -Xmx4g -XX:MaxPermSize=512m"? Thank you.

@katrinsharp
Copy link
Collaborator

Hi Veysi, thank you for submitting you PR. Please see my comment.

@veysiertekin veysiertekin force-pushed the bugfix/fix_multiline_jvm_argument branch from d5c7b1f to 54f201a Compare September 21, 2020 06:31
@veysiertekin
Copy link
Contributor Author

veysiertekin commented Sep 21, 2020

hi @katrinsharp ,
I have added a few new test cases for command lines without or partially have new lines.

@katrinsharp katrinsharp merged commit bd51414 into scalatest:master Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants