The Secret Life Of a Patch

Posted by cwright on 2007.05.15 @ 17:38

Filed under:

Thunderbird.appMozilla is an open source project that produces some widely used software. Their most noteworthy product to date is Firefox, a standards-compliant web browser.

Being open source, their projects and products are often enhanced by the contributions of others. These contributions often come in the form of a “patch” — a file that tells the computer what to change in the source code to add the contribution.

Each organization has their own policy for submitting patches, and their own rules for who gets to apply patches, what to do if one of them breaks something, etc.

On the Mozilla site, there is a huge corpus of documentation describing all the rules, regulations, and policies regarding path submission, application, and review. They also document how to create patches. The source of information is completely staggering. This is a mixed bag of blessing (all the information you need is available …) and curse (… if you’re patient enough to find it)… I’m more of a visual-step-by-step kind of learner, so I’m going to cover some of their basic patch creation and submission policies here. Note that just about all of this can be gleaned by reading all their documentation. But I found that this method was more intuitive to me, and perhaps a bit snappier (even if it did break their protocol a few times).

This is the process and timeline that I have experienced with my first patch — a simple modification to Mozilla Thunderbird to add a new “Add to Bcc:” button to the message composition window.

2007.04.02 – I complete the initial (alpha) version of the patch, and do some personal testing. Testing is very important.

2007.04.03 11:17:52 PDT – I revise my patch. This includes some layout changes, and some public testing. At this point, I create a patch for submission to Mozilla. The command I used: # diff -Naur mozilla.orig mozilla. When submitting a patch, they have several drop-down checkboxes with archaic options. This meant little to me, but I figured + was the best-looking option I had. I check that, and submitted the patch.

Mozilla Patch Option set Mozilla's Archaic Patch Selection Choices

2007.04.03 14:18:39 PDT – My first submission feedback arrives: I didn’t diff against the correct original source branch, and I missed a few preferred diff parameters. Suggested command: # diff -pu8 . Also, the + option was apparently special, and I wasn’t supposed to check that (+ means the patch has been reviewed by a module owner, and has been found acceptable for superreview.) I do some testing, and find that -pu8 seems inappropriate; -p is for C function names (my patch was to some XML), and -u8 is deprecated. Correct usage is # diff -U 8 for the desired result.

2007.04.03 20:11:08 PDT – At this point, I contact the feedback guy for more information, asking what I should do to create a more acceptable patch. I create a patch against the current CVS, using similar commands to the above. Submitted a new patch to obsolete the old one.

2007.04.03 09:25:14 +0200 – He responds with some very helpful feedback regarding things a patch must include. It must be versioned against the current CVS, and it must also include index lines for revision information. I quickly form an appropriate patch with the correct format. The correct command isn’t diff at all, but this: # cvs diff -p -u -8. This makes a patch that can be applied to the source in-place. It also indexes which files are modified, and marks their revision number in CVS. This makes backing-out the patch – if necessary – easier. 2007.04.04 02:03:03 PDT – I generate the correct form of patch, and submit again (3rd patch, obsoleting the previous two). I also request review from bienvenu. It’s a long wait, so I go do other things, like work on other patches. 2007.04.05 08:19:43 PDT – The patch is granted review (it gets that + that I mistakenly set on the first one), and automatically gets promoted to superreview from mscott. However, there is a question attached that indicates a lack of actual testing. The reviewer asks about the aesthetics of the new widget.

Then, the standard Open Source Controversy takes control… 2007.04.05 13:37:10 PDT – concerned-user

> looks reasonable to me - one question - does the contacts siderbar initially > come up wide enough to see all three buttons? No, definitely not. For all major platforms it’s too narrow. The ‘To’ and ‘CC’ buttons already fills up the complete width. If the new hbox doesn’t wrap we get an ugly scrollbar. On the other hand we shouldn’t enlarge the contact sidebar anymore. It has still the correct width. IMO the ‘BCC’ button has to wrap into the next line.

Frustrating as it is, I gather my composure and read the whole set of exchanges that have gone on in my absence. The whole “Wrap into the next line” thing really pushed my buttons since it does wrap, as designed. I specifically revised the patch before ever even posting it to include this behaviour, because, as noted, it’s ugly/unusable if it doesn’t wrap. I know this from testing, something that obviously isn’t happening anywhere else yet…

At this point, the bug officially gets assigned to me (since I’m working on the patch), and I post a reply indicating tested behavior of the patch (which no one else seems to have done thus far…):

2007.04.05 14:24:06 PDT – cwright

