Prevent Drupal 8.9 and 9.0 from being installed on PHP 8

Agree withe the direction here. Well articulated reasons in the issue summary. Let's get this in.

I’m not convinced about the direction here. We weren’t compatible with PHP7.4 but we allowed installation there.

As PHP 8.0 is not released yet - it's not an issue but making it strict will force people to always use to ignore ALL platform requirements... Which is far more then BC break

Interesting point about composer resolver - is there any idea how to measure effect of it?

We've never done this in the history of Drupal before. It's never caused issues - adding this only makes core developers lives harder and more for people to know. As @andypost points out teaching people about ignoring all platform requirements is way more of a BC break. And doing this will make it harder for us to convince our dependencies from doing the same. There is no reason to do this. PHP 8 is not out yet. It's not stable and this is not causing any problems.

I think the discussion here is awesome and civil and exactly why Drupal is such a model in the open-source world. I would chime in only with a narrow point about semantics, but I think it has a practical application:

We weren’t compatible with PHP7.4 but we allowed installation there.

"Allowed" is a good term to focus on here; with open-source software, we can't, don't and won't control how people use it. (In another context, there's good discussion about sites being run on Drupal which may run counter to the values of contributors, but that's how it goes with a permissive license, and isn't reason not to work on the software.)

That is the pedantic point; to me, this is really a question about the version constraints (upper and lower) reflecting a statement about the software's known stability and support window. We support PHP down to 7.0.8, by reason of supporting Ubuntu LTS (and other reasons articulated elsewhere.) As others point out, there's nothing stopping someone who knows what they are doing from running Drupal in all sorts of unintended environments. Adding < 8.0 communicates nothing more than "we do not vouch for this being stable on 8.0.

Furthermore, I'd point out that --ignore-platform-reqs is not the only option for expressing an installation context different from the local runtime. In our containerized workflow at Fruition, often you may run composer "locally" and run inside a Docker container running a different minor version of PHP. In that case, the platform overrides section allows a great way to articulate the environment. In this same way, you could "fake" a supported version of PHP while not overriding ALL platform requirements. Very similar, but worth a mention.

Great points about wording @bradjones1. I also pondered what word to use in my #3151695: Dissuade users from editing .htaccess issue, and ended up with "dissuade": https://www.merriam-webster.com/dictionary/dissuade

So 29 dependencies cause Drupal core not to be installable on PHP 8 now. I added a table to the issue summary at #3109885: [meta] Ensure compatibility of Drupal 9 with PHP 8.0 (as it evolves). It would take all of those to be resolved to reach this problem as far as I see. There are currently 4 Drupal core issues that make us incompatible, but one of them is RTBC and one of them is only practically testable once the others land. I think we will only need to revisit this once/if we reach a point where we updated all our dependencies to be compatible but we did not get to fix our own issues. Based on the pace of our local issues going fast vs. our external issues being stuck in some cases, I am not sure we will get into that situation.

Meant to mark postponed with #17.

Discussed with @catch and @xjm and we agreed that we should mark Drupal 8 as not compatible with PHP 8 at this point. We might return to try to change the constraints in composer.json to allow it but at the moment that's going to be a lot of work due to dependencies. (For example, easyrdf). Drupal 9 will be PHP 8 compatible - Drupal 9.1 mostly is and has a chance of being by release so we should leave that alone.

I think also we should mark Drupal 9.0 as not compatible with PHP 8.

Looks good, composer fails to build the codebase with PHP 8.

+1 RTBC

We also need to write a core blog post to announce this... adding a section to the issue summary for this task.

Retitling because there is no "until it is supported" for 8.9 and 9.0. The priority for those branches following 9.1.0 is absolute stability and security support.

Composer failing to build the codebase isn't sufficient. People still use tarballs. We need to manually test what happens with this patch when installing or updating Drupal from a previous version of Drupal, both will a composer site and will a tarball, and ensure that end users receive the correct messages.

@xjm asked us in #bugsmash to test this with tarballs. I pointed out that if you're using tarballs, there's no way this patch can stop you from installing on PHP 8, since it's only touching composer*, and the installer nor update.php or anything of the sort checks with composer.lock for you.

If we want this to happen for the tarball case, we need to expand the PHP version checking that's already happening in system_requirements()

Thanks @dww!

I think this should be an error on install and a warning on update, similar to what we did in the past as we were deprecating PHP 5. (Because running on a PHP version that might have fatal errors is preferable to not getting Drupal security updates.)

Here's a mostly untested version of the hook_requirements() changes. I don't have PHP 8 locally, and don't feel like jumping through those hoops right now. ;) So I temporarily hacked my copy of this patch to pretend 7.4 was too high, and lo, the status report warned me. ;)

We'd obviously want to update our installer and status reports tests for this, but I just wanted to upload something to get us started.

I added @todo about adding constants to includes/bootstrap.inc for this. Seems tricky and wrong to hard-code a "maximum supported version". Not sure what to do with it. Probably we don't want this as-is, but it's a start for discussion.

Based on more Slack discussion, here's some updated wording for the requirements warning/error:

t('Your PHP installation is too new. Drupal %drupal_version only supports up to PHP %php_max_version. Upgrade to Drupal 9.1 (scheduled release December 2020) or use PHP %php_max_version. See <a href="http://php.net/supported-versions.php">PHP\'s version support documentation</a> and the <a href=":php_requirements">Drupal 8 PHP requirements handbook page</a> for more information.',
[
  '%drupal_version' => \Drupal::VERSION,
  '%php_max_version' => '7.4',
  ':php_requirements' => 'https://www.drupal.org/docs/8/system-requirements/php',
]
);

Caveat. I have not read all the comments. I'm doing some preliminary testing based on a thread in the bugsmash Slack channel. So far I have tested Drupal 8.9.7 WITHOUT any patch. Before I continue to test, maybe someone can let me know if these steps make sense or not. Thanks.

1) Set version to PHP 7.4.12

2) Followed quick-start instructions on https://www.drupal.org/node/2969396 for Drupal 8.9.7

3) Go to status report page

4) Only error is "Trusted Host Settings Not enabled" error

5) Switch to PHP 8.0

6) Remove sites/default/files and sites/default/settings.php

7) Run quick-start again

8) Get tons of errors during install, e.g.

PHP Fatal error:  Declaration of Doctrine\Common\Reflection\StaticReflectionClass::getConstants() must be compatible with ReflectionClass::getConstants(?int $filter = null) in /Users/admin/Sites/drupal/drupal8/drupal-8.9.7/vendor/doctrine/common/lib/Doctrine/Common/Reflection/StaticReflectionClass.php on line 109

Fatal error: Declaration of Doctrine\Common\Reflection\StaticReflectionClass::getConstants() must be compatible with ReflectionClass::getConstants(?int $filter = null) in /Users/admin/Sites/drupal/drupal8/drupal-8.9.7/vendor/doctrine/common/lib/Doctrine/Common/Reflection/StaticReflectionClass.php on line 109

9) Double check php version

KristenBackupMBP:drupal-8.9.7 admin$ php -v
PHP 8.0.0-dev (cli) (built: Oct 29 2020 06:00:36) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.0-dev, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.0-dev, Copyright (c), by Zend Technologies
KristenBackupMBP:drupal-8.9.7 admin$ php --ini
Configuration File (php.ini) Path: /usr/local/etc/php/8.0
Loaded Configuration File:         /usr/local/etc/php/8.0/php.ini
Scan for additional .ini files in: /usr/local/etc/php/8.0/conf.d
Additional .ini files parsed:      /usr/local/etc/php/8.0/conf.d/ext-opcache.ini,
/usr/local/etc/php/8.0/conf.d/jit.ini

10) Switch back to PHP 7.4.12

11) Remove sites/default/files and sites/default/settings.php

12) Reinstall via quick-start

13) Stop web server (Ctrl-C)

14) Switch to PHP 8.0

15) Restart web server (php core/scripts/drupal quick-start)

