Git development
 help / color / mirror / Atom feed
* [PATCH 00/13] odb/source-files: move alternates into the backend
@ 2026-10-02 10:08 Patrick Steinhardt
  2026-10-02 10:08 ` [PATCH 01/13] commit-graph: require resolved packfile paths for `stdin_packs` Patrick Steinhardt
                   ` (14 more replies)
  0 siblings, 15 replies; 41+ messages in thread
From: Patrick Steinhardt @ 2026-10-02 10:08 UTC (permalink / raw)
  To: git

Hi,

Originally, when designing pluggable object databases the goal was that
the object database can have multiple sources, and every source attached
to it could use a different backend. This would have allowed for quite a
lot of flexibility, as you could trivially mix and match different kinds
of object storages in whatever way you like.

But while well-intentioned, this design led to a bunch of conceptual
problems:

  - We're now trying to read objects in source order, whereas we
    previously tried to read objects via packfiles before trying to read
    them via loose objects. This led to a performance regression when
    using alternates or when using a quarantine directory.

  - Some data structures are supposed to only ever exist once, like for
    example bitmaps and commit graphs. At the same time, those data
    structures also span across the union of all objects, so they may
    cross sources.

  - It is unclear how we can extend GIT_OBJECT_DIRECTORY or
    GIT_ALTERNATE_OBJECT_DIRECTORIES to become backend-agnostic in a
    backwards-compatible way. In general, introducing an object storage
    extension into the current status quo where alternates may have to
    be extended to become generic was proving to be painful.

  - Some mechanisms of alternates assume way too much about how exactly
    their backends work. Alternate refs for example assume that the
    alternate is backed by a filesystem path, and that this filesystem
    path may also allow us to read references. This is not a given
    though, as backends may not even have local data at all.

In short, there are a bunch of conceptual mismatches when we have
alternates and pluggable object databases coexist. So while the original
idea was nice, it does not result in a system that is easy to reason
about.

This patch series corrects course by moving alternates into the "files"
backend itself so that they become another implementation detail. It's
unfortunately on the bigger side, and I'm sorry about that, but I
couldn't really find a way to split it up further in a sensible way.

Note that the above problems aren't fixed by this series yet, but it is
the prerequisite to fix them in subsequent patch series.

The series is built on top of c46c1e3772 (Start Git 2.98 cycle,
2026-09-30). Note that there's a couple of small merge conflicts with
"seen". These can be resolved as follows:

diff --cc builtin/multi-pack-index.c
index c48212290c,6b2e58f427..0000000000
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@@ -225,8 -225,9 +225,9 @@@ static int cmd_multi_pack_index_write(i
  
  	}
  
 -	ret = write_midx_file(source->packed, opts.preferred_pack,
 +	ret = write_midx_file(packed_source, opts.preferred_pack,
- 			      opts.refs_snapshot, opts.flags);
+ 			      opts.refs_snapshot, opts.incremental_base,
+ 			      opts.flags);
  
  	free(opts.refs_snapshot);
  	return ret;
diff --cc builtin/pack-objects.c
index ca3a891dfb,fb603059a9..0000000000
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@@ -4566,10 -4624,10 +4616,10 @@@ static int add_loose_object(const struc
   * add_object_entry will weed out duplicates, so we just add every
   * loose object we find.
   */
- static void add_unreachable_loose_objects(struct rev_info *revs)
+ static void add_unreachable_loose_objects(struct stdin_packs_context *ctx)
  {
 -	for_each_loose_file_in_source(the_repository->objects->sources,
 +	for_each_loose_file_in_source(the_repository->objects->source,
- 				      add_loose_object, NULL, NULL, revs);
+ 				      add_loose_object, NULL, NULL, ctx);
  }
  
  static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)
diff --cc builtin/repack.c
index 5d06872d77,87f03b66d9..0000000000
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@@ -775,7 -809,7 +809,7 @@@ int cmd_repack(int argc
  
  		if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL, 0))
  			flags |= MIDX_WRITE_INCREMENTAL;
