Problem/Motivation

When adding a new language on admin/config/regional/language, the language files are not downloaded. If eg. the site is installed in English and French language is added, no French translation is downloaded. This is caused by two factors:

  1. Drupal does not report a reasonable version number to work with (git checkouts, alpha releases, etc. all report as Drupal 8.x). Git checkouts will never report a proper version number. Only a server side fallback solution would help with this unless we want to manually update a list of prior core versions in the code. See #2113957: Build server side version fallback system for translations. In the meantime, git_deploy was ported to Drupal 8 and can identify dev versions to the commit :)
  2. Drupal core / locale module does not fall back on prior releases from project update data if a dev version is encountered. That is what this issue is here to resolve.

Proposed resolution

- Fall back from dev versions to prior core releases based on project data.
- Add tests.

Remaining tasks

- Tests

User interface changes

None.

API changes

None.

Related

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

penyaskito’s picture

Issue tags: +D8MI, +sprint, +language-ui

Tagging.

YesCT’s picture

I can confirm, that when I add spanish, I have 0% UI strings translated. I thought #1804688: Download and import interface translations should have made it work automatically.

wusel’s picture

I have the same issue.

I add:
If I install the alpha 2 or a alpha2-xxx.dev of the D8-core and select German language during the installation, then I get the german D7-po-file (drupal-7.0.de.po) but nor D8-po-file. A German D8-po-file is available on the translation server of d.o. at http://ftp.drupal.org/files/translations/8.x/drupal/drupal-8.0-alpha2.de.po

Wusel

penyaskito’s picture

There are different issues happening here, the concrete issue @wusel is experiencing is being tracked at #1914070: Improve version fallback for install language..

Because this issue has some interesting use cases and not sure if everything is covered, I'm against marking this as duplicate.

Gábor Hojtsy’s picture

Priority: Normal » Major

That wrong version translations are downloaded and no translations are downloaded in some cases sounds like to me at least a major.

Sutharsan’s picture

If you add a language to an existing site, a po file is downloaded using the \Drupal::VERSION. Currently this is 8.x-dev for all, and therefore it will not download any translation. No po files are provided for dev's. Edit the 'version' in the info.yml file to test.

Schnitzel’s picture

Status: Active » Closed (works as designed)
FileSize
439.36 KB

As Sutharsan correctly mentioned, there are no po Files for 8.x-dev.

If you change

const VERSION = '8.0-alpha2';
in Drupal.php

and adapt local_project table it works perfectly, see screenshot for some proof.

I think that the Issue of wusel is exactly #1914070: Improve version fallback for install language.

Gábor Hojtsy’s picture

Status: Closed (works as designed) » Active

So there are two problems here. #1914070: Improve version fallback for install language. resolves the installer problem. It does not resolve the language addition problem, and that should still work with this. Making it required to change your version is not the way to solve this IMHO. The code for adding language should work with dev releases.

Gábor Hojtsy’s picture

Title: Language file problems when installing a site in a different language than English or when adding a new language » Translations not downloaded when adding a new language
Issue tags: +Needs issue summary update

Retitling for the remaining problem. The install problem is handled in #1914070: Improve version fallback for install language.. Needs issue summary update to reduce it only to the remaining problem.

wusel’s picture

@#3

I have installed drupal-8.0-alpha4 and drupal-8.x-dev_2013_10_24 on xampp 1.8.2-2 and Win7-64bit.

Thank you all, it now downloads the file "drupal-8.0-alpha2.de.po", if I choose German during install.

Wusel

Gábor Hojtsy’s picture

Yeah this bug is about adding another language on that install (or an English install). Is *that* downloading a translation now? :)

wusel’s picture

There are two bugs:

- like #11: the first language did not download the correct version of the po file:
drupal downloads the file "drupal-8.0-alpha2.de.po" and NOT "drupal-8.0-alpha4.de.po", which we can find at http://ftp.drupal.org/files/translations/8.x/drupal/drupal-8.0-alpha4.de.po

