Tuesday, September 22, 2009

How to submit a patch by email

Submitting a patch by email isn't hard, but doing it well requires some attention.
  • Create the patch from the top level of the source tree. That means, when you are creating the patch, you should be in the directory that has configure, or in the directory above it, or at least you should make your patch look as if you were. (Git does that for you automatically.) In other words, the patch should apply with -p0 or -p1. Even if you are submitting a patch to PL/Perl, do it from the top level. Consistency is nice.
  • Add all new files into the patch. If you are using anonymous CVS, you are kind of out of luck with that. Switch to Git, or if you are daring, CVSup. Or if using plain diff, add the -N option, for example diff -c -N -r. Please don't send new files separately alongside with the patch. They should be in the patch.
  • I personally don't subscribe to the dogmatic insistence on the diff -c format. Sometimes -c works better, sometimes -u. Judge for yourself. Make the patch readable.
  • Don't compress the patch. This might be controversial, but I think unless your patch is huge, compression adds annoyance and saves little else. Because the mail reader at the other end probably won't be able to open the attachment directly, sending the reader through intermediate programs or on a side-trip to the console. Replying with the patch inline to comment probably won't work either. And the mailing list web archive isn't going to do anything useful with that kind of attachment. (It is sometimes submitted that compressing an attachment prevents mangling by the mail software. That is true, but if that is a problem, you should consider getting better mail software.)
  • If you have to compress, stick with gzip. Some mail and web software can process that usefully. If you go with bzip or something else, those chances are decreasing drastically.
  • Whatever you do, there is almost never a reason to wrap a patch into a tar archive. If you must compress it, gzip is enough. If you have to send more than one file, create multiple attachments.
  • I'm not sure if anyone cares about diffstats. I don't. Patches are not judged by how much code they add or remove or which places they touch.
  • Make sure you send your patch with a proper MIME Content-Type header. You might have to look a little harder into your mail client configuration to change this. It should be text/x-diff or text/x-patch or perhaps text/plain, although I think the latter is technically incorrect. But it should be text/something, so people can open the attachment with a text viewer easily. If the attachment has a weird MIME type, you force your readers to save the attachment to disk, then open it from there, which is just annoying. Also, you'll confuse the mailing list web archive in a similar way. If you compress your patch, make extra sure that the MIME type is correct. Don't send a gzip compressed file claiming it is a tar archive.
  • Give your patch a descriptive file name. Not "patch", but perhaps "hot-standby.patch". Make sure your mail client includes that file name into the description of the attachment (in Content-Disposition), so your readers can save the patch under the same name. Don't use absolute file names for this ("/tmp/mypatch").
  • If you are using Git, you may be tempted to rebase your patch into tiny pieces and submit those as N separate email messages. Don't do that. There is nothing wrong with separating a patch into smaller patches. But the standard for commiting is not the smallest possible change that doesn't break anything, it's the smallest possible change that we would want to release if you didn't finish the next one on time.
Those are just the issues I noticed browsing through the current commit fest. Please take a moment to check your patch submission practices and your email client. A good way to check is trying to open your own patch from your own email client, and trying to view the patch in the mailing list web archive.

5 comments:

  1. Great tips, Peter! These would make an excellent addition to the "Submitting a Patch" page on the wiki. http://wiki.postgresql.org/wiki/Submitting_a_Patch

    ReplyDelete
  2. "Don't compress the patch."

    It's too bad the archives don't support attached patches better. Right now it's almost impossible to retrieve something from the archives that is in plain text.

    ReplyDelete
  3. You can fetch the attachments from http://postgresql.markmail.org/ .

    ReplyDelete
  4. This is now linked to by the patch submission page on the Wiki. I think most of the bias against uncompressed patches comes from the days when people used to lose larger patches due to the tiny attachment size filters on the hackers/patches mailing lists.

    ReplyDelete