Update pepsirf to 1.7.0 #51138
Update pepsirf to 1.7.0 #51138
Conversation
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes the deletion of three build failure metadata files associated with the Changes
Possibly related PRs
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 (3)
recipes/pepsirf/build.sh (3)
1-1: Add error handling flags for safer script execution.Consider adding
set -euo pipefailafter the shebang to ensure the script exits immediately if a command fails (-e), treat unset variables as an error (-u), and catch errors in pipelines (-o pipefail). This enhances the robustness of the script.Suggested change:
#!/bin/bash +set -euo pipefail
5-6: Remove unused environment variablesINCLUDESandLIBPATH.The variables
INCLUDESandLIBPATHare exported but not utilized in the script. Unless they are required by the build system or downstream processes, consider removing them to clean up the environment.Suggested change:
-export INCLUDES="-I${PREFIX}/include" -export LIBPATH="-L${PREFIX}/lib"
10-10: Use modern command substitution syntax$(uname)instead of backticks.For better readability and to avoid potential issues with nested commands, it's recommended to use
$(command)instead of backticks`command`for command substitution.Suggested change:
-if [[ `uname` == "Darwin" ]]; then +if [[ $(uname) == "Darwin" ]]; then
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- recipes/k8/build_failure.linux-aarch64.yaml (0 hunks)
- recipes/pepsirf/build.sh (1 hunks)
- recipes/pepsirf/build_failure.linux-64.yaml (0 hunks)
- recipes/pepsirf/build_failure.osx-64.yaml (0 hunks)
- recipes/pepsirf/meta.yaml (2 hunks)
💤 Files with no reviewable changes (3)
- recipes/k8/build_failure.linux-aarch64.yaml
- recipes/pepsirf/build_failure.linux-64.yaml
- recipes/pepsirf/build_failure.osx-64.yaml
🧰 Additional context used
🪛 yamllint
recipes/pepsirf/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (7)
recipes/pepsirf/meta.yaml (5)
1-2: LGTM: Version and checksum updated correctly.The package version has been updated to 1.7.0, and the SHA256 checksum has been changed accordingly. This aligns with the PR objective of updating pepsirf to version 1.7.0.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-9: LGTM: Source URL updated correctly.The source URL has been updated to point to the 1.7.0 release tag, which is consistent with the version update.
13-15: LGTM: Build number reset and run_exports added.The build number has been correctly reset to 0 for the new version. The addition of
run_exportswithpin_subpackageis a good practice for maintaining ABI compatibility across minor versions.
38-42: LGTM: 'About' section updates improve metadata.The changes in the 'about' section enhance the package metadata:
- Minor formatting improvements to the home URL and license.
- Addition of 'license_family: GPL3' provides clearer licensing information.
- The summary now ends with a period, improving readability.
- Addition of 'dev_url' provides a direct link to the development repository.
These changes improve the overall quality and completeness of the package metadata.
1-2: Note: Ignore yamllint syntax error for '%' character.The static analysis tool (yamllint) reports a syntax error for the '%' character at the beginning of the file. This is a false positive. The '%' is part of the Jinja2 templating syntax commonly used in Conda recipes for setting variables. The syntax here is correct and necessary for the recipe to function properly.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/pepsirf/build.sh (2)
7-8:⚠️ Potential issueEnsure
LDFLAGSis properly utilized in the build process.While
CXXFLAGSis passed to CMake with-DCMAKE_CXX_FLAGS,LDFLAGSis set but not explicitly passed to CMake. To ensure linker flags are correctly applied, consider passingLDFLAGSto CMake using-DCMAKE_EXE_LINKER_FLAGS.Suggested change:
cmake -S . -B build -DCMAKE_INSTALL_PREFIX="${PREFIX}" \ -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER="${CXX}" \ -DCMAKE_CXX_FLAGS="${CXXFLAGS}" \ + -DCMAKE_EXE_LINKER_FLAGS="${LDFLAGS}" \ "${CONFIG_ARGS}"Please verify whether
LDFLAGSneeds to be passed explicitly or if it's automatically detected by CMake in your environment.
22-23: 🛠️ Refactor suggestionLeverage
cmake --installfor standardized installation.Instead of manually setting permissions and moving the executable, you can use
cmake --installto handle the installation according to theCMakeLists.txtconfiguration. This ensures that all necessary files are installed correctly and respects any install rules defined in the project.Suggested change:
cmake --build build --target pepsirf -j "${CPU_COUNT}" -v -chmod 0755 build/pepsirf -mv build/pepsirf "${PREFIX}/bin" +cmake --install build --prefix "${PREFIX}"Ensure that the project's
CMakeLists.txtincludes appropriate install targets forpepsirfso thatcmake --installworks as intended. If the install targets are not defined, you may need to add them to the project's CMake configuration.
| - {{ compiler('cxx') }} | ||
| host: | ||
| - zlib | ||
| - boost | ||
| - boost-cpp |
There was a problem hiding this comment.
💡 Codebase verification
Incomplete Replacement of 'boost' Dependencies
The dependency has been updated from boost to boost-cpp in recipes/pepsirf/meta.yaml. However, multiple references to boost still exist throughout the codebase, indicating that the replacement is not fully implemented. This may lead to inconsistencies or build issues.
Files still referencing 'boost':
recipes/build-fail-blacklistrecipes/bioconductor-netboost/meta.yamlrecipes/bte/meta.yamlrecipes/cansam/meta.yamlrecipes/galaxy-ml/meta.yamlrecipes/metagraph/meta.yamlrecipes/reseq/meta.yamlrecipes/sankoff/meta.yamlrecipes/scelestial/meta.yamlrecipes/tagger/meta.yamlrecipes/vamos/meta.yamlrecipes/scelestial/meta.yamlrecipes/reviewer/meta.yaml- (and many others as identified in the search results)
Please ensure that all instances of boost are appropriately replaced with boost-cpp where necessary to maintain consistency and avoid potential issues.
🔗 Analysis chain
Verify the impact of changing 'boost' to 'boost-cpp'.
The dependency has been updated from 'boost' to 'boost-cpp' in both build and run requirements. This change is likely intentional to specify a more targeted dependency on Boost's C++ components. However, it's important to ensure that this change doesn't negatively impact the build process or runtime behavior.
To verify the impact of this change, you can run the following script:
Also applies to: 26-26
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the package builds and tests successfully with the new boost-cpp dependency.
# Test: Attempt to build the package
conda build .
# Test: Run the package tests
conda build . --test
Length of output: 184
Script:
#!/bin/bash
# Description: Verify that 'boost' has been replaced with 'boost-cpp' throughout the codebase.
# Search for any remaining references to 'boost'
echo "Searching for remaining 'boost' references..."
rg 'boost\b' .
# Confirm that 'boost-cpp' is correctly referenced
echo "Searching for 'boost-cpp' references..."
rg 'boost-cpp' .
Length of output: 132494
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
recipes/pepsirf/build.sh (2)
5-8: LGTM: Comprehensive environment setup with a suggestion.The environment variable setup is thorough, including necessary paths and flags for compilation. The use of -O3 for high optimization is appropriate for a release build.
Consider adding a comment explaining the purpose of the
-D_LIBCPP_DISABLE_AVAILABILITYflag for better maintainability.
16-20: LGTM: Well-structured CMake configuration and build process with a suggestion.The CMake configuration and build commands are well-structured:
- Out-of-source build is correctly implemented, keeping the source directory clean.
- The installation prefix, build type, and compiler flags are properly set.
- Parallel build using CPU_COUNT can significantly speed up the build process.
Consider making the verbose output (-v flag) optional or controlled by an environment variable. This would allow for detailed output when needed without always producing excessive information.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/pepsirf/build.sh (1 hunks)
🔇 Additional comments (3)
recipes/pepsirf/build.sh (3)
3-4: LGTM: Proper directory setup.The script correctly creates the bin directory in the $PREFIX location, which is a standard practice for build scripts. This ensures flexibility in installation locations.
10-14: LGTM: Proper OS-specific configuration.The script correctly handles OS-specific configuration:
- It accurately identifies macOS using the
unamecommand.- For macOS, it disables framework and application bundle finding, which helps avoid conflicts with system libraries.
- For other systems, it appropriately leaves CONFIG_ARGS empty.
This approach ensures better cross-platform compatibility.
22-23: LGTM: Correct handling of the built executable.The final steps are handled correctly:
- The script sets appropriate executable permissions (0755) on the built file.
- The executable is correctly moved to the bin directory in $PREFIX, which is the standard location for installed binaries.
These steps ensure that the built executable is properly installed and accessible.
Update
pepsirf: 1.6.0 → 1.7.0recipes/pepsirf(click to view/edit other files)@LadnerLabThis pull request was automatically generated (see docs).