Version check causes Twig Environment class to be loaded early

[ekes]

The test for the Twig version in bootstrap.php is loading the Environment class early, even when just parsing for code analysis. I'm just starting to look at this, and I think I'm right the test doesn't happen early in the v2 (master) branch, only v1 used by Drush? So am wondering if there is already a good alternative?

I stumbled onto the issue because of phpactor/phpactor#1043

[Chi-teck]

Right v2, does not use bootstrap.php also it depends on Twig 2+,

[ekes]

But nothing in v2 that could be moved into v1 that could avoid it actually loading the class?

Maybe querying composer might be an alternative?

[Chi-teck]

v1 supports Drupal 8 and Drupal 9, which means it should be compatible with Twig 1 and Twig 2.
v2 only supports Drupal 9, so that no need to check Twig versions.
Not sure what you mean by querying composer.

[ekes]

The way v1 is currently checking the Twig version means that it actually autoloads Twig\Environment. If you are inspecting the code rather than running it - for example as code in the linked issue does - it means that if the code for the inspector needs a different Twig\Environment it's stuck because it's already been loaded.

I'd missed that v2 is D9 only, sorry. Either way I was then looking for another way of finding out what version of Twig is available for drupal-code-generator so it doesn't actually load Twig\Environment before it is really needed. Just one thought that occurred was maybe it's possible to query Composer for the defined version.

[Chi-teck]

As far as I know Composer does not offer any API to check package versions.
Checking class constants is intended way to figure out package versions.
The best way to handle such issues is to avoid using multiple autoloaders.
https://pantheon.io/blog/trouble-two-autoloaders

[ekes]

I'm not sure I understand the point about two autoloaders. The inspector code is using composer, but as I understand it it does something with the autoloader to be able to safely inspect the other code. The inspection code isn't mine, so I don't grok exactly how it works. If you mean there is a way of inspecting your code without causing it to be impossible to use Twig 2 in the inspector code could you add it to the issue above maybe the developer will understand?

[Chi-teck]

I don't know how phpactor works internally. I guess it cannot be installed locally on Drupal 8 site because of Twig dependency. And for global installation problems like this are quite typical.

[dantleech]

You can use https://github.com/Ocramius/PackageVersions maybe to check the Twig version?

[Chi-teck]

I don't like a new dependency just for checking Twig version. The alternative solution could be using some factory method instead of class aliases. But this must be done in Drush too.

[LeoActency]

We have a similar problem on a Drupal project for which we want to use Twig 3.

The line list($twig_major_version) = sscanf(Environment::VERSION, '%d.%d.%d'); in src/bootrstrap.php loads the Twig 2 environment and this conflicts with our implementation of Twig 3.

To work around the problem we have applied the following patch so Twig 2 is loaded only in a CLI context and do not conflicts with our usage of Twig 3 for the projet :

diff --git a/src/bootstrap.php b/src/bootstrap.php
index 0cfb078..7e4ca3f 100644
--- a/src/bootstrap.php
+++ b/src/bootstrap.php
@@ -52,21 +52,23 @@ function dcg_get_twig_environment($loader) {
   return $environment;
 }

-// Determine major Twig version.
-// \Twig\Environment::MAJOR_VERSION is not suitable here because of
-// https://github.com/twigphp/Twig/pull/2945
-// Use this workaround as drupal/drupal is locked on Twig 1.38.
-list($twig_major_version) = sscanf(Environment::VERSION, '%d.%d.%d');
+if (defined('STDIN') || in_array(PHP_SAPI, ['cli', 'cli-server', 'phpdbg'], TRUE)) {
+  // Determine major Twig version.
+  // \Twig\Environment::MAJOR_VERSION is not suitable here because of
+  // https://github.com/twigphp/Twig/pull/2945
+  // Use this workaround as drupal/drupal is locked on Twig 1.38.
+  list($twig_major_version) = sscanf(Environment::VERSION, '%d.%d.%d');

-// \Twig\Environment::tokenize() signature has been changed in Twig 2, so that
-// it is not possible to maintain the same \Twig\Environment sub-class for both
-// Twig versions.
-$twig_environment_class = sprintf('DrupalCodeGenerator\Twig\Twig%dEnvironment', $twig_major_version);
-if (!class_exists('DrupalCodeGenerator\Twig\TwigEnvironment')) {
-  class_alias($twig_environment_class, 'DrupalCodeGenerator\Twig\TwigEnvironment');
-}
+  // \Twig\Environment::tokenize() signature has been changed in Twig 2, so that
+  // it is not possible to maintain the same \Twig\Environment sub-class for both
+  // Twig versions.
+  $twig_environment_class = sprintf('DrupalCodeGenerator\Twig\Twig%dEnvironment', $twig_major_version);
+  if (!class_exists('DrupalCodeGenerator\Twig\TwigEnvironment')) {
+    class_alias($twig_environment_class, 'DrupalCodeGenerator\Twig\TwigEnvironment');
+  }

-// Legacy TwigEnvironment class is still used in Drush.
-if (!class_exists('DrupalCodeGenerator\TwigEnvironment')) {
-  class_alias($twig_environment_class, 'DrupalCodeGenerator\TwigEnvironment');
+  // Legacy TwigEnvironment class is still used in Drush.
+  if (!class_exists('DrupalCodeGenerator\TwigEnvironment')) {
+    class_alias($twig_environment_class, 'DrupalCodeGenerator\TwigEnvironment');
+  }
 }

Can this be possible as a fix for the 1.x versions of this package or what do you recommend to fix this problem?

[Chi-teck]

@LeoActency DCG 1 is compatible with Twig 3.

The issue only appears when you are trying to bootstrap Drupal with some external tool which has Twig 2 dependency (i.e. Drush 8 packaged in phar file) .

[Chi-teck]

Technically, since Drush has stopped using aliased Twig environment class (drush-ops/drush#4462), we can remove that in DCG 1 as well.

[LeoActency]

@Chi-teck

In our case we want to use Twig 2 and Twig 3 for different parts of our Drupal project.
We manage to load the correct version depending the context.

But this line in src/boostrap.php is automatically loading Twig 2 (through composer autoloading) even if we are not using Drush neither DCG in the context.

[Chi-teck]

DCG 1 is compatible with Twig 3.

It's my bad. DCG 1 is compatible with Twig 1 || 2. DCG 2 is compatible with Twig 2 || 3.

[Chi-teck]

We manage to load the correct version depending the context.

How do you load them? There is no Drupal version that supports Twig 3.

[LeoActency]

It's a kind of Drupal headless for the Front Office which uses Twig 3.
And for the Back Office we continue to use Twig 2.

So we get rid of Drupal's dependence on Twig 2 in our case, there is only DCG which remains blocking through Drush dependency.

I think the point is :

How to make sure that DCG does not load a particular version of the twig environment when composer is performing autoload. So that it is still possible to use different versions of the twig environment within the same project / in different contexts.

[Chi-teck]

Here is the fix plan:

1. Remove Twig aliases from bootstrap.php
2. Use Twig factory function in Appliation same way as in Drush.
3. Declare a conflict with drush/drush < 10.3.2 in composer.json

[Chi-teck]

Fixed in DCG 1.33. Thank you for the report.