You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "nielsbasjes (via GitHub)" <gi...@apache.org> on 2024/01/21 12:34:01 UTC
[PR] Avro 3716 build under java21 [avro]
nielsbasjes opened a new pull request, #2699:
URL: https://github.com/apache/avro/pull/2699
https://issues.apache.org/jira/browse/AVRO-3716
The base goal is to build Avro under the latest available JDK and plugins while maintaining Java 8 compatibility.
This changes a lot in the JAVA build ...
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1472521744
##########
lang/java/avro/pom.xml:
##########
@@ -55,7 +55,42 @@
</includes>
</resource>
</resources>
+ <testResources>
+ <testResource>
+ <directory>src/test/resources</directory>
+ </testResource>
+ <testResource>
+ <directory>../../../share/</directory>
+ <includes>
+ <include>schemas/**</include>
+ <include>test/**</include>
+ </includes>
+ <targetPath>share/</targetPath>
+ </testResource>
+ </testResources>
+
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-toolchains-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>Build using JDK 21</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>toolchain</goal>
+ </goals>
+ <configuration>
+ <toolchains>
+ <jdk>
+ <version>21</version>
+ </jdk>
+ </toolchains>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
Review Comment:
Originally I had different JDK versions for various modules. Mainly because of the Unsafe ... which I eventually kicked.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "opwvhk (via GitHub)" <gi...@apache.org>.
opwvhk commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471415514
##########
.github/workflows/codeql-java-analysis.yml:
##########
@@ -70,17 +70,34 @@ jobs:
# queries: ./path/to/local/query, your-org/your-repo/queries@main
queries: +security-and-quality
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
Review Comment:
Although I understand that using hashes pinpoint the exact version, we can use any tag. The `setup-java` action for example, offers the tags `v4` and `v3`, but also `v4.0.0`, `v3.13.0`, etc.
Which do we prefer, taking into account build stability, readability, etc.?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on PR #2699:
URL: https://github.com/apache/avro/pull/2699#issuecomment-1913360625
Building the software using different JDK versions may yield different code than what is actually released; So the code is built only once under a specific JDK version and that is tested against compatibility with all other JDKs.
Summary of what I have changed:
- The build requires Maven 3.9.6 and JDK 8, 11, 17 and 21 (yes, all of them) installed with the correct toolchains config to be present.
- The build (Maven 3.9.6) must run under Java 21 so the latest plugins can be used.
- Depending on the module the maximum JDK is selected to build the software with a release target of Java 8.
- Using the invoker plugin all tests of the main avro module are rerun in all mentioned JDKs (yes, all tests).
- The interoperability tests were restructured into a separate module which generates van validates the files.
- An extra profile `skipQuality` which disables all testing and code quality verification.
Rough edges:
- The code relating to the Unsafe had to be removed because building it under Java 9+ meant the need for specific compiler flags that disallowed targeting Java 8.
- Some tests required Mockito which now need Java 11+.
- Some tests rely on tools.jar which no longer exists in java 9+
- Several tests relating to reflection fail on newer Java versions because it tried to access internal fields in different modules.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "opwvhk (via GitHub)" <gi...@apache.org>.
opwvhk commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471395413
##########
.github/workflows/test-lang-java.yml:
##########
@@ -35,52 +35,55 @@ concurrency:
jobs:
test:
- name: Java ${{ matrix.java }} Test
+ name: 'Java Test'
runs-on: ubuntu-latest
- strategy:
- matrix:
- java:
- - '8'
- - '11'
- - '17'
- - '21'
- - '22-ea'
steps:
- - uses: actions/checkout@v4
+ - name: 'Checkout sourcecode'
+ uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- - name: Cache Local Maven Repository
- uses: actions/cache@v4
+ - name: 'Cache Local Maven Repository'
+ uses: actions/cache@88522ab9f39a2ea568f7027eddc7d8d8bc9d59c8 # v3.3.1
Review Comment:
Why do we need to go from v4 to v3.3.1?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "opwvhk (via GitHub)" <gi...@apache.org>.
opwvhk commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471461457
##########
share/docker/m2/toolchains.xml:
##########
@@ -0,0 +1,56 @@
+<?xml version="1.0" encoding="UTF8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ https://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+-->
+<toolchains>
+ <toolchain>
+ <type>jdk</type>
+ <provides>
+ <version>8</version>
+ </provides>
+ <configuration>
+ <jdkHome>/usr/lib/jvm/java-8-openjdk-amd64</jdkHome>
Review Comment:
Do we want to use OpenJDK 8 (which is completely unsupported), or Azure Azul (which is supported until at least 2030)?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1472525221
##########
share/docker/m2/toolchains.xml:
##########
@@ -0,0 +1,56 @@
+<?xml version="1.0" encoding="UTF8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one
+ ~ or more contributor license agreements. See the NOTICE file
+ ~ distributed with this work for additional information
+ ~ regarding copyright ownership. The ASF licenses this file
+ ~ to you under the Apache License, Version 2.0 (the
+ ~ "License"); you may not use this file except in compliance
+ ~ with the License. You may obtain a copy of the License at
+ ~
+ ~ https://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+-->
+<toolchains>
+ <toolchain>
+ <type>jdk</type>
+ <provides>
+ <version>8</version>
+ </provides>
+ <configuration>
+ <jdkHome>/usr/lib/jvm/java-8-openjdk-amd64</jdkHome>
Review Comment:
I just found that I didn't change this JDK version. I only added the required toolchains.xml file to reflect the already present JDK. In this case OpenJDK 8.
Note that this version is ONLY used for testing, no software is built with it.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes merged PR #2699:
URL: https://github.com/apache/avro/pull/2699
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "iemejia (via GitHub)" <gi...@apache.org>.
iemejia commented on PR #2699:
URL: https://github.com/apache/avro/pull/2699#issuecomment-1919150291
Hi, I am busy until the second week of february so I won't be able to give any feedback on this. I will take a look afterwards in case the PR is still ongoing.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1469330232
##########
.github/workflows/codeql-java-analysis.yml:
##########
@@ -70,17 +70,34 @@ jobs:
# queries: ./path/to/local/query, your-org/your-repo/queries@main
queries: +security-and-quality
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
+ with:
+ distribution: 'temurin'
+ java-version: |
+ 8
Review Comment:
In this change set I went for just updating the build environment while maintaining backwards compatibility. As you can see it is possible.
Although fully dropping Java 8 would make it all a lot easier.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1469353829
##########
.github/workflows/test-lang-java.yml:
##########
@@ -90,28 +93,37 @@ jobs:
restore-keys: |
${{ runner.os }}-maven-
- - name: Setup Java
- uses: actions/setup-java@v4
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
with:
distribution: 'temurin'
- java-version: ${{ matrix.java }}
+ java-version: |
+ 8
+ 11
+ 17
+ 21
+
+ - name: 'Setup Maven 3.9.6'
+ uses: stCarolas/setup-maven@07fbbe97d97ef44336b7382563d66743297e442f # v4.5
+ with:
+ maven-version: 3.9.6
- - name: Setup Python for Generating Input Data
- uses: actions/setup-python@v5
+ - name: 'Setup Python for Generating Input Data'
+ uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # v5.0.0
- - name: Apt Install Compression Libs Required by Python
+ - name: 'Apt Install Compression Libs Required by Python'
Review Comment:
I know, but I really want all code to look consistent and readable. And by having these quotes the syntax highlighting in my IDE (IntelliJ) makes it visually better to read (different colors).
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on PR #2699:
URL: https://github.com/apache/avro/pull/2699#issuecomment-1921215596
I have squashed the commits. If the build passed I'm merging it.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1472505964
##########
.github/workflows/test-lang-java.yml:
##########
@@ -35,52 +35,55 @@ concurrency:
jobs:
test:
- name: Java ${{ matrix.java }} Test
+ name: 'Java Test'
runs-on: ubuntu-latest
- strategy:
- matrix:
- java:
- - '8'
- - '11'
- - '17'
- - '21'
- - '22-ea'
steps:
- - uses: actions/checkout@v4
+ - name: 'Checkout sourcecode'
+ uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- - name: Cache Local Maven Repository
- uses: actions/cache@v4
+ - name: 'Cache Local Maven Repository'
+ uses: actions/cache@88522ab9f39a2ea568f7027eddc7d8d8bc9d59c8 # v3.3.1
Review Comment:
I changed this when the 4.0.0 waas not yet released. Then later did a rebase onto main where dependabot had updated this.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471476060
##########
.github/workflows/test-lang-java.yml:
##########
@@ -35,52 +35,55 @@ concurrency:
jobs:
test:
- name: Java ${{ matrix.java }} Test
+ name: 'Java Test'
runs-on: ubuntu-latest
- strategy:
- matrix:
- java:
- - '8'
- - '11'
- - '17'
- - '21'
- - '22-ea'
steps:
- - uses: actions/checkout@v4
+ - name: 'Checkout sourcecode'
+ uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
- - name: Cache Local Maven Repository
- uses: actions/cache@v4
+ - name: 'Cache Local Maven Repository'
+ uses: actions/cache@88522ab9f39a2ea568f7027eddc7d8d8bc9d59c8 # v3.3.1
Review Comment:
Oops
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471474943
##########
lang/java/ipc/pom.xml:
##########
@@ -54,6 +54,26 @@
</resource>
</resources>
<plugins>
+ <!--<plugin>-->
+ <!-- <groupId>org.apache.maven.plugins</groupId>-->
+ <!-- <artifactId>maven-toolchains-plugin</artifactId>-->
+ <!-- <executions>-->
+ <!-- <execution>-->
+ <!-- <id>Build using JDK 21</id>-->
+ <!-- <phase>validate</phase>-->
+ <!-- <goals>-->
+ <!-- <goal>toolchain</goal>-->
+ <!-- </goals>-->
+ <!-- <configuration>-->
+ <!-- <toolchains>-->
+ <!-- <jdk>-->
+ <!-- <version>21</version>-->
+ <!-- </jdk>-->
+ <!-- </toolchains>-->
+ <!-- </configuration>-->
+ <!-- </execution>-->
+ <!-- </executions>-->
+ <!--</plugin>-->
Review Comment:
Is probably a mistake of mine
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "opwvhk (via GitHub)" <gi...@apache.org>.
opwvhk commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471439371
##########
lang/java/avro/pom.xml:
##########
@@ -55,7 +55,42 @@
</includes>
</resource>
</resources>
+ <testResources>
+ <testResource>
+ <directory>src/test/resources</directory>
+ </testResource>
+ <testResource>
+ <directory>../../../share/</directory>
+ <includes>
+ <include>schemas/**</include>
+ <include>test/**</include>
+ </includes>
+ <targetPath>share/</targetPath>
+ </testResource>
+ </testResources>
+
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-toolchains-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>Build using JDK 21</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>toolchain</goal>
+ </goals>
+ <configuration>
+ <toolchains>
+ <jdk>
+ <version>21</version>
+ </jdk>
+ </toolchains>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
Review Comment:
The toolchain plugin is also copied into the modules. Is that needed? Can't it inherit from here?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on PR #2699:
URL: https://github.com/apache/avro/pull/2699#issuecomment-1913365366
The java build fails over a file `copy.avro` missing in a generated artifact that is the OUTPUT of a test. This is a bug that needs fixing.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1469292100
##########
.github/workflows/codeql-java-analysis.yml:
##########
@@ -70,17 +70,34 @@ jobs:
# queries: ./path/to/local/query, your-org/your-repo/queries@main
queries: +security-and-quality
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
+ with:
+ distribution: 'temurin'
+ java-version: |
+ 8
Review Comment:
There was a [discussion](https://lists.apache.org/thread/bd39zhk655pgzfctq763vp3z4xrjpx58) recently at dev@ about dropping the support for Java 8.
##########
.github/workflows/codeql-java-analysis.yml:
##########
@@ -70,17 +70,34 @@ jobs:
# queries: ./path/to/local/query, your-org/your-repo/queries@main
queries: +security-and-quality
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
+ with:
+ distribution: 'temurin'
+ java-version: |
+ 8
+ 11
+ 17
+ 21
Review Comment:
Do we need several JDKs for CodeQL analysis ?
CodeQL is ignored most of the time anyway
Update: OK, I think I understand it - we need them for toolchains.xml
##########
.github/workflows/test-lang-java.yml:
##########
@@ -90,28 +93,37 @@ jobs:
restore-keys: |
${{ runner.os }}-maven-
- - name: Setup Java
- uses: actions/setup-java@v4
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
with:
distribution: 'temurin'
- java-version: ${{ matrix.java }}
+ java-version: |
+ 8
+ 11
+ 17
+ 21
+
+ - name: 'Setup Maven 3.9.6'
+ uses: stCarolas/setup-maven@07fbbe97d97ef44336b7382563d66743297e442f # v4.5
+ with:
+ maven-version: 3.9.6
- - name: Setup Python for Generating Input Data
- uses: actions/setup-python@v5
+ - name: 'Setup Python for Generating Input Data'
+ uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # v5.0.0
- - name: Apt Install Compression Libs Required by Python
+ - name: 'Apt Install Compression Libs Required by Python'
Review Comment:
The quotes are not really needed
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1469332510
##########
.github/workflows/codeql-java-analysis.yml:
##########
@@ -70,17 +70,34 @@ jobs:
# queries: ./path/to/local/query, your-org/your-repo/queries@main
queries: +security-and-quality
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
+ with:
+ distribution: 'temurin'
+ java-version: |
+ 8
+ 11
+ 17
+ 21
Review Comment:
Yes, the build checks if all needed JDK versions for the full build are present early on in the process ("fail fast").
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "opwvhk (via GitHub)" <gi...@apache.org>.
opwvhk commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471437514
##########
lang/java/avro/pom.xml:
##########
@@ -55,7 +55,42 @@
</includes>
</resource>
</resources>
+ <testResources>
+ <testResource>
+ <directory>src/test/resources</directory>
+ </testResource>
+ <testResource>
+ <directory>../../../share/</directory>
+ <includes>
+ <include>schemas/**</include>
+ <include>test/**</include>
+ </includes>
+ <targetPath>share/</targetPath>
+ </testResource>
+ </testResources>
+
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-toolchains-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>Build using JDK 21</id>
+ <phase>validate</phase>
+ <goals>
+ <goal>toolchain</goal>
+ </goals>
Review Comment:
This is not needed, as this plugin goal binds to this phase by default.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "opwvhk (via GitHub)" <gi...@apache.org>.
opwvhk commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1471454903
##########
lang/java/ipc/pom.xml:
##########
@@ -54,6 +54,26 @@
</resource>
</resources>
<plugins>
+ <!--<plugin>-->
+ <!-- <groupId>org.apache.maven.plugins</groupId>-->
+ <!-- <artifactId>maven-toolchains-plugin</artifactId>-->
+ <!-- <executions>-->
+ <!-- <execution>-->
+ <!-- <id>Build using JDK 21</id>-->
+ <!-- <phase>validate</phase>-->
+ <!-- <goals>-->
+ <!-- <goal>toolchain</goal>-->
+ <!-- </goals>-->
+ <!-- <configuration>-->
+ <!-- <toolchains>-->
+ <!-- <jdk>-->
+ <!-- <version>21</version>-->
+ <!-- </jdk>-->
+ <!-- </toolchains>-->
+ <!-- </configuration>-->
+ <!-- </execution>-->
+ <!-- </executions>-->
+ <!--</plugin>-->
Review Comment:
Why is this one commented out?
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1472502881
##########
.github/workflows/codeql-java-analysis.yml:
##########
@@ -70,17 +70,34 @@ jobs:
# queries: ./path/to/local/query, your-org/your-repo/queries@main
queries: +security-and-quality
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
Review Comment:
The way this works is that the actual exact tag of the version is appended and systems like Renovate (unsure about Dependabot) will update that comment too. So this way has both pinning to an exact version AND readbility. I prefer it this way.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-3716: Build under JDK 21 [avro]
Posted by "opwvhk (via GitHub)" <gi...@apache.org>.
opwvhk commented on code in PR #2699:
URL: https://github.com/apache/avro/pull/2699#discussion_r1474125610
##########
.github/workflows/codeql-java-analysis.yml:
##########
@@ -70,17 +70,34 @@ jobs:
# queries: ./path/to/local/query, your-org/your-repo/queries@main
queries: +security-and-quality
+ - name: 'Setup Temurin JDK 8, 11, 17 & 21'
+ uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93 # v4.0.0
Review Comment:
Nice feature. Didn't know about that.
Searched a bit: it seems dependabot/dependabot-core#5951 added support for this as well.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org