Update Symfony components to ~3.2

Thank you for creating this issue.

In theory we could directly update because we fixed all BC throws in 2.8, but I just doubt that :)

The question is basically: how long do we want people to adapt to it. To be clear, the usages of old API (<2.8) got removed from core, but contrib / custom code might still be out there.
When we add it to 8.2, we would choose 6 months ... maybe a year might be better, maybe not.

the usages of old API (<2.8) got removed from core, but contrib / custom code might still be out there.

Is there a reason the change record was left in draft or did we just forget to publish it?

Something else: if we update drupal/core composer.json we should also update drupal/core-* composer.json files whenever components depend on symfony. I've opened #2720739: [meta] drupal/core-* components and drupal/core requires/replace declarations are broken, which is about the missed "2.7.*" to "~2.8" updates.

r did we just forget to publish it?

That sounds much more likely.

I think we should close this issue as "won't fix". Symfony 3 has many API breaks, see https://github.com/symfony/symfony/blob/master/UPGRADE-3.0.md

API break examples: the ContainerAware class was removed, which is used in core and might potentially be used by contrib modules. The _method setting on routes has been renamed to _methods, so this could potentially break many of our routing definitions (if we use the Symfony routing code part for this, I have not checked).

Drupal 8 will stay on Symfony 2.8 LTS forever, because that is the non API breaking promise of Drupal 8. Symfony 3+ is Drupal 9 material.

We decided before that given the LTS of Drupal is supported for much longer than the LTS of symfony, so there is no way around that: https://www.drupal.org/node/2403135

The _method setting on routes has been renamed to _methods, so this could potentially break many of our routing definitions (if we use the Symfony routing code part for this, I have not checked).

Well, this throws deprecation notes, which is a fatal in any test we have. Its weird that ContainerAware doesn't throw anything.

Apologies, I missed #2395443: [policy, no patch] Follow symfony 2.7 or 3.0., this is the question where it was already decided that Drupal 8 will at some point adopt Symfony 3.

The looser is obviously Drupal 8 contrib and custom site modules because they will experience API breaks.

In Drupal 7 we tried to avoid API breaks at all costs, so this is a somewhat cultural change that we need to communicate more.

The looser is obviously Drupal 8 contrib and custom site modules because they will experience API breaks.

Well yeah ideally symfony 2.8 would have thrown deprecation notices for everything, and then, as said before, we should wait at least for a year to update, if not more.

We should either put this into 8.2.x very soon so there's plenty of time to find and fix bc breaks in contrib/custom. Or wait until 8.3.x is open and do it then to give them even longer.

We essentially said that stuff deprecated in 2.8 was not part of the public Drupal API so people should not be using that or can fix their usages if they are.

Would code be able to be fully compatible with both versions of Symfony, even though that is likely a bad idea? If not, that means sites will be forced to go through the trouble of upgrading to 8.2 if they want to receive security updates for core or contrib.

I am concerned about what this does to Drupal's image and its adoption. This is another break of our Semantic Versioning, and even though this change was announced long ago, people still won't care that we did, as it breaks their products as soon as they run composer update, even though their constraints were all correct.

If we do go ahead with this, I suggest we do it right now, or right after 8.2 is released, so contrib has close to six months to test and upgrade.

Would code be able to be fully compatible with both versions of Symfony, even though that is likely a bad idea?

2.8/3.0 theoretically have 100% compatible APIs, 3.0 just removed things that were already deprecated in 2.8. We'll find out how true that is in practice.

We should probably commit a patch here or in a side issue that is just fixing core to work with 3.0 (i.e. without the update itself). If that works then contrib/custom ought to be able to do the same.

2.8 was also released late last year, so long after we were in code freeze.

#2720891: Replace ContainerAware with ContainerAwareTrait is another issue

We should probably commit a patch here or in a side issue that is just fixing core to work with 3.0 (i.e. without the update itself). If that works then contrib/custom ought to be able to do the same.

Can we test this without running tests multiple times using different composer.lock files? If not, do we need to contact the DA to discuss support for this?

