Make Drupal 9 installable on PHP8

This patch allows Drupal to install on PHP 8. Won't work on PHP 7 yet - that's going to require more fun.

Turns out making in PHP 7 installable wasn't too hard...

Better way of doing the class aliasing and updating Symfony for less noise. That update will occur in #3156558: Upgrade dependencies prior to 9.1.0

We need this fix in Twig... https://github.com/twigphp/Twig/commit/663b8fa83270ae53c3c6f41de684e0282... - updating to a dev release for now.

We're going to need the ability to upgrade to PHPUnit - merging in #3127141: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3 and fixed another ::getClass() in proxy builder.

phpdocumentor/reflection-dockblock and phpspec/prophecy need minor PHP8 fixes because of reflection deprecations. I've aliased the classes and fixed them. Will look for PRs later.

Well that's fun. Upgrading to PHPUnit 9 is broken on PHP8 because the composer command fails!

00:03:37.375 Script @composer update phpunit/phpunit symfony/phpunit-bridge phpspec/prophecy symfony/yaml --with-dependencies --no-progress handling the drupal-phpunit-upgrade event returned with error code 2
00:03:37.375 Script Drupal\Core\Composer\Composer::upgradePHPUnit handling the drupal-phpunit-upgrade-check event terminated with an exception
00:03:37.674   ERROR: PHPUnit testing framework version 9 or greater is required when running on PHP 7.4 or greater. Run the command 'composer run-script drupal-phpunit-upgrade' in order to fix this.

:D

Might need to upgrade composer first... drupalci.yml might be needed.

Oh of course we need to ignore platform reqs...

Another round of fixes...

Fun with ZipArchive not supporting empty files anymore - we'll need to file an issue for behat\mink-selenium2-driver.

Here's the proper behat\mink-selenium2-driver fix and some more tweaks. I think we're very close to a total scope of work necessary to be PHP 8 compatible for Drupal 9. I think Drupal 8 is going to have a slightly harder time because it still supports PHP 7.0.

Let's finesse the hack :)

The JS tests pass locally... let's try a different way.

Spin off issues

• #3156542: \ReflectionParameter::getClass() is deprecated in PHP 8.0
• #3156878: \Drupal\Component\Datetime\DateTimePlus should pass correct parameter types to checkdate()
• #3156879: \Drupal\Component\Utility\Bytes::toInt() - ensure $size is a number type
• #3156880: \Drupal\Core\Access\CsrfTokenGenerator::validate() - ensure $token is a string before calling hash_equals()
• #3156881: Various methods of \Drupal\Core\Database\StatementInterface have changed their signature for PHP 8
• #3156882: \Drupal\Core\Render\Element\StatusReport::preRenderGroupRequirements() and \Drupal\user\PermissionHandler::sortPermissions() sorts return bools
• #3156883: \Drupal\Core\Url ensure fragment is not an empty string
• #3156885: Change \Drupal\error_test\Controller\ErrorTestController::generateWarnings() to throw E_NOTICE error compatible with PHP 8

I think all of the above issue can proceed without worrying about the how and if vendor support works for PHP 8 since the changes are all PHP 7 compatible. And hopefully we can keep the changes PHP 7.0 compatible to make things easy for Drupal 8.

Spin off issue to create that needs PHP 8 to be largely working.

• #3156887: \Drupal\system\Plugin\ImageToolkit\GDToolkit needs to support \GdImage objects for PHP 8 compatibility
  
  I think this issue will be okay for PHP 7.0 because it's really only loosening what we expect in \Drupal\system\Plugin\ImageToolkit\GDToolkit::$resource but testing this will be harder without all the above fixes and the fixes to vendor.

Maybe 3rd time lucky.

Yay! So 3rd time was lucky.

