- Issue created by @spokje
- 🇳🇱Netherlands spokje
Ok, 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_length
andsession.sid_bits_per_character
config 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).
- 🇬🇧United Kingdom longwave UK
Thanks for opening and working on this. If possible we should land this in 11.1.0-alpha1 this week.
- 🇳🇱Netherlands 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.
- 🇬🇧United Kingdom longwave UK
The Settings deprecations are for things that go in the
global $settings
usually defined in settings.php; these are container parameters but not sure we have a way to deprecate those... - 🇬🇧United Kingdom longwave UK
@catch already opened 📌 Remove sid_length and sid_bits_per_character from default.services.yml Active following the PHP 8.4 deprecation although nothing stopping us solving that here.
- 🇳🇱Netherlands spokje
@catch already opened #3471199: Remove sid_length and sid_bits_per_character from default.services.yml following the PHP 8.4 deprecation although nothing stopping us solving that here.
Ah, i thought the removal was such an unique issue, I never even tried to search for it.
The Settings deprecations are for things that go in the global $settings usually defined in settings.php; these are container parameters but not sure we have a way to deprecate those...
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) - 🇫🇷France andypost
Change record should mention upgrade to SF 7.2 as the settings been deprecated in 📌 PHP 8.4 session.sid_length and session.sid_bits_per_character are deprecated Active
- 🇳🇱Netherlands spokje
Change record should mention upgrade to SF 7.2
Not sure why this isn't sufficient?
They will be deprecated in PHP 8.4 (https://wiki.php.net/rfc/deprecations_php_8_4) and Symfony 7.2 (https://github.com/symfony/symfony/pull/57805)
- 🇬🇧United Kingdom longwave UK
Re 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. - 🇫🇷France andypost
@spokje I mean there's already published CR → which could use update but #3484048 should said that core went to SF 7.2
- 🇳🇱Netherlands spokje
Re 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.
Thanks @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? - 🇳🇱Netherlands spokje
@spokje I mean there's already published CR which could use update but #3484048 should said that core went to SF 7.2
Thanks 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. - 🇳🇱Netherlands spokje
Ok, 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 - 🇳🇱Netherlands spokje
A left-open browser window made me change the priority in #10, restoring original status
- 🇬🇧United Kingdom longwave UK
Not 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.
Automatically closed - issue fixed for 2 weeks with no activity.