613411 - package multilocale fennec builds with MOZ_CHROME_MULTILOCALE set

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Aki Sasaki (not active)]

Bug 575751 is a Fennec 4.0b3+ blocker, and requires that MOZ_CHROME_MULTILOCALE be set to a whitespace-delimited list of locales in the multilocale build during the multilocale |make package| step.

This means:

1. Android nightly multilocale builds in the buildbotcustom default branch.
Multilocale logic here is controlled by mozharness/scripts/multil10n.py.

  http://hg.mozilla.org/build/mozharness/file/bb8ae419ccaa/scripts/multil10n.py#l327

Maybe

    if package_type == multi:
        env['MOZ_CHROME_MULTILOCALE'] = ' '.join(self.locales)
        # does this need quotes around it or not?

2. Android release multilocale builds in the buildbotcustom buildbot-0.7 branch.  Multilocale logic here is also controlled by mozharness/scripts/multil10n.py, so you may fix both at the same time, but we need to test both.

3. Maemo nightly multilocale builds in the buildbotcustom buildbot-0.7 branch.

The packaging is here:
  http://hg.mozilla.org/build/buildbotcustom/file/e9c21e9c7b10/process/factory.py#l5923

a) I'm not sure if adding MOZ_CHROME_MULTILOCALE to extraArgs will work as written, because extraArgs is used in both |make package| and |make package-tests|, and because extraArgs is a single string inside a command list. You may need to change extraArgs to a list, or |'make package ' + extraArgs| or something.
You probably need quotes, e.g. MOZ_CHROME_MULTILOCALE="ar be da ..."

b) You'll probably want to self.locales to build this list, which means you should probably define self.locales in the else: here:
  http://hg.mozilla.org/build/buildbotcustom/file/e9c21e9c7b10/process/factory.py#l6013

4. Maemo release multilocale builds will pick up this fix as well, but needs testing.

To test, a) create a user repo b) check in mwu's patch c) run an 0.8.x Android m-c nightly against it (force build works), d) run an 0.7.x Maemo m-c nightly against it (you need to change the nightly scheduler to do this, not force build!)

It would be easiest to test the release builds by doing a staging release after this has landed.  Lukas knows how to do one; you can leave this up to her (ok) or ask her to show you how to do a staging release (better, if feasible).

Comment 1

[Aki Sasaki (not active)]

After nightly testing passes, make sure you coordinate landing your patches with mwu.

Comment 2

[Aki Sasaki (not active)]

Oh -- to double check, unzip the resulting apk and |ar x| the deb and check the contents for the appropriate locales.  If you need, compare vs. an official multilocale nightly.

Comment 3

[Mike Taylor [:bear]]

Attachment: mozharness changes

Comment 4

[Mike Taylor [:bear]]

Attachment: buildbot-configs changes for multi-locale

Comment 5

[Mike Taylor [:bear]]

Attachment: buildbot-custom changes for multi-locale - 1

Comment 6

[Aki Sasaki (not active)]

Comment on attachment 495529 [details] [diff] [review]
mozharness changes

I realize this is my bug from comment 0, but this should probably be |if package_type == 'multi':| (with quotes)

Comment 7

[Aki Sasaki (not active)]

Comment on attachment 495531 [details] [diff] [review]
buildbot-configs changes for multi-locale

>-MOBILE_BRANCHES['mobile-trunk']['repo_path'] = 'mozilla-central'
>+MOBILE_BRANCHES['mobile-trunk']['repo_path'] = 'http://hg.mozilla.org/users/mtaylor_mozilla.com/613411_mozilla-central'

Shouldn't this be

MOBILE_BRANCHES['mobile-trunk']['repo_path'] = 'users/mtaylor_mozilla.com/613411_mozilla-central'

?

Comment 8

[Mike Taylor [:bear]]

hmm, i'll double check to see if I have a clean patch - I learned the hard way that it won't run with that value :)

Comment 9

[Aki Sasaki (not active)]

Comment on attachment 495532 [details] [diff] [review]
buildbot-custom changes for multi-locale - 1

