* [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
* 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
* 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
* [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
* [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 11/13] transport: add client support for object-info
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: Calvin Wan <calvinwan@google.com>
Sometimes, it is beneficial to retrieve information about an object
without downloading it entirely. The server-side logic for this
functionality was implemented in commit "a2ba162cda (object-info:
support for retrieving object info, 2021-04-20)." And the wire
format is documented at
https://git-scm.com/docs/protocol-v2#_object_info.
Introduce client-side support for the object-info capability.
Add its own function for object-info separate from existing fetch
infrastructure.
Currently, the client supports requesting a list of OIDs with the size
attribute from a v2 server. If the server does not advertise this
feature (i.e., transfer.advertiseobjectinfo is set to false), the client
returns an error and exits.
Note that:
1. The entire request is written into req_buf before being sent to the
remote. This approach follows the pattern used in the
send_fetch_request() logic within 'fetch-pack.c'. Streaming the
request is not addressed in this patch.
2. A new field 'unrecognized' has been added to object_info. This new
field is set at fetch_object_info() when the object is unrecognized
by the server.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Makefile | 1 +
fetch-object-info.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++
fetch-object-info.h | 22 ++++++++
meson.build | 1 +
odb.h | 6 +++
transport-helper.c | 10 ++++
transport-internal.h | 8 +++
transport.c | 45 +++++++++++++++++
transport.h | 9 ++++
9 files changed, 240 insertions(+)
diff --git a/Makefile b/Makefile
index 1f3f099f5c..d450e0277e 100644
--- a/Makefile
+++ b/Makefile
@@ -1158,6 +1158,7 @@ LIB_OBJS += ewah/ewah_io.o
LIB_OBJS += ewah/ewah_rlw.o
LIB_OBJS += exec-cmd.o
LIB_OBJS += fetch-negotiator.o
+LIB_OBJS += fetch-object-info.o
LIB_OBJS += fetch-pack.o
LIB_OBJS += fmt-merge-msg.o
LIB_OBJS += fsck.o
diff --git a/fetch-object-info.c b/fetch-object-info.c
new file mode 100644
index 0000000000..30475a1e87
--- /dev/null
+++ b/fetch-object-info.c
@@ -0,0 +1,138 @@
+#include "git-compat-util.h"
+#include "gettext.h"
+#include "hex.h"
+#include "pkt-line.h"
+#include "connect.h"
+#include "oid-array.h"
+#include "odb.h"
+#include "fetch-object-info.h"
+#include "string-list.h"
+
+/* Sends object-info command and its arguments into the request buffer. */
+static void send_object_info_request(const int fd_out, struct object_info_args *args)
+{
+ struct strbuf req_buf = STRBUF_INIT;
+
+ write_command_and_capabilities(&req_buf, "object-info", args->server_options);
+
+ if (unsorted_string_list_has_string(args->object_info_options, "size"))
+ packet_buf_write(&req_buf, "size");
+ else if (args->object_info_options->nr)
+ BUG("only size should be in object_info_options");
+
+ if (args->oids)
+ for (size_t i = 0; i < args->oids->nr; i++)
+ packet_buf_write(&req_buf, "oid %s", oid_to_hex(&args->oids->oid[i]));
+
+ packet_buf_flush(&req_buf);
+ if (write_in_full(fd_out, req_buf.buf, req_buf.len) < 0)
+ die_errno(_("unable to write request to remote"));
+
+ strbuf_release(&req_buf);
+}
+
+static int parse_object_size(const char *s, size_t *res)
+{
+ uintmax_t uim;
+
+ if (!s[0] || s[strspn(s, "0123456789")])
+ return -1;
+ errno = 0;
+ uim = strtoumax(s, NULL, 10);
+ if (errno || uim > SIZE_MAX)
+ return -1;
+ *res = uim;
+ return 0;
+}
+
+int fetch_object_info(const enum protocol_version version, struct object_info_args *args,
+ struct packet_reader *reader, struct object_info *object_info_data,
+ const int stateless_rpc, const int fd_out)
+{
+ int size_index = -1;
+
+ switch (version) {
+ case protocol_v2:
+ if (!server_supports_v2("object-info"))
+ die(_("object-info capability is not enabled on the server"));
+ send_object_info_request(fd_out, args);
+ break;
+ case protocol_v1:
+ case protocol_v0:
+ die(_("object-info requires protocol v2"));
+ case protocol_unknown_version:
+ BUG("unknown protocol version");
+ }
+
+ for (size_t i = 0; i < args->object_info_options->nr; i++) {
+ if (packet_reader_read(reader) != PACKET_READ_NORMAL) {
+ check_stateless_delimiter(stateless_rpc, reader,
+ "stateless delimiter expected");
+ return -1;
+ }
+
+ if (!string_list_has_string(args->object_info_options, reader->line))
+ return -1;
+
+ if (!strcmp(reader->line, "size")) {
+ /*
+ * i is the number of supported options which currently
+ * is only size. No risk of overflow.
+ */
+ size_index = (int)i;
+ for (size_t j = 0; j < args->oids->nr; j++)
+ object_info_data[j].sizep =
+ xcalloc(1, sizeof(*object_info_data[j].sizep));
+ } else {
+ BUG("only size is supported");
+ }
+ }
+
+ for (size_t i = 0;
+ packet_reader_read(reader) == PACKET_READ_NORMAL &&
+ i < args->oids->nr;
+ i++) {
+ struct string_list object_info_values = STRING_LIST_INIT_DUP;
+
+ string_list_split(&object_info_values, reader->line, " ", -1);
+
+ if (strcmp(object_info_values.items[0].string,
+ oid_to_hex(&args->oids->oid[i])))
+ die(_("object-info: expected OID: %s, got %s"),
+ oid_to_hex(&args->oids->oid[i]),
+ object_info_values.items[0].string);
+
+ /*
+ * If the response is two elements but the second one is an
+ * empty string, that means that the OID is unrecognized by the
+ * server.
+ */
+ if (object_info_values.nr >= 2 &&
+ !strcmp(object_info_values.items[1].string, "")) {
+ object_info_data[i].unrecognized = 1;
+ string_list_clear(&object_info_values, 0);
+ continue;
+ }
+
+ /*
+ * Because we filter the options to be only the supported by
+ * the server we expect the server to answer with the same
+ * number of attributes requested.
+ */
+ if (args->object_info_options->nr + 1 != object_info_values.nr)
+ die("object-info: unexpected number of attributes: %s",
+ reader->line);
+
+ if (size_index >= 0 &&
+ parse_object_size(object_info_values.items[size_index + 1].string,
+ object_info_data[i].sizep))
+ die("object-info: ref %s has invalid size %s",
+ object_info_values.items[0].string,
+ object_info_values.items[size_index + 1].string);
+
+ string_list_clear(&object_info_values, 0);
+ }
+ check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected");
+
+ return 0;
+}
diff --git a/fetch-object-info.h b/fetch-object-info.h
new file mode 100644
index 0000000000..31aad98408
--- /dev/null
+++ b/fetch-object-info.h
@@ -0,0 +1,22 @@
+#ifndef FETCH_OBJECT_INFO_H
+#define FETCH_OBJECT_INFO_H
+
+#include "pkt-line.h"
+#include "protocol.h"
+
+struct object_info_args {
+ struct string_list *object_info_options;
+ const struct string_list *server_options;
+ struct oid_array *oids;
+};
+
+struct object_info;
+/*
+ * Sends git-cat-file object-info command into the request buf and read the
+ * results from packets.
+ */
+int fetch_object_info(enum protocol_version version, struct object_info_args *args,
+ struct packet_reader *reader, struct object_info *object_info_data,
+ int stateless_rpc, int fd_out);
+
+#endif /* FETCH_OBJECT_INFO_H */
diff --git a/meson.build b/meson.build
index 9434b56960..dfefcd3475 100644
--- a/meson.build
+++ b/meson.build
@@ -359,6 +359,7 @@ libgit_sources = [
'ewah/ewah_rlw.c',
'exec-cmd.c',
'fetch-negotiator.c',
+ 'fetch-object-info.c',
'fetch-pack.c',
'fmt-merge-msg.c',
'fsck.c',
diff --git a/odb.h b/odb.h
index 94754643d2..88a37febbf 100644
--- a/odb.h
+++ b/odb.h
@@ -339,6 +339,12 @@ struct object_info {
* or multiple times in the same source.
*/
struct odb_source_info *source_infop;
+
+ /*
+ * object-info protocol specific. Set by the protocol when the remote
+ * does not recognize the requested object.
+ */
+ unsigned int unrecognized:1;
};
/*
diff --git a/transport-helper.c b/transport-helper.c
index f195070788..623463dcea 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -784,6 +784,15 @@ static int fetch_refs(struct transport *transport,
return -1;
}
+static int fetch_object_info_helper(struct transport *transport)
+{
+ get_helper(transport);
+ if (process_connect(transport, 0))
+ return transport->vtable->fetch_object_info(transport);
+
+ die(_("object-info requires protocol v2"));
+}
+
struct push_update_ref_state {
struct ref *hint;
struct ref_push_report *report;
@@ -1330,6 +1339,7 @@ static struct transport_vtable vtable = {
.get_refs_list = get_refs_list,
.get_bundle_uri = get_bundle_uri,
.fetch_refs = fetch_refs,
+ .fetch_object_info = fetch_object_info_helper,
.push_refs = push_refs,
.connect = connect_helper,
.disconnect = release_helper
diff --git a/transport-internal.h b/transport-internal.h
index 051f3ab0dc..60db0bedcd 100644
--- a/transport-internal.h
+++ b/transport-internal.h
@@ -45,6 +45,14 @@ struct transport_vtable {
**/
int (*fetch_refs)(struct transport *transport, int refs_nr, struct ref **refs);
+ /*
+ * Fetch object info (only size currently) from remote without
+ * downloading the objects.
+ *
+ * Uses object-info capability of v2 protocol.
+ */
+ int (*fetch_object_info)(struct transport *transport);
+
/**
* Push the objects and refs. Send the necessary objects, and
* then, for any refs where peer_ref is set and
diff --git a/transport.c b/transport.c
index fc144f0aed..9342680531 100644
--- a/transport.c
+++ b/transport.c
@@ -9,6 +9,7 @@
#include "hook.h"
#include "pkt-line.h"
#include "fetch-pack.h"
+#include "fetch-object-info.h"
#include "remote.h"
#include "connect.h"
#include "send-pack.h"
@@ -432,6 +433,48 @@ static int get_bundle_uri(struct transport *transport)
transport->bundles, stateless_rpc);
}
+static int fetch_object_info_via_pack(struct transport *transport)
+{
+ int ret = 0;
+ struct git_transport_data *data = transport->data;
+ struct packet_reader reader;
+ struct object_info_args args = { 0 };
+
+ 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,
+ PACKET_READ_CHOMP_NEWLINE |
+ PACKET_READ_GENTLE_ON_EOF |
+ PACKET_READ_DIE_ON_ERR_PACKET);
+
+ data->version = discover_version(&reader);
+ transport->hash_algo = reader.hash_algo;
+
+ ret = fetch_object_info(data->version, &args, &reader,
+ data->options.object_info_data,
+ transport->stateless_rpc, data->fd[1]);
+
+ close(data->fd[0]);
+ if (data->fd[1] >= 0)
+ close(data->fd[1]);
+ if (finish_connect(data->conn))
+ ret = -1;
+ data->conn = NULL;
+
+ return ret;
+}
+
+int transport_fetch_object_info(struct transport *transport)
+{
+ if (!transport->vtable->fetch_object_info)
+ die(_("remote does not support object-info"));
+ return transport->vtable->fetch_object_info(transport);
+}
+
static int fetch_refs_via_pack(struct transport *transport,
int nr_heads, struct ref **to_fetch)
{
@@ -1004,6 +1047,7 @@ static struct transport_vtable taken_over_vtable = {
.get_refs_list = get_refs_via_connect,
.get_bundle_uri = get_bundle_uri,
.fetch_refs = fetch_refs_via_pack,
+ .fetch_object_info = fetch_object_info_via_pack,
.push_refs = git_transport_push,
.disconnect = disconnect_git
};
@@ -1169,6 +1213,7 @@ static struct transport_vtable builtin_smart_vtable = {
.get_refs_list = get_refs_via_connect,
.get_bundle_uri = get_bundle_uri,
.fetch_refs = fetch_refs_via_pack,
+ .fetch_object_info = fetch_object_info_via_pack,
.push_refs = git_transport_push,
.connect = connect_git,
.disconnect = disconnect_git
diff --git a/transport.h b/transport.h
index 7e5867cffa..a7869d18e0 100644
--- a/transport.h
+++ b/transport.h
@@ -55,6 +55,10 @@ struct git_transport_options {
* common commits to this oidset instead of fetching any packfiles.
*/
struct oidset *acked_commits;
+
+ struct oid_array *object_info_oids;
+ struct object_info *object_info_data;
+ struct string_list *object_info_options;
};
enum transport_family {
@@ -309,6 +313,11 @@ int transport_get_remote_bundle_uri(struct transport *transport);
const struct git_hash_algo *transport_get_hash_algo(struct transport *transport);
int transport_fetch_refs(struct transport *transport, struct ref *refs);
+/*
+ * Fetch the object info from remote
+ */
+int transport_fetch_object_info(struct transport *transport);
+
/*
* If this flag is set, unlocking will avoid to call non-async-signal-safe
* functions. This will necessarily leave behind some data structures which
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 10/13] serve: advertise object-info feature
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: Calvin Wan <calvinwan@google.com>
In order for a client to know what object-info components a server can
provide, advertise supported object-info features. This allows a client
to decide whether to query the server for object-info or fetch as a
fallback.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
serve.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/serve.c b/serve.c
index 49a6e39b1d..2b07d922b3 100644
--- a/serve.c
+++ b/serve.c
@@ -89,7 +89,7 @@ static void session_id_receive(struct repository *r UNUSED,
trace2_data_string("transfer", NULL, "client-sid", client_sid);
}
-static int object_info_advertise(struct repository *r, struct strbuf *value UNUSED)
+static int object_info_advertise(struct repository *r, struct strbuf *value)
{
if (advertise_object_info == -1 &&
repo_config_get_bool(r, "transfer.advertiseobjectinfo",
@@ -97,6 +97,9 @@ static int object_info_advertise(struct repository *r, struct strbuf *value UNUS
/* disabled by default */
advertise_object_info = 0;
}
+ /* Currently only size is supported */
+ if (value && advertise_object_info)
+ strbuf_addstr(value, "size");
return advertise_object_info;
}
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 09/13] protocol-caps: check object existence regardless of the attributes requested
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>
Currently, send_info() only checks for existence when the attribute
'size' is also requested. Requesting a bare OID, without attributes only
echoes back the OID.
Extract the existence check to be done regardless of the number of
attributes requested.
While at it, introduce a wrapper called get_object_info() similar to
odb_read_object_info() that returns OBJ_BAD on fail and adds
OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK flags.
OBJECT_INFO_SKIP_FETCH_OBJECT is so a server with a partial clone
doesn't trigger fetching objects when it gets an object-info request
with an OID that is not available locally. A server should only report
what it has locally.
Tighten the condition used to determine whether an object is
recognized. get_object_info() returns OBJ_BAD for unknown objects,
but OBJ_NONE (0) can also mean "not found". Change the check from '< 0'
to '<= OBJ_NONE' to cover both as unrecognized.
With this patch, a bare OID has two possible responses:
1. Recognized OID: the server answers with "<OID>"
2. Unrecognized OID: the server answers with "<OID> SP"
Update the object-info section in 'gitprotocol-v2.adoc':
- Require full obj-oid explicitly.
- Fix parentheses.
- Define obj-size explicitly.
- Make obj-size optional in obj-info and document the behavior
for unrecognized object IDs.
- Describe the attr header as zero or more pkt-lines, one per attribute,
matching what the server implements. A request with no attributes gets
no header.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Documentation/gitprotocol-v2.adoc | 21 ++++++++-----
protocol-caps.c | 45 ++++++++++++++++++++++++----
t/t5701-git-serve.sh | 63 +++++++++++++++++++++++++++++++++++++++
3 files changed, 115 insertions(+), 14 deletions(-)
diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index 2beb70595f..7bf62014c3 100644
--- a/Documentation/gitprotocol-v2.adoc
+++ b/Documentation/gitprotocol-v2.adoc
@@ -568,21 +568,26 @@ An `object-info` request takes the following arguments:
oid <oid>
Indicates to the server an object which the client wants to obtain
- information for.
+ information for. They must be full OIDs.
-The response of `object-info` is a list of the requested object ids
-and associated requested information, each separated by a single space.
+The response of `object-info` consists of one pkt-line per requested attribute,
+echoing the attributes the server will report, followed by one pkt-line per
+requested object id with its information, each field separated by a single
+space.
output = info flush-pkt
- info = PKT-LINE(attrs) LF)
- *PKT-LINE(obj-info LF)
-
- attrs = attr | attrs SP attrs
+ info = *PKT-LINE(attr LF)
+ *PKT-LINE(obj-info LF)
attr = "size"
- obj-info = obj-id SP obj-size
+ obj-size = 1*DIGIT
+
+ obj-info = obj-id [SP [obj-size]]
+
+If the server does not recognize the OID, the response will be `<oid> SP`
+regardless of the number of attributes requested.
bundle-uri
~~~~~~~~~~
diff --git a/protocol-caps.c b/protocol-caps.c
index 8858ea4489..02261be14d 100644
--- a/protocol-caps.c
+++ b/protocol-caps.c
@@ -30,6 +30,32 @@ static int parse_oid(const char *line, struct string_list *oid_str_list)
return 1;
}
+/*
+ * odb_read_object_info_extended() wrapper. Similar to odb_read_object_info()
+ * but uses the flags:
+ *
+ * - OBJECT_INFO_SKIP_FETCH_OBJECT so a server won't fetch an object when a
+ * object-info request asks for an OID that it doesn't have.
+ *
+ * - OBJECT_INFO_QUICK to avoid re-scanning packs when the object is not found.
+ */
+static enum object_type get_object_info(struct object_database *odb,
+ const struct object_id *oid,
+ size_t *sizep)
+{
+ enum object_type type;
+ struct object_info oi = OBJECT_INFO_INIT;
+
+ oi.typep = &type;
+ oi.sizep = sizep;
+ if (odb_read_object_info_extended(odb, oid, &oi,
+ OBJECT_INFO_LOOKUP_REPLACE |
+ OBJECT_INFO_SKIP_FETCH_OBJECT |
+ OBJECT_INFO_QUICK) < 0)
+ return OBJ_BAD;
+ return type;
+}
+
/*
* Validates and send requested info back to the client. Any errors detected
* are returned as they are detected.
@@ -62,15 +88,22 @@ static void send_info(struct repository *r, struct packet_writer *writer,
strbuf_addstr(&send_buffer, oid_str);
+ /*
+ * Check the existence of the object first.
+ * If an object is not recognized by the server append SP to
+ * the response.
+ */
+ if (get_object_info(r->objects, &oid, &object_size) <= OBJ_NONE) {
+ strbuf_addstr(&send_buffer, " ");
+ goto write;
+ }
+
if (info->size) {
- if (odb_read_object_info(r->objects, &oid, &object_size) < 0) {
- strbuf_addstr(&send_buffer, " ");
- } else {
- strbuf_addf(&send_buffer, " %"PRIuMAX,
- (uintmax_t)object_size);
- }
+ strbuf_addf(&send_buffer, " %"PRIuMAX,
+ (uintmax_t)object_size);
}
+write:
packet_writer_write(writer, "%s", send_buffer.buf);
strbuf_reset(&send_buffer);
}
diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index d4c28bae39..cacff4456c 100755
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -7,6 +7,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+unknown_oid=$(printf "test" | git hash-object --stdin)
+
test_expect_success 'setup to generate files with expected content' '
printf "agent=git/%s" "$(git version | cut -d" " -f3)" >agent_capability &&
@@ -364,6 +366,67 @@ test_expect_success 'basics of object-info' '
test_cmp expect actual
'
+test_expect_success 'bare OID request' '
+ test_config transfer.advertiseObjectInfo true &&
+
+ test-tool pkt-line pack >in <<-EOF &&
+ command=object-info
+ object-format=$(test_oid algo)
+ 0001
+ oid $(git rev-parse two:two.t)
+ 0000
+ EOF
+
+ cat >expect <<-EOF &&
+ $(git rev-parse two:two.t)
+ 0000
+ EOF
+
+ test-tool serve-v2 --stateless-rpc <in >out &&
+ test-tool pkt-line unpack <out >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'object-info with bare unrecognized OID' '
+ test_config transfer.advertiseObjectInfo true &&
+
+ test-tool pkt-line pack >in <<-EOF &&
+ command=object-info
+ object-format=$(test_oid algo)
+ 0001
+ oid $unknown_oid
+ 0000
+ EOF
+
+ printf "%s \n" "$unknown_oid" >expect &&
+ printf "0000\n" >>expect &&
+
+ test-tool serve-v2 --stateless-rpc <in >out &&
+ test-tool pkt-line unpack <out >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'object-info with size for unrecognized OID' '
+ test_config transfer.advertiseObjectInfo true &&
+
+ test-tool pkt-line pack >in <<-EOF &&
+ command=object-info
+ object-format=$(test_oid algo)
+ 0001
+ size
+ oid $unknown_oid
+ 0000
+ EOF
+
+ printf "size\n" >expect &&
+ printf "%s \n" "$unknown_oid" >>expect &&
+ printf "0000\n" >>expect &&
+
+ test-tool serve-v2 --stateless-rpc <in >out &&
+ test-tool pkt-line unpack <out >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'test capability advertisement with uploadpack.advertiseBundleURIs' '
test_config uploadpack.advertiseBundleURIs true &&
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 08/13] fetch-pack: move fetch initialization
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: Calvin Wan <calvinwan@google.com>
There are some variables initialized at the start of the
do_fetch_pack_v2() state machine. Currently, they are initialized in
FETCH_CHECK_LOCAL, which is the initial state set at the beginning
of the function.
However, a subsequent patch will allow for another initial state,
while still requiring these initialized variables.
Move the initialization to be before the state machine,
so that they are set regardless of the initial state.
Note that there is no change in behavior, because we're moving code
from the beginning of the first state to just before the execution of
the state machine.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
fetch-pack.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 3695059cd5..922a9b2581 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1735,18 +1735,18 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
reader.me = "fetch-pack";
}
+ /* v2 supports these by default */
+ allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
+ use_sideband = 2;
+ if (args->depth > 0 || args->deepen_since || args->deepen_not)
+ args->deepen = 1;
+
while (state != FETCH_DONE) {
switch (state) {
case FETCH_CHECK_LOCAL:
sort_ref_list(&ref, ref_compare_name);
QSORT(sought, nr_sought, cmp_ref_by_name);
- /* v2 supports these by default */
- allow_unadvertised_object_request |= ALLOW_REACHABLE_SHA1;
- use_sideband = 2;
- if (args->depth > 0 || args->deepen_since || args->deepen_not)
- args->deepen = 1;
-
/* Filter 'ref' by 'sought' and those that aren't local */
mark_complete_and_common_ref(negotiator, args, &ref);
filter_refs(args, &ref, sought, nr_sought);
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 07/13] connect: make write_fetch_command_and_capabilities() more generic
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>
Refactor write_fetch_command_and_capabilities(), enabling it to serve
both fetch and additional commands.
In this context, "command" refers to the "operations" supported by
Git's wire protocol Documentation/gitprotocol-v2.adoc, such as a Git
subcommand (e.g., git-fetch(1)) or a server-side operation like
"object-info" as implemented in commit a2ba162cda
(object-info: support for retrieving object info, 2021-04-20).
Refactor the function signature to accept a command instead of the
hardcoded "fetch".
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
connect.c | 8 ++++----
connect.h | 8 ++++++--
fetch-pack.c | 4 ++--
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/connect.c b/connect.c
index 9d236e7bba..9d54d40770 100644
--- a/connect.c
+++ b/connect.c
@@ -700,16 +700,16 @@ int server_supports(const char *feature)
return !!server_feature_value(feature, NULL);
}
-void write_fetch_command_and_capabilities(struct strbuf *req_buf,
- const struct string_list *server_options)
+void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
+ const struct string_list *server_options)
{
const char *hash_name;
int advertise_sid = 0;
repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
- ensure_server_supports_v2("fetch");
- packet_buf_write(req_buf, "command=fetch");
+ ensure_server_supports_v2(command);
+ packet_buf_write(req_buf, "command=%s", command);
if (server_supports_v2("agent"))
packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
if (advertise_sid && server_supports_v2("session-id"))
diff --git a/connect.h b/connect.h
index c4f6ea4b0a..957e5fe2b9 100644
--- a/connect.h
+++ b/connect.h
@@ -35,7 +35,11 @@ void check_stateless_delimiter(int stateless_rpc,
const char *error);
struct string_list;
-void write_fetch_command_and_capabilities(struct strbuf *req_buf,
- const struct string_list *server_options);
+/*
+ * Write a protocol v2 command request, along with the capability
+ * advertisements, into req_buf.
+ */
+void write_command_and_capabilities(struct strbuf *req_buf, const char *command,
+ const struct string_list *server_options);
#endif
diff --git a/fetch-pack.c b/fetch-pack.c
index f7789e8456..3695059cd5 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1386,7 +1386,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
int done_sent = 0;
struct strbuf req_buf = STRBUF_INIT;
- write_fetch_command_and_capabilities(&req_buf, args->server_options);
+ write_command_and_capabilities(&req_buf, "fetch", args->server_options);
if (args->use_thin_pack)
packet_buf_write(&req_buf, "thin-pack");
@@ -2253,7 +2253,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
the_repository, "%d",
negotiation_round);
strbuf_reset(&req_buf);
- write_fetch_command_and_capabilities(&req_buf, server_options);
+ write_command_and_capabilities(&req_buf, "fetch", server_options);
packet_buf_write(&req_buf, "wait-for-done");
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 06/13] connect: use unsigned int for hash_algo_by_name() calls
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>
hash_algo_by_name() returns "unsigned int", but multiple variables that
store its return are assigned as "int".
Change hash_algo_by_name() variables type to match its return type, also
make it const because they are never modified.
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
connect.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/connect.c b/connect.c
index 31e5ab958b..9d236e7bba 100644
--- a/connect.c
+++ b/connect.c
@@ -248,7 +248,7 @@ static void process_capabilities(struct packet_reader *reader, size_t *linelen)
feat_val = server_feature_value("object-format", &feat_len);
if (feat_val) {
char *hash_name = xstrndup(feat_val, feat_len);
- int hash_algo = hash_algo_by_name(hash_name);
+ const unsigned int hash_algo = hash_algo_by_name(hash_name);
if (hash_algo != GIT_HASH_UNKNOWN)
reader->hash_algo = &hash_algos[hash_algo];
free(hash_name);
@@ -496,7 +496,7 @@ static void send_capabilities(int fd_out, struct packet_reader *reader)
packet_write_fmt(fd_out, "agent=%s", git_user_agent_sanitized());
if (server_feature_v2("object-format", &hash_name)) {
- int hash_algo = hash_algo_by_name(hash_name);
+ const unsigned int hash_algo = hash_algo_by_name(hash_name);
if (hash_algo == GIT_HASH_UNKNOWN)
die(_("unknown object format '%s' specified by server"), hash_name);
reader->hash_algo = &hash_algos[hash_algo];
@@ -722,7 +722,7 @@ void write_fetch_command_and_capabilities(struct strbuf *req_buf,
}
if (server_feature_v2("object-format", &hash_name)) {
- int hash_algo = hash_algo_by_name(hash_name);
+ const unsigned int hash_algo = hash_algo_by_name(hash_name);
if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
die(_("mismatched algorithms: client %s; server %s"),
the_hash_algo->name, hash_name);
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 05/13] fetch-pack: move write_fetch_command_and_capabilities() to connect.c
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>
In a subsequent commit write_fetch_command_and_capabilities() will be
refactored to a more general-purpose function, making it more accessible
to additional commands in the future.
Move write_fetch_command_and_capabilities() to 'connect.c', where
there are similar purpose functions.
Because string_list is only used as a pointer, use a forward
declaration [1].
[1]: https://lore.kernel.org/git/Z0RIqUAoEob8lGfM@pks.im/
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
connect.c | 34 ++++++++++++++++++++++++++++++++++
connect.h | 4 ++++
fetch-pack.c | 34 ----------------------------------
3 files changed, 38 insertions(+), 34 deletions(-)
diff --git a/connect.c b/connect.c
index 47e39d2a73..31e5ab958b 100644
--- a/connect.c
+++ b/connect.c
@@ -700,6 +700,40 @@ int server_supports(const char *feature)
return !!server_feature_value(feature, NULL);
}
+void write_fetch_command_and_capabilities(struct strbuf *req_buf,
+ const struct string_list *server_options)
+{
+ const char *hash_name;
+ int advertise_sid = 0;
+
+ repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
+
+ ensure_server_supports_v2("fetch");
+ packet_buf_write(req_buf, "command=fetch");
+ if (server_supports_v2("agent"))
+ packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
+ if (advertise_sid && server_supports_v2("session-id"))
+ packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
+ if (server_options && server_options->nr) {
+ ensure_server_supports_v2("server-option");
+ for (size_t i = 0; i < server_options->nr; i++)
+ packet_buf_write(req_buf, "server-option=%s",
+ server_options->items[i].string);
+ }
+
+ if (server_feature_v2("object-format", &hash_name)) {
+ int hash_algo = hash_algo_by_name(hash_name);
+ if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
+ die(_("mismatched algorithms: client %s; server %s"),
+ the_hash_algo->name, hash_name);
+ packet_buf_write(req_buf, "object-format=%s", the_hash_algo->name);
+ } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1_LEGACY) {
+ die(_("the server does not support algorithm '%s'"),
+ the_hash_algo->name);
+ }
+ packet_buf_delim(req_buf);
+}
+
static const char *url_scheme_name(enum url_scheme scheme)
{
switch (scheme) {
diff --git a/connect.h b/connect.h
index aa482a37fb..c4f6ea4b0a 100644
--- a/connect.h
+++ b/connect.h
@@ -34,4 +34,8 @@ void check_stateless_delimiter(int stateless_rpc,
struct packet_reader *reader,
const char *error);
+struct string_list;
+void write_fetch_command_and_capabilities(struct strbuf *req_buf,
+ const struct string_list *server_options);
+
#endif
diff --git a/fetch-pack.c b/fetch-pack.c
index 65ebfec09f..f7789e8456 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1375,40 +1375,6 @@ static int add_haves(struct fetch_negotiator *negotiator,
return haves_added;
}
-static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
- const struct string_list *server_options)
-{
- const char *hash_name;
- int advertise_sid = 0;
-
- repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
-
- ensure_server_supports_v2("fetch");
- packet_buf_write(req_buf, "command=fetch");
- if (server_supports_v2("agent"))
- packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
- if (advertise_sid && server_supports_v2("session-id"))
- packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
- if (server_options && server_options->nr) {
- ensure_server_supports_v2("server-option");
- for (size_t i = 0; i < server_options->nr; i++)
- packet_buf_write(req_buf, "server-option=%s",
- server_options->items[i].string);
- }
-
- if (server_feature_v2("object-format", &hash_name)) {
- int hash_algo = hash_algo_by_name(hash_name);
- if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
- die(_("mismatched algorithms: client %s; server %s"),
- the_hash_algo->name, hash_name);
- packet_buf_write(req_buf, "object-format=%s", the_hash_algo->name);
- } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1_LEGACY) {
- die(_("the server does not support algorithm '%s'"),
- the_hash_algo->name);
- }
- packet_buf_delim(req_buf);
-}
-
static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
struct fetch_pack_args *args,
const struct ref *wants, struct oidset *common,
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 03/13] t1006: extract helper functions into new 'lib-cat-file.sh'
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>
Extract utility functions from the cat-file's test script
't1006-cat-file.sh' into a new 'lib-cat-file.sh' dedicated library file.
A subsequent commit will need these functions. This improves the code
reuse and readability, enabling future cat-file tests to share these
helpers without duplicating code.
While at it update the style of this line to follow coding
guidelines:
. "$TEST_DIRECTORY/lib-loose.sh"
to
. "$TEST_DIRECTORY"/lib-loose.sh
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
t/lib-cat-file.sh | 16 ++++++++++++++++
t/t1006-cat-file.sh | 15 ++-------------
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/t/lib-cat-file.sh b/t/lib-cat-file.sh
new file mode 100644
index 0000000000..7c2e877016
--- /dev/null
+++ b/t/lib-cat-file.sh
@@ -0,0 +1,16 @@
+# Library of git-cat-file related test functions.
+
+# Print a string without a trailing newline.
+echo_without_newline () {
+ printf '%s' "$*"
+}
+
+# Print a string without newlines and replace them with a NUL character (\0).
+echo_without_newline_nul () {
+ echo_without_newline "$@" | tr '\n' '\0'
+}
+
+# Calculate the length of a string.
+strlen () {
+ echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
+}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 8e2c52652c..cf65bfc88f 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -3,7 +3,8 @@
test_description='git cat-file'
. ./test-lib.sh
-. "$TEST_DIRECTORY/lib-loose.sh"
+. "$TEST_DIRECTORY"/lib-loose.sh
+. "$TEST_DIRECTORY"/lib-cat-file.sh
test_cmdmode_usage () {
test_expect_code 129 "$@" 2>err &&
@@ -99,18 +100,6 @@ do
'
done
-echo_without_newline () {
- printf '%s' "$*"
-}
-
-echo_without_newline_nul () {
- echo_without_newline "$@" | tr '\n' '\0'
-}
-
-strlen () {
- echo_without_newline "$1" | wc -c | sed -e 's/^ *//'
-}
-
run_tests () {
type=$1
object_name="$2"
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 04/13] fetch-pack: drop the static advertise_sid variable
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>
write_fetch_command_and_capabilities() is moved to 'connect.c' in a
subsequent commit. To prepare for that, drop the static variable usage
of advertise_sid.
Currently advertise_sid is set in fetch_pack_config() by reading
"transfer.advertisesid". It is used in three places:
1. In do_fetch_pack(), to clear it when the server lacks support:
if (!server_supports("session-id"))
advertise_sid = 0;
2. In find_common(), to advertise the session id over protocol v0/v1:
if (advertise_sid)
strbuf_addf(&c, " session-id=%s", trace2_session_id());
3. In write_fetch_command_and_capabilities(), to advertise it over
protocol v2:
if (advertise_sid && server_supports_v2("session-id"))
packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
About 1, the check only guards the v0/v1 path, and the v2 path
already checks server support inline in its condition. Follow the
same pattern and fold the check into the condition in find_common().
About 2 and 3, replace the static variable with a local read via
repo_config_get_bool() in each function.
Because repo_config_get_bool() leaves advertise_sid as is if it is not
set, initialize it to 0, matching its default.
Helped-by: Jonathan Tan <jonathantanmy@google.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Calvin Wan <calvinwan@google.com>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
fetch-pack.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 9eb8fc5399..65ebfec09f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -49,7 +49,6 @@ static int fetch_fsck_objects = -1;
static int transfer_fsck_objects = -1;
static int agent_supported;
static int server_supports_filtering;
-static int advertise_sid;
static struct shallow_lock shallow_lock;
static const char *alternate_shallow_file;
static struct strbuf fsck_msg_types = STRBUF_INIT;
@@ -363,6 +362,9 @@ static int find_common(struct fetch_negotiator *negotiator,
size_t state_len = 0;
struct packet_reader reader;
struct oidset negotiation_include_oids = OIDSET_INIT;
+ int advertise_sid = 0;
+
+ repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
if (args->stateless_rpc && multi_ack == 1)
die(_("the option '%s' requires '%s'"), "--stateless-rpc", "multi_ack_detailed");
@@ -414,7 +416,7 @@ static int find_common(struct fetch_negotiator *negotiator,
if (deepen_not_ok) strbuf_addstr(&c, " deepen-not");
if (agent_supported) strbuf_addf(&c, " agent=%s",
git_user_agent_sanitized());
- if (advertise_sid)
+ if (advertise_sid && server_supports("session-id"))
strbuf_addf(&c, " session-id=%s", trace2_session_id());
if (args->filter_options.choice)
strbuf_addstr(&c, " filter");
@@ -1160,9 +1162,6 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
(int)agent_len, agent_feature);
}
- if (!server_supports("session-id"))
- advertise_sid = 0;
-
if (server_supports("shallow"))
print_verbose(args, _("Server supports %s"), "shallow");
else if (args->depth > 0 || is_repository_shallow(r))
@@ -1380,6 +1379,9 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
const struct string_list *server_options)
{
const char *hash_name;
+ int advertise_sid = 0;
+
+ repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
ensure_server_supports_v2("fetch");
packet_buf_write(req_buf, "command=fetch");
@@ -1998,7 +2000,6 @@ static void fetch_pack_config(void)
repo_config_get_bool(the_repository, "repack.usedeltabaseoffset", &prefer_ofs_delta);
repo_config_get_bool(the_repository, "fetch.fsckobjects", &fetch_fsck_objects);
repo_config_get_bool(the_repository, "transfer.fsckobjects", &transfer_fsck_objects);
- repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
if (!uri_protocols.nr) {
char *str;
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 02/13] cat-file: declare loop counter inside for()
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>
Declare loop counters in the for statement when they are only used
within the loop body, limiting their scope and improving readability.
While updating the loop counters, use size_t instead of int for counters
that iterate over object counts.
Update the 'nr' parameter of dispatch_calls() to size_t as all callers
already pass a value of that type.
Helped-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Eric Ju <eric.peijian@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
builtin/cat-file.c | 13 ++++---------
fetch-pack.c | 3 +--
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b4b99a73da..03afc44c5e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -721,14 +721,12 @@ static void dispatch_calls(struct batch_options *opt,
struct strbuf *output,
struct expand_data *data,
struct queued_cmd *cmd,
- int nr)
+ size_t nr)
{
- int i;
-
if (!opt->buffer_output)
die(_("flush is only for --buffer mode"));
- for (i = 0; i < nr; i++)
+ for (size_t i = 0; i < nr; i++)
cmd[i].fn(opt, cmd[i].line, output, data);
fflush(stdout);
@@ -736,9 +734,7 @@ static void dispatch_calls(struct batch_options *opt,
static void free_cmds(struct queued_cmd *cmd, size_t *nr)
{
- size_t i;
-
- for (i = 0; i < *nr; i++)
+ for (size_t i = 0; i < *nr; i++)
FREE_AND_NULL(cmd[i].line);
*nr = 0;
@@ -765,7 +761,6 @@ static void batch_objects_command(struct batch_options *opt,
size_t alloc = 0, nr = 0;
while (strbuf_getdelim_strip_crlf(&input, stdin, opt->input_delim) != EOF) {
- int i;
const struct parse_cmd *cmd = NULL;
const char *p = NULL, *cmd_end;
struct queued_cmd call = {0};
@@ -775,7 +770,7 @@ static void batch_objects_command(struct batch_options *opt,
if (isspace(*input.buf))
die(_("whitespace before command: '%s'"), input.buf);
- for (i = 0; i < ARRAY_SIZE(commands); i++) {
+ for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
if (!skip_prefix(input.buf, commands[i].name, &cmd_end))
continue;
diff --git a/fetch-pack.c b/fetch-pack.c
index 29c41132ee..9eb8fc5399 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1388,9 +1388,8 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
if (advertise_sid && server_supports_v2("session-id"))
packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
if (server_options && server_options->nr) {
- int i;
ensure_server_supports_v2("server-option");
- for (i = 0; i < server_options->nr; i++)
+ for (size_t i = 0; i < server_options->nr; i++)
packet_buf_write(req_buf, "server-option=%s",
server_options->items[i].string);
}
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 01/13] transport-helper: fix memory leak of helper on disconnect
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>
disconnect_helper() only frees data inside of the if(data->helper) block
[1]. When the transport is disconnected without the helper being fully
started, data->name allocated in transport_helper_init()
is never freed.
Move FREE_AND_NULL(data->name) outside the conditional block so it's
always freed on disconnect.
[1]: https://lore.kernel.org/git/05fbadbae2184479c87c37675dde7bd79b3e32ab.1716465556.git.ps@pks.im/
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
transport-helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/transport-helper.c b/transport-helper.c
index 80f90eb7ba..f195070788 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -266,9 +266,9 @@ static int disconnect_helper(struct transport *transport)
close(data->helper->out);
fclose(data->out);
res = finish_command(data->helper);
- FREE_AND_NULL(data->name);
FREE_AND_NULL(data->helper);
}
+ FREE_AND_NULL(data->name);
return res;
}
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v19 00/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: <20260715-ps-eric-work-rebase-v18-0-34d7adb051bb@gmail.com>
This patch series is a continuation of Eric Ju's
(eric.peijian@gmail.com) and Calvin Wan's (calvinwan@google.com) patch
series [1] and [2] respectively.
Sometimes it is beneficial to retrieve information about an object
without having to download it completely. The server logic for
retrieving size has already been implemented and merged in a2ba162cda
(object-info: support for retrieving object info, 2021-04-20) [3].
This patch series implements the client option for it.
Eric's series adds the remote-object-info command to cat-file
--batch-command. This command allows the client to make an object-info
command request to a server that supports protocol v2.
If the server uses protocol v2 but does not support the object-info
capability, cat-file --batch-command will die.
If a user attempts to use remote-object-info with protocol v1, cat-file
--batch-command will die.
Currently, only the size (%(objectsize)) is supported end to end in this
implementation. The type (%(objecttype)) is known by the client's
allow-list and request path but is not supported on the server side
nor the response parsing. A follow up series will add full end-to-end
support for %(objecttype).
The default format for remote-object-info is set to "%(objectname)
%(objectsize)". Once %(objecttype) is supported, the default format will
be unified accordingly.
If the batch command format includes unsupported fields such as
%(objecttype), %(objectsize:disk), or %(deltabase), the command will
return empty strings for each unsupported field.
This series completes Eric's work mainly with the refactor of the
validation of the placeholders with an allow-list that filters what the
client asks with what the server is capable of providing, following Jeff
King's idea [4].
Github CI: https://github.com/pabloosabaterr/git/actions/runs/29586719871
[1]: https://lore.kernel.org/git/20250221190451.12536-1-eric.peijian@gmail.com/
[2]: https://lore.kernel.org/git/20220728230210.2952731-1-calvinwan@google.com/#t
[3]: https://git.kernel.org/pub/scm/git/git.git/commit/?id=a2ba162cda2acc171c3e36acbbc854792b093cb7
[4]: https://lore.kernel.org/git/20250313060250.GH94015@coredump.intra.peff.net/
Changes in v19:
- Changed the commit structure:
- squashed v18 10th and 11th commits into:
cat-file: add remote-object-info to batch-command
- Moved after the refactor and renamed:
connect: use unsigned int for hash_algo_by_name() calls.
- Added a new commit:
protocol-caps: ...
- protocol-caps: check object existence regardless of the attributes
requested; a bare OID request now gets an existence check
Don't lazy-fetch on the server to answer object-info
requests (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK)
- gitprotocol-v2: correct the object-info response grammar to match
the implementations
- fetch-object-info: validate that each response line echoes the
requested object ID at that position
- cat-file: the remote-object-info default format no longer leaks into
subsequent info commands in the same session
- commit message and documentation fixes
---
Calvin Wan (3):
fetch-pack: move fetch initialization
serve: advertise object-info feature
transport: add client support for object-info
Eric Ju (3):
cat-file: declare loop counter inside for()
t1006: extract helper functions into new 'lib-cat-file.sh'
cat-file: add remote-object-info to batch-command
Pablo Sabater (7):
transport-helper: fix memory leak of helper on disconnect
fetch-pack: drop the static advertise_sid variable
fetch-pack: move write_fetch_command_and_capabilities() to connect.c
connect: use unsigned int for hash_algo_by_name() calls
connect: make write_fetch_command_and_capabilities() more generic
protocol-caps: check object existence regardless of the attributes requested
cat-file: make remote-object-info allow-list adapt to the server
Documentation/git-cat-file.adoc | 28 +-
Documentation/gitprotocol-v2.adoc | 21 +-
Makefile | 1 +
builtin/cat-file.c | 214 +++++++++-
connect.c | 38 +-
connect.h | 8 +
fetch-object-info.c | 156 +++++++
fetch-object-info.h | 25 ++
fetch-pack.c | 58 +--
meson.build | 1 +
object-file.c | 10 +
odb.h | 9 +
protocol-caps.c | 45 +-
serve.c | 5 +-
t/lib-cat-file.sh | 16 +
t/meson.build | 1 +
t/t1006-cat-file.sh | 15 +-
t/t1017-cat-file-remote-object-info.sh | 747 +++++++++++++++++++++++++++++++++
t/t5701-git-serve.sh | 63 +++
transport-helper.c | 12 +-
transport-internal.h | 8 +
transport.c | 44 ++
transport.h | 9 +
23 files changed, 1437 insertions(+), 97 deletions(-)
---
base-commit: 44de1520f08d1dfebc3ab2d9f644208eaa5ac925
^ permalink raw reply
* Re: [PATCH v2] copy: drop dependency on `the_repository`
From: Phillip Wood @ 2026-07-17 15:00 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Phillip Wood
In-Reply-To: <20260716-pks-copy-wo-the-repository-v2-1-8f5e32942929@pks.im>
Hi Patrick
This version looks good to me
Thanks
Phillip
On 16/07/2026 16:28, Patrick Steinhardt wrote:
> When copying a file we need to potentially adapt permissions of the new
> file based on whether or not "core.shared" is enabled. Parsing this
> configuration makes us implicitly depend on `the_repository`.
>
> Refactor the code to instead require the caller to pass in a repository
> so that we can remove `USE_THE_REPOSITORY_VARIABLE`.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Hi,
>
> I guess the title says it all: this small patch removes the dependency
> on `the_repository` in "copy.c". Thanks!
>
> Changes in v2:
> - Adapt a couple more sites to use a repository from the context.
> - Link to v1: https://patch.msgid.link/20260716-pks-copy-wo-the-repository-v1-1-8f1e078bb82f@pks.im
>
> Patrick
> ---
> builtin/clone.c | 2 +-
> builtin/difftool.c | 4 ++--
> builtin/worktree.c | 4 ++--
> bundle-uri.c | 2 +-
> copy.c | 12 ++++++------
> copy.h | 8 ++++++--
> refs/files-backend.c | 2 +-
> rerere.c | 2 +-
> sequencer.c | 6 +++---
> setup.c | 2 +-
> 10 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index d60d1b60bc..18603dd4ce 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -335,7 +335,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
> die_errno(_("failed to create link '%s'"), dest->buf);
> option_no_hardlinks = 1;
> }
> - if (copy_file_with_time(dest->buf, src->buf, 0666))
> + if (copy_file_with_time(the_repository, dest->buf, src->buf, 0666))
> die_errno(_("failed to copy file to '%s'"), dest->buf);
> }
>
> diff --git a/builtin/difftool.c b/builtin/difftool.c
> index 26778f8515..5e7777fbe4 100644
> --- a/builtin/difftool.c
> +++ b/builtin/difftool.c
> @@ -552,7 +552,7 @@ static int run_dir_diff(struct repository *repo,
> struct stat st;
> if (stat(wtdir.buf, &st))
> st.st_mode = 0644;
> - if (copy_file(rdir.buf, wtdir.buf,
> + if (copy_file(repo, rdir.buf, wtdir.buf,
> st.st_mode)) {
> ret = error("could not copy '%s' to '%s'", wtdir.buf, rdir.buf);
> goto finish;
> @@ -658,7 +658,7 @@ static int run_dir_diff(struct repository *repo,
> warning("%s", "");
> err = 1;
> } else if (unlink(wtdir.buf) ||
> - copy_file(wtdir.buf, rdir.buf, st.st_mode))
> + copy_file(repo, wtdir.buf, rdir.buf, st.st_mode))
> warning_errno(_("could not copy '%s' to '%s'"),
> rdir.buf, wtdir.buf);
> }
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index d21c43fde3..84b01960fb 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -349,7 +349,7 @@ static void copy_sparse_checkout(const char *worktree_git_dir)
>
> if (file_exists(from_file)) {
> if (safe_create_leading_directories(the_repository, to_file) ||
> - copy_file(to_file, from_file, 0666))
> + copy_file(the_repository, to_file, from_file, 0666))
> error(_("failed to copy '%s' to '%s'; sparse-checkout may not work correctly"),
> from_file, to_file);
> }
> @@ -368,7 +368,7 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
> int bare;
>
> if (safe_create_leading_directories(the_repository, to_file) ||
> - copy_file(to_file, from_file, 0666)) {
> + copy_file(the_repository, to_file, from_file, 0666)) {
> error(_("failed to copy worktree config from '%s' to '%s'"),
> from_file, to_file);
> goto worktree_copy_cleanup;
> diff --git a/bundle-uri.c b/bundle-uri.c
> index 3b2e347288..ef37aebf30 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c
> @@ -396,7 +396,7 @@ static int copy_uri_to_file(const char *filename, const char *uri)
> uri = out;
>
> /* Copy as a file */
> - return copy_file(filename, uri, 0);
> + return copy_file(the_repository, filename, uri, 0);
> }
>
> static int unbundle_from_file(struct repository *r, const char *file)
> diff --git a/copy.c b/copy.c
> index b668209b6c..6074132050 100644
> --- a/copy.c
> +++ b/copy.c
> @@ -1,5 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> -
> #include "git-compat-util.h"
> #include "copy.h"
> #include "path.h"
> @@ -35,7 +33,8 @@ static int copy_times(const char *dst, const char *src)
> return 0;
> }
>
> -int copy_file(const char *dst, const char *src, int mode)
> +int copy_file(struct repository *repo,
> + const char *dst, const char *src, int mode)
> {
> int fdi, fdo, status;
>
> @@ -59,15 +58,16 @@ int copy_file(const char *dst, const char *src, int mode)
> if (close(fdo) != 0)
> return error_errno("%s: close error", dst);
>
> - if (!status && adjust_shared_perm(the_repository, dst))
> + if (!status && adjust_shared_perm(repo, dst))
> return -1;
>
> return status;
> }
>
> -int copy_file_with_time(const char *dst, const char *src, int mode)
> +int copy_file_with_time(struct repository *repo,
> + const char *dst, const char *src, int mode)
> {
> - int status = copy_file(dst, src, mode);
> + int status = copy_file(repo, dst, src, mode);
> if (!status)
> return copy_times(dst, src);
> return status;
> diff --git a/copy.h b/copy.h
> index 2af77cba86..1059b118d6 100644
> --- a/copy.h
> +++ b/copy.h
> @@ -1,10 +1,14 @@
> #ifndef COPY_H
> #define COPY_H
>
> +struct repository;
> +
> #define COPY_READ_ERROR (-2)
> #define COPY_WRITE_ERROR (-3)
> int copy_fd(int ifd, int ofd);
> -int copy_file(const char *dst, const char *src, int mode);
> -int copy_file_with_time(const char *dst, const char *src, int mode);
> +int copy_file(struct repository *repo,
> + const char *dst, const char *src, int mode);
> +int copy_file_with_time(struct repository *repo,
> + const char *dst, const char *src, int mode);
>
> #endif /* COPY_H */
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 3df56c25c8..442c98414e 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1736,7 +1736,7 @@ static int files_copy_or_rename_ref(struct ref_store *ref_store,
> goto out;
> }
>
> - if (copy && log && copy_file(tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
> + if (copy && log && copy_file(refs->base.repo, tmp_renamed_log.buf, sb_oldref.buf, 0644)) {
> ret = error("unable to copy logfile logs/%s to logs/"TMP_RENAMED_LOG": %s",
> oldrefname, strerror(errno));
> goto out;
> diff --git a/rerere.c b/rerere.c
> index 8232542585..bf5cfc6e51 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -756,7 +756,7 @@ static void do_rerere_one_path(struct index_state *istate,
> /* Has the user resolved it already? */
> if (variant >= 0) {
> if (!handle_file(istate, path, NULL, NULL)) {
> - copy_file(rerere_path(&buf, id, "postimage"), path, 0666);
> + copy_file(the_repository, rerere_path(&buf, id, "postimage"), path, 0666);
> id->collection->status[variant] |= RR_HAS_POSTIMAGE;
> fprintf_ln(stderr, _("Recorded resolution for '%s'."), path);
> free_rerere_id(rr_item);
> diff --git a/sequencer.c b/sequencer.c
> index 1355a99a09..63bc1ef215 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2419,7 +2419,7 @@ static int do_pick_commit(struct repository *r,
> } else {
> const char *dest = git_path_squash_msg(r);
> unlink(dest);
> - if (copy_file(dest, rebase_path_squash_msg(), 0666)) {
> + if (copy_file(r, dest, rebase_path_squash_msg(), 0666)) {
> res = error(_("could not copy '%s' to '%s'"),
> rebase_path_squash_msg(), dest);
> goto leave;
> @@ -3864,11 +3864,11 @@ static int error_failed_squash(struct repository *r,
> int subject_len,
> const char *subject)
> {
> - if (copy_file(rebase_path_message(), rebase_path_squash_msg(), 0666))
> + if (copy_file(r, rebase_path_message(), rebase_path_squash_msg(), 0666))
> return error(_("could not copy '%s' to '%s'"),
> rebase_path_squash_msg(), rebase_path_message());
> unlink(git_path_merge_msg(r));
> - if (copy_file(git_path_merge_msg(r), rebase_path_message(), 0666))
> + if (copy_file(r, git_path_merge_msg(r), rebase_path_message(), 0666))
> return error(_("could not copy '%s' to '%s'"),
> rebase_path_message(),
> git_path_merge_msg(r));
> diff --git a/setup.c b/setup.c
> index 0de56a074f..91d61a5939 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -2331,7 +2331,7 @@ static void copy_templates_1(struct repository *repo,
> strbuf_release(&lnk);
> }
> else if (S_ISREG(st_template.st_mode)) {
> - if (copy_file(path->buf, template_path->buf, st_template.st_mode))
> + if (copy_file(repo, path->buf, template_path->buf, st_template.st_mode))
> die_errno(_("cannot copy '%s' to '%s'"),
> template_path->buf, path->buf);
> }
>
> ---
> base-commit: d35c5399e3e54ac277bb391fc2f6be3e816d312b
> change-id: 20260716-pks-copy-wo-the-repository-aa01ccdbed76
>
>
^ permalink raw reply
* Re: [PATCH 2/6] MyFirstContribution: what if I don't get a reply?
From: Junio C Hamano @ 2026-07-17 14:59 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <aloOAwOtutgPbJu2@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> On Sat, Jul 11, 2026 at 12:26:46PM -0700, Junio C Hamano wrote:
>> Tell readers that pinging is a perfectly sensible thing to do when
>> they do not see a response.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> Documentation/MyFirstContribution.adoc | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
>> index 4832e5bad5..fc2ce2e785 100644
>> --- a/Documentation/MyFirstContribution.adoc
>> +++ b/Documentation/MyFirstContribution.adoc
>> @@ -1438,6 +1438,19 @@ substantial rework, and mention which parts of the current series will become
>> obsolete so reviewers can avoid spending time on them until the updated series
>> is ready.
>>
>> +=== What if I don't get a reply?
>> +
>> +If you don't receive any review comments after a week or two, do not
>> +assume your patch has been accepted or merged. In the Git project,
>> +silence does not equal approval. It usually means reviewers are busy
>> +or haven't noticed your contribution.
>
> Should we also add the third reason: reviewers are simply not interested
> in the patch? It's a bit brutal, but that's quite a common reason, too.
Yeah, I agree that it would make a good addition.
^ permalink raw reply
* Re: [PATCH v7 3/3] replay: offer an option to linearize the commit topology
From: Junio C Hamano @ 2026-07-17 14:57 UTC (permalink / raw)
To: Elijah Newren; +Cc: Toon Claes, git, Johannes Schindelin
In-Reply-To: <CABPp-BGdK8v8Qk5XB=QL_yJDPTNjSb2rN08GiPpK50V2gAj1QQ@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
> On Wed, Jul 15, 2026 at 11:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Elijah Newren <newren@gmail.com> writes:
>>
>> But if that is not the outcome they wanted, I fail to see why they
>> would feed all three branches to a single invocation of --linearize
>> in the first place. After all, the command is only doing what it
>> was asked to do.
>
> Passing several branches isn't the user asking for concatenation; it's
> the user asking for replay's core feature: update many branches at
> once. Adding --linearize to flatten a merge does have to join the
> lines which that merge combined, but it shouldn't also weld together
> branches that were never merged in the first place. The user is
> combining two intended features, and the concatenation is an emergent
> third behavior that neither of them implies.
I am not yet convinced by the above.
* The fact that the user ran 'git replay' indicates that they
want the command's core feature of updating multiple
branches.
* The fact that the user specified '--linearize' indicates that
they want a linear history, regardless of the number of positive
branch tips they gave.
So from that point of view, I still think it reasonable to expect
such a history to be linearized.
In any case, I am not the primary audience for this new feature,
and I have no desire to dictate the design one way or the other.
Let us hear what the topic author has to say.
I will mark the topic as "On hold, waiting for response".
Thanks.
^ permalink raw reply
* [PATCH v2 2/2] builtin/history: sign rewritten commits
From: Souma @ 2026-07-17 14:51 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Souma
In-Reply-To: <20260703145037.69832-1-git@5ouma.me>
The history commands create replacement commits directly instead of
using the sequencer or the commit porcelain. As a result, rewritten
commits ignore `commit.gpgSign` and cannot be signed on demand.
Read the signing configuration before parsing options so that it
establishes the default and later `-S`/`--gpg-sign` or `--no-gpg-sign`
options override it. Pass the selected key through direct rewrites and
the replay machinery.
Sign every newly created commit, including both halves of a split and
replayed descendants. Dropping the tip creates no replacement commit,
so there is nothing to sign. As with `rebase --gpg-sign`, the signature
records the attestation of the current committer to the rewritten
commit while retaining the original author identity; it does not claim
authorship of commits written by somebody else.
Document the behavior and add GPG-gated coverage for configuration,
command-line overrides, last-option-wins precedence, replayed
descendants, split commits, an explicit signing key, and the
no-new-commit drop case.
Signed-off-by: Souma <git@5ouma.me>
---
Documentation/git-history.adoc | 16 +++++--
builtin/history.c | 84 ++++++++++++++++++++++++++--------
t/t3451-history-reword.sh | 63 +++++++++++++++++++++++++
t/t3452-history-split.sh | 44 ++++++++++++++++++
t/t3453-history-fixup.sh | 39 ++++++++++++++++
t/t3454-history-drop.sh | 50 ++++++++++++++++++++
6 files changed, 272 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 28b477cd37..8345cced4c 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -8,10 +8,10 @@ git-history - EXPERIMENTAL: Rewrite history
SYNOPSIS
--------
[synopsis]
-git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]
-git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
-git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
-git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
+git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]
+git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]
+git history reword <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]]
+git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]] [--] [<pathspec>...]
DESCRIPTION
-----------
@@ -125,6 +125,14 @@ OPTIONS
`--reedit-message`::
Open an editor to modify the target commit's message.
+`-S[<key-id>]`::
+`--gpg-sign[=<key-id>]`::
+`--no-gpg-sign`::
+ GPG-sign rewritten commits. The _<key-id>_ argument is optional and
+ defaults to the committer identity; if specified, it must be stuck to
+ the option without a space. `--no-gpg-sign` is useful to countermand
+ both `commit.gpgSign` configuration and earlier `--gpg-sign`.
+
`--empty=(drop|keep|abort)`::
Control what happens when a commit becomes empty as a result of the
fixup. This can happen in two situations:
diff --git a/builtin/history.c b/builtin/history.c
index d28c1f08bb..97e0d77013 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -27,13 +27,13 @@
#include "wt-status.h"
#define GIT_HISTORY_DROP_USAGE \
- N_("git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]")
+ N_("git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]")
#define GIT_HISTORY_FIXUP_USAGE \
- N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]")
+ N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]")
#define GIT_HISTORY_REWORD_USAGE \
- N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
+ N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]]")
#define GIT_HISTORY_SPLIT_USAGE \
- N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
+ N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]] [--] [<pathspec>...]")
static void change_data_free(void *util, const char *str UNUSED)
{
@@ -105,12 +105,37 @@ enum commit_tree_flags {
COMMIT_TREE_EDIT_MESSAGE = (1 << 0),
};
+static int history_config(const char *var, const char *value,
+ const struct config_context *ctx, void *data)
+{
+ const char **sign_commit = data;
+
+ if (!strcmp(var, "commit.gpgsign")) {
+ *sign_commit = git_config_bool(var, value) ? "" : NULL;
+ return 0;
+ }
+
+ return git_default_config(var, value, ctx, NULL);
+}
+
+#define OPT_HISTORY_GPG_SIGN(v) { \
+ .type = OPTION_STRING, \
+ .short_name = 'S', \
+ .long_name = "gpg-sign", \
+ .value = (v), \
+ .argh = N_("key-id"), \
+ .help = N_("GPG-sign rewritten commits"), \
+ .flags = PARSE_OPT_OPTARG, \
+ .defval = (intptr_t)"", \
+}
+
static int commit_tree_ext(struct repository *repo,
const char *action,
struct commit *commit_with_message,
const struct commit_list *parents,
const struct object_id *old_tree,
const struct object_id *new_tree,
+ const char *sign_commit,
struct commit **out,
enum commit_tree_flags flags)
{
@@ -151,7 +176,7 @@ static int commit_tree_ext(struct repository *repo,
ret = commit_tree_extended(commit_message.buf, commit_message.len, new_tree,
parents, &rewritten_commit_oid, original_author,
- NULL, NULL, original_extra_headers);
+ NULL, sign_commit, original_extra_headers);
if (ret < 0)
goto out;
@@ -167,6 +192,7 @@ static int commit_tree_ext(struct repository *repo,
static int commit_tree_with_edited_message(struct repository *repo,
const char *action,
struct commit *original,
+ const char *sign_commit,
struct commit **out)
{
struct object_id parent_tree_oid;
@@ -188,7 +214,8 @@ static int commit_tree_with_edited_message(struct repository *repo,
}
return commit_tree_ext(repo, action, original, original->parents,
- &parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
+ &parent_tree_oid, tree_oid, sign_commit, out,
+ COMMIT_TREE_EDIT_MESSAGE);
}
enum ref_action {
@@ -344,12 +371,14 @@ static int compute_pending_ref_updates(struct rev_info *revs,
enum ref_action action,
struct commit *original,
struct commit *rewritten,
+ const char *sign_commit,
enum replay_empty_commit_action empty,
struct replay_result *result)
{
const struct name_decoration *decoration;
struct replay_revisions_options opts = {
.empty = empty,
+ .sign_commit = sign_commit,
};
char hex[GIT_MAX_HEXSZ + 1];
bool detached_head;
@@ -454,13 +483,14 @@ static int handle_reference_updates(struct rev_info *revs,
struct commit *rewritten,
const char *reflog_msg,
int dry_run,
+ const char *sign_commit,
enum replay_empty_commit_action empty)
{
struct replay_result result = { 0 };
int ret;
ret = compute_pending_ref_updates(revs, action, original, rewritten,
- empty, &result);
+ sign_commit, empty, &result);
if (ret)
goto out;
@@ -522,6 +552,7 @@ static int cmd_history_fixup(int argc,
enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
enum ref_action action = REF_ACTION_DEFAULT;
enum commit_tree_flags flags = 0;
+ const char *sign_commit = NULL;
int dry_run = 0;
struct option options[] = {
OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
@@ -535,6 +566,7 @@ static int cmd_history_fixup(int argc,
OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
N_("how to handle commits that become empty"),
PARSE_OPT_NONEG, parse_opt_empty),
+ OPT_HISTORY_GPG_SIGN(&sign_commit),
OPT_END(),
};
struct merge_result merge_result = { 0 };
@@ -546,12 +578,12 @@ static int cmd_history_fixup(int argc,
bool skip_commit = false;
int ret;
+ repo_config(repo, history_config, &sign_commit);
argc = parse_options(argc, argv, prefix, options, usage, 0);
if (argc != 1) {
ret = error(_("command expects a single revision"));
goto out;
}
- repo_config(repo, git_default_config, NULL);
if (action == REF_ACTION_DEFAULT)
action = REF_ACTION_BRANCHES;
@@ -676,7 +708,7 @@ static int cmd_history_fixup(int argc,
if (!skip_commit) {
ret = commit_tree_ext(repo, "fixup", original, original->parents,
&original_tree->object.oid, &merge_result.tree->object.oid,
- &rewritten, flags);
+ sign_commit, &rewritten, flags);
if (ret < 0) {
ret = error(_("failed writing fixed-up commit"));
goto out;
@@ -686,7 +718,7 @@ static int cmd_history_fixup(int argc,
strbuf_addf(&reflog_msg, "fixup: updating %s", argv[0]);
ret = handle_reference_updates(&revs, action, original, rewritten,
- reflog_msg.buf, dry_run, empty);
+ reflog_msg.buf, dry_run, sign_commit, empty);
if (ret < 0) {
ret = error(_("failed replaying descendants"));
goto out;
@@ -711,6 +743,7 @@ static int cmd_history_reword(int argc,
NULL,
};
enum ref_action action = REF_ACTION_DEFAULT;
+ const char *sign_commit = NULL;
int dry_run = 0;
struct option options[] = {
OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
@@ -718,6 +751,7 @@ static int cmd_history_reword(int argc,
PARSE_OPT_NONEG, parse_ref_action),
OPT_BOOL('n', "dry-run", &dry_run,
N_("perform a dry-run without updating any refs")),
+ OPT_HISTORY_GPG_SIGN(&sign_commit),
OPT_END(),
};
struct strbuf reflog_msg = STRBUF_INIT;
@@ -725,12 +759,12 @@ static int cmd_history_reword(int argc,
struct rev_info revs = { 0 };
int ret;
+ repo_config(repo, history_config, &sign_commit);
argc = parse_options(argc, argv, prefix, options, usage, 0);
if (argc != 1) {
ret = error(_("command expects a single revision"));
goto out;
}
- repo_config(repo, git_default_config, NULL);
if (action == REF_ACTION_DEFAULT)
action = REF_ACTION_BRANCHES;
@@ -745,7 +779,8 @@ static int cmd_history_reword(int argc,
if (ret)
goto out;
- ret = commit_tree_with_edited_message(repo, "reworded", original, &rewritten);
+ ret = commit_tree_with_edited_message(repo, "reworded", original,
+ sign_commit, &rewritten);
if (ret < 0) {
ret = error(_("failed writing reworded commit"));
goto out;
@@ -754,7 +789,8 @@ static int cmd_history_reword(int argc,
strbuf_addf(&reflog_msg, "reword: updating %s", argv[0]);
ret = handle_reference_updates(&revs, action, original, rewritten,
- reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
+ reflog_msg.buf, dry_run, sign_commit,
+ REPLAY_EMPTY_COMMIT_ABORT);
if (ret < 0) {
ret = error(_("failed replaying descendants"));
goto out;
@@ -816,6 +852,7 @@ static int write_ondisk_index(struct repository *repo,
static int split_commit(struct repository *repo,
struct commit *original,
struct pathspec *pathspec,
+ const char *sign_commit,
struct commit **out)
{
struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT;
@@ -893,7 +930,8 @@ static int split_commit(struct repository *repo,
* that shall be diffed against is the parent of the original commit.
*/
ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
- &split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
+ &split_tree->object.oid, sign_commit, &first_commit,
+ COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing first commit"));
goto out;
@@ -910,7 +948,8 @@ static int split_commit(struct repository *repo,
new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
- new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
+ new_tree_oid, sign_commit, &second_commit,
+ COMMIT_TREE_EDIT_MESSAGE);
if (ret < 0) {
ret = error(_("failed writing second commit"));
goto out;
@@ -938,6 +977,7 @@ static int cmd_history_split(int argc,
NULL,
};
enum ref_action action = REF_ACTION_DEFAULT;
+ const char *sign_commit = NULL;
int dry_run = 0;
struct option options[] = {
OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
@@ -945,6 +985,7 @@ static int cmd_history_split(int argc,
PARSE_OPT_NONEG, parse_ref_action),
OPT_BOOL('n', "dry-run", &dry_run,
N_("perform a dry-run without updating any refs")),
+ OPT_HISTORY_GPG_SIGN(&sign_commit),
OPT_END(),
};
struct commit *original, *rewritten = NULL;
@@ -953,12 +994,12 @@ static int cmd_history_split(int argc,
struct rev_info revs = { 0 };
int ret;
+ repo_config(repo, history_config, &sign_commit);
argc = parse_options(argc, argv, prefix, options, usage, 0);
if (argc < 1) {
ret = error(_("command expects a committish"));
goto out;
}
- repo_config(repo, git_default_config, NULL);
if (action == REF_ACTION_DEFAULT)
action = REF_ACTION_BRANCHES;
@@ -984,14 +1025,15 @@ static int cmd_history_split(int argc,
goto out;
}
- ret = split_commit(repo, original, &pathspec, &rewritten);
+ ret = split_commit(repo, original, &pathspec, sign_commit, &rewritten);
if (ret < 0)
goto out;
strbuf_addf(&reflog_msg, "split: updating %s", argv[0]);
ret = handle_reference_updates(&revs, action, original, rewritten,
- reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
+ reflog_msg.buf, dry_run, sign_commit,
+ REPLAY_EMPTY_COMMIT_ABORT);
if (ret < 0) {
ret = error(_("failed replaying descendants"));
goto out;
@@ -1081,6 +1123,7 @@ static int cmd_history_drop(int argc,
};
enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
enum ref_action action = REF_ACTION_DEFAULT;
+ const char *sign_commit = NULL;
int dry_run = 0;
struct option options[] = {
OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
@@ -1091,6 +1134,7 @@ static int cmd_history_drop(int argc,
OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
N_("how to handle descendants that become empty"),
PARSE_OPT_NONEG, parse_opt_empty),
+ OPT_HISTORY_GPG_SIGN(&sign_commit),
OPT_END(),
};
struct strbuf reflog_msg = STRBUF_INIT;
@@ -1101,12 +1145,12 @@ static int cmd_history_drop(int argc,
bool head_moves = false;
int ret;
+ repo_config(repo, history_config, &sign_commit);
argc = parse_options(argc, argv, prefix, options, usage, 0);
if (argc != 1) {
ret = error(_("command expects a single revision"));
goto out;
}
- repo_config(repo, git_default_config, NULL);
if (action == REF_ACTION_DEFAULT)
action = REF_ACTION_BRANCHES;
@@ -1134,7 +1178,7 @@ static int cmd_history_drop(int argc,
rewritten = original->parents->item;
ret = compute_pending_ref_updates(&revs, action, original, rewritten,
- empty, &result);
+ sign_commit, empty, &result);
if (ret) {
ret = error(_("failed replaying descendants"));
goto out;
diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
index de7b357685..6dbe2143d3 100755
--- a/t/t3451-history-reword.sh
+++ b/t/t3451-history-reword.sh
@@ -4,6 +4,7 @@ test_description='tests for git-history reword subcommand'
. ./test-lib.sh
. "$TEST_DIRECTORY/lib-log-graph.sh"
+. "$TEST_DIRECTORY/lib-gpg.sh"
reword_with_message () {
cat >message &&
@@ -26,6 +27,37 @@ expect_log () {
test_cmp expect actual
}
+test_reword_gpg_sign () {
+ must_fail= will=will
+ if test "x$1" = "x!"
+ then
+ must_fail=test_must_fail
+ will="will not"
+ shift
+ fi
+ conf=$1
+ shift
+
+ test_expect_success GPG "reword $* with commit.gpgsign=$conf $will sign rewritten history" "
+ test_when_finished 'rm -rf repo' &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git config commit.gpgsign $conf &&
+ reword_with_message $* HEAD~ <<-EOF &&
+ second reworded
+ EOF
+
+ $must_fail git verify-commit HEAD~ &&
+ $must_fail git verify-commit HEAD
+ )
+ "
+}
+
test_expect_success 'can reword tip of a branch' '
test_when_finished "rm -rf repo" &&
git init repo &&
@@ -77,6 +109,37 @@ test_expect_success 'can reword commit in the middle' '
)
'
+test_reword_gpg_sign ! false
+test_reword_gpg_sign true
+test_reword_gpg_sign false --gpg-sign
+test_reword_gpg_sign ! true --no-gpg-sign
+test_reword_gpg_sign ! true --gpg-sign --no-gpg-sign
+test_reword_gpg_sign false --no-gpg-sign --gpg-sign
+
+test_expect_success GPG 'reword uses an explicit signing key for rewritten history' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ reword_with_message -SB7227189 HEAD~ <<-EOF &&
+ second reworded
+ EOF
+
+ git verify-commit HEAD~ &&
+ git verify-commit HEAD &&
+ git log -2 --format=%GK >actual &&
+ cat >expect <<-\EOF &&
+ 65A0EEA02E30CAD7
+ 65A0EEA02E30CAD7
+ EOF
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'can reword commit in the middle even on detached head' '
test_when_finished "rm -rf repo" &&
git init repo &&
diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
index 8ed0cebb50..e96f492cc6 100755
--- a/t/t3452-history-split.sh
+++ b/t/t3452-history-split.sh
@@ -4,6 +4,7 @@ test_description='tests for git-history split subcommand'
. ./test-lib.sh
. "$TEST_DIRECTORY/lib-log-graph.sh"
+. "$TEST_DIRECTORY/lib-gpg.sh"
# The fake editor takes multiple arguments, each of which represents a commit
# message. Subsequent invocations of the editor will then yield those messages
@@ -36,6 +37,42 @@ expect_tree_entries () {
test_cmp expect actual
}
+test_split_gpg_sign () {
+ must_fail= will=will
+ if test "x$1" = "x!"
+ then
+ must_fail=test_must_fail
+ will="will not"
+ shift
+ fi
+ conf=$1
+ shift
+
+ test_expect_success GPG "split $* with commit.gpgsign=$conf $will sign rewritten history" "
+ test_when_finished 'rm -rf repo' &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit initial &&
+ touch bar foo &&
+ git add . &&
+ git commit -m split-me &&
+ test_commit tip &&
+
+ git config commit.gpgsign $conf &&
+ set_fake_editor 'first' 'second' &&
+ git history split $* HEAD~ <<-EOF &&
+ y
+ n
+ EOF
+
+ $must_fail git verify-commit HEAD~2 &&
+ $must_fail git verify-commit HEAD~ &&
+ $must_fail git verify-commit HEAD
+ )
+ "
+}
+
test_expect_success 'refuses to work with merge commits' '
test_when_finished "rm -rf repo" &&
git init repo &&
@@ -141,6 +178,13 @@ test_expect_success 'can split up tip commit' '
)
'
+test_split_gpg_sign ! false
+test_split_gpg_sign true
+test_split_gpg_sign false --gpg-sign
+test_split_gpg_sign ! true --no-gpg-sign
+test_split_gpg_sign ! true --gpg-sign --no-gpg-sign
+test_split_gpg_sign false --no-gpg-sign --gpg-sign
+
test_expect_success 'can split up root commit' '
test_when_finished "rm -rf repo" &&
git init repo &&
diff --git a/t/t3453-history-fixup.sh b/t/t3453-history-fixup.sh
index 868298e248..cd20a23115 100755
--- a/t/t3453-history-fixup.sh
+++ b/t/t3453-history-fixup.sh
@@ -3,6 +3,7 @@
test_description='tests for git-history fixup subcommand'
. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
fixup_with_message () {
cat >message &&
@@ -21,6 +22,37 @@ expect_changes () {
test_cmp expect actual
}
+test_fixup_gpg_sign () {
+ must_fail= will=will
+ if test "x$1" = "x!"
+ then
+ must_fail=test_must_fail
+ will="will not"
+ shift
+ fi
+ conf=$1
+ shift
+
+ test_expect_success GPG "fixup $* with commit.gpgsign=$conf $will sign rewritten history" "
+ test_when_finished 'rm -rf repo' &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git config commit.gpgsign $conf &&
+ echo fix >>second.t &&
+ git add second.t &&
+ git history fixup $* HEAD~ &&
+
+ $must_fail git verify-commit HEAD~ &&
+ $must_fail git verify-commit HEAD
+ )
+ "
+}
+
test_expect_success 'errors on missing commit argument' '
test_when_finished "rm -rf repo" &&
git init repo &&
@@ -229,6 +261,13 @@ test_expect_success 'preserves commit message and authorship' '
)
'
+test_fixup_gpg_sign ! false
+test_fixup_gpg_sign true
+test_fixup_gpg_sign false --gpg-sign
+test_fixup_gpg_sign ! true --no-gpg-sign
+test_fixup_gpg_sign ! true --gpg-sign --no-gpg-sign
+test_fixup_gpg_sign false --no-gpg-sign --gpg-sign
+
test_expect_success 'updates all descendant branches by default' '
test_when_finished "rm -rf repo" &&
git init repo --initial-branch=main &&
diff --git a/t/t3454-history-drop.sh b/t/t3454-history-drop.sh
index 68a86d1e37..5b21078a7e 100755
--- a/t/t3454-history-drop.sh
+++ b/t/t3454-history-drop.sh
@@ -4,6 +4,7 @@ test_description='tests for git-history drop subcommand'
. ./test-lib.sh
. "$TEST_DIRECTORY/lib-log-graph.sh"
+. "$TEST_DIRECTORY/lib-gpg.sh"
expect_graph () {
cat >expect &&
@@ -16,6 +17,34 @@ expect_log () {
test_cmp expect actual
}
+test_drop_gpg_sign () {
+ must_fail= will=will
+ if test "x$1" = "x!"
+ then
+ must_fail=test_must_fail
+ will="will not"
+ shift
+ fi
+ conf=$1
+ shift
+
+ test_expect_success GPG "drop $* with commit.gpgsign=$conf $will sign replayed descendants" "
+ test_when_finished 'rm -rf repo' &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git config commit.gpgsign $conf &&
+ git history drop $* HEAD~ &&
+
+ $must_fail git verify-commit HEAD
+ )
+ "
+}
+
test_expect_success 'errors on missing commit argument' '
test_when_finished "rm -rf repo" &&
git init repo &&
@@ -88,6 +117,27 @@ test_expect_success 'drops a commit in the middle and replays descendants' '
)
'
+test_drop_gpg_sign ! false
+test_drop_gpg_sign true
+test_drop_gpg_sign false --gpg-sign
+test_drop_gpg_sign ! true --no-gpg-sign
+test_drop_gpg_sign ! true --gpg-sign --no-gpg-sign
+test_drop_gpg_sign false --no-gpg-sign --gpg-sign
+
+test_expect_success GPG 'drop has no commit to sign when dropping the tip' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ test_commit first &&
+ test_commit second &&
+
+ git history drop --gpg-sign HEAD &&
+
+ test_must_fail git verify-commit HEAD
+ )
+'
+
test_expect_success 'drops the HEAD commit' '
test_when_finished "rm -rf repo" &&
git init repo &&
--
2.55.0
^ permalink raw reply related
* [PATCH v2 1/2] replay: allow callers to sign commits
From: Souma @ 2026-07-17 14:51 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Souma
In-Reply-To: <20260703145037.69832-1-git@5ouma.me>
The replay machinery creates commits directly through
`commit_tree_extended()`, but callers cannot currently request
signatures. Commands that replay rewritten history consequently cannot
carry their signing policy through to descendant commits.
Add `sign_commit` to `replay_revisions_options` and thread it through
commit creation. `NULL` preserves the existing unsigned behavior, an
empty string selects the default signing key, and a non-empty string
selects an explicit key. Existing callers zero-initialize the options
structure, so their behavior is unchanged.
Signed-off-by: Souma <git@5ouma.me>
---
replay.c | 13 ++++++++-----
replay.h | 6 ++++++
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/replay.c b/replay.c
index aac9178875..19a6402bf0 100644
--- a/replay.c
+++ b/replay.c
@@ -81,13 +81,13 @@ static struct commit *create_commit(struct repository *repo,
struct tree *tree,
struct commit *based_on,
struct commit *parent,
- enum replay_mode mode)
+ enum replay_mode mode,
+ const char *sign_commit)
{
struct object_id ret;
struct object *obj = NULL;
struct commit_list *parents = NULL;
char *author = NULL;
- char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
struct commit_extra_header *extra = NULL;
struct strbuf msg = STRBUF_INIT;
const char *out_enc = get_commit_output_encoding();
@@ -270,7 +270,8 @@ static struct commit *pick_regular_commit(struct repository *repo,
struct merge_options *merge_opt,
struct merge_result *result,
enum replay_mode mode,
- enum replay_empty_commit_action empty)
+ enum replay_empty_commit_action empty,
+ const char *sign_commit)
{
struct commit *base, *replayed_base;
struct tree *pickme_tree, *base_tree, *replayed_base_tree;
@@ -341,7 +342,8 @@ static struct commit *pick_regular_commit(struct repository *repo,
}
}
- return create_commit(repo, result->tree, pickme, replayed_base, mode);
+ return create_commit(repo, result->tree, pickme, replayed_base, mode,
+ sign_commit);
}
void replay_result_release(struct replay_result *result)
@@ -431,7 +433,8 @@ int replay_revisions(struct rev_info *revs,
last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
mode == REPLAY_MODE_REVERT ? last_commit : onto,
- &merge_opt, &result, mode, opts->empty);
+ &merge_opt, &result, mode, opts->empty,
+ opts->sign_commit);
if (!last_commit)
break;
diff --git a/replay.h b/replay.h
index 491db145e3..6ed0608911 100644
--- a/replay.h
+++ b/replay.h
@@ -57,6 +57,12 @@ struct replay_revisions_options {
*/
int contained;
+ /*
+ * Key used to sign newly-created commits. An empty string requests the
+ * default configured signing key, and NULL disables signing.
+ */
+ const char *sign_commit;
+
/*
* Controls what to do when a replayed commit becomes empty.
* Defaults to REPLAY_EMPTY_COMMIT_DROP.
--
2.55.0
^ permalink raw reply related
* [PATCH v2 0/2] history: support signing rewritten commits
From: Souma @ 2026-07-17 14:51 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Souma
In-Reply-To: <20260703145037.69832-1-git@5ouma.me>
The history commands create commits directly and via the replay
machinery, but currently have no way to honor `commit.gpgSign` or an
explicit signing request. This means users who require signed commits
lose that property when rewriting history.
Teach the replay API to accept a signing key, then expose the standard
`-S`/`--gpg-sign[=<key-id>]` and `--no-gpg-sign` interface across the
`history drop`, `history fixup`, `history reword`, and `history split`
subcommands. The selected policy applies to every new commit, including
both halves of a split and replayed descendants. A drop of the tip
creates no replacement commit and therefore has nothing to sign.
The implementation follows the precedence used by rebase, cherry-pick,
and revert: `commit.gpgSign` supplies the default, command-line options
override it, and the last command-line option wins.
The signature records the attestation of the current committer to the
rewritten commit while retaining the original author identity; it does
not claim authorship of commits written by somebody else.
Changes since v1:
- Split the replay signing plumbing into a preparatory patch
- Fold the documentation and tests into the feature patch so each
commit builds and passes t0450
- Move `sign_commit` before the output parameter of `commit_tree_ext()` and
update its callers accordingly
- Pass `NULL` to `git_default_config()`
- Document why configuration is read before command-line options
- Clarify that every rewritten commit is signed, including commits with
a different author
- Add signing support and tests for the new `history drop` subcommand
- Add coverage for selecting an explicit signing key
Souma (2):
replay: allow callers to sign commits
builtin/history: sign rewritten commits
Documentation/git-history.adoc | 16 +++++--
builtin/history.c | 84 ++++++++++++++++++++++++++--------
replay.c | 13 ++++--
replay.h | 6 +++
t/t3451-history-reword.sh | 63 +++++++++++++++++++++++++
t/t3452-history-split.sh | 44 ++++++++++++++++++
t/t3453-history-fixup.sh | 39 ++++++++++++++++
t/t3454-history-drop.sh | 50 ++++++++++++++++++++
8 files changed, 286 insertions(+), 29 deletions(-)
Range-diff against v1:
1: 60f7c13514 ! 1: 3f4dc0b982 builtin/history: sign rewritten commits
@@ Metadata
Author: Souma <git@5ouma.me>
## Commit message ##
- builtin/history: sign rewritten commits
+ replay: allow callers to sign commits
- The history commands create replacement commits directly instead of
- using the sequencer or the commit porcelain. As a result, rewritten
- commits ignore commit.gpgsign and cannot be signed on demand.
+ The replay machinery creates commits directly through
+ `commit_tree_extended()`, but callers cannot currently request
+ signatures. Commands that replay rewritten history consequently cannot
+ carry their signing policy through to descendant commits.
- Read the usual signing configuration before parsing history options.
- Add the commit-style -S/--gpg-sign knob, and pass the selected
- signing key through direct rewrites and replayed descendants.
+ Add `sign_commit` to `replay_revisions_options` and thread it through
+ commit creation. `NULL` preserves the existing unsigned behavior, an
+ empty string selects the default signing key, and a non-empty string
+ selects an explicit key. Existing callers zero-initialize the options
+ structure, so their behavior is unchanged.
Signed-off-by: Souma <git@5ouma.me>
- ## builtin/history.c ##
-@@
- #include "wt-status.h"
-
- #define GIT_HISTORY_FIXUP_USAGE \
-- N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]")
-+ N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]")
- #define GIT_HISTORY_REWORD_USAGE \
-- N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
-+ N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]]")
- #define GIT_HISTORY_SPLIT_USAGE \
-- N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
-+ N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]] [--] [<pathspec>...]")
-
- static void change_data_free(void *util, const char *str UNUSED)
- {
-@@ builtin/history.c: enum commit_tree_flags {
- COMMIT_TREE_EDIT_MESSAGE = (1 << 0),
- };
-
-+static int history_config(const char *var, const char *value,
-+ const struct config_context *ctx, void *data)
-+{
-+ const char **sign_commit = data;
-+
-+ if (!strcmp(var, "commit.gpgsign")) {
-+ *sign_commit = git_config_bool(var, value) ? "" : NULL;
-+ return 0;
-+ }
-+
-+ return git_default_config(var, value, ctx, data);
-+}
-+
-+#define OPT_HISTORY_GPG_SIGN(v) { \
-+ .type = OPTION_STRING, \
-+ .short_name = 'S', \
-+ .long_name = "gpg-sign", \
-+ .value = (v), \
-+ .argh = N_("key-id"), \
-+ .help = N_("GPG-sign rewritten commits"), \
-+ .flags = PARSE_OPT_OPTARG, \
-+ .defval = (intptr_t) "", \
-+}
-+
- static int commit_tree_ext(struct repository *repo,
- const char *action,
- struct commit *commit_with_message,
-@@ builtin/history.c: static int commit_tree_ext(struct repository *repo,
- const struct object_id *old_tree,
- const struct object_id *new_tree,
- struct commit **out,
-+ const char *sign_commit,
- enum commit_tree_flags flags)
- {
- const char *exclude_gpgsig[] = {
-@@ builtin/history.c: static int commit_tree_ext(struct repository *repo,
-
- ret = commit_tree_extended(commit_message.buf, commit_message.len, new_tree,
- parents, &rewritten_commit_oid, original_author,
-- NULL, NULL, original_extra_headers);
-+ NULL, sign_commit, original_extra_headers);
- if (ret < 0)
- goto out;
-
-@@ builtin/history.c: static int commit_tree_ext(struct repository *repo,
- static int commit_tree_with_edited_message(struct repository *repo,
- const char *action,
- struct commit *original,
-- struct commit **out)
-+ struct commit **out,
-+ const char *sign_commit)
- {
- struct object_id parent_tree_oid;
- const struct object_id *tree_oid;
-@@ builtin/history.c: static int commit_tree_with_edited_message(struct repository *repo,
- }
-
- return commit_tree_ext(repo, action, original, original->parents,
-- &parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
-+ &parent_tree_oid, tree_oid, out, sign_commit,
-+ COMMIT_TREE_EDIT_MESSAGE);
- }
-
- enum ref_action {
-@@ builtin/history.c: static int handle_reference_updates(struct rev_info *revs,
- struct commit *rewritten,
- const char *reflog_msg,
- int dry_run,
-+ const char *sign_commit,
- enum replay_empty_commit_action empty)
- {
- const struct name_decoration *decoration;
- struct replay_revisions_options opts = {
- .empty = empty,
-+ .sign_commit = sign_commit,
- };
- struct replay_result result = { 0 };
- struct ref_transaction *transaction = NULL;
-@@ builtin/history.c: static int cmd_history_fixup(int argc,
- enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
- enum ref_action action = REF_ACTION_DEFAULT;
- enum commit_tree_flags flags = 0;
-+ const char *sign_commit = NULL;
- int dry_run = 0;
- struct option options[] = {
- OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
-@@ builtin/history.c: static int cmd_history_fixup(int argc,
- OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
- N_("how to handle commits that become empty"),
- PARSE_OPT_NONEG, parse_opt_empty),
-+ OPT_HISTORY_GPG_SIGN(&sign_commit),
- OPT_END(),
- };
- struct merge_result merge_result = { 0 };
-@@ builtin/history.c: static int cmd_history_fixup(int argc,
- bool skip_commit = false;
- int ret;
-
-+ repo_config(repo, history_config, &sign_commit);
-+
- argc = parse_options(argc, argv, prefix, options, usage, 0);
- if (argc != 1) {
- ret = error(_("command expects a single revision"));
- goto out;
- }
-- repo_config(repo, git_default_config, NULL);
-
- if (action == REF_ACTION_DEFAULT)
- action = REF_ACTION_BRANCHES;
-@@ builtin/history.c: static int cmd_history_fixup(int argc,
- if (!skip_commit) {
- ret = commit_tree_ext(repo, "fixup", original, original->parents,
- &original_tree->object.oid, &merge_result.tree->object.oid,
-- &rewritten, flags);
-+ &rewritten, sign_commit, flags);
- if (ret < 0) {
- ret = error(_("failed writing fixed-up commit"));
- goto out;
-@@ builtin/history.c: static int cmd_history_fixup(int argc,
- strbuf_addf(&reflog_msg, "fixup: updating %s", argv[0]);
-
- ret = handle_reference_updates(&revs, action, original, rewritten,
-- reflog_msg.buf, dry_run, empty);
-+ reflog_msg.buf, dry_run, sign_commit, empty);
- if (ret < 0) {
- ret = error(_("failed replaying descendants"));
- goto out;
-@@ builtin/history.c: static int cmd_history_reword(int argc,
- NULL,
- };
- enum ref_action action = REF_ACTION_DEFAULT;
-+ const char *sign_commit = NULL;
- int dry_run = 0;
- struct option options[] = {
- OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
-@@ builtin/history.c: static int cmd_history_reword(int argc,
- PARSE_OPT_NONEG, parse_ref_action),
- OPT_BOOL('n', "dry-run", &dry_run,
- N_("perform a dry-run without updating any refs")),
-+ OPT_HISTORY_GPG_SIGN(&sign_commit),
- OPT_END(),
- };
- struct strbuf reflog_msg = STRBUF_INIT;
-@@ builtin/history.c: static int cmd_history_reword(int argc,
- struct rev_info revs = { 0 };
- int ret;
-
-+ repo_config(repo, history_config, &sign_commit);
-+
- argc = parse_options(argc, argv, prefix, options, usage, 0);
- if (argc != 1) {
- ret = error(_("command expects a single revision"));
- goto out;
- }
-- repo_config(repo, git_default_config, NULL);
-
- if (action == REF_ACTION_DEFAULT)
- action = REF_ACTION_BRANCHES;
-@@ builtin/history.c: static int cmd_history_reword(int argc,
- if (ret)
- goto out;
-
-- ret = commit_tree_with_edited_message(repo, "reworded", original, &rewritten);
-+ ret = commit_tree_with_edited_message(repo, "reworded", original,
-+ &rewritten, sign_commit);
- if (ret < 0) {
- ret = error(_("failed writing reworded commit"));
- goto out;
-@@ builtin/history.c: static int cmd_history_reword(int argc,
- strbuf_addf(&reflog_msg, "reword: updating %s", argv[0]);
-
- ret = handle_reference_updates(&revs, action, original, rewritten,
-- reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
-+ reflog_msg.buf, dry_run, sign_commit,
-+ REPLAY_EMPTY_COMMIT_ABORT);
- if (ret < 0) {
- ret = error(_("failed replaying descendants"));
- goto out;
-@@ builtin/history.c: static int write_ondisk_index(struct repository *repo,
- static int split_commit(struct repository *repo,
- struct commit *original,
- struct pathspec *pathspec,
-- struct commit **out)
-+ struct commit **out,
-+ const char *sign_commit)
- {
- struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT;
- struct strbuf index_file = STRBUF_INIT;
-@@ builtin/history.c: static int split_commit(struct repository *repo,
- * that shall be diffed against is the parent of the original commit.
- */
- ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
-- &split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
-+ &split_tree->object.oid, &first_commit, sign_commit,
-+ COMMIT_TREE_EDIT_MESSAGE);
- if (ret < 0) {
- ret = error(_("failed writing first commit"));
- goto out;
-@@ builtin/history.c: static int split_commit(struct repository *repo,
- new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
-
- ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
-- new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
-+ new_tree_oid, &second_commit, sign_commit,
-+ COMMIT_TREE_EDIT_MESSAGE);
- if (ret < 0) {
- ret = error(_("failed writing second commit"));
- goto out;
-@@ builtin/history.c: static int cmd_history_split(int argc,
- NULL,
- };
- enum ref_action action = REF_ACTION_DEFAULT;
-+ const char *sign_commit = NULL;
- int dry_run = 0;
- struct option options[] = {
- OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
-@@ builtin/history.c: static int cmd_history_split(int argc,
- PARSE_OPT_NONEG, parse_ref_action),
- OPT_BOOL('n', "dry-run", &dry_run,
- N_("perform a dry-run without updating any refs")),
-+ OPT_HISTORY_GPG_SIGN(&sign_commit),
- OPT_END(),
- };
- struct commit *original, *rewritten = NULL;
-@@ builtin/history.c: static int cmd_history_split(int argc,
- struct rev_info revs = { 0 };
- int ret;
-
-+ repo_config(repo, history_config, &sign_commit);
-+
- argc = parse_options(argc, argv, prefix, options, usage, 0);
- if (argc < 1) {
- ret = error(_("command expects a committish"));
- goto out;
- }
-- repo_config(repo, git_default_config, NULL);
-
- if (action == REF_ACTION_DEFAULT)
- action = REF_ACTION_BRANCHES;
-@@ builtin/history.c: static int cmd_history_split(int argc,
- goto out;
- }
-
-- ret = split_commit(repo, original, &pathspec, &rewritten);
-+ ret = split_commit(repo, original, &pathspec, &rewritten, sign_commit);
- if (ret < 0)
- goto out;
-
- strbuf_addf(&reflog_msg, "split: updating %s", argv[0]);
-
- ret = handle_reference_updates(&revs, action, original, rewritten,
-- reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
-+ reflog_msg.buf, dry_run, sign_commit,
-+ REPLAY_EMPTY_COMMIT_ABORT);
- if (ret < 0) {
- ret = error(_("failed replaying descendants"));
- goto out;
-
## replay.c ##
@@ replay.c: static struct commit *create_commit(struct repository *repo,
struct tree *tree,
2: 9935928b01 < -: ---------- doc: document history signing options
3: c017e90034 ! 2: 0e63c0b66a t345x: cover signed history rewrites
@@ Metadata
Author: Souma <git@5ouma.me>
## Commit message ##
- t345x: cover signed history rewrites
+ builtin/history: sign rewritten commits
- History signing needs regression coverage because these commands bypass the
- usual commit machinery and create replacement commits through lower-level
- APIs.
+ The history commands create replacement commits directly instead of
+ using the sequencer or the commit porcelain. As a result, rewritten
+ commits ignore `commit.gpgSign` and cannot be signed on demand.
- Add GPG-gated tests for config-driven signing, command-line signing,
- --no-gpg-sign precedence, and signing of replayed descendants after fixup,
- reword, and split.
+ Read the signing configuration before parsing options so that it
+ establishes the default and later `-S`/`--gpg-sign` or `--no-gpg-sign`
+ options override it. Pass the selected key through direct rewrites and
+ the replay machinery.
+
+ Sign every newly created commit, including both halves of a split and
+ replayed descendants. Dropping the tip creates no replacement commit,
+ so there is nothing to sign. As with `rebase --gpg-sign`, the signature
+ records the attestation of the current committer to the rewritten
+ commit while retaining the original author identity; it does not claim
+ authorship of commits written by somebody else.
+
+ Document the behavior and add GPG-gated coverage for configuration,
+ command-line overrides, last-option-wins precedence, replayed
+ descendants, split commits, an explicit signing key, and the
+ no-new-commit drop case.
Signed-off-by: Souma <git@5ouma.me>
+ ## Documentation/git-history.adoc ##
+@@ Documentation/git-history.adoc: git-history - EXPERIMENTAL: Rewrite history
+ SYNOPSIS
+ --------
+ [synopsis]
+-git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]
+-git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
+-git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
+-git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
++git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]
++git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]
++git history reword <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]]
++git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]] [--] [<pathspec>...]
+
+ DESCRIPTION
+ -----------
+@@ Documentation/git-history.adoc: OPTIONS
+ `--reedit-message`::
+ Open an editor to modify the target commit's message.
+
++`-S[<key-id>]`::
++`--gpg-sign[=<key-id>]`::
++`--no-gpg-sign`::
++ GPG-sign rewritten commits. The _<key-id>_ argument is optional and
++ defaults to the committer identity; if specified, it must be stuck to
++ the option without a space. `--no-gpg-sign` is useful to countermand
++ both `commit.gpgSign` configuration and earlier `--gpg-sign`.
++
+ `--empty=(drop|keep|abort)`::
+ Control what happens when a commit becomes empty as a result of the
+ fixup. This can happen in two situations:
+
+ ## builtin/history.c ##
+@@
+ #include "wt-status.h"
+
+ #define GIT_HISTORY_DROP_USAGE \
+- N_("git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)]")
++ N_("git history drop <commit> [--dry-run] [--update-refs=(branches|head)] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]")
+ #define GIT_HISTORY_FIXUP_USAGE \
+- N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]")
++ N_("git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)] [--[no-]gpg-sign[=<key-id>]]")
+ #define GIT_HISTORY_REWORD_USAGE \
+- N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
++ N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]]")
+ #define GIT_HISTORY_SPLIT_USAGE \
+- N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
++ N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--[no-]gpg-sign[=<key-id>]] [--] [<pathspec>...]")
+
+ static void change_data_free(void *util, const char *str UNUSED)
+ {
+@@ builtin/history.c: enum commit_tree_flags {
+ COMMIT_TREE_EDIT_MESSAGE = (1 << 0),
+ };
+
++static int history_config(const char *var, const char *value,
++ const struct config_context *ctx, void *data)
++{
++ const char **sign_commit = data;
++
++ if (!strcmp(var, "commit.gpgsign")) {
++ *sign_commit = git_config_bool(var, value) ? "" : NULL;
++ return 0;
++ }
++
++ return git_default_config(var, value, ctx, NULL);
++}
++
++#define OPT_HISTORY_GPG_SIGN(v) { \
++ .type = OPTION_STRING, \
++ .short_name = 'S', \
++ .long_name = "gpg-sign", \
++ .value = (v), \
++ .argh = N_("key-id"), \
++ .help = N_("GPG-sign rewritten commits"), \
++ .flags = PARSE_OPT_OPTARG, \
++ .defval = (intptr_t)"", \
++}
++
+ static int commit_tree_ext(struct repository *repo,
+ const char *action,
+ struct commit *commit_with_message,
+ const struct commit_list *parents,
+ const struct object_id *old_tree,
+ const struct object_id *new_tree,
++ const char *sign_commit,
+ struct commit **out,
+ enum commit_tree_flags flags)
+ {
+@@ builtin/history.c: static int commit_tree_ext(struct repository *repo,
+
+ ret = commit_tree_extended(commit_message.buf, commit_message.len, new_tree,
+ parents, &rewritten_commit_oid, original_author,
+- NULL, NULL, original_extra_headers);
++ NULL, sign_commit, original_extra_headers);
+ if (ret < 0)
+ goto out;
+
+@@ builtin/history.c: static int commit_tree_ext(struct repository *repo,
+ static int commit_tree_with_edited_message(struct repository *repo,
+ const char *action,
+ struct commit *original,
++ const char *sign_commit,
+ struct commit **out)
+ {
+ struct object_id parent_tree_oid;
+@@ builtin/history.c: static int commit_tree_with_edited_message(struct repository *repo,
+ }
+
+ return commit_tree_ext(repo, action, original, original->parents,
+- &parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
++ &parent_tree_oid, tree_oid, sign_commit, out,
++ COMMIT_TREE_EDIT_MESSAGE);
+ }
+
+ enum ref_action {
+@@ builtin/history.c: static int compute_pending_ref_updates(struct rev_info *revs,
+ enum ref_action action,
+ struct commit *original,
+ struct commit *rewritten,
++ const char *sign_commit,
+ enum replay_empty_commit_action empty,
+ struct replay_result *result)
+ {
+ const struct name_decoration *decoration;
+ struct replay_revisions_options opts = {
+ .empty = empty,
++ .sign_commit = sign_commit,
+ };
+ char hex[GIT_MAX_HEXSZ + 1];
+ bool detached_head;
+@@ builtin/history.c: static int handle_reference_updates(struct rev_info *revs,
+ struct commit *rewritten,
+ const char *reflog_msg,
+ int dry_run,
++ const char *sign_commit,
+ enum replay_empty_commit_action empty)
+ {
+ struct replay_result result = { 0 };
+ int ret;
+
+ ret = compute_pending_ref_updates(revs, action, original, rewritten,
+- empty, &result);
++ sign_commit, empty, &result);
+ if (ret)
+ goto out;
+
+@@ builtin/history.c: static int cmd_history_fixup(int argc,
+ enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
+ enum ref_action action = REF_ACTION_DEFAULT;
+ enum commit_tree_flags flags = 0;
++ const char *sign_commit = NULL;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+@@ builtin/history.c: static int cmd_history_fixup(int argc,
+ OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
+ N_("how to handle commits that become empty"),
+ PARSE_OPT_NONEG, parse_opt_empty),
++ OPT_HISTORY_GPG_SIGN(&sign_commit),
+ OPT_END(),
+ };
+ struct merge_result merge_result = { 0 };
+@@ builtin/history.c: static int cmd_history_fixup(int argc,
+ bool skip_commit = false;
+ int ret;
+
++ repo_config(repo, history_config, &sign_commit);
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single revision"));
+ goto out;
+ }
+- repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+@@ builtin/history.c: static int cmd_history_fixup(int argc,
+ if (!skip_commit) {
+ ret = commit_tree_ext(repo, "fixup", original, original->parents,
+ &original_tree->object.oid, &merge_result.tree->object.oid,
+- &rewritten, flags);
++ sign_commit, &rewritten, flags);
+ if (ret < 0) {
+ ret = error(_("failed writing fixed-up commit"));
+ goto out;
+@@ builtin/history.c: static int cmd_history_fixup(int argc,
+ strbuf_addf(&reflog_msg, "fixup: updating %s", argv[0]);
+
+ ret = handle_reference_updates(&revs, action, original, rewritten,
+- reflog_msg.buf, dry_run, empty);
++ reflog_msg.buf, dry_run, sign_commit, empty);
+ if (ret < 0) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+@@ builtin/history.c: static int cmd_history_reword(int argc,
+ NULL,
+ };
+ enum ref_action action = REF_ACTION_DEFAULT;
++ const char *sign_commit = NULL;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+@@ builtin/history.c: static int cmd_history_reword(int argc,
+ PARSE_OPT_NONEG, parse_ref_action),
+ OPT_BOOL('n', "dry-run", &dry_run,
+ N_("perform a dry-run without updating any refs")),
++ OPT_HISTORY_GPG_SIGN(&sign_commit),
+ OPT_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
+@@ builtin/history.c: static int cmd_history_reword(int argc,
+ struct rev_info revs = { 0 };
+ int ret;
+
++ repo_config(repo, history_config, &sign_commit);
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single revision"));
+ goto out;
+ }
+- repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+@@ builtin/history.c: static int cmd_history_reword(int argc,
+ if (ret)
+ goto out;
+
+- ret = commit_tree_with_edited_message(repo, "reworded", original, &rewritten);
++ ret = commit_tree_with_edited_message(repo, "reworded", original,
++ sign_commit, &rewritten);
+ if (ret < 0) {
+ ret = error(_("failed writing reworded commit"));
+ goto out;
+@@ builtin/history.c: static int cmd_history_reword(int argc,
+ strbuf_addf(&reflog_msg, "reword: updating %s", argv[0]);
+
+ ret = handle_reference_updates(&revs, action, original, rewritten,
+- reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
++ reflog_msg.buf, dry_run, sign_commit,
++ REPLAY_EMPTY_COMMIT_ABORT);
+ if (ret < 0) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+@@ builtin/history.c: static int write_ondisk_index(struct repository *repo,
+ static int split_commit(struct repository *repo,
+ struct commit *original,
+ struct pathspec *pathspec,
++ const char *sign_commit,
+ struct commit **out)
+ {
+ struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT;
+@@ builtin/history.c: static int split_commit(struct repository *repo,
+ * that shall be diffed against is the parent of the original commit.
+ */
+ ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
+- &split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
++ &split_tree->object.oid, sign_commit, &first_commit,
++ COMMIT_TREE_EDIT_MESSAGE);
+ if (ret < 0) {
+ ret = error(_("failed writing first commit"));
+ goto out;
+@@ builtin/history.c: static int split_commit(struct repository *repo,
+ new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
+
+ ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
+- new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
++ new_tree_oid, sign_commit, &second_commit,
++ COMMIT_TREE_EDIT_MESSAGE);
+ if (ret < 0) {
+ ret = error(_("failed writing second commit"));
+ goto out;
+@@ builtin/history.c: static int cmd_history_split(int argc,
+ NULL,
+ };
+ enum ref_action action = REF_ACTION_DEFAULT;
++ const char *sign_commit = NULL;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+@@ builtin/history.c: static int cmd_history_split(int argc,
+ PARSE_OPT_NONEG, parse_ref_action),
+ OPT_BOOL('n', "dry-run", &dry_run,
+ N_("perform a dry-run without updating any refs")),
++ OPT_HISTORY_GPG_SIGN(&sign_commit),
+ OPT_END(),
+ };
+ struct commit *original, *rewritten = NULL;
+@@ builtin/history.c: static int cmd_history_split(int argc,
+ struct rev_info revs = { 0 };
+ int ret;
+
++ repo_config(repo, history_config, &sign_commit);
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc < 1) {
+ ret = error(_("command expects a committish"));
+ goto out;
+ }
+- repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+@@ builtin/history.c: static int cmd_history_split(int argc,
+ goto out;
+ }
+
+- ret = split_commit(repo, original, &pathspec, &rewritten);
++ ret = split_commit(repo, original, &pathspec, sign_commit, &rewritten);
+ if (ret < 0)
+ goto out;
+
+ strbuf_addf(&reflog_msg, "split: updating %s", argv[0]);
+
+ ret = handle_reference_updates(&revs, action, original, rewritten,
+- reflog_msg.buf, dry_run, REPLAY_EMPTY_COMMIT_ABORT);
++ reflog_msg.buf, dry_run, sign_commit,
++ REPLAY_EMPTY_COMMIT_ABORT);
+ if (ret < 0) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+@@ builtin/history.c: static int cmd_history_drop(int argc,
+ };
+ enum replay_empty_commit_action empty = REPLAY_EMPTY_COMMIT_DROP;
+ enum ref_action action = REF_ACTION_DEFAULT;
++ const char *sign_commit = NULL;
+ int dry_run = 0;
+ struct option options[] = {
+ OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+@@ builtin/history.c: static int cmd_history_drop(int argc,
+ OPT_CALLBACK_F(0, "empty", &empty, "(drop|keep|abort)",
+ N_("how to handle descendants that become empty"),
+ PARSE_OPT_NONEG, parse_opt_empty),
++ OPT_HISTORY_GPG_SIGN(&sign_commit),
+ OPT_END(),
+ };
+ struct strbuf reflog_msg = STRBUF_INIT;
+@@ builtin/history.c: static int cmd_history_drop(int argc,
+ bool head_moves = false;
+ int ret;
+
++ repo_config(repo, history_config, &sign_commit);
+ argc = parse_options(argc, argv, prefix, options, usage, 0);
+ if (argc != 1) {
+ ret = error(_("command expects a single revision"));
+ goto out;
+ }
+- repo_config(repo, git_default_config, NULL);
+
+ if (action == REF_ACTION_DEFAULT)
+ action = REF_ACTION_BRANCHES;
+@@ builtin/history.c: static int cmd_history_drop(int argc,
+ rewritten = original->parents->item;
+
+ ret = compute_pending_ref_updates(&revs, action, original, rewritten,
+- empty, &result);
++ sign_commit, empty, &result);
+ if (ret) {
+ ret = error(_("failed replaying descendants"));
+ goto out;
+
## t/t3451-history-reword.sh ##
@@ t/t3451-history-reword.sh: test_description='tests for git-history reword subcommand'
@@ t/t3451-history-reword.sh: test_expect_success 'can reword commit in the middle'
+test_reword_gpg_sign ! true --no-gpg-sign
+test_reword_gpg_sign ! true --gpg-sign --no-gpg-sign
+test_reword_gpg_sign false --no-gpg-sign --gpg-sign
++
++test_expect_success GPG 'reword uses an explicit signing key for rewritten history' '
++ test_when_finished "rm -rf repo" &&
++ git init repo &&
++ (
++ cd repo &&
++ test_commit first &&
++ test_commit second &&
++ test_commit third &&
++
++ reword_with_message -SB7227189 HEAD~ <<-EOF &&
++ second reworded
++ EOF
++
++ git verify-commit HEAD~ &&
++ git verify-commit HEAD &&
++ git log -2 --format=%GK >actual &&
++ cat >expect <<-\EOF &&
++ 65A0EEA02E30CAD7
++ 65A0EEA02E30CAD7
++ EOF
++ test_cmp expect actual
++ )
++'
+
test_expect_success 'can reword commit in the middle even on detached head' '
test_when_finished "rm -rf repo" &&
@@ t/t3453-history-fixup.sh: test_expect_success 'preserves commit message and auth
test_expect_success 'updates all descendant branches by default' '
test_when_finished "rm -rf repo" &&
git init repo --initial-branch=main &&
+
+ ## t/t3454-history-drop.sh ##
+@@ t/t3454-history-drop.sh: test_description='tests for git-history drop subcommand'
+
+ . ./test-lib.sh
+ . "$TEST_DIRECTORY/lib-log-graph.sh"
++. "$TEST_DIRECTORY/lib-gpg.sh"
+
+ expect_graph () {
+ cat >expect &&
+@@ t/t3454-history-drop.sh: expect_log () {
+ test_cmp expect actual
+ }
+
++test_drop_gpg_sign () {
++ must_fail= will=will
++ if test "x$1" = "x!"
++ then
++ must_fail=test_must_fail
++ will="will not"
++ shift
++ fi
++ conf=$1
++ shift
++
++ test_expect_success GPG "drop $* with commit.gpgsign=$conf $will sign replayed descendants" "
++ test_when_finished 'rm -rf repo' &&
++ git init repo &&
++ (
++ cd repo &&
++ test_commit first &&
++ test_commit second &&
++ test_commit third &&
++
++ git config commit.gpgsign $conf &&
++ git history drop $* HEAD~ &&
++
++ $must_fail git verify-commit HEAD
++ )
++ "
++}
++
+ test_expect_success 'errors on missing commit argument' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+@@ t/t3454-history-drop.sh: test_expect_success 'drops a commit in the middle and replays descendants' '
+ )
+ '
+
++test_drop_gpg_sign ! false
++test_drop_gpg_sign true
++test_drop_gpg_sign false --gpg-sign
++test_drop_gpg_sign ! true --no-gpg-sign
++test_drop_gpg_sign ! true --gpg-sign --no-gpg-sign
++test_drop_gpg_sign false --no-gpg-sign --gpg-sign
++
++test_expect_success GPG 'drop has no commit to sign when dropping the tip' '
++ test_when_finished "rm -rf repo" &&
++ git init repo &&
++ (
++ cd repo &&
++ test_commit first &&
++ test_commit second &&
++
++ git history drop --gpg-sign HEAD &&
++
++ test_must_fail git verify-commit HEAD
++ )
++'
++
+ test_expect_success 'drops the HEAD commit' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
--
2.55.0
^ permalink raw reply
* [PATCH v2] wt-status: avoid repeated insertion for untracked paths
From: Sahitya Chandra @ 2026-07-17 14:46 UTC (permalink / raw)
To: git; +Cc: gitster, avarab, stolee, peff, ps, Sahitya Chandra
In-Reply-To: <20260716185045.229320-1-sahityajb@gmail.com>
wt_status_collect_untracked() copies entries from dir.entries and
dir.ignored into string_lists using string_list_insert(). That keeps the
destination lists sorted and deduplicated, but makes the code harder to
reason about because it rebuilds sorted lists through repeated sorted
insertion.
Collect the entries with string_list_append() instead, then sort and
deduplicate each list once with string_list_sort_u(). This preserves the
sorted, duplicate-free result while making the collection strategy explicit.
Signed-off-by: Sahitya Chandra <sahityajb@gmail.com>
---
Changes since v1:
- Use string_list_sort_u() instead of open-coding sort plus deduplication.
- Reword the subject and commit message to avoid overclaiming an O(n^2)
cost when the input from fill_directory() is already sorted.
wt-status.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 58461e02f8..57772c7501 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -832,14 +832,16 @@ static void wt_status_collect_untracked(struct wt_status *s)
for (i = 0; i < dir.nr; i++) {
struct dir_entry *ent = dir.entries[i];
if (index_name_is_other(istate, ent->name, ent->len))
- string_list_insert(&s->untracked, ent->name);
+ string_list_append(&s->untracked, ent->name);
}
+ string_list_sort_u(&s->untracked, 0);
for (i = 0; i < dir.ignored_nr; i++) {
struct dir_entry *ent = dir.ignored[i];
if (index_name_is_other(istate, ent->name, ent->len))
- string_list_insert(&s->ignored, ent->name);
+ string_list_append(&s->ignored, ent->name);
}
+ string_list_sort_u(&s->ignored, 0);
dir_clear(&dir);
base-commit: 44de1520f08d1dfebc3ab2d9f644208eaa5ac925
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] wt-status: avoid quadratic insertion for untracked paths
From: Sahitya Chandra @ 2026-07-17 14:40 UTC (permalink / raw)
To: Jeff King; +Cc: Patrick Steinhardt, git, gitster, avarab, stolee
In-Reply-To: <20260717075449.GA1832790@coredump.intra.peff.net>
On Fri, Jul 17, 2026 at 1:24 PM Jeff King <peff@peff.net> wrote:
> Yeah, I had the same question, and tried for a moment to produce an
> example before realizing that it probably is theoretical. If we are
> feeding the entries in pre-sorted order then the insert is always O(1).
>
> I think it's still worth doing this, though, as it makes the result much
> more obvious to analyze. I think it could even be O(n) if the sort
> implementation is optimized under the hood for pre-sorted inputs.
Thanks, that makes sense. I updated v2 to avoid claiming this is a
current O(n^2) problem and instead frame it as making the append, sort,
and deduplicate steps explicit.
I also switched to string_list_sort_u() as Patrick suggested.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox