sboorlagadda commented on code in PR #7930:
URL: https://github.com/apache/geode/pull/7930#discussion_r2384239909
##########
build-tools/scripts/src/main/groovy/geode-java.gradle:
##########
@@ -31,17 +31,26 @@ dependencies {
}
String javaVersion = System.properties['java.version']
-if (javaVersion.startsWith("1.8.0") &&
javaVersion.split("-")[0].split("_")[1].toInteger() < 121) {
- throw new GradleException("Java version 1.8.0_121 or later required, but was
" + javaVersion)
+def versionMajor = JavaVersion.current().majorVersion.toInteger()
+if (versionMajor < 17) {
+ throw new GradleException("Java version 17 or later required, but was " +
javaVersion)
}
// apply compiler options
gradle.taskGraph.whenReady({ graph ->
tasks.withType(JavaCompile).each { javac ->
javac.configure {
- sourceCompatibility '1.8'
- targetCompatibility '1.8'
+ sourceCompatibility '17'
+ targetCompatibility '17'
Review Comment:
sourceCompatibility = 17 is repeated again inside whenReady. May be we can
use toolchains; it’s the canonical way for JDK 17?
```
diff --git a/build-tools/scripts/src/main/groovy/geode-java.gradle
b/build-tools/scripts/src/main/groovy/geode-java.gradle
index bfd667d160a3..4bb2f5f3ce2e 100644
--- a/build-tools/scripts/src/main/groovy/geode-java.gradle
+++ b/build-tools/scripts/src/main/groovy/geode-java.gradle
@@ -20,12 +20,14 @@ plugins {
id 'org.apache.geode.gradle.geode-dependency-constraints'
}
-sourceCompatibility = 17
-targetCompatibility = 17
+// Prefer toolchains over hard-coding source/target; ensures reproducible
builds on CI/dev
+java {
+ toolchain { languageVersion = JavaLanguageVersion.of(17) }
+}
compileJava.options.encoding = 'UTF-8'
dependencies {
implementation(platform(project(':boms:geode-all-bom')))
implementation('org.apache.logging.log4j:log4j-api')
}
String javaVersion = System.properties['java.version']
@@ -37,27 +39,13 @@ if (versionMajor < 17) {
throw new GradleException("Java version 17 or later required, but was " +
javaVersion)
}
-// apply compiler options
-gradle.taskGraph.whenReady({ graph ->
- tasks.withType(JavaCompile).each { javac ->
- javac.configure {
- sourceCompatibility '17'
- targetCompatibility '17'
- options.encoding = 'UTF-8'
- options.compilerArgs.addAll([
-
'--add-exports=java.management/com.sun.jmx.remote.security=ALL-UNNAMED',
- '--add-exports=java.base/sun.nio.ch=ALL-UNNAMED',
- '--add-exports=jdk.unsupported/sun.misc=ALL-UNNAMED',
- '--add-exports=jdk.unsupported/sun.reflect=ALL-UNNAMED',
- '-Xlint:-removal',
- '-Xlint:-deprecation'
- ])
- }
- javac.options.incremental = true
- javac.options.fork = true
- }
-})
+// Apply modern, minimal compiler settings
+tasks.withType(JavaCompile).configureEach {
+ options.release = 17
+ options.encoding = 'UTF-8'
+ options.incremental = true
+ options.fork = true
+}
// ... (rest of file unchanged)
```
##########
geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java:
##########
@@ -98,7 +98,7 @@ public ResultModel deploy(
Set<DistributedMember> targetMembers;
targetMembers = findMembers(groups, null);
- List<List<Object>> results = new LinkedList<>();
+ List<Object> results = new LinkedList<>();
Review Comment:
Does any caller/formatter expecting a per-member nested list will break? You
changed `List<List<Object>> → List<Object>` and now collect CliFunctionResults
directly.
Should we either keep the old external type (List<List<Object>>)? Add/adjust
tests for the new shape.
```
diff --git
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
index 063ab5dbccd8..b5f3f4d1f9a7 100644
---
a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
+++
b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommand.java
@@ -98,7 +98,7 @@ public ResultModel deploy(
Set<DistributedMember> targetMembers;
targetMembers = findMembers(groups, null);
- List<Object> results = new LinkedList<>();
+ List<List<Object>> results = new LinkedList<>();
ManagementAgent agent = ((SystemManagementService)
getManagementService()).getManagementAgent();
RemoteStreamExporter exporter = agent.getRemoteStreamExporter();
@@ -123,9 +123,15 @@ public ResultModel deploy(
return result;
}
- private List<Object> deployJars(List<String> jarFullPaths,
+ private List<List<Object>> deployJars(List<String> jarFullPaths,
Set<DistributedMember> targetMembers,
- List<Object> results,
+ List<List<Object>> results,
RemoteStreamExporter exporter)
throws FileNotFoundException, java.rmi.RemoteException {
@@ -155,11 +161,19 @@ private List<Object> deployJars(List<String>
jarFullPaths,
new Object[] {jarNames, remoteStreams}, member);
@SuppressWarnings("unchecked")
- final List<CliFunctionResult> resultCollectorResult =
- (List<CliFunctionResult>) resultCollector.getResult();
- results.addAll(resultCollectorResult);
+ final List<CliFunctionResult> rc =
+ (List<CliFunctionResult>) resultCollector.getResult();
+ // Preserve legacy nested structure: one List<Object> per member
result
+ for (CliFunctionResult r : rc) {
+ List<Object> row = new ArrayList<>(3);
+ row.add(r.getMemberIdOrName());
+ row.add(r.getStatus()); // boolean/success flag
+ row.add(r.getMessage()); // human-readable message
+ results.add(row);
+ }
} finally {
for (RemoteInputStream ris : remoteStreams) {
try {
ris.close(true);
} catch (IOException ignore) { }
}
}
}
- return results;
+ return results;
}
```
##########
gradle.properties:
##########
@@ -64,18 +64,18 @@ geodeDockerImageName = geode:develop
#JAVA_HOME to be used for compilation
compileJVM=
-compileJVMVer=8
+compileJVMVer=17
#JAVA_HOME to be used by tests
testJVM=
-testJVMVer=8
+testJVMVer=17
repeat = 100
org.gradle.caching = true
org.gradle.configureondemand = false
org.gradle.daemon = true
-org.gradle.jvmargs = -Xmx3g
+org.gradle.jvmargs = -Xmx3g
--add-exports=java.base/sun.security.x509=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
--add-exports=java.management/com.sun.jmx.remote.security=ALL-UNNAMED
Review Comment:
You added many --add-exports=... to org.gradle.jvmargs. That forces all
Gradle runs (including downstream users building Geode as a dependency) to open
JDK internals.
Should we move these flags to the narrowest places that need them,
e.g. specific subprojects’ `Test/JavaExec/Javadoc` tasks or a shared
`testing.gradle` that only configures test tasks:
```
// example: only where needed
tasks.withType(Test).configureEach {
jvmArgs '--add-exports=java.base/sun.security.x509=ALL-UNNAMED'
}
javadoc {
options.addStringOption('-add-exports','java.base/sun.security.x509=ALL-UNNAMED')
}
```
##########
geode-gfsh/build.gradle:
##########
@@ -40,6 +40,10 @@ dependencies {
implementation('com.fasterxml.jackson.core:jackson-databind')
implementation('io.swagger.core.v3:swagger-annotations')
+ // JAXB dependencies needed for Java 11+
+ implementation('javax.xml.bind:jaxb-api')
+ implementation('com.sun.xml.bind:jaxb-impl')
Review Comment:
Should we also add activation? On Java 11+, JAXB is no longer in the JDK.
This combo can work, but you often also need activation. Newer stacks prefer
Jakarta (jakarta.xml.bind).
```
implementation 'javax.xml.bind:jaxb-api:2.3.1'
runtimeOnly 'com.sun.xml.bind:jaxb-impl:2.3.3'
runtimeOnly 'com.sun.activation:javax.activation:1.2.0'
```
##########
geode-core/src/main/java/org/apache/geode/cache/query/internal/QCompiler.java:
##########
@@ -148,9 +148,9 @@ public void compileOrderByClause(final int numOfChildren) {
}
public void compileGroupByClause(final int numOfChildren) {
- final List<CompiledPath> list = new ArrayList<>();
+ final List<CompiledValue> list = new ArrayList<>();
Review Comment:
Question: does the evaluator elsewhere require CompiledPaths?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
