Re: [PATCH lib/File/Find/taint.t] Use Test::More

[perl5.git] / Porting / patching.pod

=head1 Name

patching.pod - Appropriate format for patches to the perl source tree

=head2 Where to get this document

The latest version of this document is available from
     http://perrin.dimensional.com/perl/perlpatch.html

=head2 How to contribute to this document

You may mail corrections, additions, and suggestions to me
at dgris@dimensional.com but the preferred method would be
to follow the instructions set forth in this document and 
submit a patch 8-).

=head1 Description

=head2 Why this document exists

As an open source project Perl relies on patches and contributions from
its users to continue functioning properly and to root out the inevitable
bugs.  But, some users are unsure as to the I<right> way to prepare a patch
and end up submitting seriously malformed patches.  This makes it very
difficult for the current maintainer to integrate said patches into their
distribution.  This document sets out usage guidelines for patches in an
attempt to make everybody's life easier.

=head2 Common problems

The most common problems appear to be patches being mangled by certain
mailers (I won't name names, but most of these seem to be originating on
boxes running a certain popular commercial operating system).  Other problems
include patches not rooted in the appropriate place in the directory structure,
and patches not produced using standard utilities (such as diff).

=head1 Proper Patch Guidelines

=head2 What to patch

Generally speaking you should patch the latest development release
of perl.  The maintainers of the individual branches will see to it
that patches are picked up and applied as appropriate.

=head2 How to prepare your patch

=over 4

=item Creating your patch

First, back up the original files.  This can't be stressed enough,
back everything up _first_.

Also, please create patches against a clean distribution of the perl source.
This ensures that everyone else can apply your patch without clobbering their
source tree.

=item diff

While individual tastes vary (and are not the point here) patches should
be created using either C<-u> or C<-c> arguments to diff.  These produce,
respectively, unified diffs (where the changed line appears immediately next
to the original) and context diffs (where several lines surrounding the changes
are included).  See the manpage for diff for more details.

The preferred method for creating a unified diff suitable for feeding
to the patch program is:

	diff -u old-file new-file > patch-file

Note the order of files.  See below for how to create a patch from
two directory trees.

If your patch is for wider consumption, it may be better to create it as
a context diff as some machines have broken patch utilities that choke on
unified diffs.  A context diff is made using C<diff -c> rather than
C<diff -u>.

GNU diff has many desirable features not provided by most vendor-supplied
diffs.  Some examples using GNU diff:

    # generate a patch for a newly added file
    % diff -u /dev/null new/file
    
    # generate a patch to remove a file (patch > v2.4 will remove it cleanly)
    % diff -u old/goner /dev/null
    
    # get additions, deletions along with everything else, recursively
    % diff -ruN olddir newdir
    
    # ignore whitespace
    % diff -bu a/file b/file
    
    # show function name in every hunk (safer, more informative)
    % diff -u -F '^[_a-zA-Z0-9]+ *(' old/file new/file

=item Derived Files

Many files in the distribution are derivative--avoid patching them.
Patch the originals instead.  Most utilities (like perldoc) are in
this category, i.e. patch utils/perldoc.PL rather than utils/perldoc.
Similarly, don't create patches for files under $src_root/ext from
their copies found in $install_root/lib.  If you are unsure about the
proper location of a file that may have gotten copied while building
the source distribution, consult the C<MANIFEST>.

=item Filenames

The most usual convention when submitting patches for a single file is to make
your changes to a copy of the file with the same name as the original.  Rename
the original file in such a way that it is obvious what is being patched
($file.dist or $file.old seem to be popular).

If you are submitting patches that affect multiple files then you should
backup the entire directory tree (to $source_root.old/ for example).  This
will allow C<diff -ruN old-dir new-dir> to create all the patches at once.

=item Directories

IMPORTANT: Patches should be generated from the source root directory, not
from the directory that the patched file resides in.  This ensures that the
maintainer patches the proper file.

