Context Navigation

← Previous Change
Wiki History
Next Change →

Changes between Version 6 and Version 7 of DeveloperDocumentationNew

Timestamp:: 02/07/17 13:46:00 (7 years ago)
Author:: Samuli Seppänen
Comment:: --

Legend:

: Unmodified
: Added
: Removed
: Modified

DeveloperDocumentationNew

-                      v6
+                      v7
 [[TOC(inline, depth=1)]]
+= Introduction =
+This page outlines the development practices for the [https://github.com/OpenVPN/openvpn main OpenVPN project]. The information here is mostly applicable to our other subprojects (e.g. [https://github.com/OpenVPN/openvpn-gui openvpn-gui] and [https://github.com/OpenVPN/openvpn-build openvpn-build]), except that they allow !GitHub pull requests unlike in the main project.
 = Developer communication channels =
 …
 = Development process =
 The basic development process we follow is outlined in this diagram:
+The basic development process is outlined in this diagram:
 [[Image(wiki:DeveloperDocumentation:getting_code_to_openvpn.png,100%)]]
 In a nutshell:
+In text format:
  * Patch is sent to "openvpn-devel" mailing list for public review
 …
  * 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.
+!GitHub pull requests can be used to ''preliminary'' review especially for large patchsets. The final patches, however, ''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.
+* Patches should generally be written against the Git "master" branch.
+* Patches must apply cleanly, without merge conflicts.
+* Patch must pass all applicable automated testing (Buildbot and/or Travis)
 * 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.
 …
    * indent-with-tabs nil
 OpenVPN 2.4 uses the Allman style:
+OpenVPN 2.4 uses the Allman style ([wiki:CodeStyle full documentation]):
  * Indentation is 4 spaces, no tabs ever.
 …
  * Only use /* */-style comments
+Full documentation of the OpenVPN 2.4 style is on the [wiki:CodeStyle CodeStyle] page.
+== Notes on 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, don't hesitate to provide a patch fixing that.. He're are a few links to relevant resources:
+ * [http://mywiki.wooledge.org/Bashism Bashism - Greg's Wiki]
+ * [http://sourceforge.net/projects/checkbaskisms/ Checkbashisms]: a tool to check for "Bashisms". Also included in Debian/Ubuntu "devscripts" package.
+ * [http://gondor.apana.org.au/~herbert/dash DASH]: a minimal shell with "POSIX-compliant features only", probably useful for testing
 == Commit message ==
 …
 = Sending patches =
 We strongly encourage using [https://git-scm.com/docs/git-send-email git send-email] for sending the patches, so that your email client does not mess the formatting.
+We strongly encourage configuring and using [https://git-scm.com/docs/git-send-email git send-email] for sending the patches, so that your email client does not mess the formatting. For example, to send your latest commit as email:
+== Shell scripts ==
+{{{
+$ git send-email --to=openvpn-devel@lists.sourceforge.net HEAD~1...HEAD
+}}}
+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:
+ * [http://mywiki.wooledge.org/Bashism Bashism - Greg's Wiki]
+ * [http://sourceforge.net/projects/checkbaskisms/ Checkbashisms]: a tool to check for "Bashisms". Also included in Debian/Ubuntu "devscripts" package.
+ * [http://gondor.apana.org.au/~herbert/dash DASH]: a minimal shell with "POSIX-compliant features only", probably useful for testing
+= Code repositories =
+See CodeRepositories.
+= Practical issues =
+== Using Git ==
+= Using Git =
 If you're unfamiliar with git in general, take a look at these links:
 …
  * [http://git.or.cz/course/svn.html git for SVN users]
 == Bisecting commits to detect introduction of a bug ==
+= Code repositories =
+In most cases, it's easiest to use [http://book.git-scm.com/5_finding_issues_-_git_bisect.html 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 [ticket:190 here]. In practice, you can reset to an earlier commit with
+See CodeRepositories page.
+{{{
+$ git reset --hard <commit-id>
+}}}
+= Practical issues =
+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:
+ * http://blog.repl.ca/2009/06/converting-svn-commits-to-git-patches.html
+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 [http://thread.gmane.org/gmane.network.openvpn.devel/4869 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 [http://research.microsoft.com/en-us/projects/symdiff Symdiff].
+See DeveloperTips page.