[PATCH (BUGFIX)] Respect core.autocrlf in combined diff

[PATCH (BUGFIX)] Respect core.autocrlf in combined diff

[Alexander Gavrilov]

Fix git-diff to make it produce useful 3-way
diffs for merge conflicts in repositories with
autocrlf enabled. Otherwise it always reports
that the whole file was changed.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---

	I noticed that combined diffs, which are shown by git-diff for files
	with conflicts, are unusable on repositories with autocrlf=true. 

	Steps to reproduce:

		git init
		echo foo > file
		git add file
		git commit -m init
		git checkout -b a
		git checkout -b b
		echo bbb >> file
		git add file
		git commit -m bbb
		git checkout a
		echo aaa >> file
		git add file
		git commit -m aaa
		git config core.autocrlf true
		git merge b

	Then look at the output of git diff.

	-- Alexander

 combine-diff.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 9f80a1c..da1ca99 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -727,6 +727,18 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 				die("early EOF '%s'", elem->path);
 
 			result[len] = 0;
+
+			/* If not a fake symlink, apply filters, e.g. autocrlf */
+			if (is_file) {
+				struct strbuf buf;
+				
+				strbuf_init(&buf, 0);
+				if (convert_to_git(elem->path, result, len, &buf, safe_crlf)) {
+					free(result);
+					result = strbuf_detach(&buf, &len);
+					result_size = len;
+				}
+			}
 		}
 		else {
 		deleted_file:
-- 
1.6.0.rc2

Re: [PATCH (BUGFIX)] Respect core.autocrlf in combined diff

[Junio C Hamano]

Alexander Gavrilov <angavrilov@gmail.com> writes:

> 	Steps to reproduce:
>
> 		git init
> 		echo foo > file
> 		git add file
> 		git commit -m init
> 		git checkout -b a
> 		git checkout -b b
> 		echo bbb >> file
> 		git add file
> 		git commit -m bbb
> 		git checkout a
> 		echo aaa >> file
> 		git add file
> 		git commit -m aaa
> 		git config core.autocrlf true
> 		git merge b

Please add this kind of sequence to test scripts.

The patch from a cursory look appears correct.  Thanks.

Re: [PATCH (BUGFIX)] Respect core.autocrlf in combined diff

[Junio C Hamano]

Alexander Gavrilov <angavrilov@gmail.com> writes:

> 		...
> 		git commit -m aaa
> 		git config core.autocrlf true
> 		git merge b
>
> 	Then look at the output of git diff.

Come to think of it, this test sequence is totally bogus, isn't it?

After making the "aaa" commit, you change core.autocrlf setting, which
means that at that point your work tree is invalid and needs to be checked
out with the right crlf.

IOW, shouldn't you need to do something like this:

 		git commit -m aaa
 		git config core.autocrlf true
+		rm -f .git/index && git reset --hard
		git merge b

for the test sequence to be a valid one that (may or may not) exposes the
problem you are trying to solve?

Re: [PATCH (BUGFIX)] Respect core.autocrlf in combined diff

[Alexander Gavrilov]

On Sunday 24 August 2008 05:24:31 Junio C Hamano wrote:
> Alexander Gavrilov <angavrilov@gmail.com> writes:
> 
> > 		...
> > 		git commit -m aaa
> > 		git config core.autocrlf true
> > 		git merge b
> >
> > 	Then look at the output of git diff.
> 
> Come to think of it, this test sequence is totally bogus, isn't it?
> 
> After making the "aaa" commit, you change core.autocrlf setting, which
> means that at that point your work tree is invalid and needs to be checked
> out with the right crlf.
> 

Yes, it is more correct to reset after changing that parameter, but since
merge overwrites the file anyway, I took a shortcut. I first noticed the
problem on Windows, where autocrlf was turned on from the beginning.


By the way, am I right that currently the only way to get properly CRLFed
versions of all stages is to do something like this:

mv -f $path "$path.BACKUP"
git checkout-index -f --stage=1 $path
mv -f $path "$path.BASE"
git checkout-index -f --stage=2 $path
mv -f $path "$path.LOCAL"
git checkout-index -f --stage=3 $path
mv -f $path "$path.REMOTE"
cp -f "$path.BACKUP" $path


git-mergetool does it like this, but it produces LF files:

    base_present   && git cat-file blob ":1:$prefix$MERGED" >"$BASE" 2>/dev/null
    local_present  && git cat-file blob ":2:$prefix$MERGED" >"$LOCAL" 2>/dev/null
    remote_present && git cat-file blob ":3:$prefix$MERGED" >"$REMOTE" 2>/dev/null


Alexander