[META] Make Drupal 7 core compatible with PHP 8.0

Thanks for filing this :)

Running PHP 8 tests...

Whats going on here - I see an immediate Composer error on the tests? Anything that needs to be done about it?

In needs to override CI command to by-pass composer

@andypost: So, what do we do now about this? Do we have to contact/inform anyone who can change/fix this?

The --ignore-platform-reqs option should'nt be used for D7 since testbot CI doesn't use composer to install D7.

However, The PHP 8.0 & MySQL 5.7 errors should be fixed: D7 bootstrap fails due to multiple errors:

ArgumentCountError: Too few arguments to function _drupal_error_handler(), 4 passed and exactly 5 expected in DrupalLocalStreamWrapper->url_stat() (line 837 of /var/www/html/includes/stream_wrappers.inc).

TYPO3\PharStreamWrapper\Exception: Unexpected file extension in "phar:///usr/local/bin/drush/commands/core/drupal" in Drupal\Core\Security\PharExtensionInterceptor->assert() (line 38 of /var/www/html/misc/typo3/drupal-security/PharExtensionInterceptor.php).

Error: Call to undefined function user_access() in system_custom_theme() (line 1974 of /var/www/html/modules/system/system.module).
TYPO3\PharStreamWrapper\Exception: Unexpected file extension in "phar:///usr/local/bin/drush/commands/core/drupal" in 

PHP Deprecated:  Required parameter $connection follows optional parameter $alias in /var/www/html/includes/database/select.inc on line 967

Error: Required parameter $connection follows optional parameter $alias in /var/www/html/includes/database/select.inc, line 967

Got a deprecation warning related to PHP 8 on D7, which doesn't appear here:

- Deprecated function: The behavior of unparenthesized expressions containing both '.' and '+'/'-' will change in PHP 8: '+'/'-' will take a higher precedence in include_once() (line 1387 of (...)/includes/bootstrap.inc).

Not sure whether it should go here or in another issue ? Child issue is #3156847: [PHP 8] Parameter order fixes.

@fgm ++ to create child issue

Yes please; child issues for each separate problem.

I managed to get Drupal (current branch HEAD) working with PHP 8. There were couple changes needed in filter.module to to disable its error reporter for HTML 5 compatibility, and _drupal_error_handler update necessary to avoid ArgumentCountError exceptions, which I'm not sure are PHP 8.0 exclusive (as it could be a libxml change too).

I'm loading a patch of my copy if anyone would like to replicate it. None of patches are reviewed by community, so please don't use on any site worth its salt.

