* [PATCH 4/5] blame: consult diff process for zero-hunk detection
From: Michael Montalbo via GitGitGadget @ 2026-05-22 2:11 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.git.1779415884.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
When a diff process is configured via diff.<driver>.process,
consult it during blame's per-commit diffing. If the process
returns zero hunks for a commit's changes to a file, treat the
commit as having no changes, causing blame to attribute lines
to earlier commits.
The subprocess is long-running (one startup cost amortized
across the blame traversal), but each commit in the file's
history incurs a round-trip to the tool.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/gitattributes.adoc | 3 +++
blame.c | 43 +++++++++++++++++++++++++++++---
t/t4080-diff-process.sh | 32 ++++++++++++++++++++++++
3 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index cc724f8c63..7d66fa3aa1 100644
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -857,6 +857,9 @@ The tool responds with lines of the form
If the tool returns zero hunks with `status=success`, Git treats
the file as having no changes and produces no diff output.
+`git blame` also consults the diff process and skips commits
+where it reports zero hunks, attributing lines to earlier commits
+instead.
Tools should ignore unknown keys in the per-file request to
remain forward-compatible.
diff --git a/blame.c b/blame.c
index a3c49d132e..8a5f14db7a 100644
--- a/blame.c
+++ b/blame.c
@@ -19,6 +19,8 @@
#include "tag.h"
#include "trace2.h"
#include "blame.h"
+#include "diff-process.h"
+#include "userdiff.h"
#include "alloc.h"
#include "commit-slab.h"
#include "bloom.h"
@@ -315,16 +317,47 @@ static struct commit *fake_working_tree_commit(struct repository *r,
static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
- xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts)
+ xdl_emit_hunk_consume_func_t hunk_func, void *cb_data,
+ int xdl_opts, struct index_state *istate,
+ const char *path)
{
xpparam_t xpp = {0};
xdemitconf_t xecfg = {0};
xdemitcb_t ecb = {NULL};
+ struct xdl_hunk *ext_hunks = NULL;
+ int ret;
xpp.flags = xdl_opts;
xecfg.hunk_func = hunk_func;
ecb.priv = cb_data;
- return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
+
+ if (path && istate) {
+ struct userdiff_driver *drv;
+ drv = userdiff_find_by_path(istate, path);
+ if (drv && drv->process) {
+ size_t nr = 0;
+ if (!diff_process_get_hunks(drv, path,
+ file_a->ptr, file_a->size,
+ file_b->ptr, file_b->size,
+ &ext_hunks, &nr)) {
+ if (!nr) {
+ /*
+ * Zero hunks: the diff process
+ * considers these files equivalent.
+ * Skip so blame looks past this
+ * commit.
+ */
+ return 0;
+ }
+ xpp.external_hunks = ext_hunks;
+ xpp.external_hunks_nr = nr;
+ }
+ }
+ }
+
+ ret = xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
+ free(ext_hunks);
+ return ret;
}
static const char *get_next_line(const char *start, const char *end)
@@ -1961,7 +1994,8 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
&sb->num_read_blob, ignore_diffs);
sb->num_get_patch++;
- if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts))
+ if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts,
+ sb->revs->diffopt.repo->index, target->path))
die("unable to generate diff (%s -> %s)",
oid_to_hex(&parent->commit->object.oid),
oid_to_hex(&target->commit->object.oid));
@@ -2114,7 +2148,8 @@ static void find_copy_in_blob(struct blame_scoreboard *sb,
* file_p partially may match that image.
*/
memset(split, 0, sizeof(struct blame_entry [3]));
- if (diff_hunks(file_p, &file_o, handle_split_cb, &d, sb->xdl_opts))
+ if (diff_hunks(file_p, &file_o, handle_split_cb, &d, sb->xdl_opts,
+ NULL, NULL))
die("unable to generate diff (%s)",
oid_to_hex(&parent->commit->object.oid));
/* remainder, if any, all match the preimage */
diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
index 6f49f4e66b..5ed644b786 100755
--- a/t/t4080-diff-process.sh
+++ b/t/t4080-diff-process.sh
@@ -335,4 +335,36 @@ test_expect_success PYTHON 'diff process zero hunks suppresses diff output' '
test_must_be_empty actual
'
+test_expect_success PYTHON 'blame skips commits with zero hunks from diff process' '
+ cat >blame.c <<-\EOF &&
+ int main(void)
+ {
+ return 0;
+ }
+ EOF
+ git add blame.c &&
+ git commit -m "add blame.c" &&
+
+ cat >blame.c <<-\EOF &&
+ int main(void)
+ {
+ return 0;
+ }
+ EOF
+ git add blame.c &&
+ git commit -m "reformat blame.c" &&
+ BLAME_COMMIT=$(git rev-parse --short HEAD) &&
+
+ # Without zero-hunk mode, blame attributes the change.
+ git blame blame.c >without &&
+ grep "$BLAME_COMMIT" without &&
+
+ # With zero-hunk mode, the process considers the files equivalent
+ # and blame skips the reformat commit.
+ git -c diff.cdiff.process="$BACKEND --mode=zero-hunk" \
+ blame blame.c >with &&
+ ! grep "$BLAME_COMMIT" with
+'
+
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 5/5] diff-process-normalize: add built-in whitespace normalizer
From: Michael Montalbo via GitGitGadget @ 2026-05-22 2:11 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2120.git.1779415884.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
Add git diff-process-normalize, a built-in diff process that
detects whitespace-only changes. It compares files line by line
using xdiff_compare_lines() with XDF_IGNORE_WHITESPACE (same
logic as "git diff -w"). If all lines match, it returns zero
hunks; otherwise it returns an error so git falls back to the
builtin diff algorithm.
[diff "cdiff"]
process = git diff-process-normalize
Update documentation to describe zero-hunk behavior for diff
and blame, and document the built-in normalize tool.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
Documentation/config/diff.adoc | 2 +
Documentation/gitattributes.adoc | 15 ++++
Makefile | 1 +
builtin.h | 1 +
builtin/diff-process-normalize.c | 143 +++++++++++++++++++++++++++++++
git.c | 1 +
t/t4080-diff-process.sh | 60 +++++++++++++
7 files changed, 223 insertions(+)
create mode 100644 builtin/diff-process-normalize.c
diff --git a/Documentation/config/diff.adoc b/Documentation/config/diff.adoc
index 4ab5f60df6..475736c6ed 100644
--- a/Documentation/config/diff.adoc
+++ b/Documentation/config/diff.adoc
@@ -224,6 +224,8 @@ endif::git-diff[]
hunks that are fed into Git's diff and blame pipelines.
If the tool returns zero hunks, the file is treated as
unchanged for both diff output and blame attribution.
+ Git provides `git diff-process-normalize` as a built-in
+ tool that detects whitespace-only changes.
See linkgit:gitattributes[5] for details.
`diff.indentHeuristic`::
diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc
index 7d66fa3aa1..3f1d7affd8 100644
--- a/Documentation/gitattributes.adoc
+++ b/Documentation/gitattributes.adoc
@@ -861,6 +861,21 @@ the file as having no changes and produces no diff output.
where it reports zero hunks, attributing lines to earlier commits
instead.
+Git ships with a built-in diff process, `git diff-process-normalize`,
+that detects whitespace-only changes. Files whose only differences
+are whitespace produce zero hunks; files with non-whitespace changes
+fall back to the builtin diff algorithm. To use it:
+
+----------------------------------------------------------------
+[diff "cdiff"]
+ process = git diff-process-normalize
+----------------------------------------------------------------
+
+This is useful after running a code formatter: `git diff` shows
+no output for files that only had whitespace changes,
+`git blame` skips whitespace-only commits automatically without
+requiring a `.git-blame-ignore-revs` file.
+
Tools should ignore unknown keys in the per-file request to
remain forward-compatible.
diff --git a/Makefile b/Makefile
index 22900368dd..01acfaf7b8 100644
--- a/Makefile
+++ b/Makefile
@@ -1409,6 +1409,7 @@ BUILTIN_OBJS += builtin/diagnose.o
BUILTIN_OBJS += builtin/diff-files.o
BUILTIN_OBJS += builtin/diff-index.o
BUILTIN_OBJS += builtin/diff-pairs.o
+BUILTIN_OBJS += builtin/diff-process-normalize.o
BUILTIN_OBJS += builtin/diff-tree.o
BUILTIN_OBJS += builtin/diff.o
BUILTIN_OBJS += builtin/difftool.o
diff --git a/builtin.h b/builtin.h
index 235c51f30e..c713a0417f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -178,6 +178,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix, struct repos
int cmd_diff_index(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_diff(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_diff_pairs(int argc, const char **argv, const char *prefix, struct repository *repo);
+int cmd_diff_process_normalize(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_diff_tree(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_difftool(int argc, const char **argv, const char *prefix, struct repository *repo);
int cmd_env__helper(int argc, const char **argv, const char *prefix, struct repository *repo);
diff --git a/builtin/diff-process-normalize.c b/builtin/diff-process-normalize.c
new file mode 100644
index 0000000000..1580f6b7d9
--- /dev/null
+++ b/builtin/diff-process-normalize.c
@@ -0,0 +1,143 @@
+/*
+ * Built-in diff process that returns zero hunks for files whose
+ * only differences are whitespace, and status=error otherwise.
+ * See diff-process.c for the protocol and gitattributes(5) for usage.
+ *
+ * Uses xdiff_compare_lines() with XDF_IGNORE_WHITESPACE to compare
+ * lines, giving the same whitespace handling as "git diff -w".
+ */
+
+#include "builtin.h"
+#include "pkt-line.h"
+#include "strbuf.h"
+#include "xdiff-interface.h"
+
+/*
+ * Read a single pkt-line. Returns 1 for data, 0 for flush, -1 for EOF.
+ */
+static int read_pkt(int fd, struct strbuf *line)
+{
+ int len;
+ char *data;
+
+ if (packet_read_line_gently(fd, &len, &data) < 0)
+ return -1;
+ if (!data || !len)
+ return 0; /* flush */
+ strbuf_reset(line);
+ strbuf_add(line, data, len);
+ strbuf_rtrim(line);
+ return 1;
+}
+
+/*
+ * Read packetized content until a flush packet.
+ */
+static int read_content(int fd, struct strbuf *out)
+{
+ strbuf_reset(out);
+ if (read_packetized_to_strbuf(fd, out, PACKET_READ_GENTLE_ON_EOF) < 0)
+ return -1;
+ return 0;
+}
+
+/*
+ * Compare two buffers line by line using xdiff_compare_lines() with
+ * XDF_IGNORE_WHITESPACE (same logic as "git diff -w").
+ * Returns 1 if all lines match, 0 otherwise.
+ */
+static int whitespace_equivalent(const char *a, long size_a,
+ const char *b, long size_b)
+{
+ const char *ea = a + size_a;
+ const char *eb = b + size_b;
+
+ while (a < ea && b < eb) {
+ const char *eol_a = memchr(a, '\n', ea - a);
+ const char *eol_b = memchr(b, '\n', eb - b);
+ long len_a = (eol_a ? eol_a : ea) - a;
+ long len_b = (eol_b ? eol_b : eb) - b;
+
+ if (!xdiff_compare_lines(a, len_a, b, len_b,
+ XDF_IGNORE_WHITESPACE))
+ return 0;
+
+ a += len_a + (eol_a ? 1 : 0);
+ b += len_b + (eol_b ? 1 : 0);
+ }
+
+ /* Both sides must be exhausted */
+ return a >= ea && b >= eb;
+}
+
+int cmd_diff_process_normalize(int argc UNUSED, const char **argv UNUSED,
+ const char *prefix UNUSED,
+ struct repository *repo UNUSED)
+{
+ struct strbuf line = STRBUF_INIT;
+ struct strbuf old_content = STRBUF_INIT;
+ struct strbuf new_content = STRBUF_INIT;
+ int ret;
+
+ /* Handshake: read client greeting */
+ ret = read_pkt(0, &line);
+ if (ret <= 0 || strcmp(line.buf, "git-diff-client"))
+ return 1;
+ ret = read_pkt(0, &line);
+ if (ret <= 0 || strcmp(line.buf, "version=1"))
+ return 1;
+ read_pkt(0, &line); /* flush */
+
+ /* Send server greeting */
+ packet_write_fmt(1, "git-diff-server\n");
+ packet_write_fmt(1, "version=1\n");
+ packet_flush(1);
+
+ /* Read client capabilities until flush */
+ while ((ret = read_pkt(0, &line)) > 0)
+ ; /* consume */
+
+ /* Send our capabilities */
+ packet_write_fmt(1, "capability=hunks\n");
+ packet_flush(1);
+
+ /* Main loop: process file pairs */
+ for (;;) {
+ int have_command = 0;
+
+ /* Read request headers until flush */
+ while ((ret = read_pkt(0, &line)) > 0) {
+ if (starts_with(line.buf, "command="))
+ have_command = 1;
+ }
+ if (ret < 0)
+ break; /* EOF: client closed connection */
+ if (!have_command)
+ break;
+
+ /* Read old file content */
+ if (read_content(0, &old_content) < 0)
+ break;
+ /* Read new file content */
+ if (read_content(0, &new_content) < 0)
+ break;
+
+ if (whitespace_equivalent(old_content.buf, old_content.len,
+ new_content.buf, new_content.len)) {
+ /* Whitespace-only differences */
+ packet_flush(1); /* zero hunks */
+ packet_write_fmt(1, "status=success\n");
+ packet_flush(1);
+ } else {
+ /* Non-whitespace differences: fall back */
+ packet_flush(1);
+ packet_write_fmt(1, "status=error\n");
+ packet_flush(1);
+ }
+ }
+
+ strbuf_release(&line);
+ strbuf_release(&old_content);
+ strbuf_release(&new_content);
+ return 0;
+}
diff --git a/git.c b/git.c
index 5a40eab8a2..6239240b02 100644
--- a/git.c
+++ b/git.c
@@ -568,6 +568,7 @@ static struct cmd_struct commands[] = {
{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
{ "diff-pairs", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },
+ { "diff-process-normalize", cmd_diff_process_normalize, NO_PARSEOPT },
{ "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },
{ "difftool", cmd_difftool, RUN_SETUP_GENTLY },
{ "fast-export", cmd_fast_export, RUN_SETUP },
diff --git a/t/t4080-diff-process.sh b/t/t4080-diff-process.sh
index 5ed644b786..a6fa1df456 100755
--- a/t/t4080-diff-process.sh
+++ b/t/t4080-diff-process.sh
@@ -366,5 +366,65 @@ test_expect_success PYTHON 'blame skips commits with zero hunks from diff proces
! grep "$BLAME_COMMIT" with
'
+NORMALIZE="git diff-process-normalize"
+
+test_expect_success 'diff-process-normalize setup' '
+ echo "*.c diff=cdiff" >.gitattributes &&
+ git add .gitattributes &&
+ test_commit normalize-base
+'
+
+test_expect_success 'diff-process-normalize suppresses whitespace-only changes' '
+ cat >ws.c <<-\EOF &&
+ int main(void)
+ {
+ return 0;
+ }
+ EOF
+ git add ws.c &&
+ git commit -m "add ws.c" &&
+
+ cat >ws.c <<-\EOF &&
+ int main(void)
+ {
+ return 0;
+ }
+ EOF
+
+ git -c diff.cdiff.process="$NORMALIZE" \
+ diff ws.c >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'diff-process-normalize falls back on non-whitespace changes' '
+ cat >ws.c <<-\EOF &&
+ int main(void)
+ {
+ return 0;
+ }
+
+ int added_function(void)
+ {
+ return 99;
+ }
+ EOF
+
+ git -c diff.cdiff.process="$NORMALIZE" \
+ diff ws.c >actual &&
+ grep "added_function" actual
+'
+
+test_expect_success 'diff-process-normalize falls back on mixed whitespace and real changes' '
+ cat >ws.c <<-\EOF &&
+ int main(void)
+ {
+ return 42;
+ }
+ EOF
+
+ git -c diff.cdiff.process="$NORMALIZE" \
+ diff ws.c >actual &&
+ grep "return 42" actual
+'
test_done
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v10 2/4] branch: add --prune-merged <branch>
From: Junio C Hamano @ 2026-05-22 2:51 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren
In-Reply-To: <718e28c7e0120a826385189213cccec1f0fce1af.1779403204.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1e24c95a69..29d38e9060 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
Due to the way the patch is split between 1/4 and 2/4, it is
impossible to comment on the change to delete_branches etc. that are
needed for this step. I'll use "git diff master... builtin/" instead.
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1572a4f9ef..b89fd56112 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -1,43 +1,47 @@
> ...
> @@ -132,78 +136,87 @@ static const char *branch_get_color(enum color_branch ix)
> static int branch_merged(int kind, const char *name,
> struct commit *rev, struct commit *head_rev)
> {
This one is called from check_branch_commit(). In "--prune-merged"
code paths, as we will see below, kind==FILTER_REFS_BRANCHES is
passed.
But note that there is no reason to expect "--prune-merged" will be
the only one to pass FILTER_REFS_BRANCHES to this function. Most
notably, "git branch -d" without "-r" would set FILTER_REFS_BRANCHES
to filter.kind and passes the value here.
> /*
> * This checks whether the merge bases of branch and HEAD (or
> * the other branch this branch builds upon) contains the
> * branch, which means that the branch has already been merged
> * safely to HEAD (or the other branch).
> */
> struct commit *reference_rev = NULL;
> const char *reference_name = NULL;
> void *reference_name_to_free = NULL;
> int merged;
>
> if (kind == FILTER_REFS_BRANCHES) {
> struct branch *branch = branch_get(name);
> const char *upstream = branch_get_upstream(branch, NULL);
> struct object_id oid;
>
> if (upstream &&
> (reference_name = reference_name_to_free =
> refs_resolve_refdup(get_main_ref_store(the_repository), upstream, RESOLVE_REF_READING,
> &oid, NULL)) != NULL)
> reference_rev = lookup_commit_reference(the_repository,
> &oid);
> }
We find its upstream in reference_rev; if this is non-NULL, the
branch MUST be merged to that revision for it to be safely removed.
> if (!reference_rev)
> reference_rev = head_rev;
But when ehad_rev is given, and if there is no upstream, we check if
the branch is merged to whatever happens to be checked out instead.
For the purpose of "--prune-merged", therefore, we MUST pass NULL in
head_rev when we call this function. We'll see what is done in the
caller later.
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1e24c95a69..29d38e9060 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -172,8 +174,8 @@ static int branch_merged(int kind, const char *name,
> * any of the following code, but during the transition period,
> * a gentle reminder is in order.
> */
> - 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);
Any caller who passed head_rev==NULL still used to come into this
block, set expect to 0, and have it compared with merged. If merged
is 0 (which is what happens when reference_rev is NULL and head_rev
is NULL), the control left this block silently due to /* okay */
below (which is in the post-context that is not shown).
The updated code refuses control to come into this block when
head_rev is NULL. I am not sure why the change in this hunk is
needed.
> @@ -748,6 +750,25 @@ static int collect_forked_branch(const struct reference *ref, void *cb_data)
> return 0;
> }
>
> +static int collect_default_branch_name(struct remote *remote, void *cb_data)
> +{
> + struct string_list *protected = cb_data;
> + struct ref_store *refs = get_main_ref_store(the_repository);
> + struct strbuf head = STRBUF_INIT;
> + const char *target;
> +
> + strbuf_addf(&head, "refs/remotes/%s/HEAD", remote->name);
> + target = refs_resolve_ref_unsafe(refs, head.buf,
> + RESOLVE_REF_NO_RECURSE, NULL, NULL);
> + if (target) {
> + const char *leaf = strrchr(target, '/');
> + if (leaf)
> + string_list_insert(protected, leaf + 1);
> + }
> + strbuf_release(&head);
> + return 0;
> +}
It is strange to assume that whatever upstream repository happens to
use as its primary branch name has anything to do with how the local
repository names its primary branch. Shouldn't this instead use
init.defaultBranch configuration or something?
> @@ -781,6 +802,63 @@ static int list_forked_branches(int argc, const char **argv)
> return 0;
> }
>
> +static int prune_merged_branches(int argc, const char **argv, int quiet)
> +{
> + struct ref_store *refs = get_main_ref_store(the_repository);
> + struct string_list candidates = STRING_LIST_INIT_DUP;
> + struct string_list protected_default_names = STRING_LIST_INIT_DUP;
> + struct strvec deletable = STRVEC_INIT;
> + struct strbuf buf = STRBUF_INIT;
> + struct string_list_item *item;
> + int n_not_merged = 0;
> + int ret = 0;
> +
> + if (!argc)
> + die(_("--prune-merged requires at least one <branch>"));
> +
> + collect_forked_set(argc, argv, &candidates);
Due to poor separation of changes, all the necessary information to
assess how sane the above code is is hidden in [1/4] and not here.
> + for_each_remote(collect_default_branch_name, &protected_default_names);
This looks inefficient, and worse, incorrect.
If we have multiple branches in argv[], they may have come from
different upstream, and because you pay attention to what the
refs/remotes/<upstream>/HEAD symrefs point at, you have multiple
such "protected" default names. You'd compare each and every
candidates against these names using linear search in the
string_list to see if they are protected (inefficient), and you
reject removal of argv[1] even when it happens to be the same name
as the default in the repository from which the upstream of argv[3]
came from, i.e., has no relationship with argv[1] (incorrect).
> + for_each_string_list_item(item, &candidates) {
> + const char *short_name = item->string;
> + const char *upstream = item->util;
> +
> + strbuf_reset(&buf);
> + strbuf_addf(&buf, "refs/heads/%s", short_name);
> + if (branch_checked_out(buf.buf))
> + continue;
> +
> + if (string_list_has_string(&protected_default_names,
> + short_name))
> + continue;
> +
> + if (!refs_ref_exists(refs, upstream))
> + continue;
> +
> + strvec_push(&deletable, short_name);
> + }
> + strbuf_release(&buf);
> +
> + if (deletable.nr)
> + ret = delete_branches(deletable.nr, deletable.v,
> + 0, FILTER_REFS_BRANCHES, quiet,
> + 1, &n_not_merged);
Add comments on parameters, perhaps, to make it readable?
ret = delete_branches(deletable.nr, deletable.v,
0, /* no force */
FILTER_REFS_BRANCHES,
quiet,
1, /* warn only */
&n_not_merged);
or something?
Unfortunately the body of delete_branches() updated by this series
is not visible in this patch, making a sensible review impossible,
but if I recall correctly from what I saw in [1/4], with no-force
set, it does not leave head_rev NULL, and instead reads the HEAD
into it, and that is eventually passed to check_branch_commit()
call.
Now, check_branch_commit() passes head_rev to branch_merged(), which
falls back to "HEAD" when head_rev is given. That sounds incorrect
for at least two reasons. We are only dealing with branches that do
have upstream, so fallback should not trigger and we shouldn't be
allowing head_rev to be passed. It _might_ be debatable that it is
prudent to still expect branch_merged() not to find the upstream to
detect errors in the logic, but falling back to head_rev in such a
case does not make any sense.
> + if (n_not_merged && !quiet)
> + fprintf(stderr,
> + Q_("Skipped %d branch that is not fully merged; "
> + "delete it with 'git branch -D' if you are sure.\n",
> + "Skipped %d branches that are not fully merged; "
> + "delete them with 'git branch -D' if you are sure.\n",
> + n_not_merged),
> + n_not_merged);
I do not think we unconditionally want to see this, and "--quiet"
shouldn't be the onlyl way to squelch this message.
When !quiet, the warn_only call to check_branch_commit() would
already have reported which branches are not fully merged, and
after seeing this message a few times, even the most novice user
would know how to use "git branch -D" to remove unneeded branches.
Use of advice_if_enabled() may make it more palatable.
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 45455cb8ce..c8589cd3a6 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1798,4 +1798,143 @@ test_expect_success '--forked requires at least one <branch>' '
> test_grep "at least one <branch>" err
> '
>
> +test_expect_success '--prune-merged: setup' '
> + test_create_repo pm-upstream &&
> + test_commit -C pm-upstream base &&
> + git -C pm-upstream checkout -b next &&
> + test_commit -C pm-upstream one-commit &&
> + test_commit -C pm-upstream two-commit &&
> + git -C pm-upstream branch one HEAD~ &&
> + git -C pm-upstream branch two HEAD &&
> + git -C pm-upstream branch wip main &&
> + git -C pm-upstream checkout main
> +'
> +
> +test_expect_success '--prune-merged deletes branches integrated into upstream' '
> + test_when_finished "rm -rf pm-merged" &&
> + git clone pm-upstream pm-merged &&
> + git -C pm-merged branch one one-commit &&
> + git -C pm-merged branch --set-upstream-to=origin/next one &&
> + git -C pm-merged branch two two-commit &&
> + git -C pm-merged branch --set-upstream-to=origin/next two &&
> +
> + git -C pm-merged branch --prune-merged "origin/*" &&
> +
> + test_must_fail git -C pm-merged rev-parse --verify refs/heads/one &&
> + test_must_fail git -C pm-merged rev-parse --verify refs/heads/two
> +'
> +
> +test_expect_success '--prune-merged with a literal upstream argument' '
> + test_when_finished "rm -rf pm-literal" &&
> + git clone pm-upstream pm-literal &&
> + git -C pm-literal branch one one-commit &&
> + git -C pm-literal branch --set-upstream-to=origin/next one &&
> + git -C pm-literal branch keepme one-commit &&
> + git -C pm-literal branch --set-upstream-to=origin/main keepme &&
> +
> + git -C pm-literal branch --prune-merged origin/next &&
> +
> + test_must_fail git -C pm-literal rev-parse --verify refs/heads/one &&
> + git -C pm-literal rev-parse --verify refs/heads/keepme
> +'
> +
> +test_expect_success '--prune-merged unions multiple <branch> arguments' '
> + test_when_finished "rm -rf pm-union" &&
> + git clone pm-upstream pm-union &&
> + git -C pm-union branch one one-commit &&
> + git -C pm-union branch --set-upstream-to=origin/next one &&
> + git -C pm-union branch two base &&
> + git -C pm-union branch --set-upstream-to=origin/main two &&
> +
> + git -C pm-union branch --prune-merged origin/next origin/main &&
> +
> + test_must_fail git -C pm-union rev-parse --verify refs/heads/one &&
> + test_must_fail git -C pm-union rev-parse --verify refs/heads/two
> +'
> +
> +test_expect_success '--prune-merged with a local-branch argument' '
> + test_create_repo pm-local &&
> + test_when_finished "rm -rf pm-local" &&
> + test_commit -C pm-local base &&
> + git -C pm-local branch topic base &&
> + git -C pm-local config branch.topic.remote . &&
> + git -C pm-local config branch.topic.merge refs/heads/main &&
> + git -C pm-local checkout --detach &&
> +
> + git -C pm-local branch --prune-merged main &&
> +
> + test_must_fail git -C pm-local rev-parse --verify refs/heads/topic &&
> + git -C pm-local rev-parse --verify refs/heads/main
> +'
> +
> +test_expect_success '--prune-merged spares branches with un-integrated commits' '
> + test_when_finished "rm -rf pm-unmerged" &&
> + git clone pm-upstream pm-unmerged &&
> + git -C pm-unmerged checkout -b wip origin/wip &&
> + git -C pm-unmerged branch --set-upstream-to=origin/next wip &&
> + test_commit -C pm-unmerged local-only &&
> + git -C pm-unmerged checkout - &&
> +
> + git -C pm-unmerged branch --prune-merged "origin/*" 2>err &&
> + test_grep "not fully merged" err &&
> + test_grep "Skipped 1 branch" err &&
> + test_grep "git branch -D" err &&
> + test_grep ! "If you are sure you want to delete it" err &&
> + git -C pm-unmerged rev-parse --verify refs/heads/wip
> +'
> +
> +test_expect_success '--prune-merged skips branches whose upstream is gone' '
> + test_when_finished "rm -rf pm-upstream-gone" &&
> + git clone pm-upstream pm-upstream-gone &&
> + git -C pm-upstream-gone branch one one-commit &&
> + git -C pm-upstream-gone branch --set-upstream-to=origin/next one &&
> +
> + git -C pm-upstream-gone update-ref -d refs/remotes/origin/next &&
> + git -C pm-upstream-gone branch --prune-merged "origin/*" &&
> +
> + git -C pm-upstream-gone rev-parse --verify refs/heads/one
> +'
> +
> +test_expect_success '--prune-merged never deletes the checked-out branch' '
> + test_when_finished "rm -rf pm-head" &&
> + git clone pm-upstream pm-head &&
> + git -C pm-head checkout -b one one-commit &&
> + git -C pm-head branch --set-upstream-to=origin/next one &&
> +
> + git -C pm-head branch --prune-merged "origin/*" &&
> +
> + git -C pm-head rev-parse --verify refs/heads/one
> +'
> +
> +test_expect_success '--prune-merged spares the local default branch' '
> + test_when_finished "rm -rf pm-default" &&
> + git clone pm-upstream pm-default &&
> + git -C pm-default checkout --detach &&
> + git -C pm-default branch --prune-merged "origin/*" &&
> + git -C pm-default rev-parse --verify refs/heads/main
> +'
> +
> +test_expect_success '--prune-merged protects the default branch by name only' '
> + test_when_finished "rm -rf pm-default-alias" &&
> + git clone pm-upstream pm-default-alias &&
> + git -C pm-default-alias branch --track trunk origin/main &&
> + git -C pm-default-alias checkout --detach &&
> + git -C pm-default-alias branch --prune-merged "origin/*" &&
> + git -C pm-default-alias rev-parse --verify refs/heads/main &&
> + test_must_fail git -C pm-default-alias rev-parse --verify refs/heads/trunk
> +'
> +
> +test_expect_success '--prune-merged with literal arg also protects default-name' '
> + test_when_finished "rm -rf pm-literal-default" &&
> + git clone pm-upstream pm-literal-default &&
> + git -C pm-literal-default checkout --detach &&
> + git -C pm-literal-default branch --prune-merged origin/main &&
> + git -C pm-literal-default rev-parse --verify refs/heads/main
> +'
> +
> +test_expect_success '--prune-merged requires at least one <branch>' '
> + test_must_fail git -C pm-upstream branch --prune-merged 2>err &&
> + test_grep "at least one <branch>" err
> +'
> +
> test_done
^ permalink raw reply
* Re: [PATCH v10 2/4] branch: add --prune-merged <branch>
From: Junio C Hamano @ 2026-05-22 2:52 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren
In-Reply-To: <718e28c7e0120a826385189213cccec1f0fce1af.1779403204.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 1e24c95a69..29d38e9060 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -172,8 +174,8 @@ static int branch_merged(int kind, const char *name,
> * any of the following code, but during the transition period,
> * a gentle reminder is in order.
> */
> - 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);
Any caller who passed head_rev==NULL still used to come into this
block, set expect to 0, and have it compared with merged. If merged
is 0 (which is what happens when reference_rev is NULL and head_rev
is NULL), the control left this block silently due to /* okay */
below (which is in the post-context that is not shown).
The updated code refuses control to come into this block when
head_rev is NULL. I am not sure why the change in this hunk is
needed.
> @@ -748,6 +750,25 @@ static int collect_forked_branch(const struct reference *ref, void *cb_data)
> return 0;
> }
>
> +static int collect_default_branch_name(struct remote *remote, void *cb_data)
> +{
> + struct string_list *protected = cb_data;
> + struct ref_store *refs = get_main_ref_store(the_repository);
> + struct strbuf head = STRBUF_INIT;
> + const char *target;
> +
> + strbuf_addf(&head, "refs/remotes/%s/HEAD", remote->name);
> + target = refs_resolve_ref_unsafe(refs, head.buf,
> + RESOLVE_REF_NO_RECURSE, NULL, NULL);
> + if (target) {
> + const char *leaf = strrchr(target, '/');
> + if (leaf)
> + string_list_insert(protected, leaf + 1);
> + }
> + strbuf_release(&head);
> + return 0;
> +}
It is strange to assume that whatever upstream repository happens to
use as its primary branch name has anything to do with how the local
repository names its primary branch. Shouldn't this instead use
init.defaultBranch configuration or something?
> @@ -781,6 +802,63 @@ static int list_forked_branches(int argc, const char **argv)
> return 0;
> }
>
> +static int prune_merged_branches(int argc, const char **argv, int quiet)
> +{
> + struct ref_store *refs = get_main_ref_store(the_repository);
> + struct string_list candidates = STRING_LIST_INIT_DUP;
> + struct string_list protected_default_names = STRING_LIST_INIT_DUP;
> + struct strvec deletable = STRVEC_INIT;
> + struct strbuf buf = STRBUF_INIT;
> + struct string_list_item *item;
> + int n_not_merged = 0;
> + int ret = 0;
> +
> + if (!argc)
> + die(_("--prune-merged requires at least one <branch>"));
> +
> + collect_forked_set(argc, argv, &candidates);
Due to poor separation of changes, all the necessary information to
assess how sane the above code is is hidden in [1/4] and not here.
> + for_each_remote(collect_default_branch_name, &protected_default_names);
This looks inefficient, and worse, incorrect.
If we have multiple branches in argv[], they may have come from
different upstream, and because you pay attention to what the
refs/remotes/<upstream>/HEAD symrefs point at, you have multiple
such "protected" default names. You'd compare each and every
candidates against these names using linear search in the
string_list to see if they are protected (inefficient), and you
reject removal of argv[1] even when it happens to be the same name
as the default in the repository from which the upstream of argv[3]
came from, i.e., has no relationship with argv[1] (incorrect).
> + for_each_string_list_item(item, &candidates) {
> + const char *short_name = item->string;
> + const char *upstream = item->util;
> +
> + strbuf_reset(&buf);
> + strbuf_addf(&buf, "refs/heads/%s", short_name);
> + if (branch_checked_out(buf.buf))
> + continue;
> +
> + if (string_list_has_string(&protected_default_names,
> + short_name))
> + continue;
> +
> + if (!refs_ref_exists(refs, upstream))
> + continue;
> +
> + strvec_push(&deletable, short_name);
> + }
> + strbuf_release(&buf);
> +
> + if (deletable.nr)
> + ret = delete_branches(deletable.nr, deletable.v,
> + 0, FILTER_REFS_BRANCHES, quiet,
> + 1, &n_not_merged);
Add comments on parameters, perhaps, to make it readable?
ret = delete_branches(deletable.nr, deletable.v,
0, /* no force */
FILTER_REFS_BRANCHES,
quiet,
1, /* warn only */
&n_not_merged);
or something?
Unfortunately the body of delete_branches() updated by this series
is not visible in this patch, making a sensible review impossible,
but if I recall correctly from what I saw in [1/4], with no-force
set, it does not leave head_rev NULL, and instead reads the HEAD
into it, and that is eventually passed to check_branch_commit()
call.
Now, check_branch_commit() passes head_rev to branch_merged(), which
falls back to "HEAD" when head_rev is given. That sounds incorrect
for at least two reasons. We are only dealing with branches that do
have upstream, so fallback should not trigger and we shouldn't be
allowing head_rev to be passed. It _might_ be debatable that it is
prudent to still expect branch_merged() not to find the upstream to
detect errors in the logic, but falling back to head_rev in such a
case does not make any sense.
> + if (n_not_merged && !quiet)
> + fprintf(stderr,
> + Q_("Skipped %d branch that is not fully merged; "
> + "delete it with 'git branch -D' if you are sure.\n",
> + "Skipped %d branches that are not fully merged; "
> + "delete them with 'git branch -D' if you are sure.\n",
> + n_not_merged),
> + n_not_merged);
I do not think we unconditionally want to see this, and "--quiet"
shouldn't be the onlyl way to squelch this message.
When !quiet, the warn_only call to check_branch_commit() would
already have reported which branches are not fully merged, and
after seeing this message a few times, even the most novice user
would know how to use "git branch -D" to remove unneeded branches.
Use of advice_if_enabled() may make it more palatable.
^ permalink raw reply
* Re: [PATCH v10 2/4] branch: add --prune-merged <branch>
From: Junio C Hamano @ 2026-05-22 2:53 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget
Cc: git, Kristoffer Haugsbakk, Johannes Sixt, Phillip Wood,
Harald Nordgren
In-Reply-To: <xmqq8q9cw40a.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>
>
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 1e24c95a69..29d38e9060 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>
> Due to the way the patch is split between 1/4 and 2/4, it is
> impossible to comment on the change to delete_branches etc. that are
> needed for this step. I'll use "git diff master... builtin/" instead.
>
Please discard this version. I had unnecessary draft comments that
I used as reference in it.
^ permalink raw reply
* Re: [PATCH] connect: use "service" enum for "name" argument
From: Jeff King @ 2026-05-22 4:43 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Junio C Hamano, git
In-Reply-To: <ag7AJMbav6KgSCjj@pks.im>
On Thu, May 21, 2026 at 10:19:48AM +0200, Patrick Steinhardt wrote:
> > + switch (service) {
> > + case GIT_CONNECT_UPLOAD_PACK:
> > + return "git-upload-pack";
> > + case GIT_CONNECT_RECEIVE_PACK:
> > + return "git-receive-pack";
> > + case GIT_CONNECT_UPLOAD_ARCHIVE:
> > + return "git-upload-archive";
> > + }
> > + BUG("unknown git_connect_type: %d", service);
> > +}
>
> Shouldn't this say "unknown git_connect_service" instead of "_type"?
Oops, yes. As you probably guessed, I started with "type" before
realizing that "service" was a better word.
The patch is in next, so the fixup on top (of jk/connect-service-enum)
is below.
-- >8 --
Subject: [PATCH] transport-helper: fix typo in BUG() message
We mistakenly refer to the git_connect_service enum as "_type" rather
than "_service". Users should never see this message in practice, but it
is slightly confusing when reading the code.
Reported-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Jeff King <peff@peff.net>
---
transport-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/transport-helper.c b/transport-helper.c
index bf37c5280c..b672801ae4 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -630,7 +630,7 @@ static const char *connect_service_cmd(enum git_connect_service service)
case GIT_CONNECT_UPLOAD_ARCHIVE:
return "git-upload-archive";
}
- BUG("unknown git_connect_type: %d", service);
+ BUG("unknown git_connect_service: %d", service);
}
static int process_connect_service(struct transport *transport,
--
2.54.0.618.gdbb63b8024
^ permalink raw reply related
* Re: [PATCH] http: handle absolute-path alternates from server root
From: Jeff King @ 2026-05-22 4:55 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Junio C Hamano, git, slonkazoid
In-Reply-To: <ag7xbkTF11N22waX@pks.im>
On Thu, May 21, 2026 at 01:50:06PM +0200, Patrick Steinhardt wrote:
> > Packfile URIs help with the actual pack generation (even if we're
> > blitting out bits from the disk with verbatim packfile reuse, we still
> > have to handle gaps and compute the checksum over the output pack).
> >
> > But it doesn't help with the server computing the set of objects the
> > client needs in the first place. IIRC, packfile URIs work by the server
> > saying "oh, I was going to send you object XYZ, but you can get it from
> > this stable pack instead". So the server still has to compute the set of
> > objects (and send any that are not mentioned in URI packs). Bitmaps
> > help, but there's still non-trivial computation and storage on the
> > server.
>
> I guess it depends on the actual server-side implementation, but in the
> general case this is of course true. A server could decide to for
> example overserve objects in case the client does a full clone, or it
> could arrange packfiles in a special way that allows it to serve at
> least some kinds of requests efficiently.
True, though you still have to receive the client wants/haves before
getting to the packfile-uri phase. The alternative is for the server
send URIs during the ref advertisement. But we have that, too, these
days: the bundle-uri feature. (Which I completely forgot about while
writing my earlier email).
So I do think that bundle-uris can probably be an adequate substitute
for dumb-http in terms of reducing server load. Though...
> Packfile URIs definitely need some love to become feasible, yes, and I
> don't think they have evolved much since their introduction. I still
> feel like they are the better mechanism for offloading traffic compared
> to bundle URIs though, as we already have packfiles around anyway.
...yeah, I agree that storing both bundles and packs can be annoying for
a server, depending on your setup. In theory it would not be hard for a
slightly-clever server endpoint to store packfiles for regular Git to
use, and then generate the bundles on the fly by cat-ing the bundle
header and the packfile, both of which can be sent out as raw bytes
without further processing.
Anyway, we are far afield from the patch that started this thread. ;) I
do agree with the general notion that we _should_ be able to get
smart-http close to the server-side expense of dumb-http with a few
tricks like these.
-Peff
^ permalink raw reply
* Re: [PATCH 4/9] run-command: add support for timeout in command finisher
From: Jeff King @ 2026-05-22 5:10 UTC (permalink / raw)
To: Johannes Sixt
Cc: Siddh Raman Pant, git@vger.kernel.org, gitster@pobox.com,
newren@gmail.com, ps@pks.im, oswald.buddenhagen@gmx.de,
code@khaugsbakk.name
In-Reply-To: <cf52154c-1275-4a4b-957e-5aa17f22705c@kdbg.org>
On Thu, May 21, 2026 at 04:36:05PM +0200, Johannes Sixt wrote:
> Am 21.05.26 um 11:59 schrieb Siddh Raman Pant:
> > The timeout is for the failure path, where the external helper has
> > already stopped following that protocol or is blocked on something
> > outside git's control. Since git starts the helper and puts it on the
> > log/grep path, git also needs a bounded way to recover when that helper
> > does not make progress. Otherwise an optional note source can prevent
> > the main git command from completing.
>
> That Git communicates with a process that looks like it stopped is the
> normal case, for example:
>
> - Output is sent to the pager. The user can take their time to study the
> output. All the while, git waits patiently for the user to advance the
> pager.
>
> - Git fetch transfers large amounts of data across the network. Most of
> the time it waits for data to arrive and does nothing. The peer process
> looks like it hangs. Git does not decide to kill the connection at any
> time. It is the user's decision to do so.
>
> If the notes provider hangs, then it is not on Git to decide when it has
> waited long enough.
Yeah, I agree with your point of view. If I understand this patch series
correctly, it is about adding an external process to map commit ids to
note data. So I can think of some existing features that are quite close
to that in nature, none of which use timeouts:
- textconv filters and external diffs which process data in the middle
of a git-log invocation
- long-lived clean/smudge filters map blobs to arbitrarily large text
- cat-file's batch mode maps object ids to user-specified data about
that object
As you note, it's up to the command to be well-behaved. Git should
notice and respond appropriately if the command closes the pipe, of
course. Sometimes a timeout can help with a poorly behaved command, but
IMHO it is not worth the cost of non-determinism that it brings.
Moreover, the bits touching run-command here make me nervous, especially
after the challenges we saw in the child-cleanup topic that was reverted
just after v2.54. There is often a shell interposed between Git and the
sub-command, and we don't always know how the shell will react to
signals. Using SIGKILL will eventually get us _something_ to wait() on,
but it might not even be the process we care about!
I don't really care much about this external-notes feature one way or
the other, but if we are going to do it, I don't see any reason why it
would not behave like all of the other similar parts of Git.
-Peff
^ permalink raw reply
* Re: [PATCH v3] git-jump: pick a mode automatically when invoked without arguments
From: Jeff King @ 2026-05-22 5:28 UTC (permalink / raw)
To: Greg Hurrell via GitGitGadget
Cc: git, Greg Hurrell, Erik Cervin Edin, Junio C Hamano, Greg Hurrell
In-Reply-To: <pull.2108.v3.git.1779371110195.gitgitgadget@gmail.com>
On Thu, May 21, 2026 at 01:45:09PM +0000, Greg Hurrell via GitGitGadget wrote:
> Changes since v2; all of these in response to feedback from Junio:
>
> * Removed stray # from README.
> * Don't both teaching "auto" to select "ws" mode, because it is always
> subsumed by "diff".
> * Update usage string to make clear that git jump --stdout foo is not a
> synonym for git jump --stdout auto foo, because distinguishing
> between foo as <mode> and foo as <arg> is fraught with ambiguity.
>
> In answer to Junio's question:
>
> > If more than one interesting cases apply, what happens, and what
> > should happen?
>
> it's an ordered choice (merge > diff).
Dropping the "ws" mode from auto makes sense to me. It could be slotted
in between "merge" and "diff" (a whitespace problem always implies a
diff, but a diff does not always imply a whitespace problem). But would
that actually be useful?
My impression of the "auto" feature is: I am too lazy to type, so just
take me to the interesting bits. And interesting in my experience with
git-jump is either "I am merging, take me to the conflict" or "I am
writing new code, take me to what I already did". Limiting the second
case just to whitespace violations (assuming there is at least one)
would probably be more confusing than helpful.
You could perhaps argue for one more layer of "interesting", which is:
if there are no unstaged changes, take me to the staged ones. Looking at
staged changes can be misleading if the working tree file has moved on
(since by definition we are dumping the editor into the working tree
file, not the staged contents). But if we know the diff between index
and working tree is empty, then that is not an issue.
I do sometimes use "git jump diff --cached" explicitly for that purpose.
If sounds like Greg has been living with "auto" and finding it useful
for a while. So I'm mostly inclined to take the patch as-is, and people
can experiment with it and suggest changes after using it in practice.
But here's my one final thought. The hierarchy of "merge > diff > diff
--cached", etc, makes me wonder if we could simply concatenate the
outputs. I.e., show all of the merge conflicts and all of the diffs,
whitespace changes, and so on. But I guess you run into two issues:
1. There will be duplicates. Every merge conflict is also a diff. And
every whitespace violation is also a diff. So at the very least you
need to de-dup these (which might not be entirely trivial, as they
may overlap rather than starting at the same line).
2. When resolving a merge, I'm not sure if the diffs are actually
interesting. Usually I git-jump around until there are no more
conflicts, and then I see if it builds. But I almost never want to
jump to the other changes introduced by the side-branch. There are
too many of them, and they are probably uninteresting. So adding
both "merge" and "diff" elements to the quicklist would be
annoying.
-Peff
^ permalink raw reply
* Re: [PATCH 0/5] [RFC] diff: add diff.<driver>.process for external hunk providers
From: Junio C Hamano @ 2026-05-22 5:29 UTC (permalink / raw)
To: Michael Montalbo via GitGitGadget; +Cc: git, Michael Montalbo
In-Reply-To: <pull.2120.git.1779415884.gitgitgadget@gmail.com>
"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> This series adds diff.<driver>.process, a long-running subprocess protocol
> that lets external tools provide hunks to git's diff and blame pipelines.
>
> Over the past 18 years, git's diff pipeline accumulated many features that
> operate on hunks: word diff, function context, color-moved, indent
> heuristic, blame. External tools can replace the pipeline entirely
> (diff.<driver>.command) or select among builtin algorithms
> (diff.<driver>.algorithm), but there is no way for a tool to provide
> line-change information into the pipeline. Tools that understand code
> structure (tree-sitter parsers, format-aware analyzers, tools like
> Difftastic and Mergiraf) must bypass git's pipeline and lose access to
> everything downstream.
>
> The protocol follows filter.<driver>.process: pkt-line over stdin/stdout,
> capability negotiation, one tool invocation per git command. The tool
> receives file pairs and returns hunk descriptors that git feeds into the
> standard xdiff pipeline. All output features work normally.
>
> Zero hunks with status=success means the tool considers the files
> equivalent. git diff shows no output for the file, and git blame skips the
> commit, attributing lines to earlier commits.
>
> On error or tool crash, git falls back silently to the builtin diff
> algorithm. The feature is opt-in via diff.<driver>.process and
> .gitattributes; unconfigured files are unaffected.
>
> The series includes git diff-process-normalize, a built-in tool that
> compares files line by line ignoring whitespace (same logic as "git diff -w"
> via xdiff_compare_lines):
Interesting.
If the goal is purely to normalize content before comparison
(e.g. stripping comments or canonicalizing formatting), we already
have the `textconv` mechanism. While `textconv` is a "one-shot"
per-file process, it is significantly simpler.
I suspect, however, that the primary focus here is to allow external
tools to provide structural alignment (e.g. for AST- aware diffs
like Difftastic or Mergiraf) without losing the original content in
the display. Unlike `textconv`, which transforms the text the user
sees, this protocol lets the display remain identical to the source
while using a custom engine for the line-matching logic.
If that is the intent, it should be stated more explicitly in the
documentation and commit messages. The "whitespace-normalize"
demonstration in [PATCH 5/5] is misleading because it's exactly the
case where `textconv` would be sufficient.
I am afraid that the use of a long-running subprocess for every
diff/blame invocation adds significant complexity and overhead. In
particular, wouldn't the `blame` implementation performs a
round-trip to the subprocess for every commit in the history? Even
with a persistent process, the overhead of serializing and
deserializing the entire file content twice (old and new) for every
commit could be prohibitive for large files or deep histories.
So, I dunno.
^ permalink raw reply
* Re: [PATCH 1/5] xdiff: support external hunks via xpparam_t
From: Junio C Hamano @ 2026-05-22 5:29 UTC (permalink / raw)
To: Michael Montalbo via GitGitGadget; +Cc: git, Michael Montalbo
In-Reply-To: <8c0ea0bc0742651e634db7a3002e8cbe1240acf9.1779415884.git.gitgitgadget@gmail.com>
"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +/*
> + * Populate the changed[] arrays from externally supplied hunks,
> + * bypassing the diff algorithm. Validates that hunks are in order,
> + * non-overlapping, and within bounds.
> + *
> + * Returns 0 on success, -1 on validation failure.
> + */
> +static int xdl_populate_hunks_from_external(xdfenv_t *xe,
> + const struct xdl_hunk *hunks,
> + size_t nr_hunks)
> +{
> + size_t i;
> + long j, prev_old_end = 0, prev_new_end = 0;
> + long total_old = 0, total_new = 0;
> +
> + /*
> + * Clear changed[] arrays. xdl_prepare_env() may have dirtied
> + * them via xdl_cleanup_records(). The allocation is nrec + 2
> + * elements; changed points one past the start (see xprepare.c).
> + */
> + memset(xe->xdf1.changed - 1, 0,
> + (xe->xdf1.nrec + 2) * sizeof(bool));
> + memset(xe->xdf2.changed - 1, 0,
> + (xe->xdf2.nrec + 2) * sizeof(bool));
This, especially the starting offset of -1, looks horrible. The
internal layout of xdfenv_t might happen to match the way the above
code expects, which is how xdl_prepare_ctx() may have give you, but
it somehow feels brittle. I guess the assumption that changed[]
does not point at the beginning of the allocated area (e.g., it is a
no-no to free(xe->xdf1.changed) or realloc() it) is so pervasive that
it cannot be helped. Sigh.
> int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
> xdemitconf_t const *xecfg, xdemitcb_t *ecb) {
> xdchange_t *xscr;
> xdfenv_t xe;
> emit_func_t ef = xecfg->hunk_func ? xdl_call_hunk_func : xdl_emit_diff;
>
> - if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0) {
> -
> - return -1;
> + if (xpp->external_hunks) {
> + if (xdl_prepare_env(mf1, mf2, xpp, &xe) < 0)
> + return -1;
> + if (xdl_populate_hunks_from_external(&xe,
> + xpp->external_hunks,
> + xpp->external_hunks_nr) < 0) {
> + /*
> + * Invalid external hunks; fall back to the
> + * builtin diff algorithm. Re-runs
> + * xdl_prepare_env() via xdl_do_diff().
> + */
> + xdl_free_env(&xe);
> + if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0)
> + return -1;
If the external tool keeps sending bogus hunks, silently falling
back to what we would have done if there weren't any external stuff
may be necessary to pleasantly keep using Git, but two and a half
short comments here.
(1) "What we would have done" is exactly the same as what appears
in the corresponding "else" block. Can we make sure that we do
not have to keep updating both copies in the future with some
code rearrangement?
(2) The writer of the external tool may want to see some trace of
warning under certain flags when a failure of the tool forces
the receiving end to fallback.
(3) If the tool throws too many broken replies, perhaps we want to
disable it automatically?
> + }
> + } else {
> + if (xdl_do_diff(mf1, mf2, xpp, &xe) < 0)
> + return -1;
> }
> +
> if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
> xdl_change_compact(&xe.xdf2, &xe.xdf1, xpp->flags) < 0 ||
> xdl_build_script(&xe, &xscr) < 0) {
^ permalink raw reply
* Re: [PATCH] log: let --follow follow renames in merge commits
From: Jeff King @ 2026-05-22 5:43 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Elijah Newren, git
In-Reply-To: <ag2265RJal-tJLoW@collabora.com>
On Wed, May 20, 2026 at 03:28:11PM +0200, Miklos Vajna wrote:
> Hi Elijah, Jeff,
>
> On Tue, May 19, 2026 at 03:37:54PM +0900, Junio C Hamano <gitster@pobox.com> wrote:
> > > :-) Should I just wait more or should I resend this?
> >
> > Rather, ask other reviewers
>
> I did a small improvement to how 'git log --follow' works, as in if the
> rename happens inside the merge commit itself, then the rename was
> detected "vs the first parent", but it wasn't detected "vs other
> parents", which is painful with a "subtree" merge commit.
>
> I'm not sure if it adds value, but I can append a one-paragraph summary
> of Junio's comment in this thread to the end the commit message, to be
> more explicit that the inherent limitation of the current log follow
> design (single path, once a rename is detected, we only care about the
> new path) is not changed with the patch, this is just a fix patch so
> 'git log' works better, similar to how 'git blame' already does.
>
> May I ask you to review the patch?
I saw Junio's comment. I was about to write something very similar
before I saw that he had already done so. ;)
I think we can probably all agree that both before and after your patch,
--follow is never going to do the _right_ thing, which is to follow
paths independently down both sides of history.
I am OK conceptually with making the current broken behavior slightly
more useful if it is easy to do. But I am not sure if we are making
things more useful here or not. If we see a merge where the file "bar"
was previous "foo" on one side and "bar" on the other, our broken follow
is going to either pick "foo" or "bar" to continue with as we traverse.
But which one is right? Whichever name we choose, we are potentially
omitting results from the other side.
Right now we pick the first-parent name always. But it does not seem
more correct to me to pick one from another parent. You'd be missing
further commits using the original name along the first-parent track.
There might be a more useful rule like: if the path is untouched versus
the merge result in all parents but one (i.e., TREESAME), then choose
the parent where it was changed, including any --follow processing. But
we already do something like that for history simplification. Which
makes me wonder if you could get the results you want through some use
of history-simplification flags.
Or maybe we already do that TREESAME check. Simplification kicks in when
the traversal is limited by path, and --follow mode by definition has
such a path. But I'm not sure if the --follow code would see the
simplified parent list or not.
So I dunno. Probably some experimenting could yield more analysis there,
but with the patch as-is I'm not convinced that it is not going to make
some cases worse.
-Peff
^ permalink raw reply
* Re: [PATCH 4/9] run-command: add support for timeout in command finisher
From: Siddh Raman Pant @ 2026-05-22 5:46 UTC (permalink / raw)
To: j6t@kdbg.org, gitster@pobox.com
Cc: git@vger.kernel.org, newren@gmail.com, ps@pks.im,
oswald.buddenhagen@gmx.de, code@khaugsbakk.name
In-Reply-To: <xmqqv7cgxq0o.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
On Fri, May 22 2026 at 05:40:47 +0530, Junio C Hamano wrote:
> If a protocol builds its own way to declare "this backend is slow,
> so please do not consider less than 3 seconds of nonaction something
> to worry about but kill it off if you waited more than that" to make
> the receiving/waiting end responsible for managing timeout, that
> might be workable, but it certainly feels like a kludge. The
> protocol can instead allow an "error - for your particular request,
> we couldn't come up with an answer within a reasonable time limit"
> response (in practice, "within time limit" does not have to be the
> only reason for such an error) to be returned, I think.
I think we are confusing two different commits here.
The response read deadline is the next commit. This commit is about
force-killing a process if it doesn't respond to the initial
termination signal.
Thanks,
Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 0/8] setup: centralize object database creation
From: Patrick Steinhardt @ 2026-05-22 6:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqqzn4xp0c.fsf@gitster.g>
On Fri, May 22, 2026 at 09:32:35AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > The series is built on top of aec3f58750 (Sync with 'maint', 2026-05-21)
> > with ps/setup-wo-the-repository at df69f40c34 (setup: stop using
> > `the_repository` in `init_db()`, 2026-05-19) merged into it.
>
> FWIW, this merge needs the following merge-fix squashed into it,
> for the topic to build standalone.
>
> commit ce350f62ceb26f3276ea3b7ad78b7f8cb4c35cf7
> Author: Junio C Hamano <gitster@pobox.com>
> Date: Wed May 13 12:20:29 2026 +0900
>
> merge-fix/ps/setup-wo-the-repository
>
> with js/objects-larger-than-4gb-on-windows
>
> diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
> index 1f28ecf0f2..3fa534fbdf 100644
> --- a/t/helper/test-synthesize.c
> +++ b/t/helper/test-synthesize.c
> @@ -506,7 +506,7 @@ static int cmd__synthesize__pack(int argc, const char **argv,
> OPT_END()
> };
>
> - setup_git_directory_gently(&non_git);
> + setup_git_directory_gently(the_repository, &non_git);
> repo = the_repository;
> algo = unsafe_hash_algo(repo->hash_algo);
Oh, right, I should have mentioned this. I do have the same fixup on top
of the merge, thanks.
Patrick
^ permalink raw reply
* Re: [PATCH 8/8] setup: construct object database in `apply_repository_format()`
From: Patrick Steinhardt @ 2026-05-22 6:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq4ik0zls3.fsf@gitster.g>
On Fri, May 22, 2026 at 02:59:24AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > With the preceding changes we now always construct the repository's
> > object database before applying the repository format. Remove this
> > duplication by constructing it in `apply_repository_format()` instead.
> >
> > Note that we create the object database _after_ having set up the
> > repository's hash algorithm, but _before_ setting the compat hash
> > algorithm. This is intentional:
> >
> > - Constructing the object database may require knowledge of its
> > intended object format.
> >
> > - Setting up the compatibility hash requires the object database to be
> > initialized already, because we immediately read the loose object
> > map.
> >
> > The first point is sensible, the second maybe a little less so. Ideally,
> > it should be the responsibility of the object database itself to
> > initialize any data structures required for the compatibility hash. But
> > this would require further changes, so this is kept as-is for now.
>
> Yeah, I guess it is a good place to stop, instead of solving the
> chicken-and-egg problem in one go.
>
> > Further note that this requires us to move handling of the environment
> > variables GIT_OBJECT_DIRECTORY and GIT_ALTERNATE_OBJECT_DIRECTORIES into
> > the repository format, as well. This allows the caller more flexibility
> > around whether or not those environment variables are being honored, as
> > we do do want to respect them in "setup.c", but not in "repository.c".
>
> It seems that we really really really want to do so ;-). "do do
> want to" -> "do want to" or even "want to", perhaps.
Fixed locally, thanks! :)
Patrick
^ permalink raw reply
* Re: [PATCH 1/8] t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
From: Patrick Steinhardt @ 2026-05-22 6:06 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git
In-Reply-To: <741c2a26-7380-4d8e-aa91-fb237e9f10dc@app.fastmail.com>
On Thu, May 21, 2026 at 07:51:59PM +0200, Kristoffer Haugsbakk wrote:
> On Thu, May 21, 2026, at 09:42, Patrick Steinhardt wrote:
> > In subsequent commits we'll rework how we set up the repository. This is
> > a somewhat intricate and thus fragile sequence, there's many things that
>
> Should this be s/, there/; there/ ? Depends on if this is a list of
> three items or if “This is” is a subclause that is supposed to point at
> “there's many”.
That reads a bit better.
> > can go subtly wrong, and there are lots of interesting interactions that
> > one can discover.
> >
> > One such discovered edge case was the interaction between git-init(1)
> > and the "GIT_OBJECT_DIRECTORY" enviroment variable. When set, the
> > behaviour is that the object directory should be created at the path
> > that the variable points to. This behaviour is documented as such in
> > its man page:
> >
> > If the object storage directory is specified via the
> > GIT_OBJECT_DIRECTORY environment variable then the sha1 directories
> > are created underneath; otherwise, the default $GIT_DIR/objects
> > directory is used.
> >
> > Curiously enough though we don't seem to have any tests that exercise
> > this directly, and thus a subsequent commit inadvertently broke this
> > expectation.
>
> Isn’t it more that “the upcoming changes *would have* broken” them if
> not for this change? This seems to refer to a an alternative commit
> history where this change does not exist?
Grammar is hard :) But yeah, this of course refers to an alternative
commit history I had at one point in time that did break this.
Fixed locally, will wait a bit before sending out the next version.
Patrick
^ permalink raw reply
* Re: [PATCH 4/9] run-command: add support for timeout in command finisher
From: Siddh Raman Pant @ 2026-05-22 5:59 UTC (permalink / raw)
To: j6t@kdbg.org, peff@peff.net
Cc: git@vger.kernel.org, gitster@pobox.com, newren@gmail.com,
ps@pks.im, oswald.buddenhagen@gmx.de, code@khaugsbakk.name
In-Reply-To: <20260522051048.GA862219@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2906 bytes --]
On Fri, May 22 2026 at 10:40:48 +0530, Jeff King wrote:
> On Thu, May 21, 2026 at 04:36:05PM +0200, Johannes Sixt wrote:
>
> > Am 21.05.26 um 11:59 schrieb Siddh Raman Pant:
> > > The timeout is for the failure path, where the external helper has
> > > already stopped following that protocol or is blocked on something
> > > outside git's control. Since git starts the helper and puts it on the
> > > log/grep path, git also needs a bounded way to recover when that helper
> > > does not make progress. Otherwise an optional note source can prevent
> > > the main git command from completing.
> >
> > That Git communicates with a process that looks like it stopped is the
> > normal case, for example:
> >
> > - Output is sent to the pager. The user can take their time to study the
> > output. All the while, git waits patiently for the user to advance the
> > pager.
> >
> > - Git fetch transfers large amounts of data across the network. Most of
> > the time it waits for data to arrive and does nothing. The peer process
> > looks like it hangs. Git does not decide to kill the connection at any
> > time. It is the user's decision to do so.
> >
> > If the notes provider hangs, then it is not on Git to decide when it has
> > waited long enough.
>
> Yeah, I agree with your point of view. If I understand this patch series
> correctly, it is about adding an external process to map commit ids to
> note data. So I can think of some existing features that are quite close
> to that in nature, none of which use timeouts:
>
> - textconv filters and external diffs which process data in the middle
> of a git-log invocation
>
> - long-lived clean/smudge filters map blobs to arbitrarily large text
>
> - cat-file's batch mode maps object ids to user-specified data about
> that object
>
> As you note, it's up to the command to be well-behaved. Git should
> notice and respond appropriately if the command closes the pipe, of
> course. Sometimes a timeout can help with a poorly behaved command, but
> IMHO it is not worth the cost of non-determinism that it brings.
>
> Moreover, the bits touching run-command here make me nervous, especially
> after the challenges we saw in the child-cleanup topic that was reverted
> just after v2.54. There is often a shell interposed between Git and the
> sub-command, and we don't always know how the shell will react to
> signals. Using SIGKILL will eventually get us _something_ to wait() on,
> but it might not even be the process we care about!
>
> I don't really care much about this external-notes feature one way or
> the other, but if we are going to do it, I don't see any reason why it
> would not behave like all of the other similar parts of Git.
>
> -Peff
Okay, since the consensus here is pretty clear, I will remove this
commit and send a v2.
Thanks,
Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 16/18] odb/source-loose: wire up `write_object_stream()` callback
From: Patrick Steinhardt @ 2026-05-22 6:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8q9czm8r.fsf@gitster.g>
On Fri, May 22, 2026 at 02:49:24AM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > -int odb_source_loose_write_stream(struct odb_source_loose *loose,
> > +/*
> > + * Write the given stream into the loose object source. The only difference to
> > + * the generic implementation of this function is that we don't perform an
>
> "difference to" -> "difference from"???
I guess this is a difference between American and British English. "to"
is more popular in British English, but basically not used at all in
American English. Will adapt, thanks.
Patrick
^ permalink raw reply
* [PATCH] compat/mingw: Allow SIGKILL to kill in mingw_kill.
From: Siddh Raman Pant @ 2026-05-22 6:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kristoffer Haugsbakk, Elijah Newren,
Patrick Steinhardt
mingw_kill() only allows SIGTERM for killing a process.
Let's also allow the natural SIGKILL for the same so that callers don't
have to do ifdef soup for special Windows handling.
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
compat/mingw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419cb..00a994aa9f47 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2250,7 +2250,7 @@ int mingw_execvp(const char *cmd, char *const *argv)
int mingw_kill(pid_t pid, int sig)
{
- if (pid > 0 && sig == SIGTERM) {
+ if (pid > 0 && (sig == SIGTERM || sig == SIGKILL)) {
HANDLE h = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
if (TerminateProcess(h, -1)) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v10 1/4] branch: add --forked <branch>
From: Johannes Sixt @ 2026-05-22 6:18 UTC (permalink / raw)
To: Harald Nordgren
Cc: Kristoffer Haugsbakk, Phillip Wood, git,
Harald Nordgren via GitGitGadget
In-Reply-To: <f2df15983067ce39b6c33ab81115863d5c3567f4.1779403204.git.gitgitgadget@gmail.com>
Am 22.05.26 um 00:40 schrieb Harald Nordgren via GitGitGadget:
> diff --git a/Documentation/git-branch.adoc b/Documentation/git-branch.adoc
> index c0afddc424..3a421f6663 100644
> --- a/Documentation/git-branch.adoc
> +++ b/Documentation/git-branch.adoc
> @@ -24,6 +24,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 --forked <branch>...
I would have preferred that this option is another filter of --list
mode, not its own mode of operation. Consequently, each --forked option
would take only a single argument (which can contain globs), and can be
given multiple times.
>
> DESCRIPTION
> -----------
> @@ -199,6 +200,12 @@ This option is only applicable in non-verbose mode.
> Print the name of the current branch. In detached `HEAD` state,
> nothing is printed.
>
> +`--forked`::
> + List local branches whose configured upstream matches any
> + of the given _<branch>_ arguments. Each argument is either
> + a ref (e.g. `origin/master`, `master`) or a shell-style
> + glob (e.g. `'origin/*'`). Multiple arguments are unioned.
So this could perhaps read:
`--forked`::
List only branches whose configured upstream matches
_<branch>_. The argument can contain a shell-style glob
(e.g. `'origin/*'`). The option can be repeated to
widen the filter.
Note that there is no reason to say "local branches". ("... are unioned"
sounds strange, so this is may attempt to express the same in a
different way.)
The icing on the cake would now be that
git branch --merged origin/main --forked origin/*
provides the list of branches forked from origin that have already been
integrated.
-- Hannes
^ permalink raw reply
* Re: [PATCH] compat/mingw: Allow SIGKILL to kill in mingw_kill.
From: Junio C Hamano @ 2026-05-22 6:32 UTC (permalink / raw)
To: Siddh Raman Pant, Johannes Sixt, Johannes Schindelin
Cc: git, Kristoffer Haugsbakk, Elijah Newren, Patrick Steinhardt
In-Reply-To: <20260522061652.50078-1-siddh.raman.pant@oracle.com>
Siddh Raman Pant <siddh.raman.pant@oracle.com> writes:
> mingw_kill() only allows SIGTERM for killing a process.
>
> Let's also allow the natural SIGKILL for the same so that callers don't
> have to do ifdef soup for special Windows handling.
>
> Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> ---
> compat/mingw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I do not do windows, so I'd like to ask those much more clueful than
I am to see if they see any downsides.
The current code only handles TERM (to terminate) or 0 (to probe)
and everything else results in EINVAL, so the updated behaviour is
to pretend as if TERM is sent and do whatever PROCESS_TERMINATE
does, instead of doing nothing and erroring with EINVAL. Which does
sound like an improvement over the status quo.
What I am wondering is if there are different kind of "kill" in the
Windows land, just like there are distinction between TERM and KILL.
For example, the program ought to be able to block TERM but not
KILL. There are other termination-inducing signals like SIGQUIT but
until we start using them in our code, this emulation layer does not
have to know about them, I think.
Thanks.
> diff --git a/compat/mingw.c b/compat/mingw.c
> index aa7525f419cb..00a994aa9f47 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2250,7 +2250,7 @@ int mingw_execvp(const char *cmd, char *const *argv)
>
> int mingw_kill(pid_t pid, int sig)
> {
> - if (pid > 0 && sig == SIGTERM) {
> + if (pid > 0 && (sig == SIGTERM || sig == SIGKILL)) {
> HANDLE h = OpenProcess(PROCESS_TERMINATE, FALSE, pid);
>
> if (TerminateProcess(h, -1)) {
^ permalink raw reply
* Re: [PATCH v10 1/4] branch: add --forked <branch>
From: Junio C Hamano @ 2026-05-22 6:36 UTC (permalink / raw)
To: Johannes Sixt
Cc: Harald Nordgren, Kristoffer Haugsbakk, Phillip Wood, git,
Harald Nordgren via GitGitGadget
In-Reply-To: <273103d7-c816-4cde-9e89-b630c37b0749@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> The icing on the cake would now be that
>
> git branch --merged origin/main --forked origin/*
>
> provides the list of branches forked from origin that have already been
> integrated.
Yup, that is very nice. Also with "--merged" replaced with
"--not-merged", i.e., "our work building on top of origin's, and
still need to be finished", would give us a good list to work on.
^ permalink raw reply
* [PATCH] doc: clarify push.default=simple in triangular workflows
From: Ivan Baluta via GitGitGadget @ 2026-05-22 6:58 UTC (permalink / raw)
To: git; +Cc: Ivan Baluta, ivanbaluta
From: ivanbaluta <ivanbaluta.dev@gmail.com>
The documentation for 'simple' push mode currently focuses on the
centralized workflow. However, the implementation in builtin/push.c
falls back to 'current' behavior when pushing to a remote different
from the upstream (a triangular workflow).
Clarify this in the manual to align the documentation with the
long-standing implementation and prevent user confusion.
Signed-off-by: ivanbaluta <ivanbaluta.dev@gmail.com>
---
doc: clarify push.default=simple in triangular workflows
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2115%2Fivanbaluta%2Fdoc-push-simple-triangular-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2115/ivanbaluta/doc-push-simple-triangular-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2115
Documentation/config/push.adoc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc
index d9112b2260..979e40c3a4 100644
--- a/Documentation/config/push.adoc
+++ b/Documentation/config/push.adoc
@@ -45,6 +45,9 @@ If you are working on a centralized workflow (pushing to the same repository you
pull from, which is typically `origin`), then you need to configure an upstream
branch with the same name.
+
+In a triangular workflow (pushing to a remote different from the upstream),
+`simple` behaves like `current`.
++
This mode is the default since Git 2.0, and is the safest option suited for
beginners.
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3] git-jump: pick a mode automatically when invoked without arguments
From: Greg Hurrell @ 2026-05-22 7:33 UTC (permalink / raw)
To: Jeff King, Greg Hurrell
Cc: git, Erik Cervin Edin, Junio C Hamano,
Gregory Luke Hurrell Stewart
In-Reply-To: <20260522052821.GC861761@coredump.intra.peff.net>
On Fri, May 22, 2026, at 7:28 AM, Jeff King wrote:
>
> My impression of the "auto" feature is: I am too lazy to type, so just
> take me to the interesting bits. And interesting in my experience with
> git-jump is either "I am merging, take me to the conflict" or "I am
> writing new code, take me to what I already did". Limiting the second
> case just to whitespace violations (assuming there is at least one)
> would probably be more confusing than helpful.
> If sounds like Greg has been living with "auto" and finding it useful
> for a while. So I'm mostly inclined to take the patch as-is, and people
> can experiment with it and suggest changes after using it in practice.
Yes, the "take me to the interesting bits" is very much the mental model
I've been operating with, using the simplest definition of "interesting"
("merge conflicts", followed by "changes in the worktree"). I think that
starting simple, but leaving the door open to possibly introducing more
subtleties in the future makes the most sense.
- Greg
^ permalink raw reply
* I discovered a minor issue with `git fetch`.
From: SURA @ 2026-05-22 7:45 UTC (permalink / raw)
To: git
Hello everyone
The child processes spawned by `git fetch` can become zombie processes.
In most scenarios, these zombie processes are reaped by Process 1, so
this typically doesn't cause any problems.
However, within a Docker container, the application service itself is
sometimes designated as Process 1 (for instance, a service written in
Go). Since these application services lack the capability to reap
zombie processes, the zombies will gradually exhaust the available PID
resources.
Here are the simple steps to reproduce this issue:
1. `git clone https://github.com/SURA907/pid-1.git`
2. `cd pid-1`
3. `docker build -t pid-1 .`
4. `docker run -d --name pid-1 pid-1:latest`
5. `docker exec -it pid-1 /bin/bash`
6. `mkdir repo && cd repo && git init --bare`
7. `ps -ef`
------
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
root 29 7 0 07:17 pts/0 00:00:00 ps -ef
------
8. `git fetch https://github.com/git/git.git`
9. `ps -ef` (Run this command from a separate terminal session
connected to the container)
------
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
root 30 13 1 07:17 pts/1 00:00:00 git fetch https://github.com/git/git.git
root 31 30 0 07:17 pts/1 00:00:00 /usr/local/libexec/git-core/git
remote-https https://github.com/git/git.git
https://github.com/git/git.git
root 32 31 2 07:17 pts/1 00:00:00
/usr/local/libexec/git-core/git-remote-https
https://github.com/git/git.git https://github.com/git/git.git
root 36 30 30 07:17 pts/1 00:00:00 /usr/local/libexec/git-core/git
index-pack --stdin -v --fix-thin --keep=fetch-pack 30 on sura-pc
--pack_header=2,399455
root 38 7 0 07:17 pts/0 00:00:00 ps -ef
------
10. ps -ef (after fetch ends)
------
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
root 52 1 0 07:19 ? 00:00:00 [git] <defunct>
root 53 7 0 07:19 pts/0 00:00:00 ps -ef
------
A zombie process has appeared. It appears to originate from a `fetch`
subprocess that terminates very quickly; despite several attempts, I
have been unable to successfully capture it.
This issue was discovered within a legacy service. A few days after
upgrading to Git 2.53.0, the system's PID resources were exhausted by
zombie processes. This is likely the result of recent changes, as this
problem did not exist in earlier versions (2.4x).
To be honest, this is not an urgent matter; I have already deployed
`tini` as the init process (PID 1) to prevent the service from
becoming unavailable.
^ 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