Add cuDF JAR build support for all Maven classifiers - #23261
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
paul-aiyedun
added
feature request
|
/ok to test ff0d2c2 |
|
/ok to test b03423e |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds containerized static libcudf and cuDF Java JAR builds, architecture-specific Maven classifiers, repository assembly, local validation, Java dependencies, updated documentation, and CUDA/architecture matrix CI jobs. ChangescuDF Java build pipeline
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
java/ci/build_static_libcudf_in_container.sh (1)
25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
shellcheck disable=SC1091for the dynamic conda source.Shellcheck flags this line since
/opt/conda/etc/profile.d/conda.shisn't resolvable statically. The sibling scriptbuild_static_libcudf.shalready applies this pattern (# shellcheck disable=SC1091) for its own dynamic source. As per coding guidelines, "Use the repository's pre-commit hooks before committing and ensure CI formatting and lint checks pass."🔧 Proposed fix
+# shellcheck disable=SC1091 . /opt/conda/etc/profile.d/conda.sh🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/ci/build_static_libcudf_in_container.sh` at line 25, Add a shellcheck SC1091 disable directive immediately before the dynamic conda source command in the build script, matching the existing pattern in build_static_libcudf.sh; leave the source behavior unchanged.Sources: Coding guidelines, Linters/SAST tools
dependencies.yaml (1)
648-658: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider anchoring shared package pins with
test_java.
build_javaand the existingtest_javabundle (Lines 1034-1042) both pinmaven,openjdk=8.*, andboostindependently. Using a YAML anchor (like*cmake_ver) for these shared pins would prevent silent version drift between the two bundles over time.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dependencies.yaml` around lines 648 - 658, Update the shared package definitions in build_java and test_java so the common maven, openjdk=8.*, and boost pins are defined once via a YAML anchor and reused by both bundles. Preserve the existing cuda-profiler-api, make, and bundle-specific packages while eliminating duplicated shared pin declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr.yaml:
- Around line 47-49: Remove the temporary java-build and java-gather jobs from
the workflow, along with their corresponding entries in pr-builder.needs.
Preserve the remaining CI jobs and dependency relationships unchanged.
In `@java/ci/assemble_maven_repo.sh`:
- Around line 125-160: Update the assembly flow around the classifier loop and
POM discovery to validate all JAR inputs, consistent versions, and the required
POM before writing to the output. Stage the complete Maven version repository in
a temporary directory, including all JARs and the POM, then atomically replace
the existing version directory only after staging succeeds so failed runs leave
the prior output unchanged and obsolete classifiers are removed.
In `@java/ci/build_cudf_java_jar_in_container.sh`:
- Around line 112-116: Register the HOST_UID/HOST_GID ownership restoration as
an EXIT cleanup before any fallible Maven or copy steps in the build script.
Ensure the cleanup chowns both OUTPUT_DIR and REPO_ROOT/java/target when those
variables are set, so it runs on successful and failed exits; remove or avoid
relying on the current end-of-script-only chown block.
In `@java/ci/build_cudf_java_jar.sh`:
- Around line 153-167: Update the classifier rebuild setup around CLASSIFIER_OUT
and TARGET_SCRATCH to remove stale artifacts from CLASSIFIER_OUT before Maven
runs. Preserve the directory itself for new outputs, and keep the existing
TARGET_SCRATCH cleanup unchanged so each rebuild starts with no prior classifier
JARs or POMs.
In `@java/ci/build_static_libcudf_in_container.sh`:
- Around line 13-17: Make HOST_UID and HOST_GID truly required in the chown step
of build_static_libcudf_in_container.sh: validate that both are set and fail
clearly when either is missing, rather than conditionally skipping ownership
fixup. Preserve the existing chown behavior when both values are provided.
In `@java/ci/README.md`:
- Around line 3-15: Update the README’s recommended flow to reference scripts
under java/ci/ and document invocation of test_java_build_local.sh as the
one-command local entrypoint for the host architecture, while preserving the
existing prerequisite and build-description content.
In `@java/ci/test_java_build_local.sh`:
- Around line 192-215: Update both build_static_libcudf.sh invocations in the
Step 1 static build block to use the shared JAR_PARALLEL budget instead of
PARALLEL_LEVEL, preserving the existing minimum-one safeguard and all other
arguments.
---
Nitpick comments:
In `@dependencies.yaml`:
- Around line 648-658: Update the shared package definitions in build_java and
test_java so the common maven, openjdk=8.*, and boost pins are defined once via
a YAML anchor and reused by both bundles. Preserve the existing
cuda-profiler-api, make, and bundle-specific packages while eliminating
duplicated shared pin declarations.
In `@java/ci/build_static_libcudf_in_container.sh`:
- Line 25: Add a shellcheck SC1091 disable directive immediately before the
dynamic conda source command in the build script, matching the existing pattern
in build_static_libcudf.sh; leave the source behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2224cd39-f321-4e19-9128-ee98a7715e7c
📒 Files selected for processing (12)
.github/workflows/build.yaml.github/workflows/pr.yamldependencies.yamljava/ci/README.mdjava/ci/argparse.shjava/ci/assemble_maven_repo.shjava/ci/build_cudf_java_jar.shjava/ci/build_cudf_java_jar_in_container.shjava/ci/build_static_libcudf.shjava/ci/build_static_libcudf_in_container.shjava/ci/test_java_build_local.shjava/pom.xml
msarahan
left a comment
msarahan
left a comment
There was a problem hiding this comment.
I personally would choose a different tool for this - perhaps Make or Just. When you have sequences of things that depend on one another's output, those are a more natural fit. Just would be adding an external dependency for CI, though, and Make can be pretty squirelly. The shell scripts you have are workable. It's up to you and your team to decide what tools you're comfortable with.
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| # Local end-to-end verification of the build workflow: builds the static |
There was a problem hiding this comment.
This seems like it should be a Makefile or similar. It's not really a test, though it has served that purpose for you as you developed your workflow.
There was a problem hiding this comment.
I added this script in order to enable local end-to-end testing. It is not intended for use by any CI. Hence, the basic implementation vs using something like make.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@java/ci/build_cudf_java_jar.sh`:
- Around line 155-158: Add automated unit tests and benchmarks for the packaging
script’s contracts, covering pre-existing classifier directories, classifier
naming, and validation of exactly one JAR and POM, including
rerun/stale-artifact cases. Mock Docker execution where needed, and anchor the
coverage to the script’s classifier-output check and related packaging
validation paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3f9c58c4-168e-4ace-aab8-7de570d633ce
📒 Files selected for processing (6)
java/ci/README.mdjava/ci/assemble_maven_repo.shjava/ci/build_cudf_java_jar.shjava/ci/build_cudf_java_jar_in_container.shjava/ci/build_static_libcudf_in_container.shjava/ci/test_java_build_local.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- java/ci/README.md
- java/ci/build_static_libcudf_in_container.sh
- java/ci/assemble_maven_repo.sh
- java/ci/build_cudf_java_jar_in_container.sh
- java/ci/test_java_build_local.sh
NvTimLiu
left a comment
NvTimLiu
left a comment
There was a problem hiding this comment.
LGTM, BTW this builds JARs but does not publish them
That is correct. This only covers the build step. Publication will be handled by a different workflow. |
github-actions
Bot
added
Python
* Split the JAR build into three composable stages (static libcudf build, per-classifier JAR packaging, and Maven-repo gather) so each stage is independently runnable in CI and locally. * Link against a static libcudf built from source per CUDA version rather than a conda shared libcudf. * Emit the Maven classifier based on the host architecture the build runs on, introducing a new `-arm64` suffix to distinguish aarch64 JARs from their x86_64 counterparts. * Add `test_java_build_local.sh` as a one-command local reproducer of the full CI matrix for the host arch, with per-step timings and GPU compute-capability auto-detection.
1537952 to
f6b02ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@java/ci/build_cudf_java_jar_in_container.sh`:
- Around line 56-59: Add pipefail alongside set -e in
java/ci/build_cudf_java_jar_in_container.sh at lines 22 and
java/ci/build_static_libcudf_in_container.sh at line 19, so failures from
rapids-dependency-file-generator propagate through the tee pipelines and stop
both scripts.
In `@java/ci/README.md`:
- Line 47: Update both directory-tree code fences in the README to specify the
text language on their opening fences, resolving the MD040 markdownlint
violations.
In `@java/ci/test_java_build_local.sh`:
- Around line 192-197: Update the STEP_PARALLEL handling near the concurrent
Step 1 and Step 2 builds so PARALLEL_LEVEL=1 does not launch two simultaneous
workers: either serialize the CUDA builds when the total budget is one, or
reject values below two before launching them. Preserve the existing split
budget behavior for PARALLEL_LEVEL values of two or greater.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 28740006-5bfd-4ee6-ab8b-2da59acafe55
📒 Files selected for processing (12)
.github/workflows/build.yaml.github/workflows/pr.yamldependencies.yamljava/ci/README.mdjava/ci/argparse.shjava/ci/assemble_maven_repo.shjava/ci/build_cudf_java_jar.shjava/ci/build_cudf_java_jar_in_container.shjava/ci/build_static_libcudf.shjava/ci/build_static_libcudf_in_container.shjava/ci/test_java_build_local.shjava/pom.xml
🚧 Files skipped from review as they are similar to previous changes (6)
- java/pom.xml
- dependencies.yaml
- java/ci/argparse.sh
- java/ci/build_static_libcudf.sh
- java/ci/assemble_maven_repo.sh
- java/ci/build_cudf_java_jar.sh
| rapids-dependency-file-generator \ | ||
| --output conda \ | ||
| --file-key build_java \ | ||
| --matrix "cuda=${CUDA_MAJOR_MINOR};arch=$(arch)" | tee "${ENV_YAML_DIR}/env.yaml" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- java/ci/build_cudf_java_jar_in_container.sh ---\n'
nl -ba java/ci/build_cudf_java_jar_in_container.sh | sed -n '1,120p'
printf '\n--- java/ci/build_static_libcudf_in_container.sh ---\n'
nl -ba java/ci/build_static_libcudf_in_container.sh | sed -n '1,120p'
printf '\n--- shell options / pipeline context search ---\n'
rg -n 'set -[^\n]*e|pipefail|rapids-dependency-file-generator|tee "\$\{ENV_YAML_DIR\}/env.yaml"' java/ci/build_cudf_java_jar_in_container.sh java/ci/build_static_libcudf_in_container.shRepository: rapidsai/cudf
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- java/ci/build_cudf_java_jar_in_container.sh ---'
sed -n '1,90p' java/ci/build_cudf_java_jar_in_container.sh | nl -ba
echo
echo '--- java/ci/build_static_libcudf_in_container.sh ---'
sed -n '1,80p' java/ci/build_static_libcudf_in_container.sh | nl -ba
echo
echo '--- exact option lines / pipeline sites ---'
rg -n 'set -[^\n]*|pipefail|rapids-dependency-file-generator|tee "\$\{ENV_YAML_DIR\}/env.yaml"' \
java/ci/build_cudf_java_jar_in_container.sh \
java/ci/build_static_libcudf_in_container.shRepository: rapidsai/cudf
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -e
for f in java/ci/build_cudf_java_jar_in_container.sh java/ci/build_static_libcudf_in_container.sh; do
echo "--- $f ---"
awk '{printf "%6d %s\n", NR, $0}' "$f" | sed -n '1,120p'
echo
done
echo "--- matches ---"
grep -nE 'set -[a-zA-Z]+|pipefail|rapids-dependency-file-generator|tee "\$\{ENV_YAML_DIR\}/env.yaml"' \
java/ci/build_cudf_java_jar_in_container.sh \
java/ci/build_static_libcudf_in_container.shRepository: rapidsai/cudf
Length of output: 9347
Avoid hiding dependency-file generation failures
Both scripts only use set -e, so a failing rapids-dependency-file-generator can still be followed by a successful tee, and the pipeline won’t stop. Add set -o pipefail in:
java/ci/build_cudf_java_jar_in_container.sh#L22java/ci/build_static_libcudf_in_container.sh#L19
📍 Affects 2 files
java/ci/build_cudf_java_jar_in_container.sh#L56-L59(this comment)java/ci/build_static_libcudf_in_container.sh#L48-L51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@java/ci/build_cudf_java_jar_in_container.sh` around lines 56 - 59, Add
pipefail alongside set -e in java/ci/build_cudf_java_jar_in_container.sh at
lines 22 and java/ci/build_static_libcudf_in_container.sh at line 19, so
failures from rapids-dependency-file-generator propagate through the tee
pipelines and stop both scripts.
| single classifier JAR (e.g. `cudf-26.08.0-SNAPSHOT-cuda12.jar`) plus its POM | ||
| into a classifier-named subdirectory under `--output-dir`: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the directory-tree code fences.
markdownlint flags both fences as MD040. Use text for each opening fence.
Also applies to: 75-75
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 47-47: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@java/ci/README.md` at line 47, Update both directory-tree code fences in the
README to specify the text language on their opening fences, resolving the MD040
markdownlint violations.
Source: Linters/SAST tools
| # Both Step 1 and Step 2 launch two concurrent builds. Each build gets half | ||
| # of PARALLEL_LEVEL so together they stay within PARALLEL_LEVEL. | ||
| STEP_PARALLEL=$((PARALLEL_LEVEL / 2)) | ||
| if [[ ${STEP_PARALLEL} -lt 1 ]]; then | ||
| STEP_PARALLEL=1 | ||
| fi |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Honor a one-worker total budget.
With --parallel 1, this still launches two concurrent --parallel 1 builds. Run the CUDA builds serially for a one-worker budget, or reject values below two.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@java/ci/test_java_build_local.sh` around lines 192 - 197, Update the
STEP_PARALLEL handling near the concurrent Step 1 and Step 2 builds so
PARALLEL_LEVEL=1 does not launch two simultaneous workers: either serialize the
CUDA builds when the total budget is one, or reject values below two before
launching them. Preserve the existing split budget behavior for PARALLEL_LEVEL
values of two or greater.
|
/merge |
3003a8c
into
NVIDIA:release/26.08
* Split the JAR build into three composable stages (static libcudf build, per-classifier JAR packaging, and Maven-repo gather) so each stage is independently runnable in CI and locally. * Link against a static libcudf built from source per CUDA version rather than a conda shared libcudf. * Emit the Maven classifier based on the host architecture the build runs on, introducing a new `-arm64` suffix to distinguish aarch64 JARs from their x86_64 counterparts. * Add `test_java_build_local.sh` as a one-command local reproducer of the full CI matrix for the host arch, with per-step timings and GPU compute-capability auto-detection. Contributes to NVIDIA#22204 Authors: - https://github.com/paul-aiyedun Approvers: - Mike Sarahan (https://github.com/msarahan) - Tim Liu (https://github.com/NvTimLiu) URL: NVIDIA#23261
8 participants
Description
Split the JAR build into three composable stages (static libcudf build, per-classifier JAR packaging, and Maven-repo gather) so each stage is independently runnable in CI and locally.
Link against a static libcudf built from source per CUDA version rather than a conda shared libcudf.
Emit the Maven classifier based on the host architecture the build runs on, introducing a new
-arm64suffix to distinguish aarch64 JARs from their x86_64 counterparts.Add
test_java_build_local.shas a one-command local reproducer of the full CI matrix for the host arch, with per-step timings and GPU compute-capability auto-detection.Contributes to #22204
Checklist