1. +++ b/includes/bootstrap.inc
   @@ -2600,7 +2600,7 @@ function drupal_get_hash_salt() {
   -function _drupal_error_handler($error_level, $message, $filename, $line, $context) {
   +function _drupal_error_handler($error_level, $message, $filename, $line, $context = array()) {
   

   it looks ok

2. +++ b/includes/database/database.inc
   @@ -705,7 +705,7 @@ protected function filterComment($comment = '') {
   -  public function query($query, array $args = array(), $options = array()) {
   +  public function runQuery($query, array $args = array(), $options = array()) {
   
   +++ b/includes/database/pgsql/database.inc
   @@ -85,7 +85,7 @@ public function prepareQuery($query) {
   -  public function query($query, array $args = array(), $options = array()) {
   +  public function runQuery($query, array $args = array(), $options = array()) {
   

   why that change?
   it probably needs some workaround if query() method affected

3. +++ b/includes/database/select.inc
   @@ -964,7 +964,7 @@ class SelectQuery extends Query implements SelectQueryInterface {
   -  public function __construct($table, $alias = NULL, DatabaseConnection $connection, $options = array()) {
   +  public function __construct($table, $alias, DatabaseConnection $connection, $options = array()) {
   
   +++ b/modules/book/book.test
   @@ -146,7 +146,7 @@ function testBook() {
   -  function checkBookNode($node, $nodes, $previous = FALSE, $up = FALSE, $next = FALSE, array $breadcrumb) {
   +  function checkBookNode($node, $nodes, $previous = FALSE, $up = FALSE, $next = FALSE, array $breadcrumb = array()) {
   
   +++ b/modules/field/tests/field_test.storage.inc
   @@ -243,7 +243,7 @@ function field_test_field_storage_delete_revision($entity_type, $entity, $fields
   -function field_test_field_storage_query($field_id, $conditions, $count, &$cursor = NULL, $age) {
   +function field_test_field_storage_query($field_id, $conditions, $count, &$cursor = NULL, $age = NULL) {
   
   +++ b/modules/file/file.field.inc
   @@ -593,7 +593,7 @@ function file_field_widget_uri($field, $instance, $data = array()) {
   -function file_field_widget_value($element, $input = FALSE, $form_state) {
   +function file_field_widget_value($element, $input = FALSE, $form_state = NULL) {
   
   +++ b/modules/openid/openid.module
   @@ -743,7 +743,7 @@ function openid_association_request($public) {
   -function openid_authentication_request($claimed_id, $identity, $return_to = '', $assoc_handle = '', $service) {
   +function openid_authentication_request($claimed_id, $identity, $return_to = '', $assoc_handle = '', $service = array()) {
   
   +++ b/modules/simpletest/drupal_web_test_case.php
   @@ -3084,7 +3084,7 @@ protected function assertNoText($text, $message = '', $group = 'Other') {
   -  protected function assertTextHelper($text, $message = '', $group, $not_exists) {
   +  protected function assertTextHelper($text, $message = '', $group = NULL, $not_exists = NULL) {
   
   @@ -3150,7 +3150,7 @@ protected function assertNoUniqueText($text, $message = '', $group = 'Other') {
   -  protected function assertUniqueTextHelper($text, $message = '', $group, $be_unique) {
   +  protected function assertUniqueTextHelper($text, $message = '', $group = NULL, $be_unique = NULL) {
   
   @@ -3272,7 +3272,7 @@ protected function assertNoTitle($title, $message = '', $group = 'Other') {
   -  protected function assertThemeOutput($callback, array $variables = array(), $expected, $message = '', $group = 'Other') {
   +  protected function assertThemeOutput($callback, array $variables = array(), $expected = NULL, $message = '', $group = 'Other') {
   
   +++ b/modules/simpletest/tests/upgrade/upgrade.locale.test
   @@ -124,7 +124,7 @@ public function testLocaleUpgradeDomain() {
   -  public function assertPageInLanguage($path = NULL, $langcode) {
   +  public function assertPageInLanguage($path = NULL, $langcode = NULL) {
   

   all this fixes could go to paramemters issue

4. +++ b/modules/filter/filter.module
   @@ -1103,6 +1103,7 @@ function _filter_tips($format_id, $long = FALSE) {
    function filter_dom_load($text) {
      $dom_document = new DOMDocument();
   +  libxml_use_internal_errors(true);
   

   this line needs comment explaining why and probably call it for php>=800

Updating issue title and summary since PHP 8 is out, so it does not evolve anymore.

So, I've spent a decent amount of time looking at this, and seems like the effort required to have D7 running on PHP8 is comparable to D7 on the PHP7.4 issue.

However, here there is a single more challenging piece: database API, which inherits from PDO directly.

I will be creating some children issues in a few following days, so stay tuned :)

@Taran2L: did you find more on top of #3185918: [PP-1] [PHP 8] Fix DatabaseConnection::query signature mismatch with PDO::query?

@Gábor Hojtsy, yes. Please check the corresponding issue.

So, as promised I spent some time on this:

There are 5 issues that prevent test runs 2 of them are more or less straightforward; 3 are more complex/problematic. After fixing those 5, the rest should be more or less easy (see the 17 fails above)

Easier ones:

1. #3200407: [PHP8] ArgumentCountError: Too few arguments to function _drupal_error_handler() and friends
2. #3156847: [PHP 8] Parameter order fixes

More complex issues are all database connection management related:

1. Extending PDO directly is not an option anymore - #3185918: [PP-1] [PHP 8] Fix DatabaseConnection::query signature mismatch with PDO::query (D9 does the same)
2. Implementing the above forces us to redo the connection destruction - #3200708: [PHP8] Error: User-supplied statement does not accept constructor arguments in PDO->prepare() (D9 does the same) - BC layer can be more or less easily added
3. Fix racing condition when DrupalCacheArray is being destructed => it tries to persistent keys write to the DB -> DB connection might not be there anymore, as PHP does not guarantee the order of the object destruction -> D8/9 fixes this by not using the __destruct() but the kernel.terminate event

PoC for the 5 issues above is in the 361 MR

Thanks for all the work that's gone into this so far.

I've been looking at:

• #3200407: [PHP8] ArgumentCountError: Too few arguments to function _drupal_error_handler() and friends
• #3185918: [PP-1] [PHP 8] Fix DatabaseConnection::query signature mismatch with PDO::query
• #3200708: [PHP8] Error: User-supplied statement does not accept constructor arguments in PDO->prepare()

...and running the Database tests locally with PHP 8.0 with all of those changes.

Seems like we're very close to passing, but I've hit this exception which I wanted to record for posterity:

PDOException: There is no active transaction in DrupalTestCase->run() (line 527 of /path/to/drupal-7.x/modules/simpletest/drupal_web_test_case.php). 	Uncaught exception	database.inc	541	PDO->commit()

That seems to be roughly between DatabaseTransactionTestCase->testCommittedTransaction() and DatabaseTransactionTestCase->testTransactionStacking() but I'm not certain which test actually triggered this.

The DB test fail mentioned in #22 looks like it's caused by a change in PHP 8.0 which has already been addressed in D8/9.

Here's a backport issue to fix that in D7: #3204161: [D7] MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction

hi @mcdruid, there is another issue that needs to be committed before going into PDO issues #3156847: [PHP 8] Parameter order fixes

Thanks @Taran2L - there are now 5x PHP 8.0 issues at the top of #3192080: [meta] Priorities for 2021-04-07 release of Drupal 7 and that includes the param order one you mention.

I plan to focus on those in order to make it practical to start testing PHP 8.0 with drupalci.

Do you know of any other blockers for testing?

We've now got tests running through properly.

AFAICS there are 5 tests with failures:

$ curl -s https://dispatcher.drupalci.org/job/drupal_d7/129581/console | grep '[0-9] fail' | grep -v ' 0 fail' | perl -pe 's#<.*?>##g;s#^[0-9\:]* ##'

Form element validation 315 passes, 3 fails, and 0 exceptions
SimpleTest error collector 3 passes, 1 fail, and 2 exceptions
Drupal error handlers 22 passes, 8 fails, and 0 exceptions
SimpleTest functionality 90 passes, 23 fails, and 42 exceptions
User administration 50 passes, 17 fails, and 4 exceptions

Some of those may be covered by existing issues, but I've created one new issue per test:

• #3206428: [PHP 8] test failures in Form element validation
• #3206429: [PHP 8] test failures in Drupal error handlers
• #3206430: [PHP 8] test failures in SimpleTest functionality

(combined two test failure lines from above, both relate to SimpleTest)

• #3206431: [PHP 8] test failures in User administration

plus #3206438: [PHP 8] deprecated functions in OpenID

Wow - thanks for jumping on these last few issues!

I'd half forgotten the approach that seemed to work quite well for PHP 7.4 (#3081386-50: [META] Fully support PHP 7.4 in Drupal 7).

Let's try that with the last test of @Liam Morland's noop's patch in #5:

$ curl -s https://www.drupal.org/pift-ci-job/2017309 | grep -o 'exception: .* Line [0-9]* of .*:' | perl -pe 's#<.*?>##g' | sort | uniq -c | sort -rn

    100 exception: [Deprecated function] Line 147 of modules/openid/openid.inc:
     98 exception: [Deprecated function] Line 196 of modules/openid/openid.inc:
     36 exception: [Warning] Line 242 of modules/simpletest/simpletest.test:
      4 exception: [Warning] Line 220 of modules/simpletest/simpletest.test:
      4 exception: [ArgumentCountError] Line 64 of modules/user/user.admin.inc:
      2 exception: [Warning] Line 210 of modules/simpletest/simpletest.test:
      1 exception: [Warning] Line 43 of modules/simpletest/tests/error_test.module:
      1 exception: [DivisionByZeroError] Line 45 of modules/simpletest/tests/error_test.module:

Any of those not covered by the issues we're already working on?

• #3206428: [PHP 8] test failures in Form element validation
• #3206429: [PHP 8] test failures in Drupal error handlers
• #3206431: [PHP 8] test failures in User administration
• #3206438: [PHP 8] deprecated functions in OpenID

Will try to get these all reviewed and committed ASAP.

looking forward to this, as soon as those get committed I'll start updating all my contrib projects to enable php 8 testing

Let's test all patches combined in this noop issue.

WOW! Great work @Taran2L and @mcdruid!

Congrats!

So that's a combination of the patches from the 4 issues listed in #28 @Taran2L?

hi @mcdruid, yes, exactly

Woot woot! Tests passing, thanks for the amazing work everyone 🚀.

Committed all of those changes this morning; we now have tests passing in PHP 8(.0)!

Thanks to all the contributors - I wasn't sure we'd get this far in time for the release next week, but am pleasantly surprised.

I won't close this issue yet as there may well be more PHP 8 fixes not revealed by the test suite.

We won't leave it open indefinitely though.

Triggered php 8 tests for 'media'
https://www.drupal.org/pift-ci-job/2019363

Triggered php 8 tests for 'file_entity'

https://www.drupal.org/pift-ci-job/2019361

I will add tests for other projects probably this weekend.

Thanks again everyone!

@mcdruid, thanks for your time and fast processing times! Great job.

Thanks to everyone else involved!

Core is ready, there'll be lots of work in contrib

assistance with media and file_entity is appreciated

I would like to test PHP 8.0 & Postgresql.x. My local install showed an error, but I would prefer the automated test bot so that I can insure that it is not something peculiar about my install (it was an Ubuntu server upgraded from 18.04 to 20.04 and there were some hiccups reported by the sysadmin during upgrade).

Not sure if this is the place to report this, but when I upgrade my shared server from PHP 7.4 to 8.0, Drupal 7.81 cannot add nodes anymore. When saving them an unexpected error notification is displayed. Just viewing the websites does not cause any issues.

@Tino, please keep in mind that there's still several contrib modules that will need some changes /patching /updating to get PHP 8.0 working correctly. Thanks for reporting, if you have access to logs, please report where appropriate, open new issues if you need for various projects and share the links.

@joseph.olstad Got it, thanks! It's the Rules module (7.x-2.12).

Type: PHP
Error: Unknown named parameter $settings in FacesExtendable->__call() line 125 of /sites/all/modules/rules/includes/faces.inc).

@Tino, there is a work in progress issue for the rules module.
#3209769-8: [meta] Make D7 rules compatible with PHP 8.0/8.1

Awesome! Thank you @joseph.olstad.
The latest developement version of Rules (7.x-2.x-dev updated 10 Jun 2021 at 22:25 UTC) works with PHP 8.0.

php 8.0 tests for entity api:

the entity api module test has rules as a dependency, throws this error:

Unknown named parameter $type in FacesExtendable->__call() line 125 of rules/includes/faces.inc

not sure where to fix this yet. should the fix go into 'rules/includes/faces.inc' ?

when I googled I came accross this issue for core

here's the link to the entity api automated test run :

The entity api issue might be because the tests dependency picking up the latest tagged release of rules which doesn't have the fix instead of the dev release that does have the php 8.0 fix.

We should ask the rules maintainer to tag a php 8.0 compatible release from the php 8.0 compatible dev branch of rules
?

@mcdruid,
I guess we can mark this issue as fixed once we confirm the entity api is fixed by tagging, releasing the latest rules build ? @TR will probably do that for us soon.

there's a chain of events that have to happen for us to resolve file_entity and media module php 8.0 automated test passing as media relies on file_entity, which in turn automated tests depend on the entity api which in turn relies on rules for it's automated test coverage dependency.

so , in this order:

1. Tag and release rules 7.x-2.x branch for new tag #3210622: [D7] PHP 8.0 introduced breaking change to call_user_func_array()
2. confirm that the new rules release fixes automated testing then Tag and release entity api 7.x-1.x branch for new tag for 7.x-1.10 #3221001: Plan for release of 7.x-1.10
3. Confirm that the new entity api build fixes file_entity automated testing (test dependency) and then tag 7.x-2.x/7.x-3.x branch for new file_entity release #3208277: PHP 8 compatibility for file_entity
4. confirm that the new file_entity release fixes media php 8.0 automated test coverage , review if anything needs adjusting in the media module #3208029: PHP 8.0 compatibility for Media
5. Follow up with the core issue for php 8.0, if everything looks good, mark this issue as fixed: #3145797: [META] Make Drupal 7 core compatible with PHP 8.0
6. Then continue working on making drupal core php 8.1 compatible #3224299: [META] Make Drupal 7 core compatible with PHP 8.1

Colour me confused, but what do all these different contrib modules have to do with core?

Should they not be discussed in their own issue queue?

@MustangGB , nothing other than they use core.

this issue can probably be closed as "Fixed"

Since #37, there were only reports of contributed projects not being compatible. Let's close this one! I think it is worth tracking PHP 8 issues for Drupal 7 contributed projects (and tagging them with "PHP 8.0" especially) but this issue was for core.

How is this fixed when there are still open child issues?

@MustanGB: good point, unfortunately those were not reflected in the comments or issue summary that I've seen.

I looked at the child issues and retitled #3224299: [META] Make Drupal 7 core compatible with PHP 8.1 and made it related instead of a child. Fixing that is not required to make PHP 8.0 compatibility :)

Looks like the other three open children are core issues.

#3165377: element_property() causes a notice in 7.4 if $key is integer
#3179120: Address operator priority change in PHP 8
#3210215: [PHP 8] Error: TypeError: round() in parse_size function

Added them to the issue summary as well.

Now with 3165377 fixed, I retriggered media automated tests today with php 8.0 and it's all GREEN. I'm now triggering file_entity and I suspect it will also be all green but not sure yet.
#3165377: element_property() causes a notice in 7.4 if $key is integer

*** EDITTED ***
another update, two more GREEN php 8.0 compatible contrib modules
media is now passing PHP 8.0 tests
and thanks again to @mcdruid, he helped pinpoint a fix for file_entity also and a new release of file_entity fixes php 8.0 compatibility
#3208277: PHP 8 compatibility for file_entity

The 3x child issues mentioned in #56 (and earlier comments) are now fixed or postponed (pretty sure that one is not a problem in core).

So I'm going to close this as fixed, and we'll move on to PHP 8.1 :) Thanks everybody that contributed here.

Great stuff, and thanks to both of you (Drew and Gabor) for listening.