Re: do_verify patch for cvs top-of-tree

[Derek R. Price]

"Mark D. Baushke" wrote:

> Hi Derek,
>
> Looking at portability, I see a problem with satifying the first of
> your requests.
>
> > 1. This looks pretty good on cursory inspection, but the logmsg file
> > shouldn't always be reloaded after the verifymsg script runs.  The
> > timestamp of the file should be cached and checked for changes before
> > reloading.
>
> Unless I use sleep_past(), to wait until the next second between
> writing out the log message file for the verification script, or I use
> unportable sub-second entries from the stat structure, there is a race
> condition for determining if the log message needs to be reloaded or
> not. To be honest, I would rather re-read the file all the time
> instead of adding an extra second to the commit...
>
> Here is the patch that makes it use sleep_past(). What are your
> thoughts on this matter?

Well, I don't think the extra second would bother any but the most ineptly
scripted commits much, considering that this check will only happen once and 
will
usually be immediately following a user edit, which takes forever relative to 
the
second.  Besides - it's not always a full second.

Hmm.  I'm still inclined to think that the single stat is much less CPU and IO
intensive when no changes have been made.  I've seen some pretty drasticaly
overloaded systems which make this scenario preferrable - CVS can be quite a
resource hog as it stands.

Of course on a system where you know the script will always change the log
message, the stat is sub-optimal.

What about a CVSROOT/config option?  RereadLogAfterVerify=no|stat|always or some
such?

Any differeing opinions?  The history of the issue is here:
http://ccvs.cvshome.org/issues/show_bug.cgi?id=19 and I attached Mark's new 
patch
to it rather than forward it.

Mark, my comments on your new patch:

1)  Since you stat the file anyhow, there's no reason to get message register
time...  just sleep past the time in the pre_stbuf structure.  That could save a
second if the file was closed near the end of the current second.

2)  Why do you do this:

+       if (message_register_time)
+       {
+         sleep_past (message_register_time);
+       }

?  time() should never return 0 (exccept at exactly the first midnight on 
January
1, 1970).  Time is defined to return -1 on error.

Derek
--
Derek Price                      CVS Solutions Architect ( http://CVSHome.org )
mailto:dprice@collab.net         CollabNet ( http://collab.net )
--
Of all the gin joints in all the towns in all the world, she walks into mine.

                - Humphrey Bogart as Rick, _Casablanca_