From patchwork Fri Feb 22 06:20:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10825265 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 886F6922 for ; Fri, 22 Feb 2019 06:20:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 753BB30D15 for ; Fri, 22 Feb 2019 06:20:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6785330D2E; Fri, 22 Feb 2019 06:20:41 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI 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 F000D30D15 for ; Fri, 22 Feb 2019 06:20:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725906AbfBVGUj (ORCPT ); Fri, 22 Feb 2019 01:20:39 -0500 Received: from cloud.peff.net ([104.130.231.41]:53834 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1725873AbfBVGUj (ORCPT ); Fri, 22 Feb 2019 01:20:39 -0500 Received: (qmail 29146 invoked by uid 109); 22 Feb 2019 06:20:40 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Fri, 22 Feb 2019 06:20:40 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 19697 invoked by uid 111); 22 Feb 2019 06:20:52 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Fri, 22 Feb 2019 01:20:52 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Fri, 22 Feb 2019 01:20:37 -0500 Date: Fri, 22 Feb 2019 01:20:37 -0500 From: Jeff King To: Junio C Hamano Cc: Christian Couder , Bartosz Baranowski , git@vger.kernel.org Subject: [PATCH 1/3] bisect: use string arguments to feed internal diff-tree Message-ID: <20190222062037.GA10248@sigill.intra.peff.net> References: <20190222061949.GA9875@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190222061949.GA9875@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Commit e22278c0a0 (bisect: display first bad commit without forking a new process, 2009-05-28) converted our external call to diff-tree to an internal use of the log_tree_commit(). But rather than individually setting options in the rev_info struct (and explaining in comments how they map to command-line options), we can just pass the command-line options to setup_revisions(). This is shorter, easier to change, and less likely to break if revision.c internals change. Note that we unconditionally set the output format to "raw". The conditional in the original code didn't actually do anything useful, since nobody had an opportunity to set the format to anything. Signed-off-by: Jeff King --- bisect.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/bisect.c b/bisect.c index 3af955c4bc..8c81859835 100644 --- a/bisect.c +++ b/bisect.c @@ -896,24 +896,15 @@ static void show_diff_tree(struct repository *r, const char *prefix, struct commit *commit) { + const char *argv[] = { + "diff-tree", "--pretty", "--no-abbrev", "--raw", NULL + }; struct rev_info opt; - /* diff-tree init */ repo_init_revisions(r, &opt, prefix); git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ - opt.abbrev = 0; - opt.diff = 1; - /* This is what "--pretty" does */ - opt.verbose_header = 1; - opt.use_terminator = 0; - opt.commit_format = CMIT_FMT_DEFAULT; - - /* diff-tree init */ - if (!opt.diffopt.output_format) - opt.diffopt.output_format = DIFF_FORMAT_RAW; - - setup_revisions(0, NULL, &opt, NULL); + setup_revisions(ARRAY_SIZE(argv) - 1, argv, &opt, NULL); log_tree_commit(&opt, commit); } From patchwork Fri Feb 22 06:21:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10825267 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 7038C1390 for ; Fri, 22 Feb 2019 06:21:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5CD8C30CFE for ; Fri, 22 Feb 2019 06:21:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4D85530D15; Fri, 22 Feb 2019 06:21:37 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI 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 ED19D30CFE for ; Fri, 22 Feb 2019 06:21:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725992AbfBVGVf (ORCPT ); Fri, 22 Feb 2019 01:21:35 -0500 Received: from cloud.peff.net ([104.130.231.41]:53846 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1725868AbfBVGVf (ORCPT ); Fri, 22 Feb 2019 01:21:35 -0500 Received: (qmail 29172 invoked by uid 109); 22 Feb 2019 06:21:36 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Fri, 22 Feb 2019 06:21:36 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 19717 invoked by uid 111); 22 Feb 2019 06:21:48 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Fri, 22 Feb 2019 01:21:48 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Fri, 22 Feb 2019 01:21:33 -0500 Date: Fri, 22 Feb 2019 01:21:33 -0500 From: Jeff King To: Junio C Hamano Cc: Christian Couder , Bartosz Baranowski , git@vger.kernel.org Subject: [PATCH 2/3] bisect: fix internal diff-tree config loading Message-ID: <20190222062133.GB10248@sigill.intra.peff.net> References: <20190222061949.GA9875@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190222061949.GA9875@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When we run our internal diff-tree to show the bisected commit, we call init_revisions(), then load config, then setup_revisions(). But that order is wrong: we copy the configured defaults into the rev_info struct during the init_revisions step, so our config load wasn't actually doing anything. Signed-off-by: Jeff King --- It does feel a little weird loading config at all here, since it would potentially affect other in-process operations. This is where an external process might be cleaner. bisect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bisect.c b/bisect.c index 8c81859835..b04d7b2f63 100644 --- a/bisect.c +++ b/bisect.c @@ -901,8 +901,8 @@ static void show_diff_tree(struct repository *r, }; struct rev_info opt; - repo_init_revisions(r, &opt, prefix); git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ + repo_init_revisions(r, &opt, prefix); setup_revisions(ARRAY_SIZE(argv) - 1, argv, &opt, NULL); log_tree_commit(&opt, commit); From patchwork Fri Feb 22 06:23:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 10825271 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 05B7A922 for ; Fri, 22 Feb 2019 06:23:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E6BBB30D55 for ; Fri, 22 Feb 2019 06:23:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D9E5A30D68; Fri, 22 Feb 2019 06:23:32 +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=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI 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 C2CDC30D55 for ; Fri, 22 Feb 2019 06:23:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725882AbfBVGXa (ORCPT ); Fri, 22 Feb 2019 01:23:30 -0500 Received: from cloud.peff.net ([104.130.231.41]:53858 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1725848AbfBVGXa (ORCPT ); Fri, 22 Feb 2019 01:23:30 -0500 Received: (qmail 29201 invoked by uid 109); 22 Feb 2019 06:23:31 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with SMTP; Fri, 22 Feb 2019 06:23:31 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 19738 invoked by uid 111); 22 Feb 2019 06:23:43 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.94) with (ECDHE-RSA-AES256-GCM-SHA384 encrypted) SMTP; Fri, 22 Feb 2019 01:23:43 -0500 Authentication-Results: peff.net; auth=none Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Fri, 22 Feb 2019 01:23:28 -0500 Date: Fri, 22 Feb 2019 01:23:28 -0500 From: Jeff King To: Junio C Hamano Cc: Christian Couder , Bartosz Baranowski , git@vger.kernel.org Subject: [PATCH 3/3] bisect: make diff-tree output prettier Message-ID: <20190222062327.GC10248@sigill.intra.peff.net> References: <20190222061949.GA9875@sigill.intra.peff.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190222061949.GA9875@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP After completing a bisection, we print out the commit we found using an internal version of diff-tree. The result is aesthetically lacking: - it shows a raw diff, which is generally less informative for human readers than "--stat --summary" (which we already decided was nice for humans in format-patch's output). - by not abbreviating hashes, the result is likely to wrap on most people's terminals - we don't use "-r", so if the commit touched files in a directory, you only get to see the top-level directory mentioned - we don't specify "--cc" or similar, so merges print nothing (not even the commit message!) Even though bisect might be driven by scripts, there's no reason to consider this part of the output as machine-readable (if anything, the initial "$hash is the first bad commit" might be parsed, but we won't touch that here). Let's make it prettier and more informative for a human reading the output. While we're tweaking the options, let's also switch to using the diff "ui" config. If we're accepting that this is human-readable output, then we should respect the user's options for how to display it. Note that we have to touch a few tests in t6030. These check bisection in a corrupted repository (it's missing a subtree). They didn't fail with the previous code, because it didn't actually recurse far enough in the diff to find the broken tree. But now we'll see the corruption and complain. Adjusting the tests to expect the die() is the best fix. We still confirm that we're able to bisect within the broken repo. And we'll still print "$hash is the first bad commit" as usual before dying; showing that is a reasonable outcome in a corrupt repository (and was what might happen already, if the root tree was corrupt). Signed-off-by: Jeff King --- If we do care about the change in exit code from bisect, then it probably does make sense to go with an external process. Then it can happily die on the corruption, while bisect continues with the rest of the high-level operation. I'm not sure it really matters much, though. Once your repository is corrupted, all bets are off. It's nice that we can bisect in such a state at all. bisect.c | 4 ++-- t/t6030-bisect-porcelain.sh | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bisect.c b/bisect.c index b04d7b2f63..e87ac29a51 100644 --- a/bisect.c +++ b/bisect.c @@ -897,11 +897,11 @@ static void show_diff_tree(struct repository *r, struct commit *commit) { const char *argv[] = { - "diff-tree", "--pretty", "--no-abbrev", "--raw", NULL + "diff-tree", "--pretty", "--stat", "--summary", "--cc", NULL }; struct rev_info opt; - git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ + git_config(git_diff_ui_config, NULL); repo_init_revisions(r, &opt, prefix); setup_revisions(ARRAY_SIZE(argv) - 1, argv, &opt, NULL); diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 55835ee4a4..49a394bd75 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -681,7 +681,7 @@ test_expect_success 'bisect: --no-checkout - target in breakage' ' check_same BROKEN_HASH6 BISECT_HEAD && git bisect bad BISECT_HEAD && check_same BROKEN_HASH5 BISECT_HEAD && - git bisect good BISECT_HEAD && + test_must_fail git bisect good BISECT_HEAD && check_same BROKEN_HASH6 bisect/bad && git bisect reset ' @@ -692,7 +692,7 @@ test_expect_success 'bisect: --no-checkout - target after breakage' ' check_same BROKEN_HASH6 BISECT_HEAD && git bisect good BISECT_HEAD && check_same BROKEN_HASH8 BISECT_HEAD && - git bisect good BISECT_HEAD && + test_must_fail git bisect good BISECT_HEAD && check_same BROKEN_HASH9 bisect/bad && git bisect reset ' @@ -701,7 +701,7 @@ test_expect_success 'bisect: demonstrate identification of damage boundary' " git bisect reset && git checkout broken && git bisect start broken master --no-checkout && - git bisect run \"\$SHELL_PATH\" -c ' + test_must_fail git bisect run \"\$SHELL_PATH\" -c ' GOOD=\$(git for-each-ref \"--format=%(objectname)\" refs/bisect/good-*) && git rev-list --objects BISECT_HEAD --not \$GOOD >tmp.\$\$ && git pack-objects --stdout >/dev/null < tmp.\$\$