Commit | Line | Data |
---|---|---|
55d729e4 GS |
1 | =head1 Name |
2 | ||
3 | patching.pod - Appropriate format for patches to the perl source tree | |
4 | ||
f4dad39e | 5 | =head2 Where to get this document |
55d729e4 GS |
6 | |
7 | The latest version of this document is available from | |
f4dad39e | 8 | http://perrin.dimensional.com/perl/perlpatch.html |
55d729e4 GS |
9 | |
10 | =head2 How to contribute to this document | |
11 | ||
12 | You may mail corrections, additions, and suggestions to me | |
f556e4ac | 13 | at dgris@dimensional.com but the preferred method would be |
55d729e4 GS |
14 | to follow the instructions set forth in this document and |
15 | submit a patch 8-). | |
16 | ||
17 | =head1 Description | |
18 | ||
19 | =head2 Why this document exists | |
20 | ||
21 | As an open source project Perl relies on patches and contributions from | |
22 | its users to continue functioning properly and to root out the inevitable | |
23 | bugs. But, some users are unsure as to the I<right> way to prepare a patch | |
24 | and end up submitting seriously malformed patches. This makes it very | |
25 | difficult for the current maintainer to integrate said patches into their | |
26 | distribution. This document sets out usage guidelines for patches in an | |
27 | attempt to make everybody's life easier. | |
28 | ||
29 | =head2 Common problems | |
30 | ||
31 | The most common problems appear to be patches being mangled by certain | |
32 | mailers (I won't name names, but most of these seem to be originating on | |
54aff467 | 33 | boxes running a certain popular commercial operating system). Other problems |
55d729e4 GS |
34 | include patches not rooted in the appropriate place in the directory structure, |
35 | and patches not produced using standard utilities (such as diff). | |
36 | ||
37 | =head1 Proper Patch Guidelines | |
38 | ||
f556e4ac DG |
39 | =head2 What to patch |
40 | ||
41 | Generally speaking you should patch the latest development release | |
42 | of perl. The maintainers of the individual branches will see to it | |
43 | that patches are picked up and applied as appropriate. | |
44 | ||
55d729e4 GS |
45 | =head2 How to prepare your patch |
46 | ||
47 | =over 4 | |
48 | ||
49 | =item Creating your patch | |
50 | ||
51 | First, back up the original files. This can't be stressed enough, | |
52 | back everything up _first_. | |
53 | ||
54 | Also, please create patches against a clean distribution of the perl source. | |
54aff467 | 55 | This ensures that everyone else can apply your patch without clobbering their |
55d729e4 GS |
56 | source tree. |
57 | ||
58 | =item diff | |
59 | ||
60 | While individual tastes vary (and are not the point here) patches should | |
61 | be created using either C<-u> or C<-c> arguments to diff. These produce, | |
62 | respectively, unified diffs (where the changed line appears immediately next | |
63 | to the original) and context diffs (where several lines surrounding the changes | |
64 | are included). See the manpage for diff for more details. | |
65 | ||
54aff467 GS |
66 | The preferred method for creating a unified diff suitable for feeding |
67 | to the patch program is: | |
55d729e4 | 68 | |
54aff467 | 69 | diff -u old-file new-file > patch-file |
55d729e4 | 70 | |
54aff467 GS |
71 | Note the order of files. See below for how to create a patch from |
72 | two directory trees. | |
55d729e4 | 73 | |
54aff467 GS |
74 | If your patch is for wider consumption, it may be better to create it as |
75 | a context diff as some machines have broken patch utilities that choke on | |
76 | unified diffs. A context diff is made using C<diff -c> rather than | |
77 | C<diff -u>. | |
55d729e4 | 78 | |
9e52009c GS |
79 | GNU diff has many desirable features not provided by most vendor-supplied |
80 | diffs. Some examples using GNU diff: | |
81 | ||
82 | # generate a patch for a newly added file | |
83 | % diff -u /dev/null new/file | |
84 | ||
85 | # generate a patch to remove a file (patch > v2.4 will remove it cleanly) | |
86 | % diff -u old/goner /dev/null | |
87 | ||
88 | # get additions, deletions along with everything else, recursively | |
89 | % diff -ruN olddir newdir | |
90 | ||
91 | # ignore whitespace | |
92 | % diff -bu a/file b/file | |
93 | ||
94 | # show function name in every hunk (safer, more informative) | |
95 | % diff -u -F '^[_a-zA-Z0-9]+ *(' old/file new/file | |
96 | ||
39f9fc43 | 97 | =item Derived Files |
54aff467 GS |
98 | |
99 | Many files in the distribution are derivative--avoid patching them. | |
100 | Patch the originals instead. Most utilities (like perldoc) are in | |
101 | this category, i.e. patch utils/perldoc.PL rather than utils/perldoc. | |
102 | Similarly, don't create patches for files under $src_root/ext from | |
103 | their copies found in $install_root/lib. If you are unsure about the | |
104 | proper location of a file that may have gotten copied while building | |
105 | the source distribution, consult the C<MANIFEST>. | |
55d729e4 GS |
106 | |
107 | =item Filenames | |
108 | ||
109 | The most usual convention when submitting patches for a single file is to make | |
110 | your changes to a copy of the file with the same name as the original. Rename | |
54aff467 GS |
111 | the original file in such a way that it is obvious what is being patched |
112 | ($file.dist or $file.old seem to be popular). | |
113 | ||
114 | If you are submitting patches that affect multiple files then you should | |
115 | backup the entire directory tree (to $source_root.old/ for example). This | |
116 | will allow C<diff -ruN old-dir new-dir> to create all the patches at once. | |
55d729e4 | 117 | |
39f9fc43 A |
118 | =item Directories |
119 | ||
120 | IMPORTANT: Patches should be generated from the source root directory, not | |
121 | from the directory that the patched file resides in. This ensures that the | |
122 | maintainer patches the proper file. | |
123 | ||
124 | For larger patches that are dealing with multiple files or | |
125 | directories, Johan Vromans has written a powerful utility: makepatch. | |
126 | See the JV directory on CPAN for the current version. If you have this | |
127 | program available, it is recommended to create a duplicate of the perl | |
128 | directory tree against which you are intending to provide a patch and | |
129 | let makepatch figure out all the changes you made to your copy of the | |
130 | sources. As perl comes with a MANIFEST file, you need not delete | |
131 | object files and other derivative files from the two directory trees, | |
132 | makepatch is smart about them. | |
133 | ||
134 | Say, you have created a directory perl-5.7.1@8685/ for the perl you | |
135 | are taking as the base and a directory perl-5.7.1@8685-withfoo/ where | |
136 | you have your changes, you would run makepatch as follows: | |
137 | ||
138 | makepatch -oldman perl-5.7.1@8685/MANIFEST \ | |
139 | -newman perl-5.7.1@8685-withfoo/MANIFEST \ | |
140 | -diff "diff -u" \ | |
141 | perl-5.7.1@8685 perl-5.7.1@8685-withfoo | |
142 | ||
54aff467 GS |
143 | =item Try it yourself |
144 | ||
145 | Just to make sure your patch "works", be sure to apply it to the Perl | |
146 | distribution, rebuild everything, and make sure the testsuite runs | |
147 | without incident. | |
55d729e4 GS |
148 | |
149 | =back | |
150 | ||
151 | =head2 What to include in your patch | |
152 | ||
153 | =over 4 | |
154 | ||
155 | =item Description of problem | |
156 | ||
157 | The first thing you should include is a description of the problem that | |
158 | the patch corrects. If it is a code patch (rather than a documentation | |
159 | patch) you should also include a small test case that illustrates the | |
160 | bug. | |
161 | ||
54aff467 | 162 | =item Directions for application |
55d729e4 GS |
163 | |
164 | You should include instructions on how to properly apply your patch. | |
165 | These should include the files affected, any shell scripts or commands | |
166 | that need to be run before or after application of the patch, and | |
167 | the command line necessary for application. | |
168 | ||
169 | =item If you have a code patch | |
170 | ||
171 | If you are submitting a code patch there are several other things that | |
172 | you need to do. | |
173 | ||
174 | =over 4 | |
175 | ||
176 | =item Comments, Comments, Comments | |
177 | ||
178 | Be sure to adequately comment your code. While commenting every | |
179 | line is unnecessary, anything that takes advantage of side effects of | |
180 | operators, that creates changes that will be felt outside of the | |
181 | function being patched, or that others may find confusing should | |
182 | be documented. If you are going to err, it is better to err on the | |
183 | side of adding too many comments than too few. | |
184 | ||
185 | =item Style | |
186 | ||
54aff467 GS |
187 | In general, please follow the particular style of the code you are patching. |
188 | ||
189 | In particular, follow these general guidelines for patching Perl sources: | |
190 | ||
191 | 8-wide tabs (no exceptions!) | |
192 | 4-wide indents for code, 2-wide indents for nested CPP #defines | |
193 | try hard not to exceed 79-columns | |
194 | ANSI C prototypes | |
195 | uncuddled elses and "K&R" style for indenting control constructs | |
196 | no C++ style (//) comments, most C compilers will choke on them | |
197 | mark places that need to be revisited with XXX (and revisit often!) | |
198 | opening brace lines up with "if" when conditional spans multiple | |
199 | lines; should be at end-of-line otherwise | |
200 | in function definitions, name starts in column 0 (return value is on | |
201 | previous line) | |
202 | single space after keywords that are followed by parens, no space | |
203 | between function name and following paren | |
204 | avoid assignments in conditionals, but if they're unavoidable, use | |
205 | extra paren, e.g. "if (a && (b = c)) ..." | |
206 | "return foo;" rather than "return(foo);" | |
207 | "if (!foo) ..." rather than "if (foo == FALSE) ..." etc. | |
208 | ||
55d729e4 GS |
209 | |
210 | =item Testsuite | |
211 | ||
f4dad39e DG |
212 | When submitting a patch you should make every effort to also include |
213 | an addition to perl's regression tests to properly exercise your | |
214 | patch. Your testsuite additions should generally follow these | |
54aff467 | 215 | guidelines (courtesy of Gurusamy Sarathy <gsar@activestate.com>): |
f4dad39e DG |
216 | |
217 | Know what you're testing. Read the docs, and the source. | |
218 | Tend to fail, not succeed. | |
219 | Interpret results strictly. | |
220 | Use unrelated features (this will flush out bizarre interactions). | |
221 | Use non-standard idioms (otherwise you are not testing TIMTOWTDI). | |
f556e4ac DG |
222 | Avoid using hardcoded test numbers whenever possible (the |
223 | EXPECTED/GOT found in t/op/tie.t is much more maintainable, | |
224 | and gives better failure reports). | |
f4dad39e DG |
225 | Give meaningful error messages when a test fails. |
226 | Avoid using qx// and system() unless you are testing for them. If you | |
227 | do use them, make sure that you cover _all_ perl platforms. | |
228 | Unlink any temporary files you create. | |
229 | Promote unforeseen warnings to errors with $SIG{__WARN__}. | |
54aff467 | 230 | Be sure to use the libraries and modules shipped with the version |
f556e4ac | 231 | being tested, not those that were already installed. |
f4dad39e | 232 | Add comments to the code explaining what you are testing for. |
f556e4ac DG |
233 | Make updating the '1..42' string unnecessary. Or make sure that |
234 | you update it. | |
54aff467 GS |
235 | Test _all_ behaviors of a given operator, library, or function: |
236 | - All optional arguments | |
237 | - Return values in various contexts (boolean, scalar, list, lvalue) | |
238 | - Use both global and lexical variables | |
239 | - Don't forget the exceptional, pathological cases. | |
55d729e4 GS |
240 | |
241 | =back | |
242 | ||
243 | =item Test your patch | |
244 | ||
245 | Apply your patch to a clean distribution, compile, and run the | |
246 | regression test suite (you did remember to add one for your | |
247 | patch, didn't you). | |
248 | ||
249 | =back | |
250 | ||
251 | =head2 An example patch creation | |
252 | ||
54aff467 | 253 | This should work for most patches: |
55d729e4 GS |
254 | |
255 | cp MANIFEST MANIFEST.old | |
256 | emacs MANIFEST | |
257 | (make changes) | |
258 | cd .. | |
535aafb8 | 259 | diff -c perl5.7.42/MANIFEST.old perl5.7.42/MANIFEST > mypatch |
55d729e4 | 260 | (testing the patch:) |
535aafb8 PN |
261 | mv perl5.7.42/MANIFEST perl5.7.42/MANIFEST.new |
262 | cp perl5.7.42/MANIFEST.old perl5.7.42/MANIFEST | |
55d729e4 GS |
263 | patch -p < mypatch |
264 | (should succeed) | |
535aafb8 | 265 | diff perl5.7.42/MANIFEST perl5.7.42/MANIFEST.new |
55d729e4 GS |
266 | (should produce no output) |
267 | ||
268 | =head2 Submitting your patch | |
269 | ||
270 | =over 4 | |
271 | ||
272 | =item Mailers | |
273 | ||
274 | Please, please, please (get the point? 8-) don't use a mailer that | |
51077201 NIS |
275 | word wraps your patch. This leaves the patch essentially worthless |
276 | to the maintainers. | |
55d729e4 | 277 | |
51077201 NIS |
278 | Unfortunately many mailers word wrap the main text of messages, but |
279 | luckily you can usually send your patches as email attachments without | |
280 | them getting "helpfully" word wrapped. | |
281 | ||
282 | If you have no choice in mailers and no way to get your hands on | |
283 | a better one, there is, of course, a Perl solution. Just do this: | |
55d729e4 GS |
284 | |
285 | perl -ne 'print pack("u*",$_)' patch > patch.uue | |
286 | ||
287 | and post patch.uue with a note saying to unpack it using | |
288 | ||
289 | perl -ne 'print unpack("u*",$_)' patch.uue > patch | |
290 | ||
291 | =item Subject lines for patches | |
292 | ||
293 | The subject line on your patch should read | |
294 | ||
535aafb8 | 295 | [PATCH 5.x.x AREA] Description |
55d729e4 | 296 | |
54aff467 GS |
297 | where the x's are replaced by the appropriate version number. |
298 | The description should be a very brief but accurate summary of the | |
55d729e4 GS |
299 | problem (don't forget this is an email header). |
300 | ||
54aff467 | 301 | Examples: |
55d729e4 | 302 | |
535aafb8 | 303 | [PATCH 5.6.4 DOC] fix minor typos |
55d729e4 | 304 | |
535aafb8 | 305 | [PATCH 5.7.9 CORE] New warning for foo() when frobbing |
55d729e4 | 306 | |
535aafb8 | 307 | [PATCH 5.7.16 CONFIG] Added support for fribnatz 1.5 |
54aff467 GS |
308 | |
309 | The name of the file being patched makes for a poor subject line if | |
310 | no other descriptive text accompanies it. | |
55d729e4 GS |
311 | |
312 | =item Where to send your patch | |
313 | ||
54aff467 GS |
314 | If your patch is for a specific bug in the Perl core, it should be sent |
315 | using the perlbug utility. Don't forget to describe the problem and the | |
316 | fix adequately. | |
317 | ||
55d729e4 GS |
318 | If it is a patch to a module that you downloaded from CPAN you should |
319 | submit your patch to that module's author. | |
320 | ||
54aff467 GS |
321 | If your patch addresses one of the items described in perltodo.pod, |
322 | please discuss your approach B<before> you make the patch at | |
323 | <perl5-porters@perl.org>. Be sure to browse the archives of past | |
324 | discussions (see perltodo.pod for archive locations). | |
325 | ||
55d729e4 GS |
326 | =back |
327 | ||
328 | =head2 Applying a patch | |
329 | ||
330 | =over 4 | |
331 | ||
332 | =item General notes on applying patches | |
333 | ||
334 | The following are some general notes on applying a patch | |
335 | to your perl distribution. | |
336 | ||
337 | =over 4 | |
338 | ||
339 | =item patch C<-p> | |
340 | ||
54aff467 GS |
341 | It is generally easier to apply patches with the C<-p N> argument to |
342 | patch (where N is the number of path components to skip in the files | |
343 | found in the headers). This helps reconcile differing paths between | |
344 | the machine the patch was created on and the machine on which it is | |
345 | being applied. | |
55d729e4 GS |
346 | |
347 | =item Cut and paste | |
348 | ||
54aff467 | 349 | B<Never> cut and paste a patch into your editor. This usually clobbers |
55d729e4 GS |
350 | the tabs and confuses patch. |
351 | ||
352 | =item Hand editing patches | |
353 | ||
54aff467 GS |
354 | Avoid hand editing patches as this almost always screws up the line |
355 | numbers and offsets in the patch, making it useless. | |
55d729e4 GS |
356 | |
357 | =back | |
358 | ||
359 | =back | |
360 | ||
361 | =head2 Final notes | |
362 | ||
363 | If you follow these guidelines it will make everybody's life a little | |
364 | easier. You'll have the satisfaction of having contributed to perl, | |
365 | others will have an easy time using your work, and it should be easier | |
366 | for the maintainers to coordinate the occasionally large numbers of | |
367 | patches received. | |
368 | ||
f556e4ac DG |
369 | Also, just because you're not a brilliant coder doesn't mean that you |
370 | can't contribute. As valuable as code patches are there is always a | |
371 | need for better documentation (especially considering the general | |
372 | level of joy that most programmers feel when forced to sit down and | |
373 | write docs). If all you do is patch the documentation you have still | |
374 | contributed more than the person who sent in an amazing new feature | |
375 | that no one can use because no one understands the code (what I'm | |
376 | getting at is that documentation is both the hardest part to do | |
377 | (because everyone hates doing it) and the most valuable). | |
378 | ||
379 | Mostly, when contributing patches, imagine that it is B<you> receiving | |
380 | hundreds of patches and that it is B<your> responsibility to integrate | |
381 | them into the source. Obviously you'd want the patches to be as easy | |
382 | to apply as possible. Keep that in mind. 8-) | |
55d729e4 GS |
383 | |
384 | =head1 Last Modified | |
385 | ||
f556e4ac DG |
386 | Last modified 21 January 1999 |
387 | Daniel Grisinger <dgris@dimensional.com> | |
55d729e4 GS |
388 | |
389 | =head1 Author and Copyright Information | |
390 | ||
535aafb8 | 391 | Copyright (c) 1998, 1999 Daniel Grisinger |
55d729e4 GS |
392 | |
393 | Adapted from a posting to perl5-porters by Tim Bunce (Tim.Bunce@ig.co.uk). | |
394 | ||
395 | I'd like to thank the perl5-porters for their suggestions. |