Re: [PATCH v2 5/5] list-objects-filter: implement filter tree:none

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -	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.)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux