-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[TwigBridge] ensure compatibility with Twig 3.15 #58649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
dd8f24f
to
a2decdb
Compare
@@ -51,8 +53,8 @@ public function enterNode(Node $node, Environment $env): Node | |||
return $node; | |||
} else { | |||
$var = $this->getVarName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just discovered that this node visitor breaks reproducible compilation here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, let's fix this explicitly in a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed that this was fixed for 7.2 in #57609. Should we backport that PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that PR is actually broken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's backport (and fix the broken part) in 5.4 yes
reproducible builds FTW
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A way to generate a reproducible name that does not clash with parent or child template could be to use a hash of the template name in the variable name (with the new hardcoded name as prefix)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #58706
54684fa
to
fd70afd
Compare
I updated the Though I am stuck at how we can deal with a similar case in the |
I think I got a solution for the Status: Needs Review |
Thank you @xabbuh. |
take the changes from twigphp/Twig#4398 into account