Upgrading to Symfony5

[weaverryan]

Q	A
Branch?	2.0
Bug fix?	no
New feature?	yes
BC breaks?	yes, a slight one with a LOUD error in Symfony 5
Deprecations?	yes
Tests pass?	yes
Fixed tickets	#1241 replaces #1246, #1255
License	MIT
Doc PR	n/a

Hi!

Just in case it's helpful, I took @michellesanver's work from #1246 and @Guite's work from #1255 and then continued it to hopefully get Symfony 5 support soon. I'm working on the Symfony 5 Upgrade tutorial for SfCasts, and this is a blocker :).

[weaverryan]

Ok, this is ready and tests are green! A summary of what happened:

• A) The removal of %kernel.root_dir% is tricky because the watermark and paste filters rely on it. To help with this, I deprecated both of these (in favor of watermark_image and paste_image using a compile-time deprecation so they will see it on any page (not just on some background image request). In Symfony 5.0, this deprecation turns into an exception. That means there IS a small BC break: if you upgrade this bunde AND upgrade to Symfony 5.0 at the same time, you will get a compile-time error. This will be SUPER obvious. And so, I think it is acceptable. I have tested this layer in a real project.

*B) Added code to work around some PHPUnit deprecations and the :void signature change.

This should be good. The suite tests 3.4, 4.4 and 5.0.

Thanks!

[conradfr]

Hi,

I try to use your branch on a Symfony5 project but I got an error :

"The service "liip_imagine.filter.manager" has a dependency on a non-existent service "liip_imagine.filter.loader.paste"."

Am I missing something?

[npotier]

Hello @conradfr I had the same error message as you.

I think that the problem comes from the compiler pass that removes some services if the kernel.root_dir does not exists.

This file has been added by @weaverryan with his merge request :

// File DependencyInjection/Compiler/NonFunctionalFilterExceptionPass.php
$canFiltersStillFunction = $container->hasParameter('kernel.root_dir');
//...
// remove the definitions entirely if kernel.root_dir does not exist
if (!$canFiltersStillFunction) {
    $container->removeDefinition('liip_imagine.filter.loader.watermark');
    $container->removeDefinition('liip_imagine.filter.loader.paste');
}

As far as I understand, this problem could be solved if the NonFunctionnalFilterExceptionPass is called first while building the bundle :

// File : LiipImagineBundle.php 

class LiipImagineBundle extends Bundle
{
    public function build(ContainerBuilder $container)
    {
        parent::build($container);
        $container->addCompilerPass(new NonFunctionalFilterExceptionPass()); // here
        $container->addCompilerPass(new DriverCompilerPass());
        $container->addCompilerPass(new LoadersCompilerPass());
        $container->addCompilerPass(new FiltersCompilerPass());
        $container->addCompilerPass(new PostProcessorsCompilerPass());
        $container->addCompilerPass(new ResolversCompilerPass());
        $container->addCompilerPass(new MetadataReaderCompilerPass());
        // $container->addCompilerPass(new NonFunctionalFilterExceptionPass()); // not here

        // ...

Doing this solved my problem 😉
Il would be happy to contribute if needed.

[michellesanver]

I'm now back at work. I plan to fix, test and merge this today and roll out a new release.

Happy new year and thanks a lot for wrapping this up @weaverryan! Let's start 2020 with SF 5 :-)

[michellesanver]

This is a really clear error message! Love this solution :)

[Guite]

@michellesanver I still get

In ParameterBag.php line 98:
                                                                               
  The service "liip_imagine.filter.loader.paste" has a dependency on a non-ex  
  istent parameter "kernel.root_dir". Did you mean one of these: "kernel.proj  
  ect_dir", "kernel.cache_dir", "kernel.logs_dir"

when switching from https://github.com/Guite/LiipImagineBundle/tree/symfony5 back to https://github.com/liip/LiipImagineBundle/tree/symfony5

Any idea?

[dbu]

did you clear the cache in between? afaik symfony sometimes is confused when switching branches in vendor

[Guite]

did you clear the cache in between? afaik symfony sometimes is confused when switching branches in vendor

Thanks, that was it apparently :-)

[dbu]

Thanks, that was it apparently :-)

phew. thanks for trying out the changes! if you find anything else not working, please let us know 👍

[weaverryan]

Thank you! What a nice surprise to see this merged today INCLUDING a fix (and patch) reported by other users and completed by Michelle. You all made my day!

[selimppc]

Following links are not found:

https://github.com/liip/LiipImagineBundle/tree/symfony5
https://github.com/weaverryan/LiipImagineBundle/tree/symfony5

someone please let me know the correct one

[dbu]

this has been merged to master in #1259, symfony 5 support is available in the master branch since then.

[selimppc]

@dbu I got a problem with symfony5

would you please take a look: https://stackoverflow.com/questions/63377282/symfony-5-could-not-find-configuration-for-a-filter-in-liip-liipimaginebundl

[dbu]

sorry, i am not that deep into the bundle, just getting notifications and trying to help out.

if you track this down to a bug in the bundle, please open a new issue or a merge request.