Add linux-aarch64 and osx-arm64 support to rpsbproc #51929
Conversation
Fix compile flags in build.sh to enable linux-aarch64. Add linux-aarch64 and osx-arm64 to additional platforms.
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces modifications to the In Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
recipes/rpsbproc/meta.yaml (1)
Line range hint
39-67: Consider documenting architecture support in package description.The description section provides detailed information about the package but doesn't mention platform support. Consider adding a note about the supported architectures.
about: description: | The rpsbproc command line utility is an addition to the standalone version of Reverse Position-Specific BLAST (RPS-BLAST), also known as CD-Search (Conserved Domain Search). + + Supported architectures: x86_64, aarch64 (ARM64) on Linux, and x86_64, arm64 on macOS.recipes/rpsbproc/build.sh (1)
128-132: LGTM! Fix indentation for consistency.The architecture-specific logic for the
--with-64flag is correct and essential for supporting ARM architectures. The flag should indeed only be applied for x86_64 platforms.Fix the indentation to match the rest of the file:
if [[ "$(arch)" = "x86_64" ]]; then - CONFIGURE_FLAGS="$CONFIGURE_FLAGS --with-64" + CONFIGURE_FLAGS="$CONFIGURE_FLAGS --with-64" fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/rpsbproc/build.sh(1 hunks)recipes/rpsbproc/meta.yaml(1 hunks)
🔇 Additional comments (4)
recipes/rpsbproc/meta.yaml (1)
84-86: LGTM! Verify patch compatibility across architectures.
The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, let's ensure the patches are compatible with these architectures.
✅ Verification successful
Patches are architecture-aware and compatible with the new platforms
The analysis of the patches shows that update_configsub.patch already includes proper support for the new architectures:
- Contains explicit handling for
aarch64andarm64architectures - Includes Darwin (macOS) platform support
- Updates the config.sub to recognize osx-arm64
- Other patches don't contain any architecture-specific code
The patches are compatible with the newly added platforms and will work correctly for linux-aarch64 and osx-arm64.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if patches contain architecture-specific code or paths
# that might affect the newly added platforms
# Test: Look for architecture-specific patterns in patches
echo "Checking patches for architecture-specific code..."
for patch in recipes/rpsbproc/*.patch; do
echo "=== Analyzing $patch ==="
# Look for architecture-specific patterns
rg -i "x86|amd64|arm|aarch|darwin" "$patch" || true
done
Length of output: 3081
recipes/rpsbproc/build.sh (3)
Line range hint 8-16: Excellent debug information for platform verification.
The debug output will be valuable for verifying the build environment on new architectures. This will help track any platform-specific issues that arise.
Line range hint 71-106: Verify library availability for ARM architectures.
The script configures several library dependencies. Please ensure:
- All required libraries are available in bioconda for linux-aarch64 and osx-arm64
- Static linking (--without-dll --with-static-exe) works correctly on ARM platforms
Let's check the library availability:
Line range hint 17-24: Verify compiler optimizations for ARM architectures.
While the platform detection for the --with-64 flag is correct, please verify:
- If any ARM-specific compiler optimizations should be enabled
- That OpenMP support works correctly on ARM architectures (both linux-aarch64 and osx-arm64)
Let's check the configure script for available ARM-specific options:
Also applies to: 130-131
✅ Verification successful
Based on the search results, I can now provide a final response:
OpenMP support and ARM optimizations are properly handled in the build script.
The build script correctly:
- Detects the architecture and configures OpenMP appropriately (
--with-openmpfor x86 and--without-openmpfor macOS) - No ARM-specific compiler optimizations are needed as the NCBI toolkit handles architecture-specific optimizations internally
- The configure script uses standard autotools which ensures proper architecture support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ARM-related configuration options
cd "$SRC_DIR/ncbi_cxx/c++"
./configure.orig --help | grep -i "arm\|aarch"
# Check if OpenMP detection works on ARM
./configure.orig --help | grep -i "openmp"
Length of output: 387
Script:
#!/bin/bash
# Let's first check if we can find the configure script and its location
fd configure
# Check for any ARM or architecture specific compiler flags in build scripts
rg -i "arm|aarch" --type sh
# Look for OpenMP related configurations
rg -i "openmp" --type sh
Length of output: 19515
|
@bioconda/core - the CI is timing out whilst building this package on linux-aarch64. It does take a long time locally, given it is building some ncbi c++ package under the hood. Is there a way to bump the time up for this one? There is also an error in osx-arm64 builds - yet this also builds locally - it's not clear but it looks to be complaining about the version of developer tools on the osx-arm64 CI. |
|
Reposting for @dslarm to enable pings (courtesy of the BiocondaBot):
There is also an error in osx-arm64 builds - yet this also builds locally - it's not clear but it looks to be complaining about the version of developer tools on the osx-arm64 CI. |
|
arm64-osx error is: |
No. 60 mins is the maximum in the Free plan - https://support.circleci.com/hc/en-us/articles/4410707277083-Context-deadline-exceeded-after-1-hour-Build-timed-out-Free-tier-only |
|
Another thing: |
The following is also interesting:
|
Be careful with too many jobs though. Last time I tried it while building BLAST, it ate up all the memory causing the CI runner to grind to a halt and getting axed by the host. Maybe |
|
I’ve just pushed through a -j 4 .. let’s see how that flies! Thanks both.
From: Thom Griffioen ***@***.***>
Date: Tuesday, 5 November 2024 at 15:02
To: bioconda/bioconda-recipes ***@***.***>
Cc: David Lecomber ***@***.***>, Mention ***@***.***>
Subject: Re: [bioconda/bioconda-recipes] Add linux-aarch64 and osx-arm64 support to rpsbproc (PR #51929)
Another thing: currently the build uses make -j1 - https://github.com/bioconda/bioconda-recipes/pull/51929/files#diff-93630cbcb3d69000383f789843095591654c79ce6dba03a84d402bb27cbe94e0R159 You could try with make -j${CPU_COUNT} ...
Be careful with too many jobs though. Last time I tried it while building BLAST, it ate up all the memory causing the CI runner to grind to a halt and getting axed by the host. Maybe rpsbproc is a bit lighter to build, but the NCBI C++ toolchain is still quite resource-hungry. -j4 would probably be fine.
—
Reply to this email directly, view it on GitHub<#51929 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AJFYTFVKXK2YYAVRLCVWGVTZ7DMZFAVCNFSM6AAAAABRGHLQRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJXGQYDSNZZGM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
|
the correct make, and tripping over the system one(s).
Fix compile flags in build.sh to enable linux-aarch64. Add linux-aarch64 and osx-arm64 to additional platforms.