Comment on attachment 381429 [details] [diff] [review] (Av1) Bug 478297 [Checkin: Comment 2] http://hg.mozilla.org/comm-central/rev/5afa025fbe2b

Comment on attachment 390672 [details] [diff] [review] (Bv1) Bug 481780, m-1.9.1 patch only [Checkin: Comment 4] http://hg.mozilla.org/comm-central/rev/f1db251753d2

Maybe you could give me a rs+ for the (simple) patches I'm doing right here?

Comment on attachment 390764 [details] [diff] [review] (Cv1) Bug 451621 + bug 469225 [Checkin: Comment 7] I don't see too much use in this ARM support stuff for us, but in case it doesn't hurt to have it. And no, I don't rubberstamp anything. But I'd prefer a larger patch covering multiple bugs and closing the bug report. I hate multi-patch bug reports without clear targets of when they are FIXED.

Comment on attachment 390764 [details] [diff] [review] (Cv1) Bug 451621 + bug 469225 [Checkin: Comment 7] http://hg.mozilla.org/comm-central/rev/36cc638ec544

Comment on attachment 390873 [details] [diff] [review] (Dv1) Bug 462908 [Checkin: Comment 12] I wonder a bit how much of those vars about libs we don't build in comm-central but mozilla/ we really need, but for the moment, we seem to just have most of this in, so OK.

