[PATCH 0/2] git-blame bug

DORMANTno replies

3 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH 0/2] git-blame bug

From: Ben Willard <hidden>
Date: 2016-06-15 22:46:53

Hi,

I ran across this git-blame failure the other day: if a directory is deleted
and a file is created with the same name in a single commit, then git blame
will fail on it with this error:
	fatal: internal error in blame::find_origin
	
Anyway, patch 1/2 shows this failure with a test, and 2/2 is a possible fix.
They should apply on maint. 

Thanks,
--
Ben

Ben Willard (2):
  t8006: Add test to show blame failure
  blame: Fix corner case when a directory becomes a file

 builtin-blame.c  |   12 ++++++++----
 t/t8006-blame.sh |   21 +++++++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100755 t/t8006-blame.sh

[PATCH 1/2] t8006: Add test to show blame failure

From: Ben Willard <hidden>
Date: 2016-06-15 22:46:53

'git blame' fails on a file which was changed from a directory to a file
in a single commit.  This test shows the failure.

Signed-off-by: Ben Willard <redacted>
---
 t/t8006-blame.sh |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
 create mode 100755 t/t8006-blame.sh

diff --git a/t/t8006-blame.sh b/t/t8006-blame.sh
new file mode 100755
index 0000000..7c271f0
--- /dev/null
+++ b/t/t8006-blame.sh

@@ -0,0 +1,21 @@
+#!/bin/sh
+
+test_description='git blame on file which changed from a directory to a file in the same commit'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	mkdir dir &&
+	touch dir/file &&
+	git add dir/file &&
+	git commit -m "Add dir/file" &&
+	git rm dir/file &&
+	echo "File contents" > dir &&
+	git add dir &&
+	git commit -m "Add dir as a file"
+'
+
+test_expect_failure 'blame runs on dir' '
+	git blame dir
+'
+
+test_done

-- 
1.6.3.1.279.gd4bf4

[PATCH 2/2] blame: Fix corner case when a directory becomes a file

From: Ben Willard <hidden>
Date: 2016-06-15 22:46:53

find_origin() assumes that there will be only one listing in
diff_queued_diff, but this is not the case when a directory becomes a
file in a single commit.  So, don't fail in this case.

Signed-off-by: Ben Willard <redacted>
---
 builtin-blame.c  |   12 ++++++++----
 t/t8006-blame.sh |    2 +-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index cf74a92..3e217af 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c

@@ -363,16 +363,14 @@ static struct origin *find_origin(struct scoreboard *sb,
 	diffcore_std(&diff_opts);
 
 	/* It is either one entry that says "modified", or "created",
-	 * or nothing.
+	 * or nothing, or two entries (a "deleted" and an "add").
 	 */
 	if (!diff_queued_diff.nr) {
 		/* The path is the same as parent */
 		porigin = get_origin(sb, parent, origin->path);
 		hashcpy(porigin->blob_sha1, origin->blob_sha1);
 	}
-	else if (diff_queued_diff.nr != 1)
-		die("internal error in blame::find_origin");
-	else {
+	else if (diff_queued_diff.nr == 1) {
 		struct diff_filepair *p = diff_queued_diff.queue[0];
 		switch (p->status) {
 		default:

@@ -388,6 +386,12 @@ static struct origin *find_origin(struct scoreboard *sb,
 			break;
 		}
 	}
+	else if (diff_queued_diff.nr == 2) {
+		/* Both added and removed, (ie. directory became a file) */
+	}
+	else {
+		die("internal error in blame::find_origin");
+	}
 	diff_flush(&diff_opts);
 	diff_tree_release_paths(&diff_opts);
 	if (porigin) {

diff --git a/t/t8006-blame.sh b/t/t8006-blame.sh
index 7c271f0..17f38ff 100755
--- a/t/t8006-blame.sh
+++ b/t/t8006-blame.sh

@@ -14,7 +14,7 @@ test_expect_success 'setup' '
 	git commit -m "Add dir as a file"
 '
 
-test_expect_failure 'blame runs on dir' '
+test_expect_success 'blame runs on dir' '
 	git blame dir
 '

-- 
1.6.3.1.279.gd4bf4

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help