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

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

Open tmccombs opened 2 years ago

tmccombs commented 2 years ago

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

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)

muuki88 commented 1 year ago

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?