933290 - Firefox 28: about pages not localized in multi-locale build

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Francesco Lodolo [:flod]]

I've just updated my phone to Firefox 28.0a1 (2013-10-31) and all about pages moved from Italian to English. 

I checked about:, about:plugins, about:config and they're all displayed in English. Preferences are still in Italian though.

I don't have a clear regression range since I don't update this phone often. Aurora is working fine but it's still on 26.0a2

Comment 1

[Francesco Lodolo [:flod]]

Downloaded apk from here
http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-central-android/

assets/omni.ja -> chrome has only the en-US folder (same folder in Aurora has all locales).

Comment 2

[Francesco Lodolo [:flod]]

This one is the last build with the right folders
http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2013-10-29-03-02-01-mozilla-central-android/

First wrong build (1 MB less)
http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2013-10-30-03-02-01-mozilla-central-android/

Log
http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2013-10-30-03-02-01-mozilla-central-android/mozilla-central-android-nightly-bm62-build1-build0.txt.gz

Comment 3

[Mike Hommey [:glandium]]

From the log:

07:19:28     INFO -  it:
07:19:28     INFO -  keys: 366
07:19:28     INFO -  unchanged: 508
07:19:28     INFO -  changed: 4507
07:19:28     INFO -  errors: 1
07:19:28     INFO -  89% of entries changed
07:19:28     INFO - Return code: 0
07:19:28     INFO - *** END compare-locales it

That's a lot of strings changed...

Comment 4

[Francesco Lodolo [:flod]]

Interesting: errors 1? I don't see any error in my locale
https://l10n.mozilla.org/shipping/dashboard?locale=it

But the last build is from Oct 29 :-\
https://l10n.mozilla.org/builds/builders/compare/342805

Changed: it's the number of strings that in Italian are different (changed) from en-US, so that's expected (about 90%).

Comment 5

[Axel Hecht [:Pike]]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=518f5bff0ae4&tochange=829d7bef8b0a would be the regression range based on what flod tested, nothing in there that triggers my spider senses.

I also don't see anything in the logs that differs notably to me, but I don't understand much about our logs myself.

PS: The error is an outdated version of compare-locales.

Comment 6

[Francesco Lodolo [:flod]]

The only one I find somehow related to build and mobile
http://hg.mozilla.org/mozilla-central/rev/7d1973314652

Comment 7

[Rail Aliiev [:rail]]

