Problem/Motivation

In #2927746: Update Symfony components to 3.4.* we had to add a hack to fix yaml before parsing because our default.services.yml had the following structure:

  factory.keyvalue:
    {}
    # Default key/value storage service to use.
    # @default keyvalue.database
    # default: keyvalue.database
    # Collection-specific overrides.
    # state: keyvalue.database
  factory.keyvalue.expirable:
    {}
    # Default key/value expirable storage service to use.
    # @default keyvalue.database.expirable
    # default: keyvalue.database.expirable
  # Allowed protocols for URL generation.

Proposed resolution

Remove the hack and update to the dev version of symfony/yaml 3.4 which contains https://github.com/symfony/symfony/pull/25787 - hopefully symfony will do a bugfix release before 8.5.0 but it is better to be on the dev release than have a preg_replace()

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#7 2945275-7.patch4.6 KBalexpott
#7 5-7-interdiff.txt458 bytesalexpott
#5 2945275-5.patch4.15 KBalexpott
#5 2-5-interdiff.txt1.04 KBalexpott
#2 2945275-2.patch4.81 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2945275-2.patch4.81 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

I asked @nicolasgrekas of Symfony when the next bugfix release for Symfony 3.4 is going to be. His reply was that it would be end of this month or very early next month. So there is a good chance that 3.4.5 will be released before Drupal 8.5.0 but it will be cutting it very fine.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

The difference between 3.4.4 and 3.4.x-dev version the patch gets is:

*   7bcccefb60 - (HEAD -> 3.4, origin/3.4) bug #25787 Yaml parser regression with comments and non-strings (alexpott) (52 minutes ago) <Christian Flothmann>
|\
| * a7e2a49a78 - Yaml parser regression with comments and non-strings (53 minutes ago) <Alex Pott>
* aa956636ab - [YAML] Issue #26065: leading spaces in YAML multi-line string literals (5 days ago) <Thomas Counsell>

So there are only 2 bugs fixes different - the one we need and a fix for https://github.com/symfony/symfony/issues/26065

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
FileSize
2-5-interdiff.txt1.04 KB
2945275-5.patch4.15 KB

We've now got a proper release 3.4.5 - so we can update to that. It only includes the same two bug fixes as detailed in #4.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Related issues: +#2949225: Update Symfony components to 3.4.5

The other option here is to update all the components. This is the minimal fix - updating all the components is done in #2949225: Update Symfony components to 3.4.5

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
FileSize
5-7-interdiff.txt458 bytes
2945275-7.patch4.6 KB

Hmmm... also need to update the constraint we are not compatible with anything lower than Symfony\Yaml 3.4.5

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Version: 8.6.x-dev » 8.5.x-dev

Discussed with @larowlan, @effulgentsia, @plach, @yoroy, @gabor.hojtsy, @webchick we agreed that this minimal fix should be targeted for 8.5.0 and we'll leave #2949225: Update Symfony components to 3.4.5 till 8.5.1 or later.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan as a volunteer and at PreviousNext commented
Status: Needs review » Reviewed & tested by the community

  • catch committed 77a1e09 on 8.6.x 
    Issue #2945275 by alexpott: Remove hack to fix bug in symfony/yaml

  • catch committed 1c38391 on 8.5.x 
    Issue #2945275 by alexpott: Remove hack to fix bug in symfony/yaml

(...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!

Status: Fixed » Closed (fixed)

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