merquryfk & fastk: add linux-aarch64 build #51369
Conversation
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces modifications to two primary files: Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🔇 Additional comments (6)
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 (3)
recipes/merquryfk/build.sh (2)
8-8: Great optimization for build performance!The modification to use
make -j"${CPU_COUNT}"is an excellent improvement:
- It enables parallel execution of the make process, utilizing multiple CPU cores.
- This can significantly reduce build times, especially on multi-core systems.
However, for improved robustness, consider adding a fallback in case
${CPU_COUNT}is not set:Consider updating the line to:
-make -j"${CPU_COUNT}" +make -j"${CPU_COUNT:-1}"This ensures that if
${CPU_COUNT}is not set, it defaults to using a single core, preventing potential issues.
Line range hint
11-20: Consider adding error checking for copy operationsWhile the current script looks good, we can further improve its robustness by adding error checking for the copy operations. This ensures that the script fails if any of the required binaries are not successfully copied.
Consider wrapping the copy operations in a function with error checking:
copy_binary() { if ! cp "$1" "$PREFIX/bin"; then echo "Failed to copy $1 to $PREFIX/bin" >&2 exit 1 fi } mkdir -p "$PREFIX/bin" copy_binary HAPmaker copy_binary ASMplot copy_binary CNplot copy_binary HAPplot copy_binary MerquryFK copy_binary KatComp copy_binary KatGC copy_binary PloidyPlotThis approach will cause the script to exit with an error if any of the copy operations fail, making it easier to diagnose issues during the build process.
recipes/merquryfk/meta.yaml (1)
53-56: LGTM: Extra section correctly specifies linux-aarch64 support.The addition of the
extrasection withlinux-aarch64as an additional platform directly addresses the PR objective. This is consistent with the macOS skip directive in the build section.Consider removing the blank line 53 for better YAML formatting:
about: license: https://github.com/thegenemyers/MERQURY.FK/blob/main/LICENSE summary: MerquryFK replaces meryl with the FastK k-mer counter suite to considerably speed up analyses. home: https://github.com/thegenemyers/MERQURY.FK - extra: additional-platforms: - linux-aarch64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/merquryfk/build.sh (1 hunks)
- recipes/merquryfk/meta.yaml (2 hunks)
🧰 Additional context used
🔇 Additional comments (3)
recipes/merquryfk/build.sh (1)
3-3: Excellent addition for improved error handling and debugging!The addition of
set -xeis a great practice for shell scripts:
-eflag ensures the script exits immediately if any command fails, preventing cascading errors.-xflag enables command tracing, which is invaluable for debugging as it prints each command before execution.These options significantly enhance the script's robustness and make troubleshooting easier.
recipes/merquryfk/meta.yaml (2)
16-17: LGTM: Build section changes are appropriate.The build number increment and the addition of the macOS skip directive are correct. These changes align with the PR objectives of adding a linux-aarch64 build. The presence of the
run_exportssection is also good for maintaining compatibility with downstream recipes.
Line range hint
1-52: LGTM: Unchanged sections maintain package consistency.The package version, requirements, and test commands remain unchanged. This consistency is appropriate for a PR that's adding a new build architecture without modifying the core package functionality.
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
martin-g
changed the title
|
@BiocondaBot please fetch artifacts |
|
Package(s) built are ready for inspection:
Docker image(s) built:
|
LGTM! |
|
@BiocondaBot please add label |
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>.