Fix for Java 24 w/ upcoming new Eclipse Sisu #2311
Conversation
|
The "full-build" CI job builds Maven Site, that contains PMD report that craps out on Java 24: |
|
The IT failures: 7045 craps out due old Groovy on Java 24, the latter two due lack of sisu index, but the build does not calls index plugin explicitly... |
|
maybe it is time for: |
impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java
Outdated
Show resolved
Hide resolved
|
ITs seems fixed, full-build still plagued by PMD issue |
| @@ -142,7 +142,7 @@ private String validateOutput(boolean shave, ByteArrayOutputStream stdout, ByteA | |||
| } | |||
| // sanity checks: stderr has any OR result is empty string (no method should emit empty string) | |||
| if (stderr.size() > 0 || result.trim().isEmpty()) { | |||
| throw new ExecutorException( | |||
| System.err.println( | |||
There was a problem hiding this comment.
do we have logger for this? using system directly would cause spotbugs violations. We having this code pattern all over, so its justiciable, still questionable as bigger picture.
https://stackoverflow.com/questions/8601831/do-not-use-system-out-println-in-server-side-code
Why cannot we throw anymore, please? Of course we dont want to lose the information. As we having error we can not tread, how and why continue?
Thanks.
There was a problem hiding this comment.
We cannot throw, as this code (whole maven-executor) purpose is to execute Maven (embedded in JVM, or forked as in forked child process) real Maven mvn command in IT-like scenarios (this very code is used in Maven ITs). This codebase supports Maven 3 and Maven 4 transparently. The goal of PR is to "prepare it for Java 24".
All existing Maven versions released so far (including Maven 4 rc releases) emit warning on Java 24, and there is nothing to be done here (the MNG-8399 and/or alike). Hence, to make this code work on Java 24 AND be able to use current releases (ie. 3.9.9 or 4-rc-3), it cannot throw anymore, as err logging is part of "normal operation" on those Maven versions on Java 24. Or in other words, if we'd throw here (as before), we would not be able to run Maven 3.9.9 and 4-rc-3 and older Maven versions using Java 24, only "fixed" Maven 3.9.10 and upcoming 4-rc-4, and that would narrow the capabilities of this module too much.
The point of this PR is to prepare codebase be able to run on Java 24 as is, and this throw was one of the obstacles preventing it.
| @@ -829,6 +829,17 @@ under the License.</licenseText> | |||
| </dependency> | |||
| </dependencies> | |||
| </plugin> | |||
| <plugin> | |||
| <groupId>org.apache.maven.plugins</groupId> | |||
| <artifactId>maven-pmd-plugin</artifactId> | |||
There was a problem hiding this comment.
why cost of carry? the plugin seems not in use:
The upcoming Eclipse Sisu will not provide anymore "shaded" JARs (with out without ASM).
Also, fixed the
maven-executortest that failed on "unexpected stderr output" that now happens with Java 24 due Guava.