You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "sjvanrossum (via GitHub)" <gi...@apache.org> on 2025/05/12 10:32:10 UTC
[PR] [WIP] Refactor Java toolchain selection using Gradle toolchain management [beam]
sjvanrossum opened a new pull request, #34915:
URL: https://github.com/apache/beam/pull/34915
In-progress refactor of Java toolchain management.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment `fixes #<ISSUE NUMBER>` instead.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://github.com/apache/beam/blob/master/CONTRIBUTING.md#make-the-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Go+tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI or the [workflows README](https://github.com/apache/beam/blob/master/.github/workflows/README.md) to see a list of phrases to trigger workflows.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #34915: [Gradle] Refactor Java toolchain selection using Gradle toolchain management
URL: https://github.com/apache/beam/pull/34915
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34915:
URL: https://github.com/apache/beam/pull/34915#issuecomment-2876908105
Assigning reviewers:
R: @tvalentyn for label python.
R: @m-trieu for label java.
R: @shunping for label go.
R: @damccorm for label build.
R: @kennknowles for label website.
Note: If you would like to opt out of this review, comment `assign to next reviewer`.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093259883
##########
.github/workflows/beam_LoadTests_Java_CoGBK_Dataflow_V2_Batch_JavaVersions.yml:
##########
@@ -95,8 +95,7 @@ jobs:
with:
gradle-command: :sdks:java:testing:load-tests:run
arguments: |
- -PtestJavaVersion=${{ matrix.java_version }} \
- -Pjava${{ matrix.java_version }}Home=$JAVA_HOME_${{ matrix.java_version }}_X64 \
+ -PjavaVersion=${{ matrix.java_version }} \
Review Comment:
> it affect the JDK selection for all JavaCompile, JavaExec and Test tasks
>
> Using a single compiler version would hide these issues until the next time this build configuration changes the minimum supported JDK version.
Said differently, after this change it adds requirements for Beam to build on all supported Java versions (for components with versioning tests). This significantly increases scope of maintainence. Most importantly it would make new JDK support harder and delayed - when I did JDK21 support, at first Gradle 8.3- didn't support build on JDK21, but the work can proceed because we only need to make sure Beam runs on JDK21 fine. It won't be possible after this change.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34915:
URL: https://github.com/apache/beam/pull/34915#issuecomment-3485831474
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093422662
##########
.github/workflows/beam_LoadTests_Java_CoGBK_Dataflow_V2_Batch_JavaVersions.yml:
##########
@@ -95,8 +95,7 @@ jobs:
with:
gradle-command: :sdks:java:testing:load-tests:run
arguments: |
- -PtestJavaVersion=${{ matrix.java_version }} \
- -Pjava${{ matrix.java_version }}Home=$JAVA_HOME_${{ matrix.java_version }}_X64 \
+ -PjavaVersion=${{ matrix.java_version }} \
Review Comment:
> I think the next step would be to use the test suite plugin to copy the default test test suite target and configure each Test task to override javaLauncher, producing :path:to:project:test8 and :path:to:project:test11 for example.
Yeah, this sounds a good idea replacing `-PtestJavaVersion=xx -PjavaxxHome` that is used currently in java github action workflows involving testing java version
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093422662
##########
.github/workflows/beam_LoadTests_Java_CoGBK_Dataflow_V2_Batch_JavaVersions.yml:
##########
@@ -95,8 +95,7 @@ jobs:
with:
gradle-command: :sdks:java:testing:load-tests:run
arguments: |
- -PtestJavaVersion=${{ matrix.java_version }} \
- -Pjava${{ matrix.java_version }}Home=$JAVA_HOME_${{ matrix.java_version }}_X64 \
+ -PjavaVersion=${{ matrix.java_version }} \
Review Comment:
> I think the next step would be to use the test suite plugin to copy the default test test suite target and configure each Test task to override javaLauncher, producing :path:to:project:test8 and :path:to:project:test11 for example.
Yeah, this sounds a good idea replacing `-PtestJavaVersion=xx -PjavaxxHome` that is used currently
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34915:
URL: https://github.com/apache/beam/pull/34915#issuecomment-3451111679
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093684778
##########
.github/actions/setup-environment-action/action.yml:
##########
@@ -70,11 +70,16 @@ runs:
tox-${{ runner.os }}-py${{ inputs.python-version == 'default' && '39' || inputs.python-version }}-
- name: Install Java
- if: ${{ inputs.java-version != '' }}
+ if: ${{ inputs.java-version != 'default' && '11' }}
Review Comment:
It's using zulu jdk 11 actually - https://github.com/apache/beam/blob/cbe100a53a34abdea80ac2335d36d2297d0f578c/.github/gh-actions-self-hosted-runners/arc/images/Dockerfile#L43
> OpenJDK it's also subtly different from the Temurin release
Yeah this is a good point. Strictly speaking we should use tumerin there
But Beam repo has many other tests doesn't require JDK. It justs need a java runtime to run gradle. I don't think we want to install JDK on every single workflow run (e.g. those python/go/website tests). Arguably Java package is much heavier than Python/Go
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093684778
##########
.github/actions/setup-environment-action/action.yml:
##########
@@ -70,11 +70,16 @@ runs:
tox-${{ runner.os }}-py${{ inputs.python-version == 'default' && '39' || inputs.python-version }}-
- name: Install Java
- if: ${{ inputs.java-version != '' }}
+ if: ${{ inputs.java-version != 'default' && '11' }}
Review Comment:
It's using zulu jdk 11 actually - https://github.com/apache/beam/blob/cbe100a53a34abdea80ac2335d36d2297d0f578c/.github/gh-actions-self-hosted-runners/arc/images/Dockerfile#L43
> OpenJDK it's also subtly different from the Temurin release
Yeah this is a good point. Strictly speaking we should use tumerin there
But Beam repo has many other tests doesn't require JDK. It justs need a java runtime to run gradle. I don't think we want to install JDK on every single workflow run (e.g. those python/go/website tests)
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093680512
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1845,21 +1674,7 @@ class BeamModulePlugin implements Plugin<Project> {
project.ext.includeInJavaBom = configuration.publish
project.ext.exportJavadoc = configuration.exportJavadoc
- boolean publishEnabledByCommand = isRelease(project) || project.hasProperty('publishing')
- if (forkJavaVersion == '') {
- // project needs newer version and not served.
- // If not publishing ,disable the project. Otherwise, fail the build
- def msg = "project ${project.name} needs newer Java version to compile. Consider set -Pjava${project.javaVersion}Home"
- if (publishEnabledByCommand) {
- throw new GradleException("Publish enabled but " + msg + ".")
Review Comment:
Thanks. with this change GitHub Action now passed.
However my original question remains. `./gradlew :sdks:java:container:agent:build` on java8 still failing. My point is we don't want it to fail. User or IDE (especially true for IntelliJ) often invoke a single `./gradlew <command>` like `./gradlew build` instead of specifying module. Currently it works only if a single java8/11 installed. I don't think we want to break them.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093259883
##########
.github/workflows/beam_LoadTests_Java_CoGBK_Dataflow_V2_Batch_JavaVersions.yml:
##########
@@ -95,8 +95,7 @@ jobs:
with:
gradle-command: :sdks:java:testing:load-tests:run
arguments: |
- -PtestJavaVersion=${{ matrix.java_version }} \
- -Pjava${{ matrix.java_version }}Home=$JAVA_HOME_${{ matrix.java_version }}_X64 \
+ -PjavaVersion=${{ matrix.java_version }} \
Review Comment:
> it affect the JDK selection for all JavaCompile, JavaExec and Test tasks
>
> Using a single compiler version would hide these issues until the next time this build configuration changes the minimum supported JDK version.
Said differently, after this change it adds requirements for Beam to build on all supported Java versions (for components with versioning tests. Previously, it only requires tests build and run on supported Java versions, that is aligned with user adoption scenarios). This significantly increases scope of maintainence. Most importantly it would make new JDK support harder and delayed - when I did JDK21 support, at first Gradle 8.3- didn't support build on JDK21, but the work can proceed because we only need to make sure Beam runs on JDK21 fine. It won't be possible after this change.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34915:
URL: https://github.com/apache/beam/pull/34915#issuecomment-3228073241
This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093680512
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1845,21 +1674,7 @@ class BeamModulePlugin implements Plugin<Project> {
project.ext.includeInJavaBom = configuration.publish
project.ext.exportJavadoc = configuration.exportJavadoc
- boolean publishEnabledByCommand = isRelease(project) || project.hasProperty('publishing')
- if (forkJavaVersion == '') {
- // project needs newer version and not served.
- // If not publishing ,disable the project. Otherwise, fail the build
- def msg = "project ${project.name} needs newer Java version to compile. Consider set -Pjava${project.javaVersion}Home"
- if (publishEnabledByCommand) {
- throw new GradleException("Publish enabled but " + msg + ".")
Review Comment:
Thanks. with this change GitHub Action now passed.
However my original question remains. `./gradlew :sdks:java:container:agent:build` on java8 still failing. My point is we don't want it to fail. User or IDE (especially true for IntelliJ) often invoke a single `./gradlew <command>` like `./gradlew build` instead of specifying module. Currently it works if only a single java8/11 installed. I don't think we want to break them.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "sjvanrossum (via GitHub)" <gi...@apache.org>.
sjvanrossum commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093213865
##########
buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy:
##########
@@ -1845,21 +1674,7 @@ class BeamModulePlugin implements Plugin<Project> {
project.ext.includeInJavaBom = configuration.publish
project.ext.exportJavadoc = configuration.exportJavadoc
- boolean publishEnabledByCommand = isRelease(project) || project.hasProperty('publishing')
- if (forkJavaVersion == '') {
- // project needs newer version and not served.
- // If not publishing ,disable the project. Otherwise, fail the build
- def msg = "project ${project.name} needs newer Java version to compile. Consider set -Pjava${project.javaVersion}Home"
- if (publishEnabledByCommand) {
- throw new GradleException("Publish enabled but " + msg + ".")
Review Comment:
I added a commit to select the minimum language version when the requested language version is incompatible. When a suitable toolchain can't be discovered this will still result in a build failure.
```
> Configure project :sdks:java:container:agent
Requested a toolchain for language version 11 because a toolchain for language version 8 is incompatible with the minimum supported language version.
FAILURE: Build failed with an exception.
* What went wrong:
Could not determine the dependencies of task ':sdks:java:container:agent:compileJava'.
> Failed to query the value of extension 'errorprone' property 'enabled'.
> Failed to calculate the value of task ':sdks:java:container:agent:compileJava' property 'javaCompiler'.
> No matching toolchains found for requested specification: {languageVersion=11, vendor=any, implementation=vendor-specific} for MAC_OS on aarch64.
> No locally installed toolchains match and toolchain auto-provisioning is not enabled.
* Try:
> Learn more about toolchain auto-detection at https://docs.gradle.org/8.4/userguide/toolchains.html#sec:auto_detection.
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Get more help at https://help.gradle.org.
BUILD FAILED in 7s
```
If a compatible toolchain can be discovered (e.g. requested Java 8, but provided both Java 8 and Java 11 toolchain locations) then it still prints a warning, but the build will succeed with a successful build of the affected project instead of skipping it. The requested build target depends on that incompatible target so we should ensure that it builds.
```
> Configure project :sdks:java:container:agent
Requested a toolchain for language version 11 because a toolchain for language version 8 is incompatible with the minimum supported language version.
BUILD SUCCESSFUL in 8s
```
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Gradle] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on code in PR #34915:
URL: https://github.com/apache/beam/pull/34915#discussion_r2093259883
##########
.github/workflows/beam_LoadTests_Java_CoGBK_Dataflow_V2_Batch_JavaVersions.yml:
##########
@@ -95,8 +95,7 @@ jobs:
with:
gradle-command: :sdks:java:testing:load-tests:run
arguments: |
- -PtestJavaVersion=${{ matrix.java_version }} \
- -Pjava${{ matrix.java_version }}Home=$JAVA_HOME_${{ matrix.java_version }}_X64 \
+ -PjavaVersion=${{ matrix.java_version }} \
Review Comment:
> it affect the JDK selection for all JavaCompile, JavaExec and Test tasks
>
> Using a single compiler version would hide these issues until the next time this build configuration changes the minimum supported JDK version.
Said differently, after this change it adds requirements for Beam to build on all supported Java versions (for components with versioning tests. Previously, it only requires tests build and run on supported Java versions, that is aligned with user adoption scenarios). This significantly increases scope of maintainence. Most importantly it would make new JDK support harder and delayed - when I did JDK21 support, at first Gradle 8.3- didn't support build on JDK21, but the work wasn't blocked by Gradle 8.4 release, and can proceed because we only need to make sure Beam runs on JDK21 fine. It won't be possible after this change.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [WIP] Refactor Java toolchain selection using Gradle toolchain management [beam]
Posted by "sjvanrossum (via GitHub)" <gi...@apache.org>.
sjvanrossum closed pull request #34915: [WIP] Refactor Java toolchain selection using Gradle toolchain management
URL: https://github.com/apache/beam/pull/34915
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org