Support for mapping keys in multi-line blocks is deprecated since Symfony 4.3 and will throw a ParseException in 5.0.
Sites installed prior to Drupal 9.1.0 that are using a customized version of default.services.yml may need to update the YAML syntax in that file. See the change record on the yaml syntax change for more information.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3055189-14.patch | 2.89 KB | catch |
| #14 | 3055189-interdiff.txt | 868 bytes | catch |
| #10 | interdiff.3055189.5-10.txt | 1.25 KB | mikelutz |
| #10 | 3055189-10.drupal.patch | 2.04 KB | mikelutz |
| #5 | 3055189-5.drupal.PATCH_TO_COMMIT.patch | 816 bytes | mikelutz |
Comments
Comment #2
mikelutzTest Patch
Comment #3
mikelutzComment #5
mikelutzComment #6
catchCouldn't be more straightforward.
Comment #7
alexpottSo now I remember. We've come across this before - #2945275: Remove hack to fix bug in symfony/yaml
So the problem here is that making this change here is simple BUT we've got to cope with the sites/default/services.yml files that have been generated from this file on install. So we need to read the files and see if they have this format and then tell people to fix them.
Comment #9
alexpottWe also need to remove the line
'Support for mapping keys in multi-line blocks is deprecated since Symfony 4.3 and will throw a ParseException in 5.0.',from \Drupal\Tests\Listeners\DeprecationListenerTrait::getSkippedDeprecations()
Comment #10
mikelutzDo we need to though? If a generated file has this format, then it will trigger the error from the parser. That may be enough.
Otherwise, we will need to replicate the parser logic to detect the error and surface it to the status page. Even if we choose to do that, the sooner that this patch gets in, the fewer sites will be affected by this come Drupal 10 time, so I still suggest putting this in 8.9 and 9.0 and writing code to surface the issue to the status page in a 9.0.x only follow up.
Comment #11
alexpottI see what you mean. They'd only can the deprecation reported if they had test coverage with deprecation testing enabled. But perhaps by the time we get to Sf5 / D10 we'll be better at surfacing these things in the UI.
On balance I think you're right. I think there's no harm in a CR that tells people to about their services.yml files if they copied this.
Comment #12
mikelutzI believe the patch from #5 still works for 8.9.x.
It would be nice to be able to optionally surface runtime deprecation errors to the UI at some point, but I think the fact that we are triggering an error is enough right now. I'll whip up a quick CR.
Comment #13
xjmComment #14
catchFixing the scaffold test failure in #10.
Bumping to critical since this blocks Drupal 10.
Comment #15
catchAdded a change record and release notes snippet.
Comment #16
longwaveI don't think there is anything left to do here.
Comment #17
alexpottThis one is easier now that we have the scaffold plugin as that'll fix this for a lot of existing sites.
Committed 67a054c and pushed to 9.1.x. Thanks!
Comment #18
nickdickinsonwildeUpdated change record to refer to
default.services.ymlrather than the non-existentdefault.settings.ymlComment #21
alexpottTagging for 9.1.0 as from then on a sites default.services.yml might cause a deprecation.
Comment #22
xjmThe release note shouldn't reference an issue -- maybe it's supposed to be a link to the CR? Otherwise we should take whatever info is important and just put it in the release note.
Comment #23
alexpottFixed the link
Comment #24
xjmThe release note says
default.settings.yml, but the CR saysdefault.services.yml. Which is it?Meanwhile, making the link accessible.
Comment #25
alexpottGood spot. It’s default.services.yml - fixed accordingly.
Comment #26
xjmRemoving beta target tag that was leftover from 8.9/9.0.