(In reply to Francesco Lodolo [:flod] from comment #6)
> The only one I find somehow related to build and mobile
> http://hg.mozilla.org/mozilla-central/rev/7d1973314652

This change affects release builds only (beta and final releases). Nightly builds use http://hg.mozilla.org/mozilla-central/file/default/mobile/android/config/mozconfigs/android/nightly which is not affected by that change.

Comment 8

[Richard Newman [:rnewman]]

Also affects local builds with the multilocale script. Locales make it into the objdir:

./objdir-droid/dist/bin/chrome/es-ES/locale/es-ES/browser/overrides/plugins/plugins.dtd

but they don't make it into omni.ja.

Comment 9

[Richard Newman [:rnewman]]

Aaron: did you intend to move this back into the Fennec component?

Comment 10

[Axel Hecht [:Pike]]

I'm looking at make package AB_CD=multi inside of the logs pre and post, and the section that's missing is

07:08:33     INFO -  printf "\n[multilocale]\n" >> package-manifest
07:08:33     INFO -  for LOCALE in en-US en-US cs da de es-ES fi fr ja ko it nb-NO nl pl pt-BR pt-PT ru sk sv-SE zh-CN zh-TW ;\
07:08:33     INFO -     do \
07:08:33     INFO -       printf "bin/chrome/$LOCALE\n" >> package-manifest; \
07:08:33     INFO -       printf "bin/chrome/$LOCALE.manifest\n" >> package-manifest; \
07:08:33     INFO -     done


I can't tell for the grace of kelly why.

Comment 11

[Axel Hecht [:Pike]]

PS: the code that should trigger that is at http://mxr.mozilla.org/mozilla-central/source/mobile/android/installer/Makefile.in#48

Comment 12

[Mike Hommey [:glandium]]

Where is MOZ_CHROME_MULTILOCALE supposed to be set? It's set nowhere in mozilla-central, including mozconfigs.

Comment 13

[Aki Sasaki (not active)]

http://mxr.mozilla.org/build/search?string=MOZ_CHROME_

Comment 14

[Mike Hommey [:glandium]]

Okay, found the problem. So, the responsible changeset is http://hg.mozilla.org/mozilla-central/rev/58ca27d61309 (bug 927837), and the reason it worked before has a lot to do with luck.

So, before, this is what would happen:
- The "normal" build does make package. This creates package-manifest and an en-US apk.
- The multilocale build step does another make package with MOZ_CHROME_MULTILOCALE set.
- That runs make in mobile/android/installer. For some reason, that would trigger config.status to run. Config.status itself would change no file, but the rule executing it also touches Makefile.
- Since Makefile was touched, package-manifest is recreated, including the multilocale stuff
- packager.py is invoked with the new package-manifest.

After, this is what happens:
- The "normal" build does make package. This creates package-manifest and an en-US apk.
- The multilocale build step does another make package with MOZ_CHROME_MULTILOCALE set.
- That runs make in mobile/android/installer. That doesn't trigger config.status and doesn't touch Makefile. Package-manifest is not changed.
- packager.py is invoked again with the non-multilocale package-manifest.

I can see two possible ways out:
- Avoid doing the first make package (which would mean stop doing non-multilocale builds ; I don't know if we want to do that)
- Find a workaround of some sort.

For the latter, one possibility is to set MOZ_PKG_MANIFEST to something different from package-manifest when doing the multilocale package. That would make the second make package create and use a different package manifest, avoiding relying on the previous one being overwritten somehow.

Comment 15

[Mike Hommey [:glandium]]

Either way, it's something to fix on the releng side. Note my proposal in last paragraph could be deployed on all branches, it should work everywhere we do those multilocale builds (that is, it doesn't have to ride the trains).

Comment 16

[Axel Hecht [:Pike]]

Hi Mike, thanks for finding this out.

theory and wishful thinking, the dependencies for package-manifest aren't strong enough, right? And I'd not have an idea on how to make them strong enough. http://stackoverflow.com/questions/14840474/make-target-which-depends-on-an-environment-variable has a few hacks for targets that depend on environment variables.

what happens if we force the regeneration of package-manifest instead? Does that have any fall-out of practical relevance?

I see how it's not the pure science, but looking at the hack on stackoverflow, those aren't pure science either.

(PS: I'm not sold that this is something that releng should work around)

Comment 17

[Mike Hommey [:glandium]]

The problem is that we're effectively doing two different builds in this directory without a cleanup, and there is no way the build system can take that without explicit modifications. OTOH, the fact that these two different builds in this directory without a cleanup is a releng problem that has to be solved in some way. And I think the most reasonable short term solution is to solve it with a releng workaround. Longer term, the package manifest horror show will be gone.

Comment 18

[Richard Newman [:rnewman]]

(See also wishlist Bug 934196.)

Comment 19

[Aki Sasaki (not active)]

(In reply to Mike Hommey [:glandium] from comment #17)
> The problem is that we're effectively doing two different builds in this
> directory without a cleanup, and there is no way the build system can take
> that without explicit modifications. OTOH, the fact that these two different
> builds in this directory without a cleanup is a releng problem that has to
> be solved in some way. And I think the most reasonable short term solution
> is to solve it with a releng workaround. Longer term, the package manifest
> horror show will be gone.

Can we back out whatever caused this issue?
That seems like a much faster short term solution.

Comment 20

[Mike Hommey [:glandium]]

(In reply to Aki Sasaki [:aki] from comment #19)
> Can we back out whatever caused this issue?
> That seems like a much faster short term solution.

I'd rather not, but I'll leave it to gps to comment further.

Comment 21

[Mike Hommey [:glandium]]

FWIW, another work around is to manually touch objdir/mobile/android/installer/Makefile, which would be more reliable than relying on whatever made config.status run.

Comment 22

[Gregory Szorc [:gps]]

I would strongly prefer to not back out bug 927837 because the previous behavior required clobbers and this change also impacted a few other patches that have since landed.

We should modify the automation configs instead. Touching a file to force a config regen or just adding a step to force a config regen seems like a simple and sufficient solution.

Comment 23

[Mike Hommey [:glandium]]

I also spotted that the b2g equivalent solved the problem by doing:
  $(MOZ_PKG_MANIFEST): $(MOZ_PKG_MANIFEST_P) FORCE

Kind of horrible, but would work too...

Comment 24

[Aki Sasaki (not active)]

Attachment: fix_multi

Comment 25

[Justin Wood (:Callek)]

Comment on attachment 828223 [details] [diff] [review]
fix_multi

so so so dirty and painful. I'd love a build system peer to at least ack that this does what we want though, and glandium commented here a few times, so additional-r to him for that ack.

Comment 26

[Aki Sasaki (not active)]

Comment on attachment 828223 [details] [diff] [review]
fix_multi

http://hg.mozilla.org/build/mozharness/rev/c40b9d88f50d

Needs to be merged to production; then the next nightly should have this.

Comment 27

[[e-mail] (:bhearsum)]

in production

Comment 28

[Aki Sasaki (not active)]

Kicked off a new nightly.

Comment 29

[Aki Sasaki (not active)]

16:28:36     INFO -  removed `dist/fennec_ids.txt'
16:28:36     INFO - Return code: 0
16:28:36     INFO - Running command: ['touch', 'mobile/android/installer/Makefile'] in /builds/slave/m-cen-and-ntly-000000000000000/./build/obj-firefox
16:28:36     INFO - Copy/paste: touch mobile/android/installer/Makefile
16:28:36     INFO - Return code: 0
16:28:36     INFO - MOZ_CHROME_MULTILOCALE is en-US cs da de es-ES fi fr ja ko it nb-NO nl pl pt-BR pt-PT ru sk sv-SE zh-CN zh-TW

http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2013-11-11-12-21-33-mozilla-central-android/

Comment 30

[Francesco Lodolo [:flod]]

I can confirm that last update brought back my about: pages, including the new about:config
Is there anything else to be done before closing this bug?

Comment 31

[Aki Sasaki (not active)]

Awesome.