Skip to content

Fix git bullshit#155

Merged
poshul merged 3 commits intomasterfrom
fix_git_bullshit
Oct 17, 2025
Merged

Fix git bullshit#155
poshul merged 3 commits intomasterfrom
fix_git_bullshit

Conversation

@poshul
Copy link
Copy Markdown
Contributor

@poshul poshul commented Oct 17, 2025

Summary by CodeRabbit

  • Chores
    • Added ARM64 architecture support to the build and CI/CD infrastructure
    • Implemented architecture-specific build configurations for Linux ARM64 and x86_64 platforms
    • Enhanced deployment workflows with multi-architecture matrix builds
    • Updated development environment configuration

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 17, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request adds comprehensive ARM64 architecture support to the build and deployment system, introducing multi-architecture CI/CD workflows, architecture-specific build configurations, a new ARM64 Docker image for manylinux, and conditional logic in CMake and Boost/COIN-OR build scripts to handle processor-specific compilation flags.

Changes

Cohort / File(s) Summary
CI/CD Workflow Updates
.github/workflows/deploy-manylinux-docker.yml
Adds matrix-based architecture selection (amd64, arm64) with conditional runs-on logic, upgrades actions/checkout from v3 to v4, adds architecture-specific variables step for dynamic Dockerfile path and platform_tag, replaces static image tag construction with architecture-aware pattern.
CI/CD Workflow Updates
.github/workflows/main.yml
Introduces CMake arm64-specific installation step for ubuntu-22.04-arm, adds guard condition to skip standard Setup cmake step on arm64 platform.
Build Configuration
CMakeLists.txt
Makes BOOST_ARG conditional on CMAKE_SYSTEM_PROCESSOR, setting "address-model=64" only for x86_64/AMD64, otherwise empty string.
Build Configuration
libraries.cmake/boost.cmake
Adds architecture detection mapping CMAKE_SYSTEM_PROCESSOR to Boost architecture flags (x86, arm), replaces hard-coded architecture=x86 with dynamic BOOST_ARCHITECTURE variable in b2 build and install commands.
Build Configuration
libraries.cmake/coinor.cmake
Introduces BUILD_TRIPLET determination for COIN-OR configure on Linux (arm64, x86_64), replaces static flag inclusion with architecture-aware triplet injection.
Container Image
dockerfiles/pyopenms/manylinux/ARM64_Dockerfile
New ARM64 manylinux Dockerfile using quay.io/pypa/manylinux_2_34_aarch64 base, installs build tools via yum, builds OpenMS contrib components, includes metadata labels.
Repository Configuration
.gitignore
Adds .vscode/settings.json to ignore list.

Sequence Diagram(s)

sequenceDiagram
    participant CI as CI/CD Workflow
    participant Build as Build Matrix
    participant Detect as Arch Detection
    participant Compile as Compilation (Boost/COIN-OR)
    participant Docker as Docker Build
    
    CI->>Build: Trigger with matrix [amd64, arm64]
    Build->>Detect: Detect CMAKE_SYSTEM_PROCESSOR
    
    alt amd64
        Detect->>Compile: Set BOOST_ARCHITECTURE=x86, BOOST_ARG=address-model=64
        Compile->>Compile: Build with x86_64 flags
        Compile->>Docker: Use manylinux_2_34 base
    else arm64
        Detect->>Compile: Set BOOST_ARCHITECTURE=arm, BOOST_ARG=empty
        Compile->>Compile: Install CMake via apt
        Compile->>Compile: Build with arm flags & BUILD_TRIPLET
        Compile->>Docker: Use manylinux_2_34_aarch64 base
    end
    
    Docker->>Docker: Build platform-specific image
    Docker->>CI: Tag with platform_tag suffix
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The changes span multiple heterogeneous components (workflows, CMake, build scripts, Docker) requiring separate reasoning for architecture detection logic in each domain. However, the overall pattern is consistent and clear—adding ARM64 conditional branches. The interdependencies between workflow changes and build configuration updates should be verified, but the logic density remains moderate.

Poem

🐰 A rabbit's ode to multi-arch dreams,
Where processors dance in binary streams,
From x86 to ARM, we now embrace,
Building together in one shared space,
Dockerfiles bloom on aarch64's shore, 🏗️
Our workflows now support so much more!

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_git_bullshit

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 939466d and 5e72ce3.

📒 Files selected for processing (7)
  • .github/workflows/deploy-manylinux-docker.yml (2 hunks)
  • .github/workflows/main.yml (1 hunks)
  • .gitignore (1 hunks)
  • CMakeLists.txt (1 hunks)
  • dockerfiles/pyopenms/manylinux/ARM64_Dockerfile (1 hunks)
  • libraries.cmake/boost.cmake (2 hunks)
  • libraries.cmake/coinor.cmake (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@poshul poshul merged commit 491aab0 into master Oct 17, 2025
3 of 5 checks passed
@poshul poshul mentioned this pull request Oct 17, 2025
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