- if I then add a language like "Hungarian", I read at "admin/config/regional/language" for this installed language Hungarian: "Interface translation = 0/6253 (0%)" and there is no po file at \sites\default\files\translations\

Hint:
If I look at admin/reports/dblog I can find: "Translation file not found: http://ftp.drupal.org/files/translations/8.x/drupal/drupal-8.0-dev.hu.po."

Wusel

Gábor Hojtsy’s picture

Yeah the download of alpha2 is "expected" ATM. About two weeks ago it was downloading Drupal 7 translations. As per the current code it will download translations for the first published version of that series (alpha2 is the first with a tarball). So once betas are out, beta1 translations would be downloaded for all betas. Once rcs are out, rc1 translations, etc. This needs some serious improvements on the server side to make more intelligent. See #2113957: Build server side version fallback system for translations and #2113955: Rely on proper server side version fallback for translations for those.

As for the adding new language is where this bug is at, look at the title and issue summary :)

Sutharsan’s picture

The version of the translation to be downloaded is based on the VERSION constant. When writing the code for the version fallback I assumed this string would reflect the version of the tarball, but it doesn't. I've downloaded the D8-apha4 tarball, and found out it has const VERSION = '8.0-dev'; (in core/lib/Drupal.php). I should have been set to '8.0-alpha4' in order to load the corresponding translation file.

Gábor Hojtsy’s picture

Yeah, this would need the same fallback logic that the installer has *for dev* versions no?

Sutharsan’s picture

Hm, I wasn't paying proper attention to the issue subject/title. I was talking about the 'during installation' situation. A solution is that VERSION is set before and reset after creating the release. (unless the below packaging script can handle this).

When enabling a language, the project version is used. This version is based on the date in the .info.yml file. The packaging script is not working correctly yet, as it does not include the right version in the info.yml files. For example system.info.yml contains version: VERSION. This should be version: 8.0-alpha4. #1963092: Convert .info file rewriting in packaging plugins to deal with D8 .info.yml files is about dealing with info.yml files.

Gábor Hojtsy’s picture

Yeah, well even if that is made to work it will never work on -dev versions when adding a language, so we need that code applied to when adding a new language. Sounds like it may be just guessing a version if we don't know the core version reusing the same function as in the installer? :)

Sutharsan’s picture

Without version info, the installer code falls back to hardcoded "8.0-alpha2" version. That can be done. It its simplest form we use "8.0-alpha2" (or 4) if no version info is available. But only for core. I we want to fallback to multiple version (8.0 > RC1 > beta1 > alpha2), the impact is bigger. Currently we can not handle multiple 'check if translation version x exists' requests.

Gábor Hojtsy’s picture

So our other option is we suggest people to install git_deploy?

YesCT’s picture

Issue summary: View changes

Updated issue summary.

penyaskito’s picture

I checked that the last alpha (8.0-ALPHA7) had const VERSION = '8.0-dev'.

What's next in this issue?

Gábor Hojtsy’s picture

@penyaskito: the installer has special code to take fallback version numbers in case of dev versions. This was not added to the regular locale update code though (with the expectation that alpha/beta/rc releases would have actual version numbers instead of dev). To support this dev approach, we would need to have similar fallback code in the runtime path as well like in the installer.

robertdbailey’s picture

I'll try to make some progress on this while at DDD in Szeged.

robertdbailey’s picture

The logic for version fallback in core/includes/install.core.inc:

function install_get_localization_release($version = \Drupal::VERSION)

Its calling function, "install_check_translations", loops through its results, which would be the same logic required when adding a new language. This logic could be added each time a new language is saved, but perhaps the localizations should be downloaded automatically only when setting or changing the default language. If it were added only when the default language is set, then the correct place to add this would be in function "language_system_regional_settings_form_submit" in core/modules/language/language.module.

Does this sound correct, or should localizations be downloaded every time any language is added?

robertdbailey’s picture

