Upgrade to Symfony 2.7.0

We should also note that the Symfony folks have changed their strategy slightly. Rather than going from 2.7 to 3.0, they will also have an additional 2.8 release which will come out at the same time as Symfony 3.0 in November, Transition from Symfony 2.7 to 3.0... Symfony 2.8 on its way. The Symfony 2.8 release will also be a LTS release like Symfony 2.7, so we may want to consider as we approach November whether we want to upgrade to Symfony 2.8. However, we do not need to make that decision at this time.

Yes the test bot doesn't seem to like it. It has been queued half a dozen times now.

Not sure what the error before was, but the patch currently doesn't apply anymore.

They Symfony folks have just released BETA2 of Symfony 2.7. Symfony 2.7.0-BETA2 released. The stable production release of Symfony 2.7 is still currently scheduled for end of May/beginning of June.

I think this issue needs to be upgraded to Critical. Drupal 8 is currently on Symfony 2.6.x, which will end-of-life for bug fixes in July 2015, and end-of-life for security fixes in January 2016. It is essential (critical?) that we move on to Symfony 2.7 as soon as possible. In [policy, no patch] Follow symfony 2.7 or 3.0., this is the question, we agreed that when Symfony 2.7 beta came out we would upgrade to it, and then track additional Symfony 2.7 releases as we do additional releases of Drupal 8 betas (and then stables).

Given that Symfony 2.6 is very close to its end-of-life, I do not believe that we can do a Drupal 8 RC with Symfony 2.6, so from my perspective it is critical that we update to Symfony 2.7.

I tried to update to Symfony 2.7.0-BETA2 here https://qa.drupal.org/pifr/test/1045813

@TJacksonVA
Yeah no question at all, we should follow the symfony releases, even to 2.8 but its not a reason to put it to critical.
Its just yet antother release.

I tried to update to Symfony 2.7.0-BETA2 here https://qa.drupal.org/pifr/test/1045813

Interesting result! Opened up a follow up in #2488860: Bring phpunit bridge into drupal and use it for unit tests and simpletest to handle Deprecation

#2489122: Get rid of calls to deprecated API calls.

Let's merge stuff together and see whether it works

posting big patch for dawehner

Too much exceptions 8-)

Great work much less exceptions.

Fixed the fatal error in the unit tests (yay, symfony is using PSR-4 now, 3 directory levels less in vendor/symfony), this should hopefully allow us to see the test fails.

Still some unit test fails that I wasn't able to figure out quickly. Mostly deprecation messages, with _method and some validator stuff.

Hint: use git config --global diff.renameLimit 1500 so that rename detection works with that many files, cuts the patch size in half.

Going through some of those exceptions...

WTF @getMessagePluralization() being deprecated in favor of getPlural() but that isn't in the interface. real funny ;) Same for getMessageParameters(). I think the deprecation messages might actually be on the wrong method?

Some more fixes ...

Quick reroll.

git diff 8.0.x --binary

Fixed the remaining once.

This might apply correctly.

Here is reviewable patch.

Here is a change record: https://www.drupal.org/node/2489948

