Make WordPress Core

Opened 3 years ago

Closed 4 months ago

Last modified 3 months ago

#56009 closed task (blessed) (fixed)

Prepare for PHP 8.2

Reported by: jrf's profile jrf Owned by:
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: General Keywords: php82 has-patch
Focuses: php-compatibility Cc:

Description (last modified by oglekler)

This is a meta ticket to track the efforts to prepare for PHP 8.2.

For PHP 8.0/PHP 8.1 specific fixes, please refer to the generic WP 6.1 PHP 8.x ticket: #55656

Please link patches related to a specific PHP 8.2 related task to the appropriate dedicated issue, if there is one (see the links in the description below).

Generic/one-off PHP 8.2 related patches can be linked to this ticket.


PHP 8.2: Important dates

PHP 8.2 is expected to be released on November 24 2022.

Other note-worthy dates:

  • The first alpha was released on June 9th 2022.
  • Feature freeze will be on July 19, 2022.

Note:

The below represents the status per June 20, 2022. As PHP 8.2 is not yet in feature freeze, these statuses can still deteriorate based on additional RFCs being accepted/implemented.

Readiness of essential tooling

Composer

Current status:

  • CI for Composer itself is being run against PHP 8.2.
  • No known issues in the last release (2.3.7).

PHPUnit

Current status:

  • CI for PHPUnit itself is being run against PHP 8.2.
  • No known issues in the last release (9.5.21).

PHPUnit Polyfills

Current status:

  • CI for PHPUnit Polyfills itself is being run against PHP 8.2.
  • No known issues in the last release (1.0.3).

WP-CLI

Current status:

  • CI for WP-CLI is NOT (yet) being run against PHP 8.2.
  • Status unknown.

Other tooling

Other tooling doesn't necessarily have to run against PHP 8.2 (yet), so has not been evaluated.

Initial DevOps Tasks

Typical tasks which need to be executed to allow WordPress to prepare for PHP 8.2:

Docker

  • Add PHP 8.2 to the Docker images. A PR for this was merged on Jun 23, 2022

GitHub Actions

TODO:

  • Add PHP 8.2 to the GitHub Actions phpunit-tests.yml configuration.

Status: Committed in [53922]

Notes:

  • Test failures on PHP 8.2 should not (yet) fail the build.
  • The composer install will most likely need to run with --ignore-platform-req=php as not all dependencies of PHPUnit 9.x have declared compatibility with PHP 8.2 yet.

PHP 8.2 changes for which WordPress will need to prepare

Similar to the rest of this ticket, this list is based on the current status of PHP 8.2 and still subject to change.

Deprecation of ${} string interpolation

Only two small issues + one in the tests.

This was being tracked in #55787. Committed in [54134]

Deprecation of utf8_encode and utf8_decode

The issue will not affect WordPress Core much, but will have a significant impact on plugins/themes.

The recommendation is to make the MbString extension a requirement for WP Core to help plugins/themes mitigate this.

This is being tracked in #55603.

Deprecation of partially supported callables

No significant problems expected in WP Core.

WordPress does use callables extensively, but the particular type of callables being deprecated are not typically used within WordPress.

Deprecate dynamic properties

This is the big one and I expect a HUGE amount of problems due to this.

In my opinion two groups of patches are needed to at least try and mitigate this. I intend to write up the details for this in two separate tickets over the next week or so.

I have opened two tickets to track and address these issues:

  • #56033 for "known" dynamic properties which should be declared on the class.
  • #56034 for "unknown" dynamic properties

Ticket #55357 is related to this, but should not be actioned in isolation.

Locale-independent case conversion

Needs investigation if and if so, how extensive the impact will be on WordPress.

If there is any impact, making the MbString extension a requirement for WP Core would likely go a long way to mitigate this.

Remove support for libmysql from mysqli

This is primarily an issue which may impact webhosts.

No direct impact on WordPress itself is expected.

Status of External Dependencies

GetID3

Current status:

  • A PR has been merged to enable linting against PHP 8.2. The build passes without finding any PHP 8.2 related issues.
  • Important: the project has no test suite, so the linting passing on PHP 8.2 is only a small comfort and does not provide any real security.
  • In other words: status unknown.
  • WordPress is using the latest version (1.9.21), see #54162

PHPMailer

Current status:

  • Linting and tests are being run against PHP 8.2.
  • No known issues in the last release (6.6.2).
  • WordPress is using the latest version, see #55976

Random Compat

Current status:

  • A PR has been merged to enable running of the tests against PHP 8.2. The build passes without finding any PHP 8.2 related issues.
  • No known issues in the last release (2.0.21).
  • WordPress is using the latest version, see #55181

Requests

Current status:

  • A PR was opened to enable running of linting and tests against PHP 8.2. The PR build found one, test-only, issue in Requests for PHP 8.2, which has no impact on the production code.
  • No known issues in the last release (2.0.3).
  • WordPress was upgraded to 2.0.0 via [54997] (see #54504)

I've done a test run of Requests 1.8.1 against PHP 8.2 and based on the tests and aside from the already known PHP 8.1 deprecations, there are no relevant PHP 8.2 issues known at this moment.

SimplePie

Current status:

  • A PR is open to enable running of the tests against PHP 8.2. The PR build passes without finding any PHP 8.2 related issues.
  • No known issues in the last release (1.6.0).
  • WordPress is behind and is still using version 1.5.8, while the latest release is 1.6.0, see #55604

I've done a test run of SimplePie 1.5.8 against PHP 8.2 and based on the tests, there are no relevant PHP 8.2 issues known at this moment.

Sodium Compat

Current status:

  • A PR has been merged to enable running of the tests against PHP 8.2. The build passes without finding any PHP 8.2 related issues.
  • No known issues in the last release (1.17.1).
  • WordPress is using the latest version, see #55453 / [52988].

Change History (48)

#1 follow-up: @knutsp
3 years ago

Deprecate dynamic properties

Is adding

#[AllowDynamicProperties]

to all/most/some classes for 6.1, then remove it for classes that, after further considerations, should not be extended like this, in 6.2 and 6.3, a way to go?

It sees to me at least WP_Post, WP_Term and WP_User will need dynamic properties.

#2 follow-up: @johnbillion
3 years ago

  • Version trunk deleted

Let's also add PHPCS/WPCS and PHPCompatibility/PHPCompatibilityWP to the "Readiness of essential tooling" list.

#3 in reply to: ↑ 2 @jrf
3 years ago

Replying to johnbillion:

Let's also add PHPCS/WPCS and PHPCompatibility/PHPCompatibilityWP to the "Readiness of essential tooling" list.

@johnbillion Could you eleborate on that ?

In my opinion, PHPCS/WPCS are actually not relevant as WP won't start using any of the new PHP syntaxes until the minimum PHP version goes up to, so any problems PHPCS may have with those won't impact WordPress' PHP 8.2 readiness.
And sniffs are all set up to give the same results independently of the PHP version on which PHPCS is run, so again, that is not a problem.

The same applies for PHPCompatibility, though obviously there is an extra aspect to that: has PHPCompatibility put sniffs in place to detect incompatibilities ?

Work is ongoing for that behind the scenes, but that's a whole other (and very long) story. But for all practical purposes, it also isn't a problem as even for those sniffs not yet published, let alone released, the scan can still be executed (and is semi-regularly) due to the short line between WP and the maintainers of PHPCompatibility (aka: me) and tickets are opened for newly detected incompatibilities found (like some of the above listed tickets).

Also note that while PHPCompatibility can catch _a lot_ of issues, the most problematic changes in PHP cannot be detected by PHPCompatibility as they depend on access to the runtime value of variables or cross-file scanning, which PHPCompatibility does not have.
Think: the PHP 8.1 "passing null to non-nullable" deprecation or the PHP 8.2 dynamic properties change (which can be partially detected, but detection is limited to the file being scanned).

#4 follow-up: @johnbillion
3 years ago

Yeah I understand those tools won't be updated to sniff the new syntaxes for a while yet, I just meant we need to be sure they run on PHP 8.2 without issue.

#5 in reply to: ↑ 4 @jrf
3 years ago

Replying to johnbillion:

Yeah I understand those tools won't be updated to sniff the new syntaxes for a while yet, I just meant we need to be sure they run on PHP 8.2 without issue.

I understand what you are saying, but as sniffs are designed to give the same results independently of the PHP version on which they are run, it isn't relevant for ensuring PHP 8.2 compatibility for WordPress.

I mean, even if you run PHPCompatibility on PHP 5.4, it will still flag the same PHP 8.2 incompatibilities as when you run PHPCompatibility on PHP 8.2.

Those tools currently run on PHP 7.4 and it is not necessary to update that version. Might be a nice to have, but not a necessity. I.e. not essential.

#6 @jrf
3 years ago

  • Description modified (diff)

#7 in reply to: ↑ 1 @jrf
3 years ago

Replying to knutsp:

Deprecate dynamic properties

Is adding

#[AllowDynamicProperties]

to all/most/some classes for 6.1, then remove it for classes that, after further considerations, should not be extended like this, in 6.2 and 6.3, a way to go?

It sees to me at least WP_Post, WP_Term and WP_User will need dynamic properties.

There is a large list of classes which need handling. I have now opened ticket #56034 with an extensive proposal on how to handle this for the whole of WP Core.

Additionally, I have opened ticket #56033 to fix the "known" dynamic property issues.

@knutsp I'd appreciate it if you would have a read through of both issues and value your opinion on them (in particular on issue #56034).

This ticket was mentioned in Slack in #core-php by jrf. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by sergey. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by chaion07. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


3 years ago

This ticket was mentioned in Slack in #hosting-community by amykamala. View the logs.


3 years ago

This ticket was mentioned in PR #3119 on WordPress/wordpress-develop by jrfnl.


2 years ago
#15

  • Keywords has-patch added

Notes:

  • The composer install for PHP 8.1 was still using --ignore-platform-reqs, while that hasn't been needed anymore for quite a while, so I'm just re-using the condition for PHP 8.2 now.
  • I've also made the --ignore-platform... option used for the install more specific.

Composer 2.0 added a --ignore-platform-req=... option to selectively ignore platform requirements.
Composer 2.2 then added the ability to only ignore the upper bound of platform requirements by adding the + sign.

  • As discussed for PHP 8.1, builds against PHP 8.2 will still be allowed to fail (and will fail, not just due to new PHP 8.2 issues, but also due to unsolved PHP 8.1 issues). Allowing those builds to fail prevents merges in the Gutenberg project becoming blocked.
  • As for the Xdebug related tests: those will not be run on PHP 8.2 yet as the Docker image for PHP 8.2 does not contain Xdebug yet. Once a stable release of Xdebug 3.2.0 is available, it can be added to the Docker image and the test step can then be enabled for PHP 8.2.

Refs:

Trac ticket: https://core.trac.wordpress.org/ticket/56009

#16 @jrf
2 years ago

Add PHP 8.2 to the GitHub Actions phpunit-tests.yml configuration.

I've opened PR GH3119 to address this.

jrfnl commented on PR #3119:


2 years ago
#17

Previous similar ticket for PHP 8.1: https://core.trac.wordpress.org/ticket/53891

#18 @SergeyBiryukov
2 years ago

In 53922:

Build/Test Tools: Enable running the tests on PHP 8.2.

PHP 8.2 is expected to be released at the end of November 2022.

Enabling the tests to run in CI on PHP 8.2 allows WordPress core to start getting ready.

As an interim measure, while working through the PHP 8.1 and 8.2 issues, builds against these PHP versions are allowed to fail, so that they don't block PR merges in the Gutenberg project.

Notes:

  • The composer install command for PHP 8.1 was still using the --ignore-platform-reqs option, while that has not been needed anymore for quite a while, so the condition is now reused for PHP 8.2.
  • The --ignore-platform... option used for the install is now made more specific:
    • Composer 2.0 added a --ignore-platform-req=... option to selectively ignore platform requirements.
    • Composer 2.2 then added the ability to only ignore the upper bound of platform requirements by adding the + sign.
  • Xdebug-related tests will not be run on PHP 8.2 at this time as the Docker image for PHP 8.2 does not contain Xdebug yet. Once a stable release of Xdebug 3.2.0 is available, it can be added to the Docker image and the test step can then be enabled for PHP 8.2.

References:

Follow-up to [49077], [49162], [50299], [51588], [51604].

Props jrf, desrosj.
See #56009.

SergeyBiryukov commented on PR #3119:


2 years ago
#19

Thanks for the PR! Merged in r53922.

jrfnl commented on PR #3119:


2 years ago
#20

Thank you @SergeyBiryukov !

#21 @antonvlasenko
2 years ago

Locale-independent case conversion

I've investigated the possible influence of this PHP 8.2 change on WordPress.
Please see the examples below:
https://3v4l.org/9OGQo/rfc#v8.1.9 <- PHP 8.1 (and below)
https://3v4l.org/9OGQo/rfc#vgit.master <- PHP 8.2
The test fails on PHP 8.2 too. But, at least, strtoupper() doesn't "corrupt" the string.
This is some good news. As can be seen from the test, PHP 8.2 better processes multibyte characters passed to "regular" (non-mbstring) string functions.
I haven't found any differences in processing extended ASCII characters (128-255) in PHP 8.2 compared to PHP 8.1.

This ticket was mentioned in Slack in #core by sergey. View the logs.


2 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-test by jrf. View the logs.


2 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


2 years ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


2 years ago

#27 @SergeyBiryukov
2 years ago

In 54369:

Build/Test Tools: Remove PHP 8.1 and 8.2 from allowed failures.

With all known unit test failures now addressed, WordPress 6.1 aims to support PHP 8.1 and 8.2 as much as possible.

While full compatibility with PHP 8.1 and 8.2 is still a work in progress, this commit aims to actively prevent new PHP issues from being introduced in WordPress core.

All remaining known issues are deprecation notices. Please note, a deprecation notice is not an error, but rather an indicator of where additional work is needed for compatibility before PHP 9 (i.e. when the notices become fatal errors). With a deprecation notice, the PHP code will continue to work and nothing is broken.

Follow-up to [49077], [49162], [50299], [51588], [51604], [53922], [54072].

Props jrf, desrosj.
See #55652, #55656, #56009, #56681.

This ticket was mentioned in Slack in #core by sergey. View the logs.


2 years ago

#29 @SergeyBiryukov
2 years ago

  • Milestone changed from 6.1 to 6.2

Moving to 6.2 to continue the work started during the 6.1 cycle.

#30 @desrosj
2 years ago

In 54507:

Build/Test Tools: Remove note about some PHP versions being allowed to fail.

This removes the note stating that PHP 8.1 (see [51604]) and 8.2 (see [53922]) are temporarily allowed to fail.

The compatibility issues were resolved and continue-on-error was removed in [53922].

See #56009.

#31 @desrosj
2 years ago

In 54967:

Build/Test Tools: Run Xdebug tests on PHP 8.2.

Xdebug 3.2.0 (which adds support for PHP 8.2) has been released and is now included in the PHP 8.2 Docker container.

The tests in the xdebug group can now be run against all PHP versions currently supported by WordPress.

See https://github.com/WordPress/wpdev-docker-images/pull/92.

See #56009.

This ticket was mentioned in Slack in #core by sergey. View the logs.


2 years ago

#33 @bjorsch
2 years ago

I mentioned both this ticket and #56033 in https://github.com/WordPress/wordpress-develop/pull/3777, since the latter ticket is more relevant but also has been closed. The thing that does the pings seems to have not liked the double mention, so I'll mention it manually.

This ticket was mentioned in PR #3777 on WordPress/wordpress-develop by @bjorsch.


2 years ago
#34

The IXR_Message class declares a property _currentTag, which is never assigned or used. It does assign to currentTag, which outside of that one assignment is never used either. A search on wpdirectory didn't turn up anything using either property (but lots of results for other classes having a property of the same name).

Since there are various other underscore-prefixed properties declared on the class, including one named _currentTagContents which is used in several places, it seems likely the declared property is correct and the assignment is a typo.

Trac ticket: https://core.trac.wordpress.org/ticket/56009
Trac ticket: https://core.trac.wordpress.org/ticket/56033

@bjorsch commented on PR #3777:


2 years ago
#35

Added a test, as requested.

@SergeyBiryukov commented on PR #3777:


2 years ago
#36

Thanks for the PR! Merged in r55105.

This ticket was mentioned in Slack in #core by costdev. View the logs.


2 years ago

#38 @hellofromTonya
2 years ago

  • Description modified (diff)
  • Milestone changed from 6.2 to 6.3

Recap of the status:

  • Updated completions in the description.
  • Open TODOs:
    • Upgrade SimplePie, see #55604.
    • Handling of "unknown" dynamic properties, see #56034.
    • Address deprecations of utf8_encode() and utf8_decode() functions, see #55603.
    • Incomplete magic methods in WP_List_Table, WP_User_Query, and WP_Text_Diff_Renderer_Table, see #56876.

With the last 6.2 beta happening right now and RC1 next week, moving the remaining work to 6.3.

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


19 months ago

#40 @oglekler
19 months ago

  • Description modified (diff)
  • Milestone changed from 6.3 to 6.4

This ticket was discussed during a bug scrub, and it was decided to move it into 6.4 for work on remaining tickets.

Add props to: @hareesh-pillai, @mukesh27 and @costdev

#41 @johnbillion
18 months ago

Is there anything left to do on this meta ticket or can we close it off?

#42 @hellofromTonya
16 months ago

  • Milestone changed from 6.4 to 6.5

Yes, there's still work to do, such as dynamic properties.

Ready patches were committed.

6.4 RC1 is in a few hours. Moving the remaining work to 6.5.

This ticket was mentioned in Slack in #core by jrf. View the logs.


16 months ago

#44 @swissspidy
12 months ago

  • Milestone changed from 6.5 to 6.6

#45 @desrosj
7 months ago

  • Milestone changed from 6.6 to 6.7

This ticket was mentioned in Slack in #core by stoyangeorgiev. View the logs.


4 months ago

#47 @stoyangeorgiev
4 months ago

  • Resolution set to fixed
  • Status changed from new to closed

This one was discussed at a bug-scrub session.

It seems like all that if left here is handling dynamic properties. It looks like #56033 covered known properties. With this one being closed/fixed, all that is left is #56034 to cover unknown properties.

props to @drewapicture & @johnbillion

Last edited 4 months ago by stoyangeorgiev (previous) (diff)

#48 @jorbin
3 months ago

  • Focuses php-compatibility added
Note: See TracTickets for help on using tickets.