Closed Bug 545625 Opened 14 years ago Closed 14 years ago

back-fork ifdef'd mozStorage of bug 507414 into comm-central or mozilla-1.9.2 for TB 3.1 release

Categories

(Thunderbird :: Build Config, defect)

defect
Not set
normal

Tracking

(thunderbird3.1 beta2-fixed)

RESOLVED FIXED
Thunderbird 3.1b2
Tracking Status
thunderbird3.1 --- beta2-fixed

People

(Reporter: asuth, Assigned: asuth)

References

Details

Attachments

(2 files, 1 obsolete file)

Our original plan to avoid release branch nightmares related to mozStorage, at dveditz's suggestion, was basically to back-port whatever our 'correct' fix was to the [mozilla-1.9.2] branch in an ifdef'd fashion so it was basically NPOTB.

We are about at the stage where we have a correct fix.  The changes are sufficiently comprehensive that it would be a nightmare to try and merge them into the 1.9.2 storage in an interleaved fashion.  This leaves an ifdef'ed alternate build dir.

The current plan is to try and magic things so that we can host the directory in the comm-central repository so that we can avoid needing to clutter up mozilla-1.9.2 as well as avoiding the hassles that come with trying to do that.  Given that comm-central wraps mozilla-1.9.2 with a semi-forked set of build mechanics, this seems tractable although it could get ugly.