The logic for language fallback during Drupal installation is in code that is for core only. The place where translations are downloaded when languages are added is used more broadly. It looks like it is used for translation downloads for core as well as all other modules and themes. Should the language-fallback code be extended (when languages are added) to include checking and downloading localizations for all modules and themes?

Sutharsan’s picture

Status: Active » Postponed

The fallback code was written specifically for installing Drupal and downloading the drupal core translation (and only core, not contrib modules). It has an hardcoded fallback scheme. It would be silly to use a hardcoded scheme beyond the installation process. Once update module is available (after installation), it is used to provide the latest release version of a module (and core).

The root of the problem in this issue is that info.yml files currently do not provide a version. In the file you find "version: VERSION" instead of for example: "version: 8.0-alpha10". This is taken care of by #1963092: Convert .info file rewriting in packaging plugins to deal with D8 .info.yml files. Pending this issue, I mark the current issue as postponed. I expect that the current issue is resolved once the .info(.yml) file issue is fixed.

Gábor Hojtsy’s picture

@Sutharsan: but what would happen for git checkouts and dev branches? Do we require git_deploy for those to work in the first place?

Gábor Hojtsy’s picture

Status: Postponed » Active

So I wanted to look into resolving this sooner than later. The idea was that the runtime will have real version numbers to work with thanks in part to git_deploy. Tonight I decided to try to resolve this by porting git_deploy to D8, and so I did: #2305311: Complete, fully working Drupal 8 port of git_deploy It works well for what it does, but does not really solve the problem for core :(

1. git_deploy decidedly skips core. And in my tests it did not even get the right version (it detects 'HEAD' for a 8.x-alpha13 checkout).

2. For contribs detected, it finds full dev version numbers, eg. 8.x-1.0-alpha1.10-dev in my case because I have a checkout aligning with that. The translation updater successfully falls back from that to look at the update XML and downloads the translation well, YAY!

What do we do for core?

Gábor Hojtsy’s picture

FileSize
251.29 KB

All right, I made git_deploy work for core as well, at least for tags. See #2305311: Complete, fully working Drupal 8 port of git_deploy. Works with update status. The localization update feature still thinks its a 8.0-dev :/ Also the git deploy module version got very confused...

Sutharsan’s picture

Status: Active » Needs review
FileSize
1.43 KB

The fallback only worked for contrib modules/themes. This patch adds a version fallback for core.

Gábor Hojtsy’s picture

Issue tags: +Needs tests

This "works" now in my testing, it detects alpha13 properly with git_deploy 8.x. Unfortunately localize.drupal.org did not identify alpha13 as a version to parse for some reason so that is not available. So now it attempts to download the right version but its the fault of l.d.o that it does not work. Are there any tests for this that should be updated or added to?

Gábor Hojtsy’s picture

Status: Needs review » Needs work

So I started look into tests for this today. However in the meantime Drupal switched to semantic versioning so your patch will not be correct anymore. I wanted to update the patch but then realized, I don't know how the new semantic versions will be represented in the XML. Will there be a "minor_version" key? I wanted to look at the test coverage for this in update_status but then found it still has 7.x update tests. Ho-hum. Yeah. See #2170443-82: [meta] Create a plan for implementing semantic versioning for core.

Also was not sure how that affects the contrib version numbers and whether we should support old schemes and new as well. That is not exactly in the scope of this issue, but we need to fix that too.

The first alpha based on the new semantic versioning is coming out tomorrow, so we'll maybe see some useful data coming up on http://updates.drupal.org/release-history/drupal/8.x as to how it is structured for us to support that.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
1.42 KB
1.71 KB

alpha14 is out now, but no (Dutch) translation is yet available. This patch now updates the version detection to the new semantic version and does not support pre-semantic versions (i.e. alpha13 and before).

Gábor Hojtsy’s picture

Status: Needs review » Needs work
Issue tags: +Drupalaton 2014

We tried this at the Drupalaton multilingual training and it did not work for some reason.

