Re: [PATCH v4 5/8] negotiator: add have_sent() interface

> From: Derrick Stolee <stolee@gmail.com>
> 
> In a future change, we will introduce a capability to choose specific commit
> OIDs as 'have's in fetch negotiation, with the ability to have the
> negotiator choose more 'have's to increase coverage beyond that required
> core set. The negotiator works to avoid emitting 'have's that can reach each
> other, but that logic is hidden beneath the negotiator's iterator function
> pointer ('next'). We need a way to communicate to the negotiator that we
> have picked a 'have' so it could incorporate that into its logic.
> 
> Add a have_sent() method to the fetch_negotiator interface. This is the
> signal that allows the negotiator to track the commit as already shown and
> can perform the proper bookkeeping to avoid emitting those objects or
> anything they can reach.
> 
> For our non-trivial negotiators, it is sufficient to mark these commits as
> common, so the implementation is quite simple. This logic will be exercised
> in the next change.
> 
> Signed-off-by: Derrick Stolee <stolee@gmail.com>

> ---
>   fetch-negotiator.h    | 9 +++++++++
>   negotiator/default.c  | 8 ++++++++
>   negotiator/noop.c     | 7 +++++++
>   negotiator/skipping.c | 8 ++++++++
>   4 files changed, 32 insertions(+)
> 
> diff --git a/fetch-negotiator.h b/fetch-negotiator.h
> index e348905a1f..6ca422a064 100644
> --- a/fetch-negotiator.h
> +++ b/fetch-negotiator.h
> @@ -47,6 +47,15 @@ struct fetch_negotiator {
>   	 */
>   	int (*ack)(struct fetch_negotiator *, struct commit *);
>   
> +	/*
> +	 * Inform the negotiator that this commit has already been sent as
> +	 * a "have" line outside of the negotiator's control. The negotiator
> +	 * should avoid outputting it from next() and may use it to optimize
> +	 * further negotiation (e.g., by treating it and its ancestors as
> +	 * common).
> +	 */
> +	void (*have_sent)(struct fetch_negotiator *, struct commit *);
> +

>   	void (*release)(struct fetch_negotiator *);
>   
>   	/* internal use */
> diff --git a/negotiator/default.c b/negotiator/default.c
> index 116dedcf83..05ab616f39 100644
> --- a/negotiator/default.c
> +++ b/negotiator/default.c
> @@ -175,6 +175,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
>   	return known_to_be_common;
>   }
>   
> +static void have_sent(struct fetch_negotiator *n, struct commit *c)
> +{
> +	if (repo_parse_commit(the_repository, c))
> +		return;
> +	mark_common(n->data, c, 0, 0);
> +}
> +
>   static void release(struct fetch_negotiator *n)
>   {
>   	clear_prio_queue(&((struct negotiation_state *)n->data)->rev_list);
> @@ -188,6 +195,7 @@ void default_negotiator_init(struct fetch_negotiator *negotiator)
>   	negotiator->add_tip = add_tip;
>   	negotiator->next = next;
>   	negotiator->ack = ack;
> +	negotiator->have_sent = have_sent;
>   	negotiator->release = release;
>   	negotiator->data = CALLOC_ARRAY(ns, 1);
>   	ns->rev_list.compare = compare_commits_by_commit_date;

> diff --git a/negotiator/noop.c b/negotiator/noop.c
> index 65e3c20008..edf1b456f3 100644
> --- a/negotiator/noop.c
> +++ b/negotiator/noop.c
> @@ -29,6 +29,12 @@ static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
>   	return 0;
>   }
>   
> +static void have_sent(struct fetch_negotiator *n UNUSED,
> +		      struct commit *c UNUSED)
> +{
> +	/* nothing to do */
> +}
> +
>   static void release(struct fetch_negotiator *n UNUSED)
>   {
>   	/* nothing to release */
> @@ -40,6 +46,7 @@ void noop_negotiator_init(struct fetch_negotiator *negotiator)
>   	negotiator->add_tip = add_tip;
>   	negotiator->next = next;
>   	negotiator->ack = ack;
> +	negotiator->have_sent = have_sent;
>   	negotiator->release = release;
>   	negotiator->data = NULL;
>   }

> diff --git a/negotiator/skipping.c b/negotiator/skipping.c
> index 0a272130fb..69472c58e1 100644
> --- a/negotiator/skipping.c
> +++ b/negotiator/skipping.c
> @@ -243,6 +243,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
>   	return known_to_be_common;
>   }
>   
> +static void have_sent(struct fetch_negotiator *n, struct commit *c)
> +{
> +	if (repo_parse_commit(the_repository, c))
> +		return;
> +	mark_common(n->data, c);
> +}
> +
>   static void release(struct fetch_negotiator *n)
>   {
>   	struct data *data = n->data;
> @@ -259,6 +266,7 @@ void skipping_negotiator_init(struct fetch_negotiator *negotiator)
>   	negotiator->add_tip = add_tip;
>   	negotiator->next = next;
>   	negotiator->ack = ack;
> +	negotiator->have_sent = have_sent;
>   	negotiator->release = release;
>   	negotiator->data = CALLOC_ARRAY(data, 1);
>   	data->rev_list.compare = compare;