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

From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Christian Couder <christian.couder@gmail.com>,
	Taylor Blau <me@ttaylorr.com>, Jeff King <peff@peff.net>
Subject: [PATCH v2 0/8] rev-parse: implement object type filter
Date: Mon, 15 Mar 2021 14:14:26 +0100	[thread overview]
Message-ID: <cover.1615813673.git.ps@pks.im> (raw)
In-Reply-To: <cover.1614600555.git.ps@pks.im>

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

Hi,

this is the second 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.

Changes compared to v1:

    - I've added a patch up front which changes the uploadpack
      documentation to explicitly document that setting
      `uploadpackfilter.allow=true` will enable all future filters. I'm
      not yet saying that this is the correct thing to do, but rather
      added this patch such that we have a proper place to discuss this
      topic. In the context of object-type filters, I do think though
      that it's not an issue to default-enable type filters: they're not
      expensive to compute anyway.

    - `uploadpackfilter.<filter>.allow` documentation was updated to
      mention the new filter.

    - A bug was fixed which caused us to reset `--filter-allowed` in
      case a normal filter was converted to a combined filter. I've
      added tests to more thoroughly verify that filters work as
      expected and also filter provided objects.

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  |   3 +
 builtin/rev-list.c                  |  14 ++++
 list-objects-filter-options.c       |  18 +++++
 list-objects-filter-options.h       |   8 ++
 list-objects-filter.c               | 116 ++++++++++++++++++++++++++++
 list-objects-filter.h               |   2 +
 list-objects.c                      |  32 +++++++-
 pack-bitmap.c                       |  71 +++++++++++++++--
 revision.c                          |   4 +-
 revision.h                          |   3 -
 t/t6112-rev-list-filters-objects.sh |  76 ++++++++++++++++++
 t/t6113-rev-list-bitmap-filters.sh  |  68 +++++++++++++++-
 13 files changed, 403 insertions(+), 21 deletions(-)

Range-diff against v1:
-:  ---------- > 1:  270ff80dac uploadpack.txt: document implication of `uploadpackfilter.allow`
1:  f2ce5dac89 = 2:  ddbec75986 revision: mark commit parents as NOT_USER_GIVEN
2:  9feadba124 = 3:  d8da0b24f4 list-objects: move tag processing into its own function
3:  4aa13ee83f = 4:  5545c189c5 list-objects: support filtering by tag and commit
4:  01b9fdbb9c ! 5:  acf01472af list-objects: implement object type filter
    @@ Commit message

         Signed-off-by: Patrick Steinhardt <ps@pks.im>

    + ## Documentation/config/uploadpack.txt ##
    +@@ Documentation/config/uploadpack.txt: uploadpackfilter.allow::
    + uploadpackfilter.<filter>.allow::
    + 	Explicitly allow or ban the object filter corresponding to
    + 	`<filter>`, where `<filter>` may be one of: `blob:none`,
    +-	`blob:limit`, `tree`, `sparse:oid`, or `combine`. If using
    +-	combined filters, both `combine` and all of the nested filter
    +-	kinds must be allowed. Defaults to `uploadpackfilter.allow`.
    ++	`blob:limit`, `object:type`, `tree`, `sparse:oid`, or `combine`.
    ++	If using combined filters, both `combine` and all of the nested
    ++	filter kinds must be allowed. Defaults to `uploadpackfilter.allow`.
    + 
    + uploadpackfilter.tree.maxDepth::
    + 	Only allow `--filter=tree:<n>` when `<n>` is no more than the value of
    +
      ## Documentation/rev-list-options.txt ##
     @@ Documentation/rev-list-options.txt: or units.  n may be zero.  The suffixes k, m, and g can be used to name
      units in KiB, MiB, or GiB.  For example, 'blob:limit=1k' is the same
5:  c97fd28d8f = 6:  8073ab665b pack-bitmap: implement object type filter
6:  fe2b7a1e55 = 7:  fac3477d97 pack-bitmap: implement combined filter
7:  b43bf401df ! 8:  0e26fee8b3 rev-list: allow filtering of provided items
    @@ 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 {
      	 */
    @@ t/t6113-rev-list-bitmap-filters.sh: test_expect_success 'object:type filter' '
      '

     +test_expect_success 'object:type filter with --filter-provided' '
    -+	git rev-list --objects --filter=object:type=tag --filter-provided tag >expect &&
    ++	git rev-list --objects --filter-provided --filter=object:type=tag tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter=object:type=tag --filter-provided tag >actual &&
    ++		     --objects --filter-provided --filter=object:type=tag tag >actual &&
     +	test_cmp expect actual &&
     +
    -+	git rev-list --objects --filter=object:type=commit --filter-provided tag >expect &&
    ++	git rev-list --objects --filter-provided --filter=object:type=commit tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter=object:type=commit --filter-provided tag >actual &&
    ++		     --objects --filter-provided --filter=object:type=commit tag >actual &&
     +	test_bitmap_traversal expect actual &&
     +
    -+	git rev-list --objects --filter=object:type=tree --filter-provided tag >expect &&
    ++	git rev-list --objects --filter-provided --filter=object:type=tree tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter=object:type=tree --filter-provided tag >actual &&
    ++		     --objects --filter-provided --filter=object:type=tree tag >actual &&
     +	test_bitmap_traversal expect actual &&
     +
    -+	git rev-list --objects --filter=object:type=blob --filter-provided tag >expect &&
    ++	git rev-list --objects --filter-provided --filter=object:type=blob tag >expect &&
     +	git rev-list --use-bitmap-index \
    -+		     --objects --filter=object:type=blob --filter-provided tag >actual &&
    ++		     --objects --filter-provided --filter=object:type=blob tag >actual &&
     +	test_bitmap_traversal expect actual
     +'
     +
      test_expect_success 'combine filter' '
      	git rev-list --objects --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
      	git rev-list --use-bitmap-index \
    +@@ 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' '
    ++	git rev-list --objects --filter-provided --filter=blob:limit=1000 --filter=object:type=blob tag >expect &&
    ++	git rev-list --use-bitmap-index \
    ++		     --objects --filter-provided --filter=blob:limit=1000 --filter=object:type=blob tag >actual &&
    ++	test_bitmap_traversal expect actual &&
    ++
    ++	git cat-file --batch-check="%(objecttype) %(objectsize)" <actual >objects &&
    ++	while read objecttype objectsize
    ++	do
    ++		test "$objecttype" = blob || return 1
    ++		test "$objectsize" -le 1000 || return 1
    ++	done <objects
    ++'
    ++
    + test_done
-- 
2.30.2

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