From patchwork Thu Aug 13 05:23:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 11711685 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B8D70739 for ; Thu, 13 Aug 2020 05:23:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 94E3520829 for ; Thu, 13 Aug 2020 05:23:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725982AbgHMFXG (ORCPT ); Thu, 13 Aug 2020 01:23:06 -0400 Received: from cloud.peff.net ([104.130.231.41]:57342 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725915AbgHMFXG (ORCPT ); Thu, 13 Aug 2020 01:23:06 -0400 Received: (qmail 15529 invoked by uid 109); 13 Aug 2020 05:23:06 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 13 Aug 2020 05:23:06 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10574 invoked by uid 111); 13 Aug 2020 05:23:05 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 13 Aug 2020 01:23:05 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 13 Aug 2020 01:23:05 -0400 From: Jeff King To: Junio C Hamano Cc: Barret Rhoden , Nuthan Munaiah , git@vger.kernel.org Subject: [PATCH 1/3] t8003: check output of coalesced blame Message-ID: <20200813052305.GA2514880@coredump.intra.peff.net> References: <20200813052054.GA1962792@coredump.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200813052054.GA1962792@coredump.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Commit f0cbe742f4 (blame: add a test to cover blame_coalesce(), 2019-06-20) added a test case where blame can usefully coalesce two groups of lines. But since it relies on the normal blame output, it only exercises the code and can't tell whether the lines were actually joined into a single group. However, by using --porcelain output, we can see how git-blame considers the groupings (and likewise how the coalescing might have a real user-visible impact for a tool that uses the porcelain-output groupings). This lets us confirm that we are indeed coalescing correctly (and the fact that this test case requires coalescing can be verified by dropping the call to blame_coalesce(), causing the test to fail). Signed-off-by: Jeff King --- t/t8003-blame-corner-cases.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh index b871dd4f86..1e89494ef6 100755 --- a/t/t8003-blame-corner-cases.sh +++ b/t/t8003-blame-corner-cases.sh @@ -273,10 +273,6 @@ test_expect_success 'blame file with CRLF core.autocrlf=true' ' grep "A U Thor" actual ' -# Tests the splitting and merging of blame entries in blame_coalesce(). -# The output of blame is the same, regardless of whether blame_coalesce() runs -# or not, so we'd likely only notice a problem if blame crashes or assigned -# blame to the "splitting" commit ('SPLIT' below). test_expect_success 'blame coalesce' ' cat >giraffe <<-\EOF && ABC @@ -302,10 +298,11 @@ test_expect_success 'blame coalesce' ' git commit -m "same contents as original" && cat >expect <<-EOF && - $oid 1) ABC - $oid 2) DEF + $oid 1 1 2 + $oid 2 2 EOF - git -c core.abbrev=$(test_oid hexsz) blame -s giraffe >actual && + git blame --porcelain giraffe >actual.raw && + grep "^$oid" actual.raw >actual && test_cmp expect actual ' From patchwork Thu Aug 13 05:23:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 11711687 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6924A618 for ; Thu, 13 Aug 2020 05:23:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 50754207DA for ; Thu, 13 Aug 2020 05:23:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726102AbgHMFXm (ORCPT ); Thu, 13 Aug 2020 01:23:42 -0400 Received: from cloud.peff.net ([104.130.231.41]:57354 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725915AbgHMFXm (ORCPT ); Thu, 13 Aug 2020 01:23:42 -0400 Received: (qmail 15545 invoked by uid 109); 13 Aug 2020 05:23:42 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 13 Aug 2020 05:23:42 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10600 invoked by uid 111); 13 Aug 2020 05:23:41 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 13 Aug 2020 01:23:41 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 13 Aug 2020 01:23:41 -0400 From: Jeff King To: Junio C Hamano Cc: Barret Rhoden , Nuthan Munaiah , git@vger.kernel.org Subject: [PATCH 2/3] t8003: factor setup out of coalesce test Message-ID: <20200813052341.GB2514880@coredump.intra.peff.net> References: <20200813052054.GA1962792@coredump.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200813052054.GA1962792@coredump.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In preparation for adding more tests of blame's coalesce code, let's split the setup out from the first test, and give each of the commits a more meaningful name: - $orig for the original source that added the lines - $split for the version where they are split apart - $final for the final version that re-joins them That's not strictly necessary, but makes the follow-on tests less brittle than relying on HEAD^, etc, to name the commits. Signed-off-by: Jeff King --- t/t8003-blame-corner-cases.sh | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh index 1e89494ef6..7f39a84289 100755 --- a/t/t8003-blame-corner-cases.sh +++ b/t/t8003-blame-corner-cases.sh @@ -273,14 +273,14 @@ test_expect_success 'blame file with CRLF core.autocrlf=true' ' grep "A U Thor" actual ' -test_expect_success 'blame coalesce' ' +test_expect_success 'setup coalesce tests' ' cat >giraffe <<-\EOF && ABC DEF EOF git add giraffe && git commit -m "original file" && - oid=$(git rev-parse HEAD) && + orig=$(git rev-parse HEAD) && cat >giraffe <<-\EOF && ABC @@ -289,20 +289,24 @@ test_expect_success 'blame coalesce' ' EOF git add giraffe && git commit -m "interior SPLIT line" && + split=$(git rev-parse HEAD) && cat >giraffe <<-\EOF && ABC DEF EOF git add giraffe && git commit -m "same contents as original" && + final=$(git rev-parse HEAD) +' +test_expect_success 'blame coalesce' ' cat >expect <<-EOF && - $oid 1 1 2 - $oid 2 2 + $orig 1 1 2 + $orig 2 2 EOF - git blame --porcelain giraffe >actual.raw && - grep "^$oid" actual.raw >actual && + git blame --porcelain $final giraffe >actual.raw && + grep "^$orig" actual.raw >actual && test_cmp expect actual ' From patchwork Thu Aug 13 05:25:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 11711689 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id F0B20739 for ; Thu, 13 Aug 2020 05:25:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DD08020715 for ; Thu, 13 Aug 2020 05:25:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725978AbgHMFZc (ORCPT ); Thu, 13 Aug 2020 01:25:32 -0400 Received: from cloud.peff.net ([104.130.231.41]:57360 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725891AbgHMFZc (ORCPT ); Thu, 13 Aug 2020 01:25:32 -0400 Received: (qmail 15561 invoked by uid 109); 13 Aug 2020 05:25:32 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 13 Aug 2020 05:25:32 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10619 invoked by uid 111); 13 Aug 2020 05:25:31 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Thu, 13 Aug 2020 01:25:31 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 13 Aug 2020 01:25:31 -0400 From: Jeff King To: Junio C Hamano Cc: Barret Rhoden , Nuthan Munaiah , git@vger.kernel.org Subject: [PATCH 3/3] blame: only coalesce lines that are adjacent in result Message-ID: <20200813052531.GC2514880@coredump.intra.peff.net> References: <20200813052054.GA1962792@coredump.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200813052054.GA1962792@coredump.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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: 1) ABC 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 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 Signed-off-by: Jeff King --- 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