* [PATCH v8 3/3] contrib: wire up osxkeychain in contrib/Makefile on macOS
From: Shardul Natu via GitGitGadget @ 2026-07-08 3:21 UTC (permalink / raw)
To: git
Cc: Kristoffer Haugsbakk, Shardul Natu, Koji Nakamaru,
Patrick Steinhardt, Shardul Natu, Ben Knoble, Shardul Natu
In-Reply-To: <pull.2288.v8.git.git.1783480879.gitgitgadget@gmail.com>
From: Shardul Natu <snatu@google.com>
When running "make test" with TEST_CONTRIB_TOO=yes (which is default in
macOS CI workflows), $(MAKE) -C contrib/ test is invoked. However,
contrib/Makefile only invoked tests for diff-highlight and subtree,
meaning git-credential-osxkeychain was never built or verified during
standard CI test runs.
Add a "test" target to contrib/credential/osxkeychain/Makefile that
depends on building git-credential-osxkeychain. Additionally, wire up
credential/osxkeychain in contrib/Makefile under "all", "test", and
"clean" whenever running on macOS (Darwin).
This ensures that running "make test" or "make all" in contrib on macOS
automatically builds and links git-credential-osxkeychain, preventing
future build or symbol linking regressions from slipping through CI.
Signed-off-by: Shardul Natu <snatu@google.com>
---
contrib/Makefile | 12 ++++++++++++
contrib/credential/osxkeychain/Makefile | 4 +++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/contrib/Makefile b/contrib/Makefile
index 787cd07f52..1203c7263d 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -1,10 +1,22 @@
+include ../config.mak.uname
+-include ../config.mak.autogen
+-include ../config.mak
+
+
+ifeq ($(uname_S),Darwin)
+OS_CONTRIB += credential/osxkeychain
+endif
+
all::
+ $(foreach dir,$(OS_CONTRIB),$(MAKE) -C $(dir) $@;)
test::
$(MAKE) -C diff-highlight $@
$(MAKE) -C subtree $@
+ $(foreach dir,$(OS_CONTRIB),$(MAKE) -C $(dir) $@;)
clean::
$(MAKE) -C contacts $@
$(MAKE) -C diff-highlight $@
$(MAKE) -C subtree $@
+ $(foreach dir,$(OS_CONTRIB),$(MAKE) -C $(dir) $@;)
diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
index 219b0d7f49..d9fba07e8d 100644
--- a/contrib/credential/osxkeychain/Makefile
+++ b/contrib/credential/osxkeychain/Makefile
@@ -10,4 +10,6 @@ install:
clean:
$(MAKE) -C ../../.. clean-git-credential-osxkeychain
-.PHONY: all git-credential-osxkeychain install clean
+test: git-credential-osxkeychain
+
+.PHONY: all git-credential-osxkeychain install clean test
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] SubmittingPatches: abandoning a series
From: Michael Montalbo @ 2026-07-08 3:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano <gitster@pobox.com> writes:
> + A discussion on the list might convince you that your changes are
> + not such a good idea, in which case you are expected to explicitly
> + retract the topic, to releave the maintainer from having to worry
> + about it.
s/releave/relieve/
Also, I might reflow the end:
...in which case you are expected to explicitly retract the topic and
relieve the maintainer from having to worry about it.
> ...It is a friendly thing to do to tell the list in such a case...
Again a slight reflow suggestion (ofc my own subjective taste):
"...As a courtesy, tell the list in such a case so that..."
Finally, maybe a rule of thumb as to how old a topic a topic should get
before a "staleness" update is provided would be helpful, or maybe that
is too contextually dependent / would potentially introduce some
unwanted scheduling contract?
Overall, I think the explicit guidance is helpful. Of course, an author
whose topic is going stale may not be on top of updating its status,
stale or otherwise, by definition. However, I think providing this advice
up-front will encourage authors to be more proactive about taking action
(notify of abandonment / handoff) before a topic gets too old.
^ permalink raw reply
* [PATCH] submodule--helper: accept '-i' shorthand for update --init
From: Dominique Martinet @ 2026-07-08 3:33 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, Junio C Hamano, Glen Choo,
Atharva Raykar
Cc: Roy Eldar, git, Dominique Martinet
commit 3ad0ba722744 ("git-submodule.sh: improve variables readability")
made `git submodules update -i` pass `-i` as is to submodule--helper,
but it fails with `error: unknown switch `i'` because the helper does
not accept the short option.
All other short options supported by git-submodule.sh are properly
handle in the helper, so also add the alias for --init
Fixes: 3ad0ba722744 ("git-submodule.sh: improve variables readability")
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
This apparently can't be used much because it's been broken since v2.49,
and it's not in the help, but I guess we might as well fix this since it
used to work?
Thanks!
---
builtin/submodule--helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 1cc82a134db2..3ec8bf50532e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2990,7 +2990,7 @@ static int module_update(int argc, const char **argv, const char *prefix,
struct option module_update_options[] = {
OPT__SUPER_PREFIX(&opt.super_prefix),
OPT__FORCE(&opt.force, N_("force checkout updates"), 0),
- OPT_BOOL(0, "init", &opt.init,
+ OPT_BOOL('i', "init", &opt.init,
N_("initialize uninitialized submodules before update")),
OPT_BOOL(0, "remote", &opt.remote,
N_("use SHA-1 of submodule's remote tracking branch")),
---
base-commit: 11320dd2066f2139870e36987240abc981cb493c
^ permalink raw reply related
* [PATCH v2 0/7] git_hash_*() quality-of-life improvements
From: Jeff King @ 2026-07-08 3:52 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260707045556.GA1288172@coredump.intra.peff.net>
On Tue, Jul 07, 2026 at 12:55:57AM -0400, Jeff King wrote:
> This implements the "idempotent git_hash_discard()" discussed in this
> subthread:
>
> https://lore.kernel.org/git/20260702080707.GG2029434@coredump.intra.peff.net/
>
> with associated cleanups.
Here's a v2 addressing the comments so far. Mostly minor changes:
- fixed typos noticed by Patrick
- dropped extra braces added by coccinelle
- dropped a trailing blank line from patch 1 (this gets fixed in a
later patch as we add more content after the blank line, but I
noticed "git apply" complaining)
- a bit more explanation in patch 7 about why we don't support
idempotent final() calls
Patch list below, followed by range diff.
[1/7]: hash: use git_hash_init() consistently
[2/7]: hash: convert remaining direct function calls
[3/7]: hash: document function pointers and wrappers
[4/7]: hash: make git_hash_discard() idempotent
[5/7]: csum-file: use idempotent git_hash_discard()
[6/7]: http: use idempotent git_hash_discard()
[7/7]: hash: check ctx->active flag in all wrapper functions
builtin/fast-import.c | 4 +--
builtin/index-pack.c | 6 ++--
builtin/patch-id.c | 2 +-
builtin/receive-pack.c | 6 ++--
builtin/submodule--helper.c | 10 +++---
builtin/unpack-objects.c | 4 +--
csum-file.c | 23 +++++---------
diff.c | 4 +--
hash.c | 16 ++++++++++
hash.h | 44 +++++++++++++++++++-------
http-push.c | 2 +-
http.c | 9 ++----
http.h | 1 -
object-file.c | 14 ++++-----
pack-check.c | 2 +-
pack-write.c | 6 ++--
read-cache.c | 6 ++--
rerere.c | 2 +-
t/helper/test-hash-speed.c | 2 +-
t/helper/test-hash.c | 2 +-
t/helper/test-synthesize.c | 33 ++++++++++---------
t/unit-tests/u-hash.c | 2 +-
tools/coccinelle/hash.cocci | 63 +++++++++++++++++++++++++++++++++++++
trace2/tr2_sid.c | 2 +-
24 files changed, 177 insertions(+), 88 deletions(-)
create mode 100644 tools/coccinelle/hash.cocci
1: 2f1c8cbc98 ! 1: 911cf0dfcd hash: use git_hash_init() consistently
@@ object-file.c: static int start_loose_object_common(struct odb_source_loose *loo
stream->next_out = buf;
stream->avail_out = buflen;
- algo->init_fn(c);
-- if (compat && compat_c)
-- compat->init_fn(compat_c);
+ git_hash_init(c, algo);
-+ if (compat && compat_c) {
+ if (compat && compat_c)
+- compat->init_fn(compat_c);
+ git_hash_init(compat_c, compat);
-+ }
/* Start to feed header to zlib stream */
stream->next_in = (unsigned char *)hdr;
@@ read-cache.c: static size_t read_eoie_extension(const char *mmap, size_t mmap_si
## rerere.c ##
@@ rerere.c: static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz
- struct git_hash_ctx ctx;
struct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;
int has_conflicts = 0;
-- if (hash)
+ if (hash)
- the_hash_algo->init_fn(&ctx);
-+ if (hash) {
+ git_hash_init(&ctx, the_hash_algo);
-+ }
while (!io->getline(&buf, io)) {
if (is_cmarker(buf.buf, '<', marker_size)) {
@@ tools/coccinelle/hash.cocci (new)
+- ALGO->init_fn(CTX);
++ git_hash_init(CTX, ALGO);
+ ...>}
-+
## trace2/tr2_sid.c ##
@@ trace2/tr2_sid.c: static void tr2_sid_append_my_sid_component(void)
2: cf88edda3f ! 2: 879962bf47 hash: convert remaining direct function calls
@@ t/helper/test-synthesize.c: static int generate_pack_with_large_object(const cha
## tools/coccinelle/hash.cocci ##
@@ tools/coccinelle/hash.cocci: struct git_hash_ctx *CTX;
+ - ALGO->init_fn(CTX);
+ git_hash_init(CTX, ALGO);
...>}
-
++
+@@
+identifier f != git_hash_clone;
+expression ALGO;
3: 3c302bbe74 ! 3: f06387a467 hash: document function pointers and wrappers
@@ hash.h: struct git_hash_algo {
+/*
+ * Prepare an uninitialized hash context for use. You must eventually release
-+ * the context with with git_hash_final() (or final_oid()) or by calling
++ * the context with git_hash_final() (or final_oid()) or by calling
+ * git_hash_discard().
+ */
void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop);
4: e8b50b164a = 4: 2d876c17b1 hash: make git_hash_discard() idempotent
5: 5488debdae = 5: 3d3d5d2c63 csum-file: use idempotent git_hash_discard()
6: 15fd04f519 = 6: 91bda10e58 http: use idempotent git_hash_discard()
7: 5370cd31a8 ! 7: 2b366bc79b hash: check ctx->active flag in all wrapper functions
@@ Commit message
context onto the heap). Checking the flag lets us catch problems
consistently on every build.
- Note that we can't do the same for git_init_hash(). Even though it would
+ Note that we can't do the same for git_hash_init(). Even though it would
cause a leak to call it twice (without an intervening final/discard),
the point of the function is that the contents of the struct are
undefined before the call. But calling it twice is an even less likely
error to make, so not covering it is OK.
+ We leave git_hash_discard() alone, as its idempotent behavior is
+ convenient for callers. We _could_ try to do something similar for
+ git_hash_final(), allowing:
+
+ git_hash_final(result, &ctx);
+ git_hash_final(other_result, &ctx);
+
+ but it does not make much sense. After the first final() call we have
+ thrown away the state, so we cannot produce the same output. We could
+ come up with some sensible output (the null hash, or the empty hash),
+ but double-calls like this are more likely a bug, so our best bet is to
+ complain loudly (whereas the current code produces either nonsense
+ output or undefined behavior, depending on the backend).
+
Signed-off-by: Jeff King <peff@peff.net>
## hash.c ##
^ permalink raw reply
* [PATCH v2 1/7] hash: use git_hash_init() consistently
From: Jeff King @ 2026-07-08 3:52 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260708035235.GA41491@coredump.intra.peff.net>
We'd like to add more logic to git_hash_init(), but many callers skip it
and call algop->init_fn() directly. Let's make sure we're consistently
using the wrapper by adding a coccinelle rule.
Besides the coccinelle file itself, this is a purely mechanical
conversion based on the patch it generates. There should be no bare
init_fn() calls left (except for the one in the wrapper).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/fast-import.c | 4 ++--
builtin/index-pack.c | 6 +++---
builtin/patch-id.c | 2 +-
builtin/receive-pack.c | 6 +++---
builtin/submodule--helper.c | 2 +-
builtin/unpack-objects.c | 4 ++--
csum-file.c | 6 +++---
diff.c | 4 ++--
http-push.c | 2 +-
http.c | 4 ++--
object-file.c | 14 +++++++-------
pack-check.c | 2 +-
pack-write.c | 6 +++---
read-cache.c | 6 +++---
rerere.c | 2 +-
t/helper/test-hash-speed.c | 2 +-
t/helper/test-hash.c | 2 +-
t/helper/test-synthesize.c | 4 ++--
t/unit-tests/u-hash.c | 2 +-
tools/coccinelle/hash.cocci | 9 +++++++++
trace2/tr2_sid.c | 2 +-
21 files changed, 50 insertions(+), 41 deletions(-)
create mode 100644 tools/coccinelle/hash.cocci
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index f6473dcc8e..6692f7cd81 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -969,7 +969,7 @@ static int store_object(
hdrlen = format_object_header((char *)hdr, sizeof(hdr), type,
dat->len);
- the_hash_algo->init_fn(&c);
+ git_hash_init(&c, the_hash_algo);
git_hash_update(&c, hdr, hdrlen);
git_hash_update(&c, dat->buf, dat->len);
git_hash_final_oid(&oid, &c);
@@ -1131,7 +1131,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
hdrlen = format_object_header((char *)out_buf, out_sz, OBJ_BLOB, len);
- the_hash_algo->init_fn(&c);
+ git_hash_init(&c, the_hash_algo);
git_hash_update(&c, out_buf, hdrlen);
crc32_begin(pack_file);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index f396658468..53a8cb9dd7 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -374,7 +374,7 @@ static const char *open_pack_file(const char *pack_name)
output_fd = -1;
nothread_data.pack_fd = input_fd;
}
- the_hash_algo->init_fn(&input_ctx);
+ git_hash_init(&input_ctx, the_hash_algo);
return pack_name;
}
@@ -481,7 +481,7 @@ static void *unpack_entry_data(off_t offset, size_t size,
if (!is_delta_type(type)) {
hdrlen = format_object_header(hdr, sizeof(hdr), type, size);
- the_hash_algo->init_fn(&c);
+ git_hash_init(&c, the_hash_algo);
git_hash_update(&c, hdr, hdrlen);
} else
oid = NULL;
@@ -1291,7 +1291,7 @@ static void parse_pack_objects(unsigned char *hash)
/* Check pack integrity */
flush();
- the_hash_algo->init_fn(&tmp_ctx);
+ git_hash_init(&tmp_ctx, the_hash_algo);
git_hash_clone(&tmp_ctx, &input_ctx);
git_hash_final(hash, &tmp_ctx);
if (!hasheq(fill(the_hash_algo->rawsz), hash, the_repository->hash_algo))
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 57d9bd4a65..22f36ecf80 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -73,7 +73,7 @@ static size_t get_one_patchid(struct object_id *next_oid, struct object_id *resu
char pre_oid_str[GIT_MAX_HEXSZ + 1], post_oid_str[GIT_MAX_HEXSZ + 1];
struct git_hash_ctx ctx;
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
oidclr(result, the_repository->hash_algo);
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 19eb6a1b61..faf0f120ac 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -615,7 +615,7 @@ static void hmac_hash(unsigned char *out,
/* RFC 2104 2. (1) */
memset(key, '\0', GIT_MAX_BLKSZ);
if (the_hash_algo->blksz < key_len) {
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
git_hash_update(&ctx, key_in, key_len);
git_hash_final(key, &ctx);
} else {
@@ -629,13 +629,13 @@ static void hmac_hash(unsigned char *out,
}
/* RFC 2104 2. (3) & (4) */
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
git_hash_update(&ctx, k_ipad, sizeof(k_ipad));
git_hash_update(&ctx, text, text_len);
git_hash_final(out, &ctx);
/* RFC 2104 2. (6) & (7) */
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
git_hash_update(&ctx, k_opad, sizeof(k_opad));
git_hash_update(&ctx, out, the_hash_algo->rawsz);
git_hash_final(out, &ctx);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 1cc82a134d..bf114a7856 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -550,7 +550,7 @@ static void create_default_gitdir_config(const char *submodule_name)
/* Case 2.4: If all the above failed, try a hash of the name as a last resort */
header_len = snprintf(header, sizeof(header), "blob %zu", strlen(submodule_name));
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
the_hash_algo->update_fn(&ctx, header, header_len);
the_hash_algo->update_fn(&ctx, "\0", 1);
the_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name));
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index f3849bb654..93a9caa582 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -670,10 +670,10 @@ int cmd_unpack_objects(int argc,
/* We don't take any non-flag arguments now.. Maybe some day */
usage(unpack_usage);
}
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
unpack_all();
git_hash_update(&ctx, buffer, offset);
- the_hash_algo->init_fn(&tmp_ctx);
+ git_hash_init(&tmp_ctx, the_hash_algo);
git_hash_clone(&tmp_ctx, &ctx);
git_hash_final_oid(&oid, &tmp_ctx);
if (strict) {
diff --git a/csum-file.c b/csum-file.c
index b166f89624..7e81391524 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -175,7 +175,7 @@ struct hashfile *hashfd_ext(const struct git_hash_algo *algop,
f->skip_hash = 0;
f->algop = unsafe_hash_algo(algop);
- f->algop->init_fn(&f->ctx);
+ git_hash_init(&f->ctx, f->algop);
f->buffer_len = opts->buffer_len ? opts->buffer_len : DEFAULT_IO_BUFFER_SIZE;
f->buffer = xmalloc(f->buffer_len);
@@ -200,7 +200,7 @@ void hashfile_checkpoint_init(struct hashfile *f,
struct hashfile_checkpoint *checkpoint)
{
memset(checkpoint, 0, sizeof(*checkpoint));
- f->algop->init_fn(&checkpoint->ctx);
+ git_hash_init(&checkpoint->ctx, f->algop);
}
void hashfile_checkpoint(struct hashfile *f, struct hashfile_checkpoint *checkpoint)
@@ -252,7 +252,7 @@ int hashfile_checksum_valid(const struct git_hash_algo *algop,
if (total_len < algop->rawsz)
return 0; /* say "too short"? */
- algop->init_fn(&ctx);
+ git_hash_init(&ctx, algop);
git_hash_update(&ctx, data, data_len);
git_hash_final(got, &ctx);
diff --git a/diff.c b/diff.c
index 1568f0ed9c..589c1969e4 100644
--- a/diff.c
+++ b/diff.c
@@ -6855,7 +6855,7 @@ void flush_one_hunk(struct object_id *result, struct git_hash_ctx *ctx)
int i;
git_hash_final(hash, ctx);
- the_hash_algo->init_fn(ctx);
+ git_hash_init(ctx, the_hash_algo);
/* 20-byte sum, with carry */
for (i = 0; i < the_hash_algo->rawsz; ++i) {
carry += result->hash[i] + hash[i];
@@ -6899,7 +6899,7 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
struct git_hash_ctx ctx;
struct patch_id_t data;
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
memset(&data, 0, sizeof(struct patch_id_t));
data.ctx = &ctx;
oidclr(oid, the_repository->hash_algo);
diff --git a/http-push.c b/http-push.c
index 3c23cbba27..60f6f8f054 100644
--- a/http-push.c
+++ b/http-push.c
@@ -776,7 +776,7 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)
} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {
lock->token = xstrdup(ctx->cdata);
- the_hash_algo->init_fn(&hash_ctx);
+ git_hash_init(&hash_ctx, the_hash_algo);
git_hash_update(&hash_ctx, lock->token, strlen(lock->token));
git_hash_final(lock_token_hash, &hash_ctx);
diff --git a/http.c b/http.c
index 63abbaae8a..0341de5031 100644
--- a/http.c
+++ b/http.c
@@ -2879,7 +2879,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
git_inflate_init(&freq->stream);
- the_hash_algo->init_fn(&freq->c);
+ git_hash_init(&freq->c, the_hash_algo);
freq->hash_ctx_valid = 1;
freq->url = get_remote_object_url(base_url, hex, 0);
@@ -2916,7 +2916,7 @@ struct http_object_request *new_http_object_request(const char *base_url,
git_inflate_end(&freq->stream);
memset(&freq->stream, 0, sizeof(freq->stream));
git_inflate_init(&freq->stream);
- the_hash_algo->init_fn(&freq->c);
+ git_hash_init(&freq->c, the_hash_algo);
if (prev_posn>0) {
prev_posn = 0;
lseek(freq->localfile, 0, SEEK_SET);
diff --git a/object-file.c b/object-file.c
index e3c68cfb66..93602f8c50 100644
--- a/object-file.c
+++ b/object-file.c
@@ -124,7 +124,7 @@ int stream_object_signature(struct repository *r,
hdrlen = format_object_header(hdr, sizeof(hdr), st->type, st->size);
/* Sha1.. */
- r->hash_algo->init_fn(&c);
+ git_hash_init(&c, r->hash_algo);
git_hash_update(&c, hdr, hdrlen);
for (;;) {
char buf[1024 * 16];
@@ -320,7 +320,7 @@ static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_c
struct object_id *oid,
char *hdr, size_t *hdrlen)
{
- algo->init_fn(c);
+ git_hash_init(c, algo);
git_hash_update(c, hdr, *hdrlen);
git_hash_update(c, buf, len);
git_hash_final_oid(oid, c);
@@ -681,9 +681,9 @@ static int start_loose_object_common(struct odb_source_loose *loose,
git_deflate_init(stream, cfg->zlib_compression_level);
stream->next_out = buf;
stream->avail_out = buflen;
- algo->init_fn(c);
+ git_hash_init(c, algo);
if (compat && compat_c)
- compat->init_fn(compat_c);
+ git_hash_init(compat_c, compat);
/* Start to feed header to zlib stream */
stream->next_in = (unsigned char *)hdr;
@@ -1141,7 +1141,7 @@ static int hash_blob_stream(struct odb_write_stream *stream,
header_len = format_object_header((char *)buf, sizeof(buf),
OBJ_BLOB, size);
- hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, hash_algo);
git_hash_update(&ctx, buf, header_len);
while (!stream->is_finished) {
@@ -1313,7 +1313,7 @@ static int odb_transaction_files_write_object_stream(struct odb_transaction *bas
header_len = format_object_header((char *)obuf, sizeof(obuf),
OBJ_BLOB, size);
- transaction->base.source->odb->repo->hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, transaction->base.source->odb->repo->hash_algo);
git_hash_update(&ctx, obuf, header_len);
/*
@@ -1560,7 +1560,7 @@ static int check_stream_oid(git_zstream *stream,
unsigned long total_read;
int status = Z_OK;
- algop->init_fn(&c);
+ git_hash_init(&c, algop);
git_hash_update(&c, hdr, stream->total_out);
/*
diff --git a/pack-check.c b/pack-check.c
index 5adfb3f272..c3b8db7c5c 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -69,7 +69,7 @@ static int verify_packfile(struct repository *r,
if (!is_pack_valid(p))
return error("packfile %s cannot be accessed", p->pack_name);
- r->hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, r->hash_algo);
do {
unsigned long remaining;
unsigned char *in = use_pack(p, w_curs, offset, &remaining);
diff --git a/pack-write.c b/pack-write.c
index 83eaf88541..24033a9101 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -402,8 +402,8 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,
char *buf;
ssize_t read_result;
- hash_algo->init_fn(&old_hash_ctx);
- hash_algo->init_fn(&new_hash_ctx);
+ git_hash_init(&old_hash_ctx, hash_algo);
+ git_hash_init(&new_hash_ctx, hash_algo);
if (lseek(pack_fd, 0, SEEK_SET) != 0)
die_errno("Failed seeking to start of '%s'", pack_name);
@@ -455,7 +455,7 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,
* pack, which also means making partial_pack_offset
* big enough not to matter anymore.
*/
- hash_algo->init_fn(&old_hash_ctx);
+ git_hash_init(&old_hash_ctx, hash_algo);
partial_pack_offset = ~partial_pack_offset;
partial_pack_offset -= MSB(partial_pack_offset, 1);
}
diff --git a/read-cache.c b/read-cache.c
index 7c1cdcf696..5fa747e6fc 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1722,7 +1722,7 @@ static int verify_hdr(const struct cache_header *hdr, unsigned long size)
if (oideq(&oid, null_oid(the_hash_algo)))
return 0;
- the_hash_algo->init_fn(&c);
+ git_hash_init(&c, the_hash_algo);
git_hash_update(&c, hdr, size - the_hash_algo->rawsz);
git_hash_final(hash, &c);
if (!hasheq(hash, start, the_repository->hash_algo))
@@ -2957,7 +2957,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
*/
if (offset && record_eoie()) {
CALLOC_ARRAY(eoie_c, 1);
- the_hash_algo->init_fn(eoie_c);
+ git_hash_init(eoie_c, the_hash_algo);
}
/*
@@ -3598,7 +3598,7 @@ static size_t read_eoie_extension(const char *mmap, size_t mmap_size)
* "REUC" + <binary representation of M>)
*/
src_offset = offset;
- the_hash_algo->init_fn(&c);
+ git_hash_init(&c, the_hash_algo);
while (src_offset < mmap_size - the_hash_algo->rawsz - EOIE_SIZE_WITH_HEADER) {
/* After an array of active_nr index entries,
* there can be arbitrary number of extended
diff --git a/rerere.c b/rerere.c
index 8232542585..216100925a 100644
--- a/rerere.c
+++ b/rerere.c
@@ -439,7 +439,7 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz
struct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;
int has_conflicts = 0;
if (hash)
- the_hash_algo->init_fn(&ctx);
+ git_hash_init(&ctx, the_hash_algo);
while (!io->getline(&buf, io)) {
if (is_cmarker(buf.buf, '<', marker_size)) {
diff --git a/t/helper/test-hash-speed.c b/t/helper/test-hash-speed.c
index fbf67fe6bd..89b0268011 100644
--- a/t/helper/test-hash-speed.c
+++ b/t/helper/test-hash-speed.c
@@ -5,7 +5,7 @@
static inline void compute_hash(const struct git_hash_algo *algo, struct git_hash_ctx *ctx, uint8_t *final, const void *p, size_t len)
{
- algo->init_fn(ctx);
+ git_hash_init(ctx, algo);
git_hash_update(ctx, p, len);
git_hash_final(final, ctx);
}
diff --git a/t/helper/test-hash.c b/t/helper/test-hash.c
index f0ee61c8b4..1f7163695f 100644
--- a/t/helper/test-hash.c
+++ b/t/helper/test-hash.c
@@ -29,7 +29,7 @@ int cmd_hash_impl(int ac, const char **av, int algo, int unsafe)
die("OOPS");
}
- algop->init_fn(&ctx);
+ git_hash_init(&ctx, algop);
while (1) {
ssize_t sz, this_sz;
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index 3fa534fbdf..7719fb3a76 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -97,7 +97,7 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
/* Write the data as uncompressed zlib */
write_uncompressed_zlib(f, pack_ctx, data, len, algo);
- algo->init_fn(&ctx);
+ git_hash_init(&ctx, algo);
object_header_len = format_object_header(object_header,
sizeof(object_header),
type, len);
@@ -430,7 +430,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,
f = xfopen(path, "wb");
- algo->init_fn(&pack_ctx);
+ git_hash_init(&pack_ctx, algo);
/* Write pack header */
fwrite_or_die(f, &pack_header, sizeof(pack_header));
diff --git a/t/unit-tests/u-hash.c b/t/unit-tests/u-hash.c
index bd4ac6a6e1..19f4efd410 100644
--- a/t/unit-tests/u-hash.c
+++ b/t/unit-tests/u-hash.c
@@ -12,7 +12,7 @@ static void check_hash_data(const void *data, size_t data_length,
unsigned char hash[GIT_MAX_HEXSZ];
const struct git_hash_algo *algop = &hash_algos[i];
- algop->init_fn(&ctx);
+ git_hash_init(&ctx, algop);
git_hash_update(&ctx, data, data_length);
git_hash_final(hash, &ctx);
diff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci
new file mode 100644
index 0000000000..04270ee043
--- /dev/null
+++ b/tools/coccinelle/hash.cocci
@@ -0,0 +1,9 @@
+@@
+identifier f != git_hash_init;
+expression ALGO;
+struct git_hash_ctx *CTX;
+@@
+ f(...) {<...
+- ALGO->init_fn(CTX);
++ git_hash_init(CTX, ALGO);
+ ...>}
diff --git a/trace2/tr2_sid.c b/trace2/tr2_sid.c
index 1c1d27b0ee..131b4f5a62 100644
--- a/trace2/tr2_sid.c
+++ b/trace2/tr2_sid.c
@@ -45,7 +45,7 @@ static void tr2_sid_append_my_sid_component(void)
if (xgethostname(hostname, sizeof(hostname)))
strbuf_add(&tr2sid_buf, "Localhost", 9);
else {
- algo->init_fn(&ctx);
+ git_hash_init(&ctx, algo);
git_hash_update(&ctx, hostname, strlen(hostname));
git_hash_final(hash, &ctx);
hash_to_hex_algop_r(hex, hash, algo);
--
2.55.0.459.g1b256877c9
^ permalink raw reply related
* [PATCH v2 2/7] hash: convert remaining direct function calls
From: Jeff King @ 2026-07-08 3:52 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260708035235.GA41491@coredump.intra.peff.net>
The previous patch added a coccinelle rule to make sure callers always
use git_hash_init() rather than direct function pointers from the algo
struct.
Let's do the same for the rest of the git_hash_*() wrappers. I split
these out because they're a bit different: they implicitly use the algop
pointer in the git_hash_ctx. So when we convert:
-algo->update_fn(&ctx, buf, len);
+git_hash_update(&ctx, buf, len);
we drop the reference to algo entirely! But this is always going to be
the right thing. If "algo" does not match what is in ctx.algop, then
we'd already be invoking undefined behavior.
So in addition to making it possible to add more logic to the
git_hash_*() functions, we're avoiding the need to pass around the extra
algo pointer and make sure that it matches what's in "ctx".
The rest of the patch is the mechanical application of that coccinelle
patch, plus a minor cleanup in test-synthesize.c to drop a now-unused
function parameter (since we don't have to pass around the algo
separately anymore).
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/submodule--helper.c | 8 +++---
t/helper/test-synthesize.c | 29 ++++++++++----------
tools/coccinelle/hash.cocci | 54 +++++++++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 19 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index bf114a7856..510f193a15 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -551,10 +551,10 @@ static void create_default_gitdir_config(const char *submodule_name)
/* Case 2.4: If all the above failed, try a hash of the name as a last resort */
header_len = snprintf(header, sizeof(header), "blob %zu", strlen(submodule_name));
git_hash_init(&ctx, the_hash_algo);
- the_hash_algo->update_fn(&ctx, header, header_len);
- the_hash_algo->update_fn(&ctx, "\0", 1);
- the_hash_algo->update_fn(&ctx, submodule_name, strlen(submodule_name));
- the_hash_algo->final_fn(raw_name_hash, &ctx);
+ git_hash_update(&ctx, header, header_len);
+ git_hash_update(&ctx, "\0", 1);
+ git_hash_update(&ctx, submodule_name, strlen(submodule_name));
+ git_hash_final(raw_name_hash, &ctx);
hash_to_hex_algop_r(hex_name_hash, raw_name_hash, the_hash_algo);
strbuf_reset(&gitdir_path);
repo_git_path_append(the_repository, &gitdir_path, "modules/%s", hex_name_hash);
diff --git a/t/helper/test-synthesize.c b/t/helper/test-synthesize.c
index 7719fb3a76..fd116c87ba 100644
--- a/t/helper/test-synthesize.c
+++ b/t/helper/test-synthesize.c
@@ -25,8 +25,7 @@ static const unsigned char zeros[BLOCK_SIZE];
* Updates the pack checksum context.
*/
static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,
- const void *data, size_t len,
- const struct git_hash_algo *algo)
+ const void *data, size_t len)
{
unsigned char zlib_header[2] = { 0x78, 0x01 }; /* CMF, FLG */
unsigned char block_header[5];
@@ -37,7 +36,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,
/* Write zlib header */
fwrite_or_die(f, zlib_header, sizeof(zlib_header));
- algo->update_fn(pack_ctx, zlib_header, 2);
+ git_hash_update(pack_ctx, zlib_header, 2);
/* Write uncompressed blocks (max 64KB each) */
do {
@@ -52,11 +51,11 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,
block_header[4] = block_header[2] ^ 0xff;
fwrite_or_die(f, block_header, sizeof(block_header));
- algo->update_fn(pack_ctx, block_header, 5);
+ git_hash_update(pack_ctx, block_header, 5);
if (block_len) {
fwrite_or_die(f, block_data, block_len);
- algo->update_fn(pack_ctx, block_data, block_len);
+ git_hash_update(pack_ctx, block_data, block_len);
adler = adler32(adler, block_data, block_len);
}
@@ -68,7 +67,7 @@ static void write_uncompressed_zlib(FILE *f, struct git_hash_ctx *pack_ctx,
/* Write adler32 checksum */
put_be32(adler_buf, adler);
fwrite_or_die(f, adler_buf, sizeof(adler_buf));
- algo->update_fn(pack_ctx, adler_buf, 4);
+ git_hash_update(pack_ctx, adler_buf, 4);
}
/*
@@ -92,24 +91,24 @@ static void write_pack_object(FILE *f, struct git_hash_ctx *pack_ctx,
sizeof(pack_header),
type, len);
fwrite_or_die(f, pack_header, pack_header_len);
- algo->update_fn(pack_ctx, pack_header, pack_header_len);
+ git_hash_update(pack_ctx, pack_header, pack_header_len);
/* Write the data as uncompressed zlib */
- write_uncompressed_zlib(f, pack_ctx, data, len, algo);
+ write_uncompressed_zlib(f, pack_ctx, data, len);
git_hash_init(&ctx, algo);
object_header_len = format_object_header(object_header,
sizeof(object_header),
type, len);
- algo->update_fn(&ctx, object_header, object_header_len);
+ git_hash_update(&ctx, object_header, object_header_len);
if (data)
- algo->update_fn(&ctx, data, len);
+ git_hash_update(&ctx, data, len);
else {
for (size_t i = len / BLOCK_SIZE; i; i--)
- algo->update_fn(&ctx, zeros, BLOCK_SIZE);
- algo->update_fn(&ctx, zeros, len % BLOCK_SIZE);
+ git_hash_update(&ctx, zeros, BLOCK_SIZE);
+ git_hash_update(&ctx, zeros, len % BLOCK_SIZE);
}
- algo->final_oid_fn(oid, &ctx);
+ git_hash_final_oid(oid, &ctx);
}
/*
@@ -434,7 +433,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,
/* Write pack header */
fwrite_or_die(f, &pack_header, sizeof(pack_header));
- algo->update_fn(&pack_ctx, &pack_header, sizeof(pack_header));
+ git_hash_update(&pack_ctx, &pack_header, sizeof(pack_header));
/* 1. Write the large blob */
write_pack_object(f, &pack_ctx, OBJ_BLOB, NULL, blob_size, &blob_oid, algo);
@@ -472,7 +471,7 @@ static int generate_pack_with_large_object(const char *path, size_t blob_size,
write_pack_object(f, &pack_ctx, OBJ_COMMIT, buf.buf, buf.len, &final_commit_oid, algo);
/* Write pack trailer (checksum) */
- algo->final_fn(pack_hash, &pack_ctx);
+ git_hash_final(pack_hash, &pack_ctx);
fwrite_or_die(f, pack_hash, algo->rawsz);
if (fclose(f))
die_errno(_("could not close '%s'"), path);
diff --git a/tools/coccinelle/hash.cocci b/tools/coccinelle/hash.cocci
index 04270ee043..d0e2e5f4b1 100644
--- a/tools/coccinelle/hash.cocci
+++ b/tools/coccinelle/hash.cocci
@@ -7,3 +7,57 @@ struct git_hash_ctx *CTX;
- ALGO->init_fn(CTX);
+ git_hash_init(CTX, ALGO);
...>}
+
+@@
+identifier f != git_hash_clone;
+expression ALGO;
+struct git_hash_ctx *SRC;
+struct git_hash_ctx *DST;
+@@
+ f(...) {<...
+- ALGO->clone_fn(DST, SRC);
++ git_hash_clone(DST, SRC);
+ ...>}
+
+@@
+identifier f != git_hash_update;
+expression ALGO;
+struct git_hash_ctx *CTX;
+expression list ARGS;
+@@
+ f(...) {<...
+- ALGO->update_fn(CTX, ARGS);
++ git_hash_update(CTX, ARGS);
+ ...>}
+
+@@
+identifier f != git_hash_final;
+expression ALGO;
+struct git_hash_ctx *CTX;
+expression list ARGS;
+@@
+ f(...) {<...
+- ALGO->final_fn(ARGS, CTX);
++ git_hash_final(ARGS, CTX);
+ ...>}
+
+@@
+identifier f != git_hash_final_oid;
+expression ALGO;
+struct git_hash_ctx *CTX;
+expression list ARGS;
+@@
+ f(...) {<...
+- ALGO->final_oid_fn(ARGS, CTX);
++ git_hash_final_oid(ARGS, CTX);
+ ...>}
+
+@@
+identifier f != git_hash_discard;
+expression ALGO;
+struct git_hash_ctx *CTX;
+@@
+ f(...) {<...
+- ALGO->discard_fn(CTX);
++ git_hash_discard(CTX);
+ ...>}
--
2.55.0.459.g1b256877c9
^ permalink raw reply related
* [PATCH v2 3/7] hash: document function pointers and wrappers
From: Jeff King @ 2026-07-08 3:52 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260708035235.GA41491@coredump.intra.peff.net>
We want people to use the git_hash_*() wrappers rather than the bare
function pointers in the git_hash_algo struct. Let's document them
rather than the bare pointers, and warn people away from the pointers.
Coccinelle will eventually force the use of the wrappers, but it's
helpful to lead readers in the right direction from the start.
While we're here we can document a few other bits of wisdom I've turned
up while working in this area:
- You have to initialize the destination of a git_hash_clone(). This
is something we may eventually change for efficiency, but we should
definitely document the requirement for now.
- You must eventually finalize or discard a hash, since some backends
may allocate resources during initialization.
Signed-off-by: Jeff King <peff@peff.net>
---
hash.h | 43 ++++++++++++++++++++++++++++++++-----------
1 file changed, 32 insertions(+), 11 deletions(-)
diff --git a/hash.h b/hash.h
index 0a23ef4dfd..121ecf13aa 100644
--- a/hash.h
+++ b/hash.h
@@ -309,22 +309,15 @@ struct git_hash_algo {
/* The block size of the hash. */
size_t blksz;
- /* The hash initialization function. */
+ /*
+ * Low-level implementation hooks. Callers should use the git_hash_*
+ * wrappers below rather than invoking these directly.
+ */
git_hash_init_fn init_fn;
-
- /* The hash context cloning function. */
git_hash_clone_fn clone_fn;
-
- /* The hash update function. */
git_hash_update_fn update_fn;
-
- /* The hash finalization function. */
git_hash_final_fn final_fn;
-
- /* The hash finalization function for object IDs. */
git_hash_final_oid_fn final_oid_fn;
-
- /* Discard an initialized hash without finalizing. */
git_hash_discard_fn discard_fn;
/* The OID of the empty tree. */
@@ -341,12 +334,40 @@ struct git_hash_algo {
};
extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];
+/*
+ * Prepare an uninitialized hash context for use. You must eventually release
+ * the context with git_hash_final() (or final_oid()) or by calling
+ * git_hash_discard().
+ */
void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop);
+
+/*
+ * Clone the state of a hash. Both src and dst must have been initialized with
+ * git_hash_init().
+ */
void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src);
+
+/*
+ * Add more data to an initialized hash context.
+ */
void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len);
+
+/*
+ * Retrieve the final hash value from a context, releasing any resources.
+ */
void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx);
+
+/*
+ * Like git_hash_final(), but write the result into an object_id.
+ */
void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx);
+
+/*
+ * Discard a hash context without computing the final value, but still
+ * releasing any resources.
+ */
void git_hash_discard(struct git_hash_ctx *ctx);
+
const struct git_hash_algo *hash_algo_ptr_by_number(uint32_t algo);
struct git_hash_ctx *git_hash_alloc(void);
void git_hash_free(struct git_hash_ctx *ctx);
--
2.55.0.459.g1b256877c9
^ permalink raw reply related
* [PATCH v2 4/7] hash: make git_hash_discard() idempotent
From: Jeff King @ 2026-07-08 3:52 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260708035235.GA41491@coredump.intra.peff.net>
You must always either finalize or discard a hash context to release any
resources, but you must call only one such function. This creates extra
work for some callers, since their cleanup code paths need to know
whether they got there via their happy path (and the finalization
happened) or due to an error (in which case they need to discard).
Let's add an "active" flag that turns a redundant discard into a noop.
That lets you safely do this:
git_hash_init(&ctx, algo);
...
if (some_error)
goto out;
...
git_hash_final(result, &ctx);
out:
git_hash_discard(&ctx);
This should avoid future errors, and will also let us simplify a few
existing callers (in future patches).
Signed-off-by: Jeff King <peff@peff.net>
---
hash.c | 6 ++++++
hash.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/hash.c b/hash.c
index 55d1d41770..b1296f0018 100644
--- a/hash.c
+++ b/hash.c
@@ -285,6 +285,7 @@ void git_hash_free(struct git_hash_ctx *ctx)
void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)
{
algop->init_fn(ctx);
+ ctx->active = true;
}
void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)
@@ -300,16 +301,21 @@ void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)
void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)
{
ctx->algop->final_fn(hash, ctx);
+ ctx->active = false;
}
void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)
{
ctx->algop->final_oid_fn(oid, ctx);
+ ctx->active = false;
}
void git_hash_discard(struct git_hash_ctx *ctx)
{
+ if (!ctx->active)
+ return;
ctx->algop->discard_fn(ctx);
+ ctx->active = false;
}
uint32_t hash_algo_by_name(const char *name)
diff --git a/hash.h b/hash.h
index 121ecf13aa..cf94ad5700 100644
--- a/hash.h
+++ b/hash.h
@@ -281,6 +281,7 @@ struct git_hash_ctx {
git_SHA_CTX_unsafe sha1_unsafe;
git_SHA256_CTX sha256;
} state;
+ bool active;
};
typedef void (*git_hash_init_fn)(struct git_hash_ctx *ctx);
--
2.55.0.459.g1b256877c9
^ permalink raw reply related
* [PATCH v2 5/7] csum-file: use idempotent git_hash_discard()
From: Jeff King @ 2026-07-08 3:53 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260708035235.GA41491@coredump.intra.peff.net>
Now that it is safe to call git_hash_discard() even after finalizing it,
we can simplify our cleanup logic a bit. This is mostly undoing a few
bits of 64337aecde (csum-file: always finalize or discard hash,
2026-07-02):
- We no longer need a separate free_hashfile_memory() function for
finalize_hashfile(). It can just call free_hashfile(), which will
now discard (or not) the hash as appropriate.
- When f->skip_hash is set, we don't need to discard; we can rely on
free_hashfile() to do it.
Signed-off-by: Jeff King <peff@peff.net>
---
csum-file.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/csum-file.c b/csum-file.c
index 7e81391524..fe18ee1de3 100644
--- a/csum-file.c
+++ b/csum-file.c
@@ -55,32 +55,25 @@ void hashflush(struct hashfile *f)
}
}
-static void free_hashfile_memory(struct hashfile *f)
+void free_hashfile(struct hashfile *f)
{
+ git_hash_discard(&f->ctx);
free(f->buffer);
free(f->check_buffer);
free(f);
}
-void free_hashfile(struct hashfile *f)
-{
- git_hash_discard(&f->ctx);
- free_hashfile_memory(f);
-}
-
int finalize_hashfile(struct hashfile *f, unsigned char *result,
enum fsync_component component, unsigned int flags)
{
int fd;
hashflush(f);
- if (f->skip_hash) {
- git_hash_discard(&f->ctx);
+ if (f->skip_hash)
hashclr(f->buffer, f->algop);
- } else {
+ else
git_hash_final(f->buffer, &f->ctx);
- }
if (result)
hashcpy(result, f->buffer, f->algop);
@@ -105,7 +98,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result,
if (close(f->check_fd))
die_errno("%s: sha1 file error on close", f->name);
}
- free_hashfile_memory(f);
+ free_hashfile(f);
return fd;
}
--
2.55.0.459.g1b256877c9
^ permalink raw reply related
* [PATCH v2 6/7] http: use idempotent git_hash_discard()
From: Jeff King @ 2026-07-08 3:53 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260708035235.GA41491@coredump.intra.peff.net>
Now that it is OK to call git_hash_discard() even after finalizing the
hash, we no longer need the ctx_valid bool added by a2d8ea5a76 (http:
discard hash in dumb-http http_object_request, 2026-07-02).
Signed-off-by: Jeff King <peff@peff.net>
---
http.c | 5 +----
http.h | 1 -
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/http.c b/http.c
index 0341de5031..caccf2108e 100644
--- a/http.c
+++ b/http.c
@@ -2880,7 +2880,6 @@ struct http_object_request *new_http_object_request(const char *base_url,
git_inflate_init(&freq->stream);
git_hash_init(&freq->c, the_hash_algo);
- freq->hash_ctx_valid = 1;
freq->url = get_remote_object_url(base_url, hex, 0);
@@ -2989,7 +2988,6 @@ int finish_http_object_request(struct http_object_request *freq)
}
git_hash_final_oid(&freq->real_oid, &freq->c);
- freq->hash_ctx_valid = 0;
if (freq->zret != Z_STREAM_END) {
unlink_or_warn(freq->tmpfile.buf);
return -1;
@@ -3030,8 +3028,7 @@ void release_http_object_request(struct http_object_request **freq_p)
curl_slist_free_all(freq->headers);
strbuf_release(&freq->tmpfile);
git_inflate_end(&freq->stream);
- if (freq->hash_ctx_valid)
- git_hash_discard(&freq->c);
+ git_hash_discard(&freq->c);
free(freq);
*freq_p = NULL;
diff --git a/http.h b/http.h
index 6b0639150f..729c51904d 100644
--- a/http.h
+++ b/http.h
@@ -255,7 +255,6 @@ struct http_object_request {
struct object_id oid;
struct object_id real_oid;
struct git_hash_ctx c;
- int hash_ctx_valid;
git_zstream stream;
int zret;
int rename;
--
2.55.0.459.g1b256877c9
^ permalink raw reply related
* [PATCH v2 7/7] hash: check ctx->active flag in all wrapper functions
From: Jeff King @ 2026-07-08 3:53 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, brian m. carlson, Junio C Hamano
In-Reply-To: <20260708035235.GA41491@coredump.intra.peff.net>
It only makes sense to call git_hash_update(), etc, on a hash context
that has been initialized but not yet finalized or discarded. This is an
unlikely error to make, but it's easy for us to catch it and complain.
It's especially important because it would quietly "work" for many hash
backends (like sha1dc, which is just manipulating some bytes) but would
cause undefined behavior with others (like OpenSSL, which puts the
context onto the heap). Checking the flag lets us catch problems
consistently on every build.
Note that we can't do the same for git_hash_init(). Even though it would
cause a leak to call it twice (without an intervening final/discard),
the point of the function is that the contents of the struct are
undefined before the call. But calling it twice is an even less likely
error to make, so not covering it is OK.
We leave git_hash_discard() alone, as its idempotent behavior is
convenient for callers. We _could_ try to do something similar for
git_hash_final(), allowing:
git_hash_final(result, &ctx);
git_hash_final(other_result, &ctx);
but it does not make much sense. After the first final() call we have
thrown away the state, so we cannot produce the same output. We could
come up with some sensible output (the null hash, or the empty hash),
but double-calls like this are more likely a bug, so our best bet is to
complain loudly (whereas the current code produces either nonsense
output or undefined behavior, depending on the backend).
Signed-off-by: Jeff King <peff@peff.net>
---
hash.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hash.c b/hash.c
index b1296f0018..82f7e24404 100644
--- a/hash.c
+++ b/hash.c
@@ -290,22 +290,32 @@ void git_hash_init(struct git_hash_ctx *ctx, const struct git_hash_algo *algop)
void git_hash_clone(struct git_hash_ctx *dst, const struct git_hash_ctx *src)
{
+ if (!src->active)
+ BUG("attempt to copy from an inactive hash context");
+ if (!dst->active)
+ BUG("attempt to copy to an inactive hash context");
src->algop->clone_fn(dst, src);
}
void git_hash_update(struct git_hash_ctx *ctx, const void *in, size_t len)
{
+ if (!ctx->active)
+ BUG("attempt to update an inactive hash context");
ctx->algop->update_fn(ctx, in, len);
}
void git_hash_final(unsigned char *hash, struct git_hash_ctx *ctx)
{
+ if (!ctx->active)
+ BUG("attempt to finalize an inactive hash context");
ctx->algop->final_fn(hash, ctx);
ctx->active = false;
}
void git_hash_final_oid(struct object_id *oid, struct git_hash_ctx *ctx)
{
+ if (!ctx->active)
+ BUG("attempt to finalize an inactive hash context");
ctx->algop->final_oid_fn(oid, ctx);
ctx->active = false;
}
--
2.55.0.459.g1b256877c9
^ permalink raw reply related
* Re: [PATCH 1/7] hash: use git_hash_init() consistently
From: Jeff King @ 2026-07-08 3:54 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Patrick Steinhardt
In-Reply-To: <ak1u25b2pmRAQIxD@fruit.crustytoothpaste.net>
On Tue, Jul 07, 2026 at 09:25:48PM +0000, brian m. carlson wrote:
> On 2026-07-07 at 05:01:41, Jeff King wrote:
> > We'd like to add more logic to git_hash_init(), but many callers skip it
> > and call algop->init_fn() directly. Let's make sure we're consistently
> > using the wrapper by adding a coccinelle rule.
> >
> > Besides the coccinelle file itself, this is a purely mechanical
> > conversion based on the patch it generates. There should be no bare
> > init_fn() calls left (except for the one in the wrapper).
>
> For context, the reason `git_hash_init` exists is that our Rust code
> needs to initialize a hash context but it treats `const struct
> git_hash_algo *` as `const void *` and doesn't have any access to the
> contents of the structure. We could fix this with `cbindgen` and
> `bindgen`, but haven't done so yet.
>
> So that's why everybody has been using `init_fn` instead of
> `git_hash_init`. Anyway, I have no objections to making this the
> standard interface going forward.
Thanks, I remember there being some actual reason but couldn't recall
exactly what it was. The use of bare algo->update_fn(), etc, in two
spots was what really puzzled me. It's not wrong, but just harder to
write than the usual way. ;)
-Peff
^ permalink raw reply
* Re: [PATCH v3 0/4] parseopt: exit 0 on help
From: Jeff King @ 2026-07-08 3:59 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Junio C Hamano
In-Reply-To: <20260708001557.3581080-1-sandals@crustytoothpaste.net>
On Wed, Jul 08, 2026 at 12:15:53AM +0000, brian m. carlson wrote:
> Changes since v2:
>
> * Fix inverted condition in t1517.
> * Stop checking for old versions of SVN Perl libraries since they are
> so old nobody is using them.
> * Adjust the various cases where we choose between the error and
> non-error help output.
Thanks, I have no complaints on this version.
In the earlier thread I sketched out a hypothetical caller that might be
affected by the change, but beyond raw speculation, I don't think we
have any way of knowing how common such a thing is. So I'm inclined to
proceed and see if anybody screams during this development cycle.
-Peff
^ permalink raw reply
* [PATCH v2 01/11] object-file: rename files transaction prepare function
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
The "files" ODB transaction backend lazily creates a temporary object
directory when the first loose object is written to the transaction via
`prepare_loose_object_transaction()`. In a subsequent commit, the
temporary directory is used to also write packfiles to.
Rename the function to `odb_transaction_files_prepare()` accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/object-file.c b/object-file.c
index e3d92bbda2..a3eb8d71dd 100644
--- a/object-file.c
+++ b/object-file.c
@@ -499,7 +499,7 @@ struct odb_transaction_files {
struct transaction_packfile packfile;
};
-static void prepare_loose_object_transaction(struct odb_transaction *base)
+static void odb_transaction_files_prepare(struct odb_transaction *base)
{
struct odb_transaction_files *transaction =
container_of_or_null(base, struct odb_transaction_files, base);
@@ -761,7 +761,7 @@ int write_loose_object(struct odb_source_loose *loose,
static struct strbuf filename = STRBUF_INIT;
if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
- prepare_loose_object_transaction(loose->base.odb->transaction);
+ odb_transaction_files_prepare(loose->base.odb->transaction);
odb_loose_path(loose, &filename, oid);
@@ -825,7 +825,7 @@ int odb_source_loose_write_stream(struct odb_source_loose *loose,
int hdrlen;
if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
- prepare_loose_object_transaction(loose->base.odb->transaction);
+ odb_transaction_files_prepare(loose->base.odb->transaction);
/* Since oid is not determined, save tmp file to odb path. */
strbuf_addf(&filename, "%s/", loose->base.path);
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 00/11] receive-pack: use ODB transactions to stage object writes
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260624041920.2601961-1-jltobler@gmail.com>
Greetings,
This patch series replaces direct usage of the `tmp_objdir` interfaces
in git-receive-pack(1) to instead use the `odb_transaction` interfaces
to create/manage a staging area to write objects to. The purpose of this
change is to get git-receive-pack(1) one step closer to being ODB
backend agnostic. For now, the object writes themselves are still
"files" backend specific due to being handled by the git-index-pack(1)
and git-unpack-objects(1) child processes. This will be tackled in a
separate series though.
Changes since V1:
- Adapted other "file" ODB transaction helpers to be more consistent
with current naming scheme.
- Removed redundant NULL transaction handling from
`odb_transaction_files_begin()`.
- `odb_transaction_begin()` now returns an error if there is already
an inflight transaction pending instead of setting the `out` pointer
to NULL.
- Updated `odb_transaction_env()` to return an error code and append
environment variables to a strvec provided as an argument.
- Removed redundant setting of tmpdir environment variables for child
processes after tmpdir has been migrated.
- Split changes adding ODB transaction flags into a separate commit.
- Consistently wire the ODB transaction throughout git-receive-pack
code instead of reading it from `the_repository`.
- Updated user facing error message.
- Updated some comments to better document functions/flags.
- Clarified some commit messages.
- Fixed typos.
Thanks,
-Justin
Justin Tobler (11):
object-file: rename files transaction prepare function
object-file: rename files transaction fsync function
object-file: embed transaction flush logic in commit function
object-file: drop check for inflight transactions
object-file: propagate files transaction errors
odb/transaction: propagate begin errors
odb/transaction: propagate commit errors
odb/transaction: add transaction env interface
odb/transaction: introduce ODB transaction flags
builtin/receive-pack: drop redundant tmpdir env
builtin/receive-pack: stage incoming objects via ODB transactions
builtin/add.c | 2 +-
builtin/receive-pack.c | 69 ++++++++---------
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 2 +-
cache-tree.c | 7 +-
object-file.c | 159 +++++++++++++++++++++++++--------------
object-file.h | 8 +-
odb/source-files.c | 9 +--
odb/source-inmemory.c | 3 +-
odb/source-loose.c | 3 +-
odb/source.h | 9 ++-
odb/transaction.c | 34 +++++++--
odb/transaction.h | 60 ++++++++++++---
read-cache.c | 7 +-
14 files changed, 244 insertions(+), 130 deletions(-)
Range-diff against v1:
1: 9c14b219ad = 1: 9c14b219ad object-file: rename files transaction prepare function
-: ---------- > 2: 5703a9e93b object-file: rename files transaction fsync function
-: ---------- > 3: 4c37398ac8 object-file: embed transaction flush logic in commit function
-: ---------- > 4: 623c6b02ea object-file: drop check for inflight transactions
2: 201f543692 ! 5: ca59176657 object-file: propagate files transaction errors
@@ Commit message
the temporary directory used to stage objects, but silently ignores
these errors. Instead return errors encountered in the
`odb_transaction_files_{prepare,begin,commit}()` interfaces to allow
- callers to handle as needed.
+ callers to handle them as needed.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
@@ object-file.c: static void odb_transaction_files_prepare(struct odb_transaction
+ return 0;
}
- static void fsync_loose_object_transaction(struct odb_transaction *base,
-@@ object-file.c: static void fsync_loose_object_transaction(struct odb_transaction *base,
- /*
- * Cleanup after batch-mode fsync_object_files.
- */
--static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
-+static int flush_loose_object_transaction(struct odb_transaction_files *transaction)
- {
- struct strbuf temp_path = STRBUF_INIT;
- struct tempfile *temp;
-
- if (!transaction->objdir)
-- return;
-+ return 0;
-
- /*
- * Issue a full hardware flush against a temporary file to ensure
-@@ object-file.c: static void flush_loose_object_transaction(struct odb_transaction_files *transac
- * Make the object files visible in the primary ODB after their data is
- * fully durable.
- */
-- tmp_objdir_migrate(transaction->objdir);
-+ if (tmp_objdir_migrate(transaction->objdir))
-+ return -1;
-+
- transaction->objdir = NULL;
-+
-+ return 0;
- }
-
- /* Finalize a file on disk, and close it. */
+ static void odb_transaction_files_fsync(struct odb_transaction *base,
@@ object-file.c: int read_loose_object(struct repository *repo,
return ret;
}
@@ object-file.c: int read_loose_object(struct repository *repo,
{
struct odb_transaction_files *transaction =
container_of(base, struct odb_transaction_files, base);
+@@ object-file.c: static void odb_transaction_files_commit(struct odb_transaction *base)
+ * Make the object files visible in the primary ODB after their data is
+ * fully durable.
+ */
+- tmp_objdir_migrate(transaction->objdir);
++ if (tmp_objdir_migrate(transaction->objdir))
++ return -1;
++
+ transaction->objdir = NULL;
+ }
-- flush_loose_object_transaction(transaction);
-+ if (flush_loose_object_transaction(transaction))
-+ return -1;
flush_packfile_transaction(transaction);
+
+ return 0;
@@ object-file.c: int read_loose_object(struct repository *repo,
+ struct odb_transaction **out)
{
struct odb_transaction_files *transaction;
- struct object_database *odb = source->odb;
-
-- if (odb->transaction)
-- return NULL;
-+ if (odb->transaction) {
-+ *out = NULL;
-+ return 0;
-+ }
- transaction = xcalloc(1, sizeof(*transaction));
+@@ object-file.c: struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
transaction->base.source = source;
transaction->base.commit = odb_transaction_files_commit;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
@@ object-file.c: int read_loose_object(struct repository *repo,
## object-file.h ##
@@ object-file.h: struct odb_transaction;
- * Tell the object database to optimize for adding
* multiple objects. odb_transaction_files_commit must be called
- * to make new objects visible. If a transaction is already
-- * pending, NULL is returned.
-+ * pending, out is set to NULL.
+ * to make new objects visible.
*/
-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
+int odb_transaction_files_begin(struct odb_source *source,
@@ odb/source-files.c: static int odb_source_files_write_object_stream(struct odb_s
## odb/transaction.h ##
@@ odb/transaction.h: struct odb_transaction {
+ /* The ODB source the transaction is opened against. */
struct odb_source *source;
- /* The ODB source specific callback invoked to commit a transaction. */
+- /* The ODB source specific callback invoked to commit a transaction. */
- void (*commit)(struct odb_transaction *transaction);
++ /*
++ * The ODB source specific callback invoked to commit a transaction.
++ * Returns 0 on success, a negative error code otherwise.
++ */
+ int (*commit)(struct odb_transaction *transaction);
/*
3: 68cdd88ab5 ! 6: 717a1ce9a7 odb/transaction: propagate begin errors
@@ Commit message
convenience for callsites that do not need to handle errors
explicitly.
+ Note that `odb_transaction_begin()` now returns an error when the ODB
+ already has an inflight transaction pending. ODB transaction call sites
+ that may encounter an inflight transaction are updated to explicitly
+ handle this case.
+
Signed-off-by: Justin Tobler <jltobler@gmail.com>
## builtin/add.c ##
@@ builtin/update-index.c: int cmd_update_index(int argc,
parseopt_state = parse_options_step(&ctx, options,
## cache-tree.c ##
+@@ cache-tree.c: static int update_one(struct cache_tree *it,
+
+ int cache_tree_update(struct index_state *istate, int flags)
+ {
++ int inflight = !!the_repository->objects->transaction;
+ struct odb_transaction *transaction;
+ int skip, i;
+
@@ cache-tree.c: int cache_tree_update(struct index_state *istate, int flags)
trace_performance_enter();
trace2_region_enter("cache_tree", "update", istate->repo);
- transaction = odb_transaction_begin(the_repository->objects);
-+ odb_transaction_begin_or_die(the_repository->objects, &transaction);
++ if (!inflight)
++ odb_transaction_begin_or_die(the_repository->objects, &transaction);
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
- odb_transaction_commit(transaction);
+- odb_transaction_commit(transaction);
++ if (!inflight)
++ odb_transaction_commit(transaction);
+ trace2_region_leave("cache_tree", "update", istate->repo);
+ trace_performance_leave("cache_tree_update");
+ if (i < 0)
## object-file.c ##
@@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
@@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
if (flags & INDEX_WRITE_OBJECT) {
struct object_database *odb = the_repository->objects;
- struct odb_transaction *transaction = odb_transaction_begin(odb);
-+ struct odb_transaction *transaction;
++ struct odb_transaction *transaction = odb->transaction;
++ int inflight = !!transaction;
-+ odb_transaction_begin_or_die(odb, &transaction);
- ret = odb_transaction_write_object_stream(odb->transaction,
+- ret = odb_transaction_write_object_stream(odb->transaction,
++ if (!inflight)
++ odb_transaction_begin_or_die(odb, &transaction);
++ ret = odb_transaction_write_object_stream(transaction,
&stream,
xsize_t(st->st_size),
+ oid);
+- odb_transaction_commit(transaction);
++ if (!inflight)
++ odb_transaction_commit(transaction);
+ } else {
+ ret = hash_blob_stream(&stream,
+ the_repository->hash_algo, oid,
## odb/transaction.c ##
@@
@@ odb/transaction.c
+int odb_transaction_begin(struct object_database *odb,
+ struct odb_transaction **out)
{
-- if (odb->transaction)
-- return NULL;
+ int ret;
++
+ if (odb->transaction)
+- return NULL;
++ return -1;
- odb_source_begin_transaction(odb->sources, &odb->transaction);
-+ if (odb->transaction) {
-+ *out = NULL;
-+ return 0;
-+ }
-
-- return odb->transaction;
+ ret = odb_source_begin_transaction(odb->sources, out);
+ odb->transaction = *out;
-+
+
+- return odb->transaction;
+ return ret;
}
@@ odb/transaction.h: struct odb_transaction {
+ * Starts an ODB transaction and returns it via `out`. Subsequent objects are
+ * written to the transaction and not committed until odb_transaction_commit()
+ * is invoked on the transaction. Returns 0 on success and a negative value on
-+ * error. If the ODB already has a pending transaction, `out` is set to NULL.
++ * error. Note that it is considered an error to start a new transaction if the
++ * ODB already has an inflight transaction pending.
*/
-struct odb_transaction *odb_transaction_begin(struct object_database *odb);
+int odb_transaction_begin(struct object_database *odb,
@@ odb/transaction.h: struct odb_transaction {
* Commits an ODB transaction making the written objects visible. If the
## read-cache.c ##
+@@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix,
+ const struct pathspec *pathspec, char *ps_matched,
+ int include_sparse, int flags, int ignored_too )
+ {
++ int inflight = !!repo->objects->transaction;
+ struct odb_transaction *transaction;
+ struct update_callback_data data;
+ struct rev_info rev;
@@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix,
* This function is invoked from commands other than 'add', which
* may not have their own transaction active.
*/
- transaction = odb_transaction_begin(repo->objects);
-+ odb_transaction_begin_or_die(repo->objects, &transaction);
++ if (!inflight)
++ odb_transaction_begin_or_die(repo->objects, &transaction);
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
- odb_transaction_commit(transaction);
+- odb_transaction_commit(transaction);
++ if (!inflight)
++ odb_transaction_commit(transaction);
+ release_revisions(&rev);
+ return !!data.add_errors;
-: ---------- > 7: ff8e133965 odb/transaction: propagate commit errors
5: 82302db9f0 ! 8: 264ba94b83 odb/transaction: add transaction env interface
@@ Commit message
The ODB transaction backend is responsible for creating/managing its own
staging area for writing objects. Other child processes spawned by Git
- may need to access to uncommitted objects or write new objects in the
+ may need access to uncommitted objects or write new objects in the
staging area though.
Introduce `odb_transaction_env()` which is expected to provide the set
of environment variables needed by a child process to access the
- transaction staging area.
+ transaction's staging area.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
## object-file.c ##
+@@
+ #include "path.h"
+ #include "read-cache-ll.h"
+ #include "setup.h"
++#include "strvec.h"
+ #include "tempfile.h"
+ #include "tmp-objdir.h"
+
@@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *base)
return 0;
}
-+static const char **odb_transaction_files_env(struct odb_transaction *base)
++static int odb_transaction_files_env(struct odb_transaction *base,
++ struct strvec *env)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+
+ odb_transaction_files_prepare(&transaction->base);
++ strvec_pushv(env, tmp_objdir_env(transaction->objdir));
+
-+ return tmp_objdir_env(transaction->objdir);
++ return 0;
+}
+
int odb_transaction_files_begin(struct odb_source *source,
@@ odb/transaction.c: int odb_transaction_write_object_stream(struct odb_transactio
return transaction->write_object_stream(transaction, stream, len, oid);
}
+
-+const char **odb_transaction_env(struct odb_transaction *transaction)
++int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env)
+{
+ if (!transaction)
-+ return NULL;
++ return 0;
+
-+ return transaction->env(transaction);
++ return transaction->env(transaction, env);
+}
## odb/transaction.h ##
@@ odb/transaction.h: struct odb_transaction {
struct object_id *oid);
+
+ /*
-+ * This callback is expected to return a NULL-terminated array of
-+ * environment variables that a child process should inherit so
-+ * that its object writes participate in the transaction. The
-+ * returned array is owned by the backend and remains valid until
-+ * the transaction ends. May return NULL when the backend does not
-+ * need to expose any state to child processes.
++ * This callback is expected to populate the provided strvec with the
++ * environment variables that a child process should inherit so that its
++ * object writes participate in the transaction. Returns 0 on success, a
++ * negative error code otherwise.
+ */
-+ const char **(*env)(struct odb_transaction *transaction);
++ int (*env)(struct odb_transaction *transaction, struct strvec *env);
};
/*
@@ odb/transaction.h: int odb_transaction_write_object_stream(struct odb_transactio
size_t len, struct object_id *oid);
+/*
-+ * Returns a NULL-terminated array of environment variables that a child
++ * Populates the provided strvec with the environment variables that a child
+ * process should inherit so that its object writes participate in the
-+ * transaction, suitable for passing via child_process.env. Returns NULL if
-+ * the transaction is NULL or the backend does not expose any state to child
-+ * processes.
++ * transaction, suitable for using via child_process.env. Returns 0 on success,
++ * a negative error code otherwise. Note that, if the specified transaction is
++ * NULL, the function is a no-op and no error is returned.
+ */
-+const char **odb_transaction_env(struct odb_transaction *transaction);
++int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env);
+
#endif
6: 0674e8a3d3 ! 9: 1e0a491ef2 builtin/receive-pack: stage incoming objects via ODB transactions
@@ Metadata
Author: Justin Tobler <jltobler@gmail.com>
## Commit message ##
- builtin/receive-pack: stage incoming objects via ODB transactions
+ odb/transaction: introduce ODB transaction flags
- Objects received by git-receive-pack(1) are quarantined in a temporary
- "incoming" directory and migrated into the object database prior to the
- reference updates. The quarantine is currently managed through
- `tmp_objdir` directly. In a pluggable ODB future, how exactly an object
- gets written to a transaction may vary for a given ODB source. Refactor
- git-receive-pack(1) to use the ODB transaction interfaces to manage the
- object staging area in a more agnostic manner accordingly.
+ The temporary directory used by git-receive-pack(1) to write objects is
+ managed slightly differently than how it is done via ODB transactions:
- Note that the temporary directory created for git-receive-pack(1) is
- eagerly created and uses a different prefix name. This behavior is
- special cased in the "files" backend by having `odb_transaction_begin()`
- callers that require this behavior provide an `ODB_TRANSACTION_RECEIVE`
- flag.
+ - The temporary directory is eagerly created upfront, instead of
+ waiting for the first object write.
+
+ - The prefix name of the temporary directory is "incoming" instead of
+ "bulk-fsync".
+
+ In a subsequent commit, git-receive-pack(1) will use ODB transactions
+ instead of `tmp_objdir` directly. To provide a means to configure the
+ same transaction behavior, introduce `enum odb_transaction_flags` and
+ the ODB_TRANSACTION_RECEIVE flag intended as a signal for ODB
+ transactions using the "files" backend to be set up for
+ git-receive-pack(1). Transaction call sites are updated accordingly to
+ provide the required flag parameter.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
@@ builtin/add.c: int cmd_add(int argc,
ps_matched = xcalloc(pathspec.nr, 1);
if (add_renormalize)
- ## builtin/receive-pack.c ##
-@@ builtin/receive-pack.c: static enum {
- } use_keepalive;
- static int keepalive_in_sec = 5;
-
--static struct tmp_objdir *tmp_objdir;
--
- static struct proc_receive_ref {
- unsigned int want_add:1,
- want_delete:1,
-@@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands,
- strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT");
- }
-
-- if (tmp_objdir)
-- strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir));
-+ if (the_repository->objects->transaction)
-+ strvec_pushv(&opt.env, odb_transaction_env(the_repository->objects->transaction));
-
- prepare_push_cert_sha1(&opt);
-
-@@ builtin/receive-pack.c: static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
- !delayed_reachability_test(si, i))
- oid_array_append(&extra, &si->shallow->oid[i]);
-
-- opt.env = tmp_objdir_env(tmp_objdir);
-+ opt.env = odb_transaction_env(the_repository->objects->transaction);
- setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
- if (check_connected(command_singleton_iterator, cmd, &opt)) {
- rollback_shallow_file(the_repository, &shallow_lock);
-@@ builtin/receive-pack.c: static void set_connectivity_errors(struct command *commands,
- /* to be checked in update_shallow_ref() */
- continue;
-
-- opt.env = tmp_objdir_env(tmp_objdir);
-+ opt.env = odb_transaction_env(the_repository->objects->transaction);
- if (!check_connected(command_singleton_iterator, &singleton,
- &opt))
- continue;
-@@ builtin/receive-pack.c: static void execute_commands(struct command *commands,
- data.si = si;
- opt.err_fd = err_fd;
- opt.progress = err_fd && !quiet;
-- opt.env = tmp_objdir_env(tmp_objdir);
-+ opt.env = odb_transaction_env(the_repository->objects->transaction);
- opt.exclude_hidden_refs_section = "receive";
-
- if (check_connected(iterate_receive_command_list, &data, &opt))
-@@ builtin/receive-pack.c: static void execute_commands(struct command *commands,
- * Now we'll start writing out refs, which means the objects need
- * to be in their final positions so that other processes can see them.
- */
-- if (tmp_objdir_migrate(tmp_objdir) < 0) {
-+ if (odb_transaction_commit(the_repository->objects->transaction)) {
- for (cmd = commands; cmd; cmd = cmd->next) {
- if (!cmd->error_string)
- cmd->error_string = "unable to migrate objects to permanent storage";
- }
- return;
- }
-- tmp_objdir = NULL;
-
- check_aliased_updates(commands);
-
-@@ builtin/receive-pack.c: static void push_header_arg(struct strvec *args, struct pack_header *hdr)
- ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
- }
-
--static const char *unpack(int err_fd, struct shallow_info *si)
-+static const char *unpack(int err_fd, struct shallow_info *si,
-+ struct odb_transaction *transaction)
- {
- struct pack_header hdr;
- const char *hdr_err;
-@@ builtin/receive-pack.c: static const char *unpack(int err_fd, struct shallow_info *si)
- strvec_push(&child.args, alt_shallow_file);
- }
-
-- tmp_objdir = tmp_objdir_create(the_repository, "incoming");
-- if (!tmp_objdir) {
-- if (err_fd > 0)
-- close(err_fd);
-- return "unable to create temporary object directory";
-- }
-- strvec_pushv(&child.env, tmp_objdir_env(tmp_objdir));
--
-- /*
-- * Normally we just pass the tmp_objdir environment to the child
-- * processes that do the heavy lifting, but we may need to see these
-- * objects ourselves to set up shallow information.
-- */
-- tmp_objdir_add_as_alternate(tmp_objdir);
-+ strvec_pushv(&child.env, odb_transaction_env(transaction));
-
- if (ntohl(hdr.hdr_entries) < unpack_limit) {
- strvec_push(&child.args, "unpack-objects");
-@@ builtin/receive-pack.c: static const char *unpack(int err_fd, struct shallow_info *si)
- return NULL;
- }
-
--static const char *unpack_with_sideband(struct shallow_info *si)
-+static const char *unpack_with_sideband(struct shallow_info *si,
-+ struct odb_transaction *transaction)
- {
- struct async muxer;
- const char *ret;
-
- if (!use_sideband)
-- return unpack(0, si);
-+ return unpack(0, si, transaction);
-
- use_keepalive = KEEPALIVE_AFTER_NUL;
- memset(&muxer, 0, sizeof(muxer));
-@@ builtin/receive-pack.c: static const char *unpack_with_sideband(struct shallow_info *si)
- if (start_async(&muxer))
- return NULL;
-
-- ret = unpack(muxer.in, si);
-+ ret = unpack(muxer.in, si, transaction);
-
- finish_async(&muxer);
- return ret;
-@@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
- struct oid_array ref = OID_ARRAY_INIT;
- struct shallow_info si;
- struct packet_reader reader;
-+ struct odb_transaction *transaction = NULL;
-
- struct option options[] = {
- OPT__QUIET(&quiet, N_("quiet")),
-@@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
- if (!si.nr_ours && !si.nr_theirs)
- shallow_update = 0;
- if (!delete_only(commands)) {
-- unpack_status = unpack_with_sideband(&si);
-+ if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
-+ unpack_status = "unable to start ODB transaction";
-+ else
-+ unpack_status = unpack_with_sideband(&si, transaction);
- update_shallow_info(commands, &si, &ref);
- }
- use_keepalive = KEEPALIVE_ALWAYS;
-
## builtin/unpack-objects.c ##
@@ builtin/unpack-objects.c: static void unpack_all(void)
progress = start_progress(the_repository,
@@ builtin/update-index.c: int cmd_update_index(int argc,
## cache-tree.c ##
@@ cache-tree.c: int cache_tree_update(struct index_state *istate, int flags)
-
trace_performance_enter();
trace2_region_enter("cache_tree", "update", istate->repo);
-- odb_transaction_begin_or_die(the_repository->objects, &transaction);
-+ odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
+ if (!inflight)
+- odb_transaction_begin_or_die(the_repository->objects, &transaction);
++ odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
- odb_transaction_commit(transaction);
+ if (!inflight)
## object-file.c ##
@@ object-file.c: struct odb_transaction_files {
@@ object-file.c: static int odb_transaction_files_prepare(struct odb_transaction *
return -1;
@@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
- struct object_database *odb = the_repository->objects;
- struct odb_transaction *transaction;
+ int inflight = !!transaction;
-- odb_transaction_begin_or_die(odb, &transaction);
-+ odb_transaction_begin_or_die(odb, &transaction, 0);
- ret = odb_transaction_write_object_stream(odb->transaction,
+ if (!inflight)
+- odb_transaction_begin_or_die(odb, &transaction);
++ odb_transaction_begin_or_die(odb, &transaction, 0);
+ ret = odb_transaction_write_object_stream(transaction,
&stream,
xsize_t(st->st_size),
-@@ object-file.c: static const char **odb_transaction_files_env(struct odb_transaction *base)
+@@ object-file.c: static int odb_transaction_files_env(struct odb_transaction *base,
}
int odb_transaction_files_begin(struct odb_source *source,
@@ object-file.c: static const char **odb_transaction_files_env(struct odb_transact
+ enum odb_transaction_flags flags)
{
struct odb_transaction_files *transaction;
- struct object_database *odb = source->odb;
+
@@ object-file.c: int odb_transaction_files_begin(struct odb_source *source,
transaction->base.commit = odb_transaction_files_commit;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
@@ object-file.c: int odb_transaction_files_begin(struct odb_source *source,
+ if (flags & ODB_TRANSACTION_RECEIVE) {
+ /*
+ * ODB transactions for git-receive-pack(1) eagerly create a
-+ * temporary directory and use a different prefix.
++ * temporary directory and use a different temporary directory
++ * prefix.
++ *
++ * NEEDSWORK: This transaction flag is only used by the "files"
++ * backend to special case temporary directory set up and
++ * handling. Ideally transaction users should not have to care
++ * though. To avoid this, we could eagerly create the temporary
++ * directory and use the same prefix name for all transactions.
+ */
+ transaction->prefix = "incoming";
+ if (odb_transaction_files_prepare(&transaction->base)) {
@@ object-file.h
/* The maximum size for an object header. */
#define MAX_HEADER_LEN 32
@@ object-file.h: struct odb_transaction;
- * pending, out is set to NULL.
+ * to make new objects visible.
*/
int odb_transaction_files_begin(struct odb_source *source,
- struct odb_transaction **out);
@@ odb/transaction.c
{
int ret;
-@@ odb/transaction.c: int odb_transaction_begin(struct object_database *odb,
- return 0;
- }
+ if (odb->transaction)
+ return -1;
- ret = odb_source_begin_transaction(odb->sources, out);
+ ret = odb_source_begin_transaction(odb->sources, out, flags);
@@ odb/transaction.h
/*
* A transaction may be started for an object database prior to writing new
@@ odb/transaction.h: struct odb_transaction {
- const char **(*env)(struct odb_transaction *transaction);
+ int (*env)(struct odb_transaction *transaction, struct strvec *env);
};
++/* Flags used to configure an ODB transaction. */
+enum odb_transaction_flags {
++ /* Configures the transaction for use with git-receive-pack(1). */
+ ODB_TRANSACTION_RECEIVE = (1 << 0),
+};
+
@@ odb/transaction.h: struct odb_transaction {
* Starts an ODB transaction and returns it via `out`. Subsequent objects are
* written to the transaction and not committed until odb_transaction_commit()
@@ odb/transaction.h: struct odb_transaction {
- * error. If the ODB already has a pending transaction, `out` is set to NULL.
+ * ODB already has an inflight transaction pending.
*/
int odb_transaction_begin(struct object_database *odb,
- struct odb_transaction **out);
@@ odb/transaction.h: struct odb_transaction {
## read-cache.c ##
@@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix,
- * This function is invoked from commands other than 'add', which
* may not have their own transaction active.
*/
-- odb_transaction_begin_or_die(repo->objects, &transaction);
-+ odb_transaction_begin_or_die(repo->objects, &transaction, 0);
+ if (!inflight)
+- odb_transaction_begin_or_die(repo->objects, &transaction);
++ odb_transaction_begin_or_die(repo->objects, &transaction, 0);
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
- odb_transaction_commit(transaction);
-
+ if (!inflight)
+ odb_transaction_commit(transaction);
-: ---------- > 10: 6c8d878349 builtin/receive-pack: drop redundant tmpdir env
4: e9303f9b08 ! 11: 8db95fef56 odb/transaction: propagate commit errors
@@ Metadata
Author: Justin Tobler <jltobler@gmail.com>
## Commit message ##
- odb/transaction: propagate commit errors
+ builtin/receive-pack: stage incoming objects via ODB transactions
- When `odb_transaction_commit()` is invoked, the return value of the
- backend commit callback is silently discarded. A backend has no way
- to signal that committing failed, such as when the "files" backend
- cannot migrate its temporary object directory into the permanent
- ODB.
+ Objects received by git-receive-pack(1) are quarantined in a temporary
+ "incoming" directory and migrated into the object database prior to the
+ reference updates. The quarantine is currently managed through
+ `tmp_objdir` directly. In a pluggable ODB future, how exactly an object
+ gets written to a transaction may vary for a given ODB source. Refactor
+ git-receive-pack(1) to use the ODB transaction interfaces to manage the
+ object staging area in a more agnostic manner accordingly.
- In a subsequent commit, git-receive-pack(1) starts using ODB transaction
- to stage objects and consequently cares about such failures so it can
- handle the error appropriately. Change the commit callback signature to
- return an int error code and have `odb_transaction_commit()` forward it
- accordingly.
+ Note that the ODB transaction is now responsible for managing the
+ primary and alternate ODBs for the repository. One small change as a
+ result is that the temporary directory is now applied as the primary ODB
+ in the main process instead of an alternate. This does not change
+ anything for git-receive-pack(1) though because it only needs access to
+ the newly written objects and doesn't care how exactly it is set up.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
- ## odb/transaction.c ##
-@@ odb/transaction.c: int odb_transaction_begin(struct object_database *odb,
- return ret;
+ ## builtin/receive-pack.c ##
+@@
+ #include "sigchain.h"
+ #include "string-list.h"
+ #include "strvec.h"
+-#include "tmp-objdir.h"
+ #include "trace.h"
+ #include "trace2.h"
+ #include "version.h"
+@@ builtin/receive-pack.c: static enum {
+ } use_keepalive;
+ static int keepalive_in_sec = 5;
+
+-static struct tmp_objdir *tmp_objdir;
+-
+ static struct proc_receive_ref {
+ unsigned int want_add:1,
+ want_delete:1,
+@@ builtin/receive-pack.c: static void receive_hook_feed_state_free(void *data)
+ static int run_receive_hook(struct command *commands,
+ const char *hook_name,
+ int skip_broken,
++ struct odb_transaction *transaction,
+ const struct string_list *push_options)
+ {
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+@@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands,
+ strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT");
+ }
+
+- if (tmp_objdir)
+- strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir));
++ if (transaction)
++ odb_transaction_env(transaction, &opt.env);
+
+ prepare_push_cert_sha1(&opt);
+
+@@ builtin/receive-pack.c: static const struct object_id *command_singleton_iterator(void *cb_data)
}
--void odb_transaction_commit(struct odb_transaction *transaction)
-+int odb_transaction_commit(struct odb_transaction *transaction)
+ static void set_connectivity_errors(struct command *commands,
+- struct shallow_info *si)
++ struct shallow_info *si,
++ struct odb_transaction *transaction)
{
-+ int ret;
+ struct command *cmd;
+
+ for (cmd = commands; cmd; cmd = cmd->next) {
+ struct command *singleton = cmd;
+ struct check_connected_options opt = CHECK_CONNECTED_INIT;
++ struct strvec env = STRVEC_INIT;
+
+ if (shallow_update && si->shallow_ref[cmd->index])
+ /* to be checked in update_shallow_ref() */
+ continue;
+
+- opt.env = tmp_objdir_env(tmp_objdir);
++ odb_transaction_env(transaction, &env);
++ opt.env = env.v;
+
- if (!transaction)
-- return;
-+ return 0;
+ if (!check_connected(command_singleton_iterator, &singleton,
+ &opt))
+ continue;
- /*
- * Ensure the transaction ending matches the pending transaction.
- */
- ASSERT(transaction == transaction->source->odb->transaction);
+ cmd->error_string = "missing necessary objects";
++
++ strvec_clear(&env);
+ }
+ }
+
+@@ builtin/receive-pack.c: static void execute_commands_atomic(struct command *commands,
+ static void execute_commands(struct command *commands,
+ const char *unpacker_error,
+ struct shallow_info *si,
++ struct odb_transaction *transaction,
+ const struct string_list *push_options)
+ {
+ struct check_connected_options opt = CHECK_CONNECTED_INIT;
+@@ builtin/receive-pack.c: static void execute_commands(struct command *commands,
+ }
-- transaction->commit(transaction);
-+ ret = transaction->commit(transaction);
-+ if (ret)
-+ return ret;
+ if (!skip_connectivity_check) {
++ struct strvec env = STRVEC_INIT;
+
- transaction->source->odb->transaction = NULL;
- free(transaction);
+ if (use_sideband) {
+ memset(&muxer, 0, sizeof(muxer));
+ muxer.proc = copy_to_sideband;
+@@ builtin/receive-pack.c: static void execute_commands(struct command *commands,
+ data.si = si;
+ opt.err_fd = err_fd;
+ opt.progress = err_fd && !quiet;
+- opt.env = tmp_objdir_env(tmp_objdir);
++ odb_transaction_env(transaction, &env);
++ opt.env = env.v;
+ opt.exclude_hidden_refs_section = "receive";
+
+ if (check_connected(iterate_receive_command_list, &data, &opt))
+- set_connectivity_errors(commands, si);
++ set_connectivity_errors(commands, si, transaction);
+
+ if (use_sideband)
+ finish_async(&muxer);
+
-+ return 0;
++ strvec_clear(&env);
+ }
+
+ reject_updates_to_hidden(commands);
+@@ builtin/receive-pack.c: static void execute_commands(struct command *commands,
+ }
+ }
+
+- if (run_receive_hook(commands, "pre-receive", 0, push_options)) {
++ if (run_receive_hook(commands, "pre-receive", 0, transaction, push_options)) {
+ for (cmd = commands; cmd; cmd = cmd->next) {
+ if (!cmd->error_string)
+ cmd->error_string = "pre-receive hook declined";
+@@ builtin/receive-pack.c: static void execute_commands(struct command *commands,
+ * Now we'll start writing out refs, which means the objects need
+ * to be in their final positions so that other processes can see them.
+ */
+- if (tmp_objdir_migrate(tmp_objdir) < 0) {
++ if (odb_transaction_commit(transaction)) {
+ for (cmd = commands; cmd; cmd = cmd->next) {
+ if (!cmd->error_string)
+ cmd->error_string = "unable to migrate objects to permanent storage";
+ }
+ return;
+ }
+- tmp_objdir = NULL;
+
+ check_aliased_updates(commands);
+
+@@ builtin/receive-pack.c: static void push_header_arg(struct strvec *args, struct pack_header *hdr)
+ ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
- int odb_transaction_write_object_stream(struct odb_transaction *transaction,
-
- ## odb/transaction.h ##
-@@ odb/transaction.h: static inline void odb_transaction_begin_or_die(struct object_database *odb,
- * Commits an ODB transaction making the written objects visible. If the
- * specified transaction is NULL, the function is a no-op.
- */
--void odb_transaction_commit(struct odb_transaction *transaction);
-+int odb_transaction_commit(struct odb_transaction *transaction);
-
- /*
- * Writes the object in the provided stream into the transaction. The resulting
+-static const char *unpack(int err_fd, struct shallow_info *si)
++static const char *unpack(int err_fd, struct shallow_info *si,
++ struct odb_transaction *transaction)
+ {
+ struct pack_header hdr;
+ const char *hdr_err;
+@@ builtin/receive-pack.c: static const char *unpack(int err_fd, struct shallow_info *si)
+ strvec_push(&child.args, alt_shallow_file);
+ }
+
+- tmp_objdir = tmp_objdir_create(the_repository, "incoming");
+- if (!tmp_objdir) {
+- if (err_fd > 0)
+- close(err_fd);
+- return "unable to create temporary object directory";
+- }
+- strvec_pushv(&child.env, tmp_objdir_env(tmp_objdir));
+-
+- /*
+- * Normally we just pass the tmp_objdir environment to the child
+- * processes that do the heavy lifting, but we may need to see these
+- * objects ourselves to set up shallow information.
+- */
+- tmp_objdir_add_as_alternate(tmp_objdir);
++ odb_transaction_env(transaction, &child.env);
+
+ if (ntohl(hdr.hdr_entries) < unpack_limit) {
+ strvec_push(&child.args, "unpack-objects");
+@@ builtin/receive-pack.c: static const char *unpack(int err_fd, struct shallow_info *si)
+ return NULL;
+ }
+
+-static const char *unpack_with_sideband(struct shallow_info *si)
++static const char *unpack_with_sideband(struct shallow_info *si,
++ struct odb_transaction *transaction)
+ {
+ struct async muxer;
+ const char *ret;
+
+ if (!use_sideband)
+- return unpack(0, si);
++ return unpack(0, si, transaction);
+
+ use_keepalive = KEEPALIVE_AFTER_NUL;
+ memset(&muxer, 0, sizeof(muxer));
+@@ builtin/receive-pack.c: static const char *unpack_with_sideband(struct shallow_info *si)
+ if (start_async(&muxer))
+ return NULL;
+
+- ret = unpack(muxer.in, si);
++ ret = unpack(muxer.in, si, transaction);
+
+ finish_async(&muxer);
+ return ret;
+@@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
+ struct oid_array ref = OID_ARRAY_INIT;
+ struct shallow_info si;
+ struct packet_reader reader;
++ struct odb_transaction *transaction = NULL;
+
+ struct option options[] = {
+ OPT__QUIET(&quiet, N_("quiet")),
+@@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
+ if (!si.nr_ours && !si.nr_theirs)
+ shallow_update = 0;
+ if (!delete_only(commands)) {
+- unpack_status = unpack_with_sideband(&si);
++ if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
++ unpack_status = "unable to start object transaction";
++ else
++ unpack_status = unpack_with_sideband(&si, transaction);
+ update_shallow_info(commands, &si, &ref);
+ }
+ use_keepalive = KEEPALIVE_ALWAYS;
+- execute_commands(commands, unpack_status, &si,
++ execute_commands(commands, unpack_status, &si, transaction,
+ &push_options);
+ delete_tempfile(&pack_lockfile);
+ sigchain_push(SIGPIPE, SIG_IGN);
+@@ builtin/receive-pack.c: int cmd_receive_pack(int argc,
+ else if (report_status)
+ report(commands, unpack_status);
+ sigchain_pop(SIGPIPE);
+- run_receive_hook(commands, "post-receive", 1,
++ run_receive_hook(commands, "post-receive", 1, NULL,
+ &push_options);
+ run_update_post_hook(commands);
+ free_commands(commands);
base-commit: ab776a62a78576513ee121424adb19597fbb7613
--
2.55.0.122.gf85a7e6620
^ permalink raw reply
* [PATCH v2 02/11] object-file: rename files transaction fsync function
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
When writing an object to a "files" ODB transaction, a full hardware
flush is not initially performed during the fsync in
`fsync_loose_object_transaction()` and instead delayed until the
transaction is later committed.
To be more consistent with other "files" ODB transaction helpers, rename
the function to `odb_transaction_files_fsync()` accordingly. The
conditional in the helper is also slightly restructured to improve
clarity to readers.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/object-file.c b/object-file.c
index a3eb8d71dd..d68824bb44 100644
--- a/object-file.c
+++ b/object-file.c
@@ -518,12 +518,17 @@ static void odb_transaction_files_prepare(struct odb_transaction *base)
tmp_objdir_replace_primary_odb(transaction->objdir, 0);
}
-static void fsync_loose_object_transaction(struct odb_transaction *base,
- int fd, const char *filename)
+static void odb_transaction_files_fsync(struct odb_transaction *base,
+ int fd, const char *filename)
{
struct odb_transaction_files *transaction =
container_of_or_null(base, struct odb_transaction_files, base);
+ if (!transaction || !transaction->objdir) {
+ fsync_or_die(fd, filename);
+ return;
+ }
+
/*
* If we have an active ODB transaction, we issue a call that
* cleans the filesystem page cache but avoids a hardware flush
@@ -531,8 +536,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *base,
* before renaming the objects to their final names as part of
* flush_batch_fsync.
*/
- if (!transaction || !transaction->objdir ||
- git_fsync(fd, FSYNC_WRITEOUT_ONLY) < 0) {
+ if (git_fsync(fd, FSYNC_WRITEOUT_ONLY) < 0) {
if (errno == ENOSYS)
warning(_("core.fsyncMethod = batch is unsupported on this platform"));
fsync_or_die(fd, filename);
@@ -553,7 +557,7 @@ static void flush_loose_object_transaction(struct odb_transaction_files *transac
/*
* Issue a full hardware flush against a temporary file to ensure
* that all objects are durable before any renames occur. The code in
- * fsync_loose_object_transaction has already issued a writeout
+ * odb_transaction_files_fsync has already issued a writeout
* request, but it has not flushed any writeback cache in the storage
* hardware or any filesystem logs. This fsync call acts as a barrier
* to ensure that the data in each new object file is durable before
@@ -582,7 +586,7 @@ static void close_loose_object(struct odb_source_loose *loose,
goto out;
if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
- fsync_loose_object_transaction(loose->base.odb->transaction, fd, filename);
+ odb_transaction_files_fsync(loose->base.odb->transaction, fd, filename);
else if (fsync_object_files > 0)
fsync_or_die(fd, filename);
else
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 03/11] object-file: embed transaction flush logic in commit function
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
When a "files" transaction is committed,
`flush_loose_object_transaction()` is invoked to handle performing a
hardware flush along with migrating the temporary object directory into
the primary. In a subsequent commit, the temporary directory is also
used to write packfiles.
Instead of maintaining a separate helper function, embed the logic to
flush and migrate the temporary directory directly into
`odb_transaction_files_commit()`.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 64 ++++++++++++++++++++++-----------------------------
1 file changed, 28 insertions(+), 36 deletions(-)
diff --git a/object-file.c b/object-file.c
index d68824bb44..33bd6c6810 100644
--- a/object-file.c
+++ b/object-file.c
@@ -543,41 +543,6 @@ static void odb_transaction_files_fsync(struct odb_transaction *base,
}
}
-/*
- * Cleanup after batch-mode fsync_object_files.
- */
-static void flush_loose_object_transaction(struct odb_transaction_files *transaction)
-{
- struct strbuf temp_path = STRBUF_INIT;
- struct tempfile *temp;
-
- if (!transaction->objdir)
- return;
-
- /*
- * Issue a full hardware flush against a temporary file to ensure
- * that all objects are durable before any renames occur. The code in
- * odb_transaction_files_fsync has already issued a writeout
- * request, but it has not flushed any writeback cache in the storage
- * hardware or any filesystem logs. This fsync call acts as a barrier
- * to ensure that the data in each new object file is durable before
- * the final name is visible.
- */
- strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
- repo_get_object_directory(transaction->base.source->odb->repo));
- temp = xmks_tempfile(temp_path.buf);
- fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
- delete_tempfile(&temp);
- strbuf_release(&temp_path);
-
- /*
- * Make the object files visible in the primary ODB after their data is
- * fully durable.
- */
- tmp_objdir_migrate(transaction->objdir);
- transaction->objdir = NULL;
-}
-
/* Finalize a file on disk, and close it. */
static void close_loose_object(struct odb_source_loose *loose,
int fd, const char *filename)
@@ -1679,7 +1644,34 @@ static void odb_transaction_files_commit(struct odb_transaction *base)
struct odb_transaction_files *transaction =
container_of(base, struct odb_transaction_files, base);
- flush_loose_object_transaction(transaction);
+ if (transaction->objdir) {
+ struct strbuf temp_path = STRBUF_INIT;
+ struct tempfile *temp;
+
+ /*
+ * Issue a full hardware flush against a temporary file to ensure
+ * that all objects are durable before any renames occur. The code in
+ * odb_transaction_files_fsync has already issued a writeout
+ * request, but it has not flushed any writeback cache in the storage
+ * hardware or any filesystem logs. This fsync call acts as a barrier
+ * to ensure that the data in each new object file is durable before
+ * the final name is visible.
+ */
+ strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX",
+ repo_get_object_directory(transaction->base.source->odb->repo));
+ temp = xmks_tempfile(temp_path.buf);
+ fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));
+ delete_tempfile(&temp);
+ strbuf_release(&temp_path);
+
+ /*
+ * Make the object files visible in the primary ODB after their data is
+ * fully durable.
+ */
+ tmp_objdir_migrate(transaction->objdir);
+ transaction->objdir = NULL;
+ }
+
flush_packfile_transaction(transaction);
}
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 04/11] object-file: drop check for inflight transactions
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
ODB transactions are started via `odb_transaction_begin()` and contain
validation to avoid starting multiple transactions at the same time. The
"files" backend also has the same logic, but is redundant due to the
generic layer already handling it. Drop this validation from the "files"
backend accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 4 ----
object-file.h | 3 +--
2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/object-file.c b/object-file.c
index 33bd6c6810..e51389833a 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1678,10 +1678,6 @@ static void odb_transaction_files_commit(struct odb_transaction *base)
struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
{
struct odb_transaction_files *transaction;
- struct object_database *odb = source->odb;
-
- if (odb->transaction)
- return NULL;
transaction = xcalloc(1, sizeof(*transaction));
transaction->base.source = source;
diff --git a/object-file.h b/object-file.h
index 528c4e6e69..ea43d818f0 100644
--- a/object-file.h
+++ b/object-file.h
@@ -194,8 +194,7 @@ struct odb_transaction;
/*
* Tell the object database to optimize for adding
* multiple objects. odb_transaction_files_commit must be called
- * to make new objects visible. If a transaction is already
- * pending, NULL is returned.
+ * to make new objects visible.
*/
struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 05/11] object-file: propagate files transaction errors
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
The "files" transaction backend may encounter errors related to managing
the temporary directory used to stage objects, but silently ignores
these errors. Instead return errors encountered in the
`odb_transaction_files_{prepare,begin,commit}()` interfaces to allow
callers to handle them as needed.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 26 ++++++++++++++++++--------
object-file.h | 3 ++-
odb/source-files.c | 6 +-----
odb/transaction.h | 7 +++++--
4 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/object-file.c b/object-file.c
index e51389833a..64cb874fe7 100644
--- a/object-file.c
+++ b/object-file.c
@@ -499,7 +499,7 @@ struct odb_transaction_files {
struct transaction_packfile packfile;
};
-static void odb_transaction_files_prepare(struct odb_transaction *base)
+static int odb_transaction_files_prepare(struct odb_transaction *base)
{
struct odb_transaction_files *transaction =
container_of_or_null(base, struct odb_transaction_files, base);
@@ -511,11 +511,15 @@ static void odb_transaction_files_prepare(struct odb_transaction *base)
* added at the time they call odb_transaction_files_begin.
*/
if (!transaction || transaction->objdir)
- return;
+ return 0;
transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
- if (transaction->objdir)
- tmp_objdir_replace_primary_odb(transaction->objdir, 0);
+ if (!transaction->objdir)
+ return -1;
+
+ tmp_objdir_replace_primary_odb(transaction->objdir, 0);
+
+ return 0;
}
static void odb_transaction_files_fsync(struct odb_transaction *base,
@@ -1639,7 +1643,7 @@ int read_loose_object(struct repository *repo,
return ret;
}
-static void odb_transaction_files_commit(struct odb_transaction *base)
+static int odb_transaction_files_commit(struct odb_transaction *base)
{
struct odb_transaction_files *transaction =
container_of(base, struct odb_transaction_files, base);
@@ -1668,14 +1672,19 @@ static void odb_transaction_files_commit(struct odb_transaction *base)
* Make the object files visible in the primary ODB after their data is
* fully durable.
*/
- tmp_objdir_migrate(transaction->objdir);
+ if (tmp_objdir_migrate(transaction->objdir))
+ return -1;
+
transaction->objdir = NULL;
}
flush_packfile_transaction(transaction);
+
+ return 0;
}
-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
+int odb_transaction_files_begin(struct odb_source *source,
+ struct odb_transaction **out)
{
struct odb_transaction_files *transaction;
@@ -1683,6 +1692,7 @@ struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
transaction->base.source = source;
transaction->base.commit = odb_transaction_files_commit;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+ *out = &transaction->base;
- return &transaction->base;
+ return 0;
}
diff --git a/object-file.h b/object-file.h
index ea43d818f0..1a023226ac 100644
--- a/object-file.h
+++ b/object-file.h
@@ -196,6 +196,7 @@ struct odb_transaction;
* multiple objects. odb_transaction_files_commit must be called
* to make new objects visible.
*/
-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);
+int odb_transaction_files_begin(struct odb_source *source,
+ struct odb_transaction **out);
#endif /* OBJECT_FILE_H */
diff --git a/odb/source-files.c b/odb/source-files.c
index 5bdd042922..2545bd81d4 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -182,11 +182,7 @@ static int odb_source_files_write_object_stream(struct odb_source *source,
static int odb_source_files_begin_transaction(struct odb_source *source,
struct odb_transaction **out)
{
- struct odb_transaction *tx = odb_transaction_files_begin(source);
- if (!tx)
- return -1;
- *out = tx;
- return 0;
+ return odb_transaction_files_begin(source, out);
}
static int odb_source_files_read_alternates(struct odb_source *source,
diff --git a/odb/transaction.h b/odb/transaction.h
index 854fda06f5..d52f0533ce 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -16,8 +16,11 @@ struct odb_transaction {
/* The ODB source the transaction is opened against. */
struct odb_source *source;
- /* The ODB source specific callback invoked to commit a transaction. */
- void (*commit)(struct odb_transaction *transaction);
+ /*
+ * The ODB source specific callback invoked to commit a transaction.
+ * Returns 0 on success, a negative error code otherwise.
+ */
+ int (*commit)(struct odb_transaction *transaction);
/*
* This callback is expected to write the given object stream into
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 06/11] odb/transaction: propagate begin errors
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
When `odb_transaction_begin()` is invoked, the function returns the
transaction pointer directly. There is no way for the backend to
signal that it failed to set up its state, such as when creating the
temporary object directory backing the transaction.
In a subsequent commit, git-receive-pack(1) starts using ODB
transactions and needs to be able to report such failures rather
than silently ignore them. Refactor `odb_transaction_begin()` to
return an int error code and write the resulting transaction into an
out parameter. Also introduce `odb_transaction_begin_or_die()` as a
convenience for callsites that do not need to handle errors
explicitly.
Note that `odb_transaction_begin()` now returns an error when the ODB
already has an inflight transaction pending. ODB transaction call sites
that may encounter an inflight transaction are updated to explicitly
handle this case.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/add.c | 2 +-
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 2 +-
cache-tree.c | 7 +++++--
object-file.c | 10 +++++++---
odb/transaction.c | 12 ++++++++----
odb/transaction.h | 20 ++++++++++++++++----
read-cache.c | 7 +++++--
8 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index c859f66519..3d5d9cfdb9 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -581,7 +581,7 @@ int cmd_add(int argc,
string_list_clear(&only_match_skip_worktree, 0);
}
- transaction = odb_transaction_begin(repo->objects);
+ odb_transaction_begin_or_die(repo->objects, &transaction);
ps_matched = xcalloc(pathspec.nr, 1);
if (add_renormalize)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index f3849bb654..d0136cdd99 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -598,7 +598,7 @@ static void unpack_all(void)
progress = start_progress(the_repository,
_("Unpacking objects"), nr_objects);
CALLOC_ARRAY(obj_list, nr_objects);
- transaction = odb_transaction_begin(the_repository->objects);
+ odb_transaction_begin_or_die(the_repository->objects, &transaction);
for (i = 0; i < nr_objects; i++) {
unpack_one(i);
display_progress(progress, i + 1);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3d6646c318..17f3ea284c 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1124,7 +1124,7 @@ int cmd_update_index(int argc,
* Allow the object layer to optimize adding multiple objects in
* a batch.
*/
- transaction = odb_transaction_begin(the_repository->objects);
+ odb_transaction_begin_or_die(the_repository->objects, &transaction);
while (ctx.argc) {
if (parseopt_state != PARSE_OPT_DONE)
parseopt_state = parse_options_step(&ctx, options,
diff --git a/cache-tree.c b/cache-tree.c
index 184f7e2635..8eec1d4d52 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -474,6 +474,7 @@ static int update_one(struct cache_tree *it,
int cache_tree_update(struct index_state *istate, int flags)
{
+ int inflight = !!the_repository->objects->transaction;
struct odb_transaction *transaction;
int skip, i;
@@ -490,10 +491,12 @@ int cache_tree_update(struct index_state *istate, int flags)
trace_performance_enter();
trace2_region_enter("cache_tree", "update", istate->repo);
- transaction = odb_transaction_begin(the_repository->objects);
+ if (!inflight)
+ odb_transaction_begin_or_die(the_repository->objects, &transaction);
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
- odb_transaction_commit(transaction);
+ if (!inflight)
+ odb_transaction_commit(transaction);
trace2_region_leave("cache_tree", "update", istate->repo);
trace_performance_leave("cache_tree_update");
if (i < 0)
diff --git a/object-file.c b/object-file.c
index 64cb874fe7..cd1aa36462 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1354,13 +1354,17 @@ int index_fd(struct index_state *istate, struct object_id *oid,
if (flags & INDEX_WRITE_OBJECT) {
struct object_database *odb = the_repository->objects;
- struct odb_transaction *transaction = odb_transaction_begin(odb);
+ struct odb_transaction *transaction = odb->transaction;
+ int inflight = !!transaction;
- ret = odb_transaction_write_object_stream(odb->transaction,
+ if (!inflight)
+ odb_transaction_begin_or_die(odb, &transaction);
+ ret = odb_transaction_write_object_stream(transaction,
&stream,
xsize_t(st->st_size),
oid);
- odb_transaction_commit(transaction);
+ if (!inflight)
+ odb_transaction_commit(transaction);
} else {
ret = hash_blob_stream(&stream,
the_repository->hash_algo, oid,
diff --git a/odb/transaction.c b/odb/transaction.c
index b16e07aebf..df4275151b 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -2,14 +2,18 @@
#include "odb/source.h"
#include "odb/transaction.h"
-struct odb_transaction *odb_transaction_begin(struct object_database *odb)
+int odb_transaction_begin(struct object_database *odb,
+ struct odb_transaction **out)
{
+ int ret;
+
if (odb->transaction)
- return NULL;
+ return -1;
- odb_source_begin_transaction(odb->sources, &odb->transaction);
+ ret = odb_source_begin_transaction(odb->sources, out);
+ odb->transaction = *out;
- return odb->transaction;
+ return ret;
}
void odb_transaction_commit(struct odb_transaction *transaction)
diff --git a/odb/transaction.h b/odb/transaction.h
index d52f0533ce..36032a5365 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -1,6 +1,8 @@
#ifndef ODB_TRANSACTION_H
#define ODB_TRANSACTION_H
+#include "git-compat-util.h"
+#include "gettext.h"
#include "odb.h"
#include "odb/source.h"
@@ -36,11 +38,21 @@ struct odb_transaction {
};
/*
- * Starts an ODB transaction. Subsequent objects are written to the transaction
- * and not committed until odb_transaction_commit() is invoked on the
- * transaction. If the ODB already has a pending transaction, NULL is returned.
+ * Starts an ODB transaction and returns it via `out`. Subsequent objects are
+ * written to the transaction and not committed until odb_transaction_commit()
+ * is invoked on the transaction. Returns 0 on success and a negative value on
+ * error. Note that it is considered an error to start a new transaction if the
+ * ODB already has an inflight transaction pending.
*/
-struct odb_transaction *odb_transaction_begin(struct object_database *odb);
+int odb_transaction_begin(struct object_database *odb,
+ struct odb_transaction **out);
+
+static inline void odb_transaction_begin_or_die(struct object_database *odb,
+ struct odb_transaction **out)
+{
+ if (odb_transaction_begin(odb, out))
+ die(_("failed to start ODB transaction"));
+}
/*
* Commits an ODB transaction making the written objects visible. If the
diff --git a/read-cache.c b/read-cache.c
index 21ca58beea..d511d25834 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4012,6 +4012,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
const struct pathspec *pathspec, char *ps_matched,
int include_sparse, int flags, int ignored_too )
{
+ int inflight = !!repo->objects->transaction;
struct odb_transaction *transaction;
struct update_callback_data data;
struct rev_info rev;
@@ -4042,9 +4043,11 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
* This function is invoked from commands other than 'add', which
* may not have their own transaction active.
*/
- transaction = odb_transaction_begin(repo->objects);
+ if (!inflight)
+ odb_transaction_begin_or_die(repo->objects, &transaction);
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
- odb_transaction_commit(transaction);
+ if (!inflight)
+ odb_transaction_commit(transaction);
release_revisions(&rev);
return !!data.add_errors;
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 07/11] odb/transaction: propagate commit errors
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
When `odb_transaction_commit()` is invoked, the return value of the
backend commit callback is silently discarded. A backend has no way
to signal that committing failed, such as when the "files" backend
cannot migrate its temporary object directory into the permanent
ODB.
In a subsequent commit, git-receive-pack(1) starts using ODB transaction
to stage objects and consequently cares about such failures so it can
handle the error appropriately. Change the commit callback signature to
return an int error code and have `odb_transaction_commit()` forward it
accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
odb/transaction.c | 13 ++++++++++---
odb/transaction.h | 7 ++++---
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/odb/transaction.c b/odb/transaction.c
index df4275151b..51af2c9a61 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -16,19 +16,26 @@ int odb_transaction_begin(struct object_database *odb,
return ret;
}
-void odb_transaction_commit(struct odb_transaction *transaction)
+int odb_transaction_commit(struct odb_transaction *transaction)
{
+ int ret;
+
if (!transaction)
- return;
+ return 0;
/*
* Ensure the transaction ending matches the pending transaction.
*/
ASSERT(transaction == transaction->source->odb->transaction);
- transaction->commit(transaction);
+ ret = transaction->commit(transaction);
+ if (ret)
+ return ret;
+
transaction->source->odb->transaction = NULL;
free(transaction);
+
+ return 0;
}
int odb_transaction_write_object_stream(struct odb_transaction *transaction,
diff --git a/odb/transaction.h b/odb/transaction.h
index 36032a5365..9557ee0fd2 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -55,10 +55,11 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
}
/*
- * Commits an ODB transaction making the written objects visible. If the
- * specified transaction is NULL, the function is a no-op.
+ * Commits an ODB transaction making the written objects visible. Returns 0 on
+ * success, a negative error code otherwise. Note that, if the specified
+ * transaction is NULL, the function is a no-op and no error is returned.
*/
-void odb_transaction_commit(struct odb_transaction *transaction);
+int odb_transaction_commit(struct odb_transaction *transaction);
/*
* Writes the object in the provided stream into the transaction. The resulting
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 08/11] odb/transaction: add transaction env interface
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
The ODB transaction backend is responsible for creating/managing its own
staging area for writing objects. Other child processes spawned by Git
may need access to uncommitted objects or write new objects in the
staging area though.
Introduce `odb_transaction_env()` which is expected to provide the set
of environment variables needed by a child process to access the
transaction's staging area.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 14 ++++++++++++++
odb/transaction.c | 8 ++++++++
odb/transaction.h | 17 +++++++++++++++++
3 files changed, 39 insertions(+)
diff --git a/object-file.c b/object-file.c
index cd1aa36462..9b8ee6f36c 100644
--- a/object-file.c
+++ b/object-file.c
@@ -27,6 +27,7 @@
#include "path.h"
#include "read-cache-ll.h"
#include "setup.h"
+#include "strvec.h"
#include "tempfile.h"
#include "tmp-objdir.h"
@@ -1687,6 +1688,18 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
return 0;
}
+static int odb_transaction_files_env(struct odb_transaction *base,
+ struct strvec *env)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+
+ odb_transaction_files_prepare(&transaction->base);
+ strvec_pushv(env, tmp_objdir_env(transaction->objdir));
+
+ return 0;
+}
+
int odb_transaction_files_begin(struct odb_source *source,
struct odb_transaction **out)
{
@@ -1696,6 +1709,7 @@ int odb_transaction_files_begin(struct odb_source *source,
transaction->base.source = source;
transaction->base.commit = odb_transaction_files_commit;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+ transaction->base.env = odb_transaction_files_env;
*out = &transaction->base;
return 0;
diff --git a/odb/transaction.c b/odb/transaction.c
index 51af2c9a61..acb1c967e7 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -44,3 +44,11 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
{
return transaction->write_object_stream(transaction, stream, len, oid);
}
+
+int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env)
+{
+ if (!transaction)
+ return 0;
+
+ return transaction->env(transaction, env);
+}
diff --git a/odb/transaction.h b/odb/transaction.h
index 9557ee0fd2..1c6c97a53e 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -35,6 +35,14 @@ struct odb_transaction {
int (*write_object_stream)(struct odb_transaction *transaction,
struct odb_write_stream *stream, size_t len,
struct object_id *oid);
+
+ /*
+ * This callback is expected to populate the provided strvec with the
+ * environment variables that a child process should inherit so that its
+ * object writes participate in the transaction. Returns 0 on success, a
+ * negative error code otherwise.
+ */
+ int (*env)(struct odb_transaction *transaction, struct strvec *env);
};
/*
@@ -70,4 +78,13 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
struct odb_write_stream *stream,
size_t len, struct object_id *oid);
+/*
+ * Populates the provided strvec with the environment variables that a child
+ * process should inherit so that its object writes participate in the
+ * transaction, suitable for using via child_process.env. Returns 0 on success,
+ * a negative error code otherwise. Note that, if the specified transaction is
+ * NULL, the function is a no-op and no error is returned.
+ */
+int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env);
+
#endif
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 09/11] odb/transaction: introduce ODB transaction flags
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
The temporary directory used by git-receive-pack(1) to write objects is
managed slightly differently than how it is done via ODB transactions:
- The temporary directory is eagerly created upfront, instead of
waiting for the first object write.
- The prefix name of the temporary directory is "incoming" instead of
"bulk-fsync".
In a subsequent commit, git-receive-pack(1) will use ODB transactions
instead of `tmp_objdir` directly. To provide a means to configure the
same transaction behavior, introduce `enum odb_transaction_flags` and
the ODB_TRANSACTION_RECEIVE flag intended as a signal for ODB
transactions using the "files" backend to be set up for
git-receive-pack(1). Transaction call sites are updated accordingly to
provide the required flag parameter.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/add.c | 2 +-
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 2 +-
cache-tree.c | 2 +-
object-file.c | 29 ++++++++++++++++++++++++++---
object-file.h | 4 +++-
odb/source-files.c | 5 +++--
odb/source-inmemory.c | 3 ++-
odb/source-loose.c | 3 ++-
odb/source.h | 9 ++++++---
odb/transaction.c | 5 +++--
odb/transaction.h | 15 +++++++++++----
read-cache.c | 2 +-
13 files changed, 61 insertions(+), 22 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 3d5d9cfdb9..60ffbede2b 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -581,7 +581,7 @@ int cmd_add(int argc,
string_list_clear(&only_match_skip_worktree, 0);
}
- odb_transaction_begin_or_die(repo->objects, &transaction);
+ odb_transaction_begin_or_die(repo->objects, &transaction, 0);
ps_matched = xcalloc(pathspec.nr, 1);
if (add_renormalize)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index d0136cdd99..c3d0fc7507 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -598,7 +598,7 @@ static void unpack_all(void)
progress = start_progress(the_repository,
_("Unpacking objects"), nr_objects);
CALLOC_ARRAY(obj_list, nr_objects);
- odb_transaction_begin_or_die(the_repository->objects, &transaction);
+ odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
for (i = 0; i < nr_objects; i++) {
unpack_one(i);
display_progress(progress, i + 1);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 17f3ea284c..bf6ea60ef4 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1124,7 +1124,7 @@ int cmd_update_index(int argc,
* Allow the object layer to optimize adding multiple objects in
* a batch.
*/
- odb_transaction_begin_or_die(the_repository->objects, &transaction);
+ odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
while (ctx.argc) {
if (parseopt_state != PARSE_OPT_DONE)
parseopt_state = parse_options_step(&ctx, options,
diff --git a/cache-tree.c b/cache-tree.c
index 8eec1d4d52..99c6a0a7d0 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -492,7 +492,7 @@ int cache_tree_update(struct index_state *istate, int flags)
trace_performance_enter();
trace2_region_enter("cache_tree", "update", istate->repo);
if (!inflight)
- odb_transaction_begin_or_die(the_repository->objects, &transaction);
+ odb_transaction_begin_or_die(the_repository->objects, &transaction, 0);
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
if (!inflight)
diff --git a/object-file.c b/object-file.c
index 9b8ee6f36c..d95bdabba5 100644
--- a/object-file.c
+++ b/object-file.c
@@ -498,6 +498,7 @@ struct odb_transaction_files {
struct tmp_objdir *objdir;
struct transaction_packfile packfile;
+ const char *prefix;
};
static int odb_transaction_files_prepare(struct odb_transaction *base)
@@ -514,7 +515,7 @@ static int odb_transaction_files_prepare(struct odb_transaction *base)
if (!transaction || transaction->objdir)
return 0;
- transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
+ transaction->objdir = tmp_objdir_create(base->source->odb->repo, transaction->prefix);
if (!transaction->objdir)
return -1;
@@ -1359,7 +1360,7 @@ int index_fd(struct index_state *istate, struct object_id *oid,
int inflight = !!transaction;
if (!inflight)
- odb_transaction_begin_or_die(odb, &transaction);
+ odb_transaction_begin_or_die(odb, &transaction, 0);
ret = odb_transaction_write_object_stream(transaction,
&stream,
xsize_t(st->st_size),
@@ -1701,7 +1702,8 @@ static int odb_transaction_files_env(struct odb_transaction *base,
}
int odb_transaction_files_begin(struct odb_source *source,
- struct odb_transaction **out)
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags)
{
struct odb_transaction_files *transaction;
@@ -1710,6 +1712,27 @@ int odb_transaction_files_begin(struct odb_source *source,
transaction->base.commit = odb_transaction_files_commit;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
transaction->base.env = odb_transaction_files_env;
+
+ transaction->prefix = "bulk-fsync";
+ if (flags & ODB_TRANSACTION_RECEIVE) {
+ /*
+ * ODB transactions for git-receive-pack(1) eagerly create a
+ * temporary directory and use a different temporary directory
+ * prefix.
+ *
+ * NEEDSWORK: This transaction flag is only used by the "files"
+ * backend to special case temporary directory set up and
+ * handling. Ideally transaction users should not have to care
+ * though. To avoid this, we could eagerly create the temporary
+ * directory and use the same prefix name for all transactions.
+ */
+ transaction->prefix = "incoming";
+ if (odb_transaction_files_prepare(&transaction->base)) {
+ free(transaction);
+ return -1;
+ }
+ }
+
*out = &transaction->base;
return 0;
diff --git a/object-file.h b/object-file.h
index 1a023226ac..bdd2d67a2e 100644
--- a/object-file.h
+++ b/object-file.h
@@ -5,6 +5,7 @@
#include "object.h"
#include "odb.h"
#include "odb/source-loose.h"
+#include "odb/transaction.h"
/* The maximum size for an object header. */
#define MAX_HEADER_LEN 32
@@ -197,6 +198,7 @@ struct odb_transaction;
* to make new objects visible.
*/
int odb_transaction_files_begin(struct odb_source *source,
- struct odb_transaction **out);
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags);
#endif /* OBJECT_FILE_H */
diff --git a/odb/source-files.c b/odb/source-files.c
index 2545bd81d4..534f48aad9 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -180,9 +180,10 @@ static int odb_source_files_write_object_stream(struct odb_source *source,
}
static int odb_source_files_begin_transaction(struct odb_source *source,
- struct odb_transaction **out)
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags)
{
- return odb_transaction_files_begin(source, out);
+ return odb_transaction_files_begin(source, out, flags);
}
static int odb_source_files_read_alternates(struct odb_source *source,
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index e004566d76..9644d9d474 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -304,7 +304,8 @@ static int odb_source_inmemory_freshen_object(struct odb_source *source,
}
static int odb_source_inmemory_begin_transaction(struct odb_source *source UNUSED,
- struct odb_transaction **out UNUSED)
+ struct odb_transaction **out UNUSED,
+ enum odb_transaction_flags flags UNUSED)
{
return error("in-memory source does not support transactions");
}
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 66e6bb8d3f..57c91986b4 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -638,7 +638,8 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
}
static int odb_source_loose_begin_transaction(struct odb_source *source UNUSED,
- struct odb_transaction **out UNUSED)
+ struct odb_transaction **out UNUSED,
+ enum odb_transaction_flags flags UNUSED)
{
/* TODO: this is a known omission that we'll want to address eventually. */
return error("loose source does not support transactions");
diff --git a/odb/source.h b/odb/source.h
index 2192a101b8..3790d03ff2 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -3,6 +3,7 @@
#include "object.h"
#include "odb.h"
+#include "odb/transaction.h"
enum odb_source_type {
/*
@@ -228,7 +229,8 @@ struct odb_source {
* negative error code otherwise.
*/
int (*begin_transaction)(struct odb_source *source,
- struct odb_transaction **out);
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags);
/*
* This callback is expected to read the list of alternate object
@@ -467,9 +469,10 @@ static inline int odb_source_write_alternate(struct odb_source *source,
* Returns 0 on success, a negative error code otherwise.
*/
static inline int odb_source_begin_transaction(struct odb_source *source,
- struct odb_transaction **out)
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags)
{
- return source->begin_transaction(source, out);
+ return source->begin_transaction(source, out, flags);
}
#endif
diff --git a/odb/transaction.c b/odb/transaction.c
index acb1c967e7..007ab73c0c 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -3,14 +3,15 @@
#include "odb/transaction.h"
int odb_transaction_begin(struct object_database *odb,
- struct odb_transaction **out)
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags)
{
int ret;
if (odb->transaction)
return -1;
- ret = odb_source_begin_transaction(odb->sources, out);
+ ret = odb_source_begin_transaction(odb->sources, out, flags);
odb->transaction = *out;
return ret;
diff --git a/odb/transaction.h b/odb/transaction.h
index 1c6c97a53e..b19f180aee 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -4,7 +4,6 @@
#include "git-compat-util.h"
#include "gettext.h"
#include "odb.h"
-#include "odb/source.h"
/*
* A transaction may be started for an object database prior to writing new
@@ -45,6 +44,12 @@ struct odb_transaction {
int (*env)(struct odb_transaction *transaction, struct strvec *env);
};
+/* Flags used to configure an ODB transaction. */
+enum odb_transaction_flags {
+ /* Configures the transaction for use with git-receive-pack(1). */
+ ODB_TRANSACTION_RECEIVE = (1 << 0),
+};
+
/*
* Starts an ODB transaction and returns it via `out`. Subsequent objects are
* written to the transaction and not committed until odb_transaction_commit()
@@ -53,12 +58,14 @@ struct odb_transaction {
* ODB already has an inflight transaction pending.
*/
int odb_transaction_begin(struct object_database *odb,
- struct odb_transaction **out);
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags);
static inline void odb_transaction_begin_or_die(struct object_database *odb,
- struct odb_transaction **out)
+ struct odb_transaction **out,
+ enum odb_transaction_flags flags)
{
- if (odb_transaction_begin(odb, out))
+ if (odb_transaction_begin(odb, out, flags))
die(_("failed to start ODB transaction"));
}
diff --git a/read-cache.c b/read-cache.c
index d511d25834..50e2320c8d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4044,7 +4044,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
* may not have their own transaction active.
*/
if (!inflight)
- odb_transaction_begin_or_die(repo->objects, &transaction);
+ odb_transaction_begin_or_die(repo->objects, &transaction, 0);
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
if (!inflight)
odb_transaction_commit(transaction);
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 10/11] builtin/receive-pack: drop redundant tmpdir env
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
When performing the connectivity checks for a shallow ref in
`update_shallow_ref()`, the child process environment variables are
populated via `tmp_objdir_env()`. This is unnecessary though as
`update_shallow_ref()` is only reached after `tmp_objdir_migrate()` has
been performed which means there is no longer a temporary directory that
needs to be shared with child processes.
Drop the call to `tmp_objdir_env()` accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 19eb6a1b61..50bc05c70c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1363,7 +1363,6 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
!delayed_reachability_test(si, i))
oid_array_append(&extra, &si->shallow->oid[i]);
- opt.env = tmp_objdir_env(tmp_objdir);
setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
if (check_connected(command_singleton_iterator, cmd, &opt)) {
rollback_shallow_file(the_repository, &shallow_lock);
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
* [PATCH v2 11/11] builtin/receive-pack: stage incoming objects via ODB transactions
From: Justin Tobler @ 2026-07-08 4:14 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In-Reply-To: <20260708041412.1157499-1-jltobler@gmail.com>
Objects received by git-receive-pack(1) are quarantined in a temporary
"incoming" directory and migrated into the object database prior to the
reference updates. The quarantine is currently managed through
`tmp_objdir` directly. In a pluggable ODB future, how exactly an object
gets written to a transaction may vary for a given ODB source. Refactor
git-receive-pack(1) to use the ODB transaction interfaces to manage the
object staging area in a more agnostic manner accordingly.
Note that the ODB transaction is now responsible for managing the
primary and alternate ODBs for the repository. One small change as a
result is that the temporary directory is now applied as the primary ODB
in the main process instead of an alternate. This does not change
anything for git-receive-pack(1) though because it only needs access to
the newly written objects and doesn't care how exactly it is set up.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 68 ++++++++++++++++++++++--------------------
1 file changed, 35 insertions(+), 33 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 50bc05c70c..8b8c20dc1a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -37,7 +37,6 @@
#include "sigchain.h"
#include "string-list.h"
#include "strvec.h"
-#include "tmp-objdir.h"
#include "trace.h"
#include "trace2.h"
#include "version.h"
@@ -112,8 +111,6 @@ static enum {
} use_keepalive;
static int keepalive_in_sec = 5;
-static struct tmp_objdir *tmp_objdir;
-
static struct proc_receive_ref {
unsigned int want_add:1,
want_delete:1,
@@ -926,6 +923,7 @@ static void receive_hook_feed_state_free(void *data)
static int run_receive_hook(struct command *commands,
const char *hook_name,
int skip_broken,
+ struct odb_transaction *transaction,
const struct string_list *push_options)
{
struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
@@ -959,8 +957,8 @@ static int run_receive_hook(struct command *commands,
strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT");
}
- if (tmp_objdir)
- strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir));
+ if (transaction)
+ odb_transaction_env(transaction, &opt.env);
prepare_push_cert_sha1(&opt);
@@ -1789,24 +1787,30 @@ static const struct object_id *command_singleton_iterator(void *cb_data)
}
static void set_connectivity_errors(struct command *commands,
- struct shallow_info *si)
+ struct shallow_info *si,
+ struct odb_transaction *transaction)
{
struct command *cmd;
for (cmd = commands; cmd; cmd = cmd->next) {
struct command *singleton = cmd;
struct check_connected_options opt = CHECK_CONNECTED_INIT;
+ struct strvec env = STRVEC_INIT;
if (shallow_update && si->shallow_ref[cmd->index])
/* to be checked in update_shallow_ref() */
continue;
- opt.env = tmp_objdir_env(tmp_objdir);
+ odb_transaction_env(transaction, &env);
+ opt.env = env.v;
+
if (!check_connected(command_singleton_iterator, &singleton,
&opt))
continue;
cmd->error_string = "missing necessary objects";
+
+ strvec_clear(&env);
}
}
@@ -2027,6 +2031,7 @@ static void execute_commands_atomic(struct command *commands,
static void execute_commands(struct command *commands,
const char *unpacker_error,
struct shallow_info *si,
+ struct odb_transaction *transaction,
const struct string_list *push_options)
{
struct check_connected_options opt = CHECK_CONNECTED_INIT;
@@ -2043,6 +2048,8 @@ static void execute_commands(struct command *commands,
}
if (!skip_connectivity_check) {
+ struct strvec env = STRVEC_INIT;
+
if (use_sideband) {
memset(&muxer, 0, sizeof(muxer));
muxer.proc = copy_to_sideband;
@@ -2056,14 +2063,17 @@ static void execute_commands(struct command *commands,
data.si = si;
opt.err_fd = err_fd;
opt.progress = err_fd && !quiet;
- opt.env = tmp_objdir_env(tmp_objdir);
+ odb_transaction_env(transaction, &env);
+ opt.env = env.v;
opt.exclude_hidden_refs_section = "receive";
if (check_connected(iterate_receive_command_list, &data, &opt))
- set_connectivity_errors(commands, si);
+ set_connectivity_errors(commands, si, transaction);
if (use_sideband)
finish_async(&muxer);
+
+ strvec_clear(&env);
}
reject_updates_to_hidden(commands);
@@ -2084,7 +2094,7 @@ static void execute_commands(struct command *commands,
}
}
- if (run_receive_hook(commands, "pre-receive", 0, push_options)) {
+ if (run_receive_hook(commands, "pre-receive", 0, transaction, push_options)) {
for (cmd = commands; cmd; cmd = cmd->next) {
if (!cmd->error_string)
cmd->error_string = "pre-receive hook declined";
@@ -2105,14 +2115,13 @@ static void execute_commands(struct command *commands,
* Now we'll start writing out refs, which means the objects need
* to be in their final positions so that other processes can see them.
*/
- if (tmp_objdir_migrate(tmp_objdir) < 0) {
+ if (odb_transaction_commit(transaction)) {
for (cmd = commands; cmd; cmd = cmd->next) {
if (!cmd->error_string)
cmd->error_string = "unable to migrate objects to permanent storage";
}
return;
}
- tmp_objdir = NULL;
check_aliased_updates(commands);
@@ -2325,7 +2334,8 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
-static const char *unpack(int err_fd, struct shallow_info *si)
+static const char *unpack(int err_fd, struct shallow_info *si,
+ struct odb_transaction *transaction)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2350,20 +2360,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)
strvec_push(&child.args, alt_shallow_file);
}
- tmp_objdir = tmp_objdir_create(the_repository, "incoming");
- if (!tmp_objdir) {
- if (err_fd > 0)
- close(err_fd);
- return "unable to create temporary object directory";
- }
- strvec_pushv(&child.env, tmp_objdir_env(tmp_objdir));
-
- /*
- * Normally we just pass the tmp_objdir environment to the child
- * processes that do the heavy lifting, but we may need to see these
- * objects ourselves to set up shallow information.
- */
- tmp_objdir_add_as_alternate(tmp_objdir);
+ odb_transaction_env(transaction, &child.env);
if (ntohl(hdr.hdr_entries) < unpack_limit) {
strvec_push(&child.args, "unpack-objects");
@@ -2430,13 +2427,14 @@ static const char *unpack(int err_fd, struct shallow_info *si)
return NULL;
}
-static const char *unpack_with_sideband(struct shallow_info *si)
+static const char *unpack_with_sideband(struct shallow_info *si,
+ struct odb_transaction *transaction)
{
struct async muxer;
const char *ret;
if (!use_sideband)
- return unpack(0, si);
+ return unpack(0, si, transaction);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2445,7 +2443,7 @@ static const char *unpack_with_sideband(struct shallow_info *si)
if (start_async(&muxer))
return NULL;
- ret = unpack(muxer.in, si);
+ ret = unpack(muxer.in, si, transaction);
finish_async(&muxer);
return ret;
@@ -2622,6 +2620,7 @@ int cmd_receive_pack(int argc,
struct oid_array ref = OID_ARRAY_INIT;
struct shallow_info si;
struct packet_reader reader;
+ struct odb_transaction *transaction = NULL;
struct option options[] = {
OPT__QUIET(&quiet, N_("quiet")),
@@ -2706,11 +2705,14 @@ int cmd_receive_pack(int argc,
if (!si.nr_ours && !si.nr_theirs)
shallow_update = 0;
if (!delete_only(commands)) {
- unpack_status = unpack_with_sideband(&si);
+ if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
+ unpack_status = "unable to start object transaction";
+ else
+ unpack_status = unpack_with_sideband(&si, transaction);
update_shallow_info(commands, &si, &ref);
}
use_keepalive = KEEPALIVE_ALWAYS;
- execute_commands(commands, unpack_status, &si,
+ execute_commands(commands, unpack_status, &si, transaction,
&push_options);
delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
@@ -2719,7 +2721,7 @@ int cmd_receive_pack(int argc,
else if (report_status)
report(commands, unpack_status);
sigchain_pop(SIGPIPE);
- run_receive_hook(commands, "post-receive", 1,
+ run_receive_hook(commands, "post-receive", 1, NULL,
&push_options);
run_update_post_hook(commands);
free_commands(commands);
--
2.55.0.122.gf85a7e6620
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox