* [PATCH v6 03/10] help: move tty check for autocorrection to autocorrect.c
From: Jiamu Sun @ 2026-04-23 1:37 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Aaron Plattner, Karthik Nayak, Jiamu Sun
In-Reply-To: <SY0P300MB0801677A2A1E0FD38D06A841CE2A2@SY0P300MB0801.AUSP300.PROD.OUTLOOK.COM>
TTY checking is the autocorrect config parser's responsibility. It must
ensure the parsed value is correct and reliable. Thus, move the check to
autocorrect_resolve_config().
Signed-off-by: Jiamu Sun <39@barroit.sh>
---
autocorrect.c | 24 ++++++++++++++++--------
help.c | 6 ------
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/autocorrect.c b/autocorrect.c
index 97145d3a53ce..887d2396da44 100644
--- a/autocorrect.c
+++ b/autocorrect.c
@@ -33,18 +33,26 @@ void autocorrect_resolve_config(const char *var, const char *value,
const struct config_context *ctx, void *data)
{
int *out = data;
+ int parsed;
- if (!strcmp(var, "help.autocorrect")) {
- int v = parse_autocorrect(value);
+ if (strcmp(var, "help.autocorrect"))
+ return;
- if (!v) {
- v = git_config_int(var, value, ctx->kvi);
- if (v < 0 || v == 1)
- v = AUTOCORRECT_IMMEDIATELY;
- }
+ parsed = parse_autocorrect(value);
- *out = v;
+ /*
+ * Disable autocorrection prompt in a non-interactive session
+ */
+ if (parsed == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2)))
+ parsed = AUTOCORRECT_NEVER;
+
+ if (!parsed) {
+ parsed = git_config_int(var, value, ctx->kvi);
+ if (parsed < 0 || parsed == 1)
+ parsed = AUTOCORRECT_IMMEDIATELY;
}
+
+ *out = parsed;
}
void autocorrect_confirm(int autocorrect, const char *assumed)
diff --git a/help.c b/help.c
index ab619ed43c7a..d2b29715817e 100644
--- a/help.c
+++ b/help.c
@@ -607,12 +607,6 @@ char *help_unknown_cmd(const char *cmd)
read_early_config(the_repository, git_unknown_cmd_config, &cfg);
- /*
- * Disable autocorrection prompt in a non-interactive session
- */
- if ((cfg.autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
- cfg.autocorrect = AUTOCORRECT_NEVER;
-
if (cfg.autocorrect == AUTOCORRECT_NEVER) {
fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
exit(1);
--
2.54.0
^ permalink raw reply related
* [PATCH v6 02/10] help: make autocorrect handling reusable
From: Jiamu Sun @ 2026-04-23 1:37 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Aaron Plattner, Karthik Nayak, Jiamu Sun
In-Reply-To: <SY0P300MB0801677A2A1E0FD38D06A841CE2A2@SY0P300MB0801.AUSP300.PROD.OUTLOOK.COM>
Move config parsing and prompt/delay handling into autocorrect.c and
expose them in autocorrect.h. This makes autocorrect reusable regardless
of which target links against it.
Signed-off-by: Jiamu Sun <39@barroit.sh>
---
Makefile | 1 +
autocorrect.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
autocorrect.h | 16 ++++++++++++
help.c | 64 +++------------------------------------------
meson.build | 1 +
5 files changed, 94 insertions(+), 60 deletions(-)
create mode 100644 autocorrect.c
create mode 100644 autocorrect.h
diff --git a/Makefile b/Makefile
index cedc234173e3..ced8a9b5abb4 100644
--- a/Makefile
+++ b/Makefile
@@ -1100,6 +1100,7 @@ LIB_OBJS += archive-tar.o
LIB_OBJS += archive-zip.o
LIB_OBJS += archive.o
LIB_OBJS += attr.o
+LIB_OBJS += autocorrect.o
LIB_OBJS += base85.o
LIB_OBJS += bisect.o
LIB_OBJS += blame.o
diff --git a/autocorrect.c b/autocorrect.c
new file mode 100644
index 000000000000..97145d3a53ce
--- /dev/null
+++ b/autocorrect.c
@@ -0,0 +1,72 @@
+#include "git-compat-util.h"
+#include "autocorrect.h"
+#include "config.h"
+#include "parse.h"
+#include "strbuf.h"
+#include "prompt.h"
+#include "gettext.h"
+
+static int parse_autocorrect(const char *value)
+{
+ switch (git_parse_maybe_bool_text(value)) {
+ case 1:
+ return AUTOCORRECT_IMMEDIATELY;
+ case 0:
+ return AUTOCORRECT_SHOW;
+ default: /* other random text */
+ break;
+ }
+
+ if (!strcmp(value, "prompt"))
+ return AUTOCORRECT_PROMPT;
+ if (!strcmp(value, "never"))
+ return AUTOCORRECT_NEVER;
+ if (!strcmp(value, "immediate"))
+ return AUTOCORRECT_IMMEDIATELY;
+ if (!strcmp(value, "show"))
+ return AUTOCORRECT_SHOW;
+
+ return 0;
+}
+
+void autocorrect_resolve_config(const char *var, const char *value,
+ const struct config_context *ctx, void *data)
+{
+ int *out = data;
+
+ if (!strcmp(var, "help.autocorrect")) {
+ int v = parse_autocorrect(value);
+
+ if (!v) {
+ v = git_config_int(var, value, ctx->kvi);
+ if (v < 0 || v == 1)
+ v = AUTOCORRECT_IMMEDIATELY;
+ }
+
+ *out = v;
+ }
+}
+
+void autocorrect_confirm(int autocorrect, const char *assumed)
+{
+ if (autocorrect == AUTOCORRECT_IMMEDIATELY) {
+ fprintf_ln(stderr,
+ _("Continuing under the assumption that you meant '%s'."),
+ assumed);
+ } else if (autocorrect == AUTOCORRECT_PROMPT) {
+ char *answer;
+ struct strbuf msg = STRBUF_INIT;
+
+ strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
+ answer = git_prompt(msg.buf, PROMPT_ECHO);
+ strbuf_release(&msg);
+
+ if (!(starts_with(answer, "y") || starts_with(answer, "Y")))
+ exit(1);
+ } else {
+ fprintf_ln(stderr,
+ _("Continuing in %0.1f seconds, assuming that you meant '%s'."),
+ (float)autocorrect / 10.0, assumed);
+ sleep_millisec(autocorrect * 100);
+ }
+}
diff --git a/autocorrect.h b/autocorrect.h
new file mode 100644
index 000000000000..f5fadf9d9605
--- /dev/null
+++ b/autocorrect.h
@@ -0,0 +1,16 @@
+#ifndef AUTOCORRECT_H
+#define AUTOCORRECT_H
+
+#define AUTOCORRECT_SHOW (-4)
+#define AUTOCORRECT_PROMPT (-3)
+#define AUTOCORRECT_NEVER (-2)
+#define AUTOCORRECT_IMMEDIATELY (-1)
+
+struct config_context;
+
+void autocorrect_resolve_config(const char *var, const char *value,
+ const struct config_context *ctx, void *data);
+
+void autocorrect_confirm(int autocorrect, const char *assumed);
+
+#endif /* AUTOCORRECT_H */
diff --git a/help.c b/help.c
index 3e59d07c370b..ab619ed43c7a 100644
--- a/help.c
+++ b/help.c
@@ -22,6 +22,7 @@
#include "repository.h"
#include "alias.h"
#include "utf8.h"
+#include "autocorrect.h"
#ifndef NO_CURL
#include "git-curl-compat.h" /* For LIBCURL_VERSION only */
@@ -541,34 +542,6 @@ struct help_unknown_cmd_config {
struct cmdnames aliases;
};
-#define AUTOCORRECT_SHOW (-4)
-#define AUTOCORRECT_PROMPT (-3)
-#define AUTOCORRECT_NEVER (-2)
-#define AUTOCORRECT_IMMEDIATELY (-1)
-
-static int parse_autocorrect(const char *value)
-{
- switch (git_parse_maybe_bool_text(value)) {
- case 1:
- return AUTOCORRECT_IMMEDIATELY;
- case 0:
- return AUTOCORRECT_SHOW;
- default: /* other random text */
- break;
- }
-
- if (!strcmp(value, "prompt"))
- return AUTOCORRECT_PROMPT;
- if (!strcmp(value, "never"))
- return AUTOCORRECT_NEVER;
- if (!strcmp(value, "immediate"))
- return AUTOCORRECT_IMMEDIATELY;
- if (!strcmp(value, "show"))
- return AUTOCORRECT_SHOW;
-
- return 0;
-}
-
static int git_unknown_cmd_config(const char *var, const char *value,
const struct config_context *ctx,
void *cb)
@@ -577,17 +550,7 @@ static int git_unknown_cmd_config(const char *var, const char *value,
const char *subsection, *key;
size_t subsection_len;
- if (!strcmp(var, "help.autocorrect")) {
- int v = parse_autocorrect(value);
-
- if (!v) {
- v = git_config_int(var, value, ctx->kvi);
- if (v < 0 || v == 1)
- v = AUTOCORRECT_IMMEDIATELY;
- }
-
- cfg->autocorrect = v;
- }
+ autocorrect_resolve_config(var, value, ctx, &cfg->autocorrect);
/* Also use aliases for command lookup */
if (!parse_config_key(var, "alias", &subsection, &subsection_len,
@@ -724,27 +687,8 @@ char *help_unknown_cmd(const char *cmd)
_("WARNING: You called a Git command named '%s', "
"which does not exist."),
cmd);
- if (cfg.autocorrect == AUTOCORRECT_IMMEDIATELY)
- fprintf_ln(stderr,
- _("Continuing under the assumption that "
- "you meant '%s'."),
- assumed);
- else if (cfg.autocorrect == AUTOCORRECT_PROMPT) {
- char *answer;
- struct strbuf msg = STRBUF_INIT;
- strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
- answer = git_prompt(msg.buf, PROMPT_ECHO);
- strbuf_release(&msg);
- if (!(starts_with(answer, "y") ||
- starts_with(answer, "Y")))
- exit(1);
- } else {
- fprintf_ln(stderr,
- _("Continuing in %0.1f seconds, "
- "assuming that you meant '%s'."),
- (float)cfg.autocorrect/10.0, assumed);
- sleep_millisec(cfg.autocorrect * 100);
- }
+
+ autocorrect_confirm(cfg.autocorrect, assumed);
cmdnames_release(&cfg.aliases);
cmdnames_release(&main_cmds);
diff --git a/meson.build b/meson.build
index 11488623bfd8..be20d507826d 100644
--- a/meson.build
+++ b/meson.build
@@ -290,6 +290,7 @@ libgit_sources = [
'archive-zip.c',
'archive.c',
'attr.c',
+ 'autocorrect.c',
'base85.c',
'bisect.c',
'blame.c',
--
2.54.0
^ permalink raw reply related
* [PATCH v6 01/10] parseopt: extract subcommand handling from parse_options_step()
From: Jiamu Sun @ 2026-04-23 1:37 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Aaron Plattner, Karthik Nayak, Jiamu Sun
In-Reply-To: <SY0P300MB0801677A2A1E0FD38D06A841CE2A2@SY0P300MB0801.AUSP300.PROD.OUTLOOK.COM>
Move the subcommand branch out of parse_options_step() into a new
handle_subcommand() helper. Also, make parse_subcommand() return a
simple success/failure status.
This removes the switch over impossible parse_opt_result values and
makes the non-option path easier to follow and maintain.
Signed-off-by: Jiamu Sun <39@barroit.sh>
---
parse-options.c | 87 ++++++++++++++++++++++++++-----------------------
1 file changed, 46 insertions(+), 41 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index a676da86f5d6..803ce2ba4443 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -606,17 +606,44 @@ static enum parse_opt_result parse_nodash_opt(struct parse_opt_ctx_t *p,
return PARSE_OPT_ERROR;
}
-static enum parse_opt_result parse_subcommand(const char *arg,
- const struct option *options)
+static int parse_subcommand(const char *arg, const struct option *options)
{
- for (; options->type != OPTION_END; options++)
- if (options->type == OPTION_SUBCOMMAND &&
- !strcmp(options->long_name, arg)) {
- *(parse_opt_subcommand_fn **)options->value = options->subcommand_fn;
- return PARSE_OPT_SUBCOMMAND;
- }
+ for (; options->type != OPTION_END; options++) {
+ parse_opt_subcommand_fn **opt_val;
- return PARSE_OPT_UNKNOWN;
+ if (options->type != OPTION_SUBCOMMAND ||
+ strcmp(options->long_name, arg))
+ continue;
+
+ opt_val = options->value;
+ *opt_val = options->subcommand_fn;
+ return 0;
+ }
+
+ return -1;
+}
+
+static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx,
+ const char *arg,
+ const struct option *options,
+ const char * const usagestr[])
+{
+ int err = parse_subcommand(arg, options);
+
+ if (!err)
+ return PARSE_OPT_SUBCOMMAND;
+
+ /*
+ * arg is neither a short or long option nor a subcommand. Since this
+ * command has a default operation mode, we have to treat this arg and
+ * all remaining args as args meant to that default operation mode.
+ * So we are done parsing.
+ */
+ if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
+ return PARSE_OPT_DONE;
+
+ error(_("unknown subcommand: `%s'"), arg);
+ usage_with_options(usagestr, options);
}
static void check_typos(const char *arg, const struct option *options)
@@ -1011,38 +1038,16 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
if (*arg != '-' || !arg[1]) {
if (parse_nodash_opt(ctx, arg, options) == 0)
continue;
- if (!ctx->has_subcommands) {
- if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
- return PARSE_OPT_NON_OPTION;
- ctx->out[ctx->cpidx++] = ctx->argv[0];
- continue;
- }
- switch (parse_subcommand(arg, options)) {
- case PARSE_OPT_SUBCOMMAND:
- return PARSE_OPT_SUBCOMMAND;
- case PARSE_OPT_UNKNOWN:
- if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL)
- /*
- * arg is neither a short or long
- * option nor a subcommand. Since
- * this command has a default
- * operation mode, we have to treat
- * this arg and all remaining args
- * as args meant to that default
- * operation mode.
- * So we are done parsing.
- */
- return PARSE_OPT_DONE;
- error(_("unknown subcommand: `%s'"), arg);
- usage_with_options(usagestr, options);
- case PARSE_OPT_COMPLETE:
- case PARSE_OPT_HELP:
- case PARSE_OPT_ERROR:
- case PARSE_OPT_DONE:
- case PARSE_OPT_NON_OPTION:
- /* Impossible. */
- BUG("parse_subcommand() cannot return these");
- }
+
+ if (ctx->has_subcommands)
+ return handle_subcommand(ctx, arg, options,
+ usagestr);
+
+ if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
+ return PARSE_OPT_NON_OPTION;
+
+ ctx->out[ctx->cpidx++] = ctx->argv[0];
+ continue;
}
/* lone -h asks for help */
--
2.54.0
^ permalink raw reply related
* [PATCH v6 00/10] parseopt: add subcommand autocorrection
From: Jiamu Sun @ 2026-04-23 1:37 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Aaron Plattner, Karthik Nayak, Jiamu Sun
In-Reply-To: <SY0P300MB0801AE56F740AD087D22B35ACE2D2@SY0P300MB0801.AUSP300.PROD.OUTLOOK.COM>
Git currently provides auto-correction for builtins and aliases, but
lacks this functionality for subcommands parsed via the parse-options
API. Subcommands are also commands, and typos will occur, too. Like:
git remote add-rul
So, this series introduces subcommand auto-correction.
By default, this implementation enables autocorrection for builtins
with mandatory subcommands. However, for those using
PARSE_OPT_SUBCOMMAND_OPTIONAL, autocorrection is skipped to avoid
misinterpreting legitimate unknown arguments as mistyped subcommands.
To allow builtins with optional subcommands to explicitly opt in,
this series adds the PARSE_OPT_SUBCOMMAND_AUTOCORRECT flag, and enables
it for git-remote and git-notes.
Additionally, the existing autocorrection logic is extracted from
help.c so subcommand handling can reuse the same config parsing and
prompt/delay logic.
Some string literals are also combined so the full text is easier to
grep for.
Changes in v6:
- Adjust existing tests to fit subcommand autocorrection behavior
- Change the similar subcommand hint exit code to 129
Changes in v5:
- Make subcommand autocorrection behave the same as command
autocorrection
- Rename PARSE_OPT_SUBCOMMAND_AUTOCORR to
PARSE_OPT_SUBCOMMAND_AUTOCORRECT
- Adjust subcommand autocorrection tests to fit new behavior
Changes in v4:
- Add missing files to Meson build
- Change API prefix from autocorr to autocorrect
- Split the commit that moves tty code
- Add API documentation
- Use standard Damerau-Levenshtein distance and common practice
fuzziness thresholds
- Rename AUTOCORRECT_HINTONLY to AUTOCORRECT_HINT
- Change commit subject prefix for tests from "help:" to "parseopt:"
- Fix coding style issues
Changes in v3:
- Align with the coding guildline
- Split patch so diffs don't get hidden by code movement
- Improve commit messages
Changes in v2:
- Reword the explanation of default autocorrection behavior
Jiamu Sun (10):
parseopt: extract subcommand handling from parse_options_step()
help: make autocorrect handling reusable
help: move tty check for autocorrection to autocorrect.c
autocorrect: use mode and delay instead of magic numbers
autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINT
autocorrect: provide config resolution API
parseopt: autocorrect mistyped subcommands
parseopt: enable subcommand autocorrection for git-remote and
git-notes
parseopt: add tests for subcommand autocorrection
doc: document autocorrect API
Makefile | 1 +
autocorrect.c | 89 +++++++++++++++
autocorrect.h | 36 ++++++
builtin/notes.c | 10 +-
builtin/remote.c | 12 +-
help.c | 120 ++++----------------
meson.build | 1 +
parse-options.c | 183 +++++++++++++++++++++++-------
parse-options.h | 1 +
t/meson.build | 1 +
t/t0040-parse-options.sh | 5 +-
t/t7900-maintenance.sh | 4 +-
t/t9004-autocorrect-subcommand.sh | 58 ++++++++++
13 files changed, 366 insertions(+), 155 deletions(-)
create mode 100644 autocorrect.c
create mode 100644 autocorrect.h
create mode 100755 t/t9004-autocorrect-subcommand.sh
base-commit: f65aba1e87db64413b6d1ed5ae5a45b5a84a0997
--
2.54.0
^ permalink raw reply
* Re: [PATCH v5 00/10] parseopt: add subcommand autocorrection
From: Jiamu Sun @ 2026-04-23 1:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Aaron Plattner, Karthik Nayak
In-Reply-To: <xmqqwlxyo7w6.fsf@gitster.g>
On Wed, Apr 22, 2026 at 05:08:41PM -0700, Junio C Hamano wrote:
> It seems that the exit status of some error path (subcommand parse
> failure, presumably) has changed unexpectedly? As I am mostly
> offline for this and next week, I didn't dig any further.
My fault, forgot to adjust these tests. Since subcommand parser learned
to autocorrect misspelled names,
> t0040-parse-options.sh (Wstat: 256 (exited 1) Tests: 94 Failed: 1)
test-tool parse-subcommand cmd subcmd-o
in "subcommand - subcommands cannot be abbreviated" test now shows
similar subcommands.
> t7900-maintenance.sh (Wstat: 256 (exited 1) Tests: 72 Failed: 1)
git maintenance barf
in "help text" test now corrects "barf" to "start".
After fixing these two, all tests pass.
----
All tests successful.
Files=1043, Tests=33202, 465 wallclock secs
( 7.02 usr 2.23 sys + 1244.78 cusr 1642.22 csys = 2896.25 CPU)
Result: PASS
Also, I found most error paths exit with 129 in parse-options.c, but
the similar subcommand hint exits with 1. That's inconsistent, will
change the exit code to 129.
--
Jiamu Sun <39@barroit.sh>
<sunjiamu@outlook.com>
^ permalink raw reply
* Re: [PATCH v5 00/10] parseopt: add subcommand autocorrection
From: Junio C Hamano @ 2026-04-23 0:08 UTC (permalink / raw)
To: Jiamu Sun; +Cc: git, aplattner, karthik.188
In-Reply-To: <SY0P300MB0801AE56F740AD087D22B35ACE2D2@SY0P300MB0801.AUSP300.PROD.OUTLOOK.COM>
Jiamu Sun <39@barroit.sh> writes:
> Git currently provides auto-correction for builtins and aliases, but
> lacks this functionality for subcommands parsed via the parse-options
> API. Subcommands are also commands, and typos will occur, too. Like:
>
> git remote add-rul
>
> So, this series introduces subcommand auto-correction.
> ...
> base-commit: f65aba1e87db64413b6d1ed5ae5a45b5a84a0997
When the series is applied on top of Git 2.54, or on top of f65aba1e
(codeql: bump actions/cache from 4 to 5, 2026-04-13), the result
does not pass its self test, at least for me.
It seems that the exit status of some error path (subcommand parse
failure, presumably) has changed unexpectedly? As I am mostly
offline for this and next week, I didn't dig any further.
Thanks.
Test Summary Report
-------------------
t0040-parse-options.sh (Wstat: 256 (exited 1) Tests: 94 Failed: 1)
Failed test: 73
Non-zero exit status: 1
t7900-maintenance.sh (Wstat: 256 (exited 1) Tests: 72 Failed: 1)
Failed test: 1
Non-zero exit status: 1
^ permalink raw reply
* [PATCH] Revert "transport-helper, connect: use clean_on_exit to reap children on abnormal exit"
From: Jeff King @ 2026-04-22 23:00 UTC (permalink / raw)
To: Jan Palus; +Cc: git, Andrew Au, Junio C Hamano
In-Reply-To: <20260422223542.GA110382@coredump.intra.peff.net>
On Wed, Apr 22, 2026 at 06:35:43PM -0400, Jeff King wrote:
> So I think just reverting dd3693eb08 is probably the most immediate fix.
Here it is in patch form.
-- >8 --
Subject: Revert "transport-helper, connect: use clean_on_exit to reap children on abnormal exit"
This reverts commit dd3693eb0859274d62feac8047e1d486b3beaf31.
The goal of that commit was to avoid zombie child processes hanging
around when the parent git process is killed. But it doesn't quite work
when the child command is run by the shell:
1. If there is a shell, then we kill and wait for the shell, not the
process spawned by the shell. And so the child process, even if it
eventually exits, will hang around as a zombie forever. And this is
true of most (all?) shells: bash, dash, etc.
So we are not really accomplishing our goal in the first place.
2. Not all shells will exit immediately upon receiving a signal. In
particular, mksh will wait for its children to exit (but not
actually propagate the signal to them!) leaving us with a potential
deadlock: git is wait()ing on mksh, which is wait()ing on a child
process, but that child process is waiting on git to produce more
input (or EOF) over a pipe.
You can see several examples of this deadlock in the test suite,
for example by running:
make SHELL_PATH=/bin/mksh
cd t
./t5702-protocol-v2.sh
Because this is a regression for mksh users, and because we did not
achieve our goal even with other shells, let's revert the commit for
now. If there is a more clever way of doing the same thing, we can
consider applying it separately on top (or do nothing and just accept
the zombies and rely on PID 1 to reap them).
Reported-by: Jan Palus <jpalus@fastmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
connect.c | 4 ----
transport-helper.c | 2 --
2 files changed, 6 deletions(-)
diff --git a/connect.c b/connect.c
index fcd35c5539..a02583a102 100644
--- a/connect.c
+++ b/connect.c
@@ -1054,8 +1054,6 @@ static struct child_process *git_proxy_connect(int fd[2], char *host)
strvec_push(&proxy->args, port);
proxy->in = -1;
proxy->out = -1;
- proxy->clean_on_exit = 1;
- proxy->wait_after_clean = 1;
if (start_command(proxy))
die(_("cannot start proxy %s"), git_proxy_command);
fd[0] = proxy->out; /* read from proxy stdout */
@@ -1517,8 +1515,6 @@ struct child_process *git_connect(int fd[2], const char *url,
}
strvec_push(&conn->args, cmd.buf);
- conn->clean_on_exit = 1;
- conn->wait_after_clean = 1;
if (start_command(conn))
die(_("unable to fork"));
diff --git a/transport-helper.c b/transport-helper.c
index 4e5d1d914f..4614036c99 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -154,8 +154,6 @@ static struct child_process *get_helper(struct transport *transport)
helper->trace2_child_class = helper->args.v[0]; /* "remote-<name>" */
- helper->clean_on_exit = 1;
- helper->wait_after_clean = 1;
code = start_command(helper);
if (code < 0 && errno == ENOENT)
die(_("unable to find remote helper for '%s'"), data->name);
--
2.54.0.232.gea82d9008b
^ permalink raw reply related
* Re: [PATCH v2 1/2] revision.c: implement --reverse=before for walks
From: Mirko Faina @ 2026-04-22 22:53 UTC (permalink / raw)
To: Jeff King
Cc: git, Junio C Hamano, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Mirko Faina
In-Reply-To: <20260422224442.GB110382@coredump.intra.peff.net>
On Wed, Apr 22, 2026 at 06:44:42PM -0400, Jeff King wrote:
> On Wed, Apr 22, 2026 at 02:28:40AM +0200, Mirko Faina wrote:
>
> > diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
> > index 2d195a1474..7244e85108 100644
> > --- a/Documentation/rev-list-options.adoc
> > +++ b/Documentation/rev-list-options.adoc
> > @@ -914,10 +914,16 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5
> > avoid showing the commits from two parallel development track mixed
> > together.
> >
> > -`--reverse`::
> > - Output the commits chosen to be shown (see 'Commit Limiting'
> > - section above) in reverse order. Cannot be combined with
> > - `--walk-reflogs`.
> > +`--[no-]reverse[=(after|before)]`::
> > + Accepts `after` or `before`. Cannot be combined with
> > + `--walk-reflogs`. If `after`, output the commits chosen to be
> > + shown (see 'Commit Limiting' section above) in reverse order. If
> > + `before`, reverse the commits before filtering with `Commit
> > + Limiting` options. This option can be used multiple times, last
> > + one is applied. When the argument for `--reverse` is omitted, if
> > + the current state is in no reverse, it defaults to `after`. If
> > + it is in any reversed state, it restores the original ordering
> > + by removing the reverse state.
>
> I think this is all correct, but I found the final sentences a bit hard
> to follow (especially the phrase "reversed state"). Let me take a stab
> at it.
>
> When multiple `--reverse=` options are given, the final option
> overrides any previous options. The `--reverse` option (with no
> specifier) behaves as `--reverse=after`, except that for historical
> reasons it negates any previous reversed state (so `--reverse
> --reverse` does nothing, nor does `--reverse=before --reverse`).
>
> I dunno if that is any better, really. I hoped by mentioning "historical
> reasons" that excuses any weirdness, and readers interested in sane
> behavior can stop reading. ;)
>
> So anyway, I offer it in case you find it useful or can pick out useful
> bits from it.
Yes, it does read better, thank you.
I'd personally put a "truth table" but that would be very verbose, and I
suspect most would not find it useful :D
Thank you
^ permalink raw reply
* Re: [PATCH v2 1/2] revision.c: implement --reverse=before for walks
From: Jeff King @ 2026-04-22 22:44 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Junio C Hamano, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble
In-Reply-To: <20260422002840.303477-5-mroik@delayed.space>
On Wed, Apr 22, 2026 at 02:28:40AM +0200, Mirko Faina wrote:
> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
> index 2d195a1474..7244e85108 100644
> --- a/Documentation/rev-list-options.adoc
> +++ b/Documentation/rev-list-options.adoc
> @@ -914,10 +914,16 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5
> avoid showing the commits from two parallel development track mixed
> together.
>
> -`--reverse`::
> - Output the commits chosen to be shown (see 'Commit Limiting'
> - section above) in reverse order. Cannot be combined with
> - `--walk-reflogs`.
> +`--[no-]reverse[=(after|before)]`::
> + Accepts `after` or `before`. Cannot be combined with
> + `--walk-reflogs`. If `after`, output the commits chosen to be
> + shown (see 'Commit Limiting' section above) in reverse order. If
> + `before`, reverse the commits before filtering with `Commit
> + Limiting` options. This option can be used multiple times, last
> + one is applied. When the argument for `--reverse` is omitted, if
> + the current state is in no reverse, it defaults to `after`. If
> + it is in any reversed state, it restores the original ordering
> + by removing the reverse state.
I think this is all correct, but I found the final sentences a bit hard
to follow (especially the phrase "reversed state"). Let me take a stab
at it.
When multiple `--reverse=` options are given, the final option
overrides any previous options. The `--reverse` option (with no
specifier) behaves as `--reverse=after`, except that for historical
reasons it negates any previous reversed state (so `--reverse
--reverse` does nothing, nor does `--reverse=before --reverse`).
I dunno if that is any better, really. I hoped by mentioning "historical
reasons" that excuses any weirdness, and readers interested in sane
behavior can stop reading. ;)
So anyway, I offer it in case you find it useful or can pick out useful
bits from it.
-Peff
^ permalink raw reply
* Re: regression 2.54.0: test suite hangs indefinitely with mksh
From: Jeff King @ 2026-04-22 22:35 UTC (permalink / raw)
To: Jan Palus; +Cc: git, Andrew Au, Junio C Hamano
In-Reply-To: <aekVSi4iu9YLaPLQ@rock.grzadka>
On Wed, Apr 22, 2026 at 09:00:30PM +0200, Jan Palus wrote:
> If /bin/sh points to mksh, git's test suite hangs forever and never
> completes. An example of process tree for such hung test:
> [...]
Thanks, I can reproduce the problem easily here with mksh.
> bisect points to the following commit:
>
> commit dd3693eb0859274d62feac8047e1d486b3beaf31 (HEAD)
> Author: Andrew Au <cshung@gmail.com>
> Date: Thu Mar 12 22:49:37 2026
>
> transport-helper, connect: use clean_on_exit to reap children on abnormal exit
Yup, same here.
> From commit message alone it seems the change tries to avoid reparenting
> child processes to PID 1, however this does not seem to work for
> processes which were spawned with "sh -c".
>
> For example if /bin/sh points to bash tests pass because bash appears to
> react to SIGTERM differently -- shell process ends but its children are
> reparented anyway.
>
> mksh on the other hand seems to ignore SIGTERM (or queues it?) but
> waits for child process and so test suite never ends.
Yeah, I agree with your analysis. It looks mksh's signal handler waits
for its children. If I run:
# note start of script
date
# mksh with a slow child
mksh -c 'echo before; sleep 3; echo after' &
# wait for it to have started child, then kill
sleep 1
kill $!
# and now wait and mark the time
wait $!
date
then we see "before" but not "after", and the script takes 3 seconds to
run (since we wait for mksh to exit). Replacing "mksh" with bash or
dash, it finishes in 1 second.
So what to do?
I think we could alleviate the symptom by teaching clean_on_exit to
follow-up SIGTERM with SIGKILL. But as you note, that patch is not
really solving the original problem in the first place. If we use dash
or bash in the above script and add "ps u | grep sleep" to the end, we
can see that "sleep 3" is still running even after we've returned. So we
have not really accomplished our goal anyway.
So I think just reverting dd3693eb08 is probably the most immediate fix.
We can try again at the same goal later, though I'm not quite sure how
to do so. From Git's perspective, we do not even know the pid of the
child that the shell spawned, so we cannot really kill it or wait on it.
Conceptually we'd want the shell to propagate the signal to its
children, but doing that automatically in the general case is rather
tricky. I don't think we want to get into prepending trap commands at
the start of each shell we invoke.
For the specific cases in dd3693eb08, we probably want to signal to the
child to exit by closing the pipe its reading from. But we can't do that
via the run-command system, so it would have to be a specific signal
handler within the transport system. I think that would avoid true
deadlocks, but it still isn't quite what we want. The process won't die
until it tries to read from the parent, so if it's waiting on a long
network timeout, for example, it could mean that the parent sits around
for quite a while even after having gotten a signal.
The fallback position to what dd3693eb08 was trying to do is roughly:
let them get reparented to pid 1 and don't worry about (and people in
containers should really use "tini" or some other pid-1 replacement).
That doesn't seem like the worst place to be, and we can get there
easily with a revert.
This all does point to some general weaknesses in run-command's
clean_on_exit. If a shell is involved, we're probably not killing who we
meant to. That's usually not the end of the world (if we take "clean" to
be best-effort), but with wait_after_clean tacked on it's a recipe for a
potential hang if the shell doesn't exit. So I tried this:
diff --git a/run-command.c b/run-command.c
index c146a56532..97c223bdf1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -680,6 +680,9 @@ int start_command(struct child_process *cmd)
int failed_errno;
const char *str;
+ if (cmd->use_shell && cmd->wait_after_clean)
+ BUG("combining use_shell and wait_after_clean is not reliable");
+
/*
* In case of errors we must keep the promise to close FDs
* that have been passed in via ->in and ->out.
and ran the test suite, which results in several failures. In particular
we'll run a "!" alias as a shell command with those flags, but also for
things like exec-ing external commands (like "git-foo"). So we should be
able to produce a "hang" like this (built with SHELL_PATH=/bin/mksh):
./git -c alias.foo='!echo before; sleep 10; echo after' foo &
sleep 1
kill $!
wait
and we can see that the parent "git" process waits 10 seconds, even
though it was killed. There are two saving graces here:
1. This isn't really a deadlock, but just a slow process. We are
waiting on the alias to exit anyway, and we do not expect that the
alias command is waiting on us. This is very different from the
cases in dd3693eb08, where we are spawning something with a pipe
which could be waiting to read from us. And it looks like all of
the existing uses of wait_after_clean are like this; the parent is
conceptually doing an "exec" of the child, but for various reasons
we want it to hang around.
The exception is in http-backend, where I think we really are
interacting with the child (so if we got a signal and the child
didn't, I think we could possibly deadlock as it waits for us to
feed more data).
2. mksh seems eager to automatically exec its child when fed only a
single, simple command (like "mksh -c 'sleep 10'). And that
alleviates the problem, since the signal goes straight to the child
command, then. So for things like git-foo externals, the shell may
have gotten out of the way anyway.
So I think the existing, pre-dd3693eb08 cases are _probably_ fine, even
with mksh, though the signal propagation may not be doing quite what we
want it to do. In practice I think most signals end up delivered to the
whole process group anyway (e.g., hitting ^C in a terminal) and that
part doesn't matter so much.
-Peff
^ permalink raw reply related
* [PATCH] Fix docs for format.commitListFormat
From: Mirko Faina @ 2026-04-22 21:45 UTC (permalink / raw)
To: git; +Cc: Mirko Faina
When renaming the option --cover-letter-format to --commit-list-format
we forgot to rename the opton in the section too. Fix it.
Signed-off-by: Mirko Faina <mroik@delayed.space>
---
Unfortunately we missed this one before 2.54.
Documentation/config/format.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/config/format.adoc b/Documentation/config/format.adoc
index dbd186290b..95d19134b2 100644
--- a/Documentation/config/format.adoc
+++ b/Documentation/config/format.adoc
@@ -102,7 +102,7 @@ format.coverLetter::
Default is false.
format.commitListFormat::
- When the `--cover-letter-format` option is not given, `format-patch`
+ When the `--commit-list-format` option is not given, `format-patch`
uses the value of this variable to decide how to format the entry of
each commit. Defaults to `shortlog`.
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] revision.c: implement --reverse=before for walks
From: Mirko Faina @ 2026-04-22 19:42 UTC (permalink / raw)
To: D. Ben Knoble
Cc: git, Junio C Hamano, Patrick Steinhardt, Jean-Noël Avila,
Jeff King, Mirko Faina
In-Reply-To: <CALnO6CAjMAZhBk_WXW1wbKk1kpQScFtbY0R+mCxHTFB7=CcEDg@mail.gmail.com>
On Wed, Apr 22, 2026 at 02:24:48PM -0400, D. Ben Knoble wrote:
> > > > > > c = pop_commit(&revs->commits);
> > > > > > + if (revs->reverse == 2)
> > > > > > + revs->max_count--;
> > > > >
> > > > > Hm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising.
> > > >
> > > > get_revision() (in revision.c) handles the reverse option and updates
> > > > the "struct git_graph". get_revision() then calls
> > > > get_revision_internal(), which handles commit boundaries and max_count,
> > > > here is where it gets decreased. Since max_count gets decreased
> > > > everytime get_revision_internal() is called, if we were to leave
> > > > max_count as is before the walk (in get_revision() at line 4558), the
> > > > walk would stop before reaching the root commit. This is why the current
> > > > --reverse option is applied only after commit limiting options. So
> > > > instead we set max_count at -1 walking the whole history and storing it
> > > > in 'reversed'. Now we're in "reverse_output_stage = 1", and in this
> > > > state we never call get_revision_internal() again, instead we pop
> > > > commits from 'reversed'. Because of this we have to handle max_count
> > > > outside get_revision_internal(), so we decrement it in the snippet of
> > > > code you referenced.
> > > >
> > > > A bit verbose but hopefully it'll get my point across.
> > >
> > > I don't 100% follow, but I'm out of my depth :)
> > >
> > > I think I see that get_revision() effectively has 2 modes pertaining
> > > to reverse: reverse and reverse output stage (the former falls
> > > directly into the latter, though).
> > >
> > > After some setup, the reverse mode calls get_revision_internal() as
> > > you said. That decrements max_count as a way of counting how many
> > > commits we've seen through the loop, so if we asked for 5 we'd only
> > > process 5 commits.
> > >
> > > Then we fall into the output stage mode, which pops a commit [1].
> > >
> > > With this patch, in reverse=after we disable max_count in the first
> > > (reverse) mode, as you said. Ok: we get the whole (filtered) history
> > > then, at which point we can now shrink. That makes sense.
> > >
> > > Then in the reverse output stage mode, we pretend to have one less
> > > max_count. That's what I can't figure out. Is it because of the
> > > pop_commit()? I guess I'm not totally seeing how that interacted with
> > > the max_count in the original code: does the current code yield one
> > > extra commit in get_revision_internal() ?
> >
> > I'm not sure I understand what you're referencing with "Then in the
> > reverse output stage mode, we pretend to have one less max_count".
> >
> > If you're referring to line 4573, then...
> >
> > > You wrote that "we never call get_revision_internal() again," but I
> > > don't see why that's true with this patch and not true before it.
> > >
> > > I do agree that _somebody_ has to handle max_count after
> > > get_revision() returns with reverse=after. I'm just not sure what
> > >
> > > if (revs->reverse == 2)
> > > revs->max_count--;
> > >
> > > is doing.
> >
> > ...we're not pretending we have fewer commits. Every subsequent call to
> > get_revision() after the first call will never enter the branch at line
> > 4548 and will only enter the branch at 4568. Everytime we pop a commit
> > from 'reversed' we decrease max_count so we can limit only to the amount
> > of commits the user wants.
> >
> > So, to recap, with "reverse = 2", on the first call to get_revision() we
> > walk the whole history and store it in 'reversed' in reversed order and
> > return the first commit.
> > On subsequent calls to get_revision() we do not walk the history again,
> > we simply return the commits that have been stored in 'reversed'.
> > Everytime we pop a commit we have to decrease max_count, and we check
> > againts max_count to know if we shouldn't return anymore commits (by
> > returning NULL).
>
> …but I think this one does. I think what I missed is that in all
> "reverse" modes, get_revision() does some pre-computation and then
> yields one at a time the commits. In traditional "after" mode, the
> counting is done by get_revision_internal() [before reversal]. In the
> new mode, get_revision takes on that responsibility of
> get_revision_internal instead.
>
> Hm. That suggests to me that get_revision's responsibilities are
> becoming complex. Might be worth some version of a refactor, but idk
> which.
If the refactor is to yield the responsibility of max_count to
get_revision_internal() for the new reverse mode too, then I'm not sure
if we want that. To maintain state across calls to
get_revision_internal() we'd have to store the temporary max_count that
currently lives in get_revision() in rev_info. I don't want to add
unnecessary new fields to the struct when its space is so valuable
(afterall we're using bitpacking).
In the current implementation all of the reversal happens in the same
call to get_revision(), so state is preserved across multiple
get_revision_internal() calls through the temporary int max_count.
^ permalink raw reply
* Re: [PATCH 2/2] builtin/history: introduce "fixup" subcommand
From: D. Ben Knoble @ 2026-04-22 19:06 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Elijah Newren
In-Reply-To: <20260422-b4-pks-history-fixup-v1-2-48d4484243de@pks.im>
Yahoo, fixup!
On Wed, Apr 22, 2026 at 6:30 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> The newly introduced git-history(1) command provides functionality to
> easily edit commit history while also rebasing dependent branches. The
> functionality exposed by this command is still somewhat limited though.
>
> One common use case when editing commit history that is not yet covered
> is fixing up a specific commit. Introduce a new subcommand that allows
> the user to do exactly that by performing a three-way merge into the
> target's commit tree, using HEAD's tree as the merge base. The flow is
> thus essentially:
>
> $ echo changes >file
> $ git add file
> $ git history fixup HEAD~
>
> Like with the other commands, this will automatically rebase dependent
> branches, as well. Unlike the other commands though:
>
> - The command does not work in a bare repository as it interacts with
> the index.
>
> - The command may run into merge conflicts. If so, the command will
> simply abort.
>
> Especially the second item limits the usefulness of this command a bit.
> But there are plans to introduce first-class conflicts into Git, which
> will help use cases like this one.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Documentation/git-history.adoc | 52 ++++-
> builtin/history.c | 153 +++++++++++++
> t/meson.build | 1 +
> t/t3453-history-fixup.sh | 500 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 704 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
> index 24dc907033..3cdfc8ba02 100644
> --- a/Documentation/git-history.adoc
> +++ b/Documentation/git-history.adoc
> @@ -8,6 +8,7 @@ git-history - EXPERIMENTAL: Rewrite history
> SYNOPSIS
> --------
> [synopsis]
> +git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
> git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
> git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
>
> @@ -22,8 +23,9 @@ THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE.
> This command is related to linkgit:git-rebase[1] in that both commands can be
> used to rewrite history. There are a couple of major differences though:
>
> -* linkgit:git-history[1] can work in a bare repository as it does not need to
> - touch either the index or the worktree.
> +* Most subcommands of linkgit:git-history[1] can work in a bare repository as
> + they do not need to touch either the index or the worktree. The `fixup`
> + subcommand is an exception to this, as it reads staged changes from the index.
> * linkgit:git-history[1] does not execute any linkgit:githooks[5] at the
> current point in time. This may change in the future.
> * linkgit:git-history[1] by default updates all branches that are descendants
> @@ -53,6 +55,19 @@ COMMANDS
>
> The following commands are available to rewrite history in different ways:
>
> +`fixup <commit>`::
> + Apply the currently staged changes to the specified commit. The staged
> + changes are incorporated into the target commit's tree via a three-way
> + merge, using HEAD's tree as the merge base, which is equivalent to
> + linkgit:git-cherry-pick[1].
I'm not quite sure what, as a user of "git history fixup," I'm
supposed to take from this. Does it make conflicts less likely when
creating the new fixup? I imagine it doesn't help with conflicts
between <commit> and HEAD that newly arise.
Anyway, I'd think the mechanics are less relevant than the end-user
behavior at this point in the doc, unless the equivalence with
cherry-pick is supposed to tell me something about that behavior.
> ++
> +The commit message and authorship of the target commit are preserved by
> +default, unless you specify `--reedit-message`.
> ++
> +If applying the staged changes would result in a conflict, the command
> +aborts with an error. All branches that are descendants of the original
> +commit are updated to point to the rewritten history.
> +
> `reword <commit>`::
> Rewrite the commit message of the specified commit. All the other
> details of this commit remain unchanged. This command will spawn an
> @@ -87,6 +102,9 @@ OPTIONS
> objects will be written into the repository, so applying these printed
> ref updates is generally safe.
>
> +`--reedit-message`::
> + Open an editor to modify the target commit's message.
> +
> `--update-refs=(branches|head)`::
> Control which references will be updated by the command, if any. With
> `branches`, all local branches that point to commits which are
> @@ -96,6 +114,36 @@ OPTIONS
> EXAMPLES
> --------
>
> +Fixup a commit
> +~~~~~~~~~~~~~~
> +
> +----------
> +$ git log --oneline --stat
> +abc1234 (HEAD -> main) third
> + third.txt | 1 +
> +def5678 second
> + second.txt | 1 +
> +ghi9012 first
> + first.txt | 1 +
> +
> +$ echo "change" >>unrelated.txt
> +$ git add unrelated.txt
> +$ git history fixup ghi9012
> +
> +$ git log --oneline --stat
> +jkl3456 (HEAD -> main) third
> + third.txt | 1 +
> +mno7890 second
> + second.txt | 1 +
> +pqr1234 first
> + first.txt | 1 +
> + unrelated.txt | 1 +
> +----------
> +
> +The staged addition of `unrelated.txt` has been incorporated into the `first`
> +commit. All descendant commits have been replayed on top of the rewritten
> +history.
> +
> Split a commit
> ~~~~~~~~~~~~~~
>
> diff --git a/builtin/history.c b/builtin/history.c
> index 549e352c74..6299f0dfa9 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -10,6 +10,7 @@
> #include "gettext.h"
> #include "hex.h"
> #include "lockfile.h"
> +#include "merge-ort.h"
> #include "oidmap.h"
> #include "parse-options.h"
> #include "path.h"
> @@ -23,6 +24,8 @@
> #include "unpack-trees.h"
> #include "wt-status.h"
>
> +#define GIT_HISTORY_FIXUP_USAGE \
> + N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]")
> #define GIT_HISTORY_REWORD_USAGE \
> N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
> #define GIT_HISTORY_SPLIT_USAGE \
> @@ -434,6 +437,154 @@ static int handle_reference_updates(struct rev_info *revs,
> return ret;
> }
>
> +static int cmd_history_fixup(int argc,
> + const char **argv,
> + const char *prefix,
> + struct repository *repo)
> +{
> + const char * const usage[] = {
> + GIT_HISTORY_FIXUP_USAGE,
> + NULL,
> + };
> + enum ref_action action = REF_ACTION_DEFAULT;
> + int dry_run = 0;
> + enum commit_tree_flags flags = 0;
> + struct option options[] = {
> + OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
> + N_("control which refs should be updated"),
> + PARSE_OPT_NONEG, parse_ref_action),
> + OPT_BOOL('n', "dry-run", &dry_run,
> + N_("perform a dry-run without updating any refs")),
> + OPT_BIT(0, "reedit-message", &flags,
> + N_("open an editor to modify the commit message"),
> + COMMIT_TREE_EDIT_MESSAGE),
> + OPT_END(),
> + };
> + struct merge_result merge_result = { 0 };
> + struct merge_options merge_opts = { 0 };
> + struct strbuf reflog_msg = STRBUF_INIT;
> + struct commit *head_commit, *original, *rewritten;
> + struct tree *head_tree, *original_tree, *index_tree;
> + struct rev_info revs = { 0 };
> + int ret;
> +
> + argc = parse_options(argc, argv, prefix, options, usage, 0);
> + if (argc != 1) {
> + ret = error(_("command expects a single revision"));
> + goto out;
> + }
> + repo_config(repo, git_default_config, NULL);
> +
> + if (action == REF_ACTION_DEFAULT)
> + action = REF_ACTION_BRANCHES;
> +
> + if (is_bare_repository()) {
> + ret = error(_("cannot run fixup in a bare repository"));
> + goto out;
> + }
> +
> + /* Resolve the original commit, which is the one we want to fix up. */
> + original = lookup_commit_reference_by_name(argv[0]);
> + if (!original) {
> + ret = error(_("commit cannot be found: %s"), argv[0]);
> + goto out;
> + }
> +
> + /*
> + * Resolve HEAD so we can use its tree as the merge base: the staged
> + * changes are expressed as a diff from HEAD's tree to the index tree.
> + */
> + head_commit = lookup_commit_reference_by_name("HEAD");
> + if (!head_commit) {
> + ret = error(_("cannot look up HEAD"));
> + goto out;
> + }
> +
> + head_tree = repo_get_commit_tree(repo, head_commit);
> + if (!head_tree) {
> + ret = error(_("cannot get tree for HEAD"));
> + goto out;
> + }
> +
> + if (repo_read_index(repo) < 0) {
> + ret = error(_("unable to read index"));
> + goto out;
> + }
> +
> + if (!repo_index_has_changes(repo, head_tree, NULL)) {
> + ret = error(_("nothing to fixup: no staged changes"));
> + goto out;
> + }
> +
> + /*
> + * Write the index as a tree object. This is the "theirs" side of the
> + * three-way merge: it is HEAD's tree with the staged changes applied.
> + */
> + index_tree = write_in_core_index_as_tree(repo, repo->index);
> + if (!index_tree) {
> + ret = error(_("unable to write index as a tree"));
> + goto out;
> + }
> +
> + original_tree = repo_get_commit_tree(repo, original);
> + if (!original_tree) {
> + ret = error(_("cannot get tree for commit %s"), argv[0]);
> + goto out;
> + }
> +
> + /*
> + * Perform the three-way merge to reapply changes in the index onto the
> + * target commit. This is using basically the same logic as a
> + * cherry-pick, where the base commit is our HEAD, ours is the original
> + * tree and theirs is the index tree.
> + */
OTOH, this explanation helps quite a bit here :)
> + init_basic_merge_options(&merge_opts, repo);
> + merge_opts.ancestor = "HEAD";
> + merge_opts.branch1 = argv[0];
> + merge_opts.branch2 = "staged";
> + merge_incore_nonrecursive(&merge_opts, head_tree,
> + original_tree, index_tree, &merge_result);
> +
> + if (merge_result.clean < 0) {
> + ret = error(_("merge failed while applying fixup"));
> + goto out;
> + }
> +
> + if (!merge_result.clean) {
> + ret = error(_("fixup would produce conflicts; aborting"));
> + goto out;
> + }
> +
> + ret = setup_revwalk(repo, action, original, &revs);
> + if (ret)
> + goto out;
> +
> + ret = commit_tree_ext(repo, "fixup", original, original->parents,
> + &original_tree->object.oid, &merge_result.tree->object.oid,
> + &rewritten, flags);
> + if (ret < 0) {
> + ret = error(_("failed writing fixed-up commit"));
> + goto out;
> + }
> +
> + strbuf_addf(&reflog_msg, "fixup: updating %s", argv[0]);
> +
> + ret = handle_reference_updates(&revs, action, original, rewritten,
> + reflog_msg.buf, dry_run);
> + if (ret < 0) {
> + ret = error(_("failed replaying descendants"));
> + goto out;
> + }
> +
> + ret = 0;
> +
> +out:
> + merge_finalize(&merge_opts, &merge_result);
> + strbuf_release(&reflog_msg);
> + release_revisions(&revs);
> + return ret;
> +}
> +
> static int cmd_history_reword(int argc,
> const char **argv,
> const char *prefix,
> @@ -745,12 +896,14 @@ int cmd_history(int argc,
> struct repository *repo)
> {
> const char * const usage[] = {
> + GIT_HISTORY_FIXUP_USAGE,
> GIT_HISTORY_REWORD_USAGE,
> GIT_HISTORY_SPLIT_USAGE,
> NULL,
> };
> parse_opt_subcommand_fn *fn = NULL;
> struct option options[] = {
> + OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
> OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
> OPT_SUBCOMMAND("split", &fn, cmd_history_split),
> OPT_END(),
> diff --git a/t/meson.build b/t/meson.build
> index 7528e5cda5..f502ad8ec9 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -397,6 +397,7 @@ integration_tests = [
> 't3450-history.sh',
> 't3451-history-reword.sh',
> 't3452-history-split.sh',
> + 't3453-history-fixup.sh',
> 't3500-cherry.sh',
> 't3501-revert-cherry-pick.sh',
> 't3502-cherry-pick-merge.sh',
> diff --git a/t/t3453-history-fixup.sh b/t/t3453-history-fixup.sh
> new file mode 100755
> index 0000000000..0012b1f052
> --- /dev/null
> +++ b/t/t3453-history-fixup.sh
> @@ -0,0 +1,500 @@
> +#!/bin/sh
> +
> +test_description='tests for git-history fixup subcommand'
> +
> +. ./test-lib.sh
> +
> +fixup_with_message () {
> + cat >message &&
> + write_script fake-editor.sh <<-\EOF &&
> + cp message "$1"
> + EOF
> + test_set_editor "$(pwd)"/fake-editor.sh &&
> + git history fixup --reedit-message "$@" &&
> + rm fake-editor.sh message
> +}
> +
> +expect_changes () {
> + git log --format="%s" --numstat "$@" >actual.raw &&
> + sed '/^$/d' <actual.raw >actual &&
> + cat >expect &&
> + test_cmp expect actual
> +}
> +
> +test_expect_success 'errors on missing commit argument' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + test_must_fail git history fixup 2>err &&
> + test_grep "command expects a single revision" err
> + )
> +'
> +
> +test_expect_success 'errors on too many arguments' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + test_must_fail git history fixup HEAD HEAD 2>err &&
> + test_grep "command expects a single revision" err
> + )
> +'
> +
> +test_expect_success 'errors on unknown revision' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + test_must_fail git history fixup does-not-exist 2>err &&
> + test_grep "commit cannot be found: does-not-exist" err
> + )
> +'
> +
> +test_expect_success 'errors when nothing is staged' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + test_must_fail git history fixup HEAD 2>err &&
> + test_grep "nothing to fixup: no staged changes" err
> + )
> +'
> +
> +test_expect_success 'errors in a bare repository' '
> + test_when_finished "rm -rf repo repo.git" &&
> + git init repo &&
> + test_commit -C repo initial &&
> + git clone --bare repo repo.git &&
> + test_must_fail git -C repo.git history fixup HEAD 2>err &&
> + test_grep "cannot run fixup in a bare repository" err
> +'
> +
> +test_expect_success 'can fixup the tip commit' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + echo content >file.txt &&
> + git add file.txt &&
> + git commit -m "add file" &&
> +
> + echo fix >>file.txt &&
> + git add file.txt &&
> +
> + expect_changes <<-\EOF &&
> + add file
> + 1 0 file.txt
> + initial
> + 1 0 initial.t
> + EOF
> +
> + git symbolic-ref HEAD >branch-expect &&
> + git history fixup HEAD &&
> + git symbolic-ref HEAD >branch-actual &&
> + test_cmp branch-expect branch-actual &&
> +
> + expect_changes <<-\EOF &&
> + add file
> + 2 0 file.txt
> + initial
> + 1 0 initial.t
> + EOF
> +
> + # Verify the fix is in the tip commit tree
> + git show HEAD:file.txt >actual &&
> + printf "content\nfix\n" >expect &&
> + test_cmp expect actual &&
> +
> + git reflog >reflog &&
> + test_grep "fixup: updating HEAD" reflog
> + )
> +'
> +
> +test_expect_success 'can fixup a commit in the middle of history' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit first &&
> + echo content >file.txt &&
> + git add file.txt &&
> + git commit -m "add file" &&
> + test_commit third &&
> +
> + echo fix >>file.txt &&
> + git add file.txt &&
> +
> + expect_changes <<-\EOF &&
> + third
> + 1 0 third.t
> + add file
> + 1 0 file.txt
> + first
> + 1 0 first.t
> + EOF
> +
> + git history fixup HEAD~ &&
> +
> + expect_changes <<-\EOF &&
> + third
> + 1 0 third.t
> + add file
> + 2 0 file.txt
> + first
> + 1 0 first.t
> + EOF
> +
> + # Verify the fix landed in the "add file" commit.
> + git show HEAD~:file.txt >actual &&
> + printf "content\nfix\n" >expect &&
> + test_cmp expect actual &&
> +
> + # And verify that the replayed commit also has the change.
> + git show HEAD:file.txt >actual &&
> + printf "content\nfix\n" >expect &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success 'can fixup root commit' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + echo initial >root.txt &&
> + git add root.txt &&
> + git commit -m "root" &&
> + test_commit second &&
> +
> + expect_changes <<-\EOF &&
> + second
> + 1 0 second.t
> + root
> + 1 0 root.txt
> + EOF
> +
> + echo fix >>root.txt &&
> + git add root.txt &&
> + git history fixup HEAD~ &&
> +
> + expect_changes <<-\EOF &&
> + second
> + 1 0 second.t
> + root
> + 2 0 root.txt
> + EOF
> +
> + git show HEAD~:root.txt >actual &&
> + printf "initial\nfix\n" >expect &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success 'preserves commit message and authorship' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + echo content >file.txt &&
> + git add file.txt &&
> + git commit --author="Original <original@example.com>" -m "original message" &&
> +
> + echo fix >>file.txt &&
> + git add file.txt &&
> + git history fixup HEAD &&
> +
> + # Message preserved
> + git log -1 --format="%s" >actual &&
> + echo "original message" >expect &&
> + test_cmp expect actual &&
> +
> + # Authorship preserved
> + git log -1 --format="%an <%ae>" >actual &&
> + echo "Original <original@example.com>" >expect &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success 'updates all descendant branches by default' '
> + test_when_finished "rm -rf repo" &&
> + git init repo --initial-branch=main &&
> + (
> + cd repo &&
> + test_commit base &&
> + git branch branch &&
> + test_commit ours &&
> + git switch branch &&
> + test_commit theirs &&
> + git switch main &&
> +
> + echo fix >fix.txt &&
> + git add fix.txt &&
> + git history fixup base &&
> +
> + expect_changes --branches <<-\EOF &&
> + theirs
> + 1 0 theirs.t
> + ours
> + 1 0 ours.t
> + base
> + 1 0 base.t
> + 1 0 fix.txt
> + EOF
> +
> + # Both branches should have the fix in the base
> + git show main~:fix.txt >actual &&
> + echo fix >expect &&
> + test_cmp expect actual &&
> + git show branch~:fix.txt >actual &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success 'can fixup commit on a different branch' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit base &&
> + git branch theirs &&
> + test_commit ours &&
> + git switch theirs &&
> + test_commit theirs &&
> +
> + # Stage a change while on "theirs"
> + echo fix >fix.txt &&
> + git add fix.txt &&
> +
> + # Ensure that "ours" does not change, as it does not contain
> + # the commit in question.
> + git rev-parse ours >ours-before &&
> + git history fixup theirs &&
> + git rev-parse ours >ours-after &&
> + test_cmp ours-before ours-after &&
> +
> + git show HEAD:fix.txt >actual &&
> + echo fix >expect &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success '--dry-run prints ref updates without modifying repo' '
> + test_when_finished "rm -rf repo" &&
> + git init repo --initial-branch=main &&
> + (
> + cd repo &&
> + test_commit base &&
> + git branch branch &&
> + test_commit main-tip &&
> + git switch branch &&
> + test_commit branch-tip &&
> + git switch main &&
> +
> + echo fix >fix.txt &&
> + git add fix.txt &&
> +
> + git refs list >refs-before &&
> + git history fixup --dry-run base >updates &&
> + git refs list >refs-after &&
> + test_cmp refs-before refs-after &&
> +
> + test_grep "update refs/heads/main" updates &&
> + test_grep "update refs/heads/branch" updates &&
> +
> + expect_changes --branches <<-\EOF &&
> + branch-tip
> + 1 0 branch-tip.t
> + main-tip
> + 1 0 main-tip.t
> + base
> + 1 0 base.t
> + EOF
> +
> + git update-ref --stdin <updates &&
> + expect_changes --branches <<-\EOF
> + branch-tip
> + 1 0 branch-tip.t
> + main-tip
> + 1 0 main-tip.t
> + base
> + 1 0 base.t
> + 1 0 fix.txt
> + EOF
> + )
> +'
> +
> +test_expect_success '--update-refs=head updates only HEAD' '
> + test_when_finished "rm -rf repo" &&
> + git init repo --initial-branch=main &&
> + (
> + cd repo &&
> + test_commit base &&
> + git branch branch &&
> + test_commit main-tip &&
> + git switch branch &&
> + test_commit branch-tip &&
> +
> + echo fix >fix.txt &&
> + git add fix.txt &&
> +
> + # Only HEAD (branch) should be updated
> + git history fixup --update-refs=head base &&
> +
> + # The main branch should be unaffected.
> + expect_changes main <<-\EOF &&
> + main-tip
> + 1 0 main-tip.t
> + base
> + 1 0 base.t
> + EOF
> +
> + # But the currently checked out branch should be modified.
> + expect_changes branch <<-\EOF
> + branch-tip
> + 1 0 branch-tip.t
> + base
> + 1 0 base.t
> + 1 0 fix.txt
> + EOF
> + )
> +'
> +
> +test_expect_success '--update-refs=head refuses to rewrite commits not in HEAD ancestry' '
> + test_when_finished "rm -rf repo" &&
> + git init repo --initial-branch=main &&
> + (
> + cd repo &&
> + test_commit base &&
> + git branch other &&
> + test_commit main-tip &&
> + git switch other &&
> + test_commit other-tip &&
> +
> + echo fix >fix.txt &&
> + git add fix.txt &&
> +
> + test_must_fail git history fixup --update-refs=head main-tip 2>err &&
> + test_grep "rewritten commit must be an ancestor of HEAD" err
> + )
> +'
> +
> +test_expect_success 'aborts when fixup would produce conflicts' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> +
> + echo "line one" >file.txt &&
> + git add file.txt &&
> + git commit -m "first" &&
> +
> + echo "line two" >file.txt &&
> + git add file.txt &&
> + git commit -m "second" &&
> +
> + echo "conflicting change" >file.txt &&
> + git add file.txt &&
> +
> + git refs list >refs-before &&
> + test_must_fail git history fixup HEAD~ 2>err &&
> + test_grep "fixup would produce conflicts" err &&
> + git refs list >refs-after &&
> + test_cmp refs-before refs-after
> + )
> +'
> +
> +test_expect_success '--reedit-message opens editor for the commit message' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + test_commit initial &&
> + echo content >file.txt &&
> + git add file.txt &&
> + git commit -m "add file" &&
> +
> + echo fix >>file.txt &&
> + git add file.txt &&
> +
> + fixup_with_message HEAD <<-\EOF &&
> + add file with fix
> + EOF
> +
> + expect_changes --branches <<-\EOF
> + add file with fix
> + 2 0 file.txt
> + initial
> + 1 0 initial.t
> + EOF
> + )
> +'
> +
> +test_expect_success 'retains unstaged working tree changes after fixup' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> + touch a b &&
> + git add . &&
> + git commit -m "initial commit" &&
> + echo staged >a &&
> + echo unstaged >b &&
> + git add a &&
> + git history fixup HEAD &&
> +
> + # b is still modified in the worktree but not staged
> + cat >expect <<-\EOF &&
> + M b
> + EOF
> + git status --porcelain --untracked-files=no >actual &&
> + test_cmp expect actual
> + )
> +'
> +
> +test_expect_success 'index is clean after fixup when target is HEAD' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> +
> + test_commit initial &&
> + echo fix >fix.txt &&
> + git add fix.txt &&
> + git history fixup HEAD &&
> +
> + git status --porcelain --untracked-files=no >actual &&
> + test_must_be_empty actual
> + )
> +'
> +
> +test_expect_success 'index is unchanged on conflict' '
> + test_when_finished "rm -rf repo" &&
> + git init repo &&
> + (
> + cd repo &&
> +
> + echo base >file.txt &&
> + git add file.txt &&
> + git commit -m base &&
> + echo change >file.txt &&
> + git add file.txt &&
> + git commit -m change &&
> +
> + echo conflict >file.txt &&
> + git add file.txt &&
> +
> + git diff --cached >index-before &&
> + test_must_fail git history fixup HEAD~ &&
> + git diff --cached >index-after &&
> + test_cmp index-before index-after
> + )
> +'
> +
> +test_done
>
> --
> 2.54.0.545.g6539524ca2.dirty
>
>
Thanks
--
D. Ben Knoble
^ permalink raw reply
* regression 2.54.0: test suite hangs indefinitely with mksh
From: Jan Palus @ 2026-04-22 19:00 UTC (permalink / raw)
To: git; +Cc: Andrew Au, Junio C Hamano
If /bin/sh points to mksh, git's test suite hangs forever and never
completes. An example of process tree for such hung test:
$ pstree -a `pgrep -f ./t5702-protocol-v2.sh`
t5702-protocol- ./t5702-protocol-v2.sh
└─git -c protocol.version=0 ls-remote -o hello -o worldfile:///home/users/builder/rpm/BUILD/gi
└─sh -c git-upload-pack '/home/users/builder/rpm/BUILD/git/t/trash directory.t5702-protocol-v2/file_parent'...
└─git-upload-pack /home/users/builder/rpm/BUILD/git/t/trash directory.t5702-protocol-v2/file_parent
bisect points to the following commit:
commit dd3693eb0859274d62feac8047e1d486b3beaf31 (HEAD)
Author: Andrew Au <cshung@gmail.com>
Date: Thu Mar 12 22:49:37 2026
transport-helper, connect: use clean_on_exit to reap children on abnormal exit
When a long-running service (e.g., a source indexer) runs as PID 1
inside a container and repeatedly spawns git, git may in turn spawn
child processes such as git-remote-https or ssh. If git exits abnormally
(e.g., via exit(128) on a transport error), the normal cleanup paths
(disconnect_helper, finish_connect) are bypassed, and these children are
never waited on. The children are reparented to PID 1, which does not
reap them, so they accumulate as zombies over time.
Set clean_on_exit and wait_after_clean on child_process structs in both
transport-helper.c and connect.c so that the existing run-command
cleanup infrastructure handles reaping on any exit path. This avoids
rolling custom atexit handlers that call finish_command(), which could
deadlock if the child is blocked waiting for the parent to close a pipe.
The clean_on_exit mechanism sends SIGTERM first, then waits, ensuring
the child terminates promptly. It also handles signal-based exits, not
just atexit.
Signed-off-by: Andrew Au <cshung@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
From commit message alone it seems the change tries to avoid reparenting
child processes to PID 1, however this does not seem to work for
processes which were spawned with "sh -c".
For example if /bin/sh points to bash tests pass because bash appears to
react to SIGTERM differently -- shell process ends but its children are
reparented anyway.
mksh on the other hand seems to ignore SIGTERM (or queues it?) but
waits for child process and so test suite never ends.
^ permalink raw reply
* [PATCH] l10n: it.po: fix italian usage messages alignment
From: Matteo Beniamino @ 2026-04-22 18:25 UTC (permalink / raw)
To: git; +Cc: Jiang Xin, Matteo Beniamino
In-Reply-To: <20260422182516.26667-1-beniamino@beniamino.eu>
Fixed a misalignment in the "usage:" and " or:" lines in the italian
help messages.
Signed-off-by: Matteo Beniamino <beniamino@beniamino.eu>
---
po/it.po | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/po/it.po b/po/it.po
index 20fd8bb28d..b5ccd8c731 100644
--- a/po/it.po
+++ b/po/it.po
@@ -5400,7 +5400,7 @@ msgstr "uso: %s"
#: parse-options.c:915
#, c-format
msgid " or: %s"
-msgstr " oppure: %s"
+msgstr " o: %s"
#: parse-options.c:918
#, c-format
--
2.53.0
^ permalink raw reply related
* Trivial fix in the italian translation
From: Matteo Beniamino @ 2026-04-22 18:25 UTC (permalink / raw)
To: git; +Cc: Jiang Xin
This is the current output of `git branch -h`:
$ git branch -h
uso: git branch [<opzioni>] [-r | -a] [--merged] [--no-merged]
oppure: git branch [<options>] [-f] [--recurse-submodules] <branch-name> [<start-point>]
oppure: git branch [<options>] [-l] [<pattern>...]
oppure: git branch [<opzioni>] [-r] (-d | -D) <nome branch>...
oppure: git branch [<opzioni>] (-m | -M) [<vecchio branch>] <nuovo branch>
oppure: git branch [<opzioni>] (-c | -C) [<vecchio branch>] <nuovo branch>
oppure: git branch [<opzioni>] [-r | -a] [--points-at]
oppure: git branch [<opzioni>] [-r | -a] [--format]
[...]
As you can see `uso:` (`usage:`) and `oppure:` (`or:`) are misaligned. As
commented in the .po the colon of `oppure:` should be aligned with the
one of `usage:`. In the following patch I've substituted `oppure` with
`o` (they're interchangable, and IMHO in the terminal every column
counts) and I've aligned the colons in the two strings.
^ permalink raw reply
* Re: [PATCH] generate-configlist: collapse depfile for older Ninja
From: D. Ben Knoble @ 2026-04-22 18:35 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Patrick Steinhardt, Phillip Wood
In-Reply-To: <20260421-toon-fix-almalinux8-v1-1-aec1d54addde@iotcl.com>
On Tue, Apr 21, 2026 at 3:17 PM Toon Claes <toon@iotcl.com> wrote:
>
> The tools/generate-configlist.sh script generates two files:
> * config-list.h
> * config-list.h.d
>
> The former is included by the source code and the latter defines on
> which files the former depends.
>
> The contents of `config-list.h.d` consists of two sections:
>
> config-list.h: Documentation/config.adoc
> config-list.h: Documentation/git-config.adoc
> config-list.h: Documentation/config/add.adoc
> config-list.h: Documentation/config/advice.adoc
> config-list.h: Documentation/config/alias.adoc
> config-list.h: Documentation/config/am.adoc
> config-list.h: Documentation/config/apply.adoc
> ...
>
> This first section actually defines on which individual files
> `config-list.h` depends and thus needs to be rebuild if one of those
> changes.
>
> And the second section contains content like:
>
> Documentation/config.adoc:
> Documentation/git-config.adoc:
> Documentation/config/add.adoc:
> Documentation/config/advice.adoc:
> Documentation/config/alias.adoc:
> Documentation/config/am.adoc:
> Documentation/config/apply.adoc:
> ...
>
> These rules exist to ensure Make won't fail with the following error if
> one of the .adoc files is renamed or removed:
>
> make: *** No rule to make target 'Documentation/config.adoc', needed by 'config-list.h'.
>
> With the no-op targets defined in `config-list.h.d`, Make knows there's
> no work to be done to generate these files, so it doesn't error out if
> it doesn't exist.
>
> For the Makefile build system this works great. And since
> ebeea3c471 (build: regenerate config-list.h when Documentation changes,
> 2026-02-24) this script is also called from the Meson build system.
> Nevertheless, on AlmaLinux 8 the following build failure is seen:
>
> ninja: error: dependency cycle: config-list.h -> config-list.h
>
> This version of this distro uses Ninja 1.8.2 and it seems to have some
> issues with the format of the `config-list.h.d` file.
>
> Ninja versions before 1.10.0 do not reset the depfile parser state on
> newlines. This causes issues when the depfile has one dependency per
> line, like we have in `config-list.h.d`:
>
> config-list.h: Documentation/config.adoc
> config-list.h: Documentation/config/add.adoc
>
> The parser only recognizes the first "config-list.h:" as a target. On
> subsequent lines it is still in dependency-parsing mode, so the repeated
> output name is recorded as an input. This causes the error mentioned
> above.
>
> The bug in Ninja is fixed in 1.10, with commit
> ninja-build/ninja@1daa7470ab7e (depfile_parser: remove restriction on
> multiple outputs, 2019-11-20).
Fascinating. Thanks for finding and fixing. I did wish while embarking
on this endeavor to find more documentation of what these depfiles
should look like, so I'm not surprised to find some bugs in how they
are parsed.
^ permalink raw reply
* Re: [PATCH] revision.c: implement --reverse=before for walks
From: D. Ben Knoble @ 2026-04-22 18:24 UTC (permalink / raw)
To: Mirko Faina
Cc: git, Junio C Hamano, Patrick Steinhardt, Jean-Noël Avila,
Jeff King
In-Reply-To: <aeUqSltEWIWaPDh3@exploit>
On Sun, Apr 19, 2026 at 4:31 PM Mirko Faina <mroik@delayed.space> wrote:
> > > > > if (revs->reverse_output_stage) {
> > > > > + if (revs->reverse == 2 && revs->max_count == 0)
> > > > > + return NULL;
> > > > > +
> >
> > PS: something I spotted on a second read. [Ignoring reverse=after
> > mode] This hunk looks to me like a nice little optimization (return
> > nothing if we know max_count says we yield no commits). Of course, I
> > could see that being viable early in the function, right? When asking
> > get_revision for commits, if max_count is 0, just return NULL.
> >
> > For reverse=after mode, this condition is only true if the max_count
> > was 0 in the previous conditional, also, since we use max_count=-1
> > before iterating get_revision_internal. That means the original
> > max_count isn't touched. At any rate, it _seems_ to me that the whole
> > function could benefit from this optimization… but I wonder if it is
> > _necessary_ for correctness of reverse=after in some way that I'm not
> > seeing? Since the current version doesn't need the early bailout, why
> > does reverse=after?
>
> Just to clarify, "reverse = 2" is "--reverse=before" and not
> "--reverse=after".
Oh golly, sorry about that!
> With "reverse = 2", the snippet of code you're referencing is not an
> optimization but a requirement for correctness. With "reverse = 1" we
> just keep the max_count as is and it's used by get_revision_internal()
> to stop if that limit is reached. What we find in 'reversed' are already
> just the commits we need to return.
>
> With "reverse = 2", we first set max_count to -1 and then retrieve the
> whole history, then we set max_count to its original value. Then we
> return the commits on each call of get_revision(). Now, unlike with
> "reverse = 1", we have the whole history in 'reversed', because of that
> we need to know when to stop. That's the reason we decrement max_count
> only for "reverse = 2" and why "max_count == 0" is checked only for
> "reverse = 2".
Ok, this explanation hasn't yet clicked…
> > > > > c = pop_commit(&revs->commits);
> > > > > + if (revs->reverse == 2)
> > > > > + revs->max_count--;
> > > >
> > > > Hm. Why do we decrement here? Again, not an area I’m familiar with, but a bit surprising.
> > >
> > > get_revision() (in revision.c) handles the reverse option and updates
> > > the "struct git_graph". get_revision() then calls
> > > get_revision_internal(), which handles commit boundaries and max_count,
> > > here is where it gets decreased. Since max_count gets decreased
> > > everytime get_revision_internal() is called, if we were to leave
> > > max_count as is before the walk (in get_revision() at line 4558), the
> > > walk would stop before reaching the root commit. This is why the current
> > > --reverse option is applied only after commit limiting options. So
> > > instead we set max_count at -1 walking the whole history and storing it
> > > in 'reversed'. Now we're in "reverse_output_stage = 1", and in this
> > > state we never call get_revision_internal() again, instead we pop
> > > commits from 'reversed'. Because of this we have to handle max_count
> > > outside get_revision_internal(), so we decrement it in the snippet of
> > > code you referenced.
> > >
> > > A bit verbose but hopefully it'll get my point across.
> >
> > I don't 100% follow, but I'm out of my depth :)
> >
> > I think I see that get_revision() effectively has 2 modes pertaining
> > to reverse: reverse and reverse output stage (the former falls
> > directly into the latter, though).
> >
> > After some setup, the reverse mode calls get_revision_internal() as
> > you said. That decrements max_count as a way of counting how many
> > commits we've seen through the loop, so if we asked for 5 we'd only
> > process 5 commits.
> >
> > Then we fall into the output stage mode, which pops a commit [1].
> >
> > With this patch, in reverse=after we disable max_count in the first
> > (reverse) mode, as you said. Ok: we get the whole (filtered) history
> > then, at which point we can now shrink. That makes sense.
> >
> > Then in the reverse output stage mode, we pretend to have one less
> > max_count. That's what I can't figure out. Is it because of the
> > pop_commit()? I guess I'm not totally seeing how that interacted with
> > the max_count in the original code: does the current code yield one
> > extra commit in get_revision_internal() ?
>
> I'm not sure I understand what you're referencing with "Then in the
> reverse output stage mode, we pretend to have one less max_count".
>
> If you're referring to line 4573, then...
>
> > You wrote that "we never call get_revision_internal() again," but I
> > don't see why that's true with this patch and not true before it.
> >
> > I do agree that _somebody_ has to handle max_count after
> > get_revision() returns with reverse=after. I'm just not sure what
> >
> > if (revs->reverse == 2)
> > revs->max_count--;
> >
> > is doing.
>
> ...we're not pretending we have fewer commits. Every subsequent call to
> get_revision() after the first call will never enter the branch at line
> 4548 and will only enter the branch at 4568. Everytime we pop a commit
> from 'reversed' we decrease max_count so we can limit only to the amount
> of commits the user wants.
>
> So, to recap, with "reverse = 2", on the first call to get_revision() we
> walk the whole history and store it in 'reversed' in reversed order and
> return the first commit.
> On subsequent calls to get_revision() we do not walk the history again,
> we simply return the commits that have been stored in 'reversed'.
> Everytime we pop a commit we have to decrease max_count, and we check
> againts max_count to know if we shouldn't return anymore commits (by
> returning NULL).
…but I think this one does. I think what I missed is that in all
"reverse" modes, get_revision() does some pre-computation and then
yields one at a time the commits. In traditional "after" mode, the
counting is done by get_revision_internal() [before reversal]. In the
new mode, get_revision takes on that responsibility of
get_revision_internal instead.
Hm. That suggests to me that get_revision's responsibilities are
becoming complex. Might be worth some version of a refactor, but idk
which.
> > Of course if I'm the only one confused and others make sense of it,
> > that's ok, too.
>
> No, I completely understand. I did have to retouch the function a few
> times after writing the tests :P
>
> > [1]: I traced this to 498bcd3159 (rev-list: fix --reverse interaction
> > with --parents, 2008-08-29), but I can't fathom what the pop is doing
> > there.
>
> It's pretty much doing the same thing it does now, it's returning stored
> commits. In both versions, the initial setup when "revs->reverse" is
> true, becomes "dead code" after the first call.
And this pop makes more sense now, too. Phew!
--
D. Ben Knoble
^ permalink raw reply
* Re: [PATCH 0/2] builtin/history: introduce "fixup" subcommand
From: Tian Yuchen @ 2026-04-22 18:18 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Elijah Newren
In-Reply-To: <20260422-b4-pks-history-fixup-v1-0-48d4484243de@pks.im>
Hi Patrick,
On 4/22/26 18:28, Patrick Steinhardt wrote:
> Hi,
>
> this short patch series introduces a new "fixup" subcommand. This
> command is the first one that I felt is missing in my day to day work,
> as I end up doing fixup commits quite often.
>
> The flow is rather simple: the user stages some changes, and then they
> execute `git history fixup <commit>` to amend those changes to the given
> commit. As with the other subcommands, dependent branches will then be
> rebased automatically.
>
> This is the first command that may result in merge conflicts. For now we
> simply abort in such cases, but there are plans to introduce first-class
> conflicts into Git. So once we have them, we'll also be able to handle
> such cases more gracefully. I still think that the command is useful
> even without that conflict handling.
Thank you for developing this feature. Godsend for lazy people like me ;)
Nevertheless, I seem to have come across what appears to be a bug. I
carried out the following steps:
create a.txt -> git add -> git commit -m "base" ->
create b.txt -> git add -> git commit -m "feature" ->
create c.txt -> git add -> git commit -m "tip" ->
rm b.txt -> git add ->
git history fixup HEAD~ ->
git log --oneline --stat...
And the output looks like:
3096a65 (HEAD -> master) tip
c.txt | 1 +
1 file changed, 1 insertion(+)
699f610 feature
0be07e6 base
a.txt | 1 +
1 file changed, 1 insertion(+)
More specifically, the output of
git show HEAD~
is:
Author: Tian Yuchen <cat@malon.dev>
Date: Thu Apr 23 01:57:17 2026 +0800
feature
which is an empty commit. Is it what we expect to see? Sorry that I
don't have enough time to look at the code in detail :P
Thanks, Yuchen
^ permalink raw reply
* Re: [BUG] v2.45+: git commit -S invalidates signature for non-UTF-8 messages
From: D. Ben Knoble @ 2026-04-22 18:13 UTC (permalink / raw)
To: brian m. carlson, Kushal Das, git
In-Reply-To: <aeakf0xcjSteTMZp@fruit.crustytoothpaste.net>
On Mon, Apr 20, 2026 at 6:12 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2026-04-20 at 08:59:05, Kushal Das wrote:
> > Hi all,
> >
> > Every `git commit -S` since v2.45.0 produces a permanently-BAD
> > signature when the commit message contains bytes that are not valid
> > UTF-8 AND `i18n.commitEncoding` is unset (i.e. the default case).
> > Verification fails under both `gpg --verify` and any non-GnuPG signer.
> > The failure is deterministic: it happens every time, on every
> > non-UTF-8 commit, no card or external tooling needed.
>
> I'm not sure that's a valid configuration. The commit message either
> needs to be UTF-8 or you need to declare the encoding so Git can convert
> it.
>
> > My best guess is commit 6206089cbd0b1cb30a017ec904567f040ab4cea0 starting
> > this (and I am maybe 100% wrong in identifying the cause).
>
> It does bisect to that commit. I wrote that patch originally, but it
> got modified and sent upstream by someone else. I'm not sure where it
> got introduced, though.
According to the `amlog` notes ref: <20231002024034.2611-9-ebiederm@gmail.com>
--
D. Ben Knoble
^ permalink raw reply
* Re: [PATCH] checkout: add --autostash option for branch switching
From: Harald Nordgren @ 2026-04-22 17:58 UTC (permalink / raw)
To: phillip.wood123
Cc: chris.torek, git, gitgitgadget, haraldnordgren, peff,
phillip.wood
In-Reply-To: <09d1390e-8334-49e6-a0b5-42d298db4caa@gmail.com>
👍
Harald
^ permalink raw reply
* git-subtree rewrite
From: Ian Jackson @ 2026-04-22 17:12 UTC (permalink / raw)
To: Avery Pennarun
Cc: Colin Stagner, git, Christian Heusel, george, Christian Hesse,
Phillip Wood, Junio C Hamano
In-Reply-To: <27109.63619.90318.366157@chiark.greenend.org.uk>
Hi, Avery.
tl;dr:
Do you object if I use the name git-subtree for my rewrite?
I intend it to be forward compatible with existing git-subtree
histories and existing command line invocations.
I've been looking into your git-subtree program. Thanks for it;
it is definitely solving a very real problem reasonably well. [1]
However, I think the existing implementation (in shell) and data model
need some work.
I have done some experiments, with enough success that I have more or
less decided to try to rewrite git-subtree.
I am intending to make my rewrite able to work with existing histories
(ie, projects which have done git-subtree add and git-subtree merge).
I intend to support the existing command line interface, although I
may improve that later.
I am also hoping to be able to define the data model more formally.
The git maintainers and others on the git mailing list seem reasonably
enthusiastic about all this. My nascent rewrite is a a standalone
Rust package, and the plan would be for it to obsolete the shell
script in git.git/contrib, but live outside the git project itself.
I would like to call my new program "git-subtree" and have it use
(and extend) the exisitng `git-subtree-...:` metadata that
`git-subtree add` puts into its generated commits.
Obviously there are compatibility, packaging, and deployment
considerations, which I'm keeping in mind. I don't want to break
anyone downstream. So I will proceed reasonably cautiously.
I hope this is all OK with you. If not, or if you have questions,
please let me know, using reply-all to this email (so the mailing list
gets a copy).
If I don't hear from you I will go ahead. The actual programming work
is going to take a while so watch this space but not too closely :-).
Regards,
Ian.
[1] See also my blog post
Never use git submodules
https://diziet.dreamwidth.org/14666.html
--
Ian Jackson <ijackson@chiark.greenend.org.uk> These opinions are my own.
Pronouns: they/he. If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.
^ permalink raw reply
* [PATCH v3 7/7] send-pack: pass negotiation config in push
From: Derrick Stolee via GitGitGadget @ 2026-04-22 15:25 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2085.v3.git.1776871546.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.
This change also updates the use of --negotiation-tip into
--negotiation-restrict now that the new synonym exists.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
send-pack.c | 39 +++++++++++++++++++++++++++++++--------
send-pack.h | 2 ++
t/t5516-fetch-push.sh | 30 ++++++++++++++++++++++++++++++
transport.c | 2 ++
4 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 67d6987b1c..d18e030ce8 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -433,28 +433,48 @@ static void reject_invalid_nonce(const char *nonce, int len)
static void get_commons_through_negotiation(struct repository *r,
const char *url,
+ const struct string_list *negotiation_include,
+ const struct string_list *negotiation_restrict,
const struct ref *remote_refs,
struct oid_array *commons)
{
struct child_process child = CHILD_PROCESS_INIT;
const struct ref *ref;
int len = r->hash_algo->hexsz + 1; /* hash + NL */
- int nr_negotiation_tip = 0;
+ int nr_negotiation = 0;
child.git_cmd = 1;
child.no_stdin = 1;
child.out = -1;
strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
- for (ref = remote_refs; ref; ref = ref->next) {
- if (!is_null_oid(&ref->new_oid)) {
- strvec_pushf(&child.args, "--negotiation-tip=%s",
- oid_to_hex(&ref->new_oid));
- nr_negotiation_tip++;
+
+ if (negotiation_restrict && negotiation_restrict->nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, negotiation_restrict)
+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
+ item->string);
+ nr_negotiation = negotiation_restrict->nr;
+ } else {
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (!is_null_oid(&ref->new_oid)) {
+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
+ oid_to_hex(&ref->new_oid));
+ nr_negotiation++;
+ }
}
}
+
+ if (negotiation_include && negotiation_include->nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, negotiation_include)
+ strvec_pushf(&child.args, "--negotiation-include=%s",
+ item->string);
+ nr_negotiation += negotiation_include->nr;
+ }
+
strvec_push(&child.args, url);
- if (!nr_negotiation_tip) {
+ if (!nr_negotiation) {
child_process_clear(&child);
return;
}
@@ -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 8a2d8adffc..60b73feb34 100644
--- a/transport.c
+++ b/transport.c
@@ -921,6 +921,8 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.atomic = !!(flags & TRANSPORT_PUSH_ATOMIC);
args.push_options = transport->push_options;
args.url = transport->url;
+ args.negotiation_include = &transport->remote->negotiation_include;
+ args.negotiation_restrict = &transport->remote->negotiation_restrict;
if (flags & TRANSPORT_PUSH_CERT_ALWAYS)
args.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 6/7] remote: add remote.*.negotiationInclude config
From: Derrick Stolee via GitGitGadget @ 2026-04-22 15:25 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2085.v3.git.1776871546.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.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 27 ++++++++++++++++++
Documentation/fetch-options.adoc | 4 +++
builtin/fetch.c | 10 +++++++
remote.c | 8 ++++++
remote.h | 1 +
t/t5510-fetch.sh | 49 ++++++++++++++++++++++++++++++++
6 files changed, 99 insertions(+)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index f1d889d03e..44de6d3c1f 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -126,6 +126,33 @@ 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` and `git push`,
+ 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 ref patterns
+ 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`) 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-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. This
+option is also used during push negotiation when `push.negotiate` is
+enabled.
++
+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 decc7f6abd..c475932602 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -91,6 +91,10 @@ 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 ef50e2fbe9..827438cf98 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1626,6 +1626,16 @@ 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) {
+ transport->smart_options->negotiation_include = &remote->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 166a56408a..15f3f12184 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)
@@ -570,6 +572,12 @@ static int handle_config(const char *key, const char *value,
string_list_clear(&remote->negotiation_restrict, 0);
else
string_list_append(&remote->negotiation_restrict, value);
+ } else if (!strcmp(subkey, "negotiationinclude")) {
+ /* reset list on empty value. */
+ if (!value || !*value)
+ string_list_clear(&remote->negotiation_include, 0);
+ else
+ string_list_append(&remote->negotiation_include, value);
} 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 4316f8d4ea..db73ed5379 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1577,6 +1577,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=alpha_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 '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 SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 5/7] fetch: add --negotiation-include option for negotiation
From: Derrick Stolee via GitGitGadget @ 2026-04-22 15:25 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2085.v3.git.1776871546.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 brances. 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
negotiation algorithm kicks in and performs a priority-queue walk from the
tip commits. In order to avoid duplicates, we mark the requested objects as
COMMON so they (and their descendants) are not output by the negotiator. The
negotiator still outputs at least one have before a round is flushed, when
the server could ACK to stop the negotiation.
Also add --negotiation-include to 'git pull' passthrough options.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/fetch-options.adoc | 19 ++++++
builtin/fetch.c | 16 ++++-
builtin/pull.c | 3 +
fetch-pack.c | 112 +++++++++++++++++++++++++++++--
fetch-pack.h | 10 ++-
t/t5510-fetch.sh | 66 ++++++++++++++++++
transport.c | 4 +-
transport.h | 6 ++
8 files changed, 227 insertions(+), 9 deletions(-)
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index c07b85499f..decc7f6abd 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=<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-include`.
+
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
ancestors of the provided `--negotiation-tip=` arguments,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a1960e3e0c..ef50e2fbe9 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;
@@ -1547,10 +1548,14 @@ static void add_negotiation_restrict_tips(struct git_transport_options *smart_op
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;
}
@@ -1615,6 +1620,13 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
strbuf_release(&config_name);
}
}
+ if (negotiation_include.nr) {
+ if (transport->smart_options)
+ transport->smart_options->negotiation_include = &negotiation_include;
+ else
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-include");
+ }
return transport;
}
@@ -2582,6 +2594,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 821cc6699a..86c85b60ef 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1002,6 +1002,9 @@ int cmd_pull(int argc,
OPT_PASSTHRU_ARGV(0, "negotiation-restrict", &opt_fetch, N_("revision"),
N_("report that we have only objects reachable from this object"),
0),
+ 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..8b080b0080 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,48 @@ static void send_filter(struct fetch_pack_args *args,
}
}
+static int add_oid_to_oidset(const struct reference *ref, void *cb_data)
+{
+ struct oidset *set = cb_data;
+ if (!odb_has_object(the_repository->objects, ref->oid, 0))
+ die(_("the object %s does not exist"), oid_to_hex(ref->oid));
+ oidset_insert(set, ref->oid);
+ return 0;
+}
+
+static void resolve_negotiation_include(const struct string_list *negotiation_include,
+ struct oidset *result)
+{
+ struct string_list_item *item;
+
+ if (!negotiation_include || !negotiation_include->nr)
+ return;
+
+ for_each_string_list_item(item, negotiation_include) {
+ if (!has_glob_specials(item->string)) {
+ struct object_id oid;
+
+ /* Ignore missing reference. */
+ if (repo_get_oid(the_repository, item->string, &oid))
+ continue;
+
+ /* Fail on missing object pointed by ref. */
+ if (!odb_has_object(the_repository->objects, &oid, 0))
+ die(_("the object %s does not exist"),
+ item->string);
+
+ oidset_insert(result, &oid);
+ } else {
+ struct refs_for_each_ref_options opts = {
+ .pattern = item->string,
+ };
+ refs_for_each_ref_ext(
+ get_main_ref_store(the_repository),
+ add_oid_to_oidset, result, &opts);
+ }
+ }
+}
+
static int find_common(struct fetch_negotiator *negotiator,
struct fetch_pack_args *args,
int fd[2], struct object_id *result_oid,
@@ -347,6 +390,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 +518,33 @@ 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 */
+ resolve_negotiation_include(args->negotiation_include,
+ &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++;
+
+ /*
+ * If this is a commit, then mark as COMMON to
+ * avoid the negotiator also outputting it as
+ * a have.
+ */
+ commit = lookup_commit(the_repository, oid);
+ if (commit &&
+ !repo_parse_commit(the_repository, commit))
+ commit->object.flags |= COMMON;
+ }
+ }
+
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 +655,7 @@ done:
flushes++;
}
strbuf_release(&req_buf);
+ oidset_clear(&negotiation_include_oids);
if (!got_ready || !no_done)
consume_shallow_list(args, &reader);
@@ -1305,12 +1377,26 @@ 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)))
+ packet_buf_write(req_buf, "have %s\n",
+ oid_to_hex(oid));
+ }
+
while ((oid = negotiator->next(negotiator))) {
+ if (negotiation_include_oids &&
+ oidset_contains(negotiation_include_oids, oid))
+ continue;
packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
if (++haves_added >= *haves_to_send)
break;
@@ -1358,7 +1444,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 +1500,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 +1745,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 +1818,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
state = FETCH_SEND_REQUEST;
mark_tips(negotiator, args->negotiation_restrict_tips);
+ resolve_negotiation_include(args->negotiation_include,
+ &negotiation_include_oids);
for_each_cached_alternate(negotiator,
insert_one_alternate_object);
break;
@@ -1747,7 +1838,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 +1975,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 +2274,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 string_list *negotiation_include)
{
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 +2292,9 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
fetch_negotiator_init(the_repository, &negotiator);
mark_tips(&negotiator, negotiation_restrict_tips);
+ resolve_negotiation_include(negotiation_include,
+ &negotiation_include_oids);
+
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
@@ -2221,7 +2319,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 +2372,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..32ae94d0b4 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -23,6 +23,13 @@ struct fetch_pack_args {
*/
const struct oid_array *negotiation_restrict_tips;
+ /*
+ * If non-empty, ref patterns whose tips should always be sent
+ * as "have" lines during negotiation, regardless of what the
+ * negotiation algorithm selects.
+ */
+ const struct string_list *negotiation_include;
+
unsigned deepen_relative:1;
unsigned quiet:1;
unsigned keep_pack:1;
@@ -93,7 +100,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 string_list *negotiation_include);
/*
* 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..4316f8d4ea 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1511,6 +1511,72 @@ 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 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..8a2d8adffc 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 = data->options.negotiation_include;
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);
ret = 0;
}
goto cleanup;
diff --git a/transport.h b/transport.h
index cdeb33c16f..6092775a27 100644
--- a/transport.h
+++ b/transport.h
@@ -48,6 +48,12 @@ struct git_transport_options {
*/
struct oid_array *negotiation_restrict_tips;
+ /*
+ * If non-empty, ref patterns whose tips should always be sent
+ * as "have" lines during negotiation.
+ */
+ const struct string_list *negotiation_include;
+
/*
* If allocated, whenever transport_fetch_refs() is called, add known
* common commits to this oidset instead of fetching any packfiles.
--
gitgitgadget
^ permalink raw reply related
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