Skip to content

Fix for Java 24 w/ upcoming new Eclipse Sisu #2311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed

Fix for Java 24 w/ upcoming new Eclipse Sisu #2311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f46e2e4
Fix for Java 24 w/ upcoming new Eclipse Sisu
cstamas May 9, 2025
414a8bb
Align ASM with the one we want always
cstamas May 9, 2025
342d65f
Use snapshots
cstamas May 9, 2025
56dd285
Reformat
cstamas May 9, 2025
832e94b
Plugin repo needed as well
cstamas May 9, 2025
c4d4683
Merge remote-tracking branch 'upstream/master' into fix-java24
cstamas May 12, 2025
c7f03ee
Take Java 24 to dance as well
cstamas May 12, 2025
7ae9d2b
Drop unneeded changes
cstamas May 12, 2025
399677a
Naive attempt to fix ITs
cstamas May 12, 2025
6f472fa
Up PMD
cstamas May 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/maven.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
java: ['17', '21']
java: ['17', '21', '24']
steps:
- name: Set up JDK ${{ matrix.java }}
uses: actions/setup-java@v4
Expand Down Expand Up @@ -179,7 +179,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
java: ['17', '21']
java: ['17', '21', '24']
steps:
- name: Set up JDK ${{ matrix.java }}
uses: actions/setup-java@v4
Expand Down
1 change: 0 additions & 1 deletion apache-maven/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ under the License.
<dependency>
<groupId>org.eclipse.sisu</groupId>
<artifactId>org.eclipse.sisu.inject</artifactId>
<classifier>no_asm</classifier>
</dependency>
<dependency>
<groupId>com.google.inject</groupId>
Expand Down
2 changes: 1 addition & 1 deletion impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Contributor

@Pankraz76 Pankraz76 May 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx.

"Unexpected stdout[" + stdout.size() + "]=" + stdout + "; stderr[" + stderr.size() + "]=" + stderr);
}
return result;
Expand Down
4 changes: 2 additions & 2 deletions its/core-it-suite/src/test/resources/mng-7045/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ javax.enterprise.inject.Instance.class.getDeclaredMethod("stream")]]></script>
<dependency>
<groupId>org.apache.groovy</groupId>
<artifactId>groovy-ant</artifactId>
<version>4.0.15</version>
<version>4.0.26</version>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>org.apache.groovy</groupId>
<artifactId>groovy</artifactId>
<version>4.0.15</version>
<version>4.0.26</version>
<scope>runtime</scope>
</dependency>
</dependencies>
Expand Down
19 changes: 19 additions & 0 deletions its/core-it-suite/src/test/resources/mng-7160-extensionclassloader/extension/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,23 @@ under the License.
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.eclipse.sisu</groupId>
<artifactId>sisu-maven-plugin</artifactId>
<version>0.9.0.M3</version>
<executions>
<execution>
<id>index-project</id>
<goals>
<goal>main-index</goal>
<goal>test-index</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

</project>
14 changes: 14 additions & 0 deletions its/core-it-suite/src/test/resources/mng-7587-jsr330/plugin/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@
<release>17</release>
</configuration>
</plugin>
<plugin>
<groupId>org.eclipse.sisu</groupId>
<artifactId>sisu-maven-plugin</artifactId>
<version>0.9.0.M3</version>
<executions>
<execution>
<id>index-project</id>
<goals>
<goal>main-index</goal>
<goal>test-index</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-plugin-plugin</artifactId>
Expand Down
58 changes: 51 additions & 7 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ under the License.
<plexusXmlVersion>4.1.0</plexusXmlVersion>
<resolverVersion>2.0.8</resolverVersion>
<securityDispatcherVersion>4.1.0</securityDispatcherVersion>
<sisuVersion>0.9.0.M3</sisuVersion>
<sisuVersion>0.9.0-SNAPSHOT</sisuVersion>
<slf4jVersion>2.0.17</slf4jVersion>
<stax2ApiVersion>4.2.2</stax2ApiVersion>
<wagonVersion>3.5.3</wagonVersion>
Expand Down Expand Up @@ -411,12 +411,6 @@ under the License.
<artifactId>org.eclipse.sisu.inject</artifactId>
<version>${sisuVersion}</version>
</dependency>
<dependency>
<groupId>org.eclipse.sisu</groupId>
<artifactId>org.eclipse.sisu.inject</artifactId>
<version>${sisuVersion}</version>
<classifier>no_asm</classifier>
</dependency>
<dependency>
<groupId>jakarta.inject</groupId>
<artifactId>jakarta.inject-api</artifactId>
Expand Down Expand Up @@ -697,6 +691,33 @@ under the License.
</dependencies>
-->

<repositories>
<repository>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
<id>central-portal-snapshots</id>
<name>Central Portal Snapshots</name>
<url>https://central.sonatype.com/repository/maven-snapshots/</url>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
<id>central-portal-snapshots</id>
<name>Central Portal Snapshots</name>
<url>https://central.sonatype.com/repository/maven-snapshots/</url>
</pluginRepository>
</pluginRepositories>

<build>
<pluginManagement>
<plugins>
Expand Down Expand Up @@ -796,6 +817,29 @@ under the License.</licenseText>
</excludes>
</configuration>
</plugin>
<plugin>
<groupId>org.eclipse.sisu</groupId>
<artifactId>sisu-maven-plugin</artifactId>
<version>${sisuVersion}</version>
<dependencies>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>
<version>${asmVersion}</version>
</dependency>
</dependencies>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
Copy link
Contributor

@Pankraz76 Pankraz76 May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why cost of carry? the plugin seems not in use:

<dependencies>
<dependency>
<groupId>net.sourceforge.pmd</groupId>
<artifactId>pmd-core</artifactId>
<version>7.12.0</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</pluginManagement>
<plugins>
Expand Down