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

perl-io-compress linux aarch64 support #52152

Merged
merged 9 commits into from
Nov 19, 2024

Conversation

dslarm
Copy link
Contributor

@dslarm dslarm commented Nov 17, 2024

This adds support for linux-aarch64 for perl-io-compress.
Dependencies in conda required by perl-io-compress are now available.
This also tests whether osx-arm64 will also build.
osx-arm64 fails due to missing perl-compress-raw-bzip2 and perl-scalar-list-utils

Copy link
Contributor

coderabbitai bot commented Nov 17, 2024

📝 Walkthrough

Walkthrough

The pull request modifies the meta.yaml file for the perl-io-compress package. Key changes include the replacement of the static package name with a Jinja2 template variable, name, and an increment of the build number from 2 to 3. A new section titled run_exports is introduced under the build section, which specifies a dependency on a subpackage with a maximum pinning of "x". Additionally, an extra section is added to include support for the linux-aarch64 platform, along with a commented-out entry for osx-arm64. The existing requirements and test imports remain unchanged.

Possibly related PRs

Suggested labels

please review & merge, aarch64, osx-arm64

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: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f6eeb47 and 80b6353.

📒 Files selected for processing (3)
  • recipes/blast/meta.yaml (2 hunks)
  • recipes/perl-io-compress/meta.yaml (2 hunks)
  • recipes/r-facets/meta.yaml (2 hunks)
🧰 Additional context used
🪛 yamllint
recipes/perl-io-compress/meta.yaml

[error] 13-13: trailing spaces

(trailing-spaces)

🔇 Additional comments (6)
recipes/r-facets/meta.yaml (2)

10-10: LGTM: Build number increment is appropriate

The build number increment is necessary for the platform support changes.


42-42: Verify R package dependencies for osx-arm64 support

Before adding osx-arm64 support, we should verify that both r-base and r-pctgcdata are available for this platform.

recipes/perl-io-compress/meta.yaml (3)

10-10: LGTM: Build number increment is appropriate

The build number increment is correct as we're adding new platform support without changing the package version.


60-63: LGTM: Platform additions align with PR objectives

The addition of linux-aarch64 and osx-arm64 platforms is consistent with the PR's goal of adding ARM architecture support.

Let's verify if these platforms have the required Perl dependencies:


11-13: ⚠️ Potential issue

Critical: Unexpected bedtools dependency in run_exports

The addition of bedtools as a run_exports dependency appears incorrect:

  1. perl-io-compress is a Perl compression library that shouldn't require bedtools
  2. bedtools is not listed in the regular requirements section
  3. This addition wasn't mentioned in the PR objectives

This could unnecessarily force all downstream packages to depend on bedtools.

Let's verify if bedtools is actually used in the package:

Please remove the run_exports section as it appears to be incorrectly added.

Also, there's a trailing whitespace on line 13 that should be removed.

🧰 Tools
🪛 yamllint

[error] 13-13: trailing spaces

(trailing-spaces)

recipes/blast/meta.yaml (1)

19-19: LGTM: Build number increment is appropriate

The build number increment is correct for adding new platform support while maintaining the same package version.

Comment on lines 99 to 101
additional-platforms:
- linux-aarch64
- osx-arm64
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

Dependency availability concerns for ARM platforms

Based on the package availability check results:

  • Core system libraries (zlib, bzip2, curl, sqlite, perl) are available for ARM platforms
  • Several critical dependencies are missing ARM support in conda-forge:
    • entrez-direct
    • ncbi-vdb
    • perl-list-moreutils
    • perl-archive-tar
    • perl-json

While the patch file properly handles ARM architecture detection, the missing dependencies would prevent successful builds on ARM platforms.

  • The package should not be enabled for ARM platforms until these dependencies are available
  • Consider filing issues with the respective package maintainers to request ARM support
🔗 Analysis chain

Verify dependency availability on ARM platforms

