* [PATCH GSoC v19 12/13] cat-file: add remote-object-info to batch-command
From: Pablo Sabater @ 2026-07-17 15:05 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon
In-Reply-To: <20260717-ps-eric-work-rebase-v19-0-d4faee35764b@gmail.com>
From: Eric Ju <eric.peijian@gmail.com>
Since the info command in cat-file --batch-command prints object
info for a given object, it is natural to add another command in
cat-file --batch-command to print object info for a given object
from a remote.
Add remote-object-info command to cat-file --batch-command.
While info takes object ids one at a time, this creates overhead when
making requests to a server. So remote-object-info instead can take
multiple object ids at once.
The cat-file --batch-command command is generally implemented in the
following manner:
- Receive and parse input from user
- Call respective function attached to command
- Get object info, print object info
In --buffer mode, this changes to:
- Receive and parse input from user
- Store respective function attached to command in a queue
- After flush, loop through commands in queue
- Call respective function attached to command
- Get object info, print object info
Notice how the getting and printing of object info is accomplished one
at a time. As described above, this creates a problem for making
requests to a server. Therefore, remote-object-info is implemented in
the following manner:
- Receive and parse input from user
If command is remote-object-info:
- Get object info from remote
- Loop through and print each object info
Else:
- Call respective function attached to command
- Parse input, get object info, print object info
And finally for --buffer mode remote-object-info:
- Receive and parse input from user
- Store respective function attached to command in a queue
- After flush, loop through commands in queue:
If command is remote-object-info:
- Get object info from remote
- Loop through and print each object info
Else:
- Call respective function attached to command
- Get object info, print object info
To summarize, remote-object-info gets object info from the remote and
then loops through the object info passed in, printing the info.
In order for remote-object-info to avoid remote communication
overhead in the non-buffer mode, the objects are passed in as such:
remote-object-info <remote> <oid> <oid> ... <oid>
rather than
remote-object-info <remote> <oid>
remote-object-info <remote> <oid>
...
remote-object-info <remote> <oid>
Placeholders in the format are validated against an allow-list of the
atoms the remote path supports: "objectname" and "objectsize".
Unsupported atoms expand to an empty string, honoring how for-each-ref
handles known but inapplicable atoms.
Without this, atoms like %(objecttype) would mark data->info.typep and
because the server only sends size, type_name() would later crash.
As extra safety, even outside of the remote path, initialize
expand_data's type to OBJ_BAD and handle type_name() returning NULL.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
[pablo: added the atom allow-list validation]
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Documentation/git-cat-file.adoc | 28 +-
builtin/cat-file.c | 179 +++++++-
object-file.c | 10 +
odb.h | 3 +
t/meson.build | 1 +
t/t1017-cat-file-remote-object-info.sh | 719 +++++++++++++++++++++++++++++++++
6 files changed, 929 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
index 86b9181599..7522e24874 100644
--- a/Documentation/git-cat-file.adoc
+++ b/Documentation/git-cat-file.adoc
@@ -169,6 +169,13 @@ info <object>::
Print object info for object reference `<object>`. This corresponds to the
output of `--batch-check`.
+remote-object-info <remote> <object>...::
+ Print object info for object references `<object>` at specified
+ `<remote>` without downloading objects from the remote.
+ Raise an error when the `object-info` capability is not supported by the remote.
+ Raise an error when no object references are provided.
+ This command may be combined with `--buffer`.
+
flush::
Used with `--buffer` to execute all preceding commands that were issued
since the beginning or since the last flush was issued. When `--buffer`
@@ -301,7 +308,8 @@ one per line, and print information based on the command given. With
`--batch-command`, the `info` command followed by an object will print
information about the object the same way `--batch-check` would, and the
`contents` command followed by an object prints contents in the same way
-`--batch` would.
+`--batch` would. The `remote-object-info` command followed by a remote and
+objects IDs prints object info from the remote without downloading the objects.
You can specify the information shown for each object by using a custom
`<format>`. The `<format>` is copied literally to stdout for each
@@ -324,14 +332,12 @@ newline. The available atoms are:
reports).
`objectsize:disk`::
- The size, in bytes, that the object takes up on disk. See the
- note about on-disk sizes in the `CAVEATS` section below.
+ The size, in bytes, that the object takes up on disk.
`deltabase`::
If the object is stored as a delta on-disk, this expands to the
full hex representation of the delta base object name.
- Otherwise, expands to the null OID (all zeroes). See `CAVEATS`
- below.
+ Otherwise, expands to the null OID (all zeroes).
`rest`::
If this atom is used in the output string, input lines are split
@@ -340,8 +346,14 @@ newline. The available atoms are:
after that first run of whitespace (i.e., the "rest" of the
line) are output in place of the `%(rest)` atom.
+The command `remote-object-info` only supports the `%(objectname)` and
+`%(objectsize)` placeholders. See `CAVEATS` below for more information.
+
If no format is specified, the default format is `%(objectname)
-%(objecttype) %(objectsize)`.
+%(objecttype) %(objectsize)`, except for `remote-object-info` commands which
+use `%(objectname) %(objectsize)` because "%(objecttype)" is not supported yet.
+WARNING: When "%(objecttype)" is supported, the default format WILL be unified,
+so DO NOT RELY on the current default format to stay the same!!!
If `--batch` is specified, or if `--batch-command` is used with the `contents`
command, the object information is followed by the object contents (consisting
@@ -438,6 +450,10 @@ scripting purposes.
CAVEATS
-------
+Note that only `%(objectname)` and `%(objectsize)` are currently
+supported by the `remote-object-info` command. Using any other placeholder in
+the format string will return an empty string in its position.
+
Note that the sizes of objects on disk are reported accurately, but care
should be taken in drawing conclusions about which refs or objects are
responsible for disk usage. The size of a packed non-delta object may be
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 03afc44c5e..f5e5528a21 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -29,6 +29,22 @@
#include "promisor-remote.h"
#include "mailmap.h"
#include "write-or-die.h"
+#include "alias.h"
+#include "remote.h"
+#include "transport.h"
+
+/*
+ * Maximum length for a remote URL. While no universal standard exists,
+ * 8K is assumed to be a reasonable limit.
+ */
+#define MAX_REMOTE_URL_LEN (8 * 1024)
+
+/* Maximum number of objects allowed in a single remote-object-info request. */
+#define MAX_ALLOWED_OBJ_LIMIT 10000
+
+/* Maximum input size permitted for the remote-object-info command. */
+#define MAX_REMOTE_OBJ_INFO_LINE \
+ (MAX_REMOTE_URL_LEN + MAX_ALLOWED_OBJ_LIMIT * (GIT_MAX_HEXSZ + 1))
enum batch_mode {
BATCH_MODE_CONTENTS,
@@ -317,8 +333,18 @@ struct expand_data {
* optimized out.
*/
unsigned skip_object_info : 1;
+
+ /*
+ * Flags about when an object info is being fetched from remote.
+ */
+ unsigned is_remote:1;
+};
+#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
+
+static const char *remote_object_info_atoms[] = {
+ "objectname",
+ "objectsize",
};
-#define EXPAND_DATA_INIT { .mode = S_IFINVALID }
static int is_atom(const char *atom, const char *s, int slen)
{
@@ -329,14 +355,31 @@ static int is_atom(const char *atom, const char *s, int slen)
static int expand_atom(struct strbuf *sb, const char *atom, int len,
struct expand_data *data)
{
+ if (data->is_remote) {
+ size_t i, allowed_nr = ARRAY_SIZE(remote_object_info_atoms);
+ for (i = 0; i < allowed_nr; i++)
+ if (is_atom(remote_object_info_atoms[i], atom, len))
+ break;
+
+ /*
+ * On remote, skip unsupported atoms returning an empty sb,
+ * honoring how for-each-ref handles known but inapplicable
+ * atoms (e.g. %(tagger)).
+ */
+ if (i == allowed_nr)
+ return 1;
+ }
+
if (is_atom("objectname", atom, len)) {
if (!data->mark_query)
strbuf_add_oid_hex(sb, &data->oid);
} else if (is_atom("objecttype", atom, len)) {
- if (data->mark_query)
+ if (data->mark_query) {
data->info.typep = &data->type;
- else
- strbuf_addstr(sb, type_name(data->type));
+ } else {
+ const char *t = type_name(data->type);
+ strbuf_addstr(sb, t ? t : "");
+ }
} else if (is_atom("objectsize", atom, len)) {
if (data->mark_query)
data->info.sizep = &data->size;
@@ -636,6 +679,65 @@ static void batch_one_object(const char *obj_name,
object_context_release(&ctx);
}
+static int get_remote_info(int argc,
+ const char **argv,
+ struct object_info **remote_object_info,
+ struct oid_array *object_info_oids)
+{
+ int retval = 0;
+ struct remote *remote = NULL;
+ struct object_id oid;
+ struct string_list object_info_options = STRING_LIST_INIT_NODUP;
+ struct transport *gtransport;
+
+ remote = remote_get(argv[0]);
+ if (!remote)
+ die(_("must supply valid remote when using remote-object-info"));
+
+ oid_array_clear(object_info_oids);
+ for (size_t i = 1; i < argc; i++) {
+ if (get_oid_hex(argv[i], &oid)) {
+ size_t len = strlen(argv[i]);
+
+ if (len < the_hash_algo->hexsz && len >= 4) {
+ size_t j;
+ for (j = 0; j < len; j++)
+ if (!isxdigit(argv[i][j]))
+ break;
+ if (j == len)
+ die(_("remote-object-info does not support "
+ "short oids, %d characters required"),
+ (int)the_hash_algo->hexsz);
+ }
+ die(_("not a valid object name '%s'"), argv[i]);
+ }
+ oid_array_append(object_info_oids, &oid);
+ }
+
+ if (!object_info_oids->nr)
+ die(_("remote-object-info requires objects"));
+
+ gtransport = transport_get(remote, NULL);
+
+ if (!gtransport->smart_options) {
+ retval = -1;
+ goto cleanup;
+ }
+
+ CALLOC_ARRAY(*remote_object_info, object_info_oids->nr);
+ gtransport->smart_options->object_info_oids = object_info_oids;
+
+ string_list_append(&object_info_options, "size");
+
+ gtransport->smart_options->object_info_options = &object_info_options;
+ gtransport->smart_options->object_info_data = *remote_object_info;
+ retval = transport_fetch_object_info(gtransport);
+cleanup:
+ string_list_clear(&object_info_options, 0);
+ transport_disconnect(gtransport);
+ return retval;
+}
+
struct object_cb_data {
struct batch_options *opt;
struct expand_data *expand;
@@ -717,6 +819,72 @@ static void parse_cmd_mailmap(struct batch_options *opt UNUSED,
load_mailmap();
}
+static void parse_cmd_remote_object_info(struct batch_options *opt,
+ const char *line, struct strbuf *output,
+ struct expand_data *data)
+{
+ int count;
+ const char **argv;
+ char *line_to_split;
+ struct object_info *remote_object_info = NULL;
+ struct oid_array object_info_oids = OID_ARRAY_INIT;
+ const char *saved_format = opt->format;
+
+ if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
+ die(_("remote-object-info command too long"));
+ /*
+ * TODO: Use the default format once %(objecttype) is supported.
+ */
+ if (!opt->format)
+ opt->format = "%(objectname) %(objectsize)";
+
+ line_to_split = xstrdup(line);
+ count = split_cmdline(line_to_split, &argv);
+ if (count < 0)
+ die(_("remote-object-info: %s"), split_cmdline_strerror(count));
+ if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
+ die(_("remote-object-info supports at most %d objects"),
+ MAX_ALLOWED_OBJ_LIMIT);
+
+ if (get_remote_info(count, argv, &remote_object_info,
+ &object_info_oids))
+ die(_("failed to get object info from the remote: %s"), argv[0]);
+
+ data->skip_object_info = 1;
+ for (size_t i = 0; i < object_info_oids.nr; i++) {
+ data->oid = object_info_oids.oid[i];
+
+ if (remote_object_info[i].unrecognized) {
+ report_object_status(opt, oid_to_hex(&data->oid),
+ &data->oid, "missing");
+ continue;
+ }
+
+ if (remote_object_info[i].sizep) {
+ /*
+ * When reaching here, it means remote-object-info can retrieve
+ * information from server without downloading them.
+ */
+ data->size = *remote_object_info[i].sizep;
+ opt->batch_mode = BATCH_MODE_INFO;
+ data->is_remote = 1;
+ batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
+ data->is_remote = 0;
+ } else {
+ report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
+ }
+ }
+ data->skip_object_info = 0;
+ opt->format = saved_format;
+
+ for (size_t i = 0; i < object_info_oids.nr; i++)
+ free_object_info_contents(&remote_object_info[i]);
+ free(line_to_split);
+ free(argv);
+ free(remote_object_info);
+ oid_array_clear(&object_info_oids);
+}
+
static void dispatch_calls(struct batch_options *opt,
struct strbuf *output,
struct expand_data *data,
@@ -747,9 +915,10 @@ static const struct parse_cmd {
unsigned takes_args;
} commands[] = {
{ "contents", parse_cmd_contents, 1 },
- { "info", parse_cmd_info, 1 },
{ "flush", NULL, 0 },
+ { "info", parse_cmd_info, 1 },
{ "mailmap", parse_cmd_mailmap, 1 },
+ { "remote-object-info", parse_cmd_remote_object_info, 1 },
};
static void batch_objects_command(struct batch_options *opt,
diff --git a/object-file.c b/object-file.c
index 6453b1d6fa..07f019a0f6 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1694,3 +1694,13 @@ struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
return &transaction->base;
}
+
+void free_object_info_contents(struct object_info *object_info)
+{
+ if (!object_info)
+ return;
+ free(object_info->typep);
+ free(object_info->sizep);
+ free(object_info->disk_sizep);
+ free(object_info->delta_base_oid);
+}
diff --git a/odb.h b/odb.h
index 88a37febbf..92fa414e2c 100644
--- a/odb.h
+++ b/odb.h
@@ -623,4 +623,7 @@ void parse_alternates(const char *string,
const char *relative_base,
struct strvec *out);
+/* Free pointers inside of object_info, but not object_info itself */
+void free_object_info_contents(struct object_info *object_info);
+
#endif /* ODB_H */
diff --git a/t/meson.build b/t/meson.build
index 8ae6ab6c5f..10241e3dcc 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -171,6 +171,7 @@ integration_tests = [
't1014-read-tree-confusing.sh',
't1015-read-index-unmerged.sh',
't1016-compatObjectFormat.sh',
+ 't1017-cat-file-remote-object-info.sh',
't1020-subdirectory.sh',
't1022-read-tree-partial-clone.sh',
't1050-large.sh',
diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
new file mode 100755
index 0000000000..edc20394d8
--- /dev/null
+++ b/t/t1017-cat-file-remote-object-info.sh
@@ -0,0 +1,719 @@
+#!/bin/sh
+
+test_description='git cat-file --batch-command with remote-object-info command'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-cat-file.sh
+
+hello_content="Hello World"
+hello_size=$(strlen "$hello_content")
+hello_oid=$(echo_without_newline "$hello_content" | git hash-object --stdin)
+hello_short_oid=$(git rev-parse --short "$hello_oid")
+
+unstored_content="Hello Git"
+unstored_oid=$(echo_without_newline "$unstored_content" | git hash-object --stdin)
+
+# This is how we get 13:
+# 13 = <file mode> + <a_space> + <file name> + <a_null>, where
+# file mode is 100644, which is 6 characters;
+# file name is hello, which is 5 characters
+# a space is 1 character and a null is 1 character
+tree_size=$(($(test_oid rawsz) + 13))
+
+commit_message="Initial commit"
+
+# This is how we get 137:
+# 137 = <tree header> + <a_space> + <a newline> +
+# <Author line> + <a newline> +
+# <Committer line> + <a newline> +
+# <a newline> +
+# <commit message length>
+# An easier way to calculate is: 1. use `git cat-file commit <commit hash> | wc -c`,
+# to get 177, 2. then deduct 40 hex characters to get 137
+commit_size=$(($(test_oid hexsz) + 137))
+
+tag_header_without_oid="type blob
+tag hellotag
+tagger $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+tag_header_without_timestamp="object $hello_oid
+$tag_header_without_oid"
+tag_description="This is a tag"
+tag_content="$tag_header_without_timestamp 0 +0000
+
+$tag_description"
+
+tag_oid=$(echo_without_newline "$tag_content" | git hash-object -t tag --stdin -w)
+tag_size=$(strlen "$tag_content")
+
+set_transport_variables () {
+ hello_oid=$(echo_without_newline "$hello_content" | git hash-object --stdin)
+ tree_oid=$(git -C "$1" write-tree)
+ commit_oid=$(echo_without_newline "$commit_message" | git -C "$1" commit-tree $tree_oid)
+ tag_oid=$(echo_without_newline "$tag_content" | git -C "$1" hash-object -t tag --stdin -w)
+ tag_size=$(strlen "$tag_content")
+}
+
+# This section tests --batch-command with remote-object-info command
+# Since "%(objecttype)" is currently not supported by the command remote-object-info ,
+# the filters are set to "%(objectname) %(objectsize)" in some test cases.
+
+# Test --batch-command remote-object-info with 'git://' transport with
+# transfer.advertiseobjectinfo set to true, i.e. server has object-info capability
+. "$TEST_DIRECTORY"/lib-git-daemon.sh
+start_git_daemon --export-all
+daemon_parent=$GIT_DAEMON_DOCUMENT_ROOT_PATH/parent
+
+test_expect_success 'create repo to be served by git-daemon' '
+ git init "$daemon_parent" &&
+ echo_without_newline "$hello_content" > $daemon_parent/hello &&
+ git -C "$daemon_parent" update-index --add hello &&
+ git -C "$daemon_parent" config transfer.advertiseobjectinfo true &&
+ git clone "$GIT_DAEMON_URL/parent" -n "$daemon_parent/daemon_client_empty"
+'
+
+test_expect_success 'batch-command remote-object-info git://' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF &&
+ remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid
+ remote-object-info "$GIT_DAEMON_URL/parent" $tree_oid
+ remote-object-info "$GIT_DAEMON_URL/parent" $commit_oid
+ remote-object-info "$GIT_DAEMON_URL/parent" $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command remote-object-info git:// multiple sha1 per line' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF &&
+ remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid $tree_oid $commit_oid $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command remote-object-info git:// default filter' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ git cat-file --batch-command >actual <<-EOF &&
+ remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid $tree_oid
+ remote-object-info "$GIT_DAEMON_URL/parent" $commit_oid $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'remote-object-info does not change the default format of info' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ local_content="local object" &&
+ local_oid=$(echo_without_newline "$local_content" | git hash-object -w --stdin) &&
+ local_size=$(strlen "$local_content") &&
+
+ echo "$local_oid blob $local_size" >expect &&
+ echo "$hello_oid $hello_size" >>expect &&
+ echo "$local_oid blob $local_size" >>expect &&
+
+ git cat-file --batch-command >actual <<-EOF &&
+ info $local_oid
+ remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid
+ info $local_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command --buffer remote-object-info git://' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" --buffer >actual <<-EOF &&
+ remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid $tree_oid
+ remote-object-info "$GIT_DAEMON_URL/parent" $commit_oid $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ flush
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command -Z remote-object-info git:// default filter' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ printf "%s\0" "$hello_oid $hello_size" >expect &&
+ printf "%s\0" "$tree_oid $tree_size" >>expect &&
+ printf "%s\0" "$commit_oid $commit_size" >>expect &&
+ printf "%s\0" "$tag_oid $tag_size" >>expect &&
+
+ printf "%s\0" "$hello_oid missing" >>expect &&
+ printf "%s\0" "$tree_oid missing" >>expect &&
+ printf "%s\0" "$commit_oid missing" >>expect &&
+ printf "%s\0" "$tag_oid missing" >>expect &&
+
+ batch_input="remote-object-info $GIT_DAEMON_URL/parent $hello_oid $tree_oid
+remote-object-info $GIT_DAEMON_URL/parent $commit_oid $tag_oid
+info $hello_oid
+info $tree_oid
+info $commit_oid
+info $tag_oid
+" &&
+ echo_without_newline_nul "$batch_input" >commands_null_delimited &&
+
+ git cat-file --batch-command -Z < commands_null_delimited >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'remote-object-info does not support short oids' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ test_must_fail git cat-file --batch-command 2>err <<-EOF &&
+ remote-object-info $GIT_DAEMON_URL/parent $hello_short_oid
+ EOF
+ test_grep "does not support short oids" err
+ )
+'
+
+test_expect_success 'remote-object-info does not die on missing oid like info' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ git cat-file --batch-command >local <<-EOF &&
+ info $unstored_oid
+ EOF
+ git cat-file --batch-command >remote <<-EOF &&
+ remote-object-info $GIT_DAEMON_URL/parent $unstored_oid
+ EOF
+ test_cmp local remote
+ )
+'
+
+# This tests depends on %(objecttype) not being supported yet, once supported
+# it needs to be updated.
+test_expect_success 'unsupported placeholder on remote returns empty string' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ echo "" >expect &&
+ git cat-file --batch-command="%(objecttype)" >actual <<-EOF &&
+ remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+# Test --batch-command remote-object-info with 'git://' and
+# transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability
+test_expect_success 'batch-command remote-object-info git:// fails when transfer.advertiseobjectinfo=false' '
+ (
+ git -C "$daemon_parent" config transfer.advertiseobjectinfo false &&
+ set_transport_variables "$daemon_parent" &&
+
+ test_must_fail git cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF &&
+ remote-object-info $GIT_DAEMON_URL/parent $hello_oid $tree_oid $commit_oid $tag_oid
+ EOF
+ test_grep "object-info capability is not enabled on the server" err &&
+
+ # revert server state back
+ git -C "$daemon_parent" config transfer.advertiseobjectinfo true
+
+ )
+'
+
+stop_git_daemon
+
+# Test --batch-command remote-object-info with 'file://' transport with
+# transfer.advertiseobjectinfo set to true, i.e. server has object-info capability
+# shellcheck disable=SC2016
+test_expect_success 'create repo to be served by file:// transport' '
+ git init server &&
+ git -C server config protocol.version 2 &&
+ git -C server config transfer.advertiseobjectinfo true &&
+ echo_without_newline "$hello_content" > server/hello &&
+ git -C server update-index --add hello &&
+ git clone -n "file://$(pwd)/server" file_client_empty
+'
+
+test_expect_success 'batch-command remote-object-info file://' '
+ (
+ set_transport_variables "server" &&
+ server_path="$(pwd)/server" &&
+ cd file_client_empty &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF &&
+ remote-object-info "file://${server_path}" $hello_oid
+ remote-object-info "file://${server_path}" $tree_oid
+ remote-object-info "file://${server_path}" $commit_oid
+ remote-object-info "file://${server_path}" $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command remote-object-info file:// multiple sha1 per line' '
+ (
+ set_transport_variables "server" &&
+ server_path="$(pwd)/server" &&
+ cd file_client_empty &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF &&
+ remote-object-info "file://${server_path}" $hello_oid $tree_oid $commit_oid $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command --buffer remote-object-info file://' '
+ (
+ set_transport_variables "server" &&
+ server_path="$(pwd)/server" &&
+ cd file_client_empty &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" --buffer >actual <<-EOF &&
+ remote-object-info "file://${server_path}" $hello_oid $tree_oid
+ remote-object-info "file://${server_path}" $commit_oid $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ flush
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command remote-object-info file:// default filter' '
+ (
+ set_transport_variables "server" &&
+ server_path="$(pwd)/server" &&
+ cd file_client_empty &&
+
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ git cat-file --batch-command >actual <<-EOF &&
+ remote-object-info "file://${server_path}" $hello_oid $tree_oid
+ remote-object-info "file://${server_path}" $commit_oid $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command -Z remote-object-info file:// default filter' '
+ (
+ set_transport_variables "server" &&
+ server_path="$(pwd)/server" &&
+ cd file_client_empty &&
+
+ printf "%s\0" "$hello_oid $hello_size" >expect &&
+ printf "%s\0" "$tree_oid $tree_size" >>expect &&
+ printf "%s\0" "$commit_oid $commit_size" >>expect &&
+ printf "%s\0" "$tag_oid $tag_size" >>expect &&
+
+ printf "%s\0" "$hello_oid missing" >>expect &&
+ printf "%s\0" "$tree_oid missing" >>expect &&
+ printf "%s\0" "$commit_oid missing" >>expect &&
+ printf "%s\0" "$tag_oid missing" >>expect &&
+
+ batch_input="remote-object-info \"file://${server_path}\" $hello_oid $tree_oid
+remote-object-info \"file://${server_path}\" $commit_oid $tag_oid
+info $hello_oid
+info $tree_oid
+info $commit_oid
+info $tag_oid
+" &&
+ echo_without_newline_nul "$batch_input" >commands_null_delimited &&
+
+ git cat-file --batch-command -Z < commands_null_delimited >actual &&
+ test_cmp expect actual
+ )
+'
+
+# Test --batch-command remote-object-info with 'file://' and
+# transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability
+test_expect_success 'batch-command remote-object-info file:// fails when transfer.advertiseobjectinfo=false' '
+ (
+ set_transport_variables "server" &&
+ server_path="$(pwd)/server" &&
+ git -C "${server_path}" config transfer.advertiseobjectinfo false &&
+
+ test_must_fail git cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF &&
+ remote-object-info "file://${server_path}" $hello_oid $tree_oid $commit_oid $tag_oid
+ EOF
+ test_grep "object-info capability is not enabled on the server" err &&
+
+ # revert server state back
+ git -C "${server_path}" config transfer.advertiseobjectinfo true
+ )
+'
+
+# Test --batch-command remote-object-info with 'http://' transport with
+# transfer.advertiseobjectinfo set to true, i.e. server has object-info capability
+
+. "$TEST_DIRECTORY"/lib-httpd.sh
+start_httpd
+
+test_expect_success 'create repo to be served by http:// transport' '
+ git init "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config http.receivepack true &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config transfer.advertiseobjectinfo true &&
+ echo_without_newline "$hello_content" > $HTTPD_DOCUMENT_ROOT_PATH/http_parent/hello &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" update-index --add hello &&
+ git clone "$HTTPD_URL/smart/http_parent" -n "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty"
+'
+
+test_expect_success 'batch-command remote-object-info http://' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
+ remote-object-info "$HTTPD_URL/smart/http_parent" $tree_oid
+ remote-object-info "$HTTPD_URL/smart/http_parent" $commit_oid
+ remote-object-info "$HTTPD_URL/smart/http_parent" $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command remote-object-info http:// one line' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" >actual <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid $tree_oid $commit_oid $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command --buffer remote-object-info http://' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" &&
+
+ # These results prove remote-object-info can get object info from the remote
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ # These results prove remote-object-info did not download objects from the remote
+ echo "$hello_oid missing" >>expect &&
+ echo "$tree_oid missing" >>expect &&
+ echo "$commit_oid missing" >>expect &&
+ echo "$tag_oid missing" >>expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize)" --buffer >actual <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid $tree_oid
+ remote-object-info "$HTTPD_URL/smart/http_parent" $commit_oid $tag_oid
+ info $hello_oid
+ info $tree_oid
+ info $commit_oid
+ info $tag_oid
+ flush
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command remote-object-info http:// default filter' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" &&
+
+ echo "$hello_oid $hello_size" >expect &&
+ echo "$tree_oid $tree_size" >>expect &&
+ echo "$commit_oid $commit_size" >>expect &&
+ echo "$tag_oid $tag_size" >>expect &&
+
+ git cat-file --batch-command >actual <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid $tree_oid
+ remote-object-info "$HTTPD_URL/smart/http_parent" $commit_oid $tag_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'batch-command -Z remote-object-info http:// default filter' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_client_empty" &&
+
+ printf "%s\0" "$hello_oid $hello_size" >expect &&
+ printf "%s\0" "$tree_oid $tree_size" >>expect &&
+ printf "%s\0" "$commit_oid $commit_size" >>expect &&
+ printf "%s\0" "$tag_oid $tag_size" >>expect &&
+
+ batch_input="remote-object-info $HTTPD_URL/smart/http_parent $hello_oid $tree_oid
+remote-object-info $HTTPD_URL/smart/http_parent $commit_oid $tag_oid
+" &&
+ echo_without_newline_nul "$batch_input" >commands_null_delimited &&
+
+ git cat-file --batch-command -Z < commands_null_delimited >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'remote-object-info fails on unsupported filter option (objectsize:disk)' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+
+ echo "$hello_oid " >expect &&
+
+ git cat-file --batch-command="%(objectname) %(objectsize:disk)" >actual <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'remote-object-info fails on unsupported filter option (deltabase)' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+
+ echo "" >expect &&
+
+ git cat-file --batch-command="%(deltabase)" >actual <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'remote-object-info fails on server with legacy protocol' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+
+ test_must_fail git -c protocol.version=0 cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
+ EOF
+ test_grep "object-info requires protocol v2" err
+ )
+'
+
+test_expect_success 'remote-object-info fails on server with legacy protocol with default filter' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+
+ test_must_fail git -c protocol.version=0 cat-file --batch-command 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
+ EOF
+ test_grep "object-info requires protocol v2" err
+ )
+'
+
+test_expect_success 'remote-object-info fails on malformed OID' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ malformed_object_id="this_id_is_not_valid" &&
+
+ test_must_fail git cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $malformed_object_id
+ EOF
+ test_grep "not a valid object name '$malformed_object_id'" err
+ )
+'
+
+test_expect_success 'remote-object-info fails on malformed OID with default filter' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ malformed_object_id="this_id_is_not_valid" &&
+
+ test_must_fail git cat-file --batch-command 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $malformed_object_id
+ EOF
+ test_grep "not a valid object name '$malformed_object_id'" err
+ )
+'
+
+test_expect_success 'remote-object-info fails on not providing OID' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+
+ test_must_fail git cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent"
+ EOF
+ test_grep "remote-object-info requires objects" err
+ )
+'
+
+
+# Test --batch-command remote-object-info with 'http://' transport and
+# transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability
+test_expect_success 'batch-command remote-object-info http:// fails when transfer.advertiseobjectinfo=false ' '
+ (
+ set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config transfer.advertiseobjectinfo false &&
+
+ test_must_fail git cat-file --batch-command="%(objectname) %(objectsize)" 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid $tree_oid $commit_oid $tag_oid
+ EOF
+ test_grep "object-info capability is not enabled on the server" err &&
+
+ # revert server state back
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config transfer.advertiseobjectinfo true
+ )
+'
+
+# DO NOT add non-httpd-specific tests here, because the last part of this
+# test script is only executed when httpd is available and enabled.
+
+test_done
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 13/13] cat-file: make remote-object-info allow-list adapt to the server
From: Pablo Sabater @ 2026-07-17 15:05 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon
In-Reply-To: <20260717-ps-eric-work-rebase-v19-0-d4faee35764b@gmail.com>
The static allow-list in expand_atom() is hardcoded to allow only
"objectname" and "objectsize" for remote queries. This works because,
up to this point, servers will either support object-info with name
and size or they do not support them at all.
As object-info gains new capabilities, we cannot expect different
servers with different Git versions to have the same object-info
capabilities. Therefore, the client needs to adapt its allow-list to
what the server advertises.
The client now:
1. Requests the protocol option that the placeholder refers to (i.e.
"size" for "%(objectsize)").
2. Drops any requested option that the server does not advertise in
fetch_object_info().
3. Maps the remaining advertised options back to their placeholders and
populates remote_allowed_atoms.
4. Uses remote_allowed_atoms in expand_atom(), preserving the previous
behavior for supported placeholders.
For example, if the client requests "%(objectsize) %(objecttype)" and
the server only supports 'size', then the client only requests 'size'.
The server returns the size (i.e "42") "%(objectsize)" is expanded
normally while "%(objecttype)" expands to an empty string:
"42 "
Note that the empty string expansion is only for known but unsupported
placeholders. "%(objectcolor)" which doesn't exist would die().
This honors what for-each-ref does for known but inapplicable atoms
(placeholders).
Move object_info_options out of get_remote_info() so the caller which
has data can select what options will be requested instead of requesting
always size.
Move batch_object_write() out so output is always produced.
If there are no supported attributes, the output is a blank line.
Include "type" in the object_info_options even though the client does
not yet know how to parse the server's "type" capability.
As a result, "type" is always filtered out, allowing the tests to verify
that known but unsupported placeholders expand to an empty string.
Since the filter removes options by swapping with the last element,
the list is no longer kept sorted. Drop the pre-sort in
fetch_object_info_via_pack() and use the unsorted string_list lookup
for the response header. This has no effect in performance as the list
can only be two entries long ('size' and 'type').
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
builtin/cat-file.c | 86 +++++++++++++++++++++-------------
fetch-object-info.c | 20 +++++++-
fetch-object-info.h | 3 ++
t/t1017-cat-file-remote-object-info.sh | 28 +++++++++++
transport.c | 1 -
5 files changed, 104 insertions(+), 34 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f5e5528a21..7a0431bf2b 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -338,13 +338,11 @@ struct expand_data {
* Flags about when an object info is being fetched from remote.
*/
unsigned is_remote:1;
-};
-#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
-static const char *remote_object_info_atoms[] = {
- "objectname",
- "objectsize",
+ struct string_list remote_allowed_atoms;
};
+#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD, \
+ .remote_allowed_atoms = STRING_LIST_INIT_NODUP }
static int is_atom(const char *atom, const char *s, int slen)
{
@@ -356,17 +354,11 @@ static int expand_atom(struct strbuf *sb, const char *atom, int len,
struct expand_data *data)
{
if (data->is_remote) {
- size_t i, allowed_nr = ARRAY_SIZE(remote_object_info_atoms);
- for (i = 0; i < allowed_nr; i++)
- if (is_atom(remote_object_info_atoms[i], atom, len))
+ size_t i;
+ for (i = 0; i < data->remote_allowed_atoms.nr; i++)
+ if (is_atom(data->remote_allowed_atoms.items[i].string, atom, len))
break;
-
- /*
- * On remote, skip unsupported atoms returning an empty sb,
- * honoring how for-each-ref handles known but inapplicable
- * atoms (e.g. %(tagger)).
- */
- if (i == allowed_nr)
+ if (i == data->remote_allowed_atoms.nr)
return 1;
}
@@ -682,12 +674,12 @@ static void batch_one_object(const char *obj_name,
static int get_remote_info(int argc,
const char **argv,
struct object_info **remote_object_info,
- struct oid_array *object_info_oids)
+ struct oid_array *object_info_oids,
+ struct string_list *object_info_options)
{
int retval = 0;
struct remote *remote = NULL;
struct object_id oid;
- struct string_list object_info_options = STRING_LIST_INIT_NODUP;
struct transport *gtransport;
remote = remote_get(argv[0]);
@@ -727,13 +719,10 @@ static int get_remote_info(int argc,
CALLOC_ARRAY(*remote_object_info, object_info_oids->nr);
gtransport->smart_options->object_info_oids = object_info_oids;
- string_list_append(&object_info_options, "size");
-
- gtransport->smart_options->object_info_options = &object_info_options;
+ gtransport->smart_options->object_info_options = object_info_options;
gtransport->smart_options->object_info_data = *remote_object_info;
retval = transport_fetch_object_info(gtransport);
cleanup:
- string_list_clear(&object_info_options, 0);
transport_disconnect(gtransport);
return retval;
}
@@ -819,6 +808,21 @@ static void parse_cmd_mailmap(struct batch_options *opt UNUSED,
load_mailmap();
}
+struct protocol_placeholder_entry {
+ const char *option;
+ const char *atom;
+};
+
+static const struct protocol_placeholder_entry remote_atom_map[] = {
+ {"size", "objectsize"},
+ {"type", "objecttype"},
+ /*
+ * Add new protocol options here. Even if the server doesn't support
+ * them the allow_list will drop them if the server doesn't advertise
+ * them.
+ */
+};
+
static void parse_cmd_remote_object_info(struct batch_options *opt,
const char *line, struct strbuf *output,
struct expand_data *data)
@@ -828,6 +832,7 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
char *line_to_split;
struct object_info *remote_object_info = NULL;
struct oid_array object_info_oids = OID_ARRAY_INIT;
+ struct string_list object_info_options = STRING_LIST_INIT_NODUP;
const char *saved_format = opt->format;
if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
@@ -846,10 +851,22 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
die(_("remote-object-info supports at most %d objects"),
MAX_ALLOWED_OBJ_LIMIT);
+ if (data->info.sizep)
+ string_list_append(&object_info_options, "size");
+ if (data->info.typep)
+ string_list_append(&object_info_options, "type");
+
if (get_remote_info(count, argv, &remote_object_info,
- &object_info_oids))
+ &object_info_oids, &object_info_options))
die(_("failed to get object info from the remote: %s"), argv[0]);
+ string_list_clear(&data->remote_allowed_atoms, 0);
+ string_list_append(&data->remote_allowed_atoms, "objectname");
+ for (size_t i = 0; i < ARRAY_SIZE(remote_atom_map); i++)
+ if (unsorted_string_list_has_string(&object_info_options, remote_atom_map[i].option))
+ string_list_append(&data->remote_allowed_atoms,
+ remote_atom_map[i].atom);
+
data->skip_object_info = 1;
for (size_t i = 0; i < object_info_oids.nr; i++) {
data->oid = object_info_oids.oid[i];
@@ -860,25 +877,29 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
continue;
}
+ /*
+ * When reaching here, it means remote-object-info can retrieve
+ * information from server without downloading them.
+ */
if (remote_object_info[i].sizep) {
- /*
- * When reaching here, it means remote-object-info can retrieve
- * information from server without downloading them.
- */
data->size = *remote_object_info[i].sizep;
- opt->batch_mode = BATCH_MODE_INFO;
- data->is_remote = 1;
- batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
- data->is_remote = 0;
- } else {
- report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
}
+
+ if (remote_object_info[i].typep) {
+ data->type = *remote_object_info[i].typep;
+ }
+
+ opt->batch_mode = BATCH_MODE_INFO;
+ data->is_remote = 1;
+ batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
+ data->is_remote = 0;
}
data->skip_object_info = 0;
opt->format = saved_format;
for (size_t i = 0; i < object_info_oids.nr; i++)
free_object_info_contents(&remote_object_info[i]);
+ string_list_clear(&object_info_options, 0);
free(line_to_split);
free(argv);
free(remote_object_info);
@@ -1198,6 +1219,7 @@ static int batch_objects(struct batch_options *opt)
cleanup:
strbuf_release(&input);
strbuf_release(&output);
+ string_list_clear(&data.remote_allowed_atoms, 0);
cfg->warn_on_object_refname_ambiguity = save_warning;
return retval;
}
diff --git a/fetch-object-info.c b/fetch-object-info.c
index 30475a1e87..ba7e179c44 100644
--- a/fetch-object-info.c
+++ b/fetch-object-info.c
@@ -55,6 +55,24 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
case protocol_v2:
if (!server_supports_v2("object-info"))
die(_("object-info capability is not enabled on the server"));
+ /*
+ * When removing an element from the list it gets swapped by the
+ * last element, iterate backwards to prevent elements skipping
+ * evaluation.
+ *
+ * object_info_options->nr can be safely casted without overflow
+ * because the number of options is a small known number (the
+ * supported placeholders which currently are size and type).
+ */
+ for (int i = (int)args->object_info_options->nr - 1; i >= 0; i--)
+ if (!server_supports_feature("object-info",
+ args->object_info_options->items[i].string, 0))
+ unsorted_string_list_delete_item(args->object_info_options, i, 0);
+
+ /*
+ * Even if no options are left, we still send the oid so we get
+ * at least an existence check.
+ */
send_object_info_request(fd_out, args);
break;
case protocol_v1:
@@ -71,7 +89,7 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
return -1;
}
- if (!string_list_has_string(args->object_info_options, reader->line))
+ if (!unsorted_string_list_has_string(args->object_info_options, reader->line))
return -1;
if (!strcmp(reader->line, "size")) {
diff --git a/fetch-object-info.h b/fetch-object-info.h
index 31aad98408..269cebb3f7 100644
--- a/fetch-object-info.h
+++ b/fetch-object-info.h
@@ -14,6 +14,9 @@ struct object_info;
/*
* Sends git-cat-file object-info command into the request buf and read the
* results from packets.
+ *
+ * Modifies args->object_info_options, on return it contains only the supported
+ * options by the server.
*/
int fetch_object_info(enum protocol_version version, struct object_info_args *args,
struct packet_reader *reader, struct object_info *object_info_data,
diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
index edc20394d8..116862f9d0 100755
--- a/t/t1017-cat-file-remote-object-info.sh
+++ b/t/t1017-cat-file-remote-object-info.sh
@@ -271,6 +271,34 @@ test_expect_success 'unsupported placeholder on remote returns empty string' '
)
'
+test_expect_success 'requesting only objectname echoes back' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ echo $hello_oid >expect &&
+ git cat-file --batch-command="%(objectname)" >actual <<-EOF &&
+ remote-object-info "$GIT_DAEMON_URL/parent" $hello_oid
+ EOF
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'objectname goes through existence check' '
+ (
+ set_transport_variables "$daemon_parent" &&
+ cd "$daemon_parent/daemon_client_empty" &&
+
+ echo "$unstored_oid missing" >expect &&
+
+ git cat-file --batch-command="%(objectname)" >actual <<-EOF &&
+ remote-object-info "$GIT_DAEMON_URL/parent" $unstored_oid
+ EOF
+
+ test_cmp expect actual
+ )
+'
+
# Test --batch-command remote-object-info with 'git://' and
# transfer.advertiseobjectinfo set to false, i.e. server does not have object-info capability
test_expect_success 'batch-command remote-object-info git:// fails when transfer.advertiseobjectinfo=false' '
diff --git a/transport.c b/transport.c
index 9342680531..f0a6a45547 100644
--- a/transport.c
+++ b/transport.c
@@ -443,7 +443,6 @@ static int fetch_object_info_via_pack(struct transport *transport)
args.server_options = transport->server_options;
args.oids = transport->smart_options->object_info_oids;
args.object_info_options = transport->smart_options->object_info_options;
- string_list_sort(args.object_info_options);
connect_setup(transport, 0);
packet_reader_init(&reader, data->fd[0], NULL, 0,
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v7] show-branch: convert per-branch flags to commit-slab
From: Junio C Hamano @ 2026-07-17 15:25 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Gatla Vishweshwar Reddy, git
In-Reply-To: <aloHDhoerEhIXxFA@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Hi Gatla,
>
> On Fri, Jul 17, 2026 at 04:04:54PM +0530, Gatla Vishweshwar Reddy wrote:
>> Hi Patrick,
>>
>> I am a real person. I used AI help for structuring reply in that thread. I understand that is not
>> appropriate here and will write my own from now on.
>
> Okay. Using AI is fine to help you out, but the human-focussed bits
> should really rather be written in a way that it feels like we're
> talking to a human. We're a community here, and when you see text that
> is so obviously written by an AI it can get very frustrating eventually.
>
> 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-<.
^ permalink raw reply
* Re: Please provide help with how to fix
From: Randy Kroeger @ 2026-07-17 15:40 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: git@vger.kernel.org
In-Reply-To: <CALnO6CDGkAzu4Xz2o=VaCfwsF1WEcBw2k3-JqmkGqj1+ZpRQyA@mail.gmail.com>
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...
If I may, let me explain the issue this way:
You have two sets of parents. Set one (A1 B2) and set two (A2 B2). Each set of parents live on one street. Between the two homes, there are many homes (i.e. neighbors). Parents A1 and A2 only want to talk to each other causing both homes to ignore all the neighbors between them. Hence, if I pull, I no longer see the changes between these two commits. My goal: I want to higher a hit man to get rid of A1 and A2 (lol) so that B1 and B2 can talk to the neighbors between them, hence, when I can do a pull, I see all my changes.
I am understanding you would like logs, but the size is not small. If my explanation this time is not clear enough, can you tell me if you are ok with a shared link from OneDrive? I can paste a screen shot for you to show you what is going on and even make copies of what you might need easier.
I do appreciate the help.
Thank you.
Randy Kroeger
________________________________________
From: D. Ben Knoble <ben.knoble@gmail.com>
Sent: Thursday, July 16, 2026 5:08 PM
To: Randy Kroeger
Cc: git@vger.kernel.org
Subject: Re: Please provide help with how to fix
On Thu, Jul 16, 2026 at 4:52 PM Randy Kroeger <kroegerr@cseasy.com> wrote:
>
>
> I am having a bit of an issue trying to figure out the best route in fixing the following.
The advice I give my colleagues:
1. Don't panic
2. Figure out where you are
3. Figure out where you want to go
4. Decide how to get there
> What happened is on my second machine, in which was out dated (source code), I upgraded to VS 2026 (from 2022), then tried to do a pull. What happened was that I received a bunch of modifications, which was confusing. All I want is to pull all changes since I did last on this machine. I then had a bit of a problem with the gitignore file, so I decided to just commit it (my train of thought is it is a file being committed to source control - that is it). However, what happened is this file took on a life and decided to make itself the head and bypass all changes to the head in which it knew about last. Please see image below where the history shows a line from this commit to the parent below. This by passes a bunch of chances.
A suite of helpful "where am I" commands:
- git status: is the repo clean? before we go further, let's not lose work
- git log --graph [--oneline]: what's the shape of commits? (this
would be useful to copy/paste, optionally with annotations, in place
of your diagram below)
- git reflog HEAD: what operations brought me here, and what are some
interesting recent checkouts?
Then (repeating a line from a previous quote):
> All I want is to pull all changes since I did last on this machine.
Once we know where you are, we can talk about where you want to go.
When you say "pull all changes," do you mean a "fetch" (update my
local repository's notions of where remote branches are) or a "pull"
(merge or rebase local branches with/on their upstreams)?
> Question: How can I fix this issue? I would like to restore all my changes again and remove this bypass. I have been reviewing your documentation, but am very hesitant as my understanding, once again, may not match how GIT actually functions.
Only when you know where you are and where you want to go can you find
appropriate fixes ;)
> I greatly appreciate the help!
>
> In this example, Commit 3 was done on July 12 and since it was on a machine that had done its last pull on 6/09/2026, the commit created a new parent below Commit 5. Now when I pull, the changes for Commit4, Commit5 are not included in the pull. I am assuming I need to do a rebase, but am not 100% confident and in reading the documentation, I am still not confident.
>
> --Commit6 7/14/2026
> --Commit5 7/13/2026
> |<-Commit4 7/12/2026 -child
> | --Commit2 6/11/2026
> | --Commit1 6/10/2026
> |>-Commit4 7/12/2026 -parent
>
> Randy
I'm not sure how to interpret this diagram; perhaps you could use "git
log --graph --oneline" to show your current and desired states?
--
D. Ben Knoble
^ permalink raw reply
* [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
* [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 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 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 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
* 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 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
* [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 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
* 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
* 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: [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
* [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 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
* 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 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: [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
* [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
* [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 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 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
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