Update ncbi-amrfinderplus to 4.0.3 #51776
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request involves updates to the The 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/ncbi-amrfinderplus/meta.yaml (1)
Line range hint 31-46: Consider versioning test data sources.
Currently, test data is fetched from the master branch, which could lead to inconsistencies if the test files are updated. Consider fetching test data from the specific release tag (v4.0.3) instead.
- -O https://raw.githubusercontent.com/ncbi/amr/master/test_dna.fa \
- -O https://raw.githubusercontent.com/ncbi/amr/master/test_prot.fa \
- -O https://raw.githubusercontent.com/ncbi/amr/master/test_prot.gff \
- -O https://raw.githubusercontent.com/ncbi/amr/master/test_both.expected
+ -O https://raw.githubusercontent.com/ncbi/amr/amrfinder_v4.0.3/test_dna.fa \
+ -O https://raw.githubusercontent.com/ncbi/amr/amrfinder_v4.0.3/test_prot.fa \
+ -O https://raw.githubusercontent.com/ncbi/amr/amrfinder_v4.0.3/test_prot.gff \
+ -O https://raw.githubusercontent.com/ncbi/amr/amrfinder_v4.0.3/test_both.expected🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/ncbi-amrfinderplus/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/ncbi-amrfinderplus/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (3)
recipes/ncbi-amrfinderplus/meta.yaml (3)
Line range hint 14-17: Build configuration looks good!
The build number is correctly reset to 0 for the new version, and the run_exports pin specification is appropriate.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
Line range hint 48-67: Metadata is complete and well-documented!
All required fields including license, documentation, and identifiers are properly maintained.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
Line range hint 19-29: Verify dependency requirements for v4.0.3.
Please ensure that the existing dependency versions are still compatible with v4.0.3. Consider checking the release notes or changelog for any new requirements.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/ncbi-amrfinderplus/meta.yaml
Outdated
| {% set version = "4.0.3" %} | ||
| {% set sha256 = "80ad1e217c78024633aa6e11052f6290b445414251372e4379c373915768564d" %} |
There was a problem hiding this comment.
💡 Codebase verification
Major version update (4.0.3) introduces breaking changes - requires database update
This version update includes significant changes that users need to be aware of:
- Changed database format requiring users to run
amrfinder -Uto get database release 2024-10-22.1 or later - Modified column names in output to align with NCBI Pathogen Detection resources
- Changes in database filename patterns and hierarchy node field format
- Integration of StxTyper v1.0.27 for E. coli Stx operon typing
The package configuration in meta.yaml appropriately handles these changes:
- Build number correctly reset to 0
- Dependencies remain compatible
- Test suite includes
amrfinder -Ucommand - Run exports properly pinned at major version level (
x)
🔗 Analysis chain
Verify SHA256 and check for breaking changes.
This is a major version update (3.12.8 → 4.0.3) which might introduce breaking changes. Let's verify the source integrity.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the SHA256 checksum of the source tarball
curl -sL https://github.com/ncbi/amr/archive/amrfinder_v4.0.3.tar.gz | sha256sum
Length of output: 125
Script:
#!/bin/bash
# Let's try a different approach to verify the source and check for breaking changes
# 1. Check if the release exists and get its details
gh release view amrfinder_v4.0.3 --repo ncbi/amr --json body,tagName,name
# 2. Let's also look at the changelog or release notes in the repository
fd -g "CHANGELOG*" -g "RELEASE*" -g "HISTORY*" -X cat
# 3. Check meta.yaml for any dependency changes
cat recipes/ncbi-amrfinderplus/meta.yaml
Length of output: 3807
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
We need to enable the submodule so I'm going to try using git for checkout.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
recipes/ncbi-amrfinderplus/build.sh (1)
20-26: Track removal of temporary bug fix
This temporary fix for the Makefile issue should be removed in the next version after 4.0.3. Consider creating a tracking issue to ensure this workaround is removed in future updates.
Would you like me to create a GitHub issue to track the removal of this temporary fix in the next version?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/ncbi-amrfinderplus/build.sh (1 hunks)
- recipes/ncbi-amrfinderplus/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/ncbi-amrfinderplus/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (8)
recipes/ncbi-amrfinderplus/build.sh (4)
4-4: LGTM: Improved debug output formatting
The simplified debug output maintains clarity while providing the same diagnostic information.
8-8: Verify the impact of -O3 optimization
While the LDFLAGS addition is necessary for proper linking, the -O3 optimization flag could potentially impact build time and binary size. Please ensure this optimization level doesn't cause any issues with the package functionality.
Consider benchmarking the build with and without -O3 to evaluate the trade-offs between performance and build time.
Also applies to: 11-11
16-16: LGTM: Proper usage of build environment variables
The make command correctly incorporates build flags and environment variables. The switch to INSTALL_DIR is appropriate for installation path specification.
Also applies to: 18-18
13-14: Verify git dependency in meta.yaml
The addition of git submodule initialization aligns with the switch to a git-based workflow. The AI summary indicates git was added as a build dependency in meta.yaml, which is correct.
✅ Verification successful
Git dependency is correctly declared in meta.yaml
The verification confirms that git is properly listed as a build dependency in meta.yaml, which is required for the git submodule initialization step in build.sh.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify git is listed as a build dependency
grep -A 5 "build:" recipes/ncbi-amrfinderplus/meta.yaml
Length of output: 248
recipes/ncbi-amrfinderplus/meta.yaml (4)
9-10: LGTM: Source configuration aligns with upstream versioning
The switch to git-based source with proper version tag is appropriate. The git_rev correctly references the upstream tag format amrfinder_v{{ version }}.
20-20: LGTM: Dependencies updated appropriately
The addition of git as a build dependency is required for the git-based source. The libcurl host dependency with curl run dependency is the correct pattern for proper runtime functionality.
Also applies to: 24-25
51-54: LGTM: Metadata improvements
The about section updates improve package metadata with proper dev_url and consistent formatting.
Also applies to: 62-62
1-2: Verify SHA256 format for git-based source
The SHA256 value appears to be a git commit hash rather than a tarball checksum. When using git_url, the SHA256 should still be the checksum of the downloaded content to ensure package integrity.
Let's verify the correct checksum:
✅ Verification successful
SHA256 value is correctly set for git-based source
The SHA256 value e3007c074ac77e410914691b38d7eaaaaf2cda6c is actually correct as it matches the git commit hash of the tag amrfinder_v4.0.3. The source section is properly configured to use git-based source with git_url and git_rev, and the SHA256 in this case is meant to verify the specific git commit rather than a tarball checksum.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Clone the repo and generate the correct checksum
git clone --depth 1 --branch amrfinder_v4.0.3 https://github.com/ncbi/amr.git tmp_amr
cd tmp_amr && find . -type f -print0 | sort -z | xargs -0 sha256sum | sha256sum
Length of output: 957
Script:
#!/bin/bash
# Let's verify if this is indeed a git commit hash by checking the repository
git ls-remote --tags https://github.com/ncbi/amr.git "refs/tags/amrfinder_v4.0.3"
# Also check the source section format in meta.yaml to understand how it's configured
cat recipes/ncbi-amrfinderplus/meta.yaml | grep -A 5 "source:"
Length of output: 330
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
Update
ncbi-amrfinderplus: 3.12.8 → 4.0.3recipes/ncbi-amrfinderplus(click to view/edit other files)@ncbiThis pull request was automatically generated (see docs).