panacus: add aarch64/arm64 builds #51066
Conversation
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
martin-g
added
aarch64
📝 WalkthroughWalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
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 (
|
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
![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 (1)
recipes/panacus/build.sh (1)
7-7: Excellent update to the build process.The switch from
cargo buildtocargo installwith the specified flags (--no-track --locked --verbose) is a significant improvement. It ensures better reproducibility and aligns well with Conda's package management approach. The addition ofRUST_BACKTRACE=1and settingCARGO_HOMEare also beneficial for debugging and build consistency.Consider adding a comment explaining the purpose of the
CARGO_HOMEsetting, as it's not immediately obvious why it's set to"${BUILD_PREFIX}/.cargo". For example:# Set CARGO_HOME to ensure consistent and isolated dependency management RUST_BACKTRACE=1 CARGO_HOME="${BUILD_PREFIX}/.cargo" cargo install ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/panacus/build.sh (1 hunks)
- recipes/panacus/meta.yaml (2 hunks)
🔇 Additional comments (7)
recipes/panacus/build.sh (3)
3-3: Excellent addition for improved debugging and error handling.The
set -xecommand enhances the script's robustness by enabling command tracing (-x) and immediate exit on error (-e). This change will make debugging easier and prevent the script from continuing execution after encountering errors, which is crucial for build scripts.
Line range hint
9-10: Appropriate removal of redundant binary copy.The removal of the command to copy the built binary to
$PREFIX/binis correct, as thecargo installcommand now handles this step. Retaining the copy command forpanacus-visualize.pyis also appropriate, as this script is likely not managed by Cargo.
Line range hint
1-10: Verify cross-platform build support for aarch64/arm64.The changes to the build script improve the overall build process and reproducibility. However, they don't explicitly address the PR objective of adding aarch64/arm64 builds. While the use of
cargo installwith the--lockedflag ensures consistent builds across architectures, additional configuration might be needed to enable cross-platform builds.To ensure the PR objective is met, please verify that the
meta.yamlfile includes the necessary configuration for aarch64/arm64 builds. You can use the following script to check themeta.yamlfile:If aarch64/arm64 support is not explicitly mentioned in
meta.yaml, consider adding it to thebuildsection or adjusting theskipconditions if necessary.recipes/panacus/meta.yaml (4)
13-13: LGTM: Build number increment is appropriate.The build number increment from 0 to 1 is correct. This change is necessary when modifying the package build process or expanding platform support without changing the package version.
21-21: LGTM: Addition of Rust compiler is appropriate.Adding the Rust compiler to the build requirements is necessary for building Rust-based projects. Removing the specific version constraint allows for using the latest available Rust compiler, which is generally a good practice.
Please verify if a minimum Rust version is still required for this package. If so, consider adding it back with an appropriate version constraint.
#!/bin/bash # Description: Check if there's a minimum Rust version specified in the project # Test: Search for Rust version requirements in the project files rg --type rust 'rust-version\s*=\s*"[0-9]+\.[0-9]+(\.[0-9]+)?"' $(fd -e rs -e toml)
Line range hint
1-48: Summary: Changes look good, final checks recommended.The modifications to the meta.yaml file are appropriate and consistent with the goal of adding aarch64/arm64 builds. The build number has been incremented, the Rust compiler has been added to the build requirements, and the additional platforms have been included.
Before merging, please ensure:
- The minimum Rust version requirement has been checked and added if necessary.
- Build and test processes have been verified for the new platforms (linux-aarch64 and osx-arm64).
- All Bioconda guidelines have been followed, especially regarding the
run_exportssection mentioned in the PR description.Run the following script to check if the
run_exportssection is properly configured:✅ Verification successful
Run_exports section is properly configured.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if run_exports section is properly configured # Test: Extract and display the run_exports section echo "Checking run_exports section in meta.yaml:" sed -n '/run_exports:/,/^[^ ]/p' recipes/panacus/meta.yaml | sed '$d'Length of output: 584
46-48: LGTM: Addition of aarch64/arm64 platforms is appropriate.The addition of 'linux-aarch64' and 'osx-arm64' to the 'additional-platforms' section aligns with the PR objective and increases the accessibility of the package to users with ARM-based systems.
Please ensure that the build and test processes have been verified for these new platforms. Run the following script to check if there are any platform-specific build or test instructions:
|
Done with #50991 |
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
panacuspackage, now compatible withlinux-aarch64andosx-arm64.