jbonofre commented on code in PR #1284:
URL: https://github.com/apache/arrow-java/pull/1284#discussion_r4176198456
##########
compose.yaml:
##########
@@ -84,6 +131,28 @@ services:
/arrow-java/ci/scripts/build.sh /arrow-java /build /jni &&
/arrow-java/ci/scripts/test.sh /arrow-java /build /jni"
+ cdata-artifacts:
+ # Builds reusable C Data Interface JNI artifacts for CI test jobs.
+ <<: *cdata-prebuilt
+ environment:
+ ARROW_JAVA_CDATA: "ON"
+ command:
+ /bin/bash -c "
+ rm -rf /build-output/build &&
+ find /jni -mindepth 1 -maxdepth 1 -exec rm -rf {} + &&
+ /arrow-java/ci/scripts/jni_build.sh /arrow-java /tmp/cdata-jni
/tmp/cdata-native /jni &&
+ /arrow-java/ci/scripts/build.sh /arrow-java /build-output/build /jni"
+
+ cdata-test-prebuilt:
+ # Runs C Data Interface tests against prebuilt CI artifacts.
+ <<: *cdata-prebuilt
+ environment:
+ ARROW_JAVA_CDATA: "ON"
+ ARROW_JAVA_TEST_BASE: "OFF"
Review Comment:
With this Conda JNI jobs only run the `arrow-c-data` tests. On `main`,
`conda-jni-cdata` runs the full `mvn test` first and then the `c` module, so
vector, memory, flight and the rest are also exercised on the conda-forge
OpenJDK 17/21/25.
Is dropping that intentional? I'm fine with it as deduplication, but then
please mention it in PR description, since it says coverage is retained.
##########
.github/workflows/test.yml:
##########
@@ -34,28 +34,16 @@ permissions:
contents: read
env:
+ BUILD_JDK: "17"
Review Comment:
Building once with JDK 17 means Spotless and Error Prone no longer run in
this workflow: both profiles in the root `pom.xml` are activated with
`<jdk>[21,)</jdk>`. On `main` the JDK 21 and 25 jobs run `build.sh` with those
JDKs, so formatting and Error Prone violations fail the PR. With this change
they would pass.
It also means nothing compiles the sources with JDK 21 or 25 any more: the
`jdk: [17, 21, 25]` matrix only runs classes compiled by 17, so a change that
breaks under a newer `javac` would go unnoticed.
Could you add a compile-only job (no tests) on JDK 25, or on 21 and 25? That
brings both checks back and should only cost a few minutes of the time saved.
##########
.github/workflows/test.yml:
##########
@@ -65,82 +53,202 @@ jobs:
- name: Cache Docker Volumes
uses: actions/cache@v6
with:
- path: .docker
- key: maven-${{ matrix.jdk }}-${{ matrix.maven }}-${{
hashFiles('compose.yaml', '**/pom.xml') }}
- restore-keys: maven-${{ matrix.jdk }}-${{ matrix.maven }}-
- - name: Execute Docker Build
+ path: .docker/maven-cache
+ key: java-build-${{ env.BUILD_JDK }}-${{ env.MAVEN }}-${{
hashFiles('compose.yaml', '**/pom.xml') }}
+ restore-keys: java-build-${{ env.BUILD_JDK }}-${{ env.MAVEN }}-
+ - name: Build without tests
env:
- # Enables build caching, but not strictly required
DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ JDK: ${{ env.BUILD_JDK }}
run: |
docker compose run \
+ --rm \
-e CI=true \
-e "DEVELOCITY_ACCESS_KEY=$DEVELOCITY_ACCESS_KEY" \
- ${{ matrix.image }}
+ ubuntu-artifacts
+ sudo chown -R "$(id -u):$(id -g)" .docker
+ - name: Pack reusable artifacts
+ run: |
+ tar -czf java-build.tgz \
+ .docker/java-build \
+ .docker/maven-cache/repository/org/apache/arrow
+ - name: Upload reusable artifacts
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
+ with:
+ name: java-build
+ path: java-build.tgz
+ retention-days: 1
+ - name: Exclude reactor artifacts from Maven dependency cache
+ run: rm -rf .docker/maven-cache/repository/org/apache/arrow
- macos:
- name: ${{ matrix.arch }} macOS ${{ matrix.macos }} Java JDK ${{ matrix.jdk
}}
- runs-on: macos-${{ matrix.macos }}
+ test-java:
+ name: ${{ matrix.name || format('AMD64 Ubuntu JDK {0} Maven 3.9.16',
matrix.jdk) }}
+ needs: build-java
+ runs-on: ${{ matrix.os }}
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 30
strategy:
fail-fast: false
matrix:
+ jdk: [17, 21, 25]
+ os: [ubuntu-latest]
include:
- - arch: AArch64
+ - os: ubuntu-latest
+ compose_service: ubuntu-test-prebuilt
+ - name: AArch64 macOS latest Java JDK 17
+ os: macos-latest
jdk: 17
- macos: latest
+ compose_service: ''
+ - name: AMD64 Windows Server 2022 Java JDK 17
+ os: windows-latest
+ jdk: 17
+ compose_service: ''
+ env:
+ JDK: ${{ matrix.jdk }}
steps:
- name: Checkout Arrow
uses: actions/checkout@v7
with:
- fetch-depth: 0
submodules: recursive
- name: Set up Java
+ if: ${{ !matrix.compose_service }}
uses: actions/setup-java@v6
with:
distribution: 'temurin'
java-version: ${{ matrix.jdk }}
cache: 'maven'
- - name: Build
+ - name: Cache Docker Volumes
+ if: ${{ matrix.compose_service }}
+ uses: actions/cache@v6
+ with:
+ path: .docker/maven-cache
+ key: java-build-${{ env.BUILD_JDK }}-${{ env.MAVEN }}-${{
hashFiles('compose.yaml', '**/pom.xml') }}
+ restore-keys: java-build-${{ env.BUILD_JDK }}-${{ env.MAVEN }}-
+ - name: Download reusable artifacts
+ uses:
actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
+ with:
+ name: java-build
+ - name: Restore reusable artifacts
+ shell: bash
+ run: |
+ rm -rf \
+ .docker/java-build \
+ .docker/maven-cache/repository/org/apache/arrow
+ tar -xzf java-build.tgz
+ - name: Restore reusable artifacts for hosted runner
+ if: ${{ !matrix.compose_service }}
+ shell: bash
+ run: |
+ cp -a .docker/java-build/build build
+ rm -rf "${HOME}/.m2/repository/org/apache/arrow"
+ mkdir -p "${HOME}/.m2/repository/org/apache"
+ cp -a .docker/maven-cache/repository/org/apache/arrow
"${HOME}/.m2/repository/org/apache/"
+ - name: Test prebuilt artifacts on macOS/Windows
+ if: ${{ !matrix.compose_service }}
shell: bash
env:
+ ARROW_JAVA_TEST_PREBUILT: "ON"
DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
- run: ci/scripts/build.sh . build jni
- - name: Test
+ run: ci/scripts/test.sh . build jni
Review Comment:
On `main` the Windows job runs `ci/scripts/build.sh` before `test.sh`. Here
it only runs Surefire against classes compiled on Linux, and as far as I can
see that leaves no Maven build on Windows in any workflow (`jni-windows` in
`rc.yml` only runs `jni_windows_build.sh`, which is CMake).
A change that breaks the build on Windows (codegen paths, a plugin without a
Windows binary, line-ending-sensitive checks) would no longer be caught. I
would keep the full build for the Windows job. If we decide that is acceptable
to drop, please say so in the PR description, which currently states that the
existing platform coverage is retained.
I'm more in favor to keep Windows job.
##########
.github/workflows/test.yml:
##########
@@ -65,82 +53,202 @@ jobs:
- name: Cache Docker Volumes
uses: actions/cache@v6
with:
- path: .docker
- key: maven-${{ matrix.jdk }}-${{ matrix.maven }}-${{
hashFiles('compose.yaml', '**/pom.xml') }}
- restore-keys: maven-${{ matrix.jdk }}-${{ matrix.maven }}-
- - name: Execute Docker Build
+ path: .docker/maven-cache
+ key: java-build-${{ env.BUILD_JDK }}-${{ env.MAVEN }}-${{
hashFiles('compose.yaml', '**/pom.xml') }}
+ restore-keys: java-build-${{ env.BUILD_JDK }}-${{ env.MAVEN }}-
+ - name: Build without tests
env:
- # Enables build caching, but not strictly required
DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ JDK: ${{ env.BUILD_JDK }}
run: |
docker compose run \
+ --rm \
-e CI=true \
-e "DEVELOCITY_ACCESS_KEY=$DEVELOCITY_ACCESS_KEY" \
- ${{ matrix.image }}
+ ubuntu-artifacts
+ sudo chown -R "$(id -u):$(id -g)" .docker
+ - name: Pack reusable artifacts
+ run: |
+ tar -czf java-build.tgz \
+ .docker/java-build \
+ .docker/maven-cache/repository/org/apache/arrow
+ - name: Upload reusable artifacts
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a
# v7.0.1
+ with:
+ name: java-build
+ path: java-build.tgz
+ retention-days: 1
Review Comment:
With one day of retention, "Re-run failed jobs" stops working a day after
the run: `build-java` succeeded so it is not re-run, and the test jobs then
fail on `download-artifact`. The only way out is re-running everything, which
defeats the purpose.
Could you raise this, for example to 7 days? Same for the `cdata-build` at
line 208.
--
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]
