Rewrite to be based on php-build #200
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Waiting for php-build/php-build#796 to be approved by php-build's maintainer |
Contributor
Author
Installation test of 7.4.14 on Almalinux 10
Installation test of 8.0.0 on Almalinux 10
Installation test of 8.3.29 on Almalinux 10
|
…d/compile - Replace custom download and compilation logic with php-build - Add automatic OpenSSL 1.1 compatibility detection and installation - Add compiler warning fixes for modern GCC versions - Add automated Composer installation with signature verification - Add Xdebug installation support - Add comprehensive patches for PHP 7.4 and 8.0 compatibility - Add post-plugin-add hook for automatic php-build initialization - Add latest-stable version resolution - Add extensive documentation and testing instructions - Add support for all php-build environment variables and options This addresses build failures on modern systems and provides a more maintainable and feature-rich PHP installation experience.
…9 examples - Add comprehensive installation examples for latest stable version - Include verbose installation options and custom compiler flags - Add monitoring instructions with log file tailing - Improve command consistency using 'asdf plugin add php $(pwd)' - Document verification steps for successful installations
Rusydy
force-pushed
the
refactor-with-php-build
branch
from
a1f1048 to
8e37fe6
Compare
- Remove spaces around redirection operators (&>/dev/null, >/dev/null, >>, etc.) - Conform to shfmt standard formatting rules (-s -i 2 -ci) - Resolves CI format check failure
- Fix SC2155: Separate variable declaration and assignment to avoid masking return codes - Fix SC2086: Add proper quoting for variables to prevent word splitting - Fix SC2034: Remove unused php_status variable - Apply fixes to bin/install and bin/exec-env - All shellcheck warnings resolved while maintaining functionality
Rusydy
force-pushed
the
refactor-with-php-build
branch
from
c70d153 to
3b506dd
Compare
Replace manual asdf/PHP setup with asdf-vm/actions/plugin-test and remove ad-hoc asdf install and plugin steps. Add a non-fatal Composer version check and initialize git submodules earlier, setting executable permissions. Use asdf-vm/actions/setup and asdf-vm/actions/install to provide shfmt, shellcheck and make, and simplify lint/format commands.
Use apt-get to install shellcheck and make and download shfmt v3.8.0 Drop asdf-vm actions from workflow
Provide giturl and gitref environment inputs so the PHP setup action can access the current repository and checkout the exact commit.
Add asdf setup action and install the plugin from the current checkout, then install and set the PHP version globally. Increase the test step timeout to 30 minutes.
Rusydy
force-pushed
the
refactor-with-php-build
branch
4 times, most recently
from
9c7a281 to
a2774e5
Compare
Rusydy
force-pushed
the
refactor-with-php-build
branch
from
974b1b9 to
93a9e35
Compare
Add bzip2 to MACOS_DEPS and mark macOS 8.3.29 as legacy Set CXXFLAGS, PKG_CONFIG_PATH and ICU prefix fallback for macOS Export PHP_BUILD_CONFIGURE_OPTS to use bzip2, libiconv and openssl@3 Use portable mktemp /tmp/test_XXXXXX.c instead of --suffix=.c
Contributor
Author
Prerequisite# Clean up any previous failed attempts first
rm -rf /var/tmp/php-build/source/8.3.29Installation test of 8.3.29 on local macOS Tahoe 26.1# Set the same environment variables that worked with php-build directly
export CXXFLAGS="-std=c++17 -stdlib=libc++"
export PKG_CONFIG_PATH="$(brew --prefix icu4c@78)/lib/pkgconfig:$(brew --prefix openssl@3)/lib/pkgconfig:$(brew --prefix libxml2)/lib/pkgconfig:$PKG_CONFIG_PATH"
export PHP_BUILD_CONFIGURE_OPTS="--with-bz2=$(brew --prefix bzip2) --with-iconv=$(brew --prefix libiconv) --with-openssl=$(brew --prefix openssl@3) --with-icu-dir=$(brew --prefix icu4c@78)"
# Now run asdf install
asdf install php 8.3.29
|
Rusydy
force-pushed
the
refactor-with-php-build
branch
from
be1c242 to
2df02a5
Compare
Member
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
bugbot run |
|
Skipping Bugbot: Unable to authenticate your request. Please make sure Bugbot is properly installed and configured for this repository. |
Member
|
bugbot run |
| # Run php-build with all environment preserved | ||
| exec php-build "$@" | ||
| EOF |
There was a problem hiding this comment.
Heredoc prevents variable expansion in wrapper script
The heredoc uses <<'EOF' (quoted), which prevents variable expansion. This means the wrapper script contains literal references like export CC="${CC}" instead of the actual values. When the wrapper executes, these variables may not be set in its environment, defeating the purpose of preserving compiler settings. The heredoc delimiter should be unquoted (<<EOF) to expand $CC, $CXX, and other variables at script creation time.
bin/install
Outdated
| else | ||
| # Linux/GNU sed allows -i without argument | ||
| SED_INPLACE="sed -i" | ||
| fi |
There was a problem hiding this comment.
SED_INPLACE variable with quotes won't work correctly
The SED_INPLACE variable on macOS is set to sed -i '' with embedded quotes, but when expanded via $SED_INPLACE, the shell doesn't preserve these quotes as intended. The '' becomes two separate empty arguments rather than a single empty-string argument to -i. This causes the sed commands in patch_php_build_definitions to fail on macOS. Using an array or eval would be needed for proper quote handling.
There was a problem hiding this comment.
Pull request overview
This pull request modernizes the asdf-php plugin by transitioning from a custom build system to using php-build as the underlying build tool. The changes aim to improve compatibility with modern dependencies (ICU 74, libxml2 2.12) and support legacy PHP versions (7.4, 8.0) that require OpenSSL 1.1.
Key changes:
- Delegates PHP builds to the bundled php-build submodule with automatic detection and installation of OpenSSL 1.1 for legacy PHP versions
- Adds compatibility patches for PHP 7.4 and 8.0 to work with ICU 74 and libxml2 2.12
- Rewrites CI/CD workflow to use matrix builds across Ubuntu/macOS with better dependency management and separate lint/format jobs
Reviewed changes
Copilot reviewed 8 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| patches/php-7.4-icu-74-compat.patch | Adds ICU 74 compatibility for PHP 7.4 by updating operator return type |
| patches/php-8.0-icu-74-compat.patch | Adds ICU 74 compatibility for PHP 8.0 by updating operator return type |
| patches/php-8.0-libxml2-2.12-compat.patch | Adds libxml2 2.12 compatibility for PHP 8.0 with proper type casting |
| lib/install-openssl11.sh | Provides automated OpenSSL 1.1 installation for legacy PHP versions |
| docs/local-testing-manually.md | Adds step-by-step guide for local PHP version testing |
| bin/post-plugin-add | Initializes php-build submodule on plugin installation |
| bin/latest-stable | Resolves "latest" version queries from PHP repository tags |
| bin/install | Complete rewrite to use php-build with compatibility handling for old PHP versions |
| bin/exec-env | Automatically configures LD_LIBRARY_PATH for legacy PHP versions needing OpenSSL 1.1 |
| README.md | Expands installation and usage documentation |
| .gitmodules | Adds php-build as a submodule |
| .gitignore | Ignores vendor directory and temporary files |
| .github/workflows/workflow.yml | Rewrites CI with matrix builds for cross-platform testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Quote OSTYPE in the comparison and store sed command as an array. This
allows passing an empty -i argument on macOS (BSD sed) and running sed
consistently via "${SED_INPLACE[@]}" for both macOS and GNU sed.
Support an OPENSSL_SHA256 environment variable and attempt to retrieve the official checksum from OpenSSL/GitHub when unset. Verify downloads using sha256sum or shasum, exit on mismatch, and warn if no verification tool or checksum is available. Add a --help flag with usage examples and improve flag handling.
Set ICU_PREFIX (prefer icu4c@78) and export PKG_CONFIG_PATH. Detect tidy-html5 once and export TIDY_CONFIG as --with-tidy or --without-tidy, then reuse it in PHP_BUILD_CONFIGURE_OPTS. Remove debug tidy install logging and duplicate configure blocks.
Contributor
Author
|
I am currently focused on PHP 8.0.0 installation on macOS |
- Replace all sed 'a' (append) commands with portable awk equivalents - Fixes macOS BSD sed error: 'command a expects \ followed by text' - Remove unused SED_INPLACE variable - Maintain identical patch insertion functionality - Cross-platform compatible (macOS, Linux, BSD)
- Add OS detection for platform-specific build flags - Configure BZip2 and libiconv paths for macOS using Homebrew - Remove incompatible Linux linker flags (--no-as-needed) on macOS - Set proper LDFLAGS and CPPFLAGS for Homebrew library paths - Fixes BZip2 'Please reinstall the BZip2 distribution' error on macOS
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TLDR; to solve #198, #72 and #194
This pull request introduces significant improvements to the PHP plugin's development workflow, documentation, and compatibility with modern dependencies. The most notable changes include a complete overhaul of the GitHub Actions CI workflow for better cross-platform and legacy PHP support, new documentation for manual local testing, compatibility patches for ICU 74 and libxml2 2.12, and a script for installing OpenSSL 1.1. Additionally, a new submodule for
php-buildis added to the repository.CI/CD and Development Workflow Improvements
.github/workflows/workflow.ymlto use a single, matrix-based job supporting both Ubuntu and macOS, legacy and modern PHP versions, with improved dependency management and diagnostics. Adds linting and formatting jobs with more robust dependency installation and caching.vendor/php-buildas a submodule, ensuring consistent build tooling. (.gitmodules)Documentation Enhancements
README.mdwith clearer installation, update, and usage instructions for the asdf PHP plugin.docs/local-testing-manually.mdwith detailed, step-by-step guides for installing, uninstalling, and troubleshooting various PHP versions locally, including custom build flags and log monitoring.Compatibility and Build Fixes
patches/php-7.4-icu-74-compat.patch).patches/php-8.0-icu-74-compat.patch).patches/php-8.0-libxml2-2.12-compat.patch).Utility Scripts
lib/install-openssl11.sh, a robust script for building and installing OpenSSL 1.1 locally, with dependency checks, colored output, and instructions for environment configuration.Note
Modernizes the plugin around
php-build, expanding CI/test coverage and improving legacy compatibility.vendor/php-buildsubmodule andbin/post-plugin-addto auto-initialize it; newbin/latest-stablefor resolvinglatestbin/installto drivephp-build, auto-detect/auto-install OpenSSL 1.1 (lib/install-openssl11.sh), set compilers/flags for legacy PHP, install Composer, optional Xdebug, and patch definitions for ICU 74 and libxml2 2.12bin/exec-envto setLD_LIBRARY_PATHfor OpenSSL 1.1 when requiredlintandformatjobspatches/php-7.4-icu-74-compat.patch,patches/php-8.0-icu-74-compat.patch,patches/php-8.0-libxml2-2.12-compat.patchREADME.mdand adddocs/local-testing-manually.md; add.gitmodulesand expand.gitignoreWritten by Cursor Bugbot for commit 3492506. This will update automatically on new commits. Configure here.