While adding ARM platform support is valuable, we should verify that all dependencies are available and properly tested on these architectures.

Let's check the dependency availability:

Additionally, consider:

  1. Adding ARM-specific test cases in the test section
  2. Documenting any known performance characteristics on ARM platforms
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if all dependencies are available for ARM platforms

# Check conda-forge and bioconda channels for key dependencies on ARM
echo "Checking key dependencies availability on ARM platforms..."

for pkg in "zlib" "bzip2" "entrez-direct" "ncbi-vdb" "curl" "sqlite" "perl" "perl-list-moreutils" "perl-archive-tar" "perl-json" "rpsbproc"; do
    echo "Checking $pkg..."
    # Query conda-forge
    curl -s "https://conda.anaconda.org/conda-forge/linux-aarch64/$pkg/repodata.json" | jq -r '.packages | keys[]'
    # Query bioconda
    curl -s "https://conda.anaconda.org/bioconda/linux-aarch64/$pkg/repodata.json" | jq -r '.packages | keys[]'
done

# Check if there are any ARM-specific patches or issues in the repo
echo "Checking for ARM-related issues or patches..."
rg -i "aarch64|arm64" recipes/blast/

Length of output: 6692


Script:

#!/bin/bash
# Let's try a different approach to check package availability
# We'll use conda-search which is more reliable for checking package existence

echo "Checking dependencies in conda-forge channel for linux-aarch64..."
for pkg in "zlib" "bzip2" "entrez-direct" "ncbi-vdb" "curl" "sqlite" "perl" "perl-list-moreutils" "perl-archive-tar" "perl-json" "rpsbproc"; do
    echo "=== $pkg ==="
    conda search -c conda-forge --platform linux-aarch64 $pkg
done

# Check the patch file content to understand ARM support
echo -e "\nChecking patch file content..."
cat recipes/blast/update_configsub.patch

# Check build requirements
echo -e "\nChecking build requirements..."
cat recipes/blast/meta.yaml | grep -A 20 "requirements:"

Length of output: 71021


Script:

#!/bin/bash
# Let's check if these packages are available in conda-forge's aarch64 channel using anaconda API
echo "Checking package availability in conda-forge channel for aarch64..."

for pkg in "zlib" "bzip2" "entrez-direct" "ncbi-vdb" "curl" "sqlite" "perl" "perl-list-moreutils" "perl-archive-tar" "perl-json"; do
    echo "=== $pkg ==="
    curl -s "https://api.anaconda.org/package/conda-forge/$pkg" | jq -r '.files[] | select(.basename | contains("aarch64")) | .basename'
done

# Check if the patch file properly handles ARM architectures
echo -e "\nChecking ARM support in patch file..."
cat recipes/blast/update_configsub.patch | grep -A 5 -B 5 "aarch64\|arm64"

# Check if there are any ARM-specific build flags or configurations
echo -e "\nChecking for ARM-specific build configurations..."
cat recipes/blast/build.sh

Length of output: 21107

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 (1)
recipes/perl-io-compress/meta.yaml (1)

13-13: Remove trailing whitespace

There is unnecessary trailing whitespace on this line.

-    
+
🧰 Tools
🪛 yamllint

[error] 13-13: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 80b6353 and cdbf202.

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

[error] 13-13: trailing spaces

(trailing-spaces)

🔇 Additional comments (2)
recipes/perl-io-compress/meta.yaml (2)

10-12: LGTM: Build configuration changes look good

The build number increment and run_exports addition are appropriate. The pin_subpackage directive will help maintain ABI compatibility across dependent packages.


60-63: LGTM: Platform support addition aligns with PR objectives

The addition of linux-aarch64 and osx-arm64 platforms is correctly configured. Since the package has C/C++ compilation requirements but no platform-specific dependencies, this should work as expected.

Let's verify if similar packages in the ecosystem support these platforms:

✅ Verification successful

Platform support addition is appropriate and consistent with ecosystem

The verification shows that:

  1. perl-io-compress has appropriate build requirements including C/C++ compilers
  2. Several related Perl packages in the ecosystem (perl-net-http, perl-http-message, perl-archive-zip) use compression-related dependencies and work across platforms
  3. No platform-specific dependencies or issues were found that would prevent the package from working on aarch64/arm64
  4. The package's core dependencies (perl-compress-raw-zlib, perl-compress-raw-bzip2) are platform-agnostic

The addition of linux-aarch64 and osx-arm64 platforms is well-supported by the package's architecture and its dependency chain.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if related perl compression packages support these architectures
fd -e yaml$ -p "perl-(compress|zlib)" | xargs grep -l "linux-aarch64\|osx-arm64"

# Check for any potential platform-specific build issues in the package history
gh issue list --repo bioconda/bioconda-recipes --search "perl-io-compress aarch64"

Length of output: 169


Script:

#!/bin/bash
# Let's try a different approach to check platform support in related packages
# First, let's find all perl compression related recipes
fd "meta.yaml$" recipes/perl-compress recipes/perl-io recipes/perl-zlib

# Then let's examine their content for platform configurations
for f in $(fd "meta.yaml$" recipes/perl-compress recipes/perl-io recipes/perl-zlib); do
  echo "=== $f ==="
  grep -A 5 "^extra:" "$f" 2>/dev/null
done

# Let's also check build requirements to verify if there are any platform-specific dependencies
for f in $(fd "meta.yaml$" recipes/perl-compress recipes/perl-io recipes/perl-zlib); do
  echo "=== $f ==="
  grep -A 10 "requirements:" "$f" 2>/dev/null
done

Length of output: 928


Script:

#!/bin/bash
# Let's try a broader search approach to find related Perl packages
# First find all perl-related recipes with meta.yaml
fd "meta.yaml$" recipes/perl

# Then examine platform configurations and requirements in found files
for f in $(fd "meta.yaml$" recipes/perl); do
  if grep -q "compress\|zlib" "$f" 2>/dev/null; then
    echo "=== $f ==="
    echo "--- Platform configurations ---"
    grep -A 5 "^extra:" "$f" 2>/dev/null
    echo "--- Requirements ---"
    grep -A 10 "requirements:" "$f" 2>/dev/null
  fi
done

Length of output: 262


Script:

#!/bin/bash
# Let's try a different approach using ripgrep to search for related packages
# First, let's find all meta.yaml files that mention perl compression packages
rg -l "perl-(compress|zlib|io-compress)" -g "meta.yaml"

# Then let's check their content
for f in $(rg -l "perl-(compress|zlib|io-compress)" -g "meta.yaml"); do
  echo "=== $f ==="
  echo "--- Package Info ---"
  grep -A 2 "^package:" "$f" 2>/dev/null
  echo "--- Platform configurations ---"
  grep -A 5 "^extra:" "$f" 2>/dev/null
  echo "--- Build Requirements ---"
  grep -A 10 "requirements:" "$f" 2>/dev/null
done

Length of output: 8318

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 (1)
recipes/perl-io-compress/meta.yaml (1)

14-14: Remove trailing whitespace

Line contains trailing whitespace that should be removed.

-    
+
🧰 Tools
🪛 yamllint

[error] 14-14: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between cdbf202 and c2923d8.

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

[error] 14-14: trailing spaces

(trailing-spaces)


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

(syntax)

🔇 Additional comments (3)
recipes/perl-io-compress/meta.yaml (3)

1-1: LGTM: Good use of Jinja2 templating

Using a variable for the package name improves maintainability and follows conda-forge best practices.

Also applies to: 3-3

🧰 Tools
🪛 yamllint

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

(syntax)


11-13: LGTM: Appropriate build configuration changes

  • Build number increment is correct for ABI changes
  • The run_exports configuration ensures proper ABI compatibility across dependent packages

61-64: Verify dependency compatibility for ARM platforms

