Volume XXII, number 279Tuesday, October 6, 2026Latest message 45 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patch, 9 partsobject-file: move writing of loose objects into "loose" source

17 messages between Jul 17, 2026 and Jul 28, 2026, from Patrick Steinhardt, SZEDER Gábor, Junio C Hamano, Toon Claes, Justin Tobler.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Patrick SteinhardtJul 17, 2026, 09:32 UTC on lore
Hi,

this patch series refactors "object-file.c" so that we can move the logic that writes loose objects into "odb/source-loose.c". This ensures that the logic to read and write loose objects is mostly self-contained within that source's implementation now.

To achieve that, this series first refactors `force_object_loose()` so that it can work on top of the generic `odb_source_write_object()` instead of having to call into `write_loose_object()`. This is the bulk of the patch series.

This patch series is built on top of 44de1520f0 (Merge branch 'master' of https://github.com/j6t/git-gui, 2026-07-16) with the following two topics merged into it:

  - jt/receive-pack-use-odb-transaction at bdee7b3013
    (builtin/receive-pack: stage incoming objects via ODB transactions,
    2026-07-10).
  - jk/git-hash-cleanups at 9e396aa553 (hash: check ctx->active flag in
    all wrapper functions, 2026-07-07).
Note that jt/receive-pack-use-odb-transaction requires an evil merge:
Show changes to odb/source-packed.c +2 −1
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 06b31dd743..cbb06da038 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -545,7 +545,8 @@ static int odb_source_packed_write_object_stream(struct odb_source *source UNUSE
 }

 static int odb_source_packed_begin_transaction(struct odb_source *source UNUSED,
-                                              struct odb_transaction **out UNUSED)
+                                              struct odb_transaction **out UNUSED,
+                                              enum odb_transaction_flags flags UNUSED)
 {
        return error("packed backend cannot begin transactions");
 }

Thanks!

Patrick

---
Patrick Steinhardt (9):
      odb: compute compat object ID in `odb_write_object_ext()`
      t/u-odb-inmemory: implement wrapper for writing objects
      odb: compute object hash in `odb_write_object_ext()`
      odb: lift object existence check out of the "loose" backend
      odb: support setting mtime when writing objects
      object-file: fix memory leak in `force_object_loose()`
      object-file: force objects loose via generic interface
      object-file: move `force_object_loose()`
      object-file: move logic to write loose objects

 builtin/pack-objects.c        |  48 ++++-
 object-file.c                 | 449 +++---------------------------------------
 object-file.h                 |  32 +--
 odb.c                         |  39 +++-
 odb.h                         |  10 +-
 odb/source-files.c            |  14 +-
 odb/source-inmemory.c         |  14 +-
 odb/source-loose.c            | 402 +++++++++++++++++++++++++++++++++----
 odb/source-packed.c           |  17 +-
 odb/source.h                  |  20 +-
 read-cache.c                  |   2 +-
 t/unit-tests/u-odb-inmemory.c |  53 +++--
 12 files changed, 551 insertions(+), 549 deletions(-)


---
base-commit: 8061f1ea286318fbf976f58df3985bd60237b8a7
change-id: 20260703-pks-odb-move-loose-object-writing-8bcf1f51b04d
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 1/9] odb: compute compat object ID in `odb_write_object_ext()`

Repositories can have a compatibility hash configured, which means that such a repository is expected to maintain a mapping between canonical and compatibility object hashes. Maintaining this mapping is the responsibility of the object database sources, where we either store them as part of the loose objects map or in packfile indices v3 (once we gain support for this feature).

But besides storing these compatibility hashes, the sources are also responsible for generating the compatibility hash in the first place. This is somewhat unnecessary though, as the compatibility hash should be computed the same no matter which source is being used. The consequence is that we need to duplicate this functionality across the different backends, which does not make a lot of sense.

Refactor the code so that we instead compute the compatibility hash in `odb_write_object_ext()` and then pass the computed value to the sources. No callers need adjustment as there are none that write objects via the source interfaces directly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c                 | 26 ++++++++++++++++++++++++--
 odb.h                 | 10 ++++++----
 odb/source-files.c    |  2 +-
 odb/source-inmemory.c |  2 +-
 odb/source-loose.c    | 24 +++---------------------
 odb/source-packed.c   |  2 +-
 odb/source.h          |  4 ++--
 7 files changed, 38 insertions(+), 32 deletions(-)
Show changes to 7 files +38 −32

odb.c, odb.h, odb/source-files.c, odb/source-inmemory.c, odb/source-loose.c, odb/source-packed.c, odb/source.h

