sbt / sbt-native-packager

sbt Native Packager
https://sbt-native-packager.readthedocs.io/en/stable/
BSD 2-Clause "Simplified" License
1.59k stars 439 forks source link

"Improperly specified VM option" on Windows #1387

Closed mrubin closed 7 months ago

mrubin commented 3 years ago

Using sbt 1.4.4 to create a zip for a Play Framework (2.8) project via sbt dist.

I am specifying javaOptions just like seen in the documentation. These are my exact settings:

  javaOptions in Universal ++= Seq(
    "-J-server",
    "-Djava.awt.headless=true",
    "-Dpidfile.path=/dev/null",
    "-J-Xms576M",
    "-J-Xmx576M",
    "-J-XX:+AlwaysPreTouch",
    "-J-XX:+UseStringDeduplication",
    /* Begin G1GC Section */
    "-J-XX:+UseG1GC",
    "-J-XX:+UnlockExperimentalVMOptions",
    "-J-XX:MaxGCPauseMillis=500",             
    "-J-XX:G1NewSizePercent=30",              
    "-J-XX:G1MaxNewSizePercent=80",           
    "-J-XX:+ParallelRefProcEnabled",          
    "-J-XX:InitiatingHeapOccupancyPercent=35" 
    /* End G1GC Section */
  )

This creates an application.ini file as follows:

# options from build
-J-server
-Djava.awt.headless=true
-Dpidfile.path=/dev/null
-J-Xms576M
-J-Xmx576M
-J-XX:+AlwaysPreTouch
-J-XX:+UseStringDeduplication
-J-XX:+UseG1GC
-J-XX:+UnlockExperimentalVMOptions
-J-XX:MaxGCPauseMillis=500
-J-XX:G1NewSizePercent=30
-J-XX:G1MaxNewSizePercent=80
-J-XX:+ParallelRefProcEnabled
-J-XX:InitiatingHeapOccupancyPercent=35

This works fine on Mac and Linux, but results in "Improperly specified VM option 'MaxGCPauseMillis'" on Windows (with Java 8), when executing the .bat file in the bin folder.

Either of the following changes work for Windows, but do not work on Mac/Linux:

# options from build
-J-server
-Djava.awt.headless=true
-Dpidfile.path=/dev/null
-J-Xms576M
-J-Xmx576M
-J-XX:+AlwaysPreTouch
-J-XX:+UseStringDeduplication
-J-XX:+UseG1GC
-J-XX:+UnlockExperimentalVMOptions
"-J-XX:MaxGCPauseMillis=500"
"-J-XX:G1NewSizePercent=30"
"-J-XX:G1MaxNewSizePercent=80"
-J-XX:+ParallelRefProcEnabled
"-J-XX:InitiatingHeapOccupancyPercent=35"
# options from build
-J-server
-Djava.awt.headless=true
-Dpidfile.path=/dev/null
-J-Xms576M
-J-Xmx576M
-J-XX:+AlwaysPreTouch
-J-XX:+UseStringDeduplication
-J-XX:+UseG1GC
-J-XX:+UnlockExperimentalVMOptions
-J-XX:MaxGCPauseMillis=<500>
-J-XX:G1NewSizePercent=<30>
-J-XX:G1MaxNewSizePercent=<80>
-J-XX:+ParallelRefProcEnabled
-J-XX:InitiatingHeapOccupancyPercent=<35>

How do I set my javaOptions in a way that works on both Mac/Linux and Windows?

mrubin commented 3 years ago

Seems like the default .bat file has a bug. It's removing the equal sign and whatever is on the right of it, from the entries. I re-ran the Windows .bat file and printed !_JAVA_OPTS!.

The following application.ini entries:

-J-server
-Djava.awt.headless=true
-Dpidfile.path=/dev/null
-J-Xms576M
-J-Xmx576M
-J-XX:+AlwaysPreTouch
-J-XX:+UseStringDeduplication
-J-XX:+UseG1GC
-J-XX:+UnlockExperimentalVMOptions
-J-XX:MaxGCPauseMillis=500
-J-XX:G1NewSizePercent=30
-J-XX:G1MaxNewSizePercent=80
-J-XX:+ParallelRefProcEnabled
-J-XX:InitiatingHeapOccupancyPercent=35