16) Same site comes up and some pages are okay but there are fatal errors on other pages, e.g.

 Fatal error: Declaration of Doctrine\Common\Reflection\StaticReflectionClass::getConstants() must be compatible with ReflectionClass::getConstants(?int $filter = null) in /Users/admin/Sites/drupal/drupal8/drupal-8.9.7/vendor/doctrine/common/lib/Doctrine/Common/Reflection/StaticReflectionClass.php on line 109

I had copy/pasted surrounding code for this. @xjm pointed out the handbook page is now "evergreen" and has a different URL. So, this fixes the new string, but leaves the old string as-is since:
a) it'd be a string break to change it
b) there are redirects on d.o for this.
No follow-up needed, since 9.1.x already fixed this string. - Whoops, guess not. #NeedsFollowup

@dww and I discussed the possibility of adding test coverage for this issue. Given that these changes will be added only to 8.9 and 9.0, and given that we want to backport this within the next four days, I think manually testing the matrix of install/update; tarball/composer will be sufficient for this issue. We can add more generalized automated tests in #3103255: Add upper PHP constraint to composer.json and/or the installer if that issue goes forward.

I got the green light in Slack to test the patch, but I needed to use 8.9 dev so here's the new streamlined steps. Unfortunately, the patch didn't help. But, for some reason, the dev version doesn't show the fatal errors on the pages like 8.9.7 did (though they are in the logs).

Without patch:

1) Did quick-start install with PHP 7.4.12

2) No errors in the logs

3) Closed PHP web server (Ctrl-C)

4) Switched to PHP 8.0.0

5) Restarted PHP web server (same install)

6) Did not see fatal errors in the browser, but they were in the logs, e.g.

Deprecated function: Method ReflectionParameter::getClass() is deprecated in Drupal\Component\Utility\ArgumentsResolver->getArgument() (line 72 of /Users/admin/Sites/drupal/drupal8/drupal-8.9.x-dev/core/lib/Drupal/Component/Utility/ArgumentsResolver.php) 

7) Stopped PHP web server and removed sites/default/files and sites/default/settings.php

8) Did quick-start install from scratch with PHP 8.0.0

9) Shows errors:

KristenBackupMBP:drupal-8.9.x-dev admin$ php -v
PHP 8.0.0-dev (cli) (built: Oct 29 2020 06:00:36) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.0-dev, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.0-dev, Copyright (c), by Zend Technologies
KristenBackupMBP:drupal-8.9.x-dev admin$ php --ini
Configuration File (php.ini) Path: /usr/local/etc/php/8.0
Loaded Configuration File:         /usr/local/etc/php/8.0/php.ini
Scan for additional .ini files in: /usr/local/etc/php/8.0/conf.d
Additional .ini files parsed:      /usr/local/etc/php/8.0/conf.d/ext-opcache.ini,
/usr/local/etc/php/8.0/conf.d/jit.ini

KristenBackupMBP:drupal-8.9.x-dev admin$ php core/scripts/drupal quick-start

 Select an installation profile [Install with commonly used features pre-configured.]:
  [standard  ] Install with commonly used features pre-configured.
  [minimal   ] Build a custom site without pre-configured functionality. Suitable for advanced users.
  [demo_umami] Install an example site that shows off some of Drupal’s capabilities.
 > 

