r103763 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103762‎ | r103763 | r103764 >
Date:15:25, 20 November 2011
Author:catrope
Status:resolved (Comments)
Tags:miscextensions 
Comment:
(bug 31982) Handle fallback languages in LocalisationUpdate. Modified patch by Umherirrender
Modified paths:
  • /trunk/extensions/LocalisationUpdate/LocalisationUpdate.class.php (modified) (history)

Diff [purge]

Index: trunk/extensions/LocalisationUpdate/LocalisationUpdate.class.php
@@ -20,14 +20,26 @@
2121 * @return true
2222 */
2323 public static function onRecache( LocalisationCache $lc, $langcode, array &$cache ) {
24 - $cache['messages'] = array_merge(
25 - $cache['messages'],
26 - self::readFile( $langcode )
27 - );
 24+ // Handle fallback sequence and load all fallback messages from the cache
 25+ $codeSequence = array_merge( array( $langcode ), $cache['fallbackSequence'] );
 26+ // Iterate over the fallback sequence in reverse, otherwise the fallback
 27+ // language will override the requested language
 28+ foreach ( array_reverse( $codeSequence ) as $code ) {
 29+ if ( $code == 'en' ) {
 30+ // Skip English, otherwise we end up trying to read
 31+ // the nonexistent cache file for en a couple hundred times
 32+ continue;
 33+ }
 34+
 35+ $cache['messages'] = array_merge(
 36+ $cache['messages'],
 37+ self::readFile( $code )
 38+ );
2839
29 - $cache['deps'][] = new FileDependency(
30 - self::filename( $langcode )
31 - );
 40+ $cache['deps'][] = new FileDependency(
 41+ self::filename( $code )
 42+ );
 43+ }
3244
3345 return true;
3446 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r104041Followup r103763: per Liangent's CR comment, there was a bug where an unchang...catrope15:05, 23 November 2011
r106351Followup r103763: including the messages from previous runs is actually reall...catrope18:36, 15 December 2011

Comments

#Comment by Liangent (talk | contribs)   15:49, 20 November 2011

I guess there's an issue (I didn't really test it):


In WMF branch:

  • zh-hans
    • msg => AA
  • zh-cn
    • msg => B

In trunk:

  • zh-hans
    • msg => A
  • zh-cn
    • msg => B

So in l10ncache:

  • zh-hans
    • msg => A
  • zh-cn
    • (nothing)

onRecache for zh-cn, input:

  • msg => B

after 1st round $codeSequence merge of zh-hans:

  • msg => A

after 2nd round $codeSequence merge of zh-cn:

  • msg => A

output:

  • msg => A

expected:

  • msg => B
#Comment by Umherirrender (talk | contribs)   19:06, 20 November 2011

Yes, that is true. I have at the moment no idea, why.

But now it works, when there is no "zh-cn" message in WMF branch and no "zh-cn" message in trunk. Before this commit, the fallback was never triggered. (msg was than AA, not A). For this case the order is not relevant.

#Comment by Catrope (talk | contribs)   19:20, 20 November 2011

Hmm, yeah, that's a problem. Good point.

#Comment by Umherirrender (talk | contribs)   18:16, 21 November 2011

The cache entry for zh-hans overrides the message, but it is hard to fix, because for the one usecase there should not a merge, but in the other usecase it needs a merge.

#Comment by Catrope (talk | contribs)   13:49, 23 November 2011

Some more notes for myself:

There are three cases where LU will store nothing for a message:

  1. Message is not defined in the trunk language file. This is the case in which we want to use the fallback
  2. Translation in the trunk language file is identical to the message in the WMF branch. Here we don't want to use the fallback
  3. English version differs between trunk and WMF. In this case the message won't be stored in any language, so whether or not we use the fallback doesn't matter

For case #2, I think we'll be fine if we store the translation in the LU cache file even though it's not needed, but I'll have to investigate that in more detail.

#Comment by Catrope (talk | contribs)   15:06, 23 November 2011

I've done #2 in r104041, so this issue should now be fixed.

#Comment by Catrope (talk | contribs)   15:06, 23 November 2011

I meant to say I've done this (storing unchanged messages).

#Comment by Liangent (talk | contribs)   15:41, 23 November 2011

I guess it's still buggy:


WMF:

  • en
    • msg => A
  • de
    • msg => B

trunk:

  • en
    • msg => A
  • de
    • (nothing)

input onRecache de:

  • msg => B

1st round fallback, en, skipping:

  • msg => B

2nd round fallback, de, no msg cached:

  • msg => B

output:

  • msg => B

expected (fallback to en):

  • msg => A
#Comment by Liangent (talk | contribs)   14:28, 27 January 2012

Can we cache English messages as well?

#Comment by 😂 (talk | contribs)   14:19, 27 January 2012

Status on this?

#Comment by Liangent (talk | contribs)   14:27, 27 January 2012

Seems partly fixed and Roan decided to deploy the partly fixed version live.

#Comment by MarkAHershberger (talk | contribs)   01:55, 29 January 2012

changing back to "NEW". If things still need fixing, then restore "FIXME" and give a list of what needs to be fixed still.

#Comment by Liangent (talk | contribs)   12:22, 29 January 2012

If a de message was in wmf-branch and isn't in trunk (and developers expect it to fall back to en), it cannot be updated by LU.

Some ways to fix:

  • Iterate over all keys in English where English text is not changed when updating (= building cache), then calculate (resolve fallback now) its message text in every language and store it in cache. When recaching the message, simply merging cached data into main data.
  • or, include English in l10ncache.
#Comment by Catrope (talk | contribs)   15:28, 8 February 2012

This is a bug, yes, but it's not a regression, because fallbacks weren't handled at all before. Can we file a bug about the missing message behavior and unfixme this revision?

#Comment by Liangent (talk | contribs)   15:47, 8 February 2012

Done. bug 34272.

Status & tagging log