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

Plugin assumes javaOptions to mean all command-line options instead of JVM options #1476

Open jypma opened 2 years ago

jypma commented 2 years ago

The sbt-native-packager plugin in its documentation and behavior interprets the SBT javaOptions setting to mean "command-line options and JVM options". However, the intent to mix both command-line and JVM options is a decision of sbt-native-packager's own bash and linux scripts. Specifically the -J prefix is specific to this plugin (although sensible and smart).

However, SBT specifies javaOptions to just mean:

Options passed to a new JVM when forking.

In other words, when specifying javaOptions there's no expectation or need to prefix them with any -J, since they're always JVM options. In fact, prefixing them would make normal forked sbt run fail, since then the -J would get passed to the JVM.

Suggested fix

Introduce a new, different key, e.g. nativeScriptDefaultOptions, and have that default to everything in javaOptions, adding a -J to each element. This would either be backwards-incompatible, or some smart heuristics could e.g. leave existing entries that set Universal / javaOptions directly alone.

muuki88 commented 2 years ago

Hi @jypma

Thanks for detailed issue and for your patience on this.

From what you describe I assume that the use case is sharing javaOptions for forking (e.g. for tests and local execution) and with the production Universal / javaOptions .

I like your suggestion as from an effort and backwards compatibility perspective. I'm not too happy with the many configuration ways sbt-native-packager offers as this is really confusing for users.

What I would currently recommend and you are probably doing is implementing that heuristic yourself:

Universal / javaOptions := javaOptions.value.map(option => s"-J$option")
Universal / javaOptions ++= List(
  "-Dfoo=bar",
  // ...
)

In the end this what the implementation in native-packager would look like?

jypma commented 2 years ago

My suggestion would be actually aligning what javaOptions means in Universal to what it means in plain SBT. This means, we should only have JVM arguments in there. Since that's a breaking change, this would need a major version bump in sbt native packager.

This way, if any build would then specify a plain e.g.

javaOptions := "-Xmx1024M"

this would apply to all environments. And if a build want to customize, e.g.

Text / javaOptions := "-Xmx2048M"
Universal / javaOptions := "-Xmx1024M"

the syntax would be consistent.

This would remove the ability to specify default command-line arguments to be passed to the packaged application. So, if we want to retain that behavior, we'd introduce it under a new key, e.g. nativeScriptDefaultOptions. I think we there would have two choices:

I think I like the latter better, since within your Java program the command-line arguments are also always separate from JVM properties. It's only during the script that they're mixed. We might even call the key nativeDefaultArguments if we go in that direction.

A third alternative could be taking up with sbt to see if a "default application command-line arguments" (only, no JVM properties in there) key would make sense as a top-level SBT key. That's always a bit lacking there as well.