<em class="placeholder">Deprecated function</em>: Method ReflectionParameter::getClass() is deprecated in <em class="placeholder">Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass-&gt;processServiceCollectorPass()</em> (line <em class="placeholder">133</em> of <em class="placeholder">/Users/admin/Sites/drupal/drupal8/drupal-8.9.x-dev/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php</em>). <pre class="backtrace">Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass-&gt;processServiceCollectorPass(Array, &#039;cache_tags.invalidator&#039;, Object) (Line: 97)

Follow the steps above with patch from #31:

Same behavior as above when updating PHP 7.4.12 => 8.0.0 on existing site including errors in logs like:

Deprecated function: Method ReflectionParameter::getClass() is deprecated in Drupal\Component\Utility\ArgumentsResolver->getArgument() (line 72 of /Users/admin/Sites/drupal/drupal8/drupal-8.9.x-dev/core/lib/Drupal/Component/Utility/ArgumentsResolver.php) 

For new quick-start, also get errors right away:

KristenBackupMBP:drupal-8.9.x-dev admin$ php -v
PHP 8.0.0-dev (cli) (built: Oct 29 2020 06:00:36) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.0-dev, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.0-dev, Copyright (c), by Zend Technologies
KristenBackupMBP:drupal-8.9.x-dev admin$ php --ini
Configuration File (php.ini) Path: /usr/local/etc/php/8.0
Loaded Configuration File:         /usr/local/etc/php/8.0/php.ini
Scan for additional .ini files in: /usr/local/etc/php/8.0/conf.d
Additional .ini files parsed:      /usr/local/etc/php/8.0/conf.d/ext-opcache.ini,
/usr/local/etc/php/8.0/conf.d/jit.ini

KristenBackupMBP:drupal-8.9.x-dev admin$ php core/scripts/drupal quick-start

 Select an installation profile [Install with commonly used features pre-configured.]:
  [standard  ] Install with commonly used features pre-configured.
  [minimal   ] Build a custom site without pre-configured functionality. Suitable for advanced users.
  [demo_umami] Install an example site that shows off some of Drupal’s capabilities.
 > 

<em class="placeholder">Deprecated function</em>: Method ReflectionParameter::getClass() is deprecated in <em class="placeholder">Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass-&gt;processServiceCollectorPass()</em> (line <em class="placeholder">133</em> of <em class="placeholder">/Users/admin/Sites/drupal/drupal8/drupal-8.9.x-dev/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php</em>). <pre class="backtrace">Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass-&gt;processServiceCollectorPass(Array, &#039;cache_tags.invalidator&#039;, Object) (Line: 97)

@Kristen Pol re: #33 Thanks for testing! I was afraid install.php would blow up too soon. There's already code in there to test for the minimum PHP version and bail very early. So here's a new patch that expands that check to also bail super early if we're on PHP 8. Does this help?

Also, based on yet more Slackery w/ @xjm, we decided that '9.1' also deserves a non-translatable placeholder in the new warning. ;)

Hmm... tried #34 with PHP 8.0.0 only and didn't see any difference:

KristenBackupMBP:drupal-8.9.x-dev admin$ php core/scripts/drupal quick-start

 Select an installation profile [Install with commonly used features pre-configured.]:
  [standard  ] Install with commonly used features pre-configured.
  [minimal   ] Build a custom site without pre-configured functionality. Suitable for advanced users.
  [demo_umami] Install an example site that shows off some of Drupal’s capabilities.
 > 

<em class="placeholder">Deprecated function</em>: Method ReflectionParameter::getClass() is deprecated in <em class="placeholder">Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass-&gt;processServiceCollectorPass()</em> (line <em class="placeholder">133</em> of <em class="placeholder">/Users/admin/Sites/drupal/drupal8/drupal-8.9.x-dev/core/lib/Drupal/Core/DependencyInjection/Compiler/TaggedHandlersPass.php</em>). <pre class="backtrace">Drupal\Core\DependencyInjection\Compiler\TaggedHandlersPass-&gt;processServiceCollectorPass(Array, &#039;cache_tags.invalidator&#039;, Object) (Line: 97)

When I git diff, I see the changes in the install.php file:

diff --git a/core/install.php b/core/install.php
index 8cc3880c81..cdee501024 100644
--- a/core/install.php
+++ b/core/install.php
@@ -29,6 +29,10 @@
   print 'Your PHP installation is too old. Drupal requires at least PHP 7.0.8. See the <a href="https://www.drupal.org/requirements">system requirements</a> page for more information.';
   exit;
 }
+elseif (version_compare(PHP_VERSION, '8.0') >= 0) {
+  print 'Your PHP installation is too new. Drupal 8.9 only supports up to PHP 7.4. Upgrade to Drupal 9.1 (scheduled release December 2020) or use PHP 7.4. See the <a href="https://www.drupal.org/docs/system-requirements/php-requirements">Drupal PHP requirements handbook page</a> for more information.';
+  exit;
+}

Ooof, yeah. That's because quick-start doesn't use install.php. ;) Per @xjm, spun that out to #3179787: QuickStart and Install commands don't enforce min (or max) PHP versions.

