Re: [PATCH v6 6/6] list-objects-filter: implement filter tree:0

On Fri, Aug 17, 2018 at 2:42 PM Stefan Beller [off-list ref] wrote:

On Wed, Aug 15, 2018 at 4:23 PM Matthew DeVore [off-list ref] wrote:

quoted

Teach list-objects the "tree:0" filter which allows for filtering
out all tree and blob objects (unless other objects are explicitly
specified by the user). The purpose of this patch is to allow smaller
partial clones.

The name of this filter - tree:0 - does not explicitly specify that
it also filters out all blobs, but this should not cause much confusion
because blobs are not at all useful without the trees that refer to
them.

I also consider only:commits as a name, but this is inaccurate because
it suggests that annotated tags are omitted, but actually they are
included.

Speaking of tag objects, it is possible to tag anything, including blobs.
Would a blob that is tagged (hence reachable without a tree) be not
filtered by tree:0 (or in the future any deeper depth) ?

I think so. If I try to fetch a tagged tree or blob, it should fetch
that object itself, since I'm referring to it explicitly in the git
pack-objects arguments (I mention fetch since git rev-list apparently
doesn't support specifying non-commits on the command line). This is
similar to how I can fetch a commit that would otherwise be filtered
*if* I specify it explicitly (rather than a child commit).

If you're fetching a tagged tree, then for depth=0, it will fetch the
given tree only, and not fetch any referents of an explicitly-given
tree. For depth=1, it will fetch all direct referents.

If you're fetching a commit, then for depth=0, you will not get any
tree objects, and for depth=1, you'll get only the root tree object
and none of its referrents. So the commit itself is like a "layer" in
the depth count.

I found this series a good read, despite my unfamiliarity of the
partial cloning.

One situation where I scratched my head for a second were previous patches
that  use "test_line_count = 0 rev_list_err" whereas using test_must_be_empty
would be an equally good choice (I am more used to the latter than the former)

Done. Here is an interdiff (sorry, the tab characters are not
maintained by my mail client):

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index a1b93c72c..7e2f7ff26 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh

@@ -200,14 +200,14 @@ test_expect_success 'missing tree objects with
--missing=allow-promisor and --ex
  git -C repo config extensions.partialclone "arbitrary string" &&

  git -C repo rev-list --missing=allow-promisor --objects HEAD >objs

2>rev_list_err &&
- test_line_count = 0 rev_list_err &&
+ test_must_be_empty rev_list_err &&
  # 3 commits, 3 blobs, and 1 tree
  test_line_count = 7 objs &&

  # Do the same for --exclude-promisor-objects, but with all trees gone.
  promise_and_delete $(git -C repo rev-parse baz^{tree}) &&
  git -C repo rev-list --exclude-promisor-objects --objects HEAD >objs
2>rev_list_err &&
- test_line_count = 0 rev_list_err &&
+ test_must_be_empty rev_list_err &&
  # 3 commits, no blobs or trees
  test_line_count = 3 objs
 '

@@ -226,7 +226,7 @@ test_expect_success 'missing non-root tree object

and rev-list' '
  git -C repo config extensions.partialclone "arbitrary string" &&

  git -C repo rev-list --missing=allow-any --objects HEAD >objs
2>rev_list_err &&
- test_line_count = 0 rev_list_err &&
+ test_must_be_empty rev_list_err &&
  # 1 commit and 1 tree
  test_line_count = 2 objs
 '

diff --git a/t/t6112-rev-list-filters-objects.sh

b/t/t6112-rev-list-filters-objects.sh
index 30bf1c73e..27040d73a 100755

--- a/t/t6112-rev-list-filters-objects.sh
+++ b/t/t6112-rev-list-filters-objects.sh

@@ -206,11 +206,11 @@ test_expect_success 'rev-list W/ --missing=print

and --missing=allow-any for tre
  test_cmp expected missing_objs &&

  # do not complain when a missing tree cannot be parsed
- test_line_count = 0 rev_list_err &&
+ test_must_be_empty rev_list_err &&

  git -C r3 rev-list --missing=allow-any --objects HEAD >objs 2>rev_list_err &&
  ! grep $TREE objs &&
- test_line_count = 0 rev_list_err
+ test_must_be_empty rev_list_err
 '

 # Test tree:0 filter.

Thanks,
Stefan