Skip to content

feat: support linux arm#482

Merged
qin-ctx merged 1 commit intomainfrom
linux_arm
Mar 9, 2026
Merged

feat: support linux arm#482
qin-ctx merged 1 commit intomainfrom
linux_arm

Conversation

@zhoujh01
Copy link
Collaborator

@zhoujh01 zhoujh01 commented Mar 9, 2026

Description

Related Issue

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Complex Matrix Arch Expression

The arch matrix expression is overly complex and relies on operator precedence, which could lead to unexpected behavior. Consider simplifying for readability and maintainability.

arch: ${{ contains(inputs.os_json, 'linux') && fromJson('["x86_64","aarch64"]') || (contains(inputs.os_json, 'ubuntu-latest') && contains(inputs.os_json, 'ubuntu-24.04-arm')) && fromJson('["x86_64","aarch64"]') || contains(inputs.os_json, 'ubuntu-24.04-arm') && fromJson('["aarch64"]') || fromJson('["x86_64"]') }}
Container Version Compatibility

The build-linux job uses ubuntu:20.04 container for both x86_64 and aarch64 runners. Verify that Ubuntu 20.04 container works correctly on Ubuntu 24.04 ARM host.

container: ubuntu:20.04

@qin-ctx
Copy link
Collaborator

qin-ctx commented Mar 9, 2026

/description

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Restore general Ubuntu runner check

The current condition only triggers for specific Ubuntu labels, breaking support for
custom Ubuntu runners (e.g., ubuntu-22.04). Revert to checking for the general
'ubuntu' substring to cover all Ubuntu-based runners, including the new
ubuntu-24.04-arm.

.github/workflows/_build.yml [120-126]

-if: >-
-  inputs.build_wheels &&
-  (
-    contains(inputs.os_json, 'linux') ||
-    contains(inputs.os_json, 'ubuntu-latest') ||
-    contains(inputs.os_json, 'ubuntu-24.04-arm')
-  )
+if: inputs.build_wheels && (contains(inputs.os_json, 'ubuntu') || contains(inputs.os_json, 'linux'))
Suggestion importance[1-10]: 7

__

Why: The current condition misses custom Ubuntu runners (e.g., ubuntu-22.04) by only checking specific labels. Restoring the general 'ubuntu' substring check fixes this functional regression for custom runners.

Medium
General
Simplify arch matrix condition

The arch matrix condition is overly complex and relies on fragile operator
precedence. Simplify it by breaking down the logic into explicit, parenthesized
checks for readability and maintainability.

.github/workflows/_build.yml [136]

-arch: ${{ contains(inputs.os_json, 'linux') && fromJson('["x86_64","aarch64"]') || (contains(inputs.os_json, 'ubuntu-latest') && contains(inputs.os_json, 'ubuntu-24.04-arm')) && fromJson('["x86_64","aarch64"]') || contains(inputs.os_json, 'ubuntu-24.04-arm') && fromJson('["aarch64"]') || fromJson('["x86_64"]') }}
+arch: ${{ 
+  (contains(inputs.os_json, 'linux') || (contains(inputs.os_json, 'ubuntu-latest') && contains(inputs.os_json, 'ubuntu-24.04-arm'))) 
+  ? fromJson('["x86_64", "aarch64"]') 
+  : (contains(inputs.os_json, 'ubuntu-24.04-arm') ? fromJson('["aarch64"]') : fromJson('["x86_64"]')) 
+}}
Suggestion importance[1-10]: 6

__

Why: The original arch matrix condition is overly complex and relies on fragile operator precedence. The simplified version uses explicit ternary logic for better readability and maintainability.

Low
Restore descriptive job names

The simplified job name no longer differentiates between different OSes (e.g., macOS
arm64 vs x86_64) or Python versions, making the workflow UI confusing. Restore the
original descriptive naming with matrix variables.

.github/workflows/_build.yml [291]

-name: Build non-Linux distributions
+name: Build distribution on ${{ matrix.os == 'macos-latest' && 'macOS arm64 (macos-latest)' || matrix.os == 'macos-15-intel' && 'macOS x86_64 (macos-15-intel)' || matrix.os == 'windows-latest' && 'Windows x86_64 (windows-latest)' || matrix.os }} py${{ matrix.python-version }}
Suggestion importance[1-10]: 5

__

Why: The simplified job name makes it hard to differentiate between different OSes and Python versions in the workflow UI. Restoring the original descriptive naming improves usability.

Low

@qin-ctx qin-ctx merged commit 54fa260 into main Mar 9, 2026
46 of 47 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Mar 9, 2026
@qin-ctx qin-ctx deleted the linux_arm branch March 9, 2026 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants