Closed Bug 1383388 Opened 2 years ago Closed 2 years ago

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

Categories

(Thunderbird :: Build Config, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 58.0

People

(Reporter: Paenglab, Assigned: jorgk)

References

Details

Attachments

(3 files, 2 obsolete files)

No description provided.
Attached patch multilocale.patch (obsolete) — Splinter Review
Not tested but it should work. The path in

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

looks correct in my objdir.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8889038 - Flags: review?(jorgk)
Comment on attachment 8889038 [details] [diff] [review]
multilocale.patch

That got built.
Attachment #8889038 - Flags: review?(jorgk) → review+
Keywords: checkin-needed
Bug 1362617 was backed-out. See bug 1362617 comment 21.
Keywords: checkin-needed
Depends on: 1362617
The changes in bug 1362617 are now too complicated for my null makefile knowledge. This can someone do which knows what he does.
Assignee: richard.marti → nobody
Status: ASSIGNED → NEW
Attachment #8889038 - Attachment is obsolete: true
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
Thanks for the heads-up, Zibi. I really have no idea what I'm doing here, so perhaps Tom can take it from here.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8921997 - Flags: review?(mozilla)
Attachment #8921997 - Flags: feedback?(gandalf)
Comment on attachment 8921997 [details] [diff] [review]
1383388-multilocale.patch (v2)

lgtm!
Attachment #8921997 - Flags: feedback?(gandalf) → feedback+
Comment on attachment 8921997 [details] [diff] [review]
1383388-multilocale.patch (v2)

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

This look reasonable.
Attachment #8921997 - Flags: review?(mozilla) → review+
Pushed by mozilla@jorgk.com:
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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
I had to land this right on top of a whole lot of bustage since bug 1362617 landed.
Target Milestone: --- → Thunderbird 58.0
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?
Flags: needinfo?(gandalf)
Try was successful, here the patch also for IB and SM.
Attachment #8923075 - Attachment is obsolete: true
Attachment #8923075 - Flags: feedback?(gandalf)
Flags: needinfo?(gandalf)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/395acea4a3e7
Follow-up: Adapt to landed M-C version. rs=bustage-fix CLOSED TREE
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 :)
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.
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/` :)
Attachment #8924679 - Flags: review?(mozilla) → review+
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/comm-central/rev/87226f1198cc
Followup to support building in m-c topdir as well; r=me
You need to log in before you can comment on or make changes to this bug.