{"thread":{"id":"39757","subject":"[PATCH] rev-list: add --count to usage guide","startedAt":"2015-07-01T09:24:11Z","lastAt":"2015-07-01T19:00:13Z","messageCount":4,"participants":["Lawrence Siebert","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"265313","messageId":"1435742651-15053-1-git-send-email-lawrencesiebert@gmail.com","threadId":"39757","inReplyTo":null,"subject":"[PATCH] rev-list: add --count to usage guide","fromName":"Lawrence Siebert","fromEmail":"lawrencesiebert@gmail.com","sentAt":"2015-07-01T09:24:11Z","receivedAt":"2015-07-01T09:24:11Z","isPatch":true,"sender":{"key":"lawrencesiebert@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45034?v=4"},"body":"--count should be mentioned in the usage guide, this updates code and\ndocumentation.\n\nSigned-off-by: Lawrence Siebert \n---\n Documentation/git-rev-list.txt | 1 +\n builtin/rev-list.c | 1 +\n 2 files changed, 2 insertions(+)\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex b10ea60..7b49c85 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -56,6 +56,7 @@ SYNOPSIS\n \t [ --reverse ]\n \t [ --walk-reflogs ]\n \t [ --no-walk ] [ --do-walk ]\n+\t [ --count ]\n \t [ --use-bitmap-index ]\n \t ... [ \\-- ... ]\n \ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ff84a82..ee9e872 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -42,6 +42,7 @@ static const char rev_list_usage[] =\n \" --abbrev= | --no-abbrev\\n\"\n \" --abbrev-commit\\n\"\n \" --left-right\\n\"\n+\" --count\\n\"\n \" special purpose:\\n\"\n \" --bisect\\n\"\n \" --bisect-vars\\n\"\n-- \n1.9.1\n"},{"id":"265324","messageId":"xmqqd20bc3cj.fsf@gitster.dls.corp.google.com","threadId":"39757","inReplyTo":"1435742651-15053-1-git-send-email-lawrencesiebert@gmail.com","subject":"Re: [PATCH] rev-list: add --count to usage guide","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T15:19:40Z","receivedAt":"2015-07-01T15:19:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lawrence Siebert writes:\n\n> --count should be mentioned in the usage guide, this updates code and\n> documentation.\n>\n> Signed-off-by: Lawrence Siebert \n> ---\n\nSounds good. While at it, perhaps add a mention (perhaps by\ncreating a BUGS section at the end of the file) that --count\nwith --use-bitmap-index ignores pathspec silently?\n\n> Documentation/git-rev-list.txt | 1 +\n> builtin/rev-list.c | 1 +\n> 2 files changed, 2 insertions(+)\n>\n> diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\n> index b10ea60..7b49c85 100644\n> --- a/Documentation/git-rev-list.txt\n> +++ b/Documentation/git-rev-list.txt\n> @@ -56,6 +56,7 @@ SYNOPSIS\n> \t [ --reverse ]\n> \t [ --walk-reflogs ]\n> \t [ --no-walk ] [ --do-walk ]\n> +\t [ --count ]\n> \t [ --use-bitmap-index ]\n> \t ... [ \\-- ... ]\n> \n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index ff84a82..ee9e872 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -42,6 +42,7 @@ static const char rev_list_usage[] =\n> \" --abbrev= | --no-abbrev\\n\"\n> \" --abbrev-commit\\n\"\n> \" --left-right\\n\"\n> +\" --count\\n\"\n> \" special purpose:\\n\"\n> \" --bisect\\n\"\n> \" --bisect-vars\\n\"\n"},{"id":"265346","messageId":"20150701184217.GA8079@peff.net","threadId":"39757","inReplyTo":"xmqqd20bc3cj.fsf@gitster.dls.corp.google.com","subject":"[PATCH] rev-list: disable --use-bitmap-index when pruning commits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-01T18:42:17Z","receivedAt":"2015-07-01T18:42:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 01, 2015 at 08:19:40AM -0700, Junio C Hamano wrote:\n\n> Sounds good. While at it, perhaps add a mention (perhaps by\n> creating a BUGS section at the end of the file) that --count\n> with --use-bitmap-index ignores pathspec silently?\n\nI think we can just fix it rather than documenting the problem. :)\n\n-- >8 --\nSubject: rev-list: disable --use-bitmap-index when pruning commits\n\nThe reachability bitmaps do not have enough information to\ntell us which commits might have changed path \"foo\", so the\ncurrent code produces wrong answers for:\n\n git rev-list --use-bitmap-index --count HEAD -- foo\n\n(it silently ignores the \"foo\" limiter). Instead, we should\nfall back to doing a normal traversal (it is OK to fall\nback rather than complain, because --use-bitmap-index is a\npure optimization, and might not kick in for other reasons,\nsuch as there being no bitmaps in the repository).\n\nSigned-off-by: Jeff King \n---\n builtin/rev-list.c | 2 +-\n t/t5310-pack-bitmaps.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ff84a82..88eddbd 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -355,7 +355,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tif (bisect_list)\n \t\trevs.limited = 1;\n \n-\tif (use_bitmap_index) {\n+\tif (use_bitmap_index && !revs.prune) {\n \t\tif (revs.count && !revs.left_right && !revs.cherry_mark) {\n \t\t\tuint32_t commit_count;\n \t\t\tif (!prepare_bitmap_walk(&revs)) {\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 6003490..d446706 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -53,6 +53,12 @@ rev_list_tests() {\n \t\ttest_cmp expect actual\n \t'\n \n+\ttest_expect_success \"counting commits with limiting ($state)\" '\n+\t\tgit rev-list --count HEAD -- 1.t >expect &&\n+\t\tgit rev-list --use-bitmap-index --count HEAD -- 1.t >actual &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n \ttest_expect_success \"enumerate --objects ($state)\" '\n \t\tgit rev-list --objects --use-bitmap-index HEAD >tmp &&\n \t\tcut -d\" \" -f1 tmp2 &&\n-- \n2.5.0.rc0.336.g8460790\n"},{"id":"265351","messageId":"xmqq3817aeki.fsf@gitster.dls.corp.google.com","threadId":"39757","inReplyTo":"20150701184217.GA8079@peff.net","subject":"Re: [PATCH] rev-list: disable --use-bitmap-index when pruning commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-01T19:00:13Z","receivedAt":"2015-07-01T19:00:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Jul 01, 2015 at 08:19:40AM -0700, Junio C Hamano wrote:\n>\n>> Sounds good. While at it, perhaps add a mention (perhaps by\n>> creating a BUGS section at the end of the file) that --count\n>> with --use-bitmap-index ignores pathspec silently?\n>\n> I think we can just fix it rather than documenting the problem. :)\n\n;-) Good.\n\nWill queue; thanks.\n\n>\n> -- >8 --\n> Subject: rev-list: disable --use-bitmap-index when pruning commits\n>\n> The reachability bitmaps do not have enough information to\n> tell us which commits might have changed path \"foo\", so the\n> current code produces wrong answers for:\n>\n> git rev-list --use-bitmap-index --count HEAD -- foo\n>\n> (it silently ignores the \"foo\" limiter). Instead, we should\n> fall back to doing a normal traversal (it is OK to fall\n> back rather than complain, because --use-bitmap-index is a\n> pure optimization, and might not kick in for other reasons,\n> such as there being no bitmaps in the repository).\n>\n> Signed-off-by: Jeff King \n> ---\n> builtin/rev-list.c | 2 +-\n> t/t5310-pack-bitmaps.sh | 6 ++++++\n> 2 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index ff84a82..88eddbd 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -355,7 +355,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n> \tif (bisect_list)\n> \t\trevs.limited = 1;\n> \n> -\tif (use_bitmap_index) {\n> +\tif (use_bitmap_index && !revs.prune) {\n> \t\tif (revs.count && !revs.left_right && !revs.cherry_mark) {\n> \t\t\tuint32_t commit_count;\n> \t\t\tif (!prepare_bitmap_walk(&revs)) {\n> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\n> index 6003490..d446706 100755\n> --- a/t/t5310-pack-bitmaps.sh\n> +++ b/t/t5310-pack-bitmaps.sh\n> @@ -53,6 +53,12 @@ rev_list_tests() {\n> \t\ttest_cmp expect actual\n> \t'\n> \n> +\ttest_expect_success \"counting commits with limiting ($state)\" '\n> +\t\tgit rev-list --count HEAD -- 1.t >expect &&\n> +\t\tgit rev-list --use-bitmap-index --count HEAD -- 1.t >actual &&\n> +\t\ttest_cmp expect actual\n> +\t'\n> +\n> \ttest_expect_success \"enumerate --objects ($state)\" '\n> \t\tgit rev-list --objects --use-bitmap-index HEAD >tmp &&\n> \t\tcut -d\" \" -f1 tmp2 &&\n"}]}