From patchwork Wed Apr 10 16:24:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 10894339 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 DF8B2139A for ; Wed, 10 Apr 2019 16:24:31 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C2F7D28C1D for ; Wed, 10 Apr 2019 16:24:31 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C0E0628C12; Wed, 10 Apr 2019 16:24:31 +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 10F0E28B2C for ; Wed, 10 Apr 2019 16:24:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729582AbfDJQYa (ORCPT ); Wed, 10 Apr 2019 12:24:30 -0400 Received: from mail-vk1-f202.google.com ([209.85.221.202]:54927 "EHLO mail-vk1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727277AbfDJQY3 (ORCPT ); Wed, 10 Apr 2019 12:24:29 -0400 Received: by mail-vk1-f202.google.com with SMTP id r132so1196274vke.21 for ; Wed, 10 Apr 2019 09:24:28 -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=gJ2aSAphWMyMYVcygCMJGS0BGTB8tp8cPgxmA2+NKs0=; b=B4EbvJxxEUO45cUBWOolOm6QGTsS4uqd/XNiRjtohiyudsbsI7RtsFr3C3wjI4l3Kk uKp8sUL/QVs9TAWtLjWcwXWj5wzhiH1XM/qYGtBbNi9ucn8HA6mp6J/+23YHxf6TC9IB 5XTTpT3NGihLfSDINCC++E3xCmOX+gyJLeNabZtzl5RvikaPO0xZKnjsBBJY5RJnCvnR ROTUuIZ7aNSFljFfgSpxzA7yZ9/+2QQdMaO7UoUUpKika7L8VELuU9DywbD0ywRtYbHn knCAv7xZU6ZwRukj6RO6yZrj9UsWizp6K6GMpxo7icz+oOQzWmboaM6XTGoKPrjK0UBT Btsw== 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=gJ2aSAphWMyMYVcygCMJGS0BGTB8tp8cPgxmA2+NKs0=; b=QfrLTirDQUoqn4t0EYmF/SWWBv5u0GC4pIr3gOud39uuofWJtnMiHNmHAEK6M+Bv3e HXUs70Ah+NPO+gPrCQdMXk3Ce0Ae5omd+vW8pHScbzivqg4UGif5NgbBYpDKfaTc/eQ4 vSMrFTYTANUMTPXeCh7yz2QSYxjG/Uyt9nS940TkIAsbL8lRQCPvOrGYBUWXkNuITy6z +Dpra5vA/UmItg1Sn82N5aMUPWL+OQauOoNthSeu4Ok4xIFZS1Fwwka3tbnPxqnzcv+a ebynip5EUVQjN6G8CoaeiCEtFQ2dG2vTj74Xf7vM7Chm00w1TOo/pHCD+fPVB0QMeVSn netg== X-Gm-Message-State: APjAAAX89vM5AlnXCRlqSIxUL68GIJN+snbm89qR7xbcCnG3flJoCpX7 hLwHw+jIAneMSeBG4YErKotqz7CHbKSujFrNnun50WbYvN2JRPuUEcqzmjf+avD95/aMvAWsQ5v LG0pgBsme90eTxZZ7JztySQE0BBCGhFaivKnmJ9XrFMU/tOKiK/uM X-Google-Smtp-Source: APXvYqxutmuh4hCtMk+7lKdKvEWK5bxOYmbb9Re0cpGJ3eO1UKcYyw8CeOuBs4K1AyG7/g6zj42/jp24 X-Received: by 2002:a1f:2751:: with SMTP id n78mr4567644vkn.11.1554913468308; Wed, 10 Apr 2019 09:24:28 -0700 (PDT) Date: Wed, 10 Apr 2019 12:24:04 -0400 In-Reply-To: <20190410162409.117264-1-brho@google.com> Message-Id: <20190410162409.117264-2-brho@google.com> Mime-Version: 1.0 References: <20190410162409.117264-1-brho@google.com> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog Subject: [PATCH v6 1/6] Move init_skiplist() outside of fsck 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. Signed-off-by: Barret Rhoden --- fsck.c | 37 +-------------------------------- oidset.c | 35 +++++++++++++++++++++++++++++++ oidset.h | 8 +++++++ t/t5504-fetch-receive-strict.sh | 14 ++++++------- 4 files changed, 51 insertions(+), 43 deletions(-) diff --git a/fsck.c b/fsck.c index 2260adb71e7a..d45534ad90f5 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; } -static void init_skiplist(struct fsck_options *options, const char *path) -{ - FILE *fp; - struct strbuf sb = STRBUF_INIT; - struct object_id oid; - - fp = fopen(path, "r"); - if (!fp) - die("Could not open skip list: %s", path); - while (!strbuf_getline(&sb, fp)) { - const char *p; - const char *hash; - - /* - * 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); - 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); - } - 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")) @@ -284,7 +249,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/oidset.c b/oidset.c index fe4eb921df81..878a1b56af1c 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 c9d0f6d3cc8b..c4807749df8d 100644 --- a/oidset.h +++ b/oidset.h @@ -73,6 +73,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; diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh index 7bc706873c5b..7184f1d07f90 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 object name list: 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 object name list: 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 object name list: 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 Wed Apr 10 16:24:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 10894341 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 3B1B8139A for ; Wed, 10 Apr 2019 16:24:35 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 236DB1FF28 for ; Wed, 10 Apr 2019 16:24:35 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2195028BBC; Wed, 10 Apr 2019 16:24:35 +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 A55B828C0E for ; Wed, 10 Apr 2019 16:24:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730084AbfDJQYd (ORCPT ); Wed, 10 Apr 2019 12:24:33 -0400 Received: from mail-qt1-f201.google.com ([209.85.160.201]:35479 "EHLO mail-qt1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729881AbfDJQYd (ORCPT ); Wed, 10 Apr 2019 12:24:33 -0400 Received: by mail-qt1-f201.google.com with SMTP id c28so1617361qtd.2 for ; Wed, 10 Apr 2019 09:24:32 -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=ZEjbmaQhMa1PUBfSaF2qR0gA/5T880d3kpY/s4VibMk=; b=O5LuHjizgsHkeo69fEO6Wq2IxiThc+UkFYIN0fVdZiBJq7EkzAw5hhMNx4kiudapLt XQAGh/cSE654nSR/qqoP1G2DrBh1aDU2cFYqCOIgZobKuQPLekzY1Utt0vfdfP9iOybe iHPZOHsF7B2cksWca1IZXuGJMnujpjYg5rz/7MxHQn8QJ8dJPcD79POjlWHZirwvvdyR IAg9ckdgD7TrGMg3y8Vx4DlDaxvoTwDl0zSz+BBc4endWkuYbP0nuw/BgdnnmVBViTeT 6KXMdsQPKme3GkOeGEJsmxjSmp0xzdNrIrlyhfoQswrel9nTJ+UQGuNzFic+73R0sPWw GNmA== 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=ZEjbmaQhMa1PUBfSaF2qR0gA/5T880d3kpY/s4VibMk=; b=J/eyhhtEk9Ym4io5pxwQP9ThDgs/aE7djMWO8VHL2erVtrm08opnX1yRefTtOeqVPw lCSh0liHPlnfEK38vVgWiRLrvt/jggaFhZ3lyAfH91QvFzbXC4nmwQZpptyx6tPvwVdy dtdDBl6xI5Kum4KFtQkf9Q0lwbvUOYnmRwkoieASgHg4xNjW2Sk5Tv6R2t7NlUg4gDCG wQho/jOzywfOf3yoe2rAOvEIJBrL4epa91KTeytQYkTzWvjvaqGYsNUsByB3mwQY7ODk IbOnaYWzeQRWu2jhwOTVxruytjf2VMZZU9FLhYSaZ2mKvUIAG6Y34I6IbE0QVh1YDABT pK7A== X-Gm-Message-State: APjAAAWAzM74+JTPJ870hjhBuAiAADWPmcvCYXxSPWETujB50RQA4Jmz X0ucQyCnU1TGSu3G47HZ3TsIvdU7VOjCazK1t0MSYUn5cK6e3mnfLytuIA5UfzPKg6Zi5z0iOC2 rMsQ6ZUwmnIMgNbkS+aGqPyDZEo5mKYal7njek22oCTxLcHZfU0VJ X-Google-Smtp-Source: APXvYqz+rgL3QqDmmVEy6/vXFrl/ykDURfpPhioSvIoueFfcLaEmDhIaS5H6rSEpA6h12WCK1ZnhCRET X-Received: by 2002:a37:6903:: with SMTP id e3mr5522881qkc.48.1554913472102; Wed, 10 Apr 2019 09:24:32 -0700 (PDT) Date: Wed, 10 Apr 2019 12:24:05 -0400 In-Reply-To: <20190410162409.117264-1-brho@google.com> Message-Id: <20190410162409.117264-3-brho@google.com> Mime-Version: 1.0 References: <20190410162409.117264-1-brho@google.com> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog Subject: [PATCH v6 2/6] 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 5c07dec19035..9555a9420836 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 Wed Apr 10 16:24:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 10894343 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 F05F0139A for ; Wed, 10 Apr 2019 16:24:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D2F5728B94 for ; Wed, 10 Apr 2019 16:24:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D06B828BEE; Wed, 10 Apr 2019 16:24:40 +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 CA8F328B37 for ; Wed, 10 Apr 2019 16:24:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731154AbfDJQYh (ORCPT ); Wed, 10 Apr 2019 12:24:37 -0400 Received: from mail-pg1-f201.google.com ([209.85.215.201]:34575 "EHLO mail-pg1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729881AbfDJQYh (ORCPT ); Wed, 10 Apr 2019 12:24:37 -0400 Received: by mail-pg1-f201.google.com with SMTP id z7so2331231pgc.1 for ; Wed, 10 Apr 2019 09:24:36 -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=w5siy1FrpxLy+ZgzfoS/UUjqkgFDXj/nJ7T68iE9KT8=; b=eRBVeUppcWpHs2x2Rs2+SodhwtLoqAEAzHXpTcWyogH3bw51pDxuKF4wKikRHtOB7k 05R8UU4D/bo796BW9p2L90A0BKl9+RfhaRqgOrZJMq+U8WigHa/TSmEut1cBlEla4FZA arrngvlECU8SQxCZfNDoIJvqgAAnWYP877gj8DqfrEWfA0S5BF2NC5AfbWHCMzLtTLlA Omld3jhEP9vf92QN0KuMB0s1HbmboxFtT091DNEHt/PpyDigI6YdrG2ld9xCIQ6VdjIx l2yyi7xs4k8HG/C5dHj/4U5xW9mOvUdwyFUQJ6ffPREygbs0E3ELxhX2epyETRNmqZmN gTKA== 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=w5siy1FrpxLy+ZgzfoS/UUjqkgFDXj/nJ7T68iE9KT8=; b=s/40vjhxz7vSPREm4fHnDSbbIDu08T8iAE/CCNId1R2x9zw+RmxatounFbpeuaOWt5 zjAm904IVPzSo0iK6lIq7wqk1Y4n+gPLz1Vxjfykb/xDDvh3g9gOmCv0EfLmQVypQFLl 6pjXxXmuTgyiI7+hQwsAX7i9x1CA3LvS/8m7qMVGCVpAl2kbqUH3VglI4g543bcqZVVd IY7qCaLAtCo6IphBlp1uSOl7jKHlNG5sEsQcHc6m3Pv9ML33udRI3m1BYVK44W+jZuS6 71zciMUZdDZjqwyALqej3e5aBd0ggqKiXINaimmVC9/5lJKVIY1Shi/mWRy6YmMhtluh eH3Q== X-Gm-Message-State: APjAAAXoj9uZhz/pA2xyalvw2gprcIluv0YJ9B9zr13ONAzKeEzFcdxN p5LOOMUSSFfolMkafM/ofEYZGv3VYBv3+GB+miNjm2N4r2WFBLeRyskCn4tqzOjdNPmNFPH0UjU QUQ+eOyePB9xL85xrxFfYtySUVtZqJ/vGYBiCIYwAEw0edVU6xBGo X-Google-Smtp-Source: APXvYqyaO5Bk/yuSHAO8IK6KsmdE2jpqzSTWaUAG5AUEQOWjwH13Jx1SBZT9Qk0x+1B9fnYyAOh8BB1s X-Received: by 2002:a65:60cc:: with SMTP id r12mr954934pgv.108.1554913475963; Wed, 10 Apr 2019 09:24:35 -0700 (PDT) Date: Wed, 10 Apr 2019 12:24:06 -0400 In-Reply-To: <20190410162409.117264-1-brho@google.com> Message-Id: <20190410162409.117264-4-brho@google.com> Mime-Version: 1.0 References: <20190410162409.117264-1-brho@google.com> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog Subject: [PATCH v6 3/6] 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 optional 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 is marked as 'unblamable', and we output a hash of all zeros. 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); 00000000 13) void new_func_2(void *x, 00000000 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 | 181 ++++++++++++++++++++++++++++++-- blame.h | 3 + builtin/blame.c | 42 ++++++++ t/t8013-blame-ignore-revs.sh | 168 +++++++++++++++++++++++++++++ 7 files changed, 406 insertions(+), 10 deletions(-) create mode 100755 t/t8013-blame-ignore-revs.sh diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index dc41957afab2..8f155196c6fe 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`, one unabbreviated object name per line. + Whitespace and comments beginning with `#` are ignored. 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 9555a9420836..0bbb86ad5985 100644 --- a/blame.c +++ b/blame.c @@ -480,7 +480,8 @@ 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->unblamable == next->unblamable) { ent->num_lines += next->num_lines; ent->next = next->next; blame_origin_decref(next->suspect); @@ -732,6 +733,10 @@ static void split_overlap(struct blame_entry *split, int chunk_end_lno; memset(split, 0, sizeof(struct blame_entry [3])); + split[0].unblamable = e->unblamable; + split[1].unblamable = e->unblamable; + split[2].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 +857,7 @@ 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->unblamable = e->unblamable; n->lno = e->lno + len; n->s_lno = e->s_lno + len; n->num_lines = e->num_lines - len; @@ -860,6 +866,109 @@ 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_entry *e, + struct blame_origin *parent, + struct blame_origin *target, + int offset, int parent_slno, int parent_len, + struct blame_line_tracker *line_blames) +{ + int i, parent_idx; + + for (i = 0; i < e->num_lines; i++) { + parent_idx = e->s_lno + i + offset; + if (parent_slno <= parent_idx && + parent_idx < parent_slno + parent_len) { + line_blames[i].is_parent = 1; + line_blames[i].s_lno = parent_idx; + } else { + line_blames[i].is_parent = 0; + line_blames[i].s_lno = e->s_lno + i; + } + } +} + +/* + * 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 is made in a separate heuristic function. 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, + int offset, int parent_slno, int parent_len, + struct blame_entry **diffp, + struct blame_entry **ignoredp) +{ + struct blame_line_tracker *line_blames; + int entry_len, nr_lines, i; + + line_blames = xcalloc(sizeof(struct blame_line_tracker), + e->num_lines); + guess_line_blames(e, parent, target, offset, parent_slno, parent_len, + line_blames); + /* + * 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->unblamable = 1; + /* 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); + free(line_blames); +} + /* * Process one hunk from the patch between the current suspect for * blame_entry e and its parent. This first blames any unfinished @@ -869,13 +978,19 @@ 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; while (e && e->s_lno < tlno) { struct blame_entry *next = e->next; @@ -943,10 +1058,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, offset, + tlno + offset, parent_len, &diffp, + &ignoredp); + } else { + e->next = diffp; + diffp = e; + } e = next; } + 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 +1089,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 +1104,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 +1117,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 +1127,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 +1141,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 +1651,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 be3a895043e0..91664913d7c4 100644 --- a/blame.h +++ b/blame.h @@ -92,6 +92,7 @@ struct blame_entry { * scanning the lines over and over. */ unsigned score; + int unblamable; }; /* @@ -117,6 +118,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 177c1022a0c4..b48842d8459b 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -52,6 +52,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; @@ -346,6 +347,8 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent, char hex[GIT_MAX_HEXSZ + 1]; oid_to_hex_r(hex, &suspect->commit->object.oid); + if (ent->unblamable) + memset(hex, '0', strlen(hex)); printf("%s %d %d %d\n", hex, ent->s_lno + 1, @@ -479,6 +482,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int } } + if (ent->unblamable) + memset(hex, '0', length); printf("%.*s", length, hex); if (opt & OUTPUT_ANNOTATE_COMPAT) { const char *name; @@ -695,6 +700,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"), @@ -774,6 +789,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; @@ -785,6 +821,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; @@ -806,6 +843,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), @@ -999,6 +1038,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..df4993f98682 --- /dev/null +++ b/t/t8013-blame-ignore-revs.sh @@ -0,0 +1,168 @@ +#!/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 "^[0-9a-f]\+ [0-9]\+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse X >expect && + test_cmp expect actual && + + grep "^[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 "^[0-9a-f]\+ [0-9]\+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse A >expect && + test_cmp expect actual && + + grep "^[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, blame those lines on an +# all-zeros rev. +# 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 >y_rev && + sed -e "s/[0-9a-f]/0/g" y_rev >expect && + git blame --line-porcelain file --ignore-rev Y >blame_raw && + + grep "^[0-9a-f]\+ [0-9]\+ 3" blame_raw | sed -e "s/ .*//" >actual && + test_cmp expect actual && + + grep "^[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 "^[0-9a-f]\+ [0-9]\+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse A >expect && + test_cmp expect actual && + + grep "^[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 "^[0-9a-f]\+ [0-9]\+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse A >expect && + test_cmp expect actual && + + grep "^[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 "^[0-9a-f]\+ [0-9]\+ 1" blame_raw | sed -e "s/ .*//" >actual && + test_cmp expect actual && + + grep "^[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 object name list: 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 + ' + +# 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 "^[0-9a-f]\+ [0-9]\+ 1" blame_raw | sed -e "s/ .*//" >actual && + git rev-parse B >expect && + test_cmp expect actual && + + grep "^[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 Wed Apr 10 16:24:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 10894345 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 9A72C17E0 for ; Wed, 10 Apr 2019 16:24:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7DB88289AD for ; Wed, 10 Apr 2019 16:24:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7C22E28AD6; Wed, 10 Apr 2019 16:24:43 +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 9794528AFF for ; Wed, 10 Apr 2019 16:24:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731300AbfDJQYl (ORCPT ); Wed, 10 Apr 2019 12:24:41 -0400 Received: from mail-yw1-f74.google.com ([209.85.161.74]:36409 "EHLO mail-yw1-f74.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729881AbfDJQYl (ORCPT ); Wed, 10 Apr 2019 12:24:41 -0400 Received: by mail-yw1-f74.google.com with SMTP id j62so2263217ywe.3 for ; Wed, 10 Apr 2019 09:24: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=QLlXN3I+bp5KCEJluQJtM8Yn9PL6gAMSYHPWxkqoTfQ=; b=Ce7xBj+hKsG8a9izSuXnmp7jrNewzi534esvmt2iUNndMhfPOdSAMQfllIEeRJ160z ayonQQmwl+xnjIGMXXKkzgHuf+cyLvHfl+Q1bvhus+7FxtOjwAvnJs6bra6lTW9rFpVr DXzVJStVng3S94zuFmiX/ybpSi6z4NS7cozT/NrC6RmiKjpBMjjBHmrFpo9nuBtqJjYW HMhjaUUxDIX2PoOPoUndAwC7gkYqavkedHuboJGcb7AQAIrrj7a2+PSw+MBJaunRgqhd YU2zPwS8OR5VuCBimd0xZTqwgthIPSv9xFDA/tVESkNKZWlP3WRwdVZVLl+wic9LKOfM nKEg== 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=QLlXN3I+bp5KCEJluQJtM8Yn9PL6gAMSYHPWxkqoTfQ=; b=BuGQByCqml0Sq35pGXx2tmmyaPJran4WXyj1x98cME1//IM/D9cdKKur6fzqUsuNUb /Ut4cl+FccZz3J5/3d9V26tBkBCUOwAECjGNPXnroCtSjQP1X1KGHa/jn80lghO2ji3Z Hfo01vvAmQP4qEAK6mEVSAQPWaAITaKpm9r0rCMTBLS+VxFda9xvHzg9LCDbrMIO97RD GHJ+JLRXuwwpfLO79iwEyH9dMqCLvdd0SmxzqgRXb2LboI5sOUknQFJcLbK9pXDaASGp cNu3CuPiaKQRA0eJw9fn4jTEHh7fz7ExSQEqAoJ2Kbq0jJnZe7QUOVYm3fm0QxaSuDWb fM0A== X-Gm-Message-State: APjAAAUobB3JFSZ+waShL8y7G9+u/d5GoygpWKxJBilQJWnvJc7jCh1L hQ8hu4CWaJPe2CnpvgluY1cl4bWWua6JiprEf0EKojKjUDWbDASjCTrgutaML3dchPkvKR9jmu+ TxlsN2G5rZEIOnmTwf/Ad30NOFLmGgjTzKP09SqcdIwE2wNVBCjWx X-Google-Smtp-Source: APXvYqxsz1PF3nrblFMgz8NQ9V4eJFnzr2Qw9qN2/nJU2L+jg83jlzNH9M09GmRo14/LBdRbMB9qR8UK X-Received: by 2002:a25:6a08:: with SMTP id f8mr4442424ybc.3.1554913480062; Wed, 10 Apr 2019 09:24:40 -0700 (PDT) Date: Wed, 10 Apr 2019 12:24:07 -0400 In-Reply-To: <20190410162409.117264-1-brho@google.com> Message-Id: <20190410162409.117264-5-brho@google.com> Mime-Version: 1.0 References: <20190410162409.117264-1-brho@google.com> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog Subject: [PATCH v6 4/6] blame: add config options to handle output for 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 When ignoring commits, the commit that is blamed might not be responsible for the change. Users might want to know when a particular line has a potentially inaccurate blame. Furthermore, they might never want to see the object hash of an ignored commit. This patch adds two config options to control the output behavior. The first option can identify ignored lines by specifying blame.markIgnoredFiles. When this option is set, each blame line is marked with an '*'. 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 commit touched a line. These lines are 'unblamable.' The second option, blame.maskIgnoredUnblamables, will zero the hash of any unblamable line. For example, say we ignore e5e8d36d04cbe: e5e8d36d04cbe (Barret Rhoden 2016-04-11 13:57:54 -0400 26) appears as: 0000000000000 (Barret Rhoden 2016-04-11 13:57:54 -0400 26) Signed-off-by: Barret Rhoden --- Documentation/blame-options.txt | 6 +++++- Documentation/config/blame.txt | 9 +++++++++ blame.c | 4 ++++ blame.h | 1 + builtin/blame.c | 18 +++++++++++++++-- t/t8013-blame-ignore-revs.sh | 34 +++++++++++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 3 deletions(-) diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt index 8f155196c6fe..e7b8b5e4b87b 100644 --- a/Documentation/blame-options.txt +++ b/Documentation/blame-options.txt @@ -115,7 +115,11 @@ 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 will be + marked with a `*` in the blame output. If the + `blame.maskIgnoredUnblamables` config option is set, then those lines that + we could not attribute to another revision are outputted as all zeros. --ignore-revs-file :: Ignore revisions listed in `file`, one unabbreviated object name per line. diff --git a/Documentation/config/blame.txt b/Documentation/config/blame.txt index 4da2788f306d..bb6674227da1 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.maskIgnoredUnblamables:: + Output an object hash of all zeros for lines that were changed by an ignored + revision and that we could not attribute to another revision in the output + of linkgit:git-blame[1]. + +blame.markIgnoredLines:: + Mark lines that were changed by an ignored revision with a '*' in the + output of linkgit:git-blame[1]. diff --git a/blame.c b/blame.c index 0bbb86ad5985..a98ae00e2cfc 100644 --- a/blame.c +++ b/blame.c @@ -481,6 +481,7 @@ void blame_coalesce(struct blame_scoreboard *sb) for (ent = sb->ent; ent && (next = ent->next); ent = next) { if (ent->suspect == next->suspect && ent->s_lno + ent->num_lines == next->s_lno && + ent->ignored == next->ignored && ent->unblamable == next->unblamable) { ent->num_lines += next->num_lines; ent->next = next->next; @@ -733,6 +734,7 @@ static void split_overlap(struct blame_entry *split, int chunk_end_lno; memset(split, 0, sizeof(struct blame_entry [3])); + split[0].ignored = split[1].ignored = split[2].ignored = e->ignored; split[0].unblamable = e->unblamable; split[1].unblamable = e->unblamable; split[2].unblamable = e->unblamable; @@ -857,6 +859,7 @@ 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; @@ -922,6 +925,7 @@ static void ignore_blame_entry(struct blame_entry *e, struct blame_line_tracker *line_blames; int entry_len, nr_lines, i; + e->ignored = 1; line_blames = xcalloc(sizeof(struct blame_line_tracker), e->num_lines); guess_line_blames(e, parent, target, offset, parent_slno, parent_len, diff --git a/blame.h b/blame.h index 91664913d7c4..53df8b4c5b3f 100644 --- a/blame.h +++ b/blame.h @@ -92,6 +92,7 @@ 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 b48842d8459b..c10a6a802240 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -53,6 +53,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 mask_ignored_unblamables; +static int mark_ignored_lines; static struct date_mode blame_date_mode = { DATE_ISO8601 }; static size_t blame_date_width; @@ -347,7 +349,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent, char hex[GIT_MAX_HEXSZ + 1]; oid_to_hex_r(hex, &suspect->commit->object.oid); - if (ent->unblamable) + if (mask_ignored_unblamables && ent->unblamable) memset(hex, '0', strlen(hex)); printf("%s %d %d %d\n", hex, @@ -482,7 +484,11 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int } } - if (ent->unblamable) + if (mark_ignored_lines && ent->ignored) { + length--; + putchar('*'); + } + if (mask_ignored_unblamables && ent->unblamable) memset(hex, '0', length); printf("%.*s", length, hex); if (opt & OUTPUT_ANNOTATE_COMPAT) { @@ -710,6 +716,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.maskignoredunblamables")) { + mask_ignored_unblamables = 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 df4993f98682..cc049a390b0d 100755 --- a/t/t8013-blame-ignore-revs.sh +++ b/t/t8013-blame-ignore-revs.sh @@ -53,6 +53,7 @@ test_expect_success ignore_rev_changing_lines ' # 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 ' + git config --add blame.maskIgnoredUnblamables true && test_write_lines line-one-change line-two-changed y3 y4 >file && git add file && test_tick && @@ -123,6 +124,39 @@ test_expect_success bad_files_and_revs ' test_i18ngrep "Invalid object name: NOREV" err ' +# 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 + ' + # Resetting the repo and creating: # # A--B--M From patchwork Wed Apr 10 16:24:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 10894347 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 26F1B139A for ; Wed, 10 Apr 2019 16:24:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0A92928757 for ; Wed, 10 Apr 2019 16:24:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 07B1228AFF; Wed, 10 Apr 2019 16:24: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 1460C28B75 for ; Wed, 10 Apr 2019 16:24:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731312AbfDJQYp (ORCPT ); Wed, 10 Apr 2019 12:24:45 -0400 Received: from mail-qk1-f202.google.com ([209.85.222.202]:52520 "EHLO mail-qk1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729881AbfDJQYp (ORCPT ); Wed, 10 Apr 2019 12:24:45 -0400 Received: by mail-qk1-f202.google.com with SMTP id x23so2423470qka.19 for ; Wed, 10 Apr 2019 09:24:44 -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=JATumrRH3zrfzZ4CefAXCV/IL4oBPJjDDLxsdSLTfRI=; b=IJAEgWktu76pZ0MFObhUr0sVCtNdD+D3X1T2nAW2pU+jVYgU7yD4srCw3pOOzq9zV1 P8aQhis/kH6q0VuU55DBpG81ncgAraz2Gn6y+kYrRFdZtQarNhUz1GzNi7QLMZBhTx6l IgdaUPfx5LhT2rLYhAjLc8Ow9fBEEwt4Z6OKM5J3chm29YLHvCQNKMLhCAAE8ic+ELD3 O/NStPzbmIq4HeTN4NwSU8/x25lvXEz3YN/qDi3TibkIZC8VvQbaUbI7AiQPTZqFaDFZ 4HvTJ/YB+SYv9jlbTPHASZzpdpOpth0/9d3K4tzK/JiQE2gYsQkVbQ2hyaXbQSPINRyi VJlw== 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=JATumrRH3zrfzZ4CefAXCV/IL4oBPJjDDLxsdSLTfRI=; b=Cro+txUOi8QErx7FE9eP+auOIJ3XJtdLpxAjttYJYoe2m1rl00lGbU7Ck8zURjxmmm q1y2NgwtBj0sFaUt2kW8v01aJKWYE9xMuGdtI0rfjScTG7SzVd69yWJsWPHP+ELhYeh8 hAwzfY4H0P8h0O1GvJi8WImtX2K2EQzzWhYl9OOxs1Am/WB9g6dKdXImttHmrduOWNNT hmq1oen5gbLUu5pQwRGGSW6LPnTA7bTB62AFL8X03Frt6oKdYQ+YJ0I358d+DSb7y6Mk eZ2APRTD46C1DpCtIPtSdxXfvGISqYiINmLZpBkShjrn91FeqrUjQ6uTS0gfmxp2u6DK wS0A== X-Gm-Message-State: APjAAAWEjJRr2n5IKH+SWYnQOzgJPO4FW6dEplzpQwCS9XKi8aYanPLe 6V9aSwkr6dbMxpU+pUji3DEkhsnnGftiDSDpm1GyBUIG/SIlmC2O2vvXP/hIl0GQYplZag5t+Au IuvTnMJ4g/dOP4+sdKW6Iv6ImUJSwpMLv8ieZIDxvA4/uwsvkvI8l X-Google-Smtp-Source: APXvYqxPrKZklBOypRhBSuQBf9JCs6rqszKp3JqEPcfHLzaHe0jAYrWhJADlFteSN+6t3JtIIRJwn7fO X-Received: by 2002:ac8:3798:: with SMTP id d24mr5783536qtc.40.1554913484197; Wed, 10 Apr 2019 09:24:44 -0700 (PDT) Date: Wed, 10 Apr 2019 12:24:08 -0400 In-Reply-To: <20190410162409.117264-1-brho@google.com> Message-Id: <20190410162409.117264-6-brho@google.com> Mime-Version: 1.0 References: <20190410162409.117264-1-brho@google.com> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog Subject: [PATCH v6 5/6] 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 | 95 +++++++++++++++++++++++++++++++++++++++------------------ blame.h | 2 ++ 2 files changed, 67 insertions(+), 30 deletions(-) diff --git a/blame.c b/blame.c index a98ae00e2cfc..a42dff80b1a5 100644 --- a/blame.c +++ b/blame.c @@ -311,12 +311,63 @@ 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 */ + o->fingerprints = xcalloc(sizeof(struct fingerprint), o->num_lines); + free(line_starts); +} + +static void drop_origin_fingerprints(struct blame_origin *o) +{ + if (o->fingerprints) { + o->num_lines = 0; + FREE_AND_NULL(o->fingerprints); + } +} + /* * 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 +391,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); } /* @@ -1136,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)) @@ -1348,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; @@ -1477,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; @@ -1816,37 +1874,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 53df8b4c5b3f..5dd877bb78fc 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 mode; /* guilty gets set when shipping any suspects to the final From patchwork Wed Apr 10 16:24:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Barret Rhoden X-Patchwork-Id: 10894349 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 970C0139A for ; Wed, 10 Apr 2019 16:24:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 793D928B6E for ; Wed, 10 Apr 2019 16:24:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7735E28B54; Wed, 10 Apr 2019 16:24:51 +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 9F9C828AC4 for ; Wed, 10 Apr 2019 16:24:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731368AbfDJQYt (ORCPT ); Wed, 10 Apr 2019 12:24:49 -0400 Received: from mail-qk1-f202.google.com ([209.85.222.202]:52521 "EHLO mail-qk1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729881AbfDJQYt (ORCPT ); Wed, 10 Apr 2019 12:24:49 -0400 Received: by mail-qk1-f202.google.com with SMTP id x23so2423707qka.19 for ; Wed, 10 Apr 2019 09:24:48 -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=JYb7GTkCy2YVPCJLwzABj8bSrnvdxCn4t/H5qLhv6qo=; b=ry1ZGo8ljkmDwFLbmWyQZge8kEZFi/xPtgZuCRSlXLOg7PUG0qfiRT70K6kVI4OGYh NSaNtYcHRVxBwZG+RhjQm77SIX8sWQghku+M8zGOEb2z/yjPwLXzH3rseOe0aDfFPZqj jeRqN2pf6XXMQYAME/h7nb/KpCdbVVmDAjUIqwGtBAbDWtbjZdNhejTKOKe2oTDdHC1z CXbPe7JISctzvm5CU4HPEz5ro5PQ3vHZA8x0lMJA40KxhV06n4XVtbGh2ddQVu7eMAOl 0YIt1RH345JVyToNTaUTqtDwcxaXvkm6CdsfvNu5OZLECl0WZgctwMd1NG9OdU56whTd U61A== 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=JYb7GTkCy2YVPCJLwzABj8bSrnvdxCn4t/H5qLhv6qo=; b=GTZ/DW4/rr7hTAa+QNORocTM2ySiIKVqh4AavqZckMyedFERZGinEaGoIgfoZX9xc8 dZ1OoccboU6m7Ksk9xVZmUSPI2O68zviDEWu9nxhfkAk/K6ReuEFG4vuFbAQMm5I0+S5 Cj9VpyH9DXiAQzS9DGXC1FO/79PvLZpM7Yi6WcaQtm+WmexN2h/mZ2LE6fdDIAWjmK4B iDVW1Dswimx6ODMybOJSC/il3xbVrqkWWRgNu0QqEFFYUuu66zA+GMNnCz0ZteEJxvWZ L15CIVW9ege+WxxYFP9+mibXAI76SepZgRpcTqEIm6IEcolJJy4czJQ6Q6x82XXNt/0g su9A== X-Gm-Message-State: APjAAAUG342h3CL5EO3UCtUrKDivykLSnZf3XeSQiGxzySJBOOUX0bao Vmc7cUMvVVpsgCCDKKgw/JOUqZOVevehC9fJQNH84X2TEht+tK5OOi34XMvGLOD/9Y3C4sDZhIz R/1n2xhXnbcyQgesgXwQT5a4K2AmrUBaUns2mqcGYUnE+VzYqZNAL X-Google-Smtp-Source: APXvYqzovj3eVrYvmsknGceZUjsFlgDtcN+dklRtkQjR1dDJ+8zYktHvK3yCJogmN+3WV40p152g9QWy X-Received: by 2002:a37:a256:: with SMTP id l83mr5627446qke.14.1554913488153; Wed, 10 Apr 2019 09:24:48 -0700 (PDT) Date: Wed, 10 Apr 2019 12:24:09 -0400 In-Reply-To: <20190410162409.117264-1-brho@google.com> Message-Id: <20190410162409.117264-7-brho@google.com> Mime-Version: 1.0 References: <20190410162409.117264-1-brho@google.com> X-Mailer: git-send-email 2.21.0.392.gf8f6787159e-goog Subject: [PATCH v6 6/6] blame: use 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 This replaces 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 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 fingerprint code and the idea to use them for blame came from Michael Platings . For each line changed in the target, i.e. in a blame_entry touched by a target's diff, guess_line_blames() finds the best line in the parent, above a magic threshold. Ties are broken by proximity of the parent line number to the target's line. We actually make two passes. The first pass checks in the diff chunk associated with the blame entry - specifically from blame_chunk(). Often times, those diff chunks are small; any 'context' in a normal diff chunk is broken up into multiple calls to blame_chunk(). We make a second pass over the entire parent, with a slightly higher threshold. Here's an example of the difference the fingerprinting makes. Consider a file with four commits: 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 When we blame-ignored with the old algorithm, we get: commit-a 11) void new_func_1(void *x, commit-b 12) void *y); 00000000 13) void new_func_2(void *x, 00000000 14) void *y); commit-c 15) some_line_c commit-d 16) some_line_d Where commit-b is blamed for 12 instead of 13. With the fingerprint algorithm, we get: commit-a 11) void new_func_1(void *x, commit-b 12) void *y); commit-b 13) void new_func_2(void *x, commit-b 14) void *y); commit-c 15) some_line_c commit-d 16) some_line_d Note both lines 12 and 14 are given to commit b. Their match is above the FINGERPRINT_CHUNK_THRESHOLD, and they tied. Specifically, parent lines 11 and 12 both match these lines. The algorithm chose parent line 12, since that was closest to the target line numbers of 12 and 14. If we increase the threshold, say to 10, those two lines won't match, and will be treated as 'unblamable.' 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 00000000 32) #include That's because commit X 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 Suggested-by: Michael Platings Signed-off-by: Barret Rhoden --- blame.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 131 insertions(+), 9 deletions(-) diff --git a/blame.c b/blame.c index a42dff80b1a5..da2d664b38af 100644 --- a/blame.c +++ b/blame.c @@ -340,6 +340,84 @@ static int find_line_starts(int **line_starts, const char *buf, return num; } +struct fingerprint { + struct hashmap map; + struct hashmap_entry *entries; +}; + +static void get_fingerprint(struct fingerprint *result, + const char *line_begin, + const char *line_end) +{ + unsigned int hash; + char c0, c1; + const char *p; + int map_entry_count = line_end - line_begin - 1; + struct hashmap_entry *entry = xcalloc(map_entry_count, + sizeof(struct hashmap_entry)); + + hashmap_init(&result->map, NULL, NULL, map_entry_count); + result->entries = entry; + for (p = line_begin; p + 1 < line_end; ++p, ++entry) { + c0 = *p; + c1 = *(p + 1); + /* Ignore whitespace pairs */ + if (isspace(c0) && isspace(c1)) + continue; + hash = tolower(c0) | (tolower(c1) << 8); + hashmap_entry_init(entry, hash); + hashmap_put(&result->map, entry); + } +} + +static void free_fingerprint(struct fingerprint *f) +{ + hashmap_free(&f->map, 0); + free(f->entries); +} + +static int fingerprint_similarity(struct fingerprint *a, + struct fingerprint *b) +{ + int intersection = 0; + struct hashmap_iter iter; + struct hashmap_entry *entry; + + hashmap_iter_init(&b->map, &iter); + + while ((entry = hashmap_iter_next(&iter))) { + if (hashmap_get(&a->map, entry, NULL)) + ++intersection; + } + return intersection; +} + +static void get_line_fingerprints(struct fingerprint *fingerprints, + const char *content, + const int *line_starts, + int first_line, + int nr_lines) +{ + int i; + + line_starts += first_line; + for (i = 0; i < nr_lines; ++i) { + const char *linestart = content + line_starts[i]; + const char *lineend = content + line_starts[i + 1]; + + get_fingerprint(fingerprints + i, linestart, lineend); + } +} + +static void free_chunk_fingerprints(struct fingerprint *fingerprints, + int nr_fingerprints) +{ + int i; + + for (i = 0; i < nr_fingerprints; i++) + free_fingerprint(&fingerprints[i]); +} + static void fill_origin_fingerprints(struct blame_origin *o, mmfile_t *file) { int *line_starts; @@ -348,14 +426,16 @@ 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_chunk_fingerprints(o->fingerprints, o->num_lines); o->num_lines = 0; FREE_AND_NULL(o->fingerprints); } @@ -935,27 +1015,69 @@ static int are_lines_adjacent(struct blame_line_tracker *first, first->s_lno + 1 == second->s_lno; } +static void scan_parent_range(struct fingerprint *p_fps, + struct fingerprint *t_fps, int t_idx, + int from, int nr_lines, + int *best_sim_val, int *best_sim_idx) +{ + int sim, p_idx; + + 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; + } +} + /* - * 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 CHUNK threshold is for how similar we must be within a diff chunk, which + * is typically the adjacent '-' and '+' sections in a diff, separated by the + * ' ' context. + * + * We have a greater threshold for similarity for lines in any part of the + * parent's file. If no line in the parent meets the appropriate threshold, + * then the blame_entry will stay with the target and be considered + * 'unblamable'. */ +#define FINGERPRINT_CHUNK_THRESHOLD 1 +#define FINGERPRINT_FILE_THRESHOLD 10 + static void guess_line_blames(struct blame_entry *e, struct blame_origin *parent, struct blame_origin *target, int offset, int parent_slno, int parent_len, struct blame_line_tracker *line_blames) { - int i, parent_idx; + int i, target_idx; for (i = 0; i < e->num_lines; i++) { - parent_idx = e->s_lno + i + offset; - if (parent_slno <= parent_idx && - parent_idx < parent_slno + parent_len) { + int best_val = FINGERPRINT_CHUNK_THRESHOLD; + int best_idx = -1; + + target_idx = e->s_lno + i; + scan_parent_range(parent->fingerprints, + target->fingerprints, target_idx, + parent_slno, parent_len, + &best_val, &best_idx); + if (best_idx == -1) { + best_val = FINGERPRINT_FILE_THRESHOLD; + scan_parent_range(parent->fingerprints, + target->fingerprints, target_idx, + 0, parent->num_lines, + &best_val, &best_idx); + } + if (best_idx >= 0) { line_blames[i].is_parent = 1; - line_blames[i].s_lno = parent_idx; + line_blames[i].s_lno = best_idx; } else { line_blames[i].is_parent = 0; - line_blames[i].s_lno = e->s_lno + i; + line_blames[i].s_lno = target_idx; } } }