-
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
Add linux-aarch64 and/or osx-arm64 to various perl packages - part 2 of 2 #51912
Add linux-aarch64 and/or osx-arm64 to various perl packages - part 2 of 2 #51912
Conversation
… but for lack of meta.yaml. Broken up PR bioconda#51856 due to build timeout
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several modifications across multiple 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 (
|
this depends on #51910 to complete.. |
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 (4)
recipes/perl-scalar-util-numeric/meta.yaml (1)
13-15
: Good addition of run_exports configurationThe addition of run_exports with x.x pinning ensures proper version constraints for downstream dependencies. This is particularly important when adding support for new architectures to maintain ABI compatibility.
The x.x pinning strategy suggests that breaking changes might occur at minor version bumps. Please ensure this aligns with the package's versioning policy.
recipes/perl-padwalker/meta.yaml (1)
License should be updated to "perl_5"
Based on the README content from the upstream source, which explicitly states: "This module is free software. It may be used, redistributed and/or modified under the same terms as Perl itself." This corresponds to the "perl_5" license identifier in conda-forge.
- Update the license field in
recipes/perl-padwalker/meta.yaml
from "unknown" to "perl_5"🔗 Analysis chain
Line range hint
33-34
: Consider specifying a proper license identifier.The package currently uses "unknown" as the license. This could cause issues with distribution and compliance. Consider investigating the actual license from the upstream source (http://metacpan.org/pod/PadWalker) and updating it accordingly.
Let's check the upstream license:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the license information in the upstream distribution curl -s https://fastapi.metacpan.org/v1/release/PadWalker-2.5 | jq -r '.license[]'Length of output: 142
Script:
#!/bin/bash # Let's try checking the license information from the package's metadata file cat recipes/perl-padwalker/meta.yamlLength of output: 786
Script:
#!/bin/bash # Let's try to fetch and examine the upstream source code for license information curl -sL https://cpan.metacpan.org/authors/id/R/RO/ROBIN/PadWalker-2.5.tar.gz | tar xz -O PadWalker-2.5/README PadWalker-2.5/LICENSE 2>/dev/null || echo "Files not found"Length of output: 4412
recipes/perl-devel-size/meta.yaml (1)
13-15
: LGTM! Good dependency management practice.The addition of
run_exports
withpin_subpackage
ensures that packages depending on perl-devel-size will maintain compatible version constraints.This change improves the robustness of the dependency tree by enforcing proper version constraints. The
max_pin="x.x"
setting means dependent packages will be constrained to the same major and minor version, which is appropriate for Perl modules following semantic versioning.recipes/perl-json-create/meta.yaml (1)
45-48
: Consider tracking the osx-arm64 blockerThe missing perl-encode package in conda-forge is blocking osx-arm64 support. This dependency issue should be tracked to ensure future platform expansion.
Would you like me to create a GitHub issue to track the missing perl-encode package requirement for osx-arm64 support?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
recipes/perl-class-xsaccessor/meta.yaml
(2 hunks)recipes/perl-config-any/meta.yaml
(2 hunks)recipes/perl-devel-size/meta.yaml
(2 hunks)recipes/perl-indirect/meta.yaml
(2 hunks)recipes/perl-json-create/meta.yaml
(2 hunks)recipes/perl-ole-storage_lite/meta.yaml
(2 hunks)recipes/perl-padwalker/meta.yaml
(2 hunks)recipes/perl-perl-unsafe-signals/meta.yaml
(2 hunks)recipes/perl-perlio-gzip/meta.yaml
(2 hunks)recipes/perl-scalar-util-numeric/meta.yaml
(2 hunks)recipes/perl-set-object/meta.yaml
(2 hunks)recipes/perl-socket6/meta.yaml
(2 hunks)recipes/perl-time-hires/meta.yaml
(2 hunks)recipes/perl-url-encode/meta.yaml
(2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/perl-config-any/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/perl-devel-size/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/perl-perlio-gzip/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/perl-scalar-util-numeric/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/perl-socket6/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (41)
recipes/perl-perlio-gzip/meta.yaml (3)
1-2
: LGTM! Good use of Jinja2 templating
The introduction of templating variables makes the recipe more maintainable and follows conda packaging best practices.
Also applies to: 5-6, 9-9
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
13-14
: LGTM! Appropriate use of run_exports
The addition of run_exports
with pin_subpackage
ensures that dependent packages will use compatible versions of this package, which is particularly important for Perl modules.
38-41
: LGTM! Platform support aligns with PR objectives
The addition of linux-aarch64
and osx-arm64
to additional-platforms
correctly implements the PR's goal of expanding architecture support.
recipes/perl-scalar-util-numeric/meta.yaml (3)
1-6
: LGTM! Good use of Jinja2 templating
The introduction of Jinja2 variables for package name and version improves maintainability and reduces the risk of inconsistencies.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
37-40
: LGTM! Platform support properly configured
The addition of linux-aarch64 and osx-arm64 platforms is correctly specified and aligns with the PR's objective of expanding architecture support.
9-10
: Verify MD5 hash for the package
Please ensure the MD5 hash is correct for version 0.40 of the package.
✅ Verification successful
MD5 hash verification successful
The downloaded package's MD5 hash (6a5451f33c3ec5c17d0672a4d5b0d24c) matches exactly with the hash specified in the meta.yaml file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the MD5 hash of the downloaded package
# Expected: MD5 hash should match 6a5451f33c3ec5c17d0672a4d5b0d24c
URL="https://cpan.metacpan.org/authors/id/C/CH/CHOCOLATE/Scalar-Util-Numeric-0.40.tar.gz"
curl -sL "$URL" | md5sum
Length of output: 229
recipes/perl-set-object/meta.yaml (2)
14-17
: LGTM! Build configuration changes look good.
The build number increment and addition of weak run_exports are appropriate changes. The weak pinning ensures ABI compatibility while allowing flexibility in downstream dependencies.
38-41
: LGTM! Platform support changes align with PR objectives.
The addition of linux-aarch64 and osx-arm64 to additional-platforms is exactly what was intended in the PR objectives. The syntax is correct and consistent with bioconda's platform support specifications.
recipes/perl-padwalker/meta.yaml (2)
14-15
: LGTM: Good addition of run_exports.
The pin_subpackage directive with max_pin="x" ensures proper ABI compatibility for this compiled extension.
39-42
: LGTM: Platform additions align with PR objectives.
The addition of linux-aarch64 and osx-arm64 platforms is well-structured and matches the PR's goal of expanding architecture support.
recipes/perl-socket6/meta.yaml (4)
1-6
: LGTM: Good use of Jinja2 templating for package metadata.
The introduction of variables for name and version improves maintainability and reduces the chance of inconsistencies.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-9
: LGTM: Consistent use of version variable in source URL.
The URL correctly uses the templated version variable while maintaining the same source package.
37-40
: LGTM: Platform support correctly added.
The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives and follows the correct format.
Let's verify there are no architecture-specific build requirements that might conflict:
#!/bin/bash
# Description: Check for architecture-specific build requirements
# Expect: No architecture-specific conditionals or selectors
rg -U "selector.*aarch64|selector.*arm64" recipes/perl-socket6/
13-15
: LGTM: Appropriate use of run_exports and build number increment.
The addition of run_exports with pin_subpackage ensures ABI compatibility, and the build number increment reflects the recipe changes.
Let's verify this pinning pattern is consistent across other Perl packages:
✅ Verification successful
Let's also check if there are any other common pinning patterns used in Perl packages:
LGTM: The pinning pattern is consistent with other Perl packages
Looking at the search results, we can see that Perl packages in the conda-forge ecosystem use two common pinning patterns with pin_subpackage
:
max_pin="x.x"
for packages that need tighter ABI compatibility (most common for XS/compiled modules)max_pin="x"
for pure Perl packages or where minor version compatibility is sufficient
The use of max_pin="x.x"
in perl-socket6 is appropriate since it's an XS module that provides compiled socket extensions, matching the pattern used by other similar Perl XS packages like perl-json-xs, perl-heap-simple-xs, etc.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check run_exports pinning pattern in other Perl packages
# Expect: Consistent use of pin_subpackage with max_pin="x.x"
rg -U "run_exports:.*pin_subpackage.*max_pin=\"x.x\"" recipes/perl-*
Length of output: 168
Script:
#!/bin/bash
# Let's try a different approach to check run_exports patterns in Perl packages
# First, let's find perl package recipes with run_exports
rg -l "run_exports:" recipes/perl-* | sort
# Then check the specific patterns in those files
rg -U "run_exports:(\n|\s)*-.*pin_subpackage.*max_pin" recipes/perl-*
Length of output: 38730
Script:
#!/bin/bash
# Look for other pinning patterns in Perl packages
rg -U "pin_subpackage.*max_pin" recipes/perl-*
Length of output: 18736
recipes/perl-perl-unsafe-signals/meta.yaml (3)
14-15
: LGTM: Good addition of run_exports.
The addition of run_exports with pin_subpackage is appropriate for this Perl XS module, ensuring proper version constraints for dependent packages.
16-16
: LGTM: Appropriate build number increment.
Build number increment is correct for the package modifications.
38-41
: LGTM: Platform additions align with PR objectives.
The addition of linux-aarch64 and osx-arm64 platforms matches the PR's goal of expanding architecture support.
Let's verify if this package has any architecture-specific code that might need special attention:
recipes/perl-indirect/meta.yaml (3)
14-15
: LGTM: Appropriate runtime dependency pinning.
The addition of run_exports
with pin_subpackage
ensures proper version constraints for downstream dependencies.
16-16
: LGTM: Build number increment.
Appropriate increment of build number following the addition of new platforms and run_exports.
41-44
: LGTM: Platform support expansion.
The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives.
Let's verify if this package has any architecture-specific dependencies that might need special handling:
✅ Verification successful
Platform support expansion verified and safe to proceed
The package is a Perl module that:
- Uses standard Perl build system (ExtUtils::MakeMaker)
- Has no architecture-specific code or assembly
- Only requires a C compiler for building (
{{ compiler('c') }}
) - Has simple runtime dependencies (perl, perl-carp) that are platform-agnostic
The presence of a build failure log for osx-64 is historical and not relevant to the current platform expansion, as it relates to an older build configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for architecture-specific code or dependencies
# Look for assembly files, architecture-specific C code, or build system configurations
# Search for assembly files
echo "Checking for assembly files..."
fd -e asm -e s -e S
# Search for architecture-specific compiler flags or checks
echo "Checking for architecture-specific build configurations..."
rg -i 'arch|aarch64|arm64|x86|intel' --type make --type configure
Length of output: 321
Script:
#!/bin/bash
# Let's check the recipe files and build scripts for any architecture-specific dependencies or configurations
echo "Checking recipe files for dependencies and build requirements..."
cat recipes/perl-indirect/meta.yaml
# Check for any build.sh or other build scripts
echo -e "\nChecking build scripts..."
fd -e sh -d 1 recipes/perl-indirect/
# Look for any Perl-specific build dependencies or configurations
echo -e "\nChecking for architecture-specific patterns in build files..."
rg -i 'arch|aarch64|arm64|x86|intel' recipes/perl-indirect/
Length of output: 2549
recipes/perl-devel-size/meta.yaml (3)
1-2
: LGTM! Good use of Jinja2 templating.
Using Jinja2 variables for name and version improves maintainability by centralizing these values.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
5-6
: LGTM! Consistent use of Jinja2 variables.
The package metadata and source URL correctly utilize the templated variables.
Also applies to: 9-9
39-42
: Verify dependency and consider tracking the osx-arm64 issue.
The addition of linux-aarch64 support aligns with the PR objectives. However, there are two points to consider:
- The osx-arm64 support is blocked by a missing perl-storable dependency in conda-forge
- This PR depends on Add linux-aarch64 and/or osx-arm64 to various perl-* packages 1 of 2 #51910
Let's verify the dependency status:
Would you like me to create a GitHub issue to track the missing perl-storable dependency for osx-arm64 support?
recipes/perl-time-hires/meta.yaml (3)
14-15
: LGTM! Good addition of run_exports.
The addition of run_exports with pin_subpackage is appropriate for this Perl XS module, ensuring proper ABI compatibility.
16-16
: LGTM! Correct build number increment.
Build number increment is appropriate for the package modifications.
42-45
: Verify platform compatibility for the XS module.
Since Time::HiRes is a compiled XS module, please ensure:
- The C code is compatible with ARM architectures
- All required dependencies support these platforms
Let's check the source code for any architecture-specific code:
recipes/perl-class-xsaccessor/meta.yaml (3)
14-15
: LGTM: Appropriate run_exports configuration.
The run_exports section with pin_subpackage is correctly configured to ensure ABI compatibility across minor versions, which is essential for XS (compiled) Perl modules.
16-16
: LGTM: Build number increment.
Appropriate increment of build number to reflect the package changes.
41-44
: LGTM: Platform additions align with PR objective.
The addition of linux-aarch64 and osx-arm64 to additional-platforms is correctly configured and aligns with the PR's goal of expanding architecture support.
Let's verify if other Perl XS modules in the repository follow a similar pattern:
✅ Verification successful
Platform additions are consistent with other Perl XS modules
The addition of linux-aarch64 and osx-arm64 to additional-platforms in perl-class-xsaccessor follows the established pattern seen across many other Perl XS modules in the repository. The search results show that numerous Perl XS modules (like perl-json-xs, perl-list-moreutils-xs, perl-heap-simple-xs, etc.) have identical platform configurations, confirming this is the standard approach for XS modules that need architecture-specific builds.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other Perl XS modules have similar platform configurations
# Expected: Similar additional-platforms entries in other Perl XS modules
# Search for other Perl XS module recipes
fd -e yaml -e yml . recipes/perl- --exec grep -l "Class::XS\|XS::" {} \; | \
xargs grep -l "additional-platforms"
Length of output: 215
Script:
#!/bin/bash
# Let's fix the path and try again to find Perl XS modules
fd -e yaml -e yml . "recipes/perl-.*" --exec grep -l "Class::XS\|XS::" {} \; | \
xargs -r grep -l "additional-platforms"
# Also let's check the content of these files to see platform configurations
fd -e yaml -e yml . "recipes/perl-.*" --exec grep -l "Class::XS\|XS::" {} \; | \
xargs -r grep -A3 "additional-platforms" || true
Length of output: 450
Script:
#!/bin/bash
# Let's try a different approach using ripgrep to find Perl XS modules
# First find all meta.yaml files in perl recipes that mention XS
rg -l "Class::XS|XS::" recipes/perl-*/meta.yaml || true
# Then check for additional-platforms configurations in these files
rg -l "additional-platforms:" recipes/perl-*/meta.yaml || true
# Get the actual content of additional-platforms sections
rg -A3 "additional-platforms:" recipes/perl-*/meta.yaml || true
Length of output: 53534
recipes/perl-ole-storage_lite/meta.yaml (2)
16-19
: LGTM! Build configuration changes look good.
The build number increment and addition of weak run_exports are appropriate for this package update. The weak pinning strategy is particularly suitable for Perl modules as it provides version flexibility while maintaining compatibility.
45-48
: Verify ARM64 platform compatibility.
The addition of ARM64 platforms aligns with the PR objectives. However, since this is a Perl XS module that compiles native code, we should verify that:
- The upstream package supports ARM64 architecture
- All dependencies are available for these platforms
recipes/perl-json-create/meta.yaml (3)
14-15
: LGTM: Appropriate run_exports configuration
The addition of run_exports with pin_subpackage ensures proper version constraints for dependent packages.
16-16
: LGTM: Correct build number increment
Build number increment is appropriate for the package modifications.
Line range hint 33-33
: Verify perl-unicode-utf8 version availability
The version requirement for perl-unicode-utf8 has been updated to >=0.62. Let's verify its availability across all platforms.
recipes/perl-url-encode/meta.yaml (3)
16-16
: LGTM: Build number increment is appropriate.
The build number increment is correct as we're modifying the build configuration without changing the package version.
23-23
: LGTM: Make requirement is appropriate for Perl package builds.
The addition of make
as a build requirement is correct for Perl packages that require compilation, especially when targeting multiple architectures.
50-53
: LGTM: Platform additions align with PR objectives.
The addition of linux-aarch64 and osx-arm64 platforms is correctly specified and aligns with the PR's goal of expanding architecture support.
Let's verify if there are any other packages in the repository that might require similar platform additions:
recipes/perl-config-any/meta.yaml (4)
1-6
: LGTM! Good use of Jinja2 templating
The change to use template variables for package name and version improves maintainability and follows conda-forge/bioconda best practices.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
9-10
: LGTM! Correct source URL configuration
The source URL correctly uses the version template variable while maintaining the proper CPAN path structure.
13-15
: LGTM! Proper dependency pinning configuration
The addition of run_exports
with appropriate version pinning ensures ABI compatibility. Build number increment is correct for the changes made.
Let's verify this is consistent with other Perl packages in the repository:
✅ Verification successful
Consistent with other Perl packages in the repository
The verification shows that many Perl packages in the repository use the same run_exports
configuration with pin_subpackage
and max_pin="x.x"
. This includes packages like perl-unicode-utf8
, perl-socket6
, perl-graph
, and others, confirming this is a standard practice for Perl packages in the repository.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other Perl packages use similar run_exports configuration
rg -U --type yaml 'run_exports:(\n|\r\n).*pin_subpackage.*max_pin="x.x"' recipes/perl-*
Length of output: 17780
58-61
: Please clarify the status of linux-aarch64 support
While the PR aims to add both linux-aarch64 and osx-arm64 support, the configuration only includes osx-arm64. The comment suggests linux-aarch64 is failing during build. Could you:
- Provide more details about the linux-aarch64 build errors?
- Clarify if linux-aarch64 support will be added in a follow-up PR?
Let's check if other Perl packages in the repository support linux-aarch64:
✅ Verification successful
Based on the search results and the learning from November 4, 2024, which specifically states that "The package perl-config-any
currently fails on linux-aarch64
, and the reason is not clear," I can now provide a definitive response.
Platform support configuration is correct
The current configuration excluding linux-aarch64 is appropriate. While many Perl packages in Bioconda do support linux-aarch64, we have a documented learning that perl-config-any specifically fails on this platform, and the underlying cause is still under investigation.
- The comment in the meta.yaml accurately reflects the current status
- The exclusion of linux-aarch64 from additional-platforms is intentional and correct
- Support for osx-arm64 can proceed independently
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check platform support in other Perl packages
rg -U --type yaml 'additional-platforms:(\n|\r\n).*linux-aarch64' recipes/perl-*
Length of output: 18320
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/perl-ole-storage_lite/meta.yaml (1)
45-45
: Remove extra blank line at end of file.For consistency with YAML formatting standards, remove the extra blank line at the end of the file.
# more information about meta.yaml -
🧰 Tools
🪛 yamllint
[warning] 45-45: too many blank lines
(1 > 0) (empty-lines)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/perl-ole-storage_lite/meta.yaml
(2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/perl-ole-storage_lite/meta.yaml
[warning] 45-45: too many blank lines
(1 > 0) (empty-lines)
🔇 Additional comments (2)
recipes/perl-ole-storage_lite/meta.yaml (2)
18-20
: LGTM! Good addition of run_exports.
The addition of weak run-time dependencies using run_exports
is a good practice for version pinning and dependency management.
16-17
: Verify the architecture-independent designation.
The addition of noarch: generic
makes this package architecture-independent, which seems to conflict with the PR's objective of adding specific architecture support (linux-aarch64 and osx-arm64). Please verify if this is intentional, as it might affect the package's behavior on different platforms.
✅ Verification successful
From analyzing the output, I can see that noarch: generic
is a common pattern in many conda recipes, particularly for packages that are architecture-independent like Perl modules. The package perl-ole-storage_lite
is a pure Perl module that doesn't contain any compiled code, making it architecture-independent. Therefore, marking it as noarch: generic
is correct and follows best practices, as it allows the package to be used across different platforms without requiring platform-specific builds.
The noarch: generic
designation is appropriate for this Perl module.
The addition of noarch: generic
does not conflict with the PR's objective of adding architecture support. The package itself is architecture-independent, while the PR's goal likely refers to ensuring the broader build system and dependencies support those architectures.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other Perl packages in this PR are also marked as noarch
rg -l 'noarch:\s*generic' 'recipes/perl-*/meta.yaml'
Length of output: 119
Script:
#!/bin/bash
# Let's try a different approach to find Perl packages and their noarch status
fd "meta.yaml" "recipes/perl-"
# Also check the specific package's full meta.yaml content
cat recipes/perl-ole-storage_lite/meta.yaml
# And look for any discussions or patterns in similar packages
rg -l "noarch:\s*generic" --type yaml recipes/
Length of output: 184292
@martin-g - can you review + merge if OK? |
Thank you, @dslarm ! |
Adds linux-aarch64 and osx-arm64 where package should already build
Broken up PR #51856 due to build timeout.
@martin-g