From 882c8e351d700e1738e696dfbc6312617f394570 Mon Sep 17 00:00:00 2001 From: Jayesh Daga Date: Tue, 31 Mar 2026 10:02:53 +0000 Subject: [PATCH 01/21] cache-tree: use index state repository in trace2 calls trace2 calls in cache-tree.c use the global 'the_repository', even though cache_tree_update() has access to an explicit repository pointer via 'istate->repo'. Using the global repository can result in incorrect trace2 output when multiple repository instances are in use, as events may be attributed to the wrong repository. Use 'istate->repo' in cache_tree_update() to ensure correct repository attribution. Other call sites are left unchanged as they do not have access to a repository instance. Signed-off-by: Jayesh Daga Signed-off-by: Junio C Hamano --- cache-tree.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 60bcc07c3b8357..e4f9174c4a35cf 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -488,12 +488,12 @@ int cache_tree_update(struct index_state *istate, int flags) prefetch_cache_entries(istate, must_check_existence); trace_performance_enter(); - trace2_region_enter("cache_tree", "update", the_repository); + trace2_region_enter("cache_tree", "update", istate->repo); transaction = odb_transaction_begin(the_repository->objects); i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, "", 0, &skip, flags); odb_transaction_commit(transaction); - trace2_region_leave("cache_tree", "update", the_repository); + trace2_region_leave("cache_tree", "update", istate->repo); trace_performance_leave("cache_tree_update"); if (i < 0) return i; From 55903dc87bee544c314706c509168afbbe14d262 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Apr 2026 01:57:46 +0200 Subject: [PATCH 02/21] CodingGuidelines: document our style for flags We have recently iterated a bit on our style for flags. Document this. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index b8670751f5c705..4992e52093efdc 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -668,6 +668,18 @@ For C programs: unsigned other_field:1; unsigned field_with_longer_name:1; + - When a function `F` accepts flags, those flags should be defined as `enum + F_flags`. Individual flag definitions should start with `F` and be in + all-uppercase letters. Flag values should be represented via bit shifts. + E.g. + + enum frobnicate_flags { + FROBNICATE_FOO = (1 << 0), + FROBNICATE_BAR = (1 << 1), + }; + + int frobnicate(enum frobnicate_flags flags); + - Array names should be named in the singular form if the individual items are subject of use. E.g.: From 75c702624d9a5f60a78c2d4d5e8de83468c9c5ec Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Apr 2026 01:57:47 +0200 Subject: [PATCH 03/21] treewide: use enum for `odb_for_each_object()` flags We've got a couple of callsites where we pass `odb_for_each_object()` flags, but accept an `unsigned` flags field instead of the corresponding enum. Adapt these to accept the enum type instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.c | 2 +- odb.h | 2 +- packfile.c | 2 +- packfile.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/odb.c b/odb.c index 3f94a53df157e1..9a11c600487ded 100644 --- a/odb.c +++ b/odb.c @@ -922,7 +922,7 @@ int odb_for_each_object(struct object_database *odb, const struct object_info *request, odb_for_each_object_cb cb, void *cb_data, - unsigned flags) + enum odb_for_each_object_flags flags) { struct odb_for_each_object_options opts = { .flags = flags, diff --git a/odb.h b/odb.h index 984bafca9d652b..09affaf6a58e67 100644 --- a/odb.h +++ b/odb.h @@ -522,7 +522,7 @@ int odb_for_each_object(struct object_database *odb, const struct object_info *request, odb_for_each_object_cb cb, void *cb_data, - unsigned flags); + enum odb_for_each_object_flags flags); enum odb_count_objects_flags { /* diff --git a/packfile.c b/packfile.c index ee9c7ea1d142ec..5d3b772973b06c 100644 --- a/packfile.c +++ b/packfile.c @@ -2299,7 +2299,7 @@ int has_object_kept_pack(struct repository *r, const struct object_id *oid, int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn cb, void *data, - unsigned flags) + enum odb_for_each_object_flags flags) { uint32_t i; int r = 0; diff --git a/packfile.h b/packfile.h index 45b35973f07b37..3eb10d6b6567bf 100644 --- a/packfile.h +++ b/packfile.h @@ -352,7 +352,7 @@ typedef int each_packed_object_fn(const struct object_id *oid, void *data); int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn, void *data, - unsigned flags); + enum odb_for_each_object_flags flags); /* * Iterate through all packed objects in the given packfile store and invoke From ff2e9d85d61f2f51793acbdb4bad68d48cc8bb85 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Apr 2026 01:57:48 +0200 Subject: [PATCH 04/21] odb: rename `odb_write_object()` flags Rename `odb_write_object()` flags to be properly prefixed with the function name. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- cache-tree.c | 2 +- object-file.c | 4 ++-- odb.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index 60bcc07c3b8357..60059edfb0b680 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -456,7 +456,7 @@ static int update_one(struct cache_tree *it, hash_object_file(the_hash_algo, buffer.buf, buffer.len, OBJ_TREE, &it->oid); } else if (odb_write_object_ext(the_repository->objects, buffer.buf, buffer.len, OBJ_TREE, - &it->oid, NULL, flags & WRITE_TREE_SILENT ? WRITE_OBJECT_SILENT : 0)) { + &it->oid, NULL, flags & WRITE_TREE_SILENT ? ODB_WRITE_OBJECT_SILENT : 0)) { strbuf_release(&buffer); return -1; } diff --git a/object-file.c b/object-file.c index 4f77ce0982625d..db1a420ab6d4e6 100644 --- a/object-file.c +++ b/object-file.c @@ -909,7 +909,7 @@ static int start_loose_object_common(struct odb_source *source, fd = create_tmpfile(source->odb->repo, tmp_file, filename); if (fd < 0) { - if (flags & WRITE_OBJECT_SILENT) + if (flags & ODB_WRITE_OBJECT_SILENT) return -1; else if (errno == EACCES) return error(_("insufficient permission for adding " @@ -1042,7 +1042,7 @@ static int write_loose_object(struct odb_source *source, utb.actime = mtime; utb.modtime = mtime; if (utime(tmp_file.buf, &utb) < 0 && - !(flags & WRITE_OBJECT_SILENT)) + !(flags & ODB_WRITE_OBJECT_SILENT)) warning_errno(_("failed utime() on %s"), tmp_file.buf); } diff --git a/odb.h b/odb.h index 09affaf6a58e67..083c25609e59e8 100644 --- a/odb.h +++ b/odb.h @@ -568,12 +568,12 @@ enum { * changes that so that the object will be written as a loose object * and persisted. */ - WRITE_OBJECT_PERSIST = (1 << 0), + ODB_WRITE_OBJECT_PERSIST = (1 << 0), /* * Do not print an error in case something goes wrong. */ - WRITE_OBJECT_SILENT = (1 << 1), + ODB_WRITE_OBJECT_SILENT = (1 << 1), }; /* From b2d421ece6a8e095394e76930e6929ee036571ef Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Apr 2026 01:57:49 +0200 Subject: [PATCH 05/21] odb: use enum for `odb_write_object` flags We've got a couple of functions that accept `odb_write_object()` flags, but all of them accept the flags as an `unsigned` integer. In fact, we don't even have an `enum` for the flags field. Introduce this `enum` and adapt functions accordingly according to our coding style. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object-file.c | 3 ++- object-file.h | 3 ++- odb.c | 2 +- odb.h | 4 ++-- odb/source-files.c | 2 +- odb/source.h | 4 ++-- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/object-file.c b/object-file.c index db1a420ab6d4e6..2146104de8cc68 100644 --- a/object-file.c +++ b/object-file.c @@ -1169,7 +1169,8 @@ int odb_source_loose_write_stream(struct odb_source *source, 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) + struct object_id *compat_oid_in, + enum odb_write_object_flags 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 3686f182e4deb0..5241b8dd5c564d 100644 --- a/object-file.h +++ b/object-file.h @@ -68,7 +68,8 @@ int odb_source_loose_freshen_object(struct odb_source *source, 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); + struct object_id *compat_oid_in, + enum odb_write_object_flags flags); int odb_source_loose_write_stream(struct odb_source *source, struct odb_write_stream *stream, size_t len, diff --git a/odb.c b/odb.c index 9a11c600487ded..8220661356b29b 100644 --- a/odb.c +++ b/odb.c @@ -1053,7 +1053,7 @@ int odb_write_object_ext(struct object_database *odb, enum object_type type, struct object_id *oid, struct object_id *compat_oid, - unsigned flags) + enum odb_write_object_flags flags) { return odb_source_write_object(odb->sources, buf, len, type, oid, compat_oid, flags); diff --git a/odb.h b/odb.h index 083c25609e59e8..9aadc1177a9e60 100644 --- a/odb.h +++ b/odb.h @@ -561,7 +561,7 @@ int odb_find_abbrev_len(struct object_database *odb, int min_len, unsigned *out); -enum { +enum odb_write_object_flags { /* * By default, `odb_write_object()` does not actually write anything * into the object store, but only computes the object ID. This flag @@ -589,7 +589,7 @@ int odb_write_object_ext(struct object_database *odb, enum object_type type, struct object_id *oid, struct object_id *compat_oid, - unsigned flags); + enum odb_write_object_flags flags); static inline int odb_write_object(struct object_database *odb, const void *buf, unsigned long len, diff --git a/odb/source-files.c b/odb/source-files.c index 76797569de4280..b5abd20e971e78 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -161,7 +161,7 @@ static int odb_source_files_write_object(struct odb_source *source, enum object_type type, struct object_id *oid, struct object_id *compat_oid, - unsigned flags) + enum odb_write_object_flags flags) { return odb_source_loose_write_object(source, buf, len, type, oid, compat_oid, flags); diff --git a/odb/source.h b/odb/source.h index a9d7d0b96fde72..f706e0608a4855 100644 --- a/odb/source.h +++ b/odb/source.h @@ -197,7 +197,7 @@ struct odb_source { enum object_type type, struct object_id *oid, struct object_id *compat_oid, - unsigned flags); + enum odb_write_object_flags flags); /* * This callback is expected to persist the given object stream into @@ -405,7 +405,7 @@ static inline int odb_source_write_object(struct odb_source *source, enum object_type type, struct object_id *oid, struct object_id *compat_oid, - unsigned flags) + enum odb_write_object_flags flags) { return source->write_object(source, buf, len, type, oid, compat_oid, flags); From c63911b052dc286de5daddba8d4a20fd59348cee Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Apr 2026 01:57:50 +0200 Subject: [PATCH 06/21] odb: rename `odb_has_object()` flags Rename `odb_has_object()` flags to be properly prefixed with the function name. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 2 +- builtin/fetch.c | 4 ++-- builtin/fsck.c | 2 +- builtin/index-pack.c | 2 +- builtin/receive-pack.c | 2 +- builtin/remote.c | 2 +- builtin/show-ref.c | 2 +- builtin/unpack-objects.c | 2 +- cache-tree.c | 8 ++++---- fetch-pack.c | 4 ++-- http-push.c | 8 ++++---- http-walker.c | 4 ++-- list-objects.c | 2 +- notes.c | 2 +- object-file.c | 2 +- odb.c | 6 +++--- odb.h | 8 ++++---- reflog.c | 2 +- refs.c | 2 +- remote.c | 2 +- shallow.c | 6 +++--- walker.c | 2 +- 22 files changed, 38 insertions(+), 38 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index cd13a3a89f1851..d9fbad535868bb 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -161,7 +161,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) case 'e': ret = !odb_has_object(the_repository->objects, &oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR); + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR); goto cleanup; case 'w': diff --git a/builtin/fetch.c b/builtin/fetch.c index 4795b2a13c30e3..a22c3194670e9a 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -946,7 +946,7 @@ static int update_local_ref(struct ref *ref, int fast_forward = 0; if (!odb_has_object(the_repository->objects, &ref->new_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) die(_("object %s not found"), oid_to_hex(&ref->new_oid)); if (oideq(&ref->old_oid, &ref->new_oid)) { @@ -1396,7 +1396,7 @@ static int check_exist_and_connected(struct ref *ref_map) */ for (r = rm; r; r = r->next) { if (!odb_has_object(the_repository->objects, &r->old_oid, - HAS_OBJECT_RECHECK_PACKED)) + ODB_HAS_OBJECT_RECHECK_PACKED)) return -1; } diff --git a/builtin/fsck.c b/builtin/fsck.c index 9bab32effed7ec..4bd0faeff1f4fc 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -163,7 +163,7 @@ static int mark_object(struct object *obj, enum object_type type, if (!(obj->flags & HAS_OBJ)) { if (parent && !odb_has_object(the_repository->objects, &obj->oid, - HAS_OBJECT_RECHECK_PACKED)) { + ODB_HAS_OBJECT_RECHECK_PACKED)) { printf_ln(_("broken link from %7s %s\n" " to %7s %s"), printable_type(&parent->oid, parent->type), diff --git a/builtin/index-pack.c b/builtin/index-pack.c index d1e47279a8c7c9..d96d0eb8cf72a9 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -891,7 +891,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry, if (startup_info->have_repository) { read_lock(); collision_test_needed = odb_has_object(the_repository->objects, oid, - HAS_OBJECT_FETCH_PROMISOR); + ODB_HAS_OBJECT_FETCH_PROMISOR); read_unlock(); } diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index e34edff406959a..32b02238842fb8 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1546,7 +1546,7 @@ static const char *update(struct command *cmd, struct shallow_info *si) if (!is_null_oid(new_oid) && !odb_has_object(the_repository->objects, new_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) { + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) { error("unpack should have generated %s, " "but I can't find it!", oid_to_hex(new_oid)); ret = "bad pack"; diff --git a/builtin/remote.c b/builtin/remote.c index 0fddaa177331f6..de989ea3ba9691 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -473,7 +473,7 @@ static int get_push_ref_states(const struct ref *remote_refs, else if (is_null_oid(&ref->old_oid)) info->status = PUSH_STATUS_CREATE; else if (odb_has_object(the_repository->objects, &ref->old_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR) && + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR) && ref_newer(&ref->new_oid, &ref->old_oid)) info->status = PUSH_STATUS_FASTFORWARD; else diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 5d31acea7c779a..d50844163269df 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -37,7 +37,7 @@ static void show_one(const struct show_one_options *opts, struct object_id peeled; if (!odb_has_object(the_repository->objects, ref->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) die("git show-ref: bad ref %s (%s)", ref->name, oid_to_hex(ref->oid)); diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 6fc64e9e4b8d5a..871fc8fff5028d 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -449,7 +449,7 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size, if (!delta_data) return; if (odb_has_object(the_repository->objects, &base_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) ; /* Ok we have this one */ else if (resolve_against_held(nr, &base_oid, delta_data, delta_size)) diff --git a/cache-tree.c b/cache-tree.c index 60059edfb0b680..fe41068c3493d1 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -239,7 +239,7 @@ int cache_tree_fully_valid(struct cache_tree *it) return 0; if (it->entry_count < 0 || odb_has_object(the_repository->objects, &it->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return 0; for (i = 0; i < it->subtree_nr; i++) { if (!cache_tree_fully_valid(it->down[i]->cache_tree)) @@ -292,7 +292,7 @@ static int update_one(struct cache_tree *it, if (0 <= it->entry_count && odb_has_object(the_repository->objects, &it->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return it->entry_count; /* @@ -400,7 +400,7 @@ static int update_one(struct cache_tree *it, if (is_null_oid(oid) || (!ce_missing_ok && !odb_has_object(the_repository->objects, oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))) { + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))) { strbuf_release(&buffer); if (expected_missing) return -1; @@ -448,7 +448,7 @@ static int update_one(struct cache_tree *it, struct object_id oid; hash_object_file(the_hash_algo, buffer.buf, buffer.len, OBJ_TREE, &oid); - if (odb_has_object(the_repository->objects, &oid, HAS_OBJECT_RECHECK_PACKED)) + if (odb_has_object(the_repository->objects, &oid, ODB_HAS_OBJECT_RECHECK_PACKED)) oidcpy(&it->oid, &oid); else to_invalidate = 1; diff --git a/fetch-pack.c b/fetch-pack.c index 6ecd468ef766a8..0f24722a7072a7 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -145,7 +145,7 @@ static struct commit *deref_without_lazy_fetch(const struct object_id *oid, if (commit) { if (mark_tags_complete_and_check_obj_db) { if (!odb_has_object(the_repository->objects, oid, - HAS_OBJECT_RECHECK_PACKED)) + ODB_HAS_OBJECT_RECHECK_PACKED)) die_in_commit_graph_only(oid); } return commit; @@ -2013,7 +2013,7 @@ static void update_shallow(struct fetch_pack_args *args, struct object_id *oid = si->shallow->oid; for (i = 0; i < si->shallow->nr; i++) if (odb_has_object(the_repository->objects, &oid[i], - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) oid_array_append(&extra, &oid[i]); if (extra.nr) { setup_alternate_shallow(&shallow_lock, diff --git a/http-push.c b/http-push.c index 9ae6062198e14f..06c3acbb5d9a21 100644 --- a/http-push.c +++ b/http-push.c @@ -1449,7 +1449,7 @@ static void one_remote_ref(const char *refname) */ if (repo->can_update_info_refs && !odb_has_object(the_repository->objects, &ref->old_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) { + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) { obj = lookup_unknown_object(the_repository, &ref->old_oid); fprintf(stderr, " fetch %s for %s\n", oid_to_hex(&ref->old_oid), refname); @@ -1655,7 +1655,7 @@ static int delete_remote_branch(const char *pattern, int force) if (is_null_oid(&head_oid)) return error("Unable to resolve remote HEAD"); if (!odb_has_object(the_repository->objects, &head_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return error("Remote HEAD resolves to object %s\nwhich does not exist locally, perhaps you need to fetch?", oid_to_hex(&head_oid)); /* Remote branch must resolve to a known object */ @@ -1663,7 +1663,7 @@ static int delete_remote_branch(const char *pattern, int force) return error("Unable to resolve remote branch %s", remote_ref->name); if (!odb_has_object(the_repository->objects, &remote_ref->old_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return error("Remote branch %s resolves to object %s\nwhich does not exist locally, perhaps you need to fetch?", remote_ref->name, oid_to_hex(&remote_ref->old_oid)); /* Remote branch must be an ancestor of remote HEAD */ @@ -1886,7 +1886,7 @@ int cmd_main(int argc, const char **argv) !is_null_oid(&ref->old_oid) && !ref->force) { if (!odb_has_object(the_repository->objects, &ref->old_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR) || + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR) || !ref_newer(&ref->peer_ref->new_oid, &ref->old_oid)) { /* diff --git a/http-walker.c b/http-walker.c index e886e6486646d1..1b6d496548373e 100644 --- a/http-walker.c +++ b/http-walker.c @@ -139,7 +139,7 @@ static int fill_active_slot(void *data UNUSED) obj_req = list_entry(pos, struct object_request, node); if (obj_req->state == WAITING) { if (odb_has_object(the_repository->objects, &obj_req->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) obj_req->state = COMPLETE; else { start_object_request(obj_req); @@ -495,7 +495,7 @@ static int fetch_object(struct walker *walker, const struct object_id *oid) return error("Couldn't find request for %s in the queue", hex); if (odb_has_object(the_repository->objects, &obj_req->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) { + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) { if (obj_req->req) abort_http_object_request(&obj_req->req); abort_object_request(obj_req); diff --git a/list-objects.c b/list-objects.c index 91b23e22f71aac..724d723c484de4 100644 --- a/list-objects.c +++ b/list-objects.c @@ -75,7 +75,7 @@ static void process_blob(struct traversal_context *ctx, */ if (ctx->revs->exclude_promisor_objects && !odb_has_object(the_repository->objects, &obj->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR) && + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR) && is_promisor_object(ctx->revs->repo, &obj->oid)) return; diff --git a/notes.c b/notes.c index 51a7ef9f830a13..8f315e2a00d265 100644 --- a/notes.c +++ b/notes.c @@ -796,7 +796,7 @@ static int prune_notes_helper(const struct object_id *object_oid, struct note_delete_list *n; if (odb_has_object(the_repository->objects, object_oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return 0; /* nothing to do for this note */ /* failed to find object => prune this note */ diff --git a/object-file.c b/object-file.c index 2146104de8cc68..98a4678ca4cdf2 100644 --- a/object-file.c +++ b/object-file.c @@ -1378,7 +1378,7 @@ static int already_written(struct odb_transaction_files *transaction, { /* The object may already exist in the repository */ if (odb_has_object(transaction->base.source->odb, oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return 1; /* Might want to keep the list sorted */ diff --git a/odb.c b/odb.c index 8220661356b29b..9b28fe25ef2b14 100644 --- a/odb.c +++ b/odb.c @@ -872,15 +872,15 @@ void *odb_read_object_peeled(struct object_database *odb, } int odb_has_object(struct object_database *odb, const struct object_id *oid, - enum has_object_flags flags) + enum odb_has_object_flags flags) { unsigned object_info_flags = 0; if (!startup_info->have_repository) return 0; - if (!(flags & HAS_OBJECT_RECHECK_PACKED)) + if (!(flags & ODB_HAS_OBJECT_RECHECK_PACKED)) object_info_flags |= OBJECT_INFO_QUICK; - if (!(flags & HAS_OBJECT_FETCH_PROMISOR)) + if (!(flags & ODB_HAS_OBJECT_FETCH_PROMISOR)) object_info_flags |= OBJECT_INFO_SKIP_FETCH_OBJECT; return odb_read_object_info_extended(odb, oid, NULL, object_info_flags) >= 0; diff --git a/odb.h b/odb.h index 9aadc1177a9e60..8d739e118b73dd 100644 --- a/odb.h +++ b/odb.h @@ -395,11 +395,11 @@ int odb_read_object_info(struct object_database *odb, const struct object_id *oid, unsigned long *sizep); -enum has_object_flags { +enum odb_has_object_flags { /* Retry packed storage after checking packed and loose storage */ - HAS_OBJECT_RECHECK_PACKED = (1 << 0), + ODB_HAS_OBJECT_RECHECK_PACKED = (1 << 0), /* Allow fetching the object in case the repository has a promisor remote. */ - HAS_OBJECT_FETCH_PROMISOR = (1 << 1), + ODB_HAS_OBJECT_FETCH_PROMISOR = (1 << 1), }; /* @@ -408,7 +408,7 @@ enum has_object_flags { */ int odb_has_object(struct object_database *odb, const struct object_id *oid, - enum has_object_flags flags); + enum odb_has_object_flags flags); int odb_freshen_object(struct object_database *odb, const struct object_id *oid); diff --git a/reflog.c b/reflog.c index 1460ae9d0dd5f7..82337078d00611 100644 --- a/reflog.c +++ b/reflog.c @@ -168,7 +168,7 @@ static int tree_is_complete(const struct object_id *oid) complete = 1; while (tree_entry(&desc, &entry)) { if (!odb_has_object(the_repository->objects, &entry.oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR) || + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR) || (S_ISDIR(entry.mode) && !tree_is_complete(&entry.oid))) { tree->object.flags |= INCOMPLETE; complete = 0; diff --git a/refs.c b/refs.c index 685a0c247b571e..97cdea1f760907 100644 --- a/refs.c +++ b/refs.c @@ -425,7 +425,7 @@ int ref_resolves_to_object(const char *refname, if (flags & REF_ISBROKEN) return 0; if (!odb_has_object(repo->objects, oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) { + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) { error(_("%s does not point to a valid object!"), refname); return 0; } diff --git a/remote.c b/remote.c index 7ca2a6501b4920..a664cd166aa3b9 100644 --- a/remote.c +++ b/remote.c @@ -1723,7 +1723,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, if (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) { if (starts_with(ref->name, "refs/tags/")) reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS; - else if (!odb_has_object(the_repository->objects, &ref->old_oid, HAS_OBJECT_RECHECK_PACKED)) + else if (!odb_has_object(the_repository->objects, &ref->old_oid, ODB_HAS_OBJECT_RECHECK_PACKED)) reject_reason = REF_STATUS_REJECT_FETCH_FIRST; else if (!lookup_commit_reference_gently(the_repository, &ref->old_oid, 1) || !lookup_commit_reference_gently(the_repository, &ref->new_oid, 1)) diff --git a/shallow.c b/shallow.c index 7a3dd567950dea..a8ad92e303d24d 100644 --- a/shallow.c +++ b/shallow.c @@ -360,7 +360,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data) return 0; if (data->flags & QUICK) { if (!odb_has_object(the_repository->objects, &graft->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) return 0; } else if (data->flags & SEEN_ONLY) { struct commit *c = lookup_commit(the_repository, &graft->oid); @@ -528,7 +528,7 @@ void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa) ALLOC_ARRAY(info->theirs, sa->nr); for (size_t i = 0; i < sa->nr; i++) { if (odb_has_object(the_repository->objects, sa->oid + i, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) { + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) { struct commit_graft *graft; graft = lookup_commit_graft(the_repository, &sa->oid[i]); @@ -567,7 +567,7 @@ void remove_nonexistent_theirs_shallow(struct shallow_info *info) if (i != dst) info->theirs[dst] = info->theirs[i]; if (odb_has_object(the_repository->objects, oid + info->theirs[i], - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) dst++; } info->nr_theirs = dst; diff --git a/walker.c b/walker.c index 91332539d3a286..e98eb6da53692e 100644 --- a/walker.c +++ b/walker.c @@ -155,7 +155,7 @@ static int process(struct walker *walker, struct object *obj) obj->flags |= SEEN; if (odb_has_object(the_repository->objects, &obj->oid, - HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) { + ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR)) { /* We already have it, so we should scan it now. */ obj->flags |= TO_SCAN; } From 109bcb7d1d2f0d2f0514beec15779190c0b89575 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Apr 2026 01:57:51 +0200 Subject: [PATCH 07/21] odb: drop unneeded headers and forward decls There's a couple of unneeded forward declarations and headers in "odb.h". Drop these. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.h | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/odb.h b/odb.h index 8d739e118b73dd..3a711f6547bb00 100644 --- a/odb.h +++ b/odb.h @@ -1,19 +1,17 @@ #ifndef ODB_H #define ODB_H -#include "hashmap.h" #include "object.h" #include "oidset.h" #include "oidmap.h" #include "string-list.h" #include "thread-utils.h" -struct oidmap; -struct oidtree; +struct cached_object_entry; +struct packed_git; +struct repository; struct strbuf; struct strvec; -struct repository; -struct multi_pack_index; /* * Set this to 0 to prevent odb_read_object_info_extended() from fetching missing @@ -31,10 +29,6 @@ extern int fetch_if_missing; */ char *compute_alternate_path(const char *path, struct strbuf *err); -struct packed_git; -struct packfile_store; -struct cached_object_entry; - /* * A transaction may be started for an object database prior to writing new * objects via odb_transaction_begin(). These objects are not committed until From 42148dafdf74f8458e7a710dcb982c0be0e40566 Mon Sep 17 00:00:00 2001 From: Siddharth Shrimali Date: Wed, 1 Apr 2026 11:50:29 +0530 Subject: [PATCH 08/21] t7004: replace wc -l with modern test helpers Pipelines of the form "test $(git tag | wc -l) -eq 0" suppress git's exit code. This means a crash or unexpected failure from git tag would go undetected. Additionally, the use of $(...) creates a subshell for each check, which adds unnecessary overhead. Replace these patterns with test_must_be_empty and test_line_count. These helpers check the output of git directly from a file, ensuring git's exit code is captured properly via the preceding "&&" chain. They also provide better diagnostics on failure by printing the contents of the file when a check does not pass. Signed-off-by: Siddharth Shrimali Signed-off-by: Junio C Hamano --- t/t7004-tag.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index ce2ff2a28abaa3..faf7d97fc487d3 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -33,8 +33,10 @@ test_expect_success 'listing all tags in an empty tree should succeed' ' ' test_expect_success 'listing all tags in an empty tree should output nothing' ' - test $(git tag -l | wc -l) -eq 0 && - test $(git tag | wc -l) -eq 0 + git tag -l >actual && + test_must_be_empty actual && + git tag >actual && + test_must_be_empty actual ' test_expect_success 'sort tags, ignore case' ' @@ -178,7 +180,8 @@ test_expect_success 'listing tags using a non-matching pattern should succeed' ' ' test_expect_success 'listing tags using a non-matching pattern should output nothing' ' - test $(git tag -l xxx | wc -l) -eq 0 + git tag -l xxx >actual && + test_must_be_empty actual ' # special cases for creating tags: @@ -188,13 +191,15 @@ test_expect_success 'trying to create a tag with the name of one existing should ' test_expect_success 'trying to create a tag with a non-valid name should fail' ' - test $(git tag -l | wc -l) -eq 1 && + git tag -l >actual && + test_line_count = 1 actual && test_must_fail git tag "" && test_must_fail git tag .othertag && test_must_fail git tag "other tag" && test_must_fail git tag "othertag^" && test_must_fail git tag "other~tag" && - test $(git tag -l | wc -l) -eq 1 + git tag -l >actual && + test_line_count = 1 actual ' test_expect_success 'creating a tag using HEAD directly should succeed' ' From c0ce43376b178d876bce2073b81737ce49657def Mon Sep 17 00:00:00 2001 From: Nick Golden Date: Wed, 1 Apr 2026 15:00:33 -0400 Subject: [PATCH 09/21] read-cache: disable renames in add_files_to_cache add_files_to_cache() refreshes the index from worktree changes and does not need rename detection. When unmerged entries and a deleted stage-0 path are present together, rename detection can pair them and rewrite an unmerged diff pair to point at the deleted path. That later makes "git commit -a" and "git add -u" try to stat the deleted path and die with "unable to stat". Disable rename detection in this callback-driven staging path and add a regression test covering the crash. Signed-off-by: Nick Golden Signed-off-by: Junio C Hamano --- read-cache.c | 1 + t/t2200-add-update.sh | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/read-cache.c b/read-cache.c index 0c07c3aef78ed5..bacb841a2e8f10 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3975,6 +3975,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix, rev.diffopt.format_callback = update_callback; rev.diffopt.format_callback_data = &data; rev.diffopt.flags.override_submodule_config = 1; + rev.diffopt.detect_rename = 0; /* staging worktree changes does not need renames */ rev.max_count = 0; /* do not compare unmerged paths with stage #2 */ /* diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh index 06e83d33333dc8..0a96655cfe7348 100755 --- a/t/t2200-add-update.sh +++ b/t/t2200-add-update.sh @@ -200,6 +200,44 @@ test_expect_success 'add -u resolves unmerged paths' ' test_cmp expect actual ' +test_expect_success 'add -u avoids rename pairing on unmerged paths' ' + test_create_repo rename-crash && + ( + cd rename-crash && + test_seq 1 100 | + sed "s/.*/line &: same text/" >conflict.txt && + cp conflict.txt bystander.txt && + git add conflict.txt bystander.txt && + git commit -m "initial: two files with identical content" && + main_branch=$(git symbolic-ref --short HEAD) && + git checkout -b feature && + sed "s/^line 50:.*/line 50: FEATURE/" \ + conflict.txt >conflict.txt.tmp && + mv conflict.txt.tmp conflict.txt && + git add conflict.txt && + git commit -m "feature: modify line 50" && + git checkout "$main_branch" && + sed "s/^line 50:.*/line 50: MAIN/" \ + conflict.txt >conflict.txt.tmp && + mv conflict.txt.tmp conflict.txt && + git add conflict.txt && + git commit -m "main: modify line 50 differently" && + test_must_fail git merge feature && + rm bystander.txt && + git add -u >out && + test_must_be_empty out && + git ls-files -u >actual && + test_must_be_empty actual && + git ls-files bystander.txt conflict.txt >actual && + cat >expect <<-\EOF && + conflict.txt + EOF + test_cmp expect actual && + git diff-files --name-only >actual && + test_must_be_empty actual + ) +' + test_expect_success '"add -u non-existent" should fail' ' test_must_fail git add -u non-existent && git ls-files >actual && From e5ae639f1a25642650cefedf6478ff5903ffb2f0 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 1 Apr 2026 22:55:10 +0200 Subject: [PATCH 10/21] builtin/replay: mark options as not negatable The options '--onto', '--advance', '--revert', and '--ref-action' of git-replay(1) are not negatable. Mark them as such using PARSE_OPT_NONEG. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- builtin/replay.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/builtin/replay.c b/builtin/replay.c index a0879b020f456a..85aa9fa0a457d5 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -89,20 +89,24 @@ int cmd_replay(int argc, NULL }; struct option replay_options[] = { - OPT_STRING(0, "advance", &opts.advance, - N_("branch"), - N_("make replay advance given branch")), - OPT_STRING(0, "onto", &opts.onto, - N_("revision"), - N_("replay onto given commit")), OPT_BOOL(0, "contained", &opts.contained, N_("update all branches that point at commits in ")), - OPT_STRING(0, "revert", &opts.revert, - N_("branch"), - N_("revert commits onto given branch")), - OPT_STRING(0, "ref-action", &ref_action, - N_("mode"), - N_("control ref update behavior (update|print)")), + OPT_STRING_F(0, "onto", &opts.onto, + N_("revision"), + N_("replay onto given commit"), + PARSE_OPT_NONEG), + OPT_STRING_F(0, "advance", &opts.advance, + N_("branch"), + N_("make replay advance given branch"), + PARSE_OPT_NONEG), + OPT_STRING_F(0, "revert", &opts.revert, + N_("branch"), + N_("revert commits onto given branch"), + PARSE_OPT_NONEG), + OPT_STRING_F(0, "ref-action", &ref_action, + N_("mode"), + N_("control ref update behavior (update|print)"), + PARSE_OPT_NONEG), OPT_END() }; From 6542cacbb33490ab83ef87a5fbee694cd2863bdd Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 1 Apr 2026 22:55:11 +0200 Subject: [PATCH 11/21] replay: use stuck form in documentation and help message gitcli(7) suggests to use stuck form. Change the documentation strings to use this form. While at it, reorder them to match the order in the docs. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- Documentation/git-replay.adoc | 25 +++++++++++++------------ builtin/replay.c | 4 ++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc index 997097e4205523..5bb478c281df85 100644 --- a/Documentation/git-replay.adoc +++ b/Documentation/git-replay.adoc @@ -9,7 +9,8 @@ git-replay - EXPERIMENTAL: Replay commits on a new base, works with bare repos t SYNOPSIS -------- [verse] -(EXPERIMENTAL!) 'git replay' ([--contained] --onto | --advance | --revert ) [--ref-action[=]] +(EXPERIMENTAL!) 'git replay' ([--contained] --onto= | --advance= | --revert=) + [--ref-action=] DESCRIPTION ----------- @@ -26,7 +27,7 @@ THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE. OPTIONS ------- ---onto :: +--onto=:: Starting point at which to create the new commits. May be any valid commit, and not just an existing branch name. + @@ -34,7 +35,7 @@ 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 :: +--advance=:: Starting point at which to create the new commits; must be a branch name. + @@ -42,7 +43,7 @@ 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. ---revert :: +--revert=:: Starting point at which to create the reverted commits; must be a branch name. + @@ -79,8 +80,8 @@ The default mode can be configured via the `replay.refAction` configuration vari :: Range of commits to replay; see "Specifying Ranges" in - linkgit:git-rev-parse[1]. In `--advance ` or - `--revert ` mode, the range should have a single tip, + linkgit:git-rev-parse[1]. In `--advance=` or + `--revert=` mode, the range should have a single tip, so that it's clear to which tip the advanced or reverted should point. Any commits in the range whose changes are already present in the branch the commits are being @@ -127,7 +128,7 @@ EXAMPLES To simply rebase `mybranch` onto `target`: ------------ -$ git replay --onto target origin/main..mybranch +$ git replay --onto=target origin/main..mybranch ------------ The refs are updated atomically and no output is produced on success. @@ -135,14 +136,14 @@ 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 +$ git replay --ref-action=print --onto=target origin/main..mybranch update refs/heads/mybranch ${NEW_mybranch_HASH} ${OLD_mybranch_HASH} ------------ To cherry-pick the commits from mybranch onto target: ------------ -$ git replay --advance target origin/main..mybranch +$ git replay --advance=target origin/main..mybranch ------------ Note that the first two examples replay the exact same commits and on @@ -154,7 +155,7 @@ 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 +$ git replay --contained --onto=origin/main origin/main..tipbranch ------------ All three branches (`branch1`, `branch2`, and `tipbranch`) are updated @@ -165,7 +166,7 @@ commits to replay using the syntax `A..B`; any range expression will do: ------------ -$ git replay --onto origin/main ^base branch1 branch2 branch3 +$ git replay --onto=origin/main ^base branch1 branch2 branch3 ------------ This will simultaneously rebase `branch1`, `branch2`, and `branch3`, @@ -176,7 +177,7 @@ that they have in common, but that does not need to be the case. To revert commits on a branch: ------------ -$ git replay --revert main topic~2..topic +$ git replay --revert=main topic~2..topic ------------ This reverts the last two commits from `topic`, creating revert commits on diff --git a/builtin/replay.c b/builtin/replay.c index 85aa9fa0a457d5..fbfeb780b6a6ad 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -84,8 +84,8 @@ int cmd_replay(int argc, const char *const replay_usage[] = { N_("(EXPERIMENTAL!) git replay " - "([--contained] --onto | --advance | --revert ) " - "[--ref-action[=]] "), + "([--contained] --onto= | --advance= | --revert=)\n" + "[--ref-action=] "), NULL }; struct option replay_options[] = { From 23d83f8ddbef9adcb87671358b473e55cf90c90b Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Wed, 1 Apr 2026 22:55:12 +0200 Subject: [PATCH 12/21] replay: allow to specify a ref with option --ref When option '--onto' is passed to git-replay(1), the command will update refs from the passed to the command. When using option '--advance' or '--revert', the argument of that option is a ref that will be updated. To enable users to specify which ref to update, add option '--ref'. When using option '--ref', the refs described above are left untouched and instead the argument of this option is updated instead. Because this introduces code paths in replay.c that jump to `out` before init_basic_merge_options() is called on `merge_opt`, zero-initialize the struct. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- Documentation/git-replay.adoc | 22 +++++++++++- builtin/replay.c | 8 ++++- replay.c | 35 ++++++++++++++----- replay.h | 7 ++++ t/t3650-replay-basics.sh | 66 +++++++++++++++++++++++++++++++++++ 5 files changed, 128 insertions(+), 10 deletions(-) diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc index 5bb478c281df85..a32f72aead3750 100644 --- a/Documentation/git-replay.adoc +++ b/Documentation/git-replay.adoc @@ -10,7 +10,7 @@ SYNOPSIS -------- [verse] (EXPERIMENTAL!) 'git replay' ([--contained] --onto= | --advance= | --revert=) - [--ref-action=] + [--ref=] [--ref-action=] DESCRIPTION ----------- @@ -66,6 +66,16 @@ incompatible with `--contained` (which is a modifier for `--onto` only). Update all branches that point at commits in . Requires `--onto`. +--ref=:: + Override which reference is updated with the result of the replay. + The ref must be fully qualified. + When used with `--onto`, the `` should have a + single tip and only the specified reference is updated instead of + inferring refs from the revision range. + When used with `--advance` or `--revert`, the specified reference is + updated instead of the branch given to those options. + This option is incompatible with `--contained`. + --ref-action[=]:: Control how references are updated. The mode can be: + @@ -189,6 +199,16 @@ NOTE: For reverting an entire merge request as a single commit (rather than commit-by-commit), consider using `git merge-tree --merge-base $TIP HEAD $BASE` which can avoid unnecessary merge conflicts. +To replay onto a specific commit while updating a different reference: + +------------ +$ git replay --onto=112233 --ref=refs/heads/mybranch aabbcc..ddeeff +------------ + +This replays the range `aabbcc..ddeeff` onto commit `112233` and updates +`refs/heads/mybranch` to point at the result. This can be useful when you want +to use bare commit IDs instead of branch names. + GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/replay.c b/builtin/replay.c index fbfeb780b6a6ad..39e3a86f6c10ab 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -85,7 +85,7 @@ int cmd_replay(int argc, const char *const replay_usage[] = { N_("(EXPERIMENTAL!) git replay " "([--contained] --onto= | --advance= | --revert=)\n" - "[--ref-action=] "), + "[--ref=] [--ref-action=] "), NULL }; struct option replay_options[] = { @@ -103,6 +103,10 @@ int cmd_replay(int argc, N_("branch"), N_("revert commits onto given branch"), PARSE_OPT_NONEG), + OPT_STRING_F(0, "ref", &opts.ref, + N_("branch"), + N_("reference to update with result"), + PARSE_OPT_NONEG), OPT_STRING_F(0, "ref-action", &ref_action, N_("mode"), N_("control ref update behavior (update|print)"), @@ -126,6 +130,8 @@ int cmd_replay(int argc, opts.contained, "--contained"); die_for_incompatible_opt2(!!opts.revert, "--revert", opts.contained, "--contained"); + die_for_incompatible_opt2(!!opts.ref, "--ref", + !!opts.contained, "--contained"); /* Parse ref action mode from command line or config */ ref_mode = get_ref_action_mode(repo, ref_action); diff --git a/replay.c b/replay.c index d7239d4c8396d4..b958ddabfa1363 100644 --- a/replay.c +++ b/replay.c @@ -347,13 +347,15 @@ int replay_revisions(struct rev_info *revs, struct commit *last_commit = NULL; struct commit *commit; struct commit *onto = NULL; - struct merge_options merge_opt; + struct merge_options merge_opt = { 0 }; struct merge_result result = { .clean = 1, }; bool detached_head; char *advance; char *revert; + const char *ref; + struct object_id old_oid; enum replay_mode mode = REPLAY_MODE_PICK; int ret; @@ -364,6 +366,27 @@ int replay_revisions(struct rev_info *revs, set_up_replay_mode(revs->repo, &revs->cmdline, opts->onto, &detached_head, &advance, &revert, &onto, &update_refs); + if (opts->ref) { + struct object_id oid; + + if (update_refs && strset_get_size(update_refs) > 1) { + ret = error(_("'--ref' cannot be used with multiple revision ranges")); + goto out; + } + if (check_refname_format(opts->ref, 0) || !starts_with(opts->ref, "refs/")) { + ret = error(_("'%s' is not a valid refname"), opts->ref); + goto out; + } + ref = opts->ref; + if (!refs_read_ref(get_main_ref_store(revs->repo), opts->ref, &oid)) + oidcpy(&old_oid, &oid); + else + oidclr(&old_oid, revs->repo->hash_algo); + } else { + ref = advance ? advance : revert; + oidcpy(&old_oid, &onto->object.oid); + } + /* FIXME: Should allow replaying commits with the first as a root commit */ if (prepare_revision_walk(revs) < 0) { @@ -399,7 +422,7 @@ int replay_revisions(struct rev_info *revs, kh_value(replayed_commits, pos) = last_commit; /* Update any necessary branches */ - if (advance || revert) + if (ref) continue; for (decoration = get_name_decoration(&commit->object); @@ -433,13 +456,9 @@ int replay_revisions(struct rev_info *revs, goto out; } - /* In --advance or --revert mode, update the target ref */ - if (advance || revert) { - const char *ref = advance ? advance : revert; - replay_result_queue_update(out, ref, - &onto->object.oid, + if (ref) + replay_result_queue_update(out, ref, &old_oid, &last_commit->object.oid); - } ret = 0; diff --git a/replay.h b/replay.h index e916a5f975be26..0ab74b9805af16 100644 --- a/replay.h +++ b/replay.h @@ -24,6 +24,13 @@ struct replay_revisions_options { */ const char *onto; + /* + * Reference to update with the result of the replay. This will not + * update any refs from `onto`, `advance`, or `revert`. Ignores + * `contained`. + */ + const char *ref; + /* * Starting point at which to create revert commits; must be a branch * name. The branch will be updated to point to the revert commits. diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index 217f6fb292a068..d5c7dd1bf4aad8 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -495,4 +495,70 @@ test_expect_success 'git replay --revert incompatible with --advance' ' test_grep "cannot be used together" error ' +test_expect_success 'using --onto with --ref' ' + git branch test-ref-onto topic2 && + test_when_finished "git branch -D test-ref-onto" && + + git replay --ref-action=print --onto=main --ref=refs/heads/test-ref-onto topic1..topic2 >result && + + test_line_count = 1 result && + test_grep "^update refs/heads/test-ref-onto " result && + + git log --format=%s $(cut -f 3 -d " " result) >actual && + test_write_lines E D M L B A >expect && + test_cmp expect actual +' + +test_expect_success 'using --advance with --ref' ' + git branch test-ref-advance main && + git branch test-ref-target main && + test_when_finished "git branch -D test-ref-advance test-ref-target" && + + git replay --ref-action=print --advance=test-ref-advance --ref=refs/heads/test-ref-target topic1..topic2 >result && + + test_line_count = 1 result && + test_grep "^update refs/heads/test-ref-target " result +' + +test_expect_success 'using --revert with --ref' ' + git branch test-ref-revert topic4 && + git branch test-ref-revert-target topic4 && + test_when_finished "git branch -D test-ref-revert test-ref-revert-target" && + + git replay --ref-action=print --revert=test-ref-revert --ref=refs/heads/test-ref-revert-target topic4~1..topic4 >result && + + test_line_count = 1 result && + test_grep "^update refs/heads/test-ref-revert-target " result +' + +test_expect_success '--ref is incompatible with --contained' ' + test_must_fail git replay --onto=main --ref=refs/heads/main --contained topic1..topic2 2>err && + test_grep "cannot be used together" err +' + +test_expect_success '--ref with nonexistent fully-qualified ref' ' + test_when_finished "git update-ref -d refs/heads/new-branch" && + + git replay --onto=main --ref=refs/heads/new-branch topic1..topic2 && + + git log --format=%s -2 new-branch >actual && + test_write_lines E D >expect && + test_cmp expect actual +' + +test_expect_success '--ref must be a valid refname' ' + test_must_fail git replay --onto=main --ref="refs/heads/bad..ref" topic1..topic2 2>err && + test_grep "is not a valid refname" err +' + +test_expect_success '--ref requires fully qualified ref' ' + test_must_fail git replay --onto=main --ref=main topic1..topic2 2>err && + test_grep "is not a valid refname" err +' + +test_expect_success '--onto with --ref rejects multiple revision ranges' ' + test_must_fail git replay --onto=main --ref=refs/heads/topic2 ^topic1 topic2 topic4 2>err && + test_grep "cannot be used with multiple revision ranges" err +' + test_done From 34c17b840d5bdb8060ef6309aee04f919616c9de Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Apr 2026 09:31:14 +0200 Subject: [PATCH 13/21] reftable: introduce "reftable-system.h" header We're including a couple of standard headers like in a bunch of locations, which makes it hard for a project to plug in their own logic for making required functionality available. For us this is for example via "compat/posix.h", which already includes all of the system headers relevant to us. Introduce a new "reftable-system.h" header that allows projects to provide their own headers. This new header is supposed to contain all the project-specific bits to provide the POSIX-like environment, and some additional supporting code. With this change, we thus have the following split in our system-specific code: - "reftable/reftable-system.h" is the project-specific header that provides a POSIX-like environment. Every project is expected to provide their own implementation. - "reftable/system.h" contains the project-independent definition of the interfaces that a project needs to implement. This file should not be touched by a project. - "reftable/system.c" contains the project-specific implementation of the interfaces defined in "system.h". Again, every project is expected to provide their own implementation. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/reftable-basics.h | 2 +- reftable/reftable-block.h | 3 +-- reftable/reftable-blocksource.h | 2 +- reftable/reftable-error.h | 2 ++ reftable/reftable-fsck.h | 1 + reftable/reftable-iterator.h | 1 + reftable/reftable-merged.h | 1 + reftable/reftable-record.h | 2 +- reftable/reftable-stack.h | 1 + reftable/reftable-system.h | 15 +++++++++++++++ reftable/reftable-table.h | 1 + reftable/reftable-writer.h | 4 +--- reftable/system.h | 11 +++++++---- 13 files changed, 34 insertions(+), 12 deletions(-) create mode 100644 reftable/reftable-system.h diff --git a/reftable/reftable-basics.h b/reftable/reftable-basics.h index 6d73f19c85b6d3..dc8622682d7d06 100644 --- a/reftable/reftable-basics.h +++ b/reftable/reftable-basics.h @@ -9,7 +9,7 @@ #ifndef REFTABLE_BASICS_H #define REFTABLE_BASICS_H -#include +#include "reftable-system.h" /* A buffer that contains arbitrary byte slices. */ struct reftable_buf { diff --git a/reftable/reftable-block.h b/reftable/reftable-block.h index 0b05a8f7e376bc..94c79b5c58fb76 100644 --- a/reftable/reftable-block.h +++ b/reftable/reftable-block.h @@ -9,8 +9,7 @@ #ifndef REFTABLE_BLOCK_H #define REFTABLE_BLOCK_H -#include - +#include "reftable-system.h" #include "reftable-basics.h" #include "reftable-blocksource.h" #include "reftable-iterator.h" diff --git a/reftable/reftable-blocksource.h b/reftable/reftable-blocksource.h index f5ba867bd60a10..40c1e946462921 100644 --- a/reftable/reftable-blocksource.h +++ b/reftable/reftable-blocksource.h @@ -9,7 +9,7 @@ #ifndef REFTABLE_BLOCKSOURCE_H #define REFTABLE_BLOCKSOURCE_H -#include +#include "reftable-system.h" /* * Generic wrapper for a seekable readable file. diff --git a/reftable/reftable-error.h b/reftable/reftable-error.h index d100e0df927ca2..0535e1478bda2d 100644 --- a/reftable/reftable-error.h +++ b/reftable/reftable-error.h @@ -9,6 +9,8 @@ #ifndef REFTABLE_ERROR_H #define REFTABLE_ERROR_H +#include "reftable-system.h" + /* * Errors in reftable calls are signaled with negative integer return values. 0 * means success. diff --git a/reftable/reftable-fsck.h b/reftable/reftable-fsck.h index 007a392cf906c7..340fc7762ed76f 100644 --- a/reftable/reftable-fsck.h +++ b/reftable/reftable-fsck.h @@ -1,6 +1,7 @@ #ifndef REFTABLE_FSCK_H #define REFTABLE_FSCK_H +#include "reftable-system.h" #include "reftable-stack.h" enum reftable_fsck_error { diff --git a/reftable/reftable-iterator.h b/reftable/reftable-iterator.h index af582028c27fdc..a050cc153be711 100644 --- a/reftable/reftable-iterator.h +++ b/reftable/reftable-iterator.h @@ -9,6 +9,7 @@ #ifndef REFTABLE_ITERATOR_H #define REFTABLE_ITERATOR_H +#include "reftable-system.h" #include "reftable-record.h" struct reftable_iterator_vtable; diff --git a/reftable/reftable-merged.h b/reftable/reftable-merged.h index e5af846b32a95f..02a9966835eb15 100644 --- a/reftable/reftable-merged.h +++ b/reftable/reftable-merged.h @@ -9,6 +9,7 @@ #ifndef REFTABLE_MERGED_H #define REFTABLE_MERGED_H +#include "reftable-system.h" #include "reftable-iterator.h" /* diff --git a/reftable/reftable-record.h b/reftable/reftable-record.h index 385a74cc864985..e18c5382381c4b 100644 --- a/reftable/reftable-record.h +++ b/reftable/reftable-record.h @@ -9,8 +9,8 @@ #ifndef REFTABLE_RECORD_H #define REFTABLE_RECORD_H +#include "reftable-system.h" #include "reftable-basics.h" -#include /* * Basic data types diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h index c2415cbc6e46a6..5f7be573fabf8e 100644 --- a/reftable/reftable-stack.h +++ b/reftable/reftable-stack.h @@ -9,6 +9,7 @@ #ifndef REFTABLE_STACK_H #define REFTABLE_STACK_H +#include "reftable-system.h" #include "reftable-writer.h" /* diff --git a/reftable/reftable-system.h b/reftable/reftable-system.h new file mode 100644 index 00000000000000..4a18a6a790d737 --- /dev/null +++ b/reftable/reftable-system.h @@ -0,0 +1,15 @@ +#ifndef REFTABLE_SYSTEM_H +#define REFTABLE_SYSTEM_H + +/* + * This header defines the platform-specific bits required to compile the + * reftable library. It should provide an environment that bridges over the + * gaps between POSIX and your system, as well as the zlib interfaces. This + * header is expected to be changed by the individual project. + */ + +#define MINGW_DONT_HANDLE_IN_USE_ERROR +#include "compat/posix.h" +#include "compat/zlib-compat.h" + +#endif diff --git a/reftable/reftable-table.h b/reftable/reftable-table.h index 5f935d02e3b195..d7666b53a1ee0c 100644 --- a/reftable/reftable-table.h +++ b/reftable/reftable-table.h @@ -9,6 +9,7 @@ #ifndef REFTABLE_TABLE_H #define REFTABLE_TABLE_H +#include "reftable-system.h" #include "reftable-iterator.h" #include "reftable-block.h" #include "reftable-blocksource.h" diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h index 1e7003cd698879..065dd93dc6bc61 100644 --- a/reftable/reftable-writer.h +++ b/reftable/reftable-writer.h @@ -9,11 +9,9 @@ #ifndef REFTABLE_WRITER_H #define REFTABLE_WRITER_H +#include "reftable-system.h" #include "reftable-record.h" -#include -#include /* ssize_t */ - /* Writing single reftables */ /* reftable_write_options sets options for writing a single reftable. */ diff --git a/reftable/system.h b/reftable/system.h index c54ed4cad61f73..a7eb6acd4a4b67 100644 --- a/reftable/system.h +++ b/reftable/system.h @@ -9,11 +9,14 @@ #ifndef SYSTEM_H #define SYSTEM_H -/* This header glues the reftable library to the rest of Git */ +/* + * This header defines the platform-agnostic interface that is to be + * implemented by the project to make it work on their respective supported + * systems, and to integrate it into the project itself. This header is not + * expected to be changed by the individual project. + */ -#define MINGW_DONT_HANDLE_IN_USE_ERROR -#include "compat/posix.h" -#include "compat/zlib-compat.h" +#include "reftable-system.h" /* * Return a random 32 bit integer. This function is expected to return From b45ea595e6f6b03a749abc2c8e508504429a4cf3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Apr 2026 09:31:15 +0200 Subject: [PATCH 14/21] reftable/stack: provide fsync(3p) via system header Users of the reftable library are expected to provide their own function callback in cases they want to sync(3p) data to disk via the reftable write options. But if no such function was provided we end up calling fsync(3p) directly, which may not even be available on some systems. While dropping the explicit call to fsync(3p) would work, it would lead to an unsafe default behaviour where a project may have forgotten to set up the callback function, and that could lead to potential data loss. So this is not a great solution. Instead, drop the callback function and make it mandatory for the project to define fsync(3p). In the case of Git, we can then easily inject our custom implementation via the "reftable-system.h" header so that we continue to use `fsync_component()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs/reftable-backend.c | 6 ------ reftable/reftable-system.h | 3 +++ reftable/reftable-writer.h | 6 ------ reftable/stack.c | 13 +++---------- reftable/system.c | 6 ++++++ 5 files changed, 12 insertions(+), 22 deletions(-) diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index b124404663edf6..daea30a5b4cad9 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -366,11 +366,6 @@ static int reftable_be_config(const char *var, const char *value, return 0; } -static int reftable_be_fsync(int fd) -{ - return fsync_component(FSYNC_COMPONENT_REFERENCE, fd); -} - static struct ref_store *reftable_be_init(struct repository *repo, const char *payload, const char *gitdir, @@ -408,7 +403,6 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->write_options.disable_auto_compact = !git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1); refs->write_options.lock_timeout_ms = 100; - refs->write_options.fsync = reftable_be_fsync; repo_config(the_repository, reftable_be_config, &refs->write_options); diff --git a/reftable/reftable-system.h b/reftable/reftable-system.h index 4a18a6a790d737..76f3e33e901397 100644 --- a/reftable/reftable-system.h +++ b/reftable/reftable-system.h @@ -12,4 +12,7 @@ #include "compat/posix.h" #include "compat/zlib-compat.h" +int reftable_fsync(int fd); +#define fsync(fd) reftable_fsync(fd) + #endif diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h index 065dd93dc6bc61..a66db415c87637 100644 --- a/reftable/reftable-writer.h +++ b/reftable/reftable-writer.h @@ -61,12 +61,6 @@ struct reftable_write_options { */ long lock_timeout_ms; - /* - * Optional callback used to fsync files to disk. Falls back to using - * fsync(3P) when unset. - */ - int (*fsync)(int fd); - /* * Callback function to execute whenever the stack is being reloaded. * This can be used e.g. to discard cached information that relies on diff --git a/reftable/stack.c b/reftable/stack.c index 1c9f21dfe1eb45..fa87b46c377af2 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -29,13 +29,6 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st, return 0; } -static int stack_fsync(const struct reftable_write_options *opts, int fd) -{ - if (opts->fsync) - return opts->fsync(fd); - return fsync(fd); -} - static ssize_t reftable_write_data(int fd, const void *data, size_t size) { size_t total_written = 0; @@ -69,7 +62,7 @@ static ssize_t fd_writer_write(void *arg, const void *data, size_t sz) static int fd_writer_flush(void *arg) { struct fd_writer *writer = arg; - return stack_fsync(writer->opts, writer->fd); + return fsync(writer->fd); } static int fd_read_lines(int fd, char ***namesp) @@ -812,7 +805,7 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err = stack_fsync(&add->stack->opts, add->tables_list_lock.fd); + err = fsync(add->tables_list_lock.fd); if (err < 0) { err = REFTABLE_IO_ERROR; goto done; @@ -1480,7 +1473,7 @@ static int stack_compact_range(struct reftable_stack *st, goto done; } - err = stack_fsync(&st->opts, tables_list_lock.fd); + err = fsync(tables_list_lock.fd); if (err < 0) { err = REFTABLE_IO_ERROR; unlink(new_table_path.buf); diff --git a/reftable/system.c b/reftable/system.c index 725a25844ea179..4d7e366b55b6d6 100644 --- a/reftable/system.c +++ b/reftable/system.c @@ -5,6 +5,7 @@ #include "reftable-error.h" #include "../lockfile.h" #include "../tempfile.h" +#include "../write-or-die.h" uint32_t reftable_rand(void) { @@ -131,3 +132,8 @@ int flock_commit(struct reftable_flock *l) return 0; } + +int reftable_fsync(int fd) +{ + return fsync_component(FSYNC_COMPONENT_REFERENCE, fd); +} From aa8938573050e6ab44a46e3b9f26c0e442f835aa Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Apr 2026 09:31:16 +0200 Subject: [PATCH 15/21] reftable/fsck: use REFTABLE_UNUSED instead of UNUSED While we have the reftable-specific `REFTABLE_UNUSED` header, we accidentally introduced a new usage of the Git-specific `UNUSED` header into the reftable library in 9051638519 (reftable: add code to facilitate consistency checks, 2025-10-07). Convert the site to use `REFTABLE_UNUSED`. Ideally, we'd move the definition of `UNUSED` into "git-compat-util.h" so that it becomes in accessible to the reftable library. But this is unfortunately not easily possible as "compat/mingw-posix.h" requires this macro, and this header is included by "compat/posix.h". Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/fsck.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reftable/fsck.c b/reftable/fsck.c index 26b9115b14acd8..8e73fc83f225e5 100644 --- a/reftable/fsck.c +++ b/reftable/fsck.c @@ -63,7 +63,7 @@ static int table_check_name(struct reftable_table *table, static int table_checks(struct reftable_table *table, reftable_fsck_report_fn report_fn, - reftable_fsck_verbose_fn verbose_fn UNUSED, + reftable_fsck_verbose_fn verbose_fn REFTABLE_UNUSED, void *cb_data) { table_check_fn table_check_fns[] = { From cb0882de1979522b2fc3dc4c3064b0ad21d50b06 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Apr 2026 09:31:17 +0200 Subject: [PATCH 16/21] reftable/system: add abstraction to retrieve time in milliseconds We directly call gettimeofday(3p), which may not be available on some platforms. Provide the infrastructure to let projects easily use their own implementations of this function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/stack.c | 27 ++++----------------------- reftable/system.c | 6 ++++++ reftable/system.h | 3 +++ 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index fa87b46c377af2..1fba96ddb3366f 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -365,45 +365,26 @@ static int reftable_stack_reload_once(struct reftable_stack *st, return err; } -/* return negative if a before b. */ -static int tv_cmp(struct timeval *a, struct timeval *b) -{ - time_t diff = a->tv_sec - b->tv_sec; - int udiff = a->tv_usec - b->tv_usec; - - if (diff != 0) - return diff; - - return udiff; -} - static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st, int reuse_open) { char **names = NULL, **names_after = NULL; - struct timeval deadline; + uint64_t deadline; int64_t delay = 0; int tries = 0, err; int fd = -1; - err = gettimeofday(&deadline, NULL); - if (err < 0) - goto out; - deadline.tv_sec += 3; + deadline = reftable_time_ms() + 3000; while (1) { - struct timeval now; - - err = gettimeofday(&now, NULL); - if (err < 0) - goto out; + uint64_t now = reftable_time_ms(); /* * Only look at deadlines after the first few times. This * simplifies debugging in GDB. */ tries++; - if (tries > 3 && tv_cmp(&now, &deadline) >= 0) + if (tries > 3 && now >= deadline) goto out; fd = open(st->list_file, O_RDONLY); diff --git a/reftable/system.c b/reftable/system.c index 4d7e366b55b6d6..cd76e56be8f16f 100644 --- a/reftable/system.c +++ b/reftable/system.c @@ -4,6 +4,7 @@ #include "basics.h" #include "reftable-error.h" #include "../lockfile.h" +#include "../trace.h" #include "../tempfile.h" #include "../write-or-die.h" @@ -137,3 +138,8 @@ int reftable_fsync(int fd) { return fsync_component(FSYNC_COMPONENT_REFERENCE, fd); } + +uint64_t reftable_time_ms(void) +{ + return getnanotime() / 1000000; +} diff --git a/reftable/system.h b/reftable/system.h index a7eb6acd4a4b67..071bfa3d58962c 100644 --- a/reftable/system.h +++ b/reftable/system.h @@ -111,4 +111,7 @@ int flock_release(struct reftable_flock *l); */ int flock_commit(struct reftable_flock *l); +/* Report the time in milliseconds. */ +uint64_t reftable_time_ms(void); + #endif From 87e4eee3f94ec261a92a76d06261b227b00de461 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 2 Apr 2026 09:31:18 +0200 Subject: [PATCH 17/21] reftable/system: add abstraction to mmap files In our codebase we have a couple of wrappers around mmap(3p) that allow us to reimplement the syscall on platforms that don't have it natively, like for example Windows. Other projects that embed the reftable library may have a different infra though to hook up mmap wrappers, but these are currently hard to integrate. Provide the infrastructure to let projects easily define the mmap interface with a custom struct and custom functions. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/blocksource.c | 19 +++++++------------ reftable/system.c | 20 ++++++++++++++++++++ reftable/system.h | 18 ++++++++++++++++++ 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/reftable/blocksource.c b/reftable/blocksource.c index 573c81287fe5d8..7f7441f751342a 100644 --- a/reftable/blocksource.c +++ b/reftable/blocksource.c @@ -93,13 +93,12 @@ void block_source_from_buf(struct reftable_block_source *bs, } struct file_block_source { - uint64_t size; - unsigned char *data; + struct reftable_mmap mmap; }; static uint64_t file_size(void *b) { - return ((struct file_block_source *)b)->size; + return ((struct file_block_source *)b)->mmap.size; } static void file_release_data(void *b REFTABLE_UNUSED, struct reftable_block_data *dest REFTABLE_UNUSED) @@ -109,7 +108,7 @@ static void file_release_data(void *b REFTABLE_UNUSED, struct reftable_block_dat static void file_close(void *v) { struct file_block_source *b = v; - munmap(b->data, b->size); + reftable_munmap(&b->mmap); reftable_free(b); } @@ -117,8 +116,8 @@ static ssize_t file_read_data(void *v, struct reftable_block_data *dest, uint64_ uint32_t size) { struct file_block_source *b = v; - assert(off + size <= b->size); - dest->data = b->data + off; + assert(off + size <= b->mmap.size); + dest->data = (unsigned char *) b->mmap.data + off; dest->len = size; return size; } @@ -156,13 +155,9 @@ int reftable_block_source_from_file(struct reftable_block_source *bs, goto out; } - p->size = st.st_size; - p->data = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); - if (p->data == MAP_FAILED) { - err = REFTABLE_IO_ERROR; - p->data = NULL; + err = reftable_mmap(&p->mmap, fd, st.st_size); + if (err < 0) goto out; - } assert(!bs->ops); bs->ops = &file_vtable; diff --git a/reftable/system.c b/reftable/system.c index cd76e56be8f16f..9063641f304c1b 100644 --- a/reftable/system.c +++ b/reftable/system.c @@ -143,3 +143,23 @@ uint64_t reftable_time_ms(void) { return getnanotime() / 1000000; } + +int reftable_mmap(struct reftable_mmap *out, int fd, size_t len) +{ + void *data = xmmap_gently(NULL, len, PROT_READ, MAP_PRIVATE, fd, 0); + if (data == MAP_FAILED) + return REFTABLE_IO_ERROR; + + out->data = data; + out->size = len; + + return 0; +} + +int reftable_munmap(struct reftable_mmap *mmap) +{ + if (munmap(mmap->data, mmap->size) < 0) + return REFTABLE_IO_ERROR; + memset(mmap, 0, sizeof(*mmap)); + return 0; +} diff --git a/reftable/system.h b/reftable/system.h index 071bfa3d58962c..c0e2cbe0ffb90c 100644 --- a/reftable/system.h +++ b/reftable/system.h @@ -114,4 +114,22 @@ int flock_commit(struct reftable_flock *l); /* Report the time in milliseconds. */ uint64_t reftable_time_ms(void); +struct reftable_mmap { + void *data; + size_t size; + void *priv; +}; + +/* + * Map the file into memory. Returns 0 on success, a reftable error code on + * error. + */ +int reftable_mmap(struct reftable_mmap *out, int fd, size_t len); + +/* + * Unmap the file from memory. Returns 0 on success, a reftable error code on + * error. + */ +int reftable_munmap(struct reftable_mmap *mmap); + #endif From 7d8727ff0b621a9729c2de6a3698063b7b3ba2d6 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 7 Apr 2026 15:17:30 -0500 Subject: [PATCH 18/21] object-file: avoid ODB transaction when not writing objects In ce1661f9da (odb: add transaction interface, 2025-09-16), existing ODB transaction logic is adapted to create a transaction interface at the ODB layer. The intent here is for the ODB transaction interface to eventually provide an object source agnostic means to manage transactions. An unintended consequence of this change though is that `object-file.c:index_fd()` may enter the ODB transaction path even when no object write is requested. In non-repository contexts, this can result in a NULL dereference and segfault. One such case occurs when running git-diff(1) outside of a repository with "core.bigFileThreshold" forcing the streaming path in `index_fd()`: $ echo foo >foo $ echo bar >bar $ git -c core.bigFileThreshold=1 diff -- foo bar In this scenario, the caller only needs to compute the object ID. Object hashing does not require an ODB, so starting a transaction is both unnecessary and invalid. Fix the bug by avoiding the use of ODB transactions in `index_fd()` when callers are only interested in computing the object hash. Reported-by: Luca Stefani Signed-off-by: Justin Tobler [jc: adjusted to fd13909e (Merge branch 'jt/odb-transaction', 2025-10-02)] Signed-off-by: Junio C Hamano --- object-file.c | 49 ++++++++++++++++++++++++++++++++++------- t/t1517-outside-repo.sh | 8 +++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/object-file.c b/object-file.c index 4675c8ed6b67eb..5d72e65bdece01 100644 --- a/object-file.c +++ b/object-file.c @@ -1599,6 +1599,34 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction, return 0; } +static int hash_blob_stream(const struct git_hash_algo *hash_algo, + struct object_id *result_oid, int fd, size_t size) +{ + unsigned char buf[16384]; + struct git_hash_ctx ctx; + unsigned header_len; + + header_len = format_object_header((char *)buf, sizeof(buf), + OBJ_BLOB, size); + hash_algo->init_fn(&ctx); + git_hash_update(&ctx, buf, header_len); + + while (size) { + size_t rsize = size < sizeof(buf) ? size : sizeof(buf); + ssize_t read_result = read_in_full(fd, buf, rsize); + + if ((read_result < 0) || ((size_t)read_result != rsize)) + return -1; + + git_hash_update(&ctx, buf, rsize); + size -= read_result; + } + + git_hash_final_oid(result_oid, &ctx); + + return 0; +} + int index_fd(struct index_state *istate, struct object_id *oid, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags) @@ -1620,14 +1648,19 @@ int index_fd(struct index_state *istate, struct object_id *oid, ret = index_core(istate, oid, fd, xsize_t(st->st_size), type, path, flags); } else { - struct odb_transaction *transaction; - - transaction = odb_transaction_begin(the_repository->objects); - ret = index_blob_packfile_transaction(the_repository->objects->transaction, - oid, fd, - xsize_t(st->st_size), - path, flags); - odb_transaction_commit(transaction); + if (flags & INDEX_WRITE_OBJECT) { + struct odb_transaction *transaction; + + transaction = odb_transaction_begin(the_repository->objects); + ret = index_blob_packfile_transaction(the_repository->objects->transaction, + oid, fd, + xsize_t(st->st_size), + path, flags); + odb_transaction_commit(transaction); + } else { + ret = hash_blob_stream(the_repository->hash_algo, oid, + fd, xsize_t(st->st_size)); + } } close(fd); diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh index c824c1a25cf27e..e1d35170de61a3 100755 --- a/t/t1517-outside-repo.sh +++ b/t/t1517-outside-repo.sh @@ -93,6 +93,14 @@ test_expect_success 'diff outside repository' ' test_cmp expect actual ' +test_expect_success 'hash object exceeding bigFileThreshold outside repository' ' + ( + cd non-repo && + echo foo >foo && + git -c core.bigFileThreshold=1 hash-object --stdin Date: Wed, 8 Apr 2026 19:11:48 +0300 Subject: [PATCH 19/21] t1800: add &&-chains to test helper functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the missing &&'s so we properly propagate failures between commands in the hook helper functions. Also add a missing mkdir -p arg (found by adding the &&). Reported-by: SZEDER Gábor Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- t/t1800-hook.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 96749fc06d7223..33decc66c0ea8d 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -6,16 +6,16 @@ test_description='git-hook command and config-managed multihooks' . "$TEST_DIRECTORY"/lib-terminal.sh setup_hooks () { - test_config hook.ghi.command "/path/ghi" - test_config hook.ghi.event pre-commit --add - test_config hook.ghi.event test-hook --add - test_config_global hook.def.command "/path/def" + test_config hook.ghi.command "/path/ghi" && + test_config hook.ghi.event pre-commit --add && + test_config hook.ghi.event test-hook --add && + test_config_global hook.def.command "/path/def" && test_config_global hook.def.event pre-commit --add } setup_hookdir () { - mkdir .git/hooks - write_script .git/hooks/pre-commit <<-EOF + mkdir -p .git/hooks && + write_script .git/hooks/pre-commit <<-EOF && echo \"Legacy Hook\" EOF test_when_finished rm -rf .git/hooks From 2226ffaacd93d3fe5554687a70d9190d72596f96 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 8 Apr 2026 13:20:55 -0400 Subject: [PATCH 20/21] run_processes_parallel(): fix order of sigpipe handling In commit ec0becacc9 (run-command: add stdin callback for parallelization, 2026-01-28), we taught run_processes_parallel() to ignore SIGPIPE, since we wouldn't want a write() to a broken pipe of one of the children to take down the whole process. But there's a subtle ordering issue. After we ignore SIGPIPE, we call pp_init(), which installs its own cleanup handler for multiple signals using sigchain_push_common(), which includes SIGPIPE. So if we receive SIGPIPE while writing to a child, we'll trigger that handler first, pop it off the stack, and then re-raise (which is then ignored because of the SIG_IGN we pushed first). But what does that handler do? It tries to clean up all of the child processes, under the assumption that when we re-raise the signal we'll be exiting the process! So a hook that exits without reading all of its input will cause us to get SIGPIPE, which will put us in a signal handler that then tries to kill() that same child. This seems to be mostly harmless on Linux. The process has already exited by this point, and though kill() does not complain (since the process has not been reaped with a wait() call), it does not affect the exit status of the process. However, this seems not to be true on all platforms. This case is triggered by t5401.13, "pre-receive hook that forgets to read its input". This test fails on NonStop since that hook was converted to the run_processes_parallel() API. We can fix it by reordering the code a bit. We should run pp_init() first, and then push our SIG_IGN onto the stack afterwards, so that it is truly ignored while feeding the sub-processes. Note that we also reorder the popping at the end of the function, too. This is not technically necessary, as we are doing two pops either way, but now the pops will correctly match their pushes. This also fixes a related case that we can't test yet. If we did have more than one process to run, then one child causing SIGPIPE would cause us to kill() all of the children (which might still actually be running). But the hook API is the only user of the new feed_pipe feature, and it does not yet support parallel hook execution. So for now we'll always execute the processes sequentially. Once parallel hook execution exists, we'll be able to add a test which covers this. Reported-by: Randall S. Becker Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/run-command.c b/run-command.c index 32c290ee6a221f..574d5c40f0a1a5 100644 --- a/run-command.c +++ b/run-command.c @@ -1895,14 +1895,19 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) "max:%"PRIuMAX, (uintmax_t)opts->processes); + pp_init(&pp, opts, &pp_sig); + /* * Child tasks might receive input via stdin, terminating early (or not), so * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which * actually writes the data to children stdin fds. + * + * This _must_ come after pp_init(), because it installs its own + * SIGPIPE handler (to cleanup children), and we want to supersede + * that. */ sigchain_push(SIGPIPE, SIG_IGN); - pp_init(&pp, opts, &pp_sig); while (1) { for (i = 0; i < spawn_cap && !pp.shutdown && @@ -1928,10 +1933,10 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) } } - pp_cleanup(&pp, opts); - sigchain_pop(SIGPIPE); + pp_cleanup(&pp, opts); + if (do_trace2) trace2_region_leave(tr2_category, tr2_label, NULL); } From b15384c06f77bc2d34d0d3623a8a58218313a561 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 8 Apr 2026 11:00:10 -0700 Subject: [PATCH 21/21] A bit more post -rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.54.0.adoc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/RelNotes/2.54.0.adoc b/Documentation/RelNotes/2.54.0.adoc index 27dbfdc6a59e13..30ec959e7e5bf2 100644 --- a/Documentation/RelNotes/2.54.0.adoc +++ b/Documentation/RelNotes/2.54.0.adoc @@ -509,6 +509,12 @@ Fixes since v2.53 working tree, which was broken. (merge 339eba65a7 th/backfill-auto-detect-sparseness-fix later to maint). + * add_files_to_cache() used diff_files() to detect only the paths that + are different between the index and the working tree and add them, + which does not need rename detection, which interfered with unnecessary + conflicts. + (merge c0ce43376b ng/add-files-to-cache-wo-rename later to maint). + * Other code cleanup, docfix, build fix, etc. (merge d79fff4a11 jk/remote-tracking-ref-leakfix later to maint). (merge 7a747f972d dd/t5403-modernise later to maint).