search

mojohaus / exec-maven-plugin

Exec Maven Plugin
https://www.mojohaus.org/exec-maven-plugin/
Apache License 2.0
163 stars 96 forks source link

[#389] Add option 'blockSystemExit' to 'java' mojo #390

Closed kriegaex closed 7 months ago

kriegaex commented 7 months ago

This new option enables users to stop programs called by exec:java from calling System::exit, terminating not just the mojo but the whole Maven JVM.

Closes #389. Relates to #388.

kriegaex commented 7 months ago

@slawekjaranowski and other maintainers: Please review.

kriegaex commented 7 months ago

I force-pushed a fix for the problem that only JDK 17+ needs the extra mavenOpts for Maven Invoker ITs. I thought, the system property would just be ignored, but older JDKs interpret the value "allow" for the security manager (SM) as a class name, trying to instantiate a SM of that name.

Please run the build again.

kriegaex commented 7 months ago

@slawekjaranowski, another question: You seem to be re-starting some Windows ITs unrelated to my change. They also pass locally on my machine under Windows in the same JDK. Does that imply that you already know they are flaky? And do you know the reason why they are flaky? Is it because they run concurrently?

slawekjaranowski commented 7 months ago

I will try to fix a ITs on windows ....

slawekjaranowski commented 7 months ago

I hope fix ITs by #392 - please rebase

kriegaex commented 7 months ago

Rebased and pushed. There actually was a merge conflict, because we both added a new Maven profile in the same place. but no big deal.

kriegaex commented 7 months ago

@slawekjaranowski, is there anything else stopping you from merging? I want to keep the difference between touch time and cycle time for this PR as low as possible, because frequent context switches are just bad for productivity.

On a side note, I am surprised that you need to explicitly approve not just the first build for a pull request but every single one. This kind of regulation is kind of unusual, most OSS projects only block the first one. It forces you to micro-manage the PR and wait for the build result while maybe you do not have much else to do with regard to reviewing it. So, probably you will also switch the context to something else first and then back again.