I've noticed when the site has a non English default language, the full locale backend is used for the main D7 to D8 upgrade and also for regular update scripts.
This will just break if some of the upgrades affect the locale tables and it is specially painful since in case an exception is thrown it can trigger a new locale exception itself.
Example error messages here, https://drupal.org/node/1785086#comment-6538250
(Btw, that patch needed some tweaks itself for not breaking upgrade tests. Also js string rebuild can be triggered during this upgrades making the whole locale module needing to be kind of upgrade-safe)

How this should work IMO:
We should be using the installer locale 'st' for both, D7 upgrade and other module updates for the locale module to be able to upgrade its own tables if needed.
Also now we have an abstracted locale storage, we could switch the storage using a safe one during these operations. So my suggestion would be just dropping st() function and make t() and locale module itself decide on the storage to use. This would also make life easier for module developers since they won't need to make the distinction for update-safe code, which is not as obvious as it seems because many functions besides the ones in .install files can be triggered from that operations.


Jose Reyero’s picture

Component: translation.module » locale.module

Moving to the right module (locale)

Gábor Hojtsy’s picture

1. Well, historically we have been avoiding the use of translatable strings in the update because (a) of possible locale database changes (b) you'd not import a .po file before the update so you can see the update happening in your own language anyway. So it looks pointless to have t() or st() in the upgrade from a functionality point of view.

2. Also, we attempt to do all language related changes in a pre-boostrap in the update, so theoretically the language system should be ready by the time the update actually starts.

3. And finally, we attempt to not call out to API functions in the update, so **theoretically** the locale system would not be invoked if we keep ourselves not to include use of t() or st() in the upgrade path. That said, its hard to shield the upgrade path from the rest of the system and exceptions, errors, logging, etc. could indeed invoke the locale system, so its not a bulletproof protection.

Jose Reyero’s picture

It seems to me we should use MAINTENANCE_MODE instead of drupal_installation_attempted() in the locale get_t() function.

Jose Reyero’s picture

One (may be related) issue: The exception handling code in error.inc is plagued with 't'.
This is why we get locale exceptions thrown when handling another exception.

This code should be install/whatever safe, shouldn't it?

Jose Reyero’s picture

Title: Disable locale backend for D7 to D8 upgrade and maybe for update scripts? » Mixed locale issues with upgrade scripts, unit tests, 'Table simpletest..locales_source doesn't exist', etc...

Done some more research, see related:
#1806756: How to register storage controllers / 'target' database parameter (Looking for a common standard)
#1808864: Exception handling is not 'locale safe' (thus not database safe)

Notes from these two:
- Locale's t() is triggered from exceptions / exception handling, and this is one of the reasons it may be triggered from the installer. You only note it when doing an installation in an alternate language and something goes wrong.
- Locale translation is triggered too from hook_js_alter(), which is called from drupal_get_js(), ajax_render(), template_process_html(), template_process_maintenance_page(), so it seems this can happen from installer pages or from update scripts.
- Under certain race conditions, like module install / uninstall from unit tests, locale() may be invoked from t() when the locale module is not installed causing db exceptions like 'Table 'drupal8.simpletest575223locales_source' doesn't exist:'. We also get crashes when running unit tests from a site with locale enabled. The reason for this seems to be checking for function_exists('locale') from the text execution context so it is invoked if enabled in the site running the test and there's a t() in the unit test itself (called from the testing site, but trying to use the table in the 'test' site that doesn't exist). Though this is not a major issue and we all have learnt to run tests only from an English site without locale enabled, this should be at least documented....
... The way to fix this would be adding some 't' method to the TestBase class that does a proper checking and using $this->t() from tests instead of t()

Renamed this issue and using it for research and documentation for now. We can create another issue for the last point...?

I know these are no critical issues and they have easy workarounds (upgrade only using English) but they cause development for locale module and running tests specially painful and difficult to debug. There's this core test running a D7-D8 upgrade in a language that is not English....

Jose Reyero’s picture

More on race conditions :

These happen when you set a default language other than English. There are some circusmtances under which locale() is called from t() and language is not initialized yet. Note language is initialized from _drupal_bootstrap_full()

- From regular drupal bootstrap, _drupal_bootstrap_code() calls file_get_stream_wrappers(). Stream wrapper definitions in both system_stream_wrappers() and locale_stream_wrappers() use t() for names and descriptions. But this 't' will translate strings to default language instead of current page language.

- From update.php:
- Same, file_get_stream_wrappers() from drupal_bootstrap(DRUPAL_BOOTSTRAP_CODE);
- On the first run of update.php (no $op), when system requirements are checked, in case of error, these messages are not translated, as these functions use get_t(), but unlike the installer, it returns t() instead of st(), that would be the one to use here. (In this case locale is not loaded yet, only system module)
- Many update functions end up invoking locale(), like update_fix_compatibility() , so we should be aware of this and ensure locale storage gets a proper initialization asap in the update scripts.

- From install.php
The problem here is, when selecting a language and enabling locale t() and st() are used both during the install. Some functions invoked by the installer have hardcoded 't', like format_interval() which calls format_plural() from batch.inc (Still I am not sure whether these get translated somehow or not, but possibly not for the first stages of the installer)

For these reasons it is just not possible atm to initialize the locale storage controller using the standard Drupal container and we need an ad-hoc initialization inside locale_storage(). Some notes for the future, that would help fix this (and making the locale storage replaceable with DIC):
- Either the language initialization should be done before file_get_stream_wrappers() is run or t() shouldn't be used in hook_stream_wrappers(). Maybe this would be fixed by moving stream_wrappers into configuration system.
- get_t() should return st() instead of t() for update scripts when doing the initial requirements check.
- Since we can swap locale storages now, it may be interesting to give a try of getting rid of 'st' and replacing it with t() using a proper po file based locale storage.
- Keep an eye on the configuration system and check configuration loaded before language initialization gets proper translation at a later stage. (the locale config event listener gets added from locale_language_init())
- The update issues should be fixed to be able to deploy future updates to locale module (because right now locale is invoked before the updates even start to run...)

So this is mostly documented, nothing urgent to do besides being aware of locale updates (tables, etc) being prone to crash the update scripts, they may need some extra care.

Gábor Hojtsy’s picture

Sounds like now that we have storage controllers independently supported, we can provide an "echo" storage controller that just returns back the string as-is, and initialise to this ASAP at the start even before we can make the db storage work, and then swap this out for the DB storage when that is available. That should make t() work early at least.

Jose Reyero’s picture

Right, we can add some class magic to it really, here's some start #1813762: Introduce unified interfaces, use dependency injection for interface translation

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.