Repository navigation
Document the Twig conflict when an application loads its own Twig - #2688
Sharawey74 wants to merge 2 commits into
Conversation
Twig autoloads four files that only declare deprecated global functions such as twig_cycle(). SimpleSAMLphp does not use them, but when an application that embeds SimpleSAMLphp loads its own copy of Twig, the second copy fails with "Cannot redeclare function twig_cycle()". Drop these includes from the generated Composer autoloader when the release tarballs are built, and fail the build if any are left. Fixes simplesamlphp#2687
|
I'm not sure I follow.. Is the problem you're trying to fix here that SimpleSAMLphp and your application use different versions of Twig? |
Not quite, it's two copies of Twig in the same PHP process, even with the same version. In the issue, the application loads Debian's Twig from /usr/share/php/Twig and the tarball brings its own in vendor/. Twig's src/Resources/*.php files declare global functions such as twig_cycle() without a function_exists() guard, so the second autoloader to include them fails with "Cannot redeclare". Twig won't change that in 3.x (twigphp/Twig#4180 was closed; the functions are removed in Twig 4). I agree that editing vendor/ is not nice; I followed the solution proposed in #2687. If you prefer, I can drop the build change and instead document that an application with its own Twig should install SimpleSAMLphp with Composer rather than use the tarball, so only one Twig is loaded. @jornane, does that work for your setup? |
To me this would be the preferred solution, yes. Either this, or you could run your application and SimpleSAMLphp in different PHP-FPM pools so they don't interfere? |
As discussed in the PR, editing files in vendor/ is not the way to solve this. Revert the release-workflow step and document the problem instead. An application that loads its own copy of Twig and then the autoloader of a SimpleSAMLphp release archive gets two copies of Twig in one PHP process, and PHP stops with "Cannot redeclare function twig_cycle()". The SP integration docs now explain the error and the two ways to avoid it: install SimpleSAMLphp as a Composer dependency of the application, or run the application and SimpleSAMLphp in different PHP-FPM pools. Fixes simplesamlphp#2687
Sharawey74
changed the title
As suggested, I've dropped the build change and turned this into a documentation update: a short section in the SP integration docs about the "Cannot redeclare twig_cycle()" error, with both options, installing SimpleSAMLphp with Composer or running it in a separate PHP-FPM pool. @jornane, happy to adjust if your setup needs more. |
Fixes #2687
As discussed above, this PR no longer changes the release build or the
vendor/directory. It is now a documentation change only.Change
A short section, "Applications that use Twig themselves", at the end of "Integrating authentication with your own application" in
docs/simplesamlphp-sp.md, which is where an application loads SimpleSAMLphp's autoloader. It covers:PHP Fatal error: Cannot redeclare function twig_cycle();composer require simplesamlphp/simplesamlphp), so Composer installs one Twig for both;The release-workflow step from the first commit is reverted, so the diff against
masteris only the documentation (+29 lines in one file).Checks
markdownlintwith the repository's.markdownlintrcpasses on the changed file.