Improve check before calling t() (D8 drupal_container() patch broke drush)

t() shouldn't be available without DRUPAL_BOOTSTRAP_LANGUAGE, so this one does make sense. Thanks! I'll leave needs review in case someone has more time to do a proper review of it. As you mentioned, we will be doing some initial lazy-instantiation stuff on this once #1511482: Bootstrap for the Dependency Injection Container is in. But this patch does seem sound.

I and others worked hard to make t() always safe to call in Drupal 7 and that has now regressed in D8. Drush is going to wait for Drupal8 to fix this.

If I might poke this issue a bit, the only real problem seems to be the autoloader isn't setup. Can we just add drupal_classloader(); fairly early to the d8 bootstrap and call it a day. Its statically cached and it'd be a single call so its basically free.

Alternatively, we could poke core to add the call inside the static cache of drupal_container(). Either way, seems like a much simpler fix and fits with the re-factoring Rob is doing of the language system.

FYI: I ran into the same problem with Barracuda and the patch above is working (Drush 4), see #1459362: Add support for Drupal 8 sites since /core patch

I want this fixed in Drupal core. Drupal developers should not have to think when they can call t() and when they can't. And yes, there is a point to calling t() before the language system is available. In D7, you got translation for settings.php string mappings in this case.

Ok, Is there a core issue to follow up on?

Just a note, the fix I would suggest for Drupal core is the same I suggested here though which is registering the auto-loader as early as possible and using the container to bootstrap as needed. This fits with the DI container approach which decreases the tight coupling of bootstrap levels with functions like t() and should decrees the fragility of the bootstrap in general. Getting Drush on-board with the end goal early should decrease having to do this for each core bootstrap change during the release.

I don't know of a specific issue. effulgentsia posted a core patch to #136 at #1497230-135: Use Dependency Injection to handle object definitions . He mentions a further change which would make it always safe to call t() which would be ideal, if we can pull it off.

#1497230-136: Use Dependency Injection to handle object definitions is now committed to core. As per that comment, t() now only fails if:
- You haven't bootstrapped to DRUPAL_BOOTSTRAP_CONFIGURATION (the very first bootstrap level), and
- You haven't called drupal_classloader() yourself.

I think we need to do one of the following:
- Fix Drush to not call t() if we haven't even bootstrapped configuration.
- Make Drush call drupal_classloader() if it wants to call t() before bootstrapping configuration. I don't think this is ideal since drupal_classloader() has conditional logic based on settings.php.
- In core, separate bootstrap.inc into a file per bootstrap level, and move functions requiring a certain level into the appropriate file. That way, Drush will continue to work by just checking function_exists('t'). I think this would be good in theory, but I'm not crazy about needing an extra 6 or so includes for every request.

Is there some reason why t() can't just return an untranslated string when called too early? Thats what happens in D7.

#1540110: Drush calling t() before D8 dependency injection construct is available is closed as a dup of this issue ... (x<y) doh ... it has a patch too which keeps the theme-function check ... which I guess is a D6 thing which we should keep right?

Is there some reason why t() can't just return an untranslated string when called too early? Thats what happens in D7.

D7 doesn't make as much use of autoloaded classes during bootstrap. As far as I know, D8 is moving towards autoloaded classes for as much of bootstrap as possible (that's nowhere near complete yet, but there's a lot of interest in that). So we should consider this t() issue to be representative of maybe more to come. In general, which functions do we want to be callable prior to having a class loader? That's a question that should be raised in the core queue. I haven't yet opened a dedicated issue for that, but it's just now come up in #1569456-11: Move replace fix_gpc_magic() with a hard requirement for magic quotes to be disabled.

Is there some reason why t() can't just return an untranslated string when called too early? Thats what happens in D7.

No reason to use t() while core is not bootstrapped yet

see my prior comments.

zendoodles has made a patch that does what moshe asks for over at #1590182: Return untranslated string when t() is called too early in the bootstrap phase..

Pasting from #1590182: Return untranslated string when t() is called 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

Q: What does this error mean?

A: PHP is not able to load the class Drupal\Core\DependencyInjection\ContainerBuilder.

Q: Is it related to t() or the language system?

A: No. PHP is not able to load the class Drupal\Core\DependencyInjection\ContainerBuilder.

Q: Does Drush properly account for the new bootstrap phases in D8?

A: Dunno. DRUPAL_BOOTSTRAP_CONFIGURATION was added as very first bootstrap phase, which calls drupal_classloader(). Was it?

Q: Does Drush initialize the PSR-0 class loader in Drupal core or implement its own?

A: Dunno. grep -i 'ClassLoader' ./* on drush yields no results. Does it?

This is mostly the same as my patch from #1540110: Drush calling t() before D8 dependency injection construct is available which has been working for me since May 2. Can we please commit it already?

Edit: actually, I think I prefer the one over there...

All of the patches I've seen so far are only trying to hide the symptom.

Because we're doing a core sprint tomorrow, and everyone will ask why Drush "doesn't work" with D8, here is the patch I've been using. It's a modified version of the one linked in the duplicate issue. I'm also attaching a patched version of the entire file (includes/output.inc), just so people can Get Stuff Done tomorrow.

Just to prove what I'm saying... use this instead.

Duh, I missed #16 and only saw #17. #20 works just fine.

#1590274: Drush does not work from within a subdirectory (e.g. /core) in Drupal 8 is an unrelated follow-up.

Committed #20. I caved.

IMO, the committed fix is a good one for now. I also opened a core issue to tackle the broader scope problem of bootstrap functions that depend on classes: #1593826: Split bootstrap.inc into functions available before/by settings.php and functions available after there's a class loader. If something along those lines gets committed, then #20 can be reverted.