bugConcurrent Versions System - Bugs: bug #22781, cvs update (1.11.22 on Windows)...

 
 

bug #22781: cvs update (1.11.22 on Windows) uploads excessive numbers of files after daylight saving time changes

Submitted by:  Nelson B Bolyard <nelsonb>
Submitted on:  Sat 29 Mar 2008 10:35:17 PM UTC  
 
Category: Bug Fix (patch attached)Severity: 3 - Normal
Item Group: NoneStatus: Ready For Test
Privacy: PublicAssigned to: None
Open/Closed: OpenRelease: 
Fixed Release: 1.11.23Fixed Feature Release: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 22 Dec 2008 01:10:47 AM UTC, comment #6:

I believe this was finally fixed in cvs 1.11.23.

Sadly, the same patch does not really work for the FEATURE
branch.

Aid from someone who has a windows build environment to get
this working for cvs 1.12.14 is desirable.

Mark D. Baushke <mdb>
Project Administrator
Sat 05 Apr 2008 12:46:35 AM UTC, comment #5:

The attached zip file contains a patch, a cvs diff for cvs1-11-x-branch.

The patch has Windows line endings, and will need to be converted to Unix line endings if it is going to be applied on a Unix box.

This version of the patch:
- adds summary descriptions to the various ChangeLog files
- removes the files Windows/JmgStat.[ch], rather than modifying them, because those functions are no longer necessary, no longer used, and were producing
incorrect results on FAT file systems.
- Adjusts all the Makefiles and windows "project" files, but the changes to
the project files are untested because I don't use them. The nmake makefiles
(*.mak) work with MSVC 8 and the free version, MSVC 2005 Express.

I look forward to working with those who will evaluate this patch for inclusion in cvs 1.11.x.

/Nelson

(file #15405)

Nelson B Bolyard <nelsonb>
Sun 30 Mar 2008 10:49:01 PM UTC, comment #4:

Thanks, Mark, for fixing those source files.
Before I submit another patch, I have a few questions:
1) Would you prefer that I change my patch to use Unix line
endings before submitting it?
2) should my patch include changes to the ChangeLog files?
3) My patch changes all the *.mak files (used by nmake),
but does not change any of the other make-related files
such as .dsp, .dsw, *.dep or the Makefile.{am.in} files
in Windows-NT. Is that a problem? Are those files actually
used in your Windows builds?

Nelson B Bolyard <nelsonb>
Sun 30 Mar 2008 01:56:24 AM UTC, comment #3:

Sorry, file #15362 is incomplete. Please disregard it.

It appears that someone checked in the Windows-NT files, which
have Windows line endings, from a Unix system, without setting
the -kb sticky flag. So, the line endings are wrong when checked
out using Windows cvs 1-11-x, because Windows CVS tries to convert unix line endings to windows line endings during checkout.
The right thing would have been to either
a) convert the files to have Unix line endings before committing
from Unix, or
b) set the binary sticky flag.

