sunchao commented on code in PR #5841:
URL: https://github.com/apache/datafusion-comet/pull/5841#discussion_r4000818215
##########
.github/workflows/ci.yml:
##########
@@ -184,11 +187,41 @@ jobs:
# can actually test it.
# ---------------------------------------------------------------------------
+ # Independent checks start immediately after change selection.
+ pr_build_linux_checks:
+ name: PR Checks (Linux)
+ needs: changes
+ permissions:
+ contents: read
+ if: needs.changes.outputs.build_linux == 'true'
+ uses: ./.github/workflows/pr_build_linux_checks.yml
+
+ # Build once when any native-library consumer is selected. Each output
+ # already includes the path, event, and label policy from compute-changes.py.
+ build_linux_native:
+ name: Shared Linux Native Library
+ needs: changes
+ permissions:
+ contents: read
+ if: |
Review Comment:
Addressed: `compute-changes.py` now derives `build_linux_native` from the
selected consumer routes, and `ci.yml` reads that one output. The mapping also
supports Spark 4.1's core and Hive routes selecting the same caller, including
Hive-only label events. The selection tests exercise Python and the real CLI;
the configuration guard separately verifies exact caller gates and output
exports. Updated in 79c161b293f2147b1d25233a47a15222d7289cf9; 21 selection
tests pass, including 299,008 path/event combinations.
##########
.github/workflows/spark_sql_test_reusable.yml:
##########
@@ -52,22 +56,11 @@ env:
jobs:
- # Build the native library AND pre-compile Spark sources + Test classes in a
- # single runner, then publish two artifacts the matrix consumes:
- # - native-lib-spark-<full>-jdk<N>: libcomet.so (~50 MB)
- # - jvm-compiled-spark-<full>-jdk<N>: apache-spark.tar.gz (sources +
- # target/ + Zinc state, ~500 MB-1
GB)
- # Combining them avoids a second runner cold-start and an extra inter-job
- # artifact round-trip for the native lib, since the JVM build already
- # depends on it (the Comet Maven install bundles libcomet.so into the
- # Comet JAR before SBT resolves Spark's classpath).
- #
- # Both names carry the Spark/JDK version because ci.yml calls this workflow
- # once per Spark version inside a single run, and artifact names are scoped
- # to the run, not to the calling workflow. See "Artifact names must be unique
- # per producer" in .github/workflows/README.md.
+ # Pre-compile Spark sources and test classes once per Spark/JDK version.
+ # The native library comes from the umbrella's shared Linux producer; only
+ # the version-specific JVM artifact is published by this workflow.
build:
- name: Build Native + JVM Test Classes
+ name: Build JVM Test Classes
runs-on: ubuntu-24.04
container:
image: amd64/rust
Review Comment:
Removed the Rust toolchain setup from the JVM compilation job: it now uses
`actions/setup-java@v4`. The adjacent comment explains why the container
remains: Spark's precompiled JVM classes and Maven cache are reused by
downstream test containers, so their workspace and cache paths must stay
consistent. The Spark 3.4/JDK 11 opt-in run has now been requested on rebased
head 79c161b293f2147b1d25233a47a15222d7289cf9 to cover this setup path; its
result is still pending.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