(In reply to comment #6) > I don't see too much use in this ARM support stuff for us, but in case it > doesn't hurt to have it. (In reply to comment #9) > I wonder a bit how much of those vars about libs we don't build in comm-central > but mozilla/ we really need, but for the moment, we seem to just have most of > this in, so OK. Indeed: I'm not trying to figure out what c-c might actually use or not, I'm taking a "configure.in should be the same in c-c as in m-c, except where c-c needs something different" approach :-| (Like what you seem to want in the buildbot repos.) Still, it's not to late to stop me if you would prefer to do it the opposite way.

(In reply to comment #10) > Still, it's not to late to stop me if you would prefer to do it the opposite > way. No, let's do the sync unless in those cases where we already did remove blocks of things from our build system (which are only a few so far). In some cases, we might have code that needs some of the flags, I don't have a good view of what we are checking in c-c code and what we'd never check there at all. If I had a lot of free time, I'd probably do some checking on that and cut the unneeded parts of configure and other build system files, but then I would have looked into all that porting myself as well ;-)

Comment on attachment 390873 [details] [diff] [review] (Dv1) Bug 462908 [Checkin: Comment 12] http://hg.mozilla.org/comm-central/rev/48ed71f32871

Comment on attachment 392147 [details] [diff] [review] (Ev1) Bug 476448 (m-1.9.1 patch) [Checkin: Comment 14] http://hg.mozilla.org/comm-central/rev/ccaf6c38ce07

From { http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4611778ba22a Bug 97954: Don't cite Makefile variables in the value for LIBXUL_DIST. r=bsmedberg } Bug that bug doesn't quote it.

Comment on attachment 392513 [details] [diff] [review] (Fv1) Changeset 18864:4611778ba22a [Checkin: Comment 19] bug 97954 has a lot of checkin comments referring to it ;-)

Comment on attachment 393366 [details] [diff] [review] (Gv1) Bug 438397 (+ 2 unrelated nits) I don't see much use in introducing a flag that nothing besides XULRunner-specific files are using, as those get it in the Mozilla build system anyway. And are those two nits fixed by any Mozilla checkins as well?

Comment on attachment 392513 [details] [diff] [review] (Fv1) Changeset 18864:4611778ba22a [Checkin: Comment 19] http://hg.mozilla.org/comm-central/rev/b6f5800cf03b

(In reply to comment #18) > (From update of attachment 393366 [details] [diff] [review]) > I don't see much use in introducing a flag that nothing besides > XULRunner-specific files are using, as those get it in the Mozilla build system > anyway. You're right about m-c/XulRunner. /js/src/ has it (unused) too, but maybe this is required to be in sync'!? Wrt c-c, I'm still following my comment 10 idea, as it's the only way I can do it by comparing the files; if you don't want unused code anymore, then you will have to use your new tool you emailed me about and/to tell me which (new code) bug to actually port. > And are those two nits fixed by any Mozilla checkins as well? They are synchronizations from current m-1.9.1: I didn't check history wrt the blank line removal; the indentation was checked in wrong in http://hg.mozilla.org/comm-central/rev/32acc7b19dbe

Per irc discussion: not porting bug 438397.

Comment on attachment 393437 [details] [diff] [review] (Gv2) 2 random nits [Checkin: Comment 22] http://hg.mozilla.org/comm-central/rev/72019929e3f5

(In reply to comment #20) > You're right about m-c/XulRunner. > /js/src/ has it (unused) too, but maybe this is required to be in sync'!? js/src/configure and ./configure don't need to be in sync. js/src/configure just has a lot of crud that we haven't cleaned out yet. The only parts of the main tree and js/src that are required to be in sync are those that are actually checked by the 'check' target in js/src/Makefile.in: check-sync-dirs = $(PYTHON) $(srcdir)/config/check-sync-dirs.py check:: $(check-sync-dirs) $(srcdir)/config $(MOZ_SYNC_BUILD_FILES)/config $(check-sync-dirs) $(srcdir)/build $(MOZ_SYNC_BUILD_FILES)/build Here, srcdir will be js/src, and $MOZ_SYNC_BUILD_FILES will be the top directory of the tree. So ./js/src/config must be the same as ./config, and ./js/src/build must be the same as ./build.

Comment on attachment 393493 [details] [diff] [review] (Hv1) Bug 475064 [Checkin: Comment 25] http://hg.mozilla.org/comm-central/rev/2754edb2bbd3

Comment on attachment 394273 [details] [diff] [review] (Iv1) Bug 411726 [Checkin: Comment 27] http://hg.mozilla.org/comm-central/rev/32d7e909c526

From { http://hg.mozilla.org/releases/mozilla-1.9.1/rev/97d4e97bb54a Bug 97954: Compute fully-substituted values of NSPR_CFLAGS and NSPR_LIBS. r=bsmedberg } Bug that bug doesn't quote it. Per irc discussion with KaiRo, c-c doesn't have/want the "Recreating autoconf.mk with updated nspr-config output" block.

Comment on attachment 394722 [details] [diff] [review] (Jv1) Changeset 18865:97d4e97bb54a [Checkin: Comment 29] http://hg.mozilla.org/comm-central/rev/e1f9e6f48cc0

Comment on attachment 395453 [details] [diff] [review] (Kv1) Bug 463075 [Checkin: Comment 31] http://hg.mozilla.org/comm-central/rev/3dde9c8d8ab2

From { http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a7b2f76a6ab8 [dfb,422221] Gtk/DirectFB port core; patches from dfb tree, r=vlad } Though that bug doesn't explicitly quote it. Things to double check: MOZ_PANGO_CFLAGS: c-c doesn't seem to care about it per http://mxr.mozilla.org/comm-central/search?string=MOZ_PANGO_CFLAGS&case=on&find=%2Fconfig so I remove it. MOZ_PANGO_LIBS: Only used in STATIC_EXTRA_LIBS per http://mxr.mozilla.org/comm-central/search?string=MOZ_PANGO_LIBS&case=on&find=%2Fconfig I'm not sure whether it can be removed too or not, so I left it... MOZ_FIX_LINK_PATHS line: You added |:/usr/lib:/usr/local/lib:/lib| in bug 450957. I don't know whether we should keep or remove it now...

Comment on attachment 395582 [details] [diff] [review] (Lv1) (Bug 422221) Changeset 16526:a7b2f76a6ab8, also remove unused MOZ_PANGO_CFLAGS >+AC_SUBST(MOZ_DFB) >+ AC_SUBST(MOZ_PANGO) Those two only make sense if they're available in autoconf.mk, so you need to port the changes to that file done in http://hg.mozilla.org/mozilla-central/rev/c07f8b188874 as well. Everything else looks fine, but r- for substituting those vars that don't exist there yet.

Lv1, with comment 33 suggestion(s). Damn these bugs with no patches nor checkin comments :-( Note that other checkin happened after the one I ported :-| Thanks to you new tracking tool ;-)

Comment on attachment 395752 [details] [diff] [review] (Lv2) (Bug 422221) Changesets 16526:a7b2f76a6ab8 + 16527:c07f8b188874, also remove unused MOZ_PANGO_CFLAGS [Checkin: Comment 36] Yes, I also probably would have missed that one without my tool ;-)

Comment on attachment 395752 [details] [diff] [review] (Lv2) (Bug 422221) Changesets 16526:a7b2f76a6ab8 + 16527:c07f8b188874, also remove unused MOZ_PANGO_CFLAGS [Checkin: Comment 36] http://hg.mozilla.org/comm-central/rev/d349353f35aa

While there, also convert 3 "dnl" to "if+else+fi". I filed bug 511884 to fix the UI, as a separate issue.

Comment on attachment 395851 [details] [diff] [review] (Mv1) Bug 445391 + (m-c) Bug 485984 [Checkin: See comment 38] http://hg.mozilla.org/comm-central/rev/c11e48d0bef1 + http://hg.mozilla.org/comm-central/rev/32083046dee3 Bustage fix

Comment on attachment 396719 [details] [diff] [review] (Nv1) Bug 499144 [Checkin: Comment 40] http://hg.mozilla.org/comm-central/rev/8d39750eb0eb

Comment on attachment 396939 [details] [diff] [review] (Ov1) Bug 447930 Please watch carefully if that actually might have the opposite effect of what it's supposed to do. The big question is if actually anything in _our_ configure script makes cairo-features.h even be rebuilt. We probably shouldn't, but not sure if we do. And if we don't we should not include this patch at all. Yes, I know it's messy. And I only left this patch in my tool because I can't answer it right away. r=me _only_ if i you can show that it doesn't have the opposite effect of what it should do.

Mark, (instead of a review) could you (just) submit this patch to Thunderbird Try Server? (I'll ask KaiRo for review.)

(In reply to comment #42) > (From update of attachment 396939 [details] [diff] [review]) > Yes, I know it's messy. And I only left this patch in my tool because I can't > answer it right away. Ah, that's why I ported that bug... I checked that running |make configure| twice from c-c did not change the file: so this patch seems unneeded indeed. (In reply to comment #43) > Created an attachment (id=397455) [details] > (Ov2) Remove useless CAIRO, MOZ_TREE_FREETYPE and PANGO configurations and a > few more This "opposite" (cleanup) patch reverts: *all of '(Dv1) Bug 462908' :-> *most of '(Lv2) (Bug 422221) Changesets 16526:a7b2f76a6ab8 + 16527:c07f8b188874, also remove unused MOZ_PANGO_CFLAGS'.

Comment on attachment 397455 [details] [diff] [review] (Ov2) Remove useless CAIRO, MOZ_TREE_FREETYPE and PANGO configurations and a few more Done. It may take a few mins to show up. Serge, If you want access then follow the instructions here: https://wiki.mozilla.org/Thunderbird/Infrastructure/TryServer

(In reply to comment #45) > (From update of attachment 397455 [details] [diff] [review]) > Done. It may take a few mins to show up. Serge, If you want access then follow Thanks. It failed to compile. I'm filling separate bugs to do one removal at a time ... then will see where that leads me to ;-> > the instructions here: > https://wiki.mozilla.org/Thunderbird/Infrastructure/TryServer I filed bug 513512.

Looks like quite a lot of work to port patches one by one, if those files are meant to be the same for 99% or so. hg log --no-merges configure.in outputs a log file with 2480 lines, which means 496 changesets since March 2007. Have you considered adding the Seamonkey/Thunderbird/Calendar bits back into mozilla-central's configure files? How much code would that be? Or add more hooks to use app-specific files, which can live outside moz-central, but use moz-central's configure files?

(In reply to comment #47) > Looks like quite a lot of work to port patches one by one, if those files are > meant to be the same for 99% or so. Actually less, but we haven't sorted out completely yet which parts we just can remove. > hg log --no-merges configure.in outputs a log file with 2480 lines, which means > 496 changesets since March 2007. Not all need porting though, http://dev.seamonkey.at/?d=x&i=mozilla&m=c has a list of what needs to be ported or at least investigated to port. > Have you considered adding the Seamonkey/Thunderbird/Calendar bits back into > mozilla-central's configure files? How much code would that be? A separate build system needs a separate configure - we have discussed this in length before creating the whole comm-central repository, and together with the platform build system owners we came to the conclusion that this is the way to go, even if it's not really pretty. > Or add more hooks to use app-specific files, which can live outside > moz-central, but use moz-central's configure files? We're already doing this to a part, but I don't think it's the solution for the configure script itself. In any case, this bug is about getting thing in sync again between comm-central and 1.9.1, if you want to discuss the general approach, please do that in the newgroups / mailing lists and don't intermingle it with the actual work being done here. This bug is large enough as it is (and I dislike that in any case), even without adding general discussion here when it belongs elsewhere.

(In reply to comment #48) > This bug is large enough as it is (and I dislike that in any case), even > without adding general discussion here when it belongs elsewhere. The checkin comments also make it really difficult to find out at a glance what has actually been ported.

Serge, since c-c has branched and both c-1.9.1 and m-1.9.1 are stable and not taking unneeded patches can we close this bug and just concentrate on c-c vs m-c[1.9.2] for now?

(In reply to comment #50) I am mostly working on c-c (bug 506493) now. But there are probably still a few fixes to port from the list at http://dev.seamonkey.at/?d=x&i=mozilla&m=c for example, so I prefer to leave this bug open ftb.

Ok; lets file followups at least for anything more that needs 191 attention [so its easier to wontfix or approve- etc.] For what its worth though, if there is _any_ risk in a conf change for 191 I'd vote against approval, unless it is a stability or security fix.

(In reply to comment #34) > Created an attachment (id=395752) [details] > (Lv2) (Bug 422221) Changesets 16526:a7b2f76a6ab8 + 16527:c07f8b188874, also > remove unused MOZ_PANGO_CFLAGS Ftr, this implicitly ported bug 463887.

Comment on attachment 396939 [details] [diff] [review] (Ov1) Bug 447930 (In reply to comment #44) > I checked that running |make configure| twice from c-c did not change the file: > so this patch seems unneeded indeed. Confirming: Now, I have a little more understanding of configure" and of what was written in bug 447930: http://mxr.mozilla.org/comm-central/search?string=cairo-features.h&case=on *cairo-features.h.in in listed in toolkit-makefiles.sh, *toolkit-makefiles.sh is listed in allmakefiles.sh, *allmakefiles.sh is called by '. ${srcdir}/allmakefiles.sh' in configure, *and all this happens in mozilla-central (only). *Hence, porting this to c-c is just useless.

Moved from bug 481584.

Needed by bug 474610 port.

Comment on attachment 430064 [details] [diff] [review] (Qv1) Port bug 478221 [Checkin: Comment 57 & 65] http://hg.mozilla.org/comm-central/rev/d1cc1d78e6dd "approval-thunderbird3.0.4=?": Zero risk, build config only.

Comment on attachment 429707 [details] [diff] [review] (Pv1-191) Copy bug 481584 So now I look at it, I see no reason to take this on 1.9.1 as I doubt anyone's going to be doing much with Windows CE builds there.

Comment on attachment 430064 [details] [diff] [review] (Qv1) Port bug 478221 [Checkin: Comment 57 & 65] >+ >+include $(topsrcdir)/mozilla/toolkit/mozapps/installer/package-name.mk >+ What does this change actually fix? If the only reason for including it is for bug 474610, then please include it in the patch for that port. Also, if you want approval on this bug, I'm warning you in advance that my requirement will be that 3.0.4 will be the last patches for this bug to go in on branch, otherwise it will be a nightmare to verify.

(In reply to comment #59) > (From update of attachment 430064 [details] [diff] [review]) > >+ > >+include $(topsrcdir)/mozilla/toolkit/mozapps/installer/package-name.mk > >+ > > What does this change actually fix? At the very least, to avoid c-c to be bitten again by lacking this include, as it was with m-1.9.2. > If the only reason for including it is for bug 474610, then please include it > in the patch for that port. Yes, that bug is what I have more specifically in mind. "But" there is nothing "more" to port for it ;-) > Also, if you want approval on this bug, I'm warning you in advance that my > requirement will be that 3.0.4 will be the last patches for this bug to go in > on branch, otherwise it will be a nightmare to verify. Agreed.

Comment on attachment 432026 [details] [diff] [review] (Rv1) Copy bug 521750 [Checkin: Comment 63 & 66] http://hg.mozilla.org/comm-central/rev/5f560aa814de "approval-thunderbird3.0.4=?": Needed with m-1.9.1.

Comment on attachment 430064 [details] [diff] [review] (Qv1) Port bug 478221 [Checkin: Comment 57 & 65] a=Standard8 with the previously agreed provision that this bug is closed with the landing of these bugs. Note that tree closes at 23:59 Pacific time tonight.

Comment on attachment 430064 [details] [diff] [review] (Qv1) Port bug 478221 [Checkin: Comment 57 & 65] http://hg.mozilla.org/releases/comm-1.9.1/rev/ded59344d902

Comment on attachment 432026 [details] [diff] [review] (Rv1) Copy bug 521750 [Checkin: Comment 63 & 66] http://hg.mozilla.org/releases/comm-1.9.1/rev/517fd95ce33c

Comment on attachment 432137 [details] [diff] [review] (Sv1-191) Copy bug 545755 [Checkin: Comment 67] http://hg.mozilla.org/releases/comm-1.9.1/rev/f949a9f723ce

Verified fixed 3.0 on the basis that what is landed hasn't broken the builds.

Ftr, http://dev.seamonkey.at/?d=x&i=mozilla&m=c doesn't list any more (old) m-1.9.1 changeset to port atm :-)