search

sonata-project / SonataCoreBundle

[deprecated] SonataCoreBundle
MIT License
318 stars 138 forks source link

Allow symfony/dependency-injection 4.4.0 #720

Closed fbourigault closed 5 years ago

fbourigault commented 5 years ago

I did some investigation about why the build is failing when symfony/dependency-injection 4.4.0 is installed.

After a git bisect session, it reported to me that https://github.com/symfony/symfony/pull/32332 is responsible for the failed build.

I don't know how to fix it.

EDIT:

I run the git bisect like this

composer update
rm -r vendor/symfony/dependency-injection
composer install --prefer-source
cd vendor/symfony/dependency-injection
git bisect start v4.4.0 v4.3.8
export SYMFONY_DEPRECATIONS_HELPER=max[self]=0
git bisect run sh -c 'cd ../../.. && make test'
greg0ire commented 5 years ago

Thank you for this!

greg0ire commented 5 years ago

My hunch is that we have to change the priority of https://github.com/sonata-project/SonataCoreBundle/blob/3.x/src/CoreBundle/DependencyInjection/Compiler/AdapterCompilerPass.php, not sure to what. @alexpott please advise

fbourigault commented 5 years ago

I'm not sure about the git bisect. Before this PR, the CheckExceptionOnInvalidReferenceBehaviorPass was registered as a removing pass. After it is registered as an after removing pass.

As AdapterCompilerPass is registered as a before optimization pass, https://github.com/symfony/symfony/pull/32332 should not change anything.

:thinking:

EDIT:

I removed by hand the https://github.com/symfony/symfony/pull/32332/files#diff-951ca6c937d7f2e6ab0b88822700a12cR88-R90 lines and test passes.

IMHO, I miss some DI knowledge!

greg0ire commented 5 years ago

I'm reassured: git bisect does not lie :P

fbourigault commented 5 years ago

I found why this is now failing!

Here are the compiler passes executed when $container->compile() is called.

v4.3.8:

Symfony\Component\DependencyInjection\Compiler\MergeExtensionConfigurationPass
Symfony\Component\DependencyInjection\Compiler\ResolveClassPass
Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass
Symfony\Component\DependencyInjection\Compiler\RegisterEnvVarProcessorsPass
Sonata\CoreBundle\DependencyInjection\Compiler\AdapterCompilerPass
Symfony\Component\DependencyInjection\Compiler\ExtensionCompilerPass
Symfony\Component\DependencyInjection\Compiler\ResolvePrivatesPass

v4.4.0:

Symfony\Component\DependencyInjection\Compiler\MergeExtensionConfigurationPass
Symfony\Component\DependencyInjection\Compiler\ResolveClassPass
Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass
Symfony\Component\DependencyInjection\Compiler\RegisterEnvVarProcessorsPass
Sonata\CoreBundle\DependencyInjection\Compiler\AdapterCompilerPass
Symfony\Component\DependencyInjection\Compiler\ExtensionCompilerPass
Symfony\Component\DependencyInjection\Compiler\ResolvePrivatesPass
Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass
Symfony\Component\DependencyInjection\Compiler\ResolveHotPathPass

As you can see the two passes that where moved from remove passes to after remove passes are now executed.

This is because AbstractContainerBuilderTestCase::setUp() reset the removing passes but not the after removing passes (see https://github.com/SymfonyTest/SymfonyDependencyInjectionTest/blob/master/PhpUnit/AbstractContainerBuilderTestCase.php#L18-L23).

Fix is coming.