- 		write_midx_file(files->dirs->packed, NULL, NULL, flags);
 -		write_midx_file(files->packed, NULL, NULL, NULL, flags);
++		write_midx_file(files->dirs->packed, NULL, NULL, NULL, flags);
  	}
  
  cleanup:
diff --git a/repack-midx.c b/repack-midx.c
index d805802f04..7281003473 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -575,7 +575,7 @@ static int midx_compaction_step_include_packs(struct midx_compaction_step *step,
 
 		strbuf_reset(&path);
 		strbuf_addf(&path, "%s/%s", opts->packdir, item->string);
-		p = packfile_store_load_pack(files->packed, path.buf, 1);
+		p = packfile_store_load_pack(files->dirs->packed, path.buf, 1);
 		if (!p || open_pack_index(p)) {
 			ret = error(_("cannot open index for %s"), path.buf);
 			goto out;

Thanks!

Patrick

---
Patrick Steinhardt (13):
      commit-graph: require resolved packfile paths for `stdin_packs`
      commit-graph: stop depending on `struct odb_source`
      odb/source-files: introduce `struct odb_files_dir`
      odb: refactor `odb_for_each_alternate()` to yield dirs
      odb: refactor `odb_find_source()` to yield dirs
      odb/source-files: add the ability to have multiple object dirs
      tmp-objdir: absorb logic to set and restore primary sources
      tmp-objdir: manage quarantine as an object directory
      tmp-objdir: replace primary source at creation time
      odb/source: make `will_destroy` an implementation detail
      odb/source-files: extract reading alternates
      odb/source-files: move alternates into the backend
      odb/source: drop `read_alternates` callback

 builtin/commit-graph.c      |  41 ++--
 builtin/commit.c            |   2 +-
 builtin/count-objects.c     |   6 +-
 builtin/fast-import.c       |  20 +-
 builtin/fetch.c             |   4 +-
 builtin/fsck.c              |   6 +-
 builtin/gc.c                |  15 +-
 builtin/index-pack.c        |   4 +-
 builtin/merge.c             |   2 +-
 builtin/multi-pack-index.c  |  48 ++---
 builtin/pack-objects.c      |  66 +++---
 builtin/prune.c             |   2 +-
 builtin/repack.c            |   4 +-
 builtin/submodule--helper.c |   7 +-
 bundle.c                    |   2 +-
 commit-graph.c              | 150 +++++++-------
 commit-graph.h              |  31 ++-
 diagnose.c                  |   8 +-
 fetch-pack.c                |   2 +-
 http-walker.c               |   4 +-
 http.c                      |  12 +-
 log-tree.c                  |   3 +-
 loose.c                     |  18 +-
 midx.c                      |  43 ++--
 object-file.c               |   8 +-
 odb.c                       | 439 +++++-----------------------------------
 odb.h                       |  62 +-----
 odb/source-files.c          | 482 ++++++++++++++++++++++++++++++++++++--------
 odb/source-files.h          |  65 +++++-
 odb/source-inmemory.c       |   7 -
 odb/source-loose.c          |   9 +-
 odb/source-loose.h          |   3 +
 odb/source-packed.c         |   7 -
 odb/source.c                |   5 +-
 odb/source.h                |  57 +-----
 odb/streaming.c             |   8 +-
 odb/transaction.c           |   2 +-
 pack-bitmap.c               |   8 +-
 packfile.c                  |  28 ++-
 packfile.h                  |  21 +-
 path.c                      |   2 +-
 prune-packed.c              |   2 +-
 repack-geometry.c           |   2 +-
 repack-midx.c               |   6 +-
 repack.c                    |   6 +-
 repository.c                |   4 +-
 setup.c                     |   2 +-
 t/helper/test-read-graph.c  |   5 +-
 t/helper/test-read-midx.c   |   8 +-
 t/t4216-log-bloom.sh        |   4 +-
 tmp-objdir.c                |  67 ++++--
 tmp-objdir.h                |  18 +-
 52 files changed, 883 insertions(+), 954 deletions(-)


---
base-commit: 2f92b2890ddaf3d7ea29470c02418271c1a4cd79
change-id: 20260924-pks-odb-move-alternates-4a0babe4b0f3


^ permalink raw reply related	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2026-10-08  9:24 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-02 10:08 [PATCH 00/13] odb/source-files: move alternates into the backend Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 01/13] commit-graph: require resolved packfile paths for `stdin_packs` Patrick Steinhardt
2026-10-05 19:27   ` Karthik Nayak
2026-10-06 12:18     ` Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 02/13] commit-graph: stop depending on `struct odb_source` Patrick Steinhardt
2026-10-05 19:43   ` Karthik Nayak
2026-10-06 12:18     ` Patrick Steinhardt
2026-10-06 20:11       ` Karthik Nayak
2026-10-02 10:08 ` [PATCH 03/13] odb/source-files: introduce `struct odb_files_dir` Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 04/13] odb: refactor `odb_for_each_alternate()` to yield dirs Patrick Steinhardt
2026-10-06  8:42   ` Karthik Nayak
2026-10-02 10:08 ` [PATCH 05/13] odb: refactor `odb_find_source()` " Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 06/13] odb/source-files: add the ability to have multiple object dirs Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 07/13] tmp-objdir: absorb logic to set and restore primary sources Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 08/13] tmp-objdir: manage quarantine as an object directory Patrick Steinhardt
2026-10-06 20:18   ` Karthik Nayak
2026-10-06 20:27     ` Karthik Nayak
2026-10-02 10:08 ` [PATCH 09/13] tmp-objdir: replace primary source at creation time Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 10/13] odb/source: make `will_destroy` an implementation detail Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 11/13] odb/source-files: extract reading alternates Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 12/13] odb/source-files: move alternates into the backend Patrick Steinhardt
2026-10-06 20:51   ` Karthik Nayak
2026-10-07  5:50     ` Patrick Steinhardt
2026-10-02 10:08 ` [PATCH 13/13] odb/source: drop `read_alternates` callback Patrick Steinhardt
2026-10-06 20:53 ` [PATCH 00/13] odb/source-files: move alternates into the backend Karthik Nayak
2026-10-07  5:50   ` Patrick Steinhardt
2026-10-08  8:35 ` [PATCH v2 " Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 01/13] commit-graph: require resolved packfile paths for `stdin_packs` Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 02/13] commit-graph: stop depending on `struct odb_source` Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 03/13] odb/source-files: introduce `struct odb_files_dir` Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 04/13] odb: refactor `odb_for_each_alternate()` to yield dirs Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 05/13] odb: refactor `odb_find_source()` " Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 06/13] odb/source-files: add the ability to have multiple object dirs Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 07/13] tmp-objdir: absorb logic to set and restore primary sources Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 08/13] tmp-objdir: manage quarantine as an object directory Patrick Steinhardt
2026-10-08  8:35   ` [PATCH v2 09/13] tmp-objdir: replace primary source at creation time Patrick Steinhardt
2026-10-08  8:36   ` [PATCH v2 10/13] odb/source: make `will_destroy` an implementation detail Patrick Steinhardt
2026-10-08  8:36   ` [PATCH v2 11/13] odb/source-files: extract reading alternates Patrick Steinhardt
2026-10-08  8:36   ` [PATCH v2 12/13] odb/source-files: move alternates into the backend Patrick Steinhardt
2026-10-08  8:36   ` [PATCH v2 13/13] odb/source: drop `read_alternates` callback Patrick Steinhardt
2026-10-08  9:24   ` [PATCH v2 00/13] odb/source-files: move alternates into the backend Karthik Nayak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox