wiki:DeveloperDocumentationNew

Version 6 (modified by Samuli Seppänen, 7 years ago) (diff)

--

Table of Contents

  1. Developer communication channels
  2. Development process
  3. Making patches
    1. Basic requirements
    2. Code style
    3. Commit message
  4. Sending patches
    1. Shell scripts
  5. Code repositories
  6. Practical issues
    1. Using Git
    2. Bisecting commits to detect introduction of a bug
    3. Applying patches from emails
    4. Sending GitHub pull requests to the mailing list with git-send-email
    5. Converting SVN revisions to Git patches
    6. Poor-man's Symdiff

Developer communication channels

There are two primary developer channels:

  • ​openvpn-devel mailinglist: primary communication channel, subscription required
  • #openvpn-devel (at) irc.freenode.net: used for generic development discussions and the weekly IRC meetings

Development process

The basic development process we follow is outlined in this diagram:

In a nutshell:

  • Patch is sent to "openvpn-devel" mailing list for public review
  • Patch will be reviewed and either accepted (ACK) or rejected (NACK). The ACK consists of two parts:
    • Feature-ACK: does the patch make sense to have?
    • Code-ACK: does the patch meet our code quality standards?
  • Accepted patches go to the Git "master" branch

GitHub pull requests can be used to preliminary review, but the final patch must go to the openvpn-devel list.

Making patches

Basic requirements

  • Patches must apply cleanly, without merge conflicts. Please state which branch the patch is written against.
  • Large features may need #ifdefs so that they can be disabled on embedded systems. The developers will let you know if this is the case.
  • Man-page and Changes.rst should be updated as necessary.

Code style

OpenVPN 2.3 and earlier use mostly the following conventions:

  • Indentation is 2 spaces (no tabs)
  • Indentation follows GNU Coding Style
  • Emacs
    • c-style gnu
    • c-basic-offset 2
    • indent-with-tabs nil

OpenVPN 2.4 uses the Allman style:

  • Indentation is 4 spaces, no tabs ever.
  • Opening and closing brackets get their own line, and must match indentation.
  • Line length is 80 characters.
  • C99 is allowed. E.g. for (int i = 0; i < max; i++).
  • Only use /* */-style comments

Full documentation of the OpenVPN 2.4 style is on the CodeStyle page.

Commit message

Patches must have a short but descriptive one-line summary of the change, followed by a more descriptive text explaining in plain English why and how the patch is written. This description should not be too technical, as the patch itself will reveal the technical details. The patch should also conform to these rules (adapted from ​here):

  • Separate subject from body with a blank line
  • Limit the subject line to 50 characters
    • NOTE: this limit is under discussion
  • Capitalize the subject line
  • Do not end the subject line with a period
  • Use the imperative mood in the subject line
  • Wrap the body at 72 characters
    • NOTE: this limit is under discussion
  • Use the body to explain "what" and "why"

In addition:

  • Add a 'Signed-off-by:' line at the end (git commit --signoff).
  • Everyone who has contributed to this patch should be mentioned out of courtesy. Use a valid e-mail address and the full name whenever possible.

Sending patches

We strongly encourage using ​git send-email for sending the patches, so that your email client does not mess the formatting.

Shell scripts

Patches to shell scripts such as those in easy-rsa should be POSIX-compliant for portability reasons. If the existing scripts don't fulfill this requirement, please provide a patch :). He're are a few links to relevant resources:

Code repositories

See CodeRepositories.

Practical issues

Using Git

If you're unfamiliar with git in general, take a look at these links:

Bisecting commits to detect introduction of a bug

In most cases, it's easiest to use ​git-bisect to find the commit which introduced a problem. However, if the buildsystem is not in perfect working order all the way through from last known good commit to known bad, you may need to do manual bisecting such as was done here. In practice, you can reset to an earlier commit with 

$ git reset --hard <commit-id>

Next build and test. If it still fails, move further back in history and retry, until you find a version that works. Optimally, you should bisecting at the middle of commits between known good and known bad, then repeat the procedure until you pinpoint the bad commit.

If you have hunch which commit might have introduced the bug, you can try reverting it to see what happens: 

git revert <commit-id>

In case no later commits conflict with the commit, this will work. If there are conflicts, fix them manually or abort the revert and write a reverting commit manually.

Applying patches from emails

It's often necessary to test individual patches sent to a mailing list. Patches that are real attachments are trivial to download and merge. However, sometimes the patches are stored inline, in the message body, which makes thing slightly more difficult. If you're running Mozilla Thunderbird, you export emails containing inline emails as mbox file using ImportExportTools? add-on. These can then be applied to a git repository using git am <mbox-filename>.

Sending GitHub pull requests to the mailing list with git-send-email

Getting a "git am"-compatible patch out of a GitHub pull requests is simple: 

$ wget https://github.com/OpenVPN/openvpn/pull/<pr-number>.patch

You can then apply the patch: 

$ git am <pr-number>.patch

Then you should amend the commit message to add information and to fix errors (if any):

  • What pull request the patch was created from
  • Who ACKed the patch
  • Who relayed the patch (in case if you're not the author)
  • Fix formatting issues

And example below: 

$ git commit -s --amend
Update contrib/pull-resolv-conf/client.up for no DOMAIN

When no DOMAIN is received from push/pull, do not add either domain or
search to the resolv.conf. Fix typo in comment resolv.con[f]. Only add
new line when using domain or search.

URL: https://github.com/OpenVPN/openvpn/pull/34
Acked-by: Steffan Karger <steffan@karger.me>
Signed-off-by: Samuli Seppänen <samuli@openvpn.net>

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Author:    Jeffrey Cutter <jeff_m_cutter@yahoo.com>
# Date:      Sat Sep 12 20:03:18 2015 -0400
...

After adjusting the commit message you can send the patch using git-send-email: 

$ git-send-email --to=openvpn-devel@lists.sourceforge.net HEAD^1...HEAD

Adjust as necessary if you there is more than one patch.

Converting SVN revisions to Git patches

There's a Python script available that converts SVN revisions into patches git am can understand:

To use this script, copy it into the SVN repository root as svnrev2git.py. Then create an authors file using this format: 

svnusername, firstname lastname, email

For example: 

johndoe, John Doe, john.doe@domain.com

To use the script, call it like this: 

$ python svnrev2git.py authors <revision>
$ python svnrev2git.py authors <revision_start>-<revision_end>

For example: 

$ python svnrev2git.py authors 8126
$ python svnrev2git.py authors 8126-8225
$ for REV in 8206 8212 8219 8225; do python svnrev2git.py authors $REV;done

Note that the script is slow in processing long revision ranges: it's usually a better idea to pick the required revisions by hand.

Poor-man's Symdiff

Andj came up with a clever script in ​an IRC meeting to generate diffs that make reviewing refactoring patches easier: 

#!/bin/bash
git diff $1 $2 >/tmp/difftmp123.txt
cat /tmp/difftmp123.txt |grep "^-" |sed s/^-// >/tmp/removed123.txt
cat /tmp/difftmp123.txt |grep "^+" |sed s/^+// >/tmp/added123.txt
diff /tmp/removed123.txt /tmp/added123.txt -u

This is similar to ​Symdiff.