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