feat(linux): close three parity gaps against the Windows implementation - #32
Merged
Conversation
An audit of both implementations found the command surface all but identical - `cacert` (Windows) and `deps` (Linux) are the only differences, and both are by design. Three gaps were not by design. All three live in the same file, so they land together. 1. The PHP source tarball was never verified. It was downloaded from php.net and handed straight to the build, then cached - so a corrupt or substituted archive would be trusted on every later install too, since a cached tarball is never re-downloaded. Composer (SHA-384) and wp-cli (SHA-512) were already verified here; only PHP itself was not. The digest now comes from php.net's per-version release JSON and is checked after both the download and the cache path. A mismatch aborts and deletes the file. A missing digest, or a host with no sha256sum/shasum/openssl, degrades to a warning rather than blocking an otherwise valid install - the same fallback Windows takes. PHPVM_SKIP_HASH opts out. 2. `ext list` and `ext loaded` both ran `php -m`, so there was no way to see an extension that is available but not enabled, and one of the two verbs was dead weight. `ext list` now reports ON/OFF over the union of loaded extensions and the .so files in extension_dir; `ext loaded` is `php -m` alone. ON has to come from `php -m` rather than a directory listing, because extensions compiled into the binary own no .so. 3. `doctor` was shallower than its Windows counterpart in three ways. It only looked at the first `php` on PATH, so a distro or Homebrew PHP waiting behind phpvm went unreported - the very situation the check exists for. It checked that extension_dir merely existed, which passes an ini left pointing at another installed version, the exact case fix-ini repairs. And it read through PATH, which check 2 may have just said resolves elsewhere; it now goes through the active version's own binary. It also reports the host OpenSSL version upfront, so the OpenSSL 3 vs PHP < 8.1 limitation surfaces before a download rather than after one. bats 82 -> 112, zsh smoke 20 -> 40 checks. devhardiyanto
devhardiyanto
macOS ships bash 3.2, whose parser chokes on a `case` nested inside a command substitution - the scan could not even be sourced there, so phpvm.sh was broken outright on macOS, not merely degraded. Split PATH with parameter expansion instead. That drops the `$( ... case ... )` construct, and with it the dependency on tr and awk: the check that tells you PATH is broken should not need to find coreutils on that same PATH. It also keeps working under zsh, where `for d in $PATH` does not split on colons. The PATH tests can now hand doctor a PATH holding nothing but phpvm, so they stop depending on whether the runner ships a php of its own - which is what made them pass locally and fail on ubuntu. devhardiyanto
Those tests hand doctor a PATH holding nothing but phpvm, and bats runs its own cleanup with whatever PATH the test left behind - so `rm` went missing and the job exited 1 with all 112 assertions green. devhardiyanto
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.
What
Closes the three real behaviour gaps between
linux/phpvm.shand the Windows implementation: unverified PHP downloads,ext listsemantics, and a shallowerdoctor. Linux/macOS only — Windows is untouched apart from the version bump.Why
An audit of both implementations found the command surface all but identical.
cacert(Windows) anddeps(Linux) are the only command-level differences and both are by design. Three differences were not by design, and one of them is a genuine integrity hole.Splitting
phpvm.shinto modules the waywindows/src/is split is queued next — deliberately after this, because a split closes no gap and is safer on top of the behaviour work plus its tests.How
1. SHA-256 verification of the PHP tarball
The tarball came from php.net and went straight into the build with no check at all, then got cached — so a corrupt or substituted archive would be trusted on every later install too, because a cached tarball is never re-downloaded. Composer (SHA-384) and wp-cli (SHA-512) were already verified in this same file; only PHP itself was not.
The digest now comes from php.net's per-version release JSON and is checked after both the download and the cache path. Mismatch aborts and deletes the file. A missing digest, or a host with no
sha256sum/shasum/openssl, degrades to a warning instead of blocking an otherwise valid install — the same fallback Windows takes.PHPVM_SKIP_HASHopts out.Digest lookup can't reuse the
hash_file()trick Composer/wp-cli use: this runs before any PHP exists.2.
ext listgains ON/OFF,ext loadedbecomes distinctBoth verbs ran
php -m, so one was dead weight and there was no way to see an extension that is available but not enabled.ext listnow reports ON/OFF across the union of loaded extensions and the.sofiles inextension_dir;ext loadedisphp -malone.ON has to come from
php -mrather than a directory listing — extensions compiled into the binary (pdo,mbstring, …) own no.so, and reading the directory alone would drop them.3. Deeper
doctorphpwas inspected, so a distro or Homebrew PHP sitting behind phpvm went unreported — the exact situation the check exists for. It now walks PATH and names the second one. (Split viatr, becausefor d in $PATHdoes not split on colons in zsh.)fix-inirepairs. Now compared againstPHP_EXTENSION_DIR, mirroring Windows'Test-ExtDirMatch.Changes
linux/phpvm.sh—_phpvm_php_sha256,_phpvm_sha256_file,_phpvm_verify_tarball; verification wired intophpvm_installafter both download and cache paths;phpvm_ext_listrewritten,phpvm_ext_loadedadded,extdispatch de-duplicated;phpvm_doctorchecks 2, 3 and a new 6; help text in bothphpvm_helpandphpvm_ext_helptests/linux/verify.bats— new, 12 teststests/linux/commands.bats— +18 tests; fake php stub gainedFAKE_PHP_INI_EXT_DIRso ini and compiled-in dirs can be driven aparttests/linux/zsh-smoke.zsh— 20 → 40 checksREADME.md— download verification,ext listsemantics, doctor description,PHPVM_SKIP_HASHno longer described as Windows-onlywindows/phpvm.ps1)Testing Done
linux/install.sh+linux/phpvm.shbash -nandzsh -nclean.tar.gzdigest, never its.bz2/.xzsiblingsTwo zsh-specific risks are covered by name in the smoke suite, since bats only runs under bash: the PATH scan (colon splitting) and the
ext listON/OFF counters (awhile readfed by a here-string, which would have come back as zeroes had it been a pipeline).