+++ b/core/modules/locale/locale.compare.inc
@@ -68,11 +68,14 @@ function locale_translation_build_projects() {
+          preg_match("/^(\d+)\.\d+\.\d+-.*-dev$/", $data['info']['version'], $matches)) {

I thought this is the reason, this will not match 8.0.0-dev, because the extra - added before .* but removing that extra - did not make it work either.

Sutharsan’s picture

To test the #32 patch (manually) you also need patch in #2317005: Core version not detected due to new semantic versioning for Git Deploy.

Gábor Hojtsy’s picture

It works with that yup. I don't fully why/how it does with that regex, but it does :)

janoka’s picture

Assigned: Unassigned » janoka
Sutharsan’s picture

Status: Needs review » Needs work

The regex currently evaluates "8.0.0-alpha14-dev", and that works. But I realize that I did not look at future version number patterns. Beta and RC will probably be fine but will it be 8.0.0-dev or 8.0.x-dev? Both will fail.

Sutharsan’s picture

8.0.0-alpha14-dev: current situation when using Git Deploy
8.0.0-alpha14+42-dev: current situation when downloading the dev package.
8.0.0-dev: after 8.0.0 release when using Git Deploy.
8.0.0+3-dev: 3 commits after 8.0.0 release when downloading the dev package. (see DrupalorgProjectPackageRelease::computeRebuildVersion())

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
1.42 KB
1.58 KB

@janoka, I have not seen any activity in 24 hours, could not find you in IRC either. Are you still working on this issue? I have taken the liberty to continue working on it.

Gábor Hojtsy’s picture

What git deploy identifies for me is 8.0.0-alpha14.25-dev, note the dot between the alpha and the number and dev. I think this is due to 25 commits since the alpha? Should it use a + when doing this number format?

Sutharsan’s picture

I did not work with the latest git deploy checkout, but now I see. Is should be 8.0.0-alpha14+25-dev. Look at the .info.yml files in a D8 dev release package or check the code in DrupalorgProjectPackageRelease::computeRebuildVersion() of the drupalorg project.

Gábor Hojtsy’s picture

janoka’s picture

@Sutharsan I'm working on this issue.

Gábor Hojtsy’s picture

So while it seems like @janoka made no progress in 18 days, he has a testing issue at #2324551: János Kuszing's test issue. Since that is also not moving a lot, let's at least discuss the plan and validate with Sutharsan here:

1. Implement a test module that uses hook_system_info_alter() to alter the version of core to a dev version.
2. Implement a custom update status backend XML provider (or feed the project DB with equivalent data) that provides the fallback items for said altered core version.
3. Test that locale falls back on appropriate version based on that data.

@Sutharsan: does this sound like it makes sense for you or do you have a simpler/more complex suggestion to cover testing for this? So far @janoka's test issue only implements point 1.

Gábor Hojtsy’s picture

Issue summary: View changes
Issue tags: -Needs issue summary update

Updated issue summary.

janoka’s picture

I came back from vacation yesterday, so I would like to continue this issue, if it is possible. @Gabor and @Sutharsan: what do you think, what are your opinion?

Sutharsan’s picture

This test plan makes sense. Some thoughts:

  • We can limit the the test to locale_translation_build_projects().
  • Yes, we need to alter the project data using hook_system_info_alter(). This covers both Locale and Update module project lists.
  • I think we can get update_get_available() to produce the right data by setting appropriate data in \Drupal::keyValueExpirable('update_available_releases'). The alternative is to set up an XML, but as far as I remember that is hard to do.
Sutharsan’s picture

@janoka, the sooner the better ;)

janoka’s picture

@Sutharsan I totally understand you!

Gábor Hojtsy’s picture

Yeah the most logical step seems to be to plant some test data in \Drupal::keyValueExpirable('update_available_releases')->set('drupal', $test_data); directly in the test in a way that avoids all the conditions that would make update_get_available() run a refresh. Instead of fiddling with update XML and other things.

janoka’s picture

Inspired by our phone speaking I coded:

$available = array(
  'title' => 'Drupal core',
  'short_name' => 'drupal',
  'type' => 'project_core',
  'api_version' => '8.x',
  'project_status' => 'unsupported',
  'link' => 'https://www.drupal.org/project/drupal',
  'terms' => '',
  'releases' => array(
    '8.0.0-alpha110' => array(
      'name'          => 'drupal 8.0.0-alpha110',
      'version'       => '8.0.0-alpha110',
      'tag'           => '8.0.0-alpha110',
      'version_major' => '8',
      'version_minor' => '0',
      'version_patch' => '0',
      'version_extra' => 'alpha110',
      'status'        => 'published',
      'release_link'  => 'https://www.drupal.org/node/2316617',
      'download_link' => 'http://ftp.drupal.org/files/projects/drupal-8.0.0-alpha110.tar.gz',
      'date'          => '1407344628',
      'mdhash'        => '9d71afdd0ce541f2ff5ca2fbbca00df7',
      'filesize'      => '9172832',
      'files'         => '',
      'terms'         => array(),
    ),
    '8.0.0-alpha100' => array(
      'name'          => 'drupal 8.0.0-alpha100',
      'version'       => '8.0.0-alpha100',
      'tag'           => '8.0.0-alpha100',
      'version_major' => '8',
      'version_minor' => '0',
      'version_patch' => '0',
      'version_extra' => 'alpha100',
      'status'        => 'published',
      'release_link'  => 'https://www.drupal.org/node/2316617',
      'download_link' => 'http://ftp.drupal.org/files/projects/drupal-8.0.0-alpha100.tar.gz',
      'date'          => '1407344628',
      'mdhash'        => '9d71afdd0ce541f2ff5ca2fbbca00df7',
      'filesize'      => '9172832',
      'files'         => '',
      'terms'         => array(),
    ),
  ),
);
\Drupal::keyValueExpirable('update_available_releases')->setWithExpire('drupal', $available, 10);

I'll check update_get_available(TRUE) and locale_translation_build_projects().

janoka’s picture

I think that is not the final version. I ask you to give me hints in connection with these patches.

Status: Needs review » Needs work

The last submitted patch, 52: locale-update-2030537-52.patch, failed testing.

The last submitted patch, 52: locale-update-2030537-52-fail.patch, failed testing.

Sutharsan’s picture

From reading the interdiff, IFAIS this is the right direction. Some details:

  1. +++ b/core/modules/locale/src/Tests/LocaleUpdateNotDevTest.php
    @@ -0,0 +1,79 @@
    + * Tests that the language settings on block config appears correctly.
    + *
    

    Wrong description. Pls modify.

  2. +++ b/core/modules/locale/src/Tests/LocaleUpdateNotDevTest.php
    @@ -0,0 +1,79 @@
    +    module_load_include('compare.inc', 'locale');
    +    $admin_user = $this->drupalCreateUser(array('administer modules', 'administer languages', 'access administration pages', 'translate interface'));
    +    $this->drupalLogin($admin_user);
    +    $this->drupalPostForm('admin/config/regional/language/add', array('predefined_langcode' => 'hu'), t('Add language'));
    +
    

    Should be placed in setup().

  3. +++ b/core/modules/locale/src/Tests/LocaleUpdateNotDevTest.php
    @@ -0,0 +1,79 @@
    +    $available['last_fetch'] = time();
    

    Use REQUEST_TIME instead of time() is a small performance improvement. Only use time() if using the /actual/ time is critical.

  4. +++ b/core/modules/locale/tests/modules/locale_test_notdev/locale_test_notdev.info.yml
    @@ -0,0 +1,7 @@
    +name: 'Locale Test NotDev'
    +type: module
    +description: 'The first release with the same major release number which is not a dev release.'
    

    "Not dev" is to me not a very good name for this module. "development release" ('dev' for short) is the best alternative I can think. Either make the description very genric or describe what the module does, not what the related test case is.

  5. +++ b/core/modules/locale/tests/modules/locale_test_notdev/locale_test_notdev.module
    @@ -0,0 +1,20 @@
    + * Change the Drupal version number to a dev one and make the test scripts
    + * to be believe this is not a hidden test module, but a regular custom module.
    + */
    

    Copied from other module? Pls modify.

janoka’s picture

Dear @Sutharsan,
Thanks for your comment! I will do it.

Gábor Hojtsy’s picture

The fails on LocaleUpdateTest do not seem related to the changes introduced here. So I sent #39 for a retest to see how the "fix only" will fair in current D8 with those tests. It passed at the time.

Gábor Hojtsy’s picture

I also ran the fix part of #52 (that is I believe the same as #39) locally with the existing tests in LocaleUpdateTest, and the same fails were produced as in #52, so the fails are not new with #52. They are new due to time spent / alpha release for the pattern looked for being available. So that is down to remote data now being different at this time than it was when #39 was originally written. So I expect the #39 re-run will result in the same 15 fails.

@Sutharsan, @janoka: so looks like we need to fix/update LocaleUpdateTest as well.

janoka’s picture

Status: Needs work » Needs review
FileSize
5.13 KB
6.56 KB
9.86 KB

The last submitted patch, 39: locale-update-2030537-39.patch, failed testing.

The last submitted patch, 60: locale-update-2030537-60-fail.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 60: locale-update-2030537-60.patch, failed testing.

janoka’s picture

Assigned: janoka » Unassigned
janoka’s picture

I hope I could help you.
I will not able to undertake correcting of LocaleUpdateTest on a perfect way.
A good solution would be to simulate the response of drupal.org, like in core/modules/update/tests/modules/update_test.

Sutharsan’s picture

Assigned: Unassigned » Sutharsan

Taking a look at LocaleUpdateTest ...

Sutharsan’s picture

Assigned: Sutharsan » Unassigned
Status: Needs work » Needs review
FileSize
8.71 KB
2.29 KB

The test errors are caused by a working update system (Update module). Now that the version pattern is corrected the translation update system actually goes out to l.d.o and fetches a translation there. This problem was not discovered before probably because the update system at d.o was not yet working.

I fixed this by overriding the path at which the translation update system is looking for a translation. Therefore it will not go out to l.d.o to check for a translation. Drupal core will always be untranslated. As a bonus, this patch override on the path at which the update module checks for update status data. This prevents update module to go out to d.o to check for available translations. Both changes make the system under test independent from (l.)d.o services.

[edited: for clarity]

Status: Needs review » Needs work

The last submitted patch, 67: locale-update-2030537-67.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review

@Sutharsan: great, this should also speed up test runs a great deal. :)

Gábor Hojtsy’s picture

BTW sent for retest because HEAD got broke by a commit of #2148199: Use snapshot to warn users if the configuration has changed since last import, and the fails looked totally due to that.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks amazing. Fixes an actual user bug. Speeds up tests. Small patch. More tests. What's not to love? :)

janoka’s picture

@Sutharsan, @Gábor sounds good!

webchick’s picture

Priority: Major » Critical
Status: Reviewed & tested by the community » Fixed

This actually smells critical to me. Great fix.

Just found two small things, fixed them on commit:

  1. +++ b/core/modules/locale/src/Tests/LocaleUpdateBase.php
    @@ -42,13 +42,19 @@
    -  public static $modules = array('update', 'locale', 'locale_test');
    +  public static $modules = array('update', 'update_test', 'locale', 'locale_test',);
    

    that trailing comma looks odd.

  2. +++ b/core/modules/locale/tests/modules/locale_test_not_development_release/locale_test_not_development_release.module
    @@ -0,0 +1,20 @@
    +  if(isset($info['package']) && $info['package']=='Core') {
    

    missing some whitespace near the if and ==

Committed and pushed to 8.x. Thanks!

  • webchick committed 301cbca on 8.0.x
    Issue #2030537 by janoka, Sutharsan, Gábor Hojtsy | Outi: Fixed...
Sutharsan’s picture

Created follow-up cleanup issue #2338011: Cleanup of locale tests

Status: Fixed » Closed (fixed)

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

Gábor Hojtsy’s picture

Issue tags: -sprint