]> git.proxmox.com Git - mirror_frr.git/blame - HACKING
[doc] Update HACKING to reflect SCM changes and latest practice
[mirror_frr.git] / HACKING
CommitLineData
d9fd04c2 1-*- mode: text; -*-
d6bb5aa5
PJ
2$QuaggaId: Format:%an, %ai, %h$ $
3
4Contents:
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
20GUIDELINES FOR HACKING ON QUAGGA
21
d9fd04c2 22[this is a draft in progress]
23
863076db 24GNU coding standards apply. Indentation follows the result of
25invoking GNU indent (as of 2.2.8a) with no arguments. Note that this
26uses tabs instead of spaces where possible for leading whitespace, and
27assumes that tabs are every 8 columns. Do not attempt to redefine the
28location of tab stops. Note also that some indentation does not
29follow GNU style. This is a historical accident, and we generally
30only clean up whitespace when code is unmaintainable due to whitespace
31issues, as fewer changes from zebra lead to easier merges.
32
33For GNU emacs, use indentation style "gnu".
34
35For Vim, use the following lines (note that tabs are at 8, and that
36softtabstop sets the indentation level):
37
38set tabstop=8
39set softtabstop=2
40set shiftwidth=2
41set noexpandtab
d9fd04c2 42
2934f28e 43Be particularly careful not to break platforms/protocols that you
44cannot test.
45
46New code should have good comments, and changes to existing code
47should in many cases upgrade the comments when necessary for a
48reviewer to conclude that the change has no unintended consequences.
49
d6bb5aa5
PJ
50Each file in the Git repository should have a git format-placeholder (like
51an RCS Id keyword), somewhere very near the top, commented out appropriately
52for the file type. The placeholder used for Quagga (replacing <dollar> with
53$) is:
54
55 $QuaggaId: <dollar>Format:%an, %ai, %h<dollar> $
56
57See line 2 of HACKING for an example;
58
59This placeholder string will be expanded out by the 'git archive' commands,
60wihch is used to generate the tar archives for snapshots and releases.
697877eb 61
5e764774 62Please document fully the proper use of a new function in the header file
63in which it is declared. And please consult existing headers for
64documentation on how to use existing functions. In particular, please consult
65these header files:
66
67 lib/log.h logging levels and usage guidance
68 [more to be added]
69
1eb8ef25 70If changing an exported interface, please try to deprecate the interface in
71an orderly manner. If at all possible, try to retain the old deprecated
72interface as is, or functionally equivalent. Make a note of when the
73interface was deprecated and guard the deprecated interface definitions in
74the 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
82To ensure that the core Quagga sources do not use the deprecated interfaces
83(you should update Quagga sources to use new interfaces, if applicable)
84while allowing external sources to continue to build. Deprecated interfaces
85should be excised in the next unstable cycle.
86
74a2dd7b 87Note: If you wish, you can test for GCC and use a function
88marked with the 'deprecated' attribute. However, you must provide the
89#warning for other compilers.
90
1eb8ef25 91If changing or removing a command definition, *ensure* that you properly
92deprecate it - use the _DEPRECATED form of the appropriate DEFUN macro. This
93is *critical*. Even if the command can no longer function, you *must* still
94implement it as a do-nothing stub. Failure to follow this causes grief for
95systems administrators. Deprecated commands should be excised in the next
96unstable cycle. A list of deprecated commands should be collated for each
97release.
98
99See also below regarding SHARED LIBRARY VERSIONING.
5e764774 100
750e8146
PJ
101COMPILE-TIME CONDITIONAL CODE
102
103Please think very carefully before making code conditional at compile time,
104as it increases maintenance burdens and user confusion. In particular,
105please avoid gratuitious --enable-.... switches to the configure script -
106typically code should be good enough to be in Quagga, or it shouldn't be
107there at all.
108
109When code must be compile-time conditional, try have the compiler make it
110conditional rather than the C pre-processor. I.e. this:
111
112 if (SOME_SYMBOL)
113 frobnicate();
114
115rather than:
116
117 #ifdef SOME_SYMBOL
118 frobnicate ();
119 #endif /* SOME_SYMBOL */
120
121Note that the former approach requires ensuring that SOME_SYMBOL will be
122defined (watch your AC_DEFINEs).
74a2dd7b 123
d6bb5aa5 124COMMIT MESSAGES
2934f28e 125
d6bb5aa5 126The commit message should provide:
2934f28e 127
d6bb5aa5
PJ
128* A suitable one-line summary as the very first line of the message, in the
129 form:
2934f28e 130
d6bb5aa5 131 [topic] high-level, one line summary
ca6383ba 132
d6bb5aa5 133 Where topic may be name of a subdirectory, and/or daemon.
ca6383ba 134
d6bb5aa5
PJ
135* An optional introduction, discussing the general intent of the change.
136* a short description of each change made, preferably:
ca6383ba 137 * file by file
d6bb5aa5 138 * function by function (use of "ditto", or globs is allowed)
ca6383ba 139
d6bb5aa5 140to provide a short description of the general intent of the patch.
ca6383ba 141
d6bb5aa5
PJ
142The reason for such itemised commit messages is to encourage the author to
143self-review every line of the patch, as well as provide reviewers an index
144of which changes are intended, along with a short description for each.
145An example (where the general discussion is obviously somewhat redundant,
146given the one-line summary):
147
148[zebra] Enhance frob FSM to detect loss of frob
ca6383ba 149
150 * (general) Add a new DOWN state to the frob state machine
d6bb5aa5 151 to allow the barinator to detect loss of frob.
ca6383ba 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.
1f8f61a7 156
74a2dd7b 157
158HACKING THE BUILD SYSTEM
159
160If you change or add to the build system (configure.ac, any Makefile.am,
161etc.), try to check that the following things still work:
162
163 - make dist
164 - resulting dist tarball builds
165 - out-of-tree builds
166
167The quagga.net site relies on make dist to work to generate snapshots. It
d6bb5aa5 168must work. Common problems are to forget to have some additional file
74a2dd7b 169included in the dist, or to have a make rule refer to a source file without
170using the srcdir variable.
171
0d7e9134 172RELEASE PROCEDURE
173
d6bb5aa5
PJ
174* Tag the apppropriate commit with a release tag (follow existing
175 conventions).
0d7e9134 176 [This enables recreating the release, and is just good CM practice.]
177
d6bb5aa5
PJ
178* Create a fresh tar archive of the quagga.net repository, and do a test
179 build:
0d7e9134 180
d6bb5aa5
PJ
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
0d7e9134 185
d6bb5aa5
PJ
186 ./update-autotools
187 ./configure
188 make
189 make dist
0d7e9134 190
d6bb5aa5
PJ
191The tarball which 'make dist' creates is the tarball to be released! The
192git-archive step ensures you're working with code corresponding to that in
193the official repository, and also carries out keyword expansion. If any
194errors occur, move tags as needed and start over from the fresh checkouts.
195Do not append to tarballs, as this has produced non-standards-conforming
196tarballs in the past.
0d7e9134 197
1eb8ef25 198[TODO: collation of a list of deprecated commands. Possibly can be scripted
199to extract from vtysh/vtysh_cmd.c]
200
74a2dd7b 201
fbb67099 202TOOL VERSIONS
203
204Require versions of support tools are listed in INSTALL.quagga.txt.
205Required versions should only be done with due deliberation, as it can
206cause environments to no longer be able to compile quagga.
207
74a2dd7b 208
b7a97f82 209SHARED LIBRARY VERSIONING
210
211[this section is at the moment just gdt's opinion]
212
213Quagga builds several shared libaries (lib/libzebra, ospfd/libospf,
214ospfclient/libsopfapiclient). These may be used by external programs,
215e.g. a new routing protocol that works with the zebra daemon, or
216ospfapi clients. The libtool info pages (node Versioning) explain
217when major and minor version numbers should be changed. These values
218are set in Makefile.am near the definition of the library. If you
219make a change that requires changing the shared library version,
220please update Makefile.am.
221
222libospf exports far more than it should, and is needed by ospfapi
223clients. Only bump libospf for changes to functions for which it is
224reasonable for a user of ospfapi to call, and please err on the side
225of not bumping.
226
227There is no support intended for installing part of zebra. The core
228library libzebra and the included daemons should always be built and
229installed together.
230
74a2dd7b 231
d9fd04c2 232PATCH SUBMISSION
233
d6bb5aa5
PJ
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:
238
239 git diff -u -p -w -b mybranch..remotes/quagga.net/master
d9fd04c2 240
d6bb5aa5
PJ
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.
d9fd04c2 249
18323bb2 250* Include only one semantic change or group of changes per patch.
d9fd04c2 251
85cf0a0d 252* Do not make gratuitous changes to whitespace. See the w and b arguments
253 to diff.
d9fd04c2 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
74a2dd7b 265
d6bb5aa5 266PATCH APPLICATION
d9fd04c2 267
268* Only apply patches that meet the submission guidelines.
269
d9fd04c2 270* If the patch might break something, issue a call for testing on the
271 mailinglist.
272
d6bb5aa5
PJ
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 ..'
4134ceb7 279
d9fd04c2 280* By committing a patch, you are responsible for fixing problems
281 resulting from it (or backing it out).
282
74a2dd7b 283
d9fd04c2 284STABLE PLATFORMS AND DAEMONS
285
286The list of platforms that should be tested follow. This is a list
287derived from what quagga is thought to run on and for which
288maintainers can test or there are people on quagga-dev who are able
289and 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)
1f8f61a7 293 Solaris (strict alignment, any platform)
18323bb2 294 [future: NetBSD/sparc64]
d9fd04c2 295
296The list of daemons that are thought to be stable and that should be
297tested are:
298
299 zebra
300 bgpd
301 ripd
302 ospfd
303 ripngd
1f431d2d 304
18323bb2 305Daemons which are in a testing phase are
306
307 ospf6d
308 isisd
8035e9f0 309 watchquagga
18323bb2 310
74a2dd7b 311
9e867fe6 312IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS
313
314The 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
d6bb5aa5
PJ
319To import code from further sources, e.g. for archival purposes without
320necessarily having to review and/or fix some changeset, create a branch from
321'master':
9e867fe6 322
d6bb5aa5
PJ
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
9e867fe6 327
d6bb5aa5
PJ
328presuming 'quagga' corresponds to a file in your .git/remotes with
329configuration for the appropriate Quagga.net repository.