Re: Warning: cvsexportcommit considered dangerous

On Mon, 2007-11-05 at 00:05 +0100, Robin Rosenberg wrote:

söndag 04 november 2007 skrev Johannes Schindelin:

quoted

Hi,
On Sun, 4 Nov 2007, Steffen Prohaska wrote:

quoted

On Nov 4, 2007, at 5:41 PM, Johannes Schindelin wrote:

quoted

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.

quoted

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.

quoted

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.