* [PATCH v5 4/8] remote: add remote.*.negotiationRestrict config
From: Derrick Stolee via GitGitGadget @ 2026-05-18 20:19 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v5.git.1779135575.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
In a previous change, the --negotiation-restrict command-line option of 'git
fetch' was added as a synonym of --negotiation-tip. Both of these options
restrict the set of 'haves' the client can send as part of negotiation.
This was previously not available via a configuration option. Add a new
'remote.<name>.negotiationRestrict' multi-valued config option that updates
'git fetch <name>' to use these restrictions by default.
If the user provides even one --negotiation-restrict argument, then the
config is ignored.
An empty value resets the value list to allow ignoring earlier config
values, such as those that might be set in system or global config.
Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 18 ++++++++++++++++++
builtin/fetch.c | 28 +++++++++++++++++++++-------
remote.c | 5 +++++
remote.h | 1 +
t/t5510-fetch.sh | 26 ++++++++++++++++++++++++++
5 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 91e46f66f5..4dcf81fbce 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -107,6 +107,24 @@ priority configuration file (e.g. `.git/config` in a repository) to clear
the values inherited from a lower priority configuration files (e.g.
`$HOME/.gitconfig`).
+remote.<name>.negotiationRestrict::
+ When negotiating with this remote during `git fetch`, restrict the
+ commits advertised as "have" lines to only those reachable from refs
+ matching the given patterns. This multi-valued config option behaves
+ like `--negotiation-restrict` on the command line.
++
+Each value is either an exact ref name (e.g. `refs/heads/release`) or a
+glob pattern (e.g. `refs/heads/release/*`). The pattern syntax is the
+same as for `--negotiation-restrict`.
++
+These config values are used as defaults for the `--negotiation-restrict`
+command-line option. If `--negotiation-restrict` (or its synonym
+`--negotiation-tip`) is specified on the command line, then the config
+values are not used.
++
+Blank values signal to ignore all previous values, allowing a reset of
+the list from broader config scenarios.
+
remote.<name>.followRemoteHEAD::
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
when fetching using the configured refspecs of a remote.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2ba0051d52..a957739f37 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1601,6 +1601,19 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
else
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);
+ }
}
return transport;
}
@@ -2658,10 +2671,6 @@ int cmd_fetch(int argc,
config.display_format = DISPLAY_FORMAT_PORCELAIN;
}
- if (negotiate_only && !negotiation_restrict.nr)
- die(_("%s needs one or more %s"), "--negotiate-only",
- "--negotiation-restrict=*");
-
if (deepen_relative) {
if (deepen_relative < 0)
die(_("negative depth in --deepen is not supported"));
@@ -2749,14 +2758,19 @@ 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->acked_commits = &acked_commits;
- } else {
+
+ if (!gtransport->smart_options) {
warning(_("protocol does not support --negotiate-only, exiting"));
result = 1;
trace2_region_leave("fetch", "negotiate-only", the_repository);
goto cleanup;
}
+ if (!gtransport->smart_options->negotiation_restrict_tips)
+ die(_("%s needs one or more %s"), "--negotiate-only",
+ "--negotiation-restrict=*");
+
+ gtransport->smart_options->acked_commits = &acked_commits;
+
if (server_options.nr)
gtransport->server_options = &server_options;
result = transport_fetch_refs(gtransport, NULL);
diff --git a/remote.c b/remote.c
index 7ca2a6501b..620086e16e 100644
--- a/remote.c
+++ b/remote.c
@@ -152,6 +152,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
refspec_init_push(&ret->push);
refspec_init_fetch(&ret->fetch);
string_list_init_dup(&ret->server_options);
+ string_list_init_dup(&ret->negotiation_restrict);
ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
remote_state->remotes_alloc);
@@ -179,6 +180,7 @@ static void remote_clear(struct remote *remote)
FREE_AND_NULL(remote->http_proxy);
FREE_AND_NULL(remote->http_proxy_authmethod);
string_list_clear(&remote->server_options, 0);
+ string_list_clear(&remote->negotiation_restrict, 0);
}
static void add_merge(struct branch *branch, const char *name)
@@ -562,6 +564,9 @@ static int handle_config(const char *key, const char *value,
} else if (!strcmp(subkey, "serveroption")) {
return parse_transport_option(key, value,
&remote->server_options);
+ } else if (!strcmp(subkey, "negotiationrestrict")) {
+ return parse_transport_option(key, value,
+ &remote->negotiation_restrict);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
diff --git a/remote.h b/remote.h
index fc052945ee..e6ec37c393 100644
--- a/remote.h
+++ b/remote.h
@@ -117,6 +117,7 @@ struct remote {
char *http_proxy_authmethod;
struct string_list server_options;
+ struct string_list negotiation_restrict;
enum follow_remote_head_settings follow_remote_head;
const char *no_warn_branch;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index dc3ce56d84..eff3ce8e2d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1485,6 +1485,32 @@ test_expect_success '--negotiation-restrict and --negotiation-tip can be mixed'
check_negotiation_tip
'
+test_expect_success 'remote.<name>.negotiationRestrict used as default' '
+ setup_negotiation_tip server server 0 &&
+
+ # test the reset of the list on an empty value
+ git -C client config --add remote.origin.negotiationRestrict alpha_2 &&
+ git -C client config --add remote.origin.negotiationRestrict "" &&
+ git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
+ git -C client config --add remote.origin.negotiationRestrict beta_1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
+test_expect_success 'CLI --negotiation-restrict overrides remote config' '
+ setup_negotiation_tip server server 0 &&
+ git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
+ git -C client config --add remote.origin.negotiationRestrict beta_1 &&
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ origin alpha_s beta_s &&
+ test_grep "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep ! "fetch> have $BETA_1" trace
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 5/8] negotiator: add have_sent() interface
From: Derrick Stolee via GitGitGadget @ 2026-05-18 20:19 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v5.git.1779135575.gitgitgadget@gmail.com>
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.
Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
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;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 6/8] fetch: add --negotiation-include option for negotiation
From: Derrick Stolee via GitGitGadget @ 2026-05-18 20:19 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v5.git.1779135575.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Add a new --negotiation-include 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.
This is useful when the repository has a large number of references, so
the normal negotiation algorithm truncates the list. This is especially
important in repositories with long parallel commit histories. For
example, a repo could have a 'dev' branch for development and a
'release' branch for released versions. If the 'dev' branch isn't
selected for negotiation, then it's not a big deal because there are
many in-progress development branches with a shared history. However, if
'release' is not selected for negotiation, then the server may think
that this is the first time the client has asked for that reference,
causing a full download of its parallel commit history (and any extra
data that may be unique to that branch). This is based on a real example
where certain fetches would grow to 60+ GB when a release branch
updated.
This option is a complement to --negotiation-restrict, which reduces the
negotiation ref set to a specific list. In the earlier example, using
--negotiation-restrict to focus the negotiation to 'dev' and 'release'
would avoid those problematic downloads, but would still not allow
advertising potentially-relevant user branches. In this way, the
'include' version solves the problem I mention while allowing
negotiation to pick other references opportunistically. The two options
can also be combined to allow the best of both worlds.
The argument may be an exact ref name or a glob pattern. Non-existent
refs are silently ignored. This behavior is also updated in the ref matching
logic for the related --negotiation-restrict option to match.
The implementation outputs the requested objects as haves before the
negotiator performs its own algorithm to choose the next haves. Use the new
have_sent() interface to signal these have commits were sent before engaging
with the negotiator's next() iterator.
Also add --negotiation-include to 'git pull' passthrough options.
Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/fetch-options.adoc | 19 +++++++
builtin/fetch.c | 38 ++++++++++---
builtin/pull.c | 3 ++
fetch-pack.c | 81 +++++++++++++++++++++++++---
fetch-pack.h | 6 ++-
t/t5510-fetch.sh | 91 ++++++++++++++++++++++++++++++++
transport.c | 8 ++-
transport.h | 5 +-
8 files changed, 232 insertions(+), 19 deletions(-)
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index d39cecb446..7b897a7202 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-include=(<commit>|<glob>)`::
+ Ensure that the commits at the given tips are always sent as "have"
+ lines 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 commit is sent
+unconditionally.
++
+The argument may be an exact ref name (e.g. `refs/heads/release`), an
+object hash, 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-include`.
+
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
ancestors of the provided `--negotiation-restrict=` arguments,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a957739f37..ba56e9022b 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_restrict = STRING_LIST_INIT_NODUP;
+static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
@@ -1534,23 +1535,29 @@ static int add_oid(const struct reference *ref, void *cb_data)
return 0;
}
-static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
+static void add_negotiation_tips(struct string_list *input_list,
+ struct oid_array **output_list,
+ const char *argname)
{
struct oid_array *oids = xcalloc(1, sizeof(*oids));
int i;
- for (i = 0; i < negotiation_restrict.nr; i++) {
- const char *s = negotiation_restrict.items[i].string;
+ for (i = 0; i < input_list->nr; i++) {
+ const char *s = input_list->items[i].string;
struct refs_for_each_ref_options opts = {
.pattern = s,
};
int old_nr;
if (!has_glob_specials(s)) {
struct object_id oid;
+
+ /* Ignore missing reference. */
if (repo_get_oid(the_repository, s, &oid))
- die(_("%s is not a valid object"), s);
+ continue;
+ /* Fail on missing object pointed by ref. */
if (!odb_has_object(the_repository->objects, &oid, 0))
die(_("the object %s does not exist"), s);
+
oid_array_append(oids, &oid);
continue;
}
@@ -1559,9 +1566,9 @@ static void add_negotiation_restrict_tips(struct git_transport_options *smart_op
add_oid, oids, &opts);
if (old_nr == oids->nr)
warning(_("ignoring %s=%s because it does not match any refs"),
- "--negotiation-restrict", s);
+ argname, s);
}
- smart_options->negotiation_restrict_tips = oids;
+ *output_list = oids;
}
static struct transport *prepare_transport(struct remote *remote, int deepen,
@@ -1597,7 +1604,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
}
if (negotiation_restrict.nr) {
if (transport->smart_options)
- add_negotiation_restrict_tips(transport->smart_options);
+ add_negotiation_tips(&negotiation_restrict,
+ &transport->smart_options->negotiation_restrict_tips,
+ "--negotiation-restrict");
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-restrict");
@@ -1606,7 +1615,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
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);
+ add_negotiation_tips(&negotiation_restrict,
+ &transport->smart_options->negotiation_restrict_tips,
+ "--negotiation-restrict");
else {
struct strbuf config_name = STRBUF_INIT;
strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
@@ -1615,6 +1626,15 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
strbuf_release(&config_name);
}
}
+ if (negotiation_include.nr) {
+ if (transport->smart_options)
+ add_negotiation_tips(&negotiation_include,
+ &transport->smart_options->negotiation_include_tips,
+ "--negotiation-include");
+ else
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-include");
+ }
return transport;
}
@@ -2582,6 +2602,8 @@ int cmd_fetch(int argc,
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-include", &negotiation_include, 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),
diff --git a/builtin/pull.c b/builtin/pull.c
index cc6ce485fc..d49b09114a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1000,6 +1000,9 @@ int cmd_pull(int argc,
N_("report that we have only objects reachable from this object"),
0),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
+ OPT_PASSTHRU_ARGV(0, "negotiation-include", &opt_fetch, N_("revision"),
+ N_("ensure this ref is always sent as a negotiation have"),
+ 0),
OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
N_("check for forced-updates on all updated branches")),
OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
diff --git a/fetch-pack.c b/fetch-pack.c
index baf239adf9..96071434b8 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -25,6 +25,7 @@
#include "oidset.h"
#include "packfile.h"
#include "odb.h"
+#include "object-name.h"
#include "path.h"
#include "connected.h"
#include "fetch-negotiator.h"
@@ -332,6 +333,21 @@ static void send_filter(struct fetch_pack_args *args,
}
}
+static void add_oids_to_set(const struct oid_array *array,
+ struct oidset *set)
+{
+ if (!array)
+ return;
+
+ for (size_t i = 0; i < array->nr; i++) {
+ struct object_id *oid = &array->oid[i];
+ if (!odb_has_object(the_repository->objects, oid, 0))
+ die(_("the object %s does not exist"), oid_to_hex(oid));
+
+ oidset_insert(set, oid);
+ }
+}
+
static int find_common(struct fetch_negotiator *negotiator,
struct fetch_pack_args *args,
int fd[2], struct object_id *result_oid,
@@ -347,6 +363,7 @@ static int find_common(struct fetch_negotiator *negotiator,
struct strbuf req_buf = STRBUF_INIT;
size_t state_len = 0;
struct packet_reader reader;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
if (args->stateless_rpc && multi_ack == 1)
die(_("the option '%s' requires '%s'"), "--stateless-rpc", "multi_ack_detailed");
@@ -474,6 +491,27 @@ 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-include */
+ add_oids_to_set(args->negotiation_include_tips,
+ &negotiation_include_oids);
+ if (oidset_size(&negotiation_include_oids)) {
+ struct oidset_iter iter;
+ oidset_iter_init(&negotiation_include_oids, &iter);
+
+ while ((oid = oidset_iter_next(&iter))) {
+ struct commit *commit;
+ packet_buf_write(&req_buf, "have %s\n",
+ oid_to_hex(oid));
+ print_verbose(args, "have %s", oid_to_hex(oid));
+ count++;
+
+ commit = lookup_commit(the_repository, oid);
+ if (commit)
+ negotiator->have_sent(negotiator, commit);
+ }
+ }
+
while ((oid = negotiator->next(negotiator))) {
packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid));
print_verbose(args, "have %s", oid_to_hex(oid));
@@ -584,6 +622,7 @@ done:
flushes++;
}
strbuf_release(&req_buf);
+ oidset_clear(&negotiation_include_oids);
if (!got_ready || !no_done)
consume_shallow_list(args, &reader);
@@ -1305,11 +1344,27 @@ static void add_common(struct strbuf *req_buf, struct oidset *common)
static int add_haves(struct fetch_negotiator *negotiator,
struct strbuf *req_buf,
- int *haves_to_send)
+ int *haves_to_send,
+ struct oidset *negotiation_include_oids)
{
int haves_added = 0;
const struct object_id *oid;
+ /* Send unconditional haves from --negotiation-include */
+ if (negotiation_include_oids) {
+ struct oidset_iter iter;
+ oidset_iter_init(negotiation_include_oids, &iter);
+
+ while ((oid = oidset_iter_next(&iter))) {
+ struct commit *commit = lookup_commit(the_repository, oid);
+ if (commit) {
+ packet_buf_write(req_buf, "have %s\n",
+ oid_to_hex(oid));
+ negotiator->have_sent(negotiator, commit);
+ }
+ }
+ }
+
while ((oid = negotiator->next(negotiator))) {
packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
if (++haves_added >= *haves_to_send)
@@ -1358,7 +1413,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
struct fetch_pack_args *args,
const struct ref *wants, struct oidset *common,
int *haves_to_send, int *in_vain,
- int sideband_all, int seen_ack)
+ int sideband_all, int seen_ack,
+ struct oidset *negotiation_include_oids)
{
int haves_added;
int done_sent = 0;
@@ -1413,7 +1469,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
/* Add all of the common commits we've found in previous rounds */
add_common(&req_buf, common);
- haves_added = add_haves(negotiator, &req_buf, haves_to_send);
+ haves_added = add_haves(negotiator, &req_buf, haves_to_send,
+ negotiation_include_oids);
*in_vain += haves_added;
trace2_data_intmax("negotiation_v2", the_repository, "haves_added", haves_added);
trace2_data_intmax("negotiation_v2", the_repository, "in_vain", *in_vain);
@@ -1657,6 +1714,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
struct ref *ref = copy_ref_list(orig_ref);
enum fetch_state state = FETCH_CHECK_LOCAL;
struct oidset common = OIDSET_INIT;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
struct packet_reader reader;
int in_vain = 0, negotiation_started = 0;
int negotiation_round = 0;
@@ -1729,6 +1787,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
state = FETCH_SEND_REQUEST;
mark_tips(negotiator, args->negotiation_restrict_tips);
+ add_oids_to_set(args->negotiation_include_tips,
+ &negotiation_include_oids);
for_each_cached_alternate(negotiator,
insert_one_alternate_object);
break;
@@ -1747,7 +1807,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
&common,
&haves_to_send, &in_vain,
reader.use_sideband,
- seen_ack)) {
+ seen_ack,
+ &negotiation_include_oids)) {
trace2_region_leave_printf("negotiation_v2", "round",
the_repository, "%d",
negotiation_round);
@@ -1883,6 +1944,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
negotiator->release(negotiator);
oidset_clear(&common);
+ oidset_clear(&negotiation_include_oids);
return ref;
}
@@ -2181,12 +2243,14 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
- struct oidset *acked_commits)
+ struct oidset *acked_commits,
+ const struct oid_array *negotiation_include_tips)
{
struct fetch_negotiator negotiator;
struct packet_reader reader;
struct object_array nt_object_array = OBJECT_ARRAY_INIT;
struct strbuf req_buf = STRBUF_INIT;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
int haves_to_send = INITIAL_FLUSH;
int in_vain = 0;
int seen_ack = 0;
@@ -2197,6 +2261,9 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
fetch_negotiator_init(the_repository, &negotiator);
mark_tips(&negotiator, negotiation_restrict_tips);
+ add_oids_to_set(negotiation_include_tips,
+ &negotiation_include_oids);
+
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
@@ -2221,7 +2288,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
packet_buf_write(&req_buf, "wait-for-done");
- haves_added = add_haves(&negotiator, &req_buf, &haves_to_send);
+ haves_added = add_haves(&negotiator, &req_buf, &haves_to_send,
+ &negotiation_include_oids);
in_vain += haves_added;
if (!haves_added || (seen_ack && in_vain >= MAX_IN_VAIN))
last_iteration = 1;
@@ -2273,6 +2341,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
clear_common_flag(acked_commits);
object_array_clear(&nt_object_array);
+ oidset_clear(&negotiation_include_oids);
negotiator.release(&negotiator);
strbuf_release(&req_buf);
}
diff --git a/fetch-pack.h b/fetch-pack.h
index 6c70c942c2..6d0dec7f41 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -19,9 +19,10 @@ struct fetch_pack_args {
/*
* If not NULL, during packfile negotiation, fetch-pack will send "have"
- * lines only with these tips and their ancestors.
+ * lines for all _include_ tips and then a subset of the _restrict_ tips.
*/
const struct oid_array *negotiation_restrict_tips;
+ const struct oid_array *negotiation_include_tips;
unsigned deepen_relative:1;
unsigned quiet:1;
@@ -93,7 +94,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
- struct oidset *acked_commits);
+ struct oidset *acked_commits,
+ const struct oid_array *negotiation_include_tips);
/*
* Print an appropriate error message for each sought ref that wasn't
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index eff3ce8e2d..bc2e2af959 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1460,6 +1460,16 @@ EOF
test_cmp fatal-expect fatal-actual
'
+test_expect_success '--negotiation-tip ignores missing refs and invalid hashes' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-tip=alpha_1 --negotiation-tip=beta_1 \
+ --negotiation-tip=no-such-ref \
+ --negotiation-tip=invalid-hash \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
test_expect_success '--negotiation-restrict limits "have" lines sent' '
setup_negotiation_tip server server 0 &&
GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
@@ -1511,6 +1521,87 @@ test_expect_success 'CLI --negotiation-restrict overrides remote config' '
test_grep ! "fetch> have $BETA_1" trace
'
+test_expect_success '--negotiation-include includes configured refs as haves' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ test_grep "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success '--negotiation-include works with glob patterns' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include="refs/tags/beta_*" \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep "fetch> have $BETA_2" trace
+'
+
+test_expect_success '--negotiation-include is additive with negotiation' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success '--negotiation-include ignores non-existent refs silently' '
+ setup_negotiation_tip server server 0 &&
+
+ git -C client fetch --quiet \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/nonexistent \
+ origin alpha_s beta_s 2>err &&
+ test_must_be_empty err
+'
+
+test_expect_success '--negotiation-include avoids duplicates with negotiator' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/alpha_1 \
+ origin alpha_s beta_s &&
+
+ test_grep "fetch> have $ALPHA_1" trace >matches &&
+ test_line_count = 1 matches
+'
+
+test_expect_success '--negotiation-include avoids duplicates with v0' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client \
+ -c protocol.version=0 fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/alpha_1 \
+ origin alpha_s beta_s &&
+
+ test_grep "fetch> have $ALPHA_1" trace >matches &&
+ test_line_count = 1 matches
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
diff --git a/transport.c b/transport.c
index a3051f6733..fa54928966 100644
--- a/transport.c
+++ b/transport.c
@@ -464,6 +464,7 @@ static int fetch_refs_via_pack(struct transport *transport,
args.stateless_rpc = transport->stateless_rpc;
args.server_options = transport->server_options;
args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
+ args.negotiation_include_tips = data->options.negotiation_include_tips;
args.reject_shallow_remote = transport->smart_options->reject_shallow;
if (!data->finished_handshake) {
@@ -495,7 +496,8 @@ static int fetch_refs_via_pack(struct transport *transport,
transport->server_options,
transport->stateless_rpc,
data->fd,
- data->options.acked_commits);
+ data->options.acked_commits,
+ data->options.negotiation_include_tips);
ret = 0;
}
goto cleanup;
@@ -983,6 +985,10 @@ static int disconnect_git(struct transport *transport)
oid_array_clear(data->options.negotiation_restrict_tips);
free(data->options.negotiation_restrict_tips);
}
+ if (data->options.negotiation_include_tips) {
+ oid_array_clear(data->options.negotiation_include_tips);
+ free(data->options.negotiation_include_tips);
+ }
list_objects_filter_release(&data->options.filter_options);
oid_array_clear(&data->extra_have);
oid_array_clear(&data->shallow);
diff --git a/transport.h b/transport.h
index cdeb33c16f..97d905ecc0 100644
--- a/transport.h
+++ b/transport.h
@@ -40,13 +40,14 @@ struct git_transport_options {
/*
* This is only used during fetch. See the documentation of
- * negotiation_restrict_tips in struct fetch_pack_args.
+ * these member names in struct fetch_pack_args.
*
- * This field is only supported by transports that support connect or
+ * These fields are only supported by transports that support connect or
* stateless_connect. Set this field directly instead of using
* transport_set_option().
*/
struct oid_array *negotiation_restrict_tips;
+ struct oid_array *negotiation_include_tips;
/*
* If allocated, whenever transport_fetch_refs() is called, add known
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 7/8] remote: add remote.*.negotiationInclude config
From: Derrick Stolee via GitGitGadget @ 2026-05-18 20:19 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v5.git.1779135575.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Add a new 'remote.<name>.negotiationInclude' multi-valued config option that
provides default values for --negotiation-include when no
--negotiation-include arguments are specified over the command line. This
is a mirror of how 'remote.<name>.negotiationRestrict' specifies defaults
for the --negotiation-restrict arguments.
Each value is either an exact ref name or a glob pattern whose tips should
always be sent as 'have' lines during negotiation. The config values are
resolved through the same resolve_negotiation_include() codepath as the CLI
options.
This option is additive with the normal negotiation process: the negotiation
algorithm still runs and advertises its own selected commits, but the refs
matching the config are sent unconditionally on top of those heuristically
selected commits.
Similar to the negotiationRestrict config, an empty value resets the value
list to allow ignoring earlier config values, such as those that might be
set in system or global config.
Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 25 ++++++++++++++++
Documentation/fetch-options.adoc | 4 +++
builtin/fetch.c | 12 ++++++++
remote.c | 5 ++++
remote.h | 1 +
t/t5510-fetch.sh | 49 ++++++++++++++++++++++++++++++++
6 files changed, 96 insertions(+)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 4dcf81fbce..1951df154e 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -125,6 +125,31 @@ values are not used.
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
+remote.<name>.negotiationInclude::
+ When negotiating with this remote during `git fetch`, the client
+ advertises a list of commits that exist locally. In repos with
+ many references, this list of "haves" can be truncated. Depending
+ on data shape, dropping certain references may be expensive. This
+ multi-valued config option specifies references, commit hashes,
+ or ref pattern globs whose tips should always be sent as "have"
+ commits during fetch negotiation with this remote.
++
+Each value is either an exact ref name (e.g. `refs/heads/release`), a
+commit hash, or a glob pattern (e.g. `refs/heads/release/*`). The
+pattern syntax is the same as for `--negotiation-include`.
++
+These config values are used as defaults for the `--negotiation-include`
+command-line option. If `--negotiation-include` is specified on the
+command line, then the config values are not used.
++
+This option is additive with the normal negotiation process: the
+negotiation algorithm still runs and advertises its own selected commits,
+but the refs matching `remote.<name>.negotiationInclude` are sent
+unconditionally on top of those heuristically selected commits.
++
+Blank values signal to ignore all previous values, allowing a reset of
+the list from broader config scenarios.
+
remote.<name>.followRemoteHEAD::
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
when fetching using the configured refspecs of a remote.
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 7b897a7202..8074004377 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -91,6 +91,10 @@ 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-include`.
++
+If this option is not specified on the command line, then any
+`remote.<name>.negotiationInclude` config values for the current remote
+are used instead.
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ba56e9022b..1af6500c1d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1634,6 +1634,18 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-include");
+ } else if (remote->negotiation_include.nr) {
+ if (transport->smart_options) {
+ add_negotiation_tips(&remote->negotiation_include,
+ &transport->smart_options->negotiation_include_tips,
+ "--negotiation-include");
+ } else {
+ struct strbuf config_name = STRBUF_INIT;
+ strbuf_addf(&config_name, "remote.%s.negotiationInclude", remote->name);
+ warning(_("ignoring %s because the protocol does not support it"),
+ config_name.buf);
+ strbuf_release(&config_name);
+ }
}
return transport;
}
diff --git a/remote.c b/remote.c
index 620086e16e..6fb5758820 100644
--- a/remote.c
+++ b/remote.c
@@ -153,6 +153,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
refspec_init_fetch(&ret->fetch);
string_list_init_dup(&ret->server_options);
string_list_init_dup(&ret->negotiation_restrict);
+ string_list_init_dup(&ret->negotiation_include);
ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
remote_state->remotes_alloc);
@@ -181,6 +182,7 @@ static void remote_clear(struct remote *remote)
FREE_AND_NULL(remote->http_proxy_authmethod);
string_list_clear(&remote->server_options, 0);
string_list_clear(&remote->negotiation_restrict, 0);
+ string_list_clear(&remote->negotiation_include, 0);
}
static void add_merge(struct branch *branch, const char *name)
@@ -567,6 +569,9 @@ static int handle_config(const char *key, const char *value,
} else if (!strcmp(subkey, "negotiationrestrict")) {
return parse_transport_option(key, value,
&remote->negotiation_restrict);
+ } else if (!strcmp(subkey, "negotiationinclude")) {
+ return parse_transport_option(key, value,
+ &remote->negotiation_include);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
diff --git a/remote.h b/remote.h
index e6ec37c393..d8809b6991 100644
--- a/remote.h
+++ b/remote.h
@@ -118,6 +118,7 @@ struct remote {
struct string_list server_options;
struct string_list negotiation_restrict;
+ struct string_list negotiation_include;
enum follow_remote_head_settings follow_remote_head;
const char *no_warn_branch;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index bc2e2af959..33f61ac12a 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1587,6 +1587,55 @@ test_expect_success '--negotiation-include avoids duplicates with negotiator' '
test_line_count = 1 matches
'
+test_expect_success 'remote.<name>.negotiationInclude used as default for --negotiation-include' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ # test the reset of the list on an empty value
+ git -C client config --add remote.origin.negotiationInclude refs/tags/alpha_1 &&
+ git -C client config --add remote.origin.negotiationInclude "" &&
+ git -C client config --add remote.origin.negotiationInclude refs/tags/beta_1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=beta_2 \
+ origin alpha_s beta_s &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ test_grep ! "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success 'remote.<name>.negotiationInclude works with glob patterns' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ git -C client config --add remote.origin.negotiationInclude "refs/tags/beta_*" &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep "fetch> have $BETA_2" trace
+'
+
+test_expect_success 'CLI --negotiation-include overrides remote.<name>.negotiationInclude' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ git -C client config --add remote.origin.negotiationInclude refs/tags/beta_2 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep ! "fetch> have $BETA_2" trace
+'
+
test_expect_success '--negotiation-include avoids duplicates with v0' '
test_when_finished rm -f trace &&
setup_negotiation_tip server server 0 &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 8/8] send-pack: pass negotiation config in push
From: Derrick Stolee via GitGitGadget @ 2026-05-18 20:19 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v5.git.1779135575.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
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.<name>.negotiationInclude' and
'remote.<name>.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.
Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 6 ++++++
send-pack.c | 37 ++++++++++++++++++++++++++------
send-pack.h | 2 ++
t/t5516-fetch-push.sh | 30 ++++++++++++++++++++++++++
transport.c | 2 ++
5 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 1951df154e..eb9c8a3c48 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -122,6 +122,9 @@ command-line option. If `--negotiation-restrict` (or its synonym
`--negotiation-tip`) is specified on the command line, then the config
values are not used.
+
+These values also influence negotiation during `git push` if
+`push.negotiate` is enabled.
++
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
@@ -147,6 +150,9 @@ negotiation algorithm still runs and advertises its own selected commits,
but the refs matching `remote.<name>.negotiationInclude` are sent
unconditionally on top of those heuristically selected commits.
+
+These values also influence negotiation during `git push` if
+`push.negotiate` is enabled.
++
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
diff --git a/send-pack.c b/send-pack.c
index 3d5d36ba3b..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)) {
+
+ 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",
- oid_to_hex(&ref->new_oid));
- nr_negotiation_tip++;
+ 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;
}
@@ -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.<name>.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.<name>.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 fa54928966..a2d8958cb8 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;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 1/8] t5516: fix test order flakiness
From: Derrick Stolee via GitGitGadget @ 2026-05-18 20:19 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v5.git.1779135575.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The 'fetch follows tags by default' test sorts using 'sort -k 4', but
for-each-ref output only has 3 columns. This relies on sort treating records
with fewer fields as having an empty fourth field, which may produce
unstable results depending on locale. This appears to be an accident added
in 3f763ddf28 (fetch: set remote/HEAD if it does not exist, 2024-11-22).
Use 'sort -k 3' to match the actual number of columns in the output.
Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
t/t5516-fetch-push.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 29e2f17608..ac8447f21e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1349,7 +1349,7 @@ test_expect_success 'fetch follows tags by default' '
git for-each-ref >tmp1 &&
sed -n "p; s|refs/heads/main$|refs/remotes/origin/main|p" tmp1 |
sed -n "p; s|refs/heads/main$|refs/remotes/origin/HEAD|p" |
- sort -k 4 >../expect
+ sort -k 3 >../expect
) &&
test_when_finished "rm -rf dst" &&
git init dst &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/2] use __builtin_add_overflow() in st_add() with Clang
From: René Scharfe @ 2026-05-18 20:25 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <20260518202502.25682-1-l.s.r@web.de>
Clang and GCC optimize away comparisons of overflow checks by checking
the carry flag on x64. GCC does the same on ARM64, but Clang currently
(version 22.1) doesn't.
It does this optimization for overflow checks that use its builtin
function __builtin_add_overflow(), though. Provide a non-generic
lookalike for size_t that does the same checks as before as a fallback
and use the original with Clang. Use it on all platforms for simplicity.
On an Apple M1 I get a nice speedup for a command that builds lots of
strings using a strbuf, which exercises the st_add3() in strbuf_grow()
for every line of output:
Benchmark 1: ./git_main cat-file --batch-all-objects --batch-check='%(objectname)'
Time (mean ± σ): 120.4 ms ± 0.2 ms [User: 113.8 ms, System: 6.0 ms]
Range (min … max): 120.1 ms … 121.1 ms 24 runs
Benchmark 2: ./git cat-file --batch-all-objects --batch-check='%(objectname)'
Time (mean ± σ): 115.5 ms ± 0.1 ms [User: 108.6 ms, System: 5.8 ms]
Range (min … max): 115.2 ms … 115.8 ms 25 runs
Summary
./git cat-file --batch-all-objects --batch-check='%(objectname)' ran
1.04 ± 0.00 times faster than ./git_main cat-file --batch-all-objects --batch-check='%(objectname)'
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
git-compat-util.h | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index ae1bdc90a4..5b1d15fe4f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -614,12 +614,30 @@ static inline bool strip_suffix(const char *str, const char *suffix,
int git_open_cloexec(const char *name, int flags);
#define git_open(name) git_open_cloexec(name, O_RDONLY)
-static inline size_t st_add(size_t a, size_t b)
+
+/*
+ * Help Clang; GCC generates the same instructions for both variants on
+ * x64 and aarch64.
+ */
+#ifdef __clang__
+#define st_add_overflow __builtin_add_overflow
+#else
+static inline bool st_add_overflow(size_t a, size_t b, size_t *out)
{
if (unsigned_add_overflows(a, b))
+ return true;
+ *out = a + b;
+ return false;
+}
+#endif
+
+static inline size_t st_add(size_t a, size_t b)
+{
+ size_t result;
+ if (st_add_overflow(a, b, &result))
die("size_t overflow: %"PRIuMAX" + %"PRIuMAX,
(uintmax_t)a, (uintmax_t)b);
- return a + b;
+ return result;
}
#define st_add3(a,b,c) st_add(st_add((a),(b)),(c))
#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/2] use __builtin_add_overflow() in st_add() with Clang
From: René Scharfe @ 2026-05-18 20:25 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <c6e9b337-c4fc-4cbd-ac32-e8d3814749b0@web.de>
Changes since v2:
- Pass variable instead of st_add3() expression to ALLOC_GROW.
- Add the helper st_add_overflow() that mimics __builtin_add_overflow()
for size_t to avoid duplicating most of the definition of st_add().
strbuf: use st_add3() in strbuf_grow()
use __builtin_add_overflow() in st_add() with Clang
git-compat-util.h | 22 ++++++++++++++++++++--
strbuf.c | 6 ++----
2 files changed, 22 insertions(+), 6 deletions(-)
Interdiff against v1:
diff --git a/git-compat-util.h b/git-compat-util.h
index aa088d04bb..5b1d15fe4f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -614,25 +614,31 @@ static inline bool strip_suffix(const char *str, const char *suffix,
int git_open_cloexec(const char *name, int flags);
#define git_open(name) git_open_cloexec(name, O_RDONLY)
-/* Help Clang; GCC generates the same code for both variants. */
-#if defined(__clang__)
-static inline size_t st_add(size_t a, size_t b)
+
+/*
+ * Help Clang; GCC generates the same instructions for both variants on
+ * x64 and aarch64.
+ */
+#ifdef __clang__
+#define st_add_overflow __builtin_add_overflow
+#else
+static inline bool st_add_overflow(size_t a, size_t b, size_t *out)
{
- size_t sum;
- if (__builtin_add_overflow(a, b, &sum))
- die("size_t overflow: %"PRIuMAX" + %"PRIuMAX,
- (uintmax_t)a, (uintmax_t)b);
- return sum;
+ if (unsigned_add_overflows(a, b))
+ return true;
+ *out = a + b;
+ return false;
}
-#else
+#endif
+
static inline size_t st_add(size_t a, size_t b)
{
- if (unsigned_add_overflows(a, b))
+ size_t result;
+ if (st_add_overflow(a, b, &result))
die("size_t overflow: %"PRIuMAX" + %"PRIuMAX,
(uintmax_t)a, (uintmax_t)b);
- return a + b;
+ return result;
}
-#endif
#define st_add3(a,b,c) st_add(st_add((a),(b)),(c))
#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))
diff --git a/strbuf.c b/strbuf.c
index bb04d3910e..8610965d53 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -106,9 +106,10 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)
void strbuf_grow(struct strbuf *sb, size_t extra)
{
int new_buf = !sb->alloc;
+ size_t new_len = st_add3(sb->len, extra, 1);
if (new_buf)
sb->buf = NULL;
- ALLOC_GROW(sb->buf, st_add3(sb->len, extra, 1), sb->alloc);
+ ALLOC_GROW(sb->buf, new_len, sb->alloc);
if (new_buf)
sb->buf[0] = '\0';
}
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/2] strbuf: use st_add3() in strbuf_grow()
From: René Scharfe @ 2026-05-18 20:25 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <20260518202502.25682-1-l.s.r@web.de>
Simplify the code by calling st_add3() to do overflow checks instead of
open-coding it. This changes the error message to include the offending
summands, which can be helpful when tracking down the cause.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
strbuf.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 3e04addc22..8610965d53 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -106,12 +106,10 @@ void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)
void strbuf_grow(struct strbuf *sb, size_t extra)
{
int new_buf = !sb->alloc;
- if (unsigned_add_overflows(extra, 1) ||
- unsigned_add_overflows(sb->len, extra + 1))
- die("you want to use way too much memory");
+ size_t new_len = st_add3(sb->len, extra, 1);
if (new_buf)
sb->buf = NULL;
- ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
+ ALLOC_GROW(sb->buf, new_len, sb->alloc);
if (new_buf)
sb->buf[0] = '\0';
}
--
2.54.0
^ permalink raw reply related
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
From: Junio C Hamano @ 2026-05-19 0:03 UTC (permalink / raw)
To: Pablo Sabater
Cc: Chandra Pratap, phillip.wood, git, christian.couder, karthik.188,
jltobler, ayu.chandekar, siddharthasthana31
In-Reply-To: <CAN5EUNQoKRqt3FGLmzRGpPU1nO5jCAogP8Wm9gBZXuPbMNbQAw@mail.gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> By having is_parentless as a flag in 'git_graph' that every stage can
> access we could modify the rendering and maybe completely drop the
> commit placeholders, working on it for v4 but currently renders like
> this
>
> * A parentless
> * B parentless
> * C parentless
> * D1 child
> * D parentless
>
> (A has indentation when it could not have, but that would require a
> lookahead if the next commit is also parentless)
> But definitely a step forward.
>
> Do we want cascading or just a fixed indentation?
>
> * A parentless
> * B parentless
> * C parentless
> * D1 child
> * D parentless
I am late to the party, but I cannot get how the latter is viable.
If "A" had parent "B" whose parent was "C" that is root, wouldn't we
see the same output? Or are we adding " parentless" at the end of
the one-liner log message?
The former, with the understanding that "two '*' commit marks
vertically adjacent have parent-child relationship, otherwise we
draw line between '*' to connect them if they have parent-child
relationship", does not have such a problem.
^ permalink raw reply
* Re: [PATCH] evaluate the second argument of ALLOC_GROW only once
From: Jeff King @ 2026-05-19 0:41 UTC (permalink / raw)
To: Johannes Sixt; +Cc: René Scharfe, Git List, Junio C Hamano
In-Reply-To: <9ce768d4-0cbf-4494-a1d3-55fd3b05b61e@kdbg.org>
On Sat, May 16, 2026 at 08:55:54AM +0200, Johannes Sixt wrote:
> > Hmm, playing with it and looking a little closer, I think we don't end
> > up overflowing the buffer because you use the size_t for
> > REALLOC_ARRAY(). So the result is big, but then "alloc" is truncated.
>
> Protect against double-evaluation of "alloc", too, using
>
> size_t *palloc = &(alloc);
>
> and use *palloc in the two places, then all callers are forced to work
> with a size_t as third argument. Don't know what the damage would be,
> though.
I think it would be nice if all ALLOC_GROW() callers used a size_t, and
then we checked the size_t computation for overflow. But from a rough
guess (taking your suggestion and trying to compile) we'd need to adjust
~200 callers.
And it's not just a straight conversion:
1. Sometimes the ability to represent a negative value is important,
and each site has to be audited. If we could agree on a "as big as
size_t but signed" type, that might help.
2. Changing the alloc variable type without the matching "nr" can
actually make things worse. We tend to catch overflow-by-1 for
signed types incidentally because it results in a stupidly large
allocation request. But if made our allocations correct, then we
might overflow on "nr" and start writing to some huge negative
offset before the array.
So I think it would be a fair bit of work, though I would feel better
about the resulting state.
-Peff
^ permalink raw reply
* Re: [PATCH v2 0/2] use __builtin_add_overflow() in st_add() with Clang
From: Jeff King @ 2026-05-19 0:44 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Junio C Hamano
In-Reply-To: <20260518202502.25682-1-l.s.r@web.de>
On Mon, May 18, 2026 at 10:25:00PM +0200, René Scharfe wrote:
> Changes since v2:
> - Pass variable instead of st_add3() expression to ALLOC_GROW.
> - Add the helper st_add_overflow() that mimics __builtin_add_overflow()
> for size_t to avoid duplicating most of the definition of st_add().
>
> strbuf: use st_add3() in strbuf_grow()
> use __builtin_add_overflow() in st_add() with Clang
Thanks, this seems reasonable to me. The type-generic version of
builtin_add_overflow() is much harder, but doing it just for st_add() is
enough for our purposes here.
-Peff
^ permalink raw reply
* Re: [PATCH] alias: restore support for simple dotted aliases
From: Jeff King @ 2026-05-19 0:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonatan Holmgren, git, rsch, michael.grossfeld
In-Reply-To: <xmqqbjelp7ab.fsf@gitster.g>
On Tue, May 12, 2026 at 01:43:08PM +0900, Junio C Hamano wrote:
> Jonatan Holmgren <jonatan@jontes.page> writes:
>
> > Sorry, that wasn't a "hey we should deprecate this" code-wise, I was
> > asking from a documentation point of view, i.e. was curious how you felt
> > about what is "advisable". Shouldn't've included that in my email
>
> After this, the discussion went dark, but I think everything that
> needs saying has been said and we are in agreement that the current
> patch is a good way forward without closing doors for the future too
> tightly ;-) Let me mark the topic for 'next'.
Yeah, sorry I didn't respond to Jonatan. I think the patch as-is is
fine, and if we want to push people towards the new form in the
documentation, that can be done separately.
-Peff
^ permalink raw reply
* Re: [PATCH] revision: use priority queue in limit_list()
From: Jeff King @ 2026-05-19 0:54 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Derrick Stolee, Junio C Hamano,
Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <CAL71e4MxhcZqxPVEe38Shuqt7h5dxLDGi66hN2cFXnmg-POKWA@mail.gmail.com>
On Sun, May 17, 2026 at 05:26:06PM +0200, Kristofer Karlsson wrote:
> Another note - I think I managed to apply the same change to
> get_revision_1 too - speeding up a monorepo "git rev-list HEAD" by
> 3.3x so it seems like a reasonable thing to do.
> This simplifies process_parents and also makes
> commit_list_insert_by_date dead code.
>
> The only caveat is that get_revision_1 starts to get messier and the
> rev_info struct needs both a prio_queue and a linked list of commits -
> and then flushing everything
> from the list into the prio_queue when executing get_revision_1.
IMHO it is worth replacing rev_info's list with a prio_queue and letting
that be the source of authority. You do have to be careful to cover
cases where the list _isn't_ date-sorted, but prio_queue supports that
with a NULL comparator.
You do still have to convert between list and queue at a few spots, but
I think in the long run many of those could be converted to use a queue.
You can see my patches to do so at:
https://github.com/peff/git jk/revs-commits-prio-queue
I've been running with them locally for a few years. Mostly I hadn't
gotten around to polishing them, and I think I had wanted to do some
more perf testing. It sounds like you have a good candidate repo for
showing off the improvement. ;)
If you'd like to go in that direction, please feel free to pick out
whatever is useful from what you find on that branch.
> I don't want to pollute this patch with that change - should I start a
> separate thread for it or just revisit this later?
> (Perhaps I have too many optimization patches in flux already)
Yes, it definitely makes sense to do that as a separate change. If you
look at the patches I linked above, note that they'll get a bit simpler
by rebasing on top of your limit_list() changes, since it does some of
the same things.
-Peff
^ permalink raw reply
* Re: [PATCH v2 0/2] use __builtin_add_overflow() in st_add() with Clang
From: Junio C Hamano @ 2026-05-19 0:57 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Jeff King
In-Reply-To: <20260518202502.25682-1-l.s.r@web.de>
René Scharfe <l.s.r@web.de> writes:
> Changes since v2:
> - Pass variable instead of st_add3() expression to ALLOC_GROW.
> - Add the helper st_add_overflow() that mimics __builtin_add_overflow()
> for size_t to avoid duplicating most of the definition of st_add().
>
> strbuf: use st_add3() in strbuf_grow()
> use __builtin_add_overflow() in st_add() with Clang
Nice simplification without becoming overly ambitious. Very well
done.
Let me mark it for 'next'.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/2] commit-reach: use object flags for tips_reachable_from_bases()
From: Jeff King @ 2026-05-19 1:03 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget
Cc: git, Kristofer Karlsson, Derrick Stolee
In-Reply-To: <pull.2116.v3.git.1778947182.gitgitgadget@gmail.com>
On Sat, May 16, 2026 at 03:59:39PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> v2 of this patch, addressing Jeff King's feedback:
>
> * Replaced the decoration hash with the RESULT object flag (simpler, no
> extra data structure, handles duplicate tips naturally)
> * Fixed early-termination bug when multiple refs point to the same commit
> (the decoration API overwrites on duplicate keys)
> * Removed the now-unused index field from struct commit_and_index
> * Diff is +11/-12 lines
Using the object flag here is so much nicer. I see you're reusing the
RESULT flag. I'm not sure offhand if there might be any conflict with
other uses of that flag bit. I think probably not, since it looks like
it is cleared by the other users after they leave their respective
functions?
Using a direct set-inclusion check with the flag is nice, but we still
look at min_generation_index. If I'm understanding the code right, this
is mostly about counting the tips we've seen. Which at first glance
means we could probably replace that code with some kind of counter. But
I think maybe there is some notion of "crossing off" commits which we
don't actually visit, but which we know become un-visitable because we
traverse past their generation numbers.
I think. This is really the first time I'm looking at this code. So
AFAICT your patch as-is is correct, but it would be nice to go an ACK
from Stolee.
-Peff
^ permalink raw reply
* Re: [PATCH v8] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-05-19 1:04 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble, Johannes Sixt,
Chris Torek, Mirko Faina
In-Reply-To: <8210d60832b9a58aa4d71fc3790e44d8989564ce.1779152064.git.mroik@delayed.space>
On Tue, May 19, 2026 at 02:55:22AM +0200, Mirko Faina wrote:
> --max-count is a commit limiting option sets a maximum amount of commits
> to be shown. If a user wants to see only the first N commits of the
> history (the oldest commits) they'd have to do something like
>
> git log $(git rev-list HEAD | tail -n N | head -n 1)
>
> This is not very user-friendly.
>
> Teach get_revision() the --max-count-oldest option.
>
> Signed-off-by: Mirko Faina <mroik@delayed.space>
> ---
> Documentation/rev-list-options.adoc | 5 +-
> revision.c | 111 +++++++++++++++++++++++++++-
> revision.h | 2 +
> t/t4202-log.sh | 41 ++++++++++
> 4 files changed, 155 insertions(+), 4 deletions(-)
Sorry, forgot to write down what changed since v7. There was an issue
with the counting as --max-count-oldest counted boundary commits too.
That is simply solved by only adding on non boundaries.
That left another issue, there are now some "orphaned" boundaries when
printing the graph. In addition to that, because of how the graph
machinery works, the graph is now trying to include the parents of the
orphaned boundaries. To fix this we just flip the CHILD_SHOWN flag on
the parents of the commit we're discarding.
Hopefully this is the last version.
^ permalink raw reply
* [PATCH v8] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-05-19 0:55 UTC (permalink / raw)
To: git
Cc: Mirko Faina, Junio C Hamano, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble, Johannes Sixt,
Chris Torek
In-Reply-To: <463cc8e2764edb7de3d379f615f5cfbd0919bfa3.1778887662.git.mroik@delayed.space>
--max-count is a commit limiting option sets a maximum amount of commits
to be shown. If a user wants to see only the first N commits of the
history (the oldest commits) they'd have to do something like
git log $(git rev-list HEAD | tail -n N | head -n 1)
This is not very user-friendly.
Teach get_revision() the --max-count-oldest option.
Signed-off-by: Mirko Faina <mroik@delayed.space>
---
Documentation/rev-list-options.adoc | 5 +-
revision.c | 111 +++++++++++++++++++++++++++-
revision.h | 2 +
t/t4202-log.sh | 41 ++++++++++
4 files changed, 155 insertions(+), 4 deletions(-)
diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..e8c88d0f1c 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -16,7 +16,10 @@ ordering and formatting options, such as `--reverse`.
`-<number>`::
`-n <number>`::
`--max-count=<number>`::
- Limit the output to _<number>_ commits.
+ Limit the output to the first _<number>_ commits that would be shown.
+
+`--max-count-oldest=<number>`::
+ Limit the output to the last _<number>_ commits that would be shown.
`--skip=<number>`::
Skip _<number>_ commits before starting to show the commit output.
diff --git a/revision.c b/revision.c
index 599b3a66c3..5d53db3152 100644
--- a/revision.c
+++ b/revision.c
@@ -2339,10 +2339,28 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
}
if ((argcount = parse_long_opt("max-count", argv, &optarg))) {
+ if (revs->max_count_type == 1)
+ die_for_incompatible_opt2(1, "--max-count", 1,
+ "--max-count-oldest");
revs->max_count = parse_count(optarg);
revs->no_walk = 0;
+ revs->max_count_type = 0;
return argcount;
+ } else if ((argcount = parse_long_opt("max-count-oldest", argv, &optarg))) {
+ if (revs->max_count_type == 0 && revs->max_count != -1)
+ die_for_incompatible_opt2(1, "--max-count", 1,
+ "--max-count-oldest");
+ if (revs->skip_count > 0)
+ die_for_incompatible_opt2(1, "--skip", 1,
+ "--max-count-oldest");
+ revs->max_count = parse_count(optarg);
+ revs->no_walk = 0;
+ revs->max_count_type = 1;
+ revs->max_count_stage = 0;
} else if ((argcount = parse_long_opt("skip", argv, &optarg))) {
+ if (revs->max_count_type == 1)
+ die_for_incompatible_opt2(1, "--skip", 1,
+ "--max-count-oldest");
revs->skip_count = parse_count(optarg);
return argcount;
} else if ((*arg == '-') && isdigit(arg[1])) {
@@ -4521,15 +4539,91 @@ static struct commit *get_revision_internal(struct rev_info *revs)
return c;
}
+static void retrieve_oldest_commits(struct rev_info *revs,
+ struct commit_list **queue)
+{
+ struct commit *c;
+ int max_count = revs->max_count;
+ int queuei_count = 0;
+ int queueo_count = 0;
+ struct commit_list *queueo = NULL;
+ struct commit_list *queuei = NULL;
+ struct commit_list *reversed_queue = NULL;
+ struct commit_list *p;
+
+ revs->max_count = -1;
+ while ((c = get_revision_internal(revs))) {
+ /*
+ * We need to reset SHOWN status otherwise --graph breaks.
+ * It is fine to do, get_revision_internal() doesn't consider
+ * children commits as they have been already processed and the
+ * traversal happens only child to parent.
+ *
+ * We do this because the --graph machinery relies on the status
+ * of the parents to decide how the printing will happen.
+ *
+ * We can't simply replace this instruction with a
+ * graph_update() as it doesn't do the actualy printing, we'd
+ * have to remove any commit that goes over the
+ * --max-count-oldest limit from revs->graph.
+ */
+ c->object.flags &= ~(SHOWN | CHILD_SHOWN);
+ commit_list_insert(c, &queuei);
+ if (!(c->object.flags & BOUNDARY))
+ queuei_count++;
+ while (queuei_count + queueo_count > max_count) {
+ if (!queueo_count) {
+ while ((c = pop_commit(&queuei))) {
+ commit_list_insert(c, &queueo);
+ queueo_count++;
+ }
+ queuei_count = 0;
+ }
+ c = pop_commit(&queueo);
+ queueo_count--;
+ /* We need to do this otherwise we'll discard the
+ * commits that go over the --max-count-oldest limit but
+ * not their respective boundaries. This matters only if
+ * we're discarding the commit right before the boundary.
+ */
+ for (p = c->parents; p; p = p->next)
+ p->item->object.flags &= ~CHILD_SHOWN;
+ }
+ }
+
+ while ((c = pop_commit(&queueo)))
+ commit_list_insert(c, &reversed_queue);
+ while ((c = pop_commit(&queuei)))
+ commit_list_insert(c, &queueo);
+ while ((c = pop_commit(&queueo)))
+ commit_list_insert(c, &reversed_queue);
+
+ while ((c = pop_commit(&reversed_queue)))
+ commit_list_insert(c, queue);
+}
+
struct commit *get_revision(struct rev_info *revs)
{
struct commit *c;
struct commit_list *reversed;
+ struct commit_list *queue = NULL;
+ struct commit_list *p;
+
+ if (revs->max_count_type == 1 && !revs->max_count_stage) {
+ retrieve_oldest_commits(revs, &queue);
+ commit_list_free(revs->commits);
+ revs->commits = queue;
+ revs->max_count_stage = 1;
+ }
if (revs->reverse) {
reversed = NULL;
- while ((c = get_revision_internal(revs)))
- commit_list_insert(c, &reversed);
+ if (revs->max_count_type == 1)
+ while ((c = pop_commit(&revs->commits)))
+ commit_list_insert(c, &reversed);
+ else
+ while ((c = get_revision_internal(revs)))
+ commit_list_insert(c, &reversed);
commit_list_free(revs->commits);
revs->commits = reversed;
revs->reverse = 0;
@@ -4543,7 +4637,18 @@ struct commit *get_revision(struct rev_info *revs)
return c;
}
- c = get_revision_internal(revs);
+ if (revs->max_count_stage) {
+ c = pop_commit(&revs->commits);
+ if (c) {
+ c->object.flags |= SHOWN;
+ if (!(c->object.flags & BOUNDARY))
+ for (p = c->parents; p; p = p->next)
+ p->item->object.flags |= CHILD_SHOWN;
+ }
+ } else {
+ c = get_revision_internal(revs);
+ }
+
if (c && revs->graph)
graph_update(revs->graph, c);
if (!c) {
diff --git a/revision.h b/revision.h
index 584f1338b5..e157463cb1 100644
--- a/revision.h
+++ b/revision.h
@@ -309,6 +309,8 @@ struct rev_info {
/* special limits */
int skip_count;
int max_count;
+ unsigned int max_count_type:1;
+ unsigned int max_count_stage:1;
timestamp_t max_age;
timestamp_t max_age_as_filter;
timestamp_t min_age;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 05cee9e41b..c3c1b862d3 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1882,6 +1882,47 @@ test_expect_success 'log --graph with --name-status' '
test_cmp_graph --name-status tangle..reach
'
+test_expect_success 'log --max-count-oldest=3 --oneline' '
+ test_when_finished rm expect &&
+ git log --oneline | tail -n3 >expect &&
+ git log --oneline --max-count-oldest=3 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '
+ test_when_finished rm expect &&
+ git log --oneline --reverse | head -n3 >expect &&
+ git log --oneline --max-count-oldest=3 --reverse >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --max-count-oldest with --max-count' '
+ test_when_finished rm stderr &&
+ test_must_fail git log --max-count-oldest=3 --max-count=3 2>stderr &&
+ test_grep "cannot be used together" stderr
+'
+
+test_expect_success 'log --max-count-oldest with --skip' '
+ test_when_finished rm stderr &&
+ test_must_fail git log --max-count-oldest=3 --skip=1 2>stderr &&
+ test_grep "cannot be used together" stderr
+'
+
+test_expect_success 'log --max-count-oldest=1000 --graph --boundary' '
+ test_when_finished rm expect actual &&
+ git log --graph --boundary >expect &&
+ git log --max-count-oldest=1000 --graph --boundary >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'log --oneline --graph --boundary --max-count-oldest=1' '
+ test_when_finished rm actual &&
+ echo 2 >expect &&
+ git log --oneline --graph --boundary --max-count-oldest=1 HEAD~1..HEAD \
+ | wc -l >actual &&
+ test_cmp expect actual
+'
+
cat >expect <<-\EOF
* reach
|
--
2.54.0
^ permalink raw reply related
* [PATCH 0/3] small quote.[ch] cleanup
From: Jeff King @ 2026-05-19 1:18 UTC (permalink / raw)
To: git
I noticed some unused code while looking at an unrelated topic. So
here's a small cleanup.
[1/3]: quote.h: bump strvec forward declaration to the top
[2/3]: quote: drop sq_dequote_to_argv()
[3/3]: quote: simplify internals of dequoting
quote.c | 21 ++-------------------
quote.h | 14 ++++----------
2 files changed, 6 insertions(+), 29 deletions(-)
-Peff
^ permalink raw reply
* [PATCH 1/3] quote.h: bump strvec forward declaration to the top
From: Jeff King @ 2026-05-19 1:19 UTC (permalink / raw)
To: git
In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net>
We usually put forward declarations at the top of header files, rather
than next to the functions that need them. In theory placing it next to
the function has some explanatory value, but it's also just as likely to
become stale if other uses are added.
Signed-off-by: Jeff King <peff@peff.net>
---
quote.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/quote.h b/quote.h
index 0300c29104..400397b11a 100644
--- a/quote.h
+++ b/quote.h
@@ -2,6 +2,7 @@
#define QUOTE_H
struct strbuf;
+struct strvec;
extern int quote_path_fully;
@@ -77,7 +78,6 @@ int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);
* still modify arg in place, but unlike sq_dequote_to_argv, the strvec
* will duplicate and take ownership of the strings.
*/
-struct strvec;
int sq_dequote_to_strvec(char *arg, struct strvec *);
int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);
--
2.54.0.524.g198262df96
^ permalink raw reply related
* [PATCH 2/3] quote: drop sq_dequote_to_argv()
From: Jeff King @ 2026-05-19 1:19 UTC (permalink / raw)
To: git
In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net>
The last caller went away in f9dbb64fad (config: parse more robust
format in GIT_CONFIG_PARAMETERS, 2021-01-12), when we switched to using
sq_dequote_step().
The "to_argv()" form is not a great interface. If you care about raw
speed, then sq_dequote_step() lets you work incrementally without extra
allocations. If you care about simplicity, then sq_dequote_to_strvec()
puts the result in an encapsulated data structure. With sq_dequote_to_argv(),
you have a data dependency on the original string but still have to
remember to manually free the argv array itself (but not its elements).
So it's sort of a worst-of-both-worlds middle ground. Let's get rid of
it.
Signed-off-by: Jeff King <peff@peff.net>
---
quote.c | 5 -----
quote.h | 12 +++---------
2 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/quote.c b/quote.c
index b9f6bdc775..cff78af3a4 100644
--- a/quote.c
+++ b/quote.c
@@ -202,11 +202,6 @@ static int sq_dequote_to_argv_internal(char *arg,
return 0;
}
-int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc)
-{
- return sq_dequote_to_argv_internal(arg, argv, nr, alloc, NULL);
-}
-
int sq_dequote_to_strvec(char *arg, struct strvec *array)
{
return sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array);
diff --git a/quote.h b/quote.h
index 400397b11a..989f2388c0 100644
--- a/quote.h
+++ b/quote.h
@@ -68,15 +68,9 @@ char *sq_dequote_step(char *src, char **next);
/*
* Same as the above, but can be used to unwrap many arguments in the
- * same string separated by space. Like sq_quote, it works in place,
- * modifying arg and appending pointers into it to argv.
- */
-int sq_dequote_to_argv(char *arg, const char ***argv, int *nr, int *alloc);
-
-/*
- * Same as above, but store the unquoted strings in a strvec. We will
- * still modify arg in place, but unlike sq_dequote_to_argv, the strvec
- * will duplicate and take ownership of the strings.
+ * same string separated by space. The strvec will duplicate and take
+ * ownership of the strings, but note that "arg" is still modified in-place
+ * during parsing.
*/
int sq_dequote_to_strvec(char *arg, struct strvec *);
--
2.54.0.524.g198262df96
^ permalink raw reply related
* [PATCH 3/3] quote: simplify internals of dequoting
From: Jeff King @ 2026-05-19 1:20 UTC (permalink / raw)
To: git
In-Reply-To: <20260519011837.GA1615637@coredump.intra.peff.net>
Our sq_dequote_to_argv_internal() helper was wrapped by the to_argv()
and to_strvec() forms. Now that we have only the latter, we can stop
wrapping it and drop the argv-only bits.
Note that in theory sq_dequote_to_strvec() could take a const input
string, which would be friendlier to its callers. We couldn't do that
with the to_argv() form because it reused the input string to hold the
output elements. But since we're built on sq_dequote_step(), which
munges the input, we'd have to rework the parser. Since no callers care
about it currently, we'll leave that for another day.
Signed-off-by: Jeff King <peff@peff.net>
---
quote.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/quote.c b/quote.c
index cff78af3a4..235fac8e47 100644
--- a/quote.c
+++ b/quote.c
@@ -171,9 +171,7 @@ char *sq_dequote(char *arg)
return sq_dequote_step(arg, NULL);
}
-static int sq_dequote_to_argv_internal(char *arg,
- const char ***argv, int *nr, int *alloc,
- struct strvec *array)
+int sq_dequote_to_strvec(char *arg, struct strvec *array)
{
char *next = arg;
@@ -191,22 +189,12 @@ static int sq_dequote_to_argv_internal(char *arg,
c = *++next;
} while (isspace(c));
}
- if (argv) {
- ALLOC_GROW(*argv, *nr + 1, *alloc);
- (*argv)[(*nr)++] = dequoted;
- }
- if (array)
- strvec_push(array, dequoted);
+ strvec_push(array, dequoted);
} while (next);
return 0;
}
-int sq_dequote_to_strvec(char *arg, struct strvec *array)
-{
- return sq_dequote_to_argv_internal(arg, NULL, NULL, NULL, array);
-}
-
/* 1 means: quote as octal
* 0 means: quote as octal if (quote_path_fully)
* -1 means: never quote
--
2.54.0.524.g198262df96
^ permalink raw reply related
* Re: [PATCH 02/18] setup: stop using `the_repository` in `is_inside_worktree()`
From: Junio C Hamano @ 2026-05-19 1:22 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Tian Yuchen, git, Karthik Nayak, Elijah Newren
In-Reply-To: <agrD4p1AIPtwa5gW@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
>> I do not offhand know if other code paths that are called from this
>> function are thread-safe, but yeah, this use of file-scope static is
>> not a safe thing to do.
>
> In the current status quo this is a safe conversion to do, as we still
> have the assumption ingrained that this only works for a single repo.
> We were using static variables before, and we're still using static
> variables now.
>
> That being said, I wouldn't mind dropping the static variable if this is
> something we'd rather want to get rid of. It's a smell that I'm not
> particularly happy about myself.
>
> I'll send a revised version in a bit, thanks!
OK. In the v2 (perhaps v3?) series, I can see the removal of two
static strbuf based "optimizations" is the only change since the
previous round (after merging the previous one to the base of the
new iteration and comparing the result with the new iteration).
Looking very good. Shall we mark the topic for 'next' now?
Thanks.
diff --git w/setup.c c/setup.c
index 1847111616..6aee839d8c 100644
--- w/setup.c
+++ c/setup.c
@@ -471,17 +471,26 @@ int is_nonbare_repository_dir(struct strbuf *path)
int is_inside_git_dir(struct repository *repo)
{
- static struct strbuf buf = STRBUF_INIT;
- return is_inside_dir(strbuf_realpath(&buf, repo_get_git_dir(repo), 1));
+ struct strbuf buf = STRBUF_INIT;
+ int ret = is_inside_dir(strbuf_realpath(&buf, repo_get_git_dir(repo), 1));
+ strbuf_release(&buf);
+ return ret;
}
int is_inside_work_tree(struct repository *repo)
{
- static struct strbuf buf = STRBUF_INIT;
- const char *worktree = repo_get_work_tree(repo);
+ struct strbuf buf = STRBUF_INIT;
+ const char *worktree;
+ int ret;
+
+ worktree = repo_get_work_tree(repo);
if (!worktree)
return 0;
- return is_inside_dir(strbuf_realpath(&buf, worktree, 1));
+
+ ret = is_inside_dir(strbuf_realpath(&buf, worktree, 1));
+
+ strbuf_release(&buf);
+ return ret;
}
void setup_work_tree(struct repository *repo)
^ permalink raw reply related
* Re: [BUG] "git diff --word-diff" gives a diff while they are only space changes
From: Michael Montalbo @ 2026-05-19 2:07 UTC (permalink / raw)
To: Johannes Sixt; +Cc: vincent, git, Junio C Hamano
In-Reply-To: <89224cb5-27b1-45b6-93d8-a0ad5e2447a2@kdbg.org>
On Mon, May 18, 2026 at 12:30 AM Johannes Sixt <j6t@kdbg.org> wrote:
>
> Am 18.05.26 um 05:30 schrieb Michael Montalbo:
> > On Thu, May 14, 2026 at 12:37 AM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Michael Montalbo <mmontalbo@gmail.com> writes:
> >>
> >>> @@ -457,6 +457,11 @@ endif::git-diff[]
> >>> +
> >>> Note that despite the name of the first mode, color is used to
> >>> highlight the changed parts in all modes if enabled.
> >>> ++
> >>> +Word diff works by finding word-level changes within each hunk of
> >>> +the line-level diff. The line-level alignment determines which
> >>> +changed lines are compared to each other, which can affect the
> >>> +word-level output.
> >>
> >> The added text may not say anything wrong, but I am not sure how it
> >> helps the end user to know the way machinery works internally.
> >>
> >
> > I see what you mean. Maybe the doc should focus more on calling out
> > the user-facing implication:
> >
> > `--word-diff` finds word-level changes within each hunk of the
> > line-level diff, so changes that only affect whitespace may still
> > appear in the output.
> I don't know what this paragraph is trying to explain. I don't see how
> this would explain Vincent's observed word-diff.
>
Yeah, I was trying to explain the difference Vincent saw compared to wdiff,
but I agree with your criticism. In "beating around the bush" regarding
implementation details / making a direct comparison to wdiff, it has been
hard to craft a meaningful message.
> The thing is, "word-diff" is such a descriptive name for the operation
> that it is difficult to find a description that is even better. The
> manual page doesn't even give it a try. It defers to --word-diff-regex
> right away, which then only talks about low-level details and doesn't
> attempt to give a higher-level description what a word-diff is.
>
> I don't think you can summarize the algorithm in a single sentence. But
> then I have to ask: why write it down anyway? How does it help the
> reader? Only so that they are able to derive an explanation for a
> particular observed output? Would it have saved Vincent to write a bug
> report?
>
> If we document the algorithm in such detail, we cast it in stone. I
> wouldn't want to paint ourselves into that corner.
>
I also agree with this sentiment. I haven't been able to come up with a
message that threads the needle appropriately, so I'm open to dropping
the patch or reworking it if others have suggestions.
> -- Hannes
>
^ permalink raw reply
* Re: [BUG] "git diff --word-diff" gives a diff while they are only space changes
From: Chris Torek @ 2026-05-19 2:31 UTC (permalink / raw)
To: Michael Montalbo; +Cc: Johannes Sixt, vincent, git, Junio C Hamano
In-Reply-To: <CAC2Qwm+BLNf-2kvePKNF-FKQX3raOBzSRmwd0ZEdzmo8TqkMGA@mail.gmail.com>
On Mon, May 18, 2026 at 7:11 PM Michael Montalbo <mmontalbo@gmail.com> wrote:
> Yeah, I was trying to explain the difference Vincent saw compared to wdiff,
> but I agree with your criticism. In "beating around the bush" regarding
> implementation details / making a direct comparison to wdiff, it has been
> hard to craft a meaningful message.
My opinion is: don't do that, just get right to it.
> > If we document the algorithm in such detail, we cast it in stone. I
> > wouldn't want to paint ourselves into that corner.
> I also agree with this sentiment. I haven't been able to come up with a
> message that threads the needle appropriately, so I'm open to dropping
> the patch or reworking it if others have suggestions.
Call it an "implementation note" (or, if you like, a "practical
consideration"?).
Something along these lines might work...
Implementation Note
The --word-diff option currently operates by taking the same
line by line diff that you get without the option, then massaging
the result into a word-by-word difference. This may cause an
unnecessarily-larger diff than you would see with a more-clever
implementation. If and when Git acquires a more-clever
implementation, the output may change. Note that this is
similar to the --diff-algorithm option, which may change the
output.
Regardless of which algorithm is used, _any_ diff simply shows
_a_ way to achieve some particular change. It's impossible for
any algorithm to tell whether someone deleted two lines and
then put one back exactly as it appeared earlier, saving the
resulting text, vs deleting a single line, for instance. Only a
keystroke-by-keystroke logger would be able to tell what the
human operator actually typed into some editor. Git does
not have that information, and having it is not desired.
Chris
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox