Re: [RFC PATCH] During a shallow fetch, prevent sending over unneeded objects

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 11, 2013, 22:53 UTC
Message-ID: <7vsizkpv21.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130711220127.GK10217@login.drsnuggles.stderr.nl>

Matthijs Kooijman <matthijs@stdin.nl> writes:

[administrivia: you seem to have mail-followup-to that points at you and the list; is that really needed???]

> This happens when a client issues a fetch with a depth bigger or equal
> to the number of commits the server is ahead of the client.

Do you mean "smaller" (not "bigger")?

Show 16 quoted lines

> diff --git a/upload-pack.c b/upload-pack.c
> index 59f43d1..5885f33 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -122,6 +122,14 @@ static int do_rev_list(int in, int out, void *user_data)
>  	if (prepare_revision_walk(&revs))
>  		die("revision walk setup failed");
>  	mark_edges_uninteresting(revs.commits, &revs, show_edge);
> +	/* In case we create a new shallow root, make sure that all
> +	 * we don't send over objects that the client already has just
> +	 * because their "have" revisions are no longer reachable from
> +	 * the shallow root. */
> +	for (i = 0; i < have_obj.nr; i++) {
> +		struct commit *commit = (struct commit *)have_obj.objects[i].item;
> +		mark_tree_uninteresting(commit->tree);
> +	}

Hmph.

In your discussion (including the comment), you talk about "shallow root" (I think that is the same as what we call "shallow boundary"), but in this added block, there is nothing that checks CLIENT_SHALLOW or SHALLOW flags to special case that.

Is it a good idea to unconditionally do this for all "have" revisions?

Also there is another loop that iterates over "have" revisions just above the precontext. I wonder if this added code belongs in that loop.