Woah, woah, calm down :) I’ll try to explain. In the contacts side bar, we have the address book selector, the directory pane, and then the add-to and add-cc button hbox. With the additional hbox for the add-bcc button, it simply takes space from the directory listing. It does _Not_ change the width of the side bar. It does _Not_ change the height of the side bar. It does _Not_ change the window dimensions in any way. Its modifications to the user interface are just about the same as the ones for the extension that was mentioned in Comment #22. Nothing radical. I developed it and tested it on OS X, surely one of the “Major Platforms” you reference. Normal window size is perfectly usable. Maximizing the window does not wrap. Maximizing the window and then dragging the side-bar resizer all the way to the right still doesn’t wrap. That’s what the spring tags are for I believe (they’re in both hboxes, just in case). Would it be more helpful to just fling up some before/after screenshots?

Hopefully someone will actually test the patch before it gets included in the release branch. Empirical evidence suggests that this won’t be the case though.

2007-04-09 05:36:22 PDT – (apparently, open source developers don’t work over Easter.) I get my first positive feedback: Any chance to get this into TB 2?

Don’t get your hopes up yet though. This school of thought is shot down in short order.

2007-04-09 09:14:32 PDT

(In reply to comment #40) > add-cc button hbox. With the additional hbox for the add-bcc button, it simply > takes space from the directory listing. It does _Not_ change the width of the > side bar. It does _Not_ change the height of the side bar. It does _Not_ Especially on Mac OS X the default width of the sidebar is still too narrow. The right side of the CC-Button is still cut-off. Can you provide a screenshot after this changes were applied? Are the To and CC buttons smaller with that patch? > Would it be more helpful to just fling up some before/after screenshots? Indeed. It would be. (In reply to comment #41) > Any chance to get this into TB 2? No, it is too late for TB2.0 bcause we also have l10n changes here.

Once again, did anyone bother to test the patch? I upload a screen shot, and hope that gets around the “actually-build-and-look-at-the-screen” bug that these people are suffering from.

Screenshot to save others from having to test my patch.

Four days pass, and all is silent. Is someone testing the patch? Did they fall asleep? Finally, someone chimes in. And, they tested and patch. And they have more to gripe about…

2007-04-13 23:52:22 PDT

Thank you Christopher. Meanwhile I got my Mac OS X to build my own builds. After integrating your patch all looks fine. But there is one thing what I would prefer. IMO putting the CC and BCC button on one line would make a bit more sense due to both sending copies to given recipients. David what’s your opinion?

It took 9 days to get someone else to test this patch. And verify it’s ok-fullness. But it only took 1 day to get review, and there’s still no word on superreview. It is now no wonder why there are so many stupidly trivial extensions out there; It seems to be the only way to actually change Thunderbird’s behavior.

The distinction between “copy” and “actual” seems very arbitrary. The only reason a To: field exists in E-mail is to mess around with reply versus reply-to-all scenarios. While BCC and CC have similar operation in this way, I feel that To: and CC: are better grouped together because both are published information, while those in the BCC field are unpublished. I don’t know that there’s a right answer to this, but I have a feeling there’s going to be some debate about it anyway.

I figured there would be some kind of discussion or suggestion period at this point. However, I was quite mistaken. Instead the list remained silent for another 7 days, and then there was another request for help applying the patch.

2007-04-20 11:01:25 PDT —

How do I apply the patch on comment #30? When I click on the link, I see the code.

So basically, we can group our responses into two categories:

  • People who want the patch applied, and have little or no experience with programming or patching
  • People who like to drag their feet without qualified statements

2007-04-30 11:01:37 PDT — Some more administrative action takes place: The QA Contact is changed from undefined to message-compose@thunderbird.bugs, a bogus-looking e-mail address.

2007.05.13 – When I initially started this entry over 40 days ago, I figured it’d be 4 or 5 weeks from start to finish. This would work out to be about 1 week per line of code, which is more than fair. After all, at that rate it would take thousands of years for the bigger patches that regularly make it in somehow.

This, unfortunately, is not the case. We’ll be pushing 6 weeks now, which is ridiculous for a non-branching, cosmetic patch. I can understand the volume of show-stopping bugs, the ones that crash Thunderbird, the “important” bugs and all that. I really do. But at rates like this, I can also see why nothing has changed significantly with the Thunderbird project in several years. Why the 1.5, 2.0, and 3.0alpha are all the same program, with minor alterations. And, most importantly, why the amount of contribution to this project leave a lot to be desired. I’ll continue another entry when/if this ever wraps up, but in the meantime, I think we may be better off switching to Horde.

Man, sometimes I hate open source…