1130854 - Package Lightning with Thunderbird

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Philipp Kewisch [:Fallen][📱📆]]

This bug is meant to package Lightning in mail/installer/package-manifest.in.

Current issues include bug 910660, and the startup cache breaking because both ical.js and libical have a calIDateTime interface.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

Not only c-c and c-a, but also for betas and releases now. For nightly builds, I think we should package the files into Contents/Resources/extensions, while for betas and releases we will do Contents/Resources/distribution/extensions.

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v1

This should take care, given the patch for bug 910660 is applied.

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

One thing we might want to add is augmenting the version number of Lightning, at least on official beta/release builds to be something like 4.0packaged. This would let Thunderbird use the packaged Lightning version, but upgrade to the official version via AMO on the next update check.

Another thing left to do is make sure l10n repacks on beta/release are doing the right thing.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v2

Ok, updated patch. This should be fairly final, I'll handle repacks and such in a separate bug.

Comment 5

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8576269 [details] [diff] [review]
Fix - v2

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

For some reason, I've been having trouble importing this patch to check a local build (see bug 1144518).

This may have some impacts on buildbot steps--I'd like to see a green try push before this lands.

Arguably, you may also want to default lightning to being enabled when we build Thunderbird.

::: calendar/base/backend/install.rdf
@@ +1,4 @@
> +<?xml version="1.0"?>
> +<!--
> +  This is a dummy file to convince the packager that the components/
> +  subdirectory is not a separate base. Its needed because the components/

Nit: It's

::: mail/installer/package-manifest.in
@@ +854,5 @@
>  @RESPATH@/components/@DLL_PREFIX@mozgnome@DLL_SUFFIX@
>  #endif
> +
> +[calendar]
> +#ifdef NIGHTLY_BUILD

This section should be keyed off of Lightning being enabled in the Thunderbird build.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

Pushed to comm-central changeset 566579330863

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

argh sorry, that was just a try push.

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v3

Comments taken into account and some additions to make sure it will actually work at the end.

I haven't made --enable-calendar the default, first of all since I didn't find out how to do this on first sight, and because I don't want to put too much into this bug.

Comment 9

[Philipp Kewisch [:Fallen][📱📆]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=bf900d36093f

Comment 10

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8590952 [details] [diff] [review]
Fix - v3

Taking back review for now, still errors with unification *sigh*. We might just have to dump this in once there is a green try build and see how it goes for the beta.

Comment 11

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Part1: Basic Packaging

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Part2: Back to em:targetPlatform

We moved to using em:realTagetPlatform to have a platform value to unify on but make it platform independent in case ical.js is enabled. ssitter had some doubts in that bug and I think he was right, there is no need to do that right now. As a nice side effect, with my patch from bug 1153790 the target platform is automatically merged by the packager.

I haven't switched the Lightning postflight code to use the packager yet, I'd rather do this separately because I have something that works now and I'd rather not fiddle with it.

Comment 13

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Part3: Make mac unification work

This is that patch that actually makes unification work. With the packaging enabled we need to unify twice, once for xpi-stage/lightning and once for the packaged Lightning. Of course I could copy files over, but since the packager produces a different layout than in xpi-stage/lightning I didn't want to take any chances. I also needed to move the order in the mozconfig so that the Lightning unification is called before Thunderbird's.

Comment 14

[Philipp Kewisch [:Fallen][📱📆]]

Green try run, admittedly before splitting into three parts:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=da24c38fc3b5

Comment 15

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Part4: Don't package locales as jars

Part 4 is needed because the unifier doesn't like .jar files and glandium has valid arguments against unifying them in bug 1153790.

Comment 16

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8591859 [details] [diff] [review]
Part1: Basic Packaging

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

::: mail/locales/Makefile.in
@@ +228,5 @@
> +ifdef NIGHTLY_BUILD
> +MOZ_PKG_EXTRAL10N += extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}=$(DIST)/xpi-stage/lightning-$(AB_CD)
> +else
> +MOZ_PKG_EXTRAL10N += distribution/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}=$(DIST)/xpi-stage/lightning-$(AB_CD)
> +endif

I'm nervous about passing parameters into l10n.mk after that file is included into the Makefile. It will certainly break if anyone uses := anywhere to define the variable.

Comment 17

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8591864 [details] [diff] [review]
Part3: Make mac unification work

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

::: calendar/lightning/build/universal.mk
@@ +46,5 @@
> +endef
> +
> +define unify_lightning_repackage
> +cd $(DIST_UNI)/$1/$2 && $(ZIP) -qr ../$(XPI_PKGNAME).xpi *
> +endef

Why do you need to call this for dist/xpi-stage/lightning but not the Contents/Resources/ path? If you only need to do it once, there's no need to put this in a define.

Comment 18

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to Joshua Cranmer [:jcranmer] from comment #17)
> Why do you need to call this for dist/xpi-stage/lightning but not the
> Contents/Resources/ path? If you only need to do it once, there's no need to
> put this in a define.

The Lightning in Contents/Resources is packaged by the Thunderbird packager, while the other one needs to be packaged by us. I put it in a define to keep the actual target clean, I thought it would be nice to just call macros from the target and do the work separately.

Comment 19

[Philipp Kewisch [:Fallen][📱📆]]

(In reply to Joshua Cranmer [:jcranmer] from comment #16)
> > +MOZ_PKG_EXTRAL10N += distribution/extensions/{e2fda1a4-762b-4020-b5ad-a41df1933103}=$(DIST)/xpi-stage/lightning-$(AB_CD)
> I'm nervous about passing parameters into l10n.mk after that file is
> included into the Makefile. It will certainly break if anyone uses :=
> anywhere to define the variable.

Wouldn't it break if := is used even if l10n.mk is included afterwards? Anyway, I can move the include down to the bottom if you like.

Comment 20

[Philipp Kewisch [:Fallen][📱📆]]

remote:   https://hg.mozilla.org/comm-central/rev/1ff16a81a844
remote:   https://hg.mozilla.org/comm-central/rev/411e26a79fcf
remote:   https://hg.mozilla.org/comm-central/rev/01959590e94f
remote:   https://hg.mozilla.org/comm-central/rev/2b297bcafaa5

remote:   https://hg.mozilla.org/releases/comm-aurora/rev/c02b83de4bd2
remote:   https://hg.mozilla.org/releases/comm-aurora/rev/9d9eb9be8ddc
remote:   https://hg.mozilla.org/releases/comm-aurora/rev/e252be6476cc
remote:   https://hg.mozilla.org/releases/comm-aurora/rev/9dea3ce82148

remote:   https://hg.mozilla.org/releases/comm-beta/rev/9443963ae362
remote:   https://hg.mozilla.org/releases/comm-beta/rev/f29f3513be8b
remote:   https://hg.mozilla.org/releases/comm-beta/rev/fc7ec66919e6
remote:   https://hg.mozilla.org/releases/comm-beta/rev/b11a42c262d6

Comment 21

[Philipp Kewisch [:Fallen][📱📆]]

Fixed bustage caused by this:

https://hg.mozilla.org/comm-central/rev/f5372ecba286
https://hg.mozilla.org/releases/comm-aurora/rev/eed9922aeaf0
https://hg.mozilla.org/releases/comm-beta/rev/69557a73f991

Comment 22

[Stefan Sitter [:ssitter]]

I think this caused Bug 1160805.