Thursday, August 21, 2008

Rewriting and mork

The Great Address Book Rewrite is progressing along. Now that cardForEmail and cardForProperty are both done (note: I am including as "done" what is merely awaiting r/sr), casual users of nsIAbMDBDirectory and nsIAddrDatabase can be refactored to use generic directories.

The directory one is completely eradicated (outside its valid uses), excluding the not-as-invalid use of being used for the database and its use as an include for some constants, which I'm not sure is still valid. Databases, on the other hand, are still very much alive (I intend to kill off the one extant in nsMsgCompose shortly).

Databases are used, of course, in the MDB directory stuff. But LDAP replicates only to a database. That's probably not hard to fix. Palmsync uses the database quite liberally. That's annoying because palmsync doesn't compile by default, I don't use it enough to be able to test for regressions (I have a PocketPC, not a Palm), and I develop primarily on Linux, so my best shot is most likely to take a go at cross-compiling it.

Last but not least is the granddaddy of database usage, import. It in fact uses nsIAddrDatabase more often than addrbook itself, and that statistic is lopsided since nsLDIFService in addrbook is the LDIF import, and addrbook includes it for constants a lot and also defines the interfaces. And import consists of four different importing address books, all of which have to be changed, and at the same time. My previous aborted attempt to eradicate the database from nsLDIFService alone took hours, required careful removal of hacks, and was probably not tested. The kill-database-in-import patch will likely find itself over 100 KiB.

In any case, I now believe that the setup is sufficiently abstracted to permit me to start work on an SQL-backed address book to replace mork, with the goal of finishing it by TB 3.0.

Saturday, August 16, 2008

The Great Address Book Rewrite, now on your machine!

Since committing the changes to the nsIAbCard portion of the refactoring, the Great Address Book Rewrite has been progressing faster. I pushed the changes to bug 449618 and am writing tests for bug 450194, while yet putting finishing touches on bug 450197. As these changes come in, bits and pieces of address book code will bitrot.

So what does an extension developer who wants to keep up with trunk builds do? Use my handy address book rewriting branch. If your extension works on the tip of the tree or its various branches, it will likely work on the trunk when the patch is committed.

The repo is also world-writable (as long as you have access to, which means that anyone else with in-progress Address Book rewriting patches can push there too. Don't worry about creating new heads, I'll sort all of that out when I see changes to the repo.

Finally, I would like to point people to the tentative roadmap to the rewrite.

Thursday, August 7, 2008

The Great Address Book Rewrite

The nsIAbCard portion of the Great Address book rewrite landed not long ago. This patch represents the first major overhaul of the address book interfaces in a while. Most specifically, it removes the nsIAbMDBCard interface, adds the nsIAbItem interface, and completely overhauls the nsIAbCard interface from being a flat list of properties to essentially a spruced-up hashtable.

These changes, 295.33 KiB of them for a -U 3 patch, comprise a removal of 2,732 lines of code for 1,871 lines in a total of 52 files, some of which are OS-specific, and the culmination of over 6 months of editing and revising, a timespan also including a change of computers. I've changed the internals of nsAbCardProperty at least three times, and fixed two bugs in a heretofore unused mork class. The APIs I worked off of have changed several times, partially as a result of my experiences with the work, and partially otherwise. Finally, I have my chance to bitrot other people's WIPs instead of finding that any addressbook change bitrots the patch I have (or another in my queue).

The good news: nsIAbCard is now effectively stable to use in extensions or other core code not yet pushed. The only caveats are these:

  • getProperty* has had some late-breaking API discussions, related to how to handle properties that don't exist. It is possible, but not likely, that semantics of existing methods would be changed.
  • copy is not the best thought-out method and will change in the future.
  • equals has some potential problems, such as the fact that its use in circumstances may violate the reflexive property.
  • UUIDs were stripped from the patch pending future design decisions.
  • The two pre-existing properties will exist only as long as mailing lists are insane.

Will this patch create regressions? Probably one or two minor ones; changing an entire internal model, a heavily-used API, and implementations across several files, some of which are OS-specific, and others which are app-specific makes a regression likely. But I have taken effort to find problems, combing through the little import/export I can test with Linux, as well as using all the features I could. grep has proved invaluable, as well as a few tests of Dehydra to find callees.

What's next from here? The next logical step is the creation of nsIAbCollection and related refactoring. However, async APIs and error conditions are still in flux for that. Not all of the OS-specific directories are fully squared away with the new model, but I have limited testability in those areas. UI unforking is a place where we could win big, especially as it saves me a few places to look for usages. Import and palmsync may also want some more love. Naturally, there are also the spin-off bugs, but many of those are not immediately fixable.

For me, at least, I now have some time to work on other bugs while I wait. There are 12 other bugs in my queue, ranging from an IMAP fakeserver to an aggravating news bug to a brand new module (O.K., total rewrite of an old module), 6 of which are more or less ready to be committed. Oh well, life goes on...

Tuesday, August 5, 2008

I want a pony #2

Okay, continuing on trying to find obsolete members. One feature that I'm sure everybody would love is a JS static analyzer. And while we're on the topic of JS, how much longer until Mozilla gets ES 4.0 and static typed JS?

Maybe this isn't so much a pony as an entire ranch, though...

I want a pony

As I was walking through nsIMsgIncomingServer in search of unused attributes, I noticed that some attributes were added in a revision and their usages removed later. But the checkin comments don't give me any clear indication of when it was removed and what replaced it. Anyone fancy binary searching hundreds of checkins just to find one property?

A simple solution to this is a reverse annotate feature: a script or similar that, given a revision number, will look through the subsequent revisions and find out which version (if any) first changed a given line. Bonus points for ignoring whitespace changes. I could probably hack out such a feature myself for hg, but I know so little about CVS that it's hopeless for me to try there. And since most of the important history of mailnews is only in CVS, an hg-only becomes pointless, at least for the next, oh, 5 years.

And before anyone complains about me wanting a pony, let me tell you this: my cousin actually got a pony for Christmas last year (after asking about it for the last, oh, 17+ years).

Monday, August 4, 2008

Thanks, dbaron

Earlier today, bz r+'d bug 366791, and dbaron then checked it in. For anyone building TB, this represented about 95% of the assertions and 60+% of the output (being such a long assertion doesn't help) running TB normally. This makes breaking on assertions feasible now, with so few false positives. Now, the only spammy thing left is the "recurring into frame construction" warning.

So, in short:

Thanks dbaron!