> - case LOFS_BEGIN_TREE: > - assert(obj->type == OBJ_TREE); > - /* always include all tree objects */ > - return LOFR_MARK_SEEN | LOFR_DO_SHOW; > - > case LOFS_END_TREE: > assert(obj->type == OBJ_TREE); > return LOFR_ZERO; > > + case LOFS_BEGIN_TREE: > + assert(obj->type == OBJ_TREE); > + if (!filter_data->omit_trees) > + return LOFR_MARK_SEEN | LOFR_DO_SHOW; > + > + /* > + * Fallthrough to insert into omitted list for trees as well as > + * blobs. > + */ > + /* fallthrough */ > case LOFS_BLOB: > - assert(obj->type == OBJ_BLOB); > assert((obj->flags & SEEN) == 0);
After looking at the resulting file, I don't think saving a few lines of code (to add the OID, then return LOFR_MARK_SEEN) is worth rearranging the cases and falling through. Can you just add the OID-adding code to the LOFS_BEGIN_TREE case? > +test_expect_success 'can use tree:none to filter partial clone' ' > + rm -rf dst && > + git clone --no-checkout --filter=tree:none "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 > +' We also need to verify that the resulting partial clone works - after all relevant tests, can you also ensure that: - fsck works - a cat-file on an indirectly missing tree works (i.e. if you have commit -> A -> B and both A and B are missing, cat-file the B) - fsck still works after the cat-file There is another potential issue about expanding the documentation of the pack protocol because we now support a new type of filter, but that is fine because the protocol currently points us to the rev-list documentation (which is updated). We probably need a way for clients to query servers about which filters they support, but that is definitely beyond the scope of this patch set. > +test_expect_success 'show missing tree objects with --missing=print' ' > + git -C dst rev-list master --missing=print --quiet --objects > >missing_objs && > + sed "s/?//" missing_objs \ > + | xargs -n1 git -C srv.bare cat-file -t \ > + >missing_types && > + sort -u missing_types >missing_types.uniq && > + echo tree >expected && > + test_cmp missing_types.uniq expected > +' As stated in my review of patch 3, also test the other --missing arguments. Patches 1, 2, and 4 look good to me. (Writing this here so that I don't need to send one e-mail for each.)
