* [PATCH v3 2/3] http: avoid concurrent appends to partial packs
From: Ted Nyman @ 2026-07-21 23:29 UTC (permalink / raw)
To: git; +Cc: gitster, me, peff, ps, karthik.188, sandals, avarab
In-Reply-To: <cover.1784676106.git.tnyman@openai.com>
Pack requests stage downloads in a predictable partial-pack file so an
interrupted transfer can be resumed. Both packfile URI and ordinary dumb
HTTP requests use this staging path. Opening it in append mode forces
each write to the current end of the file, so concurrent responses can
append duplicate data and corrupt the pack.
Open the partial pack read-write without O_APPEND and seek once to its
current end. Each downloader then retains the offset matching the Range
it requested. Because the staging key must uniquely identify immutable
pack contents, overlapping responses write the same bytes at the same
offsets instead of extending the file with duplicate data.
MinGW's non-append O_RDWR open grants FILE_SHARE_DELETE only for an
existing file. Create a missing partial pack exclusively, close it, and
reopen it without O_CREAT so every retained descriptor permits another
downloader to unlink the staging path. Duplicate that descriptor for
index-pack instead of reopening the path after closing the stream;
index-pack installs its own pack and the shared staging file is only
unlinked, never renamed. Accept HTTP 416 when a partial pack is already
complete and let index-pack validate its contents.
Exercise resumed transfers, EOF ranges, overlapping 200 and 206
responses, and unlinking the staging path while index-pack still holds
its descriptor. Clarify the staging-key documentation.
Signed-off-by: Ted Nyman <tnyman@openai.com>
---
Documentation/git-http-fetch.adoc | 5 +-
http-fetch.c | 3 +-
http-push.c | 3 +-
http-walker.c | 3 +-
http.c | 56 ++++---
t/t5550-http-fetch-dumb.sh | 244 ++++++++++++++++++++++++++++++
6 files changed, 289 insertions(+), 25 deletions(-)
diff --git a/Documentation/git-http-fetch.adoc b/Documentation/git-http-fetch.adoc
index 09b5d675ee..60ca91cf3a 100644
--- a/Documentation/git-http-fetch.adoc
+++ b/Documentation/git-http-fetch.adoc
@@ -48,8 +48,9 @@ commit-id::
line (which is not expected in
this case), 'git http-fetch' fetches the packfile directly at the given
URL and uses index-pack to generate corresponding .idx and .keep files.
- The hash is used to determine the name of the temporary file and is
- arbitrary. The output of index-pack is printed to stdout. Requires
+ The hash is used to determine the name of the temporary file. It need
+ not be the pack hash, but it must uniquely identify the pack contents
+ for resumption. The output of index-pack is printed to stdout. Requires
one or more --index-pack-arg options.
--index-pack-arg=<arg>::
diff --git a/http-fetch.c b/http-fetch.c
index 601a77c3c1..05f68f306a 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -70,7 +70,8 @@ static void fetch_single_packfile(struct object_id *packfile_hash,
if (start_active_slot(preq->slot)) {
run_active_slot(preq->slot);
- if (results.curl_result != CURLE_OK) {
+ if (results.curl_result != CURLE_OK &&
+ results.http_code != 416) {
struct url_info url;
char *nurl = url_normalize(preq->url, &url);
if (!nurl || !git_env_bool("GIT_TRACE_REDACT", 1)) {
diff --git a/http-push.c b/http-push.c
index 60f6f8f054..ef8abe3908 100644
--- a/http-push.c
+++ b/http-push.c
@@ -595,7 +595,8 @@ static void finish_request(struct transfer_request *request)
} else if (request->state == RUN_FETCH_PACKED) {
int fail = 1;
- if (request->curl_result != CURLE_OK) {
+ if (request->curl_result != CURLE_OK &&
+ request->http_code != 416) {
fprintf(stderr, "Unable to get pack file %s\n%s",
request->url, curl_errorstr);
} else {
diff --git a/http-walker.c b/http-walker.c
index b58a3b2a92..abafca84d6 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -451,7 +451,8 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo,
if (start_active_slot(preq->slot)) {
run_active_slot(preq->slot);
- if (results.curl_result != CURLE_OK) {
+ if (results.curl_result != CURLE_OK &&
+ results.http_code != 416) {
error("Unable to get pack file %s\n%s", preq->url,
curl_errorstr);
goto abort;
diff --git a/http.c b/http.c
index caccf2108e..a0d399b274 100644
--- a/http.c
+++ b/http.c
@@ -2688,10 +2688,13 @@ int finish_http_pack_request(struct http_pack_request *preq)
int tmpfile_fd;
int ret = 0;
+ /* Another downloader may unlink the staging path while we index it. */
+ tmpfile_fd = xdup(fileno(preq->packfile));
fclose(preq->packfile);
preq->packfile = NULL;
-
- tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY);
+ if (lseek(tmpfile_fd, 0, SEEK_SET) < 0)
+ die_errno("unable to seek local file %s for pack",
+ preq->tmpfile.buf);
ip.git_cmd = 1;
ip.in = tmpfile_fd;
@@ -2704,13 +2707,8 @@ int finish_http_pack_request(struct http_pack_request *preq)
else
ip.no_stdout = 1;
- if (run_command(&ip)) {
+ if (run_command(&ip))
ret = -1;
- goto cleanup;
- }
-
-cleanup:
- close(tmpfile_fd);
unlink(preq->tmpfile.buf);
return ret;
}
@@ -2738,22 +2736,45 @@ struct http_pack_request *new_http_pack_request(
struct http_pack_request *new_direct_http_pack_request(
const unsigned char *packed_git_hash, char *url)
{
- off_t prev_posn = 0;
+ off_t prev_posn;
struct http_pack_request *preq;
+ int fd;
CALLOC_ARRAY(preq, 1);
strbuf_init(&preq->tmpfile, 0);
-
preq->url = url;
odb_pack_name(the_repository, &preq->tmpfile, packed_git_hash, "pack");
strbuf_addstr(&preq->tmpfile, ".temp");
- preq->packfile = fopen(preq->tmpfile.buf, "a");
- if (!preq->packfile) {
- error("Unable to open local file %s for pack",
- preq->tmpfile.buf);
+ /*
+ * MinGW's non-append O_RDWR open grants FILE_SHARE_DELETE only for an
+ * existing file; reopen a newly created file so others may unlink it.
+ */
+ for (;;) {
+ fd = open(preq->tmpfile.buf, O_RDWR);
+ if (fd >= 0 || errno != ENOENT)
+ break;
+ fd = open(preq->tmpfile.buf, O_RDWR | O_CREAT | O_EXCL, 0666);
+ if (fd >= 0) {
+ close(fd);
+ continue;
+ }
+ if (errno != EEXIST)
+ break;
+ }
+ if (fd < 0) {
+ error_errno("unable to open local file %s for pack",
+ preq->tmpfile.buf);
goto abort;
}
+ prev_posn = lseek(fd, 0, SEEK_END);
+ if (prev_posn < 0) {
+ error_errno("unable to seek local file %s for pack",
+ preq->tmpfile.buf);
+ close(fd);
+ goto abort;
+ }
+ preq->packfile = xfdopen(fd, "w");
preq->slot = get_active_slot();
preq->headers = object_request_headers();
@@ -2762,12 +2783,7 @@ struct http_pack_request *new_direct_http_pack_request(
curl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url);
curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, preq->headers);
- /*
- * If there is data present from a previous transfer attempt,
- * resume where it left off
- */
- prev_posn = ftello(preq->packfile);
- if (prev_posn>0) {
+ if (prev_posn > 0) {
if (http_is_verbose)
fprintf(stderr,
"Resuming fetch of pack %s at byte %"PRIuMAX"\n",
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index f00eeae48f..65b42c4719 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -293,6 +293,250 @@ test_expect_success 'http-fetch --packfile' '
git -C packfileclient cat-file -e "$HASH"
'
+test_expect_success 'http-fetch --packfile resumes a partial download' '
+ git init packfileclient-resume &&
+ p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git &&
+ ls objects/pack/pack-*.pack) &&
+ tmpfile="packfileclient-resume/.git/objects/pack/pack-$ARBITRARY.pack.temp" &&
+ test_copy_bytes 64 <"$HTTPD_DOCUMENT_ROOT_PATH/repo_pack.git/$p" >"$tmpfile" &&
+ GIT_TRACE_CURL="$TRASH_DIRECTORY/resume.trace" \
+ git -C packfileclient-resume http-fetch --packfile="$ARBITRARY" \
+ --index-pack-arg=index-pack --index-pack-arg=--stdin \
+ --index-pack-arg=--keep \
+ "$HTTPD_URL/dumb/repo_pack.git/$p" >out &&
+ test_grep "Range: bytes=64-" resume.trace &&
+ test_path_is_missing "$tmpfile" &&
+ git -C packfileclient-resume cat-file -e "$HASH"
+'
+
+test_expect_success 'http-fetch --packfile permits unlink while indexing' '
+ git init packfileclient-unlink &&
+ p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git &&
+ ls objects/pack/pack-*.pack) &&
+ tmpfile="packfileclient-unlink/.git/objects/pack/pack-$ARBITRARY.pack.temp" &&
+ write_script git-unlink-index-pack <<-\EOF &&
+ test -f "$GIT_TEST_PACK_TEMP" || exit 1
+ rm "$GIT_TEST_PACK_TEMP" || exit 1
+ exec git index-pack "$@"
+ EOF
+ test_when_finished "rm -f git-unlink-index-pack" &&
+ PATH="$TRASH_DIRECTORY:$PATH" \
+ GIT_TEST_PACK_TEMP="$TRASH_DIRECTORY/$tmpfile" \
+ git -C packfileclient-unlink http-fetch --packfile="$ARBITRARY" \
+ --index-pack-arg=unlink-index-pack \
+ --index-pack-arg=--stdin --index-pack-arg=--keep \
+ "$HTTPD_URL/dumb/repo_pack.git/$p" >out &&
+ test_path_is_missing "$tmpfile" &&
+ git -C packfileclient-unlink cat-file -e "$HASH"
+'
+
+test_expect_success PIPE 'concurrent http-fetch --packfile accepts a complete partial' '
+ git init packfileclient-concurrent &&
+ p=$(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git &&
+ ls objects/pack/pack-*.pack) &&
+ packhash=$(basename "$p" .pack) &&
+ packhash=${packhash#pack-} &&
+ tmpfile="packfileclient-concurrent/.git/objects/pack/pack-$packhash.pack.temp" &&
+ test_copy_bytes 64 <"$HTTPD_DOCUMENT_ROOT_PATH/repo_pack.git/$p" >"$tmpfile" &&
+ mkfifo first-ready first-continue &&
+ exec 8<>first-ready &&
+ exec 9<>first-continue &&
+ write_script git-wait-index-pack <<-\EOF &&
+ echo ready >"$GIT_TEST_WAIT_READY" &&
+ read continue <"$GIT_TEST_WAIT_CONTINUE" &&
+ exec git index-pack "$@"
+ EOF
+ {
+ (
+ if ! PATH="$TRASH_DIRECTORY:$PATH" \
+ GIT_TEST_WAIT_READY="$TRASH_DIRECTORY/first-ready" \
+ GIT_TEST_WAIT_CONTINUE="$TRASH_DIRECTORY/first-continue" \
+ GIT_TRACE_CURL="$TRASH_DIRECTORY/first.trace" \
+ git -C packfileclient-concurrent http-fetch --packfile="$packhash" \
+ --index-pack-arg=wait-index-pack \
+ --index-pack-arg=--stdin --index-pack-arg=--keep \
+ "$HTTPD_URL/dumb/repo_pack.git/$p" >first.out
+ then
+ echo failed >"$TRASH_DIRECTORY/first-ready" &&
+ exit 1
+ fi
+ ) &
+ first_pid=$!
+ } &&
+ test_when_finished "
+ echo continue >&9
+ kill $first_pid 2>/dev/null || :
+ wait $first_pid 2>/dev/null || :
+ exec 8>&-
+ exec 9>&-
+ rm -f first-ready first-continue git-wait-index-pack
+ " &&
+ read ready <&8 &&
+ test "$ready" = ready &&
+ GIT_TRACE_CURL="$TRASH_DIRECTORY/second.trace" \
+ git -C packfileclient-concurrent http-fetch --packfile="$packhash" \
+ --index-pack-arg=index-pack \
+ --index-pack-arg=--stdin --index-pack-arg=--keep \
+ "$HTTPD_URL/dumb/repo_pack.git/$p" >second.out &&
+ echo continue >&9 &&
+ wait "$first_pid" &&
+ printf "pack\t%s\n" "$packhash" >expect &&
+ test_cmp expect first.out &&
+ printf "keep\t%s\n" "$packhash" >expect &&
+ test_cmp expect second.out &&
+ test_grep "Range: bytes=64-" first.trace &&
+ test_grep "Range: bytes=[0-9]*-" second.trace &&
+ test_grep "HTTP/[0-9.]* 416" second.trace &&
+ test_path_is_missing "$tmpfile" &&
+ git -C packfileclient-concurrent cat-file -e "$HASH"
+'
+
+test_expect_success PERL,PIPE 'concurrent http-fetch --packfile cannot corrupt an overlapping download' '
+ git init packfileclient-overlap &&
+ blob=$(test-tool genrandom pack-overlap 2m |
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git \
+ hash-object -w --stdin) &&
+ packhash=$(printf "%s\n" "$blob" |
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git \
+ pack-objects "$TRASH_DIRECTORY/overlap-pack") &&
+ pack="$TRASH_DIRECTORY/overlap-pack-$packhash.pack" &&
+ tmpfile="packfileclient-overlap/.git/objects/pack/pack-$packhash.pack.temp" &&
+ mkfifo server-ready first-ready &&
+ exec 7<>server-ready &&
+ exec 8<>first-ready &&
+ write_script slow-pack-server "$PERL_PATH" <<-\EOF &&
+ use strict;
+ use warnings;
+ use IO::Socket::INET;
+
+ my ($packfile, $server_ready, $first_ready) = @ARGV;
+ open(my $in, "<:raw", $packfile) or die "open $packfile: $!";
+ my $pack = do { local $/; <$in> };
+ close($in) or die "close $packfile: $!";
+ my $server = IO::Socket::INET->new(LocalAddr => "127.0.0.1",
+ LocalPort => 0, Proto => "tcp", Listen => 2, ReuseAddr => 1)
+ or die "listen: $!";
+
+ sub signal_ready {
+ my ($file, $value) = @_;
+ open(my $out, ">", $file) or die "open $file: $!";
+ print $out "$value\n" or die "write $file: $!";
+ close($out) or die "close $file: $!";
+ }
+
+ sub write_all {
+ my ($out, $data) = @_;
+ my $offset = 0;
+ while ($offset < length($data)) {
+ my $written = syswrite($out, $data,
+ length($data) - $offset, $offset);
+ defined($written) && $written or die "write response: $!";
+ $offset += $written;
+ }
+ }
+
+ sub start_response {
+ my $out = $server->accept() or die "accept: $!";
+ <$out> or die "read request: $!";
+ my $start = 0;
+ while (<$out>) {
+ last if /^\r?\n$/;
+ $start = $1 if /^Range: bytes=(\d+)-/i;
+ }
+ $start < length($pack) or die "invalid range $start";
+ my $length = length($pack) - $start;
+ my $middle = int($length / 2);
+ my $status = $start ? "206 Partial Content" : "200 OK";
+ my $headers = "HTTP/1.1 $status\r\n" .
+ "Content-Length: $length\r\n" .
+ ($start ? "Content-Range: bytes $start-" .
+ (length($pack) - 1) . "/" . length($pack) . "\r\n" : "") .
+ "Connection: close\r\n\r\n";
+ write_all($out, $headers);
+ write_all($out, substr($pack, $start, $middle));
+ return ($out, $start + $middle);
+ }
+
+ signal_ready($server_ready, $server->sockport());
+ my ($first, $first_pos) = start_response();
+ signal_ready($first_ready, "ready");
+ my ($second, $second_pos) = start_response();
+ write_all($first, substr($pack, $first_pos));
+ write_all($second, substr($pack, $second_pos));
+ close($first) or die "close first response: $!";
+ close($second) or die "close second response: $!";
+ EOF
+ {
+ (
+ if ! "$TRASH_DIRECTORY/slow-pack-server" "$pack" \
+ "$TRASH_DIRECTORY/server-ready" \
+ "$TRASH_DIRECTORY/first-ready"
+ then
+ echo failed >"$TRASH_DIRECTORY/server-ready" &&
+ echo failed >"$TRASH_DIRECTORY/first-ready" &&
+ exit 1
+ fi
+ ) >server.log 2>&1 &
+ server_pid=$!
+ } &&
+ test_when_finished "
+ kill $server_pid 2>/dev/null || :
+ wait $server_pid 2>/dev/null || :
+ exec 7>&-
+ exec 8>&-
+ rm -f server-ready first-ready slow-pack-server
+ " &&
+ read port <&7 &&
+ url="http://127.0.0.1:$port/pack" &&
+ {
+ (
+ if ! GIT_TRACE_CURL="$TRASH_DIRECTORY/overlap-first.trace" \
+ GIT_TRACE_CURL_NO_DATA=1 \
+ git -C packfileclient-overlap http-fetch --packfile="$packhash" \
+ --index-pack-arg=index-pack \
+ --index-pack-arg=--stdin --index-pack-arg=--keep \
+ "$url" >first.out
+ then
+ echo failed >"$TRASH_DIRECTORY/first-ready" &&
+ exit 1
+ fi
+ ) &
+ first_pid=$!
+ } &&
+ test_when_finished "
+ kill $first_pid 2>/dev/null || :
+ wait $first_pid 2>/dev/null || :
+ " &&
+ read ready <&8 &&
+ test "$ready" = ready &&
+ test_path_is_file "$tmpfile" &&
+ test -s "$tmpfile" &&
+ {
+ GIT_TRACE_CURL="$TRASH_DIRECTORY/overlap-second.trace" \
+ GIT_TRACE_CURL_NO_DATA=1 \
+ git -C packfileclient-overlap http-fetch --packfile="$packhash" \
+ --index-pack-arg=index-pack \
+ --index-pack-arg=--stdin --index-pack-arg=--keep \
+ "$url" >second.out &
+ second_pid=$!
+ } &&
+ test_when_finished "
+ kill $second_pid 2>/dev/null || :
+ wait $second_pid 2>/dev/null || :
+ " &&
+ wait "$server_pid" &&
+ wait "$first_pid" &&
+ wait "$second_pid" &&
+ test_grep "HTTP/[0-9.]* 200" overlap-first.trace &&
+ test_grep "Range: bytes=[1-9][0-9]*-" overlap-second.trace &&
+ test_grep "HTTP/[0-9.]* 206" overlap-second.trace &&
+ printf "keep\t%s\npack\t%s\n" "$packhash" "$packhash" | sort >expect &&
+ sort first.out second.out >actual &&
+ test_cmp expect actual &&
+ test_path_is_missing "$tmpfile" &&
+ git -C packfileclient-overlap cat-file -e "$blob"
+'
+
test_expect_success 'fetch notices corrupt pack' '
cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
(cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad1.git &&
--
2.55.0.openai.131.g83a728de1eb6
^ permalink raw reply related
* [PATCH v3 3/3] fetch-pack: accept "pack" output for packfile URIs
From: Ted Nyman @ 2026-07-21 23:29 UTC (permalink / raw)
To: git; +Cc: gitster, me, peff, ps, karthik.188, sandals, avarab
In-Reply-To: <cover.1784676106.git.tnyman@openai.com>
When index-pack finds an existing keep file it reports pack rather than
keep. Accept either result from http-fetch, and only register a keep
lockfile when this fetch created it.
Read the pack/keep prefix and hash without consuming any following fsck
output, validate the reported pack hash against the advertised hash, and
exercise a packfile URI fetch with a pre-existing keep file.
Signed-off-by: Ted Nyman <tnyman@openai.com>
---
fetch-pack.c | 33 ++++++++++++++++++---------------
t/t5702-protocol-v2.sh | 31 +++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 15 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 29c41132ee..e9f24fbd63 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1887,9 +1887,10 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
}
for (i = 0; i < packfile_uris.nr; i++) {
+ bool created_keep;
int j;
struct child_process cmd = CHILD_PROCESS_INIT;
- char packname[GIT_MAX_HEXSZ + 1];
+ char packhash[GIT_MAX_HEXSZ + 1];
const char *uri = packfile_uris.items[i].string +
the_hash_algo->hexsz + 1;
@@ -1907,16 +1908,17 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
if (start_command(&cmd))
die("fetch-pack: unable to spawn http-fetch");
- if (read_in_full(cmd.out, packname, 5) < 0 ||
- memcmp(packname, "keep\t", 5))
- die("fetch-pack: expected keep then TAB at start of http-fetch output");
+ if (read_in_full(cmd.out, packhash, 5) != 5 ||
+ (memcmp(packhash, "keep\t", 5) &&
+ memcmp(packhash, "pack\t", 5)))
+ die("fetch-pack: expected pack or keep then TAB at start of http-fetch output");
+ created_keep = !memcmp(packhash, "keep\t", 5);
- if (read_in_full(cmd.out, packname,
- the_hash_algo->hexsz + 1) < 0 ||
- packname[the_hash_algo->hexsz] != '\n')
- die("fetch-pack: expected hash then LF at end of http-fetch output");
-
- packname[the_hash_algo->hexsz] = '\0';
+ if (read_in_full(cmd.out, packhash,
+ the_hash_algo->hexsz + 1) != the_hash_algo->hexsz + 1 ||
+ packhash[the_hash_algo->hexsz] != '\n')
+ die("fetch-pack: expected hash then LF in http-fetch output");
+ packhash[the_hash_algo->hexsz] = '\0';
parse_gitmodules_oids(cmd.out, &fsck_options.gitmodules_found);
@@ -1925,16 +1927,17 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
if (finish_command(&cmd))
die("fetch-pack: unable to finish http-fetch");
- if (memcmp(packfile_uris.items[i].string, packname,
+ if (memcmp(packfile_uris.items[i].string, packhash,
the_hash_algo->hexsz))
die("fetch-pack: pack downloaded from %s does not match expected hash %.*s",
uri, (int) the_hash_algo->hexsz,
packfile_uris.items[i].string);
- string_list_append_nodup(pack_lockfiles,
- xstrfmt("%s/pack/pack-%s.keep",
- repo_get_object_directory(the_repository),
- packname));
+ if (created_keep)
+ string_list_append_nodup(pack_lockfiles,
+ xstrfmt("%s/pack/pack-%s.keep",
+ repo_get_object_directory(the_repository),
+ packhash));
}
string_list_clear(&packfile_uris, 0);
strvec_clear(&index_pack_args);
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 74a2b7730b..0f05286de8 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -1291,6 +1291,37 @@ test_expect_success 'packfile URIs with fetch instead of clone' '
fetch "$HTTPD_URL/smart/http_parent"
'
+test_expect_success 'packfile URI preserves an existing keep file' '
+ P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ rm -rf "$P" http_child keep.expect &&
+
+ git init "$P" &&
+ git -C "$P" config uploadpack.allowsidebandall true &&
+
+ echo my-blob >"$P/my-blob" &&
+ git -C "$P" add my-blob &&
+ git -C "$P" commit -m x &&
+ configure_exclusion "$P" my-blob >h &&
+
+ git init http_child &&
+ packhash=$(cat packh) &&
+ keep="http_child/.git/objects/pack/pack-$packhash.keep" &&
+ echo pre-existing >"$keep" &&
+ cp "$keep" keep.expect &&
+
+ GIT_TEST_SIDEBAND_ALL=1 \
+ git -C http_child -c protocol.version=2 \
+ -c fetch.uriprotocols=http,https \
+ fetch "$HTTPD_URL/smart/http_parent" &&
+
+ test_path_is_file \
+ "http_child/.git/objects/pack/pack-$packhash.pack" &&
+ test_path_is_file \
+ "http_child/.git/objects/pack/pack-$packhash.idx" &&
+ test_cmp keep.expect "$keep" &&
+ git -C http_child cat-file -e "$(cat h)"
+'
+
test_expect_success 'fetching with valid packfile URI but invalid hash fails' '
P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
rm -rf "$P" http_child log &&
--
2.55.0.openai.131.g83a728de1eb6
^ permalink raw reply related
* [PATCH] regexec: work around macOS TRE memory leak on invalid UTF-8
From: Chungmin Lee @ 2026-07-22 5:31 UTC (permalink / raw)
To: git; +Cc: Chungmin Lee
On macOS the system regex engine (TRE) leaks the buffer it allocates for
a match (in tre_tnfa_run_parallel()) whenever regexec() encounters an
invalid multibyte sequence in a UTF-8 locale: it returns REG_ILLSEQ
without freeing that buffer. Because regexec_buf() is called once per
line, grepping a file that mixes text with binary data (for example a PDF
checked into a repository) leaks a buffer per line. The leaked buffer is
sized in proportion to the line length and the pattern's automaton, so a
large case-insensitive alternation over long binary lines can leak
gigabytes in a single command; this has been observed to exhaust memory
and trigger a kernel watchdog panic on the machine running "git grep".
A line that contains an invalid multibyte sequence can still contain
real matches: when a match lies on either side of the invalid bytes, TRE
finds it, returns REG_OK, and frees the buffer normally. So the fix
cannot simply reject or truncate such a line -- that would silently drop
matches the platform engine itself would report. Nor can it search the
whole line first and fall back to segmentation only on REG_ILLSEQ: by the
time regexec() returns REG_ILLSEQ it has already leaked, so the fallback
cannot prevent it.
Add a Darwin-only regexec_buf() (compat/regexec.c) that never hands the
platform matcher an invalid byte. It walks the buffer, splits it at each
invalid sequence, and searches each maximal run of valid text between
them, returning the first match. Each segment is searched with
REG_STARTEND (whose offsets are relative to the buffer, so no translation
is needed) and with REG_NOTBOL / REG_NOTEOL suppressed only when the
segment does not reach the true start or end of the buffer, so "^" and
"$" keep matching exactly where they would on the whole line. The split
points are chosen with mbrtowc(), so the buffer is cut at exactly the
bytes the platform's own decoder -- and therefore TRE, which decodes the
same way -- rejects.
The split runs only where a byte can be invalid. In a single-byte
locale (MB_CUR_MAX == 1) nothing is invalid, so the whole buffer is
searched as before and behavior is unchanged. The leak is not specific
to UTF-8 -- any multibyte locale drives TRE through the same REG_ILLSEQ
path -- so the guard keys on MB_CUR_MAX rather than the codeset name,
which is also simpler. MB_CUR_MAX reflects the LC_CTYPE the C library
installed, the same locale mbrtowc() decodes against.
regexec_buf() backs every regex caller, so this affects "git grep",
diffcore-pickaxe (-G/-S --pickaxe-regex) and diff word splitting alike.
The fix does not reproduce the platform's exact output on lines with
invalid bytes -- doing so would mean deliberately hiding matches that are
really there. No match the platform engine reported before is lost, and
every match reported now is a genuine one: each segment is searched as
valid text at its real offset, so nothing spurious is introduced.
Unanchored matches -- which the engine already found, even next to
invalid bytes -- are unchanged. The only additional matches are
zero-width ones at a true line boundary next to invalid bytes: a "^",
"$", or word boundary that holds there now matches, where the engine used
to hit the invalid byte, return REG_ILLSEQ, and report nothing even
though the match is genuine.
The workaround is compiled in only when the platform's own regex engine
is used, across the Makefile, Meson and CMake builds: it is skipped
whenever the bundled engine (which has no REG_ILLSEQ and does not leak)
is selected by NO_REGEX -- as it is under AddressSanitizer in the
Makefile and Meson builds. compat/regexec.c guards its body on the same
macro, so a build that does not enable the workaround compiles it to
nothing rather than colliding with the inline regexec_buf(). The
workaround avoids the leaking path rather than depending on it, so it
stays correct if the platform is fixed and can be dropped once a macOS
without the leak is the minimum supported baseline.
A regression test cannot bound a process's memory portably -- setrlimit
RLIMIT_AS is not enforced on macOS -- so t7810 checks the observable
behavior instead: valid text matches on both sides of an invalid byte
and in a run between two invalid sequences, and "^" and "$" do not match
across an invalid byte in mid-line. A further check, guarded by the
MACOS prerequisite because only macOS runs this code, confirms that "^"
and "$" match at the true start and end of such a line. (A line that is
entirely invalid bytes still does not match an unanchored empty-width
pattern such as "^", as before.)
Signed-off-by: Chungmin Lee <chungmin@chungminlee.com>
---
This came out of a real incident: "git grep -i" over a repository that
contains PDFs exhausted memory on an otherwise idle Mac mini and took the
machine down with a kernel watchdog panic ("no checkins from watchdogd").
The leak is in the system regex engine, not in git, but git is what
drives it into the leaking path, once per line.
Why this belongs in regexec_buf():
- regexec_buf() already exists as the place to wrap platform regexec()
quirks. It was added in v2.11.0 (backported to v2.10.1) for exactly
this kind of reason -- "a regexec_buf() helper that takes a <ptr,len>
pair with REG_STARTEND extension" (RelNotes/2.11.0).
- git already treats REG_ILLSEQ on invalid UTF-8 as a cross-platform
reality -- "As FreeBSD is not the only platform whose regexp library
reports a REG_ILLSEQ error when fed invalid UTF-8..."
(RelNotes/2.28.0). This patch keeps returning the matches around the
invalid bytes, and just stops leaking on the way there.
On the behavior change: the fix does not reproduce the platform engine's
exact output on a line with invalid bytes, because that output is "leak,
then report nothing". It never drops a match the engine reported before
and never invents a spurious one; the only additional matches are
zero-width assertions at a true line boundary next to the invalid bytes
(the log describes the exact envelope).
Reproducing (macOS, UTF-8 locale):
perl -e 'print "\377" x 16, "\n" for 1..300000' >binfile
git init -q r && mv binfile r && git -C r add binfile &&
git -C r commit -qm x
P='aa|bb|cc|dd|ee|ff|gg|hh|ii|jj|kk|ll|mm|nn|oo|pp|qq|rr|ss|tt'
/usr/bin/time -l git -C r grep -i -E "$P" >/dev/null
Max RSS on this machine (grep is multithreaded, so the stock figure
varies run to run):
stock git 2.50.1 (Apple): ~1-2 GiB
patched: ~13 MiB
The gap grows without bound with the number of invalid-byte lines and
the size of the pattern's automaton; the original incident is believed
to have reached tens of GiB before the watchdog panic.
I also have a standalone, self-verifying reproducer that measures the
heap directly (malloc_zone_statistics(), no external tools); it reports
~3072 bytes leaked per REG_ILLSEQ call, allocated in
tre_tnfa_run_parallel() (confirmed with MallocStackLogging + leaks(1)).
I can post it, and I have prepared it to file with Apple as well.
Environment:
macOS 26.5 (build 25F71), Darwin 25.5.0 arm64, Apple M4
Apple clang 21.0.0; /usr/bin/git 2.50.1 (Apple Git-155)
Tested: t7810 passes (268/268), including the invalid-byte cases added
here. (t7812 exits clean but its cases are all skipped on this machine
for lack of a suitable GETTEXT_LOCALE; it exercises the system-regex path
on platforms that have one.) Built with the platform regex engine on
macOS; NO_REGEX builds, and Makefile/Meson ASAN builds, use the bundled
engine and skip the workaround.
Makefile | 4 ++
compat/regexec.c | 108 ++++++++++++++++++++++++++++
config.mak.uname | 1 +
contrib/buildsystems/CMakeLists.txt | 5 ++
git-compat-util.h | 5 ++
meson.build | 7 ++
t/t7810-grep.sh | 28 ++++++++
7 files changed, 158 insertions(+)
create mode 100644 compat/regexec.c
diff --git a/Makefile b/Makefile
index 1cec251f4..b568dde52 100644
--- a/Makefile
+++ b/Makefile
@@ -2264,6 +2264,10 @@ ifdef USE_ENHANCED_BASIC_REGULAR_EXPRESSIONS
COMPAT_CFLAGS += -DUSE_ENHANCED_BASIC_REGULAR_EXPRESSIONS
COMPAT_OBJS += compat/regcomp_enhanced.o
endif
+ifdef DARWIN_TRE_REGEXEC_LEAK_WORKAROUND
+ COMPAT_OBJS += compat/regexec.o
+ BASIC_CFLAGS += -DREGEXEC_MAY_LEAK_ON_ILLSEQ
+endif
endif
ifdef NATIVE_CRLF
BASIC_CFLAGS += -DNATIVE_CRLF
diff --git a/compat/regexec.c b/compat/regexec.c
new file mode 100644
index 000000000..0677162a8
--- /dev/null
+++ b/compat/regexec.c
@@ -0,0 +1,108 @@
+#include "git-compat-util.h"
+
+#ifdef REGEXEC_MAY_LEAK_ON_ILLSEQ
+
+#include <wchar.h>
+
+/*
+ * macOS's libc regex engine (TRE) leaks the buffer it allocates for a
+ * match whenever regexec() encounters an invalid multibyte sequence in
+ * a multibyte locale: it returns REG_ILLSEQ without freeing that buffer.
+ * A single "git grep" over a file with binary data can call regexec()
+ * once per line and leak gigabytes, which has been observed to exhaust
+ * memory and trigger a kernel watchdog panic.
+ *
+ * The leak happens inside regexec() before it returns, so reacting to
+ * REG_ILLSEQ cannot avoid it: the invalid bytes must never reach the
+ * matcher. Split the buffer at each invalid sequence and search the
+ * surrounding runs of valid text separately. A match on either side of
+ * the invalid bytes is still found (the same result the matcher gives on
+ * valid input), but the leaking REG_ILLSEQ path is never reached.
+ *
+ * Use mbrtowc() to decide where to split, so that we split at exactly the
+ * bytes the platform's own decoder -- and thus the regex engine, which
+ * decodes the same way -- rejects. A hand-rolled validator would
+ * have to guess that boundary; being too lenient reintroduces the leak.
+ */
+
+/*
+ * Search buf[start, end) for a match. REG_STARTEND reports offsets
+ * relative to buf, so a hit needs no translation. ^ may only match at
+ * the real start of the buffer and $ only at its real end, so suppress
+ * them when this segment does not reach those boundaries.
+ */
+static int regexec_segment(const regex_t *preg, const char *buf,
+ size_t start, size_t end, size_t size,
+ size_t nmatch, regmatch_t pmatch[], int eflags)
+{
+ eflags |= REG_STARTEND;
+ if (start > 0)
+ eflags |= REG_NOTBOL;
+ if (end < size)
+ eflags |= REG_NOTEOL;
+ pmatch[0].rm_so = start;
+ pmatch[0].rm_eo = end;
+ return regexec(preg, buf, nmatch, pmatch, eflags);
+}
+
+int regexec_buf(const regex_t *preg, const char *buf, size_t size,
+ size_t nmatch, regmatch_t pmatch[], int eflags)
+{
+ size_t seg_start = 0, i = 0;
+ mbstate_t mbs;
+
+ assert(nmatch > 0 && pmatch);
+
+ /*
+ * Only a multibyte locale drives TRE through the leaking multibyte
+ * path. In a single-byte locale (MB_CUR_MAX == 1) no byte is
+ * invalid, so search the whole buffer as before. MB_CUR_MAX
+ * reflects the current LC_CTYPE, the same locale mbrtowc() below
+ * decodes against.
+ */
+ if (MB_CUR_MAX == 1) {
+ pmatch[0].rm_so = 0;
+ pmatch[0].rm_eo = size;
+ return regexec(preg, buf, nmatch, pmatch, eflags | REG_STARTEND);
+ }
+
+ memset(&mbs, 0, sizeof(mbs));
+ while (i < size) {
+ unsigned char c = (unsigned char)buf[i];
+ size_t n;
+
+ if (c < 0x80) { /* ASCII fast path */
+ i++;
+ continue;
+ }
+
+ n = mbrtowc(NULL, buf + i, size - i, &mbs);
+ if (!n) /* embedded NUL decodes to one byte */
+ n = 1;
+ if (n != (size_t)-1 && n != (size_t)-2) {
+ i += n;
+ continue;
+ }
+
+ /* buf[i] begins an invalid sequence; search the run before it */
+ if (i > seg_start) {
+ int ret = regexec_segment(preg, buf, seg_start, i, size,
+ nmatch, pmatch, eflags);
+ if (ret != REG_NOMATCH)
+ return ret;
+ }
+ i++; /* skip the invalid byte and resync */
+ seg_start = i;
+ memset(&mbs, 0, sizeof(mbs));
+ }
+
+ /*
+ * Search the final run. Do this even when it is empty (a line that
+ * ends in invalid bytes, or an empty buffer) so that "$" and
+ * empty-matching patterns still match at the true end of the buffer.
+ */
+ return regexec_segment(preg, buf, seg_start, size, size,
+ nmatch, pmatch, eflags);
+}
+
+#endif /* REGEXEC_MAY_LEAK_ON_ILLSEQ */
diff --git a/config.mak.uname b/config.mak.uname
index 9ebd24037..2402a2449 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -154,6 +154,7 @@ ifeq ($(uname_S),Darwin)
HAVE_DEV_TTY = YesPlease
COMPAT_OBJS += compat/precompose_utf8.o
BASIC_CFLAGS += -DPRECOMPOSE_UNICODE
+ DARWIN_TRE_REGEXEC_LEAK_WORKAROUND = YesPlease
BASIC_CFLAGS += -DPROTECT_HFS_DEFAULT=1
HAVE_BSD_SYSCTL = YesPlease
FREAD_READS_DIRECTORIES = UnfortunatelyYes
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index a57c4b464..5cfb48f78 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -519,6 +519,11 @@ if(NOT HAVE_REGEX)
include_directories(${CMAKE_SOURCE_DIR}/compat/regex)
list(APPEND compat_SOURCES compat/regex/regex.c )
add_compile_definitions(NO_REGEX NO_MBSUPPORT GAWK)
+elseif(APPLE)
+ # macOS's system regex engine (TRE) leaks memory when regexec()
+ # hits invalid UTF-8; work around it via compat/regexec.c.
+ list(APPEND compat_SOURCES compat/regexec.c)
+ add_compile_definitions(REGEXEC_MAY_LEAK_ON_ILLSEQ)
endif()
diff --git a/git-compat-util.h b/git-compat-util.h
index 880977640..3861c9353 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -992,6 +992,10 @@ static inline int strtol_i(char const *s, int base, int *result)
#error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
#endif
+#ifdef REGEXEC_MAY_LEAK_ON_ILLSEQ
+int regexec_buf(const regex_t *preg, const char *buf, size_t size,
+ size_t nmatch, regmatch_t pmatch[], int eflags);
+#else
static inline int regexec_buf(const regex_t *preg, const char *buf, size_t size,
size_t nmatch, regmatch_t pmatch[], int eflags)
{
@@ -1000,6 +1004,7 @@ static inline int regexec_buf(const regex_t *preg, const char *buf, size_t size,
pmatch[0].rm_eo = size;
return regexec(preg, buf, nmatch, pmatch, eflags | REG_STARTEND);
}
+#endif
#ifdef USE_ENHANCED_BASIC_REGULAR_EXPRESSIONS
int git_regcomp(regex_t *preg, const char *pattern, int cflags);
diff --git a/meson.build b/meson.build
index 3247697f7..2ce37b607 100644
--- a/meson.build
+++ b/meson.build
@@ -1387,6 +1387,13 @@ if not get_option('b_sanitize').contains('address') and get_option('regex').allo
libgit_c_args += '-DUSE_ENHANCED_BASIC_REGULAR_EXPRESSIONS'
compat_sources += 'compat/regcomp_enhanced.c'
endif
+
+ # macOS's system regex engine (TRE) leaks memory when regexec()
+ # hits invalid UTF-8; work around it via compat/regexec.c.
+ if host_machine.system() == 'darwin'
+ libgit_c_args += '-DREGEXEC_MAY_LEAK_ON_ILLSEQ'
+ compat_sources += 'compat/regexec.c'
+ endif
elif not get_option('regex').enabled()
libgit_c_args += [
'-DNO_REGEX',
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index d61c4a4d7..b325a2e11 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -89,6 +89,8 @@ test_expect_success setup '
function dummy() {}
EOF
printf "\200\nASCII\n" >invalid-utf8 &&
+ printf "before\346world\n" >invalid-utf8-embedded &&
+ printf "a\346b\347c\n" >invalid-utf8-multi &&
if test_have_prereq FUNNYNAMES
then
echo unusual >"\"unusual\" pathname" &&
@@ -595,6 +597,32 @@ test_expect_success MB_REGEX 'grep two chars in single-char multibyte file' '
LC_ALL=en_US.UTF-8 test_expect_code 1 git grep ".." reverse-question-mark
'
+test_expect_success MB_REGEX 'grep matches valid text on both sides of invalid UTF-8' '
+ LC_ALL=en_US.UTF-8 git grep -h before invalid-utf8-embedded >actual &&
+ test_cmp invalid-utf8-embedded actual &&
+ LC_ALL=en_US.UTF-8 git grep -h world invalid-utf8-embedded >actual &&
+ test_cmp invalid-utf8-embedded actual
+'
+
+test_expect_success MB_REGEX 'grep matches a run between two invalid sequences' '
+ LC_ALL=en_US.UTF-8 git grep -h b invalid-utf8-multi >actual &&
+ test_cmp invalid-utf8-multi actual
+'
+
+test_expect_success MB_REGEX 'grep does not anchor ^ or $ inside an invalid-byte line' '
+ test_expect_code 1 env LC_ALL=en_US.UTF-8 \
+ git grep -h "^world" invalid-utf8-embedded &&
+ test_expect_code 1 env LC_ALL=en_US.UTF-8 \
+ git grep -h "before\$" invalid-utf8-embedded
+'
+
+test_expect_success MACOS,MB_REGEX 'grep anchors ^ and $ at true line ends past invalid UTF-8' '
+ LC_ALL=en_US.UTF-8 git grep -h "^before" invalid-utf8-embedded >actual &&
+ test_cmp invalid-utf8-embedded actual &&
+ LC_ALL=en_US.UTF-8 git grep -h "world\$" invalid-utf8-embedded >actual &&
+ test_cmp invalid-utf8-embedded actual
+'
+
cat >expected <<EOF
file
EOF
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
--
2.55.0
^ permalink raw reply related
* Re: [PATCH] t0213: skip ancestry tests under user-mode emulation
From: Weijie Yuan @ 2026-07-22 6:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jamie Magee via GitGitGadget, git, Jamie Magee
In-Reply-To: <xmqqwluot3bz.fsf@gitster.g>
On Tue, Jul 21, 2026 at 02:55:44PM -0700, Junio C Hamano wrote:
> Weijie Yuan <wy@wyuan.org> writes:
>
> >> ...
> >> Cc: Matthew John Cheetham <mjcheetham@outlook.com>
> >> Signed-off-by: Jamie Magee <jamie.magee@gmail.com>
> >> ---
> >
> > Very sorry to say something completely outside the patch.
> >
> > But may I ask what's the point of writting the line started with "Cc:"?
> > ...
> > I know that Linux kernel has something about writting Cc in the commit
> > message, while I don't see much from Git's documentation about trailers,
> > including MyFirstContribution and SubmittingPatches.
>
> If you ask me, 'Cc:' belongs in e-mail headers, not in commit
> messages, though the Linux kernel community has a different
> convention.
>
> GitGitGadget collects 'Cc:' lines from the commit message and, when
> sending e-mails on behalf of the author, copies the recipients
> listed there, if I am not mistaken. Thus, it is not surprising that
> contributors use the trailer for that purpose.
Yeah, I noticed that this patch was sent bt GGG. But since he has
already added "Cc" at the end of the commit message, yet in the actual
email header, there is no "Matthew John Cheetham", which was confusing?
This is something I forgot to mention in the previous email, sorry.
I may have to take a closer look at GGG later.
> We do not use the 'Cc:' trailer to allow a commit author to say, "As
> the commit object indicates, I CC'd this change to that expert. I
> am no longer solely responsible for any bugs in this commit. That
> expert should have caught my mistake!" ;-)
;-) I agree!
Thanks!
^ permalink raw reply
* [PATCH v20 0/7] branch: delete-merged
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren
In-Reply-To: <pull.2285.v19.git.git.1784053493.gitgitgadget@gmail.com>
Delete branches that have already been merged on upstream.
Changes in v20:
* Protect branches transitively required by a surviving local upstream
stack. Traverse upstream chains once and defer delete-set mutation until
traversal completes.
* Make stacked-branch handling independent of ref iteration order and
update the documentation accordingly.
* Clarify variable names with regards to branch names (short) to reduce
confusion.
Changes in v19:
* Fix bug where dry-run would still remove config, added test coverage.
* Redesigned --delete-merged as a repeatable upstream selector with
optional positional patterns limiting deletion scope.
* Protect same-name upstream branches independently of push-default
configuration.
* Simplified flags handling where local caching became complicated when
mutating values.
* Clarified assertions in tests.
Changes in v18:
* Instead of keeping the whole chain of upstream branches, keep only the
ones an unmerged branch still needs. When a kept (merged) branch in turn
tracks a branch that is being deleted, clear its now-stale upstream
config.
* Rework spare_stacked_bases() to record the kept bases and, in a second
pass, clear the upstream of any whose own base is going away. Build the
to-delete list with strset_for_each_entry() instead of re-walking the
candidate array.
Changes in v17:
* Keep a merged branch when another surviving branch still tracks it as its
upstream, so --delete-merged no longer deletes a branch out from under
one stacked on top of it.
* Move the --dry-run and branch.<name>.deleteMerged opt-out fully into
their own commits.
Changes in v16:
* Convert delete_merged_branches() to take an unsigned int flags argument
instead of separate quiet/dry_run booleans, matching delete_branches()
* Reuse the strbuf across the skip-config loop (strbuf_reset per iteration,
single strbuf_release after) instead of allocating and freeing it each
time
* Rewrite the --delete-merged tests as integration tests: branches that
land commits upstream, with deletion and the checked-out, upstream-gone,
and push-equals-upstream safety cases exercised together in one run and
output asserted via test_cmp
* Collapse the many per-aspect test repos into a single reused repo set up
by a setup_repo_for_delete_merged helper, and rename helpers off the old
pm_/prune naming
* Nest single-repo setup sequences in ( cd ... ) subshells instead of
prefixing every command with -C
Changes in v15:
* Renamed --prune-merged to --delete-merged throughout. Not necessarily
final, but something to advance the discussion.
* --delete-merged now silently skips not-yet-merged branches instead of
warning.
* Initialized the delete_branches() flag locals where declared. Only force
stays deferred.
* delete_branches()/check_branch_commit() doc and code cleanups: redundant
branch NULL checks dropped, ref_array candidates = { 0 }, a BUG() for the
unreachable non-branch ref, and reworked --delete-merged doc wording.
* Broadened the --forked tests (local commits for realism, remote add -f,
--forked coverage), renamed the misleading trunk fixture, and replaced
the misnamed detached branch with git checkout --detach.
Changes in v14:
* Fixed a git branch -d -r regression (broke t5404/t5505/t5514): the
remotes path set a local force but not the DELETE_BRANCH_FORCE bit that
check_branch_commit() reads, so it wrongly ran the merge check.
* Made flags the single source of truth in delete_branches() so the bit and
the derived locals can't disagree.
* Works locally, but GitHub CI has problems that are there for other
branches too, hopefully not related
(https://github.com/git/git/pull/2285).
Changes in v13:
* Reworked --forked into a real ref-filter applied in apply_ref_filter()
instead of a post-pass, so non-matching branches are never allocated.
* Match exact --forked patterns on full refnames (only globs use the
abbreviated upstream), and dropped the old helper machinery, forward
declaration, and string_list in favor of a strvec.
* Replaced the boolean parameters of
delete_branches()/check_branch_commit() with a single unsigned int flags.
* --prune-merged now collects candidates via filter_refs() rather than its
own branch walk.
* --prune-merged now takes its patterns as positional arguments (e.g. git
branch --prune-merged origin/main 'feature*') instead of repeating the
option.
Changes in v12:
* Reworked --forked from a standalone action into a --list-mode filter.
* Switched --forked and --prune-merged to repeatable OPT_STRING_LIST
options.
* Dropped the bare-remote-name resolution for --forked, the argument is now
a ref or a glob.
Changes in v11:
* The flags now take a branch, not a remote. --forked and --prune-merged
accept a literal upstream short name like origin/main or a wildmatch
pattern like origin/. The old --all-remotes flag is gone, since origin/
covers that case.
* The prune guard now compares @{push} against @{upstream}. A branch is
spared when these are equal. That is the trunk like case, such as local
main tracking and pushing to origin/main, where "fully merged to
upstream" cannot be told apart from "just pulled". Only branches that
push somewhere other than their upstream, typically fork based topics,
are candidates. The earlier /HEAD by name guard that the reviewer
rejected is gone.
* New --dry-run for --prune-merged.
Changes in v10:
* --forked / --prune-merged now take a branch glob instead of a remote name
— origin, origin/*, origin/release-- all work. This replaces the
remote-only form and subsumes the old --all-remotes flag, which has been
dropped.
* New --dry-run for --prune-merged.
Changes in v9:
* --force no longer has special meaning with --prune-merged; reachability
is always enforced. Use git branch -D to delete an unmerged branch.
Matches how git branch's other read/safe actions treat --force.
* Synopsis drops [-f]; "not fully merged" hint points at git branch -D.
* Dropped the --prune-merged --force tests.
Changes in v8:
* Delete only when the branch's work is actually reachable from its
upstream
* Skip branches whose upstream is gone (even with --force)
* Simplified the internal safety flag to live in one place
Changes in v7:
* --prune-merged now checks if a branch is merged into its own upstream
first. If the upstream is gone, it checks against the remote's default
branch instead. If neither exists, the branch is refused (use --force to
delete anyway).
Changes in v6:
* --prune-merged now measures merged-ness against the remote's default
branch instead of the candidate's upstream — so the decision no longer
depends on which branch happens to be checked out locally.
* delete_branches() / check_branch_commit() gained a per-candidate override
that lets a caller substitute a different "what counts as merged"
reference (or skip the check). branch -d callers pass NULL and keep their
existing semantics.
* prune_merged_branches() resolves each candidate's push-remote HEAD and
threads it through, so --prune-merged --all-remotes measures each
candidate against its own remote rather than a single global reference.
Changes in v5:
* Drop commit 'fetch: add --prune-merged'
Changes in v4:
* Resolve each remote's HEAD and collect the targets into a
protected_default_refs set in collect_forked_set.
* In prune_merged_branches, skip a candidate when its upstream is a
protected default ref and the local branch name matches the default
branch's leaf name (so a local main tracking origin/main is spared, but a
renamed trunk tracking origin/main is not).
* Also skip when the candidate's push ref points at a protected default
ref, so a topic branch configured to push to origin/main is never pruned.
* Tests: spare the local default branch; only protect by matching leaf name
(not by upstream alone); spare a branch whose push ref is the remote
default.
Changes in v3:
* s/remote-tracking refs/remote-tracking branches/g
Changes in v2:
* The whole feature moved out of git fetch and into git branch. git fetch
--prune-merged now just calls git branch --prune-merged after fetching.
* The fetch.pruneLocalBranches and remote..pruneLocalBranches config
options are gone, replaced by per-branch opt-out via branch..pruneMerged.
* New git branch --forked lists local branches whose upstream lives on the
given remote (read-only building block).
* New git branch --prune-merged deletes those branches, but only if their
tip is reachable from the upstream tracking ref; --force skips that
safety check.
* New git branch --all-remotes lets --forked/--prune-merged operate across
every configured remote at once.
* The currently checked-out branch in any worktree is always preserved.
* branch..pruneMerged=false lets you exempt a branch (e.g. a long-running
topic branch) even with --force; doesn't affect explicit git branch -d.
* delete_branches() got a warn_only mode so bulk deletion prints a one-line
warning per skipped branch instead of the noisy four-line hint that git
branch -d shows.
* New section in git-branch docs; git-fetch docs trimmed to just mention
--prune-merged.
* New tests in t3200-branch.sh for the new branch flags; t5510-fetch.sh
shrunk since most logic moved.
Harald Nordgren (7):
branch: add --forked filter for --list mode
branch: convert delete_branches() to a flags argument
branch: let delete_branches skip unmerged branches on bulk refusal
branch: prepare delete_branches for a bulk caller
branch: add --delete-merged <branch>
branch: add branch.<name>.deleteMerged opt-out
branch: add --dry-run for --delete-merged
Documentation/config/branch.adoc | 7 +
Documentation/git-branch.adoc | 49 +++-
builtin/branch.c | 276 +++++++++++++++++++---
ref-filter.c | 70 ++++++
ref-filter.h | 10 +
t/t3200-branch.sh | 393 +++++++++++++++++++++++++++++++
6 files changed, 774 insertions(+), 31 deletions(-)
base-commit: 5d2e7709234afea1b6ddb25cd4f60d3d5fb3c200
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2285%2FHaraldNordgren%2Ffetch-prune-local-branches-v20
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2285/HaraldNordgren/fetch-prune-local-branches-v20
Pull-Request: https://github.com/git/git/pull/2285
Range-diff vs v19:
1: 562648132d = 1: 51c1c9d075 branch: add --forked filter for --list mode
2: c7ebd9344c = 2: 711574b2e5 branch: convert delete_branches() to a flags argument
3: 0c4f3358e3 = 3: 47c5975dc7 branch: let delete_branches skip unmerged branches on bulk refusal
4: 64a202526a = 4: 46268acec5 branch: prepare delete_branches for a bulk caller
5: a6caa5b397 ! 5: ef9f57e735 branch: add --delete-merged <branch>
@@ Commit message
A branch whose work is not yet merged into its upstream is silently
skipped, so one unmerged topic does not abort the whole sweep.
- A branch that another, surviving branch tracks as its upstream is
- also kept, so a branch is never deleted out from under one stacked
- on top of it. Such a kept branch is itself merged, so when its own
- upstream is being deleted, clear its now-stale upstream config.
+ A branch that a surviving branch depends on through a chain of local
+ upstreams is also kept, so no branch is deleted out from under stacked
+ work. Collect this transitive set without changing the candidate set
+ during ref iteration: walk upstream chains from surviving branches,
+ visit each branch at most once, and remove the collected bases only
+ after the iteration completes. This makes the result independent of
+ ref iteration order without repeated full scans.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ Documentation/git-branch.adoc: This option is only applicable in non-verbose mod
+silently skipped. Delete it with `git branch -D` if you want to
+remove it anyway.
++
-+A branch that another, surviving branch tracks as its upstream is
-+kept, so a branch is never deleted out from under one stacked on top
-+of it. If that kept branch in turn tracks a branch that is being
-+deleted, its now-stale upstream configuration is cleared.
++A branch that a surviving branch depends on through a chain of local
++upstreams is kept, so a branch is never deleted out from under stacked
++work.
+
`-v`::
`-vv`::
@@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
return 0;
}
-+struct spare_data {
-+ struct strset *deletable;
-+ struct strset *spared;
++struct stacked_branch_data {
++ struct strset *deletable_branch_names;
++ struct strset *protected_branch_names;
++ struct strset *visited_branch_names;
+};
+
-+/*
-+ * A surviving branch stacked on a deletion candidate would lose its
-+ * upstream, so drop that candidate from the delete set and remember it
-+ * in "spared" so its own upstream can be tidied up afterwards.
-+ */
-+static int spare_stacked_base(const struct reference *ref, void *cb_data)
++static int collect_stacked_branch_bases(const struct reference *ref,
++ void *cb_data)
+{
-+ struct spare_data *data = cb_data;
-+ struct branch *branch;
-+ const char *upstream, *up_short;
++ struct stacked_branch_data *data = cb_data;
++ const char *branch_name;
+
-+ if (strset_contains(data->deletable, ref->name))
-+ return 0;
-+ branch = branch_get(ref->name);
-+ upstream = branch_get_upstream(branch, NULL);
-+ if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
-+ !strset_contains(data->deletable, up_short))
++ if (!skip_prefix(ref->name, "refs/heads/", &branch_name))
++ BUG("expected local branch ref, got '%s'", ref->name);
++ if (strset_contains(data->deletable_branch_names, branch_name))
+ return 0;
+
-+ strset_remove(data->deletable, up_short);
-+ strset_add(data->spared, up_short);
++ while (strset_add(data->visited_branch_names, branch_name)) {
++ struct branch *branch = branch_get(branch_name);
++ const char *upstream_refname = branch_get_upstream(branch, NULL);
++ const char *upstream_branch_name;
++
++ if (!upstream_refname ||
++ !skip_prefix(upstream_refname, "refs/heads/",
++ &upstream_branch_name) ||
++ !strset_contains(data->deletable_branch_names,
++ upstream_branch_name))
++ break;
++
++ strset_add(data->protected_branch_names, upstream_branch_name);
++ branch_name = upstream_branch_name;
++ }
++
+ return 0;
+}
+
-+/*
-+ * Keep any branch that a surviving branch tracks as its upstream, so we
-+ * never delete a branch out from under one stacked on top of it. Such a
-+ * base is itself merged, so when its own upstream is also going away
-+ * (no surviving branch tracks it), clear the base's now-stale upstream.
-+ */
-+static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable)
++static void protect_stacked_branch_bases(struct ref_store *refs,
++ struct strset *deletable_branch_names)
+{
-+ struct strset spared = STRSET_INIT;
-+ struct spare_data data = { .deletable = deletable, .spared = &spared };
-+ struct strbuf key = STRBUF_INIT;
++ struct strset protected_branch_names = STRSET_INIT;
++ struct strset visited_branch_names = STRSET_INIT;
++ struct stacked_branch_data data = {
++ .deletable_branch_names = deletable_branch_names,
++ .protected_branch_names = &protected_branch_names,
++ .visited_branch_names = &visited_branch_names,
++ };
++ struct refs_for_each_ref_options opts = {
++ .prefix = "refs/heads/",
++ };
+ struct hashmap_iter iter;
+ struct strmap_entry *entry;
+
-+ refs_for_each_branch_ref(refs, spare_stacked_base, &data);
-+
-+ strset_for_each_entry(&spared, &iter, entry) {
-+ struct branch *branch = branch_get(entry->key);
-+ const char *upstream = branch_get_upstream(branch, NULL);
-+ const char *up_short;
-+
-+ if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
-+ !strset_contains(deletable, up_short))
-+ continue;
++ refs_for_each_ref_ext(refs, collect_stacked_branch_bases, &data, &opts);
+
-+ strbuf_reset(&key);
-+ strbuf_addf(&key, "branch.%s.merge", branch->name);
-+ repo_config_set_gently(the_repository, key.buf, NULL);
-+ strbuf_reset(&key);
-+ strbuf_addf(&key, "branch.%s.remote", branch->name);
-+ repo_config_set_gently(the_repository, key.buf, NULL);
-+ }
++ strset_for_each_entry(&protected_branch_names, &iter, entry)
++ strset_remove(deletable_branch_names, entry->key);
+
-+ strbuf_release(&key);
-+ strset_clear(&spared);
++ strset_clear(&visited_branch_names);
++ strset_clear(&protected_branch_names);
+}
+
+static int branch_pushes_to_upstream(struct branch *branch,
@@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ struct ref_filter filter = REF_FILTER_INIT;
+ struct ref_array candidates = { 0 };
-+ struct strset deletable = STRSET_INIT;
-+ struct strvec to_delete = STRVEC_INIT;
++ struct strset deletable_branch_names = STRSET_INIT;
++ struct strvec branches_to_delete = STRVEC_INIT;
+ struct hashmap_iter iter;
+ struct strmap_entry *entry;
+ size_t i;
@@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
+ filter_refs(&candidates, &filter, filter.kind);
+
+ for (i = 0; i < (size_t)candidates.nr; i++) {
-+ const char *full_name = candidates.items[i]->refname;
-+ const char *short_name;
++ const char *branch_refname = candidates.items[i]->refname;
++ const char *branch_name;
+ struct branch *branch;
-+ const char *upstream;
++ const char *upstream_refname;
+
-+ if (!skip_prefix(full_name, "refs/heads/", &short_name))
-+ BUG("filter returned non-branch ref '%s'", full_name);
-+ if (branch_checked_out(full_name))
++ if (!skip_prefix(branch_refname, "refs/heads/", &branch_name))
++ BUG("filter returned non-branch ref '%s'", branch_refname);
++ if (branch_checked_out(branch_refname))
+ continue;
+
-+ branch = branch_get(short_name);
-+ upstream = branch_get_upstream(branch, NULL);
-+ if (!upstream || !refs_ref_exists(refs, upstream))
++ branch = branch_get(branch_name);
++ upstream_refname = branch_get_upstream(branch, NULL);
++ if (!upstream_refname || !refs_ref_exists(refs, upstream_refname))
+ continue;
-+ if (branch_pushes_to_upstream(branch, upstream))
++ if (branch_pushes_to_upstream(branch, upstream_refname))
+ continue;
-+ if (check_branch_commit(short_name, short_name,
++ if (check_branch_commit(branch_name, branch_name,
+ &candidates.items[i]->objectname, NULL,
+ FILTER_REFS_BRANCHES, DELETE_BRANCH_SKIP_UNMERGED))
+ continue;
+
-+ strset_add(&deletable, short_name);
++ strset_add(&deletable_branch_names, branch_name);
+ }
+
-+ spare_stacked_bases(refs, &deletable);
++ protect_stacked_branch_bases(refs, &deletable_branch_names);
+
-+ strset_for_each_entry(&deletable, &iter, entry)
-+ strvec_push(&to_delete, entry->key);
++ strset_for_each_entry(&deletable_branch_names, &iter, entry)
++ strvec_push(&branches_to_delete, entry->key);
+
-+ if (to_delete.nr)
-+ ret = delete_branches(to_delete.nr, to_delete.v,
++ if (branches_to_delete.nr)
++ ret = delete_branches(branches_to_delete.nr, branches_to_delete.v,
+ FILTER_REFS_BRANCHES,
+ DELETE_BRANCH_SKIP_UNMERGED |
+ DELETE_BRANCH_NO_HEAD_FALLBACK |
+ flags);
+
-+ strvec_clear(&to_delete);
-+ strset_clear(&deletable);
++ strvec_clear(&branches_to_delete);
++ strset_clear(&deletable_branch_names);
+ ref_array_clear(&candidates);
+ ref_filter_clear(&filter);
+ return ret;
@@ t/t3200-branch.sh: test_expect_success '--forked requires a value' '
+ )
+'
+
-+test_expect_success '--delete-merged clears the deleted upstream of a spared branch' '
++test_expect_success '--delete-merged keeps the upstream chain of a surviving branch' '
+ setup_repo_for_delete_merged &&
+ (
+ cd repo &&
@@ t/t3200-branch.sh: test_expect_success '--forked requires a value' '
+ git commit --allow-empty -m "tip work" &&
+
+ git branch --delete-merged origin/next \
-+ --delete-merged lower &&
++ --delete-merged lower >actual 2>&1 &&
++ test_must_be_empty actual &&
+
+ check_branches <<-\EOF &&
++ lower
+ main
+ mid
+ tip
+ EOF
+
-+ git config --local --get-regexp "branch\\.(mid|tip)\\.(merge|remote)" >actual &&
++ git config --local --get-regexp "branch\\.(lower|mid|tip)\\.(merge|remote)" >actual &&
+ cat >expect <<-\EOF &&
++ branch.lower.remote origin
++ branch.lower.merge refs/heads/next
++ branch.mid.remote .
++ branch.mid.merge refs/heads/lower
+ branch.tip.remote .
+ branch.tip.merge refs/heads/mid
+ EOF
@@ t/t3200-branch.sh: test_expect_success '--forked requires a value' '
+ )
+'
+
++test_expect_success '--delete-merged result is independent of stacked branch names' '
++ setup_repo_for_delete_merged &&
++ (
++ cd repo &&
++ git branch c-lower origin/next --track &&
++ git branch b-mid c-lower --track &&
++ git checkout -b a-tip b-mid --track &&
++ git commit --allow-empty -m "tip work" &&
++
++ git branch --delete-merged origin/next \
++ --delete-merged "c-*" &&
++
++ check_branches <<-\EOF &&
++ a-tip
++ b-mid
++ c-lower
++ main
++ EOF
++
++ git branch --delete-merged origin/next \
++ --delete-merged "c-*" >actual 2>&1 &&
++ test_must_be_empty actual &&
++
++ check_branches <<-\EOF
++ a-tip
++ b-mid
++ c-lower
++ main
++ EOF
++ )
++'
++
+test_expect_success '--delete-merged requires a value' '
+ test_must_fail git -C forked branch --delete-merged 2>err &&
+ test_grep "requires a value" err
6: 734d27c908 ! 6: fa70108611 branch: add branch.<name>.deleteMerged opt-out
@@ Documentation/git-branch.adoc: A branch is not deleted when:
## builtin/branch.c ##
@@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
struct ref_array candidates = { 0 };
- struct strset deletable = STRSET_INIT;
- struct strvec to_delete = STRVEC_INIT;
+ struct strset deletable_branch_names = STRSET_INIT;
+ struct strvec branches_to_delete = STRVEC_INIT;
+ struct strbuf key = STRBUF_INIT;
struct hashmap_iter iter;
struct strmap_entry *entry;
size_t i;
@@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
- const char *short_name;
+ const char *branch_name;
struct branch *branch;
- const char *upstream;
+ const char *upstream_refname;
+ int opt_out;
- if (!skip_prefix(full_name, "refs/heads/", &short_name))
- BUG("filter returned non-branch ref '%s'", full_name);
+ if (!skip_prefix(branch_refname, "refs/heads/", &branch_name))
+ BUG("filter returned non-branch ref '%s'", branch_refname);
@@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
FILTER_REFS_BRANCHES, DELETE_BRANCH_SKIP_UNMERGED))
continue;
+ strbuf_reset(&key);
-+ strbuf_addf(&key, "branch.%s.deletemerged", short_name);
++ strbuf_addf(&key, "branch.%s.deletemerged", branch_name);
+ if (!repo_config_get_bool(the_repository, key.buf, &opt_out) &&
+ !opt_out) {
+ if (!(flags & DELETE_BRANCH_QUIET))
+ fprintf(stderr,
+ _("Skipping '%s' (branch.%s.deleteMerged is false)\n"),
-+ short_name, short_name);
++ branch_name, branch_name);
+ continue;
+ }
+
- strset_add(&deletable, short_name);
+ strset_add(&deletable_branch_names, branch_name);
}
@@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
@@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstrea
flags);
+ strbuf_release(&key);
- strvec_clear(&to_delete);
- strset_clear(&deletable);
+ strvec_clear(&branches_to_delete);
+ strset_clear(&deletable_branch_names);
ref_array_clear(&candidates);
## t/t3200-branch.sh ##
7: 7aa9d5db14 ! 7: 13bac431a3 branch: add --dry-run for --delete-merged
@@ Documentation/git-branch.adoc: git branch (-m|-M) [<old-branch>] <new-branch>
DESCRIPTION
-----------
-@@ Documentation/git-branch.adoc: kept, so a branch is never deleted out from under one stacked on top
- of it. If that kept branch in turn tracks a branch that is being
- deleted, its now-stale upstream configuration is cleared.
+@@ Documentation/git-branch.adoc: A branch that a surviving branch depends on through a chain of local
+ upstreams is kept, so a branch is never deleted out from under stacked
+ work.
+`--dry-run`::
+ With `--delete-merged`, print which branches would be
@@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int ki
char *refname = name + branch_name_pos;
if (!(flags & DELETE_BRANCH_QUIET))
printf(remote_branch
-@@ builtin/branch.c: static int spare_stacked_base(const struct reference *ref, void *cb_data)
- * base is itself merged, so when its own upstream is also going away
- * (no surviving branch tracks it), clear the base's now-stale upstream.
- */
--static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable)
-+static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable,
-+ unsigned int flags)
- {
- struct strset spared = STRSET_INIT;
- struct spare_data data = { .deletable = deletable, .spared = &spared };
-@@ builtin/branch.c: static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable
-
- refs_for_each_branch_ref(refs, spare_stacked_base, &data);
-
-- strset_for_each_entry(&spared, &iter, entry) {
-- struct branch *branch = branch_get(entry->key);
-- const char *upstream = branch_get_upstream(branch, NULL);
-- const char *up_short;
-+ if (!(flags & DELETE_BRANCH_DRY_RUN)) {
-+ strset_for_each_entry(&spared, &iter, entry) {
-+ struct branch *branch = branch_get(entry->key);
-+ const char *upstream = branch_get_upstream(branch, NULL);
-+ const char *up_short;
-
-- if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
-- !strset_contains(deletable, up_short))
-- continue;
-+ if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
-+ !strset_contains(deletable, up_short))
-+ continue;
-
-- strbuf_reset(&key);
-- strbuf_addf(&key, "branch.%s.merge", branch->name);
-- repo_config_set_gently(the_repository, key.buf, NULL);
-- strbuf_reset(&key);
-- strbuf_addf(&key, "branch.%s.remote", branch->name);
-- repo_config_set_gently(the_repository, key.buf, NULL);
-+ strbuf_reset(&key);
-+ strbuf_addf(&key, "branch.%s.merge", branch->name);
-+ repo_config_set_gently(the_repository, key.buf, NULL);
-+ strbuf_reset(&key);
-+ strbuf_addf(&key, "branch.%s.remote", branch->name);
-+ repo_config_set_gently(the_repository, key.buf, NULL);
-+ }
- }
-
- strbuf_release(&key);
-@@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
- strset_add(&deletable, short_name);
- }
-
-- spare_stacked_bases(refs, &deletable);
-+ spare_stacked_bases(refs, &deletable, flags);
-
- strset_for_each_entry(&deletable, &iter, entry)
- strvec_push(&to_delete, entry->key);
@@ builtin/branch.c: int cmd_branch(int argc,
int delete = 0, rename = 0, copy = 0, list = 0,
unset_upstream = 0, show_current = 0, edit_description = 0;
@@ t/t3200-branch.sh: test_expect_success '--delete-merged keeps the upstream of a
check_branches <<-\EOF &&
feature
main
-@@ t/t3200-branch.sh: test_expect_success '--delete-merged clears the deleted upstream of a spared bra
+@@ t/t3200-branch.sh: test_expect_success '--delete-merged keeps the upstream chain of a surviving bra
git checkout -b tip mid --track &&
git commit --allow-empty -m "tip work" &&
+ git branch --dry-run --delete-merged origin/next \
-+ --delete-merged lower &&
++ --delete-merged lower >actual 2>&1 &&
++ test_must_be_empty actual &&
+
-+ git config --local --get-regexp "branch\\.(mid|tip)\\.(merge|remote)" >actual &&
++ git config --local --get-regexp "branch\\.(lower|mid|tip)\\.(merge|remote)" >actual &&
+ cat >expect <<-\EOF &&
++ branch.lower.remote origin
++ branch.lower.merge refs/heads/next
+ branch.mid.remote .
+ branch.mid.merge refs/heads/lower
+ branch.tip.remote .
@@ t/t3200-branch.sh: test_expect_success '--delete-merged clears the deleted upstr
+ test_cmp expect actual &&
+
git branch --delete-merged origin/next \
- --delete-merged lower &&
-
+ --delete-merged lower >actual 2>&1 &&
+ test_must_be_empty actual &&
@@ t/t3200-branch.sh: test_expect_success "branch -d still deletes a deleteMerged=false branch" '
)
'
--
gitgitgadget
^ permalink raw reply
* [PATCH v20 1/7] branch: add --forked filter for --list mode
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a --forked option to "git branch" list mode that lists only
branches whose configured upstream matches <branch>. The argument
can be a ref (e.g. "origin/main", "master"), a remote name like
"origin" for the branch its origin/HEAD points at, or a shell glob
(e.g. "origin/*"), and may be repeated to widen the filter.
It is an ordinary list filter, so it combines with the others:
git branch --merged origin/main --forked 'origin/*'
lists branches forked from origin that are already merged into
origin/main, and --no-merged inverts the question.
This is the building block for --delete-merged, which deletes the
listed branches once they have landed on their upstream.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-branch.adoc | 12 +++-
builtin/branch.c | 18 +++++-
ref-filter.c | 70 ++++++++++++++++++++
ref-filter.h | 10 +++
t/t3200-branch.sh | 117 ++++++++++++++++++++++++++++++++++
5 files changed, 224 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index c0afddc424..b0d66a6deb 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -13,6 +13,7 @@ git branch [--color[=<when>] | --no-color] [--show-current]
[--column[=<options>] | --no-column] [--sort=<key>]
[--merged [<commit>]] [--no-merged [<commit>]]
[--contains [<commit>]] [--no-contains [<commit>]]
+ [(--forked <branch>)...]
[--points-at <object>] [--format=<format>]
[(-r|--remotes) | (-a|--all)]
[--list] [<pattern>...]
@@ -51,7 +52,8 @@ merged into the named commit (i.e. the branches whose tip commits are
reachable from the named commit) will be listed. With `--no-merged` only
branches not merged into the named commit will be listed. If the _<commit>_
argument is missing it defaults to `HEAD` (i.e. the tip of the current
-branch).
+branch). With `--forked`, only branches whose configured upstream matches
+the given branch or pattern will be listed.
The command's second form creates a new branch head named _<branch-name>_
which points to the current `HEAD`, or _<start-point>_ if given. As a
@@ -311,6 +313,14 @@ superproject's "origin/main", but tracks the submodule's "origin/main".
Only list branches whose tips are not reachable from
_<commit>_ (`HEAD` if not specified). Implies `--list`.
+`--forked <branch>`::
+ Only list branches whose configured upstream matches
+ _<branch>_. The argument can be a ref (e.g. `origin/main`,
+ `master`), a remote name like `origin` for the branch its
+ `origin/HEAD` points at, or a shell-style glob (e.g.
+ `'origin/*'`). The option can be repeated to widen the
+ filter. Implies `--list`.
+
`--points-at <object>`::
Only list branches of _<object>_.
diff --git a/builtin/branch.c b/builtin/branch.c
index dede60d27b..3ac1272d7e 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -30,7 +30,7 @@
#include "commit-reach.h"
static const char * const builtin_branch_usage[] = {
- N_("git branch [<options>] [-r | -a] [--merged] [--no-merged]"),
+ N_("git branch [<options>] [-r | -a] [--merged] [--no-merged] [(--forked <branch>)...]"),
N_("git branch [<options>] [-f] [--recurse-submodules] <branch-name> [<start-point>]"),
N_("git branch [<options>] [-l] [<pattern>...]"),
N_("git branch [<options>] [-r] (-d | -D) <branch-name>..."),
@@ -673,6 +673,16 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
free_worktrees(worktrees);
}
+static int parse_opt_forked(const struct option *opt, const char *arg, int unset)
+{
+ struct ref_filter *filter = opt->value;
+
+ BUG_ON_OPT_NEG(unset);
+ if (ref_filter_forked_add(filter, arg) < 0)
+ die(_("'%s' is not a valid branch or pattern"), arg);
+ return 0;
+}
+
static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
static int edit_branch_description(const char *branch_name)
@@ -793,6 +803,9 @@ int cmd_branch(int argc,
OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
OPT_MERGED(&filter, N_("print only branches that are merged")),
OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
+ OPT_CALLBACK_F(0, "forked", &filter, N_("branch"),
+ N_("print only branches whose upstream matches <branch> (repeatable)"),
+ PARSE_OPT_NONEG, parse_opt_forked),
OPT_COLUMN(0, "column", &colopts, N_("list branches in columns")),
OPT_REF_SORT(&sorting_options),
OPT_CALLBACK(0, "points-at", &filter.points_at, N_("object"),
@@ -838,7 +851,8 @@ int cmd_branch(int argc,
list = 1;
if (filter.with_commit || filter.no_commit ||
- filter.reachable_from || filter.unreachable_from || filter.points_at.nr)
+ filter.reachable_from || filter.unreachable_from ||
+ filter.points_at.nr || filter.forked.nr)
list = 1;
noncreate_actions = !!delete + !!rename + !!copy + !!new_upstream +
diff --git a/ref-filter.c b/ref-filter.c
index 284796c49b..cbdac1a19a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2744,6 +2744,72 @@ static int filter_exclude_match(struct ref_filter *filter, const char *refname)
return match_pattern(filter->exclude.v, refname, filter->ignore_case);
}
+static const char *short_upstream_name(const char *full_ref)
+{
+ const char *short_name = full_ref;
+ (void)(skip_prefix(short_name, "refs/heads/", &short_name) ||
+ skip_prefix(short_name, "refs/remotes/", &short_name));
+ return short_name;
+}
+
+/*
+ * Match the configured upstream of a branch against the registered
+ * --forked patterns. Exact patterns are compared against the full
+ * upstream refname so they are unambiguous; glob patterns are matched
+ * against the abbreviated upstream so that a glob such as origin/...
+ * works as typed.
+ */
+static int filter_forked_match(struct ref_filter *filter, const char *refname)
+{
+ const char *short_name;
+ struct branch *branch;
+ const char *upstream;
+ int i;
+
+ if (!skip_prefix(refname, "refs/heads/", &short_name))
+ return 0;
+ branch = branch_get(short_name);
+ if (!branch)
+ return 0;
+ upstream = branch_get_upstream(branch, NULL);
+ if (!upstream)
+ return 0;
+
+ for (i = 0; i < filter->forked.nr; i++) {
+ const char *pattern = filter->forked.v[i];
+ if (has_glob_specials(pattern)) {
+ if (!wildmatch(pattern, short_upstream_name(upstream),
+ WM_PATHNAME))
+ return 1;
+ } else if (!strcmp(pattern, upstream)) {
+ return 1;
+ }
+ }
+ return 0;
+}
+
+int ref_filter_forked_add(struct ref_filter *filter, const char *arg)
+{
+ struct object_id oid;
+ char *full_ref = NULL;
+
+ if (has_glob_specials(arg)) {
+ strvec_push(&filter->forked, arg);
+ return 0;
+ }
+
+ if (repo_dwim_ref(the_repository, arg, strlen(arg), &oid,
+ &full_ref, 0) == 1 &&
+ (starts_with(full_ref, "refs/heads/") ||
+ starts_with(full_ref, "refs/remotes/"))) {
+ strvec_push(&filter->forked, full_ref);
+ free(full_ref);
+ return 0;
+ }
+ free(full_ref);
+ return -1;
+}
+
/*
* We need to seek to the reference right after a given marker but excluding any
* matching references. So we seek to the lexicographically next reference.
@@ -2979,6 +3045,9 @@ static struct ref_array_item *apply_ref_filter(const struct reference *ref,
if (filter->points_at.nr && !match_points_at(&filter->points_at, ref->oid, ref->name))
return NULL;
+ if (filter->forked.nr && !filter_forked_match(filter, ref->name))
+ return NULL;
+
/*
* A merge filter is applied on refs pointing to commits. Hence
* obtain the commit using the 'oid' available and discard all
@@ -3764,6 +3833,7 @@ void ref_filter_init(struct ref_filter *filter)
void ref_filter_clear(struct ref_filter *filter)
{
strvec_clear(&filter->exclude);
+ strvec_clear(&filter->forked);
oid_array_clear(&filter->points_at);
commit_list_free(filter->with_commit);
commit_list_free(filter->no_commit);
diff --git a/ref-filter.h b/ref-filter.h
index 120221b47f..9361296e2a 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -67,6 +67,7 @@ struct ref_filter {
const char **name_patterns;
const char *start_after;
struct strvec exclude;
+ struct strvec forked;
struct oid_array points_at;
struct commit_list *with_commit;
struct commit_list *no_commit;
@@ -110,6 +111,7 @@ struct ref_format {
#define REF_FILTER_INIT { \
.points_at = OID_ARRAY_INIT, \
.exclude = STRVEC_INIT, \
+ .forked = STRVEC_INIT, \
}
#define REF_FORMAT_INIT { \
.use_color = GIT_COLOR_UNKNOWN, \
@@ -172,6 +174,14 @@ void ref_sorting_release(struct ref_sorting *);
struct ref_sorting *ref_sorting_options(struct string_list *);
/* Function to parse --merged and --no-merged options */
int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset);
+/*
+ * Register a --forked <branch> pattern on the filter. The argument is
+ * either a ref, which is resolved to its full refname, or a shell-style
+ * glob. Branches are kept only when their configured upstream matches
+ * one of the registered patterns. Returns -1 if the argument is not a
+ * valid ref or pattern.
+ */
+int ref_filter_forked_add(struct ref_filter *filter, const char *arg);
/* Get the current HEAD's description */
char *get_head_description(void);
/* Set up translated strings in the output. */
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 1ecbafbee1..4ffd224a71 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1755,4 +1755,121 @@ test_expect_success 'errors if given a bad branch name' '
test_cmp expect actual
'
+test_expect_success '--forked: setup' '
+ test_create_repo forked-upstream &&
+ (
+ cd forked-upstream &&
+ test_commit base &&
+ git branch one base &&
+ git branch two base
+ ) &&
+
+ test_create_repo forked-other &&
+ (
+ cd forked-other &&
+ test_commit other-base &&
+ git branch foreign other-base
+ ) &&
+
+ git clone forked-upstream forked &&
+ (
+ cd forked &&
+ git remote add -f other ../forked-other &&
+ git branch local-base &&
+ git branch --track local-one origin/one &&
+ git branch --track local-two origin/two &&
+ git branch --track local-foreign other/foreign &&
+ git branch --track local-onbase local-base &&
+
+ git checkout local-one &&
+ test_commit --no-tag local-one-work local-one.t &&
+ git checkout local-foreign &&
+ test_commit --no-tag local-foreign-work local-foreign.t
+ )
+'
+
+test_expect_success '--forked <upstream-tracking-branch> filters by upstream' '
+ git -C forked branch --forked origin/one --format="%(refname:short)" >actual &&
+ echo local-one >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--forked <glob> filters by wildmatch' '
+ git -C forked branch --forked "origin/*" --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-one
+ local-two
+ main
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked <local-branch> matches branches with local upstream' '
+ git -C forked branch --forked local-base --format="%(refname:short)" >actual &&
+ echo local-onbase >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--forked can be repeated to widen the filter' '
+ git -C forked branch --forked origin/one --forked other/foreign --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-one
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked combines literal and glob arguments' '
+ git -C forked branch --forked local-base --forked "other/*" --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-onbase
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked "*/*" covers every remote-tracking upstream' '
+ git -C forked branch --forked "*/*" --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-foreign
+ local-one
+ local-two
+ main
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked composes with --no-merged' '
+ git -C forked branch --forked "origin/*" --no-merged origin/one \
+ --format="%(refname:short)" >actual &&
+ echo local-one >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--forked <remote> uses the branch <remote>/HEAD points at' '
+ git -C forked branch --forked origin --format="%(refname:short)" >actual &&
+ echo main >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--forked narrows a <pattern> argument' '
+ git -C forked branch --forked "origin/*" "local-*" \
+ --format="%(refname:short)" >actual &&
+ cat >expect <<-\EOF &&
+ local-one
+ local-two
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success '--forked rejects unknown branch/pattern' '
+ test_must_fail git -C forked branch --forked nope 2>err &&
+ test_grep "not a valid branch or pattern" err
+'
+
+test_expect_success '--forked requires a value' '
+ test_must_fail git -C forked branch --forked 2>err &&
+ test_grep "requires a value" err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v20 2/7] branch: convert delete_branches() to a flags argument
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
delete_branches() takes separate force and quiet parameters, while
check_branch_commit() takes force. The next commits would grow this
collection further. Replace them with a single unsigned flags argument
and an enum.
Test the FORCE and QUIET bits directly from flags at each use site so
that mutating or forwarding flags cannot leave cached values stale.
No change in behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/branch.c | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 3ac1272d7e..09631f93f7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -189,16 +189,22 @@ static int branch_merged(int kind, const char *name,
return merged;
}
+enum delete_branch_flags {
+ DELETE_BRANCH_FORCE = (1 << 0),
+ DELETE_BRANCH_QUIET = (1 << 1),
+};
+
static int check_branch_commit(const char *branchname, const char *refname,
const struct object_id *oid, struct commit *head_rev,
- int kinds, int force)
+ int kinds, unsigned int flags)
{
struct commit *rev = lookup_commit_reference(the_repository, oid);
- if (!force && !rev) {
+ if (!(flags & DELETE_BRANCH_FORCE) && !rev) {
error(_("couldn't look up commit object for '%s'"), refname);
return -1;
}
- if (!force && !branch_merged(kinds, branchname, rev, head_rev)) {
+ if (!(flags & DELETE_BRANCH_FORCE) &&
+ !branch_merged(kinds, branchname, rev, head_rev)) {
error(_("the branch '%s' is not fully merged"), branchname);
advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
_("If you are sure you want to delete it, "
@@ -217,8 +223,8 @@ static void delete_branch_config(const char *branchname)
strbuf_release(&buf);
}
-static int delete_branches(int argc, const char **argv, int force, int kinds,
- int quiet)
+static int delete_branches(int argc, const char **argv, int kinds,
+ unsigned int flags)
{
struct commit *head_rev = NULL;
struct object_id oid;
@@ -241,7 +247,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
remote_branch = 1;
allowed_interpret = INTERPRET_BRANCH_REMOTE;
- force = 1;
+ flags |= DELETE_BRANCH_FORCE;
break;
case FILTER_REFS_BRANCHES:
fmt = "refs/heads/%s";
@@ -252,12 +258,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
}
branch_name_pos = strcspn(fmt, "%");
- if (!force)
+ if (!(flags & DELETE_BRANCH_FORCE))
head_rev = lookup_commit_reference(the_repository, &head_oid);
for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
char *target = NULL;
- int flags = 0;
+ int ref_flags = 0;
copy_branchname(&bname, argv[i], allowed_interpret);
free(name);
@@ -279,7 +285,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
RESOLVE_REF_READING
| RESOLVE_REF_NO_RECURSE
| RESOLVE_REF_ALLOW_BAD_NAME,
- &oid, &flags);
+ &oid, &ref_flags);
if (!target) {
if (remote_branch) {
error(_("remote-tracking branch '%s' not found"), bname.buf);
@@ -291,7 +297,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
| RESOLVE_REF_NO_RECURSE
| RESOLVE_REF_ALLOW_BAD_NAME,
&oid,
- &flags);
+ &ref_flags);
FREE_AND_NULL(virtual_name);
if (virtual_target)
@@ -306,16 +312,16 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
continue;
}
- if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
+ if (!(ref_flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
- force)) {
+ flags)) {
ret = 1;
goto next;
}
item = string_list_append(&refs_to_delete, name);
- item->util = xstrdup((flags & REF_ISBROKEN) ? "broken"
- : (flags & REF_ISSYMREF) ? target
+ item->util = xstrdup((ref_flags & REF_ISBROKEN) ? "broken"
+ : (ref_flags & REF_ISSYMREF) ? target
: repo_find_unique_abbrev(the_repository, &oid, DEFAULT_ABBREV));
next:
@@ -330,7 +336,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
char *name = item->string;
if (!refs_ref_exists(get_main_ref_store(the_repository), name)) {
char *refname = name + branch_name_pos;
- if (!quiet)
+ if (!(flags & DELETE_BRANCH_QUIET))
printf(remote_branch
? _("Deleted remote-tracking branch %s (was %s).\n")
: _("Deleted branch %s (was %s).\n"),
@@ -895,7 +901,9 @@ int cmd_branch(int argc,
if (delete) {
if (!argc)
die(_("branch name required"));
- ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
+ ret = delete_branches(argc, argv, filter.kind,
+ (delete > 1 ? DELETE_BRANCH_FORCE : 0) |
+ (quiet ? DELETE_BRANCH_QUIET : 0));
goto out;
} else if (show_current) {
print_current_branch_name();
--
gitgitgadget
^ permalink raw reply related
* [PATCH v20 3/7] branch: let delete_branches skip unmerged branches on bulk refusal
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a skip-unmerged mode to delete_branches() and check_branch_commit()
so a bulk caller can silently skip branches that are not fully merged
and carry on, rather than erroring with the "use 'git branch -D'"
advice that the plain "git branch -d" path emits. Existing callers are
unaffected.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/branch.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 09631f93f7..504117d1c3 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -192,6 +192,7 @@ static int branch_merged(int kind, const char *name,
enum delete_branch_flags {
DELETE_BRANCH_FORCE = (1 << 0),
DELETE_BRANCH_QUIET = (1 << 1),
+ DELETE_BRANCH_SKIP_UNMERGED = (1 << 2),
};
static int check_branch_commit(const char *branchname, const char *refname,
@@ -205,10 +206,13 @@ static int check_branch_commit(const char *branchname, const char *refname,
}
if (!(flags & DELETE_BRANCH_FORCE) &&
!branch_merged(kinds, branchname, rev, head_rev)) {
- error(_("the branch '%s' is not fully merged"), branchname);
- advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
- _("If you are sure you want to delete it, "
- "run 'git branch -D %s'"), branchname);
+ if (!(flags & DELETE_BRANCH_SKIP_UNMERGED)) {
+ error(_("the branch '%s' is not fully merged"),
+ branchname);
+ advise_if_enabled(ADVICE_FORCE_DELETE_BRANCH,
+ _("If you are sure you want to delete it, "
+ "run 'git branch -D %s'"), branchname);
+ }
return -1;
}
return 0;
@@ -315,7 +319,8 @@ static int delete_branches(int argc, const char **argv, int kinds,
if (!(ref_flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
flags)) {
- ret = 1;
+ if (!(flags & DELETE_BRANCH_SKIP_UNMERGED))
+ ret = 1;
goto next;
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v20 4/7] branch: prepare delete_branches for a bulk caller
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Teach delete_branches() a new mode for the upcoming --delete-merged
caller that checks whether a branch is merged into its upstream without
falling back to HEAD when there is no upstream. Existing callers keep
their current behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/branch.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 504117d1c3..1ef8362c12 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -168,10 +168,13 @@ static int branch_merged(int kind, const char *name,
* upstream, if any, otherwise with HEAD", we should just
* return the result of the repo_in_merge_bases() above without
* any of the following code, but during the transition period,
- * a gentle reminder is in order.
+ * a gentle reminder is in order. Callers that opt out of the
+ * HEAD fallback by passing head_rev=NULL are not interested in
+ * the reminder either: they have already established that the
+ * branch has an upstream, so HEAD is irrelevant to the decision.
*/
- if (head_rev != reference_rev) {
- int expect = head_rev ? repo_in_merge_bases(the_repository, rev, head_rev) : 0;
+ if (head_rev && head_rev != reference_rev) {
+ int expect = repo_in_merge_bases(the_repository, rev, head_rev);
if (expect < 0)
exit(128);
if (expect == merged)
@@ -193,6 +196,7 @@ enum delete_branch_flags {
DELETE_BRANCH_FORCE = (1 << 0),
DELETE_BRANCH_QUIET = (1 << 1),
DELETE_BRANCH_SKIP_UNMERGED = (1 << 2),
+ DELETE_BRANCH_NO_HEAD_FALLBACK = (1 << 3),
};
static int check_branch_commit(const char *branchname, const char *refname,
@@ -262,7 +266,8 @@ static int delete_branches(int argc, const char **argv, int kinds,
}
branch_name_pos = strcspn(fmt, "%");
- if (!(flags & DELETE_BRANCH_FORCE))
+ if (!(flags & DELETE_BRANCH_FORCE) &&
+ !(flags & DELETE_BRANCH_NO_HEAD_FALLBACK))
head_rev = lookup_commit_reference(the_repository, &head_oid);
for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
--
gitgitgadget
^ permalink raw reply related
* [PATCH v20 5/7] branch: add --delete-merged <branch>
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
git branch (--delete-merged <branch>)... [<pattern>...]
deletes local branches matching the optional patterns when their
configured upstream matches one of the --delete-merged arguments and
their tip is reachable from that upstream. The work has already landed
on the upstream they track, so the local copy is no longer needed.
The option can be repeated to widen the upstream match. Keeping the
candidate patterns as positional arguments lets users bound the set of
local branches that may be deleted independently of the upstream
selection.
A branch is not deleted when:
* it is checked out in any worktree
* its configured upstream ref no longer exists, since a missing
upstream is not by itself a sign of integration
* pushing it by name to the remote configured by
branch.<name>.remote would update its upstream, as determined by
mapping the branch ref through that remote's fetch refspec. For
example, a local "main" that tracks "origin/main" is kept even when
remote.pushDefault names a fork. Right after a pull it merely looks
fully merged.
A branch whose work is not yet merged into its upstream is silently
skipped, so one unmerged topic does not abort the whole sweep.
A branch that a surviving branch depends on through a chain of local
upstreams is also kept, so no branch is deleted out from under stacked
work. Collect this transitive set without changing the candidate set
during ref iteration: walk upstream chains from surviving branches,
visit each branch at most once, and remove the collected bases only
after the iteration completes. This makes the result independent of
ref iteration order without repeated full scans.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-branch.adoc | 30 +++++
builtin/branch.c | 159 +++++++++++++++++++++++++-
t/t3200-branch.sh | 204 ++++++++++++++++++++++++++++++++++
3 files changed, 391 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index b0d66a6deb..2a96cd7253 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -25,6 +25,7 @@ git branch (-m|-M) [<old-branch>] <new-branch>
git branch (-c|-C) [<old-branch>] <new-branch>
git branch (-d|-D) [-r] <branch-name>...
git branch --edit-description [<branch-name>]
+git branch (--delete-merged <branch>)... [<pattern>...]
DESCRIPTION
-----------
@@ -201,6 +202,35 @@ This option is only applicable in non-verbose mode.
Print the name of the current branch. In detached `HEAD` state,
nothing is printed.
+`--delete-merged <branch>`::
+ Delete local branches whose configured upstream matches
+ _<branch>_, but only when their tip is reachable from that
+ upstream. In other words, the work on the branch has already
+ landed on the upstream it tracks, so the local copy is no longer
+ needed. The option can be repeated to widen the upstream match.
+ Optional _<pattern>_ arguments limit which local branches are
+ considered, e.g. `git branch --delete-merged 'origin/*'
+ 'topic-*'`.
++
+A branch is not deleted when:
++
+--
+* its configured upstream ref no longer exists,
+* it is checked out in any worktree, or
+* pushing it by name to the remote configured by
+ `branch.<name>.remote` would update its upstream, so it cannot be
+ distinguished from a branch that just looks "fully merged" right
+ after a pull.
+--
++
+A branch whose work has not yet been merged into its upstream is
+silently skipped. Delete it with `git branch -D` if you want to
+remove it anyway.
++
+A branch that a surviving branch depends on through a chain of local
+upstreams is kept, so a branch is never deleted out from under stacked
+work.
+
`-v`::
`-vv`::
`--verbose`::
diff --git a/builtin/branch.c b/builtin/branch.c
index 1ef8362c12..b97315df35 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -21,6 +21,7 @@
#include "branch.h"
#include "path.h"
#include "string-list.h"
+#include "strmap.h"
#include "column.h"
#include "utf8.h"
#include "ref-filter.h"
@@ -38,6 +39,7 @@ static const char * const builtin_branch_usage[] = {
N_("git branch [<options>] (-c | -C) [<old-branch>] <new-branch>"),
N_("git branch [<options>] [-r | -a] [--points-at]"),
N_("git branch [<options>] [-r | -a] [--format]"),
+ N_("git branch [<options>] (--delete-merged <branch>)... [<pattern>...]"),
NULL
};
@@ -699,6 +701,149 @@ static int parse_opt_forked(const struct option *opt, const char *arg, int unset
return 0;
}
+struct stacked_branch_data {
+ struct strset *deletable_branch_names;
+ struct strset *protected_branch_names;
+ struct strset *visited_branch_names;
+};
+
+static int collect_stacked_branch_bases(const struct reference *ref,
+ void *cb_data)
+{
+ struct stacked_branch_data *data = cb_data;
+ const char *branch_name;
+
+ if (!skip_prefix(ref->name, "refs/heads/", &branch_name))
+ BUG("expected local branch ref, got '%s'", ref->name);
+ if (strset_contains(data->deletable_branch_names, branch_name))
+ return 0;
+
+ while (strset_add(data->visited_branch_names, branch_name)) {
+ struct branch *branch = branch_get(branch_name);
+ const char *upstream_refname = branch_get_upstream(branch, NULL);
+ const char *upstream_branch_name;
+
+ if (!upstream_refname ||
+ !skip_prefix(upstream_refname, "refs/heads/",
+ &upstream_branch_name) ||
+ !strset_contains(data->deletable_branch_names,
+ upstream_branch_name))
+ break;
+
+ strset_add(data->protected_branch_names, upstream_branch_name);
+ branch_name = upstream_branch_name;
+ }
+
+ return 0;
+}
+
+static void protect_stacked_branch_bases(struct ref_store *refs,
+ struct strset *deletable_branch_names)
+{
+ struct strset protected_branch_names = STRSET_INIT;
+ struct strset visited_branch_names = STRSET_INIT;
+ struct stacked_branch_data data = {
+ .deletable_branch_names = deletable_branch_names,
+ .protected_branch_names = &protected_branch_names,
+ .visited_branch_names = &visited_branch_names,
+ };
+ struct refs_for_each_ref_options opts = {
+ .prefix = "refs/heads/",
+ };
+ struct hashmap_iter iter;
+ struct strmap_entry *entry;
+
+ refs_for_each_ref_ext(refs, collect_stacked_branch_bases, &data, &opts);
+
+ strset_for_each_entry(&protected_branch_names, &iter, entry)
+ strset_remove(deletable_branch_names, entry->key);
+
+ strset_clear(&visited_branch_names);
+ strset_clear(&protected_branch_names);
+}
+
+static int branch_pushes_to_upstream(struct branch *branch,
+ const char *upstream)
+{
+ struct remote *remote = remote_get(remote_for_branch(branch, NULL));
+ char *tracking = NULL;
+ int ret = 0;
+
+ if (remote)
+ tracking = apply_refspecs(&remote->fetch, branch->refname);
+ if (tracking && !strcmp(tracking, upstream))
+ ret = 1;
+
+ free(tracking);
+ return ret;
+}
+
+static int delete_merged_branches(const struct strvec *upstreams,
+ const char **argv, unsigned int flags)
+{
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ struct ref_filter filter = REF_FILTER_INIT;
+ struct ref_array candidates = { 0 };
+ struct strset deletable_branch_names = STRSET_INIT;
+ struct strvec branches_to_delete = STRVEC_INIT;
+ struct hashmap_iter iter;
+ struct strmap_entry *entry;
+ size_t i;
+ int ret = 0;
+
+ for (i = 0; i < upstreams->nr; i++)
+ if (ref_filter_forked_add(&filter, upstreams->v[i]) < 0)
+ die(_("'%s' is not a valid branch or pattern"),
+ upstreams->v[i]);
+
+ filter.kind = FILTER_REFS_BRANCHES;
+ filter.name_patterns = argv;
+ filter_refs(&candidates, &filter, filter.kind);
+
+ for (i = 0; i < (size_t)candidates.nr; i++) {
+ const char *branch_refname = candidates.items[i]->refname;
+ const char *branch_name;
+ struct branch *branch;
+ const char *upstream_refname;
+
+ if (!skip_prefix(branch_refname, "refs/heads/", &branch_name))
+ BUG("filter returned non-branch ref '%s'", branch_refname);
+ if (branch_checked_out(branch_refname))
+ continue;
+
+ branch = branch_get(branch_name);
+ upstream_refname = branch_get_upstream(branch, NULL);
+ if (!upstream_refname || !refs_ref_exists(refs, upstream_refname))
+ continue;
+ if (branch_pushes_to_upstream(branch, upstream_refname))
+ continue;
+ if (check_branch_commit(branch_name, branch_name,
+ &candidates.items[i]->objectname, NULL,
+ FILTER_REFS_BRANCHES, DELETE_BRANCH_SKIP_UNMERGED))
+ continue;
+
+ strset_add(&deletable_branch_names, branch_name);
+ }
+
+ protect_stacked_branch_bases(refs, &deletable_branch_names);
+
+ strset_for_each_entry(&deletable_branch_names, &iter, entry)
+ strvec_push(&branches_to_delete, entry->key);
+
+ if (branches_to_delete.nr)
+ ret = delete_branches(branches_to_delete.nr, branches_to_delete.v,
+ FILTER_REFS_BRANCHES,
+ DELETE_BRANCH_SKIP_UNMERGED |
+ DELETE_BRANCH_NO_HEAD_FALLBACK |
+ flags);
+
+ strvec_clear(&branches_to_delete);
+ strset_clear(&deletable_branch_names);
+ ref_array_clear(&candidates);
+ ref_filter_clear(&filter);
+ return ret;
+}
+
static GIT_PATH_FUNC(edit_description, "EDIT_DESCRIPTION")
static int edit_branch_description(const char *branch_name)
@@ -763,6 +908,7 @@ int cmd_branch(int argc,
/* possible actions */
int delete = 0, rename = 0, copy = 0, list = 0,
unset_upstream = 0, show_current = 0, edit_description = 0;
+ struct strvec delete_merged = STRVEC_INIT;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ -816,6 +962,9 @@ int cmd_branch(int argc,
OPT_BOOL(0, "create-reflog", &reflog, N_("create the branch's reflog")),
OPT_BOOL(0, "edit-description", &edit_description,
N_("edit the description for the branch")),
+ OPT_CALLBACK_F(0, "delete-merged", &delete_merged, N_("branch"),
+ N_("delete merged branches whose upstream matches <branch> (repeatable)"),
+ PARSE_OPT_NONEG, parse_opt_strvec),
OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
OPT_MERGED(&filter, N_("print only branches that are merged")),
OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
@@ -863,7 +1012,8 @@ int cmd_branch(int argc,
0);
if (!delete && !rename && !copy && !edit_description && !new_upstream &&
- !show_current && !unset_upstream && argc == 0)
+ !show_current && !unset_upstream && !delete_merged.nr &&
+ argc == 0)
list = 1;
if (filter.with_commit || filter.no_commit ||
@@ -873,7 +1023,7 @@ int cmd_branch(int argc,
noncreate_actions = !!delete + !!rename + !!copy + !!new_upstream +
!!show_current + !!list + !!edit_description +
- !!unset_upstream;
+ !!unset_upstream + !!delete_merged.nr;
if (noncreate_actions > 1)
usage_with_options(builtin_branch_usage, options);
@@ -915,6 +1065,10 @@ int cmd_branch(int argc,
(delete > 1 ? DELETE_BRANCH_FORCE : 0) |
(quiet ? DELETE_BRANCH_QUIET : 0));
goto out;
+ } else if (delete_merged.nr) {
+ ret = delete_merged_branches(&delete_merged, argv,
+ quiet ? DELETE_BRANCH_QUIET : 0);
+ goto out;
} else if (show_current) {
print_current_branch_name();
ret = 0;
@@ -1083,6 +1237,7 @@ int cmd_branch(int argc,
ret = 0;
out:
+ strvec_clear(&delete_merged);
string_list_clear(&sorting_options, 0);
return ret;
}
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 4ffd224a71..268203089b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1872,4 +1872,208 @@ test_expect_success '--forked requires a value' '
test_grep "requires a value" err
'
+test_expect_success '--delete-merged: setup' '
+ git init -b main upstream &&
+ (
+ cd upstream &&
+ test_commit base &&
+ git checkout -b next &&
+ test_commit next-work &&
+ git checkout main
+ ) &&
+ git init -b main other &&
+ test_commit -C other other-base &&
+ git init -b main fork
+'
+
+setup_repo_for_delete_merged () {
+ rm -rf repo &&
+ git clone upstream repo &&
+ (
+ cd repo &&
+ git remote add fork ../fork &&
+ git remote add other ../other &&
+ git config push.default current &&
+ git fetch other
+ )
+}
+
+create_merged_branch () {
+ (
+ cd repo &&
+ git checkout -b "$1" origin/next --track &&
+ git commit --allow-empty -m "$1 work" &&
+ git push origin "$1:next"
+ )
+}
+
+check_branches () {
+ git for-each-ref --format="%(refname:short)" refs/heads/ >actual &&
+ cat >expect &&
+ test_cmp expect actual
+}
+
+test_expect_success '--delete-merged keeps cloned main without a default push remote' '
+ setup_repo_for_delete_merged &&
+ (
+ cd repo &&
+ git checkout --detach &&
+
+ git branch --delete-merged */* &&
+
+ check_branches <<-\EOF
+ main
+ EOF
+ )
+'
+
+test_expect_success '--delete-merged deletes only selected merged branches' '
+ setup_repo_for_delete_merged &&
+ create_merged_branch also-merged &&
+ create_merged_branch merged &&
+ (
+ cd repo &&
+ git checkout -b unmerged origin/next --track &&
+ git commit --allow-empty -m "unmerged work" &&
+ git checkout -b tracks-other other/main --track &&
+ sha=$(git rev-parse --short merged) &&
+
+ git branch --delete-merged origin/next merged >actual 2>&1 &&
+ echo "Deleted branch merged (was $sha)." >expect &&
+ test_cmp expect actual &&
+
+ check_branches <<-\EOF
+ also-merged
+ main
+ tracks-other
+ unmerged
+ EOF
+ )
+'
+
+test_expect_success '--delete-merged keeps main despite a different default push remote' '
+ setup_repo_for_delete_merged &&
+ create_merged_branch on-next &&
+ create_merged_branch checked-out &&
+ create_merged_branch upstream-gone &&
+ (
+ cd repo &&
+ git config remote.pushDefault fork &&
+ git checkout -b local-to-delete main --track &&
+ git update-ref refs/remotes/origin/topic refs/remotes/origin/next &&
+ git branch --set-upstream-to=origin/topic upstream-gone &&
+ git update-ref -d refs/remotes/origin/topic &&
+ git checkout -b tracks-other other/main --track &&
+ git checkout checked-out &&
+
+ git branch --delete-merged origin/* \
+ --delete-merged main &&
+
+ check_branches <<-\EOF
+ checked-out
+ main
+ tracks-other
+ upstream-gone
+ EOF
+ )
+'
+
+test_expect_success '--delete-merged keeps the upstream of a surviving branch' '
+ setup_repo_for_delete_merged &&
+ create_merged_branch feature &&
+ (
+ cd repo &&
+ git checkout -b topic feature --track &&
+ git commit --allow-empty -m "topic work" &&
+
+ git branch --delete-merged origin/next 2>err &&
+
+ test_must_be_empty err &&
+ check_branches <<-\EOF &&
+ feature
+ main
+ topic
+ EOF
+
+ git config --local --get-regexp "branch\\.(feature|topic)\\.(merge|remote)" >actual &&
+ cat >expect <<-\EOF &&
+ branch.feature.remote origin
+ branch.feature.merge refs/heads/next
+ branch.topic.remote .
+ branch.topic.merge refs/heads/feature
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--delete-merged keeps the upstream chain of a surviving branch' '
+ setup_repo_for_delete_merged &&
+ (
+ cd repo &&
+ git config remote.pushDefault fork &&
+ git branch lower origin/next --track &&
+ git branch mid lower --track &&
+ git checkout -b tip mid --track &&
+ git commit --allow-empty -m "tip work" &&
+
+ git branch --delete-merged origin/next \
+ --delete-merged lower >actual 2>&1 &&
+ test_must_be_empty actual &&
+
+ check_branches <<-\EOF &&
+ lower
+ main
+ mid
+ tip
+ EOF
+
+ git config --local --get-regexp "branch\\.(lower|mid|tip)\\.(merge|remote)" >actual &&
+ cat >expect <<-\EOF &&
+ branch.lower.remote origin
+ branch.lower.merge refs/heads/next
+ branch.mid.remote .
+ branch.mid.merge refs/heads/lower
+ branch.tip.remote .
+ branch.tip.merge refs/heads/mid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--delete-merged result is independent of stacked branch names' '
+ setup_repo_for_delete_merged &&
+ (
+ cd repo &&
+ git branch c-lower origin/next --track &&
+ git branch b-mid c-lower --track &&
+ git checkout -b a-tip b-mid --track &&
+ git commit --allow-empty -m "tip work" &&
+
+ git branch --delete-merged origin/next \
+ --delete-merged "c-*" &&
+
+ check_branches <<-\EOF &&
+ a-tip
+ b-mid
+ c-lower
+ main
+ EOF
+
+ git branch --delete-merged origin/next \
+ --delete-merged "c-*" >actual 2>&1 &&
+ test_must_be_empty actual &&
+
+ check_branches <<-\EOF
+ a-tip
+ b-mid
+ c-lower
+ main
+ EOF
+ )
+'
+
+test_expect_success '--delete-merged requires a value' '
+ test_must_fail git -C forked branch --delete-merged 2>err &&
+ test_grep "requires a value" err
+'
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v20 6/7] branch: add branch.<name>.deleteMerged opt-out
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Setting branch.<name>.deleteMerged=false exempts that branch from
"git branch --delete-merged", which is useful for a topic you want
to keep developing after an early round of it has been merged
upstream. Unless --quiet is given, each skip is reported so the
user knows why their topic was kept.
Explicit deletion with "git branch -d" still uses the normal merge
check and ignores this setting.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/config/branch.adoc | 7 +++++++
Documentation/git-branch.adoc | 5 +++--
builtin/branch.c | 14 +++++++++++++
t/t3200-branch.sh | 36 ++++++++++++++++++++++++++++++++
4 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/branch.adoc b/Documentation/config/branch.adoc
index a4db9fa5c8..d8483acb4f 100644
--- a/Documentation/config/branch.adoc
+++ b/Documentation/config/branch.adoc
@@ -102,3 +102,10 @@ for details).
`git branch --edit-description`. Branch description is
automatically added to the `format-patch` cover letter or
`request-pull` summary.
+
+`branch.<name>.deleteMerged`::
+ If set to `false`, branch _<name>_ is exempt from
+ `git branch --delete-merged`. Useful for a topic branch you
+ intend to develop further after an initial round has been
+ merged upstream. Defaults to true. Explicit deletion via
+ `git branch -d` is unaffected.
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index 2a96cd7253..2b206e8689 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -216,11 +216,12 @@ A branch is not deleted when:
+
--
* its configured upstream ref no longer exists,
-* it is checked out in any worktree, or
+* it is checked out in any worktree,
* pushing it by name to the remote configured by
`branch.<name>.remote` would update its upstream, so it cannot be
distinguished from a branch that just looks "fully merged" right
- after a pull.
+ after a pull, or
+* `branch.<name>.deleteMerged` is set to `false`.
--
+
A branch whose work has not yet been merged into its upstream is
diff --git a/builtin/branch.c b/builtin/branch.c
index b97315df35..6573ad7027 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -786,6 +786,7 @@ static int delete_merged_branches(const struct strvec *upstreams,
struct ref_array candidates = { 0 };
struct strset deletable_branch_names = STRSET_INIT;
struct strvec branches_to_delete = STRVEC_INIT;
+ struct strbuf key = STRBUF_INIT;
struct hashmap_iter iter;
struct strmap_entry *entry;
size_t i;
@@ -805,6 +806,7 @@ static int delete_merged_branches(const struct strvec *upstreams,
const char *branch_name;
struct branch *branch;
const char *upstream_refname;
+ int opt_out;
if (!skip_prefix(branch_refname, "refs/heads/", &branch_name))
BUG("filter returned non-branch ref '%s'", branch_refname);
@@ -822,6 +824,17 @@ static int delete_merged_branches(const struct strvec *upstreams,
FILTER_REFS_BRANCHES, DELETE_BRANCH_SKIP_UNMERGED))
continue;
+ strbuf_reset(&key);
+ strbuf_addf(&key, "branch.%s.deletemerged", branch_name);
+ if (!repo_config_get_bool(the_repository, key.buf, &opt_out) &&
+ !opt_out) {
+ if (!(flags & DELETE_BRANCH_QUIET))
+ fprintf(stderr,
+ _("Skipping '%s' (branch.%s.deleteMerged is false)\n"),
+ branch_name, branch_name);
+ continue;
+ }
+
strset_add(&deletable_branch_names, branch_name);
}
@@ -837,6 +850,7 @@ static int delete_merged_branches(const struct strvec *upstreams,
DELETE_BRANCH_NO_HEAD_FALLBACK |
flags);
+ strbuf_release(&key);
strvec_clear(&branches_to_delete);
strset_clear(&deletable_branch_names);
ref_array_clear(&candidates);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 268203089b..7111306150 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -2076,4 +2076,40 @@ test_expect_success '--delete-merged requires a value' '
test_must_fail git -C forked branch --delete-merged 2>err &&
test_grep "requires a value" err
'
+
+test_expect_success '--delete-merged honours branch.<name>.deleteMerged=false' '
+ setup_repo_for_delete_merged &&
+ create_merged_branch deleted &&
+ create_merged_branch kept &&
+ (
+ cd repo &&
+ git config branch.kept.deleteMerged false &&
+ git checkout --detach &&
+
+ git branch --delete-merged origin/next 2>err &&
+
+ test_grep "Skipping .kept." err &&
+ check_branches <<-\EOF
+ kept
+ main
+ EOF
+ )
+'
+
+test_expect_success "branch -d still deletes a deleteMerged=false branch" '
+ setup_repo_for_delete_merged &&
+ create_merged_branch kept &&
+ (
+ cd repo &&
+ git config branch.kept.deleteMerged false &&
+ git checkout --detach &&
+
+ git branch -d kept &&
+
+ check_branches <<-\EOF
+ main
+ EOF
+ )
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v20 7/7] branch: add --dry-run for --delete-merged
From: Harald Nordgren via GitGitGadget @ 2026-07-22 7:10 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git branch --dry-run --delete-merged ..." prints one line per ref that
would be deleted without modifying refs or branch configuration.
--dry-run is only meaningful together with --delete-merged and is
rejected otherwise.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-branch.adoc | 8 +++++++-
builtin/branch.c | 21 ++++++++++++++++---
t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++++-
3 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
index 2b206e8689..51dda15114 100644
--- a/Documentation/git-branch.adoc
+++ b/Documentation/git-branch.adoc
@@ -25,7 +25,7 @@ git branch (-m|-M) [<old-branch>] <new-branch>
git branch (-c|-C) [<old-branch>] <new-branch>
git branch (-d|-D) [-r] <branch-name>...
git branch --edit-description [<branch-name>]
-git branch (--delete-merged <branch>)... [<pattern>...]
+git branch [--dry-run] (--delete-merged <branch>)... [<pattern>...]
DESCRIPTION
-----------
@@ -232,6 +232,12 @@ A branch that a surviving branch depends on through a chain of local
upstreams is kept, so a branch is never deleted out from under stacked
work.
+`--dry-run`::
+ With `--delete-merged`, print which branches would be
+ deleted and exit without touching any ref. Useful for
+ sanity-checking a wide pattern like `'origin/*'` before
+ committing to the deletion.
+
`-v`::
`-vv`::
`--verbose`::
diff --git a/builtin/branch.c b/builtin/branch.c
index 6573ad7027..237eadb401 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -199,6 +199,7 @@ enum delete_branch_flags {
DELETE_BRANCH_QUIET = (1 << 1),
DELETE_BRANCH_SKIP_UNMERGED = (1 << 2),
DELETE_BRANCH_NO_HEAD_FALLBACK = (1 << 3),
+ DELETE_BRANCH_DRY_RUN = (1 << 4),
};
static int check_branch_commit(const char *branchname, const char *refname,
@@ -340,13 +341,20 @@ static int delete_branches(int argc, const char **argv, int kinds,
free(target);
}
- if (refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF))
+ if (!(flags & DELETE_BRANCH_DRY_RUN) &&
+ refs_delete_refs(get_main_ref_store(the_repository), NULL, &refs_to_delete, REF_NO_DEREF))
ret = 1;
for_each_string_list_item(item, &refs_to_delete) {
char *describe_ref = item->util;
char *name = item->string;
- if (!refs_ref_exists(get_main_ref_store(the_repository), name)) {
+ if (flags & DELETE_BRANCH_DRY_RUN) {
+ if (!(flags & DELETE_BRANCH_QUIET))
+ printf(remote_branch
+ ? _("Would delete remote-tracking branch %s (was %s).\n")
+ : _("Would delete branch %s (was %s).\n"),
+ name + branch_name_pos, describe_ref);
+ } else if (!refs_ref_exists(get_main_ref_store(the_repository), name)) {
char *refname = name + branch_name_pos;
if (!(flags & DELETE_BRANCH_QUIET))
printf(remote_branch
@@ -923,6 +931,7 @@ int cmd_branch(int argc,
int delete = 0, rename = 0, copy = 0, list = 0,
unset_upstream = 0, show_current = 0, edit_description = 0;
struct strvec delete_merged = STRVEC_INIT;
+ int dry_run = 0;
const char *new_upstream = NULL;
int noncreate_actions = 0;
/* possible options */
@@ -979,6 +988,8 @@ int cmd_branch(int argc,
OPT_CALLBACK_F(0, "delete-merged", &delete_merged, N_("branch"),
N_("delete merged branches whose upstream matches <branch> (repeatable)"),
PARSE_OPT_NONEG, parse_opt_strvec),
+ OPT_BOOL(0, "dry-run", &dry_run,
+ N_("with --delete-merged, only print which branches would be deleted")),
OPT__FORCE(&force, N_("force creation, move/rename, deletion"), PARSE_OPT_NOCOMPLETE),
OPT_MERGED(&filter, N_("print only branches that are merged")),
OPT_NO_MERGED(&filter, N_("print only branches that are not merged")),
@@ -1041,6 +1052,9 @@ int cmd_branch(int argc,
if (noncreate_actions > 1)
usage_with_options(builtin_branch_usage, options);
+ if (dry_run && !delete_merged.nr)
+ die(_("--dry-run requires --delete-merged"));
+
if (recurse_submodules_explicit) {
if (!submodule_propagate_branches)
die(_("branch with --recurse-submodules can only be used if submodule.propagateBranches is enabled"));
@@ -1081,7 +1095,8 @@ int cmd_branch(int argc,
goto out;
} else if (delete_merged.nr) {
ret = delete_merged_branches(&delete_merged, argv,
- quiet ? DELETE_BRANCH_QUIET : 0);
+ (quiet ? DELETE_BRANCH_QUIET : 0) |
+ (dry_run ? DELETE_BRANCH_DRY_RUN : 0));
goto out;
} else if (show_current) {
print_current_branch_name();
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 7111306150..eb1c57a5ca 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1938,6 +1938,19 @@ test_expect_success '--delete-merged deletes only selected merged branches' '
git checkout -b tracks-other other/main --track &&
sha=$(git rev-parse --short merged) &&
+ git branch --dry-run --delete-merged origin/next merged >actual 2>&1 &&
+ echo "Would delete branch merged (was $sha)." >expect &&
+ test_cmp expect actual &&
+ git rev-parse --verify refs/heads/merged &&
+
+ check_branches <<-\EOF &&
+ also-merged
+ main
+ merged
+ tracks-other
+ unmerged
+ EOF
+
git branch --delete-merged origin/next merged >actual 2>&1 &&
echo "Deleted branch merged (was $sha)." >expect &&
test_cmp expect actual &&
@@ -1986,9 +1999,12 @@ test_expect_success '--delete-merged keeps the upstream of a surviving branch' '
git checkout -b topic feature --track &&
git commit --allow-empty -m "topic work" &&
- git branch --delete-merged origin/next 2>err &&
+ git branch --dry-run --delete-merged origin/next >out &&
+ test_grep ! "feature" out &&
+ git branch --delete-merged origin/next 2>err &&
test_must_be_empty err &&
+
check_branches <<-\EOF &&
feature
main
@@ -2016,6 +2032,21 @@ test_expect_success '--delete-merged keeps the upstream chain of a surviving bra
git checkout -b tip mid --track &&
git commit --allow-empty -m "tip work" &&
+ git branch --dry-run --delete-merged origin/next \
+ --delete-merged lower >actual 2>&1 &&
+ test_must_be_empty actual &&
+
+ git config --local --get-regexp "branch\\.(lower|mid|tip)\\.(merge|remote)" >actual &&
+ cat >expect <<-\EOF &&
+ branch.lower.remote origin
+ branch.lower.merge refs/heads/next
+ branch.mid.remote .
+ branch.mid.merge refs/heads/lower
+ branch.tip.remote .
+ branch.tip.merge refs/heads/mid
+ EOF
+ test_cmp expect actual &&
+
git branch --delete-merged origin/next \
--delete-merged lower >actual 2>&1 &&
test_must_be_empty actual &&
@@ -2112,4 +2143,9 @@ test_expect_success "branch -d still deletes a deleteMerged=false branch" '
)
'
+test_expect_success '--dry-run without --delete-merged is rejected' '
+ test_must_fail git -C forked branch --dry-run 2>err &&
+ test_grep "requires --delete-merged" err
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/2] rebase: handle --update-refs branch symrefs
From: Son Luong Ngoc via GitGitGadget @ 2026-07-22 8:15 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Phillip Wood, Son Luong Ngoc
In-Reply-To: <pull.2126.v2.git.1780482436865.gitgitgadget@gmail.com>
git rebase --update-refs can finish rewriting the current branch and then
fail while updating a local branch that is a symbolic ref. This can happen
during a default-branch rename where refs/heads/main points at
refs/heads/master while users migrate. The failure leaves refs partially
updated even though the main rebase has succeeded.
Resolve local branch decorations before adding update-ref commands. The
first patch skips aliases whose targets are other branches and preserves the
existing handling of the current branch. The second patch keeps aliases to
non-branch refs supported while preventing duplicate and cross-worktree
updates to their resolved targets.
Changes since v2:
* Skip branch-to-branch symrefs before checked-out handling.
* Restore the unconditional current-branch skip and keep an owned copy of
the resolved HEAD name.
* Check both a non-branch symref alias and its resolved target against
checked-out reservations.
* Deduplicate aliases that share a non-branch target.
* Reserve resolved targets from other worktrees' in-progress update-refs
state.
* Split the branch-alias fix and non-branch safeguards into separate
patches.
* Rebase onto 48bbf81c29 (The 5th batch).
The focused t3400 and t3404 test suites pass with both the files and
reftable backends.
Son Luong Ngoc (2):
rebase: skip branch symref aliases
rebase: guard non-branch symref targets
branch.c | 15 ++++++
sequencer.c | 63 ++++++++++++++++++++-----
t/t3400-rebase.sh | 2 +-
t/t3404-rebase-interactive.sh | 88 +++++++++++++++++++++++++++++++++++
4 files changed, 155 insertions(+), 13 deletions(-)
base-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2126
Range-diff vs v2:
1: 68f698225c ! 1: b9a01e9141 rebase: skip branch symref aliases
@@ Metadata
## Commit message ##
rebase: skip branch symref aliases
- git rebase --update-refs can fail after the normal rebase path has
- updated the current branch when another local branch is a symref to it.
- This can happen during a default-branch rename where refs/heads/main
- points at refs/heads/master while users migrate.
+ git rebase --update-refs can finish rewriting the current branch and
+ then fail while updating a local branch that is a symbolic ref. This can
+ happen during a default-branch rename where refs/heads/main points at
+ refs/heads/master while users migrate.
- The sequencer queues update-ref commands from local branch decorations.
- Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
- decorations that are not local branches, such as HEAD and tags. A branch
- symref is different: it is still a local branch decoration, but if it
- resolves to another branch then that target branch is itself present in
- the decoration list and will be updated as a concrete branch.
+ The problem is a partially applied ref update: the main rebase has
+ already succeeded when the later ref update fails.
- Skip branch decorations whose symrefs resolve to refs/heads/*, because
- those targets are already represented by concrete branch decorations.
- This prevents aliases from scheduling a second update for the same
- branch. Keep symrefs to non-branch targets on the existing path.
+ The sequencer queues updates from local branch decorations. Commit
+ 106b6885c7 (rebase: ignore non-branch update-refs) filters out
+ decorations such as HEAD and tags. A branch symref is still a local
+ branch decoration, but refs_update_ref() dereferences it, so an alias to
+ another branch duplicates the concrete branch update.
- Preserve the existing checked-out branch handling before applying these
- skips. Such refs still need a todo-list comment instead of an update-ref
- command, even when the checked-out ref is the branch being rebased or a
- branch symref alias. Use a copy of the resolved HEAD ref so later ref
- resolution does not overwrite it.
+ Resolve local branch decorations before queuing them. Skip symrefs whose
+ targets are under refs/heads/ so that only the concrete branch update is
+ queued. Keep an owned copy of the resolved HEAD and skip the current
+ branch before checked-out handling so later ref resolution cannot change
+ the comparison.
+
+ This prevents a successful rebase from being followed by a failed,
+ partially applied ref update while preserving each alias as a symref.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
while (decoration) {
struct todo_item *item;
const char *path;
-+ const char *resolved_ref;
++ char *resolved_ref;
+ int flags = 0;
size_t base_offset = ctx->buf->len;
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
+ continue;
+ }
+
-+ path = branch_checked_out(decoration->name);
-+
-+ /*
-+ * If the branch is the current HEAD, then it will be
-+ * updated by the default rebase behavior. Exclude it from
-+ * the list of refs to update, unless it is checked out and
-+ * needs a comment in the todo list.
-+ */
-+ if (!path && head_ref && !strcmp(head_ref, decoration->name)) {
++ resolved_ref = refs_resolve_refdup(refs, decoration->name,
++ RESOLVE_REF_READING,
++ NULL, &flags);
++ if (resolved_ref && (flags & REF_ISSYMREF) &&
++ starts_with(resolved_ref, "refs/heads/")) {
++ free(resolved_ref);
+ decoration = decoration->next;
+ continue;
+ }
+
-+ resolved_ref = refs_resolve_ref_unsafe(refs, decoration->name,
-+ RESOLVE_REF_READING,
-+ NULL, &flags);
-+ if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
-+ starts_with(resolved_ref, "refs/heads/")) {
++ /*
++ * If the branch is the current HEAD, then it will be
++ * updated by the default rebase behavior.
++ */
++ if (head_ref && !strcmp(head_ref, decoration->name)) {
++ free(resolved_ref);
decoration = decoration->next;
continue;
}
+
++ path = branch_checked_out(decoration->name);
++
+ ALLOC_GROW(ctx->items,
+ ctx->items_nr + 1,
+ ctx->items_alloc);
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
memset(item, 0, sizeof(*item));
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
strbuf_commented_addf(ctx->buf, comment_line_str,
"Ref %s checked out at '%s'\n",
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
+ item->arg_len = ctx->buf->len - base_offset;
+ ctx->items_nr++;
+
++ free(resolved_ref);
decoration = decoration->next;
}
@@ sequencer.c: static int add_decorations_to_list(const struct commit *commit,
}
+ ## t/t3400-rebase.sh ##
+@@ t/t3400-rebase.sh: test_expect_success 'git rebase --update-ref with core.commentChar and branch on
+ GIT_SEQUENCE_EDITOR="cat >actual" git -c core.commentChar=% \
+ rebase -i --update-refs base &&
+ test_grep "% Ref refs/heads/wt-topic checked out at" actual &&
+- test_grep "% Ref refs/heads/topic2 checked out at" actual
++ test_grep ! "% Ref refs/heads/topic2 checked out at" actual
+ '
+
+ test_done
+
## t/t3404-rebase-interactive.sh ##
@@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs ignores non-branch decorations' '
+ ) &&
+ grep ^update-ref todo >actual &&
+ test_write_lines "update-ref refs/heads/no-conflict-branch" >expect &&
++ test_grep ! "^# Ref refs/heads/update-refs checked out" todo &&
+ test_cmp expect actual
'
test_expect_success '--update-refs updates refs correctly' '
-: ---------- > 2: a653f56ea2 rebase: guard non-branch symref targets
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 1/2] rebase: skip branch symref aliases
From: Son Luong Ngoc via GitGitGadget @ 2026-07-22 8:15 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Phillip Wood, Son Luong Ngoc,
Son Luong Ngoc
In-Reply-To: <pull.2126.v3.git.1784708107.gitgitgadget@gmail.com>
From: Son Luong Ngoc <sluongng@gmail.com>
git rebase --update-refs can finish rewriting the current branch and
then fail while updating a local branch that is a symbolic ref. This can
happen during a default-branch rename where refs/heads/main points at
refs/heads/master while users migrate.
The problem is a partially applied ref update: the main rebase has
already succeeded when the later ref update fails.
The sequencer queues updates from local branch decorations. Commit
106b6885c7 (rebase: ignore non-branch update-refs) filters out
decorations such as HEAD and tags. A branch symref is still a local
branch decoration, but refs_update_ref() dereferences it, so an alias to
another branch duplicates the concrete branch update.
Resolve local branch decorations before queuing them. Skip symrefs whose
targets are under refs/heads/ so that only the concrete branch update is
queued. Keep an owned copy of the resolved HEAD and skip the current
branch before checked-out handling so later ref resolution cannot change
the comparison.
This prevents a successful rebase from being followed by a failed,
partially applied ref update while preserving each alias as a symref.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
sequencer.c | 44 +++++++++++++++++++++++++----------
t/t3400-rebase.sh | 2 +-
t/t3404-rebase-interactive.sh | 16 +++++++++++++
3 files changed, 49 insertions(+), 13 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1355a99a09..63aba60a08 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6465,32 +6465,50 @@ static int add_decorations_to_list(const struct commit *commit,
struct todo_add_branch_context *ctx)
{
const struct name_decoration *decoration = get_name_decoration(&commit->object);
- const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
- "HEAD",
- RESOLVE_REF_READING,
- NULL,
- NULL);
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ char *head_ref = refs_resolve_refdup(refs, "HEAD",
+ RESOLVE_REF_READING,
+ NULL, NULL);
while (decoration) {
struct todo_item *item;
const char *path;
+ char *resolved_ref;
+ int flags = 0;
size_t base_offset = ctx->buf->len;
/*
- * If the branch is the current HEAD, then it will be
- * updated by the default rebase behavior.
- * Exclude it from the list of refs to update,
- * as well as any non-branch decorations.
* Non-branch decorations may be present if the pretty format
* includes "%d", which would have loaded all refs
* into the global decoration table.
*/
- if ((head_ref && !strcmp(head_ref, decoration->name)) ||
- (decoration->type != DECORATION_REF_LOCAL)) {
+ if (decoration->type != DECORATION_REF_LOCAL) {
+ decoration = decoration->next;
+ continue;
+ }
+
+ resolved_ref = refs_resolve_refdup(refs, decoration->name,
+ RESOLVE_REF_READING,
+ NULL, &flags);
+ if (resolved_ref && (flags & REF_ISSYMREF) &&
+ starts_with(resolved_ref, "refs/heads/")) {
+ free(resolved_ref);
+ decoration = decoration->next;
+ continue;
+ }
+
+ /*
+ * If the branch is the current HEAD, then it will be
+ * updated by the default rebase behavior.
+ */
+ if (head_ref && !strcmp(head_ref, decoration->name)) {
+ free(resolved_ref);
decoration = decoration->next;
continue;
}
+ path = branch_checked_out(decoration->name);
+
ALLOC_GROW(ctx->items,
ctx->items_nr + 1,
ctx->items_alloc);
@@ -6498,7 +6516,7 @@ static int add_decorations_to_list(const struct commit *commit,
memset(item, 0, sizeof(*item));
/* If the branch is checked out, then leave a comment instead. */
- if ((path = branch_checked_out(decoration->name))) {
+ if (path) {
item->command = TODO_COMMENT;
strbuf_commented_addf(ctx->buf, comment_line_str,
"Ref %s checked out at '%s'\n",
@@ -6518,9 +6536,11 @@ static int add_decorations_to_list(const struct commit *commit,
item->arg_len = ctx->buf->len - base_offset;
ctx->items_nr++;
+ free(resolved_ref);
decoration = decoration->next;
}
+ free(head_ref);
return 0;
}
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index e62e07b894..1a02f6546b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -471,7 +471,7 @@ test_expect_success 'git rebase --update-ref with core.commentChar and branch on
GIT_SEQUENCE_EDITOR="cat >actual" git -c core.commentChar=% \
rebase -i --update-refs base &&
test_grep "% Ref refs/heads/wt-topic checked out at" actual &&
- test_grep "% Ref refs/heads/topic2 checked out at" actual
+ test_grep ! "% Ref refs/heads/topic2 checked out at" actual
'
test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index e64816770a..11afa8be56 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1975,15 +1975,23 @@ test_expect_success '--update-refs ignores non-branch decorations' '
) &&
grep ^update-ref todo >actual &&
test_write_lines "update-ref refs/heads/no-conflict-branch" >expect &&
+ test_grep ! "^# Ref refs/heads/update-refs checked out" todo &&
test_cmp expect actual
'
test_expect_success '--update-refs updates refs correctly' '
+ test_when_finished "
+ test_might_fail git symbolic-ref -d refs/heads/no-conflict-branch-alias &&
+ test_might_fail git symbolic-ref -d refs/heads/second-alias
+ " &&
git checkout -B update-refs no-conflict-branch &&
git branch -f base HEAD~4 &&
git branch -f first HEAD~3 &&
git branch -f second HEAD~3 &&
git branch -f third HEAD~1 &&
+ git symbolic-ref refs/heads/no-conflict-branch-alias \
+ refs/heads/no-conflict-branch &&
+ git symbolic-ref refs/heads/second-alias refs/heads/second &&
test_commit extra2 fileX &&
git commit --amend --fixup=L &&
@@ -1991,8 +1999,16 @@ test_expect_success '--update-refs updates refs correctly' '
test_cmp_rev HEAD~3 refs/heads/first &&
test_cmp_rev HEAD~3 refs/heads/second &&
+ test_cmp_rev HEAD~3 refs/heads/second-alias &&
test_cmp_rev HEAD~1 refs/heads/third &&
test_cmp_rev HEAD refs/heads/no-conflict-branch &&
+ test_cmp_rev HEAD refs/heads/no-conflict-branch-alias &&
+ test_write_lines refs/heads/no-conflict-branch >expect &&
+ git symbolic-ref refs/heads/no-conflict-branch-alias >actual &&
+ test_cmp expect actual &&
+ test_write_lines refs/heads/second >expect &&
+ git symbolic-ref refs/heads/second-alias >actual &&
+ test_cmp expect actual &&
q_to_tab >expect <<-\EOF &&
Successfully rebased and updated refs/heads/update-refs.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/2] rebase: guard non-branch symref targets
From: Son Luong Ngoc via GitGitGadget @ 2026-07-22 8:15 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Phillip Wood, Son Luong Ngoc,
Son Luong Ngoc
In-Reply-To: <pull.2126.v3.git.1784708107.gitgitgadget@gmail.com>
From: Son Luong Ngoc <sluongng@gmail.com>
A local branch symbolic ref may point outside refs/heads/. Such an alias
cannot be skipped like a branch-to-branch alias because its concrete
target ref is absent from the local branch decoration list.
However, queuing each alias independently can update the same target ref
more than once and make the second compare-and-swap fail. A reservation
from another worktree can also name either an alias or its resolved
target ref, so checking only one form can miss an in-progress update.
Fix these cases by checking both the literal alias and its resolved
target ref against checked-out reservations. Deduplicate updates by
target ref. Also reserve both forms when loading another worktree's
update-refs state. This makes different aliases honor the same
in-progress update.
This keeps non-branch symrefs supported without allowing duplicate or
cross-worktree ref updates.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
branch.c | 15 ++++++++
sequencer.c | 19 +++++++++
t/t3404-rebase-interactive.sh | 72 +++++++++++++++++++++++++++++++++++
3 files changed, 106 insertions(+)
diff --git a/branch.c b/branch.c
index 243db7d0fc..98a50d8368 100644
--- a/branch.c
+++ b/branch.c
@@ -442,10 +442,25 @@ static void prepare_checked_out_branches(void)
&update_refs)) {
struct string_list_item *item;
for_each_string_list_item(item, &update_refs) {
+ char *resolved_ref;
+ int flags = 0;
+
old = strmap_put(¤t_checked_out_branches,
item->string,
xstrdup(wt->path));
free(old);
+
+ resolved_ref = refs_resolve_refdup(
+ get_main_ref_store(the_repository),
+ item->string, RESOLVE_REF_READING,
+ NULL, &flags);
+ if (resolved_ref && (flags & REF_ISSYMREF)) {
+ old = strmap_put(
+ ¤t_checked_out_branches,
+ resolved_ref, xstrdup(wt->path));
+ free(old);
+ }
+ free(resolved_ref);
}
string_list_clear(&update_refs, 1);
}
diff --git a/sequencer.c b/sequencer.c
index 63aba60a08..040b5bf645 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6459,6 +6459,7 @@ struct todo_add_branch_context {
size_t items_alloc;
struct strbuf *buf;
struct string_list refs_to_oids;
+ struct string_list symref_update_targets;
};
static int add_decorations_to_list(const struct commit *commit,
@@ -6473,6 +6474,7 @@ static int add_decorations_to_list(const struct commit *commit,
while (decoration) {
struct todo_item *item;
const char *path;
+ const char *checked_ref;
char *resolved_ref;
int flags = 0;
size_t base_offset = ctx->buf->len;
@@ -6508,6 +6510,17 @@ static int add_decorations_to_list(const struct commit *commit,
}
path = branch_checked_out(decoration->name);
+ if (!path && resolved_ref && (flags & REF_ISSYMREF)) {
+ checked_ref = resolved_ref;
+ path = branch_checked_out(checked_ref);
+ }
+ if (!path && resolved_ref && (flags & REF_ISSYMREF) &&
+ string_list_has_string(&ctx->symref_update_targets,
+ resolved_ref)) {
+ free(resolved_ref);
+ decoration = decoration->next;
+ continue;
+ }
ALLOC_GROW(ctx->items,
ctx->items_nr + 1,
@@ -6523,6 +6536,10 @@ static int add_decorations_to_list(const struct commit *commit,
decoration->name, path);
} else {
struct string_list_item *sti;
+
+ if (resolved_ref && (flags & REF_ISSYMREF))
+ string_list_insert(&ctx->symref_update_targets,
+ resolved_ref);
item->command = TODO_UPDATE_REF;
strbuf_addf(ctx->buf, "%s\n", decoration->name);
@@ -6554,6 +6571,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
struct todo_add_branch_context ctx = {
.buf = &todo_list->buf,
.refs_to_oids = STRING_LIST_INIT_DUP,
+ .symref_update_targets = STRING_LIST_INIT_DUP,
};
ctx.items_alloc = 2 * todo_list->nr + 1;
@@ -6579,6 +6597,7 @@ static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
res = write_update_refs_state(&ctx.refs_to_oids);
string_list_clear(&ctx.refs_to_oids, 1);
+ string_list_clear(&ctx.symref_update_targets, 0);
if (res) {
/* we failed, so clean up the new list. */
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 11afa8be56..110ed8ae63 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -2024,6 +2024,78 @@ test_expect_success '--update-refs updates refs correctly' '
test_cmp expect err.trimmed
'
+test_expect_success '--update-refs checks resolved non-branch symref target' '
+ test_when_finished "
+ git worktree remove --force checked-out-target-wt &&
+ git symbolic-ref -d refs/heads/non-branch-alias &&
+ git tag -d checked-out-target
+ " &&
+ git tag checked-out-target HEAD~1 &&
+ git symbolic-ref refs/heads/non-branch-alias refs/tags/checked-out-target &&
+ git worktree add --detach checked-out-target-wt checked-out-target &&
+ git -C checked-out-target-wt symbolic-ref HEAD refs/tags/checked-out-target &&
+
+ GIT_SEQUENCE_EDITOR="cat >todo" git rebase -i --update-refs HEAD~2 &&
+
+ test_grep "^# Ref refs/heads/non-branch-alias checked out at" todo &&
+ test_write_lines refs/tags/checked-out-target >expect &&
+ git symbolic-ref refs/heads/non-branch-alias >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--update-refs deduplicates non-branch symref targets' '
+ test_when_finished "
+ git symbolic-ref -d refs/heads/non-branch-alias-one &&
+ git symbolic-ref -d refs/heads/non-branch-alias-two &&
+ git tag -d shared-non-branch-target
+ " &&
+ git tag shared-non-branch-target HEAD~1 &&
+ git symbolic-ref refs/heads/non-branch-alias-one \
+ refs/tags/shared-non-branch-target &&
+ git symbolic-ref refs/heads/non-branch-alias-two \
+ refs/tags/shared-non-branch-target &&
+
+ GIT_SEQUENCE_EDITOR=: git rebase -i --force-rebase --update-refs HEAD~2 &&
+
+ test_cmp_rev HEAD~1 refs/heads/non-branch-alias-one &&
+ test_cmp_rev HEAD~1 refs/heads/non-branch-alias-two &&
+ test_write_lines refs/tags/shared-non-branch-target >expect &&
+ git symbolic-ref refs/heads/non-branch-alias-one >actual &&
+ test_cmp expect actual &&
+ git symbolic-ref refs/heads/non-branch-alias-two >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--update-refs honors non-branch symref reservations' '
+ test_when_finished "
+ test_might_fail git worktree remove --force reserved-target-wt &&
+ test_might_fail git symbolic-ref -d \
+ refs/heads/reserved-non-branch-alias-one &&
+ test_might_fail git symbolic-ref -d \
+ refs/heads/reserved-non-branch-alias-two &&
+ test_might_fail git tag -d reserved-non-branch-target
+ " &&
+ git tag reserved-non-branch-target HEAD~1 &&
+ git symbolic-ref refs/heads/reserved-non-branch-alias-one \
+ refs/tags/reserved-non-branch-target &&
+ git symbolic-ref refs/heads/reserved-non-branch-alias-two \
+ refs/tags/reserved-non-branch-target &&
+ git worktree add --detach reserved-target-wt HEAD &&
+ wt_gitdir=$(git -C reserved-target-wt rev-parse --absolute-git-dir) &&
+ mkdir -p "$wt_gitdir/rebase-merge" &&
+ old_oid=$(git rev-parse refs/heads/reserved-non-branch-alias-one) &&
+ test_write_lines refs/heads/reserved-non-branch-alias-one \
+ "$old_oid" "$old_oid" >"$wt_gitdir/rebase-merge/update-refs" &&
+
+ GIT_SEQUENCE_EDITOR="cat >todo" git rebase -i --update-refs HEAD~2 &&
+
+ test_grep "^# Ref refs/heads/reserved-non-branch-alias-one checked out at" \
+ todo &&
+ test_grep "^# Ref refs/heads/reserved-non-branch-alias-two checked out at" \
+ todo &&
+ test_grep ! "^update-ref refs/heads/reserved-non-branch-alias" todo
+'
+
test_expect_success 'respect user edits to update-ref steps' '
git checkout -B update-refs-break no-conflict-branch &&
git branch -f base HEAD~4 &&
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2] rebase: skip branch symref aliases
From: Son Luong Ngoc @ 2026-07-22 8:16 UTC (permalink / raw)
To: git; +Cc: phillip.wood123
In-Reply-To: <f982c386-e329-4ab0-b695-e540bcb9de3d@gmail.com>
On 04/06/2026 16:46, Phillip Wood wrote:
> A symref that points to another branch should always be skipped. When we
> look up which branches are checked out (see worktree.c:add_head_info()) we
> use
>
> refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
> "HEAD",
> 0,
> &wt->head_oid, &flags);
>
> so it will never report a symref as being checked out - it always resolves
> any symrefs first.
Yes, this is the right distinction. Patch 1 now resolves each local
branch decoration before deciding whether to queue it. If the target is
under refs/heads/, the alias is skipped unconditionally and the concrete
branch decoration remains the only update that is queued.
> If we have a symref pointing somewhere outside of "refs/heads" then we
> need to check whether the target is checked out, not the symref itself.
> I'm not sure how likely that is to happen in practice.
Patch 2 handles that case separately. It checks both the literal alias
and its resolved target ref against the checked-out reservations. I also
added a test with a non-branch target checked out in another worktree.
> If a decoration matches the current branch why don't we just skip it like
> we used to? (As an aside the existing code in wrong because if the user
> runs "git rebase --update-refs <upstream> <branch>" HEAD does not point to
> "<branch>" but lets not worry about that now)
Agreed. Patch 1 now skips the current branch before checked-out
handling, as the old code did.
The contrary expectation in t3400 came from head_ref pointing into a
buffer that was reused while resolving another decoration. That could
make the current-branch comparison fail. head_ref is now an owned copy.
The test expects the current branch to be omitted from the todo list.
While adding the non-branch coverage, I found that two aliases to the
same target ref could queue the same update twice and make the second
compare-and-swap fail. Patch 2 deduplicates those updates by target ref.
It also records resolved target refs from other worktrees' in-progress
update-refs state so that a different alias honors the same reservation.
I split the reroll into these two patches so that the branch-alias fix
and the non-branch safeguards can be reviewed independently.
Thanks for the review, and sorry for the slow response.
Thanks,
Son
On Thu, 4 Jun 2026 16:37:39 +0100, Phillip Wood
<phillip.wood123@gmail.com> wrote:
> On 03/06/2026 11:27, Son Luong Ngoc via GitGitGadget wrote:
> > From: Son Luong Ngoc <sluongng@gmail.com>
> >
> > git rebase --update-refs can fail after the normal rebase path has
> > updated the current branch when another local branch is a symref to it.
> > This can happen during a default-branch rename where refs/heads/main
> > points at refs/heads/master while users migrate.
> >
> > The sequencer queues update-ref commands from local branch decorations.
> > Commit 106b6885c7 (rebase: ignore non-branch update-refs) filters out
> > decorations that are not local branches, such as HEAD and tags. A branch
> > symref is different: it is still a local branch decoration, but if it
> > resolves to another branch then that target branch is itself present in
> > the decoration list and will be updated as a concrete branch.
> >
> > Skip branch decorations whose symrefs resolve to refs/heads/*, because
> > those targets are already represented by concrete branch decorations.
> > This prevents aliases from scheduling a second update for the same
> > branch. Keep symrefs to non-branch targets on the existing path.
>
> Makes sense
>
> > Preserve the existing checked-out branch handling before applying these
> > skips. Such refs still need a todo-list comment instead of an update-ref
> > command, even when the checked-out ref is the branch being rebased or a
> > branch symref alias. Use a copy of the resolved HEAD ref so later ref
> > resolution does not overwrite it.
>
> I don't quite understand this. A symref that points to another branch
> should always be skipped. When we look up which branches are checked out
> (see worktree.c:add_head_info()) we use
>
> refs_resolve_ref_unsafe(get_worktree_ref_store(wt),
> "HEAD",
> 0,
> &wt->head_oid, &flags);
>
> so it will never report a symref as being checked out - it always
> resolves any symrefs first.
>
> If we have a symref pointing somewhere outside of "refs/heads" then we
> need to check whether the target is checked out, not the symref itself.
> I'm not sure how likely that is to happen in practice.
>
> > diff --git a/sequencer.c b/sequencer.c
> > index 1ee4b2875b..6ab8b47108 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -6445,28 +6445,46 @@ static int add_decorations_to_list(const struct commit *commit,
> > struct todo_add_branch_context *ctx)
> > {
> > const struct name_decoration *decoration = get_name_decoration(&commit->object);
> > - const char *head_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> > - "HEAD",
> > - RESOLVE_REF_READING,
> > - NULL,
> > - NULL);
> > + struct ref_store *refs = get_main_ref_store(the_repository);
> > + char *head_ref = refs_resolve_refdup(refs, "HEAD",
> > + RESOLVE_REF_READING,
> > + NULL, NULL);
>
> This part and the test look good now
> > while (decoration) {
> > struct todo_item *item;
> > const char *path;
> > + const char *resolved_ref;
> > + int flags = 0;
> > size_t base_offset = ctx->buf->len;
> >
> > /*
> > - * If the branch is the current HEAD, then it will be
> > - * updated by the default rebase behavior.
> > - * Exclude it from the list of refs to update,
> > - * as well as any non-branch decorations.
> > * Non-branch decorations may be present if the pretty format
> > * includes "%d", which would have loaded all refs
> > * into the global decoration table.
> > */
> > - if ((head_ref && !strcmp(head_ref, decoration->name)) ||
> > - (decoration->type != DECORATION_REF_LOCAL)) {
> > + if (decoration->type != DECORATION_REF_LOCAL) {
> > + decoration = decoration->next;
> > + continue;
> > + }
>
> If a decoration matches the current branch why don't we just skip it
> like we used to? (As an aside the existing code in wrong because if the
> user runs "git rebase --update-refs <upstream> <branch>" HEAD does not
> point to "<branch>" but lets not worry about that now)
>
> > + path = branch_checked_out(decoration->name);
>
> As I said above if the symref target is anther branch we should skip it
> and if the target is not a branch then we need to check if the target is
> checked out so we need to resolve the ref before calling
> branch_checked_out().
>
> Thanks
>
> Phillip
^ permalink raw reply
* [PATCH 0/1] Extract only the message log body from git commit.
From: hardikxk @ 2026-07-22 8:38 UTC (permalink / raw)
To: git; +Cc: hardikxk
The patch fixes the `extractLogMessageFromGitCommit` function to skip all the metada of the commit object and only return back the message body.
Previously the function would return the entire data of the objects
including authors tree and SHAs. This patch fixes that to skip over all
that and just return the body of the log message.
hardikxk (1):
Extract only the message body from git commit.
git-p4.py | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
base-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f
--
2.55.0
^ permalink raw reply
* [PATCH 1/1] Extract only the message body from git commit.
From: hardikxk @ 2026-07-22 8:38 UTC (permalink / raw)
To: git; +Cc: hardikxk
In-Reply-To: <20260722083836.744338-1-hardikxk@gmail.com>
The patch fixes the `extractLogMessageFromGitCommit` function to skip all the metada of the commit object and only return back the message body.
Previously the function would return the entire data of the objects
including authors tree and SHAs. This patch fixes that to skip over all
that and just return the body of the log message.
Signed-off-by: hardikxk <hardikxk@gmail.com>
---
git-p4.py | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index c0ca7be..589efcd 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1003,12 +1003,18 @@ def branchExists(ref):
def extractLogMessageFromGitCommit(commit):
logMessage = ""
- # fixme: title is first line of commit, not 1st paragraph.
+ foundNewLine = False
foundTitle = False
for log in read_pipe_lines(["git", "cat-file", "commit", commit]):
- if not foundTitle:
+ if not foundNewLine:
+ # skip anything that is not the commit message
if len(log) == 1:
- foundTitle = True
+ foundNewLine = True
+ continue
+
+ # everything from here is the commit message
+ if not foundTitle:
+ foundTitle = True
continue
logMessage += log
--
2.55.0
^ permalink raw reply related
* Re: What's cooking in git.git (Jul 2026, #09)
From: Christian Couder @ 2026-07-22 9:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqbjc19j9k.fsf@gitster.g>
> * cc/fast-import-usage (2026-07-16) 7 commits
> - fast-import: use struct option for usage string
> - fast-import: move command state globals into 'struct fast_import_state'
> - fast-import: introduce 'struct fast_import_state'
> - fast-import: localize 'i' into the 'for' loops using it
> - api-parse-options.adoc: document hidden and OPT_*_F option macros
> - api-parse-options.adoc: document per-option flags
> - parse-options: introduce OPT_HIDDEN_GROUP
>
> The usage string of 'git fast-import' has been updated to use the
> 'parse_options' API for displaying help, and its SYNOPSIS in the
> documentation has been standardized to match.
>
> Waiting for response.
> cf. <xmqq4ihyehyb.fsf@gitster.g>
> source: <20260716165517.433849-1-christian.couder@gmail.com>
I am having a vacation, so I will likely not be able to reply soon.
Feel free to discard in the meantime.
Thanks.
^ permalink raw reply
* Re: [PATCH] completion: complete paths for git send-email
From: Ben Knoble @ 2026-07-22 10:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: Yury Norov, Yury Norov, git, Thiago Perrotta, Philippe Blain,
Rubén Justo, linux-kernel, Codex
In-Reply-To: <xmqqqzkww3ky.fsf@gitster.g>
> Le 21 juil. 2026 à 15:22, Junio C Hamano <gitster@pobox.com> a écrit :
>
> Yury Norov <ynorov@nvidia.com> writes:
>
>>> In any case, when both a '0001-my-changes.patch' file and a
>>> '0-tolerance-policy' branch exist in your repository and current
>>> working directory, running:
>>> $ git send-email 0<TAB>
>>> should offer both as candidates, I thihk. Since I only ever pass
>>> filenames to the command, I personally do not think it is a huge
>>> loss if the completion script stops looking at refs and sticks to
>>> filenames only, but others may have a use for that feature.
>> Agree. The test should create a file 0001.patch, then a tag
>> 0-tag, then a branch 0-branch, maybe something else that is
>> relevant; and then make sure every option is correctly offered
>> by autocompletion.
>> Guys please let me know if everything else is needed before I send v2.
>
> So in short, we want the problem description updated to something
> like:
>
> When branches and tags whose names share the same prefix as a
> file (or a directory???) that stores a patch exist, the attempt
> to complete that shared prefix
>
> $ git send-email that-shared-prefix<TAB>
>
> should offer both branches, tags, and files (and directories???).
> But the completion only offers branches and tags and fails to
> offer files.
>
> And the description of the solution would follow after that in the
> proposed log message.
>
> As to the tests, using 40-hex is misleading, and 0-branch as you
> said would be sufficient to reproduce and demonstrate the issue, and
> that your code change fixes it.
>
> Ben, anything I missed?
>
> Thanks.
Not from my end, though SZEDER’s review merits some thinking.
Traveling the next week+; replies may be slower (than usual, hah).
^ permalink raw reply
* Re: [PATCH v3 0/2] remote: url-based pushRemote with renamed remotes
From: Ben Knoble @ 2026-07-22 10:35 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
In-Reply-To: <pull.2358.v3.git.git.1784664859.gitgitgadget@gmail.com>
> Le 21 juil. 2026 à 16:14, Harald Nordgren via GitGitGadget <gitgitgadget@gmail.com> a écrit :
>
> Fix git status not showing the push branch after remotes are renamed, when
> branch.<name>.pushRemote is a URL matching exactly one configured remote.
>
> Changes in v4:
>
> * Match against the actual push URL, including pushurl and pushInsteadOf.
> * Clarify how rearranging remotes exposes the git status tracking problem.
> * Simplify and correct the documentation for URL-valued pushRemote.
>
> Changes in v3:
>
> * Revamp commit messages to clarify motivation.
>
> Changes in v2:
>
> * Clarify that URL push destinations already work and that this change only
> restores their tracking information.
> * Document URL values for branch.<name>.pushRemote and their @{push}
> behavior.
>
> Harald Nordgren (2):
> remote: pass repository to push tracking helper
> remote: find tracking branches for URL push destinations
>
> Documentation/config/branch.adoc | 1 +
> Documentation/revisions.adoc | 3 +
> remote.c | 43 +++++++++--
> remote.h | 2 +
> t/t5505-remote.sh | 124 +++++++++++++++++++++++++++++++
> transport.c | 5 +-
> 6 files changed, 172 insertions(+), 6 deletions(-)
>
>
> base-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2358%2FHaraldNordgren%2Fremote-resolve-url-push-tracking-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2358/HaraldNordgren/remote-resolve-url-push-tracking-v3
> Pull-Request: https://github.com/git/git/pull/2358
>
> Range-diff vs v2:
>
> 1: b1ac49de87 = 1: b1ac49de87 remote: pass repository to push tracking helper
> 2: 6e924a7fec ! 2: a343af9d50 remote: find tracking branches for URL push destinations
> @@ Metadata
> ## Commit message ##
> remote: find tracking branches for URL push destinations
>
> - Git already accepts a repository URL as branch.<name>.pushRemote and
> - can push to it. When a configured remote has the same URL, however,
> - "git status" cannot show that remote's push branch.
> + Git accepts a repository URL as branch.<name>.pushRemote and can push
> + to it. This branch setting takes precedence over remote.pushDefault.
>
> - This can happen in fork workflows when the original remote is renamed
> - to "upstream", the fork is added as "origin", and an existing
> - pushRemote value still contains the fork URL. The URL still points to
> - the right repository, so pushing works. However, @{push} is unavailable
> - because Git does not connect the URL to "origin". As a result,
> + A branch can be configured with a URL-valued pushRemote before any push
> + occurs. If the remotes are later rearranged with "git remote rename" and
> + "git remote add", the newly added remote may use that URL. The URL value
> + is unaffected by the rename and continues to take precedence over
> + remote.pushDefault. The URL and the remote then point to the same
> + repository, but Git does not connect them for tracking. Pushing works,
> + but @{push} cannot identify the remote's tracking branch. As a result,
> "git status" cannot show the push branch, and an up-to-date push can
> - leave its local tracking information stale.
> + leave its tracking information stale.
>
> - When exactly one configured remote has the URL as one of its
> - remote.<name>.url values, use its fetch refspec to find and refresh the
> - push branch. Keep the URL as the push destination so the configured
> - remote's push settings do not change existing behavior. Keep the
> - current behavior when no remote matches or multiple remotes match.
> + When exactly one configured remote uses the push destination URL, use
> + that remote for push tracking. Continue to push to the URL so the
> + configured remote's push settings do not change existing behavior. Keep
> + the current behavior when no remote matches or multiple remotes match.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
I find this explanation makes more sense ; it’s not how I use remote renames, since I don’t usually use URL-valued remotes, but I can see how things arise now. Thanks!
^ permalink raw reply
* Re: [PATCH v2 11/12] builtin/gc: fix signedness issues in ODB-related functionality
From: Patrick Steinhardt @ 2026-07-22 11:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqwluyyhv1.fsf@gitster.g>
On Mon, Jul 13, 2026 at 09:28:02AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/builtin/gc.c b/builtin/gc.c
> > index 3207182488..8cf3781313 100644
> > --- a/builtin/gc.c
> > @@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files,
> > if (e->pack->is_cruft)
> > continue;
> > if (limit) {
> > - if (e->pack->pack_size >= limit)
> > + if ((uintmax_t) e->pack->pack_size >= limit)
>
> Here, just like in too_many_loose_objects(), 'limit' is of type
> 'unsigned long'. While it makes sense to convert both sides of
> the comparison to an unsigned type, casting only the left side
> to a type that differs from the right side puzzles me.
>
> Presumably, the other side is of type 'off_t', which is signed,
> explaining the desire to cast it to an unsigned type. But I am
> not sure what happens if 'off_t' is wider than 'unsigned long'.
Yeah, `pack_size` is an `off_t`, which is signed. But we never populate
it with a negative value, so casting it to `uintmax_t` in unnecessary.
The right-hand side is already unsigned, so due to the usual arithmetic
conversion rules it would be automatically promoted to `uintmax_t`, as
well.
Patrick
^ permalink raw reply
* Re: Performance regression in connectivity check during receive-pack (git 2.54)
From: Patrick Steinhardt @ 2026-07-22 11:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Wolfgang Kritzinger, git, jltobler
In-Reply-To: <xmqqtsps76f1.fsf@gitster.g>
On Tue, Jul 21, 2026 at 07:40:02AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Yeah, and that type of regression makes sense for what a593373b09 was
> > trying to do. But I think the v2.54 behavior is wrong. We should check
> > all packs before any loose objects.
> >
> > I'm not sure of the correct fix. This is working against the whole "odb
> > sources are independent and abstract" refactoring that a593373b09 was
> > going for. But I think it's an important optimization. I guess the
> > abstract version would be that each source has "fast" and "slow" lookups
> > or something like that, and we check all fast ones before slow ones. But
> > that is pretty gross.
> >
> > I'll leave it to Patrick to ponder further. I haven't really been paying
> > a lot of attention to the odb refactoring.
>
> I think checking the fast sources before the slow ones is probably
> the best we can do if we want to retain the 'each odb source is an
> opaque object' abstraction.
Seeing that this is about the `tmp_objdir` case: one of the things that
Justin and I wanted to work on anyway is that we want to stop modifying
the list of sources during transactions in the first place. It always
felt kind of gross that we're modifying the sources when creating a
transaction, as the only reason that we do this for is so that the
writes actually go to the temporary object directory instead of to the
primary object source. And that doesn't make a lot of sense to begin
with.
The alternative to this would be to instead have logic in functions like
`odb_write()` that checks whether we have an active transaction or not.
If so, the write would go into the transaction directly instead of going
into the primary source, and consequently we wouldn't even have to
modify the list of sources at all.
This shouldn't create too much of a problem, as we typically don't
intend to even read objects that we've written into the transaction
immediately. It would avoid that we try to read objects from the
temporary object directory. And it would also allow us to eventually
move all the logic to write objects into the transactions exclusively.
I'm currently out of office though, and will be on vacation next week.
I'll explore this area a bit more though once I'm back in office in two
weeks.
Patrick
^ permalink raw reply
* Re: [PATCH 1/1] Extract only the message body from git commit.
From: Pablo Sabater @ 2026-07-22 12:23 UTC (permalink / raw)
To: hardikxk, git
In-Reply-To: <20260722083836.744338-2-hardikxk@gmail.com>
On Wed Jul 22, 2026 at 10:38 AM CEST, hardikxk wrote:
> The patch fixes the `extractLogMessageFromGitCommit` function to skip all the metada of the commit object and only return back the message body.
nit: wrap this long line to a max of ~72 columns.
nit: s/metada/metadata/
>
> Previously the function would return the entire data of the objects
> including authors tree and SHAs. This patch fixes that to skip over all
> that and just return the body of the log message.
nit: I think this can be written more clearly. Let's use present tense
and state things affirmatively:
extractLogMessageFromGitCommit() returns the entire object data,
including authors, tree and SHAs.
Make it return only the log message body.
Don't take this suggestion literally, as we find below that this log
does not match reality.
You may find Documentation/CodingGuidelines and
Documentation/SubmittingPatches interesting.
>
> Signed-off-by: hardikxk <hardikxk@gmail.com>
> ---
> git-p4.py | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index c0ca7be..589efcd 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1003,12 +1003,18 @@ def branchExists(ref):
> def extractLogMessageFromGitCommit(commit):
> logMessage = ""
>
> - # fixme: title is first line of commit, not 1st paragraph.
> + foundNewLine = False
> foundTitle = False
> for log in read_pipe_lines(["git", "cat-file", "commit", commit]):
> - if not foundTitle:
> + if not foundNewLine:
> + # skip anything that is not the commit message
> if len(log) == 1:
> - foundTitle = True
> + foundNewLine = True
> + continue
> +
> + # everything from here is the commit message
> + if not foundTitle:
> + foundTitle = True
> continue
>
> logMessage += log
Reading the code, this doesn't seem to do what the log says it does.
Testing it against what it did before this patch:
First we need to do a bit of investigation, but we end up finding
that the commit that introduced this '# fixme' was:
b016d39756 (Robustness fixes for pipes, 2007-05-23)
I couldn't find a thread about this commit.
*Note that the output does not have line breaks; I'm adding them for
readability*.
previously:
'Extract only the message body from git commit.\n\nThe patch fixes the
`extractLogMessageFromGitCommit` function to skip all the metada of the
commit object and only return back the message body.\n\nPreviously the
function would return the entire data of the objects\nincluding authors
tree and SHAs. This patch fixes that to skip over all\nthat and just
return the body of the log message.\n\nSigned-off-by: hardikxk <hardikxk@gmail.com>\n'
after the patch:
'\nThe patch fixes the `extractLogMessageFromGitCommit` function to skip
all the metada of the commit object and only return back the message
body.\n\nPreviously the function would return the entire data of the
objects\nincluding authors tree and SHAs. This patch fixes that to skip
over all\nthat and just return the body of the log message.
\n\nSigned-off-by: hardikxk <hardikxk@gmail.com>\n'
We can see that the previous output only shows the commit log, title
+ body. There were no SHAs, tree, etc., the opposite of what this
patch's log claimed.
What this patch actually does is drop the commit subject.
Is this what the '# fixme' meant? I'm making assumptions here, since I
couldn't find a thread to be sure why it was added, but I think it is
either about the loop stopping at the blank line rather than at the title
itself, or a warning that a title is just one line and not a paragraph.
Either way, this patch does not address the '# fixme' correctly.
Before continuing, I think we should try to understand what the '# fixme'
meant in the first place.
Regards,
Pablo.
^ permalink raw reply
* Re: [PATCH 0/1] Extract only the message log body from git commit.
From: Pablo Sabater @ 2026-07-22 12:38 UTC (permalink / raw)
To: hardikxk, git
In-Reply-To: <20260722083836.744338-1-hardikxk@gmail.com>
On Wed Jul 22, 2026 at 10:38 AM CEST, hardikxk wrote:
>
> The patch fixes the `extractLogMessageFromGitCommit` function to skip all the metada of the commit object and only return back the message body.
nit: Let's wrap this at ~72 columns.
>
> Previously the function would return the entire data of the objects
> including authors tree and SHAs. This patch fixes that to skip over all
> that and just return the body of the log message.
This repeats what the commit message already says. For a single-patch
series a cover letter is usually not needed.
Documentation/MyFirstContribution [1] notes that the commit message
should already explain the change at a high level, and that any extra
context can go below the '---' line instead. I would drop this cover
letter unless there's something else to say.
>
> hardikxk (1):
> Extract only the message body from git commit.
>
> git-p4.py | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>
> base-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f
[1]: https://github.com/git/git/blob/master/Documentation/MyFirstContribution.adoc#bonus-chapter-one-patch-changes
Regards,
Pablo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox