contrib/cvs-1.12/HACKING

   1 How to write code for CVS
   2
   3 * License of CVS
   4
   5     CVS is Copyright (C) 1989-2005 The Free Software Foundation, Inc.
   6
   7     This program is free software; you can redistribute it and/or modify
   8     it under the terms of the GNU General Public License as published by
   9     the Free Software Foundation; either version 1, or (at your option)
  10     any later version.
  11
  12     More details are available in the COPYING file but, in simplified
  13     terms, this means that any distributed modifications you make to
  14     this software must also be released under the GNU General Public
  15     License.
  16
  17     This program is distributed in the hope that it will be useful,
  18     but WITHOUT ANY WARRANTY; without even the implied warranty of
  19     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  20     GNU General Public License for more details.
  21
  22 * Source
  23
  24 Patches against the development version of CVS are most likely to be accepted:
  25
  26         $ export CVS_RSH="ssh"
  27         $ cvs -z3 -d:ext:anoncvs@savannah.nongnu.org:/cvsroot/cvs co ccvs
  28
  29 * Compiler options
  30
  31 If you are using GCC, you'll want to configure with -Wall, which can
  32 detect many programming errors.  This is not the default because it
  33 might cause spurious warnings, but at least on some machines, there
  34 should be no spurious warnings.  For example:
  35
  36         $ ./configure CPPFLAGS=-Wall
  37
  38 * Backwards Compatibility
  39
  40 Only bug fixes are accepted into the stable branch.  New features should be
  41 applied to the trunk.
  42
  43 If it is not inextricable from a bug fix, CVS's output (to stdout/stderr)
  44 should not be changed on the stable branch in order to best support scripts and
  45 other tools which parse CVS's output.  It is ok to change output between
  46 feature releases (on the trunk), though such changes should be noted in the
  47 NEWS file.
  48
  49 Changes in the way CVS responds to command line options, config options, etc.
  50 should be accompanied by deprecation warnings for an entire stable series of
  51 releases before being changed permanently, if at all possible.
  52
  53 * Indentation style
  54
  55 CVS mostly uses a consistent indentation style which looks like this:
  56
  57 void
  58 foo (char *arg, int c)
  59 {
  60     long aflag;
  61
  62     if (arg)
  63     {
  64         bar (arg);
  65         baz (arg);
  66     }
  67
  68     switch (c)
  69     {
  70       case 'A':
  71         aflag = 1;
  72         break;
  73       case 'E':
  74         go to myerr;
  75     }
  76
  77     printf ("Literal string line 1\n"
  78             "Literal string line 2\n"
  79             "Literal string line 3\n");
  80     return;
  81
  82   myerr:
  83     printf ("Error argument found\n");
  84 }
  85
  86   - Do not cast NULL unless it is a stdarg argument to a function.
  87
  88   - Do not cast functions returning (void *), e.g., xmalloc ().
  89
  90   - Do not cast non-stdarg arguments to a function to '(void *)'
  91     except to drop a 'const' modifier.
  92
  93   - Snuggle ! close to its expression (i.e., '! foo' => '!foo').
  94
  95   - Functions and C statements have a space before the "("
  96     and the expression does not have a leading or trailing space
  97     (i.e., 'if( foo )' => 'if (foo)'), although it is sometimes
  98     desirable to add a newline after the "(" for #ifdef'd code.
  99
 100   - For switch statements, indent 'case' by 2 and the body of the case
 101     by an additional 2 spaces.
 102
 103   - Labels should be indented by 2 spaces rather than the 4 spaces
 104     used by the rest of the current block level.
 105
 106
 107     while ((var = next_arg ()) != 0)
 108     {
 109       again:
 110         switch (var)
 111         {
 112           case ONE:
 113             code_for_case_one ();
 114             break;
 115           case TWO:
 116             code_for_case_two ();
 117             break;
 118           case THREE:
 119             push_arg (RESET_ONE);
 120             var = ONE;
 121             go to again;
 122           default:
 123             code_for_default_case ():
 124             break;
 125         }
 126     }
 127
 128   - NULL-protected free goes on one line if possible, for example:
 129
 130         if (var)
 131             free (var);
 132         if (var2 != NULL)
 133             free (var2);
 134
 135     should be written as:
 136
 137         if (var) free (var);
 138         if (var2) free (var2);
 139
 140     if the value needs to be set to NULL after the free, then use
 141
 142         if (var)
 143         {
 144             free (var):
 145             var = NULL;
 146         }
 147
 148     as the idiom.
 149
 150   - Use whitespace in arithmetic expressions, for example
 151
 152         foo (arg+2);
 153
 154     should be written as
 155
 156         foo (arg + 2);
 157
 158     likewise for normal arithmetic expression assignments.
 159
 160   - Argument lists get a space after a comma.
 161
 162   - Do not parenthesize return values unless the expression needs to
 163     span multiple lines.
 164
 165   - Cast negative constants when used in assignments or comparisons
 166     with unsigned types.
 167
 168   - Try to be consistent with block comments:
 169
 170     /* This is a good block comment (spanning multiple lines of text).
 171      * It starts with slash-star, leads each line with a star aligned with
 172      * the first, and ends with a similarly aligned star-slash on a line
 173      * by itself.
 174      */
 175
 176     /* This is a bad block comment,
 177        because it can make it hard to tell what is code
 178        and what is not code. */
 179
 180   - Sentences in comments should have a double space between each
 181     period (.) and the beginning of the next sentence.
 182
 183   - Conditional expressions that need to be split should put the ?
 184     operator on the new line.
 185
 186   - Follow GNU standards for breaking logical expressions over
 187     multiple lines where possible.
 188
 189   - Do not snuggle open-lbrace blocks.
 190
 191   - Remove '#if 0' code where possible. Add a comment FIXME if it
 192     really is a possible problem.
 193
 194   - Remove commented-out code where possible (FIXME blocks are
 195     excepted).
 196
 197 The file cvs-format.el contains settings for emacs and the NEWS file
 198 contains a set of options for the indent program which I haven't tried
 199 but which are correct as far as I know.  You will find some code which
 200 does not conform to this indentation style; the plan is to re-indent it
 201 as those sections of the code are changed (one function at a time,
 202 perhaps).
 203
 204 In a submitted patch it is acceptable to refrain from changing the
 205 indentation of large blocks of code to minimize the size of the patch;
 206 the person checking in such a patch should re-indent it.
 207
 208 * Portability
 209
 210 The general rule for portability is that it is only worth including
 211 portability cruft for systems on which people are actually testing and
 212 using new CVS releases.  Without testing, CVS will fail to be portable
 213 for any number of unanticipated reasons.
 214
 215 CVS is now assuming a freestanding C89 compiler.  If you don't have one, you
 216 should find an old release of GCC that did not require a freestanding C89
 217 compiler to build, build that on your system, build a newer release of GCC
 218 if you wish, then build CVS using GCC as your freestanding C89 compiler.
 219
 220 A freestanding C89 compiler is guaranteed to support function prototypes,
 221 void *, and assert().
 222
 223 The following headers can be assumed and are included from lib/system.h for a
 224 freestanding C89 implementation: <float.h>, <limits.h>, <stdarg.h>, <stddef.h>.
 225 We are not assuming the other standard headers listed by C89 (hosted headers)
 226 because these four headers are the only headers guaranteed to be shipped with
 227 a C89 compiler (freestanding compiler).  We are not currently assuming that the
 228 system the compiler is running on provides the rest of the C89 headers.
 229
 230 The following C89 hosted headers can be assumed due to their presence in UNIX
 231 version 7 and are included from lib/system.h: <assert.h>, <ctype.h>, <errno.h>,
 232 <math.h>, <setjmp.h>, <signal.h>, <stdio.h>.  <time.h> can also be assumed but
 233 is included via lib/xtime.h via lib/system.h to include some Autoconf magic
 234 which avoids including <time.h> and <sys/time.h> on systems that can't handle
 235 both.
 236
 237 The following C89 headers are also assumed since we believe GCC includes them
 238 even on systems where it is installed as a freestanding compiler when the
 239 system lacks them, despite their not being required: <stdlib.h>, <string.h>.
 240 When the system does not lack these headers, they can sometimes not be
 241 standards compatible, but GCC provides a script, `fixincludes', for the purpose
 242 of fixing ANSI conformance problems and we think we can rely on asking users to
 243 either use GCC or run this script to fix conformance problems manually.  A
 244 GNULIB developer has made a statement that if this turns out to be a problem,
 245 GNULIB <stdlib.h> and <string.h> substitutes could be included in GNULIB, so if
 246 we discover the problem, this should be discussed on <bug-gnulib@gnu.org>.
 247
 248 A substitute C99 <stdbool.h> is included from GNULIB for platforms that lack
 249 this header.  Please see the comments in the lib/stdbool_.h file for its
 250 limitations.
 251
 252 <sys/types.h> can be assumed despite a lack of a presence in even C99, since
 253 it has been around nearly forever and no-one has ever complained about our code
 254 assuming its existence.
 255
 256 CVS has also been assuming <pwd.h> for some time.  I am unsure of the
 257 rationale.
 258
 259 GNULIB also assumes <sys/stat.h>.  I am unsure of the rationale.
 260
 261 A substitute POSIX.2 <fnmatch.h> header and fnmatch() function is provided for
 262 systems that lack them.  Similarly for the non-standard <alloca.h> header and
 263 alloca() function.  Other substitute headers and functions are also provided
 264 when needed.  See the lib directory or the maint-aux/srclist.txt file for more
 265 information.
 266
 267 Please do not use multi-line strings.  Despite their common acceptance by many
 268 compilers, they are not part of the ANSI C specification.  As of GCC version
 269 3.3, they are no longer supported.  See the Indentation Style section above for
 270 an example of a literal string which is not multi-line but which will print
 271 multiple lines.
 272
 273 * Other style issues
 274
 275 When composing header files, do not declare function prototypes using the
 276 `extern' storage-class identifier.  Under C89, there is no functional
 277 difference between a function declaration with and without `extern', unless the
 278 function is declared `static'.  This is NOT the case for global variables.
 279 Global variables declared in header files MUST be declared `extern'.  For
 280 example:
 281
 282 /* Global variables */
 283 extern int foo;
 284 extern char *bar;
 285
 286 /* Function declarations */
 287 int make_foo(void);
 288 char *make_bar(int _foobar);
 289
 290 * Run-time behaviors
 291
 292 Use assert() to check "can't happen" conditions internal to CVS.  We
 293 realize that there are functions in CVS which instead return NULL or
 294 some such value (thus confusing the meaning of such a returned value),
 295 but we want to fix that code.  Of course, bad input data, a corrupt
 296 repository, bad options, etc., should always print a real error
 297 message instead.
 298
 299 Do not use arbitrary limits (such as PATH_MAX) except perhaps when the
 300 operating system or some external interface requires it.  We spent a
 301 lot of time getting rid of them, and we don't want to put them back.
 302 If you find any that we missed, please report it as with other bugs.
 303 In most cases such code will create security holes (for example, for
 304 anonymous read-only access via the CVS protocol, or if a WWW cgi script
 305 passes client-supplied arguments to CVS).
 306
 307 Although this is a long-term goal, it also would be nice to move CVS
 308 in the direction of reentrancy.  This reduces the size of the data
 309 segment and will allow a multi-threaded server if that is desirable.
 310 It is also useful to write the code so that it can be easily be made
 311 reentrant later.  For example, if you need to pass data to some functions,
 312 you need a static variable, but use a single pointer so that when the function
 313 is fixed to pass along the argument, then the code can easily use that
 314 argument.
 315
 316 * Coding standards in general
 317
 318 Generally speaking the GNU coding standards are mostly used by CVS
 319 (but see the exceptions mentioned above, such as indentation style,
 320 and perhaps an exception or two we haven't mentioned).  This is the
 321 file standards.text at the GNU FTP sites. The primary URL for this
 322 information is http://www.gnu.org/prep/standards/ which contains links
 323 to many different formats of the standards.
 324
 325 * Regenerating Build Files (UNIX)
 326
 327 On UNIX, if you wish to change the build files, you will need Autoconf and
 328 Automake.
 329
 330 Some combinations of Automake and Autoconf versions may break the
 331 CVS build if file timestamps aren't set correctly and people don't
 332 have the same versions the developers do, so the rules to run them
 333 automatically aren't included in the generated Makefiles unless you run
 334 configure with --enable-maintainer-mode.
 335
 336 The CVS Makefiles and configure script were built using Automake 1.9.6 and
 337 Autoconf 2.59, respectively.
 338
 339 There is a known bug in Autoconf 2.57 that will prevent the configure
 340 scripts it generates from working on some platforms.  Other combinations of
 341 autotool versions may or may not work.  If you get other versions to work,
 342 please send a report to <bug-cvs@nongnu.org>.
 343
 344 * Regenerating Build Files (Windows)
 345
 346 If for some reason you end up regenerating the *.mak files to submit a patch,
 347 please run windows-NT/fix-msvc-mak.pl to remove the absolute paths from the
 348 generated *.mak files before generating any patches.
 349
 350 * Rebuilding Yacc sources
 351
 352 The lib/getdate.y file requires GNU Bison 1.875 to rebuild lib/getdate.c.  Not
 353 having GNU Bison 1.875 should not stop the build unless the lib/getdate.c file
 354 is actually missing, perhaps deleted via `make maintainerclean'.
 355
 356 * Writing patches (strategy)
 357
 358 Only some kinds of changes are suitable for inclusion in the
 359 "official" CVS.  Bugfixes, where CVS's behavior contradicts the
 360 documentation and/or expectations that everyone agrees on, should be
 361 OK (strategically).  For features, the desirable attributes are that
 362 the need is clear and that they fit nicely into the architecture of
 363 CVS.  Is it worth the cost (in terms of complexity or any other
 364 tradeoffs involved)?  Are there better solutions?
 365
 366 If the design is not yet clear (which is true of most features), then
 367 the design is likely to benefit from more work and community input.
 368 Make a list of issues, or write documentation including rationales for
 369 how one would use the feature.  Discuss it with coworkers, a
 370 newsgroup, or a mailing list, and see what other people think.
 371 Distribute some experimental patches and see what people think.  The
 372 intention is arrive at some kind of rough community consensus before
 373 changing the "official" CVS.  Features like zlib, encryption, and
 374 the RCS library have benefited from this process in the past.
 375
 376 If longstanding CVS behavior, that people may be relying on, is
 377 clearly deficient, it can be changed, but only slowly and carefully.
 378 For example, the global -q option was introduced in CVS 1.3 but the
 379 command -q options, which the global -q replaced, were not removed
 380 until CVS 1.6.
 381
 382 * Writing patches (tactics)
 383
 384 When you first distribute a patch it may be suitable to just put forth
 385 a rough patch, or even just an idea.  But before the end of the
 386 process the following should exist:
 387
 388   - ChangeLog entry (see the GNU coding standards for details).
 389
 390   - Changes to the NEWS file and cvs.texinfo, if the change is a
 391     user-visible change worth mentioning.
 392
 393   - Somewhere, a description of what the patch fixes (often in
 394     comments in the code, or maybe the ChangeLog or documentation).
 395
 396   - Most of the time, a test case (see TESTS).  It can be quite
 397     frustrating to fix a bug only to see it reappear later, and adding
 398     the case to the testsuite, where feasible, solves this and other
 399     problems.  See the TESTS file for notes on writing new tests.
 400
 401 If you solve several unrelated problems, it is generally easier to
 402 consider the desirability of the changes if there is a separate patch
 403 for each issue.  Use context diffs or unidiffs for patches.
 404
 405 Include words like "I grant permission to distribute this patch under
 406 the terms of the GNU Public License" with your patch.  By sending a
 407 patch to bug-cvs@nongnu.org, you implicitly grant this permission.
 408
 409 Submitting a patch to bug-cvs is the way to reach the people who have
 410 signed up to receive such submissions (including CVS developers), but
 411 there may or may not be much (or any) response.  If you want to pursue
 412 the matter further, you are probably best off working with the larger
 413 CVS community.  Distribute your patch as widely as desired (mailing
 414 lists, newsgroups, web sites, whatever).  Write a web page or other
 415 information describing what the patch is for.  It is neither practical
 416 nor desirable for all/most contributions to be distributed through the
 417 "official" (whatever that means) mechanisms of CVS releases and CVS
 418 developers.  Now, the "official" mechanisms do try to incorporate
 419 those patches which seem most suitable for widespread usage, together
 420 with test cases and documentation.  So if a patch becomes sufficiently
 421 popular in the CVS community, it is likely that one of the CVS
 422 developers will eventually try to do something with it.  But dealing
 423 with the CVS developers may be the last step of the process rather
 424 than the first.
 425
 426 * What is the schedule for the next release?
 427
 428 There isn't one.  That is, upcoming releases are not announced (or
 429 even hinted at, really) until the feature freeze which is
 430 approximately 2 weeks before the final release (at this time test
 431 releases start appearing and are announced on info-cvs).  This is
 432 intentional, to avoid a last minute rush to get new features in.
 433
 434 * Mailing lists
 435
 436 In addition to the mailing lists listed in the README file, developers should
 437 take particular note of the following mailling lists:
 438
 439     bug-cvs:  This is the list which users are requested to send bug reports
 440       to.  General CVS development and design discussions also take place on
 441       this list.
 442     info-cvs:  This list is intended for user questions, but general CVS
 443       development and design discussions sometimes take place on this list.
 444     cvs-cvs:  The only messages sent to this list are sent
 445       automatically, via the CVS `loginfo' mechanism, when someone
 446       checks something in to the master CVS repository.
 447     cvs-test-results:  The only messages sent to this list are sent
 448       automatically, daily, by a script which runs "make check"
 449       and "make remotecheck" on the master CVS sources.
 450
 451 To subscribe to any of these lists, send mail to <list>-request@nongnu.org
 452 or visit http://savannah.nongnu.org/mail/?group=cvs and follow the instructions
 453 for the list you wish to subscribe to.