-
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 variants to various perl-unicode-* #51909
Add linux-aarch64 and/or osx-arm64 variants to various perl-unicode-* #51909
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces updates 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 (
|
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-unicode-map/meta.yaml (1)
Line range hint
1-44
: Consider standardizing these changes across perl-unicode- packages*The configuration changes (run_exports, additional platforms) represent good practices for conda packages. Consider applying these same patterns consistently across all perl-unicode-* packages to maintain uniformity in the ecosystem.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
recipes/perl-unicode-map/meta.yaml
(2 hunks)recipes/perl-unicode-normalize/meta.yaml
(2 hunks)recipes/perl-unicode-utf8/meta.yaml
(2 hunks)
🔇 Additional comments (9)
recipes/perl-unicode-map/meta.yaml (3)
14-15
: LGTM: Appropriate run_exports configuration
The addition of run_exports with pin_subpackage is the correct approach for Perl modules to ensure ABI compatibility.
16-16
: LGTM: Proper build number increment
Build number increment is appropriate for the package modifications.
41-44
: Platform additions look correct, but license needs attention
The addition of linux-aarch64 and osx-arm64 platforms is appropriate as there are no platform-specific constraints. However, the "unknown" license in the recipe needs attention.
Let's check if the license information is available in the source:
recipes/perl-unicode-normalize/meta.yaml (3)
14-15
: LGTM! Good use of run_exports.
The addition of run_exports with pin_subpackage ensures proper version constraints for downstream dependencies.
16-16
: LGTM! Appropriate build number increment.
Build number increment is correct for adding new platform support.
45-48
: LGTM! Platform additions align with PR objectives.
The addition of linux-aarch64 and osx-arm64 platforms is appropriate. Let's verify there are no platform-specific dependencies that might cause issues.
✅ Verification successful
Platform additions are safe and compatible
The package can safely support linux-aarch64 and osx-arm64 platforms because:
- The build script is platform-agnostic, using standard Perl build processes
- Dependencies are cross-platform:
- Core requirements are Perl-based modules (perl-carp, perl-constant, etc.)
- Build requirements only include standard tools (make and C++ compiler)
- No platform-specific selectors or conditional logic is present in meta.yaml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any platform-specific dependencies or build requirements
# that might affect the new platforms
# Check if there are any platform-specific selectors in requirements
rg '\s*-\s*\{\%.*platform' recipes/perl-unicode-normalize/meta.yaml
# Check if there are any platform-specific build scripts
fd -g 'build.*' recipes/perl-unicode-normalize/
Length of output: 256
Script:
#!/bin/bash
# Let's check the content of build.sh and meta.yaml for any platform-specific logic
cat recipes/perl-unicode-normalize/build.sh
echo "---"
cat recipes/perl-unicode-normalize/meta.yaml
Length of output: 1456
recipes/perl-unicode-utf8/meta.yaml (3)
14-15
: LGTM: Appropriate runtime dependency pinning
The addition of run_exports
with pin_subpackage
ensures proper ABI compatibility across dependent packages.
16-16
: LGTM: Appropriate build number increment
Build number increment is consistent with the package changes.
44-47
: Verify dependency availability for osx-arm64
The comment indicates that osx-arm64 support is blocked by a missing perl-encode package in conda-forge. Let's verify this and consider tracking the dependency issue.
Would you like me to create a new issue in conda-forge/perl-feedstock to track the missing perl-encode package for osx-arm64?
@martin-g |
This pull request adds support for linux-aarch64 and/or osx-arm64 perl-unicode-* where these packages ought to already work. This is a slice of previous PR request that is timing out due to size of package list.
@martin-g - can you review