Repository navigation
Keep the shared opcode cache from changing what the code does - #417
Conversation
The optimizer evaluates calls like function_exists() at compile time, so code cached by a process with the default settings skipped the ini_set() fallback of WP-CLI in a process that disables it. Cache the code without optimizing it. Behat and Composer restart without Xdebug, which clears PHP_INI_SCAN_DIR, so the WP-CLI processes lost the scan directory of the system, and the extensions configured there. Take the original directories from the restart settings instead. Also leave out SQLITE_ENABLE_UPDATE_DELETE_LIMIT, which needs a parser the amalgamation does not have. Fixes #416. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 51 minutes. View limit details
 |
| Layer / File(s) | Summary |
|---|---|
PHP settings and regression scenario src/Context/FeatureContext.php, features/testing.feature |
Generated PHP settings now set opcache.optimization_level=0 and restore the scan-directory setting from XDEBUG_HANDLER_SETTINGS when applicable. A Windows-skipped scenario checks cli info with ini_set disabled. |
SQLite build configuration
| Layer / File(s) | Summary |
|---|---|
SQLite compile flags utils/docker/Dockerfile |
The build no longer defines SQLITE_ENABLE_UPDATE_DELETE_LIMIT when compiling the older system SQLite version. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- wp-cli/wp-cli-tests#413: Adds the conditional SQLite source build and its compile flags, which this change also updates.
- wp-cli/wp-cli-tests#415: Adds SQLite compile-time features to the Docker build, including the flag removed here.
Suggested labels: bug, scope:testing, scope:distribution, scope:artifacts
Suggested reviewers: swissspidy, mrsdizzie
Merge Risk: 
7d121
-
The fix for the ini_set failure can be defeated on machines that already hold optimized cache entries from 5.4.0, because the cache directory is never versioned or cleared. Restoring the scan directory without PHPRC can also produce duplicate-extension warnings that fail test steps in some environments. Version the cache and config directories and restore PHPRC before merging.
Pre-merge checks: 
-
Check name Status Explanation Resolution Out of Scope Changes check The SQLITE_ENABLE_UPDATE_DELETE_LIMITremoval fromutils/docker/Dockerfiledoes not implement or test issue#416. The PR description attributes this change to issue#415. ThePHP_INI_SCAN_DIRre…Remove the unrelated SQLITE_ENABLE_UPDATE_DELETE_LIMITchange from this pull request, or link a coding issue that requires the SQLite build change and assess it as separate scope.-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the primary change: preventing the shared opcode cache from altering behavior. It is concise and directly related to the reported regression. Linked Issues check Issue #416requires WP-CLI scenarios with-ddisable_functions=ini_setto stop failing. The new scenario infeatures/testing.featurefirst runswp cli version, then runscli infowithini_set…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Linked Issues check
-
Explanation
Issue
#416requires WP-CLI scenarios with-ddisable_functions=ini_setto stop failing. The new scenario infeatures/testing.featurefirst runswp cli version, then runscli infowithini_setdisabled and requires return code 0.FeatureContext::get_php_tuning_env_variables()setsopcache.optimization_level=0, which prevents the shared cache from storing optimized code that removes the fallback. The supplied validation reports that the scenario passes with this change.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files. (2 skipped: 2 unsupported.)
-
Full details: Out of Scope Changes check
-
Explanation
The
SQLITE_ENABLE_UPDATE_DELETE_LIMITremoval fromutils/docker/Dockerfiledoes not implement or test issue#416. The PR description attributes this change to issue#415. ThePHP_INI_SCAN_DIRrestoration supports the test-process environment and is related to the test harness, but the SQLite build change has no demonstrated connection to the linked issue.Resolution
Remove the unrelated
SQLITE_ENABLE_UPDATE_DELETE_LIMITchange from this pull request, or link a coding issue that requires the SQLite build change and assess it as separate scope.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
github-actions
Bot
added
bug
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
A cache directory written by 5.4.0 holds optimized code, which opcache keeps using, and an older suite running at the same time would rewrite the configuration file. Name both directories after the settings, so that each set has its own. After a restart without Xdebug, PHPRC points to a configuration file that holds the scanned files already, which loaded them twice when it reached the WP-CLI processes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #416, a regression from #401 in 5.4.0.
Disabled functions
With the PHP settings of #401, the WP-CLI processes share a file-based opcode cache. The optimizer evaluates some calls at compile time, among them
function_exists()(optimizer pass 1, and others). WP-CLI defines a fallbackini_set()inphp/fallback-functions.phpbehind! function_exists( 'ini_set' ). Compiled in a process with the default settings, that condition is folded tofalseand the fallback is gone from the cached code. A later process with-ddisable_functions=ini_setgets the cached code and fails with:Reproduced by running
wp cli versionand thenWP_CLI_PHP_ARGS='-ddisable_functions=ini_set' wp cli info. It fails with any optimization level that includes pass 1, and with pass 1 alone, and passes withopcache.optimization_level=0.The cache now stores unoptimized code, so it only saves parsing and compiling, and the code does what it does without opcache, which is how the CLI runs by default. The suite of this package takes the same time either way (11 s with 4 workers, warm).
Scan directory after a restart without Xdebug
Behat (and Composer) restart without Xdebug through composer/xdebug-handler, which sets
PHP_INI_SCAN_DIRto an empty string in the restarted process. The tuning then passed only its own directory to the WP-CLI processes, which lost the scan directory of the system and every extension configured there (all of them on Debian and Ubuntu). The original directories are now taken fromXDEBUG_HANDLER_SETTINGS.SQLite option
As pointed out on #415,
SQLITE_ENABLE_UPDATE_DELETE_LIMITneeds a parser generated from the canonical sources, which the amalgamation does not have, so the option is left out.Testing
features/testing.featurethat runs WP-CLI normally and then withini_setdisabled. It fails onmainwith the error above and passes with this change.features/bootstrap.feature:506andfeatures/utils.feature:108pass with this change and fail with 5.4.1.PHP_INI_SCAN_DIRgets/etc/php/conf.d:/tmp/wp-cli-test-php-configinstead of only/tmp/wp-cli-test-php-config.Summary by CodeRabbit
cli infosucceeds and reports the PHP binary whenini_setis disabled. The scenario is skipped on Windows.