Clarify rules for testing patches

Zack Weinberg zack@codesourcery.com
Sun Nov 18 17:45:00 GMT 2001


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



More information about the Gcc-patches mailing list