get transformed into the following !_JAVA_OPTS!:

  -server -Djava.awt.headless=true -Dpidfile.path=/dev/null -Xms576M -Xmx576M -XX:+AlwaysPreTouch -XX:+UseStringDeduplication -XX:+UseG1GC -XX:+UnlockExperimentalVMOptions -XX:MaxGCPauseMillis -XX:G1NewSizePercent -XX:G1MaxNewSizePercent -XX:+ParallelRefProcEnabled -XX:InitiatingHeapOccupancyPercent
muuki88 commented 3 years ago

Hi @mrubin Thanks for the detailed error report.

Let me recap that I get this straight. Quoted works on windows, but not on Unix systems? E.g.

"-J-XX:MaxGCPauseMillis=500"

I must admit the application.ini was something I implemented years ago and wasn't the greatest idea IMHO. Having that said, as described in the docs you linked it should be possible to provide two different application.ini files.

mrubin commented 3 years ago

Hi @muuki88 - thank you for the quick response. You're correct,

"-J-XX:MaxGCPauseMillis=500"

works on Windows, but not Unix systems. I tried various tweaks and the error I get on Mac/Linux is along the lines of:

Bad root server path: /Users/.../my-project/"-J-server"

I found your suggested solution as well and it should work, but I was considering it a workaround. I don't think it's ideal long-term for the settings to be split between 2 locations - that makes it possible that they would go out of sync in the future.

It seems like the .bat file stripping the = sign is a bug - do you agree? If so, is there anyone knowledgeable with .bat files that could take a stab at a fix? It seems like that would be the proper long-term solution.

muuki88 commented 3 years ago

It seems like the .bat file stripping the = sign is a bug - do you agree?

Absolutely :+1:

If so, is there anyone knowledgeable with .bat files that could take a stab at a fix? It seems like that would be the proper long-term solution.

That's the issue :smile: Windows users are rare. Combined with sbt knowledge and time to contribute even rarer. I guess the logic is in this :parseConfig method, but I have really no idea how this works.

I found your suggested solution as well and it should work, but I was considering it a workaround. I don't think it's ideal long-term for the settings to be split between 2 locations - that makes it possible that they would go out of sync in the future.

I agree with you. What I tried to say with my previous comment "the application.ini wasn't the greatest idea" is that I would not recommend using it today. The bashScriptExtraDefines and batScriptExtraDefines are more robust and less magic as these things will just be added to the resulting scripts.

However the solution would be the same. Duplicate the configuration options.

So I'm trying to summon @dwickern or @nigredo-tori who were the last two people that were able to properly change something in the bat file :hear_no_evil: :heart_eyes:

mrubin commented 3 years ago

The issue appears to be in these lines of :process_args

Before calling :process_args, the application.ini entries look correct and after :process_args they are broken.

mrubin commented 3 years ago

https://stackoverflow.com/questions/20572424/preserving-equal-characters-in-batch-file-parameters may be relevant.

It looks like entries with = which are not surrounded by " (which does not work on Unix systems) treat the left-hand-side and right-hand-side of the equals as separate params:

-J-XX:MaxGCPauseMillis
500
-J-XX:G1NewSizePercent
30
...

And the numbers are then stripped by the ignore arguments that do not start with '-' check.

muuki88 commented 3 years ago

Thanks for the research and links. This part in the :process_args looks rather suspicious:

https://github.com/sbt/sbt-native-packager/blob/fd2c1e3f6dde2c851cd5da91b3b77ab99c4e84cd/src/main/resources/com/typesafe/sbt/packager/archetypes/scripts/bat-template#L159-L169

Maybe this is the line were we could add the additional quoting?

mrubin commented 3 years ago

I'm not sure that those lines are even hit in my scenario (where the entries in application.ini are not surrounded by double-quotes).

What I'm seeing for my scenario is that as the arguments are processed one at a time (and then shift is used to go to the next one), the left-hand-side and right-hand-side of the equals are split into separate arguments:

-J-XX:MaxGCPauseMillis
500
-J-XX:G1NewSizePercent
30
...

The entries that begin with -J are properly processed and are in the final output, and the entries that are just the numbers are also properly processed in that they don't look like a java command-line option and are stripped.

I think the issue is with the approach that :process_args is using to iterate through the arguments - the arguments which contain an = get split into two by this approach and would need to be "put back together" somehow. Or, a different approach needs to be used to iterate through the arguments - one that does not result in splitting them at the =.

mrubinwch commented 3 years ago

@muuki88 Any updates on this?

muuki88 commented 3 years ago

Nope 😔 some one from the community with windows experience would need to step up and fix this.