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

Hi,

On Mon, 17 Dec 2007, Wincent Colaiuta wrote:

El 17/12/2007, a las 11:39, Johannes Schindelin escribi?:

quoted

On Mon, 17 Dec 2007, Wincent Colaiuta wrote:

quoted

El 16/12/2007, a las 23:29, Jeff King escribi?:

quoted

On Sun, Dec 16, 2007 at 02:23:27PM -0800, Junio C Hamano wrote:

quoted

quoted

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:

	;-)

quoted

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:

quoted

+ 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