[PATCH v3 0/8] rev-parse: implement object type filter

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 15084 bytes --]

Hi,

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

This mostly addresses Peff's comments. Thanks for your feedback!

    - Removed the `base` parameter from `process_tag()`.

    - The object type filter doesn't assume ordering for the object type
      enum anymore.

    - Combined filters in the bitmap path now verify that
      `filter_bitmap` does not return any errors.

    - Renamed "--filter-provided" to "--filter-provided-revisions" and
      added documentation for it.

    - Refactored the code to not munge the `filter_provided` field in
      the filter options struct, but instead carry it in rev-list.c.

Please see the attached range-diff for more details.

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                      |  29 ++++++-
 pack-bitmap.c                       |  76 +++++++++++++++---
 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, 416 insertions(+), 34 deletions(-)

Range-diff against v2:
1:  270ff80dac = 1:  f80b9570d4 uploadpack.txt: document implication of `uploadpackfilter.allow`
2:  ddbec75986 = 2:  46c1952405 revision: mark commit parents as NOT_USER_GIVEN
3:  d8da0b24f4 ! 3:  3d792f6339 list-objects: move tag processing into its own function
    @@ list-objects.c: static void process_tree(struct traversal_context *ctx,
      
     +static void process_tag(struct traversal_context *ctx,
     +			struct tag *tag,
    -+			struct strbuf *base,
     +			const char *name)
     +{
     +	tag->object.flags |= SEEN;
    @@ list-objects.c: static void traverse_trees_and_blobs(struct traversal_context *c
      		if (obj->type == OBJ_TAG) {
     -			obj->flags |= SEEN;
     -			ctx->show_object(obj, name, ctx->show_data);
    -+			process_tag(ctx, (struct tag *)obj, base, name);
    ++			process_tag(ctx, (struct tag *)obj, name);
      			continue;
      		}
      		if (!path)
4:  5545c189c5 ! 4:  80193d6ba3 list-objects: support filtering by tag and commit
    @@ list-objects-filter.h: enum list_objects_filter_result {
     
      ## list-objects.c ##
     @@ list-objects.c: static void process_tag(struct traversal_context *ctx,
    - 			struct strbuf *base,
    + 			struct tag *tag,
      			const char *name)
      {
     -	tag->object.flags |= SEEN;
    @@ 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, base->buf,
    -+					       &base->buf[base->len],
    -+					       ctx->filter);
    ++					       &tag->object, "", 0, ctx->filter);
     +	if (r & LOFR_MARK_SEEN)
     +		tag->object.flags |= SEEN;
     +	if (r & LOFR_DO_SHOW)
5:  acf01472af = 5:  e2a14abf92 list-objects: implement object type filter
6:  8073ab665b ! 6:  46d4450d38 pack-bitmap: implement object type filter
    @@ pack-bitmap.c: static void filter_bitmap_tree_depth(struct bitmap_index *bitmap_
     +				      struct bitmap *to_filter,
     +				      enum object_type object_type)
     +{
    -+	enum object_type t;
    -+
     +	if (object_type < OBJ_COMMIT || object_type > OBJ_TAG)
     +		BUG("filter_bitmap_object_type given invalid object");
     +
    -+	for (t = OBJ_COMMIT; t <= OBJ_TAG; t++) {
    -+		if (t == object_type)
    -+			continue;
    -+		filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, t);
    -+	}
    ++	if (object_type != OBJ_TAG)
    ++		filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TAG);
    ++	if (object_type != OBJ_COMMIT)
    ++		filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_COMMIT);
    ++	if (object_type != OBJ_TREE)
    ++		filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_TREE);
    ++	if (object_type != OBJ_BLOB)
    ++		filter_bitmap_exclude_type(bitmap_git, tip_objects, to_filter, OBJ_BLOB);
     +}
     +
      static int filter_bitmap(struct bitmap_index *bitmap_git,
7:  fac3477d97 ! 7:  06a376399b pack-bitmap: implement combined filter
    @@ Commit message
     
      ## 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)
    @@ pack-bitmap.c: static int filter_bitmap(struct bitmap_index *bitmap_git,
     +	if (filter->choice == LOFC_COMBINE) {
     +		int i;
     +		for (i = 0; i < filter->sub_nr; i++) {
    -+			filter_bitmap(bitmap_git, tip_objects, to_filter,
    -+				      &filter->sub[i]);
    ++			if (filter_bitmap(bitmap_git, tip_objects, to_filter,
    ++					  &filter->sub[i]) < 0)
    ++				return -1;
     +		}
     +		return 0;
     +	}
8:  0e26fee8b3 ! 8:  796606f32b rev-list: allow filtering of provided items
    @@ Commit message
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    + ## Documentation/rev-list-options.txt ##
    +@@ 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
    ++	always be printed even if they did not match any of the filters. Only
    ++	useful with `--filter=`.
    ++
    + --filter-print-omitted::
    + 	Only useful with `--filter=`; prints a list of the objects omitted
    + 	by the filter.  Object IDs are prefixed with a ``~'' character.
    +
    + ## builtin/pack-objects.c ##
    +@@ builtin/pack-objects.c: static int pack_options_allow_reuse(void)
    + 
    + static int get_object_list_from_bitmap(struct rev_info *revs)
    + {
    +-	if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options)))
    ++	if (!(bitmap_git = prepare_bitmap_walk(revs, &filter_options, 0)))
    + 		return -1;
    + 
    + 	if (pack_options_allow_reuse() &&
    +
      ## builtin/rev-list.c ##
    +@@ builtin/rev-list.c: static inline int parse_missing_action_value(const char *value)
    + }
    + 
    + 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)
    + {
    + 	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);
    + 	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)
    + {
    + 	struct bitmap_index *bitmap_git;
    + 
    +@@ builtin/rev-list.c: static int try_bitmap_traversal(struct rev_info *revs,
    + 	if (revs->max_count >= 0)
    + 		return -1;
    + 
    +-	bitmap_git = prepare_bitmap_walk(revs, filter);
    ++	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
    + 	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)
    + {
    + 	struct bitmap_index *bitmap_git;
    + 
    + 	if (!show_disk_usage)
    + 		return -1;
    + 
    +-	bitmap_git = prepare_bitmap_walk(revs, filter);
    ++	bitmap_git = prepare_bitmap_walk(revs, filter, filter_provided_revs);
    + 	if (!bitmap_git)
    + 		return -1;
    + 
    +@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *prefix)
    + 	int bisect_show_vars = 0;
    + 	int bisect_find_all = 0;
    + 	int use_bitmap_index = 0;
    ++	int filter_provided_revs = 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 *prefix)
      			list_objects_filter_set_no_filter(&filter_options);
      			continue;
      		}
    -+		if (!strcmp(arg, "--filter-provided")) {
    -+			filter_options.filter_wants = 1;
    ++		if (!strcmp(arg, "--filter-provided-revisions")) {
    ++			filter_provided_revs = 1;
     +			continue;
     +		}
      		if (!strcmp(arg, "--filter-print-omitted")) {
      			arg_print_omitted = 1;
      			continue;
    +@@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *prefix)
    + 		progress = start_delayed_progress(show_progress, 0);
    + 
    + 	if (use_bitmap_index) {
    +-		if (!try_bitmap_count(&revs, &filter_options))
    ++		if (!try_bitmap_count(&revs, &filter_options, filter_provided_revs))
    + 			return 0;
    +-		if (!try_bitmap_disk_usage(&revs, &filter_options))
    ++		if (!try_bitmap_disk_usage(&revs, &filter_options, filter_provided_revs))
    + 			return 0;
    +-		if (!try_bitmap_traversal(&revs, &filter_options))
    ++		if (!try_bitmap_traversal(&revs, &filter_options, filter_provided_revs))
    + 			return 0;
    + 	}
    + 
     @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *prefix)
      			return show_bisect_vars(&info, reaches, all);
      	}
      
    -+	if (filter_options.filter_wants) {
    ++	if (filter_provided_revs) {
     +		struct commit_list *c;
     +		for (i = 0; i < revs.pending.nr; i++) {
     +			struct object_array_entry *pending = revs.pending.objects + i;
    @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr
      		oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);
      	if (arg_missing_action == MA_PRINT)
     
    - ## list-objects-filter-options.c ##
    -@@ list-objects-filter-options.c: static void transform_to_combine_type(
    - 		memset(filter_options, 0, sizeof(*filter_options));
    - 		filter_options->sub = sub_array;
    - 		filter_options->sub_alloc = initial_sub_alloc;
    -+		filter_options->filter_wants = sub_array[0].filter_wants;
    - 	}
    - 	filter_options->sub_nr = 1;
    - 	filter_options->choice = LOFC_COMBINE;
    -@@ list-objects-filter-options.c: void parse_list_objects_filter(
    - 		parse_error = gently_parse_list_objects_filter(
    - 			&filter_options->sub[filter_options->sub_nr - 1], arg,
    - 			&errbuf);
    -+		if (!parse_error)
    -+			filter_options->sub[filter_options->sub_nr - 1].filter_wants =
    -+				filter_options->filter_wants;
    - 	}
    - 	if (parse_error)
    - 		die("%s", errbuf.buf);
    -
    - ## list-objects-filter-options.h ##
    -@@ list-objects-filter-options.h: struct list_objects_filter_options {
    - 	 */
    - 	enum list_objects_filter_choice choice;
    - 
    -+	/*
    -+	 * "--filter-provided" was given by the user, instructing us to also
    -+	 * filter all explicitly provided objects.
    -+	 */
    -+	unsigned int filter_wants : 1;
    -+
    - 	/*
    - 	 * Choice is LOFC_DISABLED because "--no-filter" was requested.
    - 	 */
    -
      ## pack-bitmap.c ##
    +@@ pack-bitmap.c: static int can_filter_bitmap(struct list_objects_filter_options *filter)
    + }
    + 
    + 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)
    + {
    + 	unsigned int i;
    + 
     @@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
      	if (haves_bitmap)
      		bitmap_and_not(wants_bitmap, haves_bitmap);
      
     -	filter_bitmap(bitmap_git, wants, wants_bitmap, filter);
    -+	filter_bitmap(bitmap_git, (filter && filter->filter_wants) ? NULL : wants,
    ++	filter_bitmap(bitmap_git, (filter && filter_provided_revs) ? NULL : wants,
     +		      wants_bitmap, filter);
      
      	bitmap_git->result = wants_bitmap;
      	bitmap_git->haves = haves_bitmap;
     
    + ## pack-bitmap.h ##
    +@@ pack-bitmap.h: void traverse_bitmap_commit_list(struct bitmap_index *,
    + 				 show_reachable_fn show_reachable);
    + void test_bitmap_walk(struct rev_info *revs);
    + 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 reuse_partial_packfile_from_bitmap(struct bitmap_index *,
    + 				       struct packed_git **packfile,
    + 				       uint32_t *entries,
    +
    + ## reachable.c ##
    +@@ reachable.c: void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
    + 	cp.progress = progress;
    + 	cp.count = 0;
    + 
    +-	bitmap_git = prepare_bitmap_walk(revs, NULL);
    ++	bitmap_git = prepare_bitmap_walk(revs, NULL, 0);
    + 	if (bitmap_git) {
    + 		traverse_bitmap_commit_list(bitmap_git, revs, mark_object_seen);
    + 		free_bitmap_index(bitmap_git);
    +
      ## t/t6112-rev-list-filters-objects.sh ##
     @@ t/t6112-rev-list-filters-objects.sh: test_expect_success 'verify object:type=tag prints tag' '
      	test_cmp expected actual
-- 
2.31.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]