Test Result (6 failures / +3)
#slow.Drupal\BuildTests\Composer\Template\ComposerProjectTemplatesTest.Drupal\BuildTests\Composer\Template\ComposerProjectTemplatesTest
Build.Drupal\BuildTests\Framework\Tests\HtRouterTest.Drupal\BuildTests\Framework\Tests\HtRouterTest
System.Drupal\FunctionalTests\Bootstrap\UncaughtExceptionTest.Drupal\FunctionalTests\Bootstrap\UncaughtExceptionTest
Composer.Drupal\Tests\ComposerIntegrationTest.Drupal\Tests\ComposerIntegrationTest
Drupal.Drupal\Tests\Component\Serialization\YamlPeclTest.Drupal\Tests\Component\Serialization\YamlPeclTest
Scaffold.Drupal\Tests\Composer\Plugin\Scaffold\Functional\ScaffoldTest.Drupal\Tests\Composer\Plugin\Scaffold\Functional\ScaffoldTest

All these fails explainable - the composer ones due to deps incompatible with PHP 8 in their composer.json, missing pecl yaml extension. Apart from Drupal\FunctionalTests\Bootstrap\UncaughtExceptionTest - need to look into that one.

The PHP 7 fails are due to the mucking about with composer to use dev version of Twig and PHPUnit 9.

All the issues from this have been split into separately scoped issues - see #31 and the parent issue.

Fixing a couple of the PHP 8 failing tests hopefully.

Fixing htroutertest

Backing out #3127141: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3 to see if it is required. I was kind of throwing the kitchen sink at the problem when I added that one in.

So yep #3127141: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3 is needed. \Drupal\Tests\field\Kernel\Plugin\migrate\source\d7\FieldTest will fail with deprecations like

  62x: Method ReflectionParameter::getClass() is deprecated
    62x in FieldTest::testSource from Drupal\Tests\field\Kernel\Plugin\migrate\source\d7

  50x: Method ReflectionParameter::isArray() is deprecated
    50x in FieldTest::testSource from Drupal\Tests\field\Kernel\Plugin\migrate\source\d7

  36x: Method ReflectionParameter::isCallable() is deprecated
    36x in FieldTest::testSource from Drupal\Tests\field\Kernel\Plugin\migrate\source\d7

due to code in PHPUnit.

Hmmm #41 was wrong. The change in #39 causes that.

So fixing that a different way and fixing the composer hash test too.

Lol - so we need to upgrade to PHPUnit 9 for one test - \Drupal\Tests\Core\Database\ConnectionTest. And there's a very simple workaround for it. So I'm going to do that. Also one of the composer tests is fixable by updating composer. The composer update is done in #3156558: Upgrade dependencies prior to 9.1.0

So we should be down to 1 fail on 7.4 and 2 fails on PHP 8.0. The common fail is due to twig as a dev dependency - I'll try and ping fabpot for a release - and one because we don't have the pecl yaml extension on PHP 8.

The pecl yaml fail is interesting because it shouldn't be happening. The * @requires extension yaml is causing a because of our event listeners...

OK, but incomplete, skipped, or risky tests!
Tests: 6, Assertions: 0, Skipped: 7.

THE ERROR HANDLER HAS CHANGED!

Other deprecation notices (3)

  1x: The "PHPUnit\TextUI\ResultPrinter" class is considered internal This class is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not use it from "Drupal\Tests\Listeners\HtmlOutputPrinter".

  1x: The "Drupal\Tests\Listeners\DrupalListener" class implements "PHPUnit\Framework\TestListener" that is deprecated Use the `TestHook` interfaces instead.

  1x: The "Drupal\Tests\Listeners\DrupalListener" class uses "PHPUnit\Framework\TestListenerDefaultImplementation" that is deprecated The `TestListener` interface is deprecated.

The important thing is that the Symfony deprecations handler is triggering an error due to THE ERROR HANDLER HAS CHANGED!. This issue happens on PHP 7.4 too when a unit test has a @requires extension_that_does_not_exist. See I'll create another bug issue for that.

Twig 2.13.0 was released. Nice.

Rebase on 9.1.x now that we've updated our dependencies.

Adding more complicated BC layer from #3156881: Various methods of \Drupal\Core\Database\StatementInterface have changed their signature for PHP 8 so we can test on PHP 8.

Whoops.

See #3156881-12: Various methods of \Drupal\Core\Database\StatementInterface have changed their signature for PHP 8

@hussainweb please see all the sub issues of #3109885: [meta] Ensure compatibility of Drupal 9 with PHP 8.0 (as it evolves) before filing any issues and comment on the correct issue for each thing. The PDO typehinting is dealt with in #3156881: Various methods of \Drupal\Core\Database\StatementInterface have changed their signature for PHP 8

