如何正确地提交内核补丁包

原文:https://lwn.net/Articles/500443/

翻译:blog.ykyi.net 

Greg Kroah-Hartman(又Greg KH)在执行一个了不起的作务:减少内核开发者,尤其是维护者的暴躁情绪。他在日本横滨举行的全球Linux大会上的讲演呼吁公众理解内核维护者的工作,内核贡献者的什么样的行为会导致内核维护者变得暴戾。但是,如果内核贡献者们能够遵守一些约束,他代表他自己做了许多承诺。

Greg Kroah Hartman把Linux内核称之为"有史以来最宏大的软件开发项目",而且它的开发速度也是“亘古未有”。从3.0到3.4,有373个公司的2833位开发者参于了Linux内核的开发。这一年,(2011年的5月到,2012年的5月),Linux内核在每个小时会有5.79处变动。而且这个开发速度仍然要加速,如果你看看3.4的开发过程,每小时共有7.21处变动。这里所说的变动仅仅指能够被合并到主干的补丁包,而那些被拒绝的补丁包没有被统计进来。

补丁包修改了哪些文件,内核开发者就把补丁包发给负责这些文件的维护者。所以的内核维护者,现在大概有700人。他们把改变应用到130个子系统的维护者那里。再从子系统的维护者到Linus的Linux内核代码分支。最后并必Linux内核的主干。如果提交的补丁包一路通过的话。

因此,来看看为什么一些补丁包不会被接受呢?Greg Kroah Hartman以刚过去的两个星期收到的补丁包为例,这两个星期刚好处于3.5版本的Merge Window时间。Merge Window是一个他确不应该接收到许多补丁包的时间段。他应该在Merge Window开始前收到所以有可能会在Merge Window其间被发往Linux Torvalds的补丁包。不过,他说他在这两个星期的时间里收到487个补丁包。其中的大多数都有很多问题,有些补丁包来自那些本应该对内核有更好理解的内核开发者们。

坏的补丁包

Greg KH举例说明了一些他收到的坏的补丁包。其中一个补丁包被命名为:patch 48/48(一个有48个补丁集的最后一个),但是其它的47个都没有。他还收到一堆补丁但没有写清楚先后顺序。如此,他要么猜测一个顺序,这毫无疑问会失败。另一个替代方案就是安全不管这个补丁了。另外还收到过有10个补丁的补丁集,但是2号补丁确丢失了。

另外有一个通过邮件发送来的补丁包被声明“机密”。Greg KH说他经常收到此类方式的补丁。对于此类补丁,你无能为力。因为Linux是在开放的生态中开发的,你不能够给邮件列表发送一个机密邮件偷偷就合并一个补丁。很明显,这种方式发来的补丁是因为在处理邮件阶段的模板造成的,但是'机密'必须被去掉。

还有不良排版的补丁包。比如所有的Tab符都被转成了空格,Microsoft Exchange常常这么做。如果对于你,开发环境是个问题的话,那可以像IBM,Microsoft或者还有其它公司那样,在角落里再放一台Linux机器给开发者用来发送邮件。有时候diff的输出的行前空格都被剥去了,或者diff的输出并不是unified格式(见另一篇博文,讲述如何用diff生成linux内核补丁包)。虽然久经考验的Linux开发者们可以熟练的编辑原始diff输出格式,但是diff的原始输出本身是件很令人恐怖的事情,他们本不应该被如此对待编辑它们。

有些补丁包是在错误的目录下被创建的,比如在一个驱动器目录下。有个补丁包在/usr/src/linux-2.6.32目录下被创建,但是这个补丁包里面有好些错误,包括源代码树的年龄,而且它隐含假设它是在root上构建。在root上构建是相当之危险,如果linux的构建过程中出现一个bug,就有可以把整个文件系统都删除。没有一个内核核心开发者留意到了这个情况,因为他们没有使用root。有建议说把这种bug留下来当成一种威慑力(原作者开玩笑),自然不会被采纳,不过那么极端的危险情况是真的有可能发生啊。

还有离谱的,有些补丁包是针对一个本来与这个补丁包毫无关系的代码树。Greg KH说他曾经收到一个补丁包针对SCSI代码树,实在想不通这个与SCSI毫无关系的补丁关怎么会针对SCSI代码树创建。

然后还有代码风格的问题。有些补丁包没有使用Linux内核的代码风格。提交补丁包的开发者也知晓这个问题,但是他们似乎在说“我不管,让我的代码通过吧!”Greg KH说,现有一些工具可以帮助定位到有这些问题的代码并修复它们。所以,没有任何借口发送不符合代码风格的补丁包。

