Problem/Motivation

#2712647: Update Symfony components to ~3.2 and #2871253: Update Symfony components to 3.2.8 updated the Symfony components to 3.2.* but with 3.3.0 due out in May 2017 we should make sure Drupal 8.4.0 can ship with this.

Proposed resolution

Update composer.json once 3.3.0 has been tagged.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

timmillwood created an issue. See original summary.

mpdonadio’s picture

Status: Active » Needs review
FileSize
33.17 KB

Let's see if anything breaks with 3.3-beta1.

jibran’s picture

Nice one @mpdonadio.

timmillwood’s picture

Thanks for adding the beta patch. Hopefully we can keep on the ball with the symfony updates and move to 4.* in November ready for 8.5.x. Although that would mean shifting Drupal to depend on php7

Status: Needs review » Needs work

The last submitted patch, 2: 2874909-02.patch, failed testing.

mpdonadio’s picture

Status: Needs work » Needs review
FileSize
35.11 KB
1.94 KB

This fixes the proxy fail, and updates some related deprecations in 3.3. The larger problem is Request::setTrustedHeaderName() is deprecated now in favor of the X-Forwarded-* headers or the new RFC7239 Forwarded header, instead of allowing you to specify your own header names. Not sure how we handle BC with those settings in the long run.

I don't know enough about the low level container details to know what is really happening in those fails, or why just those two tests fail and everything doesn't implode. Nothing in http://symfony.com/blog/symfony-3-3-0-beta1-released jumped out at me.

Status: Needs review » Needs work

The last submitted patch, 6: 2874909-06.patch, failed testing.

pritish.kumar’s picture

Status: Needs work » Needs review
FileSize
35.11 KB
639 bytes

Fixed the coding standard issue in ReverseProxyMiddleware.php

Status: Needs review » Needs work

The last submitted patch, 8: 2874909-08.patch, failed testing.

mpdonadio’s picture

Status: Needs work » Needs review
FileSize
68.13 KB

Reroll b/c #2488860: Bring phpunit bridge into drupal and use it for unit tests and simpletest to handle Deprecation.

#2871253: Update Symfony components to 3.2.8 will also likely require a reroll when it lands.

Tagging out on this for the time being; the container fails are over my head.

jibran’s picture

Status: Needs review » Needs work

I think you forgot to rebase the local branch to 8.4.x.

mpdonadio’s picture

Status: Needs work » Needs review
FileSize
36.94 KB
43.44 KB

Sigh. That was my failed attempt to do a simple rebase and forgetting to fresh start the branch I was in.

This is the fresh start. But, upon watching composer run and closer examination of composer.lock, I noticed that specifying this way isn't pulling in the 3.3-beta1 versions of Symfony dependencies that aren't explicit in the core/composer.json (symfony/debug, symfony/dom-crawler, and symfony/browser-kit). I am not sure if that matters with these:

$ composer show -i | grep symfony
symfony-cmf/routing 1.4.0 Extends the Symfony2 routing component for dynami...
symfony/browser-kit v3.2.8 Symfony BrowserKit Component
symfony/class-loader v3.3.0-BETA1 Symfony ClassLoader Component
symfony/console v3.3.0-BETA1 Symfony Console Component
symfony/css-selector v3.3.0-BETA1 Symfony CssSelector Component
symfony/debug v3.2.8 Symfony Debug Component
symfony/dependency-injection v3.3.0-BETA1 Symfony DependencyInjection Component
symfony/dom-crawler v3.2.8 Symfony DomCrawler Component
symfony/event-dispatcher v3.3.0-BETA1 Symfony EventDispatcher Component
symfony/http-foundation v3.3.0-BETA1 Symfony HttpFoundation Component
symfony/http-kernel v3.3.0-BETA1 Symfony HttpKernel Component
symfony/phpunit-bridge v3.3.0-BETA1 Symfony PHPUnit Bridge
symfony/polyfill-iconv v1.3.0 Symfony polyfill for the Iconv extension
symfony/polyfill-mbstring v1.3.0 Symfony polyfill for the Mbstring extension
symfony/process v3.3.0-BETA1 Symfony Process Component
symfony/psr-http-message-bridge v1.0.0 PSR HTTP message bridge
symfony/routing v3.3.0-BETA1 Symfony Routing Component
symfony/serializer v3.3.0-BETA1 Symfony Serializer Component
symfony/translation v3.3.0-BETA1 Symfony Translation Component
symfony/validator v3.3.0-BETA1 Symfony Validator Component
symfony/yaml v3.3.0-BETA1 Symfony Yaml Component

Attached is a B patch, where those are added to force those to beta for the time being:

$ composer show -i | grep symfony
symfony-cmf/routing 1.4.0 Extends the Symfony2 routing component for dynami...
symfony/browser-kit v3.3.0-BETA1 Symfony BrowserKit Component
symfony/class-loader v3.3.0-BETA1 Symfony ClassLoader Component
symfony/console v3.3.0-BETA1 Symfony Console Component
symfony/css-selector v3.3.0-BETA1 Symfony CssSelector Component
symfony/debug v3.3.0-BETA1 Symfony Debug Component
symfony/dependency-injection v3.3.0-BETA1 Symfony DependencyInjection Component
symfony/dom-crawler v3.3.0-BETA1 Symfony DomCrawler Component
symfony/event-dispatcher v3.3.0-BETA1 Symfony EventDispatcher Component
symfony/http-foundation v3.3.0-BETA1 Symfony HttpFoundation Component
symfony/http-kernel v3.3.0-BETA1 Symfony HttpKernel Component
symfony/phpunit-bridge v3.3.0-BETA1 Symfony PHPUnit Bridge
symfony/polyfill-iconv v1.3.0 Symfony polyfill for the Iconv extension
symfony/polyfill-mbstring v1.3.0 Symfony polyfill for the Mbstring extension
symfony/process v3.3.0-BETA1 Symfony Process Component
symfony/psr-http-message-bridge v1.0.0 PSR HTTP message bridge
symfony/routing v3.3.0-BETA1 Symfony Routing Component
symfony/serializer v3.3.0-BETA1 Symfony Serializer Component
symfony/translation v3.3.0-BETA1 Symfony Translation Component
symfony/validator v3.3.0-BETA1 Symfony Validator Component
symfony/yaml v3.3.0-BETA1 Symfony Yaml Component

jibran’s picture

+++ b/core/composer.json
@@ -32,7 +32,10 @@
+        "symfony/debug": "3.3-beta1",
+        "symfony/dom-crawler": "3.3-beta1",
+        "symfony/browser-kit": "3.3-beta1"

No need to add these to composer.json just update these packages using cli and it'd update composer.lock.

Status: Needs review » Needs work

The last submitted patch, 12: 2874909-12-B.patch, failed testing.

jibran’s picture

Status: Needs work » Needs review
FileSize
37.99 KB

Status: Needs review » Needs work

The last submitted patch, 15: update_symfony-2874909-15.patch, failed testing.

deviantintegral’s picture

Status: Needs work » Needs review
FileSize
62.29 KB

Running with updated dependencies is causing some functional tests to fail against 8.4.x. 8.3.x is fine. I found this with a test for the Amazon SNS module, but it happens with core tests too.

docroot sudo -u www-data vendor/bin/phpunit -c core/phpunit.xml -vvv --group node --testsuite functional --debug --stop-on-failure
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

Runtime:	PHP 7.0.18-1+deb.sury.org~trusty+1
Configuration:	/var/www/docroot/core/phpunit.xml

Testing

Starting test 'Drupal\Tests\node\Functional\Migrate\d7\NodeMigrateDeriverTest::testBuilder'.
.
Starting test 'Drupal\Tests\node\Functional\MultiStepNodeFormBasicOptionsTest::testMultiStepNodeFormBasicOptions'.
E

Time: 34.68 seconds, Memory: 42.00MB

There was 1 error:

1) Drupal\Tests\node\Functional\MultiStepNodeFormBasicOptionsTest::testMultiStepNodeFormBasicOptions
Exception: Warning: Class __PHP_Incomplete_Class has no unserializer
Symfony\Component\Routing\Route->unserialize()() (Line: 120)


/var/www/docroot/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:44
/var/www/docroot/vendor/guzzlehttp/promises/src/Promise.php:203
/var/www/docroot/vendor/guzzlehttp/promises/src/Promise.php:156
/var/www/docroot/vendor/guzzlehttp/promises/src/TaskQueue.php:47
/var/www/docroot/vendor/guzzlehttp/promises/src/Promise.php:246
/var/www/docroot/vendor/guzzlehttp/promises/src/Promise.php:223
/var/www/docroot/vendor/guzzlehttp/promises/src/Promise.php:267
/var/www/docroot/vendor/guzzlehttp/promises/src/Promise.php:225
/var/www/docroot/vendor/guzzlehttp/promises/src/Promise.php:62
/var/www/docroot/vendor/guzzlehttp/guzzle/src/Client.php:129
/var/www/docroot/vendor/fabpot/goutte/Goutte/Client.php:155
/var/www/docroot/vendor/symfony/browser-kit/Client.php:315
/var/www/docroot/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:144
/var/www/docroot/vendor/behat/mink/src/Session.php:148
/var/www/docroot/core/tests/Drupal/Tests/BrowserTestBase.php:306
/var/www/docroot/core/tests/Drupal/Tests/BrowserTestBase.php:446
/var/www/docroot/core/modules/node/tests/src/Functional/NodeTestBase.php:32

FAILURES!
Tests: 2, Assertions: 18, Errors: 1.

This patch does nothing but run composer update. I expect this would break for anyone trying to use 8.4.x with a composer-based workflow.

Status: Needs review » Needs work

The last submitted patch, 18: 2874909.18-update-dependencies.patch, failed testing. View results

