Conversation
This closes apache#14094
|
@nastra @manuzhang I created this PR to directly bump to Gradle 9.4.0 and clean things up. |
manuzhang
left a comment
manuzhang
left a comment
There was a problem hiding this comment.
Are all the changes related to the Gradle upgrade?
There was a problem hiding this comment.
Pull request overview
Upgrades the build tooling to Gradle 9.4.0 and makes accompanying build/script adjustments to keep formatting and test execution working across modules.
Changes:
- Upgrade Gradle wrapper to 9.4.0 (wrapper properties + bootstrap download URL).
- Update build configuration for Gradle 9 compatibility (Spotless/formatter version bump, remove inferred processors plugin usage, adjust test artifact dependencies, conditional delta-lake integration source set wiring).
- Ensure Flink integration tests run on JUnit Platform and apply minor formatter-driven comment indentation updates in a few Java sources.
Reviewed changes
Copilot reviewed 7 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/data/vectorized/VectorizedSparkOrcReaders.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/data/SparkOrcReader.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/data/vectorized/VectorizedSparkOrcReaders.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/data/SparkOrcReader.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/data/vectorized/VectorizedSparkOrcReaders.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/data/SparkOrcReader.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/data/vectorized/VectorizedSparkOrcReaders.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/data/SparkOrcReader.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| orc/src/main/java/org/apache/iceberg/data/orc/GenericOrcReader.java | Formatter-driven comment indentation adjustment in ORC primitive switch. |
| gradlew | Update fallback wrapper-jar download URL to Gradle v9.4.0. |
| gradle/wrapper/gradle-wrapper.properties | Point wrapper distribution to Gradle 9.4.0 and update SHA256. |
| flink/v2.1/build.gradle | Add JUnit Platform deps for integration tests and enable useJUnitPlatform() for integrationTest task. |
| flink/v2.0/build.gradle | Add JUnit Platform deps for integration tests and enable useJUnitPlatform() for integrationTest task. |
| flink/v1.20/build.gradle | Add JUnit Platform deps for integration tests and enable useJUnitPlatform() for integrationTest task. |
| build.gradle | Update Spotless plugin version, adjust delta-lake integration source set wiring, and switch open-api tests to depend on testArtifacts. |
| bom/build.gradle | Add minimal build file placeholder for BOM project. |
| baseline.gradle | Remove inferred processors plugin application and bump google-java-format version used by Spotless. |
| api/src/main/java/org/apache/iceberg/transforms/ProjectionUtil.java | Minor indentation change in commented-out code block. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Yes. The reformat change is due to the Google Java Format needed with gradle 9.4.0. I'm fixing the latest build issues. |
| // The newest version of delta-core uses Spark 3.5.*. The integration test should only be built | ||
| // if iceberg-spark-3.5 is available | ||
| if (sparkVersions.contains("3.5")) { | ||
| sourceSets { |
There was a problem hiding this comment.
Because gradle 9 is sensitive to the processing order and configuration sequence.
| // 1.23.0 has an issue in formatting comments https://github.com/google/google-java-format/issues/1155 | ||
| // so we stick to 1.22.0 to produce consistent result for JDK 17/21 | ||
| googleJavaFormat("1.22.0") | ||
| googleJavaFormat("1.25.2") |
There was a problem hiding this comment.
we shouldn't be bumping the GJF with the Gradle version as that just causes unnecessary changes
There was a problem hiding this comment.
The purpose here is to be aligned with Gradle 9.4.0 requirement.
There was a problem hiding this comment.
we should be able to upgrade to 1.28.0 but I would suggest to do this in a separate PR. In that PR you can also include the version bump for the spotless plugin
| classpath 'com.palantir.baseline:gradle-baseline-java:6.90.0' | ||
| classpath 'com.diffplug.spotless:spotless-plugin-gradle:8.2.1' | ||
| classpath 'gradle.plugin.org.inferred:gradle-processors:3.7.0' | ||
| classpath 'com.diffplug.spotless:spotless-plugin-gradle:8.3.0' |
There was a problem hiding this comment.
ideally we shouldn't be bumping any plugin versions when upgrading the Gradle version. At the very least please add some details why this upgrade is necessary
There was a problem hiding this comment.
Here, we need this spotless-plugin-gradle version in order to be compatible with Gradle 9.4.0.
There was a problem hiding this comment.
I propose to have this PR "green" and then, I will be happy to downgrade (it might need to downgrade gradle version as well).
jbonofre
changed the title
This closes #14094