Greg KH还着重说了编译不能通过的问题。他说,有些内核内核贡献者把不能通过编译的补丁包也发过来。或者有些补丁包集3/6失败了,在6/6修复了。我甚于收到过补丁包在5/8失败,但是作者附带了一个说明说作者在未来某个时候会发来改正方案。另外还有补丁包很明显没有正确的内核文档部分,因为在构建文档的时候会失败,很明显补丁包的创建者根本就没有运行过内核文档抽取工具。(不想译了,太多了…呜~~累死了.)

One of the patches he got "had nothing to do with me". It was an x86 core kernel patch, which is not an area of the kernel he has ever dealt with. But the patch was sent only to him. "I get odd patches" a lot, he said.

The last patch he mentioned was 450K in size, with 4500 lines added. Somebody suggested that it be broken up, but in the meantime several maintainers actually reviewed it, so the submitter didn't really learn from that mistake.

All of this occurred during a "calm two weeks", he said. These are examples of what maintainers deal with on a weekly basis and explains why they can be grumpy. That said, he did note that this is the "best job I've ever had", but that's not to say it couldn't be improved.

If someone sends him a patch and he accepts it, that means he may have to maintain it and fix bugs in it down the road. So it's in his self interest to ignore the patch, which is an interesting dynamic, he said. The way around that is to "give me no excuse to reject your patch"; it is as simple as that, really.

Rules

Kroah-Hartman then laid out the rules that contributors need to follow in order to avoid the kinds of problems he described. Use checkpatch.pl, he said, because he will run it on your patch and it is a waste of his time to have to forward the results back when it fails. Send the patch to the right people and there is even a script available (get_maintainer.pl) to list the proper people and mailing lists where a patch should be sent.

Send the patch with a proper subject that is "short, sweet, and descriptive" because it is going to be in the kernel changelog. It should not be something like "fix bugs in driver 1/10". In addition, the changelog comment should clearly say what the patch does, but also why it is needed.

Make small changes in patches. You don't replace the scheduler in one patch, he said, you do it over five years. Small patches make it easier for reviewers and easier for maintainers to accept. In a ten-patch series, he might accept the first three, which means that the submitter just needs to continue working on the last seven. The best thing to do is to make the patch "obviously correct", which makes it easy for a maintainer to accept it.

Echoing the problems he listed earlier, he said that patches should say what tree they are based on. In addition, the order of the patches is important, as is not breaking the build. The latter "seems like it would be obvious" but he has seen too many patches that fail that test. To the extent that you can, make sure that the patch works. It is fine to submit patches for hardware that you don't have access to, but you should test on any hardware that you do have.

Review comments should not be ignored, he said. It is simply common courtesy if he takes time to review the code that those comments should be acted upon or responded to. It's fine to disagree with review comments, but submitters need to say why they disagree. If a patch gets resent, it should be accompanied with a reason for doing so. When reviewer's comments are ignored, they are unlikely to review code the next time.

Maintainer's role

When you follow those rules there are certain things you can expect from him, Kroah-Hartman said, and that you should expect from the other maintainers as well. That statement may make other maintainers mad, he joked, but it is reasonable to expect certain things. For his part, he will review patches within one or two weeks. Other maintainers do an even better job than that, he said, specifically pointing to David Miller as one who often reviews code within 48 hours of its submission. If you don't get a response to a patch within a week, it is fine to ask him what the status is.

He can't promise that he will always give constructive criticism, but he will always give "semi-constructive criticism". Sometimes he is tired or grumpy, so he can't quite get to the full "constructive" level. He will also keep submitters informed of the status of their patch. He has scripts that will help him do so, and let the submitter know when the patch gets merged into his tree or accepted into the mainline. That is unlike some other maintainers, he said, where he has submitted patches that just drop into a "big black hole" before eventually popping up in the mainline three months later.

He ended by putting up a quote from Torvalds ("Publicly making fun of people is half the fun of open source programming. …") that was made as a comment on one of Kroah-Hartman's Google+ postings. The post was a rant about a driver that had been submitted, which even contained comments suggesting that it should not be submitted upstream. He felt bad about publicly posting that at first, but Torvalds's comment made him rethink that.

Because kernel development is done in the open, we are taking "personal pride in the work we do". As the code comment indicated, the driver developer didn't think it should be submitted because they realized the code was not in the proper shape to do so. It is that pride in the work that "makes Linux the best engineering project ever", he said. Sometimes public mocking is part of the process and can actually help instill that pride more widely.

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.