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

[Jonathan Tan <jonathantanmy@google.com>]

> @@ -743,6 +743,9 @@ specification contained in <path>.
>  	A debug option to help with future "partial clone" development.
>  	This option specifies how missing objects are handled.
>  +
> +The form '--filter=tree:<depth>' omits all blobs and trees deeper than
> +<depth> from the root tree. Currently, only <depth>=0 is supported.
> ++
>  The form '--missing=error' requests that rev-list stop with an error if
>  a missing object is encountered.  This is the default action.
>  +

The "--filter" documentation should go with the other "--filter"
information, not right after --missing.

> +test_expect_success 'setup for tests of tree:0' '
> +	mkdir r1/subtree &&
> +	echo "This is a file in a subtree" > r1/subtree/file &&
> +	git -C r1 add subtree/file &&
> +	git -C r1 commit -m subtree
> +'

Style: no space after >

> +test_expect_success 'grab tree directly when using tree:0' '
> +	# We should get the tree specified directly but not its blobs or subtrees.
> +	git -C r1 pack-objects --rev --stdout --filter=tree:0 >commitsonly.pack <<-EOF &&
> +	HEAD:
> +	EOF
> +	git -C r1 index-pack ../commitsonly.pack &&
> +	git -C r1 verify-pack -v ../commitsonly.pack >objs &&
> +	grep -E "tree|blob" objs >trees_and_blobs &&
> +	test_line_count = 1 trees_and_blobs
> +'

Can we also verify that the SHA-1 in trees_and_blobs is what we
expected?

> +test_expect_success 'use fsck before and after manually fetching a missing subtree' '
> +	# push new commit so server has a subtree
> +	mkdir src/dir &&
> +	echo "in dir" > src/dir/file.txt &&

No space after >

> +	git -C src add dir/file.txt &&
> +	git -C src commit -m "file in dir" &&
> +	git -C src push -u srv master &&
> +	SUBTREE=$(git -C src rev-parse HEAD:dir) &&
> +
> +	rm -rf dst &&
> +	git clone --no-checkout --filter=tree:0 "file://$(pwd)/srv.bare" dst &&
> +	git -C dst fsck &&
> +	git -C dst cat-file -p $SUBTREE >tree_contents 2>err &&
> +	git -C dst fsck
> +'

If you don't need to redirect to err, don't do so.

Before the cat-file, also verify that the tree is missing, most likely
through a "git rev-list" with "--missing=print".

And I would grep on the tree_contents to ensure that the filename
("file.txt") is there, so that we know that we got the correct tree.

> +test_expect_success 'can use tree:0 to filter partial clone' '
> +	rm -rf dst &&
> +	git clone --no-checkout --filter=tree:0 "file://$(pwd)/srv.bare" dst &&
> +	git -C dst rev-list master --missing=allow-any --objects >fetched_objects &&
> +	cat fetched_objects \
> +		| awk -f print_1.awk \
> +		| xargs -n1 git -C dst cat-file -t >fetched_types &&
> +	sort fetched_types -u >unique_types.observed &&
> +	echo commit > unique_types.expected &&
> +	test_cmp unique_types.observed unique_types.expected
> +'
> +
> +test_expect_success 'auto-fetching of trees with --missing=error' '
> +	git -C dst rev-list master --missing=error --objects >fetched_objects &&
> +	cat fetched_objects \
> +		| awk -f print_1.awk \
> +		| xargs -n1 git -C dst cat-file -t >fetched_types &&
> +	sort fetched_types -u >unique_types.observed &&
> +	printf "blob\ncommit\ntree\n" >unique_types.expected &&
> +	test_cmp unique_types.observed unique_types.expected
> +'

These two tests seem redundant with the 'use fsck before and after
manually fetching a missing subtree' test (after the latter is
appropriately renamed). I think we only need to test this sequence once,
which can be placed in one or spread over multiple tests:

 1. partial clone with --filter=tree:0
 2. fsck works
 3. verify that trees are indeed missing
 4. autofetch a tree
 5. fsck still works