From bc6a6cf5eedb19b1b1da92ed2761ff9b1c7da627 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Tue, 17 Mar 2026 13:00:33 +0000 Subject: [PATCH 01/20] strbuf: pass correct alloc to strbuf_attach() in strbuf_reencode() reencode_string_len() allocates len+1 bytes (including the NUL) and returns the string length in len. strbuf_reencode() was calling strbuf_attach(sb, out, len, len), so alloc was one byte too small. strbuf_attach() then calls strbuf_grow(sb, 0). With alloc < len+1, ALLOC_GROW always reallocates, so we reallocated immediately after attach even when the strbuf was not extended further. Pass len+1 as the alloc argument so the existing buffer is reused and the reallocation is avoided. Signed-off-by: Vaidas Pilkauskas Signed-off-by: Junio C Hamano --- strbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/strbuf.c b/strbuf.c index 59678bf5b03e0b..bad76ea64dbe1c 100644 --- a/strbuf.c +++ b/strbuf.c @@ -168,7 +168,7 @@ int strbuf_reencode(struct strbuf *sb, const char *from, const char *to) if (!out) return -1; - strbuf_attach(sb, out, len, len); + strbuf_attach(sb, out, len, len + 1); return 0; } From a4fddb01c5bd0ecbd5e297ee571ad29ca62bf940 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Tue, 17 Mar 2026 13:00:34 +0000 Subject: [PATCH 02/20] strbuf_attach: fix call sites to pass correct alloc strbuf_attach(sb, buf, len, alloc) requires alloc > len (the buffer must have at least len+1 bytes to hold the NUL). Several call sites passed alloc == len, relying on strbuf_grow(sb, 0) inside strbuf_attach to reallocate. Fix these in mailinfo, am, refs/files-backend, fast-import, and trailer by passing len+1 when the buffer is a NUL-terminated string (or from strbuf_detach). Signed-off-by: Vaidas Pilkauskas Signed-off-by: Junio C Hamano --- builtin/am.c | 2 +- builtin/fast-import.c | 2 +- mailinfo.c | 2 +- refs/files-backend.c | 2 +- trailer.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index b66a33d8a8899c..66be33ab420ff6 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1188,7 +1188,7 @@ static void am_append_signoff(struct am_state *state) { struct strbuf sb = STRBUF_INIT; - strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len); + strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len + 1); append_signoff(&sb, 0, 0); state->msg = strbuf_detach(&sb, &state->msg_len); } diff --git a/builtin/fast-import.c b/builtin/fast-import.c index b8a7757cfda943..164d8a6198abc8 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3246,7 +3246,7 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid) cat_blob_write("\n", 1); if (oe && oe->pack_id == pack_id) { last_blob.offset = oe->idx.offset; - strbuf_attach(&last_blob.data, buf, size, size); + strbuf_attach(&last_blob.data, buf, size, size + 1); last_blob.depth = oe->depth; } else free(buf); diff --git a/mailinfo.c b/mailinfo.c index 99ac596e096e70..e52a35fde01023 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -470,7 +470,7 @@ static int convert_to_utf8(struct mailinfo *mi, return error("cannot convert from %s to %s", charset, mi->metainfo_charset); } - strbuf_attach(line, out, out_len, out_len); + strbuf_attach(line, out, out_len, out_len + 1); return 0; } diff --git a/refs/files-backend.c b/refs/files-backend.c index 240d3c3b26e0b5..bddc04099d0a4d 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1806,7 +1806,7 @@ static int commit_ref(struct ref_lock *lock) size_t len = strlen(path); struct strbuf sb_path = STRBUF_INIT; - strbuf_attach(&sb_path, path, len, len); + strbuf_attach(&sb_path, path, len, len + 1); /* * If this fails, commit_lock_file() will also fail diff --git a/trailer.c b/trailer.c index 911a81ed993ec6..3afe368db03f9a 100644 --- a/trailer.c +++ b/trailer.c @@ -1009,7 +1009,7 @@ static struct trailer_block *trailer_block_get(const struct process_trailer_opti for (ptr = trailer_lines; *ptr; ptr++) { if (last && isspace((*ptr)->buf[0])) { struct strbuf sb = STRBUF_INIT; - strbuf_attach(&sb, *last, strlen(*last), strlen(*last)); + strbuf_attach(&sb, *last, strlen(*last), strlen(*last) + 1); strbuf_addbuf(&sb, *ptr); *last = strbuf_detach(&sb, NULL); continue; From 640657ffd06999ec1ec3b1d030b7f5aac6b7f57b Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Tue, 17 Mar 2026 13:00:35 +0000 Subject: [PATCH 03/20] http: add support for HTTP 429 rate limit retries Add retry logic for HTTP 429 (Too Many Requests) responses to handle server-side rate limiting gracefully. When Git's HTTP client receives a 429 response, it can now automatically retry the request after an appropriate delay, respecting the server's rate limits. The implementation supports the RFC-compliant Retry-After header in both delay-seconds (integer) and HTTP-date (RFC 2822) formats. If a past date is provided, Git retries immediately without waiting. Retry behavior is controlled by three new configuration options (http.maxRetries, http.retryAfter, and http.maxRetryTime) which are documented in git-config(1). The retry logic implements a fail-fast approach: if any delay (whether from server header or configuration) exceeds maxRetryTime, Git fails immediately with a clear error message rather than capping the delay. This provides better visibility into rate limiting issues. The implementation includes extensive test coverage for basic retry behavior, Retry-After header formats (integer and HTTP-date), configuration combinations, maxRetryTime limits, invalid header handling, environment variable overrides, and edge cases. Signed-off-by: Vaidas Pilkauskas Signed-off-by: Junio C Hamano --- Documentation/config/http.adoc | 26 ++++ git-curl-compat.h | 8 + http.c | 144 +++++++++++++++--- http.h | 9 ++ remote-curl.c | 11 ++ t/lib-httpd.sh | 1 + t/lib-httpd/apache.conf | 8 + t/lib-httpd/http-429.sh | 98 ++++++++++++ t/meson.build | 1 + t/t5584-http-429-retry.sh | 266 +++++++++++++++++++++++++++++++++ 10 files changed, 551 insertions(+), 21 deletions(-) create mode 100644 t/lib-httpd/http-429.sh create mode 100755 t/t5584-http-429-retry.sh diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc index 9da5c298cc1d5e..849c89f36c5ad8 100644 --- a/Documentation/config/http.adoc +++ b/Documentation/config/http.adoc @@ -315,6 +315,32 @@ http.keepAliveCount:: unset, curl's default value is used. Can be overridden by the `GIT_HTTP_KEEPALIVE_COUNT` environment variable. +http.retryAfter:: + Default wait time in seconds before retrying when a server returns + HTTP 429 (Too Many Requests) without a Retry-After header. + Defaults to 0 (retry immediately). When a Retry-After header is + present, its value takes precedence over this setting; however, + automatic use of the server-provided `Retry-After` header requires + libcurl 7.66.0 or later. On older versions, configure this setting + manually to control the retry delay. Can be overridden by the + `GIT_HTTP_RETRY_AFTER` environment variable. + See also `http.maxRetries` and `http.maxRetryTime`. + +http.maxRetries:: + Maximum number of times to retry after receiving HTTP 429 (Too Many + Requests) responses. Set to 0 (the default) to disable retries. + Can be overridden by the `GIT_HTTP_MAX_RETRIES` environment variable. + See also `http.retryAfter` and `http.maxRetryTime`. + +http.maxRetryTime:: + Maximum time in seconds to wait for a single retry attempt when + handling HTTP 429 (Too Many Requests) responses. If the server + requests a delay (via Retry-After header) or if `http.retryAfter` + is configured with a value that exceeds this maximum, Git will fail + immediately rather than waiting. Default is 300 seconds (5 minutes). + Can be overridden by the `GIT_HTTP_MAX_RETRY_TIME` environment + variable. See also `http.retryAfter` and `http.maxRetries`. + http.noEPSV:: A boolean which disables using of EPSV ftp command by curl. This can be helpful with some "poor" ftp servers which don't diff --git a/git-curl-compat.h b/git-curl-compat.h index 659e5a3875e3d6..dccdd4d6e54158 100644 --- a/git-curl-compat.h +++ b/git-curl-compat.h @@ -37,6 +37,14 @@ #define GIT_CURL_NEED_TRANSFER_ENCODING_HEADER #endif +/** + * CURLINFO_RETRY_AFTER was added in 7.66.0, released in September 2019. + * It allows curl to automatically parse Retry-After headers. + */ +#if LIBCURL_VERSION_NUM >= 0x074200 +#define GIT_CURL_HAVE_CURLINFO_RETRY_AFTER 1 +#endif + /** * CURLOPT_PROTOCOLS_STR and CURLOPT_REDIR_PROTOCOLS_STR were added in 7.85.0, * released in August 2022. diff --git a/http.c b/http.c index 7815f144de3d69..0276bb6ff088a7 100644 --- a/http.c +++ b/http.c @@ -22,6 +22,8 @@ #include "object-file.h" #include "odb.h" #include "tempfile.h" +#include "date.h" +#include "trace2.h" static struct trace_key trace_curl = TRACE_KEY_INIT(CURL); static int trace_curl_data = 1; @@ -149,6 +151,11 @@ static char *cached_accept_language; static char *http_ssl_backend; static int http_schannel_check_revoke = 1; + +static long http_retry_after = 0; +static long http_max_retries = 0; +static long http_max_retry_time = 300; + /* * With the backend being set to `schannel`, setting sslCAinfo would override * the Certificate Store in cURL v7.60.0 and later, which is not what we want @@ -209,7 +216,7 @@ static inline int is_hdr_continuation(const char *ptr, const size_t size) return size && (*ptr == ' ' || *ptr == '\t'); } -static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p UNUSED) +static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p MAYBE_UNUSED) { size_t size = eltsize * nmemb; struct strvec *values = &http_auth.wwwauth_headers; @@ -575,6 +582,21 @@ static int http_options(const char *var, const char *value, return 0; } + if (!strcmp("http.retryafter", var)) { + http_retry_after = git_config_int(var, value, ctx->kvi); + return 0; + } + + if (!strcmp("http.maxretries", var)) { + http_max_retries = git_config_int(var, value, ctx->kvi); + return 0; + } + + if (!strcmp("http.maxretrytime", var)) { + http_max_retry_time = git_config_int(var, value, ctx->kvi); + return 0; + } + /* Fall back on the default ones */ return git_default_config(var, value, ctx, data); } @@ -1422,6 +1444,10 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) set_long_from_env(&curl_tcp_keepintvl, "GIT_TCP_KEEPINTVL"); set_long_from_env(&curl_tcp_keepcnt, "GIT_TCP_KEEPCNT"); + set_long_from_env(&http_retry_after, "GIT_HTTP_RETRY_AFTER"); + set_long_from_env(&http_max_retries, "GIT_HTTP_MAX_RETRIES"); + set_long_from_env(&http_max_retry_time, "GIT_HTTP_MAX_RETRY_TIME"); + curl_default = get_curl_handle(); } @@ -1871,6 +1897,10 @@ static int handle_curl_result(struct slot_results *results) } return HTTP_REAUTH; } + } else if (results->http_code == 429) { + trace2_data_intmax("http", the_repository, "http/429-retry-after", + results->retry_after); + return HTTP_RATE_LIMITED; } else { if (results->http_connectcode == 407) credential_reject(the_repository, &proxy_auth); @@ -1886,6 +1916,7 @@ int run_one_slot(struct active_request_slot *slot, struct slot_results *results) { slot->results = results; + if (!start_active_slot(slot)) { xsnprintf(curl_errorstr, sizeof(curl_errorstr), "failed to start HTTP request"); @@ -2119,10 +2150,10 @@ static void http_opt_request_remainder(CURL *curl, off_t pos) static int http_request(const char *url, void *result, int target, - const struct http_get_options *options) + struct http_get_options *options) { struct active_request_slot *slot; - struct slot_results results; + struct slot_results results = { .retry_after = -1 }; struct curl_slist *headers = http_copy_default_headers(); struct strbuf buf = STRBUF_INIT; const char *accept_language; @@ -2156,22 +2187,19 @@ static int http_request(const char *url, headers = curl_slist_append(headers, accept_language); strbuf_addstr(&buf, "Pragma:"); - if (options && options->no_cache) + if (options->no_cache) strbuf_addstr(&buf, " no-cache"); - if (options && options->initial_request && + if (options->initial_request && http_follow_config == HTTP_FOLLOW_INITIAL) curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L); headers = curl_slist_append(headers, buf.buf); /* Add additional headers here */ - if (options && options->extra_headers) { + if (options->extra_headers) { const struct string_list_item *item; - if (options && options->extra_headers) { - for_each_string_list_item(item, options->extra_headers) { - headers = curl_slist_append(headers, item->string); - } - } + for_each_string_list_item(item, options->extra_headers) + headers = curl_slist_append(headers, item->string); } headers = http_append_auth_header(&http_auth, headers); @@ -2183,7 +2211,18 @@ static int http_request(const char *url, ret = run_one_slot(slot, &results); - if (options && options->content_type) { +#ifdef GIT_CURL_HAVE_CURLINFO_RETRY_AFTER + if (ret == HTTP_RATE_LIMITED) { + curl_off_t retry_after; + if (curl_easy_getinfo(slot->curl, CURLINFO_RETRY_AFTER, + &retry_after) == CURLE_OK && retry_after > 0) + results.retry_after = (long)retry_after; + } +#endif + + options->retry_after = results.retry_after; + + if (options->content_type) { struct strbuf raw = STRBUF_INIT; curlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE, &raw); extract_content_type(&raw, options->content_type, @@ -2191,7 +2230,7 @@ static int http_request(const char *url, strbuf_release(&raw); } - if (options && options->effective_url) + if (options->effective_url) curlinfo_strbuf(slot->curl, CURLINFO_EFFECTIVE_URL, options->effective_url); @@ -2253,22 +2292,66 @@ static int update_url_from_redirect(struct strbuf *base, return 1; } -static int http_request_reauth(const char *url, +/* + * Compute the retry delay for an HTTP 429 response. + * Returns a negative value if configuration is invalid (delay exceeds + * http.maxRetryTime), otherwise returns the delay in seconds (>= 0). + */ +static long handle_rate_limit_retry(long slot_retry_after) +{ + /* Use the slot-specific retry_after value or configured default */ + if (slot_retry_after >= 0) { + /* Check if retry delay exceeds maximum allowed */ + if (slot_retry_after > http_max_retry_time) { + error(_("response requested a delay greater than http.maxRetryTime (%ld > %ld seconds)"), + slot_retry_after, http_max_retry_time); + trace2_data_string("http", the_repository, + "http/429-error", "exceeds-max-retry-time"); + trace2_data_intmax("http", the_repository, + "http/429-requested-delay", slot_retry_after); + return -1; + } + return slot_retry_after; + } else { + /* No Retry-After header provided, use configured default */ + if (http_retry_after > http_max_retry_time) { + error(_("configured http.retryAfter exceeds http.maxRetryTime (%ld > %ld seconds)"), + http_retry_after, http_max_retry_time); + trace2_data_string("http", the_repository, + "http/429-error", "config-exceeds-max-retry-time"); + return -1; + } + trace2_data_string("http", the_repository, + "http/429-retry-source", "config-default"); + return http_retry_after; + } +} + +static int http_request_recoverable(const char *url, void *result, int target, struct http_get_options *options) { + static struct http_get_options empty_opts; int i = 3; int ret; + int rate_limit_retries = http_max_retries; + + if (!options) + options = &empty_opts; if (always_auth_proactively()) credential_fill(the_repository, &http_auth, 1); ret = http_request(url, result, target, options); - if (ret != HTTP_OK && ret != HTTP_REAUTH) + if (ret != HTTP_OK && ret != HTTP_REAUTH && ret != HTTP_RATE_LIMITED) return ret; - if (options && options->effective_url && options->base_url) { + /* If retries are disabled and we got a 429, fail immediately */ + if (ret == HTTP_RATE_LIMITED && !http_max_retries) + return HTTP_ERROR; + + if (options->effective_url && options->base_url) { if (update_url_from_redirect(options->base_url, url, options->effective_url)) { credential_from_url(&http_auth, options->base_url->buf); @@ -2276,7 +2359,9 @@ static int http_request_reauth(const char *url, } } - while (ret == HTTP_REAUTH && --i) { + while ((ret == HTTP_REAUTH && --i) || + (ret == HTTP_RATE_LIMITED && --rate_limit_retries)) { + long retry_delay = -1; /* * The previous request may have put cruft into our output stream; we * should clear it out before making our next request. @@ -2301,11 +2386,28 @@ static int http_request_reauth(const char *url, default: BUG("Unknown http_request target"); } - - credential_fill(the_repository, &http_auth, 1); + if (ret == HTTP_RATE_LIMITED) { + retry_delay = handle_rate_limit_retry(options->retry_after); + if (retry_delay < 0) + return HTTP_ERROR; + + if (retry_delay > 0) { + warning(_("rate limited, waiting %ld seconds before retry"), retry_delay); + trace2_data_intmax("http", the_repository, + "http/retry-sleep-seconds", retry_delay); + sleep(retry_delay); + } + } else if (ret == HTTP_REAUTH) { + credential_fill(the_repository, &http_auth, 1); + } ret = http_request(url, result, target, options); } + if (ret == HTTP_RATE_LIMITED) { + trace2_data_string("http", the_repository, + "http/429-error", "retries-exhausted"); + return HTTP_RATE_LIMITED; + } return ret; } @@ -2313,7 +2415,7 @@ int http_get_strbuf(const char *url, struct strbuf *result, struct http_get_options *options) { - return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options); + return http_request_recoverable(url, result, HTTP_REQUEST_STRBUF, options); } /* @@ -2337,7 +2439,7 @@ int http_get_file(const char *url, const char *filename, goto cleanup; } - ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options); + ret = http_request_recoverable(url, result, HTTP_REQUEST_FILE, options); fclose(result); if (ret == HTTP_OK && finalize_object_file(the_repository, tmpfile.buf, filename)) diff --git a/http.h b/http.h index f9d459340476e4..f9ee888c3ed67e 100644 --- a/http.h +++ b/http.h @@ -20,6 +20,7 @@ struct slot_results { long http_code; long auth_avail; long http_connectcode; + long retry_after; }; struct active_request_slot { @@ -157,6 +158,13 @@ struct http_get_options { * request has completed. */ struct string_list *extra_headers; + + /* + * After a request completes, contains the Retry-After delay in seconds + * if the server returned HTTP 429 with a Retry-After header (requires + * libcurl 7.66.0 or later), or -1 if no such header was present. + */ + long retry_after; }; /* Return values for http_get_*() */ @@ -167,6 +175,7 @@ struct http_get_options { #define HTTP_REAUTH 4 #define HTTP_NOAUTH 5 #define HTTP_NOMATCHPUBLICKEY 6 +#define HTTP_RATE_LIMITED 7 /* * Requests a URL and stores the result in a strbuf. diff --git a/remote-curl.c b/remote-curl.c index 69f919454a4565..1048fb88cb60bf 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -529,6 +529,17 @@ static struct discovery *discover_refs(const char *service, int for_push) show_http_message(&type, &charset, &buffer); die(_("unable to access '%s' with http.pinnedPubkey configuration: %s"), transport_anonymize_url(url.buf), curl_errorstr); + case HTTP_RATE_LIMITED: + if (http_options.retry_after > 0) { + show_http_message(&type, &charset, &buffer); + die(_("rate limited by '%s', please try again in %ld seconds"), + transport_anonymize_url(url.buf), + http_options.retry_after); + } else { + show_http_message(&type, &charset, &buffer); + die(_("rate limited by '%s', please try again later"), + transport_anonymize_url(url.buf)); + } default: show_http_message(&type, &charset, &buffer); die(_("unable to access '%s': %s"), diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 5091db949b7f99..8a43261ffc1ae2 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -167,6 +167,7 @@ prepare_httpd() { install_script error.sh install_script apply-one-time-script.sh install_script nph-custom-auth.sh + install_script http-429.sh ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules" diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index e631ab0eb5ef05..6bdef603cdb393 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -139,6 +139,10 @@ SetEnv PERL_PATH ${PERL_PATH} SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL + + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} + SetEnv GIT_HTTP_EXPORT_ALL + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL @@ -160,6 +164,7 @@ ScriptAlias /broken_smart/ broken-smart-http.sh/ ScriptAlias /error_smart/ error-smart-http.sh/ ScriptAlias /error/ error.sh/ ScriptAliasMatch /one_time_script/(.*) apply-one-time-script.sh/$1 +ScriptAliasMatch /http_429/(.*) http-429.sh/$1 ScriptAliasMatch /custom_auth/(.*) nph-custom-auth.sh/$1 Options FollowSymlinks @@ -185,6 +190,9 @@ ScriptAliasMatch /custom_auth/(.*) nph-custom-auth.sh/$1 Options ExecCGI + + Options ExecCGI + Options ExecCGI diff --git a/t/lib-httpd/http-429.sh b/t/lib-httpd/http-429.sh new file mode 100644 index 00000000000000..c97b16145b7f92 --- /dev/null +++ b/t/lib-httpd/http-429.sh @@ -0,0 +1,98 @@ +#!/bin/sh + +# Script to return HTTP 429 Too Many Requests responses for testing retry logic. +# Usage: /http_429/// +# +# The test-context is a unique identifier for each test to isolate state files. +# The retry-after-value can be: +# - A number (e.g., "1", "2", "100") - sets Retry-After header to that many seconds +# - "none" - no Retry-After header +# - "invalid" - invalid Retry-After format +# - "permanent" - always return 429 (never succeed) +# - An HTTP-date string (RFC 2822 format) - sets Retry-After to that date +# +# On first call, returns 429. On subsequent calls (after retry), forwards to git-http-backend +# unless retry-after-value is "permanent". + +# Extract test context, retry-after value and repo path from PATH_INFO +# PATH_INFO format: /// +path_info="${PATH_INFO#/}" # Remove leading slash +test_context="${path_info%%/*}" # Get first component (test context) +remaining="${path_info#*/}" # Get rest +retry_after="${remaining%%/*}" # Get second component (retry-after value) +repo_path="${remaining#*/}" # Get rest (repo path) + +# Extract repository name from repo_path (e.g., "repo.git" from "repo.git/info/refs") +# The repo name is the first component before any "/" +repo_name="${repo_path%%/*}" + +# Use current directory (HTTPD_ROOT_PATH) for state file +# Create a safe filename from test_context, retry_after and repo_name +# This ensures all requests for the same test context share the same state file +safe_name=$(echo "${test_context}-${retry_after}-${repo_name}" | tr '/' '_' | tr -cd 'a-zA-Z0-9_-') +state_file="http-429-state-${safe_name}" + +# Check if this is the first call (no state file exists) +if test -f "$state_file" +then + # Already returned 429 once, forward to git-http-backend + # Set PATH_INFO to just the repo path (without retry-after value) + # Set GIT_PROJECT_ROOT so git-http-backend can find the repository + # Use exec to replace this process so git-http-backend gets the updated environment + PATH_INFO="/$repo_path" + export PATH_INFO + # GIT_PROJECT_ROOT points to the document root where repositories are stored + # The script runs from HTTPD_ROOT_PATH, and www/ is the document root + if test -z "$GIT_PROJECT_ROOT" + then + # Construct path: current directory (HTTPD_ROOT_PATH) + /www + GIT_PROJECT_ROOT="$(pwd)/www" + export GIT_PROJECT_ROOT + fi + exec "$GIT_EXEC_PATH/git-http-backend" +fi + +# Mark that we've returned 429 +touch "$state_file" + +# Output HTTP 429 response +printf "Status: 429 Too Many Requests\r\n" + +# Set Retry-After header based on retry_after value +case "$retry_after" in + none) + # No Retry-After header + ;; + invalid) + printf "Retry-After: invalid-format-123abc\r\n" + ;; + permanent) + # Always return 429, don't set state file for success + rm -f "$state_file" + printf "Retry-After: 1\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Permanently rate limited\n" + exit 0 + ;; + *) + # Check if it's a number + case "$retry_after" in + [0-9]*) + # Numeric value + printf "Retry-After: %s\r\n" "$retry_after" + ;; + *) + # Assume it's an HTTP-date format (passed as-is, URL decoded) + # Apache may URL-encode the path, so decode common URL-encoded characters + # %20 = space, %2C = comma, %3A = colon + retry_value=$(echo "$retry_after" | sed -e 's/%20/ /g' -e 's/%2C/,/g' -e 's/%3A/:/g') + printf "Retry-After: %s\r\n" "$retry_value" + ;; + esac + ;; +esac + +printf "Content-Type: text/plain\r\n" +printf "\r\n" +printf "Rate limited\n" diff --git a/t/meson.build b/t/meson.build index 459c52a48972e4..ee824503331632 100644 --- a/t/meson.build +++ b/t/meson.build @@ -700,6 +700,7 @@ integration_tests = [ 't5581-http-curl-verbose.sh', 't5582-fetch-negative-refspec.sh', 't5583-push-branches.sh', + 't5584-http-429-retry.sh', 't5600-clone-fail-cleanup.sh', 't5601-clone.sh', 't5602-clone-remote-exec.sh', diff --git a/t/t5584-http-429-retry.sh b/t/t5584-http-429-retry.sh new file mode 100755 index 00000000000000..a22007b2cff416 --- /dev/null +++ b/t/t5584-http-429-retry.sh @@ -0,0 +1,266 @@ +#!/bin/sh + +test_description='test HTTP 429 Too Many Requests retry logic' + +. ./test-lib.sh + +. "$TEST_DIRECTORY"/lib-httpd.sh + +start_httpd + +test_expect_success 'setup test repository' ' + test_commit initial && + git clone --bare . "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config http.receivepack true +' + +# This test suite uses a special HTTP 429 endpoint at /http_429/ that simulates +# rate limiting. The endpoint format is: +# /http_429/// +# The http-429.sh script (in t/lib-httpd) returns a 429 response with the +# specified Retry-After header on the first request for each test context, +# then forwards subsequent requests to git-http-backend. Each test context +# is isolated, allowing multiple tests to run independently. + +test_expect_success 'HTTP 429 with retries disabled (maxRetries=0) fails immediately' ' + # Set maxRetries to 0 (disabled) + test_config http.maxRetries 0 && + test_config http.retryAfter 1 && + + # Should fail immediately without any retry attempt + test_must_fail git ls-remote "$HTTPD_URL/http_429/retries-disabled/1/repo.git" 2>err && + + # Verify no retry happened (no "waiting" message in stderr) + test_grep ! -i "waiting.*retry" err +' + +test_expect_success 'HTTP 429 permanent should fail after max retries' ' + # Enable retries with a limit + test_config http.maxRetries 2 && + + # Git should retry but eventually fail when 429 persists + test_must_fail git ls-remote "$HTTPD_URL/http_429/permanent-fail/permanent/repo.git" 2>err +' + +test_expect_success 'HTTP 429 with Retry-After is retried and succeeds' ' + # Enable retries + test_config http.maxRetries 3 && + + # Git should retry after receiving 429 and eventually succeed + git ls-remote "$HTTPD_URL/http_429/retry-succeeds/1/repo.git" >output 2>err && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 without Retry-After uses configured default' ' + # Enable retries and configure default delay + test_config http.maxRetries 3 && + test_config http.retryAfter 1 && + + # Git should retry using configured default and succeed + git ls-remote "$HTTPD_URL/http_429/no-retry-after-header/none/repo.git" >output 2>err && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 retry delays are respected' ' + # Enable retries + test_config http.maxRetries 3 && + + # Time the operation - it should take at least 2 seconds due to retry delay + start=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/retry-delays-respected/2/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Verify it took at least 2 seconds (allowing some tolerance) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 fails immediately if Retry-After exceeds http.maxRetryTime' ' + # Configure max retry time to 3 seconds (much less than requested 100) + test_config http.maxRetries 3 && + test_config http.maxRetryTime 3 && + + # Should fail immediately without waiting + start=$(test-tool date getnanos) && + test_must_fail git ls-remote "$HTTPD_URL/http_429/retry-after-exceeds-max-time/100/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (no 100 second wait) + duration_int=${duration%.*} && + test "$duration_int" -lt 99 && + test_grep "greater than http.maxRetryTime" err +' + +test_expect_success 'HTTP 429 fails if configured http.retryAfter exceeds http.maxRetryTime' ' + # Test misconfiguration: retryAfter > maxRetryTime + # Configure retryAfter larger than maxRetryTime + test_config http.maxRetries 3 && + test_config http.retryAfter 100 && + test_config http.maxRetryTime 5 && + + # Should fail immediately with configuration error + start=$(test-tool date getnanos) && + test_must_fail git ls-remote "$HTTPD_URL/http_429/config-retry-after-exceeds-max-time/none/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (no 100 second wait) + duration_int=${duration%.*} && + test "$duration_int" -lt 99 && + test_grep "configured http.retryAfter.*exceeds.*http.maxRetryTime" err +' + +test_expect_success 'HTTP 429 with Retry-After HTTP-date format' ' + # Test HTTP-date format (RFC 2822) in Retry-After header + raw=$(test-tool date timestamp now) && + now="${raw#* -> }" && + future_time=$((now + 2)) && + raw=$(test-tool date show:rfc2822 $future_time) && + future_date="${raw#* -> }" && + future_date_encoded=$(echo "$future_date" | sed "s/ /%20/g") && + + # Enable retries + test_config http.maxRetries 3 && + + # Git should parse the HTTP-date and retry after the delay + start=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/http-date-format/$future_date_encoded/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should take at least 1 second (allowing tolerance for processing time) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immediately' ' + raw=$(test-tool date timestamp now) && + now="${raw#* -> }" && + future_time=$((now + 200)) && + raw=$(test-tool date show:rfc2822 $future_time) && + future_date="${raw#* -> }" && + future_date_encoded=$(echo "$future_date" | sed "s/ /%20/g") && + + # Configure max retry time much less than the 200 second delay + test_config http.maxRetries 3 && + test_config http.maxRetryTime 10 && + + # Should fail immediately without waiting 200 seconds + start=$(test-tool date getnanos) && + test_must_fail git ls-remote "$HTTPD_URL/http_429/http-date-exceeds-max-time/$future_date_encoded/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (not wait 200 seconds) + duration_int=${duration%.*} && + test "$duration_int" -lt 199 && + test_grep "http.maxRetryTime" err +' + +test_expect_success 'HTTP 429 with past HTTP-date should not wait' ' + raw=$(test-tool date timestamp now) && + now="${raw#* -> }" && + past_time=$((now - 10)) && + raw=$(test-tool date show:rfc2822 $past_time) && + past_date="${raw#* -> }" && + past_date_encoded=$(echo "$past_date" | sed "s/ /%20/g") && + + # Enable retries + test_config http.maxRetries 3 && + + # Git should retry immediately without waiting + start=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/past-http-date/$past_date_encoded/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should complete quickly (no wait for a past-date Retry-After) + duration_int=${duration%.*} && + test "$duration_int" -lt 5 && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 with invalid Retry-After format uses configured default' ' + # Configure default retry-after + test_config http.maxRetries 3 && + test_config http.retryAfter 1 && + + # Should use configured default (1 second) since header is invalid + start=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/invalid-retry-after-format/invalid/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should take at least 1 second (the configured default) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test_grep "refs/heads/" output && + test_grep "waiting.*retry" err +' + +test_expect_success 'HTTP 429 will not be retried without config' ' + # Default config means http.maxRetries=0 (retries disabled) + # When 429 is received, it should fail immediately without retry + # Do NOT configure anything - use defaults (http.maxRetries defaults to 0) + + # Should fail immediately without retry + test_must_fail git ls-remote "$HTTPD_URL/http_429/no-retry-without-config/1/repo.git" 2>err && + + # Verify no retry happened (no "waiting" message) + test_grep ! -i "waiting.*retry" err && + + # Should get 429 error + test_grep "429" err +' + +test_expect_success 'GIT_HTTP_RETRY_AFTER overrides http.retryAfter config' ' + # Configure retryAfter to 10 seconds + test_config http.maxRetries 3 && + test_config http.retryAfter 10 && + + # Override with environment variable to 1 second + start=$(test-tool date getnanos) && + GIT_HTTP_RETRY_AFTER=1 git ls-remote "$HTTPD_URL/http_429/env-retry-after-override/none/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should use env var (1 second), not config (10 seconds) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test "$duration_int" -lt 5 && + test_grep "refs/heads/" output && + test_grep "waiting.*retry" err +' + +test_expect_success 'GIT_HTTP_MAX_RETRIES overrides http.maxRetries config' ' + # Configure maxRetries to 0 (disabled) + test_config http.maxRetries 0 && + test_config http.retryAfter 1 && + + # Override with environment variable to enable retries + GIT_HTTP_MAX_RETRIES=3 git ls-remote "$HTTPD_URL/http_429/env-max-retries-override/1/repo.git" >output 2>err && + + # Should retry (env var enables it despite config saying disabled) + test_grep "refs/heads/" output && + test_grep "waiting.*retry" err +' + +test_expect_success 'GIT_HTTP_MAX_RETRY_TIME overrides http.maxRetryTime config' ' + # Configure maxRetryTime to 100 seconds (would accept 50 second delay) + test_config http.maxRetries 3 && + test_config http.maxRetryTime 100 && + + # Override with environment variable to 10 seconds (should reject 50 second delay) + start=$(test-tool date getnanos) && + test_must_fail env GIT_HTTP_MAX_RETRY_TIME=10 \ + git ls-remote "$HTTPD_URL/http_429/env-max-retry-time-override/50/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (not wait 50 seconds) because env var limits to 10 + duration_int=${duration%.*} && + test "$duration_int" -lt 49 && + test_grep "greater than http.maxRetryTime" err +' + +test_expect_success 'verify normal repository access still works' ' + git ls-remote "$HTTPD_URL/smart/repo.git" >output && + test_grep "refs/heads/" output +' + +test_done From 1d6fd6d36edebd23407a49513f6482ceb3e0f648 Mon Sep 17 00:00:00 2001 From: Bilal El Khatabi Date: Thu, 19 Mar 2026 18:06:52 +0000 Subject: [PATCH 04/20] t5315: use test_path_is_file for loose-object check Use test_path_is_file instead of test -f when checking that the loose object was written to the expected path. This uses Git's path-checking helper, which provides more specific failure output than a raw test -f check. Signed-off-by: Bilal El Khatabi Signed-off-by: Junio C Hamano --- t/t5315-pack-objects-compression.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5315-pack-objects-compression.sh b/t/t5315-pack-objects-compression.sh index 8bacd96275b0ac..d0feab17b4cc54 100755 --- a/t/t5315-pack-objects-compression.sh +++ b/t/t5315-pack-objects-compression.sh @@ -10,7 +10,7 @@ test_expect_success setup ' # make sure it resulted in a loose object ob=$(sed -e "s/\(..\).*/\1/" object-name) && ject=$(sed -e "s/..\(.*\)/\1/" object-name) && - test -f .git/objects/$ob/$ject + test_path_is_file .git/objects/$ob/$ject ' while read expect config From 41529f967f919685a3294a9fc4b45140311bebae Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 23 Mar 2026 02:01:58 -0400 Subject: [PATCH 05/20] diff-highlight: mention build instructions Once upon a time, this was just a script in a directory that could be run directly. That changed in 0c977dbc81 (diff-highlight: split code into module, 2017-06-15). Let's update the README to make it more clear that you need to run make. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/README | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README index 1db4440e68324d..9c89146fb0a1cb 100644 --- a/contrib/diff-highlight/README +++ b/contrib/diff-highlight/README @@ -39,10 +39,21 @@ visually distracting. Non-diff lines and existing diff coloration is preserved; the intent is that the output should look exactly the same as the input, except for the occasional highlight. +Build/Install +------------- + +You can build the `diff-highlight` script by running `make` from within +the diff-highlight directory. There is no `make install` target; you can +copy the built script to your $PATH. + +You can run diff-highlight's internal tests by running `make test`. Note +that you must also build Git itself first (by running `make` from the +top-level of the project). + Use --- -You can try out the diff-highlight program with: +You can try out the built diff-highlight program with: --------------------------------------------- git log -p --color | /path/to/diff-highlight From 550097a645348426a3e5944d0457925381ff8e23 Mon Sep 17 00:00:00 2001 From: Scott Baker Date: Mon, 23 Mar 2026 02:02:02 -0400 Subject: [PATCH 06/20] diff-highlight: drop perl version dependency back to 5.8 The diff-highlight code does not rely on any perl features beyond what perl 5.8 provides. We bumped it to v5.26 along with the rest of the project's perl scripts in 702d8c1f3b (Require Perl 5.26.0, 2024-10-23). There's some value in just having a uniform baseline for the project, but I think diff-highlight is special here: - it's in a contrib/ directory that is not frequently touched, so there is little risk of Git developers getting annoyed that modern perl features are not available - it provides a module used by other projects. In particular, diff-so-fancy relies on DiffHighlight.pm but does not otherwise require a perl version more modern than 5.8. Let's drop back to the more conservative requirement. Signed-off-by: Scott Baker Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/DiffHighlight.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm index f0607a4b687cf4..a5e5de3b18491c 100644 --- a/contrib/diff-highlight/DiffHighlight.pm +++ b/contrib/diff-highlight/DiffHighlight.pm @@ -1,6 +1,6 @@ package DiffHighlight; -require v5.26; +require v5.008; use warnings FATAL => 'all'; use strict; From 05002f60565cdfe2b86108c7f7c027a41d534140 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 23 Mar 2026 02:02:05 -0400 Subject: [PATCH 07/20] diff-highlight: check diff-highlight exit status in tests When testing diff-highlight, we pipe the output through a sanitizing function. This loses the exit status of diff-highlight itself, which could mean we are missing cases where it crashes or exits unexpectedly. Use an extra tempfile to avoid the pipe. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/t/t9400-diff-highlight.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index 2a9b68cf3bb8cd..7ebff8b18ff38d 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -41,8 +41,10 @@ dh_test () { git show >commit.raw } >/dev/null && - "$DIFF_HIGHLIGHT" diff.act && - "$DIFF_HIGHLIGHT" commit.act && + "$DIFF_HIGHLIGHT" diff.hi && + test_strip_patch_header diff.act && + "$DIFF_HIGHLIGHT" commit.hi && + test_strip_patch_header commit.act && test_cmp patch.exp diff.act && test_cmp patch.exp commit.act } From 0c49b950e8a0013c00051ad7a598a5ae0a2cdd07 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 23 Mar 2026 02:02:07 -0400 Subject: [PATCH 08/20] t: add matching negative attributes to test_decode_color Most of the ANSI color attributes have an "off" variant. We don't use these yet in our test suite, so we never bothered to decode them. Add the ones that match the attributes we encode so we can make use of them. There are even more attributes not covered on the positive side, so this is meant to be useful but not all-inclusive. Note that "nobold" and "nodim" are the same code, so I've decoded this as "normal intensity". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 14e238d24da9ad..f3af10fb7e0205 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -48,6 +48,9 @@ test_decode_color () { if (n == 2) return "FAINT"; if (n == 3) return "ITALIC"; if (n == 7) return "REVERSE"; + if (n == 22) return "NORMAL_INTENSITY"; + if (n == 23) return "NOITALIC"; + if (n == 27) return "NOREVERSE"; if (n == 30) return "BLACK"; if (n == 31) return "RED"; if (n == 32) return "GREEN"; From e57daf91ed0bde19595543c68c90948d5c47ebc6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 23 Mar 2026 02:02:10 -0400 Subject: [PATCH 09/20] diff-highlight: use test_decode_color in tests The diff-highlight tests use raw color bytes when comparing expected and actual output. Let's use test_decode_color, which is our usual technique in other tests. It makes reading test output diffs a bit easier, since you're not relying on your terminal to interpret the result (or worse, interpreting characters yourself via "cat -A"). This will also make it easier to add tests with new colors/attributes, without having to pre-define the byte sequences ourselves. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- .../diff-highlight/t/t9400-diff-highlight.sh | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index 7ebff8b18ff38d..4f3d55a26e66bf 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd) TEST_DIRECTORY="$CURR_DIR"/../../../t DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight -CW="$(printf "\033[7m")" # white -CR="$(printf "\033[27m")" # reset - GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . "$TEST_DIRECTORY"/test-lib.sh @@ -42,9 +39,9 @@ dh_test () { } >/dev/null && "$DIFF_HIGHLIGHT" diff.hi && - test_strip_patch_header diff.act && + test_strip_patch_header diff.act && "$DIFF_HIGHLIGHT" commit.hi && - test_strip_patch_header commit.act && + test_strip_patch_header commit.act && test_cmp patch.exp diff.act && test_cmp patch.exp commit.act } @@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -${CW}b${CR}bb - +${CW}0${CR}bb + -bbb + +0bb ccc EOF ' @@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -bb${CW}b${CR} - +bb${CW}0${CR} + -bbb + +bb0 ccc EOF ' @@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -b${CW}b${CR}b - +b${CW}0${CR}b + -bbb + +b0b ccc EOF ' @@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -b${CW}b${CR}b - +b${CW}0${CR}b + -bbb + +b0b +ccc EOF ' @@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' ' echo "unic${o_stroke}de" >b && dh_test a b <<-EOF @@ -1 +1 @@ - -unic${CW}${o_accent}${CR}de - +unic${CW}${o_stroke}${CR}de + -unic${o_accent}de + +unic${o_stroke}de EOF ' @@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' ' echo "unico${combine_circum}de" >b && dh_test a b <<-EOF @@ -1 +1 @@ - -unic${CW}o${combine_accent}${CR}de - +unic${CW}o${combine_circum}${CR}de + -unico${combine_accent}de + +unico${combine_circum}de EOF ' @@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' ' +++ b/file @@ -1,3 +1,3 @@ before - -the ${CW}old${CR} line - +the ${CW}new${CR} line + -the old line + +the new line -leading dash EOF git log --graph -p -1 | "$DIFF_HIGHLIGHT" >actual.raw && - trim_graph actual && + trim_graph actual && test_cmp expect actual ' From c6bc53ad95d5a7dda3d3a8b1bc984465c9024342 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 23 Mar 2026 02:02:13 -0400 Subject: [PATCH 10/20] diff-highlight: test color config We added configurable colors long ago in bca45fbc1f (diff-highlight: allow configurable colors, 2014-11-20), but never actually tested it. Since we'll be touching the color code in a moment, this is a good time to beef up the tests. Note that we cover both the highlight/reset style used by the default colors, as well as the normal/highlight style added by that commit (which was previously totally untested). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- .../diff-highlight/t/t9400-diff-highlight.sh | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index 4f3d55a26e66bf..b38fe2196a9cbe 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -350,4 +350,32 @@ test_expect_success 'highlight diff that removes final newline' ' EOF ' +test_expect_success 'configure set/reset colors' ' + test_config color.diff-highlight.oldhighlight bold && + test_config color.diff-highlight.oldreset nobold && + test_config color.diff-highlight.newhighlight italic && + test_config color.diff-highlight.newreset noitalic && + echo "prefix a suffix" >a && + echo "prefix b suffix" >b && + dh_test a b <<-\EOF + @@ -1 +1 @@ + -prefix a suffix + +prefix b suffix + EOF +' + +test_expect_success 'configure normal/highlight colors' ' + test_config color.diff-highlight.oldnormal red && + test_config color.diff-highlight.oldhighlight magenta && + test_config color.diff-highlight.newnormal green && + test_config color.diff-highlight.newhighlight yellow && + echo "prefix a suffix" >a && + echo "prefix b suffix" >b && + dh_test a b <<-\EOF + @@ -1 +1 @@ + -prefix a suffix + +prefix b suffix + EOF +' + test_done From bd958e91dffdba00ef94dc9bfc04b46599362f9a Mon Sep 17 00:00:00 2001 From: Scott Baker Date: Mon, 23 Mar 2026 02:02:15 -0400 Subject: [PATCH 11/20] diff-highlight: allow module callers to pass in color config Users of the module may want to pass in their own color config for a few obvious reasons: - they are pulling the config from different variables than diff-highlight itself uses - they are loading the config in a more efficient way (say, by parsing git-config --list) and don't want to incur the six (!) git-config calls that DiffHighlight.pm runs to check all config Let's allow users of the module to pass in the color config, and lazy-load it when needed if they haven't. Signed-off-by: Scott Baker Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/DiffHighlight.pm | 41 +++++++++++++++++-------- contrib/diff-highlight/README | 6 ++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm index a5e5de3b18491c..96369eadf94fba 100644 --- a/contrib/diff-highlight/DiffHighlight.pm +++ b/contrib/diff-highlight/DiffHighlight.pm @@ -9,18 +9,11 @@ use File::Spec; my $NULL = File::Spec->devnull(); -# Highlight by reversing foreground and background. You could do -# other things like bold or underline if you prefer. -my @OLD_HIGHLIGHT = ( - color_config('color.diff-highlight.oldnormal'), - color_config('color.diff-highlight.oldhighlight', "\x1b[7m"), - color_config('color.diff-highlight.oldreset', "\x1b[27m") -); -my @NEW_HIGHLIGHT = ( - color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]), - color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]), - color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2]) -); +# The color theme is initially set to nothing here to allow outside callers +# to set the colors for their application. If nothing is sent in we use +# colors from git config in load_color_config(). +our @OLD_HIGHLIGHT = (); +our @NEW_HIGHLIGHT = (); my $RESET = "\x1b[m"; my $COLOR = qr/\x1b\[[0-9;]*m/; @@ -170,6 +163,29 @@ sub show_hunk { $line_cb->(@queue); } +sub load_color_config { + # If the colors were NOT set from outside this module we load them on-demand + # from the git config. Note that only one of elements 0 and 2 in each + # array is used (depending on whether you are doing set/unset on an + # attribute, or specifying normal vs highlighted coloring). So we use + # element 1 as our check for whether colors were passed in; it should + # always be set if you want highlighting to do anything. + if (!defined $OLD_HIGHLIGHT[1]) { + @OLD_HIGHLIGHT = ( + color_config('color.diff-highlight.oldnormal'), + color_config('color.diff-highlight.oldhighlight', "\x1b[7m"), + color_config('color.diff-highlight.oldreset', "\x1b[27m") + ); + } + if (!defined $NEW_HIGHLIGHT[1]) { + @NEW_HIGHLIGHT = ( + color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]), + color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]), + color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2]) + ); + }; +} + sub highlight_pair { my @a = split_line(shift); my @b = split_line(shift); @@ -218,6 +234,7 @@ sub highlight_pair { } if (is_pair_interesting(\@a, $pa, $sa, \@b, $pb, $sb)) { + load_color_config(); return highlight_line(\@a, $pa, $sa, \@OLD_HIGHLIGHT), highlight_line(\@b, $pb, $sb, \@NEW_HIGHLIGHT); } diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README index 9c89146fb0a1cb..ed8d876a18af2a 100644 --- a/contrib/diff-highlight/README +++ b/contrib/diff-highlight/README @@ -138,6 +138,12 @@ Your script may set up one or more of the following variables: processing a logical chunk of input). The default function flushes stdout. + - @DiffHighlight::OLD_HIGHLIGHT and @DiffHighlight::NEW_HIGHLIGHT - these + arrays specify the normal, highlighted, and reset colors (in that order) + for old/new lines. If unset, values will be retrieved by calling `git + config` (see "Color Config" above). Note that these should be the literal + color bytes (starting with an ANSI escape code), not color names. + The script may then feed lines, one at a time, to DiffHighlight::handle_line(). When lines are done processing, they will be fed to $line_cb. Note that DiffHighlight may queue up many input lines (to analyze a whole hunk) From 6689a6ea493b484d6a43601f42c1633706c963d6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 23 Mar 2026 02:02:18 -0400 Subject: [PATCH 12/20] diff-highlight: fetch all config with one process When diff-highlight was written, there was no way to fetch multiple config keys _and_ have them interpreted as colors. So we were stuck with either invoking git-config once for each config key, or fetching them all and converting human-readable color names into ANSI codes ourselves. I chose the former, but it means that diff-highlight kicks off 6 git-config processes (even if you haven't configured anything, it has to check each one). But since Git 2.18.0, we can do: git config --type=color --get-regexp=^color\.diff-highlight\. to get all of them in one shot. Note that any callers which pass in colors directly to the module via @OLD_HIGHLIGHT and @NEW_HIGHLIGHT (like diff-so-fancy plans to do) are unaffected; those colors suppress any config lookup we'd do ourselves. You can see the effect like: # diff-highlight suppresses git-config's stderr, so dump # trace through descriptor 3 git show d1f33c753d | GIT_TRACE=3 diff-highlight 3>&2 >/dev/null which drops from 6 lines down to 1. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- contrib/diff-highlight/DiffHighlight.pm | 28 ++++++++++++++++++------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm index 96369eadf94fba..abe457882e5117 100644 --- a/contrib/diff-highlight/DiffHighlight.pm +++ b/contrib/diff-highlight/DiffHighlight.pm @@ -131,9 +131,21 @@ sub highlight_stdin { # of it being used in other settings. Let's handle our own # fallback, which means we will work even if git can't be run. sub color_config { + our $cached_config; my ($key, $default) = @_; - my $s = `git config --get-color $key 2>$NULL`; - return length($s) ? $s : $default; + + if (!defined $cached_config) { + $cached_config = {}; + my $data = `git config --type=color --get-regexp '^color\.diff-highlight\.' 2>$NULL`; + for my $line (split /\n/, $data) { + my ($key, $color) = split ' ', $line, 2; + $key =~ s/^color\.diff-highlight\.// or next; + $cached_config->{$key} = $color; + } + } + + my $s = $cached_config->{$key}; + return defined($s) ? $s : $default; } sub show_hunk { @@ -172,16 +184,16 @@ sub load_color_config { # always be set if you want highlighting to do anything. if (!defined $OLD_HIGHLIGHT[1]) { @OLD_HIGHLIGHT = ( - color_config('color.diff-highlight.oldnormal'), - color_config('color.diff-highlight.oldhighlight', "\x1b[7m"), - color_config('color.diff-highlight.oldreset', "\x1b[27m") + color_config('oldnormal'), + color_config('oldhighlight', "\x1b[7m"), + color_config('oldreset', "\x1b[27m") ); } if (!defined $NEW_HIGHLIGHT[1]) { @NEW_HIGHLIGHT = ( - color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]), - color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]), - color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2]) + color_config('newnormal', $OLD_HIGHLIGHT[0]), + color_config('newhighlight', $OLD_HIGHLIGHT[1]), + color_config('newreset', $OLD_HIGHLIGHT[2]) ); }; } From 649c768a25b191dbea72c7bacaed5b81ef195bf5 Mon Sep 17 00:00:00 2001 From: K Jayatheerth Date: Tue, 24 Mar 2026 07:27:33 +0530 Subject: [PATCH 13/20] remote-curl: fall back to default hash outside repo When a remote helper like git-remote-http is invoked outside of a repository (for example, by running git ls-remote in a non-git directory), setup_git_directory_gently() leaves the_hash_algo uninitialized as NULL. If the user has a globally configured fetch refspec, remote-curl attempts to parse it during initialization. Inside parse_refspec(), it checks whether the LHS of the refspec is an exact OID by evaluating llen == the_hash_algo->hexsz. Because the_hash_algo is NULL, this results in a segmentation fault. In 9e89dcb66a (builtin/ls-remote: fall back to SHA1 outside of a repo, 2024-08-02), we added a workaround to ls-remote to fall back to the default hash algorithm to prevent exactly this type of crash when parsing refspec capabilities. However, because remote-curl runs as a separate process, it does not inherit that fallback and crashes anyway. Instead of pushing a NULL-guard workaround down into parse_refspec(), fix this by mirroring the ls-remote workaround directly in remote-curl.c. If we are operating outside a repository, initialize the_hash_algo to GIT_HASH_DEFAULT. This keeps the HTTP transport consistent with non-HTTP transports that execute in-process, preventing crashes without altering the generic refspec parsing logic. Reported-by: Jo Liss Helped-by: Jeff King Signed-off-by: K Jayatheerth Signed-off-by: Junio C Hamano --- remote-curl.c | 7 +++++++ t/t5551-http-fetch-smart.sh | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/remote-curl.c b/remote-curl.c index 69f919454a4565..a489cbe6f4aff0 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -1551,6 +1551,13 @@ int cmd_main(int argc, const char **argv) goto cleanup; } + /* + * yuck, see 9e89dcb66a (builtin/ls-remote: fall back to SHA1 outside + * of a repo, 2024-08-02) + */ + if (nongit) + repo_set_hash_algo(the_repository, GIT_HASH_DEFAULT); + options.verbosity = 1; options.progress = !!isatty(2); options.thin = 1; diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index 73cf5315800fa4..a26b6c284401a2 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -782,4 +782,11 @@ test_expect_success 'tag following always works over v0 http' ' test_cmp expect actual ' +test_expect_success 'ls-remote outside repo does not segfault with fetch refspec' ' + nongit git \ + -c remote.origin.url="$HTTPD_URL/smart/repo.git" \ + -c remote.origin.fetch=anything \ + ls-remote origin +' + test_done From 4e5dc601ddc5f0d8ab035210554d9e15aa376032 Mon Sep 17 00:00:00 2001 From: K Jayatheerth Date: Tue, 24 Mar 2026 07:27:34 +0530 Subject: [PATCH 14/20] refspec: fix typo in comment Fix a long-standing typo in a comment: "refpsecs" -> "refspecs". Signed-off-by: K Jayatheerth Signed-off-by: Junio C Hamano --- refspec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refspec.c b/refspec.c index 0775358d96cacd..fb89bce1db23fb 100644 --- a/refspec.c +++ b/refspec.c @@ -85,7 +85,7 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet if (!*item->src) return 0; /* negative refspecs must not be empty */ else if (llen == the_hash_algo->hexsz && !get_oid_hex(item->src, &unused)) - return 0; /* negative refpsecs cannot be exact sha1 */ + return 0; /* negative refspecs cannot be exact sha1 */ else if (!check_refname_format(item->src, flags)) ; /* valid looking ref is ok */ else From 250e977a2b0aa8cc1c8063c64c44597a166e79f5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 24 Mar 2026 12:26:58 -0700 Subject: [PATCH 15/20] use strvec_pushv() to add another strvec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add and apply a semantic patch that simplifies the code by letting strvec_pushv() append the items of a second strvec instead of pushing them one by one. Suggested-by: Junio C Hamano Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/rebase.c | 3 +-- fetch-pack.c | 8 ++---- git.c | 3 +-- submodule.c | 4 +-- tools/coccinelle/strvec.cocci | 46 +++++++++++++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 tools/coccinelle/strvec.cocci diff --git a/builtin/rebase.c b/builtin/rebase.c index a1c7d78196750e..fa4f5d9306b856 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -182,8 +182,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) replay.signoff = opts->signoff; - for (size_t i = 0; i < opts->trailer_args.nr; i++) - strvec_push(&replay.trailer_args, opts->trailer_args.v[i]); + strvec_pushv(&replay.trailer_args, opts->trailer_args.v); replay.allow_ff = !(opts->flags & REBASE_FORCE); if (opts->allow_rerere_autoupdate) diff --git a/fetch-pack.c b/fetch-pack.c index 6ecd468ef766a8..a32224ed026592 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1024,12 +1024,8 @@ static int get_pack(struct fetch_pack_args *args, fsck_msg_types.buf); } - if (index_pack_args) { - int i; - - for (i = 0; i < cmd.args.nr; i++) - strvec_push(index_pack_args, cmd.args.v[i]); - } + if (index_pack_args) + strvec_pushv(index_pack_args, cmd.args.v); sigchain_push(SIGPIPE, SIG_IGN); diff --git a/git.c b/git.c index 2b212e6675d926..5a40eab8a26a66 100644 --- a/git.c +++ b/git.c @@ -877,8 +877,7 @@ static int run_argv(struct strvec *args) commit_pager_choice(); strvec_push(&cmd.args, "git"); - for (size_t i = 0; i < args->nr; i++) - strvec_push(&cmd.args, args->v[i]); + strvec_pushv(&cmd.args, args->v); trace_argv_printf(cmd.args.v, "trace: exec:"); diff --git a/submodule.c b/submodule.c index cd879a5cfe73ec..4c8c674aa4b047 100644 --- a/submodule.c +++ b/submodule.c @@ -1815,7 +1815,6 @@ int fetch_submodules(struct repository *r, int default_option, int quiet, int max_parallel_jobs) { - int i; struct submodule_parallel_fetch spf = SPF_INIT; const struct run_process_parallel_opts opts = { .tr2_category = "submodule", @@ -1842,8 +1841,7 @@ int fetch_submodules(struct repository *r, die(_("index file corrupt")); strvec_push(&spf.args, "fetch"); - for (i = 0; i < options->nr; i++) - strvec_push(&spf.args, options->v[i]); + strvec_pushv(&spf.args, options->v); strvec_push(&spf.args, "--recurse-submodules-default"); /* default value, "--submodule-prefix" and its value are added later */ diff --git a/tools/coccinelle/strvec.cocci b/tools/coccinelle/strvec.cocci new file mode 100644 index 00000000000000..64edb09f1c76fd --- /dev/null +++ b/tools/coccinelle/strvec.cocci @@ -0,0 +1,46 @@ +@@ +type T; +identifier i; +expression dst; +struct strvec *src_ptr; +struct strvec src_arr; +@@ +( +- for (T i = 0; i < src_ptr->nr; i++) { strvec_push(dst, src_ptr->v[i]); } ++ strvec_pushv(dst, src_ptr->v); +| +- for (T i = 0; i < src_arr.nr; i++) { strvec_push(dst, src_arr.v[i]); } ++ strvec_pushv(dst, src_arr.v); +) + +@ separate_loop_index @ +type T; +identifier i; +expression dst; +struct strvec *src_ptr; +struct strvec src_arr; +@@ + T i; + ... +( +- for (i = 0; i < src_ptr->nr; i++) { strvec_push(dst, src_ptr->v[i]); } ++ strvec_pushv(dst, src_ptr->v); +| +- for (i = 0; i < src_arr.nr; i++) { strvec_push(dst, src_arr.v[i]); } ++ strvec_pushv(dst, src_arr.v); +) + +@ unused_loop_index extends separate_loop_index @ +@@ + { + ... +- T i; + ... when != i + } + +@ depends on unused_loop_index @ +@@ + if (...) +- { + strvec_pushv(...); +- } From 3ad4921838a0b118714d3d5c70ab8f31a7c38176 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 25 Mar 2026 02:13:57 -0400 Subject: [PATCH 16/20] t0061: simplify .bat test The test added by 71f4960b91 (t0061: fix test for argv[0] with spaces (MINGW only), 2019-10-01) checks that we can use a .bat file with spaces as GIT_SSH. This is a good test in the sense that it's how the original bug was detected. And as the commit message there describes, there are some elements of the bug that are likely to come up with GIT_SSH and not elsewhere: namely that in addition to the .bat file having spaces, we must pass an argument with spaces (which happens naturally with ssh, since we pass the upload-pack shell command for the other side to run). But using GIT_SSH does complicate matters: 1. We actually run the ssh command _twice_, once to probe the ssh variant with "-G" in fill_ssh_args(), and then a second time to actually make the connection. So we have to account for that when checking the output. 2. Our fake ssh .bat file does not actually run ssh. So we expect the command to fail, but not before the .bat file has touched the "out" marker file that tells us it has run. This works now, but is fragile. In particular, the .bat file by default will echo commands it runs to stdout. From the perspective of the parent Git process, this is protocol-breaking garbage, and upon seeing it will die(). That is OK for now because we don't bother to do any cleanup of the child process. But there is a patch under discussion, dd3693eb08 (transport-helper, connect: use clean_on_exit to reap children on abnormal exit, 2026-03-12), which causes us to kill() the .bat process. This happens before it actually touches the "out" file, causing the test to fail. We can simplify this by just using the "test-tool run-command" helper. That lets us run whatever command we like with the arguments we want. The argument here has a space, which is enough to trigger the original bug that 71f4960b91 was testing. I verified that by reverting eb7c786314 (mingw: support spawning programs containing spaces in their names, 2019-07-16), the original fix, and confirming that the test fails (but succeeds without the revert). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t0061-run-command.sh | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 76d4936a879afd..a80847783b7999 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -256,16 +256,8 @@ test_expect_success MINGW 'can spawn .bat with argv[0] containing spaces' ' rm -f out && echo "echo %* >>out" >"$bat" && - # Ask git to invoke .bat; clone will fail due to fake SSH helper - test_must_fail env GIT_SSH="$bat" git clone myhost:src ssh-clone && - - # Spawning .bat can fail if there are two quoted cmd.exe arguments. - # .bat itself is first (due to spaces in name), so just one more is - # needed to verify. GIT_SSH will invoke .bat multiple times: - # 1) -G myhost - # 2) myhost "git-upload-pack src" - # First invocation will always succeed. Test the second one. - grep "git-upload-pack" out + test-tool run-command run-command "$bat" "arg with spaces" && + test_grep "arg with spaces" out ' test_done From 4be77c732c9951a60f743af04a5906fdc41c5795 Mon Sep 17 00:00:00 2001 From: Mahi Kassa Date: Wed, 25 Mar 2026 12:51:47 +0100 Subject: [PATCH 17/20] repo: factor repo usage strings into shared macros Factor the "git repo info" and "git repo structure" usage strings into shared macros so they can be reused in multiple usage arrays. This is a preparatory refactoring for subsequent changes to subcommand-specific help output. Signed-off-by: Mahi Kassa Signed-off-by: Junio C Hamano --- builtin/repo.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/builtin/repo.c b/builtin/repo.c index 55f9b9095c1f92..b5146499d08d33 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -20,11 +20,17 @@ #include "tree-walk.h" #include "utf8.h" +#define REPO_INFO_USAGE \ + "git repo info [--format=(lines|nul) | -z] [--all | ...]", \ + "git repo info --keys [--format=(lines|nul) | -z]" + +#define REPO_STRUCTURE_USAGE \ + "git repo structure [--format=(table|lines|nul) | -z]" + static const char *const repo_usage[] = { - "git repo info [--format=(lines|nul) | -z] [--all | ...]", - "git repo info --keys [--format=(lines|nul) | -z]", - "git repo structure [--format=(table|lines|nul) | -z]", - NULL + REPO_INFO_USAGE, + REPO_STRUCTURE_USAGE, + NULL, }; typedef int get_value_fn(struct repository *repo, struct strbuf *buf); From abd728cdf6ea0164cfd7ede2223f669037ff531d Mon Sep 17 00:00:00 2001 From: Mahi Kassa Date: Wed, 25 Mar 2026 12:51:48 +0100 Subject: [PATCH 18/20] repo: show subcommand-specific help text Use subcommand-specific usage arrays for "git repo info" and "git repo structure" so that each command shows only its own synopsis in help output. Add tests to cover the subcommand help behavior. Signed-off-by: Mahi Kassa Signed-off-by: Junio C Hamano --- builtin/repo.c | 14 ++++++++++++-- t/t1900-repo-info.sh | 6 ++++++ t/t1901-repo-structure.sh | 6 ++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/builtin/repo.c b/builtin/repo.c index b5146499d08d33..71a5c1c29c05fe 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -33,6 +33,16 @@ static const char *const repo_usage[] = { NULL, }; +static const char *const repo_info_usage[] = { + REPO_INFO_USAGE, + NULL, +}; + +static const char *const repo_structure_usage[] = { + REPO_STRUCTURE_USAGE, + NULL, +}; + typedef int get_value_fn(struct repository *repo, struct strbuf *buf); enum output_format { @@ -220,7 +230,7 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix, OPT_END() }; - argc = parse_options(argc, argv, prefix, options, repo_usage, 0); + argc = parse_options(argc, argv, prefix, options, repo_info_usage, 0); if (show_keys && (all_keys || argc)) die(_("--keys cannot be used with a or --all")); @@ -885,7 +895,7 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix, OPT_END() }; - argc = parse_options(argc, argv, prefix, options, repo_usage, 0); + argc = parse_options(argc, argv, prefix, options, repo_structure_usage, 0); if (argc) usage(_("too many arguments")); diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh index a9eb07abe8aaea..39bb77dda0c327 100755 --- a/t/t1900-repo-info.sh +++ b/t/t1900-repo-info.sh @@ -149,4 +149,10 @@ test_expect_success 'git repo info --keys uses lines as its default output forma test_cmp expect actual ' +test_expect_success 'git repo info -h shows only repo info usage' ' + test_must_fail git repo info -h >actual && + test_grep "git repo info" actual && + test_grep ! "git repo structure" actual +' + test_done diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh index 98921ce1cbb66f..10050abd70fd67 100755 --- a/t/t1901-repo-structure.sh +++ b/t/t1901-repo-structure.sh @@ -224,4 +224,10 @@ test_expect_success 'progress meter option' ' ) ' +test_expect_success 'git repo structure -h shows only repo structure usage' ' + test_must_fail git repo structure -h >actual && + test_grep "git repo structure" actual && + test_grep ! "git repo info" actual +' + test_done From bd66ed316c868aacdb6b5e3a21751c5bab3a4f31 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 23 Mar 2026 09:54:42 -0700 Subject: [PATCH 19/20] regexp: leave a pointer to resurrect workaround for Homebrew Recently some GitHub CI jobs were broken by update on the platform side, which was eventually resolved by image rollback, but in the meantime Dscho invented a workaround patch to sidestep the broken part of the platform. Their future image update may contain the same bug, in which case the workaround may again become needed. As we do not want to be building with workaround that avoids system regexp library altogether unless the system is known to be broken, so short of an automated "detect broken system and apply workaround" mechanism, let's use the folks who are compiling the code to detect breakage on their system and cope with the breakage ;-) Signed-off-by: Junio C Hamano --- compat/regcomp_enhanced.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/compat/regcomp_enhanced.c b/compat/regcomp_enhanced.c index 84193ce53b64f5..29c74eee995043 100644 --- a/compat/regcomp_enhanced.c +++ b/compat/regcomp_enhanced.c @@ -3,6 +3,11 @@ int git_regcomp(regex_t *preg, const char *pattern, int cflags) { + /* + * If you are on macOS with clang and fail to compile this line, + * https://lore.kernel.org/git/458ad3c1-96df-4575-ee42-e6eb754f25f6@gmx.de/ + * might be relevant. + */ if (!(cflags & REG_EXTENDED)) cflags |= REG_ENHANCED; return regcomp(preg, pattern, cflags); From cf2139f8e1680b076e115bc0b349e369b4b0ecc4 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 1 Apr 2026 10:28:07 -0700 Subject: [PATCH 20/20] The 24th batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.54.0.adoc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/Documentation/RelNotes/2.54.0.adoc b/Documentation/RelNotes/2.54.0.adoc index f7b2db616d0413..85b15284f3b958 100644 --- a/Documentation/RelNotes/2.54.0.adoc +++ b/Documentation/RelNotes/2.54.0.adoc @@ -104,6 +104,11 @@ UI, Workflows & Features line number when it encounters a corrupt patch, and correctly resets the line counter when processing multiple patch files. + * The HTTP transport learned to react to "429 Too Many Requests". + + * "git repo info -h" and "git repo structure -h" limit their help output + to the part that is specific to the subcommand. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -253,6 +258,15 @@ Performance, Internal Implementation, Development Support etc. of branches interpret_branch_name() function has been changed to use a dedicated enum type. + * Various updates to contrib/diff-highlight, including documentation + updates, test improvements, and color configuration handling. + + * Code paths that loop over another array to push each element into a + strvec have been rewritten to use strvec_pushv() instead. + + * In case homebrew breaks REG_ENHANCED again, leave a in-code comment + to suggest use of our replacement regex as a workaround. + Fixes since v2.53 ----------------- @@ -420,6 +434,17 @@ Fixes since v2.53 * "git apply -p" parses more carefully now. (merge d05d84c5f5 mf/apply-p-no-atoi later to maint). + * A test to run a .bat file with whitespaces in the name with arguments + with whitespaces in them was flaky in that sometimes it got killed + before it produced expected side effects, which has been rewritten to + make it more robust. + (merge 3ad4921838 jk/t0061-bat-test-update later to maint). + + * "git ls-remote '+refs/tags/*:refs/tags/*' https://..." run outside a + repository would dereference a NULL while trying to see if the given + refspec is a single-object refspec, which has been corrected. + (merge 4e5dc601dd kj/refspec-parsing-outside-repository later to maint). + * Other code cleanup, docfix, build fix, etc. (merge d79fff4a11 jk/remote-tracking-ref-leakfix later to maint). (merge 7a747f972d dd/t5403-modernise later to maint).