"make test" fails with current HEAD

t4103.sh:

fatal: patch with only garbage at line 30
* FAIL 7: check binary diff with replacement.
        git-checkout master
                 git-apply --check --allow-binary-replacement BF.diff

May I suggest to please run "make test" before pushing? Thanks! ;-)

Hi,

On Thu, 17 Nov 2005, Matthias Urlichs wrote:

Does not fail here.

Did you set your GIT_EXEC_PATH?

Hth, Dscho

Hi,

Johannes Schindelin:

Does fail here...

Yes. :-/

I'll dig.

Fails at home (Suse 9.3) but passes a work (Ubuntu Hoary).

*** t4103-apply-binary.sh ***
*   ok 1: stat binary diff -- should not fail.
*   ok 2: stat binary diff (copy) -- should not fail.
*   ok 3: check binary diff -- should fail.
*   ok 4: check binary diff (copy) -- should fail.
*   ok 5: check incomplete binary diff with replacement -- should fail.
*   ok 6: check incomplete binary diff with replacement (copy) -- should 
fail.
* FAIL 7: check binary diff with replacement.
         git-checkout master
                  git-apply --check --allow-binary-replacement BF.diff
* FAIL 8: check binary diff with replacement (copy).
         git-checkout master
                  git-apply --check --allow-binary-replacement CF.diff
*   ok 9: apply binary diff -- should fail.
*   ok 10: apply binary diff -- should fail.
*   ok 11: apply binary diff (copy) -- should fail.
*   ok 12: apply binary diff (copy) -- should fail.
*   ok 13: apply binary diff without replacement -- should fail.
*   ok 14: apply binary diff without replacement (copy) -- should fail.
* FAIL 15: apply binary diff.
         do_reset
                  git-apply --allow-binary-replacement --index BF.diff &&
                  test -z "$(git-diff --name-status binary)"
* FAIL 16: apply binary diff (copy).
         do_reset
                  git-apply --allow-binary-replacement --index CF.diff &&
                  test -z "$(git-diff --name-status binary)"
* failed 4 among 16 test(s)
make[1]: *** [t4103-apply-binary.sh] Error 1

Johannes Schindelin wrote:

Hi,

On Thu, 17 Nov 2005, A Large Angry SCM wrote:

Any chance you investigate this in detail? Like inserting "test_done; exit" right before that test, only running t4103-*, and then executing the command yourself?

Hth, Dscho

Hi,

Johannes Schindelin:

Thank you, I know how to debug ;-)

Already in progress.

Matthias Urlichs <smurf@smurf.noris.de> writes:

Well, I do, but the thing is, I do not have an access to your particular machine ;-).

I *think* this is the same problem as I fixed tonight with help from LASCM and John Benes, and I'm hoping to push the fix out before going to bed tonight. If you are impatient the patch has already been sent out.

Hi,

Junio C Hamano:

*Sigh*.

Sorry about that. It's not you, it's diff 2.8.7 -- which conveniently omits the word "Binary".

Of course, with LANG=de_DE.UTF-8 the situation is worse ...

... so git-diff-* might want to set LANG=C.

Matthias Urlichs <smurf@smurf.noris.de> writes:

And at this point it becomes more and more tempting to have our own internal diff generator, without relying on external diff.

A midway compromise solution would be to detect if either file is binary ourselves and not to call diff but always say "Binary files difer".

Hi, Junio C Hamano wrote:

Actually, there's a better way:

$ diff -u /dev/null /tmp/ra Binary files /dev/null and /tmp/ra differ $ echo $? 2

So the trivial fix is to emit our own "Binary files FOO and BAR differ" line if the exit status is 2.

Hi,

On Fri, 18 Nov 2005, Matthias Urlichs wrote:

-- snip --
DIAGNOSTICS
       An  exit status of 0 means no differences were found, 1 means some 
       differences were found, and 2 means trouble.
-- snap --

The exit value 2 is ambiguous for at least one version of GNU diff.

Hth, Dscho

Hi,

Johannes Schindelin:

AFAIK, We have
0 - no difference
1 - produced a reasonable diff
1 - produced a bad diff because binary stuff starts
    after 4096-or-whatever bytes and diff didn't notice
2 - didn't produce a diff -- binary files differ
2 - something else broke

Right now we simply don't consider the second -2- case, because we just successfully wrote the two files in question; there's not *that* much that can fail after that. The second -1- case is sufficiently rare too.

So I still think a "Just write a 'Binary files FOO and BAR differ' line if the exit status is 2" rule would at least fix the current bug with diff 2.8.7.

Hi,

On Fri, 18 Nov 2005, Matthias Urlichs wrote:

Dunno. Am I the only one with an odd feeling about that?

Ciao, Dscho

Hi,

Johannes Schindelin:

I don't really like it either, but frankly I'm not going to write a new (or integrate an existing) diff implementation for 1.0 ... you want to do it, feel free. ;-)

On Fri, 18 Nov 2005, Junio C Hamano wrote:

Umm.. Why not just say "if the line after the 'diff' line doesn't start with '@@ ', then it's a binary diff"?

And not care at _all_ about what it says.

We require unified diffs, and they _always_ start with '@@ ' after the header. No need to check anything else.

		Linus

Linus Torvalds <torvalds@osdl.org> writes:

Thanks, that makes sense.

Junio C Hamano <junkio@cox.net> writes:

Maybe not; parse_single_patch would have eaten '@@ -' lines so in practice that amounts to not checking anything at all.

Which is not a bad idea, though... That means we would treat all empty patches as binary diffs, which is certainly simple, but is it safe enough?

Matthias Urlichs <smurf@smurf.noris.de> writes:

And at this point it becomes more and more tempting to have our own internal diff generator, without relying on external diff.

A midway compromise solution would be to detect if either file is binary ourselves and not to call diff but always say "Binary files differ".

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Junio C Hamano wrote:

I can verify that "make test" now passes on Cygwin without NO_MMAP
uncommented.
Commit: 2e67a5f449f4026097494569f871d79bf263ab28
Desc: Cygwin *might* be helped with NO_MMAP

- --
John Benes
GPG Fingerprint: D519 25DB BB5C 38FC 9D02  02E7 596D BC50 F880 27FA
"It is not only the living who are killed in war." - Isaac Asimov
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iQIVAwUBQ35aeWF0oWcU9kCNAQIpgg/9GFi++HvW5J2GYZbcHhAld2cqd6KtgbPN oUsbPuvGKSM9uP/9pKhOxp+DKJMuFd9mK5FG63jS8i8bthbEzegErLWcfbfM1+Js 0kYIfQEZpJu0+S1uMfsHX1CfS/OtjHebQCD7M9jgMFa1kgJXZl0Zu1rvG5hxTIZV OwssVwxz9Z2/gWzKFlAwyL4UgI4/mr2f7t43AVSKI0gR0Dry8Ou6B7vjBid9clMd JNoVHF437FVzAypoSUjmgHJJt+GHYn5noQTOMD0KnokznQckFVxNdALXaoAgkT1Q adgY+8TObmTbctJ9q2Qi1Z9Y0CAncC0TEGt8D8FBxiH+eJKqEKdmL7JT3ckaQO6S WZJf/u9SnkUJ089rYbYCvV10hqXz0y5XcGxo8kRKS6XGnY+Ff0hqtxVtHqbog8+y RtPyeST3vymkPuxx3g/gHrGtXJSZStE2/uNTKyA6IHA9dRUkLn70vHRKZsKKyj/f n5Ka1CJhIw+DzN4tANsQz9U/Q1olbQIr1khykds5DOhTvdSydSQtT3yOx9Wtt8QZ OH16pbWSGRNuL/vtwmonnVDsX/JJkUKSn3Z5gwmIvThvrD5TbJjyyYBaYZ9xvtkK XShm05Xf0yaryVlyUNCjlfNoLhi+gBoTVeaZmx/c1yrDZL6I53g3S79Bktdfd/rE r0eQqS7GEWc= =+a+2 -----END PGP SIGNATURE-----

Truncate the result from readdir() in the exec-path if they end with .exe, to make it a bit more readable on Cygwin.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 Today I borrowed a Cygwin environment to build things without
 any customization (no config.mak nor make command line
 override), and I have to report that make test passed OK for
 me.

 git.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

applies-to: ff891e258456492507934e82d90fd8aacb124845 f9039f30d56e3815eaab870d8b19c7b94aded8aa