Problem/Motivation

This is a spin-off from here: #1785250: Undefined index: langcode in locale_translate_batch_import() and translations not imported

Symptoms and steps to replicate:

  • Added a few .po files to sites/default/files/translations.
  • Installed in non-English language (German) and saw no real notices, but almost nothing was displayed in German after the installation process completed.

There are two issues:

  1. The main issue is that at the time locale_translate_batch_import() is executed the stream wrapper for translations isn't registered.
    Thus the GetText::fileToDatabase() / PoStreamReader::open(), which uses drupal_realpath() fails.
  2. The other Problem is that locale_translate_batch_import() uses a try { } catch statement which fails silently.

Digging deeper showed that the locale module is enabled in the system table (it even has the flag bootstrap), but module_invoke_all('stream_wrappers'); won't call locale_stream_wrappers().
The reason seems to be that at that point module_list() just returns system and user as the values are cached.

Further investigation:
The first occurrence of st() triggers a call to file_get_stream_wrappers() - but at that time module_list() just returns system and user.
This because install_begin_request() uses module_list(NULL, $module_list); to set a fixed minimal list of modules and st() is already used in this early stage.
This leads to stall static caches since it happens that already enabled modules are excluded. And there's nothing like a drupal_static_reset() at the end of install_begin_request().
Unfortunately a simple drupal_static_reset() breaks the installation points before the translation import.

Proposed resolution

We need a way to extend the minimal module set created in install_begin_request() if applicable and reset the static caches.

Remaining tasks

User interface changes

none

API changes

none

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

das-peter’s picture

Issue tags: +Needs tests, +D8MI, +sprint

Tagging.

Gábor Hojtsy’s picture

It sounds like how when the module is enabled, it is not yet included in the list of enabled modules in the same request or that stream wrappers are initialized/retrieved before the module is enabled.

das-peter’s picture

Status: Active » Needs review
FileSize
1.03 KB

Ok, after a bit more debugging I found a possible solution. It's quite hacky but the only solution I was able to think of with my limited insight.
I would love to change the condition and the static cache backup stuff, but I don't see a more generic way to solve this.
Feedback very welcome :)

Gábor Hojtsy’s picture

I've asked catch on IRC:

