infernal: add osx-arm64 build #50997
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces updates to the Changes
📜 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/infernal/build.sh (2)
5-7: LGTM! Consider using$CCinstead of hardcodinggcc.The addition of these environment variables improves the build process by ensuring the correct tools and paths are used. This is particularly important for cross-compilation and consistent builds across different environments.
Consider using
$CCinstead of hardcodinggccin the LDFLAGS export:-export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib" +export LDFLAGS="${LDFLAGS} -L${PREFIX}/lib -fuse-ld=`$CC -print-prog-name=ld`"This change ensures that the correct linker is used, which is especially important for cross-compilation scenarios.
26-26: LGTM! Consider separating CFLAGS and LDFLAGS.The changes to the configure command are good improvements. Explicitly setting CC and CFLAGS ensures consistent compilation across different environments.
Consider separating CFLAGS and LDFLAGS for better clarity:
-./configure --prefix="${PREFIX}" CC="${CC}" CFLAGS="${CFLAGS} -O3 -L${PREFIX}/lib" "${ARCH_OPTS}" +./configure --prefix="${PREFIX}" CC="${CC}" CFLAGS="${CFLAGS} -O3" LDFLAGS="${LDFLAGS}" "${ARCH_OPTS}"This separation makes it clearer which flags are for compilation and which are for linking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/infernal/build.sh (2 hunks)
- recipes/infernal/meta.yaml (3 hunks)
🔇 Additional comments (7)
recipes/infernal/build.sh (2)
11-11: LGTM! Good improvement to the autoreconf command.The change from
autoreconf -itoautoreconf -ifis a good improvement. The-fflag forces the update of auxiliary files, which can help prevent build issues caused by outdated files.
Line range hint
1-28: Overall, these changes improve the build process and cross-compilation support.The modifications to this build script enhance its flexibility and robustness, particularly for cross-compilation scenarios. The addition of new environment variables, the update to the autoreconf command, and the changes to the configure command all contribute to a more consistent and reliable build process across different architectures, including the new osx-arm64 target.
These changes align well with the PR objectives and follow conda build best practices. The minor suggestions provided in the previous comments can further refine the script, but the current version is already a significant improvement.
recipes/infernal/meta.yaml (5)
13-13: LGTM: Build number increment is appropriate.The build number has been correctly incremented from 2 to 3. This is the expected practice when modifying a package without changing its version, ensuring that the new build can be distinguished from the previous one.
47-54: LGTM: Improved formatting in the 'about' section.The formatting changes in the 'about' section, including enclosing URLs and descriptions in quotes, enhance consistency and readability. These modifications align with best practices for YAML formatting.
Line range hint
1-62: Summary of changes and recommendationsThe changes to the
meta.yamlfile for the Infernal package are generally well-implemented and align with the PR objectives. Key points:
- The build number increment is appropriate.
- The addition of OpenMP dependencies suggests potential performance improvements.
- Formatting changes in the 'about' section enhance readability.
- Support for the osx-arm64 platform has been added.
To ensure these changes are fully supported and don't introduce any issues, please:
- Verify the usage and impact of the new OpenMP dependencies.
- Confirm that necessary build configurations and considerations for the osx-arm64 platform are in place.
Once these verifications are complete and any necessary adjustments are made, this PR should be ready for merging.
29-31: Verify the impact of new OpenMP dependencies.The addition of
libgompfor Linux andllvm-openmpfor macOS as runtime dependencies is appropriate. This suggests that the package now utilizes OpenMP for parallel processing, which could potentially improve performance.To ensure these changes don't introduce any issues, please run the following verification script:
This script will help verify that OpenMP is indeed used in the package and that the addition of these dependencies is justified.
59-59: Verify build configurations for osx-arm64 platform.The addition of 'osx-arm64' to the 'additional-platforms' list is appropriate and aligns with the PR objective. This extends support to Apple Silicon (M1/M2) Macs.
To ensure proper support for this new platform, please run the following verification script:
This script will help verify that necessary configurations and considerations for the osx-arm64 platform are in place.
✅ Verification successful
osx-arm64 build configurations are properly integrated.
The addition of 'osx-arm64' to the 'additional-platforms' list aligns with existing ARM64 configurations in the build scripts, ensuring support for Apple Silicon (M1/M2) Macs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify build configurations for osx-arm64 platform # Test 1: Check if there are any osx-arm64 specific configurations in the build script echo "Checking for osx-arm64 specific configurations in build.sh:" rg -i 'osx-arm64|darwin-arm64' build.sh # Test 2: Check if there are any conditional compilation flags for ARM architecture echo "Checking for ARM-specific compilation flags:" rg -i 'arm|aarch64' build.sh # Test 3: Check if there are any mentions of cross-compilation or ARM64 support in documentation echo "Checking for ARM64 support mentions in documentation:" rg -i 'arm64|apple\s+silicon|m1|m2' --type mdLength of output: 840
Script:
#!/bin/bash # Description: Search for build configuration files mentioning osx-arm64 or ARM architectures echo "Searching for build configuration files related to osx-arm64 or ARM64 support:" # Search common build scripts and configuration files rg -i 'osx-arm64|darwin-arm64|arm64|apple\s+silicon|m1|m2' --type make,cmake,sh,py,js,ts,bat,ps1,CMakeLists.txtLength of output: 350
Script:
#!/bin/bash # Description: Search for build configuration files mentioning osx-arm64 or ARM architectures echo "Searching for build configuration files related to osx-arm64 or ARM64 support:" # Correctly specifying multiple file types by using separate --type flags rg -i 'osx-arm64|darwin-arm64|arm64|apple\s+silicon|m1|m2' --type make --type cmake --type sh --type py --type js --type ts --type bat --type ps1 --type txtLength of output: 353
Script:
#!/bin/bash # Description: Search for build configuration files mentioning osx-arm64 or ARM architectures echo "Searching for build configuration files related to osx-arm64 or ARM64 support:" # Correctly specifying multiple file types by using separate --type flags, excluding unsupported 'bat' rg -i 'osx-arm64|darwin-arm64|arm64|apple\s+silicon|m1|m2' --type make --type cmake --type sh --type py --type js --type ts --type ps1 --type txtLength of output: 342
Script:
#!/bin/bash # Description: Search for build configuration files mentioning osx-arm64 or ARM architectures echo "Searching for build configuration files related to osx-arm64 or ARM64 support:" # Correctly specifying multiple file types by using separate --type flags, excluding unsupported 'bat' and 'ps1' rg -i 'osx-arm64|darwin-arm64|arm64|apple\s+silicon|m1|m2' --type make --type cmake --type sh --type py --type js --type ts --type txtLength of output: 34711
|
No artifacts found on the most recent builds. Either the builds failed, the artifacts have been removed due to age, or the recipe was blacklisted/skipped. |
|
No artifacts found on the most recent builds. Either the builds failed, the artifacts have been removed due to age, or the recipe was blacklisted/skipped. |
|
No artifacts found on the most recent builds. Either the builds failed, the artifacts have been removed due to age, or the recipe was blacklisted/skipped. |
|
No artifacts found on the most recent builds. Either the builds failed, the artifacts have been removed due to age, or the recipe was blacklisted/skipped. |
Describe your pull request here
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add labelcommand.@bioconda/corein a comment.Instructions for avoiding API, ABI, and CLI breakage issues
Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify
run_exports(see here for the rationale and comprehensive explanation).Add a
run_exportssection like this:with
...being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}{{ pin_subpackage("myrecipe", max_pin="x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}while replacing
"myrecipe"with eithernameif aname|lowervariable is defined in your recipe or with the lowercase name of the package in quotes.Bot commands for PR management
Please use the following BiocondaBot commands:
Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update@BiocondaBot please add labelplease review & mergelabel.@BiocondaBot please fetch artifactsYou can use this to test packages locally.
Note that the
@BiocondaBot please mergecommand is now depreciated. Please just squash and merge instead.Also, the bot watches for comments from non-members that include
@bioconda/<team>and will automatically re-post them to notify the addressed<team>.Summary by CodeRabbit
New Features
libgompfor Linux andllvm-openmpfor macOS.osx-arm64platform.Bug Fixes
Documentation
aboutsection for consistency, including URLs and descriptions.