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