Participating in the community ¶

Although Subversion was originally sponsored and hosted by CollabNet (https://www.collab.net/), 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.

The community exists mainly through mailing lists and a Subversion repository. To participate:

For several years, the Subversion Community meet in Berlin for a Hackathon once a year: 2016, 2015, 2014, 2013, 2012, 2011 and 2010 [linked from archive.org, some media links do not work].

There are many ways to join the project, either by writing code, or by testing and/or helping to manage the bug database. If you'd like to contribute, then look at:

• The bugs/issues database https://issues.apache.org/jira/projects/SVN/issues

To submit code, simply send your patches to dev@subversion.apache.org. No, wait, first read the rest of this file, then start sending patches to dev@subversion.apache.org. :-)

To help manage the issues database, read over the issue summaries, looking and testing for issues that are either invalid, or are duplicates of other issues. Both kinds are very common, the first because bugs often get unknowingly fixed as side effects of other changes in the code, and the second because people sometimes file an issue without noticing that it has already been reported. If you are not sure about an issue, post a question to dev@subversion.apache.org. ("Subversion: We're here to help you help us!")

Another way to help is to set up automated builds and test suite runs of Subversion on some platform, and have the output sent to the notifications@subversion.apache.org mailing list. See more details at the mailing lists page.

Finally, despite the online nature of the Subversion project and the human contact abstraction that results from that fact, it is important to realize that there are real people at the end of all contributions. Treat all other community members as you would expect to be treated. Review the contribution, not the contributor; don't annoy others, and don't become easily annoyed yourself.

Theory and documentation ¶

1. Design

   A design spec was written in June 2000, and is a bit out of date. But it still gives a good theoretical introduction to the inner workings of the repository, and to Subversion's various layers.

2. API Documentation

   See the section on the public API documentation for more information.

3. Delta Editors

   Karl Fogel wrote a chapter for O'Reilly's 2007 book Beautiful Code: Leading Programmers Explain How They Think covering the design and use of Subversion's delta editor interface.

4. Network Protocols

   The WebDAV Usage document is an introduction to Subversion's DAV network protocol, which is an extended dialect of HTTP and uses URLs beginning with "http://" or "https://".

   The SVN Protocol document contains a formal description of Subversion ra_svn network protocol, which is a custom protocol on port 3690 (by default), whose URLs begin with "svn://" or "svn+ssh://".

5. User Manual

   Version Control with Subversion is a book published by O'Reilly that shows in detail how to effectively use Subversion. The text of the book is free, and is actively being revised. On-line versions are available at https://svnbook.red-bean.com/. The XML source and translations to other languages are maintained in their own repository at https://sourceforge.net/projects/svnbook/.

6. System notes

   A lot of the design ideas for particular aspects of the system have been documented in individual files in the notes/ directory.

Code to read ¶

Before you can contribute code, you'll need to familiarize yourself with the existing code base and interfaces.

