You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2010/01/19 22:21:02 UTC
svn commit: r900962 [2/3] - in /subversion/site:
publish/docs/hacking/index.html publish/docs/hacking/release-process.html
publish/docs/hacking/translating.html site-map.txt
Modified: subversion/site/publish/docs/hacking/index.html
URL: http://svn.apache.org/viewvc/subversion/site/publish/docs/hacking/index.html?rev=900962&r1=900961&r2=900962&view=diff
==============================================================================
--- subversion/site/publish/docs/hacking/index.html (original)
+++ subversion/site/publish/docs/hacking/index.html Tue Jan 19 21:21:01 2010
@@ -1,9 +1,28 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
-"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+ "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Hacker's Guide to Subversion</title>
+<style type="text/css" media="screen">
+body {
+ font-family: serif;
+ font-size: 12pt;
+}
+tt, pre {
+ font-size: 11pt;
+}
+.h1 {
+ border-top: 1px black solid;
+}
+.h2, .h3, .h4 {
+ margin-left: 0.5in;
+}
+#toc, #toc ol {
+ list-style: none;
+}
+
+</style>
</head>
<body>
@@ -13,11 +32,6 @@
<p>If you are contributing code to the Subversion project, please read
this first.</p>
-<pre>
-$LastChangedDate$
-</pre>
-
-
<!-- Other pages seem to use "h2" for ToC, but I think "h1" works
better, because the ToC is fundamentally different from other
sections and therefore it's confusing when it looks the same as
@@ -25,55 +39,109 @@
<div class="h1"><!-- no 'id' or 'title' attribute for ToC -->
<h1>Table of Contents</h1>
-<ul>
-<li><a href="#participating">Participating in the community</a></li>
-<li><a href="#docs">Theory and documentation</a></li>
-<li><a href="#code-to-read">Code to read</a></li>
-<li><a href="#directory-layout">Directory layout</a></li>
-<li><a href="#interface-visibility">Code modularity and interface visibility</a></li>
-<li><a href="#coding-style">Coding style</a></li>
-<li><a href="#secure-coding">Secure coding guidelines</a></li>
-<li><a href="#destruction-of-stacked-resources">Destruction of stacked resources</a></li>
-<li><a href="#documenting">Documentation</a></li>
-<li><a href="#use-page-breaks">Using page breaks</a></li>
-<li><a href="#error-messages">Error message conventions</a></li>
-<li><a href="#other-conventions">Other conventions</a></li>
-<li><a href="#apr-pools">APR pool usage conventions</a></li>
-<li><a href="#apr-status-codes">APR status codes</a></li>
-<li><a href="#exception-handling">Exception handling</a></li>
-<li><a href="#automated-tests">Automated tests</a></li>
-<li><a href="#write-test-cases-first">Writing test cases before code</a></li>
-<li><a href="#server-debugging">Debugging the server</a></li>
-<li><a href="#net-trace">Tracing network traffic</a></li>
-<li><a href="#tracing-memory-leaks">Tracking down memory leaks</a></li>
-<li><a href="#log-messages">Writing log messages</a></li>
-<li><a href="#crediting">Crediting</a></li>
-<li><a href="#patches">Patch submission guidelines</a></li>
-<li><a href="#filing-issues">Filing bugs / issues</a></li>
-<li><a href="#issue-triage">Issue triage</a></li>
-<li><a href="#commit-access">Commit access</a></li>
-<li><a href="#branch-based-development">Branch-based development</a></li>
-<li><a href="#configury">The configuration/build system under unix</a></li>
-<li><a href="#releasing">How to release a distribution tarball</a></li>
-<li><a href="#release-numbering">Release numbering, compatibility, and deprecation</a></li>
-<li><a href="#release-stabilization">Stabilizing and maintaining releases</a></li>
-<li><a href="#tarball-signing">Signing source distribution packages</a></li>
-<li><a href="#custom-releases">Custom releases</a></li>
-<li><a href="#l10n">Localization (l10n)</a></li>
-</ul>
+<ol id="toc">
+ <li><a href="#general">General Overview</a>
+ <ol>
+ <li><a href="#participating">Participating in the community</a></li>
+ <li><a href="#docs">Theory and documentation</a></li>
+ <li><a href="#code-to-read">Code to read</a></li>
+ <li><a href="#directory-layout">Directory layout</a></li>
+ <li><a href="#branch-policy">Branch policy</a></li>
+ <li><a href="#documenting">Documentation</a></li>
+ <li><a href="#patches">Patch submission guidelines</a></li>
+ </ol>
+ </li>
+ <li><a href="#roles">Community Roles</a>
+ <ol>
+ <li><a href="#committers">Committers</a></li>
+ <li><a href="#release-manager">Release Manager</a></li>
+ <li><a href="#patch-manager">Patch Manager</a></li>
+ </ol>
+ </li>
+ <li><a href="#conventions">Coding and Commit Conventions</a>
+ <ol>
+ <li><a href="#interface-visibility">Code modularity and interface visibility</a></li>
+ <li><a href="#coding-style">Coding style</a></li>
+ <li><a href="#use-page-breaks">Using page breaks</a></li>
+ <li><a href="#error-messages">Error message conventions</a></li>
+ <li><a href="#apr-pools">APR pool usage conventions</a></li>
+ <li><a href="#apr-status-codes">APR status codes</a></li>
+ <li><a href="#exception-handling">Exception handling</a></li>
+ <li><a href="#secure-coding">Secure coding guidelines</a></li>
+ <li><a href="#destruction-of-stacked-resources">Destruction of stacked resources</a></li>
+ <li><a href="#other-conventions">Other coding conventions</a></li>
+ <li><a href="#log-messages">Writing log messages</a></li>
+ <li><a href="#crediting">Crediting</a></li>
+ </ol>
+ </li>
+ <li><a href="#building-and-testing">Building and Testing</a>
+ <ol>
+ <li><a href="#configury">The configuration/build system under unix</a></li>
+ <li><a href="#automated-tests">Automated tests</a></li>
+ <li><a href="#write-test-cases-first">Writing test cases before code</a></li>
+ </ol>
+ </li>
+ <li><a href="#debugging">Debugging Subversion</a>
+ <ol>
+ <li><a href="#server-debugging">Debugging the server</a></li>
+ <li><a href="#net-trace">Tracing network traffic</a></li>
+ <li><a href="#tracing-memory-leaks">Tracking down memory leaks</a></li>
+ </ol>
+ </li>
+ <li><a href="#issues">Bugs / Issues</a>
+ <ol>
+ <li><a href="#reporting-bugs">How to report a bug</a></li>
+ <li><a href="#bugs-where">Where to report a bug</a></li>
+ <li><a href="#issue-triage">Issue triage</a></li>
+ </ol>
+ </li>
+ <li><a href="#releasing">Making Subversion Releases</a>
+ <ol>
+ <li><a href="#release-numbering">Release numbering, compatibility, and deprecation</a></li>
+ <li><a href="#release-stabilization">Stabilizing and maintaining releases</a></li>
+ <li><a href="#tarball-signing">Signing source distribution packages (a.k.a tarballs)</a></li>
+ <li><a href="#custom-releases">Custom releases</a></li>
+ <li><a href="#release-branches">Creating and maintaining release branches</a></li>
+ <li><a href="#porting-changes">Porting changes to release branches</a></li>
+ <li><a href="#the-changes-file">Managing the <tt>CHANGES file</tt></a></li>
+ <li><a href="#before-release">Preparing to roll a release</a></li>
+ <li><a href="#rolling-release">Rolling a release</a></li>
+ <li><a href="#releasing-release">The actual releasing</a></li>
+ <li><a href="#afterwards">After a release has been made</a></li>
+ <li><a href="#releasing-not">How <em>not</em> to make a Subversion release</a></li>
+ </ol>
+ </li>
+ <li><a href="#l10n">Localization (l10n)</a>
+ <ol>
+ <li><a href="#version-requirements">Software version requirements</a></li>
+ <li><a href="#new-translation">Starting a new translation</a></li>
+ <li><a href="#verifying">Verifying your po file</a></li>
+ <li><a href="#submitting">Submitting your po file</a></li>
+ <li><a href="#updating">Updating existing po files</a></li>
+ <li><a href="#branch-maintenance">Maintenance on branches</a></li>
+ <li><a href="#po-mo-requirements">Requirements for po and mo files</a></li>
+ <li><a href="#empty-string-conventions">Conventions for the empty string msgid section</a></li>
+ <li><a href="#translation-teams">Translation teams</a></li>
+ <li><a href="#quotes">Single versus double quotes</a></li>
+ <li><a href="#translating-error-messages">Error message conventions</a></li>
+ </ol>
+ </li>
+</ol>
</div>
+<div class="h1" id="general" title="general">
+<h1>General Overview</h1>
<div class="h2" id="participating" title="participating">
<h2>Participating in the community</h2>
-<p>Although Subversion is originally sponsored and hosted by CollabNet
-(<a href="http://www.collab.net">http://www.collab.net</a>), it's a
-true open-source project under the Apache License, Version 2.0. A number of
-developers work for CollabNet, some work for other large companies
-(such as RedHat), and many others are simply excellent volunteers who
-are interested in building a better version control system.</p>
+<p>Although Subversion was originally sponsored and hosted by
+CollabNet (<a href="http://www.collab.net">http://www.collab.net</a>),
+it's a true open-source project under the Apache License, Version 2.0.
+A number of Subversion's developers are paid by their employers to
+improve Subversion, while many others are simply excellent volunteers
+who are interested in building a better version control system.</p>
<p>The community exists mainly through mailing lists and a Subversion
repository. To participate:</p>
@@ -132,7 +200,7 @@
>http://subversion.tigris.org/servlets/ProjectMailingListList</a>
in the description for the svn-breakage list.</p>
-</div>
+</div> <!-- participating -->
<div class="h2" id="docs" title="docs">
@@ -199,7 +267,7 @@
</ol>
-</div>
+</div> <!-- docs -->
<div class="h2" id="code-to-read" title="code-to-read">
@@ -257,7 +325,7 @@
questions about the security implications of a change, you are urged
to ask for review on the developer mailing list.</p>
-</div>
+</div> <!-- code-to-read -->
<div class="h2" id="directory-layout" title="directory-layout">
@@ -336,228 +404,156 @@
</li>
</ul>
-</div>
-
-<div class="h2" id="interface-visibility" title="interface-visibility">
-<h2>Code modularity and interface visibility</h2>
+</div> <!-- directory-layout -->
-<p>Subversion's code and headers files are segregated along a couple
-of key lines: library-specific vs. inter-library; public vs. private.
-This separation is done primarily because of our focus on proper
-modularity and code organization, but also because of our promises as
-providers and maintainers of a widely adopted public API. As you
-write new functions in Subversion, you'll need to carefully consider
-these things, asking some questions of yourself as you go along:</p>
-<p><em>"Are the consumers of my new code local to a particular source
-code file in a single library?"</em> If so, you probably want a static
-function in that same source file.</p>
+<div class="h2" id="branch-policy" title="branch-policy">
+<h2>Branching policy</h2>
-<p><em>"Is my new function of the sort that other source code within
-this library will need to use, but nothing *outside* the library?"</em>
-If so, you want to use a non-static, double-underscore-named function
-(such as <tt>svn_foo__do_something</tt>), with its prototype in the
-appropriate library-specific header file.</p>
+<p>The Subversion project strongly prefers that active development
+happen in the common trunk. Changes made to trunk have the highest
+visibility and get the greatest amount of exercise that can be
+expected from unreleased code. For this to be beneficial to everyone,
+though, our trunk is expected at all times to be stable. It should
+build. It should work. It might not be release-ready, but it should
+certainly be test-suite ready.</p>
+
+<p>We also strongly prefer to see large changes broken up into
+several, smaller, logical commits — each of which is expected
+to meet the aforementioned requirements of stability.</p>
+
+<p>That said, we understand that it can be nearly impossible to apply
+all these policies to particularly large changes (new features,
+sweeping code reorganizations, etc.). It is in those situations that
+you might consider using a custom branch dedicated to your development
+task. The following are some guidelines to make your branch-based
+development work go smoothly.</p>
-<p><em>"Will my code need to be accessed from a different
-library?"</em> Here you have some additional questions to answer, such
-as <em>"Should my code live in the original library I was going to put
-it in, or should it live in a more generic utility library such as
-libsvn_subr?"</em> Either way, you're now looking at using an
-inter-library header file. But see the next question before you decide
-which one...</p>
+<div class="h3" id="branch-creation-and-management" title="branch-creation-and-management">
+<h3>Branch creation and management</h3>
-<p><em>"Is my code such that it has a clean, maintainable API that can
-reasonably be maintained in perpetuity and brings value to the
-Subversion public API offering?"</em> If so, you'll be adding
-prototypes to the public API, immediately
-inside <tt>subversion/include/</tt>. If not, double-check your plans
--- maybe you haven't chosen the best way to abstract your
-functionality. But sometimes it just happens that libraries need to
-share some function that is arguably of no use to other software
-besides Subversion itself. In those cases, use the private header
-files in <tt>subversion/include/private/</tt>.</p>
+<p>There's nothing particularly complex about branch-based
+development. You make a branch from the trunk (or from whatever
+branch best serves as both source and destination for your work), and
+you do your work on it. Subversion's merge tracking feature has
+greatly helped to reduce the sort of mental overhead required to work
+in this way, so making good use of that feature (by using Subversion
+1.5 or newer clients, and by performing all merges to and from the
+roots of branches) is highly encouraged.</p>
-</div>
+</div> <!-- branch-creation-and-management -->
-<div class="h2" id="coding-style" title="coding-style">
-<h2>Coding style</h2>
+<div class="h3" id="lightweight-branches" title="lightweight-branches">
+<h3>Lightweight branches</h3>
-<p>Subversion uses ANSI C, and follows the GNU coding standards,
-except that we do not put a space between the name of a function and
-the opening parenthesis of its parameter list. Emacs users can just
-load svn-dev.el to get the right indentation behavior (most source
-files here will load it automatically, if `enable-local-eval' is set
-appropriately).</p>
+<p>If you're working on a feature or bugfix in stages involving
+multiple commits, and some of the intermediate stages aren't stable
+enough to go on trunk, then create a temporary branch in /branches.
+There's no need to ask — just do it. It's fine to try
+out experimental ideas in a temporary branch, too. And all the
+preceding applies to partial as well as full committers.</p>
-<p>Read <a href="http://www.gnu.org/prep/standards.html"
->http://www.gnu.org/prep/standards.html</a> for a full description of
-the GNU coding standards. Below is a short example demonstrating the
-most important formatting guidelines, including our
-no-space-before-param-list-paren exception:</p>
+<p>If you're just using the branch to "checkpoint" your code, and
+don't feel it's ready for review, please put some sort of notice at
+the top of the log message, such as:</p>
<pre>
- char * /* func type on own line */
- argblarg(char *arg1, int arg2) /* func name on own line */
- { /* first brace on own line */
- if ((some_very_long_condition && arg2) /* indent 2 cols */
- || remaining_condition) /* new line before operator */
- { /* brace on own line, indent 2 */
- arg1 = some_func(arg1, arg2); /* NO SPACE BEFORE PAREN */
- } /* close brace on own line */
- else
- {
- do /* format do-while like this */
- {
- arg1 = another_func(arg1);
- }
- while (*arg1);
- }
- }
+ *** checkpoint commit -- please don't waste your time reviewing it ***
</pre>
-<p>In general, be generous with parentheses even when you're sure
-about the operator precedence, and be willing to add spaces and
-newlines to avoid "code crunch". Don't worry too much about vertical
-density; it's more important to make code readable than to fit that
-extra line on the screen.</p>
+<p>And if a later commit on that branch <em>should</em> be reviewed,
+then please supply, in the log message, the appropriate 'svn diff'
+command, since the diff would likely involve two non-adjacent commits
+on that branch, and reviewers shouldn't have to spend time figuring
+out which ones they are.</p>
-</div>
+<p>When you're done with the branch — when you've
+either merged it to trunk or given up on it — please
+remember to remove it.</p>
+<p>See also the <a href="#partial-commit-access" >section on partial
+commit access</a> for our policy on offering commit access to
+experimental branches.</p>
-<div class="h2" id="secure-coding" title="secure-coding">
-<h2>Secure coding guidelines</h2>
+</div> <!-- lightweight-branches -->
-<p>Just like almost any other programming language, C has undesirable
-features which enables an attacker to make your program fail in
-predictable ways, often to the attacker's benefit. The goal of these
-guidelines is to make you aware of the pitfalls of C as they apply to
-the Subversion project. You are encouraged to keep these pitfalls in
-mind when reviewing code of your peers, as even the most skilled and
-paranoid programmers make occasional mistakes.</p>
+<div class="h3" id="branch-readme-files" title="branch-readme-files">
+<h3>BRANCH-README files</h3>
-<p>Input validation is the act of defining legal input and rejecting
-everything else. The code must perform input validation on all
-untrusted input. </p>
+<p>For branches you expect to be longer-lived, we recommend the
+creation and regular updating of a file in the root of your branch
+named <tt>BRANCH-README</tt>. Such a file provides you with a great,
+central place to describe the following aspects of your branch:</p>
-<p>Security boundaries:</p>
+<ul>
-<p>A security boundary in the Subversion server code must be
-identified as such as this enables auditors to quickly determine the
-quality of the boundary. Security boundaries exist where the running
-code has access to information the user does not or where the code
-runs with privileges above those of the user making the
-request. Typical examples of such is code that does access control or
-an application with the SUID bit set.</p>
+<li><p>The basic purpose of your branch: what bug it exists to fix, or
+ feature to implement; what issue number(s) it relates to; what
+ list discussion threads surround it; what design docs exists to
+ describe the situation.</p></li>
-<p>Functions which make calls to a security boundary must include
-validation checks of the arguments passed. Functions which themselves
-are security boundaries should audit the input received and alarm when
-invoked with improper values. </p>
+<li><p>What style of branch management you are using: is this a
+ reintegrate-able branch that will regularly be kept in sync
+ with its source branch and ultimately merged back to that
+ source branch using <tt>svn merge --reintegrate</tt>? Or is it
+ not reintegrate-able, managed in total disregard to new changes
+ made to the source branch, and expected to be merged back to
+ that source without the <tt>--reintegrate</tt> option <tt>svn
+ merge</tt>?</p></li>
-<p>[### todo: need some examples from Subversion here...]</p>
+<li><p>What tasks remain for you to accomplish on your branch? Are
+ those tasks claimed by someone? Do they need more design
+ input? How can others help you?</p></li>
-<p>String operations:</p>
+</ul>
-<p>Use the string functions provided in apr_strings.h instead of
-standard C library functions that write to strings. The APR functions
-are safer because they do bounds-checking and dest allocation
-automatically. Although there may be circumstances where it's
-theoretically safe to use plain C string functions (such as when you
-already know the lengths of the source and dest), please use the APR
-functions anyway, so the code is less brittle and more reviewable.</p>
+<p>Here is an example BRANCH-README file that demonstrates what we're
+talking about:</p>
-<p>Password storage:</p>
+<pre style="margin-left: 0.25in;">
+This branch exists for the resolution of issue #8810, per the ideas
+documented in /trunk/notes/frobnobbing-feature.txt. It is being
+managed as a reintegrate-able branch, with regular catch-up merges
+from the trunk.
-<p>Help users keep their passwords secret: When the client reads or
-writes password locally, it should ensure that the file is mode
-0600. If the file is readable by other users, the client should exit
-with a message that tells the user to change the filemode due to the
-risk of exposure.</p>
+TODO:
-</div>
+ * compose regression tests [DONE]
+ * add frob identification logic [STARTED (fitz)]
+ * add nobbing bits []
+</pre>
+<p>Why all the fuss? Because this project idealizes communication and
+collaboration, understanding that the latter is more likely to happen
+when the former is a point of emphasis.</p>
-<div class="h2" id="destruction-of-stacked-resources"
- title="destruction-of-stacked-resources">
-<h2>Destruction of stacked resources</h2>
+<p>Just remember when you merge your branch back to its source to
+delete the <tt>BRANCH-README</tt> file.</p>
-<p>Some resources need destruction to ensure correct functioning of the
-application. Such resources include files, especially since open
-files cannot be deleted on Windows.</p>
+</div> <!-- branch-readme-files -->
-<p>When writing an API which creates and returns a stream, in the
-background this stream may be stacked on a file or other stream. To
-ensure correct destruction of the resources the stream is built upon,
-it must correctly call the destructors of the stream(s) it is built
-upon (owns).</p>
+</div> <!-- branch-policy -->
-<p>At first in <a href="http://svn.haxx.se/dev/archive-2005-12/0487.shtml">
-http://svn.haxx.se/dev/archive-2005-12/0487.shtml</a>
-and later in <a href="http://svn.haxx.se/dev/archive-2005-12/0633.shtml">
-http://svn.haxx.se/dev/archive-2005-12/0633.shtml</a> this
-was discussed in more general terms for files, streams, editors and
-window handlers.</p>
-<p>As Greg Hudson put it:</p>
+<div class="h2" id="documenting" title="documenting">
+<h2>Documentation</h2>
-<blockquote>
-<p>On consideration, here is where I would like us to be:</p>
+<div class="h3" id="document-everything" title="document-everything">
+<h3>Document Everything</h3>
-<ul><li>Streams which read from or write to an underlying object own that
-object, i.e. closing the stream closes the underlying object, if
-applicable.</li>
+<p>Every function, whether public or internal, must start out with a
+documentation comment that describes what the function does. The
+documentation should mention every parameter received by the function,
+every possible return value, and (if not obvious) the conditions under
+which the function could return an error.</p>
-<li>The layer (function or data type) which created a stream is
-responsible for closing it, except when the above rule applies.</li>
+<p>For internal documentation put the parameter names in upper case
+in the doc string, even when they are not upper case in the actual
+declaration, so that they stand out to human readers.</p>
-<li>Window handlers are thought of as an odd kind of stream, and passing
-the final NULL window is considered closing the stream.</li>
-</ul>
-
-<p>If you think of apply_textdelta as creating a window handler, then I
-don't think we're too far off. svn_stream_from_aprfile isn't owning its
-subsidiary file, svn_txdelta_apply is erroneously taking responsibility
-for closing the window stream it is passed, and there may be some other
-deviations.</p>
-</blockquote>
-
-<p>There is one exception to the rules above though. When a stream is passed
-to a function as an argument (for example: the 'out' parameter of
-svn_client_cat2()), that routine can't call the streams destructor, since
-it did not create that resource.</p>
-
-<p>If svn_client_cat2() creates a stream, it must also call the destructor
-for that stream. By the above model, that stream will call the destructor
-for the 'out' parameter. This is however wrong, because the responsibility
-to destruct the 'out' parameter lies elsewhere.</p>
-
-<p>To solve this problem, at least in the stream case, svn_stream_disown()
-has been introduced. This function wraps a stream, making sure it's
-<em>not</em> destroyed, even though any streams stacked upon it may try
-to do so.</p>
-
-</div>
-
-
-<div class="h2" id="documenting" title="documenting">
-<h2>Documentation</h2>
-
-<div class="h3" id="document-everything" title="document-everything">
-<h3>Document Everything</h3>
-<p>Every function, whether public or internal, must start out with a
-documentation comment that describes what the function does. The
-documentation should mention every parameter received by the function,
-every possible return value, and (if not obvious) the conditions under
-which the function could return an error.</p>
-
-<p>For internal documentation put the parameter names in upper case
-in the doc string, even when they are not upper case in the actual
-declaration, so that they stand out to human readers.</p>
-
-<p>For public or semi-public API functions, the doc string should go
-above the function in the .h file where it is declared; otherwise, it
-goes above the function definition in the .c file.</p>
+<p>For public or semi-public API functions, the doc string should go
+above the function in the .h file where it is declared; otherwise, it
+goes above the function definition in the .c file.</p>
<p>For structure types, document each individual member of the structure as
well as the structure itself.</p>
@@ -616,10 +612,11 @@
subversion/include/*.h</a> for doxygen examples.
</p>
-</div>
+</div> <!-- document-everything -->
<div class="h3" id="doxygen-docs" title="doxygen-docs">
<h3>Public API Documentation</h3>
+
<p>We use the <a href="http://www.doxygen.org/">Doxygen</a> format for
public interface documentation. This means anything that goes in a
public header file. <a href="http://svn.collab.net/svn-doxygen/">Snapshots
@@ -648,1054 +645,1240 @@
<p>See the <a href="http://www.stack.nl/~dimitri/doxygen/manual.html">Doxygen
manual</a> for a complete list of commands.</p>
-</div>
+</div> <!-- doxygen-docs -->
-</div>
+</div> <!-- documenting -->
+<div class="h2" id="patches" title="patches">
+<h2>Patch submission guidelines</h2>
-<div class="h2" id="use-page-breaks" title="use-page-breaks">
-<h2>Using page breaks</h2>
+<p>Mail patches to dev@subversion.tigris.org, starting the subject
+line with <tt>[PATCH]</tt>. This helps our patch manager spot patches
+right away. For example:</p>
-<p>We're using page breaks (the Ctrl-L character, ASCII 12) for
-section boundaries in both code and plaintext prose files. Each
-section starts with a page break, and immediately after the page break
-comes the title of the section.</p>
+<pre>
+ Subject: [PATCH] fix for rev printing bug in svn status
+</pre>
-<p>This helps out people who use the Emacs page commands, such as
-`pages-directory' and `narrow-to-page'. Such people are not as scarce
-as you might think, and if you'd like to become one of them, then add
-(require 'page-ext) to your .emacs and type C-x C-p C-h sometime.</p>
+<p>If the patch addresses a particular issue, include the issue number
+as well: "<tt>[PATCH] issue #1729: ...</tt>". Developers
+who are interested in that particular issue will know to read the
+mail.</p>
-</div>
+<p>A patch submission should contain one logical change; please don't
+mix N unrelated changes in one submission — send N
+separate emails instead.</p>
+<p>Generate the patch using <tt>svn diff</tt> from the top of a
+Subversion trunk working copy. If the file you're diffing is not
+under revision control, you can achieve the same effect by using
+<tt>diff -u</tt>.</p>
-<div class="h2" id="error-messages" title="error-messages">
-<h2>Error message conventions</h2>
+<p>Please include a log message with your patch. A good log message
+helps potential reviewers understand the changes in your patch, and
+increases the likelihood that it will be applied. You can put the log
+message in the body of the email, or at the top of the patch
+attachment (see below). Either way, it should follow the guidelines
+given in <a href="#log-messages">Writing log messages</a>, and be
+enclosed in triple square brackets, like so:</p>
-<p>For error messages the following conventions apply:</p>
+<pre>
+ [[[
+ Fix issue #1729: Don't crash because of a missing file.
-<ul>
+ * subversion/libsvn_ra_ansible/get_editor.c
+ (frobnicate_file): Check that file exists before frobnicating.
+ ]]]
+</pre>
-<li><p>Provide specific error messages only when there is information
- to add to the general error message found in
- subversion/include/svn_error_codes.h.</p></li>
+<p>(The brackets are not actually part of the log message, they're
+just a way to clearly mark off the log message from its surrounding
+context.)</p>
-<li><p>Messages start with a capital letter.</p></li>
+<p>If possible, send the patch as an attachment with a mime-type of
+<code>text/x-diff</code>, <code>text/x-patch</code>, or <code>text/plain</code>.
+Most people's mailreaders can display those inline, and having the patch as an
+attachment allows them to extract the patch from the message conveniently.
+Never send patches in archived or compressed form (e.g., tar, gzip, zip, bzip2),
+because that prevents people from reviewing the patch directly in
+their mailreaders.</p>
-<li><p>Try keeping messages below 70 characters.</p></li>
+<p>If you can't attach the patch with one of these mime-types, or if
+the patch is very short, then it's okay to include it directly in the
+body of your message. But watch out: some mail editors munge inline
+patches by inserting unasked-for line breaks in the middle of long
+lines. If you think your mail software might do this, then please use
+an attachment instead.</p>
-<li><p>Don't end the error message with a period (".").</p></li>
+<p>If the patch implements a new feature, make sure to describe the
+feature completely in your mail; if the patch fixes a bug, describe
+the bug in detail and give a reproduction recipe. An exception to
+these guidelines is when the patch addresses a specific issue in the
+issues database — in that case, just refer to the
+issue number in your log message, as described
+in <a href="#log-messages">Writing log messages</a>.</p>
-<li><p>Don't include newline characters in error messages.</p></li>
+<p>It is normal for patches to undergo several rounds of feedback and
+change before being applied. Don't be discouraged if your patch is
+not accepted immediately — it doesn't mean you goofed,
+it just means that there are a <em>lot</em> of eyes looking at every
+code submission, and it's a rare patch that doesn't have at least a
+little room for improvement. After reading people's responses to your
+patch, make the appropriate changes and resubmit, wait for the next
+round of feedback, and lather, rinse, repeat, until some committer
+applies it.</p>
-<li><p>Quoting information is done using single quotes (e.g. "'some info'").</p></li>
+<p>If you don't get a response for a while, and don't see the patch
+applied, it may just mean that people are really busy. Go ahead and
+repost, and don't hesitate to point out that you're still waiting for
+a response. One way to think of it is that patch management is highly
+parallizable, and we need you to shoulder your share of the management
+as well as the coding. Every patch needs someone to shepherd it
+through the process, and the person best qualified to do that is the
+original submitter.</p>
-<li><p>Don't include the name of the function where the error occurs
- in the error message. If Subversion is compiled using the
- '--enable-maintainer-mode' configure-flag, it will provide this
- information by itself.</p></li>
+</div> <!-- patches -->
-<li><p>When including path or filenames in the error string, be sure
- to quote them (e.g. "Can't find '/path/to/repos/userfile'").</p></li>
+</div> <!-- general -->
-<li><p>When including path or filenames in the error string, be sure
- to convert them using <a
- href="http://svn.collab.net/svn-doxygen/svn__path_8h.html#a1"
- ><tt>svn_path_local_style()</tt></a> before inclusion (since
- paths passed to and from Subversion APIs are assumed to be
- in <a href="http://svn.collab.net/svn-doxygen/svn__path_8h.html#a0"
- >canonical form</a>).</p></li>
-<li><p>Don't use Subversion-specific abbreviations (e.g. use "repository"
- instead of "repo", "working copy" instead of "wc").</p></li>
+<div class="h1" id="roles" title="roles">
+<h1>Community Roles</h1>
-<li><p>If you want to add an explanation to the error, report it
- followed by a colon and the explanation like this:</p>
- <pre>
- "Invalid " SVN_PROP_EXTERNALS " property on '%s': "
- "target involves '.' or '..'".
- </pre></li>
+<div class="h2" id="committers" title="committers">
+<h2>Committers</h2>
-<li><p>Suggestions or other additions can be added after a semi-colon,
- like this:</p>
- <pre>
- "Can't write to '%s': object of same name already exists; remove "
- "before retrying".
- </pre></li>
+<p>Committers in the Subversion project are those folks to whom the
+right to directly commit changes to our version controlled resources
+has been granted. The project is meritocratic, which means (among
+other things) that project governance is handled by those who do the
+work. There are two types of commit access: full and partial. Full
+means anywhere in the tree, partial means only in that committer's
+specific area(s) of expertise. While every contribution is valued
+regardless of its source, not every person who contributes code to
+Subversion will earn commit access.</p>
-<li><p>Try to stay within the boundaries of these conventions, so please avoid
- separating different parts of error messages by other separators such
- as '--' and others.</p></li>
+<p>The <a href="http://svn.collab.net/repos/svn/trunk/COMMITTERS"
+>COMMITTERS</a> file lists all committers, both full and partial, and
+says the domains for each partial committer.</p>
-</ul>
+<div class="h3" id="full-commit-access" title="full-commit-access">
+<h3>How full commit access is granted</h3>
-<p>Also read about <a href="#l10n">Localization</a>.</p>
+<p>After someone has successfully contributed a few non-trivial
+patches, some full committer, usually whoever has reviewed and applied
+the most patches from that contributor, proposes them for commit
+access. This proposal is sent only to the other full committers --
+the ensuing discussion is private, so that everyone can feel
+comfortable speaking their minds. Assuming there are no objections,
+the contributor is granted commit access. The decision is made by
+consensus; there are no formal rules governing the procedure, though
+generally if someone strongly objects the access is not offered, or is
+offered on a provisional basis.</p>
-</div>
+<p><i>The primary criterion for full commit access is good
+judgment.</i></p>
+<p>You do not have to be a technical wizard, or demonstrate deep
+knowledge of the entire codebase, to become a full committer. You
+just need to know what you don't know. If your patches adhere to the
+guidelines in this file, adhere to all the usual unquantifiable rules
+of coding (code should be readable, robust, maintainable, etc.), and
+respect the Hippocratic Principle of "first, do no harm", then you
+will probably get commit access pretty quickly. The size, complexity,
+and quantity of your patches do not matter as much as the degree of
+care you show in avoiding bugs and minimizing unnecessary impact on
+the rest of the code. Many full committers are people who have not
+made major code contributions, but rather lots of small, clean fixes,
+each of which was an unambiguous improvement to the code. (Of course,
+this does not mean the project needs a bunch of very trivial patches
+whose only purpose is to gain commit access; knowing what's worth a
+patch post and what's not is part of showing good judgement :-) .)</p>
-<div class="h2" id="other-conventions" title="other-conventions">
-<h2>Other conventions</h2>
+<p>To assist developers in discovering new committers, we record
+patches and other contributions in a <a href="#crediting">special
+crediting format</a>, which is then parsed to produce a
+browser-friendly <a
+href="http://www.red-bean.com/svnproject/contribulyzer/">contribution
+list</a>, updated nightly. If you're thinking of proposing someone
+for commit access and want to look over all their changes, that <a
+href="http://www.red-bean.com/svnproject/contribulyzer/">contribution
+list</a> might be the most convenient place to do it.</p>
-<p>In addition to the GNU standards, Subversion uses these
-conventions:</p>
+</div> <!-- full-commit-access -->
-<ul>
-<li><p>When using a path or file name as input to most <a
- href="http://svn.collab.net/svn-doxygen/">Subversion APIs</a>, be
- sure to convert them to Subversion's internal/canonical form
- using the <a href="http://svn.collab.net/svn-doxygen/svn__path_8h.html#a0"
- ><tt>svn_path_internal_style()</tt></a> API. Alternately, when
- receiving a path or file name as output from a Subversion API,
- convert them into the expected form for your platform using the
- <a href="http://svn.collab.net/svn-doxygen/svn__path_8h.html#a1"
- ><tt>svn_path_local_style()</tt></a> API.</p></li>
+<div class="h3" id="partial-commit-access" title="partial-commit-access">
+<h3>How partial commit access is granted</h3>
-<li><p>Use only spaces for indenting code, never tabs. Tab display
- width is not standardized enough, and anyway it's easier to
- manually adjust indentation that uses spaces.</p>
-</li>
+<p>A full committer sponsors the partial committer. Usually this
+means the full committer has applied several patches to the same area
+from the proposed partial committer, and realizes things would be
+easier if the person were just committing directly. Approval is not
+required from the full committers; it is assumed that sponsors know
+what they're doing and will watch the partial committer's first few
+commits to make sure everything's going smoothly.</p>
-<li><p>Restrict lines to 79 columns, so that code will display well in a
- minimal standard display window. (There can be exceptions, such
- as when declaring a block of 80-column text with a few extra
- columns taken up by indentation, quotes, etc., if splitting each
- line in two would be unreasonably messy.)</p>
-</li>
+<p>Patches submitted by a partial committer may be committed by that
+committer even if they are outside that person's domain. This
+requires approval (often expressed as a +1 vote) from at least one
+full committer. In such a case, the approval should be noted in the
+log message, like so:</p>
-<li><p>All published functions, variables, and structures must be signified
- with the corresponding library name - such as libsvn_wc's
- svn_wc_adm_open. All library-internal declarations made in a
- library-private header file (such as libsvn_wc/wc.h) must be signified
- by two underscores after the library prefix (such as
- svn_wc__ensure_directory). All declarations private to a single file
- (such as the static function get_entry_url inside of
- <tt>libsvn_wc/update_editor.c</tt>) do not require any
- additional namespace decorations. Symbols that need to be used
- outside a library, but still are not public are put in a shared
- header file in the <tt>include/private/</tt> directory, and use
- the double underscore notation. Such symbols may be used by
- Subversion core code only.</p>
-
- <p>To recap:</p>
- <pre>
- /* Part of published API: subversion/include/svn_wc.h */
- svn_wc_adm_open()
- #define SVN_WC_ADM_DIR_NAME ...
- typedef enum svn_wc_schedule_t ...
+<pre>
+ Approved by: lundblad
+</pre>
- /* For use within one library only: subversion/libsvn_wc/wc.h */
- svn_wc__ensure_directory()
- #define SVN_WC__BASE_EXT ...
- typedef struct svn_wc__compat_notify_baton_t ...
+<p>Any full committer may offer anyone commit access to an
+experimental branch at any time. It is not necessary that the
+experimental branch have a high likelihood of being merged to trunk
+(although that's always a good goal to aim for). It's just as
+important that the full committer — all the full
+committers, actually — view such branches as training
+grounds for new developers, by giving feedback on the commits. The
+goal of these branches is both to get new code into Subversion and to
+get new developers into the project. See also the <a
+href="#lightweight-branches" >section on lightweight branches</a>, and
+this mail:</p>
- /* For use within one file: subversion/libsvn_wc/update_editor.c */
- get_entry_url()
- struct handler_baton {
+<pre>
+ <a href="http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=132746"
+>http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=132746</a>
+ From: Karl Fogel <kfogel@red-bean.com>
+ To: dev@subversion.tigris.org
+ Subject: branch liberalization (was: Elego tree conflicts work)
+ Date: Tue, 20 Nov 2007 10:49:38 -0800
+ Message-Id: <87y7cswy4d.fsf@red-bean.com>
+</pre>
- /* For internal use in svn core code only:
- subversion/include/private/svn_wc_private.h */
- svn_wc__entry_versioned()
- </pre>
+</div> <!-- partial-commit-access -->
- <p>Pre-Subversion 1.5, private symbols which needed to be used
- outside of a library were put into public header files,
- using the double underscore notation. This practice has been
- abandoned, and any such symbols are legacy, maintained for <a
- href="#release-numbering">backwards compatibility</a>.</p>
-</li>
+<div class="h3" id="contrib-area" title="contrib-area">
+<h3>The contrib/ area</h3>
-<li><p>In text strings that might be printed out (or otherwise made
- available) to users, use only forward quotes around paths and
- other quotable things. For example:</p>
- <pre>
- $ svn revert foo
- svn: warning: svn_wc_is_wc_root: 'foo' is not a versioned resource
- $
- </pre>
+<p>When a tool is accepted into the <i>contrib/</i> area, we
+automatically offer its author partial commit access to maintain the
+tool there. Any full committer can sponsor this. Usually no
+discussion or vote is necessary, though if there are objections then
+the usual decision-making procedures apply (attempt to reach consensus
+first, then vote among the full committers if consensus cannot be
+reached).</p>
- <p>There used to be a lot of strings that used a backtick for
- the first quote (`foo' instead of 'foo'), but that looked bad in
- some fonts, and also messed up some people's auto-highlighting,
- so we settled on the convention of always using forward
- quotes.</p>
-</li>
+<p>Code under contrib/ must be open source, but need not have the same
+license or copyright holder as Subversion itself.</p>
-<li><p>If you use Emacs, put something like this in your .emacs file,
- so you get svn-dev.el and svnbook.el when needed:</p>
- <pre>
- ;;; Begin Subversion development section
- (defun my-find-file-hook ()
- (let ((svn-tree-path (expand-file-name "~/projects/subversion"))
- (book-tree-path (expand-file-name "~/projects/svnbook")))
- (cond
- ((string-match svn-tree-path buffer-file-name)
- (load (concat svn-tree-path "/tools/dev/svn-dev")))
- ((string-match book-tree-path buffer-file-name)
- ;; Handle load exception for svnbook.el, because it tries to
- ;; load psgml, and not everyone has that available.
- (condition-case nil
- (load (concat book-tree-path "/src/tools/svnbook"))
- (error
- (message "(Ignored problem loading svnbook.el.)")))))))
+</div> <!-- contrib-area -->
- (add-hook 'find-file-hooks 'my-find-file-hook)
- ;;; End Subversion development section
- </pre>
+<div class="h3" id="obvious-fix" title="obvious-fix">
+<h3>The "obvious fix" rule</h3>
- <p>You'll need to customize the path for your setup, of course.
- You can also make the regexp to string-match more selective; for
- example, one developer says:</p>
- <pre>
- > Here's the regexp I'm using:
- >
- > "src/svn/[^/]*/\\(subversion\\|tools\\|build\\)/"
- >
- > Two things to notice there: (1) I sometimes have several
- > working copies checked out under ...src/svn, and I want the
- > regexp to match all of them; (2) I want the hook to catch only
- > in "our" directories within the working copy, so I match
- > "subversion", "tools" and "build" explicitly; I don't want to
- > use GNU style in the APR that's checked out into my repo. :-)
- </pre>
-</li>
+<p>Any committer, whether full or partial, may commit fixes for
+obvious typos, grammar mistakes, and formatting problems wherever they
+may be — in the web pages, API documentation, code
+comments, commit messages, etc. We rely on the committer's judgement
+to determine what is "obvious"; if you're not sure, just ask.</p>
-<li><p>We have a tradition of not marking files with the names of
- individual authors (i.e., we don't put lines like
- "Author: foo" or "@author foo" in a special position
- at the top of a source file). This is to discourage
- territoriality — even when a file has only one
- author, we want to make sure others feel free to make changes.
- People might be unnecessarily hesitant if someone appears to
- have staked a personal claim to the file.</p>
-</li>
+<p>Whenever you invoke the "obvious fix" rule, please say so in
+the <a href="#log-messages">log message</a> of your commit. For example:</p>
-<li><p>Put two spaces between the end of one sentence and the start of
- the next. This helps readability, and allows people to use
- their editors' sentence-motion and -manipulation commands.</p>
-</li>
+<pre>
+------------------------------------------------------------------------
+r32135 | stylesen | 2008-07-16 10:04:25 +0200 (Wed, 16 Jul 2008) | 8 lines
-<li><p>There are many other unspoken conventions maintained throughout
- the code, that are only noticed when someone unintentionally
- fails to follow them. Just try to have a sensitive eye for the
- way things are done, and when in doubt, ask.</p>
-</li>
-</ul>
+Update "check-license.py" so that it can generate license text applicable
+to this year.
-</div>
+Obvious fix.
+* tools/dev/check-license.py
+ (NEW_LICENSE): s/2005/2008/
-<div class="h2" id="apr-pools" title="apr-pools">
-<h2>APR pool usage conventions</h2>
+------------------------------------------------------------------------
+</pre>
-<p>(This assumes you already basically understand how APR pools work;
-see apr_pools.h for details.)</p>
+</div> <!-- obvious-fix -->
-<p>Applications using the Subversion libraries must call
-apr_initialize() before calling any Subversion functions.</p>
+</div> <!-- committers -->
-<p>Subversion's general pool usage strategy can be summed up in two
-principles:</p>
-<ol>
-<li><p>The call level that created a pool is the only place to clear or
- destroy that pool.</p>
-</li>
-<li><p>When iterating an unbounded number of times, create a subpool
- before entering the iteration, use it inside the loop and clear
- it at the start of each iteration, then destroy it after the loop
- is done, like so:</p>
- <pre>
- apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+<div class="h2" id="release-manager" title="release-manager">
+<h2>Release Manager</h2>
- for (i = 0; i < n; ++i)
- {
- svn_pool_clear(iterpool);
- do_operation(..., iterpool);
- }
+<p>The role of the Release Manager in the Subversion project is to
+handle the process of getting code stabilized, packaged and released
+to the general public. If we were building planes, the RM would be
+the guy looking at the construction checklists, painting the airline
+logo on the fuselage, and delivering the finished unit to the
+customer.</p>
- svn_pool_destroy(iterpool);
- </pre>
-</li>
-</ol>
+<p>As such, there is no real development associated with being an RM.
+All the work you have to do is non-coding: coordinating people,
+centralizing information, and being the public voice announcing new
+stable releases. A lot of the tasks that the RM has to do are
+repetitive, and not automated either because nobody has broken down
+and written the tools yet, or because the tasks require human
+validation that makes automation a little superfluous.</p>
-<p>Supporting the above rules, we use the following pool names as conventions
-to represent various pool lifetimes:</p>
-<ul>
-<li><p><code>result_pool</code> - The pool in which the output of a function
-should be allocated. A result pool declaration should <strong>always</strong>
-be found in a function argument list, and never inside a local block. (But
-not all functions need or have result pools.)
-</p></li>
+<p>You may be thinking at this stage that the RM's duty is
+unglamorous, and you are kinda right. If you are looking for a
+position within the project that will bring fame and fortune, you're
+better off implementing stuff that really needs to be done on trunk.
+If you're looking for something that really helps people who don't
+care about releases focus on code, then RMing is for you.</p>
-<li><p><code>scratch_pool</code> - The pool in which all function-local data
-should be allocated. This pool is also provided by the caller, who may
-choose to clear this pool immediately when control returns to it.
-</p></li>
+<p>So, you are the Release Manager. What do you need to do? This is
+what the rest of this document is about.</p>
-<li><p><code>iterpool</code> - An <em>iteration</em> pool, used inside loops,
-as per the above example.</p></li>
-</ul>
+</div> <!-- release-manager -->
-<p>(Note: Some legacy code uses a single <code>pool</code> function argument,
-which operates as both the result and scratch pools.)</p>
-<p>By using an iterpool for loop-bounded data, you ensure O(1) instead
-of O(N) memory leakage should the function return abruptly from
-within the loop (say, due to error). That's why you shouldn't make a
-subpool for data which persists throughout a function, but instead
-should use the pool passed in by the caller. That memory will be
-reclaimed when the caller's pool is cleared or destroyed. If the
-caller is invoking the callee in a loop, then trust the caller to take
-care of clearing the pool on each iteration. The same logic
-propagates all the way up the call stack.</p>
+<div class="h2" id="patch-manager" title="patch-manager">
+<h2>Patch Manager</h2>
-<p>The pool you use also helps readers of the code understand object
-lifetimes. Is a given object used only during one iteration of the
-loop, or will it need to last beyond the end of the loop? For
-example, pool choices indicate a lot about what's going on in this
-code:</p>
+<p>Subversion usually has a Patch Manager, whose job is to watch the
+dev@ mailing list and make sure that no patches "slip through the
+cracks".</p>
-<pre>
- apr_hash_t *persistent_objects = apr_hash_make(result_pool);
- apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+<p>This means watching every thread containing "[PATCH]" mails, and
+taking appropriate action based on the progress of the thread. If the
+thread resolves on its own (because the patch gets committed, or
+because there is consensus that the patch doesn't need to be applied,
+or whatever) then no further action need be taken. But if the thread
+fades out without any clear decision, then the patch needs to be saved
+in the issue tracker. This means that a summary of any discussion
+threads around that patch, and links to relevant mailing list
+archives, will be added to some issue in the tracker. For a patch
+which addresses an existing issue tracker item, the patch is saved to
+that item. Otherwise, a new issue of type 'PATCH' is filed, and the
+patch is saved to that new issue.</p>
- for (i = 0; i < n; ++i)
- {
- const char *intermediate_result;
- const char *key, *val;
-
- svn_pool_clear(iterpool);
- SVN_ERR(do_something(&intermediate_result, ..., iterpool));
- SVN_ERR(get_result(intermediate_result, &key, &val, ...,
- result_pool));
- apr_hash_set(persistent_objects, key, APR_HASH_KEY_STRING, val);
- }
+<p>The Patch Manager needs a basic technical understanding of
+Subversion, and the ability to skim a thread and get a rough
+understanding of whether consensus has been reached, and if so, of
+what kind. It does <em>not</em> require actual Subversion development
+experience or commit access. Expertise in using one's mail reading
+software is optional, but recommended :-).</p>
- svn_pool_destroy(iterpool);
- return persistent_objects;
-</pre>
+<p>The current patch manager is Gavin 'Beau' Baumanis
+<gavin@thespidernet.com>.</p>
-<p>Except for some legacy code, which was written before these
-principles were fully understood, virtually all pool usage in
-Subversion follows the above guidelines.</p>
+</div> <!-- patch-manager -->
-<p>One such legacy pattern is a tendency to allocate an object inside
-a pool, store the pool in the object, and then free that pool (either
-directly or through a close_foo() function) to destroy the object.</p>
+</div> <!-- roles -->
-<p>For example:</p>
-<pre>
- <span style="color: red;">/*** Example of how NOT to use pools. Don't be like this. ***/</span>
+<div class="h1" id="conventions" title="conventions">
+<h1>Coding and Commit Conventions</h1>
- static foo_t *
- make_foo_object(arg1, arg2, apr_pool_t *pool)
- {
- apr_pool_t *subpool = svn_pool_create(pool);
- foo_t *foo = apr_palloc(subpool, sizeof(*foo));
+<div class="h2" id="interface-visibility" title="interface-visibility">
+<h2>Code modularity and interface visibility</h2>
- foo->field1 = arg1;
- foo->field2 = arg2;
- foo->pool = subpool;
- }
+<p>Subversion's code and headers files are segregated along a couple
+of key lines: library-specific vs. inter-library; public vs. private.
+This separation is done primarily because of our focus on proper
+modularity and code organization, but also because of our promises as
+providers and maintainers of a widely adopted public API. As you
+write new functions in Subversion, you'll need to carefully consider
+these things, asking some questions of yourself as you go along:</p>
- [...]
+<p><em>"Are the consumers of my new code local to a particular source
+code file in a single library?"</em> If so, you probably want a static
+function in that same source file.</p>
- [Now some function calls make_foo_object() and returns, passing
- back a new foo object.]
+<p><em>"Is my new function of the sort that other source code within
+this library will need to use, but nothing *outside* the library?"</em>
+If so, you want to use a non-static, double-underscore-named function
+(such as <tt>svn_foo__do_something</tt>), with its prototype in the
+appropriate library-specific header file.</p>
- [...]
+<p><em>"Will my code need to be accessed from a different
+library?"</em> Here you have some additional questions to answer, such
+as <em>"Should my code live in the original library I was going to put
+it in, or should it live in a more generic utility library such as
+libsvn_subr?"</em> Either way, you're now looking at using an
+inter-library header file. But see the next question before you decide
+which one...</p>
- [Now someone, at some random call level, decides that the foo's
- lifetime is over, and calls svn_pool_destroy(foo->pool).]
-</pre>
+<p><em>"Is my code such that it has a clean, maintainable API that can
+reasonably be maintained in perpetuity and brings value to the
+Subversion public API offering?"</em> If so, you'll be adding
+prototypes to the public API, immediately
+inside <tt>subversion/include/</tt>. If not, double-check your plans
+-- maybe you haven't chosen the best way to abstract your
+functionality. But sometimes it just happens that libraries need to
+share some function that is arguably of no use to other software
+besides Subversion itself. In those cases, use the private header
+files in <tt>subversion/include/private/</tt>.</p>
-<p>This is tempting, but it defeats the point of using pools, which is
-to not worry so much about individual allocations, but rather about
-overall performance and lifetime groups. Instead, foo_t generally
-should not have a `pool' field. Just allocate as many foo objects as
-you need in the current pool — when that pool gets
-cleared or destroyed, they will all go away simultaneously.</p>
+</div> <!-- interface-visibility -->
-<p>See also the <a href="#exception-handling">Exception handling</a>
-section, for details of how resources associated with a pool are
-cleaned up when that pool is destroyed.</p>
+<div class="h2" id="coding-style" title="coding-style">
+<h2>Coding style</h2>
-<p>In summary:</p>
+<p>Subversion uses ANSI C, and follows the GNU coding standards,
+except that we do not put a space between the name of a function and
+the opening parenthesis of its parameter list. Emacs users can just
+load svn-dev.el to get the right indentation behavior (most source
+files here will load it automatically, if `enable-local-eval' is set
+appropriately).</p>
-<ul>
+<p>Read <a href="http://www.gnu.org/prep/standards.html"
+>http://www.gnu.org/prep/standards.html</a> for a full description of
+the GNU coding standards. Below is a short example demonstrating the
+most important formatting guidelines, including our
+no-space-before-param-list-paren exception:</p>
-<li><p>Objects should not have their own pools. An object is
- allocated into a pool defined by the constructor's caller. The
- caller knows the lifetime of the object and will manage it via
- the pool.</p>
-</li>
+<pre>
+ char * /* func type on own line */
+ argblarg(char *arg1, int arg2) /* func name on own line */
+ { /* first brace on own line */
+ if ((some_very_long_condition && arg2) /* indent 2 cols */
+ || remaining_condition) /* new line before operator */
+ { /* brace on own line, indent 2 */
+ arg1 = some_func(arg1, arg2); /* NO SPACE BEFORE PAREN */
+ } /* close brace on own line */
+ else
+ {
+ do /* format do-while like this */
+ {
+ arg1 = another_func(arg1);
+ }
+ while (*arg1);
+ }
+ }
+</pre>
-<li><p>Functions should not create/destroy pools for their operation;
- they should use a pool provided by the caller. Again, the
- caller knows more about how the function will be used, how
- often, how many times, etc. thus, it should be in charge of the
- function's memory usage.</p>
+<p>In general, be generous with parentheses even when you're sure
+about the operator precedence, and be willing to add spaces and
+newlines to avoid "code crunch". Don't worry too much about vertical
+density; it's more important to make code readable than to fit that
+extra line on the screen.</p>
- <p>For example, the caller might know that the app will exit upon
- the function's return. Thus, the function would create extra
- work if it built/destroyed a pool. Instead, it should use the
- passed-in pool, which the caller is going to be tossing as part
- of app-exit anyway.</p>
-</li>
+</div> <!-- coding-style -->
-<li><p>Whenever an unbounded iteration occurs, an iteration subpool
- should be used.</p>
-</li>
-<li><p>Given all of the above, it is pretty well mandatory to pass a
- pool to every function. Since objects are not recording pools
- for themselves, and the caller is always supposed to be
- managing memory, then each function needs a pool, rather than
- relying on some hidden magic pool. In limited cases, objects
- may record the pool used for their construction so that they
- can construct sub-parts, but these cases should be examined
- carefully.</p>
-</li>
-</ul>
+<div class="h2" id="use-page-breaks" title="use-page-breaks">
+<h2>Using page breaks</h2>
+<p>We're using page breaks (the Ctrl-L character, ASCII 12) for
+section boundaries in both code and plaintext prose files. Each
+section starts with a page break, and immediately after the page break
+comes the title of the section.</p>
-<p>See also <a href="#tracing-memory-leaks">Tracking down memory
-leaks</a> for tips on diagnosing pool usage problems.</p>
+<p>This helps out people who use the Emacs page commands, such as
+`pages-directory' and `narrow-to-page'. Such people are not as scarce
+as you might think, and if you'd like to become one of them, then add
+(require 'page-ext) to your .emacs and type C-x C-p C-h sometime.</p>
-</div>
+</div> <!-- use-page-breaks -->
-<div class="h2" id="apr-status-codes" title="apr-status-codes">
-<h2>APR status codes</h2>
+<div class="h2" id="error-messages" title="error-messages">
+<h2>Error message conventions</h2>
-<p>Always check for APR status codes (except APR_SUCCESS) with the
-APR_STATUS_IS_...() macros, not by direct comparison. This is required
-for portability to non-Unix platforms.</p>
+<p>For error messages the following conventions apply:</p>
-</div>
+<ul>
+<li><p>Provide specific error messages only when there is information
+ to add to the general error message found in
+ subversion/include/svn_error_codes.h.</p></li>
-<div class="h2" id="exception-handling" title="exception-handling">
-<h2>Exception handling</h2>
+<li><p>Messages start with a capital letter.</p></li>
-<p>OK, here's how to use exceptions in Subversion.</p>
+<li><p>Try keeping messages below 70 characters.</p></li>
-<ol>
+<li><p>Don't end the error message with a period (".").</p></li>
-<li><p>Exceptions are stored in svn_error_t structures:</p>
+<li><p>Don't include newline characters in error messages.</p></li>
-<pre>
-typedef struct svn_error_t
-{
- apr_status_t apr_err; /* APR error value, possibly SVN_ custom err */
- const char *message; /* details from producer of error */
- struct svn_error_t *child; /* ptr to the error we "wrap" */
- apr_pool_t *pool; /* place to generate message strings from */
- const char *file; /* Only used iff SVN_DEBUG */
- long line; /* Only used iff SVN_DEBUG */
-} svn_error_t;
-</pre>
+<li><p>Quoting information is done using single quotes (e.g. "'some info'").</p></li>
-</li>
+<li><p>Don't include the name of the function where the error occurs
+ in the error message. If Subversion is compiled using the
+ '--enable-maintainer-mode' configure-flag, it will provide this
+ information by itself.</p></li>
-<li><p>If you are the <em>original</em> creator of an error, you would do
- something like this:</p>
+<li><p>When including path or filenames in the error string, be sure
+ to quote them (e.g. "Can't find '/path/to/repos/userfile'").</p></li>
- <pre>
-return svn_error_create(SVN_ERR_FOO, NULL,
- "User not permitted to write file");
- </pre>
+<li><p>When including path or filenames in the error string, be sure
+ to convert them using <a
+ href="http://svn.collab.net/svn-doxygen/svn__path_8h.html#a1"
+ ><tt>svn_path_local_style()</tt></a> before inclusion (since
+ paths passed to and from Subversion APIs are assumed to be
+ in <a href="http://svn.collab.net/svn-doxygen/svn__path_8h.html#a0"
+ >canonical form</a>).</p></li>
- <p>NOTICE the NULL field... indicating that this error has no
- child, i.e. it is the bottom-most error.</p>
+<li><p>Don't use Subversion-specific abbreviations (e.g. use "repository"
+ instead of "repo", "working copy" instead of "wc").</p></li>
- <p>See also the <a href="#error-messages"> section on writing
- error messages</a>.</p>
+<li><p>If you want to add an explanation to the error, report it
+ followed by a colon and the explanation like this:</p>
+ <pre>
+ "Invalid " SVN_PROP_EXTERNALS " property on '%s': "
+ "target involves '.' or '..'".
+ </pre></li>
- <p>Subversion internally uses UTF-8 to store its data. This also
- applies to the 'message' string. APR is assumed to return its data
- in the current locale, so any text returned by APR needs
- conversion to UTF-8 before inclusion in the message string.</p>
-</li>
+<li><p>Suggestions or other additions can be added after a semi-colon,
+ like this:</p>
+ <pre>
+ "Can't write to '%s': object of same name already exists; remove "
+ "before retrying".
+ </pre></li>
-<li><p>If you <em>receive</em> an error, you have three choices:</p>
+<li><p>Try to stay within the boundaries of these conventions, so please avoid
+ separating different parts of error messages by other separators such
+ as '--' and others.</p></li>
- <ol>
- <li><p>Handle the error yourself. Use either your own code, or
- just call the primitive svn_handle_error(err). (This
- routine unwinds the error stack and prints out messages
- converting them from UTF-8 to the current locale.)</p>
+</ul>
- <p>When your routine receives an error which it intends to
- ignore or handle itself, be sure to clean it up using
- svn_error_clear(). Any time such an error is not cleared
- constitutes a <em>memory leak</em>.</p>
- </li>
+<p>Also read about <a href="#l10n">Localization</a>.</p>
- <li><p>Throw the error upwards, unmodified:</p>
+</div> <!-- error-messages -->
- <pre>
- error = some_routine(foo);
- if (error)
- return svn_error_return(error);
- </pre>
- <p>Actually, a better way to do this would be with the
- SVN_ERR() macro, which does the same thing:</p>
- <pre>
- SVN_ERR(some_routine(foo));
- </pre>
- </li>
+<div class="h2" id="apr-pools" title="apr-pools">
+<h2>APR pool usage conventions</h2>
- <li><p>Throw the error upwards, wrapping it in a new error
- structure by including it as the "child" argument:</p>
-
- <pre>
- error = some_routine(foo);
- if (error)
- {
- svn_error_t *wrapper = svn_error_create(SVN_ERR_FOO, error,
- "Authorization failed");
- return wrapper;
- }
- </pre>
-
- <p>Of course, there's a convenience routine which creates a
- wrapper error with the same fields as the child, except for
- your custom message:</p>
-
- <pre>
- error = some_routine(foo);
- if (error)
- {
- return svn_error_quick_wrap(error,
- "Authorization failed");
- }
- </pre>
-
- <p>The same can (and should) be done by using the SVN_ERR_W()
- macro:</p>
-
- <pre>
- SVN_ERR_W(some_routine(foo), "Authorization failed");
- </pre>
- </li>
- </ol>
-
- <p>In cases (b) and (c) it is important to know that resources
- allocated by your routine which are associated with a pool, are
- automatically cleaned up when the pool is destroyed. This means
- that there is no need to cleanup these resources before passing
- the error. There is therefore no reason not to use the SVN_ERR()
- and SVN_ERR_W() macros. Resources associated with pools are:</p>
-
- <ul>
-
- <li><p>Memory</p></li>
-
- <li><p>Files</p>
-
- <p>All files opened with apr_file_open are closed at pool
- cleanup. Subversion uses this function in its svn_io_file_*
- api, which means that files opened with svn_io_file_* or
- apr_file_open will be closed at pool cleanup.</p>
+<p>(This assumes you already basically understand how APR pools work;
+see apr_pools.h for details.)</p>
- <p>Some files (lock files for example) need to be removed when
- an operation is finished. APR has the APR_DELONCLOSE flag for
- this purpose. The following functions create files which are
- removed on pool cleanup:</p>
+<p>Applications using the Subversion libraries must call
+apr_initialize() before calling any Subversion functions.</p>
- <ul>
- <li><p>apr_file_open and svn_io_file_open (when passed the
- APR_DELONCLOSE flag)</p></li>
- <li><p>svn_io_open_unique_file (when passed TRUE in its
- delete_on_close)</p></li>
- </ul>
-
- <p>Locked files are unlocked if they were locked using
- svn_io_file_lock.</p>
- </li>
- </ul>
-</li>
+<p>Subversion's general pool usage strategy can be summed up in two
+principles:</p>
-<li><p>The <code>SVN_ERR()</code> macro will create a wrapped error when
- <code>SVN_ERR__TRACING</code> is defined. This helps developers
- determine what caused the error, and can be enabled with the
- <code>--enable-maintainer-mode</code> option to <code>configure</code>.
- </p>
+<ol>
+<li><p>The call level that created a pool is the only place to clear or
+ destroy that pool.</p>
</li>
+<li><p>When iterating an unbounded number of times, create a subpool
+ before entering the iteration, use it inside the loop and clear
+ it at the start of each iteration, then destroy it after the loop
+ is done, like so:</p>
+ <pre>
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
-<li><p>Sometimes, you just want to return whatever a called function
- returns, usually at the end of your own function. Avoid the
- temptation to directly return the result:</p>
- <pre>
- /* Don't do this! */
- return some_routine(foo);</pre>
-
- <p>Instead, use the svn_error_return meta-function to return the value.
- This ensures that stack tracing happens correctly when enabled.</p>
- <pre>
- return svn_error_return(some_routine(foo));</pre>
+ for (i = 0; i < n; ++i)
+ {
+ svn_pool_clear(iterpool);
+ do_operation(..., iterpool);
+ }
+ svn_pool_destroy(iterpool);
+ </pre>
</li>
</ol>
-</div>
+<p>Supporting the above rules, we use the following pool names as conventions
+to represent various pool lifetimes:</p>
+<ul>
+<li><p><code>result_pool</code> - The pool in which the output of a function
+should be allocated. A result pool declaration should <strong>always</strong>
+be found in a function argument list, and never inside a local block. (But
+not all functions need or have result pools.)
+</p></li>
+<li><p><code>scratch_pool</code> - The pool in which all function-local data
+should be allocated. This pool is also provided by the caller, who may
+choose to clear this pool immediately when control returns to it.
+</p></li>
-<div class="h2" id="automated-tests" title="automated-tests">
-<h2>Automated tests</h2>
+<li><p><code>iterpool</code> - An <em>iteration</em> pool, used inside loops,
+as per the above example.</p></li>
+</ul>
-<p>For a description of how to use and add tests to Subversion's
-automated test framework, please read <a
-href="http://svn.collab.net/repos/svn/trunk/subversion/tests/README"
->subversion/tests/README</a> and <a
-href="http://svn.collab.net/repos/svn/trunk/subversion/tests/cmdline/README"
->subversion/tests/cmdline/README</a>.</p>
+<p>(Note: Some legacy code uses a single <code>pool</code> function argument,
+which operates as both the result and scratch pools.)</p>
-<p>Various people have arranged for the automated test framework to
-run at regular intervals on their own machines, sending the results to
-the svn-breakage@subversion.tigris.org mailing list. The more
-different platforms the tests run on, the more quickly we can detect
-portability bugs in Subversion. If you'd like to send svn-breakage
-messages too, use the <a
-href="http://svn.collab.net/repos/svn/trunk/tools/test-scripts/svntest/"
->svntest</a> framework (start at the <a
-href="http://svn.collab.net/repos/svn/trunk/tools/test-scripts/svntest/README"
->README</a>).</p>
+<p>By using an iterpool for loop-bounded data, you ensure O(1) instead
+of O(N) memory leakage should the function return abruptly from
+within the loop (say, due to error). That's why you shouldn't make a
+subpool for data which persists throughout a function, but instead
+should use the pool passed in by the caller. That memory will be
+reclaimed when the caller's pool is cleared or destroyed. If the
+caller is invoking the callee in a loop, then trust the caller to take
+care of clearing the pool on each iteration. The same logic
+propagates all the way up the call stack.</p>
-<p>Lieven Govaerts has set up a
-<a href="http://buildbot.sourceforge.net/" >BuildBot</a> build/test
-farm at <a href="http://buildbot.subversion.org/buildbot/"
->http://buildbot.subversion.org/buildbot/</a>, see his message</p>
+<p>The pool you use also helps readers of the code understand object
+lifetimes. Is a given object used only during one iteration of the
+loop, or will it need to last beyond the end of the loop? For
+example, pool choices indicate a lot about what's going on in this
+code:</p>
<pre>
- <a href="http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=114212">http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=114212</a>
- (Thread URL: <a href="http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=450110">http://subversion.tigris.org/servlets/BrowseList?list=dev&by=thread&from=450110</a>)
- Message-ID: 20060326205918.F3C50708B0@adicia.telenet-ops.be
- From: "Lieven Govaerts" <lgo@mobsol.be>
- To: <dev@subversion.tigris.org>
- Subject: Update: Subversion build and test farm with Buildbot.
- Date: Sun, 26 Mar 2006 22:56:11 +0200
-</pre>
+ apr_hash_t *persistent_objects = apr_hash_make(result_pool);
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
-<p>for more details. (<a href="http://buildbot.sourceforge.net/"
->BuildBot</a> is a system for centrally managing multiple automated
-testing environments; it's especially useful for portability testing,
-including of uncommitted changes.)</p>
+ for (i = 0; i < n; ++i)
+ {
+ const char *intermediate_result;
+ const char *key, *val;
+
+ svn_pool_clear(iterpool);
+ SVN_ERR(do_something(&intermediate_result, ..., iterpool));
+ SVN_ERR(get_result(intermediate_result, &key, &val, ...,
+ result_pool));
+ apr_hash_set(persistent_objects, key, APR_HASH_KEY_STRING, val);
+ }
-</div>
+ svn_pool_destroy(iterpool);
+ return persistent_objects;
+</pre>
+<p>Except for some legacy code, which was written before these
+principles were fully understood, virtually all pool usage in
+Subversion follows the above guidelines.</p>
-<div class="h2" id="write-test-cases-first" title="write-test-cases-first">
-<h2>Writing test cases before code</h2>
+<p>One such legacy pattern is a tendency to allocate an object inside
+a pool, store the pool in the object, and then free that pool (either
+directly or through a close_foo() function) to destroy the object.</p>
+
+<p>For example:</p>
<pre>
-From: Karl Fogel <kfogel@collab.net>
-Subject: writing test cases
-To: dev@subversion.tigris.org
-Date: Mon, 5 Mar 2001 15:58:46 -0600
+ <span style="color: red;">/*** Example of how NOT to use pools. Don't be like this. ***/</span>
-Many of us implementing the filesystem interface have now gotten into
-the habit of writing the test cases (see fs-test.c) *before* writing
-the actual code. It's really helping us out a lot -- for one thing,
-it forces one to define the task precisely in advance, and also it
-speedily reveals the bugs in one's first try (and second, and
-third...).
+ static foo_t *
+ make_foo_object(arg1, arg2, apr_pool_t *pool)
+ {
+ apr_pool_t *subpool = svn_pool_create(pool);
+ foo_t *foo = apr_palloc(subpool, sizeof(*foo));
-I'd like to recommend this practice to everyone. If you're
-implementing an interface, or adding an entirely new feature, or even
-just fixing a bug, a test for it is a good idea. And if you're going
-to write the test anyway, you might as well write it first. :-)
+ foo->field1 = arg1;
+ foo->field2 = arg2;
+ foo->pool = subpool;
+ }
-Yoshiki Hayashi's been sending test cases with all his patches lately,
-which is what inspired me to write this mail to encourage everyone to
-do the same. Having those test cases makes patches easier to examine,
-because they show the patch's purpose very clearly. It's like having
-a second log message, one whose accuracy is verified at run-time.
+ [...]
-That said, I don't think we want a rigid policy about this, at least
-not yet. If you encounter a bug somewhere in the code, but you only
-have time to write a patch with no test case, that's okay -- having
-the patch is still useful; someone else can write the test case.
+ [Now some function calls make_foo_object() and returns, passing
+ back a new foo object.]
-As Subversion gets more complex, though, the automated test suite gets
-more crucial, so let's all get in the habit of using it early.
+ [...]
--K
+ [Now someone, at some random call level, decides that the foo's
+ lifetime is over, and calls svn_pool_destroy(foo->pool).]
</pre>
-</div>
+<p>This is tempting, but it defeats the point of using pools, which is
+to not worry so much about individual allocations, but rather about
+overall performance and lifetime groups. Instead, foo_t generally
+should not have a `pool' field. Just allocate as many foo objects as
+you need in the current pool — when that pool gets
+cleared or destroyed, they will all go away simultaneously.</p>
+<p>See also the <a href="#exception-handling">Exception handling</a>
+section, for details of how resources associated with a pool are
+cleaned up when that pool is destroyed.</p>
-<div class="h2" id="server-debugging" title="server-debugging">
-<h2>Debugging the server</h2>
+<p>In summary:</p>
-<div class="h3" id="debugging-ra-dav" title="debugging-ra-dav">
-<h3>Debugging the DAV server</h3>
+<ul>
-<p>'mod_dav_svn.so' contains the main Subversion server logic; it runs
-as a module within mod_dav, which runs as a module within httpd.
-Since httpd is probably using dynamic shared modules, you normally
-won't be able to set breakpoints in advance when you start Apache in a
-debugger such as GDB. Instead, you'll need to start up, then
-interrupt httpd, set your breakpoint, and continue:</p>
-
-<pre>
- % gdb httpd
- (gdb) run -X
- ^C
- (gdb) break some_func_in_mod_dav_svn
- (gdb) continue
-</pre>
-
-<p>The -X switch is equivalent to -DONE_PROCESS and -DNO_DETACH, which
-ensure that httpd runs as a single thread and remains attached to the
-tty, respectively. As soon as it starts, it sits and waits for
-requests; that's when you hit control-C and set your breakpoint.</p>
+<li><p>Objects should not have their own pools. An object is
+ allocated into a pool defined by the constructor's caller. The
+ caller knows the lifetime of the object and will manage it via
+ the pool.</p>
+</li>
-<p>You'll probably want to watch Apache's run-time logs</p>
+<li><p>Functions should not create/destroy pools for their operation;
+ they should use a pool provided by the caller. Again, the
+ caller knows more about how the function will be used, how
+ often, how many times, etc. thus, it should be in charge of the
+ function's memory usage.</p>
-<pre>
- /usr/local/apache2/logs/error_log
- /usr/local/apache2/logs/access_log
-</pre>
+ <p>For example, the caller might know that the app will exit upon
+ the function's return. Thus, the function would create extra
+ work if it built/destroyed a pool. Instead, it should use the
+ passed-in pool, which the caller is going to be tossing as part
+ of app-exit anyway.</p>
+</li>
-<p>to help determine what might be going wrong and where to set
-breakpoints.</p>
+<li><p>Whenever an unbounded iteration occurs, an iteration subpool
+ should be used.</p>
+</li>
-</div>
+<li><p>Given all of the above, it is pretty well mandatory to pass a
+ pool to every function. Since objects are not recording pools
+ for themselves, and the caller is always supposed to be
+ managing memory, then each function needs a pool, rather than
+ relying on some hidden magic pool. In limited cases, objects
+ may record the pool used for their construction so that they
+ can construct sub-parts, but these cases should be examined
+ carefully.</p>
+</li>
+</ul>
-<div class="h3" id="debugging-ra-svn" title="debugging-ra-svn">
-<h3>Debugging the ra_svn client and server, on Unix</h3>
-<p>Bugs in ra_svn usually manifest themselves with one of the
-following cryptic error messages:</p>
+<p>See also <a href="#tracing-memory-leaks">Tracking down memory
+leaks</a> for tips on diagnosing pool usage problems.</p>
-<pre>
- svn: Malformed network data
- svn: Connection closed unexpectedly
-</pre>
+</div> <!-- apr-pools -->
-<p>(The first message can also mean the data stream was corrupted in
-tunnel mode by user dotfiles or hook scripts; see
-<a href="http://subversion.tigris.org/issues/show_bug.cgi?id=1145"
->issue #1145</a>.) The first message generally means you to have
-to debug the client; the second message generally means you have to
-debug the server.</p>
-<p>It is easiest to debug ra_svn using a build with --disable-shared
---enable-maintainer-mode. With the latter option, the error message
-will tell you exactly what line to set a breakpoint at; otherwise,
-look up the line number at the end of marshal.c:vparse_tuple() where
-it returns the "Malformed network data" error.</p>
+<div class="h2" id="apr-status-codes" title="apr-status-codes">
+<h2>APR status codes</h2>
-<p>To debug the client, simply pull it up in gdb, set a breakpoint,
-and run the offending command:</p>
+<p>Always check for APR status codes (except APR_SUCCESS) with the
+APR_STATUS_IS_...() macros, not by direct comparison. This is required
+for portability to non-Unix platforms.</p>
-<pre>
- % gdb svn
- (gdb) break marshal.c:NNN
- (gdb) run ARGS
- Breakpoint 1, vparse_tuple (list=___, pool=___, fmt=___,
- ap=___) at subversion/libsvn_ra_svn/marshal.c:NNN
- NNN "Malformed network data");
-</pre>
+</div> <!-- apr-status-codes -->
-<p>There are several bits of useful information:</p>
-<ul>
-<li><p>A backtrace will tell you exactly what protocol exchange is
- failing.</p>
-</li>
+<div class="h2" id="exception-handling" title="exception-handling">
+<h2>Exception handling</h2>
-<li><p>"print *conn" will show you the connection buffer. read_buf,
- read_ptr, and read_end represent the read buffer, which can
- show
- you the data the marshaller is looking at. (Since read_buf isn't
- generally 0-terminated at read_end, be careful of falsely assuming
- that there's garbage data in the buffer.)</p>
-</li>
+<p>OK, here's how to use exceptions in Subversion.</p>
-<li><p>The format string determines what the marshaller was expecting to
- see.</p>
-</li>
-</ul>
+<ol>
-<p>To debug the server in daemon mode, pull it up in gdb, set a
-breakpoint (usually a "Connection closed unexpectedly" error on the
-client indicates a "Malformed network data" error on the server,
-although it can also indicate a core dump), and run it with the "-X"
-option to serve a single connection:</p>
+<li><p>Exceptions are stored in svn_error_t structures:</p>
<pre>
- % gdb svnserve
- (gdb) break marshal.c:NNN
- (gdb) run -X
+typedef struct svn_error_t
+{
+ apr_status_t apr_err; /* APR error value, possibly SVN_ custom err */
+ const char *message; /* details from producer of error */
+ struct svn_error_t *child; /* ptr to the error we "wrap" */
+ apr_pool_t *pool; /* place to generate message strings from */
+ const char *file; /* Only used iff SVN_DEBUG */
+ long line; /* Only used iff SVN_DEBUG */
+} svn_error_t;
</pre>
-<p>Then run the offending client command. From there, it's just like
-debugging the client.</p>
-
-<p>Debugging the server in tunnel mode is more of a pain. You'll need
-to stick something like "{ int x = 1; while (x); }" near the top of
-svnserve's main() and put the resulting svnserve in the user path on
-the server. Then start an operation, gdb attach the process on the
-server, "set x = 0", and step through the code as desired.</p>
+</li>
-</div>
+<li><p>If you are the <em>original</em> creator of an error, you would do
+ something like this:</p>
-</div>
+ <pre>
+return svn_error_create(SVN_ERR_FOO, NULL,
+ "User not permitted to write file");
+ </pre>
-<div class="h2" id="net-trace" title="net-trace">
-<h2>Tracing network traffic</h2>
+ <p>NOTICE the NULL field... indicating that this error has no
+ child, i.e. it is the bottom-most error.</p>
-<p>Use <a href="http://www.wireshark.org/">Wireshark</a> (formerly
-known as "Ethereal") to eavesdrop on the conversation.</p>
+ <p>See also the <a href="#error-messages"> section on writing
+ error messages</a>.</p>
-<p>First, make sure that between captures within the same wireshark
-session, you hit <em>Clear</em>, otherwise filters from one capture
-(say, an HTTP capture) might interfere with others (say, an ra_svn
-capture). </p>
+ <p>Subversion internally uses UTF-8 to store its data. This also
+ applies to the 'message' string. APR is assumed to return its data
+ in the current locale, so any text returned by APR needs
+ conversion to UTF-8 before inclusion in the message string.</p>
+</li>
-<p>Assuming you're cleared, then:</p>
+<li><p>If you <em>receive</em> an error, you have three choices:</p>
-<ol>
-<li><p>Pull down the <i>Capture</i> menu, and choose
- <i>Capture Filters</i>.</p></li>
-<li><p>If debugging the http:// (WebDAV) protocol, then in the window
- that pops up, choose "<code>HTTP TCP port (80)</code>"
- (which should result in the filter string
- "<code><i>tcp port http</i></code>").</p>
- <p>If debugging the svn:// (ra_svn) protocol, then choose <i>New</i>,
- give the new filter a name (say, "ra_svn"), and type
- "<code>tcp port 3690</code>" into the filter string box.</p>
- <p>When done, click OK.</p></li>
-<li><p>Again go to the <i>Capture</i> menu, this time choose
- <i>Interfaces</i>, and click <i>Options</i> next to the
- appropriate interface (probably you want interface "lo", for
- "loopback", assuming the server will run on the same machine as
- the client).</p></li>
-<li><p>Turn off promiscuous mode by unchecking the appropriate
- checkbox.</p></li>
-<li><p>Click the <i>Start</i> button in the lower right to start the
- capture.</p></li>
-<li><p>Run your Subversion client.</p></li>
-<li><p>Click the Stop icon (a red X over an ethernet interface card) when
- the operation is finished (or <i>Capture->Stop</i> should
[... 2925 lines stripped ...]