[10:39am] GaborHojtsy: catch: hi, das-peter found some significant issues in the installer given that caches are not properly cleared when modules are installed but also found its not as simple as just flushing caches and letting it go: http://drupal.org/node/1787520
[10:39am] Druplicon: http://drupal.org/node/1787520 => Translations not imported on installation => Drupal core, install system, normal, needs review, 3 comments, 3 IRC mentions
[10:39am] GaborHojtsy: catch: wondering if you have any opinions 
[10:40am] catch: GaborHojtsy: sun has a patch to use the non-interactive installer in simpletest that adds a reset to module_list((
[10:43am]  catch: GaborHojtsy: I would love to kill $fixed_list altogether but that depends on the extensions patch.

The extensions patch is #1331486: Move module_invoke_*() and friends to an Extensions class (thanks chx for the pointer).

So looks like we might gain some with looking at @sun's cache clear techniques and if they help us :)

Gábor Hojtsy’s picture

das-peter’s picture

I'm not sure if this will solve the issue.
Just flushing the static cache of module_list() won't work because it's likely that file_get_stream_wrappers() was called already and thus the static cache of this function needs to be flushed too.
Thus my attempt was to flush everything - except the things I now need to be kept.
The problem is that there's no generic way to know what needs to be kept.

Gábor Hojtsy’s picture

What about trying to flush only the caches that are problematic right after we enable the locale module?

das-peter’s picture

That could be a solution for the locale module but not for other modules / install profiles.
I'm afraid the current, somehow inconsistent, behaviour will lead to confusion if someone tries to create a custom install profile / installation step.
And it's simply odd that caches are invalid / incomplete on install. I don't think we should introduce this kind of special knowledge as prerequisite to create own install profiles.
The current hacky approach may introduces really nasty non-generic code - but it's in a single place and it avoids the need of special knowledge everywhere else.

^^ By that I don't vote for the hacky approach at all, but for one that ensures a consistent / predictable behaviour .

Gábor Hojtsy’s picture

Well, unfortunately the installer was a pile of hacky approaches due to the real Drupal systems not yet possible to bootstrap. Ideally by the time we can enable locale module, the full system could be bootstrapped, which would be able to do all that we need then.

Gábor Hojtsy’s picture

Issue tags: +language-ui

Tagging for interface language.

Gábor Hojtsy’s picture

#1215104: Use the non-interactive installer in WebTestBase::setUp() got committed. Does it help on a second look? :)

das-peter’s picture

Unfortunately the updated core from #1215104: Use the non-interactive installer in WebTestBase::setUp() didn't solve this issue. I've still to apply the patch posted here to get the import to run.

sun’s picture

You guys are confusing me ;)

Am I mistaken, or didn't we move the language selection + import of translations to the first installer screen, even before the system is installed...?

That surely means that Locale module cannot be enabled, and thus, the translation:// stream wrapper cannot be registered — unless registered manually.

In turn, what you most likely want is along the lines of attached patch — however, I'm fairly sure that this will break badly.

Status: Needs review » Needs work

The last submitted patch, drupal8.install-locale.13.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
2.57 KB

Let's try once more, and with more explanation.

Status: Needs review » Needs work

The last submitted patch, drupal8.install-locale.15.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
3.75 KB

Completing the big picture.

Status: Needs review » Needs work

The last submitted patch, drupal8.install-locale.17.patch, failed testing.

das-peter’s picture

didn't we move the language selection + import of translations to the first installer screen

As far as I know the only language selection was moved to the first installer screen. But during the installation the the related po file instead the db is used as source for the strings (to do so no stream wrappers are used).
The import of the strings from the po file into the db happens after the modules are enabled. And there stream wrappers are used.

I'm afraid that manually loading the locale module might fix the issue with the language import, but it's not a sustainable solution in terms of the expected behaviour. It took me a while to understand that in the installer, after enabling modules, the modules are basically not available due to the manual overwrite. Thus my attempt was to revert the manual overwrite as soon as the system & modules are installed.
What I can't say is, if in the current situation a module installation could fail because it has a e.g. dependency to another module which provides an own stream wrapper.

Gábor Hojtsy’s picture

Issue tags: -sprint

The installer is full of workarounds for workarounds. I don't think we should focus on resolving this mess, instead of fixing our issue. If you want to work on untangling the whole mess, feel free to though :)

Untagging from the sprint since this is a clean and clear bug and therefore can be returned to after December 1st, while we focus on real new features.

Gábor Hojtsy’s picture

The install batch process and the language import / selection process changed a lot recently. Does this problem still apply?

mvc’s picture

fwiw i was able to download translations during install using git checkout of d8 done today, once the files/ directory existed with appropriate permissions. i think the problem no longer applies.

jair’s picture

Issue tags: +Needs reroll

Needs reroll

jair’s picture

Issue summary: View changes

Added further investigation.

alvar0hurtad0’s picture

Assigned: Unassigned » alvar0hurtad0
Issue summary: View changes
alvar0hurtad0’s picture

Assigned: alvar0hurtad0 » Unassigned
manningpete’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
50.39 KB

Rerolled patch.

sun’s picture

Status: Needs review » Closed (cannot reproduce)

Since I recently rewrote the installer/translation tests in #2171683: Remove all Simpletest overrides and rely on native multi-site functionality instead, I can confirm that this bug no longer exists.

In fact, pretty much the opposite of this bug report exists in HEAD today:

#2173801: Installing in a different language than English takes forever to complete

→ Translations are downloaded/imported more than once in the installer, which causes a trivial single installer test to take up to ~10 minutes.

Status: Closed (cannot reproduce) » Needs work

The last submitted patch, 26: drupal8.install-locale.26.patch, failed testing.

The last submitted patch, 26: drupal8.install-locale.26.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Closed (cannot reproduce)