Like bug 437643, this should probably live in a MailNaews Core Build System component, but filing in the SeaMonkey product for now. comm-central will have its own build system, or at least the base of one, and a number of path changes etc. are needed to deal with the fact that we heavily reference files that live in mozilla-central which is now in a subdirectory of this build system. All those additions and changes need proper review to land, which should be tracked in this bug. I currently have a ~500K patch here, with ~70 files changed, ~13500 inserted and ~130 deleted lines, which sounds scary, but the major part of this is adding (slightly modified) copies of Mozilla build system files like configure.in (7900 lines), config.mk (~900 lines) or rules.mk (~2200 lines). I'll split this into "nicer" chunks (build system itself, probably with a patch against the Mozilla equivalents to ease review; Thunderbird, i.e. mail/ + other-licenses/; MailNews, i.e. mailnews/ + directory/; and SeaMonkey, i.e. suite/) for the actual review. I guess for the build system stuff itself, we'll want reviews from all the participating projects as well as Ted from the Mozilla build system perspective. For the other parts, I guess we can apply the review rules typical for those source modules, even though I'd like a rough look from Ted on those as well if he has the time.

This patch, containing the biggest chunk of all changes, adds the build system to the repository (which before that should only contain an import of a snapshot of directory/xpcom/, mail/, mailnews/, other-licenses/branding/thunderbird/ and suite/ from CVS, with the CVS directories and .cvsignore files stripped). Most of this is copied from mozilla-central, with slight modifications applied. I'll attach a patch in a minute that contains diffs of the major files against the mozilla-central version, so my changes become obvious. No such diff will be provided for the following files that are mostly trivial or too different from mozilla-central: /.hgignore /Makefile.in /aclocal.m4 /allmakefiles.sh /client.py The major changes that are introduced are as following: - configure introduces a COMM_BUILD var, set to 1, so files that are called from both our and the Mozilla build system, like build.mk, makefiles.sh, confvars.sh or even mozconfig can easily detect which build system they are run in. - configure introduces a MOZILLA_SRCDIR variable set to $topsrcdir/mozilla to abstract places that need to point to the Mozilla source directory, e.g. because referencing toolkit or files from the other build system that we still reuse directly. - autoconf.mk introduces a MOZDEPTH var for the same purpose. We should probably make things cleaner over time by removing all places where I manually had to add mozilla/ to a path and placing such generic variables there, so we are theoretically not dependent on mozilla-central sitting in mozilla/ but can potentially use other directory names as well for it. - The Mozilla configure is being called as a subconfigure script from our configure. It needs the MOZCONFIG var set so it never tries to find it relative to it's own path, we already found the right mozconfig. It also needs MOZ_BUILD_APP="$MOZ_CURRENT_PROJECT" set in the case where we are doing Mac universal builds, which have a mozconfig that relies on MOZ_BUILD_APP to be set to "i386" or "ppc" when it's called from configure, and relies on configure then resetting MOZ_BUILD_APP to the actual value given by --enable-application. This is a dirty hack but how universal builds work right now. - config.mk adds a EXPAND_MOZLOCALE_SRCDIR macro that expands locale sourcedirs for directories in mozilla-central referenced by us. We needs this for including charset settings from toolkit in our locales Makefiles. - --enable-calendar is resurrected to enable build the calendar extension (which is Lightning nowadays) - some deletions of configure/autoconf.mk stuff we do not need - We can do more in that area later, but we need to care to not remove stuff that set variables we need somewhere. I'd rather not do all of that in the first step, but delay that into a followup and to the time where we have builders out there that go non-green when we break anything. - The rest of the changes should only be path adjustments or usage of the before mentioned variables. I hope all of those changes sound reasonable. Who should/can review this besides Ted?

This is the relative patch to the current state of the respective mozilla-central files, making my changes much clearer.

