Re: [PATCH v4 6/6] list-objects-filter: implement filter tree:0
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Aug 14, 2018, 18:18 UTC
- Message-ID
- <20180814181841.34446-1-jonathantanmy@google.com>
- In-Reply-To
- <05e69ef260ca2b28b2444b94850bddf44ca0388a.1534267611.git.matvore@google.com>
Show 10 quoted lines
> @@ -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.
Show 6 quoted lines
> +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 >
Show 10 quoted lines
> +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 >
Show 11 quoted lines
> + 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.
Show 21 quoted lines
> +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