For larger patches that are dealing with multiple files or
directories, Johan Vromans has written a powerful utility: makepatch.
See the JV directory on CPAN for the current version. If you have this
program available, it is recommended to create a duplicate of the perl
directory tree against which you are intending to provide a patch and
let makepatch figure out all the changes you made to your copy of the
sources. As perl comes with a MANIFEST file, you need not delete
object files and other derivative files from the two directory trees,
makepatch is smart about them.

Say, you have created a directory perl-5.7.1@8685/ for the perl you
are taking as the base and a directory perl-5.7.1@8685-withfoo/ where
you have your changes, you would run makepatch as follows:

    makepatch -oldman perl-5.7.1@8685/MANIFEST \
              -newman perl-5.7.1@8685-withfoo/MANIFEST \
              -diff "diff -u" \
              perl-5.7.1@8685 perl-5.7.1@8685-withfoo

=item Try it yourself

Just to make sure your patch "works", be sure to apply it to the Perl
distribution, rebuild everything, and make sure the testsuite runs
without incident.

=back

=head2 What to include in your patch

=over 4

=item Description of problem

The first thing you should include is a description of the problem that
the patch corrects.  If it is a code patch (rather than a documentation
patch) you should also include a small test case that illustrates the
bug.

=item Directions for application

You should include instructions on how to properly apply your patch.
These should include the files affected, any shell scripts or commands
that need to be run before or after application of the patch, and
the command line necessary for application.

=item If you have a code patch

If you are submitting a code patch there are several other things that
you need to do.

=over 4

=item Comments, Comments, Comments

Be sure to adequately comment your code.  While commenting every
line is unnecessary, anything that takes advantage of side effects of
operators, that creates changes that will be felt outside of the
function being patched, or that others may find confusing should
be documented.  If you are going to err, it is better to err on the
side of adding too many comments than too few.

=item Style

In general, please follow the particular style of the code you are patching.

