threads / patch / 62424

v3, 9 partspackfile: add repository to struct `packed_git`

Subject: [PATCH v3 1/9] packfile: add repository to struct `packed_git`

## tl;dr

13 messages between Oct 30, 2024 and Oct 31, 2024. Diffs are folded; open one to read it.

replies: 12people: 2as markdown or json

Karthik Nayak· Oct 30, 2024, 14:32 UTC · lore

[PATCH v3 0/9] packfile: avoid using the 'the_repository' global variable

The `packfile.c` file uses the global variable 'the_repository' extensively throughout the code. Let's remove all usecases of this, by modifying the required functions to accept a 'struct repository' instead. This is to clean up usage of global state.

The first 3 patches are mostly internal to `packfile.c`, we add the repository field to the `packed_git` struct and this is used to clear up some useages of the global variables.

The next 3 patches are more disruptive, they modify the function definition of `odb_pack_name`, `has_object[_kept]_pack` and `for_each_packed_object` to receive a repository, helping remove other usages of 'the_repository' variable.

Finally, the last two patches deal with global config values. These values are localized.

This series is based off on master: 6a11438f43 (The fifth batch, 2024-10-25), with 'jk/dumb-http-finalize' merged in. I found no issues merging this with seen, but since these patches cover a lot of files, there might be some conflicts.

Changes in v3:
- Improved commit messages. In the first commit to talk about how packed_git
struct could also be part of the alternates of a repository. In the 7th commit
to talk about the motive behind removing the global variable.
- Changed 'packed_git->repo' to 'packed_git->r' to keep it consistent with the
rest of the code base.
- Replaced 'the_repository' with locally available access to the repository
struct in multiple regions.
- Removed unecessary inclusion of the 'repository.h' header file by forward
declaring the 'repository' struct.
- Replace memcpy with hashcpy.
- Change the logic in the 7th patch to use if else statements.
- Added an extra commit to cleanup `pack-bitmap.c`. 
Karthik Nayak (9):
  packfile: add repository to struct `packed_git`
  packfile: use `repository` from `packed_git` directly
  packfile: pass `repository` to static function in the file
  packfile: pass down repository to `odb_pack_name`
  packfile: pass down repository to `has_object[_kept]_pack`
  packfile: pass down repository to `for_each_packed_object`
  config: make `delta_base_cache_limit` a non-global variable
  config: make `packed_git_(limit|window_size)` non-global variables
  midx: add repository to `multi_pack_index` struct
 builtin/cat-file.c       |   7 +-
 builtin/count-objects.c  |   2 +-
 builtin/fast-import.c    |  15 ++--
 builtin/fsck.c           |  20 +++--
 builtin/gc.c             |   5 +-
 builtin/index-pack.c     |  20 +++--
 builtin/pack-objects.c   |  11 ++-
 builtin/pack-redundant.c |   2 +-
 builtin/repack.c         |   2 +-
 builtin/rev-list.c       |   2 +-
 commit-graph.c           |   4 +-
 config.c                 |  22 -----
 connected.c              |   3 +-
 diff.c                   |   3 +-
 environment.c            |   3 -
 environment.h            |   1 -
 fsck.c                   |   2 +-
 http.c                   |   4 +-
 list-objects.c           |   7 +-
 midx-write.c             |   2 +-
 midx.c                   |   3 +-
 midx.h                   |   3 +
 object-store-ll.h        |   9 +-
 pack-bitmap.c            |  97 +++++++++++++--------
 pack-objects.h           |   3 +-
 pack-write.c             |   1 +
 pack.h                   |   1 +
 packfile.c               | 182 ++++++++++++++++++++++++++-------------
 packfile.h               |  18 ++--
 promisor-remote.c        |   2 +-
 prune-packed.c           |   2 +-
 reachable.c              |   4 +-
 revision.c               |  13 +--
 tag.c                    |   2 +-
 34 files changed, 285 insertions(+), 192 deletions(-)
