Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop support for PHP < 7.1 and Symfony < 3.4 #892

Merged
merged 9 commits into from Dec 23, 2018

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Dec 10, 2018

As per discussion in Slack and #885, this PR removes support for PHP < 7.1 (as they are no longer supported) as well as Symfony < 3.4 (since they are in security support only). This will target the 1.11 minor release.

The remaining build failure is due to a deprecation which isn't ignored in the Symfony 3.4 build. I have no idea which dependency combination causes that, so if anyone would have a few pointers for me, they'd be very much appreciated.

@alcaeus alcaeus added this to the 1.11.0 milestone Dec 10, 2018
@alcaeus alcaeus self-assigned this Dec 10, 2018
.travis.yml Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@alcaeus
Copy link
Member Author

alcaeus commented Dec 11, 2018

@doctrine/team-symfony-integration Anyone know where the deprecation in https://travis-ci.org/doctrine/DoctrineBundle/jobs/466345293 is coming from? It only happens in the Symfony 3.4 build where we lock all our explicit dependencies to Symfony 3.4 (but allow newer versions of other dependencies). I can't quite figure it out what's triggering it...

@greg0ire
Copy link
Member

@alcaeus you could try getting a stack trace by using SYMFONY_DEPRECATIONS_HELPER=/MessageSelector/

@alcaeus
Copy link
Member Author

alcaeus commented Dec 11, 2018

Thanks for the hint! I traced it to the service container but I'm not sure what the root cause is:

The "Symfony\Component\Translation\MessageSelector" class is deprecated since Symfony 4.2, use IdentityTranslator instead.
Stack trace:
#0 vendor/symfony/translation/MessageSelector.php(14): trigger_error('The "Symfony\\Co...', 16384)
#1 vendor/composer/ClassLoader.php(444): include('/Users/abraun/C...')
#2 vendor/composer/ClassLoader.php(322): Composer\Autoload\includeFile('/Users/abraun/C...')
#3 [internal function]: Composer\Autoload\ClassLoader->loadClass('Symfony\\Compone...')
#4 /private/var/folders/rn/1wpg9zfn4gbd84dl26xzbfzm0000gn/T/sf_kernel_15f0720cc98d408ed5540a9748774e29/cache/test/ContainerB38ugye/sf_kernel_15f0720cc98d408ed5540a9748774e29TestDebugProjectContainer.php(567): spl_autoload_call('Symfony\\Compone...')
#5 /private/var/folders/rn/1wpg9zfn4gbd84dl26xzbfzm0000gn/T/sf_kernel_15f0720cc98d408ed5540a9748774e29/cache/test/ContainerB38ugye/getValidator_BuilderService.php(21): ContainerB38ugye\sf_kernel_15f0720cc98d408ed5540a9748774e29TestDebugProjectContainer->getTranslator_DefaultService()
#6 /private/var/folders/rn/1wpg9zfn4gbd84dl26xzbfzm0000gn/T/sf_kernel_15f0720cc98d408ed5540a9748774e29/cache/test/ContainerB38ugye/sf_kernel_15f0720cc98d408ed5540a9748774e29TestDebugProjectContainer.php(346): require('/private/var/fo...')
#7 /private/var/folders/rn/1wpg9zfn4gbd84dl26xzbfzm0000gn/T/sf_kernel_15f0720cc98d408ed5540a9748774e29/cache/test/ContainerB38ugye/getValidator_Mapping_CacheWarmerService.php(8): ContainerB38ugye\sf_kernel_15f0720cc98d408ed5540a9748774e29TestDebugProjectContainer->load('getValidator_Bu...')
#8 /private/var/folders/rn/1wpg9zfn4gbd84dl26xzbfzm0000gn/T/sf_kernel_15f0720cc98d408ed5540a9748774e29/cache/test/ContainerB38ugye/sf_kernel_15f0720cc98d408ed5540a9748774e29TestDebugProjectContainer.php(346): require('/private/var/fo...')
#9 /private/var/folders/rn/1wpg9zfn4gbd84dl26xzbfzm0000gn/T/sf_kernel_15f0720cc98d408ed5540a9748774e29/cache/test/ContainerB38ugye/getCacheWarmerService.php(9): ContainerB38ugye\sf_kernel_15f0720cc98d408ed5540a9748774e29TestDebugProjectContainer->load('getValidator_Ma...')
#10 vendor/symfony/http-kernel/CacheWarmer/CacheWarmerAggregate.php(43): ContainerB38ugye\sf_kernel_15f0720cc98d408ed5540a9748774e29TestDebugProjectContainer->{closure}()
#11 vendor/symfony/http-kernel/Kernel.php(554): Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerAggregate->warmUp('/private/var/fo...')
#12 vendor/symfony/http-kernel/Kernel.php(123): Symfony\Component\HttpKernel\Kernel->initializeContainer()
#13 Tests/Command/ImportMappingDoctrineCommandTest.php(28): Symfony\Component\HttpKernel\Kernel->boot()
#14 vendor/phpunit/phpunit/src/Framework/TestCase.php(725): Doctrine\Bundle\DoctrineBundle\Tests\Command\ImportMappingDoctrineCommandTest->setup()
#15 {main}