While adding ARM platform support, we should verify that all dependencies (perl-compress-raw-zlib, perl-compress-raw-bzip2, etc.) are available for these architectures.

✅ Verification successful

Let me try a different approach to verify the dependencies by checking the recipe files directly in the repository.


Let me try one final approach to check the dependencies directly in the recipe file.


ARM platform support looks feasible based on dependencies

The recipe's dependencies appear compatible with ARM platforms because:

  • Core build requirements use generic compiler selectors ({{ compiler('c') }}, {{ compiler('cxx') }}) which adapt to the target platform
  • Host dependencies are Perl modules that are typically architecture-independent or have broad platform support
  • No platform-specific skip conditions are present in the recipe
  • The build requirements are standard tools available on ARM platforms
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if dependencies are available for ARM platforms

# Check availability of key dependencies for ARM platforms
for pkg in perl-compress-raw-zlib perl-compress-raw-bzip2 perl-scalar-list-utils perl-encode; do
    echo "Checking $pkg..."
    # Search for conda package availability
    conda search --json "$pkg" | jq '.[] | select(.platform == "linux-aarch64" or .platform == "osx-arm64") | {name: .name, version: .version, platform: .platform}'
done

Length of output: 1673


Script:

#!/bin/bash
# Check if dependency recipes exist and if they support ARM platforms
for pkg in perl-compress-raw-zlib perl-compress-raw-bzip2 perl-scalar-list-utils perl-encode; do
    echo "=== Checking $pkg ==="
    # Find recipe file
    fd "meta.yaml" "recipes/$pkg"
    # If found, check for platform support
    if fd -q "meta.yaml" "recipes/$pkg"; then
        echo "Platform support:"
        rg -A3 "additional-platforms:|skip:" "recipes/$pkg/meta.yaml" || echo "No explicit platform configuration found"
    else
        echo "Recipe not found in repository"
    fi
    echo
done

Length of output: 2315


Script:

#!/bin/bash
# Check the requirements in the perl-io-compress recipe
echo "=== Dependencies listed in perl-io-compress recipe ==="
rg -A10 "requirements:" "recipes/perl-io-compress/meta.yaml"

# Check if there are any platform-specific skip conditions
echo -e "\n=== Platform-specific conditions ==="
rg "skip:|platform:" "recipes/perl-io-compress/meta.yaml"

Length of output: 565

@dslarm dslarm changed the title Blast linux aarch64 and possibly osx-arm64 support perl-io-compress linux aarch64 and possibly osx-arm64 support Nov 17, 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: 0

🧹 Outside diff range and nitpick comments (1)
recipes/perl-io-compress/meta.yaml (1)

14-14: Remove trailing whitespace

There is trailing whitespace on this line that should be removed.

-    
+
🧰 Tools
🪛 yamllint

[error] 14-14: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c2923d8 and 3c60e20.

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

[error] 14-14: trailing spaces

(trailing-spaces)


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

(syntax)

🔇 Additional comments (3)
recipes/perl-io-compress/meta.yaml (3)

1-1: LGTM: Good use of Jinja2 templating

Using a template variable for the package name improves maintainability and follows conda-forge best practices.

Also applies to: 3-3

🧰 Tools
🪛 yamllint

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

(syntax)


11-13: LGTM: Appropriate build configuration

The build number increment and run_exports configuration are appropriate for the platform addition. The pinning strategy will help maintain compatibility while allowing minor version updates.


61-65: Verify dependency availability for linux-aarch64

While adding linux-aarch64 support, we should verify that all required dependencies are available for this platform.

@dslarm
Copy link
Contributor Author

dslarm commented Nov 18, 2024

@martin-g

@dslarm dslarm changed the title perl-io-compress linux aarch64 and possibly osx-arm64 support perl-io-compress linux aarch64 support Nov 19, 2024
@martin-g martin-g merged commit c7b594b into bioconda:master Nov 19, 2024
6 checks passed
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.

3 participants