Upgrade from 3.3 to 3.4 & 4.0 #673

yceruto · 2017-10-14T13:44:16Z

Remaining deprecation notices:

SwiftmailerBundle

~~User Deprecated: The "swiftmailer.mailer.default.plugin.messagelogger" service is private, checking for its existence is deprecated since Symfony 3.2 and will fail in 4.0.~~
User Deprecated: The "swiftmailer.mailer.default.plugin.messagelogger" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
~~User Deprecated: The "mailer" service is private, checking for its existence is deprecated since Symfony 3.2 and will fail in 4.0.~~
The "swiftmailer.mailer.default.transport.real" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.

WhiteOctoberPagerfantaBundle

User Deprecated: The "white_october_pagerfanta.view_factory" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.

Symfony

~~The Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeSessionHandler class is deprecated since version 3.4 and will be removed in 4.0. Use the \SessionHandler class instead.~~
User Deprecated: The Symfony\Component\HttpFoundation\Session\Storage\Proxy\SessionHandlerProxy class is deprecated since version 3.4 and will be removed in 4.0. Use your session handler implementation directly.
User Deprecated: The Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy class is deprecated since version 3.4 and will be removed in 4.0. Use your session handler implementation directly.
User Deprecated: The "Doctrine\DBAL\Configuration" class is considered internal When adding a new configuration option just write a getter/setter pair and add the option to the _attributes array with a proper default value. It may change without further notice. You should not use it from "Doctrine\ORM\Configuration".
~~Setting logout_on_user_change to false is deprecated as of 3.4 and will always be true in 4.0. Set logout_on_user_change to true in your firewall configuration. (isn't a blocker for 4.0)~~
Relying on service auto-registration for type "App\Entity\Post" is deprecated since version 3.4 and won't be supported in 4.0. Create a service named "App\Entity\Post" instead. (isn't a blocker for 4.0)
User Deprecated: The "Symfony\Bridge\Doctrine\ManagerRegistry::setContainer()" method is deprecated since version 3.4 and will be removed in 4.0. Inject a PSR-11 container using the constructor instead.

yceruto · 2017-10-15T20:18:26Z

Relying on service auto-registration for type "App\Entity\Post" is deprecated since version 3.4 and won't be supported in 4.0. Create a service named "App\Entity\Post" instead.

Even though it doesn't a blocker to migrate to 4.0, look weird auto-registration for classes that already are excluded by configuration exclude: '../src/{...,Entity,...}'

javiereguiluz · 2017-10-16T06:46:56Z

The SwiftmailerBundle deprecations have been reported in symfony/swiftmailer-bundle#191.

javiereguiluz · 2017-10-16T07:11:51Z

The WhiteOctoberPagerfantaBundle deprecation is being fixed in sampart/WhiteOctoberPagerfantaBundle#173

javiereguiluz · 2017-10-16T07:41:48Z

The strange deprecation related to App\Entity\Post has been reported here: symfony/symfony#24569

stof · 2017-10-16T09:15:44Z

composer.json

-        "symfony/yaml": "^3.3",
-        "twig/extensions": "^1.5",
+        "doctrine/doctrine-fixtures-bundle": "2.4.0",
+        "erusev/parsedown": "1.6.3",


Why exact requirements everywhere ? This is clearly no a best practice (forbidding to update patch releases with a simple composer update makes maintenance harder)

NVM, it's a temporal change to avoid updating these dependencies right now, later these constraints will be reverted, just say: stop! don't move!! :) because "minimum-stability": "dev"

But 3/4 of the dependencies are not in the composer.json (as they are transitive deps), and these are still updated to dev versions. So I don't think these changes make sense (and they make the review harder, as well as the future maintenance)

these are still updated to dev versions.

That's why I added "symfony/lts": "dev-master" to avoid transitive deps updating to 4-dev, all of them are forced to 3-dev state ATM.

So I don't think these changes make sense (and they make the review harder, as well as the future maintenance)

I guess you're refering to changes like this "erusev/parsedown": "1.6.3"? if so, they are temporal changes to avoid updating these deps right now. The "minimum-stability": "dev" option forces all deps to dev unless there is a constraint that prevents it (that's I did with Symfony transitive deps symfony/lts).

We need these changes to rely on Symfony 3/4-dev deps only. I think isn't a good idea rely on all dev deps during the migration to 4.0, since we would be testing against dev deps that later they might not work or simply don't work on master.

When the upgrade process finishes, we can go back safely to normal constraints 1.6.3 > ^1.6, remove "minimum-stability": "dev" option, remove symfony/lts and fix the Symfony deps ^4.0@dev > ^4.0.