Tests/ProfilerTest.php Outdated Show resolved Hide resolved
@alcaeus
Copy link
Member Author

alcaeus commented Dec 20, 2018

Thanks @ostrolucky for finding the cause of the deprecation. For now, I changed the deprecation helper config to report all deprecations without failing the build - this solves the issue regardless of whether it's fixed upstream.

@alcaeus alcaeus requested review from ostrolucky and a team December 20, 2018 06:39
@ostrolucky
Copy link
Member

How about setting SYMFONY_DEPRECATIONS_HELPER=1 instead? That would ignore the current deprecation, but alert us when count increases, which might be useful

Copy link
Member

@ostrolucky ostrolucky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please remove argument for HttpKernelExtension in setUp method of ProfilerTest? It seems it's no longer needed. I think it was for older versions too

.travis.yml Outdated Show resolved Hide resolved
@alcaeus
Copy link
Member Author

alcaeus commented Dec 20, 2018

@ostrolucky

Can you please remove argument for HttpKernelExtension in setUp method of ProfilerTest? It seems it's no longer needed. I think it was for older versions too

Will do 👍

How about setting SYMFONY_DEPRECATIONS_HELPER=1 instead? That would ignore the current deprecation, but alert us when count increases, which might be useful

I'd rather not have the build fail on deprecations in older builds to prevent regressions like the one before (where a newer version of a peer dependency triggers a deprecation). The tests against the latest version should fail when deprecations arise, so we should spot them soon enough 👍

.travis.yml Outdated Show resolved Hide resolved
@alcaeus alcaeus force-pushed the drop-old-stuff branch 2 times, most recently from 95d104b to a80af45 Compare December 20, 2018 16:55
@alcaeus
Copy link
Member Author

alcaeus commented Dec 20, 2018

@ostrolucky apparently that argument is still required, the build against the lowest set of dependencies fails without it.

@ostrolucky
Copy link
Member

Yeah, it installed TwigBridge 2.8 heh

DependencyInjection/DoctrineExtension.php Show resolved Hide resolved
DependencyInjection/DoctrineExtension.php Show resolved Hide resolved
DependencyInjection/DoctrineExtension.php Outdated Show resolved Hide resolved
DependencyInjection/DoctrineExtension.php Show resolved Hide resolved
DependencyInjection/DoctrineExtension.php Show resolved Hide resolved
DependencyInjection/DoctrineExtension.php Show resolved Hide resolved
Tests/ContainerTest.php Show resolved Hide resolved
Tests/ContainerTest.php Show resolved Hide resolved
@ostrolucky
Copy link
Member

Not worth doing job of cs fixers manually

@greg0ire
Copy link
Member

Is there a fixer for this?

@ostrolucky
Copy link
Member

@alcaeus
Copy link
Member Author

alcaeus commented Dec 22, 2018

@greg0ire I'll work on the todo, completely forgot about that, sorry.

As for CS, I'd like to avoid style changes in this PR. doctrine/coding-standard also doesn't have any line-length rules, so I'd like to direct discussion about that to https://github.com/doctrine/coding-standard so we can come up with consistent rules on that. Thanks!

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for the todo then

This configures the deprecation helper to not fail builds on all Symfony versions except the latest stable one. We always expect no deprecations when running against the latest versions of Symfony.
Copy link
Member

@kimhemsoe kimhemsoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should look into the CS afterwards

@alcaeus alcaeus merged commit ad278ef into doctrine:master Dec 23, 2018
@alcaeus alcaeus deleted the drop-old-stuff branch December 23, 2018 08:07
@greg0ire
Copy link
Member

I'm AFK for a week so cannot create the cs discussion, especially since it seems to mean creating a PR, but feel free to

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants