sbt-native-packager icon indicating copy to clipboard operation
sbt-native-packager copied to clipboard

loadConfigFile in launcher script doesn't support spaces in options.

Open tmccombs opened this issue 2 years ago • 1 comments

Expected behaviour

If my application.ini file, loaded by the loadConfigFile contains arguments with a space in it, for example:

-J-XX:OnOutOfMemoryError=/usr/sbin/my-oom-handler arg1 arg2

then that line should be passed as a single argument to the command not as three separate arguments.

Actual behaviour

The option is split on space and printed passed as the equivalent of '-J-XX:OnOutOfMemoryError=/usr/sbin/my-oom-handler' arg1 arg2.

At the packaging stage, you can workaround it by using something like:

bashScriptExtraDefines ++= Seq(
  "addJava -XX:OnOutOfMemoryError='kill -9 %p'"
)

However, if you need to add such configuration after installing a deployed artifact (such as a deb package), the only workarounds I can find are to modify the launch script, or to pass the option on the command line. The first is undesireable because I don't want to change launch script that is managed by the deb package. The second is undesirable, because it requires changing the ExecStart line of the the systemd service.

Information

  • What sbt-native-packager are you using 1.5.2
  • What sbt version 1.5
  • What is your build system (e.g. Ubuntu, MacOS, Windows, Debian ) Ubuntu 20.04
  • What package are you building (e.g. docker, rpm, ...) deb
  • What version has your build tool (find out with e.g. rpm --version) dpkg 1.19.7
  • What is your target system (e.g. Ubuntu 16.04, CentOS 7) Ubuntu 20.04

suggested fix:

Instead of using $(loadConfigFile "$script_conf_file") directly [here](https://github.com/sbt/sbt-native-packager/blob/df35f5f39b015892e57069f566da1aae160e5517/src/main/resources/com/typesafe/sbt/packager/archetypes/scripts/bash-template#L356], first store it in array, with something like:

if [[ -f "$script_conf_file" ]]; then 
  IFS=$'\n' read -a -r -d '' config_options < <(loadConfigFile "$script_conf_file")
  set -- "${config_options[@]}" "$@"
fi

Or, alternatively, for the array readding line:

readarray -t config_options < <(loadConfigFile "$script_conf_file")

(mapfile is a synonym to readarray)

tmccombs avatar Mar 31 '22 18:03 tmccombs

Thanks @tmccombs for your patience.

To be honest I haven't deployed a deb file, systemd service or anything similar in years :grimacing: , so I can only answer from a very highlevel perspective here.

If your proposed changes aren't breaking existing behaviour, pass the current tests and hopefully don't break the ash script, then I'm fine with it :smile:

Would you like to open a PR for this?

muuki88 avatar Jan 30 '23 12:01 muuki88