diff --git a/odb.c b/odb.c
index cf6e7938c0..1d6538163b 100644
--- a/odb.c
+++ b/odb.c
@@ -989,11 +989,33 @@ int odb_write_object_ext(struct object_database *odb,
 			 const void *buf, unsigned long len,
 			 enum object_type type,
 			 struct object_id *oid,
-			 struct object_id *compat_oid,
+			 const struct object_id *compat_oid_in,
 			 enum odb_write_object_flags flags)
 {
+	const struct git_hash_algo *compat = odb->repo->compat_hash_algo;
+	struct object_id compat_oid, *compat_oid_p = NULL;
+
+	if (compat) {
+		const struct git_hash_algo *algo = odb->repo->hash_algo;
+
+		if (compat_oid_in) {
+			oidcpy(&compat_oid, compat_oid_in);
+		} else if (type == OBJ_BLOB) {
+			hash_object_file(compat, buf, len, type, &compat_oid);
+		} else {
+			struct strbuf converted = STRBUF_INIT;
+			convert_object_file(odb->repo, &converted, algo, compat,
+					    buf, len, type, 0);
+			hash_object_file(compat, converted.buf, converted.len,
+					 type, &compat_oid);
+			strbuf_release(&converted);
+		}
+
+		compat_oid_p = &compat_oid;
+	}
+
 	return odb_source_write_object(odb->sources, buf, len, type,
-				       oid, compat_oid, flags);
+				       oid, compat_oid_p, flags);
 }
 
 int odb_write_object_stream(struct object_database *odb,
diff --git a/odb.h b/odb.h
index 94754643d2..066560113e 100644
--- a/odb.h
+++ b/odb.h
@@ -585,9 +585,11 @@ enum odb_write_object_flags {
 
 /*
  * Write an object into the object database. The object is being written into
- * the local alternate of the repository. If provided, the converted object ID
- * as well as the compatibility object ID are written to the respective
- * pointers.
+ * the local alternate of the repository. If provided, the object ID of the
+ * final object is written into `oid`.
+ *
+ * If the caller provides a `compat_oid`, then this compatibility object hash
+ * will be stored instead of computing the compatibility hash ad-hoc.
  *
  * Returns 0 on success, a negative error code otherwise.
  */
@@ -595,7 +597,7 @@ int odb_write_object_ext(struct object_database *odb,
 			 const void *buf, unsigned long len,
 			 enum object_type type,
 			 struct object_id *oid,
-			 struct object_id *compat_oid,
+			 const struct object_id *compat_oid,
 			 enum odb_write_object_flags flags);
 
 static inline int odb_write_object(struct object_database *odb,
diff --git a/odb/source-files.c b/odb/source-files.c
index 4138758511..3d9f5eca32 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -163,7 +163,7 @@ static int odb_source_files_write_object(struct odb_source *source,
 					 const void *buf, size_t len,
 					 enum object_type type,
 					 struct object_id *oid,
-					 struct object_id *compat_oid,
+					 const struct object_id *compat_oid,
 					 enum odb_write_object_flags flags)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index e47bfd8fcc..e727aba427 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -231,7 +231,7 @@ static int odb_source_inmemory_write_object(struct odb_source *source,
 					    const void *buf, size_t len,
 					    enum object_type type,
 					    struct object_id *oid,
-					    struct object_id *compat_oid UNUSED,
+					    const struct object_id *compat_oid UNUSED,
 					    enum odb_write_object_flags flags UNUSED)
 {
 	struct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 3f7d04a56e..ca223109cd 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -585,32 +585,14 @@ static int odb_source_loose_freshen_object(struct odb_source *source,
 static int odb_source_loose_write_object(struct odb_source *source,
 					 const void *buf, size_t len,
 					 enum object_type type, struct object_id *oid,
-					 struct object_id *compat_oid_in,
+					 const struct object_id *compat_oid,
 					 enum odb_write_object_flags flags)
 {
 	struct odb_source_loose *loose = odb_source_loose_downcast(source);
 	const struct git_hash_algo *algo = source->odb->repo->hash_algo;
-	const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
-	struct object_id compat_oid;
 	char hdr[MAX_HEADER_LEN];
 	size_t hdrlen = sizeof(hdr);
 
-	/* Generate compat_oid */
-	if (compat) {
-		if (compat_oid_in)
-			oidcpy(&compat_oid, compat_oid_in);
-		else if (type == OBJ_BLOB)
-			hash_object_file(compat, buf, len, type, &compat_oid);
-		else {
-			struct strbuf converted = STRBUF_INIT;
-			convert_object_file(source->odb->repo, &converted, algo, compat,
-					    buf, len, type, 0);
-			hash_object_file(compat, converted.buf, converted.len,
-					 type, &compat_oid);
-			strbuf_release(&converted);
-		}
-	}
-
 	/* Normally if we have it in the pack then we do not bother writing
 	 * it out into .git/objects/??/?{38} file.
 	 */
@@ -619,8 +601,8 @@ static int odb_source_loose_write_object(struct odb_source *source,
 		return 0;
 	if (write_loose_object(loose, oid, hdr, hdrlen, buf, len, 0, flags))
 		return -1;
-	if (compat)
-		return repo_add_loose_object_map(loose, oid, &compat_oid);
+	if (compat_oid)
+		return repo_add_loose_object_map(loose, oid, compat_oid);
 	return 0;
 }
 
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 8d9ce197cc..af0d533375 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -530,7 +530,7 @@ static int odb_source_packed_write_object(struct odb_source *source UNUSED,
 					  size_t len UNUSED,
 					  enum object_type type UNUSED,
 					  struct object_id *oid UNUSED,
-					  struct object_id *compat_oid UNUSED,
+					  const struct object_id *compat_oid UNUSED,
 					  unsigned flags UNUSED)
 {
 	return error("packed backend cannot write objects");
diff --git a/odb/source.h b/odb/source.h
index cd63dba91f..b3c1ca3a66 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -207,7 +207,7 @@ struct odb_source {
 			    const void *buf, size_t len,
 			    enum object_type type,
 			    struct object_id *oid,
-			    struct object_id *compat_oid,
+			    const struct object_id *compat_oid,
 			    enum odb_write_object_flags flags);
 
 	/*
@@ -417,7 +417,7 @@ static inline int odb_source_write_object(struct odb_source *source,
 					  const void *buf, unsigned long len,
 					  enum object_type type,
 					  struct object_id *oid,
-					  struct object_id *compat_oid,
+					  const struct object_id *compat_oid,
 					  enum odb_write_object_flags flags)
 {
 	return source->write_object(source, buf, len, type, oid,
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 2/9] t/u-odb-inmemory: implement wrapper for writing objects

In the next commit we're about to change how objects are being written into the object database source. Prepare for this refactoring by introducing a wrapper function into our unit tests so that we don't have to adjust all callsites.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/unit-tests/u-odb-inmemory.c | 48 +++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 25 deletions(-)
Show changes to t/unit-tests/u-odb-inmemory.c +23 −25
diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c
index 6844bfc37c..2dbc3ab1df 100644
--- a/t/unit-tests/u-odb-inmemory.c
+++ b/t/unit-tests/u-odb-inmemory.c
@@ -1,5 +1,6 @@
 #include "unit-test.h"
 #include "hex.h"
+#include "object-file.h"
 #include "odb/source-inmemory.h"
 #include "odb/streaming.h"
 #include "oidset.h"
@@ -36,6 +37,16 @@ static void cl_assert_object_info(struct odb_source_inmemory *source,
 	free(actual_content);
 }
 
+static void cl_assert_write_object(struct odb_source_inmemory *source,
+				   const char *content,
+				   enum object_type type,
+				   struct object_id *oid)
+{
+	size_t content_len = strlen(content);
+	cl_must_pass(odb_source_write_object(&source->base, content, content_len,
+					     type, oid, NULL, 0));
+}
+
 void test_odb_inmemory__initialize(void)
 {
 	odb = odb_new(&repo, "", "");
@@ -78,8 +89,7 @@ void test_odb_inmemory__read_written_object(void)
 	const char data[] = "foobar";
 	struct object_id written_oid;
 
-	cl_must_pass(odb_source_write_object(&source->base, data, strlen(data),
-					     OBJ_BLOB, &written_oid, NULL, 0));
+	cl_assert_write_object(source, data, OBJ_BLOB, &written_oid);
 	cl_assert_equal_s(oid_to_hex(&written_oid), FOOBAR_OID);
 	cl_assert_object_info(source, &written_oid, OBJ_BLOB, "foobar");
 
@@ -94,8 +104,7 @@ void test_odb_inmemory__read_stream_object(void)
 	const char data[] = "foobar";
 	char buf[3] = { 0 };
 
-	cl_must_pass(odb_source_write_object(&source->base, data, strlen(data),
-					     OBJ_BLOB, &written_oid, NULL, 0));
+	cl_assert_write_object(source, data, OBJ_BLOB, &written_oid);
 
 	cl_must_pass(odb_source_read_object_stream(&stream, &source->base,
 						   &written_oid));
@@ -141,8 +150,7 @@ void test_odb_inmemory__for_each_object(void)
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "%d", i);
 
-		cl_must_pass(odb_source_write_object(&source->base, buf.buf, buf.len,
-						     OBJ_BLOB, &written_oid, NULL, 0));
+		cl_assert_write_object(source, buf.buf, OBJ_BLOB, &written_oid);
 		cl_must_pass(oidset_insert(&expected_oids, &written_oid));
 	}
 
@@ -174,12 +182,9 @@ void test_odb_inmemory__for_each_object_can_abort_iteration(void)
 	struct object_id written_oid;
 	unsigned counter = 0;
 
-	cl_must_pass(odb_source_write_object(&source->base, "1", 1,
-					     OBJ_BLOB, &written_oid, NULL, 0));
-	cl_must_pass(odb_source_write_object(&source->base, "2", 1,
-					     OBJ_BLOB, &written_oid, NULL, 0));
-	cl_must_pass(odb_source_write_object(&source->base, "3", 1,
-					     OBJ_BLOB, &written_oid, NULL, 0));
+	cl_assert_write_object(source, "1", OBJ_BLOB, &written_oid);
+	cl_assert_write_object(source, "2", OBJ_BLOB, &written_oid);
+	cl_assert_write_object(source, "3", OBJ_BLOB, &written_oid);
 
 	cl_assert_equal_i(odb_source_for_each_object(&source->base, NULL,
 						     abort_after_two_objects,
@@ -199,12 +204,9 @@ void test_odb_inmemory__count_objects(void)
 	cl_must_pass(odb_source_count_objects(&source->base, 0, &count));
 	cl_assert_equal_u(count, 0);
 
-	cl_must_pass(odb_source_write_object(&source->base, "1", 1,
-					     OBJ_BLOB, &written_oid, NULL, 0));
-	cl_must_pass(odb_source_write_object(&source->base, "2", 1,
-					     OBJ_BLOB, &written_oid, NULL, 0));
-	cl_must_pass(odb_source_write_object(&source->base, "3", 1,
-					     OBJ_BLOB, &written_oid, NULL, 0));
+	cl_assert_write_object(source, "1", OBJ_BLOB, &written_oid);
+	cl_assert_write_object(source, "2", OBJ_BLOB, &written_oid);
+	cl_assert_write_object(source, "3", OBJ_BLOB, &written_oid);
 
 	cl_must_pass(odb_source_count_objects(&source->base, 0, &count));
 	cl_assert_equal_u(count, 3);
@@ -228,8 +230,7 @@ void test_odb_inmemory__find_abbrev_len(void)
 	 *
 	 * With only one blob written we expect a length of 4.
 	 */
-	cl_must_pass(odb_source_write_object(&source->base, "368317", strlen("368317"),
-					     OBJ_BLOB, &oid1, NULL, 0));
+	cl_assert_write_object(source, "368317", OBJ_BLOB, &oid1);
 	cl_must_pass(odb_source_find_abbrev_len(&source->base, &oid1, 4,
 						&abbrev_len));
 	cl_assert_equal_u(abbrev_len, 4);
@@ -238,8 +239,7 @@ void test_odb_inmemory__find_abbrev_len(void)
 	 * With both objects present, the shared 10-character prefix means we
 	 * need at least 11 characters to uniquely identify either object.
 	 */
-	cl_must_pass(odb_source_write_object(&source->base, "514796", strlen("514796"),
-					     OBJ_BLOB, &oid2, NULL, 0));
+	cl_assert_write_object(source, "514796", OBJ_BLOB, &oid2);
 	cl_must_pass(odb_source_find_abbrev_len(&source->base, &oid1, 4,
 						&abbrev_len));
 	cl_assert_equal_u(abbrev_len, 11);
@@ -257,9 +257,7 @@ void test_odb_inmemory__freshen_object(void)
 	cl_must_pass(parse_oid_hex_algop(RANDOM_OID, &oid, &end, repo.hash_algo));
 	cl_assert_equal_i(odb_source_freshen_object(&source->base, &oid), 0);
 
-	cl_must_pass(odb_source_write_object(&source->base, "foobar",
-					     strlen("foobar"), OBJ_BLOB,
-					     &written_oid, NULL, 0));
+	cl_assert_write_object(source, "foobar", OBJ_BLOB, &written_oid);
 	cl_assert_equal_i(odb_source_freshen_object(&source->base,
 						    &written_oid), 1);
 
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 3/9] odb: compute object hash in `odb_write_object_ext()`

Same as in a preceding commit, compute the object hash in `odb_write_object_ext()` so that we can unify this logic.

Besides unification, this change also allows us to lift the object existence check out of the "loose" backend into the generic layer, which will happen in the next commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 object-file.c                 | 35 ++++++++---------------------------
 object-file.h                 |  4 ----
 odb.c                         |  2 ++
 odb/source-files.c            |  2 +-
 odb/source-inmemory.c         |  6 +++---
 odb/source-loose.c            | 12 +++++++-----
 odb/source-packed.c           |  2 +-
 odb/source.h                  |  4 ++--
 t/unit-tests/u-odb-inmemory.c |  1 +
 9 files changed, 25 insertions(+), 43 deletions(-)
Show changes to 9 files +25 −43

object-file.c, object-file.h, odb.c, odb/source-files.c, odb/source-inmemory.c, odb/source-loose.c, odb/source-packed.c, odb/source.h, t/unit-tests/u-odb-inmemory.c

diff --git a/object-file.c b/object-file.c
index 5283292f1e..9ca14f484d 100644
--- a/object-file.c
+++ b/object-file.c
@@ -316,31 +316,6 @@ int parse_loose_header(const char *hdr, struct object_info *oi)
 	return 0;
 }
 
-static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_ctx *c,
-			     const void *buf, size_t len,
-			     struct object_id *oid,
-			     char *hdr, size_t *hdrlen)
-{
-	git_hash_init(c, algo);
-	git_hash_update(c, hdr, *hdrlen);
-	git_hash_update(c, buf, len);
-	git_hash_final_oid(oid, c);
-}
-
-void write_object_file_prepare(const struct git_hash_algo *algo,
-			       const void *buf, size_t len,
-			       enum object_type type, struct object_id *oid,
-			       char *hdr, size_t *hdrlen)
-{
-	struct git_hash_ctx c;
-
-	/* Generate the header */
-	*hdrlen = format_object_header(hdr, *hdrlen, type, len);
-
-	/* Hash (function pointers) computation */
-	hash_object_body(algo, &c, buf, len, oid, hdr, hdrlen);
-}
-
 #define CHECK_COLLISION_DEST_VANISHED -2
 
 static int check_collision(const char *source, const char *dest)
@@ -476,10 +451,16 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 		      size_t len, enum object_type type,
 		      struct object_id *oid)
 {
+	struct git_hash_ctx c;
 	char hdr[MAX_HEADER_LEN];
-	size_t hdrlen = sizeof(hdr);
+	int hdrlen;
+
+	hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
 
-	write_object_file_prepare(algo, buf, len, type, oid, hdr, &hdrlen);
+	git_hash_init(&c, algo);
+	git_hash_update(&c, hdr, hdrlen);
+	git_hash_update(&c, buf, len);
+	git_hash_final_oid(oid, &c);
 }
 
 struct transaction_packfile {
diff --git a/object-file.h b/object-file.h
index d04ffa6493..08aafcda0d 100644
--- a/object-file.h
+++ b/object-file.h
@@ -134,10 +134,6 @@ int finalize_object_file_flags(struct repository *repo,
 void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 		      size_t len, enum object_type type,
 		      struct object_id *oid);
-void write_object_file_prepare(const struct git_hash_algo *algo,
-			       const void *buf, size_t len,
-			       enum object_type type, struct object_id *oid,
-			       char *hdr, size_t *hdrlen);
 int write_loose_object(struct odb_source_loose *loose,
 		       const struct object_id *oid, char *hdr,
 		       int hdrlen, const void *buf, unsigned long len,
diff --git a/odb.c b/odb.c
index 1d6538163b..4adbdf8a64 100644
--- a/odb.c
+++ b/odb.c
@@ -995,6 +995,8 @@ int odb_write_object_ext(struct object_database *odb,
 	const struct git_hash_algo *compat = odb->repo->compat_hash_algo;
 	struct object_id compat_oid, *compat_oid_p = NULL;
 
+	hash_object_file(odb->repo->hash_algo, buf, len, type, oid);
+
 	if (compat) {
 		const struct git_hash_algo *algo = odb->repo->hash_algo;
 
diff --git a/odb/source-files.c b/odb/source-files.c
index 3d9f5eca32..06dfc8dd78 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -162,7 +162,7 @@ static int odb_source_files_freshen_object(struct odb_source *source,
 static int odb_source_files_write_object(struct odb_source *source,
 					 const void *buf, size_t len,
 					 enum object_type type,
-					 struct object_id *oid,
+					 const struct object_id *oid,
 					 const struct object_id *compat_oid,
 					 enum odb_write_object_flags flags)
 {
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index e727aba427..963d520317 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -230,15 +230,13 @@ static int odb_source_inmemory_count_objects(struct odb_source *source,
 static int odb_source_inmemory_write_object(struct odb_source *source,
 					    const void *buf, size_t len,
 					    enum object_type type,
-					    struct object_id *oid,
+					    const struct object_id *oid,
 					    const struct object_id *compat_oid UNUSED,
 					    enum odb_write_object_flags flags UNUSED)
 {
 	struct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);
 	struct inmemory_object *object;
 
-	hash_object_file(source->odb->repo->hash_algo, buf, len, type, oid);
-
 	if (!inmemory->objects) {
 		CALLOC_ARRAY(inmemory->objects, 1);
 		oidtree_init(inmemory->objects);
@@ -285,6 +283,8 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source,
 		goto out;
 	}
 
+	hash_object_file(source->odb->repo->hash_algo, data, total_read, OBJ_BLOB, oid);
+
 	ret = odb_source_inmemory_write_object(source, data, len, OBJ_BLOB, oid,
 					       NULL, 0);
 	if (ret < 0)
diff --git a/odb/source-loose.c b/odb/source-loose.c
index ca223109cd..d4715da6d1 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -584,19 +584,21 @@ static int odb_source_loose_freshen_object(struct odb_source *source,
 
 static int odb_source_loose_write_object(struct odb_source *source,
 					 const void *buf, size_t len,
-					 enum object_type type, struct object_id *oid,
+					 enum object_type type,
+					 const struct object_id *oid,
 					 const struct object_id *compat_oid,
 					 enum odb_write_object_flags flags)
 {
 	struct odb_source_loose *loose = odb_source_loose_downcast(source);
-	const struct git_hash_algo *algo = source->odb->repo->hash_algo;
 	char hdr[MAX_HEADER_LEN];
-	size_t hdrlen = sizeof(hdr);
+	int hdrlen;
+
+	hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
 
-	/* Normally if we have it in the pack then we do not bother writing
+	/*
+	 * Normally if we have it in the pack then we do not bother writing
 	 * it out into .git/objects/??/?{38} file.
 	 */
-	write_object_file_prepare(algo, buf, len, type, oid, hdr, &hdrlen);
 	if (odb_freshen_object(source->odb, oid))
 		return 0;
 	if (write_loose_object(loose, oid, hdr, hdrlen, buf, len, 0, flags))
diff --git a/odb/source-packed.c b/odb/source-packed.c
index af0d533375..f7f1706447 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -529,7 +529,7 @@ static int odb_source_packed_write_object(struct odb_source *source UNUSED,
 					  const void *buf UNUSED,
 					  size_t len UNUSED,
 					  enum object_type type UNUSED,
-					  struct object_id *oid UNUSED,
+					  const struct object_id *oid UNUSED,
 					  const struct object_id *compat_oid UNUSED,
 					  unsigned flags UNUSED)
 {
diff --git a/odb/source.h b/odb/source.h
index b3c1ca3a66..c4e94c9d0d 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -206,7 +206,7 @@ struct odb_source {
 	int (*write_object)(struct odb_source *source,
 			    const void *buf, size_t len,
 			    enum object_type type,
-			    struct object_id *oid,
+			    const struct object_id *oid,
 			    const struct object_id *compat_oid,
 			    enum odb_write_object_flags flags);
 
@@ -416,7 +416,7 @@ static inline int odb_source_freshen_object(struct odb_source *source,
 static inline int odb_source_write_object(struct odb_source *source,
 					  const void *buf, unsigned long len,
 					  enum object_type type,
-					  struct object_id *oid,
+					  const struct object_id *oid,
 					  const struct object_id *compat_oid,
 					  enum odb_write_object_flags flags)
 {
diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c
index 2dbc3ab1df..28a69fc244 100644
--- a/t/unit-tests/u-odb-inmemory.c
+++ b/t/unit-tests/u-odb-inmemory.c
@@ -43,6 +43,7 @@ static void cl_assert_write_object(struct odb_source_inmemory *source,
 				   struct object_id *oid)
 {
 	size_t content_len = strlen(content);
+	hash_object_file(repo.hash_algo, content, content_len, type, oid);
 	cl_must_pass(odb_source_write_object(&source->base, content, content_len,
 					     type, oid, NULL, 0));
 }
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 4/9] odb: lift object existence check out of the "loose" backend

Before writing a new loose object we first check whether the object already exists in any of the sources attached to the object database. This results in a couple of issues:

  - We have a layering violation, where the source needs to be aware of
    objects stored in any of the other sources.
  - Every backend would have to reimplement this check, which feels
    somewhat pointless.
  - It is not possible to easily write an object into a source in case
    the same object already exists in another source.

Refactor the code and lift up the object existence check from the "loose" backend into the generic ODB layer. No callers need adjustment as none of them write via a specific source, but via the ODB layer.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 odb.c              | 7 +++++++
 odb/source-loose.c | 8 ++------
 2 files changed, 9 insertions(+), 6 deletions(-)
Show changes to 2 files +9 −6

odb.c, odb/source-loose.c

diff --git a/odb.c b/odb.c
index 4adbdf8a64..bfeca76f4e 100644
--- a/odb.c
+++ b/odb.c
@@ -997,6 +997,13 @@ int odb_write_object_ext(struct object_database *odb,
 
 	hash_object_file(odb->repo->hash_algo, buf, len, type, oid);
 
+	/*
+	 * We can skip the write in case we already have the object available.
+	 * In that case, we only freshen its mtime.
+	 */
+	if (odb_freshen_object(odb, oid))
+		return 0;
+
 	if (compat) {
 		const struct git_hash_algo *algo = odb->repo->hash_algo;
 
diff --git a/odb/source-loose.c b/odb/source-loose.c
index d4715da6d1..04af1a54a3 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -595,16 +595,12 @@ static int odb_source_loose_write_object(struct odb_source *source,
 
 	hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
 
-	/*
-	 * Normally if we have it in the pack then we do not bother writing
-	 * it out into .git/objects/??/?{38} file.
-	 */
-	if (odb_freshen_object(source->odb, oid))
-		return 0;
 	if (write_loose_object(loose, oid, hdr, hdrlen, buf, len, 0, flags))
 		return -1;
+
 	if (compat_oid)
 		return repo_add_loose_object_map(loose, oid, compat_oid);
+
 	return 0;
 }
 
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 5/9] odb: support setting mtime when writing objects

The function `force_object_loose()` is used to loosen packed objects before repacking. It passes the pack's mtime along so that the newly written loose object inherits the same timestamp. This matters for object pruning, which uses the mtime to determine whether an object is old enough to be pruned.

In a subsequent commit, `force_object_loose()` will be converted to use the generic `odb_source_write_object()` interface instead of calling `write_loose_object()` directly. But the generic interface doesn't yet support setting a specific mtime, which makes it impossible to implement the logic as of now.

Prepare for the change by introducing a new `mtime` parameter to this function that we plumb through the stack. If set, the backends are instructed to set the object's mtime accordingly. If unset, the backends are expected to use the current time instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c        |  2 +-
 object-file.c                 | 29 ++++++++++++++++++++---------
 object-file.h                 |  8 +++++---
 odb.c                         |  6 +++---
 odb/source-files.c            | 10 ++++++----
 odb/source-inmemory.c         |  6 ++++--
 odb/source-loose.c            |  8 +++++---
 odb/source-packed.c           | 13 +++++++++++--
 odb/source.h                  | 12 ++++++++----
 read-cache.c                  |  2 +-
 t/unit-tests/u-odb-inmemory.c |  6 +++---
 11 files changed, 67 insertions(+), 35 deletions(-)
Show changes to 11 files +67 −35

builtin/pack-objects.c, object-file.c, object-file.h, odb.c, odb/source-files.c, odb/source-inmemory.c, odb/source-loose.c, odb/source-packed.c, odb/source.h, read-cache.c, t/unit-tests/u-odb-inmemory.c

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ea5eab4cf8..e64a96f1a7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4642,7 +4642,7 @@ static void loosen_unused_packed_objects(void)
 			    !has_sha1_pack_kept_or_nonlocal(&oid) &&
 			    !loosened_object_can_be_discarded(&oid, p->mtime)) {
 				if (force_object_loose(the_repository->objects->sources,
-						       &oid, p->mtime))
+						       &oid, &p->mtime))
 					die(_("unable to force loose object"));
 				loosened_objects_nr++;
 			}
diff --git a/object-file.c b/object-file.c
index 9ca14f484d..5b07530950 100644
--- a/object-file.c
+++ b/object-file.c
@@ -67,9 +67,17 @@ const char *odb_loose_path(struct odb_source_loose *loose,
 }
 
 /* Returns 1 if we have successfully freshened the file, 0 otherwise. */
-static int freshen_file(const char *fn)
+static int freshen_file(const char *fn, const time_t *mtime)
 {
-	return !utime(fn, NULL);
+	struct utimbuf times, *timesp = NULL;
+
+	if (mtime) {
+		times.actime = *mtime;
+		times.modtime = *mtime;
+		timesp = &times;
+	}
+
+	return !utime(fn, timesp);
 }
 
 /*
@@ -79,11 +87,12 @@ static int freshen_file(const char *fn)
  * either does not exist on disk, or has a stale mtime and may be subject to
  * pruning).
  */
-int check_and_freshen_file(const char *fn, int freshen)
+int check_and_freshen_file(const char *fn, int freshen,
+			   const time_t *mtime)
 {
 	if (access(fn, F_OK))
 		return 0;
-	if (freshen && !freshen_file(fn))
+	if (freshen && !freshen_file(fn, mtime))
 		return 0;
 	return 1;
 }
@@ -706,7 +715,7 @@ static int end_loose_object_common(struct odb_source_loose *loose,
 int write_loose_object(struct odb_source_loose *loose,
 		       const struct object_id *oid, char *hdr,
 		       int hdrlen, const void *buf, unsigned long len,
-		       time_t mtime, unsigned flags)
+		       const time_t *mtime, unsigned flags)
 {
 	int fd, ret;
 	unsigned char compressed[4096];
@@ -751,9 +760,11 @@ int write_loose_object(struct odb_source_loose *loose,
 	close_loose_object(loose, fd, tmp_file.buf);
 
 	if (mtime) {
-		struct utimbuf utb;
-		utb.actime = mtime;
-		utb.modtime = mtime;
+		struct utimbuf utb = {
+			.actime = *mtime,
+			.modtime = *mtime,
+		};
+
 		if (utime(tmp_file.buf, &utb) < 0 &&
 		    !(flags & ODB_WRITE_OBJECT_SILENT))
 			warning_errno(_("failed utime() on %s"), tmp_file.buf);
@@ -883,7 +894,7 @@ int odb_source_loose_write_stream(struct odb_source_loose *loose,
 }
 
 int force_object_loose(struct odb_source *source,
-		       const struct object_id *oid, time_t mtime)
+		       const struct object_id *oid, const time_t *mtime)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
 	const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
diff --git a/object-file.h b/object-file.h
index 08aafcda0d..9fd540afb6 100644
--- a/object-file.h
+++ b/object-file.h
@@ -99,7 +99,8 @@ int format_object_header(char *str, size_t size, enum object_type type,
 			 size_t objsize);
 
 int force_object_loose(struct odb_source *source,
-		       const struct object_id *oid, time_t mtime);
+		       const struct object_id *oid,
+		       const time_t *mtime);
 
 /**
  * With in-core object data in "buf", rehash it to make sure the
@@ -137,10 +138,11 @@ void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 int write_loose_object(struct odb_source_loose *loose,
 		       const struct object_id *oid, char *hdr,
 		       int hdrlen, const void *buf, unsigned long len,
-		       time_t mtime, unsigned flags);
+		       const time_t *mtime, unsigned flags);
 
 /* Helper to check and "touch" a file */
-int check_and_freshen_file(const char *fn, int freshen);
+int check_and_freshen_file(const char *fn, int freshen,
+			   const time_t *mtime);
 
 /*
  * Open the loose object at path, check its hash, and return the contents,
diff --git a/odb.c b/odb.c
index bfeca76f4e..dabd481f57 100644
--- a/odb.c
+++ b/odb.c
@@ -738,7 +738,7 @@ int odb_pretend_object(struct object_database *odb,
 		return 0;
 
 	return odb_source_write_object(odb->inmemory_objects,
-				       buf, len, type, oid, NULL, 0);
+				       buf, len, type, oid, NULL, NULL, 0);
 }
 
 void *odb_read_object(struct object_database *odb,
@@ -829,7 +829,7 @@ int odb_freshen_object(struct object_database *odb,
 	struct odb_source *source;
 	odb_prepare_alternates(odb);
 	for (source = odb->sources; source; source = source->next)
-		if (odb_source_freshen_object(source, oid))
+		if (odb_source_freshen_object(source, oid, NULL))
 			return 1;
 	return 0;
 }
@@ -1024,7 +1024,7 @@ int odb_write_object_ext(struct object_database *odb,
 	}
 
 	return odb_source_write_object(odb->sources, buf, len, type,
-				       oid, compat_oid_p, flags);
+				       oid, compat_oid_p, NULL, flags);
 }
 
 int odb_write_object_stream(struct object_database *odb,
diff --git a/odb/source-files.c b/odb/source-files.c
index 06dfc8dd78..4df4e1af6c 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -150,11 +150,12 @@ static int odb_source_files_find_abbrev_len(struct odb_source *source,
 }
 
 static int odb_source_files_freshen_object(struct odb_source *source,
-					   const struct object_id *oid)
+					   const struct object_id *oid,
+					   const time_t *mtime)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
-	if (odb_source_freshen_object(&files->packed->base, oid) ||
-	    odb_source_freshen_object(&files->loose->base, oid))
+	if (odb_source_freshen_object(&files->packed->base, oid, mtime) ||
+	    odb_source_freshen_object(&files->loose->base, oid, mtime))
 		return 1;
 	return 0;
 }
@@ -164,11 +165,12 @@ static int odb_source_files_write_object(struct odb_source *source,
 					 enum object_type type,
 					 const struct object_id *oid,
 					 const struct object_id *compat_oid,
+					 const time_t *mtime,
 					 enum odb_write_object_flags flags)
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
 	return odb_source_write_object(&files->loose->base, buf, len, type,
-				       oid, compat_oid, flags);
+				       oid, compat_oid, mtime, flags);
 }
 
 static int odb_source_files_write_object_stream(struct odb_source *source,
diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c
index 963d520317..3e71611b8e 100644
--- a/odb/source-inmemory.c
+++ b/odb/source-inmemory.c
@@ -232,6 +232,7 @@ static int odb_source_inmemory_write_object(struct odb_source *source,
 					    enum object_type type,
 					    const struct object_id *oid,
 					    const struct object_id *compat_oid UNUSED,
+					    const time_t *mtime UNUSED,
 					    enum odb_write_object_flags flags UNUSED)
 {
 	struct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);
@@ -286,7 +287,7 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source,
 	hash_object_file(source->odb->repo->hash_algo, data, total_read, OBJ_BLOB, oid);
 
 	ret = odb_source_inmemory_write_object(source, data, len, OBJ_BLOB, oid,
-					       NULL, 0);
+					       NULL, NULL, 0);
 	if (ret < 0)
 		goto out;
 
@@ -296,7 +297,8 @@ static int odb_source_inmemory_write_object_stream(struct odb_source *source,
 }
 
 static int odb_source_inmemory_freshen_object(struct odb_source *source,
-					      const struct object_id *oid)
+					      const struct object_id *oid,
+					      const time_t *mtime UNUSED)
 {
 	struct odb_source_inmemory *inmemory = odb_source_inmemory_downcast(source);
 	if (find_cached_object(inmemory, oid))
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 04af1a54a3..520a30157c 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -574,12 +574,13 @@ static int odb_source_loose_count_objects(struct odb_source *source,
 }
 
 static int odb_source_loose_freshen_object(struct odb_source *source,
-					   const struct object_id *oid)
+					   const struct object_id *oid,
+					   const time_t *mtime)
 {
 	struct odb_source_loose *loose = odb_source_loose_downcast(source);
 	static struct strbuf path = STRBUF_INIT;
 	odb_loose_path(loose, &path, oid);
-	return !!check_and_freshen_file(path.buf, 1);
+	return !!check_and_freshen_file(path.buf, 1, mtime);
 }
 
 static int odb_source_loose_write_object(struct odb_source *source,
@@ -587,6 +588,7 @@ static int odb_source_loose_write_object(struct odb_source *source,
 					 enum object_type type,
 					 const struct object_id *oid,
 					 const struct object_id *compat_oid,
+					 const time_t *mtime,
 					 enum odb_write_object_flags flags)
 {
 	struct odb_source_loose *loose = odb_source_loose_downcast(source);
@@ -595,7 +597,7 @@ static int odb_source_loose_write_object(struct odb_source *source,
 
 	hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
 
-	if (write_loose_object(loose, oid, hdr, hdrlen, buf, len, 0, flags))
+	if (write_loose_object(loose, oid, hdr, hdrlen, buf, len, mtime, flags))
 		return -1;
 
 	if (compat_oid)
diff --git a/odb/source-packed.c b/odb/source-packed.c
index f7f1706447..5e5da9bc54 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -507,18 +507,26 @@ static int odb_source_packed_find_abbrev_len(struct odb_source *source,
 }
 
 static int odb_source_packed_freshen_object(struct odb_source *source,
-					    const struct object_id *oid)
+					    const struct object_id *oid,
+					    const time_t *mtime)
 {
 	struct odb_source_packed *packed = odb_source_packed_downcast(source);
+	struct utimbuf times, *timesp = NULL;
 	struct pack_entry e;
 
+	if (mtime) {
+		times.actime = *mtime;
+		times.modtime = *mtime;
+		timesp = &times;
+	}
+
 	if (!find_pack_entry(packed, oid, &e))
 		return 0;
 	if (e.p->is_cruft)
 		return 0;
 	if (e.p->freshened)
 		return 1;
-	if (utime(e.p->pack_name, NULL))
+	if (utime(e.p->pack_name, timesp))
 		return 0;
 	e.p->freshened = 1;
 
@@ -531,6 +539,7 @@ static int odb_source_packed_write_object(struct odb_source *source UNUSED,
 					  enum object_type type UNUSED,
 					  const struct object_id *oid UNUSED,
 					  const struct object_id *compat_oid UNUSED,
+					  const time_t *mtime UNUSED,
 					  unsigned flags UNUSED)
 {
 	return error("packed backend cannot write objects");
diff --git a/odb/source.h b/odb/source.h
index c4e94c9d0d..fc04dd5cda 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -190,7 +190,8 @@ struct odb_source {
 	 * has been freshened.
 	 */
 	int (*freshen_object)(struct odb_source *source,
-			      const struct object_id *oid);
+			      const struct object_id *oid,
+			      const time_t *mtime);
 
 	/*
 	 * This callback is expected to persist the given object into the
@@ -208,6 +209,7 @@ struct odb_source {
 			    enum object_type type,
 			    const struct object_id *oid,
 			    const struct object_id *compat_oid,
+			    const time_t *mtime,
 			    enum odb_write_object_flags flags);
 
 	/*
@@ -403,9 +405,10 @@ static inline int odb_source_find_abbrev_len(struct odb_source *source,
  * not exist.
  */
 static inline int odb_source_freshen_object(struct odb_source *source,
-					    const struct object_id *oid)
+					    const struct object_id *oid,
+					    const time_t *mtime)
 {
-	return source->freshen_object(source, oid);
+	return source->freshen_object(source, oid, mtime);
 }
 
 /*
@@ -418,10 +421,11 @@ static inline int odb_source_write_object(struct odb_source *source,
 					  enum object_type type,
 					  const struct object_id *oid,
 					  const struct object_id *compat_oid,
+					  const time_t *mtime,
 					  enum odb_write_object_flags flags)
 {
 	return source->write_object(source, buf, len, type, oid,
-				    compat_oid, flags);
+				    compat_oid, mtime, flags);
 }
 
 /*
diff --git a/read-cache.c b/read-cache.c
index 3510b49edf..c67930177f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2342,7 +2342,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
  */
 static void freshen_shared_index(const char *shared_index, int warn)
 {
-	if (!check_and_freshen_file(shared_index, 1) && warn)
+	if (!check_and_freshen_file(shared_index, 1, NULL) && warn)
 		warning(_("could not freshen shared index '%s'"), shared_index);
 }
 
diff --git a/t/unit-tests/u-odb-inmemory.c b/t/unit-tests/u-odb-inmemory.c
index 28a69fc244..ddf2db5c81 100644
--- a/t/unit-tests/u-odb-inmemory.c
+++ b/t/unit-tests/u-odb-inmemory.c
@@ -45,7 +45,7 @@ static void cl_assert_write_object(struct odb_source_inmemory *source,
 	size_t content_len = strlen(content);
 	hash_object_file(repo.hash_algo, content, content_len, type, oid);
 	cl_must_pass(odb_source_write_object(&source->base, content, content_len,
-					     type, oid, NULL, 0));
+					     type, oid, NULL, NULL, 0));
 }
 
 void test_odb_inmemory__initialize(void)
@@ -256,11 +256,11 @@ void test_odb_inmemory__freshen_object(void)
 	const char *end;
 
 	cl_must_pass(parse_oid_hex_algop(RANDOM_OID, &oid, &end, repo.hash_algo));
-	cl_assert_equal_i(odb_source_freshen_object(&source->base, &oid), 0);
+	cl_assert_equal_i(odb_source_freshen_object(&source->base, &oid, NULL), 0);
 
 	cl_assert_write_object(source, "foobar", OBJ_BLOB, &written_oid);
 	cl_assert_equal_i(odb_source_freshen_object(&source->base,
-						    &written_oid), 1);
+						    &written_oid, NULL), 1);
 
 	odb_source_free(&source->base);
 }
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 6/9] object-file: fix memory leak in `force_object_loose()`

We return an error when converting the given object to the compatibility hash algorithm fails. This early return causes a memory leak though, because we don't free the content buffer that we've already read before via `odb_read_object_info_extended()`.

Plug the memory leak by creating a common exit path where the buffer gets free'd.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 object-file.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)
Show changes to object-file.c +18 −8
diff --git a/object-file.c b/object-file.c
index 5b07530950..067a63a4f1 100644
--- a/object-file.c
+++ b/object-file.c
@@ -898,7 +898,7 @@ int force_object_loose(struct odb_source *source,
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
 	const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
-	void *buf;
+	void *buf = NULL;
 	size_t len;
 	struct object_info oi = OBJECT_INFO_INIT;
 	struct object_id compat_oid;
@@ -916,19 +916,29 @@ int force_object_loose(struct odb_source *source,
 	oi.typep = &type;
 	oi.sizep = &len;
 	oi.contentp = &buf;
-	if (odb_read_object_info_extended(source->odb, oid, &oi, 0))
-		return error(_("cannot read object for %s"), oid_to_hex(oid));
+	if (odb_read_object_info_extended(source->odb, oid, &oi, 0)) {
+		ret = error(_("cannot read object for %s"), oid_to_hex(oid));
+		goto out;
+	}
+
 	if (compat) {
-		if (repo_oid_to_algop(source->odb->repo, oid, compat, &compat_oid))
-			return error(_("cannot map object %s to %s"),
-				     oid_to_hex(oid), compat->name);
+		if (repo_oid_to_algop(source->odb->repo, oid, compat, &compat_oid)) {
+			ret = error(_("cannot map object %s to %s"),
+				    oid_to_hex(oid), compat->name);
+			goto out;
+		}
 	}
+
 	hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
 	ret = write_loose_object(files->loose, oid, hdr, hdrlen, buf, len, mtime, 0);
-	if (!ret && compat)
+	if (ret)
+		goto out;
+
+	if (compat)
 		ret = repo_add_loose_object_map(files->loose, oid, &compat_oid);
-	free(buf);
 
+out:
+	free(buf);
 	return ret;
 }
 
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 7/9] object-file: force objects loose via generic interface

When repacking objects we may end up "loosening" objects via `force_objects_loose()`. The implementation of this logic still sits with "object-file.c" even though it is ultimately an implementation detail of the "files" backend.

Moving this logic around is non-trivial though as we depend on `write_loose_object()`, which is an internal implementation detail of how we write loose objects. Until now it wasn't possible to use the generic function `odb_source_write_object()` though, because the "loose" implementation thereof would skip writing the object in case it already exists in any other source.

This restriction was lifted over the preceding commits though, where this object existence check is now handled on the object database level and not on the individual source level anymore. Consequently, it is now possible to use generic interfaces.

Refactor the code accordingly so that we can move the logic around in a subsequent commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 object-file.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)
Show changes to object-file.c +7 −12
diff --git a/object-file.c b/object-file.c
index 067a63a4f1..89825feed0 100644
--- a/object-file.c
+++ b/object-file.c
@@ -898,13 +898,11 @@ int force_object_loose(struct odb_source *source,
 {
 	struct odb_source_files *files = odb_source_files_downcast(source);
 	const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
-	void *buf = NULL;
-	size_t len;
 	struct object_info oi = OBJECT_INFO_INIT;
-	struct object_id compat_oid;
+	struct object_id compat_oid, *compat_oid_p = NULL;
 	enum object_type type;
-	char hdr[MAX_HEADER_LEN];
-	int hdrlen;
+	void *buf = NULL;
+	size_t len;
 	int ret;
 
 	for (struct odb_source *s = source->odb->sources; s; s = s->next) {
@@ -927,15 +925,12 @@ int force_object_loose(struct odb_source *source,
 				    oid_to_hex(oid), compat->name);
 			goto out;
 		}
-	}
 
-	hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
-	ret = write_loose_object(files->loose, oid, hdr, hdrlen, buf, len, mtime, 0);
-	if (ret)
-		goto out;
+		compat_oid_p = &compat_oid;
+	}
 
-	if (compat)
-		ret = repo_add_loose_object_map(files->loose, oid, &compat_oid);
+	ret = odb_source_write_object(&files->loose->base, buf, len, type, oid,
+				      compat_oid_p, mtime, 0);
 
 out:
 	free(buf);
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 8/9] object-file: move `force_object_loose()`

In the preceding commits we have refactored `force_object_loose()` to not call internal functions anymore for writing the object. Instead, it now only uses generic functions that are accessible to all callers.

Consequently, we can now easily move the function to its only caller, which is git-pack-objects(1). Do so.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/pack-objects.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 object-file.c          | 44 --------------------------------------------
 object-file.h          |  4 ----
 3 files changed, 46 insertions(+), 48 deletions(-)
Show changes to 3 files +46 −48

builtin/pack-objects.c, object-file.c, object-file.h

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e64a96f1a7..bb3bc486e8 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -32,6 +32,7 @@
 #include "list.h"
 #include "packfile.h"
 #include "object-file.h"
+#include "object-file-convert.h"
 #include "odb.h"
 #include "odb/streaming.h"
 #include "replace-object.h"
@@ -4622,6 +4623,51 @@ static int loosened_object_can_be_discarded(const struct object_id *oid,
 	return 1;
 }
 
+static int force_object_loose(struct odb_source *source,
+			      const struct object_id *oid,
+			      const time_t *mtime)
+{
+	struct odb_source_files *files = odb_source_files_downcast(source);
+	const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
+	struct object_info oi = OBJECT_INFO_INIT;
+	struct object_id compat_oid, *compat_oid_p = NULL;
+	enum object_type type;
+	void *buf = NULL;
+	size_t len;
+	int ret;
+
+	for (struct odb_source *s = source->odb->sources; s; s = s->next) {
+		struct odb_source_files *files = odb_source_files_downcast(s);
+		if (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0))
+			return 0;
+	}
+
+	oi.typep = &type;
+	oi.sizep = &len;
+	oi.contentp = &buf;
+	if (odb_read_object_info_extended(source->odb, oid, &oi, 0)) {
+		ret = error(_("cannot read object for %s"), oid_to_hex(oid));
+		goto out;
+	}
+
+	if (compat) {
+		if (repo_oid_to_algop(source->odb->repo, oid, compat, &compat_oid)) {
+			ret = error(_("cannot map object %s to %s"),
+				    oid_to_hex(oid), compat->name);
+			goto out;
+		}
+
+		compat_oid_p = &compat_oid;
+	}
+
+	ret = odb_source_write_object(&files->loose->base, buf, len, type, oid,
+				      compat_oid_p, mtime, 0);
+
+out:
+	free(buf);
+	return ret;
+}
+
 static void loosen_unused_packed_objects(void)
 {
 	struct packed_git *p;
diff --git a/object-file.c b/object-file.c
index 89825feed0..b867d8d9de 100644
--- a/object-file.c
+++ b/object-file.c
@@ -893,50 +893,6 @@ int odb_source_loose_write_stream(struct odb_source_loose *loose,
 	return err;
 }
 
-int force_object_loose(struct odb_source *source,
-		       const struct object_id *oid, const time_t *mtime)
-{
-	struct odb_source_files *files = odb_source_files_downcast(source);
-	const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
-	struct object_info oi = OBJECT_INFO_INIT;
-	struct object_id compat_oid, *compat_oid_p = NULL;
-	enum object_type type;
-	void *buf = NULL;
-	size_t len;
-	int ret;
-
-	for (struct odb_source *s = source->odb->sources; s; s = s->next) {
-		struct odb_source_files *files = odb_source_files_downcast(s);
-		if (!odb_source_read_object_info(&files->loose->base, oid, NULL, 0))
-			return 0;
-	}
-
-	oi.typep = &type;
-	oi.sizep = &len;
-	oi.contentp = &buf;
-	if (odb_read_object_info_extended(source->odb, oid, &oi, 0)) {
-		ret = error(_("cannot read object for %s"), oid_to_hex(oid));
-		goto out;
-	}
-
-	if (compat) {
-		if (repo_oid_to_algop(source->odb->repo, oid, compat, &compat_oid)) {
-			ret = error(_("cannot map object %s to %s"),
-				    oid_to_hex(oid), compat->name);
-			goto out;
-		}
-
-		compat_oid_p = &compat_oid;
-	}
-
-	ret = odb_source_write_object(&files->loose->base, buf, len, type, oid,
-				      compat_oid_p, mtime, 0);
-
-out:
-	free(buf);
-	return ret;
-}
-
 /*
  * We can't use the normal fsck_error_function() for index_mem(),
  * because we don't yet have a valid oid for it to report. Instead,
diff --git a/object-file.h b/object-file.h
index 9fd540afb6..31781a9c53 100644
--- a/object-file.h
+++ b/object-file.h
@@ -98,10 +98,6 @@ int for_each_file_in_obj_subdir(unsigned int subdir_nr,
 int format_object_header(char *str, size_t size, enum object_type type,
 			 size_t objsize);
 
-int force_object_loose(struct odb_source *source,
-		       const struct object_id *oid,
-		       const time_t *mtime);
-
 /**
  * With in-core object data in "buf", rehash it to make sure the
  * object name actually matches "oid" to detect object corruption.
-- 
2.55.0.407.g700c83d4f3.dirty
Patrick SteinhardtJul 17, 2026, 09:32 UTC in reply to Patrick Steinhardt on lore

[PATCH 9/9] object-file: move logic to write loose objects

The logic to write loose objects is split up across "object-file.c" and "odb/source-loose.c". This split is somewhat weird, but it is the result of two things:

  - `force_object_loose()` used to reach into internals of how exactly
    we write objects.
  - The logic of writing objects is intertwined with potentially
    starting a transaction.

We have refactored `force_object_loose()` over preceding commits to work via generic interfaces now, so this reason doesn't exist anymore. But the second reason still does, as our management of "files" transactions and their ad-hoc creation is still very messy. This area definitely requires further work, and that work is indeed ongoing.

That being said, we can already move the writing logic into the "loose" backend rather easily. All we have to do is to expose two functions that relate to the transactions.

Expose these two functions and move the writing logic into the "loose" backend accordingly so that it becomes more self-contained. Note that this requires us to drop a reference to `the_repository` in favor of using the source's repository in `start_loose_object_common()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 object-file.c      | 360 +----------------------------------------------------
 object-file.h      |  22 +---
 odb/source-loose.c | 354 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 357 insertions(+), 379 deletions(-)
Show changes to 3 files +357 −379

object-file.c, object-file.h, odb/source-loose.c

diff --git a/object-file.c b/object-file.c
index b867d8d9de..bdc97d7943 100644
--- a/object-file.c
+++ b/object-file.c
@@ -491,7 +491,7 @@ struct odb_transaction_files {
 	const char *prefix;
 };
 
-static int odb_transaction_files_prepare(struct odb_transaction *base)
+int odb_transaction_files_prepare(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
 		container_of_or_null(base, struct odb_transaction_files, base);
@@ -514,8 +514,8 @@ static int odb_transaction_files_prepare(struct odb_transaction *base)
 	return 0;
 }
 
-static void odb_transaction_files_fsync(struct odb_transaction *base,
-					int fd, const char *filename)
+void odb_transaction_files_fsync(struct odb_transaction *base,
+				 int fd, const char *filename)
 {
 	struct odb_transaction_files *transaction =
 		container_of_or_null(base, struct odb_transaction_files, base);
@@ -539,360 +539,6 @@ static void odb_transaction_files_fsync(struct odb_transaction *base,
 	}
 }
 
-/* Finalize a file on disk, and close it. */
-static void close_loose_object(struct odb_source_loose *loose,
-			       int fd, const char *filename)
-{
-	if (loose->base.will_destroy)
-		goto out;
-
-	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
-		odb_transaction_files_fsync(loose->base.odb->transaction, fd, filename);
-	else if (fsync_object_files > 0)
-		fsync_or_die(fd, filename);
-	else
-		fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd,
-				       filename);
-
-out:
-	if (close(fd) != 0)
-		die_errno(_("error when closing loose object file"));
-}
-
-/* Size of directory component, including the ending '/' */
-static inline int directory_size(const char *filename)
-{
-	const char *s = strrchr(filename, '/');
-	if (!s)
-		return 0;
-	return s - filename + 1;
-}
-
-/*
- * This creates a temporary file in the same directory as the final
- * 'filename'
- *
- * We want to avoid cross-directory filename renames, because those
- * can have problems on various filesystems (FAT, NFS, Coda).
- */
-static int create_tmpfile(struct repository *repo,
-			  struct strbuf *tmp, const char *filename)
-{
-	int fd, dirlen = directory_size(filename);
-
-	strbuf_reset(tmp);
-	strbuf_add(tmp, filename, dirlen);
-	strbuf_addstr(tmp, "tmp_obj_XXXXXX");
-	fd = git_mkstemp_mode(tmp->buf, 0444);
-	if (fd < 0 && dirlen && errno == ENOENT) {
-		/*
-		 * Make sure the directory exists; note that the contents
-		 * of the buffer are undefined after mkstemp returns an
-		 * error, so we have to rewrite the whole buffer from
-		 * scratch.
-		 */
-		strbuf_reset(tmp);
-		strbuf_add(tmp, filename, dirlen - 1);
-		if (mkdir(tmp->buf, 0777) && errno != EEXIST)
-			return -1;
-		if (adjust_shared_perm(repo, tmp->buf))
-			return -1;
-
-		/* Try again */
-		strbuf_addstr(tmp, "/tmp_obj_XXXXXX");
-		fd = git_mkstemp_mode(tmp->buf, 0444);
-	}
-	return fd;
-}
-
-/**
- * Common steps for loose object writers to start writing loose
- * objects:
- *
- * - Create tmpfile for the loose object.
- * - Setup zlib stream for compression.
- * - Start to feed header to zlib stream.
- *
- * Returns a "fd", which should later be provided to
- * end_loose_object_common().
- */
-static int start_loose_object_common(struct odb_source_loose *loose,
-				     struct strbuf *tmp_file,
-				     const char *filename, unsigned flags,
-				     git_zstream *stream,
-				     unsigned char *buf, size_t buflen,
-				     struct git_hash_ctx *c, struct git_hash_ctx *compat_c,
-				     char *hdr, int hdrlen)
-{
-	const struct git_hash_algo *algo = loose->base.odb->repo->hash_algo;
-	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
-	int fd;
-	struct repo_config_values *cfg = repo_config_values(the_repository);
-
-	fd = create_tmpfile(loose->base.odb->repo, tmp_file, filename);
-	if (fd < 0) {
-		if (flags & ODB_WRITE_OBJECT_SILENT)
-			return -1;
-		else if (errno == EACCES)
-			return error(_("insufficient permission for adding "
-				       "an object to repository database %s"),
-				     loose->base.path);
-		else
-			return error_errno(
-				_("unable to create temporary file"));
-	}
-
-	/*  Setup zlib stream for compression */
-	git_deflate_init(stream, cfg->zlib_compression_level);
-	stream->next_out = buf;
-	stream->avail_out = buflen;
-	git_hash_init(c, algo);
-	if (compat && compat_c)
-		git_hash_init(compat_c, compat);
-
-	/*  Start to feed header to zlib stream */
-	stream->next_in = (unsigned char *)hdr;
-	stream->avail_in = hdrlen;
-	while (git_deflate(stream, 0) == Z_OK)
-		; /* nothing */
-	git_hash_update(c, hdr, hdrlen);
-	if (compat && compat_c)
-		git_hash_update(compat_c, hdr, hdrlen);
-
-	return fd;
-}
-
-/**
- * Common steps for the inner git_deflate() loop for writing loose
- * objects. Returns what git_deflate() returns.
- */
-static int write_loose_object_common(struct odb_source_loose *loose,
-				     struct git_hash_ctx *c, struct git_hash_ctx *compat_c,
-				     git_zstream *stream, const int flush,
-				     unsigned char *in0, const int fd,
-				     unsigned char *compressed,
-				     const size_t compressed_len)
-{
-	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
-	int ret;
-
-	ret = git_deflate(stream, flush ? Z_FINISH : 0);
-	git_hash_update(c, in0, stream->next_in - in0);
-	if (compat && compat_c)
-		git_hash_update(compat_c, in0, stream->next_in - in0);
-	if (write_in_full(fd, compressed, stream->next_out - compressed) < 0)
-		die_errno(_("unable to write loose object file"));
-	stream->next_out = compressed;
-	stream->avail_out = compressed_len;
-
-	return ret;
-}
-
-/**
- * Common steps for loose object writers to end writing loose objects:
- *
- * - End the compression of zlib stream.
- * - Get the calculated oid to "oid".
- */
-static int end_loose_object_common(struct odb_source_loose *loose,
-				   struct git_hash_ctx *c, struct git_hash_ctx *compat_c,
-				   git_zstream *stream, struct object_id *oid,
-				   struct object_id *compat_oid)
-{
-	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
-	int ret;
-
-	ret = git_deflate_end_gently(stream);
-	if (ret != Z_OK)
-		return ret;
-	git_hash_final_oid(oid, c);
-	if (compat && compat_c)
-		git_hash_final_oid(compat_oid, compat_c);
-
-	return Z_OK;
-}
-
-int write_loose_object(struct odb_source_loose *loose,
-		       const struct object_id *oid, char *hdr,
-		       int hdrlen, const void *buf, unsigned long len,
-		       const time_t *mtime, unsigned flags)
-{
-	int fd, ret;
-	unsigned char compressed[4096];
-	git_zstream stream;
-	struct git_hash_ctx c;
-	struct object_id parano_oid;
-	static struct strbuf tmp_file = STRBUF_INIT;
-	static struct strbuf filename = STRBUF_INIT;
-
-	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
-		odb_transaction_files_prepare(loose->base.odb->transaction);
-
-	odb_loose_path(loose, &filename, oid);
-
-	fd = start_loose_object_common(loose, &tmp_file, filename.buf, flags,
-				       &stream, compressed, sizeof(compressed),
-				       &c, NULL, hdr, hdrlen);
-	if (fd < 0)
-		return -1;
-
-	/* Then the data itself.. */
-	stream.next_in = (void *)buf;
-	stream.avail_in = len;
-	do {
-		unsigned char *in0 = stream.next_in;
-
-		ret = write_loose_object_common(loose, &c, NULL, &stream, 1, in0, fd,
-						compressed, sizeof(compressed));
-	} while (ret == Z_OK);
-
-	if (ret != Z_STREAM_END)
-		die(_("unable to deflate new object %s (%d)"), oid_to_hex(oid),
-		    ret);
-	ret = end_loose_object_common(loose, &c, NULL, &stream, &parano_oid, NULL);
-	if (ret != Z_OK)
-		die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
-		    ret);
-	if (!oideq(oid, &parano_oid))
-		die(_("confused by unstable object source data for %s"),
-		    oid_to_hex(oid));
-
-	close_loose_object(loose, fd, tmp_file.buf);
-
-	if (mtime) {
-		struct utimbuf utb = {
-			.actime = *mtime,
-			.modtime = *mtime,
-		};
-
-		if (utime(tmp_file.buf, &utb) < 0 &&
-		    !(flags & ODB_WRITE_OBJECT_SILENT))
-			warning_errno(_("failed utime() on %s"), tmp_file.buf);
-	}
-
-	return finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
-					  FOF_SKIP_COLLISION_CHECK);
-}
-
-int odb_source_loose_write_stream(struct odb_source_loose *loose,
-				  struct odb_write_stream *in_stream, size_t len,
-				  struct object_id *oid)
-{
-	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
-	struct object_id compat_oid;
-	int fd, ret, err = 0, flush = 0;
-	unsigned char compressed[4096];
-	git_zstream stream;
-	struct git_hash_ctx c, compat_c;
-	struct strbuf tmp_file = STRBUF_INIT;
-	struct strbuf filename = STRBUF_INIT;
-	unsigned char buf[8192];
-	int dirlen;
-	char hdr[MAX_HEADER_LEN];
-	int hdrlen;
-
-	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
-		odb_transaction_files_prepare(loose->base.odb->transaction);
-
-	/* Since oid is not determined, save tmp file to odb path. */
-	strbuf_addf(&filename, "%s/", loose->base.path);
-	hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, len);
-
-	/*
-	 * Common steps for write_loose_object and stream_loose_object to
-	 * start writing loose objects:
-	 *
-	 *  - Create tmpfile for the loose object.
-	 *  - Setup zlib stream for compression.
-	 *  - Start to feed header to zlib stream.
-	 */
-	fd = start_loose_object_common(loose, &tmp_file, filename.buf, 0,
-				       &stream, compressed, sizeof(compressed),
-				       &c, &compat_c, hdr, hdrlen);
-	if (fd < 0) {
-		err = -1;
-		goto cleanup;
-	}
-
-	/* Then the data itself.. */
-	do {
-		unsigned char *in0 = stream.next_in;
-
-		if (!stream.avail_in && !in_stream->is_finished) {
-			ssize_t read_len = odb_write_stream_read(in_stream, buf,
-								 sizeof(buf));
-			if (read_len < 0) {
-				close(fd);
-				err = -1;
-				goto cleanup;
-			}
-
-			stream.avail_in = read_len;
-			stream.next_in = buf;
-			in0 = buf;
-			/* All data has been read. */
-			if (in_stream->is_finished)
-				flush = 1;
-		}
-		ret = write_loose_object_common(loose, &c, &compat_c, &stream, flush, in0, fd,
-						compressed, sizeof(compressed));
-		/*
-		 * Unlike write_loose_object(), we do not have the entire
-		 * buffer. If we get Z_BUF_ERROR due to too few input bytes,
-		 * then we'll replenish them in the next input_stream->read()
-		 * call when we loop.
-		 */
-	} while (ret == Z_OK || ret == Z_BUF_ERROR);
-
-	if (stream.total_in != len + hdrlen)
-		die(_("write stream object %"PRIuMAX" != %"PRIuMAX), (uintmax_t)stream.total_in,
-		    (uintmax_t)len + hdrlen);
-
-	/*
-	 * Common steps for write_loose_object and stream_loose_object to
-	 * end writing loose object:
-	 *
-	 *  - End the compression of zlib stream.
-	 *  - Get the calculated oid.
-	 */
-	if (ret != Z_STREAM_END)
-		die(_("unable to stream deflate new object (%d)"), ret);
-	ret = end_loose_object_common(loose, &c, &compat_c, &stream, oid, &compat_oid);
-	if (ret != Z_OK)
-		die(_("deflateEnd on stream object failed (%d)"), ret);
-	close_loose_object(loose, fd, tmp_file.buf);
-
-	if (odb_freshen_object(loose->base.odb, oid)) {
-		unlink_or_warn(tmp_file.buf);
-		goto cleanup;
-	}
-	odb_loose_path(loose, &filename, oid);
-
-	/* We finally know the object path, and create the missing dir. */
-	dirlen = directory_size(filename.buf);
-	if (dirlen) {
-		struct strbuf dir = STRBUF_INIT;
-		strbuf_add(&dir, filename.buf, dirlen);
-
-		if (safe_create_dir_in_gitdir(loose->base.odb->repo, dir.buf) &&
-		    errno != EEXIST) {
-			err = error_errno(_("unable to create directory %s"), dir.buf);
-			strbuf_release(&dir);
-			goto cleanup;
-		}
-		strbuf_release(&dir);
-	}
-
-	err = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
-					 FOF_SKIP_COLLISION_CHECK);
-	if (!err && compat)
-		err = repo_add_loose_object_map(loose, oid, &compat_oid);
-cleanup:
-	strbuf_release(&tmp_file);
-	strbuf_release(&filename);
-	return err;
-}
-
 /*
  * We can't use the normal fsck_error_function() for index_mem(),
  * because we don't yet have a valid oid for it to report. Instead,
diff --git a/object-file.h b/object-file.h
index 31781a9c53..805f2cfa28 100644
--- a/object-file.h
+++ b/object-file.h
@@ -24,20 +24,6 @@ int index_path(struct index_state *istate, struct object_id *oid, const char *pa
 struct object_info;
 struct odb_source;
 
-/*
- * Write the given stream into the loose object source. The only difference
- * from the generic implementation of this function is that we don't perform an
- * object existence check here.
- *
- * TODO: We should stop exposing this function altogether and move it into
- * "odb/source-loose.c". This requires a couple of refactorings though to make
- * `force_object_loose()` generic and is thus postponed to a later point in
- * time.
- */
-int odb_source_loose_write_stream(struct odb_source_loose *source,
-				  struct odb_write_stream *stream, size_t len,
-				  struct object_id *oid);
-
 /*
  * Put in `buf` the name of the file in the local object database that
  * would be used to store a loose object with the specified oid.
@@ -131,10 +117,6 @@ int finalize_object_file_flags(struct repository *repo,
 void hash_object_file(const struct git_hash_algo *algo, const void *buf,
 		      size_t len, enum object_type type,
 		      struct object_id *oid);
-int write_loose_object(struct odb_source_loose *loose,
-		       const struct object_id *oid, char *hdr,
-		       int hdrlen, const void *buf, unsigned long len,
-		       const time_t *mtime, unsigned flags);
 
 /* Helper to check and "touch" a file */
 int check_and_freshen_file(const char *fn, int freshen,
@@ -195,4 +177,8 @@ int odb_transaction_files_begin(struct odb_source *source,
 				struct odb_transaction **out,
 				enum odb_transaction_flags flags);
 
+int odb_transaction_files_prepare(struct odb_transaction *base);
+void odb_transaction_files_fsync(struct odb_transaction *base,
+				 int fd, const char *filename);
+
 #endif /* OBJECT_FILE_H */
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 520a30157c..ef0e919277 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -11,8 +11,11 @@
 #include "odb/source-loose.h"
 #include "odb/streaming.h"
 #include "oidtree.h"
+#include "path.h"
 #include "repository.h"
 #include "strbuf.h"
+#include "tempfile.h"
+#include "write-or-die.h"
 
 static int append_loose_object(const struct object_id *oid,
 			       const char *path UNUSED,
@@ -583,6 +586,241 @@ static int odb_source_loose_freshen_object(struct odb_source *source,
 	return !!check_and_freshen_file(path.buf, 1, mtime);
 }
 
+/* Finalize a file on disk, and close it. */
+static void close_loose_object(struct odb_source_loose *loose,
+			       int fd, const char *filename)
+{
+	if (loose->base.will_destroy)
+		goto out;
+
+	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
+		odb_transaction_files_fsync(loose->base.odb->transaction, fd, filename);
+	else if (fsync_object_files > 0)
+		fsync_or_die(fd, filename);
+	else
+		fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd,
+				       filename);
+
+out:
+	if (close(fd) != 0)
+		die_errno(_("error when closing loose object file"));
+}
+
+/* Size of directory component, including the ending '/' */
+static inline int directory_size(const char *filename)
+{
+	const char *s = strrchr(filename, '/');
+	if (!s)
+		return 0;
+	return s - filename + 1;
+}
+
+/*
+ * This creates a temporary file in the same directory as the final
+ * 'filename'
+ *
+ * We want to avoid cross-directory filename renames, because those
+ * can have problems on various filesystems (FAT, NFS, Coda).
+ */
+static int create_tmpfile(struct repository *repo,
+			  struct strbuf *tmp, const char *filename)
+{
+	int fd, dirlen = directory_size(filename);
+
+	strbuf_reset(tmp);
+	strbuf_add(tmp, filename, dirlen);
+	strbuf_addstr(tmp, "tmp_obj_XXXXXX");
+	fd = git_mkstemp_mode(tmp->buf, 0444);
+	if (fd < 0 && dirlen && errno == ENOENT) {
+		/*
+		 * Make sure the directory exists; note that the contents
+		 * of the buffer are undefined after mkstemp returns an
+		 * error, so we have to rewrite the whole buffer from
+		 * scratch.
+		 */
+		strbuf_reset(tmp);
+		strbuf_add(tmp, filename, dirlen - 1);
+		if (mkdir(tmp->buf, 0777) && errno != EEXIST)
+			return -1;
+		if (adjust_shared_perm(repo, tmp->buf))
+			return -1;
+
+		/* Try again */
+		strbuf_addstr(tmp, "/tmp_obj_XXXXXX");
+		fd = git_mkstemp_mode(tmp->buf, 0444);
+	}
+	return fd;
+}
+
+/**
+ * Common steps for loose object writers to start writing loose
+ * objects:
+ *
+ * - Create tmpfile for the loose object.
+ * - Setup zlib stream for compression.
+ * - Start to feed header to zlib stream.
+ *
+ * Returns a "fd", which should later be provided to
+ * end_loose_object_common().
+ */
+static int start_loose_object_common(struct odb_source_loose *loose,
+				     struct strbuf *tmp_file,
+				     const char *filename, unsigned flags,
+				     git_zstream *stream,
+				     unsigned char *buf, size_t buflen,
+				     struct git_hash_ctx *c, struct git_hash_ctx *compat_c,
+				     char *hdr, int hdrlen)
+{
+	const struct git_hash_algo *algo = loose->base.odb->repo->hash_algo;
+	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
+	int fd;
+	struct repo_config_values *cfg = repo_config_values(loose->base.odb->repo);
+
+	fd = create_tmpfile(loose->base.odb->repo, tmp_file, filename);
+	if (fd < 0) {
+		if (flags & ODB_WRITE_OBJECT_SILENT)
+			return -1;
+		else if (errno == EACCES)
+			return error(_("insufficient permission for adding "
+				       "an object to repository database %s"),
+				     loose->base.path);
+		else
+			return error_errno(
+				_("unable to create temporary file"));
+	}
+
+	/*  Setup zlib stream for compression */
+	git_deflate_init(stream, cfg->zlib_compression_level);
+	stream->next_out = buf;
+	stream->avail_out = buflen;
+	git_hash_init(c, algo);
+	if (compat && compat_c)
+		git_hash_init(compat_c, compat);
+
+	/*  Start to feed header to zlib stream */
+	stream->next_in = (unsigned char *)hdr;
+	stream->avail_in = hdrlen;
+	while (git_deflate(stream, 0) == Z_OK)
+		; /* nothing */
+	git_hash_update(c, hdr, hdrlen);
+	if (compat && compat_c)
+		git_hash_update(compat_c, hdr, hdrlen);
+
+	return fd;
+}
+
+/**
+ * Common steps for the inner git_deflate() loop for writing loose
+ * objects. Returns what git_deflate() returns.
+ */
+static int write_loose_object_common(struct odb_source_loose *loose,
+				     struct git_hash_ctx *c, struct git_hash_ctx *compat_c,
+				     git_zstream *stream, const int flush,
+				     unsigned char *in0, const int fd,
+				     unsigned char *compressed,
+				     const size_t compressed_len)
+{
+	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
+	int ret;
+
+	ret = git_deflate(stream, flush ? Z_FINISH : 0);
+	git_hash_update(c, in0, stream->next_in - in0);
+	if (compat && compat_c)
+		git_hash_update(compat_c, in0, stream->next_in - in0);
+	if (write_in_full(fd, compressed, stream->next_out - compressed) < 0)
+		die_errno(_("unable to write loose object file"));
+	stream->next_out = compressed;
+	stream->avail_out = compressed_len;
+
+	return ret;
+}
+
+/**
+ * Common steps for loose object writers to end writing loose objects:
+ *
+ * - End the compression of zlib stream.
+ * - Get the calculated oid to "oid".
+ */
+static int end_loose_object_common(struct odb_source_loose *loose,
+				   struct git_hash_ctx *c, struct git_hash_ctx *compat_c,
+				   git_zstream *stream, struct object_id *oid,
+				   struct object_id *compat_oid)
+{
+	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
+	int ret;
+
+	ret = git_deflate_end_gently(stream);
+	if (ret != Z_OK)
+		return ret;
+	git_hash_final_oid(oid, c);
+	if (compat && compat_c)
+		git_hash_final_oid(compat_oid, compat_c);
+
+	return Z_OK;
+}
+
+static int write_loose_object(struct odb_source_loose *loose,
+			      const struct object_id *oid, char *hdr,
+			      int hdrlen, const void *buf, unsigned long len,
+			      const time_t *mtime, unsigned flags)
+{
+	int fd, ret;
+	unsigned char compressed[4096];
+	git_zstream stream;
+	struct git_hash_ctx c;
+	struct object_id parano_oid;
+	static struct strbuf tmp_file = STRBUF_INIT;
+	static struct strbuf filename = STRBUF_INIT;
+
+	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
+		odb_transaction_files_prepare(loose->base.odb->transaction);
+
+	odb_loose_path(loose, &filename, oid);
+
+	fd = start_loose_object_common(loose, &tmp_file, filename.buf, flags,
+				       &stream, compressed, sizeof(compressed),
+				       &c, NULL, hdr, hdrlen);
+	if (fd < 0)
+		return -1;
+
+	/* Then the data itself.. */
+	stream.next_in = (void *)buf;
+	stream.avail_in = len;
+	do {
+		unsigned char *in0 = stream.next_in;
+
+		ret = write_loose_object_common(loose, &c, NULL, &stream, 1, in0, fd,
+						compressed, sizeof(compressed));
+	} while (ret == Z_OK);
+
+	if (ret != Z_STREAM_END)
+		die(_("unable to deflate new object %s (%d)"), oid_to_hex(oid),
+		    ret);
+	ret = end_loose_object_common(loose, &c, NULL, &stream, &parano_oid, NULL);
+	if (ret != Z_OK)
+		die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
+		    ret);
+	if (!oideq(oid, &parano_oid))
+		die(_("confused by unstable object source data for %s"),
+		    oid_to_hex(oid));
+
+	close_loose_object(loose, fd, tmp_file.buf);
+
+	if (mtime) {
+		struct utimbuf utb = {
+			.actime = *mtime,
+			.modtime = *mtime,
+		};
+
+		if (utime(tmp_file.buf, &utb) < 0 &&
+		    !(flags & ODB_WRITE_OBJECT_SILENT))
+			warning_errno(_("failed utime() on %s"), tmp_file.buf);
+	}
+
+	return finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
+					  FOF_SKIP_COLLISION_CHECK);
+}
+
 static int odb_source_loose_write_object(struct odb_source *source,
 					 const void *buf, size_t len,
 					 enum object_type type,
@@ -611,12 +849,120 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
 						size_t len,
 						struct object_id *oid)
 {
+	struct odb_source_loose *loose = odb_source_loose_downcast(source);
+	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
+	struct object_id compat_oid;
+	int fd, ret, err = 0, flush = 0;
+	unsigned char compressed[4096];
+	git_zstream stream;
+	struct git_hash_ctx c, compat_c;
+	struct strbuf tmp_file = STRBUF_INIT;
+	struct strbuf filename = STRBUF_INIT;
+	unsigned char buf[8192];
+	int dirlen;
+	char hdr[MAX_HEADER_LEN];
+	int hdrlen;
+
+	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
+		odb_transaction_files_prepare(loose->base.odb->transaction);
+
+	/* Since oid is not determined, save tmp file to odb path. */
+	strbuf_addf(&filename, "%s/", loose->base.path);
+	hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, len);
+
 	/*
-	 * TODO: the implementation should be moved here, see the comment on
-	 * the called function in "object-file.h".
+	 * Common steps for write_loose_object and stream_loose_object to
+	 * start writing loose objects:
+	 *
+	 *  - Create tmpfile for the loose object.
+	 *  - Setup zlib stream for compression.
+	 *  - Start to feed header to zlib stream.
 	 */