Meanwhile, here's #34 with just the changes to composer* and install.php. Seems that Drupal 8.9 dies too early with PHP 8 for the system_requirements() check to even matter.

XPOST with @dww...

#34 will not work with quickstart. In order to work with quickstart this check needs to be inside install_drupal(). We should probably move both PHP version checks there. The minimum version was here because when it was PHP 5.whatever it needed to be here in order to not error on the autoloader - but we don't care about PHP 5 anymore - right?

+++ b/core/modules/system/system.install
@@ -240,6 +241,26 @@ function system_requirements($phase) {
+  // @todo In https://www.drupal.org/project/drupal/issues/3103255 consider
+  //   converting this to a constant and a more general solution.
+  elseif (version_compare($phpversion, '8.0.0') >= 0) {
+    // Note: The URL for the PHP documentation is included as part of the
+    // translatable text, since PHP has documentation in various
+    // languages. However, the URL to the drupal.org handbook page is a
+    // placeholder, since we only have English versions of Drupal documentation,
+    // and we don't want translations to change where this link is pointing.
+    $requirements['php']['description'] = t('Your PHP installation is too new. Drupal %drupal_version only supports up to PHP %php_max_version. Upgrade to Drupal %drupal_php8_first_version (scheduled release December 2020) or use PHP %php_max_version. See <a href="http://php.net/supported-versions.php">PHP\'s version support documentation</a> and the <a href=":php_requirements">Drupal PHP requirements handbook page</a> for more information.',
+      [
+        '%drupal_version' => \Drupal::VERSION,
+        '%drupal_php8_first_version' => '9.1',
+        '%php_max_version' => '7.4',
+        ':php_requirements' => 'https://www.drupal.org/docs/system-requirements/php-requirements',
+      ]
+    );
+    $requirements['php']['severity'] = $phase === 'install' ? REQUIREMENT_ERROR : REQUIREMENT_WARNING;
+  }

This is will never ever be executed. Because there is no way of getting to this point without breaking Drupal. If you're installed already or even if you're not.

This is why we not do this in Drupal code and leave it up to composer. Composer checks PHP versions that's one of its responsibilities. Obviously that'd be easier to actually make policy if we didn't still have the tarballs.

I tested using ddev and PHP 8 with /core/install.php and see the message below. Note: if you want to test with ddev and you aren't using the edge version, you will need to completely uninstall ddev and then install the edge version that supports PHP 8: https://github.com/drud/ddev/releases/tag/v1.16.0-alpha8

Your PHP installation is too new. Drupal 8.9 only supports up to PHP 7.4. Upgrade to Drupal 9.1 (scheduled release December 2020) or use PHP 7.4. See the Drupal PHP requirements handbook page for more information.

I get a fatal error if not going to /core/install.php.

 Fatal error: Declaration of Drupal\Core\Database\Statement::fetchAll($mode = null, $column_index = null, $constructor_arguments = null) must be compatible with PDOStatement::fetchAll(int $mode = PDO::FETCH_BOTH, mixed ...$args) in /var/www/html/core/lib/Drupal/Core/Database/Statement.php on line 169

Was #37 a crosspost? It seems to be a review of a previous patch. The patch in #36 is on track for what we need, I think.

One thing that is one with #36 is that on Chrome the text appears all messed up...

Adding P tags around the message fixes it. I have no idea why this occurs and if it something to do with browser plugins or behaviour other chrome users would experience.

Re #40 - yeah this is due to a chrome extension - "JSON Viewer" ... this must be happening because we're really sending a proper response at all - i.e. telling the browser what type of content it is or doing any of the normal things.

If we want to present proper content to the browser then doing something like

  if (version_compare(PHP_VERSION, '8.0') >= 0) {
    if (PHP_SAPI !== 'cli') {
      $response = new Response(
        'Your PHP installation is too new. Drupal 8.9 only supports up to PHP 7.4. Upgrade to Drupal 9.1 (scheduled release December 2020) or use PHP 7.4. See the <a href="https://www.drupal.org/docs/system-requirements/php-requirements">Drupal PHP requirements handbook page</a> for more information.',
        500
      );
      $response->send();
    }
    else {
      print 'Your PHP installation is too new. Drupal 8.9 only supports up to PHP 7.4. Upgrade to Drupal 9.1 (scheduled release December 2020) or use PHP 7.4. See the Drupal PHP requirements handbook page for more information - https://www.drupal.org/docs/system-requirements/php-requirements.';
    }
    exit(1);
  }

Would work but we need to do this after the autoloader so the Symfony response object is available. Not sure that this is worth it.

We could also just leave out the link, or inline it without markup. The critical information is in the message.

I was debating whether we should add a status report warning for an already installed site, for when an existing siteʻs hosting is upgraded to PHP 8, but given that a simple fetchAll() for a DB query fatals in @Kristen Polʻs testing, theyʻll never see such a message anyway and itʻd be really hard not to notice immediately that the site is hosed.

I think it's absolutely fine to drop the link. Overall I can't see that many people reaching this situation - will take a while for hosting and distributions to catch up (would be a different matter if 9.1 wasn't de-facto compatible and we had to do it there too).

Given that my broken link stuff was down to an extension that not everyone has I think it's okay to keep it too. Also we can always remove it later it was decide to prioritise CLI messaging (due to moving this to somewhere that will benefit quickstart.

I've tested this on PHP 8 and PHP 7 and it works as expected.

Here's the patch for 9.0.x - added the text from #36 to the patch from #20

lol should have read said text because it mentions the Drupal version. Note we can't use \Drupal::VERSION here because this is prior to the autoloader.

Oh and there's a load more work to do on the release note / change record etc... which needs to be in place prior to the rtbc.

So, based on the pushback on #3180207: Update laminas-diactoros, laminas-escaper and laminas-feed for PHP 8 compatibility for using a dev version of Laminas packages, it is starting to look again like 9.1 won't fully support PHP 8 either. Given that, we might need to say 9.2 instead of 9.1? Or at least add some more nuance to the message. Composer users will never get far enough to see this message and it's been suggested that tarballs may run on PHP 8 already, but if it's not "official" support we would need to indicate that somehow. (That would be another thing to manually test with the latest 9.1.x-dev tarball, also.)

Maybe:

Update to the latest release of Drupal 9 for improved PHP 8 support, or use PHP 7.4. See the Drupal PHP requirements handbook page for more information: https://www.drupal.org/docs/system-requirements/php-requirements

And we create a final non-security release of 9.0 on the day we ship 9.1 (rather than yesterday which would normally have been the last 9.0 bugfix release).

+1 to #49 ... I kept the html link due to #44 but it's nice to have the same message in 8.9. and 9.0.

I added a change record - https://www.drupal.org/node/3180764

I added a release note to the issue summary - I'm not sure about the text for the core blog post - looking at the change record https://www.drupal.org/node/3180764 - won;'t that do as a basis?

+++ b/core/install.php
@@ -29,6 +29,10 @@
+elseif (version_compare(PHP_VERSION, '8.0') >= 0) {

Just wondering why not - elseif (version_compare(PHP_VERSION, '8.0', '>=')) which looks more readable

@andypost I guess this is c&p from

if (version_compare(PHP_VERSION, '7.3.0') < 0) {
  print 'Your PHP installation is too old. Drupal requires at least PHP 7.3.0. See the <a href="https://www.drupal.org/requirements">system requirements</a> page for more information.';
  exit;
}

Changed because I think you're right

Back to RTBC

+++ b/core/install.php
@@ -30,7 +30,7 @@
-  print 'Your PHP installation is too new. Drupal 9.0 only supports up to PHP 7.4. Upgrade to Drupal 9.1 (scheduled release December 2020) or use PHP 7.4. See the <a href="https://www.drupal.org/docs/system-requirements/php-requirements">Drupal PHP requirements handbook page</a> for more information.';
+  print 'Update to the latest release of Drupal 9 for improved PHP 8 support, or use PHP 7.4. See the <a href="https://www.drupal.org/requirements">system requirements</a> page for more information.';

New message looks laconic

Committed/pushed the respective patches to 9.0.x and 8.9.x, thanks!