Re: [PATCH 1/2] repository: make repo_clear() idempotent
flat view
From: Jeff King <hidden>
Date: 2026-09-02 06:29:41
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Wed, Sep 02, 2026 at 01:55:27AM -0400, Jeff King wrote:
Arguably this should not be a pointer at all, but the pool code is weirdly asymmetric. It offers only "new" which allocates a struct, but only "clear" to clean it up (but not deallocate). Might be worth fixing, but out of scope for this series.
I took a quick stab at this, and it gets ugly. There is no mutual
recursion between the parsed_object_pool and repository struct
definitions, but we do end up in a header include loop:
- repository.h would need object.h (to include the pool struct)
- object.h includes hash.h for object_id, etc
- hash.h (sometimes) includes repository.h so it can define
the_hash_algo when USE_THE_REPOSITORY_VARIABLE is defined
We could break the cycle if we had a separate the-repository.h which
looked like this:
struct repository;
extern struct repository *the_repository;
and then included that from hash.h. But then callers which want to use
the_hash_algo would need to include repository.h themselves. It is
just a macro looking at the_repository->hash_algo, so they need the
actual repository definition. About 9 files need to start including
repository.h themselves to make it work. Though a few of them _ought_ to
be including it anyway; they are not using the_hash_algo at all, but
just lucky that hash.h happens to bring repository.h when
USE_THE_REPOSITORY_VARIABLE is set.
An alternative would be to define the_hash_algo as its own pointer,
like:
diff --git a/hash.h b/hash.h
index cf94ad5700..9e21ac6480 100644
--- a/hash.h
+++ b/hash.h@@ -269,8 +269,7 @@ enum get_oid_result { }; #ifdef USE_THE_REPOSITORY_VARIABLE -# include "repository.h" -# define the_hash_algo the_repository->hash_algo +extern struct git_hash_algo *the_hash_algo; #endif /* A suitably aligned type for stack allocations of hash contexts. */
diff --git a/repository.c b/repository.c
index db4f9d006e..f70c4deecf 100644
--- a/repository.c
+++ b/repository.c@@ -30,6 +30,7 @@ extern struct repository *the_repository; /* The main repository */ static struct repository the_repo; struct repository *the_repository = &the_repo; +struct the_hash_algo = &the_repo->hash_algo; /* * An escape hatch: if we hit a bug in the production code that fails
That makes the_hash_algo just work without most code caring about repositories at all. But of course it reveals yet more spots which are relying on hash.h mentioning the_repository. :-/ I'm not sure how much it's worth untangling all of this, but probably not enough just to remove pointer indirection from repo->parsed_objects. -Peff