added addApp and addJava to ash template like in bash template

[botany]

Fixes #862
I have only tested manually to generate tested in BusyBox v1.22.1.
If any tests needed for this functionality, please guide me how to do them.

[lightbend-cla-validator]

Hi @botany,

Thank you for your contribution! We really value the time you've taken to put this together.

Before we proceed with reviewing this pull request, please sign the Typesafe Contributors License Agreement:

http://www.lightbend.com/contribute/cla

[muuki88]

Hi Botany,

Thanks a lot for trying to implement this :)

How do you use the newly introduced functions? I guess you are using the bashScriptExtraDefines?

The $java_args configure the same thing as the $opts do, right? Not sure if we need two ways of configuring java options.

javaOptions in Universal	bashScriptExtraDefines
javaOptions in Universal += "-Xmx2g"	bashScriptExtraDefines += "addJava -Xmx2g"

I would rather prefer the first option rather then supporting two if there's no good reason.

The $app_commands ( maybe app_args are a better name? ) may be different. There is the $@ rest parameter for the start command, but if you don't have any control on how your image is started this doesn't help. And again, would you add them via bashScriptExtraDefines ?

Regarding the tests, we currently don't have any on our CI running, but please add one that shows things are working in the sbt-test/ash directory.

We may also start documenting the features supported by the AshScript here.

The Developer Guide describes how to build the documentation and run the tests.

[botany]

Hi Nepomuk,

Yep, I'm adding them via bashScriptExtraDefines.
When I first faced the problem I went through the docs here

which explicitly support adding configuration properties via different options.
There're actually not 2 but 3 ways to configure this:

• application.ini file
• javaOptions in Universal
• bashScriptExtraDefines

Unfortunately the way it works for the bash script isn't the way it works for the ash script.
The current version of the ash script supports only application.ini and only for java arguments.
If you look at the way it's configured in bash template (including naming and args passing approach):

You will see that all I did is moved some parts into the ash script and fixed some minor syntax things as sh and bash aren't the same.

To my humble opinion it's too late to change this confusing behavior for the bash template cause breaking things for existing users is not good. On the other hand the ash template behavior should probably be similar to the bash template cause essentially they are the same.

Regarding tests, I will take a look, tnx.

[muuki88]

Unfortunately the way it works for the bash script isn't the way it works for the ash script.
The current version of the ash script supports only application.ini and only for java arguments.

My hope was to keep only the relevant stuff in the ash script and not build another big ball of bash. However I see your point. Switching the start script template should not change the behavior. A feature may not be available, but it never should behave differently.

To my humble opinion it's too late to change this confusing behavior for the bash template cause breaking things for existing users is not good

That's a valid point. If a clean up happens, we should perform it for every usage.

In short: Code looks okay. Test and docs should be added :)

[muuki88]

Can you take a look at my comment in #878 . Maybe we can fix this along with your pr.

[muuki88]

A test demonstrating a working behavior is missing. There are already test available you can use as an example.

[muuki88]

Close in favor of #942