Enforce openjdk 21, part 4/4 (Purple, Sage, Sigs, Virusinterpreter) #52274
Conversation
📝 WalkthroughWalkthroughThis pull request introduces modifications to the Possibly related PRs
Suggested labels
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 (
|
![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 (1)
recipes/hmftools-purple/meta.yaml (1)
Line range hint
16-31: Recipe structure follows best practicesThe recipe demonstrates good practices:
- Proper use of
run_exportsfor dependency pinning- Well-defined R and Bioconductor dependencies
- Appropriate version constraints for critical dependencies like circos
Consider documenting the rationale for the specific version constraints in the recipe, particularly for dependencies like
circos >=0.69.6andperl-gd >=2.76, to help future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
recipes/hmftools-purple/meta.yaml(1 hunks)recipes/hmftools-sage/meta.yaml(1 hunks)recipes/hmftools-sigs/meta.yaml(1 hunks)recipes/hmftools-virus-interpreter/meta.yaml(1 hunks)
🔇 Additional comments (6)
recipes/hmftools-sigs/meta.yaml (2)
14-14: LGTM: Build number increment is appropriate
The build number increment is correct as we're modifying package requirements.
21-21: LGTM: OpenJDK requirement change aligns with project goals
The OpenJDK version constraint >=8,<=21 appropriately enforces OpenJDK 21 as the upper bound while maintaining backward compatibility.
Let's verify consistency of OpenJDK requirements across related packages:
✅ Verification successful
OpenJDK requirement update aligns with other HMF tools
The OpenJDK version constraint >=8,<=21 in hmftools-sigs is consistent with the evolving pattern in the HMF tools suite. Several other HMF tools (virus-interpreter, redux, purple) have already adopted this upper bound constraint, while others are still using the minimum requirement of >=8. This change maintains backward compatibility while adding a sensible upper bound.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify OpenJDK requirements are consistent across hmftools packages
# Expected: All hmftools packages should use the same OpenJDK version constraint
# Search for OpenJDK requirements in hmftools recipes
fd 'meta.yaml' recipes/hmftools- --exec rg -l 'openjdk' {} | \
while read -r file; do
echo "=== $file ==="
rg 'openjdk.*' "$file"
done
Length of output: 178
Script:
#!/bin/bash
# Let's try a different approach to find hmftools recipes
fd 'meta.yaml' -p 'hmftools-.*' --exec rg -l 'openjdk' {} | \
while read -r file; do
echo "=== $file ==="
rg 'openjdk.*' "$file"
done
Length of output: 171
Script:
#!/bin/bash
# Let's try a simpler approach to find all meta.yaml files with openjdk requirements
rg -l "openjdk" ".*meta.yaml$"
# Then examine the content of found files
echo "=== Found files with their openjdk requirements ==="
rg "openjdk.*" ".*meta.yaml$"
Length of output: 281
Script:
#!/bin/bash
# Search for meta.yaml files containing openjdk requirements
find . -name "meta.yaml" -type f -exec grep -l "openjdk" {} \;
# Show the openjdk requirements from these files
echo "=== OpenJDK requirements in meta.yaml files ==="
find . -name "meta.yaml" -type f -exec sh -c 'echo "=== $1 ==="; grep "openjdk" "$1"' sh {} \;
Length of output: 200000
Script:
#!/bin/bash
# Let's try a more targeted approach to find OpenJDK requirements in hmftools packages
rg -l "openjdk" recipes/hmftools-*/meta.yaml | \
while read -r file; do
echo "=== $file ==="
rg "openjdk.*" "$file"
done
Length of output: 4904
recipes/hmftools-virus-interpreter/meta.yaml (1)
14-14: LGTM! Build number increment is appropriate.
The build number increment follows Conda packaging best practices for package updates.
recipes/hmftools-purple/meta.yaml (1)
15-15: LGTM: Build number increment is appropriate
The build number increment is correct and follows Conda packaging guidelines.
recipes/hmftools-sage/meta.yaml (2)
15-15: LGTM: Build number increment is appropriate
The build number increment is correct as we're rebuilding the package with modified dependencies.
22-22: Verify OpenJDK version compatibility with upstream
While the OpenJDK version constraint >=9,<=21 aligns with the PR objectives, let's verify this range is supported by the upstream project.
| run_exports: | ||
| - {{ pin_subpackage("hmftools-virus-interpreter", max_pin="x.x") }} | ||
|
|
||
| requirements: | ||
| run: | ||
| - openjdk >=8 | ||
| - openjdk >=8,<=21 |
There was a problem hiding this comment.
💡 Codebase verification
OpenJDK constraints are inconsistent across hmftools packages
The verification reveals inconsistencies in OpenJDK version constraints across hmftools packages:
- Most packages specify only a lower bound
>=8 - A few packages (
virus-interpreter,sage,redux,purple,sigs) include an upper bound<=21 - One package (
sage) has a different lower bound>=9 - One package (
break-point-inspector) has no version constraint
The current change to virus-interpreter (>=8,<=21) is not consistent with the majority of hmftools packages, which only specify a lower bound.
🔗 Analysis chain
Verify consistency of OpenJDK constraints across hmftools packages.
While the constraint change aligns with the PR objective to enforce OpenJDK 21, let's verify this is consistent across all hmftools packages.
Consider if the upper bound of <=21 might be too restrictive for future updates. A more flexible approach might be to use >=8,<22 to allow for point releases of OpenJDK 21, while still preventing upgrades to future major versions without explicit testing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check OpenJDK constraints across hmftools packages
# Expected: All hmftools packages should have the same OpenJDK constraint
echo "Checking OpenJDK constraints in hmftools packages:"
fd -t f "meta.yaml" recipes/hmftools- -x grep -H "openjdk" {}
Length of output: 277
Script:
#!/bin/bash
# Let's try a different approach using ripgrep to find hmftools recipes
# and check their OpenJDK constraints
echo "Searching for hmftools meta.yaml files and their OpenJDK constraints:"
rg -l "hmftools" --type yaml | grep "meta.yaml" | xargs grep -H "openjdk"
Length of output: 1492
| run_exports: | ||
| - {{ pin_subpackage('hmftools-purple', max_pin="x") }} | ||
|
|
||
| requirements: | ||
| run: | ||
| - zlib | ||
| - openjdk >=8 | ||
| - openjdk >=8,<=21 |
There was a problem hiding this comment.
💡 Codebase verification
OpenJDK version constraints are inconsistent across hmftools packages
The verification reveals inconsistencies in OpenJDK version constraints across hmftools packages:
- Only 4 packages specify the upper bound
<=21: purple, redux, sage (>=9), and virus-interpreter - All other packages (19) only specify the lower bound
>=8 - One package (sage) has a different minimum version requirement
>=9
These inconsistencies should be addressed to maintain uniformity across the hmftools suite.
🔗 Analysis chain
Verify OpenJDK version constraint consistency
The OpenJDK version constraint >=8,<=21 aligns with the PR objectives. Let's verify this constraint is consistent across related packages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check OpenJDK version constraints across hmftools packages
# Expected: All hmftools packages should use the same OpenJDK version constraint
echo "Checking OpenJDK constraints in hmftools packages:"
fd 'meta.yaml$' recipes/hmftools- --exec grep -H 'openjdk' {}
Length of output: 279
Script:
#!/bin/bash
# Let's try a different approach to find hmftools packages
echo "Finding hmftools packages:"
fd 'meta.yaml$' | grep 'hmftools'
echo -e "\nChecking OpenJDK constraints in these packages:"
fd 'meta.yaml$' | grep 'hmftools' | xargs grep -H 'openjdk'
Length of output: 2266
|
@BiocondaBot please add label |
|
@luan-n-nguyen can you remind me please why this is needed? Is JDK22+ breaking things? |
|
Hi, yes this is to prevent the Ljdk.internal.vm.FillerArray issue which happens during multithreading with memory pressure. We tried changing the java code to circumvent this error, but the only surefire way to prevent the error was to ensure openjdk 22 was not used. |
Describe your pull request here
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>.