Update ms2rescore-rs to 0.4.0.post1 #49308
Conversation
|
The OSX-64 build fails with: I think the problem is that the installed deployment target is 13.3: I don't know why the wheel uses 13.0! |
BiocondaBot
changed the title
BiocondaBot
changed the title
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several changes across three main files related to the The Lastly, the Possibly related PRs
Suggested reviewers
Warning Rate limit exceeded@BiocondaBot has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 34 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (5)recipes/ms2rescore-rs/meta.yaml (5)
Given the reported OSX-64 build failures, please ensure that setup.patch addresses the macOS deployment target issue mentioned in the PR comments (wheel targeting macOS 13.0 vs system 13.3).
The minimum Python version requirement has been increased (skip: True for py<38). Please confirm:
Line range hint Given the significant version bump (0.2.0 → 0.4.0.post1), please verify that all dependency versions (especially maturin 1.5,<2.0) are still appropriate for the new version. 🧰 Tools🪛 yamllint[error] 1-1: syntax error: found character '%' that cannot start any token (syntax)
Good additions of license_family and dev_url fields, improving package metadata completeness. Line range hint Given the reported OSX build issues and the significant version bump, please ensure:
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: 1
🧹 Outside diff range and nitpick comments (1)
recipes/ms2rescore-rs/meta.yaml (1)
Line range hint
1-52: Address OSX build failureThe reported OSX-64 build failure indicates a wheel compatibility issue with macOS deployment target (13.0 vs 13.3). Consider:
- Adding explicit
MACOSX_DEPLOYMENT_TARGETin build configuration- Updating build requirements to specify compatible macOS SDK version
Example addition to fix the OSX build:
build: script_env: - MACOSX_DEPLOYMENT_TARGET=13.3 # [osx]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
recipes/ms2rescore-rs/build.sh(1 hunks)recipes/ms2rescore-rs/meta.yaml(3 hunks)recipes/ms2rescore-rs/setup.patch(1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/ms2rescore-rs/build.sh
[warning] 8-8: RUST_BACKTRACE appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (8)
recipes/ms2rescore-rs/build.sh (2)
10-10: Verify maturin build configuration for macOS compatibility
Given the reported OSX-64 build failure and wheel compatibility issues (macOS 13.0 vs 13.3), consider adding platform-specific build configurations.
Consider adding:
- Platform-specific target configuration for macOS
- Explicit minimum deployment target setting
Would you like assistance in implementing these changes?
13-13: Consider pip install flags impact
The combination of --no-deps --no-build-isolation --no-cache-dir flags:
- Skips dependency installation (
--no-deps) - Bypasses build isolation (
--no-build-isolation) - Disables pip cache (
--no-cache-dir)
While this might speed up installation, it could mask dependency-related issues. The increased verbosity (-vvv) will help in debugging.
recipes/ms2rescore-rs/setup.patch (2)
9-12: LGTM! Verify these build configuration changes resolve the OSX build issues.
The added maturin configurations are good practices for production builds:
- Explicit Python source directory specification
- Binary stripping for reduced size
- Release profile for optimizations
21-27: Verify compatibility with PyO3 0.21.2 upgrade.
The PyO3 dependency has been upgraded from 0.20.0 to 0.21.2 with added error handling features. While this is a good improvement:
- Ensure compatibility with the new PyO3 version
- Test the error handling with the new "anyhow" feature
recipes/ms2rescore-rs/meta.yaml (4)
2-3: LGTM: Version and checksum update verified
The version bump to 0.4.0 and its corresponding SHA256 checksum are correctly updated.
12-13: Verify the setup.patch contents for OSX compatibility
Given the reported OSX-64 build failure related to wheel compatibility, please ensure the setup.patch properly addresses macOS deployment target settings.
Could you share the contents of setup.patch to verify if it handles the macOS deployment target issue mentioned in the PR comments?
16-17: Review Python version constraint change
The Python version constraint has been updated from py < 37 to py < 38. Please verify:
- This aligns with the package's compatibility requirements
- This won't affect existing users negatively
43-43: LGTM: Enhanced metadata
Good addition of license_family and dev_url fields, improving package metadata completeness.
Also applies to: 45-45
| # build statically linked binary with Rust | ||
| RUST_BACKTRACE=1 |
There was a problem hiding this comment.
Address unused environment variable
The RUST_BACKTRACE=1 environment variable is set but not exported, making it ineffective for the subsequent commands.
Apply this fix:
-RUST_BACKTRACE=1
+export RUST_BACKTRACE=1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # build statically linked binary with Rust | |
| RUST_BACKTRACE=1 | |
| # build statically linked binary with Rust | |
| export RUST_BACKTRACE=1 |
🧰 Tools
🪛 Shellcheck
[warning] 8-8: RUST_BACKTRACE appears unused. Verify use (or export if used externally).
(SC2034)
BiocondaBot
changed the title
|
@mencian, the recent build fails with the same error message, but for Linux: Not sure why the wheels are not recognized for the platform. Could it have been introduced in the cleanup work in the recipe? |
|
Usually this error means that you try to use a wheel that is built for another version of Python, i.e. your wheel is for 3.11 but the current version of Python is non 3.11. |
|
Could this have to do with the manylinux tag? I noticed other recipes with Maturin get a |
|
With the following updated build script, it does build and install with the correct Python versions: #!/bin/bash
# -e = exit on first error
# -x = print every executed command
set -ex
# build statically linked binary with Rust
RUST_BACKTRACE=1
# Build the package using maturin - should produce *.whl files.
maturin build --interpreter "${PYTHON}" -b pyo3 --release --strip
# Install *.whl files using pip
${PYTHON} -m pip install target/wheels/*.whl --no-deps --no-build-isolation --no-cache-dir -vvv |
|
@martin-g, unless I misread the logs, it is building and testing with the correct Python versions. I did notice that the built wheel is manylinux_2_24, not 2_17 / 2014. Is there a requirement for manylinux tag for Bioconda? |
|
But earlier the logs show: and You could add a debug statement in build.sh to see which version of Python is actually used but to me it still looks like a mismatch of the versions. |
You might be right here! |
|
It is using the correct Python version. In the latest log (https://github.com/bioconda/bioconda-recipes/actions/runs/11930282933/job/33250889144?pr=49308), Python 3.10 is used by bioconda utils on the build system, but for building and testing 3.11 is used consistently: Do note that if run successfully, it will build and test for various Python versions. Is there a way to build and test with glibc 2.24 for manylinux_2_24 support? |
|
AFAIK there is no way to use different version of sysroot/glibc - https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/d3a9f984e7a73dc954e566463b603637c96e3ea2/recipe/conda_build_config.yaml#L17C1-L21C1 |
|
It does confuse me that the GitHub Action for PyPI manages to build a |
Update
ms2rescore-rs: 0.2.0 → 0.4.0.post1recipes/ms2rescore-rs(click to view/edit other files)This pull request was automatically generated (see docs).