@Xano I think jibran's strategy is fine. We should have all the new things in 2.8 already, so we can update our usage before even updating to 3.0. Ideally this issue then only has a patch with the composer updates and nothing breaks. No additional DA support necessary.

Working on #2721139: Replace deprecated files ParameterBag usage in the meantime.

All these methods are removed in Symfony 3 so to remove these from core we have to bump Symfony.

I'm happy that we got rid of any of our usages of scope though.

And another one: #2721179: Replace deprecated Symfony ExecutionContextInterface

Rolling in those 2 issues to see how far we get now on the testbot.

Removed one more scope test in ContainerTest.

Auto git merge with 8.2.x, no other changes.

I should really execute the phpunit tests locally instead of waiting 110 minutes until the testbot aborts. Wow, our phpunit testbot runner is quite broken.

while grepping for IntrospectableContainerInterface I found #2721315: ThemeTestSubscriber::onView() doesn't do what it claims.

More fixes for the moved ValidatorInterface and the removed IntrospectableContainerInterface.

Something is broken for me with the phpunit execution when running

../vendor/bin/phpunit --stop-on-failure
PHP Fatal error:  Call to undefined function Drupal\Core\EventSubscriber\error_displayable() in core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php on line 75

Looks like includes/errors.inc is not included when it should have been, let's see what the testbot thinks.

Looks like includes/errors.inc is not included when it should have been, let's see what the testbot thinks.

How is that related with symfony 3.0, mhhhh

Some more progress.

* HTTP methods are now not part of the requirements of a route anymore in Symfony 3.
* Request::create() ingores _format query parameter.

And I merged in 8.2.x. Feel free to take this over from here.

Filed #2726645: Do not @deprecate EntityManager until all usages are removed (helps the Symfony 3 update).

Current status: update.php batching is broken. Somehow Drupal returns HTML when the batch JS expects JSON. This seems to be another instance of the problem that Request in Symfony 3 does not understand _format=json in URL query parameters. I have not figured out yet how and where this is different.

You can test this by adding an empty system_update_8015() function to system.install. Then go to /update.php and Try to run updates => kaboom.

Filed #2728815: Batch API uses request attributes instead of query in Symfony 3

Merged in changes from #2712633: Update symfony-cmf/routing to 1.4.0.

