* Re: [PATCH] pkt-line: initialize packet_buffer to avoid macOS linker warning
From: Harald Nordgren @ 2026-05-28 8:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
In-Reply-To: <CAHwyqnWjHTpWfbMcBHOabny5NQN7xTZmxew2yDWWu3AoosngWA@mail.gmail.com>
So maybe we can do something like this then?
```
+ # Silence Xcode 16.3+ linker warning about __DATA,__common alignment.
+ LD_MAJOR_VERSION = $(shell ld -v 2>&1 | sed -n
's/.*PROJECT:ld-\([0-9]*\).*/\1/p')
+ ifeq ($(shell test "$(LD_MAJOR_VERSION)" -ge 1167 && echo 1),1)
+ BASIC_CFLAGS += -fno-common
+ endif
```
Harald
On Thu, May 28, 2026 at 9:40 AM Harald Nordgren
<haraldnordgren@gmail.com> wrote:
>
> > According to Internet, Xcode 16.3 or newer introduced this insanity,
> > it seems. How about adding -fno-common to your CFLAGS? If it
> > solves the issue, then we can think about teaching config.mak.uname
> > to detect macOS with problematic versions of compilers and add the
> > flag as workaround.
>
> Yes, this works:
>
> ```
> make -s CFLAGS_APPEND="-fno-common"
> ```
>
>
> Harald
^ permalink raw reply
* Re: [PATCH] pkt-line: initialize packet_buffer to avoid macOS linker warning
From: Harald Nordgren @ 2026-05-28 7:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git
In-Reply-To: <xmqqse7cjku5.fsf@gitster.g>
> According to Internet, Xcode 16.3 or newer introduced this insanity,
> it seems. How about adding -fno-common to your CFLAGS? If it
> solves the issue, then we can think about teaching config.mak.uname
> to detect macOS with problematic versions of compilers and add the
> flag as workaround.
Yes, this works:
```
make -s CFLAGS_APPEND="-fno-common"
```
Harald
^ permalink raw reply
* Re: [PATCH 2/2] rebase: skip branch symref aliases
From: Kristoffer Haugsbakk @ 2026-05-28 7:08 UTC (permalink / raw)
To: Koji Nakamaru, git; +Cc: Son Luong Ngoc
In-Reply-To: <0ab0a717441e9fc7c494da194065a948a35a7f01.1779946921.git.gitgitgadget@gmail.com>
On Thu, May 28, 2026, at 07:42, Son Luong Ngoc via GitGitGadget wrote:
>[snip]
> -test_expect_failure '--update-refs skips branch symrefs to current branch' '
> +test_expect_success '--update-refs skips branch symrefs to current branch' '
> test_when_finished "
> test_might_fail git rebase --abort &&
> git checkout primary &&
This style of fixing a bug by:
• Add failing test `test_expect_failure` in the first commit
• Fix the bug in the next commit and flip to `test_expect_success`
Is legitimate and makes it easier to verify that the test really
exercises the regression. But in this project it is preferred to
just do the bug fix + regression test in one patch.
See https://lore.kernel.org/git/xmqqfrdk3aqy.fsf@gitster.g/
> --
> gitgitgadget
^ permalink raw reply
* [PATCH v2 2/2] commit: remove deprecated functions
From: kristofferhaugsbakk @ 2026-05-28 7:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Patrick Steinhardt, git
In-Reply-To: <V2_CV_commit.h_remove_deprecated.732@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
These functions were deprecated in a series of commits merged in
52882024 (Merge branch 'ps/commit-list-functions-renamed', 2026-02-13).
The compatibility was for in-flight topics at the time.
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v2: add ack
commit.h | 19 -------------------
1 file changed, 19 deletions(-)
diff --git a/commit.h b/commit.h
index 58150045afa..5352056f87a 100644
--- a/commit.h
+++ b/commit.h
@@ -203,25 +203,6 @@ struct commit_list *commit_list_reverse(struct commit_list *list);
void commit_list_free(struct commit_list *list);
-/*
- * Deprecated compatibility functions for `struct commit_list`, to be removed
- * once Git 2.53 is released.
- */
-static inline struct commit_list *copy_commit_list(struct commit_list *l)
-{
- return commit_list_copy(l);
-}
-
-static inline struct commit_list *reverse_commit_list(struct commit_list *l)
-{
- return commit_list_reverse(l);
-}
-
-static inline void free_commit_list(struct commit_list *l)
-{
- commit_list_free(l);
-}
-
struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
const char *repo_logmsg_reencode(struct repository *r,
--
2.54.0.16.g8f27b399cbe
^ permalink raw reply related
* [PATCH v2 1/2] *: replace deprecated free_commit_list
From: kristofferhaugsbakk @ 2026-05-28 7:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Patrick Steinhardt, git
In-Reply-To: <V2_CV_commit.h_remove_deprecated.732@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Replace `free_commit_list` with `commit_list_free`. The former was
deprecated in 9f18d089 (commit: rename `free_commit_list()` to conform
to coding guidelines, 2026-01-15).
This allows us to remove all the deprecated functions in the
next commit:
• `copy_commit_list`
• `reverse_commit_list`
• `free_commit_list`
Acked-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
v2: add ack
builtin/history.c | 4 ++--
replay.c | 2 +-
upload-pack.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 0fc06fb2045..091465a59e2 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -284,7 +284,7 @@ static int setup_revwalk(struct repository *repo,
commit_list_insert(original, &from_list);
ret = repo_is_descendant_of(repo, head, from_list);
- free_commit_list(from_list);
+ commit_list_free(from_list);
if (ret < 0) {
ret = error(_("cannot determine descendance"));
@@ -892,7 +892,7 @@ static int split_commit(struct repository *repo,
if (index_file.len)
unlink(index_file.buf);
strbuf_release(&index_file);
- free_commit_list(parents);
+ commit_list_free(parents);
release_index(&index);
return ret;
}
diff --git a/replay.c b/replay.c
index 4ef8abb6077..da531d5bc68 100644
--- a/replay.c
+++ b/replay.c
@@ -120,7 +120,7 @@ static struct commit *create_commit(struct repository *repo,
out:
repo_unuse_commit_buffer(repo, based_on, message);
free_commit_extra_headers(extra);
- free_commit_list(parents);
+ commit_list_free(parents);
strbuf_release(&msg);
free(author);
return (struct commit *)obj;
diff --git a/upload-pack.c b/upload-pack.c
index 9f6d6fe48c8..2bf450ab288 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -886,7 +886,7 @@ static void deepen(struct upload_pack_data *data, int depth)
data->deepen_relative, depth,
SHALLOW, NOT_SHALLOW);
send_shallow(data, result);
- free_commit_list(result);
+ commit_list_free(result);
}
send_unshallow(data);
@@ -900,7 +900,7 @@ static void deepen_by_rev_list(struct upload_pack_data *data,
disable_commit_graph(the_repository);
result = get_shallow_commits_by_rev_list(argv, SHALLOW, NOT_SHALLOW);
send_shallow(data, result);
- free_commit_list(result);
+ commit_list_free(result);
send_unshallow(data);
}
--
2.54.0.16.g8f27b399cbe
^ permalink raw reply related
* [PATCH v2 0/2] commit: remove deprecated functions
From: kristofferhaugsbakk @ 2026-05-28 7:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Patrick Steinhardt, git
In-Reply-To: <CV_commit.h_remove_deprecated.714@msgid.xyz>
From: Kristoffer Haugsbakk <code@khaugsbakk.name>
Topic name: kh/commit-deprecated
Topic summary: Remove deprecated comments that were slated for removal
after Git 2.53.0.
See the comment:
/*
* Deprecated compatibility functions for `struct commit_list`, to be removed
* once Git 2.53 is released.
*/
I merged in `seen` and `next` yesterday and found no new in-flight usages
of these functions.
Update Thursday: retested `seen` (commit [1]) and `next` (commit [2]).
† 1: 7821a69c (Merge branch 'za/completion-hide-dotfiles' into seen, 2026-05-27)
† 2: 2f8565e1 (Sync with 'master', 2026-05-27)
I commented on this patch but apparently it hasn’t hit any of these
integration branches yet:
Patch: replay: support replaying 2-parent merges
Link: https://lore.kernel.org/git/920cc022-8b63-4dbb-a41d-957ee01a5efd@app.fastmail.com/
§ Changes v2
Add ack by Patrick, the author of these compatibility functions.
§ Link to v1
https://lore.kernel.org/git/CV_commit.h_remove_deprecated.714@msgid.xyz/#t
[1/2] *: replace deprecated free_commit_list
[2/2] commit: remove deprecated functions
builtin/history.c | 4 ++--
commit.h | 19 -------------------
replay.c | 2 +-
upload-pack.c | 4 ++--
4 files changed, 5 insertions(+), 24 deletions(-)
Interdiff against v1:
base-commit: 56a4f3c3a221adf1df9b39da69b8a6890f803157
--
2.54.0.16.g8f27b399cbe
^ permalink raw reply
* Re: [PATCH 1/2] *: replace deprecated free_commit_list
From: Kristoffer Haugsbakk @ 2026-05-28 6:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <ahfRdFXJf9SRaz5q@pks.im>
On Thu, May 28, 2026, at 07:24, Patrick Steinhardt wrote:
> On Wed, May 27, 2026 at 03:59:25PM +0200,
>>[snip]
>> ---
>> builtin/history.c | 4 ++--
>> replay.c | 2 +-
>> upload-pack.c | 4 ++--
>> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> Heh. Funny to see that I introduced the new functions, and that I was
> also the one that continued using the old ones most :)
To be honest with you that’s the first thing I checked. “I wonder if
Patrick...” x)
^ permalink raw reply
* Re: [PATCH v3 0/4] Add support for an external command for fetching notes
From: Siddh Raman Pant @ 2026-05-28 5:59 UTC (permalink / raw)
To: git@vger.kernel.org
Cc: oswald.buddenhagen@gmx.de, gitster@pobox.com,
code@khaugsbakk.name, j6t@kdbg.org, peff@peff.net, ps@pks.im,
sandals@crustytoothpaste.net, newren@gmail.com
In-Reply-To: <cover.1779532562.git.siddh.raman.pant@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 3389 bytes --]
Hi,
Pinging since it's been almost a week (v2 was sent on Fri)...
I sent close/during the weekend, so I think this fell through.
Sorry if this bothers. Just curious as I haven't heard back like the
last time.
Thanks,
Siddh
On Sat, May 23 2026 at 16:08:08 +0530, Siddh Raman Pant wrote:
> v2: https://lore.kernel.org/git/cover.1779464886.git.siddh.raman.pant@oracle.com/
> v1: https://lore.kernel.org/git/cover.1779207350.git.siddh.raman.pant@oracle.com/
>
> <...insert text from v1 cover here...>
>
> Changes since v2:
> - Removed stale help text talking about force-killing helper process.
>
> Changes since v1:
> - Removed Documentation commit and sent as a standalone patch.
> - Removed finish_command_with_timeout addition (and thus sleep_nanosec).
> - Squashed the external notes command code, doc, and test commits.
> - Removed horizontal separators from note-external.c.
> - Removed global variables from translation unit and instead store config in
> a dedicated new struct member in struct display_notes_opt.
> - Reworded the main commit to have better explanation of the motivation.
>
> Siddh Raman Pant (4):
> notes: convert raw arg in format_display_notes() to bool
> wrapper: add support for timeout and deadline in read helpers
> t3301: cover generic displayed notes behavior
> notes: support an external command to display notes
>
> Documentation/config/notes.adoc | 59 +++
> Documentation/git-format-patch.adoc | 11 +-
> Documentation/git-range-diff.adoc | 6 +
> Documentation/pretty-options.adoc | 9 +
> Makefile | 2 +
> builtin/log.c | 17 +-
> builtin/name-rev.c | 9 +-
> builtin/range-diff.c | 2 +
> contrib/completion/git-completion.bash | 4 +-
> log-tree.c | 10 +-
> meson.build | 1 +
> notes-external.c | 414 ++++++++++++++++++
> notes-external.h | 53 +++
> notes.c | 266 +++++++++---
> notes.h | 33 +-
> revision.c | 36 +-
> strbuf.c | 26 +-
> strbuf.h | 4 +
> t/helper/meson.build | 1 +
> t/helper/test-external-notes | 64 +++
> t/helper/test-notes-external-config-reset.c | 24 ++
> t/helper/test-tool.c | 1 +
> t/helper/test-tool.h | 1 +
> t/lib-notes.sh | 19 +
> t/t3206-range-diff.sh | 68 +++
> t/t3301-notes.sh | 448 ++++++++++++++++++++
> t/t6120-describe.sh | 17 +
> wrapper.c | 139 +++++-
> wrapper.h | 23 +
> 29 files changed, 1691 insertions(+), 76 deletions(-)
> create mode 100644 notes-external.c
> create mode 100644 notes-external.h
> create mode 100755 t/helper/test-external-notes
> create mode 100644 t/helper/test-notes-external-config-reset.c
> create mode 100644 t/lib-notes.sh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 2/2] rebase: skip branch symref aliases
From: Son Luong Ngoc via GitGitGadget @ 2026-05-28 5:42 UTC (permalink / raw)
To: git; +Cc: Son Luong Ngoc, Son Luong Ngoc
In-Reply-To: <pull.2126.git.1779946921.gitgitgadget@gmail.com>
From: Son Luong Ngoc <sluongng@gmail.com>
rebase --update-refs records local branch decorations before replaying
commits. If a decoration is a symbolic branch such as refs/heads/main
pointing at refs/heads/master, updating it later dereferences back to
master and can fail because the normal rebase path already moved that
branch.
Resolve local branch symref decorations to their referents before
queuing update-ref commands, and skip duplicates. This keeps branch
aliases from scheduling a second update for the same underlying branch
while still using the existing old-OID check for the single queued
update.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
sequencer.c | 63 +++++++++++++++++++++++++++++------
t/t3404-rebase-interactive.sh | 2 +-
2 files changed, 53 insertions(+), 12 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1ee4b2875b..4a83d1337c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6445,15 +6445,22 @@ 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",
+ struct ref_store *refs = get_main_ref_store(the_repository);
+ const char *head_ref = refs_resolve_ref_unsafe(refs, "HEAD",
RESOLVE_REF_READING,
- NULL,
- NULL);
+ NULL, NULL);
+ char *resolved_head_ref = refs_resolve_refdup(refs, "HEAD",
+ RESOLVE_REF_READING,
+ NULL, NULL);
+ struct strbuf update_ref = STRBUF_INIT;
while (decoration) {
struct todo_item *item;
const char *path;
+ const char *ref = decoration->name;
+ const char *resolved_ref;
+ int is_symref = 0;
+ int flags = 0;
size_t base_offset = ctx->buf->len;
/*
@@ -6461,12 +6468,44 @@ static int add_decorations_to_list(const struct commit *commit,
* updated by the default rebase behavior.
* Exclude it from the list of refs to update,
* as well as any non-branch decorations.
+ *
+ * Resolve branch symrefs after checking for the current HEAD so
+ * that aliases do not schedule duplicate updates for their
+ * referents.
+ *
* 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 (head_ref && !strcmp(head_ref, ref)) {
+ decoration = decoration->next;
+ continue;
+ }
+
+ strbuf_reset(&update_ref);
+ resolved_ref = refs_resolve_ref_unsafe(refs, ref,
+ RESOLVE_REF_READING |
+ RESOLVE_REF_NO_RECURSE,
+ NULL, &flags);
+ if ((flags & REF_ISSYMREF) && resolved_ref) {
+ if (!starts_with(resolved_ref, "refs/heads/")) {
+ decoration = decoration->next;
+ continue;
+ }
+
+ strbuf_addstr(&update_ref, resolved_ref);
+ ref = update_ref.buf;
+ is_symref = 1;
+ }
+
+ if ((is_symref && resolved_head_ref &&
+ !strcmp(resolved_head_ref, ref)) ||
+ string_list_has_string(&ctx->refs_to_oids, ref)) {
decoration = decoration->next;
continue;
}
@@ -6478,19 +6517,19 @@ 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 = branch_checked_out(ref))) {
item->command = TODO_COMMENT;
strbuf_commented_addf(ctx->buf, comment_line_str,
"Ref %s checked out at '%s'\n",
- decoration->name, path);
+ ref, path);
} else {
struct string_list_item *sti;
item->command = TODO_UPDATE_REF;
- strbuf_addf(ctx->buf, "%s\n", decoration->name);
+ strbuf_addf(ctx->buf, "%s\n", ref);
sti = string_list_insert(&ctx->refs_to_oids,
- decoration->name);
- sti->util = init_update_ref_record(decoration->name);
+ ref);
+ sti->util = init_update_ref_record(ref);
}
item->offset_in_buf = base_offset;
@@ -6501,6 +6540,8 @@ static int add_decorations_to_list(const struct commit *commit,
decoration = decoration->next;
}
+ strbuf_release(&update_ref);
+ free(resolved_head_ref);
return 0;
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 42ba8cc313..29447c0fc3 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1978,7 +1978,7 @@ test_expect_success '--update-refs ignores non-branch decorations' '
test_cmp expect actual
'
-test_expect_failure '--update-refs skips branch symrefs to current branch' '
+test_expect_success '--update-refs skips branch symrefs to current branch' '
test_when_finished "
test_might_fail git rebase --abort &&
git checkout primary &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/2] t3404: add failing branch symref test
From: Son Luong Ngoc via GitGitGadget @ 2026-05-28 5:42 UTC (permalink / raw)
To: git; +Cc: Son Luong Ngoc, Son Luong Ngoc
In-Reply-To: <pull.2126.git.1779946921.gitgitgadget@gmail.com>
From: Son Luong Ngoc <sluongng@gmail.com>
rebase --update-refs queues local branch decorations by their literal
refnames. When a branch such as refs/heads/main is a symbolic ref to
the current branch, the normal rebase path first updates the current
branch and the queued symref update later tries to update the same
referent with the old value it recorded before the rebase.
Add a known-breakage test that exercises this case so that the fix can
flip it to test_expect_success. The expected behavior is that the branch
symref keeps pointing at the rebased current branch.
Signed-off-by: Son Luong Ngoc <sluongng@gmail.com>
---
t/t3404-rebase-interactive.sh | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c27..42ba8cc313 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1978,6 +1978,31 @@ test_expect_success '--update-refs ignores non-branch decorations' '
test_cmp expect actual
'
+test_expect_failure '--update-refs skips branch symrefs to current branch' '
+ test_when_finished "
+ test_might_fail git rebase --abort &&
+ git checkout primary &&
+ test_might_fail git symbolic-ref -d refs/heads/update-refs-symref-alias &&
+ test_might_fail git branch -D update-refs-symref update-refs-symref-base
+ " &&
+ git checkout -B update-refs-symref-base primary &&
+ test_commit --no-tag update-refs-symref-base symref-base.t &&
+ git checkout -B update-refs-symref &&
+ test_commit --no-tag update-refs-symref-topic symref-topic.t &&
+ git checkout update-refs-symref-base &&
+ test_commit --no-tag update-refs-symref-newbase symref-newbase.t &&
+ git checkout update-refs-symref &&
+ git symbolic-ref refs/heads/update-refs-symref-alias refs/heads/update-refs-symref &&
+
+ git rebase --update-refs update-refs-symref-base 2>err &&
+
+ test_cmp_rev update-refs-symref-base update-refs-symref^ &&
+ test_cmp_rev refs/heads/update-refs-symref refs/heads/update-refs-symref-alias &&
+ test_write_lines refs/heads/update-refs-symref >expect &&
+ git symbolic-ref refs/heads/update-refs-symref-alias >actual &&
+ test_cmp expect actual
+'
+
test_expect_success '--update-refs updates refs correctly' '
git checkout -B update-refs no-conflict-branch &&
git branch -f base HEAD~4 &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/2] rebase: handle --update-refs branch symrefs
From: Son Luong Ngoc via GitGitGadget @ 2026-05-28 5:41 UTC (permalink / raw)
To: git; +Cc: Son Luong Ngoc
git rebase --update-refs can fail after the normal rebase path has
successfully updated the current branch when another local branch is a
symbolic ref to it.
One practical way to arrive at that setup is a default branch rename from
master to main. While the migration is in progress, a user may keep
refs/heads/main as a symbolic ref to refs/heads/master so that both names
continue to work locally.
If pull.rebase is enabled, a plain git pull can then finish the rebase of
master and still fail while trying to update the main alias. The reported
failure looked like this, with line breaks adjusted for the cover letter:
Successfully rebased and updated refs/heads/master.
error: update_ref failed for ref 'refs/heads/main':
cannot lock ref 'refs/heads/main':
is at fc2c7bd5f17abec7861ef759edcd33a1e16662a1
but expected 531cabdfb49098d6ffa502ed4bf91d1b35edfcfa
Updated the following refs with --update-refs:
Failed to update the following refs with --update-refs:
refs/heads/main
The sequencer builds its update-ref todo commands from local branch
decorations. It excludes the current branch by comparing the literal
decoration name with the resolved HEAD ref, so refs/heads/main is not
recognized as the current branch alias. The final update then dereferences
the alias back to master, but master has already moved, so the old-OID check
fails.
This series keeps that failure mode explicit by adding a known-breakage test
first, so that the behavioral expectation is clear before the fix. The fix
then resolves local branch symref decorations before queuing update-ref
commands, skips aliases of the current branch, and skips duplicate
referents. That keeps the existing old-OID protection on the single real
branch update. It also avoids teaching the final ref update step to treat
this race-looking mismatch as success.
Patch overview:
* Patch 1 records the branch-symref failure in t3404.
* Patch 2 flips the test and canonicalizes sequencer behavior.
Son Luong Ngoc (2):
t3404: add failing branch symref test
rebase: skip branch symref aliases
sequencer.c | 63 +++++++++++++++++++++++++++++------
t/t3404-rebase-interactive.sh | 25 ++++++++++++++
2 files changed, 77 insertions(+), 11 deletions(-)
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2126%2Fsluongng%2Fsl%2Frebase-update-refs-symrefs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2126/sluongng/sl/rebase-update-refs-symrefs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2126
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 2/2] commit: remove deprecated functions
From: Patrick Steinhardt @ 2026-05-28 5:24 UTC (permalink / raw)
To: kristofferhaugsbakk; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <commit.h_remove_deprecated.716@msgid.xyz>
On Wed, May 27, 2026 at 03:59:26PM +0200, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> These functions were deprecated in a series of commits merged in
> 52882024 (Merge branch 'ps/commit-list-functions-renamed', 2026-02-13).
>
> The compatibility was for in-flight topics at the time.
Yup, makes sense. Thanks for following through with the cleanup!
Patrick
^ permalink raw reply
* Re: [PATCH 1/2] *: replace deprecated free_commit_list
From: Patrick Steinhardt @ 2026-05-28 5:24 UTC (permalink / raw)
To: kristofferhaugsbakk; +Cc: git, Kristoffer Haugsbakk
In-Reply-To: <commit.h_replace_deprecated.715@msgid.xyz>
On Wed, May 27, 2026 at 03:59:25PM +0200, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> Replace `free_commit_list` with `commit_list_free`. The former was
> deprecated in 9f18d089 (commit: rename `free_commit_list()` to conform
> to coding guidelines, 2026-01-15).
>
> This allows us to remove all the deprecated functions in the
> next commit:
>
> • `copy_commit_list`
> • `reverse_commit_list`
> • `free_commit_list`
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
> builtin/history.c | 4 ++--
> replay.c | 2 +-
> upload-pack.c | 4 ++--
> 3 files changed, 5 insertions(+), 5 deletions(-)
Heh. Funny to see that I introduced the new functions, and that I was
also the one that continued using the old ones most :)
Patrick
^ permalink raw reply
* Suggestion: Real-time or Conflict-Reducing Collaboration Support for Specific Directories
From: 胡锦 @ 2026-05-28 5:15 UTC (permalink / raw)
To: git
Dear Git Team,
My name is Hujin, and I am an IT engineer from China. I have been using Git for more than 10 years.
First of all, I would like to express my appreciation for Git. In my experience, Git remains one of the best and most reliable version control systems in modern software development. It has played an important role in many projects I have worked on.
However, I have also encountered a recurring difficulty in daily use. For certain types of files, especially files under script directories or other frequently modified directories, conflicts happen quite often because updates are not synchronized in real time. In some project scenarios, multiple engineers may edit related scripts at the same time, and this can lead to repeated merge conflicts and extra coordination costs.
I wonder whether Git could provide, in future versions, some optional features similar to online collaborative documents, or directory-level collaboration mechanisms for specific files or folders. For example, users could enable special real-time update, lock, notification, or conflict-reduction behavior for selected directories such as scripts. This could help reduce conflicts and make Git even more convenient for teams working on highly shared files.
I understand that Git is designed as a distributed version control system, and such functionality may not be simple to implement. Still, I believe an optional feature in this direction could be very helpful for many engineering teams.
Thank you for your great work on Git. I look forward to hearing your thoughts.
Best regards,Hujin
^ permalink raw reply
* (no subject)
From: 胡锦 @ 2026-05-28 5:08 UTC (permalink / raw)
To: git
Dear Git Team,
My name is Hujin, and I am an IT engineer from China. I have been using Git for more than 10 years.
First of all, I would like to express my appreciation for Git. In my experience, Git remains one of the best and most reliable version control systems in modern software development. It has played an important role in many projects I have worked on.
However, I have also encountered a recurring difficulty in daily use. For certain types of files, especially files under script directories or other frequently modified directories, conflicts happen quite often because updates are not synchronized in real time. In some project scenarios, multiple engineers may edit related scripts at the same time, and this can lead to repeated merge conflicts and extra coordination costs.
I wonder whether Git could provide, in future versions, some optional features similar to online collaborative documents, or directory-level collaboration mechanisms for specific files or folders. For example, users could enable special real-time update, lock, notification, or conflict-reduction behavior for selected directories such as scripts. This could help reduce conflicts and make Git even more convenient for teams working on highly shared files.
I understand that Git is designed as a distributed version control system, and such functionality may not be simple to implement. Still, I believe an optional feature in this direction could be very helpful for many engineering teams.
Thank you for your great work on Git. I look forward to hearing your thoughts.
Best regards,Hujin
^ permalink raw reply
* Re: [PATCH] pkt-line: initialize packet_buffer to avoid macOS linker warning
From: Junio C Hamano @ 2026-05-28 3:04 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren
In-Reply-To: <pull.2313.git.git.1779901919956.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> pkt-line: initialize packet_buffer to avoid macOS linker warning
>
> Removes this warning:
>
> $ make -s -j8
> GIT_VERSION=2.54.0.380.gc69baaf57b
> ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
This sounds nuts.
Not complaining at you, but we are talking about char[]; what
alignment constraints are they talking about?
> diff --git a/pkt-line.c b/pkt-line.c
> index 3fc3e9ea70..cfd2799677 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -8,7 +8,7 @@
> #include "trace.h"
> #include "write-or-die.h"
>
> -char packet_buffer[LARGE_PACKET_MAX];
> +char packet_buffer[LARGE_PACKET_MAX] = {0};
I do not like this; it sounds more like a workaround for broken
linker (and compiler to certain degree).
This, compiled with a stupid compiler that is too faithful to the
source text, may make the resulting object file on disk larger by
64kB, since the original said "I need 64kB area in BSS with its
starting address recorded as 'packet_buffer'" (which costs almost
nothing) and the updated says "Here is a 64kB of literal data"
(which would record the literal data, even if its bytes happen to be
all NUL). Luckily both versions of GCC and Clang I have notices
that the literal data is all NUL and still keeps the area in BSS
with no change in the object file size or output from "size
packet-line.o", so to me and others on similar systems as I use,
this probably is a benign no-op, but not everywhere.
Are there different versions of C compiler available on macOS for
you to try? I am hoping that even though vendor compilers tend to
lag a bit behind from the public upstream, the problems may have
already been fixed in more fresher versions.
... goes and looks ...
According to Internet, Xcode 16.3 or newer introduced this insanity,
it seems. How about adding -fno-common to your CFLAGS? If it
solves the issue, then we can think about teaching config.mak.uname
to detect macOS with problematic versions of compilers and add the
flag as workaround.
> static const char *packet_trace_prefix = "git";
> static struct trace_key trace_packet = TRACE_KEY_INIT(PACKET);
> static struct trace_key trace_pack = TRACE_KEY_INIT(PACKFILE);
>
> base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
^ permalink raw reply
* [PATCH v3 3/3] daemon: guard NULL REMOTE_PORT in execute() logging
From: Sebastien Tardif via GitGitGadget @ 2026-05-28 2:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Sebastien Tardif, Sebastien Tardif
In-Reply-To: <pull.2300.v3.git.git.1779937016.gitgitgadget@gmail.com>
From: Sebastien Tardif <sebtardif@ncf.ca>
REMOTE_ADDR and REMOTE_PORT are both set by the same code path in
handle(), so when the existing REMOTE_ADDR check passes, REMOTE_PORT
is guaranteed to be non-NULL. Guard REMOTE_PORT as well so that a
future change that breaks this invariant does not pass NULL to
printf's %s, which is undefined behavior.
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/daemon.c b/daemon.c
index 103c08d868..78cca8673f 100644
--- a/daemon.c
+++ b/daemon.c
@@ -753,7 +753,7 @@ static int execute(void)
struct strvec env = STRVEC_INIT;
if (addr)
- loginfo("Connection from %s:%s", addr, port);
+ loginfo("Connection from %s:%s", addr, port ? port : "?");
set_keep_alive(0);
alarm(init_timeout ? init_timeout : timeout);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/3] daemon: fix IPv6 address truncation in ip2str()
From: Sebastien Tardif via GitGitGadget @ 2026-05-28 2:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Sebastien Tardif, Sebastien Tardif
In-Reply-To: <pull.2300.v3.git.git.1779937016.gitgitgadget@gmail.com>
From: Sebastien Tardif <sebtardif@ncf.ca>
The sockaddr struct size (ai_addrlen) is passed as the output buffer
size to inet_ntop(). For IPv6, sizeof(sockaddr_in6) is 28 bytes but
INET6_ADDRSTRLEN is 46, so long IPv6 addresses are silently truncated.
Fix this by passing sizeof(ip) instead, which is the actual size of
the destination buffer. Drop the now-unused len parameter from
ip2str() and update all callers.
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
daemon.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/daemon.c b/daemon.c
index 80fa0226d8..103c08d868 100644
--- a/daemon.c
+++ b/daemon.c
@@ -947,7 +947,7 @@ struct socketlist {
size_t alloc;
};
-static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
+static const char *ip2str(int family, struct sockaddr *sin)
{
#ifdef NO_IPV6
static char ip[INET_ADDRSTRLEN];
@@ -958,11 +958,11 @@ static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
switch (family) {
#ifndef NO_IPV6
case AF_INET6:
- inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, len);
+ inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, sizeof(ip));
break;
#endif
case AF_INET:
- inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, len);
+ inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, sizeof(ip));
break;
default:
xsnprintf(ip, sizeof(ip), "<unknown>");
@@ -1019,14 +1019,14 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
if (bind(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
logerror("Could not bind to %s: %s",
- ip2str(ai->ai_family, ai->ai_addr, ai->ai_addrlen),
+ ip2str(ai->ai_family, ai->ai_addr),
strerror(errno));
close(sockfd);
continue; /* not fatal */
}
if (listen(sockfd, 5) < 0) {
logerror("Could not listen to %s: %s",
- ip2str(ai->ai_family, ai->ai_addr, ai->ai_addrlen),
+ ip2str(ai->ai_family, ai->ai_addr),
strerror(errno));
close(sockfd);
continue; /* not fatal */
@@ -1080,7 +1080,7 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
if ( bind(sockfd, (struct sockaddr *)&sin, sizeof sin) < 0 ) {
logerror("Could not bind to %s: %s",
- ip2str(AF_INET, (struct sockaddr *)&sin, sizeof(sin)),
+ ip2str(AF_INET, (struct sockaddr *)&sin),
strerror(errno));
close(sockfd);
return 0;
@@ -1088,7 +1088,7 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
if (listen(sockfd, 5) < 0) {
logerror("Could not listen to %s: %s",
- ip2str(AF_INET, (struct sockaddr *)&sin, sizeof(sin)),
+ ip2str(AF_INET, (struct sockaddr *)&sin),
strerror(errno));
close(sockfd);
return 0;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/3] daemon: fix IPv6 address corruption in lookup_hostname()
From: Sebastien Tardif via GitGitGadget @ 2026-05-28 2:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Sebastien Tardif, Sebastien Tardif
In-Reply-To: <pull.2300.v3.git.git.1779937016.gitgitgadget@gmail.com>
From: Sebastien Tardif <sebtardif@ncf.ca>
getaddrinfo() is called with AF_UNSPEC hints, so it may return IPv6
results. However, the code unconditionally casts ai_addr to
sockaddr_in and passes AF_INET to inet_ntop(). On IPv6-only hosts,
this reads from the wrong struct offset, producing garbage IP
addresses.
Fix this by checking ai_family and extracting the address pointer
into a local variable before calling inet_ntop() once with the
correct family. Die on unexpected address families.
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
daemon.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/daemon.c b/daemon.c
index 0a7b1aae44..80fa0226d8 100644
--- a/daemon.c
+++ b/daemon.c
@@ -674,9 +674,20 @@ static void lookup_hostname(struct hostinfo *hi)
gai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);
if (!gai) {
- struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
+ void *addr;
+
+ if (ai->ai_family == AF_INET) {
+ struct sockaddr_in *sa = (void *)ai->ai_addr;
+ addr = &sa->sin_addr;
+ } else if (ai->ai_family == AF_INET6) {
+ struct sockaddr_in6 *sa6 = (void *)ai->ai_addr;
+ addr = &sa6->sin6_addr;
+ } else {
+ die("unexpected address family: %d",
+ ai->ai_family);
+ }
- inet_ntop(AF_INET, &sin_addr->sin_addr,
+ inet_ntop(ai->ai_family, addr,
addrbuf, sizeof(addrbuf));
strbuf_addstr(&hi->ip_address, addrbuf);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/3] daemon: fix network address handling bugs
From: Sebastien Tardif via GitGitGadget @ 2026-05-28 2:56 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Sebastien Tardif
In-Reply-To: <pull.2300.v2.git.git.1779905911.gitgitgadget@gmail.com>
Fix three related issues in daemon.c's network address handling:
IPv6 address corruption in lookup_hostname(): getaddrinfo() is called with
AF_UNSPEC hints, so it may return IPv6 results. However, the code
unconditionally casts ai_addr to sockaddr_in and passes AF_INET to
inet_ntop(). On IPv6-only hosts, this reads from the wrong struct offset,
producing garbage IP addresses. Fixed by checking ai_family and handling
both AF_INET and AF_INET6.
IPv6 address truncation in ip2str(): The sockaddr struct size (ai_addrlen)
is passed as the output buffer size to inet_ntop(). For IPv6,
sizeof(sockaddr_in6) is 28 bytes but INET6_ADDRSTRLEN is 46, so long IPv6
addresses are silently truncated. Fixed by passing sizeof(ip) instead, and
dropping the now-unused len parameter.
NULL pointer in execute() logging: REMOTE_PORT environment variable is used
in a format string without a NULL check (only REMOTE_ADDR was checked). If
REMOTE_PORT is unset, NULL is passed to printf's %s, which is undefined
behavior. Fixed by using a fallback string.
Changes since v1:
* Split the single patch into three separate commits, one per fix, per
Patrick's review.
* Deduplicated the address family handling in lookup_hostname(): instead of
duplicating the inet_ntop() call for each family, the address pointer is
extracted into a local void *addr variable first, then inet_ntop() is
called once, per Patrick's suggestion.
* The (void *) intermediate cast on ai_addr is used intentionally: C
guarantees any object pointer round-trips safely through void *, and it
keeps the per-family blocks shorter than spelling out the full struct
casts.
* For the REMOTE_PORT NULL guard: both REMOTE_ADDR and REMOTE_PORT are set
by the same code path in handle(), so neither should be NULL
independently. The guard makes the code consistent with the existing
REMOTE_ADDR check and avoids undefined behavior from printf %s with a
NULL argument.
* Die on unexpected address families in lookup_hostname() rather than
silently leaving addrbuf uninitialized.
Sebastien Tardif (3):
daemon: fix IPv6 address corruption in lookup_hostname()
daemon: fix IPv6 address truncation in ip2str()
daemon: guard NULL REMOTE_PORT in execute() logging
daemon.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2300%2FSebTardif%2Ffix%2Fdaemon-ipv6-and-null-port-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2300/SebTardif/fix/daemon-ipv6-and-null-port-v3
Pull-Request: https://github.com/git/git/pull/2300
Range-diff vs v2:
1: b2d8143811 = 1: b2d8143811 daemon: fix IPv6 address corruption in lookup_hostname()
2: 5c01ec3cad = 2: 5c01ec3cad daemon: fix IPv6 address truncation in ip2str()
3: e312735716 ! 3: 4e74294071 daemon: guard NULL REMOTE_PORT in execute() logging
@@ Commit message
daemon: guard NULL REMOTE_PORT in execute() logging
REMOTE_ADDR and REMOTE_PORT are both set by the same code path in
- handle(), so neither should be NULL independently. However, the
- existing code checks REMOTE_ADDR before the loginfo() call but not
- REMOTE_PORT. If REMOTE_PORT were unset, NULL would be passed to
+ handle(), so when the existing REMOTE_ADDR check passes, REMOTE_PORT
+ is guaranteed to be non-NULL. Guard REMOTE_PORT as well so that a
+ future change that breaks this invariant does not pass NULL to
printf's %s, which is undefined behavior.
- Add a fallback string for the NULL case, matching the existing
- REMOTE_ADDR guard for consistency.
-
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
## daemon.c ##
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH] http: fix memory leak in fetch_and_setup_pack_index()
From: Lorenzo Pegorari @ 2026-05-28 1:22 UTC (permalink / raw)
To: Jeff King; +Cc: git, Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox
In-Reply-To: <20260519191743.GA2269222@coredump.intra.peff.net>
On Tue, May 19, 2026 at 03:17:43PM -0400, Jeff King wrote:
> On Tue, May 19, 2026 at 04:54:45PM +0200, LorenzoPegorari wrote:
>
> > Inside the function `fetch_and_setup_pack_index()`, when the pack
> > obtained using `fetch_pack_index()` fails to be verified by
> > `parse_pack_index()`, the function returns without closing and freeing
> > said pack.
> >
> > Fix this by calling `close_pack_index()` to munmap the index file for
> > the leaking pack (which might have been mmapped by `fetch_pack_index()`
> > or `verify_pack_index()`), and then free it.
>
> OK, I agree we are leaking here, but after reading the patch I'm left
> with a few questions.
>
> > ret = verify_pack_index(new_pack);
> > - if (!ret)
> > - close_pack_index(new_pack);
> > +
> > + close_pack_index(new_pack);
>
> This part was a little confusing at first, because it looked like we are
> already closing the index. But we were doing so on _success_, not on
> failure. Which is a little funny since the point is to be able to read
> from it later, but OK.
>
> At any rate, that is an existing oddity, and I agree that closing it
> before freeing the struct is obviously the right thing to do.
It is indeed weird that we are closing only on success, and not on
failure.
> > free(tmp_idx);
> > - if (ret)
> > + if (ret) {
> > + free(new_pack);
> > return -1;
> > + }
>
> And here we free the actual struct. Good.
>
> But this existing free(tmp_idx) is what puzzles me. We do not need the
> filename anymore regardless of success or failure, so freeing it makes
> sense. But earlier in the function we have:
>
> new_pack = parse_pack_index(the_repository, sha1, tmp_idx);
> if (!new_pack) {
> unlink(tmp_idx);
> free(tmp_idx);
>
> return -1; /* parse_pack_index() already issued error message */
> }
>
> So on parse failure we actually unlink it, but not on verification
> failure. Which seems like it would leave cruft after the process ends.
> And I suspect we probably we did prior to 63aca3f7f1 (dumb-http: store
> downloaded pack idx as tempfile, 2024-10-25), when we started
> registering it as a tempfile to be deleted at process exit.
>
> So I _think_ we could get away with dropping the existing unlink() call
> and just let it get cleaned up at process exit. But if we are going to
> keep it, do we want to also unlink() in this error path? At which point
> it might make more sense to have an "out" label to consolidate all of
> this cleanup.
>
> If we are going to unlink() here it may also make sense to just return
> the tempfile struct from fetch_pack_index(), and then we can call
> delete_tempfile() on it. See the in-code comment in 63aca3f7f1 which
> mentions this hackery.
>
> So I dunno. I think your patch is doing the right thing as-is, but it
> may be worth taking a moment to clean this up a bit further.
The `unlink()` indeed is weird. Pointing me to the commit 63aca3f7f1
really helped me understand how the code changed and the current
situation. Thanks a lot for that.
I've tried testing as thoroughly as possible whether removing the
`unlink()` function call wouldn't change the expected behavior.
*I think* that it can be removed safely, but I'm not 100% sure yet.
If this is the case, I think adding a `goto` "cleanup label" is not
necessary.
> -Peff
Thank you so much Peff for going through this patch,
Lorenzo
^ permalink raw reply
* Re: [PATCH] describe: bail of --contains --all is used with --exclude or --match
From: Jacob Keller @ 2026-05-28 0:43 UTC (permalink / raw)
To: Tuomas Ahola; +Cc: Jacob Keller, git
In-Reply-To: <20260519083559.onq6r%taahol@utu.fi>
On Tue, May 19, 2026 at 1:36 AM Tuomas Ahola <taahol@utu.fi> wrote:
>
> Jacob Keller <jacob.e.keller@intel.com> wrote:
>
> > From: Jacob Keller <jacob.keller@gmail.com>
> >
> > If you try to use git describe --contains with --all, the exclude and
> > match patterns are silently ignored.
> >
> > This results in unexpected behavior, as you may try to provide patterns
> > and expect it to change the result.
> >
>
> I got just bitten by that, and yes, it was quite unexpected.
>
> > Check for this, and have describe die when it encounters this, instead
> > of silently ignoring the provided options.
> >
> > Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> > ---
> >
> > I just found this while trying to use it, the patterns weren't being applied
> > properly.
> >
> > This is pretty quick/dirty, I haven't had time to write a test, or anything.
> >
>
> Would you like to resurrect the patch? It seems it was never merged,
> nor the underlying problem fixed:
>
> ```
> $ git describe --contains --all --match=bogus
> master
> $ git describe --contains --all --exclude="*"
> master
> ```
>
Apologies for a delayed response. I'll try to look into reviving this tomorrow.
^ permalink raw reply
* Re: git mv after the fact
From: Junio C Hamano @ 2026-05-27 23:22 UTC (permalink / raw)
To: Chris Torek; +Cc: Frieder Hannenheim, git
In-Reply-To: <CAPx1GvetxY1T7cuFN_xe51EURr-ED2BqW3E82jj90ko3PSYSyg@mail.gmail.com>
Chris Torek <chris.torek@gmail.com> writes:
>> Chris Torek <chris.torek@gmail.com> writes:
>>> A flag for "git mv" would be convenient (and slightly moreefficient ...
>>
>
> On Tue, May 26, 2026 at 8:09 PM Junio C Hamano <gitster@pobox.com> wrote:
>> May be convenient, but I do not get the "efficient" part.
>
> A normal `git mv` renames the index entry and the file in the working
> tree without running `git add` on the *contents*, so there's no new hash
> computation. Presumably a `git mv --after foo bar` would do the same: verify
> that there is no existing `bar` in the index, that there is an existing `foo` in
> the index, and that there is no `foo` but there is a `bar` in the working tree,
> and then it would rename (add-and-remove, really, because of sorting)
> the index entry, without scanning the working tree contents.
>
> In other words, we skip reading the 3 terabyte file, or whatever.
Yup, that matches what I wrote. We do not rehash and we only write
the index just once.
> Anyway, comparing to `git rm --cached`:
>
>> I think the requested "feature" is not all that outrageous. It
>> would be a similar value as a morning-after correction measure for
>> "oops, I moved the file in the filesystem without telling Git".
>
> I agree, but I also don't see it as valuable enough to bother
> writing a proper implementation.
Yup, I was merely agreeing with your "convenient" comment. I do not
think it is such a high-priority item for any active developers
among us to drop something they are doing and writing it instead.
^ permalink raw reply
* [PATCH 3/3] pack-objects: support `--delta-islands` with `--path-walk`
From: Taylor Blau @ 2026-05-27 23:18 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1779923907.git.me@ttaylorr.com>
Since the inception of `--path-walk`, this option has had a documented
incompatibility with `--delta-islands`.
When discussing those original patches on the list, a message from
Stolee in [1] noted the following:
this could be remedied by [...] doing a separate walk to identify
islands using the normal method
In a related portion of the thread, Peff explains[2]:
The delta islands code already does its own tree walk to propagate
the bits down (it does rely on the base walk's show_commit() to
propagate through the commits).
Once each object has its island bitmaps, I think however you
choose to come up with delta candidates [...] you should be able
to use it. It's fundamentally just answering the question of "am
I allowed to delta between these two objects".
That is similar to what this patch does, and it turns out the cheaper
option is sufficient: perform the same island side effects from the
path-walk callback rather than doing a second walk.
Recall how delta-islands are computed during a normal repack:
- `show_commit()` calls `propagate_island_marks()` for each commit,
which merges the commit's island bitset onto its root tree object and
onto each of its parent commits.
- `show_object()` for a tree records the tree's depth derived from the
slash-separated pathname. Subsequent `resolve_tree_islands()` uses
that depth to walk trees in increasing-depth order, propagating each
tree's marks to its children.
- At delta-search time, `in_same_island()` enforces that a delta
target's island bitmap is a subset of its base's: every island that
reaches the target must also reach the base.
Path-walk's enumeration callback is `add_objects_by_path()`. It already
adds objects to `to_pack`, but until now did not perform the
island-related side effects. Two things are needed:
- For each commit batch, call `propagate_island_marks()` on commits,
exactly as `show_commit()` does.
We have to be careful about the order in which we call this function,
and we must see a commit before its parents in order to have
island marks to propagate.
The path-walk batch preserves that order. Path-walk appends commits
to its `OBJ_COMMIT` batch as they come back from the same
`get_revision()` loop the regular traversal uses, and
`add_objects_by_path()` iterates the batch in array order. So every
commit reaches `propagate_island_marks()` in the same sequence that
`show_commit()` would have seen it, and the descendant-first chain
that the algorithm relies on is intact.
Skip island propagation for excluded commits to match the regular
traversal, whose `show_commit()` callback is only invoked for
interesting commits. Boundary commits may still be present in
path-walk's callback so they can serve as thin-pack bases, but they
should not contribute island marks.
- For each tree batch, record the tree's depth from the path. Use the
`record_tree_depth()` helper from the previous commit so both
callbacks behave identically, including the max-depth-wins behavior
when a tree is reached via more than one path. The helper accepts
both the `show_object()` path shape ("foo", "foo/bar") and the
path-walk shape with a trailing slash ("foo/", "foo/bar/"), so depths
recorded from either traversal mode are directly comparable.
This is implicit in the implementation sketch from Peff above.
`resolve_tree_islands()` sorts trees by `oe->tree_depth` in
increasing-depth order before propagating marks down, so that a
parent tree's marks are finalized before its children inherit them.
Without recording the depth at path-walk time, every
path-walk-discovered tree would land at depth 0 in `to_pack`, the
sort would lose its ordering, and children could inherit marks from
parents whose own contributions had not yet been merged in.
With those two pieces in place, `resolve_tree_islands()` receives the
same island inputs from path-walk as it would from the regular
traversal, so the existing island checks can be reused unchanged.
Drop the documented incompatibility between `--path-walk` and
`--delta-islands`, and add t5320 coverage for path-walk island repacks
with and without bitmap writing, as well as the same-island case where a
delta remains allowed.
[1]: https://lore.kernel.org/git/9aa2471b-0850-4707-9733-d3b33609f5f2@gmail.com/
[2]: https://lore.kernel.org/git/20240911063203.GA1538586@coredump.intra.peff.net/
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-pack-objects.adoc | 14 +++++++-------
builtin/pack-objects.c | 22 ++++++++++++++++++----
t/t5320-delta-islands.sh | 29 +++++++++++++++++++++++++++++
3 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 0adce8961a3..65cd00c152f 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -402,13 +402,13 @@ will be automatically changed to version `1`.
of filenames that cause collisions in Git's default name-hash
algorithm.
+
-Incompatible with `--delta-islands`. When `--use-bitmap-index` is
-specified with `--path-walk`, a successful bitmap traversal is used for
-object enumeration, with path-walk remaining as the fallback traversal
-when the bitmap cannot satisfy the request. The `--path-walk` option
-supports the `--filter=<spec>` forms `blob:none`, `blob:limit=<n>`,
-`tree:0`, `object:type=<type>`, and `sparse:<oid>`. These supported filter
-types can be combined with the `combine:<spec>+<spec>` form.
+When `--use-bitmap-index` is specified with `--path-walk`, a successful
+bitmap traversal is used for object enumeration, with path-walk
+remaining as the fallback traversal when the bitmap cannot satisfy the
+request. The `--path-walk` option supports the `--filter=<spec>` forms
+`blob:none`, `blob:limit=<n>`, `tree:0`, `object:type=<type>`, and
+`sparse:<oid>`. These supported filter types can be combined with the
+`combine:<spec>+<spec>` form.
DELTA ISLANDS
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ec02e2b21d2..f48ea7a888b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4737,13 +4737,29 @@ static int add_objects_by_path(const char *path,
add_object_entry(oid, type, path, exclude);
- if (type == OBJ_COMMIT && write_bitmap_index) {
+ if (type == OBJ_COMMIT) {
struct commit *commit;
+ if (!write_bitmap_index && !use_delta_islands)
+ continue;
+
commit = lookup_commit(the_repository, oid);
if (!commit)
die(_("could not find commit %s"), oid_to_hex(oid));
- index_commit_for_bitmap(commit);
+ if (write_bitmap_index)
+ index_commit_for_bitmap(commit);
+ /*
+ * Skip island propagation for boundary commits.
+ * The regular traversal's show_commit() is only
+ * called for interesting commits; matching that
+ * here keeps path-walk from doing extra work that
+ * would only be a no-op anyway (boundary commits
+ * are not in island_marks).
+ */
+ if (use_delta_islands && !exclude)
+ propagate_island_marks(the_repository, commit);
+ } else if (type == OBJ_TREE && use_delta_islands) {
+ record_tree_depth(oid, path);
}
}
@@ -5205,8 +5221,6 @@ int cmd_pack_objects(int argc,
const char *option = NULL;
if (!path_walk_filter_compatible(&filter_options))
option = "--filter";
- else if (use_delta_islands)
- option = "--delta-islands";
if (option) {
warning(_("cannot use %s with %s"),
diff --git a/t/t5320-delta-islands.sh b/t/t5320-delta-islands.sh
index 2c961c70963..9b28344a0a3 100755
--- a/t/t5320-delta-islands.sh
+++ b/t/t5320-delta-islands.sh
@@ -53,6 +53,35 @@ test_expect_success 'separate islands disallows delta' '
! is_delta_base $two $one
'
+test_expect_success 'path-walk island repack respects islands' '
+ GIT_TRACE2_EVENT="$(pwd)/trace.path-walk-islands" \
+ git -c "pack.island=refs/heads/(.*)" repack -adfi \
+ --path-walk 2>err &&
+ test_region pack-objects path-walk trace.path-walk-islands &&
+ test_grep ! "cannot use --delta-islands with --path-walk" err &&
+ ! is_delta_base $one $two &&
+ ! is_delta_base $two $one
+'
+
+test_expect_success 'path-walk island bitmap repack respects islands' '
+ GIT_TRACE2_EVENT="$(pwd)/trace.path-walk-island-bitmap" \
+ git -c "pack.island=refs/heads/(.*)" repack -a -d -f -i -b \
+ --path-walk 2>err &&
+ test_region pack-objects path-walk trace.path-walk-island-bitmap &&
+ test_path_is_file .git/objects/pack/*.bitmap &&
+ git rev-list --test-bitmap --use-bitmap-index one &&
+ test_grep ! "cannot use --delta-islands with --path-walk" err &&
+ ! is_delta_base $one $two &&
+ ! is_delta_base $two $one
+'
+
+test_expect_success 'path-walk same island allows delta' '
+ GIT_TRACE2_EVENT="$(pwd)/trace.path-walk-same-island" \
+ git -c "pack.island=refs/heads" repack -adfi --path-walk &&
+ test_region pack-objects path-walk trace.path-walk-same-island &&
+ is_delta_base $one $two
+'
+
test_expect_success 'same island allows delta' '
git -c "pack.island=refs/heads" repack -adfi &&
is_delta_base $one $two
--
2.54.0.22.ga642305e3c9
^ permalink raw reply related
* [PATCH 2/3] pack-objects: extract `record_tree_depth()` helper
From: Taylor Blau @ 2026-05-27 23:18 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1779923907.git.me@ttaylorr.com>
Prepare for a subsequent change that needs to record tree depths from a
second call site by factoring the delta-islands tree-depth bookkeeping
out of `show_object()` and into a helper, `record_tree_depth()`.
The helper looks up the object in `to_pack`, returns early when the
object was not added there, computes the depth from the slash count in
the supplied name, and preserves the existing max-depth-wins behavior
when a tree is reached by more than one path.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/pack-objects.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e4dcb563b7d..ec02e2b21d2 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2722,6 +2722,22 @@ static inline void oe_set_tree_depth(struct packing_data *pack,
pack->tree_depth[e - pack->objects] = tree_depth;
}
+static void record_tree_depth(const struct object_id *oid, const char *name)
+{
+ const char *p;
+ unsigned depth;
+ struct object_entry *ent;
+
+ /* the empty string is a root tree, which is depth 0 */
+ depth = *name ? 1 : 0;
+ for (p = strchr(name, '/'); p; p = strchr(p + 1, '/'))
+ depth++;
+
+ ent = packlist_find(&to_pack, oid);
+ if (ent && depth > oe_tree_depth(&to_pack, ent))
+ oe_set_tree_depth(&to_pack, ent, depth);
+}
+
/*
* Return the size of the object without doing any delta
* reconstruction (so non-deltas are true object sizes, but deltas
@@ -4375,20 +4391,8 @@ static void show_object(struct object *obj, const char *name,
add_preferred_base_object(name);
add_object_entry(&obj->oid, obj->type, name, 0);
- if (use_delta_islands) {
- const char *p;
- unsigned depth;
- struct object_entry *ent;
-
- /* the empty string is a root tree, which is depth 0 */
- depth = *name ? 1 : 0;
- for (p = strchr(name, '/'); p; p = strchr(p + 1, '/'))
- depth++;
-
- ent = packlist_find(&to_pack, &obj->oid);
- if (ent && depth > oe_tree_depth(&to_pack, ent))
- oe_set_tree_depth(&to_pack, ent, depth);
- }
+ if (use_delta_islands)
+ record_tree_depth(&obj->oid, name);
}
static void show_object__ma_allow_any(struct object *obj, const char *name, void *data)
--
2.54.0.22.ga642305e3c9
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox