-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update mehari to 0.30.1 #51208
base: master
Are you sure you want to change the base?
Update mehari to 0.30.1 #51208
Conversation
📝 WalkthroughWalkthroughThe pull request introduces changes to the In the Suggested reviewers
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
recipes/mehari/meta.yaml (1)
Summary Inconsistencies and Missing Usage Instructions
- The
summary
section inREADME.md
is empty, whereasmeta.yaml
contains a detailed summary. Please updateREADME.md
to reflect the summary frommeta.yaml
to ensure consistency.- No
usage
instructions were found inREADME.md
. Please verify ifusage
details need to be added or updated to support the new version.🔗 Analysis chain
Line range hint
1-62
: Verify if additional changes are needed in the recipe.The update seems to focus on the version, checksum, and build requirements. Please confirm:
- Are there any changes needed in the build script (build.sh) to support the new version or dependencies?
- Do the test commands need to be updated to verify the new version's functionality?
- Is the summary in the "about" section still accurate for version 0.29.0?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential changes in the build process or functionality # Test 1: Check if there are any changes to the build process in the project's documentation curl -sL https://raw.githubusercontent.com/varfish-org/mehari/v0.29.0/README.md | grep -i 'build' # Test 2: Check for any new CLI options or changes in the help output curl -sL https://raw.githubusercontent.com/varfish-org/mehari/v0.29.0/README.md | grep -i 'usage'Length of output: 591
Script:
#!/bin/bash # Description: Corrected script to verify build process and CLI usage in README.md # Test 1: Check for 'build' instructions in README.md curl -sL https://raw.githubusercontent.com/varfish-org/mehari/v0.29.0/README.md | grep -i 'build' # Test 2: Check for 'usage' instructions or CLI changes in README.md curl -sL https://raw.githubusercontent.com/varfish-org/mehari/v0.29.0/README.md | grep -i 'usage' # Test 3: Check for changes in build.sh or equivalent build scripts fd build.sh | xargs grep -i 'version\|dependencies\|compiler' # Test 4: Verify if new dependencies are introduced in meta.yaml ast-grep --lang yaml --pattern $'requirements: build: $$$ libprotobuf rocksdb snappy host: $$$ run: $$$ ' # Test 5: Check if the summary has been updated to reflect new functionalities curl -sL https://raw.githubusercontent.com/varfish-org/mehari/v0.29.0/README.md | grep -i 'summary'Length of output: 149453
Script:
#!/bin/bash # Description: Corrected script to verify build process and CLI usage in README.md and meta.yaml # Navigate to the root directory of the repository cd "$(git rev-parse --show-toplevel)" || exit # Test 1: Verify 'build' instructions in README.md echo "Checking for 'build' instructions in README.md..." grep -i 'build' README.md # Test 2: Verify 'usage' instructions in README.md echo "Checking for 'usage' instructions in README.md..." grep -i 'usage' README.md # Test 3: Verify sections in meta.yaml beyond version and sha256 echo "Checking for additional sections and changes in meta.yaml..." ast-grep --lang yaml --pattern 'package: $_ version: $_ source: url: $_ sha256: $_ patches: - $_ build: skip: $_ number: $_ run_exports: - $_ requirements: build: - $_ - $_ - $_ - $_ - $_ - $_ - $_ - $_ - $_ - $_ host: - $_ - $_ - $_ - $_ - $_ run: - $_ test: commands: - $_ about: home: $_ license: $_ license_family: $_ summary: $_ dev_url: $_ extra: additional-platforms: - $_' meta.yaml # Test 4: Compare 'summary' in README.md and meta.yaml for consistency echo "Comparing 'summary' sections between README.md and meta.yaml..." readme_summary=$(grep -A3 'summary:' README.md | grep -v 'summary:') meta_summary=$(ast-grep --lang yaml 'summary: $_' meta.yaml | awk -F': ' '{print $2}') echo "Summary in README.md:" echo "$readme_summary" echo "Summary in meta.yaml:" echo "$meta_summary" if [ "$readme_summary" == "$meta_summary" ]; then echo "Summary sections are consistent." else echo "Summary sections differ." fiLength of output: 2501
Script:
#!/bin/bash # Description: Further verification to locate and inspect meta.yaml and check for 'usage' instructions # Navigate to the root directory of the repository cd "$(git rev-parse --show-toplevel)" || exit # Verify the existence of meta.yaml echo "Checking if meta.yaml exists..." if [ -f "recipes/mehari/meta.yaml" ]; then echo "meta.yaml found." else echo "meta.yaml not found in recipes/mehari/" exit 1 fi # Display relevant sections from meta.yaml to verify changes echo "Displaying 'build', 'requirements', and 'summary' sections from meta.yaml:" awk '/build:/,/requirements:/' recipes/mehari/meta.yaml awk '/summary:/,/dev_url:/' recipes/mehari/meta.yaml # Check for 'usage' instructions in README.md again to confirm absence echo "Re-checking for 'usage' instructions in README.md..." grep -i 'usage' README.md # Compare 'summary' sections again if meta.yaml is found echo "Comparing 'summary' sections between README.md and meta.yaml..." readme_summary=$(grep -A3 'summary:' README.md | grep -v 'summary:' | tr -d '\n' | tr -d ' ') meta_summary=$(grep -A3 'summary:' recipes/mehari/meta.yaml | grep -v 'summary:' | tr -d '\n' | tr -d ' ') echo "Summary in README.md: $readme_summary" echo "Summary in meta.yaml: $meta_summary" if [ "$readme_summary" == "$meta_summary" ]; then echo "Summary sections are consistent." else echo "Summary sections differ." fiLength of output: 2247
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/mehari/build.sh (1 hunks)
- recipes/mehari/meta.yaml (2 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/mehari/build.sh
[error] 5-5: Remove spaces around = to assign.
(SC2290)
[error] 6-6: Remove spaces around = to assign.
(SC2290)
🪛 yamllint
recipes/mehari/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (3)
recipes/mehari/build.sh (1)
Line range hint
1-9
: Overall build script reviewThe addition of ROCKSDB_LIB_DIR and SNAPPY_LIB_DIR environment variables is a good step to support the new dependencies in mehari 0.29.0. Once the syntax is fixed and the library locations are verified, the build script looks good to proceed.
Remember to update the meta.yaml file (if not already done) to reflect these new dependencies:
- Add rocksdb and snappy to the build requirements.
- Consider adding them to the run requirements if they are also needed at runtime.
🧰 Tools
🪛 Shellcheck
[error] 5-5: Remove spaces around = to assign.
(SC2290)
[error] 6-6: Remove spaces around = to assign.
(SC2290)
recipes/mehari/meta.yaml (2)
28-30
: Verify new build requirements and their impact.New build requirements have been added: libprotobuf, rocksdb, and snappy. Please confirm:
- Are these new dependencies necessary for version 0.29.0?
- Do they introduce any potential compatibility issues or significant changes in the package's behavior?
- Are there any corresponding changes needed in the run requirements or build script to support these new dependencies?
✅ Verification successful
New build dependencies are properly integrated and necessary.
The additions of
libprotobuf
,rocksdb
, andsnappy
inrecipes/mehari/meta.yaml
are confirmed to be necessary for version 0.29.0. These dependencies are consistently referenced in both themeta.yaml
andbuild.sh
files, ensuring proper integration and functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for mentions of new dependencies in the project's changelog or release notes # Test: Search for mentions of the new dependencies in the changelog or release notes curl -sL https://raw.githubusercontent.com/varfish-org/mehari/v0.29.0/CHANGELOG.md | grep -iE 'libprotobuf|rocksdb|snappy'Length of output: 591
Script:
#!/bin/bash # Description: Search the entire codebase for mentions of libprotobuf, rocksdb, and snappy to verify their inclusion. # Search for 'libprotobuf' rg -i 'libprotobuf' > libprotobuf_mentions.txt # Search for 'rocksdb' rg -i 'rocksdb' > rocksdb_mentions.txt # Search for 'snappy' rg -i 'snappy' > snappy_mentions.txt # Output the results echo "Libprotobuf Mentions:" cat libprotobuf_mentions.txt echo "Rocksdb Mentions:" cat rocksdb_mentions.txt echo "Snappy Mentions:" cat snappy_mentions.txtLength of output: 2607
1-2
: Verify SHA256 checksum for the new version.The version has been correctly updated to 0.29.0. Please ensure that the new SHA256 checksum (7b4d233bcce0946a4f42e3c8a422062bfadc63dee5bce3927763f7b90b1dd70d) matches the checksum of the v0.29.0 release tarball.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
recipes/mehari/meta.yaml (2)
51-51
: Acknowledged: rocksdb package limitation for linux-aarch64Thank you for adding this informative comment about the unavailability of the rocksdb package for the linux-aarch64 platform. This explanation is helpful for understanding the current limitations of the build process.
Would you like me to create a GitHub issue to track this limitation and ensure it's addressed when a compatible rocksdb package becomes available for linux-aarch64?
52-54
: Consider keeping the extra section with a clear unsupported statusInstead of commenting out the entire extra section, consider keeping it with a clear indication that linux-aarch64 is currently unsupported. This approach maintains the information for future reference while clearly showing the current status.
Here's a suggested modification:
extra: additional-platforms: # Currently unsupported due to missing rocksdb package # - linux-aarch64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/mehari/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/mehari/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (2)
recipes/mehari/meta.yaml (2)
1-2
: Ignore yamllint error for Jinja2 syntaxThe yamllint tool reports a syntax error for the '%' character at the beginning of the file. This is a false positive as the '%' is part of the Jinja2 templating syntax commonly used in conda recipe files. The syntax is correct and should not be changed.
To suppress this warning in future linting, you could add a
# yamllint disable-line rule:syntax
comment to these lines, but it's not strictly necessary as conda-build correctly interprets this syntax.🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
28-30
: New build dependencies addedThe addition of
libprotobuf
,rocksdb
, andsnappy
as build dependencies is appropriate for the updated version of mehari. These libraries are likely required for the new features or improvements in version 0.29.1.To verify the availability of these dependencies:
recipes/mehari/meta.yaml
Outdated
{% set version = "0.29.1" %} | ||
{% set sha256 = "467d1f219f02c8b23e253fb02bbe0bb2339fa0007c1707671aeb694040481961" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Version update verified
The version in meta.yaml
has been correctly updated to 0.29.1, matching the latest GitHub release. However, the PR objectives mention updating to 0.29.0. Please update the PR objectives to reflect the accurate version 0.29.1.
🔗 Analysis chain
Verify the version update
The version has been updated to 0.29.1, which is different from the 0.29.0 mentioned in the PR objectives. Please confirm if this is the intended version update.
The SHA256 checksum has been updated accordingly, which is correct when updating the package version.
To verify the correct version:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the latest release version of mehari
# Expected result: The latest release version should match the version in meta.yaml
# Fetch the latest release version from GitHub
LATEST_VERSION=$(gh release list --repo varfish-org/mehari --limit 1 | awk '{print $1}' | sed 's/^v//')
# Compare with the version in meta.yaml
YAML_VERSION=$(awk '/^{% set version = / {print $4}' recipes/mehari/meta.yaml | tr -d '"')
echo "Latest GitHub release: $LATEST_VERSION"
echo "Version in meta.yaml: $YAML_VERSION"
if [ "$LATEST_VERSION" = "$YAML_VERSION" ]; then
echo "Versions match."
else
echo "Version mismatch detected."
fi
Length of output: 420
Script:
#!/bin/bash
# Description: Correctly extract the version from meta.yaml and verify it against the latest GitHub release
# Expected result: The latest release version should match the version in meta.yaml
# Fetch the latest release version from GitHub
LATEST_VERSION=$(gh release list --repo varfish-org/mehari --limit 1 | awk '{print $1}' | sed 's/^v//')
# Extract the version from meta.yaml using grep and sed
YAML_VERSION=$(grep '^{% set version =' recipes/mehari/meta.yaml | sed -E 's/.*"([^"]+)".*/\1/')
echo "Latest GitHub release: $LATEST_VERSION"
echo "Version in meta.yaml: $YAML_VERSION"
if [ "$LATEST_VERSION" = "$YAML_VERSION" ]; then
echo "Versions match."
else
echo "Version mismatch detected."
fi
Length of output: 424
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/mehari/meta.yaml (1)
51-54
: Acknowledged: rocksdb package unavailability for linux-aarch64.The comment about the unavailability of the rocksdb package for linux-aarch64 is informative. The commented-out
extra
section suggests a potential future addition for linux-aarch64 support.Consider creating an issue to track the addition of linux-aarch64 support once the rocksdb package becomes available for this platform. This will help ensure that the support is added in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/mehari/meta.yaml (3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/mehari/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (2)
recipes/mehari/meta.yaml (2)
28-30
: LGTM: Build requirements updated appropriately.The addition of
libprotobuf
,rocksdb
, andsnappy
as build requirements is correct and aligns with the package update. These dependencies are likely necessary for the new version of mehari.
1-2
: Version discrepancy detected. Please verify the correct version.The version in
meta.yaml
has been updated to 0.29.5, which differs from both the PR objectives (mentioning 0.29.1) and the past review comment (verifying 0.29.1). Please confirm if 0.29.5 is the intended version for this update.Let's verify the latest release version:
Please update the PR objectives and the
meta.yaml
file to reflect the correct version after verification.🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/mehari/meta.yaml (1)
52-55
: Platform limitation documentedGood documentation of the linux-aarch64 platform limitation due to missing rocksdb package. Consider:
- Creating an issue to track this platform support gap
- Adding a skip condition in the build section for linux-aarch64
Would you like me to help create an issue to track the linux-aarch64 platform support?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/mehari/meta.yaml
(4 hunks)
🧰 Additional context used
🪛 yamllint
recipes/mehari/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (3)
recipes/mehari/meta.yaml (3)
1-2
: Version discrepancy detected between PR objectives and implementation
The version has been updated to 0.30.0, but this differs from:
- PR objectives (mentions 0.29.6)
- Past review comments (mentions 0.29.1)
Please clarify if this version jump to 0.30.0 is intentional and update the PR objectives accordingly.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
28-30
: Verify compatibility of new build dependencies
New build dependencies have been added:
- libprotobuf
- rocksdb =8.10.2 (pinned version)
- snappy
Please ensure:
- The pinned rocksdb version 8.10.2 is the minimum required version
- These dependencies are available in the bioconda channel
39-39
: Verify runtime dependency on rocksdb
The rocksdb runtime dependency is pinned to the same version as the build dependency (8.10.2). This is good practice for version consistency.
Update
mehari
: 0.28.1 → 0.30.1recipes/mehari
(click to view/edit other files)@varfish-org
This pull request was automatically generated (see docs).