| Message ID | cover.1617967252.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | rev-parse: implement object type filter | expand |
Patrick Steinhardt <ps@pks.im> writes: > Subject: Re: [PATCH v3 0/8] rev-parse: implement object type filter > > 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. Do you truly mean rev-parse, or is it just a typo for rev-list? > 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
On Sat, Apr 10, 2021 at 11:02:55PM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > Subject: Re: [PATCH v3 0/8] rev-parse: implement object type filter > > > > 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. > > Do you truly mean rev-parse, or is it just a typo for rev-list? It's a typo both in the series' title and here in the text. Patrick
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