Do not merge: prove the mbregex job fails - #468
Closed
KevinBatdorf wants to merge 3 commits into
Closed
Conversation
The next major renders code server-side and needs mb_ereg_*. mbstring built with --disable-mbregex has no ini switch and no add-on package to fix it — only a different PHP build — and wordpress.org can gate updates on PHP version but not on extensions. Left alone, those sites would auto-update into a version they cannot render. So the plugin holds itself back: a capability check behind the blocks.codeBlockPro.canHighlight filter, and a site_transient_update_plugins filter that drops this plugin's own entry when the check fails. Verified on a playground site — with the check forced false the Plugins screen offers no update, the Updates screen doesn't list the plugin, and the menu bubble reads 0; other plugins are untouched. Note that playground's PHP does have mbregex, so the editor-facing phases have to drive the filter seam rather than the real capability. Reading, not writing, is filtered on purpose. The stored transient still carries the update, so nothing is lost if a later release can run here and the gate comes off. CI runs the suite twice, once with mb_ereg_search_init taken away by disable_functions. No prebuilt PHP offers mbstring without mbregex, and that ini setting reproduces the only part that matters: function_exists returns false while the rest of mbstring keeps working. So one test covers the unfiltered path for real in both directions, rather than trusting the filter seam to stand in for a build nobody can install. This also stands up composer and phpunit, which the repo had neither of — tests run against a WordPress checkout named by WP_ABSPATH, with a dedicated database so they never touch a working install. Requires PHP stays at 7.0: bumping it would stop this release reaching the very sites it exists for. Co-Authored-By: Claude <noreply@anthropic.com>
Hiding mb_ereg_search_init with disable_functions made the capability check false, but it was still a stand-in: mbstring kept the rest of the mb_ereg family, and nothing proved the plugin behaves on the build this release exists for. No prebuilt PHP ships mbstring without mbregex, so the only way to get one is to compile it. The Dockerfile builds PHP with --enable-mbstring --disable-mbregex and without libonig, so a build that quietly keeps mbregex fails instead of passing. Confirmed locally on 8.4.24: mbstring loads, mb_substr works, the whole mb_ereg family is gone, and phpinfo no longer reports an oniguruma version. WordPress installs and the suite passes, and with the transient filter removed the unfiltered test fails — so it is reading the build, not a filter. The image is pushed to GHCR and the second job runs inside it. Public packages cost nothing and don't expire, unlike Actions cache entries, which are evicted after a week idle and would silently recompile. Both jobs live in one workflow so the image is built before the job that pulls it, and the database now comes from the service's own MYSQL_DATABASE rather than a client call, since the image carries no mysql client. The mbregex-absent job talks to the service by hostname because a job running in a container can't reach it on 127.0.0.1. Co-Authored-By: Claude <noreply@anthropic.com>
Checks that mbstring is loaded rather than that mbregex is usable. A normal build is unaffected, so this is meant to go red only in the job that runs on a PHP compiled without mbregex — proving that job catches a wrong check rather than passing alongside everything else. Throwaway commit. Not for merge. Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
Confirmed what it was for: the mbregex-absent job went red while the normal one stayed green. Closing. |
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
1 participant
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.
Throwaway PR. It exists to watch a test go red on purpose, then it gets closed.
The check was changed to look at whether mbstring is loaded instead of whether the regex piece is usable. A normal server is unaffected, so the ordinary test run should stay green — only the run on a PHP built without that piece should fail. If both stay green, the second run isn't earning its place.
PR description generated by Claude.