Check out a copy of Subversion (anonymously, if you don't yet have an account with commit access) — so you can look at the code.

Within 'subversion/include/' are a bunch of header files with huge doc comments. If you read through these, you'll have a pretty good understanding of the implementation details. Here's a suggested perusal order:

1. the basic building blocks: svn_string.h, svn_error.h, svn_types.h

2. svn_io.h, svn_path.h, svn_hash.h, svn_xml.h

3. the critical interface: svn_delta.h

4. client-side interfaces: svn_ra.h, svn_wc.h, svn_client.h

5. the repository and versioned filesystem: svn_repos.h, svn_fs.h

Subversion tries to stay portable by using only the C89/C90 dialect of ANSI/ISO C and by using the Apache Portable Runtime (APR) library. APR is the portability layer used by the Apache httpd server, and more information can be found at https://apr.apache.org/.

Because Subversion depends so heavily on APR, it may be hard to understand Subversion without first glancing over certain header files in APR (look in 'apr/include/'):

• memory pools: apr_pools.h

• filesystem access: apr_file_io.h

• hashes and arrays: apr_hash.h, apr_tables.h

Subversion also tries to deliver reliable and secure software. This can only be achieved by developers who understand secure programming in the C programming language. Please see 'notes/assurance.txt' for the full rationale behind this. Specifically, you should make it a point to carefully read David Wheeler's Secure Programming (as mentioned in 'notes/assurance.txt'). If at any point you have questions about the security implications of a change, you are urged to ask for review on the developer mailing list.

Directory layout ¶

A rough guide to the source tree:

• doc/
  User and Developer documentation.

• tools/
  Stuff that works with Subversion, but that Subversion doesn't depend on. Code in tools/ is maintained collectively by the Subversion project, and is under the same open source copyright as Subversion itself.

• contrib/
  Stuff that works with Subversion, but that Subversion doesn't depend on, and that is maintained by individuals who may or may not participate in Subversion development. Code in contrib/ is open source, but may have a different license or copyright holder than Subversion itself.

• subversion/
  Source code to Subversion itself (as opposed to external libraries).

• subversion/include/
  Public header files for users of Subversion libraries.

• subversion/include/private/
  Private header files shared internally by Subversion libraries.

• subversion/libsvn_fs/
  The versioning "filesystem" API.

• subversion/libsvn_repos/
  Repository functionality built around the `libsvn_fs' core.

• subversion/libsvn_delta/
  Common code for tree deltas, text deltas, and property deltas.

• subversion/libsvn_wc/
  Common code for working copies.

• subversion/libsvn_ra/
  Common code for repository access.

• subversion/libsvn_client/
  Common code for client operations.

• subversion/svn/
  The command line client.

• subversion/tests/
  Automated test suite.

Branching policy ¶

The Subversion project strongly prefers that active development happens 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.

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.

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.

This branch exists for the resolution of issue #8810, per the ideas
documented in /trunk/notes/frobnobbing-feature.txt.  It is a feature
branch, receiving regular sync merges from /trunk, and expected to be
reintegrated back thereto.

TODO:

  * compose regression tests        [DONE]
  * add frob identification logic   [STARTED (fitz)]
  * add nobbing bits                []

Documentation ¶

  /*** How not to document.  Don't do this. ***/

  /* Make a foo object. */
  static foo_t *
  make_foo_object(arg1, arg2, apr_pool_t *pool)
  {
     /* Create a subpool. */
     apr_pool_t *subpool = svn_pool_create(pool);

     /* Allocate a foo object from the main pool */
     foo_t *foo = apr_palloc(pool, sizeof(*foo));
     ...
  }

      /* Transmit the segment (if its within the scope of our concern). */
      SVN_ERR(maybe_crop_and_send_segment(segment, start_rev, end_rev,
                                          receiver, receiver_baton, subpool));

      /* If we've set CURRENT_REV to SVN_INVALID_REVNUM, we're done
         (and didn't ever reach END_REV).  */
      if (! SVN_IS_VALID_REVNUM(current_rev))
        break;

      /* If there's a gap in the history, we need to report as much
         (if the gap is within the scope of our concern). */
      if (segment->range_start - current_rev < 1)
        {
          svn_location_segment_t *gap_segment;
          gap_segment = apr_pcalloc(subpool, sizeof(*gap_segment));
          gap_segment->range_end = segment->range_start - 1;
          gap_segment->range_start = current_rev + 1;
          gap_segment->path = NULL;
          SVN_ERR(maybe_crop_and_send_segment(gap_segment, start_rev, end_rev,
                                              receiver, receiver_baton,
                                              subpool));
        }

Patch submission guidelines ¶

svn checkout https://svn.apache.org/repos/asf/subversion/trunk/ svn-trunk

   Subject: [PATCH] fix for rev printing bug in svn status

   [[[
   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.
   ]]]

Committers ¶

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.

The COMMITTERS file lists all committers, both full and partial, and says the domains for each partial committer.

   Approved by: lundblad

   https://svn.haxx.se/dev/archive-2007-11/0848.shtml
   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>

------------------------------------------------------------------------
r32135 | stylesen | 2008-07-16 10:04:25 +0200 (Wed, 16 Jul 2008) | 8 lines

Update "check-license.py" so that it can generate license text applicable
to this year.

Obvious fix.

* tools/dev/check-license.py
  (NEW_LICENSE): s/2005/2008/

------------------------------------------------------------------------

Release Manager ¶

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.

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. You can read more about the release process in the section Making Subversion Releases.

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.

In an effort to encourage wider dispersion of release management knowledge, the RM role is currently rotating among various victims volunteers.

Patch Manager ¶

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".

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 the correct type — 'DEFECT', 'FEATURE', or 'ENHANCEMENT' (not 'PATCH') — is filed, the patch is saved to that new issue, and the "patch" keyword is recorded on the issue.

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 not require actual Subversion development experience or commit access. Expertise in using one's mail reading software is optional, but recommended :-).

The current Patch Manager is: Gavin 'Beau' Baumanis <gavin@thespidernet.com>.

Code modularity and interface visibility ¶

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:

"Are the consumers of my new code local to a particular source code file in a single library?" If so, you probably want a static function in that same source file.

"Is my new function of the sort that other source code within this library will need to use, but nothing *outside* the library?" If so, you want to use a non-static, double-underscore-named function (such as svn_foo__do_something), with its prototype in the appropriate library-specific header file.

"Will my code need to be accessed from a different library?" Here you have some additional questions to answer, such as "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?" Either way, you're now looking at using an inter-library header file. But see the next question before you decide which one...

"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?" If so, you'll be adding prototypes to the public API, immediately inside subversion/include/. 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 subversion/include/private/.

Coding style ¶

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).

Read https://www.gnu.org/prep/standards.html 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:

   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);
       }
   }

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.

Using page breaks ¶

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.

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.

Error message conventions ¶

For error messages the following conventions apply:

• Provide specific error messages only when there is information to add to the general error message found in subversion/include/svn_error_codes.h.

• Messages start with a capital letter.

• Try keeping messages below 70 characters.

• Don't end the error message with a period (".").

• Don't include newline characters in error messages.

• Quoting information is done using single quotes (e.g. "'some info'").

• 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.

• When including path or filenames in the error string, be sure to quote them (e.g. "Can't find '/path/to/repos/userfile'").

• When including path or filenames in the error string, be sure to convert them using svn_dirent_local_style() before inclusion (since paths passed to and from Subversion APIs are assumed to be in canonical form).

• Don't use Subversion-specific abbreviations (e.g. use "repository" instead of "repo", "working copy" instead of "wc").

• If you want to add an explanation to the error, report it followed by a colon and the explanation like this:

         "Invalid " SVN_PROP_EXTERNALS " property on '%s': "
         "target involves '.' or '..'".
       

• Suggestions or other additions can be added after a semi-colon, like this:

         "Can't write to '%s': object of same name already exists; remove "
         "before retrying".
       

• 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.

Also read about Localization.

APR pool usage conventions ¶

(This assumes you already basically understand how APR pools work; see apr_pools.h for details.)

Applications using the Subversion libraries must call apr_initialize() before calling any Subversion functions.

Subversion's general pool usage strategy can be summed up in two principles:

1. The call level that created a pool is the only place to clear or destroy that pool.

2. 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:

            apr_pool_t *iterpool = svn_pool_create(scratch_pool);
   
            for (i = 0; i < n; ++i)
            {
              svn_pool_clear(iterpool);
              do_operation(..., iterpool);
            }
   
            svn_pool_destroy(iterpool);
          

Supporting the above rules, we use the following pool names as conventions to represent various pool lifetimes:

• result_pool: The pool in which the output of a function should be allocated. A result pool declaration should always be found in a function argument list, and never inside a local block. (But not all functions need or have result pools.)

• scratch_pool: 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.

• iterpool: An iteration pool, used inside loops, as per the above example.

(Note: Some legacy code uses a single pool function argument, which operates as both the result and scratch pools.)

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.

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:

      apr_hash_t *persistent_objects = apr_hash_make(result_pool);
      apr_pool_t *iterpool = svn_pool_create(scratch_pool);

      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);
      }

      svn_pool_destroy(iterpool);
      return persistent_objects;

Except for some legacy code, which was written before these principles were fully understood, virtually all pool usage in Subversion follows the above guidelines.

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.

For example:

   /*** Example of how NOT to use pools.  Don't be like this. ***/

   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));

      foo->field1 = arg1;
      foo->field2 = arg2;
      foo->pool   = subpool;
   }

   [...]

   [Now some function calls make_foo_object() and returns, passing
   back a new foo object.]

   [...]

   [Now someone, at some random call level, decides that the foo's
   lifetime is over, and calls svn_pool_destroy(foo->pool).]

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.

See also the Exception handling section, for details of how resources associated with a pool are cleaned up when that pool is destroyed.

In summary:

• 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.

• 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.

  For example, think of a function that is being called multiple times in a tight loop. The caller clears the scratch pool on each iteration. Thus, the creation of an internal subpool is unnecessary and could be a significant overhead; instead, the function should just use the passed-in pool.

• Whenever an unbounded iteration occurs, an iteration subpool should be used.

• 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.

See also Tracking down memory leaks for tips on diagnosing pool usage problems.

APR status codes ¶

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.

Exception handling ¶

OK, here's how to use exceptions in Subversion.

1. Exceptions are stored in svn_error_t structures:

   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;
   

2. If you are the original creator of an error, you would do something like this:

   return svn_error_create(SVN_ERR_FOO, NULL, 
                           "User not permitted to write file");
       

   NOTICE the NULL field... indicating that this error has no child, i.e. it is the bottom-most error.

   See also the section on writing error messages.

   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.

3. If you receive an error, you have three choices:

   1. 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.)

      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 memory leak.

      A function that returns an error is not required to initialize its output parameters.

   2. Throw the error upwards, unmodified:

              error = some_routine(foo);
              if (error)
                return svn_error_trace(error);
              

      Actually, a better way to do this would be with the SVN_ERR() macro, which does the same thing:

              SVN_ERR(some_routine(foo));
              

   3. Throw the error upwards, wrapping it in a new error structure by including it as the "child" argument:

              error = some_routine(foo);
              if (error)
                {
                 svn_error_t *wrapper = svn_error_create(SVN_ERR_FOO, error,
                                                         "Authorization failed");
                 return wrapper;
                }
              

      Of course, there's a convenience routine which creates a wrapper error with the same fields as the child, except for your custom message:

              error = some_routine(foo);
              if (error)
                {
                 return svn_error_quick_wrap(error, 
                                             "Authorization failed");
                }
              

      The same can (and should) be done by using the SVN_ERR_W() macro:

                SVN_ERR_W(some_routine(foo), "Authorization failed");
              

   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:

   • Memory

   • Files

     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.

     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:

     • apr_file_open and svn_io_file_open (when passed the APR_DELONCLOSE flag)

     • svn_io_open_unique_file (when passed TRUE in its delete_on_close)

     Locked files are unlocked if they were locked using svn_io_file_lock.

4. The SVN_ERR() macro will create a wrapped error when SVN_ERR__TRACING is defined. This helps developers determine what caused the error, and can be enabled with the --enable-maintainer-mode option to configure.

5. 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:

       /* Don't do this! */
       return some_routine(foo);

   Instead, use the svn_error_trace meta-function to return the value. This ensures that stack tracing happens correctly when enabled.

       return svn_error_trace(some_routine(foo));

Secure coding guidelines ¶

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.

Input validation is the act of defining legal input and rejecting everything else. The code must perform input validation on all untrusted input.

Security boundaries:

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.

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.

[### todo: need some examples from Subversion here...]

String operations:

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.

Password storage:

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.

Destruction of stacked resources ¶

Some resources need destruction to ensure correct functioning of the application. Such resources include files, especially since open files cannot be deleted on Windows.

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).

At first in https://svn.haxx.se/dev/archive-2005-12/0487.shtml and later in https://svn.haxx.se/dev/archive-2005-12/0633.shtml this was discussed in more general terms for files, streams, editors and window handlers.

As Greg Hudson put it:

On consideration, here is where I would like us to be:

• Streams which read from or write to an underlying object own that object, i.e. closing the stream closes the underlying object, if applicable.
• The layer (function or data type) which created a stream is responsible for closing it, except when the above rule applies.
• Window handlers are thought of as an odd kind of stream, and passing the final NULL window is considered closing the stream.

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.

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.

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.

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 not destroyed, even though any streams stacked upon it may try to do so.

Variadic argument lists ¶

When you call a function that accepts a variable number of arguments and expects the list to be terminated with a null pointer constant (an example of such a function is apr_pstrcat), do not use the NULL symbol to terminate the list. Depending on compiler and platform, NULL may or may not be a pointer-sized constant; if it isn't the function may end up reading data beyond the end of the argument list.

Instead, use SVN_VA_NULL (defined since 1.9 in svn_types.h), which is guaranteed to be a null pointer constant. For example:

   return apr_pstrcat(cmd->temp_pool, "Cannot parse expression '",
                      arg2, "' in SVNPath: ", expr_err, SVN_VA_NULL);

Other coding conventions ¶

In addition to the GNU standards, Subversion uses these conventions:

• When using a path or file name as input to most Subversion APIs, be sure to convert them to Subversion's internal/canonical form using the svn_dirent_internal_style() 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 svn_dirent_local_style() API.

• 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.

• 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.)

• 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 libsvn_wc/update_editor.c) 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 include/private/ directory, and use the double underscore notation. Such symbols may be used by Subversion core code only.

  To recap:

           /* 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 ...
  
           /* 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 ...
  
           /* For use within one file: subversion/libsvn_wc/update_editor.c */ 
           get_entry_url()
           struct handler_baton {
  
           /* For internal use in svn core code only:
              subversion/include/private/svn_wc_private.h */
           svn_wc__entry_versioned()
        

  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 backwards compatibility.

• 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:

           $ svn revert foo
           svn: warning: svn_wc_is_wc_root: 'foo' is not a versioned resource
           $
        

  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.

• If you use Emacs, put something like this in your .emacs file, so you get svn-dev.el and svnbook.el when needed:

           ;;; 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.)")))))))
  
           (add-hook 'find-file-hooks 'my-find-file-hook)
           ;;; End Subversion development section
        

  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:

        > 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. :-)
        

• 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.

• 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.

• 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.

Writing log messages ¶

Every commit needs a log message.

The intended audience for a log message is a developer who is already familiar with Subversion, but not necessarily familiar with this particular commit. Usually when someone goes back and reads a change, he no longer has in his head all the context around that change. This is true even if he is the author of the change! All the discussions and mailing list threads and everything else may be forgotten; the only clue to what the change is about comes from the log message and the diff itself. People revisit changes with surprising frequency, too: for example, it might be months after the original commit and now the change is being ported to a maintenance branch.

The log message is the introduction to the change.

• If you are working on a branch, prefix your log message with:

     On the 'name-of-branch' branch: (Start of your log message)
  

• Start your log message with one line indicating the general nature of the change, and follow that with a descriptive paragraph if necessary.

This helps put developers in the right frame of mind for reading the rest of the log message.

If the commit is just one simple change to one file, then you can dispense with the general description and simply go straight to the detailed description, in the standard filename-then-symbol format shown below.

Throughout the log message, use full sentences, not sentence fragments. Fragments are more often ambiguous, and it takes only a few more seconds to write out what you mean. Certain fragments like "Doc fix", "New file", or "New function" are acceptable because they are standard idioms, and all further details should appear in the source code.

The log message should name every affected function, variable, macro, makefile target, grammar rule, etc, including the names of symbols that are being removed in this commit. This helps people searching through the logs later. Don't hide names in wildcards, because the globbed portion may be what someone searches for later. For example, this is bad:

   * subversion/libsvn_ra_pigeons/twirl.c
     (twirling_baton_*): Removed these obsolete structures.
     (handle_parser_warning): Pass data directly to callees, instead
      of storing in twirling_baton_*.

   * subversion/libsvn_ra_pigeons/twirl.h: Fix indentation.

Later on, when someone is trying to figure out what happened to `twirling_baton_fast', they may not find it if they just search for "_fast". A better entry would be:

   * subversion/libsvn_ra_pigeons/twirl.c
     (twirling_baton_fast, twirling_baton_slow): Removed these
      obsolete structures. 
     (handle_parser_warning): Pass data directly to callees, instead
      of storing in twirling_baton_*. 

   * subversion/libsvn_ra_pigeons/twirl.h: Fix indentation.

The wildcard is okay in the description for `handle_parser_warning', but only because the two structures were mentioned by full name elsewhere in the log entry.

You should also include property changes in your log messages. For example, if you were to modify the "svn:ignore" property on the trunk, you might put something like this in your log:

   * trunk/ (svn:ignore): Ignore 'build'.

The above only applies to properties you maintain, not those maintained by subversion like "svn:mergeinfo".

Note how each file gets its own entry prefixed with an "*", and the changes within a file are grouped by symbol, with the symbols listed in parentheses followed by a colon, followed by text describing the change. Please adhere to this format, even when only one file is changed — not only does consistency aid readability, it also allows software to colorize log entries automatically.

As an exception to the above, if you make exactly the same change in several files, list all the changed files in one entry. For example:

   * subversion/libsvn_ra_pigeons/twirl.c,
     subversion/libsvn_ra_pigeons/roost.c:
     Include svn_private_config.h.

If all the changed files are deep inside the source tree, you can shorten the file name entries by noting the common prefix before the change entries:

   [in subversion/bindings/swig/birdsong]

   * dialects/nightingale.c (get_base_pitch): Allow 3/4-tone
     pitch variation to account for trait variability amongst
     isolated populations Erithacus megarhynchos.

   * dialects/gallus_domesticus.c: Remove. Unreliable due to
     extremely low brain-to-body mass ratio.

If your change is related to a specific issue in the issue tracker, then include a string like "issue #N" in the log message, but make sure you still summarize what the change is about. For example, if a patch resolves issue #1729, then the log message might be:

   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.

Try to put related changes together. For example, if you create svn_ra_get_ansible2(), deprecating svn_ra_get_ansible(), then those two things should be near each other in the log message:

   * subversion/include/svn_ra.h
     (svn_ra_get_ansible2): New prototype, obsoletes svn_ra_get_ansible.
     (svn_ra_get_ansible): Deprecate.

For large changes or change groups, group the log entry into paragraphs separated by blank lines. Each paragraph should be a set of changes that accomplishes a single goal, and each group should start with a sentence or two summarizing the change. Truly independent changes should be made in separate commits, of course.

See Crediting for how to give credit to someone else if you are committing their patch, or committing a change they suggested.

One should never need the log entries to understand the current code. If you find yourself writing a significant explanation in the log, you should consider carefully whether your text doesn't actually belong in a comment, alongside the code it explains. Here's an example of doing it right:

   (consume_count): If `count' is unreasonable, return 0 and don't
    advance input pointer.

And then, in `consume_count' in `cplus-dem.c':

   while (isdigit((unsigned char)**type))
     {
       count *= 10;
       count += **type - '0';
       /* A sanity check.  Otherwise a symbol like
         `_Utf390_1__1_9223372036854775807__9223372036854775'
         can cause this function to return a negative value.
         In this case we just consume until the end of the string.  */
      if (count > strlen(*type))
        {
          *type = save;
          return 0;
        }

This is why a new function, for example, needs only a log entry saying "New Function" --- all the details should be in the source.

You can make common-sense exceptions to the need to name everything that was changed. For example, if you have made a change which requires trivial changes throughout the rest of the program (e.g., renaming a variable), you needn't name all the functions affected, you can just say "All callers changed". When renaming any symbol, please remember to mention both the old and new names, for traceability; see r861020 for an example.

In general, there is a tension between making entries easy to find by searching for identifiers, and wasting time or producing unreadable entries by being exhaustive. Use the above guidelines and your best judgment, and be considerate of your fellow developers. (Also, use "svn log" to see how others have been writing their log entries.)

Log messages for documentation or translation have somewhat looser guidelines. The requirement to name every symbol obviously does not apply, and if the change is just one more increment in a continuous process such as translation, it's not even necessary to name every file. Just briefly summarize the change, for example: "More work on Malagasy translation." Please write your log messages in English, so everybody involved in the project can understand the changes you made.

If you're using a 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, after the 'On the 'xxx' branch notice', such as:

   *** checkpoint commit -- please don't waste your time reviewing it ***

And if a later commit on that branch should 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.

Crediting ¶

It is very important to record code contributions in a consistent and parseable way. This allows us to write scripts to figure out who has been actively contributing — and what they have contributed — so we can spot potential new committers quickly. The Subversion project uses human-readable but machine-parseable fields in log messages to accomplish this.

When committing a patch written by someone else, use "Patch by: " at the beginning of a line to indicate the author:

   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.

   Patch by: J. Random <jrandom@example.com>

If multiple individuals wrote the patch, list them each on a separate line — making sure to start each continuation line with whitespace. Non-committers should be listed by name, if known, and e-mail. Full and partial committers should be listed by their canonical usernames from COMMITTERS (the leftmost column in that file). Additionally, "me" is an acceptable shorthand for the person actually committing the change.

   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.

   Patch by: J. Random <jrandom@example.com>
             Enrico Caruso <codingtenor@codingtenor.com>
             jcommitter
             me

If someone found the bug or pointed out the problem, but didn't write the patch, indicate their contribution with "Found by: " (or "Reported by: "):

   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.

   Found by: J. Random <jrandom@example.com>

If someone suggested something useful, but didn't write the patch, indicate their contribution with "Suggested by: ":

   Extend the Contribulyzer syntax to distinguish finds from ideas.

   * www/hacking.html (crediting): Adjust accordingly.

   Suggested by: dlr

If someone test-drove a patch, use "Tested by: ":

    Fix issue #23: random crashes on FreeBSD 3.14.
    
    Tested by: Old Platformer
    (I couldn't reproduce the problem, but Old hasn't seen any crashes since
    he applied the patch.)
    
    * subversion/libsvn_fs_sieve/obliterate.c
      (cover_up): Account for sieve(2) returning 6.

If someone reviewed the change, use "Review by: " (or "Reviewed by: " if you prefer):

   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.

   Review by: Eagle Eyes <eeyes@example.com>

A field may have multiple lines, and a log message may contain any combination of fields:

   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.

   Patch by: J. Random <jrandom@example.com>
             Enrico Caruso <codingtenor@codingtenor.com>
             me
   Found by: J. Random <jrandom@example.com>
   Review by: Eagle Eyes <eeyes@example.com>
              jcommitter

Further details about a contribution should be listed in a parenthetical aside immediately after the corresponding field. Such an aside always applies to the field right above it; in the following example, the fields have been spaced out for readability, but note that the spacing is optional and not necessary for parseability:

   Fix issue #1729: Don't crash because of a missing file.

   * subversion/libsvn_ra_ansible/get_editor.c
     (frobnicate_file): Check that file exists before frobnicating.

   Patch by: J. Random <jrandom@example.com>
   (Tweaked by me.)

   Review by: Eagle Eyes <eeyes@example.com>
              jcommitter
   (Eagle Eyes caught an off-by-one-error in the basename extraction.)

Currently, these fields

   Patch by:
   Suggested by:
   Found by:
   Review by:
   Tested by:

are the only officially-supported crediting fields (where "supported" means scripts know to look for them), and they are widely used in Subversion log messages. Future fields will probably be of the form "VERB by: ", and from time to time someone may use a field that sounds official but really is not — for example, there are a few instances of "Inspired by: ". These are okay, but try to use an official field, or a parenthetical aside, in preference to creating your own. Also, don't use "Reported by: " when the reporter is already recorded in an issue; instead, simply refer to the issue.

Look over Subversion's existing log messages to see how to use these fields in practice. This command from the top of your trunk working copy will help:

svn log | contrib/client-side/search-svnlog.pl "(Patch|Review|Suggested) by: "

Note: The "Approved by: " field seen in some commit messages is totally unrelated to these crediting fields, and is generally not parsed by scripts. It is simply the standard syntax for indicating either who approved a partial committer's commit outside their usual area, or (in the case of merges to release branches) who voted for the change to be merged.

Github ¶

The Subversion repository is mirrored to GitHub at https://github.com/apache/subversion/.

Some users might create pull requests in GitHub. If the code is committed to the Subversion repository, make sure to include text in the log message to automatically close the pull request:

This fixes #NNN in GitHub

To manage pull requests without committing code, you must have a GitHub account connected to your ASF id and you must have the triager role assigned to your account by ASF Infra.

The configuration/build system under unix ¶

Greg Stein wrote a custom build system for Subversion, which had been using `automake' and recursive Makefiles. Now it uses a single, top-level Makefile, generated from Makefile.in (which is kept under revision control). `Makefile.in' in turn includes `build-outputs.mk', which is automatically generated from `build.conf' by the `gen-make.py' script. Thus, the latter two are under revision control, but `build-outputs.mk' is not.

Here is Greg's original mail describing the system, followed by some advice about hacking it:

   From: Greg Stein <gstein@lyra.org>
   Subject:  new build system (was: Re: CVS update: MODIFIED: ac-helpers ...)
   To: dev@subversion.tigris.org
   Date: Thu, 24 May 2001 07:20:55 -0700
   Message-ID: <20010524072055.F5402@lyra.org>

   On Thu, May 24, 2001 at 01:40:17PM -0000, gstein@tigris.org wrote:
   >   User: gstein
   >   Date: 01/05/24 06:40:17
   >
   >   Modified:    ac-helpers .cvsignore svn-apache.m4
   >   Added:       .        Makefile.in
   >   Log:
   >   Switch over to the new non-recursive build system.
   >...

   Okay... this is it. We're now on the build system.

       "It works on my machine."

   I suspect there may be some tweaks to make on different OSs. I'd be
   interested to hear if Ben can really build with normal BSD make. It
   should be possible.

   The code supports building, installation, checking, and
   dependencies. It does *NOT* yet deal with the doc/ subdirectory. That
   is next; I figured this could be rolled out and get the kinks worked
   out while I do the doc/ stuff.  Oh, it doesn't build Neon or APR yet
   either. I also saw a problem where libsvn_fs wasn't getting built
   before linking one of the test proggies (see below).

   Basic operation: same as before.

   $ ./autogen.sh
   $ ./configure OPTIONS
   $ make
   $ make check
   $ make install

   There are some "make check" scripts that need to be fixed up. That'll
   happen RSN. Some of them create their own log, rather than spewing to
   stdout (where the top-level make will place the output into
   [TOP]/tests.log).

   The old Makefile.am files are still around, but I'll be tossing those
   along with a bunch of tweaks to all the .cvsignore files. There are a
   few other cleanups, too. But that can happen as a step two.

   [ $ cvs rm -f `find . -name Makefile.rm`

     See the mistake in that line? I didn't when I typed it. The find
     returned nothing, so cvs rm -f proceeded to delete my entire
     tree. And the -f made sure to delete all my source files, too. Good
     fugging thing that I had my mods in some Emacs buffers, or I'd be
     bitching.

     I am *so* glad that Ben coded SVN to *not* delete locally modified
     files *and* that we have an "undel" command. I had to go and tweak a
     bazillion Entries files to undo the delete...
   ]

   The top-level make has a number of shortcuts in it (well, actually in
   build-outputs.mk):

   $ make subversion/libsvn_fs/libsvn_fs.la

   or

   $ make libsvn_fs

   The two are the same. So... when your test proggie fails to link
   because libsvn_fs isn't around, just run "make libsvn_fs" to build it
   immediately, then go back to the regular "make".

   Note that the system still conditionally builds the FS stuff based
   on whether DB (See 'Building on Unix' below) is available, and
   mod_dav_svn if Apache is available.

   Handy hint: if you don't like dependencies, then you can do:

   $ ./autogen.sh -s

   That will skip the dependency generation that goes into
   build-outputs.mk. It makes the script run quite a bit faster (48 secs
   vs 2 secs on my poor little Pentium 120).

   Note that if you change build.conf, you can simply run:

   $ ./gen-make.py build.conf

   to regen build-outputs.mk. You don't have to go back through the whole
   autogen.sh / configure process.

   You should also note that autogen.sh and configure run much faster now
   that we don't have the automake crap. Oh, and our makefiles never
   re-run configure on you out of the blue (gawd, I hated when automake
   did that to me).

   Obviously, there are going to be some tweaky things going on. I also
   think that the "shadow" builds or whatever they're called (different
   source and build dirs) are totally broken. Something tweaky will have
   to happen there.  But, thankfully, we only have one Makefile to deal
   with.

   Note that I arrange things so that we have one generated file
   (build-outputs.mk), and one autoconf-generated file (Makefile from
   .in).  I also tried to shove as much logic/rules into
   Makefile.in. Keeping build-outputs.mk devoid of rules (thus, implying
   gen-make.py devoid of rules in its output generation) manes that
   tweaking rules in Makefile.in is much more approachable to people.

   I think that is about it. Send problems to the dev@ list and/or feel
   free to dig in and fix them yourself. My next steps are mostly
   cleanup. After that, I'm going to toss out our use of libtool and rely
   on APR's libtool setup (no need for us to replicate what APR already
   did).

   Cheers,
   -g

   --
   Greg Stein, http://www.lyra.org/

