Closed (fixed)
Project:
Drupal core
Version:
10.2.x-dev
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
9 Oct 2023 at 10:15 UTC
Updated:
28 Feb 2024 at 14:38 UTC
Jump to comment: Most recent
Comments
Comment #2
spokjeComment #4
spokjeOk, 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.Comment #5
spokjeWhat Have We (or at least I) Learned So Far:
- SF6.4 deprecates
ContainerAwareTraitandContainerAwareInterface.This affects:
Out of those
seem likely candidates for becoming Service Subscribers?
-
ComponentsIsolatedBuildTestdoesn't like stability lowering-
ComposerProjectTemplatesTest::testTemplateCreateProjectseems to have problems with putting the set stability level into the version string.-
ComposerProjectTemplatesTest::testMinimumStabilityStrictnessseems to be having problems with not finding any 'dev' stability level since they are all in the excluded-from-comparingdrupal/core.EDIT: Seems like
\Composer\Semver\VersionParser::parseStabilitydoesn't parse SF's verion-string of6.4.x-dev 3f5752fcorrectly and returns it asstable(#3392814: testMinimumStabilityStrictness struggles with dev versions )Comment #6
spokjeComment #7
spokjeComment #8
catchSome of these can probably be service subscribers, but
ClassResolverespecially, definitely can't, so I think we need to forkContainerAwareTraitor otherwise keep things more or less the same for that.Comment #9
bradjones1Updating 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:
Comment #12
longwaveWe 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.
Comment #13
spokjeRebased as-is.
Comment #14
spokjeComment #15
longwaveThanks! Retitling this issue again as the remaining SF7 prep work will have to happen in followups.
Comment #16
berdirShould 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?
Comment #17
longwaveThe 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.
Comment #18
longwaveLet's open some followups grouped by type of deprecation if we can.
Comment #19
spokjeCan I have some "blessing" on splitting off an issue for converting
BookNavigationCacheContextandMenuActiveTrailsCacheContextinto 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:
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 :)
Comment #20
longwaveYep 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.
Comment #24
lauriiiRebased 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.
Comment #26
spokjeCreated child issues, remove tag.
Comment #27
catchMoving back to fixed, I came here to commit this ;)
Comment #28
spokjeDamn keeping tabs open for too long and not refreshing!
Sorry, also reverted the version back to 10.2.x-dev.
Comment #29
wim leers@Spokje:
I can review those when you get to them! 👍
Comment #31
bkosborne