Revised clarifications for patch-testing instructions

[Zack Weinberg <zack at codesourcery dot com>]

Here's the revised content changes.  H-P, does this address your
concerns about simulator testing?

zw

===================================================================
Index: contribute.html
--- contribute.html	2002/01/03 20:25:54	1.44
+++ contribute.html	2002/01/03 21:03:56
@@ -43,6 +43,76 @@ your request.</p>
 <p>Small changes can be accepted without a copyright assignment form
 on file.</p>
 
+<!-- referenced from cvswrite.html -->
+<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>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.  We encourage you to test changes
+with as many host and target combinations as is practical.  In
+addition to using real hardware, you can 
+<a href="simtest-howto.html">use simulators</a> to increase your test
+coverage.</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 &lt;html&gt; tag,
+then use the "upload file" mode of the validator.</p>
+
+<blockquote><pre>
+&lt;?xml version="1.0" encoding="ISO-8859-1"?&gt;
+&lt;!DOCTYPE html 
+          PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
+          "DTD/xhtml1-transitional.dtd"&gt;
+&lt;html xmlns="http://www.w3.org/1999/xhtml"; xml:lang="en" lang="en"&gt;
+</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>
+
 <h2>Submitting Patches</h2>
 
 <p>Every patch must have several pieces of information before we can
@@ -82,14 +152,8 @@ 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>
===================================================================
Index: cvswrite.html
--- cvswrite.html	2002/01/03 20:25:54	1.45
+++ cvswrite.html	2002/01/03 21:03:56
@@ -105,20 +105,24 @@ 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>
+  <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
-  going through any kind of approval process.</p>
+  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
@@ -156,38 +160,24 @@ mainline still needs approval in the usu
 <hr />
 <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 only need to 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>, state the
+canonical name(s) of the platform(s) you 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
@@ -212,6 +202,14 @@ filenames to the cvs command.  We recomm
 when performing checkins to avoid accidental checkins of local
 code.</p>
 
+<p>We prefer that each CVS checkin be of a complete, single logical
+change, which may affect multiple files.  The log message for that
+checkin should be the complete ChangeLog entry for the change.  This
+makes it easier to correlate changes across files, and minimizes the
+time the repository is inconsistent.  If you have several unrelated
+changes, you should check them in with separate cvs commit
+commands.</p>
+
 <ol>
 <li>Sync your sources with the master repository via "<tt>cvs
 update</tt>" before attempting a checkin; this will save you a little
@@ -421,14 +419,6 @@ 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>