Remove composer integration PHP version test (rely on DrupalCI PHP version runs instead)

Ubuntu 16.04's default PHP version is 7.0, which will be supported by Ubuntu until 2021. Therefore, do we need to support PHP 7.0 beyond 8.7's release? If so, would it be possible to retain a test that ensures PHP 7.0 compatibility of dependencies (e.g., via platform configuration, which would then implicitly pin Symfony to 3.x) in addition to a separate composer integration test for PHP 7.1.3+ / Symfony 4?

There's also #2992116: Bump core dependencies minimum version to PHP 7.0

Per #5, clarifying issue title :)

(I was coming here to say something like #4, until I read #5.)

We need some docs here as per #2. Thanks!

Composer install worked on PHP5 so all good here.

How about this instead? Just a docs change (and rename of the constant), but I'm curious if this captures the intent of this issue more clearly, or if I'm misunderstanding the intent.

Though if the docs in #10 are correct, then why don't we just leave 8.7.x as-is, and just raise the value of the constant in the patches that are updating to Symfony 4, and other D9-only library versions?

+++ b/core/tests/Drupal/Tests/ComposerIntegrationTest.php
@@ -12,13 +12,22 @@
+  const MIN_DRUPAL_9_PHP_VERSION = '7.1.3';

7.1 will be unsupported after 1 Dec 2019 so for that reason this name is incorrect. As per http://php.net/supported-versions.php min D9 PHP version could only be 7.2.

If we're already willing to decide that D9's minimum PHP version will be 7.2, then why change our composer integration test to 7.1.3 instead of 7.2?

I think setting up D9's minimum PHP version to an unsupported release is not a right thing to do so I'd say yes, we should set it to 7.2.

Allowing contrib test runs against Symfony 4/5 with 8.x would help everyone get ready for them being required.

That would indeed be very valuable!

If we've got #2976407: Use drupalci.yml for composer dependency min/max testing - DO NOT COMMIT in place can't we remove this test? I think @mile23 might already by testing our components using travis with min / max testing.

I was wondering why someone was poking at #2976407: Use drupalci.yml for composer dependency min/max testing - DO NOT COMMIT :-)

The reason for the test is to make sure that the lock file doesn't include dependencies that are outside our target PHP versions. That way d.o won't package a tarball that is incompatible.

We did this in lieu of adding a config:platform:php:5.6 to composer.json because it would mean that anyone (unfortunately) using drupal/drupal to install with Composer would be stuck with PHP 5.6.

So there are a bunch of solutions, here in order of real preference:

• Add real regression tests for dependencies using all the PHP versions Drupal is expected to run under.
• Quit using a lock file.
• Change the test for whatever new target minimum PHP is the real one.
• Remove the test and build the tarball on a PHP 5 platform and then file an issue when it breaks.
• Quit building tarballs.

The plan is that we're going to relegate drupal/drupal away from any use other than assembling the dependencies for a dev codebase. That's going to be real after we get #2982680: Add composer-ready project templates to Drupal core. I'm thinking we should remove this test when that happens, as long as we have dependency regression tests. We don't have that yet, so the test should stay.

The real problem with this test is that it runs after you've updated to the new PHPUnit version, so your lock file has a bunch of updated dependencies in it. So therefore, to get around it, mark it as skipped in your patch, or you can revert or delete composer.lock before running tests in your CI process.

The change here should reflect the minimum PHP version supported by 8.7.0. If we're doing Symfony 4 and that needs 7.1, then there's your answer.

Given the current PHP requirements page: https://www.drupal.org/docs/8/system-requirements/php-requirements

...It seems like we shouldn't need to guarantee PHP 5 for Drupal 8.7.0 release, just PHP 7.0.x.

So this means a couple of things:

For this issue, we should bump up the minimum version in the test. This isn't sufficient for Symfony 4, but it's the listed requirement for Drupal 8.7.0.

Out of scope here, we should remove all the PHP 5 scaffold code like the existence of drupal-phpunit-upgrade.

How I made this patch:

• I changed ComposerIntegrationTest::MIN_PHP_VERSION to 7.0.8.
• I set drupal/drupal's config:platform:php to 7.0.8.
• I said composer update
• I reverted drupal/drupal's composer.json (since we don't actually want that in the repo).
• I said composer update --lock so we don't have a content hash problem.

I arrived at PHP 7.0.8 because that's the minimum for symfony/http-foundation, according to messages like this:

  Problem 1
    - This package requires php ^5.5.9|>=7.0.8 but your PHP version (7.0.0; Package overridden via config.platform (actual: 7.2.8)) does not satisfy that requirement.
  Problem 2
    - symfony/http-foundation v3.4.15 requires php ^5.5.9|>=7.0.8 -> your PHP version (7.2.8) overridden by "config.platform.php" version (7.0.0) does not satisfy that requirement.
[ .. etc .. ]

Anyone who's using PHP 5 can then use composer update and pray, or remove the lock file and use install. Theoretically, we could have tests to see whether we actually support PHP 5, but then we'd be supporting PHP 5.

And, most preferably of all, hopefully we'll get #2982680: Add composer-ready project templates to Drupal core before 8.7.0 release and people won't be using our lock file.

Discussed with @catch and I agree with #20; I think it's okay to either update the test to increase the version number, or to remove the test entirely. We can just make it part of our process to run tests on all PHP versions prior to commit for dependency update issues, the same way that we run tests on all databases when we make changes to the Views or database APIs. And the sooner we unblock #2976394: Allow Symfony 4.4 to be installed in Drupal 8, the better.

We can't do #19 quite yet though. :) The plan is to keep all branches (including 8.7.x) 100% PHP 5-compliant until 8.7.0, in order to keep backports safe, and thereafter to avoid PHP 5 regressions in 8.7.x and earlier so that sites can still update to security releases even if they're on PHP 5. Only 8.8.x should update to non-PHP-5-safe dependency versions. See #2842431-176: [policy] Remove PHP 5.5, 5.6 support in Drupal 8.7 for specifics.

The plan is to keep all branches (including 8.7.x) 100% PHP 5-compliant until 8.7.0

OK, so because of the way we build the tarball that means leaving the test in until just after the 8.7.0 release, or really until the planned stop of support for PHP 5. Which is a little unclear, by the way. :-)

The test is supposed to be annoying. It is literally the only automated test that says we support PHP 5.

If anyone wants to RTBC #22 I won't object, tho, because I don't want to be as annoying as that test.

Like I said: If someone wants to RTBC that's fine and go for it. Core 8.7.x currently has a PHP 5 build on commit in DrupalCI.

Whilst we support PHP5 we need to be running at least one test run against it. We are so I'm not sure what this test really buys us. So +1 to removing this. Once we have #2976407: Use drupalci.yml for composer dependency min/max testing - DO NOT COMMIT this test is moot anyways.

Crediting @xjm and @alexpott for their review of #20 / #22.

Removing credit from myself, as my earlier work is moot with the new direction.

Pushed to 8.7.x.