Bug explanation:
When setting the storage location to a directory and the download method to
"local and remote files", the first update downloads the file(s) to the local
storage location, but the next update will see the local file as newer than the
remote file, choosing the local file. The result is having to update the
translations 2 times!

To recap in one sentence: The first time l10n_update downloads remotely, the
next time it sees the local file (that it just downloaded in the last update) as
an update and updates the system with it.

I think this is what's going on:
When l10n_update downloads a file remotely, it saves the timestamp of this
remote file from its header in the history (the table {l10n_update_file}).
On the next update, it will compare the timestamp set in the history with the
mtime of the local file, seeing that the local file is newer, thinking it's an
update. This leads to a second update from the same file.

Steps to reproduce:

  1. Download and extract Drupal
    $ sudo -u www-data drush dl
  2. Make translations directory
    $ cd drupal-7.2
    $ sudo -u www-data mkdir -p sites/all/translations
  3. Download and extract l10n_update together with some modules for l10n_update
    to update translations for
    $ sudo -u www-data drush dl l10n_update-1.x-dev wysiwyg admin_menu advanced_help
  4. Set up databases
    $ mysqladmin -u root -p create testsite1
    $ mysqladmin -u root -p create testsite2
    mysql> GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, INDEX, ALTER ON testsite1.* TO 'testsite1'@'localhost' IDENTIFIED BY 'password';
    mysql> GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, INDEX, ALTER ON testsite2.* TO 'testsite2'@'localhost' IDENTIFIED BY 'password';
    mysql> exit
  5. Install the sites
    $ sudo -u www-data drush site-install standard --db-url="mysql://testsite1:password@localhost/testsite1" --uri=http://testsite1.local --sites-subdir=testsite1.local -v
    $ sudo -u www-data drush site-install standard --db-url="mysql://testsite2:password@localhost/testsite2" --uri=http://testsite2.local --sites-subdir=testsite2.local -v
  6. Enable some modules together with l10n_update for the first site
    $ drush en --uri=http://testsite1.local l10n_update admin_menu
  7. Enable some modules together with l10n_update for the second site, making
    sure that it has at least 1 module that the first site hasn't
    $ drush en --uri=http://testsite2.local l10n_update admin_menu advanced_help
  8. Set the storage location for both sites
    $ drush vset --uri=http://testsite1.local --yes l10n_update_download_store "sites/all/translations"
    $ drush vset --uri=http://testsite2.local --yes l10n_update_download_store "sites/all/translations"
  9. Add predefined language to first site
    The language French has been created and can now be used. More information is available on the help screen.
    One translation file imported for the enabled modules.
    2 projects updated: admin_menu, drupal.
    French translation strings added: 4676, updated: 0, deleted: 0.
    $ ls -Al sites/all/translations
    -rw-r--r-- 1 www-data www-data  12963 2011-06-25 19:08 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data 701342 2011-06-25 19:08 drupal-7.2.fr.po
  10. Add predefined language to second site
    The language French has been created and can now be used. More information is available on the help screen.
    2 translation files imported for the enabled modules.
    3 projects updated: admin_menu, advanced_help, drupal.
    French translation strings added: 4677, updated: 0, deleted: 0.
    $ ls -Al sites/all/translations
    -rw-r--r-- 1 www-data www-data  12963 2011-06-25 19:08 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data   1844 2011-06-25 19:18 advanced_help-7.x-1.0-beta1.fr.po
    -rw-r--r-- 1 www-data www-data 701342 2011-06-25 19:08 drupal-7.2.fr.po
  11. Do another check for translation updates for the first site
    $ sudo -u www-data drush l10n-update -l http://testsite1.local
    Fetching update information for all projects / all languages.
    Found 2 projects to update.
    Updating translation.
    Downloading and importing files.
    2 projects updated: admin_menu, drupal.
    French translation strings added: 0, updated: 4724, deleted: 0.
    $ ls -Al sites/all/translations
    -rw-r--r-- 1 www-data www-data  12963 2011-06-25 19:08 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data   1844 2011-06-25 19:18 advanced_help-7.x-1.0-beta1.fr.po
    -rw-r--r-- 1 www-data www-data 701342 2011-06-25 19:08 drupal-7.2.fr.po
  12. Do another check for translation updates for the second site
    $ sudo -u www-data drush l10n-update -l http://testsite2.local
    Fetching update information for all projects / all languages.
    Found 1 projects to update.
    Updating translation.
    Downloading and importing files.
    One project updated: advanced_help.
    French translation strings added: 0, updated: 15, deleted: 0.
    $ ls -Al sites/all/translations
    -rw-r--r-- 1 www-data www-data  12963 2011-06-25 19:08 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data   1844 2011-06-25 19:18 advanced_help-7.x-1.0-beta1.fr.po
    -rw-r--r-- 1 www-data www-data 701342 2011-06-25 19:08 drupal-7.2.fr.po

