[PATCH v2 00/13] odb: stop registering in-memory sources

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]




Hi,
the object database has a list of sources that is used for two
different purposes:
 - We use it to track the list of alternates.
 - We use it to track temporary in-memory sources that we create for
 various purposes. Most importantly, this is used to link object
 database sources from submodules into the main store.
This dual-use is quite awkward, as it mixes two different levels of
concerns and thus as a consequence makes both harder to reason about.
It's also a source of bugs: we make assumptions about the ordering of
sources all over the place, and we furthermore assume in other places
that the sources only contain alternates in the first place. I don't
think this surfaces in the form of real bugs, but I've long disliked
this dual-use.
Furthermore, we want to migrate handling of alternates into the "files"
backend itself in a subsequent patch series. This is most importantly to
fix a performance regression by making the backend own all of its
alternates, but it also fixes a couple of longer-standing design issues
that I've been struggling with [1].
Most importantly though: this whole machinery is not even needed at all.
A couple years ago we have already refactored our codebase so that
submodule sources don't even have to be linked into the main object
database anymore. And all the other use cases where we link sources into
the main object database can be trivially converted, too.
So this patch series does exactly that: it removes the mechanism to link
ad-hoc sources into the object database entirely. This ensures that the
list of sources is exactly the list of alternates, and that makes it
easier to move them into the "files" backend in a subsequent patch
series.
There is one exception though: creating transactions still creates a
temporary quarantine directory. This mechanism is left as-is for now,
but as it's an implementation detail of the "files" backend anyway
that's not conflicting with our above stated goals.
This series is built on top of 1630431f32 (The 21st batch, 2026年08月31日)
with ty/repository-fetch-if-missing at 508ec9837c (repository: move
fetch_if_missing into struct repository, 2026年08月15日) merged into it.
There's still two merge conflicts, but these are trivial to resolve: in
"odb.c" and "odb.h" you simply remove both ours and theirs, and in
"builtin/multi-pack-index.c" you only need to munge the parameters a
bit.
Changes in v2:
 - Adapt `cache_tree_fully_valid()` to take a `struct index_state` as
 input instead of taking both a repository and a cache tree, as
 suggested by Junio.
 - Link to v1: https://patch.msgid.link/20260901-pks-odb-registering-in-memory-sources-v1-0-97a312d5fa25@xxxxxx
Thanks!
Patrick
[1]: <amLgMqkqxR8mKIbT@xxxxxx>
---
Patrick Steinhardt (13):
 cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
 cache-tree: remove dependency on `the_repository`
 submodule-config: remove uses of `the_repository`
 submodule-config: stop using `the_hash_algo`
 submodule-config: stop registering submodule sources
 builtin/grep: stop registering submodule ODB as source
 odb: remove infrastructure to register submodule sources
 tmp-objdir: drop unused function to register alternate
 odb/packed: fix memory leaks when freeing source
 builtin/multi-pack-index: refuse unknown sources with "--object-dir="
 t/helper: adapt read-midx to not link ad-hoc source anymore
 t/helper: stop registering alternates in "ref-store" command
 odb: remove the ability to link sources ad-hoc
 builtin/checkout.c | 2 +-
 builtin/commit.c | 2 +-
 builtin/fetch.c | 2 +-
 builtin/grep.c | 28 +++-------
 builtin/multi-pack-index.c | 3 +-
 builtin/submodule--helper.c | 8 +--
 cache-tree.c | 95 +++++++++++++++++++---------------
 cache-tree.h | 7 +--
 odb.c | 42 ---------------
 odb.h | 22 --------
 odb/source-packed.c | 1 +
 read-cache-ll.h | 5 +-
 read-cache.c | 9 ++--
 sequencer.c | 2 +-
 sparse-index.c | 2 +-
 submodule-config.c | 59 +++++++++++----------
 submodule-config.h | 12 +++--
 submodule.c | 2 +-
 t/README | 7 ---
 t/helper/test-read-midx.c | 43 ++++++++++-----
 t/helper/test-ref-store.c | 8 ---
 t/helper/test-submodule.c | 4 +-
 t/t5319-multi-pack-index.sh | 9 ++--
 t/t5526-fetch-submodules.sh | 3 --
 t/t5531-deep-submodule-push.sh | 3 --
 t/t5545-push-options.sh | 3 --
 t/t5572-pull-submodule.sh | 3 --
 t/t6437-submodule-merge.sh | 3 --
 t/t7418-submodule-sparse-gitmodules.sh | 3 --
 t/t7814-grep-recurse-submodules.sh | 3 --
 tmp-objdir.c | 5 --
 tmp-objdir.h | 6 ---
 unpack-trees.c | 9 ++--
 33 files changed, 168 insertions(+), 247 deletions(-)
