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 02:00:07PM -0800, Zack Weinberg wrote:
> On Wed, Nov 28, 2001 at 09:39:57PM +0000, Joseph S. Myers wrote:
> > On Wed, 28 Nov 2001, Zack Weinberg wrote:
> >
> > > What do you think of this? (patches are unreadable so I'm just
> > > attaching both complete files).
> >
> > What file version are these supposed to be based on? You seem to be
> > reverting a lot of clarifying changes to those files from the past few
> > months.
>
> Oops, I forgot to update my copy of the website first. Will revise
> and resubmit.
Here's the revised edition. I got all the formatting fixes out of the
way first, so these diffs are actually readable.
zw
===================================================================
Index: contribute.html
--- contribute.html 2001/11/28 23:24:24 1.41
+++ contribute.html 2001/11/29 00:00:38
@@ -11,7 +11,7 @@
<p>We strongly encourage contributions of code, bugfixes, new
optimizations, new features, documentation updates, web page
-improvements, etc. for GCC</p>
+improvements, etc. for GCC.</p>
<p>There are certain legal requirements and style issues which all
contributions must meet.</p>
@@ -36,13 +36,79 @@ copyright assignment form filled out and
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. Note that it's a good idea to put
-<a href="mailto:assignments@gnu.org";>assignments@gnu.org</a> in copy with
+the relevant forms. Note that it's a good idea to send <a
+href="mailto:assignments@gnu.org";>assignments@gnu.org</a> a copy of
your request.</p>
<p>Small changes 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. If the test framework
+permits, you should automate these tests and add them to GCC's test
+suite. You must also perform regression tests to ensure that your
+patch does not break anything else.</p>
+
+<p>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.</p>
+
+<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>
+
+<p>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>.)</p>
+
+<p>Changes to the web site must <a
+href="http://validator.w3.org/";>validate</a> as XHTML 1.0
+Transitional. The web pages as checked into CVS do not include
+DOCTYPE declarations; they are automatically added when the web
+server checks out its copy. To validate your changes,
+temporarily insert this header in place of the <html> tag,
+then use the "upload file" mode of the validator.</p>
+
+<blockquote><pre>
+<?xml version="1.0" encoding="ISO-8859-1"?>
+<!DOCTYPE html
+ PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
+ "DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml"; xml:lang="en" lang="en">
+</pre></blockquote>
+
+<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. Include all your new test cases in your
+testsuite run.</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>
+
<h2>Submitting Patches</h2>
<p>Every patch must have several pieces of information before we can
@@ -82,19 +148,13 @@ no longer apply by the time your patch i
<dt>Bootstrapping and testing</dt>
<dd>
-A list of targets where the patch survived a bootstrap of GCC. A
-bootstrap on at least one host is required for any non-trivial change.
-A full GCC testsuite run is also required. You can also <a
-href="simtest-howto.html">use one or more simulators</a> to increase
-your test coverage. With your patch, provide a list of hosts where
-the GCC testsuite was run, including an analysis of any regressions.
-Documentation changes do not require a bootstrap, but must pass
-<code>make info</code> and <code>make dvi</code>.
+A list of targets where the patch survived the testing described
+above.
</dd>
<dt>The patch itself</dt>
<dd>
-If you are accessing the <a href="cvs.html"> CVS repository</a> at
+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
===================================================================
Index: cvswrite.html
--- cvswrite.html 2001/11/28 23:35:19 1.44
+++ cvswrite.html 2001/11/29 00:00:38
@@ -16,11 +16,12 @@ our significant developers. Maintainers
the <a href="gnatswrite.html">GNATS database</a>.</p>
<hr />
-<h3>Contents</h3>
+<h2>Contents</h2>
<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>
@@ -87,8 +88,8 @@ ForwardX11 no
<h2>Web pages</h2>
-<p>Note that when you check in changes to our web pages, these will
-automatically be checked out into the web server's data area.</p>
+<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>
@@ -100,20 +101,25 @@ developer to follow the appropriate poli
<dl>
<dt>Global write permission.</dt>
- <dd><p>A very limited number of developers have global write permission
- over the entire repository.</p></dd>
- <dt>Localized write permission.</dt>
- <dd><p>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.</p>
+ <dd><p>A very limited number of developers have global write
+ permission over the entire repository. They may check in changes to
+ any part of the compiler without approval from anyone else. They
+ may also approve other people's changes to any part of the
+ compiler.</p></dd>
+
+ <dt>Localized write permission.</dt> <dd><p>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 approval from anyone else, and
+ approve other people's changes in those files. They must get
+ approval from the appropriate maintainers for changes elsewhere in
+ the compiler.</p>
<p>Maintainers of a port maintain the files in config/<i>port</i>/,
the configure fragments for the port, documentation for the port and
test cases for features or bugs specific to this port. Port
- maintainers do not have approval rights in other files.</p>
- </dd>
+ maintainers do not have approval rights in other files.</p></dd>
<dt>Write after approval.</dt>
<dd><p>This is folks that make regular contributions, but do not
@@ -149,40 +155,26 @@ copyright assignments on file. Merging
mainline still needs approval in the usual way.</p>
<hr />
-<h2><a name="checkin">Checking in a change</a></h2>
+<h2><a name="testing">Testing changes</a></h2>
-<p>It is expected that before you check in any change you will do the
-following:</p>
+<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>
-<ul>
-<li>If your change is to code that is not in a front-end, or is to the
- C front-end, verify that the compiler bootstraps with your change.
- You must bootstrap all languages, not just C. You must bootstrap
- with exactly the change that you intended to check in; it's not
- good enough to have bootstrapped with 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 recommended that you
- also <a href="simtest-howto.html">use a simulator</a> to increase
- your test coverage.</li>
-
-<li>If your change is to code that is not in a front-end, or is to the
- C front-end, verify that the all of the GCC regression tests
- behave identically before and after your patch. You can do a
- <code>make -k check</code> at the top of the tree to run all of
- the tests.</li>
-
-<li>If your change is to code that is in a front-end, other than the C
- front-end, you need to verify only that the tests for that
- language have not regressed. You need not bootstrap, or test
- other languages, since there is no way you could have affected
- them. If there is a run-time library written in the language
- compiled by your front-end, you should, however, verify that it
- continues to build.</li>
-
-<li>When you post your change to <code>gcc-patches</code>, indicate
- what platform you have used for testing.</li>
-</ul>
+<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
@@ -190,6 +182,9 @@ work done. There will always be bugs, b
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