This proves the unintended behaviour of timestamps in the history. The first
site remotely downloaded 2 files. On the second update, these were the local
files that were seen as an update. The second site remotely downloaded 1 file
(using the already downloaded files for the 2 other modules). On the second
update this was the file that was seen as an update.

Possible solution:
When a remote file is downloaded, set the timestamp in the history as the mtime
of the downloaded file, instead of taking the timestamp from the header of the
remote file. This way the timestamp in the history will be the same as the
timestamp of the local file. On the next update, l10n_update will not see these
local files as an update to what is already installed.

I would be happy to provide more information and test further to find a
solution to this.

Comments

sutharsan’s picture

Status: Active » Needs review
StatusFileSize
new2.46 KB

I found the same an inconsistency between the content of 'timestamp' for files which were downloaded and files which were imported from sites/all/translations. In the first case the timestamp is time/date of the po-file is generated by the packaging script. In the second case the timestamp is the file's mtime. I came to the same conclusion and used the download timestamp instead of the header/packaging script time/date.

Try the attached patch.

Tor Arne Thune’s picture

Status: Needs review » Needs work

Okay, so I followed the steps to reproduce, but applied the patch after step 5.
The results are in steps 10-13 below. As can be seen, l10n_update finds no
updates on the initial enabling of the language or on invoking l10n-update after.

  1. Download and apply the patch
    $ sudo -u www-data wget -P sites/all/modules/l10n_update http://drupal.org/files/issues/l10n_update-1200032-1.patch
    $ sudo -u www-data git apply -v --directory sites/all/modules/l10n_update sites/all/modules/l10n_update/l10n_update-1200032-1.patch
    sites/all/modules/l10n_update/l10n_update-1200032-1.patch:12: trailing whitespace.
     * Eliminates a difference between the download time 
    Checking patch sites/all/modules/l10n_update/l10n_update.check.inc...
    Checking patch sites/all/modules/l10n_update/l10n_update.drush.inc...
    Applied patch sites/all/modules/l10n_update/l10n_update.check.inc cleanly.
    Applied patch sites/all/modules/l10n_update/l10n_update.drush.inc cleanly.
    warning: 1 line adds whitespace errors.
  2. (...)
  3. (...)
  4. (...)
  5. Add predefined language to first site
    The language French has been created and can now be used. More information is available on the help screen.
    One translation file imported for the enabled modules.
    $ ls -Al sites/all/translations
    total 0
  6. Add predefined language to second site
    The language French has been created and can now be used. More information is available on the help screen.
    2 translation files imported for the enabled modules.
    $ ls -Al sites/all/translations
    total 0
  7. Do another check for translation updates for the first site
    $ sudo -u www-data drush l10n-update -l http://testsite1.local
    Fetching update information for all projects / all languages.
    All translations up to date
    $ ls -Al sites/all/translations
    total 0
  8. Do another check for translation updates for the second site
    $ sudo -u www-data drush l10n-update -l http://testsite2.local
    Fetching update information for all projects / all languages.
    All translations up to date
    $ ls -Al sites/all/translations
    total 0
sutharsan’s picture

StatusFileSize
new2.46 KB

Rerolled patch against recent dev.

sutharsan’s picture

Status: Needs work » Needs review
StatusFileSize
new2.56 KB

Found a small bug in the patch. Added some comment to the part where the magic happens.

sutharsan’s picture

StatusFileSize
new2.56 KB

Patch without trailing spaces.

Tor Arne Thune’s picture

Status: Needs review » Reviewed & tested by the community