In particular, follow these general guidelines for patching Perl sources:

    8-wide tabs (no exceptions!)
    4-wide indents for code, 2-wide indents for nested CPP #defines
    try hard not to exceed 79-columns
    ANSI C prototypes
    uncuddled elses and "K&R" style for indenting control constructs
    no C++ style (//) comments, most C compilers will choke on them
    mark places that need to be revisited with XXX (and revisit often!)
    opening brace lines up with "if" when conditional spans multiple
        lines; should be at end-of-line otherwise
    in function definitions, name starts in column 0 (return value is on
        previous line)
    single space after keywords that are followed by parens, no space
        between function name and following paren
    avoid assignments in conditionals, but if they're unavoidable, use
        extra paren, e.g. "if (a && (b = c)) ..."
    "return foo;" rather than "return(foo);"
    "if (!foo) ..." rather than "if (foo == FALSE) ..." etc.


=item Testsuite

When submitting a patch you should make every effort to also include
an addition to perl's regression tests to properly exercise your
patch.  Your testsuite additions should generally follow these
guidelines (courtesy of Gurusamy Sarathy <gsar@activestate.com>):

	Know what you're testing.  Read the docs, and the source.
	Tend to fail, not succeed.
	Interpret results strictly.
	Use unrelated features (this will flush out bizarre interactions).
	Use non-standard idioms (otherwise you are not testing TIMTOWTDI).
	Avoid using hardcoded test numbers whenever possible (the 
          EXPECTED/GOT found in t/op/tie.t is much more maintainable, 
          and gives better failure reports).
	Give meaningful error messages when a test fails.
	Avoid using qx// and system() unless you are testing for them.  If you
	  do use them, make sure that you cover _all_ perl platforms.
	Unlink any temporary files you create.
	Promote unforeseen warnings to errors with $SIG{__WARN__}.
	Be sure to use the libraries and modules shipped with the version 
          being tested, not those that were already installed.
	Add comments to the code explaining what you are testing for.
	Make updating the '1..42' string unnecessary.  Or make sure that 
          you update it.
	Test _all_ behaviors of a given operator, library, or function:
	  - All optional arguments
	  - Return values in various contexts (boolean, scalar, list, lvalue)
	  - Use both global and lexical variables
	  - Don't forget the exceptional, pathological cases.

=back

=item Test your patch

Apply your patch to a clean distribution, compile, and run the
regression test suite (you did remember to add one for your
patch, didn't you).

=back

=head2 An example patch creation

This should work for most patches:

      cp MANIFEST MANIFEST.old
      emacs MANIFEST
      (make changes)
      cd ..
      diff -c perl5.7.42/MANIFEST.old perl5.7.42/MANIFEST > mypatch
      (testing the patch:)
      mv perl5.7.42/MANIFEST perl5.7.42/MANIFEST.new
      cp perl5.7.42/MANIFEST.old perl5.7.42/MANIFEST
      patch -p < mypatch
      (should succeed)
      diff perl5.7.42/MANIFEST perl5.7.42/MANIFEST.new
      (should produce no output)

=head2 Submitting your patch

=over 4

=item Mailers

Please, please, please (get the point? 8-) don't use a mailer that
word wraps your patch or that MIME encodes it.  Both of these leave
the patch essentially worthless to the maintainer.

If you have no choice in mailers and no way to get your hands on a
better one there is, of course, a perl solution.  Just do this:

      perl -ne 'print pack("u*",$_)' patch > patch.uue

and post patch.uue with a note saying to unpack it using

      perl -ne 'print unpack("u*",$_)' patch.uue > patch

=item Subject lines for patches

The subject line on your patch should read

    [PATCH 5.x.x AREA] Description

where the x's are replaced by the appropriate version number.
The description should be a very brief but accurate summary of the
problem (don't forget this is an email header).

Examples:

    [PATCH 5.6.4 DOC] fix minor typos

    [PATCH 5.7.9 CORE] New warning for foo() when frobbing

    [PATCH 5.7.16 CONFIG] Added support for fribnatz 1.5

The name of the file being patched makes for a poor subject line if
no other descriptive text accompanies it.

=item Where to send your patch

If your patch is for a specific bug in the Perl core, it should be sent
using the perlbug utility.  Don't forget to describe the problem and the
fix adequately.

If it is a patch to a module that you downloaded from CPAN you should
submit your patch to that module's author.

If your patch addresses one of the items described in perltodo.pod,
please discuss your approach B<before> you make the patch at
<perl5-porters@perl.org>.  Be sure to browse the archives of past
discussions (see perltodo.pod for archive locations).

=back

=head2 Applying a patch

=over 4

=item General notes on applying patches

The following are some general notes on applying a patch
to your perl distribution.

=over 4

=item patch C<-p>

It is generally easier to apply patches with the C<-p N> argument to
patch (where N is the number of path components to skip in the files
found in the headers).  This helps reconcile differing paths between
the machine the patch was created on and the machine on which it is
being applied.

=item Cut and paste

B<Never> cut and paste a patch into your editor.  This usually clobbers
the tabs and confuses patch.

=item Hand editing patches

Avoid hand editing patches as this almost always screws up the line
numbers and offsets in the patch, making it useless.

=back

=back

=head2 Final notes

If you follow these guidelines it will make everybody's life a little
easier.  You'll have the satisfaction of having contributed to perl,
others will have an easy time using your work, and it should be easier
for the maintainers to coordinate the occasionally large numbers of 
patches received.

Also, just because you're not a brilliant coder doesn't mean that you
can't contribute.  As valuable as code patches are there is always a
need for better documentation (especially considering the general
level of joy that most programmers feel when forced to sit down and
write docs).  If all you do is patch the documentation you have still
contributed more than the person who sent in an amazing new feature
that no one can use because no one understands the code (what I'm
getting at is that documentation is both the hardest part to do
(because everyone hates doing it) and the most valuable).

Mostly, when contributing patches, imagine that it is B<you> receiving
hundreds of patches and that it is B<your> responsibility to integrate
them into the source.  Obviously you'd want the patches to be as easy
to apply as possible.  Keep that in mind.  8-)

=head1 Last Modified

Last modified 21 January 1999 
Daniel Grisinger <dgris@dimensional.com>

=head1 Author and Copyright Information

Copyright (c) 1998, 1999 Daniel Grisinger

Adapted from a posting to perl5-porters by Tim Bunce (Tim.Bunce@ig.co.uk).

I'd like to thank the perl5-porters for their suggestions.