threads / rfc / 24759

RFC patch, 2 partsblame: fix output in case of replacement by using parse_commit_repl()

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

## tl;dr

3 messages between Aug 17, 2010 and Aug 17, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Christian Couder· Aug 17, 2010, 01:59 UTC · lore

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(-)

Show changes to 2 files +10 −2

builtin/blame.c, t/t6050-replace.sh

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

Nguyen Thai Ngoc Duy· Aug 17, 2010, 03:44 UTC · re: Christian Couder · lore

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

On Tue, Aug 17, 2010 at 11:59 AM, Christian Couder <chriscool@tuxfamily.org> wrote:

Show 20 quoted lines

> 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

Christian Couder· Aug 17, 2010, 06:16 UTC · re: Nguyen Thai Ngoc Duy · lore

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

On Tuesday 17 August 2010 05:44:11 Nguyen Thai Ngoc Duy wrote:

Show 30 quoted lines

> 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.