Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
The drupal/core package does not require symfony/expression-language, because there is no real dependency regardless of the fact that Drupal\Component\DependencyInjection\Dumper\OptimizedPhpArrayDumper and Drupal\Tests\Component\DependencyInjection\Dumper\OptimizedPhpArrayDumperTest use Symfony\Component\ExpressionLanguage\Expression
From core/tests/Drupal/Tests/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumperTest.php
/**
* As Drupal Core does not ship with ExpressionLanguage component we need to
* define a dummy, else it cannot be tested.
*/
namespace Symfony\Component\ExpressionLanguage {
if (!class_exists('\Symfony\Component\ExpressionLanguage\Expression')) {
/**
* Dummy class to ensure non-existent Symfony component can be tested.
*/
class Expression {
/**
* Gets the string representation of the expression.
*/
public function __toString() {
return 'dummy_expression';
}
}
}
}
From core/lib/Drupal/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumper.php
elseif ($value instanceof Expression) {
throw new RuntimeException('Unable to use expressions as the Symfony ExpressionLanguage component is not installed.');
}
Proposed resolution
Remove symfony/expression-language from the list of required components in lib/Drupal/Component/DependencyInjection/composer.json
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2747083-8.patch | 649 bytes | Eric_A |
Comments
Comment #2
Eric_A CreditAttribution: Eric_A commentedComment #3
Eric_A CreditAttribution: Eric_A commentedComment #4
Eric_A CreditAttribution: Eric_A commentedComment #5
alexpottShouldn't we move this to suggest then? See https://getcomposer.org/doc/04-schema.md#suggest
Comment #6
Eric_A CreditAttribution: Eric_A commentedYes, probably a good idea.
Arguments against:
- Unlike Symfony\Component\DependencyInjection\Dumper\PhpDumper, there currently is no conditional behavior in Drupal\Component\DependencyInjection\Dumper\OptimizedPhpArrayDumper. It just throws a RuntimeException if an instance of Expression is passed.
Arguments in favor:
- It makes a little difference for the test. The fact that component tests live in drupal/core does not make a real difference, since in order to get subcomponents, chances are you'll do so by requiring drupal/core.
- Suggesting it has value from a documentation point of view. It will make people think before creating a bug report that the Expression class is being used but the component not being required.
Here's a patch that copies the suggestion from symfony/dependency-injection.
Comment #7
Eric_A CreditAttribution: Eric_A commentedReroll was needed because of #2744357: drupal/core-* components Symfony requirements conflict with drupal/core.
Comment #8
jibranLooks much better now.
Comment #9
Mile23In #2337283: Add a composer.json file to every component I determined these dependencies by looking at all the
usestatements in each component.If
symfony/expression-languageis not a dependency then a) we should never reference it inusestatements, and b) we shouldn't test against its existence.For instance,
OptimizedPhpArrayDumpersays:Which automatically means it's a dependency. If we're not expecting this class to exist, then when
dumpValue()says:...we should get a class not found fatal before we ever throw the
RuntimeException.Comment #10
Eric_A CreditAttribution: Eric_A commentedThe use statement makes class aliasing possible and doing run-time checks like
class_exists()andinstanceof, which fits the use case of composersuggest. There's no hard dependency, otherwise core would have crashed, given that we currently only process core/composer.json and not the components dependencies.Sure, there's room for improvement in drupal/core-dependency-injection, and we could make symfony/expression-language a hard drupal/core (dev) dependency, but that seems out of scope here.
Could you elaborate a bit on what work you feel needs to be done here?
Comment #11
Eric_A CreditAttribution: Eric_A commentedAlso, note the similarity in code with Symfony\Component\DependencyInjection\Dumper\PhpDumper. It's all runtime checking.
And in the symfony component it's a dev requirement, so it's only retrieved when the component is the root package, not when the component is a dependency.
Comment #12
Mile23OK, so I was thinking out of my butt a little bit there. :-)
suggestis good.And if it isn't, we'll find out after the split.
Comment #14
Mile23Unrelated fails? Let's see. Restarting test.
Comment #16
Eric_A CreditAttribution: Eric_A commentedPer #8 and #12.
Comment #17
alexpottCommitted and pushed 88d101c96afbe9a3e82e177101a42508e714ff92 to 8.2.x and ff3b3e8 to 8.1.x. Thanks!