From e987df5fe62b8b29be4cdcdeb3704681ada2b29e Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:08 -0700 Subject: list-objects-filter: implement composite filters Allow combining filters such that only objects accepted by all filters are shown. The motivation for this is to allow getting directory listings without also fetching blobs. This can be done by combining blob:none with tree:. There are massive repositories that have larger-than-expected trees - even if you include only a single commit. A combined filter supports any number of subfilters, and is written in the following form: combine:++ Certain non-alphanumeric characters in each filter must be URL-encoded. For now, combined filters must be specified in this form. In a subsequent commit, rev-list will support multiple --filter arguments which will have the same effect as specifying one filter argument starting with "combine:". The documentation will be updated in that commit, as the URL-encoding scheme is in general not meant to be used directly by the user, and it is better to describe the URL-encoding feature in terms of the repeated flag. Helped-by: Emily Shaffer Helped-by: Jeff Hostetler Helped-by: Johannes Schindelin Helped-by: Jonathan Tan Helped-by: Junio C Hamano Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- t/t6112-rev-list-filters-objects.sh | 151 +++++++++++++++++++++++++++++++++++- 1 file changed, 150 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh index acd7f5ab80..05d4f2e9c2 100755 --- a/t/t6112-rev-list-filters-objects.sh +++ b/t/t6112-rev-list-filters-objects.sh @@ -278,7 +278,19 @@ test_expect_success 'verify skipping tree iteration when not collecting omits' ' test_line_count = 2 actual && # Make sure no other trees were considered besides the root. - ! grep "Skipping contents of tree [^.]" filter_trace + ! grep "Skipping contents of tree [^.]" filter_trace && + + # Try this again with "combine:". If both sub-filters are skipping + # trees, the composite filter should also skip trees. This is not + # important unless the user does combine:tree:X+tree:Y or another filter + # besides "tree:" is implemented in the future which can skip trees. + GIT_TRACE=1 git -C r3 rev-list \ + --objects --filter=combine:tree:1+tree:3 HEAD 2>filter_trace && + + # Only skip the dir1/ tree, which is shared between the two commits. + grep "Skipping contents of tree " filter_trace >actual && + test_write_lines "Skipping contents of tree dir1/..." >expected && + test_cmp expected actual ' # Test tree:# filters. @@ -330,6 +342,112 @@ test_expect_success 'verify tree:3 includes everything expected' ' test_line_count = 10 actual ' +test_expect_success 'combine:... for a simple combination' ' + git -C r3 rev-list --objects --filter=combine:tree:2+blob:none HEAD \ + >actual && + + expect_has HEAD "" && + expect_has HEAD~1 "" && + expect_has HEAD dir1 && + + # There are also 2 commit objects + test_line_count = 5 actual +' + +test_expect_success 'combine:... with URL encoding' ' + git -C r3 rev-list --objects \ + --filter=combine:tree%3a2+blob:%6Eon%65 HEAD >actual && + + expect_has HEAD "" && + expect_has HEAD~1 "" && + expect_has HEAD dir1 && + + # There are also 2 commit objects + test_line_count = 5 actual +' + +expect_invalid_filter_spec () { + spec="$1" && + err="$2" && + + test_must_fail git -C r3 rev-list --objects --filter="$spec" HEAD \ + >actual 2>actual_stderr && + test_must_be_empty actual && + test_i18ngrep "$err" actual_stderr +} + +test_expect_success 'combine:... while URL-encoding things that should not be' ' + expect_invalid_filter_spec combine%3Atree:2+blob:none \ + "invalid filter-spec" +' + +test_expect_success 'combine: with nothing after the :' ' + expect_invalid_filter_spec combine: "expected something after combine:" +' + +test_expect_success 'parse error in first sub-filter in combine:' ' + expect_invalid_filter_spec combine:tree:asdf+blob:none \ + "expected .tree:." +' + +test_expect_success 'combine:... with non-encoded reserved chars' ' + expect_invalid_filter_spec combine:tree:2+sparse:@xyz \ + "must escape char in sub-filter-spec: .@." && + expect_invalid_filter_spec combine:tree:2+sparse:\` \ + "must escape char in sub-filter-spec: .\`." && + expect_invalid_filter_spec combine:tree:2+sparse:~abc \ + "must escape char in sub-filter-spec: .\~." +' + +test_expect_success 'validate err msg for "combine:+"' ' + expect_invalid_filter_spec combine:tree:2+ "expected .tree:." +' + +test_expect_success 'combine:... with edge-case hex digits: Ff Aa 0 9' ' + git -C r3 rev-list --objects --filter="combine:tree:2+bl%6Fb:n%6fne" \ + HEAD >actual && + test_line_count = 5 actual && + git -C r3 rev-list --objects --filter="combine:tree%3A2+blob%3anone" \ + HEAD >actual && + test_line_count = 5 actual && + git -C r3 rev-list --objects --filter="combine:tree:%30" HEAD >actual && + test_line_count = 2 actual && + git -C r3 rev-list --objects --filter="combine:tree:%39+blob:none" \ + HEAD >actual && + test_line_count = 5 actual +' + +test_expect_success 'add a sparse pattern blob whose path has reserved chars' ' + cp r3/pattern r3/pattern1+renamed% && + git -C r3 add pattern1+renamed% && + git -C r3 commit -m "add sparse pattern file with reserved chars" +' + +test_expect_success 'combine:... with more than two sub-filters' ' + git -C r3 rev-list --objects \ + --filter=combine:tree:3+blob:limit=40+sparse:oid=master:pattern \ + HEAD >actual && + + expect_has HEAD "" && + expect_has HEAD~1 "" && + expect_has HEAD~2 "" && + expect_has HEAD dir1 && + expect_has HEAD dir1/sparse1 && + expect_has HEAD dir1/sparse2 && + + # Should also have 3 commits + test_line_count = 9 actual && + + # Try again, this time making sure the last sub-filter is only + # URL-decoded once. + cp actual expect && + + git -C r3 rev-list --objects \ + --filter=combine:tree:3+blob:limit=40+sparse:oid=master:pattern1%2brenamed%25 \ + HEAD >actual && + test_cmp expect actual +' + # Test provisional omit collection logic with a repo that has objects appearing # at multiple depths - first deeper than the filter's threshold, then shallow. @@ -373,6 +491,37 @@ test_expect_success 'verify skipping tree iteration when collecting omits' ' test_cmp expect actual ' +test_expect_success 'setup r5' ' + git init r5 && + mkdir -p r5/subdir && + + echo 1 >r5/short-root && + echo 12345 >r5/long-root && + echo a >r5/subdir/short-subdir && + echo abcde >r5/subdir/long-subdir && + + git -C r5 add short-root long-root subdir && + git -C r5 commit -m "commit msg" +' + +test_expect_success 'verify collecting omits in combined: filter' ' + # Note that this test guards against the naive implementation of simply + # giving both filters the same "omits" set and expecting it to + # automatically merge them. + git -C r5 rev-list --objects --quiet --filter-print-omitted \ + --filter=combine:tree:2+blob:limit=3 HEAD >actual && + + # Expect 0 trees/commits, 3 blobs omitted (all blobs except short-root) + omitted_1=$(echo 12345 | git hash-object --stdin) && + omitted_2=$(echo a | git hash-object --stdin) && + omitted_3=$(echo abcde | git hash-object --stdin) && + + grep ~$omitted_1 actual && + grep ~$omitted_2 actual && + grep ~$omitted_3 actual && + test_line_count = 3 actual +' + # Test tree: where a tree is iterated to twice - once where a subentry is # too deep to be included, and again where the blob inside it is shallow enough # to be included. This makes sure we don't use LOFR_MARK_SEEN incorrectly (we -- cgit v1.2.3 From cf9ceb5a12cad9c9153d227a0f497d1b522ce085 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:10 -0700 Subject: list-objects-filter-options: make filter_spec a string_list Make the filter_spec string a string_list rather than a raw C string. The list of strings must be concatted together to make a complete filter_spec. A future patch will use this capability to build "combine:" filter specs gradually. A strbuf would seem to be a more natural choice for this object, but it unfortunately requires initialization besides just zero'ing out the memory. This results in all container structs, and all containers of those structs, etc., to also require initialization. Initializing them all would be more cumbersome that simply using a string_list, which behaves properly when its contents are zero'd. For the purposes of code simplification, change behavior in how filter specs are conveyed over the protocol: do not normalize the tree: filter specs since there should be no server in existence that supports tree:# but not tree:#k etc. Helped-by: Junio C Hamano Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- t/t6112-rev-list-filters-objects.sh | 7 ------- 1 file changed, 7 deletions(-) (limited to 't') diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh index 05d4f2e9c2..27ba15719a 100755 --- a/t/t6112-rev-list-filters-objects.sh +++ b/t/t6112-rev-list-filters-objects.sh @@ -590,11 +590,4 @@ test_expect_success 'expand blob limit in protocol' ' grep "blob:limit=1024" trace ' -test_expect_success 'expand tree depth limit in protocol' ' - GIT_TRACE_PACKET="$(pwd)/tree_trace" git -c protocol.version=2 clone \ - --filter=tree:0k "file://$(pwd)/r2" tree && - ! grep "tree:0k" tree_trace && - grep "tree:0" tree_trace -' - test_done -- cgit v1.2.3 From 489fc9ee718b7c8594f17b55f090ac5292c655e1 Mon Sep 17 00:00:00 2001 From: Matthew DeVore Date: Thu, 27 Jun 2019 15:54:12 -0700 Subject: list-objects-filter-options: allow mult. --filter Allow combining of multiple filters by simply repeating the --filter flag. Before this patch, the user had to combine them in a single flag somewhat awkwardly (e.g. --filter=combine:FOO+BAR), including URL-encoding the individual filters. To make this work, in the --filter flag parsing callback, rather than error out when we detect that the filter_options struct is already populated, we modify it in-place to contain the added sub-filter. The existing sub-filter becomes the lhs of the combined filter, and the next sub-filter becomes the rhs. We also have to URL-encode the LHS and RHS sub-filters. We can simplify the operation if the LHS is already a combine: filter. In that case, we just append the URL-encoded RHS sub-filter to the LHS spec to get the new spec. Helped-by: Emily Shaffer Helped-by: Jeff Hostetler Helped-by: Jeff King Helped-by: Junio C Hamano Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- t/t5616-partial-clone.sh | 19 +++++++++++++++ t/t6112-rev-list-filters-objects.sh | 46 +++++++++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 5 deletions(-) (limited to 't') diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index b91ef548f8..32b7d72f3c 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -208,6 +208,25 @@ test_expect_success 'use fsck before and after manually fetching a missing subtr test_cmp unique_types.expected unique_types.observed ' +test_expect_success 'implicitly construct combine: filter with repeated flags' ' + GIT_TRACE=$(pwd)/trace git clone --bare \ + --filter=blob:none --filter=tree:1 \ + "file://$(pwd)/srv.bare" pc2 && + grep "trace:.* git pack-objects .*--filter=combine:blob:none+tree:1" \ + trace && + git -C pc2 rev-list --objects --missing=allow-any HEAD >objects && + + # We should have gotten some root trees. + grep " $" objects && + # Should not have gotten any non-root trees or blobs. + ! grep " ." objects && + + xargs -n 1 git -C pc2 cat-file -t types && + sort -u types >unique_types.actual && + test_write_lines commit tree >unique_types.expected && + test_cmp unique_types.expected unique_types.actual +' + test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' ' rm -rf src dst && git init src && diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh index 27ba15719a..de0e5a5d36 100755 --- a/t/t6112-rev-list-filters-objects.sh +++ b/t/t6112-rev-list-filters-objects.sh @@ -351,7 +351,16 @@ test_expect_success 'combine:... for a simple combination' ' expect_has HEAD dir1 && # There are also 2 commit objects - test_line_count = 5 actual + test_line_count = 5 actual && + + cp actual expected && + + # Try again using repeated --filter - this is equivalent to a manual + # combine with "combine:...+..." + git -C r3 rev-list --objects --filter=combine:tree:2 \ + --filter=blob:none HEAD >actual && + + test_cmp expected actual ' test_expect_success 'combine:... with URL encoding' ' @@ -417,10 +426,12 @@ test_expect_success 'combine:... with edge-case hex digits: Ff Aa 0 9' ' test_line_count = 5 actual ' -test_expect_success 'add a sparse pattern blob whose path has reserved chars' ' +test_expect_success 'add sparse pattern blobs whose paths have reserved chars' ' cp r3/pattern r3/pattern1+renamed% && - git -C r3 add pattern1+renamed% && - git -C r3 commit -m "add sparse pattern file with reserved chars" + cp r3/pattern "r3/p;at%ter+n" && + cp r3/pattern r3/^~pattern && + git -C r3 add pattern1+renamed% "p;at%ter+n" ^~pattern && + git -C r3 commit -m "add sparse pattern files with reserved chars" ' test_expect_success 'combine:... with more than two sub-filters' ' @@ -445,7 +456,32 @@ test_expect_success 'combine:... with more than two sub-filters' ' git -C r3 rev-list --objects \ --filter=combine:tree:3+blob:limit=40+sparse:oid=master:pattern1%2brenamed%25 \ HEAD >actual && - test_cmp expect actual + test_cmp expect actual && + + # Use the same composite filter again, but with a pattern file name that + # requires encoding multiple characters, and use implicit filter + # combining. + test_when_finished "rm -f trace1" && + GIT_TRACE=$(pwd)/trace1 git -C r3 rev-list --objects \ + --filter=tree:3 --filter=blob:limit=40 \ + --filter=sparse:oid="master:p;at%ter+n" \ + HEAD >actual && + + test_cmp expect actual && + grep "Add to combine filter-spec: sparse:oid=master:p%3bat%25ter%2bn" \ + trace1 && + + # Repeat the above test, but this time, the characters to encode are in + # the LHS of the combined filter. + test_when_finished "rm -f trace2" && + GIT_TRACE=$(pwd)/trace2 git -C r3 rev-list --objects \ + --filter=sparse:oid=master:^~pattern \ + --filter=tree:3 --filter=blob:limit=40 \ + HEAD >actual && + + test_cmp expect actual && + grep "Add to combine filter-spec: sparse:oid=master:%5e%7epattern" \ + trace2 ' # Test provisional omit collection logic with a repo that has objects appearing -- cgit v1.2.3