gemini-code-assist[bot] commented on code in PR #38974:
URL: https://github.com/apache/beam/pull/38974#discussion_r3430818163
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1645,6 +1656,10 @@ class BeamModulePlugin implements Plugin<Project> {
useJUnit()
executable = "${testJavaHome}/bin/java"
}
+ // redirect java exec tasks (expansion service, run, shadowJar execs)
to specified JDK
+ project.tasks.withType(JavaExec).configureEach {
+ executable = "${testJavaHome}/bin/java"
+ }
Review Comment:
If `testJavaHome` is null or empty (e.g., if the required Java version could
not be resolved), configuring the `JavaExec` tasks with
`${testJavaHome}/bin/java` will result in an invalid path like `null/bin/java`,
causing tasks to fail with a `FileNotFoundException` or similar error.
Adding a null/empty check before configuring the executable ensures
defensive programming and allows the tasks to gracefully fall back to the
default JVM if the specified JDK is not available.
```
// redirect java exec tasks (expansion service, run, shadowJar
execs) to specified JDK
if (testJavaHome) {
project.tasks.withType(JavaExec).configureEach {
executable = "${testJavaHome}/bin/java"
}
}
```
##########
.github/actions/setup-environment-action/action.yml:
##########
@@ -69,9 +69,15 @@ runs:
tox-${{ runner.os }}-py${{ inputs.python-version == 'default' &&
'310' || inputs.python-version }}-${{ hashFiles('sdks/python/tox.ini') }}-
tox-${{ runner.os }}-py${{ inputs.python-version == 'default' &&
'310' || inputs.python-version }}-
+ - name: Install Java 17 fallback
+ if: ${{ inputs.java-version == 'default' }}
+ uses: actions/setup-java@v4
+ with:
+ distribution: 'temurin'
+ java-version: '17'
Review Comment:
The fallback installation of Java 17 will set the `JAVA_HOME` environment
variable, but this is immediately overwritten by the subsequent "Install Java"
step which sets `JAVA_HOME` to Java 11. Since `actions/setup-java` does not
automatically set architecture-specific environment variables like
`JAVA_HOME_17_X64` or `JAVA_HOME_17_ARM64`, the installed Java 17 path is lost
and cannot be discovered by Gradle on environments where Java 17 is not
pre-installed.
To fix this, we should explicitly export the Java 17 path to
`JAVA_HOME_17_X64` and `JAVA_HOME_17_ARM64` in the environment right after
installing it.
```yaml
- name: Install Java 17 fallback
if: ${{ inputs.java-version == 'default' }}
uses: actions/setup-java@v4
with:
distribution: 'temurin'
java-version: '17'
- name: Set Java 17 Home Env
if: ${{ inputs.java-version == 'default' }}
run: |
echo "JAVA_HOME_17_X64=$JAVA_HOME" >> $GITHUB_ENV
echo "JAVA_HOME_17_ARM64=$JAVA_HOME" >> $GITHUB_ENV
shell: bash
```
--
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]
