From b0082b9d5918e3548c987303a19de0d69bbcabaa Mon Sep 17 00:00:00 2001 From: Thomas Rast Date: Tue, 19 Jun 2012 22:04:57 +0200 Subject: [PATCH 1/2] Demonstrate git-show is broken with ranges The logic of git-show has remained largely unchanged since around 5d7eeee (git-show: grok blobs, trees and tags, too, 2006-12-14): start a revision walker with no_walk=1, look at its pending objects and handle them one-by-one. For commits, this means stuffing them into a new queue all alone, and running the walker. Then Linus's f222abd (Make 'git show' more useful, 2009-07-13) came along and set no_walk=0 whenever the user specifies a range. Which appears to work fine, until you actually prod it hard enough, as the preceding commit shows: UNINTERESTING commits will be marked as such, but not walked further to propagate the marks. Demonstrate this with the main tests of this patch: 'showing a range walks (Y shape)'. The Y shape of history ensures that propagating the UNINTERESTING marks is necessary to correctly exclude the main1 commit. The only example I could find actually requires that the negative revisions are listed later, and in this scenario a dotted range actually works. However, it is easy to find examples in git.git where a dotted range is wrong, e.g. $ git show v1.7.0..v1.7.1 | grep ^commit | wc -l 1297 $ git rev-list v1.7.0..v1.7.1 | wc -l 702 While there, also test a few other things that are not covered so far: the -N way of triggering a range (added in 5853cae, DWIM 'git show -5' to 'git show --do-walk -5', 2010-06-01), and the interactions of tags, commits and ranges. Pointed out by Dr_Memory on #git. Signed-off-by: Thomas Rast Signed-off-by: Junio C Hamano --- t/t7007-show.sh | 91 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/t/t7007-show.sh b/t/t7007-show.sh index cce222f052..891b143e0c 100755 --- a/t/t7007-show.sh +++ b/t/t7007-show.sh @@ -17,4 +17,95 @@ test_expect_success 'showing a tag that point at a missing object' ' test_must_fail git --no-pager show foo-tag ' +test_expect_success 'set up a bit of history' ' + test_commit main1 && + test_commit main2 && + test_commit main3 && + git tag -m "annotated tag" annotated && + git checkout -b side HEAD^^ && + test_commit side2 && + test_commit side3 +' + +test_expect_success 'showing two commits' ' + cat >expect <<-EOF && + commit $(git rev-parse main2) + commit $(git rev-parse main3) + EOF + git show main2 main3 >actual && + grep ^commit actual >actual.filtered && + test_cmp expect actual.filtered +' + +test_expect_success 'showing a range walks (linear)' ' + cat >expect <<-EOF && + commit $(git rev-parse main3) + commit $(git rev-parse main2) + EOF + git show main1..main3 >actual && + grep ^commit actual >actual.filtered && + test_cmp expect actual.filtered +' + +test_expect_success 'showing a range walks (Y shape, ^ first)' ' + cat >expect <<-EOF && + commit $(git rev-parse main3) + commit $(git rev-parse main2) + EOF + git show ^side3 main3 >actual && + grep ^commit actual >actual.filtered && + test_cmp expect actual.filtered +' + +test_expect_failure 'showing a range walks (Y shape, ^ last)' ' + cat >expect <<-EOF && + commit $(git rev-parse main3) + commit $(git rev-parse main2) + EOF + git show main3 ^side3 >actual && + grep ^commit actual >actual.filtered && + test_cmp expect actual.filtered +' + +test_expect_success 'showing with -N walks' ' + cat >expect <<-EOF && + commit $(git rev-parse main3) + commit $(git rev-parse main2) + EOF + git show -2 main3 >actual && + grep ^commit actual >actual.filtered && + test_cmp expect actual.filtered +' + +test_expect_success 'showing annotated tag' ' + cat >expect <<-EOF && + tag annotated + commit $(git rev-parse annotated^{commit}) + EOF + git show annotated >actual && + grep -E "^(commit|tag)" actual >actual.filtered && + test_cmp expect actual.filtered +' + +test_expect_success 'showing annotated tag plus commit' ' + cat >expect <<-EOF && + tag annotated + commit $(git rev-parse annotated^{commit}) + commit $(git rev-parse side3) + EOF + git show annotated side3 >actual && + grep -E "^(commit|tag)" actual >actual.filtered && + test_cmp expect actual.filtered +' + +test_expect_failure 'showing range' ' + cat >expect <<-EOF && + commit $(git rev-parse main3) + commit $(git rev-parse main2) + EOF + git show ^side3 annotated >actual && + grep -E "^(commit|tag)" actual >actual.filtered && + test_cmp expect actual.filtered +' + test_done From c5941f1aac071addc1c9b0781c323b588c542420 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 19 Jun 2012 14:15:57 -0700 Subject: [PATCH 2/2] show: fix "range implies walking" Signed-off-by: Junio C Hamano --- builtin/log.c | 3 +++ t/t7007-show.sh | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 56bc555d11..9ea2eb7bb0 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -451,6 +451,9 @@ int cmd_show(int argc, const char **argv, const char *prefix) opt.tweak = show_rev_tweak_rev; cmd_log_init(argc, argv, prefix, &rev, &opt); + if (!rev.no_walk) + return cmd_log_walk(&rev); + count = rev.pending.nr; objects = rev.pending.objects; for (i = 0; i < count && !ret; i++) { diff --git a/t/t7007-show.sh b/t/t7007-show.sh index 891b143e0c..a40cd3630c 100755 --- a/t/t7007-show.sh +++ b/t/t7007-show.sh @@ -57,7 +57,7 @@ test_expect_success 'showing a range walks (Y shape, ^ first)' ' test_cmp expect actual.filtered ' -test_expect_failure 'showing a range walks (Y shape, ^ last)' ' +test_expect_success 'showing a range walks (Y shape, ^ last)' ' cat >expect <<-EOF && commit $(git rev-parse main3) commit $(git rev-parse main2) @@ -98,7 +98,7 @@ test_expect_success 'showing annotated tag plus commit' ' test_cmp expect actual.filtered ' -test_expect_failure 'showing range' ' +test_expect_success 'showing range' ' cat >expect <<-EOF && commit $(git rev-parse main3) commit $(git rev-parse main2)