Update Symfony components to ~2.8.16

Given those are also on the branch tests, moving this to RTBC.

We can discuss 8.2.x backport once it's in. In theory it's a good idea but there are a couple of indirect library changes that aren't patch level changes (polyfill and browser).

I went through every change in the various components, stumbled upon the getTrustedHost() changes in the Request object, but they are converted to a 400 http error, so not an actual unhandled exception.

Another change I struggled a bit with was:

diff --git a/src/Symfony/Component/Serializer/Serializer.php b/src/Symfony/Component/Serializer/Serializer.php
index 5994355..0259bfe 100644
--- a/src/Symfony/Component/Serializer/Serializer.php
+++ b/src/Symfony/Component/Serializer/Serializer.php
@@ -250,15 +250,6 @@ class Serializer implements SerializerInterface, NormalizerInterface, Denormaliz
             return $normalizer->denormalize($data, $class, $format, $context);
         }

-        foreach ($this->normalizers as $normalizer) {
-            if (
-                $normalizer instanceof DenormalizerInterface &&
-                $normalizer->supportsDenormalization($data, $class, $format)
-            ) {
-                return $normalizer->denormalize($data, $class, $format, $context);
-            }
-        }
-

Turns out the call to getDenormalizer in

    private function denormalizeObject($data, $class, $format, array $context = array())
    {
        if (!$this->normalizers) {
            throw new LogicException('You must register at least one normalizer to be able to denormalize objects.');
        }

        if ($normalizer = $this->getDenormalizer($data, $class, $format)) {
            return $normalizer->denormalize($data, $class, $format, $context);
        }

does the same already, so this particular change is fine.

The change in

diff --git a/src/Symfony/Component/Validator/Constraint.php b/src/Symfony/Component/Validator/Constraint.php
index 61def98..fc5288d 100644
--- a/src/Symfony/Component/Validator/Constraint.php
+++ b/src/Symfony/Component/Validator/Constraint.php
@@ -129,6 +129,9 @@ abstract class Constraint
             unset($options['value']);
         }

+        if (is_array($options)) {
+            reset($options);
+        }

seems like it could cause issues. Is this just me?

@dawehner The reset() looks unnecessary to me rather than potentially harmful - anything specific as to why it could cause problems?

@dawehner The reset() looks unnecessary to me rather than potentially harmful - anything specific as to why it could cause problems?

Oh I somehow read something like $options = reset($options);. I doubt this line causes any harm

RTBC +1

Moving this back to 8.2.x, but we should only even consider committing this there if we have a PHP 7.1 environment consistently green IMO, if not waiting for 8.3.x isn't the worst thing in the world.

Arghh should've given dawehner commit credit for the line-by-line review, added issue credit at least.

Also moving version back.

One reason why it we should maybe better add it to 8.2 is the potential incident of a security release of symfony 2.8.x. We want to make the transition as smooth as possible in the future, so just having the security update instead of also the other patch level changes could be helpful.

Rolled back for now, this may have re-introduced
https://www.drupal.org/pift-ci-job/567249

Pretty sure it was that commit - branch tests failed immediately following.

Another plug for #2825358: Event class constants for event names can cause fatal errors when a module is installed then...

We discussed several alternatives to this in irc:

- implementing our own APCu classloader, unfortunately this would also mean re-implementin wincache, xcache support etc.

- adding a chained class loader that encapsulates the cached and fallback classloader, so that $this->classloader is still 100% accurate. This adds both code and runtime complexity to do about the same thing as the change here - we'd be re-implementing PHP's support for multiple classloaders really.

- an upstream Symfony PR to make the negative caching configurable - might be worth it, this change, in a patch release no less, makes it not really useable for projects that allow the UI to determine what code gets loaded.

We should probably open a follow-up to discuss some of these further, but I think the patch @alexpott posted is the minimal impact change we can do.

...
- Installing symfony/routing (v2.8.4) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony-cmf/routing (1.4.0) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/polyfill-apcu (v1.1.1) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/class-loader (v2.8.4) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/console (v2.8.4) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/dependency-injection (v2.8.4) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/polyfill-iconv (v1.1.1) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/process (v2.8.4) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/psr-http-message-bridge (v0.2) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/serializer (v2.8.4) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/translation (v2.8.4) Loading from cache
> Drupal\Core\Composer\Composer::vendorTestCodeCleanup
  - Installing symfony/validator (v2.8.4) Loading from cache
...
Generating autoload files
> Drupal\Core\Composer\Composer::preAutoloadDump
> Drupal\Core\Composer\Composer::ensureHtaccess
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
Nothing to install or update
Generating autoload files
> Drupal\Core\Composer\Composer::preAutoloadDump
> Drupal\Core\Composer\Composer::ensureHtaccess

Testbot didn't install composer dependencies from composer.lock (https://dispatcher.drupalci.org/job/default/295565/consoleText). Same issue in https://www.drupal.org/node/2572605

Committed/pushed to 8.3.x, thanks!

Back to 8.2.x again.

Re-arranged the steps to patch first, then composer: #2842443: Patches should be applied before running composer., drupalci should pick up those changes now.

I talked with alex about the browser-kit version number. I think updating is fine, especially we don't have a direct dependency, which means it is just use in tests.

OK committed both patches to 8.3.x and 8.2.x respectively.

We need to keep an eye out for unexpected regressions in 8.2.x, but otherwise this should mean full PHP 7.1 support in the next patch release of core.

Looks great, thank you!

Committed/pushed both versions to both versions, thanks!

maybe file CR for 8.2?

Created #2859772: Update Symfony components to ~2.8.18.