Re: [PATCH v4] fetch-pack: support negotiation tip whitelist
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2019-06-18 13:36:37
On Tue, Jul 03 2018, Jonathan Tan wrote:
During negotiation, fetch-pack eventually reports as "have" lines all commits reachable from all refs. Allow the user to restrict the commits sent in this way by providing a whitelist of tips; only the tips themselves and their ancestors will be sent.
I discovered a bug in this...
quoted hunk ↗ jump to hunk
@@ -230,7 +246,7 @@ static int find_common(struct fetch_negotiator *negotiator, if (args->stateless_rpc && multi_ack == 1) die(_("--stateless-rpc requires multi_ack_detailed")); - for_each_ref(rev_list_insert_ref_oid, negotiator); + mark_tips(negotiator, args->negotiation_tips); for_each_cached_alternate(negotiator, insert_one_alternate_object); fetching = 0;
Here we blindly add objects found in an alternate repo. I found and
debugged this with this:
diff --git a/fetch-negotiator.h b/fetch-negotiator.h
index 9e3967ce66..cbe71c9c8d 100644
--- a/fetch-negotiator.h
+++ b/fetch-negotiator.h
@@ -33,2 +33,3 @@ struct fetch_negotiator {
void (*add_tip)(struct fetch_negotiator *, struct commit *);
+ int done_adding;
diff --git a/fetch-pack.c b/fetch-pack.c
index 3f24d0c8a6..6b43b4f8f1 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -238,2 +238,3 @@ static void mark_tips(struct fetch_negotiator *negotiator,
&negotiation_tips->oid[i]);
+ negotiator->done_adding = 1;
return;
diff --git a/negotiator/default.c b/negotiator/default.c
index 4b78f6bf36..4e45f05f25 100644
--- a/negotiator/default.c
+++ b/negotiator/default.c
@@ -137,2 +137,4 @@ static void add_tip(struct fetch_negotiator *n, struct commit *c)
{
+ if (n->done_adding)
+ return;
n->known_common = NULL;
@@ -166,2 +168,3 @@ void default_negotiator_init(struct fetch_negotiator *negotiator)
negotiator->add_tip = add_tip;
+ negotiator->done_adding = 0;
negotiator->next = next;
Perhaps something like that with an assert() is a good idea for the
negotiation backend code in general? It seems rather fragile to depend
on there being no other codepath that calls add_tip() again after some
other code (--negotiation-tip=*) that expects it not to be called again.