One dubious but positive side-effect of this strategy is that we can land the patch from bug 507414 in comm-central even though said patch has not yet been reviewed (and may suffer from review latency given the size of the patch and the reviewer's busy-ness).

The patch on bug 507414 is almost entirely source compatible with all existing consumers.  The exceptions consist of a few lines in the toolkit places component which Thunderbird does not build.  Presumably SeaMonkey does use places so we would want to make sure to conditionalize on Thunderbird.
If putting our forked directory in comm-central does get ugly, we should definitely weigh that against exactly what the hassle of putting it in mozilla-central are and discuss that with mozilla-central owners, given that the cost to them should be quite small.  Since presumably this directory would be Not Part Of The (Firefox) Build, we could potentially land it pre-review there as well.
But this is all premature, let's see if it works well in CC.

If we get lucky and the review turnaround time ends up being quicker than expected (ie completed by Tuesday, Feb 23), then maybe review becomes a non-issue as far as getting this in for Tb3.1b1, which is one of the high-level goals here.
(In reply to comment #1)
> If we get lucky and the review turnaround time ends up being quicker than
> expected (ie completed by Tuesday, Feb 23), then maybe review becomes a
> non-issue as far as getting this in for Tb3.1b1, which is one of the high-level
> goals here.
At present, I don't see why we can't be done with reviews and patches by then, assuming asuth isn't working on anything major other than this.  That may be a poor assumption.
this is basically "rsync -r mozilla-central/storage/ comm-1.9.2/storage-backport/" for the v2.2 patch on bug 507414 with some test_true_async fixes that will go into v2.3.

The fix-ups are in the next patch.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Er, not going to post the fix-up patch yet.  There's some form of lingering asymmetry between a build from clean and an incremental build.  I thought I had resolved it but it would appear I just changed from favoring a working incremental build to favoring a from-scratch build.
Group: mozilla-messaging-confidential
Whiteboard: [needs input asuth]
Thursday at the earliest.
Whiteboard: [needs input asuth] → [needs response dmose]
Group: mozilla-messaging-confidential
blocking-thunderbird3.1: beta1+ → beta2+
Whiteboard: [needs response dmose]
This patch goes on top of the other one.  ThunderbirdTry builds passed.

Note that these two patches do *not* clobber-upgrade the SQLite.  So unless your mozilla dir is using our release branch, you get SQLite 3.16.1.  I was hoping mozilla-1.9.2 would be our friend, but that doesn't look like it's going to happen; I'll probably address that but from a review perspective it doesn't matter.
Attachment #431492 - Flags: review?(bugzilla)
Oh, and important note: the make changes are magicked from mail/build.mk so this will not affect other comm-central targets.  (Which is good, since anyone trying to build places with the back-fork would be saddened by the build failures they encountered.  Since I understand that SeaMonkey has moved directly to 1.9.3, this should not present any long-term gloda problems for them.)
Comment on attachment 431492 [details] [diff] [review]
v1 make changes to sub comm-central/storage-backport for mozilla/storage for mail/ builds http://hg.mozilla.org/comm-central/rev/4be99c0f6036

>diff --git a/mail/build.mk b/mail/build.mk
>--- a/mail/build.mk
>+++ b/mail/build.mk
>@@ -41,6 +41,13 @@
> include $(topsrcdir)/toolkit/toolkit-tiers.mk
> endif
> 
>+## storage backfork.
>+# replace toolkit's storage with our own
>+tier_gecko_dirs := $(patsubst storage,../storage-backport,$(tier_gecko_dirs))
>+# necko also has a dependency...
>+tier_necko_dirs := $(patsubst storage/public,../storage-backport/public,$(tier_necko_dirs))
>+

nit: can we add a comment that this is temporary until we branch for MOZILLA_1_9_2_BRANCH?

I feel that if we're going to stick with 3.6.16.1 for nightly builds for a bit in the hopes that 1.9.2 takes 3.6.22 on branch before we cut beta 2, then we should have a follow-up bug that blocks beta 2 to make the decision as to what we go with.

r=Standard8 with those
Attachment #431492 - Flags: review?(bugzilla) → review+
Attachment #431488 - Attachment description: transplant bug 507414 patch v5 to comm-central/storage-backport → transplant bug 507414 patch v5 to comm-central/storage-backport http://hg.mozilla.org/comm-central/rev/b305fcdd0628
Attachment #431492 - Attachment description: v1 make changes to sub comm-central/storage-backport for mozilla/storage for mail/ builds → v1 make changes to sub comm-central/storage-backport for mozilla/storage for mail/ builds http://hg.mozilla.org/comm-central/rev/4be99c0f6036
Pushed both patches to comm-central; tip of the push is landing of bug 545621:
http://hg.mozilla.org/comm-central/rev/700a775e14da

I'm going to leave this bug open until 507414 has a final patch landed on mozilla-central and we land that variant here.  (Note that at some point mozilla-central's storage will diverge once storage assumes infallible allocation as is now possible on mozilla-central.)
pushed a bloat orange bustage fix:
http://hg.mozilla.org/comm-central/rev/6d5d6fd32ac8

In a nutshell, when gloda was disabled it would not actually shut itself down properly.  However, since the mozStorageConnection destructor calls 'close', this did not particularly matter.  But with the change in storage to cause 'close' to explode on connections where 'executeAsync' has been used, this auto-cleanup no longer works.  We must explicitly cleanup with 'asyncClose'.
(In reply to comment #11)
> pushed a bloat orange bustage fix:
> http://hg.mozilla.org/comm-central/rev/6d5d6fd32ac8

This made the Mail Lk go back down to 26.6 KB.  Windows is still orange, but I think it is because Mail A went from (pre-patch) 309927 to (post-patch) 310005-ish.  That doesn't seem like a meaningful change; does something need to be remastered or will it normalize automatically?  (And Mail MH never really changed).
Whiteboard: [need to land a SQLite 3.6.22 back-fork, can close after bug 507414 lands on m-c trunk and adapted to here]
Blocks: 519543
Blocks: 551783
I have pushed what I believe to be a correct backport of SQLite 3.6.22 into comm-central in the sqlite-backport directory:
http://hg.mozilla.org/comm-central/rev/e1e838f974a5
http://hg.mozilla.org/comm-central/rev/cac6fa2ca198

The first patch is an rsync of mozilla-central's (1.9.3) db/sqlite3 tree, the second patch adapts it to life at comm-central/sqlite-backport and reverts the "mozsqlite3" library change that happened on mozilla-central.  I did this because there are packaging ramifications to that change and I would like for us to coordinate that with the actual landing of said change in mozilla-1.9.2 assuming it happens.

I'm going to watch the builders; I pushed some stuff to the try server that gives me reasonable confidence but our windows try server appears to have died of something unrelated to my patch after the first push and is basically unresponsive.
I backed out the revision that actually did something with sqlite-backport:
http://hg.mozilla.org/comm-central/rev/9860af44e25d

Then I landed a revised version that I hoped would fix things.  The former had issues because of the build-system inconsistency where comm-central installs things by module in dist/include/MODULE but mozilla does not; I attempted to address this by disabling that broken logic as I did in storage...
http://hg.mozilla.org/comm-central/rev/c00d894c9331

But that also did not turn out so well; probably some undesired interaction of other convolutions specific to making sqlite build under our build system without exploding, so it also got backed out:
http://hg.mozilla.org/comm-central/rev/8b13001bc702

Note that the net result is that we still have an rsync'ed version of mozilla-central/db/sqlite3 in comm-central/storage-backport as a result of this.


I'll either figure this out sometime tomorrow or throw up my hands and just beg the mozilla-central people to save us from having to deal with our own build system and land 3.6.22 on 1.9.2.
It's worth noting there are also some very unpleasant interactions going on with nspr's system_wrappers that could also enter the picture.
I just checked the 1.9.2 release schedule and I don't see them saving us; they are currently frozen for an estimated March 30th release.

I propose we just continue to use our release branch for the time being or make a new release branch that we just update the SQLite of to 3.6.22.  If someone else wants to finish up the SQLite back-fork I'm fine with that, but at this point it seems like a waste of demoralizing painful effort to finish up given that 1.9.2 will eventually land 3.6.22 and my demoralizing painful effort to-do list is already full up with tax season upon us...

nb: I wasn't actually trying to be macho when I de-emphasized the need for 1.9.2 to accept 3.6.22 imminently claiming we could back-fork it; I just don't think mozilla-central should have to be burdened by the needs/limitations of Thunderbird.
Whiteboard: [need to land a SQLite 3.6.22 back-fork, can close after bug 507414 lands on m-c trunk and adapted to here] → [use existing or new release branch with SQLite 3.6.22 for beta 1, can close after bug 507414 lands on m-c trunk and adapted to here]
Whiteboard: [use existing or new release branch with SQLite 3.6.22 for beta 1, can close after bug 507414 lands on m-c trunk and adapted to here] → [use existing or new release branch with SQLite 3.6.22 for beta 2, can close after bug 507414 lands on m-c trunk and adapted to here]
My vote would be to get bug 530667 nominated for a 1.9.2.3 landing *now* and see if we can push them for a decision within the next week - I note that they already have some bugs approved for 1.9.2.3 (although none have landed yet, the fact is they can do).

If we can then get a landing on or before the 30th March, we can cut the 3.1b2 release with that in place which would mean good prep for rc1 and we can get the tinderboxes unrestricted again.
I have requested the approval1.9.2.3 flag on bug 530667 with some pleading.
Now that the fix for bug 507414 has landed in its final form on mozilla-central, I have updated our copy.  pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/d4ef96917d25

As the current plan stands, this bug remains open to track getting SQLite 3.6.22 landed on mozilla-1.9.2 so we can stop using our 3.1 beta 1 release branch to provide us with 3.6.22.
Whiteboard: [use existing or new release branch with SQLite 3.6.22 for beta 2, can close after bug 507414 lands on m-c trunk and adapted to here] → [trying to get 3.6.22 landed in mozilla-1.9.2]
Forking Mozilla's mozStorage APIs in TB is one thing. Checking all of sqlite (4MB) is another. Unlike CVS, which only checks out HEAD, distributed VCS generally download, clone and locally store the whole history. That means that even if you remove the directory now, we'll forever download, store, copy, delete etc. 4 MB of source more. To be increased in case of updates.

A better way to import third party source is to create an hg repo for the third party source, and then either use the VCS's repo linking system (not sure whether hg has that) or use our poor man's version of it, in client.py, which checks out venkman and co. sqlite should have been just like that.
(In reply to comment #20)
> Forking Mozilla's mozStorage APIs in TB is one thing. Checking all of sqlite
> (4MB) is another. Unlike CVS, which only checks out HEAD, distributed VCS
> generally download, clone and locally store the whole history. That means that
> even if you remove the directory now, we'll forever download, store, copy,
> delete etc. 4 MB of source more. To be increased in case of updates.
If you are that concerned about bandwidth, you have other options:
https://developer.mozilla.org/En/Mozilla_Source_Code_%28Mercurial%29#Bundles

We shouldn't be constraining ourselves based on download size of our code repository unless it is absurd.  This goes double for a stable release branch which barely gets any changes.
> you have other options

None of them are good for a normal developer who routinely blows away trees, clones and removes trees. It's not just about download, but file operations, too.
We shouldn't *waste* resources - esp. time -, if there are alternatives. I just pointed one out.

> This goes double for a stable release branch which barely gets any changes.

This is on trunk (c-c). Indeed, I wouldn't care if it was a release branch (which is a separate repo), but it's not.
(In reply to comment #22)
> None of them are good for a normal developer who routinely blows away trees,
> clones and removes trees. It's not just about download, but file operations,
> too.
I can't see how this would be a normal workflow for a developer.  Any use case I can think of mercurial has tools for that wouldn't require you to do that.

> We shouldn't *waste* resources - esp. time -, if there are alternatives. I just
> pointed one out.
Engineering resources aren't exactly cheap.  Engineer time is very valuable.
SQLite 3.6.22 w/sqlite.def change was authorized and landed on mozilla-1.9.2 in bug 530667.

Accordingly, I have removed sqlite-backport from comm-central:
http://hg.mozilla.org/comm-central/rev/9e25d86f73cd

I have transplanted the fix from bug 555124 to storage-backport as part of the whole "we want the net results of the proper async patch on bug 507414 in storage-backport":
http://hg.mozilla.org/comm-central/rev/e7e255bfeb1a

I am resolving this bug as fixed as there is nothing left to do.  We are generally victorious.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [trying to get 3.6.22 landed in mozilla-1.9.2]
blocking-thunderbird3.1: beta2+ → ---
Target Milestone: --- → Thunderbird 3.1b2
You need to log in before you can comment on or make changes to this bug.