]>
Commit | Line | Data |
---|---|---|
f7c9fe4b MCC |
1 | .. _development_posting: |
2 | ||
3 | Posting patches | |
4 | =============== | |
75b02146 JC |
5 | |
6 | Sooner or later, the time comes when your work is ready to be presented to | |
7 | the community for review and, eventually, inclusion into the mainline | |
8 | kernel. Unsurprisingly, the kernel development community has evolved a set | |
9 | of conventions and procedures which are used in the posting of patches; | |
10 | following them will make life much easier for everybody involved. This | |
11 | document will attempt to cover these expectations in reasonable detail; | |
f77af637 | 12 | more information can also be found in the files |
9db370de | 13 | :ref:`Documentation/process/submitting-patches.rst <submittingpatches>` |
f77af637 | 14 | and :ref:`Documentation/process/submit-checklist.rst <submitchecklist>`. |
75b02146 JC |
15 | |
16 | ||
f7c9fe4b MCC |
17 | When to post |
18 | ------------ | |
75b02146 JC |
19 | |
20 | There is a constant temptation to avoid posting patches before they are | |
21 | completely "ready." For simple patches, that is not a problem. If the | |
22 | work being done is complex, though, there is a lot to be gained by getting | |
23 | feedback from the community before the work is complete. So you should | |
24 | consider posting in-progress work, or even making a git tree available so | |
25 | that interested developers can catch up with your work at any time. | |
26 | ||
27 | When posting code which is not yet considered ready for inclusion, it is a | |
28 | good idea to say so in the posting itself. Also mention any major work | |
29 | which remains to be done and any known problems. Fewer people will look at | |
30 | patches which are known to be half-baked, but those who do will come in | |
31 | with the idea that they can help you drive the work in the right direction. | |
32 | ||
33 | ||
f7c9fe4b MCC |
34 | Before creating patches |
35 | ----------------------- | |
75b02146 JC |
36 | |
37 | There are a number of things which should be done before you consider | |
38 | sending patches to the development community. These include: | |
39 | ||
40 | - Test the code to the extent that you can. Make use of the kernel's | |
41 | debugging tools, ensure that the kernel will build with all reasonable | |
42 | combinations of configuration options, use cross-compilers to build for | |
43 | different architectures, etc. | |
44 | ||
45 | - Make sure your code is compliant with the kernel coding style | |
46 | guidelines. | |
47 | ||
48 | - Does your change have performance implications? If so, you should run | |
49 | benchmarks showing what the impact (or benefit) of your change is; a | |
50 | summary of the results should be included with the patch. | |
51 | ||
52 | - Be sure that you have the right to post the code. If this work was done | |
53 | for an employer, the employer likely has a right to the work and must be | |
54 | agreeable with its release under the GPL. | |
55 | ||
56 | As a general rule, putting in some extra thought before posting code almost | |
57 | always pays back the effort in short order. | |
58 | ||
59 | ||
f7c9fe4b MCC |
60 | Patch preparation |
61 | ----------------- | |
75b02146 JC |
62 | |
63 | The preparation of patches for posting can be a surprising amount of work, | |
64 | but, once again, attempting to save time here is not generally advisable | |
65 | even in the short term. | |
66 | ||
67 | Patches must be prepared against a specific version of the kernel. As a | |
68 | general rule, a patch should be based on the current mainline as found in | |
5c050fb9 JC |
69 | Linus's git tree. When basing on mainline, start with a well-known release |
70 | point - a stable or -rc release - rather than branching off the mainline at | |
71 | an arbitrary spot. | |
72 | ||
73 | It may become necessary to make versions against -mm, linux-next, or a | |
74 | subsystem tree, though, to facilitate wider testing and review. Depending | |
75 | on the area of your patch and what is going on elsewhere, basing a patch | |
76 | against these other trees can require a significant amount of work | |
77 | resolving conflicts and dealing with API changes. | |
75b02146 JC |
78 | |
79 | Only the most simple changes should be formatted as a single patch; | |
80 | everything else should be made as a logical series of changes. Splitting | |
81 | up patches is a bit of an art; some developers spend a long time figuring | |
82 | out how to do it in the way that the community expects. There are a few | |
83 | rules of thumb, however, which can help considerably: | |
84 | ||
85 | - The patch series you post will almost certainly not be the series of | |
86 | changes found in your working revision control system. Instead, the | |
87 | changes you have made need to be considered in their final form, then | |
88 | split apart in ways which make sense. The developers are interested in | |
89 | discrete, self-contained changes, not the path you took to get to those | |
90 | changes. | |
91 | ||
92 | - Each logically independent change should be formatted as a separate | |
93 | patch. These changes can be small ("add a field to this structure") or | |
94 | large (adding a significant new driver, for example), but they should be | |
95 | conceptually small and amenable to a one-line description. Each patch | |
96 | should make a specific change which can be reviewed on its own and | |
97 | verified to do what it says it does. | |
98 | ||
99 | - As a way of restating the guideline above: do not mix different types of | |
100 | changes in the same patch. If a single patch fixes a critical security | |
101 | bug, rearranges a few structures, and reformats the code, there is a | |
102 | good chance that it will be passed over and the important fix will be | |
103 | lost. | |
104 | ||
105 | - Each patch should yield a kernel which builds and runs properly; if your | |
106 | patch series is interrupted in the middle, the result should still be a | |
107 | working kernel. Partial application of a patch series is a common | |
108 | scenario when the "git bisect" tool is used to find regressions; if the | |
109 | result is a broken kernel, you will make life harder for developers and | |
110 | users who are engaging in the noble work of tracking down problems. | |
111 | ||
5c050fb9 | 112 | - Do not overdo it, though. One developer once posted a set of edits |
75b02146 JC |
113 | to a single file as 500 separate patches - an act which did not make him |
114 | the most popular person on the kernel mailing list. A single patch can | |
115 | be reasonably large as long as it still contains a single *logical* | |
5c050fb9 | 116 | change. |
75b02146 JC |
117 | |
118 | - It can be tempting to add a whole new infrastructure with a series of | |
119 | patches, but to leave that infrastructure unused until the final patch | |
120 | in the series enables the whole thing. This temptation should be | |
121 | avoided if possible; if that series adds regressions, bisection will | |
122 | finger the last patch as the one which caused the problem, even though | |
123 | the real bug is elsewhere. Whenever possible, a patch which adds new | |
124 | code should make that code active immediately. | |
125 | ||
126 | Working to create the perfect patch series can be a frustrating process | |
127 | which takes quite a bit of time and thought after the "real work" has been | |
128 | done. When done properly, though, it is time well spent. | |
129 | ||
130 | ||
f7c9fe4b MCC |
131 | Patch formatting and changelogs |
132 | ------------------------------- | |
75b02146 JC |
133 | |
134 | So now you have a perfect series of patches for posting, but the work is | |
135 | not done quite yet. Each patch needs to be formatted into a message which | |
136 | quickly and clearly communicates its purpose to the rest of the world. To | |
137 | that end, each patch will be composed of the following: | |
138 | ||
139 | - An optional "From" line naming the author of the patch. This line is | |
140 | only necessary if you are passing on somebody else's patch via email, | |
141 | but it never hurts to add it when in doubt. | |
142 | ||
143 | - A one-line description of what the patch does. This message should be | |
144 | enough for a reader who sees it with no other context to figure out the | |
145 | scope of the patch; it is the line that will show up in the "short form" | |
146 | changelogs. This message is usually formatted with the relevant | |
147 | subsystem name first, followed by the purpose of the patch. For | |
148 | example: | |
149 | ||
f7c9fe4b MCC |
150 | :: |
151 | ||
75b02146 JC |
152 | gpio: fix build on CONFIG_GPIO_SYSFS=n |
153 | ||
154 | - A blank line followed by a detailed description of the contents of the | |
155 | patch. This description can be as long as is required; it should say | |
156 | what the patch does and why it should be applied to the kernel. | |
157 | ||
158 | - One or more tag lines, with, at a minimum, one Signed-off-by: line from | |
159 | the author of the patch. Tags will be described in more detail below. | |
160 | ||
5d98932a JC |
161 | The items above, together, form the changelog for the patch. Writing good |
162 | changelogs is a crucial but often-neglected art; it's worth spending | |
163 | another moment discussing this issue. When writing a changelog, you should | |
164 | bear in mind that a number of different people will be reading your words. | |
165 | These include subsystem maintainers and reviewers who need to decide | |
166 | whether the patch should be included, distributors and other maintainers | |
167 | trying to decide whether a patch should be backported to other kernels, bug | |
168 | hunters wondering whether the patch is responsible for a problem they are | |
169 | chasing, users who want to know how the kernel has changed, and more. A | |
170 | good changelog conveys the needed information to all of these people in the | |
171 | most direct and concise way possible. | |
172 | ||
173 | To that end, the summary line should describe the effects of and motivation | |
174 | for the change as well as possible given the one-line constraint. The | |
175 | detailed description can then amplify on those topics and provide any | |
176 | needed additional information. If the patch fixes a bug, cite the commit | |
5c050fb9 JC |
177 | which introduced the bug if possible (and please provide both the commit ID |
178 | and the title when citing commits). If a problem is associated with | |
5d98932a JC |
179 | specific log or compiler output, include that output to help others |
180 | searching for a solution to the same problem. If the change is meant to | |
181 | support other changes coming in later patch, say so. If internal APIs are | |
182 | changed, detail those changes and how other developers should respond. In | |
183 | general, the more you can put yourself into the shoes of everybody who will | |
184 | be reading your changelog, the better that changelog (and the kernel as a | |
185 | whole) will be. | |
186 | ||
187 | Needless to say, the changelog should be the text used when committing the | |
188 | change to a revision control system. It will be followed by: | |
75b02146 JC |
189 | |
190 | - The patch itself, in the unified ("-u") patch format. Using the "-p" | |
191 | option to diff will associate function names with changes, making the | |
192 | resulting patch easier for others to read. | |
193 | ||
194 | You should avoid including changes to irrelevant files (those generated by | |
195 | the build process, for example, or editor backup files) in the patch. The | |
196 | file "dontdiff" in the Documentation directory can help in this regard; | |
197 | pass it to diff with the "-X" option. | |
198 | ||
bf33a9d4 TL |
199 | The tags already briefly mentioned above are used to provide insights how |
200 | the patch came into being. They are described in detail in the | |
201 | :ref:`Documentation/process/submitting-patches.rst <submittingpatches>` | |
202 | document; what follows here is a brief summary. | |
203 | ||
204 | One tag is used to refer to earlier commits which introduced problems fixed by | |
205 | the patch:: | |
206 | ||
207 | Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID") | |
208 | ||
209 | Another tag is used for linking web pages with additional backgrounds or | |
0d828200 MB |
210 | details, for example an earlier discussion which leads to the patch or a |
211 | document with a specification implemented by the patch:: | |
bf33a9d4 TL |
212 | |
213 | Link: https://example.com/somewhere.html optional-other-stuff | |
214 | ||
215 | Many maintainers when applying a patch also add this tag to link to the | |
216 | latest public review posting of the patch; often this is automatically done | |
217 | by tools like b4 or a git hook like the one described in | |
218 | 'Documentation/maintainer/configure-git.rst'. | |
219 | ||
0d828200 MB |
220 | If the URL points to a public bug report being fixed by the patch, use the |
221 | "Closes:" tag instead:: | |
222 | ||
223 | Closes: https://example.com/issues/1234 optional-other-stuff | |
224 | ||
225 | Some bug trackers have the ability to close issues automatically when a | |
226 | commit with such a tag is applied. Some bots monitoring mailing lists can | |
227 | also track such tags and take certain actions. Private bug trackers and | |
228 | invalid URLs are forbidden. | |
229 | ||
230 | Another kind of tag is used to document who was involved in the development of | |
bf33a9d4 | 231 | the patch. Each of these uses this format:: |
f7c9fe4b | 232 | |
75b02146 JC |
233 | tag: Full Name <email address> optional-other-stuff |
234 | ||
235 | The tags in common use are: | |
236 | ||
237 | - Signed-off-by: this is a developer's certification that he or she has | |
238 | the right to submit the patch for inclusion into the kernel. It is an | |
239 | agreement to the Developer's Certificate of Origin, the full text of | |
f77af637 FV |
240 | which can be found in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>` |
241 | Code without a proper signoff cannot be merged into the mainline. | |
75b02146 | 242 | |
24a2bb90 SC |
243 | - Co-developed-by: states that the patch was co-created by several developers; |
244 | it is a used to give attribution to co-authors (in addition to the author | |
245 | attributed by the From: tag) when multiple people work on a single patch. | |
246 | Every Co-developed-by: must be immediately followed by a Signed-off-by: of | |
247 | the associated co-author. Details and examples can be found in | |
248 | :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`. | |
8ee25f6f | 249 | |
75b02146 JC |
250 | - Acked-by: indicates an agreement by another developer (often a |
251 | maintainer of the relevant code) that the patch is appropriate for | |
252 | inclusion into the kernel. | |
253 | ||
254 | - Tested-by: states that the named person has tested the patch and found | |
255 | it to work. | |
256 | ||
257 | - Reviewed-by: the named developer has reviewed the patch for correctness; | |
f77af637 | 258 | see the reviewer's statement in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>` |
f27e1d24 | 259 | for more detail. |
75b02146 JC |
260 | |
261 | - Reported-by: names a user who reported a problem which is fixed by this | |
262 | patch; this tag is used to give credit to the (often underappreciated) | |
263 | people who test our code and let us know when things do not work | |
0d828200 MB |
264 | correctly. Note, this tag should be followed by a Closes: tag pointing to |
265 | the report, unless the report is not available on the web. The Link: tag | |
266 | can be used instead of Closes: if the patch fixes a part of the issue(s) | |
267 | being reported. | |
75b02146 JC |
268 | |
269 | - Cc: the named person received a copy of the patch and had the | |
270 | opportunity to comment on it. | |
271 | ||
2f993509 TL |
272 | Be careful in the addition of tags to your patches, as only Cc: is appropriate |
273 | for addition without the explicit permission of the person named; using | |
274 | Reported-by: is fine most of the time as well, but ask for permission if | |
275 | the bug was reported in private. | |
75b02146 JC |
276 | |
277 | ||
f7c9fe4b MCC |
278 | Sending the patch |
279 | ----------------- | |
75b02146 JC |
280 | |
281 | Before you mail your patches, there are a couple of other things you should | |
282 | take care of: | |
283 | ||
284 | - Are you sure that your mailer will not corrupt the patches? Patches | |
285 | which have had gratuitous white-space changes or line wrapping performed | |
286 | by the mail client will not apply at the other end, and often will not | |
287 | be examined in any detail. If there is any doubt at all, mail the patch | |
5c050fb9 | 288 | to yourself and convince yourself that it shows up intact. |
75b02146 | 289 | |
f77af637 FV |
290 | :ref:`Documentation/process/email-clients.rst <email_clients>` has some |
291 | helpful hints on making specific mail clients work for sending patches. | |
75b02146 JC |
292 | |
293 | - Are you sure your patch is free of silly mistakes? You should always | |
294 | run patches through scripts/checkpatch.pl and address the complaints it | |
295 | comes up with. Please bear in mind that checkpatch.pl, while being the | |
296 | embodiment of a fair amount of thought about what kernel patches should | |
297 | look like, is not smarter than you. If fixing a checkpatch.pl complaint | |
298 | would make the code worse, don't do it. | |
299 | ||
300 | Patches should always be sent as plain text. Please do not send them as | |
301 | attachments; that makes it much harder for reviewers to quote sections of | |
302 | the patch in their replies. Instead, just put the patch directly into your | |
303 | message. | |
304 | ||
305 | When mailing patches, it is important to send copies to anybody who might | |
306 | be interested in it. Unlike some other projects, the kernel encourages | |
307 | people to err on the side of sending too many copies; don't assume that the | |
308 | relevant people will see your posting on the mailing lists. In particular, | |
309 | copies should go to: | |
310 | ||
311 | - The maintainer(s) of the affected subsystem(s). As described earlier, | |
312 | the MAINTAINERS file is the first place to look for these people. | |
313 | ||
314 | - Other developers who have been working in the same area - especially | |
315 | those who might be working there now. Using git to see who else has | |
316 | modified the files you are working on can be helpful. | |
317 | ||
318 | - If you are responding to a bug report or a feature request, copy the | |
319 | original poster as well. | |
320 | ||
321 | - Send a copy to the relevant mailing list, or, if nothing else applies, | |
322 | the linux-kernel list. | |
323 | ||
324 | - If you are fixing a bug, think about whether the fix should go into the | |
2eb7f204 JP |
325 | next stable update. If so, [email protected] should get a copy of |
326 | the patch. Also add a "Cc: [email protected]" to the tags within | |
327 | the patch itself; that will cause the stable team to get a notification | |
328 | when your fix goes into the mainline. | |
75b02146 JC |
329 | |
330 | When selecting recipients for a patch, it is good to have an idea of who | |
331 | you think will eventually accept the patch and get it merged. While it | |
332 | is possible to send patches directly to Linus Torvalds and have him merge | |
333 | them, things are not normally done that way. Linus is busy, and there are | |
334 | subsystem maintainers who watch over specific parts of the kernel. Usually | |
335 | you will be wanting that maintainer to merge your patches. If there is no | |
336 | obvious maintainer, Andrew Morton is often the patch target of last resort. | |
337 | ||
338 | Patches need good subject lines. The canonical format for a patch line is | |
339 | something like: | |
340 | ||
f7c9fe4b MCC |
341 | :: |
342 | ||
75b02146 JC |
343 | [PATCH nn/mm] subsys: one-line description of the patch |
344 | ||
345 | where "nn" is the ordinal number of the patch, "mm" is the total number of | |
346 | patches in the series, and "subsys" is the name of the affected subsystem. | |
5c050fb9 | 347 | Clearly, nn/mm can be omitted for a single, standalone patch. |
75b02146 JC |
348 | |
349 | If you have a significant series of patches, it is customary to send an | |
350 | introductory description as part zero. This convention is not universally | |
351 | followed though; if you use it, remember that information in the | |
352 | introduction does not make it into the kernel changelogs. So please ensure | |
353 | that the patches, themselves, have complete changelog information. | |
354 | ||
355 | In general, the second and following parts of a multi-part patch should be | |
356 | sent as a reply to the first part so that they all thread together at the | |
357 | receiving end. Tools like git and quilt have commands to mail out a set of | |
358 | patches with the proper threading. If you have a long series, though, and | |
5c050fb9 | 359 | are using git, please stay away from the --chain-reply-to option to avoid |
75b02146 | 360 | creating exceptionally deep nesting. |