* Re: [PATCH 4/4] last-modified: keep per-path Bloom filters for wildcard pathspecs
From: Toon Claes @ 2026-07-17 19:16 UTC (permalink / raw)
To: git; +Cc: Gusted, Jeff King
In-Reply-To: <20260717-toon-speed-up-last-modified-v1-4-410418f18614@iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> The last-modified builtin expands the pathspec to a set of literal paths
> and builds a Bloom key for each. During the walk it looks those keys up
> in the commit's filter to decide whether the commit is worth diffing.
> These lookups need `bloom_filter_settings` for the key hashing.
>
> prepare_revision_walk() runs prepare_to_use_bloom_filter() to build the
> pathspec key vectors. For a pathspec that cannot be turned into a Bloom
> key, such as a top-level wildcard like "*.c", that function gives up and
> clears `bloom_filter_settings`.
>
> Restore `bloom_filter_settings` after prepare_revision_walk() so the
> per-path check keeps working for wildcard pathspecs.
>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> builtin/last-modified.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/builtin/last-modified.c b/builtin/last-modified.c
> index e8ee610404..adc7cd8c74 100644
> --- a/builtin/last-modified.c
> +++ b/builtin/last-modified.c
> @@ -360,6 +360,14 @@ static int last_modified_run(struct last_modified *lm)
>
> prepare_revision_walk(&lm->rev);
>
> + /*
> + * prepare_revision_walk() clears bloom_filter_settings for pathspecs
> + * without a Bloom key. Restore it so the per-path check keeps working.
> + */
> + if (!lm->rev.bloom_filter_settings)
> + lm->rev.bloom_filter_settings =
> + get_bloom_filter_settings(lm->rev.repo);
> +
@Peff, as far I could tell:
* This change was not needed to be able to use the Bloom filters with
the pathspec.
* Only restoring bloom_filter_settings was needed. In your patch you're
calling prepare_to_use_bloom_filter(), but that is being called by
prepare_revision_walk(). Thus the restoring of the filter settings
I've added after that function.
> max_count = lm->rev.max_count;
>
> init_active_paths_for_commit(&lm->active_paths);
>
> --
> 2.53.0.1323.g189a785ab5
>
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH 0/4] last-modified: use the pathspec's Bloom key to pre-filter commits
From: Toon Claes @ 2026-07-17 19:13 UTC (permalink / raw)
To: git; +Cc: Gusted, Jeff King
In-Reply-To: <20260717-toon-speed-up-last-modified-v1-0-410418f18614@iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> - Patch 3 uses this new helper in git-last-modified(1).
> - Patch 4 is bonus change, which optimizes when working with wildcard
> pathspecs.
I just realize I forgot to add Helped-by or Based-on-patches-by trailers
for Peff. I'm happy to add them on reroll.
--
Cheers,
Toon
^ permalink raw reply
* Re: git-last-modified(1) slower than git-log(1)?
From: Toon Claes @ 2026-07-17 19:12 UTC (permalink / raw)
To: Gusted, git, Jeff King
In-Reply-To: <87v7afffpa.fsf@emacs.iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> You're actually not the first to notice this, and I've been aware of
> this.
Hah, wait, ... the previous reporter was ... you[1].
> The thing is, you're testing the difference on a single file.
I was mixing up use-cases, and the problem you're stating here isn't
related to your previous report. Now while I did sent out patches for
the problem in this email, the issue in the other email isn't addressed
with it. Unfortunately.
(nerdsniping Peff :-))
Gusted <gusted@codeberg.org> writes:
> The current implementation of Forgejo (inherited from Gitea) works
> roughly like this:
> 1. Run `git log --name-status -c --format=commit%x00%H %P%x00" --parents
> --no-renames -t -z $OID -- :(literal)some/path`, the output of this is
> quite complex and possible outputs more information than necessary.
That's quite clever actually.
> 2. The output of this is piped to some code to a parser and reconstructs
> what commit ID last modified each file in the directory.
My only worry would be this could end up in a very long list of
(duplicate) commits. But you can probably filter out data as you read in
lines.
> 3. Via `git cat-file --batch` get each unique commits information.
Yes we use git-cat-file(1) in batch mode too.
But, and I learned this from blog post[2] from GitHub about Git v2.55,
git v2.55 now has git-format-rev(1)[3]. You can pipe the output of
git-last-modified(1) into that and get everything you want at once
(maybe).
I haven't tried yet to integrate that into GitLab to see if it would
bring any gains.
[1]: https://lore.kernel.org/git/03f96860-29fc-42a7-a220-c3ec65eb8516@codeberg.org/
[2]: https://github.blog/open-source/git/highlights-from-git-2-55/
[3]: https://git-scm.com/docs/git-format-rev
--
Cheers,
Toon
^ permalink raw reply
* git-history(1) fixup broken with worktrees?
From: Toon Claes @ 2026-07-17 18:54 UTC (permalink / raw)
To: Patrick Steinhardt, git
Hi,
Imagine this repoducer:
$ git init
$ echo Hello > README
$ git add .
$ git commit -m'initial commit'
$ git worktree add ../feature
$ echo world >> README
$ git add .
$ git history fixup HEAD
$ cd ../feature
Now running git-status(1) in that other worktree gives me:
$ git status
On branch feature
Changes to be committed:
(use "git restore --staged <file>..." to unstage)
modified: README
And:
$ git diff --staged
diff --git a/README b/README
index 65a56c3..e965047 100644
--- a/README
+++ b/README
@@ -1,2 +1 @@
Hello
-world
So suddenly my other worktree is dirty? With staged changes?
And I didn't even touch it.
Now the commit history is correct:
$ git log --graph --oneline --all
* 16ef548 (HEAD -> feature, main) initial commit
--
Cheers,
Toon
^ permalink raw reply
* [PATCH v2 3/3] bisect: add --auto-reset to leave when done
From: Harald Nordgren via GitGitGadget @ 2026-07-17 18:27 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2335.v2.git.git.1784312854.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
When a bisection finishes, "git bisect" reports the first bad commit
but leaves the session active until "git bisect reset" is run by hand.
Add an "--auto-reset[=<where>]" option, accepted by both "git bisect
start" and "git bisect run", that resets as soon as the first bad commit
is found. The "original" value returns to the commit checked out before
"git bisect start", while "found" leaves the first bad commit checked
out; omitting the value defaults to "original".
Persist the selected target in a BISECT_AUTO_RESET state file and perform
the reset quietly. Reject this option together with "--no-checkout",
since that mode must not check out either target.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-bisect.adoc | 14 ++++-
bisect.c | 2 +
builtin/bisect.c | 100 ++++++++++++++++++++++++++++++-
t/t6030-bisect-porcelain.sh | 107 ++++++++++++++++++++++++++++++++++
4 files changed, 219 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-bisect.adoc b/Documentation/git-bisect.adoc
index d2115b2990..afae508463 100644
--- a/Documentation/git-bisect.adoc
+++ b/Documentation/git-bisect.adoc
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[synopsis]
git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
+ [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-(good|old) | --term-(bad|new)]
@@ -20,7 +20,7 @@ git bisect reset [<commit>]
git bisect (visualize|view)
git bisect replay <logfile>
git bisect log
-git bisect run <cmd> [<arg>...]
+git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]
git bisect help
DESCRIPTION
@@ -385,6 +385,16 @@ ignored.
This option is particularly useful in avoiding false positives when a merged
branch contained broken or non-buildable commits, but the merge itself was OK.
+`--auto-reset[=<where>]`::
+ Once the first bad commit is found, report it and clean up the
+ bisection state. `<where>` may be `original` to return to the commit
+ checked out before `git bisect start`, or `found` to leave the first
+ bad commit checked out. If `<where>` is omitted, it defaults to
+ `original`.
++
+This option may be given to `git bisect start` or to `git bisect run`. It
+cannot be used for a bisection started with `--no-checkout`.
+
EXAMPLES
--------
diff --git a/bisect.c b/bisect.c
index 94c7028d2a..a34309dd35 100644
--- a/bisect.c
+++ b/bisect.c
@@ -488,6 +488,7 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
+static GIT_PATH_FUNC(git_path_bisect_auto_reset, "BISECT_AUTO_RESET")
static void read_bisect_paths(struct strvec *array)
{
@@ -1211,6 +1212,7 @@ int bisect_clean_state(void)
unlink_or_warn(git_path_bisect_run());
unlink_or_warn(git_path_bisect_terms());
unlink_or_warn(git_path_bisect_first_parent());
+ unlink_or_warn(git_path_bisect_auto_reset());
/*
* Cleanup BISECT_START last to support the --no-checkout option
* introduced in the commit 4796e823a.
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 27d30b549e..6e835b7d2a 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -24,11 +24,12 @@ static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
static GIT_PATH_FUNC(git_path_bisect_log, "BISECT_LOG")
static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES")
static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
+static GIT_PATH_FUNC(git_path_bisect_auto_reset, "BISECT_AUTO_RESET")
static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_START_USAGE \
N_("git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]\n" \
- " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]")
+ " [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
#define BUILTIN_GIT_BISECT_BAD_USAGE \
N_("git bisect (bad|new|<term-new>) [<rev>]")
#define BUILTIN_GIT_BISECT_GOOD_USAGE \
@@ -48,7 +49,7 @@ static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
#define BUILTIN_GIT_BISECT_LOG_USAGE \
"git bisect log"
#define BUILTIN_GIT_BISECT_RUN_USAGE \
- N_("git bisect run <cmd> [<arg>...]")
+ N_("git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]")
#define BUILTIN_GIT_BISECT_HELP_USAGE \
"git bisect help"
@@ -68,6 +69,12 @@ static const char * const git_bisect_usage[] = {
NULL
};
+enum auto_reset_mode {
+ AUTO_RESET_NONE,
+ AUTO_RESET_ORIGINAL,
+ AUTO_RESET_FOUND,
+};
+
struct add_bisect_ref_data {
struct rev_info *revs;
unsigned int object_flags;
@@ -268,6 +275,59 @@ static int bisect_reset(const char *commit, int quiet)
return bisect_clean_state();
}
+static int parse_auto_reset(const char *value, enum auto_reset_mode *mode)
+{
+ if (!strcmp(value, "original"))
+ *mode = AUTO_RESET_ORIGINAL;
+ else if (!strcmp(value, "found"))
+ *mode = AUTO_RESET_FOUND;
+ else
+ return error(_("invalid value for '--auto-reset': '%s'"), value);
+
+ return 0;
+}
+
+static const char *auto_reset_mode_name(enum auto_reset_mode mode)
+{
+ switch (mode) {
+ case AUTO_RESET_ORIGINAL:
+ return "original";
+ case AUTO_RESET_FOUND:
+ return "found";
+ case AUTO_RESET_NONE:
+ BUG("no name for unset auto-reset mode");
+ }
+ BUG("unknown auto-reset mode %d", mode);
+}
+
+static int bisect_auto_reset(struct bisect_terms *terms)
+{
+ struct strbuf value = STRBUF_INIT;
+ enum auto_reset_mode mode;
+ char *commit = NULL;
+ int res;
+
+ if (strbuf_read_file(&value, git_path_bisect_auto_reset(), 0) < 0) {
+ res = error_errno(_("could not read '%s'"),
+ git_path_bisect_auto_reset());
+ goto cleanup;
+ }
+ strbuf_trim(&value);
+ if (parse_auto_reset(value.buf, &mode)) {
+ res = -1;
+ goto cleanup;
+ }
+
+ if (mode == AUTO_RESET_FOUND)
+ commit = xstrfmt("refs/bisect/%s", terms->term_bad);
+ res = bisect_reset(commit, 1);
+
+cleanup:
+ free(commit);
+ strbuf_release(&value);
+ return res;
+}
+
static void log_commit(FILE *fp,
const char *fmt, const char *state,
struct commit *commit)
@@ -688,6 +748,8 @@ static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
res = bisect_successful(terms);
+ if (!res && !is_empty_or_missing_file(git_path_bisect_auto_reset()))
+ res = bisect_auto_reset(terms);
return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
} else if (res == BISECT_ONLY_SKIPPED_LEFT) {
res = bisect_skipped_commits(terms);
@@ -711,6 +773,7 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
{
int no_checkout = 0;
int first_parent_only = 0;
+ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
int flags, pathspec_pos;
enum bisect_error res = BISECT_OK;
@@ -743,6 +806,13 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
no_checkout = 1;
} else if (!strcmp(arg, "--first-parent")) {
first_parent_only = 1;
+ } else if (!strcmp(arg, "--auto-reset")) {
+ auto_reset = AUTO_RESET_ORIGINAL;
+ } else if (skip_prefix(arg, "--auto-reset=", &arg)) {
+ if (parse_auto_reset(arg, &auto_reset)) {
+ res = BISECT_FAILED;
+ goto finish;
+ }
} else if (!strcmp(arg, "--term-good") ||
!strcmp(arg, "--term-old")) {
i++;
@@ -780,6 +850,10 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
break;
}
}
+ if (auto_reset != AUTO_RESET_NONE && no_checkout) {
+ res = error(_("'--auto-reset' cannot be used with '--no-checkout'"));
+ goto finish;
+ }
pathspec_pos = i;
/*
@@ -857,6 +931,10 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
if (first_parent_only)
write_file(git_path_bisect_first_parent(), "\n");
+ if (auto_reset != AUTO_RESET_NONE)
+ write_file(git_path_bisect_auto_reset(), "%s\n",
+ auto_reset_mode_name(auto_reset));
+
if (no_checkout) {
if (repo_get_oid(the_repository, start_head.buf, &oid) < 0) {
res = error(_("invalid ref: '%s'"), start_head.buf);
@@ -1235,6 +1313,8 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
{
int res = BISECT_OK;
struct strbuf command = STRBUF_INIT;
+ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
+ const char *auto_reset_arg;
const char *new_state;
int temporary_stdout_fd, saved_stdout;
int is_first_run = 1;
@@ -1242,6 +1322,22 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
if (bisect_next_check(terms, NULL))
return BISECT_FAILED;
+ if (argc && !strcmp(argv[0], "--auto-reset"))
+ auto_reset = AUTO_RESET_ORIGINAL;
+ else if (argc && skip_prefix(argv[0], "--auto-reset=", &auto_reset_arg)) {
+ if (parse_auto_reset(auto_reset_arg, &auto_reset))
+ return BISECT_FAILED;
+ }
+
+ if (auto_reset != AUTO_RESET_NONE) {
+ if (refs_ref_exists(get_main_ref_store(the_repository), "BISECT_HEAD"))
+ return error(_("'--auto-reset' cannot be used with '--no-checkout'"));
+ write_file(git_path_bisect_auto_reset(), "%s\n",
+ auto_reset_mode_name(auto_reset));
+ argc--;
+ argv++;
+ }
+
if (!argc) {
error(_("bisect run failed: no command provided."));
return BISECT_FAILED;
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 081116220a..826e54efea 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -43,6 +43,37 @@ test_bisect_usage () {
test_cmp expect actual
}
+test_bisect_state_file () {
+ test_path_is_file "$(git rev-parse --git-path "$1")"
+}
+
+test_bisect_state_missing () {
+ test_path_is_missing "$(git rev-parse --git-path "$1")"
+}
+
+bisect_start_and_finish () {
+ git bisect start "$1" $HASH4 $HASH2 &&
+ git bisect bad
+}
+
+bisect_run_auto_reset () {
+ write_script test_script.sh <<-\EOF &&
+ ! grep Another hello >/dev/null
+ EOF
+ git bisect start $HASH4 $HASH2 &&
+ git bisect run "$1" ./test_script.sh >my_bisect_log.txt &&
+ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt
+}
+
+test_auto_reset_fails () {
+ local pattern="$1" &&
+ local state_file="$2" &&
+ shift 2 &&
+ test_must_fail "$@" 2>err &&
+ test_grep -- "$pattern" err &&
+ test_bisect_state_missing "$state_file"
+}
+
test_expect_success 'bisect usage' "
test_bisect_usage 1 git bisect reset extra1 extra2 <<-\EOF &&
error: 'git bisect reset' requires either no argument or a commit
@@ -453,6 +484,82 @@ test_expect_success '"git bisect run" simple case' '
git bisect reset
'
+test_expect_success '"git bisect start --auto-reset" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ git checkout main &&
+ bisect_start_and_finish --auto-reset &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START &&
+
+ bisect_start_and_finish --auto-reset=original &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect start --auto-reset=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_start_and_finish --auto-reset=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect run --auto-reset" defaults to original' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_run_auto_reset --auto-reset &&
+ test "$HASH4" = "$(git rev-parse HEAD)" &&
+ test main = "$(git branch --show-current)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '"git bisect run --auto-reset=found" leaves first bad checked out' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_run_auto_reset --auto-reset=found &&
+ test "$HASH3" = "$(git rev-parse HEAD)" &&
+ test_bisect_state_missing BISECT_START
+'
+
+test_expect_success '--auto-reset rejects an unknown reset target' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ test_auto_reset_fails \
+ "invalid value for.*--auto-reset.*unknown" BISECT_START \
+ git bisect start --auto-reset=unknown $HASH4 $HASH2 &&
+
+ git bisect start $HASH4 $HASH2 &&
+ test_auto_reset_fails \
+ "invalid value for.*--auto-reset.*unknown" BISECT_AUTO_RESET \
+ git bisect run --auto-reset=unknown true
+'
+
+test_expect_success '--auto-reset cannot be used with --no-checkout' '
+ test_when_finished "git bisect reset" &&
+ test_auto_reset_fails \
+ "cannot be used with.*--no-checkout" BISECT_START \
+ git bisect start --auto-reset=original --no-checkout $HASH4 $HASH2 &&
+
+ git bisect start --no-checkout $HASH4 $HASH2 &&
+ test_auto_reset_fails \
+ "cannot be used with.*--no-checkout" BISECT_AUTO_RESET \
+ git bisect run --auto-reset=found true
+'
+
+test_expect_success 'without --auto-reset the bisection state is kept' '
+ test_when_finished "git bisect reset" &&
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
+test_expect_success '--auto-reset does not leak into a later bisection' '
+ test_when_finished "git bisect reset; git checkout main" &&
+ bisect_start_and_finish --auto-reset &&
+
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
+ test_bisect_state_file BISECT_START
+'
+
# We want to automatically find the commit that
# added "Ciao" into hello.
test_expect_success '"git bisect run" with more complex "git bisect start"' '
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/3] bisect: let bisect_reset() optionally check out quietly
From: Harald Nordgren via GitGitGadget @ 2026-07-17 18:27 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2335.v2.git.git.1784312854.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Add a "quiet" parameter to bisect_reset() that passes "--quiet" to the
checkout restoring the original HEAD, suppressing its progress and
branch-status output.
No caller sets the flag yet, so behavior is unchanged.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/bisect.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 69ea14b1b6..27d30b549e 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -230,7 +230,7 @@ static int write_terms(const char *bad, const char *good)
return res;
}
-static int bisect_reset(const char *commit)
+static int bisect_reset(const char *commit, int quiet)
{
struct strbuf branch = STRBUF_INIT;
@@ -251,8 +251,10 @@ static int bisect_reset(const char *commit)
struct child_process cmd = CHILD_PROCESS_INIT;
cmd.git_cmd = 1;
- strvec_pushl(&cmd.args, "checkout", "--ignore-other-worktrees",
- branch.buf, "--", NULL);
+ strvec_pushl(&cmd.args, "checkout", "--ignore-other-worktrees", NULL);
+ if (quiet)
+ strvec_push(&cmd.args, "--quiet");
+ strvec_pushl(&cmd.args, branch.buf, "--", NULL);
if (run_command(&cmd)) {
error(_("could not check out original"
" HEAD '%s'. Try 'git bisect"
@@ -1085,7 +1087,7 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
if (is_empty_or_missing_file(filename))
return error(_("cannot read file '%s' for replaying"), filename);
- if (bisect_reset(NULL))
+ if (bisect_reset(NULL, 0))
return BISECT_FAILED;
fp = fopen(filename, "r");
@@ -1334,7 +1336,7 @@ static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNU
if (argc > 1)
return error(_("'%s' requires either no argument or a commit"),
"git bisect reset");
- return bisect_reset(argc ? argv[0] : NULL);
+ return bisect_reset(argc ? argv[0] : NULL, 0);
}
static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNUSED,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/3] bisect: read run output from the open descriptor
From: Harald Nordgren via GitGitGadget @ 2026-07-17 18:27 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2335.v2.git.git.1784312854.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git bisect run" redirects each step's output into BISECT_RUN, then
prints it back by reopening the file by name. Read it from the already
open descriptor instead; this behaves the same and no longer needs the
file to be reachable by name.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/bisect.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 798e28f501..69ea14b1b6 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -178,17 +178,13 @@ static int append_to_file(const char *path, const char *format, ...)
return res;
}
-static int print_file_to_stdout(const char *path)
+static int print_fd_to_stdout(int fd)
{
- int fd = open(path, O_RDONLY);
- int ret = 0;
-
- if (fd < 0)
- return error_errno(_("cannot open file '%s' for reading"), path);
+ if (lseek(fd, 0, SEEK_SET) < 0)
+ return error_errno(_("failed to rewind BISECT_RUN output"));
if (copy_fd(fd, 1) < 0)
- ret = error_errno(_("failed to read '%s'"), path);
- close(fd);
- return ret;
+ return error_errno(_("failed to read BISECT_RUN output"));
+ return 0;
}
static int check_term_format(const char *term, const char *orig_term)
@@ -1291,7 +1287,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
else
new_state = terms->term_bad;
- temporary_stdout_fd = open(git_path_bisect_run(), O_CREAT | O_WRONLY | O_TRUNC, 0666);
+ temporary_stdout_fd = open(git_path_bisect_run(), O_CREAT | O_RDWR | O_TRUNC, 0666);
if (temporary_stdout_fd < 0) {
res = error_errno(_("cannot open file '%s' for writing"), git_path_bisect_run());
@@ -1307,9 +1303,9 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
fflush(stdout);
dup2(saved_stdout, 1);
close(saved_stdout);
- close(temporary_stdout_fd);
- print_file_to_stdout(git_path_bisect_run());
+ print_fd_to_stdout(temporary_stdout_fd);
+ close(temporary_stdout_fd);
if (res == BISECT_ONLY_SKIPPED_LEFT)
error(_("bisect run cannot continue any more"));
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/3] bisect: add --auto-reset to leave when done
From: Harald Nordgren via GitGitGadget @ 2026-07-17 18:27 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren
In-Reply-To: <pull.2335.git.git.1784180159.gitgitgadget@gmail.com>
Add a --auto-reset option to git bisect that resets the bisect session when
culprit is found.
Changes in v2:
* Add option --auto-reset[=<where>] with option to go to final commit as
well as original.
* Refactored tests.
Harald Nordgren (3):
bisect: read run output from the open descriptor
bisect: let bisect_reset() optionally check out quietly
bisect: add --auto-reset to leave when done
Documentation/git-bisect.adoc | 14 +++-
bisect.c | 2 +
builtin/bisect.c | 132 +++++++++++++++++++++++++++++-----
t/t6030-bisect-porcelain.sh | 107 +++++++++++++++++++++++++++
4 files changed, 234 insertions(+), 21 deletions(-)
base-commit: 44de1520f08d1dfebc3ab2d9f644208eaa5ac925
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2335%2FHaraldNordgren%2Fbisect-auto-reset-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2335/HaraldNordgren/bisect-auto-reset-v2
Pull-Request: https://github.com/git/git/pull/2335
Range-diff vs v1:
1: 3fd365835d = 1: 0de8b12f65 bisect: read run output from the open descriptor
2: a7670baafc = 2: 8a2dcdf305 bisect: let bisect_reset() optionally check out quietly
3: a9194b1d00 ! 3: 5b3704fbd4 bisect: add --auto-reset to leave when done
@@ Metadata
## Commit message ##
bisect: add --auto-reset to leave when done
- When a bisection finished, "git bisect" reported the first bad commit
- but left the session active until "git bisect reset" was run by hand.
+ When a bisection finishes, "git bisect" reports the first bad commit
+ but leaves the session active until "git bisect reset" is run by hand.
- Add an "--auto-reset" option, accepted by both "git bisect start" and
- "git bisect run", that resets as soon as the first bad commit is found,
- returning to the commit checked out before "git bisect start". The flag
- is persisted in a BISECT_AUTO_RESET state file and the restoring
- checkout is done quietly.
+ Add an "--auto-reset[=<where>]" option, accepted by both "git bisect
+ start" and "git bisect run", that resets as soon as the first bad commit
+ is found. The "original" value returns to the commit checked out before
+ "git bisect start", while "found" leaves the first bad commit checked
+ out; omitting the value defaults to "original".
+
+ Persist the selected target in a BISECT_AUTO_RESET state file and perform
+ the reset quietly. Reject this option together with "--no-checkout",
+ since that mode must not check out either target.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ Documentation/git-bisect.adoc: SYNOPSIS
[synopsis]
git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]
- [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]
-+ [--no-checkout] [--first-parent] [--auto-reset] [<bad> [<good>...]] [--] [<pathspec>...]
++ [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]
git bisect (bad|new|<term-new>) [<rev>]
git bisect (good|old|<term-old>) [<rev>...]
git bisect terms [--term-(good|old) | --term-(bad|new)]
@@ Documentation/git-bisect.adoc: git bisect reset [<commit>]
git bisect replay <logfile>
git bisect log
-git bisect run <cmd> [<arg>...]
-+git bisect run [--auto-reset] <cmd> [<arg>...]
++git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]
git bisect help
DESCRIPTION
@@ Documentation/git-bisect.adoc: ignored.
This option is particularly useful in avoiding false positives when a merged
branch contained broken or non-buildable commits, but the merge itself was OK.
-+`--auto-reset`::
-+ Once the first bad commit is found, clean up the bisection state and
-+ return to the commit that was checked out before `git bisect start`,
-+ as if `git bisect reset` had been run. The first bad commit is still
-+ reported before resetting.
++`--auto-reset[=<where>]`::
++ Once the first bad commit is found, report it and clean up the
++ bisection state. `<where>` may be `original` to return to the commit
++ checked out before `git bisect start`, or `found` to leave the first
++ bad commit checked out. If `<where>` is omitted, it defaults to
++ `original`.
++
-+This option may be given to `git bisect start` or to `git bisect run`.
++This option may be given to `git bisect start` or to `git bisect run`. It
++cannot be used for a bisection started with `--no-checkout`.
+
EXAMPLES
--------
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
#define BUILTIN_GIT_BISECT_START_USAGE \
N_("git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-old>]\n" \
- " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<pathspec>...]")
-+ " [--no-checkout] [--first-parent] [--auto-reset] [<bad> [<good>...]] [--] [<pathspec>...]")
++ " [--no-checkout] [--first-parent] [--auto-reset[=<where>]] [<bad> [<good>...]] [--] [<pathspec>...]")
#define BUILTIN_GIT_BISECT_BAD_USAGE \
N_("git bisect (bad|new|<term-new>) [<rev>]")
#define BUILTIN_GIT_BISECT_GOOD_USAGE \
@@ builtin/bisect.c: static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN")
"git bisect log"
#define BUILTIN_GIT_BISECT_RUN_USAGE \
- N_("git bisect run <cmd> [<arg>...]")
-+ N_("git bisect run [--auto-reset] <cmd> [<arg>...]")
++ N_("git bisect run [--auto-reset[=<where>]] <cmd> [<arg>...]")
#define BUILTIN_GIT_BISECT_HELP_USAGE \
"git bisect help"
+@@ builtin/bisect.c: static const char * const git_bisect_usage[] = {
+ NULL
+ };
+
++enum auto_reset_mode {
++ AUTO_RESET_NONE,
++ AUTO_RESET_ORIGINAL,
++ AUTO_RESET_FOUND,
++};
++
+ struct add_bisect_ref_data {
+ struct rev_info *revs;
+ unsigned int object_flags;
+@@ builtin/bisect.c: static int bisect_reset(const char *commit, int quiet)
+ return bisect_clean_state();
+ }
+
++static int parse_auto_reset(const char *value, enum auto_reset_mode *mode)
++{
++ if (!strcmp(value, "original"))
++ *mode = AUTO_RESET_ORIGINAL;
++ else if (!strcmp(value, "found"))
++ *mode = AUTO_RESET_FOUND;
++ else
++ return error(_("invalid value for '--auto-reset': '%s'"), value);
++
++ return 0;
++}
++
++static const char *auto_reset_mode_name(enum auto_reset_mode mode)
++{
++ switch (mode) {
++ case AUTO_RESET_ORIGINAL:
++ return "original";
++ case AUTO_RESET_FOUND:
++ return "found";
++ case AUTO_RESET_NONE:
++ BUG("no name for unset auto-reset mode");
++ }
++ BUG("unknown auto-reset mode %d", mode);
++}
++
++static int bisect_auto_reset(struct bisect_terms *terms)
++{
++ struct strbuf value = STRBUF_INIT;
++ enum auto_reset_mode mode;
++ char *commit = NULL;
++ int res;
++
++ if (strbuf_read_file(&value, git_path_bisect_auto_reset(), 0) < 0) {
++ res = error_errno(_("could not read '%s'"),
++ git_path_bisect_auto_reset());
++ goto cleanup;
++ }
++ strbuf_trim(&value);
++ if (parse_auto_reset(value.buf, &mode)) {
++ res = -1;
++ goto cleanup;
++ }
++
++ if (mode == AUTO_RESET_FOUND)
++ commit = xstrfmt("refs/bisect/%s", terms->term_bad);
++ res = bisect_reset(commit, 1);
++
++cleanup:
++ free(commit);
++ strbuf_release(&value);
++ return res;
++}
++
+ static void log_commit(FILE *fp,
+ const char *fmt, const char *state,
+ struct commit *commit)
@@ builtin/bisect.c: static enum bisect_error bisect_next(struct bisect_terms *terms, const char *pre
if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
res = bisect_successful(terms);
+ if (!res && !is_empty_or_missing_file(git_path_bisect_auto_reset()))
-+ res = bisect_reset(NULL, 1);
++ res = bisect_auto_reset(terms);
return res ? res : BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND;
} else if (res == BISECT_ONLY_SKIPPED_LEFT) {
res = bisect_skipped_commits(terms);
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
{
int no_checkout = 0;
int first_parent_only = 0;
-+ int auto_reset = 0;
++ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
int flags, pathspec_pos;
enum bisect_error res = BISECT_OK;
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *ter
} else if (!strcmp(arg, "--first-parent")) {
first_parent_only = 1;
+ } else if (!strcmp(arg, "--auto-reset")) {
-+ auto_reset = 1;
++ auto_reset = AUTO_RESET_ORIGINAL;
++ } else if (skip_prefix(arg, "--auto-reset=", &arg)) {
++ if (parse_auto_reset(arg, &auto_reset)) {
++ res = BISECT_FAILED;
++ goto finish;
++ }
} else if (!strcmp(arg, "--term-good") ||
!strcmp(arg, "--term-old")) {
i++;
+@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
+ break;
+ }
+ }
++ if (auto_reset != AUTO_RESET_NONE && no_checkout) {
++ res = error(_("'--auto-reset' cannot be used with '--no-checkout'"));
++ goto finish;
++ }
+ pathspec_pos = i;
+
+ /*
@@ builtin/bisect.c: static enum bisect_error bisect_start(struct bisect_terms *terms, int argc,
if (first_parent_only)
write_file(git_path_bisect_first_parent(), "\n");
-+ if (auto_reset)
-+ write_file(git_path_bisect_auto_reset(), "\n");
++ if (auto_reset != AUTO_RESET_NONE)
++ write_file(git_path_bisect_auto_reset(), "%s\n",
++ auto_reset_mode_name(auto_reset));
+
if (no_checkout) {
if (repo_get_oid(the_repository, start_head.buf, &oid) < 0) {
res = error(_("invalid ref: '%s'"), start_head.buf);
+@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
+ {
+ int res = BISECT_OK;
+ struct strbuf command = STRBUF_INIT;
++ enum auto_reset_mode auto_reset = AUTO_RESET_NONE;
++ const char *auto_reset_arg;
+ const char *new_state;
+ int temporary_stdout_fd, saved_stdout;
+ int is_first_run = 1;
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
if (bisect_next_check(terms, NULL))
return BISECT_FAILED;
-+ if (argc && !strcmp(argv[0], "--auto-reset")) {
-+ write_file(git_path_bisect_auto_reset(), "\n");
++ if (argc && !strcmp(argv[0], "--auto-reset"))
++ auto_reset = AUTO_RESET_ORIGINAL;
++ else if (argc && skip_prefix(argv[0], "--auto-reset=", &auto_reset_arg)) {
++ if (parse_auto_reset(auto_reset_arg, &auto_reset))
++ return BISECT_FAILED;
++ }
++
++ if (auto_reset != AUTO_RESET_NONE) {
++ if (refs_ref_exists(get_main_ref_store(the_repository), "BISECT_HEAD"))
++ return error(_("'--auto-reset' cannot be used with '--no-checkout'"));
++ write_file(git_path_bisect_auto_reset(), "%s\n",
++ auto_reset_mode_name(auto_reset));
+ argc--;
+ argv++;
+ }
@@ builtin/bisect.c: static int bisect_run(struct bisect_terms *terms, int argc, co
return BISECT_FAILED;
## t/t6030-bisect-porcelain.sh ##
+@@ t/t6030-bisect-porcelain.sh: test_bisect_usage () {
+ test_cmp expect actual
+ }
+
++test_bisect_state_file () {
++ test_path_is_file "$(git rev-parse --git-path "$1")"
++}
++
++test_bisect_state_missing () {
++ test_path_is_missing "$(git rev-parse --git-path "$1")"
++}
++
++bisect_start_and_finish () {
++ git bisect start "$1" $HASH4 $HASH2 &&
++ git bisect bad
++}
++
++bisect_run_auto_reset () {
++ write_script test_script.sh <<-\EOF &&
++ ! grep Another hello >/dev/null
++ EOF
++ git bisect start $HASH4 $HASH2 &&
++ git bisect run "$1" ./test_script.sh >my_bisect_log.txt &&
++ test_grep "$HASH3 is the first .bad. commit" my_bisect_log.txt
++}
++
++test_auto_reset_fails () {
++ local pattern="$1" &&
++ local state_file="$2" &&
++ shift 2 &&
++ test_must_fail "$@" 2>err &&
++ test_grep -- "$pattern" err &&
++ test_bisect_state_missing "$state_file"
++}
++
+ test_expect_success 'bisect usage' "
+ test_bisect_usage 1 git bisect reset extra1 extra2 <<-\EOF &&
+ error: 'git bisect reset' requires either no argument or a commit
@@ t/t6030-bisect-porcelain.sh: test_expect_success '"git bisect run" simple case' '
git bisect reset
'
-+test_expect_success '"git bisect start --auto-reset" leaves the bisection' '
-+ test_when_finished "git bisect reset" &&
-+ git bisect start --auto-reset $HASH4 $HASH2 &&
-+ git bisect bad &&
-+ test_path_is_missing "$(git rev-parse --git-path BISECT_START)"
++test_expect_success '"git bisect start --auto-reset" defaults to original' '
++ test_when_finished "git bisect reset; git checkout main" &&
++ git checkout main &&
++ bisect_start_and_finish --auto-reset &&
++ test "$HASH4" = "$(git rev-parse HEAD)" &&
++ test main = "$(git branch --show-current)" &&
++ test_bisect_state_missing BISECT_START &&
++
++ bisect_start_and_finish --auto-reset=original &&
++ test "$HASH4" = "$(git rev-parse HEAD)" &&
++ test main = "$(git branch --show-current)" &&
++ test_bisect_state_missing BISECT_START
+'
+
-+test_expect_success '"git bisect run --auto-reset" leaves the bisection' '
-+ test_when_finished "git bisect reset" &&
-+ write_script test_script.sh <<-\EOF &&
-+ ! grep Another hello >/dev/null
-+ EOF
++test_expect_success '"git bisect start --auto-reset=found" leaves first bad checked out' '
++ test_when_finished "git bisect reset; git checkout main" &&
++ bisect_start_and_finish --auto-reset=found &&
++ test "$HASH3" = "$(git rev-parse HEAD)" &&
++ test_bisect_state_missing BISECT_START
++'
++
++test_expect_success '"git bisect run --auto-reset" defaults to original' '
++ test_when_finished "git bisect reset; git checkout main" &&
++ bisect_run_auto_reset --auto-reset &&
++ test "$HASH4" = "$(git rev-parse HEAD)" &&
++ test main = "$(git branch --show-current)" &&
++ test_bisect_state_missing BISECT_START
++'
++
++test_expect_success '"git bisect run --auto-reset=found" leaves first bad checked out' '
++ test_when_finished "git bisect reset; git checkout main" &&
++ bisect_run_auto_reset --auto-reset=found &&
++ test "$HASH3" = "$(git rev-parse HEAD)" &&
++ test_bisect_state_missing BISECT_START
++'
++
++test_expect_success '--auto-reset rejects an unknown reset target' '
++ test_when_finished "git bisect reset; git checkout main" &&
++ test_auto_reset_fails \
++ "invalid value for.*--auto-reset.*unknown" BISECT_START \
++ git bisect start --auto-reset=unknown $HASH4 $HASH2 &&
++
+ git bisect start $HASH4 $HASH2 &&
-+ git bisect run --auto-reset ./test_script.sh >my_bisect_log.txt &&
-+ grep "$HASH3 is the first .bad. commit" my_bisect_log.txt &&
-+ test_path_is_missing "$(git rev-parse --git-path BISECT_START)"
++ test_auto_reset_fails \
++ "invalid value for.*--auto-reset.*unknown" BISECT_AUTO_RESET \
++ git bisect run --auto-reset=unknown true
++'
++
++test_expect_success '--auto-reset cannot be used with --no-checkout' '
++ test_when_finished "git bisect reset" &&
++ test_auto_reset_fails \
++ "cannot be used with.*--no-checkout" BISECT_START \
++ git bisect start --auto-reset=original --no-checkout $HASH4 $HASH2 &&
++
++ git bisect start --no-checkout $HASH4 $HASH2 &&
++ test_auto_reset_fails \
++ "cannot be used with.*--no-checkout" BISECT_AUTO_RESET \
++ git bisect run --auto-reset=found true
+'
+
+test_expect_success 'without --auto-reset the bisection state is kept' '
+ test_when_finished "git bisect reset" &&
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
-+ test_path_is_file "$(git rev-parse --git-path BISECT_START)"
++ test_bisect_state_file BISECT_START
+'
+
+test_expect_success '--auto-reset does not leak into a later bisection' '
-+ test_when_finished "git bisect reset" &&
-+ git bisect start --auto-reset $HASH4 $HASH2 &&
-+ git bisect bad &&
++ test_when_finished "git bisect reset; git checkout main" &&
++ bisect_start_and_finish --auto-reset &&
++
+ git bisect start $HASH4 $HASH2 &&
+ git bisect bad &&
-+ test_path_is_file "$(git rev-parse --git-path BISECT_START)"
++ test_bisect_state_file BISECT_START
+'
+
# We want to automatically find the commit that
--
gitgitgadget
^ permalink raw reply
* Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options
From: Siddharth Shrimali @ 2026-07-17 18:00 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, christian.couder, siddharthasthana31, me, ps,
johannes.schindelin, l.s.r
In-Reply-To: <xmqqh5lyej6f.fsf@gitster.g>
On Fri, 17 Jul 2026 at 02:38, Junio C Hamano <gitster@pobox.com> wrote:
>
> > + die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
> > + !!filter_to, "--filter-to");
> > +
> > + die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
> > + write_bitmaps > 0, "--write-bitmap-index");
>
> Hmph. Since this step does not change the parsing or configuration
> for write_bitmaps, we cannot tell if (write_bitmaps == 1) at this
> point in the execution came from the command line (e.g., an earlier
> call to parse_options() around line 247 of builtin/repack.c) or from
> the configuration files (e.g., a call to repo_config() around
> line 245). In other words, wouldn't it be ...
>
> > + write_bitmaps = 0;
>
> ... way too late to drop the flag here?
>
right, thanks! The commit message claims I distinguish an explicit
-b/--write-bitmap-index from a config-provided default, but the code
only tests write_bitmaps > 0, which cannot tell the two apart at this point
For v2, alongside other changes, i'll distinguish the two, so that an explicit
-b on the command line errors out, while a config-provided default is
silently disabled for the duration of the command..
Thanks!
Siddharth Shrimali
^ permalink raw reply
* Re: [PATCH v7] show-branch: convert per-branch flags to commit-slab
From: Junio C Hamano @ 2026-07-17 17:53 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Gatla Vishweshwar Reddy, git
In-Reply-To: <xmqqse5h63js.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
>> We've seen a strong uptick in threads that are obviously AI generated,
>> only, and at times it just feels like one is merely talking to a prompt.
>> This just doesn't scale well, as it leads to constant iterations and
>> back and forth without much thinking being involved. So we require the
>> other side to stop every once in a while and invest the necessary time,
>> too. Otherwise the community will simply stop working, and that doesn't
>> serve anyone well.
>>
>> Sorry if I came across as harsh.
>
> Thanks for saying this.
>
> When viewed in that light, what the v7 patch does is extremely
> incoherent. It gives the impression of having been generated by an
> automated assistant and sent without human oversight, or perhaps
> drafted in a state of severe late-night exhaustion. For instance,
> the commit message claims to have lifted 'max_revs' completely, yet
> the proposed documentation updates still reference a hard limit of
> 64. It also removes the local definition of 'UNINTERESTING', even
> though the comment immediately above it still advises our future
> selves to migrate to the shared definition eventually.
>
> It appears the automation was not used merely for structuring the
> reply; the changes in the patch itself show signs of having been
> generated and sent out without any human oversight X-<.
By the way, the two items that follow 'For instance' above are mere
examples. Addressing them alone will not suffice.
Like you, I have no desire to interact with a human proxy sitting
between an automated generator and the mailing list, relaying what
the machine produced under the guise of having reviewed it, without
understanding or thinking through the implications. We want to see
changes designed from the ground up by human minds, not synthetic
slop.
It is fine to use automated assistants to proofread what you have
produced yourself. To judge whether to adopt or discard their
suggestions, you must possess a deeper understanding of the problem
domain than the tools do. At the very least, to employ a 'human
produces, machine assists' workflow, you must first compose the
draft yourself before asking the tool to polish it, which inherently
requires sufficient domain knowledge and forces you to think about
the problem.
In contrast, blindly relaying what the machine generated, while
claiming to have reviewed it carefully, is something anyone can do
without even realizing how superficial that review actually was.
The 'machine generates, human reviews' model is fundamentally broken
when compared to its converse.
Pardon if this sounds even harsher.
^ permalink raw reply
* Re: [PATCH v2] copy: drop dependency on `the_repository`
From: Junio C Hamano @ 2026-07-17 17:09 UTC (permalink / raw)
To: Phillip Wood; +Cc: Patrick Steinhardt, git, Phillip Wood
In-Reply-To: <b0df688a-3b26-48f6-8b1c-98530483885e@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> Hi Patrick
>
> This version looks good to me
>
> Thanks
>
> Phillip
The changes to 'copy.c' are fairly straightforward and
trivially correct. The resulting updates to the callers
are also minor, as requiring them to pass 'the_repository'
introduces no change in behavior by definition.
Thanks. Let's mark the topic for 'next'.
^ permalink raw reply
* Re: [PATCH 3/3] bisect: add --auto-reset to leave when done
From: Junio C Hamano @ 2026-07-17 16:43 UTC (permalink / raw)
To: Harald Nordgren; +Cc: Harald Nordgren via GitGitGadget, git
In-Reply-To: <CAHwyqnWOnDoMpMP7gwrQWRmaJqBrSDpdLN0Dp7U6vPU0GKSmEg@mail.gmail.com>
Harald Nordgren <haraldnordgren@gmail.com> writes:
> That's a great idea!
May not be, though. Depending on what the bisect session finds,
where I want to reset to may probably be different. But that merely
means that in such a case, I cannot use "bisect start --autoreset".
^ permalink raw reply
* [PATCH v8] show-branch: convert per-branch flags to commit-slab
From: Gatla Vishweshwar Reddy @ 2026-07-17 16:32 UTC (permalink / raw)
To: gitster; +Cc: git, Gatla Vishweshwar Reddy
In-Reply-To: <xmqqse5h63js.fsf@gitster.g>
show-branch uses commit->object.flags to store per-branch
reachability bits, one bit per branch starting at REV_SHIFT.
The flags word has only a fixed number of available bits, limiting
the number of branches that can be shown simultaneously.
Convert the per-branch bits to a dedicated commit-slab using uint64_t
as the element type, initialized with a stride via
init_commit_rev_flags_with_stride(). Keep the UNINTERESTING bit in
object.flags where it belongs, as it is used for revision walking and
does not need to be in the per-branch slab. With UNINTERESTING removed
from the slab, REV_SHIFT becomes 0 and all 64 bits of uint64_t are
available for branch tracking.
Add helper functions get_rev_flags_ptr(), peek_rev_flags_ptr(),
has_any_rev_flags(), or_rev_flag_bit(), test_rev_flag_bit(),
has_all_rev_flags(), has_only_rev_flag_bit(), and
has_subset_rev_flags() to encapsulate per-bit slab access cleanly.
Use has_only_rev_flag_bit() in show_independent() to preserve the
original semantics: a commit is independent only if reachable from
exactly one tip. Use has_subset_rev_flags() in join_revs() to skip
parents whose flags already include all of the current commit's flags,
avoiding redundant propagation and queue thrashing in dense histories.
Fix join_revs() to correctly propagate UNINTERESTING to parents using
a local commit_is_merge_base variable, without smudging the commit
itself, matching the original behavior.
Remove the hardcoded MAX_REVS limit entirely by dynamically growing
the ref_name[] array with ALLOC_GROW, and allocating rev[] and
reflog_msg[] dynamically after all refs are collected. Compute
flags_stride at runtime as (ref_name_cnt + 63) / 64 so the slab
supports an arbitrary number of branches.
Remove the hardcoded branch limit from documentation since there is
no longer a fixed maximum. Add tests to verify show-branch works
correctly with more than 27 branches. Include revision.h for the
shared UNINTERESTING definition.
Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
> the commit message claims to have lifted 'max_revs' completely,
> yet the proposed documentation updates still reference a hard
> limit of 64.
Fixed. The sentence "It cannot show more than 64 branches and
commits at a time." is removed from the documentation entirely.
There is no longer any hardcoded limit in code or docs.
> It also removes the local definition of 'UNINTERESTING', even
> though the comment immediately above it still advises our future
> selves to migrate to the shared definition eventually.
The TODO comment is still present in v8. The local #define
UNINTERESTING was removed and revision.h is included instead,
which is what the TODO comment asks for as a future step. The
comment remains to remind future contributors to complete the
migration of UNINTERESTING usage out of object.flags entirely.
> It gives the impression of having been generated by an automated
> assistant and sent without human oversight.
Noted. I reviewed v8 carefully before sending.
---
Changes in v8:
- Remove "cannot show more than 64 branches" from documentation
- Fix commit message to accurately describe the documentation change
Documentation/git-show-branch.adoc | 2 -
builtin/show-branch.c | 234 ++++++++++++++++++-----------
t/t3202-show-branch.sh | 32 ++++
3 files changed, 179 insertions(+), 89 deletions(-)
diff --git a/Documentation/git-show-branch.adoc b/Documentation/git-show-branch.adoc
index 7e86d54a24..3df107b7f4 100644
--- a/Documentation/git-show-branch.adoc
+++ b/Documentation/git-show-branch.adoc
@@ -22,8 +22,6 @@ Shows the commit ancestry graph starting from the commits named
with <rev>s or <glob>s (or all refs under refs/heads
and/or refs/tags) semi-visually.
-It cannot show more than 26 branches and commits at a time.
-
It uses `showbranch.default` multi-valued configuration items if
no <rev> or <glob> is given on the command line.
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f02831b085..4f49a8d05b 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -9,6 +9,7 @@
#include "hex.h"
#include "pretty.h"
#include "refs.h"
+#include "revision.h"
#include "color.h"
#include "strvec.h"
#include "object-name.h"
@@ -35,15 +36,11 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
static struct strvec default_args = STRVEC_INIT;
/*
- * TODO: convert this use of commit->object.flags to commit-slab
- * instead to store a pointer to ref name directly. Then use the same
- * UNINTERESTING definition from revision.h here.
+ * TODO: store a pointer to ref name directly in the commit-slab
+ * instead, and use the UNINTERESTING definition from revision.h
+ * here once that is done.
*/
-#define UNINTERESTING 01
-
-#define REV_SHIFT 2
-#define MAX_REVS (FLAG_BITS - REV_SHIFT) /* should not exceed bits_per_int - REV_SHIFT */
-
+#define REV_SHIFT 0
#define DEFAULT_REFLOG 4
static const char *get_color_code(int idx)
@@ -79,11 +76,87 @@ struct commit_name {
define_commit_slab(commit_name_slab, struct commit_name *);
static struct commit_name_slab name_slab;
+define_commit_slab(commit_rev_flags, uint64_t);
+static struct commit_rev_flags rev_flags_slab;
+static int flags_stride; /* number of uint64_t words per commit */
+
static struct commit_name *commit_to_name(struct commit *commit)
{
return *commit_name_slab_at(&name_slab, commit);
}
+static uint64_t *get_rev_flags_ptr(struct commit *commit)
+{
+ return commit_rev_flags_at(&rev_flags_slab, commit);
+}
+
+static uint64_t *peek_rev_flags_ptr(struct commit *commit)
+{
+ return commit_rev_flags_peek(&rev_flags_slab, commit);
+}
+
+static int has_any_rev_flags(struct commit *commit)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ int i;
+ if (!f)
+ return 0;
+ for (i = 0; i < flags_stride; i++)
+ if (f[i])
+ return 1;
+ return 0;
+}
+
+static void or_rev_flag_bit(struct commit *commit, int branch)
+{
+ get_rev_flags_ptr(commit)[branch / 64] |= UINT64_C(1) << (branch % 64);
+}
+
+static int test_rev_flag_bit(struct commit *commit, int branch)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ return f && !!(f[branch / 64] & (UINT64_C(1) << (branch % 64)));
+}
+
+static int has_all_rev_flags(struct commit *commit, int num_rev)
+{
+ int i;
+ for (i = 0; i < num_rev; i++)
+ if (!test_rev_flag_bit(commit, i))
+ return 0;
+ return 1;
+}
+
+static int has_only_rev_flag_bit(struct commit *commit, int branch)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ int i;
+ if (!f)
+ return 0;
+ for (i = 0; i < flags_stride; i++) {
+ uint64_t expected = (i == branch / 64)
+ ? (UINT64_C(1) << (branch % 64))
+ : 0;
+ if (f[i] != expected)
+ return 0;
+ }
+ return 1;
+}
+
+static int has_subset_rev_flags(struct commit *src, struct commit *dst)
+{
+ uint64_t *s = peek_rev_flags_ptr(src);
+ uint64_t *d = peek_rev_flags_ptr(dst);
+ int i;
+ if (!s)
+ return 1; /* src has no flags, trivially a subset */
+ if (!d)
+ return 0; /* dst has no flags but src has some */
+ for (i = 0; i < flags_stride; i++)
+ if (s[i] & ~d[i])
+ return 0;
+ return 1;
+}
/* Name the commit as nth generation ancestor of head_name;
* we count only the first-parent relationship for naming purposes.
@@ -215,7 +288,7 @@ static void name_commits(struct commit_list *list,
static int mark_seen(struct commit *commit, struct commit_list **seen_p)
{
- if (!commit->object.flags) {
+ if (!has_any_rev_flags(commit)) {
commit_list_insert(commit, seen_p);
return 1;
}
@@ -226,39 +299,43 @@ static void join_revs(struct prio_queue *queue,
struct commit_list **seen_p,
int num_rev, int extra)
{
- int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
while (queue->nr) {
struct commit_list *parents;
int still_interesting = !!interesting(queue);
struct commit *commit = prio_queue_peek(queue);
bool get_pending = true;
- int flags = commit->object.flags & all_mask;
if (!still_interesting && extra <= 0)
break;
mark_seen(commit, seen_p);
- if ((flags & all_revs) == all_revs)
- flags |= UNINTERESTING;
- parents = commit->parents;
-
- while (parents) {
- struct commit *p = parents->item;
- int this_flag = p->object.flags;
- parents = parents->next;
- if ((this_flag & flags) == flags)
- continue;
- repo_parse_commit(the_repository, p);
- if (mark_seen(p, seen_p) && !still_interesting)
- extra--;
- p->object.flags |= flags;
- if (get_pending)
- prio_queue_replace(queue, p);
- else
- prio_queue_put(queue, p);
- get_pending = false;
+ {
+ int commit_is_merge_base = has_all_rev_flags(commit, num_rev);
+ parents = commit->parents;
+
+ while (parents) {
+ struct commit *p = parents->item;
+ parents = parents->next;
+ if (has_subset_rev_flags(commit, p) &&
+ (!commit_is_merge_base || (p->object.flags & UNINTERESTING)))
+ continue;
+ repo_parse_commit(the_repository, p);
+ if (mark_seen(p, seen_p) && !still_interesting)
+ extra--;
+ {
+ int _b;
+ for (_b = 0; _b < num_rev; _b++)
+ if (test_rev_flag_bit(commit, _b))
+ or_rev_flag_bit(p, _b);
+ }
+ if (commit_is_merge_base)
+ p->object.flags |= UNINTERESTING;
+ if (get_pending)
+ prio_queue_replace(queue, p);
+ else
+ prio_queue_put(queue, p);
+ get_pending = false;
+ }
}
if (get_pending)
prio_queue_get(queue);
@@ -278,7 +355,7 @@ static void join_revs(struct prio_queue *queue,
struct commit *c = s->item;
struct commit_list *parents;
- if (((c->object.flags & all_revs) != all_revs) &&
+ if (!has_all_rev_flags(c, num_rev) &&
!(c->object.flags & UNINTERESTING))
continue;
@@ -335,8 +412,9 @@ static void show_one_commit(struct commit *commit, int no_name)
strbuf_release(&pretty);
}
-static char *ref_name[MAX_REVS + 1];
+static char **ref_name;
static int ref_name_cnt;
+static int ref_name_alloc;
static const char *find_digit_prefix(const char *s, int *v)
{
@@ -409,12 +487,7 @@ static int append_ref(const char *refname, const struct object_id *oid,
if (!strcmp(refname, ref_name[i]))
return 0;
}
- if (MAX_REVS <= ref_name_cnt) {
- warning(Q_("ignoring %s; cannot handle more than %d ref",
- "ignoring %s; cannot handle more than %d refs",
- MAX_REVS), refname, MAX_REVS);
- return 0;
- }
+ ALLOC_GROW(ref_name, ref_name_cnt + 2, ref_name_alloc);
ref_name[ref_name_cnt++] = xstrdup(refname);
ref_name[ref_name_cnt] = NULL;
return 0;
@@ -511,15 +584,12 @@ static int rev_is_head(const char *head, const char *name)
static int show_merge_base(const struct commit_list *seen, int num_rev)
{
- int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
int exit_status = 1;
for (const struct commit_list *s = seen; s; s = s->next) {
struct commit *commit = s->item;
- int flags = commit->object.flags & all_mask;
- if (!(flags & UNINTERESTING) &&
- ((flags & all_revs) == all_revs)) {
+ if (!(commit->object.flags & UNINTERESTING) &&
+ has_all_rev_flags(commit, num_rev)) {
puts(oid_to_hex(&commit->object.oid));
exit_status = 0;
commit->object.flags |= UNINTERESTING;
@@ -528,17 +598,13 @@ static int show_merge_base(const struct commit_list *seen, int num_rev)
return exit_status;
}
-static int show_independent(struct commit **rev,
- int num_rev,
- unsigned int *rev_mask)
+static int show_independent(struct commit **rev, int num_rev)
{
int i;
for (i = 0; i < num_rev; i++) {
struct commit *commit = rev[i];
- unsigned int flag = rev_mask[i];
-
- if (commit->object.flags == flag)
+ if (has_only_rev_flag_bit(commit, i))
puts(oid_to_hex(&commit->object.oid));
commit->object.flags |= UNINTERESTING;
}
@@ -560,8 +626,7 @@ static void append_one_rev(const char *av)
match_ref_slash = count_slashes(av);
refs_for_each_ref(get_main_ref_store(the_repository),
append_matching_ref, NULL);
- if (saved_matches == ref_name_cnt &&
- ref_name_cnt < MAX_REVS)
+ if (saved_matches == ref_name_cnt)
error(_("no matching refs with %s"), av);
sort_ref_range(saved_matches, ref_name_cnt);
return;
@@ -603,13 +668,12 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
* Otherwise, if it is a merge that is reachable from only one
* tip, it is not that interesting.
*/
- int i, flag, count;
+ int i, count;
for (i = 0; i < n; i++)
if (rev[i] == commit)
return 0;
- flag = commit->object.flags;
for (i = count = 0; i < n; i++) {
- if (flag & (1u << (i + REV_SHIFT)))
+ if (test_rev_flag_bit(commit, i))
count++;
}
if (count == 1)
@@ -644,14 +708,12 @@ int cmd_show_branch(int ac,
const char *prefix,
struct repository *repo UNUSED)
{
- struct commit *rev[MAX_REVS], *commit;
- char *reflog_msg[MAX_REVS] = {0};
+ struct commit **rev = NULL, *commit;
+ char **reflog_msg = NULL;
struct commit_list *seen = NULL;
struct prio_queue queue = { compare_commits_by_commit_date };
- unsigned int rev_mask[MAX_REVS];
int num_rev, i, extra = 0;
int all_heads = 0, all_remotes = 0;
- int all_mask, all_revs;
enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
char *head;
struct object_id head_oid;
@@ -713,8 +775,6 @@ int cmd_show_branch(int ac,
const char **args_copy = NULL;
int ret;
- init_commit_name_slab(&name_slab);
-
repo_config(the_repository, git_show_branch_config, NULL);
/* If nothing is specified, try the default first */
@@ -778,10 +838,7 @@ int cmd_show_branch(int ac,
if (ac != 1)
die(_("--reflog option needs one branch name"));
- if (MAX_REVS < reflog)
- die(Q_("only %d entry can be shown at one time.",
- "only %d entries can be shown at one time.",
- MAX_REVS), MAX_REVS);
+
if (!repo_dwim_ref(the_repository, *av, strlen(*av), &oid,
&ref, 0))
die(_("no such ref %s"), *av);
@@ -868,14 +925,17 @@ int cmd_show_branch(int ac,
goto out;
}
+ flags_stride = (ref_name_cnt + 63) / 64;
+ if (!flags_stride)
+ flags_stride = 1;
+ init_commit_rev_flags_with_stride(&rev_flags_slab, flags_stride);
+ CALLOC_ARRAY(rev, ref_name_cnt);
+ CALLOC_ARRAY(reflog_msg, ref_name_cnt);
+
for (num_rev = 0; ref_name[num_rev]; num_rev++) {
struct object_id revkey;
- unsigned int flag = 1u << (num_rev + REV_SHIFT);
+ int first_seen;
- if (MAX_REVS <= num_rev)
- die(Q_("cannot handle more than %d rev.",
- "cannot handle more than %d revs.",
- MAX_REVS), MAX_REVS);
if (repo_get_oid(the_repository, ref_name[num_rev], &revkey))
die(_("'%s' is not a valid ref."), ref_name[num_rev]);
commit = lookup_commit_reference(the_repository, &revkey);
@@ -885,17 +945,15 @@ int cmd_show_branch(int ac,
repo_parse_commit(the_repository, commit);
mark_seen(commit, &seen);
- /* rev#0 uses bit REV_SHIFT, rev#1 uses bit REV_SHIFT+1,
- * and so on. REV_SHIFT bits from bit 0 are used for
- * internal bookkeeping.
+ /* rev#0 uses bit 0, rev#1 uses bit 1,
+ * and so on. All bits are available for branch tracking.
*/
- commit->object.flags |= flag;
- if (commit->object.flags == flag)
+ first_seen = !has_any_rev_flags(commit);
+ or_rev_flag_bit(commit, num_rev);
+ if (first_seen)
prio_queue_put(&queue, commit);
rev[num_rev] = commit;
}
- for (i = 0; i < num_rev; i++)
- rev_mask[i] = rev[i]->object.flags;
if (0 <= extra)
join_revs(&queue, &seen, num_rev, extra);
@@ -908,7 +966,7 @@ int cmd_show_branch(int ac,
}
if (independent) {
- ret = show_independent(rev, num_rev, rev_mask);
+ ret = show_independent(rev, num_rev);
goto out;
}
@@ -958,13 +1016,9 @@ int cmd_show_branch(int ac,
if (!sha1_name && !no_name)
name_commits(seen, rev, ref_name, num_rev);
- all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
for (struct commit_list *l = seen; l; l = l->next) {
struct commit *commit = l->item;
- int this_flag = commit->object.flags;
- int is_merge_point = ((this_flag & all_revs) == all_revs);
+ int is_merge_point = has_all_rev_flags(commit, num_rev);
shown_merge_point |= is_merge_point;
@@ -973,14 +1027,14 @@ int cmd_show_branch(int ac,
commit->parents->next);
if (topics &&
!is_merge_point &&
- (this_flag & (1u << REV_SHIFT)))
+ test_rev_flag_bit(commit, 0))
continue;
if (!sparse && is_merge &&
omit_in_dense(commit, rev, num_rev))
continue;
for (i = 0; i < num_rev; i++) {
int mark;
- if (!(this_flag & (1u << (i + REV_SHIFT))))
+ if (!test_rev_flag_bit(commit, i))
mark = ' ';
else if (is_merge)
mark = '-';
@@ -1006,10 +1060,16 @@ int cmd_show_branch(int ac,
ret = 0;
out:
- for (size_t i = 0; i < ARRAY_SIZE(reflog_msg); i++)
+ for (i = 0; i < ref_name_cnt; i++)
free(reflog_msg[i]);
+ free(reflog_msg);
+ free(rev);
+ for (i = 0; i < ref_name_cnt; i++)
+ free(ref_name[i]);
+ free(ref_name);
commit_list_free(seen);
clear_prio_queue(&queue);
+ clear_commit_rev_flags(&rev_flags_slab);
free(args_copy);
free(head);
return ret;
diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh
index a1139f79e2..d04f642998 100755
--- a/t/t3202-show-branch.sh
+++ b/t/t3202-show-branch.sh
@@ -283,4 +283,36 @@ test_expect_success '--reflog handles missing reflog' '
test_must_be_empty actual
'
+test_expect_success 'show-branch with 30 branches succeeds' '
+ git checkout initial &&
+ for i in $(test_seq 11 30)
+ do
+ git checkout -b branch$i initial &&
+ test_commit --no-tag branch$i || return 1
+ done &&
+ git show-branch $(git for-each-ref \
+ --sort=version:refname \
+ --format="%(refname:strip=2)" \
+ "refs/heads/branch*") >actual &&
+ test_line_count -ge 30 actual
+'
+
+test_expect_success 'show-branch --independent with 30 branches' '
+ git show-branch --independent $(git for-each-ref \
+ --sort=version:refname \
+ --format="%(refname:strip=2)" \
+ "refs/heads/branch*") >actual &&
+ test_line_count -ge 30 actual
+'
+
+test_expect_success 'show-branch --merge-base with 30 branches' '
+ git rev-parse initial >expect &&
+ git show-branch --merge-base $(git for-each-ref \
+ --sort=version:refname \
+ --format="%(refname:strip=2)" \
+ "refs/heads/branch*") >actual &&
+ test_cmp expect actual
+'
+
+
test_done
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] userdiff: add support for Swift
From: Junio C Hamano @ 2026-07-17 16:27 UTC (permalink / raw)
To: Shlok Kulshreshtha
Cc: git, D. Ben Knoble, René Scharfe, Johannes Sixt,
Eric Sunshine, Scott L. Burson
In-Reply-To: <20260717140232.6722-1-diy2903@gmail.com>
Shlok Kulshreshtha <diy2903@gmail.com> writes:
> Add a built-in userdiff driver for the Swift programming language so that
> diff hunk headers and word diffs work out of the box for ".swift" files.
I do not work with this language myself, so I have no idea how well
the pattern proposed here matches real-world code. Will queue, but
it needs reviews and validaion from active Swift users before it can
advance to 'next' and eventually to a future Git release.
Thanks.
^ permalink raw reply
* Re: [PATCH] trace2: tolerate failed timestamp formatting
From: Taylor Blau @ 2026-07-17 16:24 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget; +Cc: git, gitster, Derrick Stolee
In-Reply-To: <pull.2178.git.1784131932489.gitgitgadget@gmail.com>
On Wed, Jul 15, 2026 at 04:12:11PM +0000, Derrick Stolee via GitGitGadget wrote:
> This change removes all uses of xsnprintf() from the trace2/ directory.
> There are two uses of xstrdup() that could be considered for removal,
> but they only die() on out-of-memory errors instead of formatting
> issues. I chose to leave those in place for now.
I may be missing some Git for Windows context, but I dug into this a
little and I'm not sure 'gettimeofday()' is the culprit...
In my understanding Git for Windows's 'gettext.h' appears[1] to redirect
the 'vsnprintf()' inside 'xsnprintf()' to 'libintl_vsnprintf()'. In this
case, we have seven '%' placeholders. Gettext can store only six plus
its end marker inline, so parsing the seventh causes an allocation
before any timestamp values are read.
A failure there would produce the observed -1, after which 'xsnprintf()'
dies and trace2 can recurse.
I think that also explains why calling 'snprintf()' directly helps.
tr2_tbuf.c doesn't include gettext.h, so I think it bypasses libintl. If
I'm reading compat/mingw.c correctly, 'gettimeofday()' fills tv and
always returns zero [2], making the zero-initialization unrelated.
Would it make more sense to fix the xsnprintf()/libintl boundary and
treat Trace2 reentrancy separately? I still can't explain why the
allocation failed, so there may be another GfW-specific piece I’m
missing.
I think something like the following (untested) would prevent the
redirection to `libintl_vsnprintf()`:
--- 8< ---
diff --git a/wrapper.c b/wrapper.c
index 16f5a63fbb..2976d4e110 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -7,7 +7,14 @@
#include "git-compat-util.h"
#include "abspath.h"
#include "parse.h"
+
+/*
+ * xsnprintf() only formats non-translated strings. On MinGW, avoid
+ * redirecting its vsnprintf() call to libintl's allocating replacement.
+ */
+#define _INTL_NO_DEFINE_MACRO_VSNPRINTF
#include "gettext.h"
+#undef _INTL_NO_DEFINE_MACRO_VSNPRINTF
#include "strbuf.h"
#include "trace2.h"
--- >8 ---
Thanks,
Taylor
[1]: https://github.com/git-for-windows/git-sdk-64/blob/1351ad2fc39a1f74c56b2cc2b38107ec8df8eb40/mingw64/include/libintl.h#L731-L754
[2]: https://github.com/microsoft/git/blob/vfs-2.55.0/compat/mingw.c#L1609-L1618
^ permalink raw reply related
* Re: Please provide help with how to fix
From: Weijie Yuan @ 2026-07-17 16:12 UTC (permalink / raw)
To: Randy Kroeger; +Cc: D. Ben Knoble, git@vger.kernel.org
In-Reply-To: <BY5PR06MB6548A6BD9BFC90367098FC6EBAC62@BY5PR06MB6548.namprd06.prod.outlook.com>
On Fri, Jul 17, 2026 at 03:40:56PM +0000, Randy Kroeger wrote:
> HI Ben,
>
> I have been a software developer for 30 years and in the last seven,
> have been an independent contractor. Trust me. You read too much
> into my intro in giving advice. However, I can offer advice when it
> comes to emails in the old fashioned text format, please avoid
> interleaving your responses. Depending on the device today, it is as
> confusing as what you mentioned my diagram being... haha...
Hi Randy,
I highly respect your experience. But interleaving is how Git operates,
and it surely includes the Linux kernel as well.
There are many online resources explaining this. Here's one, please take
a look ;-)
https://useplaintext.email/#etiquette
And by the way, you didn't wrap your text. This is another established
rule that has become conventional over time.
Thanks.
^ permalink raw reply
* [PATCH 2/2] rebase: remember fixup -c after skipping fixup/squash
From: Phillip Wood @ 2026-07-17 16:06 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
In-Reply-To: <cover.1784304378.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
When the final command in a chain of "fixup" and "squash" commands
is skipped, we should prompt the user to edit the commit message
if the chain contains a "fixup -c" command that was not skipped.
Unfortunately, commit_staged_changes() only looks for completed "squash"
commands and so does not prompt the user to edit the message. Fix
this by recording whether a fixup command has the "-c" flag set and
then checking whether we have seen either a "fixup -c" or a "squash"
command. Add regression tests for skipping a command in the middle
of the chain (which currently works but has no test coverage), and
for skipping the final command (which is fixed by this patch).
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 20 +++++++++++---
t/t3437-rebase-fixup-options.sh | 47 +++++++++++++++++++++++++++++++++
2 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index af3d2c72616..25ef076216c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1924,6 +1924,13 @@ static int seen_squash(struct replay_ctx *ctx)
{
return starts_with(ctx->current_fixups.buf, "squash") ||
strstr(ctx->current_fixups.buf, "\nsquash");
+}
+
+/* Does the current fixup chain contain a "fixup -c" command? */
+static int seen_fixup_edit_msg(struct replay_ctx *ctx)
+{
+ return starts_with(ctx->current_fixups.buf, "fixup -c") ||
+ strstr(ctx->current_fixups.buf, "\nfixup -c");
}
static void update_comment_bufs(struct strbuf *buf1, struct strbuf *buf2, int n)
@@ -2148,9 +2155,14 @@ static int update_squash_messages(struct repository *r,
strbuf_release(&buf);
if (!res) {
- strbuf_addf(&ctx->current_fixups, "%s%s %s",
+ const char *fixup_flag = "";
+
+ if (is_fixup_flag(command, flag) && (flag & TODO_EDIT_FIXUP_MSG))
+ fixup_flag = " -c";
+
+ strbuf_addf(&ctx->current_fixups, "%s%s%s %s",
ctx->current_fixups.len ? "\n" : "",
- command_to_string(command),
+ command_to_string(command), fixup_flag,
oid_to_hex(&commit->object.oid));
res = write_message(ctx->current_fixups.buf,
ctx->current_fixups.len,
@@ -5391,8 +5403,8 @@ static int commit_staged_changes(struct repository *r,
* message, no need to bother the user with
* opening the commit message in the editor.
*/
- if (!starts_with(p, "squash ") &&
- !strstr(p, "\nsquash "))
+ if (!seen_squash(ctx) &&
+ !seen_fixup_edit_msg(ctx))
flags = (flags & ~EDIT_MSG) | CLEANUP_MSG;
} else if (is_fixup(peek_command(todo_list, 0))) {
/*
diff --git a/t/t3437-rebase-fixup-options.sh b/t/t3437-rebase-fixup-options.sh
index 5d306a47692..a4b2a631654 100755
--- a/t/t3437-rebase-fixup-options.sh
+++ b/t/t3437-rebase-fixup-options.sh
@@ -184,6 +184,53 @@ test_expect_success 'multiple fixup -c opens editor once' '
get_author HEAD >actual-author &&
test_cmp expected-author actual-author &&
test_commit_message HEAD expected-message
+'
+
+test_expect_success 'fixup -c is remembered after skipping final fixup' '
+ test_when_finished "test_might_fail git rebase --abort" &&
+ cat >todo <<-\EOF &&
+ pick B
+ fixup -c A1
+ fixup A3
+ EOF
+ (
+ set_fake_editor &&
+ set_replace_editor todo &&
+ test_must_fail git rebase -i A A &&
+ git show && cat .git/rebase-merge/message-squash &&
+ FAKE_COMMIT_AMEND=edited git rebase --skip
+ ) &&
+ test_commit_message HEAD <<-\EOF
+ new subject
+
+ new
+ body
+
+ edited
+ EOF
+'
+test_expect_success 'fixup -c is remembered after skipping later fixup' '
+ test_when_finished "test_might_fail git rebase --abort" &&
+ cat >todo <<-\EOF &&
+ pick B
+ fixup -c A1
+ fixup A3
+ fixup A2
+ EOF
+ (
+ set_fake_editor &&
+ set_replace_editor todo &&
+ test_must_fail git rebase -i A A &&
+ FAKE_COMMIT_AMEND=edited git rebase --skip
+ ) &&
+ test_commit_message HEAD <<-\EOF
+ new subject
+
+ new
+ body
+
+ edited
+ EOF
'
test_expect_success 'sequence squash, fixup & fixup -c gives combined message' '
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 1/2] rebase -i: fix counting of fixups after rebase --skip
From: Phillip Wood @ 2026-07-17 16:06 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
In-Reply-To: <cover.1784304378.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
When the sequencer processes a chain of "fixup" and "squash" commands
it keeps a list of the commands that have been executed. If there are
conflicts, then the list is saved when the rebase stops for the user to
resolve them. When the rebase resumes, the list is loaded and is used
to initialize the count of how many "fixup" and "squash" commands have
been processed; if a command has been skipped with "git rebase --skip",
then the last command needs to be popped off the end of the list.
To count the number of commands, commit_staged_changes() uses the
number of newlines in the file plus one. This is due to the slightly
unusual way the list is constructed - instead of appending a newline
when a command is added, a newline is inserted before the command
if the current count is greater than zero. Therefore, when we pop a
skipped command off the list, we should also remove the newline that
precedes it. Otherwise, when a new command is added, a blank line
will be left before it, which will contribute to the fixup count the
next time the file is read. Unfortunately, the preceding newline is
not removed, leading to an incorrect count. Fix this by removing the
newline that appears before the skipped command.
In addition to fixing the code that removes a skipped command from the
list, the code that reads the list is fixed to skip blank lines. We
have had reports of users starting a rebase with one version of
git and continuing it with another. Often this happens because the
version of git bundled with an IDE or TUI differs from the one used
at the command line. By fixing both the reading and writing ends of
the problem we ensure the count is correct when an older version of
git reads the fixup file written by a newer version and vice versa.
Triggering the incorrect count requires the user to skip two "fixup" or
"squash" commands before the final command in the chain. An existing
test is extended to prevent future regressions. The consequence of
miscounting is not serious: we just print the wrong count in the
header of the commit message template.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 11 ++++++++++-
t/t3418-rebase-continue.sh | 36 ++++++++++++++++++++++++++++++++----
2 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 1355a99a092..af3d2c72616 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3281,7 +3281,13 @@ static int read_populate_opts(struct replay_opts *opts)
const char *p = ctx->current_fixups.buf;
ctx->current_fixup_count = 1;
while ((p = strchr(p, '\n'))) {
- ctx->current_fixup_count++;
+ /*
+ * Older versions of git accidentally
+ * inserted blank lines when a fixup
+ * was skipped.
+ */
+ if (p[1] != '\n')
+ ctx->current_fixup_count++;
p++;
}
}
@@ -5353,6 +5359,9 @@ static int commit_staged_changes(struct repository *r,
if (!len)
BUG("Incorrect current_fixups:\n%s", p);
while (len && p[len - 1] != '\n')
+ len--;
+ /* Remove trailing newline */
+ if (len)
len--;
strbuf_setlen(&ctx->current_fixups, len);
if (write_message(p, len, rebase_path_current_fixups(),
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index f9b8999db50..3c248e97364 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -134,6 +134,7 @@ test_expect_success '--skip after failed fixup cleans commit message' '
EOF
: skip and continue &&
+ test_config commit.status false &&
echo "cp \"\$1\" .git/copy.txt" | write_script copy-editor.sh &&
(test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) &&
@@ -145,7 +146,8 @@ test_expect_success '--skip after failed fixup cleans commit message' '
: now, let us ensure that "squash" is handled correctly &&
git reset --hard wants-fixup-3 &&
- test_must_fail env FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1" \
+ test_must_fail env \
+ FAKE_LINES="1 squash 2 squash 1 squash 3 squash 1 squash 4 squash 1" \
git rebase -i HEAD~4 &&
: the second squash failed, but there are two more in the chain &&
@@ -171,19 +173,45 @@ test_expect_success '--skip after failed fixup cleans commit message' '
fixup 2
EOF
+ (test_set_editor "$PWD/copy-editor.sh" &&
+ test_must_fail git rebase --skip) &&
+ : not the final squash, no need to edit the commit message &&
+ test_path_is_missing .git/copy.txt &&
+
+ : The first, third and fifth squashes succeeded, therefore: &&
+ cat >expect <<-\EOF &&
+ # This is a combination of 4 commits.
+ # This is the 1st commit message:
+
+ wants-fixup
+
+ # This is the commit message #2:
+
+ fixup 1
+
+ # This is the commit message #3:
+
+ fixup 2
+
+ # This is the commit message #4:
+
+ fixup 3
+ EOF
+ test_commit_message HEAD expect &&
+
(test_set_editor "$PWD/copy-editor.sh" && git rebase --skip) &&
test_commit_message HEAD <<-\EOF &&
wants-fixup
fixup 1
fixup 2
+
+ fixup 3
EOF
: Final squash failed, but there was still a squash &&
- head -n1 .git/copy.txt >first-line &&
- test_grep "# This is a combination of 3 commits" first-line &&
- test_grep "# This is the commit message #3:" .git/copy.txt
+ test_cmp expect .git/copy.txt
'
test_expect_success 'setup rerere database' '
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 0/2] rebase: a couple of fixup fixes
From: Phillip Wood @ 2026-07-17 16:06 UTC (permalink / raw)
To: git; +Cc: Phillip Wood
These patches fix a couple of small bugs in the way skipped "fixup"
and "squash" commands are handled. A skipped command can lead to
an incorrect commit count in the template message which is fixed in
patch 1. It can also mean we fail to open the editor after a "fixup
-c" command which is fixed in patch 2
base-commit: d35c5399e3e54ac277bb391fc2f6be3e816d312b
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Frebase-fixup-fixes-part-1%2Fv1
View-Changes-At: https://github.com/phillipwood/git/compare/d35c5399e...7c8075ff2
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/rebase-fixup-fixes-part-1/v1
Phillip Wood (2):
rebase -i: fix counting of fixups after rebase --skip
rebase: remember fixup -c after skipping fixup/squash
sequencer.c | 31 ++++++++++++++++++----
t/t3418-rebase-continue.sh | 36 ++++++++++++++++++++++---
t/t3437-rebase-fixup-options.sh | 47 +++++++++++++++++++++++++++++++++
3 files changed, 105 insertions(+), 9 deletions(-)
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply
* Re: [PATCH v7 3/4] environment: move trust_executable_bit into repo_config_values
From: Junio C Hamano @ 2026-07-17 16:01 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, ps, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260717063559.1633567-4-cat@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
> diff --git a/environment.c b/environment.c
> index fc3ed8bb1c..75069a884d 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -41,7 +41,6 @@
> static int pack_compression_seen;
> static int zlib_compression_seen;
>
> -int trust_executable_bit = 1;
> int trust_ctime = 1;
> int check_stat = 1;
> int has_symlinks = 1;
> @@ -142,6 +141,13 @@ int is_bare_repository(void)
> return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
> }
>
> +int repo_trust_executable_bit(struct repository *repo)
> +{
> + return repo->gitdir?
> + repo_config_values(repo)->trust_executable_bit :
> + 1;
> +}
> +
> int have_git_dir(void)
> {
> return startup_info->have_repository
Two comments.
* Missing SP before '?'. It may be easier to read if it is written
like this:
return repo->gitdir
? repo_config_values(repo)->trust_executable_bit
: 1;
which more clearly highlights the ternary structure. If you tilt
your head 90 degrees to the left, you can almost see the parse
tree of the expression.
* Does it make sense to protect against a NULL 'repo' case, as
repo_protect_ntfs() and repo_protect_hfs() helpers do? Or is it
better to crash loudly with a segfault to let the developer know
they have a bug to fix? I lean toward the latter myself, and if
we go that route, we should probably stop using 'repo &&
repo->gitdir' elsewhere, rather than sweeping the problem under
the rug with defensive checks.
^ permalink raw reply
* [PATCH 4/4] last-modified: keep per-path Bloom filters for wildcard pathspecs
From: Toon Claes @ 2026-07-17 15:47 UTC (permalink / raw)
To: git; +Cc: Gusted, Jeff King, Toon Claes
In-Reply-To: <20260717-toon-speed-up-last-modified-v1-0-410418f18614@iotcl.com>
The last-modified builtin expands the pathspec to a set of literal paths
and builds a Bloom key for each. During the walk it looks those keys up
in the commit's filter to decide whether the commit is worth diffing.
These lookups need `bloom_filter_settings` for the key hashing.
prepare_revision_walk() runs prepare_to_use_bloom_filter() to build the
pathspec key vectors. For a pathspec that cannot be turned into a Bloom
key, such as a top-level wildcard like "*.c", that function gives up and
clears `bloom_filter_settings`.
Restore `bloom_filter_settings` after prepare_revision_walk() so the
per-path check keeps working for wildcard pathspecs.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/last-modified.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/builtin/last-modified.c b/builtin/last-modified.c
index e8ee610404..adc7cd8c74 100644
--- a/builtin/last-modified.c
+++ b/builtin/last-modified.c
@@ -360,6 +360,14 @@ static int last_modified_run(struct last_modified *lm)
prepare_revision_walk(&lm->rev);
+ /*
+ * prepare_revision_walk() clears bloom_filter_settings for pathspecs
+ * without a Bloom key. Restore it so the per-path check keeps working.
+ */
+ if (!lm->rev.bloom_filter_settings)
+ lm->rev.bloom_filter_settings =
+ get_bloom_filter_settings(lm->rev.repo);
+
max_count = lm->rev.max_count;
init_active_paths_for_commit(&lm->active_paths);
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH 3/4] last-modified: check pathspec against Bloom filter first
From: Toon Claes @ 2026-07-17 15:47 UTC (permalink / raw)
To: git; +Cc: Gusted, Jeff King, Toon Claes
In-Reply-To: <20260717-toon-speed-up-last-modified-v1-0-410418f18614@iotcl.com>
When git-last-modified(1) starts, it builds a list of all the paths
matching the pathspec it needs to find the last modifying commit for.
For example, every file and subdirectory listed by:
$ git last-modified -t --max-depth=0 -- src/
As it resolves a commit for each path during the revision walk, it drops
that path from the list.
To avoid diffing trees for every commit, Bloom filters are used when
available. For each remaining path, the commit's Bloom filter is checked
to see whether the commit changed that path. The Bloom filter says
either "no" or "maybe", and only in the latter case is the diff
calculated.
git-log(1) does this differently. It does not expand the pathspec but
checks the Bloom filter against the pathspec itself. This way, commits
not touching any path matching the pathspec can be discarded as a whole.
Apply this same check to git-last-modified(1). In a previous commit the
function revs_maybe_changed_in_bloom(), used by git-log(1), was made
public. Use this as a pre-filter in git-last-modified(1). After this
pre-filter, paths are still checked one-by-one to only find those which
don't have a "last commit" yet.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
builtin/last-modified.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/builtin/last-modified.c b/builtin/last-modified.c
index 5478182f2e..e8ee610404 100644
--- a/builtin/last-modified.c
+++ b/builtin/last-modified.c
@@ -272,6 +272,9 @@ static bool maybe_changed_path(struct last_modified *lm,
if (!filter)
return true;
+ if (revs_maybe_changed_in_bloom(&lm->rev, filter) == 0)
+ return false;
+
hashmap_for_each_entry(&lm->paths, &iter, ent, hashent) {
if (active && !bitmap_get(active, ent->diff_idx))
continue;
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH 2/4] revision: expose check for paths maybe changed in Bloom filter
From: Toon Claes @ 2026-07-17 15:47 UTC (permalink / raw)
To: git; +Cc: Gusted, Jeff King, Toon Claes
In-Reply-To: <20260717-toon-speed-up-last-modified-v1-0-410418f18614@iotcl.com>
check_maybe_different_in_bloom_filter() looks up a commit's changed-path
Bloom filter and consults it to see whether the commit might have
modified any of the paths in the pathspec that `revs` was set up with.
In a follow-up commit we want to reuse this logic from another builtin.
That caller, however, has already looked up the commit's Bloom filter
for its own purposes, so having the function look it up again would mean
a redundant lookup.
Extract the filter-consulting part into a new public function,
revs_maybe_changed_in_bloom(). This function takes an already looked-up
`struct bloom_filter` instead of a commit.
The existing check_maybe_different_in_bloom_filter() becomes a thin
wrapper that looks up the filter and delegates.
Expose the new function via revision.h so other builtins can reuse the
exact same filtering that `git log <pathspec>` performs.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
revision.c | 31 +++++++++++++++++++++----------
revision.h | 17 +++++++++++++++++
2 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/revision.c b/revision.c
index f3c9407a66..040b30b5ee 100644
--- a/revision.c
+++ b/revision.c
@@ -748,26 +748,20 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs,
struct commit *commit)
{
struct bloom_filter *filter;
- int result = 0;
-
- if (!revs->bloom_keyvecs_nr)
- return -1;
+ int result;
if (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY)
return -1;
filter = get_bloom_filter(revs->repo, commit);
-
if (!filter) {
count_bloom_filter_not_present++;
return -1;
}
- for (size_t nr = 0; !result && nr < revs->bloom_keyvecs_nr; nr++) {
- result = bloom_filter_contains_vec(filter,
- revs->bloom_keyvecs[nr],
- revs->bloom_filter_settings);
- }
+ result = revs_maybe_changed_in_bloom(revs, filter);
+ if (result < 0)
+ return result;
if (result)
count_bloom_filter_maybe++;
@@ -777,6 +771,23 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs,
return result;
}
+int revs_maybe_changed_in_bloom(struct rev_info *revs,
+ struct bloom_filter *filter)
+{
+ int result = 0;
+
+ if (!revs->bloom_keyvecs_nr)
+ return -1;
+
+ for (size_t nr = 0; !result && nr < revs->bloom_keyvecs_nr; nr++) {
+ result = bloom_filter_contains_vec(filter,
+ revs->bloom_keyvecs[nr],
+ revs->bloom_filter_settings);
+ }
+
+ return result;
+}
+
static int rev_compare_tree(struct rev_info *revs,
struct commit *parent, struct commit *commit, int nth_parent)
{
diff --git a/revision.h b/revision.h
index 569b3fa1cb..7569c210cc 100644
--- a/revision.h
+++ b/revision.h
@@ -68,6 +68,7 @@ struct string_list;
struct saved_parents;
struct follow_pathspec_slab;
struct bloom_keyvec;
+struct bloom_filter;
struct bloom_filter_settings;
struct option;
struct parse_opt_ctx_t;
@@ -493,6 +494,22 @@ void reset_revision_walk(void);
*/
int prepare_revision_walk(struct rev_info *revs);
+/**
+ * Take in a changed-path Bloom filter that belongs to a commit, and consult it
+ * to see if it might have modified any of the paths in the `revs`.
+ * The caller should look up `filter`, probably with get_bloom_filter().
+ * prepare_revision_walk() needs to be called in advance to ensure
+ * pathspec key vectors are set up.
+ *
+ * Returns -1 if no sensible answer could be given because of missing
+ * preconditions (no pathspec key vectors).
+ * Returns 0 if the commit definitely did not change any of the paths and 1 if
+ * the commit maybe has changed one of them, although that might be a
+ * false-positive.
+ */
+int revs_maybe_changed_in_bloom(struct rev_info *revs,
+ struct bloom_filter *filter);
+
/* Drain the commits linked list into the priority queue. */
void rev_info_commit_list_to_queue(struct rev_info *revs);
/**
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH 1/4] revision: move bloom keyvec precondition into function
From: Toon Claes @ 2026-07-17 15:46 UTC (permalink / raw)
To: git; +Cc: Gusted, Jeff King, Toon Claes
In-Reply-To: <20260717-toon-speed-up-last-modified-v1-0-410418f18614@iotcl.com>
There are currently two callsites calling
check_maybe_different_in_bloom_filter(). They both check if
revs->bloom_keyvecs_nr is not zero before they call that function.
Move bloom_keyvecs_nr precondition into
check_maybe_different_in_bloom_filter() to simplify the code.
Note that this changes `bloom_ret` to become -1 when there are no Bloom
key vectors, which results in `count_bloom_filter_false_positive` not
being incremented. This is unobservable, as the Bloom statistics are
only reported when key vectors were set up.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
revision.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index 137a86d33b..f3c9407a66 100644
--- a/revision.c
+++ b/revision.c
@@ -750,6 +750,9 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs,
struct bloom_filter *filter;
int result = 0;
+ if (!revs->bloom_keyvecs_nr)
+ return -1;
+
if (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY)
return -1;
@@ -804,7 +807,7 @@ static int rev_compare_tree(struct rev_info *revs,
return REV_TREE_SAME;
}
- if (revs->bloom_keyvecs_nr && !nth_parent) {
+ if (!nth_parent) {
bloom_ret = check_maybe_different_in_bloom_filter(revs, commit);
if (bloom_ret == 0)
@@ -831,7 +834,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit,
if (!t1)
return 0;
- if (!nth_parent && revs->bloom_keyvecs_nr) {
+ if (!nth_parent) {
bloom_ret = check_maybe_different_in_bloom_filter(revs, commit);
if (!bloom_ret)
return 1;
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH 0/4] last-modified: use the pathspec's Bloom key to pre-filter commits
From: Toon Claes @ 2026-07-17 15:46 UTC (permalink / raw)
To: git; +Cc: Gusted, Jeff King, Toon Claes
We have received a report[1] git-last-modified(1) is slow compared to
git-log(1) if you want to find the last commit for all entries in a
directory. For example running the following command on ziglang/zig[2]:
$ git last-modified -t --max-depth=0 $OID -- doc/langref/
Turns out to find results about 2.5 times slower than:
$ git log --name-status -c --format=commit%x00%H %P%x00" \
--parents --no-renames -t -z $OID -- :(literal)doc/langref
Now the latter needs some post-processing to come to the same results,
the total solution still is faster than integrating
git-last-modified(1).
After some research we've discovered the Bloom filters aren't used
optimally. But it turns out the code powering git-log(1) can fairly easy
be reused. We do this in a few steps:
- Patch 1 moves a condition around so it becomes deduplicated and
eventually can be reused by git-last-modified(1).
- Patch 2 exposes a helper from revision.c publicly. The function is
split out so the Bloom filter wouldn't be looked up twice from
git-last-modified(1).
- Patch 3 uses this new helper in git-last-modified(1).
- Patch 4 is bonus change, which optimizes when working with wildcard
pathspecs.
Below are benchmark on the ziglang/zig repository for the `doc/langref/`
directory (with commit-graphs writting using `--changed-paths`):
Benchmark 1: master last-modified
Time (mean ± σ): 52.6 ms ± 4.0 ms [User: 49.2 ms, System: 3.0 ms]
Range (min … max): 48.2 ms … 73.8 ms 62 runs
Benchmark 2: HEAD last-modified
Time (mean ± σ): 14.3 ms ± 1.8 ms [User: 12.0 ms, System: 2.1 ms]
Range (min … max): 10.5 ms … 18.9 ms 182 runs
Benchmark 3: git log
Time (mean ± σ): 17.4 ms ± 1.4 ms [User: 13.5 ms, System: 3.7 ms]
Range (min … max): 15.0 ms … 26.1 ms 185 runs
Summary
HEAD last-modified ran
1.22 ± 0.18 times faster than git log
3.66 ± 0.55 times faster than master last-modified
Similar timings are seen across a few other repositories (like GitLab's
monolith gitlab-org/gitlab)
[1]: https://lore.kernel.org/git/17f356ff-7bfb-47f5-b714-62a95cc8b821@codeberg.org/
[2]: https://codeberg.org/ziglang/zig
---
Toon Claes (4):
revision: move bloom keyvec precondition into function
revision: expose check for paths maybe changed in Bloom filter
last-modified: check pathspec against Bloom filter first
last-modified: keep per-path Bloom filters for wildcard pathspecs
builtin/last-modified.c | 11 +++++++++++
revision.c | 32 +++++++++++++++++++++++---------
revision.h | 17 +++++++++++++++++
3 files changed, 51 insertions(+), 9 deletions(-)
---
base-commit: 55526a18268bbc1ddaf8a6b7850c33d984eac9e9
change-id: 20260716-toon-speed-up-last-modified-b04ea1f21831
^ 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