Problem/Motivation

After #1497230: Use Dependency Injection to handle object definitions drush has been broken when calling t() too early in the bootstrap phase. 

Fatal error: Class 'Drupal\Core\DependencyInjection\ContainerBuilder' not found in ...xxxxxx/drupal/core/includes/bootstrap.inc on line 2354

After much discussion about if this should be fixed in core or in drush in IRC and the issue queue including:
#1589182: Fatal error: Class '...ContainerBuilder' not found when executing drush status on a not yet installed Drupal
#1541990: Improve check before calling t() (D8 drupal_container() patch broke drush)
#1540110: Drush calling t() before D8 dependency injection construct is available

...and finally #322549: t() should work regardless of the bootstrap phase has a more complex and convoluted history. I suspect this patch is related, and will possibly be mute after that has settled out. However, we don't need core to be broken in this way in the meanwhile.

Proposed resolution

This patch is based on Moshe's comment: http://drupal.org/node/1569456#comment-5981226 We simply wrap ContainerBuilder in a conditional and return the untranslated string if Drupal is not ready yet.

Patch follows...

CommentFileSizeAuthor
#3 drupal8.t-containerbuilder.3.patch877 bytessun
#1 translate_or_dont-1590182-1.patch797 bytesZenDoodles

Comments

ZenDoodles’s picture

ZenDoodles commented
Status: Active » Needs review
StatusFileSize
new translate_or_dont-1590182-1.patch797 bytes

Status: Needs review » Needs work

The last submitted patch, translate_or_dont-1590182-1.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Needs review
Issue tags: +Dependency Injection (DI)
StatusFileSize
new drupal8.t-containerbuilder.3.patch877 bytes 
+++ b/core/includes/bootstrap.inc
@@ -1380,7 +1380,13 @@ function t($string, array $args = array(), array $options = array()) {
+    if (class_exists('ContainerBuilder')) {

I'm fairly sure you need to specify the fully qualified class name:

Drupal\Core\DependencyInjection\ContainerBuilder

+++ b/core/includes/bootstrap.inc
@@ -1380,7 +1380,13 @@ function t($string, array $args = array(), array $options = array()) {
+      return (empty($args)) ? format_string($string) : format_string($string,$args);

When $args is empty, you need to return $string directly.

Lastly, I'm not sure whether this is the correct solution attempt and whether it doesn't actually make things worse.

ZenDoodles’s picture

ZenDoodles commented
Status: Needs review » Needs work

After the failures, I was just coming back to post a patch that assumes english if ContainerBuilder doesn't exist yet. Not sure which approach is better. 

+++ b/core/includes/bootstrap.incundefined
@@ -1378,6 +1378,13 @@ function drupal_unpack($obj, $field = 'data') {
+  // If ContainerBuilder is not loaded, then the language subsystem has not been
+  // initialized yet. In that case, do not attempt to translate and only format
+  // the string. Also do not attempt to load ContainerBuilder.

This part on the other hand --WAY better than mine!

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs work » Active
Issue tags: +Needs tests

I'm fairly sure this only fixes a symptom by hiding it, but does not fix the actual cause.

Furthermore, I first want to see a test, which demonstrates that this bug actually exists in Drupal core (and not only Drush).

The actual error message of the fatal error itself clearly hints at Drush not really/properly accounting for the new PSR-0 based classloader in D8 core. Therefore, I've re-opened the Drush issue:
#1541990: Improve check before calling t() (D8 drupal_container() patch broke drush)

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

#1541990: Improve check before calling t() (D8 drupal_container() patch broke drush) was fixed.

It's a quick stop-gap fix only, but Drush needs to figure out how to adapt its bootstrap to correctly initialize the PSR-0 classloader in D8+ before calling into any API functions or subsystems.

@moshe pointed out an alternative direction, in which Drush dt() wouldn't attempt to use Drupal's t() in the first place. That said, since that call to t() happens without/before a Drupal bootstrap, format_string() (since D7) would be a more appropriate function to call, since nothing is translated in the first place.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Closed (duplicate)