Skip to content
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 1 of 2 #51910

Merged

Conversation

dslarm
Copy link
Contributor

@dslarm dslarm commented Nov 4, 2024

This breaks PR #51856 up to smaller pieces to get past build-timeout due to size of package list.
This adds osx-arm64 and linux-aarch64 where the packages would already work but for lack of meta.yaml or historic missing dependency in conda-forge.

@dslarm
Copy link
Contributor Author

dslarm commented Nov 4, 2024

@martin-g -g

Copy link
Contributor

coderabbitai bot commented Nov 4, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request introduces modifications to the meta.yaml files across several Perl packages. The key changes include the implementation of Jinja2 templating for the package name and version, allowing for dynamic assignment rather than hardcoded values. The build numbers for the packages have been incremented, and a new run_exports section has been added to specify dependencies on subpackages with maximum pinning. Additionally, an extra section has been included in each package's metadata, listing linux-aarch64 and osx-arm64 as additional supported platforms. These changes enhance the configurability of the package metadata and expand compatibility with various architectures.

Possibly related PRs

Suggested labels

aarch64, osx-arm64, please review & merge

Suggested reviewers

  • martin-g

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (3)
recipes/perl-math-random/meta.yaml (1)

Line range hint 1-41: Consider adding package maintainer metadata.

The package lacks maintainer information in the extra section, which is typically useful for long-term maintenance.

Consider adding maintainer metadata:

 extra:
   additional-platforms:
     - linux-aarch64
     - osx-arm64
+  recipe-maintainers:
+    - dslarm
🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/perl-bloom-faster/meta.yaml (1)

33-33: Consider clarifying the package license

The license is currently marked as "unknown". It would be beneficial to determine and specify the actual license for better package distribution compliance.

recipes/perl-heap-simple-xs/meta.yaml (1)

Line range hint 1-40: Consider adding license file information.

While not directly related to the platform additions, the recipe indicates an "unknown" license. Consider adding the actual license information and license_file field if available.

Would you like me to help identify the correct license information from the package's source?

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f15d991 and f84507d.

