On Fri, Feb 14, 2020 at 01:22:13PM -0500, Jeff King wrote:
> The "--use-bitmap-index" option is usually aspirational: if we have
> bitmaps and the request can be fulfilled more quickly using them we'll
> do so, but otherwise fall back to a non-bitmap traversal.
>
> The exception is object filtering, which explicitly dies if the two
> options are combined. Let's convert this to the usual fallback behavior.
> This is a minor convenience for now (since the caller can easily know
> that --filter and --use-bitmap-index don't combine), but will become
> much more useful as we start to support _some_ filters with bitmaps, but
> not others.
Yeah, I think that this convenience is worth it early on instead of
lumping in these changes in a future patch.
> The test infrastructure here is bigger than necessary for checking this
> one small feature. But it will serve as the basis for more filtering
> bitmap tests in future patches.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> builtin/rev-list.c | 4 ++--
> t/t6113-rev-list-bitmap-filters.sh | 24 ++++++++++++++++++++++++
> 2 files changed, 26 insertions(+), 2 deletions(-)
> create mode 100755 t/t6113-rev-list-bitmap-filters.sh
>
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index e28d62ec64..bce406bd1e 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -521,8 +521,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
> if (revs.show_notes)
> die(_("rev-list does not support display of notes"));
>
> - if (filter_options.choice && use_bitmap_index)
> - die(_("cannot combine --use-bitmap-index with object filtering"));
> + if (filter_options.choice)
> + use_bitmap_index = 0;
>
> save_commit_buffer = (revs.verbose_header ||
> revs.grep_filter.pattern_list ||
> diff --git a/t/t6113-rev-list-bitmap-filters.sh b/t/t6113-rev-list-bitmap-filters.sh
> new file mode 100755
> index 0000000000..977f8d0930
> --- /dev/null
> +++ b/t/t6113-rev-list-bitmap-filters.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +
> +test_description='rev-list combining bitmaps and filters'
> +. ./test-lib.sh
> +
> +test_expect_success 'set up bitmapped repo' '
> + # one commit will have bitmaps, the other will not
> + test_commit one &&
> + git repack -adb &&
> + test_commit two
> +'
> +
> +test_expect_success 'filters fallback to non-bitmap traversal' '
> + # use a path-based filter, since they are inherently incompatible with
> + # bitmaps (i.e., this test will never get confused by later code to
> + # combine the features)
> + filter=$(echo "!one" | git hash-object -w --stdin) &&
> + git rev-list --objects --filter=sparse:oid=$filter HEAD >expect &&
> + git rev-list --use-bitmap-index \
> + --objects --filter=sparse:oid=$filter HEAD >actual &&
> + test_cmp expect actual
> +'
> +
> +test_done
> --
> 2.25.0.796.gcc29325708
Makes sense.
Thanks,
Taylor
