From 59454646906cc0d191170e69c5f3f080ad838995 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 1 Sep 2026 15:53:01 +0000 Subject: [PATCH 01/10] t/lib-httpd: fix apply-one-time-script race under concurrent requests apply-one-time-script.sh is a test helper that executes a "one-time-script" responsible for modifying the response normally returned by git-http-backend. apply-one-time-script.sh should run "one-time-script" once and return a modified response once. However, sometimes a race between multiple concurrent requests causes apply-one-time-script.sh to misbehave and return multiple modified responses or an empty response that results in: fatal: ... The requested URL returned error: 500 fatal: could not fetch from promisor remote This can be seen in the flaky failure of t5616.47 on the macOS CI runners. Fix the logic that checks if "one-time-script" has returned its modified response by chaining "rm one-time-script" with its execution. This ensures a racing script does not also have the opportunity to execute "one-time-script". Add t/t5567-one-time-script.sh to verify the race is fixed. Implement a stub "git-http-backend" that intentionally invokes a concurrent request, and check that only one modified response is returned without error. Signed-off-by: Michael Montalbo Signed-off-by: Junio C Hamano --- t/lib-httpd/apply-one-time-script.sh | 38 +++++++---- t/meson.build | 1 + t/t5567-one-time-script.sh | 96 ++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 14 deletions(-) create mode 100755 t/t5567-one-time-script.sh diff --git a/t/lib-httpd/apply-one-time-script.sh b/t/lib-httpd/apply-one-time-script.sh index b1682944e280e2..eac21a3a8e739a 100644 --- a/t/lib-httpd/apply-one-time-script.sh +++ b/t/lib-httpd/apply-one-time-script.sh @@ -6,21 +6,31 @@ # # This can be used to simulate the effects of the repository changing in # between HTTP request-response pairs. -if test -f one-time-script -then - LC_ALL=C - export LC_ALL +test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend" + +LC_ALL=C +export LC_ALL - "$GIT_EXEC_PATH/git-http-backend" >out - ./one-time-script out >out_modified +out=out.$$ +modified=out-modified.$$ +"$GIT_EXEC_PATH/git-http-backend" >"$out" - if cmp -s out out_modified - then - cat out - else - cat out_modified - rm one-time-script - fi +# Since Apache can execute this script for multiple requests +# concurrently, we chain "rm one-time-script" with the logic +# for generating a modified response. If the "rm" ran separately, +# a concurrent request could pass the "test -f" above and +# erroneously result in multiple modified responses or an empty +# body depending on the race state. +# +# We discard stderr for ./one-time-script since it is possible +# ./one-time-script has been removed already, which is expected +# sometimes. In this case, the unmodified response will be returned. +if ./one-time-script "$out" 2>/dev/null >"$modified" && + ! cmp -s "$out" "$modified" && + rm one-time-script 2>/dev/null +then + cat "$modified" else - "$GIT_EXEC_PATH/git-http-backend" + cat "$out" fi +rm -f "$out" "$modified" diff --git a/t/meson.build b/t/meson.build index 3219264fe7d497..a118a4d7196b17 100644 --- a/t/meson.build +++ b/t/meson.build @@ -707,6 +707,7 @@ integration_tests = [ 't5564-http-proxy.sh', 't5565-push-multiple.sh', 't5566-push-group.sh', + 't5567-one-time-script.sh', 't5570-git-daemon.sh', 't5571-pre-push-hook.sh', 't5572-pull-submodule.sh', diff --git a/t/t5567-one-time-script.sh b/t/t5567-one-time-script.sh new file mode 100755 index 00000000000000..a8429ef3c3d092 --- /dev/null +++ b/t/t5567-one-time-script.sh @@ -0,0 +1,96 @@ +#!/bin/sh + +test_description='apply-one-time-script CGI helper is safe under concurrent requests' + +. ./test-lib.sh + +HELPER="$TEST_DIRECTORY/lib-httpd/apply-one-time-script.sh" + +test_expect_success PIPE 'helper only serves one rewritten response for concurrent requests' ' + mkdir workdir fakebin && + ENTERED="$PWD/entered" && + GATE="$PWD/gate" && + export ENTERED GATE && + mkfifo "$ENTERED" "$GATE" && + + # A stub git-http-backend that returns a response based on + # $ROLE. For $ROLE = modify, return the response string + # "packfile", which ends up being modified by the example + # one-time-script below. + # + # Otherwise, run the branch returning a response that + # should be passed through, and block until released + # by "read -r $GATE". + write_script fakebin/git-http-backend <<-\EOF && + printf "Status: 200 OK\r\n" + printf "Content-Type: application/x-git-result\r\n" + printf "\r\n" + if test "$ROLE" = modify + then + printf "packfile\n" + else + echo entered >"$ENTERED" + read -r released <"$GATE" + printf "refs\n" + fi + EOF + + # An example one-time-script for apply-one-time-script + # to execute. Checks for "packfile" in the response + # that will be returned, and replaces it with a + # modified response. Passes through responses without + # "packfile" in them. + write_script workdir/one-time-script <<-\EOF && + if grep packfile "$1" >/dev/null + then + sed "/packfile/q" "$1" && + printf "REPLACED\n" + else + cat "$1" + fi + EOF + + GIT_EXEC_PATH="$PWD/fakebin" && + export GIT_EXEC_PATH && + + # Ensure $GATE has a reader so the test does not block indefinitely if + # the helper is buggy and "echo released >&9" below does not unblock + # the unmodified response gate. + exec 9<>"$GATE" && + + # Launch the passthrough request in the background. Record its pid + # so it can be killed when the test finishes if, for some reason, the + # request stays blocked and would stall a test runner. + { ( + cd workdir && + ROLE=passthrough sh "$HELPER" >../passthrough.out 2>../passthrough.err + ) & } && + passthrough_pid=$! && + test_when_finished "kill $passthrough_pid 2>/dev/null || :" && + + # Wait until the passthrough request is "in-flight" and paused + # mid-response. + read -r entered <"$ENTERED" && + + # Launch the request for a modified response while the passthrough + # request is concurrently "in-flight" and paused. + ( + cd workdir && + ROLE=modify sh "$HELPER" >../modify.out 2>../modify.err + ) && + + # Unblock the passthrough request, allowing git-http-backend to + # complete its response. + echo released >&9 && + { wait "$passthrough_pid" || :; } && + + test_must_be_empty passthrough.err && + test_must_be_empty modify.err && + test_grep "Status: 200 OK" passthrough.out && + test_grep "Status: 200 OK" modify.out && + test_grep REPLACED modify.out && + test_grep ! REPLACED passthrough.out && + test_grep refs passthrough.out +' + +test_done From b86132120dd67d67da285f075884aa9efc4c59d7 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 1 Sep 2026 15:53:02 +0000 Subject: [PATCH 02/10] t/lib-httpd: make http-429 first-request check atomic http-429.sh is a helper for testing retry logic. It uses "test -f" to check for the existence of a state file and later uses "touch" or "rm -f" on that file to determine if it should return a 429. This method of managing state can fail if the helper script is invoked concurrently. However, this failure does not currently manifest itself since the helper is invoked sequentially. As a preventive measure, fix the state management logic so it relies on an atomic mkdir operation to mark that a 429 was returned. When $retry_after is "permanent", always return 429 now that we do not rely on a state file that is "touch"ed and "rm"ed to indicate when to respond with a 429. Signed-off-by: Michael Montalbo Signed-off-by: Junio C Hamano --- t/lib-httpd/http-429.sh | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/t/lib-httpd/http-429.sh b/t/lib-httpd/http-429.sh index c97b16145b7f92..1a5d7987db1fac 100644 --- a/t/lib-httpd/http-429.sh +++ b/t/lib-httpd/http-429.sh @@ -3,7 +3,7 @@ # 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 test-context is a unique identifier for each test to isolate state directories. # 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 @@ -26,14 +26,16 @@ repo_path="${remaining#*/}" # Get rest (repo path) # 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 +# Use current directory (HTTPD_ROOT_PATH) to hold state directory +# Create a safe directory name from test_context, retry_after and repo_name +# This ensures all requests for the same test context share the same state directory safe_name=$(echo "${test_context}-${retry_after}-${repo_name}" | tr '/' '_' | tr -cd 'a-zA-Z0-9_-') -state_file="http-429-state-${safe_name}" +state="http-429-state-${safe_name}" -# Check if this is the first call (no state file exists) -if test -f "$state_file" +# Check if this is the first call (no state directory exists), or if +# the retry-after-value is "permanent", which indicates a 429 must be +# returned for every request (even if the state directory exists). +if test "$retry_after" != permanent && ! mkdir "$state" 2>/dev/null then # Already returned 429 once, forward to git-http-backend # Set PATH_INFO to just the repo path (without retry-after value) @@ -52,9 +54,6 @@ then 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" @@ -67,8 +66,7 @@ case "$retry_after" in printf "Retry-After: invalid-format-123abc\r\n" ;; permanent) - # Always return 429, don't set state file for success - rm -f "$state_file" + # Always return 429 printf "Retry-After: 1\r\n" printf "Content-Type: text/plain\r\n" printf "\r\n" From c2a48fdcb56fed3a79bf43b7eac0a4b5ffb32317 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 1 Sep 2026 15:53:03 +0000 Subject: [PATCH 03/10] t/lib-httpd: document writing concurrency-safe CGI helpers Update t/lib-httpd.sh to document the fixes applied to apply-one-time-script.sh and http-429.sh for future developers working on helper scripts. Add concrete examples of patterns and anti-patterns that should be considered when handling state management. Signed-off-by: Michael Montalbo Signed-off-by: Junio C Hamano --- t/lib-httpd.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index fc646447d5c038..92b619b782ffb1 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -159,6 +159,18 @@ prepare_httpd() { mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH" cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH" cp "$TEST_PATH"/proxy-passwd "$HTTPD_ROOT_PATH" + # Apache can run the following scripts concurrently per request. Make + # sure any state management logic is resilient to race conditions. + # + # For example: + # - use "mkdir dir" to ensure only one request "succeeds" under some + # condition (see http-429.sh). + # - chain (&&) atomic operations like "rm marker" (no -f) with the + # logic that is guarded by the marker instead of relying on a + # separate "test -f" and "rm marker" check + # (see apply-one-time-script.sh). + # - use scratch file names that include the process ID ($$), so + # concurrent requests do not overwrite each other's state. install_script incomplete-length-upload-pack-v2-http.sh install_script incomplete-body-upload-pack-v2-http.sh install_script error-no-report.sh From 609d2a88349963d1b355062eec97506ec0e69f65 Mon Sep 17 00:00:00 2001 From: Wolfgang Faust Date: Tue, 1 Sep 2026 17:13:21 -0700 Subject: [PATCH 04/10] imap-send: add --draft to set IMAP \Draft flag The documented purpose of imap-send is to upload draft emails for sending later, but it did not have any way to mark the messages as \Draft, so some email clients presented the result as an un-editable, un-sendable email even if it happened to be in a "Drafts" folder. Signed-off-by: Wolfgang Faust Signed-off-by: Junio C Hamano --- Documentation/git-imap-send.adoc | 9 ++++++++- git-curl-compat.h | 8 ++++++++ imap-send.c | 15 +++++++++++++-- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/Documentation/git-imap-send.adoc b/Documentation/git-imap-send.adoc index 538b91afc06dde..fab3d82e020271 100644 --- a/Documentation/git-imap-send.adoc +++ b/Documentation/git-imap-send.adoc @@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder SYNOPSIS -------- [synopsis] -git imap-send [-v] [-q] [--[no-]curl] [(--folder|-f) ] +git imap-send [-v] [-q] [--[no-]curl] [--[no-]draft] [(--folder|-f) ] git imap-send --list @@ -55,6 +55,13 @@ OPTIONS using libcurl. Ignored if Git was built with the NO_OPENSSL option set. +`--draft`:: +`--no-draft`:: + Mark uploaded messages with the IMAP `\Draft` flag. The default is `--no-draft`. ++ +With libcurl, `--draft` requires version 8.13.0 or later. +Older libcurl still uploads the message but cannot set the flag. + `--list`:: Run the IMAP LIST command to output a list of all the folders present. diff --git a/git-curl-compat.h b/git-curl-compat.h index dccdd4d6e54158..032aaf7126c977 100644 --- a/git-curl-compat.h +++ b/git-curl-compat.h @@ -67,4 +67,12 @@ #define GIT_CURL_HAVE_CURLOPT_TCP_KEEPCNT #endif +/** + * CURLOPT_UPLOAD_FLAGS and CURLULFLAG_* were added in 8.13.0, + * released in April 2025. + */ +#if LIBCURL_VERSION_NUM >= 0x080D00 +#define GIT_CURL_HAVE_CURLOPT_UPLOAD_FLAGS +#endif + #endif diff --git a/imap-send.c b/imap-send.c index cfd6a5120c50e4..7a50f9e5ab2060 100644 --- a/imap-send.c +++ b/imap-send.c @@ -35,6 +35,7 @@ #include "setup.h" #include "strbuf.h" #ifdef USE_CURL_FOR_IMAP_SEND +#include "git-curl-compat.h" #include "http.h" #endif @@ -49,10 +50,11 @@ static int verbosity; static int list_folders; static int use_curl = USE_CURL_DEFAULT; +static int opt_draft; static char *opt_folder; static char const * const imap_send_usage[] = { - N_("git imap-send [-v] [-q] [--[no-]curl] [(--folder|-f) ] < "), + N_("git imap-send [-v] [-q] [--[no-]curl] [--[no-]draft] [(--folder|-f) ] < "), "git imap-send --list", NULL }; @@ -60,6 +62,7 @@ static char const * const imap_send_usage[] = { static struct option imap_send_options[] = { OPT__VERBOSITY(&verbosity), OPT_BOOL(0, "curl", &use_curl, "use libcurl to communicate with the IMAP server"), + OPT_BOOL(0, "draft", &opt_draft, "mark uploaded messages with the IMAP \\Draft flag"), OPT_STRING('f', "folder", &opt_folder, "folder", "specify the IMAP folder"), OPT_BOOL(0, "list", &list_folders, "list all folders on the IMAP server"), OPT_END() @@ -1416,7 +1419,8 @@ static int imap_store_msg(struct imap_store *ctx, struct strbuf *msg) box = ctx->name; prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix; - ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" ", prefix, box); + ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" %s", prefix, box, + opt_draft ? "(\\Draft) " : ""); imap->caps = imap->rcaps; if (ret != DRV_OK) return ret; @@ -1718,6 +1722,13 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server, curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf); + if (opt_draft) { +#ifdef GIT_CURL_HAVE_CURLOPT_UPLOAD_FLAGS + curl_easy_setopt(curl, CURLOPT_UPLOAD_FLAGS, CURLULFLAG_DRAFT); +#else + warning("--draft requires libcurl 8.13.0 or later"); +#endif + } fprintf(stderr, "Sending %d message%s to %s folder...\n", total, (total != 1) ? "s" : "", server->folder); while (1) { From 786fc390465f535cc2a09922b064e457faa98881 Mon Sep 17 00:00:00 2001 From: Harald Nordgren Date: Thu, 3 Sep 2026 14:39:57 +0000 Subject: [PATCH 05/10] stash: reserve exit status 1 for conflicts "git stash apply", "pop" and "branch" exit with status 1 both when applying the stash entry resulted in conflicts and when they fail for other reasons, so callers cannot tell the two apart. Follow the convention of "git merge-tree" and the merge strategies, which exit with status 1 to indicate conflicts and with a different non-zero status for errors: those subcommands now exit with status 1 only when applying the stash entry resulted in conflicts, in which case the stash entry is left in place, and exit with status 128, the status die() uses, when they fail for other reasons. Document the exit statuses. The only subcommand implementations that can return a positive value are "apply", "pop" and "branch", which return the value of do_apply_stash(): "apply" returns it directly, and "pop" and "branch" drop the stash entry, via do_drop_stash(), which always returns 0, only when the application succeeded. do_apply_stash() only returns a positive value when the three-way merge was unclean. cmd_stash() now maps negative values to 128 and passes positive values through as the exit status, so exit status 1 unambiguously indicates conflicts. enum stash_apply_result makes the convention explicit, and the autostash helpers use it to tell users that their stashed changes were saved when applying them fails. Signed-off-by: Harald Nordgren Signed-off-by: Junio C Hamano --- Documentation/git-stash.adoc | 9 +++ builtin/stash.c | 33 +++++++--- sequencer.c | 113 ++++++++++++++++++++++------------- sequencer.h | 19 +++--- stash.h | 21 +++++++ t/t3903-stash.sh | 25 +++++++- 6 files changed, 160 insertions(+), 60 deletions(-) create mode 100644 stash.h diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc index 50bb89f48362a4..fc6a9a008cf207 100644 --- a/Documentation/git-stash.adoc +++ b/Documentation/git-stash.adoc @@ -426,6 +426,15 @@ include::includes/cmd-config-section-all.adoc[] :git-stash: 1 include::config/stash.adoc[] +EXIT STATUS +----------- + +The `git stash` subcommands exit with status 0 on success. The +subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`, +exit with status 1 when applying the stash entry resulted in conflicts, +in which case the stash entry is left in place, and with a non-zero +status other than 1 when they fail for other reasons. + SEE ALSO -------- diff --git a/builtin/stash.c b/builtin/stash.c index c4809f299a313b..860c9be4a1551a 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -10,6 +10,7 @@ #include "object-name.h" #include "parse-options.h" #include "refs.h" +#include "stash.h" #include "lockfile.h" #include "cache-tree.h" #include "unpack-trees.h" @@ -640,10 +641,12 @@ static void unstage_changes_unless_new(struct object_id *orig_tree) die(_("could not write index")); } -static int do_apply_stash(const char *prefix, struct stash_info *info, - int index, int quiet, - const char *label_ours, const char *label_theirs, - const char *label_base) +static enum stash_apply_result do_apply_stash(const char *prefix, + struct stash_info *info, + int index, int quiet, + const char *label_ours, + const char *label_theirs, + const char *label_base) { int clean, ret; int has_index = index; @@ -717,8 +720,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, /* * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the - * merge was clean, and nonzero if the merge was unclean or encountered - * an error. + * merge was clean, and 1 if the merge was unclean or a negative value + * if it encountered an error. */ ret = clean >= 0 ? !clean : clean; @@ -2492,10 +2495,22 @@ int cmd_stash(int argc, strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file, (uintmax_t)pid); - if (fn) - return !!fn(argc, argv, prefix, repo); - else if (!argc) + if (fn) { + ret = fn(argc, argv, prefix, repo); + + /* + * The subcommand implementations return 0 on success, a + * negative value on failure, and STASH_APPLY_CONFLICT + * when applying a stash entry resulted in conflicts. + * Map failures to 128, the status die() uses, so that + * exit status 1 unambiguously indicates conflicts. + */ + if (ret < 0) + return 128; + return ret; + } else if (!argc) { return !!push_stash_unassumed(0, NULL, prefix, repo); + } /* Assume 'stash push' */ strvec_push(&args, "push"); diff --git a/sequencer.c b/sequencer.c index 57855b0066ac98..f662cfc6c4b381 100644 --- a/sequencer.c +++ b/sequencer.c @@ -19,6 +19,7 @@ #include "commit.h" #include "sequencer.h" #include "run-command.h" +#include "stash.h" #include "hook.h" #include "utf8.h" #include "cache-tree.h" @@ -4727,31 +4728,50 @@ void create_autostash_ref(struct repository *r, const char *refname, create_autostash_internal(r, NULL, refname, message, silent); } -static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply, - const char *label_ours, const char *label_theirs, - const char *label_base, - const char *stash_msg) +static enum stash_apply_result do_stash_apply(const char *stash_oid, + const char *label_ours, + const char *label_theirs, + const char *label_base) { struct child_process child = CHILD_PROCESS_INIT; - int ret = 0; - if (attempt_apply) { - child.git_cmd = 1; - child.no_stdout = 1; - child.no_stderr = 1; - strvec_push(&child.args, "stash"); - strvec_push(&child.args, "apply"); - if (label_ours) - strvec_pushf(&child.args, "--label-ours=%s", label_ours); - if (label_theirs) - strvec_pushf(&child.args, "--label-theirs=%s", label_theirs); - if (label_base) - strvec_pushf(&child.args, "--label-base=%s", label_base); - strvec_push(&child.args, stash_oid); - ret = run_command(&child); - } - - if (attempt_apply && !ret) + child.git_cmd = 1; + child.no_stdout = 1; + child.no_stderr = 1; + strvec_push(&child.args, "stash"); + strvec_push(&child.args, "apply"); + if (label_ours) + strvec_pushf(&child.args, "--label-ours=%s", label_ours); + if (label_theirs) + strvec_pushf(&child.args, "--label-theirs=%s", label_theirs); + if (label_base) + strvec_pushf(&child.args, "--label-base=%s", label_base); + strvec_push(&child.args, stash_oid); + + switch (run_command(&child)) { + case 0: + return STASH_APPLY_CLEAN; + case STASH_APPLY_CONFLICT: + return STASH_APPLY_CONFLICT; + default: + return STASH_APPLY_ERROR; + } +} + +static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid, + int attempt_apply, + const char *label_ours, + const char *label_theirs, + const char *label_base, + const char *stash_msg) +{ + enum stash_apply_result ret = STASH_APPLY_CLEAN; + + if (attempt_apply) + ret = do_stash_apply(stash_oid, label_ours, label_theirs, + label_base); + + if (attempt_apply && ret == STASH_APPLY_CLEAN) fprintf(stderr, _("Applied autostash.\n")); else { struct child_process store = CHILD_PROCESS_INIT; @@ -4765,13 +4785,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply, strvec_push(&store.args, stash_oid); if (run_command(&store)) ret = error(_("cannot store %s"), stash_oid); - else if (attempt_apply) + else if (attempt_apply && ret == STASH_APPLY_CONFLICT) fprintf(stderr, _("Your local changes are stashed, however applying them\n" "resulted in conflicts. You can either resolve the conflicts\n" "and then discard the stash with \"git stash drop\", or, if you\n" "do not want to resolve them now, run \"git reset --hard\" and\n" "apply the local changes later by running \"git stash pop\".\n")); + else if (attempt_apply) + ret = error(_("could not apply autostash; " + "your changes are safe in the stash")); else fprintf(stderr, _("Autostash exists; creating a new stash entry.\n" @@ -4783,15 +4806,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply, return ret; } -static int apply_save_autostash(const char *path, int attempt_apply) +static enum stash_apply_result apply_save_autostash(const char *path, + int attempt_apply) { struct strbuf stash_oid = STRBUF_INIT; - int ret = 0; + enum stash_apply_result ret = STASH_APPLY_CLEAN; if (!read_oneliner(&stash_oid, path, READ_ONELINER_SKIP_IF_EMPTY)) { strbuf_release(&stash_oid); - return 0; + return STASH_APPLY_CLEAN; } strbuf_trim(&stash_oid); @@ -4803,37 +4827,40 @@ static int apply_save_autostash(const char *path, int attempt_apply) return ret; } -int save_autostash(const char *path) +enum stash_apply_result save_autostash(const char *path) { return apply_save_autostash(path, 0); } -int apply_autostash(const char *path) +enum stash_apply_result apply_autostash(const char *path) { return apply_save_autostash(path, 1); } -int apply_autostash_oid(const char *stash_oid) +enum stash_apply_result apply_autostash_oid(const char *stash_oid) { return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL); } -static int apply_save_autostash_ref(struct repository *r, const char *refname, - int attempt_apply, - const char *label_ours, const char *label_theirs, - const char *label_base, - const char *stash_msg) +static enum stash_apply_result apply_save_autostash_ref(struct repository *r, + const char *refname, + int attempt_apply, + const char *label_ours, + const char *label_theirs, + const char *label_base, + const char *stash_msg) { struct object_id stash_oid; char stash_oid_hex[GIT_MAX_HEXSZ + 1]; - int flag, ret; + int flag; + enum stash_apply_result ret; if (!refs_ref_exists(get_main_ref_store(r), refname)) - return 0; + return STASH_APPLY_CLEAN; if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname, RESOLVE_REF_READING, &stash_oid, &flag)) - return -1; + return STASH_APPLY_ERROR; if (flag & REF_ISSYMREF) return error(_("autostash reference is a symref")); @@ -4848,15 +4875,19 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname, return ret; } -int save_autostash_ref(struct repository *r, const char *refname) +enum stash_apply_result save_autostash_ref(struct repository *r, + const char *refname) { return apply_save_autostash_ref(r, refname, 0, NULL, NULL, NULL, NULL); } -int apply_autostash_ref(struct repository *r, const char *refname, - const char *label_ours, const char *label_theirs, - const char *label_base, const char *stash_msg) +enum stash_apply_result apply_autostash_ref(struct repository *r, + const char *refname, + const char *label_ours, + const char *label_theirs, + const char *label_base, + const char *stash_msg) { return apply_save_autostash_ref(r, refname, 1, label_ours, label_theirs, label_base, diff --git a/sequencer.h b/sequencer.h index 3164bd437d6a22..37e44a99fa86e5 100644 --- a/sequencer.h +++ b/sequencer.h @@ -3,6 +3,7 @@ #include "strbuf.h" #include "strvec.h" +#include "stash.h" #include "wt-status.h" struct commit; @@ -231,13 +232,17 @@ void commit_post_rewrite(struct repository *r, void create_autostash(struct repository *r, const char *path); void create_autostash_ref(struct repository *r, const char *refname, const char *message, bool silent); -int save_autostash(const char *path); -int save_autostash_ref(struct repository *r, const char *refname); -int apply_autostash(const char *path); -int apply_autostash_oid(const char *stash_oid); -int apply_autostash_ref(struct repository *r, const char *refname, - const char *label_ours, const char *label_theirs, - const char *label_base, const char *stash_msg); +enum stash_apply_result save_autostash(const char *path); +enum stash_apply_result save_autostash_ref(struct repository *r, + const char *refname); +enum stash_apply_result apply_autostash(const char *path); +enum stash_apply_result apply_autostash_oid(const char *stash_oid); +enum stash_apply_result apply_autostash_ref(struct repository *r, + const char *refname, + const char *label_ours, + const char *label_theirs, + const char *label_base, + const char *stash_msg); #define SUMMARY_INITIAL_COMMIT (1 << 0) #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1) diff --git a/stash.h b/stash.h new file mode 100644 index 00000000000000..14ba4f946d9881 --- /dev/null +++ b/stash.h @@ -0,0 +1,21 @@ +#ifndef STASH_H +#define STASH_H + +enum stash_apply_result { + /* The stash was applied cleanly, or there was nothing to apply. */ + STASH_APPLY_CLEAN = 0, + + /* + * The stash could not be applied because it resulted in + * conflicts. The stash entry is left in place. The "git stash + * apply", "pop" and "branch" subcommands exit with this status + * in this case, mirroring the convention of "git merge-tree" and + * the merge strategies. + */ + STASH_APPLY_CONFLICT = 1, + + /* Something went wrong. */ + STASH_APPLY_ERROR = -1, +}; + +#endif /* STASH_H */ diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index ecc35aae82a5fe..cad2615b2bca99 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -1791,13 +1791,13 @@ test_expect_success 'stash.index=false overridden by --index' ' test_cmp expect file ' -test_expect_success 'apply with custom conflict labels' ' +test_expect_success 'apply exits 1 on conflicts' ' git reset --hard initial && test_commit label-base conflict-file base-content && echo stashed >conflict-file && git stash push -m "stashed" && test_commit label-upstream conflict-file upstream-content && - test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH && + test_expect_code 1 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH && test_grep "^<<<<<<< UP" conflict-file && test_grep "^||||||| Stash base" conflict-file && test_grep "^>>>>>>> STASH" conflict-file @@ -1809,11 +1809,30 @@ test_expect_success 'apply with empty conflict labels' ' echo stashed >conflict-file && git stash push -m "stashed" && test_commit empty-label-upstream conflict-file upstream-content && - test_must_fail git stash apply --label-ours= --label-theirs= && + test_expect_code 1 git stash apply --label-ours= --label-theirs= && test_grep "^<<<<<<<$" conflict-file && test_grep "^>>>>>>>$" conflict-file ' +test_expect_success 'pop exits 1 on conflicts and keeps the stash entry' ' + git reset --hard initial && + echo stashed >file && + git stash push -m pop-stashed && + test_commit pop-upstream file upstream-content && + test_expect_code 1 git stash pop && + git stash list >list && + test_grep pop-stashed list +' + +test_expect_success 'stash branch exits with a non-1 status on errors' ' + git reset --hard initial && + echo stashed >file && + git stash push -m branch-stashed && + test_expect_code 128 git stash branch conflicting-branch refs/heads/does-not-exist && + git stash list >list && + test_grep branch-stashed list +' + test_expect_success 'stash show --include-untracked includes untracked files' ' git reset --hard && From 2ba77ea82891678dee18fe869b0aecaa1019298e Mon Sep 17 00:00:00 2001 From: Harald Nordgren Date: Thu, 3 Sep 2026 14:39:58 +0000 Subject: [PATCH 06/10] checkout: separate autostash conflict advice from branch-switch message "git checkout -m" stashes the user's local changes when it cannot perform the checkout, and then applies the stash. When applying the stash results in conflicts, the advice on how to deal with them is printed directly on top of the branch-switch message ("Switched to branch ..."), making the two hard to tell apart. Print a blank line in between so that the advice and the branch-switch message are visually distinct. apply_autostash_ref() reports whether applying the stash resulted in conflicts via its enum stash_apply_result return value, so only print the blank line in the conflicted case. Signed-off-by: Harald Nordgren Signed-off-by: Junio C Hamano --- builtin/checkout.c | 15 +++++++++------ t/t7201-co.sh | 16 ++++++++++++---- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index b78b3a1d16def4..9cb3e6d0ee6090 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1163,6 +1163,7 @@ static int switch_branches(const struct checkout_opts *opts, int flag, writeout_error = 0; int do_merge = 1; int created_autostash = 0; + enum stash_apply_result autostash_res = STASH_APPLY_CLEAN; struct strbuf old_commit_shortname = STRBUF_INIT; struct strbuf autostash_msg = STRBUF_INIT; const char *stash_label_base = NULL; @@ -1234,12 +1235,12 @@ static int switch_branches(const struct checkout_opts *opts, git_config_push_parameter(cfg.buf); strbuf_release(&cfg); } - apply_autostash_ref(the_repository, - "CHECKOUT_AUTOSTASH_HEAD", - new_branch_info->name, - "local", - stash_label_base, - autostash_msg.buf); + autostash_res = apply_autostash_ref(the_repository, + "CHECKOUT_AUTOSTASH_HEAD", + new_branch_info->name, + "local", + stash_label_base, + autostash_msg.buf); } if (ret) { branch_info_release(&old_branch_info); @@ -1252,6 +1253,8 @@ static int switch_branches(const struct checkout_opts *opts, if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit) orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit); + if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet) + fputc('\n', stderr); update_refs_for_switch(opts, &old_branch_info, new_branch_info); if (created_autostash) { diff --git a/t/t7201-co.sh b/t/t7201-co.sh index 7613b1d2a446b0..641d6239dce544 100755 --- a/t/t7201-co.sh +++ b/t/t7201-co.sh @@ -236,10 +236,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' ' test_must_fail git checkout side 2>stderr && test_grep "Your local changes" stderr && git checkout -m side >actual 2>&1 && - test_grep "resulted in conflicts" actual && - test_grep "git stash drop" actual && - test_grep "git stash pop" actual && - test_grep "The following paths have local changes" actual && + cat >expect <<-EOF && + Your local changes are stashed, however applying them + resulted in conflicts. You can either resolve the conflicts + and then discard the stash with "git stash drop", or, if you + do not want to resolve them now, run "git reset --hard" and + apply the local changes later by running "git stash pop". + + Switched to branch ${SQ}side${SQ} + The following paths have local changes: + M one + EOF + test_cmp expect actual && git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual && sed /^index/d actual >actual.trimmed && cat >expect <<-EOF && From fd6c4dc80be5eb13df3141cb546a3c6515e799ea Mon Sep 17 00:00:00 2001 From: Siddharth Asthana Date: Fri, 4 Sep 2026 02:15:51 +0530 Subject: [PATCH 07/10] rev-list: add --missing-only option to filter output When working with partial clones, callers often need only the missing object IDs. Today that means post-processing --missing=print to drop present objects and strip the leading '?': git rev-list --objects --all --missing=print | perl -ne 'print if s/^[?]//' This is for a one-shot walk, not a fetch loop. Callers already have --missing=print and strip the leading '?'. Gitaly does that when packing a quarantine: '?' lines are objects that must already exist in the main repo. Tests do the same (is this blob still missing). --missing-only is just that list without the prefix. Add --missing-only. Use it with --missing=print or --missing=print-info to print only missing objects. --missing= still picks the format; --missing-only only filters. The leading '?' is omitted. With print-info, path= and type= are still shown. Require --missing=print or --missing=print-info. Reject --count and --disk-usage. Signed-off-by: Siddharth Asthana Signed-off-by: Junio C Hamano --- Documentation/rev-list-options.adoc | 13 ++++++++ builtin/rev-list.c | 42 ++++++++++++++++++++++--- t/t6022-rev-list-missing.sh | 49 +++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 5 deletions(-) diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc index fd831f0ec64744..bd9f3456902c8f 100644 --- a/Documentation/rev-list-options.adoc +++ b/Documentation/rev-list-options.adoc @@ -1083,6 +1083,19 @@ If some tips passed to the traversal are missing, they will be considered as missing too, and the traversal will ignore them. In case we cannot get their Object ID though, an error will be raised. +`--missing-only`:: + When used together with `--missing=print` or `--missing=print-info`, + suppress all output for present objects and print only the missing + ones. The selected `--missing=` format is preserved (so + `--missing=print-info` still emits `path=` / `type=` fields), but the + leading ``?'' prefix used by the non-`-z` forms is omitted. This is + useful for scripting, as a simpler and faster alternative to + post-processing the output of `--missing=print`. ++ +This option is incompatible with `--count` and `--disk-usage`. +It is an error to use `--missing-only` without `--missing=print` or +`--missing=print-info`. + `--exclude-promisor-objects`:: (For internal use only.) Prefilter object traversal at promisor boundary. This is used with partial clone. This is diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 02818b81c63fd7..09c6d272200f16 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -111,6 +111,13 @@ enum missing_action { MA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */ }; static enum missing_action arg_missing_action; +static int arg_missing_only; + +static inline int should_collect_missing(void) +{ + return arg_missing_action == MA_PRINT || + arg_missing_action == MA_PRINT_INFO; +} /* display only the oid of each object encountered */ static int arg_show_object_names = 1; @@ -156,7 +163,14 @@ static void print_missing_object(struct missing_objects_map_entry *entry, { struct strbuf sb = STRBUF_INIT; - if (line_term) + /* + * --missing-only filters present objects out of the walk output. + * It still uses the selected --missing= format for missing ones, + * except the human "?" prefix is omitted (script-friendly OIDs). + */ + if (arg_missing_only && line_term) + printf("%s", oid_to_hex(&entry->entry.oid)); + else if (line_term) printf("?%s", oid_to_hex(&entry->entry.oid)); else printf("%s%cmissing=yes", oid_to_hex(&entry->entry.oid), @@ -246,6 +260,11 @@ static void show_commit(struct commit *commit, void *data) return; } + if (arg_missing_only) { + finish_commit(commit); + return; + } + if (show_disk_usage) total_disk_usage += get_object_disk_usage(&commit->object); @@ -384,6 +403,8 @@ static void show_object(struct object *obj, const char *name, void *cb_data) if (finish_object(obj, name, cb_data)) return; display_progress(progress, ++progress_counter); + if (arg_missing_only) + return; if (show_disk_usage) total_disk_usage += get_object_disk_usage(obj); if (info->flags & REV_LIST_QUIET) @@ -749,12 +770,17 @@ int cmd_rev_list(int argc, revs.exclude_promisor_objects = 1; } else if (skip_prefix(arg, "--missing=", &arg)) { parse_missing_action_value(arg); + } else if (!strcmp(arg, "--missing-only")) { + arg_missing_only = 1; } else if (!strcmp(arg, "-z")) { line_term = '\0'; info_term = '\0'; } } + if (arg_missing_only && !should_collect_missing()) + die(_("--missing-only requires --missing=print or --missing=print-info")); + die_for_incompatible_opt2(revs.exclude_promisor_objects, "--exclude_promisor_objects", arg_missing_action, "--missing"); @@ -864,6 +890,9 @@ int cmd_rev_list(int argc, continue; } + if (!strcmp(arg, "--missing-only")) + continue; + usage(rev_list_usage); } @@ -910,6 +939,11 @@ int cmd_rev_list(int argc, (revs.left_right || revs.cherry_mark)) die(_("marked counting and '%s' cannot be used together"), "--objects"); + die_for_incompatible_opt2(arg_missing_only, "--missing-only", + revs.count, "--count"); + die_for_incompatible_opt2(arg_missing_only, "--missing-only", + show_disk_usage, "--disk-usage"); + save_commit_buffer = (revs.verbose_header || revs.grep_filter.pattern_list || revs.grep_filter.header_list); @@ -967,8 +1001,7 @@ int cmd_rev_list(int argc, if (arg_print_omitted) oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE); - if (arg_missing_action == MA_PRINT || - arg_missing_action == MA_PRINT_INFO) { + if (should_collect_missing()) { struct oidset_iter iter; struct object_id *oid; @@ -994,8 +1027,7 @@ int cmd_rev_list(int argc, printf("~%s\n", oid_to_hex(oid)); oidset_clear(&omitted_objects); } - if (arg_missing_action == MA_PRINT || - arg_missing_action == MA_PRINT_INFO) { + if (should_collect_missing()) { struct missing_objects_map_entry *entry; struct oidmap_iter iter; diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh index 1e472a45afa21f..1bd2c3bc4f8075 100755 --- a/t/t6022-rev-list-missing.sh +++ b/t/t6022-rev-list-missing.sh @@ -198,6 +198,55 @@ do ' done +for obj in "HEAD~1" "HEAD~1^{tree}" "HEAD:1.t" +do + test_expect_success "rev-list --missing-only with missing $obj" ' + oid="$(git rev-parse $obj)" && + path=".git/objects/$(test_oid_to_path $oid)" && + + mv "$path" "$path.hidden" && + test_when_finished "mv $path.hidden $path" && + + git rev-list --missing=print --missing-only --objects \ + --no-object-names HEAD >actual && + + echo $oid >expect && + test_cmp expect actual + ' +done + +test_expect_success "--missing-only requires --missing=print or --missing=print-info" ' + test_must_fail git rev-list --missing-only --objects HEAD 2>err && + test_grep "requires --missing=print" err +' + +test_expect_success "--missing-only is incompatible with --count" ' + test_must_fail git rev-list --missing=print --missing-only \ + --count --objects HEAD 2>err && + test_grep "cannot be used together" err +' + +test_expect_success "--missing-only is incompatible with --disk-usage" ' + test_must_fail git rev-list --missing=print --missing-only \ + --disk-usage --objects HEAD 2>err && + test_grep "cannot be used together" err +' + +test_expect_success "--missing-only works with --missing=print-info" ' + oid="$(git rev-parse HEAD:1.t)" && + path=".git/objects/$(test_oid_to_path $oid)" && + + mv "$path" "$path.hidden" && + test_when_finished "mv $path.hidden $path" && + + git rev-list --missing=print-info --missing-only --objects \ + --no-object-names HEAD >actual && + + # Filter keeps print-info fields; only the "?" prefix is dropped. + echo "$oid path=1.t type=blob" >expect && + test_cmp expect actual +' + test_expect_success "-z nul-delimited --missing" ' test_when_finished rm -rf repo && From 20c0a93007eb3d4b4659a6ae450b7b2377b1c236 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 4 Sep 2026 09:03:05 +0200 Subject: [PATCH 08/10] rerere: extract logic to determine whether entries are stale When garbage collecting rerere entries we need to figure out whether any given entry is stale before pruning it. In a subsequent commit we're about to introduce a second caller that wants to determine staleness, but the logic is not currently reusable. Extract the logic to compute staleness by introducing two new helper functions `rerere_gc_cutoffs()` and `rerere_id_is_stale()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- rerere.c | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/rerere.c b/rerere.c index 3d3bd0db16a737..073422dbf313eb 100644 --- a/rerere.c +++ b/rerere.c @@ -1173,22 +1173,44 @@ static void unlink_rr_item(struct rerere_id *id) strbuf_release(&buf); } -static void prune_one(struct rerere_id *id, - timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve) +static void rerere_gc_cutoffs(struct repository *r, + timestamp_t *cutoff_resolve, + timestamp_t *cutoff_noresolve) +{ + timestamp_t now = time(NULL); + + if (repo_config_get_expiry_in_days(r, "gc.rerereresolved", + cutoff_resolve, now)) + *cutoff_resolve = now - 60 * 86400; + if (repo_config_get_expiry_in_days(r, "gc.rerereunresolved", + cutoff_noresolve, now)) + *cutoff_noresolve = now - 15 * 86400; +} + +static bool rerere_id_is_stale(struct rerere_id *id, + timestamp_t cutoff_resolve, + timestamp_t cutoff_noresolve) { timestamp_t then; timestamp_t cutoff; then = rerere_last_used_at(id); - if (then) + if (then) { cutoff = cutoff_resolve; - else { + } else { then = rerere_created_at(id); if (!then) - return; + return false; cutoff = cutoff_noresolve; } - if (then < cutoff) + + return then < cutoff; +} + +static void prune_one(struct rerere_id *id, + timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve) +{ + if (rerere_id_is_stale(id, cutoff_resolve, cutoff_noresolve)) unlink_rr_item(id); } @@ -1206,18 +1228,14 @@ void rerere_gc(struct repository *r, struct string_list *rr) DIR *dir; struct dirent *e; int i; - timestamp_t now = time(NULL); - timestamp_t cutoff_noresolve = now - 15 * 86400; - timestamp_t cutoff_resolve = now - 60 * 86400; + timestamp_t cutoff_noresolve; + timestamp_t cutoff_resolve; struct strbuf buf = STRBUF_INIT; if (setup_rerere(r, rr, 0) < 0) return; - repo_config_get_expiry_in_days(the_repository, "gc.rerereresolved", - &cutoff_resolve, now); - repo_config_get_expiry_in_days(the_repository, "gc.rerereunresolved", - &cutoff_noresolve, now); + rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve); repo_config(the_repository, git_default_config, NULL); dir = opendir(repo_git_path_replace(the_repository, &buf, "rr-cache")); if (!dir) From e77f412d84a9717745f6a86de0255ad90a597a2d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 4 Sep 2026 09:03:06 +0200 Subject: [PATCH 09/10] builtin/maintenance: improve heuristic for "rerere gc" The "rerere-gc" maintenance task is responsible for pruning rerere entries older than a certain configurable cutoff point. Whether or not the task gets run during auto-maintenance can be configured via "maintenance.rerere-gc.auto": - A negative value indicates that maintenance should always run. - A zero value indicates that maintenance should never run. - Otherwise, a positive value indicates that maintenance should always run in case we have at least a single rerere entry. While the first two conditions are sensible, the last one is less so as it does not account for whether we would even prune old entries in the first place. Instead, it effectively implies that we unconditionally spawn "git rerere gc" when rerere is enabled. Chances are high though that there is nothing to prune, as the default cutoff dates are 60 days for resolved rerere entries and 15 days for unresolved ones. Besides being a waste of compute, it also obstructs concurrent processes that want to write new resolutions as garbage collection takes a central lock file, as reported in [1]. That race is a longstanding one that existed even before we introduced fine-grained maintenance tasks, and the proper fix is to use a locking timeout in the writing processes. But the race is made worse by us performing garbage collection a lot more often. Refine the heuristic to take into account whether any entries can be pruned in the first place. This ensures that we'll only ever run this task in situations where it will do anything, and should thus result in a lot less frequent invocations of "git rerere gc". Furthermore, tweak the meaning of "maintenance.rerere-gc.auto" so that positive values allow the user to configure the number of prunable entries that need to exist before we run it and set the default value to 512. This number is pulled out of thin air, but it ensures that we know to batch-delete entries instead of pruning every single entry that is older than the cutoff point. Note that this now requires us to actually open the rerere-entry directories and stat the individual files in there, which does add a bit of overhead when one has lots of rerere entries. To counteract this overhead, we thus use the same sampling heuristic as we do for loose objects, where we only consider those entries that start with a "17". [1]: Reported-by: Thomas Bachem Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/config/maintenance.adoc | 8 ++-- builtin/gc.c | 28 +++--------- rerere.c | 50 ++++++++++++++++++++++ rerere.h | 6 +++ t/t7900-maintenance.sh | 61 +++++++++++++++++++++------ 5 files changed, 113 insertions(+), 40 deletions(-) diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc index da8be9f812c68d..77977dcc48eee9 100644 --- a/Documentation/config/maintenance.adoc +++ b/Documentation/config/maintenance.adoc @@ -121,10 +121,10 @@ maintenance.rerere-gc.auto:: This integer config option controls how often the `rerere-gc` task should be run as part of `git maintenance run --auto`. If zero, then the `rerere-gc` task will not run with the `--auto` option. A negative - value will force the task to run every time. Otherwise, any positive - value implies the command will run when the "rr-cache" directory exists - and has at least one entry, regardless of whether it is stale or not. - This heuristic may be refined in the future. The default value is 1. + value will force the task to run every time. Otherwise, a positive + value implies the command should run when the estimated number of stale + entries that would be pruned is greater than or equal to the configured + value. The default value is 512. maintenance.worktree-prune.auto:: This integer config option controls how often the `worktree-prune` task diff --git a/builtin/gc.c b/builtin/gc.c index de2f9e7fed5b3e..57a3520263d7be 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -396,31 +396,15 @@ static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED, static int rerere_gc_condition(struct gc_config *cfg UNUSED) { - struct strbuf path = STRBUF_INIT; - int should_gc = 0, limit = 1; - DIR *dir = NULL; + int limit = 512; repo_config_get_int(the_repository, "maintenance.rerere-gc.auto", &limit); - if (limit <= 0) { - should_gc = limit < 0; - goto out; - } - - /* - * We skip garbage collection in case we either have no "rr-cache" - * directory or when it doesn't contain at least one entry. - */ - repo_git_path_replace(the_repository, &path, "rr-cache"); - dir = opendir(path.buf); - if (!dir) - goto out; - should_gc = !!readdir_skip_dot_and_dotdot(dir); + if (!limit) + return 0; /* never prune */ + if (limit < 0) + return 1; /* always prune */ -out: - strbuf_release(&path); - if (dir) - closedir(dir); - return should_gc; + return rerere_gc_needed(the_repository, (size_t)limit); } #define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \ diff --git a/rerere.c b/rerere.c index 073422dbf313eb..1c3745d9e3279a 100644 --- a/rerere.c +++ b/rerere.c @@ -1222,6 +1222,56 @@ static int is_rr_cache_dirname(const char *path) return !parse_oid_hex(path, &oid, &end) && !*end; } +bool rerere_gc_needed(struct repository *r, size_t limit) +{ + timestamp_t cutoff_resolve, cutoff_noresolve; + struct strbuf buf = STRBUF_INIT; + bool needed = false; + struct dirent *e; + size_t count = 0; + DIR *dir; + + dir = opendir(repo_git_path_replace(r, &buf, "rr-cache")); + if (!dir) + goto out; + + rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve); + + while ((e = readdir_skip_dot_and_dotdot(dir))) { + struct rerere_id id; + + /* + * We estimate the number of stale entries by only considering + * those starting with "17". This is the same strategy that we + * use for estimating the number of loose objects. + */ + if (!starts_with(e->d_name, "17") || + !is_rr_cache_dirname(e->d_name)) + continue; + + id.collection = find_rerere_dir(e->d_name); + for (id.variant = 0; + id.variant < id.collection->status_nr; + id.variant++) { + if (rerere_id_is_stale(&id, cutoff_resolve, + cutoff_noresolve)) { + count += 256; + if (count >= limit) { + needed = true; + goto out; + } + } + } + } + +out: + if (dir) + closedir(dir); + free_rerere_dirs(); + strbuf_release(&buf); + return needed; +} + void rerere_gc(struct repository *r, struct string_list *rr) { struct string_list to_remove = STRING_LIST_INIT_DUP; diff --git a/rerere.h b/rerere.h index d4b5f7c932006a..feeb0e2c9fe61b 100644 --- a/rerere.h +++ b/rerere.h @@ -39,6 +39,12 @@ int rerere_remaining(struct repository *, struct string_list *); void rerere_clear(struct repository *, struct string_list *); void rerere_gc(struct repository *, struct string_list *); +/* + * Check whether garbage collection for rerere entries is needed, which is + * the case when there's at least `limit` stale entries that would be pruned. + */ +bool rerere_gc_needed(struct repository *r, size_t limit); + #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \ N_("update the index with reused conflict resolution if possible")) diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 5fbb16f0f0e59c..4f65fa9439c0b8 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -1016,37 +1016,70 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' ' test_expect_rerere_gc git maintenance run --task=rerere-gc ' -test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' ' +test_expect_success 'rerere-gc task with --auto only prunes with stale entries' ' test_when_finished "rm -rf .git/rr-cache" && + entry_1=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) && + entry_2=.git/rr-cache/172$(echo $ZERO_OID | cut -c4-) && + entry_3=.git/rr-cache/173$(echo $ZERO_OID | cut -c4-) && + + # Without the "rr-cache" directory there is nothing to prune. ! git maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc && - mkdir .git/rr-cache && + + # Fresh unresolved entries are not stale. + for e in $entry_1 $entry_2 $entry_3 + do + mkdir -p $e && + echo preimage >$e/preimage || return 1 + done && ! git maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc && - : >.git/rr-cache/entry && + + # Entries are sampled using the "17" prefix, so we scale up the + # estimate by 256. A single entry is not sufficient to reach the + # default limit of 512. + test-tool chmtime =-$((16 * 86400)) $entry_1/preimage && + ! git maintenance is-needed --auto --task=rerere-gc && + + # A second prunable entry will reach the limit though and will thus get + # pruned. + test-tool chmtime =-$((16 * 86400)) $entry_2/preimage && git maintenance is-needed --auto --task=rerere-gc && - test_expect_rerere_gc git maintenance run --auto --task=rerere-gc + + # The prunable entries are gone, the other one remains. + test_expect_rerere_gc git maintenance run --auto --task=rerere-gc && + test_path_is_missing $entry_1 && + test_path_is_missing $entry_2 && + test_path_is_dir $entry_3 ' test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.auto' ' test_when_finished "rm -rf .git/rr-cache" && + entry=.git/rr-cache/171$(echo $ZERO_OID | cut -c4-) && # A negative value should always prune. git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc && - # A positive value prunes when there is at least one entry. - ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc && - test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc && - mkdir .git/rr-cache && - ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc && - test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc && - : >.git/rr-cache/entry-1 && - git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc && - test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc && + # A positive value prunes only when the estimated number of stale + # entries is at least as big. A single sampled entry counts for 256 + # estimated entries. + mkdir -p $entry && + echo preimage >$entry/preimage && + test-tool chmtime =-$((16 * 86400)) $entry/preimage && + + ! git -c maintenance.rerere-gc.auto=257 maintenance is-needed --auto --task=rerere-gc && + test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=257 maintenance run --auto --task=rerere-gc && + test_path_is_dir $entry && + + git -c maintenance.rerere-gc.auto=256 maintenance is-needed --auto --task=rerere-gc && + test_expect_rerere_gc git -c maintenance.rerere-gc.auto=256 maintenance run --auto --task=rerere-gc && + test_path_is_missing $entry && # Zero should never prune. - : >.git/rr-cache/entry-1 && + mkdir -p $entry && + echo preimage >$entry/preimage && + test-tool chmtime =-$((16 * 86400)) $entry/preimage && ! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc ' From 339ab2a8f14c0c304ae2f28df1a859f3d2cf610c Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 14 Sep 2026 14:02:39 -0700 Subject: [PATCH 10/10] 3rd batch for -rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.56.0.adoc | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Documentation/RelNotes/2.56.0.adoc b/Documentation/RelNotes/2.56.0.adoc index 9c816018fe83d0..214ab1e87b0371 100644 --- a/Documentation/RelNotes/2.56.0.adoc +++ b/Documentation/RelNotes/2.56.0.adoc @@ -153,6 +153,16 @@ UI, Workflows & Features 'CHERRY_PICK_HEAD' ref. A test has also been added to ensure this behavior holds even when the operation stops for conflicts. + * The 'git imap-send' command has been taught to take the '--draft' + option to mark uploaded messages as drafts, which helps some email + clients render them properly for editing and sending. + + * The git rev-list command has been augmented with a '--missing-only' + option that filters the output to only show missing objects, + stripping the leading '?' character and suppressing present objects, + which is useful when used in combination with '--missing=print' or + '--missing=print-info'. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -528,6 +538,14 @@ Performance, Internal Implementation, Development Support etc. the cumulative time spent downloading external packs and the number of advertised URIs without emitting a separate event per pack. + * CGI helper scripts used by HTTP-related test scripts have been updated + to use atomic filesystem operations, preventing race conditions when + Apache handles concurrent requests. + + * "git maintenance" triggered "rerere gc" in unappropriate times and + interfered with "git rebase" etc. too much. The conditions "rerere + gc" gets triggered have been tweaked. + Fixes since v2.55 ----------------- @@ -859,6 +877,12 @@ Fixes since v2.55 expression syntax that breaks on older Perl versions. (merge 8a631963a9 ta/lint-gitlink-older-perl-fix later to maint). + * The autostash fallback in 'git checkout -m' has been refined to only + retry when there are local changes. Additionally, a blank line now + visually separates autostash conflict advice from the subsequent + branch-switch message. + (merge 2ba77ea828 hn/checkout-m-autostash-refine later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 026636128f ss/submittingpatches-typofix later to maint). (merge d2af22cc21 jc/rerere-doc-typofix later to maint).