sbt / sbt-native-packager

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

javaOptions should not come after mainclass #598

Closed woshilaiceshide closed 8 years ago

woshilaiceshide commented 9 years ago

to reproduce the bug with sbt-native-packager(1.0.2) in a give sbt project,

  1. in project/plugins.sbt: addSbtPlugin("com.typesafe.sbt" % "sbt-aspectj" % "0.10.2")
  2. in build.sbt: javaOptions in Universal ++= (AspectjKeys.weaverOptions in Aspectj).value
  3. type "dist"
  4. in the target/universal/xxx-xxx.zip/bin/xxx, we will find that "-javaagent:.../aspectjweaver-1.8.5.jar" is fed to mainclass as the application's argument, which is not expected.

I use the following statement in build.sbt as a workaround: bashScriptExtraDefines += s"""addJava "${(AspectjKeys.weaverOptions in Aspectj).value.mkString(" ")}""""

woshilaiceshide commented 9 years ago

the workaround mentioned in the previous is wrong, use the following: bashScriptExtraDefines ++= (AspectjKeys.weaverOptions in Aspectj).value.map("addJava " + _)

muuki88 commented 9 years ago

Sorry, this was a point of missing documentation. Prefixing arguments with -J makes the start script recognize these as jvm arguments. So in your case

javaOptions in Universal ++= (AspectjKeys.weaverOptions in Aspectj).value.map { prop =>
   "-J" + prop
}

I know this is counter-intuitive to what javaOptions means. However it's better than the bashScriptExtraDefines IMHO.

You can test your settings easly by starting the bash script with -d, e.g.

sbt stage
./target/universal/stage/bin/test-project-simple -d
woshilaiceshide commented 9 years ago

why not make javaOptions goes before mainClass automatically? "javaOptions" implies "java's options, not app's options".

muuki88 commented 9 years ago

I agree that this is a bit confusing. We kept it this way as the configuration files worked the same way before. So you don't have to change anything.

An additional setting appOptions would be appropriate. If you want to do a pull request? However I'm not sure how much this feature is used and how big the impact would be.

woshilaiceshide commented 9 years ago

I WILL make a pull request, in which the javaOptions will be "java's options", and "appOptions" will be "app's options". This will happen after this busy week.

muuki88 commented 9 years ago

Awesome @woshilaiceshide :)

It would be really awesome if previous javaOptions in Universal configurations would still work. The easiest way would be to alter (prefixing with -J, beside some exceptions, see bashscript) the settings before they get written to the application.ini file.

A new appOptions setting would then be written unchanged. I hope we can keep things backwards compatible and don't have to touch the bashscript and avoid using the bashScriptExtraDefines

woshilaiceshide commented 9 years ago

I am very sorry for the above promise. If you do not mind, should I delay my pull request for some weeks. My busy days make my time fly recently. Your reply is expected.

muuki88 commented 9 years ago

That's no problem. Just ping when you need any help after you started :)

kayvank commented 8 years ago

I had a similar issue and using the suggesion above resolved it.

javaOptions in Universal ++= (AspectjKeys.weaverOptions in Aspectj).value.map { prop =>
  "-J" + prop
}
muuki88 commented 8 years ago

Thanks for sharing :)