Changes between Version 6 and Version 7 of DeveloperDocumentationNew


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

--

Legend:

Unmodified
Added
Removed
Modified
  • DeveloperDocumentationNew

    v6 v7  
    11[[TOC(inline, depth=1)]]
     2
     3= Introduction =
     4
     5This 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.
    26
    37= Developer communication channels =
     
    1014= Development process =
    1115
    12 The basic development process we follow is outlined in this diagram:
     16The basic development process is outlined in this diagram:
    1317
    1418[[Image(wiki:DeveloperDocumentation:getting_code_to_openvpn.png,100%)]]
    1519
    16 In a nutshell:
     20In text format:
    1721
    1822 * Patch is sent to "openvpn-devel" mailing list for public review
     
    2226 * Accepted patches go to the Git "master" branch
    2327
    24 !GitHub pull requests can be used to ''preliminary'' review, but the final patch must go to the openvpn-devel list.
     28!GitHub pull requests can be used to ''preliminary'' review especially for large patchsets. The final patches, however, ''must'' go to the openvpn-devel list.
    2529
    2630= Making patches =
     
    2832== Basic requirements ==
    2933
    30 * Patches must apply cleanly, without merge conflicts.  Please state which branch the patch is written against.
     34* Patches should generally be written against the Git "master" branch.
     35* Patches must apply cleanly, without merge conflicts.
     36* Patch must pass all applicable automated testing (Buildbot and/or Travis)
    3137* 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.
    3238* Man-page and ''Changes.rst'' should be updated as necessary.
     
    4349   * indent-with-tabs nil
    4450
    45 OpenVPN 2.4 uses the Allman style:
     51OpenVPN 2.4 uses the Allman style ([wiki:CodeStyle full documentation]):
    4652
    4753 * Indentation is 4 spaces, no tabs ever.
     
    5157 * Only use /* */-style comments
    5258
    53 Full documentation of the OpenVPN 2.4 style is on the [wiki:CodeStyle CodeStyle] page.
     59== Notes on Shell scripts ==
     60
     61Patches 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:
     62
     63 * [http://mywiki.wooledge.org/Bashism Bashism - Greg's Wiki]
     64 * [http://sourceforge.net/projects/checkbaskisms/ Checkbashisms]: a tool to check for "Bashisms". Also included in Debian/Ubuntu "devscripts" package.
     65 * [http://gondor.apana.org.au/~herbert/dash DASH]: a minimal shell with "POSIX-compliant features only", probably useful for testing
    5466
    5567== Commit message ==
     
    7486= Sending patches =
    7587
    76 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.
     88We 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:
    7789
    78 == Shell scripts ==
     90{{{
     91$ git send-email --to=openvpn-devel@lists.sourceforge.net HEAD~1...HEAD
     92}}}
    7993
    80 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:
    81 
    82  * [http://mywiki.wooledge.org/Bashism Bashism - Greg's Wiki]
    83  * [http://sourceforge.net/projects/checkbaskisms/ Checkbashisms]: a tool to check for "Bashisms". Also included in Debian/Ubuntu "devscripts" package.
    84  * [http://gondor.apana.org.au/~herbert/dash DASH]: a minimal shell with "POSIX-compliant features only", probably useful for testing
    85 
    86 = Code repositories =
    87 
    88 See CodeRepositories.
    89 
    90 = Practical issues =
    91 
    92 == Using Git ==
     94= Using Git =
    9395
    9496If you're unfamiliar with git in general, take a look at these links:
     
    100102 * [http://git.or.cz/course/svn.html git for SVN users]
    101103
    102 == Bisecting commits to detect introduction of a bug ==
     104= Code repositories =
    103105
    104 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
     106See CodeRepositories page.
    105107
    106 {{{
    107 $ git reset --hard <commit-id>
    108 }}}
     108= Practical issues =
    109109
    110 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.
    111 
    112 If you have hunch which commit might have introduced the bug, you can try reverting it to see what happens:
    113 
    114 {{{
    115 git revert <commit-id>
    116 }}}
    117 
    118 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.
    119 
    120 == Applying patches from emails ==
    121 
    122 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>''.
    123 
    124 == Sending !GitHub pull requests to the mailing list with git-send-email ==
    125 
    126 Getting a "git am"-compatible patch out of a !GitHub pull requests is simple:
    127 {{{
    128 $ wget https://github.com/OpenVPN/openvpn/pull/<pr-number>.patch
    129 }}}
    130 You can then apply the patch:
    131 {{{
    132 $ git am <pr-number>.patch
    133 }}}
    134 Then you should amend the commit message to add information and to fix errors (if any):
    135 
    136 * What pull request the patch was created from
    137 * Who ACKed the patch
    138 * Who relayed the patch (in case if you're not the author)
    139 * Fix formatting issues
    140 
    141 And example below:
    142 
    143 {{{
    144 $ git commit -s --amend
    145 Update contrib/pull-resolv-conf/client.up for no DOMAIN
    146 
    147 When no DOMAIN is received from push/pull, do not add either domain or
    148 search to the resolv.conf. Fix typo in comment resolv.con[f]. Only add
    149 new line when using domain or search.
    150 
    151 URL: https://github.com/OpenVPN/openvpn/pull/34
    152 Acked-by: Steffan Karger <steffan@karger.me>
    153 Signed-off-by: Samuli Seppänen <samuli@openvpn.net>
    154 
    155 # Please enter the commit message for your changes. Lines starting
    156 # with '#' will be ignored, and an empty message aborts the commit.
    157 #
    158 # Author:    Jeffrey Cutter <jeff_m_cutter@yahoo.com>
    159 # Date:      Sat Sep 12 20:03:18 2015 -0400
    160 ...
    161 }}}
    162 After adjusting the commit message you can send the patch using git-send-email:
    163 {{{
    164 $ git-send-email --to=openvpn-devel@lists.sourceforge.net HEAD^1...HEAD
    165 }}}
    166 Adjust as necessary if you there is more than one patch.
    167 
    168 == Converting SVN revisions to Git patches ==
    169 
    170 There's a Python script available that converts SVN revisions into patches ''git am'' can understand:
    171 
    172  * http://blog.repl.ca/2009/06/converting-svn-commits-to-git-patches.html
    173 
    174 To use this script, copy it into the SVN repository root as ''svnrev2git.py''. Then create an authors file using this format:
    175 
    176 {{{
    177 svnusername, firstname lastname, email
    178 }}}
    179 
    180 For example:
    181 
    182 {{{
    183 johndoe, John Doe, john.doe@domain.com
    184 }}}
    185 To use the script, call it like this:
    186 
    187 {{{
    188 $ python svnrev2git.py authors <revision>
    189 $ python svnrev2git.py authors <revision_start>-<revision_end>
    190 }}}
    191 
    192 For example:
    193 
    194 {{{
    195 $ python svnrev2git.py authors 8126
    196 $ python svnrev2git.py authors 8126-8225
    197 $ for REV in 8206 8212 8219 8225; do python svnrev2git.py authors $REV;done
    198 }}}
    199 
    200 Note that the script is slow in processing long revision ranges: it's usually a better idea to pick the required revisions by hand.
    201 
    202 == Poor-man's Symdiff ==
    203 
    204 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:
    205 
    206 {{{
    207 #!/bin/bash
    208 git diff $1 $2 >/tmp/difftmp123.txt
    209 cat /tmp/difftmp123.txt |grep "^-" |sed s/^-// >/tmp/removed123.txt
    210 cat /tmp/difftmp123.txt |grep "^+" |sed s/^+// >/tmp/added123.txt
    211 diff /tmp/removed123.txt /tmp/added123.txt -u
    212 }}}
    213 
    214 This is similar to [http://research.microsoft.com/en-us/projects/symdiff Symdiff].
     110See DeveloperTips page.