420391 - Make --enable-static the default

Status: RESOLVED WONTFIX

(Thunderbird :: Build Config, defect)

Description

[Fabien Tassin]

Attachment: unix installer patch

make install doesn't produce anything on linux. Here is a patch.

Comment 1

[Magnus Melin [:mkmelin]]

Firefox has one, so I don't see why not.
Haven't tried it (yet), but don't we also need the other files that firefox has in the corresponding dir http://lxr.mozilla.org/seamonkey/source/browser/installer/unix/ ?

Comment 2

[Phil Ringnalda (:philor)]

Sometimes, the forced "steps to reproduce, expected results, actual results" structure of the simple bug filing page really really helps.

Your summary says "installer" (a single distributable executable which will install the application), comment 0 says "make install" (which copies the built application to your system): which did you actually mean?

Judging by your patch, your steps to reproduce are "1. Build with --enable-shared. 2. Try to make install (or make installer, one or the other)." Why? --enable-shared is a way to make a build with XPCOM components as separate shared libs rather than linking them into the executable, which runs slower than --enable-static but lets you quickly rebuild just parts of the app while you are making changes in just one part of the tree. There's no point in either install or installer on a slow build just so you can put it somewhere where you won't get the only benefit it offers.

(And Firefox *doesn't* have a Linux installer, it has some leftover files that they've been too lazy to remove - that's the old xpinstall installer stuff for which the backend is long gone.)

Comment 3

[Fabien Tassin]

I said installer because that's where the missing files are located (unix/installer).
Yet, it's not an installer per se, but more the make install rules file for unix. (I'll update the title)

Obviously, I've tested my patch and now, 'make install' produces the expected tree in $(DESTDIR)/usr/lib/thunderbird-3.0a1pre (and friends).
The patch installs all that's in dist/bin (at least it was true when I've submitted the patch), what I'm not totally sure is that it doesn't install more than it should, i.e. some files that are not really needed for thunderbird. yet it doesn't hurt.

Comment 4

[Phil Ringnalda (:philor)]

Okay, I'll try just one question. make install currently works with --enable-static, and your patch indicates that you want it to work with --enable-shared. Why?

Comment 5

[Fabien Tassin]

I don't build with --enable-static at all, and shared is the default.
I just want 'make install' to act like for firefox, xulrunner, seamonkey, etc.

If my approach is wrong, please tell me how I can fix this differently.

Comment 6

[Phil Ringnalda (:philor)]

You can fix it by building with --enable-static :)

You used to have to do that with Fx, and probably xulrunner (though I don't remember), before libxul came along and bsmedberg decided to to switch the default from the previous assumption that most people building were developers who wanted --enable-shared to the current assumption that most people would just live with a static --enable-libxul.

I think SeaMonkey is allowing you to package/install shared builds, but at the expense of filling their tinderbox logs with so many expected packaging errors <http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1205334960.1205340068.30260.gz> that it's unlikely anyone would notice something significant among them.

Tb isn't willing to do that, so to make your patch acceptable, you would need to (at the very least, I haven't tested things like whether the Windows installer has other expectations of a static build) change the Windows packages-static file into a packages file, and add a unix/packages file, with #ifdefs in both to only try to package the appropriate files depending on whether it's a static or shared build, and you would then need to be persuasive about how it was going to be maintained - core hackers have traditionally been terrible about remembering to make packaging changes, but Firefox running tests on packaged static/libxul builds catches them, and tests that they have, but because Thunderbird is not going to ship or test (slower) shared builds, even once we have testing infrastructure your packaged or make installed shared builds will be totally untested, and probably broken as often as not.

Doesn't just adding |ac_add_options --enable-static| to your mozconfig when you want to make install seem like an easier, faster, safer way out?

Comment 7

[Mark Banner (:standard8)]

(In reply to comment #6)
> I think SeaMonkey is allowing you to package/install shared builds, but at the
> expense of filling their tinderbox logs with so many expected packaging errors
> <http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1205334960.1205340068.30260.gz>
> that it's unlikely anyone would notice something significant among them.

Actually, SeaMonkey does actually do shared builds for release. The errors that you've shown come under one of the following banners:

* Files that used to be included, no longer reqd and shouldn't be in the packages file
* Files that will soon be included but haven't been yet.
* Misc errors that for various reasons aren't easy to tidy up.


Having a packages-static file on Linux is definitely something that I think would be useful. If I remember correctly on SeaMonkey we really needed this when we started doing Unit tests - it ensures we get the right set of files for our builds.

Additionally we shouldn't have ifdefs in the packages file, the packages-static files should be set up as per the standard release build procedures.

Having a separate packages file for shared libs might be possible as Phil mentioned, but definitely not desirable, as I could quite easily see it getting out of date quickly, its also an unnecessary overhead for us to maintain.

I seem to remember that on SeaMonkey we had a similar bug to include items for static builds (as SeaMonkey only releases with shared) and we decided to WONTFIX it because it would imply that the SeaMonkey project supports SeaMonkey being shipped in a configuration other than to what the standard release builds are (and the other configuration is never tested).

Comment 8

[Phil Ringnalda (:philor)]

Hmm, I'd buy the xptlink argument from bug 395535 (though I'm not sure exactly how much speed it buys, or when), but the "only ship what we want" argument needs some morphing, since NO_PKG_FILES does that just fine without being nearly as high-maintenance as packages-static - probably "use a less obscure method for shipping only what we want" would work better. The "needed for tests" argument I can't evaluate, since I don't know what it was or where (the only thing I'm familiar with for tests and packaging is that sometimes, if you're lucky, a test will show you that you forgot to add something to packages-static).

