Prepare for PHP 8.6: drop removed internal APIs, add 8.6 build presets - #141
Merged
Conversation
|
Tick the box to add this pull request to the merge queue (same as
|
php-src master (8.6) removes several internal APIs this extension relies on. None of the replacements are version-specific, so no PHP_VERSION_ID guards are needed — the new spellings compile identically on 8.3-8.5. - XtOffsetOf() -> offsetof(): the alias is gone in 8.6. 101 call sites across src/ and include/, plus the descriptor generator, which would otherwise re-emit the removed macro on the next build. - zend_binary_zval_strcmp() -> zend_binary_strcmp() in ValueHash.c. It was removed as unsafe for relying on the zvals having a specific type; the enclosing switch already establishes IS_STRING, so the operands are unwrapped explicitly. - compile-php.sh: --with-pic was renamed to --enable-pic and the old spelling now hard-errors, so the flag is selected by version. Adds PHP 8.6 to the preset matrix (18 new presets). Regenerating CMakePresets.json also picks up pre-existing drift: the committed file still carried PHP_DRIVER_STATIC / PHP_DRIVER_BACKEND, which the generator renamed to PHP_SCYLLADB_* some time ago.
CodeLieutenant
force-pushed
the
compat/php-8.6
branch
from
August 4, 2026 09:07
6375722 to
c6fa33d
Compare
The lint job only checks files changed in the PR, so the offsetof sweep pulled Decimal.c and Tuple.c into scope and surfaced two pre-existing violations: - modernize-macro-to-enum: the double bit-width constants become a C23 fixed-underlying-type enum, matching the style used elsewhere. The mask macros still derive from them, so the values are unchanged. - readability-redundant-casting: index is already zend_ulong at both zend_hash_index_find() call sites. No behaviour change.
Same readability-redundant-casting class as the previous commit, in files the offsetof sweep pulled into lint scope: - Collection.c: index is already zend_ulong - DefaultIndex.c: value_str_length is already size_t - Timestamp.c: the outer cast wraps an expression that is already double Verified by running the lint job's own command over all 38 changed files locally: zero errors.
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.
Checked php-src
UPGRADING.INTERNALSagainst this codebase and fixed everything 8.6 breaks.What changed
XtOffsetOf()→offsetof()101 call sites across
src/andinclude/— every object-handler.offsetassignment and everyPHP_SCYLLADB_GET_*fetch macro. Also fixedtools/gen_descriptor/gen_class_descriptor.php, which would otherwise re-emit the removed macro into every*_descriptor.con the next build.zend_binary_zval_strcmp()→zend_binary_strcmp()One site,
src/Type/ValueHash.c. The enclosingswitch (Z_TYPE_P(...))already establishesIS_STRING, so the operands are unwrapped explicitly withZ_STRVAL_P/Z_STRLEN_P. No behaviour change.--with-pic→--enable-picscripts/compile-php.shselects the flag by version, so-v 8.5and-v 8.6both work.PHP 8.6 presets
PHP86added to thePHPVersionenum ingenerate-presets.php;CMakePresets.jsonregenerated (+18 presets: 3 build types × 3 backends × ZTS/NTS).Reviewer notes
XtOffsetOfwas always just an alias, andzend_binary_strcmphas existed throughout — so there is no#ifdef PHP_VERSION_IDanywhere in this diff.CMakePresets.jsonhas unrelated churn. The committed file was already stale relative to its generator: it still carriedPHP_DRIVER_STATIC/PHP_DRIVER_BACKEND, renamed toPHP_SCYLLADB_*in the generator some time ago. Regenerating picks that up (108 of the deleted lines). Pre-existing drift, not introduced here.CLAUDE.mdand.claude/skills/still documentXtOffsetOfas the canonical pattern in 9 places, and there is no CI matrix row for 8.6 yet. Both left for follow-ups.UPGRADING.INTERNALSapplies — the codebase has nozend_parse_parameters(),ZVAL_IS_NULL,INI_STR/INT/BOOL,zval_dtor,WRONG_PARAM_COUNT,Z_GC_*,Z_COPYABLE,EMPTY_SWITCH_DEFAULT_CASE,ZEND_RESULT_CODE,php_error_docref1/2, or underscore-prefixed stream calls. Theext/session,ext/uri,ext/xml, mysqlnd and intl changes don't touch us, andcmake/TargetOptimizations.cmakealready passes-undefined dynamic_lookupon Darwin, matching the new libtool behaviour.Verification
Built and tested against PHP 8.5 NTS debug (
DebugPHP8.5NTS), since 8.6 isn't released yet — this PR is forward-compat only and cannot be end-to-end verified on 8.6 today.cmake --build out/DebugPHP8.5NTS— clean, 228/228, no new warningspest --testsuite=Unit— 803 passed (12058 assertions), 27 skipped, 1 failedThe single failure is
UuidTest > it produces unique UUIDs across separate processes, a known pre-existing cross-process environment issue on this machine, unrelated to these changes.