fix: build the aiconfigurator wheel as manylinux_2_28#1330
Merged
Conversation
The wheel carries a native Rust/PyO3 (abi3) extension but was built on ubuntu:jammy and tagged linux_x86_64 -- a bare platform tag that PyPI rejects and that effectively pins the wheel to the build host's glibc. Build it as a portable manylinux_2_28_x86_64 wheel instead (glibc >= 2.28; RHEL 8+ / Ubuntu 20.04+). Move the build stage onto quay.io/pypa/manylinux_2_28_x86_64 (glibc 2.28) with an explicit rustup toolchain and uv, pinned to the image's CPython 3.12. maturin's built-in auditwheel checks the manylinux_2_28 policy and stamps the tag -- no separate auditwheel step. uv build makes maturin's PEP517 backend force --compatibility off, so the target is re-asserted via MATURIN_PEP517_ARGS on the build command. README notes the new tag. Verified: auditwheel reports the wheel consistent with manylinux_2_28_x86_64 (only whitelisted libc/libm/libpthread/libgcc_s referenced, nothing bundled); it installs and runs on the jammy runtime image; verify_release_wheels.py passes. Signed-off-by: Anant Sharma <anants@nvidia.com>
Contributor
WalkthroughThe Docker build now targets manylinux_2_28 with a rustup-installed Rust toolchain, pins the Python interpreter used in the build image, and forces wheel compatibility settings during build and verification. The README note and build-system constraint now match the published wheel target. ChangesManylinux 2.28 build and docs alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docker/Dockerfile (1)
39-39: 📐 Maintainability & Code Quality | 🔵 TrivialConsider pinning the Rust toolchain instead of the floating
stablechannel.
--default-toolchain stablewill resolve to whatever the latest stable Rust release is at build time, so image rebuilds months apart can pick up different compiler versions, hurting build reproducibility for a release artifact.
[recommended_refactor_effort_low_reward_moderate]-RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs \ - | sh -s -- -y --profile minimal --default-toolchain stable +RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs \ + | sh -s -- -y --profile minimal --default-toolchain 1.83.0🤖 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 `@docker/Dockerfile` at line 39, The Rust install step is using the floating stable channel, which makes rebuilds non-reproducible for release images. Update the toolchain selection in the Dockerfile’s Rust bootstrap step to a pinned Rust version instead of --default-toolchain stable, and keep the install aligned with the existing rustup setup so future builds use the same compiler version.
🤖 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 `@docker/Dockerfile`:
- Around line 38-39: The rustup installation step in the Dockerfile can succeed
even when curl fails because the pipe is executed without pipefail, so the
failure is hidden and rustup is never installed. Update the RUN step around the
curl-to-sh invocation to avoid piping directly, and make sure the curl result is
explicitly checked before running the installer so the build fails fast if the
download does not succeed.
- Line 43: The UV_PYTHON setting points to the wrong interpreter name in the
Dockerfile. Update the ENV value in the container setup to use the cp312-cp312
Python executable path that actually exists there, and keep the change localized
to the UV_PYTHON definition so the image verifier can find the interpreter
reliably.
- Around line 59-70: The wheel build in the Dockerfile relies on the maturin PEP
517 compatibility behavior, but the current version range still permits older
1.5.x releases that do not include the needed fix. Tighten the maturin minimum
version in the build-system requirements used by the `uv build`/`maturin` path,
or alternatively move the manylinux compatibility setting into backend config so
the Docker build no longer depends on that version-specific CLI behavior.
---
Nitpick comments:
In `@docker/Dockerfile`:
- Line 39: The Rust install step is using the floating stable channel, which
makes rebuilds non-reproducible for release images. Update the toolchain
selection in the Dockerfile’s Rust bootstrap step to a pinned Rust version
instead of --default-toolchain stable, and keep the install aligned with the
existing rustup setup so future builds use the same compiler version.
🪄 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: f6709aa7-78b6-4f70-be4a-6ba8d5ea3014
📒 Files selected for processing (2)
README.mddocker/Dockerfile
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: AIC Accuracy Regression Testing
- GitHub Check: Cargo Deny
- GitHub Check: Build and Test (unit)
- GitHub Check: Rust/Python engine-step parity
- GitHub Check: Build and Test (e2e)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit inference engine (.claude/rules/generator/config_schema.md)
**/*.md: Add computation rules in the rules authoring system when a new parameter requires non-trivial value computation beyond simple defaults
Add template modifications in the template authoring system when a new parameter requires special handling in generated artifacts
Files:
README.md
**/*
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
README.mddocker/Dockerfile
🔇 Additional comments (2)
docker/Dockerfile (1)
18-37: LGTM!README.md (1)
32-38: LGTM!
Parameterize the wheel build stage by target architecture so it produces
both manylinux_2_28_x86_64 and manylinux_2_28_aarch64 wheels from one
Dockerfile.
BuildKit populates TARGETARCH (amd64/arm64) from --platform (default: the
build host's arch); `FROM manylinux-${TARGETARCH}` selects the matching PyPA
base (quay.io/pypa/manylinux_2_28_{x86_64,aarch64}) and only that stage is
built. MATURIN_PEP517_ARGS is unchanged -- maturin appends the arch, so the
tag resolves to _x86_64 or _aarch64 automatically. The crate is
architecture-agnostic (no target_arch code / x86 intrinsics) and the parquet
data payload is identical across arches.
Build an aarch64 wheel natively on ARM, or from an x86 host via QEMU/buildx:
docker buildx build --platform linux/arm64 --target build --load .
Also drop a stale comment that referenced a [tool.maturin] compatibility
value that is not set (the target is asserted via MATURIN_PEP517_ARGS).
amd64 build re-verified unchanged; the aarch64 base pull and rustup
toolchain install validated under buildx QEMU emulation.
Signed-off-by: Anant Sharma <anants@nvidia.com>
Contributor
AIC Accuracy Regression TestingWorkflow result: Old revision: Summary
Comparison summary table
|
docker/Dockerfile: download the rustup installer to disk instead of piping `curl | sh`, so a failed download aborts the build (the pipeline previously reported only sh's exit status and silently skipped rustup). Add curl retries for transient network blips and pin the toolchain to 1.96.0 so release wheels build reproducibly. pyproject.toml: require maturin>=1.5.1. 1.5.1 fixed `--compatibility` handling when run as a PEP517 backend, which the manylinux_2_28 wheel build depends on (MATURIN_PEP517_ARGS="--compatibility manylinux_2_28"); 1.5.0 ignored it. Signed-off-by: Anant Sharma <anants@nvidia.com>
Trim the verbose block comments added for the manylinux_2_28 wheel build to their essential rationale (why manylinux/glibc 2.28, why MATURIN_PEP517_ARGS overrides the PEP517 compatibility default, why pin the interpreter). Signed-off-by: Anant Sharma <anants@nvidia.com>
Resolve the docker/Dockerfile conflict with #1329 (aiconfigurator-core wheel removal): keep the manylinux_2_28 multi-arch build but drop the now-removed core-wheel build step, so only the self-contained aiconfigurator wheel is built. main's simplified verifier already asserts no aiconfigurator-core wheel is present and the runtime stage installs aiconfigurator-*.whl only. Signed-off-by: Anant Sharma <anants@nvidia.com>
simone-chen
reviewed
Jul 8, 2026
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docker/Dockerfile (1)
44-44: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
UV_PYTHONstill points atpython3, which doesn't exist in this image.This exact issue was raised in a prior review round and remains unfixed. manylinux
cp*-cp*directories expose the interpreter asbin/python, notbin/python3— confirmed by cibuildwheel's own existence check (test -x /opt/python/cp312-cp312/bin/python) and other manylinux usage reports. With the current path,"${UV_PYTHON}"at line 65 will fail with "no such file or directory."🛠️ Suggested fix
-ENV UV_PYTHON=/opt/python/cp312-cp312/bin/python3 +ENV UV_PYTHON=/opt/python/cp312-cp312/bin/python🤖 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 `@docker/Dockerfile` at line 44, The Dockerfile still sets UV_PYTHON to a non-existent interpreter path, so update the ENV value in the Dockerfile to point at the manylinux interpreter name actually provided by the cp312-cp312 installation. Use the UV_PYTHON setting and the later "${UV_PYTHON}" invocation as the reference points, and make sure the path matches the bin/python executable exposed by the /opt/python/cp312-cp312 directory so the install step can find it.
🤖 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.
Duplicate comments:
In `@docker/Dockerfile`:
- Line 44: The Dockerfile still sets UV_PYTHON to a non-existent interpreter
path, so update the ENV value in the Dockerfile to point at the manylinux
interpreter name actually provided by the cp312-cp312 installation. Use the
UV_PYTHON setting and the later "${UV_PYTHON}" invocation as the reference
points, and make sure the path matches the bin/python executable exposed by the
/opt/python/cp312-cp312 directory so the install step can find it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4a2851d2-2614-4fe9-9520-4f893dccae66
📒 Files selected for processing (2)
README.mddocker/Dockerfile
✅ Files skipped from review due to trivial changes (1)
- README.md
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: AIC Accuracy Regression Testing
- GitHub Check: Build and Test (unit)
- GitHub Check: Cargo Deny
- GitHub Check: Build and Test (e2e)
- GitHub Check: Rust/Python engine-step parity
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: - Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.
- Do not use a suggested change when the fix requires broader design choices, multiple files, generated artifacts, unavailable context, or validation that cannot be inferred from the diff.
- If a comment is not directly applicable, state the smallest concrete next step and why a one-click suggestion is not safe.
Files:
docker/Dockerfile
🔇 Additional comments (2)
docker/Dockerfile (2)
18-25: LGTM!
63-65: LGTM!Also applies to: 69-69
jasonqinzhou
pushed a commit
that referenced
this pull request
Jul 8, 2026
* fix: build the aiconfigurator wheel as manylinux_2_28
The wheel carries a native Rust/PyO3 (abi3) extension but was built on
ubuntu:jammy and tagged linux_x86_64 -- a bare platform tag that PyPI
rejects and that effectively pins the wheel to the build host's glibc.
Build it as a portable manylinux_2_28_x86_64 wheel instead (glibc >= 2.28;
RHEL 8+ / Ubuntu 20.04+).
Move the build stage onto quay.io/pypa/manylinux_2_28_x86_64 (glibc 2.28)
with an explicit rustup toolchain and uv, pinned to the image's CPython
3.12. maturin's built-in auditwheel checks the manylinux_2_28 policy and
stamps the tag -- no separate auditwheel step. uv build makes maturin's
PEP517 backend force --compatibility off, so the target is re-asserted via
MATURIN_PEP517_ARGS on the build command. README notes the new tag.
Verified: auditwheel reports the wheel consistent with manylinux_2_28_x86_64
(only whitelisted libc/libm/libpthread/libgcc_s referenced, nothing bundled);
it installs and runs on the jammy runtime image; verify_release_wheels.py passes.
Signed-off-by: Anant Sharma <anants@nvidia.com>
* ci: build aiconfigurator wheels for arm64 as well as x86-64
Parameterize the wheel build stage by target architecture so it produces
both manylinux_2_28_x86_64 and manylinux_2_28_aarch64 wheels from one
Dockerfile.
BuildKit populates TARGETARCH (amd64/arm64) from --platform (default: the
build host's arch); `FROM manylinux-${TARGETARCH}` selects the matching PyPA
base (quay.io/pypa/manylinux_2_28_{x86_64,aarch64}) and only that stage is
built. MATURIN_PEP517_ARGS is unchanged -- maturin appends the arch, so the
tag resolves to _x86_64 or _aarch64 automatically. The crate is
architecture-agnostic (no target_arch code / x86 intrinsics) and the parquet
data payload is identical across arches.
Build an aarch64 wheel natively on ARM, or from an x86 host via QEMU/buildx:
docker buildx build --platform linux/arm64 --target build --load .
Also drop a stale comment that referenced a [tool.maturin] compatibility
value that is not set (the target is asserted via MATURIN_PEP517_ARGS).
amd64 build re-verified unchanged; the aarch64 base pull and rustup
toolchain install validated under buildx QEMU emulation.
Signed-off-by: Anant Sharma <anants@nvidia.com>
* fix: harden Rust bootstrap and tighten maturin pin for wheel builds
docker/Dockerfile: download the rustup installer to disk instead of piping
`curl | sh`, so a failed download aborts the build (the pipeline previously
reported only sh's exit status and silently skipped rustup). Add curl retries
for transient network blips and pin the toolchain to 1.96.0 so release wheels
build reproducibly.
pyproject.toml: require maturin>=1.5.1. 1.5.1 fixed `--compatibility` handling
when run as a PEP517 backend, which the manylinux_2_28 wheel build depends on
(MATURIN_PEP517_ARGS="--compatibility manylinux_2_28"); 1.5.0 ignored it.
Signed-off-by: Anant Sharma <anants@nvidia.com>
* docs: condense wheel-build comments in Dockerfile
Trim the verbose block comments added for the manylinux_2_28 wheel build to
their essential rationale (why manylinux/glibc 2.28, why MATURIN_PEP517_ARGS
overrides the PEP517 compatibility default, why pin the interpreter).
Signed-off-by: Anant Sharma <anants@nvidia.com>
---------
Signed-off-by: Anant Sharma <anants@nvidia.com>
(cherry picked from commit d01ce3d)
nv-nmailhot
pushed a commit
that referenced
this pull request
Jul 8, 2026
* fix: build the aiconfigurator wheel as manylinux_2_28
The wheel carries a native Rust/PyO3 (abi3) extension but was built on
ubuntu:jammy and tagged linux_x86_64 -- a bare platform tag that PyPI
rejects and that effectively pins the wheel to the build host's glibc.
Build it as a portable manylinux_2_28_x86_64 wheel instead (glibc >= 2.28;
RHEL 8+ / Ubuntu 20.04+).
Move the build stage onto quay.io/pypa/manylinux_2_28_x86_64 (glibc 2.28)
with an explicit rustup toolchain and uv, pinned to the image's CPython
3.12. maturin's built-in auditwheel checks the manylinux_2_28 policy and
stamps the tag -- no separate auditwheel step. uv build makes maturin's
PEP517 backend force --compatibility off, so the target is re-asserted via
MATURIN_PEP517_ARGS on the build command. README notes the new tag.
Verified: auditwheel reports the wheel consistent with manylinux_2_28_x86_64
(only whitelisted libc/libm/libpthread/libgcc_s referenced, nothing bundled);
it installs and runs on the jammy runtime image; verify_release_wheels.py passes.
* ci: build aiconfigurator wheels for arm64 as well as x86-64
Parameterize the wheel build stage by target architecture so it produces
both manylinux_2_28_x86_64 and manylinux_2_28_aarch64 wheels from one
Dockerfile.
BuildKit populates TARGETARCH (amd64/arm64) from --platform (default: the
build host's arch); `FROM manylinux-${TARGETARCH}` selects the matching PyPA
base (quay.io/pypa/manylinux_2_28_{x86_64,aarch64}) and only that stage is
built. MATURIN_PEP517_ARGS is unchanged -- maturin appends the arch, so the
tag resolves to _x86_64 or _aarch64 automatically. The crate is
architecture-agnostic (no target_arch code / x86 intrinsics) and the parquet
data payload is identical across arches.
Build an aarch64 wheel natively on ARM, or from an x86 host via QEMU/buildx:
docker buildx build --platform linux/arm64 --target build --load .
Also drop a stale comment that referenced a [tool.maturin] compatibility
value that is not set (the target is asserted via MATURIN_PEP517_ARGS).
amd64 build re-verified unchanged; the aarch64 base pull and rustup
toolchain install validated under buildx QEMU emulation.
* fix: harden Rust bootstrap and tighten maturin pin for wheel builds
docker/Dockerfile: download the rustup installer to disk instead of piping
`curl | sh`, so a failed download aborts the build (the pipeline previously
reported only sh's exit status and silently skipped rustup). Add curl retries
for transient network blips and pin the toolchain to 1.96.0 so release wheels
build reproducibly.
pyproject.toml: require maturin>=1.5.1. 1.5.1 fixed `--compatibility` handling
when run as a PEP517 backend, which the manylinux_2_28 wheel build depends on
(MATURIN_PEP517_ARGS="--compatibility manylinux_2_28"); 1.5.0 ignored it.
* docs: condense wheel-build comments in Dockerfile
Trim the verbose block comments added for the manylinux_2_28 wheel build to
their essential rationale (why manylinux/glibc 2.28, why MATURIN_PEP517_ARGS
overrides the PEP517 compatibility default, why pin the interpreter).
---------
(cherry picked from commit d01ce3d)
Signed-off-by: Anant Sharma <anants@nvidia.com>
Co-authored-by: Anant Sharma <anants@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The wheel carries a native Rust/PyO3 (abi3) extension but was built on ubuntu:jammy and tagged linux_x86_64 -- a bare platform tag that PyPI rejects and that effectively pins the wheel to the build host's glibc. Build it as a portable manylinux_2_28_x86_64 wheel instead (glibc >= 2.28; RHEL 8+ / Ubuntu 20.04+).
Move the build stage onto quay.io/pypa/manylinux_2_28_x86_64 (glibc 2.28) with an explicit rustup toolchain and uv, pinned to the image's CPython 3.12. maturin's built-in auditwheel checks the manylinux_2_28 policy and stamps the tag -- no separate auditwheel step. uv build makes maturin's PEP517 backend force --compatibility off, so the target is re-asserted via MATURIN_PEP517_ARGS on the build command. README notes the new tag.
Summary by CodeRabbit
Documentation
manylinux_2_28_x86_64compatibility andglibc >= 2.28requirement.Chores
manylinux_2_28multi-arch setup and deterministic compatibility tagging.maturinbuild constraint for consistency.