Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tmccombs opened this issue Mar 31, 2022 · 1 comment
Open
Labels
universal Zip, tar.gz, tgz and bash issues

Comments

@tmccombs
Copy link

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)

@muuki88 muuki88 added the universal Zip, tar.gz, tgz and bash issues label Jan 30, 2023
@muuki88
Copy link
Contributor

muuki88 commented Jan 30, 2023

Thanks @tmccombs for your patience.

To be honest I haven't deployed a deb file, systemd service or anything similar in years 😬 , 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 😄

Would you like to open a PR for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
universal Zip, tar.gz, tgz and bash issues
Projects
None yet
Development

No branches or pull requests

2 participants