Skip to content

feat: try riscv64 #1743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: try riscv64 #1743

wants to merge 1 commit into from

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented Feb 5, 2025

@mayeut mayeut force-pushed the musl-riscv64 branch 2 times, most recently from 65a8415 to 80777f9 Compare February 6, 2025 06:43
@mayeut mayeut changed the title feat: try musllinux riscv64 feat: try riscv64 Feb 6, 2025
@mayeut mayeut force-pushed the musl-riscv64 branch 4 times, most recently from e933dc4 to 4f89353 Compare February 13, 2025 05:26
@mayeut mayeut force-pushed the musl-riscv64 branch 3 times, most recently from d2f5fc4 to 8080ec5 Compare May 11, 2025 06:45
@auvipy auvipy requested review from auvipy and Copilot July 5, 2025 10:27
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds initial riscv64 support across packaging scripts, multi-arch deployment, and CI workflows.

  • Extend finalize.sh to handle riscv64-specific tool installs
  • Update deploy_multiarch.sh to include riscv64 in image and architecture lists
  • Modify GitHub Actions matrix to build and QEMU-emulate riscv64 with manylinux_2_35

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
docker/build_scripts/finalize.sh Handle riscv64 in auditwheel tool installation cases
deploy_multiarch.sh Add manylinux_2_35 and include riscv64 in IMAGES and ARCHS
.github/workflows/build.yml Extend build matrix with riscv64, update QEMU setup condition
Comments suppressed due to low confidence (1)

.github/workflows/build.yml:87

  • [nitpick] The QEMU step now applies to both s390x and riscv64 platforms. Consider renaming the step (e.g., "Set up QEMU for multi-arch") or updating its comment to reflect the expanded scope.
      - name: Set up QEMU

*_riscv64-cmake|*_riscv64-swig) manylinux_pkg_install "${TOOL}";;
*_riscv64-patchelf)
manylinux_pkg_install cmake
pipx install patchelf==0.17.2.1
Copy link
Preview

Copilot AI Jul 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard-coding patchelf==0.17.2.1 reduces flexibility when bumping versions. Consider installing the tool via the same --require-hashes flow (e.g., use ${TOOL_PATH}) or centralize the version in a variable.

Suggested change
pipx install patchelf==0.17.2.1
pipx install "patchelf==${PATCHELF_VERSION}"

Copilot uses AI. Check for mistakes.

Comment on lines 34 to 35
manylinux_2_31) ARCHS=("armv7l" "riscv64");;
manylinux_2_35) ARCHS=("armv7l" "riscv64");;
Copy link
Preview

Copilot AI Jul 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manylinux_2_31 and manylinux_2_35 cases share identical ARCHS. You can combine them into one pattern (manylinux_2_31|manylinux_2_35) to reduce duplication.

Suggested change
manylinux_2_31) ARCHS=("armv7l" "riscv64");;
manylinux_2_35) ARCHS=("armv7l" "riscv64");;
manylinux_2_31|manylinux_2_35) ARCHS=("armv7l" "riscv64");;

Copilot uses AI. Check for mistakes.

@mayeut mayeut force-pushed the musl-riscv64 branch 3 times, most recently from 9bf17cf to d03ff20 Compare July 8, 2025 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant