Add hmftools-peach 2.0.0 #51150
Add hmftools-peach 2.0.0 #51150
Conversation
📝 WalkthroughWalkthroughThis pull request introduces the Changes
Possibly related PRs
Suggested labels
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 (
|
|
@BiocondaBot please fetch artifacts |
|
Package(s) built are ready for inspection:
Docker image(s) built:
|
|
@BiocondaBot please add label |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
recipes/hmftools-peach/build.sh (1)
12-12: LGTM: Correct permission setting with a minor suggestion.The execute permission is correctly set for the script. For clarity, you might consider using the symbolic mode:
-chmod 0755 "${PREFIX}/bin/peach" +chmod u=rwx,go=rx "${PREFIX}/bin/peach"This change is optional but can make the permissions more readable.
recipes/hmftools-peach/meta.yaml (4)
8-10: LGTM: Source URL and checksum are correctly specified.The source URL and SHA256 checksum are properly defined. The URL correctly points to the GitHub release of the JAR file, and the use of Jinja2 variables ensures consistency.
Consider adding a comment above the source section to explain the origin of the JAR file, e.g.:
# Download JAR file from GitHub releases source: url: https://github.com/hartwigmedical/hmftools/releases/download/peach-v{{ version }}/peach_v{{ version }}.jar sha256: '{{ sha256 }}'This can help future maintainers understand the source of the package.
12-16: LGTM: Build configuration is correctly specified.The build configuration is properly set up:
- "noarch: generic" is appropriate for a Java JAR file.
- The build number is correctly set to 0 for a new package.
- The run_exports directive ensures proper version pinning for dependencies.
Consider adding a brief comment explaining the purpose of the run_exports directive, e.g.:
build: noarch: generic number: 0 # Ensure that the package is updated when its dependencies are updated run_exports: - {{ pin_subpackage("hmftools-peach", max_pin="x.x") }}This can help future maintainers understand the reasoning behind this configuration.
18-20: LGTM: Runtime requirements are correctly specified.The runtime requirement for OpenJDK 8 or higher is appropriate for a Java application. This allows for compatibility with newer Java versions while ensuring a minimum supported version.
Consider specifying an upper bound for the OpenJDK version to ensure compatibility with tested versions. For example:
requirements: run: - openjdk >=8,<12This can help prevent potential issues with untested future Java versions. However, make sure to verify the actual compatibility range of the application before making this change.
22-30: LGTM: Test command and metadata are well-defined.The test command and about section are properly specified:
- The test command checks the version of the peach tool, which is a good basic test.
- The about section includes important metadata such as the homepage, license, and a brief summary.
Consider the following minor improvements:
- Add more comprehensive tests if possible, e.g., running the tool with a sample input file.
- Include a
license_fileentry in the about section to specify the location of the license file in the source code.- Add a
dev_urlentry pointing to the GitHub repository for easier access to development resources.Example:
test: commands: - 'peach -version | grep Peach' # Add more test commands here if possible about: home: https://github.com/hartwigmedical/hmftools/blob/master/peach/README.md license: GPL-3.0-only license_family: GPL3 license_file: LICENSE # Add this line (adjust the filename if necessary) summary: Infer haplotypes for interpretation in a pharmacogenomic context dev_url: https://github.com/hartwigmedical/hmftools # Add this lineThese additions can provide more information and resources for users and maintainers of the package.
recipes/hmftools-peach/peach.sh (1)
8-20: LGTM with a minor suggestion: Robust directory resolutionThe script correctly resolves its own directory, handling symlinks. This is a good practice for ensuring the script works correctly regardless of how it's invoked.
However, there's a minor improvement we can make:
On line 19, let's add quotes to prevent potential word splitting:
-ENV_PREFIX="$(dirname $(dirname $DIR))" +ENV_PREFIX="$(dirname "$(dirname "$DIR")")"This change ensures the script behaves correctly even if the path contains spaces.
🧰 Tools
🪛 Shellcheck
[warning] 19-19: Quote this to prevent word splitting.
(SC2046)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- recipes/hmftools-peach/build.sh (1 hunks)
- recipes/hmftools-peach/meta.yaml (1 hunks)
- recipes/hmftools-peach/peach.sh (1 hunks)
🧰 Additional context used
🪛 Shellcheck
recipes/hmftools-peach/build.sh
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
recipes/hmftools-peach/peach.sh
[warning] 19-19: Quote this to prevent word splitting.
(SC2046)
[warning] 62-62: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
🪛 yamllint
recipes/hmftools-peach/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (12)
recipes/hmftools-peach/build.sh (3)
1-5: LGTM: Proper script setup and directory creation.The initial setup of the script is well-structured:
- Correct shebang for bash.
- Appropriate use of environment variables for Bioconda.
- Safe directory creation using the
-pflag.
10-11: LGTM: Proper script installation and symlink creation.The script correctly:
- Copies the
peach.shscript to the target directory.- Creates a symlink in the
bindirectory for easy access.These steps follow Bioconda best practices for making the tool accessible.
1-12: Verify the existence ofpeach.shin the recipe directory.The build script assumes the existence of a
peach.shfile in the recipe directory, which is copied to the target directory. However, this file is not present in the provided review files.Please ensure that
peach.shexists in the recipe directory. You can verify this with the following command:If
peach.shis missing, it should be added to the recipe to ensure the build process completes successfully.✅ Verification successful
Verified:
peach.shexists in the recipe directory.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence of peach.sh in the recipe directory if [ -f recipes/hmftools-peach/peach.sh ]; then echo "peach.sh exists in the recipe directory" else echo "Error: peach.sh is missing from the recipe directory" exit 1 fiLength of output: 133
🧰 Tools
🪛 Shellcheck
[warning] 7-7: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
recipes/hmftools-peach/meta.yaml (3)
1-2: LGTM: Version and checksum are correctly defined.The package version and SHA256 checksum are properly set using Jinja2 templating. This approach allows for easier updates in the future.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
4-6: LGTM: Package name and version are correctly specified.The package name "hmftools-peach" and version (using the Jinja2 variable) are properly defined. This ensures consistency with the version specified at the top of the file.
1-30: Overall, the meta.yaml file is well-structured and follows Bioconda guidelines.The recipe for hmftools-peach 2.0.0 is correctly defined with appropriate package information, build configuration, dependencies, and metadata. The suggested improvements are minor and aim to enhance maintainability and user experience.
To finalize this review:
- Consider implementing the suggested improvements in the previous comments.
- Ensure that the package builds successfully and passes all Bioconda checks.
- Verify that the SHA256 checksum matches the downloaded JAR file.
Run the following script to verify the SHA256 checksum:
This script will download the JAR file, calculate its checksum, and compare it with the one specified in the meta.yaml file.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/hmftools-peach/peach.sh (6)
1-7: LGTM: Solid script setupThe script starts with good practices:
- Correct shebang
- Proper error handling with
set -eu -o pipefail- Setting UTF-8 locale for consistency
These choices contribute to a robust and portable script.
21-28: LGTM: Flexible Java executable setupThis section intelligently sets up the Java executable:
- It starts with the Anaconda-installed Java.
- If JAVA_HOME is set and valid, it uses that instead.
This approach provides flexibility while ensuring a working Java installation is always used.
30-60: LGTM: Comprehensive argument processingThis section demonstrates excellent handling of command-line arguments:
- It separates JVM options (memory and system properties) from application arguments.
- It properly quotes arguments to handle spaces and special characters.
- It provides default memory options if none are specified.
This approach ensures flexibility in how the tool can be invoked while maintaining robustness.
69-69: LGTM: Clear script terminationThe explicit
exitcommand at the end of the script is a good practice. It clearly indicates the intended end of the script execution.
1-69: Overall assessment: Well-structured and robust scriptThis script for executing the PEACH tool is well-written and follows many best practices for shell scripting. It handles various scenarios gracefully, including:
- Proper error handling
- Flexible Java executable selection
- Comprehensive argument processing
- Intelligent choice between classpath and JAR execution
The suggested improvements are minor and mainly focus on enhancing robustness and clarity. Once these small changes are implemented, the script will be in excellent shape for production use.
🧰 Tools
🪛 Shellcheck
[warning] 19-19: Quote this to prevent word splitting.
(SC2046)
[warning] 62-62: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
64-68: LGTM with a note: Flexible Java executionThis section intelligently chooses between using
-cp(classpath) and-jarbased on the first argument, which is a good practice for flexibility.Note on
evalusage:
Whileevalis generally discouraged due to potential security risks, its use here appears safe as all variables are properly quoted and controlled. However, it's worth being aware of this for future maintenance.To ensure the safety of the
evalusage, please run the following script to check for any unquoted variables or potential injection points:✅ Verification successful
LGTM with a note: Flexible Java execution
This section intelligently chooses between using
-cp(classpath) and-jarbased on the first argument, which is a good practice for flexibility.Note on
evalusage:
Whileevalis generally discouraged due to potential security risks, its use here is safe as all variables are properly quoted and controlled. Verification did not reveal any unquoted variables or potential injection points.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential security issues in eval usage # Test: Search for eval usage and its context rg --type bash -A 5 -B 5 'eval.*\$'Length of output: 66
Script:
#!/bin/bash # Description: Check for potential security issues in eval usage with correct file type # Test: Search for eval usage and its context in shell scripts rg --type shell -A 5 -B 5 'eval.*\$'Length of output: 68
Script:
#!/bin/bash # Description: Check for potential security issues in eval usage within shell scripts # Find all .sh files and search for eval usage with variables fd --type f --extension sh | xargs grep -n 'eval .*\$'Length of output: 16639
| pass_arr=($pass_args) | ||
| if [[ ${pass_arr[0]:=} == org* ]] |
There was a problem hiding this comment.
Improve array creation to prevent word splitting
The current method of creating the pass_arr array could lead to word splitting or unexpected globbing. Let's improve it for better reliability:
Replace these lines:
-pass_arr=($pass_args)
-if [[ ${pass_arr[0]:=} == org* ]]
+if [[ $pass_args == org* ]]This change eliminates the need for the array and directly checks the $pass_args string, which is safer and avoids potential issues with word splitting or globbing.
Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Shellcheck
[warning] 62-62: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
(SC2206)
Add recipe for hmftools-peach 2.0.0
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
New Features
build.shscript for automated installation and setup of thehmftools-peachpackage.meta.yamlfile with package metadata, versioning, and requirements, including OpenJDK.peach.shscript to execute the PEACH tool with enhanced configuration and error handling.Documentation
hmftools-peachpackage, including project homepage and licensing information.