@hussainweb there's no order documented - they shouldn't overlap so there's no order to apply. This patch does have upstream fixes. It fixes the doctrine issue using a class alias - you have to run composer install to get these changes (i.e. there's no need to hack Doctrine code).

Here's a base patch and patch that includes the latest patches on
#3156881-16: Various methods of \Drupal\Core\Database\StatementInterface have changed their signature for PHP 8
#3156542-25: \ReflectionParameter::getClass() is deprecated in PHP 8.0
#3156887-11: \Drupal\system\Plugin\ImageToolkit\GDToolkit needs to support \GdImage objects for PHP 8 compatibility

We need #3127141-78: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3 too.

Our override of PHPSpec's ClassMirror is not necessary anymore. So removed that. And swapped out the PHPUnit 9 patch for #3127141-84: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3

Fixing some more issues - we need two more issues about named arguments and passing by reference incorrectly.

Aliasing the Reader class so we can concentrate on the Drupal issues.

New base patch and applying latest set of patches from:
#3156542-27: \ReflectionParameter::getClass() is deprecated in PHP 8.0
#3156881-16: Various methods of \Drupal\Core\Database\StatementInterface have changed their signature for PHP 8
#3173991-2: Incorrect passby reference use in anonymous functions
#3174022-2: call_user_func_array() and named arguments in PHP 8
#3156887-17: \Drupal\system\Plugin\ImageToolkit\GDToolkit needs to support \GdImage objects for PHP 8 compatibility
#3127141-86: Use PHPUnit 9 for PHP 7.4+, while keeping support for PHPUnit 8.4 in PHP 7.3

@hussainweb please include the full patch as well as the base patch - otherwise we're not identify new PHP 8 errors - which is the point of this issue.

Patch attached is #104 +

• #3156887-19: \Drupal\system\Plugin\ImageToolkit\GDToolkit needs to support \GdImage objects for PHP 8 compatibility
• #3174662-64: Encapsulate \PDOStatement instead of extending from it
• #3156542-27: \ReflectionParameter::getClass() is deprecated in PHP 8.0

Patch adds the new issues and fixes rest of tests.

• #3177541-2: stream_open() needs to cope with a failure in \Drupal\Core\StreamWrapper\LocalStream::getLocalPath() better
• #3177545-2: \Drupal\field\Entity\FieldStorageConfig::getCardinality() needs to be more type safe
• #3177546-2: \Drupal\views\Plugin\views\PluginBase::listLanguages() incorrectly uses in_array() resulting in listing additional languages

Now just the views filter thing to fix. That one looks funky. This includes #3177557-2: \Drupal\error_test\Controller\ErrorTestController::generateWarnings() notice is not a notice in PHP 8

Yep funky indeed. The current code is only working because of a != instead of a !==. I need to open another issue about this on it's own so we can discuss it further.

Rebased #111 on HEAD - a lot PHP 8 compatibility patches have been committed. I think we now need to open issues for the rest of the test stuff that's in this.

I'm waiting for the rtbc PHP 8.0 issues to be committed before re-rolling this. Patch will be smaller :)

This removes the doctrine upstream fix and moves us to the dev version of the selenium2 driver and applies #3174928: Improve the stability of core JS testing and prepare for update of MinkSelenium2Driver

@andypost why does it matter? this is all temporary.

I think the phpdocumentor stuff might have been fixed upstream - let's see. Let's ignore the composer test stuff for now... it's not relevant till we have stable packages to build against...

Rerolling now the last 2 core blockers are in... all we have now are external dependencies and #3176655: Deprecate GoutteDriver use in core and use Behat\Mink\Driver\BrowserKitDriver directly instead - which is an external dep but solved by getting rid of it.

There are change to MySQL that seems to be affecting us to do with the way transactions are handled. I'm pretty sure that the change is https://github.com/php/php-src/commit/990bb34891c83d12c5129fd781893704f9...