Range-diff against v2:
 1:  26d2461cc3 !  1:  5afb9af0af packfile: add repository to struct `packed_git`
    @@ Commit message
         on the global `the_repository` object in `packfile.c` by simply using
         repository information now readily available in the struct.
     
    +    We do need to consider that a pack file could be part of the alternates
    +    of a repository, but considering that we only have one repository struct
    +    and also that we currently anyways use 'the_repository'. We should be
    +    OK with this change.
    +
         We also modify `alloc_packed_git` to ensure that the repository is added
         to newly created `packed_git` structs. This requires modifying the
         function and all its callee to pass the repository object down the
         levels.
     
    +    Helped-by: Taylor Blau <me@ttaylorr.com>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## builtin/fast-import.c ##
    @@ builtin/fast-import.c: static void start_packfile(void)
      
      	p->pack_fd = pack_fd;
      	p->do_not_close = 1;
    -+	p->repo = the_repository;
    ++	p->r = the_repository;
      	pack_file = hashfd(pack_fd, p->pack_name);
      
      	pack_data = p;
    @@ builtin/fast-import.c: static void end_packfile(void)
      
      		/* Register the packfile with core git's machinery. */
     -		new_p = add_packed_git(idx_name, strlen(idx_name), 1);
    -+		new_p = add_packed_git(the_repository, idx_name, strlen(idx_name), 1);
    ++		new_p = add_packed_git(pack_data->r, idx_name, strlen(idx_name), 1);
      		if (!new_p)
      			die("core git rejected index %s", idx_name);
      		all_packs[pack_id] = new_p;
    @@ midx.c: int prepare_midx_pack(struct repository *r, struct multi_pack_index *m,
     
      ## object-store-ll.h ##
     @@
    - #include "hashmap.h"
    - #include "object.h"
    - #include "list.h"
    -+#include "repository.h"
    - #include "thread-utils.h"
    - #include "oidset.h"
    + struct oidmap;
    + struct oidtree;
    + struct strbuf;
    ++struct repository;
      
    + struct object_directory {
    + 	struct object_directory *next;
     @@ object-store-ll.h: struct packed_git {
      	 */
      	const uint32_t *mtimes_map;
      	size_t mtimes_size;
     +
     +	/* repo dentoes the repository this packed file belongs to */
    -+	struct repository *repo;
    ++	struct repository *r;
     +
      	/* something like ".git/objects/pack/xxxxx.pack" */
      	char pack_name[FLEX_ARRAY]; /* more */
    @@ packfile.c: uint32_t get_pack_fanout(struct packed_git *p, uint32_t value)
      }
      
     -static struct packed_git *alloc_packed_git(int extra)
    -+static struct packed_git *alloc_packed_git(struct repository *repo, int extra)
    ++static struct packed_git *alloc_packed_git(struct repository *r, int extra)
      {
      	struct packed_git *p = xmalloc(st_add(sizeof(*p), extra));
      	memset(p, 0, sizeof(*p));
      	p->pack_fd = -1;
    -+	p->repo = repo;
    ++	p->r = r;
      	return p;
      }
      
    @@ packfile.c: static char *pack_path_from_idx(const char *idx_path)
      }
      
     -struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path)
    -+struct packed_git *parse_pack_index(struct repository *repo,
    -+				    unsigned char *sha1, const char *idx_path)
    ++struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1,
    ++				    const char *idx_path)
      {
      	char *path = pack_path_from_idx(idx_path);
      	size_t alloc = st_add(strlen(path), 1);
     -	struct packed_git *p = alloc_packed_git(alloc);
    -+	struct packed_git *p = alloc_packed_git(repo, alloc);
    ++	struct packed_git *p = alloc_packed_git(r, alloc);
      
      	memcpy(p->pack_name, path, alloc); /* includes NUL */
      	free(path);
    @@ packfile.c: void unuse_pack(struct pack_window **w_cursor)
      }
      
     -struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
    -+struct packed_git *add_packed_git(struct repository *repo, const char *path,
    ++struct packed_git *add_packed_git(struct repository *r, const char *path,
     +				  size_t path_len, int local)
      {
      	struct stat st;
    @@ packfile.c: struct packed_git *add_packed_git(const char *path, size_t path_len,
      	 */
      	alloc = st_add3(path_len, strlen(".promisor"), 1);
     -	p = alloc_packed_git(alloc);
    -+	p = alloc_packed_git(repo, alloc);
    ++	p = alloc_packed_git(r, alloc);
      	memcpy(p->pack_name, path, path_len);
      
      	xsnprintf(p->pack_name + path_len, alloc - path_len, ".keep");
    @@ packfile.h: const char *pack_basename(struct packed_git *p);
       * packs. You probably want add_packed_git() instead.
       */
     -struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
    -+struct packed_git *parse_pack_index(struct repository *repo,
    -+				    unsigned char *sha1, const char *idx_path);
    ++struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1,
    ++				    const char *idx_path);
      
      typedef void each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
      				      const char *file_name, void *data);
    @@ packfile.h: void close_pack(struct packed_git *);
      void unuse_pack(struct pack_window **);
      void clear_delta_base_cache(void);
     -struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
    -+struct packed_git *add_packed_git(struct repository *repo, const char *path,
    ++struct packed_git *add_packed_git(struct repository *r, const char *path,
     +				  size_t path_len, int local);
      
      /*
 2:  ca03355686 !  2:  5350b4f9fb packfile: use `repository` from `packed_git` directly
    @@ Commit message
         where there is no struct `packed_git` locally available, which will be
         fixed in the following commits.
     
    +    Helped-by: Taylor Blau <me@ttaylorr.com>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## packfile.c ##
    @@ packfile.c: static int check_packed_git_idx(const char *path, struct packed_git
      	int fd = git_open(path), ret;
      	struct stat st;
     -	const unsigned int hashsz = the_hash_algo->rawsz;
    -+	const unsigned int hashsz = p->repo->hash_algo->rawsz;
    ++	const unsigned int hashsz = p->r->hash_algo->rawsz;
      
      	if (fd < 0)
      		return -1;
    -@@ packfile.c: struct packed_git *parse_pack_index(struct repository *repo,
    +@@ packfile.c: struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1,
      
      	memcpy(p->pack_name, path, alloc); /* includes NUL */
      	free(path);
     -	hashcpy(p->hash, sha1, the_repository->hash_algo);
    -+	hashcpy(p->hash, sha1, p->repo->hash_algo);
    ++	hashcpy(p->hash, sha1, p->r->hash_algo);
      	if (check_packed_git_idx(idx_path, p)) {
      		free(p);
      		return NULL;
    @@ packfile.c: static int unuse_one_window(struct packed_git *current)
      	if (current)
      		scan_windows(current, &lru_p, &lru_w, &lru_l);
     -	for (p = the_repository->objects->packed_git; p; p = p->next)
    -+	for (p = current->repo->objects->packed_git; p; p = p->next)
    ++	for (p = current->r->objects->packed_git; p; p = p->next)
      		scan_windows(p, &lru_p, &lru_w, &lru_l);
      	if (lru_p) {
      		munmap(lru_w->base, lru_w->len);
    @@ packfile.c: static int open_packed_git_1(struct packed_git *p)
      	unsigned char *idx_hash;
      	ssize_t read_result;
     -	const unsigned hashsz = the_hash_algo->rawsz;
    -+	const unsigned hashsz = p->repo->hash_algo->rawsz;
    ++	const unsigned hashsz = p->r->hash_algo->rawsz;
      
      	if (open_pack_index(p))
      		return error("packfile %s index unavailable", p->pack_name);
    @@ packfile.c: static int open_packed_git_1(struct packed_git *p)
      		return error("packfile %s signature is unavailable", p->pack_name);
      	idx_hash = ((unsigned char *)p->index_data) + p->index_size - hashsz * 2;
     -	if (!hasheq(hash, idx_hash, the_repository->hash_algo))
    -+	if (!hasheq(hash, idx_hash, p->repo->hash_algo))
    ++	if (!hasheq(hash, idx_hash, p->r->hash_algo))
      		return error("packfile %s does not match index", p->pack_name);
      	return 0;
      }
    @@ packfile.c: unsigned char *use_pack(struct packed_git *p,
      	if (!p->pack_size && p->pack_fd == -1 && open_packed_git(p))
      		die("packfile %s cannot be accessed", p->pack_name);
     -	if (offset > (p->pack_size - the_hash_algo->rawsz))
    -+	if (offset > (p->pack_size - p->repo->hash_algo->rawsz))
    ++	if (offset > (p->pack_size - p->r->hash_algo->rawsz))
      		die("offset beyond end of packfile (truncated pack?)");
      	if (offset < 0)
      		die(_("offset before end of packfile (broken .idx?)"));
    -@@ packfile.c: struct packed_git *add_packed_git(struct repository *repo, const char *path,
    +@@ packfile.c: struct packed_git *add_packed_git(struct repository *r, const char *path,
      	struct stat st;
      	size_t alloc;
      	struct packed_git *p;
    @@ packfile.c: struct packed_git *add_packed_git(struct repository *repo, const cha
      
      	/*
      	 * Make sure a corresponding .pack file exists and that
    -@@ packfile.c: struct packed_git *add_packed_git(struct repository *repo, const char *path,
    +@@ packfile.c: struct packed_git *add_packed_git(struct repository *r, const char *path,
      	p->pack_size = st.st_size;
      	p->pack_local = local;
      	p->mtime = st.st_mtime;
     -	if (path_len < the_hash_algo->hexsz ||
     -	    get_hash_hex(path + path_len - the_hash_algo->hexsz, p->hash))
     -		hashclr(p->hash, the_repository->hash_algo);
    -+	if (path_len < repo->hash_algo->hexsz ||
    -+	    get_oid_hex_algop(path + path_len - repo->hash_algo->hexsz, &oid,
    -+			      repo->hash_algo))
    -+		hashclr(p->hash, repo->hash_algo);
    ++	if (path_len < r->hash_algo->hexsz ||
    ++	    get_oid_hex_algop(path + path_len - r->hash_algo->hexsz, &oid,
    ++			      r->hash_algo))
    ++		hashclr(p->hash, r->hash_algo);
     +	else
    -+		memcpy(p->hash, oid.hash, repo->hash_algo->rawsz);
    ++		hashcpy(p->hash, oid.hash, r->hash_algo);
     +
      	return p;
      }
    @@ packfile.c: off_t get_delta_base(struct packed_git *p,
      		/* The base entry _must_ be in the same pack */
      		struct object_id oid;
     -		oidread(&oid, base_info, the_repository->hash_algo);
    -+		oidread(&oid, base_info, p->repo->hash_algo);
    ++		oidread(&oid, base_info, p->r->hash_algo);
      		base_offset = find_pack_entry_one(&oid, p);
     -		*curpos += the_hash_algo->rawsz;
    -+		*curpos += p->repo->hash_algo->rawsz;
    ++		*curpos += p->r->hash_algo->rawsz;
      	} else
      		die("I am totally screwed");
      	return base_offset;
    @@ packfile.c: static int get_delta_base_oid(struct packed_git *p,
      	if (type == OBJ_REF_DELTA) {
      		unsigned char *base = use_pack(p, w_curs, curpos, NULL);
     -		oidread(oid, base, the_repository->hash_algo);
    -+		oidread(oid, base, p->repo->hash_algo);
    ++		oidread(oid, base, p->r->hash_algo);
      		return 0;
      	} else if (type == OBJ_OFS_DELTA) {
      		uint32_t base_pos;
    @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p,
      			}
      		} else
     -			oidclr(oi->delta_base_oid, the_repository->hash_algo);
    -+			oidclr(oi->delta_base_oid, p->repo->hash_algo);
    ++			oidclr(oi->delta_base_oid, p->r->hash_algo);
      	}
      
      	oi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :
    @@ packfile.c: int bsearch_pack(const struct object_id *oid, const struct packed_gi
      	const unsigned char *index_fanout = p->index_data;
      	const unsigned char *index_lookup;
     -	const unsigned int hashsz = the_hash_algo->rawsz;
    -+	const unsigned int hashsz = p->repo->hash_algo->rawsz;
    ++	const unsigned int hashsz = p->r->hash_algo->rawsz;
      	int index_lookup_width;
      
      	if (!index_fanout)
    @@ packfile.c: int nth_packed_object_id(struct object_id *oid,
      {
      	const unsigned char *index = p->index_data;
     -	const unsigned int hashsz = the_hash_algo->rawsz;
    -+	const unsigned int hashsz = p->repo->hash_algo->rawsz;
    ++	const unsigned int hashsz = p->r->hash_algo->rawsz;
      	if (!index) {
      		if (open_pack_index(p))
      			return -1;
    @@ packfile.c: int nth_packed_object_id(struct object_id *oid,
      	if (p->index_version == 1) {
      		oidread(oid, index + st_add(st_mult(hashsz + 4, n), 4),
     -			the_repository->hash_algo);
    -+			p->repo->hash_algo);
    ++			p->r->hash_algo);
      	} else {
      		index += 8;
     -		oidread(oid, index + st_mult(hashsz, n),
     -			the_repository->hash_algo);
    -+		oidread(oid, index + st_mult(hashsz, n), p->repo->hash_algo);
    ++		oidread(oid, index + st_mult(hashsz, n), p->r->hash_algo);
      	}
      	return 0;
      }
    @@ packfile.c: void check_pack_index_ptr(const struct packed_git *p, const void *vp
      {
      	const unsigned char *index = p->index_data;
     -	const unsigned int hashsz = the_hash_algo->rawsz;
    -+	const unsigned int hashsz = p->repo->hash_algo->rawsz;
    ++	const unsigned int hashsz = p->r->hash_algo->rawsz;
      	index += 4 * 256;
      	if (p->index_version == 1) {
      		return ntohl(*((uint32_t *)(index + st_mult(hashsz + 4, n))));
    @@ packfile.c: int for_each_object_in_pack(struct packed_git *p,
      
      	if (flags & FOR_EACH_OBJECT_PACK_ORDER) {
     -		if (load_pack_revindex(the_repository, p))
    -+		if (load_pack_revindex(p->repo, p))
    ++		if (load_pack_revindex(p->r, p))
      			return -1;
      	}
      
    @@ packfile.c: static int add_promisor_object(const struct object_id *oid,
      	int we_parsed_object;
      
     -	obj = lookup_object(the_repository, oid);
    -+	obj = lookup_object(pack->repo, oid);
    ++	obj = lookup_object(pack->r, oid);
      	if (obj && obj->parsed) {
      		we_parsed_object = 0;
      	} else {
      		we_parsed_object = 1;
     -		obj = parse_object(the_repository, oid);
    -+		obj = parse_object(pack->repo, oid);
    ++		obj = parse_object(pack->r, oid);
      	}
      
      	if (!obj)
 -:  ---------- >  3:  5b975cb6d6 packfile: pass `repository` to static function in the file
 4:  73ba9945a7 !  4:  13a166fcca packfile: pass down repository to `odb_pack_name`
    @@ builtin/fast-import.c: static char *keep_pack(const char *curr_index_name)
      	int keep_fd;
      
     -	odb_pack_name(&name, pack_data->hash, "keep");
    -+	odb_pack_name(the_repository, &name, pack_data->hash, "keep");
    ++	odb_pack_name(pack_data->r, &name, pack_data->hash, "keep");
      	keep_fd = odb_pack_keep(name.buf);
      	if (keep_fd < 0)
      		die_errno("cannot create keep file");
    @@ builtin/fast-import.c: static char *keep_pack(const char *curr_index_name)
      		die_errno("failed to write keep file");
      
     -	odb_pack_name(&name, pack_data->hash, "pack");
    -+	odb_pack_name(the_repository, &name, pack_data->hash, "pack");
    ++	odb_pack_name(pack_data->r, &name, pack_data->hash, "pack");
      	if (finalize_object_file(pack_data->pack_name, name.buf))
      		die("cannot store pack file");
      
     -	odb_pack_name(&name, pack_data->hash, "idx");
    -+	odb_pack_name(the_repository, &name, pack_data->hash, "idx");
    ++	odb_pack_name(pack_data->r, &name, pack_data->hash, "idx");
      	if (finalize_object_file(curr_index_name, name.buf))
      		die("cannot store index file");
      	free((void *)curr_index_name);
    @@ builtin/fast-import.c: static void unkeep_all_packs(void)
      	for (k = 0; k < pack_id; k++) {
      		struct packed_git *p = all_packs[k];
     -		odb_pack_name(&name, p->hash, "keep");
    -+		odb_pack_name(p->repo, &name, p->hash, "keep");
    ++		odb_pack_name(p->r, &name, p->hash, "keep");
      		unlink_or_warn(name.buf);
      	}
      	strbuf_release(&name);
    @@ builtin/index-pack.c: static void rename_tmp_packfile(const char **final_name,
      			    ext, *final_name);
     
      ## builtin/pack-redundant.c ##
    -@@ builtin/pack-redundant.c: static void load_all(void)
    - 	}
    - }
    - 
    --int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, struct repository *repo UNUSED) {
    -+int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, struct repository *repo) {
    - 	int i; int i_still_use_this = 0; struct pack_list *min = NULL, *red, *pl;
    - 	struct llist *ignore;
    - 	struct strbuf idx_name = STRBUF_INIT;
     @@ builtin/pack-redundant.c: int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s
      	pl = red = pack_list_difference(local_packs, min);
      	while (pl) {
      		printf("%s\n%s\n",
     -		       odb_pack_name(&idx_name, pl->pack->hash, "idx"),
    -+		       odb_pack_name(repo, &idx_name, pl->pack->hash, "idx"),
    ++		       odb_pack_name(pl->pack->r, &idx_name, pl->pack->hash, "idx"),
      		       pl->pack->pack_name);
      		pl = pl->next;
      	}
    @@ packfile.c
     -char *odb_pack_name(struct strbuf *buf,
     -		    const unsigned char *hash,
     -		    const char *ext)
    -+char *odb_pack_name(struct repository *repo, struct strbuf *buf,
    ++char *odb_pack_name(struct repository *r, struct strbuf *buf,
     +		    const unsigned char *hash, const char *ext)
      {
      	strbuf_reset(buf);
     -	strbuf_addf(buf, "%s/pack/pack-%s.%s", repo_get_object_directory(the_repository),
     -		    hash_to_hex(hash), ext);
    -+	strbuf_addf(buf, "%s/pack/pack-%s.%s", repo_get_object_directory(repo),
    -+		    hash_to_hex_algop(hash, repo->hash_algo), ext);
    ++	strbuf_addf(buf, "%s/pack/pack-%s.%s", repo_get_object_directory(r),
    ++		    hash_to_hex_algop(hash, r->hash_algo), ext);
      	return buf->buf;
      }
      
    @@ packfile.h: struct pack_entry {
       * Example: odb_pack_name(out, sha1, "idx") => ".git/objects/pack/pack-1234..idx"
       */
     -char *odb_pack_name(struct strbuf *buf, const unsigned char *sha1, const char *ext);
    -+char *odb_pack_name(struct repository *repo, struct strbuf *buf,
    ++char *odb_pack_name(struct repository *r, struct strbuf *buf,
     +		    const unsigned char *hash, const char *ext);
      
      /*
 5:  4e883a4d1c !  5:  1fac06f19e packfile: pass down repository to `has_object[_kept]_pack`
    @@ builtin/pack-objects.c: static int want_found_object(const struct object_id *oid
      		if (ignore_packed_keep_in_core && p->pack_keep_in_core)
      			return 0;
     -		if (has_object_kept_pack(oid, flags))
    -+		if (has_object_kept_pack(the_repository, oid, flags))
    ++		if (has_object_kept_pack(p->r, oid, flags))
      			return 0;
      	}
      
    @@ builtin/pack-objects.c: static void show_cruft_commit(struct commit *commit, voi
      static int cruft_include_check_obj(struct object *obj, void *data UNUSED)
      {
     -	return !has_object_kept_pack(&obj->oid, IN_CORE_KEEP_PACKS);
    -+	return !has_object_kept_pack(the_repository, &obj->oid, IN_CORE_KEEP_PACKS);
    ++	return !has_object_kept_pack(to_pack.repo, &obj->oid, IN_CORE_KEEP_PACKS);
      }
      
      static int cruft_include_check(struct commit *commit, void *data)
    @@ diff.c: static int reuse_worktree_file(struct index_state *istate,
      	 */
     -	if (!FAST_WORKING_DIRECTORY && !want_file && has_object_pack(oid))
     +	if (!FAST_WORKING_DIRECTORY && !want_file &&
    -+	    has_object_pack(the_repository, oid))
    ++	    has_object_pack(istate->repo, oid))
      		return 0;
      
      	/*
    @@ packfile.c: int find_kept_pack_entry(struct repository *r,
      }
      
     -int has_object_pack(const struct object_id *oid)
    -+int has_object_pack(struct repository *repo, const struct object_id *oid)
    ++int has_object_pack(struct repository *r, const struct object_id *oid)
      {
      	struct pack_entry e;
     -	return find_pack_entry(the_repository, oid, &e);
    -+	return find_pack_entry(repo, oid, &e);
    ++	return find_pack_entry(r, oid, &e);
      }
      
     -int has_object_kept_pack(const struct object_id *oid, unsigned flags)
    -+int has_object_kept_pack(struct repository *repo, const struct object_id *oid,
    ++int has_object_kept_pack(struct repository *r, const struct object_id *oid,
     +			 unsigned flags)
      {
      	struct pack_entry e;
     -	return find_kept_pack_entry(the_repository, oid, flags, &e);
    -+	return find_kept_pack_entry(repo, oid, flags, &e);
    ++	return find_kept_pack_entry(r, oid, flags, &e);
      }
      
      int for_each_object_in_pack(struct packed_git *p,
    @@ packfile.h: const struct packed_git *has_packed_and_bad(struct repository *, con
      
     -int has_object_pack(const struct object_id *oid);
     -int has_object_kept_pack(const struct object_id *oid, unsigned flags);
    -+int has_object_pack(struct repository *repo, const struct object_id *oid);
    -+int has_object_kept_pack(struct repository *repo, const struct object_id *oid,
    ++int has_object_pack(struct repository *r, const struct object_id *oid);
    ++int has_object_kept_pack(struct repository *r, const struct object_id *oid,
     +			 unsigned flags);
      
      /*
 6:  7c599e16f6 !  6:  a5fb3b1a4a packfile: pass down repository to `for_each_packed_object`
    @@ builtin/pack-objects.c: static void show_object__ma_allow_promisor(struct object
      	 */
     -	if (!has_object(the_repository, &obj->oid, 0) && is_promisor_object(&obj->oid))
     +	if (!has_object(the_repository, &obj->oid, 0) &&
    -+	    is_promisor_object(the_repository, &obj->oid))
    ++	    is_promisor_object(to_pack.repo, &obj->oid))
      		return;
      
      	show_object(obj, name, data);
    @@ builtin/pack-objects.c: static int add_object_in_unpacked_pack(const struct obje
      static void add_objects_in_unpacked_packs(void)
      {
     -	if (for_each_packed_object(add_object_in_unpacked_pack, NULL,
    -+	if (for_each_packed_object(the_repository,
    ++	if (for_each_packed_object(to_pack.repo,
     +				   add_object_in_unpacked_pack,
     +				   NULL,
      				   FOR_EACH_OBJECT_PACK_ORDER |
    @@ packfile.c: static int add_promisor_object(const struct object_id *oid,
      }
      
     -int is_promisor_object(const struct object_id *oid)
    -+int is_promisor_object(struct repository *repo, const struct object_id *oid)
    ++int is_promisor_object(struct repository *r, const struct object_id *oid)
      {
      	static struct oidset promisor_objects;
      	static int promisor_objects_prepared;
    @@ packfile.c: static int add_promisor_object(const struct object_id *oid,
      	if (!promisor_objects_prepared) {
     -		if (repo_has_promisor_remote(the_repository)) {
     -			for_each_packed_object(add_promisor_object,
    -+		if (repo_has_promisor_remote(repo)) {
    -+			for_each_packed_object(repo, add_promisor_object,
    ++		if (repo_has_promisor_remote(r)) {
    ++			for_each_packed_object(r, add_promisor_object,
      					       &promisor_objects,
      					       FOR_EACH_OBJECT_PROMISOR_ONLY |
      					       FOR_EACH_OBJECT_PACK_ORDER);
     
      ## packfile.h ##
    -@@ packfile.h: int has_object_kept_pack(struct repository *repo, const struct object_id *oid,
    +@@ packfile.h: int has_object_kept_pack(struct repository *r, const struct object_id *oid,
       * Return 1 if an object in a promisor packfile is or refers to the given
       * object, 0 otherwise.
       */
     -int is_promisor_object(const struct object_id *oid);
    -+int is_promisor_object(struct repository *repo, const struct object_id *oid);
    ++int is_promisor_object(struct repository *r, const struct object_id *oid);
      
      /*
       * Expose a function for fuzz testing.
 7:  89ee3f48ce !  7:  6e5951ceea config: make `delta_base_cache_limit` a non-global variable
    @@ Commit message
         this value from the repository config, since the value is only used once
         in the entire subsystem.
     
    +    These changes are made to remove the usage of `delta_base_cache_limit`
    +    as a global variable in `packfile.c`. This brings us one step closer to
    +    removing the `USE_THE_REPOSITORY_VARIABLE` definition in `packfile.c`
    +    which we complete in the next patch.
    +
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## builtin/gc.c ##
    @@ packfile.c
     +#include "config.h"
     +#include "pack-objects.h"
      
    - char *odb_pack_name(struct repository *repo, struct strbuf *buf,
    + char *odb_pack_name(struct repository *r, struct strbuf *buf,
      		    const unsigned char *hash, const char *ext)
     @@ packfile.c: void clear_delta_base_cache(void)
      }
 8:  e99b1dad54 !  8:  ec9061fbbd config: make `packed_git_(limit|window_size)` non-global variables
    @@ Commit message
         means we can also remove the `USE_THE_REPOSITORY_VARIABLE` guard from
         the file.
     
    +    Helped-by: Taylor Blau <me@ttaylorr.com>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## builtin/fast-import.c ##
    @@ packfile.c
     +#define PACKFILE_CONFIG_INIT \
     +{ \
     +	.packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE, \
    -+	.packed_git_limit = DEFAULT_PACKED_GIT_LIMIT,  \
    ++	.packed_git_limit = DEFAULT_PACKED_GIT_LIMIT, \
     +}
     +
    - char *odb_pack_name(struct repository *repo, struct strbuf *buf,
    + char *odb_pack_name(struct repository *r, struct strbuf *buf,
      		    const unsigned char *hash, const char *ext)
      {
     @@ packfile.c: static size_t pack_mapped;
    @@ packfile.c: static size_t pack_mapped;
     -void pack_report(void)
     +static int packfile_config(const char *var, const char *value,
     +			   const struct config_context *ctx, void *cb)
    - {
    ++{
     +	struct packfile_config *config = cb;
     +
     +	if (!strcmp(var, "core.packedgitwindowsize")) {
    @@ packfile.c: static size_t pack_mapped;
     +			config->packed_git_window_size = 1;
     +		config->packed_git_window_size *= pgsz_x2;
     +		return 0;
    -+	}
    -+
    -+	if (!strcmp(var, "core.packedgitlimit")) {
    ++	} else if (!strcmp(var, "core.packedgitlimit")) {
     +		config->packed_git_limit = git_config_ulong(var, value, ctx->kvi);
     +		return 0;
    ++	} else {
    ++		return git_default_config(var, value, ctx, cb);
     +	}
    -+
    -+	return git_default_config(var, value, ctx, cb);
     +}
     +
    -+
     +void pack_report(struct repository *repo)
    -+{
    + {
     +	struct packfile_config config = PACKFILE_CONFIG_INIT;
     +	repo_config(repo, packfile_config, &config);
     +
    @@ packfile.c: unsigned char *use_pack(struct packed_git *p,
     +			size_t window_align;
      			off_t len;
      
    -+			repo_config(p->repo, packfile_config, &config);
    ++			repo_config(p->r, packfile_config, &config);
     +			window_align = config.packed_git_window_size / 2;
     +
      			if (p->pack_fd == -1 && open_packed_git(p))
 3:  1f8ef580e5 !  9:  c0b386412d packfile: pass `repository` to static function in the file
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    packfile: pass `repository` to static function in the file
    +    midx: add repository to `multi_pack_index` struct
     
    -    Some of the static functions in the `packfile.c` access global
    -    variables, which can simply be avoiding by passing the `repository`
    -    struct down to them. Let's do that.
    +    The `multi_pack_index` struct represents the MIDX for a repository.
    +    Here, we add a pointer to the repository in this struct, allowing direct
    +    use of the repository variable without relying on the global
    +    `the_repository` struct.
    +
    +    With this addition, we can determine the repository associated with a
    +    `bitmap_index` struct. A `bitmap_index` points to either a `packed_git`
    +    or a `multi_pack_index`, both of which have direct repository
    +    references. To support this, we introduce a static helper function,
    +    `bitmap_repo`, in `pack-bitmap.c`, which retrieves a repository given a
    +    `bitmap_index`.
    +
    +    With this, we clear up all usages of `the_repository` within
    +    `pack-bitmap.c` and also remove the `USE_THE_REPOSITORY_VARIABLE`
    +    definition. Bringing us another step closer to remove all global
    +    variable usage.
    +
    +    Although this change also opens up the potential to clean up `midx.c`,
    +    doing so would require additional refactoring to pass the repository
    +    struct to functions where the MIDX struct is created: a task better
    +    suited for future patches.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    - ## packfile.c ##
    -@@ packfile.c: static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, struc
    - 	*accept_windows_inuse = has_windows_inuse;
    + ## midx.c ##
    +@@ midx.c: static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir
    + 	m->data = midx_map;
    + 	m->data_len = midx_size;
    + 	m->local = local;
    ++	m->r = the_repository;
    + 
    + 	m->signature = get_be32(m->data);
    + 	if (m->signature != MIDX_SIGNATURE)
    +
    + ## midx.h ##
    +@@ midx.h: struct multi_pack_index {
    + 
    + 	const char **pack_names;
    + 	struct packed_git **packs;
    ++
    ++	struct repository *r;
    ++
    + 	char object_dir[FLEX_ARRAY];
    + };
    + 
    +
    + ## pack-bitmap.c ##
    +@@
    +-#define USE_THE_REPOSITORY_VARIABLE
    +-
    + #include "git-compat-util.h"
    + #include "commit.h"
    + #include "gettext.h"
    +@@ pack-bitmap.c: static uint32_t bitmap_num_objects(struct bitmap_index *index)
    + 	return index->pack->num_objects;
      }
      
    --static int close_one_pack(void)
    -+static int close_one_pack(struct repository *repo)
    ++static struct repository *bitmap_repo(struct bitmap_index *bitmap_git)
    ++{
    ++	if (bitmap_is_midx(bitmap_git))
    ++		return bitmap_git->midx->r;
    ++	return bitmap_git->pack->r;
    ++}
    ++
    + static int load_bitmap_header(struct bitmap_index *index)
      {
    - 	struct packed_git *p, *lru_p = NULL;
    - 	struct pack_window *mru_w = NULL;
    - 	int accept_windows_inuse = 1;
    + 	struct bitmap_disk_header *header = (void *)index->map;
    +-	size_t header_size = sizeof(*header) - GIT_MAX_RAWSZ + the_hash_algo->rawsz;
    ++	const struct git_hash_algo *hash_algo = bitmap_repo(index)->hash_algo;
    ++
    ++	size_t header_size = sizeof(*header) - GIT_MAX_RAWSZ + hash_algo->rawsz;
      
    --	for (p = the_repository->objects->packed_git; p; p = p->next) {
    -+	for (p = repo->objects->packed_git; p; p = p->next) {
    - 		if (p->pack_fd == -1)
    - 			continue;
    - 		find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse);
    -@@ packfile.c: static int open_packed_git_1(struct packed_git *p)
    - 			pack_max_fds = 1;
    +-	if (index->map_size < header_size + the_hash_algo->rawsz)
    ++	if (index->map_size < header_size + hash_algo->rawsz)
    + 		return error(_("corrupted bitmap index (too small)"));
    + 
    + 	if (memcmp(header->magic, BITMAP_IDX_SIGNATURE, sizeof(BITMAP_IDX_SIGNATURE)) != 0)
    +@@ pack-bitmap.c: static int load_bitmap_header(struct bitmap_index *index)
    + 	{
    + 		uint32_t flags = ntohs(header->options);
    + 		size_t cache_size = st_mult(bitmap_num_objects(index), sizeof(uint32_t));
    +-		unsigned char *index_end = index->map + index->map_size - the_hash_algo->rawsz;
    ++		unsigned char *index_end = index->map + index->map_size - hash_algo->rawsz;
    + 
    + 		if ((flags & BITMAP_OPT_FULL_DAG) == 0)
    + 			BUG("unsupported options for bitmap index file "
    +@@ pack-bitmap.c: static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
    + 	if (bitmap_git->pack || bitmap_git->midx) {
    + 		struct strbuf buf = STRBUF_INIT;
    + 		get_midx_filename(&buf, midx->object_dir);
    +-		trace2_data_string("bitmap", the_repository,
    ++		trace2_data_string("bitmap", bitmap_repo(bitmap_git),
    + 				   "ignoring extra midx bitmap file", buf.buf);
    + 		close(fd);
    + 		strbuf_release(&buf);
    +@@ pack-bitmap.c: static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
    + 		goto cleanup;
    + 
    + 	if (!hasheq(get_midx_checksum(bitmap_git->midx), bitmap_git->checksum,
    +-		    the_repository->hash_algo)) {
    ++		    bitmap_repo(bitmap_git)->hash_algo)) {
    + 		error(_("checksum doesn't match in MIDX and bitmap"));
    + 		goto cleanup;
    + 	}
    +@@ pack-bitmap.c: static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
    + 	}
    + 
    + 	for (i = 0; i < bitmap_git->midx->num_packs; i++) {
    +-		if (prepare_midx_pack(the_repository, bitmap_git->midx, i)) {
    ++		if (prepare_midx_pack(bitmap_repo(bitmap_git),
    ++				      bitmap_git->midx,
    ++				      i)) {
    + 			warning(_("could not open pack %s"),
    + 				bitmap_git->midx->pack_names[i]);
    + 			goto cleanup;
    +@@ pack-bitmap.c: static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
      	}
      
    --	while (pack_max_fds <= pack_open_fds && close_one_pack())
    -+	while (pack_max_fds <= pack_open_fds && close_one_pack(p->repo))
    - 		; /* nothing */
    + 	if (bitmap_git->pack || bitmap_git->midx) {
    +-		trace2_data_string("bitmap", the_repository,
    +-				   "ignoring extra bitmap file", packfile->pack_name);
    ++		trace2_data_string("bitmap", bitmap_repo(bitmap_git),
    ++				   "ignoring extra bitmap file",
    ++				   packfile->pack_name);
    + 		close(fd);
    + 		return -1;
    + 	}
    +@@ pack-bitmap.c: static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
    + 		return -1;
    + 	}
      
    - 	p->pack_fd = git_open(p->pack_name);
    -@@ packfile.c: static int open_packed_git(struct packed_git *p)
    - 	return -1;
    +-	trace2_data_string("bitmap", the_repository, "opened bitmap file",
    +-			   packfile->pack_name);
    ++	trace2_data_string("bitmap", bitmap_repo(bitmap_git),
    ++			   "opened bitmap file", packfile->pack_name);
    + 	return 0;
      }
      
    --static int in_window(struct pack_window *win, off_t offset)
    -+static int in_window(struct repository *repo, struct pack_window *win,
    -+		     off_t offset)
    +@@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_git(struct repository *r)
    + 
    + struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
    + {
    +-	struct repository *r = the_repository;
    ++	struct repository *r = midx->r;
    + 	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
    + 
    + 	if (!open_midx_bitmap_1(bitmap_git, midx) && !load_bitmap(r, bitmap_git))
    +@@ pack-bitmap.c: static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
      {
    - 	/* We must promise at least one full hash after the
    - 	 * offset is available from this window, otherwise the offset
    -@@ packfile.c: static int in_window(struct pack_window *win, off_t offset)
    + 	struct bitmap_boundary_cb cb;
    + 	struct object_list *root;
    ++	struct repository *repo;
    + 	unsigned int i;
    + 	unsigned int tmp_blobs, tmp_trees, tmp_tags;
    + 	int any_missing = 0;
    +@@ pack-bitmap.c: static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
    + 	cb.base = bitmap_new();
    + 	object_array_init(&cb.boundary);
    + 
    ++	repo = bitmap_repo(bitmap_git);
    ++
    + 	revs->ignore_missing_links = 1;
    + 
    + 	if (bitmap_git->pseudo_merges.nr) {
    +@@ pack-bitmap.c: static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
    + 	 * revision walk to (a) OR in any bitmaps that are UNINTERESTING
    + 	 * between the tips and boundary, and (b) record the boundary.
      	 */
    - 	off_t win_off = win->offset;
    - 	return win_off <= offset
    --		&& (offset + the_hash_algo->rawsz) <= (win_off + win->len);
    -+		&& (offset + repo->hash_algo->rawsz) <= (win_off + win->len);
    +-	trace2_region_enter("pack-bitmap", "boundary-prepare", the_repository);
    ++	trace2_region_enter("pack-bitmap", "boundary-prepare", repo);
    + 	if (prepare_revision_walk(revs))
    + 		die("revision walk setup failed");
    +-	trace2_region_leave("pack-bitmap", "boundary-prepare", the_repository);
    ++	trace2_region_leave("pack-bitmap", "boundary-prepare", repo);
    + 
    +-	trace2_region_enter("pack-bitmap", "boundary-traverse", the_repository);
    ++	trace2_region_enter("pack-bitmap", "boundary-traverse", repo);
    + 	revs->boundary = 1;
    + 	traverse_commit_list_filtered(revs,
    + 				      show_boundary_commit,
    + 				      show_boundary_object,
    + 				      &cb, NULL);
    + 	revs->boundary = 0;
    +-	trace2_region_leave("pack-bitmap", "boundary-traverse", the_repository);
    ++	trace2_region_leave("pack-bitmap", "boundary-traverse", repo);
    + 
    + 	revs->blob_objects = tmp_blobs;
    + 	revs->tree_objects = tmp_trees;
    +@@ pack-bitmap.c: static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
    + 	/*
    + 	 * Then add the boundary commit(s) as fill-in traversal tips.
    + 	 */
    +-	trace2_region_enter("pack-bitmap", "boundary-fill-in", the_repository);
    ++	trace2_region_enter("pack-bitmap", "boundary-fill-in", repo);
    + 	for (i = 0; i < cb.boundary.nr; i++) {
    + 		struct object *obj = cb.boundary.objects[i].item;
    + 		if (bitmap_walk_contains(bitmap_git, cb.base, &obj->oid))
    +@@ pack-bitmap.c: static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
    + 	}
    + 	if (revs->pending.nr)
    + 		cb.base = fill_in_bitmap(bitmap_git, revs, cb.base, NULL);
    +-	trace2_region_leave("pack-bitmap", "boundary-fill-in", the_repository);
    ++	trace2_region_leave("pack-bitmap", "boundary-fill-in", repo);
    + 
    + cleanup:
    + 	object_array_clear(&cb.boundary);
    +@@ pack-bitmap.c: static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,
    + 			ofs = pack_pos_to_offset(pack, pos);
    + 		}
    + 
    +-		if (packed_object_info(the_repository, pack, ofs, &oi) < 0) {
    ++		if (packed_object_info(bitmap_repo(bitmap_git), pack, ofs,
    ++				       &oi) < 0) {
    + 			struct object_id oid;
    + 			nth_bitmap_object_oid(bitmap_git, &oid,
    + 					      pack_pos_to_index(pack, pos));
    +@@ pack-bitmap.c: static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,
    + 	} else {
    + 		struct eindex *eindex = &bitmap_git->ext_index;
    + 		struct object *obj = eindex->objects[pos - bitmap_num_objects(bitmap_git)];
    +-		if (oid_object_info_extended(the_repository, &obj->oid, &oi, 0) < 0)
    ++		if (oid_object_info_extended(bitmap_repo(bitmap_git), &obj->oid,
    ++					     &oi, 0) < 0)
    + 			die(_("unable to get size of %s"), oid_to_hex(&obj->oid));
    + 	}
    + 
    +@@ pack-bitmap.c: static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
    + 		bitmap_unset(result, i);
    + 
    + 	for (i = 0; i < eindex->count; ++i) {
    +-		if (has_object_pack(the_repository, &eindex->objects[i]->oid))
    ++		if (has_object_pack(bitmap_repo(bitmap_git),
    ++				    &eindex->objects[i]->oid))
    + 			bitmap_unset(result, objects_nr + i);
    + 	}
      }
    +@@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
    + 	struct bitmap *haves_bitmap = NULL;
      
    - unsigned char *use_pack(struct packed_git *p,
    -@@ packfile.c: unsigned char *use_pack(struct packed_git *p,
    - 	if (offset < 0)
    - 		die(_("offset before end of packfile (broken .idx?)"));
    - 
    --	if (!win || !in_window(win, offset)) {
    -+	if (!win || !in_window(p->repo, win, offset)) {
    - 		if (win)
    - 			win->inuse_cnt--;
    - 		for (win = p->windows; win; win = win->next) {
    --			if (in_window(win, offset))
    -+			if (in_window(p->repo, win, offset))
    - 				break;
    + 	struct bitmap_index *bitmap_git;
    ++	struct repository *repo;
    + 
    + 	/*
    + 	 * We can't do pathspec limiting with bitmaps, because we don't know
    +@@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
    + 	if (!use_boundary_traversal)
    + 		object_array_clear(&revs->pending);
    + 
    ++	repo = bitmap_repo(bitmap_git);
    ++
    + 	if (haves) {
    +-		if (use_boundary_traversal) {
    +-			trace2_region_enter("pack-bitmap", "haves/boundary", the_repository);
    ++		if (use_boundary_traversal)
    ++		{
    ++			trace2_region_enter("pack-bitmap", "haves/boundary", repo);
    + 			haves_bitmap = find_boundary_objects(bitmap_git, revs, haves);
    +-			trace2_region_leave("pack-bitmap", "haves/boundary", the_repository);
    +-		} else {
    +-			trace2_region_enter("pack-bitmap", "haves/classic", the_repository);
    ++			trace2_region_leave("pack-bitmap", "haves/boundary", repo);
    ++		}
    ++		else
    ++		{
    ++			trace2_region_enter("pack-bitmap", "haves/classic", repo);
    + 			revs->ignore_missing_links = 1;
    + 			haves_bitmap = find_objects(bitmap_git, revs, haves, NULL);
    + 			reset_revision_walk();
    + 			revs->ignore_missing_links = 0;
    +-			trace2_region_leave("pack-bitmap", "haves/classic", the_repository);
    ++			trace2_region_leave("pack-bitmap", "haves/classic", repo);
      		}
    - 		if (!win) {
    + 
    + 		if (!haves_bitmap)
    +@@ pack-bitmap.c: struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
    + 	object_list_free(&wants);
    + 	object_list_free(&haves);
    + 
    +-	trace2_data_intmax("bitmap", the_repository, "pseudo_merges_satisfied",
    ++	trace2_data_intmax("bitmap", repo, "pseudo_merges_satisfied",
    + 			   pseudo_merges_satisfied_nr);
    +-	trace2_data_intmax("bitmap", the_repository, "pseudo_merges_cascades",
    ++	trace2_data_intmax("bitmap", repo, "pseudo_merges_cascades",
    + 			   pseudo_merges_cascades_nr);
    +-	trace2_data_intmax("bitmap", the_repository, "bitmap/hits",
    ++	trace2_data_intmax("bitmap", repo, "bitmap/hits",
    + 			   existing_bitmaps_hits_nr);
    +-	trace2_data_intmax("bitmap", the_repository, "bitmap/misses",
    ++	trace2_data_intmax("bitmap", repo, "bitmap/misses",
    + 			   existing_bitmaps_misses_nr);
    +-	trace2_data_intmax("bitmap", the_repository, "bitmap/roots_with_bitmap",
    ++	trace2_data_intmax("bitmap", repo, "bitmap/roots_with_bitmap",
    + 			   roots_with_bitmaps_nr);
    +-	trace2_data_intmax("bitmap", the_repository, "bitmap/roots_without_bitmap",
    ++	trace2_data_intmax("bitmap", repo, "bitmap/roots_without_bitmap",
    + 			   roots_without_bitmaps_nr);
    + 
    + 	return bitmap_git;
    +@@ pack-bitmap.c: void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
    + 					struct bitmap **reuse_out,
    + 					int multi_pack_reuse)
    + {
    +-	struct repository *r = the_repository;
    ++	struct repository *r = bitmap_repo(bitmap_git);
    + 	struct bitmapped_pack *packs = NULL;
    + 	struct bitmap *result = bitmap_git->result;
    + 	struct bitmap *reuse;
    +@@ pack-bitmap.c: int rebuild_bitmap(const uint32_t *reposition,
    + uint32_t *create_bitmap_mapping(struct bitmap_index *bitmap_git,
    + 				struct packing_data *mapping)
    + {
    +-	struct repository *r = the_repository;
    ++	struct repository *r = bitmap_repo(bitmap_git);
    + 	uint32_t i, num_objects;
    + 	uint32_t *reposition;
    + 
    +@@ pack-bitmap.c: static off_t get_disk_usage_for_extended(struct bitmap_index *bitmap_git)
    + 				st_add(bitmap_num_objects(bitmap_git), i)))
    + 			continue;
    + 
    +-		if (oid_object_info_extended(the_repository, &obj->oid, &oi, 0) < 0)
    ++		if (oid_object_info_extended(bitmap_repo(bitmap_git), &obj->oid,
    ++					     &oi, 0) < 0)
    + 			die(_("unable to get disk usage of '%s'"),
    + 			    oid_to_hex(&obj->oid));
    + 
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

The struct `packed_git` holds information regarding a packed object file. Let's add the repository variable to this object, to represent the repository that this packfile belongs to. This helps remove dependency on the global `the_repository` object in `packfile.c` by simply using repository information now readily available in the struct.

We do need to consider that a pack file could be part of the alternates of a repository, but considering that we only have one repository struct and also that we currently anyways use 'the_repository'. We should be OK with this change.

We also modify `alloc_packed_git` to ensure that the repository is added to newly created `packed_git` structs. This requires modifying the function and all its callee to pass the repository object down the levels.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/fast-import.c |  3 ++-
 builtin/index-pack.c  |  6 ++++--
 commit-graph.c        |  2 +-
 connected.c           |  3 ++-
 http.c                |  2 +-
 midx-write.c          |  2 +-
 midx.c                |  2 +-
 object-store-ll.h     |  5 +++++
 packfile.c            | 15 +++++++++------
 packfile.h            |  6 ++++--
 10 files changed, 30 insertions(+), 16 deletions(-)
Show changes to 10 files +30 −16

builtin/fast-import.c, builtin/index-pack.c, commit-graph.c, connected.c, http.c, midx-write.c, midx.c, object-store-ll.h, packfile.c, packfile.h

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 76d5c20f14..f8d3d7e0c7 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -765,6 +765,7 @@ static void start_packfile(void)
 
 	p->pack_fd = pack_fd;
 	p->do_not_close = 1;
+	p->r = the_repository;
 	pack_file = hashfd(pack_fd, p->pack_name);
 
 	pack_data = p;
@@ -888,7 +889,7 @@ static void end_packfile(void)
 		idx_name = keep_pack(create_index());
 
 		/* Register the packfile with core git's machinery. */
-		new_p = add_packed_git(idx_name, strlen(idx_name), 1);
+		new_p = add_packed_git(pack_data->r, idx_name, strlen(idx_name), 1);
 		if (!new_p)
 			die("core git rejected index %s", idx_name);
 		all_packs[pack_id] = new_p;
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 9d23b41b3a..be2f99625e 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1552,7 +1552,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
 
 	if (do_fsck_object) {
 		struct packed_git *p;
-		p = add_packed_git(final_index_name, strlen(final_index_name), 0);
+		p = add_packed_git(the_repository, final_index_name,
+				   strlen(final_index_name), 0);
 		if (p)
 			install_packed_git(the_repository, p);
 	}
@@ -1650,7 +1651,8 @@ static void read_v2_anomalous_offsets(struct packed_git *p,
 
 static void read_idx_option(struct pack_idx_option *opts, const char *pack_name)
 {
-	struct packed_git *p = add_packed_git(pack_name, strlen(pack_name), 1);
+	struct packed_git *p = add_packed_git(the_repository, pack_name,
+					      strlen(pack_name), 1);
 
 	if (!p)
 		die(_("Cannot open existing pack file '%s'"), pack_name);
diff --git a/commit-graph.c b/commit-graph.c
index 5bd89c0acd..83dd69bfeb 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1914,7 +1914,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx,
 		struct packed_git *p;
 		strbuf_setlen(&packname, dirlen);
 		strbuf_addstr(&packname, pack_indexes->items[i].string);
-		p = add_packed_git(packname.buf, packname.len, 1);
+		p = add_packed_git(ctx->r, packname.buf, packname.len, 1);
 		if (!p) {
 			ret = error(_("error adding pack %s"), packname.buf);
 			goto cleanup;
diff --git a/connected.c b/connected.c
index a9e2e13995..3099da84f3 100644
--- a/connected.c
+++ b/connected.c
@@ -54,7 +54,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 		strbuf_add(&idx_file, transport->pack_lockfiles.items[0].string,
 			   base_len);
 		strbuf_addstr(&idx_file, ".idx");
-		new_pack = add_packed_git(idx_file.buf, idx_file.len, 1);
+		new_pack = add_packed_git(the_repository, idx_file.buf,
+					  idx_file.len, 1);
 		strbuf_release(&idx_file);
 	}
 
diff --git a/http.c b/http.c
index 510332ab04..7e5be05207 100644
--- a/http.c
+++ b/http.c
@@ -2437,7 +2437,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head,
 	if (!tmp_idx)
 		return -1;
 
-	new_pack = parse_pack_index(sha1, tmp_idx);
+	new_pack = parse_pack_index(the_repository, sha1, tmp_idx);
 	if (!new_pack) {
 		unlink(tmp_idx);
 		free(tmp_idx);
diff --git a/midx-write.c b/midx-write.c
index b3a5f6c516..c57726ef94 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -154,7 +154,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
 			return;
 
 		ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc);
-		p = add_packed_git(full_path, full_path_len, 0);
+		p = add_packed_git(the_repository, full_path, full_path_len, 0);
 		if (!p) {
 			warning(_("failed to add packfile '%s'"),
 				full_path);
diff --git a/midx.c b/midx.c
index e82d4f2e65..8edb75f51d 100644
--- a/midx.c
+++ b/midx.c
@@ -464,7 +464,7 @@ int prepare_midx_pack(struct repository *r, struct multi_pack_index *m,
 					strhash(key.buf), key.buf,
 					struct packed_git, packmap_ent);
 	if (!p) {
-		p = add_packed_git(pack_name.buf, pack_name.len, m->local);
+		p = add_packed_git(r, pack_name.buf, pack_name.len, m->local);
 		if (p) {
 			install_packed_git(r, p);
 			list_add_tail(&p->mru, &r->objects->packed_git_mru);
diff --git a/object-store-ll.h b/object-store-ll.h
index 53b8e693b1..e8a22ab5fc 100644
--- a/object-store-ll.h
+++ b/object-store-ll.h
@@ -10,6 +10,7 @@
 struct oidmap;
 struct oidtree;
 struct strbuf;
+struct repository;
 
 struct object_directory {
 	struct object_directory *next;
@@ -135,6 +136,10 @@ struct packed_git {
 	 */
 	const uint32_t *mtimes_map;
 	size_t mtimes_size;
+
+	/* repo dentoes the repository this packed file belongs to */
+	struct repository *r;
+
 	/* something like ".git/objects/pack/xxxxx.pack" */
 	char pack_name[FLEX_ARRAY]; /* more */
 };
diff --git a/packfile.c b/packfile.c
index 9560f0a33c..1423f23f57 100644
--- a/packfile.c
+++ b/packfile.c
@@ -217,11 +217,12 @@ uint32_t get_pack_fanout(struct packed_git *p, uint32_t value)
 	return ntohl(level1_ofs[value]);
 }
 
-static struct packed_git *alloc_packed_git(int extra)
+static struct packed_git *alloc_packed_git(struct repository *r, int extra)
 {
 	struct packed_git *p = xmalloc(st_add(sizeof(*p), extra));
 	memset(p, 0, sizeof(*p));
 	p->pack_fd = -1;
+	p->r = r;
 	return p;
 }
 
@@ -233,11 +234,12 @@ static char *pack_path_from_idx(const char *idx_path)
 	return xstrfmt("%.*s.pack", (int)len, idx_path);
 }
 
-struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path)
+struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1,
+				    const char *idx_path)
 {
 	char *path = pack_path_from_idx(idx_path);
 	size_t alloc = st_add(strlen(path), 1);
-	struct packed_git *p = alloc_packed_git(alloc);
+	struct packed_git *p = alloc_packed_git(r, alloc);
 
 	memcpy(p->pack_name, path, alloc); /* includes NUL */
 	free(path);
@@ -703,7 +705,8 @@ void unuse_pack(struct pack_window **w_cursor)
 	}
 }
 
-struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
+struct packed_git *add_packed_git(struct repository *r, const char *path,
+				  size_t path_len, int local)
 {
 	struct stat st;
 	size_t alloc;
@@ -721,7 +724,7 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
 	 * the use xsnprintf double-checks that)
 	 */
 	alloc = st_add3(path_len, strlen(".promisor"), 1);
-	p = alloc_packed_git(alloc);
+	p = alloc_packed_git(r, alloc);
 	memcpy(p->pack_name, path, path_len);
 
 	xsnprintf(p->pack_name + path_len, alloc - path_len, ".keep");
@@ -877,7 +880,7 @@ static void prepare_pack(const char *full_name, size_t full_name_len,
 
 		/* Don't reopen a pack we already have. */
 		if (!hashmap_get(&data->r->objects->pack_map, &hent, pack_name)) {
-			p = add_packed_git(full_name, full_name_len, data->local);
+			p = add_packed_git(data->r, full_name, full_name_len, data->local);
 			if (p)
 				install_packed_git(data->r, p);
 		}
diff --git a/packfile.h b/packfile.h
index 08f88a7ff5..aee69d1a0b 100644
--- a/packfile.h
+++ b/packfile.h
@@ -46,7 +46,8 @@ const char *pack_basename(struct packed_git *p);
  * and does not add the resulting packed_git struct to the internal list of
  * packs. You probably want add_packed_git() instead.
  */
-struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path);
+struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1,
+				    const char *idx_path);
 
 typedef void each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len,
 				      const char *file_name, void *data);
@@ -113,7 +114,8 @@ void close_pack(struct packed_git *);
 void close_object_store(struct raw_object_store *o);
 void unuse_pack(struct pack_window **);
 void clear_delta_base_cache(void);
-struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
+struct packed_git *add_packed_git(struct repository *r, const char *path,
+				  size_t path_len, int local);
 
 /*
  * Unlink the .pack and associated extension files.
-- 
2.47.0
Taylor Blau· Oct 30, 2024, 20:00 UTC · re: Karthik Nayak · lore

Re: [PATCH v3 1/9] packfile: add repository to struct `packed_git`

On Wed, Oct 30, 2024 at 03:32:26PM +0100, Karthik Nayak wrote:
Show 6 quoted lines
> [...]
>
> We do need to consider that a pack file could be part of the alternates
> of a repository, but considering that we only have one repository struct
> and also that we currently anyways use 'the_repository'. We should be
> OK with this change.
Nicely explained.
Show 20 quoted lines
> diff --git a/object-store-ll.h b/object-store-ll.h
> index 53b8e693b1..e8a22ab5fc 100644
> --- a/object-store-ll.h
> +++ b/object-store-ll.h
> @@ -10,6 +10,7 @@
>  struct oidmap;
>  struct oidtree;
>  struct strbuf;
> +struct repository;
>
>  struct object_directory {
>  	struct object_directory *next;
> @@ -135,6 +136,10 @@ struct packed_git {
>  	 */
>  	const uint32_t *mtimes_map;
>  	size_t mtimes_size;
> +
> +	/* repo dentoes the repository this packed file belongs to */
> +	struct repository *r;
> +

Hmm. What I meant in my earlier suggestion was that we should leave the member of the struct called "repo", but change the name only in function arguments.

Sorry to split hairs, but I am somewhat opposed to having such a short variable name in a struct. In either event, the comment should be made consistent with the variable name.

Show 13 quoted lines
>  	/* something like ".git/objects/pack/xxxxx.pack" */
>  	char pack_name[FLEX_ARRAY]; /* more */
>  };
> diff --git a/packfile.c b/packfile.c
> index 9560f0a33c..1423f23f57 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -217,11 +217,12 @@ uint32_t get_pack_fanout(struct packed_git *p, uint32_t value)
>  	return ntohl(level1_ofs[value]);
>  }
>
> -static struct packed_git *alloc_packed_git(int extra)
> +static struct packed_git *alloc_packed_git(struct repository *r, int extra)
This spot I would leave alone.
Show 5 quoted lines
>  {
>  	struct packed_git *p = xmalloc(st_add(sizeof(*p), extra));
>  	memset(p, 0, sizeof(*p));
>  	p->pack_fd = -1;
> +	p->r = r;
And this spot I would change to:
    p->repo = r;
The rest is looking good.

Thanks, Taylor

Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 3/9] packfile: pass `repository` to static function in the file

Some of the static functions in the `packfile.c` access global variables, which can simply be avoiding by passing the `repository` struct down to them. Let's do that.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 packfile.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
Show changes to packfile.c +8 −7
diff --git a/packfile.c b/packfile.c
index ecb284fd98..a391474031 100644
--- a/packfile.c
+++ b/packfile.c
@@ -460,13 +460,13 @@ static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, struc
 	*accept_windows_inuse = has_windows_inuse;
 }
 
-static int close_one_pack(void)
+static int close_one_pack(struct repository *r)
 {
 	struct packed_git *p, *lru_p = NULL;
 	struct pack_window *mru_w = NULL;
 	int accept_windows_inuse = 1;
 
-	for (p = the_repository->objects->packed_git; p; p = p->next) {
+	for (p = r->objects->packed_git; p; p = p->next) {
 		if (p->pack_fd == -1)
 			continue;
 		find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse);
@@ -555,7 +555,7 @@ static int open_packed_git_1(struct packed_git *p)
 			pack_max_fds = 1;
 	}
 
-	while (pack_max_fds <= pack_open_fds && close_one_pack())
+	while (pack_max_fds <= pack_open_fds && close_one_pack(p->r))
 		; /* nothing */
 
 	p->pack_fd = git_open(p->pack_name);
@@ -610,7 +610,8 @@ static int open_packed_git(struct packed_git *p)
 	return -1;
 }
 
-static int in_window(struct pack_window *win, off_t offset)
+static int in_window(struct repository *r, struct pack_window *win,
+		     off_t offset)
 {
 	/* We must promise at least one full hash after the
 	 * offset is available from this window, otherwise the offset
@@ -620,7 +621,7 @@ static int in_window(struct pack_window *win, off_t offset)
 	 */
 	off_t win_off = win->offset;
 	return win_off <= offset
-		&& (offset + the_hash_algo->rawsz) <= (win_off + win->len);
+		&& (offset + r->hash_algo->rawsz) <= (win_off + win->len);
 }
 
 unsigned char *use_pack(struct packed_git *p,
@@ -642,11 +643,11 @@ unsigned char *use_pack(struct packed_git *p,
 	if (offset < 0)
 		die(_("offset before end of packfile (broken .idx?)"));
 
-	if (!win || !in_window(win, offset)) {
+	if (!win || !in_window(p->r, win, offset)) {
 		if (win)
 			win->inuse_cnt--;
 		for (win = p->windows; win; win = win->next) {
-			if (in_window(win, offset))
+			if (in_window(p->r, win, offset))
 				break;
 		}
 		if (!win) {
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 2/9] packfile: use `repository` from `packed_git` directly

In the previous commit, we introduced the `repository` structure inside `packed_git`. This provides an alternative route instead of using the global `the_repository` variable. Let's modify `packfile.c` now to use this field wherever possible instead of relying on the global state. There are still a few instances of `the_repository` usage in the file, where there is no struct `packed_git` locally available, which will be fixed in the following commits.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 packfile.c | 50 +++++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 23 deletions(-)
Show changes to packfile.c +27 −23
diff --git a/packfile.c b/packfile.c
index 1423f23f57..ecb284fd98 100644
--- a/packfile.c
+++ b/packfile.c
@@ -79,7 +79,7 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 	size_t idx_size;
 	int fd = git_open(path), ret;
 	struct stat st;
-	const unsigned int hashsz = the_hash_algo->rawsz;
+	const unsigned int hashsz = p->r->hash_algo->rawsz;
 
 	if (fd < 0)
 		return -1;
@@ -243,7 +243,7 @@ struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1,
 
 	memcpy(p->pack_name, path, alloc); /* includes NUL */
 	free(path);
-	hashcpy(p->hash, sha1, the_repository->hash_algo);
+	hashcpy(p->hash, sha1, p->r->hash_algo);
 	if (check_packed_git_idx(idx_path, p)) {
 		free(p);
 		return NULL;
@@ -278,7 +278,7 @@ static int unuse_one_window(struct packed_git *current)
 
 	if (current)
 		scan_windows(current, &lru_p, &lru_w, &lru_l);
-	for (p = the_repository->objects->packed_git; p; p = p->next)
+	for (p = current->r->objects->packed_git; p; p = p->next)
 		scan_windows(p, &lru_p, &lru_w, &lru_l);
 	if (lru_p) {
 		munmap(lru_w->base, lru_w->len);
@@ -540,7 +540,7 @@ static int open_packed_git_1(struct packed_git *p)
 	unsigned char hash[GIT_MAX_RAWSZ];
 	unsigned char *idx_hash;
 	ssize_t read_result;
-	const unsigned hashsz = the_hash_algo->rawsz;
+	const unsigned hashsz = p->r->hash_algo->rawsz;
 
 	if (open_pack_index(p))
 		return error("packfile %s index unavailable", p->pack_name);
@@ -597,7 +597,7 @@ static int open_packed_git_1(struct packed_git *p)
 	if (read_result != hashsz)
 		return error("packfile %s signature is unavailable", p->pack_name);
 	idx_hash = ((unsigned char *)p->index_data) + p->index_size - hashsz * 2;
-	if (!hasheq(hash, idx_hash, the_repository->hash_algo))
+	if (!hasheq(hash, idx_hash, p->r->hash_algo))
 		return error("packfile %s does not match index", p->pack_name);
 	return 0;
 }
@@ -637,7 +637,7 @@ unsigned char *use_pack(struct packed_git *p,
 	 */
 	if (!p->pack_size && p->pack_fd == -1 && open_packed_git(p))
 		die("packfile %s cannot be accessed", p->pack_name);
-	if (offset > (p->pack_size - the_hash_algo->rawsz))
+	if (offset > (p->pack_size - p->r->hash_algo->rawsz))
 		die("offset beyond end of packfile (truncated pack?)");
 	if (offset < 0)
 		die(_("offset before end of packfile (broken .idx?)"));
@@ -711,6 +711,7 @@ struct packed_git *add_packed_git(struct repository *r, const char *path,
 	struct stat st;
 	size_t alloc;
 	struct packed_git *p;
+	struct object_id oid;
 
 	/*
 	 * Make sure a corresponding .pack file exists and that
@@ -751,9 +752,13 @@ struct packed_git *add_packed_git(struct repository *r, const char *path,
 	p->pack_size = st.st_size;
 	p->pack_local = local;
 	p->mtime = st.st_mtime;
-	if (path_len < the_hash_algo->hexsz ||
-	    get_hash_hex(path + path_len - the_hash_algo->hexsz, p->hash))
-		hashclr(p->hash, the_repository->hash_algo);
+	if (path_len < r->hash_algo->hexsz ||
+	    get_oid_hex_algop(path + path_len - r->hash_algo->hexsz, &oid,
+			      r->hash_algo))
+		hashclr(p->hash, r->hash_algo);
+	else
+		hashcpy(p->hash, oid.hash, r->hash_algo);
+
 	return p;
 }
 
@@ -1243,9 +1248,9 @@ off_t get_delta_base(struct packed_git *p,
 	} else if (type == OBJ_REF_DELTA) {
 		/* The base entry _must_ be in the same pack */
 		struct object_id oid;
-		oidread(&oid, base_info, the_repository->hash_algo);
+		oidread(&oid, base_info, p->r->hash_algo);
 		base_offset = find_pack_entry_one(&oid, p);
-		*curpos += the_hash_algo->rawsz;
+		*curpos += p->r->hash_algo->rawsz;
 	} else
 		die("I am totally screwed");
 	return base_offset;
@@ -1266,7 +1271,7 @@ static int get_delta_base_oid(struct packed_git *p,
 {
 	if (type == OBJ_REF_DELTA) {
 		unsigned char *base = use_pack(p, w_curs, curpos, NULL);
-		oidread(oid, base, the_repository->hash_algo);
+		oidread(oid, base, p->r->hash_algo);
 		return 0;
 	} else if (type == OBJ_OFS_DELTA) {
 		uint32_t base_pos;
@@ -1608,7 +1613,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,
 				goto out;
 			}
 		} else
-			oidclr(oi->delta_base_oid, the_repository->hash_algo);
+			oidclr(oi->delta_base_oid, p->r->hash_algo);
 	}
 
 	oi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :
@@ -1897,7 +1902,7 @@ int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32
 {
 	const unsigned char *index_fanout = p->index_data;
 	const unsigned char *index_lookup;
-	const unsigned int hashsz = the_hash_algo->rawsz;
+	const unsigned int hashsz = p->r->hash_algo->rawsz;
 	int index_lookup_width;
 
 	if (!index_fanout)
@@ -1922,7 +1927,7 @@ int nth_packed_object_id(struct object_id *oid,
 			 uint32_t n)
 {
 	const unsigned char *index = p->index_data;
-	const unsigned int hashsz = the_hash_algo->rawsz;
+	const unsigned int hashsz = p->r->hash_algo->rawsz;
 	if (!index) {
 		if (open_pack_index(p))
 			return -1;
@@ -1933,11 +1938,10 @@ int nth_packed_object_id(struct object_id *oid,
 	index += 4 * 256;
 	if (p->index_version == 1) {
 		oidread(oid, index + st_add(st_mult(hashsz + 4, n), 4),
-			the_repository->hash_algo);
+			p->r->hash_algo);
 	} else {
 		index += 8;
-		oidread(oid, index + st_mult(hashsz, n),
-			the_repository->hash_algo);
+		oidread(oid, index + st_mult(hashsz, n), p->r->hash_algo);
 	}
 	return 0;
 }
@@ -1959,7 +1963,7 @@ void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
 {
 	const unsigned char *index = p->index_data;
-	const unsigned int hashsz = the_hash_algo->rawsz;
+	const unsigned int hashsz = p->r->hash_algo->rawsz;
 	index += 4 * 256;
 	if (p->index_version == 1) {
 		return ntohl(*((uint32_t *)(index + st_mult(hashsz + 4, n))));
@@ -2159,7 +2163,7 @@ int for_each_object_in_pack(struct packed_git *p,
 	int r = 0;
 
 	if (flags & FOR_EACH_OBJECT_PACK_ORDER) {
-		if (load_pack_revindex(the_repository, p))
+		if (load_pack_revindex(p->r, p))
 			return -1;
 	}
 
@@ -2227,7 +2231,7 @@ int for_each_packed_object(each_packed_object_fn cb, void *data,
 }
 
 static int add_promisor_object(const struct object_id *oid,
-			       struct packed_git *pack UNUSED,
+			       struct packed_git *pack,
 			       uint32_t pos UNUSED,
 			       void *set_)
 {
@@ -2235,12 +2239,12 @@ static int add_promisor_object(const struct object_id *oid,
 	struct object *obj;
 	int we_parsed_object;
 
-	obj = lookup_object(the_repository, oid);
+	obj = lookup_object(pack->r, oid);
 	if (obj && obj->parsed) {
 		we_parsed_object = 0;
 	} else {
 		we_parsed_object = 1;
-		obj = parse_object(the_repository, oid);
+		obj = parse_object(pack->r, oid);
 	}
 
 	if (!obj)
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 4/9] packfile: pass down repository to `odb_pack_name`

The function `odb_pack_name` currently relies on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/fast-import.c    | 8 ++++----
 builtin/index-pack.c     | 4 ++--
 builtin/pack-redundant.c | 2 +-
 http.c                   | 2 +-
 packfile.c               | 9 ++++-----
 packfile.h               | 3 ++-
 6 files changed, 14 insertions(+), 14 deletions(-)
Show changes to 6 files +14 −14

builtin/fast-import.c, builtin/index-pack.c, builtin/pack-redundant.c, http.c, packfile.c, packfile.h

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index f8d3d7e0c7..2ca8198153 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -806,7 +806,7 @@ static char *keep_pack(const char *curr_index_name)
 	struct strbuf name = STRBUF_INIT;
 	int keep_fd;
 
-	odb_pack_name(&name, pack_data->hash, "keep");
+	odb_pack_name(pack_data->r, &name, pack_data->hash, "keep");
 	keep_fd = odb_pack_keep(name.buf);
 	if (keep_fd < 0)
 		die_errno("cannot create keep file");
@@ -814,11 +814,11 @@ static char *keep_pack(const char *curr_index_name)
 	if (close(keep_fd))
 		die_errno("failed to write keep file");
 
-	odb_pack_name(&name, pack_data->hash, "pack");
+	odb_pack_name(pack_data->r, &name, pack_data->hash, "pack");
 	if (finalize_object_file(pack_data->pack_name, name.buf))
 		die("cannot store pack file");
 
-	odb_pack_name(&name, pack_data->hash, "idx");
+	odb_pack_name(pack_data->r, &name, pack_data->hash, "idx");
 	if (finalize_object_file(curr_index_name, name.buf))
 		die("cannot store index file");
 	free((void *)curr_index_name);
@@ -832,7 +832,7 @@ static void unkeep_all_packs(void)
 
 	for (k = 0; k < pack_id; k++) {
 		struct packed_git *p = all_packs[k];
-		odb_pack_name(&name, p->hash, "keep");
+		odb_pack_name(p->r, &name, p->hash, "keep");
 		unlink_or_warn(name.buf);
 	}
 	strbuf_release(&name);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index be2f99625e..eaefb41761 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1479,7 +1479,7 @@ static void write_special_file(const char *suffix, const char *msg,
 	if (pack_name)
 		filename = derive_filename(pack_name, "pack", suffix, &name_buf);
 	else
-		filename = odb_pack_name(&name_buf, hash, suffix);
+		filename = odb_pack_name(the_repository, &name_buf, hash, suffix);
 
 	fd = odb_pack_keep(filename);
 	if (fd < 0) {
@@ -1507,7 +1507,7 @@ static void rename_tmp_packfile(const char **final_name,
 {
 	if (!*final_name || strcmp(*final_name, curr_name)) {
 		if (!*final_name)
-			*final_name = odb_pack_name(name, hash, ext);
+			*final_name = odb_pack_name(the_repository, name, hash, ext);
 		if (finalize_object_file(curr_name, *final_name))
 			die(_("unable to rename temporary '*.%s' file to '%s'"),
 			    ext, *final_name);
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index d2c1c4e5ec..01d1362c5b 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -690,7 +690,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s
 	pl = red = pack_list_difference(local_packs, min);
 	while (pl) {
 		printf("%s\n%s\n",
-		       odb_pack_name(&idx_name, pl->pack->hash, "idx"),
+		       odb_pack_name(pl->pack->r, &idx_name, pl->pack->hash, "idx"),
 		       pl->pack->pack_name);
 		pl = pl->next;
 	}
diff --git a/http.c b/http.c
index 7e5be05207..50d8811cea 100644
--- a/http.c
+++ b/http.c
@@ -2579,7 +2579,7 @@ struct http_pack_request *new_direct_http_pack_request(
 
 	preq->url = url;
 
-	odb_pack_name(&preq->tmpfile, packed_git_hash, "pack");
+	odb_pack_name(the_repository, &preq->tmpfile, packed_git_hash, "pack");
 	strbuf_addstr(&preq->tmpfile, ".temp");
 	preq->packfile = fopen(preq->tmpfile.buf, "a");
 	if (!preq->packfile) {
diff --git a/packfile.c b/packfile.c
index a391474031..ce701255dd 100644
--- a/packfile.c
+++ b/packfile.c
@@ -25,13 +25,12 @@
 #include "pack-revindex.h"
 #include "promisor-remote.h"
 
-char *odb_pack_name(struct strbuf *buf,
-		    const unsigned char *hash,
-		    const char *ext)
+char *odb_pack_name(struct repository *r, struct strbuf *buf,
+		    const unsigned char *hash, const char *ext)
 {
 	strbuf_reset(buf);
-	strbuf_addf(buf, "%s/pack/pack-%s.%s", repo_get_object_directory(the_repository),
-		    hash_to_hex(hash), ext);
+	strbuf_addf(buf, "%s/pack/pack-%s.%s", repo_get_object_directory(r),
+		    hash_to_hex_algop(hash, r->hash_algo), ext);
 	return buf->buf;
 }
 
diff --git a/packfile.h b/packfile.h
index aee69d1a0b..51187f2393 100644
--- a/packfile.h
+++ b/packfile.h
@@ -29,7 +29,8 @@ struct pack_entry {
  *
  * Example: odb_pack_name(out, sha1, "idx") => ".git/objects/pack/pack-1234..idx"
  */
-char *odb_pack_name(struct strbuf *buf, const unsigned char *sha1, const char *ext);
+char *odb_pack_name(struct repository *r, struct strbuf *buf,
+		    const unsigned char *hash, const char *ext);
 
 /*
  * Return the basename of the packfile, omitting any containing directory
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 5/9] packfile: pass down repository to `has_object[_kept]_pack`

The functions `has_object[_kept]_pack` currently rely on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Let's remove its usage from these functions and any related ones.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/count-objects.c | 2 +-
 builtin/fsck.c          | 2 +-
 builtin/pack-objects.c  | 4 ++--
 diff.c                  | 3 ++-
 list-objects.c          | 3 ++-
 pack-bitmap.c           | 2 +-
 packfile.c              | 9 +++++----
 packfile.h              | 5 +++--
 prune-packed.c          | 2 +-
 reachable.c             | 2 +-
 revision.c              | 4 ++--
 11 files changed, 21 insertions(+), 17 deletions(-)
Show changes to 11 files +21 −17

builtin/count-objects.c, builtin/fsck.c, builtin/pack-objects.c, diff.c, list-objects.c, pack-bitmap.c, packfile.c, packfile.h, prune-packed.c, reachable.c, revision.c

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 04d80887e0..1e89148ed7 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -67,7 +67,7 @@ static int count_loose(const struct object_id *oid, const char *path,
 	else {
 		loose_size += on_disk_bytes(st);
 		loose++;
-		if (verbose && has_object_pack(oid))
+		if (verbose && has_object_pack(the_repository, oid))
 			packed_loose++;
 	}
 	return 0;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 7f4e2f0414..bb56eb98ac 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -272,7 +272,7 @@ static void check_reachable_object(struct object *obj)
 	if (!(obj->flags & HAS_OBJ)) {
 		if (is_promisor_object(&obj->oid))
 			return;
-		if (has_object_pack(&obj->oid))
+		if (has_object_pack(the_repository, &obj->oid))
 			return; /* it is in pack - forget about it */
 		printf_ln(_("missing %s %s"),
 			  printable_type(&obj->oid, obj->type),
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0800714267..ceb7e76b10 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1529,7 +1529,7 @@ static int want_found_object(const struct object_id *oid, int exclude,
 			return 0;
 		if (ignore_packed_keep_in_core && p->pack_keep_in_core)
 			return 0;
-		if (has_object_kept_pack(oid, flags))
+		if (has_object_kept_pack(p->r, oid, flags))
 			return 0;
 	}
 
@@ -3627,7 +3627,7 @@ static void show_cruft_commit(struct commit *commit, void *data)
 
 static int cruft_include_check_obj(struct object *obj, void *data UNUSED)
 {
-	return !has_object_kept_pack(&obj->oid, IN_CORE_KEEP_PACKS);
+	return !has_object_kept_pack(to_pack.repo, &obj->oid, IN_CORE_KEEP_PACKS);
 }
 
 static int cruft_include_check(struct commit *commit, void *data)
diff --git a/diff.c b/diff.c
index dceac20d18..266ddf18e7 100644
--- a/diff.c
+++ b/diff.c
@@ -4041,7 +4041,8 @@ static int reuse_worktree_file(struct index_state *istate,
 	 * objects however would tend to be slower as they need
 	 * to be individually opened and inflated.
 	 */
-	if (!FAST_WORKING_DIRECTORY && !want_file && has_object_pack(oid))
+	if (!FAST_WORKING_DIRECTORY && !want_file &&
+	    has_object_pack(istate->repo, oid))
 		return 0;
 
 	/*
diff --git a/list-objects.c b/list-objects.c
index 985d008799..31236a8dc9 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -41,7 +41,8 @@ static void show_object(struct traversal_context *ctx,
 {
 	if (!ctx->show_object)
 		return;
-	if (ctx->revs->unpacked && has_object_pack(&object->oid))
+	if (ctx->revs->unpacked && has_object_pack(ctx->revs->repo,
+						   &object->oid))
 		return;
 
 	ctx->show_object(object, name, ctx->show_data);
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 4fa9dfc771..d34ba9909a 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1889,7 +1889,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 		bitmap_unset(result, i);
 
 	for (i = 0; i < eindex->count; ++i) {
-		if (has_object_pack(&eindex->objects[i]->oid))
+		if (has_object_pack(the_repository, &eindex->objects[i]->oid))
 			bitmap_unset(result, objects_nr + i);
 	}
 }
diff --git a/packfile.c b/packfile.c
index ce701255dd..3894646573 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2143,16 +2143,17 @@ int find_kept_pack_entry(struct repository *r,
 	return 0;
 }
 
-int has_object_pack(const struct object_id *oid)
+int has_object_pack(struct repository *r, const struct object_id *oid)
 {
 	struct pack_entry e;
-	return find_pack_entry(the_repository, oid, &e);
+	return find_pack_entry(r, oid, &e);
 }
 
-int has_object_kept_pack(const struct object_id *oid, unsigned flags)
+int has_object_kept_pack(struct repository *r, const struct object_id *oid,
+			 unsigned flags)
 {
 	struct pack_entry e;
-	return find_kept_pack_entry(the_repository, oid, flags, &e);
+	return find_kept_pack_entry(r, oid, flags, &e);
 }
 
 int for_each_object_in_pack(struct packed_git *p,
diff --git a/packfile.h b/packfile.h
index 51187f2393..b09fb2c530 100644
--- a/packfile.h
+++ b/packfile.h
@@ -193,8 +193,9 @@ const struct packed_git *has_packed_and_bad(struct repository *, const struct ob
 int find_pack_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e);
 int find_kept_pack_entry(struct repository *r, const struct object_id *oid, unsigned flags, struct pack_entry *e);
 
-int has_object_pack(const struct object_id *oid);
-int has_object_kept_pack(const struct object_id *oid, unsigned flags);
+int has_object_pack(struct repository *r, const struct object_id *oid);
+int has_object_kept_pack(struct repository *r, const struct object_id *oid,
+			 unsigned flags);
 
 /*
  * Return 1 if an object in a promisor packfile is or refers to the given
diff --git a/prune-packed.c b/prune-packed.c
index 2bb99c29df..d1c65ab10e 100644
--- a/prune-packed.c
+++ b/prune-packed.c
@@ -24,7 +24,7 @@ static int prune_object(const struct object_id *oid, const char *path,
 {
 	int *opts = data;
 
-	if (!has_object_pack(oid))
+	if (!has_object_pack(the_repository, oid))
 		return 0;
 
 	if (*opts & PRUNE_PACKED_DRY_RUN)
diff --git a/reachable.c b/reachable.c
index 3e9b3dd0a4..09d2c50079 100644
--- a/reachable.c
+++ b/reachable.c
@@ -239,7 +239,7 @@ static int want_recent_object(struct recent_data *data,
 			      const struct object_id *oid)
 {
 	if (data->ignore_in_core_kept_packs &&
-	    has_object_kept_pack(oid, IN_CORE_KEEP_PACKS))
+	    has_object_kept_pack(data->revs->repo, oid, IN_CORE_KEEP_PACKS))
 		return 0;
 	return 1;
 }
diff --git a/revision.c b/revision.c
index f5f5b84f2b..d1d152a67b 100644
--- a/revision.c
+++ b/revision.c
@@ -4103,10 +4103,10 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi
 {
 	if (commit->object.flags & SHOWN)
 		return commit_ignore;
-	if (revs->unpacked && has_object_pack(&commit->object.oid))
+	if (revs->unpacked && has_object_pack(revs->repo, &commit->object.oid))
 		return commit_ignore;
 	if (revs->no_kept_objects) {
-		if (has_object_kept_pack(&commit->object.oid,
+		if (has_object_kept_pack(revs->repo, &commit->object.oid,
 					 revs->keep_pack_cache_flags))
 			return commit_ignore;
 	}
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 7/9] config: make `delta_base_cache_limit` a non-global variable

The `delta_base_cache_limit` variable is a global config variable used by multiple subsystems. Let's make this non-global, by adding this variable to the stack of each of the subsystems where it is used.

In `gc.c` we add it to the `gc_config` struct and also the constructor function. In `index-pack.c` we add it to the `pack_idx_option` struct and its constructor. Finally, in `packfile.c` we dynamically retrieve this value from the repository config, since the value is only used once in the entire subsystem.

These changes are made to remove the usage of `delta_base_cache_limit` as a global variable in `packfile.c`. This brings us one step closer to removing the `USE_THE_REPOSITORY_VARIABLE` definition in `packfile.c` which we complete in the next patch.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/gc.c         |  5 ++++-
 builtin/index-pack.c | 10 +++++++---
 config.c             |  5 -----
 environment.c        |  1 -
 environment.h        |  1 -
 pack-objects.h       |  3 ++-
 pack-write.c         |  1 +
 pack.h               |  1 +
 packfile.c           | 13 +++++++++++--
 9 files changed, 26 insertions(+), 14 deletions(-)
Show changes to 9 files +26 −14

builtin/gc.c, builtin/index-pack.c, config.c, environment.c, environment.h, pack-objects.h, pack-write.c, pack.h, packfile.c

diff --git a/builtin/gc.c b/builtin/gc.c
index d52735354c..9a10eb58bc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -138,6 +138,7 @@ struct gc_config {
 	char *repack_filter_to;
 	unsigned long big_pack_threshold;
 	unsigned long max_delta_cache_size;
+	unsigned long delta_base_cache_limit;
 };
 
 #define GC_CONFIG_INIT { \
@@ -153,6 +154,7 @@ struct gc_config {
 	.prune_expire = xstrdup("2.weeks.ago"), \
 	.prune_worktrees_expire = xstrdup("3.months.ago"), \
 	.max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \
+	.delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \
 }
 
 static void gc_config_release(struct gc_config *cfg)
@@ -205,6 +207,7 @@ static void gc_config(struct gc_config *cfg)
 
 	git_config_get_ulong("gc.bigpackthreshold", &cfg->big_pack_threshold);
 	git_config_get_ulong("pack.deltacachesize", &cfg->max_delta_cache_size);
+	git_config_get_ulong("core.deltabasecachelimit", &cfg->delta_base_cache_limit);
 
 	if (!git_config_get_string("gc.repackfilter", &owned)) {
 		free(cfg->repack_filter);
@@ -416,7 +419,7 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
 	 * read_sha1_file() (either at delta calculation phase, or
 	 * writing phase) also fills up the delta base cache
 	 */
-	heap += delta_base_cache_limit;
+	heap += cfg->delta_base_cache_limit;
 	/* and of course pack-objects has its own delta cache */
 	heap += cfg->max_delta_cache_size;
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index eaefb41761..23bfa45403 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1238,7 +1238,7 @@ static void parse_pack_objects(unsigned char *hash)
  *   recursively checking if the resulting object is used as a base
  *   for some more deltas.
  */
-static void resolve_deltas(void)
+static void resolve_deltas(struct pack_idx_option *opts)
 {
 	int i;
 
@@ -1254,7 +1254,7 @@ static void resolve_deltas(void)
 					  nr_ref_deltas + nr_ofs_deltas);
 
 	nr_dispatched = 0;
-	base_cache_limit = delta_base_cache_limit * nr_threads;
+	base_cache_limit = opts->delta_base_cache_limit * nr_threads;
 	if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) {
 		init_thread();
 		work_lock();
@@ -1604,6 +1604,10 @@ static int git_index_pack_config(const char *k, const char *v,
 		else
 			opts->flags &= ~WRITE_REV;
 	}
+	if (!strcmp(k, "core.deltabasecachelimit")) {
+		opts->delta_base_cache_limit = git_config_ulong(k, v, ctx->kvi);
+		return 0;
+	}
 	return git_default_config(k, v, ctx, cb);
 }
 
@@ -1930,7 +1934,7 @@ int cmd_index_pack(int argc,
 	parse_pack_objects(pack_hash);
 	if (report_end_of_input)
 		write_in_full(2, "\0", 1);
-	resolve_deltas();
+	resolve_deltas(&opts);
 	conclude_pack(fix_thin_pack, curr_pack, pack_hash);
 	free(ofs_deltas);
 	free(ref_deltas);
diff --git a/config.c b/config.c
index a11bb85da3..728ef98e42 100644
--- a/config.c
+++ b/config.c
@@ -1515,11 +1515,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.deltabasecachelimit")) {
-		delta_base_cache_limit = git_config_ulong(var, value, ctx->kvi);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.autocrlf")) {
 		if (value && !strcasecmp(value, "input")) {
 			auto_crlf = AUTO_CRLF_INPUT;
diff --git a/environment.c b/environment.c
index a2ce998081..8e5022c282 100644
--- a/environment.c
+++ b/environment.c
@@ -51,7 +51,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
 size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
 size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
-size_t delta_base_cache_limit = 96 * 1024 * 1024;
 unsigned long big_file_threshold = 512 * 1024 * 1024;
 char *editor_program;
 char *askpass_program;
diff --git a/environment.h b/environment.h
index 923e12661e..2f43340f0b 100644
--- a/environment.h
+++ b/environment.h
@@ -165,7 +165,6 @@ extern int zlib_compression_level;
 extern int pack_compression_level;
 extern size_t packed_git_window_size;
 extern size_t packed_git_limit;
-extern size_t delta_base_cache_limit;
 extern unsigned long big_file_threshold;
 extern unsigned long pack_size_limit_cfg;
 extern int max_allowed_tree_depth;
diff --git a/pack-objects.h b/pack-objects.h
index b9898a4e64..3f6f504203 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -7,7 +7,8 @@
 
 struct repository;
 
-#define DEFAULT_DELTA_CACHE_SIZE (256 * 1024 * 1024)
+#define DEFAULT_DELTA_CACHE_SIZE       (256 * 1024 * 1024)
+#define DEFAULT_DELTA_BASE_CACHE_LIMIT (96 * 1024 * 1024)
 
 #define OE_DFS_STATE_BITS	2
 #define OE_DEPTH_BITS		12
diff --git a/pack-write.c b/pack-write.c
index 8c7dfddc5a..98a8c0e785 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -21,6 +21,7 @@ void reset_pack_idx_option(struct pack_idx_option *opts)
 	memset(opts, 0, sizeof(*opts));
 	opts->version = 2;
 	opts->off32_limit = 0x7fffffff;
+	opts->delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
 }
 
 static int sha1_compare(const void *_a, const void *_b)
diff --git a/pack.h b/pack.h
index 02bbdfb19c..1a33751565 100644
--- a/pack.h
+++ b/pack.h
@@ -58,6 +58,7 @@ struct pack_idx_option {
 	 */
 	int anomaly_alloc, anomaly_nr;
 	uint32_t *anomaly;
+	unsigned long delta_base_cache_limit;
 };
 
 void reset_pack_idx_option(struct pack_idx_option *);
diff --git a/packfile.c b/packfile.c
index 9eca5a86a7..56a5c55a5d 100644
--- a/packfile.c
+++ b/packfile.c
@@ -24,6 +24,8 @@
 #include "commit-graph.h"
 #include "pack-revindex.h"
 #include "promisor-remote.h"
+#include "config.h"
+#include "pack-objects.h"
 
 char *odb_pack_name(struct repository *r, struct strbuf *buf,
 		    const unsigned char *hash, const char *ext)
@@ -1496,7 +1498,9 @@ void clear_delta_base_cache(void)
 }
 
 static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
-	void *base, unsigned long base_size, enum object_type type)
+				 void *base, unsigned long base_size,
+				 unsigned long delta_base_cache_limit,
+				 enum object_type type)
 {
 	struct delta_base_cache_entry *ent;
 	struct list_head *lru, *tmp;
@@ -1697,6 +1701,9 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
 	struct unpack_entry_stack_ent *delta_stack = small_delta_stack;
 	int delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC;
 	int base_from_cache = 0;
+	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
+
+	repo_config_get_ulong(r, "core.deltabasecachelimit", &delta_base_cache_limit);
 
 	write_pack_access_log(p, obj_offset);
 
@@ -1878,7 +1885,9 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
 		 * before we are done using it.
 		 */
 		if (!external_base)
-			add_delta_base_cache(p, base_obj_offset, base, base_size, type);
+			add_delta_base_cache(p, base_obj_offset, base,
+					     base_size, delta_base_cache_limit,
+					     type);
 
 		free(delta_data);
 		free(external_base);
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 8/9] config: make `packed_git_(limit|window_size)` non-global variables

The variables `packed_git_window_size` and `packed_git_limit` are global config variables used in the `packfile.c` file. Since it is only used in this file, let's change it from being a global config variable to a local variable for the subsystem.

We do this by introducing a new local `packfile_config` struct in `packfile.c` and also adding the required function to parse the said config. We then use this within `packfile.c` to obtain the variables.

With this, we rid `packfile.c` from all global variable usage and this means we can also remove the `USE_THE_REPOSITORY_VARIABLE` guard from the file.

Helped-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/fast-import.c |  4 +--
 config.c              | 17 -------------
 environment.c         |  2 --
 packfile.c            | 57 +++++++++++++++++++++++++++++++++++++------
 packfile.h            |  2 +-
 5 files changed, 52 insertions(+), 30 deletions(-)
Show changes to 5 files +52 −30

builtin/fast-import.c, config.c, environment.c, packfile.c, packfile.h

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 2ca8198153..c159cc584b 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3539,7 +3539,7 @@ static void parse_argv(void)
 int cmd_fast_import(int argc,
 		    const char **argv,
 		    const char *prefix,
-		    struct repository *repo UNUSED)
+		    struct repository *repo)
 {
 	unsigned int i;
 
@@ -3660,7 +3660,7 @@ int cmd_fast_import(int argc,
 		fprintf(stderr, "       pools:    %10lu KiB\n", (unsigned long)((tree_entry_allocd + fi_mem_pool.pool_alloc) /1024));
 		fprintf(stderr, "     objects:    %10" PRIuMAX " KiB\n", (alloc_count*sizeof(struct object_entry))/1024);
 		fprintf(stderr, "---------------------------------------------------------------------\n");
-		pack_report();
+		pack_report(repo);
 		fprintf(stderr, "---------------------------------------------------------------------\n");
 		fprintf(stderr, "\n");
 	}
diff --git a/config.c b/config.c
index 728ef98e42..2c295f7430 100644
--- a/config.c
+++ b/config.c
@@ -1493,28 +1493,11 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.packedgitwindowsize")) {
-		int pgsz_x2 = getpagesize() * 2;
-		packed_git_window_size = git_config_ulong(var, value, ctx->kvi);
-
-		/* This value must be multiple of (pagesize * 2) */
-		packed_git_window_size /= pgsz_x2;
-		if (packed_git_window_size < 1)
-			packed_git_window_size = 1;
-		packed_git_window_size *= pgsz_x2;
-		return 0;
-	}
-
 	if (!strcmp(var, "core.bigfilethreshold")) {
 		big_file_threshold = git_config_ulong(var, value, ctx->kvi);
 		return 0;
 	}
 
-	if (!strcmp(var, "core.packedgitlimit")) {
-		packed_git_limit = git_config_ulong(var, value, ctx->kvi);
-		return 0;
-	}
-
 	if (!strcmp(var, "core.autocrlf")) {
 		if (value && !strcasecmp(value, "input")) {
 			auto_crlf = AUTO_CRLF_INPUT;
diff --git a/environment.c b/environment.c
index 8e5022c282..8389a27270 100644
--- a/environment.c
+++ b/environment.c
@@ -49,8 +49,6 @@ int fsync_object_files = -1;
 int use_fsync = -1;
 enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
 enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
-size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
-size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
 unsigned long big_file_threshold = 512 * 1024 * 1024;
 char *editor_program;
 char *askpass_program;
diff --git a/packfile.c b/packfile.c
index 56a5c55a5d..426606585f 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 
 #include "git-compat-util.h"
 #include "environment.h"
@@ -27,6 +26,17 @@
 #include "config.h"
 #include "pack-objects.h"
 
+struct packfile_config {
+	unsigned long packed_git_window_size;
+	unsigned long packed_git_limit;
+};
+
+#define PACKFILE_CONFIG_INIT \
+{ \
+	.packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE, \
+	.packed_git_limit = DEFAULT_PACKED_GIT_LIMIT, \
+}
+
 char *odb_pack_name(struct repository *r, struct strbuf *buf,
 		    const unsigned char *hash, const char *ext)
 {
@@ -48,15 +58,41 @@ static size_t pack_mapped;
 #define SZ_FMT PRIuMAX
 static inline uintmax_t sz_fmt(size_t s) { return s; }
 
-void pack_report(void)
+static int packfile_config(const char *var, const char *value,
+			   const struct config_context *ctx, void *cb)
+{
+	struct packfile_config *config = cb;
+
+	if (!strcmp(var, "core.packedgitwindowsize")) {
+		int pgsz_x2 = getpagesize() * 2;
+		config->packed_git_window_size = git_config_ulong(var, value, ctx->kvi);
+
+		/* This value must be multiple of (pagesize * 2) */
+		config->packed_git_window_size /= pgsz_x2;
+		if (config->packed_git_window_size < 1)
+			config->packed_git_window_size = 1;
+		config->packed_git_window_size *= pgsz_x2;
+		return 0;
+	} else if (!strcmp(var, "core.packedgitlimit")) {
+		config->packed_git_limit = git_config_ulong(var, value, ctx->kvi);
+		return 0;
+	} else {
+		return git_default_config(var, value, ctx, cb);
+	}
+}
+
+void pack_report(struct repository *repo)
 {
+	struct packfile_config config = PACKFILE_CONFIG_INIT;
+	repo_config(repo, packfile_config, &config);
+
 	fprintf(stderr,
 		"pack_report: getpagesize()            = %10" SZ_FMT "\n"
 		"pack_report: core.packedGitWindowSize = %10" SZ_FMT "\n"
 		"pack_report: core.packedGitLimit      = %10" SZ_FMT "\n",
 		sz_fmt(getpagesize()),
-		sz_fmt(packed_git_window_size),
-		sz_fmt(packed_git_limit));
+		sz_fmt(config.packed_git_window_size),
+		sz_fmt(config.packed_git_limit));
 	fprintf(stderr,
 		"pack_report: pack_used_ctr            = %10u\n"
 		"pack_report: pack_mmap_calls          = %10u\n"
@@ -652,20 +688,25 @@ unsigned char *use_pack(struct packed_git *p,
 				break;
 		}
 		if (!win) {
-			size_t window_align = packed_git_window_size / 2;
+			struct packfile_config config = PACKFILE_CONFIG_INIT;
+			size_t window_align;
 			off_t len;
 
+			repo_config(p->r, packfile_config, &config);
+			window_align = config.packed_git_window_size / 2;
+
 			if (p->pack_fd == -1 && open_packed_git(p))
 				die("packfile %s cannot be accessed", p->pack_name);
 
 			CALLOC_ARRAY(win, 1);
 			win->offset = (offset / window_align) * window_align;
 			len = p->pack_size - win->offset;
-			if (len > packed_git_window_size)
-				len = packed_git_window_size;
+			if (len > config.packed_git_window_size)
+				len = config.packed_git_window_size;
 			win->len = (size_t)len;
 			pack_mapped += win->len;
-			while (packed_git_limit < pack_mapped
+
+			while (config.packed_git_limit < pack_mapped
 				&& unuse_one_window(p))
 				; /* nothing */
 			win->base = xmmap_gently(NULL, win->len,
diff --git a/packfile.h b/packfile.h
index addb95b0c4..58104fa009 100644
--- a/packfile.h
+++ b/packfile.h
@@ -89,7 +89,7 @@ unsigned long repo_approximate_object_count(struct repository *r);
 struct packed_git *find_oid_pack(const struct object_id *oid,
 				 struct packed_git *packs);
 
-void pack_report(void);
+void pack_report(struct repository *repo);
 
 /*
  * mmap the index file for the specified packfile (if it is not
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 6/9] packfile: pass down repository to `for_each_packed_object`

The function `for_each_packed_object` currently relies on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Let's remove its usage from this function and closely related function `is_promisor_object`.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/cat-file.c     |  7 ++++---
 builtin/fsck.c         | 18 +++++++++++-------
 builtin/pack-objects.c |  7 +++++--
 builtin/repack.c       |  2 +-
 builtin/rev-list.c     |  2 +-
 commit-graph.c         |  2 +-
 fsck.c                 |  2 +-
 list-objects.c         |  4 ++--
 object-store-ll.h      |  4 ++--
 packfile.c             | 14 +++++++-------
 packfile.h             |  2 +-
 promisor-remote.c      |  2 +-
 reachable.c            |  2 +-
 revision.c             |  9 +++++----
 tag.c                  |  2 +-
 15 files changed, 44 insertions(+), 35 deletions(-)
Show changes to 15 files +44 −35

builtin/cat-file.c, builtin/fsck.c, builtin/pack-objects.c, builtin/repack.c, builtin/rev-list.c, commit-graph.c, fsck.c, list-objects.c, object-store-ll.h, packfile.c, packfile.h, promisor-remote.c, reachable.c, revision.c, tag.c

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index bfdfb51c7c..d67b101c20 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -827,15 +827,16 @@ static int batch_objects(struct batch_options *opt)
 			cb.seen = &seen;
 
 			for_each_loose_object(batch_unordered_loose, &cb, 0);
-			for_each_packed_object(batch_unordered_packed, &cb,
-					       FOR_EACH_OBJECT_PACK_ORDER);
+			for_each_packed_object(the_repository, batch_unordered_packed,
+					       &cb, FOR_EACH_OBJECT_PACK_ORDER);
 
 			oidset_clear(&seen);
 		} else {
 			struct oid_array sa = OID_ARRAY_INIT;
 
 			for_each_loose_object(collect_loose_object, &sa, 0);
-			for_each_packed_object(collect_packed_object, &sa, 0);
+			for_each_packed_object(the_repository, collect_packed_object,
+					       &sa, 0);
 
 			oid_array_for_each_unique(&sa, batch_object_cb, &cb);
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index bb56eb98ac..0196c54eb6 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -150,7 +150,7 @@ static int mark_object(struct object *obj, enum object_type type,
 		return 0;
 	obj->flags |= REACHABLE;
 
-	if (is_promisor_object(&obj->oid))
+	if (is_promisor_object(the_repository, &obj->oid))
 		/*
 		 * Further recursion does not need to be performed on this
 		 * object since it is a promisor object (so it does not need to
@@ -270,7 +270,7 @@ static void check_reachable_object(struct object *obj)
 	 * do a full fsck
 	 */
 	if (!(obj->flags & HAS_OBJ)) {
-		if (is_promisor_object(&obj->oid))
+		if (is_promisor_object(the_repository, &obj->oid))
 			return;
 		if (has_object_pack(the_repository, &obj->oid))
 			return; /* it is in pack - forget about it */
@@ -391,7 +391,10 @@ static void check_connectivity(void)
 		 * traversal.
 		 */
 		for_each_loose_object(mark_loose_unreachable_referents, NULL, 0);
-		for_each_packed_object(mark_packed_unreachable_referents, NULL, 0);
+		for_each_packed_object(the_repository,
+				       mark_packed_unreachable_referents,
+				       NULL,
+				       0);
 	}
 
 	/* Look up all the requirements, warn about missing objects.. */
@@ -488,7 +491,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,
 						     refname, timestamp);
 			obj->flags |= USED;
 			mark_object_reachable(obj);
-		} else if (!is_promisor_object(oid)) {
+		} else if (!is_promisor_object(the_repository, oid)) {
 			error(_("%s: invalid reflog entry %s"),
 			      refname, oid_to_hex(oid));
 			errors_found |= ERROR_REACHABLE;
@@ -531,7 +534,7 @@ static int fsck_handle_ref(const char *refname, const char *referent UNUSED, con
 
 	obj = parse_object(the_repository, oid);
 	if (!obj) {
-		if (is_promisor_object(oid)) {
+		if (is_promisor_object(the_repository, oid)) {
 			/*
 			 * Increment default_refs anyway, because this is a
 			 * valid ref.
@@ -966,7 +969,8 @@ int cmd_fsck(int argc,
 
 	if (connectivity_only) {
 		for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
-		for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
+		for_each_packed_object(the_repository,
+				       mark_packed_for_connectivity, NULL, 0);
 	} else {
 		prepare_alt_odb(the_repository);
 		for (odb = the_repository->objects->odb; odb; odb = odb->next)
@@ -1011,7 +1015,7 @@ int cmd_fsck(int argc,
 							   &oid);
 
 			if (!obj || !(obj->flags & HAS_OBJ)) {
-				if (is_promisor_object(&oid))
+				if (is_promisor_object(the_repository, &oid))
 					continue;
 				error(_("%s: object missing"), oid_to_hex(&oid));
 				errors_found |= ERROR_OBJECT;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ceb7e76b10..755d521440 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3858,7 +3858,8 @@ static void show_object__ma_allow_promisor(struct object *obj, const char *name,
 	 * Quietly ignore EXPECTED missing objects.  This avoids problems with
 	 * staging them now and getting an odd error later.
 	 */
-	if (!has_object(the_repository, &obj->oid, 0) && is_promisor_object(&obj->oid))
+	if (!has_object(the_repository, &obj->oid, 0) &&
+	    is_promisor_object(to_pack.repo, &obj->oid))
 		return;
 
 	show_object(obj, name, data);
@@ -3927,7 +3928,9 @@ static int add_object_in_unpacked_pack(const struct object_id *oid,
 
 static void add_objects_in_unpacked_packs(void)
 {
-	if (for_each_packed_object(add_object_in_unpacked_pack, NULL,
+	if (for_each_packed_object(to_pack.repo,
+				   add_object_in_unpacked_pack,
+				   NULL,
 				   FOR_EACH_OBJECT_PACK_ORDER |
 				   FOR_EACH_OBJECT_LOCAL_ONLY |
 				   FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS |
diff --git a/builtin/repack.c b/builtin/repack.c
index d6bb37e84a..96a4fa234b 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -404,7 +404,7 @@ static void repack_promisor_objects(const struct pack_objects_args *args,
 	 * {type -> existing pack order} ordering when computing deltas instead
 	 * of a {type -> size} ordering, which may produce better deltas.
 	 */
-	for_each_packed_object(write_oid, &cmd,
+	for_each_packed_object(the_repository, write_oid, &cmd,
 			       FOR_EACH_OBJECT_PROMISOR_ONLY);
 
 	if (cmd.in == -1) {
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index f62bcbf2b1..43c42621e3 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -121,7 +121,7 @@ static inline void finish_object__ma(struct object *obj)
 		return;
 
 	case MA_ALLOW_PROMISOR:
-		if (is_promisor_object(&obj->oid))
+		if (is_promisor_object(the_repository, &obj->oid))
 			return;
 		die("unexpected missing %s object '%s'",
 		    type_name(obj->type), oid_to_hex(&obj->oid));
diff --git a/commit-graph.c b/commit-graph.c
index 83dd69bfeb..e2e2083951 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1960,7 +1960,7 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx)
 		ctx->progress = start_delayed_progress(
 			_("Finding commits for commit graph among packed objects"),
 			ctx->approx_nr_objects);
-	for_each_packed_object(add_packed_commits, ctx,
+	for_each_packed_object(ctx->r, add_packed_commits, ctx,
 			       FOR_EACH_OBJECT_PACK_ORDER);
 	if (ctx->progress_done < ctx->approx_nr_objects)
 		display_progress(ctx->progress, ctx->approx_nr_objects);
diff --git a/fsck.c b/fsck.c
index 3756f52459..87ce999a49 100644
--- a/fsck.c
+++ b/fsck.c
@@ -1295,7 +1295,7 @@ static int fsck_blobs(struct oidset *blobs_found, struct oidset *blobs_done,
 
 		buf = repo_read_object_file(the_repository, oid, &type, &size);
 		if (!buf) {
-			if (is_promisor_object(oid))
+			if (is_promisor_object(the_repository, oid))
 				continue;
 			ret |= report(options,
 				      oid, OBJ_BLOB, msg_missing,
diff --git a/list-objects.c b/list-objects.c
index 31236a8dc9..d11a389b3a 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -75,7 +75,7 @@ static void process_blob(struct traversal_context *ctx,
 	 */
 	if (ctx->revs->exclude_promisor_objects &&
 	    !repo_has_object_file(the_repository, &obj->oid) &&
-	    is_promisor_object(&obj->oid))
+	    is_promisor_object(ctx->revs->repo, &obj->oid))
 		return;
 
 	pathlen = path->len;
@@ -180,7 +180,7 @@ static void process_tree(struct traversal_context *ctx,
 		 * an incomplete list of missing objects.
 		 */
 		if (revs->exclude_promisor_objects &&
-		    is_promisor_object(&obj->oid))
+		    is_promisor_object(revs->repo, &obj->oid))
 			return;
 
 		if (!revs->do_not_die_on_missing_objects)
diff --git a/object-store-ll.h b/object-store-ll.h
index e8a22ab5fc..ce73d9a670 100644
--- a/object-store-ll.h
+++ b/object-store-ll.h
@@ -550,7 +550,7 @@ typedef int each_packed_object_fn(const struct object_id *oid,
 int for_each_object_in_pack(struct packed_git *p,
 			    each_packed_object_fn, void *data,
 			    enum for_each_object_flags flags);
-int for_each_packed_object(each_packed_object_fn, void *,
-			   enum for_each_object_flags flags);
+int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,
+			   void *data, enum for_each_object_flags flags);
 
 #endif /* OBJECT_STORE_LL_H */
diff --git a/packfile.c b/packfile.c
index 3894646573..9eca5a86a7 100644
--- a/packfile.c
+++ b/packfile.c
@@ -2200,15 +2200,15 @@ int for_each_object_in_pack(struct packed_git *p,
 	return r;
 }
 
-int for_each_packed_object(each_packed_object_fn cb, void *data,
-			   enum for_each_object_flags flags)
+int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,
+			   void *data, enum for_each_object_flags flags)
 {
 	struct packed_git *p;
 	int r = 0;
 	int pack_errors = 0;
 
-	prepare_packed_git(the_repository);
-	for (p = get_all_packs(the_repository); p; p = p->next) {
+	prepare_packed_git(repo);
+	for (p = get_all_packs(repo); p; p = p->next) {
 		if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
 			continue;
 		if ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) &&
@@ -2286,14 +2286,14 @@ static int add_promisor_object(const struct object_id *oid,
 	return 0;
 }
 
-int is_promisor_object(const struct object_id *oid)
+int is_promisor_object(struct repository *r, const struct object_id *oid)
 {
 	static struct oidset promisor_objects;
 	static int promisor_objects_prepared;
 
 	if (!promisor_objects_prepared) {
-		if (repo_has_promisor_remote(the_repository)) {
-			for_each_packed_object(add_promisor_object,
+		if (repo_has_promisor_remote(r)) {
+			for_each_packed_object(r, add_promisor_object,
 					       &promisor_objects,
 					       FOR_EACH_OBJECT_PROMISOR_ONLY |
 					       FOR_EACH_OBJECT_PACK_ORDER);
diff --git a/packfile.h b/packfile.h
index b09fb2c530..addb95b0c4 100644
--- a/packfile.h
+++ b/packfile.h
@@ -201,7 +201,7 @@ int has_object_kept_pack(struct repository *r, const struct object_id *oid,
  * Return 1 if an object in a promisor packfile is or refers to the given
  * object, 0 otherwise.
  */
-int is_promisor_object(const struct object_id *oid);
+int is_promisor_object(struct repository *r, const struct object_id *oid);
 
 /*
  * Expose a function for fuzz testing.
diff --git a/promisor-remote.c b/promisor-remote.c
index 9345ae3db2..c714f4f007 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -283,7 +283,7 @@ void promisor_remote_get_direct(struct repository *repo,
 	}
 
 	for (i = 0; i < remaining_nr; i++) {
-		if (is_promisor_object(&remaining_oids[i]))
+		if (is_promisor_object(repo, &remaining_oids[i]))
 			die(_("could not fetch %s from promisor remote"),
 			    oid_to_hex(&remaining_oids[i]));
 	}
diff --git a/reachable.c b/reachable.c
index 09d2c50079..ecf7ccf504 100644
--- a/reachable.c
+++ b/reachable.c
@@ -324,7 +324,7 @@ int add_unseen_recent_objects_to_traversal(struct rev_info *revs,
 	if (ignore_in_core_kept_packs)
 		flags |= FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS;
 
-	r = for_each_packed_object(add_recent_packed, &data, flags);
+	r = for_each_packed_object(revs->repo, add_recent_packed, &data, flags);
 
 done:
 	oidset_clear(&data.extra_recent_oids);
diff --git a/revision.c b/revision.c
index d1d152a67b..45dc6d2819 100644
--- a/revision.c
+++ b/revision.c
@@ -390,7 +390,8 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
 	if (!object) {
 		if (revs->ignore_missing)
 			return NULL;
-		if (revs->exclude_promisor_objects && is_promisor_object(oid))
+		if (revs->exclude_promisor_objects &&
+		    is_promisor_object(revs->repo, oid))
 			return NULL;
 		if (revs->do_not_die_on_missing_objects) {
 			oidset_insert(&revs->missing_commits, oid);
@@ -432,7 +433,7 @@ static struct commit *handle_commit(struct rev_info *revs,
 			if (revs->ignore_missing_links || (flags & UNINTERESTING))
 				return NULL;
 			if (revs->exclude_promisor_objects &&
-			    is_promisor_object(&tag->tagged->oid))
+			    is_promisor_object(revs->repo, &tag->tagged->oid))
 				return NULL;
 			if (revs->do_not_die_on_missing_objects && oid) {
 				oidset_insert(&revs->missing_commits, oid);
@@ -1211,7 +1212,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
 			     revs->do_not_die_on_missing_objects;
 		if (repo_parse_commit_gently(revs->repo, p, gently) < 0) {
 			if (revs->exclude_promisor_objects &&
-			    is_promisor_object(&p->object.oid)) {
+			    is_promisor_object(revs->repo, &p->object.oid)) {
 				if (revs->first_parent_only)
 					break;
 				continue;
@@ -3915,7 +3916,7 @@ int prepare_revision_walk(struct rev_info *revs)
 		revs->treesame.name = "treesame";
 
 	if (revs->exclude_promisor_objects) {
-		for_each_packed_object(mark_uninteresting, revs,
+		for_each_packed_object(revs->repo, mark_uninteresting, revs,
 				       FOR_EACH_OBJECT_PROMISOR_ONLY);
 	}
 
diff --git a/tag.c b/tag.c
index d24170e340..beef9571b5 100644
--- a/tag.c
+++ b/tag.c
@@ -84,7 +84,7 @@ struct object *deref_tag(struct repository *r, struct object *o, const char *war
 			o = NULL;
 		}
 	if (!o && warn) {
-		if (last_oid && is_promisor_object(last_oid))
+		if (last_oid && is_promisor_object(r, last_oid))
 			return NULL;
 		if (!warnlen)
 			warnlen = strlen(warn);
-- 
2.47.0
Karthik Nayak· Oct 30, 2024, 14:32 UTC · re: Karthik Nayak · lore

[PATCH v3 9/9] midx: add repository to `multi_pack_index` struct

The `multi_pack_index` struct represents the MIDX for a repository. Here, we add a pointer to the repository in this struct, allowing direct use of the repository variable without relying on the global `the_repository` struct.

With this addition, we can determine the repository associated with a `bitmap_index` struct. A `bitmap_index` points to either a `packed_git` or a `multi_pack_index`, both of which have direct repository references. To support this, we introduce a static helper function, `bitmap_repo`, in `pack-bitmap.c`, which retrieves a repository given a `bitmap_index`.

With this, we clear up all usages of `the_repository` within `pack-bitmap.c` and also remove the `USE_THE_REPOSITORY_VARIABLE` definition. Bringing us another step closer to remove all global variable usage.

Although this change also opens up the potential to clean up `midx.c`, doing so would require additional refactoring to pass the repository struct to functions where the MIDX struct is created: a task better suited for future patches.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 midx.c        |  1 +
 midx.h        |  3 ++
 pack-bitmap.c | 97 +++++++++++++++++++++++++++++++--------------------
 3 files changed, 64 insertions(+), 37 deletions(-)
Show changes to 3 files +64 −37

midx.c, midx.h, pack-bitmap.c

diff --git a/midx.c b/midx.c
index 8edb75f51d..7a34473010 100644
--- a/midx.c
+++ b/midx.c
@@ -131,6 +131,7 @@ static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir
 	m->data = midx_map;
 	m->data_len = midx_size;
 	m->local = local;
+	m->r = the_repository;
 
 	m->signature = get_be32(m->data);
 	if (m->signature != MIDX_SIGNATURE)
diff --git a/midx.h b/midx.h
index 42d4f8d149..7d39fb24e9 100644
--- a/midx.h
+++ b/midx.h
@@ -71,6 +71,9 @@ struct multi_pack_index {
 
 	const char **pack_names;
 	struct packed_git **packs;
+
+	struct repository *r;
+
 	char object_dir[FLEX_ARRAY];
 };
 
diff --git a/pack-bitmap.c b/pack-bitmap.c
index d34ba9909a..ef9958b96e 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "git-compat-util.h"
 #include "commit.h"
 #include "gettext.h"
@@ -177,12 +175,21 @@ static uint32_t bitmap_num_objects(struct bitmap_index *index)
 	return index->pack->num_objects;
 }
 
+static struct repository *bitmap_repo(struct bitmap_index *bitmap_git)
+{
+	if (bitmap_is_midx(bitmap_git))
+		return bitmap_git->midx->r;
+	return bitmap_git->pack->r;
+}
+
 static int load_bitmap_header(struct bitmap_index *index)
 {
 	struct bitmap_disk_header *header = (void *)index->map;
-	size_t header_size = sizeof(*header) - GIT_MAX_RAWSZ + the_hash_algo->rawsz;
+	const struct git_hash_algo *hash_algo = bitmap_repo(index)->hash_algo;
+
+	size_t header_size = sizeof(*header) - GIT_MAX_RAWSZ + hash_algo->rawsz;
 
-	if (index->map_size < header_size + the_hash_algo->rawsz)
+	if (index->map_size < header_size + hash_algo->rawsz)
 		return error(_("corrupted bitmap index (too small)"));
 
 	if (memcmp(header->magic, BITMAP_IDX_SIGNATURE, sizeof(BITMAP_IDX_SIGNATURE)) != 0)
@@ -196,7 +203,7 @@ static int load_bitmap_header(struct bitmap_index *index)
 	{
 		uint32_t flags = ntohs(header->options);
 		size_t cache_size = st_mult(bitmap_num_objects(index), sizeof(uint32_t));
-		unsigned char *index_end = index->map + index->map_size - the_hash_algo->rawsz;
+		unsigned char *index_end = index->map + index->map_size - hash_algo->rawsz;
 
 		if ((flags & BITMAP_OPT_FULL_DAG) == 0)
 			BUG("unsupported options for bitmap index file "
@@ -409,7 +416,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 	if (bitmap_git->pack || bitmap_git->midx) {
 		struct strbuf buf = STRBUF_INIT;
 		get_midx_filename(&buf, midx->object_dir);
-		trace2_data_string("bitmap", the_repository,
+		trace2_data_string("bitmap", bitmap_repo(bitmap_git),
 				   "ignoring extra midx bitmap file", buf.buf);
 		close(fd);
 		strbuf_release(&buf);
@@ -427,7 +434,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 		goto cleanup;
 
 	if (!hasheq(get_midx_checksum(bitmap_git->midx), bitmap_git->checksum,
-		    the_repository->hash_algo)) {
+		    bitmap_repo(bitmap_git)->hash_algo)) {
 		error(_("checksum doesn't match in MIDX and bitmap"));
 		goto cleanup;
 	}
@@ -438,7 +445,9 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
 	}
 
 	for (i = 0; i < bitmap_git->midx->num_packs; i++) {
-		if (prepare_midx_pack(the_repository, bitmap_git->midx, i)) {
+		if (prepare_midx_pack(bitmap_repo(bitmap_git),
+				      bitmap_git->midx,
+				      i)) {
 			warning(_("could not open pack %s"),
 				bitmap_git->midx->pack_names[i]);
 			goto cleanup;
@@ -492,8 +501,9 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
 	}
 
 	if (bitmap_git->pack || bitmap_git->midx) {
-		trace2_data_string("bitmap", the_repository,
-				   "ignoring extra bitmap file", packfile->pack_name);
+		trace2_data_string("bitmap", bitmap_repo(bitmap_git),
+				   "ignoring extra bitmap file",
+				   packfile->pack_name);
 		close(fd);
 		return -1;
 	}
@@ -518,8 +528,8 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git
 		return -1;
 	}
 
-	trace2_data_string("bitmap", the_repository, "opened bitmap file",
-			   packfile->pack_name);
+	trace2_data_string("bitmap", bitmap_repo(bitmap_git),
+			   "opened bitmap file", packfile->pack_name);
 	return 0;
 }
 
@@ -649,7 +659,7 @@ struct bitmap_index *prepare_bitmap_git(struct repository *r)
 
 struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
 {
-	struct repository *r = the_repository;
+	struct repository *r = midx->r;
 	struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
 
 	if (!open_midx_bitmap_1(bitmap_git, midx) && !load_bitmap(r, bitmap_git))
@@ -1213,6 +1223,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
 {
 	struct bitmap_boundary_cb cb;
 	struct object_list *root;
+	struct repository *repo;
 	unsigned int i;
 	unsigned int tmp_blobs, tmp_trees, tmp_tags;
 	int any_missing = 0;
@@ -1222,6 +1233,8 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
 	cb.base = bitmap_new();
 	object_array_init(&cb.boundary);
 
+	repo = bitmap_repo(bitmap_git);
+
 	revs->ignore_missing_links = 1;
 
 	if (bitmap_git->pseudo_merges.nr) {
@@ -1280,19 +1293,19 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
 	 * revision walk to (a) OR in any bitmaps that are UNINTERESTING
 	 * between the tips and boundary, and (b) record the boundary.
 	 */
-	trace2_region_enter("pack-bitmap", "boundary-prepare", the_repository);
+	trace2_region_enter("pack-bitmap", "boundary-prepare", repo);
 	if (prepare_revision_walk(revs))
 		die("revision walk setup failed");
-	trace2_region_leave("pack-bitmap", "boundary-prepare", the_repository);
+	trace2_region_leave("pack-bitmap", "boundary-prepare", repo);
 
-	trace2_region_enter("pack-bitmap", "boundary-traverse", the_repository);
+	trace2_region_enter("pack-bitmap", "boundary-traverse", repo);
 	revs->boundary = 1;
 	traverse_commit_list_filtered(revs,
 				      show_boundary_commit,
 				      show_boundary_object,
 				      &cb, NULL);
 	revs->boundary = 0;
-	trace2_region_leave("pack-bitmap", "boundary-traverse", the_repository);
+	trace2_region_leave("pack-bitmap", "boundary-traverse", repo);
 
 	revs->blob_objects = tmp_blobs;
 	revs->tree_objects = tmp_trees;
@@ -1304,7 +1317,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
 	/*
 	 * Then add the boundary commit(s) as fill-in traversal tips.
 	 */
-	trace2_region_enter("pack-bitmap", "boundary-fill-in", the_repository);
+	trace2_region_enter("pack-bitmap", "boundary-fill-in", repo);
 	for (i = 0; i < cb.boundary.nr; i++) {
 		struct object *obj = cb.boundary.objects[i].item;
 		if (bitmap_walk_contains(bitmap_git, cb.base, &obj->oid))
@@ -1314,7 +1327,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git,
 	}
 	if (revs->pending.nr)
 		cb.base = fill_in_bitmap(bitmap_git, revs, cb.base, NULL);
-	trace2_region_leave("pack-bitmap", "boundary-fill-in", the_repository);
+	trace2_region_leave("pack-bitmap", "boundary-fill-in", repo);
 
 cleanup:
 	object_array_clear(&cb.boundary);
@@ -1718,7 +1731,8 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,
 			ofs = pack_pos_to_offset(pack, pos);
 		}
 
-		if (packed_object_info(the_repository, pack, ofs, &oi) < 0) {
+		if (packed_object_info(bitmap_repo(bitmap_git), pack, ofs,
+				       &oi) < 0) {
 			struct object_id oid;
 			nth_bitmap_object_oid(bitmap_git, &oid,
 					      pack_pos_to_index(pack, pos));
@@ -1727,7 +1741,8 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,
 	} else {
 		struct eindex *eindex = &bitmap_git->ext_index;
 		struct object *obj = eindex->objects[pos - bitmap_num_objects(bitmap_git)];
-		if (oid_object_info_extended(the_repository, &obj->oid, &oi, 0) < 0)
+		if (oid_object_info_extended(bitmap_repo(bitmap_git), &obj->oid,
+					     &oi, 0) < 0)
 			die(_("unable to get size of %s"), oid_to_hex(&obj->oid));
 	}
 
@@ -1889,7 +1904,8 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
 		bitmap_unset(result, i);
 
 	for (i = 0; i < eindex->count; ++i) {
-		if (has_object_pack(the_repository, &eindex->objects[i]->oid))
+		if (has_object_pack(bitmap_repo(bitmap_git),
+				    &eindex->objects[i]->oid))
 			bitmap_unset(result, objects_nr + i);
 	}
 }
@@ -1907,6 +1923,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 	struct bitmap *haves_bitmap = NULL;
 
 	struct bitmap_index *bitmap_git;
+	struct repository *repo;
 
 	/*
 	 * We can't do pathspec limiting with bitmaps, because we don't know
@@ -1980,18 +1997,23 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 	if (!use_boundary_traversal)
 		object_array_clear(&revs->pending);
 
+	repo = bitmap_repo(bitmap_git);
+
 	if (haves) {
-		if (use_boundary_traversal) {
-			trace2_region_enter("pack-bitmap", "haves/boundary", the_repository);
+		if (use_boundary_traversal)
+		{
+			trace2_region_enter("pack-bitmap", "haves/boundary", repo);
 			haves_bitmap = find_boundary_objects(bitmap_git, revs, haves);
-			trace2_region_leave("pack-bitmap", "haves/boundary", the_repository);
-		} else {
-			trace2_region_enter("pack-bitmap", "haves/classic", the_repository);
+			trace2_region_leave("pack-bitmap", "haves/boundary", repo);
+		}
+		else
+		{
+			trace2_region_enter("pack-bitmap", "haves/classic", repo);
 			revs->ignore_missing_links = 1;
 			haves_bitmap = find_objects(bitmap_git, revs, haves, NULL);
 			reset_revision_walk();
 			revs->ignore_missing_links = 0;
-			trace2_region_leave("pack-bitmap", "haves/classic", the_repository);
+			trace2_region_leave("pack-bitmap", "haves/classic", repo);
 		}
 
 		if (!haves_bitmap)
@@ -2025,17 +2047,17 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
 	object_list_free(&wants);
 	object_list_free(&haves);
 
-	trace2_data_intmax("bitmap", the_repository, "pseudo_merges_satisfied",
+	trace2_data_intmax("bitmap", repo, "pseudo_merges_satisfied",
 			   pseudo_merges_satisfied_nr);
-	trace2_data_intmax("bitmap", the_repository, "pseudo_merges_cascades",
+	trace2_data_intmax("bitmap", repo, "pseudo_merges_cascades",
 			   pseudo_merges_cascades_nr);
-	trace2_data_intmax("bitmap", the_repository, "bitmap/hits",
+	trace2_data_intmax("bitmap", repo, "bitmap/hits",
 			   existing_bitmaps_hits_nr);
-	trace2_data_intmax("bitmap", the_repository, "bitmap/misses",
+	trace2_data_intmax("bitmap", repo, "bitmap/misses",
 			   existing_bitmaps_misses_nr);
-	trace2_data_intmax("bitmap", the_repository, "bitmap/roots_with_bitmap",
+	trace2_data_intmax("bitmap", repo, "bitmap/roots_with_bitmap",
 			   roots_with_bitmaps_nr);
-	trace2_data_intmax("bitmap", the_repository, "bitmap/roots_without_bitmap",
+	trace2_data_intmax("bitmap", repo, "bitmap/roots_without_bitmap",
 			   roots_without_bitmaps_nr);
 
 	return bitmap_git;
@@ -2256,7 +2278,7 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
 					struct bitmap **reuse_out,
 					int multi_pack_reuse)
 {
-	struct repository *r = the_repository;
+	struct repository *r = bitmap_repo(bitmap_git);
 	struct bitmapped_pack *packs = NULL;
 	struct bitmap *result = bitmap_git->result;
 	struct bitmap *reuse;
@@ -2792,7 +2814,7 @@ int rebuild_bitmap(const uint32_t *reposition,
 uint32_t *create_bitmap_mapping(struct bitmap_index *bitmap_git,
 				struct packing_data *mapping)
 {
-	struct repository *r = the_repository;
+	struct repository *r = bitmap_repo(bitmap_git);
 	uint32_t i, num_objects;
 	uint32_t *reposition;
 
@@ -2948,7 +2970,8 @@ static off_t get_disk_usage_for_extended(struct bitmap_index *bitmap_git)
 				st_add(bitmap_num_objects(bitmap_git), i)))
 			continue;
 
-		if (oid_object_info_extended(the_repository, &obj->oid, &oi, 0) < 0)
+		if (oid_object_info_extended(bitmap_repo(bitmap_git), &obj->oid,
+					     &oi, 0) < 0)
 			die(_("unable to get disk usage of '%s'"),
 			    oid_to_hex(&obj->oid));
 
-- 
2.47.0
Taylor Blau· Oct 30, 2024, 20:13 UTC · re: Karthik Nayak · lore

Re: [PATCH v3 9/9] midx: add repository to `multi_pack_index` struct

On Wed, Oct 30, 2024 at 03:32:34PM +0100, Karthik Nayak wrote:
Show 38 quoted lines
> The `multi_pack_index` struct represents the MIDX for a repository.
> Here, we add a pointer to the repository in this struct, allowing direct
> use of the repository variable without relying on the global
> `the_repository` struct.
>
> With this addition, we can determine the repository associated with a
> `bitmap_index` struct. A `bitmap_index` points to either a `packed_git`
> or a `multi_pack_index`, both of which have direct repository
> references. To support this, we introduce a static helper function,
> `bitmap_repo`, in `pack-bitmap.c`, which retrieves a repository given a
> `bitmap_index`.
>
> With this, we clear up all usages of `the_repository` within
> `pack-bitmap.c` and also remove the `USE_THE_REPOSITORY_VARIABLE`
> definition. Bringing us another step closer to remove all global
> variable usage.
>
> Although this change also opens up the potential to clean up `midx.c`,
> doing so would require additional refactoring to pass the repository
> struct to functions where the MIDX struct is created: a task better
> suited for future patches.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  midx.c        |  1 +
>  midx.h        |  3 ++
>  pack-bitmap.c | 97 +++++++++++++++++++++++++++++++--------------------
>  3 files changed, 64 insertions(+), 37 deletions(-)
>
> diff --git a/midx.c b/midx.c
> index 8edb75f51d..7a34473010 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -131,6 +131,7 @@ static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir
>  	m->data = midx_map;
>  	m->data_len = midx_size;
>  	m->local = local;
> +	m->r = the_repository;
Same note here about calling this 'r' rather than 'repo'.

I do wonder if it creates any awkwardness to have the_repository assigned here unconditionally when we do specify the object_dir. I think it's OK so long as we don't start replacing 'm->object_dir' with 'm->repo->objects->odb->path'.

Show 28 quoted lines
> @@ -1980,18 +1997,23 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
>  	if (!use_boundary_traversal)
>  		object_array_clear(&revs->pending);
>
> +	repo = bitmap_repo(bitmap_git);
> +
>  	if (haves) {
> -		if (use_boundary_traversal) {
> -			trace2_region_enter("pack-bitmap", "haves/boundary", the_repository);
> +		if (use_boundary_traversal)
> +		{
> +			trace2_region_enter("pack-bitmap", "haves/boundary", repo);
>  			haves_bitmap = find_boundary_objects(bitmap_git, revs, haves);
> -			trace2_region_leave("pack-bitmap", "haves/boundary", the_repository);
> -		} else {
> -			trace2_region_enter("pack-bitmap", "haves/classic", the_repository);
> +			trace2_region_leave("pack-bitmap", "haves/boundary", repo);
> +		}
> +		else
> +		{
> +			trace2_region_enter("pack-bitmap", "haves/classic", repo);
>  			revs->ignore_missing_links = 1;
>  			haves_bitmap = find_objects(bitmap_git, revs, haves, NULL);
>  			reset_revision_walk();
>  			revs->ignore_missing_links = 0;
> -			trace2_region_leave("pack-bitmap", "haves/classic", the_repository);
> +			trace2_region_leave("pack-bitmap", "haves/classic", repo);
>  		}
Odd braces?

Thanks, Taylor

karthik nayak· Oct 31, 2024, 09:34 UTC · re: Taylor Blau · lore

Re: [PATCH v3 9/9] midx: add repository to `multi_pack_index` struct

Taylor Blau <me@ttaylorr.com> writes:
Show 42 quoted lines
> On Wed, Oct 30, 2024 at 03:32:34PM +0100, Karthik Nayak wrote:
>> The `multi_pack_index` struct represents the MIDX for a repository.
>> Here, we add a pointer to the repository in this struct, allowing direct
>> use of the repository variable without relying on the global
>> `the_repository` struct.
>>
>> With this addition, we can determine the repository associated with a
>> `bitmap_index` struct. A `bitmap_index` points to either a `packed_git`
>> or a `multi_pack_index`, both of which have direct repository
>> references. To support this, we introduce a static helper function,
>> `bitmap_repo`, in `pack-bitmap.c`, which retrieves a repository given a
>> `bitmap_index`.
>>
>> With this, we clear up all usages of `the_repository` within
>> `pack-bitmap.c` and also remove the `USE_THE_REPOSITORY_VARIABLE`
>> definition. Bringing us another step closer to remove all global
>> variable usage.
>>
>> Although this change also opens up the potential to clean up `midx.c`,
>> doing so would require additional refactoring to pass the repository
>> struct to functions where the MIDX struct is created: a task better
>> suited for future patches.
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  midx.c        |  1 +
>>  midx.h        |  3 ++
>>  pack-bitmap.c | 97 +++++++++++++++++++++++++++++++--------------------
>>  3 files changed, 64 insertions(+), 37 deletions(-)
>>
>> diff --git a/midx.c b/midx.c
>> index 8edb75f51d..7a34473010 100644
>> --- a/midx.c
>> +++ b/midx.c
>> @@ -131,6 +131,7 @@ static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir
>>  	m->data = midx_map;
>>  	m->data_len = midx_size;
>>  	m->local = local;
>> +	m->r = the_repository;
>
> Same note here about calling this 'r' rather than 'repo'.
>
My bad, I'll fix it in the next version.
Show 36 quoted lines
> I do wonder if it creates any awkwardness to have the_repository
> assigned here unconditionally when we do specify the object_dir. I think
> it's OK so long as we don't start replacing 'm->object_dir' with
> 'm->repo->objects->odb->path'.
>
>> @@ -1980,18 +1997,23 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs,
>>  	if (!use_boundary_traversal)
>>  		object_array_clear(&revs->pending);
>>
>> +	repo = bitmap_repo(bitmap_git);
>> +
>>  	if (haves) {
>> -		if (use_boundary_traversal) {
>> -			trace2_region_enter("pack-bitmap", "haves/boundary", the_repository);
>> +		if (use_boundary_traversal)
>> +		{
>> +			trace2_region_enter("pack-bitmap", "haves/boundary", repo);
>>  			haves_bitmap = find_boundary_objects(bitmap_git, revs, haves);
>> -			trace2_region_leave("pack-bitmap", "haves/boundary", the_repository);
>> -		} else {
>> -			trace2_region_enter("pack-bitmap", "haves/classic", the_repository);
>> +			trace2_region_leave("pack-bitmap", "haves/boundary", repo);
>> +		}
>> +		else
>> +		{
>> +			trace2_region_enter("pack-bitmap", "haves/classic", repo);
>>  			revs->ignore_missing_links = 1;
>>  			haves_bitmap = find_objects(bitmap_git, revs, haves, NULL);
>>  			reset_revision_walk();
>>  			revs->ignore_missing_links = 0;
>> -			trace2_region_leave("pack-bitmap", "haves/classic", the_repository);
>> +			trace2_region_leave("pack-bitmap", "haves/classic", repo);
>>  		}
>
> Odd braces?
>
Indeed. Will fix this too.
> Thanks,
> Taylor
Thanks for the review.

← back to recent threads