1383388 - Port bug 1362617 to C-C [Generalize MOZ_CHROME_MULTILOCALE to work for browser as well]

Status: RESOLVED FIXED

(Thunderbird :: Build Config, enhancement)

Comment 1

[Richard Marti (:Paenglab)]

Attachment: multilocale.patch

Not tested but it should work. The path in

DPKG_LOCALE_MANIFEST=$(topobjdir)/toolkit/locales/locale-manifest.in

looks correct in my objdir.

Comment 2

[Jorg K (CEST = GMT+2)]

Looks good, try here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3a3302f65e5386a9d35cc43ae371c8645a5ecffc

Comment 3

[Jorg K (CEST = GMT+2)]

Comment on attachment 8889038 [details] [diff] [review]
multilocale.patch

That got built.

Comment 4

[Richard Marti (:Paenglab)]

Bug 1362617 was backed-out. See bug 1362617 comment 21.

Comment 5

[Richard Marti (:Paenglab)]

The changes in bug 1362617 are now too complicated for my null makefile knowledge. This can someone do which knows what he does.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

Jorg - heads up. We're getting ready to land it.

The two differences I believe for this patch is that we now added `MOZ_CHROME_LOCALE_ENTRIES` to your locales/Makefile.in which has the list of entries for each locale.

Based on the resources I see you removing from package-manifest.in, I think you want to set it to:

```
MOZ_CHROME_LOCALE_ENTRIES=@RESPATH@/chrome/
```

and it'll populate in PKG_LOCALE_MANIFEST.

For `PKG_LOCALE_MANIFEST` instead of pointing it at toolkit, you'll now have it in the objdir matching your locales dir, so sth like:

```
DEFINES += -DPKG_LOCALE_MANIFEST=$(topobjdir)/{mail|suite|im}/locales/locale-manifest.in
```

Let me know if you need more help! I'll be happy to look at the patch

Comment 7

[Jorg K (CEST = GMT+2)]

Attachment: 1383388-multilocale.patch (v2)

Thanks for the heads-up, Zibi. I really have no idea what I'm doing here, so perhaps Tom can take it from here.

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8921997 [details] [diff] [review]
1383388-multilocale.patch (v2)

lgtm!

Comment 9

[Tom Prince [:tomprince]]

Comment on attachment 8921997 [details] [diff] [review]
1383388-multilocale.patch (v2)

Review of attachment 8921997 [details] [diff] [review]:
-----------------------------------------------------------------

This look reasonable.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8d4167a5c1e0
Port bug 1362617 to C-C [Generalize MOZ_CHROME_MULTILOCALE to work for browser as well]. r=zibi,tomprince CLOSED TREE

Comment 11

[Jorg K (CEST = GMT+2)]

I had to land this right on top of a whole lot of bustage since bug 1362617 landed.

Comment 12

[Jorg K (CEST = GMT+2)]

That busted the build:

mozbuild.preprocessor.Error: ('/builds/slave/tb-c-cen-lx-000000000000000000/build/mail/installer/package-manifest.in', 66, 'FILE_NOT_FOUND', '/builds/slave/tb-c-cen-lx-000000000000000000/build/objdir-tb/mail/locales/locale-manifest.in')

https://hg.mozilla.org/comm-central/rev/8d4167a5c1e0#l4.13
+DEFINES += -DPKG_LOCALE_MANIFEST=$(topobjdir)/mail/locales/locale-manifest.in

Who builds that?
Looks like M-C now settled for:
https://hg.mozilla.org/mozilla-central/rev/9ded8c9fd0d2#l2.12
+DEFINES += -DPKG_LOCALE_MANIFEST=$(topobjdir)/browser/installer/locale-manifest.in
+MOZ_CHROME_LOCALE_ENTRIES=@RESPATH@/browser/chrome/ @RESPATH@/chrome/

Zibi, any hints?

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: 1383388-follow-up.patch (v1)

This makes a little more like M-C:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d6999483fd3ba1b164334adc62c36f57ecdd23ce

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: 1383388-follow-up.patch (v2)

Try was successful, here the patch also for IB and SM.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/395acea4a3e7
Follow-up: Adapt to landed M-C version. rs=bustage-fix CLOSED TREE

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Oh, my fault. Yes, we had to change a couple things after I reviewed your patch and I failed to update it here. Glad it works now :)

Comment 17

[Jorg K (CEST = GMT+2)]

The only doubt I had was whether
  MOZ_CHROME_LOCALE_ENTRIES=@RESPATH@/chrome/
is enough or we should do it like FF
  MOZ_CHROME_LOCALE_ENTRIES=@RESPATH@/mail/chrome/ @RESPATH@/chrome/

Works with the former.

Comment 18

[Zibi Braniecki [:zbraniecki][:gandalf]]

The only big we moved is the entries in `/mail/installer/package-manifest.in`, which for mail were:

```
-@RESPATH@/chrome/@AB_CD@@JAREXT@
-@RESPATH@/chrome/@AB_CD@.manifest
```

are now generated from python *per locale*. For now it doesn't change much because we're building only single locale builds, but later, we may be able to just do `MOZ_CHROME_MULTILOCALE="fr pl de" ./mach package` and get chrome entries packaged for all of those locales.

So yes, in case of mail|im|suite you only need `MOZ_CHROME_LOCALE_ENTRIES=@RESPATH@/chrome/` :)

Comment 19

[Tom Prince [:tomprince]]

Attachment: Bug 1383388: Followup to support building in m-c topdir as well;

Review commit: https://reviewboard.mozilla.org/r/195910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/195910/

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/87226f1198cc
Followup to support building in m-c topdir as well; r=me