From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Christian Couder <christian.couder@gmail.com>,
Bartosz Baranowski <bbaranow@redhat.com>,
git@vger.kernel.org
Subject: [PATCH 3/3] bisect: make diff-tree output prettier
Date: Fri, 22 Feb 2019 01:23:28 -0500 [thread overview]
Message-ID: <20190222062327.GC10248@sigill.intra.peff.net> (raw)
In-Reply-To: <20190222061949.GA9875@sigill.intra.peff.net>
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 <peff@peff.net>
---
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.\$\$
--
2.21.0.rc2.577.g06bbe9cbd1
next prev parent reply other threads:[~2019-02-22 6:23 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-21 10:47 git bisect - good vs bad output is different Bartosz Baranowski
2019-02-21 21:39 ` Junio C Hamano
2019-02-22 6:19 ` [PATCH 0/3] prettier bisect output Jeff King
2019-02-22 6:20 ` [PATCH 1/3] bisect: use string arguments to feed internal diff-tree Jeff King
2019-02-22 6:21 ` [PATCH 2/3] bisect: fix internal diff-tree config loading Jeff King
2019-03-03 17:59 ` Christian Couder
2019-03-05 4:15 ` Jeff King
2019-02-22 6:23 ` Jeff King [this message]
2019-02-22 17:49 ` [PATCH 3/3] bisect: make diff-tree output prettier Junio C Hamano
2019-02-23 13:44 ` Jeff King
2019-03-03 18:25 ` Christian Couder
2019-02-22 17:39 ` [PATCH 0/3] prettier bisect output Junio C Hamano
2019-03-03 18:33 ` Christian Couder
2019-03-05 4:16 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190222062327.GC10248@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=bbaranow@redhat.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).