I'd suggest that someone fix the line endings on the affected
files, and then recommit them. Then I will be able to produce
a good patch with cvs diff. (I'd do it if I could.)

Nelson B Bolyard <nelsonb>
Sun 30 Mar 2008 01:34:28 AM UTC, comment #2:

Thanks to a generous hint from Larry Jones, I checked out the ccvs module on the cvs1-11-x-branch, applied my patch to it, and generated the attached patch file as a unified CVS diff.
Thanks Larry.

Caveat: the attached patch file has Windows line endings.

(file #15362)

Nelson B Bolyard <nelsonb>
Sat 29 Mar 2008 10:43:01 PM UTC, comment #1:

I would have submitted this patch as the output of a CVS diff against the repository, if I could have done so. If there's a way to get anonymous read-only access to the cvs server for the CvS source code, I couldn't find it.

Nelson B Bolyard <nelsonb>
Sat 29 Mar 2008 10:35:17 PM UTC, original submission:

Last November, and again a few weeks ago, right after Daylight Savings Time changed in the USA, I found that a simple cvs update (using cvs 1.11.22 on Windows XP) took a very long time. I found it was uploading over half the files in the sandbox to the server, even though they were nearly all unchanged. So I dived into the source code, and found the problems, and have now fixed them to my own satisfaction.

Attached is a patch for rev 1.11.22 that solves the problem (hopefully) once and for all. This patch also includes the fix to bug 22710.

Someone had previously done work to try to correct the Unix-like time_t timestamps obtained from stat and lstat, and that may have helped at one time, but that change was insufficient, IMO. It changed SOME stat and lstat calls to use code that attempted to correct the time_t's. But it didn't get ALL the calls to stat and lstat, and didn't get ANY calls to fstat.

The BIG problem, untouched by that code, was that it didn't fix the DST bug in Windows' utime function. When you call utime to set the modification time on a file to some date in the past that is NOT in the same DST "season" as the present time, utime will add or subtract an hour from the time you specify, causing the file's modification time to NOT be the same as on the server. And (as already known) the *stat functions also compensate for timestamps that are in a different DST "season" by adding or subtracting an hour in the opposite direction of the change made by utime. Consequently, if you set a file's modification time with utime and immediately read it back with (unmodified) stat (as CVS update does), it will appear that the time has been set correctly, but it may actually have been set to a time that is an hour off.

I fixed the problem as follows:
a) wrote new stat, lstat, and fstat function implementations that get the UTC times via calls to Windows GetFileTime function and correctly convert them to Unix time_t's.
b) write a new utime implementation that correctly converts the Unit time_t to a UTC FILETIME, and sets the time stamp using Windows' SetFileTime call.
c) changed ALL stat, lstat, fstat and utime calls to use those new functions on Windows.

I tested this on both NTFS and FAT file systems, and in my tests, it got the right results in all test cases on both types of file systems.

If you have a sandbox that has files with modification times in both seasons, then sadly, a massive update is unavoidable to correct the timestamps on the files whose timestamps are an hour off. But with this patch, the new timesstamps are correct UTC timestamps, and future DST season changes should NOT cause more massive uploading.

As part of implementing this fix, I had to port the code to build with MSVC version V8 (a.k.a Express 2005). This means time_t's are 64 bits, and many of the standard header files that were absent in older versions are now present. I had to fix some function declarations that conflicted with Windows' existing declarations. Also, some formerly used variables names are now reserved words, so I had to rename some variables, e.g. errCode -> errorCode.

I also had to change the Makefiles to deal with several differences, including
a) several of the old compiler command line switches are no longer supported
b) Extra build step to combine the manifest files (new in VC8) into the .exe,
c) Fixed the Makefiles to output the full commands it executes, just like on Unix. :)

The result will not run on Windows 9x or ME, but will run on Win2K and later
OSes.

I've been using CVS for over 10 years now, and am happy to be able to make a small contribution to it in return for all the good service it has given me. This is my first real submission of a patch of any magnitude for CVS, so the patch may not meet the CVS team's coding style standards or other standards of which I am not aware. I invite any/all feedback.

/Nelson

Nelson B Bolyard <nelsonb>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #15405:  ccvs_patch_2.zip added by nelsonb (19KiB - application/x-zip-compressed - Next generation of this patch)
file #15362:  FINAL_CVS_DIFF.zip added by nelsonb (15KiB - application/x-zip-compressed - cvs diff against cvs1-11-x-branch)
file #15361:  FINAL_PATCH.zip added by nelsonb (16KiB - application/x-zip-compressed - zip file containing patch. If a plain text patch file is preferred, please advise.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by mdb (Posted a comment)
  • -unavailable- added by nelsonb (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 22 Dec 2008 01:10:47 AM UTCmdbStatusNone=>Ready For Test
      Fixed ReleaseNone=>1.11.23
    Sat 05 Apr 2008 12:46:35 AM UTCnelsonbAttached File-=>Added ccvs_patch_2.zip, #15405
    Sun 30 Mar 2008 01:34:28 AM UTCnelsonbAttached File-=>Added FINAL_CVS_DIFF.zip, #15362
    Sat 29 Mar 2008 10:35:17 PM UTCnelsonbAttached File-=>Added FINAL_PATCH.zip, #15361

    Back to the top


    Powered by Savane 3.1-cleanup1