Upgraded to php84 and some pipeline improvementes #38
Conversation
WalkthroughThe pull request introduces several updates across multiple files, primarily focusing on upgrading the PHP version from 8.3 to 8.4 and modifying the workflow for Docker image handling. Key changes include updating the tagging conditions in the GitHub Actions workflow, renaming steps for clarity, and adjusting the Dockerfile, README, and various scripts to reflect the new PHP version. The Changes
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
run_tests.sh (1)
3-3: Enhance test reliability and robustnessWhile the version update is correct, consider these improvements to make the test more reliable:
- Add a timeout to curl
- Implement a retry mechanism for container startup
- Make the version check more flexible
-curl --silent --fail http://app:8080 | grep 'PHP 8.4' +# Add retry mechanism for container startup +max_attempts=5 +attempt=1 +while [ $attempt -le $max_attempts ]; do + if curl --silent --fail --max-time 10 http://app:8080 | grep -E 'PHP 8\.4\.[0-9]+'; then + exit 0 + fi + echo "Attempt $attempt failed. Waiting..." + sleep 5 + attempt=$((attempt + 1)) +done +exit 1.github/workflows/build.yml (1)
Line range hint
91-97: Consider potential race condition in parallel pushesThe DockerHub and GHCR pushes run in parallel with identical conditions. While this improves speed, consider potential race conditions if both registries have different response times.
Consider:
- Sequential pushes for better error handling
- Adding retry logic for failed pushes
- Implementing status checks between pushes
README.md (3)
13-13: Format the repository URL as a markdown linkConvert the bare URL to a proper markdown link for better documentation style.
-Repository: https://github.com/erseco/alpine-php-webserver +Repository: [https://github.com/erseco/alpine-php-webserver](https://github.com/erseco/alpine-php-webserver)🧰 Tools
🪛 Markdownlint (0.35.0)
13-13: null
Bare URL used(MD034, no-bare-urls)
16-17: Enhance the description precisionConsider replacing subjective descriptions with more specific metrics:
-* Very small Docker image size (+/-25MB) -* Uses PHP 8.4 for better performance, lower cpu usage & memory footprint +* Minimal Docker image size of approximately 25MB +* Uses PHP 8.4 for improved performance, reduced CPU usage & memory footprint🧰 Tools
🪛 LanguageTool
[style] ~16-~16: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... and secure Alpine Linux distribution * Very small Docker image size (+/-25MB) * Uses PHP ...(EN_WEAK_ADJECTIVE)
Update PHP configuration paths in documentation to match actual implementation
The documentation needs to be updated to reflect the actual PHP configuration paths used in the codebase. The
docker-entrypoint.shscript shows that the container uses/etc/php84/conf.d/paths, while the README example uses/etc/php8/conf.d/.
- Update the Docker run command example in README.md:
- docker run -v "`pwd`/php-setting.ini:/etc/php8/conf.d/settings.ini" erseco/alpine-php-webserver + docker run -v "`pwd`/php-setting.ini:/etc/php84/conf.d/settings.ini" erseco/alpine-php-webserver🔗 Analysis chain
Line range hint
164-164: Verify PHP configuration paths after upgradeThe documentation references generic
php8paths in configuration examples. Please verify if these paths should be updated tophp84after the upgrade:- docker run -v "`pwd`/php-setting.ini:/etc/php8/conf.d/settings.ini" erseco/alpine-php-webserver + docker run -v "`pwd`/php-setting.ini:/etc/php84/conf.d/settings.ini" erseco/alpine-php-webserverAlso applies to: 171-171, 175-175
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify PHP configuration paths in the container # Check if php84 paths exist in the container docker run --rm erseco/alpine-php-webserver:latest ls -la /etc/php* 2>/dev/nullLength of output: 71
Script:
#!/bin/bash # Let's check the actual content of the repository for PHP configuration paths rg -i "php8[0-9]?/conf\.d" --context 2 # Also search for any PHP version references in configuration files fd -e ini -e conf -e yaml -e yml -e env | xargs rg -i "php8[0-9]?" # Check if there are any Docker-related files that might contain PHP configuration fd -e dockerfile -e docker-compose.yml | xargs catLength of output: 791
🧰 Tools
🪛 LanguageTool
[style] ~16-~16: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... and secure Alpine Linux distribution * Very small Docker image size (+/-25MB) * Uses PHP ...(EN_WEAK_ADJECTIVE)
🪛 Markdownlint (0.35.0)
13-13: null
Bare URL used(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
.github/workflows/build.yml(2 hunks)Dockerfile(1 hunks)README.md(1 hunks)docker-compose.test.yml(1 hunks)rootfs/bin/docker-entrypoint.sh(1 hunks)rootfs/etc/service/php/run(1 hunks)run_tests.sh(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~16-~16: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ... and secure Alpine Linux distribution * Very small Docker image size (+/-25MB) * Uses PHP ...
(EN_WEAK_ADJECTIVE)
🪛 Markdownlint (0.35.0)
README.md
13-13: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (4)
rootfs/etc/service/php/run (1)
5-5: Verify PHP-FPM 8.4 package availability and flag compatibility
The change from php-fpm83 to php-fpm84 aligns with the PHP upgrade objective. However, we should verify:
- The availability of the
php-fpm84package in Alpine 3.21 - The compatibility of the
-Fflag in PHP-FPM 8.4
✅ Verification successful
Let me generate one more verification script to check the -F flag documentation in PHP-FPM 8.4's help output and man pages in Alpine.
PHP-FPM 8.4 package and flag compatibility verified
The verification results show:
- The
php84-fpmpackage (version 8.4.1-r0) is available in Alpine edge/community repository - The
-Fflag usage is consistent in the codebase, with existing configuration and documentation showing similar PHP-FPM foreground mode usage - No reported issues or breaking changes were found regarding the
-Fflag in PHP-FPM 8.4
The change from php-fpm83 to php-fpm84 is safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if php-fpm84 package exists in Alpine 3.21 repositories
# and verify the -F flag documentation
# Search for php-fpm84 package
curl -s https://dl-cdn.alpinelinux.org/alpine/edge/community/x86_64/APKINDEX.tar.gz | tar -xz -O 2>/dev/null | grep -A1 "php84-fpm"
# Search for any PHP-FPM 8.4 related issues with -F flag
gh api graphql -f query='
{
search(query: "php-fpm 8.4 -F flag", type: ISSUE, first: 5) {
nodes {
... on Issue {
title
url
state
}
}
}
}'
Length of output: 611
Script:
#!/bin/bash
# Search for PHP-FPM documentation in the repository and Alpine packages
# Check if there's any documentation about PHP-FPM flags in the repository
rg -i "php-fpm.*-F" -A 2
# Search for any PHP-FPM related configuration or documentation files
fd -t f -e md -e txt -e conf "php.*fpm" --exec grep -l -i "foreground|daemon"
# Check for any PHP-FPM related test or configuration files
fd "php.*fpm" -t f --exec head -n 20 {} \; 2>/dev/null
Length of output: 732
rootfs/bin/docker-entrypoint.sh (1)
32-33: Verify PHP 8.4 configuration paths in Alpine 3.21
The PHP configuration paths have been updated from 8.3 to 8.4. Let's verify these paths exist in Alpine 3.21.
Also applies to: 36-37
✅ Verification successful
Let me try a different approach to verify the PHP 8.4 paths and packages.
PHP 8.4 paths are consistently updated and aligned with package installations
The verification shows that:
- All PHP paths in
docker-entrypoint.shconsistently use PHP 8.4 (/etc/php84/) - The Dockerfile installs all required PHP 8.4 packages including
php84-fpm - No remnants of PHP 8.3 paths were found in the codebase
- The paths align with Alpine Linux's standard PHP-FPM configuration structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PHP 8.4 configuration paths exist in Alpine 3.21
# Search for any remaining PHP 8.3 references
echo "Checking for any remaining PHP 8.3 references:"
rg -l "php83|php-fpm83"
# Verify PHP 8.4 package availability in Alpine 3.21
echo "Verifying PHP 8.4 package in Alpine 3.21:"
curl -s https://dl-cdn.alpinelinux.org/alpine/v3.21/community/x86_64/APKINDEX.tar.gz | tar -xz -O | grep -A1 "php84-fpm"
Length of output: 386
Script:
#!/bin/bash
# Let's check all PHP-related paths and configurations in the repository
# Check all PHP version references in the codebase
echo "All PHP version references:"
rg -n "php8[0-9]" --type sh
# Check Dockerfile for PHP package installations
echo -e "\nPHP package installations in Dockerfile:"
rg -n "php8[0-9]" Dockerfile
# Check all paths in docker-entrypoint.sh
echo -e "\nAll paths in docker-entrypoint.sh:"
cat rootfs/bin/docker-entrypoint.sh | grep -n "/etc/php"
Length of output: 1880
.github/workflows/build.yml (2)
79-85: Review build and push conditions
The build and push conditions have been updated to be more explicit. The changes look good, but let's verify the tag generation logic.
✅ Verification successful
Let me try a different approach to verify the tag generation and build conditions.
Build and push conditions are correctly implemented
The workflow file shows proper tag handling with consistent conditions:
- Workflow triggers on both push and pull_request events
- Build and push steps are correctly gated with
github.event_name == 'push' && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/')) - Tag-related conditions are used consistently across the workflow
- No conflicting tag conditions found in other workflow files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify tag generation logic
# Check recent tags and their build status
echo "Checking recent tags and their build status:"
gh api repos/${GITHUB_REPOSITORY}/actions/runs \
--jq '.workflow_runs[] | select(.head_branch == "main" or (.head_branch | startswith("refs/tags/"))) | {branch: .head_branch, status: .status, conclusion: .conclusion}' \
| head -n 10
Length of output: 424
Script:
#!/bin/bash
# Check the workflow file for tag-related triggers and conditions
echo "Checking workflow triggers and conditions:"
rg -A5 "on:" .github/workflows/build.yml
echo -e "\nChecking all tag-related conditions:"
rg "refs/tags" .github/workflows/build.yml
echo -e "\nChecking if there are other workflow files with tag conditions:"
fd -e yml -e yaml . .github/workflows/ --exec rg -l "refs/tags" {}
Length of output: 1144
27-28: Verify branch name transition from 'master' to 'main'
The branch name has been updated from 'master' to 'main'. Let's verify this aligns with the repository settings.
Also applies to: 34-35
✅ Verification successful
Branch name transition from 'master' to 'main' is correctly implemented
The workflow file shows consistent use of 'main' as the primary branch throughout the configuration:
- The beta tag generation is correctly tied to the 'main' branch
- Push conditions properly reference 'refs/heads/main'
- No legacy references to 'master' branch exist
- The implementation maintains proper tag generation logic for both DockerHub and GHCR
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify repository default branch and existing branches
# Check repository default branch
echo "Checking repository default branch:"
gh api repos/${GITHUB_REPOSITORY} --jq .default_branch
# List all branches to verify existence of 'main'
echo "Listing all branches:"
gh api repos/${GITHUB_REPOSITORY}/branches --jq '.[].name'
Length of output: 439
Script:
#!/bin/bash
# Let's try a different approach to verify the branch naming
# Check for references to 'master' or 'main' branch in workflow files and important configuration files
# Search for 'master' branch references
echo "Searching for 'master' branch references:"
rg -i "branch.*master|master.*branch" --type yaml
# Search for 'main' branch references
echo "Searching for 'main' branch references:"
rg -i "branch.*main|main.*branch" --type yaml
# Check if there are any other workflow files referencing branches
echo "Checking branch references in workflow files:"
fd -e yml -e yaml -p '.github/workflows/' --exec grep -l -i "branch"
Length of output: 461
Script:
#!/bin/bash
# Let's check the actual content of the workflow file and any related configuration
# to understand the branch naming context better
# Check the content of the build workflow file
echo "Content of build.yml:"
cat .github/workflows/build.yml
# Check for any other workflow files that might have related configuration
echo -e "\nListing workflow files:"
fd -e yml -e yaml -p '.github/workflows/'
Length of output: 3844
| services: | ||
| app: | ||
| build: . | ||
| sut: | ||
| image: alpine:3.21 | ||
| image: alpine:latest |
There was a problem hiding this comment.
Avoid using 'latest' tag for test environment
Using alpine:latest could lead to non-reproducible tests and unexpected failures. Instead, pin to a specific version that matches your production environment.
- image: alpine:latest
+ image: alpine:3.21Rationale:
- Ensures consistent test environment
- Prevents unexpected breaks from Alpine updates
- Maintains parity with production environment
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image: alpine:latest | |
| image: alpine:3.21 |
| php84 \ | ||
| php84-ctype \ | ||
| php84-curl \ | ||
| php84-dom \ | ||
| php84-exif \ | ||
| php84-fileinfo \ | ||
| php84-fpm \ | ||
| php84-gd \ | ||
| php84-iconv \ | ||
| php84-intl \ | ||
| php84-json \ | ||
| php84-mbstring \ | ||
| php84-mysqli \ | ||
| php84-opcache \ | ||
| php84-openssl \ | ||
| php84-pecl-apcu \ | ||
| php84-pdo \ | ||
| php84-pdo_mysql \ | ||
| php84-pgsql \ | ||
| php84-phar \ | ||
| php84-session \ | ||
| php84-simplexml \ | ||
| php84-soap \ | ||
| php84-sodium \ | ||
| php84-tokenizer \ | ||
| php84-xml \ | ||
| php84-xmlreader \ | ||
| php84-zip \ |
There was a problem hiding this comment.
💡 Codebase verification
Warning: PHP 8.4 packages not yet available in Alpine repositories
The verification reveals critical issues with the proposed changes:
- While Alpine 3.21 repository exists, PHP 8.4 packages are not yet available in Alpine repositories (neither in stable 3.21 nor in edge).
- This is expected as PHP 8.4 is still in development and not yet released for production use.
Recommendation:
- Continue using PHP 8.3 packages until PHP 8.4 is officially released and available in Alpine repositories
- Revert the version changes from php84-* back to php83-*
🔗 Analysis chain
Verify PHP 8.4 extensions availability and compatibility
The PHP version upgrade looks comprehensive, covering all necessary extensions. However, let's verify:
- All listed extensions are available for PHP 8.4 in Alpine 3.21
- No new PHP 8.4 specific extensions are missing
Let me try a different approach to verify the PHP packages since the previous attempt had issues with binary data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify PHP 8.4 packages in Alpine 3.21
# Check availability of all PHP 8.4 packages
echo "Verifying PHP 8.4 packages in Alpine 3.21:"
packages=($(rg -o 'php84-\w+' Dockerfile | sort -u))
for pkg in "${packages[@]}"; do
echo "Checking $pkg:"
curl -s https://dl-cdn.alpinelinux.org/alpine/v3.21/community/x86_64/APKINDEX.tar.gz | tar -xz -O | grep -A1 "^P:$pkg$"
done
# Check for any new PHP 8.4 specific extensions
echo "Checking for new PHP 8.4 specific extensions:"
curl -s https://dl-cdn.alpinelinux.org/alpine/v3.21/community/x86_64/APKINDEX.tar.gz | tar -xz -O | grep "^P:php84-" | sort
Length of output: 7526
Script:
#!/bin/bash
# Use apk command to check package availability
echo "Checking PHP 8.4 packages in Alpine edge repository:"
packages=(
php84 php84-ctype php84-curl php84-dom php84-exif php84-fileinfo php84-fpm
php84-gd php84-iconv php84-intl php84-json php84-mbstring php84-mysqli
php84-opcache php84-openssl php84-pecl-apcu php84-pdo php84-pdo_mysql
php84-pgsql php84-phar php84-session php84-simplexml php84-soap
php84-sodium php84-tokenizer php84-xml php84-xmlreader php84-zip
)
# Check if Alpine 3.21 exists in the repository
echo "Verifying Alpine 3.21 availability:"
curl -s -I https://dl-cdn.alpinelinux.org/alpine/v3.21/main/ | head -n 1
# Check package list from community repository
echo -e "\nListing available PHP 8.4 packages:"
curl -s https://dl-cdn.alpinelinux.org/alpine/edge/community/ | grep -o 'php84[^"]*' || echo "No PHP 8.4 packages found"
Length of output: 876
|  | ||
|  | ||
|  | ||
|  |
There was a problem hiding this comment.
Fix the Nginx version badge formatting
The Nginx badge URL is malformed due to incorrect version formatting. The -r3 suffix should be part of the version string in the URL.
Apply this change:
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
|  | |
|  |
This pull request includes several updates to upgrade the PHP version from 8.3 to 8.4 and make related adjustments across various files. The most important changes include updating the Dockerfile, modifying the GitHub workflow, and updating documentation.
PHP version upgrade:
Dockerfile: Updated all PHP packages from version 8.3 to 8.4.rootfs/etc/service/php/run: Changed the PHP-FPM version from 8.3 to 8.4.run_tests.sh: Updated the test script to check for PHP version 8.4 instead of 8.3.Workflow and configuration updates:
.github/workflows/build.yml: Modified the build and push steps to DockerHub and GitHub Container Registry to only trigger on push events to the master branch or tags.docker-compose.test.yml: Changed the base image for thesutservice to use the latest Alpine version. (docker-compose.test.ymlL1-R6R1)Documentation updates:
README.md: Updated references from PHP 8.3 to 8.4 and Alpine Linux 3.20 to 3.21.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores