Re: [PATCH] Bugfix: GIT_EXTERNAL_DIFF with more than one changed files

From: Jeff King <peff@peff.net>
Date: Feb 13, 2009, 18:07 UTC
Message-ID: <20090213180740.GA31860@coredump.intra.peff.net>
In-Reply-To: <7vskmjl729.fsf@gitster.siamese.dyndns.org>

On Thu, Feb 12, 2009 at 12:43:42PM -0800, Junio C Hamano wrote:

Show 12 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > So this bug should trigger only in the face of reusing worktree files. I
> > checked your test; it constructs a diff between the worktree and the
> > index, so it correctly finds the problem.
> >
> > Acked-by: Jeff King <peff@peff.net>
> 
> Thanks, both.
> 
> Jeff, according to your analysis, this shouldn't trigger when
> core.autocrlf is set, should it?

Depending on the diff you are doing. If one of the sides is the worktree, then we should always be using the worktree file. But for "diff --cached" it depends on the file contents matching the index (in theory, it would work for arbitrary tree diffs when one side matches the worktree, but see the comment in reuse_worktree_file -- if nobody has looked at the cache already, we don't load it just for this).

I tried to construct a simple test that shows this behavior, but I couldn't. I did:

  mkdir repo && cd repo && git init

  git config core.autocrlf true

  printf 'one\r\n' >file1
  printf 'one\r\n' >file2
  git add .
  git commit -m one

  printf 'two\r\n' >file1
  printf 'two\r\n' >file2
  git add -u

  PAGER=cat GIT_EXTERNAL_DIFF=echo git diff --cached

which should fail without the core.autocrlf setting, but work otherwise. But it doesn't, and the reason is that the content in the index actually has the CRLF:

  $ xxd < file1
  0000000: 7477 6f0d 0a                             two..
  $ git cat-file blob :file1 | xxd
  0000000: 7477 6f0d 0a                             two..

which has me confused. Am I using autocrlf wrong? I have been fortunate enough in the past never to work on filesystems that needed such a thing.

-Peff