git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/8] midx-write: add repository field to `write_midx_context`

From
Karthik Nayak <karthik.188@gmail.com>
Date
Nov 15, 2024, 13:42 UTC
Message-ID
<20241115-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-v1-2-761f8a2c7775@gmail.com>
In-Reply-To
<20241115-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-v1-0-761f8a2c7775@gmail.com>

The struct `write_midx_context` is used to pass context for creating MIDX files. Adding the repository field here ensures that most functions within `midx-write.c` have access to the field and can use that instead of the global `repository` variable. This involves passing the `repository` field to `write_midx_internal`. To do this, we add `the_repository` usage to two non-static functions, which we'll remove in upcoming commits.

With this, modify the static functions in `midx-write.c` to not use global variables. This means, either we use existing alternatives (like `repository` struct), or we pass down required fields from other functions.

Signed-off-by: default avatarKarthik Nayak <karthik.188@gmail.com>
---
 midx-write.c | 97 ++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 52 insertions(+), 45 deletions(-)
diff --git a/midx-write.c b/midx-write.c
index c57726ef9475df693890d61627ce91409c1def7c..a384f7ddc8a396d0cffd528132bb8fcdc6b37e24 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -35,13 +35,13 @@ extern void clear_incremental_midx_files_ext(const char *object_dir,
 extern int cmp_idx_or_pack_name(const char *idx_or_pack_name,
 				const char *idx_name);
 
-static size_t write_midx_header(struct hashfile *f,
-				unsigned char num_chunks,
+static size_t write_midx_header(const struct git_hash_algo *hash_algo,
+				struct hashfile *f, unsigned char num_chunks,
 				uint32_t num_packs)
 {
 	hashwrite_be32(f, MIDX_SIGNATURE);
 	hashwrite_u8(f, MIDX_VERSION);
-	hashwrite_u8(f, oid_version(the_hash_algo));
+	hashwrite_u8(f, oid_version(hash_algo));
 	hashwrite_u8(f, num_chunks);
 	hashwrite_u8(f, 0); /* unused */
 	hashwrite_be32(f, num_packs);
@@ -110,6 +110,8 @@ struct write_midx_context {
 	uint32_t num_multi_pack_indexes_before;
 
 	struct string_list *to_include;
+
+	struct repository *repo;
 };
 
 static int should_include_pack(const struct write_midx_context *ctx,
@@ -154,7 +156,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(the_repository, full_path, full_path_len, 0);
+		p = add_packed_git(ctx->repo, full_path, full_path_len, 0);
 		if (!p) {
 			warning(_("failed to add packfile '%s'"),
 				full_path);
@@ -480,7 +482,7 @@ static int write_midx_oid_lookup(struct hashfile *f,
 				 void *data)
 {
 	struct write_midx_context *ctx = data;
-	unsigned char hash_len = the_hash_algo->rawsz;
+	unsigned char hash_len = ctx->repo->hash_algo->rawsz;
 	struct pack_midx_entry *list = ctx->entries;
 	uint32_t i;
 
@@ -605,7 +607,7 @@ static uint32_t *midx_pack_order(struct write_midx_context *ctx)
 	uint32_t *pack_order, base_objects = 0;
 	uint32_t i;
 
-	trace2_region_enter("midx", "midx_pack_order", the_repository);
+	trace2_region_enter("midx", "midx_pack_order", ctx->repo);
 
 	if (ctx->incremental && ctx->base_midx)
 		base_objects = ctx->base_midx->num_objects +
@@ -640,7 +642,7 @@ static uint32_t *midx_pack_order(struct write_midx_context *ctx)
 	}
 	free(data);
 
-	trace2_region_leave("midx", "midx_pack_order", the_repository);
+	trace2_region_leave("midx", "midx_pack_order", ctx->repo);
 
 	return pack_order;
 }
@@ -651,9 +653,10 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,
 	struct strbuf buf = STRBUF_INIT;
 	char *tmp_file;
 
-	trace2_region_enter("midx", "write_midx_reverse_index", the_repository);
+	trace2_region_enter("midx", "write_midx_reverse_index", ctx->repo);
 
-	strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex(midx_hash));
+	strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex_algop(midx_hash,
+								    ctx->repo->hash_algo));
 
 	tmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr,
 					midx_hash, WRITE_REV);
@@ -664,7 +667,7 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,
 	strbuf_release(&buf);
 	free(tmp_file);
 
-	trace2_region_leave("midx", "write_midx_reverse_index", the_repository);
+	trace2_region_leave("midx", "write_midx_reverse_index", ctx->repo);
 }
 
 static void prepare_midx_packing_data(struct packing_data *pdata,
@@ -672,10 +675,10 @@ static void prepare_midx_packing_data(struct packing_data *pdata,
 {
 	uint32_t i;
 
-	trace2_region_enter("midx", "prepare_midx_packing_data", the_repository);
+	trace2_region_enter("midx", "prepare_midx_packing_data", ctx->repo);
 
 	memset(pdata, 0, sizeof(struct packing_data));
-	prepare_packing_data(the_repository, pdata);
+	prepare_packing_data(ctx->repo, pdata);
 
 	for (i = 0; i < ctx->entries_nr; i++) {
 		uint32_t pos = ctx->pack_order[i];
@@ -686,7 +689,7 @@ static void prepare_midx_packing_data(struct packing_data *pdata,
 			       ctx->info[ctx->pack_perm[from->pack_int_id]].p);
 	}
 
-	trace2_region_leave("midx", "prepare_midx_packing_data", the_repository);
+	trace2_region_leave("midx", "prepare_midx_packing_data", ctx->repo);
 }
 
 static int add_ref_to_pending(const char *refname, const char *referent UNUSED,
@@ -702,7 +705,7 @@ static int add_ref_to_pending(const char *refname, const char *referent UNUSED,
 		return 0;
 	}
 
-	if (!peel_iterated_oid(the_repository, oid, &peeled))
+	if (!peel_iterated_oid(revs->repo, oid, &peeled))
 		oid = &peeled;
 
 	object = parse_object_or_die(oid, refname);
@@ -760,7 +763,7 @@ static int read_refs_snapshot(const char *refs_snapshot,
 			hex = &buf.buf[1];
 		}
 
-		if (parse_oid_hex(hex, &oid, &end) < 0)
+		if (parse_oid_hex_algop(hex, &oid, &end, revs->repo->hash_algo) < 0)
 			die(_("could not parse line: %s"), buf.buf);
 		if (*end)
 			die(_("malformed line: %s"), buf.buf);
@@ -783,17 +786,16 @@ static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr
 	struct rev_info revs;
 	struct bitmap_commit_cb cb = {0};
 
-	trace2_region_enter("midx", "find_commits_for_midx_bitmap",
-			    the_repository);
+	trace2_region_enter("midx", "find_commits_for_midx_bitmap", ctx->repo);
 
 	cb.ctx = ctx;
 
-	repo_init_revisions(the_repository, &revs, NULL);
+	repo_init_revisions(ctx->repo, &revs, NULL);
 	if (refs_snapshot) {
 		read_refs_snapshot(refs_snapshot, &revs);
 	} else {
 		setup_revisions(0, NULL, &revs, NULL);
-		refs_for_each_ref(get_main_ref_store(the_repository),
+		refs_for_each_ref(get_main_ref_store(ctx->repo),
 				  add_ref_to_pending, &revs);
 	}
 
@@ -821,13 +823,12 @@ static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr
 
 	release_revisions(&revs);
 
-	trace2_region_leave("midx", "find_commits_for_midx_bitmap",
-			    the_repository);
+	trace2_region_leave("midx", "find_commits_for_midx_bitmap", ctx->repo);
 
 	return cb.commits;
 }
 
-static int write_midx_bitmap(const char *midx_name,
+static int write_midx_bitmap(struct repository *r, const char *midx_name,
 			     const unsigned char *midx_hash,
 			     struct packing_data *pdata,
 			     struct commit **commits,
@@ -840,9 +841,9 @@ static int write_midx_bitmap(const char *midx_name,
 	struct bitmap_writer writer;
 	struct pack_idx_entry **index;
 	char *bitmap_name = xstrfmt("%s-%s.bitmap", midx_name,
-					hash_to_hex(midx_hash));
+				    hash_to_hex_algop(midx_hash, r->hash_algo));
 
-	trace2_region_enter("midx", "write_midx_bitmap", the_repository);
+	trace2_region_enter("midx", "write_midx_bitmap", r);
 
 	if (flags & MIDX_WRITE_BITMAP_HASH_CACHE)
 		options |= BITMAP_OPT_HASH_CACHE;
@@ -859,7 +860,7 @@ static int write_midx_bitmap(const char *midx_name,
 	for (i = 0; i < pdata->nr_objects; i++)
 		index[i] = &pdata->objects[i].idx;
 
-	bitmap_writer_init(&writer, the_repository, pdata);
+	bitmap_writer_init(&writer, r, pdata);
 	bitmap_writer_show_progress(&writer, flags & MIDX_PROGRESS);
 	bitmap_writer_build_type_index(&writer, index);
 
@@ -892,7 +893,7 @@ static int write_midx_bitmap(const char *midx_name,
 	free(bitmap_name);
 	bitmap_writer_free(&writer);
 
-	trace2_region_leave("midx", "write_midx_bitmap", the_repository);
+	trace2_region_leave("midx", "write_midx_bitmap", r);
 
 	return ret;
 }
@@ -944,7 +945,7 @@ static int fill_packs_from_midx(struct write_midx_context *ctx,
 			 */
 			if (flags & MIDX_WRITE_REV_INDEX ||
 			    preferred_pack_name) {
-				if (prepare_midx_pack(the_repository, m,
+				if (prepare_midx_pack(ctx->repo, m,
 						      m->num_packs_in_base + i)) {
 					error(_("could not load pack"));
 					return 1;
@@ -1049,7 +1050,7 @@ static void clear_midx_files(const char *object_dir,
 	strbuf_release(&buf);
 }
 
-static int write_midx_internal(const char *object_dir,
+static int write_midx_internal(struct repository *r, const char *object_dir,
 			       struct string_list *packs_to_include,
 			       struct string_list *packs_to_drop,
 			       const char *preferred_pack_name,
@@ -1070,7 +1071,9 @@ static int write_midx_internal(const char *object_dir,
 	const char **keep_hashes = NULL;
 	struct chunkfile *cf;
 
-	trace2_region_enter("midx", "write_midx_internal", the_repository);
+	trace2_region_enter("midx", "write_midx_internal", r);
+
+	ctx.repo = r;
 
 	ctx.incremental = !!(flags & MIDX_WRITE_INCREMENTAL);
 	if (ctx.incremental && (flags & MIDX_WRITE_BITMAP))
@@ -1087,8 +1090,7 @@ static int write_midx_internal(const char *object_dir,
 			  midx_name.buf);
 
 	if (!packs_to_include || ctx.incremental) {
-		struct multi_pack_index *m = lookup_multi_pack_index(the_repository,
-								     object_dir);
+		struct multi_pack_index *m = lookup_multi_pack_index(r, object_dir);
 		if (m && !midx_checksum_valid(m)) {
 			warning(_("ignoring existing multi-pack-index; checksum mismatch"));
 			m = NULL;
@@ -1351,7 +1353,7 @@ static int write_midx_internal(const char *object_dir,
 	add_chunk(cf, MIDX_CHUNKID_OIDFANOUT, MIDX_CHUNK_FANOUT_SIZE,
 		  write_midx_oid_fanout);
 	add_chunk(cf, MIDX_CHUNKID_OIDLOOKUP,
-		  st_mult(ctx.entries_nr, the_hash_algo->rawsz),
+		  st_mult(ctx.entries_nr, r->hash_algo->rawsz),
 		  write_midx_oid_lookup);
 	add_chunk(cf, MIDX_CHUNKID_OBJECTOFFSETS,
 		  st_mult(ctx.entries_nr, MIDX_CHUNK_OFFSET_WIDTH),
@@ -1373,7 +1375,8 @@ static int write_midx_internal(const char *object_dir,
 			  write_midx_bitmapped_packs);
 	}
 
-	write_midx_header(f, get_num_chunks(cf), ctx.nr - dropped_packs);
+	write_midx_header(r->hash_algo, f, get_num_chunks(cf),
+			  ctx.nr - dropped_packs);
 	write_chunkfile(cf, &ctx);
 
 	finalize_hashfile(f, midx_hash, FSYNC_COMPONENT_PACK_METADATA,
@@ -1405,7 +1408,7 @@ static int write_midx_internal(const char *object_dir,
 		FREE_AND_NULL(ctx.entries);
 		ctx.entries_nr = 0;
 
-		if (write_midx_bitmap(midx_name.buf, midx_hash, &pdata,
+		if (write_midx_bitmap(r, midx_name.buf, midx_hash, &pdata,
 				      commits, commits_nr, ctx.pack_order,
 				      flags) < 0) {
 			error(_("could not write multi-pack bitmap"));
@@ -1449,12 +1452,13 @@ static int write_midx_internal(const char *object_dir,
 		strbuf_release(&final_midx_name);
 
 		keep_hashes[ctx.num_multi_pack_indexes_before] =
-			xstrdup(hash_to_hex(midx_hash));
+			xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo));
 
 		for (i = 0; i < ctx.num_multi_pack_indexes_before; i++) {
 			uint32_t j = ctx.num_multi_pack_indexes_before - i - 1;
 
-			keep_hashes[j] = xstrdup(hash_to_hex(get_midx_checksum(m)));
+			keep_hashes[j] = xstrdup(hash_to_hex_algop(get_midx_checksum(m),
+								   r->hash_algo));
 			m = m->base_midx;
 		}
 
@@ -1462,11 +1466,11 @@ static int write_midx_internal(const char *object_dir,
 			fprintf(get_lock_file_fp(&lk), "%s\n", keep_hashes[i]);
 	} else {
 		keep_hashes[ctx.num_multi_pack_indexes_before] =
-			xstrdup(hash_to_hex(midx_hash));
+			xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo));
 	}
 
 	if (ctx.m || ctx.base_midx)
-		close_object_store(the_repository->objects);
+		close_object_store(ctx.repo->objects);
 
 	if (commit_lock_file(&lk) < 0)
 		die_errno(_("could not write multi-pack-index"));
@@ -1495,7 +1499,7 @@ static int write_midx_internal(const char *object_dir,
 	}
 	strbuf_release(&midx_name);
 
-	trace2_region_leave("midx", "write_midx_internal", the_repository);
+	trace2_region_leave("midx", "write_midx_internal", r);
 
 	return result;
 }
@@ -1505,8 +1509,8 @@ int write_midx_file(const char *object_dir,
 		    const char *refs_snapshot,
 		    unsigned flags)
 {
-	return write_midx_internal(object_dir, NULL, NULL, preferred_pack_name,
-				   refs_snapshot, flags);
+	return write_midx_internal(the_repository, object_dir, NULL, NULL,
+				   preferred_pack_name, refs_snapshot, flags);
 }
 
 int write_midx_file_only(const char *object_dir,
@@ -1515,8 +1519,9 @@ int write_midx_file_only(const char *object_dir,
 			 const char *refs_snapshot,
 			 unsigned flags)
 {
-	return write_midx_internal(object_dir, packs_to_include, NULL,
-				   preferred_pack_name, refs_snapshot, flags);
+	return write_midx_internal(the_repository, object_dir, packs_to_include,
+				   NULL, preferred_pack_name, refs_snapshot,
+				   flags);
 }
 
 int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags)
@@ -1572,7 +1577,8 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
 	free(count);
 
 	if (packs_to_drop.nr)
-		result = write_midx_internal(object_dir, NULL, &packs_to_drop, NULL, NULL, flags);
+		result = write_midx_internal(r, object_dir, NULL,
+					     &packs_to_drop, NULL, NULL, flags);
 
 	string_list_clear(&packs_to_drop, 0);
 
@@ -1769,7 +1775,8 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
 		goto cleanup;
 	}
 
-	result = write_midx_internal(object_dir, NULL, NULL, NULL, NULL, flags);
+	result = write_midx_internal(r, object_dir, NULL, NULL, NULL, NULL,
+				     flags);
 
 cleanup:
 	free(include_pack);
-- 
2.47.0
Previous: karthik nayakNext: brian m. carlson
Message 6 of 70 in “Change midx.c and midx-write.c to not use global variables”
  1. 0/8 Change midx.c and midx-write.c to not use global variablesKarthik Nayak, Nov 15, 2024
  2. 1/8 builtin: pass repository to sub commandsKarthik Nayak, Nov 15, 2024
  3. shejialuoNov 16, 2024
  4. Patrick SteinhardtNov 18, 2024
  5. karthik nayakNov 18, 2024
  6. 2/8 midx-write: add repository field to `write_midx_context`Karthik Nayak, Nov 15, 2024
  7. brian m. carlsonNov 15, 2024
  8. karthik nayakNov 18, 2024
  9. Patrick SteinhardtNov 18, 2024
  10. karthik nayakNov 18, 2024
  11. 3/8 midx-write: pass down repository to `write_midx_file[_only]`Karthik Nayak, Nov 15, 2024
  12. Patrick SteinhardtNov 18, 2024
  13. 4/8 midx: cleanup internal usage of `the_repository` and `the_hash_algo`Karthik Nayak, Nov 15, 2024
  14. Patrick SteinhardtNov 18, 2024
  15. karthik nayakNov 18, 2024
  16. 5/8 midx: pass `repository` to `load_multi_pack_index`Karthik Nayak, Nov 15, 2024
  17. 6/8 midx: pass down `hash_algo` to `get_midx_filename[_ext]`Karthik Nayak, Nov 15, 2024
  18. shejialuoNov 16, 2024
  19. karthik nayakNov 18, 2024
  20. shejialuoNov 19, 2024
  21. 7/8 midx: pass down `hash_algo` to `get_split_midx_filename_ext`Karthik Nayak, Nov 15, 2024
  22. shejialuoNov 16, 2024
  23. 8/8 midx: inline the `MIDX_MIN_SIZE` definitionKarthik Nayak, Nov 15, 2024
  24. karthik nayakNov 15, 2024
  25. 00/10 Change midx.c and midx-write.c to not use global variablesKarthik Nayak, Nov 19, 2024
  26. 01/10 builtin: pass repository to sub commandsKarthik Nayak, Nov 19, 2024
  27. 03/10 midx-write: use `revs->repo` inside `read_refs_snapshot`Karthik Nayak, Nov 19, 2024
  28. shejialuoNov 20, 2024
  29. Richard KerryNov 20, 2024
  30. Taylor BlauNov 20, 2024
  31. karthik nayakNov 21, 2024
  32. Taylor BlauNov 20, 2024
  33. 05/10 midx-write: pass down repository to `write_midx_file[_only]`Karthik Nayak, Nov 19, 2024
  34. Taylor BlauNov 20, 2024
  35. 06/10 midx: cleanup internal usage of `the_repository` and `the_hash_algo`Karthik Nayak, Nov 19, 2024
  36. 07/10 midx: pass `repository` to `load_multi_pack_index`Karthik Nayak, Nov 19, 2024
  37. 08/10 midx: pass down `hash_algo` to `get_midx_filename[_ext]`Karthik Nayak, Nov 19, 2024
  38. Christian CouderNov 20, 2024
  39. karthik nayakNov 21, 2024
  40. 10/10 midx: inline the `MIDX_MIN_SIZE` definitionKarthik Nayak, Nov 19, 2024
  41. shejialuoNov 20, 2024
  42. karthik nayakNov 21, 2024
  43. 09/10 midx: pass down `hash_algo` to `get_split_midx_filename_ext`Karthik Nayak, Nov 19, 2024
  44. Christian CouderNov 20, 2024
  45. Taylor BlauNov 20, 2024
  46. karthik nayakNov 21, 2024
  47. 02/10 midx-write: pass down repository to static functionsKarthik Nayak, Nov 19, 2024
  48. Christian CouderNov 20, 2024
  49. Taylor BlauNov 20, 2024
  50. karthik nayakNov 21, 2024
  51. Taylor BlauNov 20, 2024
  52. karthik nayakNov 21, 2024
  53. 04/10 write-midx: add repository field to `write_midx_context`Karthik Nayak, Nov 19, 2024
  54. Christian CouderNov 20, 2024
  55. Taylor BlauNov 20, 2024
  56. Junio C HamanoNov 21, 2024
  57. Junio C HamanoNov 21, 2024
  58. karthik nayakNov 22, 2024
  59. 0/8 Change midx.c and midx-write.c to not use global variablesKarthik Nayak, Nov 27, 2024
  60. 1/8 midx-write: pass down repository to static functionsKarthik Nayak, Nov 27, 2024
  61. 2/8 midx-write: use `revs->repo` inside `read_refs_snapshot`Karthik Nayak, Nov 27, 2024
  62. 3/8 write-midx: add repository field to `write_midx_context`Karthik Nayak, Nov 27, 2024
  63. 4/8 midx-write: pass down repository to `write_midx_file[_only]`Karthik Nayak, Nov 27, 2024
  64. 5/8 midx: cleanup internal usage of `the_repository` and `the_hash_algo`Karthik Nayak, Nov 27, 2024
  65. 6/8 midx: pass `repository` to `load_multi_pack_index`Karthik Nayak, Nov 27, 2024
  66. 7/8 midx: pass down `hash_algo` to functions using global variablesKarthik Nayak, Nov 27, 2024
  67. 8/8 midx: inline the `MIDX_MIN_SIZE` definitionKarthik Nayak, Nov 27, 2024
  68. Junio C HamanoNov 28, 2024
  69. Patrick SteinhardtDec 3, 2024
  70. Junio C HamanoDec 3, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.