Re: [PATCH 2/3] pack-objects: WIP add max-blob-size filtering

[Jonathan Tan <jonathantanmy@google.com>]

On Thu, 22 Jun 2017 20:36:14 +0000
Jeff Hostetler <git@jeffhostetler.com> wrote:

> +static signed long max_blob_size = -1;

FYI Junio suggested "blob-max-bytes" when he looked at my patch [1].

[1] https://public-inbox.org/git/xmqqmv9ryoym.fsf@gitster.mtv.corp.google.com/

[snip]

> +/*
> + * Filter blobs by pathname or size.
> + * Return 1 to mark the blob SEEN so that it will not be reported again.
> + * Return 0 to allow it to be presented again.
> + */
> +static int filter_blob(
> +	struct object *obj,
> +	const char *pathname,
> +	const char *entryname,
> +	void *data)
> +{
> +	assert(obj->type == OBJ_BLOB);
> +	assert((obj->flags & SEEN) == 0);
> +	assert((obj->flags & OBJECT_ADDED) == 0);
> +	assert(max_blob_size >= 0);
> +
> +	/*
> +	 * Always include blobs for special files of the form ".git*".
> +	 */
> +	if ((strncmp(entryname, ".git", 4) == 0) && entryname[4]) {
> +		if (obj->flags & BLOB_OMITTED) {
> +			/*
> +			 * TODO
> +			 * TODO Remove this blob from the omitted blob list.
> +			 * TODO
> +			 */
> +			obj->flags &= ~BLOB_OMITTED;
> +		}
> +		show_object(obj, pathname, data);
> +		return 1;
> +	}
> +
> +	/*
> +	 * We already know the blob is too big because it was previously
> +	 * omitted.  We still don't want it yet.  DO NOT mark it SEEN
> +	 * in case it is associated with a ".git*" path in another tree
> +	 * or commit.
> +	 */
> +	if (obj->flags & BLOB_OMITTED)
> +		return 0;
> +
> +	/*
> +	 * We only want blobs that are LESS THAN the maximum.
> +	 * This allows zero to mean NO BLOBS.

That is not what maximum means...

This is the reason why I originally called it "limit", but after some
reflection, I decided that it is no big deal to always send zero-sized
blobs. The client must be able to handle the presence of blobs anyway
(because it will receive the ".git" ones), and excluding all blobs
regardless of size does not remove the necessity of obtaining their
sizes, since we need those sizes to put in the omitted blob list.

> +	 */
> +	if (max_blob_size > 0) {
> +		unsigned long s;
> +		enum object_type t = sha1_object_info(obj->oid.hash, &s);
> +		assert(t == OBJ_BLOB);
> +		if (s < max_blob_size) {
> +			show_object(obj, pathname, data);
> +			return 1;
> +		}
> +	}
> +
> +	/*
> +	 * TODO
> +	 * TODO (Provisionally) add this blob to the omitted blob list.
> +	 * TODO

As for the omitted blob list itself, you can see from my patch [2] that I
used a hashmap, but the decorate.h functionality might work too (I
haven't looked into that in detail though).

[2] https://public-inbox.org/git/6f7934621717141ce3bb6bc05cf1d59c7900ccc5.1496432147.git.jonathantanmy@google.com/