[PATCH] Fix #define logic regarding info format handling

[C. Michael Pilato]

This is totally a drive-by patch.  I happened to be reading this section
of code, and something didn't look quite right.  I admit a general
ignorance of the CVS codebase, but I hope that doesn't get in the way of
someone knowledgeable evaluating this patch.

The code segment in question is this from commit.c:

    /* On success, retrieve the new version number of the file and
       copy it into the log information (see logmsg.c
       (logfile_write) for more details).  We should only update
       the version number for files that have been added or
       modified but not removed since classify_file_internal
       will return the version number of a file even after it has
       been removed from the archive, which is not the behavior we
       want for our commitlog messages; we want the old version
       number and then "NONE, unless UseNewInfoFmtStrings has
       been specified in the config.  Expose the real version in that
       case and allow the trigger scripts to decide how to use it.  */

    if (ci->status != T_REMOVED
#ifdef SUPPORT_OLD_INFO_FMT_STRINGS
        || config->UseNewInfoFmtStrings
#endif
        )
    {
    ...

My understanding of that somewhat confusing comment is that the
new-style info handling doesn't care if ci->status is T_REMOVED or not
-- it will always populate the new revision slot.  Old-style info
handling (which requires both support for such things, and configuration
that dictates it), doesn't want the new revision for removed things --
it just wants the old revision and then "NONE".  This all seems like a
reasonable thing to do, too.  The comment is a bit confusing because it
seems to run like "behavior unless condition, unless condition".
(Comparing with the 1.11.21 sources, I can see that that final ", unless
condition" segment was just tacked onto a version of this comment that
predates new info format styles altogether.)

The code, however, doesn't seem to reflect that understanding, hence the
patch.  Of course, if my understanding of the comment is wrong, then my
patch is probably likewise wrong.

I'm not subscribed to this list, but would love to be Cc:ed on
follow-ups to this thread.  Also, I'm happy to submit a follow-up patch
that fixes the comment a little more comprehensible.  Thanks.

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

* ccvs/src/commit.c
  (commit_fileproc): Fix #ifdef logic bug.  When not supporting old
    info format strings, behave in the new way (storing new revisions
    of removed files).  Otherwise, our behavior depends on the
    config->UseNewInfoFmtStrings.


Index: src/commit.c
===================================================================
RCS file: /sources/cvs/ccvs/src/commit.c,v
retrieving revision 1.268
diff -u -r1.268 commit.c
--- src/commit.c        31 May 2006 16:03:02 -0000      1.268
+++ src/commit.c        31 Aug 2006 17:10:51 -0000
@@ -1584,11 +1584,9 @@
            been specified in the config.  Expose the real version in that
            case and allow the trigger scripts to decide how to use it.  */
             
-        if (ci->status != T_REMOVED
 #ifdef SUPPORT_OLD_INFO_FMT_STRINGS
-           || config->UseNewInfoFmtStrings
+       if (ci->status != T_REMOVED || config->UseNewInfoFmtStrings)
 #endif
-           )
        {
            p = findnode (ulist, finfo->file);
            if (p)

signature.asc
Description: OpenPGP digital signature