From f82e430b4e46120e0ef67959e0ef9d8ab9282c56 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:41:56 +0100 Subject: [PATCH 01/25] odb: fix subtle logic to check whether an alternate is usable When adding an alternate to the object database we first check whether or not the path is usable. A path is usable if: - It actually exists. - We don't have it in our object sources yet. While the former check is trivial enough, the latter part is somewhat subtle and prone for bugs. This is because the function doesn't only check whether or not the given path is usable. But if it _is_ usable, we also store that path in the map of object sources immediately. The tricky part here is that the path that gets stored in the map is _not_ copied. Instead, we rely on the fact that subsequent code uses `strbuf_detach()` to store the exact same allocated memory in the created object source. Consequently, the memory is owned by the source but _also_ stored in the map. This subtlety is easy to miss, so if one decides to refactor this code one can easily end up breaking this mechanism. Make the relationship more explicit by not storing the path as part of `alt_odb_usable()`. Instead, store the path after we have created the source so that we can use the source's path pointer directly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/odb.c b/odb.c index 00a6e71568b598..57d85ed9505e3f 100644 --- a/odb.c +++ b/odb.c @@ -86,17 +86,16 @@ int odb_mkstemp(struct object_database *odb, /* * Return non-zero iff the path is usable as an alternate object database. */ -static int alt_odb_usable(struct object_database *o, - struct strbuf *path, - const char *normalized_objdir, khiter_t *pos) +static int alt_odb_usable(struct object_database *o, const char *path, + const char *normalized_objdir) { int r; /* Detect cases where alternate disappeared */ - if (!is_directory(path->buf)) { + if (!is_directory(path)) { error(_("object directory %s does not exist; " "check .git/objects/info/alternates"), - path->buf); + path); return 0; } @@ -113,11 +112,14 @@ static int alt_odb_usable(struct object_database *o, assert(r == 1); /* never used */ kh_value(o->source_by_path, p) = o->sources; } - if (fspatheq(path->buf, normalized_objdir)) + + if (fspatheq(path, normalized_objdir)) + return 0; + + if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path)) return 0; - *pos = kh_put_odb_path_map(o->source_by_path, path->buf, &r); - /* r: 0 = exists, 1 = never used, 2 = deleted */ - return r == 0 ? 0 : 1; + + return 1; } /* @@ -148,6 +150,7 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb, struct strbuf pathbuf = STRBUF_INIT; struct strbuf tmp = STRBUF_INIT; khiter_t pos; + int ret; if (!is_absolute_path(dir) && relative_base) { strbuf_realpath(&pathbuf, relative_base, 1); @@ -172,20 +175,21 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb, strbuf_reset(&tmp); strbuf_realpath(&tmp, odb->sources->path, 1); - if (!alt_odb_usable(odb, &pathbuf, tmp.buf, &pos)) + if (!alt_odb_usable(odb, pathbuf.buf, tmp.buf)) goto error; CALLOC_ARRAY(alternate, 1); alternate->odb = odb; alternate->local = false; - /* pathbuf.buf is already in r->objects->source_by_path */ alternate->path = strbuf_detach(&pathbuf, NULL); /* add the alternate entry */ *odb->sources_tail = alternate; odb->sources_tail = &(alternate->next); - alternate->next = NULL; - assert(odb->source_by_path); + + pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret); + if (!ret) + BUG("source must not yet exist"); kh_value(odb->source_by_path, pos) = alternate; /* recursively add alternates */ From 0820a4b120f310d87ac8817ade63896a901c9267 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:41:57 +0100 Subject: [PATCH 02/25] odb: introduce `odb_source_new()` We have three different locations where we create a new ODB source. Deduplicate the logic via a new `odb_source_new()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 23 ++++++++++++++++------- odb.h | 4 ++++ repository.c | 14 +++++++++----- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/odb.c b/odb.c index 57d85ed9505e3f..d2d4c514ae5c09 100644 --- a/odb.c +++ b/odb.c @@ -141,6 +141,20 @@ static void read_info_alternates(struct object_database *odb, const char *relative_base, int depth); +struct odb_source *odb_source_new(struct object_database *odb, + const char *path, + bool local) +{ + struct odb_source *source; + + CALLOC_ARRAY(source, 1); + source->odb = odb; + source->local = local; + source->path = xstrdup(path); + + return source; +} + static struct odb_source *link_alt_odb_entry(struct object_database *odb, const char *dir, const char *relative_base, @@ -178,10 +192,7 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb, if (!alt_odb_usable(odb, pathbuf.buf, tmp.buf)) goto error; - CALLOC_ARRAY(alternate, 1); - alternate->odb = odb; - alternate->local = false; - alternate->path = strbuf_detach(&pathbuf, NULL); + alternate = odb_source_new(odb, pathbuf.buf, false); /* add the alternate entry */ *odb->sources_tail = alternate; @@ -341,9 +352,7 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, * Make a new primary odb and link the old primary ODB in as an * alternate */ - source = xcalloc(1, sizeof(*source)); - source->odb = odb; - source->path = xstrdup(dir); + source = odb_source_new(odb, dir, false); /* * Disable ref updates while a temporary odb is active, since diff --git a/odb.h b/odb.h index e6602dd90c833c..2bec895d1352f4 100644 --- a/odb.h +++ b/odb.h @@ -89,6 +89,10 @@ struct odb_source { char *path; }; +struct odb_source *odb_source_new(struct object_database *odb, + const char *path, + bool local); + struct packed_git; struct packfile_store; struct cached_object_entry; diff --git a/repository.c b/repository.c index 6faf5c73981ebf..6aaa7ba00869bf 100644 --- a/repository.c +++ b/repository.c @@ -160,20 +160,24 @@ void repo_set_gitdir(struct repository *repo, * until after xstrdup(root). Then we can free it. */ char *old_gitdir = repo->gitdir; + char *objects_path = NULL; repo->gitdir = xstrdup(gitfile ? gitfile : root); free(old_gitdir); repo_set_commondir(repo, o->commondir); + expand_base_dir(&objects_path, o->object_dir, + repo->commondir, "objects"); if (!repo->objects->sources) { - CALLOC_ARRAY(repo->objects->sources, 1); - repo->objects->sources->odb = repo->objects; - repo->objects->sources->local = true; + repo->objects->sources = odb_source_new(repo->objects, + objects_path, true); repo->objects->sources_tail = &repo->objects->sources->next; + free(objects_path); + } else { + free(repo->objects->sources->path); + repo->objects->sources->path = objects_path; } - expand_base_dir(&repo->objects->sources->path, o->object_dir, - repo->commondir, "objects"); repo->objects->sources->disable_ref_updates = o->disable_ref_updates; From c2da110411919387fef0f869181fb510d06295d8 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:41:58 +0100 Subject: [PATCH 03/25] odb: adjust naming to free object sources The functions `free_object_directory()` and `free_object_directories()` are responsible for freeing a single object source or all object sources connected to an object database, respectively. The associated structure has been renamed from `struct object_directory` to `struct odb_source` in a1e2581a1e (object-store: rename `object_directory` to `odb_source`, 2025-07-01) though, so the names are somewhat stale nowadays. Rename them to mention the new struct name instead. Furthermore, while at it, adapt them to our modern naming schema where we first have the subject followed by a verb. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/odb.c b/odb.c index d2d4c514ae5c09..77490d7fdbeb4b 100644 --- a/odb.c +++ b/odb.c @@ -365,7 +365,7 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, return source->next; } -static void free_object_directory(struct odb_source *source) +static void odb_source_free(struct odb_source *source) { free(source->path); odb_clear_loose_cache(source); @@ -387,7 +387,7 @@ void odb_restore_primary_source(struct object_database *odb, BUG("we expect the old primary object store to be the first alternate"); odb->sources = restore_source; - free_object_directory(cur_source); + odb_source_free(cur_source); } char *compute_alternate_path(const char *path, struct strbuf *err) @@ -1015,13 +1015,13 @@ struct object_database *odb_new(struct repository *repo) return o; } -static void free_object_directories(struct object_database *o) +static void odb_free_sources(struct object_database *o) { while (o->sources) { struct odb_source *next; next = o->sources->next; - free_object_directory(o->sources); + odb_source_free(o->sources); o->sources = next; } kh_destroy_odb_path_map(o->source_by_path); @@ -1039,7 +1039,7 @@ void odb_clear(struct object_database *o) o->commit_graph = NULL; o->commit_graph_attempted = 0; - free_object_directories(o); + odb_free_sources(o); o->sources_tail = NULL; o->loaded_alternates = 0; From 0cc12dedef2885dba8cf2635697767d394baf91f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:41:59 +0100 Subject: [PATCH 04/25] object-file: move `fetch_if_missing` The `fetch_if_missing` global variable is declared in "object-file.h" but defined in "odb.c". The variable relates to the whole object database instead of only loose objects, so move the declaration into "odb.h" accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.h | 8 -------- odb.h | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/object-file.h b/object-file.h index 3fd48dcafbf1dc..097e9764be169e 100644 --- a/object-file.h +++ b/object-file.h @@ -7,14 +7,6 @@ struct index_state; -/* - * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing - * blobs. This has a difference only if extensions.partialClone is set. - * - * Its default value is 1. - */ -extern int fetch_if_missing; - enum { INDEX_WRITE_OBJECT = (1 << 0), INDEX_FORMAT_CHECK = (1 << 1), diff --git a/odb.h b/odb.h index 2bec895d1352f4..2346ffeca85961 100644 --- a/odb.h +++ b/odb.h @@ -14,6 +14,14 @@ struct strbuf; struct repository; struct multi_pack_index; +/* + * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing + * blobs. This has a difference only if extensions.partialClone is set. + * + * Its default value is 1. + */ +extern int fetch_if_missing; + /* * Compute the exact path an alternate is at and returns it. In case of * error NULL is returned and the human readable error is added to `err` From ece43d9dc70b1717484ee78b66aef4f9390c2b2b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:00 +0100 Subject: [PATCH 05/25] object-file: introduce `struct odb_source_loose` Currently, all state that relates to loose objects is held directly by the `struct odb_source`. Introduce a new `struct odb_source_loose` to hold the state instead so that it is entirely self-contained. This structure will eventually morph into the backend for accessing loose objects. As such, this is part of the refactorings to introduce pluggable object databases. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.c | 13 +++++++++++++ object-file.h | 7 +++++++ odb.c | 2 ++ odb.h | 3 +++ 4 files changed, 25 insertions(+) diff --git a/object-file.c b/object-file.c index 4675c8ed6b67eb..cd6aa561fa7db2 100644 --- a/object-file.c +++ b/object-file.c @@ -1995,3 +1995,16 @@ void object_file_transaction_commit(struct odb_transaction *transaction) transaction->odb->transaction = NULL; free(transaction); } + +struct odb_source_loose *odb_source_loose_new(struct odb_source *source) +{ + struct odb_source_loose *loose; + CALLOC_ARRAY(loose, 1); + loose->source = source; + return loose; +} + +void odb_source_loose_free(struct odb_source_loose *loose) +{ + free(loose); +} diff --git a/object-file.h b/object-file.h index 097e9764be169e..695a7e8e7c4b0b 100644 --- a/object-file.h +++ b/object-file.h @@ -18,6 +18,13 @@ int index_path(struct index_state *istate, struct object_id *oid, const char *pa struct odb_source; +struct odb_source_loose { + struct odb_source *source; +}; + +struct odb_source_loose *odb_source_loose_new(struct odb_source *source); +void odb_source_loose_free(struct odb_source_loose *loose); + /* * Populate and return the loose object cache array corresponding to the * given object ID. diff --git a/odb.c b/odb.c index 77490d7fdbeb4b..2d06ab0bb85c27 100644 --- a/odb.c +++ b/odb.c @@ -151,6 +151,7 @@ struct odb_source *odb_source_new(struct object_database *odb, source->odb = odb; source->local = local; source->path = xstrdup(path); + source->loose = odb_source_loose_new(source); return source; } @@ -368,6 +369,7 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb, static void odb_source_free(struct odb_source *source) { free(source->path); + odb_source_loose_free(source->loose); odb_clear_loose_cache(source); loose_object_map_clear(&source->loose_map); free(source); diff --git a/odb.h b/odb.h index 2346ffeca85961..49b398bedae640 100644 --- a/odb.h +++ b/odb.h @@ -48,6 +48,9 @@ struct odb_source { /* Object database that owns this object source. */ struct object_database *odb; + /* Private state for loose objects. */ + struct odb_source_loose *loose; + /* * Used to store the results of readdir(3) calls when we are OK * sacrificing accuracy due to races for speed. That includes From 90a93f9dea88532623ef7422dbc21d8dc70a58dd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:01 +0100 Subject: [PATCH 06/25] object-file: move loose object cache into loose source Our loose objects use a cache that (optionally) stores all objects for each of the opened sharding directories. This cache is located in the `struct odb_source`, but now that we have `struct odb_source_loose` it makes sense to move it into the latter structure so that all state that relates to loose objects is entirely self-contained. Do so. While at it, rename corresponding functions to have a prefix that relates to `struct odb_source_loose`. Note that despite this prefix, the functions still accept a `struct odb_source` as input. This is done intentionally: once we introduce pluggable object databases, we will continue to accept this struct but then do a cast inside these functions to `struct odb_source_loose`. This design is similar to how we do it for our ref backends. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- loose.c | 9 +++++---- object-file.c | 35 +++++++++++++++++++---------------- object-file.h | 16 ++++++++++++++-- object-name.c | 2 +- odb.c | 1 - odb.h | 12 ------------ 6 files changed, 39 insertions(+), 36 deletions(-) diff --git a/loose.c b/loose.c index e8ea6e7e24ba31..8cc7573ff2b2d9 100644 --- a/loose.c +++ b/loose.c @@ -1,6 +1,7 @@ #include "git-compat-util.h" #include "hash.h" #include "path.h" +#include "object-file.h" #include "odb.h" #include "hex.h" #include "repository.h" @@ -54,7 +55,7 @@ static int insert_loose_map(struct odb_source *source, inserted |= insert_oid_pair(map->to_compat, oid, compat_oid); inserted |= insert_oid_pair(map->to_storage, compat_oid, oid); if (inserted) - oidtree_insert(source->loose_objects_cache, compat_oid); + oidtree_insert(source->loose->cache, compat_oid); return inserted; } @@ -66,9 +67,9 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source if (!source->loose_map) loose_object_map_init(&source->loose_map); - if (!source->loose_objects_cache) { - ALLOC_ARRAY(source->loose_objects_cache, 1); - oidtree_init(source->loose_objects_cache); + if (!source->loose->cache) { + ALLOC_ARRAY(source->loose->cache, 1); + oidtree_init(source->loose->cache); } insert_loose_map(source, repo->hash_algo->empty_tree, repo->compat_hash_algo->empty_tree); diff --git a/object-file.c b/object-file.c index cd6aa561fa7db2..fef00d6d3d082a 100644 --- a/object-file.c +++ b/object-file.c @@ -223,7 +223,7 @@ static int quick_has_loose(struct repository *r, odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { - if (oidtree_contains(odb_loose_cache(source, oid), oid)) + if (oidtree_contains(odb_source_loose_cache(source, oid), oid)) return 1; } return 0; @@ -1802,44 +1802,44 @@ static int append_loose_object(const struct object_id *oid, return 0; } -struct oidtree *odb_loose_cache(struct odb_source *source, - const struct object_id *oid) +struct oidtree *odb_source_loose_cache(struct odb_source *source, + const struct object_id *oid) { int subdir_nr = oid->hash[0]; struct strbuf buf = STRBUF_INIT; - size_t word_bits = bitsizeof(source->loose_objects_subdir_seen[0]); + size_t word_bits = bitsizeof(source->loose->subdir_seen[0]); size_t word_index = subdir_nr / word_bits; size_t mask = (size_t)1u << (subdir_nr % word_bits); uint32_t *bitmap; if (subdir_nr < 0 || - (size_t) subdir_nr >= bitsizeof(source->loose_objects_subdir_seen)) + (size_t) subdir_nr >= bitsizeof(source->loose->subdir_seen)) BUG("subdir_nr out of range"); - bitmap = &source->loose_objects_subdir_seen[word_index]; + bitmap = &source->loose->subdir_seen[word_index]; if (*bitmap & mask) - return source->loose_objects_cache; - if (!source->loose_objects_cache) { - ALLOC_ARRAY(source->loose_objects_cache, 1); - oidtree_init(source->loose_objects_cache); + return source->loose->cache; + if (!source->loose->cache) { + ALLOC_ARRAY(source->loose->cache, 1); + oidtree_init(source->loose->cache); } strbuf_addstr(&buf, source->path); for_each_file_in_obj_subdir(subdir_nr, &buf, source->odb->repo->hash_algo, append_loose_object, NULL, NULL, - source->loose_objects_cache); + source->loose->cache); *bitmap |= mask; strbuf_release(&buf); - return source->loose_objects_cache; + return source->loose->cache; } void odb_clear_loose_cache(struct odb_source *source) { - oidtree_clear(source->loose_objects_cache); - FREE_AND_NULL(source->loose_objects_cache); - memset(&source->loose_objects_subdir_seen, 0, - sizeof(source->loose_objects_subdir_seen)); + oidtree_clear(source->loose->cache); + FREE_AND_NULL(source->loose->cache); + memset(&source->loose->subdir_seen, 0, + sizeof(source->loose->subdir_seen)); } static int check_stream_oid(git_zstream *stream, @@ -2006,5 +2006,8 @@ struct odb_source_loose *odb_source_loose_new(struct odb_source *source) void odb_source_loose_free(struct odb_source_loose *loose) { + if (!loose) + return; + odb_clear_loose_cache(loose->source); free(loose); } diff --git a/object-file.h b/object-file.h index 695a7e8e7c4b0b..90da69cf5f7d59 100644 --- a/object-file.h +++ b/object-file.h @@ -20,6 +20,18 @@ struct odb_source; struct odb_source_loose { struct odb_source *source; + + /* + * Used to store the results of readdir(3) calls when we are OK + * sacrificing accuracy due to races for speed. That includes + * object existence with OBJECT_INFO_QUICK, as well as + * our search for unique abbreviated hashes. Don't use it for tasks + * requiring greater accuracy! + * + * Be sure to call odb_load_loose_cache() before using. + */ + uint32_t subdir_seen[8]; /* 256 bits */ + struct oidtree *cache; }; struct odb_source_loose *odb_source_loose_new(struct odb_source *source); @@ -29,8 +41,8 @@ void odb_source_loose_free(struct odb_source_loose *loose); * Populate and return the loose object cache array corresponding to the * given object ID. */ -struct oidtree *odb_loose_cache(struct odb_source *source, - const struct object_id *oid); +struct oidtree *odb_source_loose_cache(struct odb_source *source, + const struct object_id *oid); /* Empty the loose object cache for the specified object directory. */ void odb_clear_loose_cache(struct odb_source *source); diff --git a/object-name.c b/object-name.c index 766c757042a389..8ce0ef7c23a6bd 100644 --- a/object-name.c +++ b/object-name.c @@ -116,7 +116,7 @@ static void find_short_object_filename(struct disambiguate_state *ds) struct odb_source *source; for (source = ds->repo->objects->sources; source && !ds->ambiguous; source = source->next) - oidtree_each(odb_loose_cache(source, &ds->bin_pfx), + oidtree_each(odb_source_loose_cache(source, &ds->bin_pfx), &ds->bin_pfx, ds->len, match_prefix, ds); } diff --git a/odb.c b/odb.c index 2d06ab0bb85c27..87d84688c638cc 100644 --- a/odb.c +++ b/odb.c @@ -370,7 +370,6 @@ static void odb_source_free(struct odb_source *source) { free(source->path); odb_source_loose_free(source->loose); - odb_clear_loose_cache(source); loose_object_map_clear(&source->loose_map); free(source); } diff --git a/odb.h b/odb.h index 49b398bedae640..77104396afe4aa 100644 --- a/odb.h +++ b/odb.h @@ -51,18 +51,6 @@ struct odb_source { /* Private state for loose objects. */ struct odb_source_loose *loose; - /* - * Used to store the results of readdir(3) calls when we are OK - * sacrificing accuracy due to races for speed. That includes - * object existence with OBJECT_INFO_QUICK, as well as - * our search for unique abbreviated hashes. Don't use it for tasks - * requiring greater accuracy! - * - * Be sure to call odb_load_loose_cache() before using. - */ - uint32_t loose_objects_subdir_seen[8]; /* 256 bits */ - struct oidtree *loose_objects_cache; - /* Map between object IDs for loose objects. */ struct loose_object_map *loose_map; From be659c97eae3b68e38b71f0a67067dede23903b5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:02 +0100 Subject: [PATCH 07/25] object-file: hide internals when we need to reprepare loose sources There are two different situations where we have to clear the cache of loose objects: - When freeing the loose object source itself to avoid memory leaks. - When repreparing the loose object source so that any potentially- stale data is getting evicted from the cache. The former is already handled by `odb_source_loose_free()`. But the latter case is still done manually by in `odb_reprepare()`, so we are leaking internals into that code. Introduce a new `odb_source_loose_reprepare()` function as an equivalent to `packfile_store_prepare()` to hide these implementation details. Furthermore, while at it, rename the function `odb_clear_loose_cache()` to `odb_source_loose_clear()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.c | 17 +++++++++++------ object-file.h | 6 +++--- odb.c | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/object-file.c b/object-file.c index fef00d6d3d082a..20daa629a1dda9 100644 --- a/object-file.c +++ b/object-file.c @@ -1834,12 +1834,17 @@ struct oidtree *odb_source_loose_cache(struct odb_source *source, return source->loose->cache; } -void odb_clear_loose_cache(struct odb_source *source) +static void odb_source_loose_clear_cache(struct odb_source_loose *loose) { - oidtree_clear(source->loose->cache); - FREE_AND_NULL(source->loose->cache); - memset(&source->loose->subdir_seen, 0, - sizeof(source->loose->subdir_seen)); + oidtree_clear(loose->cache); + FREE_AND_NULL(loose->cache); + memset(&loose->subdir_seen, 0, + sizeof(loose->subdir_seen)); +} + +void odb_source_loose_reprepare(struct odb_source *source) +{ + odb_source_loose_clear_cache(source->loose); } static int check_stream_oid(git_zstream *stream, @@ -2008,6 +2013,6 @@ void odb_source_loose_free(struct odb_source_loose *loose) { if (!loose) return; - odb_clear_loose_cache(loose->source); + odb_source_loose_clear_cache(loose); free(loose); } diff --git a/object-file.h b/object-file.h index 90da69cf5f7d59..bec855e8e53f95 100644 --- a/object-file.h +++ b/object-file.h @@ -37,6 +37,9 @@ struct odb_source_loose { struct odb_source_loose *odb_source_loose_new(struct odb_source *source); void odb_source_loose_free(struct odb_source_loose *loose); +/* Reprepare the loose source by emptying the loose object cache. */ +void odb_source_loose_reprepare(struct odb_source *source); + /* * Populate and return the loose object cache array corresponding to the * given object ID. @@ -44,9 +47,6 @@ void odb_source_loose_free(struct odb_source_loose *loose); struct oidtree *odb_source_loose_cache(struct odb_source *source, const struct object_id *oid); -/* Empty the loose object cache for the specified object directory. */ -void odb_clear_loose_cache(struct odb_source *source); - /* * Put in `buf` the name of the file in the local object database that * would be used to store a loose object with the specified oid. diff --git a/odb.c b/odb.c index 87d84688c638cc..b3e8d4a49cb07e 100644 --- a/odb.c +++ b/odb.c @@ -1071,7 +1071,7 @@ void odb_reprepare(struct object_database *o) odb_prepare_alternates(o); for (source = o->sources; source; source = source->next) - odb_clear_loose_cache(source); + odb_source_loose_reprepare(source); o->approximate_object_count_valid = 0; From 376016ec71c3a6c883f2ca77a3f1c0245fd60dc2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:03 +0100 Subject: [PATCH 08/25] object-file: move loose object map into loose source The loose object map is used to map from the repository's canonical object hash to the compatibility hash. As the name indicates, this map is only used for loose objects, and as such it is tied to a specific loose object source. Same as with preceding commits, move this map into the loose object source accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- loose.c | 10 +++++----- object-file.c | 1 + object-file.h | 3 +++ odb.c | 1 - odb.h | 3 --- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/loose.c b/loose.c index 8cc7573ff2b2d9..56cf64b648bf80 100644 --- a/loose.c +++ b/loose.c @@ -49,7 +49,7 @@ static int insert_loose_map(struct odb_source *source, const struct object_id *oid, const struct object_id *compat_oid) { - struct loose_object_map *map = source->loose_map; + struct loose_object_map *map = source->loose->map; int inserted = 0; inserted |= insert_oid_pair(map->to_compat, oid, compat_oid); @@ -65,8 +65,8 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT; FILE *fp; - if (!source->loose_map) - loose_object_map_init(&source->loose_map); + if (!source->loose->map) + loose_object_map_init(&source->loose->map); if (!source->loose->cache) { ALLOC_ARRAY(source->loose->cache, 1); oidtree_init(source->loose->cache); @@ -125,7 +125,7 @@ int repo_read_loose_object_map(struct repository *repo) int repo_write_loose_object_map(struct repository *repo) { - kh_oid_map_t *map = repo->objects->sources->loose_map->to_compat; + kh_oid_map_t *map = repo->objects->sources->loose->map->to_compat; struct lock_file lock; int fd; khiter_t iter; @@ -231,7 +231,7 @@ int repo_loose_object_map_oid(struct repository *repo, khiter_t pos; for (source = repo->objects->sources; source; source = source->next) { - struct loose_object_map *loose_map = source->loose_map; + struct loose_object_map *loose_map = source->loose->map; if (!loose_map) continue; map = (to == repo->compat_hash_algo) ? diff --git a/object-file.c b/object-file.c index 20daa629a1dda9..ccc67713fad38f 100644 --- a/object-file.c +++ b/object-file.c @@ -2014,5 +2014,6 @@ void odb_source_loose_free(struct odb_source_loose *loose) if (!loose) return; odb_source_loose_clear_cache(loose); + loose_object_map_clear(&loose->map); free(loose); } diff --git a/object-file.h b/object-file.h index bec855e8e53f95..f8a96a45f57703 100644 --- a/object-file.h +++ b/object-file.h @@ -32,6 +32,9 @@ struct odb_source_loose { */ uint32_t subdir_seen[8]; /* 256 bits */ struct oidtree *cache; + + /* Map between object IDs for loose objects. */ + struct loose_object_map *map; }; struct odb_source_loose *odb_source_loose_new(struct odb_source *source); diff --git a/odb.c b/odb.c index b3e8d4a49cb07e..d1df9609e21d1e 100644 --- a/odb.c +++ b/odb.c @@ -370,7 +370,6 @@ static void odb_source_free(struct odb_source *source) { free(source->path); odb_source_loose_free(source->loose); - loose_object_map_clear(&source->loose_map); free(source); } diff --git a/odb.h b/odb.h index 77104396afe4aa..f9a3137a34aa3b 100644 --- a/odb.h +++ b/odb.h @@ -51,9 +51,6 @@ struct odb_source { /* Private state for loose objects. */ struct odb_source_loose *loose; - /* Map between object IDs for loose objects. */ - struct loose_object_map *loose_map; - /* * private data * From ff7ad5cb3936514ec0be32531ff6274b53dbe091 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:04 +0100 Subject: [PATCH 09/25] object-file: read objects via the loose object source When reading an object via `loose_object_info()` or `map_loose_object()` we hand in the whole repository. We then iterate through each of the object sources to figure out whether that source has the object in question. This logic is reversing responsibility though: a specific backend should only care about one specific source, where the object sources themselves are then managed by the object database. Refactor the code accordingly by passing an object source to both of these functions instead. The different sources are then handled by either `do_oid_object_info_extended()`, which sits on the object database level, and by `open_istream_loose()`. The latter function arguably is still at the wrong level, but this will be cleaned up at a later point in time. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.c | 68 +++++++++++++++++++-------------------------------- object-file.h | 15 ++++++------ odb.c | 9 +++++-- streaming.c | 11 ++++++++- 4 files changed, 50 insertions(+), 53 deletions(-) diff --git a/object-file.c b/object-file.c index ccc67713fad38f..6d6e9a5a2ad3c8 100644 --- a/object-file.c +++ b/object-file.c @@ -167,25 +167,22 @@ int stream_object_signature(struct repository *r, const struct object_id *oid) } /* - * Find "oid" as a loose object in the local repository or in an alternate. + * Find "oid" as a loose object in given source. * Returns 0 on success, negative on failure. * * The "path" out-parameter will give the path of the object we found (if any). * Note that it may point to static storage and is only valid until another * call to stat_loose_object(). */ -static int stat_loose_object(struct repository *r, const struct object_id *oid, +static int stat_loose_object(struct odb_source_loose *loose, + const struct object_id *oid, struct stat *st, const char **path) { - struct odb_source *source; static struct strbuf buf = STRBUF_INIT; - odb_prepare_alternates(r->objects); - for (source = r->objects->sources; source; source = source->next) { - *path = odb_loose_path(source, &buf, oid); - if (!lstat(*path, st)) - return 0; - } + *path = odb_loose_path(loose->source, &buf, oid); + if (!lstat(*path, st)) + return 0; return -1; } @@ -194,39 +191,24 @@ static int stat_loose_object(struct repository *r, const struct object_id *oid, * Like stat_loose_object(), but actually open the object and return the * descriptor. See the caveats on the "path" parameter above. */ -static int open_loose_object(struct repository *r, +static int open_loose_object(struct odb_source_loose *loose, const struct object_id *oid, const char **path) { - int fd; - struct odb_source *source; - int most_interesting_errno = ENOENT; static struct strbuf buf = STRBUF_INIT; + int fd; - odb_prepare_alternates(r->objects); - for (source = r->objects->sources; source; source = source->next) { - *path = odb_loose_path(source, &buf, oid); - fd = git_open(*path); - if (fd >= 0) - return fd; + *path = odb_loose_path(loose->source, &buf, oid); + fd = git_open(*path); + if (fd >= 0) + return fd; - if (most_interesting_errno == ENOENT) - most_interesting_errno = errno; - } - errno = most_interesting_errno; return -1; } -static int quick_has_loose(struct repository *r, +static int quick_has_loose(struct odb_source_loose *loose, const struct object_id *oid) { - struct odb_source *source; - - odb_prepare_alternates(r->objects); - for (source = r->objects->sources; source; source = source->next) { - if (oidtree_contains(odb_source_loose_cache(source, oid), oid)) - return 1; - } - return 0; + return !!oidtree_contains(odb_source_loose_cache(loose->source, oid), oid); } /* @@ -252,12 +234,12 @@ static void *map_fd(int fd, const char *path, unsigned long *size) return map; } -void *map_loose_object(struct repository *r, - const struct object_id *oid, - unsigned long *size) +void *odb_source_loose_map_object(struct odb_source *source, + const struct object_id *oid, + unsigned long *size) { const char *p; - int fd = open_loose_object(r, oid, &p); + int fd = open_loose_object(source->loose, oid, &p); if (fd < 0) return NULL; @@ -407,9 +389,9 @@ int parse_loose_header(const char *hdr, struct object_info *oi) return 0; } -int loose_object_info(struct repository *r, - const struct object_id *oid, - struct object_info *oi, int flags) +int odb_source_loose_read_object_info(struct odb_source *source, + const struct object_id *oid, + struct object_info *oi, int flags) { int status = 0; int fd; @@ -422,7 +404,7 @@ int loose_object_info(struct repository *r, enum object_type type_scratch; if (oi->delta_base_oid) - oidclr(oi->delta_base_oid, r->hash_algo); + oidclr(oi->delta_base_oid, source->odb->repo->hash_algo); /* * If we don't care about type or size, then we don't @@ -435,15 +417,15 @@ int loose_object_info(struct repository *r, if (!oi->typep && !oi->sizep && !oi->contentp) { struct stat st; if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK)) - return quick_has_loose(r, oid) ? 0 : -1; - if (stat_loose_object(r, oid, &st, &path) < 0) + return quick_has_loose(source->loose, oid) ? 0 : -1; + if (stat_loose_object(source->loose, oid, &st, &path) < 0) return -1; if (oi->disk_sizep) *oi->disk_sizep = st.st_size; return 0; } - fd = open_loose_object(r, oid, &path); + fd = open_loose_object(source->loose, oid, &path); if (fd < 0) { if (errno != ENOENT) error_errno(_("unable to open loose object %s"), oid_to_hex(oid)); diff --git a/object-file.h b/object-file.h index f8a96a45f57703..ca13d3d64e722b 100644 --- a/object-file.h +++ b/object-file.h @@ -43,6 +43,14 @@ void odb_source_loose_free(struct odb_source_loose *loose); /* Reprepare the loose source by emptying the loose object cache. */ void odb_source_loose_reprepare(struct odb_source *source); +int odb_source_loose_read_object_info(struct odb_source *source, + const struct object_id *oid, + struct object_info *oi, int flags); + +void *odb_source_loose_map_object(struct odb_source *source, + const struct object_id *oid, + unsigned long *size); + /* * Populate and return the loose object cache array corresponding to the * given object ID. @@ -66,9 +74,6 @@ const char *odb_loose_path(struct odb_source *source, int has_loose_object(struct odb_source *source, const struct object_id *oid); -void *map_loose_object(struct repository *r, const struct object_id *oid, - unsigned long *size); - /* * Iterate over the files in the loose-object parts of the object * directory "path", triggering the following callbacks: @@ -196,10 +201,6 @@ int check_object_signature(struct repository *r, const struct object_id *oid, */ int stream_object_signature(struct repository *r, const struct object_id *oid); -int loose_object_info(struct repository *r, - const struct object_id *oid, - struct object_info *oi, int flags); - enum finalize_object_file_flags { FOF_SKIP_COLLISION_CHECK = 1, }; diff --git a/odb.c b/odb.c index d1df9609e21d1e..4c0b4fdcd54ce1 100644 --- a/odb.c +++ b/odb.c @@ -697,13 +697,18 @@ static int do_oid_object_info_extended(struct object_database *odb, return 0; } + odb_prepare_alternates(odb); + while (1) { + struct odb_source *source; + if (find_pack_entry(odb->repo, real, &e)) break; /* Most likely it's a loose object. */ - if (!loose_object_info(odb->repo, real, oi, flags)) - return 0; + for (source = odb->sources; source; source = source->next) + if (!odb_source_loose_read_object_info(source, real, oi, flags)) + return 0; /* Not a loose object; someone else may have just packed it. */ if (!(flags & OBJECT_INFO_QUICK)) { diff --git a/streaming.c b/streaming.c index 4b13827668e67a..00ad649ae397f3 100644 --- a/streaming.c +++ b/streaming.c @@ -230,12 +230,21 @@ static int open_istream_loose(struct git_istream *st, struct repository *r, enum object_type *type) { struct object_info oi = OBJECT_INFO_INIT; + struct odb_source *source; + oi.sizep = &st->size; oi.typep = type; - st->u.loose.mapped = map_loose_object(r, oid, &st->u.loose.mapsize); + odb_prepare_alternates(r->objects); + for (source = r->objects->sources; source; source = source->next) { + st->u.loose.mapped = odb_source_loose_map_object(source, oid, + &st->u.loose.mapsize); + if (st->u.loose.mapped) + break; + } if (!st->u.loose.mapped) return -1; + switch (unpack_loose_header(&st->z, st->u.loose.mapped, st->u.loose.mapsize, st->u.loose.hdr, sizeof(st->u.loose.hdr))) { From 05130c6c9eed9ff7450e9067d7215032eb914c10 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:05 +0100 Subject: [PATCH 10/25] object-file: rename `has_loose_object()` Rename `has_loose_object()` to `odb_source_loose_has_object()` so that it becomes clear that this is tied to a specific loose object source. This matches our modern naming schema for functions. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 4 ++-- object-file.c | 6 +++--- object-file.h | 16 ++++++++-------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index b5454e5df137b4..69e80b1443a9b7 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1716,7 +1716,7 @@ static int want_object_in_pack_mtime(const struct object_id *oid, */ struct odb_source *source = the_repository->objects->sources->next; for (; source; source = source->next) - if (has_loose_object(source, oid)) + if (odb_source_loose_has_object(source, oid)) return 0; } @@ -3978,7 +3978,7 @@ static void add_cruft_object_entry(const struct object_id *oid, enum object_type int found = 0; for (; !found && source; source = source->next) - if (has_loose_object(source, oid)) + if (odb_source_loose_has_object(source, oid)) found = 1; /* diff --git a/object-file.c b/object-file.c index 6d6e9a5a2ad3c8..79e7ab8d2e3d0e 100644 --- a/object-file.c +++ b/object-file.c @@ -99,8 +99,8 @@ static int check_and_freshen_source(struct odb_source *source, return check_and_freshen_file(path.buf, freshen); } -int has_loose_object(struct odb_source *source, - const struct object_id *oid) +int odb_source_loose_has_object(struct odb_source *source, + const struct object_id *oid) { return check_and_freshen_source(source, oid, 0); } @@ -1161,7 +1161,7 @@ int force_object_loose(struct odb_source *source, int ret; for (struct odb_source *s = source->odb->sources; s; s = s->next) - if (has_loose_object(s, oid)) + if (odb_source_loose_has_object(s, oid)) return 0; oi.typep = &type; diff --git a/object-file.h b/object-file.h index ca13d3d64e722b..065a44bb8a019e 100644 --- a/object-file.h +++ b/object-file.h @@ -51,6 +51,14 @@ void *odb_source_loose_map_object(struct odb_source *source, const struct object_id *oid, unsigned long *size); +/* + * Return true iff an object database source has a loose object + * with the specified name. This function does not respect replace + * references. + */ +int odb_source_loose_has_object(struct odb_source *source, + const struct object_id *oid); + /* * Populate and return the loose object cache array corresponding to the * given object ID. @@ -66,14 +74,6 @@ const char *odb_loose_path(struct odb_source *source, struct strbuf *buf, const struct object_id *oid); -/* - * Return true iff an object database source has a loose object - * with the specified name. This function does not respect replace - * references. - */ -int has_loose_object(struct odb_source *source, - const struct object_id *oid); - /* * Iterate over the files in the loose-object parts of the object * directory "path", triggering the following callbacks: From f2bd88a308a2754e727cb462e03102307cdfe004 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:06 +0100 Subject: [PATCH 11/25] object-file: refactor freshening of objects When writing an object that already exists in our object database we skip the write and instead only update mtimes of the object, either in its packed or loose object format. This logic is wholly contained in "object-file.c", but that file is really only concerned with loose objects. So it does not really make sense that it also contains the logic to freshen a packed object. Introduce a new `odb_freshen_object()` function that sits on the object database level and two functions `packfile_store_freshen_object()` and `odb_source_loose_freshen_object()`. Like this, the format-specific functions can be part of their respective subsystems, while the backend agnostic function to freshen an object sits at the object database layer. Note that this change also moves the logic that iterates through object sources from the object source layer into the object database layer. This change is intentional: object sources should ideally only have to worry about themselves, and coordination of different sources should be handled on the object database level. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.c | 33 +++++---------------------------- object-file.h | 3 +++ odb.c | 16 ++++++++++++++++ odb.h | 3 +++ packfile.c | 16 ++++++++++++++++ packfile.h | 3 +++ 6 files changed, 46 insertions(+), 28 deletions(-) diff --git a/object-file.c b/object-file.c index 79e7ab8d2e3d0e..893c32adcddbbd 100644 --- a/object-file.c +++ b/object-file.c @@ -968,30 +968,10 @@ static int write_loose_object(struct odb_source *source, FOF_SKIP_COLLISION_CHECK); } -static int freshen_loose_object(struct object_database *odb, - const struct object_id *oid) +int odb_source_loose_freshen_object(struct odb_source *source, + const struct object_id *oid) { - odb_prepare_alternates(odb); - for (struct odb_source *source = odb->sources; source; source = source->next) - if (check_and_freshen_source(source, oid, 1)) - return 1; - return 0; -} - -static int freshen_packed_object(struct object_database *odb, - const struct object_id *oid) -{ - struct pack_entry e; - if (!find_pack_entry(odb->repo, oid, &e)) - return 0; - if (e.p->is_cruft) - return 0; - if (e.p->freshened) - return 1; - if (!freshen_file(e.p->pack_name)) - return 0; - e.p->freshened = 1; - return 1; + return !!check_and_freshen_source(source, oid, 1); } int stream_loose_object(struct odb_source *source, @@ -1073,12 +1053,10 @@ int stream_loose_object(struct odb_source *source, die(_("deflateEnd on stream object failed (%d)"), ret); close_loose_object(source, fd, tmp_file.buf); - if (freshen_packed_object(source->odb, oid) || - freshen_loose_object(source->odb, oid)) { + if (odb_freshen_object(source->odb, oid)) { unlink_or_warn(tmp_file.buf); goto cleanup; } - odb_loose_path(source, &filename, oid); /* We finally know the object path, and create the missing dir. */ @@ -1137,8 +1115,7 @@ int write_object_file(struct odb_source *source, * it out into .git/objects/??/?{38} file. */ write_object_file_prepare(algo, buf, len, type, oid, hdr, &hdrlen); - if (freshen_packed_object(source->odb, oid) || - freshen_loose_object(source->odb, oid)) + if (odb_freshen_object(source->odb, oid)) return 0; if (write_loose_object(source, oid, hdr, hdrlen, buf, len, 0, flags)) return -1; diff --git a/object-file.h b/object-file.h index 065a44bb8a019e..ee5b24cec66c34 100644 --- a/object-file.h +++ b/object-file.h @@ -59,6 +59,9 @@ void *odb_source_loose_map_object(struct odb_source *source, int odb_source_loose_has_object(struct odb_source *source, const struct object_id *oid); +int odb_source_loose_freshen_object(struct odb_source *source, + const struct object_id *oid); + /* * Populate and return the loose object cache array corresponding to the * given object ID. diff --git a/odb.c b/odb.c index 4c0b4fdcd54ce1..17734bdaffe8e6 100644 --- a/odb.c +++ b/odb.c @@ -987,6 +987,22 @@ int odb_has_object(struct object_database *odb, const struct object_id *oid, return odb_read_object_info_extended(odb, oid, NULL, object_info_flags) >= 0; } +int odb_freshen_object(struct object_database *odb, + const struct object_id *oid) +{ + struct odb_source *source; + + if (packfile_store_freshen_object(odb->packfiles, oid)) + return 1; + + odb_prepare_alternates(odb); + for (source = odb->sources; source; source = source->next) + if (odb_source_loose_freshen_object(source, oid)) + return 1; + + return 0; +} + void odb_assert_oid_type(struct object_database *odb, const struct object_id *oid, enum object_type expect) { diff --git a/odb.h b/odb.h index f9a3137a34aa3b..2653247e0cc871 100644 --- a/odb.h +++ b/odb.h @@ -396,6 +396,9 @@ int odb_has_object(struct object_database *odb, const struct object_id *oid, unsigned flags); +int odb_freshen_object(struct object_database *odb, + const struct object_id *oid); + void odb_assert_oid_type(struct object_database *odb, const struct object_id *oid, enum object_type expect); diff --git a/packfile.c b/packfile.c index 1ae2b2fe1eda77..40f733dd234900 100644 --- a/packfile.c +++ b/packfile.c @@ -819,6 +819,22 @@ struct packed_git *packfile_store_load_pack(struct packfile_store *store, return p; } +int packfile_store_freshen_object(struct packfile_store *store, + const struct object_id *oid) +{ + struct pack_entry e; + if (!find_pack_entry(store->odb->repo, oid, &e)) + return 0; + if (e.p->is_cruft) + return 0; + if (e.p->freshened) + return 1; + if (utime(e.p->pack_name, NULL)) + return 0; + e.p->freshened = 1; + return 1; +} + void (*report_garbage)(unsigned seen_bits, const char *path); static void report_helper(const struct string_list *list, diff --git a/packfile.h b/packfile.h index c9d0b93446b5f5..58fcc88e20224b 100644 --- a/packfile.h +++ b/packfile.h @@ -163,6 +163,9 @@ struct list_head *packfile_store_get_packs_mru(struct packfile_store *store); struct packed_git *packfile_store_load_pack(struct packfile_store *store, const char *idx_path, int local); +int packfile_store_freshen_object(struct packfile_store *store, + const struct object_id *oid); + struct pack_window { struct pack_window *next; unsigned char *base; From bfb1b2b4ac5cfa99f7d2503b404d282714d84bdf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:07 +0100 Subject: [PATCH 12/25] object-file: rename `write_object_file()` Rename `write_object_file()` to `odb_source_loose_write_object()` so that it becomes clear that this is tied to a specific loose object source. This matches our modern naming schema for functions. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.c | 8 ++++---- object-file.h | 10 +++++----- odb.c | 3 ++- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/object-file.c b/object-file.c index 893c32adcddbbd..fdc644a4275373 100644 --- a/object-file.c +++ b/object-file.c @@ -1084,10 +1084,10 @@ int stream_loose_object(struct odb_source *source, return err; } -int write_object_file(struct odb_source *source, - const void *buf, unsigned long len, - enum object_type type, struct object_id *oid, - struct object_id *compat_oid_in, unsigned flags) +int odb_source_loose_write_object(struct odb_source *source, + const void *buf, unsigned long len, + enum object_type type, struct object_id *oid, + struct object_id *compat_oid_in, unsigned flags) { const struct git_hash_algo *algo = source->odb->repo->hash_algo; const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo; diff --git a/object-file.h b/object-file.h index ee5b24cec66c34..36a60e15c40547 100644 --- a/object-file.h +++ b/object-file.h @@ -62,6 +62,11 @@ int odb_source_loose_has_object(struct odb_source *source, int odb_source_loose_freshen_object(struct odb_source *source, const struct object_id *oid); +int odb_source_loose_write_object(struct odb_source *source, + const void *buf, unsigned long len, + enum object_type type, struct object_id *oid, + struct object_id *compat_oid_in, unsigned flags); + /* * Populate and return the loose object cache array corresponding to the * given object ID. @@ -168,11 +173,6 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, struct object_info; int parse_loose_header(const char *hdr, struct object_info *oi); -int write_object_file(struct odb_source *source, - const void *buf, unsigned long len, - enum object_type type, struct object_id *oid, - struct object_id *compat_oid_in, unsigned flags); - struct input_stream { const void *(*read)(struct input_stream *, unsigned long *len); void *data; diff --git a/odb.c b/odb.c index 17734bdaffe8e6..da44f1d63b43b2 100644 --- a/odb.c +++ b/odb.c @@ -1021,7 +1021,8 @@ int odb_write_object_ext(struct object_database *odb, struct object_id *compat_oid, unsigned flags) { - return write_object_file(odb->sources, buf, len, type, oid, compat_oid, flags); + return odb_source_loose_write_object(odb->sources, buf, len, type, + oid, compat_oid, flags); } struct object_database *odb_new(struct repository *repo) From 3e5e360888316ed1a44da69bf134bb6ec70aee1b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 3 Nov 2025 08:42:08 +0100 Subject: [PATCH 13/25] object-file: refactor writing objects via a stream We have two different ways to write an object into the database: - We either provide the full buffer and write the object all at once. - Or we provide an input stream that has a `read()` function so that we can chunk the object. The latter is especially used for large objects, where it may be too expensive to hold the complete object in memory all at once. While we already have `odb_write_object()` at the ODB-layer, we don't have an equivalent for streaming an object. Introduce a new function `odb_write_object_stream()` to address this gap so that callers don't have to be aware of the inner workings of how to stream an object to disk with a specific object source. Rename `stream_loose_object()` to `odb_source_loose_write_stream()` to clarify its scope. This matches our modern best practices around how to name functions. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/unpack-objects.c | 7 +++---- object-file.c | 6 +++--- object-file.h | 14 ++++---------- odb.c | 7 +++++++ odb.h | 10 ++++++++++ 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index ef79e43715d362..6fc64e9e4b8d5a 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -363,7 +363,7 @@ struct input_zstream_data { int status; }; -static const void *feed_input_zstream(struct input_stream *in_stream, +static const void *feed_input_zstream(struct odb_write_stream *in_stream, unsigned long *readlen) { struct input_zstream_data *data = in_stream->data; @@ -393,7 +393,7 @@ static void stream_blob(unsigned long size, unsigned nr) { git_zstream zstream = { 0 }; struct input_zstream_data data = { 0 }; - struct input_stream in_stream = { + struct odb_write_stream in_stream = { .read = feed_input_zstream, .data = &data, }; @@ -402,8 +402,7 @@ static void stream_blob(unsigned long size, unsigned nr) data.zstream = &zstream; git_inflate_init(&zstream); - if (stream_loose_object(the_repository->objects->sources, - &in_stream, size, &info->oid)) + if (odb_write_object_stream(the_repository->objects, &in_stream, size, &info->oid)) die(_("failed to write object in stream")); if (data.status != Z_STREAM_END) diff --git a/object-file.c b/object-file.c index fdc644a4275373..811c569ed36aa4 100644 --- a/object-file.c +++ b/object-file.c @@ -974,9 +974,9 @@ int odb_source_loose_freshen_object(struct odb_source *source, return !!check_and_freshen_source(source, oid, 1); } -int stream_loose_object(struct odb_source *source, - struct input_stream *in_stream, size_t len, - struct object_id *oid) +int odb_source_loose_write_stream(struct odb_source *source, + struct odb_write_stream *in_stream, size_t len, + struct object_id *oid) { const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo; struct object_id compat_oid; diff --git a/object-file.h b/object-file.h index 36a60e15c40547..eeffa67bbda631 100644 --- a/object-file.h +++ b/object-file.h @@ -67,6 +67,10 @@ int odb_source_loose_write_object(struct odb_source *source, enum object_type type, struct object_id *oid, struct object_id *compat_oid_in, unsigned flags); +int odb_source_loose_write_stream(struct odb_source *source, + struct odb_write_stream *stream, size_t len, + struct object_id *oid); + /* * Populate and return the loose object cache array corresponding to the * given object ID. @@ -173,16 +177,6 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream, struct object_info; int parse_loose_header(const char *hdr, struct object_info *oi); -struct input_stream { - const void *(*read)(struct input_stream *, unsigned long *len); - void *data; - int is_finished; -}; - -int stream_loose_object(struct odb_source *source, - struct input_stream *in_stream, size_t len, - struct object_id *oid); - int force_object_loose(struct odb_source *source, const struct object_id *oid, time_t mtime); diff --git a/odb.c b/odb.c index da44f1d63b43b2..3ec21ef24e16bb 100644 --- a/odb.c +++ b/odb.c @@ -1025,6 +1025,13 @@ int odb_write_object_ext(struct object_database *odb, oid, compat_oid, flags); } +int odb_write_object_stream(struct object_database *odb, + struct odb_write_stream *stream, size_t len, + struct object_id *oid) +{ + return odb_source_loose_write_stream(odb->sources, stream, len, oid); +} + struct object_database *odb_new(struct repository *repo) { struct object_database *o = xmalloc(sizeof(*o)); diff --git a/odb.h b/odb.h index 2653247e0cc871..9bb28008b1d953 100644 --- a/odb.h +++ b/odb.h @@ -492,4 +492,14 @@ static inline int odb_write_object(struct object_database *odb, return odb_write_object_ext(odb, buf, len, type, oid, NULL, 0); } +struct odb_write_stream { + const void *(*read)(struct odb_write_stream *, unsigned long *len); + void *data; + int is_finished; +}; + +int odb_write_object_stream(struct object_database *odb, + struct odb_write_stream *stream, size_t len, + struct object_id *oid); + #endif /* ODB_H */ From e031fa100603af74def6bf2a646c731e4fcd12fc Mon Sep 17 00:00:00 2001 From: Siddharth Asthana Date: Thu, 6 Nov 2025 00:45:59 +0530 Subject: [PATCH 14/25] replay: use die_for_incompatible_opt2() for option validation In preparation for adding the --ref-action option, convert option validation to use die_for_incompatible_opt2(). This helper provides standardized error messages for mutually exclusive options. The following commit introduces --ref-action which will be incompatible with certain other options. Using die_for_incompatible_opt2() now means that commit can cleanly add its validation using the same pattern, keeping the validation logic consistent and maintainable. This also aligns git-replay's option handling with how other Git commands manage option conflicts, using the established die_for_incompatible_opt*() helper family. Signed-off-by: Siddharth Asthana Signed-off-by: Junio C Hamano --- builtin/replay.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/replay.c b/builtin/replay.c index 6172c8aacc9873..b64fc72063ee8e 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -330,9 +330,9 @@ int cmd_replay(int argc, usage_with_options(replay_usage, replay_options); } - if (advance_name_opt && contained) - die(_("options '%s' and '%s' cannot be used together"), - "--advance", "--contained"); + die_for_incompatible_opt2(!!advance_name_opt, "--advance", + contained, "--contained"); + advance_name = xstrdup_or_null(advance_name_opt); repo_init_revisions(repo, &revs, prefix); From 15cd4ef1f495e51f7db39583b7f562e7170da3d2 Mon Sep 17 00:00:00 2001 From: Siddharth Asthana Date: Thu, 6 Nov 2025 00:46:00 +0530 Subject: [PATCH 15/25] replay: make atomic ref updates the default behavior The git replay command currently outputs update commands that can be piped to update-ref to achieve a rebase, e.g. git replay --onto main topic1..topic2 | git update-ref --stdin This separation had advantages for three special cases: * it made testing easy (when state isn't modified from one step to the next, you don't need to make temporary branches or have undo commands, or try to track the changes) * it provided a natural can-it-rebase-cleanly (and what would it rebase to) capability without automatically updating refs, similar to a --dry-run * it provided a natural low-level tool for the suite of hash-object, mktree, commit-tree, mktag, merge-tree, and update-ref, allowing users to have another building block for experimentation and making new tools However, it should be noted that all three of these are somewhat special cases; users, whether on the client or server side, would almost certainly find it more ergonomic to simply have the updating of refs be the default. For server-side operations in particular, the pipeline architecture creates process coordination overhead. Server implementations that need to perform rebases atomically must maintain additional code to: 1. Spawn and manage a pipeline between git-replay and git-update-ref 2. Coordinate stdout/stderr streams across the pipe boundary 3. Handle partial failure states if the pipeline breaks mid-execution 4. Parse and validate the update-ref command output Change the default behavior to update refs directly, and atomically (at least to the extent supported by the refs backend in use). This eliminates the process coordination overhead for the common case. For users needing the traditional pipeline workflow, add a new --ref-action= option that preserves the original behavior: git replay --ref-action=print --onto main topic1..topic2 | git update-ref --stdin The mode can be: * update (default): Update refs directly using an atomic transaction * print: Output update-ref commands for pipeline use Test suite changes: All existing tests that expected command output now use --ref-action=print to preserve their original behavior. This keeps the tests valid while allowing them to verify that the pipeline workflow still works correctly. New tests were added to verify: - Default atomic behavior (no output, refs updated directly) - Bare repository support (server-side use case) - Equivalence between traditional pipeline and atomic updates - Real atomicity using a lock file to verify all-or-nothing guarantee - Test isolation using test_when_finished to clean up state - Reflog messages include replay mode and target A following commit will add a replay.refAction configuration option for users who prefer the traditional pipeline output as their default behavior. Helped-by: Elijah Newren Helped-by: Patrick Steinhardt Helped-by: Christian Couder Helped-by: Phillip Wood Signed-off-by: Siddharth Asthana Signed-off-by: Junio C Hamano --- Documentation/git-replay.adoc | 61 ++++++++++++------- builtin/replay.c | 111 +++++++++++++++++++++++++++++++--- t/t3650-replay-basics.sh | 67 +++++++++++++++++--- 3 files changed, 199 insertions(+), 40 deletions(-) diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc index 0b12bf8aa4df42..2ef74ddb127b0c 100644 --- a/Documentation/git-replay.adoc +++ b/Documentation/git-replay.adoc @@ -9,15 +9,16 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t SYNOPSIS -------- [verse] -(EXPERIMENTAL!) 'git replay' ([--contained] --onto | --advance ) ... +(EXPERIMENTAL!) 'git replay' ([--contained] --onto | --advance ) [--ref-action[=]] ... DESCRIPTION ----------- Takes ranges of commits and replays them onto a new location. Leaves -the working tree and the index untouched, and updates no references. -The output of this command is meant to be used as input to -`git update-ref --stdin`, which would update the relevant branches +the working tree and the index untouched. By default, updates the +relevant references using an atomic transaction (all refs update or +none). Use `--ref-action=print` to avoid automatic ref updates and +instead get update commands that can be piped to `git update-ref --stdin` (see the OUTPUT section below). THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE. @@ -29,18 +30,27 @@ OPTIONS Starting point at which to create the new commits. May be any valid commit, and not just an existing branch name. + -When `--onto` is specified, the update-ref command(s) in the output will -update the branch(es) in the revision range to point at the new -commits, similar to the way how `git rebase --update-refs` updates -multiple branches in the affected range. +When `--onto` is specified, the branch(es) in the revision range will be +updated to point at the new commits, similar to the way `git rebase --update-refs` +updates multiple branches in the affected range. --advance :: Starting point at which to create the new commits; must be a branch name. + -When `--advance` is specified, the update-ref command(s) in the output -will update the branch passed as an argument to `--advance` to point at -the new commits (in other words, this mimics a cherry-pick operation). +The history is replayed on top of the and is updated to +point at the tip of the resulting history. This is different from `--onto`, +which uses the target only as a starting point without updating it. + +--ref-action[=]:: + Control how references are updated. The mode can be: ++ +-- + * `update` (default): Update refs directly using an atomic transaction. + All refs are updated or none are (all-or-nothing behavior). + * `print`: Output update-ref commands for pipeline use. This is the + traditional behavior where output can be piped to `git update-ref --stdin`. +-- :: Range of commits to replay. More than one can @@ -54,8 +64,11 @@ include::rev-list-options.adoc[] OUTPUT ------ -When there are no conflicts, the output of this command is usable as -input to `git update-ref --stdin`. It is of the form: +By default, or with `--ref-action=update`, this command produces no output on +success, as refs are updated directly using an atomic transaction. + +When using `--ref-action=print`, the output is usable as input to +`git update-ref --stdin`. It is of the form: update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} @@ -81,6 +94,14 @@ To simply rebase `mybranch` onto `target`: ------------ $ git replay --onto target origin/main..mybranch +------------ + +The refs are updated atomically and no output is produced on success. + +To see what would be updated without actually updating: + +------------ +$ git replay --ref-action=print --onto target origin/main..mybranch update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH} ------------ @@ -88,33 +109,29 @@ To cherry-pick the commits from mybranch onto target: ------------ $ git replay --advance target origin/main..mybranch -update refs/heads/target ${NEW_target_HASH} ${OLD_target_HASH} ------------ Note that the first two examples replay the exact same commits and on top of the exact same new base, they only differ in that the first -provides instructions to make mybranch point at the new commits and -the second provides instructions to make target point at them. +updates mybranch to point at the new commits and the second updates +target to point at them. What if you have a stack of branches, one depending upon another, and you'd really like to rebase the whole set? ------------ $ git replay --contained --onto origin/main origin/main..tipbranch -update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} -update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} -update refs/heads/tipbranch ${NEW_tipbranch_HASH} ${OLD_tipbranch_HASH} ------------ +All three branches (`branch1`, `branch2`, and `tipbranch`) are updated +atomically. + When calling `git replay`, one does not need to specify a range of commits to replay using the syntax `A..B`; any range expression will do: ------------ $ git replay --onto origin/main ^base branch1 branch2 branch3 -update refs/heads/branch1 ${NEW_branch1_HASH} ${OLD_branch1_HASH} -update refs/heads/branch2 ${NEW_branch2_HASH} ${OLD_branch2_HASH} -update refs/heads/branch3 ${NEW_branch3_HASH} ${OLD_branch3_HASH} ------------ This will simultaneously rebase `branch1`, `branch2`, and `branch3`, diff --git a/builtin/replay.c b/builtin/replay.c index b64fc72063ee8e..94e60b5b107516 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -20,6 +20,11 @@ #include #include +enum ref_action_mode { + REF_ACTION_UPDATE, + REF_ACTION_PRINT, +}; + static const char *short_commit_name(struct repository *repo, struct commit *commit) { @@ -284,6 +289,38 @@ static struct commit *pick_regular_commit(struct repository *repo, return create_commit(repo, result->tree, pickme, replayed_base); } +static enum ref_action_mode parse_ref_action_mode(const char *ref_action, const char *source) +{ + if (!ref_action || !strcmp(ref_action, "update")) + return REF_ACTION_UPDATE; + if (!strcmp(ref_action, "print")) + return REF_ACTION_PRINT; + die(_("invalid %s value: '%s'"), source, ref_action); +} + +static int handle_ref_update(enum ref_action_mode mode, + struct ref_transaction *transaction, + const char *refname, + const struct object_id *new_oid, + const struct object_id *old_oid, + const char *reflog_msg, + struct strbuf *err) +{ + switch (mode) { + case REF_ACTION_PRINT: + printf("update %s %s %s\n", + refname, + oid_to_hex(new_oid), + oid_to_hex(old_oid)); + return 0; + case REF_ACTION_UPDATE: + return ref_transaction_update(transaction, refname, new_oid, old_oid, + NULL, NULL, 0, reflog_msg, err); + default: + BUG("unknown ref_action_mode %d", mode); + } +} + int cmd_replay(int argc, const char **argv, const char *prefix, @@ -294,6 +331,8 @@ int cmd_replay(int argc, struct commit *onto = NULL; const char *onto_name = NULL; int contained = 0; + const char *ref_action = NULL; + enum ref_action_mode ref_mode = REF_ACTION_UPDATE; struct rev_info revs; struct commit *last_commit = NULL; @@ -302,12 +341,15 @@ int cmd_replay(int argc, struct merge_result result; struct strset *update_refs = NULL; kh_oid_map_t *replayed_commits; + struct ref_transaction *transaction = NULL; + struct strbuf transaction_err = STRBUF_INIT; + struct strbuf reflog_msg = STRBUF_INIT; int ret = 0; - const char * const replay_usage[] = { + const char *const replay_usage[] = { N_("(EXPERIMENTAL!) git replay " "([--contained] --onto | --advance ) " - "..."), + "[--ref-action[=]] ..."), NULL }; struct option replay_options[] = { @@ -319,6 +361,9 @@ int cmd_replay(int argc, N_("replay onto given commit")), OPT_BOOL(0, "contained", &contained, N_("advance all branches contained in revision-range")), + OPT_STRING(0, "ref-action", &ref_action, + N_("mode"), + N_("control ref update behavior (update|print)")), OPT_END() }; @@ -333,6 +378,10 @@ int cmd_replay(int argc, die_for_incompatible_opt2(!!advance_name_opt, "--advance", contained, "--contained"); + /* Parse ref action mode */ + if (ref_action) + ref_mode = parse_ref_action_mode(ref_action, "--ref-action"); + advance_name = xstrdup_or_null(advance_name_opt); repo_init_revisions(repo, &revs, prefix); @@ -389,6 +438,24 @@ int cmd_replay(int argc, determine_replay_mode(repo, &revs.cmdline, onto_name, &advance_name, &onto, &update_refs); + /* Build reflog message */ + if (advance_name_opt) + strbuf_addf(&reflog_msg, "replay --advance %s", advance_name_opt); + else + strbuf_addf(&reflog_msg, "replay --onto %s", + oid_to_hex(&onto->object.oid)); + + /* Initialize ref transaction if using update mode */ + if (ref_mode == REF_ACTION_UPDATE) { + transaction = ref_store_transaction_begin(get_main_ref_store(repo), + 0, &transaction_err); + if (!transaction) { + ret = error(_("failed to begin ref transaction: %s"), + transaction_err.buf); + goto cleanup; + } + } + if (!onto) /* FIXME: Should handle replaying down to root commit */ die("Replaying down to root commit is not supported yet!"); @@ -434,10 +501,16 @@ int cmd_replay(int argc, if (decoration->type == DECORATION_REF_LOCAL && (contained || strset_contains(update_refs, decoration->name))) { - printf("update %s %s %s\n", - decoration->name, - oid_to_hex(&last_commit->object.oid), - oid_to_hex(&commit->object.oid)); + if (handle_ref_update(ref_mode, transaction, + decoration->name, + &last_commit->object.oid, + &commit->object.oid, + reflog_msg.buf, + &transaction_err) < 0) { + ret = error(_("failed to update ref '%s': %s"), + decoration->name, transaction_err.buf); + goto cleanup; + } } decoration = decoration->next; } @@ -445,10 +518,24 @@ int cmd_replay(int argc, /* In --advance mode, advance the target ref */ if (result.clean == 1 && advance_name) { - printf("update %s %s %s\n", - advance_name, - oid_to_hex(&last_commit->object.oid), - oid_to_hex(&onto->object.oid)); + if (handle_ref_update(ref_mode, transaction, advance_name, + &last_commit->object.oid, + &onto->object.oid, + reflog_msg.buf, + &transaction_err) < 0) { + ret = error(_("failed to update ref '%s': %s"), + advance_name, transaction_err.buf); + goto cleanup; + } + } + + /* Commit the ref transaction if we have one */ + if (transaction && result.clean == 1) { + if (ref_transaction_commit(transaction, &transaction_err)) { + ret = error(_("failed to commit ref transaction: %s"), + transaction_err.buf); + goto cleanup; + } } merge_finalize(&merge_opt, &result); @@ -460,6 +547,10 @@ int cmd_replay(int argc, ret = result.clean; cleanup: + if (transaction) + ref_transaction_free(transaction); + strbuf_release(&transaction_err); + strbuf_release(&reflog_msg); release_revisions(&revs); free(advance_name); diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index 58b37599357827..ec79234c8044c7 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -52,7 +52,7 @@ test_expect_success 'setup bare' ' ' test_expect_success 'using replay to rebase two branches, one on top of other' ' - git replay --onto main topic1..topic2 >result && + git replay --ref-action=print --onto main topic1..topic2 >result && test_line_count = 1 result && @@ -68,7 +68,7 @@ test_expect_success 'using replay to rebase two branches, one on top of other' ' ' test_expect_success 'using replay on bare repo to rebase two branches, one on top of other' ' - git -C bare replay --onto main topic1..topic2 >result-bare && + git -C bare replay --ref-action=print --onto main topic1..topic2 >result-bare && test_cmp expect result-bare ' @@ -86,7 +86,7 @@ test_expect_success 'using replay to perform basic cherry-pick' ' # 2nd field of result is refs/heads/main vs. refs/heads/topic2 # 4th field of result is hash for main instead of hash for topic2 - git replay --advance main topic1..topic2 >result && + git replay --ref-action=print --advance main topic1..topic2 >result && test_line_count = 1 result && @@ -102,7 +102,7 @@ test_expect_success 'using replay to perform basic cherry-pick' ' ' test_expect_success 'using replay on bare repo to perform basic cherry-pick' ' - git -C bare replay --advance main topic1..topic2 >result-bare && + git -C bare replay --ref-action=print --advance main topic1..topic2 >result-bare && test_cmp expect result-bare ' @@ -115,7 +115,7 @@ test_expect_success 'replay fails when both --advance and --onto are omitted' ' ' test_expect_success 'using replay to also rebase a contained branch' ' - git replay --contained --onto main main..topic3 >result && + git replay --ref-action=print --contained --onto main main..topic3 >result && test_line_count = 2 result && cut -f 3 -d " " result >new-branch-tips && @@ -139,12 +139,12 @@ test_expect_success 'using replay to also rebase a contained branch' ' ' test_expect_success 'using replay on bare repo to also rebase a contained branch' ' - git -C bare replay --contained --onto main main..topic3 >result-bare && + git -C bare replay --ref-action=print --contained --onto main main..topic3 >result-bare && test_cmp expect result-bare ' test_expect_success 'using replay to rebase multiple divergent branches' ' - git replay --onto main ^topic1 topic2 topic4 >result && + git replay --ref-action=print --onto main ^topic1 topic2 topic4 >result && test_line_count = 2 result && cut -f 3 -d " " result >new-branch-tips && @@ -168,7 +168,7 @@ test_expect_success 'using replay to rebase multiple divergent branches' ' ' test_expect_success 'using replay on bare repo to rebase multiple divergent branches, including contained ones' ' - git -C bare replay --contained --onto main ^main topic2 topic3 topic4 >result && + git -C bare replay --ref-action=print --contained --onto main ^main topic2 topic3 topic4 >result && test_line_count = 4 result && cut -f 3 -d " " result >new-branch-tips && @@ -217,4 +217,55 @@ test_expect_success 'merge.directoryRenames=false' ' --onto rename-onto rename-onto..rename-from ' +test_expect_success 'default atomic behavior updates refs directly' ' + # Use a separate branch to avoid contaminating topic2 for later tests + git branch test-atomic topic2 && + test_when_finished "git branch -D test-atomic" && + + # Test default atomic behavior (no output, refs updated) + git replay --onto main topic1..test-atomic >output && + test_must_be_empty output && + + # Verify ref was updated + git log --format=%s test-atomic >actual && + test_write_lines E D M L B A >expect && + test_cmp expect actual && + + # Verify reflog message includes SHA of onto commit + git reflog test-atomic -1 --format=%gs >reflog-msg && + ONTO_SHA=$(git rev-parse main) && + echo "replay --onto $ONTO_SHA" >expect-reflog && + test_cmp expect-reflog reflog-msg +' + +test_expect_success 'atomic behavior in bare repository' ' + # Store original state for cleanup + START=$(git -C bare rev-parse topic2) && + test_when_finished "git -C bare update-ref refs/heads/topic2 $START" && + + # Test atomic updates work in bare repo + git -C bare replay --onto main topic1..topic2 >output && + test_must_be_empty output && + + # Verify ref was updated in bare repo + git -C bare log --format=%s topic2 >actual && + test_write_lines E D M L B A >expect && + test_cmp expect actual +' + +test_expect_success 'reflog message for --advance mode' ' + # Store original state + START=$(git rev-parse main) && + test_when_finished "git update-ref refs/heads/main $START" && + + # Test --advance mode reflog message + git replay --advance main topic1..topic2 >output && + test_must_be_empty output && + + # Verify reflog message includes --advance and branch name + git reflog main -1 --format=%gs >reflog-msg && + echo "replay --advance main" >expect-reflog && + test_cmp expect-reflog reflog-msg +' + test_done From 336ac90c06ec757f613faae4ffc6c32578a99cd1 Mon Sep 17 00:00:00 2001 From: Siddharth Asthana Date: Thu, 6 Nov 2025 00:46:01 +0530 Subject: [PATCH 16/25] replay: add replay.refAction config option Add a configuration variable to control the default behavior of git replay for updating references. This allows users who prefer the traditional pipeline output to set it once in their config instead of passing --ref-action=print with every command. The config variable uses string values that mirror the behavior modes: * replay.refAction = update (default): atomic ref updates * replay.refAction = print: output commands for pipeline Helped-by: Junio C Hamano Helped-by: Elijah Newren Helped-by: Christian Couder Helped-by: Phillip Wood Signed-off-by: Siddharth Asthana Signed-off-by: Junio C Hamano --- Documentation/config/replay.adoc | 11 ++++++++ Documentation/git-replay.adoc | 2 ++ builtin/replay.c | 24 ++++++++++++++--- t/t3650-replay-basics.sh | 46 ++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 Documentation/config/replay.adoc diff --git a/Documentation/config/replay.adoc b/Documentation/config/replay.adoc new file mode 100644 index 00000000000000..7d549d2f0e5195 --- /dev/null +++ b/Documentation/config/replay.adoc @@ -0,0 +1,11 @@ +replay.refAction:: + Specifies the default mode for handling reference updates in + `git replay`. The value can be: ++ +-- + * `update`: Update refs directly using an atomic transaction (default behavior). + * `print`: Output update-ref commands for pipeline use. +-- ++ +This setting can be overridden with the `--ref-action` command-line option. +When not configured, `git replay` defaults to `update` mode. diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc index 2ef74ddb127b0c..dcb26e8a8e88ca 100644 --- a/Documentation/git-replay.adoc +++ b/Documentation/git-replay.adoc @@ -51,6 +51,8 @@ which uses the target only as a starting point without updating it. * `print`: Output update-ref commands for pipeline use. This is the traditional behavior where output can be piped to `git update-ref --stdin`. -- ++ +The default mode can be configured via the `replay.refAction` configuration variable. :: Range of commits to replay. More than one can diff --git a/builtin/replay.c b/builtin/replay.c index 94e60b5b107516..6606a2c94bc671 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -8,6 +8,7 @@ #include "git-compat-util.h" #include "builtin.h" +#include "config.h" #include "environment.h" #include "hex.h" #include "lockfile.h" @@ -298,6 +299,22 @@ static enum ref_action_mode parse_ref_action_mode(const char *ref_action, const die(_("invalid %s value: '%s'"), source, ref_action); } +static enum ref_action_mode get_ref_action_mode(struct repository *repo, const char *ref_action) +{ + const char *config_value = NULL; + + /* Command line option takes precedence */ + if (ref_action) + return parse_ref_action_mode(ref_action, "--ref-action"); + + /* Check config value */ + if (!repo_config_get_string_tmp(repo, "replay.refAction", &config_value)) + return parse_ref_action_mode(config_value, "replay.refAction"); + + /* Default to update mode */ + return REF_ACTION_UPDATE; +} + static int handle_ref_update(enum ref_action_mode mode, struct ref_transaction *transaction, const char *refname, @@ -332,7 +349,7 @@ int cmd_replay(int argc, const char *onto_name = NULL; int contained = 0; const char *ref_action = NULL; - enum ref_action_mode ref_mode = REF_ACTION_UPDATE; + enum ref_action_mode ref_mode; struct rev_info revs; struct commit *last_commit = NULL; @@ -378,9 +395,8 @@ int cmd_replay(int argc, die_for_incompatible_opt2(!!advance_name_opt, "--advance", contained, "--contained"); - /* Parse ref action mode */ - if (ref_action) - ref_mode = parse_ref_action_mode(ref_action, "--ref-action"); + /* Parse ref action mode from command line or config */ + ref_mode = get_ref_action_mode(repo, ref_action); advance_name = xstrdup_or_null(advance_name_opt); diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index ec79234c8044c7..cf3aacf3551f8e 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -268,4 +268,50 @@ test_expect_success 'reflog message for --advance mode' ' test_cmp expect-reflog reflog-msg ' +test_expect_success 'replay.refAction=print config option' ' + # Store original state + START=$(git rev-parse topic2) && + test_when_finished "git branch -f topic2 $START" && + + # Test with config set to print + test_config replay.refAction print && + git replay --onto main topic1..topic2 >output && + test_line_count = 1 output && + test_grep "^update refs/heads/topic2 " output +' + +test_expect_success 'replay.refAction=update config option' ' + # Store original state + START=$(git rev-parse topic2) && + test_when_finished "git branch -f topic2 $START" && + + # Test with config set to update + test_config replay.refAction update && + git replay --onto main topic1..topic2 >output && + test_must_be_empty output && + + # Verify ref was updated + git log --format=%s topic2 >actual && + test_write_lines E D M L B A >expect && + test_cmp expect actual +' + +test_expect_success 'command-line --ref-action overrides config' ' + # Store original state + START=$(git rev-parse topic2) && + test_when_finished "git branch -f topic2 $START" && + + # Set config to update but use --ref-action=print + test_config replay.refAction update && + git replay --ref-action=print --onto main topic1..topic2 >output && + test_line_count = 1 output && + test_grep "^update refs/heads/topic2 " output +' + +test_expect_success 'invalid replay.refAction value' ' + test_config replay.refAction invalid && + test_must_fail git replay --onto main topic1..topic2 2>error && + test_grep "invalid.*replay.refAction.*value" error +' + test_done From 46207a54cca1402532f6e658503a9f6b7ad36fb8 Mon Sep 17 00:00:00 2001 From: Queen Ediri Jessa Date: Wed, 5 Nov 2025 15:38:49 +0100 Subject: [PATCH 17/25] doc: clarify server behavior for invalid 'want' lines in HTTP protocol Update the documentation to clearly describe how the server responds when a client sends an invalid or malformed `want` line during the HTTP protocol exchange. The server includes the offending object name in its error message. Signed-off-by: Queen Ediri Jessa Signed-off-by: Junio C Hamano --- Documentation/gitprotocol-http.adoc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/gitprotocol-http.adoc b/Documentation/gitprotocol-http.adoc index d024010414aa6d..e2ef7f045953d8 100644 --- a/Documentation/gitprotocol-http.adoc +++ b/Documentation/gitprotocol-http.adoc @@ -443,7 +443,8 @@ If no "want" objects are received, send an error: TODO: Define error if no "want" lines are requested. If any "want" object is not reachable, send an error: -TODO: Define error if an invalid "want" is requested. +When a Git server receives an invalid or malformed `want` line, it +responds with an error message that includes the offending object name. Create an empty list, `s_common`. From 42ed0468663dd493c0a0e00edc83b668369157d6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 11 Nov 2025 17:36:47 -0500 Subject: [PATCH 18/25] attr: avoid recursion when expanding attribute macros Given a set of attribute macros like: [attr]a1 a2 [attr]a2 a3 ... [attr]a300000 -text file a1 expanding the attributes for "file" requires expanding "a1" to "a2", "a2" to "a3", and so on until hitting a non-macro expansion ("-text", in this case). We implement this via recursion: fill_one() calls macroexpand_one(), which then recurses back to fill_one(). As a result, very deep macro chains like the one above can run out of stack space and cause us to segfault. The required stack space is fairly small; I needed on the order of 200,000 entries to get a segfault on Linux. So it's unlikely anybody would hit this accidentally, leaving only malicious inputs. There you can easily construct a repo which will segfault on clone (we look at attributes during the checkout step, but you'd see the same trying to do other operations, like diff in a bare repo). It's mostly harmless, since anybody constructing such a repo is only preventing victims from cloning their evil garbage, but it could be a nuisance for hosting sites. One option to prevent this is to limit the depth of recursion we'll allow. This is conceptually easy to implement, but it raises other questions: what should the limit be, and do we need a configuration knob for it? The recursion here is simple enough that we can avoid those questions by just converting it to iteration instead. Rather than iterate over the states of a match_attr in fill_one(), we'll put them all in a queue, and the expansion of each can add to the queue rather than recursing. Note that this is a LIFO queue in order to keep the same depth-first order we did with the recursive implementation. I've avoided using the word "stack" in the code because the term is already heavily used to refer to the stack of .gitattribute files that matches the tree structure of the repository. The test uses a limited stack size so we can trigger the problem with a much smaller input than the one shown above. The value here (3000) is enough to trigger the issue on my x86_64 Linux machine. Reported-by: Ben Stav Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- attr.c | 50 +++++++++++++++++++++++++++++-------------- t/t0003-attributes.sh | 20 +++++++++++++++++ 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/attr.c b/attr.c index d1daeb0b4d90a6..4999b7e09da930 100644 --- a/attr.c +++ b/attr.c @@ -1064,24 +1064,52 @@ static int path_matches(const char *pathname, int pathlen, pattern, prefix, pat->patternlen); } -static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem); +struct attr_state_queue { + const struct attr_state **items; + size_t alloc, nr; +}; + +static void attr_state_queue_push(struct attr_state_queue *t, + const struct match_attr *a) +{ + for (size_t i = 0; i < a->num_attr; i++) { + ALLOC_GROW(t->items, t->nr + 1, t->alloc); + t->items[t->nr++] = &a->state[i]; + } +} + +static const struct attr_state *attr_state_queue_pop(struct attr_state_queue *t) +{ + return t->nr ? t->items[--t->nr] : NULL; +} + +static void attr_state_queue_release(struct attr_state_queue *t) +{ + free(t->items); +} static int fill_one(struct all_attrs_item *all_attrs, const struct match_attr *a, int rem) { - size_t i; + struct attr_state_queue todo = { 0 }; + const struct attr_state *state; - for (i = a->num_attr; rem > 0 && i > 0; i--) { - const struct git_attr *attr = a->state[i - 1].attr; + attr_state_queue_push(&todo, a); + while (rem > 0 && (state = attr_state_queue_pop(&todo))) { + const struct git_attr *attr = state->attr; const char **n = &(all_attrs[attr->attr_nr].value); - const char *v = a->state[i - 1].setto; + const char *v = state->setto; if (*n == ATTR__UNKNOWN) { + const struct all_attrs_item *item = + &all_attrs[attr->attr_nr]; *n = v; rem--; - rem = macroexpand_one(all_attrs, attr->attr_nr, rem); + if (item->macro && item->value == ATTR__TRUE) + attr_state_queue_push(&todo, item->macro); } } + attr_state_queue_release(&todo); return rem; } @@ -1106,16 +1134,6 @@ static int fill(const char *path, int pathlen, int basename_offset, return rem; } -static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem) -{ - const struct all_attrs_item *item = &all_attrs[nr]; - - if (item->macro && item->value == ATTR__TRUE) - return fill_one(all_attrs, item->macro, rem); - else - return rem; -} - /* * Marks the attributes which are macros based on the attribute stack. * This prevents having to search through the attribute stack each time diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 3c98b622f25b76..582e207aa12eb1 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -664,4 +664,24 @@ test_expect_success 'user defined builtin_objectmode values are ignored' ' test_cmp expect err ' +test_expect_success ULIMIT_STACK_SIZE 'deep macro recursion' ' + n=3000 && + { + i=0 && + while test $i -lt $n; do + echo "[attr]a$i a$((i+1))" && + i=$((i+1)) || + return 1 + done && + echo "[attr]a$n -text" && + echo "file a0" + } >.gitattributes && + { + echo "file: text: unset" && + test_seq -f "file: a%d: set" 0 $n + } >expect && + run_with_limited_stack git check-attr -a file >actual && + test_cmp expect actual +' + test_done From 4580bcd2354aab9369164d936f7ccaa21fc98c98 Mon Sep 17 00:00:00 2001 From: Koji Nakamaru Date: Fri, 14 Nov 2025 06:04:30 +0000 Subject: [PATCH 19/25] osxkeychain: avoid incorrectly skipping store operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git-credential-osxkeychain skips storing a credential if its "get" action sets "state[]=osxkeychain:seen=1". This behavior was introduced in e1ab45b2 (osxkeychain: state to skip unnecessary store operations, 2024-05-15), which appeared in v2.46. However, this state[] persists even if a credential returned by "git-credential-osxkeychain get" is invalid and a subsequent helper's "get" operation returns a valid credential. Another subsequent helper (such as [1]) may expect git-credential-osxkeychain to store the valid credential, but the "store" operation is incorrectly skipped because it only checks "state[]=osxkeychain:seen=1". To solve this issue, "state[]=osxkeychain:seen" needs to contain enough information to identify whether the current "store" input matches the output from the previous "get" operation (and not a credential from another helper). Set "state[]=osxkeychain:seen" to a value encoding the credential output by "get", and compare it with a value encoding the credential input by "store". [1]: https://github.com/hickford/git-credential-oauth Reported-by: Petter Sælen Helped-by: Junio C Hamano Helped-by: brian m. carlson Signed-off-by: Koji Nakamaru Signed-off-by: Junio C Hamano --- contrib/credential/osxkeychain/Makefile | 41 +++++- .../osxkeychain/git-credential-osxkeychain.c | 120 ++++++++++++++---- contrib/credential/osxkeychain/meson.build | 1 + 3 files changed, 132 insertions(+), 30 deletions(-) diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile index 9680717abe44c6..c68445b82dc3e5 100644 --- a/contrib/credential/osxkeychain/Makefile +++ b/contrib/credential/osxkeychain/Makefile @@ -1,21 +1,55 @@ # The default target of this Makefile is... all:: git-credential-osxkeychain +include ../../../config.mak.uname -include ../../../config.mak.autogen -include ../../../config.mak +ifdef ZLIB_NG + BASIC_CFLAGS += -DHAVE_ZLIB_NG + ifdef ZLIB_NG_PATH + BASIC_CFLAGS += -I$(ZLIB_NG_PATH)/include + EXTLIBS += $(call libpath_template,$(ZLIB_NG_PATH)/$(lib)) + endif + EXTLIBS += -lz-ng +else + ifdef ZLIB_PATH + BASIC_CFLAGS += -I$(ZLIB_PATH)/include + EXTLIBS += $(call libpath_template,$(ZLIB_PATH)/$(lib)) + endif + EXTLIBS += -lz +endif +ifndef NO_ICONV + ifdef NEEDS_LIBICONV + ifdef ICONVDIR + BASIC_CFLAGS += -I$(ICONVDIR)/include + ICONV_LINK = $(call libpath_template,$(ICONVDIR)/$(lib)) + else + ICONV_LINK = + endif + ifdef NEEDS_LIBINTL_BEFORE_LIBICONV + ICONV_LINK += -lintl + endif + EXTLIBS += $(ICONV_LINK) -liconv + endif +endif +ifndef LIBC_CONTAINS_LIBINTL + EXTLIBS += -lintl +endif + prefix ?= /usr/local gitexecdir ?= $(prefix)/libexec/git-core CC ?= gcc -CFLAGS ?= -g -O2 -Wall +CFLAGS ?= -g -O2 -Wall -I../../.. $(BASIC_CFLAGS) +LDFLAGS ?= $(BASIC_LDFLAGS) $(EXTLIBS) INSTALL ?= install RM ?= rm -f %.o: %.c $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $< -git-credential-osxkeychain: git-credential-osxkeychain.o +git-credential-osxkeychain: git-credential-osxkeychain.o ../../../libgit.a $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) \ -framework Security -framework CoreFoundation @@ -23,6 +57,9 @@ install: git-credential-osxkeychain $(INSTALL) -d -m 755 $(DESTDIR)$(gitexecdir) $(INSTALL) -m 755 $< $(DESTDIR)$(gitexecdir) +../../../libgit.a: + cd ../../..; make libgit.a + clean: $(RM) git-credential-osxkeychain git-credential-osxkeychain.o diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c b/contrib/credential/osxkeychain/git-credential-osxkeychain.c index 611c9798b3ae5c..b18026703418a9 100644 --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c @@ -2,6 +2,9 @@ #include #include #include +#include "git-compat-util.h" +#include "strbuf.h" +#include "wrapper.h" #define ENCODING kCFStringEncodingUTF8 static CFStringRef protocol; /* Stores constant strings - not memory managed */ @@ -12,7 +15,7 @@ static CFStringRef username; static CFDataRef password; static CFDataRef password_expiry_utc; static CFDataRef oauth_refresh_token; -static int state_seen; +static char *state_seen; static void clear_credential(void) { @@ -48,27 +51,6 @@ static void clear_credential(void) #define STRING_WITH_LENGTH(s) s, sizeof(s) - 1 -__attribute__((format (printf, 1, 2), __noreturn__)) -static void die(const char *err, ...) -{ - char msg[4096]; - va_list params; - va_start(params, err); - vsnprintf(msg, sizeof(msg), err, params); - fprintf(stderr, "%s\n", msg); - va_end(params); - clear_credential(); - exit(1); -} - -static void *xmalloc(size_t len) -{ - void *ret = malloc(len); - if (!ret) - die("Out of memory"); - return ret; -} - static CFDictionaryRef create_dictionary(CFAllocatorRef allocator, ...) { va_list args; @@ -112,6 +94,66 @@ static void write_item(const char *what, const char *buf, size_t len) putchar('\n'); } +static void write_item_strbuf(struct strbuf *sb, const char *what, const char *buf, int n) +{ + char s[32]; + + xsnprintf(s, sizeof(s), "__%s=", what); + strbuf_add(sb, s, strlen(s)); + strbuf_add(sb, buf, n); +} + +static void write_item_strbuf_cfstring(struct strbuf *sb, const char *what, CFStringRef ref) +{ + char *buf; + int len; + + if (!ref) + return; + len = CFStringGetMaximumSizeForEncoding(CFStringGetLength(ref), ENCODING) + 1; + buf = xmalloc(len); + if (CFStringGetCString(ref, buf, len, ENCODING)) + write_item_strbuf(sb, what, buf, strlen(buf)); + free(buf); +} + +static void write_item_strbuf_cfnumber(struct strbuf *sb, const char *what, CFNumberRef ref) +{ + short n; + char buf[32]; + + if (!ref) + return; + if (!CFNumberGetValue(ref, kCFNumberShortType, &n)) + return; + xsnprintf(buf, sizeof(buf), "%d", n); + write_item_strbuf(sb, what, buf, strlen(buf)); +} + +static void write_item_strbuf_cfdata(struct strbuf *sb, const char *what, CFDataRef ref) +{ + char *buf; + int len; + + if (!ref) + return; + buf = (char *)CFDataGetBytePtr(ref); + if (!buf || strlen(buf) == 0) + return; + len = CFDataGetLength(ref); + write_item_strbuf(sb, what, buf, len); +} + +static void encode_state_seen(struct strbuf *sb) +{ + strbuf_add(sb, "osxkeychain:seen=", strlen("osxkeychain:seen=")); + write_item_strbuf_cfstring(sb, "host", host); + write_item_strbuf_cfnumber(sb, "port", port); + write_item_strbuf_cfstring(sb, "path", path); + write_item_strbuf_cfstring(sb, "username", username); + write_item_strbuf_cfdata(sb, "password", password); +} + static void find_username_in_item(CFDictionaryRef item) { CFStringRef account_ref; @@ -124,6 +166,7 @@ static void find_username_in_item(CFDictionaryRef item) write_item("username", "", 0); return; } + username = CFStringCreateCopy(kCFAllocatorDefault, account_ref); username_buf = (char *)CFStringGetCStringPtr(account_ref, ENCODING); if (username_buf) @@ -163,6 +206,7 @@ static OSStatus find_internet_password(void) } data = CFDictionaryGetValue(item, kSecValueData); + password = CFDataCreateCopy(kCFAllocatorDefault, data); write_item("password", (const char *)CFDataGetBytePtr(data), @@ -173,7 +217,14 @@ static OSStatus find_internet_password(void) CFRelease(item); write_item("capability[]", "state", strlen("state")); - write_item("state[]", "osxkeychain:seen=1", strlen("osxkeychain:seen=1")); + { + struct strbuf sb; + + strbuf_init(&sb, 1024); + encode_state_seen(&sb); + write_item("state[]", sb.buf, strlen(sb.buf)); + strbuf_release(&sb); + } out: CFRelease(attrs); @@ -288,13 +339,22 @@ static OSStatus add_internet_password(void) CFDictionaryRef attrs; OSStatus result; - if (state_seen) - return errSecSuccess; - /* Only store complete credentials */ if (!protocol || !host || !username || !password) return -1; + if (state_seen) { + struct strbuf sb; + + strbuf_init(&sb, 1024); + encode_state_seen(&sb); + if (!strcmp(state_seen, sb.buf)) { + strbuf_release(&sb); + return errSecSuccess; + } + strbuf_release(&sb); + } + data = CFDataCreateMutableCopy(kCFAllocatorDefault, 0, password); if (password_expiry_utc) { CFDataAppendBytes(data, @@ -403,8 +463,9 @@ static void read_credential(void) (UInt8 *)v, strlen(v)); else if (!strcmp(buf, "state[]")) { - if (!strcmp(v, "osxkeychain:seen=1")) - state_seen = 1; + int len = strlen("osxkeychain:seen="); + if (!strncmp(v, "osxkeychain:seen=", len)) + state_seen = xstrdup(v); } /* * Ignore other lines; we don't know what they mean, but @@ -443,5 +504,8 @@ int main(int argc, const char **argv) clear_credential(); + if (state_seen) + free(state_seen); + return 0; } diff --git a/contrib/credential/osxkeychain/meson.build b/contrib/credential/osxkeychain/meson.build index 3c7677f736c684..ec91d0c14b0eb1 100644 --- a/contrib/credential/osxkeychain/meson.build +++ b/contrib/credential/osxkeychain/meson.build @@ -1,6 +1,7 @@ executable('git-credential-osxkeychain', sources: 'git-credential-osxkeychain.c', dependencies: [ + libgit, dependency('CoreFoundation'), dependency('Security'), ], From df90eccd931dfd8e6ecbc0c18c5037c85cc115dc Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Fri, 14 Nov 2025 15:04:47 +0100 Subject: [PATCH 20/25] doc: commit: link to git-status(1) on all format options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--branch` and `--long` refer to git-status(1) options but they don’t tell us what `short-format` and `long-format` are, respectively. And `--null` mentions “status” but does not link to the command. Refer to git-config(1) on `--branch` like `--short` does. `long-format` is the git-status(1) output. So we can just say that directly. Replace “status” with a `linkgit` on `--null`. Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- Documentation/git-commit.adoc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Documentation/git-commit.adoc b/Documentation/git-commit.adoc index 54c207ad45eaa2..8329c1034b9b30 100644 --- a/Documentation/git-commit.adoc +++ b/Documentation/git-commit.adoc @@ -146,7 +146,8 @@ See linkgit:git-rebase[1] for details. linkgit:git-status[1] for details. Implies `--dry-run`. `--branch`:: - Show the branch and tracking info even in short-format. + Show the branch and tracking info even in short-format. See + linkgit:git-status[1] for details. `--porcelain`:: When doing a dry-run, give the output in a porcelain-ready @@ -154,12 +155,13 @@ See linkgit:git-rebase[1] for details. `--dry-run`. `--long`:: - When doing a dry-run, give the output in the long-format. - Implies `--dry-run`. + When doing a dry-run, give the output in the long-format. This + is the default output of linkgit:git-status[1]. Implies + `--dry-run`. `-z`:: `--null`:: - When showing `short` or `porcelain` status output, print the + When showing `short` or `porcelain` linkgit:git-status[1] output, print the filename verbatim and terminate the entries with _NUL_, instead of _LF_. If no format is given, implies the `--porcelain` output format. Without the `-z` option, filenames with "unusual" characters are From 66c78e0653a4e60c625b8400da31da0ba5bd1286 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Sat, 15 Nov 2025 00:58:17 +0000 Subject: [PATCH 21/25] object-file: disallow adding submodules of different hash algo The design of the hash algorithm transition plan is that objects stored must be entirely in one algorithm since we lack any way to indicate a mix of algorithms. This also includes submodules, but we have traditionally not enforced this, which leads to various problems when trying to clone or check out the the submodule from the remote. Since this cannot work in the general case, restrict adding a submodule of a different algorithm to the index. Add tests for git add and git submodule add that these are rejected. Note that we cannot check this in git fsck because the malformed submodule is stored in the tree as an object ID which is either truncated (when a SHA-256 submodule is added to a SHA-1 repository) or padded with zeros (when a SHA-1 submodule is added to a SHA-256 repository). We cannot detect even the latter case because someone could have an actual submodule that actually ends in 24 zeros, which would be a false positive. Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- object-file.c | 6 +++++- t/t3700-add.sh | 25 +++++++++++++++++++++++++ t/t7400-submodule-basic.sh | 25 +++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/object-file.c b/object-file.c index 2bc36ab3ee8cbf..6ff6cf75499d30 100644 --- a/object-file.c +++ b/object-file.c @@ -1296,7 +1296,11 @@ int index_path(struct index_state *istate, struct object_id *oid, strbuf_release(&sb); break; case S_IFDIR: - return repo_resolve_gitlink_ref(istate->repo, path, "HEAD", oid); + if (repo_resolve_gitlink_ref(istate->repo, path, "HEAD", oid)) + return -1; + if (&hash_algos[oid->algo] != istate->repo->hash_algo) + return error(_("cannot add a submodule of a different hash algorithm")); + break; default: return error(_("%s: unsupported file type"), path); } diff --git a/t/t3700-add.sh b/t/t3700-add.sh index df580a5806b4f1..9a2c8dbcc23d91 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -541,6 +541,31 @@ test_expect_success 'all statuses changed in folder if . is given' ' ) ' +test_expect_success 'cannot add a submodule of a different algorithm' ' + git init --object-format=sha256 sha256 && + ( + cd sha256 && + test_commit abc && + git init --object-format=sha1 submodule && + test_commit -C submodule def && + test_must_fail git add submodule 2>err && + test_grep "cannot add a submodule of a different hash algorithm" err && + git ls-files --stage >entries && + test_grep ! ^160000 entries + ) && + git init --object-format=sha1 sha1 && + ( + cd sha1 && + test_commit abc && + git init --object-format=sha256 submodule && + test_commit -C submodule def && + test_must_fail git add submodule 2>err && + test_grep "cannot add a submodule of a different hash algorithm" err && + git ls-files --stage >entries && + test_grep ! ^160000 entries + ) +' + test_expect_success CASE_INSENSITIVE_FS 'path is case-insensitive' ' path="$(pwd)/BLUB" && touch "$path" && diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index fd3e7e355e4ffc..e6b551daad7e58 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -407,6 +407,31 @@ test_expect_success 'submodule add in subdirectory with relative path should fai test_grep toplevel output.err ' +test_expect_success 'submodule add of a different algorithm fails' ' + git init --object-format=sha256 sha256 && + ( + cd sha256 && + test_commit abc && + git init --object-format=sha1 submodule && + test_commit -C submodule def && + test_must_fail git submodule add "$submodurl" submodule 2>err && + test_grep "cannot add a submodule of a different hash algorithm" err && + git ls-files --stage >entries && + test_grep ! ^160000 entries + ) && + git init --object-format=sha1 sha1 && + ( + cd sha1 && + test_commit abc && + git init --object-format=sha256 submodule && + test_commit -C submodule def && + test_must_fail git submodule add "$submodurl" submodule 2>err && + test_grep "cannot add a submodule of a different hash algorithm" err && + git ls-files --stage >entries && + test_grep ! ^160000 entries + ) +' + test_expect_success 'setup - add an example entry to .gitmodules' ' git config --file=.gitmodules submodule.example.url git://example.com/init.git ' From 6fe288bfbcbbabc3d399dd71f876dccf71affff0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 15 Nov 2025 00:58:18 +0000 Subject: [PATCH 22/25] read-cache: drop submodule check from add_to_cache() In add_to_cache(), we treat any directories as submodules, and complain if we can't resolve their HEAD. This call to resolve_gitlink_ref() was added by f937bc2f86 (add: error appropriately on repository with no commits, 2019-04-09), with the goal of improving the error message for empty repositories. But we already resolve the submodule HEAD in index_path(), which is where we find the actual oid we're going to use. Resolving it again here introduces some downsides: 1. It's more work, since we have to open up the submodule repository's files twice. 2. There are call paths that get to index_path() without going through add_to_cache(). For instance, we'd want a similar informative message if "git diff empty" finds that it can't resolve the submodule's HEAD. (In theory we can also get there through update-index, but AFAICT it refuses to consider directories as submodules at all, and just complains about them). 3. The resolution in index_path() catches more errors that we don't handle here. In particular, it will validate that the object format for the submodule matches that of the superproject. This isn't a bug, since our call in add_to_cache() throws away the oid it gets without looking at it. But it certainly caused confusion for me when looking at where the object-format check should go. So instead of resolving the submodule HEAD in add_to_cache(), let's just teach the call in index_path() to actually produce an error message (which it already does for other cases). That's probably what f937bc2f86 should have done in the first place, and it gives us a single point of resolution when adding a submodule to the index. The resulting output is slightly more verbose, as we propagate the error up the call stack, but I think that's OK (and again, matches many other errors we get when indexing fails). I've left the text of the error message as-is, though it is perhaps overly specific. There are many reasons that resolving the submodule HEAD might fail, though outside of corruption or system errors it is probably most likely that the submodule HEAD is simply on an unborn branch. Signed-off-by: Jeff King Signed-off-by: brian m. carlson Signed-off-by: Junio C Hamano --- object-file.c | 2 +- read-cache.c | 3 --- t/t3700-add.sh | 1 + 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/object-file.c b/object-file.c index 6ff6cf75499d30..bb0c77b45d89a5 100644 --- a/object-file.c +++ b/object-file.c @@ -1297,7 +1297,7 @@ int index_path(struct index_state *istate, struct object_id *oid, break; case S_IFDIR: if (repo_resolve_gitlink_ref(istate->repo, path, "HEAD", oid)) - return -1; + return error(_("'%s' does not have a commit checked out"), path); if (&hash_algos[oid->algo] != istate->repo->hash_algo) return error(_("cannot add a submodule of a different hash algorithm")); break; diff --git a/read-cache.c b/read-cache.c index 06ad74db2286ae..e34c5c56c637fd 100644 --- a/read-cache.c +++ b/read-cache.c @@ -707,7 +707,6 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, int add_option = (ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE| (intent_only ? ADD_CACHE_NEW_ONLY : 0)); unsigned hash_flags = pretend ? 0 : INDEX_WRITE_OBJECT; - struct object_id oid; if (flags & ADD_CACHE_RENORMALIZE) hash_flags |= INDEX_RENORMALIZE; @@ -717,8 +716,6 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st, namelen = strlen(path); if (S_ISDIR(st_mode)) { - if (repo_resolve_gitlink_ref(the_repository, path, "HEAD", &oid) < 0) - return error(_("'%s' does not have a commit checked out"), path); while (namelen && path[namelen-1] == '/') namelen--; } diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 9a2c8dbcc23d91..af93e53c12cfe3 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -388,6 +388,7 @@ test_expect_success 'error on a repository with no commits' ' test_must_fail git add empty >actual 2>&1 && cat >expect <<-EOF && error: '"'empty/'"' does not have a commit checked out + error: unable to index file '"'empty/'"' fatal: adding files failed EOF test_cmp expect actual From 878fef8ebf6cf513842de14284ee58f4d92fcef3 Mon Sep 17 00:00:00 2001 From: Jiang Xin Date: Sat, 15 Nov 2025 08:36:10 -0500 Subject: [PATCH 23/25] t/unit-tests: add UTF-8 width tests for CJK chars The file "builtin/repo.c" uses utf8_strwidth() to calculate the display width of UTF-8 characters in a table, but the resulting output is still misaligned. Add test cases for both utf8_strwidth and utf8_strnwidth to verify that they correctly compute the display width for UTF-8 characters. Also updated the build configuration in Makefile and meson.build to include the new test suite in the build process. Signed-off-by: Jiang Xin Signed-off-by: Junio C Hamano --- Makefile | 1 + t/meson.build | 1 + t/unit-tests/u-utf8-width.c | 97 +++++++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 t/unit-tests/u-utf8-width.c diff --git a/Makefile b/Makefile index 7e0f77e2988e3b..2a675461540c26 100644 --- a/Makefile +++ b/Makefile @@ -1525,6 +1525,7 @@ CLAR_TEST_SUITES += u-string-list CLAR_TEST_SUITES += u-strvec CLAR_TEST_SUITES += u-trailer CLAR_TEST_SUITES += u-urlmatch-normalization +CLAR_TEST_SUITES += u-utf8-width CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X) CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES)) CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o diff --git a/t/meson.build b/t/meson.build index a5531df415ffe2..dc43d69636d15c 100644 --- a/t/meson.build +++ b/t/meson.build @@ -24,6 +24,7 @@ clar_test_suites = [ 'unit-tests/u-strvec.c', 'unit-tests/u-trailer.c', 'unit-tests/u-urlmatch-normalization.c', + 'unit-tests/u-utf8-width.c', ] clar_sources = [ diff --git a/t/unit-tests/u-utf8-width.c b/t/unit-tests/u-utf8-width.c new file mode 100644 index 00000000000000..3766f19726ed9d --- /dev/null +++ b/t/unit-tests/u-utf8-width.c @@ -0,0 +1,97 @@ +#include "unit-test.h" +#include "utf8.h" +#include "strbuf.h" + +/* + * Test utf8_strnwidth with various Chinese strings + * Chinese characters typically have a width of 2 columns when displayed + */ +void test_utf8_width__strnwidth_chinese(void) +{ + const char *str; + + /* Test basic ASCII - each character should have width 1 */ + cl_assert_equal_i(5, utf8_strnwidth("Hello", 5, 0)); + /* skip_ansi = 1 */ + cl_assert_equal_i(5, utf8_strnwidth("Hello", 5, 1)); + + /* Test simple Chinese characters - each should have width 2 */ + /* "你好" is 6 bytes (3 bytes per char in UTF-8), 4 display columns */ + cl_assert_equal_i(4, utf8_strnwidth("你好", 6, 0)); + + /* Test mixed ASCII and Chinese - ASCII = 1 column, Chinese = 2 columns */ + /* "h"(1) + "i"(1) + "你"(2) + "好"(2) = 6 */ + cl_assert_equal_i(6, utf8_strnwidth("Hi你好", 8, 0)); + + /* Test longer Chinese string */ + /* 5 Chinese chars = 10 display columns */ + cl_assert_equal_i(10, utf8_strnwidth("你好世界!", 15, 0)); + + /* Test individual Chinese character width */ + cl_assert_equal_i(2, utf8_strnwidth("中", 3, 0)); + + /* Test empty string */ + cl_assert_equal_i(0, utf8_strnwidth("", 0, 0)); + + /* Test length limiting */ + str = "你好世界"; + /* Only first char "你"(2 columns) within 3 bytes */ + cl_assert_equal_i(2, utf8_strnwidth(str, 3, 0)); + /* First two chars "你好"(4 columns) in 6 bytes */ + cl_assert_equal_i(4, utf8_strnwidth(str, 6, 0)); +} + +/* + * Tests for utf8_strwidth (simpler version without length limit) + */ +void test_utf8_width__strwidth_chinese(void) +{ + /* Test basic ASCII */ + cl_assert_equal_i(5, utf8_strwidth("Hello")); + + /* Test Chinese characters */ + /* 2 Chinese chars = 4 display columns */ + cl_assert_equal_i(4, utf8_strwidth("你好")); + + /* Test longer Chinese string */ + /* 5 Chinese chars = 10 display columns */ + cl_assert_equal_i(10, utf8_strwidth("你好世界!")); + + /* Test mixed ASCII and Chinese */ + /* 5 ASCII (5 cols) + 2 Chinese (4 cols) = 9 */ + cl_assert_equal_i(9, utf8_strwidth("Hello世界")); + /* 2 ASCII (2 cols) + 2 Chinese (4 cols) + 1 ASCII (1 col) = 7 */ + cl_assert_equal_i(7, utf8_strwidth("Hi世界!")); +} + +/* + * Additional tests with other East Asian characters + */ +void test_utf8_width__strnwidth_japanese_korean(void) +{ + /* Japanese characters (should also be 2 columns each) */ + /* 5 Japanese chars x 2 cols each = 10 display columns */ + cl_assert_equal_i(10, utf8_strnwidth("こんにちは", 15, 0)); + + /* Korean characters (should also be 2 columns each) */ + /* 5 Korean chars x 2 cols each = 10 display columns */ + cl_assert_equal_i(10, utf8_strnwidth("안녕하세요", 15, 0)); +} + +/* + * Test utf8_strnwidth with CJK strings and ANSI sequences + */ +void test_utf8_width__strnwidth_cjk_with_ansi(void) +{ + /* Test CJK with ANSI sequences */ + const char *ansi_test = "\033[1m你好\033[0m"; + int width = utf8_strnwidth(ansi_test, strlen(ansi_test), 1); + /* Should skip ANSI sequences and count "你好" as 4 columns */ + cl_assert_equal_i(4, width); + + /* Test mixed ASCII, CJK, and ANSI */ + ansi_test = "Hello\033[32m世界\033[0m!"; + width = utf8_strnwidth(ansi_test, strlen(ansi_test), 1); + /* "Hello"(5) + "世界"(4) + "!"(1) = 10 */ + cl_assert_equal_i(10, width); +} From 7a03a10a3a746dd8565a3a0e6126f60523b41738 Mon Sep 17 00:00:00 2001 From: Jiang Xin Date: Sat, 15 Nov 2025 08:36:11 -0500 Subject: [PATCH 24/25] builtin/repo: fix table alignment for UTF-8 characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The output table from "git repo structure" is misaligned when displaying UTF-8 characters (e.g., non-ASCII glyphs). E.g.: | 仓库结构 | 值 | | -------------- | ---- | | * 引用 | | | * 计数 | 67 | The previous implementation used simple width formatting with printf() which didn't properly handle multi-byte UTF-8 characters, causing misaligned table columns when displaying repository structure information. This change modifies the stats_table_print_structure function to use strbuf_utf8_align() instead of basic printf width specifiers. This ensures proper column alignment regardless of the character encoding of the content being displayed. Also add test cases for strbuf_utf8_align(), a function newly introduced in "builtin/repo.c". Signed-off-by: Jiang Xin Signed-off-by: Junio C Hamano --- builtin/repo.c | 21 +++++++++++++++++---- t/unit-tests/u-utf8-width.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/builtin/repo.c b/builtin/repo.c index 9d4749f79befa8..e3adb353a24ae8 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -292,14 +292,20 @@ static void stats_table_print_structure(const struct stats_table *table) int name_col_width = utf8_strwidth(name_col_title); int value_col_width = utf8_strwidth(value_col_title); struct string_list_item *item; + struct strbuf buf = STRBUF_INIT; if (table->name_col_width > name_col_width) name_col_width = table->name_col_width; if (table->value_col_width > value_col_width) value_col_width = table->value_col_width; - printf("| %-*s | %-*s |\n", name_col_width, name_col_title, - value_col_width, value_col_title); + strbuf_addstr(&buf, "| "); + strbuf_utf8_align(&buf, ALIGN_LEFT, name_col_width, name_col_title); + strbuf_addstr(&buf, " | "); + strbuf_utf8_align(&buf, ALIGN_LEFT, value_col_width, value_col_title); + strbuf_addstr(&buf, " |"); + printf("%s\n", buf.buf); + printf("| "); for (int i = 0; i < name_col_width; i++) putchar('-'); @@ -317,9 +323,16 @@ static void stats_table_print_structure(const struct stats_table *table) value = entry->value; } - printf("| %-*s | %*s |\n", name_col_width, item->string, - value_col_width, value); + strbuf_reset(&buf); + strbuf_addstr(&buf, "| "); + strbuf_utf8_align(&buf, ALIGN_LEFT, name_col_width, item->string); + strbuf_addstr(&buf, " | "); + strbuf_utf8_align(&buf, ALIGN_RIGHT, value_col_width, value); + strbuf_addstr(&buf, " |"); + printf("%s\n", buf.buf); } + + strbuf_release(&buf); } static void stats_table_clear(struct stats_table *table) diff --git a/t/unit-tests/u-utf8-width.c b/t/unit-tests/u-utf8-width.c index 3766f19726ed9d..86e09c3574a331 100644 --- a/t/unit-tests/u-utf8-width.c +++ b/t/unit-tests/u-utf8-width.c @@ -95,3 +95,40 @@ void test_utf8_width__strnwidth_cjk_with_ansi(void) /* "Hello"(5) + "世界"(4) + "!"(1) = 10 */ cl_assert_equal_i(10, width); } + +/* + * Test the strbuf_utf8_align function with CJK characters + */ +void test_utf8_width__strbuf_utf8_align(void) +{ + struct strbuf buf = STRBUF_INIT; + + /* Test left alignment with CJK */ + strbuf_utf8_align(&buf, ALIGN_LEFT, 10, "你好"); + /* Since "你好" is 4 display columns, we need 6 more spaces to reach 10 */ + cl_assert_equal_s("你好 ", buf.buf); + strbuf_reset(&buf); + + /* Test right alignment with CJK */ + strbuf_utf8_align(&buf, ALIGN_RIGHT, 8, "世界"); + /* "世界" is 4 display columns, so we need 4 leading spaces */ + cl_assert_equal_s(" 世界", buf.buf); + strbuf_reset(&buf); + + /* Test center alignment with CJK */ + strbuf_utf8_align(&buf, ALIGN_MIDDLE, 10, "中"); + /* "中" is 2 display columns, so (10-2)/2 = 4 spaces on left, 4 on right */ + cl_assert_equal_s(" 中 ", buf.buf); + strbuf_reset(&buf); + + strbuf_utf8_align(&buf, ALIGN_MIDDLE, 5, "中"); + /* "中" is 2 display columns, so (5-2)/2 = 1 spaces on left, 2 on right */ + cl_assert_equal_s(" 中 ", buf.buf); + strbuf_reset(&buf); + + /* Test alignment that is smaller than string width */ + strbuf_utf8_align(&buf, ALIGN_LEFT, 2, "你好"); + /* Since "你好" is 4 display columns, it should not be truncated */ + cl_assert_equal_s("你好", buf.buf); + strbuf_release(&buf); +} From 6ab38b7e9cc7adafc304f3204616a4debd49c6e9 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 24 Nov 2025 15:46:25 -0800 Subject: [PATCH 25/25] The third batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.53.0.adoc | 33 +++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.53.0.adoc b/Documentation/RelNotes/2.53.0.adoc index 997ae7476c2942..7882bc59e80a92 100644 --- a/Documentation/RelNotes/2.53.0.adoc +++ b/Documentation/RelNotes/2.53.0.adoc @@ -7,6 +7,10 @@ UI, Workflows & Features * "git maintenance" command learned "is-needed" subcommand to tell if it is necessary to perform various maintenance tasks. + * "git replay" (experimental) learned to perform ref updates itself + in a transaction by default, instead of emitting where each refs + should point at and leaving the actual update to another command. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -22,10 +26,37 @@ Performance, Internal Implementation, Development Support etc. changes, disable rename/copy detection to skip more expensive processing whose result will be discarded anyway. + * A part of code paths that deals with loose objects has been cleaned + up. + -Fixes since v2.51 +Fixes since v2.52 ----------------- * Ever since we added whitespace rules for this project, we misspelt an entry, which has been corrected. (merge 358e94dc70 jc/gitattributes-whitespace-no-indent-fix later to maint). + + * The code to expand attribute macros has been rewritten to avoid + recursion to avoid running out of stack space in an uncontrolled + way. + (merge 42ed046866 jk/attr-macroexpand-wo-recursion later to maint). + + * Adding a repository that uses a different hash function is a no-no, + but "git submodule add" did nt prevent it, which has been corrected. + (merge 6fe288bfbc bc/submodule-force-same-hash later to maint). + + * An earlier check added to osx keychain credential helper to avoid + storing the credential itself supplied was overeager and rejected + credential material supplied by other helper backends that it would + have wanted to store, which has been corrected. + (merge 4580bcd235 kn/osxkeychain-idempotent-store-fix later to maint). + + * The "git repo structure" subcommand tried to align its output but + mixed up byte count and display column width, which has been + corrected. + (merge 7a03a10a3a jx/repo-struct-utf8width-fix later to maint). + + * Other code cleanup, docfix, build fix, etc. + (merge 46207a54cc qj/doc-http-bad-want-response later to maint). + (merge df90eccd93 kh/doc-commit-extra-references later to maint).