-	struct odb_source_loose *loose = odb_source_loose_downcast(source);
-	return odb_source_loose_write_stream(loose, in_stream, len, oid);
+	fd = start_loose_object_common(loose, &tmp_file, filename.buf, 0,
+				       &stream, compressed, sizeof(compressed),
+				       &c, &compat_c, hdr, hdrlen);
+	if (fd < 0) {
+		err = -1;
+		goto cleanup;
+	}
+
+	/* Then the data itself.. */
+	do {
+		unsigned char *in0 = stream.next_in;
+
+		if (!stream.avail_in && !in_stream->is_finished) {
+			ssize_t read_len = odb_write_stream_read(in_stream, buf,
+								 sizeof(buf));
+			if (read_len < 0) {
+				close(fd);
+				err = -1;
+				goto cleanup;
+			}
+
+			stream.avail_in = read_len;
+			stream.next_in = buf;
+			in0 = buf;
+			/* All data has been read. */
+			if (in_stream->is_finished)
+				flush = 1;
+		}
+		ret = write_loose_object_common(loose, &c, &compat_c, &stream, flush, in0, fd,
+						compressed, sizeof(compressed));
+		/*
+		 * Unlike write_loose_object(), we do not have the entire
+		 * buffer. If we get Z_BUF_ERROR due to too few input bytes,
+		 * then we'll replenish them in the next input_stream->read()
+		 * call when we loop.
+		 */
+	} while (ret == Z_OK || ret == Z_BUF_ERROR);
+
+	if (stream.total_in != len + hdrlen)
+		die(_("write stream object %"PRIuMAX" != %"PRIuMAX), (uintmax_t)stream.total_in,
+		    (uintmax_t)len + hdrlen);
+
+	/*
+	 * Common steps for write_loose_object and stream_loose_object to
+	 * end writing loose object:
+	 *
+	 *  - End the compression of zlib stream.
+	 *  - Get the calculated oid.
+	 */
+	if (ret != Z_STREAM_END)
+		die(_("unable to stream deflate new object (%d)"), ret);
+	ret = end_loose_object_common(loose, &c, &compat_c, &stream, oid, &compat_oid);
+	if (ret != Z_OK)
+		die(_("deflateEnd on stream object failed (%d)"), ret);
+	close_loose_object(loose, fd, tmp_file.buf);
+
+	if (odb_freshen_object(loose->base.odb, oid)) {
+		unlink_or_warn(tmp_file.buf);
+		goto cleanup;
+	}
+	odb_loose_path(loose, &filename, oid);
+
+	/* We finally know the object path, and create the missing dir. */
+	dirlen = directory_size(filename.buf);
+	if (dirlen) {
+		struct strbuf dir = STRBUF_INIT;
+		strbuf_add(&dir, filename.buf, dirlen);
+
+		if (safe_create_dir_in_gitdir(loose->base.odb->repo, dir.buf) &&
+		    errno != EEXIST) {
+			err = error_errno(_("unable to create directory %s"), dir.buf);
+			strbuf_release(&dir);
+			goto cleanup;
+		}
+		strbuf_release(&dir);
+	}
+
+	err = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
+					 FOF_SKIP_COLLISION_CHECK);
+	if (!err && compat)
+		err = repo_add_loose_object_map(loose, oid, &compat_oid);
+cleanup:
+	strbuf_release(&tmp_file);
+	strbuf_release(&filename);
+	return err;
 }
 
 static int odb_source_loose_begin_transaction(struct odb_source *source UNUSED,
-- 
2.55.0.407.g700c83d4f3.dirty
SZEDER GáborJul 18, 2026, 19:39 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 0/9] object-file: move writing of loose objects into "loose" source

