675501 - Build places unconditionally

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Comment 1

[Matheus Kerschbaum]

Attachment: patch

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 549657 [details] [diff] [review]
patch

I don't think we want to do this ... I think there are non-Firefox Gecko consumers that don't use places.  Mak?

Comment 3

[Marco Bonardo [:mak]]

I'm not sure either, most consumers I know use it, but my knowledge on that is incomplete, since I don't know all the projects around.  I'd suggest opening a thread in platform to get feedback before doing something like this.

Comment 4

[Matheus Kerschbaum]

Fennec, Xulrunner, Seamonkey, Thunderbird, Calendar and Camino use places. Is there any other major Gecko consumer out there?

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Ok, I didn't think Thunderbird did.

Comment 6

[Justin Wood (:Callek)]

Songbird I don't think uses places, but is based on XULRunner iirc. Same with Komodo, CC-ing Mook since I bet he would know on both counts.

Comment 7

[Matheus Kerschbaum]

Actually Thunderbird doesn't seem to use Places either (http://mxr.mozilla.org/comm-central/source/mail/app/profile/all-thunderbird.js#627). I still think the build-time flag is useless though, since consumers can disable places with a simple pref.

Comment 8

[Justin Wood (:Callek)]

(In reply to comment #7)
> Actually Thunderbird doesn't seem to use Places either
> (http://mxr.mozilla.org/comm-central/source/mail/app/profile/all-thunderbird.
> js#627). I still think the build-time flag is useless though, since
> consumers can disable places with a simple pref.

There are places interfaces that TB uses/needed last I checked, and why they actually build with PLACES.

Comment 9

[Marco Bonardo [:mak]]

bluegriffon, tom tom home use xulrunner as well

Comment 10

[Marco Bonardo [:mak]]

(In reply to comment #7)
> Actually Thunderbird doesn't seem to use Places either
> (http://mxr.mozilla.org/comm-central/source/mail/app/profile/all-thunderbird.
> js#627). I still think the build-time flag is useless though, since
> consumers can disable places with a simple pref.

I think the problem is not really with disabling it, but on libxul and xulrunner size, but I don't know if they care that much about those small improvements.

Comment 11

[Marco Bonardo [:mak]]

also even if you flip the pref the database is created regardless and with chunked growth (for fragmentation) its minimum size is 10MB, even if empty.

Comment 12

[Mark Banner (:standard8)]

Thunderbird does indeed build places but have it disabled. iirc there's some stuff we use even with it disabled, but in any case, we've been moving our build closer to FF so we're fine with removing the compile option.

Comment 13

[:Mook]

FWIW, as far as I can tell both Songbird and Komodo use the default here (i.e. build with Places).  http://timeline.songbirdnest.com/vendor/browser/trunk/xulrunner-1.9.2/mozconfigs/xulrunner.linux.release.i686.mozconfig for Songbird, and somewhere in http://grok.openkomodo.com/source/xref/openkomodo/trunk/mozilla/build.py#1483 for Komodo (sorry, that's such a mess...)

Instantbird also appears to use the default: http://lxr.instantbird.org/instantbird/source/tools/buildbot-configs/linux/mozconfig-release

It looks like BlueGriffon explicitly enables it for some reason: http://sources.disruptive-innovations.com/bluegriffon/trunk/config/mozconfig.linux

Comment 14

[Marco Bonardo [:mak]]

well, at this point I'd suggest to just announce the change in m.d.platform and if there are no complain for a couple weeks, proceed with it.

Comment 15

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 549657 [details] [diff] [review]
patch

r=me

I agree that we should announce this on .platform before landing.

Comment 16

[Marco Bonardo [:mak]]

fwiw, Mobile on Android wants to move to their own history implementation on top of the system history implementation, so they may want to actually undefine MOZ_PLACES.

I feel like this should go on the back burner till a decision is made.

Comment 17

[Matheus Kerschbaum]

This is a really small cleanup so I don't really mind WONTFIXing it :)