1 // This file is part of the Doxygen Developer Manual
2 /** @page patchguide Patch Guidelines
4 \attention You can't send patches to the mailing list anymore at all. Nowadays
5 you are expected to send patches to the OpenOCD Gerrit GIT server for a
8 \attention If you already have a Gerrit account and want to try a
9 different sign in method, please first sign in as usually, press your
10 name in the upper-right corner, go to @a Settings, select @a
11 Identities pane, press <em>Link Another Identity</em> button. In case
12 you already have duplicated accounts, ask administrators for manual
15 \attention If you're behind a corporate wall with http only access to the
16 world, you can still use these instructions!
18 @section gerrit Submitting patches to the OpenOCD Gerrit server
20 OpenOCD is to some extent a "self service" open source project, so to
21 contribute, you must follow the standard procedures to have the best
22 possible chance to get your changes accepted.
24 The procedure to create a patch is essentially:
28 - send the changes to the Gerrit server for review
29 - correct the patch and re-send it according to review feedback
31 Your patch (or commit) should be a "good patch": focus it on a single
32 issue, and make it easily reviewable. Don't make
33 it so large that it's hard to review; split large
34 patches into smaller ones (this will also help
35 to track down bugs later). All patches should
36 be "clean", which includes preserving the existing
37 coding style and updating documentation as needed. When adding a new
38 command, the corresponding documentation should be added to
39 @c doc/openocd.texi in the same commit. OpenOCD runs on both Little
40 Endian and Big Endian hosts so the code can't count on specific byte
41 ordering (in other words, must be endian-clean).
43 There are several additional methods of improving the quality of your
46 - Runtime testing with Valgrind Memcheck
48 This helps to spot memory leaks, undefined behaviour due to
49 uninitialized data or wrong indexing, memory corruption, etc.
51 - Clang Static Analyzer
53 Using this tool uncovers many different kinds of bugs in C code,
54 with problematic execution paths fully explained. It is a part of
55 standard Clang installation.
57 To generate a report, run this in the OpenOCD source directory:
59 mkdir build-scanbuild; cd build-scanbuild
60 scan-build ../configure
61 scan-build make CFLAGS="-std=gnu99 -I. -I../../jimtcl"
64 - Runtime testing with sanitizers
66 Both GCC and LLVM/Clang include advanced instrumentation options to
67 detect undefined behaviour and many kinds of memory
68 errors. Available with @c -fsanitize=* command arguments.
72 mkdir build-sanitizers; cd build-sanitizers
73 ../configure CC=clang CFLAGS="-fno-omit-frame-pointer \
74 -fsanitize=address -fsanitize=undefined -ggdb3"
76 export ASAN_OPTIONS=detect_stack_use_after_return=1
77 src/openocd -s ../tcl -f /path/to/openocd.cfg
80 - Sparse Static Analyzer
82 Using this tool allows identifying some bug in C code.
83 In the future, OpenOCD would use the sparse attribute 'bitwise' to
84 detect incorrect endianness assignments.
88 mkdir build-sparse; cd build-sparse
89 ../configure CC=cgcc CFLAGS="-Wsparse-all -Wno-declaration-after-statement \
90 -Wno-unknown-attribute -Wno-transparent-union -Wno-tautological-compare \
91 -Wno-vla -Wno-flexible-array-array -D__FLT_EVAL_METHOD__=0"
95 Please consider performing these additional checks where appropriate
96 (especially Clang Static Analyzer for big portions of new code) and
97 mention the results (e.g. "Valgrind-clean, no new Clang analyzer
98 warnings") in the commit message.
100 Say in the commit message if it's a bugfix (describe the bug) or a new
101 feature. Don't expect patches to merge immediately
102 for the next release. Be ready to rework patches
103 in response to feedback.
105 Add yourself to the GPL copyright for non-trivial changes.
107 @section stepbystep Step by step procedure
109 -# Create a Gerrit account at: https://review.openocd.org
110 - On subsequent sign ins, use the full URL prefaced with 'http://'
111 For example: http://user_identifier.open_id_provider.com
112 -# Add a username to your profile.
113 After creating the Gerrit account and signing in, you will need to
114 add a username to your profile. To do this, go to 'Settings', and
115 add a username of your choice.
116 Your username will be required in step 3 and substituted wherever
117 the string 'USERNAME' is found.
118 -# Create an SSH public key following the directions on github:
119 https://help.github.com/articles/generating-ssh-keys . You can skip step 3
120 (adding key to Github account) and 4 (testing) - these are useful only if
121 you actually use Github or want to test whether the new key works fine.
122 -# Add this new SSH key to your Gerrit account:
123 go to 'Settings' > 'SSH Public Keys', paste the contents of
124 ~/.ssh/id_rsa.pub into the text field (if it's not visible click on
125 'Add Key ...' button) and confirm by clicking 'Add' button.
126 -# Clone the git repository, rather than just download the source:
128 git clone git://git.code.sf.net/p/openocd/code openocd
130 or if you have problems with the "git:" protocol, use
131 the slower http protocol:
133 git clone http://git.code.sf.net/p/openocd/code openocd
135 -# Set up Gerrit with your local repository. All this does it
136 to instruct git locally how to send off the changes.
137 -# Add a new remote to git using Gerrit username:
139 git remote add review ssh://USERNAME@review.openocd.org:29418/openocd.git
140 git config remote.review.push HEAD:refs/for/master
144 git remote add review https://USERNAME@review.openocd.org/p/openocd.git
145 git config remote.review.push HEAD:refs/for/master
147 The http password is configured from your gerrit settings - https://review.openocd.org/#/settings/http-password.
148 \note If you want to simplify http access you can also add your http password to the url as follows:
150 git remote add review https://USERNAME:PASSWORD@review.openocd.org/p/openocd.git
152 \note All contributions should be pushed to @c refs/for/master on the
153 Gerrit server, even if you plan to use several local branches for different
154 topics. It is possible because @c for/master is not a traditional Git
156 -# You will need to install this hook, we will look into a better solution:
158 scp -p -P 29418 USERNAME@review.openocd.org:hooks/commit-msg .git/hooks/
162 wget https://review.openocd.org/tools/hooks/commit-msg
163 mv commit-msg .git/hooks
164 chmod +x .git/hooks/commit-msg
166 \note A script exists to simplify the two items above. Execute:
168 tools/initial.sh <username>
170 With @<username@> being your Gerrit username.
171 -# Set up git with your name and email:
173 git config --global user.name "John Smith"
174 git config --global user.email "john@smith.org"
176 -# Work on your patches. Split the work into
177 multiple small patches that can be reviewed and
178 applied separately and safely to the OpenOCD
182 work - edit files using your favorite editor.
183 run "git commit -s -a" to commit all changes.
184 run tools/checkpatch.sh to verify your patch style is ok.
187 \note use "git add ." before commit to add new files.
189 \note check @ref checkpatch for hint about checkpatch script
191 Commit message template, notice the short first line.
192 The field '<c>specify touched area</c>'
193 should identify the main part or subsystem the patch touches.
195 specify touched area: short comment
197 Longer comments over several lines, explaining (where applicable) the
198 reason for the patch and the general idea the solution is based on,
199 any major design decisions, etc. Limit each comment line's length to 75
200 characters; since 75 it's too short for a URL, you can put the URL in a
201 separate line preceded by 'Link: '.
207 flash/nor/atsame5: add SAME59 support
212 flash/nor: flash driver for XYZ123
214 Add new flash driver for internal flash of ...
217 target/cortex_m: fix segmentation fault in cmd 'soft_reset_halt'
219 soft_reset_halt command failed reproducibly under following conditions: ...
220 Test for NULL pointer and return error ...
222 Reported-by: John Reporter <rep9876@gmail.com>
223 Fixes: 123456789abc ("target: the commit where the problem started")
224 BugLink: https://sourceforge.net/p/openocd/tickets/999/
229 See "git log" for more examples.
231 -# Next you need to make sure that your patches
232 are on top of the latest stuff on the server and
233 that there are no conflicts:
235 git pull --rebase origin master
237 -# Send the patches to the Gerrit server for review:
241 -# Forgot something, want to add more? Just make the changes and do:
247 Further reading: http://www.coreboot.org/Git
249 @section checkpatch About checkpatch script
251 OpenOCD source code includes the script checkpatch to let developers to
252 verify their patches before submitting them for review (see @ref gerrit).
254 Every patch for OpenOCD project that is submitted for review on Gerrit
255 is tested by Jenkins. Jenkins will run the checkpatch script to analyze
257 If the script highlights either errors or warnings, Gerrit will add the
258 score "-1" to the patch and maintainers will probably ignore the patch,
259 waiting for the developer to send a fixed version.
261 The script checkpatch verifies the SPDX tag for new files against a very
262 short list of license tags.
263 If the license of your contribution is not listed there, but compatible
264 with OpenOCD license, please alert the maintainers or add the missing
265 license in the first patch of your patch series.
267 The script checkpatch has been originally developed for the Linux kernel
268 source code, thus includes specific tests and checks related to Linux
269 coding style and to Linux code structure. While the script has been
270 adapted for OpenOCD specificities, it still includes some Linux related
271 test. It is then possible that it triggers sometimes some <em>false
274 If you think that the error identified by checkpatch is a false
275 positive, please report it to the openocd-devel mailing list or prepare
276 a patch for fixing checkpatch and send it to Gerrit for review.
278 \attention The procedure below is allowed only for <em>exceptional
279 cases</em>. Do not use it to submit normal patches.
281 There are <em>exceptional cases</em> in which you need to skip some of
282 the tests from checkpatch in order to pass the approval from Gerrit.
284 For example, a patch that modify one line inside a big comment block
285 will not show the beginning or the end of the comment block. This can
286 prevent checkpatch to detect the comment block. Checkpatch can wrongly
287 consider the modified comment line as a code line, triggering a set of
290 Only for <em>exceptional cases</em>, it is allowed to submit patches
291 to Gerrit with the special field 'Checkpatch-ignore:' in the commit
292 message. This field will cause checkpatch to ignore the error types
293 listed in the field, only for the patch itself.
294 The error type is printed by checkpatch on failure.
295 For example the names of Windows APIs mix lower and upper case chars,
296 in violation of OpenOCD coding style, triggering a 'CAMELCASE' error:
298 CHECK:CAMELCASE: Avoid CamelCase: <WSAGetLastError>
299 #96105: FILE: src/helper/log.c:505:
300 + error_code = WSAGetLastError();
302 Adding in the commit message of the patch the line:
304 Checkpatch-ignore: CAMELCASE
306 will force checkpatch to ignore the CAMELCASE error.
308 @section timeline When can I expect my contribution to be committed?
310 The code review is intended to take as long as a week or two to allow
311 maintainers and contributors who work on OpenOCD only in their spare
312 time opportunity to perform a review and raise objections.
314 With Gerrit much of the urgency of getting things committed has been
315 removed as the work in progress is safely stored in Gerrit and
316 available if someone needs to build on your work before it is
317 submitted to the official repository.
319 Another factor that contributes to the desire for longer cool-off
320 times (the time a patch lies around without any further changes or
321 comments), it means that the chances of quality regression on the
322 master branch will be much reduced.
324 If a contributor pushes a patch, it is considered good form if another
325 contributor actually approves and submits that patch.
327 It should be noted that a negative review in Gerrit ("-1" or "-2") may (but does
328 not have to) be disregarded if all conditions listed below are met:
330 - the concerns raised in the review have been addressed (or explained),
331 - reviewer does not re-examine the change in a month,
332 - reviewer does not answer e-mails for another month.
334 @section browsing Browsing Patches
335 All OpenOCD patches can be reviewed <a href="https://review.openocd.org/">here</a>.
337 @section reviewing Reviewing Patches
338 From the main <a href="https://review.openocd.org/#/q/status:open,n,z">Review
339 page</a> select the patch you want to review and click on that patch. On the
340 appearing page select the download method (top right). Apply the
341 patch. After building and testing you can leave a note with the "Reply"
342 button and mark the patch with -1, 0 and +1.
345 This file contains the @ref patchguide page.