But while I'd buy that in another bug, it seems like a stretch to morph this bug into "use a packages-static rather than NO_PKG_FILES to get what we want in packaged static builds" given where it started. I'd think "make --enable-static the default, so that like every other Moz product the default for static/shared/libxul is the same as what's shipped" (which it seems would solve Fabien's original problem, since based on comment 5 it was actually "I want to just build without specifying how, and then make install" rather than "I want to build --enable-shared and then make install") would be a more reasonable morph for this bug.

Comment 9

[Mark Banner (:standard8)]

(In reply to comment #8)
> But while I'd buy that in another bug, it seems like a stretch to morph this
> bug into "use a packages-static rather than NO_PKG_FILES to get what we want in
> packaged static builds" given where it started. I'd think "make --enable-static
> the default, so that like every other Moz product the default for
> static/shared/libxul is the same as what's shipped" (which it seems would solve
> Fabien's original problem, since based on comment 5 it was actually "I want to
> just build without specifying how, and then make install" rather than "I want
> to build --enable-shared and then make install") would be a more reasonable
> morph for this bug.

Ok, that's fine by me, I probably won't raise a bug for the packages-static bit yet (I'll probably do that once we've got a Ts test set up in some form).

Comment 10

[Alexander Sack]

In the end we want to build with libxul(-sdk); is this actually on the radar for tbird 3 cycle? If so, wouldn't this patch be a step in the right direction?

To address your question of how the packages file could be properly maintained:

We (ubuntu) started to track tbird trunk and we have tools to check if there are files missing in make install that are available in dist/ .... we would be more than happy to to contribute patches/bugs in case we see that something goes out of sync here.

Is that good enough to make you feel comfortable about this?

Comment 11

[Phil Ringnalda (:philor)]

I presume by "this patch" you mean a static packages-static patch that doesn't yet exist, not this packages-shared patch. In that case, no, it's a step in an orthogonal direction. To make sure I wasn't missing something, I asked bsmedberg, and he said that there's no requirement to use a MOZ_PKG_MANIFEST to run as an XULRunner app.

Using one does exactly two things: it lets you create things in dist that you don't want to ship without having to bother adding them to MOZ_PKG_REMOVALS or NO_PKG_FILES (at the cost of having to add things you do want to ship to your MOZ_PKG_MANIFEST), and it lets you use xptlink to combine all your separate .xpt files into a single mail.xpt file (more because nobody ever bothered writing code to pass a directory listing of xpts to xptlink than because of anything inherent in a MOZ_PKG_MANIFEST, I think).

Because nobody bothers adding things like xpts we aren't really using to NO_PKG_FILES, I'm sort of mildly (if rather malevolently) in favor of taking a unix/packages-static, just because I think developers (especially extension developers) are slightly more likely to run across a packaged Linux build than a Windows build, and so they'll notice that we're failing to package something they want to use, but the actual primary benefit we're likely to see is a Ts improvement from reading in a single xpt at startup, rather than 140-odd separate tiny files.

Given a working Ts that shows a measurable improvement from linking xpts, I'd be happy to take a patch for a packages-static. Given an example of a wanted file that we currently fail to package, that we can only or more conveniently package with a packages-static, or an unwanted file that we currently package, that we can only or more conveniently not package with a packages-static, I'd be happy to take a patch. What I don't have any interest in doing is taking it for things it will not do: it can only make us pass tests if we're currently removing a needed file (in which case we could just as easily stop removing it) or including an unneeded file that gets in the way (in which case we could probably just as easily remove it, but we'd have to know what it was to know that), and it will not take us either closer to or farther from being able to build libxul. And given that our Linux startup time doesn't seem intolerable, I don't see any real point in taking it before we have an actual test to tell us how much good it does.

(And I've changed my mind: this bug already *has* morphed from Fabien's "I just want make install to work without having to specify anything at all" into "find some reason to add a unix/packages-static" so we might as well leave it as that, and use another bug to change the default from shared to static, if we're willing to do so.)

Comment 12

[Phil Ringnalda (:philor)]

Attachment: Fix v.1

package-compare output to follow

Comment 13

[Phil Ringnalda (:philor)]

Attachment: make package-compare v. 1

Looks to be the same things we're not packaging on Windows, plus nsinstall (which I don't *think* we want to be packaging, based mostly on Fx not packaging it).

Comment 14

[Phil Ringnalda (:philor)]

Attachment: Fix v.2

Poor little nsLDAPPrefsService.js, never even got the chance to be statically packaged.

Comment 15

[Mark Banner (:standard8)]

Comment on attachment 338429 [details] [diff] [review]
Fix v.2

Unless I'm very much mistaken, I think we need to add the now-removed xpt files to removed-files.in (maybe in a linux ifdef), so that the updater knows to remove those files when we commit this and everyone does an update. Otherwise I think thunderbird may get confused.

One minor comment, I think it would be nice if at least the lists within the subdirectories, were in alphabetical order. I'm not too fussed about this if you want to keep it the same as the windows file, but it would make deciding where to add things a little easier ;-)

Comment 16

[Phil Ringnalda (:philor)]

Standard8: we already have an #ifdef XP_WIN section in removed-files.in which removes the individual .xpts that were around at the time of 1.0->1.5. Any feeling about whether we should turn that into a (XP_WIN or (XP_UNIX not XP_MACOSX)) with current stuff added, or just drop it since it'll only come into play for a 1.0 -> 3.0 pave-over install on Windows?

Comment 17

[Phil Ringnalda (:philor)]

Stupid midair-antiprotection, what made you think I wanted to remove that dependency?

Comment 18

[Phil Ringnalda (:philor)]

Attachment: Fix v.3

Freshened up, with the removed-files bits. Lightly dependent on bug 453110, so it won't be arguing with NO_PKG_FILES over a few things.

Comment 19

[Mark Banner (:standard8)]

Comment on attachment 352970 [details] [diff] [review]
Fix v.3

I think we discussed this on irc and decided to wait on the outcome of the Fennec bug for packaging xpis as a potentially better option.

Comment 20

[Phil Ringnalda (:philor)]

But really, the only reasonable morph is to what Fabien actually wanted to file, "I want to have make install work without having to add --enable-static to my .mozconfig" since in the meantime we've added a package manifest for Linux and Mac, and a secret way to package a shared build, and he'll still get the same error message.

Comment 21

[Mark Banner (:standard8)]

We've now moved to a libxul style Thunderbird build on trunk. The defaults for trunk are to build libxul. Static is no longer supported, and shared builds will probably go away fairly soon as well.

Therefore marking as wontfix.