Here's the MailNews, patch, i.e. all needed changes in directory/ and mailnews/. directory/ doesn't actually need much, but I added a makefiles.sh for directory/xpcom, just so that we are nice and pregenerate the Makefiles for speed. The mailnews/ changes are mainly adaptations to using the new buildsystem vars (MOZILLA_SRCDIR/MOZDEPTH) to reference files from mozilla-central where we do so, and test adjustments because the unit test system still fully lies in mozilla/ so the test files need to reference file in our own source by going down one level, i.e. using ../mailnews/ paths. This is not as beautiful as we'd like, but functional and AFAIK the only way to do it without creating our own unit tests system in parallel to Mozilla's (which we probably don't want to do for now). It would be nicer if the xpcshell test functions could reference files relative to the original path where the running test_*.js came from.

Here are the Thunderbird changes. Those consist mainly of adding a app-config.mk that defines MOZ_THUNDERBIRD, so we can remove those app-specific things from configure in the future (it also defines the var needed to install Lightning right into the Thunderbird build), as well as usage of the new MOZILLA_SRCDIR/MOZDEPTH variables to reference mozilla-central files. build.mk gets some changes to correctly realize what to build inside the Mozilla build system (toolkit, extensions) and what in our build system (everything else). makefiles.sh should only return something for our build system. confvars.sh now reads MOZ_APP_VERSION itself and also sets THUNDERBIRD_VERSION, though we should IMHO phase out those brand_version vars completely.It also cares to not set MOZ_LDAP_XPCOM for the Mozilla build system, as mozilla-central break with that option due to not having directory/. We should fix mozilla-central to ignore this later on (there's a bunch of resulting cleanup I want to do there anyway). And the XUL files including macWindowMenu.inc need to care that toolkit is now below mozilla/.

And last not least, the SeaMonkey changes. Similar to Thunderbird, those consist mainly of adding a app-config.mk that defines MOZ_SUITE, so we can remove those app-specific things from configure in the future (it also defines the vars needed to build the XPFE download manager from mozilla-central and to install Lightning right into the SeaMonkey build), as well as usage of the new MOZILLA_SRCDIR/MOZDEPTH variables to reference mozilla-central files. build.mk gets some changes to correctly realize what to build inside the Mozilla build system (toolkit, extensions) and what in our build system (everything else). As I ran into the problem of a wrong path for the license file, I went to fix bug 366344 as well by importing our own MPL license.txt file into our code. makefiles.sh should only return something for our build system. confvars.sh now reads MOZ_APP_VERSION itself and also sets SEAMONKEY_VERSION, though we should IMHO phase out those brand_version vars completely. It also cares to not set MOZ_LDAP_XPCOM for the Mozilla build system, as mozilla-central break with that option due to not having directory/. We should fix mozilla-central to ignore this later on (there's a bunch of resulting cleanup I want to do there anyway).

Comment on attachment 329059 [details] [diff] [review] Thunderbird changes, v1 Ted, I'd like you to take a look at what build.mk does for our products, I hope that sounds sane enough.

Comment on attachment 329062 [details] [diff] [review] SeaMonkey changes, v1 Ted, I'd like you to take a look at what build.mk does for our products, I hope that sounds sane enough.

Comment on attachment 329056 [details] [diff] [review] creating the buildsystem, v1 >diff --git a/.hgignore b/.hgignore Looks good, maybe add http://hg.mozilla.org/index.cgi/mozilla-central/rev/98e43ce43c79 as well? >diff --git a/build/macosx/universal/flight.mk b/build Hard for me to comment on this, but based on diff of this file (in attachment 329057 [details] [diff] [review]) it looks good. >diff --git a/client.mk b/client.mk >+# To build a tree, >+# 1. hg clone ssh://hg.mozilla.org/comm-central comm Why ssh vs. http? (http should be best for most users) >+MOZCONFIG_MODULES := mozilla/build/unix/uniq.pl >+ >+ >+ >+run_for_side_effects := \ no idea why m-c has 3 newlines here, but perhaps trim it a bit? >+EXTRA_CONFIG_DEPS := \ >+ $(TOPSRCDIR)/mozilla/aclocal.m4 \ >+ $(wildcard $(TOPSRCDIR)/mozilla/build/autoconf/*.m4) \ >+ $(NULL) We need to account for comm-central's aclocal.m4 as well: $(TOPSRCDIR)/aclocal.m4 (aclocal.m4 gets run by configure if its in current working dir, so we need both here) >+CONFIG_STATUS_DEPS := \ >+ $(TOPSRCDIR)/configure \ >+ $(TOPSRCDIR)/.mozconfig.mk \ >+ $(TOPSRCDIR)/mozilla/configure \ >+ $(wildcard $(TOPSRCDIR)/directory/c-sdk/configure) \ >+ $(wildcard $(addsuffix confvars.sh,$(wildcard $(TOPSRCDIR)/*/))) \ >+ $(NULL) we need to keep the mozilla-central deps here as we don't chain against their client.mk and use our configure to call theirs: $(TOPSRCDIR)/mozilla/allmakefiles.sh $(wildcard $(TOPSRCDIR)/nsprpub/configure) $(wildcard $(TOPSRCDIR)/mozilla/config/milestone.txt) $(wildcard $(TOPSRCDIR)/config/chrome-versions.sh) I'm not sure about the confvars.sh for mozilla/ (ted?) And while we're here dont' forget our (new) allmakefiles.sh >+#################################### >+# Other targets >+ >+# Pass these target onto the real build system >+install export libs clean realclean distclean alldep maybe_clobber_profiledbuild:: $(OBJDIR)/Makefile $(OBJDIR)/config.status >+ $(MOZ_MAKE) $@ >+ s/ maybe_clobber_profilebuild// per no PGO support. >+.PHONY: checkout real_checkout depend build profiledbuild maybe_clobber_profiledbuild export libs alldep install clean realclean distclean cleansrcdir pull_all build_all clobber clobber_all pull_and_build_all everything configure preflight_all preflight postflight postflight_all we don't need the PGO phony targets either. >diff --git a/client.py b/client.py >+def do_cvs_checkout(modules, tag, cvsroot, cvs, checkoutdir): >+ for module in modules: >+ (parent, leaf) = os.path.split(module) >+ print "CVS checkout begin: " + datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S UTC") >+ print "CVS checkout end: " + datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M:%S UTC") Wouldn't this make more sense to print once, and outside the for loop? (similar to what client.mk currently logs) >diff --git a/Makefile.in b/Makefile.in >diff --git a/aclocal.m4 b/aclocal.m4 >diff --git a/allmakefiles.sh b/allmakefiles.sh All look good. (Further files all checked, with diff in attachment 329057 [details] [diff] [review]; if I have extra time I'll try to give them a more detailed overview) >diff --git a/configure.in b/configure.in -SQLITE_VERSION=3.5.4 +SQLITE_VERSION=3.5.9 Why the sqlite version number change (vs. mozilla-central)? -FIREFOX_VERSION=`cat $topsrcdir/browser/config/version.txt` +FIREFOX_VERSION=`cat $MOZILLA_SRCDIR/browser/config/version.txt` You delete this var from our autoconfig.mk, if this is wanted, add it back there; or remove this. -MOZ_ARG_ENABLE_BOOL(calendar,, - MOZ_OLD_CALENDAR=1, ... -if test "$MOZ_OLD_CALENDAR"; then - AC_MSG_WARN([Building with the calendar extension is no longer supported.]) ... +MOZ_ARG_ENABLE_BOOL(calendar, +[ --enable-calendar Enable building of the Lightning calendar extension], even though this would add branding to configure, perhaps --enable-lightning or --enable-calendar-extension as an option, and obsolete --enable-calendar itself? (That would be a decision for calendar team though, imo). *** I'm assuming that absence of an arg in our configure.in does not preclude it from being used in mozilla/configure.in (if thats wrong there are additional comments I'll need to make). >diff --git a/config/config.mk b/config/config.mk DEHYDRA_MODULES = \ - $(topsrcdir)/xpcom/analysis/final.js \ + $(MOZILLA_SRCDIR)/xpcom/analysis/stack.js \ $(NULL) TREEHYDRA_MODULES = \ - $(topsrcdir)/xpcom/analysis/outparams.js \ - $(topsrcdir)/xpcom/analysis/stack.js \ + $(MOZILLA_SRCDIR)/xpcom/analysis/outparams.js \ $(NULL) Huh? (re: .js script changes) >diff --git a/config/rules.mk b/config/rules.mk checkout: - $(MAKE) -C $(topsrcdir) -f client.mk checkout + $(PYTHON) $(topsrcdir)/client.py checkout This target probably is not even supported [for mozilla-central], but "why not" keep it working, sure ;-) ...rest in this seems good to me. (r+ with those nits addressed; if ted's comments overrule any of my own, go with his.)

Comment on attachment 329062 [details] [diff] [review] SeaMonkey changes, v1 My review should not count the licence.txt bits, I'm no legal buff.

(In reply to comment #8) > (From update of attachment 329056 [details] [diff] [review]) > >diff --git a/.hgignore b/.hgignore > > Looks good, maybe add > http://hg.mozilla.org/index.cgi/mozilla-central/rev/98e43ce43c79 as well? Thanks, will do. > >diff --git a/client.mk b/client.mk > Why ssh vs. http? (http should be best for most users) Sure, probably some glitch of mine. > We need to account for comm-central's aclocal.m4 as well: > $(TOPSRCDIR)/aclocal.m4 (aclocal.m4 gets run by configure if its in current > working dir, so we need both here) Sure, I probably added that aclocal.m4 after editing this. > we need to keep the mozilla-central deps here as we don't chain against their > client.mk and use our configure to call theirs: > > $(TOPSRCDIR)/mozilla/allmakefiles.sh > $(wildcard $(TOPSRCDIR)/nsprpub/configure) > $(wildcard $(TOPSRCDIR)/mozilla/config/milestone.txt) > $(wildcard $(TOPSRCDIR)/config/chrome-versions.sh) True, except chrome-versions.sh, which is obsolete (should be part of a cleanup in mozilla-central, should have only been used by xpfe-based chrome). > I'm not sure about the confvars.sh for mozilla/ (ted?) I don't think so, confvars.sh are only used for the dir given in --enable-application, AFAIK. > And while we're here dont' forget our (new) allmakefiles.sh Sure, thanks. > s/ maybe_clobber_profilebuild// per no PGO support. Thanks, right. PGO stuff should be only added once we actually do support PGO. > >diff --git a/client.py b/client.py > Wouldn't this make more sense to print once, and outside the for loop? (similar > to what client.mk currently logs) Not sure, I just copied that from the old version of mozilla-central's client.py, what do others think about this? > >diff --git a/configure.in b/configure.in > Why the sqlite version number change (vs. mozilla-central)? Because they backend out a SQLite update since I copied the file from mozilla-central. That version number, along with a few others, is probably irrelevant for us and can be stripped from our version, but I want to leave this until after we have working stuff and automated testing of that repo (i.e. after the switch). > -FIREFOX_VERSION=`cat $topsrcdir/browser/config/version.txt` > +FIREFOX_VERSION=`cat $MOZILLA_SRCDIR/browser/config/version.txt` > > You delete this var from our autoconfig.mk, if this is wanted, add it back > there; or remove this. Right. We probably should delete all those $mozapp_VERSION vars here in the long run, we should be able to do it right away for this one. > even though this would add branding to configure, perhaps --enable-lightning or > --enable-calendar-extension as an option, and obsolete --enable-calendar > itself? (That would be a decision for calendar team though, imo). Back in the old days, --enable-calendar meant enabling the Calendar extension, as Lightning is basically the modern version of this, it sounds reasonable to me to just put that same option back into use again. We can surely change it, but it sounds reasonable to me to just resurrect this (has been dead for ages now, actually). > *** > I'm assuming that absence of an arg in our configure.in does not preclude it > from being used in mozilla/configure.in (if thats wrong there are additional > comments I'll need to make). You're right there. > >diff --git a/config/config.mk b/config/config.mk > DEHYDRA_MODULES = \ > - $(topsrcdir)/xpcom/analysis/final.js \ > + $(MOZILLA_SRCDIR)/xpcom/analysis/stack.js \ > $(NULL) > > TREEHYDRA_MODULES = \ > - $(topsrcdir)/xpcom/analysis/outparams.js \ > - $(topsrcdir)/xpcom/analysis/stack.js \ > + $(MOZILLA_SRCDIR)/xpcom/analysis/outparams.js \ > $(NULL) > > Huh? (re: .js script changes) Why not? Actually, I haven't tested *hydra and don't know if it's reasonable to keep this in our build system, but if it's supposed to work at all, the xpcom/ dir lives under the MOZILLA_SRCDIR, not under our topsrcdir. > >diff --git a/config/rules.mk b/config/rules.mk > checkout: > - $(MAKE) -C $(topsrcdir) -f client.mk checkout > + $(PYTHON) $(topsrcdir)/client.py checkout > > This target probably is not even supported [for mozilla-central], but "why not" > keep it working, sure ;-) I thought the same ;-) (In reply to comment #9) > (From update of attachment 329062 [details] [diff] [review]) > My review should not count the licence.txt bits, I'm no legal buff. license.txt is just a plain copy of the mozilla/LICENSE file (which is bound to be replaced by some web page pointer, AFAIK). Your nits have been committed to calemaisu-test as changeset 322c38bb61b1, thanks for looking into those patches.

(In reply to comment #10) > (In reply to comment #8) > > I'm not sure about the confvars.sh for mozilla/ (ted?) > > I don't think so, confvars.sh are only used for the dir given in > --enable-application, AFAIK. Actually after looking (just now) at how confvars.sh is used, we *do* want that added here, as if a confvars.sh file changes we need to reconfigure for that app (and client.mk has no easy way to distinguish just on app, so we'll assume all are important, as mozilla-central does). > > >diff --git a/configure.in b/configure.in > > Why the sqlite version number change (vs. mozilla-central)? > > Because they backend out a SQLite update since I copied the file from > mozilla-central. That version number, along with a few others, is probably > irrelevant for us and can be stripped from our version, but I want to leave > this until after we have working stuff and automated testing of that repo (i.e. > after the switch). > Ok, sure; but for our commit of the repo we should (need?) to have the correct version numbers here. (I'm all for future removing of the unneeded stuff) > > >diff --git a/config/config.mk b/config/config.mk > > DEHYDRA_MODULES = \ > > - $(topsrcdir)/xpcom/analysis/final.js \ > > + $(MOZILLA_SRCDIR)/xpcom/analysis/stack.js \ > > $(NULL) > > > > TREEHYDRA_MODULES = \ > > - $(topsrcdir)/xpcom/analysis/outparams.js \ > > - $(topsrcdir)/xpcom/analysis/stack.js \ > > + $(MOZILLA_SRCDIR)/xpcom/analysis/outparams.js \ > > $(NULL) > > > > Huh? (re: .js script changes) > > Why not? Actually, I haven't tested *hydra and don't know if it's reasonable to > keep this in our build system, but if it's supposed to work at all, the xpcom/ > dir lives under the MOZILLA_SRCDIR, not under our topsrcdir. Look at the added and removed lines again - .../final.js \ + .../stack.js \ ... - .../outparams.js \ - .../stack.js \ + .../outparams.js \ that is what I meant, not the correcting for new paths. :-)

Comment on attachment 329058 [details] [diff] [review] mailnews changes, v1 >--- a/mailnews/compose/build/Makefile.in >+++ b/mailnews/compose/build/Makefile.in >- $(DEPTH)/xpcom/MoreFiles/$(LIB_PREFIX)macmorefiles_s.$(LIB_SUFFIX) >+ $(DEPTH)/mozilla/xpcom/MoreFiles/$(LIB_PREFIX)macmorefiles_s.$(LIB_SUFFIX) How about + ... $(MOZDEPTH)/xpcom/... instead? ...also would be nice to avoid all those "../" additions in tests, but in interest of getting things working, no problem. (p.s. I'm not a mailnews peer; but extra eyes don't hurt)

Comment on attachment 329059 [details] [diff] [review] Thunderbird changes, v1 Not a mail/ peer either, but assuming the following change is intended (or is needed for some reason I don't get) r+ >--- a/mail/app/Makefile.in >+++ b/mail/app/Makefile.in > ifneq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT))) > TK_LIBS := -framework Cocoa $(TK_LIBS) >+ifdef BUILD_STATIC_LIBS >+TK_LIBS := -framework SystemConfiguration $(TK_LIBS) >+endif > endif

Comment on attachment 329062 [details] [diff] [review] SeaMonkey changes, v1 >--- a/suite/locales/Makefile.in >+++ b/suite/locales/Makefile.in >-include $(topsrcdir)/toolkit/mozapps/installer/packager.mk >-include $(call EXPAND_LOCALE_SRCDIR,toolkit/locales)/installer/windows/charset.mk >+include $(topsrcdir)/mozilla/toolkit/mozapps/installer/packager.mk >+include $(call EXPAND_MOZLOCALE_SRCDIR,toolkit/locales)/installer/windows/charset.mk actually $(MOZILLA_SRCDIR) here too :-)

(In reply to comment #11) > [confvars.sh] Cleared this up via IRC - we are watching the confvars.sh files in our repo, that line is in, but we are never looking at any in mozilla/ (--enable-application=../$MOZ_BUILD_APP on calling mozilla/configure) so we don't need to watch them. > > > >diff --git a/configure.in b/configure.in > [sqlite version number change (vs. mozilla-central)] > > Ok, sure; but for our commit of the repo we should (need?) to have the correct > version numbers here. (I'm all for future removing of the unneeded stuff) I don't think we need it but I can correct it to state whatever version is current when we actually check this into comm-central. > > > >diff --git a/config/config.mk b/config/config.mk > > > DEHYDRA_MODULES = \ > > > TREEHYDRA_MODULES = \ > > > > > > Huh? (re: .js script changes) > > Look at the added and removed lines again D'oh! Sure, correcting that. (In reply to comment #12) > >--- a/mailnews/compose/build/Makefile.in > >+ $(DEPTH)/mozilla/xpcom/MoreFiles/$(LIB_PREFIX)macmorefiles_s.$(LIB_SUFFIX) > > How about + ... $(MOZDEPTH)/xpcom/... instead? Thanks, apparently didn't see this one when introducing that var. > ...also would be nice to avoid all those "../" additions in tests, but in > interest of getting things working, no problem. I'm fully with you, but as I explained in comment #3, I know no better way. (In reply to comment #13) > >--- a/mail/app/Makefile.in > >+ifdef BUILD_STATIC_LIBS This part is intentional, Standard8 pointed me to it being needed. (In reply to comment #14) > >--- a/suite/locales/Makefile.in > >+include $(topsrcdir)/mozilla/toolkit/mozapps/installer/packager.mk > > actually $(MOZILLA_SRCDIR) here too :-) Thanks, another oversight. All those nits fixed in calemaisu-test changeset b59390f307e3.

Just some comments and thoughts to begin with: - With this repository pulled you can't build Firefox/xulrunner in the same way as SeaMonkey/Thunderbird etc. We should point this out in documentation - I think it would be worth getting that started now on devmo so that we at least have draft versions. I'm going to say documentation is a blocker to switching over - we need to tell our developers how to use this, and I think hooking into devmo would be the best way. - If I use client.py to update mozilla-central, then there is no -v option so that I know what files changed. - I would complain about not having fast-update for cvs, but given the size of what we are now pulling, I'm not fussed about it for getting this going. - Please file a follow-up bug for tidying up what we really do and don't need in autoconf.mk.in, configure.in etc - I'm not sure about the big differences in the build.mk files. Why should we have so few things defined for the main mozilla build, but more for our own? Especially as the tier_app items are different. Definitely worth some comments there. - Just to note that I think we should try and tidy up (i.e. remove) the LDAP c-sdk/xpcom configure options etc from mozilla-central asap. Though I need to post a message or two on newsgroups about that. - For the ../ changes in tests, we'd have to change do_get_file and do_import_script functions in head.js, is there any way we can do that easily as I think it would simplify the changes we have here, and the perceived logic. http://mxr.mozilla.org/mozilla-central/source/tools/test-harness/xpcshell-simple/head.js#136 - Is editor/ui actually built by anyone else, if not we should probably have a bug on removing it from mozilla-central and pull the cvs one, as I wouldn't be surprised if they aren't in sync. - Any chance you could do a new version of attachment 329057 [details] [diff] [review] now you've address Justin's comments?

Comment on attachment 329059 [details] [diff] [review] Thunderbird changes, v1 >-MOZ_APP_VERSION=$THUNDERBIRD_VERSION >+MOZ_APP_VERSION=`cat $topsrcdir/$MOZ_BUILD_APP/config/version.txt` >+THUNDERBIRD_VERSION=$MOZ_APP_VERSION I don't see why you've added the processing here, when it is still in configure.in.

(In reply to comment #16) > [documentation] From what I see, the build process for comm-central and mozilla-central is nearly the same, with the exception of client.py, but I agree it's a good idea to document that. Oh, and you still can build FF/XR from the code in the mozilla/ subdir :) > - If I use client.py to update mozilla-central, then there is no -v option so > that I know what files changed. Hmm, should we add that by default? If we want to make it option-driven, I need help on how to do it, as I'm still quite weak on python stuff. > - I would complain about not having fast-update for cvs, but given the size of > what we are now pulling, I'm not fussed about it for getting this going. The amount of things we are pulling from cvs should decrease even more over time, esp. when calendar will join comm-central and when we can ditch wallet and webdav. > - Please file a follow-up bug for tidying up what we really do and don't need > in autoconf.mk.in, configure.in etc Filed bug 445141 on that. > - I'm not sure about the big differences in the build.mk files. Why should we > have so few things defined for the main mozilla build, but more for our own? > Especially as the tier_app items are different. Definitely worth some comments > there. The build.mk are the main files that actually drive our builds, defining what is built at all. We should built everything but toolkit in our build system in theory, but extensions/ being in mozilla/ (and the stuff in there relying on that to a certain part) forces us to build that dir with the Mozilla build system. The additions there are mainly: - Caring that webdav is built in the Mozilla buildsystem, with extensions, if Lightning is built (this was previously call by the Lighting build itself, but that now run in our build system). - Building LDAP ourself in our build system (was previously done by toolkit, assuming it lived with Mozilla) - Calling the Lightning build if needed - Call editor/ui from mozilla/ (can be built in our own build system, sounds like an easy candidate to move over to comm-central in the future - filed bug 445146 but there might be room for discussion) > - Just to note that I think we should try and tidy up (i.e. remove) the LDAP > c-sdk/xpcom configure options etc from mozilla-central asap. That's part of the cleanup I just filed bug 445143 for. :) > - For the ../ changes in tests, we'd have to change do_get_file and > do_import_script functions in head.js, is there any way we can do that easily > as I think it would simplify the changes we have here, and the perceived logic. This is generic code used for all xpcshell tests, right? I tried to not dig into lots of other people's code but get things fixed so they work first - we can always improve stuff to be more smooth later on. > - Is editor/ui actually built by anyone else, if not we should probably have a > bug on removing it from mozilla-central and pull the cvs one, as I wouldn't be > surprised if they aren't in sync. Callek cared they are in sync already. I'd rather not pull more things from cvs, it better to use what's in hg already and only move it to the right place (i.e. in our repo) when everything else is working. This whole switch thing is big enough already, I hate to complicate and push it out even more. > - Any chance you could do a new version of attachment 329057 [details] [diff] [review] now you've address > Justin's comments? I can surely do that, I just wasn't sure if I should wait for more comments from other people first. (In reply to comment #17) > (From update of attachment 329059 [details] [diff] [review]) > >-MOZ_APP_VERSION=$THUNDERBIRD_VERSION > >+MOZ_APP_VERSION=`cat $topsrcdir/$MOZ_BUILD_APP/config/version.txt` > >+THUNDERBIRD_VERSION=$MOZ_APP_VERSION > > I don't see why you've added the processing here, when it is still in > configure.in. We should remove it from configure.in, and, as I said in comment #4, "we should IMHO phase out those brand_version vars completely". configure.in should not need to care about app-specifics, as we nowadays have confvars.sh exactly for those app-specific things.

Here's a new relative buildsystem patch, I fixed Callek's nits, make codesighs work, and incorporated a few fixes that went into the same files on mozilla-central.

(In reply to comment #18) > (In reply to comment #16) > > - If I use client.py to update mozilla-central, then there is no -v option so > > that I know what files changed. > > Hmm, should we add that by default? If we want to make it option-driven, I need > help on how to do it, as I'm still quite weak on python stuff. I don't mind tackling this; I'd prefer to wait until after we switch to Hg though, can probably get davida to review (as he claims he is quite good with python). > > - For the ../ changes in tests, we'd have to change do_get_file and > > do_import_script functions in head.js, is there any way we can do that easily > > as I think it would simplify the changes we have here, and the perceived logic. > > This is generic code used for all xpcshell tests, right? I tried to not dig > into lots of other people's code but get things fixed so they work first - we > can always improve stuff to be more smooth later on. I was thinking the same thing as Standard8 here; but I would rather have us on Hg and working (with running tests on boxen) before I even consider touching the test system, and that means biting the bullet with the "../" stuff. Can we get a followup bug on this as well (so we don't loose it)? > > - Is editor/ui actually built by anyone else, if not we should probably have a > > bug on removing it from mozilla-central and pull the cvs one, as I wouldn't be > > surprised if they aren't in sync. > > Callek cared they are in sync already. I'd rather not pull more things from > cvs, it better to use what's in hg already and only move it to the right place > (i.e. in our repo) when everything else is working. This whole switch thing is > big enough already, I hate to complicate and push it out even more. As KaiRo said, I did research to ensure editor/ui and xpfe/ were both in sync, I'll do a second-check right after we switch - just to be sure nothing slipped in-between. If there are other "in-mozilla-central but not used there" CVS files/dirs, let me know in a followup bug and I'll investigate those as well.

Comment on attachment 329056 [details] [diff] [review] creating the buildsystem, v1 >diff --git a/Makefile.in b/Makefile.in make tier_xxx is broken, I'm not worried about that for this first version, but I think we should file a bug on it. I've mainly looked at the diffs for this patch, but seeing as the system works I'm happy to go with it for the time being - obviously we'll do tidy up as we go (as per some of the other bugs).

Comment on attachment 329059 [details] [diff] [review] Thunderbird changes, v1 Looks good. One thing, given http://wiki.mozilla.org/SeaMonkey:hg-based_build#Build_error_when_importing_example_mozconfig I think we could just remove/not add mail/config/mozconfig r=me with that fixed.

The only thing I haven't tried is doing an xulrunner based build - given that we're not really anywhere near to that, I'm not going to worry about it working initially.

Comment on attachment 329056 [details] [diff] [review] creating the buildsystem, v1 From configure.in: +# XXX: do we need those in comm-central at all?? +AC_CONFIG_HEADER( +mozilla/netwerk/necko-config.h +mozilla/xpcom/xpcom-config.h +mozilla/xpcom/xpcom-private.h +) No, you don't, the other configure will handle that. +if test "$MOZ_OPTIMIZE" = "1"; then + ac_configure_args="$ac_configure_args --enable-optimize" +fi You might need to change this, the way it's written if someone does --enable-optimize=-O2, you won't pass the compiler flags down to the other configure. Overall this looks fine. I'm mostly concerned with how you're going to keep up to date with changes we make to the moz build system, but I guess they're bound to diverge as we remove things that you're not interested in from your build system, and remove suite/tbird specific bits from ours. I think you'll need to have a plan for that going forward, but maybe it's not as big of a deal as I think.

(In reply to comment #24) > +if test "$MOZ_OPTIMIZE" = "1"; then > + ac_configure_args="$ac_configure_args --enable-optimize" > +fi > > You might need to change this, the way it's written if someone does > --enable-optimize=-O2, you won't pass the compiler flags down to the other > configure. Actually, I just tested with a manual call to our configure from an empty dir to act as objdir, and the autoconf.mk inside mozilla/ ended up with |MOZ_OPTIMIZE_FLAGS = -O2| just like ours, so it looks to me like there's no problem. In the common case, where we have a mozconfig, we also have both configure scripts reading that mozconfig. > Overall this looks fine. I'm mostly concerned with how you're going to keep up > to date with changes we make to the moz build system, but I guess they're bound > to diverge as we remove things that you're not interested in from your build > system, and remove suite/tbird specific bits from ours. I think you'll need to > have a plan for that going forward, but maybe it's not as big of a deal as I > think. I subscribed to all the mozilla-central hgweb log feeds of the core buildsystem files we have copied, so I should get notified of changes there and should be able to keep up with those we need to port over.

all build system patches pushed to comm-central: creating the buildsystem: http://hg.mozilla.org/comm-central/index.cgi/rev/66c9c3ea3cbe mailnews changes: http://hg.mozilla.org/comm-central/index.cgi/rev/c087100cf4cf SeaMonkey changes: http://hg.mozilla.org/comm-central/index.cgi/rev/451a80b01ba2 Thunderbird changes: http://hg.mozilla.org/comm-central/index.cgi/rev/201855946b54