Re: Warning: cvsexportcommit considered dangerous

[Alex Bennee <kernel-hacker@bennee.com>]

On Mon, 2007-11-05 at 00:05 +0100, Robin Rosenberg wrote:
> söndag 04 november 2007 skrev Johannes Schindelin:
> > Hi,
> > On Sun, 4 Nov 2007, Steffen Prohaska wrote:
> > > On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:
> > > > ever since the up-to-date check was changed to use just one call to 
> > > > "cvs status", a bug was present.  Now cvsexportcommit expects "cvs 
> > > > status" to return the results in the same order as the file names were 
> > > > passed.
> > > I do not know why it wasn't applied. I forgot re-checking after my 
> > > vacation.

I think at the time it was felt the speed hit was too great on large
trees. Although my view still is we should always go for correctness
over speed.

> > It slipped by me, because of holiday, too.  (I was on my well needed 
> > holiday then.)
> > 
> > But that patch really seems like a step back to me.  The line "File: ... 
> > Status: ..." should be parsable enough to fix the bug properly, instead of 
> > undoing the optimisation.
> Unfortunately it's not that easy to parse. It *can* be done by looking at the
> repository path, and the CVS/Root etc, but it's not nice. 

I also worry about corner cases in parsing code, especially if you have
to start poking around in CVS/Root files. Another (ugly) method could be
a two pass attempt, falling back to an individual status call if the
"optimized" version isn't sure.

> > AFAICS Robin replied with a "let's see if a proper fix materialises", and 
> > I kind of hope that it will materialise soon.

I've not had another go at fixing this mainly due to being too busy. The
current patch works for me.

> Still hoping :). BTW, wouldn't this err on the right side anyway, i.e. if an
> existing file was not up to date and was wrongly thought to not exist or a new 
> file was thought to be up-to-date, I would get an error and would not be able
> to commit. I've never seen it though and I always have a clean CVS checkout
> so the potential bug seems unlikely to me.

It's not just new files that can break (btw another fix has gone in to
ensure directories are added to CVS trees in order). 

-- 
Alex, homepage: http://www.bennee.com/~alex/
Nothing in life is to be feared. It is only to be understood.