📒 Files selected for processing (10)
  • recipes/perl-bloom-faster/meta.yaml (2 hunks)
  • recipes/perl-class-methodmaker/meta.yaml (2 hunks)
  • recipes/perl-crypt-openssl-rsa/meta.yaml (2 hunks)
  • recipes/perl-file-copy-recursive/meta.yaml (2 hunks)
  • recipes/perl-filesys-df/meta.yaml (2 hunks)
  • recipes/perl-heap-simple-xs/meta.yaml (2 hunks)
  • recipes/perl-ipc-sharelite/meta.yaml (2 hunks)
  • recipes/perl-json-parse/meta.yaml (2 hunks)
  • recipes/perl-math-cdf/meta.yaml (2 hunks)
  • recipes/perl-math-random/meta.yaml (2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/perl-bloom-faster/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/perl-filesys-df/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/perl-heap-simple-xs/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/perl-math-cdf/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

recipes/perl-math-random/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (35)
recipes/perl-math-random/meta.yaml (5)

1-2: LGTM: Package metadata templating follows best practices.

The use of Jinja2 templating 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: Template variables are correctly referenced.

The package name, version, and source URL correctly use the templated values with proper Jinja2 syntax.

Also applies to: 9-9


15-15: LGTM: Build number increment is appropriate.

The build number has been incremented from 6 to 7, which is correct when making changes to the package configuration.


38-41: Verify platform compatibility for the added architectures.

The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, we should verify:

  1. That the C compiler settings in the build section support cross-compilation
  2. That there are no architecture-specific dependencies that might cause issues
✅ Verification successful

Let me analyze the results and generate a final response.

The package appears safe for the added architectures

The package has minimal dependencies (only perl as both host and run requirement) and no architecture-specific build configurations. While some Perl packages in the ecosystem do have architecture-specific limitations (as shown by the search results), Math::Random is not one of them. The package is a pure Perl implementation of random number generators, making it inherently portable across architectures.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the package has any architecture-specific dependencies
rg -A 10 "host:|run:" recipes/perl-math-random/meta.yaml

# Look for any architecture-specific build configurations in similar Perl packages
rg -l "skip: True.*aarch64|arm64" recipes/perl-*/meta.yaml

Length of output: 16369


13-14: Verify the maximum version pinning strategy.

The run_exports section with max_pin="x.x" means that only patch-level updates will be automatically accepted. For Perl packages, this is typically a conservative approach, but let's verify this aligns with the package's compatibility guarantees.

✅ Verification successful

The output shows that many Perl packages in the repository use the same max_pin="x.x" pinning strategy, including various types of packages from basic utilities to complex bioinformatics tools. This widespread adoption suggests it's a well-established convention in the conda-forge ecosystem for Perl packages.

The x.x version pinning strategy is appropriate for this Perl package.

The consistent use across hundreds of Perl packages in the conda-forge ecosystem indicates this is a proven approach. For Math::Random specifically, this conservative pinning strategy is suitable as it ensures compatibility while allowing patch-level updates, which is appropriate for a mathematical library where backwards compatibility is important.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if other Perl packages in the repository use similar pinning
rg -l 'pin_subpackage.*max_pin="x\.x"' --type yaml recipes/perl-*/meta.yaml

Length of output: 16211

recipes/perl-math-cdf/meta.yaml (4)

1-2: LGTM! Good use of Jinja2 templating.

The introduction of Jinja2 variables for package name and version follows conda-forge best practices, making the recipe more maintainable.

🧰 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 defined variables while maintaining the same package identity.

Also applies to: 9-9


36-39: Verify dependency availability on new platforms.

The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, please ensure all dependencies (especially the C compiler and Perl) are available on these platforms in conda-forge.

✅ Verification successful

Dependencies should be compatible with the new platforms

The recipe shows that the package has minimal dependencies:

  • A C compiler (available on all platforms via conda-forge)
  • Perl (a core package available on all platforms)
  • make (a basic build tool available everywhere)

Since all these dependencies are fundamental tools available across architectures in conda-forge, and there are no platform-specific constraints or skip conditions in the recipe, the addition of linux-aarch64 and osx-arm64 platforms should work as expected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check dependency availability on the new platforms
# Look for the compiler package
rg '"{{ compiler.*}}"' recipes/perl-math-cdf/meta.yaml -A 5 -B 5

# Check if perl is available for these platforms in conda-forge
gh api -H "Accept: application/vnd.github+json" \
  /repos/conda-forge/perl-feedstock/contents/recipe/meta.yaml | \
  jq -r '.content' | base64 -d | grep -A 5 'skip_list:'

Length of output: 404


Script:

#!/bin/bash
# Let's check the build requirements and host requirements in the recipe
cat recipes/perl-math-cdf/meta.yaml

# Let's also check if there are any platform-specific requirements or skip conditions
rg "skip:|skip_list:|platform:" recipes/perl-math-cdf/meta.yaml

Length of output: 828


13-15: Consider reviewing the pinning strategy.

While adding run_exports is good practice, the max_pin="x.x" might be too strict for a Perl package. Perl packages typically follow semantic versioning where patch updates are backward compatible.

Consider using max_pin="x" instead to allow patch version updates:

  run_exports:
-    - {{ pin_subpackage(name, max_pin="x.x") }}
+    - {{ pin_subpackage(name, max_pin="x") }}
recipes/perl-bloom-faster/meta.yaml (4)

1-3: LGTM! Good use of Jinja2 templating

The templating approach makes the recipe more maintainable by centralizing version-specific information.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


6-7: LGTM! Consistent use of template variables

The package metadata correctly references the template variables.


14-16: Verify the pinning strategy for run_exports

The addition of run_exports with x-pinning ensures ABI compatibility. However, let's verify if this is consistent with other Perl packages in bioconda.

✅ Verification successful

Based on the results, I can now generate the final response since we have sufficient information about the pinning strategy across Perl packages in bioconda.

The x-pinning strategy for run_exports is appropriate

The pinning strategy using max_pin="x" is consistent with the majority of Perl packages in bioconda. From the search results, we can see this same pattern in many packages like:

  • perl-json
  • perl-json-xs
  • perl-fastx-reader
  • perl-class-methodmaker
  • perl-algorithm-cluster
  • perl-biox-seq

While some packages use max_pin="x.x", the x pinning is more common for Perl packages and is appropriate here since Perl modules typically maintain backward compatibility within major versions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check pinning strategy in other Perl packages
rg -l 'pin_subpackage.*max_pin' recipes/perl-*/meta.yaml | while read -r file; do
    echo "=== $file ==="
    grep -A 2 'pin_subpackage' "$file"
done

Length of output: 23843


37-40: Verify architecture compatibility for new platforms

While adding support for linux-aarch64 and osx-arm64 aligns with the PR objectives, we should verify:

  1. The underlying C library (libbloom) supports these architectures
  2. No architecture-specific code exists in the Perl extension
✅ Verification successful

Architecture support for linux-aarch64 and osx-arm64 appears compatible

Based on the verification:

  1. No architecture-specific code or assembly was found in the source files
  2. The package uses standard build tools (make and C compiler)
  3. No direct dependencies on architecture-specific libraries
  4. The package appears to be a standard Perl XS module with C code that should be compatible with the target architectures

The build system will use the appropriate compiler and flags for each target architecture, making the addition of linux-aarch64 and osx-arm64 platforms reasonable.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for architecture-specific code
echo "=== Checking for architecture-specific code ==="
rg -l 'x86|amd64|arm|aarch64' recipes/perl-bloom-faster/

# Check if other Perl XS modules support these architectures
echo "=== Checking other Perl XS modules platform support ==="
rg -l 'additional-platforms:.*linux-aarch64' recipes/perl-*/meta.yaml

Length of output: 15998


Script:

#!/bin/bash
# Check for architecture-specific code in source files
echo "=== Checking source files for architecture-specific code ==="
fd -e c -e h -e xs . recipes/perl-bloom-faster/ -x rg -l 'x86|amd64|arm|aarch64|__asm|#if.*defined.*(__.*__)' {}

# Check build dependencies that might affect architecture support
echo "=== Checking build dependencies ==="
rg -A5 "requirements:" recipes/perl-bloom-faster/meta.yaml

# Check if libbloom is being used and its source
echo "=== Checking for libbloom usage ==="
fd . recipes/perl-bloom-faster/ -x rg -l 'libbloom' {}

Length of output: 640

recipes/perl-filesys-df/meta.yaml (4)

1-8: LGTM! Good use of Jinja2 templating

The introduction of Jinja2 variables for package metadata improves maintainability and reduces the risk of inconsistencies.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


Line range hint 1-39: Overall changes look good and align with project goals

The modifications follow conda packaging best practices and properly implement the architecture expansion goals of the PR. The use of Jinja2 templating, proper version pinning, and explicit architecture support declarations make the recipe more maintainable and robust.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


14-16: Verify consistent pinning strategy across Perl packages

The addition of run_exports with x.x pinning is appropriate for Perl packages. Let's verify this pattern is consistent across other modified packages.

#!/bin/bash
# Description: Check for consistent run_exports pinning across modified Perl packages
# Expected: All modified Perl packages should use the same pinning strategy

# Find all modified perl-* meta.yaml files
git diff --name-only origin/master | grep -E "recipes/perl-.*/meta.yaml" | while read -r file; do
    echo "=== Checking $file ==="
    # Show the run_exports section if it exists
    rg -A 2 "run_exports:" "$file" || echo "No run_exports found"
done

36-39: Verify architecture compatibility for native code compilation

Since this package requires a C compiler and includes native code, we should verify compatibility with the added architectures.

Consider adding a comment in the recipe documenting any architecture-specific considerations or testing that has been performed.

✅ Verification successful

Architecture compatibility looks safe to proceed

The package appears safe for the added architectures (linux-aarch64, osx-arm64) based on:

  • Simple C compiler requirement without architecture-specific dependencies
  • No architecture-specific skip conditions in the recipe
  • Standard Perl XS module pattern that relies on the system's C compiler
  • No platform-specific code patterns found in the recipe

The package uses standard POSIX filesystem APIs through Perl's XS system, which should work consistently across the specified architectures.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for architecture-specific code or dependencies
# Expected: No architecture-specific limitations that would prevent building on aarch64

# Look for architecture-specific code patterns
echo "=== Checking for architecture-specific code ==="
rg -i "(?:x86|amd64|arm|aarch64|endian|asm|inline|__asm)" recipes/perl-filesys-df

# Check if other packages in the dependency tree support these architectures
echo "=== Checking build requirements ==="
rg -A 10 "requirements:" recipes/perl-filesys-df/meta.yaml

Length of output: 2838


Script:

#!/bin/bash
# Let's check the recipe content and its dependencies
echo "=== Checking recipe content ==="
cat recipes/perl-filesys-df/meta.yaml

# Check for any architecture-specific files in the source code
echo "=== Checking source code ==="
rg -l "df.*\.[ch]$" recipes/perl-filesys-df/

# Check if there are any skip conditions for specific platforms
echo "=== Checking skip conditions ==="
rg "skip:" recipes/perl-filesys-df/meta.yaml -A 2

Length of output: 1097


Script:

#!/bin/bash
# Let's check the upstream source code for any architecture-specific code
echo "=== Checking upstream source ==="
# First, create a temporary directory
mkdir -p /tmp/perl-filesys-df
cd /tmp/perl-filesys-df

# Download and extract the source
curl -L https://cpan.metacpan.org/authors/id/I/IG/IGUTHRIE/Filesys-Df-0.92.tar.gz | tar xz

# Check for architecture-specific code in C files
echo "=== Checking C source files for architecture-specific code ==="
find . -type f -name "*.[ch]" -exec cat {} \;

# Return to original directory
cd -

Length of output: 754

recipes/perl-ipc-sharelite/meta.yaml (2)

14-15: LGTM! Good addition of run_exports.

The addition of run_exports with pin_subpackage ensures proper version constraints for dependent packages, which is crucial for maintaining ABI compatibility.


39-42: Verify platform compatibility.

The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, let's verify if there are any platform-specific build requirements or known issues.

✅ Verification successful

Based on the search results, I can see that several packages in the repository have ARM64/AArch64 support, including some Perl packages. However, let me verify specifically for perl-ipc-sharelite and its dependencies to ensure there are no platform-specific issues.


Let me check the meta.yaml file specifically to understand what platform-specific content it contains.


Based on the analysis of the package recipe, I can now provide a final response:

Platform compatibility verified for perl-ipc-sharelite

The package is suitable for ARM platforms (linux-aarch64 and osx-arm64) because:

  • It uses standard build tools (make, perl) that are available on ARM
  • The build process is platform-agnostic, using standard Perl build systems (Build.PL/Makefile.PL)
  • No platform-specific code or conditionals are present in the build scripts
  • The package compiles from source using the platform's native compiler (cxx)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for platform-specific issues or dependencies in related packages

# Test 1: Look for any platform-specific build requirements or patches
rg -l "(?i)(aarch64|arm64)" --type yaml

# Test 2: Check if other Perl packages have platform-specific considerations
fd -e yaml . recipes/perl- --exec rg -l "(?i)(aarch64|arm64)" {}

# Test 3: Look for any conditional build steps or patches
rg "(?i)(if.*aarch64|if.*arm64)" --type yaml

Length of output: 30449


Script:

#!/bin/bash
# Check perl-ipc-sharelite recipe and its build files for any platform-specific issues
rg -l "(?i)(skip|skip_if|platform|arch|aarch64|arm64)" recipes/perl-ipc-sharelite/

# Check build.sh for any platform-specific build steps
cat recipes/perl-ipc-sharelite/build.sh

# Check for any build failures or skip files
fd "(build_failure|skip)" recipes/perl-ipc-sharelite/

Length of output: 690


Script:

#!/bin/bash
# Check meta.yaml content for platform-specific details
cat recipes/perl-ipc-sharelite/meta.yaml

# Check if this package has been successfully built for ARM platforms before
rg -l "perl-ipc-sharelite.*\.(tar\.bz2|conda)" .conda-bld/noarch/
rg -l "perl-ipc-sharelite.*\.(tar\.bz2|conda)" .conda-bld/linux-aarch64/

Length of output: 1075

recipes/perl-heap-simple-xs/meta.yaml (5)

1-2: LGTM: Jinja2 template variables are correctly defined.

The template variables for name and version are properly defined following conda-forge packaging best practices.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


5-6: LGTM: Template variables are correctly used.

The package name, version, and source URL correctly utilize the Jinja2 template variables, ensuring consistency throughout the recipe.

Also applies to: 9-9


15-15: LGTM: Build number increment.

The build number has been properly incremented to trigger a rebuild with the new changes.


37-40: Verify platform compatibility.

The addition of linux-aarch64 and osx-arm64 platforms looks good, but we should verify there are no platform-specific dependencies that might cause issues.

Let's check for any platform-specific build requirements or known issues:

#!/bin/bash
# Check if the package has any platform-specific build requirements
rg -A 5 "skip: True  # \[.*aarch64.*\]" recipes/perl-heap-simple-xs/meta.yaml

# Check for any skipped tests or known issues on these platforms
rg -A 5 "aarch64|arm64" recipes/perl-heap-simple-xs/

13-14: Verify the run_exports pinning strategy.

The run_exports section with max_pin="x.x" means that dependent packages will be constrained to the same major and minor version. For Perl packages, this is typically more strict than necessary.

Let's check if other Perl packages in the repository use similar pinning:

✅ Verification successful

Let me try another search strategy to find run_exports patterns in Perl packages.


Based on the search results, I can now provide a final response:

The x.x pinning strategy is appropriate for this XS (compiled) Perl module

Looking at other Perl XS (compiled) modules in the repository, there's a mix of pinning strategies:

  • Some use x.x pinning: perl-heap-simple-xs, perl-math-cdf, perl-ipc-sharelite, perl-filesys-df
  • Others use x pinning: perl-json-xs, perl-class-methodmaker

For compiled Perl modules (XS), the x.x pinning is actually appropriate since binary compatibility can be affected by minor version changes. The current pinning strategy aligns with several other XS modules in the repository and provides better stability guarantees.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for run_exports patterns in Perl package recipes
rg -l "run_exports" recipes/perl-* | xargs rg "pin_subpackage.*max_pin"

Length of output: 246


Script:

#!/bin/bash
# Search for run_exports in perl packages using ripgrep
rg -U "run_exports:(\n|\s+)-.*pin_subpackage.*" recipes/perl-*

# Also get a list of perl packages with run_exports for reference
rg -l "run_exports:" recipes/perl-*

Length of output: 36114

recipes/perl-json-parse/meta.yaml (3)

14-15: LGTM: Proper use of run_exports for version pinning.

The addition of run_exports with pin_subpackage ensures that dependent packages will use compatible versions, maintaining ABI compatibility.


16-16: LGTM: Appropriate build number increment.

Build number increment from 3 to 4 is correct as metadata changes require a new build.


39-42: Verify platform compatibility before enabling.

The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, we should verify that all dependencies are available for these platforms.

recipes/perl-file-copy-recursive/meta.yaml (3)

14-15: LGTM: Proper use of run_exports for dependency management.

The addition of run_exports with pinned subpackage dependencies follows conda packaging best practices for version compatibility.


16-16: LGTM: Appropriate build number increment.

Build number increment from 3 to 4 is correct as metadata changes require a new build number.


43-46: Verify platform compatibility before enabling.

The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, we should verify that all dependencies are available for these platforms.

✅ Verification successful

Platform compatibility verification looks safe

The package has minimal dependencies that are known to be widely available across architectures:

  • Build dependencies: only make and standard C compiler
  • Host dependencies: only perl and standard perl modules
  • Run dependencies: only perl

Since these are fundamental packages that are typically available across all platforms, and perl itself is well-supported on both linux-aarch64 and osx-arm64, enabling these additional platforms should be safe.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify dependency availability for new platforms
# Check if all host and run dependencies are available for the new platforms

# Get all dependencies
echo "Checking dependencies for linux-aarch64 and osx-arm64..."
rg -A 10 'requirements:' | rg '^\s+- \w+'

# Note: In a real conda environment, you would use:
# conda search --platform linux-aarch64 <package>
# conda search --platform osx-arm64 <package>
# to verify each dependency's availability

Length of output: 164


Script:

#!/bin/bash
# Let's try to get the content of the meta.yaml file first to see all dependencies
cat recipes/perl-file-copy-recursive/meta.yaml

Length of output: 987

recipes/perl-class-methodmaker/meta.yaml (2)

15-17: LGTM: Build configuration changes look good!

The addition of run_exports with proper pinning ensures ABI compatibility, and the build number increment follows conda-build best practices.


49-52: Verify platform compatibility for the package.

The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, let's verify that there are no platform-specific build requirements or constraints in the package's build process.

recipes/perl-crypt-openssl-rsa/meta.yaml (3)

16-19: LGTM: Build number increment and run_exports configuration

The build number increment and weak pinning in run_exports section are appropriate for this Perl package.


Line range hint 22-33: Verify runtime dependency configuration

There's an inconsistency in the dependency configuration: perl-crypt-openssl-random is required for building but commented out in runtime dependencies. Please verify if this package is needed at runtime.

#!/bin/bash
# Description: Check usage of Crypt::OpenSSL::Random in the package

# Search for usage of Crypt::OpenSSL::Random in the package source
echo "Checking Crypt::OpenSSL::Random usage:"
rg "use.*Crypt::OpenSSL::Random" "recipes/perl-crypt-openssl-rsa/*/Crypt-OpenSSL-RSA-*"

51-54: Verify buildability on new architectures

The addition of linux-aarch64 and osx-arm64 platforms looks good, but we should verify that all dependencies are available on these architectures.

✅ Verification successful

Based on the search results, I can see that both linux-aarch64 and osx-arm64 platforms are widely supported across many bioconda packages. The review comment was asking to verify if all dependencies are available on these architectures. Let me check specifically for the dependencies of perl-crypt-openssl-rsa.


Let me check the dependencies more specifically.


Based on the analysis of the recipe files and dependencies:

  1. perl-crypt-openssl-guess is noarch (generic), meaning it's platform-independent
  2. openssl is a core package available on all platforms through conda-forge
  3. perl is also available on all platforms through conda-forge
  4. The package itself (perl-crypt-openssl-rsa) is a C extension that can be compiled on these architectures since it uses standard compiler toolchain

Architecture support looks valid

The addition of linux-aarch64 and osx-arm64 platforms is appropriate since:

  • All dependencies are available on these architectures
  • The package is a C extension that can be built using standard compilers
  • The underlying OpenSSL libraries are well-supported on these architectures
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify availability of dependencies on new architectures

# Check if openssl is available for the new architectures
echo "Checking openssl availability:"
rg -l "pkg: openssl" | xargs rg "linux-aarch64|osx-arm64"

# Check if perl-crypt-openssl-guess is available
echo "Checking perl-crypt-openssl-guess availability:"
rg -l "pkg: perl-crypt-openssl-guess" | xargs rg "linux-aarch64|osx-arm64"

# Check if perl-crypt-openssl-random is available
echo "Checking perl-crypt-openssl-random availability:"
rg -l "pkg: perl-crypt-openssl-random" | xargs rg "linux-aarch64|osx-arm64"

Length of output: 133345


Script:

#!/bin/bash
# Check dependencies in the recipe file
echo "Checking dependencies in perl-crypt-openssl-rsa recipe:"
cat recipes/perl-crypt-openssl-rsa/meta.yaml

# Check if openssl is available for these architectures
echo -e "\nChecking openssl availability:"
rg -l "pkg: openssl" | xargs cat

Length of output: 1501


Script:

#!/bin/bash
# Check if the direct dependencies have aarch64/arm64 support
echo "Checking perl-crypt-openssl-guess recipe:"
cat recipes/perl-crypt-openssl-guess/meta.yaml

echo -e "\nChecking perl-crypt-openssl-random recipe:"
cat recipes/perl-crypt-openssl-random/meta.yaml

# Check if perl itself supports these architectures
echo -e "\nChecking perl support:"
rg "pkg: perl" | xargs cat

Length of output: 1122

@dslarm dslarm changed the title Add linux-aarch64 and/or osx-arm64 to packages. Add linux-aarch64 and/or osx-arm64 to various perl-* packages 1 of 2 Nov 4, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 01de73f and a83dc29.

📒 Files selected for processing (1)
  • recipes/perl-math-random/meta.yaml (2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/perl-math-random/meta.yaml

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)

🔇 Additional comments (3)
recipes/perl-math-random/meta.yaml (3)

1-6: LGTM: Package definition using Jinja2 templating

The implementation of Jinja2 templating for package name and version follows conda-forge best practices.

🧰 Tools
🪛 yamllint

[error] 1-1: syntax error: found character '%' that cannot start any token

(syntax)


9-10: LGTM: Source configuration

The source URL correctly uses templating and follows CPAN's URL structure.


40-43: LGTM: Platform additions with a note

The addition of linux-aarch64 and osx-arm64 platforms aligns with the PR objectives. However, please note that the osx-arm64 support will only take effect after addressing the OSX skip directive issue mentioned above.

#!/bin/bash
# Check if these platforms are commonly supported in other perl packages
rg -l "additional-platforms:(\n|\s+)-\s+(linux-aarch64|osx-arm64)" recipes/perl-*/meta.yaml

Comment on lines +13 to +17
run_exports:
- {{ pin_subpackage(name, max_pin="x.x") }}
number: 7
# skip osx on x86 - builds fail in CI
skip: True # [osx]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Update OSX skip configuration to be architecture-specific

The review comment is correct. There is a clear inconsistency in the package configuration:

  1. The comment indicates builds should only be skipped for x86 OSX: # skip osx on x86 - builds fail in CI
  2. The skip directive skip: True # [osx] blocks all OSX builds
  3. The package explicitly lists osx-arm64 as an additional platform, which conflicts with the skip directive

The skip directive should be updated to:

skip: True  # [osx and x86_64]

This will:

  • Only skip x86_64 builds on OSX as intended
  • Allow osx-arm64 builds to proceed
  • Match the documented intention in the comment
  • Be consistent with the additional-platforms configuration
🔗 Analysis chain

Verify OSX build skip configuration

The skip directive skip: True # [osx] skips all OSX builds, but:

  1. The comment suggests it should only skip x86 builds
  2. This conflicts with the addition of osx-arm64 platform support

Consider updating the skip directive to specifically target x86:

- skip: True # [osx]
+ skip: True  # [osx and x86_64]
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify if other perl packages have similar OSX skip patterns
rg -l "skip.*osx" recipes/perl-*/meta.yaml

Length of output: 15891


Script:

#!/bin/bash
# Let's try a different approach to find OSX skip patterns
rg -l "skip.*\[osx" recipes/perl-*/meta.yaml

Length of output: 15970


Script:

#!/bin/bash
# Also check the specific file content
cat recipes/perl-math-random/meta.yaml

Length of output: 775


Script:

#!/bin/bash
# Check if there are any osx-arm64 related entries
rg "osx-arm64" recipes/perl-math-random/meta.yaml

Length of output: 65

@mencian mencian merged commit b022dd4 into bioconda:master Nov 5, 2024
6 checks passed
@dslarm dslarm deleted the perl-various-linux-arch64-osx-arm64-part1 branch November 11, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants