]>
Commit | Line | Data |
---|---|---|
d9fd04c2 | 1 | -*- mode: text; -*- |
d6bb5aa5 PJ |
2 | $QuaggaId: Format:%an, %ai, %h$ $ |
3 | ||
4 | Contents: | |
5 | ||
6 | * GUIDELINES FOR HACKING ON QUAGGA | |
7 | * COMPILE-TIME CONDITIONAL CODE | |
8 | * COMMIT MESSAGE | |
9 | * HACKING THE BUILD SYSTEM | |
10 | * RELEASE PROCEDURE | |
11 | * SHARED LIBRARY VERSIONING | |
12 | * RELEASE PROCEDURE | |
13 | * TOOL VERSIONS | |
14 | * SHARED LIBRARY VERSIONING | |
15 | * PATCH SUBMISSION | |
16 | * PATCH APPLICATION | |
17 | * STABLE PLATFORMS AND DAEMONS | |
18 | * IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS | |
d9fd04c2 | 19 | |
a8e474a5 | 20 | |
d9fd04c2 | 21 | GUIDELINES FOR HACKING ON QUAGGA |
22 | ||
d9fd04c2 | 23 | [this is a draft in progress] |
24 | ||
863076db | 25 | GNU coding standards apply. Indentation follows the result of |
26 | invoking GNU indent (as of 2.2.8a) with no arguments. Note that this | |
27 | uses tabs instead of spaces where possible for leading whitespace, and | |
28 | assumes that tabs are every 8 columns. Do not attempt to redefine the | |
29 | location of tab stops. Note also that some indentation does not | |
30 | follow GNU style. This is a historical accident, and we generally | |
31 | only clean up whitespace when code is unmaintainable due to whitespace | |
32 | issues, as fewer changes from zebra lead to easier merges. | |
33 | ||
34 | For GNU emacs, use indentation style "gnu". | |
35 | ||
36 | For Vim, use the following lines (note that tabs are at 8, and that | |
37 | softtabstop sets the indentation level): | |
38 | ||
39 | set tabstop=8 | |
40 | set softtabstop=2 | |
41 | set shiftwidth=2 | |
42 | set noexpandtab | |
d9fd04c2 | 43 | |
2934f28e | 44 | Be particularly careful not to break platforms/protocols that you |
45 | cannot test. | |
46 | ||
47 | New code should have good comments, and changes to existing code | |
48 | should in many cases upgrade the comments when necessary for a | |
49 | reviewer to conclude that the change has no unintended consequences. | |
50 | ||
d6bb5aa5 PJ |
51 | Each file in the Git repository should have a git format-placeholder (like |
52 | an RCS Id keyword), somewhere very near the top, commented out appropriately | |
53 | for the file type. The placeholder used for Quagga (replacing <dollar> with | |
54 | $) is: | |
55 | ||
56 | $QuaggaId: <dollar>Format:%an, %ai, %h<dollar> $ | |
57 | ||
58 | See line 2 of HACKING for an example; | |
59 | ||
60 | This placeholder string will be expanded out by the 'git archive' commands, | |
61 | wihch is used to generate the tar archives for snapshots and releases. | |
697877eb | 62 | |
5e764774 | 63 | Please document fully the proper use of a new function in the header file |
64 | in which it is declared. And please consult existing headers for | |
65 | documentation on how to use existing functions. In particular, please consult | |
66 | these header files: | |
67 | ||
68 | lib/log.h logging levels and usage guidance | |
69 | [more to be added] | |
70 | ||
1eb8ef25 | 71 | If changing an exported interface, please try to deprecate the interface in |
72 | an orderly manner. If at all possible, try to retain the old deprecated | |
73 | interface as is, or functionally equivalent. Make a note of when the | |
74 | interface was deprecated and guard the deprecated interface definitions in | |
75 | the header file, ie: | |
76 | ||
77 | /* Deprecated: 20050406 */ | |
78 | #if !defined(QUAGGA_NO_DEPRECATED_INTERFACES) | |
79 | #warning "Using deprecated <libname> (interface(s)|function(s))" | |
80 | ... | |
81 | #endif /* QUAGGA_NO_DEPRECATED_INTERFACES */ | |
82 | ||
83 | To ensure that the core Quagga sources do not use the deprecated interfaces | |
84 | (you should update Quagga sources to use new interfaces, if applicable) | |
85 | while allowing external sources to continue to build. Deprecated interfaces | |
86 | should be excised in the next unstable cycle. | |
87 | ||
74a2dd7b | 88 | Note: If you wish, you can test for GCC and use a function |
89 | marked with the 'deprecated' attribute. However, you must provide the | |
90 | #warning for other compilers. | |
91 | ||
1eb8ef25 | 92 | If changing or removing a command definition, *ensure* that you properly |
93 | deprecate it - use the _DEPRECATED form of the appropriate DEFUN macro. This | |
94 | is *critical*. Even if the command can no longer function, you *must* still | |
95 | implement it as a do-nothing stub. Failure to follow this causes grief for | |
96 | systems administrators. Deprecated commands should be excised in the next | |
97 | unstable cycle. A list of deprecated commands should be collated for each | |
98 | release. | |
99 | ||
100 | See also below regarding SHARED LIBRARY VERSIONING. | |
5e764774 | 101 | |
a8e474a5 | 102 | |
750e8146 PJ |
103 | COMPILE-TIME CONDITIONAL CODE |
104 | ||
105 | Please think very carefully before making code conditional at compile time, | |
106 | as it increases maintenance burdens and user confusion. In particular, | |
107 | please avoid gratuitious --enable-.... switches to the configure script - | |
108 | typically code should be good enough to be in Quagga, or it shouldn't be | |
109 | there at all. | |
110 | ||
111 | When code must be compile-time conditional, try have the compiler make it | |
112 | conditional rather than the C pre-processor. I.e. this: | |
113 | ||
114 | if (SOME_SYMBOL) | |
115 | frobnicate(); | |
116 | ||
117 | rather than: | |
118 | ||
119 | #ifdef SOME_SYMBOL | |
120 | frobnicate (); | |
121 | #endif /* SOME_SYMBOL */ | |
122 | ||
123 | Note that the former approach requires ensuring that SOME_SYMBOL will be | |
124 | defined (watch your AC_DEFINEs). | |
74a2dd7b | 125 | |
a8e474a5 | 126 | |
d6bb5aa5 | 127 | COMMIT MESSAGES |
2934f28e | 128 | |
d6bb5aa5 | 129 | The commit message should provide: |
2934f28e | 130 | |
d7a97797 GT |
131 | * A suitable one-line summary followed by a blank line as the very |
132 | first line of the message, in the form: | |
2934f28e | 133 | |
3de4277b | 134 | topic: high-level, one line summary |
ca6383ba | 135 | |
3de4277b PJ |
136 | Where topic would tend to be name of a subdirectory, and/or daemon, unless |
137 | there's a more suitable topic (e.g. 'build'). This topic is used to | |
138 | organise change summaries in release announcements. | |
ca6383ba | 139 | |
d6bb5aa5 | 140 | * An optional introduction, discussing the general intent of the change. |
3de4277b | 141 | * A short description of each change made, preferably: |
ca6383ba | 142 | * file by file |
d6bb5aa5 | 143 | * function by function (use of "ditto", or globs is allowed) |
ca6383ba | 144 | |
3de4277b PJ |
145 | to provide a short description of the general intent of the patch, in terms |
146 | of the problem it solves and how it achieves it, to help reviewers | |
a8e474a5 | 147 | understand. |
3de4277b | 148 | |
d7a97797 GT |
149 | The one-line summary must be limited to 54 characters, and all other |
150 | lines to 72 characters. | |
ca6383ba | 151 | |
d6bb5aa5 PJ |
152 | The reason for such itemised commit messages is to encourage the author to |
153 | self-review every line of the patch, as well as provide reviewers an index | |
a8e474a5 | 154 | of which changes are intended, along with a short description for each. |
3de4277b PJ |
155 | Some discretion is obviously required. A C-to-english description is not |
156 | desireable. For short patches, a per-function/file break-down may be | |
157 | redundant. For longer patches, such a break-down may be essential. | |
158 | ||
d6bb5aa5 PJ |
159 | An example (where the general discussion is obviously somewhat redundant, |
160 | given the one-line summary): | |
161 | ||
3de4277b | 162 | zebra: Enhance frob FSM to detect loss of frob |
ca6383ba | 163 | |
3de4277b | 164 | * (general) Add a new DOWN state to the frob state machine |
a8e474a5 | 165 | to allow the barinator to detect loss of frob. |
3de4277b PJ |
166 | * frob.h: (struct frob) Add DOWN state flag. |
167 | * frob.c: (frob_change) set/clear DOWN appropriately on state change. | |
168 | * bar.c: (barinate) Check frob for DOWN state. | |
1f8f61a7 | 169 | |
d7a97797 GT |
170 | Note that the commit message format follows git norms, so that "git |
171 | log --oneline" will have useful output. | |
74a2dd7b | 172 | |
173 | HACKING THE BUILD SYSTEM | |
174 | ||
175 | If you change or add to the build system (configure.ac, any Makefile.am, | |
176 | etc.), try to check that the following things still work: | |
177 | ||
178 | - make dist | |
179 | - resulting dist tarball builds | |
a8e474a5 | 180 | - out-of-tree builds |
74a2dd7b | 181 | |
182 | The quagga.net site relies on make dist to work to generate snapshots. It | |
d6bb5aa5 | 183 | must work. Common problems are to forget to have some additional file |
74a2dd7b | 184 | included in the dist, or to have a make rule refer to a source file without |
185 | using the srcdir variable. | |
186 | ||
a8e474a5 | 187 | |
0d7e9134 | 188 | RELEASE PROCEDURE |
189 | ||
d6bb5aa5 PJ |
190 | * Tag the apppropriate commit with a release tag (follow existing |
191 | conventions). | |
0d7e9134 | 192 | [This enables recreating the release, and is just good CM practice.] |
193 | ||
d6bb5aa5 PJ |
194 | * Create a fresh tar archive of the quagga.net repository, and do a test |
195 | build: | |
0d7e9134 | 196 | |
d6bb5aa5 PJ |
197 | git-clone git:///code.quagga.net/quagga.git quagga |
198 | git-archive --remote=git://code.quagga.net/quagga.git \ | |
199 | --prefix=quagga-release/ master | tar -xf - | |
200 | cd quagga-release | |
0d7e9134 | 201 | |
3de4277b | 202 | autoreconf -i |
d6bb5aa5 PJ |
203 | ./configure |
204 | make | |
205 | make dist | |
0d7e9134 | 206 | |
d6bb5aa5 PJ |
207 | The tarball which 'make dist' creates is the tarball to be released! The |
208 | git-archive step ensures you're working with code corresponding to that in | |
209 | the official repository, and also carries out keyword expansion. If any | |
a8e474a5 | 210 | errors occur, move tags as needed and start over from the fresh checkouts. |
d6bb5aa5 PJ |
211 | Do not append to tarballs, as this has produced non-standards-conforming |
212 | tarballs in the past. | |
0d7e9134 | 213 | |
3de4277b PJ |
214 | See also: http://wiki.quagga.net/index.php/Main/Processes |
215 | ||
1eb8ef25 | 216 | [TODO: collation of a list of deprecated commands. Possibly can be scripted |
217 | to extract from vtysh/vtysh_cmd.c] | |
218 | ||
74a2dd7b | 219 | |
fbb67099 | 220 | TOOL VERSIONS |
221 | ||
222 | Require versions of support tools are listed in INSTALL.quagga.txt. | |
223 | Required versions should only be done with due deliberation, as it can | |
224 | cause environments to no longer be able to compile quagga. | |
225 | ||
74a2dd7b | 226 | |
b7a97f82 | 227 | SHARED LIBRARY VERSIONING |
228 | ||
229 | [this section is at the moment just gdt's opinion] | |
230 | ||
231 | Quagga builds several shared libaries (lib/libzebra, ospfd/libospf, | |
232 | ospfclient/libsopfapiclient). These may be used by external programs, | |
233 | e.g. a new routing protocol that works with the zebra daemon, or | |
234 | ospfapi clients. The libtool info pages (node Versioning) explain | |
235 | when major and minor version numbers should be changed. These values | |
236 | are set in Makefile.am near the definition of the library. If you | |
237 | make a change that requires changing the shared library version, | |
238 | please update Makefile.am. | |
239 | ||
240 | libospf exports far more than it should, and is needed by ospfapi | |
241 | clients. Only bump libospf for changes to functions for which it is | |
242 | reasonable for a user of ospfapi to call, and please err on the side | |
243 | of not bumping. | |
244 | ||
245 | There is no support intended for installing part of zebra. The core | |
246 | library libzebra and the included daemons should always be built and | |
247 | installed together. | |
248 | ||
74a2dd7b | 249 | |
5195e17f GT |
250 | GIT COMMIT SUBSMISSION |
251 | ||
252 | The preferred method for changes is to provide git commits via a | |
253 | publically-accessible git repository. | |
254 | ||
255 | All content guidelines in PATCH SUBMISSION apply. | |
256 | ||
257 | ||
d9fd04c2 | 258 | PATCH SUBMISSION |
259 | ||
d6bb5aa5 | 260 | * Send a clean diff against the 'master' branch of the quagga.git |
a8e474a5 | 261 | repository, in unified diff format, preferably with the '-p' argument to |
d6bb5aa5 | 262 | show C function affected by any chunk, and with the -w and -b arguments to |
a8e474a5 | 263 | minimise changes. E.g: |
d6bb5aa5 | 264 | |
3de4277b | 265 | git diff -up mybranch..remotes/quagga.net/master |
d9fd04c2 | 266 | |
5195e17f GT |
267 | It is preferable to use git format-patch, and even more preferred to |
268 | publish a git repostory. | |
d6bb5aa5 | 269 | |
5195e17f | 270 | If not using git format-patch, Include the commit message in the email. |
d6bb5aa5 | 271 | |
5195e17f GT |
272 | * After a commit, code should have comments explaining to the reviewer |
273 | why it is correct, without reference to history. The commit message | |
274 | should explain why the change is correct. | |
d6bb5aa5 | 275 | |
5195e17f | 276 | * Include NEWS entries as appropriate. |
d9fd04c2 | 277 | |
18323bb2 | 278 | * Include only one semantic change or group of changes per patch. |
d9fd04c2 | 279 | |
85cf0a0d | 280 | * Do not make gratuitous changes to whitespace. See the w and b arguments |
281 | to diff. | |
d9fd04c2 | 282 | |
283 | * State on which platforms and with what daemons the patch has been | |
284 | tested. Understand that if the set of testing locations is small, | |
285 | and the patch might have unforeseen or hard to fix consequences that | |
286 | there may be a call for testers on quagga-dev, and that the patch | |
287 | may be blocked until test results appear. | |
288 | ||
289 | If there are no users for a platform on quagga-dev who are able and | |
290 | willing to verify -current occasionally, that platform may be | |
291 | dropped from the "should be checked" list. | |
292 | ||
74a2dd7b | 293 | |
d6bb5aa5 | 294 | PATCH APPLICATION |
d9fd04c2 | 295 | |
296 | * Only apply patches that meet the submission guidelines. | |
297 | ||
d9fd04c2 | 298 | * If the patch might break something, issue a call for testing on the |
299 | mailinglist. | |
300 | ||
d6bb5aa5 PJ |
301 | * Give an appropriate commit message (see above), and use the --author |
302 | argument to git-commit, if required, to ensure proper attribution (you | |
303 | should still be listed as committer) | |
304 | ||
305 | * Immediately after commiting, double-check (with git-log and/or gitk). If | |
306 | there's a small mistake you can easily fix it with 'git commit --amend ..' | |
4134ceb7 | 307 | |
d9fd04c2 | 308 | * By committing a patch, you are responsible for fixing problems |
309 | resulting from it (or backing it out). | |
310 | ||
74a2dd7b | 311 | |
d9fd04c2 | 312 | STABLE PLATFORMS AND DAEMONS |
313 | ||
314 | The list of platforms that should be tested follow. This is a list | |
315 | derived from what quagga is thought to run on and for which | |
316 | maintainers can test or there are people on quagga-dev who are able | |
317 | and willing to verify that -current does or does not work correctly. | |
318 | ||
319 | BSD (Free, Net or Open, any platform) # without capabilities | |
320 | GNU/Linux (any distribution, i386) | |
1f8f61a7 | 321 | Solaris (strict alignment, any platform) |
18323bb2 | 322 | [future: NetBSD/sparc64] |
d9fd04c2 | 323 | |
324 | The list of daemons that are thought to be stable and that should be | |
325 | tested are: | |
326 | ||
327 | zebra | |
328 | bgpd | |
329 | ripd | |
330 | ospfd | |
331 | ripngd | |
1f431d2d | 332 | |
18323bb2 | 333 | Daemons which are in a testing phase are |
334 | ||
335 | ospf6d | |
336 | isisd | |
8035e9f0 | 337 | watchquagga |
18323bb2 | 338 | |
74a2dd7b | 339 | |
9e867fe6 | 340 | IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS |
341 | ||
342 | The source code of Quagga is based on two vendors: | |
343 | ||
344 | zebra_org (http://www.zebra.org/) | |
345 | isisd_sf (http://isisd.sf.net/) | |
346 | ||
d6bb5aa5 PJ |
347 | To import code from further sources, e.g. for archival purposes without |
348 | necessarily having to review and/or fix some changeset, create a branch from | |
349 | 'master': | |
9e867fe6 | 350 | |
d6bb5aa5 PJ |
351 | git checkout -b archive/foo master |
352 | <apply changes> | |
353 | git commit -a "Joe Bar <joe@example.com>" | |
354 | git push quagga archive/foo | |
9e867fe6 | 355 | |
d6bb5aa5 PJ |
356 | presuming 'quagga' corresponds to a file in your .git/remotes with |
357 | configuration for the appropriate Quagga.net repository. |