* [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-12 6:07 ` Patrick Steinhardt
2026-08-11 17:54 ` [PATCH v3 2/9] odb/transaction: add transaction finalize interface Justin Tobler
` (8 subsequent siblings)
9 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When git-receive-pack(1) stores an incoming packfile with
git-index-pack(1), a ".keep" file is written alongside it to hold the
pack in place until the references have been updated, and is removed
afterwards. The path used to remove it is derived via
`index_pack_lockfile()` from the repository's primary object directory.
In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10), git-receive-pack(1) started using the ODB
transaction interfaces instead of managing a temporary directory
directly. When starting an ODB transaction, the sources list is
reordered to insert the newly created transaction source first as the
primary to ensure writes are routed to it accordingly.
Prior to using ODB transactions, git-receive-pack(1) would only set the
temporary directory as the primary source for the child
git-index-pack(1) and git-unpack-objects(1) processes it spawned and the
parent process would set the temporary directory set as an alternate
only. By using ODB transactions, the ODB source list is also reordered
for the parent process which results in `index_pack_lockfile()` deriving
the ".keep" path relative to the temporary directory instead the actual
main ODB source path. Consequently, this prevents the ".keep" file from
being properly removed after being migrated into the main ODB source
post-commit.
Update `index_pack_lockfile()` to operate on an ODB source explicitly
provided to it and update call sites accordingly to pass the expected
ODB source.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 8 +++++++-
fetch-pack.c | 2 +-
pack-write.c | 7 ++++---
pack.h | 4 +++-
t/t5547-push-quarantine.sh | 14 ++++++++++++++
5 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 86933d8d7e..d74b787148 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si,
if (status)
return "index-pack fork failed";
- lockfile = index_pack_lockfile(the_repository, child.out, NULL);
+ /*
+ * The lockfile filepath is expected to be the final location of
+ * the ".keep" file after being migrated to the main ODB source.
+ * This ensures the lockfile can be found and removed later
+ * after the ODB transaction has been committed.
+ */
+ lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
if (lockfile) {
pack_lockfile = register_tempfile(lockfile);
free(lockfile);
diff --git a/fetch-pack.c b/fetch-pack.c
index 922a9b2581..6df5813b33 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1075,7 +1075,7 @@ static int get_pack(struct fetch_pack_args *args,
die(_("fetch-pack: unable to fork off %s"), cmd_name);
if (do_keep && (pack_lockfiles || fsck_objects)) {
int is_well_formed;
- char *pack_lockfile = index_pack_lockfile(the_repository,
+ char *pack_lockfile = index_pack_lockfile(the_repository->objects->sources,
cmd.out,
&is_well_formed);
diff --git a/pack-write.c b/pack-write.c
index 24033a9101..85674e4b72 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -469,10 +469,11 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,
fsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);
}
-char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)
+char *index_pack_lockfile(struct odb_source *source, int ip_out,
+ int *is_well_formed)
{
char packname[GIT_MAX_HEXSZ + 6];
- const int len = r->hash_algo->hexsz + 6;
+ const int len = source->odb->repo->hash_algo->hexsz + 6;
/*
* The first thing we expect from index-pack's output
@@ -489,7 +490,7 @@ char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)
packname[len-1] = 0;
if (skip_prefix(packname, "keep\t", &name))
return xstrfmt("%s/pack/pack-%s.keep",
- repo_get_object_directory(r), name);
+ source->path, name);
return NULL;
}
if (is_well_formed)
diff --git a/pack.h b/pack.h
index 1cde92082b..68dcf08cf3 100644
--- a/pack.h
+++ b/pack.h
@@ -3,6 +3,7 @@
#include "object.h"
#include "csum-file.h"
+#include "odb/source.h"
struct packed_git;
struct pack_window;
@@ -105,7 +106,8 @@ off_t write_pack_header(struct hashfile *f, uint32_t);
void fixup_pack_header_footer(const struct git_hash_algo *, int,
unsigned char *, const char *, uint32_t,
unsigned char *, off_t);
-char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed);
+char *index_pack_lockfile(struct odb_source *source, int fd,
+ int *is_well_formed);
struct ref;
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 0798ddab02..400a597606 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -70,4 +70,18 @@ test_expect_success 'updating a ref from quarantine is forbidden' '
git -C update.git fsck
'
+test_expect_success '.keep file is removed after push' '
+ test_when_finished rm -rf keep.git &&
+ git init --bare keep.git &&
+
+ git -C keep.git config set receive.unpackLimit 0 &&
+ test_commit foo &&
+ git push keep.git HEAD &&
+ pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
+ keep="${pack%.pack}.keep" &&
+
+ test_path_is_file "$pack" &&
+ test_path_is_missing "$keep"
+'
+
test_done
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files
2026-08-11 17:54 ` [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
@ 2026-08-12 6:07 ` Patrick Steinhardt
2026-08-13 21:45 ` Justin Tobler
0 siblings, 1 reply; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-12 6:07 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Tue, Aug 11, 2026 at 12:54:07PM -0500, Justin Tobler wrote:
> When git-receive-pack(1) stores an incoming packfile with
> git-index-pack(1), a ".keep" file is written alongside it to hold the
> pack in place until the references have been updated, and is removed
> afterwards. The path used to remove it is derived via
> `index_pack_lockfile()` from the repository's primary object directory.
>
> In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
> transactions, 2026-07-10), git-receive-pack(1) started using the ODB
> transaction interfaces instead of managing a temporary directory
> directly. When starting an ODB transaction, the sources list is
> reordered to insert the newly created transaction source first as the
> primary to ensure writes are routed to it accordingly.
>
> Prior to using ODB transactions, git-receive-pack(1) would only set the
> temporary directory as the primary source for the child
> git-index-pack(1) and git-unpack-objects(1) processes it spawned and the
> parent process would set the temporary directory set as an alternate
> only. By using ODB transactions, the ODB source list is also reordered
> for the parent process which results in `index_pack_lockfile()` deriving
> the ".keep" path relative to the temporary directory instead the actual
Nit: s/instead/& of/
> main ODB source path. Consequently, this prevents the ".keep" file from
> being properly removed after being migrated into the main ODB source
> post-commit.
Hm. Are the temporary packs written into the transaction-managed tempdir
now, or do they still end up in the main object directory?
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 86933d8d7e..d74b787148 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si,
> if (status)
> return "index-pack fork failed";
>
> - lockfile = index_pack_lockfile(the_repository, child.out, NULL);
> + /*
> + * The lockfile filepath is expected to be the final location of
> + * the ".keep" file after being migrated to the main ODB source.
> + * This ensures the lockfile can be found and removed later
> + * after the ODB transaction has been committed.
> + */
> + lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
> if (lockfile) {
> pack_lockfile = register_tempfile(lockfile);
> free(lockfile);
Okay. So previously, we wrote the ".keep" file into the main repository,
whereas now we write it into the temporary object directory? Is the
packfile itself also written in there?
What I'm wondering is why we even need a ".keep" file at all anymore if
we're not storing it in the main object directory. It wouldn't help us
to avoid the race, because after committing the transaction the ".keep"
file would remain in the temporary directory, whereas the packfile would
have been migrated to the main object directory. So it doesn't have a
".keep" file at that point, and neither have references been updated to
point to the new objects yet.
So I wonder whether instead, we'd have to:
1. Start the transaction, creating the temporary object directory.
2. Write the packfile into the temporary object directory, but don't
create a ".keep" file.
3. At commit time, first write a ".keep" file in the main object
directory and then migrate the packfile over.
4. At finalization time, prune the ".keep" file from the main object
directory.
That would retain the current properties of the system, but as far as I
can see this is not what we're doing here.
> diff --git a/pack.h b/pack.h
> index 1cde92082b..68dcf08cf3 100644
> --- a/pack.h
> +++ b/pack.h
> @@ -3,6 +3,7 @@
>
> #include "object.h"
> #include "csum-file.h"
> +#include "odb/source.h"
>
> struct packed_git;
> struct pack_window;
Let's add a forward declaration instead of including this header.
> diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
> index 0798ddab02..400a597606 100755
> --- a/t/t5547-push-quarantine.sh
> +++ b/t/t5547-push-quarantine.sh
> @@ -70,4 +70,18 @@ test_expect_success 'updating a ref from quarantine is forbidden' '
> git -C update.git fsck
> '
>
> +test_expect_success '.keep file is removed after push' '
> + test_when_finished rm -rf keep.git &&
> + git init --bare keep.git &&
> +
> + git -C keep.git config set receive.unpackLimit 0 &&
> + test_commit foo &&
> + git push keep.git HEAD &&
> + pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
> + keep="${pack%.pack}.keep" &&
> +
> + test_path_is_file "$pack" &&
> + test_path_is_missing "$keep"
> +'
This would feel a bit safer if we had a hook that verifies that we
indeed have the ".keep" file in the right spot before committing
everything.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 78+ messages in thread* Re: [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files
2026-08-12 6:07 ` Patrick Steinhardt
@ 2026-08-13 21:45 ` Justin Tobler
2026-08-14 7:46 ` Patrick Steinhardt
0 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-13 21:45 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, gitster
On 26/08/12 08:07AM, Patrick Steinhardt wrote:
> On Tue, Aug 11, 2026 at 12:54:07PM -0500, Justin Tobler wrote:
> > When git-receive-pack(1) stores an incoming packfile with
> > git-index-pack(1), a ".keep" file is written alongside it to hold the
> > pack in place until the references have been updated, and is removed
> > afterwards. The path used to remove it is derived via
> > `index_pack_lockfile()` from the repository's primary object directory.
> >
> > In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
> > transactions, 2026-07-10), git-receive-pack(1) started using the ODB
> > transaction interfaces instead of managing a temporary directory
> > directly. When starting an ODB transaction, the sources list is
> > reordered to insert the newly created transaction source first as the
> > primary to ensure writes are routed to it accordingly.
> >
> > Prior to using ODB transactions, git-receive-pack(1) would only set the
> > temporary directory as the primary source for the child
> > git-index-pack(1) and git-unpack-objects(1) processes it spawned and the
> > parent process would set the temporary directory set as an alternate
> > only. By using ODB transactions, the ODB source list is also reordered
> > for the parent process which results in `index_pack_lockfile()` deriving
> > the ".keep" path relative to the temporary directory instead the actual
>
> Nit: s/instead/& of/
Will fix.
> > main ODB source path. Consequently, this prevents the ".keep" file from
> > being properly removed after being migrated into the main ODB source
> > post-commit.
>
> Hm. Are the temporary packs written into the transaction-managed tempdir
> now, or do they still end up in the main object directory?
The packfile and associated ".keep" lockfiles are both initially written
into the temporary directory managed by the ODB transaction. On
transaction commit, they are then both migrated to the main ODB.
When registering the keep tempfile, we need to record the future
post-commit location of the keep file that way it can be removed when
`odb_transaction_finalize()` is invoked. This matches the original
behavior prior to ODB transaction being introduced in
git-receive-pack(1).
> > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> > index 86933d8d7e..d74b787148 100644
> > --- a/builtin/receive-pack.c
> > +++ b/builtin/receive-pack.c
> > @@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si,
> > if (status)
> > return "index-pack fork failed";
> >
> > - lockfile = index_pack_lockfile(the_repository, child.out, NULL);
> > + /*
> > + * The lockfile filepath is expected to be the final location of
> > + * the ".keep" file after being migrated to the main ODB source.
> > + * This ensures the lockfile can be found and removed later
> > + * after the ODB transaction has been committed.
> > + */
> > + lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
> > if (lockfile) {
> > pack_lockfile = register_tempfile(lockfile);
> > free(lockfile);
>
> Okay. So previously, we wrote the ".keep" file into the main repository,
> whereas now we write it into the temporary object directory? Is the
> packfile itself also written in there?
Not quite, both the packfile and keep file were written to the temporary
directory and continue to do so.
Prior to bdee7b3013 (builtin/receive-pack: stage incoming objects via
ODB transactions, 2026-07-10), the ".keep" files were also being written
to the quarantine directory and migrated alongside the packfiles. The
main git-receive-pack(1) process always kept the primary ODB as the
first entry in the source list though ensuring that the "filename"
registered for keep tempfile was the final location. With ODB
transactions though, the source list order _does_ get changed and
resulted in the keep tempfile not knowing about its final location.
Consequently, it is no longer cleaned up.
> What I'm wondering is why we even need a ".keep" file at all anymore if
> we're not storing it in the main object directory. It wouldn't help us
> to avoid the race, because after committing the transaction the ".keep"
> file would remain in the temporary directory, whereas the packfile would
> have been migrated to the main object directory. So it doesn't have a
> ".keep" file at that point, and neither have references been updated to
> point to the new objects yet.
The ".keep" file does end up in the main ODB alongside the packfile when
the transaction is committed. The main problem here is that it is not
being cleaned up because the post-migration path does not match what the
registered tempfile tracks.
> So I wonder whether instead, we'd have to:
>
> 1. Start the transaction, creating the temporary object directory.
>
> 2. Write the packfile into the temporary object directory, but don't
> create a ".keep" file.
>
> 3. At commit time, first write a ".keep" file in the main object
> directory and then migrate the packfile over.
>
> 4. At finalization time, prune the ".keep" file from the main object
> directory.
>
> That would retain the current properties of the system, but as far as I
> can see this is not what we're doing here.
With this patch, this is effectly what we are doing already. The main
difference is that we are creating the ".keep" file alongside the
packfile via git-index-pack(1) and migrating both when
`odb_transaction_commit()` is invoked.
We could stop relying on git-index-pack(1) to generate the ".keep" file
and instead generate it ourselves during the commit phase as you
suggested, but I'm not sure that would really buy us anything right now.
For now, I think it would be fine to keep the changes more minimal.
I'll try to clarify the commit message a bit in the next version to
better explain what is happening.
> > diff --git a/pack.h b/pack.h
> > index 1cde92082b..68dcf08cf3 100644
> > --- a/pack.h
> > +++ b/pack.h
> > @@ -3,6 +3,7 @@
> >
> > #include "object.h"
> > #include "csum-file.h"
> > +#include "odb/source.h"
> >
> > struct packed_git;
> > struct pack_window;
>
> Let's add a forward declaration instead of including this header.
Will do.
> > diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
> > index 0798ddab02..400a597606 100755
> > --- a/t/t5547-push-quarantine.sh
> > +++ b/t/t5547-push-quarantine.sh
> > @@ -70,4 +70,18 @@ test_expect_success 'updating a ref from quarantine is forbidden' '
> > git -C update.git fsck
> > '
> >
> > +test_expect_success '.keep file is removed after push' '
> > + test_when_finished rm -rf keep.git &&
> > + git init --bare keep.git &&
> > +
> > + git -C keep.git config set receive.unpackLimit 0 &&
> > + test_commit foo &&
> > + git push keep.git HEAD &&
> > + pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
> > + keep="${pack%.pack}.keep" &&
> > +
> > + test_path_is_file "$pack" &&
> > + test_path_is_missing "$keep"
> > +'
>
> This would feel a bit safer if we had a hook that verifies that we
> indeed have the ".keep" file in the right spot before committing
> everything.
I'll try to set something up in the next version. Thanks.
-Justin
^ permalink raw reply [flat|nested] 78+ messages in thread* Re: [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files
2026-08-13 21:45 ` Justin Tobler
@ 2026-08-14 7:46 ` Patrick Steinhardt
0 siblings, 0 replies; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-14 7:46 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Thu, Aug 13, 2026 at 04:45:16PM -0500, Justin Tobler wrote:
> On 26/08/12 08:07AM, Patrick Steinhardt wrote:
> > On Tue, Aug 11, 2026 at 12:54:07PM -0500, Justin Tobler wrote:
> > > When git-receive-pack(1) stores an incoming packfile with
> > > git-index-pack(1), a ".keep" file is written alongside it to hold the
> > > pack in place until the references have been updated, and is removed
> > > afterwards. The path used to remove it is derived via
> > > `index_pack_lockfile()` from the repository's primary object directory.
> > >
> > > In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
> > > transactions, 2026-07-10), git-receive-pack(1) started using the ODB
> > > transaction interfaces instead of managing a temporary directory
> > > directly. When starting an ODB transaction, the sources list is
> > > reordered to insert the newly created transaction source first as the
> > > primary to ensure writes are routed to it accordingly.
> > >
> > > Prior to using ODB transactions, git-receive-pack(1) would only set the
> > > temporary directory as the primary source for the child
> > > git-index-pack(1) and git-unpack-objects(1) processes it spawned and the
> > > parent process would set the temporary directory set as an alternate
> > > only. By using ODB transactions, the ODB source list is also reordered
> > > for the parent process which results in `index_pack_lockfile()` deriving
> > > the ".keep" path relative to the temporary directory instead the actual
> >
> > Nit: s/instead/& of/
>
> Will fix.
>
> > > main ODB source path. Consequently, this prevents the ".keep" file from
> > > being properly removed after being migrated into the main ODB source
> > > post-commit.
> >
> > Hm. Are the temporary packs written into the transaction-managed tempdir
> > now, or do they still end up in the main object directory?
>
> The packfile and associated ".keep" lockfiles are both initially written
> into the temporary directory managed by the ODB transaction. On
> transaction commit, they are then both migrated to the main ODB.
>
> When registering the keep tempfile, we need to record the future
> post-commit location of the keep file that way it can be removed when
> `odb_transaction_finalize()` is invoked. This matches the original
> behavior prior to ODB transaction being introduced in
> git-receive-pack(1).
>
> > > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> > > index 86933d8d7e..d74b787148 100644
> > > --- a/builtin/receive-pack.c
> > > +++ b/builtin/receive-pack.c
> > > @@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si,
> > > if (status)
> > > return "index-pack fork failed";
> > >
> > > - lockfile = index_pack_lockfile(the_repository, child.out, NULL);
> > > + /*
> > > + * The lockfile filepath is expected to be the final location of
> > > + * the ".keep" file after being migrated to the main ODB source.
> > > + * This ensures the lockfile can be found and removed later
> > > + * after the ODB transaction has been committed.
> > > + */
> > > + lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
> > > if (lockfile) {
> > > pack_lockfile = register_tempfile(lockfile);
> > > free(lockfile);
> >
> > Okay. So previously, we wrote the ".keep" file into the main repository,
> > whereas now we write it into the temporary object directory? Is the
> > packfile itself also written in there?
>
> Not quite, both the packfile and keep file were written to the temporary
> directory and continue to do so.
>
> Prior to bdee7b3013 (builtin/receive-pack: stage incoming objects via
> ODB transactions, 2026-07-10), the ".keep" files were also being written
> to the quarantine directory and migrated alongside the packfiles. The
> main git-receive-pack(1) process always kept the primary ODB as the
> first entry in the source list though ensuring that the "filename"
> registered for keep tempfile was the final location. With ODB
> transactions though, the source list order _does_ get changed and
> resulted in the keep tempfile not knowing about its final location.
> Consequently, it is no longer cleaned up.
>
> > What I'm wondering is why we even need a ".keep" file at all anymore if
> > we're not storing it in the main object directory. It wouldn't help us
> > to avoid the race, because after committing the transaction the ".keep"
> > file would remain in the temporary directory, whereas the packfile would
> > have been migrated to the main object directory. So it doesn't have a
> > ".keep" file at that point, and neither have references been updated to
> > point to the new objects yet.
>
> The ".keep" file does end up in the main ODB alongside the packfile when
> the transaction is committed. The main problem here is that it is not
> being cleaned up because the post-migration path does not match what the
> registered tempfile tracks.
>
> > So I wonder whether instead, we'd have to:
> >
> > 1. Start the transaction, creating the temporary object directory.
> >
> > 2. Write the packfile into the temporary object directory, but don't
> > create a ".keep" file.
> >
> > 3. At commit time, first write a ".keep" file in the main object
> > directory and then migrate the packfile over.
> >
> > 4. At finalization time, prune the ".keep" file from the main object
> > directory.
> >
> > That would retain the current properties of the system, but as far as I
> > can see this is not what we're doing here.
>
> With this patch, this is effectly what we are doing already. The main
> difference is that we are creating the ".keep" file alongside the
> packfile via git-index-pack(1) and migrating both when
> `odb_transaction_commit()` is invoked.
>
> We could stop relying on git-index-pack(1) to generate the ".keep" file
> and instead generate it ourselves during the commit phase as you
> suggested, but I'm not sure that would really buy us anything right now.
> For now, I think it would be fine to keep the changes more minimal.
>
> I'll try to clarify the commit message a bit in the next version to
> better explain what is happening.
Thanks for the explanation, this helped a lot!
Patrick
^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v3 2/9] odb/transaction: add transaction finalize interface
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-11 17:54 ` [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-12 6:07 ` Patrick Steinhardt
2026-08-11 17:54 ` [PATCH v3 3/9] builtin/receive-pack: pass shallow file explicitly Justin Tobler
` (7 subsequent siblings)
9 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When committing an ODB transaction via `odb_transaction_commit()`, the
staged objects are made visible and the underlying transaction is freed
at the same time. Coupling these two steps does not leave room for any
post-commit transaction operations to be introduced though. Such a
capability is useful if an ODB transaction backend needs to hold on to
lockfiles after transaction commit until references are updated, as is
the case with the existing "files" backend in git-receive-pack(1).
Stop freeing the transaction in `odb_transaction_commit()` and introduce
`odb_transaction_finalize()` to explicitly clean up the transaction
accordingly. Note that the finalize interface also provides an optional
callback for any backend-specific deferred cleanup. In a subsequent
commit, the "files" transaction backend will use this to remove ".keep"
files generated for packfiles received via git-receive-pack(1) after
references have been updated. In preparation for this, the
`odb_transaction_finalize()` call site in git-receive-pack(1) is made
after the reference updates are finished.
All other callers commit a transaction and immediately finalize it with
no work in between and cannot meaningfully recover should either step
fail, so introduce an `odb_transaction_commit_and_finalize_or_die()`
helper that performs both and dies on error. Call sites are updated
accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/add.c | 4 ++--
builtin/receive-pack.c | 1 +
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 4 ++--
cache-tree.c | 2 +-
object-file.c | 2 +-
odb/transaction.c | 14 ++++++++++++++
odb/transaction.h | 23 +++++++++++++++++++++++
read-cache.c | 2 +-
9 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 60ffbede2b..ad418a5952 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -393,7 +393,7 @@ int cmd_add(int argc,
char *seen = NULL;
char *ps_matched = NULL;
struct lock_file lock_file = LOCK_INIT;
- struct odb_transaction *transaction;
+ struct odb_transaction *transaction = NULL;
repo_config(repo, add_config, NULL);
@@ -600,7 +600,7 @@ int cmd_add(int argc,
if (chmod_arg && pathspec.nr)
exit_status |= chmod_pathspec(repo, &pathspec, chmod_arg[0], show_only);
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
finish:
if (write_locked_index(repo->index, &lock_file,
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d74b787148..ed1edcbe93 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2720,6 +2720,7 @@ int cmd_receive_pack(int argc,
use_keepalive = KEEPALIVE_ALWAYS;
execute_commands(commands, unpack_status, &si, transaction,
&push_options);
+ odb_transaction_finalize(transaction);
delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 4263edfbec..d6a2d616d9 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -603,7 +603,7 @@ static void unpack_all(void)
unpack_one(i);
display_progress(progress, i + 1);
}
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
stop_progress(&progress);
if (delta_list)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 241abd4332..b25d4ecb10 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1156,7 +1156,7 @@ int cmd_update_index(int argc,
* a transaction.
*/
if (transaction && verbose) {
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
transaction = NULL;
}
@@ -1224,7 +1224,7 @@ int cmd_update_index(int argc,
/*
* By now we have added all of the new objects
*/
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
if (split_index > 0) {
if (repo_config_get_split_index(the_repository) == 0)
diff --git a/cache-tree.c b/cache-tree.c
index d92f513286..a220372a42 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -538,7 +538,7 @@ int cache_tree_update(struct index_state *istate, int flags)
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
trace2_region_leave("cache_tree", "update", istate->repo);
trace_performance_leave("cache_tree_update");
if (i < 0)
diff --git a/object-file.c b/object-file.c
index ec35c318bc..4d03c167d5 100644
--- a/object-file.c
+++ b/object-file.c
@@ -965,7 +965,7 @@ int index_fd(struct index_state *istate, struct object_id *oid,
xsize_t(st->st_size),
oid);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
} else {
ret = hash_blob_stream(&stream,
the_repository->hash_algo, oid,
diff --git a/odb/transaction.c b/odb/transaction.c
index dab7da6a9a..9e9a982778 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -33,6 +33,20 @@ int odb_transaction_commit(struct odb_transaction *transaction)
ret = transaction->commit(transaction);
transaction->source->odb->transaction = NULL;
+
+ return ret;
+}
+
+int odb_transaction_finalize(struct odb_transaction *transaction)
+{
+ int ret = 0;
+
+ if (!transaction)
+ return 0;
+
+ if (transaction->finalize)
+ ret = transaction->finalize(transaction);
+
free(transaction);
return ret;
diff --git a/odb/transaction.h b/odb/transaction.h
index 4cb2eafcbf..6ed39b3d0e 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -22,6 +22,13 @@ struct odb_transaction {
*/
int (*commit)(struct odb_transaction *transaction);
+ /*
+ * Optional ODB source specific callback invoked when the transaction
+ * needs to perform any deferred cleanup after objects have been
+ * committed. Returns 0 on success, a negative error code otherwise.
+ */
+ int (*finalize)(struct odb_transaction *transaction);
+
/*
* This callback is expected to write the given object stream into
* the ODB transaction. Note that for now, only blobs support streaming.
@@ -75,6 +82,22 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
*/
int odb_transaction_commit(struct odb_transaction *transaction);
+/*
+ * Finalizes an ODB transaction, performing any deferred cleanup and freeing it.
+ * Must be called for every successfully started transaction. Note that, if the
+ * specified transaction is NULL, the function is a no-op. Returns 0 on success,
+ * a negative error code otherwise.
+ */
+int odb_transaction_finalize(struct odb_transaction *transaction);
+
+static inline void odb_transaction_commit_and_finalize_or_die(struct odb_transaction *transaction)
+{
+ if (odb_transaction_commit(transaction))
+ die(_("failed to commit ODB transaction"));
+ if (odb_transaction_finalize(transaction))
+ die(_("failed to finalize ODB transaction"));
+}
+
/*
* Writes the object in the provided stream into the transaction. The resulting
* object ID is written into the out pointer. Returns 0 on success, a negative
diff --git a/read-cache.c b/read-cache.c
index 6c449f393d..0cd0ef85ec 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4049,7 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
odb_transaction_begin_or_die(repo->objects, &transaction, 0);
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
release_revisions(&rev);
return !!data.add_errors;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v3 2/9] odb/transaction: add transaction finalize interface
2026-08-11 17:54 ` [PATCH v3 2/9] odb/transaction: add transaction finalize interface Justin Tobler
@ 2026-08-12 6:07 ` Patrick Steinhardt
0 siblings, 0 replies; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-12 6:07 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Tue, Aug 11, 2026 at 12:54:08PM -0500, Justin Tobler wrote:
> When committing an ODB transaction via `odb_transaction_commit()`, the
> staged objects are made visible and the underlying transaction is freed
> at the same time. Coupling these two steps does not leave room for any
> post-commit transaction operations to be introduced though. Such a
> capability is useful if an ODB transaction backend needs to hold on to
> lockfiles after transaction commit until references are updated, as is
> the case with the existing "files" backend in git-receive-pack(1).
>
> Stop freeing the transaction in `odb_transaction_commit()` and introduce
> `odb_transaction_finalize()` to explicitly clean up the transaction
> accordingly. Note that the finalize interface also provides an optional
> callback for any backend-specific deferred cleanup. In a subsequent
> commit, the "files" transaction backend will use this to remove ".keep"
> files generated for packfiles received via git-receive-pack(1) after
> references have been updated. In preparation for this, the
> `odb_transaction_finalize()` call site in git-receive-pack(1) is made
> after the reference updates are finished.
>
> All other callers commit a transaction and immediately finalize it with
> no work in between and cannot meaningfully recover should either step
> fail, so introduce an `odb_transaction_commit_and_finalize_or_die()`
"step fail"? I guess this ought to just read "fail"?
> helper that performs both and dies on error. Call sites are updated
> accordingly.
>
> Signed-off-by: Justin Tobler <jltobler@gmail.com>
> ---
> builtin/add.c | 4 ++--
> builtin/receive-pack.c | 1 +
> builtin/unpack-objects.c | 2 +-
> builtin/update-index.c | 4 ++--
> cache-tree.c | 2 +-
> object-file.c | 2 +-
> odb/transaction.c | 14 ++++++++++++++
> odb/transaction.h | 23 +++++++++++++++++++++++
> read-cache.c | 2 +-
> 9 files changed, 46 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 60ffbede2b..ad418a5952 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -393,7 +393,7 @@ int cmd_add(int argc,
> char *seen = NULL;
> char *ps_matched = NULL;
> struct lock_file lock_file = LOCK_INIT;
> - struct odb_transaction *transaction;
> + struct odb_transaction *transaction = NULL;
>
> repo_config(repo, add_config, NULL);
>
> @@ -600,7 +600,7 @@ int cmd_add(int argc,
>
> if (chmod_arg && pathspec.nr)
> exit_status |= chmod_pathspec(repo, &pathspec, chmod_arg[0], show_only);
> - odb_transaction_commit(transaction);
> + odb_transaction_commit_and_finalize_or_die(transaction);
>
> finish:
> if (write_locked_index(repo->index, &lock_file,
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index d74b787148..ed1edcbe93 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -2720,6 +2720,7 @@ int cmd_receive_pack(int argc,
> use_keepalive = KEEPALIVE_ALWAYS;
> execute_commands(commands, unpack_status, &si, transaction,
> &push_options);
> + odb_transaction_finalize(transaction);
> delete_tempfile(&pack_lockfile);
> sigchain_push(SIGPIPE, SIG_IGN);
> if (report_status_v2)
> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index 4263edfbec..d6a2d616d9 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -603,7 +603,7 @@ static void unpack_all(void)
> unpack_one(i);
> display_progress(progress, i + 1);
> }
> - odb_transaction_commit(transaction);
> + odb_transaction_commit_and_finalize_or_die(transaction);
> stop_progress(&progress);
>
> if (delta_list)
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 241abd4332..b25d4ecb10 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -1156,7 +1156,7 @@ int cmd_update_index(int argc,
> * a transaction.
> */
> if (transaction && verbose) {
> - odb_transaction_commit(transaction);
> + odb_transaction_commit_and_finalize_or_die(transaction);
> transaction = NULL;
> }
>
> @@ -1224,7 +1224,7 @@ int cmd_update_index(int argc,
> /*
> * By now we have added all of the new objects
> */
> - odb_transaction_commit(transaction);
> + odb_transaction_commit_and_finalize_or_die(transaction);
>
> if (split_index > 0) {
> if (repo_config_get_split_index(the_repository) == 0)
> diff --git a/cache-tree.c b/cache-tree.c
> index d92f513286..a220372a42 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -538,7 +538,7 @@ int cache_tree_update(struct index_state *istate, int flags)
> i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
> "", 0, &skip, flags);
> if (!inflight)
> - odb_transaction_commit(transaction);
> + odb_transaction_commit_and_finalize_or_die(transaction);
> trace2_region_leave("cache_tree", "update", istate->repo);
> trace_performance_leave("cache_tree_update");
> if (i < 0)
> diff --git a/object-file.c b/object-file.c
> index ec35c318bc..4d03c167d5 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -965,7 +965,7 @@ int index_fd(struct index_state *istate, struct object_id *oid,
> xsize_t(st->st_size),
> oid);
> if (!inflight)
> - odb_transaction_commit(transaction);
> + odb_transaction_commit_and_finalize_or_die(transaction);
> } else {
> ret = hash_blob_stream(&stream,
> the_repository->hash_algo, oid,
> diff --git a/odb/transaction.c b/odb/transaction.c
> index dab7da6a9a..9e9a982778 100644
> --- a/odb/transaction.c
> +++ b/odb/transaction.c
> @@ -33,6 +33,20 @@ int odb_transaction_commit(struct odb_transaction *transaction)
>
> ret = transaction->commit(transaction);
> transaction->source->odb->transaction = NULL;
> +
> + return ret;
> +}
> +
> +int odb_transaction_finalize(struct odb_transaction *transaction)
> +{
> + int ret = 0;
> +
> + if (!transaction)
> + return 0;
> +
> + if (transaction->finalize)
> + ret = transaction->finalize(transaction);
> +
> free(transaction);
>
> return ret;
> diff --git a/odb/transaction.h b/odb/transaction.h
> index 4cb2eafcbf..6ed39b3d0e 100644
> --- a/odb/transaction.h
> +++ b/odb/transaction.h
> @@ -22,6 +22,13 @@ struct odb_transaction {
> */
> int (*commit)(struct odb_transaction *transaction);
>
> + /*
> + * Optional ODB source specific callback invoked when the transaction
> + * needs to perform any deferred cleanup after objects have been
> + * committed. Returns 0 on success, a negative error code otherwise.
> + */
> + int (*finalize)(struct odb_transaction *transaction);
> +
> /*
> * This callback is expected to write the given object stream into
> * the ODB transaction. Note that for now, only blobs support streaming.
> @@ -75,6 +82,22 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
> */
> int odb_transaction_commit(struct odb_transaction *transaction);
>
> +/*
> + * Finalizes an ODB transaction, performing any deferred cleanup and freeing it.
> + * Must be called for every successfully started transaction. Note that, if the
> + * specified transaction is NULL, the function is a no-op. Returns 0 on success,
> + * a negative error code otherwise.
> + */
> +int odb_transaction_finalize(struct odb_transaction *transaction);
> +
> +static inline void odb_transaction_commit_and_finalize_or_die(struct odb_transaction *transaction)
> +{
> + if (odb_transaction_commit(transaction))
> + die(_("failed to commit ODB transaction"));
> + if (odb_transaction_finalize(transaction))
> + die(_("failed to finalize ODB transaction"));
> +}
> +
> /*
> * Writes the object in the provided stream into the transaction. The resulting
> * object ID is written into the out pointer. Returns 0 on success, a negative
> diff --git a/read-cache.c b/read-cache.c
> index 6c449f393d..0cd0ef85ec 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -4049,7 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
> odb_transaction_begin_or_die(repo->objects, &transaction, 0);
> run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
> if (!inflight)
> - odb_transaction_commit(transaction);
> + odb_transaction_commit_and_finalize_or_die(transaction);
>
> release_revisions(&rev);
> return !!data.add_errors;
> --
> 2.55.0.424.g13c7afec21
>
^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v3 3/9] builtin/receive-pack: pass shallow file explicitly
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-11 17:54 ` [PATCH v3 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
2026-08-11 17:54 ` [PATCH v3 2/9] odb/transaction: add transaction finalize interface Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-11 17:54 ` [PATCH v3 4/9] builtin/receive-pack: read unpack limit config lazily Justin Tobler
` (6 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
If shallow information is provided during `unpack()`, a temporary
shallow file is created and stored in global state. In a subsequent
commit, the `unpack()` logic is moved behind a generic ODB transaction
interface to handle writing packfiles and thus can no longer rely on
such global state. Lift the setup of the temporary shallow file out of
`unpack()` and wire it through to its call sites explicitly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ed1edcbe93..135105deae 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -86,7 +86,6 @@ static const char *head_name;
static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
-static const char *alt_shallow_file;
static struct strbuf push_cert = STRBUF_INIT;
static struct object_id push_cert_oid;
static struct signature_check sigcheck;
@@ -2334,8 +2333,8 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
-static const char *unpack(int err_fd, struct shallow_info *si,
- struct odb_transaction *transaction)
+static const char *unpack(struct odb_transaction *transaction,
+ const char *shallow_file, int err_fd)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2354,10 +2353,9 @@ static const char *unpack(int err_fd, struct shallow_info *si,
return hdr_err;
}
- if (si->nr_ours || si->nr_theirs) {
- alt_shallow_file = setup_temporary_shallow(si->shallow);
+ if (shallow_file) {
strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, alt_shallow_file);
+ strvec_push(&child.args, shallow_file);
}
odb_transaction_env(transaction, &child.env);
@@ -2433,14 +2431,14 @@ static const char *unpack(int err_fd, struct shallow_info *si,
return NULL;
}
-static const char *unpack_with_sideband(struct shallow_info *si,
- struct odb_transaction *transaction)
+static const char *unpack_with_sideband(struct odb_transaction *transaction,
+ const char *shallow_file)
{
struct async muxer;
const char *ret;
if (!use_sideband)
- return unpack(0, si, transaction);
+ return unpack(transaction, shallow_file, 0);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2449,13 +2447,14 @@ static const char *unpack_with_sideband(struct shallow_info *si,
if (start_async(&muxer))
return NULL;
- ret = unpack(muxer.in, si, transaction);
+ ret = unpack(transaction, shallow_file, muxer.in);
finish_async(&muxer);
return ret;
}
-static void prepare_shallow_update(struct shallow_info *si)
+static void prepare_shallow_update(struct shallow_info *si,
+ const char *shallow_file)
{
int i, j, k, bitmap_size = DIV_ROUND_UP(si->ref->nr, 32);
@@ -2495,12 +2494,13 @@ static void prepare_shallow_update(struct shallow_info *si)
* command. check_connected() will be done with
* true .git/shallow though.
*/
- setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1);
+ setenv(GIT_SHALLOW_FILE_ENVIRONMENT, shallow_file, 1);
}
static void update_shallow_info(struct command *commands,
struct shallow_info *si,
- struct oid_array *ref)
+ struct oid_array *ref,
+ const char *shallow_file)
{
struct command *cmd;
int *ref_status;
@@ -2519,7 +2519,7 @@ static void update_shallow_info(struct command *commands,
si->ref = ref;
if (shallow_update) {
- prepare_shallow_update(si);
+ prepare_shallow_update(si, shallow_file);
return;
}
@@ -2711,11 +2711,17 @@ int cmd_receive_pack(int argc,
if (!si.nr_ours && !si.nr_theirs)
shallow_update = 0;
if (!delete_only(commands)) {
+ const char *alt_shallow_file = NULL;
+
+ if (si.nr_ours || si.nr_theirs)
+ alt_shallow_file = setup_temporary_shallow(si.shallow);
+
if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
unpack_status = "unable to start object transaction";
else
- unpack_status = unpack_with_sideband(&si, transaction);
- update_shallow_info(commands, &si, &ref);
+ unpack_status = unpack_with_sideband(transaction, alt_shallow_file);
+
+ update_shallow_info(commands, &si, &ref, alt_shallow_file);
}
use_keepalive = KEEPALIVE_ALWAYS;
execute_commands(commands, unpack_status, &si, transaction,
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v3 4/9] builtin/receive-pack: read unpack limit config lazily
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (2 preceding siblings ...)
2026-08-11 17:54 ` [PATCH v3 3/9] builtin/receive-pack: pass shallow file explicitly Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-11 17:54 ` [PATCH v3 5/9] builtin/receive-pack: lift global state out of unpack() Justin Tobler
` (5 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), the `receive.unpackLimit` and
`transfer.unpackLimit` configuration decides whether an incoming
packfile should be exploded into loose objects or kept as a packfile
on-disk. In a subsequent commit, the logic to write the incoming
packfile is made ODB backend agnostic and moved behind a pluggable ODB
transaction interface. Consequently, whether to explode a packfile is a
detail of how a particular backend stores objects and should not be a
part of the generic interface itself.
In preparation for this, instead resolve the unpack limit lazily inside
`unpack()` by reading the configuration directly. The now-unused unpack
limit globals are dropped accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 135105deae..971dc3f52e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -62,12 +62,9 @@ static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
static int receive_fsck_objects = -1;
static int transfer_fsck_objects = -1;
static struct strbuf fsck_msg_types = STRBUF_INIT;
-static int receive_unpack_limit = -1;
-static int transfer_unpack_limit = -1;
static int advertise_atomic_push = 1;
static int advertise_push_options;
static int advertise_sid;
-static int unpack_limit = 100;
static off_t max_input_size;
static int report_status;
static int report_status_v2;
@@ -157,16 +154,6 @@ static int receive_pack_config(const char *var, const char *value,
return 0;
}
- if (strcmp(var, "receive.unpacklimit") == 0) {
- receive_unpack_limit = git_config_int(var, value, ctx->kvi);
- return 0;
- }
-
- if (strcmp(var, "transfer.unpacklimit") == 0) {
- transfer_unpack_limit = git_config_int(var, value, ctx->kvi);
- return 0;
- }
-
if (strcmp(var, "receive.fsck.skiplist") == 0) {
char *path;
@@ -2333,6 +2320,16 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
+static unsigned int get_unpack_limit(struct repository *repo)
+{
+ unsigned int limit = 100;
+
+ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
+ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
+
+ return limit;
+}
+
static const char *unpack(struct odb_transaction *transaction,
const char *shallow_file, int err_fd)
{
@@ -2360,7 +2357,7 @@ static const char *unpack(struct odb_transaction *transaction,
odb_transaction_env(transaction, &child.env);
- if (ntohl(hdr.hdr_entries) < unpack_limit) {
+ if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
strvec_push(&child.args, "unpack-objects");
push_header_arg(&child.args, &hdr);
if (quiet)
@@ -2658,11 +2655,6 @@ int cmd_receive_pack(int argc,
if (cert_nonce_seed)
push_cert_nonce = prepare_push_cert_nonce(service_dir, time(NULL));
- if (0 <= receive_unpack_limit)
- unpack_limit = receive_unpack_limit;
- else if (0 <= transfer_unpack_limit)
- unpack_limit = transfer_unpack_limit;
-
switch (determine_protocol_version_server()) {
case protocol_v2:
/*
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v3 5/9] builtin/receive-pack: lift global state out of unpack()
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (3 preceding siblings ...)
2026-08-11 17:54 ` [PATCH v3 4/9] builtin/receive-pack: read unpack limit config lazily Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-11 17:54 ` [PATCH v3 6/9] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
` (4 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), writing the packfile to the transaction is
handled via `unpack()` which relies on global variables to decide how to
invoke the underlying git-index-pack(1) or git-unpack-objects(1) child
processes. In a subsequent commit, the `unpack()` logic is moved behind
a generic ODB transaction interface to handle writing packfiles and thus
can no longer rely on these globals.
Lift the global state out of `unpack()` by instead storing this state in
a `struct unpack_opts` that gets passed to the function explicitly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 63 +++++++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 22 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 971dc3f52e..f062b93b8d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2330,18 +2330,24 @@ static unsigned int get_unpack_limit(struct repository *repo)
return limit;
}
+struct unpack_opts {
+ const char *fsck_msg_types;
+ const char *shallow_file;
+ off_t max_input_size;
+ int fsck_objects;
+ int reject_thin;
+ int err_fd;
+ int quiet;
+};
+
static const char *unpack(struct odb_transaction *transaction,
- const char *shallow_file, int err_fd)
+ const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
int status;
struct child_process child = CHILD_PROCESS_INIT;
- int fsck_objects = (receive_fsck_objects >= 0
- ? receive_fsck_objects
- : transfer_fsck_objects >= 0
- ? transfer_fsck_objects
- : 0);
+ int err_fd = opts->err_fd;
hdr_err = parse_pack_header(&hdr);
if (hdr_err) {
@@ -2350,9 +2356,9 @@ static const char *unpack(struct odb_transaction *transaction,
return hdr_err;
}
- if (shallow_file) {
+ if (opts->shallow_file) {
strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, shallow_file);
+ strvec_push(&child.args, opts->shallow_file);
}
odb_transaction_env(transaction, &child.env);
@@ -2360,14 +2366,14 @@ static const char *unpack(struct odb_transaction *transaction,
if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
strvec_push(&child.args, "unpack-objects");
push_header_arg(&child.args, &hdr);
- if (quiet)
+ if (opts->quiet)
strvec_push(&child.args, "-q");
- if (fsck_objects)
+ if (opts->fsck_objects)
strvec_pushf(&child.args, "--strict%s",
- fsck_msg_types.buf);
- if (max_input_size)
+ opts->fsck_msg_types);
+ if (opts->max_input_size)
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)max_input_size);
+ (uintmax_t)opts->max_input_size);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -2388,18 +2394,18 @@ static const char *unpack(struct odb_transaction *transaction,
(uintmax_t)getpid(),
hostname);
- if (!quiet && err_fd)
+ if (!opts->quiet && err_fd)
strvec_push(&child.args, "--show-resolving-progress");
- if (use_sideband)
+ if (err_fd)
strvec_push(&child.args, "--report-end-of-input");
- if (fsck_objects)
+ if (opts->fsck_objects)
strvec_pushf(&child.args, "--strict%s",
- fsck_msg_types.buf);
- if (!reject_thin)
+ opts->fsck_msg_types);
+ if (!opts->reject_thin)
strvec_push(&child.args, "--fix-thin");
- if (max_input_size)
+ if (opts->max_input_size)
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)max_input_size);
+ (uintmax_t)opts->max_input_size);
child.out = -1;
child.err = err_fd;
child.git_cmd = 1;
@@ -2431,11 +2437,23 @@ static const char *unpack(struct odb_transaction *transaction,
static const char *unpack_with_sideband(struct odb_transaction *transaction,
const char *shallow_file)
{
+ struct unpack_opts opts = {
+ .fsck_objects = (receive_fsck_objects >= 0
+ ? receive_fsck_objects
+ : transfer_fsck_objects >= 0
+ ? transfer_fsck_objects
+ : 0),
+ .fsck_msg_types = fsck_msg_types.buf,
+ .max_input_size = max_input_size,
+ .shallow_file = shallow_file,
+ .reject_thin = reject_thin,
+ .quiet = quiet,
+ };
struct async muxer;
const char *ret;
if (!use_sideband)
- return unpack(transaction, shallow_file, 0);
+ return unpack(transaction, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2444,7 +2462,8 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction,
if (start_async(&muxer))
return NULL;
- ret = unpack(transaction, shallow_file, muxer.in);
+ opts.err_fd = muxer.in;
+ ret = unpack(transaction, &opts);
finish_async(&muxer);
return ret;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v3 6/9] builtin/receive-pack: report unpack errors via strbuf
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (4 preceding siblings ...)
2026-08-11 17:54 ` [PATCH v3 5/9] builtin/receive-pack: lift global state out of unpack() Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-11 17:54 ` [PATCH v3 7/9] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
` (3 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When writing packfiles via `unpack()`, error messages are returned
directly by the function. In preparation for `unpack()` logic being
moved behind a generic ODB transaction interface, update the function to
instead write any error messages to a caller provided strbuf and return
a negative value on error. Call sites are updated to use the error
strbuf accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 63 ++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f062b93b8d..6df872697b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2015,7 +2015,7 @@ static void execute_commands_atomic(struct command *commands,
}
static void execute_commands(struct command *commands,
- const char *unpacker_error,
+ int unpacker_error,
struct shallow_info *si,
struct odb_transaction *transaction,
const struct string_list *push_options)
@@ -2340,8 +2340,8 @@ struct unpack_opts {
int quiet;
};
-static const char *unpack(struct odb_transaction *transaction,
- const struct unpack_opts *opts)
+static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
+ const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2353,7 +2353,8 @@ static const char *unpack(struct odb_transaction *transaction,
if (hdr_err) {
if (err_fd > 0)
close(err_fd);
- return hdr_err;
+ strbuf_addstr(err_msg, hdr_err);
+ return -1;
}
if (opts->shallow_file) {
@@ -2378,8 +2379,10 @@ static const char *unpack(struct odb_transaction *transaction,
child.err = err_fd;
child.git_cmd = 1;
status = run_command(&child);
- if (status)
- return "unpack-objects abnormal exit";
+ if (status) {
+ strbuf_addstr(err_msg, "unpack-objects abnormal exit");
+ return -1;
+ }
} else {
char hostname[HOST_NAME_MAX + 1];
char *lockfile;
@@ -2410,8 +2413,10 @@ static const char *unpack(struct odb_transaction *transaction,
child.err = err_fd;
child.git_cmd = 1;
status = start_command(&child);
- if (status)
- return "index-pack fork failed";
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack fork failed");
+ return -1;
+ }
/*
* The lockfile filepath is expected to be the final location of
@@ -2427,15 +2432,18 @@ static const char *unpack(struct odb_transaction *transaction,
close(child.out);
status = finish_command(&child);
- if (status)
- return "index-pack abnormal exit";
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack abnormal exit");
+ return -1;
+ }
odb_reprepare(the_repository->objects);
}
- return NULL;
+ return 0;
}
-static const char *unpack_with_sideband(struct odb_transaction *transaction,
- const char *shallow_file)
+static int unpack_with_sideband(struct odb_transaction *transaction,
+ const char *shallow_file,
+ struct strbuf *err_msg)
{
struct unpack_opts opts = {
.fsck_objects = (receive_fsck_objects >= 0
@@ -2450,20 +2458,20 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction,
.quiet = quiet,
};
struct async muxer;
- const char *ret;
+ int ret;
if (!use_sideband)
- return unpack(transaction, &opts);
+ return unpack(transaction, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
muxer.proc = copy_to_sideband;
muxer.in = -1;
if (start_async(&muxer))
- return NULL;
+ return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, &opts);
+ ret = unpack(transaction, err_msg, &opts);
finish_async(&muxer);
return ret;
@@ -2552,13 +2560,13 @@ static void update_shallow_info(struct command *commands,
free(ref_status);
}
-static void report(struct command *commands, const char *unpack_status)
+static void report(struct command *commands, const struct strbuf *unpack_status)
{
struct command *cmd;
struct strbuf buf = STRBUF_INIT;
packet_buf_write(&buf, "unpack %s\n",
- unpack_status ? unpack_status : "ok");
+ unpack_status->len ? unpack_status->buf : "ok");
for (cmd = commands; cmd; cmd = cmd->next) {
if (!cmd->error_string)
packet_buf_write(&buf, "ok %s\n",
@@ -2576,14 +2584,14 @@ static void report(struct command *commands, const char *unpack_status)
strbuf_release(&buf);
}
-static void report_v2(struct command *commands, const char *unpack_status)
+static void report_v2(struct command *commands, const struct strbuf *unpack_status)
{
struct command *cmd;
struct strbuf buf = STRBUF_INIT;
struct ref_push_report *report;
packet_buf_write(&buf, "unpack %s\n",
- unpack_status ? unpack_status : "ok");
+ unpack_status->len ? unpack_status->buf : "ok");
for (cmd = commands; cmd; cmd = cmd->next) {
int count = 0;
@@ -2707,8 +2715,8 @@ int cmd_receive_pack(int argc,
PACKET_READ_DIE_ON_ERR_PACKET);
if ((commands = read_head_info(&reader, &shallow))) {
- const char *unpack_status = NULL;
struct string_list push_options = STRING_LIST_INIT_DUP;
+ struct strbuf unpack_status = STRBUF_INIT;
if (use_push_options)
read_push_options(&reader, &push_options);
@@ -2728,22 +2736,22 @@ int cmd_receive_pack(int argc,
alt_shallow_file = setup_temporary_shallow(si.shallow);
if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
- unpack_status = "unable to start object transaction";
+ strbuf_addstr(&unpack_status, "unable to start object transaction");
else
- unpack_status = unpack_with_sideband(transaction, alt_shallow_file);
+ unpack_with_sideband(transaction, alt_shallow_file, &unpack_status);
update_shallow_info(commands, &si, &ref, alt_shallow_file);
}
use_keepalive = KEEPALIVE_ALWAYS;
- execute_commands(commands, unpack_status, &si, transaction,
+ execute_commands(commands, !!unpack_status.len, &si, transaction,
&push_options);
odb_transaction_finalize(transaction);
delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
- report_v2(commands, unpack_status);
+ report_v2(commands, &unpack_status);
else if (report_status)
- report(commands, unpack_status);
+ report(commands, &unpack_status);
sigchain_pop(SIGPIPE);
run_receive_hook(commands, "post-receive", 1, NULL,
&push_options);
@@ -2768,6 +2776,7 @@ int cmd_receive_pack(int argc,
if (auto_update_server_info)
update_server_info(the_repository, 0);
clear_shallow_info(&si);
+ strbuf_release(&unpack_status);
}
if (use_sideband)
packet_flush(1);
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v3 7/9] builtin/receive-pack: explicitly pass packfile fd
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (5 preceding siblings ...)
2026-08-11 17:54 ` [PATCH v3 6/9] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-11 17:54 ` [PATCH v3 8/9] odb: return temporary ODB source when set Justin Tobler
` (2 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When processing the incoming packfile in git-receive-pack(1), `unpack()`
assumes it should always read it from stdin. In preparation for
`unpack()` logic being moved behind a generic ODB transaction interface,
update the function signature to take the an explicit fd provided by
callers to read the incoming packfile from instead. Call sites are
updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6df872697b..b369466783 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2292,9 +2292,9 @@ static void read_push_options(struct packet_reader *reader,
}
}
-static const char *parse_pack_header(struct pack_header *hdr)
+static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
{
- switch (read_pack_header(0, hdr)) {
+ switch (read_pack_header(pack_fd, hdr)) {
case PH_ERROR_EOF:
return "eof before pack header was fully read";
@@ -2340,8 +2340,8 @@ struct unpack_opts {
int quiet;
};
-static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
- const struct unpack_opts *opts)
+static int unpack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg, const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2349,7 +2349,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
struct child_process child = CHILD_PROCESS_INIT;
int err_fd = opts->err_fd;
- hdr_err = parse_pack_header(&hdr);
+ hdr_err = parse_pack_header(&hdr, pack_fd);
if (hdr_err) {
if (err_fd > 0)
close(err_fd);
@@ -2376,6 +2376,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
(uintmax_t)opts->max_input_size);
child.no_stdout = 1;
+ child.in = pack_fd;
child.err = err_fd;
child.git_cmd = 1;
status = run_command(&child);
@@ -2410,6 +2411,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
(uintmax_t)opts->max_input_size);
child.out = -1;
+ child.in = pack_fd;
child.err = err_fd;
child.git_cmd = 1;
status = start_command(&child);
@@ -2461,7 +2463,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
int ret;
if (!use_sideband)
- return unpack(transaction, err_msg, &opts);
+ return unpack(transaction, 0, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2471,7 +2473,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, err_msg, &opts);
+ ret = unpack(transaction, 0, err_msg, &opts);
finish_async(&muxer);
return ret;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v3 8/9] odb: return temporary ODB source when set
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (6 preceding siblings ...)
2026-08-11 17:54 ` [PATCH v3 7/9] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-12 6:07 ` Patrick Steinhardt
2026-08-11 17:54 ` [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
9 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When invoked, `odb_set_temporary_primary_source()` installs a temporary
object directory as the new primary ODB source. A caller that wants to
operate on the ODB source of the open transaction must assume that it is
the first entry in the ODB source list which is a bit awkward and
fragile.
Instead, return the newly installed source directly and report the
previous primary source via a new `prev_source` out parameter. Propagate
the installed source through `tmp_objdir_replace_primary_odb()` and
start storing it in the "files" ODB transaction so a subsequent commit
can easily access it without relying on the ODB source list ordering.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 3 ++-
odb.c | 9 +++++++--
odb.h | 6 ++++--
tmp-objdir.c | 8 +++++---
tmp-objdir.h | 6 ++++--
5 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/object-file.c b/object-file.c
index 4d03c167d5..db63587f6d 100644
--- a/object-file.c
+++ b/object-file.c
@@ -485,6 +485,7 @@ struct odb_transaction_files {
struct odb_transaction base;
struct tmp_objdir *objdir;
+ struct odb_source *quarantine;
struct transaction_packfile packfile;
const char *prefix;
};
@@ -507,7 +508,7 @@ int odb_transaction_files_prepare(struct odb_transaction *base)
if (!transaction->objdir)
return error(_("unable to create temporary object directory"));
- tmp_objdir_replace_primary_odb(transaction->objdir, 0);
+ transaction->quarantine = tmp_objdir_replace_primary_odb(transaction->objdir, 0);
return 0;
}
diff --git a/odb.c b/odb.c
index caf1d0f542..8afcb6b637 100644
--- a/odb.c
+++ b/odb.c
@@ -226,7 +226,8 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
}
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
- const char *dir, int will_destroy)
+ const char *dir, int will_destroy,
+ struct odb_source **prev_source)
{
struct odb_source *source;
@@ -250,7 +251,11 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
source->will_destroy = will_destroy;
source->next = odb->sources;
odb->sources = source;
- return source->next;
+
+ if (prev_source)
+ *prev_source = source->next;
+
+ return source;
}
void odb_restore_primary_source(struct object_database *odb,
diff --git a/odb.h b/odb.h
index fca67e8253..bdfcb9509a 100644
--- a/odb.h
+++ b/odb.h
@@ -199,10 +199,12 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha
/*
* Replace the current writable object directory with the specified temporary
- * object directory; returns the former primary source.
+ * object directory and return the newly installed primary source. The former
+ * primary source is reported via `prev_source` when non-NULL.
*/
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
- const char *dir, int will_destroy);
+ const char *dir, int will_destroy,
+ struct odb_source **prev_source);
/*
* Restore the primary source that was previously replaced by
diff --git a/tmp-objdir.c b/tmp-objdir.c
index d199d39e7c..e633d97e0e 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -327,11 +327,13 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)
odb_add_to_alternates_memory(t->repo->objects, t->path.buf);
}
-void tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy)
+struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t,
+ int will_destroy)
{
if (t->prev_source)
BUG("the primary object database is already replaced");
- t->prev_source = odb_set_temporary_primary_source(t->repo->objects,
- t->path.buf, will_destroy);
t->will_destroy = will_destroy;
+
+ return odb_set_temporary_primary_source(t->repo->objects, t->path.buf,
+ will_destroy, &t->prev_source);
}
diff --git a/tmp-objdir.h b/tmp-objdir.h
index ccf800faa7..81eb927413 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -64,8 +64,10 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
/*
* Replaces the writable object store in the current process with the temporary
* object directory and makes the former main object store an alternate.
- * If will_destroy is nonzero, the object directory may not be migrated.
+ * If will_destroy is nonzero, the object directory may not be migrated. Returns
+ * the newly installed primary source.
*/
-void tmp_objdir_replace_primary_odb(struct tmp_objdir *, int will_destroy);
+struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *,
+ int will_destroy);
#endif /* TMP_OBJDIR_H */
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v3 8/9] odb: return temporary ODB source when set
2026-08-11 17:54 ` [PATCH v3 8/9] odb: return temporary ODB source when set Justin Tobler
@ 2026-08-12 6:07 ` Patrick Steinhardt
0 siblings, 0 replies; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-12 6:07 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Tue, Aug 11, 2026 at 12:54:14PM -0500, Justin Tobler wrote:
> When invoked, `odb_set_temporary_primary_source()` installs a temporary
> object directory as the new primary ODB source. A caller that wants to
> operate on the ODB source of the open transaction must assume that it is
> the first entry in the ODB source list which is a bit awkward and
> fragile.
>
> Instead, return the newly installed source directly and report the
> previous primary source via a new `prev_source` out parameter. Propagate
> the installed source through `tmp_objdir_replace_primary_odb()` and
> start storing it in the "files" ODB transaction so a subsequent commit
> can easily access it without relying on the ODB source list ordering.
Makes sense. I'm looking forward to the day where we get rid of this
mechanism altogether.
Patrick
^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (7 preceding siblings ...)
2026-08-11 17:54 ` [PATCH v3 8/9] odb: return temporary ODB source when set Justin Tobler
@ 2026-08-11 17:54 ` Justin Tobler
2026-08-14 8:51 ` Patrick Steinhardt
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
9 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-11 17:54 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), the incoming packfile is written to the ODB via
`unpack()`, which spawns git-index-pack(1) or git-unpack-objects(1)
directly. With pluggable object databases, an alternative backend may
need to handle writing packfile data differently though.
Introduce `odb_transaction_write_pack()` as a generic interface to
handle writing a packfile to a transaction and use the logic from
`unpack()` as the "files" backend implementation. Note that when storing
the objects as a packfile, git-index-pack(1) also writes a ".keep"
lockfile next to it to prevent a concurrent repack from removing the new
pack prior to reference updates being performed. The "files" transaction
backend is responsible for managing these ".keep" files and removes them
post-commit once the transaction is finalized.
Call sites in git-receive-pack(1) are updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 160 +-------------------------------------
object-file.c | 172 +++++++++++++++++++++++++++++++++++++++++
odb/transaction.c | 7 ++
odb/transaction.h | 62 +++++++++++++++
4 files changed, 244 insertions(+), 157 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b369466783..e6e54ba55f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -15,7 +15,6 @@
#include "gpg-interface.h"
#include "hex.h"
#include "hook.h"
-#include "lockfile.h"
#include "object.h"
#include "object-file.h"
#include "object-name.h"
@@ -23,7 +22,6 @@
#include "oid-array.h"
#include "oidset.h"
#include "pack.h"
-#include "packfile.h"
#include "parse-options.h"
#include "pkt-line.h"
#include "protocol.h"
@@ -2292,162 +2290,11 @@ static void read_push_options(struct packet_reader *reader,
}
}
-static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
-{
- switch (read_pack_header(pack_fd, hdr)) {
- case PH_ERROR_EOF:
- return "eof before pack header was fully read";
-
- case PH_ERROR_PACK_SIGNATURE:
- return "protocol error (pack signature mismatch detected)";
-
- case PH_ERROR_PROTOCOL:
- return "protocol error (pack version unsupported)";
-
- default:
- return "unknown error in parse_pack_header";
-
- case 0:
- return NULL;
- }
-}
-
-static struct tempfile *pack_lockfile;
-
-static void push_header_arg(struct strvec *args, struct pack_header *hdr)
-{
- strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
- ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
-}
-
-static unsigned int get_unpack_limit(struct repository *repo)
-{
- unsigned int limit = 100;
-
- repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
- repo_config_get_uint(repo, "receive.unpacklimit", &limit);
-
- return limit;
-}
-
-struct unpack_opts {
- const char *fsck_msg_types;
- const char *shallow_file;
- off_t max_input_size;
- int fsck_objects;
- int reject_thin;
- int err_fd;
- int quiet;
-};
-
-static int unpack(struct odb_transaction *transaction, int pack_fd,
- struct strbuf *err_msg, const struct unpack_opts *opts)
-{
- struct pack_header hdr;
- const char *hdr_err;
- int status;
- struct child_process child = CHILD_PROCESS_INIT;
- int err_fd = opts->err_fd;
-
- hdr_err = parse_pack_header(&hdr, pack_fd);
- if (hdr_err) {
- if (err_fd > 0)
- close(err_fd);
- strbuf_addstr(err_msg, hdr_err);
- return -1;
- }
-
- if (opts->shallow_file) {
- strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, opts->shallow_file);
- }
-
- odb_transaction_env(transaction, &child.env);
-
- if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
- strvec_push(&child.args, "unpack-objects");
- push_header_arg(&child.args, &hdr);
- if (opts->quiet)
- strvec_push(&child.args, "-q");
- if (opts->fsck_objects)
- strvec_pushf(&child.args, "--strict%s",
- opts->fsck_msg_types);
- if (opts->max_input_size)
- strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)opts->max_input_size);
- child.no_stdout = 1;
- child.in = pack_fd;
- child.err = err_fd;
- child.git_cmd = 1;
- status = run_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "unpack-objects abnormal exit");
- return -1;
- }
- } else {
- char hostname[HOST_NAME_MAX + 1];
- char *lockfile;
-
- strvec_pushl(&child.args, "index-pack", "--stdin", NULL);
- push_header_arg(&child.args, &hdr);
-
- if (xgethostname(hostname, sizeof(hostname)))
- xsnprintf(hostname, sizeof(hostname), "localhost");
- strvec_pushf(&child.args,
- "--keep=receive-pack %"PRIuMAX" on %s",
- (uintmax_t)getpid(),
- hostname);
-
- if (!opts->quiet && err_fd)
- strvec_push(&child.args, "--show-resolving-progress");
- if (err_fd)
- strvec_push(&child.args, "--report-end-of-input");
- if (opts->fsck_objects)
- strvec_pushf(&child.args, "--strict%s",
- opts->fsck_msg_types);
- if (!opts->reject_thin)
- strvec_push(&child.args, "--fix-thin");
- if (opts->max_input_size)
- strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)opts->max_input_size);
- child.out = -1;
- child.in = pack_fd;
- child.err = err_fd;
- child.git_cmd = 1;
- status = start_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "index-pack fork failed");
- return -1;
- }
-
- /*
- * The lockfile filepath is expected to be the final location of
- * the ".keep" file after being migrated to the main ODB source.
- * This ensures the lockfile can be found and removed later
- * after the ODB transaction has been committed.
- */
- lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
- if (lockfile) {
- pack_lockfile = register_tempfile(lockfile);
- free(lockfile);
- }
- close(child.out);
-
- status = finish_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "index-pack abnormal exit");
- return -1;
- }
- odb_reprepare(the_repository->objects);
- }
- return 0;
-}
-
static int unpack_with_sideband(struct odb_transaction *transaction,
const char *shallow_file,
struct strbuf *err_msg)
{
- struct unpack_opts opts = {
+ struct odb_transaction_write_pack_opts opts = {
.fsck_objects = (receive_fsck_objects >= 0
? receive_fsck_objects
: transfer_fsck_objects >= 0
@@ -2463,7 +2310,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
int ret;
if (!use_sideband)
- return unpack(transaction, 0, err_msg, &opts);
+ return odb_transaction_write_pack(transaction, 0, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2473,7 +2320,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, 0, err_msg, &opts);
+ ret = odb_transaction_write_pack(transaction, 0, err_msg, &opts);
finish_async(&muxer);
return ret;
@@ -2748,7 +2595,6 @@ int cmd_receive_pack(int argc,
execute_commands(commands, !!unpack_status.len, &si, transaction,
&push_options);
odb_transaction_finalize(transaction);
- delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
report_v2(commands, &unpack_status);
diff --git a/object-file.c b/object-file.c
index db63587f6d..a957bc126f 100644
--- a/object-file.c
+++ b/object-file.c
@@ -10,6 +10,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "git-compat-util.h"
+#include "config.h"
#include "convert.h"
#include "dir.h"
#include "environment.h"
@@ -26,6 +27,7 @@
#include "packfile.h"
#include "path.h"
#include "read-cache-ll.h"
+#include "run-command.h"
#include "setup.h"
#include "strvec.h"
#include "tempfile.h"
@@ -488,6 +490,10 @@ struct odb_transaction_files {
struct odb_source *quarantine;
struct transaction_packfile packfile;
const char *prefix;
+
+ struct tempfile **pack_lockfiles;
+ size_t pack_lockfiles_nr;
+ size_t pack_lockfiles_alloc;
};
int odb_transaction_files_prepare(struct odb_transaction *base)
@@ -1291,6 +1297,170 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
return 0;
}
+static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
+{
+ switch (read_pack_header(pack_fd, hdr)) {
+ case PH_ERROR_EOF:
+ return "eof before pack header was fully read";
+
+ case PH_ERROR_PACK_SIGNATURE:
+ return "protocol error (pack signature mismatch detected)";
+
+ case PH_ERROR_PROTOCOL:
+ return "protocol error (pack version unsupported)";
+
+ default:
+ return "unknown error in parse_pack_header";
+
+ case 0:
+ return NULL;
+ }
+}
+
+static void push_header_arg(struct strvec *args, struct pack_header *hdr)
+{
+ strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
+ ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
+}
+
+static unsigned int get_unpack_limit(struct repository *repo)
+{
+ unsigned int limit = 100;
+
+ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
+ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
+
+ return limit;
+}
+
+static int odb_transaction_files_write_pack(struct odb_transaction *base,
+ int pack_fd, struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+ struct repository *repo = base->source->odb->repo;
+ struct child_process child = CHILD_PROCESS_INIT;
+ struct pack_header hdr;
+ const char *hdr_err;
+ int err_fd = opts->err_fd;
+ int status;
+
+ hdr_err = parse_pack_header(&hdr, pack_fd);
+ if (hdr_err) {
+ if (err_fd > 0)
+ close(err_fd);
+ strbuf_addstr(err_msg, hdr_err);
+ return -1;
+ }
+
+ if (opts->shallow_file) {
+ strvec_push(&child.args, "--shallow-file");
+ strvec_push(&child.args, opts->shallow_file);
+ }
+
+ odb_transaction_env(base, &child.env);
+
+ if (ntohl(hdr.hdr_entries) < get_unpack_limit(repo)) {
+ strvec_push(&child.args, "unpack-objects");
+ push_header_arg(&child.args, &hdr);
+ if (opts->quiet)
+ strvec_push(&child.args, "-q");
+ if (opts->fsck_objects)
+ strvec_pushf(&child.args, "--strict%s",
+ opts->fsck_msg_types);
+ if (opts->max_input_size)
+ strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
+ (uintmax_t)opts->max_input_size);
+ child.no_stdout = 1;
+ child.in = pack_fd;
+ child.err = err_fd;
+ child.git_cmd = 1;
+ status = run_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "unpack-objects abnormal exit");
+ return -1;
+ }
+ } else {
+ char hostname[HOST_NAME_MAX + 1];
+ char *lockfile;
+
+ strvec_pushl(&child.args, "index-pack", "--stdin", NULL);
+ push_header_arg(&child.args, &hdr);
+
+ if (xgethostname(hostname, sizeof(hostname)))
+ xsnprintf(hostname, sizeof(hostname), "localhost");
+ strvec_pushf(&child.args,
+ "--keep=receive-pack %"PRIuMAX" on %s",
+ (uintmax_t)getpid(),
+ hostname);
+
+ if (!opts->quiet && err_fd)
+ strvec_push(&child.args, "--show-resolving-progress");
+ if (err_fd)
+ strvec_push(&child.args, "--report-end-of-input");
+ if (opts->fsck_objects)
+ strvec_pushf(&child.args, "--strict%s",
+ opts->fsck_msg_types);
+ if (!opts->reject_thin)
+ strvec_push(&child.args, "--fix-thin");
+ if (opts->max_input_size)
+ strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
+ (uintmax_t)opts->max_input_size);
+ child.out = -1;
+ child.in = pack_fd;
+ child.err = err_fd;
+ child.git_cmd = 1;
+ status = start_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack fork failed");
+ return -1;
+ }
+
+ /*
+ * The lockfile filepath is expected to be the final location of
+ * the ".keep" file after being migrated to the main ODB source.
+ * This ensures the lockfile can be found and removed later
+ * after the ODB transaction has been committed.
+ */
+ lockfile = index_pack_lockfile(base->source, child.out, NULL);
+ if (lockfile) {
+ ALLOC_GROW(transaction->pack_lockfiles,
+ transaction->pack_lockfiles_nr + 1,
+ transaction->pack_lockfiles_alloc);
+ transaction->pack_lockfiles[transaction->pack_lockfiles_nr++] =
+ register_tempfile(lockfile);
+ free(lockfile);
+ }
+ close(child.out);
+
+ status = finish_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack abnormal exit");
+ return -1;
+ }
+
+ odb_source_prepare(transaction->quarantine,
+ ODB_PREPARE_FLUSH_CACHES);
+ }
+
+ return 0;
+}
+
+static int odb_transaction_files_finalize(struct odb_transaction *base)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+ int ret = 0;
+
+ for (size_t i = 0; i < transaction->pack_lockfiles_nr; i++)
+ ret |= delete_tempfile(&transaction->pack_lockfiles[i]);
+
+ free(transaction->pack_lockfiles);
+
+ return ret;
+}
+
static int odb_transaction_files_env(struct odb_transaction *base,
struct strvec *env)
{
@@ -1314,7 +1484,9 @@ int odb_transaction_files_begin(struct odb_source *source,
transaction = xcalloc(1, sizeof(*transaction));
transaction->base.source = source;
transaction->base.commit = odb_transaction_files_commit;
+ transaction->base.finalize = odb_transaction_files_finalize;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+ transaction->base.write_pack = odb_transaction_files_write_pack;
transaction->base.env = odb_transaction_files_env;
transaction->prefix = "bulk-fsync";
diff --git a/odb/transaction.c b/odb/transaction.c
index 9e9a982778..c9144e6cd6 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -59,6 +59,13 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
return transaction->write_object_stream(transaction, stream, len, oid);
}
+int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts)
+{
+ return transaction->write_pack(transaction, pack_fd, err_msg, opts);
+}
+
int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env)
{
if (!transaction)
diff --git a/odb/transaction.h b/odb/transaction.h
index 6ed39b3d0e..8cb06c1191 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -4,6 +4,50 @@
#include "gettext.h"
#include "odb.h"
+/*
+ * Options controlling how odb_transaction_write_pack() ingests a packfile.
+ */
+struct odb_transaction_write_pack_opts {
+ /*
+ * Optional fsck severity configuration to apply when incoming objects
+ * are verified.
+ */
+ const char *fsck_msg_types;
+
+ /*
+ * Path to an alternative shallow file describing the shallow boundaries
+ * to honor while ingesting the pack.
+ */
+ const char *shallow_file;
+
+ /*
+ * The max size in bytes of the incoming packfile allowed. No limit is
+ * enforced when set to 0.
+ */
+ off_t max_input_size;
+
+ /*
+ * Whether the validity of incoming objects should be verified.
+ */
+ int fsck_objects;
+
+ /*
+ * Whether to reject an incoming packfile if it is "thin".
+ */
+ int reject_thin;
+
+ /*
+ * Optional file descriptor for reporting progress and errors. Set to 0
+ * for none.
+ */
+ int err_fd;
+
+ /*
+ * Suppresses progress reporting.
+ */
+ int quiet;
+};
+
/*
* A transaction may be started for an object database prior to writing new
* objects via odb_transaction_begin(). These objects are not committed until
@@ -40,6 +84,15 @@ struct odb_transaction {
int (*write_object_stream)(struct odb_transaction *transaction,
struct odb_write_stream *stream, size_t len,
struct object_id *oid);
+ /*
+ * This callback is expected to ingest the packfile readable via
+ * `pack_fd` into the transaction. Returns 0 on success, a negative
+ * error code otherwise. On failure, a human-readable description is
+ * appended to `err_msg`.
+ */
+ int (*write_pack)(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts);
/*
* This callback is expected to populate the provided strvec with the
@@ -107,6 +160,15 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
struct odb_write_stream *stream,
size_t len, struct object_id *oid);
+/*
+ * Ingests the packfile readable via `pack_fd` into the transaction. Returns 0
+ * on success, a negative error code otherwise. On failure, a human-readable
+ * description is appended to `err_msg`.
+ */
+int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts);
+
/*
* Populates the provided strvec with the environment variables that a child
* process should inherit so that its object writes participate in the
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles
2026-08-11 17:54 ` [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
@ 2026-08-14 8:51 ` Patrick Steinhardt
2026-08-14 13:40 ` Justin Tobler
0 siblings, 1 reply; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-14 8:51 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Tue, Aug 11, 2026 at 12:54:15PM -0500, Justin Tobler wrote:
report_v2(commands, &unpack_status);
> diff --git a/object-file.c b/object-file.c
> index db63587f6d..a957bc126f 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1291,6 +1297,170 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
> return 0;
> }
>
> +static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
> +{
> + switch (read_pack_header(pack_fd, hdr)) {
> + case PH_ERROR_EOF:
> + return "eof before pack header was fully read";
> +
> + case PH_ERROR_PACK_SIGNATURE:
> + return "protocol error (pack signature mismatch detected)";
> +
> + case PH_ERROR_PROTOCOL:
> + return "protocol error (pack version unsupported)";
> +
> + default:
> + return "unknown error in parse_pack_header";
> +
> + case 0:
> + return NULL;
> + }
> +}
> +
> +static void push_header_arg(struct strvec *args, struct pack_header *hdr)
> +{
> + strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
> + ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
> +}
> +
> +static unsigned int get_unpack_limit(struct repository *repo)
> +{
> + unsigned int limit = 100;
> +
> + repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
> + repo_config_get_uint(repo, "receive.unpacklimit", &limit);
> +
> + return limit;
> +}
One thing I noticed just now: as the intention is that `write_pack()`
will be called for more use cases than only git-receive-pack(1) we'll
have to add a way to tell the callback what scenario they are running
in. I still think moving the unpack limit into the backend is sensible,
but now we're not givint it enough information.
Patrick
^ permalink raw reply [flat|nested] 78+ messages in thread* Re: [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles
2026-08-14 8:51 ` Patrick Steinhardt
@ 2026-08-14 13:40 ` Justin Tobler
2026-08-17 5:17 ` Patrick Steinhardt
0 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-14 13:40 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, gitster
On 26/08/14 10:51AM, Patrick Steinhardt wrote:
> On Tue, Aug 11, 2026 at 12:54:15PM -0500, Justin Tobler wrote:
> > +static unsigned int get_unpack_limit(struct repository *repo)
> > +{
> > + unsigned int limit = 100;
> > +
> > + repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
> > + repo_config_get_uint(repo, "receive.unpacklimit", &limit);
> > +
> > + return limit;
> > +}
>
> One thing I noticed just now: as the intention is that `write_pack()`
> will be called for more use cases than only git-receive-pack(1) we'll
> have to add a way to tell the callback what scenario they are running
> in. I still think moving the unpack limit into the backend is sensible,
> but now we're not givint it enough information.
So we already have transaction flags like ODB_TRANSACTION_RECEIVE that
can be used to differentiate certain callers that may require slightly
different behavior in the backend.
In a followup series where I expand usage of odb_transaction_write_pack
to git-fetch-pack(1), I was originally planning on using this flag and
also adding ODB_TRANSACTION_FETCH accordingly. It's probably a good idea
to go ahead though and start using the transaction flags here in this
series too. Will update in the next version.
-Justin
^ permalink raw reply [flat|nested] 78+ messages in thread* Re: [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles
2026-08-14 13:40 ` Justin Tobler
@ 2026-08-17 5:17 ` Patrick Steinhardt
0 siblings, 0 replies; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-17 5:17 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Fri, Aug 14, 2026 at 08:40:32AM -0500, Justin Tobler wrote:
> On 26/08/14 10:51AM, Patrick Steinhardt wrote:
> > On Tue, Aug 11, 2026 at 12:54:15PM -0500, Justin Tobler wrote:
> > > +static unsigned int get_unpack_limit(struct repository *repo)
> > > +{
> > > + unsigned int limit = 100;
> > > +
> > > + repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
> > > + repo_config_get_uint(repo, "receive.unpacklimit", &limit);
> > > +
> > > + return limit;
> > > +}
> >
> > One thing I noticed just now: as the intention is that `write_pack()`
> > will be called for more use cases than only git-receive-pack(1) we'll
> > have to add a way to tell the callback what scenario they are running
> > in. I still think moving the unpack limit into the backend is sensible,
> > but now we're not givint it enough information.
>
> So we already have transaction flags like ODB_TRANSACTION_RECEIVE that
> can be used to differentiate certain callers that may require slightly
> different behavior in the backend.
>
> In a followup series where I expand usage of odb_transaction_write_pack
> to git-fetch-pack(1), I was originally planning on using this flag and
> also adding ODB_TRANSACTION_FETCH accordingly. It's probably a good idea
> to go ahead though and start using the transaction flags here in this
> series too. Will update in the next version.
Ah, that makes sense then. And I agree, introducing that flag now
already makes it a bit more obvious for how future series will look
like. Thanks!
Patrick
^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes
2026-08-11 17:54 ` [PATCH v3 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (8 preceding siblings ...)
2026-08-11 17:54 ` [PATCH v3 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-19 21:53 ` [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
` (9 more replies)
9 siblings, 10 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
Greetings,
With bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10), git-receive-pack(1) started using the ODB
transaction interfaces to stage incoming objects. While this brought the
command closer to being ODB backend agnostic, the underlying
git-index-pack(1) and git-unpack-objects(1) processes used to actually
write the objects to the transaction are still fundamentally tied to the
"files" backend.
This series aims to address this by introducing a generic
`odb_transaction_write_pack()` transaction interface to handle writing
the incoming packfile to the transaction. The existing logic in
git-receive-pack(1) that spawns the child processes to write the
packfile becomes the "files" backend implementation of this interface.
Chances since V3:
- In preparation for future `odb_transaction_write_pack()` users, the
unpack limit takes into consideration odb_transaction_flags to augment
configutation.
- Added additional test assertion in first patch to ensure keep file is
generated and placed in quarantine directory.
- Removed an include statement in favor of just forward declaring a
struct.
- Updated some commit messages.
Changes since V2:
- Added a patch to address a bug causing ".keep" files from not being
removed.
- Started handling errors at transaction commit and finalize call sites
instead of ignoring them. We also make sure
`odb_transaction_finalize()` runs after every successful commit
callsite to ensure proper cleanup.
- Updated the code handling lazy loading of unpack limit configuration
to not longer cache the value.
- Added a patch to begin explictly tracking the ODB source used by the
"files" transaction to avoid relying on the ordering of the ODB source
list.
- Updated some commit messages to improve clarity.
Changes since V1:
- Changed the "release" interface name to "finalize" and updated it to
return error codes.
- Marked some function parameters as const.
- Unpack limit configuration is now resolved in the ODB transaction
backend instead of wiring it through the interface.
- When writing a packfile to the transaction, now only the transaction
source is prepared.
- Updated some commit messages.
- Updated some code formatting.
Thanks for the review,
-Justin
Justin Tobler (9):
builtin/receive-pack: properly clean up keep files
odb/transaction: add transaction finalize interface
builtin/receive-pack: pass shallow file explicitly
builtin/receive-pack: read unpack limit config lazily
builtin/receive-pack: lift global state out of unpack()
builtin/receive-pack: report unpack errors via strbuf
builtin/receive-pack: explicitly pass packfile fd
odb: return temporary ODB source when set
odb/transaction: add transaction interface to write packfiles
builtin/add.c | 4 +-
builtin/receive-pack.c | 211 ++++++++-----------------------------
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 4 +-
cache-tree.c | 2 +-
fetch-pack.c | 2 +-
object-file.c | 183 +++++++++++++++++++++++++++++++-
odb.c | 9 +-
odb.h | 6 +-
odb/transaction.c | 21 ++++
odb/transaction.h | 85 +++++++++++++++
pack-write.c | 7 +-
pack.h | 4 +-
read-cache.c | 2 +-
t/t5547-push-quarantine.sh | 22 ++++
tmp-objdir.c | 8 +-
tmp-objdir.h | 6 +-
17 files changed, 390 insertions(+), 188 deletions(-)
Range-diff against v3:
1: 58569303f9 ! 1: 13a57feea7 builtin/receive-pack: properly clean up keep files
@@ Commit message
builtin/receive-pack: properly clean up keep files
When git-receive-pack(1) stores an incoming packfile with
- git-index-pack(1), a ".keep" file is written alongside it to hold the
- pack in place until the references have been updated, and is removed
- afterwards. The path used to remove it is derived via
+ git-index-pack(1), a ".keep" file is written alongside it in the
+ transaction quarantine directory and also gets migrated to the main ODB
+ when the ODB transaction is committed. This keep lockfile ensures the
+ packfile remains in place until the references have been updated and is
+ removed afterwards. The path used to remove it is derived via
`index_pack_lockfile()` from the repository's primary object directory.
In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
@@ Commit message
parent process would set the temporary directory set as an alternate
only. By using ODB transactions, the ODB source list is also reordered
for the parent process which results in `index_pack_lockfile()` deriving
- the ".keep" path relative to the temporary directory instead the actual
- main ODB source path. Consequently, this prevents the ".keep" file from
- being properly removed after being migrated into the main ODB source
- post-commit.
+ the ".keep" path relative to the temporary directory instead of the
+ actual main ODB source path. Consequently, this prevents the ".keep"
+ file from being properly removed after being migrated into the main ODB
+ source post-commit.
Update `index_pack_lockfile()` to operate on an ODB source explicitly
provided to it and update call sites accordingly to pass the expected
@@ pack-write.c: char *index_pack_lockfile(struct repository *r, int ip_out, int *i
## pack.h ##
@@
-
- #include "object.h"
- #include "csum-file.h"
-+#include "odb/source.h"
-
struct packed_git;
struct pack_window;
+ struct repository;
++struct odb_source;
+
+ /*
+ * Packed object header
@@ pack.h: off_t write_pack_header(struct hashfile *f, uint32_t);
void fixup_pack_header_footer(const struct git_hash_algo *, int,
unsigned char *, const char *, uint32_t,
@@ t/t5547-push-quarantine.sh: test_expect_success 'updating a ref from quarantine
+ git init --bare keep.git &&
+
+ git -C keep.git config set receive.unpackLimit 0 &&
++
++ # While incoming objects are still quarantined, validate that the keep
++ # lockfile does indeed exist.
++ test_hook -C keep.git pre-receive <<-\EOF &&
++ keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" &&
++ test -f "$keep"
++ EOF
++
+ test_commit foo &&
+ git push keep.git HEAD &&
+ pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
2: dba9696866 ! 2: 49254af71c odb/transaction: add transaction finalize interface
@@ Commit message
after the reference updates are finished.
All other callers commit a transaction and immediately finalize it with
- no work in between and cannot meaningfully recover should either step
- fail, so introduce an `odb_transaction_commit_and_finalize_or_die()`
- helper that performs both and dies on error. Call sites are updated
+ no work in between and cannot meaningfully recover should either fail,
+ so introduce an `odb_transaction_commit_and_finalize_or_die()` helper
+ that performs both and dies on error. Call sites are updated
accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
3: 09bc00a070 = 3: 882cf06bc3 builtin/receive-pack: pass shallow file explicitly
4: 2586ea4041 = 4: 8deec37a09 builtin/receive-pack: read unpack limit config lazily
5: adf325095e = 5: 92d56134f0 builtin/receive-pack: lift global state out of unpack()
6: 29f407bf36 = 6: d614b10715 builtin/receive-pack: report unpack errors via strbuf
7: b85f5e868c = 7: bc5839ad8e builtin/receive-pack: explicitly pass packfile fd
8: 620eafe035 = 8: 13540b91b8 odb: return temporary ODB source when set
9: 2e75a8bd6c ! 9: 62d46d5c07 odb/transaction: add transaction interface to write packfiles
@@ object-file.c
#include "setup.h"
#include "strvec.h"
#include "tempfile.h"
-@@ object-file.c: struct odb_transaction_files {
+@@ object-file.c: struct transaction_packfile {
+
+ struct odb_transaction_files {
+ struct odb_transaction base;
++ enum odb_transaction_flags flags;
+
+ struct tmp_objdir *objdir;
struct odb_source *quarantine;
struct transaction_packfile packfile;
const char *prefix;
@@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *b
+ ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
+}
+
-+static unsigned int get_unpack_limit(struct repository *repo)
++static unsigned int get_unpack_limit(struct repository *repo,
++ enum odb_transaction_flags flags)
+{
-+ unsigned int limit = 100;
++ unsigned int limit = 0;
+
-+ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
-+ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
++ if (flags & ODB_TRANSACTION_RECEIVE) {
++ limit = 100;
++ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
++ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
++ }
+
+ return limit;
+}
@@ object-file.c: static int odb_transaction_files_commit(struct odb_transaction *b
+
+ odb_transaction_env(base, &child.env);
+
-+ if (ntohl(hdr.hdr_entries) < get_unpack_limit(repo)) {
++ if (ntohl(hdr.hdr_entries) < get_unpack_limit(repo, transaction->flags)) {
+ strvec_push(&child.args, "unpack-objects");
+ push_header_arg(&child.args, &hdr);
+ if (opts->quiet)
@@ object-file.c: int odb_transaction_files_begin(struct odb_source *source,
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+ transaction->base.write_pack = odb_transaction_files_write_pack;
transaction->base.env = odb_transaction_files_env;
++ transaction->flags = flags;
transaction->prefix = "bulk-fsync";
+ if (flags & ODB_TRANSACTION_RECEIVE) {
## odb/transaction.c ##
@@ odb/transaction.c: int odb_transaction_write_object_stream(struct odb_transaction *transaction,
base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
--
2.55.0.424.g13c7afec21
^ permalink raw reply [flat|nested] 78+ messages in thread* [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-20 6:46 ` Patrick Steinhardt
2026-08-19 21:53 ` [PATCH v4 2/9] odb/transaction: add transaction finalize interface Justin Tobler
` (8 subsequent siblings)
9 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When git-receive-pack(1) stores an incoming packfile with
git-index-pack(1), a ".keep" file is written alongside it in the
transaction quarantine directory and also gets migrated to the main ODB
when the ODB transaction is committed. This keep lockfile ensures the
packfile remains in place until the references have been updated and is
removed afterwards. The path used to remove it is derived via
`index_pack_lockfile()` from the repository's primary object directory.
In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10), git-receive-pack(1) started using the ODB
transaction interfaces instead of managing a temporary directory
directly. When starting an ODB transaction, the sources list is
reordered to insert the newly created transaction source first as the
primary to ensure writes are routed to it accordingly.
Prior to using ODB transactions, git-receive-pack(1) would only set the
temporary directory as the primary source for the child
git-index-pack(1) and git-unpack-objects(1) processes it spawned and the
parent process would set the temporary directory set as an alternate
only. By using ODB transactions, the ODB source list is also reordered
for the parent process which results in `index_pack_lockfile()` deriving
the ".keep" path relative to the temporary directory instead of the
actual main ODB source path. Consequently, this prevents the ".keep"
file from being properly removed after being migrated into the main ODB
source post-commit.
Update `index_pack_lockfile()` to operate on an ODB source explicitly
provided to it and update call sites accordingly to pass the expected
ODB source.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 8 +++++++-
fetch-pack.c | 2 +-
pack-write.c | 7 ++++---
pack.h | 4 +++-
t/t5547-push-quarantine.sh | 22 ++++++++++++++++++++++
5 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 86933d8d7e..d74b787148 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si,
if (status)
return "index-pack fork failed";
- lockfile = index_pack_lockfile(the_repository, child.out, NULL);
+ /*
+ * The lockfile filepath is expected to be the final location of
+ * the ".keep" file after being migrated to the main ODB source.
+ * This ensures the lockfile can be found and removed later
+ * after the ODB transaction has been committed.
+ */
+ lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
if (lockfile) {
pack_lockfile = register_tempfile(lockfile);
free(lockfile);
diff --git a/fetch-pack.c b/fetch-pack.c
index 922a9b2581..6df5813b33 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1075,7 +1075,7 @@ static int get_pack(struct fetch_pack_args *args,
die(_("fetch-pack: unable to fork off %s"), cmd_name);
if (do_keep && (pack_lockfiles || fsck_objects)) {
int is_well_formed;
- char *pack_lockfile = index_pack_lockfile(the_repository,
+ char *pack_lockfile = index_pack_lockfile(the_repository->objects->sources,
cmd.out,
&is_well_formed);
diff --git a/pack-write.c b/pack-write.c
index 24033a9101..85674e4b72 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -469,10 +469,11 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,
fsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);
}
-char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)
+char *index_pack_lockfile(struct odb_source *source, int ip_out,
+ int *is_well_formed)
{
char packname[GIT_MAX_HEXSZ + 6];
- const int len = r->hash_algo->hexsz + 6;
+ const int len = source->odb->repo->hash_algo->hexsz + 6;
/*
* The first thing we expect from index-pack's output
@@ -489,7 +490,7 @@ char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)
packname[len-1] = 0;
if (skip_prefix(packname, "keep\t", &name))
return xstrfmt("%s/pack/pack-%s.keep",
- repo_get_object_directory(r), name);
+ source->path, name);
return NULL;
}
if (is_well_formed)
diff --git a/pack.h b/pack.h
index 1cde92082b..ada506b5c5 100644
--- a/pack.h
+++ b/pack.h
@@ -7,6 +7,7 @@
struct packed_git;
struct pack_window;
struct repository;
+struct odb_source;
/*
* Packed object header
@@ -105,7 +106,8 @@ off_t write_pack_header(struct hashfile *f, uint32_t);
void fixup_pack_header_footer(const struct git_hash_algo *, int,
unsigned char *, const char *, uint32_t,
unsigned char *, off_t);
-char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed);
+char *index_pack_lockfile(struct odb_source *source, int fd,
+ int *is_well_formed);
struct ref;
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 0798ddab02..3da253cc1a 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -70,4 +70,26 @@ test_expect_success 'updating a ref from quarantine is forbidden' '
git -C update.git fsck
'
+test_expect_success '.keep file is removed after push' '
+ test_when_finished rm -rf keep.git &&
+ git init --bare keep.git &&
+
+ git -C keep.git config set receive.unpackLimit 0 &&
+
+ # While incoming objects are still quarantined, validate that the keep
+ # lockfile does indeed exist.
+ test_hook -C keep.git pre-receive <<-\EOF &&
+ keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" &&
+ test -f "$keep"
+ EOF
+
+ test_commit foo &&
+ git push keep.git HEAD &&
+ pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
+ keep="${pack%.pack}.keep" &&
+
+ test_path_is_file "$pack" &&
+ test_path_is_missing "$keep"
+'
+
test_done
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files
2026-08-19 21:53 ` [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
@ 2026-08-20 6:46 ` Patrick Steinhardt
2026-08-20 21:33 ` Justin Tobler
0 siblings, 1 reply; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-20 6:46 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Wed, Aug 19, 2026 at 04:53:03PM -0500, Justin Tobler wrote:
> diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
> index 0798ddab02..3da253cc1a 100755
> --- a/t/t5547-push-quarantine.sh
> +++ b/t/t5547-push-quarantine.sh
> @@ -70,4 +70,26 @@ test_expect_success 'updating a ref from quarantine is forbidden' '
> git -C update.git fsck
> '
>
> +test_expect_success '.keep file is removed after push' '
> + test_when_finished rm -rf keep.git &&
> + git init --bare keep.git &&
> +
> + git -C keep.git config set receive.unpackLimit 0 &&
> +
> + # While incoming objects are still quarantined, validate that the keep
> + # lockfile does indeed exist.
> + test_hook -C keep.git pre-receive <<-\EOF &&
> + keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" &&
> + test -f "$keep"
> + EOF
Good. So we know that the file exists while the transaction is
running...
> + test_commit foo &&
> + git push keep.git HEAD &&
> + pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
> + keep="${pack%.pack}.keep" &&
> +
> + test_path_is_file "$pack" &&
> + test_path_is_missing "$keep"
> +'
... and we know that the packfile exists without its ".keep" file once
the transaction has been committed.
What we don't verify is that the ".keep" file is getting migrated to the
target repository and stays intact while we're updating references. So
do we maybe want to add the following diff so that we test for the full
lifecycle of the ".keep" file?
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 3da253cc1a..a722a01e8d 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -83,11 +83,19 @@ test_expect_success '.keep file is removed after push' '
test -f "$keep"
EOF
+ # And when updating references the keep-file should have been migrated
+ # to the actual repository.
+ test_hook -C keep.git reference-transaction <<-\EOF &&
+ keep="$(ls objects/pack/pack-*.keep)" &&
+ test -f "$keep"
+ EOF
+
test_commit foo &&
git push keep.git HEAD &&
+
+ # Once done, there should be no ".keep" files anywhere anymore.
pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
keep="${pack%.pack}.keep" &&
-
test_path_is_file "$pack" &&
test_path_is_missing "$keep"
'
Patrick
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files
2026-08-20 6:46 ` Patrick Steinhardt
@ 2026-08-20 21:33 ` Justin Tobler
0 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 21:33 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, gitster
On 26/08/20 08:46AM, Patrick Steinhardt wrote:
> On Wed, Aug 19, 2026 at 04:53:03PM -0500, Justin Tobler wrote:
> What we don't verify is that the ".keep" file is getting migrated to the
> target repository and stays intact while we're updating references. So
> do we maybe want to add the following diff so that we test for the full
> lifecycle of the ".keep" file?
>
> diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
> index 3da253cc1a..a722a01e8d 100755
> --- a/t/t5547-push-quarantine.sh
> +++ b/t/t5547-push-quarantine.sh
> @@ -83,11 +83,19 @@ test_expect_success '.keep file is removed after push' '
> test -f "$keep"
> EOF
>
> + # And when updating references the keep-file should have been migrated
> + # to the actual repository.
> + test_hook -C keep.git reference-transaction <<-\EOF &&
> + keep="$(ls objects/pack/pack-*.keep)" &&
> + test -f "$keep"
> + EOF
> +
> test_commit foo &&
> git push keep.git HEAD &&
> +
> + # Once done, there should be no ".keep" files anywhere anymore.
> pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
> keep="${pack%.pack}.keep" &&
> -
> test_path_is_file "$pack" &&
> test_path_is_missing "$keep"
> '
Makes sense, I'll add something similar in the next version. Thanks :)
-Justin
^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v4 2/9] odb/transaction: add transaction finalize interface
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-19 21:53 ` [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-20 6:46 ` Patrick Steinhardt
2026-08-19 21:53 ` [PATCH v4 3/9] builtin/receive-pack: pass shallow file explicitly Justin Tobler
` (7 subsequent siblings)
9 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When committing an ODB transaction via `odb_transaction_commit()`, the
staged objects are made visible and the underlying transaction is freed
at the same time. Coupling these two steps does not leave room for any
post-commit transaction operations to be introduced though. Such a
capability is useful if an ODB transaction backend needs to hold on to
lockfiles after transaction commit until references are updated, as is
the case with the existing "files" backend in git-receive-pack(1).
Stop freeing the transaction in `odb_transaction_commit()` and introduce
`odb_transaction_finalize()` to explicitly clean up the transaction
accordingly. Note that the finalize interface also provides an optional
callback for any backend-specific deferred cleanup. In a subsequent
commit, the "files" transaction backend will use this to remove ".keep"
files generated for packfiles received via git-receive-pack(1) after
references have been updated. In preparation for this, the
`odb_transaction_finalize()` call site in git-receive-pack(1) is made
after the reference updates are finished.
All other callers commit a transaction and immediately finalize it with
no work in between and cannot meaningfully recover should either fail,
so introduce an `odb_transaction_commit_and_finalize_or_die()` helper
that performs both and dies on error. Call sites are updated
accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/add.c | 4 ++--
builtin/receive-pack.c | 1 +
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 4 ++--
cache-tree.c | 2 +-
object-file.c | 2 +-
odb/transaction.c | 14 ++++++++++++++
odb/transaction.h | 23 +++++++++++++++++++++++
read-cache.c | 2 +-
9 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 60ffbede2b..ad418a5952 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -393,7 +393,7 @@ int cmd_add(int argc,
char *seen = NULL;
char *ps_matched = NULL;
struct lock_file lock_file = LOCK_INIT;
- struct odb_transaction *transaction;
+ struct odb_transaction *transaction = NULL;
repo_config(repo, add_config, NULL);
@@ -600,7 +600,7 @@ int cmd_add(int argc,
if (chmod_arg && pathspec.nr)
exit_status |= chmod_pathspec(repo, &pathspec, chmod_arg[0], show_only);
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
finish:
if (write_locked_index(repo->index, &lock_file,
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d74b787148..ed1edcbe93 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2720,6 +2720,7 @@ int cmd_receive_pack(int argc,
use_keepalive = KEEPALIVE_ALWAYS;
execute_commands(commands, unpack_status, &si, transaction,
&push_options);
+ odb_transaction_finalize(transaction);
delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 4263edfbec..d6a2d616d9 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -603,7 +603,7 @@ static void unpack_all(void)
unpack_one(i);
display_progress(progress, i + 1);
}
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
stop_progress(&progress);
if (delta_list)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 241abd4332..b25d4ecb10 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1156,7 +1156,7 @@ int cmd_update_index(int argc,
* a transaction.
*/
if (transaction && verbose) {
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
transaction = NULL;
}
@@ -1224,7 +1224,7 @@ int cmd_update_index(int argc,
/*
* By now we have added all of the new objects
*/
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
if (split_index > 0) {
if (repo_config_get_split_index(the_repository) == 0)
diff --git a/cache-tree.c b/cache-tree.c
index d92f513286..a220372a42 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -538,7 +538,7 @@ int cache_tree_update(struct index_state *istate, int flags)
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
trace2_region_leave("cache_tree", "update", istate->repo);
trace_performance_leave("cache_tree_update");
if (i < 0)
diff --git a/object-file.c b/object-file.c
index ec35c318bc..4d03c167d5 100644
--- a/object-file.c
+++ b/object-file.c
@@ -965,7 +965,7 @@ int index_fd(struct index_state *istate, struct object_id *oid,
xsize_t(st->st_size),
oid);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
} else {
ret = hash_blob_stream(&stream,
the_repository->hash_algo, oid,
diff --git a/odb/transaction.c b/odb/transaction.c
index dab7da6a9a..9e9a982778 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -33,6 +33,20 @@ int odb_transaction_commit(struct odb_transaction *transaction)
ret = transaction->commit(transaction);
transaction->source->odb->transaction = NULL;
+
+ return ret;
+}
+
+int odb_transaction_finalize(struct odb_transaction *transaction)
+{
+ int ret = 0;
+
+ if (!transaction)
+ return 0;
+
+ if (transaction->finalize)
+ ret = transaction->finalize(transaction);
+
free(transaction);
return ret;
diff --git a/odb/transaction.h b/odb/transaction.h
index 4cb2eafcbf..6ed39b3d0e 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -22,6 +22,13 @@ struct odb_transaction {
*/
int (*commit)(struct odb_transaction *transaction);
+ /*
+ * Optional ODB source specific callback invoked when the transaction
+ * needs to perform any deferred cleanup after objects have been
+ * committed. Returns 0 on success, a negative error code otherwise.
+ */
+ int (*finalize)(struct odb_transaction *transaction);
+
/*
* This callback is expected to write the given object stream into
* the ODB transaction. Note that for now, only blobs support streaming.
@@ -75,6 +82,22 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
*/
int odb_transaction_commit(struct odb_transaction *transaction);
+/*
+ * Finalizes an ODB transaction, performing any deferred cleanup and freeing it.
+ * Must be called for every successfully started transaction. Note that, if the
+ * specified transaction is NULL, the function is a no-op. Returns 0 on success,
+ * a negative error code otherwise.
+ */
+int odb_transaction_finalize(struct odb_transaction *transaction);
+
+static inline void odb_transaction_commit_and_finalize_or_die(struct odb_transaction *transaction)
+{
+ if (odb_transaction_commit(transaction))
+ die(_("failed to commit ODB transaction"));
+ if (odb_transaction_finalize(transaction))
+ die(_("failed to finalize ODB transaction"));
+}
+
/*
* Writes the object in the provided stream into the transaction. The resulting
* object ID is written into the out pointer. Returns 0 on success, a negative
diff --git a/read-cache.c b/read-cache.c
index 6c449f393d..0cd0ef85ec 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4049,7 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
odb_transaction_begin_or_die(repo->objects, &transaction, 0);
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
release_revisions(&rev);
return !!data.add_errors;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v4 2/9] odb/transaction: add transaction finalize interface
2026-08-19 21:53 ` [PATCH v4 2/9] odb/transaction: add transaction finalize interface Justin Tobler
@ 2026-08-20 6:46 ` Patrick Steinhardt
0 siblings, 0 replies; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-20 6:46 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Wed, Aug 19, 2026 at 04:53:04PM -0500, Justin Tobler wrote:
> When committing an ODB transaction via `odb_transaction_commit()`, the
> staged objects are made visible and the underlying transaction is freed
> at the same time. Coupling these two steps does not leave room for any
> post-commit transaction operations to be introduced though. Such a
> capability is useful if an ODB transaction backend needs to hold on to
> lockfiles after transaction commit until references are updated, as is
> the case with the existing "files" backend in git-receive-pack(1).
>
> Stop freeing the transaction in `odb_transaction_commit()` and introduce
> `odb_transaction_finalize()` to explicitly clean up the transaction
> accordingly. Note that the finalize interface also provides an optional
> callback for any backend-specific deferred cleanup. In a subsequent
> commit, the "files" transaction backend will use this to remove ".keep"
> files generated for packfiles received via git-receive-pack(1) after
> references have been updated. In preparation for this, the
> `odb_transaction_finalize()` call site in git-receive-pack(1) is made
> after the reference updates are finished.
>
> All other callers commit a transaction and immediately finalize it with
> no work in between and cannot meaningfully recover should either fail,
> so introduce an `odb_transaction_commit_and_finalize_or_die()` helper
> that performs both and dies on error. Call sites are updated
> accordingly.
That paragraph is a bit hard to read. How about:
All other callers commit a transaction and immediately finalize it
without any work happening in between those two operations.
Consequently, they cannot meaningfully recover in case either of
them would fail, and spelling out these two separate steps with
proper error handling would be quite repetitive and pointless.
Introduce a helper `odb_transaction_commit_and_finalize_or_die()` to
help those call sites and update them accordingly.
Patrick
^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v4 3/9] builtin/receive-pack: pass shallow file explicitly
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-19 21:53 ` [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
2026-08-19 21:53 ` [PATCH v4 2/9] odb/transaction: add transaction finalize interface Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-19 21:53 ` [PATCH v4 4/9] builtin/receive-pack: read unpack limit config lazily Justin Tobler
` (6 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
If shallow information is provided during `unpack()`, a temporary
shallow file is created and stored in global state. In a subsequent
commit, the `unpack()` logic is moved behind a generic ODB transaction
interface to handle writing packfiles and thus can no longer rely on
such global state. Lift the setup of the temporary shallow file out of
`unpack()` and wire it through to its call sites explicitly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ed1edcbe93..135105deae 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -86,7 +86,6 @@ static const char *head_name;
static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
-static const char *alt_shallow_file;
static struct strbuf push_cert = STRBUF_INIT;
static struct object_id push_cert_oid;
static struct signature_check sigcheck;
@@ -2334,8 +2333,8 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
-static const char *unpack(int err_fd, struct shallow_info *si,
- struct odb_transaction *transaction)
+static const char *unpack(struct odb_transaction *transaction,
+ const char *shallow_file, int err_fd)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2354,10 +2353,9 @@ static const char *unpack(int err_fd, struct shallow_info *si,
return hdr_err;
}
- if (si->nr_ours || si->nr_theirs) {
- alt_shallow_file = setup_temporary_shallow(si->shallow);
+ if (shallow_file) {
strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, alt_shallow_file);
+ strvec_push(&child.args, shallow_file);
}
odb_transaction_env(transaction, &child.env);
@@ -2433,14 +2431,14 @@ static const char *unpack(int err_fd, struct shallow_info *si,
return NULL;
}
-static const char *unpack_with_sideband(struct shallow_info *si,
- struct odb_transaction *transaction)
+static const char *unpack_with_sideband(struct odb_transaction *transaction,
+ const char *shallow_file)
{
struct async muxer;
const char *ret;
if (!use_sideband)
- return unpack(0, si, transaction);
+ return unpack(transaction, shallow_file, 0);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2449,13 +2447,14 @@ static const char *unpack_with_sideband(struct shallow_info *si,
if (start_async(&muxer))
return NULL;
- ret = unpack(muxer.in, si, transaction);
+ ret = unpack(transaction, shallow_file, muxer.in);
finish_async(&muxer);
return ret;
}
-static void prepare_shallow_update(struct shallow_info *si)
+static void prepare_shallow_update(struct shallow_info *si,
+ const char *shallow_file)
{
int i, j, k, bitmap_size = DIV_ROUND_UP(si->ref->nr, 32);
@@ -2495,12 +2494,13 @@ static void prepare_shallow_update(struct shallow_info *si)
* command. check_connected() will be done with
* true .git/shallow though.
*/
- setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1);
+ setenv(GIT_SHALLOW_FILE_ENVIRONMENT, shallow_file, 1);
}
static void update_shallow_info(struct command *commands,
struct shallow_info *si,
- struct oid_array *ref)
+ struct oid_array *ref,
+ const char *shallow_file)
{
struct command *cmd;
int *ref_status;
@@ -2519,7 +2519,7 @@ static void update_shallow_info(struct command *commands,
si->ref = ref;
if (shallow_update) {
- prepare_shallow_update(si);
+ prepare_shallow_update(si, shallow_file);
return;
}
@@ -2711,11 +2711,17 @@ int cmd_receive_pack(int argc,
if (!si.nr_ours && !si.nr_theirs)
shallow_update = 0;
if (!delete_only(commands)) {
+ const char *alt_shallow_file = NULL;
+
+ if (si.nr_ours || si.nr_theirs)
+ alt_shallow_file = setup_temporary_shallow(si.shallow);
+
if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
unpack_status = "unable to start object transaction";
else
- unpack_status = unpack_with_sideband(&si, transaction);
- update_shallow_info(commands, &si, &ref);
+ unpack_status = unpack_with_sideband(transaction, alt_shallow_file);
+
+ update_shallow_info(commands, &si, &ref, alt_shallow_file);
}
use_keepalive = KEEPALIVE_ALWAYS;
execute_commands(commands, unpack_status, &si, transaction,
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v4 4/9] builtin/receive-pack: read unpack limit config lazily
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (2 preceding siblings ...)
2026-08-19 21:53 ` [PATCH v4 3/9] builtin/receive-pack: pass shallow file explicitly Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-19 21:53 ` [PATCH v4 5/9] builtin/receive-pack: lift global state out of unpack() Justin Tobler
` (5 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), the `receive.unpackLimit` and
`transfer.unpackLimit` configuration decides whether an incoming
packfile should be exploded into loose objects or kept as a packfile
on-disk. In a subsequent commit, the logic to write the incoming
packfile is made ODB backend agnostic and moved behind a pluggable ODB
transaction interface. Consequently, whether to explode a packfile is a
detail of how a particular backend stores objects and should not be a
part of the generic interface itself.
In preparation for this, instead resolve the unpack limit lazily inside
`unpack()` by reading the configuration directly. The now-unused unpack
limit globals are dropped accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 135105deae..971dc3f52e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -62,12 +62,9 @@ static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
static int receive_fsck_objects = -1;
static int transfer_fsck_objects = -1;
static struct strbuf fsck_msg_types = STRBUF_INIT;
-static int receive_unpack_limit = -1;
-static int transfer_unpack_limit = -1;
static int advertise_atomic_push = 1;
static int advertise_push_options;
static int advertise_sid;
-static int unpack_limit = 100;
static off_t max_input_size;
static int report_status;
static int report_status_v2;
@@ -157,16 +154,6 @@ static int receive_pack_config(const char *var, const char *value,
return 0;
}
- if (strcmp(var, "receive.unpacklimit") == 0) {
- receive_unpack_limit = git_config_int(var, value, ctx->kvi);
- return 0;
- }
-
- if (strcmp(var, "transfer.unpacklimit") == 0) {
- transfer_unpack_limit = git_config_int(var, value, ctx->kvi);
- return 0;
- }
-
if (strcmp(var, "receive.fsck.skiplist") == 0) {
char *path;
@@ -2333,6 +2320,16 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
+static unsigned int get_unpack_limit(struct repository *repo)
+{
+ unsigned int limit = 100;
+
+ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
+ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
+
+ return limit;
+}
+
static const char *unpack(struct odb_transaction *transaction,
const char *shallow_file, int err_fd)
{
@@ -2360,7 +2357,7 @@ static const char *unpack(struct odb_transaction *transaction,
odb_transaction_env(transaction, &child.env);
- if (ntohl(hdr.hdr_entries) < unpack_limit) {
+ if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
strvec_push(&child.args, "unpack-objects");
push_header_arg(&child.args, &hdr);
if (quiet)
@@ -2658,11 +2655,6 @@ int cmd_receive_pack(int argc,
if (cert_nonce_seed)
push_cert_nonce = prepare_push_cert_nonce(service_dir, time(NULL));
- if (0 <= receive_unpack_limit)
- unpack_limit = receive_unpack_limit;
- else if (0 <= transfer_unpack_limit)
- unpack_limit = transfer_unpack_limit;
-
switch (determine_protocol_version_server()) {
case protocol_v2:
/*
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v4 5/9] builtin/receive-pack: lift global state out of unpack()
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (3 preceding siblings ...)
2026-08-19 21:53 ` [PATCH v4 4/9] builtin/receive-pack: read unpack limit config lazily Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-19 21:53 ` [PATCH v4 6/9] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
` (4 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), writing the packfile to the transaction is
handled via `unpack()` which relies on global variables to decide how to
invoke the underlying git-index-pack(1) or git-unpack-objects(1) child
processes. In a subsequent commit, the `unpack()` logic is moved behind
a generic ODB transaction interface to handle writing packfiles and thus
can no longer rely on these globals.
Lift the global state out of `unpack()` by instead storing this state in
a `struct unpack_opts` that gets passed to the function explicitly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 63 +++++++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 22 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 971dc3f52e..f062b93b8d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2330,18 +2330,24 @@ static unsigned int get_unpack_limit(struct repository *repo)
return limit;
}
+struct unpack_opts {
+ const char *fsck_msg_types;
+ const char *shallow_file;
+ off_t max_input_size;
+ int fsck_objects;
+ int reject_thin;
+ int err_fd;
+ int quiet;
+};
+
static const char *unpack(struct odb_transaction *transaction,
- const char *shallow_file, int err_fd)
+ const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
int status;
struct child_process child = CHILD_PROCESS_INIT;
- int fsck_objects = (receive_fsck_objects >= 0
- ? receive_fsck_objects
- : transfer_fsck_objects >= 0
- ? transfer_fsck_objects
- : 0);
+ int err_fd = opts->err_fd;
hdr_err = parse_pack_header(&hdr);
if (hdr_err) {
@@ -2350,9 +2356,9 @@ static const char *unpack(struct odb_transaction *transaction,
return hdr_err;
}
- if (shallow_file) {
+ if (opts->shallow_file) {
strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, shallow_file);
+ strvec_push(&child.args, opts->shallow_file);
}
odb_transaction_env(transaction, &child.env);
@@ -2360,14 +2366,14 @@ static const char *unpack(struct odb_transaction *transaction,
if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
strvec_push(&child.args, "unpack-objects");
push_header_arg(&child.args, &hdr);
- if (quiet)
+ if (opts->quiet)
strvec_push(&child.args, "-q");
- if (fsck_objects)
+ if (opts->fsck_objects)
strvec_pushf(&child.args, "--strict%s",
- fsck_msg_types.buf);
- if (max_input_size)
+ opts->fsck_msg_types);
+ if (opts->max_input_size)
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)max_input_size);
+ (uintmax_t)opts->max_input_size);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -2388,18 +2394,18 @@ static const char *unpack(struct odb_transaction *transaction,
(uintmax_t)getpid(),
hostname);
- if (!quiet && err_fd)
+ if (!opts->quiet && err_fd)
strvec_push(&child.args, "--show-resolving-progress");
- if (use_sideband)
+ if (err_fd)
strvec_push(&child.args, "--report-end-of-input");
- if (fsck_objects)
+ if (opts->fsck_objects)
strvec_pushf(&child.args, "--strict%s",
- fsck_msg_types.buf);
- if (!reject_thin)
+ opts->fsck_msg_types);
+ if (!opts->reject_thin)
strvec_push(&child.args, "--fix-thin");
- if (max_input_size)
+ if (opts->max_input_size)
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)max_input_size);
+ (uintmax_t)opts->max_input_size);
child.out = -1;
child.err = err_fd;
child.git_cmd = 1;
@@ -2431,11 +2437,23 @@ static const char *unpack(struct odb_transaction *transaction,
static const char *unpack_with_sideband(struct odb_transaction *transaction,
const char *shallow_file)
{
+ struct unpack_opts opts = {
+ .fsck_objects = (receive_fsck_objects >= 0
+ ? receive_fsck_objects
+ : transfer_fsck_objects >= 0
+ ? transfer_fsck_objects
+ : 0),
+ .fsck_msg_types = fsck_msg_types.buf,
+ .max_input_size = max_input_size,
+ .shallow_file = shallow_file,
+ .reject_thin = reject_thin,
+ .quiet = quiet,
+ };
struct async muxer;
const char *ret;
if (!use_sideband)
- return unpack(transaction, shallow_file, 0);
+ return unpack(transaction, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2444,7 +2462,8 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction,
if (start_async(&muxer))
return NULL;
- ret = unpack(transaction, shallow_file, muxer.in);
+ opts.err_fd = muxer.in;
+ ret = unpack(transaction, &opts);
finish_async(&muxer);
return ret;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v4 6/9] builtin/receive-pack: report unpack errors via strbuf
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (4 preceding siblings ...)
2026-08-19 21:53 ` [PATCH v4 5/9] builtin/receive-pack: lift global state out of unpack() Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-19 21:53 ` [PATCH v4 7/9] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
` (3 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When writing packfiles via `unpack()`, error messages are returned
directly by the function. In preparation for `unpack()` logic being
moved behind a generic ODB transaction interface, update the function to
instead write any error messages to a caller provided strbuf and return
a negative value on error. Call sites are updated to use the error
strbuf accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 63 ++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f062b93b8d..6df872697b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2015,7 +2015,7 @@ static void execute_commands_atomic(struct command *commands,
}
static void execute_commands(struct command *commands,
- const char *unpacker_error,
+ int unpacker_error,
struct shallow_info *si,
struct odb_transaction *transaction,
const struct string_list *push_options)
@@ -2340,8 +2340,8 @@ struct unpack_opts {
int quiet;
};
-static const char *unpack(struct odb_transaction *transaction,
- const struct unpack_opts *opts)
+static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
+ const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2353,7 +2353,8 @@ static const char *unpack(struct odb_transaction *transaction,
if (hdr_err) {
if (err_fd > 0)
close(err_fd);
- return hdr_err;
+ strbuf_addstr(err_msg, hdr_err);
+ return -1;
}
if (opts->shallow_file) {
@@ -2378,8 +2379,10 @@ static const char *unpack(struct odb_transaction *transaction,
child.err = err_fd;
child.git_cmd = 1;
status = run_command(&child);
- if (status)
- return "unpack-objects abnormal exit";
+ if (status) {
+ strbuf_addstr(err_msg, "unpack-objects abnormal exit");
+ return -1;
+ }
} else {
char hostname[HOST_NAME_MAX + 1];
char *lockfile;
@@ -2410,8 +2413,10 @@ static const char *unpack(struct odb_transaction *transaction,
child.err = err_fd;
child.git_cmd = 1;
status = start_command(&child);
- if (status)
- return "index-pack fork failed";
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack fork failed");
+ return -1;
+ }
/*
* The lockfile filepath is expected to be the final location of
@@ -2427,15 +2432,18 @@ static const char *unpack(struct odb_transaction *transaction,
close(child.out);
status = finish_command(&child);
- if (status)
- return "index-pack abnormal exit";
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack abnormal exit");
+ return -1;
+ }
odb_reprepare(the_repository->objects);
}
- return NULL;
+ return 0;
}
-static const char *unpack_with_sideband(struct odb_transaction *transaction,
- const char *shallow_file)
+static int unpack_with_sideband(struct odb_transaction *transaction,
+ const char *shallow_file,
+ struct strbuf *err_msg)
{
struct unpack_opts opts = {
.fsck_objects = (receive_fsck_objects >= 0
@@ -2450,20 +2458,20 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction,
.quiet = quiet,
};
struct async muxer;
- const char *ret;
+ int ret;
if (!use_sideband)
- return unpack(transaction, &opts);
+ return unpack(transaction, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
muxer.proc = copy_to_sideband;
muxer.in = -1;
if (start_async(&muxer))
- return NULL;
+ return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, &opts);
+ ret = unpack(transaction, err_msg, &opts);
finish_async(&muxer);
return ret;
@@ -2552,13 +2560,13 @@ static void update_shallow_info(struct command *commands,
free(ref_status);
}
-static void report(struct command *commands, const char *unpack_status)
+static void report(struct command *commands, const struct strbuf *unpack_status)
{
struct command *cmd;
struct strbuf buf = STRBUF_INIT;
packet_buf_write(&buf, "unpack %s\n",
- unpack_status ? unpack_status : "ok");
+ unpack_status->len ? unpack_status->buf : "ok");
for (cmd = commands; cmd; cmd = cmd->next) {
if (!cmd->error_string)
packet_buf_write(&buf, "ok %s\n",
@@ -2576,14 +2584,14 @@ static void report(struct command *commands, const char *unpack_status)
strbuf_release(&buf);
}
-static void report_v2(struct command *commands, const char *unpack_status)
+static void report_v2(struct command *commands, const struct strbuf *unpack_status)
{
struct command *cmd;
struct strbuf buf = STRBUF_INIT;
struct ref_push_report *report;
packet_buf_write(&buf, "unpack %s\n",
- unpack_status ? unpack_status : "ok");
+ unpack_status->len ? unpack_status->buf : "ok");
for (cmd = commands; cmd; cmd = cmd->next) {
int count = 0;
@@ -2707,8 +2715,8 @@ int cmd_receive_pack(int argc,
PACKET_READ_DIE_ON_ERR_PACKET);
if ((commands = read_head_info(&reader, &shallow))) {
- const char *unpack_status = NULL;
struct string_list push_options = STRING_LIST_INIT_DUP;
+ struct strbuf unpack_status = STRBUF_INIT;
if (use_push_options)
read_push_options(&reader, &push_options);
@@ -2728,22 +2736,22 @@ int cmd_receive_pack(int argc,
alt_shallow_file = setup_temporary_shallow(si.shallow);
if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
- unpack_status = "unable to start object transaction";
+ strbuf_addstr(&unpack_status, "unable to start object transaction");
else
- unpack_status = unpack_with_sideband(transaction, alt_shallow_file);
+ unpack_with_sideband(transaction, alt_shallow_file, &unpack_status);
update_shallow_info(commands, &si, &ref, alt_shallow_file);
}
use_keepalive = KEEPALIVE_ALWAYS;
- execute_commands(commands, unpack_status, &si, transaction,
+ execute_commands(commands, !!unpack_status.len, &si, transaction,
&push_options);
odb_transaction_finalize(transaction);
delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
- report_v2(commands, unpack_status);
+ report_v2(commands, &unpack_status);
else if (report_status)
- report(commands, unpack_status);
+ report(commands, &unpack_status);
sigchain_pop(SIGPIPE);
run_receive_hook(commands, "post-receive", 1, NULL,
&push_options);
@@ -2768,6 +2776,7 @@ int cmd_receive_pack(int argc,
if (auto_update_server_info)
update_server_info(the_repository, 0);
clear_shallow_info(&si);
+ strbuf_release(&unpack_status);
}
if (use_sideband)
packet_flush(1);
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v4 7/9] builtin/receive-pack: explicitly pass packfile fd
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (5 preceding siblings ...)
2026-08-19 21:53 ` [PATCH v4 6/9] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-19 21:53 ` [PATCH v4 8/9] odb: return temporary ODB source when set Justin Tobler
` (2 subsequent siblings)
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When processing the incoming packfile in git-receive-pack(1), `unpack()`
assumes it should always read it from stdin. In preparation for
`unpack()` logic being moved behind a generic ODB transaction interface,
update the function signature to take the an explicit fd provided by
callers to read the incoming packfile from instead. Call sites are
updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6df872697b..b369466783 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2292,9 +2292,9 @@ static void read_push_options(struct packet_reader *reader,
}
}
-static const char *parse_pack_header(struct pack_header *hdr)
+static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
{
- switch (read_pack_header(0, hdr)) {
+ switch (read_pack_header(pack_fd, hdr)) {
case PH_ERROR_EOF:
return "eof before pack header was fully read";
@@ -2340,8 +2340,8 @@ struct unpack_opts {
int quiet;
};
-static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
- const struct unpack_opts *opts)
+static int unpack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg, const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2349,7 +2349,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
struct child_process child = CHILD_PROCESS_INIT;
int err_fd = opts->err_fd;
- hdr_err = parse_pack_header(&hdr);
+ hdr_err = parse_pack_header(&hdr, pack_fd);
if (hdr_err) {
if (err_fd > 0)
close(err_fd);
@@ -2376,6 +2376,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
(uintmax_t)opts->max_input_size);
child.no_stdout = 1;
+ child.in = pack_fd;
child.err = err_fd;
child.git_cmd = 1;
status = run_command(&child);
@@ -2410,6 +2411,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
(uintmax_t)opts->max_input_size);
child.out = -1;
+ child.in = pack_fd;
child.err = err_fd;
child.git_cmd = 1;
status = start_command(&child);
@@ -2461,7 +2463,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
int ret;
if (!use_sideband)
- return unpack(transaction, err_msg, &opts);
+ return unpack(transaction, 0, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2471,7 +2473,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, err_msg, &opts);
+ ret = unpack(transaction, 0, err_msg, &opts);
finish_async(&muxer);
return ret;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v4 8/9] odb: return temporary ODB source when set
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (6 preceding siblings ...)
2026-08-19 21:53 ` [PATCH v4 7/9] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-19 21:53 ` [PATCH v4 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
9 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When invoked, `odb_set_temporary_primary_source()` installs a temporary
object directory as the new primary ODB source. A caller that wants to
operate on the ODB source of the open transaction must assume that it is
the first entry in the ODB source list which is a bit awkward and
fragile.
Instead, return the newly installed source directly and report the
previous primary source via a new `prev_source` out parameter. Propagate
the installed source through `tmp_objdir_replace_primary_odb()` and
start storing it in the "files" ODB transaction so a subsequent commit
can easily access it without relying on the ODB source list ordering.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 3 ++-
odb.c | 9 +++++++--
odb.h | 6 ++++--
tmp-objdir.c | 8 +++++---
tmp-objdir.h | 6 ++++--
5 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/object-file.c b/object-file.c
index 4d03c167d5..db63587f6d 100644
--- a/object-file.c
+++ b/object-file.c
@@ -485,6 +485,7 @@ struct odb_transaction_files {
struct odb_transaction base;
struct tmp_objdir *objdir;
+ struct odb_source *quarantine;
struct transaction_packfile packfile;
const char *prefix;
};
@@ -507,7 +508,7 @@ int odb_transaction_files_prepare(struct odb_transaction *base)
if (!transaction->objdir)
return error(_("unable to create temporary object directory"));
- tmp_objdir_replace_primary_odb(transaction->objdir, 0);
+ transaction->quarantine = tmp_objdir_replace_primary_odb(transaction->objdir, 0);
return 0;
}
diff --git a/odb.c b/odb.c
index caf1d0f542..8afcb6b637 100644
--- a/odb.c
+++ b/odb.c
@@ -226,7 +226,8 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
}
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
- const char *dir, int will_destroy)
+ const char *dir, int will_destroy,
+ struct odb_source **prev_source)
{
struct odb_source *source;
@@ -250,7 +251,11 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
source->will_destroy = will_destroy;
source->next = odb->sources;
odb->sources = source;
- return source->next;
+
+ if (prev_source)
+ *prev_source = source->next;
+
+ return source;
}
void odb_restore_primary_source(struct object_database *odb,
diff --git a/odb.h b/odb.h
index fca67e8253..bdfcb9509a 100644
--- a/odb.h
+++ b/odb.h
@@ -199,10 +199,12 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha
/*
* Replace the current writable object directory with the specified temporary
- * object directory; returns the former primary source.
+ * object directory and return the newly installed primary source. The former
+ * primary source is reported via `prev_source` when non-NULL.
*/
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
- const char *dir, int will_destroy);
+ const char *dir, int will_destroy,
+ struct odb_source **prev_source);
/*
* Restore the primary source that was previously replaced by
diff --git a/tmp-objdir.c b/tmp-objdir.c
index d199d39e7c..e633d97e0e 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -327,11 +327,13 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)
odb_add_to_alternates_memory(t->repo->objects, t->path.buf);
}
-void tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy)
+struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t,
+ int will_destroy)
{
if (t->prev_source)
BUG("the primary object database is already replaced");
- t->prev_source = odb_set_temporary_primary_source(t->repo->objects,
- t->path.buf, will_destroy);
t->will_destroy = will_destroy;
+
+ return odb_set_temporary_primary_source(t->repo->objects, t->path.buf,
+ will_destroy, &t->prev_source);
}
diff --git a/tmp-objdir.h b/tmp-objdir.h
index ccf800faa7..81eb927413 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -64,8 +64,10 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
/*
* Replaces the writable object store in the current process with the temporary
* object directory and makes the former main object store an alternate.
- * If will_destroy is nonzero, the object directory may not be migrated.
+ * If will_destroy is nonzero, the object directory may not be migrated. Returns
+ * the newly installed primary source.
*/
-void tmp_objdir_replace_primary_odb(struct tmp_objdir *, int will_destroy);
+struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *,
+ int will_destroy);
#endif /* TMP_OBJDIR_H */
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v4 9/9] odb/transaction: add transaction interface to write packfiles
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (7 preceding siblings ...)
2026-08-19 21:53 ` [PATCH v4 8/9] odb: return temporary ODB source when set Justin Tobler
@ 2026-08-19 21:53 ` Justin Tobler
2026-08-20 6:46 ` Patrick Steinhardt
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
9 siblings, 1 reply; 78+ messages in thread
From: Justin Tobler @ 2026-08-19 21:53 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), the incoming packfile is written to the ODB via
`unpack()`, which spawns git-index-pack(1) or git-unpack-objects(1)
directly. With pluggable object databases, an alternative backend may
need to handle writing packfile data differently though.
Introduce `odb_transaction_write_pack()` as a generic interface to
handle writing a packfile to a transaction and use the logic from
`unpack()` as the "files" backend implementation. Note that when storing
the objects as a packfile, git-index-pack(1) also writes a ".keep"
lockfile next to it to prevent a concurrent repack from removing the new
pack prior to reference updates being performed. The "files" transaction
backend is responsible for managing these ".keep" files and removes them
post-commit once the transaction is finalized.
Call sites in git-receive-pack(1) are updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 160 +-----------------------------------
object-file.c | 178 +++++++++++++++++++++++++++++++++++++++++
odb/transaction.c | 7 ++
odb/transaction.h | 62 ++++++++++++++
4 files changed, 250 insertions(+), 157 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b369466783..e6e54ba55f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -15,7 +15,6 @@
#include "gpg-interface.h"
#include "hex.h"
#include "hook.h"
-#include "lockfile.h"
#include "object.h"
#include "object-file.h"
#include "object-name.h"
@@ -23,7 +22,6 @@
#include "oid-array.h"
#include "oidset.h"
#include "pack.h"
-#include "packfile.h"
#include "parse-options.h"
#include "pkt-line.h"
#include "protocol.h"
@@ -2292,162 +2290,11 @@ static void read_push_options(struct packet_reader *reader,
}
}
-static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
-{
- switch (read_pack_header(pack_fd, hdr)) {
- case PH_ERROR_EOF:
- return "eof before pack header was fully read";
-
- case PH_ERROR_PACK_SIGNATURE:
- return "protocol error (pack signature mismatch detected)";
-
- case PH_ERROR_PROTOCOL:
- return "protocol error (pack version unsupported)";
-
- default:
- return "unknown error in parse_pack_header";
-
- case 0:
- return NULL;
- }
-}
-
-static struct tempfile *pack_lockfile;
-
-static void push_header_arg(struct strvec *args, struct pack_header *hdr)
-{
- strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
- ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
-}
-
-static unsigned int get_unpack_limit(struct repository *repo)
-{
- unsigned int limit = 100;
-
- repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
- repo_config_get_uint(repo, "receive.unpacklimit", &limit);
-
- return limit;
-}
-
-struct unpack_opts {
- const char *fsck_msg_types;
- const char *shallow_file;
- off_t max_input_size;
- int fsck_objects;
- int reject_thin;
- int err_fd;
- int quiet;
-};
-
-static int unpack(struct odb_transaction *transaction, int pack_fd,
- struct strbuf *err_msg, const struct unpack_opts *opts)
-{
- struct pack_header hdr;
- const char *hdr_err;
- int status;
- struct child_process child = CHILD_PROCESS_INIT;
- int err_fd = opts->err_fd;
-
- hdr_err = parse_pack_header(&hdr, pack_fd);
- if (hdr_err) {
- if (err_fd > 0)
- close(err_fd);
- strbuf_addstr(err_msg, hdr_err);
- return -1;
- }
-
- if (opts->shallow_file) {
- strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, opts->shallow_file);
- }
-
- odb_transaction_env(transaction, &child.env);
-
- if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
- strvec_push(&child.args, "unpack-objects");
- push_header_arg(&child.args, &hdr);
- if (opts->quiet)
- strvec_push(&child.args, "-q");
- if (opts->fsck_objects)
- strvec_pushf(&child.args, "--strict%s",
- opts->fsck_msg_types);
- if (opts->max_input_size)
- strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)opts->max_input_size);
- child.no_stdout = 1;
- child.in = pack_fd;
- child.err = err_fd;
- child.git_cmd = 1;
- status = run_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "unpack-objects abnormal exit");
- return -1;
- }
- } else {
- char hostname[HOST_NAME_MAX + 1];
- char *lockfile;
-
- strvec_pushl(&child.args, "index-pack", "--stdin", NULL);
- push_header_arg(&child.args, &hdr);
-
- if (xgethostname(hostname, sizeof(hostname)))
- xsnprintf(hostname, sizeof(hostname), "localhost");
- strvec_pushf(&child.args,
- "--keep=receive-pack %"PRIuMAX" on %s",
- (uintmax_t)getpid(),
- hostname);
-
- if (!opts->quiet && err_fd)
- strvec_push(&child.args, "--show-resolving-progress");
- if (err_fd)
- strvec_push(&child.args, "--report-end-of-input");
- if (opts->fsck_objects)
- strvec_pushf(&child.args, "--strict%s",
- opts->fsck_msg_types);
- if (!opts->reject_thin)
- strvec_push(&child.args, "--fix-thin");
- if (opts->max_input_size)
- strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)opts->max_input_size);
- child.out = -1;
- child.in = pack_fd;
- child.err = err_fd;
- child.git_cmd = 1;
- status = start_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "index-pack fork failed");
- return -1;
- }
-
- /*
- * The lockfile filepath is expected to be the final location of
- * the ".keep" file after being migrated to the main ODB source.
- * This ensures the lockfile can be found and removed later
- * after the ODB transaction has been committed.
- */
- lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
- if (lockfile) {
- pack_lockfile = register_tempfile(lockfile);
- free(lockfile);
- }
- close(child.out);
-
- status = finish_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "index-pack abnormal exit");
- return -1;
- }
- odb_reprepare(the_repository->objects);
- }
- return 0;
-}
-
static int unpack_with_sideband(struct odb_transaction *transaction,
const char *shallow_file,
struct strbuf *err_msg)
{
- struct unpack_opts opts = {
+ struct odb_transaction_write_pack_opts opts = {
.fsck_objects = (receive_fsck_objects >= 0
? receive_fsck_objects
: transfer_fsck_objects >= 0
@@ -2463,7 +2310,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
int ret;
if (!use_sideband)
- return unpack(transaction, 0, err_msg, &opts);
+ return odb_transaction_write_pack(transaction, 0, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2473,7 +2320,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, 0, err_msg, &opts);
+ ret = odb_transaction_write_pack(transaction, 0, err_msg, &opts);
finish_async(&muxer);
return ret;
@@ -2748,7 +2595,6 @@ int cmd_receive_pack(int argc,
execute_commands(commands, !!unpack_status.len, &si, transaction,
&push_options);
odb_transaction_finalize(transaction);
- delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
report_v2(commands, &unpack_status);
diff --git a/object-file.c b/object-file.c
index db63587f6d..265c5f7a3c 100644
--- a/object-file.c
+++ b/object-file.c
@@ -10,6 +10,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "git-compat-util.h"
+#include "config.h"
#include "convert.h"
#include "dir.h"
#include "environment.h"
@@ -26,6 +27,7 @@
#include "packfile.h"
#include "path.h"
#include "read-cache-ll.h"
+#include "run-command.h"
#include "setup.h"
#include "strvec.h"
#include "tempfile.h"
@@ -483,11 +485,16 @@ struct transaction_packfile {
struct odb_transaction_files {
struct odb_transaction base;
+ enum odb_transaction_flags flags;
struct tmp_objdir *objdir;
struct odb_source *quarantine;
struct transaction_packfile packfile;
const char *prefix;
+
+ struct tempfile **pack_lockfiles;
+ size_t pack_lockfiles_nr;
+ size_t pack_lockfiles_alloc;
};
int odb_transaction_files_prepare(struct odb_transaction *base)
@@ -1291,6 +1298,174 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
return 0;
}
+static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
+{
+ switch (read_pack_header(pack_fd, hdr)) {
+ case PH_ERROR_EOF:
+ return "eof before pack header was fully read";
+
+ case PH_ERROR_PACK_SIGNATURE:
+ return "protocol error (pack signature mismatch detected)";
+
+ case PH_ERROR_PROTOCOL:
+ return "protocol error (pack version unsupported)";
+
+ default:
+ return "unknown error in parse_pack_header";
+
+ case 0:
+ return NULL;
+ }
+}
+
+static void push_header_arg(struct strvec *args, struct pack_header *hdr)
+{
+ strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
+ ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
+}
+
+static unsigned int get_unpack_limit(struct repository *repo,
+ enum odb_transaction_flags flags)
+{
+ unsigned int limit = 0;
+
+ if (flags & ODB_TRANSACTION_RECEIVE) {
+ limit = 100;
+ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
+ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
+ }
+
+ return limit;
+}
+
+static int odb_transaction_files_write_pack(struct odb_transaction *base,
+ int pack_fd, struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+ struct repository *repo = base->source->odb->repo;
+ struct child_process child = CHILD_PROCESS_INIT;
+ struct pack_header hdr;
+ const char *hdr_err;
+ int err_fd = opts->err_fd;
+ int status;
+
+ hdr_err = parse_pack_header(&hdr, pack_fd);
+ if (hdr_err) {
+ if (err_fd > 0)
+ close(err_fd);
+ strbuf_addstr(err_msg, hdr_err);
+ return -1;
+ }
+
+ if (opts->shallow_file) {
+ strvec_push(&child.args, "--shallow-file");
+ strvec_push(&child.args, opts->shallow_file);
+ }
+
+ odb_transaction_env(base, &child.env);
+
+ if (ntohl(hdr.hdr_entries) < get_unpack_limit(repo, transaction->flags)) {
+ strvec_push(&child.args, "unpack-objects");
+ push_header_arg(&child.args, &hdr);
+ if (opts->quiet)
+ strvec_push(&child.args, "-q");
+ if (opts->fsck_objects)
+ strvec_pushf(&child.args, "--strict%s",
+ opts->fsck_msg_types);
+ if (opts->max_input_size)
+ strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
+ (uintmax_t)opts->max_input_size);
+ child.no_stdout = 1;
+ child.in = pack_fd;
+ child.err = err_fd;
+ child.git_cmd = 1;
+ status = run_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "unpack-objects abnormal exit");
+ return -1;
+ }
+ } else {
+ char hostname[HOST_NAME_MAX + 1];
+ char *lockfile;
+
+ strvec_pushl(&child.args, "index-pack", "--stdin", NULL);
+ push_header_arg(&child.args, &hdr);
+
+ if (xgethostname(hostname, sizeof(hostname)))
+ xsnprintf(hostname, sizeof(hostname), "localhost");
+ strvec_pushf(&child.args,
+ "--keep=receive-pack %"PRIuMAX" on %s",
+ (uintmax_t)getpid(),
+ hostname);
+
+ if (!opts->quiet && err_fd)
+ strvec_push(&child.args, "--show-resolving-progress");
+ if (err_fd)
+ strvec_push(&child.args, "--report-end-of-input");
+ if (opts->fsck_objects)
+ strvec_pushf(&child.args, "--strict%s",
+ opts->fsck_msg_types);
+ if (!opts->reject_thin)
+ strvec_push(&child.args, "--fix-thin");
+ if (opts->max_input_size)
+ strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
+ (uintmax_t)opts->max_input_size);
+ child.out = -1;
+ child.in = pack_fd;
+ child.err = err_fd;
+ child.git_cmd = 1;
+ status = start_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack fork failed");
+ return -1;
+ }
+
+ /*
+ * The lockfile filepath is expected to be the final location of
+ * the ".keep" file after being migrated to the main ODB source.
+ * This ensures the lockfile can be found and removed later
+ * after the ODB transaction has been committed.
+ */
+ lockfile = index_pack_lockfile(base->source, child.out, NULL);
+ if (lockfile) {
+ ALLOC_GROW(transaction->pack_lockfiles,
+ transaction->pack_lockfiles_nr + 1,
+ transaction->pack_lockfiles_alloc);
+ transaction->pack_lockfiles[transaction->pack_lockfiles_nr++] =
+ register_tempfile(lockfile);
+ free(lockfile);
+ }
+ close(child.out);
+
+ status = finish_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack abnormal exit");
+ return -1;
+ }
+
+ odb_source_prepare(transaction->quarantine,
+ ODB_PREPARE_FLUSH_CACHES);
+ }
+
+ return 0;
+}
+
+static int odb_transaction_files_finalize(struct odb_transaction *base)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+ int ret = 0;
+
+ for (size_t i = 0; i < transaction->pack_lockfiles_nr; i++)
+ ret |= delete_tempfile(&transaction->pack_lockfiles[i]);
+
+ free(transaction->pack_lockfiles);
+
+ return ret;
+}
+
static int odb_transaction_files_env(struct odb_transaction *base,
struct strvec *env)
{
@@ -1314,8 +1489,11 @@ int odb_transaction_files_begin(struct odb_source *source,
transaction = xcalloc(1, sizeof(*transaction));
transaction->base.source = source;
transaction->base.commit = odb_transaction_files_commit;
+ transaction->base.finalize = odb_transaction_files_finalize;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+ transaction->base.write_pack = odb_transaction_files_write_pack;
transaction->base.env = odb_transaction_files_env;
+ transaction->flags = flags;
transaction->prefix = "bulk-fsync";
if (flags & ODB_TRANSACTION_RECEIVE) {
diff --git a/odb/transaction.c b/odb/transaction.c
index 9e9a982778..c9144e6cd6 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -59,6 +59,13 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
return transaction->write_object_stream(transaction, stream, len, oid);
}
+int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts)
+{
+ return transaction->write_pack(transaction, pack_fd, err_msg, opts);
+}
+
int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env)
{
if (!transaction)
diff --git a/odb/transaction.h b/odb/transaction.h
index 6ed39b3d0e..8cb06c1191 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -4,6 +4,50 @@
#include "gettext.h"
#include "odb.h"
+/*
+ * Options controlling how odb_transaction_write_pack() ingests a packfile.
+ */
+struct odb_transaction_write_pack_opts {
+ /*
+ * Optional fsck severity configuration to apply when incoming objects
+ * are verified.
+ */
+ const char *fsck_msg_types;
+
+ /*
+ * Path to an alternative shallow file describing the shallow boundaries
+ * to honor while ingesting the pack.
+ */
+ const char *shallow_file;
+
+ /*
+ * The max size in bytes of the incoming packfile allowed. No limit is
+ * enforced when set to 0.
+ */
+ off_t max_input_size;
+
+ /*
+ * Whether the validity of incoming objects should be verified.
+ */
+ int fsck_objects;
+
+ /*
+ * Whether to reject an incoming packfile if it is "thin".
+ */
+ int reject_thin;
+
+ /*
+ * Optional file descriptor for reporting progress and errors. Set to 0
+ * for none.
+ */
+ int err_fd;
+
+ /*
+ * Suppresses progress reporting.
+ */
+ int quiet;
+};
+
/*
* A transaction may be started for an object database prior to writing new
* objects via odb_transaction_begin(). These objects are not committed until
@@ -40,6 +84,15 @@ struct odb_transaction {
int (*write_object_stream)(struct odb_transaction *transaction,
struct odb_write_stream *stream, size_t len,
struct object_id *oid);
+ /*
+ * This callback is expected to ingest the packfile readable via
+ * `pack_fd` into the transaction. Returns 0 on success, a negative
+ * error code otherwise. On failure, a human-readable description is
+ * appended to `err_msg`.
+ */
+ int (*write_pack)(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts);
/*
* This callback is expected to populate the provided strvec with the
@@ -107,6 +160,15 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
struct odb_write_stream *stream,
size_t len, struct object_id *oid);
+/*
+ * Ingests the packfile readable via `pack_fd` into the transaction. Returns 0
+ * on success, a negative error code otherwise. On failure, a human-readable
+ * description is appended to `err_msg`.
+ */
+int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts);
+
/*
* Populates the provided strvec with the environment variables that a child
* process should inherit so that its object writes participate in the
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* Re: [PATCH v4 9/9] odb/transaction: add transaction interface to write packfiles
2026-08-19 21:53 ` [PATCH v4 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
@ 2026-08-20 6:46 ` Patrick Steinhardt
0 siblings, 0 replies; 78+ messages in thread
From: Patrick Steinhardt @ 2026-08-20 6:46 UTC (permalink / raw)
To: Justin Tobler; +Cc: git, gitster
On Wed, Aug 19, 2026 at 04:53:11PM -0500, Justin Tobler wrote:
> diff --git a/object-file.c b/object-file.c
> index db63587f6d..265c5f7a3c 100644
> --- a/object-file.c
> +++ b/object-file.c
[snip]
> +static unsigned int get_unpack_limit(struct repository *repo,
> + enum odb_transaction_flags flags)
> +{
> + unsigned int limit = 0;
> +
> + if (flags & ODB_TRANSACTION_RECEIVE) {
> + limit = 100;
> + repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
> + repo_config_get_uint(repo, "receive.unpacklimit", &limit);
> + }
> +
> + return limit;
> +}
Okay, instead of assuming that we're running in git-receive-pack(1) we
now pass this information along via the flags so that we can pick the
correct limit for the given operation. That's somewhat pointless right
now as no other operations use this infra yet, but the upside is that it
makes it obvious for how to extend the mechanism going forward.
Also, we no longer cache the value and the logic to derive it has become
a lot simpler. Good.
Patrick
^ permalink raw reply [flat|nested] 78+ messages in thread
* [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes
2026-08-19 21:53 ` [PATCH v4 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (8 preceding siblings ...)
2026-08-19 21:53 ` [PATCH v4 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
` (8 more replies)
9 siblings, 9 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
Greetings,
With bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10), git-receive-pack(1) started using the ODB
transaction interfaces to stage incoming objects. While this brought the
command closer to being ODB backend agnostic, the underlying
git-index-pack(1) and git-unpack-objects(1) processes used to actually
write the objects to the transaction are still fundamentally tied to the
"files" backend.
This series aims to address this by introducing a generic
`odb_transaction_write_pack()` transaction interface to handle writing
the incoming packfile to the transaction. The existing logic in
git-receive-pack(1) that spawns the child processes to write the
packfile becomes the "files" backend implementation of this interface.
Changes since V4:
- Added an additional test assertion in the frist patch to ensure keep
files are also migrated to the main ODB prior to being removed when
the transaction is finalized.
- Updated a commit message.
Chances since V3:
- In preparation for future `odb_transaction_write_pack()` users, the
unpack limit takes into consideration odb_transaction_flags to augment
configutation.
- Added additional test assertion in first patch to ensure keep file is
generated and placed in quarantine directory.
- Removed an include statement in favor of just forward declaring a
struct.
- Updated some commit messages.
Changes since V2:
- Added a patch to address a bug causing ".keep" files from not being
removed.
- Started handling errors at transaction commit and finalize call sites
instead of ignoring them. We also make sure
`odb_transaction_finalize()` runs after every successful commit
callsite to ensure proper cleanup.
- Updated the code handling lazy loading of unpack limit configuration
to not longer cache the value.
- Added a patch to begin explictly tracking the ODB source used by the
"files" transaction to avoid relying on the ordering of the ODB source
list.
- Updated some commit messages to improve clarity.
Changes since V1:
- Changed the "release" interface name to "finalize" and updated it to
return error codes.
- Marked some function parameters as const.
- Unpack limit configuration is now resolved in the ODB transaction
backend instead of wiring it through the interface.
- When writing a packfile to the transaction, now only the transaction
source is prepared.
- Updated some commit messages.
- Updated some code formatting.
Thanks for the review,
-Justin
Justin Tobler (9):
builtin/receive-pack: properly clean up keep files
odb/transaction: add transaction finalize interface
builtin/receive-pack: pass shallow file explicitly
builtin/receive-pack: read unpack limit config lazily
builtin/receive-pack: lift global state out of unpack()
builtin/receive-pack: report unpack errors via strbuf
builtin/receive-pack: explicitly pass packfile fd
odb: return temporary ODB source when set
odb/transaction: add transaction interface to write packfiles
builtin/add.c | 4 +-
builtin/receive-pack.c | 211 ++++++++-----------------------------
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 4 +-
cache-tree.c | 2 +-
fetch-pack.c | 2 +-
object-file.c | 183 +++++++++++++++++++++++++++++++-
odb.c | 9 +-
odb.h | 6 +-
odb/transaction.c | 21 ++++
odb/transaction.h | 85 +++++++++++++++
pack-write.c | 7 +-
pack.h | 4 +-
read-cache.c | 2 +-
t/t5547-push-quarantine.sh | 31 ++++++
tmp-objdir.c | 8 +-
tmp-objdir.h | 6 +-
17 files changed, 399 insertions(+), 188 deletions(-)
Range-diff against v4:
1: 13a57feea7 ! 1: 1bae015e8c builtin/receive-pack: properly clean up keep files
@@ t/t5547-push-quarantine.sh: test_expect_success 'updating a ref from quarantine
+
+ git -C keep.git config set receive.unpackLimit 0 &&
+
-+ # While incoming objects are still quarantined, validate that the keep
-+ # lockfile does indeed exist.
++ # While incoming objects are still quarantined, validate that the
++ # ".keep" lockfile is present in the quarantine directory.
+ test_hook -C keep.git pre-receive <<-\EOF &&
+ keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" &&
+ test -f "$keep"
+ EOF
+
++ # After quarantined objects are migrated, validate that the ".keep"
++ # lockfile is migrated and present in the main ODB.
++ test_hook -C keep.git reference-transaction <<-\EOF &&
++ keep="$(ls objects/pack/pack-*.keep)" &&
++ test -f "$keep"
++ EOF
++
+ test_commit foo &&
+ git push keep.git HEAD &&
++
++ # Once the operation is complete, validate that the ".keep" lockfile has
++ # been removed.
+ pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
+ keep="${pack%.pack}.keep" &&
-+
+ test_path_is_file "$pack" &&
+ test_path_is_missing "$keep"
+'
2: 49254af71c ! 2: a2a10966a8 odb/transaction: add transaction finalize interface
@@ Commit message
`odb_transaction_finalize()` call site in git-receive-pack(1) is made
after the reference updates are finished.
- All other callers commit a transaction and immediately finalize it with
- no work in between and cannot meaningfully recover should either fail,
- so introduce an `odb_transaction_commit_and_finalize_or_die()` helper
- that performs both and dies on error. Call sites are updated
- accordingly.
+ All other callers commit a transaction and immediately finalize it
+ without any work happening in between those two operations.
+ Consequently, they cannot meaningfully recover in case either of them
+ would fail, and spelling out these two separate steps with proper error
+ handling would be quite repetitive and pointless. Introduce a helper
+ `odb_transaction_commit_and_finalize_or_die()` for those call sites and
+ update them accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
3: 882cf06bc3 = 3: 063b1830a1 builtin/receive-pack: pass shallow file explicitly
4: 8deec37a09 = 4: 04c42ebefd builtin/receive-pack: read unpack limit config lazily
5: 92d56134f0 = 5: f4a633a212 builtin/receive-pack: lift global state out of unpack()
6: d614b10715 = 6: 9b89af0bd8 builtin/receive-pack: report unpack errors via strbuf
7: bc5839ad8e = 7: edb54e79f6 builtin/receive-pack: explicitly pass packfile fd
8: 13540b91b8 = 8: 452affa42f odb: return temporary ODB source when set
9: 62d46d5c07 = 9: dae4b96bc3 odb/transaction: add transaction interface to write packfiles
base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
--
2.55.0.424.g13c7afec21
^ permalink raw reply [flat|nested] 78+ messages in thread* [PATCH v5 1/9] builtin/receive-pack: properly clean up keep files
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 2/9] odb/transaction: add transaction finalize interface Justin Tobler
` (7 subsequent siblings)
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When git-receive-pack(1) stores an incoming packfile with
git-index-pack(1), a ".keep" file is written alongside it in the
transaction quarantine directory and also gets migrated to the main ODB
when the ODB transaction is committed. This keep lockfile ensures the
packfile remains in place until the references have been updated and is
removed afterwards. The path used to remove it is derived via
`index_pack_lockfile()` from the repository's primary object directory.
In bdee7b3013 (builtin/receive-pack: stage incoming objects via ODB
transactions, 2026-07-10), git-receive-pack(1) started using the ODB
transaction interfaces instead of managing a temporary directory
directly. When starting an ODB transaction, the sources list is
reordered to insert the newly created transaction source first as the
primary to ensure writes are routed to it accordingly.
Prior to using ODB transactions, git-receive-pack(1) would only set the
temporary directory as the primary source for the child
git-index-pack(1) and git-unpack-objects(1) processes it spawned and the
parent process would set the temporary directory set as an alternate
only. By using ODB transactions, the ODB source list is also reordered
for the parent process which results in `index_pack_lockfile()` deriving
the ".keep" path relative to the temporary directory instead of the
actual main ODB source path. Consequently, this prevents the ".keep"
file from being properly removed after being migrated into the main ODB
source post-commit.
Update `index_pack_lockfile()` to operate on an ODB source explicitly
provided to it and update call sites accordingly to pass the expected
ODB source.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 8 +++++++-
fetch-pack.c | 2 +-
pack-write.c | 7 ++++---
pack.h | 4 +++-
t/t5547-push-quarantine.sh | 31 +++++++++++++++++++++++++++++++
5 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 86933d8d7e..d74b787148 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2412,7 +2412,13 @@ static const char *unpack(int err_fd, struct shallow_info *si,
if (status)
return "index-pack fork failed";
- lockfile = index_pack_lockfile(the_repository, child.out, NULL);
+ /*
+ * The lockfile filepath is expected to be the final location of
+ * the ".keep" file after being migrated to the main ODB source.
+ * This ensures the lockfile can be found and removed later
+ * after the ODB transaction has been committed.
+ */
+ lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
if (lockfile) {
pack_lockfile = register_tempfile(lockfile);
free(lockfile);
diff --git a/fetch-pack.c b/fetch-pack.c
index 922a9b2581..6df5813b33 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1075,7 +1075,7 @@ static int get_pack(struct fetch_pack_args *args,
die(_("fetch-pack: unable to fork off %s"), cmd_name);
if (do_keep && (pack_lockfiles || fsck_objects)) {
int is_well_formed;
- char *pack_lockfile = index_pack_lockfile(the_repository,
+ char *pack_lockfile = index_pack_lockfile(the_repository->objects->sources,
cmd.out,
&is_well_formed);
diff --git a/pack-write.c b/pack-write.c
index 24033a9101..85674e4b72 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -469,10 +469,11 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,
fsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);
}
-char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)
+char *index_pack_lockfile(struct odb_source *source, int ip_out,
+ int *is_well_formed)
{
char packname[GIT_MAX_HEXSZ + 6];
- const int len = r->hash_algo->hexsz + 6;
+ const int len = source->odb->repo->hash_algo->hexsz + 6;
/*
* The first thing we expect from index-pack's output
@@ -489,7 +490,7 @@ char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)
packname[len-1] = 0;
if (skip_prefix(packname, "keep\t", &name))
return xstrfmt("%s/pack/pack-%s.keep",
- repo_get_object_directory(r), name);
+ source->path, name);
return NULL;
}
if (is_well_formed)
diff --git a/pack.h b/pack.h
index 1cde92082b..ada506b5c5 100644
--- a/pack.h
+++ b/pack.h
@@ -7,6 +7,7 @@
struct packed_git;
struct pack_window;
struct repository;
+struct odb_source;
/*
* Packed object header
@@ -105,7 +106,8 @@ off_t write_pack_header(struct hashfile *f, uint32_t);
void fixup_pack_header_footer(const struct git_hash_algo *, int,
unsigned char *, const char *, uint32_t,
unsigned char *, off_t);
-char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed);
+char *index_pack_lockfile(struct odb_source *source, int fd,
+ int *is_well_formed);
struct ref;
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh
index 0798ddab02..1b7097179e 100755
--- a/t/t5547-push-quarantine.sh
+++ b/t/t5547-push-quarantine.sh
@@ -70,4 +70,35 @@ test_expect_success 'updating a ref from quarantine is forbidden' '
git -C update.git fsck
'
+test_expect_success '.keep file is removed after push' '
+ test_when_finished rm -rf keep.git &&
+ git init --bare keep.git &&
+
+ git -C keep.git config set receive.unpackLimit 0 &&
+
+ # While incoming objects are still quarantined, validate that the
+ # ".keep" lockfile is present in the quarantine directory.
+ test_hook -C keep.git pre-receive <<-\EOF &&
+ keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" &&
+ test -f "$keep"
+ EOF
+
+ # After quarantined objects are migrated, validate that the ".keep"
+ # lockfile is migrated and present in the main ODB.
+ test_hook -C keep.git reference-transaction <<-\EOF &&
+ keep="$(ls objects/pack/pack-*.keep)" &&
+ test -f "$keep"
+ EOF
+
+ test_commit foo &&
+ git push keep.git HEAD &&
+
+ # Once the operation is complete, validate that the ".keep" lockfile has
+ # been removed.
+ pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
+ keep="${pack%.pack}.keep" &&
+ test_path_is_file "$pack" &&
+ test_path_is_missing "$keep"
+'
+
test_done
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 2/9] odb/transaction: add transaction finalize interface
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-20 23:49 ` [PATCH v5 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 3/9] builtin/receive-pack: pass shallow file explicitly Justin Tobler
` (6 subsequent siblings)
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When committing an ODB transaction via `odb_transaction_commit()`, the
staged objects are made visible and the underlying transaction is freed
at the same time. Coupling these two steps does not leave room for any
post-commit transaction operations to be introduced though. Such a
capability is useful if an ODB transaction backend needs to hold on to
lockfiles after transaction commit until references are updated, as is
the case with the existing "files" backend in git-receive-pack(1).
Stop freeing the transaction in `odb_transaction_commit()` and introduce
`odb_transaction_finalize()` to explicitly clean up the transaction
accordingly. Note that the finalize interface also provides an optional
callback for any backend-specific deferred cleanup. In a subsequent
commit, the "files" transaction backend will use this to remove ".keep"
files generated for packfiles received via git-receive-pack(1) after
references have been updated. In preparation for this, the
`odb_transaction_finalize()` call site in git-receive-pack(1) is made
after the reference updates are finished.
All other callers commit a transaction and immediately finalize it
without any work happening in between those two operations.
Consequently, they cannot meaningfully recover in case either of them
would fail, and spelling out these two separate steps with proper error
handling would be quite repetitive and pointless. Introduce a helper
`odb_transaction_commit_and_finalize_or_die()` for those call sites and
update them accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/add.c | 4 ++--
builtin/receive-pack.c | 1 +
builtin/unpack-objects.c | 2 +-
builtin/update-index.c | 4 ++--
cache-tree.c | 2 +-
object-file.c | 2 +-
odb/transaction.c | 14 ++++++++++++++
odb/transaction.h | 23 +++++++++++++++++++++++
read-cache.c | 2 +-
9 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index 60ffbede2b..ad418a5952 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -393,7 +393,7 @@ int cmd_add(int argc,
char *seen = NULL;
char *ps_matched = NULL;
struct lock_file lock_file = LOCK_INIT;
- struct odb_transaction *transaction;
+ struct odb_transaction *transaction = NULL;
repo_config(repo, add_config, NULL);
@@ -600,7 +600,7 @@ int cmd_add(int argc,
if (chmod_arg && pathspec.nr)
exit_status |= chmod_pathspec(repo, &pathspec, chmod_arg[0], show_only);
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
finish:
if (write_locked_index(repo->index, &lock_file,
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d74b787148..ed1edcbe93 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2720,6 +2720,7 @@ int cmd_receive_pack(int argc,
use_keepalive = KEEPALIVE_ALWAYS;
execute_commands(commands, unpack_status, &si, transaction,
&push_options);
+ odb_transaction_finalize(transaction);
delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 4263edfbec..d6a2d616d9 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -603,7 +603,7 @@ static void unpack_all(void)
unpack_one(i);
display_progress(progress, i + 1);
}
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
stop_progress(&progress);
if (delta_list)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 241abd4332..b25d4ecb10 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1156,7 +1156,7 @@ int cmd_update_index(int argc,
* a transaction.
*/
if (transaction && verbose) {
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
transaction = NULL;
}
@@ -1224,7 +1224,7 @@ int cmd_update_index(int argc,
/*
* By now we have added all of the new objects
*/
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
if (split_index > 0) {
if (repo_config_get_split_index(the_repository) == 0)
diff --git a/cache-tree.c b/cache-tree.c
index d92f513286..a220372a42 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -538,7 +538,7 @@ int cache_tree_update(struct index_state *istate, int flags)
i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
"", 0, &skip, flags);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
trace2_region_leave("cache_tree", "update", istate->repo);
trace_performance_leave("cache_tree_update");
if (i < 0)
diff --git a/object-file.c b/object-file.c
index ec35c318bc..4d03c167d5 100644
--- a/object-file.c
+++ b/object-file.c
@@ -965,7 +965,7 @@ int index_fd(struct index_state *istate, struct object_id *oid,
xsize_t(st->st_size),
oid);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
} else {
ret = hash_blob_stream(&stream,
the_repository->hash_algo, oid,
diff --git a/odb/transaction.c b/odb/transaction.c
index dab7da6a9a..9e9a982778 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -33,6 +33,20 @@ int odb_transaction_commit(struct odb_transaction *transaction)
ret = transaction->commit(transaction);
transaction->source->odb->transaction = NULL;
+
+ return ret;
+}
+
+int odb_transaction_finalize(struct odb_transaction *transaction)
+{
+ int ret = 0;
+
+ if (!transaction)
+ return 0;
+
+ if (transaction->finalize)
+ ret = transaction->finalize(transaction);
+
free(transaction);
return ret;
diff --git a/odb/transaction.h b/odb/transaction.h
index 4cb2eafcbf..6ed39b3d0e 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -22,6 +22,13 @@ struct odb_transaction {
*/
int (*commit)(struct odb_transaction *transaction);
+ /*
+ * Optional ODB source specific callback invoked when the transaction
+ * needs to perform any deferred cleanup after objects have been
+ * committed. Returns 0 on success, a negative error code otherwise.
+ */
+ int (*finalize)(struct odb_transaction *transaction);
+
/*
* This callback is expected to write the given object stream into
* the ODB transaction. Note that for now, only blobs support streaming.
@@ -75,6 +82,22 @@ static inline void odb_transaction_begin_or_die(struct object_database *odb,
*/
int odb_transaction_commit(struct odb_transaction *transaction);
+/*
+ * Finalizes an ODB transaction, performing any deferred cleanup and freeing it.
+ * Must be called for every successfully started transaction. Note that, if the
+ * specified transaction is NULL, the function is a no-op. Returns 0 on success,
+ * a negative error code otherwise.
+ */
+int odb_transaction_finalize(struct odb_transaction *transaction);
+
+static inline void odb_transaction_commit_and_finalize_or_die(struct odb_transaction *transaction)
+{
+ if (odb_transaction_commit(transaction))
+ die(_("failed to commit ODB transaction"));
+ if (odb_transaction_finalize(transaction))
+ die(_("failed to finalize ODB transaction"));
+}
+
/*
* Writes the object in the provided stream into the transaction. The resulting
* object ID is written into the out pointer. Returns 0 on success, a negative
diff --git a/read-cache.c b/read-cache.c
index 6c449f393d..0cd0ef85ec 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4049,7 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
odb_transaction_begin_or_die(repo->objects, &transaction, 0);
run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
if (!inflight)
- odb_transaction_commit(transaction);
+ odb_transaction_commit_and_finalize_or_die(transaction);
release_revisions(&rev);
return !!data.add_errors;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 3/9] builtin/receive-pack: pass shallow file explicitly
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
2026-08-20 23:49 ` [PATCH v5 1/9] builtin/receive-pack: properly clean up keep files Justin Tobler
2026-08-20 23:49 ` [PATCH v5 2/9] odb/transaction: add transaction finalize interface Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 4/9] builtin/receive-pack: read unpack limit config lazily Justin Tobler
` (5 subsequent siblings)
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
If shallow information is provided during `unpack()`, a temporary
shallow file is created and stored in global state. In a subsequent
commit, the `unpack()` logic is moved behind a generic ODB transaction
interface to handle writing packfiles and thus can no longer rely on
such global state. Lift the setup of the temporary shallow file out of
`unpack()` and wire it through to its call sites explicitly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ed1edcbe93..135105deae 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -86,7 +86,6 @@ static const char *head_name;
static void *head_name_to_free;
static int sent_capabilities;
static int shallow_update;
-static const char *alt_shallow_file;
static struct strbuf push_cert = STRBUF_INIT;
static struct object_id push_cert_oid;
static struct signature_check sigcheck;
@@ -2334,8 +2333,8 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
-static const char *unpack(int err_fd, struct shallow_info *si,
- struct odb_transaction *transaction)
+static const char *unpack(struct odb_transaction *transaction,
+ const char *shallow_file, int err_fd)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2354,10 +2353,9 @@ static const char *unpack(int err_fd, struct shallow_info *si,
return hdr_err;
}
- if (si->nr_ours || si->nr_theirs) {
- alt_shallow_file = setup_temporary_shallow(si->shallow);
+ if (shallow_file) {
strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, alt_shallow_file);
+ strvec_push(&child.args, shallow_file);
}
odb_transaction_env(transaction, &child.env);
@@ -2433,14 +2431,14 @@ static const char *unpack(int err_fd, struct shallow_info *si,
return NULL;
}
-static const char *unpack_with_sideband(struct shallow_info *si,
- struct odb_transaction *transaction)
+static const char *unpack_with_sideband(struct odb_transaction *transaction,
+ const char *shallow_file)
{
struct async muxer;
const char *ret;
if (!use_sideband)
- return unpack(0, si, transaction);
+ return unpack(transaction, shallow_file, 0);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2449,13 +2447,14 @@ static const char *unpack_with_sideband(struct shallow_info *si,
if (start_async(&muxer))
return NULL;
- ret = unpack(muxer.in, si, transaction);
+ ret = unpack(transaction, shallow_file, muxer.in);
finish_async(&muxer);
return ret;
}
-static void prepare_shallow_update(struct shallow_info *si)
+static void prepare_shallow_update(struct shallow_info *si,
+ const char *shallow_file)
{
int i, j, k, bitmap_size = DIV_ROUND_UP(si->ref->nr, 32);
@@ -2495,12 +2494,13 @@ static void prepare_shallow_update(struct shallow_info *si)
* command. check_connected() will be done with
* true .git/shallow though.
*/
- setenv(GIT_SHALLOW_FILE_ENVIRONMENT, alt_shallow_file, 1);
+ setenv(GIT_SHALLOW_FILE_ENVIRONMENT, shallow_file, 1);
}
static void update_shallow_info(struct command *commands,
struct shallow_info *si,
- struct oid_array *ref)
+ struct oid_array *ref,
+ const char *shallow_file)
{
struct command *cmd;
int *ref_status;
@@ -2519,7 +2519,7 @@ static void update_shallow_info(struct command *commands,
si->ref = ref;
if (shallow_update) {
- prepare_shallow_update(si);
+ prepare_shallow_update(si, shallow_file);
return;
}
@@ -2711,11 +2711,17 @@ int cmd_receive_pack(int argc,
if (!si.nr_ours && !si.nr_theirs)
shallow_update = 0;
if (!delete_only(commands)) {
+ const char *alt_shallow_file = NULL;
+
+ if (si.nr_ours || si.nr_theirs)
+ alt_shallow_file = setup_temporary_shallow(si.shallow);
+
if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
unpack_status = "unable to start object transaction";
else
- unpack_status = unpack_with_sideband(&si, transaction);
- update_shallow_info(commands, &si, &ref);
+ unpack_status = unpack_with_sideband(transaction, alt_shallow_file);
+
+ update_shallow_info(commands, &si, &ref, alt_shallow_file);
}
use_keepalive = KEEPALIVE_ALWAYS;
execute_commands(commands, unpack_status, &si, transaction,
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 4/9] builtin/receive-pack: read unpack limit config lazily
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (2 preceding siblings ...)
2026-08-20 23:49 ` [PATCH v5 3/9] builtin/receive-pack: pass shallow file explicitly Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 5/9] builtin/receive-pack: lift global state out of unpack() Justin Tobler
` (4 subsequent siblings)
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), the `receive.unpackLimit` and
`transfer.unpackLimit` configuration decides whether an incoming
packfile should be exploded into loose objects or kept as a packfile
on-disk. In a subsequent commit, the logic to write the incoming
packfile is made ODB backend agnostic and moved behind a pluggable ODB
transaction interface. Consequently, whether to explode a packfile is a
detail of how a particular backend stores objects and should not be a
part of the generic interface itself.
In preparation for this, instead resolve the unpack limit lazily inside
`unpack()` by reading the configuration directly. The now-unused unpack
limit globals are dropped accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 135105deae..971dc3f52e 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -62,12 +62,9 @@ static enum deny_action deny_delete_current = DENY_UNCONFIGURED;
static int receive_fsck_objects = -1;
static int transfer_fsck_objects = -1;
static struct strbuf fsck_msg_types = STRBUF_INIT;
-static int receive_unpack_limit = -1;
-static int transfer_unpack_limit = -1;
static int advertise_atomic_push = 1;
static int advertise_push_options;
static int advertise_sid;
-static int unpack_limit = 100;
static off_t max_input_size;
static int report_status;
static int report_status_v2;
@@ -157,16 +154,6 @@ static int receive_pack_config(const char *var, const char *value,
return 0;
}
- if (strcmp(var, "receive.unpacklimit") == 0) {
- receive_unpack_limit = git_config_int(var, value, ctx->kvi);
- return 0;
- }
-
- if (strcmp(var, "transfer.unpacklimit") == 0) {
- transfer_unpack_limit = git_config_int(var, value, ctx->kvi);
- return 0;
- }
-
if (strcmp(var, "receive.fsck.skiplist") == 0) {
char *path;
@@ -2333,6 +2320,16 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
}
+static unsigned int get_unpack_limit(struct repository *repo)
+{
+ unsigned int limit = 100;
+
+ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
+ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
+
+ return limit;
+}
+
static const char *unpack(struct odb_transaction *transaction,
const char *shallow_file, int err_fd)
{
@@ -2360,7 +2357,7 @@ static const char *unpack(struct odb_transaction *transaction,
odb_transaction_env(transaction, &child.env);
- if (ntohl(hdr.hdr_entries) < unpack_limit) {
+ if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
strvec_push(&child.args, "unpack-objects");
push_header_arg(&child.args, &hdr);
if (quiet)
@@ -2658,11 +2655,6 @@ int cmd_receive_pack(int argc,
if (cert_nonce_seed)
push_cert_nonce = prepare_push_cert_nonce(service_dir, time(NULL));
- if (0 <= receive_unpack_limit)
- unpack_limit = receive_unpack_limit;
- else if (0 <= transfer_unpack_limit)
- unpack_limit = transfer_unpack_limit;
-
switch (determine_protocol_version_server()) {
case protocol_v2:
/*
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 5/9] builtin/receive-pack: lift global state out of unpack()
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (3 preceding siblings ...)
2026-08-20 23:49 ` [PATCH v5 4/9] builtin/receive-pack: read unpack limit config lazily Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 6/9] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
` (3 subsequent siblings)
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), writing the packfile to the transaction is
handled via `unpack()` which relies on global variables to decide how to
invoke the underlying git-index-pack(1) or git-unpack-objects(1) child
processes. In a subsequent commit, the `unpack()` logic is moved behind
a generic ODB transaction interface to handle writing packfiles and thus
can no longer rely on these globals.
Lift the global state out of `unpack()` by instead storing this state in
a `struct unpack_opts` that gets passed to the function explicitly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 63 +++++++++++++++++++++++++++---------------
1 file changed, 41 insertions(+), 22 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 971dc3f52e..f062b93b8d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2330,18 +2330,24 @@ static unsigned int get_unpack_limit(struct repository *repo)
return limit;
}
+struct unpack_opts {
+ const char *fsck_msg_types;
+ const char *shallow_file;
+ off_t max_input_size;
+ int fsck_objects;
+ int reject_thin;
+ int err_fd;
+ int quiet;
+};
+
static const char *unpack(struct odb_transaction *transaction,
- const char *shallow_file, int err_fd)
+ const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
int status;
struct child_process child = CHILD_PROCESS_INIT;
- int fsck_objects = (receive_fsck_objects >= 0
- ? receive_fsck_objects
- : transfer_fsck_objects >= 0
- ? transfer_fsck_objects
- : 0);
+ int err_fd = opts->err_fd;
hdr_err = parse_pack_header(&hdr);
if (hdr_err) {
@@ -2350,9 +2356,9 @@ static const char *unpack(struct odb_transaction *transaction,
return hdr_err;
}
- if (shallow_file) {
+ if (opts->shallow_file) {
strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, shallow_file);
+ strvec_push(&child.args, opts->shallow_file);
}
odb_transaction_env(transaction, &child.env);
@@ -2360,14 +2366,14 @@ static const char *unpack(struct odb_transaction *transaction,
if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
strvec_push(&child.args, "unpack-objects");
push_header_arg(&child.args, &hdr);
- if (quiet)
+ if (opts->quiet)
strvec_push(&child.args, "-q");
- if (fsck_objects)
+ if (opts->fsck_objects)
strvec_pushf(&child.args, "--strict%s",
- fsck_msg_types.buf);
- if (max_input_size)
+ opts->fsck_msg_types);
+ if (opts->max_input_size)
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)max_input_size);
+ (uintmax_t)opts->max_input_size);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -2388,18 +2394,18 @@ static const char *unpack(struct odb_transaction *transaction,
(uintmax_t)getpid(),
hostname);
- if (!quiet && err_fd)
+ if (!opts->quiet && err_fd)
strvec_push(&child.args, "--show-resolving-progress");
- if (use_sideband)
+ if (err_fd)
strvec_push(&child.args, "--report-end-of-input");
- if (fsck_objects)
+ if (opts->fsck_objects)
strvec_pushf(&child.args, "--strict%s",
- fsck_msg_types.buf);
- if (!reject_thin)
+ opts->fsck_msg_types);
+ if (!opts->reject_thin)
strvec_push(&child.args, "--fix-thin");
- if (max_input_size)
+ if (opts->max_input_size)
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)max_input_size);
+ (uintmax_t)opts->max_input_size);
child.out = -1;
child.err = err_fd;
child.git_cmd = 1;
@@ -2431,11 +2437,23 @@ static const char *unpack(struct odb_transaction *transaction,
static const char *unpack_with_sideband(struct odb_transaction *transaction,
const char *shallow_file)
{
+ struct unpack_opts opts = {
+ .fsck_objects = (receive_fsck_objects >= 0
+ ? receive_fsck_objects
+ : transfer_fsck_objects >= 0
+ ? transfer_fsck_objects
+ : 0),
+ .fsck_msg_types = fsck_msg_types.buf,
+ .max_input_size = max_input_size,
+ .shallow_file = shallow_file,
+ .reject_thin = reject_thin,
+ .quiet = quiet,
+ };
struct async muxer;
const char *ret;
if (!use_sideband)
- return unpack(transaction, shallow_file, 0);
+ return unpack(transaction, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2444,7 +2462,8 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction,
if (start_async(&muxer))
return NULL;
- ret = unpack(transaction, shallow_file, muxer.in);
+ opts.err_fd = muxer.in;
+ ret = unpack(transaction, &opts);
finish_async(&muxer);
return ret;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 6/9] builtin/receive-pack: report unpack errors via strbuf
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (4 preceding siblings ...)
2026-08-20 23:49 ` [PATCH v5 5/9] builtin/receive-pack: lift global state out of unpack() Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 7/9] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
` (2 subsequent siblings)
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When writing packfiles via `unpack()`, error messages are returned
directly by the function. In preparation for `unpack()` logic being
moved behind a generic ODB transaction interface, update the function to
instead write any error messages to a caller provided strbuf and return
a negative value on error. Call sites are updated to use the error
strbuf accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 63 ++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f062b93b8d..6df872697b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2015,7 +2015,7 @@ static void execute_commands_atomic(struct command *commands,
}
static void execute_commands(struct command *commands,
- const char *unpacker_error,
+ int unpacker_error,
struct shallow_info *si,
struct odb_transaction *transaction,
const struct string_list *push_options)
@@ -2340,8 +2340,8 @@ struct unpack_opts {
int quiet;
};
-static const char *unpack(struct odb_transaction *transaction,
- const struct unpack_opts *opts)
+static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
+ const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2353,7 +2353,8 @@ static const char *unpack(struct odb_transaction *transaction,
if (hdr_err) {
if (err_fd > 0)
close(err_fd);
- return hdr_err;
+ strbuf_addstr(err_msg, hdr_err);
+ return -1;
}
if (opts->shallow_file) {
@@ -2378,8 +2379,10 @@ static const char *unpack(struct odb_transaction *transaction,
child.err = err_fd;
child.git_cmd = 1;
status = run_command(&child);
- if (status)
- return "unpack-objects abnormal exit";
+ if (status) {
+ strbuf_addstr(err_msg, "unpack-objects abnormal exit");
+ return -1;
+ }
} else {
char hostname[HOST_NAME_MAX + 1];
char *lockfile;
@@ -2410,8 +2413,10 @@ static const char *unpack(struct odb_transaction *transaction,
child.err = err_fd;
child.git_cmd = 1;
status = start_command(&child);
- if (status)
- return "index-pack fork failed";
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack fork failed");
+ return -1;
+ }
/*
* The lockfile filepath is expected to be the final location of
@@ -2427,15 +2432,18 @@ static const char *unpack(struct odb_transaction *transaction,
close(child.out);
status = finish_command(&child);
- if (status)
- return "index-pack abnormal exit";
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack abnormal exit");
+ return -1;
+ }
odb_reprepare(the_repository->objects);
}
- return NULL;
+ return 0;
}
-static const char *unpack_with_sideband(struct odb_transaction *transaction,
- const char *shallow_file)
+static int unpack_with_sideband(struct odb_transaction *transaction,
+ const char *shallow_file,
+ struct strbuf *err_msg)
{
struct unpack_opts opts = {
.fsck_objects = (receive_fsck_objects >= 0
@@ -2450,20 +2458,20 @@ static const char *unpack_with_sideband(struct odb_transaction *transaction,
.quiet = quiet,
};
struct async muxer;
- const char *ret;
+ int ret;
if (!use_sideband)
- return unpack(transaction, &opts);
+ return unpack(transaction, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
muxer.proc = copy_to_sideband;
muxer.in = -1;
if (start_async(&muxer))
- return NULL;
+ return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, &opts);
+ ret = unpack(transaction, err_msg, &opts);
finish_async(&muxer);
return ret;
@@ -2552,13 +2560,13 @@ static void update_shallow_info(struct command *commands,
free(ref_status);
}
-static void report(struct command *commands, const char *unpack_status)
+static void report(struct command *commands, const struct strbuf *unpack_status)
{
struct command *cmd;
struct strbuf buf = STRBUF_INIT;
packet_buf_write(&buf, "unpack %s\n",
- unpack_status ? unpack_status : "ok");
+ unpack_status->len ? unpack_status->buf : "ok");
for (cmd = commands; cmd; cmd = cmd->next) {
if (!cmd->error_string)
packet_buf_write(&buf, "ok %s\n",
@@ -2576,14 +2584,14 @@ static void report(struct command *commands, const char *unpack_status)
strbuf_release(&buf);
}
-static void report_v2(struct command *commands, const char *unpack_status)
+static void report_v2(struct command *commands, const struct strbuf *unpack_status)
{
struct command *cmd;
struct strbuf buf = STRBUF_INIT;
struct ref_push_report *report;
packet_buf_write(&buf, "unpack %s\n",
- unpack_status ? unpack_status : "ok");
+ unpack_status->len ? unpack_status->buf : "ok");
for (cmd = commands; cmd; cmd = cmd->next) {
int count = 0;
@@ -2707,8 +2715,8 @@ int cmd_receive_pack(int argc,
PACKET_READ_DIE_ON_ERR_PACKET);
if ((commands = read_head_info(&reader, &shallow))) {
- const char *unpack_status = NULL;
struct string_list push_options = STRING_LIST_INIT_DUP;
+ struct strbuf unpack_status = STRBUF_INIT;
if (use_push_options)
read_push_options(&reader, &push_options);
@@ -2728,22 +2736,22 @@ int cmd_receive_pack(int argc,
alt_shallow_file = setup_temporary_shallow(si.shallow);
if (odb_transaction_begin(the_repository->objects, &transaction, ODB_TRANSACTION_RECEIVE))
- unpack_status = "unable to start object transaction";
+ strbuf_addstr(&unpack_status, "unable to start object transaction");
else
- unpack_status = unpack_with_sideband(transaction, alt_shallow_file);
+ unpack_with_sideband(transaction, alt_shallow_file, &unpack_status);
update_shallow_info(commands, &si, &ref, alt_shallow_file);
}
use_keepalive = KEEPALIVE_ALWAYS;
- execute_commands(commands, unpack_status, &si, transaction,
+ execute_commands(commands, !!unpack_status.len, &si, transaction,
&push_options);
odb_transaction_finalize(transaction);
delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
- report_v2(commands, unpack_status);
+ report_v2(commands, &unpack_status);
else if (report_status)
- report(commands, unpack_status);
+ report(commands, &unpack_status);
sigchain_pop(SIGPIPE);
run_receive_hook(commands, "post-receive", 1, NULL,
&push_options);
@@ -2768,6 +2776,7 @@ int cmd_receive_pack(int argc,
if (auto_update_server_info)
update_server_info(the_repository, 0);
clear_shallow_info(&si);
+ strbuf_release(&unpack_status);
}
if (use_sideband)
packet_flush(1);
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 7/9] builtin/receive-pack: explicitly pass packfile fd
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (5 preceding siblings ...)
2026-08-20 23:49 ` [PATCH v5 6/9] builtin/receive-pack: report unpack errors via strbuf Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 8/9] odb: return temporary ODB source when set Justin Tobler
2026-08-20 23:49 ` [PATCH v5 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When processing the incoming packfile in git-receive-pack(1), `unpack()`
assumes it should always read it from stdin. In preparation for
`unpack()` logic being moved behind a generic ODB transaction interface,
update the function signature to take the an explicit fd provided by
callers to read the incoming packfile from instead. Call sites are
updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6df872697b..b369466783 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2292,9 +2292,9 @@ static void read_push_options(struct packet_reader *reader,
}
}
-static const char *parse_pack_header(struct pack_header *hdr)
+static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
{
- switch (read_pack_header(0, hdr)) {
+ switch (read_pack_header(pack_fd, hdr)) {
case PH_ERROR_EOF:
return "eof before pack header was fully read";
@@ -2340,8 +2340,8 @@ struct unpack_opts {
int quiet;
};
-static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
- const struct unpack_opts *opts)
+static int unpack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg, const struct unpack_opts *opts)
{
struct pack_header hdr;
const char *hdr_err;
@@ -2349,7 +2349,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
struct child_process child = CHILD_PROCESS_INIT;
int err_fd = opts->err_fd;
- hdr_err = parse_pack_header(&hdr);
+ hdr_err = parse_pack_header(&hdr, pack_fd);
if (hdr_err) {
if (err_fd > 0)
close(err_fd);
@@ -2376,6 +2376,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
(uintmax_t)opts->max_input_size);
child.no_stdout = 1;
+ child.in = pack_fd;
child.err = err_fd;
child.git_cmd = 1;
status = run_command(&child);
@@ -2410,6 +2411,7 @@ static int unpack(struct odb_transaction *transaction, struct strbuf *err_msg,
strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
(uintmax_t)opts->max_input_size);
child.out = -1;
+ child.in = pack_fd;
child.err = err_fd;
child.git_cmd = 1;
status = start_command(&child);
@@ -2461,7 +2463,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
int ret;
if (!use_sideband)
- return unpack(transaction, err_msg, &opts);
+ return unpack(transaction, 0, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2471,7 +2473,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, err_msg, &opts);
+ ret = unpack(transaction, 0, err_msg, &opts);
finish_async(&muxer);
return ret;
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 8/9] odb: return temporary ODB source when set
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (6 preceding siblings ...)
2026-08-20 23:49 ` [PATCH v5 7/9] builtin/receive-pack: explicitly pass packfile fd Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
2026-08-20 23:49 ` [PATCH v5 9/9] odb/transaction: add transaction interface to write packfiles Justin Tobler
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
When invoked, `odb_set_temporary_primary_source()` installs a temporary
object directory as the new primary ODB source. A caller that wants to
operate on the ODB source of the open transaction must assume that it is
the first entry in the ODB source list which is a bit awkward and
fragile.
Instead, return the newly installed source directly and report the
previous primary source via a new `prev_source` out parameter. Propagate
the installed source through `tmp_objdir_replace_primary_odb()` and
start storing it in the "files" ODB transaction so a subsequent commit
can easily access it without relying on the ODB source list ordering.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
object-file.c | 3 ++-
odb.c | 9 +++++++--
odb.h | 6 ++++--
tmp-objdir.c | 8 +++++---
tmp-objdir.h | 6 ++++--
5 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/object-file.c b/object-file.c
index 4d03c167d5..db63587f6d 100644
--- a/object-file.c
+++ b/object-file.c
@@ -485,6 +485,7 @@ struct odb_transaction_files {
struct odb_transaction base;
struct tmp_objdir *objdir;
+ struct odb_source *quarantine;
struct transaction_packfile packfile;
const char *prefix;
};
@@ -507,7 +508,7 @@ int odb_transaction_files_prepare(struct odb_transaction *base)
if (!transaction->objdir)
return error(_("unable to create temporary object directory"));
- tmp_objdir_replace_primary_odb(transaction->objdir, 0);
+ transaction->quarantine = tmp_objdir_replace_primary_odb(transaction->objdir, 0);
return 0;
}
diff --git a/odb.c b/odb.c
index caf1d0f542..8afcb6b637 100644
--- a/odb.c
+++ b/odb.c
@@ -226,7 +226,8 @@ struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
}
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
- const char *dir, int will_destroy)
+ const char *dir, int will_destroy,
+ struct odb_source **prev_source)
{
struct odb_source *source;
@@ -250,7 +251,11 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
source->will_destroy = will_destroy;
source->next = odb->sources;
odb->sources = source;
- return source->next;
+
+ if (prev_source)
+ *prev_source = source->next;
+
+ return source;
}
void odb_restore_primary_source(struct object_database *odb,
diff --git a/odb.h b/odb.h
index fca67e8253..bdfcb9509a 100644
--- a/odb.h
+++ b/odb.h
@@ -199,10 +199,12 @@ struct odb_source *odb_find_source_or_die(struct object_database *odb, const cha
/*
* Replace the current writable object directory with the specified temporary
- * object directory; returns the former primary source.
+ * object directory and return the newly installed primary source. The former
+ * primary source is reported via `prev_source` when non-NULL.
*/
struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
- const char *dir, int will_destroy);
+ const char *dir, int will_destroy,
+ struct odb_source **prev_source);
/*
* Restore the primary source that was previously replaced by
diff --git a/tmp-objdir.c b/tmp-objdir.c
index d199d39e7c..e633d97e0e 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -327,11 +327,13 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *t)
odb_add_to_alternates_memory(t->repo->objects, t->path.buf);
}
-void tmp_objdir_replace_primary_odb(struct tmp_objdir *t, int will_destroy)
+struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *t,
+ int will_destroy)
{
if (t->prev_source)
BUG("the primary object database is already replaced");
- t->prev_source = odb_set_temporary_primary_source(t->repo->objects,
- t->path.buf, will_destroy);
t->will_destroy = will_destroy;
+
+ return odb_set_temporary_primary_source(t->repo->objects, t->path.buf,
+ will_destroy, &t->prev_source);
}
diff --git a/tmp-objdir.h b/tmp-objdir.h
index ccf800faa7..81eb927413 100644
--- a/tmp-objdir.h
+++ b/tmp-objdir.h
@@ -64,8 +64,10 @@ void tmp_objdir_add_as_alternate(const struct tmp_objdir *);
/*
* Replaces the writable object store in the current process with the temporary
* object directory and makes the former main object store an alternate.
- * If will_destroy is nonzero, the object directory may not be migrated.
+ * If will_destroy is nonzero, the object directory may not be migrated. Returns
+ * the newly installed primary source.
*/
-void tmp_objdir_replace_primary_odb(struct tmp_objdir *, int will_destroy);
+struct odb_source *tmp_objdir_replace_primary_odb(struct tmp_objdir *,
+ int will_destroy);
#endif /* TMP_OBJDIR_H */
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread* [PATCH v5 9/9] odb/transaction: add transaction interface to write packfiles
2026-08-20 23:49 ` [PATCH v5 0/9] builtin/receive-pack: support pluggable packfile writes Justin Tobler
` (7 preceding siblings ...)
2026-08-20 23:49 ` [PATCH v5 8/9] odb: return temporary ODB source when set Justin Tobler
@ 2026-08-20 23:49 ` Justin Tobler
8 siblings, 0 replies; 78+ messages in thread
From: Justin Tobler @ 2026-08-20 23:49 UTC (permalink / raw)
To: git; +Cc: ps, gitster, Justin Tobler
In git-receive-pack(1), the incoming packfile is written to the ODB via
`unpack()`, which spawns git-index-pack(1) or git-unpack-objects(1)
directly. With pluggable object databases, an alternative backend may
need to handle writing packfile data differently though.
Introduce `odb_transaction_write_pack()` as a generic interface to
handle writing a packfile to a transaction and use the logic from
`unpack()` as the "files" backend implementation. Note that when storing
the objects as a packfile, git-index-pack(1) also writes a ".keep"
lockfile next to it to prevent a concurrent repack from removing the new
pack prior to reference updates being performed. The "files" transaction
backend is responsible for managing these ".keep" files and removes them
post-commit once the transaction is finalized.
Call sites in git-receive-pack(1) are updated accordingly.
Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
builtin/receive-pack.c | 160 +-----------------------------------
object-file.c | 178 +++++++++++++++++++++++++++++++++++++++++
odb/transaction.c | 7 ++
odb/transaction.h | 62 ++++++++++++++
4 files changed, 250 insertions(+), 157 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b369466783..e6e54ba55f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -15,7 +15,6 @@
#include "gpg-interface.h"
#include "hex.h"
#include "hook.h"
-#include "lockfile.h"
#include "object.h"
#include "object-file.h"
#include "object-name.h"
@@ -23,7 +22,6 @@
#include "oid-array.h"
#include "oidset.h"
#include "pack.h"
-#include "packfile.h"
#include "parse-options.h"
#include "pkt-line.h"
#include "protocol.h"
@@ -2292,162 +2290,11 @@ static void read_push_options(struct packet_reader *reader,
}
}
-static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
-{
- switch (read_pack_header(pack_fd, hdr)) {
- case PH_ERROR_EOF:
- return "eof before pack header was fully read";
-
- case PH_ERROR_PACK_SIGNATURE:
- return "protocol error (pack signature mismatch detected)";
-
- case PH_ERROR_PROTOCOL:
- return "protocol error (pack version unsupported)";
-
- default:
- return "unknown error in parse_pack_header";
-
- case 0:
- return NULL;
- }
-}
-
-static struct tempfile *pack_lockfile;
-
-static void push_header_arg(struct strvec *args, struct pack_header *hdr)
-{
- strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
- ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
-}
-
-static unsigned int get_unpack_limit(struct repository *repo)
-{
- unsigned int limit = 100;
-
- repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
- repo_config_get_uint(repo, "receive.unpacklimit", &limit);
-
- return limit;
-}
-
-struct unpack_opts {
- const char *fsck_msg_types;
- const char *shallow_file;
- off_t max_input_size;
- int fsck_objects;
- int reject_thin;
- int err_fd;
- int quiet;
-};
-
-static int unpack(struct odb_transaction *transaction, int pack_fd,
- struct strbuf *err_msg, const struct unpack_opts *opts)
-{
- struct pack_header hdr;
- const char *hdr_err;
- int status;
- struct child_process child = CHILD_PROCESS_INIT;
- int err_fd = opts->err_fd;
-
- hdr_err = parse_pack_header(&hdr, pack_fd);
- if (hdr_err) {
- if (err_fd > 0)
- close(err_fd);
- strbuf_addstr(err_msg, hdr_err);
- return -1;
- }
-
- if (opts->shallow_file) {
- strvec_push(&child.args, "--shallow-file");
- strvec_push(&child.args, opts->shallow_file);
- }
-
- odb_transaction_env(transaction, &child.env);
-
- if (ntohl(hdr.hdr_entries) < get_unpack_limit(the_repository)) {
- strvec_push(&child.args, "unpack-objects");
- push_header_arg(&child.args, &hdr);
- if (opts->quiet)
- strvec_push(&child.args, "-q");
- if (opts->fsck_objects)
- strvec_pushf(&child.args, "--strict%s",
- opts->fsck_msg_types);
- if (opts->max_input_size)
- strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)opts->max_input_size);
- child.no_stdout = 1;
- child.in = pack_fd;
- child.err = err_fd;
- child.git_cmd = 1;
- status = run_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "unpack-objects abnormal exit");
- return -1;
- }
- } else {
- char hostname[HOST_NAME_MAX + 1];
- char *lockfile;
-
- strvec_pushl(&child.args, "index-pack", "--stdin", NULL);
- push_header_arg(&child.args, &hdr);
-
- if (xgethostname(hostname, sizeof(hostname)))
- xsnprintf(hostname, sizeof(hostname), "localhost");
- strvec_pushf(&child.args,
- "--keep=receive-pack %"PRIuMAX" on %s",
- (uintmax_t)getpid(),
- hostname);
-
- if (!opts->quiet && err_fd)
- strvec_push(&child.args, "--show-resolving-progress");
- if (err_fd)
- strvec_push(&child.args, "--report-end-of-input");
- if (opts->fsck_objects)
- strvec_pushf(&child.args, "--strict%s",
- opts->fsck_msg_types);
- if (!opts->reject_thin)
- strvec_push(&child.args, "--fix-thin");
- if (opts->max_input_size)
- strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
- (uintmax_t)opts->max_input_size);
- child.out = -1;
- child.in = pack_fd;
- child.err = err_fd;
- child.git_cmd = 1;
- status = start_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "index-pack fork failed");
- return -1;
- }
-
- /*
- * The lockfile filepath is expected to be the final location of
- * the ".keep" file after being migrated to the main ODB source.
- * This ensures the lockfile can be found and removed later
- * after the ODB transaction has been committed.
- */
- lockfile = index_pack_lockfile(transaction->source, child.out, NULL);
- if (lockfile) {
- pack_lockfile = register_tempfile(lockfile);
- free(lockfile);
- }
- close(child.out);
-
- status = finish_command(&child);
- if (status) {
- strbuf_addstr(err_msg, "index-pack abnormal exit");
- return -1;
- }
- odb_reprepare(the_repository->objects);
- }
- return 0;
-}
-
static int unpack_with_sideband(struct odb_transaction *transaction,
const char *shallow_file,
struct strbuf *err_msg)
{
- struct unpack_opts opts = {
+ struct odb_transaction_write_pack_opts opts = {
.fsck_objects = (receive_fsck_objects >= 0
? receive_fsck_objects
: transfer_fsck_objects >= 0
@@ -2463,7 +2310,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
int ret;
if (!use_sideband)
- return unpack(transaction, 0, err_msg, &opts);
+ return odb_transaction_write_pack(transaction, 0, err_msg, &opts);
use_keepalive = KEEPALIVE_AFTER_NUL;
memset(&muxer, 0, sizeof(muxer));
@@ -2473,7 +2320,7 @@ static int unpack_with_sideband(struct odb_transaction *transaction,
return 0;
opts.err_fd = muxer.in;
- ret = unpack(transaction, 0, err_msg, &opts);
+ ret = odb_transaction_write_pack(transaction, 0, err_msg, &opts);
finish_async(&muxer);
return ret;
@@ -2748,7 +2595,6 @@ int cmd_receive_pack(int argc,
execute_commands(commands, !!unpack_status.len, &si, transaction,
&push_options);
odb_transaction_finalize(transaction);
- delete_tempfile(&pack_lockfile);
sigchain_push(SIGPIPE, SIG_IGN);
if (report_status_v2)
report_v2(commands, &unpack_status);
diff --git a/object-file.c b/object-file.c
index db63587f6d..265c5f7a3c 100644
--- a/object-file.c
+++ b/object-file.c
@@ -10,6 +10,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "git-compat-util.h"
+#include "config.h"
#include "convert.h"
#include "dir.h"
#include "environment.h"
@@ -26,6 +27,7 @@
#include "packfile.h"
#include "path.h"
#include "read-cache-ll.h"
+#include "run-command.h"
#include "setup.h"
#include "strvec.h"
#include "tempfile.h"
@@ -483,11 +485,16 @@ struct transaction_packfile {
struct odb_transaction_files {
struct odb_transaction base;
+ enum odb_transaction_flags flags;
struct tmp_objdir *objdir;
struct odb_source *quarantine;
struct transaction_packfile packfile;
const char *prefix;
+
+ struct tempfile **pack_lockfiles;
+ size_t pack_lockfiles_nr;
+ size_t pack_lockfiles_alloc;
};
int odb_transaction_files_prepare(struct odb_transaction *base)
@@ -1291,6 +1298,174 @@ static int odb_transaction_files_commit(struct odb_transaction *base)
return 0;
}
+static const char *parse_pack_header(struct pack_header *hdr, int pack_fd)
+{
+ switch (read_pack_header(pack_fd, hdr)) {
+ case PH_ERROR_EOF:
+ return "eof before pack header was fully read";
+
+ case PH_ERROR_PACK_SIGNATURE:
+ return "protocol error (pack signature mismatch detected)";
+
+ case PH_ERROR_PROTOCOL:
+ return "protocol error (pack version unsupported)";
+
+ default:
+ return "unknown error in parse_pack_header";
+
+ case 0:
+ return NULL;
+ }
+}
+
+static void push_header_arg(struct strvec *args, struct pack_header *hdr)
+{
+ strvec_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
+ ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
+}
+
+static unsigned int get_unpack_limit(struct repository *repo,
+ enum odb_transaction_flags flags)
+{
+ unsigned int limit = 0;
+
+ if (flags & ODB_TRANSACTION_RECEIVE) {
+ limit = 100;
+ repo_config_get_uint(repo, "transfer.unpacklimit", &limit);
+ repo_config_get_uint(repo, "receive.unpacklimit", &limit);
+ }
+
+ return limit;
+}
+
+static int odb_transaction_files_write_pack(struct odb_transaction *base,
+ int pack_fd, struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+ struct repository *repo = base->source->odb->repo;
+ struct child_process child = CHILD_PROCESS_INIT;
+ struct pack_header hdr;
+ const char *hdr_err;
+ int err_fd = opts->err_fd;
+ int status;
+
+ hdr_err = parse_pack_header(&hdr, pack_fd);
+ if (hdr_err) {
+ if (err_fd > 0)
+ close(err_fd);
+ strbuf_addstr(err_msg, hdr_err);
+ return -1;
+ }
+
+ if (opts->shallow_file) {
+ strvec_push(&child.args, "--shallow-file");
+ strvec_push(&child.args, opts->shallow_file);
+ }
+
+ odb_transaction_env(base, &child.env);
+
+ if (ntohl(hdr.hdr_entries) < get_unpack_limit(repo, transaction->flags)) {
+ strvec_push(&child.args, "unpack-objects");
+ push_header_arg(&child.args, &hdr);
+ if (opts->quiet)
+ strvec_push(&child.args, "-q");
+ if (opts->fsck_objects)
+ strvec_pushf(&child.args, "--strict%s",
+ opts->fsck_msg_types);
+ if (opts->max_input_size)
+ strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
+ (uintmax_t)opts->max_input_size);
+ child.no_stdout = 1;
+ child.in = pack_fd;
+ child.err = err_fd;
+ child.git_cmd = 1;
+ status = run_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "unpack-objects abnormal exit");
+ return -1;
+ }
+ } else {
+ char hostname[HOST_NAME_MAX + 1];
+ char *lockfile;
+
+ strvec_pushl(&child.args, "index-pack", "--stdin", NULL);
+ push_header_arg(&child.args, &hdr);
+
+ if (xgethostname(hostname, sizeof(hostname)))
+ xsnprintf(hostname, sizeof(hostname), "localhost");
+ strvec_pushf(&child.args,
+ "--keep=receive-pack %"PRIuMAX" on %s",
+ (uintmax_t)getpid(),
+ hostname);
+
+ if (!opts->quiet && err_fd)
+ strvec_push(&child.args, "--show-resolving-progress");
+ if (err_fd)
+ strvec_push(&child.args, "--report-end-of-input");
+ if (opts->fsck_objects)
+ strvec_pushf(&child.args, "--strict%s",
+ opts->fsck_msg_types);
+ if (!opts->reject_thin)
+ strvec_push(&child.args, "--fix-thin");
+ if (opts->max_input_size)
+ strvec_pushf(&child.args, "--max-input-size=%"PRIuMAX,
+ (uintmax_t)opts->max_input_size);
+ child.out = -1;
+ child.in = pack_fd;
+ child.err = err_fd;
+ child.git_cmd = 1;
+ status = start_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack fork failed");
+ return -1;
+ }
+
+ /*
+ * The lockfile filepath is expected to be the final location of
+ * the ".keep" file after being migrated to the main ODB source.
+ * This ensures the lockfile can be found and removed later
+ * after the ODB transaction has been committed.
+ */
+ lockfile = index_pack_lockfile(base->source, child.out, NULL);
+ if (lockfile) {
+ ALLOC_GROW(transaction->pack_lockfiles,
+ transaction->pack_lockfiles_nr + 1,
+ transaction->pack_lockfiles_alloc);
+ transaction->pack_lockfiles[transaction->pack_lockfiles_nr++] =
+ register_tempfile(lockfile);
+ free(lockfile);
+ }
+ close(child.out);
+
+ status = finish_command(&child);
+ if (status) {
+ strbuf_addstr(err_msg, "index-pack abnormal exit");
+ return -1;
+ }
+
+ odb_source_prepare(transaction->quarantine,
+ ODB_PREPARE_FLUSH_CACHES);
+ }
+
+ return 0;
+}
+
+static int odb_transaction_files_finalize(struct odb_transaction *base)
+{
+ struct odb_transaction_files *transaction =
+ container_of(base, struct odb_transaction_files, base);
+ int ret = 0;
+
+ for (size_t i = 0; i < transaction->pack_lockfiles_nr; i++)
+ ret |= delete_tempfile(&transaction->pack_lockfiles[i]);
+
+ free(transaction->pack_lockfiles);
+
+ return ret;
+}
+
static int odb_transaction_files_env(struct odb_transaction *base,
struct strvec *env)
{
@@ -1314,8 +1489,11 @@ int odb_transaction_files_begin(struct odb_source *source,
transaction = xcalloc(1, sizeof(*transaction));
transaction->base.source = source;
transaction->base.commit = odb_transaction_files_commit;
+ transaction->base.finalize = odb_transaction_files_finalize;
transaction->base.write_object_stream = odb_transaction_files_write_object_stream;
+ transaction->base.write_pack = odb_transaction_files_write_pack;
transaction->base.env = odb_transaction_files_env;
+ transaction->flags = flags;
transaction->prefix = "bulk-fsync";
if (flags & ODB_TRANSACTION_RECEIVE) {
diff --git a/odb/transaction.c b/odb/transaction.c
index 9e9a982778..c9144e6cd6 100644
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -59,6 +59,13 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
return transaction->write_object_stream(transaction, stream, len, oid);
}
+int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts)
+{
+ return transaction->write_pack(transaction, pack_fd, err_msg, opts);
+}
+
int odb_transaction_env(struct odb_transaction *transaction, struct strvec *env)
{
if (!transaction)
diff --git a/odb/transaction.h b/odb/transaction.h
index 6ed39b3d0e..8cb06c1191 100644
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -4,6 +4,50 @@
#include "gettext.h"
#include "odb.h"
+/*
+ * Options controlling how odb_transaction_write_pack() ingests a packfile.
+ */
+struct odb_transaction_write_pack_opts {
+ /*
+ * Optional fsck severity configuration to apply when incoming objects
+ * are verified.
+ */
+ const char *fsck_msg_types;
+
+ /*
+ * Path to an alternative shallow file describing the shallow boundaries
+ * to honor while ingesting the pack.
+ */
+ const char *shallow_file;
+
+ /*
+ * The max size in bytes of the incoming packfile allowed. No limit is
+ * enforced when set to 0.
+ */
+ off_t max_input_size;
+
+ /*
+ * Whether the validity of incoming objects should be verified.
+ */
+ int fsck_objects;
+
+ /*
+ * Whether to reject an incoming packfile if it is "thin".
+ */
+ int reject_thin;
+
+ /*
+ * Optional file descriptor for reporting progress and errors. Set to 0
+ * for none.
+ */
+ int err_fd;
+
+ /*
+ * Suppresses progress reporting.
+ */
+ int quiet;
+};
+
/*
* A transaction may be started for an object database prior to writing new
* objects via odb_transaction_begin(). These objects are not committed until
@@ -40,6 +84,15 @@ struct odb_transaction {
int (*write_object_stream)(struct odb_transaction *transaction,
struct odb_write_stream *stream, size_t len,
struct object_id *oid);
+ /*
+ * This callback is expected to ingest the packfile readable via
+ * `pack_fd` into the transaction. Returns 0 on success, a negative
+ * error code otherwise. On failure, a human-readable description is
+ * appended to `err_msg`.
+ */
+ int (*write_pack)(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts);
/*
* This callback is expected to populate the provided strvec with the
@@ -107,6 +160,15 @@ int odb_transaction_write_object_stream(struct odb_transaction *transaction,
struct odb_write_stream *stream,
size_t len, struct object_id *oid);
+/*
+ * Ingests the packfile readable via `pack_fd` into the transaction. Returns 0
+ * on success, a negative error code otherwise. On failure, a human-readable
+ * description is appended to `err_msg`.
+ */
+int odb_transaction_write_pack(struct odb_transaction *transaction, int pack_fd,
+ struct strbuf *err_msg,
+ const struct odb_transaction_write_pack_opts *opts);
+
/*
* Populates the provided strvec with the environment variables that a child
* process should inherit so that its object writes participate in the
--
2.55.0.424.g13c7afec21
^ permalink raw reply related [flat|nested] 78+ messages in thread