Subject: [PATCH v4 0/8] rev-list: implement object type filter

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 12 Apr 2021 15:37:14 +0200

Hi,

this is the fourth version of my patch series which implements a new
`object:type` filter for git-rev-list(1) and git-upload-pack(1) and
extends support for bitmap indices to work with combined filters.

Changes compared to v3:

    - Small style fix to not pass an empty string and `0`, but instead
      simply pass two `NULL` pointers to
      `list_objects_filter__filter_object, pointed out by Junio.

    - I've changed patch 7/8 as proposed by Peff: support of combined
      filters is now determined directly in `filter_bitmap()`, without
      having to mirror all filter types in the new `filter_supported()`
      function.

    - Renamed `--filter-provided-revisions` to
      `--filter-provided-objects` as proposed by Peff and addressed both
      commit message and tests as pointed out by Philip.

Thanks for all your feedback! As alawys, the range-diff is attached
below.

Patrick

Patrick Steinhardt (8):
  uploadpack.txt: document implication of `uploadpackfilter.allow`
  revision: mark commit parents as NOT_USER_GIVEN
  list-objects: move tag processing into its own function
  list-objects: support filtering by tag and commit
  list-objects: implement object type filter
  pack-bitmap: implement object type filter
  pack-bitmap: implement combined filter
  rev-list: allow filtering of provided items

 Documentation/config/uploadpack.txt |   9 ++-
 Documentation/rev-list-options.txt  |   8 ++
 builtin/pack-objects.c              |   2 +-
 builtin/rev-list.c                  |  36 ++++++---
 list-objects-filter-options.c       |  14 ++++
 list-objects-filter-options.h       |   2 +
 list-objects-filter.c               | 116 ++++++++++++++++++++++++++++
 list-objects-filter.h               |   2 +
 list-objects.c                      |  30 ++++++-
 pack-bitmap.c                       |  45 +++++++++--
 pack-bitmap.h                       |   3 +-
 reachable.c                         |   2 +-
 revision.c                          |   4 +-
 revision.h                          |   3 -
 t/t6112-rev-list-filters-objects.sh |  76 ++++++++++++++++++
 t/t6113-rev-list-bitmap-filters.sh  |  68 +++++++++++++++-
 16 files changed, 390 insertions(+), 30 deletions(-)

Range-diff against v3:
1:  f80b9570d4 = 1:  f80b9570d4 uploadpack.txt: document implication of `uploadpackfilter.allow`
2:  46c1952405 = 2:  46c1952405 revision: mark commit parents as NOT_USER_GIVEN
3:  3d792f6339 = 3:  3d792f6339 list-objects: move tag processing into its own function
4:  80193d6ba3 ! 4:  674da0f9ac list-objects: support filtering by tag and commit
    @@ list-objects.c: static void process_tag(struct traversal_context *ctx,
     +	enum list_objects_filter_result r;
     +
     +	r = list_objects_filter__filter_object(ctx->revs->repo, LOFS_TAG,
    -+					       &tag->object, "", 0, ctx->filter);
    ++					       &tag->object, NULL, NULL,
    ++					       ctx->filter);
     +	if (r & LOFR_MARK_SEEN)
     +		tag->object.flags |= SEEN;
     +	if (r & LOFR_DO_SHOW)
5:  e2a14abf92 = 5:  d22a5fd37d list-objects: implement object type filter
6:  46d4450d38 = 6:  17c9f66bbc pack-bitmap: implement object type filter
7:  06a376399b ! 7:  759ac54bb2 pack-bitmap: implement combined filter
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <[email-masked]>
     
      ## pack-bitmap.c ##
    -@@ pack-bitmap.c: static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
    - 		filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_BLOB);
    - }
    - 
    -+static int filter_supported(struct list_objects_filter_options *filter)
    -+{
    -+	int i;
    -+
    -+	switch (filter->choice) {
    -+	case LOFC_BLOB_NONE:
    -+	case LOFC_BLOB_LIMIT:
    -+	case LOFC_OBJECT_TYPE:
    -+		return 1;
    -+	case LOFC_TREE_DEPTH:
    -+		if (filter->tree_exclude_depth == 0)
    -+			return 1;
    -+		return 0;
    -+	case LOFC_COMBINE:
    -+		for (i = 0; i < filter->sub_nr; i++)
    -+			if (!filter_supported(&filter->sub[i]))
    -+				return 0;
    -+		return 1;
    -+	default:
    -+		return 0;
    -+	}
    -+}
    -+
    - static int filter_bitmap(struct bitmap_index *bitmap_git,
    - 			 struct object_list *tip_objects,
    - 			 struct bitmap *to_filter,
    -@@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
    - {
    - 	if (!filter || filter->choice == LOFC_DISABLED)
    - 		return 0;
    -+	if (!filter_supported(filter))
    -+		return -1;
    - 
    - 	if (filter->choice == LOFC_BLOB_NONE) {
    - 		if (bitmap_git)
     @@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
      		return 0;
      	}
      
    --	if (filter->choice == LOFC_TREE_DEPTH &&
    --	    filter->tree_exclude_depth == 0) {
    -+	if (filter->choice == LOFC_TREE_DEPTH) {
    - 		if (bitmap_git)
    - 			filter_bitmap_tree_depth(bitmap_git, tip_objects,
    - 						 to_filter,
    -@@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
    - 		return 0;
    - 	}
    - 
    --	/* filter choice not handled */
    --	return -1;
     +	if (filter->choice == LOFC_COMBINE) {
     +		int i;
     +		for (i = 0; i < filter->sub_nr; i++) {
    @@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
     +		return 0;
     +	}
     +
    -+	BUG("unsupported filter choice");
    + 	/* filter choice not handled */
    + 	return -1;
      }
    - 
    - static int can_filter_bitmap(struct list_objects_filter_options *filter)
     
      ## t/t6113-rev-list-bitmap-filters.sh ##
     @@ t/t6113-rev-list-bitmap-filters.sh: test_expect_success 'object:type filter' '
8:  cf2297b413 ! 8:  c779d222cf rev-list: allow filtering of provided items
    @@ Commit message
         that even if the user wants to only see blobs, he'll still see commits
         of provided references.
     
    -    Improve this by introducing a new `--filter-provided` option to the
    -    git-rev-parse(1) command. If given, then all user-provided references
    -    will be subject to filtering.
    +    Improve this by introducing a new `--filter-provided-objects` option
    +    to the git-rev-parse(1) command. If given, then all user-provided
    +    references will be subject to filtering.
     
         Signed-off-by: Patrick Steinhardt <[email-masked]>
     
    @@ Documentation/rev-list-options.txt: equivalent.
      --no-filter::
      	Turn off any previous `--filter=` argument.
      
    -+--filter-provided-revisions::
    -+	Filter the list of explicitly provided revisions, which would otherwise
    ++--filter-provided-objects::
    ++	Filter the list of explicitly provided objects, which would otherwise
     +	always be printed even if they did not match any of the filters. Only
     +	useful with `--filter=`.
     +
    @@ builtin/rev-list.c: static inline int parse_missing_action_value(const char *val
      static int try_bitmap_count(struct rev_info *revs,
     -			    struct list_objects_filter_options *filter)
     +			    struct list_objects_filter_options *filter,
    -+			    int filter_provided_revs)
    ++			    int filter_provided_objects)
      {
      	uint32_t commit_count = 0,
      		 tag_count = 0,
    @@ builtin/rev-list.c: static int try_bitmap_count(struct rev_info *revs,
      	max_count = revs->max_count;
      
     -	bitmap_git = prepare_bitmap_walk(revs, filter);
    -+	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
    ++	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
      	if (!bitmap_git)
      		return -1;
      
    @@ builtin/rev-list.c: static int try_bitmap_count(struct rev_info *revs,
      static int try_bitmap_traversal(struct rev_info *revs,
     -				struct list_objects_filter_options *filter)
     +				struct list_objects_filter_options *filter,
    -+				int filter_provided_revs)
    ++				int filter_provided_objects)
      {
      	struct bitmap_index *bitmap_git;
      
    @@ builtin/rev-list.c: static int try_bitmap_traversal(struct rev_info *revs,
      		return -1;
      
     -	bitmap_git = prepare_bitmap_walk(revs, filter);
    -+	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
    ++	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
      	if (!bitmap_git)
      		return -1;
      
    @@ builtin/rev-list.c: static int try_bitmap_traversal(struct rev_info *revs,
      static int try_bitmap_disk_usage(struct rev_info *revs,
     -				 struct list_objects_filter_options *filter)
     +				 struct list_objects_filter_options *filter,
    -+				 int filter_provided_revs)
    ++				 int filter_provided_objects)
      {
      	struct bitmap_index *bitmap_git;
      
    @@ builtin/rev-list.c: static int try_bitmap_traversal(struct rev_info *revs,
      		return -1;
      
     -	bitmap_git = prepare_bitmap_walk(revs, filter);
    -+	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
    ++	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_objects);
      	if (!bitmap_git)
      		return -1;
      
    @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
      	int bisect_show_vars = 0;
      	int bisect_find_all = 0;
      	int use_bitmap_index = 0;
    -+	int filter_provided_revs = 0;
    ++	int filter_provided_objects = 0;
      	const char *show_progress = NULL;
      
      	if (argc == 2 && !strcmp(argv[1], "-h"))
    @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
      			list_objects_filter_set_no_filter(&filter_options);
      			continue;
      		}
    -+		if (!strcmp(arg, "--filter-provided-revisions")) {
    -+			filter_provided_revs = 1;
    ++		if (!strcmp(arg, "--filter-provided-objects")) {
    ++			filter_provided_objects = 1;
     +			continue;
     +		}
      		if (!strcmp(arg, "--filter-print-omitted")) {
    @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
      
      	if (use_bitmap_index) {
     -		if (!try_bitmap_count(&revs, &filter_options))
    -+		if (!try_bitmap_count(&revs, &filter_options, filter_provided_revs))
    ++		if (!try_bitmap_count(&revs, &filter_options, filter_provided_objects))
      			return 0;
     -		if (!try_bitmap_disk_usage(&revs, &filter_options))
    -+		if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_revs))
    ++		if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_objects))
      			return 0;
     -		if (!try_bitmap_traversal(&revs, &filter_options))
    -+		if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_revs))
    ++		if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_objects))
      			return 0;
      	}
      
    @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
      			return show_bisect_vars(&info, reaches, all);
      	}
      
    -+	if (filter_provided_revs) {
    ++	if (filter_provided_objects) {
     +		struct commit_list *c;
     +		for (i = 0; i < revs.pending.nr; i++) {
     +			struct object_array_entry *pending = revs.pending.objects + i;
    @@ pack-bitmap.c: static int can_filter_bitmap(struct list_objects_filter_options *
      struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
     -					 struct list_objects_filter_options *filter)
     +					 struct list_objects_filter_options *filter,
    -+					 int filter_provided_revs)
    ++					 int filter_provided_objects)
      {
      	unsigned int i;
      
    @@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
      		bitmap_and_not(wants_bitmap, haves_bitmap);
      
     -	filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
    -+	filter_bitmap(bitmap_git, (filter && filter_provided_revs) ? NULL : wants,
    ++	filter_bitmap(bitmap_git, (filter && filter_provided_objects) ? NULL : wants,
     +		      wants_bitmap, filter);
      
      	bitmap_git->result = wants_bitmap;
    @@ pack-bitmap.h: void traverse_bitmap_commit_list(struct bitmap_index *,
      struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
     -					 struct list_objects_filter_options *filter);
     +					 struct list_objects_filter_options *filter,
    -+					 int filter_provided_revs);
    ++					 int filter_provided_objects);
      int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
      				       struct packed_git **packfile,
      				       uint32_t *entries,
    @@ t/t6112-rev-list-filters-objects.sh: test_expect_success 'verify object:type=tag
      	test_cmp expected actual
      '
      
    -+test_expect_success 'verify object:type=blob prints only blob with --filter-provided-revisions' '
    ++test_expect_success 'verify object:type=blob prints only blob with --filter-provided-objects' '
     +	printf "%s blob\n" $(git -C object-type rev-parse HEAD:blob) >expected &&
     +	git -C object-type rev-list --objects \
    -+		--filter=object:type=blob --filter-provided-revisions HEAD >actual &&
    ++		--filter=object:type=blob --filter-provided-objects HEAD >actual &&
     +	test_cmp expected actual
     +'
     +
    -+test_expect_success 'verify object:type=tree prints only tree with --filter-provided-revisions' '
    ++test_expect_success 'verify object:type=tree prints only tree with --filter-provided-objects' '
     +	printf "%s \n" $(git -C object-type rev-parse HEAD^{tree}) >expected &&
     +	git -C object-type rev-list --objects \
    -+		--filter=object:type=tree HEAD --filter-provided-revisions >actual &&
    ++		--filter=object:type=tree HEAD --filter-provided-objects >actual &&
     +	test_cmp expected actual
     +'
     +
    -+test_expect_success 'verify object:type=commit prints only commit with --filter-provided-revisions' '
    ++test_expect_success 'verify object:type=commit prints only commit with --filter-provided-objects' '
     +	git -C object-type rev-parse HEAD >expected &&
     +	git -C object-type rev-list --objects \
    -+		--filter=object:type=commit --filter-provided-revisions HEAD >actual &&
    ++		--filter=object:type=commit --filter-provided-objects HEAD >actual &&
     +	test_cmp expected actual
     +'
     +
    -+test_expect_success 'verify object:type=tag prints only tag with --filter-provided-revisions' '
    ++test_expect_success 'verify object:type=tag prints only tag with --filter-provided-objects' '
     +	printf "%s tag\n" $(git -C object-type rev-parse tag) >expected &&
     +	git -C object-type rev-list --objects \
    -+		--filter=object:type=tag --filter-provided-revisions tag >actual &&
    ++		--filter=object:type=tag --filter-provided-objects tag >actual &&
     +	test_cmp expected actual
     +'
     +
    @@ t/t6113-rev-list-bitmap-filters.sh: test_expect_success 'object:type filter' '
      	test_bitmap_traversal expect actual
      '
      
    -+test_expect_success 'object:type filter with --filter-provided-revisions' '
    -+	git rev-list --objects --filter-provided-revisions --filter=object:type=tag tag >expect &&
    ++test_expect_success 'object:type filter with --filter-provided-objects' '
    ++	git rev-list --objects --filter-provided-objects --filter=object:type=tag tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter-provided-revisions --filter=object:type=tag tag >actual &&
    ++		     --objects --filter-provided-objects --filter=object:type=tag tag >actual &&
     +	test_cmp expect actual &&
     +
    -+	git rev-list --objects --filter-provided-revisions --filter=object:type=commit tag >expect &&
    ++	git rev-list --objects --filter-provided-objects --filter=object:type=commit tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter-provided-revisions --filter=object:type=commit tag >actual &&
    ++		     --objects --filter-provided-objects --filter=object:type=commit tag >actual &&
     +	test_bitmap_traversal expect actual &&
     +
    -+	git rev-list --objects --filter-provided-revisions --filter=object:type=tree tag >expect &&
    ++	git rev-list --objects --filter-provided-objects --filter=object:type=tree tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter-provided-revisions --filter=object:type=tree tag >actual &&
    ++		     --objects --filter-provided-objects --filter=object:type=tree tag >actual &&
     +	test_bitmap_traversal expect actual &&
     +
    -+	git rev-list --objects --filter-provided-revisions --filter=object:type=blob tag >expect &&
    ++	git rev-list --objects --filter-provided-objects --filter=object:type=blob tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter-provided-revisions --filter=object:type=blob tag >actual &&
    ++		     --objects --filter-provided-objects --filter=object:type=blob tag >actual &&
     +	test_bitmap_traversal expect actual
     +'
     +
    @@ t/t6113-rev-list-bitmap-filters.sh: test_expect_success 'combine filter' '
      	test_bitmap_traversal expect actual
      '
      
    -+test_expect_success 'combine filter with --filter-provided-revisions' '
    -+	git rev-list --objects --filter-provided-revisions --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
    ++test_expect_success 'combine filter with --filter-provided-objects' '
    ++	git rev-list --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter-provided-revisions --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
    ++		     --objects --filter-provided-objects --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
     +	test_bitmap_traversal expect actual &&
     +
     +	git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
-- 
2.31.1