If it isn't reasonable, please, what would be the alternative during this upgrade?

javiereguiluz · 2017-10-17T06:36:39Z

In this PR we can also move src/Resources/TwigBundle/views/* to templates/bundles/TwigBundle/

javiereguiluz · 2017-10-17T08:21:42Z

Makefile was removed from Flex (see symfony/flex#184) so we need to remove it here too. Thanks!

yceruto · 2017-10-17T15:51:06Z

@javiereguiluz done! also the session deprecations are gone after update vendors.

stof · 2017-10-18T10:55:36Z

src/Command/AddUserCommand.php

@@ -44,6 +44,10 @@
 */
 class AddUserCommand extends Command
 {
+    // to make your command lazily loaded, either define its $defaultName static property,


either does not make sense if you don't have a or later

Indeed, thanks!

stof · 2017-10-18T10:56:05Z

src/Command/AddUserCommand.php

@@ -68,8 +72,7 @@ public function __construct(EntityManagerInterface $em, UserPasswordEncoderInter
    protected function configure()
    {
        $this
-            // a good practice is to use the 'app:' prefix to group all your custom application commands
-            ->setName('app:add-user')
+            // you don't need to call ->setName() for configuring the command when it is lazy.


this comment is useless (people won't see setName at all)

javiereguiluz · 2017-10-18T14:52:09Z

src/Command/AddUserCommand.php

@@ -44,6 +44,10 @@
 */
 class AddUserCommand extends Command
 {
+    // to make your command lazily loaded, configures the $defaultName static property,


Typo: configures the $defaultName... -> configure the $defaultName...

Suggestion: only when the app:add-user command is actually called -> only when the command is actually called

Thanks Javier, done.

yceruto · 2017-10-19T00:23:47Z

After Fix a deprecation when accessing the mailer service swiftmailer-bundle#200 "mailer" warning is gone.
Also I updated the composer.json to move all non-symfony deps to the latest stable version. (removed static constraints)

yceruto · 2017-10-19T01:22:56Z

After Fix deprecations when accessing to private services from container swiftmailer-bundle#203 remaining warnings from swiftmailer-bundle are gone.

yceruto · 2017-10-19T13:39:28Z

symfony/lts v3 has been released so, updating its constraint here.

yceruto · 2017-10-19T20:28:04Z

Doctrine warnings are gone after update to doctrine/doctrine-bundle 1.6.

javiereguiluz · 2017-10-23T10:42:48Z

@yceruto I'm testing the latest changes of this PR. Your work is great. Thank you! I only have a question: when running tests I just see 1 deprecation, but when running the application in the browser I see many more. Do you know why?

stof · 2017-10-23T10:48:58Z

@javiereguiluz I think some of them are coming from the DebugClassLoader, and so depend on whether the class is loaded or no.

stof · 2017-10-23T10:50:11Z

and the profiler also redisplays the deprecations from the compile-time, even though the cached container was used. The PHPUnit Bridge does not. It only reports deprecations actually happening during the tests.

javiereguiluz · 2017-10-23T10:51:06Z

Do you consider this behavior something to be improved? I'd like to run tests and get ALL deprecations, instead of having to browse the application manually. Thanks!

stof · 2017-10-23T10:56:07Z

@javiereguiluz reporting compile-time deprecations in the bridge would be quite hard. There is nothing triggering a E_USER_DEPRECATED here for them. The data collector is reading the cached file in which they were saved.
The PHPUnit bridge cannot do it, as it cannot be sure that the test being executed actually used the container at all (most unit tests don't use it, and we don't want them to report deprecations from a previous functional test, as you would then have to mark the whole testsuite as legacy).
If you want to have compile-time deprecations in your testsuite, write a test forcing a cache warmup so that the cache is not used.

yceruto · 2017-10-23T12:36:13Z

@dmaicher we might need a new release to avoid use dev-master constraint for Symfony 4 ;) thanks!

yceruto · 2017-10-23T12:42:33Z

We can't move forward until white-october/pagerfanta-bundle adds FrameworkBundle 4.0 compat in its composer.json.

dmaicher · 2017-10-23T14:40:48Z

@yceruto I created a new release that allows Symfony 4

javiereguiluz · 2017-10-26T13:46:32Z

I've tested the latest changes and I still see these four deprecations. How can we solve them? Thanks!

The "Doctrine\DBAL\Configuration" class is considered internal When adding a new configuration option just write a getter/setter pair and add the option to the _attributes array with a proper default value. It may change without further notice. You should not use it from "Doctrine\ORM\Configuration".
Setting logout_on_user_change to false is deprecated as of 3.4 and will always be true in 4.0. Set logout_on_user_change to true in your firewall configuration.
The "Doctrine\DBAL\Configuration" class is considered internal When adding a new configuration option just write a getter/setter pair and add the option to the _attributes array with a proper default value. It may change without further notice. You should not use it from "Doctrine\ORM\Configuration".
Relying on service auto-registration for type "App\Entity\Post" is deprecated since version 3.4 and won't be supported in 4.0. Create a service named "App\Entity\Post" instead.

javiereguiluz · 2017-10-27T07:41:29Z

In my local fork I've removed all deprecations as follows:

Change "minimum-stability": "beta" to "minimum-stability": "dev" so you can get the latest Symfony code and get rid of:
- The "Doctrine\DBAL\Configuration" class is considered internal When adding a new configuration option just write a getter/setter pair and add the option to the _attributes array with a proper default value. It may change without further notice. You should not use it from "Doctrine\ORM\Configuration".
- The "Doctrine\DBAL\Configuration" class is considered internal When adding a new configuration option just write a getter/setter pair and add the option to the _attributes array with a proper default value. It may change without further notice. You should not use it from "Doctrine\ORM\Configuration".
Edit config/packages/security.yaml and add logout_on_user_change: true to the main firewall to get rid of:
- Setting logout_on_user_change to false is deprecated as of 3.4 and will always be true in 4.0. Set logout_on_user_change to true in your firewall configuration.
Update the src/Kernel.php file according to https://github.com/symfony/recipes/pull/221/files to get rid of:
- Relying on service auto-registration for type "App\Entity\Post" is deprecated since version 3.4 and won't be supported in 4.0. Create a service named "App\Entity\Post" instead.

javiereguiluz · 2017-10-29T18:39:15Z

@yceruto minor comment. We should also update in this PR the following "bundle notation" to use the new FQCN-based notation:

demo/templates/base.html.twig

Line 101 in c68bcdd

{{ render_esi(controller('FrameworkBundle:Template:template', {

yceruto · 2017-10-31T04:04:23Z

@javiereguiluz perhaps we can ignore 2 & 3, as they'll disappear in 4.0

yceruto · 2017-10-31T17:32:52Z

I don't know why, but after update deps doctrine/doctrine-migrations-bundle has been installed transitively, which it's weird :/

ogizanagi · 2017-10-31T17:41:54Z

@yceruto : It's now part of the orm-pack

ogizanagi · 2017-10-31T18:56:21Z

About

Relying on service auto-registration for type "App\Entity\Post" is deprecated since version 3.4 and won't be supported in 4.0. Create a service named "App\Entity\Post" instead. (isn't a blocker for 4.0)

You should set the container.autowiring.strict_mode to true :)

It's done directly in the Kernel in the fwb recipe

yceruto · 2017-10-31T19:33:11Z

@ogizanagi yeah, I know :) but the parameter is useless in 4.0 and this PR ends there.

Same for the another one, setting logout_on_user_change to true removes the warning, but in 4.0 it's true by default, so we don't need to do something either.

The only blocker to move to 4.0 is #673 (comment) right now :/

yceruto · 2017-11-02T18:42:11Z

sampart/WhiteOctoberPagerfantaBundle#173 merged! We are already in Symfony 4.0!

Just waiting for a new release of white-october/pagerfanta-bundle to see the test green.

yceruto · 2017-11-06T13:55:15Z

Tests are green now, this PR is ready? or should we wait for the stable release?

javiereguiluz · 2017-11-08T21:22:55Z

@yceruto to make your life easier, I think we should merge this as is ... and keep updating to other betas, RCs and the stable version in separate PRs. That would also help others test these changes more easily. If nobody disagrees with this idea, I'll merge it tomorrow morning. Thanks!

yceruto · 2017-11-08T21:32:45Z

@javiereguiluz totally agree!

javiereguiluz · 2017-11-09T08:48:21Z

Merged! We're in Symfony 4!!! Thanks @yceruto!

TomasVotruba · 2017-12-12T17:05:39Z

config/services.yaml

    # when the service definition only contains arguments, you can omit the
    # 'arguments' key and define the arguments just below the service class
-    App\Twig\AppExtension:
-        $locales: '%app_locales%'
-
    App\EventSubscriber\CommentNotificationSubscriber:
        $sender: '%app.notifications.email_sender%'


Maybe rename property and remove this definition too?

yceruto changed the title ~~Upgrade to 3.3 -> 3.4 -> 4.0~~ Upgrade from 3.3 to 3.4 & 4.0 Oct 15, 2017

javiereguiluz mentioned this pull request Oct 16, 2017

Remove deprecations in PagerfantaExtension sampart/WhiteOctoberPagerfantaBundle#173

Merged

javiereguiluz mentioned this pull request Oct 16, 2017

Unexpected deprecation message related to an entity symfony/symfony#24569

Closed

stof requested changes Oct 16, 2017

View reviewed changes

javiereguiluz mentioned this pull request Oct 17, 2017

Remove Makefile #680

Closed

yceruto changed the title ~~Upgrade from 3.3 to 3.4 & 4.0~~ (WIP) Upgrade from 3.3 to 3.4 & 4.0 Oct 17, 2017

stof reviewed Oct 18, 2017

View reviewed changes

javiereguiluz reviewed Oct 18, 2017

View reviewed changes

javiereguiluz added the Feature label Oct 23, 2017

javiereguiluz mentioned this pull request Nov 3, 2017

Performance comparison of 3.3.9 vs 4.0 beta2 symfony/symfony#24808

Closed

javiereguiluz changed the title ~~(WIP) Upgrade from 3.3 to 3.4 & 4.0~~ Upgrade from 3.3 to 3.4 & 4.0 Nov 6, 2017

yceruto added 2 commits November 8, 2017 10:34

Migrate to Symfony 4.0

5e7c07e

[Best Practice 4.0] Remove Action suffix from controllers

b289391

javiereguiluz added the FINISHED label Nov 9, 2017

javiereguiluz closed this in a3b4eae Nov 9, 2017

yceruto deleted the upgrade_to_34_40 branch November 9, 2017 12:09

TomasVotruba reviewed Dec 12, 2017

View reviewed changes

TomasVotruba mentioned this pull request Dec 12, 2017

[WIP] Require min. PHP 7.1 and Symfony 3.4 CoreSphere/ConsoleBundle#81

Closed

Upgrade from 3.3 to 3.4 & 4.0 #673

Upgrade from 3.3 to 3.4 & 4.0 #673

Conversation

yceruto commented Oct 14, 2017 • edited Loading

SwiftmailerBundle

WhiteOctoberPagerfantaBundle

Symfony

yceruto commented Oct 15, 2017 • edited Loading

javiereguiluz commented Oct 16, 2017

javiereguiluz commented Oct 16, 2017

javiereguiluz commented Oct 16, 2017

Choose a reason for hiding this comment

yceruto Oct 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yceruto Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

javiereguiluz commented Oct 17, 2017

javiereguiluz commented Oct 17, 2017

yceruto commented Oct 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yceruto commented Oct 19, 2017 • edited Loading

yceruto commented Oct 19, 2017 • edited Loading

yceruto commented Oct 19, 2017 • edited Loading

yceruto commented Oct 19, 2017 • edited Loading

javiereguiluz commented Oct 23, 2017

stof commented Oct 23, 2017

stof commented Oct 23, 2017

javiereguiluz commented Oct 23, 2017

stof commented Oct 23, 2017

yceruto commented Oct 23, 2017

yceruto commented Oct 23, 2017 • edited Loading

dmaicher commented Oct 23, 2017

javiereguiluz commented Oct 26, 2017

javiereguiluz commented Oct 27, 2017 • edited Loading

javiereguiluz commented Oct 29, 2017

yceruto commented Oct 31, 2017 • edited Loading

yceruto commented Oct 31, 2017

ogizanagi commented Oct 31, 2017

ogizanagi commented Oct 31, 2017

yceruto commented Oct 31, 2017

yceruto commented Nov 2, 2017 • edited Loading

yceruto commented Nov 6, 2017 • edited Loading

javiereguiluz commented Nov 8, 2017

yceruto commented Nov 8, 2017

javiereguiluz commented Nov 9, 2017

Choose a reason for hiding this comment

yceruto commented Oct 14, 2017 •

edited

Loading

yceruto commented Oct 15, 2017 •

edited

Loading

yceruto Oct 16, 2017 •

edited

Loading

yceruto Oct 17, 2017 •

edited

Loading

yceruto commented Oct 17, 2017 •

edited

Loading

yceruto commented Oct 19, 2017 •

edited

Loading

yceruto commented Oct 19, 2017 •

edited

Loading

yceruto commented Oct 19, 2017 •

edited

Loading

yceruto commented Oct 19, 2017 •

edited

Loading

yceruto commented Oct 23, 2017 •

edited

Loading

javiereguiluz commented Oct 27, 2017 •

edited

Loading

yceruto commented Oct 31, 2017 •

edited

Loading

yceruto commented Nov 2, 2017 •

edited

Loading

yceruto commented Nov 6, 2017 •

edited

Loading