Follow up for #1848490: Import translations automatically during installation

Problem/Motivation

$ drush -y si --account-pass=admin --db-url="mysql://root:root@localhost/d8-git" --site-name=8.x
You are about to create a sites/default/files directory and create a sites/default/settings.php file and DROP all tables in your 'd8-git' database. Do you want to continue? (y/n): y
Starting Drupal installation. This takes a few seconds ... [ok]
Installation complete. User name: admin User password: admin [ok]
Strict warning: Only variables should be passed by reference in [error]
install_select_language() (line 1344 of
core/includes/install.core.inc).

Proposed resolution

TBD.

Remaining tasks

Decide if this is an install problem or a drush one.

User interface changes

No UI changes.

API changes

Maybe parameter type changes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

/**
 * Selects which language to use during installation.
 *
 * @param $install_state
 *   An array of information about the current installation state. The chosen
 *   langcode will be added here, if it was not already selected previously, as
 *   will a list of all available languages.
 *
 * @return
 *   For interactive installations, a form or other page output allowing the
 *   language to be selected or providing information about language selection,
 *   if a language has not been chosen. Otherwise, an exception is thrown if a
 *   language cannot be chosen automatically.
 */
function install_select_language(&$install_state) {
  include_once DRUPAL_ROOT . '/core/includes/standard.inc';

  // Find all available translation files.
  $files = install_find_translations();
  $install_state['translations'] += $files;

  // If a valid language code is set, continue with the next installation step.
  // When translations from the localization server are used, any language code
  // is accepted because the standard language list is kept in sync with the
  // langauges available at http://localize.drupal.org.
  // When files from the translation directory are used, we only accept
  // languages for which a file is available.
  if (!empty($_POST['langcode'])) {
    $standard_languages = standard_language_list();
    $langcode = $_POST['langcode'];
    if ($langcode == 'en' || isset($files[$langcode]) || isset($standard_languages[$langcode])) {
      $install_state['parameters']['langcode'] = $langcode;
      return;
    }
  }

  if (empty($install_state['parameters']['langcode'])) {
    // If we are performing an interactive installation, we display a form to
    // select a right language. If no translation files were found in the
    // translations directory, the form shows a list of standard languages. If
    // translation files were found the form shows a select list of the
    // corresponding languages to choose from.
    if ($install_state['interactive']) {
      drupal_set_title(st('Choose language'));
      include_once DRUPAL_ROOT . '/core/includes/form.inc';
      $elements = drupal_get_form('install_select_language_form', count($files) > 1 ? $files : array());
      return drupal_render($elements);
    }
    // If we are performing a non-interactive installation. If only one language
    // (English) is available, assume the user knows what he is doing. Otherwise
    // thow an error.
    else {
      if (count($files) == 1) {
        $install_state['parameters']['langcode'] = array_shift(array_keys($files)); // Line 1344.
        return;
      }
      else {
        throw new Exception(st('Sorry, you must select a language to continue the installation.'));
      }
    }
  }
}
YesCT’s picture

Category: feature » bug
YesCT’s picture

Status: Active » Needs work
FileSize
1 KB

Also, the @return is missing typing. maybe should be array|null
And start the description with "Returns nothing or for interactive..."
But that is not related to this issue and should be a sep clean up issue probably.

Without any patch, even though drush says [error] the site looks to be installed ok. Maybe it's passing along an exception?

About the patch: I actually dont know if this will help at all. Local testing still give the error.

YesCT’s picture

Status: Needs work » Needs review
alexpott’s picture

FileSize
529 bytes

Getting this myself... but the fix seems to be in a different place... line 1344 is:

  $install_state['parameters']['langcode'] = array_shift(array_keys($files));

The issue is explained by http://stackoverflow.com/questions/6698428/return-first-key-of-associati... basically:

array_shift(); expects an array passed by reference

alexpott’s picture

ps. this is the only place in the entire codebase where I can find "array_shift.*array_key"
pps. the current(array_keys()) trick is used only in one place too :) Drupal\Core\Entity\Field\Type\Field::setValue()

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

I don't think this needs a test of its own, it doesn't actually break anything. Just a notice.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. I don't think it's possible to test anything in the installer, unfortunately.

Automatically closed -- issue fixed for 2 weeks with no activity.