Thread (9 messages) 9 messages, 3 authors, 2026-09-03

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help