Range-diff versus v1:
 -: ---------- > 1: 4870d0661d cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
 1: a4e2648526 ! 2: 755652bfe1 cache-tree: remove dependency on `the_repository`
 @@ Commit message
 
 Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
 
 - ## builtin/checkout.c ##
 -@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
 - 		}
 - 	}
 - 
 --	if (!cache_tree_fully_valid(the_repository->index->cache_tree))
 -+	if (!cache_tree_fully_valid(the_repository, the_repository->index->cache_tree))
 - 		cache_tree_update(the_repository->index, WRITE_TREE_SILENT | WRITE_TREE_REPAIR);
 - 
 - 	if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK))
 -
 - ## builtin/commit.c ##
 -@@ builtin/commit.c: static const char *prepare_index(const char **argv, const char *prefix,
 - 				 LOCK_DIE_ON_ERROR);
 - 		refresh_cache_or_die(refresh_flags);
 - 		if (the_repository->index->cache_changed
 --		 || !cache_tree_fully_valid(the_repository->index->cache_tree))
 -+		 || !cache_tree_fully_valid(the_repository, the_repository->index->cache_tree))
 - 			cache_tree_update(the_repository->index, WRITE_TREE_SILENT);
 - 		if (write_locked_index(the_repository->index, &index_lock,
 - 				 COMMIT_LOCK | SKIP_IF_UNCHANGED))
 -
 ## cache-tree.c ##
 @@
 -#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
 -@@ cache-tree.c: static void discard_unused_subtrees(struct cache_tree *it)
 - 	}
 - }
 - 
 --int cache_tree_fully_valid(struct cache_tree *it)
 -+int cache_tree_fully_valid(struct repository *repo, struct cache_tree *it)
 - {
 - 	int i;
 - 	if (!it)
 - 		return 0;
 - 	if (it->entry_count < 0 ||
 --	 !odb_has_object(the_repository->objects, &it->oid,
 -+	 !odb_has_object(repo->objects, &it->oid,
 - 			 ODB_HAS_OBJECT_RECHECK_PACKED | ODB_HAS_OBJECT_FETCH_PROMISOR))
 - 		return 0;
 - 	for (i = 0; i < it->subtree_nr; i++) {
 --		if (!cache_tree_fully_valid(it->down[i]->cache_tree))
 -+		if (!cache_tree_fully_valid(repo, it->down[i]->cache_tree))
 - 			return 0;
 - 	}
 - 	return 1;
 +@@ cache-tree.c: int cache_tree_fully_valid(struct index_state *istate)
 + 						istate->cache_tree);
 }
 
 -static int must_check_existence(const struct cache_entry *ce)
 @@ cache-tree.c: static struct cache_tree *read_one(const char **buffer, unsigned l
 
 	return result;
 }
 -@@ cache-tree.c: struct tree *write_in_core_index_as_tree(struct repository *repo,
 - 	int was_valid, ret;
 - 
 - 	was_valid = index_state->cache_tree &&
 --		 cache_tree_fully_valid(index_state->cache_tree);
 -+		 cache_tree_fully_valid(repo, index_state->cache_tree);
 - 
 - 	ret = write_index_as_tree_internal(&o, index_state, was_valid, 0, NULL);
 - 	if (ret == WRITE_TREE_UNMERGED_INDEX) {
 @@ cache-tree.c: int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
 	hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
 
 @@ cache-tree.c: int write_index_as_tree(struct object_id *oid, struct index_state
 	if (entries < 0) {
 		ret = WRITE_TREE_UNREADABLE_INDEX;
 		goto out;
 -@@ cache-tree.c: int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
 - 
 - 	was_valid = !(flags & WRITE_TREE_IGNORE_CACHE_TREE) &&
 - 		 index_state->cache_tree &&
 --		 cache_tree_fully_valid(index_state->cache_tree);
 -+		 cache_tree_fully_valid(index_state->repo, index_state->cache_tree);
 - 
 - 	ret = write_index_as_tree_internal(oid, index_state, was_valid, flags,
 - 					 prefix);
 @@ cache-tree.c: static void prime_cache_tree_rec(struct repository *r,
 			struct cache_tree_sub *sub;
 			struct tree *subtree = lookup_tree(r, &entry.oid);
 @@ cache-tree.h: struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const c
 +struct cache_tree *cache_tree_read(struct repository *repo,
 +				 const char *buffer, unsigned long size);
 
 --int cache_tree_fully_valid(struct cache_tree *);
 -+int cache_tree_fully_valid(struct repository *, struct cache_tree *);
 + int cache_tree_fully_valid(struct index_state *);
 int cache_tree_update(struct index_state *, int);
 - int cache_tree_verify(struct repository *, struct index_state *);
 - 
 
 ## read-cache-ll.h ##
 @@ read-cache-ll.h: void validate_cache_entries(const struct index_state *istate);
 @@ read-cache.c: void prefetch_cache_entries(const struct index_state *istate,
 		if (!odb_read_object_info_extended(the_repository->objects,
 						 &ce->oid, NULL,
 
 - ## sequencer.c ##
 -@@ sequencer.c: static int do_recursive_merge(struct repository *r,
 - 
 - static struct object_id *get_cache_tree_oid(struct index_state *istate)
 - {
 --	if (!cache_tree_fully_valid(istate->cache_tree))
 -+	if (!cache_tree_fully_valid(istate->repo, istate->cache_tree))
 - 		if (cache_tree_update(istate, 0)) {
 - 			error(_("unable to update cache tree"));
 - 			return NULL;
 -
 - ## sparse-index.c ##
 -@@ sparse-index.c: int convert_to_sparse(struct index_state *istate, int flags)
 - 	if (index_has_unmerged_entries(istate))
 - 		return 0;
 - 
 --	if (!cache_tree_fully_valid(istate->cache_tree)) {
 -+	if (!cache_tree_fully_valid(istate->repo, istate->cache_tree)) {
 - 		/* Clear and recompute the cache-tree */
 - 		cache_tree_free(&istate->cache_tree);
 - 
 -
 ## unpack-trees.c ##
 @@ unpack-trees.c: static void report_collided_checkout(struct index_state *index)
 	string_list_clear(&list, 0);
 @@ unpack-trees.c: static int check_updates(struct unpack_trees_options *o,
 			size_t last_pc_queue_size = pc_queue_size();
 
 			if (ce->ce_flags & CE_WT_REMOVE)
 -@@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 - 			}
 - 
 - 			if (!o->skip_cache_tree_update &&
 --			 !cache_tree_fully_valid(o->internal.result.cache_tree))
 -+			 !cache_tree_fully_valid(the_repository, o->internal.result.cache_tree))
 - 				cache_tree_update(&o->internal.result,
 - 						 WRITE_TREE_SILENT |
 - 						 WRITE_TREE_REPAIR);
 2: bedb614b5c = 3: 46bbe548a6 submodule-config: remove uses of `the_repository`
 3: 8fa02cdd5c = 4: 65f8634bda submodule-config: stop using `the_hash_algo`
 4: ebf1dfb548 = 5: 4faf4fdd57 submodule-config: stop registering submodule sources
 5: dfe1a212f7 = 6: dee5aca311 builtin/grep: stop registering submodule ODB as source
 6: 1644e49419 = 7: 2ce128799a odb: remove infrastructure to register submodule sources
 7: 3f3ccfba74 = 8: 0a174f6304 tmp-objdir: drop unused function to register alternate
 8: f1341b797b = 9: d22c376cd2 odb/packed: fix memory leaks when freeing source
 9: c1f6d9ebdd = 10: 09405ade4c builtin/multi-pack-index: refuse unknown sources with "--object-dir="
10: c5b9c1c96e = 11: 557b3ab923 t/helper: adapt read-midx to not link ad-hoc source anymore
11: 3f43f73cd8 = 12: 78b26c172a t/helper: stop registering alternates in "ref-store" command
12: 4c670d726f = 13: 1ca49caf1c odb: remove the ability to link sources ad-hoc
---
base-commit: e5d60560f61f520e9ea350645a6cc9770b0f1607
change-id: 20260811-pks-odb-registering-in-memory-sources-88648cd95735

[Index of Archives] [Linux Kernel Development] [Gcc Help] [IETF Annouce] [DCCP] [Netdev] [Networking] [Security] [V4L] [Bugtraq] [Yosemite] [MIPS Linux] [ARM Linux] [Linux Security] [Linux RAID] [Linux SCSI] [Fedora Users]

(追記) (追記ここまで)
Powered by Linux

AltStyle によって変換されたページ (->オリジナル) /