Re: [PATCH] Re-re-re-fix common tail optimization

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Mon, 17 Dec 2007, Wincent Colaiuta wrote:

> El 17/12/2007, a las 11:39, Johannes Schindelin escribi?:
> 
> > On Mon, 17 Dec 2007, Wincent Colaiuta wrote:
> > 
> > > El 16/12/2007, a las 23:29, Jeff King escribi?:
> > > 
> > > > On Sun, Dec 16, 2007 at 02:23:27PM -0800, Junio C Hamano wrote:
> > > > 
> > > > > > Aren't we using "git diff" for the second diff there nowadays?
> > > > > 
> > > > > Some people seem to think that is a good idea, but I generally 
> > > > > do not like using "git diff" between expect and actual (both 
> > > > > untracked) inside tests.  The last "diff" is about validating 
> > > > > what git does and using "git diff" there would make the test 
> > > > > meaningless when "git diff" itself is broken.
> > > > 
> > > > I think that is a valid concern. But ISTR that were some issues 
> > > > with using GNU diff. Commit 5bd74506 mentions getting rid of the 
> > > > dependency in all existing tests, but gives no reason.
> > > 
> > > I'd say it's safe and sensible to use "git diff" in all tests 
> > > *except* for tests of "git diff" itself.
> > 
> > To the contrary.  It has to test "git diff", so it must use "git 
> > diff".
> 
> Obviously, you can only test "git diff" by actually running it.

Sorry, I should have made clear that I meant this as funny:

	;-)

> > As for the reference output: we include the expected diffs as texts, 
> > and therefore do not really have to rely on having GNU diff installed.
> > 
> > Besides, we cannot even test the goodies like "rename from" by 
> > comparing to GNU diff's output.
> 
> Sorry, I didn't make myself clear. That's not what I was proposing at 
> all. I was talking about this kind of example:
> 
> > + git diff -U0 | sed -e "/^index/d" -e "s/$z2047/Z/g" >actual &&
> > + diff -u expect actual
> 
> First line uses "git diff", if the second line uses "git diff" as well 
> and "git diff" happens to be broken then you're using a broken tool to 
> test a broken tool, as Junio already pointed out.

Hmm.  There is some chicken-and-egg problem here (I read the thread, but 
did not really see a problem, as I assumed that _other_ tests would assure 
that "git diff --no-index" works as expected).

But as at least one released version of GNU diff has a pretty serious bug, 
I would rather not rely too much on diff.  (BTW this was the reason I 
wanted --no-index so badly.)

So yeah, the second "diff" cannot be "git diff".  Maybe "cmp", but not 
"git diff".

Ciao,
Dscho