And here is some advice for those changing or testing the configuration/build system:

   From: Karl Fogel <kfogel@collab.net>
   To: dev@subversion.tigris.org
   Subject: when changing build/config stuff, always do this first
   Date: Wed 28 Nov 2001

   Yo everyone: if you change part of the configuration/build system,
   please make sure to clean out any old installed Subversion libs
   *before* you try building with your changes.  If you don't do this,
   your changes may appear to work fine, when in fact they would fail if
   run on a truly pristine system.

   This script demonstrates what I mean by "clean out".  This is
   `/usr/local/cleanup.sh' on my system.  It cleans out the Subversion
   libs (and the installed httpd-2.0 libs, since I'm often reinstalling
   that too):

      #!/bin/sh

      # Take care of libs
      cd /usr/local/lib || exit 1
      rm -f APRVARS
      rm -f libapr*
      rm -f libexpat*
      rm -f libneon*
      rm -f libsvn*

      # Take care of headers
      cd /usr/local/include || exit 1
      rm -f apr*
      rm -f svn*
      rm -f neon/*

      # Take care of headers
      cd /usr/local/apache2/lib || exit 1
      rm -f *

   When someone reports a configuration bug and you're trying to
   reproduce it, run this first. :-)

   The voice of experience,
   -Karl

Automated tests ¶

For a description of how to use and add tests to Subversion's automated test framework, please read subversion/tests/README and subversion/tests/cmdline/README.

Build farm ¶

The ASF Infrastructure team manages a BuildBot build/test farm. The Buildbot waterfall for the Subversion project is located here:

• Production builds
• Backport and RAT status
• Compatibility tests
• The whole farm

For more information about build services, head over to ci2.apache.org.

If you'd like to receive notifications about buildbot build and test failures, please subscribe to the notifications@ mailing list.

Buildbot is configured in the Infra repository, specifically, the subversion.py file.

Writing test cases before code ¶

From: Karl Fogel <kfogel@collab.net>
Subject: writing test cases
To: dev@subversion.tigris.org
Date: Mon, 5 Mar 2001 15:58:46 -0600

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...).

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. :-)

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.

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

Debugging with SVN_DBG ¶

The SVN_DBG debugging tool is a C Preprocessor macro that sends debugging output to stdout (by default) or stderr whilst not interfering with the SVN test suite.

It provides an alternative to a debugger such as gdb, or alternatively, extra information to assist in the use of a debugger. It might be especially useful in situations where a debugger cannot be used.

The svn_debug module contains two debug aid macros that print the file:line of the call and printf-like arguments to the #SVN_DBG_OUTPUT stdio stream (#stdout by default):

SVN_DBG( ( const char *fmt, ...) ) /* double braces are neccessary */

SVN_DBG_PROPS( ( apr_hash_t *props, const char *header_fmt, ...) )

Controlling SVN_DBG output:

• SVN_DBG is enabled whenever svn is configured with --enable-maintainer-mode.
• The SVN test suite turns off SVN_DBG output automatically, to suppress the output manually, set the SVN_DBG_QUIET variable to 1 in your shell environment.
• When you are done, please be sure to remove any instances of the SVN_DBG and SVN_DBG_PROPS macros from any code you are committing and from any patch that you send to the list. (AKA: Do not forget a scalpel in the patient!)

The SVN_DBG macro definitions and code are located in:

• subversion/include/private/svn_debug.h
• subversion/libsvn_subr/debug.c

Sample patch showing usage of the SVN_DBG macro:

Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c     (revision 1476635)
+++ subversion/libsvn_fs_fs/fs_fs.c     (working copy)
@@ -2303,6 +2303,9 @@ get_node_revision_body(node_revision_t **noderev_p
 
   /* First, try a cache lookup. If that succeeds, we are done here. */
   SVN_ERR(get_cached_node_revision_body(noderev_p, fs, id, &is_cached, pool));
+  SVN_DBG(("Getting %s from: %s\n", 
+           svn_fs_fs__id_unparse(id),
+           is_cached ? "cache" : "disk"));
   if (is_cached)
     return SVN_NO_ERROR;

Sample patch showing usage of the SVN_DBG_PROPS macro:

Index: subversion/svn/proplist-cmd.c
===================================================================
--- subversion/svn/proplist-cmd.c	(revision 1475745)
+++ subversion/svn/proplist-cmd.c	(working copy)
@@ -221,6 +221,7 @@ svn_cl__proplist(apr_getopt_t *os,
                                       URL, &(opt_state->start_revision),
                                       &rev, ctx, scratch_pool));
+      /*  this can be called with svn proplist  --revprop -r <rev> */
+      SVN_DBG_PROPS((proplist,"The variable apr_hash_t *proplist contains: "));
       if (opt_state->xml)
         {
           svn_stringbuf_t *sb = NULL;

Debugging the server ¶

   % gdb httpd
   (gdb) run -X
   ^C
   (gdb) break some_func_in_mod_dav_svn
   (gdb) continue

   /usr/local/apache2/logs/error_log
   /usr/local/apache2/logs/access_log

  svn: Malformed network data
  svn: Connection closed unexpectedly

  % 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");

  % gdb svnserve
  (gdb) break marshal.c:NNN
  (gdb) run -X

Tracing network traffic ¶

Tracing the network traffic between the client and the server can be helpful in debugging. There are several ways to go about doing a network trace (and this list is not exhaustive).

You may well want to disable compression when doing a network trace — see the http-compression parameter in the servers configuration file.

svn ls http://example.com/svn/repos/trunk \
    --config-option servers:global:http-proxy-host=localhost \
    --config-option servers:global:http-proxy-port=9630 \
    --config-option servers:global:http-compression=no

Tracking down memory leaks ¶

Our use of APR pools makes it unusual for us to have memory leaks in the strictest sense; all the memory we allocate will be cleaned up eventually. But sometimes an operation takes more memory than it should; for instance, a checkout of a large source tree should not use much more memory than a checkout of a small source tree. When that happens, it generally means we're allocating memory from a pool whose lifetime is too long.

If you have a favorite memory leak tracking tool, you can configure with --enable-pool-debug (which will make every pool allocation use its own malloc()), arrange to exit in the middle of the operation, and go to it. If not, here's another way:

• Configure with --enable-pool-debug=verbose-alloc. Make sure to rebuild all of APR and Subversion so that every allocation gets file-and-line information.

• Run the operation, piping stderr to a file. Hopefully you have lots of disk space.

• In the file, you'll see lots of lines which look like:

      POOL DEBUG: [5383/1024] PCALLOC (      2763/      2763/      5419) \
      0x08102D48 "subversion/svn/main.c:612"                             \
      <subversion/libsvn_subr/auth.c:122> (118/118/0)
     

  What you care about most is the tenth field (the one in quotes), which gives you the file and line number where the pool for this allocation was created. Go to that file and line and determine the lifetime of the pool. In the example above, main.c:612 indicates that this allocation was made in the top-level pool of the svn client. If this were an allocation which was repeated many times during the course of an operation, that would indicate a source of a memory leak. The eleventh field (the one in brackets) gives the file and line number of the allocation itself.

Mailing Lists ¶

• Where to post
• When to post
• Formatting
• Replying
• Sending patches
• Languages and encodings

The advice below is based on years of experience with the Subversion mailing lists, and addresses the problems seen most frequently on those lists. It should not be taken as a complete guide to mailing list etiquette — you can find one of those on the Net pretty easily if you want one.

If you follow these conventions when posting to our mailing lists, your post is much more likely to be read and answered.

   Blue asparagus
     |
     |_ Re: Blue asparagus
         |
         |_ Yellow elephants (was: Re: Blue asparagus)    <-- ### switch ###
            |
            |_ Re: Yellow elephants

Bugs / Issues ¶

• How to report a bug
• Where to report a bug
• Milestone management
• Issue triage
• Security

Subversion isn't perfect software. It contains bugs, lacks features, and has room for improvement like any other piece of software. Like most software projects, the Subversion project makes use of an issue tracking tool to manage known outstanding issues with the software. But perhaps unlike most software projects, we try to keep our issue tracker relatively free of debris. It's not that we don't want to hear about Subversion's problems — after all, we can't fix what we don't know is broken. It's just that we've found a mismanaged issue tracker to be more of a hindrance than a help.

This mail pretty much says it all (except that nowadays you should post to the users@ list before posting to the dev@ list):

   From: Karl Fogel <kfogel@collab.net>
   Subject: Please ask on the list before filing a new issue.
   To: dev@subversion.tigris.org
   Date: Tue, 30 Jul 2002 10:51:24 (CDT)
   
   Folks, we're getting tons of new issues, which is a Good Thing in
   general, but some of them don't really belong in the issue tracker.
   They're things that would be better solved by a quick conversation
   here on the dev list.  Compilation problems, behavior questions,
   feature ideas that have been discussed before, that sort of thing.
   
   *Please* be more conservative about filing issues.  The issues
   database is physically much more cumbersome than email.  It wastes
   people's time to have conversations in the issues database that should
   be had in email.  (This is not a libel against the issue tracker, it's
   just a result of the fact that the issues database is for permanent
   storage and flow annotation, not for real-time conversation.)
   
   If you encounter a situation where Subversion is clearly behaving
   wrongly, or behaving opposite to what the documentation says, then
   it's okay to file the issue right away (after searching to make sure
   it isn't already filed, of course!).  But if you're
   
      a) Requesting a new feature, or
      b) Having build problems, or
      c) Not sure what the behavior should be, or
      d) Disagreeing with current intended behavior, or
      e) Not TOTALLY sure that others would agree this is a bug, or
      f) For any reason at all not sure this should be filed,
   
   ...then please post to the dev list first.  You'll get a faster
   response, and others won't be forced to use the issues database to
   have the initial real-time conversations.
   
   Nothing is lost this way.  If we eventually conclude that it should be
   in the issue tracker, then we can still file it later, after the
   description and reproduction recipe have been honed on the dev list.
   
   Thank you,
   -Karl

The following are the policies that we ask folks to abide by when reporting problems or requested enhancements to Subversion.

    for i in issues_marked_as("---"):
      if issue_is_a_dup_of_some_other_issue(i):
        close_as_dup(i)
      elif issue_is_invalid(i):
        # A frequent reason for invalidity is that the reporter
        # did not follow the "buddy system" for filing.
        close_as_invalid(i)
      elif issue_already_fixed(i):
        version = fixed_in_release(i)
        move_to_milestone(i, version)
        close_as_fixed(i)
      elif issue_unreproducible(i):
        close_as_worksforme(i)
      elif issue_is_real_but_we_won't_fix_it(i):
        close_as_wontfix(i)
      elif issue_is_closeable_for_some_other_reason(i):
        close_it_for_that_reason(i)

      # Else issue should remain open, so DTRT with it...

      # Set priority, environment, component, etc, as needed.
      adjust_all_fields_that_need_adjustment(i)

      # Figure out where to put it.
      if issue_is_a_lovely_fantasy(i):
        move_to_milestone(i, "blue-sky")
      if issue_is_not_important_enough_to_block_any_particular_release(i):
        move_to_milestone(i, "nonblocking")
      elif issue_resolution_would_require_incompatible_changes(i):
        move_to_milestone(i, "2.0-consider")
      elif issue_hurts_people_somewhat(i):
        move_to_milestone(i, "1.6-consider")  # or whatever
      elif issue_hurts_people_a_lot(i):
        move_to_milestone(i, "1.5-consider")
      elif issue_hurts_and_hurts_and_should_block_the_release(i):
        move_to_milestone(i, "1.5")

Making Subversion Releases ¶

• Subversion release process
• Release numbering
• Inter-release compatibility
• Custom releases
• Deprecation
• Stabilizing and maintaining releases
• Creating a Subversion release
• Notify translators
• Preparing to roll a release
• Rolling a release
• Signing source distribution packages (a.k.a tarballs)
• The actual releasing
• Creating and maintaining release branches
• Preparing to create a new minor release branch
• Creating a new minor release branch
• Porting changes to a release branch
• Managing the CHANGES file
• Writing the initial content for a branch
• Adding content for patch release
• How not to make a Subversion release

This document can generally be broken into three parts, in increasing order of specificity:

• High-level release planning and policies, such as version numbering and compatibility. This answers questions like "When does a tarball get created?" and "What should be the content of a tarball?"
• What steps to take when it is time to create a release. This section addresses the question of "How do I manage a release?"
• How to construct a set of release tarballs. This section discusses the steps required to go from source code in the repository to a set of distributable .tar.gz or .zip files with the desired content.

Subversion release managers use a set of steps codified in a Python script named release.py. This script can be used to perform most of the automated steps in the release process. Run it with the -h option to get more information.

In addition to the following project-specific guidelines, aspiring release managers may also want to read up on the general Apache release policies. They can seem a bit patchwork at times, but give a good idea of general best practice and how Subversion fits in the larger ASF ecosystem.

   1.0.1  -->  first stable patch release of 1.0
   1.1.0  -->  next stable minor release of 1.x after 1.0.x
   1.1.1  -->  first stable patch release of 1.1.x
   1.1.2  -->  second stable patch release of 1.1.x
   1.2.0  -->  next stable minor release after that

   version 1.7.1-dev (under development)

   /**
    * Similar to svn_repos_dump_fs3(), but with a @a feedback_stream instead of
    * handling feedback via the @a notify_func handler
    *
    * @since New in 1.1.
    * @deprecated Provided for backward compatibility with the 1.6 API.
    */
   SVN_DEPRECATED
   svn_error_t *
   svn_repos_dump_fs2(svn_repos_t *repos,
                      svn_stream_t *dumpstream,
                      svn_stream_t *feedback_stream,
                      svn_revnum_t start_rev,
                      svn_revnum_t end_rev,
                      svn_boolean_t incremental,
                      svn_boolean_t use_deltas,
                      svn_cancel_func_t cancel_func,
                      void *cancel_baton,
                      apr_pool_t *pool);

   * r98765 (issue #56789)
     Make commit editor take a closure object for future mindreading.
     Justification:
       API stability, as prep for future enhancement.
     Branch: 1.8.x-r98765
     Notes:
       There was consensus on the desirability of this feature in
       the near future; see thread at http://... (Message-Id: blahblah).
       Merge with --accept=mc.
     Concerns:
       Vetoed by jerenkrantz due to privacy concerns with the
       implementation; see thread at http://... (Message-Id: blahblah)
     Votes:
       +1: ghudson, bliss
       +0: cmpilato
       -0: gstein
       -1: jerenkrantz

 * r31833
   svndumpfilter: Don't match prefixes to partial path components.
   Fixes #desc4 of issue #1853.
   Votes:
     +1: danielsh, hwright
     +1 (non-binding): stylesen

   * r30643, r30653, r30785
     Update bash completion script.
     Votes:
       +1: arfrever (r30785 only), stylesen

sed -e '/^ *Translat.*:$/,/:$/!d' -e 's/^ *[^ ]* *\([^>]*>\).*/\1/;t;d' COMMITTERS

./tools/po/l10n-report.py

mkdir -p /opt/svnrm && cd /opt/svnrm && $SVN_SRC_DIR/tools/dist/release.py build-env X.Y.Z

./release.py roll X.Y.Z 1234

    a) tar zxvf subversion-X.Y.Z.tar.gz;  cd subversion-X.Y.Z
    b) ./configure
 
       See INSTALL, section III.B for detailed instructions on
       configuring/building Subversion.
 
       If you installed Apache in some place other than the default, as
       mentioned above, you will need to use the same
       --prefix=/usr/local/apache2 option as used to configure Apache.
 
       You may also want to use --enable-mod-activation, which will
       automatically enable the required Subversion modules in the
       Apache config file.
 
    c) make
    d) make check
    e) make install (this activates mod_dav)
    f) make davcheck
 
       For this, start up Apache after having configured according to
       the directions in subversion/tests/cmdline/README.
 
       Make sure, that if you maintain a development installation of
       apache, that you check the config file and update it for the
       new release area where you're testing the tar-ball.
 
       (Unless you rename the tree which gets extracted from the
       tarball to match what's in httpd.conf, you will need to edit
       httpd.conf)
 
    g) make svncheck
 
       First, start up svnserve with these args:
 
          $ subversion/svnserve/svnserve -d -r \
            `pwd`/subversion/tests/cmdline
 
       -d tells svnserve to run as a daemon
       -r tells svnserve to use the following directory as the
          logical file system root directory.
 
       After svnserve is running as a daemon 'make svncheck' should run
 
    h) Then test that you can check out the Subversion repository
       with this environment:
 
          subversion/svn/svn co https://svn.apache.org/repos/asf/subversion/trunk
 
    i) Verify that the perl, python, and ruby swig bindings at least compile.
       If you can't do this, then have another developer verify.
 
       (see bindings/swig/INSTALL for details)
 
       Ensure that ./configure detected a suitable version of swig,
       perl, python, and ruby.  Then:
 
          make swig-py
          make check-swig-py
          sudo make install-swig-py
 
          make swig-pl
          make check-swig-pl
          sudo make install-swig-pl
 
          make swig-rb
          make check-swig-rb
          sudo make install-swig-rb

    j) Verify that the javahl bindings at least compile.
       If you can't do this, then have another developer verify.
 
       (see subversion/bindings/javahl/README for details)
 
       Ensure that ./configure detected a suitable jdk, and then
       possibly re-run with '--enable-javahl', '--with-jdk=' and
       '--with-junit=':
 
          make javahl
          sudo make install-javahl
          make check-javahl

    k) Verify that the ctypes python bindings at least compile.
       If you can't do this then have another developer verify.

       (see subversion/bindings/ctypes-python/README for details)

       Ensure that ./configure detected a suitable ctypesgen, and
       then possibly re-run with '--with-ctypesgen':

          make ctypes-python
          sudo make install-ctypes-python
          make check-ctypes-python 

    l) Verify that get-deps.sh works and does not emit any errors.

          ./get-deps.sh

    release.py sign-candidates X.Y.Z

    gpg -ba subversion-X.Y.Z.tar.bz2
    gpg -ba subversion-X.Y.Z.tar.gz
    gpg -ba subversion-X.Y.Z.zip

    release.py create-tag X.Y.Z 1234

release.py post-candidates 1.7.0

    release.py get-keys
    release.py --target /path/to/dist/dev/subversion/wc check-sigs 1.7.0-rc4

    gpg -ba -o - subversion-1.7.0-rc4.tar.bz2 >> subversion-1.7.0-rc4.tar.bz2.asc

 release.py --target /path/to/dist/dev/subversion/wc sign-candidates 1.7.0-rc4

    bzip2 -cd subversion-1.7.0-rc4.tar.bz2 \
    | gzip -9n > subversion-1.7.0-rc4.tar.gz

release.py move-to-dist 1.7.0

release.py clean-dist

curl -u USERNAME "https://reporter.apache.org/addrelease.py?date=`date +%s`&committee=subversion&version=VERSION&xdate=`date +%F`"

release.py --verbose create-release-branch A.B.0

release.py --verbose write-release-notes A.B.0 > .../docs/release-notes/A.B.html
svn add .../docs/release-notes/A.B.html
svn ci -m "Add release notes template." .../docs/release-notes/A.B.html

svn checkout --depth=empty https://svn-master.apache.org/repos/asf/subversion/branches ~svnsvn/src/svn
svn up ~svnsvn/src/svn/A.B.x  # for each supported branch

sudo -H -u svnsvn  svn up ~svnsvn/src/svn/A.B.x

sudo -H -u svnsvn  svn up -r0 ~svnsvn/src/svn/Z.Z.x

release.py write-changelog

Localization (l10n) ¶

• Localization overview
• Software version requirements
• Starting a new translation
• Verifying your po file
• Submitting your po file
• Updating existing po files
• Maintenance on branches
• Requirements for po and mo files
• Conventions for the empty string msgid section
• Translation teams
• Single versus double quotes
• Error message conventions

Translation has been divided into two domains. First, there is the translation of server messages sent to connecting clients. This issue has been punted for now. Second there is the translation of the client and its libraries.

   #include "svn_private_config.h"

  # SOME DESCRIPTIVE TITLE.
  # Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
  # This file is distributed under the same license as the PACKAGE package.
  # FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.

  # <Your language> translation for subversion package
  #    Licensed to the Apache Software Foundation (ASF) under one
  #    or more contributor license agreements.  See the NOTICE file
  #    distributed with this work for additional information
  #    regarding copyright ownership.  The ASF licenses this file
  #    to you under the Apache License, Version 2.0 (the
  #    "License"); you may not use this file except in compliance
  #    with the License.  You may obtain a copy of the License at
  #
  #      http://www.apache.org/licenses/LICENSE-2.0
  #
  #    Unless required by applicable law or agreed to in writing,
  #    software distributed under the License is distributed on an
  #    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
  #    KIND, either express or implied.  See the License for the
  #    specific language governing permissions and limitations
  #    under the License.

  "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
  "Language-Team: LANGUAGE <LL@li.org>\n"

  "Last-Translator: Subversion Developers <dev@subversion.apache.org>\n"
  "Language-Team: YOUR LANGUAGE <dev@subversion.apache.org>\n"

    make locale-gnu-po-update

    make locale-gnu-po-update PO=ll

  svn cat -r X ^/subversion/trunk/subversion/po/YY.po | \
    po-merge.py YY.po