Problem/Motivation

Symfony 6.4 will be released in November 2023 (https://symfony.com/releases/6.4).

Steps to reproduce

Proposed resolution

Start updating now to Symfony 6.4 to see what deprecations, if any, we need to fix.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3392616

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Spokje created an issue. See original summary.

spokje’s picture

Assigned: Unassigned » spokje

spokje’s picture

Ok, so the first let's-see-what-breaks-run stops at PHPStan complaining about https://github.com/symfony/symfony/blob/6.4/UPGRADE-6.4.md#dependencyinj....

Let's see if anything else breaks if we temporarily suppress all of these deprecations by adding them to core/phpstan-baseline.neon.

spokje’s picture

What Have We (or at least I) Learned So Far:

- SF6.4 deprecates ContainerAwareTrait and ContainerAwareInterface.
This affects:

core/lib/Drupal/Core/Access/CheckProvider.php                       
core/lib/Drupal/Core/Cache/CacheFactory.php                           
core/lib/Drupal/Core/Cache/CacheTagsInvalidator.php                 
core/lib/Drupal/Core/Cache/ChainedFastBackendFactory.php            
core/lib/Drupal/Core/Cache/Context/MenuActiveTrailsCacheContext.php  
core/lib/Drupal/Core/DependencyInjection/ClassResolver.php          
core/lib/Drupal/Core/DrupalKernelInterface.php                  
core/lib/Drupal/Core/Entity/EntityTypeManager.php                   
core/lib/Drupal/Core/EventSubscriber/KernelDestructionSubscriber.php  
core/lib/Drupal/Core/Logger/LoggerChannelFactory.php                 
core/lib/Drupal/Core/Queue/QueueFactory.php                           
core/lib/Drupal/Core/StackMiddleware/Session.php                    
core/lib/Drupal/Core/StreamWrapper/StreamWrapperManager.php         
core/lib/Drupal/Core/Update/UpdateHookRegistryFactory.php           
core/lib/Drupal/Core/Update/UpdateRegistryFactory.php                 
core/modules/book/src/Cache/BookNavigationCacheContext.php          
core/modules/media_library/src/OpenerResolver.php                   
core/modules/system/tests/modules/render_placeholder_message_test/src/RenderPlaceholderMessageTestController.php  
core/modules/system/tests/modules/service_provider_test/src/TestClass.php  
core/tests/Drupal/Tests/Core/Controller/ControllerResolverTest.php  
core/tests/Drupal/Tests/Core/DependencyInjection/DependencySerializationTest.php  
core/tests/Drupal/Tests/Core/Utility/CallableResolverTest.php

Out of those

core/modules/book/src/Cache/BookNavigationCacheContext.php          
core/lib/Drupal/Core/Cache/Context/MenuActiveTrailsCacheContext.php  

seem likely candidates for becoming Service Subscribers?

- ComponentsIsolatedBuildTest doesn't like stability lowering
- ComposerProjectTemplatesTest::testTemplateCreateProject seems to have problems with putting the set stability level into the version string.
- ComposerProjectTemplatesTest::testMinimumStabilityStrictness seems to be having problems with not finding any 'dev' stability level since they are all in the excluded-from-comparing drupal/core.
EDIT: Seems like \Composer\Semver\VersionParser::parseStability doesn't parse SF's verion-string of 6.4.x-dev 3f5752f correctly and returns it as stable (#3392814: testMinimumStabilityStrictness struggles with dev versions )

spokje’s picture

Assigned: spokje » Unassigned
spokje’s picture

Status: Active » Needs work
catch’s picture

Some of these can probably be service subscribers, but ClassResolver especially, definitely can't, so I think we need to fork ContainerAwareTrait or otherwise keep things more or less the same for that.

bradjones1’s picture

Title: Update to Symfony 6.4 » Update to Symfony 6.4 / Symfony 7.0 readiness

Updating title to help with discoverability for both Symfony 6.4 and Symfony 7.0 searches.

Marking #3394694: [Meta] Symfony 7 compatibility as a duplicate, however I am including the following note and encouraging anyone who can speak to Drupal's position on postponements of strong typing to chime in:

Symfony 7 will include types on many methods, which requires that extending classes and interface implementations will need to be updated. There is an issue upstream for collecting feedback, and with compatibility-checking steps.

longwave’s picture

Issue tags: +Needs reroll

We are planning to release Drupal 10.2 with Symfony 6.4, so I think we should get this in as-is and then work on the new deprecations in followup issues.

I also think we need to figure out what to do with the Symfony 7 typing issue as we will have to follow suit for Drupal 11 and so we need to start making a plan for how to notify contrib/custom code now, that certainly deserves its own issue.

MR needs rebasing against 10.2.x/11.x so leaving at needs work for now.

spokje’s picture

Status: Needs work » Needs review

Rebased as-is.

spokje’s picture

Issue tags: -Needs reroll
longwave’s picture

Title: Update to Symfony 6.4 / Symfony 7.0 readiness » Update to Symfony 6.4
Status: Needs review » Reviewed & tested by the community

Thanks! Retitling this issue again as the remaining SF7 prep work will have to happen in followups.

berdir’s picture

Should we have follow-ups to deal with the ContainerAware deprecation (and others if there are, didn't check)? This is still beta, do we want to commit it like this or wait for a stable release?

We could either way start to work on the ContainerAware topic, likely with separate issues for any non-trivial conversion?

longwave’s picture

The stable Symfony release doesn't land until possibly the end of November, but we need to put out 10.2.0-beta1 before that and so it should be on the Symfony beta release as well; better than doing the upgrade afterwards. We've done this before for 10.0 and 10.1.

longwave’s picture

Issue tags: +Needs followup

Let's open some followups grouped by type of deprecation if we can.

spokje’s picture

Can I have some "blessing" on splitting off an issue for converting BookNavigationCacheContext and MenuActiveTrailsCacheContext into lazy services?

This won't do much with the whole ContainerAware deprecation, it just shifts it to another point.
But by the looks of it, these two services are ContainerAware just for the lazy loading, and looking at the time of creation of both, we didn't really have the lazy services concept up and running.
For consistency reasons, I think we should convert them into those, but unsure if the people in here agree, since we of course have to do a lot of construct deprecation dancing.

For all the other deprecations, I agree with @catch in #8:

so I think we need to fork ContainerAwareTrait or otherwise keep things more or less the same for that.

Forking ContainerAwareTrait into core seems the only way to me. As far as I can see all deprecation added in this issue come from this deprecation, so there's not much to split IMHO, but happy (and easy) to be convinced otherwise :)

longwave’s picture

Yep it looks like those two can be converted to using lazy services, feel free to spin off issues for that.

And yes I think realistically we need to fork ContainerAwareTrait as it seems unlikely we will be able to remove all uses of that or ContainerAwareInterface before Drupal 11, although we should still try and convert as much as possible to using dependency injection. Perhaps we need one issue to copy the trait/interface into core, and another meta issue to reduce use of ContainerAware classes where possible.

lauriii made their first commit to this issue’s fork.

  • lauriii committed d582b8c5 on 11.x
    Issue #3392616 by Spokje, longwave: Update to Symfony 6.4
    

lauriii’s picture

Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: +10.2.0 release notes

Rebased the MR because there was a conflict in the composer.lock. Only change needed to resolve the rebase conflict was to update the hash.

Committed d582b8c and pushed to 11.x. Also cherry-picked to 10.2.x. Thanks!

I didn't add anything to the release notes because there was already section that mentions the Symfony update.

  • lauriii committed c02c22f7 on 10.2.x
    Issue #3392616 by Spokje, longwave: Update to Symfony 6.4
    
    (cherry...
spokje’s picture

Version: 10.2.x-dev » 11.x-dev
Status: Fixed » Reviewed & tested by the community
Issue tags: -Needs followup

Created child issues, remove tag.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Moving back to fixed, I came here to commit this ;)

spokje’s picture

Version: 11.x-dev » 10.2.x-dev

Damn keeping tabs open for too long and not refreshing!

Sorry, also reverted the version back to 10.2.x-dev.

wim leers’s picture

@Spokje:

Can I have some "blessing" on splitting off an issue for converting BookNavigationCacheContext and MenuActiveTrailsCacheContext into lazy services?

I can review those when you get to them! 👍

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

bkosborne’s picture