chore: setup-java Oracle action supports Java 21+ only#3215
Merged
Conversation
WalkthroughThe pull request updates the Java version handling in the matrix configuration. The constant Changes
Sequence Diagram(s)sequenceDiagram
participant M as Matrix Config
participant V as Version Checker
participant D as Distribution Setter
M->>V: Check java_version
V-->>M: Return (java_version === eaJava or java_version >= 21)
alt Condition met
M->>D: Set java_distribution to 'oracle'
else
M->>D: No change to java_distribution
end
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
juherr
approved these changes
Mar 6, 2025
Contributor
Author
|
pax waving does not support java 24: |
…bytecode waving for services Previously, java services required bytecode waving in the runtime to expose as osgi servies. Now we use pax-logging module so it does it automatically
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
testng-test-osgi/src/test/java/org/testng/test/osgi/DefaultTestngOsgiOptions.java(1 hunks)testng-test-osgi/testng-test-osgi-build.gradle.kts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: 24, oracle, ubuntu, America/New_York, tr_TR
- GitHub Check: 21, liberica, macos, America/New_York, tr_TR, stress JIT
- GitHub Check: 17, zulu, same hashcode, windows, Pacific/Chatham, fr_FR
- GitHub Check: 17, microsoft, macos, Pacific/Chatham, tr_TR, stress JIT
- GitHub Check: 11, microsoft, macos, UTC, tr_TR, stress JIT
- GitHub Check: 11, liberica, windows, America/New_York, fr_FR, stress JIT
- GitHub Check: 11, liberica, macos, UTC, ru_RU
🔇 Additional comments (3)
testng-test-osgi/src/test/java/org/testng/test/osgi/DefaultTestngOsgiOptions.java (1)
43-44: Consistent update to OSGi optionsThe addition of Pax Logging bundles aligns with the changes in the build script, ensuring a consistent approach to logging in the OSGi environment.
Note that even with the switch to Pax Logging, the code still uses the Logback configuration file (line 42), which is appropriate because Pax Logging uses Logback as its underlying implementation in this setup.
testng-test-osgi/testng-test-osgi-build.gradle.kts (2)
48-50: Introduce Pax Logging API dependencyThe addition of
testRuntimeOnly("org.ops4j.pax.logging:pax-logging-api:2.2.8") { because("It will actually be used in osgi for logging") }provides the necessary logging API for the OSGi environment. Verify that version 2.2.8 is adequately tested for compatibility with Java 21+ and that it integrates well with your overall logging configuration.
51-53: Integrate Pax Logging Logback adapterThe new dependency
testRuntimeOnly("org.ops4j.pax.logging:pax-logging-logback:2.2.8") { because("It will actually be used in osgi for logging, basic slf4j+logback is hard to launch in osgi, see https://stackoverflow.com/a/77867804") }completes the switch from the removed Logback dependencies by providing an appropriate logback adapter within the Pax Logging framework. Please ensure that any existing logging configurations (for example, the system property for
logback.configurationFileset later in the build script) are still applicable and do not conflict with this new setup.
Comment on lines
+48
to
+53
| testRuntimeOnly("org.ops4j.pax.logging:pax-logging-api:2.2.8") { | ||
| because("It will actually be used in osgi for logging") | ||
| } | ||
| testRuntimeOnly("org.ops4j.pax.logging:pax-logging-logback:2.2.8") { | ||
| because("It will actually be used in osgi for logging, basic slf4j+logback is hard to launch in osgi, see https://stackoverflow.com/a/77867804") | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good shift to specialized OSGi logging framework
The addition of Pax Logging dependencies is a good approach to address logging in OSGi environments. This change likely helps address the Java 24 compatibility issues mentioned in the PR objectives (ClassFormatError in the OSGi environment).
Some notes:
- The change from
testImplementation(for the removed Logback dependencies) totestRuntimeOnlycorrectly reflects that these are runtime-only dependencies - The reference to Stack Overflow provides helpful context for why this change is necessary
🌐 Web query:
Is Pax Logging 2.2.8 compatible with Java 24?
💡 Result:
Pax Logging 2.2.8 does not appear to exist based on the provided sources, which reference versions up to 2.0.8 for Pax Logging 2.x releases[1][2]. For compatibility with modern Java versions:
- Pax Logging 2.0.8 (the latest 2.x version mentioned) includes fixes for JDK11 compatibility[1], but there is no explicit information about support for newer Java versions like Java 24.
- Java 24 is not a released version as of March 2025 (the current date in the context)[1][4]. Software compatibility with unreleased JDK versions is typically unconfirmed unless explicitly stated by the project.
For future Java versions, check the official Pax Logging release notes for updates. Current releases focus on OSGi R7 logging standards and integration with frameworks like Log4j2 and Logback[1][3].
Citations:
- 1: https://ops4j1.jira.com/wiki/spaces/paxlogging/overview
- 2: https://javadoc.io/doc/org.ops4j.pax.logging/pax-logging-log4j2/latest/index.html
- 3: https://github.com/ops4j/org.ops4j.pax.logging
- 4: https://karaf.apache.org/download
- 5: https://docs.redhat.com/en/documentation/red_hat_fuse/7.1/html/apache_camel_component_reference/paxlogging-component
- 6: pax-logging-api should not bundle "foreign" Java classes. ops4j/org.ops4j.pax.logging#519
- 7: https://github.com/ops4j/org.ops4j.pax.logging/blob/main/RELEASE-NOTES.html
- 8: https://mvnrepository.com/artifact/org.ops4j.pax.logging/pax-logging-api
- 9: https://community.openhab.org/t/effectively-available-log-appenders-slf4j-vs-log4j2-vs-osgi-vs-ops4j-pax-logging/118054
- 10: https://deps.dev/maven/org.ops4j.pax.logging:pax-logging-log4j2/1.10.1
Action Required: Verify Dependency Versions and JDK Compatibility
- The change from
testImplementationtotestRuntimeOnlyis appropriate for runtime-only logging dependencies in OSGi. - However, web research indicates that version 2.2.8 of the Pax Logging dependencies does not appear to exist—sources only list versions up to 2.0.8. Please verify if the intended version is a custom snapshot or if it needs to be updated.
- Additionally, the comment about addressing "Java 24 compatibility issues" is misleading since Java 24 has not been released as of March 2025. Consider revising this note to reflect compatibility with supported JDK versions or remove the reference until Java 24 is officially available.
- The provided Stack Overflow reference remains useful context for the logging challenges in OSGi.
Member
|
@krmahadevan wdyt? |
krmahadevan
approved these changes
Mar 7, 2025
This was referenced Dec 30, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit