threads / patch / 15180

patchRespect core.autocrlf in combined diff

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

## tl;dr

4 messages between Aug 23, 2008 and Aug 24, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Alexander Gavrilov· Aug 23, 2008, 19:21 UTC · lore

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(-)
Show changes to combine-diff.c +12 −0
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
Junio C Hamano· Aug 24, 2008, 00:56 UTC · re: Alexander Gavrilov · lore

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

Alexander Gavrilov <angavrilov@gmail.com> writes:
Show 17 quoted lines
> 	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.
Junio C Hamano· Aug 24, 2008, 01:24 UTC · re: Alexander Gavrilov · lore

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

Alexander Gavrilov <angavrilov@gmail.com> writes:
Show 6 quoted lines
> 		...
> 		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?

Alexander Gavrilov· Aug 24, 2008, 09:21 UTC · re: Junio C Hamano · lore

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

On Sunday 24 August 2008 05:24:31 Junio C Hamano wrote:
Show 15 quoted lines
> 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

← back to recent threads