git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Barret Rhoden <brho@google.com>, Nuthan Munaiah <nm6061@rit.edu>,
	git@vger.kernel.org
Subject: [PATCH 3/3] blame: only coalesce lines that are adjacent in result
Date: Thu, 13 Aug 2020 01:25:31 -0400	[thread overview]
Message-ID: <20200813052531.GC2514880@coredump.intra.peff.net> (raw)
In-Reply-To: <20200813052054.GA1962792@coredump.intra.peff.net>

After blame has finished but before we produce any output, we coalesce
groups of lines that were adjacent in the original suspect (which may
have been split apart by lines in intermediate commits which went away).
However, this can cause incorrect output if the lines are not also
adjacent in the result. For instance, the case in t8003 has:

  ABC
  DEF

which becomes

  ABC
  SPLIT
  DEF

Blaming only lines 1 and 3 in the result yields two blame groups (one
for each line) that were adjacent in the original. That's enough for us
to coalesce them into a single group, but that loses information: our
output routines assume they're adjacent in the result as well, and we
output:

  <oid> 1) ABC
  <oid> 2) SPLIT

This is nonsense for two reasons:

  - we were asked about line 3, not line 2; we should not output the
    SPLIT line at all

  - commit <oid> did not touch the SPLIT line at all! We found the
    correct blame for line 3, but the bug is actually in the output
    stage, which is showing the wrong line number and content from the
    final file.

We can fix this by only coalescing when both the suspect and result
lines are adjacent. That fixes this bug, but keeps coalescing in cases
where want it (e.g., the existing test in t8003 where SPLIT goes away,
and the lines really are adjacent in the result).

Reported-by: Nuthan Munaiah <nm6061@rit.edu>
Signed-off-by: Jeff King <peff@peff.net>
---
 blame.c                       | 1 +
 t/t8003-blame-corner-cases.sh | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/blame.c b/blame.c
index 82fa16d658..1be1cd82a2 100644
--- a/blame.c
+++ b/blame.c
@@ -1184,6 +1184,7 @@ void blame_coalesce(struct blame_scoreboard *sb)
 	for (ent = sb->ent; ent && (next = ent->next); ent = next) {
 		if (ent->suspect == next->suspect &&
 		    ent->s_lno + ent->num_lines == next->s_lno &&
+		    ent->lno + ent->num_lines == next->lno &&
 		    ent->ignored == next->ignored &&
 		    ent->unblamable == next->unblamable) {
 			ent->num_lines += next->num_lines;
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 7f39a84289..ba8013b002 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -310,4 +310,13 @@ test_expect_success 'blame coalesce' '
 	test_cmp expect actual
 '
 
+test_expect_success 'blame does not coalesce non-adjacent result lines' '
+	cat >expect <<-EOF &&
+	$orig 1) ABC
+	$orig 3) DEF
+	EOF
+	git blame --no-abbrev -s -L1,1 -L3,3 $split giraffe >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.28.0.570.ge2c3ee08e4

  parent reply	other threads:[~2020-08-13  5:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-07  1:05 `git blame` Line Number Off-by-one Nuthan Munaiah
2020-08-07 21:21 ` Jeff King
2020-08-07 21:33   ` Jeff King
2020-08-07 21:45     ` Jeff King
2020-08-07 22:05     ` Junio C Hamano
2020-08-07 22:26       ` Jeff King
2020-08-07 22:35         ` Jeff King
2020-08-13  5:20           ` [PATCH 0/3] blame: fix bug in coalescing non-adjacent "-L" chunks Jeff King
2020-08-13  5:23             ` [PATCH 1/3] t8003: check output of coalesced blame Jeff King
2020-08-13 17:04               ` Junio C Hamano
2020-08-13 17:22                 ` Jeff King
2020-08-13  5:23             ` [PATCH 2/3] t8003: factor setup out of coalesce test Jeff King
2020-08-13  5:25             ` Jeff King [this message]
2020-08-13 16:59               ` [PATCH 3/3] blame: only coalesce lines that are adjacent in result Junio C Hamano
2020-08-13 18:41               ` Junio C Hamano
2020-08-13 12:25             ` [PATCH 0/3] blame: fix bug in coalescing non-adjacent "-L" chunks Barret Rhoden

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200813052531.GC2514880@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=brho@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=nm6061@rit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).