Closed (fixed)
Project:
Drupal core
Version:
11.x-dev
Component:
other
Priority:
Critical
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
27 Oct 2024 at 17:47 UTC
Updated:
14 Nov 2024 at 09:09 UTC
Jump to comment: Most recent
Comments
Comment #2
spokjeLet's see what breaks.
Comment #4
spokjeOk, so it seems to all boil down to https://github.com/symfony/symfony/pull/57805 where (as in PHP 8.4) both the
session.sid_lengthandsession.sid_bits_per_characterconfig options are deprecated for removal /ignoring in SF8.0.Remove all references to both, as I did in MR!9967 makes TestBot happy again.
Let's see if I can properly deprecate those settings in the next commit(s).
Comment #5
longwaveThanks for opening and working on this. If possible we should land this in 11.1.0-alpha1 this week.
Comment #6
longwaveComment #7
spokje@longwave Ah, wasn't aware we're already so close to the 11.1 alpha.
Anyway: Besides the two deprecations mentioned in #4 we should be fine.
I _was_ going to ask what status this issue should have, I was guessing postponed on the actual release of SF7.2.0, but seeing this might/should/could/will/must land in 11.1 alpha, I'll put this on NR.
Comment #8
longwaveThe Settings deprecations are for things that go in the
global $settingsusually defined in settings.php; these are container parameters but not sure we have a way to deprecate those...Comment #9
longwave@catch already opened #3471199: Remove sid_length and sid_bits_per_character deprecation from CoreServiceProvider following the PHP 8.4 deprecation although nothing stopping us solving that here.
Comment #10
spokjeAh, i thought the removal was such an unique issue, I never even tried to search for it.
Do we need something like
\Drupal\Core\DependencyInjection\Compiler\DeprecatedServicePass, but for deprecated parameters?(And yes, I'm blissfully unaware of most Containter-related magic)
Comment #11
spokjeReversed the incorrect deprecation attempt.
Comment #12
andypostComment #13
andypostChange record should mention upgrade to SF 7.2 as the settings been deprecated in #3465836: PHP 8.4 session.sid_length and session.sid_bits_per_character are deprecated
Comment #14
spokjeNot sure why this isn't sufficient?
https://www.drupal.org/node/3484048
Comment #15
longwaveRe the deprecation, maybe for this time we should just trigger directly from
CoreServiceProvider::register()if$container->hasParameter(...). We can refactor to make it more dynamic or reusable later, if we have to deprecate something else.Comment #16
andypost@spokje I mean there's already published CR which could use update but #3484048 should said that core went to SF 7.2
Comment #17
spokjeThanks @longwave
Makes sense, but:
1. It seems the SF-deprecation takes precedence, will try later with that one suppressed.
2. I think this needs a test? If so, where the *BLEEP* should I put it?
Comment #18
spokjeThanks for clearing that up @andypost, I completely misunderstood that one.
And that is a _mighty_ well written CR.
The only "problem" I have with it that it is for both 10.4.0 and 11.1.0, where SF7.2 only applies to 11.1.0.
I'm sure there's a very nice way to put that into the CR, but as a non native English speaker, I don't dare to touch it, it's to nice for me to break it.
Added https://www.drupal.org/project/drupal/issues/3483978 as the CR for this issue and tagged this issue for a CR update
Comment #19
spokjeOk, so at the very least now our own deprecation warnings show up, instead of the standard SF ones.
This now needs:
- A review
- Tests?
- A rewrite of the already published CR where we mention SF7.2, but only for 11.1.0
Comment #20
spokjeA left-open browser window made me change the priority in #10, restoring original status
Comment #21
longwaveNot sure this needs a specific change record, I looked back and I don't find that we wrote change records for previous dependency updates in most cases - this is more of a release notes thing, so tagging for that instead. The change record for the session configuration options looks great to me already, I don't think we need to make any additional notes.
Comment #23
catchCommitted/pushed to 11.x, thanks!