[PATCH v3 0/6] odb: make packfile generation pluggable
Patrick Steinhardt <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
Hi,
this patch series makes packfile generation pluggable.
Note that this series only makes those parts pluggable that are required
for the transport layer. The other parts that relate to packfile
generation as required by our repository maintenance is kept as-is, as
there is a bunch of options there that are way too specific to the
"files" backend to be portable. This should ultimately not be much of a
problem though, as maintenance itself is already pluggable in the first
place.
It's a bit of a shame though for git-pack-objects(1), which still isn't
usable with alternate backends. I tried several times to find good
solutions for making it fully pluggable, but due to the backend-specific
options it's an utter mess. I want to eventually address this though:
same as with git-refs(1), I want to introduce git-objects(1) to care
about all things ODB. And as part of that command we can also introduce
a command that generates packfiles in a generic fashion, without all the
cruft that git-pack-objects(1) has. This is part of a future patch
series though.
Changes in v3:
- Fix a use-after-scope bug on abnormal exit when child processes are
cleaned up via `mark_child_for_cleanup()`, as noticed by Elijah.
- Link to v2: https://patch.msgid.link/[email protected]
Changes in v2:
- Mostly remove the dependencies on `the_repository` in "bundle.c".
- Link to v1: https://patch.msgid.link/[email protected]
The series is built on top of 2c78326f81 (The 11th batch, 2026-08-05).
Thanks!
Patrick
---
Patrick Steinhardt (6):
odb: introduce interface to generate packfiles
upload-pack: generate packfiles via the object database
send-pack: generate packfiles via the object database
builtin/bundle: refactor option handling for progress meter
bundle: get (mostly) rid of `the_repository`
bundle: generate packfiles via the object database
builtin/bundle.c | 31 ++++------
bundle.c | 97 ++++++++++++++++++--------------
bundle.h | 3 +-
odb.c | 21 +++++++
odb.h | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
odb/source-files.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++++
odb/source.h | 33 +++++++++++
send-pack.c | 101 +++++++++++----------------------
t/t5516-fetch-push.sh | 12 ++--
upload-pack.c | 125 +++++++++++++++--------------------------
10 files changed, 506 insertions(+), 218 deletions(-)
Range-diff versus v2:
1: 34a7d13ccb ! 1: d1282ffc64 odb: introduce interface to generate packfiles
@@ odb/source-files.c: int odb_source_files_optimize(struct odb_source *source,
+ struct odb_pack_generator **out,
+ const struct odb_generate_pack_options *opts)
+{
-+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct odb_pack_generator_files *generator;
++ struct child_process *cp;
+ FILE *in;
+
++ CALLOC_ARRAY(generator, 1);
++ child_process_init(&generator->cp);
++ cp = &generator->cp;
++
+ /*
+ * The hook is expected to spawn "$hook git pack-objects <args...>"
+ * and to behave like git-pack-objects(1) would have. This can for
+ * example be used to serve precomputed packfiles.
+ */
+ if (opts->pack_objects_hook) {
-+ strvec_push(&cp.args, opts->pack_objects_hook);
-+ strvec_push(&cp.args, "git");
-+ cp.use_shell = 1;
++ strvec_push(&cp->args, opts->pack_objects_hook);
++ strvec_push(&cp->args, "git");
++ cp->use_shell = 1;
+ } else {
-+ cp.git_cmd = 1;
++ cp->git_cmd = 1;
+ }
+
+ /*
@@ odb/source-files.c: int odb_source_files_optimize(struct odb_source *source,
+ * be neutralized.
+ */
+ if (opts->shallows.nr) {
-+ strvec_push(&cp.args, "--shallow-file");
-+ strvec_push(&cp.args, "");
++ strvec_push(&cp->args, "--shallow-file");
++ strvec_push(&cp->args, "");
+ }
-+ strvec_push(&cp.args, "pack-objects");
-+ strvec_push(&cp.args, "--revs");
-+ strvec_push(&cp.args, "--stdout");
++ strvec_push(&cp->args, "pack-objects");
++ strvec_push(&cp->args, "--revs");
++ strvec_push(&cp->args, "--stdout");
+ if (opts->thin)
-+ strvec_push(&cp.args, "--thin");
++ strvec_push(&cp->args, "--thin");
+ if (opts->shallow)
-+ strvec_push(&cp.args, "--shallow");
++ strvec_push(&cp->args, "--shallow");
+ if (opts->ofs_delta)
-+ strvec_push(&cp.args, "--delta-base-offset");
++ strvec_push(&cp->args, "--delta-base-offset");
+ if (opts->include_tag)
-+ strvec_push(&cp.args, "--include-tag");
++ strvec_push(&cp->args, "--include-tag");
+ if (opts->missing_allow_promisor)
-+ strvec_push(&cp.args, "--missing=allow-promisor");
++ strvec_push(&cp->args, "--missing=allow-promisor");
+ if (opts->disable_bitmaps)
-+ strvec_push(&cp.args, "--no-use-bitmap-index");
++ strvec_push(&cp->args, "--no-use-bitmap-index");
+ switch (opts->progress) {
+ case ODB_GENERATE_PACK_PROGRESS_NONE:
-+ strvec_push(&cp.args, "--quiet");
++ strvec_push(&cp->args, "--quiet");
+ break;
+ case ODB_GENERATE_PACK_PROGRESS_STANDARD:
-+ strvec_push(&cp.args, "--progress");
++ strvec_push(&cp->args, "--progress");
+ break;
+ case ODB_GENERATE_PACK_PROGRESS_VERBOSE:
-+ strvec_push(&cp.args, "--all-progress");
++ strvec_push(&cp->args, "--all-progress");
+ break;
+ default:
+ BUG("unknown progress option %d", opts->progress);
+ }
+ if (opts->filter_spec)
-+ strvec_pushf(&cp.args, "--filter=%s", opts->filter_spec);
++ strvec_pushf(&cp->args, "--filter=%s", opts->filter_spec);
+ if (opts->uri_protocols)
+ for (size_t i = 0; i < opts->uri_protocols->nr; i++)
-+ strvec_pushf(&cp.args, "--uri-protocol=%s",
++ strvec_pushf(&cp->args, "--uri-protocol=%s",
+ opts->uri_protocols->items[i].string);
+
-+ cp.in = -1;
-+ cp.out = opts->pack_fd;
-+ cp.err = opts->progress_fd;
-+ cp.clean_on_exit = 1;
++ cp->in = -1;
++ cp->out = opts->pack_fd;
++ cp->err = opts->progress_fd;
++ cp->clean_on_exit = 1;
+
-+ if (start_command(&cp))
++ if (start_command(cp)) {
++ free(generator);
+ return error(_("could not spawn pack-objects"));
++ }
+
+ /*
+ * Feed the objects to pack-objects. This is safe to do synchronously
+ * because pack-objects consumes all of its standard input before it
+ * starts to generate the pack.
+ */
-+ in = xfdopen(cp.in, "w");
++ in = xfdopen(cp->in, "w");
+ for (size_t i = 0; i < opts->shallows.nr; i++)
+ fprintf(in, "--shallow %s\n", oid_to_hex(&opts->shallows.oid[i]));
+ for (size_t i = 0; i < opts->wants.nr; i++)
@@ odb/source-files.c: int odb_source_files_optimize(struct odb_source *source,
+ error(_("error writing to pack-objects"));
+ fclose(in);
+ if (opts->pack_fd < 0)
-+ close(cp.out);
++ close(cp->out);
+ if (opts->progress_fd < 0)
-+ close(cp.err);
-+ finish_command(&cp);
++ close(cp->err);
++ finish_command(cp);
++ free(generator);
+ return -1;
+ }
+ fclose(in);
+
-+ CALLOC_ARRAY(generator, 1);
-+ generator->base.out = opts->pack_fd < 0 ? cp.out : -1;
-+ generator->base.err = opts->progress_fd < 0 ? cp.err : -1;
++ generator->base.out = opts->pack_fd < 0 ? cp->out : -1;
++ generator->base.err = opts->progress_fd < 0 ? cp->err : -1;
+ generator->base.finish = odb_pack_generator_files_finish;
-+ generator->cp = cp;
+
+ *out = &generator->base;
+ return 0;
2: c7234030ad = 2: b17dfd945b upload-pack: generate packfiles via the object database
3: 9866e8af3d = 3: 8e9be66b36 send-pack: generate packfiles via the object database
4: 1cd0c10438 = 4: 3dfc5df91d builtin/bundle: refactor option handling for progress meter
5: 621c9bb411 = 5: 9f938bce19 bundle: get (mostly) rid of `the_repository`
6: 5c1ee3d116 = 6: 41395b1444 bundle: generate packfiles via the object database
---
base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
change-id: 20260807-b4-pks-odb-generate-pack-f30fbcdef3fc