Shipping a profile with multiple languages without locale module not possible

There's core/profiles/testing_multilingual profile with languages that was added with tests in #2336743: When more than one language is added in the profile, the installer ignores those

Core install stops because language module is not dependent on locale module

$ git grep locale_translation_use_remote_source
core/includes/install.core.inc:1695:      if (locale_translation_use_remote_source()) {
core/modules/locale/locale.compare.inc:207:  if (locale_translation_use_remote_source()) {
core/modules/locale/locale.fetch.inc:100:      if (locale_translation_use_remote_source()) {
core/modules/locale/locale.module:945:function locale_translation_use_remote_source() {
core/modules/locale/locale.translation.inc:439:    'use_remote' => locale_translation_use_remote_source(),
core/modules/locale/src/Form/TranslationStatusForm.php:252:    if (locale_translation_use_remote_source() && $remote_path && $local_path) {

Major, because expected, needs discus a dependencies or document current behaviour

@andypost: But language module SHOULD NOT depend on locale, the dependency is the other way around. Neiher any calls in your grep are from language to locale. So that should not be a problem.

Can this still be reproduced?

Add tag to get help with manual testing.

My step to test for erreur is :

Apply this patch
Install Drupal, to language and profile minimal.

For 4 testing install, no erreur in install.

So the idea is install_profile_info() adds the locale module as an automatic dependency on the profile if the install was in a foreign language. Looks like you have multiple languages given that you install in English and install_import_translations() expects locale module to be available then. But it is not. We can skip these steps if locale module is not on the profile explicitly or implicitly.

The reason of the error message is that we try to use a function from a module not added yet. In the attached patch we check first if the module is already loaded.

Edit: Unfortunately my patch doesn't solve the basic problem so just ignore it. The shortest way I think is just adding locale module beside language to the dependencies in install profile info.yml file.

With the proposed patch it would still add the next batch operation (visible in the hunk).

Also there's issue with drush si my_profile, if you choose --locale=de install of other languages from profile will be skipped (English as well)

This prevents language imports from running during install if the local module is not enabled.

Patch #21 worked for me with 8.2.7 (and surprisingly did also apply \o/)

Hi, this patch #21 is good for me.

Hi, the patch #21 worked for me.

I confirm the bug. Attaching a "test only" patch.

Compared to #21, my solution avoids using the module handler service. It just inspects the $install_state['profile_info']['install'] value.

In fact #21 is correct. #30 doesn't work with Drush.

This is a bit tricky - normally we force the locale module on for a multilingual install - see install_profile_info()

$locale = !empty($langcode) && $langcode != 'en' ? ['locale'] : [];

So what concerns me is that if you selected another language in the installer the install is going to work and locale is going to be installed. I'm wondering if we should change this condition to also check to see if the profile provides any non english configurable languages and if so, install locale.

@alexpott, having multiple languages installed doesn’t necessary mean that we should have the locale module enabled. While selecting a non-English language to run the installer requires locale as this module is about UI localization and the installer needs translated UI strings. On our project we have this case: the site’s UI is only in English but we have some content translated in different languages. We don’t need ‘locale’, then why should be that module enabled? More: Why should we import UI translated strings that we’ll never use? I think the logic from the patch makes more sense: We don’t import UI translations if the module that handles UI translations is not enabled.

I'm in the same situation as @claudiu.cristea - I have multiple languages for content translation but don't want the locale module.
The solution in #31 worked great for me, thanks.

I researched why the locale module is enabled in install_profile_info() and I can confirm it is required so that the installation procedure itself can be shown in the correct language when installing a profile in a language other than English.

Outside of testing there is only one call to install_profile_info() that passes the language code and this is located in install_load_profile() which is called as part of the installation procedure (install_drupal() -> install_begin_request()). If the installation is interactive, this will allow Drupal to show translated versions of the forms that ask the user's credentials, timezone etc. It needs the locale module for this.

Now if a non-interactive installation of a profile is done (e.g. drush si my_profile) then we do not need to show translated forms, and locale is not needed.

The change introduced here is safe because it checks whether the locale module is enabled _after_ the above code from install_profile_info() is run. So if the locale module is required to display the installation forms in a different language, then it will be enabled and everything will work as expected. I checked we have coverage for this case and it is covered in InstallerTranslationTest.

I reviewed the fix and the test and both look good. Failing test can be found in #2413191-28: Shipping a profile with multiple languages without locale module not possible.

I've credited @Gábor Hojtsy for code review and suggesting the correct fix and @pfrenssen for a good rtbc comment.

Committed and pushed 08183b22a3 to 8.8.x and 6cd7a078da to 8.7.x. Thanks!

Hmmm... didn't push - the new test is going to fail because it extends InstallerTest but does not override \Drupal\FunctionalTests\Installer\InstallerTest::testInstaller() so it's in effect running two tests. I've queued up a retest of the old patch against 8.8.x to prove this. I'm currently fixing this.

This has been once RTBCed in #35. I'm only reviewing the changes from #38.

Committed and pushed d37aea09c0 to 8.8.x and 670f162640 to 8.7.x. Thanks!

As I only made very minor changes to the test and not runtime code I think it's okay for me to commit this.

Backported to 8.7.x as a non-disruptive major bug fix.