* [PATCH 0/9] object-file: move writing of loose objects into "loose" source
@ 2026-07-17 9:32 Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 1/9] odb: compute compat object ID in `odb_write_object_ext()` Patrick Steinhardt
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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");
}
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/9] odb: compute compat object ID in `odb_write_object_ext()`
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 2/9] t/u-odb-inmemory: implement wrapper for writing objects Patrick Steinhardt
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/9] t/u-odb-inmemory: implement wrapper for writing objects
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 1/9] odb: compute compat object ID in `odb_write_object_ext()` Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 3/9] odb: compute object hash in `odb_write_object_ext()` Patrick Steinhardt
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/9] odb: compute object hash in `odb_write_object_ext()`
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 1/9] odb: compute compat object ID in `odb_write_object_ext()` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 2/9] t/u-odb-inmemory: implement wrapper for writing objects Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 4/9] odb: lift object existence check out of the "loose" backend Patrick Steinhardt
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/9] odb: lift object existence check out of the "loose" backend
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
` (2 preceding siblings ...)
2026-07-17 9:32 ` [PATCH 3/9] odb: compute object hash in `odb_write_object_ext()` Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 5/9] odb: support setting mtime when writing objects Patrick Steinhardt
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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;
+
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/9] odb: support setting mtime when writing objects
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
` (3 preceding siblings ...)
2026-07-17 9:32 ` [PATCH 4/9] odb: lift object existence check out of the "loose" backend Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 6/9] object-file: fix memory leak in `force_object_loose()` Patrick Steinhardt
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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 = ×
+ }
+
+ 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 = ×
+ }
+
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 6/9] object-file: fix memory leak in `force_object_loose()`
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
` (4 preceding siblings ...)
2026-07-17 9:32 ` [PATCH 5/9] odb: support setting mtime when writing objects Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 7/9] object-file: force objects loose via generic interface Patrick Steinhardt
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 7/9] object-file: force objects loose via generic interface
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
` (5 preceding siblings ...)
2026-07-17 9:32 ` [PATCH 6/9] object-file: fix memory leak in `force_object_loose()` Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 8/9] object-file: move `force_object_loose()` Patrick Steinhardt
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 8/9] object-file: move `force_object_loose()`
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
` (6 preceding siblings ...)
2026-07-17 9:32 ` [PATCH 7/9] object-file: force objects loose via generic interface Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 9/9] object-file: move logic to write loose objects Patrick Steinhardt
2026-07-18 19:39 ` [PATCH 0/9] object-file: move writing of loose objects into "loose" source SZEDER Gábor
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 9/9] object-file: move logic to write loose objects
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
` (7 preceding siblings ...)
2026-07-17 9:32 ` [PATCH 8/9] object-file: move `force_object_loose()` Patrick Steinhardt
@ 2026-07-17 9:32 ` Patrick Steinhardt
2026-07-18 19:39 ` [PATCH 0/9] object-file: move writing of loose objects into "loose" source SZEDER Gábor
9 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-07-17 9:32 UTC (permalink / raw)
To: git; +Cc: Justin Tobler
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(-)
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, ¶no_oid, NULL);
- if (ret != Z_OK)
- die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
- ret);
- if (!oideq(oid, ¶no_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, ¶no_oid, NULL);
+ if (ret != Z_OK)
+ die(_("deflateEnd on object %s failed (%d)"), oid_to_hex(oid),
+ ret);
+ if (!oideq(oid, ¶no_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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/9] object-file: move writing of loose objects into "loose" source
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
` (8 preceding siblings ...)
2026-07-17 9:32 ` [PATCH 9/9] object-file: move logic to write loose objects Patrick Steinhardt
@ 2026-07-18 19:39 ` SZEDER Gábor
9 siblings, 0 replies; 11+ messages in thread
From: SZEDER Gábor @ 2026-07-18 19:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Patrick Steinhardt, git, Justin Tobler
Hi Junio,
On Fri, Jul 17, 2026 at 11:32:08AM +0200, Patrick Steinhardt wrote:
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-18 19:39 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 9:32 [PATCH 0/9] object-file: move writing of loose objects into "loose" source Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 1/9] odb: compute compat object ID in `odb_write_object_ext()` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 2/9] t/u-odb-inmemory: implement wrapper for writing objects Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 3/9] odb: compute object hash in `odb_write_object_ext()` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 4/9] odb: lift object existence check out of the "loose" backend Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 5/9] odb: support setting mtime when writing objects Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 6/9] object-file: fix memory leak in `force_object_loose()` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 7/9] object-file: force objects loose via generic interface Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 8/9] object-file: move `force_object_loose()` Patrick Steinhardt
2026-07-17 9:32 ` [PATCH 9/9] object-file: move logic to write loose objects Patrick Steinhardt
2026-07-18 19:39 ` [PATCH 0/9] object-file: move writing of loose objects into "loose" source SZEDER Gábor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox