Use Java Process API to launch Axon Ivy Engine #476
Conversation
alexsuter
commented
Jul 29, 2024
alexsuter
commented
- No longer need commons.exec
- No need to SIGKILL engine in tests which occurs in side-effects on windows and ugly errors in build log
|
sounds great 🎸 |
| @@ -133,7 +134,10 @@ private ProcessBuilder toProcessBuilder(Command command) { | |||
| cmds.add("-Dosgi.install.area=" + osgiDir.toAbsolutePath()); | |||
|
|
|||
| if (StringUtils.isNotBlank(context.vmOptions.additionalVmOptions())) { | |||
| cmds.add(context.vmOptions.additionalVmOptions()); // todo no escape | |||
There was a problem hiding this comment.
Hello @ivy-rew
I've one issue here. The additionalVmOptions were passed unescaped to commons.exec as one-liner. This is not possible with Java Process API.
A quick-dirty hack which might not work for all uses cases is to split them by whitespace, but I think the real solution would be to change additionalVmOptions from a one-liner to a List.
You see such a pattern also in other plugins:
https://maven.apache.org/plugins/maven-compiler-plugin/compile-mojo.html#compilerArgs
What do you think should we go with this solution, which works at least for engine-integration-test or should I already introduce a new one and deprecate the old one? And implement the old one with this approach?
There was a problem hiding this comment.
Yes, I would definitively support a real list argument type, just like we also do to configure JDT: https://axonivy.github.io/project-build-plugin/release/11.3/compileProject-mojo.html#compilerOptions
And another yes, I'd deprecate the old parameter and make it work by analysing whitespaces ... or maybe glean some code from commons.exec that did this logic. Though I do not think it's worth to investigate a lot into this backward compatibility ... I think these parameters have rarely been configured in projects.
There was a problem hiding this comment.
Sounds good. I will make a new PR for this.
- No longer need commons.exec - No need to SIGKILL engine in tests which occurs in side-effects on windows and ugly errors in build log
|
fyi @ivy-cst |
