* [PATCH v2 1/3] daemon: fix IPv6 address corruption in lookup_hostname()
From: Sebastien Tardif via GitGitGadget @ 2026-05-27 18:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Sebastien Tardif, Sebastien Tardif
In-Reply-To: <pull.2300.v2.git.git.1779905911.gitgitgadget@gmail.com>
From: Sebastien Tardif <sebtardif@ncf.ca>
getaddrinfo() is called with AF_UNSPEC hints, so it may return IPv6
results. However, the code unconditionally casts ai_addr to
sockaddr_in and passes AF_INET to inet_ntop(). On IPv6-only hosts,
this reads from the wrong struct offset, producing garbage IP
addresses.
Fix this by checking ai_family and extracting the address pointer
into a local variable before calling inet_ntop() once with the
correct family. Die on unexpected address families.
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
daemon.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/daemon.c b/daemon.c
index 0a7b1aae44..80fa0226d8 100644
--- a/daemon.c
+++ b/daemon.c
@@ -674,9 +674,20 @@ static void lookup_hostname(struct hostinfo *hi)
gai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);
if (!gai) {
- struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
+ void *addr;
+
+ if (ai->ai_family == AF_INET) {
+ struct sockaddr_in *sa = (void *)ai->ai_addr;
+ addr = &sa->sin_addr;
+ } else if (ai->ai_family == AF_INET6) {
+ struct sockaddr_in6 *sa6 = (void *)ai->ai_addr;
+ addr = &sa6->sin6_addr;
+ } else {
+ die("unexpected address family: %d",
+ ai->ai_family);
+ }
- inet_ntop(AF_INET, &sin_addr->sin_addr,
+ inet_ntop(ai->ai_family, addr,
addrbuf, sizeof(addrbuf));
strbuf_addstr(&hi->ip_address, addrbuf);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 0/3] daemon: fix network address handling bugs
From: Sebastien Tardif via GitGitGadget @ 2026-05-27 18:18 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Sebastien Tardif
In-Reply-To: <pull.2300.git.git.1778773592.gitgitgadget@gmail.com>
Fix three related issues in daemon.c's network address handling:
IPv6 address corruption in lookup_hostname(): getaddrinfo() is called with
AF_UNSPEC hints, so it may return IPv6 results. However, the code
unconditionally casts ai_addr to sockaddr_in and passes AF_INET to
inet_ntop(). On IPv6-only hosts, this reads from the wrong struct offset,
producing garbage IP addresses. Fixed by checking ai_family and handling
both AF_INET and AF_INET6.
IPv6 address truncation in ip2str(): The sockaddr struct size (ai_addrlen)
is passed as the output buffer size to inet_ntop(). For IPv6,
sizeof(sockaddr_in6) is 28 bytes but INET6_ADDRSTRLEN is 46, so long IPv6
addresses are silently truncated. Fixed by passing sizeof(ip) instead, and
dropping the now-unused len parameter.
NULL pointer in execute() logging: REMOTE_PORT environment variable is used
in a format string without a NULL check (only REMOTE_ADDR was checked). If
REMOTE_PORT is unset, NULL is passed to printf's %s, which is undefined
behavior. Fixed by using a fallback string.
Changes since v1:
* Split the single patch into three separate commits, one per fix, per
Patrick's review.
* Deduplicated the address family handling in lookup_hostname(): instead of
duplicating the inet_ntop() call for each family, the address pointer is
extracted into a local void *addr variable first, then inet_ntop() is
called once, per Patrick's suggestion.
* The (void *) intermediate cast on ai_addr is used intentionally: C
guarantees any object pointer round-trips safely through void *, and it
keeps the per-family blocks shorter than spelling out the full struct
casts.
* For the REMOTE_PORT NULL guard: both REMOTE_ADDR and REMOTE_PORT are set
by the same code path in handle(), so neither should be NULL
independently. The guard makes the code consistent with the existing
REMOTE_ADDR check and avoids undefined behavior from printf %s with a
NULL argument.
* Die on unexpected address families in lookup_hostname() rather than
silently leaving addrbuf uninitialized.
Sebastien Tardif (3):
daemon: fix IPv6 address corruption in lookup_hostname()
daemon: fix IPv6 address truncation in ip2str()
daemon: guard NULL REMOTE_PORT in execute() logging
daemon.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2300%2FSebTardif%2Ffix%2Fdaemon-ipv6-and-null-port-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2300/SebTardif/fix/daemon-ipv6-and-null-port-v2
Pull-Request: https://github.com/git/git/pull/2300
Range-diff vs v1:
1: b2d8143811 = 1: b2d8143811 daemon: fix IPv6 address corruption in lookup_hostname()
2: 5c01ec3cad = 2: 5c01ec3cad daemon: fix IPv6 address truncation in ip2str()
3: 1b2f9d1a07 ! 3: e312735716 daemon: guard NULL REMOTE_PORT in execute() logging
@@ Metadata
## Commit message ##
daemon: guard NULL REMOTE_PORT in execute() logging
- The REMOTE_PORT environment variable is used in a format string
- without a NULL check, while REMOTE_ADDR is checked. If REMOTE_PORT
- is unset, NULL is passed to printf's %s, which is undefined behavior.
+ REMOTE_ADDR and REMOTE_PORT are both set by the same code path in
+ handle(), so neither should be NULL independently. However, the
+ existing code checks REMOTE_ADDR before the loginfo() call but not
+ REMOTE_PORT. If REMOTE_PORT were unset, NULL would be passed to
+ printf's %s, which is undefined behavior.
- Add a fallback string for the NULL case.
+ Add a fallback string for the NULL case, matching the existing
+ REMOTE_ADDR guard for consistency.
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
--
gitgitgadget
^ permalink raw reply
* [PATCH] pkt-line: initialize packet_buffer to avoid macOS linker warning
From: Harald Nordgren via GitGitGadget @ 2026-05-27 17:11 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
From: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
pkt-line: initialize packet_buffer to avoid macOS linker warning
Removes this warning:
$ make -s -j8
GIT_VERSION=2.54.0.380.gc69baaf57b
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
ld: warning: reducing alignment of section __DATA,__common from 0x8000 to 0x4000 because it exceeds segment maximum alignment
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2313%2FHaraldNordgren%2Fpkt-line-init-buffer-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2313/HaraldNordgren/pkt-line-init-buffer-v1
Pull-Request: https://github.com/git/git/pull/2313
pkt-line.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pkt-line.c b/pkt-line.c
index 3fc3e9ea70..cfd2799677 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -8,7 +8,7 @@
#include "trace.h"
#include "write-or-die.h"
-char packet_buffer[LARGE_PACKET_MAX];
+char packet_buffer[LARGE_PACKET_MAX] = {0};
static const char *packet_trace_prefix = "git";
static struct trace_key trace_packet = TRACE_KEY_INIT(PACKET);
static struct trace_key trace_pack = TRACE_KEY_INIT(PACKFILE);
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 6/8] pack-bitmap: sort bitmaps before XORing
From: Taylor Blau @ 2026-05-27 16:56 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <20260527100406.GG981444@coredump.intra.peff.net>
On Wed, May 27, 2026 at 06:04:06AM -0400, Jeff King wrote:
> On Tue, May 19, 2026 at 12:12:50PM -0400, Taylor Blau wrote:
>
> > Reachability bitmaps may be stored as XORs against nearby bitmaps, up to
> > 10 away. However, when callers provide selected commits in an arbitrary
> > order, the writer may miss good ancestor/descendant pairs and produce
> > much larger bitmap files without changing query coverage.
> >
> > Sort the selected bitmaps in date order (from oldest to newest) before
> > computing XOR offsets, leaving pseudo-merge bitmaps alone (which we will
> > deal with separately in following commits).
>
> That order certainly makes the most sense. I'd have thought we ended up
> there incidentally because of the order in which we consider the
> commits, but perhaps not. I wonder if this got much worse when we
> re-wrote the bitmap generation code a few years ago.
>
> That was in v2.31.0, I think. Repacking linux.git with bitmaps, though,
> I couldn't find any difference in size between v2.30 and v2.31. They're
> both ~67M. But that also didn't shrink with this patch, either.
>
> If you have some spare CPU cycles to burn, I would be interested in a
> comparison of the bitmap size of your test repo using v2.30.0, v2.31.1,
> and this patch.
I started running this experiment, but I don't think I actually have
enough CPU cycles to let it finish ;-). Pre-v2.31 bitmap generation is
*really* slow[^1], and after multiple hours (forcing the same selection
of bitmaps by back-porting and adjusting 'test-tool bitmap') I couldn't
seem to make any meaningful progress.
I'm sure that you could get some plausible numbers out of benchmarking
this on a smaller repository. In case you're interested, here's the
patch I wrote on top of v2.30.0:
--- 8< ---
diff --git a/Makefile b/Makefile
index 7b64106930a..9ce9f2b483c 100644
--- a/Makefile
+++ b/Makefile
@@ -690,6 +690,7 @@ X =
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
TEST_BUILTINS_OBJS += test-advise.o
+TEST_BUILTINS_OBJS += test-bitmap.o
TEST_BUILTINS_OBJS += test-bloom.o
TEST_BUILTINS_OBJS += test-chmtime.o
TEST_BUILTINS_OBJS += test-config.o
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 5e998bdaa79..55dbb475120 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -113,7 +113,7 @@ void bitmap_writer_build_type_index(struct packing_data *to_pack,
static struct object **seen_objects;
static unsigned int seen_objects_nr, seen_objects_alloc;
-static inline void push_bitmapped_commit(struct commit *commit, struct ewah_bitmap *reused)
+void bitmap_writer_push_commit(struct commit *commit, struct ewah_bitmap *reused)
{
if (writer.selected_nr >= writer.selected_alloc) {
writer.selected_alloc = (writer.selected_alloc + 32) * 2;
@@ -402,7 +402,7 @@ void bitmap_writer_select_commits(struct commit **indexed_commits,
if (indexed_commits_nr < 100) {
for (i = 0; i < indexed_commits_nr; ++i)
- push_bitmapped_commit(indexed_commits[i], NULL);
+ bitmap_writer_push_commit(indexed_commits[i], NULL);
return;
}
@@ -440,7 +440,7 @@ void bitmap_writer_select_commits(struct commit **indexed_commits,
}
}
- push_bitmapped_commit(chosen, reused_bitmap);
+ bitmap_writer_push_commit(chosen, reused_bitmap);
i += next + 1;
display_progress(writer.progress, i);
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 1203120c432..a882efdb16a 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -74,6 +74,8 @@ void bitmap_writer_build_type_index(struct packing_data *to_pack,
struct pack_idx_entry **index,
uint32_t index_nr);
void bitmap_writer_reuse_bitmaps(struct packing_data *to_pack);
+void bitmap_writer_push_commit(struct commit *commit,
+ struct ewah_bitmap *reused);
void bitmap_writer_select_commits(struct commit **indexed_commits,
unsigned int indexed_commits_nr, int max_bitmaps);
void bitmap_writer_build(struct packing_data *to_pack);
diff --git a/t/helper/test-bitmap.c b/t/helper/test-bitmap.c
new file mode 100644
index 00000000000..9be03377c06
--- /dev/null
+++ b/t/helper/test-bitmap.c
@@ -0,0 +1,119 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
+#include "test-tool.h"
+#include "git-compat-util.h"
+#include "commit.h"
+#include "packfile.h"
+#include "pack-bitmap.h"
+
+static int add_packed_object(const struct object_id *oid,
+ struct packed_git *pack,
+ uint32_t pos,
+ void *_data)
+{
+ struct packing_data *packed = _data;
+ struct object_entry *entry;
+ struct object_info oi = OBJECT_INFO_INIT;
+ enum object_type type;
+
+ oi.typep = &type;
+
+ entry = packlist_alloc(packed, oid);
+ entry->in_pack_offset = nth_packed_object_offset(pack, pos);
+ entry->idx.offset = entry->in_pack_offset;
+ if (packed_object_info(the_repository, pack, entry->in_pack_offset, &oi) < 0)
+ die("could not get type of object %s",
+ oid_to_hex(oid));
+ oe_set_type(entry, type);
+ oe_set_in_pack(packed, entry, pack);
+
+ return 0;
+}
+
+static int idx_oid_cmp(const void *va, const void *vb)
+{
+ const struct pack_idx_entry *a = *(const struct pack_idx_entry **)va;
+ const struct pack_idx_entry *b = *(const struct pack_idx_entry **)vb;
+
+ return oidcmp(&a->oid, &b->oid);
+}
+
+static int bitmap_write(const char *basename)
+{
+ struct packed_git *p = NULL;
+ struct packing_data packed = { 0 };
+ struct pack_idx_entry **index;
+ struct strbuf buf = STRBUF_INIT;
+ uint32_t i;
+
+ prepare_repo_settings(the_repository);
+ for (p = get_all_packs(the_repository); p; p = p->next) {
+ if (!strcmp(pack_basename(p), basename))
+ break;
+ }
+
+ if (!p)
+ die("could not find pack '%s'", basename);
+
+ if (open_pack_index(p))
+ die("cannot open pack index for '%s'", p->pack_name);
+
+ prepare_packing_data(the_repository, &packed);
+
+ for_each_object_in_pack(p, add_packed_object, &packed,
+ FOR_EACH_OBJECT_PACK_ORDER);
+
+ /*
+ * Build the index array now that data.packed.objects[] is
+ * fully allocated (packlist_alloc() may have reallocated it
+ * during the loop above).
+ */
+ ALLOC_ARRAY(index, p->num_objects);
+ for (i = 0; i < p->num_objects; i++)
+ index[i] = &packed.objects[i].idx;
+
+ bitmap_writer_build_type_index(&packed, index, p->num_objects);
+
+ while (strbuf_getline_lf(&buf, stdin) != EOF) {
+ struct object_id oid;
+ struct commit *c;
+
+ if (get_oid_hex(buf.buf, &oid))
+ die("invalid OID: %s", buf.buf);
+
+ c = lookup_commit(the_repository, &oid);
+ if (!c || repo_parse_commit(the_repository, c))
+ die("could not parse commit %s", buf.buf);
+
+ bitmap_writer_push_commit(c, NULL);
+ }
+
+ bitmap_writer_build(&packed);
+
+ bitmap_writer_set_checksum(p->hash);
+
+ QSORT(index, p->num_objects, idx_oid_cmp);
+
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, p->pack_name);
+ strbuf_strip_suffix(&buf, ".pack");
+ strbuf_addstr(&buf, ".bitmap");
+ bitmap_writer_finish(index, p->num_objects, buf.buf, 0);
+
+ strbuf_release(&buf);
+ free(index);
+
+ return 0;
+}
+
+int cmd__bitmap(int argc, const char **argv)
+{
+ setup_git_directory();
+
+ if (argc == 3 && !strcmp(argv[1], "write"))
+ return bitmap_write(argv[2]);
+
+ usage("\ttest-tool bitmap write <pack-basename> < <commit-list>");
+
+ return -1;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 9d6d14d9293..c43d8c0977b 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -15,6 +15,7 @@ struct test_cmd {
static struct test_cmd cmds[] = {
{ "advise", cmd__advise_if_enabled },
+ { "bitmap", cmd__bitmap },
{ "bloom", cmd__bloom },
{ "chmtime", cmd__chmtime },
{ "config", cmd__config },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index a6470ff62c4..27e6e40ffcb 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -5,6 +5,7 @@
#include "git-compat-util.h"
int cmd__advise_if_enabled(int argc, const char **argv);
+int cmd__bitmap(int argc, const char **argv);
int cmd__bloom(int argc, const char **argv);
int cmd__chmtime(int argc, const char **argv);
int cmd__config(int argc, const char **argv);
--- >8 ---
> > On our same testing repository from previous commits, this change shrunk
> > our selection of 1,261 bitmaps from ~635.46 MiB to 176.4 MiB for a
> > ~72.24% reduction in the on-disk size of our *.bitmap file. The time to
> > generate the smaller bitmap file decreased by ~3.69 seconds, though this
> > is likely mostly noise.
>
> Certainly good numbers. The obvious follow-up question is: how does the
> reading side fare? I'd expect it to be a little better, if only because
> there are fewer bytes to consider when XOR-ing. But if there's some
> hidden assumption we're missing, then it could get wildly worse. It
> would be good to confirm that that didn't happen. ;)
It doesn't make a huge difference. Prior to this patch, the timings on
my test repository for 'git rev-list --count --all --objects
--use-bitmap-index' go from:
- 2.180 ± 0.019 seconds (with pseudo-merges)
- 52.149 ± 0.224 seconds (without pseudo-merges)
, and after applying this patch, it changes to:
- 2.611 ± 0.023 seconds (with pseudo-merges)
- 51.963 ± 0.131 seconds (without pseudo-merges)
It looks like there is a minor slow-down on pseudo-merges, and a minor
speed-up without them. The difference is small enough that I'm willing
to treat it as run-to-run noise.
> > static void compute_xor_offsets(struct bitmap_writer *writer)
> > {
> > static const int MAX_XOR_OFFSET_SEARCH = 10;
> >
> > int i, next = 0;
> > + int nr = bitmap_writer_nr_selected_commits(writer);
> > +
> > + if (nr > 1) {
> > + QSORT(writer->selected, nr, bitmapped_commit_date_cmp);
> > +
> > + for (i = 0; i < nr; i++) {
> > + struct bitmapped_commit *stored = &writer->selected[i];
> > + khiter_t hash_pos = kh_get_oid_map(writer->bitmaps,
> > + stored->commit->object.oid);
> > +
> > + if (hash_pos == kh_end(writer->bitmaps))
> > + BUG("selected commit missing from bitmap map: %s",
> > + oid_to_hex(&stored->commit->object.oid));
> > +
> > + kh_value(writer->bitmaps, hash_pos) = stored;
> > + }
> > + }
>
> OK. It took me a minute to wrap my head around this. The real work is
> done by QSORT(). But because we maintain a hash pointing into that
> array, we have to go through each hash entry and fix up its pointer.
Yup.
> Looks correct.
Thanks,
Taylor
[^1]: ...and I have great empathy for Stolee's suffering here when
benchmarking his performance improvements to the bitmap generation
code from back in the day! ;-).
^ permalink raw reply related
* What's cooking in git.git (May 2026, #08)
From: Junio C Hamano @ 2026-05-27 16:06 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and is a candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with a URL
to a message that raises issues but they are by no means exhaustive.
A topic without enough support may be discarded after a long period
of no activity (of course they can be resubmitted when new interests
arise).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-scm/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* ds/fetch-negotiation-options (2026-05-19) 8 commits
(merged to 'next' on 2026-05-21 at ff57fd9c97)
+ send-pack: pass negotiation config in push
+ remote: add remote.*.negotiationInclude config
+ fetch: add --negotiation-include option for negotiation
+ negotiator: add have_sent() interface
+ remote: add remote.*.negotiationRestrict config
+ transport: rename negotiation_tips
+ fetch: add --negotiation-restrict option
+ t5516: fix test order flakiness
The negotiation tip options in "git fetch" have been reworked to
allow requiring certain refs to be sent as "have" lines, and to
restrict negotiation to a specific set of refs.
source: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
* en/batch-prefetch (2026-05-14) 4 commits
(merged to 'next' on 2026-05-20 at 722acf81c8)
+ grep: prefetch necessary blobs
+ builtin/log: prefetch necessary blobs for `git cherry`
+ patch-ids.h: add missing trailing parenthesis in documentation comment
+ promisor-remote: document caller filtering contract
In a lazy clone, "git cherry" and "git grep" often fetch necessary
blob objects one by one from promisor remotes. It has been corrected
to collect necessary object names and fetch them in bulk to gain
reasonable performance.
cf. <0da4f159-8d4b-49e2-93c1-25aa0bf69371@gmail.com>
source: <pull.2089.v3.git.1778775928.gitgitgadget@gmail.com>
* jk/sq-dequote-cleanup (2026-05-18) 3 commits
(merged to 'next' on 2026-05-21 at fbedf2daea)
+ quote: simplify internals of dequoting
+ quote: drop sq_dequote_to_argv()
+ quote.h: bump strvec forward declaration to the top
Code simplification.
source: <20260519011837.GA1615637@coredump.intra.peff.net>
* jt/odb-transaction-write (2026-05-14) 7 commits
(merged to 'next' on 2026-05-21 at 61108abe4d)
+ odb/transaction: make `write_object_stream()` pluggable
+ object-file: generalize packfile writes to use odb_write_stream
+ object-file: avoid fd seekback by checking object size upfront
+ object-file: remove flags from transaction packfile writes
+ odb: update `struct odb_write_stream` read() callback
+ odb/transaction: use pluggable `begin_transaction()`
+ odb: split `struct odb_transaction` into separate header
(this branch is used by ps/odb-in-memory and ps/odb-source-loose.)
ODB transaction interface is being reworked to explicitly handle
object writes.
source: <20260514183740.1505171-1-jltobler@gmail.com>
* kk/limit-list-optim (2026-05-14) 1 commit
(merged to 'next' on 2026-05-19 at f17450dd1b)
+ revision: use priority queue in limit_list()
The limit_list() function that is one of the core part of the
revision traversal infrastructure has been optimized by replacing
its use of linear list with priority queue.
source: <pull.2114.git.1778777491939.gitgitgadget@gmail.com>
* kk/merge-octopus-optim (2026-05-11) 1 commit
(merged to 'next' on 2026-05-20 at afe427dc66)
+ merge: use repo_in_merge_bases for octopus up-to-date check
The logic to determine that branches in an octopus merge are
independent has been optimized.
cf. <c5b333f1-0db6-4aec-a369-6503cb924e7f@gmail.com>
source: <pull.2110.git.1778566286543.gitgitgadget@gmail.com>
* kn/refs-fsck-skip-lock-files (2026-05-17) 1 commit
(merged to 'next' on 2026-05-21 at 91e30e3543)
+ refs/files: skip lock files during consistency checks
The consistency checks for the files reference backend have been updated
to skip lock files earlier, avoiding unnecessary parsing of
intermediate files.
source: <20260517-refs-fsck-skip-lock-files-v3-1-b24dfd673c7e@gmail.com>
* pb/doc-diff-format-updates (2026-05-15) 3 commits
(merged to 'next' on 2026-05-20 at fe8d31e9f9)
+ diff-format.adoc: mode and hash are 0* for unmerged paths from index only
+ diff-format.adoc: 'git diff-files' prints two lines for unmerged files
+ diff-format.adoc: remove mention of diff-tree specific output
Doc updates.
source: <pull.2304.git.git.1778860091.gitgitgadget@gmail.com>
* ps/odb-in-memory (2026-04-10) 18 commits
(merged to 'next' on 2026-05-21 at c8709aa17f)
+ t/unit-tests: add tests for the in-memory object source
+ odb: generic in-memory source
+ odb/source-inmemory: stub out remaining functions
+ odb/source-inmemory: implement `freshen_object()` callback
+ odb/source-inmemory: implement `count_objects()` callback
+ odb/source-inmemory: implement `find_abbrev_len()` callback
+ odb/source-inmemory: implement `for_each_object()` callback
+ odb/source-inmemory: convert to use oidtree
+ oidtree: add ability to store data
+ cbtree: allow using arbitrary wrapper structures for nodes
+ odb/source-inmemory: implement `write_object_stream()` callback
+ odb/source-inmemory: implement `write_object()` callback
+ odb/source-inmemory: implement `read_object_stream()` callback
+ odb/source-inmemory: implement `read_object_info()` callback
+ odb: fix unnecessary call to `find_cached_object()`
+ odb/source-inmemory: implement `free()` callback
+ odb: introduce "in-memory" source
+ Merge branch 'jt/odb-transaction-write' into ps/odb-in-memory
(this branch is used by ps/odb-source-loose; uses jt/odb-transaction-write.)
Add a new odb "in-memory" source that is meant to only hold
tentative objects (like the virtual blob object that represents the
working tree file used by "git blame").
source: <20260410-b4-pks-odb-source-inmemory-v3-0-22fd0fad58fe@pks.im>
* ps/setup-wo-the-repository (2026-05-19) 18 commits
(merged to 'next' on 2026-05-21 at d8fb5a7b3e)
+ setup: stop using `the_repository` in `init_db()`
+ setup: stop using `the_repository` in `create_reference_database()`
+ setup: stop using `the_repository` in `initialize_repository_version()`
+ setup: stop using `the_repository` in `check_repository_format()`
+ setup: stop using `the_repository` in `upgrade_repository_format()`
+ setup: stop using `the_repository` in `setup_git_directory()`
+ setup: stop using `the_repository` in `setup_git_directory_gently()`
+ setup: stop using `the_repository` in `setup_git_env()`
+ setup: stop using `the_repository` in `set_git_work_tree()`
+ setup: stop using `the_repository` in `setup_work_tree()`
+ setup: stop using `the_repository` in `enter_repo()`
+ setup: stop using `the_repository` in `verify_non_filename()`
+ setup: stop using `the_repository` in `verify_filename()`
+ setup: stop using `the_repository` in `path_inside_repo()`
+ setup: stop using `the_repository` in `prefix_path()`
+ setup: stop using `the_repository` in `is_inside_work_tree()`
+ setup: stop using `the_repository` in `is_inside_git_dir()`
+ setup: replace use of `the_repository` in static functions
(this branch is used by ps/setup-centralize-odb-creation.)
Many uses of the_repository has been updated to use a more
appropriate struct repository instance in setup.c codepath.
source: <20260519-pks-setup-wo-the-repository-v3-0-a00d8ea8b07f@pks.im>
* ps/t3903-cover-stash-include-untracked (2026-05-16) 1 commit
(merged to 'next' on 2026-05-20 at f1e7ac1cbd)
+ stash: add coverage for show --include-untracked
Test coverage has been added to "git stash --include-untracked".
source: <20260516183347.4323-2-pushkarkumarsingh1970@gmail.com>
* rs/trailer-fold-optim (2026-05-15) 1 commit
(merged to 'next' on 2026-05-20 at 38c9fb15c2)
+ trailer: change strbuf in-place in unfold_value()
Code simplification.
source: <816be07e-2cd6-48fe-ae93-57fa0f2543ed@web.de>
* rs/use-builtin-add-overflow-explicitly-on-clang (2026-05-18) 2 commits
(merged to 'next' on 2026-05-21 at c223b71079)
+ use __builtin_add_overflow() in st_add() with Clang
+ strbuf: use st_add3() in strbuf_grow()
Micro optimization of codepaths that compute allocation sizes carefully.
source: <20260518202502.25682-1-l.s.r@web.de>
* tb/incremental-midx-part-3.3 (2026-05-19) 16 commits
(merged to 'next' on 2026-05-21 at 6c11c1a739)
+ repack: allow `--write-midx=incremental` without `--geometric`
+ repack: introduce `--write-midx=incremental`
+ repack: implement incremental MIDX repacking
+ packfile: ensure `close_pack_revindex()` frees in-memory revindex
+ builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK`
+ repack-geometry: prepare for incremental MIDX repacking
+ repack-midx: extract `repack_fill_midx_stdin_packs()`
+ repack-midx: factor out `repack_prepare_midx_command()`
+ midx: expose `midx_layer_contains_pack()`
+ repack: track the ODB source via existing_packs
+ midx: support custom `--base` for incremental MIDX writes
+ midx: introduce `--no-write-chain-file` for incremental MIDX writes
+ midx: use `strvec` for `keep_hashes`
+ midx: build `keep_hashes` array in order
+ midx: use `strset` for retained MIDX files
+ midx-write: handle noop writes when converting incremental chains
The repacking code has been refactored and compaction of MIDX layers
have been implemented, and incremental strategy that does not require
all-into-one repacking has been introduced.
source: <cover.1779206239.git.me@ttaylorr.com>
* tb/pseudo-merge-bugfixes (2026-05-11) 9 commits
(merged to 'next' on 2026-05-19 at ecee155d5c)
+ pack-bitmap: prevent pattern leak on pseudo-merge re-assignment
+ Documentation: fix broken `sampleRate` in gitpacking(7)
+ pack-bitmap: reject pseudo-merge "sampleRate" of 0
+ pack-bitmap: parse commits in `find_pseudo_merge_group_for_ref()`
+ pack-bitmap: fix pseudo-merge lookup for shared commits
+ pack-bitmap: fix inverted binary search in `pseudo_merge_at()`
+ pack-bitmap-write: sort pseudo-merge commit lookup table in pack order
+ t5333: demonstrate various pseudo-merge bugs
+ t/helper: add 'test-tool bitmap write' subcommand
(this branch is used by tb/bitmap-build-performance.)
Fixes many bugs in pseudo-merge code.
source: <cover.1778546804.git.me@ttaylorr.com>
--------------------------------------------------
[New Topics]
* za/completion-hide-dotfiles (2026-05-26) 1 commit
- completion: hide dotfiles for selected path completion
The path completion for commands like `git rm` and `git mv`, is being
updated to hide dotfiles by default, unless the user explicitly starts
the path with a dot, matching standard shell-completion behavior.
Will merge to 'next'?
source: <pull.2311.v2.git.git.1779808987825.gitgitgadget@gmail.com>
--------------------------------------------------
[Cooking]
* kk/fetch-store-ref-optimization (2026-05-24) 1 commit
- fetch: pass transport to post-fetch connectivity check
When fetching from a transport that provides a self-contained pack,
pass the transport pointer to the post-fetch `check_connected()` call
to optimize connectivity check.
Will merge to 'next'?
source: <pull.2123.git.1779625693328.gitgitgadget@gmail.com>
* ds/restore-sparse-index (2026-05-26) 2 commits
- restore: avoid sparse index expansion
- t1092: test 'git restore' with sparse index
'git restore --staged' has been optimized to avoid unnecessarily expanding
the sparse index when operating on paths within the sparse checkout
definition, by handling sparse directory entries at the tree level.
Will merge to 'next'.
source: <pull.2121.v2.git.1779827195.gitgitgadget@gmail.com>
* kk/commit-reach-optim (2026-05-25) 3 commits
- commit-reach: replace queue_has_nonstale() scan with O(1) tracking
- commit-reach: deduplicate queue entries in paint_down_to_common
- object.h: fix stale entries in object flag allocation table
The check for non-stale commits in the priority queue used by
`paint_down_to_common` and `ahead_behind` has been optimized by
replacing an O(N) scan with an O(1) counter, yielding performance
improvements in repositories with wide histories.
Will merge to 'next'?
source: <pull.2124.v2.git.1779719286.gitgitgadget@gmail.com>
* ar/receive-pack-worktree-env (2026-05-25) 1 commit
(merged to 'next' on 2026-05-27 at 9c246d1969)
+ receive-pack: fix updateInstead with core.worktree
The GIT_WORK_TREE variable prepared to invoke the push-to-checkout
hook was leaking into the environment even when there was no hook
used and broke the default push-to-deploy (i.e., let "git checkout"
update the working tree only when the working tree is clean).
Will merge to 'master'.
source: <20260525162311.66240-2-hi@alyssa.is>
* ib/doc-push-default-simple (2026-05-25) 1 commit
- doc: clarify push.default=simple behavior
The documentation for `push.default = simple` has been clarified to
better explain its behavior, making it clear that it pushes the
current branch to a same-named branch on the remote, and detailing
the upstream requirements for centralized workflows.
Comments?
source: <pull.2115.v2.git.1779767888508.gitgitgadget@gmail.com>
* jc/doc-monitor-ghci (2026-05-24) 1 commit
- SubmittingPatches: proactively monitor GHCI pages
Encourage original authors to monitor the CI status.
Will merge to 'next'?
source: <xmqq1pf0gpp3.fsf@gitster.g>
* ec/commit-fixup-options (2026-05-26) 2 commits
- commit: allow -c/-C for all kinds of --fixup
- commit: allow -m/-F for all kinds of --fixup
The -m/-F/-c/-C options to supply commit log message from outside the
editor are now supported for all "git commit --fixup" variations.
Comments?
source: <cover.1779792311.git.erik@cervined.in>
* gh/jump-auto-mode (2026-05-21) 1 commit
- git-jump: pick a mode automatically when invoked without arguments
The 'git-jump' command (in contrib/) has been taught to automatically
pick a mode (merge, diff, or ws) when invoked without arguments.
Comments?
source: <pull.2108.v3.git.1779371110195.gitgitgadget@gmail.com>
* sp/doc-range-diff-takes-notes (2026-05-20) 1 commit
(merged to 'next' on 2026-05-22 at 020bec81b7)
+ Documentation/git-range-diff: add missing notes options in synopsis
Docfix.
Will merge to 'master'.
source: <20260521052841.73775-1-siddh.raman.pant@oracle.com>
* ps/odb-source-loose (2026-05-21) 19 commits
- odb/source-loose: drop pointer to the "files" source
- odb/source-loose: stub out remaining callbacks
- odb/source-loose: wire up `write_object_stream()` callback
- object-file: refactor writing objects to use loose source
- odb/source-loose: wire up `write_object()` callback
- loose: refactor object map to operate on `struct odb_source_loose`
- odb/source-loose: wire up `freshen_object()` callback
- odb/source-loose: drop `odb_source_loose_has_object()`
- odb/source-loose: wire up `count_objects()` callback
- odb/source-loose: wire up `find_abbrev_len()` callback
- odb/source-loose: wire up `for_each_object()` callback
- odb/source-loose: wire up `read_object_stream()` callback
- odb/source-loose: wire up `read_object_info()` callback
- odb/source-loose: wire up `close()` callback
- odb/source-loose: wire up `reprepare()` callback
- odb/source-loose: start converting to a proper `struct odb_source`
- odb/source-loose: store pointer to "files" instead of generic source
- odb/source-loose: move loose source into "odb/" subsystem
- Merge branch 'ps/odb-in-memory' into ps/odb-source-loose
The loose object source has been refactored into a proper `struct
odb_source`.
Comments?
source: <20260521-b4-pks-odb-source-loose-v1-0-6553b399be2d@pks.im>
* ps/setup-centralize-odb-creation (2026-05-25) 9 commits
- setup: construct object database in `apply_repository_format()`
- repository: stop reading loose object map twice on repo init
- setup: stop initializing object database without repository
- setup: stop creating the object database in `setup_git_env()`
- repository: stop initializing the object database in `repo_set_gitdir()`
- setup: deduplicate logic to apply repository format
- setup: drop `setup_git_env()`
- t0001: plug test gaps for git-init(1) with GIT_OBJECT_DIRECTORY
- Merge branch 'ps/setup-wo-the-repository' into ps/setup-centralize-odb-creation
The setup logic to discover and configure repositories has been
refactored, and the initialization of the object database has been
centralized.
Comments?
source: <20260526-b4-pks-setup-centralize-odb-creation-v2-0-2fa5b385c13e@pks.im>
* ps/gitlab-ci-macOS-improvements (2026-05-21) 2 commits
(merged to 'next' on 2026-05-22 at aaa3c7021e)
+ gitlab-ci: update macOS image
+ gitlab-ci: upgrade macOS runners
Update GitLab CI jobs that exercise macOS.
Will merge to 'master'.
source: <20260521-b4-pks-gitlab-ci-updates-v1-0-53bb46ed33e0@pks.im>
* kh/doc-hook (2026-05-21) 4 commits
(merged to 'next' on 2026-05-25 at 5e41d13adf)
+ doc: hook: don’t self-link via config include
+ doc: config: include existing git-hook(1) section
+ doc: hook: consistently capitalize Git
+ doc: hook: remove stray backtick
Doc updates.
Will merge to 'master'.
cf. <2832179.mvXUDI8C0e@piment-oiseau>
source: <CV_doc_hook.6f0@msgid.xyz>
* kh/doc-replay-config (2026-05-21) 4 commits
- doc: replay: move “default” to the right-hand-side
- doc: replay: use a nested definition list
- doc: replay: simplify replay.refAction description
- doc: link to config for git-replay(1)
Doc update for "git replay" to actually refer to its configuration
variables.
Comments?
source: <CV_doc_replay_config.709@msgid.xyz>
* jk/commit-graph-lazy-load-fallback (2026-05-18) 1 commit
(merged to 'next' on 2026-05-22 at d1188df466)
+ commit: fall back to full read when maybe_tree is NULL
The logic to lazy-load trees from the commit-graph has been made
more robust by falling back to reading the commit object when
the commit-graph is no longer available.
Will merge to 'master'.
source: <20260519061534.GA1709881@coredump.intra.peff.net>
* jk/connect-service-enum (2026-05-21) 2 commits
(merged to 'next' on 2026-05-24 at 293561cbc5)
+ transport-helper: fix typo in BUG() message
(merged to 'next' on 2026-05-21 at fd80c61e21)
+ connect: use "service" enum for "name" argument
The "name" argument in git_connect() and related functions has been
converted to a "service" enum to improve type safety and clarify its
purpose.
Will merge to 'master'.
source: <20260519052219.GA1703179@coredump.intra.peff.net>
source: <20260522044352.GA861761@coredump.intra.peff.net>
* aj/stash-patch-optimize-temporary-index (2026-05-22) 1 commit
- stash: reuse cached index entries in --patch temporary index
"git stash -p" has been optimized by reusing cached index
entries in its temporary index, avoiding unnecessary lstat()
calls on unchanged files.
Will merge to 'next'?
source: <pull.2306.v2.git.git.1779491545531.gitgitgadget@gmail.com>
* tb/bitmap-build-performance (2026-05-19) 9 commits
- pack-bitmap: build pseudo-merge bitmaps after regular bitmaps
- pack-bitmap: remember pseudo-merge parents
- pack-bitmap: sort bitmaps before XORing
- pack-bitmap: cache object positions during fill
- pack-bitmap: consolidate `find_object_pos()` success path
- pack-bitmap: reuse stored selected bitmaps
- pack-bitmap: check subtree bits before recursing
- pack-bitmap: pass object position to `fill_bitmap_tree()`
- Merge branch 'tb/pseudo-merge-bugfixes' into tb/bitmap-build-performance
Reachability bitmap generation has been significantly optimized. By
reordering tree traversal, caching object positions, and refining how
pseudo-merge bitmaps are constructed, the performance of "git repack
--write-midx-bitmaps" is improved, especially for large repositories
and when using pseudo-merges.
Comments?
source: <cover.1779207127.git.me@ttaylorr.com>
* hn/status-pull-advice-qualified (2026-05-21) 1 commit
- remote: qualify "git pull" advice for non-upstream compareBranches
Advice shown by "git status" when the local branch is behind or has
diverged from its push branch has been updated to suggest "git pull
<remote> <branch>".
Comments?
source: <pull.2301.v4.git.git.1779372367317.gitgitgadget@gmail.com>
* rs/strbuf-add-uint (2026-05-12) 4 commits
- ls-tree: use strbuf_add_uint()
- ls-files: use strbuf_add_uint()
- cat-file: use strbuf_add_uint()
- strbuf: add strbuf_add_uint()
Adding a decimal integer with strbuf_addf("%u") appears commonly;
they have been optimized by using a custom formatter.
Comments?
source: <20260512115603.80780-1-l.s.r@web.de>
* ta/approxidate-noon-fix (2026-05-21) 4 commits
(merged to 'next' on 2026-05-25 at 2dd9ce3c54)
+ approxidate: use deferred mday adjustments for "specials"
+ approxidate: make "specials" respect fixed day-of-month
+ t0006: add support for approxidate test date adjustment
+ approxidate: make "today" wrap to midnight
"Friday noon" asked in the morning on Sunday was parsed to be one
day before the specified time, which has been corrected.
Will merge to 'master'.
source: <20260521105408.8222-1-taahol@utu.fi>
* mm/doc-word-diff (2026-05-13) 1 commit
- doc: clarify that --word-diff operates on line-level hunks
The documentation for "--word-diff" has been extended with a bit of
implementation detail of where these different words come from.
Comments?
source: <pull.2113.git.1778686956622.gitgitgadget@gmail.com>
* rs/strbuf-add-oid-hex (2026-05-13) 1 commit
- hex: add and use strbuf_add_oid_hex()
Formatting object name in full hexadecimal form has been optimized
by using a new strbuf_add_oid_hex() helper function.
Comments?
source: <183aa0fd-d455-4ec9-9c42-d511fac8b3e4@web.de>
* ed/check-connected-close-err-fd (2026-05-16) 1 commit
(merged to 'next' on 2026-05-22 at 00d592399e)
+ Merge branch 'ed/check-connected-close-err-fd-2.53' into ed/check-connected-close-err-fd
(this branch uses ed/check-connected-close-err-fd-2.53.)
File descriptor leak fix.
Will merge to 'master'.
(this branch uses ed/check-connected-close-err-fd-2.53.)
* ed/check-connected-close-err-fd-2.53 (2026-05-14) 1 commit
(merged to 'next' on 2026-05-22 at 1017d0e022)
+ connected: close err_fd in promisor fast-path
(this branch is used by ed/check-connected-close-err-fd.)
File descriptor leak fix (for 2.54 maintenance track).
Will merge to 'master'.
source: <pull.2303.git.git.1778827194448.gitgitgadget@gmail.com>
* kk/tips-reachable-from-bases-optim (2026-05-16) 2 commits
(merged to 'next' on 2026-05-22 at 87d6b8e666)
+ t6600: add tests for duplicate tips in tips_reachable_from_bases()
+ commit-reach: use object flags for tips_reachable_from_bases()
Revision traversal optimization.
Will merge to 'master'.
source: <pull.2116.v3.git.1778947182.gitgitgadget@gmail.com>
* tc/generate-configlist-fix-for-older-ninja (2026-05-15) 1 commit
(merged to 'next' on 2026-05-22 at 8322bfb8f2)
+ generate-configlist: collapse depfile for older Ninja
Build update.
Will merge to 'master'.
source: <20260515-toon-fix-almalinux8-v3-1-b545a0647f0f@iotcl.com>
* hn/config-typo-advice (2026-05-25) 1 commit
- config: suggest the correct form when key contains "=" in set context
"git config foo.bar=baz" is not likely to be a request to read the
value of such a variable with '=' in its name; rather it is plausible
that the user meant "git config set foo.bar baz". Give advice when
giving an error message.
Comments?
source: <pull.2302.v3.git.git.1779697995418.gitgitgadget@gmail.com>
* ja/doc-synopsis-style-again (2026-05-25) 6 commits
- doc: convert git-imap-send synopsis and options to new style
- doc: convert git-apply synopsis and options to new style
- doc: convert git-am synopsis and options to new style
- doc: convert git-grep synopsis and options to new style
- doc: git bisect: clarify the usage of the synopsis vs actual command
- doc: convert git-bisect to synopsis style
A batch of documentation pages has been updated to use the modern
synopsis style.
Will merge to 'next'?
source: <pull.2117.v2.git.1779704908.gitgitgadget@gmail.com>
* jt/config-lock-timeout (2026-05-17) 1 commit
- config: retry acquiring config.lock, configurable via core.configLockTimeout
Configuration file locking now retries for a short period, avoiding
failures when multiple processes attempt to update the configuration
simultaneously.
Comments?
cf. <xmqqzf1xbl4i.fsf@gitster.g>
source: <20260517132111.1014901-1-joerg@thalheim.io>
* hn/branch-prune-merged (2026-05-22) 6 commits
- branch: add --dry-run for --prune-merged
- branch: add branch.<name>.pruneMerged opt-out
- branch: add --prune-merged <branch>
- branch: prepare delete_branches for a bulk caller
- branch: let delete_branches warn instead of error on bulk refusal
- branch: add --forked <branch>
"git branch" command learned "--prune-merged" option to remove
local branches that have already been merged to the remote-tracking
branches they track.
Comments?
source: <pull.2285.v11.git.git.1779449498.gitgitgadget@gmail.com>
* st/daemon-sockaddr-fixes (2026-05-14) 3 commits
- daemon: guard NULL REMOTE_PORT in execute() logging
- daemon: fix IPv6 address truncation in ip2str()
- daemon: fix IPv6 address corruption in lookup_hostname()
Correct use of sockaddr API in "git daemon".
Waiting for response(s) to review comment(s).
cf. <agGLRC1ziF5F8Okh@pks.im>
source: <pull.2300.git.git.1778773592.gitgitgadget@gmail.com>
* ob/more-repo-config-values (2026-04-23) 8 commits
- env: move "warn_on_object_refname_ambiguity" into `struct repo_config_values`
- env: move "sparse_expect_files_outside_of_patterns" into `repo_config_values`
- env: move "core_sparse_checkout_cone" into `struct repo_config_values`
- environment: move "precomposed_unicode" into `struct repo_config_values`
- environment: move "pack_compression_level" into `struct repo_config_values`
- environment: move `zlib_compression_level` into `struct repo_config_values`
- environment: move "check_stat" into `struct repo_config_values`
- environment: move "trust_ctime" into `struct repo_config_values`
Expecting a reroll.
cf. <CAD=f0L8-_3sDGGkCzF4WA0xmUtaY_qiz__3zq5AemLgwTsqvsg@mail.gmail.com>
cf. <xmqqlddqu013.fsf@gitster.g>
source: <20260423165432.143598-1-belkid98@gmail.com>
* cc/promisor-auto-config-url-more (2026-05-19) 9 commits
- doc: promisor: improve acceptFromServer entry
- promisor-remote: auto-configure unknown remotes
- promisor-remote: trust known remotes matching acceptFromServerUrl
- promisor-remote: introduce promisor.acceptFromServerUrl
- promisor-remote: add 'local_name' to 'struct promisor_info'
- urlmatch: add url_normalize_pattern() helper
- urlmatch: change 'allow_globs' arg to bool
- t5710: simplify 'mkdir X' followed by 'git -C X init'
- Merge branch 'cc/promisor-auto-config-url' into cc/promisor-auto-config-url-more
The handling of promisor-remote protocol capability has been
loosened to allow the other side to add to the list of promisor
remotes via the promisor.acceptFromServerURL configuration
variable.
Comments?
source: <20260519153808.494105-1-christian.couder@gmail.com>
* hn/checkout-track-fetch (2026-05-23) 2 commits
- checkout: extend --track with a "fetch" mode to refresh start-point
- branch: expose helpers for finding the remote owning a tracking ref
"git checkout --track=..." learned to optionally fetch the branch
from the remote the new branch will work with.
Comments?
source: <pull.2281.v13.git.git.1779565714.gitgitgadget@gmail.com>
* mf/revision-max-count-oldest (2026-05-18) 1 commit
- revision.c: implement --max-count-oldest
"git rev-list" (and "git log" family of commands) learned a new "--max-count-oldest"
that picks oldest N commits in the range instead of the usual newest.
Comments?
source: <8210d60832b9a58aa4d71fc3790e44d8989564ce.1779152064.git.mroik@delayed.space>
* mm/line-log-cleanup (2026-05-25) 3 commits
- line-log: allow non-patch diff formats with -L
- line-log: integrate -L output with the standard log-tree pipeline
- revision: move -L setup before output_format-to-diff derivation
The `git log -L` implementation has been refactored to use the
standard diff output pipeline, enabling pickaxe and diff-filter to
work as expected. Additionally, metadata-only diff formats like
--raw and --name-only are now supported with -L.
Will merge to 'next'?
source: <pull.2120.v2.git.1779733799.gitgitgadget@gmail.com>
* ds/path-walk-filters (2026-05-22) 14 commits
(merged to 'next' on 2026-05-25 at eccb829b10)
+ path-walk: support `combine` filter
+ path-walk: support `object:type` filter
+ path-walk: support `tree:0` filter
+ t6601: tag otherwise-unreachable trees
+ pack-objects: support sparse:oid filter with path-walk
+ path-walk: add pl_sparse_trees to control tree pruning
+ path-walk: support blob size limit filter
+ backfill: die on incompatible filter options
+ path-walk: support blobless filter
+ path-walk: always emit directly-requested objects
+ t/perf: add pack-objects filter and path-walk benchmark
+ pack-objects: pass --objects with --path-walk
+ t5620: make test work with path-walk var
+ Merge branch 'en/backfill-fixes-and-edges' into ds/path-walk-filters
The "git pack-objects --path-walk" traversal has been integrated
with several object filters, including blobless and sparse filters.
Will merge to 'master'.
source: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
* en/ort-harden-against-corrupt-trees (2026-04-20) 5 commits
- cache-tree: fix verify_cache() to catch non-adjacent D/F conflicts
- merge-ort: abort merge when trees have duplicate entries
- merge-ort: free diff pairs queue in clear_or_reinit_internal_opts()
- merge-ort: drop unnecessary show_all_errors from collect_merge_info()
- merge-ort: propagate callback errors from traverse_trees_wrapper()
"ort" merge backend handles merging corrupt trees better by
aborting when it should.
Needs review.
source: <pull.2096.git.1776731171.gitgitgadget@gmail.com>
* pw/status-rebase-todo (2026-05-01) 2 commits
- status: improve rebase todo list parsing
- sequencer: factor out parsing of todo commands
The display of the rebase todo list in "git status" has been
improved to correctly abbreviate object IDs for more commands and
avoid misinterpreting refs as object IDs.
Needs review.
source: <cover.1777648598.git.phillip.wood@dunelm.org.uk>
* cl/conditional-config-on-worktree-path (2026-05-24) 2 commits
- config: add "worktree" and "worktree/i" includeIf conditions
- config: refactor include_by_gitdir() into include_by_path()
The [includeIf "condition"] conditional inclusion facility for
configuration files has learned to use the location of worktree
in its condition.
Ready?
source: <20260525-includeif-worktree-v5-0-1efe525d025a@black-desk.cn>
* ps/shift-root-in-graph (2026-04-27) 1 commit
- graph: add indentation for commits preceded by a parentless commit
In a history with more than one root commit, "git log --graph
--oneline" stuffed an unrelated commit immediately below a root
commit, which has been corrected by making the spot below a root
unavailable.
Waiting for response(s) to review comment(s).
cf. <20260513230216.GA1378627@coredump.intra.peff.net>
source: <20260427102838.44867-2-pabloosabaterr@gmail.com>
* lp/repack-propagate-promisor-debugging-info (2026-04-18) 6 commits
- repack-promisor: add missing headers
- t7703: test for promisor file content after geometric repack
- t7700: test for promisor file content after repack
- repack-promisor: preserve content of promisor files after repack
- repack-promisor add helper to fill promisor file after repack
- pack-write: add explanation to promisor file content
When fetching objects into a lazily cloned repository, .promisor
files are created with information meant to help debugging. "git
repack" has been taught to carry this information forward to
packfiles that are newly created.
Needs review.
cf. <xmqqse7xm8av.fsf@gitster.g>
source: <cover.1776384902.git.lorenzo.pegorari2002@gmail.com>
* th/promisor-quiet-per-repo (2026-04-06) 1 commit
- promisor-remote: fix promisor.quiet to use the correct repository
The "promisor.quiet" configuration variable was not used from
relevant submodules when commands like "grep --recurse-submodules"
triggered a lazy fetch, which has been corrected.
Comments?
source: <20260406183041.783800-1-vikingtc4@gmail.com>
* sa/cat-file-batch-mailmap-switch (2026-04-15) 1 commit
(merged to 'next' on 2026-05-22 at 197a9bad73)
+ cat-file: add mailmap subcommand to --batch-command
"git cat-file --batch" learns an in-line command "mailmap"
that lets the user toggle use of mailmap.
Will merge to 'master'.
cf. <xmqqwlwy4v7t.fsf@gitster.g>
source: <20260416033250.4327-2-siddharthasthana31@gmail.com>
* jd/unpack-trees-wo-the-repository (2026-03-31) 2 commits
- unpack-trees: use repository from index instead of global
- unpack-trees: use repository from index instead of global
A handful of inappropriate uses of the_repository have been
rewritten to use the right repository structure instance in the
unpack-trees.c codepath.
Comments?
source: <pull.2258.v2.git.git.1774971267.gitgitgadget@gmail.com>
* kh/doc-trailers (2026-04-13) 9 commits
- doc: interpret-trailers: document comment line treatment
- doc: interpret-trailers: commit to “trailer block” term
- doc: interpret-trailers: add key format example
- doc: interpret-trailers: explain key format
- doc: interpret-trailers: explain the format after the intro
- doc: interpret-trailers: not just for commit messages
- doc: interpret-trailers: use “metadata” in Name as well
- doc: interpret-trailers: replace “lines” with “metadata”
- doc: interpret-trailers: stop fixating on RFC 822
Documentation updates.
Needs review.
cf. <xmqq1pfivfa3.fsf@gitster.g>
source: <V2_CV_doc_int-tr_key_format.613@msgid.xyz>
* ps/graph-lane-limit (2026-03-27) 3 commits
(merged to 'next' on 2026-05-22 at ca1c5e8432)
+ graph: add truncation mark to capped lanes
+ graph: add --graph-lane-limit option
+ graph: limit the graph width to a hard-coded max
The graph output from commands like "git log --graph" can now be
limited to a specified number of lanes, preventing overly wide output
in repositories with many branches.
Will merge to 'master'.
cf. <bdff0a5d-b738-4053-9b72-08eba88156de@kdbg.org>
source: <20260328001113.1275291-1-pabloosabaterr@gmail.com>
* jr/bisect-custom-terms-in-output (2026-05-14) 3 commits
(merged to 'next' on 2026-05-22 at 1ccd1056c9)
+ rev-parse: use selected alternate terms to look up refs
+ bisect: print bisect terms in single quotes
+ bisect: use selected alternate terms in status output
"git bisect" now uses the selected terms (e.g., old/new) more
consistently in its output.
Will merge to 'master'.
source: <20260514-bisect-terms-v4-0-b3e3cf1b06ce@schlaraffenlan.de>
* ua/push-remote-group (2026-05-03) 3 commits
- push: support pushing to a remote group
- remote: move remote group resolution to remote.c
- remote: fix sign-compare warnings in push_cas_option
"git push" learned to take a "remote group" name to push to, which
causes pushes to multiple places, just like "git fetch" would do.
Comments?
source: <20260503153402.1333220-1-usmanakinyemi202@gmail.com>
* js/parseopt-subcommand-autocorrection (2026-04-27) 11 commits
- SQUASH???
- doc: document autocorrect API
- parseopt: add tests for subcommand autocorrection
- parseopt: enable subcommand autocorrection for git-remote and git-notes
- parseopt: autocorrect mistyped subcommands
- autocorrect: provide config resolution API
- autocorrect: rename AUTOCORRECT_SHOW to AUTOCORRECT_HINT
- autocorrect: use mode and delay instead of magic numbers
- help: move tty check for autocorrection to autocorrect.c
- help: make autocorrect handling reusable
- parseopt: extract subcommand handling from parse_options_step()
The parse-options library learned to auto-correct misspelled
subcommand names.
Expecting a reroll.
cf. <xmqqcxz2tzpr.fsf@gitster.g>
source: <SY0P300MB0801677A2A1E0FD38D06A841CE2A2@SY0P300MB0801.AUSP300.PROD.OUTLOOK.COM>
* jc/neuter-sideband-post-3.0 (2026-03-05) 2 commits
- sideband: delay sanitizing by default to Git v3.0
- Merge branch 'jc/neuter-sideband-fixup' into jc/neuter-sideband-post-3.0
The final step, split from earlier attempt by Dscho, to loosen the
sideband restriction for now and tighten later at Git v3.0 boundary.
On hold to help the base topic with wider exposure.
(this branch uses jc/neuter-sideband-fixup.)
source: <20260305233452.3727126-8-gitster@pobox.com>
* cs/subtree-split-recursion (2026-03-05) 3 commits
- contrib/subtree: reduce recursion during split
- contrib/subtree: functionalize split traversal
- contrib/subtree: reduce function side-effects
When processing large history graphs on Debian or Ubuntu, "git
subtree" can die with a "recursion depth reached" error.
Comments?
source: <20260305-cs-subtree-split-recursion-v2-0-7266be870ba9@howdoi.land>
* pt/fsmonitor-linux (2026-04-15) 13 commits
(merged to 'next' on 2026-05-22 at 5d99c1765d)
+ fsmonitor: convert shown khash to strset in do_handle_client
+ fsmonitor: add tests for Linux
+ fsmonitor: add timeout to daemon stop command
+ fsmonitor: close inherited file descriptors and detach in daemon
+ run-command: add close_fd_above_stderr option
+ fsmonitor: implement filesystem change listener for Linux
+ fsmonitor: rename fsm-settings-darwin.c to fsm-settings-unix.c
+ fsmonitor: rename fsm-ipc-darwin.c to fsm-ipc-unix.c
+ fsmonitor: use pthread_cond_timedwait for cookie wait
+ compat/win32: add pthread_cond_timedwait
+ fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
+ fsmonitor: fix khash memory leak in do_handle_client
+ t9210, t9211: disable GIT_TEST_SPLIT_INDEX for scalar clone tests
The fsmonitor daemon has been implemented for Linux.
Will merge to 'master'.
cf. <xmqqa4u5nnxq.fsf@gitster.g>
source: <pull.2147.v15.git.git.1776259657.gitgitgadget@gmail.com>
^ permalink raw reply
* [PATCH] commit-reach: stop sorting in paint_down_to_common()
From: René Scharfe @ 2026-05-27 15:52 UTC (permalink / raw)
To: Git List
None of the three callers of paint_down_to_common() care about the order
of its result list: merge_bases_many() sorts it again after removing
stale items, remove_redundant_no_gen() and repo_in_merge_bases_many()
throw the list away without even looking at it. So drop the unnecessary
commit_list_sort_by_date() call.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
commit-reach.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/commit-reach.c b/commit-reach.c
index 5a52be90a6..056a7ed8d8 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -137,7 +137,6 @@ static int paint_down_to_common(struct repository *r,
}
clear_prio_queue(&queue);
- commit_list_sort_by_date(result);
return 0;
}
--
2.54.0
^ permalink raw reply related
* [PATCH 3/3] revision: use priority queue for non-limited streaming walks
From: Kristofer Karlsson via GitGitGadget @ 2026-05-27 15:50 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2127.git.1779897003.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
The streaming (non-limited) walk in get_revision_1() inserts newly
discovered parent commits into a date-sorted queue via
commit_list_insert_by_date(), which scans the linked list to find the
insertion point -- O(w) per insert, where w is the width of the active
walk frontier. Replace this with an O(log w) priority queue.
Add a commit_queue field to rev_info alongside the existing commits
linked list. The two representations are mutually exclusive: setup
and external callers that need list access use the linked list, then
get_revision_1() lazily drains it into the priority queue on first
call. Add a REV_WALK_NO_WALK enum value to distinguish the no_walk
case (which still uses the commit list) from the streaming case.
The conversion function rev_info_commit_list_to_queue() is public so
callers that know they will iterate can convert early.
Combined with the limit_list() priority queue change already in
master, this eliminates all O(w) sorted linked-list insertion from
the revision walk machinery.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit.c | 13 -------------
commit.h | 2 --
revision.c | 55 +++++++++++++++++++++++++++++-------------------------
revision.h | 12 +++++++++++-
4 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/commit.c b/commit.c
index e3e7352e69..5112c7b2af 100644
--- a/commit.c
+++ b/commit.c
@@ -729,19 +729,6 @@ void commit_list_free(struct commit_list *list)
pop_commit(&list);
}
-struct commit_list * commit_list_insert_by_date(struct commit *item, struct commit_list **list)
-{
- struct commit_list **pp = list;
- struct commit_list *p;
- while ((p = *pp) != NULL) {
- if (p->item->date < item->date) {
- break;
- }
- pp = &p->next;
- }
- return commit_list_insert(item, pp);
-}
-
static int commit_list_compare_by_date(const struct commit_list *a,
const struct commit_list *b)
{
diff --git a/commit.h b/commit.h
index 58150045af..385492fbb1 100644
--- a/commit.h
+++ b/commit.h
@@ -191,8 +191,6 @@ int commit_list_contains(struct commit *item,
struct commit_list **commit_list_append(struct commit *commit,
struct commit_list **next);
unsigned commit_list_count(const struct commit_list *l);
-struct commit_list *commit_list_insert_by_date(struct commit *item,
- struct commit_list **list);
void commit_list_sort_by_date(struct commit_list **list);
/* Shallow copy of the input list */
diff --git a/revision.c b/revision.c
index 9d0fc696d0..4bb3b16e43 100644
--- a/revision.c
+++ b/revision.c
@@ -1116,7 +1116,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
}
static int process_parents(struct rev_info *revs, struct commit *commit,
- struct commit_list **list, struct prio_queue *queue)
+ struct prio_queue *queue)
{
struct commit_list *parent = commit->parents;
unsigned pass_flags;
@@ -1158,8 +1158,6 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
if (p->object.flags & SEEN)
continue;
p->object.flags |= (SEEN | NOT_USER_GIVEN);
- if (list)
- commit_list_insert_by_date(p, list);
if (queue)
prio_queue_put(queue, p);
if (revs->exclude_first_parent_only)
@@ -1207,8 +1205,6 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
p->object.flags |= pass_flags | CHILD_VISITED;
if (!(p->object.flags & SEEN)) {
p->object.flags |= (SEEN | NOT_USER_GIVEN);
- if (list)
- commit_list_insert_by_date(p, list);
if (queue)
prio_queue_put(queue, p);
}
@@ -1470,7 +1466,7 @@ static int limit_list(struct rev_info *revs)
if (revs->max_age != -1 && (commit->date < revs->max_age))
obj->flags |= UNINTERESTING;
- if (process_parents(revs, commit, NULL, &queue) < 0) {
+ if (process_parents(revs, commit, &queue) < 0) {
clear_prio_queue(&queue);
return -1;
}
@@ -3257,6 +3253,7 @@ static void free_void_commit_list(void *list)
void release_revisions(struct rev_info *revs)
{
commit_list_free(revs->commits);
+ clear_prio_queue(&revs->commit_queue);
commit_list_free(revs->ancestry_path_bottoms);
release_display_notes(&revs->notes_opt);
object_array_clear(&revs->pending);
@@ -3726,7 +3723,7 @@ static void explore_walk_step(struct rev_info *revs)
if (revs->max_age != -1 && (c->date < revs->max_age))
c->object.flags |= UNINTERESTING;
- if (process_parents(revs, c, NULL, NULL) < 0)
+ if (process_parents(revs, c, NULL) < 0)
return;
if (c->object.flags & UNINTERESTING)
@@ -3902,7 +3899,7 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
{
struct commit_list *p;
struct topo_walk_info *info = revs->topo_walk_info;
- if (process_parents(revs, commit, NULL, NULL) < 0) {
+ if (process_parents(revs, commit, NULL) < 0) {
if (!revs->ignore_missing_links)
die("Failed to traverse parents of commit %s",
oid_to_hex(&commit->object.oid));
@@ -3938,6 +3935,13 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)
}
}
+void rev_info_commit_list_to_queue(struct rev_info *revs)
+{
+ while (revs->commits)
+ prio_queue_put(&revs->commit_queue, pop_commit(&revs->commits));
+}
+
+
int prepare_revision_walk(struct rev_info *revs)
{
int i;
@@ -4006,7 +4010,7 @@ static enum rewrite_result rewrite_one_1(struct rev_info *revs,
for (;;) {
struct commit *p = *pp;
if (!revs->limited)
- if (process_parents(revs, p, NULL, queue) < 0)
+ if (process_parents(revs, p, queue) < 0)
return rewrite_one_error;
if (p->object.flags & UNINTERESTING)
return rewrite_one_ok;
@@ -4020,27 +4024,18 @@ static enum rewrite_result rewrite_one_1(struct rev_info *revs,
}
}
-static void merge_queue_into_list(struct prio_queue *q, struct commit_list **list)
+static void merge_queue_into_prio_queue(struct prio_queue *from,
+ struct prio_queue *to)
{
- while (q->nr) {
- struct commit *item = prio_queue_peek(q);
- struct commit_list *p = *list;
-
- if (p && p->item->date >= item->date)
- list = &p->next;
- else {
- p = commit_list_insert(item, list);
- list = &p->next; /* skip newly added item */
- prio_queue_get(q); /* pop item */
- }
- }
+ while (from->nr)
+ prio_queue_put(to, prio_queue_get(from));
}
static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp)
{
struct prio_queue queue = { compare_commits_by_commit_date };
enum rewrite_result ret = rewrite_one_1(revs, pp, &queue);
- merge_queue_into_list(&queue, &revs->commits);
+ merge_queue_into_prio_queue(&queue, &revs->commit_queue);
clear_prio_queue(&queue);
return ret;
}
@@ -4331,6 +4326,7 @@ enum rev_walk_mode {
REV_WALK_REFLOG,
REV_WALK_TOPO,
REV_WALK_LIMITED,
+ REV_WALK_NO_WALK,
REV_WALK_STREAMING,
};
@@ -4342,6 +4338,8 @@ static enum rev_walk_mode get_walk_mode(struct rev_info *revs)
return REV_WALK_TOPO;
if (revs->limited)
return REV_WALK_LIMITED;
+ if (revs->no_walk)
+ return REV_WALK_NO_WALK;
return REV_WALK_STREAMING;
}
@@ -4349,6 +4347,9 @@ static struct commit *get_revision_1(struct rev_info *revs)
{
enum rev_walk_mode mode = get_walk_mode(revs);
+ if (mode == REV_WALK_STREAMING && revs->commits)
+ rev_info_commit_list_to_queue(revs);
+
while (1) {
struct commit *commit;
@@ -4360,9 +4361,12 @@ static struct commit *get_revision_1(struct rev_info *revs)
commit = next_topo_commit(revs);
break;
case REV_WALK_LIMITED:
- case REV_WALK_STREAMING:
+ case REV_WALK_NO_WALK:
commit = pop_commit(&revs->commits);
break;
+ case REV_WALK_STREAMING:
+ commit = prio_queue_get(&revs->commit_queue);
+ break;
}
if (!commit)
@@ -4390,12 +4394,13 @@ static struct commit *get_revision_1(struct rev_info *revs)
break;
case REV_WALK_STREAMING:
if (process_parents(revs, commit,
- &revs->commits, NULL) < 0) {
+ &revs->commit_queue) < 0) {
if (!revs->ignore_missing_links)
die("Failed to traverse parents of commit %s",
oid_to_hex(&commit->object.oid));
}
break;
+ case REV_WALK_NO_WALK:
case REV_WALK_LIMITED:
break;
}
diff --git a/revision.h b/revision.h
index 584f1338b5..04982a3d47 100644
--- a/revision.h
+++ b/revision.h
@@ -12,6 +12,7 @@
#include "decorate.h"
#include "ident.h"
#include "list-objects-filter-options.h"
+#include "prio-queue.h"
#include "strvec.h"
/**
@@ -122,8 +123,14 @@ struct oidset;
struct topo_walk_info;
struct rev_info {
- /* Starting list */
+ /*
+ * Work queue of commits, stored as either a linked list or a
+ * priority queue, but never both at the same time.
+ * rev_info_commit_list_to_queue() converts list to queue.
+ */
struct commit_list *commits;
+ struct prio_queue commit_queue;
+
struct object_array pending;
struct repository *repo;
@@ -400,6 +407,7 @@ struct rev_info {
* uninitialized.
*/
#define REV_INFO_INIT { \
+ .commit_queue = { .compare = compare_commits_by_commit_date }, \
.abbrev = DEFAULT_ABBREV, \
.simplify_history = 1, \
.pruning.flags.recursive = 1, \
@@ -478,6 +486,8 @@ void reset_revision_walk(void);
*/
int prepare_revision_walk(struct rev_info *revs);
+/* Drain the commits linked list into the priority queue. */
+void rev_info_commit_list_to_queue(struct rev_info *revs);
/**
* Takes a pointer to a `rev_info` structure and iterates over it, returning a
* `struct commit *` each time you call it. The end of the revision list is
--
gitgitgadget
^ permalink raw reply related
* [PATCH 2/3] revision: introduce rev_walk_mode to clarify get_revision_1()
From: Kristofer Karlsson via GitGitGadget @ 2026-05-27 15:50 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2127.git.1779897003.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
get_revision_1() dispatches to different walk strategies based on a
combination of rev_info flags: reflog_info, topo_walk_info, and
limited. These conditions are checked in multiple places within
the function -- once to select the next commit, and again to decide
how to expand parents -- and the two chains must stay in sync.
Extract the mode selection into a rev_walk_mode enum and a small
get_walk_mode() helper, resolved once at the top of get_revision_1().
Both dispatch sites now switch on the same mode variable, making it
obvious that they agree and easier to verify that all modes are
handled.
No functional change.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
revision.c | 62 ++++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 48 insertions(+), 14 deletions(-)
diff --git a/revision.c b/revision.c
index e1970b9c5d..9d0fc696d0 100644
--- a/revision.c
+++ b/revision.c
@@ -4327,22 +4327,48 @@ static void track_linear(struct rev_info *revs, struct commit *commit)
revs->previous_parents = commit_list_copy(commit->parents);
}
+enum rev_walk_mode {
+ REV_WALK_REFLOG,
+ REV_WALK_TOPO,
+ REV_WALK_LIMITED,
+ REV_WALK_STREAMING,
+};
+
+static enum rev_walk_mode get_walk_mode(struct rev_info *revs)
+{
+ if (revs->reflog_info)
+ return REV_WALK_REFLOG;
+ if (revs->topo_walk_info)
+ return REV_WALK_TOPO;
+ if (revs->limited)
+ return REV_WALK_LIMITED;
+ return REV_WALK_STREAMING;
+}
+
static struct commit *get_revision_1(struct rev_info *revs)
{
+ enum rev_walk_mode mode = get_walk_mode(revs);
+
while (1) {
struct commit *commit;
- if (revs->reflog_info)
+ switch (mode) {
+ case REV_WALK_REFLOG:
commit = next_reflog_entry(revs->reflog_info);
- else if (revs->topo_walk_info)
+ break;
+ case REV_WALK_TOPO:
commit = next_topo_commit(revs);
- else
+ break;
+ case REV_WALK_LIMITED:
+ case REV_WALK_STREAMING:
commit = pop_commit(&revs->commits);
+ break;
+ }
if (!commit)
return NULL;
- if (revs->reflog_info)
+ if (mode == REV_WALK_REFLOG)
commit->object.flags &= ~(ADDED | SEEN | SHOWN);
/*
@@ -4350,20 +4376,28 @@ static struct commit *get_revision_1(struct rev_info *revs)
* the parents here. We also need to do the date-based limiting
* that we'd otherwise have done in limit_list().
*/
- if (!revs->limited) {
- if (revs->max_age != -1 &&
- comparison_date(revs, commit) < revs->max_age)
- continue;
+ if (mode != REV_WALK_LIMITED &&
+ revs->max_age != -1 &&
+ comparison_date(revs, commit) < revs->max_age)
+ continue;
- if (revs->reflog_info)
- try_to_simplify_commit(revs, commit);
- else if (revs->topo_walk_info)
- expand_topo_walk(revs, commit);
- else if (process_parents(revs, commit, &revs->commits, NULL) < 0) {
+ switch (mode) {
+ case REV_WALK_REFLOG:
+ try_to_simplify_commit(revs, commit);
+ break;
+ case REV_WALK_TOPO:
+ expand_topo_walk(revs, commit);
+ break;
+ case REV_WALK_STREAMING:
+ if (process_parents(revs, commit,
+ &revs->commits, NULL) < 0) {
if (!revs->ignore_missing_links)
die("Failed to traverse parents of commit %s",
- oid_to_hex(&commit->object.oid));
+ oid_to_hex(&commit->object.oid));
}
+ break;
+ case REV_WALK_LIMITED:
+ break;
}
switch (simplify_commit(revs, commit)) {
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/3] pack-objects: call release_revisions() after cruft traversal
From: Kristofer Karlsson via GitGitGadget @ 2026-05-27 15:50 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2127.git.1779897003.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
enumerate_and_traverse_cruft_objects() initializes a rev_info on the
stack but never calls release_revisions() afterwards. This is not
visible on master but becomes a leak once the revision walking
machinery uses dynamically allocated structures.
Add the missing release_revisions() call.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
builtin/pack-objects.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 480cc0bd8c..67025e8625 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4275,6 +4275,7 @@ static void enumerate_and_traverse_cruft_objects(struct string_list *fresh_packs
traverse_commit_list(&revs, show_cruft_commit, show_cruft_object, NULL);
stop_progress(&progress_state);
+ release_revisions(&revs);
}
static void read_cruft_objects(void)
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/3] revision: use priority queue for streaming walks
From: Kristofer Karlsson via GitGitGadget @ 2026-05-27 15:49 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson
This is a follow-up to kk/limit-list-optim (now in master), which replaced
the O(N) sorted linked-list insertion in limit_list() with a priority queue.
In the review thread for that patch, I mentioned that the same approach
could be applied to the streaming (non-limited) walk in get_revision_1().
Junio suggested doing it as a separate topic, and Peff noted he had a local
branch (jk/revs-commits-prio-queue) doing a broader conversion of
revs->commits to prio_queue entirely.
This series takes a lighter-weight approach: it keeps the linked list for
setup and external callers, and adds a separate commit_queue field that the
streaming walk drains into on first use. This avoids touching bisect,
line-log, and list-objects code, at the cost of a "only one should be
non-empty" invariant between the two fields.
Together with the limit_list() change already in master, this eliminates all
commit_list_insert_by_date() callers from revision.c.
Patch 1 is a small leak fix -- a missing release_revisions() call in
pack-objects that becomes visible once the commit queue uses a dynamically
allocated prio_queue array.
Patch 2 introduces a rev_walk_mode enum to replace the repeated if/else
chains in get_revision_1(). The function dispatches on walk mode in multiple
places (next commit, expand parents, flag clearing) and these chains must
stay in sync. The enum resolves the mode once and both dispatch sites switch
on the same variable. This is a lighter alternative to the vtable-based
refactoring I mentioned before. No functional change.
Patch 3 is the actual conversion of the streaming walk to use a priority
queue.
== Why this helps ==
The streaming walk in get_revision_1() inserts newly discovered parent
commits into a date-sorted queue. On master, this uses
commit_list_insert_by_date(), which walks the linked list to find the
insertion point -- O(w) per insert, where w is the queue width (active walk
frontier).
In merge-heavy repositories, the walk frontier stays wide:
Repository Commits Peak width Avg width
=======================================
monorepo (2.4M) 2,420K 2,653 1,700 linux.git 1,445K 581 235 git.git 82K 188
82
On the monorepo, each of the 2.4M commits requires scanning an average of
1,700 list entries to find the insertion point. With the priority queue,
this drops to ~11 heap comparisons.
== Benchmarks ==
All benchmarks: best of 3 runs, same machine, commit-graph present.
Streaming walks (affected by this series):
git rev-list --count HEAD (monorepo, 2.4M commits)
master: 17.94s
patched: 3.38s (5.3x faster)
git rev-list HEAD (monorepo, full output)
master: 27.72s
patched: 8.61s (2.8x faster, I/O-bound fraction unchanged)
Regression checks -- non-merge-heavy repos (streaming path, but frontier
stays narrow so O(w) insertion was never the bottleneck):
git rev-list --count HEAD (linux.git, 1.4M commits)
master: 1.76s
patched: 1.81s (no change)
git rev-list HEAD (linux.git, full output)
master: 4.46s
patched: 4.52s (no change)
git rev-list --count HEAD (git.git, 82K commits)
master: 83ms
patched: 86ms (no change)
Regression checks -- other walk modes (not affected by this series):
git rev-list --count HEAD~5000...HEAD (monorepo, limited path)
master: 7.36s
patched: 7.02s (no change)
== Profile breakdown ==
perf profiling of rev-list --count HEAD on the monorepo shows where the time
goes:
master (17.94s): commit_list_insert_by_date 79% 14.25s fixed overhead
(parse/lookup) 21% 3.69s
patched (3.38s): heap ops (compare + sift) 16% 0.53s fixed overhead
(parse/lookup) 84% 2.85s
The queue maintenance itself sped up 27x (14.25s to 0.53s). The overall 5.3x
is lower because the fixed costs -- object lookup (17%), commit-graph
parsing (14%), memory allocation (10%) -- are roughly constant between the
two versions at ~3s.
This means the patch removes the dominant bottleneck entirely. After the
patch, the walk cost is dominated by irreducible per-commit work (parsing
and object lookup) which scales linearly with commit count regardless of
frontier width.
Kristofer Karlsson (3):
pack-objects: call release_revisions() after cruft traversal
revision: introduce rev_walk_mode to clarify get_revision_1()
revision: use priority queue for non-limited streaming walks
builtin/pack-objects.c | 1 +
commit.c | 13 -----
commit.h | 2 -
revision.c | 113 +++++++++++++++++++++++++++--------------
revision.h | 12 ++++-
5 files changed, 88 insertions(+), 53 deletions(-)
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2127%2Fspkrka%2Fstreaming-prio-queue-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2127/spkrka/streaming-prio-queue-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2127
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v3 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl
From: Christian Couder @ 2026-05-27 15:37 UTC (permalink / raw)
To: Kristoffer Haugsbakk
Cc: git, Junio C Hamano, Patrick Steinhardt, Taylor Blau,
Karthik Nayak, Elijah Newren, Toon Claes, Christian Couder
In-Reply-To: <97b9f2cd-7c82-4d4c-b574-31176074e566@app.fastmail.com>
On Sat, May 23, 2026 at 5:17 PM Kristoffer Haugsbakk
<kristofferhaugsbakk@fastmail.com> wrote:
>
> On Tue, May 19, 2026, at 17:38, Christian Couder wrote:
> >[snip]
> >
> > Let's then use this helper in should_accept_remote() so that, a known
> > remote whose URL matches the allowlist is accepted.
>
> I don’t understand this comma break?
I have removed it in the v4 I just sent. Sorry for the confusion.
> > ++
> > +Before matching, both the advertised URL and the pattern are
> > +normalized: the scheme and host are lowercased, percent-encoded
>
> This next paragraph seems to go back to describing how things work. But
> this paragraph as well as all of the following ones belong to this list
> item:
>
> 4. Be careful using globs [...]
>
> Before matching, [...]
>
> The glob pattern can [...]
>
> If a remote with the [...]
>
> For the security implications [...]
>
> promisor.checkFields
> [...]
>
> I don’t know what the intent is. But using an open block will delimit
> the ordered list.
>
> diff --git Documentation/config/promisor.adoc Documentation/config/promisor.adoc
> index cc728bb0b5e..f07a2e883bd 100644
> --- Documentation/config/promisor.adoc
> +++ Documentation/config/promisor.adoc
> @@ -109,6 +109,7 @@ and to update fields (such as authentication tokens) on known remotes
> without further confirmation. To minimize security risks, follow these
> guidelines:
> +
> +--
> 1. Start with a secure protocol scheme, like `https://` or `ssh://`.
> +
> 2. Only allow domain names or paths where you control and trust _ALL_
> @@ -130,6 +131,7 @@ guidelines:
> subdomain. This is extremely dangerous on shared hosting platforms
> (e.g., `https://*.github.io/*` trusts every user's site on the
> entire platform).
> +--
> +
> Before matching, both the advertised URL and the pattern are
> normalized: the scheme and host are lowercased, percent-encoded
Thanks for the suggestion, it is indeed much better this way, and this
is what is used in v4.
^ permalink raw reply
* Re: [PATCH v3 4/8] promisor-remote: add 'local_name' to 'struct promisor_info'
From: Christian Couder @ 2026-05-27 15:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Christian Couder
In-Reply-To: <87a4tvq6pr.fsf@gitster.g>
On Wed, May 20, 2026 at 2:12 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christian Couder <christian.couder@gmail.com> writes:
> > +static const char *promisor_info_internal_name(struct promisor_info *p)
> > +{
> > + return p->local_name ? p->local_name : p->name;
> > +}
>
> Hmph.
>
> > @@ -829,7 +836,7 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised,
> > {
> > struct promisor_info *p;
> > struct string_list_item *item;
> > - const char *remote_name = advertised->name;
> > + const char *remote_name = promisor_info_internal_name(advertised);
>
> Is this really a "remote_name", though? As ...
>
> > @@ -937,7 +944,8 @@ static void filter_promisor_remote(struct repository *repo,
> > /* Apply accepted remotes to the stable repo state */
> > for_each_string_list_item(item, accepted_remotes) {
> > struct promisor_info *info = item->util;
> > - struct promisor_remote *r = repo_promisor_remote_find(repo, info->name);
> > + const char *local = promisor_info_internal_name(info);
>
> ... this name "local" is "the name the thing is locally known to
> us", promisor_info_local_name() might be a better name? I dunno.
> I jsut found it odd that the return value of the same function is
> stored in variables named "remote" and "local" at the same time ;-)
In the v4 I just sent, I renamed promisor_info_internal_name() to
promisor_info_local_name(), and "remote_name" is the name of the local
variable in both places to be more consistent.
Thanks.
^ permalink raw reply
* Re: [PATCH 5/8] pack-bitmap: cache object positions during fill
From: Taylor Blau @ 2026-05-27 14:46 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <20260527094535.GF981444@coredump.intra.peff.net>
On Wed, May 27, 2026 at 05:45:35AM -0400, Jeff King wrote:
> > Combat this by adding a small, direct-mapped cache to the bitmap writer
> > which maps object IDs to their corresponding bit positions. Size the
> > cache according to the number of objects being written, with fixed lower
> > and upper bounds so small repositories do not pay for a large table and
> > large repositories can avoid most repeated packlist and MIDX lookups.
>
> Introducing another layer of data structure feels so dirty, but it's
> hard to argue with the numbers. We are looking up oids in the packlist,
> so it's already O(lg n). Your cache here is essentially a hash lookup,
> which is O(1)-ish (with collisions causing eviction rather than growth).
> And it presumably works because there's a lot of locality in lookups
> (between commits X and X^1, their top-level trees will be almost
> identical but we have to resolve the bits to find out which entries are
> new).
>
> It does make me wonder if we'd see similar improvements if we just
> turned the packlist into a regular hash table. Or maybe not, because
> then we'd have to do actual probing.
I haven't run that experiment directly, but I share your suspicion. I
wrote a 2- and 4-way associative cache implementation as alternatives
before settling on the direct-mapped approach in this patch. I found
that associative caches regardless of cache lines nearly nuked any of
the performance gains that we got as a result of this patch.
> So this really is a somewhat unique situation. It _might_ be applicable
> for the reading side of bitmaps, though. When we do fill-in traversal we
> end up with this same "read a tree, find the bit for each entry, and 99%
> of the time find that it is already in the bitmap".
I think it's certainly likely. In my experience, many object to
bit-position queries are extremely cache-friendly. And in practice, many
large repositories have MIDXs with many tens of millions of objects. So
even on a O(log n) lookup, caching seems to help a lot.
> > In our example repository from above and in earlier commits, this
> > results in a ~9.4% reduction in runtime relative to the previous commit:
> >
> > +------------------+-------------+-------------+---------------------+
> > | | HEAD^ | HEAD | Delta |
> > +------------------+-------------+-------------+---------------------+
> > | elapsed | 324.8 s | 294.1 s | -30.7 s (-9.4%) |
> > | cycles | 1,508.6 B | 1,365.5 B | -143.0 B (-9.5%) |
> > | instructions | 1,436.6 B | 1,389.8 B | -46.9 B (-3.3%) |
> > | CPI | 1.050 | 0.983 | -0.068 (-6.4%) |
> > +------------------+-------------+-------------+---------------------+
>
> I show a 26% speed up on linux.git (1m37 down to 1m12). Very cool.
Glad it reproduces ;-).
> > +static uint32_t store_cached_object_pos(struct bitmap_writer *writer,
> > + const struct object_id *oid,
> > + uint32_t pos)
> > +{
> > + size_t slot;
> > +
> > + if (pos & BITMAP_POS_CACHE_VALID)
> > + return pos; /* too large to cache */
>
> Cute, I wondered what would happen if we went past 2^31. I suspect there
> are other parts of the code that do not behave that well around that
> size, but it is good that we are not introducing any new surprises.
Yeah, I suspect that there are many breakages past the 32-bit unsigned
maximum number of objects. I figure the easiest thing to do in this
patch is to avoid making that situation worse by simply not caching
objects that are near that limit.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 3/8] pack-bitmap: reuse stored selected bitmaps
From: Taylor Blau @ 2026-05-27 14:40 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <20260527092412.GD981444@coredump.intra.peff.net>
On Wed, May 27, 2026 at 05:24:12AM -0400, Jeff King wrote:
> On Tue, May 19, 2026 at 12:12:41PM -0400, Taylor Blau wrote:
>
> > Building bitmaps from scratch on the same test repository from the
> > previous commits yields a significant speed-up:
> >
> > +------------------+-------------+-------------+---------------------+
> > | | HEAD^ | HEAD | Delta |
> > +------------------+-------------+-------------+---------------------+
> > | elapsed | 562.8 s | 324.8 s | -237.9 s (-42.3%) |
> > | cycles | 2,621.3 B | 1,508.6 B | -1,112.7 B (-42.4%) |
> > | instructions | 2,348.9 B | 1,436.6 B | -912.3 B (-38.8%) |
> > | CPI | 1.116 | 1.050 | -0.066 (-5.9%) |
> > +------------------+-------------+-------------+---------------------+
>
> Oh my, that's a rather nice speedup. I can reproduce here on linux.git
> (~47% improvement).
>
> > When `fill_bitmap_commit()` reaches an ancestor that was selected for
> > its own bitmap and processed earlier, its object closure is already
> > stored in `writer->bitmaps` as an EWAH bitmap. As a result, walking
> > through that commit's tree and parents again is redundant.
> >
> > Teach `fill_bitmap_commit()` to notice that case. For non-root commits in
> > the walk, look for a stored selected bitmap and OR it into the bitmap
> > being built. If one exists, skip the commit, its tree, and its parents.
>
> I feel like this _shouldn't_ be necessary, because the idea of the
> current writing code is to go from the roots up, following inverted
> parent pointers, and passing the bitmap up as we go. So whenever we
> visit a commit we should in theory have all of the ancestor's bits set
> in that bitmap. But I remember that the simple-and-stupid approach ended
> up being too memory hungry, so we pick some focal points in the graph
> and then fill them independently.
It's sharing within the non-first parent history that is killing us
here. I think what you said is true in a completely linear repository
with no merges. But since we only pass commit masks from commits to
their first parents, we don't reuse any already-generated bitmaps for
common points in history not shared between commits' first parents.
> I wondered about "c != commit" here. "c" is the commit we're traversing,
> and "commit" is the one for which we're trying to build the bitmap. So
> we would not expect to ever have an entry in writer->bitmaps for "c"
> yet, but the conditional is just short-circuiting the hash lookup.
Exactly.
> The rest of the patch looks obviously correct. The trace2 bits aren't
> strictly necessary, of course, but some metrics might help with further
> tuning.
Yeah, these were for my own curiosity as much as anything. I had written
them as a temporary measure in order to write the "[...] there are 1,261
commits selected for bitmap coverage, and 1,382 maximal commits induced
[...]" portion of the commit message above.
Once I had written it, I found the result useful enough to keep around.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 2/8] pack-bitmap: check subtree bits before recursing
From: Taylor Blau @ 2026-05-27 14:36 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <20260527090348.GC981444@coredump.intra.peff.net>
On Wed, May 27, 2026 at 05:03:48AM -0400, Jeff King wrote:
> On Tue, May 19, 2026 at 12:12:39PM -0400, Taylor Blau wrote:
>
> > In the previous commit, we adjusted the callers of `fill_bitmap_tree()`
> > to pass in the bit position of the tree they wish to fill.
> >
> > This commit makes use of that information at the call site to avoid
> > setting up a stack frame for fill_bitmap_tree() entirely whenever a
> > tree's bit position is already set.
>
> OK, this one at least has a plausible explanation. ;)
>
> I can reproduce your speedup on linux.git (~5% again). I don't love that
> we have to duplicate the logic in each of the callers, but there are
> only two sites (and unlikely to ever be more). And it is only one line,
> the comment notwithstanding. That seems like a good tradeoff for a
> multiple-second speedup.
Yup, exactly. There are naturally only two callers: one to handle a
commit's root tree, and another for recursive calls to handle subtrees.
As a result, I'm OK with the duplication here.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 1/8] pack-bitmap: pass object position to `fill_bitmap_tree()`
From: Taylor Blau @ 2026-05-27 14:36 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <20260527085740.GB981444@coredump.intra.peff.net>
On Wed, May 27, 2026 at 04:57:40AM -0400, Jeff King wrote:
> It is indeed surprising. There's a possible candidate for the speedup
> here:
>
> > @@ -482,8 +479,12 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
> > while (tree_entry(&desc, &entry)) {
> > switch (object_type(entry.mode)) {
> > case OBJ_TREE:
> > + pos = find_object_pos(writer, &entry.oid, &found);
> > + if (!found)
> > + return -1;
> > if (fill_bitmap_tree(writer, bitmap,
> > - lookup_tree(writer->repo, &entry.oid)) < 0)
> > + lookup_tree(writer->repo,
> > + &entry.oid), pos) < 0)
> > return -1;
> > break;
>
> Whenever "found" is false, we cut out early and skip the hash lookup in
> lookup_tree() entirely. But that should almost never happen! It implies
> that a reachable object is not in the pack/midx, and thus the bitmaps is
> not closed (and we'll refuse to generate it).
That's right, and I had actually written something like the following
while developing this patch:
--- 8< ---
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 2d5ff8fd406..328e1c13df3 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -481,7 +481,7 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
case OBJ_TREE:
pos = find_object_pos(writer, &entry.oid, &found);
if (!found)
- return -1;
+ BUG("huh??");
if (fill_bitmap_tree(writer, bitmap,
lookup_tree(writer->repo,
&entry.oid), pos) < 0)
--- >8 ---
, but couldn't trigger it in either the test suite nor in my sample
repository. I left it in there as a sanity measure.
> So it really is the case that we do the same operations in a different
> order. Weird.
Yeah, I puzzled over this for quite a while myself. I really think that
this is reordering produces more favorable cache behavior or codegen
that results in a meaningful speedup.
> But the patch itself looks correct to me, and I get ~6% speedup on a
> from-scratch bitmap generation of linux.git. I guess it could vary
> between architectures and compilers (I'm using gcc on x86), but since
> the reorg is setting us up for further optimizations in the next patch,
> I suppose there's no need to look a gift horse in the mouth.
Good, I'm glad that it was reproducible on your machine. And I agree
;-).
Thanks,
Taylor
^ permalink raw reply related
* [PATCH v4 8/8] doc: promisor: improve acceptFromServer entry
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
The entry for the `promisor.acceptFromServer` in
"Documentation/config/promisor.adoc" has a number of issues:
- it's not clear if new remotes and URLs can be created,
- it looks like a big block of text,
- it's not easy to see all the options,
- it's not easy to see which option is the default one,
- for "knownName", it says "advertised by the client" instead of
"advertised by the server",
- it doesn't refer to the new related `acceptFromServerUrl`
option.
Let's address all these issues by rewording large parts of it
and using bullet points for the different options.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config/promisor.adoc | 53 ++++++++++++++++++++----------
1 file changed, 35 insertions(+), 18 deletions(-)
diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
index 455ce40be8..f07a2e883b 100644
--- a/Documentation/config/promisor.adoc
+++ b/Documentation/config/promisor.adoc
@@ -32,24 +32,41 @@ variable is set to "true", and the "name" and "url" fields are always
advertised regardless of this setting.
promisor.acceptFromServer::
- If set to "all", a client will accept all the promisor remotes
- a server might advertise using the "promisor-remote"
- capability. If set to "knownName" the client will accept
- promisor remotes which are already configured on the client
- and have the same name as those advertised by the client. This
- is not very secure, but could be used in a corporate setup
- where servers and clients are trusted to not switch name and
- URLs. If set to "knownUrl", the client will accept promisor
- remotes which have both the same name and the same URL
- configured on the client as the name and URL advertised by the
- server. This is more secure than "all" or "knownName", so it
- should be used if possible instead of those options. Default
- is "none", which means no promisor remote advertised by a
- server will be accepted. By accepting a promisor remote, the
- client agrees that the server might omit objects that are
- lazily fetchable from this promisor remote from its responses
- to "fetch" and "clone" requests from the client. Name and URL
- comparisons are case sensitive. See linkgit:gitprotocol-v2[5].
+ Controls which promisor remotes advertised by a server (using the
+ "promisor-remote" protocol capability) a client will accept. By
+ accepting a promisor remote, the client agrees that the server
+ might omit objects that are lazily fetchable from this promisor
+ remote from its responses to "fetch" and "clone" requests.
++
+Note that this option does not cause new remotes to be automatically
+created in the client's configuration. It only allows remotes which
+are somehow already configured to be trusted for the current
+operation, or their fields to be updated (if `promisor.storeFields` is
+set and the remote already exists locally). To allow Git to
+automatically create and persist new remotes from server
+advertisements, use `promisor.acceptFromServerUrl`.
++
+The available options are:
++
+* `none` (default): No promisor remote advertised by a server will be
+ accepted.
++
+* `knownUrl`: The client will accept promisor remotes that are already
+ configured on the client and have both the same name and the same URL
+ as advertised by the server. This is more secure than `all` or
+ `knownName`, and should be used if possible instead of those options.
++
+* `knownName`: The client will accept promisor remotes that are already
+ configured on the client and have the same name as those advertised
+ by the server. This is not very secure, but could be used in a corporate
+ setup where servers and clients are trusted to not switch names and URLs.
++
+* `all`: The client will accept all the promisor remotes a server might
+ advertise. This is the least secure option and should only be used in
+ fully trusted environments.
++
+Name and URL comparisons are case-sensitive. See linkgit:gitprotocol-v2[5]
+for protocol details.
promisor.acceptFromServerUrl::
A glob pattern to specify which server-advertised URLs a
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 7/8] promisor-remote: auto-configure unknown remotes
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
Previous commits have introduced the `promisor.acceptFromServerUrl`
config variable to allowlist some URLs advertised by a server through
the "promisor-remote" protocol capability.
However the new `promisor.acceptFromServerUrl` mechanism, like the old
`promisor.acceptFromServer` mechanism, still requires a remote to
already exist in the client's local configuration before it can be
accepted. This places a significant manual burden on users to
pre-configure these remotes, and creates friction for administrators
who have to troubleshoot or manually provision these setups for their
teams.
To eliminate this burden, let's automatically create a new `[remote]`
section in the client's config when a server advertises an unknown
remote whose URL matches a `promisor.acceptFromServerUrl` glob pattern.
Concretely, let's add four helpers:
- sanitize_remote_name(): turn an arbitrary URL-derived string into a
valid remote name by replacing non-alphanumeric characters,
collapsing runs of '-', and prepending "promisor-auto-".
- promisor_remote_name_from_url(): normalize the URL and extract
host+port+path to build a human-readable base name, then pass it
through sanitize_remote_name().
- configure_auto_promisor_remote(): write the remote.*.url,
remote.*.promisor and remote.*.advertisedAs keys to the repo
config.
- handle_matching_allowed_url(): pick the final name (user-supplied
alias or auto-generated), handle collisions by appending "-1",
"-2", etc., then call configure_auto_promisor_remote().
Let's also add should_accept_new_remote_url() which reuses the
url_matches_accept_list() helper introduced in a previous commit to
find a matching pattern, then delegates to handle_matching_allowed_url()
to create the remote.
And then let's call should_accept_new_remote_url() from the '!item'
(unknown remote) branch of should_accept_remote(), setting
`reload_config` so that the newly-written config is picked up.
Finally let's document all that by:
- expanding the `promisor.acceptFromServerUrl` entry to describe
auto-creation, the optional "name=" prefix syntax, the
"promisor-auto-*" generation rules, and numeric-suffix collision
handling, and by
- adding a "remote.<name>.advertisedAs" entry to "remote.adoc".
Also let's extend the precedence paragraph added by a previous commit
to mention this new acceptance path: until now, the only way for
`promisor.acceptFromServerUrl` to trigger acceptance was to allow
field updates for a known remote. With this commit, it can also trigger
auto-creation of a previously-unknown remote whose advertised URL
matches the allowlist.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config/promisor.adoc | 39 +++--
Documentation/config/remote.adoc | 9 ++
promisor-remote.c | 201 +++++++++++++++++++++++++-
t/t5710-promisor-remote-capability.sh | 104 +++++++++++++
4 files changed, 340 insertions(+), 13 deletions(-)
diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
index 605473c82f..455ce40be8 100644
--- a/Documentation/config/promisor.adoc
+++ b/Documentation/config/promisor.adoc
@@ -54,7 +54,8 @@ promisor.acceptFromServer::
promisor.acceptFromServerUrl::
A glob pattern to specify which server-advertised URLs a
client is allowed to act on. When a URL matches, the client
- will accept the advertised remote as a promisor remote and may
+ will accept the advertised remote as a promisor remote, may
+ automatically create a new remote configuration for it and may
automatically accept field updates (such as authentication
tokens) from the server, even if `promisor.acceptFromServer`
is set to `none` (the default).
@@ -65,12 +66,13 @@ this option in _ANY_ config file read by Git.
+
When both `promisor.acceptFromServer` and `promisor.acceptFromServerUrl`
are set, `promisor.acceptFromServerUrl` is consulted first and takes
-precedence: if a matching pattern leads to acceptance (by accepting
-field updates for a known remote whose URL matches both the local
-configuration and the allowlist), the advertised remote is accepted
-regardless of the `promisor.acceptFromServer` setting. If no pattern
-in `promisor.acceptFromServerUrl` triggers acceptance, the decision
-is left to `promisor.acceptFromServer`.
+precedence: if a matching pattern leads to acceptance (either by
+auto-configuring an unknown remote or by accepting field updates for
+a known remote whose URL matches both the local configuration and the
+allowlist), the advertised remote is accepted regardless of the
+`promisor.acceptFromServer` setting. If no pattern in
+`promisor.acceptFromServerUrl` triggers acceptance, the decision is
+left to `promisor.acceptFromServer`.
+
Note however that, even when an advertised URL matches a pattern in
`promisor.acceptFromServerUrl`, an already-existing remote on the
@@ -85,9 +87,10 @@ documentation of that option.)
Be _VERY_ careful with these patterns: `*` matches any sequence of
characters within the 'host' and 'path' parts of a URL (but cannot
cross part boundaries). An overly broad pattern is a major security
-risk, as a matching URL allows a server to update fields (such as
-authentication tokens) on known remotes without further confirmation.
-To minimize security risks, follow these guidelines:
+risk, as a matching URL allows a server to auto-configure new remotes
+and to update fields (such as authentication tokens) on known remotes
+without further confirmation. To minimize security risks, follow these
+guidelines:
+
--
1. Start with a secure protocol scheme, like `https://` or `ssh://`.
@@ -123,6 +126,22 @@ ignored during matching. Note that embedding credentials in URLs is
discouraged. Passing authentication tokens via the `token` field of
the `promisor-remote` capability is strongly preferred.
+
+The glob pattern can optionally be prefixed with a remote name and an
+equals sign (e.g., `cdn=https://cdn.example.com/*`). If such a prefix
+is provided, accepted remotes will be saved under that name. If no
+such prefix is provided, a safe remote name will be automatically
+generated by sanitizing the URL and prefixing it with
+`promisor-auto-`.
++
+If a remote with the chosen name already exists but points to a
+different URL, Git will append a numeric suffix (e.g., `-1`, `-2`) to
+the name to prevent overwriting existing configurations. You should
+make sure that this doesn't happen often though, as remotes will be
+rejected if the numeric suffix increases too much. In all cases, the
+original name advertised by the server is recorded in the
+`remote.<name>.advertisedAs` configuration variable for tracing and
+debugging purposes.
++
For the security implications of accepting a promisor remote, see the
documentation of `promisor.acceptFromServer`. For details on the
protocol, see linkgit:gitprotocol-v2[5].
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 91e46f66f5..6e2bbdf457 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -91,6 +91,15 @@ remote.<name>.promisor::
When set to true, this remote will be used to fetch promisor
objects.
+remote.<name>.advertisedAs::
+ When a promisor remote is automatically configured using
+ information advertised by a server through the
+ `promisor-remote` protocol capability (see
+ `promisor.acceptFromServerUrl`), the server's originally
+ advertised name is saved in this variable. This is for
+ information, tracing and debugging purposes. Users should not
+ typically modify or create such configuration entries.
+
remote.<name>.partialclonefilter::
The filter that will be applied when fetching from this promisor remote.
Changing or clearing this value will only affect fetches for new commits.
diff --git a/promisor-remote.c b/promisor-remote.c
index 04a5bb9939..8fb5e40f67 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -813,10 +813,197 @@ static struct allowed_url *url_matches_accept_list(
return NULL;
}
-static int should_accept_remote(enum accept_promisor accept,
+/*
+ * Sanitize the buffer to make it a valid remote name coming from the
+ * server by:
+ *
+ * - replacing any non alphanumeric character with a '-'
+ * - stripping any leading '-',
+ * - condensing multiple '-' into one,
+ * - prepending "promisor-auto-",
+ * - validating the result.
+ */
+static int sanitize_remote_name(struct strbuf *buf, const char *url)
+{
+ char prev = '-';
+ for (size_t i = 0; i < buf->len; ) {
+ if (!isalnum(buf->buf[i]))
+ buf->buf[i] = '-';
+ if (prev == '-' && buf->buf[i] == '-') {
+ strbuf_remove(buf, i, 1);
+ } else {
+ prev = buf->buf[i];
+ i++;
+ }
+ }
+
+ strbuf_strip_suffix(buf, "-");
+
+ if (!buf->len) {
+ warning(_("couldn't generate a valid remote name from "
+ "advertised url '%s', ignoring this remote"), url);
+ return -1;
+ }
+
+ strbuf_insertstr(buf, 0, "promisor-auto-");
+
+ if (!valid_remote_name(buf->buf)) {
+ warning(_("generated remote name '%s' from advertised url '%s' "
+ "is invalid, ignoring this remote"), buf->buf, url);
+ return -1;
+ }
+
+ return 0;
+}
+
+static char *promisor_remote_name_from_url(const char *url)
+{
+ struct url_info url_info = { 0 };
+ char *normalized = url_normalize(url, &url_info);
+ struct strbuf buf = STRBUF_INIT;
+
+ if (!normalized) {
+ warning(_("couldn't normalize advertised url '%s', "
+ "ignoring this remote"), url);
+ return NULL;
+ }
+
+ if (url_info.host_len) {
+ strbuf_add(&buf, normalized + url_info.host_off, url_info.host_len);
+ strbuf_addch(&buf, '-');
+ }
+
+ if (url_info.port_len) {
+ strbuf_add(&buf, normalized + url_info.port_off, url_info.port_len);
+ strbuf_addch(&buf, '-');
+ }
+
+ if (url_info.path_len) {
+ strbuf_add(&buf, normalized + url_info.path_off, url_info.path_len);
+ strbuf_trim_trailing_dir_sep(&buf);
+ strbuf_strip_suffix(&buf, ".git");
+ }
+
+ free(normalized);
+
+ if (sanitize_remote_name(&buf, url)) {
+ strbuf_release(&buf);
+ return NULL;
+ }
+
+ return strbuf_detach(&buf, NULL);
+}
+
+static void configure_auto_promisor_remote(struct repository *repo,
+ const char *name,
+ const char *url,
+ const char *advertised_as,
+ bool reuse)
+{
+ char *key;
+
+ if (!reuse) {
+ fprintf(stderr, _("Auto-creating promisor remote '%s' for URL '%s'\n"),
+ name, url);
+
+ key = xstrfmt("remote.%s.url", name);
+ repo_config_set_gently(repo, key, url);
+ free(key);
+ }
+
+ /* NB: when reusing, this promotes an existing non-promisor remote */
+ key = xstrfmt("remote.%s.promisor", name);
+ repo_config_set_gently(repo, key, "true");
+ free(key);
+
+ if (advertised_as) {
+ key = xstrfmt("remote.%s.advertisedAs", name);
+ repo_config_set_gently(repo, key, advertised_as);
+ free(key);
+ }
+}
+
+#define MAX_REMOTES_WITH_SIMILAR_NAMES 20
+
+/* Return the allocated local name, or NULL on failure */
+static char *handle_matching_allowed_url(struct repository *repo,
+ char *allowed_name,
+ const char *remote_url,
+ const char *remote_name)
+{
+ char *name;
+ char *basename = allowed_name ?
+ xstrdup(allowed_name) :
+ promisor_remote_name_from_url(remote_url);
+ int i = 0;
+ bool reuse = false;
+
+ if (!basename)
+ return NULL;
+
+ name = xstrdup(basename);
+
+ while (i < MAX_REMOTES_WITH_SIMILAR_NAMES) {
+ char *url_key = xstrfmt("remote.%s.url", name);
+ const char *existing_url;
+ int exists = !repo_config_get_string_tmp(repo, url_key, &existing_url);
+
+ free(url_key);
+
+ if (!exists)
+ break; /* Free to use */
+
+ if (!strcmp(existing_url, remote_url)) {
+ reuse = true;
+ break; /* Same URL, so safe to reuse */
+ }
+
+ i++;
+ free(name);
+ name = xstrfmt("%s-%d", basename, i);
+ }
+
+ if (i < MAX_REMOTES_WITH_SIMILAR_NAMES) {
+ configure_auto_promisor_remote(repo, name,
+ remote_url, remote_name,
+ reuse);
+ } else {
+ warning(_("too many remotes accepted with name like '%s-X', "
+ "ignoring this remote"), basename);
+ FREE_AND_NULL(name);
+ }
+
+ free(basename);
+ return name;
+}
+
+static int should_accept_new_remote_url(struct repository *repo,
+ struct string_list *accept_urls,
+ struct promisor_info *advertised)
+{
+ struct allowed_url *allowed = url_matches_accept_list(accept_urls,
+ advertised->url);
+ if (allowed) {
+ char *name = handle_matching_allowed_url(repo,
+ allowed->remote_name,
+ advertised->url,
+ advertised->name);
+ if (name) {
+ free((char *)advertised->local_name);
+ advertised->local_name = name;
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static int should_accept_remote(struct repository *repo,
+ enum accept_promisor accept,
struct promisor_info *advertised,
struct string_list *accept_urls,
- struct string_list *config_info)
+ struct string_list *config_info,
+ bool *reload_config)
{
struct promisor_info *p;
struct string_list_item *item;
@@ -833,6 +1020,13 @@ static int should_accept_remote(enum accept_promisor accept,
if (!item) {
/* We don't know about that remote */
+
+ int res = should_accept_new_remote_url(repo, accept_urls, advertised);
+ if (res) {
+ *reload_config = true;
+ return res;
+ }
+
if (accept == ACCEPT_ALL)
return all_fields_match(advertised, config_info, NULL);
return 0;
@@ -1093,7 +1287,8 @@ static void filter_promisor_remote(struct repository *repo,
string_list_sort(&config_info);
}
- if (should_accept_remote(accept, advertised, &accept_urls, &config_info)) {
+ if (should_accept_remote(repo, accept, advertised, &accept_urls,
+ &config_info, &reload_config)) {
if (!store_info)
store_info = store_info_new(repo);
if (promisor_store_advertised_fields(advertised, store_info))
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 0659b2ac15..549acff23f 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -458,6 +458,107 @@ test_expect_success "clone with 'None', URL allowlisted, but client has differen
initialize_server 1 "$oid"
'
+test_expect_success "clone with URL allowlisted and no remote already configured" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+ test_when_finished "rm -f full_names" &&
+
+ GIT_NO_LAZY_FETCH=0 git clone \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that exactly one remote has been auto-created, identified
+ # by "remote.<name>.advertisedAs" == "lop".
+ git -C client config get --all --show-names --regexp \
+ "remote\..*\.advertisedas" >full_names &&
+ test_line_count = 1 full_names &&
+ REMOTE_NAME=$(sed "s/^remote\.\(.*\)\.advertisedas .*$/\1/" full_names) &&
+
+ # Check ".url" and ".promisor" values
+ printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" >expect &&
+ git -C client config "remote.$REMOTE_NAME.url" >actual &&
+ git -C client config "remote.$REMOTE_NAME.promisor" >>actual &&
+ test_cmp expect actual &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "clone with named URL allowlisted and no pre-configured remote" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ GIT_NO_LAZY_FETCH=0 git clone \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="cdn=$ENCODED_TRASH_DIRECTORY_URL/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that a remote has been auto-created with the right "cdn" name and fields.
+ printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" "lop" >expect &&
+ git -C client config "remote.cdn.url" >actual &&
+ git -C client config "remote.cdn.promisor" >>actual &&
+ git -C client config "remote.cdn.advertisedAs" >>actual &&
+ test_cmp expect actual &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "clone with URL allowlisted but colliding name" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ GIT_NO_LAZY_FETCH=0 git clone -c remote.cdn.promisor=true \
+ -c remote.cdn.fetch="+refs/heads/*:refs/remotes/lop/*" \
+ -c remote.cdn.url="https://example.com/cdn" \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="cdn=$ENCODED_TRASH_DIRECTORY_URL/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that a remote has been auto-created with the right "cdn-1" name and fields.
+ printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" "lop" >expect &&
+ git -C client config "remote.cdn-1.url" >actual &&
+ git -C client config "remote.cdn-1.promisor" >>actual &&
+ git -C client config "remote.cdn-1.advertisedAs" >>actual &&
+ test_cmp expect actual &&
+
+ # Check that the original "cdn" remote was not overwritten.
+ printf "%s\n" "https://example.com/cdn" "true" >expect &&
+ git -C client config "remote.cdn.url" >actual &&
+ git -C client config "remote.cdn.promisor" >>actual &&
+ test_cmp expect actual &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "clone with URL allowlisted and reusable remote" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ GIT_NO_LAZY_FETCH=0 git clone \
+ -c remote.cdn.fetch="+refs/heads/*:refs/remotes/lop/*" \
+ -c remote.cdn.url="$TRASH_DIRECTORY_URL/lop" \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="cdn=$ENCODED_TRASH_DIRECTORY_URL/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that the existing "cdn" remote has been properly updated.
+ printf "%s\n" "$TRASH_DIRECTORY_URL/lop" "true" "lop" "+refs/heads/*:refs/remotes/lop/*" >expect &&
+ git -C client config "remote.cdn.url" >actual &&
+ git -C client config "remote.cdn.promisor" >>actual &&
+ git -C client config "remote.cdn.advertisedAs" >>actual &&
+ git -C client config "remote.cdn.fetch" >>actual &&
+ test_cmp expect actual &&
+
+ # Check that no new "cdn-1" remote has been created.
+ test_must_fail git -C client config "remote.cdn-1.url" &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
test_expect_success "clone with invalid promisor.acceptFromServerUrl" '
git -C server config promisor.advertise true &&
test_when_finished "rm -rf client" &&
@@ -472,6 +573,9 @@ test_expect_success "clone with invalid promisor.acceptFromServerUrl" '
# Check that a warning was emitted
test_grep "invalid remote name '\''bad name'\''" err &&
+ # Check that no remote was auto-created
+ test_must_fail git -C client config get --regexp "remote\..*\.advertisedas" &&
+
# Check that the largest object is not missing on the server
check_missing_objects server 0 "" &&
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 6/8] promisor-remote: trust known remotes matching acceptFromServerUrl
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
A previous commit introduced the `promisor.acceptFromServerUrl` config
variable along with the machinery to parse and validate the URL glob
patterns and optional remote name prefixes it contains. However, these
URL patterns are not yet tied into the client's acceptance logic.
When a promisor remote is already configured locally, its fields (like
authentication tokens) may occasionally need to be refreshed by the
server. If `promisor.acceptFromServer` is set to the secure default
("None"), these updates are rejected, potentially causing future
fetches to fail.
To enable such targeted updates for trusted URLs, let's use the URL
patterns from `promisor.acceptFromServerUrl` as an additional URL
based allowlist.
Concretely, let's check the advertised URLs against the URL glob
patterns by introducing a new small helper function called
url_matches_accept_list(), which iterates over the glob patterns and
returns the first matching allowed_url entry (or NULL).
The URL matching is done component by component: scheme and port are
compared exactly, the host and path are matched with wildmatch().
Before matching, the advertised URL is passed through url_normalize()
so that case variations in the scheme/host, percent-encoding tricks,
and ".." path segments cannot bypass the allowlist.
The username and password components of the URL are intentionally
ignored during matching to allow servers to rotate them, though using
the 'token' field of the capability is preferred over embedding
credentials in the URL.
Let's then use this helper in should_accept_remote() so that a known
remote whose URL matches the allowlist is accepted.
To prepare for this new logic, let's also:
- Add an 'accept_urls' parameter to should_accept_remote().
- Replace the BUG() guard in the ACCEPT_KNOWN_URL case with an
explicit 'if (accept == ACCEPT_KNOWN_URL) return' and a new
BUG() guard in the ACCEPT_NONE case.
- Call accept_from_server_url() from filter_promisor_remote()
and relax its early return so that the function is entered when
`accept_urls` has entries even if `accept == ACCEPT_NONE`.
With this, many organizations may only need something like:
git config set --global \
promisor.acceptFromServerUrl "https://my-org.com/*"
to accept only their own remotes. And if they need to accept additional
remotes in some specific repos, they can also set:
git config set promisor.acceptFromServer knownUrl
and configure the additional remote manually only in the repos where
they are needed.
Let's then properly document `promisor.acceptFromServerUrl` in
"promisor.adoc" as an additive security allowlist for known remotes,
including the URL normalization behavior and the component-wise
matching, and let's mention it in "gitprotocol-v2.adoc".
Also let's clarify in the documentation how
`promisor.acceptFromServerUrl` interacts with
`promisor.acceptFromServer`:
- Precedence: when both options are set,
`promisor.acceptFromServerUrl` is consulted first. If a matching
pattern leads to acceptance, the remote is accepted regardless of
`promisor.acceptFromServer`. Otherwise the decision is left to
`promisor.acceptFromServer`.
- URL-mismatch guard: even when the advertised URL matches the
allowlist, an already-existing client-side remote whose configured
URL differs from the advertised one is not accepted through
`promisor.acceptFromServerUrl`. `promisor.acceptFromServer=all` and
`=knownName` keep their pre-existing, looser semantics.
The precedence paragraph is intentionally scoped here to known remotes
only (field updates). A following commit that introduces auto-creation
of unknown remotes will extend it to cover that case as well.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config/promisor.adoc | 76 +++++++++++++++++++
Documentation/gitprotocol-v2.adoc | 9 ++-
promisor-remote.c | 102 +++++++++++++++++++++++---
t/t5710-promisor-remote-capability.sh | 71 ++++++++++++++++++
4 files changed, 244 insertions(+), 14 deletions(-)
diff --git a/Documentation/config/promisor.adoc b/Documentation/config/promisor.adoc
index b0fa43b839..605473c82f 100644
--- a/Documentation/config/promisor.adoc
+++ b/Documentation/config/promisor.adoc
@@ -51,6 +51,82 @@ promisor.acceptFromServer::
to "fetch" and "clone" requests from the client. Name and URL
comparisons are case sensitive. See linkgit:gitprotocol-v2[5].
+promisor.acceptFromServerUrl::
+ A glob pattern to specify which server-advertised URLs a
+ client is allowed to act on. When a URL matches, the client
+ will accept the advertised remote as a promisor remote and may
+ automatically accept field updates (such as authentication
+ tokens) from the server, even if `promisor.acceptFromServer`
+ is set to `none` (the default).
++
+This option can appear multiple times in config files. An advertised
+URL will be accepted if it matches _ANY_ glob pattern specified by
+this option in _ANY_ config file read by Git.
++
+When both `promisor.acceptFromServer` and `promisor.acceptFromServerUrl`
+are set, `promisor.acceptFromServerUrl` is consulted first and takes
+precedence: if a matching pattern leads to acceptance (by accepting
+field updates for a known remote whose URL matches both the local
+configuration and the allowlist), the advertised remote is accepted
+regardless of the `promisor.acceptFromServer` setting. If no pattern
+in `promisor.acceptFromServerUrl` triggers acceptance, the decision
+is left to `promisor.acceptFromServer`.
++
+Note however that, even when an advertised URL matches a pattern in
+`promisor.acceptFromServerUrl`, an already-existing remote on the
+client whose name matches the advertised name but whose configured URL
+differs from the advertised one will _NOT_ be accepted through
+`promisor.acceptFromServerUrl`. This prevents a server from silently
+re-pointing an existing client-side remote at a different URL. (Such a
+remote may still be accepted through `promisor.acceptFromServer=all`
+or `=knownName`, which have their own, looser semantics; see the
+documentation of that option.)
++
+Be _VERY_ careful with these patterns: `*` matches any sequence of
+characters within the 'host' and 'path' parts of a URL (but cannot
+cross part boundaries). An overly broad pattern is a major security
+risk, as a matching URL allows a server to update fields (such as
+authentication tokens) on known remotes without further confirmation.
+To minimize security risks, follow these guidelines:
++
+--
+1. Start with a secure protocol scheme, like `https://` or `ssh://`.
++
+2. Only allow domain names or paths where you control and trust _ALL_
+ the content. Be especially careful with shared hosting platforms
+ like `github.com` or `gitlab.com`. A broad pattern like
+ `https://gitlab.com/*` is dangerous because it trusts every
+ repository on the entire platform. Always restrict such patterns to
+ your specific organization or namespace (e.g.,
+ `https://gitlab.com/your-org/*`).
++
+3. Never use globs at the end of domain names. For example,
+ `https://cdn.your-org.com/*` might be safe, but
+ `https://cdn.your-org.com*/*` is a major security risk because
+ the latter matches `https://cdn.your-org.com.hacker.net/repo`.
++
+4. Be careful using globs at the beginning of domain names. While the
+ code ensures a `*` in the host cannot cross into the path, a
+ pattern like `https://*.example.com/*` will still match any
+ subdomain. This is extremely dangerous on shared hosting platforms
+ (e.g., `https://*.github.io/*` trusts every user's site on the
+ entire platform).
+--
++
+Before matching, both the advertised URL and the pattern are
+normalized: the scheme and host are lowercased, percent-encoded
+characters are decoded where possible, and path segments like `..`
+are resolved. The port must also match exactly (e.g.,
+`https://example.com:8080/*` will not match a URL advertised on
+port 9999). The username and password components of the URL are
+ignored during matching. Note that embedding credentials in URLs is
+discouraged. Passing authentication tokens via the `token` field of
+the `promisor-remote` capability is strongly preferred.
++
+For the security implications of accepting a promisor remote, see the
+documentation of `promisor.acceptFromServer`. For details on the
+protocol, see linkgit:gitprotocol-v2[5].
+
promisor.checkFields::
A comma or space separated list of additional remote related
field names. A client checks if the values of these fields
diff --git a/Documentation/gitprotocol-v2.adoc b/Documentation/gitprotocol-v2.adoc
index befa697d21..2beb70595f 100644
--- a/Documentation/gitprotocol-v2.adoc
+++ b/Documentation/gitprotocol-v2.adoc
@@ -866,10 +866,11 @@ the server advertised, the client shouldn't advertise the
On the server side, the "promisor.advertise" and "promisor.sendFields"
configuration options can be used to control what it advertises. On
-the client side, the "promisor.acceptFromServer" configuration option
-can be used to control what it accepts, and the "promisor.storeFields"
-option, to control what it stores. See the documentation of these
-configuration options in linkgit:git-config[1] for more information.
+the client side, the "promisor.acceptFromServer" and
+"promisor.acceptFromServerUrl" configuration options can be used to
+control what it accepts, and the "promisor.storeFields" option, to
+control what it stores. See the documentation of these configuration
+options in linkgit:git-config[1] for more information.
Note that in the future it would be nice if the "promisor-remote"
protocol capability could be used by the server, when responding to
diff --git a/promisor-remote.c b/promisor-remote.c
index 8d4f6e0a72..04a5bb9939 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -14,6 +14,7 @@
#include "url.h"
#include "urlmatch.h"
#include "version.h"
+#include "wildmatch.h"
struct promisor_remote_config {
struct promisor_remote *promisors;
@@ -742,8 +743,79 @@ static void load_accept_from_server_url(struct repository *repo,
}
}
+static bool match_pattern_url(const char *pat, size_t pat_len,
+ const char *url, size_t url_len)
+{
+ char *p_str = xstrndup(pat, pat_len);
+ char *u_str = xstrndup(url, url_len);
+ bool res = !wildmatch(p_str, u_str, 0);
+
+ free(p_str);
+ free(u_str);
+
+ return res;
+}
+
+static bool match_one_url(const struct url_info *pi, const struct url_info *ui)
+{
+ const char *pat = pi->url;
+ const char *url = ui->url;
+
+ /*
+ * Schemes must match exactly. They are case-folded by
+ * url_normalize(), so strncmp() suffices.
+ */
+ if (pi->scheme_len != ui->scheme_len || strncmp(pat, url, pi->scheme_len))
+ return false;
+
+ /*
+ * Ports must match exactly. url_normalize() strips default
+ * ports (like 443 for https), so length and content
+ * comparisons are sufficient.
+ */
+ if (pi->port_len != ui->port_len ||
+ strncmp(pat + pi->port_off, url + ui->port_off, pi->port_len))
+ return false;
+
+ /*
+ * Match host and path separately to prevent a '*' in the host
+ * portion of the pattern from matching across the '/'
+ * boundary into the path.
+ */
+
+ return match_pattern_url(pat + pi->host_off, pi->host_len,
+ url + ui->host_off, ui->host_len) &&
+ match_pattern_url(pat + pi->path_off, pi->path_len,
+ url + ui->path_off, ui->path_len);
+}
+
+static struct allowed_url *url_matches_accept_list(
+ struct string_list *accept_urls, const char *url)
+{
+ struct string_list_item *item;
+ struct url_info url_info;
+
+ url_info.url = url_normalize(url, &url_info);
+
+ if (!url_info.url)
+ return NULL;
+
+ for_each_string_list_item(item, accept_urls) {
+ struct allowed_url *allowed = item->util;
+
+ if (match_one_url(&allowed->pattern_info, &url_info)) {
+ free(url_info.url);
+ return allowed;
+ }
+ }
+
+ free(url_info.url);
+ return NULL;
+}
+
static int should_accept_remote(enum accept_promisor accept,
struct promisor_info *advertised,
+ struct string_list *accept_urls,
struct string_list *config_info)
{
struct promisor_info *p;
@@ -756,23 +828,27 @@ static int should_accept_remote(enum accept_promisor accept,
"this remote should have been rejected earlier",
remote_name);
- if (accept == ACCEPT_ALL)
- return all_fields_match(advertised, config_info, NULL);
-
/* Get config info for that promisor remote */
item = string_list_lookup(config_info, remote_name);
- if (!item)
+ if (!item) {
/* We don't know about that remote */
+ if (accept == ACCEPT_ALL)
+ return all_fields_match(advertised, config_info, NULL);
return 0;
+ }
p = item->util;
- if (accept == ACCEPT_KNOWN_NAME)
+ /* Known remote in the allowlist? */
+ if (!strcmp(p->url, remote_url) && url_matches_accept_list(accept_urls, remote_url))
return all_fields_match(advertised, config_info, p);
- if (accept != ACCEPT_KNOWN_URL)
- BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
+ if (accept == ACCEPT_ALL)
+ return all_fields_match(advertised, config_info, NULL);
+
+ if (accept == ACCEPT_KNOWN_NAME)
+ return all_fields_match(advertised, config_info, p);
if (strcmp(p->url, remote_url)) {
warning(_("known remote named '%s' but with URL '%s' instead of '%s', "
@@ -781,7 +857,13 @@ static int should_accept_remote(enum accept_promisor accept,
return 0;
}
- return all_fields_match(advertised, config_info, p);
+ if (accept == ACCEPT_KNOWN_URL)
+ return all_fields_match(advertised, config_info, p);
+
+ if (accept != ACCEPT_NONE)
+ BUG("Unhandled 'enum accept_promisor' value '%d'", accept);
+
+ return 0;
}
static int skip_field_name_prefix(const char *elem, const char *field_name, const char **value)
@@ -991,7 +1073,7 @@ static void filter_promisor_remote(struct repository *repo,
/* Load and validate the acceptFromServerUrl config */
load_accept_from_server_url(repo, &accept_urls);
- if (accept == ACCEPT_NONE)
+ if (accept == ACCEPT_NONE && !accept_urls.nr)
return;
/* Parse remote info received */
@@ -1011,7 +1093,7 @@ static void filter_promisor_remote(struct repository *repo,
string_list_sort(&config_info);
}
- if (should_accept_remote(accept, advertised, &config_info)) {
+ if (should_accept_remote(accept, advertised, &accept_urls, &config_info)) {
if (!store_info)
store_info = store_info_new(repo);
if (promisor_store_advertised_fields(advertised, store_info))
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index 3b39505380..0659b2ac15 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -387,6 +387,77 @@ test_expect_success "clone with 'KnownUrl' and empty url, so not advertised" '
check_missing_objects server 1 "$oid"
'
+test_expect_success "clone with 'None' but URL allowlisted" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
+ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+ -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "clone with 'None' but URL not in allowlist" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
+ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+ -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="https://example.com/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that the largest object is not missing on the server
+ check_missing_objects server 0 "" &&
+
+ # Reinitialize server so that the largest object is missing again
+ initialize_server 1 "$oid"
+'
+
+test_expect_success "clone with 'None' but URL allowlisted in one pattern out of two" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
+ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+ -c remote.lop.url="$TRASH_DIRECTORY_URL/lop" \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="https://example.com/*" \
+ -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that the largest object is still missing on the server
+ check_missing_objects server 1 "$oid"
+'
+
+test_expect_success "clone with 'None', URL allowlisted, but client has different URL" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ # The client configures "lop" with a different URL (serverTwo) than
+ # what the server advertises (lop). Even though the advertised URL
+ # matches the allowlist, the remote is rejected because the
+ # configured URL does not match the advertised one.
+ GIT_NO_LAZY_FETCH=0 git clone -c remote.lop.promisor=true \
+ -c remote.lop.fetch="+refs/heads/*:refs/remotes/lop/*" \
+ -c remote.lop.url="$TRASH_DIRECTORY_URL/serverTwo" \
+ -c promisor.acceptfromserver=None \
+ -c promisor.acceptFromServerUrl="$ENCODED_TRASH_DIRECTORY_URL/*" \
+ --no-local --filter="blob:limit=5k" server client &&
+
+ # Check that the largest object is not missing on the server
+ check_missing_objects server 0 "" &&
+
+ # Reinitialize server so that the largest object is missing again
+ initialize_server 1 "$oid"
+'
+
test_expect_success "clone with invalid promisor.acceptFromServerUrl" '
git -C server config promisor.advertise true &&
test_when_finished "rm -rf client" &&
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 5/8] promisor-remote: introduce promisor.acceptFromServerUrl
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
The "promisor-remote" protocol capability allows servers to advertise
promisor remotes, but doesn't allow these remotes to be automatically
configured on the client.
Let's introduce a new `promisor.acceptFromServerUrl` config variable
which contains a glob pattern, so that advertised remotes with a URL
matching that pattern will be automatically configured.
The glob pattern can optionally be prefixed with a remote name which
will be used as the name of the new local remote.
For now though, let's only introduce the functions to read and validate
the glob patterns and the optional prefixes.
Checking if the URLs of the advertised remotes match the glob patterns
and taking the appropriate action is left for a following commit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
promisor-remote.c | 90 +++++++++++++++++++++++++++
t/t5710-promisor-remote-capability.sh | 21 +++++++
2 files changed, 111 insertions(+)
diff --git a/promisor-remote.c b/promisor-remote.c
index 138a412893..8d4f6e0a72 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -12,6 +12,7 @@
#include "packfile.h"
#include "environment.h"
#include "url.h"
+#include "urlmatch.h"
#include "version.h"
struct promisor_remote_config {
@@ -657,6 +658,90 @@ static bool has_control_char(const char *s)
return false;
}
+struct allowed_url {
+ char *remote_name;
+ char *url_pattern;
+ struct url_info pattern_info;
+};
+
+static void allowed_url_free(void *util, const char *str UNUSED)
+{
+ struct allowed_url *allowed = util;
+
+ if (!allowed)
+ return;
+
+ /* Depending on prefix, free either remote_name or url_pattern */
+ free(allowed->remote_name ? allowed->remote_name : allowed->url_pattern);
+ free(allowed->pattern_info.url);
+ free(allowed);
+}
+
+static struct allowed_url *valid_accept_url(const char *url)
+{
+ char *dup, *p;
+ struct allowed_url *allowed;
+
+ if (!url)
+ return NULL;
+
+ dup = xstrdup(url);
+ p = strchr(dup, '=');
+ if (p) {
+ *p = '\0';
+ if (!valid_remote_name(dup)) {
+ warning(_("invalid remote name '%s' before '=' sign "
+ "in '%s' from promisor.acceptFromServerUrl config"),
+ dup, url);
+ free(dup);
+ return NULL;
+ }
+ p++;
+ } else {
+ p = dup;
+ }
+
+ if (has_control_char(p)) {
+ warning(_("invalid url pattern '%s' "
+ "in '%s' from promisor.acceptFromServerUrl config"), p, url);
+ free(dup);
+ return NULL;
+ }
+
+ allowed = xmalloc(sizeof(*allowed));
+ allowed->remote_name = (p == dup) ? NULL : dup;
+ allowed->url_pattern = p;
+ allowed->pattern_info.url = url_normalize_pattern(p, &allowed->pattern_info);
+ if (!allowed->pattern_info.url) {
+ warning(_("invalid url pattern '%s' "
+ "in '%s' from promisor.acceptFromServerUrl config"), p, url);
+ free(dup);
+ free(allowed);
+ return NULL;
+ }
+
+ return allowed;
+}
+
+static void load_accept_from_server_url(struct repository *repo,
+ struct string_list *accept_urls)
+{
+ const struct string_list *config_urls;
+
+ if (!repo_config_get_string_multi(repo, "promisor.acceptfromserverurl", &config_urls)) {
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, config_urls) {
+ struct allowed_url *allowed = valid_accept_url(item->string);
+ if (allowed) {
+ struct string_list_item *new;
+ new = string_list_append(accept_urls, item->string);
+ new->util = allowed;
+ }
+ }
+ }
+}
+
static int should_accept_remote(enum accept_promisor accept,
struct promisor_info *advertised,
struct string_list *config_info)
@@ -901,6 +986,10 @@ static void filter_promisor_remote(struct repository *repo,
struct string_list_item *item;
bool reload_config = false;
enum accept_promisor accept = accept_from_server(repo);
+ struct string_list accept_urls = STRING_LIST_INIT_DUP;
+
+ /* Load and validate the acceptFromServerUrl config */
+ load_accept_from_server_url(repo, &accept_urls);
if (accept == ACCEPT_NONE)
return;
@@ -934,6 +1023,7 @@ static void filter_promisor_remote(struct repository *repo,
}
}
+ string_list_clear_func(&accept_urls, allowed_url_free);
promisor_info_list_clear(&config_info);
string_list_clear(&remote_info, 0);
store_info_free(store_info);
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index bf1cc54605..3b39505380 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -387,6 +387,27 @@ test_expect_success "clone with 'KnownUrl' and empty url, so not advertised" '
check_missing_objects server 1 "$oid"
'
+test_expect_success "clone with invalid promisor.acceptFromServerUrl" '
+ git -C server config promisor.advertise true &&
+ test_when_finished "rm -rf client" &&
+
+ # As "bad name" contains a space, which is not a valid remote name,
+ # the pattern should be rejected with a warning and no remote created.
+ GIT_NO_LAZY_FETCH=0 git clone \
+ -c promisor.acceptfromserver=None \
+ -c "promisor.acceptFromServerUrl=bad name=https://example.com/*" \
+ --no-local --filter="blob:limit=5k" server client 2>err &&
+
+ # Check that a warning was emitted
+ test_grep "invalid remote name '\''bad name'\''" err &&
+
+ # Check that the largest object is not missing on the server
+ check_missing_objects server 0 "" &&
+
+ # Reinitialize server so that the largest object is missing again
+ initialize_server 1 "$oid"
+'
+
test_expect_success "clone with promisor.sendFields" '
git -C server config promisor.advertise true &&
test_when_finished "rm -rf client" &&
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 4/8] promisor-remote: add 'local_name' to 'struct promisor_info'
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
In a following commit, we will store promisor remote information under
a remote name different than the one the server advertised.
To prepare for this change, let's add a new 'char *local_name' member
to 'struct promisor_info', and let's update the related functions.
While at it, let's also add a small promisor_info_local_name() helper
that returns `local_name` when set, `name` otherwise, and let's use
this small helper in promisor_store_advertised_fields() and in the
post-loop of filter_promisor_remote() so that lookups against the local
repo configuration use the right name.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
promisor-remote.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/promisor-remote.c b/promisor-remote.c
index 38fa050542..138a412893 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -434,13 +434,14 @@ static struct string_list *fields_stored(void)
* Struct for promisor remotes involved in the "promisor-remote"
* protocol capability.
*
- * Except for "name", each <member> in this struct and its <value>
- * should correspond (either on the client side or on the server side)
- * to a "remote.<name>.<member>" config variable set to <value> where
- * "<name>" is a promisor remote name.
+ * Except for "name" and "local_name", each <member> in this struct
+ * and its <value> should correspond (either on the client side or on
+ * the server side) to a "remote.<name>.<member>" config variable set
+ * to <value> where "<name>" is a promisor remote name.
*/
struct promisor_info {
- const char *name;
+ const char *name; /* name the server advertised */
+ const char *local_name; /* name used locally (may be auto-generated) */
const char *url;
const char *filter;
const char *token;
@@ -449,6 +450,7 @@ struct promisor_info {
static void promisor_info_free(struct promisor_info *p)
{
free((char *)p->name);
+ free((char *)p->local_name);
free((char *)p->url);
free((char *)p->filter);
free((char *)p->token);
@@ -462,6 +464,11 @@ static void promisor_info_list_clear(struct string_list *list)
string_list_clear(list, 0);
}
+static const char *promisor_info_local_name(struct promisor_info *p)
+{
+ return p->local_name ? p->local_name : p->name;
+}
+
static void set_one_field(struct promisor_info *p,
const char *field, const char *value)
{
@@ -829,7 +836,7 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised,
{
struct promisor_info *p;
struct string_list_item *item;
- const char *remote_name = advertised->name;
+ const char *remote_name = promisor_info_local_name(advertised);
bool reload_config = false;
if (!(store_info->store_filter || store_info->store_token))
@@ -937,7 +944,8 @@ static void filter_promisor_remote(struct repository *repo,
/* Apply accepted remotes to the stable repo state */
for_each_string_list_item(item, accepted_remotes) {
struct promisor_info *info = item->util;
- struct promisor_remote *r = repo_promisor_remote_find(repo, info->name);
+ const char *remote_name = promisor_info_local_name(info);
+ struct promisor_remote *r = repo_promisor_remote_find(repo, remote_name);
if (r) {
r->accepted = 1;
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 3/8] urlmatch: add url_normalize_pattern() helper
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
In a following commit, we will need to normalize a URL glob pattern
(which may contain '*' in the host portion) and extract its component
offsets (host, path, etc.) for separate matching. Let's export a
dedicated helper function url_normalize_pattern() for that purpose.
It works like url_normalize(), but passes allow_globs=true to the
internal url_normalize_1(), so that '*' characters in the host are
accepted rather than rejected.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
urlmatch.c | 5 +++++
urlmatch.h | 12 ++++++++++++
2 files changed, 17 insertions(+)
diff --git a/urlmatch.c b/urlmatch.c
index b2d88a5289..20bc2d009c 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -441,6 +441,11 @@ char *url_normalize(const char *url, struct url_info *out_info)
return url_normalize_1(url, out_info, false);
}
+char *url_normalize_pattern(const char *url, struct url_info *out_info)
+{
+ return url_normalize_1(url, out_info, true);
+}
+
char *url_parse(const char *url_orig, struct url_info *out_info)
{
struct strbuf url;
diff --git a/urlmatch.h b/urlmatch.h
index 6b3ce42858..db1a335e72 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -37,6 +37,18 @@ struct url_info {
char *url_normalize(const char *, struct url_info *);
char *url_parse(const char *, struct url_info *);
+/*
+ * Like url_normalize(), but also allows '*' glob characters in the host
+ * portion. Use this when normalizing URL patterns from user configuration.
+ *
+ * Note that '*' is a valid path character per RFC 3986 (as a sub-delim),
+ * so glob patterns using '*' in the path are also accepted.
+ *
+ * Returns a newly allocated normalized string and fills out_info if
+ * non-NULL, or NULL if the pattern is invalid.
+ */
+char *url_normalize_pattern(const char *url, struct url_info *out_info);
+
struct urlmatch_item {
size_t hostmatch_len;
size_t pathmatch_len;
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 2/8] urlmatch: change 'allow_globs' arg to bool
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
The last argument of url_normalize_1() is `char allow_globs` but it is
used as a boolean, not as a char.
Let's convert it to a `bool`, and while at it convert the two calls to
url_normalize_1() so they pass 'true' or 'false' instead of '1' or '0'.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
urlmatch.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/urlmatch.c b/urlmatch.c
index bf8cce6de9..b2d88a5289 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -112,7 +112,7 @@ static int match_host(const struct url_info *url_info,
return (!url_len && !pat_len);
}
-static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
+static char *url_normalize_1(const char *url, struct url_info *out_info, bool allow_globs)
{
/*
* Normalize NUL-terminated url using the following rules:
@@ -438,7 +438,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
char *url_normalize(const char *url, struct url_info *out_info)
{
- return url_normalize_1(url, out_info, 0);
+ return url_normalize_1(url, out_info, false);
}
char *url_parse(const char *url_orig, struct url_info *out_info)
@@ -704,7 +704,7 @@ int urlmatch_config_entry(const char *var, const char *value,
struct url_info norm_info;
config_url = xmemdupz(key, dot - key);
- norm_url = url_normalize_1(config_url, &norm_info, 1);
+ norm_url = url_normalize_1(config_url, &norm_info, true);
if (norm_url)
retval = match_urls(url, &norm_info, &matched);
else if (collect->fallback_match_fn)
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 1/8] t5710: simplify 'mkdir X' followed by 'git -C X init'
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder,
Christian Couder
In-Reply-To: <20260527140820.1438165-1-christian.couder@gmail.com>
It's simpler and more efficient to just use `git init client` instead
of `mkdir client && git -C client init`.
So let's replace the latter with the former.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
t/t5710-promisor-remote-capability.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/t/t5710-promisor-remote-capability.sh b/t/t5710-promisor-remote-capability.sh
index b404ad9f0a..bf1cc54605 100755
--- a/t/t5710-promisor-remote-capability.sh
+++ b/t/t5710-promisor-remote-capability.sh
@@ -177,8 +177,7 @@ test_expect_success "init + fetch with promisor.advertise set to 'true'" '
git -C server config promisor.advertise true &&
test_when_finished "rm -rf client" &&
- mkdir client &&
- git -C client init &&
+ git init client &&
git -C client config remote.lop.promisor true &&
git -C client config remote.lop.fetch "+refs/heads/*:refs/remotes/lop/*" &&
git -C client config remote.lop.url "$TRASH_DIRECTORY_URL/lop" &&
@@ -231,8 +230,7 @@ test_expect_success "init + fetch two promisors but only one advertised" '
# Create a promisor that will be configured but not be used
git init --bare unused_lop &&
- mkdir client &&
- git -C client init &&
+ git init client &&
git -C client config remote.unused_lop.promisor true &&
git -C client config remote.unused_lop.fetch "+refs/heads/*:refs/remotes/unused_lop/*" &&
git -C client config remote.unused_lop.url "$TRASH_DIRECTORY_URL/unused_lop" &&
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply related
* [PATCH v4 0/8] Auto-configure advertised remotes via URL allowlist
From: Christian Couder @ 2026-05-27 14:08 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Kristoffer Haugsbakk, Christian Couder
In-Reply-To: <20260519153808.494105-1-christian.couder@gmail.com>
Currently, the "promisor-remote" protocol capability allows a server
to advertise promisor remotes (and their tokens/filters), but the
client's `promisor.acceptFromServer` mechanism requires these remotes
to already exist in the config.
This is a significant burden for users and administrators who have to
pre-configure remotes.
This patch series improves on this by introducing a new
`promisor.acceptFromServerUrl` config option, which provides an
additive, URL-based security allowlist.
Multiple `promisor.acceptFromServerUrl` config options can be provided
in different config files. Each one should contain a URL glob pattern
which can optionally be prefixed with a remote name in the
"[<name>=]<pattern>" format.
The goal is for something like a simple:
git config set --global promisor.acceptFromServerUrl "https://my-org.com/*"
to be all that is needed for internal work in many organizations.
With this new config option:
- The server can update fields (like tokens) for known remotes,
provided their URL matches the allowlist, even if
`acceptFromServer` is set to `None`.
- Unknown remotes advertised by the server can be automatically
configured on the client if their URL matches the allowlist.
- If there is no `<name>` prefix before the glob pattern matched, the
auto-configured remote is named using the
"promisor-auto-<sanitized-url>" format. So the same auto-configured
remote config entry will be reused for the same URL.
- If a `<name>` prefix is provided, it will be used for the
auto-configured remote config entry.
- If the chosen name (auto-generated or prefixed) already exists but
points to a different URL, overwriting the existing config is
prevented by appending a numeric suffix (e.g., -1, -2) to the name
and auto-configuring using that name.
- The server's originally advertised name is always saved in the
`remote.<name>.advertisedAs` config variable of the auto-configured
remote for tracing and debugging.
Security considerations:
- Advertised URLs and glob patterns are routed through
url_normalize() / url_normalize_pattern() before matching, to
prevent percent-encoding, case variation, or path-traversal (..)
bypasses.
- URL matching is done component by component: scheme and port
must match exactly (no wildcards), the host is matched with
WM_PATHNAME so a '*' cannot cross the '/' boundary into the
path, and the path is matched without WM_PATHNAME so '*' can
still span multi-level paths.
- Auto-generated remote names are sanitized (non-alphanumeric
characters are replaced with '-', runs of '-' are collapsed)
and prefixed with 'promisor-auto-'. User-supplied names (from
the 'name=<pattern>' syntax) are validated with
valid_remote_name(). Together, these prevent a server from
maliciously overwriting standard remotes (like 'origin').
- If the auto-generated or user-supplied name collides with an
existing remote configured to a different URL, a numeric
suffix ('-1', '-2', ...) is appended, up to a bounded limit,
so a server cannot hijack an existing remote by name.
- Known remotes are still subject to URL consistency checks:
even if an advertised URL matches the allowlist, it is only
accepted for a known remote if it matches the URL already
configured locally for that remote.
- The documentation explains in detail how to write secure glob
patterns in `promisor.acceptFromServerUrl`, and highlights the
risks of overly broad patterns on shared hosting platforms.
High level description of the patches
=====================================
- Patch 1/8 is a very small preparatory patch that simplifies some
tests a bit.
- Patches 2/8 and 3/8 expose and adapt a url_normalize_pattern()
helper function in the urlmatch API.
- Patch 4/8 adapts `struct promisor_info` by adding a new
`local_name` member to it to prepare for the next patches.
- Patches 5/8 to 7/8 implement the core feature. They introduce the
parsing machinery, add the additive allowlist for known remotes
(with url_normalize() security), and finally implement the
auto-creation and collision resolution for unknown remotes.
- Patch 8/8 cleans up and modernizes the existing
`promisor.acceptFromServer` documentation.
Changes compared to v3
======================
Thanks to Toon, Kristoffer, Patrick and Junio for reviewing the
previous versions of this series and of the preparatory series.
This has been rebased onto master @ 56a4f3c3a2 (The 8th batch,
2026-05-25) to avoid a trivial conflict in "urlmatch.c".
Only minor changes have been made since v3, in the following patches:
- Patch 4/8 ("promisor-remote: add 'local_name' to 'struct
promisor_info'"):
- The promisor_info_internal_name() function has been renamed
promisor_info_local_name() for clarity.
- A `const char *local` local variable has been renamed
`remote_name` for consistency with another similar variable.
- Patch 6/8 ("promisor-remote: trust known remotes matching
acceptFromServerUrl"):
- A spurious comma in the commit message has been deleted.
- The `promisor.acceptFromServerUrl` documentation in
"Documentation/config/promisor.adoc" now uses `--` to separate a
numbered list from the surrounding paragraphs.
CI tests
========
They all pass, see:
https://github.com/chriscool/git/actions/runs/26514308470
Range diff since v3
===================
1: ab231c0896 = 1: 9fcc7d9d5e t5710: simplify 'mkdir X' followed by 'git -C X init'
2: b3e66f329f ! 2: ec558c2b9c urlmatch: change 'allow_globs' arg to bool
@@ urlmatch.c: static char *url_normalize_1(const char *url, struct url_info *out_i
+ return url_normalize_1(url, out_info, false);
}
- static size_t url_match_prefix(const char *url,
+ char *url_parse(const char *url_orig, struct url_info *out_info)
@@ urlmatch.c: int urlmatch_config_entry(const char *var, const char *value,
struct url_info norm_info;
3: 813d748dd6 ! 3: 79ee353449 urlmatch: add url_normalize_pattern() helper
@@ urlmatch.c: char *url_normalize(const char *url, struct url_info *out_info)
+ return url_normalize_1(url, out_info, true);
+}
+
- static size_t url_match_prefix(const char *url,
- const char *url_prefix,
- size_t url_prefix_len)
+ char *url_parse(const char *url_orig, struct url_info *out_info)
+ {
+ struct strbuf url;
## urlmatch.h ##
@@ urlmatch.h: struct url_info {
-
char *url_normalize(const char *, struct url_info *);
+ char *url_parse(const char *, struct url_info *);
+/*
+ * Like url_normalize(), but also allows '*' glob characters in the host
4: e92863bee8 ! 4: 037fd46ac7 promisor-remote: add 'local_name' to 'struct promisor_info'
@@ Commit message
To prepare for this change, let's add a new 'char *local_name' member
to 'struct promisor_info', and let's update the related functions.
- While at it, let's also add a small promisor_info_internal_name()
- helper that returns `local_name` when set, `name` otherwise, and let's
- use this small helper in promisor_store_advertised_fields() and in the
+ While at it, let's also add a small promisor_info_local_name() helper
+ that returns `local_name` when set, `name` otherwise, and let's use
+ this small helper in promisor_store_advertised_fields() and in the
post-loop of filter_promisor_remote() so that lookups against the local
repo configuration use the right name.
@@ promisor-remote.c: static void promisor_info_list_clear(struct string_list *list
string_list_clear(list, 0);
}
-+static const char *promisor_info_internal_name(struct promisor_info *p)
++static const char *promisor_info_local_name(struct promisor_info *p)
+{
+ return p->local_name ? p->local_name : p->name;
+}
@@ promisor-remote.c: static bool promisor_store_advertised_fields(struct promisor_
struct promisor_info *p;
struct string_list_item *item;
- const char *remote_name = advertised->name;
-+ const char *remote_name = promisor_info_internal_name(advertised);
++ const char *remote_name = promisor_info_local_name(advertised);
bool reload_config = false;
if (!(store_info->store_filter || store_info->store_token))
@@ promisor-remote.c: static void filter_promisor_remote(struct repository *repo,
for_each_string_list_item(item, accepted_remotes) {
struct promisor_info *info = item->util;
- struct promisor_remote *r = repo_promisor_remote_find(repo, info->name);
-+ const char *local = promisor_info_internal_name(info);
-+ struct promisor_remote *r = repo_promisor_remote_find(repo, local);
++ const char *remote_name = promisor_info_local_name(info);
++ struct promisor_remote *r = repo_promisor_remote_find(repo, remote_name);
if (r) {
r->accepted = 1;
5: 7e1b106404 = 5: 532adb7ca9 promisor-remote: introduce promisor.acceptFromServerUrl
6: f00eed4bf2 ! 6: b970f5647c promisor-remote: trust known remotes matching acceptFromServerUrl
@@ Commit message
the 'token' field of the capability is preferred over embedding
credentials in the URL.
- Let's then use this helper in should_accept_remote() so that, a known
+ Let's then use this helper in should_accept_remote() so that a known
remote whose URL matches the allowlist is accepted.
To prepare for this new logic, let's also:
@@ Documentation/config/promisor.adoc: promisor.acceptFromServer::
+authentication tokens) on known remotes without further confirmation.
+To minimize security risks, follow these guidelines:
++
++--
+1. Start with a secure protocol scheme, like `https://` or `ssh://`.
++
+2. Only allow domain names or paths where you control and trust _ALL_
@@ Documentation/config/promisor.adoc: promisor.acceptFromServer::
+ subdomain. This is extremely dangerous on shared hosting platforms
+ (e.g., `https://*.github.io/*` trusts every user's site on the
+ entire platform).
++--
++
+Before matching, both the advertised URL and the pattern are
+normalized: the scheme and host are lowercased, percent-encoded
7: af06fb31db ! 7: 1875228a7b promisor-remote: auto-configure unknown remotes
@@ Documentation/config/promisor.adoc: documentation of that option.)
+without further confirmation. To minimize security risks, follow these
+guidelines:
+
+ --
1. Start with a secure protocol scheme, like `https://` or `ssh://`.
- +
@@ Documentation/config/promisor.adoc: ignored during matching. Note that embedding credentials in URLs is
discouraged. Passing authentication tokens via the `token` field of
the `promisor-remote` capability is strongly preferred.
8: 92075d88d8 = 8: 351ece0b90 doc: promisor: improve acceptFromServer entry
Christian Couder (8):
t5710: simplify 'mkdir X' followed by 'git -C X init'
urlmatch: change 'allow_globs' arg to bool
urlmatch: add url_normalize_pattern() helper
promisor-remote: add 'local_name' to 'struct promisor_info'
promisor-remote: introduce promisor.acceptFromServerUrl
promisor-remote: trust known remotes matching acceptFromServerUrl
promisor-remote: auto-configure unknown remotes
doc: promisor: improve acceptFromServer entry
Documentation/config/promisor.adoc | 148 +++++++--
Documentation/config/remote.adoc | 9 +
Documentation/gitprotocol-v2.adoc | 9 +-
promisor-remote.c | 413 ++++++++++++++++++++++++--
t/t5710-promisor-remote-capability.sh | 202 ++++++++++++-
urlmatch.c | 11 +-
urlmatch.h | 12 +
7 files changed, 756 insertions(+), 48 deletions(-)
--
2.54.0.275.g96c817d129.dirty
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox