andygrove commented on code in PR #5841:
URL: https://github.com/apache/datafusion-comet/pull/5841#discussion_r3989409782
##########
.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:
#5850 moved routing policy out of `ci.yml` expressions and into
`compute-changes.py`, and this nine-way OR is the same kind of policy. Would it
work to compute a `build_linux_native` output there as the union of the
consumer outputs, so this becomes `needs.changes.outputs.build_linux_native ==
'true'`? Then `test-native-build-selection.py` would not need to regex-parse
the YAML `if:` block to prove the two sides agree, and the README routing table
gets a single row like the others.
##########
.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:
Now that this job only compiles JVM test classes, does it still need the
`amd64/rust` container and the Rust toolchain from `setup-builder`? If the
skip-native-build Maven path genuinely needs them, a short comment here would
save the next reader the same question.
--
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]
