Re: [PATCH v2 5/5] list-objects-filter: implement filter tree:none
From: Jonathan Tan <hidden>
Date: 2018-08-13 18:29:58
quoted hunk ↗ jump to hunk
- 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?
quoted hunk ↗ jump to hunk
+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.
quoted hunk ↗ jump to hunk
+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.)