It's to do with the fact that MySQL doesn't support DDL statements in transactions. And if there is a DDL statement then it's going to auto-commit and close the transaction. We've had code to handle this forever but I think the change above no reports errors where before it didn't. So before we were trying to commit when there was no transaction but it didn't matter. Now it's throwing an exception.

Fun. Atm in HEAD our transactional processing is a little bit faked for MySQL when there is DDL statements.

Thanks for digging @andypost yep #2736777: MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction is exactly what we facing. It's a pretty major behaviour change in PHP's MySQL but it is one that makes sense.

Points to a better fix here too... I'm going to add and retitle #2736777: MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction to #3109885: [meta] Ensure compatibility of Drupal 9 with PHP 8.0 (as it evolves)

Updated with latest work on #2736777: MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction

Updating to the dev version of laminas-feed since that supports PHP 8 and removing unnecessary changes.

Updated with latest work on #2736777: MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction

Updated with latest work on #2736777: MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction

Updated with latest work on #2736777: MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction

Is in #2736777: MySQL on PHP 8 now errors when committing or rolling back when there is no active transaction... so a nice small patch here - that only concerns dependencies - yay.

Rerolled - fixed conflicts in composer.lock

The laminas releases are out and I worked out a way to fix \Behat\Mink\Driver\Selenium2Driver in \Drupal\FunctionalJavascriptTests\DrupalSelenium2Driver

So we should now be able to test without the ignore platform reqs...

We can fix the platform in the root composer.json. There's an issue for this. Now we've got the composer projects this feel completely okay to add now.

@andypost I think it is happening because DrupalCI is using Composer v1 and that is not PHP 8 compatible. it is using phar:///usr/local/bin/composer

@andypost nope I'm wrong... Running 2.0.3 (2020-10-28 15:50:55) with PHP 8.0.0RC3 on Linux / 4.9.0-0.bpo.6-amd64

The laminas dependency update has landed...

Opened #3183420: Override \Behat\Mink\Driver\Selenium2Driver::uploadFile() in DrupalSelenium2Driver and bumped #3156595: Make Drupal 9 installable on PHP8

@effulgentsia I'm pretty sure this about Composer 2 - and how we define path repos in \Drupal\BuildTests\Composer\Template\ComposerProjectTemplatesTest::makeVendorPackage()

In that method we define them with relative paths so it would not surprise me if upgrade composer 2 on Drupal CI did fix this.

That said I wonder if the patch attached fixes this.

Re setting platform php in composer - there is a much wider debate of the topic in #3098281: Ensure that 'composer update' evaluates dependencies using the correct PHP version - also an RTBC patch that fixes this - also it sets it to our minimum PHP and tests for this. I meant to link this in #141 but added a link to this issue - ho hum.

Patch attached is #3098281: Ensure that 'composer update' evaluates dependencies using the correct PHP version + #3183825: Use absolute instead of relative paths within the packages.json created in ComposerProjectTemplatesTest

I guess it might be worth explaining why we need #3183825: Use absolute instead of relative paths within the packages.json created in ComposerProjectTemplatesTest - this is because on PHP 8 our ROOT composer.lock does contain a set of installable packages on PHP 8 - as PHPUnit is locked v8. So we need to use the platform setting to run install as though we're on PHP 7. The ability to do this isn one of a number of reasons why I think constraining the maximum PHP version in composer.json is wrong. Another very important point to note is the root composer.json file is core development's alone. No site uses this. You are not supposed to start a site from checking out Drupal and if you composer will tell you are doing it wrong. All this and more is discussed on #3183825: Use absolute instead of relative paths within the packages.json created in ComposerProjectTemplatesTest.

There is another solution and one that we should consider for 9.2.x. We can lock to PHPUnit 9. PHPUnit 9 is compatible with Drupal 9's minimum PHP version and also is the first PHPUnit to get life support. The the root composer.lock would be compatible with both PHP 7.3 and PHP 8. I'm going to open an issue to discuss making this change. That said I think #3183825: Use absolute instead of relative paths within the packages.json created in ComposerProjectTemplatesTest is viable solution for 9.1 and has other benefits for the core dev community.

Going to mark this issue as fixed because Drupal 9.2.x is currently installable on PHP 8.0 and it's part of the automated test suite. Crediting everyone who commented on the issue for posterity.