From: Matthew John Cheetham Date: Tue, 12 May 2026 15:14:49 GMT Subject: Re: [PATCH v3 7/7] send-pack: pass negotiation config in push Message-ID: In-Reply-To: On 2026-04-22 16:25, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee > > When push.negotiate is enabled, 'git push' spawns a child 'git fetch > --negotiate-only' process to find common commits. Pass > --negotiation-include and --negotiation-restrict options from the > 'remote..negotiationInclude' and > 'remote..negotiationRestrict' config keys to this child process. > > When negotiationRestrict is configured, it replaces the default > behavior of using all remote refs as negotiation tips. This allows > the user to control which local refs are used for push negotiation. > > When negotiationInclude is configured, the specified ref patterns > are passed as --negotiation-include to ensure their tips are always > sent as 'have' lines during push negotiation. > > This change also updates the use of --negotiation-tip into > --negotiation-restrict now that the new synonym exists. > > Signed-off-by: Derrick Stolee > --- > send-pack.c | 39 +++++++++++++++++++++++++++++++-------- > send-pack.h | 2 ++ > t/t5516-fetch-push.sh | 30 ++++++++++++++++++++++++++++++ > transport.c | 2 ++ > 4 files changed, 65 insertions(+), 8 deletions(-) This patch wires up the negotiation behaviour with push, added in the previous patches. > diff --git a/send-pack.c b/send-pack.c > index 67d6987b1c..d18e030ce8 100644 > --- a/send-pack.c > +++ b/send-pack.c > @@ -433,28 +433,48 @@ static void reject_invalid_nonce(const char *nonce, int len) > > static void get_commons_through_negotiation(struct repository *r, > const char *url, > + const struct string_list *negotiation_include, > + const struct string_list *negotiation_restrict, > const struct ref *remote_refs, > struct oid_array *commons) > { > struct child_process child = CHILD_PROCESS_INIT; > const struct ref *ref; > int len = r->hash_algo->hexsz + 1; /* hash + NL */ > - int nr_negotiation_tip = 0; > + int nr_negotiation = 0; > > child.git_cmd = 1; > child.no_stdin = 1; > child.out = -1; > strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL); > - for (ref = remote_refs; ref; ref = ref->next) { > - if (!is_null_oid(&ref->new_oid)) { > - strvec_pushf(&child.args, "--negotiation-tip=%s", > - oid_to_hex(&ref->new_oid)); > - nr_negotiation_tip++; > + > + if (negotiation_restrict && negotiation_restrict->nr) { > + struct string_list_item *item; > + for_each_string_list_item(item, negotiation_restrict) > + strvec_pushf(&child.args, "--negotiation-restrict=%s", > + item->string); > + nr_negotiation = negotiation_restrict->nr; > + } else { > + for (ref = remote_refs; ref; ref = ref->next) { > + if (!is_null_oid(&ref->new_oid)) { > + strvec_pushf(&child.args, "--negotiation-restrict=%s", > + oid_to_hex(&ref->new_oid)); > + nr_negotiation++; > + } > } > } > + > + if (negotiation_include && negotiation_include->nr) { > + struct string_list_item *item; > + for_each_string_list_item(item, negotiation_include) > + strvec_pushf(&child.args, "--negotiation-include=%s", > + item->string); > + nr_negotiation += negotiation_include->nr; > + } > + > strvec_push(&child.args, url); > > - if (!nr_negotiation_tip) { > + if (!nr_negotiation) { > child_process_clear(&child); > return; > } Reads cleanly, and also updates the calls to fetch to use the new preferred option name `restrict` vs the older `tip`. Nice! > @@ -528,7 +548,10 @@ int send_pack(struct repository *r, > repo_config_get_bool(r, "push.negotiate", &push_negotiate); > if (push_negotiate) { > trace2_region_enter("send_pack", "push_negotiate", r); > - get_commons_through_negotiation(r, args->url, remote_refs, &commons); > + get_commons_through_negotiation(r, args->url, > + args->negotiation_include, > + args->negotiation_restrict, > + remote_refs, &commons); > trace2_region_leave("send_pack", "push_negotiate", r); > } > > diff --git a/send-pack.h b/send-pack.h > index c5ded2d200..13850c98bb 100644 > --- a/send-pack.h > +++ b/send-pack.h > @@ -18,6 +18,8 @@ struct repository; > > struct send_pack_args { > const char *url; > + const struct string_list *negotiation_include; > + const struct string_list *negotiation_restrict; > unsigned verbose:1, > quiet:1, > porcelain:1, > diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh > index ac8447f21e..177cbc6c75 100755 > --- a/t/t5516-fetch-push.sh > +++ b/t/t5516-fetch-push.sh > @@ -254,6 +254,36 @@ test_expect_success 'push with negotiation does not attempt to fetch submodules' > ! grep "Fetching submodule" err > ' > > +test_expect_success 'push with negotiation and remote..negotiationInclude' ' > + test_when_finished rm -rf negotiation_include && > + mk_empty negotiation_include && > + git push negotiation_include $the_first_commit:refs/remotes/origin/first_commit && > + test_commit -C negotiation_include unrelated_commit && > + git -C negotiation_include config receive.hideRefs refs/remotes/origin/first_commit && > + test_when_finished "rm event" && > + GIT_TRACE2_EVENT="$(pwd)/event" \ > + git -c protocol.version=2 -c push.negotiate=1 \ > + -c remote.negotiation_include.negotiationInclude=refs/heads/main \ > + push negotiation_include refs/heads/main:refs/remotes/origin/main && > + test_grep \"key\":\"total_rounds\" event && > + grep_wrote 2 event # 1 commit, 1 tree > +' > + > +test_expect_success 'push with negotiation and remote..negotiationRestrict' ' > + test_when_finished rm -rf negotiation_restrict && > + mk_empty negotiation_restrict && > + git push negotiation_restrict $the_first_commit:refs/remotes/origin/first_commit && > + test_commit -C negotiation_restrict unrelated_commit && > + git -C negotiation_restrict config receive.hideRefs refs/remotes/origin/first_commit && > + test_when_finished "rm event" && > + GIT_TRACE2_EVENT="$(pwd)/event" \ > + git -c protocol.version=2 -c push.negotiate=1 \ > + -c remote.negotiation_restrict.negotiationRestrict=refs/heads/main \ > + push negotiation_restrict refs/heads/main:refs/remotes/origin/main && > + test_grep \"key\":\"total_rounds\" event && > + grep_wrote 2 event # 1 commit, 1 tree > +' > + > test_expect_success 'push without wildcard' ' > mk_empty testrepo && > > diff --git a/transport.c b/transport.c > index 8a2d8adffc..60b73feb34 100644 > --- a/transport.c > +++ b/transport.c > @@ -921,6 +921,8 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re > args.atomic = !!(flags & TRANSPORT_PUSH_ATOMIC); > args.push_options = transport->push_options; > args.url = transport->url; > + args.negotiation_include = &transport->remote->negotiation_include; > + args.negotiation_restrict = &transport->remote->negotiation_restrict; > > if (flags & TRANSPORT_PUSH_CERT_ALWAYS) > args.push_cert = SEND_PACK_PUSH_CERT_ALWAYS; I've got no other comments on this patch :-) Thanks, Matthew