fix support for top-level main methods in Scala 3
#1592
Conversation
|
At least one commit author (matthias.berndt@ttmzero.com) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user |
|
Hi @mberndt123, Thank you for your contribution! We really value the time you've taken to put this together. We see that you have signed the Lightbend Contributors License Agreement before, however, the CLA has changed since you last signed it. |
|
At least one commit author (matthias.berndt@ttmzero.com) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user |
mberndt123
changed the title
main methods in Scala 3
|
At least one commit author (matthias.berndt@ttmzero.com) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user |
| @@ -129,4 +129,4 @@ java_cmd="$(get_java_cmd)" | |||
| # If a configuration file exist, read the contents to $opts | |||
| [ -f "$script_conf_file" ] && opts=$(loadConfigFile "$script_conf_file") | |||
|
|
|||
| eval "exec $java_cmd $java_opts -classpath $app_classpath $opts $app_mainclass $app_commands $residual_args" | |||
| exec $java_cmd $java_opts -classpath $app_classpath $opts $app_mainclass $app_commands $residual_args | |||
There was a problem hiding this comment.
I don't know why eval was used here. It breaks app_mainclass if it contains $ symbols, so I've removed it and it doesn't seem to break any tests, so… I guess it's fine?
There was a problem hiding this comment.
I'm not sure if #1334 added a test but it seems like eval was added there.
There was a problem hiding this comment.
OK, I've added that to the test file. Ironically, the tests still pass, and they fail if I put the eval back in. I think everything here is broken, both the way "residual args" are being handled and the test that is supposed to test them.
There was a problem hiding this comment.
There was a problem hiding this comment.
I've now fixed the tests and the escaping of residual args. I've also added tests for all kinds of nasty things that might confuse a shell script and they're all passed through correctly.
|
At least one commit author (matthias.berndt@ttmzero.com) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user |
|
At least one commit author (matthias.berndt@ttmzero.com) is not linked to a user. See https://help.github.com/en/articles/why-are-my-commits-linked-to-the-wrong-user#commits-are-not-linked-to-any-user |
src/main/resources/com/typesafe/sbt/packager/archetypes/scripts/ash-template
Show resolved
Hide resolved
Thanks @dwickern, is there anything else I need to do to get this merged? |
app_mainclass is currently not escaped correctly. This leads to problems in Scala 3 when you define
mainas a function at the top level (as opposed to inside anobject. In this case, the main class generated by the Scala compiler will contain $ symbols that need to be escaped.The
LauncherJarPluginabusesmainClassto set the-jar foo.jarparameter instead of a main class, so this case needs to be treated specially (I find this an ugly hack but haven't found a cleaner solution).The
scripted-dockertest failures seem to be unrelated to my changes as they also occur in other CI runs