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

Trying to fix imagick install #13

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Trying to fix imagick install #13

merged 1 commit into from
Nov 27, 2024

Conversation

SilverFire
Copy link
Member

@SilverFire SilverFire commented Nov 27, 2024

Summary by CodeRabbit

  • New Features

    • Enhanced installation process for PHP extensions, including the imagick extension, using a simplified script.
    • Improved configuration options for New Relic and XDebug, including distributed tracing support and updated variable names.
  • Bug Fixes

    • Addressed configuration logic for development settings based on the environment.
  • Chores

    • Streamlined Dockerfile and entrypoint script for better maintainability and clarity.

Copy link

coderabbitai bot commented Nov 27, 2024

Walkthrough

The pull request modifies the Dockerfile for the PHP 8.4 FPM Alpine image to streamline the installation of PHP extensions, particularly the imagick extension, by utilizing a new installation script. Additionally, the hiqdev-docker-entrypoint script is updated to enhance New Relic and XDebug configurations, including enabling distributed tracing and renaming a configuration variable. These changes aim to improve the setup and functionality of the Docker image while maintaining existing configurations.

Changes

File Change Summary
src/8.4/fpm-alpine/Dockerfile - Added command to copy install-php-extensions script from mlocati/php-extension-installer.
- Updated imagick installation to use the new script.
src/8.4/fpm-alpine/hiqdev-docker-entrypoint - Added New Relic distributed tracing configuration.
- Renamed xdebug.remote_autostart to xdebug.start_with_request.
- Added checks for environment variables.

Possibly related PRs

  • Working on PHP 8.4 build #12: The changes in the Makefile for PHP 8.4 build process are related as they define the build targets that include the fpm-alpine Dockerfile, which is the focus of the main PR.

Poem

🐰 In the Docker world, we hop and play,
New scripts and settings brighten the day.
With imagick now easy, and tracing in sight,
Our PHP's ready, everything feels right!
So here's to the changes, let’s cheer and rejoice,
For every new feature, we’ll raise up our voice! 🎉


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

@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

🧹 Outside diff range and nitpick comments (1)
src/8.4/fpm-alpine/Dockerfile (1)

26-26: Good choice using php-extension-installer!

Using mlocati/php-extension-installer is a more reliable approach for installing PHP extensions. It handles dependencies automatically and reduces potential installation issues.

Consider using multi-stage builds to minimize the final image size if image size is a concern.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 015bfc6 and aef3cea.

📒 Files selected for processing (2)
  • src/8.4/fpm-alpine/Dockerfile (2 hunks)
  • src/8.4/fpm-alpine/hiqdev-docker-entrypoint (1 hunks)
🔇 Additional comments (2)
src/8.4/fpm-alpine/hiqdev-docker-entrypoint (1)

62-63: XDebug configuration update looks good

The change from remote_autostart to start_with_request is correct as this aligns with XDebug 3's configuration requirements. The start_with_request setting was introduced to replace the legacy remote_autostart option.

src/8.4/fpm-alpine/Dockerfile (1)

Line range hint 3-7: Ensure consistent version pinning strategy

While some extensions have pinned versions (XDebug, Memcached), others don't. Consider:

  1. Using pinned versions for all extensions to ensure reproducible builds
  2. Adding a comment or issue reference for tracking the New Relic agent version update

Let's check if there are any updates for the pinned versions:

✅ Verification successful

The pinned versions are up-to-date, but version pinning strategy needs attention

The verification shows that all pinned versions in the Dockerfile are using the latest available releases:

  • XDebug: 3.4.0beta1 (latest beta)
  • Memcached: 3.3.0 (latest stable)
  • SSH2: 1.4.1 (latest stable)

Let's verify if there's any tracking for the New Relic update:


Version management is well-maintained with proper tracking

The pinned versions are using the latest available releases, and the New Relic installation is properly tracked with a reference to issue #909 in their repository. The current setup demonstrates good practices:

  • XDebug, Memcached, and SSH2 are on their latest versions
  • New Relic integration is commented out with a clear link to the blocking issue
  • GNU libiconv version is pinned with a specific release

No immediate action is required, though consider documenting the version pinning strategy for future maintenance.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check latest versions of PHP extensions

# Check latest versions
echo "Checking XDebug versions..."
curl -s https://pecl.php.net/rest/r/xdebug/allreleases.xml | grep -oP '<v>.*?</v>' | head -n5

echo "Checking Memcached versions..."
curl -s https://pecl.php.net/rest/r/memcached/allreleases.xml | grep -oP '<v>.*?</v>' | head -n5

echo "Checking SSH2 versions..."
curl -s https://pecl.php.net/rest/r/ssh2/allreleases.xml | grep -oP '<v>.*?</v>' | head -n5

Length of output: 694


Script:

#!/bin/bash
# Check for New Relic related comments or issues
rg -i "new.?relic" --type md
rg -i "new.?relic.*todo" 
rg -i "new.?relic.*issue"

Length of output: 225

@@ -74,8 +76,7 @@ RUN set -xe \
docker-php-ext-install zip bcmath soap pcntl gmp && \
docker-php-ext-enable mailparse imap && \

pecl install imagick && \
docker-php-ext-enable imagick && \
install-php-extensions Imagick/imagick@65e27f2bc0 && \
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider updating to the latest stable version 3.7.0

The commit hash 65e27f2bc0 appears invalid or no longer exists, but there is a stable release version 3.7.0 available. I recommend updating the installation line to use this stable version tag instead:

install-php-extensions Imagick/imagick@3.7.0 && \

This change will:

  • Ensure reproducible builds with a well-tested release
  • Improve maintainability with clear version tracking
  • Prevent potential issues with commit hash references
🔗 Analysis chain

Consider using a stable version tag instead of commit hash

While pinning to a specific commit (65e27f2bc0) ensures reproducible builds, using a stable version tag would be more maintainable and less prone to issues if the repository history is rewritten.

Let's verify if there's a stable release tag available:


Let me try a different approach to verify the commit and available tags.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if there's a stable release tag for the imagick extension
# that corresponds to or is newer than the specified commit

# Clone the repository and check tags
git clone https://github.com/Imagick/imagick.git /tmp/imagick
cd /tmp/imagick
git tag --contains 65e27f2bc0 | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$' || echo "No stable version found containing this commit"

Length of output: 386


Script:

#!/bin/bash
# Let's verify if this is the correct repository and check available tags
gh repo view Imagick/imagick --json description,url

# Check recent tags
gh api repos/Imagick/imagick/tags --paginate | jq -r '.[].name' | head -n 5

Length of output: 264

@SilverFire SilverFire merged commit 58b9bc9 into master Nov 27, 2024
2 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.

1 participant