+++ b/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php
@@ -159,6 +156,19 @@ public function getListeners($event_name = NULL) {
    */
+  public function getListenerPriority($eventName, $listener) {
+    if (isset($this->listeners[$eventName])) {
+      foreach ($this->listeners[$eventName] as $priority => $listeners) {
+        if (($key = array_search($listener, $listeners, TRUE)) !== FALSE) {
+          return $priority;
+        }

See also #2682373: Implement ContainerAwareEventDispatcher::getListenerPriority()

Filed #2730129: DrupalKernel must never persist service_container for Symfony 3 update.

Thanks Berdir, another depending issue we need to solve.

Yes, switched to ::class syntax and merged in changes from child issues.

Perfect, now this is postponed on all the remaining child issues.

Removed #2729247: Replace remaining $request->get() usage to prepare core for Symfony 3 because it is not a hard requirement for the Symfony 3 update.

Merging in changes from #2726645: Do not @deprecate EntityManager until all usages are removed (helps the Symfony 3 update) to see if tests still pass on Symfony 3.

Looks like this is was PP-3 because of #2712637: Update stack/builder for Symfony 3 compatibility #2682373: Implement ContainerAwareEventDispatcher::getListenerPriority() and #2726645: Do not @deprecate EntityManager until all usages are removed (helps the Symfony 3 update).

#2682373: Implement ContainerAwareEventDispatcher::getListenerPriority() and #2726645: Do not @deprecate EntityManager until all usages are removed (helps the Symfony 3 update) are now fixed. So I think this should be PP-1

catch just committed the last one, so this is ready to be rerolled again.

Fixed conflicts on:

• composer.lock - stack/builder was bumped to v1.0.4, however this patch sets it to dev-master. I've kept our changes here, although I'm not entirely sure about this decision.
• core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php
• core/modules/system/src/Tests/DrupalKernel/DrupalKernelTest.php

1. +++ b/composer.lock
   @@ -1091,16 +1049,16 @@
                "name": "stack/builder",
   -            "version": "v1.0.4",
   +            "version": "dev-master",
   

   nope, that should not happen. stack/builder should stay on 1.0.4 which is Symfony 3 compatible.

2. +++ b/core/composer.json
   @@ -25,7 +25,7 @@
   -        "stack/builder": "1.0.*",
   +        "stack/builder": "dev-master",
   

   same here, that should not be changed.

Right, let me try that again...

1. +++ b/core/composer.json
   @@ -25,7 +25,7 @@
   -        "stack/builder": "1.0.*",
   +        "stack/builder": "dev-master",
   

   that is no longer needed

2. +++ b/core/modules/rest/tests/src/Unit/CollectRoutesTest.php
   @@ -130,8 +130,8 @@ public function testRoutesRequirements() {
   -    $this->assertEquals(count($requirements_1), 0, 'First route has no requirement.');
   -    $this->assertEquals(count($requirements_2), 2, 'Views route with rest export had the format and method requirements added.');
   +    $this->assertEquals(0, count($requirements_1), 'First route has no requirement.');
   +    $this->assertEquals(1, count($requirements_2), 'Views route with rest export had the format requirement added.');
   

   If we are switching things anyway here we could use assertCount as well

OK here's a proper reroll + that change to core/composer.json

Settting it to needs work based on #79.2

1. +++ b/composer.lock
   @@ -4155,12 +3995,14 @@
   -    "stability-flags": [],
   +    "stability-flags": {
   +        "stack/builder": 20
   +    },
   

   hm, why is this still here? Can you run composer update on the symfony packages only again?

2. +++ b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
   @@ -46,20 +46,16 @@ private function getProxyInstantiator()
      /**
   -   * Direct copy of the parent function.
   +   * Shares a given service in the container.
   +   *
   +   * @param Definition $definition
   +   * @param mixed      $service
   +   * @param string     $id
       */
   

   why are we changing that comment? I mean, it is useful to have the @param comments here, but we should also still mention that the method is a direct copy. Or is it not a direct copy anymore? Then we should also mention that and what we modified.

3. +++ b/core/modules/rest/tests/src/Unit/CollectRoutesTest.php
   @@ -130,8 +130,8 @@ public function testRoutesRequirements() {
   -    $this->assertEquals(count($requirements_1), 0, 'First route has no requirement.');
   -    $this->assertEquals(count($requirements_2), 2, 'Views route with rest export had the format and method requirements added.');
   +    $this->assertEquals(0, count($requirements_1), 'First route has no requirement.');
   +    $this->assertEquals(1, count($requirements_2), 'Views route with rest export had the format requirement added.');
   

   Agreed with dawehner, let's use assertCount() here if we change the lines anyway.

Agreed with dawehner, let's use assertCount() here if we change the lines anyway.

On the other hand there is not a real reason to change these as part of this issue if you are honest.

OK my last attempt at a proper reroll here... I'll get out of your hair now :S

+++ b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
@@ -46,20 +46,16 @@ private function getProxyInstantiator()
   /**
-   * Direct copy of the parent function.
+   * Shares a given service in the container.
+   *
+   * @param Definition $definition
+   * @param mixed      $service
+   * @param string     $id
    */
   protected function shareService(Definition $definition, $service, $id)

why are we changing that comment? I mean, it is useful to have the @param comments here, but we should also still mention that the method is a direct copy. "Direct copy of the private parent function." as additional comment I would say.

Otherwise looks good!

I checked http://symfony.com/doc/current/contributing/community/releases.html and it seems no Symfony 3.x version until 3.4 will be an LTS, so I think it is fine to upgrade until Symfony 3 gets into LTS mode. At least we are not experiencing additional breaking changes comparing 3.0 and 3.1, which is a good sign.

At least we are not experiencing additional breaking changes comparing 3.0 and 3.1, which is a good sign.

Sadly this is not strictly right. They throw E_USER_DEPRECATED, which we convert to test failures.

here we go with change record https://www.drupal.org/node/2743809 but it needs polishing before publishing what exact changes has been done.

Addressed #88.
Added another paragraph for the same.

1. +++ b/core/composer.json
   @@ -5,19 +5,19 @@
   +        "symfony/class-loader": "~3.0",
   +        "symfony/console": "~3.0",
   +        "symfony/dependency-injection": "~3.0",
   +        "symfony/event-dispatcher": "~3.0",
   +        "symfony/http-foundation": "~3.0",
   +        "symfony/http-kernel": "~3.0",
   +        "symfony/routing": "~3.0",
   +        "symfony/serializer": "~3.0",
   +        "symfony/translation": "~3.0",
   +        "symfony/validator": "~3.0",
   +        "symfony/process": "~3.0",
   

   should we still explicit use "~3.1" instead? Feels a bit safer to be honest.

2. +++ b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
   @@ -47,19 +47,17 @@ private function getProxyInstantiator()
      /**
       * Direct copy of the parent function.
   +   *
   +   * Shares a given service in the container.
   +   *
   +   * @param Definition $definition
   +   * @param mixed      $service
   +   * @param string     $id
       */
      protected function shareService(Definition $definition, $service, $id)
      {
   

   We could just use {@inheritdoc} and call it a day, can't we?

Addressing #92
Besides what @dawehner mentioned, also bumped the versions for symfony/yaml and symfony/css-selector

Patch looks good.

Now we need to list all the breaking changes from child issues that we had to fix in the change record.

https://twitter.com/dunglas/status/740123859324653568 makes it sound like we should be updating prophecy as well?

#96 +1
sounds like a followup would be good for it.

@bojanz
If we haven't run into it yet, we won't do now. Its a pure composer.json thing, see https://github.com/phpspec/prophecy/pull/273

I don't see how #94 address the concern of being more explicit with Symfony 3.1 support with the related libraries. Many libraries specifically reference Symfony "~3.0"

Many libraries specifically reference Symfony "~3.0"

Well that is not a problem in that sense. Its SEMV, so it will use 3.1.

Completed the list of API breaks we experienced at https://www.drupal.org/node/2743809 , please review!

I think we can ignore the drupal component composer.json files in this issue and leave that to the related issue. Same for the prophecy update, should also be a separate issue.

I think its fine as it is. Note: The amount of changes is so small, because we updated to 2.8 before, which throws those exception warnings before, especially see https://www.drupal.org/node/2611816#comment-10578742 so the problems would have caught people before.

After discussing with @catch we where wondering if there is a chance that the patch can be done without the library updates. This way we can prove that contrib can be compatible with 2.8 and 3.1 at the same time. This is important because otherwise modules are going to have to start adding requirements checks on core and also DrupalCI tests modules against 8.2.x so they'll need to fix that sooner.

I'm particularly interested in the ExecutionContextInterface changes, but also YAML seems not impossible to run into. If we can do those without the update, then it verifies those are 2.8 deprecations and not fresh breaks.

Nope, not possible for ExecutionContext, see my comment in #2721179-2: Replace deprecated Symfony ExecutionContextInterface.

So a way they could have done that, would be to leave the old interface in the old place @deprecated, and add the new one - then code could be written for both. However it feels relatively unlikely that 8.x contrib/custom code is using this, or at least the best way to find out is probably to commit this and see what breaks. If we end up with contrib modules that can't have working 8.1.x and 8.2.x versions, we could potentially fork 2.8 in 8.1.x to add back the old interface or something.

So a way they could have done that, would be to leave the old interface in the old place @deprecated, and add the new one - then code could be written for both.

They have done that for quite a while since symfony 2.5 or so. The step onto symfony 3 was simply to remove the old deprecated class/interfaces.

2.8 doesn't have the new interface though?

@catch it does have the new interfaces but it's not so useful (unfortunately)... you get errors like...

Declaration of Drupal\link\Plugin\Validation\Constraint\LinkAccessConstraintValidator::initialize() must be compatible with Symfony\Component\Validator\ConstraintValidatorInterface::initialize(Symfony\Component\Validator\ExecutionContextInterface $context) in /Volumes/devdisk/dev/sites/drupal8alt.dev/core/modules/link/src/Plugin/Validation/Constraint/LinkAccessConstraintValidator.php on line 15

And this is because the new interface does not extend the old one and so ConstraintValidatorInterface::initialize() can't be satisfied :(

So yeah this patch problem does contain the minimum amount of change. The container changes are tricky too because they remove a param from ContainerInterface::set() so it's not possible just make the changes to our container.

So given this the question no becomes how do we manage this change for the affect contrib modules?

Contrib modules that are affected by one of this breaking changes have 2 choices:
1) Make their code compatible with 8.2.x and tell users they must update to 8.2.x (otherwise their sites break on a stable 8.1.x)
2) Start a new branch with a new major version, for example Rules 8.x-3.x compatible with core 8.1.x and Rules 8.x-4.x compatible with core 8.2.x.

We should probably put that in the change notice?

Looked into the YAML parse() change, but we cannot do that beforehand since there is no replacement in Symfony 2.8. It got simply replaced in 3.0, so it has to be done with this issue.

Managing contrib modules: I think we cannot do anything except documenting the API breaks - the good thing is that probably only a small portion of contrib modules will be affected. I would say we should rather commit this sooner than later so that contrib has time to catch up with the API breaks until 8.2.0 is released and make the decision if they raise their minimum core requirement or start a new major branch.

Let's give this a couple of days for any input we might be missing and then put it back to RTBC.

No I agree with #114. If there had been a change which would affect a lot of contrib modules, then I'd want to strongly consider hacking forward-compatibility into 8.1.x somehow (like subclassing something in both branches and encouraging people to use that instead, or forking 2.8). However there's only theoretical breakage of a small number of modules here, and we won't find out without committing it. I think we'll get more feedback if this is RTBC so moving it there, but I won't commit for a couple of days (if someone else wants to before I get to it that's up to them).

Spoke to @alexpott about this and he was concerned about the validator change.

I think I have a workaround which would allow for 8.1.x and 8.2.x compatible modules.

+++ b/core/modules/system/tests/modules/theme_test/src/EventSubscriber/ThemeTestSubscriber.php
@@ -17,7 +17,8 @@ class ThemeTestSubscriber implements EventSubscriberInterface {
diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php

diff --git a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php
index 5905e12..246628a 100644

index 5905e12..246628a 100644
--- a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php

--- a/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php
+++ b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php

+++ b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php
+++ b/core/modules/user/src/Plugin/Validation/Constraint/UserMailRequired.php
@@ -4,7 +4,7 @@

@@ -4,7 +4,7 @@
 
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidatorInterface;
-use Symfony\Component\Validator\ExecutionContextInterface;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
 
 /**

This interface change means if you're using it, you definitely can't have a module that works in both 8.1.x and 8.2.x. I doubt we can do anything upstream, but we can provide a bridge in core.

Would suggest a new issue to do the following:

- Add a new, empty, ExecutionContextnterface to core.

- In 8.1.x it extends the old Symfony interface

- In 8.2.x it extends the new Symfony interface.

- Update all these use statements to use the new interface in core.

Then the actual 3.1.0 patch shouldn't need to change anything in any of these modules, and we've proved it's viable for contrib.

If you're calling methods that have changed then you're out of luck, but core isn't and we can only do so much.

Not pretty but meh. Requires modules to update, but not to branch.

Here's the issue, postponing this one again #2746337: Provide bridge between 2.8 and 3.0 ExecutionContextInterface.

Reroll to check if this still passes. Updated to Symfony components 3.1.2.

In #2746337: Provide bridge between 2.8 and 3.0 ExecutionContextInterface we had the idea of class aliases, but they cannot be developed in a separate issue and have to be done in this issue as part of the update.

I just tested the class_alias() patch from #2746337: Provide bridge between 2.8 and 3.0 ExecutionContextInterface on top of this Symfony 3 patch by executing the validation unit tests with ../vendor/bin/phpunit tests/Drupal/Tests/Core/Validation/.

PHP Fatal error:  Cannot use Symfony\Component\Validator\Context\ExecutionContextInterface as ExecutionContextInterface because the name is already in use in vendor/symfony/validator/ConstraintValidator.php on line 14

So it seems to me that the class_alias() call registers the name ExecutionContextInterface globally and then PHP fatals as soon as it encounters a use statement using ExecutionContextInterface. Maybe I'm doing something wrong?

Since we have not found any way around the minor interface API breaks I think we have to move on with the current patch.

@jibran: can you review the patch and set this back RTBC if you approve?

Ah, the good old migrate module random test fails. Do we have an issue for those, since they appear on every 20th testrun or so?

Resetting to RTBC, assuming testbot will pass now.

Hm, maybe the migrate test fails are not so random after all?

Let's fix all the bugs in migrate, starting with #2767503: MigrateAggregatorStubTest typo in test namespace.

MigrateUpgrade7Test was always failing for me locally (with Symfony 2 and 3), I finally figured out why: #2767595: Support "-" in database table names. *facepalm*

I can't reproduce the MigrateUpgrade7Test fail locally and can't see on the testbot what assertions went wrong because of the fatal error.

Let's try again with more robust migrate tests that should at least not fatal.

New conflicts from https://www.drupal.org/node/1869548.
Apply patch from https://www.drupal.org/node/2776105

interdiff to #118

Check if this still passes. Postpone on https://www.drupal.org/node/2776105

update symfony/* to 3.1.3
http://symfony.com/blog/symfony-3-1-3-released

Is #128 still needed?

Add OPTIONS and TRACE to the list of safe methods. Added to v2.8.9, too.
https://github.com/symfony/symfony/pull/19321

Down to 3 failures.

• interdiff to #118
• update symfony/* to 3.1.3
• add OPTIONS and TRACE to the list of safe methods

Need help with the last failures!

+++ b/core/tests/Drupal/Tests/Core/EventSubscriber/DefaultExceptionSubscriberTest.php
@@ -25,7 +25,8 @@ public function testOnExceptionWithUnknownFormat() {
-    $request = Request::create('/test?_format=bananas');
+    $request = Request::create('/test');
+    $request->setRequestFormat('bananas');

You shouldn't do this. Format handling is broken as is evident by the other failing tests and this is part of that.

https://github.com/symfony/symfony/issues/8966
https://github.com/symfony/symfony/commit/7115c1e1d913dfccbe14d5eb8f515d... is where this is broke. Symfony explicitly removed the functionality we use for format handling :(

NM, I forgot we didn't trust that code and wrote NegotiationMiddleware to handle setting the format. Someone might have even known this was coming I don't remember. The test was just lazy so here's a fix and documented why both cases set the format directly. This should fix the tests.

As a review, there are quite a few unrelated code reorganizations in tests that make the patch look a bit bigger that it is but things seem in order.

Great! I ran composer update symfony/* locally and get the same composer.lock outcome.

The rest of the updates look good as well, thanks for the comments about the request format middleware!

I did minor updates to the change record at https://www.drupal.org/node/2743809 and I think we are ready to get this into 8.3.x. Not sure about 8.2.x, but since we are in beta already we probably do not want this disruptive change there?

Just went through the patch and the changes looked sane. The changes in core/lib/Drupal/Component/DependencyInjection also look complete. The file did not had unused methods or so.

I'd love to see this get in now, but let's also make sure we have a plan to fix the version conflicts with our sub-packages fast enough: we should fix our drupal/core-* embedded-but-destined-to-be-split-off components composer.json files here if workable, or create a quickly to be resolved follow-up issue. See #2744357-17: drupal/core-* components Symfony requirements conflict with drupal/core for reasoning why we didn't just change everything to ~2.8. With the same reasoning applied here we should not just change everything to ~3.1. Most of these symfony requirements must change to something like >= 2.7. (Now at ~2.7 or ~2.8.) Changed components that now actually require ~3.1 must of course declare ~3.1.

And perhaps we need an issue to have a test for our sub-packages that detects symfony version conflicts with drupal/core.

Any chance of getting a waiver and getting this into the next beta of 8.2.x? It would be a shame to wait until 8.3.x in order to get this kind of significant change in. For developers working on modules and new websites, they would be better served to have Symfony 3.x as soon as possible, rather than wait until 8.3.x when will be upgrading to Symfony 3.2 or 3.3 (which will be an even bigger jump at that time, as opposed to going from Symfony 3.1.x to 3.2.x or 3.3.x).

I believe people overestimate the amount of changes from symfony 2.8 to symfony 3, especially given that symfony 2.8 has most of of the APIs already.
Given that I'm not convinced that rushing it into 8.2.x would be necessarily a good idea. Feel free to disagree with me :)

I think that was a random testbot fail, queued another test run.

I've not seen the random fail before.

Re #148 I mostly agree with this, except the core patch shows that there are certain changes where it's not possible (or at least not easy) to have code that's compatible with both Symfony 2.x and 3.x. Really comes down to ExecutionContextInterface which we've already discussed.

Question for me is whether this is going to apply to contrib modules - since DrupalCi tests contrib against 8.3.x, committing only to 8.3.x now could mean test failures on DrupalCI which when the module is updated, will break against 8.2.x. Given that, applying to both branches now, or applying to 8.3.x and 8.4.x when 8.3.x goes to beta would mean a shorter window of incompatibility.

+++ b/core/modules/link/src/Plugin/Validation/Constraint/LinkNotExistingInternalConstraintValidator.php
@@ -17,7 +17,7 @@ class LinkNotExistingInternalConstraintValidator implements ConstraintValidatorI
 
diff --git a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php

diff --git a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php
index 4f706f6..cd8a263 100644

index 4f706f6..cd8a263 100644
--- a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php

--- a/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php
+++ b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php

+++ b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php
+++ b/core/modules/link/src/Plugin/Validation/Constraint/LinkTypeConstraint.php
@@ -5,7 +5,7 @@

@@ -5,7 +5,7 @@
 use Drupal\link\LinkItemInterface;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidatorInterface;
-use Symfony\Component\Validator\ExecutionContextInterface;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
 

This is the change that concerns me.

Then we already discussed whether we could/should add back Symfony\Component\Validator\ExecutionContextInterface extends Symfony\Component\Validator\Context\ExecutionContextInterface to allow code to run on both versions (since we don't care about the method changes).

update to symfony 3.1.4

I'm going to re-open #2721179: Replace deprecated Symfony ExecutionContextInterface and postpone this issue on that - at least we should open an upstream Symfony issue to discuss the problem with them.

Thanks for updating the patch, back to postponed.

And back to review.

Simple reroll just ignoring all hunks that don't apply anymore after #2721179: Replace deprecated Symfony ExecutionContextInterface.

Hm, I don't understand the test fails in ConfigImportAllTest, but at least I have the same test fails locally, so not a testbot issue.

The backtrace leading to the error looks very weird, AccountProxy should not load user entities when we just want the ID. See #2753733: AccountProxy can do unnecessary user loads to get an ID.

The second fail looks like a regression of #2776235: Cached autoloader misses cause failures when missed class becomes available.

Possibly a bit crufty, but here's a re-roll.

Looks like the composer.lock is messed up on PHP 5.5:

Your requirements could not be resolved to an installable set of packages.
14:38:36 
14:38:36   Problem 1
14:38:36     - Installation request for zendframework/zend-stdlib 3.1.0 -> satisfiable by zendframework/zend-stdlib[3.1.0].
14:38:36     - zendframework/zend-stdlib 3.1.0 requires php ^5.6 || ^7.0 -> your PHP version (5.5.9) does not satisfy that requirement.
14:38:36   Problem 2
14:38:36     - zendframework/zend-stdlib 3.1.0 requires php ^5.6 || ^7.0 -> your PHP version (5.5.9) does not satisfy that requirement.
14:38:36     - zendframework/zend-feed 2.7.0 requires zendframework/zend-stdlib ^2.7 || ^3.0 -> satisfiable by zendframework/zend-stdlib[3.1.0].
14:38:36     - Installation request for zendframework/zend-feed 2.7.0 -> satisfiable by zendframework/zend-feed[2.7.0].

Interesting. I did this with PHP7, just running a PHP7 test to see how that goes...

Looks like your composer update was a bit too aggressive?
New patch with composer update symfony/*

Argl, why is password-compat still in there? composer--

I guess the fail is due to \Drupal\content_translation\ContentTranslationUpdatesManager::getSubscribedEvents() - this was supposed to be fixed by #2776235: Cached autoloader misses cause failures when missed class becomes available - I wonder how and why this change is breaking that...

@alexpott: ContentTranslationUpdatesManager has a class_exists() check and the fatal error is triggered from PluginEventSubscriber in migrate module itself. So not sure how that should be related?

Did a bit of debugging in ConfigImportAllTest. It installs all modules, then exports the configuration, then uninstalls as many modules as possible, then imports the configuration again which should re-enable the modules.

When modifying the test I could trace it down to 2 modules being uninstalled before the configuration sync:

    $modules_to_uninstall = [
      'language' => TRUE,
      'migrate' => TRUE,
    ];

Some combination of language module and migrate module is at play here. language module has ConfigSubscriber which does a container rebuild onConfigSave, could that be related? Uninstalling language module also means that content translation, config translation and interface translation is uninstalled.

Whatever I tried manually in the admin backend, I could not reproduce the fatal error. It only happens in this test case. Any hints what we could try next would be appreciated.

Forgot to attach my debugging hacks.

#176 PHP7 test has same number of fails, but the fail is in YAML parsing not the autoloader. I also can't reproduce the autoloader fail with PHP7 locally.

So looks like a PHP 5.5/5.6 issues with the autoloader, and then we have a separate YAML parsing PHP 7 failure.

See how this does on PHP7 - fixing the YAML deprecation.

Damn negative caches especially in persistent caches like APCu. I'm not sure how we've not hit this before but anyhow. APCu will cache the class_exists miss across requests. Which means we need to add the list of modules to the prefix. This isn't happening on PHP7 because I think we've not got APCu on in that environment.

+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -1029,31 +1064,6 @@ protected function initializeSettings(Request $request) {
-        $loader = new ApcClassLoader($prefix, $this->classLoader);
...
-        $loader = new WinCacheClassLoader($prefix, $this->classLoader);
...
-        $loader = new XcacheClassLoader($prefix, $this->classLoader);

The alternative is to implement our our versions of these that don't negatively cache but I think the solution of using the module list is probably best.

The biggest question about #187 is if moving the classloader swapping out to later is acceptable - I think it is because I'm not sure that anything can come in-between \Drupal\Core\DrupalKernel::initializeSettings() and \Drupal\Core\DrupalKernel::initializeContainer() so whilst there might be a few classes that are no longer loaded using the APCu (or equivalent) autoloader this number can not grow.

+++ b/composer.lock
@@ -891,48 +891,6 @@
         {
-            "name": "ircmaxell/password-compat",
-            "version": "v1.0.4",

@@ -1720,73 +1681,20 @@
-        },
-        {
-            "name": "symfony/polyfill-apcu",

@@ -1891,143 +1799,29 @@
-        },
-        {
-            "name": "symfony/polyfill-php54",
...
-        {
-            "name": "symfony/polyfill-php55",

it is a bit confusing that those entries from the composer.lock file just disappear

@dawehner: those entries disappear because the libraries disappear because Symfony 3 does not need them anymore.

Great that alexpott found a classloader solution! We have a test fail in Drupal\datetime\Tests\Views\FilterDateTimeTest, which I could not reproduce locally. Queuing all the environments for testing again.

@dawehner: those entries disappear because the libraries disappear because Symfony 3 does not need them anymore.

Ah gotcha, I guess Drupal doesn't use them?

Interesting stuff with the classloader. One question (might be a dumb question, I don't understand all this 100%): Could this also be a problem if modules are updated and provide new classes or remove old ones?

Still test fails in ConfigImportAllTest, we need to figure those out next.

@klausi I've run it 20 times locally - no fails... weird.

I've opened #2825358: Event class constants for event names can cause fatal errors when a module is installed to discuss whether we really need those event classes at all.

password-compat, polyfill-php54 and polyfill-php55 are only for PHP < 5.5 support so it's correct that we don't need any of those.

Sent DrupalCI for a retest to see if those are random.

https://www.drupal.org/pift-ci-job/524064 is the fail.