VC and CVS/Entries under NT

VC and CVS/Entries under NT

[Andre Spiegel]

Dave Abrahams recently reported a problem where VC incorrectly assumed
that files were edited, when they were in fact unmodified.  The reason
was that Emacs did not correctly compare the file's modification time
with the time stamp in CVS/Entries.  After a recent patch by Stefan
Monnier, vc-cvs.el tried to do that by converting the modification time
to a string and comparing it textually to the time stamp:

    (let* ((mtime (nth 5 (file-attributes file)))
           (system-time-locale "C")
           (mtstr (format-time-string "%c" mtime 'utc)))
      ;; Solaris sometimes uses "Wed Sep 05" instead of  "Wed Sep  5".
      ;; See "grep '[^a-z_]ctime' cvs/src/*.c" for reference.
      (if (= (aref mtstr 8) ?0)
          (setq mtstr (concat (substring mtstr 0 8) " " 
                              (substring mtstr 9))))
          
The problem is that under NT, the resulting string is still not in "C"
locale format, but looks like "8/15/02 18:37:55", when it should be 
"Thu Aug 15 18:37:55 2002".

This may be a bug of Emacs and/or NT, and should be investigated.  In
any case, I find it simply wrong to compare the time stamps textually,
rather than to parse the time stamp from CVS/Entries and compare it
numerically to the file's modification time.

Stefan's argument for his code is that CVS does it the same way, but the
problem reported by Dave shows how easily this can break, for whatever
reason (the Solaris problem mentioned in the comment is another example
for such an issue).  We simply cannot guarantee that Emacs' way of
producing the time string yields exactly the same result as whatever CVS
did to make it.  To isolate ourselves from such issues, I have installed
my previous version of the code which does a numerical comparison:

    (let ((mtime (nth 5 (file-attributes file))))
      (require 'parse-time)
      (let ((parsed-time
             (parse-time-string (concat (match-string 2) " +0000"))))
        (cond ((and (not (string-match "\\+" (match-string 2)))
                    (car parsed-time)
                    (equal mtime (apply 'encode-time parsed-time)))

This code fixes Dave's problem, and the Solaris issue mentioned in
Stefan's comment.  If anybody sees any problems with it, or has
suggestions how to improve it, please let me know.

Re: VC and CVS/Entries under NT

[Kevin Rodgers]

Andre Spiegel wrote:

>     (let ((mtime (nth 5 (file-attributes file))))
>       (require 'parse-time)
>       (let ((parsed-time
>              (parse-time-string (concat (match-string 2) " +0000"))))
>         (cond ((and (not (string-match "\\+" (match-string 2)))
>                     (car parsed-time)
>                     (equal mtime (apply 'encode-time parsed-time)))
> 
> This code fixes Dave's problem, and the Solaris issue mentioned in
> Stefan's comment.  If anybody sees any problems with it, or has
> suggestions how to improve it, please let me know.

(require 'parse-time) could be moved to the top level at the beginning
of vc-cvs.el; better yet, it could be elimintated if parse-time.el had
an autoload cookie for the parse-time-string function (which is the only
entry point for that library).

Is there a good reason for parse-time.el to be in the gnus directory?


-- 
Kevin Rodgers

Re: VC and CVS/Entries under NT

[Richard Stallman]

    Is there a good reason for parse-time.el to be in the gnus directory?

It is in the calendar directory now.

Re: VC and CVS/Entries under NT

[Andre Spiegel]

On Wed, 2003-09-24 at 18:00, Kevin Rodgers wrote:

> (require 'parse-time) could be moved to the top level at the beginning
> of vc-cvs.el; better yet, it could be elimintated if parse-time.el had
> an autoload cookie for the parse-time-string function (which is the only
> entry point for that library).

I've added an autoload cookie and removed the require.  Thanks.