Closed Bug 933290 Opened 9 years ago Closed 9 years ago

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

Categories

(Release Engineering :: General, defect)

defect
Not set
major

Tracking

(firefox28 affected, fennec28+)

RESOLVED FIXED
Tracking Status
firefox28 --- affected
fennec 28+ ---

People

(Reporter: flod, Assigned: aki)

References

Details

Attachments

(1 file)

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
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).
Version: 28 Branch → Trunk
Component: General → Releases
Product: Firefox → Release Engineering
QA Contact: rail
Version: Trunk → other
Component: Releases → General
Product: Release Engineering → Firefox for Android
QA Contact: rail
Version: other → Trunk
tracking-fennec: --- → ?
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...
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%).
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.
The only one I find somehow related to build and mobile
http://hg.mozilla.org/mozilla-central/rev/7d1973314652
(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.
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.
Aaron: did you intend to move this back into the Fennec component?
Blocks: 933272
Severity: normal → major
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.
Where is MOZ_CHROME_MULTILOCALE supposed to be set? It's set nowhere in mozilla-central, including mozconfigs.
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.
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).
Component: General → Other
Product: Firefox for Android → Release Engineering
QA Contact: joduinn
Version: Trunk → unspecified
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)
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.
(See also wishlist Bug 934196.)
(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.
(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.
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.
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.
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...
Attached patch fix_multiSplinter Review
Attachment #828223 - Flags: review?(bugspam.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.
Attachment #828223 - Flags: review?(mh+mozilla)
Attachment #828223 - Flags: review?(bugspam.Callek)
Attachment #828223 - Flags: review+
Attachment #828223 - Flags: review?(mh+mozilla) → review+
tracking-fennec: ? → 28+
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.
Attachment #828223 - Flags: checked-in+
Assignee: nobody → aki
in production
Kicked off a new nightly.
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/
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?
Awesome.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.