phpize: fix silently stale build files when the PHP install is read-only - #23570
Open
GromNaN wants to merge 2 commits into
Open
phpize: fix silently stale build files when the PHP install is read-only#23570GromNaN wants to merge 2 commits into
GromNaN wants to merge 2 commits into
Conversation
Many PHP installations, for example Homebrew, Nix and distribution packages, install lib/php/build/* and run-tests.php read-only. Plain cp creates the destination with the source mode on the first phpize run. On later runs in the same project, cp cannot open the read-only destination for writing and fails with "Permission denied". cp -f unlinks and recreates the destination, so it only needs write permission on the directory, not on the target file.
Both cp calls run in subshells with no status check, and phpize_copy_files is called unconditionally, so phpize exits 0 even when the copy fails. The extension is then configured with stale files left over from a previous run. After a PHP upgrade in place, this silently builds against the previous PHP's php.m4, Makefile.global and run-tests.php. Propagate the failure with "|| exit 1", matching the style already used in phpize_autotools().
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
phpize_copy_files()copiesbuild/*andrun-tests.phpfrom the PHP installation into the extension directory using plaincp.Homebrew installs those files read-only, mode 444 and 555. On the first
phpizerun the destination is created with the source's mode, so it is read-only too. On every subsequent run in the same extension directory,cpcannot open the destination for writing and fails with "Permission denied". Any installation that makes these files read-only will hit the same thing.The failure is not detected. Both
cpcalls run in subshells whose exit status is discarded,phpize_copy_filesis called unconditionally, andphpizeexits 0. Sophpizereports success while leaving the previous run's files in place.That is the real hazard. The visible error lines are only a symptom; the consequence is that the extension is then configured and built against stale
php.m4,Makefile.globalandrun-tests.php. After upgrading PHP in place, an extension can be configured with the previous PHP's build system without any signal that something went wrong.Two changes, split so they can be judged separately:
cp -f. It unlinks and recreates the destination, so it needs write permission on the directory only, not on the target file. This alone fixes the failure.-fis POSIX.|| exit 1to both subshells so a copy failure abortsphpizeinstead of continuing with stale files. This matches the style already used byphpize_autotools()just below.Verified on macOS with Homebrew PHP 8.5.10: the substituted script still passes
sh -n, andcpfails whilecp -fsucceeds against a mode 444 destination.