From patchwork Thu Jun 20 16:38:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007409 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6A93A924 for ; Thu, 20 Jun 2019 16:38:34 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5C3C728812 for ; Thu, 20 Jun 2019 16:38:34 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 501FC28816; Thu, 20 Jun 2019 16:38:34 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AA9D128812 for ; Thu, 20 Jun 2019 16:38:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732078AbfFTQid (ORCPT ); Thu, 20 Jun 2019 12:38:33 -0400 Received: from mail-pf1-f202.google.com ([209.85.210.202]:56319 "EHLO mail-pf1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732055AbfFTQib (ORCPT ); Thu, 20 Jun 2019 12:38:31 -0400 Received: by mail-pf1-f202.google.com with SMTP id i26so2334638pfo.22 for ; Thu, 20 Jun 2019 09:38:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=T6e6gPqlkRPSRXWcnblPBQNF/TMjH+mE+j/ew3Vnn80=; b=KEb7RC8KrthU5a2fhwfrMa6ADS+BUWmcuIbAvBNxAg9qJCpyM7SoB2Nn6KAtoIM0B1 PcR0Aq6zC25e45+F+dkTp0S3WRCsAt/ul6RceDfpjvcdA4y7GBaVgaiQDXbCyZr2196W QALp/UaHBqzS5rLiZQfSW3yNCN6f8X4kBFJ6z+lRfLQtPFmI/wHEFTezaG9/mQHFlz2r Aml+BYUdScInROwElgX5gEn87lLIka11/HbE/hBToNn24ZlJkhszGbiwaa3OBMHQrEvU DJlfz7NLwSoGHXM6UT0dv+N0wXralOX09k46JDDS6V1XlPosqRLq+MdeCvhGvSS4wQx/ 64VA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=T6e6gPqlkRPSRXWcnblPBQNF/TMjH+mE+j/ew3Vnn80=; b=CmRmh8xoXMHQvaTqFUcjRpEORfvecKtOQ3858uqfpG9+8ZI4EOidchzUIClP1rDCuY YpDz2iHM17LenEeEPL+YQ9HdR61+d+z0AHgzeWsxgIfIYbUaO/nUdYSorjSLyI/qayVI JgWfjVEsY1tjb3GdmPmDvroBUYiGDcW0weGsqJEt/R4pUmB4kHWj4sTonsPynWWnlD6M UE/2akFa+A1s7+zCqZboLx972+9u7uHD+fBf6pltEDm/3K3NVkbTgikDxncoGIRJRfzl WKf9gFQUyBbE1UERWmbn90kI+apz2NQVNS2bLhin7/8tLC2LfPYnFiYrdo/aDULp9Apf fg4Q== X-Gm-Message-State: APjAAAXXR1xz7xVjlKJcHb8bjqVvaSZyxInjWm3COZJHmMhNif+wyIk2 zY35Bciq/UezrMAMJOfa7LwzU76TumlYMw2uJfsB+uUyx+ZWK48qx9E8UWIQQtku06M8H+Qd3xN CdQJu25IS/YEDdce5/D9qRqozDSCjwvaLPJRYfs5LAYOjSODR3yJv X-Google-Smtp-Source: APXvYqzaKhDCOaB7RxpS8Qvsp4zYBRPW4oQlivHt3XiOJRfaS0aYs1t0Fea4/ibGtmsdDcsWdi+r661+ X-Received: by 2002:a63:63c1:: with SMTP id x184mr7365421pgb.213.1561048710927; Thu, 20 Jun 2019 09:38:30 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:12 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-2-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 1/9] fsck: rename and touch up init_skiplist() From: Barret Rhoden To: git@vger.kernel.org Cc: " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller , Michael Platings Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP init_skiplist() took a file consisting of SHA-1s and comments and added the objects to an oidset. This functionality is useful for other commands and will be moved to oidset.c in a future commit. In preparation for that move, this commit renames it to oidset_parse_file() to reflect its more generic usage and cleans up a few of the names. Signed-off-by: Barret Rhoden --- fsck.c | 18 +++++++++--------- t/t5504-fetch-receive-strict.sh | 14 +++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/fsck.c b/fsck.c index 4703f5556145..a28cba6b05dd 100644 --- a/fsck.c +++ b/fsck.c @@ -181,7 +181,7 @@ static int fsck_msg_type(enum fsck_msg_id msg_id, return msg_type; } -static void init_skiplist(struct fsck_options *options, const char *path) +void oidset_parse_file(struct oidset *set, const char *path) { FILE *fp; struct strbuf sb = STRBUF_INIT; @@ -189,26 +189,26 @@ static void init_skiplist(struct fsck_options *options, const char *path) fp = fopen(path, "r"); if (!fp) - die("Could not open skip list: %s", path); + die("could not open object name list: %s", path); while (!strbuf_getline(&sb, fp)) { const char *p; - const char *hash; + const char *name; /* * Allow trailing comments, leading whitespace * (including before commits), and empty or whitespace * only lines. */ - hash = strchr(sb.buf, '#'); - if (hash) - strbuf_setlen(&sb, hash - sb.buf); + name = strchr(sb.buf, '#'); + if (name) + strbuf_setlen(&sb, name - sb.buf); strbuf_trim(&sb); if (!sb.len) continue; if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0') - die("Invalid SHA-1: %s", sb.buf); - oidset_insert(&options->skiplist, &oid); + die("invalid object name: %s", sb.buf); + oidset_insert(set, &oid); } if (ferror(fp)) die_errno("Could not read '%s'", path); @@ -284,7 +284,7 @@ void fsck_set_msg_types(struct fsck_options *options, const char *values) if (!strcmp(buf, "skiplist")) { if (equal == len) die("skiplist requires a path"); - init_skiplist(options, buf + equal + 1); + oidset_parse_file(&options->skiplist, buf + equal + 1); buf += len + 1; continue; } diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh index 7bc706873c5b..fdfe179b1188 100755 --- a/t/t5504-fetch-receive-strict.sh +++ b/t/t5504-fetch-receive-strict.sh @@ -164,9 +164,9 @@ test_expect_success 'fsck with unsorted skipList' ' test_expect_success 'fsck with invalid or bogus skipList input' ' git -c fsck.skipList=/dev/null -c fsck.missingEmail=ignore fsck && test_must_fail git -c fsck.skipList=does-not-exist -c fsck.missingEmail=ignore fsck 2>err && - test_i18ngrep "Could not open skip list: does-not-exist" err && + test_i18ngrep "could not open.*: does-not-exist" err && test_must_fail git -c fsck.skipList=.git/config -c fsck.missingEmail=ignore fsck 2>err && - test_i18ngrep "Invalid SHA-1: \[core\]" err + test_i18ngrep "invalid object name: \[core\]" err ' test_expect_success 'fsck with other accepted skipList input (comments & empty lines)' ' @@ -193,7 +193,7 @@ test_expect_success 'fsck no garbage output from comments & empty lines errors' test_expect_success 'fsck with invalid abbreviated skipList input' ' echo $commit | test_copy_bytes 20 >SKIP.abbreviated && test_must_fail git -c fsck.skipList=SKIP.abbreviated fsck 2>err-abbreviated && - test_i18ngrep "^fatal: Invalid SHA-1: " err-abbreviated + test_i18ngrep "^fatal: invalid object name: " err-abbreviated ' test_expect_success 'fsck with exhaustive accepted skipList input (various types of comments etc.)' ' @@ -226,10 +226,10 @@ test_expect_success 'push with receive.fsck.skipList' ' test_must_fail git push --porcelain dst bogus && git --git-dir=dst/.git config receive.fsck.skipList does-not-exist && test_must_fail git push --porcelain dst bogus 2>err && - test_i18ngrep "Could not open skip list: does-not-exist" err && + test_i18ngrep "could not open.*: does-not-exist" err && git --git-dir=dst/.git config receive.fsck.skipList config && test_must_fail git push --porcelain dst bogus 2>err && - test_i18ngrep "Invalid SHA-1: \[core\]" err && + test_i18ngrep "invalid object name: \[core\]" err && git --git-dir=dst/.git config receive.fsck.skipList SKIP && git push --porcelain dst bogus @@ -255,10 +255,10 @@ test_expect_success 'fetch with fetch.fsck.skipList' ' test_must_fail git --git-dir=dst/.git fetch "file://$(pwd)" $refspec && git --git-dir=dst/.git config fetch.fsck.skipList does-not-exist && test_must_fail git --git-dir=dst/.git fetch "file://$(pwd)" $refspec 2>err && - test_i18ngrep "Could not open skip list: does-not-exist" err && + test_i18ngrep "could not open.*: does-not-exist" err && git --git-dir=dst/.git config fetch.fsck.skipList dst/.git/config && test_must_fail git --git-dir=dst/.git fetch "file://$(pwd)" $refspec 2>err && - test_i18ngrep "Invalid SHA-1: \[core\]" err && + test_i18ngrep "invalid object name: \[core\]" err && git --git-dir=dst/.git config fetch.fsck.skipList dst/.git/SKIP && git --git-dir=dst/.git fetch "file://$(pwd)" $refspec From patchwork Thu Jun 20 16:38:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007411 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9FB59924 for ; Thu, 20 Jun 2019 16:38:36 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 918F228812 for ; Thu, 20 Jun 2019 16:38:36 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8597328816; Thu, 20 Jun 2019 16:38:36 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2225E28812 for ; Thu, 20 Jun 2019 16:38:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732086AbfFTQif (ORCPT ); Thu, 20 Jun 2019 12:38:35 -0400 Received: from mail-pl1-f202.google.com ([209.85.214.202]:43514 "EHLO mail-pl1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726837AbfFTQie (ORCPT ); Thu, 20 Jun 2019 12:38:34 -0400 Received: by mail-pl1-f202.google.com with SMTP id t2so1901293plo.10 for ; Thu, 20 Jun 2019 09:38:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=lY2g25qlH/7Gavjz5snKkuIJIxTc1qkxz3A0cDGgC5E=; b=D4ZNGAkdGV9xNS/QO7phsLS4I5Zsdwv4GUz88FikSlLP+y1RllNqfrewfEmTlO07rP Bo7URaVTqH65dSd5jAQDE/bkieiZoUKqS8V6oLc+bII43VsZcifPYNTGBkQQ55fkjCcz Yl8GcDw6k6+kQiIeXCY1ey4AGdKyYKp7FCxMn10FMLFkktilICJ8WWudjwWVi88O+NTo UMg3Wy4PVXFXEHWT9OyRFrlIwKrzG76lhU006yoO014hZyOdyEkb1icSn8+MZLfwF+io WW/QKqrh0EWA+Rralo0QpEiM4ChXNylPxOvGMmfpqGbPyOQV8wJ4IjCSzOd0OznSQ/t9 VMbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=lY2g25qlH/7Gavjz5snKkuIJIxTc1qkxz3A0cDGgC5E=; b=afu2wP9VD5HhcM6m8PXRxNVinB1yuyg0qv++AzDCytMwneThyJRiyCMDMfmljM5wEt s498ztu1AEhA3Du+FQgGx8hrLKCGEeXQu0T/jjws/mUkXg/Qak/kcxs7MQUjvfo+ADXq tSS63j6kICPivTyppdv7/0XIrGQakHaOc3izdLpNDctllPJ6X7dvRCxBfe3sx58AYOmu zgzRo4PuivczRUM5pVDg/Vu6pWOxdSHM/NvvI9bSTt8O6ools047GNvK1S+KDMYRY9jG w7jBz0cIycivB7Hyss8jKrIo1mSrhZJSk2WDQDy4kbvvdW8GTZV/01f4vGb88U/VhSwe bo9g== X-Gm-Message-State: APjAAAVx1ubNB2hhtxVUEkeCYmbsfgPP3Zlzlr5QHjhcRGBKkogwb9fa Ai9C0Y2ETQCYCWTUZY5z2GF4NlGabEShMsA4kv4QiafJ5FYVs9FC75DBtX7M1S8CwvjIsBAF/x4 k2+5bcXd6xOUwEvOWOcDOPsULjJh/ycmBVTDvvTq5vNuDZU4X5GfR X-Google-Smtp-Source: APXvYqzGXXf9VV44VN7Me+PM4cL/G4HipXqT8YwZowBUQnpS8gFEt3bhtaKFvcMkX9Kz801CmnYbVhYU X-Received: by 2002:a63:ec13:: with SMTP id j19mr13261598pgh.174.1561048713867; Thu, 20 Jun 2019 09:38:33 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:13 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-3-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 2/9] Move oidset_parse_file() to oidset.c From: Barret Rhoden To: git@vger.kernel.org Cc: " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller , Michael Platings Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Signed-off-by: Barret Rhoden --- fsck.c | 35 ----------------------------------- oidset.c | 35 +++++++++++++++++++++++++++++++++++ oidset.h | 8 ++++++++ 3 files changed, 43 insertions(+), 35 deletions(-) diff --git a/fsck.c b/fsck.c index a28cba6b05dd..58ff3c4de992 100644 --- a/fsck.c +++ b/fsck.c @@ -181,41 +181,6 @@ static int fsck_msg_type(enum fsck_msg_id msg_id, return msg_type; } -void oidset_parse_file(struct oidset *set, const char *path) -{ - FILE *fp; - struct strbuf sb = STRBUF_INIT; - struct object_id oid; - - fp = fopen(path, "r"); - if (!fp) - die("could not open object name list: %s", path); - while (!strbuf_getline(&sb, fp)) { - const char *p; - const char *name; - - /* - * Allow trailing comments, leading whitespace - * (including before commits), and empty or whitespace - * only lines. - */ - name = strchr(sb.buf, '#'); - if (name) - strbuf_setlen(&sb, name - sb.buf); - strbuf_trim(&sb); - if (!sb.len) - continue; - - if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0') - die("invalid object name: %s", sb.buf); - oidset_insert(set, &oid); - } - if (ferror(fp)) - die_errno("Could not read '%s'", path); - fclose(fp); - strbuf_release(&sb); -} - static int parse_msg_type(const char *str) { if (!strcmp(str, "error")) diff --git a/oidset.c b/oidset.c index fe4eb921df81..584be63e520a 100644 --- a/oidset.c +++ b/oidset.c @@ -35,3 +35,38 @@ void oidset_clear(struct oidset *set) kh_release_oid(&set->set); oidset_init(set, 0); } + +void oidset_parse_file(struct oidset *set, const char *path) +{ + FILE *fp; + struct strbuf sb = STRBUF_INIT; + struct object_id oid; + + fp = fopen(path, "r"); + if (!fp) + die("could not open object name list: %s", path); + while (!strbuf_getline(&sb, fp)) { + const char *p; + const char *name; + + /* + * Allow trailing comments, leading whitespace + * (including before commits), and empty or whitespace + * only lines. + */ + name = strchr(sb.buf, '#'); + if (name) + strbuf_setlen(&sb, name - sb.buf); + strbuf_trim(&sb); + if (!sb.len) + continue; + + if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0') + die("invalid object name: %s", sb.buf); + oidset_insert(set, &oid); + } + if (ferror(fp)) + die_errno("Could not read '%s'", path); + fclose(fp); + strbuf_release(&sb); +} diff --git a/oidset.h b/oidset.h index 14f18f791fea..2dbca84d798f 100644 --- a/oidset.h +++ b/oidset.h @@ -61,6 +61,14 @@ int oidset_remove(struct oidset *set, const struct object_id *oid); */ void oidset_clear(struct oidset *set); +/** + * Add the contents of the file 'path' to an initialized oidset. Each line is + * an unabbreviated object name. Comments begin with '#', and trailing comments + * are allowed. Leading whitespace and empty or white-space only lines are + * ignored. + */ +void oidset_parse_file(struct oidset *set, const char *path); + struct oidset_iter { kh_oid_t *set; khiter_t iter; From patchwork Thu Jun 20 16:38:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007413 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EC460924 for ; Thu, 20 Jun 2019 16:38:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DE2C028812 for ; Thu, 20 Jun 2019 16:38:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D254028816; Thu, 20 Jun 2019 16:38:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 712DC28812 for ; Thu, 20 Jun 2019 16:38:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732096AbfFTQii (ORCPT ); Thu, 20 Jun 2019 12:38:38 -0400 Received: from mail-vs1-f74.google.com ([209.85.217.74]:41836 "EHLO mail-vs1-f74.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726837AbfFTQih (ORCPT ); Thu, 20 Jun 2019 12:38:37 -0400 Received: by mail-vs1-f74.google.com with SMTP id a200so1068989vsd.8 for ; Thu, 20 Jun 2019 09:38:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=0JhjSv/cDBl67LJ1rJqf4EGtcLtjCHcQTlMhWD7PD5s=; b=qsBroerz+yhBKSFTQN2WqkCKmdFNHY9w3WSS6S5a6GGzn1yPKSb4V1MTdUvF74PVGN +ri/1R5gDNIl8ZtYr5/PCETCxZ1joPAAFMn2jDJfd4vrNg3X59mBDMn/398baTKY0w3V A504K86kZWylJfKnT7KIpW1YG1ajQ7FqipV5WkvtUWJ2Otghb1n10Io8/73rw4VItUlp evUddgkjOWXIgoVx2y8TGCmgRKkrixN7KdjLomEqGVOql7WSUwVN5JMbxrEfD/H6cRaL 7dbxY5YqvQ5BrAQ4f0SA+dWD9sDw/YFrkDMSCmkXusJpYyj7J+E3jWgqPo2skm92qYBB UbOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=0JhjSv/cDBl67LJ1rJqf4EGtcLtjCHcQTlMhWD7PD5s=; b=rj+0T3u28uDWE1LsO/Ky92UrCl/QJoRjg7HTlDpin406pFNueisKF/jI6u46BBMpln 7yCz/2Kt8HP24qCJmODBO5EHkrD4acLw///5n3Oh8xmLsJnjrsn2POQEM+cmMFhufWl/ f8X2wBlQFPrm9ecp7uQayfQuyKUSNGK5o7OYvL1mf0PJMITpb4PFtHCC0yz2D2bwdWJb aI/NDtoIR4TQZSXdXIa2VPmHqm6PXSbpccthxanpLW7CoCir5nLlukqOcV0TtQn5ZyR5 Zh8Px6u3LZiZ6s9Il/xZIOUKPu07nZ0TZlYzkA6MMRHIHNioiwWcN+0AD3Qvx/NKbq5y awDA== X-Gm-Message-State: APjAAAW/ckpb4jmpWuLmp2YB6oy4IxU90U8wBUyUOO6l7vgPA2ApB0cn yf0DlJd1YO9/fRUYRQumHPnR3JOfmwNwDpihsfXciaPedbzOLNOLYVd5rp2h5V6eFkJZrN/w2AR 5w5ZbwpEk4EkAfyTnOQune5G8+TxftdX5wm0RVsapkDOGn8GKO0Lc X-Google-Smtp-Source: APXvYqwOSlRokopA9mMriKKM1ezkRlfg8pInL1qR6p8q1gkn1h93JhdcB6jC5hXxd8A/UPG72NTIQ9HO X-Received: by 2002:ab0:2789:: with SMTP id t9mr3625324uap.69.1561048716670; Thu, 20 Jun 2019 09:38:36 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:14 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-4-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 3/9] blame: use a helper function in blame_chunk() From: Barret Rhoden To: git@vger.kernel.org Cc: " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller , Michael Platings Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The same code for splitting a blame_entry at a particular line was used twice in blame_chunk(), and I'll use the helper again in an upcoming patch. Signed-off-by: Barret Rhoden --- blame.c | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/blame.c b/blame.c index 145eaf2faf9c..5369be9a2233 100644 --- a/blame.c +++ b/blame.c @@ -839,6 +839,27 @@ static struct blame_entry *reverse_blame(struct blame_entry *head, return tail; } +/* + * Splits a blame entry into two entries at 'len' lines. The original 'e' + * consists of len lines, i.e. [e->lno, e->lno + len), and the second part, + * which is returned, consists of the remainder: [e->lno + len, e->lno + + * e->num_lines). The caller needs to sort out the reference counting for the + * new entry's suspect. + */ +static struct blame_entry *split_blame_at(struct blame_entry *e, int len, + struct blame_origin *new_suspect) +{ + struct blame_entry *n = xcalloc(1, sizeof(struct blame_entry)); + + n->suspect = new_suspect; + n->lno = e->lno + len; + n->s_lno = e->s_lno + len; + n->num_lines = e->num_lines - len; + e->num_lines = len; + e->score = 0; + return n; +} + /* * Process one hunk from the patch between the current suspect for * blame_entry e and its parent. This first blames any unfinished @@ -865,14 +886,9 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq, */ if (e->s_lno + e->num_lines > tlno) { /* Move second half to a new record */ - int len = tlno - e->s_lno; - struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry)); - n->suspect = e->suspect; - n->lno = e->lno + len; - n->s_lno = e->s_lno + len; - n->num_lines = e->num_lines - len; - e->num_lines = len; - e->score = 0; + struct blame_entry *n; + + n = split_blame_at(e, tlno - e->s_lno, e->suspect); /* Push new record to diffp */ n->next = diffp; diffp = n; @@ -919,14 +935,10 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq, * Move second half to a new record to be * processed by later chunks */ - int len = same - e->s_lno; - struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry)); - n->suspect = blame_origin_incref(e->suspect); - n->lno = e->lno + len; - n->s_lno = e->s_lno + len; - n->num_lines = e->num_lines - len; - e->num_lines = len; - e->score = 0; + struct blame_entry *n; + + n = split_blame_at(e, same - e->s_lno, + blame_origin_incref(e->suspect)); /* Push new record to samep */ n->next = samep; samep = n; From patchwork Thu Jun 20 16:38:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007415 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2D8C013AF for ; Thu, 20 Jun 2019 16:38:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1C1EA28812 for ; Thu, 20 Jun 2019 16:38:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1052628816; Thu, 20 Jun 2019 16:38:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5627128812 for ; Thu, 20 Jun 2019 16:38:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732112AbfFTQil (ORCPT ); Thu, 20 Jun 2019 12:38:41 -0400 Received: from mail-qk1-f202.google.com ([209.85.222.202]:35195 "EHLO mail-qk1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726837AbfFTQil (ORCPT ); Thu, 20 Jun 2019 12:38:41 -0400 Received: by mail-qk1-f202.google.com with SMTP id 5so4366464qki.2 for ; Thu, 20 Jun 2019 09:38:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=2twuuft9dOD7A9Bk8XvaIatM8rf67eBPmCO7eDlB7T0=; b=f7nqNvCk6A1u+QMlBYV86v6Lxh1RUCzxBLYfTPCW2Edyn/oBHkw8/Zn4gm2AJfQJv+ zGTzsk+EBsIS+o/qmqdfN6nbNLYnaQVjfLLi6KtLNs10XDOzgpgcVlHHLFswon+EjQR3 wjetOc4fWKcRssRrlMk4nbDQOIhNx1wavZzu1YaUYVL0KFIHl2fTwzFpGi0fmVKNylqz 9klGgO8ymYqRc42gUiHyW4NOSCDh1ADI0Wnk/9bFgtiAJ7xS8t5/Z8TPBXSa3Zb+ZXg9 l/hBuvUM0yIGR9hrYkP6fTeFq4h/hwcM3LpwoHKsW2TNMwo/1VjpbvN4GtqrLL9KC3rK vb/g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=2twuuft9dOD7A9Bk8XvaIatM8rf67eBPmCO7eDlB7T0=; b=ub5OcSGYe3DKhQpDYs6Fx1El7YOcK0cSKZBat3vGvNMrW0o1577+5oulRGZhj7wdNj VKw/hbgkyV/xwPNHAvLXGlI3RREeqIshJVmigM9rzdAH8M2Sym8LHwphCmXiCkxmvDb/ NH30ZzuYIiTMAcv2brWWdoDmvpIyxYa2JcUArOc8YtKuEVxkRTq9ZJ8+agj9Ft0IXByV ZepvEJBgUo1nfOpeYKQvH3Ysl7LrIoBg0Ykk2UjJ3HxpogTCEJ3LDmouHJxx18x5ij1I cJbiAk+v1zvFrX8i7j4L3kl7IqIb7Jpoh2S8P/YGbPFOkCTgqbIN3Fos8tS7P14hzSgc I4gQ== X-Gm-Message-State: APjAAAUXh46T4wlDfhEP8FDu0sSXoV5bEjF4ZM71eBk/QkYf7EMAPSIV akDQMwumL7kI02VrUvhg/NlS/Sg7Ecn2IEYOeizQJn2GNjCe6YtPzZmjD4uOCvvO92gTZNiVGw1 a976cTNKcpVssbcFd6wP/EN2SkXHFGARPyYv516Le0CDQAXYpK3pA X-Google-Smtp-Source: APXvYqy0GdMjl9CFzitGCZ1HHf9gIKtYO6nd93pfT+JIqCPOZz0AJQ4wp8sRS1F3sjtHHYe6a4FznDgu X-Received: by 2002:a0c:b90a:: with SMTP id u10mr40450675qvf.201.1561048719379; Thu, 20 Jun 2019 09:38:39 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:15 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-5-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 4/9] blame: add the ability to ignore commits and their changes From: Barret Rhoden To: git@vger.kernel.org Cc: " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller , Michael Platings Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Commits that make formatting changes or function renames are often not interesting when blaming a file. A user may deem such a commit as 'not interesting' and want to ignore and its changes it when assigning blame. For example, say a file has the following git history / rev-list: ---O---A---X---B---C---D---Y---E---F Commits X and Y both touch a particular line, and the other commits do not: X: "Take a third parameter" -MyFunc(1, 2); +MyFunc(1, 2, 3); Y: "Remove camelcase" -MyFunc(1, 2, 3); +my_func(1, 2, 3); git-blame will blame Y for the change. I'd like to be able to ignore Y: both the existence of the commit as well as any changes it made. This differs from -S rev-list, which specifies the list of commits to process for the blame. We would still process Y, but just don't let the blame 'stick.' This patch adds the ability for users to ignore a revision with --ignore-rev=rev, which may be repeated. They can specify a set of files of full object names of revs, e.g. SHA-1 hashes, one per line. A single file may be specified with the blame.ignoreRevFile config option or with --ignore-rev-file=file. Both the config option and the command line option may be repeated multiple times. An empty file name "" will clear the list of revs from previously processed files. Config options are processed before command line options. For a typical use case, projects will maintain the file containing revisions for commits that perform mass reformatting, and their users have the option to ignore all of the commits in that file. Additionally, a user can use the --ignore-rev option for one-off investigation. To go back to the example above, X was a substantive change to the function, but not the change the user is interested in. The user inspected X, but wanted to find the previous change to that line - perhaps a commit that introduced that function call. To make this work, we can't simply remove all ignored commits from the rev-list. We need to diff the changes introduced by Y so that we can ignore them. We let the blames get passed to Y, just like when processing normally. When Y is the target, we make sure that Y does not *keep* any blames. Any changes that Y is responsible for get passed to its parent. Note we make one pass through all of the scapegoats (parents) to attempt to pass blame normally; we don't know if we *need* to ignore the commit until we've checked all of the parents. The blame_entry will get passed up the tree until we find a commit that has a diff chunk that affects those lines. One issue is that the ignored commit *did* make some change, and there is no general solution to finding the line in the parent commit that corresponds to a given line in the ignored commit. That makes it hard to attribute a particular line within an ignored commit's diff correctly. For example, the parent of an ignored commit has this, say at line 11: commit-a 11) #include "a.h" commit-b 12) #include "b.h" Commit X, which we will ignore, swaps these lines: commit-X 11) #include "b.h" commit-X 12) #include "a.h" We can pass that blame entry to the parent, but line 11 will be attributed to commit A, even though "include b.h" came from commit B. The blame mechanism will be looking at the parent's view of the file at line number 11. ignore_blame_entry() is set up to allow alternative algorithms for guessing per-line blames. Any line that is not attributed to the parent will continue to be blamed on the ignored commit as if that commit was not ignored. Upcoming patches have the ability to detect these lines and mark them in the blame output. The existing algorithm is simple: blame each line on the corresponding line in the parent's diff chunk. Any lines beyond that stay with the target. For example, the parent of an ignored commit has this, say at line 11: commit-a 11) void new_func_1(void *x, void *y); commit-b 12) void new_func_2(void *x, void *y); commit-c 13) some_line_c commit-d 14) some_line_d After a commit 'X', we have: commit-X 11) void new_func_1(void *x, commit-X 12) void *y); commit-X 13) void new_func_2(void *x, commit-X 14) void *y); commit-c 15) some_line_c commit-d 16) some_line_d Commit X nets two additionally lines: 13 and 14. The current guess_line_blames() algorithm will not attribute these to the parent, whose diff chunk is only two lines - not four. When we ignore with the current algorithm, we get: commit-a 11) void new_func_1(void *x, commit-b 12) void *y); commit-X 13) void new_func_2(void *x, commit-X 14) void *y); commit-c 15) some_line_c commit-d 16) some_line_d Note that line 12 was blamed on B, though B was the commit for new_func_2(), not new_func_1(). Even when guess_line_blames() finds a line in the parent, it may still be incorrect. Signed-off-by: Barret Rhoden --- Documentation/blame-options.txt | 14 +++ Documentation/config/blame.txt | 7 ++ Documentation/git-blame.txt | 1 + blame.c | 176 +++++++++++++++++++++++++-- blame.h | 2 + builtin/blame.c | 38 ++++++ t/t8013-blame-ignore-revs.sh | 203 ++++++++++++++++++++++++++++++++ 7 files changed, 432 insertions(+), 9 deletions(-) create mode 100755 t/t8013-blame-ignore-revs.sh diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index dc41957afab2..2c2d1ceb5653 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -110,5 +110,19 @@ commit. And the default value is 40. If there are more than one `-C` options given, the argument of the last `-C` will take effect. +--ignore-rev :: + Ignore changes made by the revision when assigning blame, as if the + change never happened. Lines that were changed or added by an ignored + commit will be blamed on the previous commit that changed that line or + nearby lines. This option may be specified multiple times to ignore + more than one revision. + +--ignore-revs-file :: + Ignore revisions listed in `file`, which must be in the same format as an + `fsck.skipList`. This option may be repeated, and these files will be + processed after any files specified with the `blame.ignoreRevsFile` config + option. An empty file name, `""`, will clear the list of revs from + previously processed files. + -h:: Show help message. diff --git a/Documentation/config/blame.txt b/Documentation/config/blame.txt index 67b5c1d1e02a..4da2788f306d 100644 --- a/Documentation/config/blame.txt +++ b/Documentation/config/blame.txt @@ -19,3 +19,10 @@ blame.showEmail:: blame.showRoot:: Do not treat root commits as boundaries in linkgit:git-blame[1]. This option defaults to false. + +blame.ignoreRevsFile:: + Ignore revisions listed in the file, one unabbreviated object name per + line, in linkgit:git-blame[1]. Whitespace and comments beginning with + `#` are ignored. This option may be repeated multiple times. Empty + file names will reset the list of ignored revisions. This option will + be handled before the command line option `--ignore-revs-file`. diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt index 16323eb80e31..7e8154199635 100644 --- a/Documentation/git-blame.txt +++ b/Documentation/git-blame.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] [-L ] [-S ] [-M] [-C] [-C] [-C] [--since=] + [--ignore-rev ] [--ignore-revs-file ] [--progress] [--abbrev=] [ | --contents | --reverse ..] [--] diff --git a/blame.c b/blame.c index 5369be9a2233..290bc97f31db 100644 --- a/blame.c +++ b/blame.c @@ -860,6 +860,103 @@ static struct blame_entry *split_blame_at(struct blame_entry *e, int len, return n; } +struct blame_line_tracker { + int is_parent; + int s_lno; +}; + +static int are_lines_adjacent(struct blame_line_tracker *first, + struct blame_line_tracker *second) +{ + return first->is_parent == second->is_parent && + first->s_lno + 1 == second->s_lno; +} + +/* + * This cheap heuristic assigns lines in the chunk to their relative location in + * the parent's chunk. Any additional lines are left with the target. + */ +static void guess_line_blames(struct blame_origin *parent, + struct blame_origin *target, + int tlno, int offset, int same, int parent_len, + struct blame_line_tracker *line_blames) +{ + int i, best_idx, target_idx; + int parent_slno = tlno + offset; + + for (i = 0; i < same - tlno; i++) { + target_idx = tlno + i; + best_idx = target_idx + offset; + if (best_idx < parent_slno + parent_len) { + line_blames[i].is_parent = 1; + line_blames[i].s_lno = best_idx; + } else { + line_blames[i].is_parent = 0; + line_blames[i].s_lno = target_idx; + } + } +} + +/* + * This decides which parts of a blame entry go to the parent (added to the + * ignoredp list) and which stay with the target (added to the diffp list). The + * actual decision was made in a separate heuristic function, and those answers + * for the lines in 'e' are in line_blames. This consumes e, essentially + * putting it on a list. + * + * Note that the blame entries on the ignoredp list are not necessarily sorted + * with respect to the parent's line numbers yet. + */ +static void ignore_blame_entry(struct blame_entry *e, + struct blame_origin *parent, + struct blame_origin *target, + struct blame_entry **diffp, + struct blame_entry **ignoredp, + struct blame_line_tracker *line_blames) +{ + int entry_len, nr_lines, i; + + /* + * We carve new entries off the front of e. Each entry comes from a + * contiguous chunk of lines: adjacent lines from the same origin + * (either the parent or the target). + */ + entry_len = 1; + nr_lines = e->num_lines; /* e changes in the loop */ + for (i = 0; i < nr_lines; i++) { + struct blame_entry *next = NULL; + + /* + * We are often adjacent to the next line - only split the blame + * entry when we have to. + */ + if (i + 1 < nr_lines) { + if (are_lines_adjacent(&line_blames[i], + &line_blames[i + 1])) { + entry_len++; + continue; + } + next = split_blame_at(e, entry_len, + blame_origin_incref(e->suspect)); + } + if (line_blames[i].is_parent) { + blame_origin_decref(e->suspect); + e->suspect = blame_origin_incref(parent); + e->s_lno = line_blames[i - entry_len + 1].s_lno; + e->next = *ignoredp; + *ignoredp = e; + } else { + /* e->s_lno is already in the target's address space. */ + e->next = *diffp; + *diffp = e; + } + assert(e->num_lines == entry_len); + e = next; + entry_len = 1; + } + assert(!e); +} + /* * Process one hunk from the patch between the current suspect for * blame_entry e and its parent. This first blames any unfinished @@ -869,13 +966,20 @@ static struct blame_entry *split_blame_at(struct blame_entry *e, int len, * -C options may lead to overlapping/duplicate source line number * ranges, all we can rely on from sorting/merging is the order of the * first suspect line number. + * + * tlno: line number in the target where this chunk begins + * same: line number in the target where this chunk ends + * offset: add to tlno to get the chunk starting point in the parent + * parent_len: number of lines in the parent chunk */ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq, - int tlno, int offset, int same, - struct blame_origin *parent) + int tlno, int offset, int same, int parent_len, + struct blame_origin *parent, + struct blame_origin *target, int ignore_diffs) { struct blame_entry *e = **srcq; - struct blame_entry *samep = NULL, *diffp = NULL; + struct blame_entry *samep = NULL, *diffp = NULL, *ignoredp = NULL; + struct blame_line_tracker *line_blames = NULL; while (e && e->s_lno < tlno) { struct blame_entry *next = e->next; @@ -924,6 +1028,14 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq, */ samep = NULL; diffp = NULL; + + if (ignore_diffs && same - tlno > 0) { + line_blames = xcalloc(sizeof(struct blame_line_tracker), + same - tlno); + guess_line_blames(parent, target, tlno, offset, same, + parent_len, line_blames); + } + while (e && e->s_lno < same) { struct blame_entry *next = e->next; @@ -943,10 +1055,29 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq, n->next = samep; samep = n; } - e->next = diffp; - diffp = e; + if (ignore_diffs) { + ignore_blame_entry(e, parent, target, &diffp, &ignoredp, + line_blames + e->s_lno - tlno); + } else { + e->next = diffp; + diffp = e; + } e = next; } + free(line_blames); + if (ignoredp) { + /* + * Note ignoredp is not sorted yet, and thus neither is dstq. + * That list must be sorted before we queue_blames(). We defer + * sorting until after all diff hunks are processed, so that + * guess_line_blames() can pick *any* line in the parent. The + * slight drawback is that we end up sorting all blame entries + * passed to the parent, including those that are unrelated to + * changes made by the ignored commit. + */ + **dstq = reverse_blame(ignoredp, **dstq); + *dstq = &ignoredp->next; + } **srcq = reverse_blame(diffp, reverse_blame(samep, e)); /* Move across elements that are in the unblamable portion */ if (diffp) @@ -955,7 +1086,9 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq, struct blame_chunk_cb_data { struct blame_origin *parent; + struct blame_origin *target; long offset; + int ignore_diffs; struct blame_entry **dstq; struct blame_entry **srcq; }; @@ -968,7 +1101,8 @@ static int blame_chunk_cb(long start_a, long count_a, if (start_a - start_b != d->offset) die("internal error in blame::blame_chunk_cb"); blame_chunk(&d->dstq, &d->srcq, start_b, start_a - start_b, - start_b + count_b, d->parent); + start_b + count_b, count_a, d->parent, d->target, + d->ignore_diffs); d->offset = start_a + count_a - (start_b + count_b); return 0; } @@ -980,7 +1114,7 @@ static int blame_chunk_cb(long start_a, long count_a, */ static void pass_blame_to_parent(struct blame_scoreboard *sb, struct blame_origin *target, - struct blame_origin *parent) + struct blame_origin *parent, int ignore_diffs) { mmfile_t file_p, file_o; struct blame_chunk_cb_data d; @@ -990,7 +1124,9 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb, return; /* nothing remains for this target */ d.parent = parent; + d.target = target; d.offset = 0; + d.ignore_diffs = ignore_diffs; d.dstq = &newdest; d.srcq = &target->suspects; fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob); @@ -1002,8 +1138,13 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb, oid_to_hex(&parent->commit->object.oid), oid_to_hex(&target->commit->object.oid)); /* The rest are the same as the parent */ - blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, parent); + blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, 0, + parent, target, 0); *d.dstq = NULL; + if (ignore_diffs) + newdest = llist_mergesort(newdest, get_next_blame, + set_next_blame, + compare_blame_suspect); queue_blames(sb, parent, newdest); return; @@ -1507,11 +1648,28 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin, blame_origin_incref(porigin); origin->previous = porigin; } - pass_blame_to_parent(sb, origin, porigin); + pass_blame_to_parent(sb, origin, porigin, 0); if (!origin->suspects) goto finish; } + /* + * Pass remaining suspects for ignored commits to their parents. + */ + if (oidset_contains(&sb->ignore_list, &commit->object.oid)) { + for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse); + i < num_sg && sg; + sg = sg->next, i++) { + struct blame_origin *porigin = sg_origin[i]; + + if (!porigin) + continue; + pass_blame_to_parent(sb, origin, porigin, 1); + if (!origin->suspects) + goto finish; + } + } + /* * Optionally find moves in parents' files. */ diff --git a/blame.h b/blame.h index d62f80fa74c4..bd2f23ca36cf 100644 --- a/blame.h +++ b/blame.h @@ -117,6 +117,8 @@ struct blame_scoreboard { /* linked list of blames */ struct blame_entry *ent; + struct oidset ignore_list; + /* look-up a line in the final buffer */ int num_lines; int *lineno; diff --git a/builtin/blame.c b/builtin/blame.c index 21cde57e711e..b8ef1e547cae 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -53,6 +53,7 @@ static int no_whole_file_rename; static int show_progress; static char repeated_meta_color[COLOR_MAXLEN]; static int coloring_mode; +static struct string_list ignore_revs_file_list = STRING_LIST_INIT_NODUP; static struct date_mode blame_date_mode = { DATE_ISO8601 }; static size_t blame_date_width; @@ -696,6 +697,16 @@ static int git_blame_config(const char *var, const char *value, void *cb) parse_date_format(value, &blame_date_mode); return 0; } + if (!strcmp(var, "blame.ignorerevsfile")) { + const char *str; + int ret; + + ret = git_config_pathname(&str, var, value); + if (ret) + return ret; + string_list_insert(&ignore_revs_file_list, str); + return 0; + } if (!strcmp(var, "color.blame.repeatedlines")) { if (color_parse_mem(value, strlen(value), repeated_meta_color)) warning(_("invalid color '%s' in color.blame.repeatedLines"), @@ -775,6 +786,27 @@ static int is_a_rev(const char *name) return OBJ_NONE < oid_object_info(the_repository, &oid, NULL); } +static void build_ignorelist(struct blame_scoreboard *sb, + struct string_list *ignore_revs_file_list, + struct string_list *ignore_rev_list) +{ + struct string_list_item *i; + struct object_id oid; + + oidset_init(&sb->ignore_list, 0); + for_each_string_list_item(i, ignore_revs_file_list) { + if (!strcmp(i->string, "")) + oidset_clear(&sb->ignore_list); + else + oidset_parse_file(&sb->ignore_list, i->string); + } + for_each_string_list_item(i, ignore_rev_list) { + if (get_oid_committish(i->string, &oid)) + die(_("cannot find revision %s to ignore"), i->string); + oidset_insert(&sb->ignore_list, &oid); + } +} + int cmd_blame(int argc, const char **argv, const char *prefix) { struct rev_info revs; @@ -786,6 +818,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) struct progress_info pi = { NULL, 0 }; struct string_list range_list = STRING_LIST_INIT_NODUP; + struct string_list ignore_rev_list = STRING_LIST_INIT_NODUP; int output_option = 0, opt = 0; int show_stats = 0; const char *revs_file = NULL; @@ -807,6 +840,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix) OPT_BIT('s', NULL, &output_option, N_("Suppress author name and timestamp (Default: off)"), OUTPUT_NO_AUTHOR), OPT_BIT('e', "show-email", &output_option, N_("Show author email instead of name (Default: off)"), OUTPUT_SHOW_EMAIL), OPT_BIT('w', NULL, &xdl_opts, N_("Ignore whitespace differences"), XDF_IGNORE_WHITESPACE), + OPT_STRING_LIST(0, "ignore-rev", &ignore_rev_list, N_("rev"), N_("Ignore when blaming")), + OPT_STRING_LIST(0, "ignore-revs-file", &ignore_revs_file_list, N_("file"), N_("Ignore revisions from ")), OPT_BIT(0, "color-lines", &output_option, N_("color redundant metadata from previous line differently"), OUTPUT_COLOR_LINE), OPT_BIT(0, "color-by-age", &output_option, N_("color lines by age"), OUTPUT_SHOW_AGE_WITH_COLOR), @@ -1012,6 +1047,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix) sb.contents_from = contents_from; sb.reverse = reverse; sb.repo = the_repository; + build_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list); + string_list_clear(&ignore_revs_file_list, 0); + string_list_clear(&ignore_rev_list, 0); setup_scoreboard(&sb, path, &o); lno = sb.num_lines; diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh new file mode 100755 index 000000000000..fdb2fa879781 --- /dev/null +++ b/t/t8013-blame-ignore-revs.sh @@ -0,0 +1,203 @@ +#!/bin/sh + +test_description='ignore revisions when blaming' +. ./test-lib.sh + +# Creates: +# A--B--X +# A added line 1 and B added line 2. X makes changes to those lines. Sanity +# check that X is blamed for both lines. +test_expect_success setup ' + test_commit A file line1 && + + echo line2 >>file && + git add file && + test_tick && + git commit -m B && + git tag B && + + test_write_lines line-one line-two >file && + git add file && + test_tick && + git commit -m X && + git tag X && + + git blame --line-porcelain file >blame_raw && + + grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse X >expect && + test_cmp expect actual && + + grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse X >expect && + test_cmp expect actual + ' + +# Ignore X, make sure A is blamed for line 1 and B for line 2. +test_expect_success ignore_rev_changing_lines ' + git blame --line-porcelain --ignore-rev X file >blame_raw && + + grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse A >expect && + test_cmp expect actual && + + grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse B >expect && + test_cmp expect actual + ' + +# For ignored revs that have added 'unblamable' lines, attribute those to the +# ignored commit. +# A--B--X--Y +# Where Y changes lines 1 and 2, and adds lines 3 and 4. The added lines ought +# to have nothing in common with "line-one" or "line-two", to keep any +# heuristics from matching them with any lines in the parent. +test_expect_success ignore_rev_adding_unblamable_lines ' + test_write_lines line-one-change line-two-changed y3 y4 >file && + git add file && + test_tick && + git commit -m Y && + git tag Y && + + git rev-parse Y >expect && + git blame --line-porcelain file --ignore-rev Y >blame_raw && + + grep -E "^[0-9a-f]+ [0-9]+ 3" blame_raw | sed -e "s/ .*//" >actual && + test_cmp expect actual && + + grep -E "^[0-9a-f]+ [0-9]+ 4" blame_raw | sed -e "s/ .*//" >actual && + test_cmp expect actual + ' + +# Ignore X and Y, both in separate files. Lines 1 == A, 2 == B. +test_expect_success ignore_revs_from_files ' + git rev-parse X >ignore_x && + git rev-parse Y >ignore_y && + git blame --line-porcelain file --ignore-revs-file ignore_x --ignore-revs-file ignore_y >blame_raw && + + grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse A >expect && + test_cmp expect actual && + + grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse B >expect && + test_cmp expect actual + ' + +# Ignore X from the config option, Y from a file. +test_expect_success ignore_revs_from_configs_and_files ' + git config --add blame.ignoreRevsFile ignore_x && + git blame --line-porcelain file --ignore-revs-file ignore_y >blame_raw && + + grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse A >expect && + test_cmp expect actual && + + grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse B >expect && + test_cmp expect actual + ' + +# Override blame.ignoreRevsFile (ignore_x) with an empty string. X should be +# blamed now for lines 1 and 2, since we are no longer ignoring X. +test_expect_success override_ignore_revs_file ' + git blame --line-porcelain file --ignore-revs-file "" --ignore-revs-file ignore_y >blame_raw && + git rev-parse X >expect && + + grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + test_cmp expect actual && + + grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual && + test_cmp expect actual + ' +test_expect_success bad_files_and_revs ' + test_must_fail git blame file --ignore-rev NOREV 2>err && + test_i18ngrep "cannot find revision NOREV to ignore" err && + + test_must_fail git blame file --ignore-revs-file NOFILE 2>err && + test_i18ngrep "could not open.*: NOFILE" err && + + echo NOREV >ignore_norev && + test_must_fail git blame file --ignore-revs-file ignore_norev 2>err && + test_i18ngrep "invalid object name: NOREV" err + ' +# The heuristic called by guess_line_blames() tries to find the size of a +# blame_entry 'e' in the parent's address space. Those calculations need to +# check for negative or zero values for when a blame entry is completely outside +# the window of the parent's version of a file. +# +# This happens when one commit adds several lines (commit B below). A later +# commit (C) changes one line in the middle of B's change. Commit C gets blamed +# for its change, and that breaks up B's change into multiple blame entries. +# When processing B, one of the blame_entries is outside A's window (which was +# zero - it had no lines added on its side of the diff). +# +# A--B--C, ignore B to test the ignore heuristic's boundary checks. +test_expect_success ignored_chunk_negative_parent_size ' + rm -rf .git/ && + git init && + + test_write_lines L1 L2 L7 L8 L9 >file && + git add file && + test_tick && + git commit -m A && + git tag A && + + test_write_lines L1 L2 L3 L4 L5 L6 L7 L8 L9 >file && + git add file && + test_tick && + git commit -m B && + git tag B && + + test_write_lines L1 L2 L3 L4 xxx L6 L7 L8 L9 >file && + git add file && + test_tick && + git commit -m C && + git tag C && + + git blame file --ignore-rev B >blame_raw + ' + +# Resetting the repo and creating: +# +# A--B--M +# \ / +# C-+ +# +# 'A' creates a file. B changes line 1, and C changes line 9. M merges. +test_expect_success ignore_merge ' + rm -rf .git/ && + git init && + + test_write_lines L1 L2 L3 L4 L5 L6 L7 L8 L9 >file && + git add file && + test_tick && + git commit -m A && + git tag A && + + test_write_lines BB L2 L3 L4 L5 L6 L7 L8 L9 >file && + git add file && + test_tick && + git commit -m B && + git tag B && + + git reset --hard A && + test_write_lines L1 L2 L3 L4 L5 L6 L7 L8 CC >file && + git add file && + test_tick && + git commit -m C && + git tag C && + + test_merge M B && + git blame --line-porcelain file --ignore-rev M >blame_raw && + + grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse B >expect && + test_cmp expect actual && + + grep -E "^[0-9a-f]+ [0-9]+ 9" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse C >expect && + test_cmp expect actual + ' + +test_done From patchwork Thu Jun 20 16:38:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007417 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 89FC313AF for ; Thu, 20 Jun 2019 16:38:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 79D4B28815 for ; Thu, 20 Jun 2019 16:38:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6D73028812; Thu, 20 Jun 2019 16:38:46 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9BA5228812 for ; Thu, 20 Jun 2019 16:38:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732099AbfFTQio (ORCPT ); Thu, 20 Jun 2019 12:38:44 -0400 Received: from mail-qt1-f202.google.com ([209.85.160.202]:37768 "EHLO mail-qt1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731974AbfFTQio (ORCPT ); Thu, 20 Jun 2019 12:38:44 -0400 Received: by mail-qt1-f202.google.com with SMTP id g56so4360256qte.4 for ; Thu, 20 Jun 2019 09:38:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=k+qVOvEoLg452cO5ev0b1305aLPkQsAjh+71utxKwWY=; b=rxTcObiiT3Y1lj0HWAvdmkMwUtzINDwMh32GcSo+hVMiAVGEW26cP9rMA7Ayz+5y8i GlUPvfQJhe1QTWSGqoPKOWTaikBbKI6jBdaWwZYkATtInTYrc+5KpdtLmp8DFNx0jxiy Yr2zurthZbz5+9Ma3EKcPETFDUNli1jFXh9PQzzMWWbul9mlt2iQtC/yO3tyoQYG+rub usckZLf4swihjhaTasFIXj5nL6DrHy96Y/z4VZpOL9s2oRiJ+GzX81izS1qeW0H/8QUZ Gxk9IVJNAvMKq8Cpx6NSGZLf2CRvz0J71y0Risl6Vyj2bP2OADn7ffqtsrSAPQcNwd3P O+4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=k+qVOvEoLg452cO5ev0b1305aLPkQsAjh+71utxKwWY=; b=cPJBj9gAEEtqB+kys0amUv9LTkidaBifn23wnVDM7IDotTF7mRsfZYlhLwxoPv2938 iz2HgFC9ygs12ne7QiQYV7mF9M4Y96Ih5HfrUN+U8NrmIU1+dbt6g1OA2bhrdpQH0z9R wCrbK8RMozofj9Bjz/vwV1NT7yeLifjeOcIDu/SCOzv1lYEuDOg9MYoAt0dSSpKKhzlx S7BdREv9l6GKs1bt5O/tT4Za3DLwQgAC/CPRpPEeJAJTuwhTbqebTFpI/kvYwji/XTv5 IwGoTtjpqu/E3iGcb16sEoipLXM04cR3uKrnmznEmBk9hrhNF2Evp/RA0mZV2M/wZ+u4 CKcQ== X-Gm-Message-State: APjAAAW8w2bAn3cOhtQVkHxj3DnyGrww0KcN9DAmJDk42vcPWaK5H9Hx eOz/vrPVd+D5aSb9cT28gtCLDsAvJq/QR7RJ/OJL1iwPF+bNwyHTYlQTlGCJrlsEjBMEF8q5KKa 6rzPHwnFblOVIbyZ7AuLeYE0rBv1kljFryYLRYF8awTw23WRlO9av X-Google-Smtp-Source: APXvYqzYNIUs1tMaK/aKe3d5eir1VEZOstcOU8b9UQO1pFBZj3ipwcXSfyTKY0rHWHjBq//vMKMAGB/J X-Received: by 2002:a37:9f0a:: with SMTP id i10mr38132742qke.91.1561048722792; Thu, 20 Jun 2019 09:38:42 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:16 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-6-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 5/9] blame: add config options for the output of ignored or unblamable lines From: Barret Rhoden To: git@vger.kernel.org Cc: " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller , Michael Platings Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When ignoring commits, the commit that is blamed might not be responsible for the change, due to the inaccuracy of our heuristic. Users might want to know when a particular line has a potentially inaccurate blame. Furthermore, guess_line_blames() may fail to find any parent commit for a given line touched by an ignored commit. Those 'unblamable' lines remain blamed on an ignored commit. Users might want to know if a line is unblamable so that they do not spend time investigating a commit they know is uninteresting. This patch adds two config options to mark these two types of lines in the output of blame. The first option can identify ignored lines by specifying blame.markIgnoredLines. When this option is set, each blame line that was blamed on a commit other than the ignored commit is marked with a '?'. For example: 278b6158d6fdb (Barret Rhoden 2016-04-11 13:57:54 -0400 26) appears as: ?278b6158d6fd (Barret Rhoden 2016-04-11 13:57:54 -0400 26) where the '?' is placed before the commit, and the hash has one fewer characters. Sometimes we are unable to even guess at what ancestor commit touched a line. These lines are 'unblamable.' The second option, blame.markUnblamableLines, will mark the line with '*'. For example, say we ignore e5e8d36d04cbe, yet we are unable to blame this line on another commit: e5e8d36d04cbe (Barret Rhoden 2016-04-11 13:57:54 -0400 26) appears as: *e5e8d36d04cb (Barret Rhoden 2016-04-11 13:57:54 -0400 26) When these config options are used together, every line touched by an ignored commit will be marked with either a '?' or a '*'. Signed-off-by: Barret Rhoden --- Documentation/blame-options.txt | 7 +++- Documentation/config/blame.txt | 9 +++++ blame.c | 14 ++++++- blame.h | 2 + builtin/blame.c | 18 +++++++++ t/t8013-blame-ignore-revs.sh | 71 +++++++++++++++++++++++++++++++++ 6 files changed, 119 insertions(+), 2 deletions(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 2c2d1ceb5653..5d122db6e9e6 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -115,7 +115,12 @@ take effect. change never happened. Lines that were changed or added by an ignored commit will be blamed on the previous commit that changed that line or nearby lines. This option may be specified multiple times to ignore - more than one revision. + more than one revision. If the `blame.markIgnoredLines` config option + is set, then lines that were changed by an ignored commit and attributed to + another commit will be marked with a `?` in the blame output. If the + `blame.markUnblamableLines` config option is set, then those lines touched + by an ignored commit that we could not attribute to another revision are + marked with a '*'. --ignore-revs-file :: Ignore revisions listed in `file`, which must be in the same format as an diff --git a/Documentation/config/blame.txt b/Documentation/config/blame.txt index 4da2788f306d..9468e8599c0c 100644 --- a/Documentation/config/blame.txt +++ b/Documentation/config/blame.txt @@ -26,3 +26,12 @@ blame.ignoreRevsFile:: `#` are ignored. This option may be repeated multiple times. Empty file names will reset the list of ignored revisions. This option will be handled before the command line option `--ignore-revs-file`. + +blame.markUnblamables:: + Mark lines that were changed by an ignored revision that we could not + attribute to another commit with a '*' in the output of + linkgit:git-blame[1]. + +blame.markIgnoredLines:: + Mark lines that were changed by an ignored revision that we attributed to + another commit with a '?' in the output of linkgit:git-blame[1]. diff --git a/blame.c b/blame.c index 290bc97f31db..21ae76603f5c 100644 --- a/blame.c +++ b/blame.c @@ -480,7 +480,9 @@ 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->s_lno + ent->num_lines == next->s_lno && + ent->ignored == next->ignored && + ent->unblamable == next->unblamable) { ent->num_lines += next->num_lines; ent->next = next->next; blame_origin_decref(next->suspect); @@ -730,8 +732,14 @@ static void split_overlap(struct blame_entry *split, struct blame_origin *parent) { int chunk_end_lno; + int i; memset(split, 0, sizeof(struct blame_entry [3])); + for (i = 0; i < 3; i++) { + split[i].ignored = e->ignored; + split[i].unblamable = e->unblamable; + } + if (e->s_lno < tlno) { /* there is a pre-chunk part not blamed on parent */ split[0].suspect = blame_origin_incref(e->suspect); @@ -852,6 +860,8 @@ static struct blame_entry *split_blame_at(struct blame_entry *e, int len, struct blame_entry *n = xcalloc(1, sizeof(struct blame_entry)); n->suspect = new_suspect; + n->ignored = e->ignored; + n->unblamable = e->unblamable; n->lno = e->lno + len; n->s_lno = e->s_lno + len; n->num_lines = e->num_lines - len; @@ -940,12 +950,14 @@ static void ignore_blame_entry(struct blame_entry *e, blame_origin_incref(e->suspect)); } if (line_blames[i].is_parent) { + e->ignored = 1; blame_origin_decref(e->suspect); e->suspect = blame_origin_incref(parent); e->s_lno = line_blames[i - entry_len + 1].s_lno; e->next = *ignoredp; *ignoredp = e; } else { + e->unblamable = 1; /* e->s_lno is already in the target's address space. */ e->next = *diffp; *diffp = e; diff --git a/blame.h b/blame.h index bd2f23ca36cf..2458b68f0e22 100644 --- a/blame.h +++ b/blame.h @@ -92,6 +92,8 @@ struct blame_entry { * scanning the lines over and over. */ unsigned score; + int ignored; + int unblamable; }; /* diff --git a/builtin/blame.c b/builtin/blame.c index b8ef1e547cae..ce5b0f283843 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -54,6 +54,8 @@ static int show_progress; static char repeated_meta_color[COLOR_MAXLEN]; static int coloring_mode; static struct string_list ignore_revs_file_list = STRING_LIST_INIT_NODUP; +static int mark_unblamable_lines; +static int mark_ignored_lines; static struct date_mode blame_date_mode = { DATE_ISO8601 }; static size_t blame_date_width; @@ -481,6 +483,14 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int } } + if (mark_unblamable_lines && ent->unblamable) { + length--; + putchar('*'); + } + if (mark_ignored_lines && ent->ignored) { + length--; + putchar('?'); + } printf("%.*s", length, hex); if (opt & OUTPUT_ANNOTATE_COMPAT) { const char *name; @@ -707,6 +717,14 @@ static int git_blame_config(const char *var, const char *value, void *cb) string_list_insert(&ignore_revs_file_list, str); return 0; } + if (!strcmp(var, "blame.markunblamablelines")) { + mark_unblamable_lines = git_config_bool(var, value); + return 0; + } + if (!strcmp(var, "blame.markignoredlines")) { + mark_ignored_lines = git_config_bool(var, value); + return 0; + } if (!strcmp(var, "color.blame.repeatedlines")) { if (color_parse_mem(value, strlen(value), repeated_meta_color)) warning(_("invalid color '%s' in color.blame.repeatedLines"), diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh index fdb2fa879781..36dc31eb3913 100755 --- a/t/t8013-blame-ignore-revs.sh +++ b/t/t8013-blame-ignore-revs.sh @@ -121,6 +121,77 @@ test_expect_success bad_files_and_revs ' test_must_fail git blame file --ignore-revs-file ignore_norev 2>err && test_i18ngrep "invalid object name: NOREV" err ' + +# For ignored revs that have added 'unblamable' lines, mark those lines with a +# '*' +# A--B--X--Y +# Lines 3 and 4 are from Y and unblamable. This was set up in +# ignore_rev_adding_unblamable_lines. +test_expect_success mark_unblamable_lines ' + git config --add blame.markUnblamableLines true && + + git blame --ignore-rev Y file >blame_raw && + echo "*" >expect && + + sed -n "3p" blame_raw | cut -c1 >actual && + test_cmp expect actual && + + sed -n "4p" blame_raw | cut -c1 >actual && + test_cmp expect actual + ' + +# Commit Z will touch the first two lines. Y touched all four. +# A--B--X--Y--Z +# The blame output when ignoring Z should be: +# ?Y ... 1) +# ?Y ... 2) +# Y ... 3) +# Y ... 4) +# We're checking only the first character +test_expect_success mark_ignored_lines ' + git config --add blame.markIgnoredLines true && + + test_write_lines line-one-Z line-two-Z y3 y4 >file && + git add file && + test_tick && + git commit -m Z && + git tag Z && + + git blame --ignore-rev Z file >blame_raw && + echo "?" >expect && + + sed -n "1p" blame_raw | cut -c1 >actual && + test_cmp expect actual && + + sed -n "2p" blame_raw | cut -c1 >actual && + test_cmp expect actual && + + sed -n "3p" blame_raw | cut -c1 >actual && + ! test_cmp expect actual && + + sed -n "4p" blame_raw | cut -c1 >actual && + ! test_cmp expect actual + ' + +# For ignored revs that added 'unblamable' lines and more recent commits changed +# the blamable lines, mark the unblamable lines with a +# '*' +# A--B--X--Y--Z +# Lines 3 and 4 are from Y and unblamable, as set up in +# ignore_rev_adding_unblamable_lines. Z changed lines 1 and 2. +test_expect_success mark_unblamable_lines_intermediate ' + git config --add blame.markUnblamableLines true && + + git blame --ignore-rev Y file >blame_raw 2>stderr && + echo "*" >expect && + + sed -n "3p" blame_raw | cut -c1 >actual && + test_cmp expect actual && + + sed -n "4p" blame_raw | cut -c1 >actual && + test_cmp expect actual + ' + # The heuristic called by guess_line_blames() tries to find the size of a # blame_entry 'e' in the parent's address space. Those calculations need to # check for negative or zero values for when a blame entry is completely outside From patchwork Thu Jun 20 16:38:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007419 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8A7AC924 for ; Thu, 20 Jun 2019 16:38:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7BE2628812 for ; Thu, 20 Jun 2019 16:38:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6FCBA28816; Thu, 20 Jun 2019 16:38:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D0E2D28812 for ; Thu, 20 Jun 2019 16:38:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726837AbfFTQir (ORCPT ); Thu, 20 Jun 2019 12:38:47 -0400 Received: from mail-qk1-f202.google.com ([209.85.222.202]:39999 "EHLO mail-qk1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732143AbfFTQiq (ORCPT ); Thu, 20 Jun 2019 12:38:46 -0400 Received: by mail-qk1-f202.google.com with SMTP id c1so4338760qkl.7 for ; Thu, 20 Jun 2019 09:38:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=3vUH0ayNmtnhK3Sm2/bdUw0LamMwoFqtqmHkcM8KVZ8=; b=lmvDqIoXX4f/19L/HaS3ey2Wj8/+ukkcnWRfacMRdS8+NFg56qouJkXzwoFF9UQeJb EXTdk5uXOiSAmlT4keK7XQBxu5QDKiLZoBebn9Au1+wDXLbiXH/mAyxgEbA8KKQSsmkI edh9VCBYApiE5RlSSQDsEmFxo99LVKvaG/nh9yeBCXcWwYTF1I1NF7ttmU4Xm2aSaf81 KF/iVFQnsjnD82wN1fFxpbTlFG9qBK16ELZhyTLzpNmegWkcLGd+Nd6mqo+rqbMKb5HS uJLfMIrUOP1E94pePBFEdMaxrCsrqvIo8gtbYPKnC0BQlg9H5vCz0LJPrWku3AqHoauu /5UQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=3vUH0ayNmtnhK3Sm2/bdUw0LamMwoFqtqmHkcM8KVZ8=; b=DF0MY80teN4xD+nufDULOyBhCsH54FFDG0In33ueM6nthRB4ezIZwBUDpQkv48VVn1 YUuPUp7boQ2PqoFD39PAg7q6fMMMKp6yyF6mQAtJe8VmgQKSKA5ysyhvSW+ErHioA+L+ c+PnZSUGERriOAuZuzrwbSXTn7SyHQ3pjVvuS2dNm+QN8pAfxje7nrjgaj9d5qFpjqx+ DfaeY2K2tEfPVR9R7I0gOnZPZXl8EIntr46cY0WaqoKLurbQMrKCVHGnQq9dSt/S7OM3 SuVoeSUQ0OYaclJGnZi14p/MXBCQLTCskbfKNw87KdC/HcO1DmnU04ha4rbih8G4b/MB GUmQ== X-Gm-Message-State: APjAAAUbAzrPhk30r9h0sb7Pju+CUMR7iBmQ99JnEU3e9LDUrCS8pAoY Qb7NrwgAGXBI7qbscNA49qzGXwqRUxNg3ZtogzGYt+bG6n+DwzPLl1VWh4+X2+2PbM8a3XwwmBL 3FrAH6agV/+4uXoQiI4yIiW1okKTrq2HzZp+x4hHPMJXMWdCq+6N8 X-Google-Smtp-Source: APXvYqxSObrAOercP/Ul4GSEGOB/E9oG90O34jzLve3b95IxS7kVoQ0s/+rJRNqPCFCdNbcPXveMoDfx X-Received: by 2002:a37:b87:: with SMTP id 129mr87336730qkl.132.1561048725777; Thu, 20 Jun 2019 09:38:45 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:17 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-7-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 6/9] blame: optionally track line fingerprints during fill_blame_origin() From: Barret Rhoden To: git@vger.kernel.org Cc: Michael Platings , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP fill_blame_origin() is a convenient place to store data that we will use throughout the lifetime of a blame_origin. Some heuristics for ignoring commits during a blame session can make use of this storage. In particular, we will calculate a fingerprint for each line of a file for blame_origins involved in an ignored commit. In this commit, we only calculate the line_starts, reusing the existing code from the scoreboard's line_starts. In an upcoming commit, we will actually compute the fingerprints. This feature will be used when we attempt to pass blame entries to parents when we "ignore" a commit. Most uses of fill_blame_origin() will not require this feature, hence the flag parameter. Multiple calls to fill_blame_origin() are idempotent, and any of them can request the creation of the fingerprints structure. Suggested-by: Michael Platings Signed-off-by: Barret Rhoden --- blame.c | 90 ++++++++++++++++++++++++++++++++++++++------------------- blame.h | 2 ++ 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/blame.c b/blame.c index 21ae76603f5c..49698a306e5a 100644 --- a/blame.c +++ b/blame.c @@ -311,12 +311,58 @@ static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb); } +static const char *get_next_line(const char *start, const char *end) +{ + const char *nl = memchr(start, '\n', end - start); + + return nl ? nl + 1 : end; +} + +static int find_line_starts(int **line_starts, const char *buf, + unsigned long len) +{ + const char *end = buf + len; + const char *p; + int *lineno; + int num = 0; + + for (p = buf; p < end; p = get_next_line(p, end)) + num++; + + ALLOC_ARRAY(*line_starts, num + 1); + lineno = *line_starts; + + for (p = buf; p < end; p = get_next_line(p, end)) + *lineno++ = p - buf; + + *lineno = len; + + return num; +} + +static void fill_origin_fingerprints(struct blame_origin *o, mmfile_t *file) +{ + int *line_starts; + + if (o->fingerprints) + return; + o->num_lines = find_line_starts(&line_starts, o->file.ptr, + o->file.size); + /* TODO: Will fill in fingerprints in a future commit */ + free(line_starts); +} + +static void drop_origin_fingerprints(struct blame_origin *o) +{ +} + /* * Given an origin, prepare mmfile_t structure to be used by the * diff machinery */ static void fill_origin_blob(struct diff_options *opt, - struct blame_origin *o, mmfile_t *file, int *num_read_blob) + struct blame_origin *o, mmfile_t *file, + int *num_read_blob, int fill_fingerprints) { if (!o->file.ptr) { enum object_type type; @@ -340,11 +386,14 @@ static void fill_origin_blob(struct diff_options *opt, } else *file = o->file; + if (fill_fingerprints) + fill_origin_fingerprints(o, file); } static void drop_origin_blob(struct blame_origin *o) { FREE_AND_NULL(o->file.ptr); + drop_origin_fingerprints(o); } /* @@ -1141,8 +1190,10 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb, d.ignore_diffs = ignore_diffs; d.dstq = &newdest; d.srcq = &target->suspects; - fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob); - fill_origin_blob(&sb->revs->diffopt, target, &file_o, &sb->num_read_blob); + fill_origin_blob(&sb->revs->diffopt, parent, &file_p, + &sb->num_read_blob, ignore_diffs); + fill_origin_blob(&sb->revs->diffopt, target, &file_o, + &sb->num_read_blob, ignore_diffs); sb->num_get_patch++; if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts)) @@ -1353,7 +1404,8 @@ static void find_move_in_parent(struct blame_scoreboard *sb, if (!unblamed) return; /* nothing remains for this target */ - fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob); + fill_origin_blob(&sb->revs->diffopt, parent, &file_p, + &sb->num_read_blob, 0); if (!file_p.ptr) return; @@ -1482,7 +1534,8 @@ static void find_copy_in_parent(struct blame_scoreboard *sb, norigin = get_origin(parent, p->one->path); oidcpy(&norigin->blob_oid, &p->one->oid); norigin->mode = p->one->mode; - fill_origin_blob(&sb->revs->diffopt, norigin, &file_p, &sb->num_read_blob); + fill_origin_blob(&sb->revs->diffopt, norigin, &file_p, + &sb->num_read_blob, 0); if (!file_p.ptr) continue; @@ -1822,37 +1875,14 @@ void assign_blame(struct blame_scoreboard *sb, int opt) } } -static const char *get_next_line(const char *start, const char *end) -{ - const char *nl = memchr(start, '\n', end - start); - return nl ? nl + 1 : end; -} - /* * To allow quick access to the contents of nth line in the * final image, prepare an index in the scoreboard. */ static int prepare_lines(struct blame_scoreboard *sb) { - const char *buf = sb->final_buf; - unsigned long len = sb->final_buf_size; - const char *end = buf + len; - const char *p; - int *lineno; - int num = 0; - - for (p = buf; p < end; p = get_next_line(p, end)) - num++; - - ALLOC_ARRAY(sb->lineno, num + 1); - lineno = sb->lineno; - - for (p = buf; p < end; p = get_next_line(p, end)) - *lineno++ = p - buf; - - *lineno = len; - - sb->num_lines = num; + sb->num_lines = find_line_starts(&sb->lineno, sb->final_buf, + sb->final_buf_size); return sb->num_lines; } diff --git a/blame.h b/blame.h index 2458b68f0e22..4a9e1270b036 100644 --- a/blame.h +++ b/blame.h @@ -51,6 +51,8 @@ struct blame_origin { */ struct blame_entry *suspects; mmfile_t file; + int num_lines; + void *fingerprints; struct object_id blob_oid; unsigned short mode; /* guilty gets set when shipping any suspects to the final From patchwork Thu Jun 20 16:38:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007427 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 37E231932 for ; Thu, 20 Jun 2019 16:38:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2408C28815 for ; Thu, 20 Jun 2019 16:38:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 182B528816; Thu, 20 Jun 2019 16:38:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E05542881A for ; Thu, 20 Jun 2019 16:38:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732180AbfFTQiw (ORCPT ); Thu, 20 Jun 2019 12:38:52 -0400 Received: from mail-pg1-f201.google.com ([209.85.215.201]:35601 "EHLO mail-pg1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732175AbfFTQiu (ORCPT ); Thu, 20 Jun 2019 12:38:50 -0400 Received: by mail-pg1-f201.google.com with SMTP id c18so2130397pgk.2 for ; Thu, 20 Jun 2019 09:38:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc:content-transfer-encoding; bh=N/4ZZxMyUAOhKvBKVCV2PWLJ9Rrc23IBcB3GOHg6VDw=; b=dMyc05+wQP14zpuJvz9PlV5HwOHqoQ7Yd4VFpELrgnOSSSq3K0yftCWknD5E8pKgou Sn7hBAkww7Z+wCruUMAuglwaQ1gsZ/lJTYYXoVdX1xFuGsL+6cCuCjJeVEkSHYYcxTWa Q6iSujQK8BYQ47bw1Qmr9B5+FaKuAOEfm1qEcxSiyGHJ8+PKst2uWchbPzSEXu3VJWrn CnT+ZwWvJtt25z/dJRpEDRkMZ4JOd+brPyi1C1rd5nniPbpIGAhqQ9n0FCUqWHHkaVyg O8A970S3JL9x6I2YgH1jwwq33DA8OXTnsw8QfiUi0vG5WTrKaJq4O7+cZZiRin2F2hWy kImw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc:content-transfer-encoding; bh=N/4ZZxMyUAOhKvBKVCV2PWLJ9Rrc23IBcB3GOHg6VDw=; b=mMEYCs0fH77GPCH5klhSZszTg6fFBDYK5NWMZn9Su0tMUWov7r2nc01MOecl2Mp6BI 015/bjpGKy9WW+jAAIacsu+mIFCZkSwrduZ/KR6JXSlDmE7FB1D+AsseCjXYT1zIP1sA DuCA2uB5KpVp4cx5xnHJmp++6l+tX9Ldb3reVx1r9v+kk9ZQYnULxnwWc9U+Gyu6OW3S DwoTRSlTOUOrgtiw9I+ZhZTmuF7yuKBTHmZs8Rb1CCqNtcXh2b7w4lzqJUHXTDvTRqsv 9Qe/s1S9sk+sFTdzA22+1CTuCaIFEmUa+v3Vuq+JztAih/Ss5hyqZO8damr7atxpPHof /frQ== X-Gm-Message-State: APjAAAVDDbAAoyl60TVBRvANyr3L4MYewi6MQu1LZ42VQhmUNiRPs3v+ Yxc2Y8cDnkCIkJo7ppSONEmF6SWe8dEnR0CBWIGMQaz6PKVqy6wzm23pE5V2K4XJCLIfLJ/mMOf 7sbUuOexwFp68c2L+a4ugwtAePVuqRTlWdegVufKIZ/hWOysCxY4x X-Google-Smtp-Source: APXvYqxYfuroOGyZ5DL6776PvhEg8pT6UTO7uZMk0TZKWcKoaMEzf2IwpGXQzfcY0F6vaE1suuIrCjkK X-Received: by 2002:a65:408d:: with SMTP id t13mr13589363pgp.373.1561048729188; Thu, 20 Jun 2019 09:38:49 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:18 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-8-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 7/9] blame: add a fingerprint heuristic to match ignored lines From: Barret Rhoden To: git@vger.kernel.org Cc: Michael Platings , " =?utf-8?b?w4Z2YXIgQXJuZmrDtnI=?= =?utf-8?b?w7AgQmphcm1hc29u?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Michael Platings This algorithm will replace the heuristic used to identify lines from ignored commits with one that finds likely candidate lines in the parent's version of the file. The actual replacement occurs in an upcoming commit. The old heuristic simply assigned lines in the target to the same line number (plus offset) in the parent. The new function uses a fingerprinting algorithm to detect similarity between lines. The new heuristic is designed to accurately match changes made mechanically by formatting tools such as clang-format and clang-tidy. These tools make changes such as breaking up lines to fit within a character limit or changing identifiers to fit with a naming convention. The heuristic is not intended to match more extensive refactoring changes and may give misleading results in such cases. In most cases formatting tools preserve line ordering, so the heuristic is optimised for such cases. (Some types of changes do reorder lines e.g. sorting keep the line content identical, the git blame -M option can already be used to address this). The reason that it is advantageous to rely on ordering is due to source code repeating the same character sequences often e.g. declaring an identifier on one line and using that identifier on several subsequent lines. This means that lines can look very similar to each other which presents a problem when doing fuzzy matching. Relying on ordering gives us extra clues to point towards the true match. The heuristic operates on a single diff chunk change at a time. It creates a “fingerprint” for each line on each side of the change. Fingerprints are described in detail in the comment for `struct fingerprint`, but essentially are a multiset of the character pairs in a line. The heuristic first identifies the line in the target entry whose fingerprint is most clearly matched to a line fingerprint in the parent entry. Where fingerprints match identically, the position of the lines is used as a tie-break. The heuristic locks in the best match, and subtracts the fingerprint of the line in the target entry from the fingerprint of the line in the parent entry to prevent other lines being matched on the same parts of that line. It then repeats the process recursively on the section of the chunk before the match, and then the section of the chunk after the match. Here's an example of the difference the fingerprinting makes. Consider a file with two commits: commit-a 1) void func_1(void *x, void *y); commit-b 2) void func_2(void *x, void *y); After a commit 'X', we have: commit-X 1) void func_1(void *x, commit-X 2) void *y); commit-X 3) void func_2(void *x, commit-X 4) void *y); When we blame-ignored with the old algorithm, we get: commit-a 1) void func_1(void *x, commit-b 2) void *y); commit-X 3) void func_2(void *x, commit-X 4) void *y); Where commit-b is blamed for 2 instead of 3. With the fingerprint algorithm, we get: commit-a 1) void func_1(void *x, commit-a 2) void *y); commit-b 3) void func_2(void *x, commit-b 4) void *y); Note line 2 could be matched with either commit-a or commit-b as it is equally similar to both lines, but is matched with commit-a because its position as a fraction of the new line range is more similar to commit-a as a fraction of the old line range. Line 4 is also equally similar to both lines, but as it appears after line 3 which will be matched first it cannot be matched with an earlier line. For many more examples, see t/t8014-blame-ignore-fuzzy.sh which contains example parent and target files and the line numbers in the parent that must be matched. Signed-off-by: Michael Platings --- blame.c | 642 ++++++++++++++++++++++++++++++++++ t/t8014-blame-ignore-fuzzy.sh | 440 +++++++++++++++++++++++ 2 files changed, 1082 insertions(+) create mode 100755 t/t8014-blame-ignore-fuzzy.sh diff --git a/blame.c b/blame.c index 49698a306e5a..103838546e07 100644 --- a/blame.c +++ b/blame.c @@ -340,6 +340,648 @@ static int find_line_starts(int **line_starts, const char *buf, return num; } +struct fingerprint_entry; + +/* A fingerprint is intended to loosely represent a string, such that two + * fingerprints can be quickly compared to give an indication of the similarity + * of the strings that they represent. + * + * A fingerprint is represented as a multiset of the lower-cased byte pairs in + * the string that it represents. Whitespace is added at each end of the + * string. Whitespace pairs are ignored. Whitespace is converted to '\0'. + * For example, the string "Darth Radar" will be converted to the following + * fingerprint: + * {"\0d", "da", "da", "ar", "ar", "rt", "th", "h\0", "\0r", "ra", "ad", "r\0"} + * + * The similarity between two fingerprints is the size of the intersection of + * their multisets, including repeated elements. See fingerprint_similarity for + * examples. + * + * For ease of implementation, the fingerprint is implemented as a map + * of byte pairs to the count of that byte pair in the string, instead of + * allowing repeated elements in a set. + */ +struct fingerprint { + struct hashmap map; + /* As we know the maximum number of entries in advance, it's + * convenient to store the entries in a single array instead of having + * the hashmap manage the memory. + */ + struct fingerprint_entry *entries; +}; + +/* A byte pair in a fingerprint. Stores the number of times the byte pair + * occurs in the string that the fingerprint represents. + */ +struct fingerprint_entry { + /* The hashmap entry - the hash represents the byte pair in its + * entirety so we don't need to store the byte pair separately. + */ + struct hashmap_entry entry; + /* The number of times the byte pair occurs in the string that the + * fingerprint represents. + */ + int count; +}; + +/* See `struct fingerprint` for an explanation of what a fingerprint is. + * \param result the fingerprint of the string is stored here. This must be + * freed later using free_fingerprint. + * \param line_begin the start of the string + * \param line_end the end of the string + */ +static void get_fingerprint(struct fingerprint *result, + const char *line_begin, + const char *line_end) +{ + unsigned int hash, c0 = 0, c1; + const char *p; + int max_map_entry_count = 1 + line_end - line_begin; + struct fingerprint_entry *entry = xcalloc(max_map_entry_count, + sizeof(struct fingerprint_entry)); + struct fingerprint_entry *found_entry; + + hashmap_init(&result->map, NULL, NULL, max_map_entry_count); + result->entries = entry; + for (p = line_begin; p <= line_end; ++p, c0 = c1) { + /* Always terminate the string with whitespace. + * Normalise whitespace to 0, and normalise letters to + * lower case. This won't work for multibyte characters but at + * worst will match some unrelated characters. + */ + if ((p == line_end) || isspace(*p)) + c1 = 0; + else + c1 = tolower(*p); + hash = c0 | (c1 << 8); + /* Ignore whitespace pairs */ + if (hash == 0) + continue; + hashmap_entry_init(entry, hash); + + found_entry = hashmap_get(&result->map, entry, NULL); + if (found_entry) { + found_entry->count += 1; + } else { + entry->count = 1; + hashmap_add(&result->map, entry); + ++entry; + } + } +} + +static void free_fingerprint(struct fingerprint *f) +{ + hashmap_free(&f->map, 0); + free(f->entries); +} + +/* Calculates the similarity between two fingerprints as the size of the + * intersection of their multisets, including repeated elements. See + * `struct fingerprint` for an explanation of the fingerprint representation. + * The similarity between "cat mat" and "father rather" is 2 because "at" is + * present twice in both strings while the similarity between "tim" and "mit" + * is 0. + */ +static int fingerprint_similarity(struct fingerprint *a, struct fingerprint *b) +{ + int intersection = 0; + struct hashmap_iter iter; + const struct fingerprint_entry *entry_a, *entry_b; + + hashmap_iter_init(&b->map, &iter); + + while ((entry_b = hashmap_iter_next(&iter))) { + if ((entry_a = hashmap_get(&a->map, entry_b, NULL))) { + intersection += entry_a->count < entry_b->count ? + entry_a->count : entry_b->count; + } + } + return intersection; +} + +/* Subtracts byte-pair elements in B from A, modifying A in place. + */ +static void fingerprint_subtract(struct fingerprint *a, struct fingerprint *b) +{ + struct hashmap_iter iter; + struct fingerprint_entry *entry_a; + const struct fingerprint_entry *entry_b; + + hashmap_iter_init(&b->map, &iter); + + while ((entry_b = hashmap_iter_next(&iter))) { + if ((entry_a = hashmap_get(&a->map, entry_b, NULL))) { + if (entry_a->count <= entry_b->count) + hashmap_remove(&a->map, entry_b, NULL); + else + entry_a->count -= entry_b->count; + } + } +} + +/* Calculate fingerprints for a series of lines. + * Puts the fingerprints in the fingerprints array, which must have been + * preallocated to allow storing line_count elements. + */ +static void get_line_fingerprints(struct fingerprint *fingerprints, + const char *content, const int *line_starts, + long first_line, long line_count) +{ + int i; + const char *linestart, *lineend; + + line_starts += first_line; + for (i = 0; i < line_count; ++i) { + linestart = content + line_starts[i]; + lineend = content + line_starts[i + 1]; + get_fingerprint(fingerprints + i, linestart, lineend); + } +} + +static void free_line_fingerprints(struct fingerprint *fingerprints, + int nr_fingerprints) +{ + int i; + + for (i = 0; i < nr_fingerprints; i++) + free_fingerprint(&fingerprints[i]); +} + +/* This contains the data necessary to linearly map a line number in one half + * of a diff chunk to the line in the other half of the diff chunk that is + * closest in terms of its position as a fraction of the length of the chunk. + */ +struct line_number_mapping { + int destination_start, destination_length, + source_start, source_length; +}; + +/* Given a line number in one range, offset and scale it to map it onto the + * other range. + * Essentially this mapping is a simple linear equation but the calculation is + * more complicated to allow performing it with integer operations. + * Another complication is that if a line could map onto many lines in the + * destination range then we want to choose the line at the center of those + * possibilities. + * Example: if the chunk is 2 lines long in A and 10 lines long in B then the + * first 5 lines in B will map onto the first line in the A chunk, while the + * last 5 lines will all map onto the second line in the A chunk. + * Example: if the chunk is 10 lines long in A and 2 lines long in B then line + * 0 in B will map onto line 2 in A, and line 1 in B will map onto line 7 in A. + */ +static int map_line_number(int line_number, + const struct line_number_mapping *mapping) +{ + return ((line_number - mapping->source_start) * 2 + 1) * + mapping->destination_length / + (mapping->source_length * 2) + + mapping->destination_start; +} + +/* Get a pointer to the element storing the similarity between a line in A + * and a line in B. + * + * The similarities are stored in a 2-dimensional array. Each "row" in the + * array contains the similarities for a line in B. The similarities stored in + * a row are the similarities between the line in B and the nearby lines in A. + * To keep the length of each row the same, it is padded out with values of -1 + * where the search range extends beyond the lines in A. + * For example, if max_search_distance_a is 2 and the two sides of a diff chunk + * look like this: + * a | m + * b | n + * c | o + * d | p + * e | q + * Then the similarity array will contain: + * [-1, -1, am, bm, cm, + * -1, an, bn, cn, dn, + * ao, bo, co, do, eo, + * bp, cp, dp, ep, -1, + * cq, dq, eq, -1, -1] + * Where similarities are denoted either by -1 for invalid, or the + * concatenation of the two lines in the diff being compared. + * + * \param similarities array of similarities between lines in A and B + * \param line_a the index of the line in A, in the same frame of reference as + * closest_line_a. + * \param local_line_b the index of the line in B, relative to the first line + * in B that similarities represents. + * \param closest_line_a the index of the line in A that is deemed to be + * closest to local_line_b. This must be in the same + * frame of reference as line_a. This value defines + * where similarities is centered for the line in B. + * \param max_search_distance_a maximum distance in lines from the closest line + * in A for other lines in A for which + * similarities may be calculated. + */ +static int *get_similarity(int *similarities, + int line_a, int local_line_b, + int closest_line_a, int max_search_distance_a) +{ + assert(abs(line_a - closest_line_a) <= + max_search_distance_a); + return similarities + line_a - closest_line_a + + max_search_distance_a + + local_line_b * (max_search_distance_a * 2 + 1); +} + +#define CERTAIN_NOTHING_MATCHES -2 +#define CERTAINTY_NOT_CALCULATED -1 + +/* Given a line in B, first calculate its similarities with nearby lines in A + * if not already calculated, then identify the most similar and second most + * similar lines. The "certainty" is calculated based on those two + * similarities. + * + * \param start_a the index of the first line of the chunk in A + * \param length_a the length in lines of the chunk in A + * \param local_line_b the index of the line in B, relative to the first line + * in the chunk. + * \param fingerprints_a array of fingerprints for the chunk in A + * \param fingerprints_b array of fingerprints for the chunk in B + * \param similarities 2-dimensional array of similarities between lines in A + * and B. See get_similarity() for more details. + * \param certainties array of values indicating how strongly a line in B is + * matched with some line in A. + * \param second_best_result array of absolute indices in A for the second + * closest match of a line in B. + * \param result array of absolute indices in A for the closest match of a line + * in B. + * \param max_search_distance_a maximum distance in lines from the closest line + * in A for other lines in A for which + * similarities may be calculated. + * \param map_line_number_in_b_to_a parameter to map_line_number(). + */ +static void find_best_line_matches( + int start_a, + int length_a, + int start_b, + int local_line_b, + struct fingerprint *fingerprints_a, + struct fingerprint *fingerprints_b, + int *similarities, + int *certainties, + int *second_best_result, + int *result, + const int max_search_distance_a, + const struct line_number_mapping *map_line_number_in_b_to_a) +{ + + int i, search_start, search_end, closest_local_line_a, *similarity, + best_similarity = 0, second_best_similarity = 0, + best_similarity_index = 0, second_best_similarity_index = 0; + + /* certainty has already been calculated so no need to redo the work */ + if (certainties[local_line_b] != CERTAINTY_NOT_CALCULATED) + return; + + closest_local_line_a = map_line_number( + local_line_b + start_b, map_line_number_in_b_to_a) - start_a; + + search_start = closest_local_line_a - max_search_distance_a; + if (search_start < 0) + search_start = 0; + + search_end = closest_local_line_a + max_search_distance_a + 1; + if (search_end > length_a) + search_end = length_a; + + for (i = search_start; i < search_end; ++i) { + similarity = get_similarity(similarities, + i, local_line_b, + closest_local_line_a, + max_search_distance_a); + if (*similarity == -1) { + /* This value will never exceed 10 but assert just in + * case + */ + assert(abs(i - closest_local_line_a) < 1000); + /* scale the similarity by (1000 - distance from + * closest line) to act as a tie break between lines + * that otherwise are equally similar. + */ + *similarity = fingerprint_similarity( + fingerprints_b + local_line_b, + fingerprints_a + i) * + (1000 - abs(i - closest_local_line_a)); + } + if (*similarity > best_similarity) { + second_best_similarity = best_similarity; + second_best_similarity_index = best_similarity_index; + best_similarity = *similarity; + best_similarity_index = i; + } else if (*similarity > second_best_similarity) { + second_best_similarity = *similarity; + second_best_similarity_index = i; + } + } + + if (best_similarity == 0) { + /* this line definitely doesn't match with anything. Mark it + * with this special value so it doesn't get invalidated and + * won't be recalculated. + */ + certainties[local_line_b] = CERTAIN_NOTHING_MATCHES; + result[local_line_b] = -1; + } else { + /* Calculate the certainty with which this line matches. + * If the line matches well with two lines then that reduces + * the certainty. However we still want to prioritise matching + * a line that matches very well with two lines over matching a + * line that matches poorly with one line, hence doubling + * best_similarity. + * This means that if we have + * line X that matches only one line with a score of 3, + * line Y that matches two lines equally with a score of 5, + * and line Z that matches only one line with a score or 2, + * then the lines in order of certainty are X, Y, Z. + */ + certainties[local_line_b] = best_similarity * 2 - + second_best_similarity; + + /* We keep both the best and second best results to allow us to + * check at a later stage of the matching process whether the + * result needs to be invalidated. + */ + result[local_line_b] = start_a + best_similarity_index; + second_best_result[local_line_b] = + start_a + second_best_similarity_index; + } +} + +/* + * This finds the line that we can match with the most confidence, and + * uses it as a partition. It then calls itself on the lines on either side of + * that partition. In this way we avoid lines appearing out of order, and + * retain a sensible line ordering. + * \param start_a index of the first line in A with which lines in B may be + * compared. + * \param start_b index of the first line in B for which matching should be + * done. + * \param length_a number of lines in A with which lines in B may be compared. + * \param length_b number of lines in B for which matching should be done. + * \param fingerprints_a mutable array of fingerprints in A. The first element + * corresponds to the line at start_a. + * \param fingerprints_b array of fingerprints in B. The first element + * corresponds to the line at start_b. + * \param similarities 2-dimensional array of similarities between lines in A + * and B. See get_similarity() for more details. + * \param certainties array of values indicating how strongly a line in B is + * matched with some line in A. + * \param second_best_result array of absolute indices in A for the second + * closest match of a line in B. + * \param result array of absolute indices in A for the closest match of a line + * in B. + * \param max_search_distance_a maximum distance in lines from the closest line + * in A for other lines in A for which + * similarities may be calculated. + * \param max_search_distance_b an upper bound on the greatest possible + * distance between lines in B such that they will + * both be compared with the same line in A + * according to max_search_distance_a. + * \param map_line_number_in_b_to_a parameter to map_line_number(). + */ +static void fuzzy_find_matching_lines_recurse( + int start_a, int start_b, + int length_a, int length_b, + struct fingerprint *fingerprints_a, + struct fingerprint *fingerprints_b, + int *similarities, + int *certainties, + int *second_best_result, + int *result, + int max_search_distance_a, + int max_search_distance_b, + const struct line_number_mapping *map_line_number_in_b_to_a) +{ + int i, invalidate_min, invalidate_max, offset_b, + second_half_start_a, second_half_start_b, + second_half_length_a, second_half_length_b, + most_certain_line_a, most_certain_local_line_b = -1, + most_certain_line_certainty = -1, + closest_local_line_a; + + for (i = 0; i < length_b; ++i) { + find_best_line_matches(start_a, + length_a, + start_b, + i, + fingerprints_a, + fingerprints_b, + similarities, + certainties, + second_best_result, + result, + max_search_distance_a, + map_line_number_in_b_to_a); + + if (certainties[i] > most_certain_line_certainty) { + most_certain_line_certainty = certainties[i]; + most_certain_local_line_b = i; + } + } + + /* No matches. */ + if (most_certain_local_line_b == -1) + return; + + most_certain_line_a = result[most_certain_local_line_b]; + + /* + * Subtract the most certain line's fingerprint in B from the matched + * fingerprint in A. This means that other lines in B can't also match + * the same parts of the line in A. + */ + fingerprint_subtract(fingerprints_a + most_certain_line_a - start_a, + fingerprints_b + most_certain_local_line_b); + + /* Invalidate results that may be affected by the choice of most + * certain line. + */ + invalidate_min = most_certain_local_line_b - max_search_distance_b; + invalidate_max = most_certain_local_line_b + max_search_distance_b + 1; + if (invalidate_min < 0) + invalidate_min = 0; + if (invalidate_max > length_b) + invalidate_max = length_b; + + /* As the fingerprint in A has changed, discard previously calculated + * similarity values with that fingerprint. + */ + for (i = invalidate_min; i < invalidate_max; ++i) { + closest_local_line_a = map_line_number( + i + start_b, map_line_number_in_b_to_a) - start_a; + + /* Check that the lines in A and B are close enough that there + * is a similarity value for them. + */ + if (abs(most_certain_line_a - start_a - closest_local_line_a) > + max_search_distance_a) { + continue; + } + + *get_similarity(similarities, most_certain_line_a - start_a, + i, closest_local_line_a, + max_search_distance_a) = -1; + } + + /* More invalidating of results that may be affected by the choice of + * most certain line. + * Discard the matches for lines in B that are currently matched with a + * line in A such that their ordering contradicts the ordering imposed + * by the choice of most certain line. + */ + for (i = most_certain_local_line_b - 1; i >= invalidate_min; --i) { + /* In this loop we discard results for lines in B that are + * before most-certain-line-B but are matched with a line in A + * that is after most-certain-line-A. + */ + if (certainties[i] >= 0 && + (result[i] >= most_certain_line_a || + second_best_result[i] >= most_certain_line_a)) { + certainties[i] = CERTAINTY_NOT_CALCULATED; + } + } + for (i = most_certain_local_line_b + 1; i < invalidate_max; ++i) { + /* In this loop we discard results for lines in B that are + * after most-certain-line-B but are matched with a line in A + * that is before most-certain-line-A. + */ + if (certainties[i] >= 0 && + (result[i] <= most_certain_line_a || + second_best_result[i] <= most_certain_line_a)) { + certainties[i] = CERTAINTY_NOT_CALCULATED; + } + } + + /* Repeat the matching process for lines before the most certain line. + */ + if (most_certain_local_line_b > 0) { + fuzzy_find_matching_lines_recurse( + start_a, start_b, + most_certain_line_a + 1 - start_a, + most_certain_local_line_b, + fingerprints_a, fingerprints_b, similarities, + certainties, second_best_result, result, + max_search_distance_a, + max_search_distance_b, + map_line_number_in_b_to_a); + } + /* Repeat the matching process for lines after the most certain line. + */ + if (most_certain_local_line_b + 1 < length_b) { + second_half_start_a = most_certain_line_a; + offset_b = most_certain_local_line_b + 1; + second_half_start_b = start_b + offset_b; + second_half_length_a = + length_a + start_a - second_half_start_a; + second_half_length_b = + length_b + start_b - second_half_start_b; + fuzzy_find_matching_lines_recurse( + second_half_start_a, second_half_start_b, + second_half_length_a, second_half_length_b, + fingerprints_a + second_half_start_a - start_a, + fingerprints_b + offset_b, + similarities + + offset_b * (max_search_distance_a * 2 + 1), + certainties + offset_b, + second_best_result + offset_b, result + offset_b, + max_search_distance_a, + max_search_distance_b, + map_line_number_in_b_to_a); + } +} + +/* Find the lines in the parent line range that most closely match the lines in + * the target line range. This is accomplished by matching fingerprints in each + * blame_origin, and choosing the best matches that preserve the line ordering. + * See struct fingerprint for details of fingerprint matching, and + * fuzzy_find_matching_lines_recurse for details of preserving line ordering. + * + * The performance is believed to be O(n log n) in the typical case and O(n^2) + * in a pathological case, where n is the number of lines in the target range. + */ +static int *fuzzy_find_matching_lines(struct blame_origin *parent, + struct blame_origin *target, + int tlno, int parent_slno, int same, + int parent_len) +{ + /* We use the terminology "A" for the left hand side of the diff AKA + * parent, and "B" for the right hand side of the diff AKA target. */ + int start_a = parent_slno; + int length_a = parent_len; + int start_b = tlno; + int length_b = same - tlno; + + struct line_number_mapping map_line_number_in_b_to_a = { + start_a, length_a, start_b, length_b + }; + + struct fingerprint *fingerprints_a = parent->fingerprints; + struct fingerprint *fingerprints_b = target->fingerprints; + + int i, *result, *second_best_result, + *certainties, *similarities, similarity_count; + + /* + * max_search_distance_a means that given a line in B, compare it to + * the line in A that is closest to its position, and the lines in A + * that are no greater than max_search_distance_a lines away from the + * closest line in A. + * + * max_search_distance_b is an upper bound on the greatest possible + * distance between lines in B such that they will both be compared + * with the same line in A according to max_search_distance_a. + */ + int max_search_distance_a = 10, max_search_distance_b; + + if (length_a <= 0) + return NULL; + + if (max_search_distance_a >= length_a) + max_search_distance_a = length_a ? length_a - 1 : 0; + + max_search_distance_b = ((2 * max_search_distance_a + 1) * length_b + - 1) / length_a; + + result = xcalloc(sizeof(int), length_b); + second_best_result = xcalloc(sizeof(int), length_b); + certainties = xcalloc(sizeof(int), length_b); + + /* See get_similarity() for details of similarities. */ + similarity_count = length_b * (max_search_distance_a * 2 + 1); + similarities = xcalloc(sizeof(int), similarity_count); + + for (i = 0; i < length_b; ++i) { + result[i] = -1; + second_best_result[i] = -1; + certainties[i] = CERTAINTY_NOT_CALCULATED; + } + + for (i = 0; i < similarity_count; ++i) + similarities[i] = -1; + + fuzzy_find_matching_lines_recurse(start_a, start_b, + length_a, length_b, + fingerprints_a + start_a, + fingerprints_b + start_b, + similarities, + certainties, + second_best_result, + result, + max_search_distance_a, + max_search_distance_b, + &map_line_number_in_b_to_a); + + free(similarities); + free(certainties); + free(second_best_result); + + return result; +} + static void fill_origin_fingerprints(struct blame_origin *o, mmfile_t *file) { int *line_starts; diff --git a/t/t8014-blame-ignore-fuzzy.sh b/t/t8014-blame-ignore-fuzzy.sh new file mode 100755 index 000000000000..844396615271 --- /dev/null +++ b/t/t8014-blame-ignore-fuzzy.sh @@ -0,0 +1,440 @@ +#!/bin/sh + +test_description='git blame ignore fuzzy heuristic' +. ./test-lib.sh + +# short circuit until blame has the fuzzy capabilities +test_done + +pick_author='s/^[0-9a-f^]* *(\([^ ]*\) .*/\1/' + +# Each test is composed of 4 variables: +# titleN - the test name +# aN - the initial content +# bN - the final content +# expectedN - the line numbers from aN that we expect git blame +# on bN to identify, or "Final" if bN itself should +# be identified as the origin of that line. + +# We start at test 2 because setup will show as test 1 +title2="Regression test for partially overlapping search ranges" +cat <a2 +1 +2 +3 +abcdef +5 +6 +7 +ijkl +9 +10 +11 +pqrs +13 +14 +15 +wxyz +17 +18 +19 +EOF +cat <b2 +abcde +ijk +pqr +wxy +EOF +cat <expected2 +4 +8 +12 +16 +EOF + +title3="Combine 3 lines into 2" +cat <a3 +if ((maxgrow==0) || + ( single_line_field && (field->dcols < maxgrow)) || + (!single_line_field && (field->drows < maxgrow))) +EOF +cat <b3 +if ((maxgrow == 0) || (single_line_field && (field->dcols < maxgrow)) || + (!single_line_field && (field->drows < maxgrow))) { +EOF +cat <expected3 +2 +3 +EOF + +title4="Add curly brackets" +cat <a4 + if (rows) *rows = field->rows; + if (cols) *cols = field->cols; + if (frow) *frow = field->frow; + if (fcol) *fcol = field->fcol; +EOF +cat <b4 + if (rows) { + *rows = field->rows; + } + if (cols) { + *cols = field->cols; + } + if (frow) { + *frow = field->frow; + } + if (fcol) { + *fcol = field->fcol; + } +EOF +cat <expected4 +1 +1 +Final +2 +2 +Final +3 +3 +Final +4 +4 +Final +EOF + + +title5="Combine many lines and change case" +cat <a5 +for(row=0,pBuffer=field->buf; + row 0) + { + wmove( win, row, 0 ); + waddnstr( win, pBuffer, len ); +EOF +cat <b5 +for (Row = 0, PBuffer = field->buf; Row < Height; Row++, PBuffer += Width) { + if ((Len = (int)(afterEndOfData(PBuffer, Width) - PBuffer)) > 0) { + wmove(win, Row, 0); + waddnstr(win, PBuffer, Len); +EOF +cat <expected5 +1 +5 +7 +8 +EOF + +title6="Rename and combine lines" +cat <a6 +bool need_visual_update = ((form != (FORM *)0) && + (form->status & _POSTED) && + (form->current==field)); + +if (need_visual_update) + Synchronize_Buffer(form); + +if (single_line_field) +{ + growth = field->cols * amount; + if (field->maxgrow) + growth = Minimum(field->maxgrow - field->dcols,growth); + field->dcols += growth; + if (field->dcols == field->maxgrow) +EOF +cat <b6 +bool NeedVisualUpdate = ((Form != (FORM *)0) && (Form->status & _POSTED) && + (Form->current == field)); + +if (NeedVisualUpdate) { + synchronizeBuffer(Form); +} + +if (SingleLineField) { + Growth = field->cols * amount; + if (field->maxgrow) { + Growth = Minimum(field->maxgrow - field->dcols, Growth); + } + field->dcols += Growth; + if (field->dcols == field->maxgrow) { +EOF +cat <expected6 +1 +3 +4 +5 +6 +Final +7 +8 +10 +11 +12 +Final +13 +14 +EOF + +# Both lines match identically so position must be used to tie-break. +title7="Same line twice" +cat <a7 +abc +abc +EOF +cat <b7 +abcd +abcd +EOF +cat <expected7 +1 +2 +EOF + +title8="Enforce line order" +cat <a8 +abcdef +ghijkl +ab +EOF +cat <b8 +ghijk +abcd +EOF +cat <expected8 +2 +3 +EOF + +title9="Expand lines and rename variables" +cat <a9 +int myFunction(int ArgumentOne, Thing *ArgTwo, Blah XuglyBug) { + Squiggle FabulousResult = squargle(ArgumentOne, *ArgTwo, + XuglyBug) + EwwwGlobalWithAReallyLongNameYepTooLong; + return FabulousResult * 42; +} +EOF +cat <b9 +int myFunction(int argument_one, Thing *arg_asdfgh, + Blah xugly_bug) { + Squiggle fabulous_result = squargle(argument_one, + *arg_asdfgh, xugly_bug) + + g_ewww_global_with_a_really_long_name_yep_too_long; + return fabulous_result * 42; +} +EOF +cat <expected9 +1 +1 +2 +3 +3 +4 +5 +EOF + +title10="Two close matches versus one less close match" +cat <a10 +abcdef +abcdef +ghijkl +EOF +cat <b10 +gh +abcdefx +EOF +cat <expected10 +Final +2 +EOF + +# The first line of b matches best with the last line of a, but the overall +# match is better if we match it with the the first line of a. +title11="Piggy in the middle" +cat <a11 +abcdefg +ijklmn +abcdefgh +EOF +cat <b11 +abcdefghx +ijklm +EOF +cat <expected11 +1 +2 +EOF + +title12="No trailing newline" +printf "abc\ndef" >a12 +printf "abx\nstu" >b12 +cat <expected12 +1 +Final +EOF + +title13="Reorder includes" +cat <a13 +#include "c.h" +#include "b.h" +#include "a.h" +#include "e.h" +#include "d.h" +EOF +cat <b13 +#include "a.h" +#include "b.h" +#include "c.h" +#include "d.h" +#include "e.h" +EOF +cat <expected13 +3 +2 +1 +5 +4 +EOF + +last_test=13 + +test_expect_success setup ' + { for i in $(test_seq 2 $last_test) + do + # Append each line in a separate commit to make it easy to + # check which original line the blame output relates to. + + line_count=0 && + { while IFS= read line + do + line_count=$((line_count+1)) && + echo "$line" >>"$i" && + git add "$i" && + test_tick && + GIT_AUTHOR_NAME="$line_count" git commit -m "$line_count" + done } <"a$i" + done } && + + { for i in $(test_seq 2 $last_test) + do + # Overwrite the files with the final content. + cp b$i $i && + git add $i + done } && + test_tick && + + # Commit the final content all at once so it can all be + # referred to with the same commit ID. + GIT_AUTHOR_NAME=Final git commit -m Final && + + IGNOREME=$(git rev-parse HEAD) +' + +for i in $(test_seq 2 $last_test); do + eval title="\$title$i" + test_expect_success "$title" \ + "git blame -M9 --ignore-rev $IGNOREME $i >output && + sed -e \"$pick_author\" output >actual && + test_cmp expected$i actual" +done + +# This invoked a null pointer dereference when the chunk callback was called +# with a zero length parent chunk and there were no more suspects. +test_expect_success 'Diff chunks with no suspects' ' + test_write_lines xy1 A B C xy1 >file && + git add file && + test_tick && + GIT_AUTHOR_NAME=1 git commit -m 1 && + + test_write_lines xy2 A B xy2 C xy2 >file && + git add file && + test_tick && + GIT_AUTHOR_NAME=2 git commit -m 2 && + REV_2=$(git rev-parse HEAD) && + + test_write_lines xy3 A >file && + git add file && + test_tick && + GIT_AUTHOR_NAME=3 git commit -m 3 && + REV_3=$(git rev-parse HEAD) && + + test_write_lines 1 1 >expected && + + git blame --ignore-rev $REV_2 --ignore-rev $REV_3 file >output && + sed -e "$pick_author" output >actual && + + test_cmp expected actual + ' + +test_expect_success 'position matching' ' + test_write_lines abc def >file2 && + git add file2 && + test_tick && + GIT_AUTHOR_NAME=1 git commit -m 1 && + + test_write_lines abc def abc def >file2 && + git add file2 && + test_tick && + GIT_AUTHOR_NAME=2 git commit -m 2 && + + test_write_lines abcx defx abcx defx >file2 && + git add file2 && + test_tick && + GIT_AUTHOR_NAME=3 git commit -m 3 && + REV_3=$(git rev-parse HEAD) && + + test_write_lines abcy defy abcx defx >file2 && + git add file2 && + test_tick && + GIT_AUTHOR_NAME=4 git commit -m 4 && + REV_4=$(git rev-parse HEAD) && + + test_write_lines 1 1 2 2 >expected && + + git blame --ignore-rev $REV_3 --ignore-rev $REV_4 file2 >output && + sed -e "$pick_author" output >actual && + + test_cmp expected actual + ' + +# This fails if each blame entry is processed independently instead of +# processing each diff change in full. +test_expect_success 'preserve order' ' + test_write_lines bcde >file3 && + git add file3 && + test_tick && + GIT_AUTHOR_NAME=1 git commit -m 1 && + + test_write_lines bcde fghij >file3 && + git add file3 && + test_tick && + GIT_AUTHOR_NAME=2 git commit -m 2 && + + test_write_lines bcde fghij abcd >file3 && + git add file3 && + test_tick && + GIT_AUTHOR_NAME=3 git commit -m 3 && + + test_write_lines abcdx fghijx bcdex >file3 && + git add file3 && + test_tick && + GIT_AUTHOR_NAME=4 git commit -m 4 && + REV_4=$(git rev-parse HEAD) && + + test_write_lines abcdx fghijy bcdex >file3 && + git add file3 && + test_tick && + GIT_AUTHOR_NAME=5 git commit -m 5 && + REV_5=$(git rev-parse HEAD) && + + test_write_lines 1 2 3 >expected && + + git blame --ignore-rev $REV_4 --ignore-rev $REV_5 file3 >output && + sed -e "$pick_author" output >actual && + + test_cmp expected actual + ' + +test_done From patchwork Thu Jun 20 16:38:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007425 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8F22B924 for ; Thu, 20 Jun 2019 16:38:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 816BE28812 for ; Thu, 20 Jun 2019 16:38:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 75FD728816; Thu, 20 Jun 2019 16:38:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EA8B828812 for ; Thu, 20 Jun 2019 16:38:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732196AbfFTQiz (ORCPT ); Thu, 20 Jun 2019 12:38:55 -0400 Received: from mail-pf1-f201.google.com ([209.85.210.201]:52062 "EHLO mail-pf1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732175AbfFTQix (ORCPT ); Thu, 20 Jun 2019 12:38:53 -0400 Received: by mail-pf1-f201.google.com with SMTP id 145so2342399pfv.18 for ; Thu, 20 Jun 2019 09:38:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=A3Z0cWOfQhQzlRlFoUMY3K2LatW/nPAWWeEJ0b23eQY=; b=iqvKlbuQeazwuHHa1lXQBSK+O2tLFdvh2wek0az7O8BbaXmlr7w2vew+mdjxmbJyNo 33i9eqPbZRmfnZRGiyD6ywg2viKdWuxF/e90iIaExY1Mi9iO5tjRnONkuwwSQO5nK83z yReRDbuxseLzpymE77dtJCVngVab7umLBbed1lS1CGBIUQzgLuhoQWCOdT855AaYBU77 P31jiT22HLSAPw/D4IPzjBKIAYpaRnNQid6rXZMfRZLmvxlpWFOgDcGvv3ycJz/Na+uW IQuxN4kmExm1zqticRFk6Rpm1BwOa+EukrwNiVwu2fHLwHMSs8AbmM4KpjFPzQpcQR82 IT8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=A3Z0cWOfQhQzlRlFoUMY3K2LatW/nPAWWeEJ0b23eQY=; b=KJy8lEIk3J2yBgkMNF4KdlKVOOYSiJrXLZBv9SxcrKCEkecElgNwTRASWgXzSlxxQ0 fzzv/NfHsEavrge+yTMadtGOMlJkcig3OYsMl4aG5hLduJPxlyrn3eY+fuRBHYPrAaR1 TA9R46xV+W1pjfkdnHFIe1tjZl/ROTE/bGy+vGCQxqZHWQR0EaD1qNM5VqbNrlSsygAh gKlNXMuVF6lInJSAU3kJF2CSc6eSn2c0+AI3MDHiqa5PEbCjX2DiGvUF/dSksFdqESPo Ls7HT00D/Ks/Bf+Zkb3uxHHhchpSDdj4PDFfLe6HdrOtXt8W4HH/y3yQU00n95Y2gvcX gw0g== X-Gm-Message-State: APjAAAUyYASYIHINEX5b6JsoDhXcugUAZTZAkvkt/w7HO25D6llm8GaC Z4x9M68BJZKAAYMo+3/AjqPNtAeoTqBJLVYuSi9wVBQC+v3nL3MfL50kE6SV1OTYdm97YhKjhMF FnJAn6comTJy0wXt2xAOhqFA/QNuqN7ZTdJuG0ly+0eBrfDZTMkFo X-Google-Smtp-Source: APXvYqyEqA9WCRwvU5oTvrBCiCgo+k9A8Nt5bMuI+P1O/UqPZxjmWXu4Vzvb37CKoCu/9WPcjDrFNAfi X-Received: by 2002:a63:f146:: with SMTP id o6mr13318950pgk.179.1561048731912; Thu, 20 Jun 2019 09:38:51 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:19 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-9-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 8/9] blame: use the fingerprint heuristic to match ignored lines From: Barret Rhoden To: git@vger.kernel.org Cc: " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller , Michael Platings Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This commit integrates the fuzzy fingerprint heuristic into guess_line_blames(). We actually make two passes. The first pass uses the fuzzy algorithm to find a match within the current diff chunk. If that fails, the second pass searches the entire parent file for the best match. For an example of scanning the entire parent for a match, consider: commit-a 30) #include commit-b 31) #include commit-c 32) #include Then commit X alphabetizes them: commit-X 30) #include commit-X 31) #include commit-X 32) #include If we just check the parent's chunk (i.e. the first pass), we'd get: commit-b 30) #include commit-c 31) #include commit-X 32) #include That's because commit X actually consists of two chunks: one chunk is removing sys/header_a.h, then some context, and the second chunk is adding sys/header_a.h. If we scan the entire parent file, we get: commit-b 30) #include commit-c 31) #include commit-a 32) #include Signed-off-by: Barret Rhoden --- blame.c | 60 ++++++++++++++++++++++++++++++++--- t/t8014-blame-ignore-fuzzy.sh | 3 -- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/blame.c b/blame.c index 103838546e07..f81ec9a8cf80 100644 --- a/blame.c +++ b/blame.c @@ -990,12 +990,19 @@ static void fill_origin_fingerprints(struct blame_origin *o, mmfile_t *file) return; o->num_lines = find_line_starts(&line_starts, o->file.ptr, o->file.size); - /* TODO: Will fill in fingerprints in a future commit */ + o->fingerprints = xcalloc(sizeof(struct fingerprint), o->num_lines); + get_line_fingerprints(o->fingerprints, o->file.ptr, line_starts, + 0, o->num_lines); free(line_starts); } static void drop_origin_fingerprints(struct blame_origin *o) { + if (o->fingerprints) { + free_line_fingerprints(o->fingerprints, o->num_lines); + o->num_lines = 0; + FREE_AND_NULL(o->fingerprints); + } } /* @@ -1573,9 +1580,34 @@ static int are_lines_adjacent(struct blame_line_tracker *first, first->s_lno + 1 == second->s_lno; } +static int scan_parent_range(struct fingerprint *p_fps, + struct fingerprint *t_fps, int t_idx, + int from, int nr_lines) +{ + int sim, p_idx; + #define FINGERPRINT_FILE_THRESHOLD 10 + int best_sim_val = FINGERPRINT_FILE_THRESHOLD; + int best_sim_idx = -1; + + for (p_idx = from; p_idx < from + nr_lines; p_idx++) { + sim = fingerprint_similarity(&t_fps[t_idx], &p_fps[p_idx]); + if (sim < best_sim_val) + continue; + /* Break ties with the closest-to-target line number */ + if (sim == best_sim_val && best_sim_idx != -1 && + abs(best_sim_idx - t_idx) < abs(p_idx - t_idx)) + continue; + best_sim_val = sim; + best_sim_idx = p_idx; + } + return best_sim_idx; +} + /* - * This cheap heuristic assigns lines in the chunk to their relative location in - * the parent's chunk. Any additional lines are left with the target. + * The first pass checks the blame entry (from the target) against the parent's + * diff chunk. If that fails for a line, the second pass tries to match that + * line to any part of parent file. That catches cases where a change was + * broken into two chunks by 'context.' */ static void guess_line_blames(struct blame_origin *parent, struct blame_origin *target, @@ -1584,11 +1616,22 @@ static void guess_line_blames(struct blame_origin *parent, { int i, best_idx, target_idx; int parent_slno = tlno + offset; + int *fuzzy_matches; + fuzzy_matches = fuzzy_find_matching_lines(parent, target, + tlno, parent_slno, same, + parent_len); for (i = 0; i < same - tlno; i++) { target_idx = tlno + i; - best_idx = target_idx + offset; - if (best_idx < parent_slno + parent_len) { + if (fuzzy_matches && fuzzy_matches[i] >= 0) { + best_idx = fuzzy_matches[i]; + } else { + best_idx = scan_parent_range(parent->fingerprints, + target->fingerprints, + target_idx, 0, + parent->num_lines); + } + if (best_idx >= 0) { line_blames[i].is_parent = 1; line_blames[i].s_lno = best_idx; } else { @@ -1596,6 +1639,7 @@ static void guess_line_blames(struct blame_origin *parent, line_blames[i].s_lno = target_idx; } } + free(fuzzy_matches); } /* @@ -2372,6 +2416,12 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin, if (!porigin) continue; pass_blame_to_parent(sb, origin, porigin, 1); + /* + * Preemptively drop porigin so we can refresh the + * fingerprints if we use the parent again, which can + * occur if you ignore back-to-back commits. + */ + drop_origin_blob(porigin); if (!origin->suspects) goto finish; } diff --git a/t/t8014-blame-ignore-fuzzy.sh b/t/t8014-blame-ignore-fuzzy.sh index 844396615271..6f1a94caef22 100755 --- a/t/t8014-blame-ignore-fuzzy.sh +++ b/t/t8014-blame-ignore-fuzzy.sh @@ -3,9 +3,6 @@ test_description='git blame ignore fuzzy heuristic' . ./test-lib.sh -# short circuit until blame has the fuzzy capabilities -test_done - pick_author='s/^[0-9a-f^]* *(\([^ ]*\) .*/\1/' # Each test is composed of 4 variables: From patchwork Thu Jun 20 16:38:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 11007451 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E49F5924 for ; Thu, 20 Jun 2019 16:40:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D7D802881A for ; Thu, 20 Jun 2019 16:40:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CC4C528846; Thu, 20 Jun 2019 16:40:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, USER_IN_DEF_DKIM_WL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7B7652881A for ; Thu, 20 Jun 2019 16:40:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732375AbfFTQjx (ORCPT ); Thu, 20 Jun 2019 12:39:53 -0400 Received: from mail-pg1-f201.google.com ([209.85.215.201]:53291 "EHLO mail-pg1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732201AbfFTQi4 (ORCPT ); Thu, 20 Jun 2019 12:38:56 -0400 Received: by mail-pg1-f201.google.com with SMTP id j36so2096337pgb.20 for ; Thu, 20 Jun 2019 09:38:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=bspsNAjXAX8xQZooyKU2OPmxbmU3vPrBvNkTDWda4Bk=; b=rmpXWiruWC8XLfRoeRhEpnkYEST40axHm45tfpsuoIUySrW4USfeL9Y82EZVDV/kq8 vgZbtPSbUC4L7GP1023l4IVQDdNq1yLDwzDWwcwXBeMNneeRy2QypArHMlhImcO1VCqd vy6Bub9aAG/6C4D/+/HP0cbZS6IHr5uZYvDA3bwtHNOLSD518Mq73scUkEnfAVkNTqtH cCrqI/ILMmuu/Zdkl/hm/ibIevtduTgRRIvk4z4P65jUmk9DDGTbhJ1VbDDCmCQ+1wVS bGHz2dlXBlUfoT5zncPXcdAnuA1KfDziUk3aZLgHhx0tg4E7pmVTgdjBd5cdWljZvCZ+ Q1YQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=bspsNAjXAX8xQZooyKU2OPmxbmU3vPrBvNkTDWda4Bk=; b=pksS02XWy0bh4bCl7j6SXa2m6nZnQ65JIXxF59LjMiGJZchX5DdknoZuV0yONskXaI ga99zrwZMXuSci2lcfmI/yV3SjlvjsvylW3lDLDQsZ8zgjBcOM/KjfqF5ZUquS8oA+EI fwQ8Uchp7GtHARj78EHTLIgkfn7zF7mw0aDYipYydp5a9754BQ1NYE8rooeQmP7Q/D84 0xRK0AwdPMdW7DU5dRsetOfFDg2ZPhHpP0YFasDxKF7VU6YI8cjwybF4PvYazUvgaa4s vz0zwhNdLNL9hFnEkA0BUVl4o9efhvTI4C7qIOwq+I4BGbN6CNDpiliPZuwvVtoCEnVZ wkWg== X-Gm-Message-State: APjAAAXWRN9zSRuazfF5ceqGwpzogVjgNl5iFYSokKb6H7HOCGwwmWC6 DXk4POcI3ol+xHAz/m020vl/q9vLkpOPX17ry/cvaHDYP9fxZ8VDwM2Y2CU7755fHi2XevrFJGa tz6rpD5ULKlAzsxSzG8pENz0Y5uiVmRNzVhS9RIPkg+TOjQIYhKYd X-Google-Smtp-Source: APXvYqwBSo9EVcoG5YEWoaYHd42bwSIpU/TMfSDmSpNg+X/FMYCvMA6QWvfH6wzydi8cTN3SdULv0iba X-Received: by 2002:a63:1d1d:: with SMTP id d29mr13585339pgd.259.1561048735136; Thu, 20 Jun 2019 09:38:55 -0700 (PDT) Date: Thu, 20 Jun 2019 12:38:20 -0400 In-Reply-To: <20190620163820.231316-1-brho@google.com> Message-Id: <20190620163820.231316-10-brho@google.com> Mime-Version: 1.0 References: <20190620163820.231316-1-brho@google.com> X-Mailer: git-send-email 2.22.0.410.gd8fdbe21b5-goog Subject: [PATCH v9 9/9] blame: add a test to cover blame_coalesce() From: Barret Rhoden To: git@vger.kernel.org Cc: " =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= " , David Kastrup , Jeff King , Jeff Smith , Johannes Schindelin , Junio C Hamano , " =?utf-8?q?Ren=C3=A9_Scharfe?= " , Stefan Beller , Michael Platings Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Signed-off-by: Barret Rhoden --- t/t8003-blame-corner-cases.sh | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh index c92a47b6d5b1..1c5fb1d1f8c9 100755 --- a/t/t8003-blame-corner-cases.sh +++ b/t/t8003-blame-corner-cases.sh @@ -275,4 +275,40 @@ 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 + DEF + EOF + git add giraffe && + git commit -m "original file" && + oid=$(git rev-parse HEAD) && + + cat >giraffe <<-\EOF && + ABC + SPLIT + DEF + EOF + git add giraffe && + git commit -m "interior SPLIT line" && + + cat >giraffe <<-\EOF && + ABC + DEF + EOF + git add giraffe && + git commit -m "same contents as original" && + + cat >expect <<-EOF && + $oid 1) ABC + $oid 2) DEF + EOF + git -c core.abbrev=40 blame -s giraffe >actual && + test_cmp expect actual +' + test_done