From 04949538203ba7aeb298a607280715d4f26dbe0a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 5 Mar 2026 15:34:46 -0800 Subject: [PATCH 01/61] sideband: mask control characters The output of `git clone` is a vital component for understanding what has happened when things go wrong. However, these logs are partially under the control of the remote server (via the "sideband", which typically contains what the remote `git pack-objects` process sends to `stderr`), and is currently not sanitized by Git. This makes Git susceptible to ANSI escape sequence injection (see CWE-150, https://cwe.mitre.org/data/definitions/150.html), which allows attackers to corrupt terminal state, to hide information, and even to insert characters into the input buffer (i.e. as if the user had typed those characters). To plug this vulnerability, disallow any control character in the sideband, replacing them instead with the common `^` (e.g. `^[` for `\x1b`, `^A` for `\x01`). There is likely a need for more fine-grained controls instead of using a "heavy hammer" like this, which will be introduced subsequently. Helped-by: Phillip Wood Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- sideband.c | 17 +++++++++++++++-- t/t5409-colorize-remote-messages.sh | 12 ++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/sideband.c b/sideband.c index ea7c25211ef7e1..c1bbadccac682b 100644 --- a/sideband.c +++ b/sideband.c @@ -66,6 +66,19 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref list_config_item(list, prefix, keywords[i].keyword); } +static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n) +{ + strbuf_grow(dest, n); + for (; n && *src; src++, n--) { + if (!iscntrl(*src) || *src == '\t' || *src == '\n') { + strbuf_addch(dest, *src); + } else { + strbuf_addch(dest, '^'); + strbuf_addch(dest, *src == 0x7f ? '?' : 0x40 + *src); + } + } +} + /* * Optionally highlight one keyword in remote output if it appears at the start * of the line. This should be called for a single line only, which is @@ -81,7 +94,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n) int i; if (!want_color_stderr(use_sideband_colors())) { - strbuf_add(dest, src, n); + strbuf_add_sanitized(dest, src, n); return; } @@ -114,7 +127,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n) } } - strbuf_add(dest, src, n); + strbuf_add_sanitized(dest, src, n); } diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh index fa5de4500a4f50..aa5b57057148e0 100755 --- a/t/t5409-colorize-remote-messages.sh +++ b/t/t5409-colorize-remote-messages.sh @@ -98,4 +98,16 @@ test_expect_success 'fallback to color.ui' ' grep "error: error" decoded ' +test_expect_success 'disallow (color) control sequences in sideband' ' + write_script .git/color-me-surprised <<-\EOF && + printf "error: Have you \\033[31mread\\033[m this?\\n" >&2 + exec "$@" + EOF + test_config_global uploadPack.packObjectsHook ./color-me-surprised && + test_commit need-at-least-one-commit && + git clone --no-local . throw-away 2>stderr && + test_decode_color decoded && + test_grep ! RED decoded +' + test_done From 9ed1625a581a35d7ec2d851258cf4c7fc08c1ed7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 5 Mar 2026 15:34:47 -0800 Subject: [PATCH 02/61] sideband: introduce an "escape hatch" to allow control characters The preceding commit fixed the vulnerability whereas sideband messages (that are under the control of the remote server) could contain ANSI escape sequences that would be sent to the terminal verbatim. However, this fix may not be desirable under all circumstances, e.g. when remote servers deliberately add coloring to their messages to increase their urgency. To help with those use cases, give users a way to opt-out of the protections: `sideband.allowControlCharacters`. Suggested-by: brian m. carlson Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/config.adoc | 2 ++ Documentation/config/sideband.adoc | 5 +++++ sideband.c | 10 ++++++++++ t/t5409-colorize-remote-messages.sh | 8 +++++++- 4 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 Documentation/config/sideband.adoc diff --git a/Documentation/config.adoc b/Documentation/config.adoc index 62eebe7c54501c..dcea3c0c15e2a9 100644 --- a/Documentation/config.adoc +++ b/Documentation/config.adoc @@ -523,6 +523,8 @@ include::config/sequencer.adoc[] include::config/showbranch.adoc[] +include::config/sideband.adoc[] + include::config/sparse.adoc[] include::config/splitindex.adoc[] diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc new file mode 100644 index 00000000000000..3fb5045cd79581 --- /dev/null +++ b/Documentation/config/sideband.adoc @@ -0,0 +1,5 @@ +sideband.allowControlCharacters:: + By default, control characters that are delivered via the sideband + are masked, to prevent potentially unwanted ANSI escape sequences + from being sent to the terminal. Use this config setting to override + this behavior. diff --git a/sideband.c b/sideband.c index c1bbadccac682b..682f1cbbedb9b8 100644 --- a/sideband.c +++ b/sideband.c @@ -26,6 +26,8 @@ static struct keyword_entry keywords[] = { { "error", GIT_COLOR_BOLD_RED }, }; +static int allow_control_characters; + /* Returns a color setting (GIT_COLOR_NEVER, etc). */ static enum git_colorbool use_sideband_colors(void) { @@ -39,6 +41,9 @@ static enum git_colorbool use_sideband_colors(void) if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN) return use_sideband_colors_cached; + repo_config_get_bool(the_repository, "sideband.allowcontrolcharacters", + &allow_control_characters); + if (!repo_config_get_string_tmp(the_repository, key, &value)) use_sideband_colors_cached = git_config_colorbool(key, value); else if (!repo_config_get_string_tmp(the_repository, "color.ui", &value)) @@ -68,6 +73,11 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n) { + if (allow_control_characters) { + strbuf_add(dest, src, n); + return; + } + strbuf_grow(dest, n); for (; n && *src; src++, n--) { if (!iscntrl(*src) || *src == '\t' || *src == '\n') { diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh index aa5b57057148e0..9caee9a07f1556 100755 --- a/t/t5409-colorize-remote-messages.sh +++ b/t/t5409-colorize-remote-messages.sh @@ -105,9 +105,15 @@ test_expect_success 'disallow (color) control sequences in sideband' ' EOF test_config_global uploadPack.packObjectsHook ./color-me-surprised && test_commit need-at-least-one-commit && + git clone --no-local . throw-away 2>stderr && test_decode_color decoded && - test_grep ! RED decoded + test_grep ! RED decoded && + + rm -rf throw-away && + git -c sideband.allowControlCharacters clone --no-local . throw-away 2>stderr && + test_decode_color decoded && + test_grep RED decoded ' test_done From 12f0fda905b4af3a15c125f96808e49ddbe39742 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 5 Mar 2026 15:34:48 -0800 Subject: [PATCH 03/61] sideband: do allow ANSI color sequences by default The preceding two commits introduced special handling of the sideband channel to neutralize ANSI escape sequences before sending the payload to the terminal, and `sideband.allowControlCharacters` to override that behavior. However, as reported by brian m. carlson, some `pre-receive` hooks that are actively used in practice want to color their messages and therefore rely on the fact that Git passes them through to the terminal, even though they have no way to determine whether the receiving side can actually handle Escape sequences (think e.g. about the practice recommended by Git that third-party applications wishing to use Git functionality parse the output of Git commands). In contrast to other ANSI escape sequences, it is highly unlikely that coloring sequences can be essential tools in attack vectors that mislead Git users e.g. by hiding crucial information. Therefore we can have both: Continue to allow ANSI coloring sequences to be passed to the terminal by default, and neutralize all other ANSI Escape sequences. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/config/sideband.adoc | 18 ++++++-- sideband.c | 66 +++++++++++++++++++++++++++-- t/t5409-colorize-remote-messages.sh | 16 ++++++- 3 files changed, 91 insertions(+), 9 deletions(-) diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc index 3fb5045cd79581..b55c73726fe2c7 100644 --- a/Documentation/config/sideband.adoc +++ b/Documentation/config/sideband.adoc @@ -1,5 +1,17 @@ sideband.allowControlCharacters:: By default, control characters that are delivered via the sideband - are masked, to prevent potentially unwanted ANSI escape sequences - from being sent to the terminal. Use this config setting to override - this behavior. + are masked, except ANSI color sequences. This prevents potentially + unwanted ANSI escape sequences from being sent to the terminal. Use + this config setting to override this behavior: ++ +-- + `default`:: + `color`:: + Allow ANSI color sequences, line feeds and horizontal tabs, + but mask all other control characters. This is the default. + `false`:: + Mask all control characters other than line feeds and + horizontal tabs. + `true`:: + Allow all control characters to be sent to the terminal. +-- diff --git a/sideband.c b/sideband.c index 682f1cbbedb9b8..eeba6fa2ca8dd6 100644 --- a/sideband.c +++ b/sideband.c @@ -26,7 +26,12 @@ static struct keyword_entry keywords[] = { { "error", GIT_COLOR_BOLD_RED }, }; -static int allow_control_characters; +static enum { + ALLOW_NO_CONTROL_CHARACTERS = 0, + ALLOW_ANSI_COLOR_SEQUENCES = 1<<0, + ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES, + ALLOW_ALL_CONTROL_CHARACTERS = 1<<1, +} allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES; /* Returns a color setting (GIT_COLOR_NEVER, etc). */ static enum git_colorbool use_sideband_colors(void) @@ -41,8 +46,26 @@ static enum git_colorbool use_sideband_colors(void) if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN) return use_sideband_colors_cached; - repo_config_get_bool(the_repository, "sideband.allowcontrolcharacters", - &allow_control_characters); + switch (repo_config_get_maybe_bool(the_repository, "sideband.allowcontrolcharacters", &i)) { + case 0: /* Boolean value */ + allow_control_characters = i ? ALLOW_ALL_CONTROL_CHARACTERS : + ALLOW_NO_CONTROL_CHARACTERS; + break; + case -1: /* non-Boolean value */ + if (repo_config_get_string_tmp(the_repository, "sideband.allowcontrolcharacters", + &value)) + ; /* huh? `get_maybe_bool()` returned -1 */ + else if (!strcmp(value, "default")) + allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES; + else if (!strcmp(value, "color")) + allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES; + else + warning(_("unrecognized value for `sideband." + "allowControlCharacters`: '%s'"), value); + break; + default: + break; /* not configured */ + } if (!repo_config_get_string_tmp(the_repository, key, &value)) use_sideband_colors_cached = git_config_colorbool(key, value); @@ -71,9 +94,41 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref list_config_item(list, prefix, keywords[i].keyword); } +static int handle_ansi_color_sequence(struct strbuf *dest, const char *src, int n) +{ + int i; + + /* + * Valid ANSI color sequences are of the form + * + * ESC [ [ [; ]*] m + * + * These are part of the Select Graphic Rendition sequences which + * contain more than just color sequences, for more details see + * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR. + */ + + if (allow_control_characters != ALLOW_ANSI_COLOR_SEQUENCES || + n < 3 || src[0] != '\x1b' || src[1] != '[') + return 0; + + for (i = 2; i < n; i++) { + if (src[i] == 'm') { + strbuf_add(dest, src, i + 1); + return i; + } + if (!isdigit(src[i]) && src[i] != ';') + break; + } + + return 0; +} + static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n) { - if (allow_control_characters) { + int i; + + if (allow_control_characters == ALLOW_ALL_CONTROL_CHARACTERS) { strbuf_add(dest, src, n); return; } @@ -82,6 +137,9 @@ static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n) for (; n && *src; src++, n--) { if (!iscntrl(*src) || *src == '\t' || *src == '\n') { strbuf_addch(dest, *src); + } else if ((i = handle_ansi_color_sequence(dest, src, n))) { + src += i; + n -= i; } else { strbuf_addch(dest, '^'); strbuf_addch(dest, *src == 0x7f ? '?' : 0x40 + *src); diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh index 9caee9a07f1556..e5092d3b426cd3 100755 --- a/t/t5409-colorize-remote-messages.sh +++ b/t/t5409-colorize-remote-messages.sh @@ -100,7 +100,7 @@ test_expect_success 'fallback to color.ui' ' test_expect_success 'disallow (color) control sequences in sideband' ' write_script .git/color-me-surprised <<-\EOF && - printf "error: Have you \\033[31mread\\033[m this?\\n" >&2 + printf "error: Have you \\033[31mread\\033[m this?\\a\\n" >&2 exec "$@" EOF test_config_global uploadPack.packObjectsHook ./color-me-surprised && @@ -108,12 +108,24 @@ test_expect_success 'disallow (color) control sequences in sideband' ' git clone --no-local . throw-away 2>stderr && test_decode_color decoded && + test_grep RED decoded && + test_grep "\\^G" stderr && + tr -dc "\\007" actual && + test_must_be_empty actual && + + rm -rf throw-away && + git -c sideband.allowControlCharacters=false \ + clone --no-local . throw-away 2>stderr && + test_decode_color decoded && test_grep ! RED decoded && + test_grep "\\^G" stderr && rm -rf throw-away && git -c sideband.allowControlCharacters clone --no-local . throw-away 2>stderr && test_decode_color decoded && - test_grep RED decoded + test_grep RED decoded && + tr -dc "\\007" actual && + test_file_not_empty actual ' test_done From 128914438a0d2d55ae34314a0881f55a797024d5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 5 Mar 2026 15:34:49 -0800 Subject: [PATCH 04/61] sideband: add options to allow more control sequences to be passed through Even though control sequences that erase characters are quite juicy for attack scenarios, where attackers are eager to hide traces of suspicious activities, during the review of the side band sanitizing patch series concerns were raised that there might be some legimitate scenarios where Git server's `pre-receive` hooks use those sequences in a benign way. Control sequences to move the cursor can likewise be used to hide tracks by overwriting characters, and have been equally pointed out as having legitimate users. Let's add options to let users opt into passing through those ANSI Escape sequences: `sideband.allowControlCharacters` now supports also `cursor` and `erase`, and it parses the value as a comma-separated list. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/config/sideband.adoc | 9 ++- sideband.c | 91 ++++++++++++++++++++++++----- t/t5409-colorize-remote-messages.sh | 38 ++++++++++++ 3 files changed, 123 insertions(+), 15 deletions(-) diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc index b55c73726fe2c7..2bf04262840b02 100644 --- a/Documentation/config/sideband.adoc +++ b/Documentation/config/sideband.adoc @@ -2,13 +2,20 @@ sideband.allowControlCharacters:: By default, control characters that are delivered via the sideband are masked, except ANSI color sequences. This prevents potentially unwanted ANSI escape sequences from being sent to the terminal. Use - this config setting to override this behavior: + this config setting to override this behavior (the value can be + a comma-separated list of the following keywords): + -- `default`:: `color`:: Allow ANSI color sequences, line feeds and horizontal tabs, but mask all other control characters. This is the default. + `cursor:`: + Allow control sequences that move the cursor. This is + disabled by default. + `erase`:: + Allow control sequences that erase charactrs. This is + disabled by default. `false`:: Mask all control characters other than line feeds and horizontal tabs. diff --git a/sideband.c b/sideband.c index eeba6fa2ca8dd6..0b420ca3193888 100644 --- a/sideband.c +++ b/sideband.c @@ -29,9 +29,43 @@ static struct keyword_entry keywords[] = { static enum { ALLOW_NO_CONTROL_CHARACTERS = 0, ALLOW_ANSI_COLOR_SEQUENCES = 1<<0, + ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1, + ALLOW_ANSI_ERASE = 1<<2, ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES, - ALLOW_ALL_CONTROL_CHARACTERS = 1<<1, -} allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES; + ALLOW_ALL_CONTROL_CHARACTERS = 1<<3, +} allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES; + +static inline int skip_prefix_in_csv(const char *value, const char *prefix, + const char **out) +{ + if (!skip_prefix(value, prefix, &value) || + (*value && *value != ',')) + return 0; + *out = value + !!*value; + return 1; +} + +static void parse_allow_control_characters(const char *value) +{ + allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS; + while (*value) { + if (skip_prefix_in_csv(value, "default", &value)) + allow_control_characters |= ALLOW_DEFAULT_ANSI_SEQUENCES; + else if (skip_prefix_in_csv(value, "color", &value)) + allow_control_characters |= ALLOW_ANSI_COLOR_SEQUENCES; + else if (skip_prefix_in_csv(value, "cursor", &value)) + allow_control_characters |= ALLOW_ANSI_CURSOR_MOVEMENTS; + else if (skip_prefix_in_csv(value, "erase", &value)) + allow_control_characters |= ALLOW_ANSI_ERASE; + else if (skip_prefix_in_csv(value, "true", &value)) + allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS; + else if (skip_prefix_in_csv(value, "false", &value)) + allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS; + else + warning(_("unrecognized value for `sideband." + "allowControlCharacters`: '%s'"), value); + } +} /* Returns a color setting (GIT_COLOR_NEVER, etc). */ static enum git_colorbool use_sideband_colors(void) @@ -55,13 +89,8 @@ static enum git_colorbool use_sideband_colors(void) if (repo_config_get_string_tmp(the_repository, "sideband.allowcontrolcharacters", &value)) ; /* huh? `get_maybe_bool()` returned -1 */ - else if (!strcmp(value, "default")) - allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES; - else if (!strcmp(value, "color")) - allow_control_characters = ALLOW_ANSI_COLOR_SEQUENCES; else - warning(_("unrecognized value for `sideband." - "allowControlCharacters`: '%s'"), value); + parse_allow_control_characters(value); break; default: break; /* not configured */ @@ -94,7 +123,7 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref list_config_item(list, prefix, keywords[i].keyword); } -static int handle_ansi_color_sequence(struct strbuf *dest, const char *src, int n) +static int handle_ansi_sequence(struct strbuf *dest, const char *src, int n) { int i; @@ -106,14 +135,47 @@ static int handle_ansi_color_sequence(struct strbuf *dest, const char *src, int * These are part of the Select Graphic Rendition sequences which * contain more than just color sequences, for more details see * https://en.wikipedia.org/wiki/ANSI_escape_code#SGR. + * + * The cursor movement sequences are: + * + * ESC [ n A - Cursor up n lines (CUU) + * ESC [ n B - Cursor down n lines (CUD) + * ESC [ n C - Cursor forward n columns (CUF) + * ESC [ n D - Cursor back n columns (CUB) + * ESC [ n E - Cursor next line, beginning (CNL) + * ESC [ n F - Cursor previous line, beginning (CPL) + * ESC [ n G - Cursor to column n (CHA) + * ESC [ n ; m H - Cursor position (row n, col m) (CUP) + * ESC [ n ; m f - Same as H (HVP) + * + * The sequences to erase characters are: + * + * + * ESC [ 0 J - Clear from cursor to end of screen (ED) + * ESC [ 1 J - Clear from cursor to beginning of screen (ED) + * ESC [ 2 J - Clear entire screen (ED) + * ESC [ 3 J - Clear entire screen + scrollback (ED) - xterm extension + * ESC [ 0 K - Clear from cursor to end of line (EL) + * ESC [ 1 K - Clear from cursor to beginning of line (EL) + * ESC [ 2 K - Clear entire line (EL) + * ESC [ n M - Delete n lines (DL) + * ESC [ n P - Delete n characters (DCH) + * ESC [ n X - Erase n characters (ECH) + * + * For a comprehensive list of common ANSI Escape sequences, see + * https://www.xfree86.org/current/ctlseqs.html */ - if (allow_control_characters != ALLOW_ANSI_COLOR_SEQUENCES || - n < 3 || src[0] != '\x1b' || src[1] != '[') + if (n < 3 || src[0] != '\x1b' || src[1] != '[') return 0; for (i = 2; i < n; i++) { - if (src[i] == 'm') { + if (((allow_control_characters & ALLOW_ANSI_COLOR_SEQUENCES) && + src[i] == 'm') || + ((allow_control_characters & ALLOW_ANSI_CURSOR_MOVEMENTS) && + strchr("ABCDEFGHf", src[i])) || + ((allow_control_characters & ALLOW_ANSI_ERASE) && + strchr("JKMPX", src[i]))) { strbuf_add(dest, src, i + 1); return i; } @@ -128,7 +190,7 @@ static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n) { int i; - if (allow_control_characters == ALLOW_ALL_CONTROL_CHARACTERS) { + if ((allow_control_characters & ALLOW_ALL_CONTROL_CHARACTERS)) { strbuf_add(dest, src, n); return; } @@ -137,7 +199,8 @@ static void strbuf_add_sanitized(struct strbuf *dest, const char *src, int n) for (; n && *src; src++, n--) { if (!iscntrl(*src) || *src == '\t' || *src == '\n') { strbuf_addch(dest, *src); - } else if ((i = handle_ansi_color_sequence(dest, src, n))) { + } else if (allow_control_characters != ALLOW_NO_CONTROL_CHARACTERS && + (i = handle_ansi_sequence(dest, src, n))) { src += i; n -= i; } else { diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh index e5092d3b426cd3..896e790bf955cd 100755 --- a/t/t5409-colorize-remote-messages.sh +++ b/t/t5409-colorize-remote-messages.sh @@ -128,4 +128,42 @@ test_expect_success 'disallow (color) control sequences in sideband' ' test_file_not_empty actual ' +test_decode_csi() { + awk '{ + while (match($0, /\033/) != 0) { + printf "%sCSI ", substr($0, 1, RSTART-1); + $0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1); + } + print + }' +} + +test_expect_success 'control sequences in sideband allowed by default' ' + write_script .git/color-me-surprised <<-\EOF && + printf "error: \\033[31mcolor\\033[m\\033[Goverwrite\\033[Gerase\\033[K\\033?25l\\n" >&2 + exec "$@" + EOF + test_config_global uploadPack.packObjectsHook ./color-me-surprised && + test_commit need-at-least-one-commit-at-least && + + rm -rf throw-away && + git clone --no-local . throw-away 2>stderr && + test_decode_color color-decoded && + test_decode_csi decoded && + test_grep ! "CSI \\[K" decoded && + test_grep ! "CSI \\[G" decoded && + test_grep "\\^\\[?25l" decoded && + + rm -rf throw-away && + git -c sideband.allowControlCharacters=erase,cursor,color \ + clone --no-local . throw-away 2>stderr && + test_decode_color color-decoded && + test_decode_csi decoded && + test_grep "RED" decoded && + test_grep "CSI \\[K" decoded && + test_grep "CSI \\[G" decoded && + test_grep ! "\\^\\[\\[K" decoded && + test_grep ! "\\^\\[\\[G" decoded +' + test_done From 602c83f0efed46c2e86a36273673bf8776ded04e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 5 Mar 2026 15:34:50 -0800 Subject: [PATCH 05/61] sideband: offer to configure sanitizing on a per-URL basis The main objection against sanitizing the sideband that was raised during the review of the sideband sanitizing patches, first on the git-security mailing list, then on the public mailing list, was that there are some setups where server-side `pre-receive` hooks want to error out, giving colorful messages to the users on the client side (if they are not redirecting the output into a file, that is). To avoid breaking such setups, the default chosen by the sideband sanitizing patches is to pass through ANSI color sequences. Still, there might be some use case out there where that is not enough. Therefore the `sideband.allowControlCharacters` config setting allows for configuring levels of sanitizing. As Junio Hamano pointed out, to keep users safe by default, we need to be able to scope this to some servers because while a user may trust their company's Git server, the same might not apply to other Git servers. To allow for this, let's imitate the way `http..*` offers to scope config settings to certain URLs, by letting users override the `sideband.allowControlCharacters` setting via `sideband..allowControlCharacters`. Suggested-by: Junio Hamano Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- Documentation/config/sideband.adoc | 4 ++ sideband.c | 81 ++++++++++++++++++++--------- sideband.h | 14 +++++ t/t5409-colorize-remote-messages.sh | 24 +++++++++ transport.c | 3 ++ 5 files changed, 102 insertions(+), 24 deletions(-) diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc index 2bf04262840b02..32088bbf2f0a40 100644 --- a/Documentation/config/sideband.adoc +++ b/Documentation/config/sideband.adoc @@ -22,3 +22,7 @@ sideband.allowControlCharacters:: `true`:: Allow all control characters to be sent to the terminal. -- + +sideband..*:: + Apply the `sideband.*` option selectively to specific URLs. The + same URL matching logic applies as for `http..*` settings. diff --git a/sideband.c b/sideband.c index 0b420ca3193888..a90db9e2880cba 100644 --- a/sideband.c +++ b/sideband.c @@ -10,6 +10,7 @@ #include "help.h" #include "pkt-line.h" #include "write-or-die.h" +#include "urlmatch.h" struct keyword_entry { /* @@ -27,13 +28,14 @@ static struct keyword_entry keywords[] = { }; static enum { - ALLOW_NO_CONTROL_CHARACTERS = 0, - ALLOW_ANSI_COLOR_SEQUENCES = 1<<0, - ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1, - ALLOW_ANSI_ERASE = 1<<2, - ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES, - ALLOW_ALL_CONTROL_CHARACTERS = 1<<3, -} allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES; + ALLOW_CONTROL_SEQUENCES_UNSET = -1, + ALLOW_NO_CONTROL_CHARACTERS = 0, + ALLOW_ANSI_COLOR_SEQUENCES = 1<<0, + ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1, + ALLOW_ANSI_ERASE = 1<<2, + ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES, + ALLOW_ALL_CONTROL_CHARACTERS = 1<<3, +} allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET; static inline int skip_prefix_in_csv(const char *value, const char *prefix, const char **out) @@ -45,8 +47,19 @@ static inline int skip_prefix_in_csv(const char *value, const char *prefix, return 1; } -static void parse_allow_control_characters(const char *value) +int sideband_allow_control_characters_config(const char *var, const char *value) { + switch (git_parse_maybe_bool(value)) { + case 0: + allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS; + return 0; + case 1: + allow_control_characters = ALLOW_ALL_CONTROL_CHARACTERS; + return 0; + default: + break; + } + allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS; while (*value) { if (skip_prefix_in_csv(value, "default", &value)) @@ -62,9 +75,37 @@ static void parse_allow_control_characters(const char *value) else if (skip_prefix_in_csv(value, "false", &value)) allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS; else - warning(_("unrecognized value for `sideband." - "allowControlCharacters`: '%s'"), value); + warning(_("unrecognized value for '%s': '%s'"), var, value); } + return 0; +} + +static int sideband_config_callback(const char *var, const char *value, + const struct config_context *ctx UNUSED, + void *data UNUSED) +{ + if (!strcmp(var, "sideband.allowcontrolcharacters")) + return sideband_allow_control_characters_config(var, value); + + return 0; +} + +void sideband_apply_url_config(const char *url) +{ + struct urlmatch_config config = URLMATCH_CONFIG_INIT; + char *normalized_url; + + if (!url) + BUG("must not call sideband_apply_url_config(NULL)"); + + config.section = "sideband"; + config.collect_fn = sideband_config_callback; + + normalized_url = url_normalize(url, &config.url); + repo_config(the_repository, urlmatch_config_entry, &config); + free(normalized_url); + string_list_clear(&config.vars, 1); + urlmatch_config_release(&config); } /* Returns a color setting (GIT_COLOR_NEVER, etc). */ @@ -80,20 +121,12 @@ static enum git_colorbool use_sideband_colors(void) if (use_sideband_colors_cached != GIT_COLOR_UNKNOWN) return use_sideband_colors_cached; - switch (repo_config_get_maybe_bool(the_repository, "sideband.allowcontrolcharacters", &i)) { - case 0: /* Boolean value */ - allow_control_characters = i ? ALLOW_ALL_CONTROL_CHARACTERS : - ALLOW_NO_CONTROL_CHARACTERS; - break; - case -1: /* non-Boolean value */ - if (repo_config_get_string_tmp(the_repository, "sideband.allowcontrolcharacters", - &value)) - ; /* huh? `get_maybe_bool()` returned -1 */ - else - parse_allow_control_characters(value); - break; - default: - break; /* not configured */ + if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET) { + if (!repo_config_get_value(the_repository, "sideband.allowcontrolcharacters", &value)) + sideband_allow_control_characters_config("sideband.allowcontrolcharacters", value); + + if (allow_control_characters == ALLOW_CONTROL_SEQUENCES_UNSET) + allow_control_characters = ALLOW_DEFAULT_ANSI_SEQUENCES; } if (!repo_config_get_string_tmp(the_repository, key, &value)) diff --git a/sideband.h b/sideband.h index 5a25331be55d30..d15fa4015fa0a3 100644 --- a/sideband.h +++ b/sideband.h @@ -30,4 +30,18 @@ int demultiplex_sideband(const char *me, int status, void send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max); +/* + * Apply sideband configuration for the given URL. This should be called + * when a transport is created to allow URL-specific configuration of + * sideband behavior (e.g., sideband..allowControlCharacters). + */ +void sideband_apply_url_config(const char *url); + +/* + * Parse and set the sideband allow control characters configuration. + * The var parameter should be the key name (without section prefix). + * Returns 0 if the variable was recognized and handled, non-zero otherwise. + */ +int sideband_allow_control_characters_config(const char *var, const char *value); + #endif diff --git a/t/t5409-colorize-remote-messages.sh b/t/t5409-colorize-remote-messages.sh index 896e790bf955cd..3010913bb113e4 100755 --- a/t/t5409-colorize-remote-messages.sh +++ b/t/t5409-colorize-remote-messages.sh @@ -166,4 +166,28 @@ test_expect_success 'control sequences in sideband allowed by default' ' test_grep ! "\\^\\[\\[G" decoded ' +test_expect_success 'allow all control sequences for a specific URL' ' + write_script .git/eraser <<-\EOF && + printf "error: Ohai!\\r\\033[K" >&2 + exec "$@" + EOF + test_config_global uploadPack.packObjectsHook ./eraser && + test_commit one-more-please && + + rm -rf throw-away && + git clone --no-local . throw-away 2>stderr && + test_decode_color color-decoded && + test_decode_csi decoded && + test_grep ! "CSI \\[K" decoded && + test_grep "\\^\\[\\[K" decoded && + + rm -rf throw-away && + git -c "sideband.file://.allowControlCharacters=true" \ + clone --no-local "file://$PWD" throw-away 2>stderr && + test_decode_color color-decoded && + test_decode_csi decoded && + test_grep "CSI \\[K" decoded && + test_grep ! "\\^\\[\\[K" decoded +' + test_done diff --git a/transport.c b/transport.c index c7f06a7382e605..1602065953a54e 100644 --- a/transport.c +++ b/transport.c @@ -29,6 +29,7 @@ #include "object-name.h" #include "color.h" #include "bundle-uri.h" +#include "sideband.h" static enum git_colorbool transport_use_color = GIT_COLOR_UNKNOWN; static char transport_colors[][COLOR_MAXLEN] = { @@ -1245,6 +1246,8 @@ struct transport *transport_get(struct remote *remote, const char *url) ret->hash_algo = &hash_algos[GIT_HASH_SHA1_LEGACY]; + sideband_apply_url_config(ret->url); + return ret; } From 826cc4722088a02d0ae240c1267b5b74d476b153 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 5 Mar 2026 15:34:51 -0800 Subject: [PATCH 06/61] sideband: drop 'default' configuration The topic so far allows users to tweak the configuration variable sideband.allowControlCharacters to override the hardcoded default, but among which there is the value called 'default'. The plan [*] of the series is to loosen the setting by a later commit in the series and schedule it to tighten at the Git 3.0 boundary for end users, at which point, the meaning of this 'default' value will change. Which is a dubious design. A user expresses their preference by setting configuration variable in order to guard against sudden change brought in by changes to the hardcoded default behaviour, and letting them set it to 'default' that will change at the Git 3.0 boundary defeats its purpose. If a user wants to say "I am easy and can go with whatever hardcoded default Git implementors choose for me", they simply leave the configuration variable unspecified. Let's remove it from the state before Git 3.0 so that those users who set it to 'default' will not see the behaviour changed under their feet all of sudden. Signed-off-by: Junio C Hamano --- Documentation/config/sideband.adoc | 1 - sideband.c | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Documentation/config/sideband.adoc b/Documentation/config/sideband.adoc index 32088bbf2f0a40..96fade7f5fee39 100644 --- a/Documentation/config/sideband.adoc +++ b/Documentation/config/sideband.adoc @@ -6,7 +6,6 @@ sideband.allowControlCharacters:: a comma-separated list of the following keywords): + -- - `default`:: `color`:: Allow ANSI color sequences, line feeds and horizontal tabs, but mask all other control characters. This is the default. diff --git a/sideband.c b/sideband.c index a90db9e2880cba..04282a568edd90 100644 --- a/sideband.c +++ b/sideband.c @@ -33,8 +33,8 @@ static enum { ALLOW_ANSI_COLOR_SEQUENCES = 1<<0, ALLOW_ANSI_CURSOR_MOVEMENTS = 1<<1, ALLOW_ANSI_ERASE = 1<<2, - ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES, ALLOW_ALL_CONTROL_CHARACTERS = 1<<3, + ALLOW_DEFAULT_ANSI_SEQUENCES = ALLOW_ANSI_COLOR_SEQUENCES } allow_control_characters = ALLOW_CONTROL_SEQUENCES_UNSET; static inline int skip_prefix_in_csv(const char *value, const char *prefix, @@ -62,9 +62,7 @@ int sideband_allow_control_characters_config(const char *var, const char *value) allow_control_characters = ALLOW_NO_CONTROL_CHARACTERS; while (*value) { - if (skip_prefix_in_csv(value, "default", &value)) - allow_control_characters |= ALLOW_DEFAULT_ANSI_SEQUENCES; - else if (skip_prefix_in_csv(value, "color", &value)) + if (skip_prefix_in_csv(value, "color", &value)) allow_control_characters |= ALLOW_ANSI_COLOR_SEQUENCES; else if (skip_prefix_in_csv(value, "cursor", &value)) allow_control_characters |= ALLOW_ANSI_CURSOR_MOVEMENTS; From f584f9d36129f8af18251bd0f193c914ed8b0cfb Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Wed, 11 Mar 2026 14:19:38 +0000 Subject: [PATCH 07/61] promisor-remote: prevent lazy-fetch recursion in child fetch fetch_objects() spawns a child `git fetch` to lazily fill in missing objects. That child's index-pack, when it receives a thin pack containing a REF_DELTA against a still-missing base, calls promisor_remote_get_direct() -- which is fetch_objects() again. With negotiationAlgorithm=noop the client advertises no "have" lines, so a well-behaved server sends requested objects un-deltified or deltified only against objects in the same pack. A server that nevertheless sends REF_DELTA against a base the client does not have is misbehaving; however the client should not recurse unboundedly in response. Propagate GIT_NO_LAZY_FETCH=1 into the child fetch's environment so that if the child's index-pack encounters such a REF_DELTA, it hits the existing guard at the top of fetch_objects() and fails fast instead of recursing. Depth-1 lazy fetch (the whole point of fetch_objects()) is unaffected: only the child and its descendants see the variable. Add a test that injects a thin pack containing a REF_DELTA against a missing base via HTTP, triggering the recursion path through index-pack's promisor_remote_get_direct() call. With the fix, the child's fetch_objects() sees GIT_NO_LAZY_FETCH=1 and blocks the depth-2 fetch with a "lazy fetching disabled" warning. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- promisor-remote.c | 7 +++++ t/t5616-partial-clone.sh | 60 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/promisor-remote.c b/promisor-remote.c index 77ebf537e2b3ee..2f56e89404b19a 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo, child.in = -1; if (repo != the_repository) prepare_other_repo_env(&child.env, repo->gitdir); + /* + * Prevent the child's index-pack from recursing back into + * fetch_objects() when resolving REF_DELTA bases it does not + * have. With noop negotiation the server should never need + * to send such deltas, so a depth-2 fetch would not help. + */ + strvec_pushf(&child.env, "%s=1", NO_LAZY_FETCH_ENVIRONMENT); strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop", "fetch", remote_name, "--no-tags", "--no-write-fetch-head", "--recurse-submodules=no", diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 1e354e057fa12c..27f131c8d9ba57 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -907,6 +907,66 @@ test_expect_success PERL_TEST_HELPERS 'tolerate server sending REF_DELTA against ! test -e "$HTTPD_ROOT_PATH/one-time-script" ' +test_expect_success PERL_TEST_HELPERS 'lazy-fetch of REF_DELTA with missing base does not recurse' ' + SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" && + rm -rf "$SERVER" repo && + test_create_repo "$SERVER" && + test_config -C "$SERVER" uploadpack.allowfilter 1 && + test_config -C "$SERVER" uploadpack.allowanysha1inwant 1 && + + # Create a commit with 2 blobs to be used as delta base and content. + for i in $(test_seq 10) + do + echo "this is a line" >>"$SERVER/foo.txt" && + echo "this is another line" >>"$SERVER/bar.txt" || return 1 + done && + git -C "$SERVER" add foo.txt bar.txt && + git -C "$SERVER" commit -m initial && + BLOB_FOO=$(git -C "$SERVER" rev-parse HEAD:foo.txt) && + BLOB_BAR=$(git -C "$SERVER" rev-parse HEAD:bar.txt) && + + # Partial clone with blob:none. The client has commits and + # trees but no blobs. + test_config -C "$SERVER" protocol.version 2 && + git -c protocol.version=2 clone --no-checkout \ + --filter=blob:none $HTTPD_URL/one_time_script/server repo && + + # Sanity check: client does not have either blob locally. + git -C repo rev-list --objects --ignore-missing \ + -- $BLOB_FOO >objlist && + test_line_count = 0 objlist && + + # Craft a thin pack where BLOB_FOO is a REF_DELTA against + # BLOB_BAR. Since the client has neither blob (blob:none + # filter), the delta base will be missing. This simulates a + # misbehaving server that sends REF_DELTA against an object + # the client does not have. + test-tool -C "$SERVER" pack-deltas --num-objects=1 >thin.pack <<-EOF && + REF_DELTA $BLOB_FOO $BLOB_BAR + EOF + + replace_packfile thin.pack && + + # Trigger a lazy fetch for BLOB_FOO. The child fetch spawned + # by fetch_objects() receives our crafted thin pack. Its + # index-pack encounters the missing delta base (BLOB_BAR) and + # tries to lazy-fetch it via promisor_remote_get_direct(). + # + # With the fix: fetch_objects() propagates GIT_NO_LAZY_FETCH=1 + # to the child, so the depth-2 fetch is blocked and we see the + # "lazy fetching disabled" warning. The object cannot be + # resolved, so cat-file fails. + # + # Without the fix: the depth-2 fetch would proceed, potentially + # recursing unboundedly with a persistently misbehaving server. + test_must_fail git -C repo -c protocol.version=2 \ + cat-file -p $BLOB_FOO 2>err && + test_grep "lazy fetching disabled" err && + + # Ensure that the one-time-script was used. + ! test -e "$HTTPD_ROOT_PATH/one-time-script" +' + # DO NOT add non-httpd-specific tests here, because the last part of this # test script is only executed when httpd is available and enabled. From 386fe44951c3d0f8eaee98809aae5f2d886bac83 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:14 +0900 Subject: [PATCH 08/61] parseopt: extract subcommand handling from parse_options_step() 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> Signed-off-by: Junio C Hamano --- parse-options.c | 87 ++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 41 deletions(-) diff --git a/parse-options.c b/parse-options.c index c9cafc21b90355..02a4f00919f6d6 100644 --- a/parse-options.c +++ b/parse-options.c @@ -605,17 +605,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) @@ -990,38 +1017,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 */ From e0245a1169b2acddd94be4da02c426419507f0b5 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:15 +0900 Subject: [PATCH 09/61] help: make autocorrect handling reusable 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> Signed-off-by: Junio C Hamano --- 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 f3264d0a37cc50..6111631c2caaea 100644 --- a/Makefile +++ b/Makefile @@ -1098,6 +1098,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 00000000000000..97145d3a53ce07 --- /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 00000000000000..f5fadf9d96059b --- /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 95f576c5c81d9f..4acb6ca585ff8f 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 4b536e012481ca..0429e80a5c966c 100644 --- a/meson.build +++ b/meson.build @@ -283,6 +283,7 @@ libgit_sources = [ 'archive-zip.c', 'archive.c', 'attr.c', + 'autocorrect.c', 'base85.c', 'bisect.c', 'blame.c', From 916b96c0ec006216fcb9475fea37c9bc8e6b6505 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:16 +0900 Subject: [PATCH 10/61] help: move tty check for autocorrection to autocorrect.c 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> Signed-off-by: Junio C Hamano --- autocorrect.c | 24 ++++++++++++++++-------- help.c | 6 ------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/autocorrect.c b/autocorrect.c index 97145d3a53ce07..887d2396da44b9 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 4acb6ca585ff8f..983057970e7c7f 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); From a6e0ccbd38e4b274fa2360bc8a49d8049d1ded95 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:17 +0900 Subject: [PATCH 11/61] autocorrect: use mode and delay instead of magic numbers Drop magic numbers and describe autocorrect config with a mode enum and an integer delay. This reduces errors when mutating config values and makes the values easier to access. Signed-off-by: Jiamu Sun <39@barroit.sh> Signed-off-by: Junio C Hamano --- autocorrect.c | 46 +++++++++++++++++++++++----------------------- autocorrect.h | 20 ++++++++++++++------ help.c | 9 +++++---- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/autocorrect.c b/autocorrect.c index 887d2396da44b9..2484546fc731d9 100644 --- a/autocorrect.c +++ b/autocorrect.c @@ -6,7 +6,7 @@ #include "prompt.h" #include "gettext.h" -static int parse_autocorrect(const char *value) +static enum autocorrect_mode parse_autocorrect(const char *value) { switch (git_parse_maybe_bool_text(value)) { case 1: @@ -19,49 +19,49 @@ static int parse_autocorrect(const char *value) if (!strcmp(value, "prompt")) return AUTOCORRECT_PROMPT; - if (!strcmp(value, "never")) + else if (!strcmp(value, "never")) return AUTOCORRECT_NEVER; - if (!strcmp(value, "immediate")) + else if (!strcmp(value, "immediate")) return AUTOCORRECT_IMMEDIATELY; - if (!strcmp(value, "show")) + else if (!strcmp(value, "show")) return AUTOCORRECT_SHOW; - - return 0; + else + return AUTOCORRECT_DELAY; } void autocorrect_resolve_config(const char *var, const char *value, const struct config_context *ctx, void *data) { - int *out = data; - int parsed; + struct autocorrect *conf = data; if (strcmp(var, "help.autocorrect")) return; - parsed = parse_autocorrect(value); + conf->mode = parse_autocorrect(value); /* * Disable autocorrection prompt in a non-interactive session */ - if (parsed == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2))) - parsed = AUTOCORRECT_NEVER; + if (conf->mode == AUTOCORRECT_PROMPT && (!isatty(0) || !isatty(2))) + conf->mode = AUTOCORRECT_NEVER; - if (!parsed) { - parsed = git_config_int(var, value, ctx->kvi); - if (parsed < 0 || parsed == 1) - parsed = AUTOCORRECT_IMMEDIATELY; - } + if (conf->mode == AUTOCORRECT_DELAY) { + conf->delay = git_config_int(var, value, ctx->kvi); - *out = parsed; + if (!conf->delay) + conf->mode = AUTOCORRECT_SHOW; + else if (conf->delay < 0 || conf->delay == 1) + conf->mode = AUTOCORRECT_IMMEDIATELY; + } } -void autocorrect_confirm(int autocorrect, const char *assumed) +void autocorrect_confirm(struct autocorrect *conf, const char *assumed) { - if (autocorrect == AUTOCORRECT_IMMEDIATELY) { + if (conf->mode == AUTOCORRECT_IMMEDIATELY) { fprintf_ln(stderr, _("Continuing under the assumption that you meant '%s'."), assumed); - } else if (autocorrect == AUTOCORRECT_PROMPT) { + } else if (conf->mode == AUTOCORRECT_PROMPT) { char *answer; struct strbuf msg = STRBUF_INIT; @@ -71,10 +71,10 @@ void autocorrect_confirm(int autocorrect, const char *assumed) if (!(starts_with(answer, "y") || starts_with(answer, "Y"))) exit(1); - } else { + } else if (conf->mode == AUTOCORRECT_DELAY) { fprintf_ln(stderr, _("Continuing in %0.1f seconds, assuming that you meant '%s'."), - (float)autocorrect / 10.0, assumed); - sleep_millisec(autocorrect * 100); + conf->delay / 10.0, assumed); + sleep_millisec(conf->delay * 100); } } diff --git a/autocorrect.h b/autocorrect.h index f5fadf9d96059b..5506a36f11a7cc 100644 --- a/autocorrect.h +++ b/autocorrect.h @@ -1,16 +1,24 @@ #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; +enum autocorrect_mode { + AUTOCORRECT_SHOW, + AUTOCORRECT_NEVER, + AUTOCORRECT_PROMPT, + AUTOCORRECT_IMMEDIATELY, + AUTOCORRECT_DELAY, +}; + +struct autocorrect { + enum autocorrect_mode mode; + int delay; +}; + 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); +void autocorrect_confirm(struct autocorrect *conf, const char *assumed); #endif /* AUTOCORRECT_H */ diff --git a/help.c b/help.c index 983057970e7c7f..a89ac5aced9994 100644 --- a/help.c +++ b/help.c @@ -538,7 +538,7 @@ int is_in_cmdlist(struct cmdnames *c, const char *s) } struct help_unknown_cmd_config { - int autocorrect; + struct autocorrect autocorrect; struct cmdnames aliases; }; @@ -607,7 +607,7 @@ char *help_unknown_cmd(const char *cmd) read_early_config(the_repository, git_unknown_cmd_config, &cfg); - if (cfg.autocorrect == AUTOCORRECT_NEVER) { + if (cfg.autocorrect.mode == AUTOCORRECT_NEVER) { fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd); exit(1); } @@ -673,7 +673,8 @@ char *help_unknown_cmd(const char *cmd) n++) ; /* still counting */ } - if (cfg.autocorrect && cfg.autocorrect != AUTOCORRECT_SHOW && n == 1 && + + if (cfg.autocorrect.mode != AUTOCORRECT_SHOW && n == 1 && SIMILAR_ENOUGH(best_similarity)) { char *assumed = xstrdup(main_cmds.names[0]->name); @@ -682,7 +683,7 @@ char *help_unknown_cmd(const char *cmd) "which does not exist."), cmd); - autocorrect_confirm(cfg.autocorrect, assumed); + autocorrect_confirm(&cfg.autocorrect, assumed); cmdnames_release(&cfg.aliases); cmdnames_release(&main_cmds); From f06f1f043cbf58b82f1243fd09fc2c8b0202a853 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:18 +0900 Subject: [PATCH 12/61] autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINT AUTOCORRECT_SHOW is ambiguous. Its purpose is to show commands similar to the unknown one and take no other action. Rename it to fit the semantics. Signed-off-by: Jiamu Sun <39@barroit.sh> Signed-off-by: Junio C Hamano --- autocorrect.c | 6 +++--- autocorrect.h | 2 +- help.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/autocorrect.c b/autocorrect.c index 2484546fc731d9..de0fa282c934a8 100644 --- a/autocorrect.c +++ b/autocorrect.c @@ -12,7 +12,7 @@ static enum autocorrect_mode parse_autocorrect(const char *value) case 1: return AUTOCORRECT_IMMEDIATELY; case 0: - return AUTOCORRECT_SHOW; + return AUTOCORRECT_HINT; default: /* other random text */ break; } @@ -24,7 +24,7 @@ static enum autocorrect_mode parse_autocorrect(const char *value) else if (!strcmp(value, "immediate")) return AUTOCORRECT_IMMEDIATELY; else if (!strcmp(value, "show")) - return AUTOCORRECT_SHOW; + return AUTOCORRECT_HINT; else return AUTOCORRECT_DELAY; } @@ -49,7 +49,7 @@ void autocorrect_resolve_config(const char *var, const char *value, conf->delay = git_config_int(var, value, ctx->kvi); if (!conf->delay) - conf->mode = AUTOCORRECT_SHOW; + conf->mode = AUTOCORRECT_HINT; else if (conf->delay < 0 || conf->delay == 1) conf->mode = AUTOCORRECT_IMMEDIATELY; } diff --git a/autocorrect.h b/autocorrect.h index 5506a36f11a7cc..328807242c15ab 100644 --- a/autocorrect.h +++ b/autocorrect.h @@ -4,7 +4,7 @@ struct config_context; enum autocorrect_mode { - AUTOCORRECT_SHOW, + AUTOCORRECT_HINT, AUTOCORRECT_NEVER, AUTOCORRECT_PROMPT, AUTOCORRECT_IMMEDIATELY, diff --git a/help.c b/help.c index a89ac5aced9994..2d441ded3f1489 100644 --- a/help.c +++ b/help.c @@ -674,7 +674,7 @@ char *help_unknown_cmd(const char *cmd) ; /* still counting */ } - if (cfg.autocorrect.mode != AUTOCORRECT_SHOW && n == 1 && + if (cfg.autocorrect.mode != AUTOCORRECT_HINT && n == 1 && SIMILAR_ENOUGH(best_similarity)) { char *assumed = xstrdup(main_cmds.names[0]->name); From 7cd07f167d2980ad58de08a8bd7787d2ef5882b9 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:19 +0900 Subject: [PATCH 13/61] autocorrect: provide config resolution API Add autocorrect_resolve(). This resolves and populates the correct values for autocorrect config. Make autocorrect config callback internal. The API is meant to provide a high-level way to retrieve the config. Allowing access to the config callback from outside violates that intent. Additionally, in some cases, without access to the config callback, two config iterations cannot be merged into one, which can hurt performance. This is fine, as the code path that calls autocorrect_resolve() is cold. Signed-off-by: Jiamu Sun <39@barroit.sh> Signed-off-by: Junio C Hamano --- autocorrect.c | 15 ++++++++++++--- autocorrect.h | 5 +---- help.c | 40 +++++++++++++++++----------------------- 3 files changed, 30 insertions(+), 30 deletions(-) diff --git a/autocorrect.c b/autocorrect.c index de0fa282c934a8..b2ee9f51e8c09a 100644 --- a/autocorrect.c +++ b/autocorrect.c @@ -1,3 +1,5 @@ +#define USE_THE_REPOSITORY_VARIABLE + #include "git-compat-util.h" #include "autocorrect.h" #include "config.h" @@ -29,13 +31,13 @@ static enum autocorrect_mode parse_autocorrect(const char *value) return AUTOCORRECT_DELAY; } -void autocorrect_resolve_config(const char *var, const char *value, - const struct config_context *ctx, void *data) +static int resolve_autocorrect(const char *var, const char *value, + const struct config_context *ctx, void *data) { struct autocorrect *conf = data; if (strcmp(var, "help.autocorrect")) - return; + return 0; conf->mode = parse_autocorrect(value); @@ -53,6 +55,13 @@ void autocorrect_resolve_config(const char *var, const char *value, else if (conf->delay < 0 || conf->delay == 1) conf->mode = AUTOCORRECT_IMMEDIATELY; } + + return 0; +} + +void autocorrect_resolve(struct autocorrect *conf) +{ + read_early_config(the_repository, resolve_autocorrect, conf); } void autocorrect_confirm(struct autocorrect *conf, const char *assumed) diff --git a/autocorrect.h b/autocorrect.h index 328807242c15ab..0d3e819262edee 100644 --- a/autocorrect.h +++ b/autocorrect.h @@ -1,8 +1,6 @@ #ifndef AUTOCORRECT_H #define AUTOCORRECT_H -struct config_context; - enum autocorrect_mode { AUTOCORRECT_HINT, AUTOCORRECT_NEVER, @@ -16,8 +14,7 @@ struct autocorrect { int delay; }; -void autocorrect_resolve_config(const char *var, const char *value, - const struct config_context *ctx, void *data); +void autocorrect_resolve(struct autocorrect *conf); void autocorrect_confirm(struct autocorrect *conf, const char *assumed); diff --git a/help.c b/help.c index 2d441ded3f1489..81efdb13d4a375 100644 --- a/help.c +++ b/help.c @@ -537,32 +537,23 @@ int is_in_cmdlist(struct cmdnames *c, const char *s) return 0; } -struct help_unknown_cmd_config { - struct autocorrect autocorrect; - struct cmdnames aliases; -}; - -static int git_unknown_cmd_config(const char *var, const char *value, - const struct config_context *ctx, - void *cb) +static int resolve_aliases(const char *var, const char *value UNUSED, + const struct config_context *ctx UNUSED, void *data) { - struct help_unknown_cmd_config *cfg = cb; + struct cmdnames *aliases = data; const char *subsection, *key; size_t subsection_len; - autocorrect_resolve_config(var, value, ctx, &cfg->autocorrect); - - /* Also use aliases for command lookup */ if (!parse_config_key(var, "alias", &subsection, &subsection_len, &key)) { if (subsection) { /* [alias "name"] command = value */ if (!strcmp(key, "command")) - add_cmdname(&cfg->aliases, subsection, + add_cmdname(aliases, subsection, subsection_len); } else { /* alias.name = value */ - add_cmdname(&cfg->aliases, key, strlen(key)); + add_cmdname(aliases, key, strlen(key)); } } @@ -599,22 +590,26 @@ static const char bad_interpreter_advice[] = char *help_unknown_cmd(const char *cmd) { - struct help_unknown_cmd_config cfg = { 0 }; + struct cmdnames aliases = { 0 }; + struct autocorrect autocorrect = { 0 }; int i, n, best_similarity = 0; struct cmdnames main_cmds = { 0 }; struct cmdnames other_cmds = { 0 }; struct cmdname_help *common_cmds; - read_early_config(the_repository, git_unknown_cmd_config, &cfg); + autocorrect_resolve(&autocorrect); - if (cfg.autocorrect.mode == AUTOCORRECT_NEVER) { + if (autocorrect.mode == AUTOCORRECT_NEVER) { fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd); exit(1); } load_command_list("git-", &main_cmds, &other_cmds); - add_cmd_list(&main_cmds, &cfg.aliases); + /* Also use aliases for command lookup */ + read_early_config(the_repository, resolve_aliases, &aliases); + + add_cmd_list(&main_cmds, &aliases); add_cmd_list(&main_cmds, &other_cmds); QSORT(main_cmds.names, main_cmds.cnt, cmdname_compare); uniq(&main_cmds); @@ -674,18 +669,17 @@ char *help_unknown_cmd(const char *cmd) ; /* still counting */ } - if (cfg.autocorrect.mode != AUTOCORRECT_HINT && n == 1 && + if (autocorrect.mode != AUTOCORRECT_HINT && n == 1 && SIMILAR_ENOUGH(best_similarity)) { char *assumed = xstrdup(main_cmds.names[0]->name); fprintf_ln(stderr, - _("WARNING: You called a Git command named '%s', " - "which does not exist."), + _("WARNING: You called a Git command named '%s', which does not exist."), cmd); - autocorrect_confirm(&cfg.autocorrect, assumed); + autocorrect_confirm(&autocorrect, assumed); - cmdnames_release(&cfg.aliases); + cmdnames_release(&aliases); cmdnames_release(&main_cmds); cmdnames_release(&other_cmds); return assumed; From be9df6de6e2cfd09ffc327e9d4edd451248296f9 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:20 +0900 Subject: [PATCH 14/61] parseopt: autocorrect mistyped subcommands Try to autocorrect the mistyped mandatory subcommand before showing an error and exiting. Subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL are skipped. Use standard Damerau-Levenshtein distance (weights 1, 1, 1, 1) to establish a predictable, mathematically sound baseline. Scale the allowed edit distance based on input length to prevent false positives on short commands, following common practice for fuzziness thresholds (e.g., Elasticsearch's AUTO fuzziness): - Length 0-2: 0 edits allowed - Length 3-5: 1 edit allowed - Length 6+: 2 edits allowed Signed-off-by: Jiamu Sun <39@barroit.sh> Signed-off-by: Junio C Hamano --- parse-options.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/parse-options.c b/parse-options.c index 02a4f00919f6d6..1f1b72762790c0 100644 --- a/parse-options.c +++ b/parse-options.c @@ -6,6 +6,8 @@ #include "strbuf.h" #include "string-list.h" #include "utf8.h" +#include "autocorrect.h" +#include "levenshtein.h" static int disallow_abbreviated_options; @@ -622,13 +624,77 @@ static int parse_subcommand(const char *arg, const struct option *options) return -1; } +static void find_subcommands(struct string_list *list, + const struct option *options) +{ + for (; options->type != OPTION_END; options++) { + if (options->type == OPTION_SUBCOMMAND) + string_list_append(list, options->long_name); + } +} + +static int similar_enough(const char *cmd, unsigned int edit) +{ + size_t len = strlen(cmd); + unsigned int allowed = len < 3 ? 0 : len < 6 ? 1 : 2; + + return edit <= allowed; +} + +static const char *autocorrect_subcommand(const char *cmd, + struct string_list *cmds) +{ + struct autocorrect autocorrect = { 0 }; + unsigned int min = UINT_MAX; + unsigned int ties = 0; + struct string_list_item *cand; + struct string_list_item *best = NULL; + + autocorrect_resolve(&autocorrect); + + /* + * Builtin subcommands are small enough that printing them all via + * usage_with_options() is sufficient. Therefore, AUTOCORRECT_HINT + * acts like AUTOCORRECT_NEVER. + */ + if (autocorrect.mode == AUTOCORRECT_HINT || + autocorrect.mode == AUTOCORRECT_NEVER) + return NULL; + + for_each_string_list_item(cand, cmds) { + unsigned int edit = levenshtein(cmd, cand->string, 1, 1, 1, 1); + + if (edit < min) { + min = edit; + best = cand; + ties = 0; + } else if (edit == min) { + ties++; + } + } + + if (!ties && similar_enough(cmd, min)) { + fprintf_ln(stderr, + _("WARNING: You called a subcommand named '%s', which does not exist."), + cmd); + + autocorrect_confirm(&autocorrect, best->string); + return best->string; + } + + return NULL; +} + 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); + int err; + const char *assumed; + struct string_list cmds = STRING_LIST_INIT_NODUP; + err = parse_subcommand(arg, options); if (!err) return PARSE_OPT_SUBCOMMAND; @@ -641,8 +707,17 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx, if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL) return PARSE_OPT_DONE; - error(_("unknown subcommand: `%s'"), arg); - usage_with_options(usagestr, options); + find_subcommands(&cmds, options); + assumed = autocorrect_subcommand(arg, &cmds); + + if (!assumed) { + error(_("unknown subcommand: `%s'"), arg); + usage_with_options(usagestr, options); + } + + string_list_clear(&cmds, 0); + parse_subcommand(assumed, options); + return PARSE_OPT_SUBCOMMAND; } static void check_typos(const char *arg, const struct option *options) From ae8b7e1d200977165755c2e6d5a22e1df8ab6bf1 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:21 +0900 Subject: [PATCH 15/61] parseopt: enable subcommand autocorrection for git-remote and git-notes Add PARSE_OPT_SUBCOMMAND_AUTOCORR to enable autocorrection for subcommands parsed with PARSE_OPT_SUBCOMMAND_OPTIONAL. Use it for git-remote and git-notes, so mistyped subcommands can be automatically corrected, and builtin entry points no longer need to handle the unknown subcommand error path themselves. This is safe for these two builtins, because they either resolve to a single subcommand or take no subcommand at all. This means that if the subcommand parser encounters an unknown argument, it must be a mistyped subcommand. Signed-off-by: Jiamu Sun <39@barroit.sh> Signed-off-by: Junio C Hamano --- builtin/notes.c | 10 +++------- builtin/remote.c | 12 ++++-------- parse-options.c | 16 +++++++++------- parse-options.h | 1 + 4 files changed, 17 insertions(+), 22 deletions(-) diff --git a/builtin/notes.c b/builtin/notes.c index 9af602bdd7b402..087eb898a4415f 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -1149,14 +1149,10 @@ int cmd_notes(int argc, repo_config(the_repository, git_default_config, NULL); argc = parse_options(argc, argv, prefix, options, git_notes_usage, - PARSE_OPT_SUBCOMMAND_OPTIONAL); - if (!fn) { - if (argc) { - error(_("unknown subcommand: `%s'"), argv[0]); - usage_with_options(git_notes_usage, options); - } + PARSE_OPT_SUBCOMMAND_OPTIONAL | + PARSE_OPT_SUBCOMMAND_AUTOCORR); + if (!fn) fn = list; - } if (override_notes_ref) { struct strbuf sb = STRBUF_INIT; diff --git a/builtin/remote.c b/builtin/remote.c index 0fddaa177331f6..9415f6cb03e807 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1953,15 +1953,11 @@ int cmd_remote(int argc, }; argc = parse_options(argc, argv, prefix, options, builtin_remote_usage, - PARSE_OPT_SUBCOMMAND_OPTIONAL); + PARSE_OPT_SUBCOMMAND_OPTIONAL | + PARSE_OPT_SUBCOMMAND_AUTOCORR); - if (fn) { + if (fn) return !!fn(argc, argv, prefix, repo); - } else { - if (argc) { - error(_("unknown subcommand: `%s'"), argv[0]); - usage_with_options(builtin_remote_usage, options); - } + else return !!show_all(); - } } diff --git a/parse-options.c b/parse-options.c index 1f1b72762790c0..0b84061a381153 100644 --- a/parse-options.c +++ b/parse-options.c @@ -698,14 +698,16 @@ static enum parse_opt_result handle_subcommand(struct parse_opt_ctx_t *ctx, 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) + if (ctx->flags & PARSE_OPT_SUBCOMMAND_OPTIONAL && + !(ctx->flags & PARSE_OPT_SUBCOMMAND_AUTOCORR)) { + /* + * 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; + } find_subcommands(&cmds, options); assumed = autocorrect_subcommand(arg, &cmds); diff --git a/parse-options.h b/parse-options.h index 706de9729f6b3f..f29ac337893c92 100644 --- a/parse-options.h +++ b/parse-options.h @@ -40,6 +40,7 @@ enum parse_opt_flags { PARSE_OPT_ONE_SHOT = 1 << 5, PARSE_OPT_SHELL_EVAL = 1 << 6, PARSE_OPT_SUBCOMMAND_OPTIONAL = 1 << 7, + PARSE_OPT_SUBCOMMAND_AUTOCORR = 1 << 8, }; enum parse_opt_option_flags { From 273faabea8db47247564df092adf5a675ddfa284 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:22 +0900 Subject: [PATCH 16/61] parseopt: add tests for subcommand autocorrection These tests cover default behavior (help.autocorrect is unset), no correction, immediate correction, delayed correction, and rejection when the typo is too dissimilar. Signed-off-by: Jiamu Sun <39@barroit.sh> Signed-off-by: Junio C Hamano --- t/meson.build | 1 + t/t9004-autocorrect-subcommand.sh | 51 +++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100755 t/t9004-autocorrect-subcommand.sh diff --git a/t/meson.build b/t/meson.build index f66a73f8a07d93..bf0503d705a9a3 100644 --- a/t/meson.build +++ b/t/meson.build @@ -973,6 +973,7 @@ integration_tests = [ 't9001-send-email.sh', 't9002-column.sh', 't9003-help-autocorrect.sh', + 't9004-autocorrect-subcommand.sh', 't9100-git-svn-basic.sh', 't9101-git-svn-props.sh', 't9102-git-svn-deep-rmdir.sh', diff --git a/t/t9004-autocorrect-subcommand.sh b/t/t9004-autocorrect-subcommand.sh new file mode 100755 index 00000000000000..d10031659b940b --- /dev/null +++ b/t/t9004-autocorrect-subcommand.sh @@ -0,0 +1,51 @@ +#!/bin/sh + +test_description='subcommand auto-correction test + +Test autocorrection for subcommands with different +help.autocorrect mode.' + +. ./test-lib.sh + +test_expect_success 'setup' " + echo '^error: unknown subcommand: ' >grep_unknown +" + +test_expect_success 'default is not to autocorrect' ' + test_must_fail git worktree lsit 2>actual && + head -n1 actual >first && test_grep -f grep_unknown first +' + +for mode in false no off 0 show never +do + test_expect_success "'$mode' disables autocorrection" " + test_config help.autocorrect $mode && + + test_must_fail git worktree lsit 2>actual && + head -n1 actual >first && test_grep -f grep_unknown first + " +done + +for mode in -39 immediate 1 +do + test_expect_success "autocorrect immediately with '$mode'" - <<-EOT + test_config help.autocorrect $mode && + + git worktree lsit 2>actual && + test_grep "you meant 'list'\.$" actual + EOT +done + +test_expect_success 'delay path is executed' - <<-\EOT + test_config help.autocorrect 2 && + + git worktree lsit 2>actual && + test_grep '^Continuing in 0.2 seconds, ' actual +EOT + +test_expect_success 'deny if too dissimilar' - <<-\EOT + test_must_fail git remote rensnr 2>actual && + head -n1 actual >first && test_grep -f grep_unknown first +EOT + +test_done From 916b45080534e9603094dda39c4599a2f2a466d1 Mon Sep 17 00:00:00 2001 From: Jiamu Sun <39@barroit.sh> Date: Tue, 17 Mar 2026 00:36:23 +0900 Subject: [PATCH 17/61] doc: document autocorrect API Explain behaviors for autocorrect_resolve(), autocorrect_confirm(), and struct autocorrect. Signed-off-by: Jiamu Sun <39@barroit.sh> Signed-off-by: Junio C Hamano --- autocorrect.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/autocorrect.h b/autocorrect.h index 0d3e819262edee..bfa3ba20a4fb73 100644 --- a/autocorrect.h +++ b/autocorrect.h @@ -9,13 +9,24 @@ enum autocorrect_mode { AUTOCORRECT_DELAY, }; +/** + * `mode` indicates which action will be performed by autocorrect_confirm(). + * `delay` is the timeout before autocorrect_confirm() returns, in tenths of a + * second. Use it only with AUTOCORRECT_DELAY. + */ struct autocorrect { enum autocorrect_mode mode; int delay; }; +/** + * Resolve the autocorrect configuration into `conf`. + */ void autocorrect_resolve(struct autocorrect *conf); +/** + * Interact with the user in different ways depending on `conf->mode`. + */ void autocorrect_confirm(struct autocorrect *conf, const char *assumed); #endif /* AUTOCORRECT_H */ From ee832e326985499ca3f17855f69dca22a4d41406 Mon Sep 17 00:00:00 2001 From: Usman Akinyemi Date: Thu, 26 Mar 2026 00:39:05 +0530 Subject: [PATCH 18/61] remote: move remote group resolution to remote.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `get_remote_group`, `add_remote_or_group`, and the `remote_group_data` struct are currently defined as static helpers inside builtin/fetch.c. They implement generic remote group resolution that is not specific to fetch — they parse `remotes.` config entries and resolve a name to either a list of group members or a single configured remote. Move them to remote.c and declare them in remote.h so that other builtins can use the same logic without duplication. Useful for the next patch. Suggested-by: Junio C Hamano Signed-off-by: Usman Akinyemi Signed-off-by: Junio C Hamano --- builtin/fetch.c | 42 ------------------------------------------ remote.c | 37 +++++++++++++++++++++++++++++++++++++ remote.h | 12 ++++++++++++ 3 files changed, 49 insertions(+), 42 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 8a36cf67b5f140..966cc58f730150 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2138,48 +2138,6 @@ static int get_one_remote_for_fetch(struct remote *remote, void *priv) return 0; } -struct remote_group_data { - const char *name; - struct string_list *list; -}; - -static int get_remote_group(const char *key, const char *value, - const struct config_context *ctx UNUSED, - void *priv) -{ - struct remote_group_data *g = priv; - - if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) { - /* split list by white space */ - while (*value) { - size_t wordlen = strcspn(value, " \t\n"); - - if (wordlen >= 1) - string_list_append_nodup(g->list, - xstrndup(value, wordlen)); - value += wordlen + (value[wordlen] != '\0'); - } - } - - return 0; -} - -static int add_remote_or_group(const char *name, struct string_list *list) -{ - int prev_nr = list->nr; - struct remote_group_data g; - g.name = name; g.list = list; - - repo_config(the_repository, get_remote_group, &g); - if (list->nr == prev_nr) { - struct remote *remote = remote_get(name); - if (!remote_is_configured(remote, 0)) - return 0; - string_list_append(list, remote->name); - } - return 1; -} - static void add_options_to_argv(struct strvec *argv, const struct fetch_config *config) { diff --git a/remote.c b/remote.c index 7ca2a6501b4920..3d62384792c323 100644 --- a/remote.c +++ b/remote.c @@ -2114,6 +2114,43 @@ int get_fetch_map(const struct ref *remote_refs, return 0; } +int get_remote_group(const char *key, const char *value, + const struct config_context *ctx UNUSED, + void *priv) +{ + struct remote_group_data *g = priv; + + if (skip_prefix(key, "remotes.", &key) && !strcmp(key, g->name)) { + /* split list by white space */ + while (*value) { + size_t wordlen = strcspn(value, " \t\n"); + + if (wordlen >= 1) + string_list_append_nodup(g->list, + xstrndup(value, wordlen)); + value += wordlen + (value[wordlen] != '\0'); + } + } + + return 0; +} + +int add_remote_or_group(const char *name, struct string_list *list) +{ + int prev_nr = list->nr; + struct remote_group_data g; + g.name = name; g.list = list; + + repo_config(the_repository, get_remote_group, &g); + if (list->nr == prev_nr) { + struct remote *remote = remote_get(name); + if (!remote_is_configured(remote, 0)) + return 0; + string_list_append(list, remote->name); + } + return 1; +} + int resolve_remote_symref(struct ref *ref, struct ref *list) { if (!ref->symref) diff --git a/remote.h b/remote.h index fc052945ee451d..8ff2bd88fa1b29 100644 --- a/remote.h +++ b/remote.h @@ -347,6 +347,18 @@ int branch_has_merge_config(struct branch *branch); int branch_merge_matches(struct branch *, int n, const char *); +/* list of the remote in a group as configured */ +struct remote_group_data { + const char *name; + struct string_list *list; +}; + +int get_remote_group(const char *key, const char *value, + const struct config_context *ctx, + void *priv); + +int add_remote_or_group(const char *name, struct string_list *list); + /** * Return the fully-qualified refname of the tracking branch for `branch`. * I.e., what "branch@{upstream}" would give you. Returns NULL if no From aac9bf095f1a3f1e7cdc74c6d4e537f671b93a5f Mon Sep 17 00:00:00 2001 From: Usman Akinyemi Date: Thu, 26 Mar 2026 00:39:06 +0530 Subject: [PATCH 19/61] push: support pushing to a remote group MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git fetch` accepts a remote group name (configured via `remotes.` in config) and fetches from each member remote. `git push` has no equivalent — it only accepts a single remote name. Teach `git push` to resolve its repository argument through `add_remote_or_group()`, which was made public in the previous patch, so that a user can push to all remotes in a group with: git push When the argument resolves to a single remote, the behaviour is identical to before. When it resolves to a group, each member remote is pushed in sequence. The group push path rebuilds the refspec list (`rs`) from scratch for each member remote so that per-remote push mappings configured via `remote..push` are resolved correctly against each specific remote. Without this, refspec entries would accumulate across iterations and each subsequent remote would receive a growing list of duplicated entries. Mirror detection (`remote->mirror`) is also evaluated per remote using a copy of the flags, so that a mirror remote in the group cannot set TRANSPORT_PUSH_FORCE on subsequent non-mirror remotes in the same group. Suggested-by: Junio C Hamano Signed-off-by: Usman Akinyemi Signed-off-by: Junio C Hamano --- Documentation/git-push.adoc | 73 ++++++++++++++++-- builtin/push.c | 123 +++++++++++++++++++++-------- t/meson.build | 1 + t/t5566-push-group.sh | 150 ++++++++++++++++++++++++++++++++++++ 4 files changed, 306 insertions(+), 41 deletions(-) create mode 100755 t/t5566-push-group.sh diff --git a/Documentation/git-push.adoc b/Documentation/git-push.adoc index e5ba3a67421edc..b7f617a290592b 100644 --- a/Documentation/git-push.adoc +++ b/Documentation/git-push.adoc @@ -18,17 +18,28 @@ git push [--all | --branches | --mirror | --tags] [--follow-tags] [--atomic] [-n DESCRIPTION ----------- - -Updates one or more branches, tags, or other references in a remote -repository from your local repository, and sends all necessary data -that isn't already on the remote. +Updates one or more branches, tags, or other references in one or more +remote repositories from your local repository, and sends all necessary +data that isn't already on the remote. The simplest way to push is `git push `. `git push origin main` will push the local `main` branch to the `main` branch on the remote named `origin`. -The `` argument defaults to the upstream for the current branch, -or `origin` if there's no configured upstream. +You can also push to multiple remotes at once by using a remote group. +A remote group is a named list of remotes configured via `remotes.` +in your git config: + + $ git config remotes.all-remotes "origin gitlab backup" + +Then `git push all-remotes` will push to `origin`, `gitlab`, and +`backup` in turn, as if you had run `git push` against each one +individually. Each remote is pushed independently using its own +push mapping configuration. There is a `remotes.` entry in +the configuration file. (See linkgit:git-config[1]). + +The `` argument defaults to the upstream for the current +branch, or `origin` if there's no configured upstream. To decide which branches, tags, or other refs to push, Git uses (in order of precedence): @@ -55,8 +66,10 @@ OPTIONS __:: The "remote" repository that is the destination of a push operation. This parameter can be either a URL - (see the section <> below) or the name - of a remote (see the section <> below). + (see the section <> below), the name + of a remote (see the section <> below), + or the name of a remote group + (see the section <> below). `...`:: Specify what destination ref to update with what source object. @@ -430,6 +443,50 @@ further recursion will occur. In this case, `only` is treated as `on-demand`. include::urls-remotes.adoc[] +[[REMOTE-GROUPS]] +REMOTE GROUPS +------------- + +A remote group is a named list of remotes configured via `remotes.` +in your git config: + + $ git config remotes.all-remotes "r1 r2 r3" + +When a group name is given as the `` argument, the push is +performed to each member remote in turn. The defining principle is: + + git push all-remotes + +is exactly equivalent to: + + git push r1 + git push r2 + ... + git push rN + +where r1, r2, ..., rN are the members of `all-remotes`. No special +behaviour is added or removed — the group is purely a shorthand for +running the same push command against each member remote individually. + +The behaviour upon failure depends on the kind of error encountered: + +If a member remote rejects the push, for example due to a +non-fast-forward update, force needed but not given, an existing tag, +or a server-side hook refusing a ref, Git reports the error and continues +pushing to the remaining remotes in the group. The overall exit code is +non-zero if any member push fails. + +If a member remote cannot be contacted at all, for example because the +repository does not exist, authentication fails, or the network is +unreachable, the push stops at that point and the remaining remotes +are not attempted. + +This means the user is responsible for ensuring that the sequence of +individual pushes makes sense. If `git push r1`` would fail for a given +set of options and arguments, then `git push all-remotes` will fail in +the same way when it reaches r1. The group push does not do anything +special to make a failing individual push succeed. + OUTPUT ------ diff --git a/builtin/push.c b/builtin/push.c index 7100ffba5da17e..ed292c48fc45e6 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -552,12 +552,13 @@ int cmd_push(int argc, int flags = 0; int tags = 0; int push_cert = -1; - int rc; + int rc = 0; + int base_flags; const char *repo = NULL; /* default repository */ struct string_list push_options_cmdline = STRING_LIST_INIT_DUP; + struct string_list remote_group = STRING_LIST_INIT_DUP; struct string_list *push_options; const struct string_list_item *item; - struct remote *remote; struct option options[] = { OPT__VERBOSITY(&verbosity), @@ -620,39 +621,45 @@ int cmd_push(int argc, else if (recurse_submodules == RECURSE_SUBMODULES_ONLY) flags |= TRANSPORT_RECURSE_SUBMODULES_ONLY; - if (tags) - refspec_append(&rs, "refs/tags/*"); - if (argc > 0) repo = argv[0]; - remote = pushremote_get(repo); - if (!remote) { - if (repo) - die(_("bad repository '%s'"), repo); - die(_("No configured push destination.\n" - "Either specify the URL from the command-line or configure a remote repository using\n" - "\n" - " git remote add \n" - "\n" - "and then push using the remote name\n" - "\n" - " git push \n")); - } - - if (argc > 0) - set_refspecs(argv + 1, argc - 1, remote); - - if (remote->mirror) - flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE); - - if (flags & TRANSPORT_PUSH_ALL) { - if (argc >= 2) - die(_("--all can't be combined with refspecs")); - } - if (flags & TRANSPORT_PUSH_MIRROR) { - if (argc >= 2) - die(_("--mirror can't be combined with refspecs")); + if (repo) { + if (!add_remote_or_group(repo, &remote_group)) { + /* + * Not a configured remote name or group name. + * Try treating it as a direct URL or path, e.g. + * git push /tmp/foo.git + * git push https://github.com/user/repo.git + * pushremote_get() creates an anonymous remote + * from the URL so the loop below can handle it + * identically to a named remote. + */ + struct remote *r = pushremote_get(repo); + if (!r) + die(_("bad repository '%s'"), repo); + string_list_append(&remote_group, r->name); + } + } else { + struct remote *r = pushremote_get(NULL); + if (!r) + die(_("No configured push destination.\n" + "Either specify the URL from the command-line or configure a remote repository using\n" + "\n" + " git remote add \n" + "\n" + "and then push using the remote name\n" + "\n" + " git push \n" + "\n" + "To push to multiple remotes at once, configure a remote group using\n" + "\n" + " git config remotes. \" \"\n" + "\n" + "and then push using the group name\n" + "\n" + " git push \n")); + string_list_append(&remote_group, r->name); } if (!is_empty_cas(&cas) && (flags & TRANSPORT_PUSH_FORCE_IF_INCLUDES)) @@ -662,10 +669,60 @@ int cmd_push(int argc, if (strchr(item->string, '\n')) die(_("push options must not have new line characters")); - rc = do_push(flags, push_options, remote); + /* + * Push to each remote in remote_group. For a plain "git push " + * or a default push, remote_group has exactly one entry and the loop + * runs once — there is nothing structurally special about that case. + * For a group, the loop runs once per member remote. + * + * Mirror detection and the --mirror/--all + refspec conflict checks + * are done per remote inside the loop. A remote configured with + * remote.NAME.mirror=true implies mirror mode for that remote only — + * other non-mirror remotes in the same group are unaffected. + * + * rs is rebuilt from scratch for each remote so that per-remote push + * mappings (remote.NAME.push config) are resolved against the correct + * remote. iter_flags is derived from a clean snapshot of flags taken + * before the loop so that a mirror remote cannot bleed + * TRANSPORT_PUSH_FORCE into subsequent non-mirror remotes in the + * same group. + */ + base_flags = flags; + for (size_t i = 0; i < remote_group.nr; i++) { + int iter_flags = base_flags; + struct remote *r = pushremote_get(remote_group.items[i].string); + if (!r) + die(_("no such remote or remote group: %s"), + remote_group.items[i].string); + + if (r->mirror) + iter_flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE); + + if (iter_flags & TRANSPORT_PUSH_ALL) { + if (argc >= 2) + die(_("--all can't be combined with refspecs")); + } + if (iter_flags & TRANSPORT_PUSH_MIRROR) { + if (argc >= 2) + die(_("--mirror can't be combined with refspecs")); + } + + refspec_clear(&rs); + rs = (struct refspec) REFSPEC_INIT_PUSH; + + if (tags) + refspec_append(&rs, "refs/tags/*"); + if (argc > 0) + set_refspecs(argv + 1, argc - 1, r); + + rc |= do_push(iter_flags, push_options, r); + } + string_list_clear(&push_options_cmdline, 0); string_list_clear(&push_options_config, 0); + string_list_clear(&remote_group, 0); clear_cas_option(&cas); + if (rc == -1) usage_with_options(push_usage, options); else diff --git a/t/meson.build b/t/meson.build index 9b2fa4dee807d6..215df033e07e32 100644 --- a/t/meson.build +++ b/t/meson.build @@ -700,6 +700,7 @@ integration_tests = [ 't5563-simple-http-auth.sh', 't5564-http-proxy.sh', 't5565-push-multiple.sh', + 't5566-push-group.sh', 't5570-git-daemon.sh', 't5571-pre-push-hook.sh', 't5572-pull-submodule.sh', diff --git a/t/t5566-push-group.sh b/t/t5566-push-group.sh new file mode 100755 index 00000000000000..b9962946c7656b --- /dev/null +++ b/t/t5566-push-group.sh @@ -0,0 +1,150 @@ +#!/bin/sh + +test_description='push to remote group' + +. ./test-lib.sh + +test_expect_success 'setup' ' + for i in 1 2 3 + do + git init --bare dest-$i.git && + git -C dest-$i.git symbolic-ref HEAD refs/heads/not-a-branch || + return 1 + done && + test_tick && + git commit --allow-empty -m "initial" && + git config set remote.remote-1.url "file://$(pwd)/dest-1.git" && + git config set remote.remote-1.fetch "+refs/heads/*:refs/remotes/remote-1/*" && + git config set remote.remote-2.url "file://$(pwd)/dest-2.git" && + git config set remote.remote-2.fetch "+refs/heads/*:refs/remotes/remote-2/*" && + git config set remote.remote-3.url "file://$(pwd)/dest-3.git" && + git config set remote.remote-3.fetch "+refs/heads/*:refs/remotes/remote-3/*" && + git config set remotes.all-remotes "remote-1 remote-2 remote-3" +' + +test_expect_success 'push to remote group updates all members correctly' ' + git push all-remotes HEAD:refs/heads/main && + git rev-parse HEAD >expect && + for i in 1 2 3 + do + git -C dest-$i.git rev-parse refs/heads/main >actual || + return 1 + test_cmp expect actual || return 1 + done +' + +test_expect_success 'push second commit to group updates all members' ' + test_tick && + git commit --allow-empty -m "second" && + git push all-remotes HEAD:refs/heads/main && + git rev-parse HEAD >expect && + for i in 1 2 3 + do + git -C dest-$i.git rev-parse refs/heads/main >actual || + return 1 + test_cmp expect actual || return 1 + done +' + +test_expect_success 'push to single remote in group does not affect others' ' + test_tick && + git commit --allow-empty -m "third" && + git push remote-1 HEAD:refs/heads/main && + git -C dest-1.git rev-parse refs/heads/main >hash-after-1 && + git -C dest-2.git rev-parse refs/heads/main >hash-after-2 && + ! test_cmp hash-after-1 hash-after-2 +' + +test_expect_success 'mirror remote in group with refspec fails' ' + git config set remote.remote-1.mirror true && + test_must_fail git push all-remotes HEAD:refs/heads/main 2>err && + test_grep "mirror" err && + git config unset remote.remote-1.mirror +' +test_expect_success 'push.default=current works with group push' ' + git config set push.default current && + test_tick && + git commit --allow-empty -m "fifth" && + git push all-remotes && + git config unset push.default +' + +test_expect_success 'push continues past rejection to remaining remotes' ' + for i in c1 c2 c3 + do + git init --bare dest-$i.git || return 1 + done && + git config set remote.c1.url "file://$(pwd)/dest-c1.git" && + git config set remote.c2.url "file://$(pwd)/dest-c2.git" && + git config set remote.c3.url "file://$(pwd)/dest-c3.git" && + git config set remotes.continue-group "c1 c2 c3" && + + test_tick && + git commit --allow-empty -m "base for continue test" && + + # initial sync + git push continue-group HEAD:refs/heads/main && + + # advance c2 independently + git clone dest-c2.git tmp-c2 && + ( + cd tmp-c2 && + git checkout -b main origin/main && + test_commit c2_independent && + git push origin HEAD:refs/heads/main + ) && + rm -rf tmp-c2 && + + test_tick && + git commit --allow-empty -m "local diverging commit" && + + # push: c2 rejects, others succeed + test_must_fail git push continue-group HEAD:refs/heads/main && + + git rev-parse HEAD >expect && + git -C dest-c1.git rev-parse refs/heads/main >actual-c1 && + git -C dest-c3.git rev-parse refs/heads/main >actual-c3 && + test_cmp expect actual-c1 && + test_cmp expect actual-c3 && + + # c2 should not have the new commit + git -C dest-c2.git rev-parse refs/heads/main >actual-c2 && + ! test_cmp expect actual-c2 +' + +test_expect_success 'fatal connection error stops remaining remotes' ' + for i in f1 f2 f3 + do + git init --bare dest-$i.git || return 1 + done && + git config set remote.f1.url "file://$(pwd)/dest-f1.git" && + git config set remote.f2.url "file://$(pwd)/dest-f2.git" && + git config set remote.f3.url "file://$(pwd)/dest-f3.git" && + git config set remotes.fatal-group "f1 f2 f3" && + + test_tick && + git commit --allow-empty -m "base for fatal test" && + + # initial sync + git push fatal-group HEAD:refs/heads/main && + + # break f2 + git config set remote.f2.url "file:///tmp/does-not-exist-$$" && + + test_tick && + git commit --allow-empty -m "after fatal setup" && + + test_must_fail git push fatal-group HEAD:refs/heads/main && + + git rev-parse HEAD >expect && + git -C dest-f1.git rev-parse refs/heads/main >actual-f1 && + test_cmp expect actual-f1 && + + # f3 should not be updated + git -C dest-f3.git rev-parse refs/heads/main >actual-f3 && + ! test_cmp expect actual-f3 && + + git config set remote.f2.url "file://$(pwd)/dest-f2.git" +' + +test_done From 5f82f4be85231b3ea099fb56af6984bd8afced48 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 27 Mar 2026 15:18:50 -0700 Subject: [PATCH 20/61] SQUASH??? - futureproof against the attack of the "main" --- t/t5566-push-group.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t5566-push-group.sh b/t/t5566-push-group.sh index b9962946c7656b..32b8c82cea23a6 100755 --- a/t/t5566-push-group.sh +++ b/t/t5566-push-group.sh @@ -2,6 +2,9 @@ test_description='push to remote group' +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=default +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + . ./test-lib.sh test_expect_success 'setup' ' From c878e03212c744de9da6321c59c13f9f3a73c087 Mon Sep 17 00:00:00 2001 From: Siddharth Asthana Date: Tue, 31 Mar 2026 17:41:11 +0530 Subject: [PATCH 21/61] cat-file: add mailmap subcommand to --batch-command git-cat-file(1)'s --batch-command works with the --use-mailmap option, but this option needs to be set when the process is created. This means we cannot change this option mid-operation. At GitLab, Gitaly keeps interacting with a long-lived git-cat-file process and it would be useful if --batch-command supported toggling mailmap dynamically on an existing process. Add a `mailmap` subcommand to --batch-command that takes a boolean argument. The command now uses `git_parse_maybe_bool()` and supports all standard Git boolean values. Mailmap data is loaded lazily and kept in memory, while a helper centralizes the one-time load path used both at startup and from the batch-command handler. Extend tests to cover runtime toggling, startup option interactions (`--mailmap`/`--no-mailmap`), accepted boolean forms, and invalid values. Signed-off-by: Siddharth Asthana Signed-off-by: Junio C Hamano --- Documentation/git-cat-file.adoc | 7 +++ builtin/cat-file.c | 32 ++++++++-- t/t4203-mailmap.sh | 105 ++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 4 deletions(-) diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc index c139f55a168d61..3db925e1de3df9 100644 --- a/Documentation/git-cat-file.adoc +++ b/Documentation/git-cat-file.adoc @@ -174,6 +174,13 @@ flush:: since the beginning or since the last flush was issued. When `--buffer` is used, no output will come until a `flush` is issued. When `--buffer` is not used, commands are flushed each time without issuing `flush`. + +mailmap :: + Enable or disable mailmap for subsequent commands. ++ +The `` argument accepts the same boolean values as +linkgit:git-config[1]. When enabled, mailmap data is loaded on first +use and kept in memory until the process exits. -- + diff --git a/builtin/cat-file.c b/builtin/cat-file.c index b6f12f41d6070a..a3dab0b40cdef9 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -57,6 +57,14 @@ static int use_mailmap; static char *replace_idents_using_mailmap(char *, size_t *); +static void load_mailmap(void) +{ + if (mailmap.strdup_strings) + return; + + read_mailmap(the_repository, &mailmap); +} + static char *replace_idents_using_mailmap(char *object_buf, size_t *size) { struct strbuf sb = STRBUF_INIT; @@ -692,6 +700,21 @@ static void parse_cmd_info(struct batch_options *opt, batch_one_object(line, output, opt, data); } +static void parse_cmd_mailmap(struct batch_options *opt UNUSED, + const char *line, + struct strbuf *output UNUSED, + struct expand_data *data UNUSED) +{ + int value = git_parse_maybe_bool(line); + + if (value < 0) + die(_("mailmap: invalid boolean '%s'"), line); + + if (value > 0) + load_mailmap(); + use_mailmap = value; +} + static void dispatch_calls(struct batch_options *opt, struct strbuf *output, struct expand_data *data, @@ -725,9 +748,10 @@ static const struct parse_cmd { parse_cmd_fn_t fn; unsigned takes_args; } commands[] = { - { "contents", parse_cmd_contents, 1}, - { "info", parse_cmd_info, 1}, - { "flush", NULL, 0}, + { "contents", parse_cmd_contents, 1 }, + { "info", parse_cmd_info, 1 }, + { "flush", NULL, 0 }, + { "mailmap", parse_cmd_mailmap, 1 }, }; static void batch_objects_command(struct batch_options *opt, @@ -1128,7 +1152,7 @@ int cmd_cat_file(int argc, opt_epts = (opt == 'e' || opt == 'p' || opt == 't' || opt == 's'); if (use_mailmap) - read_mailmap(the_repository, &mailmap); + load_mailmap(); switch (batch.objects_filter.choice) { case LOFC_DISABLED: diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh index 74b7ddccb26d59..249548eb9bc159 100755 --- a/t/t4203-mailmap.sh +++ b/t/t4203-mailmap.sh @@ -1133,6 +1133,111 @@ test_expect_success 'git cat-file --batch-command returns correct size with --us test_cmp expect actual ' +test_expect_success 'git cat-file --batch-command mailmap yes enables mailmap mid-stream' ' + test_when_finished "rm .mailmap" && + cat >.mailmap <<-\EOF && + C O Mitter Orig + EOF + commit_sha=$(git rev-parse HEAD) && + git cat-file commit HEAD >commit_no_mailmap.out && + git cat-file --use-mailmap commit HEAD >commit_mailmap.out && + size_no_mailmap=$(wc -c actual && + echo $commit_sha commit $size_no_mailmap >expect && + echo $commit_sha commit $size_mailmap >>expect && + test_cmp expect actual +' + +test_expect_success 'git cat-file --batch-command mailmap no disables mailmap mid-stream' ' + test_when_finished "rm .mailmap" && + cat >.mailmap <<-\EOF && + C O Mitter Orig + EOF + commit_sha=$(git rev-parse HEAD) && + git cat-file commit HEAD >commit_no_mailmap.out && + git cat-file --use-mailmap commit HEAD >commit_mailmap.out && + size_no_mailmap=$(wc -c actual && + echo $commit_sha commit $size_mailmap >expect && + echo $commit_sha commit $size_no_mailmap >>expect && + test_cmp expect actual +' + +test_expect_success 'git cat-file --batch-command mailmap works in --buffer mode' ' + test_when_finished "rm .mailmap" && + cat >.mailmap <<-\EOF && + C O Mitter Orig + EOF + commit_sha=$(git rev-parse HEAD) && + git cat-file commit HEAD >commit_no_mailmap.out && + git cat-file --use-mailmap commit HEAD >commit_mailmap.out && + size_no_mailmap=$(wc -c actual && + echo $commit_sha commit $size_mailmap >expect && + echo $commit_sha commit $size_no_mailmap >>expect && + test_cmp expect actual +' + +test_expect_success 'git cat-file --batch-command mailmap no overrides startup --mailmap' ' + test_when_finished "rm .mailmap" && + cat >.mailmap <<-\EOF && + C O Mitter Orig + EOF + commit_sha=$(git rev-parse HEAD) && + git cat-file --use-mailmap commit HEAD >commit_mailmap.out && + size_mailmap=$(wc -c commit_no_mailmap.out && + size_no_mailmap=$(wc -c actual && + echo $commit_sha commit $size_mailmap >expect && + echo $commit_sha commit $size_no_mailmap >>expect && + test_cmp expect actual +' + +test_expect_success 'git cat-file --batch-command mailmap yes overrides startup --no-mailmap' ' + test_when_finished "rm .mailmap" && + cat >.mailmap <<-\EOF && + C O Mitter Orig + EOF + commit_sha=$(git rev-parse HEAD) && + git cat-file commit HEAD >commit_no_mailmap.out && + size_no_mailmap=$(wc -c commit_mailmap.out && + size_mailmap=$(wc -c actual && + echo $commit_sha commit $size_no_mailmap >expect && + echo $commit_sha commit $size_mailmap >>expect && + test_cmp expect actual +' + +test_expect_success 'git cat-file --batch-command mailmap accepts true/false' ' + test_when_finished "rm .mailmap" && + cat >.mailmap <<-\EOF && + C O Mitter Orig + EOF + commit_sha=$(git rev-parse HEAD) && + git cat-file commit HEAD >commit_no_mailmap.out && + size_no_mailmap=$(wc -c commit_mailmap.out && + size_mailmap=$(wc -c actual && + echo $commit_sha commit $size_mailmap >expect && + echo $commit_sha commit $size_no_mailmap >>expect && + test_cmp expect actual +' + +test_expect_success 'git cat-file --batch-command mailmap rejects invalid boolean' ' + echo "mailmap maybe" >in && + test_must_fail git cat-file --batch-command err && + test_grep "mailmap: invalid boolean .*maybe" err +' + test_expect_success 'git cat-file --mailmap works with different author and committer' ' test_when_finished "rm .mailmap" && cat >.mailmap <<-\EOF && From 0df739ea34ad8bc54f4ad6fdc978d97cd3c82759 Mon Sep 17 00:00:00 2001 From: Jayesh Daga Date: Tue, 31 Mar 2026 15:34:26 +0000 Subject: [PATCH 22/61] unpack-trees: use repository from index instead of global unpack_trees() currently initializes its repository from the global 'the_repository', even though a repository instance is already available via the source index. Use 'o->src_index->repo' instead of the global variable, reducing reliance on global repository state. This is a step towards eliminating global repository usage in unpack_trees(). Suggested-by: Patrick Steinhardt Signed-off-by: Jayesh Daga Signed-off-by: Junio C Hamano --- unpack-trees.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index 998a1e6dc70cae..191b9d4769a7b4 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1780,14 +1780,14 @@ static int clear_ce_flags(struct index_state *istate, xsnprintf(label, sizeof(label), "clear_ce_flags(0x%08lx,0x%08lx)", (unsigned long)select_mask, (unsigned long)clear_mask); - trace2_region_enter("unpack_trees", label, the_repository); + trace2_region_enter("unpack_trees", label, istate->repo); rval = clear_ce_flags_1(istate, istate->cache, istate->cache_nr, &prefix, select_mask, clear_mask, pl, 0, 0); - trace2_region_leave("unpack_trees", label, the_repository); + trace2_region_leave("unpack_trees", label, istate->repo); stop_progress(&istate->progress); return rval; @@ -1903,7 +1903,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options BUG("o->df_conflict_entry is an output only field"); trace_performance_enter(); - trace2_region_enter("unpack_trees", "unpack_trees", the_repository); + trace2_region_enter("unpack_trees", "unpack_trees", repo); prepare_repo_settings(repo); if (repo->settings.command_requires_full_index) { @@ -2007,9 +2007,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options } trace_performance_enter(); - trace2_region_enter("unpack_trees", "traverse_trees", the_repository); + trace2_region_enter("unpack_trees", "traverse_trees", repo); ret = traverse_trees(o->src_index, len, t, &info); - trace2_region_leave("unpack_trees", "traverse_trees", the_repository); + trace2_region_leave("unpack_trees", "traverse_trees", repo); trace_performance_leave("traverse_trees"); if (ret < 0) goto return_failed; @@ -2106,7 +2106,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options dir_clear(o->internal.dir); o->internal.dir = NULL; } - trace2_region_leave("unpack_trees", "unpack_trees", the_repository); + trace2_region_leave("unpack_trees", "unpack_trees", repo); trace_performance_leave("unpack_trees"); return ret; From c74767b7a33325dcbf58df6977bc9442b0c35375 Mon Sep 17 00:00:00 2001 From: Jayesh Daga Date: Tue, 31 Mar 2026 15:34:27 +0000 Subject: [PATCH 23/61] unpack-trees: use repository from index instead of global unpack_trees() currently initializes its repository from the global 'the_repository', even though a repository instance is already available via the source index. Use 'o->src_index->repo' instead of the global variable, reducing reliance on global repository state. This is a step towards eliminating global repository usage in unpack_trees(). Suggested-by: Patrick Steinhardt Signed-off-by: Jayesh Daga Signed-off-by: Junio C Hamano --- unpack-trees.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unpack-trees.c b/unpack-trees.c index 191b9d4769a7b4..b42020f16b10ae 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1882,7 +1882,7 @@ static int verify_absent(const struct cache_entry *, */ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o) { - struct repository *repo = the_repository; + struct repository *repo = o->src_index->repo; int i, ret; static struct cache_entry *dfc; struct pattern_list pl; From 2b9665c4e82a1a62d93a64e9892574d6e03ec019 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:49 -0400 Subject: [PATCH 24/61] convert: add const to fix strchr() warnings C23 versions of libc (like recent glibc) may provide generic versions of strchr() that match constness between the input and return value. The idea being that the compiler can detect when it implicitly converts a const pointer into a non-const one (which then emits a warning). There are a few cases here where the result pointer does not need to be non-const at all, and we should mark it as such. That silences the warning (and avoids any potential problems with trying to write via those pointers). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- convert.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/convert.c b/convert.c index a34ec6ecdc057e..eae36c8a5936f4 100644 --- a/convert.c +++ b/convert.c @@ -1168,7 +1168,8 @@ static int ident_to_worktree(const char *src, size_t len, struct strbuf *buf, int ident) { struct object_id oid; - char *to_free = NULL, *dollar, *spc; + char *to_free = NULL; + const char *dollar, *spc; int cnt; if (!ident) From 2fb6a18782ff8f2d97b44c8812f9027f3812f970 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:51 -0400 Subject: [PATCH 25/61] http: add const to fix strchr() warnings The "path" field of a "struct repo" (a custom http-push struct, not to be confused with "struct repository) is a pointer into a const argv string, and is never written to. The compiler does not traditionally complain about assigning from a const pointer because it happens via strchr(). But with some C23 libc versions (notably recent glibc), it has started to do so. Let's mark the field as const to silence the warnings. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http-push.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http-push.c b/http-push.c index 9ae6062198e14f..96df6344ee6245 100644 --- a/http-push.c +++ b/http-push.c @@ -99,7 +99,7 @@ static struct object_list *objects; struct repo { char *url; - char *path; + const char *path; int path_len; int has_info_refs; int can_update_info_refs; From eedc7ecc66aefa085aae9bf51b56aa11eeb23950 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:56 -0400 Subject: [PATCH 26/61] transport-helper: drop const to fix strchr() warnings We implicitly drop the const from our "key" variable when we do: char *p = strchr(key, ' '); which causes compilation with some C23 versions of libc (notably recent glibc) to complain. We need "p" to remain writable, since we assign NUL over the space we found. We can solve this by also making "key" writable. This works because it comes from a strbuf, which is itself a writable string. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- transport-helper.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/transport-helper.c b/transport-helper.c index 4d95d84f9e4d05..4614036c99a5a0 100644 --- a/transport-helper.c +++ b/transport-helper.c @@ -781,7 +781,8 @@ static int push_update_ref_status(struct strbuf *buf, if (starts_with(buf->buf, "option ")) { struct object_id old_oid, new_oid; - const char *key, *val; + char *key; + const char *val; char *p; if (!state->hint || !(state->report || state->new_report)) From 031d29d6fbf12284d391c23f04d15970c3bac11c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:14:58 -0400 Subject: [PATCH 27/61] pager: explicitly cast away strchr() constness When we do: char *cp = strchr(argv[i], '='); it implicitly removes the constness from argv[i]. We need "cp" to remain writable (since we overwrite it with a NUL). In theory we should be able to drop the const from argv[i], because it is a sub-pointer into our duplicated pager_env variable. But we get it from split_cmdline(), which uses the traditional "const char **" type for argv. This is overly limiting, but changing it would be awkward for all the other callers of split_cmdline(). Let's do an explicit cast with a note about why it is OK. This is enough to silence compiler warnings about the implicit const problems. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pager.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pager.c b/pager.c index 5531fff50eb73f..35b210e0484f90 100644 --- a/pager.c +++ b/pager.c @@ -108,10 +108,11 @@ const char *git_pager(struct repository *r, int stdout_is_tty) static void setup_pager_env(struct strvec *env) { - const char **argv; + char **argv; int i; char *pager_env = xstrdup(PAGER_ENV); - int n = split_cmdline(pager_env, &argv); + /* split_cmdline splits in place, so we know the result is writable */ + int n = split_cmdline(pager_env, (const char ***)&argv); if (n < 0) die("malformed build-time PAGER_ENV: %s", From 21c57efc77ccdef2d6186874024593ded59ecc65 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:01 -0400 Subject: [PATCH 28/61] run-command: explicitly cast away constness when assigning to void We do this: char *equals = strchr(*e, '='); which implicitly removes the constness from "*e" and cause the compiler to complain. We never write to "equals", but later assign it to a string_list util field, which is defined as non-const "void *". We have to cast somewhere, but doing so at the assignment to util is the least-bad place, since that is the source of the confusion. Sadly we are still open to accidentally writing to the string via the util pointer, but that is the cost of using void pointers, which lose all type information. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/run-command.c b/run-command.c index 32c290ee6a221f..d6980c79b356fd 100644 --- a/run-command.c +++ b/run-command.c @@ -604,11 +604,11 @@ static void trace_add_env(struct strbuf *dst, const char *const *deltaenv) /* Last one wins, see run-command.c:prep_childenv() for context */ for (e = deltaenv; e && *e; e++) { struct strbuf key = STRBUF_INIT; - char *equals = strchr(*e, '='); + const char *equals = strchr(*e, '='); if (equals) { strbuf_add(&key, *e, equals - *e); - string_list_insert(&envs, key.buf)->util = equals + 1; + string_list_insert(&envs, key.buf)->util = (void *)(equals + 1); } else { string_list_insert(&envs, *e)->util = NULL; } From bc4fd55984ce8c0fb99c8672ef2702acbbd98521 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:03 -0400 Subject: [PATCH 29/61] find_last_dir_sep(): convert inline function to macro The find_last_dir_sep() function is implemented as an inline function which takes in a "const char *" and returns a "char *" via strrchr(). That means that just like strrchr(), it quietly removes the const from our pointer, which could lead to accidentally writing to the resulting string. But C23 versions of libc (including recent glibc) annotate strrchr() such that the compiler can detect when const is implicitly lost, and it now complains about the call in this inline function. We can't just switch the return type of the function to "const char *", though. Some callers really do want a non-const string to be returned (and are OK because they are feeding a non-const string into the function). The most general solution is for us to annotate find_last_dir_sep() in the same way that is done for strrchr(). But doing so relies on using C23 generics, which we do not otherwise require. Since this inline function is wrapping a single call to strrchr(), we can take a shortcut. If we implement it as a macro, then the original type information is still available to strrchr(), and it does the check for us. Note that this is just one implementation of find_last_dir_sep(). There is an alternate implementation in compat/win32/path-utils.h. It doesn't suffer from the same warning, as it does not use strrchr() and just casts away const explicitly. That's not ideal, and eventually we may want to conditionally teach it the same C23 generic trick that strrchr() uses. But it has been that way forever, and our goal here is just quieting new warnings, not improving const-checking. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 4b4ea2498f13ef..4bb59b31017e46 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -335,11 +335,7 @@ static inline int is_path_owned_by_current_uid(const char *path, #endif #ifndef find_last_dir_sep -static inline char *git_find_last_dir_sep(const char *path) -{ - return strrchr(path, '/'); -} -#define find_last_dir_sep git_find_last_dir_sep +#define find_last_dir_sep(path) strrchr((path), '/') #endif #ifndef has_dir_sep From 25e5ceb9ee21d8806c9a3651e4f10241155f6e14 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:05 -0400 Subject: [PATCH 30/61] pseudo-merge: fix disk reads from find_pseudo_merge() The goal of this commit was to fix a const warning when compiling with new versions of glibc, but ended up untangling a much deeper problem. The find_pseudo_merge() function does a bsearch() on the "commits" pointer of a pseudo_merge_map. This pointer ultimately comes from memory mapped from the on-disk bitmap file, and is thus not writable. The "commits" array is correctly marked const, but the result from bsearch() is returned directly as a non-const pseudo_merge_commit struct. Since new versions of glibc annotate bsearch() in a way that detects the implicit loss of const, the compiler now warns. My first instinct was that we should be returning a const struct. That requires apply_pseudo_merges_for_commit() to mark its local pointer as const. But that doesn't work! If the offset field has the high-bit set, we look it up in the extended table via nth_pseudo_merge_ext(). And that function then feeds our const struct to read_pseudo_merge_commit_at(), which writes into it by byte-swapping from the on-disk mmap. But I think this points to a larger problem with find_pseudo_merge(). It is not just that the return value is missing const, but it is missing that byte-swapping! And we know that byte-swapping is needed here, because the comparator we use for bsearch() also calls our read_pseudo_merge_commit_at() helper. So I think the interface is all wrong here. We should not be returning a pointer to a struct which was cast from on-disk data. We should be filling in a caller-provided struct using the bytes we found, byte-swapping the values. That of course raises the dual question: how did this ever work, and does it work now? The answer to the first part is: this code does not seem to be triggered in the test suite at all. If we insert a BUG("foo") call into apply_pseudo_merges_for_commit(), it never triggers. So I think there is something wrong or missing from the test setup, and this bears further investigation. Sadly the answer to the second part ("does it work now") is still "no idea". I _think_ this takes us in a positive direction, but my goal here is mainly to quiet the compiler warning. Further bug-hunting on this experimental feature can be done separately. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- pseudo-merge.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/pseudo-merge.c b/pseudo-merge.c index a2d5bd85f95ebf..ff18b6c364245e 100644 --- a/pseudo-merge.c +++ b/pseudo-merge.c @@ -638,14 +638,21 @@ static int pseudo_merge_commit_cmp(const void *va, const void *vb) return 0; } -static struct pseudo_merge_commit *find_pseudo_merge(const struct pseudo_merge_map *pm, - uint32_t pos) +static int find_pseudo_merge(const struct pseudo_merge_map *pm, uint32_t pos, + struct pseudo_merge_commit *out) { + const unsigned char *at; + if (!pm->commits_nr) - return NULL; + return 0; - return bsearch(&pos, pm->commits, pm->commits_nr, - PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp); + at = bsearch(&pos, pm->commits, pm->commits_nr, + PSEUDO_MERGE_COMMIT_RAWSZ, pseudo_merge_commit_cmp); + if (!at) + return 0; + + read_pseudo_merge_commit_at(out, at); + return 1; } int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, @@ -653,16 +660,15 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, struct commit *commit, uint32_t commit_pos) { struct pseudo_merge *merge; - struct pseudo_merge_commit *merge_commit; + struct pseudo_merge_commit merge_commit; int ret = 0; - merge_commit = find_pseudo_merge(pm, commit_pos); - if (!merge_commit) + if (!find_pseudo_merge(pm, commit_pos, &merge_commit)) return 0; - if (merge_commit->pseudo_merge_ofs & ((uint64_t)1<<63)) { + if (merge_commit.pseudo_merge_ofs & ((uint64_t)1<<63)) { struct pseudo_merge_commit_ext ext = { 0 }; - off_t ofs = merge_commit->pseudo_merge_ofs & ~((uint64_t)1<<63); + off_t ofs = merge_commit.pseudo_merge_ofs & ~((uint64_t)1<<63); uint32_t i; if (pseudo_merge_ext_at(pm, &ext, ofs) < -1) { @@ -673,11 +679,11 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, } for (i = 0; i < ext.nr; i++) { - if (nth_pseudo_merge_ext(pm, &ext, merge_commit, i) < 0) + if (nth_pseudo_merge_ext(pm, &ext, &merge_commit, i) < 0) return ret; merge = pseudo_merge_at(pm, &commit->object.oid, - merge_commit->pseudo_merge_ofs); + merge_commit.pseudo_merge_ofs); if (!merge) return ret; @@ -687,7 +693,7 @@ int apply_pseudo_merges_for_commit(const struct pseudo_merge_map *pm, } } else { merge = pseudo_merge_at(pm, &commit->object.oid, - merge_commit->pseudo_merge_ofs); + merge_commit.pseudo_merge_ofs); if (!merge) return ret; From cefb8b7b47415aff6ad3ecdc6f3c546fa7abfd16 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:07 -0400 Subject: [PATCH 31/61] skip_prefix(): check const match between in and out params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skip_prefix() function takes in a "const char *" string, and returns via a "const char **" out-parameter that points somewhere in that string. This is fine if you are operating on a const string, like: const char *in = ...; const char *out; if (skip_prefix(in, "foo", &out)) ...look at out... It is also OK if "in" is not const but "out" is, as we add an implicit const when we pass "in" to the function. But there's another case where this is limiting. If we want both fields to be non-const, like: char *in = ...; char *out; if (skip_prefix(in, "foo", &out)) *out = '\0'; it doesn't work. The compiler will complain about the type mismatch in passing "&out" to a parameter which expects "const char **". So to make this work, we have to do an explicit cast. But such a cast is ugly, and also means that we run afoul of making this mistake: const char *in = ...; char *out; if (skip_prefix(in, "foo", (const char **)&out)) *out = '\0'; which causes us to write to the memory pointed by "in", which was const. We can imagine these four cases as: (1) const in, const out (2) non-const in, const out (3) non-const in, non-const out (4) const in, non-const out Cases (1) and (2) work now. We would like case (3) to work but it doesn't. But we would like to catch case (4) as a compile error. So ideally the rule is "the out-parameter must be at least as const as the in-parameter". We can do this with some macro trickery. We wrap skip_prefix() in a macro so that it has access to the real types of in/out. And then we pass those parameters through another macro which: 1. Fails if the "at least as const" rule is not filled. 2. Casts to match the signature of the real skip_prefix(). There are a lot of ways to implement the "fails" part. You can use __builtin_types_compatible_p() to check, and then either our BUILD_ASSERT macros or _Static_assert to fail. But that requires some conditional compilation based on compiler feature. That's probably OK (the fallback would be to just cast without catching case 4). But we can do better. The macro I have here uses a ternary with a dead branch that tries to assign "in" to "out", which should work everywhere and lets the compiler catch the problem in the usual way. With an input like this: int foo(const char *x, const char **y); #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out))) void ok_const(const char *x, const char **y) { foo(x, y); } void ok_nonconst(char *x, char **y) { foo(x, y); } void ok_add_const(char *x, const char **y) { foo(x, y); } void bad_drop_const(const char *x, char **y) { foo(x, y); } gcc reports: foo.c: In function ‘bad_drop_const’: foo.c:2:35: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] 2 | ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) | ^ foo.c:4:31: note: in expansion of macro ‘CONST_OUTPARAM’ 4 | #define foo(in,out) foo((in), CONST_OUTPARAM((in), (out))) | ^~~~~~~~~~~~~~ foo.c:23:9: note: in expansion of macro ‘foo’ 23 | foo(x, y); | ^~~ It's a bit verbose, but I think makes it reasonably clear what's going on. Using BUILD_ASSERT_OR_ZERO() ends up much worse. Using _Static_assert you can be a bit more informative, but that's not something we use at all yet in our code-base (it's an old gnu-ism later standardized in C11). Our generic macro only works for "const char **", which is something we could improve by using typeof(in). But that introduces more portability questions, and also some weird corner cases (e.g., around implicit void conversion). This patch just introduces the concept. We'll make use of it in future patches. Note that we rename skip_prefix() to skip_prefix_impl() here, to avoid expanding the macro when defining the function. That's not strictly necessary since we could just define the macro after defining the inline function. But that would not be the case for a non-inline function (and we will apply this technique to them later, and should be consistent). It also gives us more freedom about where to define the macro. I did so right above the definition here, which I think keeps the relevant bits together. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 4bb59b31017e46..e9629b2a9dc6d4 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -463,6 +463,21 @@ void set_warn_routine(report_fn routine); report_fn get_warn_routine(void); void set_die_is_recursing_routine(int (*routine)(void)); +/* + * Check that an out-parameter that is "at least as const as" a matching + * in-parameter. For example, skip_prefix() will return "out" that is a subset + * of "str". So: + * + * const str, const out: ok + * non-const str, const out: ok + * non-const str, non-const out: ok + * const str, non-const out: compile error + * + * See the skip_prefix macro below for an example of use. + */ +#define CONST_OUTPARAM(in, out) \ + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) + /* * If the string "str" begins with the string found in "prefix", return true. * The "out" parameter is set to "str + strlen(prefix)" (i.e., to the point in @@ -479,8 +494,10 @@ void set_die_is_recursing_routine(int (*routine)(void)); * [skip prefix if present, otherwise use whole string] * skip_prefix(name, "refs/heads/", &name); */ -static inline bool skip_prefix(const char *str, const char *prefix, - const char **out) +#define skip_prefix(str, prefix, out) \ + skip_prefix_impl((str), (prefix), CONST_OUTPARAM((str), (out))) +static inline bool skip_prefix_impl(const char *str, const char *prefix, + const char **out) { do { if (!*prefix) { From d3cd819e8bb21189b7bf3b2718898b610b85b119 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:10 -0400 Subject: [PATCH 32/61] pkt-line: make packet_reader.line non-const The "line" member of a packet_reader struct is marked as const. This kind of makes sense, because it's not its own allocated buffer that should be freed, and we often use const to indicate that. But it is always writable, because it points into the non-const "buffer" member. And we rely on this writability in places like send-pack and receive-pack, where we parse incoming packet contents by writing NULs over delimiters. This has traditionally worked because we implicitly cast away the constness with strchr() like: const char *head; char *p; head = reader->line; p = strchr(head, ' '); Since C23 libc provides a generic strchr() to detect this implicit const removal, this now generate a compiler warning on some platforms (like recent glibc). We can fix it by marking "line" as non-const, as well as a few intermediate variables (like "head" in the above example). Note that by itself, switching to a non-const variable would cause problems with this line in send-pack.c: if (!skip_prefix(reader->line, "unpack ", &reader->line)) But due to our skip_prefix() magic introduced in the previous commit, this compiles fine (both the in and out-parameters are non-const, so we know it is safe). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 7 ++++--- pkt-line.h | 2 +- send-pack.c | 7 ++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index e34edff406959a..a6af16c4e7e03f 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1025,8 +1025,8 @@ static int read_proc_receive_report(struct packet_reader *reader, for (;;) { struct object_id old_oid, new_oid; - const char *head; - const char *refname; + char *head; + char *refname; char *p; enum packet_read_status status; @@ -1050,7 +1050,8 @@ static int read_proc_receive_report(struct packet_reader *reader, } *p++ = '\0'; if (!strcmp(head, "option")) { - const char *key, *val; + char *key; + const char *val; if (!hint || !(report || new_report)) { if (!once++) diff --git a/pkt-line.h b/pkt-line.h index 3b33cc64f34dcc..e6cf85e34ee3c4 100644 --- a/pkt-line.h +++ b/pkt-line.h @@ -184,7 +184,7 @@ struct packet_reader { int pktlen; /* the last line read */ - const char *line; + char *line; /* indicates if a line has been peeked */ int line_peeked; diff --git a/send-pack.c b/send-pack.c index 07ecfae4de92a2..b4361d5610dc91 100644 --- a/send-pack.c +++ b/send-pack.c @@ -175,8 +175,8 @@ static int receive_status(struct repository *r, ret = receive_unpack_status(reader); while (1) { struct object_id old_oid, new_oid; - const char *head; - const char *refname; + char *head; + char *refname; char *p; if (packet_reader_read(reader) != PACKET_READ_NORMAL) break; @@ -190,7 +190,8 @@ static int receive_status(struct repository *r, *p++ = '\0'; if (!strcmp(head, "option")) { - const char *key, *val; + char *key; + const char *val; if (!hint || !(report || new_report)) { if (!once++) From c39512600f85aa88f368dc6bd13baeb183ae52ad Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:12 -0400 Subject: [PATCH 33/61] range-diff: drop const to fix strstr() warnings This is another case where we implicitly drop the "const" from a pointer by feeding it to strstr() and assigning the result to a non-const pointer. This is OK in practice, since the const pointer originally comes from a writable source (a strbuf), but C23 libc implementations have started to complain about it. We do write to the output pointer, so it needs to remain non-const. We can just switch the input pointer to also be non-const in this case. By itself that would run into problems with calls to skip_prefix(), but since that function has now been taught to match in/out constness automatically, it just works without us doing anything further. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- range-diff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/range-diff.c b/range-diff.c index 2712a9a107ab06..8e2dd2eb193eb9 100644 --- a/range-diff.c +++ b/range-diff.c @@ -88,7 +88,7 @@ static int read_patches(const char *range, struct string_list *list, line = contents.buf; size = contents.len; for (; size > 0; size -= len, line += len) { - const char *p; + char *p; char *eol; eol = memchr(line, '\n', size); From 8a0566b42b133b73423c801a7ab6f356de69f51a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:14 -0400 Subject: [PATCH 34/61] http: drop const to fix strstr() warning In redact_sensitive_header(), a C23 implementation of libc will complain that strstr() assigns the result from "const char *cookie" to "char *semicolon". Ultimately the memory is writable. We're fed a strbuf, generate a const pointer "sensitive_header" within it using skip_iprefix(), and then assign the result to "cookie". So we can solve this by dropping the const from "cookie" and "sensitive_header". However, this runs afoul of skip_iprefix(), which wants a "const char **" for its out-parameter. We can solve that by teaching skip_iprefix() the same "make sure out is at least as const as in" magic that we recently taught to skip_prefix(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 6 ++++-- http.c | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index e9629b2a9dc6d4..4ddac619921a71 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -902,8 +902,10 @@ static inline size_t xsize_t(off_t len) * is done via tolower(), so it is strictly ASCII (no multi-byte characters or * locale-specific conversions). */ -static inline bool skip_iprefix(const char *str, const char *prefix, - const char **out) +#define skip_iprefix(str, prefix, out) \ + skip_iprefix_impl((str), (prefix), CONST_OUTPARAM((str), (out))) +static inline bool skip_iprefix_impl(const char *str, const char *prefix, + const char **out) { do { if (!*prefix) { diff --git a/http.c b/http.c index 8ea1b9d1f68c16..8801bd22feed4a 100644 --- a/http.c +++ b/http.c @@ -726,7 +726,7 @@ static int has_proxy_cert_password(void) static int redact_sensitive_header(struct strbuf *header, size_t offset) { int ret = 0; - const char *sensitive_header; + char *sensitive_header; if (trace_curl_redact && (skip_iprefix(header->buf + offset, "Authorization:", &sensitive_header) || @@ -743,7 +743,7 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset) } else if (trace_curl_redact && skip_iprefix(header->buf + offset, "Cookie:", &sensitive_header)) { struct strbuf redacted_header = STRBUF_INIT; - const char *cookie; + char *cookie; while (isspace(*sensitive_header)) sensitive_header++; From f1b8a4d10888922eacaa9552d9925d3330ed5d8b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 2 Apr 2026 00:15:16 -0400 Subject: [PATCH 35/61] refs/files-backend: drop const to fix strchr() warning In show_one_reflog_ent(), we're fed a writable strbuf buffer, which we parse into the various reflog components. We write a NUL over email_end to tie off one of the fields, and thus email_end must be non-const. But with a C23 implementation of libc, strchr() will now complain when assigning the result to a non-const pointer from a const one. So we can fix this by making the source pointer non-const. But there's a catch. We derive that source pointer by parsing the line with parse_oid_hex_algop(), which requires a const pointer for its out-parameter. We can work around that by teaching it to use our CONST_OUTPARAM() trick, just like skip_prefix(). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- hex.c | 6 +++--- hex.h | 6 ++++-- refs/files-backend.c | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/hex.c b/hex.c index 865a232167553d..bc756722ca623b 100644 --- a/hex.c +++ b/hex.c @@ -54,9 +54,9 @@ int get_oid_hex(const char *hex, struct object_id *oid) return get_oid_hex_algop(hex, oid, the_hash_algo); } -int parse_oid_hex_algop(const char *hex, struct object_id *oid, - const char **end, - const struct git_hash_algo *algop) +int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, + const char **end, + const struct git_hash_algo *algop) { int ret = get_oid_hex_algop(hex, oid, algop); if (!ret) diff --git a/hex.h b/hex.h index e9ccb54065c0bc..1e9a65d83a4f6b 100644 --- a/hex.h +++ b/hex.h @@ -40,8 +40,10 @@ char *oid_to_hex(const struct object_id *oid); /* same static buffer */ * other invalid character. end is only updated on success; otherwise, it is * unmodified. */ -int parse_oid_hex_algop(const char *hex, struct object_id *oid, const char **end, - const struct git_hash_algo *algo); +#define parse_oid_hex_algop(hex, oid, end, algo) \ + parse_oid_hex_algop_impl((hex), (oid), CONST_OUTPARAM((hex), (end)), (algo)) +int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, const char **end, + const struct git_hash_algo *algo); /* * These functions work like get_oid_hex and parse_oid_hex, but they will parse diff --git a/refs/files-backend.c b/refs/files-backend.c index 7ce0d574781ffd..ad543ad7513248 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2190,7 +2190,7 @@ static int show_one_reflog_ent(struct files_ref_store *refs, char *email_end, *message; timestamp_t timestamp; int tz; - const char *p = sb->buf; + char *p = sb->buf; /* old SP new SP name SP time TAB msg LF */ if (!sb->len || sb->buf[sb->len - 1] != '\n' || From f7d92bc039ef70c6fed9e297df4cb115d694b9c6 Mon Sep 17 00:00:00 2001 From: Chen Linxuan Date: Fri, 3 Apr 2026 15:02:28 +0800 Subject: [PATCH 36/61] config: refactor include_by_gitdir() into include_by_path() The include_by_gitdir() function matches the realpath of a given path against a glob pattern, but its interface is tightly coupled to the gitdir condition: it takes a struct config_options *opts and extracts opts->git_dir internally. Refactor it into a more generic include_by_path() helper that takes a const char *path parameter directly, and update the gitdir and gitdir/i callers to pass opts->git_dir explicitly. No behavior change, just preparing for the addition of a new worktree condition that will reuse the same path-matching logic with a different path. Signed-off-by: Chen Linxuan Signed-off-by: Junio C Hamano --- config.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/config.c b/config.c index 156f2a24fa0027..7d5dae0e845085 100644 --- a/config.c +++ b/config.c @@ -235,23 +235,20 @@ static int prepare_include_condition_pattern(const struct key_value_info *kvi, return 0; } -static int include_by_gitdir(const struct key_value_info *kvi, - const struct config_options *opts, - const char *cond, size_t cond_len, int icase) +static int include_by_path(const struct key_value_info *kvi, + const char *path, + const char *cond, size_t cond_len, int icase) { struct strbuf text = STRBUF_INIT; struct strbuf pattern = STRBUF_INIT; size_t prefix; int ret = 0; - const char *git_dir; int already_tried_absolute = 0; - if (opts->git_dir) - git_dir = opts->git_dir; - else + if (!path) goto done; - strbuf_realpath(&text, git_dir, 1); + strbuf_realpath(&text, path, 1); strbuf_add(&pattern, cond, cond_len); ret = prepare_include_condition_pattern(kvi, &pattern, &prefix); if (ret < 0) @@ -284,7 +281,7 @@ static int include_by_gitdir(const struct key_value_info *kvi, * which'll do the right thing */ strbuf_reset(&text); - strbuf_add_absolute_path(&text, git_dir); + strbuf_add_absolute_path(&text, path); already_tried_absolute = 1; goto again; } @@ -400,9 +397,9 @@ static int include_condition_is_true(const struct key_value_info *kvi, const struct config_options *opts = inc->opts; if (skip_prefix_mem(cond, cond_len, "gitdir:", &cond, &cond_len)) - return include_by_gitdir(kvi, opts, cond, cond_len, 0); + return include_by_path(kvi, opts->git_dir, cond, cond_len, 0); else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len)) - return include_by_gitdir(kvi, opts, cond, cond_len, 1); + return include_by_path(kvi, opts->git_dir, cond, cond_len, 1); else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len)) return include_by_branch(inc, cond, cond_len); else if (skip_prefix_mem(cond, cond_len, "hasconfig:remote.*.url:", &cond, From da1f520f136d3418a7bd29d947e0bd8630a2379d Mon Sep 17 00:00:00 2001 From: Chen Linxuan Date: Fri, 3 Apr 2026 15:02:29 +0800 Subject: [PATCH 37/61] config: add "worktree" and "worktree/i" includeIf conditions The includeIf mechanism already supports matching on the .git directory path (gitdir) and the currently checked out branch (onbranch). But in multi-worktree setups the .git directory of a linked worktree points into the main repository's .git/worktrees/ area, which makes gitdir patterns cumbersome when one wants to include config based on the working tree's checkout path instead. Introduce two new condition keywords: - worktree: matches the realpath of the current worktree's working directory (i.e. repo_get_work_tree()) against a glob pattern. This is the path returned by git rev-parse --show-toplevel. - worktree/i: is the case-insensitive variant. The implementation reuses the include_by_path() helper introduced in the previous commit, passing the worktree path in place of the gitdir. The condition never matches in bare repositories (where there is no worktree) or during early config reading (where no repository is available). Add documentation describing the new conditions and their supported pattern features (glob wildcards, **/ and /**, ~ expansion, ./ relative paths, and trailing-/ prefix matching). Add tests covering bare repositories, multiple worktrees, and symlinked worktree paths. Signed-off-by: Chen Linxuan Signed-off-by: Junio C Hamano --- Documentation/config.adoc | 50 +++++++++++++++++++++++++++++ config.c | 6 ++++ t/t1305-config-include.sh | 66 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/Documentation/config.adoc b/Documentation/config.adoc index 62eebe7c54501c..a4f3ec90509816 100644 --- a/Documentation/config.adoc +++ b/Documentation/config.adoc @@ -146,6 +146,48 @@ refer to linkgit:gitignore[5] for details. For convenience: This is the same as `gitdir` except that matching is done case-insensitively (e.g. on case-insensitive file systems) +`worktree`:: + The data that follows the keyword `worktree` and a colon is used as a + glob pattern. If the working directory of the current worktree matches + the pattern, the include condition is met. ++ +The worktree location is the path where files are checked out (as returned +by `git rev-parse --show-toplevel`). This is different from `gitdir`, which +matches the `.git` directory path. In a linked worktree, the worktree path +is the directory where that worktree's files are located, not the main +repository's `.git` directory. ++ +The pattern can contain standard globbing wildcards and two additional +ones, `**/` and `/**`, that can match multiple path components. Please +refer to linkgit:gitignore[5] for details. For convenience: + + * If the pattern starts with `~/`, `~` will be substituted with the + content of the environment variable `HOME`. + + * If the pattern starts with `./`, it is replaced with the directory + containing the current config file. + + * If the pattern does not start with either `~/`, `./` or `/`, `**/` + will be automatically prepended. For example, the pattern `foo/bar` + becomes `**/foo/bar` and would match `/any/path/to/foo/bar`. + + * If the pattern ends with `/`, `**` will be automatically added. For + example, the pattern `foo/` becomes `foo/**`. In other words, it + matches "foo" and everything inside, recursively. ++ +This condition will never match in a bare repository (which has no worktree). ++ +This is useful when you need to use different `user.name`, `user.email`, or +GPG keys in different worktrees of the same repository. While +`extensions.worktreeConfig` also allows per-worktree configuration, it +requires changes inside each repository. This condition can be set in the +user's global configuration file (e.g. `~/.config/git/config`) and applies +to multiple repositories at once. + +`worktree/i`:: + This is the same as `worktree` except that matching is done + case-insensitively (e.g. on case-insensitive file systems) + `onbranch`:: The data that follows the keyword `onbranch` and a colon is taken to be a pattern with standard globbing wildcards and two additional @@ -244,6 +286,14 @@ Example [includeIf "gitdir:~/to/group/"] path = /path/to/foo.inc +; include if the worktree is at /path/to/project-build +[includeIf "worktree:/path/to/project-build"] + path = build-config.inc + +; include for all worktrees inside /path/to/group +[includeIf "worktree:/path/to/group/"] + path = group-config.inc + ; relative paths are always relative to the including ; file (if the condition is true); their location is not ; affected by the condition diff --git a/config.c b/config.c index 7d5dae0e845085..6d0c2d0725e4f7 100644 --- a/config.c +++ b/config.c @@ -400,6 +400,12 @@ static int include_condition_is_true(const struct key_value_info *kvi, return include_by_path(kvi, opts->git_dir, cond, cond_len, 0); else if (skip_prefix_mem(cond, cond_len, "gitdir/i:", &cond, &cond_len)) return include_by_path(kvi, opts->git_dir, cond, cond_len, 1); + else if (skip_prefix_mem(cond, cond_len, "worktree:", &cond, &cond_len)) + return include_by_path(kvi, inc->repo ? repo_get_work_tree(inc->repo) : NULL, + cond, cond_len, 0); + else if (skip_prefix_mem(cond, cond_len, "worktree/i:", &cond, &cond_len)) + return include_by_path(kvi, inc->repo ? repo_get_work_tree(inc->repo) : NULL, + cond, cond_len, 1); else if (skip_prefix_mem(cond, cond_len, "onbranch:", &cond, &cond_len)) return include_by_branch(inc, cond, cond_len); else if (skip_prefix_mem(cond, cond_len, "hasconfig:remote.*.url:", &cond, diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index 6e51f892f320bb..8a5ba4b884d3ff 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -396,4 +396,70 @@ test_expect_success 'onbranch without repository but explicit nonexistent Git di test_must_fail nongit git --git-dir=nonexistent config get foo.bar ' +# worktree: conditional include tests + +test_expect_success 'conditional include, worktree bare repo' ' + git init --bare wt-bare && + ( + cd wt-bare && + echo "[includeIf \"worktree:/\"]path=bar-bare" >>config && + echo "[test]wtbare=1" >bar-bare && + test_must_fail git config test.wtbare + ) +' + +test_expect_success 'conditional include, worktree multiple worktrees' ' + git init wt-multi && + ( + cd wt-multi && + test_commit initial && + git worktree add -b linked-branch ../wt-linked HEAD && + git worktree add -b prefix-branch ../wt-prefix/linked HEAD + ) && + wt_main="$(cd wt-multi && pwd)" && + wt_linked="$(cd wt-linked && pwd)" && + wt_prefix_parent="$(cd wt-prefix && pwd)" && + cat >>wt-multi/.git/config <<-EOF && + [includeIf "worktree:$wt_main"] + path = main-config + [includeIf "worktree:$wt_linked"] + path = linked-config + [includeIf "worktree:$wt_prefix_parent/"] + path = prefix-config + EOF + echo "[test]mainvar=main" >wt-multi/.git/main-config && + echo "[test]linkedvar=linked" >wt-multi/.git/linked-config && + echo "[test]prefixvar=prefix" >wt-multi/.git/prefix-config && + echo main >expect && + git -C wt-multi config test.mainvar >actual && + test_cmp expect actual && + test_must_fail git -C wt-multi config test.linkedvar && + test_must_fail git -C wt-multi config test.prefixvar && + echo linked >expect && + git -C wt-linked config test.linkedvar >actual && + test_cmp expect actual && + test_must_fail git -C wt-linked config test.mainvar && + test_must_fail git -C wt-linked config test.prefixvar && + echo prefix >expect && + git -C wt-prefix/linked config test.prefixvar >actual && + test_cmp expect actual && + test_must_fail git -C wt-prefix/linked config test.mainvar && + test_must_fail git -C wt-prefix/linked config test.linkedvar +' + +test_expect_success SYMLINKS 'conditional include, worktree resolves symlinks' ' + mkdir real-wt && + ln -s real-wt link-wt && + git init link-wt/repo && + ( + cd link-wt/repo && + # repo->worktree resolves symlinks, so use real path in pattern + echo "[includeIf \"worktree:**/real-wt/repo\"]path=bar-link" >>.git/config && + echo "[test]wtlink=2" >.git/bar-link && + echo 2 >expect && + git config test.wtlink >actual && + test_cmp expect actual + ) +' + test_done From ab16b9ae0574e80e87d61bdba282aa58b06d0043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Fri, 3 Apr 2026 12:01:35 +0200 Subject: [PATCH 38/61] config: retry acquiring config.lock for 100ms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When multiple processes write to a config file concurrently, they contend on its ".lock" file, which is acquired via open(O_EXCL) with no retry. The losers fail immediately with "could not lock config file". Two processes writing unrelated keys (say, "branch.a.remote" and "branch.b.remote") have no semantic conflict, yet one of them fails for a purely mechanical reason. This bites in practice when running `git worktree add -b` concurrently against the same repository. Each invocation makes several writes to ".git/config" to set up branch tracking, and tooling that creates worktrees in parallel sees intermittent failures. Worse, `git worktree add` does not propagate the failed config write to its exit code: the worktree is created and the command exits 0, but tracking configuration is silently dropped. The lock is held only for the duration of rewriting a small file, so retrying for 100 ms papers over any realistic contention while still failing fast if a stale lock has been left behind by a crashed process. This mirrors what we already do for individual reference locks (4ff0f01cb7 (refs: retry acquiring reference locks for 100ms, 2017-08-21)). Signed-off-by: Jörg Thalheim Signed-off-by: Junio C Hamano --- config.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index 156f2a24fa0027..f7aff8725de4a3 100644 --- a/config.c +++ b/config.c @@ -2903,6 +2903,14 @@ char *git_config_prepare_comment_string(const char *comment) return prepared; } +/* + * How long to retry acquiring config.lock when another process holds it. + * The lock is held only for the duration of rewriting a small file, so + * 100 ms covers any realistic contention while still failing fast if + * a stale lock has been left behind by a crashed process. + */ +#define CONFIG_LOCK_TIMEOUT_MS 100 + static void validate_comment_string(const char *comment) { size_t leading_blanks; @@ -2986,7 +2994,8 @@ int repo_config_set_multivar_in_file_gently(struct repository *r, * The lock serves a purpose in addition to locking: the new * contents of .git/config will be written into it. */ - fd = hold_lock_file_for_update(&lock, config_filename, 0); + fd = hold_lock_file_for_update_timeout(&lock, config_filename, 0, + CONFIG_LOCK_TIMEOUT_MS); if (fd < 0) { error_errno(_("could not lock config file %s"), config_filename); ret = CONFIG_NO_LOCK; @@ -3331,7 +3340,8 @@ static int repo_config_copy_or_rename_section_in_file( if (!config_filename) config_filename = filename_buf = repo_git_path(r, "config"); - out_fd = hold_lock_file_for_update(&lock, config_filename, 0); + out_fd = hold_lock_file_for_update_timeout(&lock, config_filename, 0, + CONFIG_LOCK_TIMEOUT_MS); if (out_fd < 0) { ret = error(_("could not lock config file %s"), config_filename); goto out; From 58589c20e555de187c8026ac721467919595543f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 4 Apr 2026 01:42:11 -0400 Subject: [PATCH 39/61] git-compat-util: fix CONST_OUTPARAM typo and indentation There's a typo in the comment, making it hard to understand. And the macro itself is indented with spaces rather than tab. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- git-compat-util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 4ddac619921a71..ae1bdc90a4cd6a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -464,7 +464,7 @@ report_fn get_warn_routine(void); void set_die_is_recursing_routine(int (*routine)(void)); /* - * Check that an out-parameter that is "at least as const as" a matching + * Check that an out-parameter is "at least as const as" a matching * in-parameter. For example, skip_prefix() will return "out" that is a subset * of "str". So: * @@ -476,7 +476,7 @@ void set_die_is_recursing_routine(int (*routine)(void)); * See the skip_prefix macro below for an example of use. */ #define CONST_OUTPARAM(in, out) \ - ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) + ((const char **)(0 ? ((*(out) = (in)),(out)) : (out))) /* * If the string "str" begins with the string found in "prefix", return true. From 1d980196adfd79ae0936e681e8d98c57d9900785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:44 +0000 Subject: [PATCH 40/61] doc: convert git-difftool manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands to synopsis style * use __ for arguments * fix conditional text to sentence limits Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/config/difftool.adoc | 24 ++++----- Documentation/config/mergetool.adoc | 8 +-- Documentation/git-difftool.adoc | 80 ++++++++++++++--------------- 3 files changed, 56 insertions(+), 56 deletions(-) diff --git a/Documentation/config/difftool.adoc b/Documentation/config/difftool.adoc index 4f7d40ce242b78..1b8d48381357aa 100644 --- a/Documentation/config/difftool.adoc +++ b/Documentation/config/difftool.adoc @@ -1,43 +1,43 @@ -diff.tool:: +`diff.tool`:: Controls which diff tool is used by linkgit:git-difftool[1]. This variable overrides the value configured in `merge.tool`. The list below shows the valid built-in values. Any other value is treated as a custom diff tool and requires - that a corresponding difftool..cmd variable is defined. + that a corresponding `difftool..cmd` variable is defined. -diff.guitool:: +`diff.guitool`:: Controls which diff tool is used by linkgit:git-difftool[1] when - the -g/--gui flag is specified. This variable overrides the value + the `-g`/`--gui` flag is specified. This variable overrides the value configured in `merge.guitool`. The list below shows the valid built-in values. Any other value is treated as a custom diff tool - and requires that a corresponding difftool..cmd variable + and requires that a corresponding `difftool..cmd` variable is defined. include::{build_dir}/mergetools-diff.adoc[] -difftool..cmd:: +`difftool..cmd`:: Specify the command to invoke the specified diff tool. The specified command is evaluated in shell with the following - variables available: 'LOCAL' is set to the name of the temporary - file containing the contents of the diff pre-image and 'REMOTE' + variables available: `LOCAL` is set to the name of the temporary + file containing the contents of the diff pre-image and `REMOTE` is set to the name of the temporary file containing the contents of the diff post-image. + See the `--tool=` option in linkgit:git-difftool[1] for more details. -difftool..path:: +`difftool..path`:: Override the path for the given tool. This is useful in case your tool is not in the PATH. -difftool.trustExitCode:: +`difftool.trustExitCode`:: Exit difftool if the invoked diff tool returns a non-zero exit status. + See the `--trust-exit-code` option in linkgit:git-difftool[1] for more details. -difftool.prompt:: +`difftool.prompt`:: Prompt before each invocation of the diff tool. -difftool.guiDefault:: +`difftool.guiDefault`:: Set `true` to use the `diff.guitool` by default (equivalent to specifying the `--gui` argument), or `auto` to select `diff.guitool` or `diff.tool` depending on the presence of a `DISPLAY` environment variable value. The diff --git a/Documentation/config/mergetool.adoc b/Documentation/config/mergetool.adoc index 7064f5a462cb56..7afdcad92b3934 100644 --- a/Documentation/config/mergetool.adoc +++ b/Documentation/config/mergetool.adoc @@ -52,13 +52,13 @@ if `merge.tool` is configured as __), Git will consult `mergetool..layout` to determine the tool's layout. If the variant-specific configuration is not available, `vimdiff` ' s is used as - fallback. If that too is not available, a default layout with 4 windows - will be used. To configure the layout, see the 'BACKEND SPECIFIC HINTS' + fallback. If that too is not available, a default layout with 4 windows + will be used. ifdef::git-mergetool[] - section. +To configure the layout, see the 'BACKEND SPECIFIC HINTS' section. endif::[] ifndef::git-mergetool[] - section in linkgit:git-mergetool[1]. +To configure the layout, see the 'BACKEND SPECIFIC HINTS' section in linkgit:git-mergetool[1]. endif::[] `mergetool.hideResolved`:: diff --git a/Documentation/git-difftool.adoc b/Documentation/git-difftool.adoc index 064bc683471f21..dd7cacf95e35df 100644 --- a/Documentation/git-difftool.adoc +++ b/Documentation/git-difftool.adoc @@ -7,64 +7,64 @@ git-difftool - Show changes using common diff tools SYNOPSIS -------- -[verse] -'git difftool' [] [ []] [--] [...] +[synopsis] +git difftool [] [ []] [--] [...] DESCRIPTION ----------- -'git difftool' is a Git command that allows you to compare and edit files -between revisions using common diff tools. 'git difftool' is a frontend -to 'git diff' and accepts the same options and arguments. See +`git difftool` is a Git command that allows you to compare and edit files +between revisions using common diff tools. `git difftool` is a frontend +to `git diff` and accepts the same options and arguments. See linkgit:git-diff[1]. OPTIONS ------- --d:: ---dir-diff:: +`-d`:: +`--dir-diff`:: Copy the modified files to a temporary location and perform a directory diff on them. This mode never prompts before launching the diff tool. --y:: ---no-prompt:: +`-y`:: +`--no-prompt`:: Do not prompt before launching a diff tool. ---prompt:: +`--prompt`:: Prompt before each invocation of the diff tool. This is the default behaviour; the option is provided to override any configuration settings. ---rotate-to=:: - Start showing the diff for the given path, +`--rotate-to=`:: + Start showing the diff for __, the paths before it will move to the end and output. ---skip-to=:: - Start showing the diff for the given path, skipping all +`--skip-to=`:: + Start showing the diff for __, skipping all the paths before it. --t :: ---tool=:: - Use the diff tool specified by . Valid values include +`-t `:: +`--tool=`:: + Use the diff tool specified by __. Valid values include emerge, kompare, meld, and vimdiff. Run `git difftool --tool-help` - for the list of valid settings. + for the list of valid __ settings. + -If a diff tool is not specified, 'git difftool' +If a diff tool is not specified, `git difftool` will use the configuration variable `diff.tool`. If the -configuration variable `diff.tool` is not set, 'git difftool' +configuration variable `diff.tool` is not set, `git difftool` will pick a suitable default. + You can explicitly provide a full path to the tool by setting the configuration variable `difftool..path`. For example, you can configure the absolute path to kdiff3 by setting -`difftool.kdiff3.path`. Otherwise, 'git difftool' assumes the +`difftool.kdiff3.path`. Otherwise, `git difftool` assumes the tool is available in PATH. + Instead of running one of the known diff tools, -'git difftool' can be customized to run an alternative program +`git difftool` can be customized to run an alternative program by specifying the command line to invoke in a configuration variable `difftool..cmd`. + -When 'git difftool' is invoked with this tool (either through the +When `git difftool` is invoked with this tool (either through the `-t` or `--tool` option or the `diff.tool` configuration variable) the configured command line will be invoked with the following variables available: `$LOCAL` is set to the name of the temporary @@ -74,30 +74,30 @@ of the diff post-image. `$MERGED` is the name of the file which is being compared. `$BASE` is provided for compatibility with custom merge tool commands and has the same value as `$MERGED`. ---tool-help:: +`--tool-help`:: Print a list of diff tools that may be used with `--tool`. ---symlinks:: ---no-symlinks:: - 'git difftool''s default behavior is to create symlinks to the +`--symlinks`:: +`--no-symlinks`:: + `git difftool`'s default behavior is to create symlinks to the working tree when run in `--dir-diff` mode and the right-hand side of the comparison yields the same content as the file in the working tree. + -Specifying `--no-symlinks` instructs 'git difftool' to create copies +Specifying `--no-symlinks` instructs `git difftool` to create copies instead. `--no-symlinks` is the default on Windows. --x :: ---extcmd=:: +`-x `:: +`--extcmd=`:: Specify a custom command for viewing diffs. - 'git-difftool' ignores the configured defaults and runs + `git-difftool` ignores the configured defaults and runs ` $LOCAL $REMOTE` when this option is specified. Additionally, `$BASE` is set in the environment. --g:: ---gui:: ---no-gui:: - When 'git-difftool' is invoked with the `-g` or `--gui` option +`-g`:: +`--gui`:: +`--no-gui`:: + When `git-difftool` is invoked with the `-g` or `--gui` option the default diff tool will be read from the configured `diff.guitool` variable instead of `diff.tool`. This may be selected automatically using the configuration variable @@ -106,20 +106,20 @@ instead. `--no-symlinks` is the default on Windows. fallback in the order of `merge.guitool`, `diff.tool`, `merge.tool` until a tool is found. ---trust-exit-code:: ---no-trust-exit-code:: +`--trust-exit-code`:: +`--no-trust-exit-code`:: Errors reported by the diff tool are ignored by default. - Use `--trust-exit-code` to make 'git-difftool' exit when an + Use `--trust-exit-code` to make `git-difftool` exit when an invoked diff tool returns a non-zero exit code. + -'git-difftool' will forward the exit code of the invoked tool when +`git-difftool` will forward the exit code of the invoked tool when `--trust-exit-code` is used. See linkgit:git-diff[1] for the full list of supported options. CONFIGURATION ------------- -'git difftool' falls back to 'git mergetool' config variables when the +`git difftool` falls back to `git mergetool` config variables when the difftool equivalents have not been defined. include::includes/cmd-config-section-rest.adoc[] From 5594be68eaa0fc9c87f7a50be09b85762415f070 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:45 +0000 Subject: [PATCH 41/61] doc: convert git-range-diff manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands and options to synopsis style * use __ for arguments * small style fixes Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/git-range-diff.adoc | 50 +++++++++++++++---------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc index b5e85d37f1bee7..880557084533fb 100644 --- a/Documentation/git-range-diff.adoc +++ b/Documentation/git-range-diff.adoc @@ -7,8 +7,8 @@ git-range-diff - Compare two commit ranges (e.g. two versions of a branch) SYNOPSIS -------- -[verse] -'git range-diff' [--color=[]] [--no-color] [] +[synopsis] +git range-diff [--color=[]] [--no-color] [] [--no-dual-color] [--creation-factor=] [--left-only | --right-only] [--diff-merges=] [--remerge-diff] @@ -21,14 +21,14 @@ DESCRIPTION This command shows the differences between two versions of a patch series, or more generally, two commit ranges (ignoring merge commits). -In the presence of `` arguments, these commit ranges are limited +In the presence of __ arguments, these commit ranges are limited accordingly. To that end, it first finds pairs of commits from both commit ranges that correspond with each other. Two commits are said to correspond when the diff between their patches (i.e. the author information, the commit message and the commit diff) is reasonably small compared to the -patches' size. See ``Algorithm`` below for details. +patches' size. See 'Algorithm' below for details. Finally, the list of matching commits is shown in the order of the second commit range, with unmatched commits being inserted just after @@ -37,7 +37,7 @@ all of their ancestors have been shown. There are three ways to specify the commit ranges: - ` `: Either commit range can be of the form - `..`, `^!` or `^-`. See `SPECIFYING RANGES` + `..`, `^!` or `^-`. See 'SPECIFYING RANGES' in linkgit:gitrevisions[7] for more details. - `...`. This is equivalent to @@ -48,7 +48,7 @@ There are three ways to specify the commit ranges: OPTIONS ------- ---no-dual-color:: +`--no-dual-color`:: When the commit diffs differ, `git range-diff` recreates the original diffs' coloring, and adds outer -/+ diff markers with the *background* being red/green to make it easier to see e.g. @@ -56,33 +56,33 @@ OPTIONS + Additionally, the commit diff lines that are only present in the first commit range are shown "dimmed" (this can be overridden using the `color.diff.` -config setting where `` is one of `contextDimmed`, `oldDimmed` and +config setting where __ is one of `contextDimmed`, `oldDimmed` and `newDimmed`), and the commit diff lines that are only present in the second commit range are shown in bold (which can be overridden using the config -settings `color.diff.` with `` being one of `contextBold`, +settings `color.diff.` with __ being one of `contextBold`, `oldBold` or `newBold`). + This is known to `range-diff` as "dual coloring". Use `--no-dual-color` to revert to color all lines according to the outer diff markers (and completely ignore the inner diff when it comes to color). ---creation-factor=:: - Set the creation/deletion cost fudge factor to ``. +`--creation-factor=`:: + Set the creation/deletion cost fudge factor to __. Defaults to 60. Try a larger value if `git range-diff` erroneously considers a large change a total rewrite (deletion of one commit and addition of another), and a smaller one in the reverse case. - See the ``Algorithm`` section below for an explanation of why this is + See the 'Algorithm' section below for an explanation of why this is needed. ---left-only:: +`--left-only`:: Suppress commits that are missing from the first specified range - (or the "left range" when using the `...` format). + (or the "left range" when using the `...` form). ---right-only:: +`--right-only`:: Suppress commits that are missing from the second specified range - (or the "right range" when using the `...` format). + (or the "right range" when using the `...` form). ---diff-merges=:: +`--diff-merges=`:: Instead of ignoring merge commits, generate diffs for them using the corresponding `--diff-merges=` option of linkgit:git-log[1], and include them in the comparison. @@ -93,30 +93,30 @@ have produced. In other words, if a merge commit is the result of a non-conflicting `git merge`, the `remerge` mode will represent it with an empty diff. ---remerge-diff:: +`--remerge-diff`:: Convenience option, equivalent to `--diff-merges=remerge`. ---notes[=]:: ---no-notes:: +`--notes[=]`:: +`--no-notes`:: This flag is passed to the `git log` program (see linkgit:git-log[1]) that generates the patches. - :: +` `:: Compare the commits specified by the two ranges, where - `` is considered an older version of ``. + __ is considered an older version of __. -...:: +`...`:: Equivalent to passing `..` and `..`. - :: +` `:: Equivalent to passing `..` and `..`. - Note that `` does not need to be the exact branch point + Note that __ does not need to be the exact branch point of the branches. Example: after rebasing a branch `my-topic`, `git range-diff my-topic@{u} my-topic@{1} my-topic` would show the differences introduced by the rebase. `git range-diff` also accepts the regular diff options (see -linkgit:git-diff[1]), most notably the `--color=[]` and +linkgit:git-diff[1]), most notably the `--color[=]` and `--no-color` options. These options are used when generating the "diff between patches", i.e. to compare the author, commit message and diff of corresponding old/new commits. There is currently no means to tweak most of the From f4c1b8e3fe855355f3e4c84d3e1a50b9957bd240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:46 +0000 Subject: [PATCH 42/61] doc: convert git-shortlog manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands and options to synopsis style * use __ for arguments * small style fixes Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/git-shortlog.adoc | 60 ++++++++++++++++----------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/Documentation/git-shortlog.adoc b/Documentation/git-shortlog.adoc index a11b57c1cd7b2d..e067d39b3880a8 100644 --- a/Documentation/git-shortlog.adoc +++ b/Documentation/git-shortlog.adoc @@ -3,63 +3,63 @@ git-shortlog(1) NAME ---- -git-shortlog - Summarize 'git log' output +git-shortlog - Summarize `git log` output SYNOPSIS -------- -[verse] -'git shortlog' [] [] [[--] ...] -git log --pretty=short | 'git shortlog' [] +[synopsis] +git shortlog [] [] [[--] ...] +git log --pretty=short | git shortlog [] DESCRIPTION ----------- -Summarizes 'git log' output in a format suitable for inclusion +Summarizes `git log` output in a format suitable for inclusion in release announcements. Each commit will be grouped by author and title. Additionally, "[PATCH]" will be stripped from the commit description. If no revisions are passed on the command line and either standard input -is not a terminal or there is no current branch, 'git shortlog' will +is not a terminal or there is no current branch, `git shortlog` will output a summary of the log read from standard input, without reference to the current repository. OPTIONS ------- --n:: ---numbered:: +`-n`:: +`--numbered`:: Sort output according to the number of commits per author instead of author alphabetic order. --s:: ---summary:: +`-s`:: +`--summary`:: Suppress commit description and provide a commit count summary only. --e:: ---email:: +`-e`:: +`--email`:: Show the email address of each author. ---format[=]:: +`--format[=]`:: Instead of the commit subject, use some other information to - describe each commit. '' can be any string accepted - by the `--format` option of 'git log', such as '* [%h] %s'. - (See the "PRETTY FORMATS" section of linkgit:git-log[1].) + describe each commit. __ can be any string accepted + by the `--format` option of `git log`, such as '* [%h] %s'. + (See the 'PRETTY FORMATS' section of linkgit:git-log[1].) + Each pretty-printed commit will be rewrapped before it is shown. ---date=:: +`--date=`:: Show dates formatted according to the given date string. (See - the `--date` option in the "Commit Formatting" section of + the `--date` option in the 'Commit Formatting' section of linkgit:git-log[1]). Useful with `--group=format:`. ---group=:: - Group commits based on ``. If no `--group` option is - specified, the default is `author`. `` is one of: +`--group=`:: + Group commits based on __. If no `--group` option is + specified, the default is `author`. __ is one of: + -- - `author`, commits are grouped by author - `committer`, commits are grouped by committer (the same as `-c`) - - `trailer:`, the `` is interpreted as a case-insensitive + - `trailer:`, the __ is interpreted as a case-insensitive commit message trailer (see linkgit:git-interpret-trailers[1]). For example, if your project uses `Reviewed-by` trailers, you might want to see who has been reviewing with @@ -76,7 +76,7 @@ unless the `--email` option is specified. If the value cannot be parsed as an identity, it will be taken literally and completely. - `format:`, any string accepted by the `--format` option of - 'git log'. (See the "PRETTY FORMATS" section of + `git log`. (See the 'PRETTY FORMATS' section of linkgit:git-log[1].) -- + @@ -85,11 +85,11 @@ value (but again, only once per unique value in that commit). For example, `git shortlog --group=author --group=trailer:co-authored-by` counts both authors and co-authors. --c:: ---committer:: +`-c`:: +`--committer`:: This is an alias for `--group=committer`. --w[[,[,]]]:: +`-w[[,[,]]]`:: Linewrap the output by wrapping each line at `width`. The first line of each entry is indented by `indent1` spaces, and the second and subsequent lines are indented by `indent2` spaces. `width`, @@ -98,16 +98,16 @@ counts both authors and co-authors. If width is `0` (zero) then indent the lines of the output without wrapping them. -:: +``:: Show only commits in the specified revision range. When no - is specified, it defaults to `HEAD` (i.e. the + __ is specified, it defaults to `HEAD` (i.e. the whole history leading to the current commit). `origin..HEAD` specifies all the commits reachable from the current commit (i.e. `HEAD`), but not from `origin`. For a complete list of - ways to spell , see the "Specifying Ranges" + ways to spell __, see the 'Specifying Ranges' section of linkgit:gitrevisions[7]. -[--] ...:: +`[--] ...`:: Consider only commits that are enough to explain how the files that match the specified paths came to be. + From 80f4b802e964559c65b08641c07a8acb95d0617e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 4 Apr 2026 17:12:47 +0000 Subject: [PATCH 43/61] doc: convert git-describe manual page to synopsis style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * convert commands and options to synopsis style * use __ for arguments Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/git-describe.adoc | 96 ++++++++++++++++----------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/Documentation/git-describe.adoc b/Documentation/git-describe.adoc index 08ff715709ccd1..b2cb1e47e46c67 100644 --- a/Documentation/git-describe.adoc +++ b/Documentation/git-describe.adoc @@ -7,10 +7,10 @@ git-describe - Give an object a human readable name based on an available ref SYNOPSIS -------- -[verse] -'git describe' [--all] [--tags] [--contains] [--abbrev=] [...] -'git describe' [--all] [--tags] [--contains] [--abbrev=] --dirty[=] -'git describe' +[synopsis] +git describe [--all] [--tags] [--contains] [--abbrev=] [...] +git describe [--all] [--tags] [--contains] [--abbrev=] --dirty[=] +git describe DESCRIPTION ----------- @@ -22,70 +22,70 @@ abbreviated object name of the most recent commit. The result is a "human-readable" object name which can also be used to identify the commit to other git commands. -By default (without --all or --tags) `git describe` only shows +By default (without `--all` or `--tags`) `git describe` only shows annotated tags. For more information about creating annotated tags -see the -a and -s options to linkgit:git-tag[1]. +see the `-a` and `-s` options to linkgit:git-tag[1]. If the given object refers to a blob, it will be described as `:`, such that the blob can be found -at `` in the ``, which itself describes the +at __ in the __, which itself describes the first commit in which this blob occurs in a reverse revision walk -from HEAD. +from `HEAD`. OPTIONS ------- -...:: - Commit-ish object names to describe. Defaults to HEAD if omitted. +`...`:: + Commit-ish object names to describe. Defaults to `HEAD` if omitted. ---dirty[=]:: ---broken[=]:: +`--dirty[=]`:: +`--broken[=]`:: Describe the state of the working tree. When the working - tree matches HEAD, the output is the same as "git describe - HEAD". If the working tree has local modification "-dirty" + tree matches `HEAD`, the output is the same as `git describe HEAD`. + If the working tree has local modification, `-dirty` is appended to it. If a repository is corrupt and Git cannot determine if there is local modification, Git will - error out, unless `--broken' is given, which appends - the suffix "-broken" instead. + error out, unless `--broken` is given, which appends + the suffix `-broken` instead. ---all:: +`--all`:: Instead of using only the annotated tags, use any ref found in `refs/` namespace. This option enables matching any known branch, remote-tracking branch, or lightweight tag. ---tags:: +`--tags`:: Instead of using only the annotated tags, use any tag found in `refs/tags` namespace. This option enables matching a lightweight (non-annotated) tag. ---contains:: +`--contains`:: Instead of finding the tag that predates the commit, find the tag that comes after the commit, and thus contains it. - Automatically implies --tags. + Automatically implies `--tags`. ---abbrev=:: +`--abbrev=`:: Instead of using the default number of hexadecimal digits (which will vary according to the number of objects in the repository with - a default of 7) of the abbreviated object name, use digits, or - as many digits as needed to form a unique object name. An of 0 + a default of 7) of the abbreviated object name, use __ digits, or + as many digits as needed to form a unique object name. An __ of 0 will suppress long format, only showing the closest tag. ---candidates=:: +`--candidates=`:: Instead of considering only the 10 most recent tags as candidates to describe the input commit-ish consider - up to candidates. Increasing above 10 will take + up to __ candidates. Increasing __ above 10 will take slightly longer but may produce a more accurate result. - An of 0 will cause only exact matches to be output. + An __ of 0 will cause only exact matches to be output. ---exact-match:: +`--exact-match`:: Only output exact matches (a tag directly references the - supplied commit). This is a synonym for --candidates=0. + supplied commit). This is a synonym for `--candidates=0`. ---debug:: +`--debug`:: Verbosely display information about the searching strategy being employed to standard error. The tag name will still be printed to standard out. ---long:: +`--long`:: Always output the long format (the tag, the number of commits and the abbreviated commit name) even when it matches a tag. This is useful when you want to see parts of the commit object name @@ -94,8 +94,8 @@ OPTIONS describe such a commit as v1.2-0-gdeadbee (0th commit since tag v1.2 that points at object deadbee....). ---match :: - Only consider tags matching the given `glob(7)` pattern, +`--match `:: + Only consider tags matching the given `glob`(7) pattern, excluding the "refs/tags/" prefix. If used with `--all`, it also considers local branches and remote-tracking references matching the pattern, excluding respectively "refs/heads/" and "refs/remotes/" @@ -104,22 +104,22 @@ OPTIONS matching any of the patterns will be considered. Use `--no-match` to clear and reset the list of patterns. ---exclude :: - Do not consider tags matching the given `glob(7)` pattern, excluding +`--exclude `:: + Do not consider tags matching the given `glob`(7) pattern, excluding the "refs/tags/" prefix. If used with `--all`, it also does not consider local branches and remote-tracking references matching the pattern, - excluding respectively "refs/heads/" and "refs/remotes/" prefix; + excluding respectively "`refs/heads/`" and "`refs/remotes/`" prefix; references of other types are never considered. If given multiple times, a list of patterns will be accumulated and tags matching any of the - patterns will be excluded. When combined with --match a tag will be - considered when it matches at least one --match pattern and does not - match any of the --exclude patterns. Use `--no-exclude` to clear and + patterns will be excluded. When combined with `--match` a tag will be + considered when it matches at least one `--match` pattern and does not + match any of the `--exclude` patterns. Use `--no-exclude` to clear and reset the list of patterns. ---always:: +`--always`:: Show uniquely abbreviated commit object as fallback. ---first-parent:: +`--first-parent`:: Follow only the first parent commit upon seeing a merge commit. This is useful when you wish to not match tags on branches merged in the history of the target commit. @@ -139,8 +139,8 @@ an abbreviated object name for the commit itself ("2414721") at the end. The number of additional commits is the number -of commits which would be displayed by "git log v1.0.4..parent". -The hash suffix is "-g" + an unambiguous abbreviation for the tip commit +of commits which would be displayed by `git log v1.0.4..parent`. +The hash suffix is "`-g`" + an unambiguous abbreviation for the tip commit of parent (which was `2414721b194453f058079d897d13c4e377f92dc6`). The length of the abbreviation scales as the repository grows, using the approximate number of objects in the repository and a bit of math @@ -149,12 +149,12 @@ The "g" prefix stands for "git" and is used to allow describing the version of a software depending on the SCM the software is managed with. This is useful in an environment where people may use different SCMs. -Doing a 'git describe' on a tag-name will just show the tag name: +Doing a `git describe` on a tag-name will just show the tag name: [torvalds@g5 git]$ git describe v1.0.4 v1.0.4 -With --all, the command can use branch heads as references, so +With `--all`, the command can use branch heads as references, so the output shows the reference path as well: [torvalds@g5 git]$ git describe --all --abbrev=4 v1.0.5^2 @@ -163,7 +163,7 @@ the output shows the reference path as well: [torvalds@g5 git]$ git describe --all --abbrev=4 HEAD^ heads/lt/describe-7-g975b -With --abbrev set to 0, the command can be used to find the +With `--abbrev` set to 0, the command can be used to find the closest tagname without any suffix: [torvalds@g5 git]$ git describe --abbrev=0 v1.0.5^2 @@ -179,13 +179,13 @@ be sufficient to disambiguate these commits. SEARCH STRATEGY --------------- -For each commit-ish supplied, 'git describe' will first look for +For each commit-ish supplied, `git describe` will first look for a tag which tags exactly that commit. Annotated tags will always be preferred over lightweight tags, and tags with newer dates will always be preferred over tags with older dates. If an exact match is found, its name will be output and searching will stop. -If an exact match was not found, 'git describe' will walk back +If an exact match was not found, `git describe` will walk back through the commit history to locate an ancestor commit which has been tagged. The ancestor's tag will be output along with an abbreviation of the input commit-ish's SHA-1. If `--first-parent` was @@ -203,7 +203,7 @@ BUGS Tree objects as well as tag objects not pointing at commits, cannot be described. When describing blobs, the lightweight tags pointing at blobs are ignored, -but the blob is still described as : despite the lightweight +but the blob is still described as `:` despite the lightweight tag being favorable. GIT From 5bdb9883bda41024e3c47f1cddf69977d82fe3a9 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Sun, 5 Apr 2026 12:31:59 +0200 Subject: [PATCH 44/61] doc: replace git config --list/-l with `list` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace uses of `git config --list` (short or long) with the subcommand `list` since `--list` is deprecated. We will change the “man page” phrasing in gitcvs-migration(7) in the next commit, since we are already visiting that sentence. But note that we leave the “man page” phrasing in the sentence that we touch in gittutorial(7) since it’s a tutorial and not a manual page. We can be more wordy in a tutorial context. Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/git-var.adoc | 2 +- Documentation/gitcvs-migration.adoc | 2 +- Documentation/gitprotocol-v2.adoc | 2 +- Documentation/gittutorial.adoc | 2 +- Documentation/technical/api-trace2.adoc | 2 +- Documentation/user-manual.adoc | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/git-var.adoc b/Documentation/git-var.adoc index b606c2d649979f..697c10adedcca6 100644 --- a/Documentation/git-var.adoc +++ b/Documentation/git-var.adoc @@ -22,7 +22,7 @@ OPTIONS Display the logical variables. In addition, all the variables of the Git configuration file .git/config are listed as well. (However, the configuration variables listing functionality - is deprecated in favor of `git config -l`.) + is deprecated in favor of `git config list`.) EXAMPLES -------- diff --git a/Documentation/gitcvs-migration.adoc b/Documentation/gitcvs-migration.adoc index 1cd1283d0f817d..2883834b714701 100644 --- a/Documentation/gitcvs-migration.adoc +++ b/Documentation/gitcvs-migration.adoc @@ -49,7 +49,7 @@ them first before running git pull. ================================ The 'pull' command knows where to get updates from because of certain configuration variables that were set by the first 'git clone' -command; see `git config -l` and the linkgit:git-config[1] man +command; see `git config list` and the linkgit:git-config[1] man page for details. ================================ diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc index f985cb4c474953..ac5102b64f99ec 100644 --- a/Documentation/gitprotocol-v2.adoc +++ b/Documentation/gitprotocol-v2.adoc @@ -659,7 +659,7 @@ use by the client, MUST indicate prerequisites (in any) with standard applicable. + The advertised URI may alternatively contain a plaintext file that `git -config --list` would accept (with the `--file` option). The key-value +config list` would accept (with the `--file` option). The key-value pairs in this list are in the `bundle.*` namespace (see linkgit:git-config[1]). diff --git a/Documentation/gittutorial.adoc b/Documentation/gittutorial.adoc index f89ad30cf652b4..519b8d8be2cc46 100644 --- a/Documentation/gittutorial.adoc +++ b/Documentation/gittutorial.adoc @@ -432,7 +432,7 @@ bob$ git config --get remote.origin.url ------------------------------------- (The complete configuration created by `git clone` is visible using -`git config -l`, and the linkgit:git-config[1] man page +`git config list`, and the linkgit:git-config[1] man page explains the meaning of each option.) Git also keeps a pristine copy of Alice's `master` branch under the diff --git a/Documentation/technical/api-trace2.adoc b/Documentation/technical/api-trace2.adoc index cf493dae03f3ac..918e517c2e6edb 100644 --- a/Documentation/technical/api-trace2.adoc +++ b/Documentation/technical/api-trace2.adoc @@ -1253,7 +1253,7 @@ it. $ git config --system color.ui never $ git config --global color.ui always $ git config --local color.ui auto -$ git config --list --show-scope | grep 'color.ui' +$ git config list --show-scope | grep 'color.ui' system color.ui=never global color.ui=always local color.ui=auto diff --git a/Documentation/user-manual.adoc b/Documentation/user-manual.adoc index 64009baf370231..5ec65cebe29dd7 100644 --- a/Documentation/user-manual.adoc +++ b/Documentation/user-manual.adoc @@ -2865,7 +2865,7 @@ stored in Git configuration variables, which you can see using linkgit:git-config[1]: ------------------------------------------------- -$ git config -l +$ git config list core.repositoryformatversion=0 core.filemode=true core.logallrefupdates=true From 57177ad139c3d83116459c5a8ec323b8d2a050f4 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Sun, 5 Apr 2026 12:32:00 +0200 Subject: [PATCH 45/61] =?UTF-8?q?doc:=20gitcvs-migration:=20rephrase=20?= =?UTF-8?q?=E2=80=9Cman=20page=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let’s change the phrasing around the `linkgit` while we’re visiting this file (see previous commit[1]). We use the section syntax to refer to man pages, so writing “man page” next to it is a bit redundant. We can be more concise and just lean on the preposition “in”. And in order to avoid this double “git”: see `git config list` in git-config(1) ... We can rephrase to the subcommand, which is a typical pattern (config or option followed by “in git-command(1)”). † 1: Which also discusses why we do not change a similar phrasing in gittutorial(7) Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/gitcvs-migration.adoc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/gitcvs-migration.adoc b/Documentation/gitcvs-migration.adoc index 2883834b714701..905d08cd5f991c 100644 --- a/Documentation/gitcvs-migration.adoc +++ b/Documentation/gitcvs-migration.adoc @@ -49,8 +49,7 @@ them first before running git pull. ================================ The 'pull' command knows where to get updates from because of certain configuration variables that were set by the first 'git clone' -command; see `git config list` and the linkgit:git-config[1] man -page for details. +command; see the subcommand `list` in linkgit:git-config[1] for details. ================================ You can update the shared repository with your changes by first committing From 295fb82264cc8be565a5da8259b103cbc6a41728 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 6 Apr 2026 13:27:26 +0000 Subject: [PATCH 46/61] t6600: test --maximal-only and --independent Add a test that verifies the 'git rev-list --maximal-only' option produces the same set of commits as 'git merge-base --independent'. This equivalence was noted when the feature was first created, but we are about to update the implementation to use a common algorithm in this case where the user intention is identical. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/t6600-test-reach.sh | 45 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh index 2613075894282d..dc0421ed2f3726 100755 --- a/t/t6600-test-reach.sh +++ b/t/t6600-test-reach.sh @@ -837,4 +837,49 @@ test_expect_success 'rev-list --maximal-only (range)' ' --first-parent --exclude-first-parent-only ' +test_expect_success 'rev-list --maximal-only matches merge-base --independent' ' + # Mix of independent and dependent + git merge-base --independent \ + refs/heads/commit-5-2 \ + refs/heads/commit-3-2 \ + refs/heads/commit-2-5 >expect && + sort expect >expect.sorted && + git rev-list --maximal-only \ + refs/heads/commit-5-2 \ + refs/heads/commit-3-2 \ + refs/heads/commit-2-5 >actual && + sort actual >actual.sorted && + test_cmp expect.sorted actual.sorted && + + # All independent commits. + git merge-base --independent \ + refs/heads/commit-5-2 \ + refs/heads/commit-4-3 \ + refs/heads/commit-3-4 \ + refs/heads/commit-2-5 >expect && + sort expect >expect.sorted && + git rev-list --maximal-only \ + refs/heads/commit-5-2 \ + refs/heads/commit-4-3 \ + refs/heads/commit-3-4 \ + refs/heads/commit-2-5 >actual && + sort actual >actual.sorted && + test_cmp expect.sorted actual.sorted && + + # Only one independent. + git merge-base --independent \ + refs/heads/commit-1-1 \ + refs/heads/commit-4-2 \ + refs/heads/commit-4-4 \ + refs/heads/commit-8-4 >expect && + sort expect >expect.sorted && + git rev-list --maximal-only \ + refs/heads/commit-1-1 \ + refs/heads/commit-4-2 \ + refs/heads/commit-4-4 \ + refs/heads/commit-8-4 >actual && + sort actual >actual.sorted && + test_cmp expect.sorted actual.sorted +' + test_done From e8e5453ab8794cf29afe0a616d74319442b676bd Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 6 Apr 2026 13:27:27 +0000 Subject: [PATCH 47/61] p6011: add perf test for rev-list --maximal-only Add a performance test that compares 'git rev-list --maximal-only' against 'git merge-base --independent'. These two commands are asking essentially the same thing, but the rev-list implementation is more generic and hence slower. These performance tests will demonstrate that in the current state and also be used to show the equivalence in the future. We also add a case with '--since' to force the generic walk logic for rev-list even when we make that future change to use the merge-base algorithm on a simple walk. When run on my copy of git.git, I see these results: Test HEAD ---------------------------------------------- 6011.2: merge-base --independent 0.03 6011.3: rev-list --maximal-only 0.06 6011.4: rev-list --maximal-only --since 0.06 These numbers are low, but the --independent calculation is interesting due to having a lot of local branches that are actually independent. Running the same test on a fresh clone of the Linux kernel repository shows a larger difference between the algorithms, especially because the --independent algorithm is extremely fast when there are no independent references selected: Test HEAD ---------------------------------------------- 6011.2: merge-base --independent 0.00 6011.3: rev-list --maximal-only 0.70 6011.4: rev-list --maximal-only --since 0.70 Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/perf/p6011-rev-list-maximal.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100755 t/perf/p6011-rev-list-maximal.sh diff --git a/t/perf/p6011-rev-list-maximal.sh b/t/perf/p6011-rev-list-maximal.sh new file mode 100755 index 00000000000000..e868e83ff847b3 --- /dev/null +++ b/t/perf/p6011-rev-list-maximal.sh @@ -0,0 +1,29 @@ +#!/bin/sh + +test_description='Test --maximal-only and --independent options' + +. ./perf-lib.sh + +test_perf_default_repo + +test_expect_success 'setup' ' + git for-each-ref --format="%(*objecttype) %(objecttype) %(objectname)" \ + "refs/heads/*" "refs/tags/*" | + sed -n -e "s/^commit commit //p" -e "s/^ commit //p" | + head -n 50 >commits && + git commit-graph write --reachable +' + +test_perf 'merge-base --independent' ' + git merge-base --independent $(cat commits) >/dev/null +' + +test_perf 'rev-list --maximal-only' ' + git rev-list --maximal-only $(cat commits) >/dev/null +' + +test_perf 'rev-list --maximal-only --since' ' + git rev-list --maximal-only --since=2000-01-01 $(cat commits) >/dev/null +' + +test_done From b0ba57daa86dfba0a6551613452d1cb2301266a2 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 6 Apr 2026 13:27:28 +0000 Subject: [PATCH 48/61] rev-list: use reduce_heads() for --maximal-only The 'git rev-list --maximal-only' option filters the output to only independent commits. A commit is independent if it is not reachable from other listed commits. Currently this is implemented by doing a full revision walk and marking parents with CHILD_VISITED to skip non-maximal commits. The 'git merge-base --independent' command computes the same result using reduce_heads(), which uses the more efficient remove_redundant() algorithm. This is significantly faster because it avoids walking the entire commit graph. Add a fast path in rev-list that detects when --maximal-only is the only interesting option and all input commits are positive (no revision ranges). In this case, use reduce_heads() directly instead of doing a full revision walk. In order to preserve the rest of the output filtering, this computation is done opportunistically in a new prepare_maximal_independent() method when possible. If successful, it populates revs->commits with the list of independent commits and set revs->no_walk to prevent any other walk from occurring. This allows us to have any custom output be handled using the existing output code hidden inside traverse_commit_list_filtered(). A new test is added to demonstrate that this output is preserved. The fast path is only used when no other flags complicate the walk or output format: no UNINTERESTING commits, no limiting options (max-count, age filters, path filters, grep filters), no output formatting beyond plain OIDs, and no object listing flags. Running the p6011 performance test for my copy of git.git, I see the following improvement with this change: Test HEAD~1 HEAD ------------------------------------------------------------ 6011.2: merge-base --independent 0.03 0.03 +0.0% 6011.3: rev-list --maximal-only 0.06 0.03 -50.0% 6011.4: rev-list --maximal-only --since 0.06 0.06 +0.0% And for a fresh clone of the Linux kernel repository, I see: Test HEAD~1 HEAD ------------------------------------------------------------ 6011.2: merge-base --independent 0.00 0.00 = 6011.3: rev-list --maximal-only 0.70 0.00 -100.0% 6011.4: rev-list --maximal-only --since 0.70 0.70 +0.0% In both cases, the performance is indeed matching the behavior of 'git merge-base --independent', as expected. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/rev-list.c | 59 ++++++++++++++++++++++++++++++++++++++++ t/t6000-rev-list-misc.sh | 31 +++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 854d82ece368b3..8f63003709242e 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -25,6 +25,7 @@ #include "oidset.h" #include "oidmap.h" #include "packfile.h" +#include "commit-reach.h" #include "quote.h" #include "strbuf.h" @@ -633,6 +634,61 @@ static int try_bitmap_disk_usage(struct rev_info *revs, return 0; } +/* + * If revs->maximal_only is set and no other walk modifiers are provided, + * run a faster computation to filter the independent commits and prepare + * them for output. Set revs->no_walk to prevent later walking. + * + * If this algorithm doesn't apply, then no changes are made to revs. + */ +static void prepare_maximal_independent(struct rev_info *revs) +{ + struct commit_list *c; + + if (!revs->maximal_only) + return; + + for (c = revs->commits; c; c = c->next) { + if (c->item->object.flags & UNINTERESTING) + return; + } + + if (revs->limited || + revs->topo_order || + revs->first_parent_only || + revs->reverse || + revs->max_count >= 0 || + revs->skip_count >= 0 || + revs->min_age != (timestamp_t)-1 || + revs->max_age != (timestamp_t)-1 || + revs->min_parents > 0 || + revs->max_parents >= 0 || + revs->prune_data.nr || + revs->count || + revs->left_right || + revs->boundary || + revs->tag_objects || + revs->tree_objects || + revs->blob_objects || + revs->filter.choice || + revs->reflog_info || + revs->diff || + revs->grep_filter.pattern_list || + revs->grep_filter.header_list || + revs->verbose_header || + revs->print_parents || + revs->edge_hint || + revs->unpacked || + revs->no_kept_objects || + revs->line_level_traverse) + return; + + reduce_heads_replace(&revs->commits); + + /* Modify 'revs' to only output this commit list. */ + revs->no_walk = 1; +} + int cmd_rev_list(int argc, const char **argv, const char *prefix, @@ -875,6 +931,9 @@ int cmd_rev_list(int argc, if (prepare_revision_walk(&revs)) die("revision walk setup failed"); + + prepare_maximal_independent(&revs); + if (revs.tree_objects) mark_edges_uninteresting(&revs, show_edge, 0); diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh index d0a2a866100d56..a95ba576fa24b2 100755 --- a/t/t6000-rev-list-misc.sh +++ b/t/t6000-rev-list-misc.sh @@ -263,4 +263,35 @@ test_expect_success 'rev-list --boundary incompatible with --maximal-only' ' test_grep "cannot be used together" err ' +test_expect_success 'rev-list --maximal-only and --pretty' ' + test_when_finished rm -rf repo && + + git init repo && + test_commit -C repo 1 && + oid1=$(git -C repo rev-parse HEAD) && + test_commit -C repo 2 && + oid2=$(git -C repo rev-parse HEAD) && + git -C repo checkout --detach HEAD~1 && + test_commit -C repo 3 && + oid3=$(git -C repo rev-parse HEAD) && + + cat >expect <<-EOF && + commit $oid3 + $oid3 + commit $oid2 + $oid2 + EOF + + git -C repo rev-list --pretty="%H" --maximal-only $oid1 $oid2 $oid3 >out && + test_cmp expect out && + + cat >expect <<-EOF && + $oid3 + $oid2 + EOF + + git -C repo log --pretty="%H" --maximal-only $oid1 $oid2 $oid3 >out && + test_cmp expect out +' + test_done From fa1468a1f7c7765a6c7dd1faca4c9dc241d0538c Mon Sep 17 00:00:00 2001 From: Trieu Huynh Date: Tue, 7 Apr 2026 03:30:41 +0900 Subject: [PATCH 49/61] promisor-remote: fix promisor.quiet to use the correct repository fetch_objects() reads the promisor.quiet configuration from the_repository instead of the repo parameter it receives. This means that when git lazy-fetches objects for a non-main repository, eg. a submodule that is itself a partial clone opened via repo_submodule_init(). The submodule's own promisor.quiet setting is ignored and the superproject's setting is used instead. Fix by replacing the_repository with repo in the repo_config_get_bool() call. The practical trigger is git grep --recurse-submodules on a superproject where the submodule is a partial clone. Add a test where promisor.quiet is set only in a partial-clone submodule; a lazy fetch triggered by "git grep --recurse-submodules" must honor that setting. Signed-off-by: Trieu Huynh Signed-off-by: Junio C Hamano --- promisor-remote.c | 2 +- t/t0410-partial-clone.sh | 45 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 96fa215b06a924..225260b05f8d65 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -46,7 +46,7 @@ static int fetch_objects(struct repository *repo, "fetch", remote_name, "--no-tags", "--no-write-fetch-head", "--recurse-submodules=no", "--filter=blob:none", "--stdin", NULL); - if (!repo_config_get_bool(the_repository, "promisor.quiet", &quiet) && quiet) + if (!repo_config_get_bool(repo, "promisor.quiet", &quiet) && quiet) strvec_push(&child.args, "--quiet"); if (start_command(&child)) die(_("promisor-remote: unable to fork off fetch subprocess")); diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 52e19728a3fca0..dff442da2090b5 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -717,7 +717,29 @@ test_expect_success 'setup for promisor.quiet tests' ' git -C server rm foo.t && git -C server commit -m remove && git -C server config uploadpack.allowanysha1inwant 1 && - git -C server config uploadpack.allowfilter 1 + git -C server config uploadpack.allowfilter 1 && + + # Setup for submodule repo test: superproject whose submodule is a + # partial clone, so that promisor.quiet is read via a non-main repo. + rm -rf sub-pc-src sub-pc-srv.bare super-src super-work && + git init sub-pc-src && + test_commit -C sub-pc-src initial file.txt "hello" && + + git clone --bare sub-pc-src sub-pc-srv.bare && + git -C sub-pc-srv.bare config uploadpack.allowfilter 1 && + git -C sub-pc-srv.bare config uploadpack.allowanysha1inwant 1 && + + git init super-src && + git -C super-src -c protocol.file.allow=always \ + submodule add "file://$(pwd)/sub-pc-srv.bare" sub && + git -C super-src commit -m "add submodule" && + + git -c protocol.file.allow=always clone super-src super-work && + git -C super-work -c protocol.file.allow=always \ + submodule update --init --filter=blob:none sub && + + # Allow file:// in the submodule so that lazy-fetch subprocesses work. + git -C super-work/sub config protocol.file.allow always ' test_expect_success TTY 'promisor.quiet=false shows progress messages' ' @@ -752,6 +774,27 @@ test_expect_success TTY 'promisor.quiet=unconfigured shows progress messages' ' grep "Receiving objects" err ' +test_expect_success 'promisor.quiet from submodule repo is honored' ' + rm -f pc-quiet-trace && + + # Set promisor.quiet only in the submodule, not the superproject. + git -C super-work/sub config promisor.quiet true && + + # Push a new commit+blob to the server; the blob stays missing in the + # partial-clone submodule until a lazy fetch is triggered. + test_commit -C sub-pc-src updated new-file.txt "world" && + git -C sub-pc-src push "$(pwd)/sub-pc-srv.bare" HEAD:master && + git -C super-work/sub -c protocol.file.allow=always fetch origin && + git -C super-work/sub reset --mixed origin/master && + + # grep descends into the submodule and triggers a lazy fetch for the + # missing blob; verify the fetch subprocess carries --quiet. + GIT_TRACE2_EVENT="$(pwd)/pc-quiet-trace" \ + git -C super-work grep --cached --recurse-submodules "world" \ + 2>/dev/null && + grep negotiationAlgorithm pc-quiet-trace | grep -e --quiet +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd From 521731213c905f0dfec6a55393f010d185492c85 Mon Sep 17 00:00:00 2001 From: David Lin Date: Mon, 6 Apr 2026 15:27:11 -0400 Subject: [PATCH 50/61] cache-tree: fix inverted object existence check in cache_tree_fully_valid The negation in front of the object existence check in cache_tree_fully_valid() was lost in 062b914c84 (treewide: convert users of `repo_has_object_file()` to `has_object()`, 2025-04-29), turning `!repo_has_object_file(...)` into `has_object(...)` instead of `!has_object(...)`. This makes cache_tree_fully_valid() always report the cache tree as invalid when objects exist (the common case), forcing callers like write_index_as_tree() to call cache_tree_update() on every invocation. An odb_has_object() check inside update_one() avoids a full tree rebuild, but the unnecessary call still pays the cost of opening an ODB transaction and, in partial clones, a promisor remote check. Restore the missing negation and add a test that verifies write-tree takes the cache-tree shortcut when the cache tree is valid. Helped-by: Derrick Stolee Signed-off-by: David Lin Signed-off-by: Junio C Hamano --- cache-tree.c | 2 +- t/t0090-cache-tree.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/cache-tree.c b/cache-tree.c index 66ef2becbe01a4..366b1d7dcd8081 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -239,7 +239,7 @@ int cache_tree_fully_valid(struct cache_tree *it) if (!it) return 0; if (it->entry_count < 0 || - odb_has_object(the_repository->objects, &it->oid, + !odb_has_object(the_repository->objects, &it->oid, HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) return 0; for (i = 0; i < it->subtree_nr; i++) { diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh index d901588294668c..0964718d7f33f5 100755 --- a/t/t0090-cache-tree.sh +++ b/t/t0090-cache-tree.sh @@ -278,4 +278,12 @@ test_expect_success 'switching trees does not invalidate shared index' ' ) ' +test_expect_success 'cache-tree is used by write-tree when valid' ' + test_commit use-valid && + + # write-tree with a valid cache-tree should skip cache_tree_update + GIT_TRACE2_PERF="$(pwd)/trace.output" git write-tree && + test_grep ! region_enter.*cache_tree.*update trace.output +' + test_done From 8808e61fd3e953c3534633b8b5adc5b243dd696f Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:34 +0200 Subject: [PATCH 51/61] promisor-remote: try accepted remotes before others in get_direct() When a server advertises promisor remotes and the client accepts some of them, those remotes carry the server's intent: 'fetch missing objects preferably from here', and the client agrees with that for the remotes it accepts. However promisor_remote_get_direct() actually iterates over all promisor remotes in list order, which is the order they appear in the config files (except perhaps for the one appearing in the `extensions.partialClone` config variable which is tried last). This means an existing, but not accepted, promisor remote, could be tried before the accepted ones, which does not reflect the intent of the agreement between client and server. If the client doesn't care about what the server suggests, it should accept nothing and rely on its remotes as they are already configured. To better reflect the agreement between client and server, let's make promisor_remote_get_direct() try the accepted promisor remotes before the non-accepted ones. Concretely, let's extract a try_promisor_remotes() helper and call it twice from promisor_remote_get_direct(): - first with an `accepted_only=true` argument to try only the accepted remotes, - then with `accepted_only=false` to fall back to any remaining remote. Ensuring that accepted remotes are preferred will be even more important if in the future a mechanism is developed to allow the client to auto-configure remotes that the server advertises. This will in particular avoid fetching from the server (which is already configured as a promisor remote) before trying the auto-configured remotes, as these new remotes would likely appear at the end of the config file, and as the server might not appear in the `extensions.partialClone` config variable. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- Documentation/gitprotocol-v2.adoc | 4 ++ promisor-remote.c | 44 ++++++++++++----- t/t5710-promisor-remote-capability.sh | 69 +++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 13 deletions(-) diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc index f985cb4c474953..4fcb1a7bda1be7 100644 --- a/Documentation/gitprotocol-v2.adoc +++ b/Documentation/gitprotocol-v2.adoc @@ -848,6 +848,10 @@ advertised, it can reply with "promisor-remote=" where where `pr-name` is the urlencoded name of a promisor remote the server advertised and the client accepts. +The promisor remotes that the client accepted will be tried before the +other configured promisor remotes when the client attempts to fetch +missing objects. + Note that, everywhere in this document, the ';' and ',' characters MUST be encoded if they appear in `pr-name` or `field-value`. diff --git a/promisor-remote.c b/promisor-remote.c index 96fa215b06a924..7ce7d22f952e70 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -268,11 +268,35 @@ static int remove_fetched_oids(struct repository *repo, return remaining_nr; } +static int try_promisor_remotes(struct repository *repo, + struct object_id **remaining_oids, + int *remaining_nr, int *to_free, + bool accepted_only) +{ + struct promisor_remote *r = repo->promisor_remote_config->promisors; + + for (; r; r = r->next) { + if (accepted_only != r->accepted) + continue; + if (fetch_objects(repo, r->name, *remaining_oids, *remaining_nr) < 0) { + if (*remaining_nr == 1) + continue; + *remaining_nr = remove_fetched_oids(repo, remaining_oids, + *remaining_nr, *to_free); + if (*remaining_nr) { + *to_free = 1; + continue; + } + } + return 1; /* all fetched */ + } + return 0; +} + void promisor_remote_get_direct(struct repository *repo, const struct object_id *oids, int oid_nr) { - struct promisor_remote *r; struct object_id *remaining_oids = (struct object_id *)oids; int remaining_nr = oid_nr; int to_free = 0; @@ -283,19 +307,13 @@ void promisor_remote_get_direct(struct repository *repo, promisor_remote_init(repo); - for (r = repo->promisor_remote_config->promisors; r; r = r->next) { - if (fetch_objects(repo, r->name, remaining_oids, remaining_nr) < 0) { - if (remaining_nr == 1) - continue; - remaining_nr = remove_fetched_oids(repo, &remaining_oids, - remaining_nr, to_free); - if (remaining_nr) { - to_free = 1; - continue; - } - } + /* Try accepted remotes first (those the server told us to use) */ + if (try_promisor_remotes(repo, &remaining_oids, &remaining_nr, + &to_free, true)) + goto all_fetched; + if (try_promisor_remotes(repo, &remaining_oids, &remaining_nr, + &to_free, false)) goto all_fetched; - } for (i = 0; i < remaining_nr; i++) { if (is_promisor_object(repo, &remaining_oids[i])) diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh index 357822c01a7530..bf0eed9f109742 100755 --- a/t/t5710-promisor-remote-capability.sh +++ b/t/t5710-promisor-remote-capability.sh @@ -166,6 +166,75 @@ test_expect_success "init + fetch with promisor.advertise set to 'true'" ' check_missing_objects server 1 "$oid" ' +test_expect_success "clone with two promisors but only one advertised" ' + git -C server config promisor.advertise true && + test_when_finished "rm -rf client unused_lop" && + + # Create a promisor that will be configured but not be used + git init --bare unused_lop && + + # Clone from server to create a client + GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ + -c remote.unused_lop.promisor=true \ + -c remote.unused_lop.fetch="+refs/heads/*:refs/remotes/unused_lop/*" \ + -c remote.unused_lop.url="file://$(pwd)/unused_lop" \ + -c remote.lop.promisor=true \ + -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ + -c remote.lop.url="file://$(pwd)/lop" \ + -c promisor.acceptfromserver=All \ + --no-local --filter="blob:limit=5k" server client && + + # Check that "unused_lop" appears before "lop" in the config + printf "remote.%s.promisor true\n" "unused_lop" "lop" "origin" >expect && + git -C client config get --all --show-names --regexp "^remote\..*\.promisor$" >actual && + test_cmp expect actual && + + # Check that "lop" was tried + test_grep " fetch lop " trace && + # Check that "unused_lop" was not contacted + # This means "lop", the accepted promisor, was tried first + test_grep ! " fetch unused_lop " trace && + + # Check that the largest object is still missing on the server + check_missing_objects server 1 "$oid" +' + +test_expect_success "init + fetch two promisors but only one advertised" ' + git -C server config promisor.advertise true && + test_when_finished "rm -rf client unused_lop" && + + # Create a promisor that will be configured but not be used + git init --bare unused_lop && + + mkdir client && + git -C client init && + git -C client config remote.unused_lop.promisor true && + git -C client config remote.unused_lop.fetch "+refs/heads/*:refs/remotes/unused_lop/*" && + git -C client config remote.unused_lop.url "file://$(pwd)/unused_lop" && + git -C client config remote.lop.promisor true && + git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" && + git -C client config remote.lop.url "file://$(pwd)/lop" && + git -C client config remote.server.url "file://$(pwd)/server" && + git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" && + git -C client config promisor.acceptfromserver All && + + # Check that "unused_lop" appears before "lop" in the config + printf "remote.%s.promisor true\n" "unused_lop" "lop" >expect && + git -C client config get --all --show-names --regexp "^remote\..*\.promisor$" >actual && + test_cmp expect actual && + + GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git -C client fetch --filter="blob:limit=5k" server && + + # Check that "lop" was tried + test_grep " fetch lop " trace && + # Check that "unused_lop" was not contacted + # This means "lop", the accepted promisor, was tried first + test_grep ! " fetch unused_lop " trace && + + # Check that the largest object is still missing on the server + check_missing_objects server 1 "$oid" +' + test_expect_success "clone with promisor.acceptfromserver set to 'KnownName'" ' git -C server config promisor.advertise true && test_when_finished "rm -rf client" && From 720b7c26c82ef212852897bedb0d38eee78cb531 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:35 +0200 Subject: [PATCH 52/61] promisor-remote: pass config entry to all_fields_match() directly The `in_list == 0` path of all_fields_match() looks up the remote in `config_info` by `advertised->name` repeatedly, even though every caller in should_accept_remote() has already performed this lookup and holds the result in `p`. To avoid this useless work, let's replace the `int in_list` parameter with a `struct promisor_info *config_entry` pointer: - When NULL (ACCEPT_ALL mode): scan the whole `config_info` list, as the old `in_list == 1` path did. - When non-NULL: match against that single config entry directly, avoiding the redundant string_list_lookup() call. This removes the hidden dependency on `advertised->name` inside all_fields_match(), which would be wrong if in the future auto-configured remotes are implemented, as the local config name may differ from the server's advertised name. While at it, let's also add a comment before all_fields_match() and match_field_against_config() to help understand how things work and help avoid similar issues. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 7ce7d22f952e70..6c935f855af752 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -575,6 +575,12 @@ enum accept_promisor { ACCEPT_ALL }; +/* + * Check if a specific field and its advertised value match the local + * configuration of a given promisor remote. + * + * Returns 1 if they match, 0 otherwise. + */ static int match_field_against_config(const char *field, const char *value, struct promisor_info *config_info) { @@ -586,9 +592,18 @@ static int match_field_against_config(const char *field, const char *value, return 0; } +/* + * Check that the advertised fields match the local configuration. + * + * When 'config_entry' is NULL (ACCEPT_ALL mode), every checked field + * must match at least one remote in 'config_info'. + * + * When 'config_entry' points to a specific remote's config, the + * checked fields are compared against that single remote only. + */ static int all_fields_match(struct promisor_info *advertised, struct string_list *config_info, - int in_list) + struct promisor_info *config_entry) { struct string_list *fields = fields_checked(); struct string_list_item *item_checked; @@ -597,7 +612,6 @@ static int all_fields_match(struct promisor_info *advertised, int match = 0; const char *field = item_checked->string; const char *value = NULL; - struct string_list_item *item; if (!strcasecmp(field, promisor_field_filter)) value = advertised->filter; @@ -607,7 +621,11 @@ static int all_fields_match(struct promisor_info *advertised, if (!value) return 0; - if (in_list) { + if (config_entry) { + match = match_field_against_config(field, value, + config_entry); + } else { + struct string_list_item *item; for_each_string_list_item(item, config_info) { struct promisor_info *p = item->util; if (match_field_against_config(field, value, p)) { @@ -615,12 +633,6 @@ static int all_fields_match(struct promisor_info *advertised, break; } } - } else { - item = string_list_lookup(config_info, advertised->name); - if (item) { - struct promisor_info *p = item->util; - match = match_field_against_config(field, value, p); - } } if (!match) @@ -640,7 +652,7 @@ static int should_accept_remote(enum accept_promisor accept, const char *remote_url = advertised->url; if (accept == ACCEPT_ALL) - return all_fields_match(advertised, config_info, 1); + return all_fields_match(advertised, config_info, NULL); /* Get config info for that promisor remote */ item = string_list_lookup(config_info, remote_name); @@ -652,7 +664,7 @@ static int should_accept_remote(enum accept_promisor accept, p = item->util; if (accept == ACCEPT_KNOWN_NAME) - return all_fields_match(advertised, config_info, 0); + return all_fields_match(advertised, config_info, p); if (accept != ACCEPT_KNOWN_URL) BUG("Unhandled 'enum accept_promisor' value '%d'", accept); @@ -663,7 +675,7 @@ static int should_accept_remote(enum accept_promisor accept, } if (!strcmp(p->url, remote_url)) - return all_fields_match(advertised, config_info, 0); + return all_fields_match(advertised, config_info, p); warning(_("known remote named '%s' but with URL '%s' instead of '%s'"), remote_name, p->url, remote_url); From 4ed9283b36bc8652954578c3024a00b6e70f8960 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:36 +0200 Subject: [PATCH 53/61] promisor-remote: clarify that a remote is ignored In should_accept_remote() and parse_one_advertised_remote(), when a remote is ignored, we tell users why it is ignored in a warning, but we don't tell them that the remote is actually ignored. Let's clarify that, so users have a better idea of what's actually happening. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 6c935f855af752..8e062ec16098ac 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -670,15 +670,16 @@ static int should_accept_remote(enum accept_promisor accept, BUG("Unhandled 'enum accept_promisor' value '%d'", accept); if (!remote_url || !*remote_url) { - warning(_("no or empty URL advertised for remote '%s'"), remote_name); + warning(_("no or empty URL advertised for remote '%s', " + "ignoring this remote"), remote_name); return 0; } if (!strcmp(p->url, remote_url)) return all_fields_match(advertised, config_info, p); - warning(_("known remote named '%s' but with URL '%s' instead of '%s'"), - remote_name, p->url, remote_url); + warning(_("known remote named '%s' but with URL '%s' instead of '%s', " + "ignoring this remote"), remote_name, p->url, remote_url); return 0; } @@ -722,8 +723,8 @@ static struct promisor_info *parse_one_advertised_remote(const char *remote_info string_list_clear(&elem_list, 0); if (!info->name || !info->url) { - warning(_("server advertised a promisor remote without a name or URL: %s"), - remote_info); + warning(_("server advertised a promisor remote without a name or URL: '%s', " + "ignoring this remote"), remote_info); promisor_info_free(info); return NULL; } From 3b4f0403d19738a26f0da58f4efc6f4e2473fcac Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:37 +0200 Subject: [PATCH 54/61] promisor-remote: reject empty name or URL in advertised remote In parse_one_advertised_remote(), we check for a NULL remote name and remote URL, but not for empty ones. An empty URL seems possible as url_percent_decode("") doesn't return NULL. In promisor_config_info_list(), we ignore remotes with empty URLs, so a Git server should not advertise remotes with empty URLs. It's possible that a buggy or malicious server would do it though. So let's tighten the check in parse_one_advertised_remote() to also reject empty strings at parse time. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/promisor-remote.c b/promisor-remote.c index 8e062ec16098ac..8322349ae8ba87 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -722,7 +722,7 @@ static struct promisor_info *parse_one_advertised_remote(const char *remote_info string_list_clear(&elem_list, 0); - if (!info->name || !info->url) { + if (!info->name || !*info->name || !info->url || !*info->url) { warning(_("server advertised a promisor remote without a name or URL: '%s', " "ignoring this remote"), remote_info); promisor_info_free(info); From 64f0f6b88aea33546afd1271862b486fafe7e9cc Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:38 +0200 Subject: [PATCH 55/61] promisor-remote: refactor should_accept_remote() control flow A previous commit made sure we now reject empty URLs early at parse time. This makes the existing warning() in case a remote URL is NULL or empty very unlikely to be useful. In future work, we also plan to add URL-based acceptance logic into should_accept_remote(). To adapt to previous changes and prepare for upcoming changes, let's restructure the control flow in should_accept_remote(). Concretely, let's: - Replace the warning() in case of an empty URL with a BUG(), as a previous commit made sure empty URLs are rejected early at parse time. - Move that modified empty-URL check to the very top of the function, so that every acceptance mode, instead of only ACCEPT_KNOWN_URL, is covered. - Invert the URL comparison: instead of returning on match and warning on mismatch, return early on mismatch and let the match case fall through. This opens a single exit path at the bottom of the function for future commits to extend. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 8322349ae8ba87..5860a3d3f36e09 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -651,6 +651,11 @@ static int should_accept_remote(enum accept_promisor accept, const char *remote_name = advertised->name; const char *remote_url = advertised->url; + if (!remote_url || !*remote_url) + BUG("no or empty URL advertised for remote '%s'; " + "this remote should have been rejected earlier", + remote_name); + if (accept == ACCEPT_ALL) return all_fields_match(advertised, config_info, NULL); @@ -669,19 +674,14 @@ static int should_accept_remote(enum accept_promisor accept, if (accept != ACCEPT_KNOWN_URL) BUG("Unhandled 'enum accept_promisor' value '%d'", accept); - if (!remote_url || !*remote_url) { - warning(_("no or empty URL advertised for remote '%s', " - "ignoring this remote"), remote_name); + if (strcmp(p->url, remote_url)) { + warning(_("known remote named '%s' but with URL '%s' instead of '%s', " + "ignoring this remote"), + remote_name, p->url, remote_url); return 0; } - if (!strcmp(p->url, remote_url)) - return all_fields_match(advertised, config_info, p); - - warning(_("known remote named '%s' but with URL '%s' instead of '%s', " - "ignoring this remote"), remote_name, p->url, remote_url); - - return 0; + return all_fields_match(advertised, config_info, p); } static int skip_field_name_prefix(const char *elem, const char *field_name, const char **value) From 16a4372a3df7579429b7bc23e984bd797a4b7b8d Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:39 +0200 Subject: [PATCH 56/61] promisor-remote: refactor has_control_char() In a future commit we are going to check if some strings contain control characters, so let's refactor the logic to do that in a new has_control_char() helper function. It cleans up the code a bit anyway. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 5860a3d3f36e09..d60518f19c053e 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -642,6 +642,14 @@ static int all_fields_match(struct promisor_info *advertised, return 1; } +static bool has_control_char(const char *s) +{ + for (const char *c = s; *c; c++) + if (iscntrl(*c)) + return true; + return false; +} + static int should_accept_remote(enum accept_promisor accept, struct promisor_info *advertised, struct string_list *config_info) @@ -772,18 +780,14 @@ static bool valid_filter(const char *filter, const char *remote_name) return !res; } -/* Check that a token doesn't contain any control character */ static bool valid_token(const char *token, const char *remote_name) { - const char *c = token; - - for (; *c; c++) - if (iscntrl(*c)) { - warning(_("invalid token '%s' for remote '%s' " - "will not be stored"), - token, remote_name); - return false; - } + if (has_control_char(token)) { + warning(_("invalid token '%s' for remote '%s' " + "will not be stored"), + token, remote_name); + return false; + } return true; } From 7557a562434804d27f1417fe94c4081e2ee7e68b Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:40 +0200 Subject: [PATCH 57/61] promisor-remote: refactor accept_from_server() In future commits, we are going to add more logic to filter_promisor_remote() which is already doing a lot of things. Let's alleviate that by moving the logic that checks and validates the value of the `promisor.acceptFromServer` config variable into its own accept_from_server() helper function. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index d60518f19c053e..8d80ef6040534c 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -862,20 +862,12 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised, return reload_config; } -static void filter_promisor_remote(struct repository *repo, - struct strvec *accepted, - const char *info) +static enum accept_promisor accept_from_server(struct repository *repo) { const char *accept_str; enum accept_promisor accept = ACCEPT_NONE; - struct string_list config_info = STRING_LIST_INIT_NODUP; - struct string_list remote_info = STRING_LIST_INIT_DUP; - struct store_info *store_info = NULL; - struct string_list_item *item; - bool reload_config = false; - struct string_list accepted_filters = STRING_LIST_INIT_DUP; - if (!repo_config_get_string_tmp(the_repository, "promisor.acceptfromserver", &accept_str)) { + if (!repo_config_get_string_tmp(repo, "promisor.acceptfromserver", &accept_str)) { if (!*accept_str || !strcasecmp("None", accept_str)) accept = ACCEPT_NONE; else if (!strcasecmp("KnownUrl", accept_str)) @@ -889,6 +881,21 @@ static void filter_promisor_remote(struct repository *repo, accept_str, "promisor.acceptfromserver"); } + return accept; +} + +static void filter_promisor_remote(struct repository *repo, + struct strvec *accepted, + const char *info) +{ + struct string_list config_info = STRING_LIST_INIT_NODUP; + struct string_list remote_info = STRING_LIST_INIT_DUP; + struct store_info *store_info = NULL; + struct string_list_item *item; + bool reload_config = false; + struct string_list accepted_filters = STRING_LIST_INIT_DUP; + enum accept_promisor accept = accept_from_server(repo); + if (accept == ACCEPT_NONE) return; From e0f80d8876960442dd2645215c4fe5e1b1d80fc3 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:41 +0200 Subject: [PATCH 58/61] promisor-remote: keep accepted promisor_info structs alive In filter_promisor_remote(), the instances of `struct promisor_info` for accepted remotes are dismantled into separate parallel data structures (the 'accepted' strvec for server names, and 'accepted_filters' for filter strings) and then immediately freed. Instead, let's keep these instances on an 'accepted_remotes' list. This way the post-loop phase can iterate a single list to build the protocol reply, apply advertised filters, and mark remotes as accepted, rather than iterating three separate structures. This refactoring also prepares for a future commit that will add a 'local_name' member to 'struct promisor_info'. Since struct instances stay alive, downstream code will be able to simply read both names from them rather than needing yet another parallel strvec. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 8d80ef6040534c..74e65e9dd0de48 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -890,10 +890,10 @@ static void filter_promisor_remote(struct repository *repo, { struct string_list config_info = STRING_LIST_INIT_NODUP; struct string_list remote_info = STRING_LIST_INIT_DUP; + struct string_list accepted_remotes = STRING_LIST_INIT_NODUP; struct store_info *store_info = NULL; struct string_list_item *item; bool reload_config = false; - struct string_list accepted_filters = STRING_LIST_INIT_DUP; enum accept_promisor accept = accept_from_server(repo); if (accept == ACCEPT_NONE) @@ -922,17 +922,10 @@ static void filter_promisor_remote(struct repository *repo, if (promisor_store_advertised_fields(advertised, store_info)) reload_config = true; - strvec_push(accepted, advertised->name); - - /* Capture advertised filters for accepted remotes */ - if (advertised->filter) { - struct string_list_item *i; - i = string_list_append(&accepted_filters, advertised->name); - i->util = xstrdup(advertised->filter); - } + string_list_append(&accepted_remotes, advertised->name)->util = advertised; + } else { + promisor_info_free(advertised); } - - promisor_info_free(advertised); } promisor_info_list_clear(&config_info); @@ -942,24 +935,23 @@ static void filter_promisor_remote(struct repository *repo, if (reload_config) repo_promisor_remote_reinit(repo); - /* Apply accepted remote filters to the stable repo state */ - for_each_string_list_item(item, &accepted_filters) { - struct promisor_remote *r = repo_promisor_remote_find(repo, item->string); - if (r) { - free(r->advertised_filter); - r->advertised_filter = item->util; - item->util = NULL; - } - } + /* Apply accepted remotes to the stable repo state */ + for_each_string_list_item(item, &accepted_remotes) { + struct promisor_info *info = item->util; + struct promisor_remote *r = repo_promisor_remote_find(repo, info->name); - string_list_clear(&accepted_filters, 1); + strvec_push(accepted, info->name); - /* Mark the remotes as accepted in the repository state */ - for (size_t i = 0; i < accepted->nr; i++) { - struct promisor_remote *r = repo_promisor_remote_find(repo, accepted->v[i]); - if (r) + if (r) { r->accepted = 1; + if (info->filter) { + free(r->advertised_filter); + r->advertised_filter = xstrdup(info->filter); + } + } } + + promisor_info_list_clear(&accepted_remotes); } void promisor_remote_reply(const char *info, char **accepted_out) From d56e483b03bfe46340af5cdbcddec8858661d2e9 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:42 +0200 Subject: [PATCH 59/61] promisor-remote: remove the 'accepted' strvec In a previous commit, filter_promisor_remote() was refactored to keep accepted 'struct promisor_info' instances alive instead of dismantling them into separate parallel data structures. Let's go one step further and replace the 'struct strvec *accepted' argument passed to filter_promisor_remote() with a 'struct string_list *accepted_remotes' argument. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- promisor-remote.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/promisor-remote.c b/promisor-remote.c index 74e65e9dd0de48..38fa05054227f6 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -885,12 +885,11 @@ static enum accept_promisor accept_from_server(struct repository *repo) } static void filter_promisor_remote(struct repository *repo, - struct strvec *accepted, + struct string_list *accepted_remotes, const char *info) { struct string_list config_info = STRING_LIST_INIT_NODUP; struct string_list remote_info = STRING_LIST_INIT_DUP; - struct string_list accepted_remotes = STRING_LIST_INIT_NODUP; struct store_info *store_info = NULL; struct string_list_item *item; bool reload_config = false; @@ -922,7 +921,7 @@ static void filter_promisor_remote(struct repository *repo, if (promisor_store_advertised_fields(advertised, store_info)) reload_config = true; - string_list_append(&accepted_remotes, advertised->name)->util = advertised; + string_list_append(accepted_remotes, advertised->name)->util = advertised; } else { promisor_info_free(advertised); } @@ -936,12 +935,10 @@ static void filter_promisor_remote(struct repository *repo, repo_promisor_remote_reinit(repo); /* Apply accepted remotes to the stable repo state */ - for_each_string_list_item(item, &accepted_remotes) { + for_each_string_list_item(item, accepted_remotes) { struct promisor_info *info = item->util; struct promisor_remote *r = repo_promisor_remote_find(repo, info->name); - strvec_push(accepted, info->name); - if (r) { r->accepted = 1; if (info->filter) { @@ -950,23 +947,23 @@ static void filter_promisor_remote(struct repository *repo, } } } - - promisor_info_list_clear(&accepted_remotes); } void promisor_remote_reply(const char *info, char **accepted_out) { - struct strvec accepted = STRVEC_INIT; + struct string_list accepted_remotes = STRING_LIST_INIT_NODUP; - filter_promisor_remote(the_repository, &accepted, info); + filter_promisor_remote(the_repository, &accepted_remotes, info); if (accepted_out) { - if (accepted.nr) { + if (accepted_remotes.nr) { struct strbuf reply = STRBUF_INIT; - for (size_t i = 0; i < accepted.nr; i++) { - if (i) + struct string_list_item *item; + + for_each_string_list_item(item, &accepted_remotes) { + if (reply.len) strbuf_addch(&reply, ';'); - strbuf_addstr_urlencode(&reply, accepted.v[i], allow_unsanitized); + strbuf_addstr_urlencode(&reply, item->string, allow_unsanitized); } *accepted_out = strbuf_detach(&reply, NULL); } else { @@ -974,7 +971,7 @@ void promisor_remote_reply(const char *info, char **accepted_out) } } - strvec_clear(&accepted); + promisor_info_list_clear(&accepted_remotes); } void mark_promisor_remotes_as_accepted(struct repository *r, const char *remotes) From 8eb863597f630efe08f96ed12f8defbe5a5f0b1d Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Tue, 7 Apr 2026 13:52:43 +0200 Subject: [PATCH 60/61] t5710: use proper file:// URIs for absolute paths In t5710, we frequently construct local file URIs using `file://$(pwd)`. On Unix-like systems, $(pwd) returns an absolute path starting with a slash (e.g., `/tmp/repo`), resulting in a valid 3-slash URI with an empty host (`file:///tmp/repo`). However, on Windows, $(pwd) returns a path starting with a drive letter (e.g., `D:/a/repo`). This results in a 2-slash URI (`file://D:/a/repo`). Standard URI parsers misinterpret this format, treating `D:` as the host rather than part of the absolute path. This is to be expected because RFC 8089 says that the `//` prefix with an empty local host must be followed by an absolute path starting with a slash. While this hasn't broken the existing tests (because the old `promisor.acceptFromServer` logic relies entirely on strict `strcmp()` without normalizing the URLs), it will break future commits that pass these URLs through `url_normalize()` or similar functions. To future-proof the tests and ensure cross-platform URI compliance, let's introduce a $TRASH_DIRECTORY_URL helper variable that explicitly guarantees a leading slash for the path component, ensuring valid 3-slash `file:///` URIs on all operating systems. While at it, let's also introduce $ENCODED_TRASH_DIRECTORY_URL to handle some common special characters in directory paths. To be extra safe, let's skip all the tests if there are uncommon special characters in the directory path. Then let's replace all instances of `file://$(pwd)` with $TRASH_DIRECTORY_URL across the test script, and let's simplify the `sendFields` and `checkFields` tests to use $ENCODED_TRASH_DIRECTORY_URL directly. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- t/t5710-promisor-remote-capability.sh | 79 +++++++++++++++++---------- 1 file changed, 51 insertions(+), 28 deletions(-) diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh index bf0eed9f109742..b404ad9f0a9e3d 100755 --- a/t/t5710-promisor-remote-capability.sh +++ b/t/t5710-promisor-remote-capability.sh @@ -76,6 +76,31 @@ copy_to_lop () { cp "$path" "$path2" } +# On Windows, `pwd` returns a path like 'D:/foo/bar'. Prepend '/' to turn +# it into '/D:/foo/bar', which is what git expects in file:// URLs on Windows. +# On Unix, the path already starts with '/', so this is a no-op. +pwd_path=$(pwd) +case "$pwd_path" in +[a-zA-Z]:*) pwd_path="/$pwd_path" ;; +esac + +# Allowed characters: alphanumeric, standard path/URI (_ . ~ / : -), +# and those percent-encoded below (% space = , ;) +rest=$(printf "%s" "$pwd_path" | tr -d 'a-zA-Z0-9_.~/:% =,;-') +if test -n "$rest" +then + skip_all="PWD contains unsupported special characters" + test_done +fi + +TRASH_DIRECTORY_URL="file://$pwd_path" + +encoded_path=$(printf "%s" "$pwd_path" | + sed -e 's/%/%25/g' -e 's/ /%20/g' -e 's/=/%3D/g' \ + -e 's/;/%3B/g' -e 's/,/%2C/g') + +ENCODED_TRASH_DIRECTORY_URL="file://$encoded_path" + test_expect_success "setup for testing promisor remote advertisement" ' # Create another bare repo called "lop" (for Large Object Promisor) git init --bare lop && @@ -88,7 +113,7 @@ test_expect_success "setup for testing promisor remote advertisement" ' initialize_server 1 "$oid" && # Configure lop as promisor remote for server - git -C server remote add lop "file://$(pwd)/lop" && + git -C server remote add lop "$TRASH_DIRECTORY_URL/lop" && git -C server config remote.lop.promisor true && git -C lop config uploadpack.allowFilter true && @@ -104,7 +129,7 @@ test_expect_success "clone with promisor.advertise set to 'true'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && @@ -119,7 +144,7 @@ test_expect_success "clone with promisor.advertise set to 'false'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && @@ -137,7 +162,7 @@ test_expect_success "clone with promisor.acceptfromserver set to 'None'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=None \ --no-local --filter="blob:limit=5k" server client && @@ -156,8 +181,8 @@ test_expect_success "init + fetch with promisor.advertise set to 'true'" ' git -C client init && git -C client config remote.lop.promisor true && git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" && - git -C client config remote.lop.url "file://$(pwd)/lop" && - git -C client config remote.server.url "file://$(pwd)/server" && + git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" && + git -C client config remote.server.url "$TRASH_DIRECTORY_URL/server" && git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" && git -C client config promisor.acceptfromserver All && GIT_NO_LAZY_FETCH=0 git -C client fetch --filter="blob:limit=5k" server && @@ -177,10 +202,10 @@ test_expect_success "clone with two promisors but only one advertised" ' GIT_TRACE="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.unused_lop.promisor=true \ -c remote.unused_lop.fetch="+refs/heads/*:refs/remotes/unused_lop/*" \ - -c remote.unused_lop.url="file://$(pwd)/unused_lop" \ + -c remote.unused_lop.url="$TRASH_DIRECTORY_URL/unused_lop" \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && @@ -210,11 +235,11 @@ test_expect_success "init + fetch two promisors but only one advertised" ' git -C client init && git -C client config remote.unused_lop.promisor true && git -C client config remote.unused_lop.fetch "+refs/heads/*:refs/remotes/unused_lop/*" && - git -C client config remote.unused_lop.url "file://$(pwd)/unused_lop" && + git -C client config remote.unused_lop.url "$TRASH_DIRECTORY_URL/unused_lop" && git -C client config remote.lop.promisor true && git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" && - git -C client config remote.lop.url "file://$(pwd)/lop" && - git -C client config remote.server.url "file://$(pwd)/server" && + git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" && + git -C client config remote.server.url "$TRASH_DIRECTORY_URL/server" && git -C client config remote.server.fetch "+refs/heads/*:refs/remotes/server/*" && git -C client config promisor.acceptfromserver All && @@ -242,7 +267,7 @@ test_expect_success "clone with promisor.acceptfromserver set to 'KnownName'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownName \ --no-local --filter="blob:limit=5k" server client && @@ -257,7 +282,7 @@ test_expect_success "clone with 'KnownName' and different remote names" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.serverTwo.promisor=true \ -c remote.serverTwo.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.serverTwo.url="file://$(pwd)/lop" \ + -c remote.serverTwo.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownName \ --no-local --filter="blob:limit=5k" server client && @@ -294,7 +319,7 @@ test_expect_success "clone with promisor.acceptfromserver set to 'KnownUrl'" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -311,7 +336,7 @@ test_expect_success "clone with 'KnownUrl' and different remote urls" ' # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/serverTwo" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/serverTwo" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -326,7 +351,7 @@ test_expect_success "clone with 'KnownUrl' and url not configured on the server" git -C server config promisor.advertise true && test_when_finished "rm -rf client" && - test_when_finished "git -C server config set remote.lop.url \"file://$(pwd)/lop\"" && + test_when_finished "git -C server config set remote.lop.url \"$TRASH_DIRECTORY_URL/lop\"" && git -C server config unset remote.lop.url && # Clone from server to create a client @@ -335,7 +360,7 @@ test_expect_success "clone with 'KnownUrl' and url not configured on the server" # missing, so the remote name will be used instead which will fail. test_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -347,7 +372,7 @@ test_expect_success "clone with 'KnownUrl' and empty url, so not advertised" ' git -C server config promisor.advertise true && test_when_finished "rm -rf client" && - test_when_finished "git -C server config set remote.lop.url \"file://$(pwd)/lop\"" && + test_when_finished "git -C server config set remote.lop.url \"$TRASH_DIRECTORY_URL/lop\"" && git -C server config set remote.lop.url "" && # Clone from server to create a client @@ -356,7 +381,7 @@ test_expect_success "clone with 'KnownUrl' and empty url, so not advertised" ' # so the remote name will be used instead which will fail. test_must_fail env GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=KnownUrl \ --no-local --filter="blob:limit=5k" server client && @@ -380,13 +405,12 @@ test_expect_success "clone with promisor.sendFields" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && # Check that fields are properly transmitted - ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") && - PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" && + PR1="name=lop,url=$ENCODED_TRASH_DIRECTORY_URL/lop,partialCloneFilter=blob:none" && PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" && test_grep "clone< promisor-remote=$PR1;$PR2" trace && test_grep "clone> promisor-remote=lop;otherLop" trace && @@ -411,15 +435,14 @@ test_expect_success "clone with promisor.checkFields" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c remote.lop.partialCloneFilter="blob:none" \ -c promisor.acceptfromserver=All \ -c promisor.checkFields=partialcloneFilter \ --no-local --filter="blob:limit=5k" server client && # Check that fields are properly transmitted - ENCODED_URL=$(echo "file://$(pwd)/lop" | sed -e "s/ /%20/g") && - PR1="name=lop,url=$ENCODED_URL,partialCloneFilter=blob:none" && + PR1="name=lop,url=$ENCODED_TRASH_DIRECTORY_URL/lop,partialCloneFilter=blob:none" && PR2="name=otherLop,url=https://invalid.invalid,partialCloneFilter=blob:limit=10k,token=fooBar" && test_grep "clone< promisor-remote=$PR1;$PR2" trace && test_grep "clone> promisor-remote=lop" trace && @@ -449,7 +472,7 @@ test_expect_success "clone with promisor.storeFields=partialCloneFilter" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c remote.lop.token="fooYYY" \ -c remote.lop.partialCloneFilter="blob:none" \ -c promisor.acceptfromserver=All \ @@ -501,7 +524,7 @@ test_expect_success "clone and fetch with --filter=auto" ' GIT_TRACE_PACKET="$(pwd)/trace" GIT_NO_LAZY_FETCH=0 git clone \ -c remote.lop.promisor=true \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter=auto server client 2>err && @@ -558,7 +581,7 @@ test_expect_success "clone with promisor.advertise set to 'true' but don't delet # Clone from server to create a client GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \ - -c remote.lop.url="file://$(pwd)/lop" \ + -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \ -c promisor.acceptfromserver=All \ --no-local --filter="blob:limit=5k" server client && From a7aa55698863b92a2a6a936162f6bdfa90e749c1 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 8 Apr 2026 11:00:35 -0700 Subject: [PATCH 61/61] ### match next