>-                command = ['perl', 'tools/release/version-bump.pl',
>-                           '-w', repoName, '-t', self.releaseTag, '-a', appName,
>+                command = ['perl', 'tools/scripts/release/version-bump.pl',
>+                           '-w', repoName,  '-a', appName,

Not sure why this is here?

>+                 locales=None,

Not sure why you're adding locales to MobileBuildFactory.
Android handles locales through mozharness; Maemo handles it in MaemoBuildFactory.

>+        if self.multiLocale:
>+            makePackageCommand += ['AB_CD=%s' % locale, 'MOZ_CHROME_MULTILOCALE="%s"' % ' '.join(self.locales)]

Aha.
However, since make package happens multiple times (en-US and multi) you need to check to see if locale == 'multi' before adding this.

An easier solution might be to move the packaging for android multilocale to mozharness.

Comment 10

[Mike Taylor [:bear]]

(In reply to comment #9)
> Comment on attachment 495532 [details] [diff] [review]
> buildbot-custom changes for multi-locale - 1
> 
> >-                command = ['perl', 'tools/release/version-bump.pl',
> >-                           '-w', repoName, '-t', self.releaseTag, '-a', appName,
> >+                command = ['perl', 'tools/scripts/release/version-bump.pl',
> >+                           '-w', repoName,  '-a', appName,
> 
> Not sure why this is here?
> 

ahh - that's a build infra fix I needed to keep working - was a bug that I think may have landed earlier


> >+                 locales=None,
> 
> Not sure why you're adding locales to MobileBuildFactory.
> Android handles locales through mozharness; Maemo handles it in
> MaemoBuildFactory.

without it my checkconfig was failing

> 
> >+        if self.multiLocale:
> >+            makePackageCommand += ['AB_CD=%s' % locale, 'MOZ_CHROME_MULTILOCALE="%s"' % ' '.join(self.locales)]
> 
> Aha.
> However, since make package happens multiple times (en-US and multi) you need
> to check to see if locale == 'multi' before adding this.
> 
> An easier solution might be to move the packaging for android multilocale to
> mozharness.

moving it to mozharness makes a lot of sense

Comment 11

[Aki Sasaki (not active)]

Attachment: mozharness diff for MOZ_CHROME_MULTILOCALE

This adds AB_CD=multi to the command, adds the MOZ_CHROME_MULTILOCALE env var, and adds make package-tests to the package actions.

A multilocale android nightly staging build worked properly, with the following in its logs:

14:29:51     INFO - MOZ_CHROME_MULTILOCALE is cs da de es-ES fi fr it nb-NO nl pl pt-PT ru
<snip a few lines>
14:30:02     INFO -  printf "\n[multilocale]\n" >> package-manifest
14:30:02     INFO -  for LOCALE in cs da de es-ES fi fr it nb-NO nl pl pt-PT ru ;\
14:30:02     INFO -  	do \
14:30:02     INFO -  	  printf "bin/chrome/$LOCALE\n" >> package-manifest; \
14:30:02     INFO -  	  printf "bin/chrome/$LOCALE.manifest\n" >> package-manifest; \
14:30:02     INFO -  	done

Comment 12

[Aki Sasaki (not active)]

Attachment: buildbotcustom 08x MOZ_CHROME_MULTILOCALE

No changes except moving the multilocale packaging steps into mozharness.

Comment 13

[Aki Sasaki (not active)]

Doing this in Maemo land got me a deb, but it wasn't multilocale.

In the objdir:

[cltbld@linux-ix-slave02 objdir]$ find . -name nl\*
./dist/manifests/multilocale/chrome/nl.manifest
./dist/bin/chrome/nl.manifest
./dist/bin/chrome/nl.jar
./dist/xpi-stage/locale-nl/chrome/nl.manifest
./dist/xpi-stage/locale-nl/chrome/nl.jar

In the log:

printf "\n[multilocale]\n" >> package-manifest
for LOCALE in cs da de es-ES fi fr it nb-NO nl pl pt-PT ru ;\
	do \
	  printf "bin/chrome/$LOCALE\n" >> package-manifest; \
	  printf "bin/chrome/$LOCALE.manifest\n" >> package-manifest; \
	done
Creating package directory...
rm -f -rf ../../dist/xpt rm -f -rf ../../dist/manifests
/scratchbox/tools/bin/perl -I/home/cltbld/build/mobile-trunk-maemo5-gtk-nightly/mozilla-central/xpinstall/packager -e 'use Packager; Packager::Copy( "/home/cltbld/build/mobile-trunk-maemo5-gtk-nightly/mozilla-central/objdir/mobile/installer/../../dist", "/home/cltbld/build/mobile-trunk-maemo5-gtk-nightly/mozilla-central/objdir/mobile/installer/../../dist/fennec", "package-manifest", "unix", 1, 0, 1);'
[multi]
Warning: package error or possible missing or unnecessary file: bin/chrome/multi.jar (package-manifest, 14).
Warning: package error or possible missing or unnecessary file: bin/chrome/multi.manifest (package-manifest, 15).
bin/defaults/pref/mobile-l10n.js
Warning: package error or possible missing or unnecessary file: bin/defaults/profile/bookmarks.html (package-manifest, 18).
Warning: package error or possible missing or unnecessary file: bin/defaults/profile/localstore.rdf (package-manifest, 19).
Warning: package error or possible missing or unnecessary file: bin/defaults/profile/mimeTypes.rdf (package-manifest, 20).

<snip a bunch of lines>

[multilocale]
Warning: package error or possible missing or unnecessary file: bin/chrome/cs (package-manifest, 446).
bin/chrome/cs.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/da (package-manifest, 448).
bin/chrome/da.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/de (package-manifest, 450).
bin/chrome/de.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/es-ES (package-manifest, 452).
bin/chrome/es-ES.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/fi (package-manifest, 454).
bin/chrome/fi.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/fr (package-manifest, 456).
bin/chrome/fr.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/it (package-manifest, 458).
bin/chrome/it.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/nb-NO (package-manifest, 460).
bin/chrome/nb-NO.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/nl (package-manifest, 462).
bin/chrome/nl.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/pl (package-manifest, 464).
bin/chrome/pl.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/pt-PT (package-manifest, 466).
bin/chrome/pt-PT.manifest
Warning: package error or possible missing or unnecessary file: bin/chrome/ru (package-manifest, 468).
bin/chrome/ru.manifest

I'm going to guess that putting these into jar files is breaking the manifest?

Comment 14

[Aki Sasaki (not active)]

Attachment: buildbotcustom 07x MOZ_CHROME_MULTILOCALE

* add android changes (in the 08x patch) to 07x for android release builds
* add MOZ_CHROME_MULTILOCALE to env for maemo packaging if multilocale build
* add env=env, -k scratchbox option to actually use that env in make package, make deb steps
* set self.locale, use that instead of a local 'locale' variable in maemo multilocale; this is later used to build MOZ_CHROME_MULTILOCALE

When maemo multilocale moves to 08x, it'll use mozharness and should Just Work.

Comment 15

[Aki Sasaki (not active)]

Comment on attachment 496716 [details] [diff] [review]
buildbotcustom 07x MOZ_CHROME_MULTILOCALE

Feel up to reviewing this?
The staging maemo 5 gtk multilocale mozilla-central build created a deb with locale jars in 07x land with this patch + mwu's 4 patches.

Comment 16

[Aki Sasaki (not active)]

Comment on attachment 496006 [details] [diff] [review]
buildbotcustom 08x MOZ_CHROME_MULTILOCALE

If you think someone else should review these, or need a second opinion, mark as such or let me know.

Comment 17

[Aki Sasaki (not active)]

Comment on attachment 496005 [details] [diff] [review]
mozharness diff for MOZ_CHROME_MULTILOCALE

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

Comment 18

[Aki Sasaki (not active)]

Comment on attachment 496716 [details] [diff] [review]
buildbotcustom 07x MOZ_CHROME_MULTILOCALE

http://hg.mozilla.org/build/buildbotcustom/rev/3255cbcbc9a8

Comment 19

[Aki Sasaki (not active)]

Comment on attachment 496006 [details] [diff] [review]
buildbotcustom 08x MOZ_CHROME_MULTILOCALE

http://hg.mozilla.org/build/buildbotcustom/rev/da493d9a8e9f