[RFC/PATCH 2/2] blame: fix output in case of replacement by using parse_commit_repl()

[RFC/PATCH 2/2] blame: fix output in case of replacement by using parse_commit_repl()

[Christian Couder]

git blame was not showing the sha1 of the replacement commit while
it was properly showing other information from the replacement commit,
because parse_commit() is buggy regarding replacement.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/blame.c    |    4 ++--
 t/t6050-replace.sh |    8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 8f9e7b0..c469e09 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1226,7 +1226,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 
 			if (sg_origin[i])
 				continue;
-			if (parse_commit(p))
+			if (parse_commit_repl(&p))
 				continue;
 			porigin = find(sb, p, origin);
 			if (!porigin)
@@ -1558,7 +1558,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
 		origin_incref(suspect);
 		commit = suspect->commit;
 		if (!commit->object.parsed)
-			parse_commit(commit);
+			parse_commit_repl(&commit);
 		if (reverse ||
 		    (!(commit->object.flags & UNINTERESTING) &&
 		     !(revs->max_age != -1 && commit->date < revs->max_age)))
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 4185b7c..881a2e8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -225,6 +225,14 @@ test_expect_success 'index-pack and replacements' '
 	git index-pack test-*.pack
 '
 
+test_expect_success 'blame and replacements' '
+	git blame hello > blame.txt &&
+	grep "$(git rev-parse --short $S)" blame.txt > lines.txt &&
+	grep "O Thor" lines.txt &&
+	grep "line 9" lines.txt &&
+	grep "line 10" lines.txt
+'
+
 #
 #
 test_done
-- 
1.7.2.1.351.g275bf

Re: [RFC/PATCH 2/2] blame: fix output in case of replacement by using parse_commit_repl()

[Nguyen Thai Ngoc Duy]

On Tue, Aug 17, 2010 at 11:59 AM, Christian Couder
<chriscool@tuxfamily.org> wrote:
> git blame was not showing the sha1 of the replacement commit while
> it was properly showing other information from the replacement commit,
> because parse_commit() is buggy regarding replacement.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  builtin/blame.c    |    4 ++--
>  t/t6050-replace.sh |    8 ++++++++
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 8f9e7b0..c469e09 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1226,7 +1226,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
>
>                        if (sg_origin[i])
>                                continue;
> -                       if (parse_commit(p))
> +                       if (parse_commit_repl(&p))

This is the hard way. There are another ~40 parse_commit() calls
elsewhere. Most of them are in git commands, so we can check one by
one. There are some calls in revision.c, sha1_name.c, tree.c,
upload-pack.c and walker.c, which may be used in many places. Checking
them would be harder.
-- 
Duy

Re: [RFC/PATCH 2/2] blame: fix output in case of replacement by using parse_commit_repl()

[Christian Couder]

On Tuesday 17 August 2010 05:44:11 Nguyen Thai Ngoc Duy wrote:
> On Tue, Aug 17, 2010 at 11:59 AM, Christian Couder
> 
> <chriscool@tuxfamily.org> wrote:
> > git blame was not showing the sha1 of the replacement commit while
> > it was properly showing other information from the replacement commit,
> > because parse_commit() is buggy regarding replacement.
> > 
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> > ---
> >  builtin/blame.c    |    4 ++--
> >  t/t6050-replace.sh |    8 ++++++++
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/builtin/blame.c b/builtin/blame.c
> > index 8f9e7b0..c469e09 100644
> > --- a/builtin/blame.c
> > +++ b/builtin/blame.c
> > @@ -1226,7 +1226,7 @@ static void pass_blame(struct scoreboard *sb,
> > struct origin *origin, int opt)
> > 
> >                        if (sg_origin[i])
> >                                continue;
> > -                       if (parse_commit(p))
> > +                       if (parse_commit_repl(&p))
> 
> This is the hard way. There are another ~40 parse_commit() calls
> elsewhere. Most of them are in git commands, so we can check one by
> one. There are some calls in revision.c, sha1_name.c, tree.c,
> upload-pack.c and walker.c, which may be used in many places. Checking
> them would be harder.

Yeah, I will try to provide test cases but it may not be possible in some 
cases.

Best regards,
Christian.