Problem/Motivation

We added a TwigExtensionPass in #2544932: Twig should not rely on loading php from shared file system to generate a hash for all extensions and handle when Twig extensions change (when modules/core update, etc.).

While working on #2568171: Upgrade to Twig 1.22 and implement our own cache class @znerol found that Twig 1.22 adds its own handling for when Twig extensions change, which probably means we don't need our custom code anymore. See https://github.com/twigphp/Twig/pull/1824.

Proposed resolution

Try to remove TwigExtensionPass and the associated code.

Remaining tasks

  • Patch
  • Review

User interface changes

n/a

API changes

Removal of TwigExtensionPass.

Data model changes

n/a

Comments

Cottser created an issue. See original summary.

Cottser’s picture

Status: Active » Closed (works as designed)

After spending some time with this I think their handling could help us but it doesn't really work with modules being installed/uninstalled, only with file mtime. If I've overlooked something we can re-open but I think we still need our twig_extension_hash because modules can provide Twig extensions that end up changing the compiled Twig template.

Additionally, isTemplateFresh() is only called when Twig auto_reload is enabled which would not be the case on production environments.

Where I think this could help is in making extension development easier because it would integrate better with auto_reload.

    public function isTemplateFresh($name, $time)
    {
        foreach ($this->extensions as $extension) {
            $r = new ReflectionObject($extension);
            if (filemtime($r->getFileName()) > $time) {
                return false;
            }
        }

        return $this->getLoader()->isFresh($name, $time);
    }
Cottser’s picture

So evidently I looked at the wrong PR this morning but I think there still may be a case which is covered now and would not be covered with the latest upstream code:

  1. Twig auto_reload disabled
  2. A module updates one of its Twig extensions

Currently that would result in a different twig_extension_hash (because of the extension mtime) and a different cache folder for the Twig template.

If we remove the compiler pass, I think the template would only get updated if auto_reload is enabled or caches are cleared.