496683 - Organize removed-files.in, make it not suck

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Phil Ringnalda (:philor)]

Between Tb0.6 and Tb0.7, in the spring of 2004, we switched from shipping shared builds to static builds, and started linking xpts. That meant that the Windows installer needed to remove the dlls that only existed in shared builds, and the separate xpts. Those removals then became the start of our removed-files.in: a chunk of dlls (not @DLL_SUFFIX@s, since we never had a Linux installer, though they aren't ifdeffed, so on Linux and Mac the updater tries to remove gkgfxwin.dll) and a chunk of ifdef XP_WIN xpts.

Since then, we've inserted the myspell dlls (oopsie, those should have been @DLL_SUFFIX@, but the string of .dll must have thrown someone off) in the dlls, and at least three instances of xpts (including one I did, despite knowing better) that should have been removed on all platforms, or more to the point all platforms that don't link xpts so the separate ones might have existed, but which were inserted in the XP_WIN section where they do nothing unless someone's installing over the top of a 0.6.

Now, bug 495299 wants us to go back to "shipping" shared builds, too, which means adding a chunk with the ~50 dlls that exist in a shared build but not a static build, in an ifdef MOZ_STATIC_BUILD, and removing the ones of those that existed in the old days from the no-ifdef chunk, adding even more opportunity for people to put their pan-tree generic removal in the wrong place.

So, we need a couple of things: an organization, complete with comments explaining what separate particular chunks are, that will guide people to where and how they should be adding generic removals, and to make it easier to find the right spot and harder to find the wrong spot by getting rid of things that we don't really care to support.

We're stuck with a (trunk only, so far, since it looks like bug 469873 is going to have trouble landing on 1.9.1) chunk of xpt removals for Linux, but we only need to remove xpts on Windows if we continue to support installing 3.0 over the top of 0.6; we're stuck with going from a 3.0 shared build to a 3.0 static build, but we can at least drop a few things that we only need to remove if someone notices that they installed Tb in May 2004 and never updated it, and decides to drop a Tb3 on top of it.

My proposed plan is to remove the ancient stuff, move all the generic removals to the top, with top and bottom comments saying "*** hey, add your stuff here ***", then use the two huge talkback chunks as a buffer before the Linux xpts and shared libs and another bottom comment saying "*** hey, add your stuff up at the top, not down here ***"

Testing for Windows is easy, since you can just install over the top, and I've already discovered that we're missing some things from 1.0 and 1.5, but Linux/Mac is tougher, since you have to use the updater, and I've never run across developer-oriented instructions on how to generate the equivalent of what a major update would do.

Comment 1

[Phil Ringnalda (:philor)]

Attachment: WIP: blame-b-gone

I still think this is worth doing, considering the number of errors I spotted while moving things (and the ones left to do: I'm pretty sure we don't want to wind up leaving the 5.0 LDAP files laying around in Linux and Mac), but it's sure going to be hard on blame.

Comment 2

[Phil Ringnalda (:philor)]

Since I'll inevitably forget, and might wind up here again, testing on Linux is:

1. add |ac_add_options --enable-update-packaging| to your mozconfig
2. make -f client.mk build && cd ../tb && make package
3. make -C mozilla/tools/update-packaging/ complete-patch
4. untar the build to wherever/tb3/
5. untar old builds to wherever/tbn/
6. cp mozilla/dist/update/thunderbird-3.0b3pre.en-US.linux-i686.complete.mar wherever/update/update.mar (the rename to update.mar being important)
7. in wherever/tbn/, ./updater ../update 0
8. in wherever, diff -rq tbn tb3

Comment 3

[Phil Ringnalda (:philor)]

Attachment: Fix

A few additions, along with the reorganization, to make it easier to test that I wasn't breaking removing anything. This gets us clean Linux updates from 1.5 and 2.0 (I thought I remembered 1.0 having an updater, but it doesn't seem to have), clean pave-overs on Windows from 1.0, 1.5, and 2.0, and pretty clean updates on Mac from 1.5 and 2.0 (Mac has to be awkward about everything, so I'll chase the last bits separately).

Comment 4

[Phil Ringnalda (:philor)]

Oh, and the Mac updating bits I'll have forgotten by next time:

cp tbn.app/Contents/MacOS/updater.app/Contents/MacOS/updater update/updater
cd tbn.app
../update/updater ../update 0

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Can you summarize the actual end-user effects of your proposal?  In particular, if someone attempts install 3.0 directly on top of 0.6, what's the failure mode?  And presumably installing 3.0 on top of a 2.0 that was installed on top of 0.6 should Just Work, right?

Comment 6

[Phil Ringnalda (:philor)]

(In reply to comment #5)
> if someone attempts install 3.0 directly on top of 0.6, what's the failure
> mode?

Just what I had hoped it would be: total failure, it doesn't even start (I was a little worried that it would be something annoying like "it crashes intermittently, but only if you foo with bar set up").

>  And presumably installing 3.0 on top of a 2.0 that was installed on top
> of 0.6 should Just Work, right?

Yep. I'll try exactly that, but in theory if anyone checked the results of the installer or the updater even once between 0.6 and 2.0 then for the things that matter for 0.6, dlls and xpts, the installer should turn 0.6->2.0 into exactly a 2.0 (plus some harmless empty directories if they did 0.6->1.5 and then the updater from 1.5 to 2.0), so the 3.0 updater or installer wouldn't be able to tell the difference.

Comment 7

[Phil Ringnalda (:philor)]

The only interesting things left behind on a 0.6->2.0 (and thus a 0.6->2.0->3.0) are gkgfx.dll and mozz.dll, both of which were in shared builds and not in static builds at the 0.8 time when the "remove shared stuff" went in, so I don't know why they've been left hanging around all this time (other than the obvious answer, that nobody ever actually did a 0.6->0.8 pave-over and then diffed with a fresh 0.8).

On the bright side, they both are _current_ shared dlls, too, so the ifdef MOZ_STATIC_BUILD section from bug 495299 that started me down the path to packaging hell will finally clean them up.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #6)
> (In reply to comment #5)
> > if someone attempts install 3.0 directly on top of 0.6, what's the failure
> > mode?
> 
> Just what I had hoped it would be: total failure, it doesn't even start (I was
> a little worried that it would be something annoying like "it crashes
> intermittently, but only if you foo with bar set up").

Would it be easy to make the installer simply pop up a message exhorting the user to uninstall the old thing instead of installing and failing?

Comment 9

[Phil Ringnalda (:philor)]

Dunno, do you know anything about NSIS installers?

Comment 10

[Phil Ringnalda (:philor)]

Since Standard8 was curious about how much testing I was doing, it goes like this:

Download copies of the releases you need to test. For Linux and Mac, 1.5.0.14, 2.0.0.21, 3.0a1, 3.0a2, 3.0a3, 3.0b1, 3.0b2; for Windows, zips for 0.1, 0.2, 0.3, 0.4, and 0.5, and installers for 0.6, 0.7.3, 0.8, 0.9, 1.0.8, 1.5.0.14, 2.0.0.21, 3.0a1, 3.0a2, 3.0a3, 3.0b1, 3.0b2. You'll make your life easier if you then untar/install them into a directory of cleaninstalls/, and then copy those for testing, so you don't have to see the installer so many times.

From a single changeset (per OS, doesn't matter if it's the same cross-OS), build without a patch, that's your control. Install or untar it.

From the same changeset, build with the patch, and on Linux and Mac, build a complete mar.

On Linux and Mac, apply the mar (seven times), then diff -rq the result against your control. Remember that the updater doesn't remove empty directories, so you have to check them to be sure that's what you're seeing.

On Windows, install over the top (seventeen times), plus at strategic times check the impact of having installed the intermediate releases (what's that, 17! times?), and diff -rq the result against your control. Empty directories should be removed, but then you probably mostly need to add them to have them removed.

That tells you what you aren't removed-filesing - in a step when you need to be sure that those things aren't regressions, just the longstanding missed things, you also need to build a complete mar from the control build on Linux and Mac and apply it, or install the control Windows build over the top, and then diff the diffs from that with the diffs from the updates from the patched build.

Comment 11

[Phil Ringnalda (:philor)]

Attachment: Baby step 1 - reorg

The first step: nothing added or removed (except comments, and some ifdefs from turning things like "remove the Mac-specific Talkback files from 1.5, oh, and one updater image" into two blocks), this is purely to give me a workable base to work from.

As you'll see from the forthcoming verification logs, making sure that a removed-files patch has absolutely no effect over a trillion OS/version combinations is a thrill a minute.

Comment 12

[Phil Ringnalda (:philor)]

Attachment: reorg verification logs

Files differing isn't interesting (in removed-files.in's view), and you can't (or can't easily, anyway) build without changing buildid and thus application.ini and platform.ini, and we like touching our jars so much we do it no matter what, so quite a few files change between two builds that only have a different removed-files.in, so I tend to grep for the "only in" lines that are the sign that there's actually a difference in files. Which there isn't, here, which is the point.

Comment 13

[Phil Ringnalda (:philor)]

Attachment: Baby step 1 - reorg [checked in]

Good thing I don't make my living by knowing the alphabet. Oh, wait...

Comment 14

[Phil Ringnalda (:philor)]

Attachment: Baby step 2 - clean up after 3.0 alphas and betas [checked in]

This gets us clean pave-overs on Windows, and clean updates other than the empty directories that the updater leaves behind on Linux and Mac. The one thing I'm leaving behind is the Thunderbird.app/Contents/Resources/mail-biff-badge.png that was in 3.0a1 - far as I know, nobody has used a ../ path in removed-files.in, and even if it does happen to work, I don't want to guinea pig for a harmless image in an alpha.

Comment 15

[Phil Ringnalda (:philor)]

Attachment: 3.0 verification logs

Lightly edited to remove the fact that I'm too lazy to change channel-prefs just to silence that diff, and the differing install/uninstall logs on Windows, though I left in the yelping about the leftover redist files on Windows from official builds using VC80.CRT.manifest and me using VC90.CRT.manifest (when I bother), just in case someone has a good idea about packaging and unpackaging that, other than the existing "if you don't use the One True Version, it's up to you."

Comment 16

[Phil Ringnalda (:philor)]

Attachment: Baby step 3 - clean up after 2.0 [checked in]

I do believe someone's been down this path before: pave-over from 2.0/Win was already clean. Linux and Mac have the usual scattering of dead xpts and random bits we never really wanted to ship and US things and things where we were only removing the Windows variation of the filename, and the mail-biff-badge.png I should have remembered _disappeared_ after a1, not _only appeared in_. But even though ../Resources/ does actually work, I'm still not convinced I want to be doing it.

Comment 17

[Phil Ringnalda (:philor)]

Attachment: 2.0 verification logs

Comment 18

[Phil Ringnalda (:philor)]

Attachment: Baby step 4 - clean up after 1.5 [checked in]

And so we bid a fond farewell to Mac and Linux - if there was a way to make the 1.0 updater update to a different version, or trigger it manually, it's dropped out of our collective memory.

Comment 19

[Phil Ringnalda (:philor)]

Attachment: 1.5 verification logs

Comment 20

[Phil Ringnalda (:philor)]

Attachment: Toddler step 5 - clean up after 1.0, 0.9 and 0.8 [checked in]

Not much changed in terms of leftovers through these three, only 0.8 adding in the en-win.jar, so I collapsed them.

Comment 21

[Phil Ringnalda (:philor)]

Attachment: 1.0-0.9-0.8 verification logs

Comment 22

[Phil Ringnalda (:philor)]

Attachment: Baby step 6 - clean up after 0.7 [checked in]

Since 0.7 was the start of linking xpts, from here on back they're just failures of the "when we started linking on Windows" block (plus this seemed like an opportune time to remove that dom_smil.xpt, which we don't really need to remove on Windows where there are only separate xpts if you've got a build from years before dom_smil existed).

Comment 23

[Phil Ringnalda (:philor)]

Attachment: 0.7 verification log

Comment 24

[Phil Ringnalda (:philor)]

Attachment: Baby step 7 - clean up after 0.6 [checked in]

And 0.6 was the last of the shared builds, so it starts the additions to the list of shared dlls that should have been removed starting with 0.7.

Comment 25

[Phil Ringnalda (:philor)]

Attachment: 0.6 verification log

Comment 26

[Phil Ringnalda (:philor)]

Attachment: Big boy step 8 - clean up after 0.5, 0.4, 0.3, 0.2 and 0.1 [checked in]

Another collapsed chunk, since 0.5-0.2 had exactly the same things, and 0.1 only added a couple of xpts and a text file telling profile sharing clients which pref branches to not share.

Comment 27

[Phil Ringnalda (:philor)]

Attachment: 0.5-0.1 verification log

Comment 28

[Mark Banner (:standard8)]

Comment on attachment 382906 [details] [diff] [review]
Baby step 1 - reorg [checked in]

Calendar's removed-files.in has blank lines as well, so this should be fine. r=Standard8

Comment 29

[Phil Ringnalda (:philor)]

Comment on attachment 382906 [details] [diff] [review]
Baby step 1 - reorg [checked in]

http://hg.mozilla.org/comm-central/rev/030a8d89219d

Comment 30

[Phil Ringnalda (:philor)]

Comment on attachment 382921 [details] [diff] [review]
Baby step 2 - clean up after 3.0 alphas and betas [checked in]

http://hg.mozilla.org/comm-central/rev/447abd9e3c3f

Comment 31

[Phil Ringnalda (:philor)]

Comment on attachment 383103 [details] [diff] [review]
Baby step 3 - clean up after 2.0 [checked in]

http://hg.mozilla.org/comm-central/rev/1ef4d3df6dd3

Comment 32

[Phil Ringnalda (:philor)]

Attachment: Wild leap 9 - package shared builds

While this works just fine, allowing packaging shared builds and making updates or pave-overs from static to shared or shared to static work perfectly, it's a little too much and I don't have the slightest idea how to fix it. The targets for package and install and installer aren't actually ours, they live in toolkit/mozapps/installer/packager.mk, and I don't know how to allow package to skip a BUILD_STATIC_LIBS check while making install and installer still error out.

Comment 33

[Mark Banner (:standard8)]

Comment on attachment 383110 [details] [diff] [review]
Baby step 4 - clean up after 1.5 [checked in]

>@@ -75,23 +77,27 @@ components/websrvcs.xpt
> components/widget_mac.xpt
> #endif
> components/@DLL_PREFIX@xpcom_compat_c@DLL_SUFFIX@
> components/xpcom_obsolete.xpt
> components/xpti.dat
> components/xptitemp.dat
> components/myspell/en-US.dic
> components/myspell/en-US.aff
>+components/myspell/

So from what I can tell, the updater can cope with directories but isn't actually able to remove them (maybe just linux/mac issue?). So are we listing them in case of a future fix? If so, then you also need to include:

defaults/messenger/
defaults/wallet/
init.d/
extensions/[e-mail]/

(assuming they aren't already listed).

>+.autoreg

So everyone else creates and packages this. However iirc Benjamin said that this isn't really needed anymore, just no one has got rid of it from FF. Though I guess I could be wrong.

Comment 34

[Phil Ringnalda (:philor)]

Nope, my intention wasn't to add directories in hopes of the updater dealing (beyond the way it deals, which is to output a string of "ignoring instruction to remove directory" at the end of mar generation), it was only to add them when the installer (which does deal) was leaving an empty behind.

So I didn't add init.d/ because it never existed on the only platform where we have an installer. Talkback is already removed (first, rather than the usual last, because of the way it splits into platform-dependent stuff at the end), defaults/wallet/ appears to me to be cleaning up after a failure of NO_PKG_FILES to prevent shipping the schemas on Linux/Mac (I strongly doubt we ever intentionally packaged those on Windows), and defaults/messenger/ isn't empty - it still has a mailViews.dat in it, it just had an awful lot of other mailViews.dats in it in various and sundry places over the years.

Comment 35

[Phil Ringnalda (:philor)]

Oh, dunno how I confused myself into thinking we had the wallet schemas in NO_PKG_FILES - we just apparently didn't ever package them on Windows, and always did ship them on Linux and Mac.

Comment 36

[Phil Ringnalda (:philor)]

Comment on attachment 383110 [details] [diff] [review]
Baby step 4 - clean up after 1.5 [checked in]

http://hg.mozilla.org/comm-central/rev/64e4dfc7992c

Comment 37

[Phil Ringnalda (:philor)]

Comment on attachment 383122 [details] [diff] [review]
Toddler step 5 - clean up after 1.0, 0.9 and 0.8 [checked in]

http://hg.mozilla.org/comm-central/rev/427431b18362

Comment 38

[Phil Ringnalda (:philor)]

Comment on attachment 383124 [details] [diff] [review]
Baby step 6 - clean up after 0.7 [checked in]

http://hg.mozilla.org/comm-central/rev/25a32bafab57

Comment 39

[Phil Ringnalda (:philor)]

Comment on attachment 383132 [details] [diff] [review]
Baby step 7 - clean up after 0.6 [checked in]

http://hg.mozilla.org/comm-central/rev/80808bd8af10

Comment 40

[Phil Ringnalda (:philor)]

Comment on attachment 383134 [details] [diff] [review]
Big boy step 8 - clean up after 0.5, 0.4, 0.3, 0.2 and 0.1 [checked in]

http://hg.mozilla.org/comm-central/rev/1ba2d6e21cbf

Comment 41

[Mark Banner (:standard8)]

(In reply to comment #32)
> Created an attachment (id=383405) [details]
> Wild leap 9 - package shared builds
> 
> While this works just fine, allowing packaging shared builds and making updates
> or pave-overs from static to shared or shared to static work perfectly, it's a
> little too much and I don't have the slightest idea how to fix it.

What do you mean by a little too much?

> The targets
> for package and install and installer aren't actually ours, they live in
> toolkit/mozapps/installer/packager.mk, and I don't know how to allow package to
> skip a BUILD_STATIC_LIBS check while making install and installer still error
> out.

I'm really not sure what you're talking about here.

Comment 42

[Phil Ringnalda (:philor)]

We want to allow |make package| with a shared build, but disallow |make install| and |make installer| except for --enable-static, because shared is the default and so there's no other place to tell people doing |configure && make && make install| that they are getting an unnecessarily slow build because our defaults are more interested in making life easy for developers than for user-builders.

But because |make package| just runs the package target in mail/build.mk, which just does |make -C mail/installer|, package runs the default target in mail/installer/Makefile, which is (roughly speaking) the first target which does anything. The first target which does anything is the libs:: target in toolkit/mozapps/installer/packager.mk, which is included in mail/installer/Makefile.in, and which depends on the make-package target (also in packager.mk) which depends on stage-package (also in packager.mk) where things finally happen.

In theory, we probably should be able to work around that, though it will wind up sort of fragile since someone hacking on packager.mk isn't going to expect that we've done something freaky like having a target in our Makefile.in that depends on libs:: and then does something pointless just to make make treat it as the default target, but in fact since I don't know much about make and nobody has thus far volunteered to help me be crazy (actually, I think Ted said something more like "that way madness lies"), I haven't managed to make it work yet.

Comment 43

[Mark Banner (:standard8)]

Well it sounds like we should change our defaults, though that's probably a discussion for a different bug.

Whilst not ideal, how about we allow an override by environment variable? Hence our buildbots could specify it to allow the packaging to take place. Although other people building Thunderbird could also specify it, we could include in the documentation a note as to a) why we don't recommend it or support it and b) if you publish details of this option, please also publish our note as well.

It is a shame we haven't made it to libxul yet, it would be so much easier.

Comment 44

[Phil Ringnalda (:philor)]

Attachment: Wild leap 9 - package shared builds

Yeah, that seems reasonable.

Comment 45

[Mark Banner (:standard8)]

Comment on attachment 386511 [details] [diff] [review]
Wild leap 9 - package shared builds

> NO_PKG_FILES = \
> 	thunderbird-bin.elf \
> 	thunderbird-config \
> 	regxpcom* \
>-	xpcshell* \
> 	xpidl* \
> 	xpt_dump* \
> 	xpt_link* \
> 	plugins \
> 	res/bloatcycle.html \
> 	$(NULL)

Actually we want to keep this here. xpcshell gets packaged under package-tests on Firefox, not into package.

>diff --git a/mail/installer/windows/packages-static b/mail/installer/windows/packages-static
>--- a/mail/installer/windows/packages-static
>+++ b/mail/installer/windows/packages-static

>+bin\xpcshell.exe

Hence we don't need this line either.

r=Standard8 with that fixed. Thanks for all the tidy up.

Comment 46

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/27b3a0d2aa23 and I think we must be done here.