mpdonadio’s picture

#18 seems to be missing composer.json and the changes to ReverseProxyMiddleware.

mpdonadio’s picture

Status: Needs work » Needs review
FileSize
37.85 KB
20.86 KB

Ok, back to tildes in composer.json, which results in 3.3.2 in the lockfile after `composer update 'symfony/*'`.

Interdiff is against #15. Didn't see anything odd in the lockfile.

Still expect fails because deprecations from phpunit-bridge, and other quirks.

Status: Needs review » Needs work

The last submitted patch, 21: 2874909-21.patch, failed testing. View results

jibran’s picture

Status: Needs review » Needs work

The last submitted patch, 23: 2874909-23.patch, failed testing. View results

jibran’s picture

Status: Needs work » Needs review
FileSize
3.16 KB
48.28 KB

We have a @see to https://github.com/symfony/symfony/pull/12521 in \Drupal\Tests\Component\EventDispatcher\ContainerAwareEventDispatcherTest and
[EventDispatcher] Add Drupal EventDispatcher is closed in favor of [DI][EventDispatcher] Add & wire closure-proxy argument type. Any suggestions how to fix The Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher class is deprecated since version 3.3 and will be removed in 4.0.

Status: Needs review » Needs work

The last submitted patch, 25: update_symfony-2874909-25.patch, failed testing. View results

hswong3i’s picture

hswong3i’s picture

Status: Needs work » Needs review
cilefen’s picture

timmillwood’s picture

Looks like if this gets in to 8.4.x we'll need to increase the minimum PHP requirement to 5.6 (currently it's 5.5.9).

Queuing some tests to be sure.

Status: Needs review » Needs work

The last submitted patch, 27: drupal-update_symfony_3.3-2874909-27.patch, failed testing. View results

timmillwood’s picture

Issue tags: +Needs reroll

ok, we can check this after reroll.

Jo Fitzgerald’s picture

Status: Needs review » Needs work

The last submitted patch, 34: 2874909-34.patch, failed testing. View results

Eric_A’s picture

pk188’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
81.02 KB

Re rolled.

Status: Needs review » Needs work

The last submitted patch, 37: 2874909-37.patch, failed testing. View results

timmillwood’s picture

Status: Needs work » Postponed

Not sure it's worth doing anymore re-rolls until we've moved Drupal to have a minimum PHP version of 5.6

jibran’s picture

composer show symfony/*
symfony/browser-kit             v3.3.2 Symfony BrowserKit Component
symfony/class-loader            v3.3.2 Symfony ClassLoader Component
symfony/console                 v3.3.2 Symfony Console Component
symfony/css-selector            v3.3.2 Symfony CssSelector Component
symfony/debug                   v3.3.2 Symfony Debug Component
symfony/dependency-injection    v3.3.2 Symfony DependencyInjection Component
symfony/dom-crawler             v3.3.2 Symfony DomCrawler Component
symfony/event-dispatcher        v3.3.2 Symfony EventDispatcher Component
symfony/http-foundation         v3.3.2 Symfony HttpFoundation Component
symfony/http-kernel             v3.3.0 Symfony HttpKernel Component
symfony/phpunit-bridge          v3.3.2 Symfony PHPUnit Bridge
symfony/polyfill-iconv          v1.4.0 Symfony polyfill for the Iconv extension
symfony/polyfill-mbstring       v1.4.0 Symfony polyfill for the Mbstring extension
symfony/process                 v3.3.2 Symfony Process Component
symfony/psr-http-message-bridge v1.0.0 PSR HTTP message bridge
symfony/routing                 v3.3.2 Symfony Routing Component
symfony/serializer              v3.3.2 Symfony Serializer Component
symfony/translation             v3.3.2 Symfony Translation Component
symfony/validator               v3.3.2 Symfony Validator Component
symfony/yaml                    v3.3.2 Symfony Yaml Component
timmillwood’s picture

It seems the following libraries depend on PHP 5.6:

  • doctrine/annotations
  • doctrine/collections
  • doctrine/common
  • zendframework/zend-feed
  • zendframework/zend-stdlib
jibran’s picture

Yup, those are all listed in #2864037: [META] Update core dependencies.

Status: Needs review » Needs work

The last submitted patch, 40: update_symfony-2874909-40.patch, failed testing. View results

Eric_A’s picture

Issue tags: +Needs reroll
jibran’s picture

Mile23’s picture

slasher13’s picture

- use caret over tilde in composer.json
- update symfony/* to 3.3.5

Status: Needs review » Needs work

The last submitted patch, 48: update_symfony-2874909-48.patch, failed testing. View results

Mile23’s picture

From the test results: https://www.drupal.org/pift-ci-job/720562

The Symfony\Component\EventDispatcher\ContainerAwareEventDispatcher class is deprecated since version 3.3 and will be removed in 4.0. Use EventDispatcher with closure factories instead: 8x

That looks quite non-trivial.