* [PATCH] Makefile: fix up lib directory move
From: Ramsay Jones @ 2026-07-10 18:38 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: GIT Mailing-list, Junio C Hamano
Commit 9759608622 ("Move libgit.a sources into separate "lib/" directory",
2026-06-22) moved some files into a lib directory, but forgot to update
a sparse dependency in the Makefile, resulting in a sparse error:
SP lib/pack-revindex.c
lib/pack-revindex.c:78:17: error: memset with byte count of 262144
make: *** [Makefile:3446: lib/pack-revindex.sp] Error 1
Add the missing 'lib/' prefix to the pack-revindex.sp path.
Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---
Hi Patrick,
If you need to re-roll your 'ps/libgit-in-subdir' branch, could you please squash
this into the relevant patch. (This patch was created directly on top of the 'seen'
branch, rather than on top of your branch).
Thanks
ATB,
Ramsay Jones
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 703772ba4f..a36d2c1942 100644
--- a/Makefile
+++ b/Makefile
@@ -2974,7 +2974,7 @@ lib/gettext.sp lib/gettext.s lib/gettext.o: EXTRA_CPPFLAGS = \
http-push.sp lib/http.sp lib/http-walker.sp remote-curl.sp imap-send.sp: SP_EXTRA_FLAGS += \
-DCURL_DISABLE_TYPECHECK
-pack-revindex.sp: SP_EXTRA_FLAGS += -Wno-memcpy-max-count
+lib/pack-revindex.sp: SP_EXTRA_FLAGS += -Wno-memcpy-max-count
ifdef NO_EXPAT
lib/http-walker.sp lib/http-walker.s lib/http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT
--
2.55.0
^ permalink raw reply related
* cygwin v2.55.0 test failures
From: Ramsay Jones @ 2026-07-10 18:32 UTC (permalink / raw)
To: GIT Mailing-list
Cc: Johannes Schindelin, Patrick Steinhardt, Junio C Hamano,
Johannes Sixt, Adam Dinwoodie, Torsten Bögershausen
Sorry for being a bit tardy, but I was somewhat unwell and, as a result,
AFK for some weeks during the last part of the v2.55.0 cycle. So, I'm
still catching up. As part of that, I found that the cygwin test-suite
fails for v2.55.0, like so:
$ tail -13 test-out-2-55-rel
Test Summary Report
-------------------
unit-tests/bin/unit-tests.exe (Wstat: 256 (exited 1) Tests: 249 Failed: 1)
Failed test: 242
Non-zero exit status: 1
t9904-url-parse.sh (Wstat: 256 (exited 1) Tests: 53 Failed: 4)
Failed tests: 39, 42-43, 47
Non-zero exit status: 1
Files=1047, Tests=32848, 4584 wallclock secs (29.05 usr 85.59 sys + 9679.08 cusr 14883.94 csys = 24677.67 CPU)
Result: FAIL
make[1]: *** [Makefile:78: prove] Error 1
make[1]: Leaving directory '/home/ramsay/git/t'
make: *** [Makefile:3381: test] Error 2
$
Unsurprisingly, the -rc0, -rc1 and -rc2 builds show the same failures.
After a quick squint, the reason for the failure looked familiar ... ;)
Indeed, I have been aware of the reason for this failure since v2.23.1
at the end of 2019. For those who don't instantly recognize it, this was
one of a series of security releases, which mainly affected GfW. From
the release Notes:
Git v2.23.1 Release Notes
=========================
This release merges up the fixes that appear in v2.14.6, v2.15.4,
v2.17.3, v2.20.2 and in v2.21.1, addressing the security issues
CVE-2019-1348, CVE-2019-1349, CVE-2019-1350, CVE-2019-1351,
CVE-2019-1352, CVE-2019-1353, CVE-2019-1354, CVE-2019-1387, and
CVE-2019-19604; see the release notes for those versions for details.
For example, the test-suite for v2.25.0-rc0 looks like:
$ tail -17 test-out/test-out-2-25-rc0
Test Summary Report
-------------------
t5500-fetch-pack.sh (Wstat: 256 Tests: 369 Failed: 12)
Failed tests: 142, 145, 161-162, 239, 242, 258-259, 336
339, 355-356
Non-zero exit status: 1
t5580-clone-push-unc.sh (Wstat: 256 Tests: 6 Failed: 1)
Failed test: 4
Non-zero exit status: 1
t5601-clone.sh (Wstat: 256 Tests: 104 Failed: 4)
Failed tests: 62-64, 66
Non-zero exit status: 1
Files=890, Tests=21098, 16304 wallclock secs ( 0.48 usr 0.73 sys + 2374.33 cusr 7438.69 csys = 9814.24 CPU)
Result: FAIL
make[1]: *** [Makefile:52: prove] Error 1
make[1]: Leaving directory '/home/ramsay/git/t'
make: *** [Makefile:2754: test] Error 2
$
[t5580-clone-push-unc.sh was later renamed to t5580-unc-paths.sh]
From about that time, I had hacked up a 'fix' for cygwin, but hadn't decided
how to proceed with an actual patch. :)
So, I cherry-picked the 'fix' onto 'master' (then on v2.55.0 release). I was
a little surprised it went without problem, given how much 'git-compat-util.h'
has changed, but it just required a 'slide' from line 203 back to line 153.
This patch is equivalent to the 'git-compat-util.h' only part of the patch
given below.
This fixed up the current test failures (unit-tests and t9904-url-parse.sh).
Note that Patrick wanted to have a clean test-suite run on cygwin, so in
commit 5f8af25ff9 ("t5500, t5601: skip tests which exercise paths with '[::1]'
on Cygwin", 2024-10-16), he suppressed the test failures in t5500 and t5601.
(that was about the time of the v2.48.0 release).
The changes to tests t5500 and t5601, in the patch given below, essentially
reverts Patrick's commit 5f8af25ff9. This fixes all of the tests in t5601 and
ten of the twelve failures in t5500. (I don't recall what happened to t5580,
the single failure - the push test - was fixed somewhere between v2.43.0 and
v2.44.0-rc0).
As luck would have it, I left a note to myself about the remaining two
failure cases. This leads to the remaining hunk, to connect.c, in the patch
below; ie. the removal of a conditional (which should only fire for GfW and
cygwin). The '#ifdef DUMMY/#endif' should probably be replaced with an
'#ifdef GIT_WINDOWS_NATIVE/#endif' so that GfW is not affected. (Having said
that, I suspect that even GfW should drop it ['somebody was smoking something
exotic'], but I have no way to test it, so ...).
With this final hunk, this patch results in a clean test-suite run. :)
So, what does this mean? Well, I should probably not leave it another five
years, or thereabouts, before actually sending a real patch (series) to fix
this up properly! ;)
But what is properly? The 'fix' works, but the problem was principally caused
by cygwin being a bit schizophrenic about the 'pathname utilities' and its
support for POSIX only paths, WIN32 only paths or both.
For example, f82a97eb91 ("mingw: handle `subst`-ed "DOS drives"", 2019-09-06)
notes in the commit message:
Note: `[::1]:repo` is a valid URL, but not a valid path on Windows.
As `[` is now considered a valid drive letter, we need to be very
careful to avoid misinterpreting such a string as valid local path in
`url_is_local_not_ssh()`. To do that, we use the just-introduced
function `is_valid_path()` (which will label the string as invalid file
name because of the colon characters).
This fixes CVE-2019-1351.
Note that part of the fix involves a cygwin specific 'is_valid_path()' which
would otherwise be defined, like Linux, as simply '1' in git-compat-util.h.
I was a bit surprised that an IPv6 address was not parsed at a higher level
and take priority over an (possible) '[' drive letter, rather than being a
side-effect of calling an 'is_valid_path()' helper function. ;)
The 'is_valid_path()' was also part of the security fixes to check for some
'illegal' win32 paths which included those ending in spaces or periods or
for names such as AUX, COM1, LPTn, NUL, PRN, CONIN$, etc,. I know that some
of these paths are valid in cygwin (eg. those ending in spaces or periods)
so the win32 version of that function cannot be used without change. (this
could lead to one part of git allowing you to use said paths and other
parts not)! Note that some of the CVEs may actually still apply to cygwin.
I didn't check.
Also, note that I replaced the win32 version of the 'offset_1st_component()'
function. I can't quite remember why I did that, but I think the fact that
the win32 version doesn't ensure that the 'path' consists of both the <server>
and <share> component before returning the offset. (ie make sure that the
'path' consists of "//<server>/<share>/" at the very least and return one,
for '/', otherwise).
Personally, I would be quite happy to rip out all win32 path handling and
only support POSIX paths (I have been using cygwin since about 1996 and
have only ever used win32 paths when testing git ... that is the whole
point of cygwin! :) ), but I already know that that is a no-go. (there is
always somebody that complains when you suggest it).
So, for now anyway, it seems that I need to tidy up the patch and move in
the opposite direction to e.g. commit 1cadad6f65 ("git clone <url>
C:\cygwin\home\USER\repo' is working (again)", 2018-12-15).
Part of the reason for vacillating on the correct way forward with this
patch, was because I have often thought that I should use the cygwin API
to cater to both POSIX and win32 paths. For example, we could possibly use
the 'cygwin_conv_path()' function to do the path conversion (somewhat
similar to the macos pre-composed-utf8 stuff, minus the directory reading).
However, I think that would open a different can of worms, including some
potential memory leaks. So, not exactly a slam dunk.
[I also had a note-to-self about 'mixed / and \ urls' in the config file
which is exposed by these same tests. So, another patch may be needed?]
Anyway, something to think about. Hmm, I suspect it would be best to just
tidy up this patch first. ;)
Just FYI. Thanks!
ATB,
Ramsay Jones
----- >8 -----
Subject: [PATCH] cygwin: fix up IPv6 scp urls
---
connect.c | 2 ++
git-compat-util.h | 39 +++++++++++++++++++++++++++++++++++++++
t/t5500-fetch-pack.sh | 14 ++++----------
t/t5601-clone.sh | 11 ++---------
4 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/connect.c b/connect.c
index 47e39d2a73..6f5715e938 100644
--- a/connect.c
+++ b/connect.c
@@ -1088,10 +1088,12 @@ static enum url_scheme parse_connect_url(const char *url_orig, char **ret_host,
if (scheme == URL_SCHEME_LOCAL)
path = end;
+#ifdef DUMMY
else if (scheme == URL_SCHEME_FILE && *host != '/' &&
!has_dos_drive_prefix(host) &&
offset_1st_component(host - 2) > 1)
path = host - 2; /* include the leading "//" */
+#endif
else if (scheme == URL_SCHEME_FILE && has_dos_drive_prefix(end))
path = end; /* "file://$(pwd)" may be "file://C:/projects/repo" */
else
diff --git a/git-compat-util.h b/git-compat-util.h
index 8809776407..645ff96048 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -153,6 +153,45 @@ static inline int is_xplatform_dir_sep(int c)
#if defined(__CYGWIN__)
#include "compat/win32/path-utils.h"
+
+static inline int is_valid_cygwin_path(const char *path)
+{
+ if (path && strlen(path) > 2 &&
+ path[0] == '[' && path[1] == ':' &&
+ strchr(&path[2], ':'))
+ return 0;
+ return 1;
+}
+
+#define is_valid_path(path) is_valid_cygwin_path(path)
+
+static inline int cygwin_offset_1st_component(const char *path)
+{
+ int ret = 0;
+
+ /* C: prefix */
+ if ((ret = has_dos_drive_prefix(path)))
+ return ret;
+
+ /* //server/share/ prefix */
+ if (is_dir_sep(path[0]) && is_dir_sep(path[1])) {
+ char *pos = strpbrk(path + 2, "\\/");
+ if (pos && *(pos + 1) && !is_dir_sep(*(pos + 1))) {
+ pos = strpbrk(pos + 1, "\\/");
+ if (pos && *(pos + 1))
+ return pos + 1 - path;
+ }
+ }
+
+ /* / prefix */
+ if (is_dir_sep(path[0]))
+ return 1;
+
+ return 0;
+}
+
+#undef offset_1st_component
+#define offset_1st_component cygwin_offset_1st_component
#endif
#if defined(__MINGW32__)
/* pull in Windows compatibility stuff */
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 649a615ec9..2c6c0a04be 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -774,7 +774,7 @@ do
# file with scheme
for p in file
do
- test_expect_success !WINDOWS "fetch-pack --diag-url $p://$h/$r" '
+ test_expect_success !MINGW "fetch-pack --diag-url $p://$h/$r" '
check_prot_path $p://$h/$r $p "/$r"
'
test_expect_success MINGW "fetch-pack --diag-url $p://$h/$r" '
@@ -784,7 +784,7 @@ do
check_prot_path $p:///$r $p "/$r"
'
# No "/~" -> "~" conversion for file
- test_expect_success !WINDOWS "fetch-pack --diag-url $p://$h/~$r" '
+ test_expect_success !MINGW "fetch-pack --diag-url $p://$h/~$r" '
check_prot_path $p://$h/~$r $p "/~$r"
'
test_expect_success MINGW "fetch-pack --diag-url $p://$h/~$r" '
@@ -806,17 +806,11 @@ do
p=ssh
for h in host [::1]
do
- expectation="success"
- if test_have_prereq CYGWIN && test "$h" = "[::1]"
- then
- expectation="failure"
- fi
-
- test_expect_$expectation "fetch-pack --diag-url $h:$r" '
+ test_expect_success "fetch-pack --diag-url $h:$r" '
check_prot_host_port_path $h:$r $p "$h" NONE "$r"
'
# Do "/~" -> "~" conversion
- test_expect_$expectation "fetch-pack --diag-url $h:/~$r" '
+ test_expect_success "fetch-pack --diag-url $h:/~$r" '
check_prot_host_port_path $h:/~$r $p "$h" NONE "~$r"
'
done
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 3dd229c186..35e9f3eb0d 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -530,17 +530,10 @@ do
'
done
-# Parsing of paths that look like IPv6 addresses is broken on Cygwin.
-expectation_for_ipv6_tests=success
-if test_have_prereq CYGWIN
-then
- expectation_for_ipv6_tests=failure
-fi
-
#ipv6
for repo in rep rep/home/project 123
do
- test_expect_$expectation_for_ipv6_tests "clone [::1]:$repo" '
+ test_expect_success "clone [::1]:$repo" '
test_clone_url [::1]:$repo ::1 "$repo"
'
done
@@ -553,7 +546,7 @@ test_expect_success !SANITIZE_LEAK "clone host:/~repo" '
test_clone_url host:/~repo host "~repo"
'
-test_expect_$expectation_for_ipv6_tests !SANITIZE_LEAK "clone [::1]:/~repo" '
+test_expect_success !SANITIZE_LEAK "clone [::1]:/~repo" '
test_clone_url [::1]:/~repo ::1 "~repo"
'
--
2.55.0
^ permalink raw reply related
* Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target
From: Yuxuan Chen @ 2026-07-10 18:30 UTC (permalink / raw)
To: phillip.wood123@gmail.com
Cc: farid.m.zakaria@gmail.com, git@vger.kernel.org, gitster@pobox.com,
newren@gmail.com, phillip.wood@dunelm.org.uk, ps@pks.im,
Yuxuan Chen
In-Reply-To: <afb76b98-661a-4663-8e8b-fd00572db5ba@gmail.com>
From: Yuxuan Chen <i@yuxuan.ch>
Hi Phillip,
I'm Yuxuan, and I work with Farid at Meta. Thank you for reviewing this patch.
It addresses a workflow problem for us, and we appreciate your feedback.
Regarding
> Using an empty commit has a marker has the advantage that applying it cannot
> create conflicts, so you only have to deal with the conflicts caused by the
> commit being dropped, not the by fixup not applying cleanly.
I am concerned, however, that representing a `drop!` commit as an empty marker
would be semantically unsound. We expect `rebase --autosquash` to drop the
target commit, but until that rebase happens, the repository is not in a state
where we consider the target commit dropped: the target's changes are still
present, and the empty marker changes nothing. Therefore, I think a `drop!`
commit should contain the inverse of the patch we intend to drop. That way,
the repository state reflects the intended removal even before autosquash
rewrites the history.
I recognize that applying the inverse patch may cause conflicts. However,
this is not a new problem; `git revert` has the same issue when the inverse
patch does not apply cleanly. Such conflicts reflect the actual difficulty of
undoing the change at that point in the history.
Thanks,
Yuxuan
^ permalink raw reply
* Re: [PATCH v8 4/4] graph: indent visual root in graph
From: Mirko Faina @ 2026-07-10 18:07 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, ayu.chandekar, chandrapratap3519, christian.couder, gitster,
jltobler, karthik.188, krka, peff, phillip.wood,
siddharthasthana31
In-Reply-To: <20260710-ps-pre-commit-indent-v8-4-d3b636463bf4@gmail.com>
On Fri, Jul 10, 2026 at 12:37:07PM +0200, Pablo Sabater wrote:
> When rendering a graph, if the history contains multiple "visual roots",
> actual roots or commits that look like roots (i.e. have their parents
> filtered out) can end up being vertically adjacent to unrelated commits,
> falsely appearing to be related.
>
> A fix for this issue was already attempted [1] a while ago.
>
> This happens because the commits fill the space from left to right and
> when a visual root ends, its column becomes free for the following
> commit even if they are not related. Once this happens the unrelated
> commit is rendered below the visual root. Because there is no special
> character or way to identify when a visual root is rendered making the
> graph confusing.
>
> By indenting the visual roots when there are still commits to show the
> vertical adjacency can be avoided.
>
> Add is_visual_root flag to git_graph making it visible in all graph states,
> give graph_update() a new function, graph_is_visual_root() to know if the
> current commit is a visual root and set is_visual_root.
> The different handled cases are:
>
> - If a visual root has children: similar to GRAPH_PRE_COMMIT state when
> octopus merges need space, an edge row needs to be printed to connect
> the child with the indented visual root. A new state GRAPH_PRE_ROOT is
> needed to connect the child with the visual root:
>
> * child of the visual root
> \ GRAPH_PRE_ROOT
> * visual root indented
>
> - If a visual root is child-less we can skip GRAPH_PRE_ROOT state and
> render the indented commit directly.
>
> * visual root indented
> * unrelated commit
>
> - If two or more visual roots are adjacent: by having a lookahead to the
> next commit that will be rendered, if the next commit is also a visual
> root and we are on a visual root, meaning two visual root adjacent in
> the history, the top one can omit the indent, making the one below to
> indent only once, if there are more adjacent visual commits, the
> indentation will increase for each adjacent one, cascading.
>
> * visual root
> * visual root
> * visual root
> * last commit
>
> Even if the last commit is a root, because there is nothing that will be
> rendered below we can omit the indentation on purpose.
>
> [1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
>
> Helped-by: Kristofer Karlsson <krka@spotify.com>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> graph.c | 235 +++++++++++++++++++
> t/meson.build | 1 +
> t/t4218-log-graph-indentation.sh | 473 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 709 insertions(+)
This doesn't seem to work for every visual root e.g.
git log --graph --oneline --author="Mirko Faina"
The visual roots are not indented.
> +/*
> + * A commit can be a visual root when:
> + *
> + * - It has no parents.
> + *
> + * - It has parents but they are all filtered out and
> + * commit->parents arrives NULL.
> + *
> + * - It is not a boundary commit. Boundary commits also have no visible
> + * parents, but they are not selected as visual roots because they cannot
> + * cause the ambiguity of being vertically adjacent because:
> + *
> + * 1. A boundary only appears because an included commit is its child.
> + * Children are always above, and the renderer draws an edge down to
> + * the boundary from that child. Rather than starting a column like a
> + * visual root would do, it inherits its child column.
> + *
> + * 2. Included commits cannot appear below a boundary. Boundaries are
> + * ancestors of the exclusion point; if an included commit were an
> + * ancestor of the boundary it would be excluded and not rendered.
> + * Boundaries therefore always sink to the bottom.
> + */
> +static int graph_is_visual_root_candidate(struct commit *c)
> +{
> + return c->parents == NULL && !(c->object.flags & BOUNDARY);
> +}
I suspect this behaviour is due to these assumptions being too strict.
When we use the --author option the parents are not filtered out, so it
doesn't return NULL desipte being a visual root. We realize it is a
visual root only on the next commit, but once we are on the next commit
we can't indent as we have already printed this commit.
We realize only on the next commit after hitting simplify_commit(), it
calls get_commit_action() and checks if should keep the commit based on
the regex we provided. If the regex is not matched the commit is just
ignored (we do not filter parents based on regex when we expand a topo
walk).
At least that's what I gather, if anyone can confirm this...
^ permalink raw reply
* Re: [PATCH v2] sequencer: honor --empty when a fixup!/squash! empties its target
From: Michael Montalbo @ 2026-07-10 17:55 UTC (permalink / raw)
To: Farid Zakaria
Cc: git, Phillip Wood, Elijah Newren, Patrick Steinhardt,
Junio C Hamano
In-Reply-To: <20260710-fz-autosquash-empty-v2-1-fa1e277e05f8@gmail.com>
On Fri, Jul 10, 2026 at 10:42:37AM -0700, Farid Zakaria wrote:
>
> +test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
> + git reset --hard base &&
> + test_commit --no-tag addX fileX 1 &&
> + test_commit --no-tag changeX fileX 2 &&
> + test_commit --no-tag later fileW hello &&
> + echo 1 >fileX &&
> + git commit -m "fixup! changeX" fileX &&
> +
> + git rebase -i --autosquash --empty=drop HEAD~4 &&
> +
> + git log --format=%s >actual &&
> + ! grep changeX actual &&
> + grep addX actual &&
> + grep later actual &&
> + echo 1 >expect &&
> + test_cmp expect fileX &&
> + echo hello >expect &&
> + test_cmp expect fileW
> +'
> +
Just a small drive-by comment. We should switch these from `grep`
to `test_grep` (and `! grep` to `test_grep !`) here and for all
the other tests. `test_grep` provides better error diagnostics,
and bare `grep` used as an assertion in tests will be flagged by
the linting process in the near future.
^ permalink raw reply
* [PATCH v2] sequencer: honor --empty when a fixup!/squash! empties its target
From: Farid Zakaria @ 2026-07-10 17:42 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Elijah Newren, Patrick Steinhardt, Junio C Hamano,
Farid Zakaria
When "git rebase --autosquash" melds a "fixup!" or "squash!" commit into
its target, the result can be a commit that no longer changes anything
relative to its parent, for example when the melded change reverts the
target. Rather than dropping or keeping this empty commit, the rebase
stops with
You asked to amend the most recent commit, but doing so would
make it empty. ...
and the "--empty" option has no effect on it. This makes backing a
change out of a series awkward: reverting a commit as a "fixup!" and
running "git rebase --autosquash --empty=drop" ought to remove both the
commit and its revert, but it halts instead.
A "fixup!" is applied by amending HEAD, so the melded commit has HEAD's
parent as its parent and is empty when the index matches the tree of that
parent, not of HEAD. do_pick_commit() only compares against HEAD, so it
never notices that the meld cancelled the commit out and falls through to
"git commit --amend", which refuses to create an empty commit.
After melding a fixup or squash, check whether the amended commit is
empty -- its index matches the tree of HEAD's parent -- and, if so, honor
"--empty" just as for a commit that becomes empty when picked: keep it,
drop it, or halt.
When "--empty=drop" applies, the emptied commit has already been created
by the preceding "pick", so drop it by moving HEAD back to its parent.
The commit is dropped rather than rewritten, so discard the pending
rewrite records and do not record the fixup either, leaving nothing for
the post-rewrite machinery; a following "label" or "update-ref" then sees
HEAD at the parent.
Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
---
At Meta we maintain a fork of LLVM that we regularly rebase onto
upstream. A set of internal patches rides on top, and we keep each one
as a single commit by folding follow-up changes into it with autosquash
"fixup!" commits. That works well for evolving a patch, but not for
retiring one: to back an internal patch out today we hand-edit the
interactive rebase todo list to delete the commit and its scattered
fixups, which is fiddly and easy to get wrong. (The history is rewritten
either way, so a force-push is still needed; what this avoids is the
manual todo surgery.)
It would be nicer to retire a patch the same way we amend one: commit a
revert of it as a "fixup!" and let autosquash fold the two together.
The net change is empty, so the commit should just drop out of the
series. Today it does not -- the rebase stops instead.
For example, starting from a commit we want to retire:
$ git log --oneline
4d5e6f7 add feature patch
9a1b2c3 base
# revert the feature and mark the revert as a fixup of it
$ git revert --no-edit HEAD
$ git commit --amend -m "fixup! add feature patch"
$ git rebase -i --autosquash --empty=drop 9a1b2c3
Rebasing (2/2)
You asked to amend the most recent commit, but doing so would
make it empty. You can repeat your command with --allow-empty [...]
Could not apply 8e9f0a1... # fixup! add feature patch
The "--empty=drop" is ignored. "--empty" only governs commits that are
picked empty, whereas a "fixup!" is applied by amending, and the
emptiness of an amended commit is measured against the wrong parent. So
the rebase falls through to "git commit --amend", which refuses to
create an empty commit, and halts.
With this patch the emptied commit is recognized and handled according
to "--empty", the same as any other commit that becomes empty during a
rebase:
$ git rebase -i --autosquash --empty=drop 9a1b2c3
Rebasing (2/2)
dropping 8e9f0a1... fixup! add feature patch -- resulting commit is empty
Successfully rebased and updated refs/heads/main.
$ git log --oneline
9a1b2c3 base
"--empty=keep" retains it as an empty commit, and "--empty=stop" (the
default under "-i") halts so the user can decide -- matching how these
options already behave for commits that become empty when picked.
Changes in v2 (thanks to Phillip Wood's review):
* An emptied fixup/squash now honors --empty in all cases, including
when the commit it was folded into started out empty; v1 kept that
case regardless of --empty.
* On drop, the dropped commit and its fixup are no longer recorded as
rewritten, so nothing spurious reaches the post-rewrite machinery.
* Added tests for the empty-placeholder + fixup cases and for the
not-recorded-as-rewritten behavior; adjusted t3415 "abort last squash".
* Link to v1: https://lore.kernel.org/r/20260709-fz-autosquash-empty-v1-1-84cb494c3613@gmail.com
---
base-commit: f60db8d575adb79761d363e026fb49bddf330c73
---
Documentation/git-rebase.adoc | 12 ++++
sequencer.c | 148 +++++++++++++++++++++++++++++++++++-------
t/t3415-rebase-autosquash.sh | 138 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 275 insertions(+), 23 deletions(-)
diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
index f6c22d1598..7eb8bbe95f 100644
--- a/Documentation/git-rebase.adoc
+++ b/Documentation/git-rebase.adoc
@@ -282,6 +282,11 @@ by `git log --cherry-mark ...`) are detected and dropped as a
preliminary step (unless `--reapply-cherry-picks` or `--keep-base` is
passed).
+
+A commit can also become empty as a result of `--autosquash`, when a
+`fixup!` or `squash!` commit cancels out all of the changes of the
+commit it is melded into. Such a commit is treated the same way and is
+dropped, kept, or stopped at according to this option.
++
See also INCOMPATIBLE OPTIONS below.
--no-keep-empty::
@@ -591,6 +596,13 @@ changed from `pick` to `squash`, `fixup` or `fixup -C`, respectively, and they
are moved right after the commit they modify. The `--interactive` option can
be used to review and edit the todo list before proceeding.
+
+If melding a `fixup!` or `squash!` commit cancels out all of the changes of
+the commit it is applied to, the result is an empty commit. The handling of
+these empty commits can be configured with the `--empty` option: the emptied
+commit is dropped, kept, or stopped at. This makes it possible to back a
+change out of a series by committing a revert of it as a `fixup!` and letting
+`--autosquash --empty=drop` remove both.
++
The recommended way to create commits with squash markers is by using the
`--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
linkgit:git-commit[1], which take the target commit as an argument and
diff --git a/sequencer.c b/sequencer.c
index 0fe8fed6c3..bc24132c7c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1817,6 +1817,39 @@ static int allow_empty(struct repository *r,
return 0;
}
+/*
+ * Melding a "fixup!"/"squash!" amends HEAD, so the resulting commit is empty
+ * when the index matches the tree of HEAD's parent (rather than of HEAD, as a
+ * plain pick would). Returns 1 if the amended commit would be empty, 0 if not,
+ * and negative on error.
+ */
+static int amended_commit_is_empty(struct repository *r)
+{
+ struct object_id head_oid, *cache_tree_oid;
+ const struct object_id *parent_tree_oid;
+ struct commit *head_commit;
+
+ if (repo_get_oid(r, "HEAD", &head_oid))
+ return error(_("could not resolve HEAD commit"));
+ head_commit = lookup_commit_reference(r, &head_oid);
+ if (!head_commit || repo_parse_commit(r, head_commit))
+ return -1;
+
+ if (head_commit->parents) {
+ struct commit *parent = head_commit->parents->item;
+ if (repo_parse_commit(r, parent))
+ return -1;
+ parent_tree_oid = get_commit_tree_oid(parent);
+ } else {
+ parent_tree_oid = the_hash_algo->empty_tree;
+ }
+
+ if (!(cache_tree_oid = get_cache_tree_oid(r->index)))
+ return -1;
+
+ return oideq(cache_tree_oid, parent_tree_oid);
+}
+
static struct {
char c;
const char *str;
@@ -2260,10 +2293,34 @@ static const char *reflog_message(struct replay_opts *opts,
return buf.buf;
}
+/*
+ * A "fixup!"/"squash!" that melds into HEAD may empty it out. In that case,
+ * with --empty=drop, we want to drop the commit entirely. Since the commit
+ * being amended has already been created (by the preceding "pick"), and the
+ * index and worktree already match the tree of its parent, dropping it is a
+ * matter of moving HEAD back to that parent.
+ */
+static int reset_head_to_parent(struct repository *r, struct replay_opts *opts,
+ struct object_id *head)
+{
+ struct commit *head_commit = lookup_commit_reference(r, head);
+
+ if (!head_commit || repo_parse_commit(r, head_commit))
+ return error(_("could not parse HEAD commit"));
+ if (!head_commit->parents)
+ return error(_("cannot drop the root commit"));
+
+ return refs_update_ref(get_main_ref_store(r),
+ reflog_message(opts, "fixup",
+ "dropping emptied commit"),
+ "HEAD", &head_commit->parents->item->object.oid,
+ head, 0, UPDATE_REFS_MSG_ON_ERR);
+}
+
static int do_pick_commit(struct repository *r,
struct todo_item *item,
struct replay_opts *opts,
- int final_fixup, int *check_todo)
+ int final_fixup, int *check_todo, int *dropped)
{
struct replay_ctx *ctx = opts->ctx;
unsigned int flags = should_edit(opts) ? EDIT_MSG : 0;
@@ -2277,6 +2334,9 @@ static int do_pick_commit(struct repository *r,
enum todo_command command = item->command;
struct commit *commit = item->commit;
+ if (dropped)
+ *dropped = 0;
+
if (is_rebase_i(opts))
reflog_action = reflog_message(
opts, command_to_string(item->command), NULL);
@@ -2493,23 +2553,67 @@ static int do_pick_commit(struct repository *r,
}
drop_commit = 0;
- allow = allow_empty(r, opts, commit);
- if (allow < 0) {
- res = allow;
- goto leave;
- } else if (allow == 1) {
- flags |= ALLOW_EMPTY;
- } else if (allow == 2) {
- drop_commit = 1;
- refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
- NULL, REF_NO_DEREF);
- unlink(git_path_merge_msg(r));
- refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
- NULL, REF_NO_DEREF);
- fprintf(stderr,
- _("dropping %s %s -- patch contents already upstream\n"),
- oid_to_hex(&commit->object.oid), msg.subject);
- } /* else allow == 0 and there's nothing special to do */
+ if (flags & AMEND_MSG) {
+ /*
+ * A "fixup!"/"squash!" amends HEAD. Separately from the usual
+ * empty-commit handling, check whether applying it leaves the
+ * commit empty and, if so, honor --empty (keep, drop, or -- when
+ * neither is requested -- halt below in do_commit), just as for a
+ * commit that becomes empty when picked.
+ */
+ int melded_empty = amended_commit_is_empty(r);
+ if (melded_empty < 0) {
+ res = melded_empty;
+ goto leave;
+ } else if (melded_empty && opts->keep_redundant_commits) {
+ flags |= ALLOW_EMPTY;
+ } else if (melded_empty && opts->drop_redundant_commits) {
+ drop_commit = 1;
+ refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
+ NULL, REF_NO_DEREF);
+ unlink(git_path_merge_msg(r));
+ refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
+ NULL, REF_NO_DEREF);
+ /*
+ * The commit the fixup was melded into was already
+ * created by the preceding "pick", so drop it by moving
+ * HEAD back to its parent. Since the commit is being
+ * dropped rather than rewritten, discard the pending
+ * rewrite records and tell our caller not to add one, so
+ * that neither the dropped commit nor the fixup is
+ * recorded as rewritten.
+ */
+ res = reset_head_to_parent(r, opts, &head);
+ if (res)
+ goto leave;
+ unlink(rebase_path_rewritten_pending());
+ if (dropped)
+ *dropped = 1;
+ fprintf(stderr,
+ _("dropping %s %s -- resulting commit is empty\n"),
+ oid_to_hex(&commit->object.oid), msg.subject);
+ }
+ /* else the meld is non-empty, or empty but neither kept nor
+ * dropped, in which case do_commit halts on the empty result. */
+ } else {
+ allow = allow_empty(r, opts, commit);
+ if (allow < 0) {
+ res = allow;
+ goto leave;
+ } else if (allow == 1) {
+ flags |= ALLOW_EMPTY;
+ } else if (allow == 2) {
+ drop_commit = 1;
+ refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
+ NULL, REF_NO_DEREF);
+ unlink(git_path_merge_msg(r));
+ refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
+ NULL, REF_NO_DEREF);
+ fprintf(stderr,
+ _("dropping %s %s -- patch contents already upstream\n"),
+ oid_to_hex(&commit->object.oid), msg.subject);
+ } /* else allow == 0 and there's nothing special to do */
+ }
if (!opts->no_commit && !drop_commit) {
if (author || command == TODO_REVERT || (flags & AMEND_MSG))
res = do_commit(r, msg_file, author, reflog_action,
@@ -4958,12 +5062,12 @@ static int pick_one_commit(struct repository *r,
struct replay_opts *opts,
int *check_todo, int* reschedule)
{
- int res;
+ int res, dropped = 0;
struct todo_item *item = todo_list->items + todo_list->current;
const char *arg = todo_item_get_arg(todo_list, item);
res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
- check_todo);
+ check_todo, &dropped);
if (is_rebase_i(opts) && res < 0) {
/* Reschedule */
*reschedule = 1;
@@ -4980,7 +5084,7 @@ static int pick_one_commit(struct repository *r,
return error_with_patch(r, commit,
arg, item->arg_len, opts, res, !res);
}
- if (is_rebase_i(opts) && !res)
+ if (is_rebase_i(opts) && !res && !dropped)
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
if (res && is_fixup(item->command)) {
@@ -5545,7 +5649,7 @@ static int single_pick(struct repository *r,
TODO_PICK : TODO_REVERT;
item.commit = cmit;
- return do_pick_commit(r, &item, opts, 0, &check_todo);
+ return do_pick_commit(r, &item, opts, 0, &check_todo, NULL);
}
int sequencer_pick_revisions(struct repository *r,
diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 5033411a43..b4607f44b3 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -461,8 +461,10 @@ test_expect_success 'abort last squash' '
git commit --allow-empty -m second &&
git commit --allow-empty --squash HEAD &&
+ : "squashing empty onto empty leaves an empty commit; --empty=keep" &&
+ : "keeps it so the squash still reaches the editor, which aborts" &&
test_must_fail git -c core.editor="grep -q ^pick" \
- rebase -ki --autosquash HEAD~4 &&
+ rebase -ki --autosquash --empty=keep HEAD~4 &&
: do not finish the squash, but resolve it manually &&
git commit --allow-empty --amend -m edited-first &&
git rebase --skip &&
@@ -510,4 +512,138 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
test_commit_message HEAD -m "something"
'
+test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
+ git reset --hard base &&
+ test_commit --no-tag addX fileX 1 &&
+ test_commit --no-tag changeX fileX 2 &&
+ test_commit --no-tag later fileW hello &&
+ echo 1 >fileX &&
+ git commit -m "fixup! changeX" fileX &&
+
+ git rebase -i --autosquash --empty=drop HEAD~4 &&
+
+ git log --format=%s >actual &&
+ ! grep changeX actual &&
+ grep addX actual &&
+ grep later actual &&
+ echo 1 >expect &&
+ test_cmp expect fileX &&
+ echo hello >expect &&
+ test_cmp expect fileW
+'
+
+test_expect_success 'fixup! that empties its target is kept with --empty=keep' '
+ git reset --hard base &&
+ test_commit --no-tag addY fileY 1 &&
+ test_commit --no-tag changeY fileY 2 &&
+ echo 1 >fileY &&
+ git commit -m "fixup! changeY" fileY &&
+
+ git rebase -i --autosquash --empty=keep HEAD~3 &&
+
+ git log --format=%s >actual &&
+ grep changeY actual &&
+ : "the retained commit is empty" &&
+ git diff --exit-code HEAD~1 HEAD &&
+ echo 1 >expect &&
+ test_cmp expect fileY
+'
+
+test_expect_success 'fixup! that empties its target stops with --empty=stop' '
+ git reset --hard base &&
+ test_commit --no-tag addZ fileZ 1 &&
+ test_commit --no-tag changeZ fileZ 2 &&
+ echo 1 >fileZ &&
+ git commit -m "fixup! changeZ" fileZ &&
+
+ test_when_finished "git rebase --abort" &&
+ test_must_fail git rebase -i --autosquash --empty=stop HEAD~3
+'
+
+test_expect_success 'squash! that empties its target is dropped with --empty=drop' '
+ git reset --hard base &&
+ test_commit --no-tag addS fileS 1 &&
+ test_commit --no-tag changeS fileS 2 &&
+ echo 1 >fileS &&
+ git commit -m "squash! changeS" fileS &&
+
+ git rebase -i --autosquash --empty=drop HEAD~3 &&
+
+ git log --format=%s >actual &&
+ ! grep changeS actual &&
+ grep addS actual &&
+ echo 1 >expect &&
+ test_cmp expect fileS
+'
+
+test_expect_success 'fixup! filling in an empty commit keeps a non-empty commit' '
+ git reset --hard base &&
+ git commit --allow-empty -m placeholder &&
+ test_commit --no-tag "fixup! placeholder" fileP content &&
+
+ git rebase -i --autosquash --empty=stop HEAD~2 &&
+
+ git log --format=%s >actual &&
+ grep placeholder actual &&
+ echo content >expect &&
+ test_cmp expect fileP &&
+ : "the once-empty placeholder is no longer empty" &&
+ test_must_fail git diff --exit-code HEAD~1 HEAD
+'
+
+test_expect_success 'fixup! leaving an empty commit empty stops with --empty=stop' '
+ git reset --hard base &&
+ git commit --allow-empty -m placeholder &&
+ git commit --allow-empty -m "fixup! placeholder" &&
+
+ test_when_finished "git rebase --abort" &&
+ test_must_fail git rebase -i --autosquash --empty=stop HEAD~2
+'
+
+test_expect_success 'fixup! leaving an empty commit empty is dropped with --empty=drop' '
+ git reset --hard base &&
+ git commit --allow-empty -m placeholder &&
+ git commit --allow-empty -m "fixup! placeholder" &&
+
+ git rebase -i --autosquash --empty=drop HEAD~2 &&
+
+ git log --format=%s >actual &&
+ ! grep placeholder actual
+'
+
+test_expect_success 'fixup! leaving an empty commit empty is kept with --empty=keep' '
+ git reset --hard base &&
+ git commit --allow-empty -m placeholder &&
+ git commit --allow-empty -m "fixup! placeholder" &&
+
+ git rebase -i --autosquash --empty=keep HEAD~2 &&
+
+ git log --format=%s >actual &&
+ grep placeholder actual &&
+ git diff --exit-code HEAD~1 HEAD
+'
+
+test_expect_success 'a dropped emptied fixup is not recorded as rewritten' '
+ git reset --hard base &&
+ test_commit --no-tag preR fileR 1 &&
+ test_commit --no-tag changeR fileR 2 &&
+ R=$(git rev-parse HEAD) &&
+ echo 1 >fileR &&
+ git commit -m "fixup! changeR" fileR &&
+ F=$(git rev-parse HEAD) &&
+ test_commit --no-tag keepR fileK keep &&
+
+ test_when_finished "rm -f .git/hooks/post-rewrite actual.rewrites" &&
+ write_script .git/hooks/post-rewrite <<-\EOF &&
+ cat >actual.rewrites
+ EOF
+
+ git rebase -i --autosquash --empty=drop HEAD~4 &&
+
+ : "changeR and its fixup were dropped, so must not be reported as" &&
+ : "rewritten, but the surviving keepR must be" &&
+ ! grep -e "$R" -e "$F" actual.rewrites &&
+ grep "$(git rev-parse HEAD)" actual.rewrites
+'
+
test_done
^ permalink raw reply related
* Re: [PATCH v3 0/2] prio-queue: use bottom-up sift for extract-min
From: Kristofer Karlsson @ 2026-07-10 17:40 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <dfab9ff4-fbfa-4ea2-bea3-09c1d1b1cc18@web.de>
On Fri, 10 Jul 2026 at 18:37, René Scharfe <l.s.r@web.de> wrote:
>
> On 7/8/26 7:49 PM, Kristofer Karlsson via GitGitGadget wrote:
> > Note: sift_up() currently uses swap, matching the existing code style. It
> > could be further optimized to use copy (hold the element in a temp, shift
> > parents down, write once), but that would require changing compare() to
> > accept element values instead of array indices. Left for a potential
> > follow-up.
>
> Same for sift_down_root(), I guess? It could almost halve the number of
> writes, right? I wonder how much of that benefit will be eaten by
> caching.
Hm yes indeed, I stopped looking past sift_up() when I realized I should
not expand the scope of the change. But I think the CPU cache
effectively makes the swap almost as cheap in practice.
- Kristofer
^ permalink raw reply
* [PATCH v2 3/3] t/README: document writing concurrency-safe helpers
From: Michael Montalbo via GitGitGadget @ 2026-07-10 17:30 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2171.v2.git.1783704657.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
The apply-one-time-script.sh and http-429.sh fixes addressed the same
underlying problem: a test helper assuming it has exclusive access to a
file when the web server can run it for several requests at once. The
atomic idioms that avoid this are not specific to CGI or to HTTP, so
document them generally, alongside the other guidance for writing tests,
and leave a pointer from the lib-httpd helper list rather than a local
comment. The note covers the anti-pattern (a "test -f" then a separate
act) and the two safe operations (mkdir to elect a winner, rename to
consume a one-shot marker), citing Git's own lockfile machinery and
make_symlink() as precedent.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
t/README | 32 ++++++++++++++++++++++++++++++++
t/lib-httpd.sh | 3 +++
2 files changed, 35 insertions(+)
diff --git a/t/README b/t/README
index 085921be4b..a9d425f392 100644
--- a/t/README
+++ b/t/README
@@ -854,6 +854,38 @@ from the test harness library. At the end of the script, call
'test_done'.
+Writing concurrency-safe helpers
+--------------------------------
+
+Some test code runs concurrently: a test may background work with '&',
+and the helper scripts installed for the web server (in t/lib-httpd) are
+run once per request, so the same script can execute for several
+requests at once. Such code cannot assume it has exclusive access to a
+file.
+
+When exactly one of several concurrent processes needs to "win" a
+decision, a single atomic filesystem operation can make it, rather than
+a check followed by a separate action. A "test -f X" then "touch X"
+(or "rm X") races: two processes can both pass the check before either
+acts. Two atomic operations avoid this:
+
+ - "mkdir dir", which fails if the directory already exists, so that
+ exactly one caller wins, electing a first or only request (see
+ t/lib-httpd/http-429.sh).
+
+ - "mv src dst" (rename), which fails if the source is gone, so that
+ exactly one caller consumes it, claiming a planted one-shot marker
+ (see t/lib-httpd/apply-one-time-script.sh).
+
+A "$$" suffix on per-request scratch files keeps concurrent invocations
+from clobbering each other's fixed-name files.
+
+This is a standard shell locking idiom, and the same reasoning behind
+Git's own lockfile machinery, which creates its lock with O_CREAT|O_EXCL,
+and make_symlink() in t/test-lib.sh, which uses an mkdir lock: an atomic
+operation whose failure indicates that another process got there first.
+
+
Test harness library
--------------------
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index fc646447d5..d64f9c8c2d 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -159,6 +159,9 @@ prepare_httpd() {
mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH"
cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
cp "$TEST_PATH"/proxy-passwd "$HTTPD_ROOT_PATH"
+ # The web server can run any of these CGI scripts for two requests at
+ # once; a helper that keeps state between requests must do so with an
+ # atomic operation. See "Writing concurrency-safe helpers" in t/README.
install_script incomplete-length-upload-pack-v2-http.sh
install_script incomplete-body-upload-pack-v2-http.sh
install_script error-no-report.sh
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/3] t/lib-httpd: make http-429 first-request check atomic
From: Michael Montalbo via GitGitGadget @ 2026-07-10 17:30 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2171.v2.git.1783704657.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
http-429.sh records "already returned 429 once" with a "test -f"
followed by a "touch" of a shared state file. That check-then-act is not
atomic: Apache can run this CGI for several requests at once, and two of
them can both pass the "test -f" before either "touch"es, so both treat
themselves as the first request. The retry flow that drives this
endpoint is mostly sequential, so this has not been seen to fail, but
the race is latent.
Decide whether this is the first request with a single atomic mkdir,
which fails if the directory already exists, so exactly one of any
concurrent requests is rate-limited and the rest are forwarded.
Skipping state for "permanent" is required for correctness, not just an
optimization. The marker tells a later or concurrent request that a 429
has already been served, so that it forwards to git-http-backend instead
of rate-limiting. Since "permanent" must return 429 to every request,
that marker must never become visible to another such request.
The original did not achieve this by staying stateless: its "touch" of
the marker ran unconditionally, and the "permanent" case removed it
afterward with "rm -f". That create-then-remove leaves a window in which
a concurrent "permanent" request sees the marker and is forwarded. It is
the same class of check-then-act race this patch removes from the
first-request check, latent for the same reason: the flow is mostly
sequential. This version fuses the check and the mark into one atomic
mkdir and, rather than recreate the pattern as mkdir-then-rmdir, skips
the mkdir for "permanent" with a "!= permanent" guard. No marker is ever
created, so there is no window and every "permanent" request
rate-limits.
There is no accompanying regression test. The check and the set are
adjacent commands with no external step in between to synchronize on, so
the overlap cannot be forced deterministically, only reproduced
probabilistically; the fix is preventive.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
t/lib-httpd/http-429.sh | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/t/lib-httpd/http-429.sh b/t/lib-httpd/http-429.sh
index c97b16145b..9746ec67ae 100644
--- a/t/lib-httpd/http-429.sh
+++ b/t/lib-httpd/http-429.sh
@@ -26,14 +26,24 @@ repo_path="${remaining#*/}" # Get rest (repo path)
# The repo name is the first component before any "/"
repo_name="${repo_path%%/*}"
-# Use current directory (HTTPD_ROOT_PATH) for state file
-# Create a safe filename from test_context, retry_after and repo_name
-# This ensures all requests for the same test context share the same state file
+# Use current directory (HTTPD_ROOT_PATH) for state.
+# Create a safe name from test_context, retry_after and repo_name so that all
+# requests for the same test context share the same state.
safe_name=$(echo "${test_context}-${retry_after}-${repo_name}" | tr '/' '_' | tr -cd 'a-zA-Z0-9_-')
-state_file="http-429-state-${safe_name}"
+state="http-429-state-${safe_name}"
-# Check if this is the first call (no state file exists)
-if test -f "$state_file"
+# This endpoint returns 429 to the first request and forwards later ones to
+# git-http-backend, so the retry succeeds. Apache can run this CGI for several
+# requests at once, so a single atomic "mkdir" elects that first request: the
+# one whose mkdir succeeds returns 429 and leaves the directory behind as the
+# "already rate-limited" marker; every later request finds the directory (mkdir
+# fails) and is forwarded.
+#
+# "permanent" is the exception: it must return 429 to every request and never
+# succeed, so it skips the mkdir and records no state. A leftover directory
+# would make its own later requests find the marker and be forwarded, which is
+# exactly what "permanent" must not do.
+if test "$retry_after" != permanent && ! mkdir "$state" 2>/dev/null
then
# Already returned 429 once, forward to git-http-backend
# Set PATH_INFO to just the repo path (without retry-after value)
@@ -52,9 +62,6 @@ then
exec "$GIT_EXEC_PATH/git-http-backend"
fi
-# Mark that we've returned 429
-touch "$state_file"
-
# Output HTTP 429 response
printf "Status: 429 Too Many Requests\r\n"
@@ -67,8 +74,7 @@ case "$retry_after" in
printf "Retry-After: invalid-format-123abc\r\n"
;;
permanent)
- # Always return 429, don't set state file for success
- rm -f "$state_file"
+ # Always return 429
printf "Retry-After: 1\r\n"
printf "Content-Type: text/plain\r\n"
printf "\r\n"
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests
From: Michael Montalbo via GitGitGadget @ 2026-07-10 17:30 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2171.v2.git.1783704657.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
apply-one-time-script.sh checks for the "one-time-script" marker, runs
it, captures the git-http-backend response in the fixed-name files "out"
and "out_modified", and removes the marker only after it has finished
serving the modified response. Because the client receives the response
body before that removal, it can start its next request while the marker
still exists. Apache can then run this CGI for two requests at once: a
partial fetch that receives a REF_DELTA against a missing promisor
object lazily fetches that base while the first response is still in
flight. The second request passes the marker check, the first request
then removes the marker, and the second fails to exec the now-missing
marker, emits no output, and the server answers HTTP 500:
fatal: ... The requested URL returned error: 500
fatal: could not fetch <oid> from promisor remote
This has been seen as a flaky failure of t5616.47 on the macOS CI
runners.
Claim the marker atomically with a rename, and only once the one-time
script has succeeded and actually changed the response; give the scratch
files per-request names. A request that loses the rename, or whose
script fails or leaves the response unchanged, serves the unmodified
body and keeps the marker for a later request. No path emits an empty
body, so the HTTP 500 no longer occurs.
Running the one-time script more than once is fine; the only thing to
avoid is serving a second, racing request's modified output. Two
requests can both find the marker and run the script before either
renames it away, but the rename is atomic, so exactly one of them wins:
it serves its modified body and consumes the marker. The loser's rename
fails because the marker is already gone, so it discards the modified
output it produced and serves the unmodified body instead. The rename,
not running the script, is what is serialized.
Add t5567 to lock this down. The overlap depends on timing, so a live
httpd test such as t5616.47 (the real code path) passes almost every
time even against the buggy helper; t5567 instead drives the helper
directly with a fake git-http-backend and forces the overlap with FIFOs.
Against the pre-fix helper it fails with the same shell error seen in
the field:
./one-time-script: No such file or directory
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
t/lib-httpd/apply-one-time-script.sh | 44 +++++++++----
t/meson.build | 1 +
t/t5567-one-time-script.sh | 96 ++++++++++++++++++++++++++++
3 files changed, 127 insertions(+), 14 deletions(-)
create mode 100755 t/t5567-one-time-script.sh
diff --git a/t/lib-httpd/apply-one-time-script.sh b/t/lib-httpd/apply-one-time-script.sh
index b1682944e2..adb9cec528 100644
--- a/t/lib-httpd/apply-one-time-script.sh
+++ b/t/lib-httpd/apply-one-time-script.sh
@@ -6,21 +6,37 @@
#
# This can be used to simulate the effects of the repository changing in
# between HTTP request-response pairs.
-if test -f one-time-script
-then
- LC_ALL=C
- export LC_ALL
+#
+# Apache can run this CGI for concurrent requests (for example a partial fetch
+# that lazily fetches a missing object while the first response is still in
+# flight), so the helper claims the marker atomically with a rename, and only
+# once it has decided to modify the response. A request that loses the race
+# finds the marker already gone and serves its response unchanged; no request
+# is left emitting an empty body, which the server would report as HTTP 500.
+# Scratch files are per-request ($$) so concurrent requests do not clobber each
+# other.
+#
+# The script may run more than once: the marker is consumed when the response
+# actually changes (the rename after "cmp"), not when the script runs, so a
+# request whose response is not the targeted one runs the script, sees no
+# change, and leaves the marker for a later request. That is safe because the
+# scripts are stateless filters over the captured response.
- "$GIT_EXEC_PATH/git-http-backend" >out
- ./one-time-script out >out_modified
+test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend"
- if cmp -s out out_modified
- then
- cat out
- else
- cat out_modified
- rm one-time-script
- fi
+LC_ALL=C
+export LC_ALL
+
+out=out.$$
+modified=out-modified.$$
+"$GIT_EXEC_PATH/git-http-backend" >"$out"
+
+if ./one-time-script "$out" 2>/dev/null >"$modified" &&
+ ! cmp -s "$out" "$modified" &&
+ mv one-time-script one-time-script.$$ 2>/dev/null
+then
+ cat "$modified"
else
- "$GIT_EXEC_PATH/git-http-backend"
+ cat "$out"
fi
+rm -f "$out" "$modified" one-time-script.$$
diff --git a/t/meson.build b/t/meson.build
index 3219264fe7..a118a4d719 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -707,6 +707,7 @@ integration_tests = [
't5564-http-proxy.sh',
't5565-push-multiple.sh',
't5566-push-group.sh',
+ 't5567-one-time-script.sh',
't5570-git-daemon.sh',
't5571-pre-push-hook.sh',
't5572-pull-submodule.sh',
diff --git a/t/t5567-one-time-script.sh b/t/t5567-one-time-script.sh
new file mode 100755
index 0000000000..cd8e656005
--- /dev/null
+++ b/t/t5567-one-time-script.sh
@@ -0,0 +1,96 @@
+#!/bin/sh
+
+test_description='apply-one-time-script CGI helper is safe under concurrent requests'
+
+. ./test-lib.sh
+
+HELPER="$TEST_DIRECTORY/lib-httpd/apply-one-time-script.sh"
+
+test_expect_success PIPE 'concurrent requests: one rewritten, one passed through, neither empty' '
+ mkdir workdir fakebin &&
+ ENTERED="$PWD/entered" &&
+ GATE="$PWD/gate" &&
+ export ENTERED GATE &&
+ mkfifo "$ENTERED" "$GATE" &&
+
+ # Stand in for git-http-backend. The modify role returns a response
+ # containing "packfile", which the one-time script rewrites. The
+ # passthrough role returns a response that is left untouched, but first
+ # announces that it has entered the helper and then blocks, so that it
+ # is still in flight when the modify role claims and removes the marker.
+ write_script fakebin/git-http-backend <<-\EOF &&
+ printf "Status: 200 OK\r\n"
+ printf "Content-Type: application/x-git-result\r\n"
+ printf "\r\n"
+ if test "$ROLE" = modify
+ then
+ printf "packfile\n"
+ else
+ echo entered >"$ENTERED"
+ read -r released <"$GATE"
+ printf "refs\n"
+ fi
+ EOF
+
+ # The transform that replace_packfile would install as one-time-script:
+ # rewrite responses that contain "packfile", leave the rest alone.
+ write_script workdir/one-time-script <<-\EOF &&
+ if grep packfile "$1" >/dev/null
+ then
+ sed "/packfile/q" "$1" &&
+ printf "REPLACED\n"
+ else
+ cat "$1"
+ fi
+ EOF
+
+ GIT_EXEC_PATH="$PWD/fakebin" &&
+ export GIT_EXEC_PATH &&
+
+ # Hold GATE open read-write on fd 9 for the duration, so releasing the
+ # passthrough request below cannot block even if that request has
+ # already exited (it keeps a reader on the FIFO).
+ exec 9<>"$GATE" &&
+
+ # Launch the passthrough request in the background. It enters the
+ # helper, signals us through ENTERED, then blocks on GATE inside the
+ # fake backend. The braces keep the && chain intact while backgrounding
+ # only the subshell, so "wait" can reap it by pid; kill it on any exit
+ # so a stray blocked child cannot hold the test output open and stall a
+ # reader such as prove.
+ { (
+ cd workdir &&
+ ROLE=passthrough sh "$HELPER" >../passthrough.out 2>../passthrough.err
+ ) & } &&
+ passthrough_pid=$! &&
+ test_when_finished "kill $passthrough_pid 2>/dev/null || :" &&
+
+ # Wait until the passthrough request is past the marker check.
+ read -r entered <"$ENTERED" &&
+
+ # Run the modifying request to completion while the passthrough request
+ # is still blocked.
+ (
+ cd workdir &&
+ ROLE=modify sh "$HELPER" >../modify.out 2>../modify.err
+ ) &&
+
+ # Release the passthrough request and let it finish. Ignore the helper
+ # exit status here so a broken helper is diagnosed by the assertions
+ # below rather than aborting the test.
+ echo released >&9 &&
+ { wait "$passthrough_pid" || :; } &&
+
+ # Neither request may error out or produce an empty (HTTP 500) body,
+ # and each must have played its role: the modify request rewrote its
+ # response and the passthrough request came through untouched.
+ test_must_be_empty passthrough.err &&
+ test_must_be_empty modify.err &&
+ test_grep "Status: 200 OK" passthrough.out &&
+ test_grep "Status: 200 OK" modify.out &&
+ test_grep REPLACED modify.out &&
+ test_grep ! REPLACED passthrough.out &&
+ test_grep refs passthrough.out
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/3] t/lib-httpd: make CGI test helpers concurrency-safe
From: Michael Montalbo via GitGitGadget @ 2026-07-10 17:30 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo
In-Reply-To: <pull.2171.git.1783479584.gitgitgadget@gmail.com>
The httpd tests share a handful of CGI helper scripts under t/lib-httpd. Two
of them keep state between requests in the shared HTTPD_ROOT_PATH on the
assumption that the web server hands them one request at a time. It does
not: Apache serves requests concurrently, and a single Git operation can
open more than one request to the same endpoint at once. A partial fetch
that receives a REF_DELTA against a missing promisor object lazily fetches
that base while the first response is still being served.
Under that overlap apply-one-time-script.sh loses: two requests both pass
its "test -f one-time-script" check, one removes the marker, and the other
fails to exec it and emits an empty body, which the server answers as HTTP
500. In the field this is an occasional failure[1] of:
t5616.47 tolerate server sending REF_DELTA against missing promisor objects
on the macOS CI runners, with:
fatal: ... The requested URL returned error: 500 fatal: could not fetch from
promisor remote
I could not reproduce it against a live server (the window is tiny and
timing-dependent), but the macOS CI error log names the exact failure, and
the new test reproduces the helper's shell error.
http-429.sh keeps its "already returned 429 once" state with the same
non-atomic test-and-set. Its retry flow is mostly sequential so it seems
less likely to fail, but it is the same latent race.
Each fix is local: claim/consume the one-shot marker with an atomic rename,
and elect the first request with an atomic mkdir, rather than a "test -f"
followed by a separate remove or touch.
* Patch 1 fixes apply-one-time-script.sh (the actual flake) and adds t5567,
which drives the helper directly with no web server so the overlap can be
forced deterministically.
* Patch 2 makes http-429.sh atomic.
* Patch 3 documents the atomic idioms generally in t/README (they are not
specific to CGI or HTTP), citing Git's own lockfile machinery and
make_symlink(), with a pointer from the lib-httpd list.
Changes since v1:
* Clarify that one-time-script.sh can and should be able to run more than
once. Explain that the constraint on the script execution is that one and
only one modified response is guaranteed to be returned to the client.
* The existing behavior w.r.t. inconsistent use of locale C vs. inherited
locale when executing t/lib-httpd/apply-one-time-script.sh has been
retained from the original version, and is left as future potential
cleanup.
* Spell out why the logic changes to the "permanent mode check" in
t/lib-httpd/http-429.sh are needed for correctness, rather than an
optimization opportunity.
[1]
https://github.com/gitgitgadget/git/actions/runs/28756172690/job/85263916762?pr=2169
Michael Montalbo (3):
t/lib-httpd: fix apply-one-time-script race under concurrent requests
t/lib-httpd: make http-429 first-request check atomic
t/README: document writing concurrency-safe helpers
t/README | 32 ++++++++++
t/lib-httpd.sh | 3 +
t/lib-httpd/apply-one-time-script.sh | 44 +++++++++----
t/lib-httpd/http-429.sh | 28 ++++----
t/meson.build | 1 +
t/t5567-one-time-script.sh | 96 ++++++++++++++++++++++++++++
6 files changed, 179 insertions(+), 25 deletions(-)
create mode 100755 t/t5567-one-time-script.sh
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2171%2Fmmontalbo%2Fmm%2Flib-httpd-cgi-safe-proto-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2171/mmontalbo/mm/lib-httpd-cgi-safe-proto-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2171
Range-diff vs v1:
1: 9f48aa6d6d ! 1: 79b56402c0 t/lib-httpd: fix apply-one-time-script race under concurrent requests
@@ Commit message
body and keeps the marker for a later request. No path emits an empty
body, so the HTTP 500 no longer occurs.
+ Running the one-time script more than once is fine; the only thing to
+ avoid is serving a second, racing request's modified output. Two
+ requests can both find the marker and run the script before either
+ renames it away, but the rename is atomic, so exactly one of them wins:
+ it serves its modified body and consumes the marker. The loser's rename
+ fails because the marker is already gone, so it discards the modified
+ output it produced and serves the unmodified body instead. The rename,
+ not running the script, is what is serialized.
+
Add t5567 to lock this down. The overlap depends on timing, so a live
httpd test such as t5616.47 (the real code path) passes almost every
time even against the buggy helper; t5567 instead drives the helper
@@ t/lib-httpd/apply-one-time-script.sh
+# is left emitting an empty body, which the server would report as HTTP 500.
+# Scratch files are per-request ($$) so concurrent requests do not clobber each
+# other.
-+
-+test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend"
++#
++# The script may run more than once: the marker is consumed when the response
++# actually changes (the rename after "cmp"), not when the script runs, so a
++# request whose response is not the targeted one runs the script, sees no
++# change, and leaves the marker for a later request. That is safe because the
++# scripts are stateless filters over the captured response.
- "$GIT_EXEC_PATH/git-http-backend" >out
- ./one-time-script out >out_modified
-+LC_ALL=C
-+export LC_ALL
++test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend"
- if cmp -s out out_modified
- then
@@ t/lib-httpd/apply-one-time-script.sh
- cat out_modified
- rm one-time-script
- fi
++LC_ALL=C
++export LC_ALL
++
+out=out.$$
+modified=out-modified.$$
+"$GIT_EXEC_PATH/git-http-backend" >"$out"
2: efd34c1715 ! 2: 5f56f32a74 t/lib-httpd: make http-429 first-request check atomic
@@ Commit message
which fails if the directory already exists, so exactly one of any
concurrent requests is rate-limited and the rest are forwarded.
+ Skipping state for "permanent" is required for correctness, not just an
+ optimization. The marker tells a later or concurrent request that a 429
+ has already been served, so that it forwards to git-http-backend instead
+ of rate-limiting. Since "permanent" must return 429 to every request,
+ that marker must never become visible to another such request.
+
+ The original did not achieve this by staying stateless: its "touch" of
+ the marker ran unconditionally, and the "permanent" case removed it
+ afterward with "rm -f". That create-then-remove leaves a window in which
+ a concurrent "permanent" request sees the marker and is forwarded. It is
+ the same class of check-then-act race this patch removes from the
+ first-request check, latent for the same reason: the flow is mostly
+ sequential. This version fuses the check and the mark into one atomic
+ mkdir and, rather than recreate the pattern as mkdir-then-rmdir, skips
+ the mkdir for "permanent" with a "!= permanent" guard. No marker is ever
+ created, so there is no window and every "permanent" request
+ rate-limits.
+
There is no accompanying regression test. The check and the set are
adjacent commands with no external step in between to synchronize on, so
the overlap cannot be forced deterministically, only reproduced
@@ t/lib-httpd/http-429.sh: repo_path="${remaining#*/}" # Get rest (repo path)
-# Check if this is the first call (no state file exists)
-if test -f "$state_file"
-+# Apache can run this CGI for concurrent requests, so the script decides
-+# whether this is the first call with a single atomic "mkdir": it succeeds for
-+# exactly one of any racing requests and fails for the rest. "permanent"
-+# always rate-limits and records no state.
++# This endpoint returns 429 to the first request and forwards later ones to
++# git-http-backend, so the retry succeeds. Apache can run this CGI for several
++# requests at once, so a single atomic "mkdir" elects that first request: the
++# one whose mkdir succeeds returns 429 and leaves the directory behind as the
++# "already rate-limited" marker; every later request finds the directory (mkdir
++# fails) and is forwarded.
++#
++# "permanent" is the exception: it must return 429 to every request and never
++# succeed, so it skips the mkdir and records no state. A leftover directory
++# would make its own later requests find the marker and be forwarded, which is
++# exactly what "permanent" must not do.
+if test "$retry_after" != permanent && ! mkdir "$state" 2>/dev/null
then
# Already returned 429 once, forward to git-http-backend
3: 771d264d29 = 3: f158e1f92e t/README: document writing concurrency-safe helpers
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v3 2/2] prio-queue: use cascade for unfused gets
From: Kristofer Karlsson @ 2026-07-10 17:28 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <10fad562-90b8-4feb-b7ab-d61015872127@web.de>
On Fri, 10 Jul 2026 at 18:37, René Scharfe <l.s.r@web.de> wrote:
>
> I see a 1% slowdown on an Apple M5 as well in both cases. I can't
> reproduce it on a Ryzen laptop, but that's too noisy to measure 1%
> changes anyway.
>
> Checked the total number of prio_queue comparisons with the crude patch
> below, and as expected they go down, from 70386235 to 60682175 for Git
> and from 473983445 to 439809087 for Linux. So there's less work to do,
> still user time goes up -- no idea why.
[snip]
> The patch looks fine, though. It introduces struct assignments, but
> they should be OK. Tried replacing them with swap() instead (which
> does a useless extra write), but that didn't change the performance
> (still 1% slowdown). Odd.
First of all, thanks again for the very comprehensive
investigation on your own hardware!
I don't have any Apple machine to test on but I reran your
exact operation on my machine
(Lenovo Thinkpad Intel(R) Core(TM) Ultra 7 155U)
and just saw noise.
As you say, this feels _logically_ better since it's fewer
compares but perhaps this boils down to the cost difference
between executing CPU operations versus memory
access and the CPU cache?
My random guess:
cascade_down() has fewer operations and compares
but needs to visit all levels of the heap,
while sift_down_root perhaps stops slightly earlier,
so the memory region right before the end
gets fewer visits and reduces pressure on
the cache.
I have no idea if my guess is correct,
it's maybe more subtle than that, but
I think ultimately this points to the fact
that while the change is a theoretical
improvement, the real world hardware
tradeoffs make it a non-obvious change.
I think this means we should simply drop the change
and move on -- it produced bigger gains
before making the lazy prio_queue the default,
but now it seems like it is pure noise, unless
the comparator function grows more expensive
in the future.
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH v18 2/7] branch: convert delete_branches() to a flags argument
From: Harald Nordgren @ 2026-07-10 17:25 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <ed72dd31-5d68-4e78-9123-7061b388ecaa@gmail.com>
> This means we have two sources of truth because we modify "flags" later.
> The idea of replacing the old function parameters with local variables
> only works if we're not passing the flags variable on to another
> function so I think we should replace all instances of "force" and
> "quiet" with flags & DELETE_BRANCH_FORCE/QUIET. That way we have a
> single source of truth and should avoid any future regressions like the
> one we saw in an earlier iteration.
Yes, it makes sense.
I just hope we remember this discussion, so another reviewer doesn't
push in the other direction later because it seems like low-hanging
fruit.
Harald
^ permalink raw reply
* Re: [PATCH] b4: include change-id in cover template
From: Junio C Hamano @ 2026-07-10 17:02 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: me, git
In-Reply-To: <alCxgqybWb9eDEG0@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> On Fri, Jul 10, 2026 at 03:22:13PM +0800, Chen Linxuan via B4 Relay wrote:
>> From: Chen Linxuan <me@black-desk.cn>
>>
>> With b4 0.15.2, I hit a local failure after sending a series with the
>> in-tree cover template. The generated sent/<change-id>-vN tag contained
>> base-commit, but did not contain change-id, and later b4 commands failed
>> when trying to read it:
>>
>> CRITICAL: Tag sent/... does not contain change-id info
>>
>> Looking at b4's source, the sent tag message is derived from the rendered
>> cover letter. The same code later parses that tag and expects both
>> base-commit and change-id to be present. The default b4 cover template
>> has both trailers, but our in-tree template only has base-commit.
>>
>> Add the missing change-id trailer next to base-commit so sent tags
>> produced from the project template remain readable by b4's reroll and
>> comparison logic.
>
> Ah, that's indeed an oversight on my side. So this change looks good to
> me, thanks!
>
> Patrick
I would have slightly preferred to see the observation part fully in
present tense (instead of "I did and it failed"), perhaps like so:
When sending a series with the in-tree cover template, the
generated sent/<change-id>-vN tag contains base-commit but lacks
change-id. Later b4 commands fail when trying to read it.
but that is a minor point.
Will queue. Thanks, both.
^ permalink raw reply
* Re: [PATCH 0/7] refs: remove use of `the_repository`
From: Junio C Hamano @ 2026-07-10 16:57 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <alCN2Afi4gTSSajg@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Hm, curious, I cannot reproduce any of these failures at all, everything
> is passing locally when merging "seen" into my branch. Did you maybe
> mismerge the changes in "setup.c" by accident? That seems like the most
> likely reason as you mention that it breaks lots of tests, and "setup.c"
> is of course involved with all of them.
It is more than probable that it was what happened. Will retry the
merge during the integration run I'll make later today.
Thanks.
>
> For reference, this is what the final result of the conflicting part
> looks like on my side:
>
> if (real_git_dir) {
> struct stat st;
>
> if (!exist_ok && !stat(git_dir, &st))
> die(_("%s already exists"), git_dir);
>
> if (!exist_ok && !stat(real_git_dir, &st))
> die(_("%s already exists"), real_git_dir);
>
> apply_and_export_relative_gitdir(repo, real_git_dir, 1);
> git_dir = repo_get_git_dir(repo);
> separate_git_dir(repo, git_dir, original_git_dir);
> } else {
> apply_and_export_relative_gitdir(repo, git_dir, 1);
> git_dir = repo_get_git_dir(repo);
> }
>
> Thanks!
>
> Patrick
^ permalink raw reply
* Re: [PATCH v3 11/11] builtin/receive-pack: stage incoming objects via ODB transactions
From: Junio C Hamano @ 2026-07-10 16:52 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, ps
In-Reply-To: <alEBEbwOMFkVfuk9@denethor>
Justin Tobler <jltobler@gmail.com> writes:
> On 26/07/08 08:49PM, Junio C Hamano wrote:
>> Justin Tobler <jltobler@gmail.com> writes:
>> > update_shallow_info(commands, &si, &ref);
>> > }
>> > use_keepalive = KEEPALIVE_ALWAYS;
>> > - execute_commands(commands, unpack_status, &si,
>> > + execute_commands(commands, unpack_status, &si, transaction,
>> > &push_options);
>>
>> And in such a case, execute_commands() returns without committing
>> the transaction. Is there a need to add and make an
>> odb_transaction_abort() call or something in such a case?
>> Everything should be cleaned up upon process exit, and on file based
>> backends, we probably let the tempfile/lockfile API do their thing
>> to clean up, but are there other things we may want to clean up?
>
> As you mentioned, if we exit before committing the ODB transaction, the
> temporary directory will get cleaned up when the process exits. I don't
> think there is anything else we need to cleanup that wouldn't be handled
> at exit though. Regardless, I do plan to add `odb_transaction_abort()`
> in a followup series and I think it would be nice to have an explicit
> "abort" here when we know that we are not going to commit anyways. I
> would like to defer this to my next series though.
Sounds good. We cannot trigger receive-pack as a subroutine call in
a long running daemon until that happens, but that is OK for now.
One step at a time.
^ permalink raw reply
* Re: [PATCH] sequencer: honor --empty when a fixup!/squash! empties its target
From: Farid Zakaria @ 2026-07-10 16:42 UTC (permalink / raw)
To: Phillip Wood, Farid Zakaria, git
Cc: Phillip Wood, Elijah Newren, Patrick Steinhardt, Junio C Hamano
In-Reply-To: <afb76b98-661a-4663-8e8b-fd00572db5ba@gmail.com>
On Fri Jul 10, 2026 at 6:28 AM PDT, Phillip Wood wrote:
Phillip,
Thank you for responding. This is my first submission to the Git mailing
list.
I want to be forthecoming that I'm not familiar with the Git codebase,
and (maybe unsurprisingly) I have been leveraging LLMs to help me
understand the code and write the patch. I consulted the Git
contribution guidelines and it says that should be OK as long as it's
not "slop". I'm diligent to remaining in the loop (HITL) and reviewing
the code and tests to the best of my understanding of the codebase.
I will send the V2 shortly. Thank you!
> Hi Farid
>
> On 10/07/2026 05:13, Farid Zakaria wrote:
>> When "git rebase --autosquash" melds a "fixup!" or "squash!" commit into
>> its target, the result can be a commit that no longer changes anything
>> relative to its parent, for example when the melded change reverts the
>> target. Rather than dropping or keeping this empty commit, the rebase
>> stops with
>>
>> You asked to amend the most recent commit, but doing so would
>> make it empty. ...
>>
>> and the "--empty" option has no effect on it. This makes backing a
>> change out of a series awkward: reverting a commit as a "fixup!" and
>> running "git rebase --autosquash --empty=drop" ought to remove both the
>> commit and its revert, but it halts instead.
>
> I agree this is a use case that we want to support
>
>> The reason is that allow_empty() decides emptiness with
>> is_index_unchanged(), which compares the index to HEAD. A "fixup!" is
>> applied by amending HEAD, so the commit it produces has HEAD's parent as
>> its parent; it is empty when the index matches the tree of that parent,
>> not of HEAD. A meld that cancels out its target is therefore never
>> recognized as having become empty, and falls through to "git commit
>> --amend", which refuses to create an empty commit.
>
> and with this diagnosis.
>
>> Teach is_index_unchanged() to compare against the tree of HEAD's parent
>> when amending, and teach allow_empty() to classify the result as "became
>> empty" (and thus subject to --empty) unless the commit being melded into
>> was itself already empty, in which case it "started empty" and is
>> governed by allow_empty as before.
>
> However, I think that rather than changing the current check which
> changes the behavior of a fixup commit that becomes empty we should add
> an additional check to see if applying the fixup makes the target commit
> empty. With the patch here a fixup commit that becomes empty is only
> seen as empty if the commit being fixed up is empty in which case we
> always accept the fixup, whereas the current behavior is always to
> respect what --empty says. When I'm planning out a series of commits I
> sometimes create empty commits where the messages says what I'm
> intending to do and then I create fixups for them when I get round to
> writing the code. If one of those fixups becomes empty I want to know
> about it because it means I need to drop the empty commit that's being
> fixed up as well.
>
Thank you for this catch. I will apply the changes you suggest in V2 and
add test cases for this missing behavior.
>> When --empty=drop applies, the emptied commit has already been created
>> by the preceding "pick", so drop it by moving HEAD back to its parent.
>> Do so before the rewritten-commit list is flushed, so that --update-refs
>> and the other rewrite consumers map the dropped commit to its parent.
>
> If we're dropping the commit then we should not record it as rewritten
> so we need to remove the rewritten-pending file. Any labels and
> update-ref commands that come immediately after the dropped commit will
> see HEAD pointing to the dropped commits rewritten parent.
>
I will address this in V2 as well.
>> Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
>> ---
>> At Meta we maintain a fork of LLVM that we regularly rebase onto
>> upstream. A set of internal patches rides on top, and we keep each one
>> as a single commit by folding follow-up changes into it with autosquash
>> "fixup!" commits. That works well for evolving a patch, but not for
>> retiring one: to back an internal patch out today we delete it from the
>> history by hand with an interactive rebase and then force-push, which is
>> easy to get wrong on a shared branch.
>
> You'll still need a forced push though because you're dropping the
> commit. I think the change you're proposing to git would be useful but
> you could automate your existing workflow by setting GIT_SEQUENCE_EDITOR
> to a script that drops the commit and it's fixups from the todo list.
>
True. I guess I should have clarified we run with a script that already
uses GIT_SEQUENCE_EDITOR and force-pushes. I just wanted to avoid cases
where a developer has to intervene in the rebase and force-push.
Developers must still intervene and force-push when a conflict arises in
our workflow though....
>> One open question, for a possible follow-up. A natural next step would
>> be a "revert!" autosquash directive (and a "git commit --revert" to
>> create it), mirroring "fixup!"/"squash!", so
>> that retiring a patch would not require generating the reverse diff by
>> hand. I have deliberately left it out of this series, because its
>> semantics are not obvious: in particular, whether a "revert!" commit
>> should carry the reverse patch as its own content (and thus be an
>> ordinary fixup that this patch already drops), or be an empty marker
>> that instructs the rebase to revert the target commit during the meld.
>> Opinions on whether such a directive is wanted, and which of those two
>> shapes is preferred, would be welcome before I attempt it.
>
> I think having support for creating and squashing revert! (or possibly
> drop!) commits is a good idea (I've a feeling there is some discussion
> about that in the gitgitgadget issue tracker). Using an empty commit has
> a marker has the advantage that applying it cannot create conflicts, so
> you only have to deal with the conflicts caused by the commit being
> dropped, not the by fixup not applying cleanly.
>
This seems like a nice ergonomic improvement but I chose to leave it out
since it seems particularly thorny to get right.
I know for us at Meta, we use Phabricator (similar to Gerrit) and
working with empty commits is I think problematic for the code review
tooling (unsubstantiated).
If this current patch series makes it's way through though I can tackle
an approach as an RFC.
> Thanks
>
> Phillip
>
>> ---
>> base-commit: f60db8d575adb79761d363e026fb49bddf330c73
>> ---
>> Documentation/git-rebase.adoc | 12 ++++++
>> sequencer.c | 96 +++++++++++++++++++++++++++++++++++++++----
>> t/t3415-rebase-autosquash.sh | 64 +++++++++++++++++++++++++++++
>> 3 files changed, 163 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/git-rebase.adoc b/Documentation/git-rebase.adoc
>> index f6c22d1598..7eb8bbe95f 100644
>> --- a/Documentation/git-rebase.adoc
>> +++ b/Documentation/git-rebase.adoc
>> @@ -282,6 +282,11 @@ by `git log --cherry-mark ...`) are detected and dropped as a
>> preliminary step (unless `--reapply-cherry-picks` or `--keep-base` is
>> passed).
>> +
>> +A commit can also become empty as a result of `--autosquash`, when a
>> +`fixup!` or `squash!` commit cancels out all of the changes of the
>> +commit it is melded into. Such a commit is treated the same way and is
>> +dropped, kept, or stopped at according to this option.
>> ++
>> See also INCOMPATIBLE OPTIONS below.
>>
>> --no-keep-empty::
>> @@ -591,6 +596,13 @@ changed from `pick` to `squash`, `fixup` or `fixup -C`, respectively, and they
>> are moved right after the commit they modify. The `--interactive` option can
>> be used to review and edit the todo list before proceeding.
>> +
>> +If melding a `fixup!` or `squash!` commit cancels out all of the changes of
>> +the commit it is applied to, the result is an empty commit. The handling of
>> +these empty commits can be configured with the `--empty` option: the emptied
>> +commit is dropped, kept, or stopped at. This makes it possible to back a
>> +change out of a series by committing a revert of it as a `fixup!` and letting
>> +`--autosquash --empty=drop` remove both.
>> ++
>> The recommended way to create commits with squash markers is by using the
>> `--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
>> linkgit:git-commit[1], which take the target commit as an argument and
>> diff --git a/sequencer.c b/sequencer.c
>> index 0fe8fed6c3..435b100e3d 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -823,7 +823,7 @@ static struct object_id *get_cache_tree_oid(struct index_state *istate)
>> return &istate->cache_tree->oid;
>> }
>>
>> -static int is_index_unchanged(struct repository *r)
>> +static int is_index_unchanged(struct repository *r, int amend)
>> {
>> struct object_id head_oid, *cache_tree_oid;
>> const struct object_id *head_tree_oid;
>> @@ -856,7 +856,26 @@ static int is_index_unchanged(struct repository *r)
>> if (repo_parse_commit(r, head_commit))
>> return -1;
>>
>> - head_tree_oid = get_commit_tree_oid(head_commit);
>> + if (amend) {
>> + /*
>> + * When amending (e.g. melding a "fixup!" or "squash!"),
>> + * the commit we are about to create replaces HEAD, so
>> + * its parent is HEAD's parent. It is therefore empty
>> + * when the index matches the tree of HEAD's parent
>> + * rather than the tree of HEAD itself.
>> + */
>> + if (head_commit->parents) {
>> + struct commit *parent =
>> + head_commit->parents->item;
>> + if (repo_parse_commit(r, parent))
>> + return -1;
>> + head_tree_oid = get_commit_tree_oid(parent);
>> + } else {
>> + head_tree_oid = the_hash_algo->empty_tree;
>> + }
>> + } else {
>> + head_tree_oid = get_commit_tree_oid(head_commit);
>> + }
>> }
>>
>> if (!(cache_tree_oid = get_cache_tree_oid(istate)))
>> @@ -1786,7 +1805,7 @@ static int is_original_commit_empty(struct commit *commit)
>> */
>> static int allow_empty(struct repository *r,
>> struct replay_opts *opts,
>> - struct commit *commit)
>> + struct commit *commit, int amend)
>> {
>> int index_unchanged, originally_empty;
>>
>> @@ -1798,13 +1817,33 @@ static int allow_empty(struct repository *r,
>> * drop_redundant_commits determine whether the commit should be kept or
>> * dropped. If neither is specified, halt.
>> */
>> - index_unchanged = is_index_unchanged(r);
>> + index_unchanged = is_index_unchanged(r, amend);
>> if (index_unchanged < 0)
>> return index_unchanged;
>> if (!index_unchanged)
>> return 0; /* we do not have to say --allow-empty */
>>
>> - originally_empty = is_original_commit_empty(commit);
>> + /*
>> + * When amending (melding a "fixup!"/"squash!"), the resulting commit
>> + * replaces HEAD, so whether it "started" empty or "became" empty is
>> + * decided by whether the commit being melded into was itself empty: if
>> + * HEAD had content that the fixup cancelled out, the commit became empty
>> + * and is subject to keep/drop_redundant; if HEAD was already empty, the
>> + * commit started empty and is subject to allow_empty as usual.
>> + */
>> + if (amend) {
>> + struct object_id head_oid;
>> + struct commit *head_commit;
>> +
>> + if (repo_get_oid(r, "HEAD", &head_oid))
>> + return error(_("could not resolve HEAD commit"));
>> + head_commit = lookup_commit_reference(r, &head_oid);
>> + if (!head_commit)
>> + return -1;
>> + originally_empty = is_original_commit_empty(head_commit);
>> + } else {
>> + originally_empty = is_original_commit_empty(commit);
>> + }
>> if (originally_empty < 0)
>> return originally_empty;
>> if (originally_empty)
>> @@ -2260,6 +2299,30 @@ static const char *reflog_message(struct replay_opts *opts,
>> return buf.buf;
>> }
>>
>> +/*
>> + * A "fixup!"/"squash!" that melds into HEAD may empty it out. In that case,
>> + * with --empty=drop, we want to drop the commit entirely. Since the commit
>> + * being amended has already been created (by the preceding "pick"), and the
>> + * index and worktree already match the tree of its parent, dropping it is a
>> + * matter of moving HEAD back to that parent.
>> + */
>> +static int reset_head_to_parent(struct repository *r, struct replay_opts *opts,
>> + struct object_id *head)
>> +{
>> + struct commit *head_commit = lookup_commit_reference(r, head);
>> +
>> + if (!head_commit || repo_parse_commit(r, head_commit))
>> + return error(_("could not parse HEAD commit"));
>> + if (!head_commit->parents)
>> + return error(_("cannot drop the root commit"));
>> +
>> + return refs_update_ref(get_main_ref_store(r),
>> + reflog_message(opts, "fixup",
>> + "dropping emptied commit"),
>> + "HEAD", &head_commit->parents->item->object.oid,
>> + head, 0, UPDATE_REFS_MSG_ON_ERR);
>> +}
>> +
>> static int do_pick_commit(struct repository *r,
>> struct todo_item *item,
>> struct replay_opts *opts,
>> @@ -2493,7 +2556,7 @@ static int do_pick_commit(struct repository *r,
>> }
>>
>> drop_commit = 0;
>> - allow = allow_empty(r, opts, commit);
>> + allow = allow_empty(r, opts, commit, flags & AMEND_MSG);
>> if (allow < 0) {
>> res = allow;
>> goto leave;
>> @@ -2506,9 +2569,24 @@ static int do_pick_commit(struct repository *r,
>> unlink(git_path_merge_msg(r));
>> refs_delete_ref(get_main_ref_store(r), "", "AUTO_MERGE",
>> NULL, REF_NO_DEREF);
>> - fprintf(stderr,
>> - _("dropping %s %s -- patch contents already upstream\n"),
>> - oid_to_hex(&commit->object.oid), msg.subject);
>> + if (flags & AMEND_MSG) {
>> + /*
>> + * The "fixup!"/"squash!" emptied out the commit it was
>> + * melded into; that commit was already created by the
>> + * preceding "pick", so drop it by moving HEAD back to
>> + * its parent.
>> + */
>> + res = reset_head_to_parent(r, opts, &head);
>> + if (res)
>> + goto leave;
>> + fprintf(stderr,
>> + _("dropping %s %s -- resulting commit is empty\n"),
>> + oid_to_hex(&commit->object.oid), msg.subject);
>> + } else {
>> + fprintf(stderr,
>> + _("dropping %s %s -- patch contents already upstream\n"),
>> + oid_to_hex(&commit->object.oid), msg.subject);
>> + }
>> } /* else allow == 0 and there's nothing special to do */
>> if (!opts->no_commit && !drop_commit) {
>> if (author || command == TODO_REVERT || (flags & AMEND_MSG))
>> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
>> index 5033411a43..508dcc7527 100755
>> --- a/t/t3415-rebase-autosquash.sh
>> +++ b/t/t3415-rebase-autosquash.sh
>> @@ -510,4 +510,68 @@ test_expect_success 'pick and fixup respect commit.cleanup' '
>> test_commit_message HEAD -m "something"
>> '
>>
>> +test_expect_success 'fixup! that empties its target is dropped with --empty=drop' '
>> + git reset --hard base &&
>> + test_commit --no-tag addX fileX 1 &&
>> + test_commit --no-tag changeX fileX 2 &&
>> + test_commit --no-tag later fileW hello &&
>> + echo 1 >fileX &&
>> + git commit -m "fixup! changeX" fileX &&
>> +
>> + git rebase -i --autosquash --empty=drop HEAD~4 &&
>> +
>> + git log --format=%s >actual &&
>> + ! grep changeX actual &&
>> + grep addX actual &&
>> + grep later actual &&
>> + echo 1 >expect &&
>> + test_cmp expect fileX &&
>> + echo hello >expect &&
>> + test_cmp expect fileW
>> +'
>> +
>> +test_expect_success 'fixup! that empties its target is kept with --empty=keep' '
>> + git reset --hard base &&
>> + test_commit --no-tag addY fileY 1 &&
>> + test_commit --no-tag changeY fileY 2 &&
>> + echo 1 >fileY &&
>> + git commit -m "fixup! changeY" fileY &&
>> +
>> + git rebase -i --autosquash --empty=keep HEAD~3 &&
>> +
>> + git log --format=%s >actual &&
>> + grep changeY actual &&
>> + : "the retained commit is empty" &&
>> + git diff --exit-code HEAD~1 HEAD &&
>> + echo 1 >expect &&
>> + test_cmp expect fileY
>> +'
>> +
>> +test_expect_success 'fixup! that empties its target stops with --empty=stop' '
>> + git reset --hard base &&
>> + test_commit --no-tag addZ fileZ 1 &&
>> + test_commit --no-tag changeZ fileZ 2 &&
>> + echo 1 >fileZ &&
>> + git commit -m "fixup! changeZ" fileZ &&
>> +
>> + test_when_finished "git rebase --abort" &&
>> + test_must_fail git rebase -i --autosquash --empty=stop HEAD~3
>> +'
>> +
>> +test_expect_success 'squash! that empties its target is dropped with --empty=drop' '
>> + git reset --hard base &&
>> + test_commit --no-tag addS fileS 1 &&
>> + test_commit --no-tag changeS fileS 2 &&
>> + echo 1 >fileS &&
>> + git commit -m "squash! changeS" fileS &&
>> +
>> + git rebase -i --autosquash --empty=drop HEAD~3 &&
>> +
>> + git log --format=%s >actual &&
>> + ! grep changeS actual &&
>> + grep addS actual &&
>> + echo 1 >expect &&
>> + test_cmp expect fileS
>> +'
>> +
>> test_done
>>
>>
>>
>>
^ permalink raw reply
* [PATCH GSoC v16 13/13] cat-file: make remote-object-info allow-list dynamic
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@gmail.com>
The static allow-list in expand_atom() is hardcoded to only allow
"objectname" and "objectsize" for remote queries. This works because
up to this point all servers will either support object-info with name
and size or they do not support them at all, but we cannot expect that
in a future different servers with different git versions to have the
same object-info capabilities. Therefore, the allow_list needs to be
dynamic depending on what the server advertises.
The client will now:
1. Request the protocol option that the placeholder refers to (i.e.
"size" when "%(objectsize)").
2. Filters the request in fetch_object_info() dropping any option that
the server does not advertise.
3. After the fetching, the options that haven't been dropped are the ones
fetched and supported by the server, these supported options are
mapped and remote_allowed_atoms is populated with the placeholders.
4. expand_atom() checks remote_allowed_atoms with the same behaviour as
the static allow_list had.
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 there will always be an output even if
all the placeholders are not supported by the server (returns an empty
line).
Include "type" in the object_info_options so once the server supports
it, the clients know already how to request it.
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 | 97 +++++++++++++++++++++++++++++++++++------------------
fetch-object-info.c | 20 +++++++++++
2 files changed, 84 insertions(+), 33 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index af388b6238..20d9705a7f 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;
}
@@ -683,12 +675,12 @@ static int get_remote_info(struct batch_options *opt,
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;
/*
@@ -736,15 +728,12 @@ static int get_remote_info(struct batch_options *opt,
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");
-
- if (object_info_options.nr > 0) {
- gtransport->smart_options->object_info_options = &object_info_options;
+ if (object_info_options->nr > 0) {
+ 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;
}
@@ -830,6 +819,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)
@@ -839,6 +843,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;
if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
die(_("remote-object-info command too long"));
@@ -851,32 +856,57 @@ 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(opt, count, argv, &remote_object_info,
- &object_info_oids))
+ &object_info_oids, &object_info_options))
goto cleanup;
+ 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++) {
+ int found = 0;
data->oid = object_info_oids.oid[i];
+ /*
+ * 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");
+ found = 1;
}
+
+ if (remote_object_info[i].typep) {
+ data->type = *remote_object_info[i].typep;
+ found = 1;
+ }
+
+ if (!found && object_info_options.nr > 0) {
+ report_object_status(opt, oid_to_hex(&data->oid),
+ &data->oid, "missing");
+ continue;
+ }
+
+ 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;
cleanup:
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);
@@ -1192,6 +1222,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 91685cb355..e9b1011d07 100644
--- a/fetch-object-info.c
+++ b/fetch-object-info.c
@@ -55,6 +55,26 @@ 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
+ * beacuse 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);
+ /*
+ * If no options are left after the filtering, avoid unnecessary
+ * request to the server.
+ */
+ if (!args->object_info_options->nr)
+ return 0;
+
send_object_info_request(fd_out, args);
break;
case protocol_v1:
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v16 12/13] cat-file: validate remote atoms with an allow-list
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@gmail.com>
strstr() is not enough to validate the format placeholders in
remote-object-info causing two errors:
1. Atoms recognized by expand_atom() but the remote doesn't returns 1,
but data->type contains garbage causing segfault.
2. expand_atom() returns 0 for unknown atoms, calling
strbuf_expand_bad_format() which ends up dying, blocking local
queries if the same format is shared.
Add an allow-list with the supported atoms at the top of expand_atom().
In remote mode, unsupported atoms return 1 leaving the buffer empty,
honoring how for-each-ref handles known but inapplicable atoms.
As extra safety, initialize data->type to OBJ_BAD and add a NULL check
for type_name() so uninitialized data doesn't cause segfault.
Update tests that expect previous die() behavior to expect an empty
string and add an explicit test for empty string return on unknown
placeholder.
Update cat-file command documentation regarding remote-object-info.
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Documentation/git-cat-file.adoc | 2 +-
builtin/cat-file.c | 41 +++++++++++++++++++++++++++-------
t/t1017-cat-file-remote-object-info.sh | 27 ++++++++++++++++++----
3 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
index a7fa6674c3..643eac9245 100644
--- a/Documentation/git-cat-file.adoc
+++ b/Documentation/git-cat-file.adoc
@@ -451,7 +451,7 @@ CAVEATS
Note that since only `%(objectname)` and `%(objectsize)` are currently
supported by the `remote-object-info` command. Using any other placeholder in
-the format string will raise an error.
+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
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 77ecccdda3..af388b6238 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -333,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)
{
@@ -345,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;
@@ -709,10 +736,6 @@ static int get_remote_info(struct batch_options *opt,
CALLOC_ARRAY(*remote_object_info, object_info_oids->nr);
gtransport->smart_options->object_info_oids = object_info_oids;
- /* 'objectsize' is the only option currently supported */
- if (!strstr(opt->format, "%(objectsize)"))
- die(_("%s is currently not supported with remote-object-info"), opt->format);
-
string_list_append(&object_info_options, "size");
if (object_info_options.nr > 0) {
@@ -842,7 +865,9 @@ static void parse_cmd_remote_object_info(struct batch_options *opt,
*/
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");
}
diff --git a/t/t1017-cat-file-remote-object-info.sh b/t/t1017-cat-file-remote-object-info.sh
index 49b6660934..6bc863c391 100755
--- a/t/t1017-cat-file-remote-object-info.sh
+++ b/t/t1017-cat-file-remote-object-info.sh
@@ -236,6 +236,21 @@ test_expect_success 'remote-object-info does not die on missing oid like info' '
)
'
+# 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' '
@@ -575,10 +590,12 @@ test_expect_success 'remote-object-info fails on unsupported filter option (obje
set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
- test_must_fail git cat-file --batch-command="%(objectsize:disk)" 2>err <<-EOF &&
+ 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_grep "%(objectsize:disk) is currently not supported with remote-object-info" err
+ test_cmp expect actual
)
'
@@ -587,10 +604,12 @@ test_expect_success 'remote-object-info fails on unsupported filter option (delt
set_transport_variables "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
cd "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
- test_must_fail git cat-file --batch-command="%(deltabase)" 2>err <<-EOF &&
+ echo "" >expect &&
+
+ git cat-file --batch-command="%(deltabase)" >actual <<-EOF &&
remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
EOF
- test_grep "%(deltabase) is currently not supported with remote-object-info" err
+ test_cmp expect actual
)
'
--
2.54.0
^ permalink raw reply related
* [PATCH GSoC v16 11/13] cat-file: add remote-object-info to batch-command
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@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>
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>
---
Documentation/git-cat-file.adoc | 29 +-
builtin/cat-file.c | 144 ++++++-
object-file.c | 10 +
odb.h | 3 +
t/meson.build | 1 +
t/t1017-cat-file-remote-object-info.sh | 680 +++++++++++++++++++++++++++++++++
6 files changed, 859 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc
index 86b9181599..a7fa6674c3 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,15 +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
at the first whitespace boundary. All characters before that
@@ -340,8 +345,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 +449,10 @@ scripting purposes.
CAVEATS
-------
+Note that since only `%(objectname)` and `%(objectsize)` are currently
+supported by the `remote-object-info` command. Using any other placeholder in
+the format string will raise an error.
+
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 26ad07b62c..77ecccdda3 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,
@@ -636,6 +652,80 @@ static void batch_one_object(const char *obj_name,
object_context_release(&ctx);
}
+static int get_remote_info(struct batch_options *opt,
+ 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;
+
+ /*
+ * TODO: Change the format to "%(objectname) %(objectsize)" when
+ * remote-object-info command is used. Once we start supporting objecttype
+ * the default format should change to DEFAULT_FORMAT.
+ */
+ if (!opt->format)
+ opt->format = "%(objectname) %(objectsize)";
+
+ 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;
+
+ /* 'objectsize' is the only option currently supported */
+ if (!strstr(opt->format, "%(objectsize)"))
+ die(_("%s is currently not supported with remote-object-info"), opt->format);
+
+ string_list_append(&object_info_options, "size");
+
+ if (object_info_options.nr > 0) {
+ 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 +807,57 @@ 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;
+
+ if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
+ die(_("remote-object-info command too long"));
+
+ 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(opt, count, argv, &remote_object_info,
+ &object_info_oids))
+ goto cleanup;
+
+ 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].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;
+ batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
+ } else {
+ report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
+ }
+ }
+ data->skip_object_info = 0;
+
+cleanup:
+ 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,
@@ -748,8 +889,9 @@ static const struct parse_cmd {
} commands[] = {
{ "contents", parse_cmd_contents, 1 },
{ "info", parse_cmd_info, 1 },
- { "flush", NULL, 0 },
{ "mailmap", parse_cmd_mailmap, 1 },
+ { "remote-object-info", parse_cmd_remote_object_info, 1 },
+ { "flush", NULL, 0 },
};
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 3834a0dcbf..42e3934035 100644
--- a/odb.h
+++ b/odb.h
@@ -573,4 +573,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 7c3c070426..562923c257 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..49b6660934
--- /dev/null
+++ b/t/t1017-cat-file-remote-object-info.sh
@@ -0,0 +1,680 @@
+#!/bin/sh
+
+test_description='git cat-file --batch-command with remote-object-info command'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./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 --enable=receive-pack
+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_TRACE_PACKET=1 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 '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
+ )
+'
+
+# 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" &&
+
+ test_must_fail git cat-file --batch-command="%(objectsize:disk)" 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
+ EOF
+ test_grep "%(objectsize:disk) is currently not supported with remote-object-info" err
+ )
+'
+
+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" &&
+
+ test_must_fail git cat-file --batch-command="%(deltabase)" 2>err <<-EOF &&
+ remote-object-info "$HTTPD_URL/smart/http_parent" $hello_oid
+ EOF
+ test_grep "%(deltabase) is currently not supported with remote-object-info" err
+ )
+'
+
+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 v16 10/13] transport: add client support for object-info
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon, Calvin Wan
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@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 object IDs with
the size feature 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 exit.
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. When the server does not recognize an OID, following the v2 protocol,
the server returns "<OID> SP", when this happens,
fetch_object_info() sets the corresponding size pointer to NULL so
that callers can detect and handle it.
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 | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++
fetch-object-info.h | 22 +++++++++++
fetch-pack.h | 1 +
meson.build | 1 +
transport-helper.c | 13 +++++-
transport-internal.h | 8 ++++
transport.c | 46 ++++++++++++++++++++++
transport.h | 10 +++++
9 files changed, 209 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 1f3f099f5c..cdabdb3771 100644
--- a/Makefile
+++ b/Makefile
@@ -1159,6 +1159,7 @@ LIB_OBJS += ewah/ewah_rlw.o
LIB_OBJS += exec-cmd.o
LIB_OBJS += fetch-negotiator.o
LIB_OBJS += fetch-pack.o
+LIB_OBJS += fetch-object-info.o
LIB_OBJS += fmt-merge-msg.o
LIB_OBJS += fsck.o
LIB_OBJS += fsmonitor.o
diff --git a/fetch-object-info.c b/fetch-object-info.c
new file mode 100644
index 0000000000..91685cb355
--- /dev/null
+++ b/fetch-object-info.c
@@ -0,0 +1,109 @@
+#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
+ 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 size_t 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(_("unsupported protocol version. expected 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")) {
+ size_index = 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 (size_index >= 0) {
+ if (!strcmp(object_info_values.items[1 + size_index].string, "")) {
+ FREE_AND_NULL(object_info_data[i].sizep);
+ string_list_clear(&object_info_values, 0);
+ continue;
+ }
+
+ if (parse_object_size(object_info_values.items[1 + size_index].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[1 + size_index].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..d35284bd6b
--- /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"
+#include "odb.h"
+
+struct object_info_args {
+ struct string_list *object_info_options;
+ const struct string_list *server_options;
+ struct oid_array *oids;
+};
+
+/*
+ * 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/fetch-pack.h b/fetch-pack.h
index 6d0dec7f41..0fba340a84 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -16,6 +16,7 @@ struct fetch_pack_args {
const struct string_list *deepen_not;
struct list_objects_filter_options filter_options;
const struct string_list *server_options;
+ struct object_info *object_info_data;
/*
* If not NULL, during packfile negotiation, fetch-pack will send "have"
diff --git a/meson.build b/meson.build
index ca235801cf..19fad57da9 100644
--- a/meson.build
+++ b/meson.build
@@ -347,6 +347,7 @@ libgit_sources = [
'exec-cmd.c',
'fetch-negotiator.c',
'fetch-pack.c',
+ 'fetch-object-info.c',
'fmt-merge-msg.c',
'fsck.c',
'fsmonitor.c',
diff --git a/transport-helper.c b/transport-helper.c
index f195070788..f97e6d7b29 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -727,8 +727,7 @@ static int fetch_refs(struct transport *transport,
/*
* If we reach here, then the server, the client, and/or the transport
- * helper does not support protocol v2. --negotiate-only requires
- * protocol v2.
+ * helper does not support protocol v2. --negotiate-only.
*/
if (data->transport_options.acked_commits) {
warning(_("--negotiate-only requires protocol v2"));
@@ -784,6 +783,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 +1338,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..3e0a6558b7 100644
--- a/transport.c
+++ b/transport.c
@@ -1,3 +1,4 @@
+#include "compat/posix.h"
#define USE_THE_REPOSITORY_VARIABLE
#include "git-compat-util.h"
@@ -9,6 +10,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 +434,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 +1048,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 +1214,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..9e85a4cd35 100644
--- a/transport.h
+++ b/transport.h
@@ -6,6 +6,7 @@
#include "list-objects-filter-options.h"
#include "string-list.h"
#include "connect.h"
+#include "odb.h"
struct git_transport_options {
unsigned thin : 1;
@@ -55,6 +56,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 +314,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 v16 09/13] serve: advertise object-info feature
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon, Calvin Wan
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@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.
While at it, 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.
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>
---
Documentation/gitprotocol-v2.adoc | 11 ++++++++---
serve.c | 5 ++++-
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index 2beb70595f..d3530c52ea 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 object IDs.
The response of `object-info` is a list of the requested object ids
and associated requested information, each separated by a single space.
output = info flush-pkt
- info = PKT-LINE(attrs) LF)
+ info = PKT-LINE(attrs LF)
*PKT-LINE(obj-info LF)
attrs = attr | attrs SP attrs
+ obj-size = 1*DIGIT
+
attr = "size"
- obj-info = obj-id SP obj-size
+ obj-info = obj-id SP [obj-size]
+
+ If the server does not recognize the object id, the response will be
+ `obj-id SP` regardless of the number of attributes requested.
bundle-uri
~~~~~~~~~~
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 v16 08/13] fetch-pack: move fetch initialization
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon, Calvin Wan
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@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 783e3474a6..2ab5ba108b 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 v16 07/13] connect: make write_fetch_command_and_capabilities() more generic
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@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 https://git-scm.com/docs/protocol-v2, such as a Git
subcommand (e.g., git-fetch(1)) or a server-side operation like
"object-info" as implemented in commit a2ba162
(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 c09947cc56..127ed4a2e6 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..c2bf492ed9 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);
+/*
+ * Writes a command along with the requested server capabilities/features into a
+ * request buffer.
+ */
+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 5e7c4f1d46..783e3474a6 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 v16 06/13] fetch-pack: move write_fetch_command_and_capabilities() to connect.c
From: Pablo Sabater @ 2026-07-10 16:41 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, chandrapratap3519, chriscool, eric.peijian,
gitster, jltobler, karthik.188, peff, toon
In-Reply-To: <20260710-ps-eric-work-rebase-v16-0-66e07b58a8fe@gmail.com>
write_fetch_command_and_capabilities() is refactored in a subsequent
commit where it becomes 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..c09947cc56 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)) {
+ 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);
+ 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 8e04db8640..5e7c4f1d46 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)) {
- 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);
- 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
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