Repeated my steps to reproduce but applied your new patch in #4. Looks like it worked! Great work.

  1. Download and apply the patch
    $ sudo -u www-data wget -P sites/all/modules/l10n_update http://drupal.org/files/l10n_update-1200032-4.patch
    $ sudo -u www-data git apply -v --directory sites/all/modules/l10n_update sites/all/modules/l10n_update/l10n_update-1200032-4.patch
    sites/all/modules/l10n_update/l10n_update-1200032-4.patch:25: trailing whitespace.
        // We override the file timestamp here. The default file time stamp is the 
    sites/all/modules/l10n_update/l10n_update-1200032-4.patch:26: trailing whitespace.
        // file creation time on the l.d.o server. We change the time to the 
    sites/all/modules/l10n_update/l10n_update-1200032-4.patch:27: trailing whitespace.
        // creation on the webserver. On multi sites which share a common 
    Checking patch sites/all/modules/l10n_update/l10n_update.check.inc...
    Checking patch sites/all/modules/l10n_update/l10n_update.drush.inc...
    Applied patch sites/all/modules/l10n_update/l10n_update.check.inc cleanly.
    Applied patch sites/all/modules/l10n_update/l10n_update.drush.inc cleanly.
    warning: 3 lines add whitespace errors.
  2. (...)
  3. (...)
  4. (...)
  5. Add predefined language to first site
    The language French has been created and can now be used. More information is available on the help screen.
    One translation file imported for the enabled modules.
    3 projects updated: admin_menu, drupal, l10n_update.
    French translation strings added: 4760, updated: 0, deleted: 0.
    $ ls -Al sites/all/translations
    total 724
    -rw-r--r-- 1 www-data www-data  12963 2012-01-23 21:36 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data 705903 2012-01-23 21:37 drupal-7.10.fr.po
    -rw-r--r-- 1 www-data www-data  14763 2012-01-23 21:42 l10n_update-7.x-1.0-beta2.fr.po
  6. Add predefined language to second site
    The language French has been created and can now be used. More information is available on the help screen.
    One translation file imported for the enabled modules.
    4 projects updated: admin_menu, advanced_help, drupal, l10n_update.
    French translation strings added: 4775, updated: 0, deleted: 0.
    total 728
    -rw-r--r-- 1 www-data www-data  12963 2012-01-23 21:36 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data   2041 2012-01-23 21:45 advanced_help-7.x-1.0.fr.po
    -rw-r--r-- 1 www-data www-data 705903 2012-01-23 21:37 drupal-7.10.fr.po
    -rw-r--r-- 1 www-data www-data  14763 2012-01-23 21:42 l10n_update-7.x-1.0-beta2.fr.po
  7. Do another check for translation updates for the first site
    $ sudo -u www-data drush l10n-update -l http://testsite1.local
    Fetching update information for all projects / all languages.
    All translations up to date
    $ ls -Al sites/all/translations
    total 728
    -rw-r--r-- 1 www-data www-data  12963 2012-01-23 21:36 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data   2041 2012-01-23 21:45 advanced_help-7.x-1.0.fr.po
    -rw-r--r-- 1 www-data www-data 705903 2012-01-23 21:37 drupal-7.10.fr.po
    -rw-r--r-- 1 www-data www-data  14763 2012-01-23 21:42 l10n_update-7.x-1.0-beta2.fr.po
  8. Do another check for translation updates for the second site
    $ sudo -u www-data drush l10n-update -l http://testsite2.local
    Fetching update information for all projects / all languages.
    All translations up to date
    $ ls -Al sites/all/translations
    total 728
    -rw-r--r-- 1 www-data www-data  12963 2012-01-23 21:36 admin_menu-7.x-3.0-rc1.fr.po
    -rw-r--r-- 1 www-data www-data   2041 2012-01-23 21:45 advanced_help-7.x-1.0.fr.po
    -rw-r--r-- 1 www-data www-data 705903 2012-01-23 21:37 drupal-7.10.fr.po
    -rw-r--r-- 1 www-data www-data  14763 2012-01-23 21:42 l10n_update-7.x-1.0-beta2.fr.po
sutharsan’s picture

Status: Reviewed & tested by the community » Needs review

This patch also fixes this scenario:

  • Single site, Using Local and remote translations, Translations are saved to disk
  • Add a new language. Translations get imported.
  • Go to the Translation status page and click 'Refresh information'.

Without the patch the modules turn Yellow (translations available). With the patch they stay green.

Tor Arne Thune’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new32.5 KB
new32.58 KB

Yes, works like you described. Installed sites with "Using Local and remote translations" and translations saved to "sites/all/translations".

Installing a new site then adding a predefined language and checking for translation updates after the import (without patch applied): http://drupal.org/files/TranslationUpdatesWithoutPatch.png

Installing a new site then adding a predefined language and checking for translation updates after the import (with patch applied): http://drupal.org/files/TranslationUpdatesWithPatch_0.png

sutharsan’s picture

Status: Reviewed & tested by the community » Fixed
StatusFileSize
new3.86 KB

This patch is now committed. Additionally it removes the import_date field which no longer has valuable data.

Status: Fixed » Closed (fixed)

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