1. +++ b/core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php
   @@ -43,10 +43,6 @@ public function __construct(ParameterBagInterface $parameterBag = NULL) {
   -    if ($this->hasDefinition($id) && ($definition = $this->getDefinition($id)) && $definition->isSynchronized()) {
   -      $this->synchronize($id);
   -    }
   

   So we just don't need this anymore? Fine by me

2. +++ b/core/lib/Drupal/Core/RouteProcessor/RouteProcessorCurrent.php
   @@ -39,8 +39,15 @@ public function __construct(RouteMatchInterface $route_match) {
   +        $requirements = $current_route->getRequirements();
   +        unset($requirements['_format']);
   +        unset($requirements['_method']);
   +        unset($requirements['_schema']);
   +        $route->setRequirements($requirements);
   

   This seems scary, and needs a comment. (and maybe tests?!)

3. +++ b/core/tests/Drupal/Tests/Core/TypedData/RecursiveContextualValidatorTest.php
   @@ -81,7 +81,7 @@ protected function setUp() {
   -    $translator = new DefaultTranslator();
   +    $translator = new IdentityTranslator();
   

   ?!

This seems scary, and needs a comment. (and maybe tests?!)

Well, we the method and schema are used later, so its really about the _format being accidentally unset.

?!

As explained, DefaultTranslator got marked as deprecated. Note: The .gitignore file which existed before is now tracked as part symfony/validator itself.

Forgot the comment.

+++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
@@ -351,7 +351,7 @@ public function massageFormValues(array $values, array $form, FormStateInterface
     /** @var \Symfony\Component\Validator\ConstraintViolationInterface $violation */
     foreach ($violations as $offset => $violation) {
-      $parameters = $violation->getMessageParameters();
+      $parameters = $violation->getParameters();
       if (isset($parameters['@uri'])) {
         $parameters['@uri'] = static::getUriAsDisplayableString($parameters['@uri']);
         $violations->set($offset, new ConstraintViolation(
@@ -361,7 +361,7 @@ public function flagErrors(FieldItemListInterface $items, ConstraintViolationLis

@@ -361,7 +361,7 @@ public function flagErrors(FieldItemListInterface $items, ConstraintViolationLis
           $violation->getRoot(),
           $violation->getPropertyPath(),
           $violation->getInvalidValue(),
-          $violation->getMessagePluralization(),
+          $violation->getPlural(),
           $violation->getCode()

Do you have more information what this is about?

As I said above, the new methods aren't defined on the interface and are documented as alias of the old ones.

Either the documentation needs to be updated to reflect the deprecation or it needs to be changed (e.g., the other methods should have the trigger_error()). So I'm pretty sure we need a PR and should create that before they release the stable release, but I'm not sure what the PR should actually do...

Do you have more information what this is about?

As I said above, the new methods aren't defined on the interface and are documented as alias of the old ones.

Either the documentation needs to be updated to reflect the deprecation or it needs to be changed (e.g., the other methods should have the trigger_error()). So I'm pretty sure we need a PR and should create that before they release the stable release, but I'm not sure what the PR should actually do...

OH right I totally forgot to tell you about that. When I talked with fago about the weirdness of the validator he told me that webmozart plans to get rid of the interface and rely entirely on a value object here in 3.0 so we are kinda prepared here and its fine what we are doing. The fact that its not part of the interface is caused by having a BC promise on the interface.

I went through this today with @dawehner, and I think he adequately addressed @Berdir's question in #45. I consider it RTBC.

We can mark

• #2488860: Bring phpunit bridge into drupal and use it for unit tests and simpletest to handle Deprecation
• #2489122: Get rid of calls to deprecated API calls.
• #2486883: Upgrade to Symfony 2.6.7

as won't fix now.

#46: Ok, fine, but the documentation there is really bad :(

+++ b/core/composer.lock
@@ -2191,21 +2191,20 @@
             "name": "symfony/class-loader",
-            "version": "v2.6.6",
-            "target-dir": "Symfony/Component/ClassLoader",
+            "version": "v2.7.0-BETA2",

This is still 2.7 BETA, I don't think we want to commit this already?

The PHP7 fix is also AFAIK not yet part of this.

It's great that it's green now, so I'd suggest to postpone/wait for the final release and then do the update?

We might also need a change record for some of the changes caused by this and the beta evaluation doesn't seem correct, there is definitely distruption?

As this patch updates our forked YamlFileLoader, it would be great if it also would include the changes missed during the 2.5 and 2.6 updates #2375339: Update DependencyInjection YamlFileLoader for Symfony 2.6.

Yes we should only update to the stable, great that this is green though.

Also CNW for #50.

Since this required updates to get working, bumping to critical - core shouldn't be relying on deprecated Symfony code for 8.0.0.

According to the symfony release schedule we will have an update to the stable version in may, so let's just wait a bit more and get it done.

Postponed awaiting the Symfony 2.7 LTS release. See http://symfony.com/roadmap.

If this is postponed till the stable release then can we re-open #2486883: Upgrade to Symfony 2.6.7?

If this is postponed till the stable release then can we re-open #2486883: Upgrade to Symfony 2.6.7?

What would be the gain? The LTS release will be there in a short time.

Since 2.6.8 contain security fixes.

Reroll after #2493807: Add symfony/console component to core

2.7.0 got tagged, see https://github.com/symfony/symfony/commit/7493c2bef54fb818c5304bdd9d2194...

and officially released: Symfony 2.7.0 released

Fixed the test fail.

This also means that TypedDataTest is now passing on PHP7, this should be the last real test fail.

Renamed our class and generated patch with --binary.

As per #47.

What about #50/#51?

Sure, let's do that. Patch btw. nearly applied.

Updated the recommended commit message ...

(Saving issue credit to correspond to the proposed commit message.)

@webchick, @alexpott, @catch, @effulgentsia and I all agreed this was critical for its impacts on both #2400407: [meta] Ensure vendor (PHP) libraries are on latest stable release (part of #2485119: [meta] The Drupal 8.0.0-rc1 Release Checklist) and #2454439: [META] Support PHP 7, and D8 core needs to use a version of Symfony that will also be supported throughout the D8 cycle.

Leaving this one for someone else to commit though. :) And yay!

diff -u core/vendor/symfony/dependency-injection/Loader/YamlFileLoader.php core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php

Still is shows hunks which look like upstream changes which need to be replicated into our YamlFileLoader. Will examine them and post another patch soon.

1. Symfony 82db9c2e: [DependencyInjection] deprecated synchronized services
2. Second hunk fixes line indention
3. Symfony 0eb34f32: [DependencyInjection] Added support for keys "method" and "arguments" in "calls" statement for yaml format

Only posting interdiff here. I have no idea on how to properly produce the whole diff (mine results in 13M but #70 is only 3M).

Good catch, I guess those changes happened in the meantime since december?

@znerol
The trick is to use git diff --binary

The trick is to use git diff --binary *and* have git renames configured *and* git config --global diff.renameLimit 1500, see #26 :)

Interdiff seems fine to me there were no other changes, lets get this in.

I think we should completely rip out support for synchronised services in our container. This patch removes some of the test coverage since the methods throw PHP warnings (because deprecated).

Getting this is quickly because if it causes pain in any other criticals it is good to know early - same goes for contrib. Committed 73b5652 and pushed to 8.0.x. Thanks!

#2504967: Upgrade to Symfony 2.7.1

Published the change record (yes, a bit late, but still important ...)

The upgrade has caused #2695109: Cache bins are not deleted when the module that declares them is uninstalled