Hi Junio,
On Fri, Jul 17, 2026 at 11:32:08AM +0200, Patrick Steinhardt wrote:
Show 39 quoted lines
> Hi,
> 
> this patch series refactors "object-file.c" so that we can move the
> logic that writes loose objects into "odb/source-loose.c". This ensures
> that the logic to read and write loose objects is mostly self-contained
> within that source's implementation now.
> 
> To achieve that, this series first refactors `force_object_loose()` so
> that it can work on top of the generic `odb_source_write_object()`
> instead of having to call into `write_loose_object()`. This is the bulk
> of the patch series.
> 
> This patch series is built on top of 44de1520f0 (Merge branch 'master'
> of https://github.com/j6t/git-gui, 2026-07-16) with the following two
> topics merged into it:
> 
>   - jt/receive-pack-use-odb-transaction at bdee7b3013
>     (builtin/receive-pack: stage incoming objects via ODB transactions,
>     2026-07-10).
> 
>   - jk/git-hash-cleanups at 9e396aa553 (hash: check ctx->active flag in
>     all wrapper functions, 2026-07-07).
> 
> Note that jt/receive-pack-use-odb-transaction requires an evil merge:
> 
> diff --git a/odb/source-packed.c b/odb/source-packed.c
> index 06b31dd743..cbb06da038 100644
> --- a/odb/source-packed.c
> +++ b/odb/source-packed.c
> @@ -545,7 +545,8 @@ static int odb_source_packed_write_object_stream(struct odb_source *source UNUSE
>  }
> 
>  static int odb_source_packed_begin_transaction(struct odb_source *source UNUSED,
> -                                              struct odb_transaction **out UNUSED)
> +                                              struct odb_transaction **out UNUSED,
> +                                              enum odb_transaction_flags flags UNUSED)
>  {
>         return error("packed backend cannot begin transactions");
>  }

It seems that you performed this evil merge when merging the topic jt/receive-pack-use-odb-transaction into jch as 9727bd8447 (Merge branch 'jt/receive-pack-use-odb-transactions' into jch, 2026-07-17), but forgot to do so when creating the base for this patch series as 1d64e64326 (Merge branch 'jt/receive-pack-use-odb-transactions' into ps/odb-move-loose-object-writing, 2026-07-17). Consequently, neither 1d64e64326 nor any of the the commits of this patch series can be built because of the mismatching function signature:

  GIT_VERSION=2.55.0.256.g1d64e64326
  odb/source-packed.c: In function ‘odb_source_packed_new’:
  odb/source-packed.c:778:40: error: assignment to ‘int (*)(struct odb_source *, struct odb_transaction **, enum odb_transaction_flags)’ from incompatible pointer type ‘int (*)(struct odb_source *, struct odb_transaction **)’ [-Werror=incompatible-pointer-types]
    778 |         packed->base.begin_transaction = odb_source_packed_begin_transaction;
        |                                        ^
  cc1: all warnings being treated as errors
  make: *** [Makefile:2924: odb/source-packed.o] Error 1
Junio C HamanoJul 19, 2026, 01:04 UTC in reply to SZEDER Gábor on lore

Re: [PATCH 0/9] object-file: move writing of loose objects into "loose" source

SZEDER Gábor <szeder.dev@gmail.com> writes:
Show 27 quoted lines
> Hi Junio,
> ...
>> Note that jt/receive-pack-use-odb-transaction requires an evil merge:
>> 
>> diff --git a/odb/source-packed.c b/odb/source-packed.c
>> index 06b31dd743..cbb06da038 100644
>> --- a/odb/source-packed.c
>> +++ b/odb/source-packed.c
>> @@ -545,7 +545,8 @@ static int odb_source_packed_write_object_stream(struct odb_source *source UNUSE
>>  }
>> 
>>  static int odb_source_packed_begin_transaction(struct odb_source *source UNUSED,
>> -                                              struct odb_transaction **out UNUSED)
>> +                                              struct odb_transaction **out UNUSED,
>> +                                              enum odb_transaction_flags flags UNUSED)
>>  {
>>         return error("packed backend cannot begin transactions");
>>  }
>
> It seems that you performed this evil merge when merging the topic
> jt/receive-pack-use-odb-transaction into jch as 9727bd8447 (Merge
> branch 'jt/receive-pack-use-odb-transactions' into jch, 2026-07-17),
> but forgot to do so when creating the base for this patch series as
> 1d64e64326 (Merge branch 'jt/receive-pack-use-odb-transactions' into
> ps/odb-move-loose-object-writing, 2026-07-17).  Consequently, neither
> 1d64e64326 nor any of the the commits of this patch series can be
> built because of the mismatching function signature:

