-
Notifications
You must be signed in to change notification settings - Fork 185
fetch: rework negotiation tip options #2085
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
466c56a
fe87539
4332cbf
24f8e9b
0e00590
ab78cb6
49d8d3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,7 @@ the current repository has the same history as the source repository. | |
| `.git/shallow`. This option updates `.git/shallow` and accepts such | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> - warning("ignoring --negotiation-tip=%s because it does not match any refs",
> - s);
> + warning(_("ignoring %s=%s because it does not match any refs"),
> + "--negotiation-restrict", s);
> - warning("ignoring --negotiation-tip because the protocol does not support it");
> + warning(_("ignoring %s because the protocol does not support it"),
> + "--negotiation-restrict");
These are nice touches to make sure translators cannot possibly
botch these option names that must be given verbatim.
> }
> return transport;
> }
> @@ -2567,6 +2568,8 @@ int cmd_fetch(int argc,
> OPT_IPVERSION(&family),
> OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
> N_("report that we have only objects reachable from this object")),
> + OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
> + N_("report that we have only objects reachable from this object")),
Is OPT_ALIAS() suitable for this?
> @@ -2657,7 +2660,7 @@ int cmd_fetch(int argc,
> }
>
> if (negotiate_only && !negotiation_tip.nr)
> - die(_("--negotiate-only needs one or more --negotiation-tip=*"));
> + die(_("--negotiate-only needs one or more --negotiation-restrict=*"));
OK. Shouldn't this also do the "%s" thing?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Derrick Stolee wrote on the Git mailing list (how to reply to this email): On 4/15/26 5:57 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >> - warning("ignoring --negotiation-tip=%s because it does not match any refs",
>> - s);
>> + warning(_("ignoring %s=%s because it does not match any refs"),
>> + "--negotiation-restrict", s);
>> - warning("ignoring --negotiation-tip because the protocol does not support it");
>> + warning(_("ignoring %s because the protocol does not support it"),
>> + "--negotiation-restrict");
> > These are nice touches to make sure translators cannot possibly
> botch these option names that must be given verbatim.
>> @@ -2657,7 +2660,7 @@ int cmd_fetch(int argc,
>> }
>>
>> if (negotiate_only && !negotiation_tip.nr)
>> - die(_("--negotiate-only needs one or more --negotiation-tip=*"));
>> + die(_("--negotiate-only needs one or more --negotiation-restrict=*"));
>
> OK. Shouldn't this also do the "%s" thing?
I think I had focused on adding "%s" to strings that were not
previously translated, but adjusting the string under translation
is enough to require retranslation. I should make it easier to
translate, too.
>> }
>> return transport;
>> }
>> @@ -2567,6 +2568,8 @@ int cmd_fetch(int argc,
>> OPT_IPVERSION(&family),
>> OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
>> N_("report that we have only objects reachable from this object")),
>> + OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
>> + N_("report that we have only objects reachable from this object")),
> > Is OPT_ALIAS() suitable for this?
I was not aware of this. Thanks for the pointer!
I do plan to make "negotiation-tip" an alias for "negotiation-restrict"
based on the new preference for *-restrict as the "real" option now. Is
that the right way to do this?
Thanks,
-StoleeThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Junio C Hamano wrote on the Git mailing list (how to reply to this email): Derrick Stolee <stolee@gmail.com> writes:
>>> OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
>>> N_("report that we have only objects reachable from this object")),
>>> + OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
>>> + N_("report that we have only objects reachable from this object")),
>>
>> Is OPT_ALIAS() suitable for this?
>
> I was not aware of this. Thanks for the pointer!
>
> I do plan to make "negotiation-tip" an alias for "negotiation-restrict"
> based on the new preference for *-restrict as the "real" option now. Is
> that the right way to do this?
Let's see.
$ git grep OPT_ALIAS builtin/clone.c
builtin/clone.c: OPT_ALIAS(0, "recursive", "recurse-submodules"),
$ git clone -h
usage: git clone [<options>] [--] <repo> [<dir>]
-v, --[no-]verbose be more verbose
-q, --[no-]quiet be more quiet
...
--[no-]recurse-submodules[=<pathspec>]
initialize submodules in the clone
--[no-]recursive[=<pathspec>]
alias of --recurse-submodules
...
I think we gave the operation the name "recursive", with a common
short sightedness that anything we are adding "recursive" for is the
only kind of recursiveness, and then prepared for a future where
things other than submodules can also be sources of recursiveness by
making "recurse-submodules" the official name, while still allowing
historical name as the synonym.
In this case, if "-restrict" will become the official name, it
should be listed first, and then the historical name should be made
its alias.
So
OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, ...),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
would be the right combination in the correct order, I think.
Mention the official thing first, and then tell that another thing
is an alias to what the readers have already seen after that (e.g.,
c28b036f (clone: reorder --recursive/--recurse-submodules,
2020-03-16)).
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Derrick Stolee wrote on the Git mailing list (how to reply to this email): On 4/20/2026 6:32 AM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
>
>>>> OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
>>>> N_("report that we have only objects reachable from this object")),
>>>> + OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
>>>> + N_("report that we have only objects reachable from this object")),
>>>
>>> Is OPT_ALIAS() suitable for this?
>>
>> I was not aware of this. Thanks for the pointer!
>>
>> I do plan to make "negotiation-tip" an alias for "negotiation-restrict"
>> based on the new preference for *-restrict as the "real" option now. Is
>> that the right way to do this?
>
> Let's see.
...
> So
>
> OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, ...),
> OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
>
> would be the right combination in the correct order, I think.
> Mention the official thing first, and then tell that another thing
> is an alias to what the readers have already seen after that (e.g.,
> c28b036f (clone: reorder --recursive/--recurse-submodules,
> 2020-03-16)).
Thanks! This is indeed what I have in my local copy in preparation
for v3. It helps to have early confirmation about this.
-Stolee
|
||
| refs. | ||
|
|
||
| `--negotiation-restrict=(<commit>|<glob>)`:: | ||
| `--negotiation-tip=(<commit>|<glob>)`:: | ||
| By default, Git will report, to the server, commits reachable | ||
| from all local refs to find common commits in an attempt to | ||
|
|
@@ -58,6 +59,9 @@ the current repository has the same history as the source repository. | |
| local ref is likely to have commits in common with the | ||
| upstream ref being fetched. | ||
| + | ||
| `--negotiation-restrict` is the preferred name for this option; | ||
| `--negotiation-tip` is accepted as a synonym. | ||
| + | ||
| This option may be specified more than once; if so, Git will report | ||
| commits reachable from any of the given commits. | ||
| + | ||
|
|
@@ -69,6 +73,29 @@ See also the `fetch.negotiationAlgorithm` and `push.negotiate` | |
| configuration variables documented in linkgit:git-config[1], and the | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Junio C Hamano wrote on the Git mailing list (how to reply to this email): "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +`--negotiation-require=<revision>`::
> + Ensure that the given ref tip is always sent as a "have" line
> + during fetch negotiation, regardless of what the negotiation
> + algorithm selects. This is useful to guarantee that common
> + history reachable from specific refs is always considered, even
> + when `--negotiation-restrict` restricts the set of tips or when
> + the negotiation algorithm would otherwise skip them.
> ++
> +This option may be specified more than once; if so, each ref is sent
> +unconditionally.
> ++
> +The argument may be an exact ref name (e.g. `refs/heads/release`) or a
> +glob pattern (e.g. `refs/heads/release/{asterisk}`). The pattern syntax
> +is the same as for `--negotiation-restrict`.
> ++
> +If `--negotiation-restrict` is used, the have set is first restricted by
> +that option and then increased to include the tips specified by
> +`--negotiation-require`.
Very readable. Nice.
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 57b2b667ff..b60652e6b1 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -99,6 +99,7 @@ static struct transport *gsecondary;
> static struct refspec refmap = REFSPEC_INIT_FETCH;
> static struct string_list server_options = STRING_LIST_INIT_DUP;
> static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
> +static struct string_list negotiation_require = STRING_LIST_INIT_NODUP;
I thought _tip was renamed to _restrict in an earlier step, but that
was only in the transport in [3/7]. Perhaps we want to rename the
file-scope static variable negotiation_tip to negotiation_restrict
in an earlier step, like in [2/7]?
> + for_each_string_list_item(item, negotiation_require) {
> + if (!has_glob_specials(item->string)) {
> + struct object_id oid;
> + if (repo_get_oid(the_repository, item->string, &oid))
> + continue;
The configuration (or command line) says --nego-require=refs/heads/main
but this old repository only has refs/heads/master; we do not want
to error out in such a case.
Is it true, though? nego-{require,restrict} feels quite tied to
each project and unless the configuration or command line options
are applied blindly regardless of the project, such an error should
not happen. Perhaps the user who gives a command line option
"--nego-require=refs/heads/naster" may want to be reminded of a
possible typo?
> + if (!odb_has_object(the_repository->objects, &oid, 0))
> + continue;
This is a bit curious. When does the first condition holds but not
the second? A lazy clone whose ref-tip contains a missing commit
promised by somebody else?
In the presense of "promised objects are allowed to be missing"
rule, silently skipping a missing object here is certainly
conservative, but this is not an object that is buried deep in a
tree hierarchy, but the top-level commit or tag that is directly
pointed at by a ref, isn't it? I am a bit uneasy that we ignore
such potential repository corruption (i.e., a missing object may not
be something a promisor remote promised but simply missing).
> @@ -474,7 +511,25 @@ static int find_common(struct fetch_negotiator *negotiator,
> trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
> flushes = 0;
> retval = -1;
> +
> + /* Send unconditional haves from --negotiation-require */
> + resolve_negotiation_require(args->negotiation_require,
> + &negotiation_require_oids);
> + if (oidset_size(&negotiation_require_oids)) {
> + struct oidset_iter iter;
> + oidset_iter_init(&negotiation_require_oids, &iter);
> +
> + while ((oid = oidset_iter_next(&iter))) {
> + packet_buf_write(&req_buf, "have %s\n",
> + oid_to_hex(oid));
> + print_verbose(args, "have %s", oid_to_hex(oid));
> + }
> + }
OK. I think it makes sense to send these early. We have already
dealt with the usual "tips" by calling mark_tips() way earlier, but
that hasn't produced any "have" yet, and these will go before the
ones from traversal. We do not traverse from these "require" and
that may be why these are not called "_tips"?
And sending these early means the other side has much less chance to
say "we've heard enough, stop!", so in a sense they are of much
higher priority "have"s (I wonder what happens when they do want to
say "stop!" while we are giving a lot of "have" from this loop,
though).
> while ((oid = negotiator->next(negotiator))) {
> + /* avoid duplicate oids from --negotiation-require */
> + if (oidset_contains(&negotiation_require_oids, oid))
> + continue;
If objects rechable from "require" are traversed like others, then
this "avoid duplicate" would become unnecessary, right?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Patrick Steinhardt wrote on the Git mailing list (how to reply to this email): On Wed, Apr 15, 2026 at 03:14:24PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
>
> Add a new --negotiation-require option to 'git fetch', which ensures
> that certain ref tips are always sent as 'have' lines during fetch
> negotiation, regardless of what the negotiation algorithm selects.
When reading "--negotiation-require" my mind immediately shifts towards
a mode where we require the remote to have a specific reference, and if
not we'll abort. That's of course not what you're proposing here, but I
would think that I may not be the only person making that connection.
Would an alternative like "--negotiation-include" or
"--negotiation-expand" be better?
> diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
> index c07b85499f..85ffc5b32b 100644
> --- a/Documentation/fetch-options.adoc
> +++ b/Documentation/fetch-options.adoc
> @@ -73,6 +73,25 @@ See also the `fetch.negotiationAlgorithm` and `push.negotiate`
> configuration variables documented in linkgit:git-config[1], and the
> `--negotiate-only` option below.
>
> +`--negotiation-require=<revision>`::
> + Ensure that the given ref tip is always sent as a "have" line
> + during fetch negotiation, regardless of what the negotiation
> + algorithm selects. This is useful to guarantee that common
> + history reachable from specific refs is always considered, even
> + when `--negotiation-restrict` restricts the set of tips or when
> + the negotiation algorithm would otherwise skip them.
> ++
> +This option may be specified more than once; if so, each ref is sent
> +unconditionally.
> ++
> +The argument may be an exact ref name (e.g. `refs/heads/release`) or a
> +glob pattern (e.g. `refs/heads/release/{asterisk}`). The pattern syntax
> +is the same as for `--negotiation-restrict`.
> ++
> +If `--negotiation-restrict` is used, the have set is first restricted by
> +that option and then increased to include the tips specified by
> +`--negotiation-require`.
This interaction makes sense. You can basically say "send only local
branches, but please _also_ send that one particular ref over there".
> diff --git a/fetch-pack.c b/fetch-pack.c
> index baf239adf9..a0029253f1 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -474,7 +511,25 @@ static int find_common(struct fetch_negotiator *negotiator,
> trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
> flushes = 0;
> retval = -1;
> +
> + /* Send unconditional haves from --negotiation-require */
> + resolve_negotiation_require(args->negotiation_require,
> + &negotiation_require_oids);
> + if (oidset_size(&negotiation_require_oids)) {
> + struct oidset_iter iter;
> + oidset_iter_init(&negotiation_require_oids, &iter);
> +
> + while ((oid = oidset_iter_next(&iter))) {
> + packet_buf_write(&req_buf, "have %s\n",
> + oid_to_hex(oid));
> + print_verbose(args, "have %s", oid_to_hex(oid));
> + }
> + }
Okay, so here we now unconditionally send our requested object IDs.
One thing I was wondering is whether we need to flush eventually. It can
happen that the user specifies millions of refs, either intentionally or
by accident. But I guess the answer might be "no", as the intent of the
feature is that we indeed want to send all of those to the remote side,
and the remote is being asked to consider all of those OIDs.
PatrickThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Derrick Stolee wrote on the Git mailing list (how to reply to this email): On 4/20/2026 4:11 AM, Patrick Steinhardt wrote:
> On Wed, Apr 15, 2026 at 03:14:24PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> Add a new --negotiation-require option to 'git fetch', which ensures
>> that certain ref tips are always sent as 'have' lines during fetch
>> negotiation, regardless of what the negotiation algorithm selects.
>
> When reading "--negotiation-require" my mind immediately shifts towards
> a mode where we require the remote to have a specific reference, and if
> not we'll abort. That's of course not what you're proposing here, but I
> would think that I may not be the only person making that connection.
>
> Would an alternative like "--negotiation-include" or
> "--negotiation-expand" be better?
"include" does sound good to me. I'm open to it. I'll let this idea
stew and try prepping my local branch in this direction.
>> + /* Send unconditional haves from --negotiation-require */
>> + resolve_negotiation_require(args->negotiation_require,
>> + &negotiation_require_oids);
>> + if (oidset_size(&negotiation_require_oids)) {
>> + struct oidset_iter iter;
>> + oidset_iter_init(&negotiation_require_oids, &iter);
>> +
>> + while ((oid = oidset_iter_next(&iter))) {
>> + packet_buf_write(&req_buf, "have %s\n",
>> + oid_to_hex(oid));
>> + print_verbose(args, "have %s", oid_to_hex(oid));
>> + }
>> + }
>
> Okay, so here we now unconditionally send our requested object IDs.
>
> One thing I was wondering is whether we need to flush eventually. It can
> happen that the user specifies millions of refs, either intentionally or
> by accident. But I guess the answer might be "no", as the intent of the
> feature is that we indeed want to send all of those to the remote side,
> and the remote is being asked to consider all of those OIDs.
The idea is indeed to send all of the requested OIDs, but this does
present an interesting behavior where the Git client can allow the
user to misconfigure themselves to send larger-than-normal negotiation
requests. Previously, the client would protect the negotiation with a
maximum set of haves.
Is there any concern about this becoming a vector for increased load
on servers?
Would it be good to have some kind of advice message when the config
matches a set of haves that we think is too large? That would maybe
be a way to help users get out of a self-made problem.
Thanks,
-Stolee
|
||
| `--negotiate-only` option below. | ||
|
|
||
| `--negotiation-require=<revision>`:: | ||
| Ensure that the given ref tip is always sent as a "have" line | ||
| during fetch negotiation, regardless of what the negotiation | ||
| algorithm selects. This is useful to guarantee that common | ||
| history reachable from specific refs is always considered, even | ||
| when `--negotiation-restrict` restricts the set of tips or when | ||
| the negotiation algorithm would otherwise skip them. | ||
| + | ||
| This option may be specified more than once; if so, each ref is sent | ||
| unconditionally. | ||
| + | ||
| The argument may be an exact ref name (e.g. `refs/heads/release`) or a | ||
| glob pattern (e.g. `refs/heads/release/{asterisk}`). The pattern syntax | ||
| is the same as for `--negotiation-restrict`. | ||
| + | ||
| If `--negotiation-restrict` is used, the have set is first restricted by | ||
| that option and then increased to include the tips specified by | ||
| `--negotiation-require`. | ||
| + | ||
| If this option is not specified on the command line, then any | ||
| `remote.<name>.negotiationRequire` config values for the current remote | ||
| are used instead. | ||
|
|
||
| `--negotiate-only`:: | ||
| Do not fetch anything from the server, and instead print the | ||
| ancestors of the provided `--negotiation-tip=` arguments, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,7 +98,8 @@ static struct transport *gtransport; | |
| static struct transport *gsecondary; | ||
| static struct refspec refmap = REFSPEC_INIT_FETCH; | ||
| static struct string_list server_options = STRING_LIST_INIT_DUP; | ||
| static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP; | ||
| static struct string_list negotiation_restrict = STRING_LIST_INIT_NODUP; | ||
| static struct string_list negotiation_require = STRING_LIST_INIT_NODUP; | ||
|
|
||
| struct fetch_config { | ||
| enum display_format display_format; | ||
|
|
@@ -1534,13 +1535,13 @@ static int add_oid(const struct reference *ref, void *cb_data) | |
| return 0; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Patrick Steinhardt wrote on the Git mailing list (how to reply to this email): On Wed, Apr 15, 2026 at 03:14:22PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 3bcb0c9686..4c3c5f2faa 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
Don't we want to also rename the local `negotiation_tip` variable in
`cmd_fetch()`?
Patrick |
||
| } | ||
|
|
||
| static void add_negotiation_tips(struct git_transport_options *smart_options) | ||
| static void add_negotiation_restrict_tips(struct git_transport_options *smart_options) | ||
| { | ||
| struct oid_array *oids = xcalloc(1, sizeof(*oids)); | ||
| int i; | ||
|
|
||
| for (i = 0; i < negotiation_tip.nr; i++) { | ||
| const char *s = negotiation_tip.items[i].string; | ||
| for (i = 0; i < negotiation_restrict.nr; i++) { | ||
| const char *s = negotiation_restrict.items[i].string; | ||
| struct refs_for_each_ref_options opts = { | ||
| .pattern = s, | ||
| }; | ||
|
|
@@ -1558,10 +1559,10 @@ static void add_negotiation_tips(struct git_transport_options *smart_options) | |
| refs_for_each_ref_ext(get_main_ref_store(the_repository), | ||
| add_oid, oids, &opts); | ||
| if (old_nr == oids->nr) | ||
| warning("ignoring --negotiation-tip=%s because it does not match any refs", | ||
| s); | ||
| warning(_("ignoring %s=%s because it does not match any refs"), | ||
| "--negotiation-restrict", s); | ||
| } | ||
| smart_options->negotiation_tips = oids; | ||
| smart_options->negotiation_restrict_tips = oids; | ||
| } | ||
|
|
||
| static struct transport *prepare_transport(struct remote *remote, int deepen, | ||
|
|
@@ -1595,11 +1596,42 @@ static struct transport *prepare_transport(struct remote *remote, int deepen, | |
| set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER, spec); | ||
| set_option(transport, TRANS_OPT_FROM_PROMISOR, "1"); | ||
| } | ||
| if (negotiation_tip.nr) { | ||
| if (negotiation_restrict.nr) { | ||
| if (transport->smart_options) | ||
| add_negotiation_tips(transport->smart_options); | ||
| add_negotiation_restrict_tips(transport->smart_options); | ||
| else | ||
| warning("ignoring --negotiation-tip because the protocol does not support it"); | ||
| warning(_("ignoring %s because the protocol does not support it"), | ||
| "--negotiation-restrict"); | ||
| } else if (remote->negotiation_restrict.nr) { | ||
| struct string_list_item *item; | ||
| for_each_string_list_item(item, &remote->negotiation_restrict) | ||
| string_list_append(&negotiation_restrict, item->string); | ||
| if (transport->smart_options) | ||
| add_negotiation_restrict_tips(transport->smart_options); | ||
| else { | ||
| struct strbuf config_name = STRBUF_INIT; | ||
| strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name); | ||
| warning(_("ignoring %s because the protocol does not support it"), | ||
| config_name.buf); | ||
| strbuf_release(&config_name); | ||
| } | ||
| } | ||
| if (negotiation_require.nr) { | ||
| if (transport->smart_options) | ||
| transport->smart_options->negotiation_require = &negotiation_require; | ||
| else | ||
| warning(_("ignoring %s because the protocol does not support it"), | ||
| "--negotiation-require"); | ||
| } else if (remote->negotiation_require.nr) { | ||
| if (transport->smart_options) { | ||
| transport->smart_options->negotiation_require = &remote->negotiation_require; | ||
| } else { | ||
| struct strbuf config_name = STRBUF_INIT; | ||
| strbuf_addf(&config_name, "remote.%s.negotiationRequire", remote->name); | ||
| warning(_("ignoring %s because the protocol does not support it"), | ||
| config_name.buf); | ||
| strbuf_release(&config_name); | ||
| } | ||
| } | ||
| return transport; | ||
| } | ||
|
|
@@ -2565,8 +2597,11 @@ int cmd_fetch(int argc, | |
| N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg), | ||
| OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")), | ||
| OPT_IPVERSION(&family), | ||
| OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"), | ||
| OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_restrict, N_("revision"), | ||
| N_("report that we have only objects reachable from this object")), | ||
| OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"), | ||
| OPT_STRING_LIST(0, "negotiation-require", &negotiation_require, N_("revision"), | ||
| N_("ensure this ref is always sent as a negotiation have")), | ||
| OPT_BOOL(0, "negotiate-only", &negotiate_only, | ||
| N_("do not fetch a packfile; instead, print ancestors of negotiation tips")), | ||
| OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options), | ||
|
|
@@ -2656,9 +2691,6 @@ int cmd_fetch(int argc, | |
| config.display_format = DISPLAY_FORMAT_PORCELAIN; | ||
| } | ||
|
|
||
| if (negotiate_only && !negotiation_tip.nr) | ||
| die(_("--negotiate-only needs one or more --negotiation-tip=*")); | ||
|
|
||
| if (deepen_relative) { | ||
| if (deepen_relative < 0) | ||
| die(_("negative depth in --deepen is not supported")); | ||
|
|
@@ -2746,6 +2778,10 @@ int cmd_fetch(int argc, | |
| if (!remote) | ||
| die(_("must supply remote when using --negotiate-only")); | ||
| gtransport = prepare_transport(remote, 1, &filter_options); | ||
| if (!gtransport->smart_options || | ||
| !gtransport->smart_options->negotiation_restrict_tips) | ||
| die(_("%s needs one or more %s"), "--negotiate-only", | ||
| "--negotiation-restrict=*"); | ||
| if (gtransport->smart_options) { | ||
| gtransport->smart_options->acked_commits = &acked_commits; | ||
| } else { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Junio C Hamano wrote on the Git mailing list (how to reply to this email):