{"thread":{"id":"64882","subject":"[PATCH 0/4] odb: support ODB source specific transaction handling","startedAt":"2026-01-28T23:45:29Z","lastAt":"2026-02-05T11:22:23Z","messageCount":32,"participants":["Justin Tobler","Patrick Steinhardt","Junio C Hamano","Toon Claes","Karthik Nayak"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"534788","messageId":"20260128234519.2721179-1-jltobler@gmail.com","threadId":"64882","inReplyTo":null,"subject":"[PATCH 0/4] odb: support ODB source specific transaction handling","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-28T23:45:15Z","receivedAt":"2026-01-28T23:45:29Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Greetings,\n\nODB transactions provide a mechanism for writing new objects in the\nrepository that are in a pending state until they are committed. With\npluggable ODBs, each source will likely need to handle transactions\ndifferently. This patch series prepares ODB transaction handling to be\ndefined for each ODB source by creating a generic odb_transaction\nstructure that stores a callback for committing the transaction.\n\nFor now, `struct odb_transaction_loose` is the only transaction\nimplementation and what is always returned when creating a transaction.\nAdditionally, this series lifts transaction logic that is common to all\nbackends out of the implementation layer into\n`odb_transaction_{begin,commit}()` layer.\n\nThanks,\n-Justin\n\nJustin Tobler (4):\n odb: store ODB source in `struct odb_transaction`\n object-file: rename transaction functions\n odb: prepare `struct odb_transaction` to support more sources\n odb: transparently handle common transaction behavior\n\n object-file.c | 77 +++++++++++++++++++++++++--------------------------\n object-file.h | 10 ++-----\n odb.c | 22 +++++++++++++--\n odb.h | 17 ++++++++++++\n 4 files changed, 77 insertions(+), 49 deletions(-)\n\n\nbase-commit: ea717645d199f6f1b66058886475db3e8c9330e9\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"534789","messageId":"20260128234519.2721179-2-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260128234519.2721179-1-jltobler@gmail.com","subject":"[PATCH 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-28T23:45:16Z","receivedAt":"2026-01-28T23:45:29Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Each `struct odb_transaction` currently stores a reference to the\n`struct object_database`. Since transactions are handled per object\nsource, instead store a reference to the source.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex e7e4c3348f..196509b252 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -711,7 +711,7 @@ struct transaction_packfile {\n };\n \n struct odb_transaction {\n-\tstruct object_database *odb;\n+\tstruct odb_source *source;\n \n \tstruct tmp_objdir *objdir;\n \tstruct transaction_packfile packfile;\n@@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n \tif (!transaction || transaction->objdir)\n \t\treturn;\n \n-\ttransaction->objdir = tmp_objdir_create(transaction->odb->repo, \"bulk-fsync\");\n+\ttransaction->objdir = tmp_objdir_create(transaction->source->odb->repo, \"bulk-fsync\");\n \tif (transaction->objdir)\n \t\ttmp_objdir_replace_primary_odb(transaction->objdir, 0);\n }\n@@ -772,7 +772,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)\n \t * the final name is visible.\n \t */\n \tstrbuf_addf(&temp_path, \"%s/bulk_fsync_XXXXXX\",\n-\t\t repo_get_object_directory(transaction->odb->repo));\n+\t\t repo_get_object_directory(transaction->source->odb->repo));\n \ttemp = xmks_tempfile(temp_path.buf);\n \tfsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));\n \tdelete_tempfile(&temp);\n@@ -1344,7 +1344,7 @@ static int already_written(struct odb_transaction *transaction,\n \t\t\t struct object_id *oid)\n {\n \t/* The object may already exist in the repository */\n-\tif (odb_has_object(transaction->odb, oid,\n+\tif (odb_has_object(transaction->source->odb, oid,\n \t\t\t HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))\n \t\treturn 1;\n \n@@ -1365,7 +1365,7 @@ static void prepare_packfile_transaction(struct odb_transaction *transaction,\n \tif (!(flags & INDEX_WRITE_OBJECT) || state->f)\n \t\treturn;\n \n-\tstate->f = create_tmp_packfile(transaction->odb->repo,\n+\tstate->f = create_tmp_packfile(transaction->source->odb->repo,\n \t\t\t\t &state->pack_tmp_name);\n \treset_pack_idx_option(&state->pack_idx_opts);\n \n@@ -1469,7 +1469,7 @@ static int stream_blob_to_pack(struct transaction_packfile *state,\n static void flush_packfile_transaction(struct odb_transaction *transaction)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n-\tstruct repository *repo = transaction->odb->repo;\n+\tstruct repository *repo = transaction->source->odb->repo;\n \tunsigned char hash[GIT_MAX_RAWSZ];\n \tstruct strbuf packname = STRBUF_INIT;\n \tchar *idx_tmp_name = NULL;\n@@ -1494,7 +1494,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)\n \t}\n \n \tstrbuf_addf(&packname, \"%s/pack/pack-%s.\",\n-\t\t repo_get_object_directory(transaction->odb->repo),\n+\t\t repo_get_object_directory(transaction->source->odb->repo),\n \t\t hash_to_hex_algop(hash, repo->hash_algo));\n \n \tstage_tmp_packfiles(repo, &packname, state->pack_tmp_name,\n@@ -1553,7 +1553,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n \n \theader_len = format_object_header((char *)obuf, sizeof(obuf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\ttransaction->odb->repo->hash_algo->init_fn(&ctx);\n+\ttransaction->source->odb->repo->hash_algo->init_fn(&ctx);\n \tgit_hash_update(&ctx, obuf, header_len);\n \n \t/* Note: idx is non-NULL when we are writing */\n@@ -1993,7 +1993,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)\n \t\treturn NULL;\n \n \tCALLOC_ARRAY(odb->transaction, 1);\n-\todb->transaction->odb = odb;\n+\todb->transaction->source = source;\n \n \treturn odb->transaction;\n }\n@@ -2006,11 +2006,11 @@ void object_file_transaction_commit(struct odb_transaction *transaction)\n \t/*\n \t * Ensure the transaction ending matches the pending transaction.\n \t */\n-\tASSERT(transaction == transaction->odb->transaction);\n+\tASSERT(transaction == transaction->source->odb->transaction);\n \n \tflush_loose_object_transaction(transaction);\n \tflush_packfile_transaction(transaction);\n-\ttransaction->odb->transaction = NULL;\n+\ttransaction->source->odb->transaction = NULL;\n \tfree(transaction);\n }\n \n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"534790","messageId":"20260128234519.2721179-3-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260128234519.2721179-1-jltobler@gmail.com","subject":"[PATCH 2/4] object-file: rename transaction functions","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-28T23:45:17Z","receivedAt":"2026-01-28T23:45:30Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"In a subsequent commit, ODB transactions are made more generic to\nfacilitate each ODB source providing its own transaction handling.\nRename `object_file_transaction_{begin,commit}()` to\n`odb_transaction_loose_{begin,commit}()` to better match the future\nsource specific transaction implementation.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 6 +++---\n object-file.h | 6 +++---\n odb.c | 4 ++--\n 3 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 196509b252..790be25f08 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -723,7 +723,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n \t * We lazily create the temporary object directory\n \t * the first time an object might be added, since\n \t * callers may not know whether any objects will be\n-\t * added at the time they call object_file_transaction_begin.\n+\t * added at the time they call odb_transaction_loose_begin.\n \t */\n \tif (!transaction || transaction->objdir)\n \t\treturn;\n@@ -1985,7 +1985,7 @@ int read_loose_object(struct repository *repo,\n \treturn ret;\n }\n \n-struct odb_transaction *object_file_transaction_begin(struct odb_source *source)\n+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n {\n \tstruct object_database *odb = source->odb;\n \n@@ -1998,7 +1998,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)\n \treturn odb->transaction;\n }\n \n-void object_file_transaction_commit(struct odb_transaction *transaction)\n+void odb_transaction_loose_commit(struct odb_transaction *transaction)\n {\n \tif (!transaction)\n \t\treturn;\ndiff --git a/object-file.h b/object-file.h\nindex 1229d5f675..03f0474656 100644\n--- a/object-file.h\n+++ b/object-file.h\n@@ -202,16 +202,16 @@ struct odb_transaction;\n \n /*\n * Tell the object database to optimize for adding\n- * multiple objects. object_file_transaction_commit must be called\n+ * multiple objects. odb_transaction_loose_commit must be called\n * to make new objects visible. If a transaction is already\n * pending, NULL is returned.\n */\n-struct odb_transaction *object_file_transaction_begin(struct odb_source *source);\n+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);\n \n /*\n * Tell the object database to make any objects from the\n * current transaction visible.\n */\n-void object_file_transaction_commit(struct odb_transaction *transaction);\n+void odb_transaction_loose_commit(struct odb_transaction *transaction);\n \n #endif /* OBJECT_FILE_H */\ndiff --git a/odb.c b/odb.c\nindex ac70b6a099..90dcbca821 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -1153,10 +1153,10 @@ void odb_reprepare(struct object_database *o)\n \n struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n {\n-\treturn object_file_transaction_begin(odb->sources);\n+\treturn odb_transaction_loose_begin(odb->sources);\n }\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n {\n-\tobject_file_transaction_commit(transaction);\n+\todb_transaction_loose_commit(transaction);\n }\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"534791","messageId":"20260128234519.2721179-4-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260128234519.2721179-1-jltobler@gmail.com","subject":"[PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-28T23:45:18Z","receivedAt":"2026-01-28T23:45:30Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Each ODB transaction should be specific to the ODB source it pertains\nto. Update `struct odb_transaction` to store a commit callback specific\nto its object source type. For now `struct odb_transaction_loose` is the\nonly transaction type and what is always returned when starting a\ntransaction.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 80 ++++++++++++++++++++++++++++-----------------------\n object-file.h | 6 ----\n odb.c | 5 +++-\n odb.h | 17 +++++++++++\n 4 files changed, 65 insertions(+), 43 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 790be25f08..e4739fc0cc 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -710,15 +710,17 @@ struct transaction_packfile {\n \tuint32_t nr_written;\n };\n \n-struct odb_transaction {\n-\tstruct odb_source *source;\n+struct odb_transaction_loose {\n+\tstruct odb_transaction base;\n \n \tstruct tmp_objdir *objdir;\n \tstruct transaction_packfile packfile;\n };\n \n-static void prepare_loose_object_transaction(struct odb_transaction *transaction)\n+static void prepare_loose_object_transaction(struct odb_transaction *base)\n {\n+\tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n+\n \t/*\n \t * We lazily create the temporary object directory\n \t * the first time an object might be added, since\n@@ -728,14 +730,16 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n \tif (!transaction || transaction->objdir)\n \t\treturn;\n \n-\ttransaction->objdir = tmp_objdir_create(transaction->source->odb->repo, \"bulk-fsync\");\n+\ttransaction->objdir = tmp_objdir_create(base->source->odb->repo, \"bulk-fsync\");\n \tif (transaction->objdir)\n \t\ttmp_objdir_replace_primary_odb(transaction->objdir, 0);\n }\n \n-static void fsync_loose_object_transaction(struct odb_transaction *transaction,\n+static void fsync_loose_object_transaction(struct odb_transaction *base,\n \t\t\t\t\t int fd, const char *filename)\n {\n+\tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n+\n \t/*\n \t * If we have an active ODB transaction, we issue a call that\n \t * cleans the filesystem page cache but avoids a hardware flush\n@@ -754,7 +758,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *transaction,\n /*\n * Cleanup after batch-mode fsync_object_files.\n */\n-static void flush_loose_object_transaction(struct odb_transaction *transaction)\n+static void flush_loose_object_transaction(struct odb_transaction_loose *transaction)\n {\n \tstruct strbuf temp_path = STRBUF_INIT;\n \tstruct tempfile *temp;\n@@ -772,7 +776,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)\n \t * the final name is visible.\n \t */\n \tstrbuf_addf(&temp_path, \"%s/bulk_fsync_XXXXXX\",\n-\t\t repo_get_object_directory(transaction->source->odb->repo));\n+\t\t repo_get_object_directory(transaction->base.source->odb->repo));\n \ttemp = xmks_tempfile(temp_path.buf);\n \tfsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));\n \tdelete_tempfile(&temp);\n@@ -1340,11 +1344,11 @@ static int index_core(struct index_state *istate,\n \treturn ret;\n }\n \n-static int already_written(struct odb_transaction *transaction,\n+static int already_written(struct odb_transaction_loose *transaction,\n \t\t\t struct object_id *oid)\n {\n \t/* The object may already exist in the repository */\n-\tif (odb_has_object(transaction->source->odb, oid,\n+\tif (odb_has_object(transaction->base.source->odb, oid,\n \t\t\t HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))\n \t\treturn 1;\n \n@@ -1358,14 +1362,14 @@ static int already_written(struct odb_transaction *transaction,\n }\n \n /* Lazily create backing packfile for the state */\n-static void prepare_packfile_transaction(struct odb_transaction *transaction,\n+static void prepare_packfile_transaction(struct odb_transaction_loose *transaction,\n \t\t\t\t\t unsigned flags)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n \tif (!(flags & INDEX_WRITE_OBJECT) || state->f)\n \t\treturn;\n \n-\tstate->f = create_tmp_packfile(transaction->source->odb->repo,\n+\tstate->f = create_tmp_packfile(transaction->base.source->odb->repo,\n \t\t\t\t &state->pack_tmp_name);\n \treset_pack_idx_option(&state->pack_idx_opts);\n \n@@ -1466,10 +1470,10 @@ static int stream_blob_to_pack(struct transaction_packfile *state,\n \treturn 0;\n }\n \n-static void flush_packfile_transaction(struct odb_transaction *transaction)\n+static void flush_packfile_transaction(struct odb_transaction_loose *transaction)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n-\tstruct repository *repo = transaction->source->odb->repo;\n+\tstruct repository *repo = transaction->base.source->odb->repo;\n \tunsigned char hash[GIT_MAX_RAWSZ];\n \tstruct strbuf packname = STRBUF_INIT;\n \tchar *idx_tmp_name = NULL;\n@@ -1494,7 +1498,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)\n \t}\n \n \tstrbuf_addf(&packname, \"%s/pack/pack-%s.\",\n-\t\t repo_get_object_directory(transaction->source->odb->repo),\n+\t\t repo_get_object_directory(transaction->base.source->odb->repo),\n \t\t hash_to_hex_algop(hash, repo->hash_algo));\n \n \tstage_tmp_packfiles(repo, &packname, state->pack_tmp_name,\n@@ -1534,7 +1538,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)\n * binary blobs, they generally do not want to get any conversion, and\n * callers should avoid this code path when filters are requested.\n */\n-static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n+static int index_blob_packfile_transaction(struct odb_transaction_loose *transaction,\n \t\t\t\t\t struct object_id *result_oid, int fd,\n \t\t\t\t\t size_t size, const char *path,\n \t\t\t\t\t unsigned flags)\n@@ -1553,7 +1557,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n \n \theader_len = format_object_header((char *)obuf, sizeof(obuf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\ttransaction->source->odb->repo->hash_algo->init_fn(&ctx);\n+\ttransaction->base.source->odb->repo->hash_algo->init_fn(&ctx);\n \tgit_hash_update(&ctx, obuf, header_len);\n \n \t/* Note: idx is non-NULL when we are writing */\n@@ -1629,10 +1633,11 @@ int index_fd(struct index_state *istate, struct object_id *oid,\n \t\tret = index_core(istate, oid, fd, xsize_t(st->st_size),\n \t\t\t\t type, path, flags);\n \t} else {\n+\t\tstruct object_database *odb = the_repository->objects;\n \t\tstruct odb_transaction *transaction;\n \n-\t\ttransaction = odb_transaction_begin(the_repository->objects);\n-\t\tret = index_blob_packfile_transaction(the_repository->objects->transaction,\n+\t\ttransaction = odb_transaction_begin(odb);\n+\t\tret = index_blob_packfile_transaction((struct odb_transaction_loose *)odb->transaction,\n \t\t\t\t\t\t oid, fd,\n \t\t\t\t\t\t xsize_t(st->st_size),\n \t\t\t\t\t\t path, flags);\n@@ -1985,35 +1990,38 @@ int read_loose_object(struct repository *repo,\n \treturn ret;\n }\n \n-struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n+static void odb_transaction_loose_commit(struct odb_transaction *base)\n {\n-\tstruct object_database *odb = source->odb;\n-\n-\tif (odb->transaction)\n-\t\treturn NULL;\n-\n-\tCALLOC_ARRAY(odb->transaction, 1);\n-\todb->transaction->source = source;\n-\n-\treturn odb->transaction;\n-}\n-\n-void odb_transaction_loose_commit(struct odb_transaction *transaction)\n-{\n-\tif (!transaction)\n-\t\treturn;\n+\tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n \n \t/*\n \t * Ensure the transaction ending matches the pending transaction.\n \t */\n-\tASSERT(transaction == transaction->source->odb->transaction);\n+\tASSERT(base == base->source->odb->transaction);\n \n \tflush_loose_object_transaction(transaction);\n \tflush_packfile_transaction(transaction);\n-\ttransaction->source->odb->transaction = NULL;\n+\tbase->source->odb->transaction = NULL;\n \tfree(transaction);\n }\n \n+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n+{\n+\tstruct odb_transaction_loose *transaction;\n+\tstruct object_database *odb = source->odb;\n+\n+\tif (odb->transaction)\n+\t\treturn NULL;\n+\n+\ttransaction = xcalloc(1, sizeof(*transaction));\n+\ttransaction->base.source = source;\n+\ttransaction->base.commit = odb_transaction_loose_commit;\n+\n+\todb->transaction = &transaction->base;\n+\n+\treturn &transaction->base;\n+}\n+\n struct odb_source_loose *odb_source_loose_new(struct odb_source *source)\n {\n \tstruct odb_source_loose *loose;\ndiff --git a/object-file.h b/object-file.h\nindex 03f0474656..1bd355b7f4 100644\n--- a/object-file.h\n+++ b/object-file.h\n@@ -208,10 +208,4 @@ struct odb_transaction;\n */\n struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);\n \n-/*\n- * Tell the object database to make any objects from the\n- * current transaction visible.\n- */\n-void odb_transaction_loose_commit(struct odb_transaction *transaction);\n-\n #endif /* OBJECT_FILE_H */\ndiff --git a/odb.c b/odb.c\nindex 90dcbca821..5b112f2464 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -1158,5 +1158,8 @@ struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n {\n-\todb_transaction_loose_commit(transaction);\n+\tif (!transaction)\n+\t\treturn;\n+\n+\ttransaction->commit(transaction);\n }\ndiff --git a/odb.h b/odb.h\nindex bab07755f4..83d3a37805 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -77,7 +77,24 @@ struct odb_source {\n struct packed_git;\n struct packfile_store;\n struct cached_object_entry;\n+\n+/*\n+ * A transaction may be started for an object database prior to writing new\n+ * objects via odb_transaction_begin(). These objects are not committed until\n+ * odb_transaction_commit() is invoked. Only a single transaction may be pending\n+ * at a time.\n+ *\n+ * Each ODB source is expected to implement its own transaction handling.\n+ */\n struct odb_transaction;\n+typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);\n+struct odb_transaction {\n+\t/* The ODB source the transaction is opened against. */\n+\tstruct odb_source *source;\n+\n+\t/* The ODB source specific callback invoked to commit a transaction. */\n+\todb_transaction_commit_fn commit;\n+};\n \n /*\n * The object database encapsulates access to objects in a repository. It\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"534792","messageId":"20260128234519.2721179-5-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260128234519.2721179-1-jltobler@gmail.com","subject":"[PATCH 4/4] odb: transparently handle common transaction behavior","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-28T23:45:19Z","receivedAt":"2026-01-28T23:45:31Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"A new ODB transaction is created and returned via\n`odb_transaction_begin()` and stored in the ODB. Only a single\ntransaction may be pending at a time. If the ODB already has a\ntransaction, the function is expected to return NULL. Similarly, when\ncommitting a transaction via `odb_transaction_commit()` the transaction\nbeing committed must match the pending transaction and upon commit reset\nthe ODB transaction to NULL.\n\nThese behaviors apply regardless of the ODB transaction implementation.\nMove the corresponding logic into `odb_transaction_{begin,commit}()`\naccordingly.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 9 ---------\n odb.c | 17 ++++++++++++++++-\n 2 files changed, 16 insertions(+), 10 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex e4739fc0cc..0def5efa91 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -1994,15 +1994,8 @@ static void odb_transaction_loose_commit(struct odb_transaction *base)\n {\n \tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n \n-\t/*\n-\t * Ensure the transaction ending matches the pending transaction.\n-\t */\n-\tASSERT(base == base->source->odb->transaction);\n-\n \tflush_loose_object_transaction(transaction);\n \tflush_packfile_transaction(transaction);\n-\tbase->source->odb->transaction = NULL;\n-\tfree(transaction);\n }\n \n struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n@@ -2017,8 +2010,6 @@ struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n \ttransaction->base.source = source;\n \ttransaction->base.commit = odb_transaction_loose_commit;\n \n-\todb->transaction = &transaction->base;\n-\n \treturn &transaction->base;\n }\n \ndiff --git a/odb.c b/odb.c\nindex 5b112f2464..332860735e 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -1153,7 +1153,15 @@ void odb_reprepare(struct object_database *o)\n \n struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n {\n-\treturn odb_transaction_loose_begin(odb->sources);\n+\tstruct odb_transaction *transaction;\n+\n+\tif (odb->transaction)\n+\t\treturn NULL;\n+\n+\ttransaction = odb_transaction_loose_begin(odb->sources);\n+\todb->transaction = transaction;\n+\n+\treturn transaction;\n }\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n@@ -1161,5 +1169,12 @@ void odb_transaction_commit(struct odb_transaction *transaction)\n \tif (!transaction)\n \t\treturn;\n \n+\t/*\n+\t * Ensure the transaction ending matches the pending transaction.\n+\t */\n+\tASSERT(transaction == transaction->source->odb->transaction);\n+\n \ttransaction->commit(transaction);\n+\ttransaction->source->odb->transaction = NULL;\n+\tfree(transaction);\n }\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"534805","messageId":"aXtDYY0Ao24Mpgyb@pks.im","threadId":"64882","inReplyTo":"20260128234519.2721179-2-jltobler@gmail.com","subject":"Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-29T11:24:17Z","receivedAt":"2026-01-29T11:24:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:\n> Each `struct odb_transaction` currently stores a reference to the\n> `struct object_database`. Since transactions are handled per object\n> source, instead store a reference to the source.\n\nMakes sense.\n\n> diff --git a/object-file.c b/object-file.c\n> index e7e4c3348f..196509b252 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n> \tif (!transaction || transaction->objdir)\n> \t\treturn;\n> \n> -\ttransaction->objdir = tmp_objdir_create(transaction->odb->repo, \"bulk-fsync\");\n> +\ttransaction->objdir = tmp_objdir_create(transaction->source->odb->repo, \"bulk-fsync\");\n> \tif (transaction->objdir)\n> \t\ttmp_objdir_replace_primary_odb(transaction->objdir, 0);\n> }\n\nThis makes me wonder whether we should first refactor the `tmp_objdir`\nsubsystem to receive a source instead of a repository as input.\nOtherwise we \"pretend\" that the transaction is on the source level, but\nwe ultimately still end up creating the temporary directory in the\nrepository's object directory unconditionally.\n\nIt wouldn't really change anything right now as we only ever write\nobjects via the primary object source anyway, so the end result would be\nthe same. But it just feels like a good first step to me to fix this\nconceptual inconsistency, and it shouldn't be too involved either as\n`tmp_objdir_create()` only has three callsites.\n\nPatrick\n"},{"id":"534806","messageId":"aXtDZyFoSQahkxBa@pks.im","threadId":"64882","inReplyTo":"20260128234519.2721179-4-jltobler@gmail.com","subject":"Re: [PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-29T11:24:23Z","receivedAt":"2026-01-29T11:24:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 28, 2026 at 05:45:18PM -0600, Justin Tobler wrote:\n\nThe bit about supporting \"more sources\" in the subject reads a bit weird\nto me. We still only handle a single source in a transaction, not\nmultiple ones. I guess what you rather want to say is that we handle\n\"generic\" sources? How about:\n\n odb: prepare `struct odb_source` to become generic\n\n> Each ODB transaction should be specific to the ODB source it pertains\n> to.\n\nThis is a claim that should probably be backed up a bit. I myself\nobviously agree with it, but I think it should be noted _why_ we want to\nhave this in the first place.\n\nThe patch itself looks as expected to me, as we split up `struct\nodb_transaction` into two structures:\n\n - `struct odb_transaction` continues to exist, but is now the generic\n part that simply contains the source and a function pointer.\n\n - `struct odb_transaction_loose` is the backend-specific\n implementation.\n\nOne question though: is this tansaction really specific to loose\nobjects? We also seem to be handling packfiles there in\n`prepare_packfile_transaction()`, so it rather feels like this is\nspecific to the whole \"files\" backend. I might be misunderstanding\nthough.\n\n> --- a/object-file.h\n> +++ b/object-file.h\n> @@ -208,10 +208,4 @@ struct odb_transaction;\n> */\n> struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);\n\nRight, we still need this function to be public right now. But\neventually it'll be converted into a function pointer part of the\n`struct odb_source`.\n\n> -/*\n> - * Tell the object database to make any objects from the\n> - * current transaction visible.\n> - */\n> -void odb_transaction_loose_commit(struct odb_transaction *transaction);\n\nBut this doesn't have to be public anymore as we have it available to us\nvia `tranasction->commit()`. Makes sense.\n\nPatrick\n"},{"id":"534807","messageId":"aXtDbTuATmSkWRx_@pks.im","threadId":"64882","inReplyTo":"20260128234519.2721179-5-jltobler@gmail.com","subject":"Re: [PATCH 4/4] odb: transparently handle common transaction behavior","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-29T11:24:29Z","receivedAt":"2026-01-29T11:24:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 28, 2026 at 05:45:19PM -0600, Justin Tobler wrote:\n> A new ODB transaction is created and returned via\n> `odb_transaction_begin()` and stored in the ODB. Only a single\n> transaction may be pending at a time. If the ODB already has a\n> transaction, the function is expected to return NULL. Similarly, when\n> committing a transaction via `odb_transaction_commit()` the transaction\n> being committed must match the pending transaction and upon commit reset\n> the ODB transaction to NULL.\n> \n> These behaviors apply regardless of the ODB transaction implementation.\n> Move the corresponding logic into `odb_transaction_{begin,commit}()`\n> accordingly.\n\nBesides deduplicating the logic, it also makes the ODB source only\ncare about itself, whereas the ODB is responsible for managing the\ntransaction's lifecycle. Which I think is a good thing.\n\n> diff --git a/odb.c b/odb.c\n> index 5b112f2464..332860735e 100644\n> --- a/odb.c\n> +++ b/odb.c\n> @@ -1153,7 +1153,15 @@ void odb_reprepare(struct object_database *o)\n> \n> struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n> {\n> -\treturn odb_transaction_loose_begin(odb->sources);\n> +\tstruct odb_transaction *transaction;\n> +\n> +\tif (odb->transaction)\n> +\t\treturn NULL;\n> +\n> +\ttransaction = odb_transaction_loose_begin(odb->sources);\n> +\todb->transaction = transaction;\n> +\n> +\treturn transaction;\n> }\n> \n> void odb_transaction_commit(struct odb_transaction *transaction)\n\nNit: the `transaction` variable seems somewhat pointless to me.\n\nPatrick\n"},{"id":"534830","messageId":"xmqqcy2sb4qr.fsf@gitster.g","threadId":"64882","inReplyTo":"aXtDYY0Ao24Mpgyb@pks.im","subject":"Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-29T19:25:48Z","receivedAt":"2026-01-29T19:26:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:\n>> Each `struct odb_transaction` currently stores a reference to the\n>> `struct object_database`. Since transactions are handled per object\n>> source, instead store a reference to the source.\n>\n> Makes sense.\n>\n>> diff --git a/object-file.c b/object-file.c\n>> index e7e4c3348f..196509b252 100644\n>> --- a/object-file.c\n>> +++ b/object-file.c\n>> @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n>> \tif (!transaction || transaction->objdir)\n>> \t\treturn;\n>> \n>> -\ttransaction->objdir = tmp_objdir_create(transaction->odb->repo, \"bulk-fsync\");\n>> +\ttransaction->objdir = tmp_objdir_create(transaction->source->odb->repo, \"bulk-fsync\");\n>> \tif (transaction->objdir)\n>> \t\ttmp_objdir_replace_primary_odb(transaction->objdir, 0);\n>> }\n>\n> This makes me wonder whether we should first refactor the `tmp_objdir`\n> subsystem to receive a source instead of a repository as input.\n> Otherwise we \"pretend\" that the transaction is on the source level, but\n> we ultimately still end up creating the temporary directory in the\n> repository's object directory unconditionally.\n>\n> It wouldn't really change anything right now as we only ever write\n> objects via the primary object source anyway, so the end result would be\n> the same. But it just feels like a good first step to me to fix this\n> conceptual inconsistency, and it shouldn't be too involved either as\n> `tmp_objdir_create()` only has three callsites.\n\nI agree with your \"not really change anything right now\" comment,\nbut a new odb source that will be invented in the future may not\neven be file based, and a generic-sounding tmp_objdir_create() that\ncreates a temporary directory on the filesystem may not even be an\nappropriate abstraction.\n\nIf we have two or more odb sources both are filesystem based, on the\nother hand, I do not think it is particulary bad if these two odb\nsources belonging to the same repository took a temporary directory\nout of that repository. As long as one temporary object directory\ntaken by one odb source is not used to commit the transaction into\nthe other odb source, it would be fine, no?\n\nThanks.\n\n\n"},{"id":"534831","messageId":"aXu1IDsaUbmUzHop@denethor","threadId":"64882","inReplyTo":"aXtDYY0Ao24Mpgyb@pks.im","subject":"Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-29T19:30:54Z","receivedAt":"2026-01-29T19:31:01Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/01/29 12:24PM, Patrick Steinhardt wrote:\n> On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:\n> > diff --git a/object-file.c b/object-file.c\n> > index e7e4c3348f..196509b252 100644\n> > --- a/object-file.c\n> > +++ b/object-file.c\n> > @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n> > \tif (!transaction || transaction->objdir)\n> > \t\treturn;\n> > \n> > -\ttransaction->objdir = tmp_objdir_create(transaction->odb->repo, \"bulk-fsync\");\n> > +\ttransaction->objdir = tmp_objdir_create(transaction->source->odb->repo, \"bulk-fsync\");\n> > \tif (transaction->objdir)\n> > \t\ttmp_objdir_replace_primary_odb(transaction->objdir, 0);\n> > }\n> \n> This makes me wonder whether we should first refactor the `tmp_objdir`\n> subsystem to receive a source instead of a repository as input.\n> Otherwise we \"pretend\" that the transaction is on the source level, but\n> we ultimately still end up creating the temporary directory in the\n> repository's object directory unconditionally.\n> \n> It wouldn't really change anything right now as we only ever write\n> objects via the primary object source anyway, so the end result would be\n> the same. But it just feels like a good first step to me to fix this\n> conceptual inconsistency, and it shouldn't be too involved either as\n> `tmp_objdir_create()` only has three callsites.\n\nYa this seems reasonable and I agree that it makes sense to do as a\nfirst step. In the next version I'll make this change in a prepatory\ncommit.\n\n-Justin\n"},{"id":"534833","messageId":"aXu1fWWdP3dYKdHf@denethor","threadId":"64882","inReplyTo":"aXtDZyFoSQahkxBa@pks.im","subject":"Re: [PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-29T19:41:23Z","receivedAt":"2026-01-29T19:41:28Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/01/29 12:24PM, Patrick Steinhardt wrote:\n> On Wed, Jan 28, 2026 at 05:45:18PM -0600, Justin Tobler wrote:\n> \n> The bit about supporting \"more sources\" in the subject reads a bit weird\n> to me. We still only handle a single source in a transaction, not\n> multiple ones. I guess what you rather want to say is that we handle\n> \"generic\" sources? How about:\n> \n> odb: prepare `struct odb_source` to become generic\n\nThe intent was that with generic transactions we it becomes possible to\nsupport additional ODB sources each with their own transaction\nimplementation, but I agree the current wording is not great. I'll\nupdate in the next version.\n\n> > Each ODB transaction should be specific to the ODB source it pertains\n> > to.\n> \n> This is a claim that should probably be backed up a bit. I myself\n> obviously agree with it, but I think it should be noted _why_ we want to\n> have this in the first place.\n\nYa that's fair. I'll update the commit message in the next version to\nproperly explain the intent here.\n\n> The patch itself looks as expected to me, as we split up `struct\n> odb_transaction` into two structures:\n> \n> - `struct odb_transaction` continues to exist, but is now the generic\n> part that simply contains the source and a function pointer.\n> \n> - `struct odb_transaction_loose` is the backend-specific\n> implementation.\n> \n> One question though: is this tansaction really specific to loose\n> objects? We also seem to be handling packfiles there in\n> `prepare_packfile_transaction()`, so it rather feels like this is\n> specific to the whole \"files\" backend. I might be misunderstanding\n> though.\n\nThe current transaction backend is primarily used to facilitate bulk\nwriting of what would otherwise be loose objects into a packfile. I\nwould like to eventually expand the use of ODB transactions though to\ncover areas like git-recieve-pack(1) which currently uses tmp-objdir\ndirectly. So it probably makes sense to call this odb_transaction_files.\nWill update in the next version.\n\n-Justin\n"},{"id":"534834","messageId":"aXu4nttn-SWcMmLL@denethor","threadId":"64882","inReplyTo":"xmqqcy2sb4qr.fsf@gitster.g","subject":"Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-29T20:12:05Z","receivedAt":"2026-01-29T20:12:09Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/01/29 11:25AM, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> > This makes me wonder whether we should first refactor the `tmp_objdir`\n> > subsystem to receive a source instead of a repository as input.\n> > Otherwise we \"pretend\" that the transaction is on the source level, but\n> > we ultimately still end up creating the temporary directory in the\n> > repository's object directory unconditionally.\n> >\n> > It wouldn't really change anything right now as we only ever write\n> > objects via the primary object source anyway, so the end result would be\n> > the same. But it just feels like a good first step to me to fix this\n> > conceptual inconsistency, and it shouldn't be too involved either as\n> > `tmp_objdir_create()` only has three callsites.\n> \n> I agree with your \"not really change anything right now\" comment,\n> but a new odb source that will be invented in the future may not\n> even be file based, and a generic-sounding tmp_objdir_create() that\n> creates a temporary directory on the filesystem may not even be an\n> appropriate abstraction.\n\nYup, the tmp-objdir system is really only useful in the context of file\nbased ODB sources. I figure future ODB sources will have to have a\ndifferent mechanism to temporarly store objects.\n\nInterestingly, it looks like there are only three users of odb-tmpdir:\nremerge-diffs, git-recieve-pack, and ODB transactions. All of these\nuse-cases seems like a reasonble fit to create an ODB transaction\ninstead of managing the tmpdir directly. In the case of remerge-diffs\nthe transaction would need to always be aborted. If this is done, then a\ntmpdir could become an internal detail of the ODB transaction for the\nfiles backend.\n\n> If we have two or more odb sources both are filesystem based, on the\n> other hand, I do not think it is particulary bad if these two odb\n> sources belonging to the same repository took a temporary directory\n> out of that repository. As long as one temporary object directory\n> taken by one odb source is not used to commit the transaction into\n> the other odb source, it would be fine, no?\n\nI believe that we only support having a single tmpdir per Git process\nvia `the_tmp_objdir` global anyway. So maybe on second thought\nconfiguring per source doesn't make much sense especially if only the\n\"files\" ODB source would make use of it. If we do go down the route of\nmerging the tmpdir with the \"files\" ODB transaction implementation, we\ncould probably just pass the concrete \"files\" ODB source to it instead.\n\n-Justin\n"},{"id":"534836","messageId":"xmqqwm109n97.fsf@gitster.g","threadId":"64882","inReplyTo":"aXu4nttn-SWcMmLL@denethor","subject":"Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-29T20:28:52Z","receivedAt":"2026-01-29T20:28:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler writes:\n\n> Interestingly, it looks like there are only three users of odb-tmpdir:\n> remerge-diffs, git-recieve-pack, and ODB transactions. All of these\n> use-cases seems like a reasonble fit to create an ODB transaction\n> instead of managing the tmpdir directly. In the case of remerge-diffs\n> the transaction would need to always be aborted. If this is done, then a\n> tmpdir could become an internal detail of the ODB transaction for the\n> files backend.\n\n;-) I agree 100%.\n\n\"Prepare to create objects that may be undone in the end\", \"Now make\nthese objects we created so far as parmanent part of the object\nstore\", \"Reject those objects we created so far as the transaction\ncreated them is being aborted\" are requests at the right abstraction\nlevel. \"Give me a temporary object directory\" is not.\n"},{"id":"534840","messageId":"aXvVeTVujuTzuPp0@denethor","threadId":"64882","inReplyTo":"xmqqwm109n97.fsf@gitster.g","subject":"Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-01-29T21:54:28Z","receivedAt":"2026-01-29T21:54:33Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/01/29 12:28PM, Junio C Hamano wrote:\n> Justin Tobler writes:\n> \n> > Interestingly, it looks like there are only three users of odb-tmpdir:\n> > remerge-diffs, git-recieve-pack, and ODB transactions. All of these\n> > use-cases seems like a reasonble fit to create an ODB transaction\n> > instead of managing the tmpdir directly. In the case of remerge-diffs\n> > the transaction would need to always be aborted. If this is done, then a\n> > tmpdir could become an internal detail of the ODB transaction for the\n> > files backend.\n> \n> ;-) I agree 100%.\n> \n> \"Prepare to create objects that may be undone in the end\", \"Now make\n> these objects we created so far as parmanent part of the object\n> store\", \"Reject those objects we created so far as the transaction\n> created them is being aborted\" are requests at the right abstraction\n> level. \"Give me a temporary object directory\" is not.\n\nOk, I'll go ahead and leave the tmp-objdir stuff alone in this patch\nseries and look into extending the ODB transaction usage to replace\nexisting tmpdir callsites in a followup series.\n\nThanks,\n-Justin\n"},{"id":"535030","messageId":"20260203001002.2500198-1-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260128234519.2721179-1-jltobler@gmail.com","subject":"[PATCH v2 0/4] odb: support ODB source specific transaction handling","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-03T00:09:58Z","receivedAt":"2026-02-03T00:10:12Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Greetings,\n\nODB transactions provide a mechanism for writing new objects in the\nrepository that are in a pending state until they are committed. With\npluggable ODBs, each source will likely need to handle transactions\ndifferently. This patch series prepares ODB transaction handling to be\ndefined for each ODB source by creating a generic odb_transaction\nstructure that stores a callback for committing the transaction.\n\nFor now, `struct odb_transaction_files` is the only transaction\nimplementation and what is always returned when creating a transaction.\nAdditionally, this series lifts transaction logic that is common to all\nbackends out of the implementation layer into\n`odb_transaction_{begin,commit}()` layer.\n\nChanges since V1:\n\n- Renamed the files transaction type to be `odb_transaction_files`.\n Associated functions were also renamed accordingly.\n- Updated commit messages.\n- The `tmp_objdir` subsystem has been left unchange in the version. In a\n followup series, I plan to couple tmpdirs directly to ODB files\n transactions.\n\nThanks,\n-Justin\n\nJustin Tobler (4):\n odb: store ODB source in `struct odb_transaction`\n object-file: rename transaction functions\n odb: prepare `struct odb_transaction` to become generic\n odb: transparently handle common transaction behavior\n\n object-file.c | 77 +++++++++++++++++++++++++--------------------------\n object-file.h | 10 ++-----\n odb.c | 19 +++++++++++--\n odb.h | 17 ++++++++++++\n 4 files changed, 74 insertions(+), 49 deletions(-)\n\nRange-diff against v1:\n1: 525c96f5f2 = 1: 525c96f5f2 odb: store ODB source in `struct odb_transaction`\n2: 5ee1085453 ! 2: 02528f49fb object-file: rename transaction functions\n @@ Commit message\n In a subsequent commit, ODB transactions are made more generic to\n facilitate each ODB source providing its own transaction handling.\n Rename `object_file_transaction_{begin,commit}()` to\n - `odb_transaction_loose_{begin,commit}()` to better match the future\n + `odb_transaction_files_{begin,commit}()` to better match the future\n source specific transaction implementation.\n \n Signed-off-by: Justin Tobler \n @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transacti\n \t * the first time an object might be added, since\n \t * callers may not know whether any objects will be\n -\t * added at the time they call object_file_transaction_begin.\n -+\t * added at the time they call odb_transaction_loose_begin.\n ++\t * added at the time they call odb_transaction_files_begin.\n \t */\n \tif (!transaction || transaction->objdir)\n \t\treturn;\n @@ object-file.c: int read_loose_object(struct repository *repo,\n }\n \n -struct odb_transaction *object_file_transaction_begin(struct odb_source *source)\n -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n {\n \tstruct object_database *odb = source->odb;\n \n @@ object-file.c: struct odb_transaction *object_file_transaction_begin(struct odb_\n }\n \n -void object_file_transaction_commit(struct odb_transaction *transaction)\n -+void odb_transaction_loose_commit(struct odb_transaction *transaction)\n ++void odb_transaction_files_commit(struct odb_transaction *transaction)\n {\n \tif (!transaction)\n \t\treturn;\n @@ object-file.h: struct odb_transaction;\n /*\n * Tell the object database to optimize for adding\n - * multiple objects. object_file_transaction_commit must be called\n -+ * multiple objects. odb_transaction_loose_commit must be called\n ++ * multiple objects. odb_transaction_files_commit must be called\n * to make new objects visible. If a transaction is already\n * pending, NULL is returned.\n */\n -struct odb_transaction *object_file_transaction_begin(struct odb_source *source);\n -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);\n ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);\n \n /*\n * Tell the object database to make any objects from the\n * current transaction visible.\n */\n -void object_file_transaction_commit(struct odb_transaction *transaction);\n -+void odb_transaction_loose_commit(struct odb_transaction *transaction);\n ++void odb_transaction_files_commit(struct odb_transaction *transaction);\n \n #endif /* OBJECT_FILE_H */\n \n @@ odb.c: void odb_reprepare(struct object_database *o)\n struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n {\n -\treturn object_file_transaction_begin(odb->sources);\n -+\treturn odb_transaction_loose_begin(odb->sources);\n ++\treturn odb_transaction_files_begin(odb->sources);\n }\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n {\n -\tobject_file_transaction_commit(transaction);\n -+\todb_transaction_loose_commit(transaction);\n ++\todb_transaction_files_commit(transaction);\n }\n3: a06bfd8c55 ! 3: 081c82428b odb: prepare `struct odb_transaction` to support more sources\n @@ Metadata\n Author: Justin Tobler \n \n ## Commit message ##\n - odb: prepare `struct odb_transaction` to support more sources\n + odb: prepare `struct odb_transaction` to become generic\n \n - Each ODB transaction should be specific to the ODB source it pertains\n - to. Update `struct odb_transaction` to store a commit callback specific\n - to its object source type. For now `struct odb_transaction_loose` is the\n + An ODB transaction handles how objects are stored temporarily and\n + eventually committed. Due to object storage being implemented\n + differently for a given ODB source, the ODB transactions must be\n + implemented in a manner specific to the source the objects are being\n + written to. To provide generic transactions, `struct odb_transaction` is\n + updated to store a commit callback that can be configured to support a\n + specific ODB source. For now `struct odb_transaction_files` is the\n only transaction type and what is always returned when starting a\n transaction.\n \n @@ object-file.c: struct transaction_packfile {\n \n -struct odb_transaction {\n -\tstruct odb_source *source;\n -+struct odb_transaction_loose {\n ++struct odb_transaction_files {\n +\tstruct odb_transaction base;\n \n \tstruct tmp_objdir *objdir;\n @@ object-file.c: struct transaction_packfile {\n -static void prepare_loose_object_transaction(struct odb_transaction *transaction)\n +static void prepare_loose_object_transaction(struct odb_transaction *base)\n {\n -+\tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n ++\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n +\n \t/*\n \t * We lazily create the temporary object directory\n @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transacti\n +static void fsync_loose_object_transaction(struct odb_transaction *base,\n \t\t\t\t\t int fd, const char *filename)\n {\n -+\tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n ++\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n +\n \t/*\n \t * If we have an active ODB transaction, we issue a call that\n @@ object-file.c: static void fsync_loose_object_transaction(struct odb_transaction\n * Cleanup after batch-mode fsync_object_files.\n */\n -static void flush_loose_object_transaction(struct odb_transaction *transaction)\n -+static void flush_loose_object_transaction(struct odb_transaction_loose *transaction)\n ++static void flush_loose_object_transaction(struct odb_transaction_files *transaction)\n {\n \tstruct strbuf temp_path = STRBUF_INIT;\n \tstruct tempfile *temp;\n @@ object-file.c: static int index_core(struct index_state *istate,\n }\n \n -static int already_written(struct odb_transaction *transaction,\n -+static int already_written(struct odb_transaction_loose *transaction,\n ++static int already_written(struct odb_transaction_files *transaction,\n \t\t\t struct object_id *oid)\n {\n \t/* The object may already exist in the repository */\n @@ object-file.c: static int already_written(struct odb_transaction *transaction,\n \n /* Lazily create backing packfile for the state */\n -static void prepare_packfile_transaction(struct odb_transaction *transaction,\n -+static void prepare_packfile_transaction(struct odb_transaction_loose *transaction,\n ++static void prepare_packfile_transaction(struct odb_transaction_files *transaction,\n \t\t\t\t\t unsigned flags)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n @@ object-file.c: static int stream_blob_to_pack(struct transaction_packfile *state\n }\n \n -static void flush_packfile_transaction(struct odb_transaction *transaction)\n -+static void flush_packfile_transaction(struct odb_transaction_loose *transaction)\n ++static void flush_packfile_transaction(struct odb_transaction_files *transaction)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n -\tstruct repository *repo = transaction->source->odb->repo;\n @@ object-file.c: static void flush_packfile_transaction(struct odb_transaction *tr\n * callers should avoid this code path when filters are requested.\n */\n -static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n -+static int index_blob_packfile_transaction(struct odb_transaction_loose *transaction,\n ++static int index_blob_packfile_transaction(struct odb_transaction_files *transaction,\n \t\t\t\t\t struct object_id *result_oid, int fd,\n \t\t\t\t\t size_t size, const char *path,\n \t\t\t\t\t unsigned flags)\n @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,\n -\t\ttransaction = odb_transaction_begin(the_repository->objects);\n -\t\tret = index_blob_packfile_transaction(the_repository->objects->transaction,\n +\t\ttransaction = odb_transaction_begin(odb);\n -+\t\tret = index_blob_packfile_transaction((struct odb_transaction_loose *)odb->transaction,\n ++\t\tret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,\n \t\t\t\t\t\t oid, fd,\n \t\t\t\t\t\t xsize_t(st->st_size),\n \t\t\t\t\t\t path, flags);\n @@ object-file.c: int read_loose_object(struct repository *repo,\n \treturn ret;\n }\n \n --struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n -+static void odb_transaction_loose_commit(struct odb_transaction *base)\n +-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n ++static void odb_transaction_files_commit(struct odb_transaction *base)\n {\n -\tstruct object_database *odb = source->odb;\n -\n @@ object-file.c: int read_loose_object(struct repository *repo,\n -\treturn odb->transaction;\n -}\n -\n --void odb_transaction_loose_commit(struct odb_transaction *transaction)\n +-void odb_transaction_files_commit(struct odb_transaction *transaction)\n -{\n -\tif (!transaction)\n -\t\treturn;\n -+\tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n ++\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n \n \t/*\n \t * Ensure the transaction ending matches the pending transaction.\n @@ object-file.c: int read_loose_object(struct repository *repo,\n \tfree(transaction);\n }\n \n -+struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n ++struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n +{\n -+\tstruct odb_transaction_loose *transaction;\n ++\tstruct odb_transaction_files *transaction;\n +\tstruct object_database *odb = source->odb;\n +\n +\tif (odb->transaction)\n @@ object-file.c: int read_loose_object(struct repository *repo,\n +\n +\ttransaction = xcalloc(1, sizeof(*transaction));\n +\ttransaction->base.source = source;\n -+\ttransaction->base.commit = odb_transaction_loose_commit;\n ++\ttransaction->base.commit = odb_transaction_files_commit;\n +\n +\todb->transaction = &transaction->base;\n +\n @@ object-file.c: int read_loose_object(struct repository *repo,\n ## object-file.h ##\n @@ object-file.h: struct odb_transaction;\n */\n - struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source);\n + struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);\n \n -/*\n - * Tell the object database to make any objects from the\n - * current transaction visible.\n - */\n --void odb_transaction_loose_commit(struct odb_transaction *transaction);\n +-void odb_transaction_files_commit(struct odb_transaction *transaction);\n -\n #endif /* OBJECT_FILE_H */\n \n @@ odb.c: struct odb_transaction *odb_transaction_begin(struct object_database *odb\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n {\n --\todb_transaction_loose_commit(transaction);\n +-\todb_transaction_files_commit(transaction);\n +\tif (!transaction)\n +\t\treturn;\n +\n4: 476ed1235c ! 4: ed8164a3a3 odb: transparently handle common transaction behavior\n @@ Commit message\n Signed-off-by: Justin Tobler \n \n ## object-file.c ##\n -@@ object-file.c: static void odb_transaction_loose_commit(struct odb_transaction *base)\n +@@ object-file.c: static void odb_transaction_files_commit(struct odb_transaction *base)\n {\n - \tstruct odb_transaction_loose *transaction = (struct odb_transaction_loose *)base;\n + \tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n \n -\t/*\n -\t * Ensure the transaction ending matches the pending transaction.\n @@ object-file.c: static void odb_transaction_loose_commit(struct odb_transaction *\n -\tfree(transaction);\n }\n \n - struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n -@@ object-file.c: struct odb_transaction *odb_transaction_loose_begin(struct odb_source *source)\n + struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n +@@ object-file.c: struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n \ttransaction->base.source = source;\n - \ttransaction->base.commit = odb_transaction_loose_commit;\n + \ttransaction->base.commit = odb_transaction_files_commit;\n \n -\todb->transaction = &transaction->base;\n -\n @@ odb.c: void odb_reprepare(struct object_database *o)\n \n struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n {\n --\treturn odb_transaction_loose_begin(odb->sources);\n -+\tstruct odb_transaction *transaction;\n -+\n +-\treturn odb_transaction_files_begin(odb->sources);\n +\tif (odb->transaction)\n +\t\treturn NULL;\n +\n -+\ttransaction = odb_transaction_loose_begin(odb->sources);\n -+\todb->transaction = transaction;\n ++\todb->transaction = odb_transaction_files_begin(odb->sources);\n +\n -+\treturn transaction;\n ++\treturn odb->transaction;\n }\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n\nbase-commit: ea717645d199f6f1b66058886475db3e8c9330e9\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"535031","messageId":"20260203001002.2500198-2-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260203001002.2500198-1-jltobler@gmail.com","subject":"[PATCH v2 1/4] odb: store ODB source in `struct odb_transaction`","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-03T00:09:59Z","receivedAt":"2026-02-03T00:10:12Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Each `struct odb_transaction` currently stores a reference to the\n`struct object_database`. Since transactions are handled per object\nsource, instead store a reference to the source.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex e7e4c3348f..196509b252 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -711,7 +711,7 @@ struct transaction_packfile {\n };\n \n struct odb_transaction {\n-\tstruct object_database *odb;\n+\tstruct odb_source *source;\n \n \tstruct tmp_objdir *objdir;\n \tstruct transaction_packfile packfile;\n@@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n \tif (!transaction || transaction->objdir)\n \t\treturn;\n \n-\ttransaction->objdir = tmp_objdir_create(transaction->odb->repo, \"bulk-fsync\");\n+\ttransaction->objdir = tmp_objdir_create(transaction->source->odb->repo, \"bulk-fsync\");\n \tif (transaction->objdir)\n \t\ttmp_objdir_replace_primary_odb(transaction->objdir, 0);\n }\n@@ -772,7 +772,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)\n \t * the final name is visible.\n \t */\n \tstrbuf_addf(&temp_path, \"%s/bulk_fsync_XXXXXX\",\n-\t\t repo_get_object_directory(transaction->odb->repo));\n+\t\t repo_get_object_directory(transaction->source->odb->repo));\n \ttemp = xmks_tempfile(temp_path.buf);\n \tfsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));\n \tdelete_tempfile(&temp);\n@@ -1344,7 +1344,7 @@ static int already_written(struct odb_transaction *transaction,\n \t\t\t struct object_id *oid)\n {\n \t/* The object may already exist in the repository */\n-\tif (odb_has_object(transaction->odb, oid,\n+\tif (odb_has_object(transaction->source->odb, oid,\n \t\t\t HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))\n \t\treturn 1;\n \n@@ -1365,7 +1365,7 @@ static void prepare_packfile_transaction(struct odb_transaction *transaction,\n \tif (!(flags & INDEX_WRITE_OBJECT) || state->f)\n \t\treturn;\n \n-\tstate->f = create_tmp_packfile(transaction->odb->repo,\n+\tstate->f = create_tmp_packfile(transaction->source->odb->repo,\n \t\t\t\t &state->pack_tmp_name);\n \treset_pack_idx_option(&state->pack_idx_opts);\n \n@@ -1469,7 +1469,7 @@ static int stream_blob_to_pack(struct transaction_packfile *state,\n static void flush_packfile_transaction(struct odb_transaction *transaction)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n-\tstruct repository *repo = transaction->odb->repo;\n+\tstruct repository *repo = transaction->source->odb->repo;\n \tunsigned char hash[GIT_MAX_RAWSZ];\n \tstruct strbuf packname = STRBUF_INIT;\n \tchar *idx_tmp_name = NULL;\n@@ -1494,7 +1494,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)\n \t}\n \n \tstrbuf_addf(&packname, \"%s/pack/pack-%s.\",\n-\t\t repo_get_object_directory(transaction->odb->repo),\n+\t\t repo_get_object_directory(transaction->source->odb->repo),\n \t\t hash_to_hex_algop(hash, repo->hash_algo));\n \n \tstage_tmp_packfiles(repo, &packname, state->pack_tmp_name,\n@@ -1553,7 +1553,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n \n \theader_len = format_object_header((char *)obuf, sizeof(obuf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\ttransaction->odb->repo->hash_algo->init_fn(&ctx);\n+\ttransaction->source->odb->repo->hash_algo->init_fn(&ctx);\n \tgit_hash_update(&ctx, obuf, header_len);\n \n \t/* Note: idx is non-NULL when we are writing */\n@@ -1993,7 +1993,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)\n \t\treturn NULL;\n \n \tCALLOC_ARRAY(odb->transaction, 1);\n-\todb->transaction->odb = odb;\n+\todb->transaction->source = source;\n \n \treturn odb->transaction;\n }\n@@ -2006,11 +2006,11 @@ void object_file_transaction_commit(struct odb_transaction *transaction)\n \t/*\n \t * Ensure the transaction ending matches the pending transaction.\n \t */\n-\tASSERT(transaction == transaction->odb->transaction);\n+\tASSERT(transaction == transaction->source->odb->transaction);\n \n \tflush_loose_object_transaction(transaction);\n \tflush_packfile_transaction(transaction);\n-\ttransaction->odb->transaction = NULL;\n+\ttransaction->source->odb->transaction = NULL;\n \tfree(transaction);\n }\n \n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"535032","messageId":"20260203001002.2500198-3-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260203001002.2500198-1-jltobler@gmail.com","subject":"[PATCH v2 2/4] object-file: rename transaction functions","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-03T00:10:00Z","receivedAt":"2026-02-03T00:10:14Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"In a subsequent commit, ODB transactions are made more generic to\nfacilitate each ODB source providing its own transaction handling.\nRename `object_file_transaction_{begin,commit}()` to\n`odb_transaction_files_{begin,commit}()` to better match the future\nsource specific transaction implementation.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 6 +++---\n object-file.h | 6 +++---\n odb.c | 4 ++--\n 3 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 196509b252..7b34a2b274 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -723,7 +723,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n \t * We lazily create the temporary object directory\n \t * the first time an object might be added, since\n \t * callers may not know whether any objects will be\n-\t * added at the time they call object_file_transaction_begin.\n+\t * added at the time they call odb_transaction_files_begin.\n \t */\n \tif (!transaction || transaction->objdir)\n \t\treturn;\n@@ -1985,7 +1985,7 @@ int read_loose_object(struct repository *repo,\n \treturn ret;\n }\n \n-struct odb_transaction *object_file_transaction_begin(struct odb_source *source)\n+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n {\n \tstruct object_database *odb = source->odb;\n \n@@ -1998,7 +1998,7 @@ struct odb_transaction *object_file_transaction_begin(struct odb_source *source)\n \treturn odb->transaction;\n }\n \n-void object_file_transaction_commit(struct odb_transaction *transaction)\n+void odb_transaction_files_commit(struct odb_transaction *transaction)\n {\n \tif (!transaction)\n \t\treturn;\ndiff --git a/object-file.h b/object-file.h\nindex 1229d5f675..b4a3341a89 100644\n--- a/object-file.h\n+++ b/object-file.h\n@@ -202,16 +202,16 @@ struct odb_transaction;\n \n /*\n * Tell the object database to optimize for adding\n- * multiple objects. object_file_transaction_commit must be called\n+ * multiple objects. odb_transaction_files_commit must be called\n * to make new objects visible. If a transaction is already\n * pending, NULL is returned.\n */\n-struct odb_transaction *object_file_transaction_begin(struct odb_source *source);\n+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);\n \n /*\n * Tell the object database to make any objects from the\n * current transaction visible.\n */\n-void object_file_transaction_commit(struct odb_transaction *transaction);\n+void odb_transaction_files_commit(struct odb_transaction *transaction);\n \n #endif /* OBJECT_FILE_H */\ndiff --git a/odb.c b/odb.c\nindex ac70b6a099..a5e6fd01a9 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -1153,10 +1153,10 @@ void odb_reprepare(struct object_database *o)\n \n struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n {\n-\treturn object_file_transaction_begin(odb->sources);\n+\treturn odb_transaction_files_begin(odb->sources);\n }\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n {\n-\tobject_file_transaction_commit(transaction);\n+\todb_transaction_files_commit(transaction);\n }\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"535033","messageId":"20260203001002.2500198-4-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260203001002.2500198-1-jltobler@gmail.com","subject":"[PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-03T00:10:01Z","receivedAt":"2026-02-03T00:10:15Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"An ODB transaction handles how objects are stored temporarily and\neventually committed. Due to object storage being implemented\ndifferently for a given ODB source, the ODB transactions must be\nimplemented in a manner specific to the source the objects are being\nwritten to. To provide generic transactions, `struct odb_transaction` is\nupdated to store a commit callback that can be configured to support a\nspecific ODB source. For now `struct odb_transaction_files` is the\nonly transaction type and what is always returned when starting a\ntransaction.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 80 ++++++++++++++++++++++++++++-----------------------\n object-file.h | 6 ----\n odb.c | 5 +++-\n odb.h | 17 +++++++++++\n 4 files changed, 65 insertions(+), 43 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 7b34a2b274..d7e153c1b9 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -710,15 +710,17 @@ struct transaction_packfile {\n \tuint32_t nr_written;\n };\n \n-struct odb_transaction {\n-\tstruct odb_source *source;\n+struct odb_transaction_files {\n+\tstruct odb_transaction base;\n \n \tstruct tmp_objdir *objdir;\n \tstruct transaction_packfile packfile;\n };\n \n-static void prepare_loose_object_transaction(struct odb_transaction *transaction)\n+static void prepare_loose_object_transaction(struct odb_transaction *base)\n {\n+\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n+\n \t/*\n \t * We lazily create the temporary object directory\n \t * the first time an object might be added, since\n@@ -728,14 +730,16 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction\n \tif (!transaction || transaction->objdir)\n \t\treturn;\n \n-\ttransaction->objdir = tmp_objdir_create(transaction->source->odb->repo, \"bulk-fsync\");\n+\ttransaction->objdir = tmp_objdir_create(base->source->odb->repo, \"bulk-fsync\");\n \tif (transaction->objdir)\n \t\ttmp_objdir_replace_primary_odb(transaction->objdir, 0);\n }\n \n-static void fsync_loose_object_transaction(struct odb_transaction *transaction,\n+static void fsync_loose_object_transaction(struct odb_transaction *base,\n \t\t\t\t\t int fd, const char *filename)\n {\n+\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n+\n \t/*\n \t * If we have an active ODB transaction, we issue a call that\n \t * cleans the filesystem page cache but avoids a hardware flush\n@@ -754,7 +758,7 @@ static void fsync_loose_object_transaction(struct odb_transaction *transaction,\n /*\n * Cleanup after batch-mode fsync_object_files.\n */\n-static void flush_loose_object_transaction(struct odb_transaction *transaction)\n+static void flush_loose_object_transaction(struct odb_transaction_files *transaction)\n {\n \tstruct strbuf temp_path = STRBUF_INIT;\n \tstruct tempfile *temp;\n@@ -772,7 +776,7 @@ static void flush_loose_object_transaction(struct odb_transaction *transaction)\n \t * the final name is visible.\n \t */\n \tstrbuf_addf(&temp_path, \"%s/bulk_fsync_XXXXXX\",\n-\t\t repo_get_object_directory(transaction->source->odb->repo));\n+\t\t repo_get_object_directory(transaction->base.source->odb->repo));\n \ttemp = xmks_tempfile(temp_path.buf);\n \tfsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp));\n \tdelete_tempfile(&temp);\n@@ -1340,11 +1344,11 @@ static int index_core(struct index_state *istate,\n \treturn ret;\n }\n \n-static int already_written(struct odb_transaction *transaction,\n+static int already_written(struct odb_transaction_files *transaction,\n \t\t\t struct object_id *oid)\n {\n \t/* The object may already exist in the repository */\n-\tif (odb_has_object(transaction->source->odb, oid,\n+\tif (odb_has_object(transaction->base.source->odb, oid,\n \t\t\t HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR))\n \t\treturn 1;\n \n@@ -1358,14 +1362,14 @@ static int already_written(struct odb_transaction *transaction,\n }\n \n /* Lazily create backing packfile for the state */\n-static void prepare_packfile_transaction(struct odb_transaction *transaction,\n+static void prepare_packfile_transaction(struct odb_transaction_files *transaction,\n \t\t\t\t\t unsigned flags)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n \tif (!(flags & INDEX_WRITE_OBJECT) || state->f)\n \t\treturn;\n \n-\tstate->f = create_tmp_packfile(transaction->source->odb->repo,\n+\tstate->f = create_tmp_packfile(transaction->base.source->odb->repo,\n \t\t\t\t &state->pack_tmp_name);\n \treset_pack_idx_option(&state->pack_idx_opts);\n \n@@ -1466,10 +1470,10 @@ static int stream_blob_to_pack(struct transaction_packfile *state,\n \treturn 0;\n }\n \n-static void flush_packfile_transaction(struct odb_transaction *transaction)\n+static void flush_packfile_transaction(struct odb_transaction_files *transaction)\n {\n \tstruct transaction_packfile *state = &transaction->packfile;\n-\tstruct repository *repo = transaction->source->odb->repo;\n+\tstruct repository *repo = transaction->base.source->odb->repo;\n \tunsigned char hash[GIT_MAX_RAWSZ];\n \tstruct strbuf packname = STRBUF_INIT;\n \tchar *idx_tmp_name = NULL;\n@@ -1494,7 +1498,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)\n \t}\n \n \tstrbuf_addf(&packname, \"%s/pack/pack-%s.\",\n-\t\t repo_get_object_directory(transaction->source->odb->repo),\n+\t\t repo_get_object_directory(transaction->base.source->odb->repo),\n \t\t hash_to_hex_algop(hash, repo->hash_algo));\n \n \tstage_tmp_packfiles(repo, &packname, state->pack_tmp_name,\n@@ -1534,7 +1538,7 @@ static void flush_packfile_transaction(struct odb_transaction *transaction)\n * binary blobs, they generally do not want to get any conversion, and\n * callers should avoid this code path when filters are requested.\n */\n-static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n+static int index_blob_packfile_transaction(struct odb_transaction_files *transaction,\n \t\t\t\t\t struct object_id *result_oid, int fd,\n \t\t\t\t\t size_t size, const char *path,\n \t\t\t\t\t unsigned flags)\n@@ -1553,7 +1557,7 @@ static int index_blob_packfile_transaction(struct odb_transaction *transaction,\n \n \theader_len = format_object_header((char *)obuf, sizeof(obuf),\n \t\t\t\t\t OBJ_BLOB, size);\n-\ttransaction->source->odb->repo->hash_algo->init_fn(&ctx);\n+\ttransaction->base.source->odb->repo->hash_algo->init_fn(&ctx);\n \tgit_hash_update(&ctx, obuf, header_len);\n \n \t/* Note: idx is non-NULL when we are writing */\n@@ -1629,10 +1633,11 @@ int index_fd(struct index_state *istate, struct object_id *oid,\n \t\tret = index_core(istate, oid, fd, xsize_t(st->st_size),\n \t\t\t\t type, path, flags);\n \t} else {\n+\t\tstruct object_database *odb = the_repository->objects;\n \t\tstruct odb_transaction *transaction;\n \n-\t\ttransaction = odb_transaction_begin(the_repository->objects);\n-\t\tret = index_blob_packfile_transaction(the_repository->objects->transaction,\n+\t\ttransaction = odb_transaction_begin(odb);\n+\t\tret = index_blob_packfile_transaction((struct odb_transaction_files *)odb->transaction,\n \t\t\t\t\t\t oid, fd,\n \t\t\t\t\t\t xsize_t(st->st_size),\n \t\t\t\t\t\t path, flags);\n@@ -1985,35 +1990,38 @@ int read_loose_object(struct repository *repo,\n \treturn ret;\n }\n \n-struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n+static void odb_transaction_files_commit(struct odb_transaction *base)\n {\n-\tstruct object_database *odb = source->odb;\n-\n-\tif (odb->transaction)\n-\t\treturn NULL;\n-\n-\tCALLOC_ARRAY(odb->transaction, 1);\n-\todb->transaction->source = source;\n-\n-\treturn odb->transaction;\n-}\n-\n-void odb_transaction_files_commit(struct odb_transaction *transaction)\n-{\n-\tif (!transaction)\n-\t\treturn;\n+\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n \n \t/*\n \t * Ensure the transaction ending matches the pending transaction.\n \t */\n-\tASSERT(transaction == transaction->source->odb->transaction);\n+\tASSERT(base == base->source->odb->transaction);\n \n \tflush_loose_object_transaction(transaction);\n \tflush_packfile_transaction(transaction);\n-\ttransaction->source->odb->transaction = NULL;\n+\tbase->source->odb->transaction = NULL;\n \tfree(transaction);\n }\n \n+struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n+{\n+\tstruct odb_transaction_files *transaction;\n+\tstruct object_database *odb = source->odb;\n+\n+\tif (odb->transaction)\n+\t\treturn NULL;\n+\n+\ttransaction = xcalloc(1, sizeof(*transaction));\n+\ttransaction->base.source = source;\n+\ttransaction->base.commit = odb_transaction_files_commit;\n+\n+\todb->transaction = &transaction->base;\n+\n+\treturn &transaction->base;\n+}\n+\n struct odb_source_loose *odb_source_loose_new(struct odb_source *source)\n {\n \tstruct odb_source_loose *loose;\ndiff --git a/object-file.h b/object-file.h\nindex b4a3341a89..a62d0de394 100644\n--- a/object-file.h\n+++ b/object-file.h\n@@ -208,10 +208,4 @@ struct odb_transaction;\n */\n struct odb_transaction *odb_transaction_files_begin(struct odb_source *source);\n \n-/*\n- * Tell the object database to make any objects from the\n- * current transaction visible.\n- */\n-void odb_transaction_files_commit(struct odb_transaction *transaction);\n-\n #endif /* OBJECT_FILE_H */\ndiff --git a/odb.c b/odb.c\nindex a5e6fd01a9..349b4218a5 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -1158,5 +1158,8 @@ struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n {\n-\todb_transaction_files_commit(transaction);\n+\tif (!transaction)\n+\t\treturn;\n+\n+\ttransaction->commit(transaction);\n }\ndiff --git a/odb.h b/odb.h\nindex bab07755f4..83d3a37805 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -77,7 +77,24 @@ struct odb_source {\n struct packed_git;\n struct packfile_store;\n struct cached_object_entry;\n+\n+/*\n+ * A transaction may be started for an object database prior to writing new\n+ * objects via odb_transaction_begin(). These objects are not committed until\n+ * odb_transaction_commit() is invoked. Only a single transaction may be pending\n+ * at a time.\n+ *\n+ * Each ODB source is expected to implement its own transaction handling.\n+ */\n struct odb_transaction;\n+typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);\n+struct odb_transaction {\n+\t/* The ODB source the transaction is opened against. */\n+\tstruct odb_source *source;\n+\n+\t/* The ODB source specific callback invoked to commit a transaction. */\n+\todb_transaction_commit_fn commit;\n+};\n \n /*\n * The object database encapsulates access to objects in a repository. It\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"535034","messageId":"20260203001002.2500198-5-jltobler@gmail.com","threadId":"64882","inReplyTo":"20260203001002.2500198-1-jltobler@gmail.com","subject":"[PATCH v2 4/4] odb: transparently handle common transaction behavior","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-03T00:10:02Z","receivedAt":"2026-02-03T00:10:16Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"A new ODB transaction is created and returned via\n`odb_transaction_begin()` and stored in the ODB. Only a single\ntransaction may be pending at a time. If the ODB already has a\ntransaction, the function is expected to return NULL. Similarly, when\ncommitting a transaction via `odb_transaction_commit()` the transaction\nbeing committed must match the pending transaction and upon commit reset\nthe ODB transaction to NULL.\n\nThese behaviors apply regardless of the ODB transaction implementation.\nMove the corresponding logic into `odb_transaction_{begin,commit}()`\naccordingly.\n\nSigned-off-by: Justin Tobler \n---\n object-file.c | 9 ---------\n odb.c | 14 +++++++++++++-\n 2 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex d7e153c1b9..1b62996ef0 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -1994,15 +1994,8 @@ static void odb_transaction_files_commit(struct odb_transaction *base)\n {\n \tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n \n-\t/*\n-\t * Ensure the transaction ending matches the pending transaction.\n-\t */\n-\tASSERT(base == base->source->odb->transaction);\n-\n \tflush_loose_object_transaction(transaction);\n \tflush_packfile_transaction(transaction);\n-\tbase->source->odb->transaction = NULL;\n-\tfree(transaction);\n }\n \n struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n@@ -2017,8 +2010,6 @@ struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)\n \ttransaction->base.source = source;\n \ttransaction->base.commit = odb_transaction_files_commit;\n \n-\todb->transaction = &transaction->base;\n-\n \treturn &transaction->base;\n }\n \ndiff --git a/odb.c b/odb.c\nindex 349b4218a5..1679cc0465 100644\n--- a/odb.c\n+++ b/odb.c\n@@ -1153,7 +1153,12 @@ void odb_reprepare(struct object_database *o)\n \n struct odb_transaction *odb_transaction_begin(struct object_database *odb)\n {\n-\treturn odb_transaction_files_begin(odb->sources);\n+\tif (odb->transaction)\n+\t\treturn NULL;\n+\n+\todb->transaction = odb_transaction_files_begin(odb->sources);\n+\n+\treturn odb->transaction;\n }\n \n void odb_transaction_commit(struct odb_transaction *transaction)\n@@ -1161,5 +1166,12 @@ void odb_transaction_commit(struct odb_transaction *transaction)\n \tif (!transaction)\n \t\treturn;\n \n+\t/*\n+\t * Ensure the transaction ending matches the pending transaction.\n+\t */\n+\tASSERT(transaction == transaction->source->odb->transaction);\n+\n \ttransaction->commit(transaction);\n+\ttransaction->source->odb->transaction = NULL;\n+\tfree(transaction);\n }\n-- \n2.52.0.373.g68cb7f9e92\n\n"},{"id":"535036","messageId":"xmqqjywuvd7o.fsf@gitster.g","threadId":"64882","inReplyTo":"20260203001002.2500198-1-jltobler@gmail.com","subject":"Re: [PATCH v2 0/4] odb: support ODB source specific transaction handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-03T01:16:11Z","receivedAt":"2026-02-03T01:16:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler writes:\n\n> Changes since V1:\n>\n> - Renamed the files transaction type to be `odb_transaction_files`.\n> Associated functions were also renamed accordingly.\n> - Updated commit messages.\n> - The `tmp_objdir` subsystem has been left unchange in the version. In a\n> followup series, I plan to couple tmpdirs directly to ODB files\n> transactions.\n\nAh, I was wondering why the changes are so small (it looked\nessentially s/loose/files/ and nothing else to me), but if we are\nleaving the tmp_objdir stuff out of the scope (which is sensible),\nthen there aren't much left to do relative to the previous\niteration.\n\nLooking good. Will replace.\n\nThanks.\n"},{"id":"535070","messageId":"87o6m5rff8.fsf@iotcl.com","threadId":"64882","inReplyTo":"20260203001002.2500198-4-jltobler@gmail.com","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-02-03T15:54:19Z","receivedAt":"2026-02-03T15:54:42Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Justin Tobler writes:\n\n> An ODB transaction handles how objects are stored temporarily and\n> eventually committed. Due to object storage being implemented\n> differently for a given ODB source, the ODB transactions must be\n> implemented in a manner specific to the source the objects are being\n> written to. To provide generic transactions, `struct odb_transaction` is\n> updated to store a commit callback that can be configured to support a\n> specific ODB source. For now `struct odb_transaction_files` is the\n> only transaction type and what is always returned when starting a\n> transaction.\n>\n> Signed-off-by: Justin Tobler \n> ---\n> object-file.c | 80 ++++++++++++++++++++++++++++-----------------------\n> object-file.h | 6 ----\n> odb.c | 5 +++-\n> odb.h | 17 +++++++++++\n> 4 files changed, 65 insertions(+), 43 deletions(-)\n>\n> diff --git a/object-file.c b/object-file.c\n> index 7b34a2b274..d7e153c1b9 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -710,15 +710,17 @@ struct transaction_packfile {\n> \tuint32_t nr_written;\n> };\n> \n> -struct odb_transaction {\n> -\tstruct odb_source *source;\n> +struct odb_transaction_files {\n> +\tstruct odb_transaction base;\n> \n> \tstruct tmp_objdir *objdir;\n> \tstruct transaction_packfile packfile;\n> };\n> \n> -static void prepare_loose_object_transaction(struct odb_transaction *transaction)\n> +static void prepare_loose_object_transaction(struct odb_transaction *base)\n> {\n> +\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n\nSo you're assuming `struct odb_transaction` is the first field in\n`struct odb_transaction_files`?\n\nI think it would be safer to do this instead:\n\n+\tstruct odb_transaction_files *transaction =\n+\t\tcontainer_of(base, struct odb_transaction_files, base);\n\n(this also can be applied in a few other places in this patch)\n\n-- \nCheers,\nToon\n"},{"id":"535071","messageId":"aYIirIQveQiNWzsx@denethor","threadId":"64882","inReplyTo":"87o6m5rff8.fsf@iotcl.com","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-03T16:46:22Z","receivedAt":"2026-02-03T16:46:27Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/03 04:54PM, Toon Claes wrote:\n> Justin Tobler writes:\n> > -static void prepare_loose_object_transaction(struct odb_transaction *transaction)\n> > +static void prepare_loose_object_transaction(struct odb_transaction *base)\n> > {\n> > +\tstruct odb_transaction_files *transaction = (struct odb_transaction_files *)base;\n> \n> So you're assuming `struct odb_transaction` is the first field in\n> `struct odb_transaction_files`?\n\nYes, `struct odb_transaction_files` contains a reference to `struct\nodb_transaction` as its first member which is the \"base\" transaction\ntype. This way we can easily cast to the containing \"concrete\" type.\n\n> I think it would be safer to do this instead:\n> \n> +\tstruct odb_transaction_files *transaction =\n> +\t\tcontainer_of(base, struct odb_transaction_files, base);\n> \n> (this also can be applied in a few other places in this patch)\n\nSo long as `struct odb_transaction` remains the first member of `struct\nodb_transaction_files`, using `container_of()` is functionally the same\nthing as casting. This is also how we do things for `odb_read_stream`\nand `odb_loose_read_stream` so I don't think it is too big of a deal\neither way.\n\nI've made the change locally, but will hold off from sending another\nversion unless folks feel strongly about this change or there is other\nfeedback.\n\nThanks,\n-Justin\n"},{"id":"535109","messageId":"xmqqfr7hqvzj.fsf@gitster.g","threadId":"64882","inReplyTo":"aYIirIQveQiNWzsx@denethor","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-03T22:54:08Z","receivedAt":"2026-02-03T22:54:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler writes:\n\n> So long as `struct odb_transaction` remains the first member of `struct\n> odb_transaction_files`, using `container_of()` is functionally the same\n> thing as casting. This is also how we do things for `odb_read_stream`\n> and `odb_loose_read_stream` so I don't think it is too big of a deal\n> either way.\n\nIt would be a good theme to clean them all up, together with\nexisting ones, in a separate topic. Code hygiene matters.\n"},{"id":"535117","messageId":"aYLmdIh8WB7nSZPL@pks.im","threadId":"64882","inReplyTo":"xmqqjywuvd7o.fsf@gitster.g","subject":"Re: [PATCH v2 0/4] odb: support ODB source specific transaction handling","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-04T06:25:56Z","receivedAt":"2026-02-04T06:26:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Feb 02, 2026 at 05:16:11PM -0800, Junio C Hamano wrote:\n> Justin Tobler writes:\n> \n> > Changes since V1:\n> >\n> > - Renamed the files transaction type to be `odb_transaction_files`.\n> > Associated functions were also renamed accordingly.\n> > - Updated commit messages.\n> > - The `tmp_objdir` subsystem has been left unchange in the version. In a\n> > followup series, I plan to couple tmpdirs directly to ODB files\n> > transactions.\n> \n> Ah, I was wondering why the changes are so small (it looked\n> essentially s/loose/files/ and nothing else to me), but if we are\n> leaving the tmp_objdir stuff out of the scope (which is sensible),\n> then there aren't much left to do relative to the previous\n> iteration.\n\nYeah, agreed. As long as there is a subsequent patch series that fixes\nthe `tmp_objdir` thing I'm happy. It'd be a no-op refactoring for now\nanyway given that we always write via the temporary object directory,\nbut it's a good cleanup to do on top.\n\nAnyway, I'm happy with the status quo of this patch series, thanks!\n\nPatrick\n"},{"id":"535118","messageId":"aYLmel06ANrskuG5@pks.im","threadId":"64882","inReplyTo":"xmqqfr7hqvzj.fsf@gitster.g","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-02-04T06:26:02Z","receivedAt":"2026-02-04T06:26:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Feb 03, 2026 at 02:54:08PM -0800, Junio C Hamano wrote:\n> Justin Tobler writes:\n> \n> > So long as `struct odb_transaction` remains the first member of `struct\n> > odb_transaction_files`, using `container_of()` is functionally the same\n> > thing as casting. This is also how we do things for `odb_read_stream`\n> > and `odb_loose_read_stream` so I don't think it is too big of a deal\n> > either way.\n> \n> It would be a good theme to clean them all up, together with\n> existing ones, in a separate topic. Code hygiene matters.\n\nFair indeed. Justin, will you handle this cleanup once this topic here\nis merged or shall I do it?\n\nPatrick\n"},{"id":"535127","messageId":"CAOLa=ZRb1eVSD42Obr_m+3KUy0Bh=0XmOZt8ofrbzy4Mp8xfwQ@mail.gmail.com","threadId":"64882","inReplyTo":"20260203001002.2500198-4-jltobler@gmail.com","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-04T10:31:45Z","receivedAt":"2026-02-04T10:31:48Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler writes:\n\n[snip]\n\n> +\n> +/*\n> + * A transaction may be started for an object database prior to writing new\n> + * objects via odb_transaction_begin(). These objects are not committed until\n> + * odb_transaction_commit() is invoked. Only a single transaction may be pending\n> + * at a time.\n> + *\n> + * Each ODB source is expected to implement its own transaction handling.\n> + */\n> struct odb_transaction;\n\nNit: Wouldn't it be nicer to just the below `struct odb_transaction`\nhere and drop this line?\n\n> +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);\n> +struct odb_transaction {\n> +\t/* The ODB source the transaction is opened against. */\n> +\tstruct odb_source *source;\n> +\n> +\t/* The ODB source specific callback invoked to commit a transaction. */\n> +\todb_transaction_commit_fn commit;\n> +};\n>\n> /*\n> * The object database encapsulates access to objects in a repository. It\n> --\n> 2.52.0.373.g68cb7f9e92\n\nJust a question in general, is the idea to eventually also add support\nfor {prepare, rollback} within odb transactions?\n"},{"id":"535128","messageId":"CAOLa=ZT_7o_YquQ_mAg6sn=gq0Rx4Tga4vNsVsPt3jCUh=3tzw@mail.gmail.com","threadId":"64882","inReplyTo":"20260203001002.2500198-5-jltobler@gmail.com","subject":"Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-04T10:34:20Z","receivedAt":"2026-02-04T10:34:22Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler writes:\n\n> A new ODB transaction is created and returned via\n> `odb_transaction_begin()` and stored in the ODB. Only a single\n> transaction may be pending at a time. If the ODB already has a\n> transaction, the function is expected to return NULL. Similarly, when\n> committing a transaction via `odb_transaction_commit()` the transaction\n> being committed must match the pending transaction and upon commit reset\n> the ODB transaction to NULL.\n>\n\nBut isn't this merely a limitation of the current implementation of the\nfiles transactions? Couldn't a potential ODB source support parallel\ntransactions where this might no longer hold?\n\n> These behaviors apply regardless of the ODB transaction implementation.\n> Move the corresponding logic into `odb_transaction_{begin,commit}()`\n> accordingly.\n>\n\n[snip]\n"},{"id":"535175","messageId":"aYN9p7crUDUlog8p@denethor","threadId":"64882","inReplyTo":"aYLmel06ANrskuG5@pks.im","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-04T17:15:15Z","receivedAt":"2026-02-04T17:15:17Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/04 07:26AM, Patrick Steinhardt wrote:\n> On Tue, Feb 03, 2026 at 02:54:08PM -0800, Junio C Hamano wrote:\n> > Justin Tobler writes:\n> > \n> > > So long as `struct odb_transaction` remains the first member of `struct\n> > > odb_transaction_files`, using `container_of()` is functionally the same\n> > > thing as casting. This is also how we do things for `odb_read_stream`\n> > > and `odb_loose_read_stream` so I don't think it is too big of a deal\n> > > either way.\n> > \n> > It would be a good theme to clean them all up, together with\n> > existing ones, in a separate topic. Code hygiene matters.\n> \n> Fair indeed. Justin, will you handle this cleanup once this topic here\n> is merged or shall I do it?\n\nI've already have a patch that addresses `odb_transaction_files`\nand `odb_loose_read_stream`. I'll check to see if there are any others I\nnotice and send a followup series after this one gets merged.\n\n-Justin\n"},{"id":"535182","messageId":"aYOCm3vzfDmnZhhu@denethor","threadId":"64882","inReplyTo":"CAOLa=ZRb1eVSD42Obr_m+3KUy0Bh=0XmOZt8ofrbzy4Mp8xfwQ@mail.gmail.com","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-04T17:38:55Z","receivedAt":"2026-02-04T17:38:59Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/04 02:31AM, Karthik Nayak wrote:\n> Justin Tobler writes:\n> \n> [snip]\n> \n> > +\n> > +/*\n> > + * A transaction may be started for an object database prior to writing new\n> > + * objects via odb_transaction_begin(). These objects are not committed until\n> > + * odb_transaction_commit() is invoked. Only a single transaction may be pending\n> > + * at a time.\n> > + *\n> > + * Each ODB source is expected to implement its own transaction handling.\n> > + */\n> > struct odb_transaction;\n> \n> Nit: Wouldn't it be nicer to just the below `struct odb_transaction`\n> here and drop this line?\n\nI assume you mean drop the typedef in favor of defining it directly in\nthe struct and thus removing the need for the forward declation. I kind\nof like having a typedef for the function callback, but I don't feel too\nstrongly either way. I've ammended locally, but will hold off from\nsending another version unless there is other feedback.\n\n> > +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);\n> > +struct odb_transaction {\n> > +\t/* The ODB source the transaction is opened against. */\n> > +\tstruct odb_source *source;\n> > +\n> > +\t/* The ODB source specific callback invoked to commit a transaction. */\n> > +\todb_transaction_commit_fn commit;\n> > +};\n> >\n> > /*\n> > * The object database encapsulates access to objects in a repository. It\n> > --\n> > 2.52.0.373.g68cb7f9e92\n> \n> Just a question in general, is the idea to eventually also add support\n> for {prepare, rollback} within odb transactions?\n\nI'm not quite sure yet about \"prepare\", but certainly an \"abort\" or\n\"rollback\" will be introduced in a followup series. This will be useful\nas we expand ODB transaction usage to other operations that require the\nability to remove temporary objects such as remerge-diffs.\n\nThanks,\n-Justin\n"},{"id":"535183","messageId":"aYOEQUIPXPIYeCw-@denethor","threadId":"64882","inReplyTo":"CAOLa=ZT_7o_YquQ_mAg6sn=gq0Rx4Tga4vNsVsPt3jCUh=3tzw@mail.gmail.com","subject":"Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2026-02-04T17:50:29Z","receivedAt":"2026-02-04T17:50:33Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 26/02/04 10:34AM, Karthik Nayak wrote:\n> Justin Tobler writes:\n> \n> > A new ODB transaction is created and returned via\n> > `odb_transaction_begin()` and stored in the ODB. Only a single\n> > transaction may be pending at a time. If the ODB already has a\n> > transaction, the function is expected to return NULL. Similarly, when\n> > committing a transaction via `odb_transaction_commit()` the transaction\n> > being committed must match the pending transaction and upon commit reset\n> > the ODB transaction to NULL.\n> \n> But isn't this merely a limitation of the current implementation of the\n> files transactions? Couldn't a potential ODB source support parallel\n> transactions where this might no longer hold?\n\nJust to clarify, this limitation exists per Git process. For the time\nbeing, we only support writing objects to a single ODB source so a\nsingle transaction for object writes seems reasonable for now.\nFurthermore, the current \"files\" transaction backend relies on the\ntmp_odjdir subsystem which means only a single temp odjdir may exist for\na Git process to write objects to. \n\n-Justin\n"},{"id":"535229","messageId":"CAOLa=ZSdU5jRqQhaehvqnVebcXT8TiMqCdSSqFYEh5XVJtrNdA@mail.gmail.com","threadId":"64882","inReplyTo":"aYOCm3vzfDmnZhhu@denethor","subject":"Re: [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-05T11:20:57Z","receivedAt":"2026-02-05T11:21:00Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler writes:\n\n> On 26/02/04 02:31AM, Karthik Nayak wrote:\n>> Justin Tobler writes:\n>>\n>> [snip]\n>>\n>> > +\n>> > +/*\n>> > + * A transaction may be started for an object database prior to writing new\n>> > + * objects via odb_transaction_begin(). These objects are not committed until\n>> > + * odb_transaction_commit() is invoked. Only a single transaction may be pending\n>> > + * at a time.\n>> > + *\n>> > + * Each ODB source is expected to implement its own transaction handling.\n>> > + */\n>> > struct odb_transaction;\n>>\n>> Nit: Wouldn't it be nicer to just the below `struct odb_transaction`\n>> here and drop this line?\n>\n> I assume you mean drop the typedef in favor of defining it directly in\n> the struct and thus removing the need for the forward declation. I kind\n> of like having a typedef for the function callback, but I don't feel too\n> strongly either way. I've ammended locally, but will hold off from\n> sending another version unless there is other feedback.\n>\n\nAll good.\n\n>> > +typedef void (*odb_transaction_commit_fn)(struct odb_transaction *transaction);\n>> > +struct odb_transaction {\n>> > +\t/* The ODB source the transaction is opened against. */\n>> > +\tstruct odb_source *source;\n>> > +\n>> > +\t/* The ODB source specific callback invoked to commit a transaction. */\n>> > +\todb_transaction_commit_fn commit;\n>> > +};\n>> >\n>> > /*\n>> > * The object database encapsulates access to objects in a repository. It\n>> > --\n>> > 2.52.0.373.g68cb7f9e92\n>>\n>> Just a question in general, is the idea to eventually also add support\n>> for {prepare, rollback} within odb transactions?\n>\n> I'm not quite sure yet about \"prepare\", but certainly an \"abort\" or\n> \"rollback\" will be introduced in a followup series. This will be useful\n> as we expand ODB transaction usage to other operations that require the\n> ability to remove temporary objects such as remerge-diffs.\n>\n\nAlright.\n"},{"id":"535230","messageId":"CAOLa=ZQyW4VPuYC+isjx7h1m5-vix0N9M5+TsXocHaFU+Aydbg@mail.gmail.com","threadId":"64882","inReplyTo":"aYOEQUIPXPIYeCw-@denethor","subject":"Re: [PATCH v2 4/4] odb: transparently handle common transaction behavior","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-05T11:22:21Z","receivedAt":"2026-02-05T11:22:23Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler writes:\n\n> On 26/02/04 10:34AM, Karthik Nayak wrote:\n>> Justin Tobler writes:\n>>\n>> > A new ODB transaction is created and returned via\n>> > `odb_transaction_begin()` and stored in the ODB. Only a single\n>> > transaction may be pending at a time. If the ODB already has a\n>> > transaction, the function is expected to return NULL. Similarly, when\n>> > committing a transaction via `odb_transaction_commit()` the transaction\n>> > being committed must match the pending transaction and upon commit reset\n>> > the ODB transaction to NULL.\n>>\n>> But isn't this merely a limitation of the current implementation of the\n>> files transactions? Couldn't a potential ODB source support parallel\n>> transactions where this might no longer hold?\n>\n> Just to clarify, this limitation exists per Git process.\n\nI missed this, makes sense now.\n\n> For the time\n> being, we only support writing objects to a single ODB source so a\n> single transaction for object writes seems reasonable for now.\n> Furthermore, the current \"files\" transaction backend relies on the\n> tmp_odjdir subsystem which means only a single temp odjdir may exist for\n> a Git process to write objects to.\n>\n> -Justin\n\nThanks\n"}]}