Update twig/twig from v1.25.0 to v1.32.0

We can only update to 1.32.0 because of a BC break https://github.com/twigphp/Twig/commit/f8d4c096ae32bc5ee642e628cad70d91e...

@slasher13, what BC break are you referring to? How empty works I'm guessing as the patch failure indicates.

For now we can make this change in 8.4 and look at a follow-up to tease out that BC break

Committed/pushed to 8.4.x, thanks!

Moving to 8.3.x and tagging for cherry-pick (but not doing that yet, will discuss with other committer on timing etc.).

It looks like 1.27.0 has had a BC break as well. Given that, I suggest that we don't backport this to 8.3.x.

That's fine with me, moving to fixed.

If there's a security release we might need to revisit it, but we can do that then.

We were running into a weird PHP on PHP 5.5 due to some performance optimizations that is quite easy to reproduce by accessing something like node.some_field.0 in a twig templates.

See https://github.com/twigphp/Twig/issues/2447 for the fun details.

After discussing a bit with @alexpott and seeing various comments here about BC breaks in various minor updates, Re-opening this to discuss reverting and enforcing this:

Note that our composer.json doesn't prevent anyone from installing those newer versions, if we on explicitly do not update, then we should possibly explicitly lock it down in composer.json as well, otherwise anyone who is using drupal/core with composer gets the latest versions anyway.

It looks like this is still in 8.4.x. Can we clarify what the current status is? I don't quite follow what @Berdir is suggesting in #13. (Maybe there is a missing code example or paragraph?)

The bug that I was reported was fixed, so by updating again to the latest version, we should be OK.

What I meant is that we should not just update the version that we happen to provide in our lock file. We should update the composer.json file to ensure everyone is using a compatible version.

Meaning, updating our constraint in core/composer.json from '"twig/twig": "^1.23.1",' to '"twig/twig": "^1.33.2", as that is the version that contains the fix for the bug I reported.

It's worth nothing that composer now actually updates to the latest 1.x version, which is 1.34.4. We could either update composer.json also to that or we could use --prefer-lowest to get exactly that version. But anyone using drupal-composer without drupal-core-strict will get 1.34.4 anyway.

We could also open a new issue to do that, as technically, this isn't matching the issue title anymore.

Hm, looks like an empty Markup object is no longer considered empty.

+++ b/core/tests/Drupal/KernelTests/Core/Theme/TwigMarkupInterfaceTest.php
@@ -57,7 +57,6 @@ public function providerTestMarkupInterfaceEmpty() {
-      'empty SafeMarkupTestMarkup' => ['<span></span>', SafeMarkupTestMarkup::create('')],

After https://github.com/twigphp/Twig/pull/2420 this assertion is not correct anymore. Instead of removing this use case, could we update this with the new expected value (empty string)

Would be happy to get feedback from one of the release managers related to the version constraints and BC breaks in the upstream package.

I do like the suggestion from #14 about adding an explicit test. I am hesitant to backport this during RC because the test assertion shows there's a BC break.

Is the version of Twig we're currently on broken with the bug in #12? Really a new issue should have been created from #11 on I think.

Does #19 about the empty markup object happen on the minimum version with the fix for https://github.com/twigphp/Twig/issues/2447? If it's introduced in 1.34 instead of 1.33, we could update to 1.33.2 for this release to be safer, and do the 1.34 update in 8.5.x only with a CR about the BC breaks. But if the BC is in 1.33.2 already then that does not help us.

Twig really needs to stop breaking BC in minor versions...

The BC break is on 1.33.0 so, unfortunately, that is not an option.

The BC is a bug fix, I think we should move to 1.35, in a new issue because this one has been committed in #8.

Created follow-up for #29 #2912542: Update twig/twig from v1.32.0 to v1.35.0

Still makes sense to upgrade before release

Restoring correct issue title and metadata.