This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Clarify rules for testing patches
On Wed, Nov 28, 2001 at 07:46:52PM +0000, Joseph S. Myers wrote:
> On Wed, 28 Nov 2001, Zack Weinberg wrote:
>
> > Since there seems to be some confusion about exactly what "bootstrap
> > GCC" means, I propose the following patch for contribute.html.
>
> There's also some duplication with cvswrite.html to resolve.
> cvswrite.html should probably point to contribute.html for details of
> testing requirements, while making clear when the committer needs to do
> tests before checkin (if not just checking in their own patch tested
> immediately before) (for example, are they needed for patches being
> checked in for someone without write access; are they needed for a patch
> tested long ago; are they needed if some conflicts between the patch and
> subsequent changes need to be fixed in an obvious way for it to apply?).
What do you think of this? (patches are unreadable so I'm just
attaching both complete files).
zw
==== contribute.html ====
<html>
<head>
<meta name="description" content="Contributing to the GCC project." />
<meta name="keywords"
content="GCC, standards, copyright, patches, contributing" />
<title>Contributing to GCC</title>
</head>
<body>
<h1 align="center">Contributing to GCC</h1>
<p>We strongly encourage contributions of code, bugfixes, new
optimizations, new features, documentation updates, web page
improvements, etc. for GCC</p>
<p>There are certain legal requirements and style issues which all
contributions must meet.</p>
<h2>Coding Standards</h2>
<p>All contributions must conform to the <a
href="http://www.gnu.org/prep/standards_toc.html";>GNU Coding
Standards</a>. There are also some <a
href="codingconventions.html">additional coding conventions for
GCC</a>. Submissions which do not conform to the standards will be
returned with a request to reformat the changes.</p>
<h2>Copyright Assignment</h2>
<p>Before we can accept code contributions from you, we need a
copyright assignment form filled out and filed with the FSF.</p>
<p><a href="http://www.gnu.org/prep/maintain.html#SEC6";>See some
documentation by the FSF</a> for details and contact us (either via
the <a href="mailto:gcc@gcc.gnu.org";>gcc@gcc.gnu.org</a> list or the
GCC maintainer that is taking care of your contributions) to obtain
the relevant forms.</p>
<p>Small changes (less than ten lines, or where there is no other way
to write the change) can be accepted without a copyright assignment
form on file.</p>
<h2><a name="testing">Testing patches</a></h2>
<p>All patches must be thoroughly tested. You will of course have
tested that your change does what you expected it to do: fix a bug,
improve an optimization, add a new feature. You must also perform
regression tests to ensure that your patch does not break anything
else. Patches fall into three categories:</p>
<ul>
<li> If your change is to code that is not in a front-end, or is to the
C front-end, you must perform a complete build of GCC and the
runtime libraries included with it, on at least one target. You
must bootstrap all languages, not just C. You must also run all
of the test suites included with GCC and its runtime libraries.
For a normal native configuration, running <code>make
bootstrap</code> followed by <code>make -k check</code> from the
top level of the GCC tree (<strong>not</strong> the
<code>gcc</code> subdirectory) will accomplish this.</li>
<li> <p>If your change is to a front-end other than the C front-end, or
a runtime library other than <code>libgcc</code>, you need to
verify only that the runtime library for that language still
builds and the tests for that language have not regressed. (Most
languages have tests stored both in the <code>gcc</code>
subdirectory, and in the directory for the runtime library.) You
need not bootstrap, or test other languages, since there is no way
you could have affected them.</p>
<p>Since the Ada front end is written in Ada, if you change it you
must perform a complete bootstrap; however, running other language
test suites is not necessary.</p></li>
<li> Documentation changes do not require a bootstrap, but you must
perform <code>make info</code> and <code>make dvi</code> and
correct any errors. (You may ignore complaints about overfull or
underfull hboxes from <code>make dvi</code>.) Changes to the web
site must <a href="http://validator.w3.org/";>validate</a> as HTML
4.01 Transitional.</li>
</ul>
<p>In all cases you must test exactly the change that you intend to
submit; it's not good enough to test an earlier variant. The tree
where you perform this test should not have any other changes applied
to it, because otherwise you cannot be sure that your patch will work
correctly on its own.</p>
<p>Much of GCC is code used only by some targets, and not all targets
support all languages. If your patch affects code which may not be
used by all targets, make sure that at least one of the targets you
test with uses the code you changed. If you have access to only a few
platforms, you should <a href="simtest-howto.html">use a simulator</a>
to increase your test coverage.</p>
<p>Your patch should add test cases for any new features added or bugs
fixed to the testsuite, if they are not already present. Sometimes
this is not possible; if so, say why in the description of your patch.
When you run the testsuite, run it with the new test cases in place.</p>
<h2>Submitting Patches</h2>
<p>Every patch must have several pieces of information before we can
properly evaluate it.</p>
<ul>
<li> A description of the bug and how your patch fixes this bug. For
new features, a description of the feature and your
implementation. This description needs to be detailed enough
that the people who will review your change can tell what is
going on. Do not assume the issue is obvious.</li>
<li> If the patch adds a new feature, the patch must
also add documentation for that feature to the GCC manual.
Similarly, if it changes documented behavior, the patch must update
the documentation.</li>
<li> A ChangeLog entry as plaintext; see the various ChangeLog files
for format and content. Note that, unlike some other projects,
we do require ChangeLogs also for documentation (i.e., .texi
files). They are not required for updates to the web pages.</li>
<li> A list of systems where GCC was built and the test suites were run.
If there were any regressions from an unpatched tree, you must
include an analysis demonstrating that they are not bugs in your
patch.</li>
<li> The patch itself. If you are accessing the <a href="cvs.html">
CVS repository</a> at gcc.gnu.org, use
"<code>cvs update; cvs diff -c3p</code>"; else, use
"<code>diff -c3p OLD NEW</code>" or
"<code>diff -up OLD NEW</code>".
If your version of diff does not support these options, then get
the latest version of GNU diff.</li>
</ul>
<p>We accept patches as plain text (preferred for the compilers
themselves), MIME attachments (preferred for the web pages),
or as uuencoded gzipped text.</p>
<p>When you have all these pieces, bundle them up in a mail message and
send it to
<a href="mailto:gcc-patches@gcc.gnu.org";>gcc-patches@gcc.gnu.org</a>.
All patches and related discussion should be sent to the gcc-patches
mailinglist. For further information on the GCC CVS repository, see
the <a href="cvs.html">Anonymous read-only CVS access</a> and
<a href="cvswrite.html">Read-write CVS access</a> pages.</p>
<p>Sometimes the maintainers do not notice patches. If you do not
receive a response to a patch that you have submitted within a month
or so, it is a good idea to chase it by sending a follow-up email to
<a href="mailto:gcc-patches@gcc.gnu.org";>gcc-patches@gcc.gnu.org</a>.
Please be sure to include in the follow-up email the URL of the entry
in the mailing list archive of the original submission.</p>
</body>
</html>
==== cvswrite.html ====
<html>
<head>
<meta name="description"
content="Instructions for read-write access to the GCC CVS repository." />
<meta name="keywords"
content="GCC, patches, checking in, CVS, SSH, access policy" />
<title>Read-write CVS access</title>
</head>
<body>
<h1 align="center">Read-write CVS access</h1>
<p>We have read/write access to the CVS repository available for all
our significant developers. Maintainers are also encouraged to edit
the <a href="gnatswrite.html">GNATS database</a>.</p>
<hr />
<h3>Contents</h3>
<ol>
<li><a href="#authenticated">Authenticated access</a></li>
<li><a href="#setup">Setting up your local CVS tree</a></li>
<li><a href="#policies">Write access policies</a></li>
<li><a href="#testing">Testing changes</a></li>
<li><a href="#checkin">Checking in a change</a></li>
<li><a href="#example">Example check-in session</a></li>
<li><a href="#branches">Creating branches</a></li>
</ol>
<hr />
<h2><a name="authenticated">Authenticated access</a></h2>
<p>Authenticated access is provided via the SSH protocol. Please
provide us with your public key which you can generate via the
"ssh-keygen" program. This will store your public key in the file
.ssh/identity.pub in your home directory. Please use <a
href="http://sources.redhat.com/cgi-bin/pdw/ps_form.cgi";>this form</a>
to supply the file and your other details.</p>
<p>Once we have this information we will set up an account on
gcc.gnu.org and inform you by mail. At this point you should be able
to check out a tree with CVS and add yourself to the MAINTAINERS file
to test write access. See <a href="#checkin">Checking in a change</a>
for how to proceed with checking in your changes.</p>
<hr />
<h2><a name="setup">Setting up your local CVS tree</a></h2>
<p>Once you can login to the machine, it's trivial to start using ssh
from your remote machine. Set CVS_RSH in your environment to "ssh".
Then issue the command</p>
<blockquote><code>
cvs -z 9 -d :ext:username@gcc.gnu.org:/cvs/gcc co gcc
</code></blockquote>
<p>where <code>username</code> is your user name at
<code>gcc.gnu.org</code>. This will check out a new CVS tree that you
should be able to work with in the normal fashion, including
committing changes.</p>
<p>It is also possible to convert an existing CVS tree to use ssh by
replacing all the "Root" files in the local tree with the following
contents:</p>
<blockquote><code>
username@gcc.gnu.org:/cvs/gcc
</code></blockquote>
<p>To avoid the nuisance of having to supply your passphrase for each
operation, you may want to use <code>ssh-agent</code>(1) followed by
<code>ssh-add</code>(1) and entering your passphrase once for all.
Either start your session as a child of <code>ssh-agent</code> or run
it as a daemon and set the values of the environment variables
<code>SSH_AUTHENTICATION_SOCKET</code> and <code>SSH_AGENT_PID</code>
in each relevant process to what <code>ssh-agent</code> prints when it
starts. To avoid messages about (lack of) X11 forwarding, put in your
<samp>$HOME/.ssh/config</samp> an entry like:</p>
<blockquote>
<pre>
Host gcc.gnu.org
ForwardX11 no
</pre>
</blockquote>
<h2>Web pages</h2>
<p>When you check in changes to our web pages, they will automatically
be checked out into the web server's data area.</p>
<hr />
<h2><a name="policies">Write access policies</a></h2>
<p>The GCC project grants some developers various levels of write
access to the GCC master sources. CVS doesn't provide fine grained
control over access to the repository; therefore, we depend on each
developer to follow the appropriate policies.</p>
<dl>
<dt>Global write permission.</dt> <dd>A very limited number of
developers have global write permission over the entire
repository.</dd>
<dt>Localized write permission.</dt> <dd>This is for people who have
primary responsibility for ports, front ends, or significant hunks
of code in the compiler. These folks are allowed to make changes in
code they maintain without going through any kind of approval
process.</dd>
<dt>Write after approval.</dt> <dd>This is folks that make regular
contributions, but do not fall into one of the two previous
categories. People with write after approval need to submit their
patches to the list; once the patches have been approved by the
appropriate maintainers the patches may be checked into the GCC
sources.</dd>
</dl>
<p>The list of folks with write access to the repository can be found
in the <code>MAINTAINERS</code> file in the GCC distribution.</p>
<p>Also note that fixes for obvious typos in ChangeLog files, docs,
web pages, comments and similar stuff need not be approved. Just
check in the fix. We don't want to get overly anal about checkin
policies.</p>
<p>When you have checked in a patch exactly as it has been approved,
you do not no need to tell that to people -- including the approver.
People interested in when a particular patch is committed can check
CVS or the <a href="http://gcc.gnu.org/ml/gcc-cvs/";>gcc-cvs</a>
list.</p>
<p>Maintainers are free to import files maintained outside the tree
from their official versions without explicit write approval. Such
files would include <code>config.guess</code> and
<code>config.sub</code>.</p>
<p>Any maintainer with CVS write access may <a href="#branches">create
and use a branch</a> for development, including outside the parts of
the compiler they maintain, provided that changes on the branch have
copyright assignments on file. Merging such developments back to the
mainline still needs approval in the usual way.</p>
<hr />
<h2><a name="testing">Testing changes</a></h2>
<p>All changes must be tested according to the <a
href="contribute.html#testing">instructions for testing patches</a>
before they are checked in. If you wrote the patch yourself, you
should test it yourself, unless there is a reason why someone else
must do it for you (for instance, if you are fixing a problem on a
system you do not have access to). If you are checking in a patch for
someone else, you need only test it if they did not.</p>
<p>You must test exactly the change you intend to check in; it is not
good enough to have tested an earlier variant. (Unless the only
changes from the earlier variant are formatting and comment changes;
if there are <strong>any</strong> changes to the code itself you
should re-bootstrap.) It is a good idea to re-test patches which were
submitted a long time ago before applying them, even if nothing
appears to have changed.</p>
<p>When you post your change to <code>gcc-patches</code>, indicate
what platform you have used for testing.</p>
<p>These rules are designed to ensure that checked-in code does not
contain bugs that prevent other people from continuing to get their
work done. There will always be bugs, but these rules help to
minimize the amount of time where the tree does not build at
all. Repeated failure to adhere to these rules could result in the
revocation of check-in privileges by the Steering Committee.</p>
<hr />
<h2><a name="checkin">Checking in a change</a></h2>
<p>The following is meant to provide a very quick overview of how to
check in a change. It is not meant to be a replacement for the CVS
manual but instead a supplement. The CVS manual is distributed as
part of the CVS sources as a texinfo file. <a
href="http://www.cvshome.org/cyclic/cvs/doc-blandy.html";>
http://www.cvshome.org/cyclic/cvs/doc-blandy.html</a> contains a link
to an reasonably simple introduction to CVS.</p>
<p>In all the commands listed below, you can give an explicit list of
filenames to the cvs command. We recommend you list files explicitly
when performing checkins to avoid accidental checkins of local
code.</p>
<ol>
<li>Sync your sources with the master repository via "<code>cvs
update</code>" before attempting a checkin; this will save you a
little time if someone else has modified that file since the last time
you sync'd your sources. It will also identify any files in your
local tree that you have modified.</li>
<li>Apply the patch to your local tree and update the
<code>ChangeLog</code> file. Use the current date/time for the
<code>ChangeLog</code> entry, not the time that the patch was
submitted.</li>
<li>Make sure to rebuild any generated files that would be affected by
the patch. Make sure to check them in along with the files explicitly
modified by the patch.</li>
<li>We recommend using "<code>cvs diff</code>" after applying a patch
to a local tree. Review the output to make sure that only the changes
you wanted to check in will be checked in. Also check to see if the
copyright dates need to be updated.</li>
<li>Use "<code>cvs commit</code>" to check in the patch. You can
enter the log message via the "<code>-m</code>" argument to commit, or
wait for the editor window to appear and enter the log message in the
editor window.</li>
<li>After exiting the editor, CVS will connect to the GCC cvs server
and check in your changes. When your prompt returns the checkin is
finished. A message will be sent to the "<code>gcc-cvs</code>"
mailing list indicating that a change was made. CVS will provide a
message if an error occurs and it will not check in any files.</li>
</ol>
<hr />
<h2><a name="example">Example check-in session</a></h2>
<p>Here's an actual check-in session for a patch John Carr recently
sent to the GCC list. This was the <code>ChangeLog</code> for his
change:</p>
<blockquote>
<pre>
Sun Feb 8 08:02:13 1998 John Carr <jfc@mit.edu>
* bitmap.c (bitmap_debug_file): HOST_PTR_PRINTF converts a pointer,
not a HOST_WIDE_INT.
* calls.c (expand_call): Change test of expand_inline_function
return value to stop compiler warning.
* genattrtab.c (RTL_HASH): Cast pointer to long, not HOST_WIDE_INT.
</pre>
</blockquote>
<h3>First, I sync my local repository.</h3>
<blockquote>
<pre>
[/law/gcc] cvs update
? libobjc
? gcc/.ada
? gcc/jump.c.SAVE
? gcc/loop.c.SAVE
M MAINTAINERS
M Makefile.in
M gcc/loop.c
M gcc/cp/parse.c
M gcc/objc/Make-lang.in
M gcc/objc/Makefile.in
</pre>
</blockquote>
<p>The question marks indicate files in my local repository that are
not part of the official sources. The "M" indicates files I've
changed locally for some unrelated work -- thus I have to be careful
to avoid checking them in. A "U" would have indicated a file that CVS
updated because my local copy was out of date relative to the master
sources.</p>
<p>The local repository is now up to date.</p>
<h3>Apply the patch to the local source</h3>
<blockquote>
<pre>
[/law/gcc/gcc] patch < ~/Mail/gcc/pendingpatches/42
Hmm... Looks like a new-style context diff to me...
The text leading up to this was:
<i>[ uninteresting text deleted ]</i>
|*** bitmap.c.egcs Sat Dec 20 06:31:11 1997
|--- bitmap.c Sun Feb 8 08:01:32 1998
--------------------------
Patching file bitmap.c using Plan A...
Hunk #1 succeeded at 563.
Hunk #2 succeeded at 573.
Hmm... The next patch looks like a new-style context diff to me...
The text leading up to this was:
--------------------------
|*** calls.c.egcs Sun Feb 8 07:44:02 1998
|--- calls.c Sun Feb 8 08:00:08 1998
--------------------------
Patching file calls.c using Plan A...
Hunk #1 succeeded at 730.
Hmm... The next patch looks like a new-style context diff to me...
The text leading up to this was:
--------------------------
|*** genattrtab.c.egcs Sun Feb 8 07:44:04 1998
|--- genattrtab.c Sun Feb 8 08:05:36 1998
--------------------------
Patching file genattrtab.c using Plan A...
Hunk #1 succeeded at 506.
done
</pre>
</blockquote>
<h3>Add ChangeLog entry by hand</h3>
<p><code>ChangeLog</code> entries should be handled as straight text;
patches against <code>ChangeLog</code>s rarely apply correctly.</p>
<blockquote>
<pre>
[/law/gcc/gcc] vi ChangeLog
</pre>
</blockquote>
<h3>Review changes for correctness</h3>
<p>The patch and its associated <code>ChangeLog</code> entry are in my
local tree; now I run <code>cvs diff</code> on the modified files and
review the output.</p>
<blockquote>
<pre>
[/law/gcc/gcc] cvs diff -c3p bitmap.c calls.c genattrtab.c > /tmp/FOO
</pre>
</blockquote>
<p>Look at <code>/tmp/FOO</code> and verify that only the changes we want
are in the diff file.</p>
<h3>Update Copyrights</h3>
<p>Review the changed files to see if any copyrights need updating, in
this particular case all three needed their copyrights updated.</p>
<blockquote>
<pre>
[/law/gcc/gcc] vi bitmap.c calls.c genattrtab.c
</pre>
</blockquote>
<h3>Commit the changes to the central repository</h3>
<blockquote>
<pre>
[/law/gcc/gcc] cvs commit ChangeLog bitmap.c calls.c genattrtab.c
</pre>
</blockquote>
<p>My editor starts and I enter the log message; the lines starting
with <code>CVS:</code> are automatically added by CVS and will be
automatically removed:</p>
<blockquote>
<pre>
* bitmap.c (bitmap_debug_file): HOST_PTR_PRINTF converts a pointer,
not a HOST_WIDE_INT.
* calls.c (expand_call): Change test of expand_inline_function
return value to stop compiler warning.
* genattrtab.c (RTL_HASH): Cast pointer to long, not HOST_WIDE_INT.
CVS: ----------------------------------------------------------------------
CVS: Enter Log. Lines beginning with `CVS:' are removed automatically
CVS:
CVS: Committing in .
CVS:
CVS: Modified Files:
CVS: ChangeLog bitmap.c calls.c genattrtab.c
CVS: ----------------------------------------------------------------------
</pre>
</blockquote>
<p>Now write & quit from the editor, and CVS will start the actual
checkin process....</p>
<blockquote>
<pre>
Checking in ChangeLog;
/cvs/gcc/./gcc/ChangeLog,v <-- ChangeLog
new revision: 1.746; previous revision: 1.745
done
Checking in bitmap.c;
/cvs/gcc/./gcc/bitmap.c,v <-- bitmap.c
new revision: 1.6; previous revision: 1.5
done
Checking in calls.c;
/cvs/gcc/./gcc/calls.c,v <-- calls.c
new revision: 1.12; previous revision: 1.11
done
Checking in genattrtab.c;
/cvs/gcc/./gcc/genattrtab.c,v <-- genattrtab.c
new revision: 1.15; previous revision: 1.14
done
[/law/gcc/gcc]
</pre>
</blockquote>
<p>Note that only a single CVS log message was used for all the files;
this is the normal and expected behavior. It is not necessary to
perform multiple commits and split up the CVS log entry on a file by
file basis. Note this implies that each checkin is for a single
logical change that may effect multiple files. If you have several
unrelated changes, you should check them in with separate cvs commit
commands.</p>
<p>And that's it!</p>
<hr />
<h2><a name="branches">Creating branches</a></h2>
<p>When creating a branch for development, you should first tag the
branchpoint.</p>
</body>
</html>