| -*- mode: text; -*- |
| $Id$ |
| |
| GUIDELINES FOR HACKING ON QUAGGA |
| |
| [this is a draft in progress] |
| |
| GNU coding standards apply. Indentation follows the result of |
| invoking GNU indent (as of 2.2.8a) with no arguments. Note that this |
| uses tabs instead of spaces where possible for leading whitespace, and |
| assumes that tabs are every 8 columns. Do not attempt to redefine the |
| location of tab stops. Note also that some indentation does not |
| follow GNU style. This is a historical accident, and we generally |
| only clean up whitespace when code is unmaintainable due to whitespace |
| issues, as fewer changes from zebra lead to easier merges. |
| |
| For GNU emacs, use indentation style "gnu". |
| |
| For Vim, use the following lines (note that tabs are at 8, and that |
| softtabstop sets the indentation level): |
| |
| set tabstop=8 |
| set softtabstop=2 |
| set shiftwidth=2 |
| set noexpandtab |
| |
| Be particularly careful not to break platforms/protocols that you |
| cannot test. |
| |
| New code should have good comments, and changes to existing code |
| should in many cases upgrade the comments when necessary for a |
| reviewer to conclude that the change has no unintended consequences. |
| |
| Each file in CVS should have the RCS keyword Id, somewhere very near |
| the top, commented out appropriately for the file type. Just add |
| <dollar>Id:<dollar>, replacing <dollar> with $. See line 2 of HACKING |
| for an example; on checkout :$ is expanded to include the value. |
| |
| Please document fully the proper use of a new function in the header file |
| in which it is declared. And please consult existing headers for |
| documentation on how to use existing functions. In particular, please consult |
| these header files: |
| |
| lib/log.h logging levels and usage guidance |
| [more to be added] |
| |
| If changing an exported interface, please try to deprecate the interface in |
| an orderly manner. If at all possible, try to retain the old deprecated |
| interface as is, or functionally equivalent. Make a note of when the |
| interface was deprecated and guard the deprecated interface definitions in |
| the header file, ie: |
| |
| /* Deprecated: 20050406 */ |
| #if !defined(QUAGGA_NO_DEPRECATED_INTERFACES) |
| #warning "Using deprecated <libname> (interface(s)|function(s))" |
| ... |
| #endif /* QUAGGA_NO_DEPRECATED_INTERFACES */ |
| |
| To ensure that the core Quagga sources do not use the deprecated interfaces |
| (you should update Quagga sources to use new interfaces, if applicable) |
| while allowing external sources to continue to build. Deprecated interfaces |
| should be excised in the next unstable cycle. |
| |
| Note: If you wish, you can test for GCC and use a function |
| marked with the 'deprecated' attribute. However, you must provide the |
| #warning for other compilers. |
| |
| If changing or removing a command definition, *ensure* that you properly |
| deprecate it - use the _DEPRECATED form of the appropriate DEFUN macro. This |
| is *critical*. Even if the command can no longer function, you *must* still |
| implement it as a do-nothing stub. Failure to follow this causes grief for |
| systems administrators. Deprecated commands should be excised in the next |
| unstable cycle. A list of deprecated commands should be collated for each |
| release. |
| |
| See also below regarding SHARED LIBRARY VERSIONING. |
| |
| COMPILE-TIME CONDITIONAL CODE |
| |
| Please think very carefully before making code conditional at compile time, |
| as it increases maintenance burdens and user confusion. In particular, |
| please avoid gratuitious --enable-.... switches to the configure script - |
| typically code should be good enough to be in Quagga, or it shouldn't be |
| there at all. |
| |
| When code must be compile-time conditional, try have the compiler make it |
| conditional rather than the C pre-processor. I.e. this: |
| |
| if (SOME_SYMBOL) |
| frobnicate(); |
| |
| rather than: |
| |
| #ifdef SOME_SYMBOL |
| frobnicate (); |
| #endif /* SOME_SYMBOL */ |
| |
| Note that the former approach requires ensuring that SOME_SYMBOL will be |
| defined (watch your AC_DEFINEs). |
| |
| CHANGELOG |
| |
| Add a ChangeLog entry whenever changing code, except for minor fixes |
| to a commit (with a ChangeLog entry) within the last few days. |
| |
| Most directories have a ChangeLog file; changes to code in that |
| directory should go in the per-directory ChangeLog. Global or |
| structural changes should also be mentioned in the top-level |
| ChangeLog. |
| |
| Certain directories do not contain project code, but contain project |
| meta-data, eg packaging information, changes to files in these |
| directory may not require the global ChangeLog to be updated (at the |
| discretion of the maintainer who usually maintains that meta-data). |
| Also, CVS meta-data such as cvsignore files do not require ChangeLog |
| updates, just a sane commit message. |
| |
| The ChangeLog should provide: |
| |
| * The date, in YYYY-MM-DD format |
| * The author's name and email. |
| * a short description of each change made |
| * file by file |
| * function by function (use of "ditto" is allowed) |
| |
| (detailed discussion of non-obvious reasoning behind and/or |
| implications of a change should be made in comments in the code |
| concerned). The changelog optionally may have a (general) description, |
| to provide a short description of the general intent of the patch. The |
| reason for such itemised ChangeLogs is to encourage the author to |
| self-review every line of the patch, as well as provide reviewers an |
| index of which changes are intended, along with a short description for |
| each. E.g.: |
| |
| 2012-05-29 Joe Bar <joe@example.com> |
| |
| * (general) Add a new DOWN state to the frob state machine |
| to allow the barinator to detect loss of frob. |
| * frob.h: (struct frob) Add DOWN state flag. |
| * frob.c: (frob_change) set/clear DOWN appropriately on state |
| change. |
| * bar.c: (barinate) Check frob for DOWN state. |
| |
| |
| HACKING THE BUILD SYSTEM |
| |
| If you change or add to the build system (configure.ac, any Makefile.am, |
| etc.), try to check that the following things still work: |
| |
| - make dist |
| - resulting dist tarball builds |
| - out-of-tree builds |
| |
| The quagga.net site relies on make dist to work to generate snapshots. It |
| must work. Commong problems are to forget to have some additional file |
| included in the dist, or to have a make rule refer to a source file without |
| using the srcdir variable. |
| |
| RELEASE PROCEDURE |
| |
| Tag the repository with release tag (follow existing conventions). |
| [This enables recreating the release, and is just good CM practice.] |
| |
| Check out the tag, and do a test build. |
| |
| In an empty directory, do a fresh checkout with -r <release-tag> |
| [This makes the dates in the tarball be the modified dates in CVS.] |
| |
| ./configure |
| make dist |
| |
| If any errors occur, move tags as needed and start over from the fresh |
| checkouts. Do not append to tarballs, as this has produced |
| non-standards-conforming tarballs in the past. |
| |
| [TODO: collation of a list of deprecated commands. Possibly can be scripted |
| to extract from vtysh/vtysh_cmd.c] |
| |
| |
| TOOL VERSIONS |
| |
| Require versions of support tools are listed in INSTALL.quagga.txt. |
| Required versions should only be done with due deliberation, as it can |
| cause environments to no longer be able to compile quagga. |
| |
| |
| SHARED LIBRARY VERSIONING |
| |
| [this section is at the moment just gdt's opinion] |
| |
| Quagga builds several shared libaries (lib/libzebra, ospfd/libospf, |
| ospfclient/libsopfapiclient). These may be used by external programs, |
| e.g. a new routing protocol that works with the zebra daemon, or |
| ospfapi clients. The libtool info pages (node Versioning) explain |
| when major and minor version numbers should be changed. These values |
| are set in Makefile.am near the definition of the library. If you |
| make a change that requires changing the shared library version, |
| please update Makefile.am. |
| |
| libospf exports far more than it should, and is needed by ospfapi |
| clients. Only bump libospf for changes to functions for which it is |
| reasonable for a user of ospfapi to call, and please err on the side |
| of not bumping. |
| |
| There is no support intended for installing part of zebra. The core |
| library libzebra and the included daemons should always be built and |
| installed together. |
| |
| |
| PATCH SUBMISSION |
| |
| * Send a clean diff against the head of CVS in unified diff format, eg by: |
| cvs <cvs opts> diff -upwb .... |
| |
| * Include ChangeLog and NEWS entries as appropriate before the patch |
| (or in it if you are 100% up to date). A good ChangeLog makes it easier to |
| review a patch, hence failure to include a good ChangeLog is prejudicial |
| to proper review of the patch, and hence the possibility of inclusion. |
| |
| * Include only one semantic change or group of changes per patch. |
| |
| * Do not make gratuitous changes to whitespace. See the w and b arguments |
| to diff. |
| |
| * State on which platforms and with what daemons the patch has been |
| tested. Understand that if the set of testing locations is small, |
| and the patch might have unforeseen or hard to fix consequences that |
| there may be a call for testers on quagga-dev, and that the patch |
| may be blocked until test results appear. |
| |
| If there are no users for a platform on quagga-dev who are able and |
| willing to verify -current occasionally, that platform may be |
| dropped from the "should be checked" list. |
| |
| |
| PATCH APPLICATION TO CVS |
| |
| * Only apply patches that meet the submission guidelines. |
| |
| * If a patch is large (perhaps more than 100 new/changed lines), tag |
| the repository before and after the change with e.g. before-foo-fix |
| and after-foo-fix. |
| |
| * If the patch might break something, issue a call for testing on the |
| mailinglist. |
| |
| * Give an appropriate commit message, prefixed with a category name |
| (either the name of the daemon, the library component or the general |
| topic) and a one-line short summary of the change as the first line, |
| suitable for use as a Subject in an email. The ChangeLog entry should |
| suffice as the body of the commit message, if it does not, then the |
| ChangeLog entry itself needs to be corrected. The commit message text |
| should be identical to that added to the ChangeLog message. (One |
| suggestion: when commiting, use your editor to read in the ChangeLog |
| and delete all previous ChangeLogs.) An example: |
| |
| ---------------------------------------------------------------- |
| [frob] Defangulator needs to specify frob |
| |
| 2012-05-12 Joe Bar <joe@example.com> |
| |
| * frobinate.c: (frob_lookup) fix NULL dereference |
| (defangulate) check whether frob is in state FROB_VALID |
| before defangulating. |
| ---------------------------------------------------------------- |
| |
| * By committing a patch, you are responsible for fixing problems |
| resulting from it (or backing it out). |
| |
| |
| STABLE PLATFORMS AND DAEMONS |
| |
| The list of platforms that should be tested follow. This is a list |
| derived from what quagga is thought to run on and for which |
| maintainers can test or there are people on quagga-dev who are able |
| and willing to verify that -current does or does not work correctly. |
| |
| BSD (Free, Net or Open, any platform) # without capabilities |
| GNU/Linux (any distribution, i386) |
| Solaris (strict alignment, any platform) |
| [future: NetBSD/sparc64] |
| |
| The list of daemons that are thought to be stable and that should be |
| tested are: |
| |
| zebra |
| bgpd |
| ripd |
| ospfd |
| ripngd |
| |
| Daemons which are in a testing phase are |
| |
| ospf6d |
| isisd |
| watchquagga |
| |
| |
| IMPORT OR UPDATE VENDOR SPECIFIC ROUTING PROTOCOLS |
| |
| The source code of Quagga is based on two vendors: |
| |
| zebra_org (http://www.zebra.org/) |
| isisd_sf (http://isisd.sf.net/) |
| |
| [20041105: Is isisd.sf.netf still where isisd word is happening, or is |
| the quagga repo now the canonical place? The last tarball on sf is |
| two years old. --gdt] |
| |
| In order to import source code, the following procedure should be used: |
| |
| * Tag the Current Quagga CVS repository: |
| |
| cvs tag import_isisd_sf_20031223 |
| |
| * Import the source code into the Quagga's framework. You must not modified |
| this source code. It will be merged later. |
| |
| cd dir_isisd |
| export CVSROOT=:pserver:LOGIN@anoncvs.quagga.net:/var/cvsroot |
| cvs import quagga/isisd isisd_sf isisd_sf_20031223 |
| ---COMMENTS--- |
| Vendor: [isisd_sf] Sampo's ISISd from Sourceforge |
| Tag: [isisd_sf_20031217] Current CVS release |
| --- |
| |
| * Update your Quagga's directory: |
| |
| cd dir_quagga |
| cvs update -dP |
| |
| or |
| |
| cvs co -d quagga_isisd quagga |
| |
| * Merge the code, then commit: |
| |
| cvs commit |
| |