Re: Large file actual performance report; cvs use of ,v header is sometimes non-optimal.

[Todd Denniston]

Bulgrien, Kevin wrote, On 01/18/2008 10:12 AM:
> > Also considering the age of his copy of CVS
>
> It and older versions have served us well for many years.  It is
> in use here because commercial packages in past years trashed
> repositories.  CVS has sustained no losses that were not
> human induced.  This was not a criticism of CVS in the
> least.

Do you mean a commercial package variant of CVS???  if so which one, so it can 
be avoided.
BTW I have ran the same repositories (same ,v files) starting with cvs 1.9.x 
-> 1.10.x -> 1.11.x, just run `cvs init` each time I changed cvs and never had 
a problem. However I ALWAYS make a verified backup before switching, out of 
pure paranoia. :)

Actually, I fibbed, I had a problem with 1.11.1p1 -> 1.11.2
it seems that Solaris 2.6 had a memmap problem:
http://lists.gnu.org/archive/html/bug-cvs/2003-11/msg00152.html
Fortunately it did not damage the repository or the sandbox, it just failed to 
complete some actions.


<SNIP>
> > Then there is the troubling bit about
> > {so the next commit was destined to fail since the working directory
> > CVS/Entries file is now erroneous and marks the working 
> > directory file as
> > a new deleted revision 1.25.  See the prior post "FYI: cvs can break a
> > checked out working directory" for details.  }
> > Along with:
> > {The commit of the new 1.23 should have been quite fast (on 
> > the order of
> > minutes) because it is took place at the top of HEAD, but 
> > instead it takes
> > perhaps 8-12 hours, and, in fact, fails with an error saying 
> > 1.25 can not
> > be found.  This is the situation where the title "cvs use of 
> > ,v header is
> > sometimes non-optimal" comes to play.}
>  
> > 1) I suspect this could be solved with either an sandbox 
> > update or new 
> > checkout.  The manual should probably indicate something 
> > along the lines of 
> > "If you are dangerous enough to mess with the dragons in the 
> > `cvs admin` 
> > command, then (before and) afterwards you need to tell all 
> > users 'Hey! You out 
> > of the pool! do a new checkout before attempting any new work.'"....
> > OH wait!! Quoth the book of cederqvist
> > file:///tmp/cederqvist-1.11.21.html/cvs_16.html#IDX236
> > last paragraph:
> > "Make sure that no-one has checked out a copy of the revision 
> > you outdate. 
> > Strange things will happen if he starts to edit it and tries 
> > to check it back 
> > in. For this reason, this option is not a good way to take 
> > back a bogus commit".
>
> Well, sure.  The sandbox was borked, but this wasn't realized
> until after the operation.  And of course, per the reference
> article, update after admin -o would have prevented the problem.
> We are all human, and we all make occasional oversights.  All of
> this was fully disclosed in the OP and in referenced posts.
> Amazing how a poster can admit to something and people still
> have to hammer on it anyway, like that helps anyone want to post
> to the list, but at least this comment has facts and manual
> references, so comes across not so personal.
>  

I started writing this from the perspective of 'hey CVS devs, we need to 
document it' and then I looked at the manual myself and noted they beat me to 
it. Sometimes my humor is a bit off :}

> > 2) I tend to agree that CVS should have at least checked the 
> > repository for 
> > 'is 1.25 still the head/does it still exist?' on commit 
> > before working on 
> > anything else, because it always checks to see if you need to 
> > do an update 
> > before commit.  Is this already the case in newer CVS's?
> > And before someone says 'but he messed up the repo', which I 
> > agree... it 
> > should be trivial while checking 'if [current repo HEAD rev > 
> > current Entries 
> > rev]' also check 'if [current repo HEAD rev < current Entries 
> > rev]; {throw 
> > error/check to see if Entries rev still exists}'... at least 
> > for someone who 
> > knows where the first test is taking place now in the code.
> > [Arthur, if you look this up in CVSNT, please drop a note to 
> > what file it is 
> > in for CVSNT, and the 10 lines around it... If I have to make 
> > a patch that 
> > would maybe help me find it in CVS.]
>
> I did not even go so far as to say CVS should have done anything
> as for all I know, the traversal of the diffs may have been
> intentional even though I could not discern that.

You did not say it, I however AM saying it.
My opinion short: cvs can and should detect this condition, and warn|error 
about it.

My opinion long:
As I recall:
A) CVS always compares CVS/Entries with the ,v file's head before doing a 
commit, to make sure that you are already up to date with the repo.
  Reason: so in r1.25 Jim doesn't loose the change Sue just made in r1.24.
  This means either the cvs client (at the sandbox) or the cvs server (at the 
repository) knows:
  current repo HEAD rev
and
  current Entries rev

B) other than `cvs admin` there are NO _cvs_ commands that should make the 
HEAD be less than the version in CVS/Entries.

Therefore, either the cvs client or cvs server has enough information to 
detect that
current repo HEAD rev < current Entries rev
And because this condition should only exist in two situations with both 
implying a BIG problem with the sandbox, cvs client|server SHOULD be able to 
warn|error when it detects the condition.

--
Todd Denniston
Crane Division, Naval Surface Warfare Center (NSWC Crane)
Harnessing the Power of Technology for the Warfighter