Thanks for noticing. Very much appreciated.

Will fix-up.
Junio C HamanoJul 19, 2026, 05:48 UTC in reply to Junio C Hamano on lore

Re: [PATCH 0/9] object-file: move writing of loose objects into "loose" source

Junio C Hamano <gitster@pobox.com> writes:
Show 15 quoted lines
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
>> It seems that you performed this evil merge when merging the topic
>> jt/receive-pack-use-odb-transaction into jch as 9727bd8447 (Merge
>> branch 'jt/receive-pack-use-odb-transactions' into jch, 2026-07-17),
>> but forgot to do so when creating the base for this patch series as
>> 1d64e64326 (Merge branch 'jt/receive-pack-use-odb-transactions' into
>> ps/odb-move-loose-object-writing, 2026-07-17).  Consequently, neither
>> 1d64e64326 nor any of the the commits of this patch series can be
>> built because of the mismatching function signature:
>
> Thanks for noticing.
> Very much appreciated.
>
> Will fix-up.

I've rebuilt the topic in question and pushed the results out. I also added some more automation to catch this kind of mistakes early to make it less likely to happen again.

Thanks.
Toon ClaesJul 22, 2026, 13:25 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 4/9] odb: lift object existence check out of the "loose" backend

Patrick Steinhardt <ps@pks.im> writes:
Show 38 quoted lines
> Before writing a new loose object we first check whether the object
> already exists in any of the sources attached to the object database.
> This results in a couple of issues:
>
>   - We have a layering violation, where the source needs to be aware of
>     objects stored in any of the other sources.
>
>   - Every backend would have to reimplement this check, which feels
>     somewhat pointless.
>
>   - It is not possible to easily write an object into a source in case
>     the same object already exists in another source.
>
> Refactor the code and lift up the object existence check from the
> "loose" backend into the generic ODB layer. No callers need adjustment
> as none of them write via a specific source, but via the ODB layer.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c              | 7 +++++++
>  odb/source-loose.c | 8 ++------
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/odb.c b/odb.c
> index 4adbdf8a64..bfeca76f4e 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -997,6 +997,13 @@ int odb_write_object_ext(struct object_database *odb,
>  
>  	hash_object_file(odb->repo->hash_algo, buf, len, type, oid);
>  
> +	/*
> +	 * We can skip the write in case we already have the object available.
> +	 * In that case, we only freshen its mtime.
> +	 */
> +	if (odb_freshen_object(odb, oid))
> +		return 0;
> +

For the loose backend, this calls check_and_freshen_file(), which code comment says:

    /*
     * All of the check_and_freshen functions return 1 if the file exists and was
     * freshened (if freshening was requested), 0 otherwise. If they return
     * 0, you should not assume that it is safe to skip a write of the object (it
     * either does not exist on disk, or has a stale mtime and may be subject to
     * pruning).
     */

And I see similar logic in the packed backend. So a non-zero value skips the write and the file was freshened. Makes sense.

-- 
Cheers,
Toon
Toon ClaesJul 22, 2026, 14:26 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 9/9] object-file: move logic to write loose objects

Patrick Steinhardt <ps@pks.im> writes:
Show 19 quoted lines
> The logic to write loose objects is split up across "object-file.c" and
> "odb/source-loose.c". This split is somewhat weird, but it is the result
> of two things:
>
>   - `force_object_loose()` used to reach into internals of how exactly
>     we write objects.
>
>   - The logic of writing objects is intertwined with potentially
>     starting a transaction.
>
> We have refactored `force_object_loose()` over preceding commits to work
> via generic interfaces now, so this reason doesn't exist anymore. But
> the second reason still does, as our management of "files" transactions
> and their ad-hoc creation is still very messy. This area definitely
> requires further work, and that work is indeed ongoing.
>
> That being said, we can already move the writing logic into the "loose"
> backend rather easily. All we have to do is to expose two functions that
> relate to the transactions.

I'm a bit on the fence that should have gone in a separte commit, but it's fine.

> Expose these two functions and move the writing logic into the "loose"
> backend accordingly so that it becomes more self-contained. Note that
> this requires us to drop a reference to `the_repository` in favor of
> using the source's repository in `start_loose_object_common()`.
Yay! Thanks for calling that out, it standed out in the zebra diff.
Show 5 quoted lines
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  object-file.c      | 360 +----------------------------------------------------
>  object-file.h      |  22 +---
>  odb/source-loose.c | 354 +++++++++++++++++++++++++++++++++++++++++++++++++++-
That's a pretty large diff, but luckily the zebra diff helps a lot.
> -int write_loose_object(struct odb_source_loose *loose,
> -		       const struct object_id *oid, char *hdr,
> -		       int hdrlen, const void *buf, unsigned long len,
> -		       const time_t *mtime, unsigned flags)

This line is not colored being moved because it was made static, which makes sense.

Show 18 quoted lines
> diff --git a/object-file.h b/object-file.h
> index 31781a9c53..805f2cfa28 100644
> --- a/object-file.h
> +++ b/object-file.h
> @@ -24,20 +24,6 @@ int index_path(struct index_state *istate, struct object_id *oid, const char *pa
>  struct object_info;
>  struct odb_source;
>  
> -/*
> - * Write the given stream into the loose object source. The only difference
> - * from the generic implementation of this function is that we don't perform an
> - * object existence check here.
> - *
> - * TODO: We should stop exposing this function altogether and move it into
> - * "odb/source-loose.c". This requires a couple of refactorings though to make
> - * `force_object_loose()` generic and is thus postponed to a later point in
> - * time.
> - */
This was added by you on 2026-06-01, so thanks for addressing this.
Show 28 quoted lines
> @@ -611,12 +849,120 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
>  						size_t len,
>  						struct object_id *oid)
>  {
> +	struct odb_source_loose *loose = odb_source_loose_downcast(source);
> +	const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
> +	struct object_id compat_oid;
> +	int fd, ret, err = 0, flush = 0;
> +	unsigned char compressed[4096];
> +	git_zstream stream;
> +	struct git_hash_ctx c, compat_c;
> +	struct strbuf tmp_file = STRBUF_INIT;
> +	struct strbuf filename = STRBUF_INIT;
> +	unsigned char buf[8192];
> +	int dirlen;
> +	char hdr[MAX_HEADER_LEN];
> +	int hdrlen;
> +
> +	if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
> +		odb_transaction_files_prepare(loose->base.odb->transaction);
> +
> +	/* Since oid is not determined, save tmp file to odb path. */
> +	strbuf_addf(&filename, "%s/", loose->base.path);
> +	hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, len);
> +
>  	/*
> -	 * TODO: the implementation should be moved here, see the comment on
> -	 * the called function in "object-file.h".
So this is what you did, as suggested, by yourself.
Show 9 quoted lines
> +	 * Common steps for write_loose_object and stream_loose_object to
> +	 * start writing loose objects:
> +	 *
> +	 *  - Create tmpfile for the loose object.
> +	 *  - Setup zlib stream for compression.
> +	 *  - Start to feed header to zlib stream.
>  	 */
> -	struct odb_source_loose *loose = odb_source_loose_downcast(source);
> -	return odb_source_loose_write_stream(loose, in_stream, len, oid);

This line is marked as removed in the zebra diff, but that's because the code is being inlined into this odb_source_loose_write_object_stream() function.

All good.
-- 
Cheers,
Toon
Justin ToblerJul 28, 2026, 22:02 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 1/9] odb: compute compat object ID in `odb_write_object_ext()`

On 26/07/17 11:32AM, Patrick Steinhardt wrote:
Show 6 quoted lines
> Repositories can have a compatibility hash configured, which means that
> such a repository is expected to maintain a mapping between canonical
> and compatibility object hashes. Maintaining this mapping is the
> responsibility of the object database sources, where we either store
> them as part of the loose objects map or in packfile indices v3 (once we
> gain support for this feature).

Makes sense. Each ODB source should be responsible to tracking how an objects maps from one hash to another for compatibility.

Show 6 quoted lines
> But besides storing these compatibility hashes, the sources are also
> responsible for generating the compatibility hash in the first place.
> This is somewhat unnecessary though, as the compatibility hash should be
> computed the same no matter which source is being used. The consequence
> is that we need to duplicate this functionality across the different
> backends, which does not make a lot of sense.

Agreed, there is no need to duplicate logic as the hashes that get generated should be the same regardless of the backend.

Show 49 quoted lines
> Refactor the code so that we instead compute the compatibility hash in
> `odb_write_object_ext()` and then pass the computed value to the
> sources. No callers need adjustment as there are none that write objects
> via the source interfaces directly.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c                 | 26 ++++++++++++++++++++++++--
>  odb.h                 | 10 ++++++----
>  odb/source-files.c    |  2 +-
>  odb/source-inmemory.c |  2 +-
>  odb/source-loose.c    | 24 +++---------------------
>  odb/source-packed.c   |  2 +-
>  odb/source.h          |  4 ++--
>  7 files changed, 38 insertions(+), 32 deletions(-)
> 
> diff --git a/odb.c b/odb.c
> index cf6e7938c0..1d6538163b 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -989,11 +989,33 @@ int odb_write_object_ext(struct object_database *odb,
>  			 const void *buf, unsigned long len,
>  			 enum object_type type,
>  			 struct object_id *oid,
> -			 struct object_id *compat_oid,
> +			 const struct object_id *compat_oid_in,
>  			 enum odb_write_object_flags flags)
>  {
> +	const struct git_hash_algo *compat = odb->repo->compat_hash_algo;
> +	struct object_id compat_oid, *compat_oid_p = NULL;
> +
> +	if (compat) {
> +		const struct git_hash_algo *algo = odb->repo->hash_algo;
> +
> +		if (compat_oid_in) {
> +			oidcpy(&compat_oid, compat_oid_in);
> +		} else if (type == OBJ_BLOB) {
> +			hash_object_file(compat, buf, len, type, &compat_oid);
> +		} else {
> +			struct strbuf converted = STRBUF_INIT;
> +			convert_object_file(odb->repo, &converted, algo, compat,
> +					    buf, len, type, 0);
> +			hash_object_file(compat, converted.buf, converted.len,
> +					 type, &compat_oid);
> +			strbuf_release(&converted);
> +		}
> +
> +		compat_oid_p = &compat_oid;
> +	}

Here we lift up the logic to gnerate the compat hash out of the backend and into `odb_write_object_ext()` so the resulting hash can be wired to the ODB source callback to write the object. The logic itself is the same and looks good.

The rest of this patch is mainly just updating the callsites accordingly and also looks good.

-Justin
Justin ToblerJul 28, 2026, 22:10 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 2/9] t/u-odb-inmemory: implement wrapper for writing objects

On 26/07/17 11:32AM, Patrick Steinhardt wrote:
> In the next commit we're about to change how objects are being written
> into the object database source. Prepare for this refactoring by
> introducing a wrapper function into our unit tests so that we don't have
> to adjust all callsites.

Ok, I assume future patches will extend the `cl_assert_write_object()` wrapper function logic which would make sense. The patch itself looks trivially correct.

-Justin

Back to recent threads