Blame

51ccdb Samuli Seppänen 2025-02-28 15:44:25 1
# Introduction 
2
3
Most of the content here is the result of the [the weekly IRC discussions](/Meetings).
4
5
# Developer communication channels
6
7
There are two primary developer channels:
8
9
* **[openvpn-devel mailinglist](https://sourceforge.net/p/openvpn/mailman/openvpn-devel/)**: primary communication channel, subscription required
10
* **#openvpn-devel (at) libera.chat:** used for generic development discussions and the weekly IRC meetings (requires libera.chat registration)
11
12
# Development processes
13
14
## Daily development
15
f629ab Samuli Seppänen 2025-02-28 15:46:25 16
![Getting code to OpenVPN](./getting_code_to_openvpn.png "Diagram of OpenVPN development process")
51ccdb Samuli Seppänen 2025-02-28 15:44:25 17
18
The basic development process we follow is outlined in the diagram. So, the daily development process works like this:
19
20
* All patches must be eventually sent to "openvpn-devel" mailing list for review. The subject should preferably be prefixed with **[PATCH]**
21
* All patches need to be reviewed and accepted (ACK). The ACK process is logically split into two (see below).
22
* Regular developers can push to [Gerrit](https://gerrit.openvpn.net) and have their patches reviewed there before the final version is sent to the openvpn-devel mailing list.
23
* All accepted patches go to the OpenVPN **master** branch (Git)
24
25
The author must send patches to the devel mailing list. This is so to open up for a public discussion of the changes, and to allow the ACK process to work. All patches going into the tree will contain explicit Acked-by: references, to document who gave the approval.
26
27
**NOTE:**
28
29
* You can see a list of replies to a patch including the ACKs in [Gerrit](https://gerrit.openvpn.net).
30
* Patches sent *directly* to a development tree maintainer will be rejected
31
* GitHub pull requests can be useful with large patchsets, or if you want to gauge interest in your patch. The final patch needs to go to "openvpn-devel" mailing list, however.
32
33
## Adding placeholder bug reports
34
35
In the [IRC meeting](http://thread.gmane.org/gmane.network.openvpn.devel/3571) on 22nd Apr 2010 it was agreed that placeholder bug reports should be filed even for bugs which are fixed before anyone reports them as bugs. This allows users to check if the bugs they encounter have already been fixed.
36
37
## Making a release
38
39
Prior to making a release the following happens:
40
41
* Features to include will be merged to the **master** branch.
42
* Features will be stabilized until they don't change much.
43
* Beta release will be published based on latest **master** (to get people to _really_ test the code)
44
* This should have most features for the release included, and as such be mostly testing and bug fixing. However, new features could still change as much as needed and old features could be changed if absolutely needed.
45
* RC release will published based on latest **master**
46
* This should focus only on stabilization and bug fixes. At this point we know for sure which features should be included. Only in worst cases should a feature taken out, e.g. if it really is horrendous to stability.
47
* Once the code-base is deemed stable enough, an official release will be made
48
* Once there is need to use the **master** branch for the next release version (i.e. new features should be merged that will not go into the current release), a **release/2.x** branch will branched off. This might happen at any point during the Beta phase but if there is no need could also be delayed until after the official release.
49
50
The project currently aims for a 2 year release cycle. Shorter release cycles don't seem achievable with the current level of test automation.
51
52
## Feature deprecation
53
54
Feature removal process is another workflow which also goes through the general workflow to get a feature removed. This process serves two purposes:
55
56
* Maintain backwards compatibility and minimize the impact of feature removal (for users)
57
* Keeping the codebase clean and understandable (for developers)
58
e85d29 flichtenheld 2025-10-13 13:42:18 59
Features considered for removal are tracked on the [Deprecated Options](../Pages/Deprecated%20options) page.
51ccdb Samuli Seppänen 2025-02-28 15:44:25 60
61
## Community patches and the acceptance process of these patches
62
63
All patches needs to receive an ACK (Acknowledged) before being accepted in to the Git repository. To encourage participation from non-developers the ACK is split into two parts:
64
65
* *"This patch does the right thing ACK"*. Also known as *feature ACK*. One does not have to be a developer to give this ACK.
66
* *"This patch does the thing (it does) right"*. Also known as *code quality ACK*. People who understand the code can give this.
67
68
Note that both ACKs can be given by a single person.
69
70
A patch has to get both ACKs to be included into the development tree. The ACK process is used to ensure that only useful and high-quality code gets into OpenVPN. Hopefully that will make it easier for those who don't feel they are "good enough" developers to try to send patches anyway.
71
This whole review process is to give and receive feedback, where we as a community can help making OpenVPN better - together. For example, the more experienced developers can share their advice to those who are less experienced. Those being less experienced can share their understanding of the code as well, which might shed some light to issues or solutions which are better. Non-developers can also participate by sharing their expertise on OpenVPN in general by giving feature ACKs (NACKs) when applicable. This way everyone can learn something and hopefully even get wiser.
72
73
To explain the Signed-off-by and Acked-by process a bit further. In git you have two "data fields" which are populated automatically when commits are done, author and committer. The committer will be the one who applies patches from the mailing list or the one doing commits to a tree which are fetched via the remote access features in git.
74
75
Then you have the author field which is usually present in the patch file itself. This field might be missing if *git format-patch* has not been used to create the patch file sent to the mailing list. In these cases, it is expected that the author of the patch is the same as the sender - unless the commit message in the patch indicates something else.
76
77
If the author information is present, the sender field will be treated differently as well, in those cases where someone sends a patch on behalf of somebody else.
78
79
So to summarize, we track these fields almost automatically:
80
* author - information about the person who *wrote* the patch
81
* sender - information about who *sent / forwarded* the patch for inclusion
82
* committer - information about who *applied* the patch to the git tree
83
84
When a patch author sends his patch to somewhere, he should make sure it contains a `Signed-off-by: {Full name} <email@example.com>` line in the commit text. This is to officially announce that declare that this patch is ready for further processing. When doing git commits, you can add this indication automatically by doing *git commit -s* (or *--signed-off*).
85
86
The one who receives the patch and finds it good to go even further (if you want to do some kind of "internal" review before sending it further to the public) will add his/hers *Signed-off-by* reference as well when sending further again. This way we can track who has been looking at the patch. And the more people who have eye-balled the patch and added their *Signed-off* line to the patch, the better! By doing so, the reviewer indicates that the patch is good.
87
88
Before it goes into the tree, a final "Acked-by:" note is added, indicating who accepted it for inclusion into the tree,
89
90
### Example of patch submission and the review process
91
92
Lets say John Doe writes a patch, sends it to Jane Doe and she sends it
93
Bob Boss for inclusion. The process would be something like this:
94
95
**John Doe:**
96
```
149536 ordex 2025-12-04 08:32:27 97
Author: John Doe <john.doe@example.com> [auto-generated by git]
51ccdb Samuli Seppänen 2025-02-28 15:44:25 98
99
Short summary of the patch
100
149536 ordex 2025-12-04 08:32:27 101
A more detailed description of this patch fixing x.y.z. This can
51ccdb Samuli Seppänen 2025-02-28 15:44:25 102
even span over several lines to explain the patch.
103
149536 ordex 2025-12-04 08:32:27 104
Signed-off-by: John Doe <john.doe@example.com>
51ccdb Samuli Seppänen 2025-02-28 15:44:25 105
149536 ordex 2025-12-04 08:32:27 106
<the patch itself>
51ccdb Samuli Seppänen 2025-02-28 15:44:25 107
```
108
109
**Jane Doe will send it further as:**
110
```
149536 ordex 2025-12-04 08:32:27 111
Author: John Doe <john.doe@example.com> [auto-generated by git]
51ccdb Samuli Seppänen 2025-02-28 15:44:25 112
113
Short summary of the patch
114
149536 ordex 2025-12-04 08:32:27 115
A more detailed description of this patch fixing x.y.z. This can
51ccdb Samuli Seppänen 2025-02-28 15:44:25 116
even span over several lines to explain the patch.
117
149536 ordex 2025-12-04 08:32:27 118
Signed-off-by: John Doe <john.doe@example.com>
119
Signed-off-by: Jane Doe <jane@doe.net>
51ccdb Samuli Seppänen 2025-02-28 15:44:25 120
149536 ordex 2025-12-04 08:32:27 121
<the patch itself>
51ccdb Samuli Seppänen 2025-02-28 15:44:25 122
```
123
124
**When Bob Boss ACKs the patch**, the final commit to be found in the
125
source code repository will be:
126
```
149536 ordex 2025-12-04 08:32:27 127
Author: John Doe <john.doe@example.com> [auto-generated by git]
51ccdb Samuli Seppänen 2025-02-28 15:44:25 128
129
Short summary of the patch
130
149536 ordex 2025-12-04 08:32:27 131
A more detailed description of this patch fixing x.y.z. This can
51ccdb Samuli Seppänen 2025-02-28 15:44:25 132
even span over several lines to explain the patch.
133
149536 ordex 2025-12-04 08:32:27 134
Signed-off-by: John Doe <john.doe@example.com>
135
Signed-off-by: Jane Doe <jane@doe.net>
136
Acked-by: Bob Boss <bob@bigbucks.com>
137
Signed-off-by: David Sommerseth <dazo@users.sourceforge.net>
51ccdb Samuli Seppänen 2025-02-28 15:44:25 138
139
149536 ordex 2025-12-04 08:32:27 140
<the patch itself>
51ccdb Samuli Seppänen 2025-02-28 15:44:25 141
```
142
143
The Acked-by: can be the one who does the final commit to the source code repository, but it can also be someone who doesn't do the final commit. The last Signed-off-by should normally be the person who does the final commit. This is usually the person who "touched" the patch by adding the Acked-by line(s).
144
145
However, you should not see any commits which are written by (author) and Acked-by by the same person. Then the process has failed to give a qualified review.
146
147
You may ask why we have this "bureaucracy" (which is a fair question!). It is simply to keep all who send in patches, those who review them and finally accept them into the source code repository more accountable for their work. This process will document each accepted patch and give an indication that it has been through a certain set of reviews. This is to ensure that we don't accept bad code easily into the source code repository. This is just to keep this project as transparent and open as possible.
148
149
# Handling security issues
150
151
The way OpenVPN project handles security issues was discussed and agreed upon in the IRC meeting on [15th July 2010](http://thread.gmane.org/gmane.network.openvpn.devel/3841). The goal is to disclose security issues in 3 weeks - or less, if a fix is ready. If a fix is not ready in 3 weeks the issue should be disclosed nevertheless and provide workarounds (if any) to users and then fix the issue a.s.a.p. Also, *all* security issues - whether they're theoretical or being exploited - should be fixed. Also agreed that our users should be informed about vulnerabilities in external software OpenVPN depends on (e.g. OpenSSL). This will be done after developers of the external software have already disclosed the vulnerability.
152
153
# Patch submission via Gerrit
154
155
The OpenVPN project has an instance of the [Gerrit code review tool](https://gerrit-review.googlesource.com/), hosted at https://gerrit.openvpn.net/. This is intended to aid in the submission and review of bigger patches or patch series (i.e. patches that do depend on each other). Gerrit allows to
156
157
* Track review comments on a patch
158
* Review changes between different revisions of a patch
159
* Get test builds for patches
160
161
The use of Gerrit is encouraged over using [GitHub Pull-Requests](https://github.com/OpenVPN/openvpn/pulls). The main difference is that our development workflow is currently patch orientated. Even when you submit a patch series, each patch needs to stand on its own, and produce a working build. Each patch will be merged individually. Gerrit supports this model, while Pull Requests like in !GitHub or similar products are more branch-oriented.
162
cab909 Samuli Seppänen 2025-04-22 06:24:48 163
Note that we currently do not directly merge the patches via the Gerrit tool. Instead they are still submitted to the mailing list for the final step of the workflow as described above. We have written a small script that facilitates sending a patch reviewed through Gerrit to the list. You can find [gerrit-send-mail.py](https://github.com/OpenVPN/openvpn/blob/master/dev-tools/gerrit-send-mail.py) in the `dev-tools` directory in the Git repository. That script will take care of adding *Acked-By* lines automatically based on the Gerrit reviews. Anyone can submit a patch to the mailing list once it is reviewed and approved. But of course we encourage the patch author to take care of it themselves.
51ccdb Samuli Seppänen 2025-02-28 15:44:25 164
cf34e1 Samuli Seppänen 2025-03-26 12:48:13 165
For more information and tips about using Gerrit see [GerritBestPractises](../Development/GerritBestPractices).
51ccdb Samuli Seppänen 2025-02-28 15:44:25 166
167
# Patch quality
168
169
## Formatting
170
171
Existing codebase uses an Allman based style. Please visit the [Code Style](/Development/CodeStyle) wikipage for more details.
172
173
## Conventions
174
175
All *patches*, regardless of their type need to be created following a few rules:
176
177
* Must be sent as unified diff (diff -u) or preferably generated by *git format-patch*
178
* 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 technical details. More on [writing good commit messages here](http://who-t.blogspot.com/2009/12/on-commit-messages.html)
179
* Patches should contain a `Signed-off-by:` line at the end (*git commit --signoff*). If missing, it will be added with the sender of the patch as the signed-off person.
180
* Patches must apply cleanly, without merge conflicts. Please state which code base the patch is written against. If based on the git tree, please state which branch it is to be applied to.
181
* Everyone who has contributed to this patch should be mentioned out of courtesy and respect to all contributors and helpers on the way. If suitable valid e-mail address should be used, preferably in addition with full name.
182
183
## Code contributions
184
185
All *code patches* need to meet certain generic quality criteria before being accepted:
186
187
* All code should be useful and beneficial for several OpenVPN users. This way we avoid spoiling the code base with features which is only requested for very special conditions.
188
* New features need to make use of #ifdef's so that they can be disabled at compile-time. This is to enable better support for embedded systems and to track which code belongs to which feature.
189
* Patch needs to respect our [Code Style](/Development/CodeStyle) to keep the code base understandable and maintainable.
190
191
Also note that the *documentation* (e.g. man pages) need to be updated, if
192
193
* New functionality has been introduced
194
* Old functionality has changed
195
* Functionality has been removed
196
197
## Shell scripts
198
199
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:
200
201
* [Bashism - Greg's Wiki](http://mywiki.wooledge.org/Bashism)
202
* [Checkbashisms](http://sourceforge.net/projects/checkbaskisms): a tool to check for "Bashisms". Also included in !Debian/Ubuntu "devscripts" package.
203
* [DASH](http://gondor.apana.org.au/~herbert/dash): a minimal shell with "POSIX-compliant features only", probably useful for testing
204
205
# Code repositories
206
207
Contents of this section are now [here](/Pages/CodeRepositories).
208
209
# Core Developer Infrastructure
210
211
There is some additional infrastructure available for developers that are part of the core development team.
212
213
## Community VPN
214
215
To use the internal infrastructure you first need VPN access to it. If you think you should have access you should know who to ask for your config.
216
217
## Buildbot
218
219
Some of our CI builds happen on our Buildbot instance. Failed builds will be documented with mails to the [openvpn-builds mailinglist](https://sourceforge.net/p/openvpn/mailman/openvpn-builds/) mailing list, but if you have access to the Community VPN you can also directly access the web interface.
220
221
This is also integrated with out Gerrit server, but only changes submitted by whitelisted developer accounts will trigger builds.
222
223
## HTTP Proxy
224
225
There is a HTTP Proxy available at `http://community-test-proxy.openvpn.in:8080`. You need to be logged into the Community VPN to be able to access it. It is intended to be used for openvpn tests only.
226
227
## Socks5 Proxy
228
229
There is a Sock5 Proxy available at `community-test-proxy.openvpn.in:1080`. It allows both TCP and UDP to be proxied.
230
You need to be logged into the Community VPN to be able to access it. It is intended to be used for openvpn tests only.
231
232
## HTTP Proxy with NTLM authentication
233
234
There is a NTLM Proxy available at `10.18.0.99:8080`. You need to be logged into the Community VPN to be able to access it. It is intended to be used for openvpn tests only.
235
You can use both NTLMv1 and NTLMv2 style authentication against it.
236
237
Some explanation on [how we set up a NTLM proxy for testing](/Pages/NtlmProxyTestSetup).
238
239
# Practical issues
240
241
## Using Git
242
243
If you're unfamiliar with git in general, take a look at these links:
244
245
* [git crash course](/Development/GitCrashCourse)
246
* [Git homepage](http://git-scm.com)
247
* http://progit.org/book/
248
* http://www.kernel.org/pub/software/scm/git/docs/gittutorial.html
249
* [git for SVN users](http://git.or.cz/course/svn.html)
250
251
## Bisecting commits to detect introduction of a bug
252
253
In most cases, it's easiest to use [git-bisect](http://book.git-scm.com/5_finding_issues_-_git_bisect.html) 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 Trac ticket 190. In practice, you can reset to an earlier commit with
254
255
```
256
$ git reset --hard <commit-id>
257
```
258
259
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.
260
261
If you have hunch which commit might have introduced the bug, you can try reverting it to see what happens:
262
263
```
264
git revert <commit-id>
265
```
266
267
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.
268
269
## Applying patches from emails
270
271
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>*.
272
d7faf4 Samuli Seppänen 2025-03-26 13:06:47 273
## Sending GitHub pull requests to the mailing list
51ccdb Samuli Seppänen 2025-02-28 15:44:25 274
275
Getting a "git am"-compatible patch out of a GitHub pull requests is simple:
276
```
277
$ wget https://github.com/OpenVPN/openvpn/pull/<pr-number>.patch
278
```
279
You can then apply the patch:
280
```
281
$ git am <pr-number>.patch
282
```
283
Then you should amend the commit message to add information and to fix errors (if any):
284
285
* What pull request the patch was created from
286
* Who ACKed the patch
287
* Who relayed the patch (in case if you're not the author)
288
* Fix formatting issues
289
290
An example below:
291
292
```
293
$ git commit -s --amend
294
Update contrib/pull-resolv-conf/client.up for no DOMAIN
295
296
When no DOMAIN is received from push/pull, do not add either domain or
297
search to the resolv.conf. Fix typo in comment resolv.con[f]. Only add
298
new line when using domain or search.
299
300
URL: https://github.com/OpenVPN/openvpn/pull/34
301
Acked-by: Steffan Karger <steffan@karger.me>
302
Signed-off-by: Samuli Seppänen <samuli@openvpn.net>
303
304
# Please enter the commit message for your changes. Lines starting
305
# with '#' will be ignored, and an empty message aborts the commit.
306
#
307
# Author: Jeffrey Cutter <jeff_m_cutter@yahoo.com>
308
# Date: Sat Sep 12 20:03:18 2015 -0400
309
...
310
```
311
After adjusting the commit message you can send the patch using git-send-email.
312
313
## Sending patches with git-send-email
314
315
First, subscribe to openvpn-devel@lists.sourceforge.net:
316
317
[https://sourceforge.net/projects/openvpn/lists/openvpn-devel]
318
319
Now, configure git send-email in ~/.config/git/config.
320
(This file might be located at ~/.gitconfig instead.)
321
The required settings can be obtained from your email provider.
322
The email account has to be *the same one you used to subscribe to openvpn-devel*.
323
```
324
[sendemail]
325
smtpServer = your.smtp.server
326
smtpServerPort = 587
327
smtpEncryption = tls
328
smtpUser = your_smtp_user
329
from = Your Name <your@email.tld>
330
```
331
The above example is for the most common auth method, STARTTLS.
332
For SMTPS, do the below instead:
333
```
334
[sendemail]
335
smtpServer = your.smtp.server
336
smtpEncryption = ssl
337
smtpUser = your_smtp_user
338
from = Your Name <your@email.tld>
339
```
340
341
On Archlinux, git send-email requires two extra packages:
342
```
343
# pacman -S perl-io-socket-ssl perl-authen-sasl
344
```
345
346
Now, it's ready. A single commit can be sent with:
347
348
```
349
$ git send-email --to=openvpn-devel@lists.sourceforge.net HEAD^
350
```
351
352
Adjust the revision range as necessary if there are more than one patch.
353
d7faf4 Samuli Seppänen 2025-03-26 13:06:47 354
## Poor-man's Symdiff
51ccdb Samuli Seppänen 2025-02-28 15:44:25 355
356
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:
357
358
```
359
#!/bin/bash
360
git diff $1 $2 >/tmp/difftmp123.txt
361
cat /tmp/difftmp123.txt |grep "^-" |sed s/^-// >/tmp/removed123.txt
362
cat /tmp/difftmp123.txt |grep "^+" |sed s/^+// >/tmp/added123.txt
363
diff /tmp/removed123.txt /tmp/added123.txt -u
364
```
365
366
This is similar to [http://research.microsoft.com/en-us/projects/symdiff Symdiff].