{"thread":{"id":"62817","subject":"[PATCH 0/5] pack-write: cleanup usage of global variables","startedAt":"2025-01-16T11:35:19Z","lastAt":"2025-01-24T15:47:17Z","messageCount":36,"participants":["Karthik Nayak via B4 Relay","Patrick Steinhardt","Junio C Hamano","Karthik Nayak","Toon Claes"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"510685","messageId":"20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com","threadId":"62817","inReplyTo":null,"subject":"[PATCH 0/5] pack-write: cleanup usage of global variables","fromName":"Karthik Nayak via B4 Relay","fromEmail":"devnull+karthik.188.gmail.com@kernel.org","sentAt":"2025-01-16T11:35:12Z","receivedAt":"2025-01-16T11:35:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This is a small series to remove global variable usage from\n`pack-write.c`. Mostly it bubble's up the usage of global variables to\nupper layers. The only exception is in `write-midx.c`, which was cleaned\nof global variable usage, so there, we use the repo that is in available\nin the context.\n\nThis series is based on fbe8d3079d (Git 2.48, 2025-01-10) with\n'ps/more-sign-compare' and 'ps/the-repository' merged in.\n\nThere are no conflicts with topics in 'next', however there is a\nconflict with 'tb/incremental-midx-part-2' in 'seen', the fix is simple\nbut happy to merge that in too if necessary.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\nKarthik Nayak (5):\n      pack-write: pass hash_algo to `fixup_pack_header_footer()`\n      pack-write: pass repository to `index_pack_lockfile()`\n      pack-write: pass hash_algo to `write_idx_file()`\n      pack-write: pass hash_algo to `write_rev_file()`\n      pack-write: pass hash_algo to `write_rev_*()`\n\n builtin/fast-import.c  | 11 +++---\n builtin/index-pack.c   | 11 +++---\n builtin/pack-objects.c | 12 +++---\n builtin/receive-pack.c |  2 +-\n bulk-checkin.c         |  7 ++--\n fetch-pack.c           |  4 +-\n midx-write.c           |  4 +-\n pack-write.c           | 99 +++++++++++++++++++++++++++-----------------------\n pack.h                 | 30 ++++++++++++---\n 9 files changed, 106 insertions(+), 74 deletions(-)\n---\n\n\n\n---\n\nbase-commit: 8b2efc058aaa3d1437678616bccf7c5f7ce1f92b\nchange-id: 20250110-kn-the-repo-cleanup-44144fa42dc3\n\nThanks\n- Karthik\n\n\n"},{"id":"510686","messageId":"20250116-kn-the-repo-cleanup-v1-1-a2f4c8e1c4c3@gmail.com","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com","subject":"[PATCH 1/5] pack-write: pass hash_algo to `fixup_pack_header_footer()`","fromName":"Karthik Nayak via B4 Relay","fromEmail":"devnull+karthik.188.gmail.com@kernel.org","sentAt":"2025-01-16T11:35:13Z","receivedAt":"2025-01-16T11:35:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nThe `fixup_pack_header_footer()` function uses the global\n`the_hash_algo` variable to access the repository's hash function. To\navoid global variable usage, pass the hash function from the layers\nabove.\n\nAltough the layers above could have access to the hash function\ninternally, simply pass in `the_hash_algo`. This avoids any\ncompatibility issues and bubbles up global variable usage to upper\nlayers which can be eventually resolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/fast-import.c  |  7 ++++---\n builtin/index-pack.c   |  2 +-\n builtin/pack-objects.c |  5 +++--\n bulk-checkin.c         |  2 +-\n pack-write.c           | 28 ++++++++++++++--------------\n pack.h                 |  4 +++-\n 6 files changed, 26 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa..6baf2b1b71e2443a7987a41e0cd246076682bf58 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -878,9 +878,10 @@ static void end_packfile(void)\n \n \t\tclose_pack_windows(pack_data);\n \t\tfinalize_hashfile(pack_file, cur_pack_oid.hash, FSYNC_COMPONENT_PACK, 0);\n-\t\tfixup_pack_header_footer(pack_data->pack_fd, pack_data->hash,\n-\t\t\t\t\t pack_data->pack_name, object_count,\n-\t\t\t\t\t cur_pack_oid.hash, pack_size);\n+\t\tfixup_pack_header_footer(the_hash_algo, pack_data->pack_fd,\n+\t\t\t\t\t pack_data->hash, pack_data->pack_name,\n+\t\t\t\t\t object_count, cur_pack_oid.hash,\n+\t\t\t\t\t pack_size);\n \n \t\tif (object_count <= unpack_limit) {\n \t\t\tif (!loosen_small_pack(pack_data)) {\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 0bef61c57232e198ba539cd44ec301d26dcb0eb8..6c5e3483f4fe67fb2e26132c55b1f8395d60c11f 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1390,7 +1390,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha\n \t\tstrbuf_release(&msg);\n \t\tfinalize_hashfile(f, tail_hash, FSYNC_COMPONENT_PACK, 0);\n \t\thashcpy(read_hash, pack_hash, the_repository->hash_algo);\n-\t\tfixup_pack_header_footer(output_fd, pack_hash,\n+\t\tfixup_pack_header_footer(the_hash_algo, output_fd, pack_hash,\n \t\t\t\t\t curr_pack, nr_objects,\n \t\t\t\t\t read_hash, consumed_bytes-the_hash_algo->rawsz);\n \t\tif (!hasheq(read_hash, tail_hash, the_repository->hash_algo))\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex d51c021d99d9f470c04b7ec52565ab2f4c1c19ae..ffc62930b68c9b4152057572ede216381a4b0991 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1319,8 +1319,9 @@ static void write_pack_file(void)\n \t\t\t */\n \n \t\t\tint fd = finalize_hashfile(f, hash, FSYNC_COMPONENT_PACK, 0);\n-\t\t\tfixup_pack_header_footer(fd, hash, pack_tmp_name,\n-\t\t\t\t\t\t nr_written, hash, offset);\n+\t\t\tfixup_pack_header_footer(the_hash_algo, fd, hash,\n+\t\t\t\t\t\t pack_tmp_name, nr_written,\n+\t\t\t\t\t\t hash, offset);\n \t\t\tclose(fd);\n \t\t\tif (write_bitmap_index) {\n \t\t\t\tif (write_bitmap_index != WRITE_BITMAP_QUIET)\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex 5044cb7fa083d692a3797e2491c27b61ec44c69c..c4b085f57f74fb8b998576ac9d84fed9e01ed0ed 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -70,7 +70,7 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state)\n \t\t\t\t  CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);\n \t} else {\n \t\tint fd = finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK, 0);\n-\t\tfixup_pack_header_footer(fd, hash, state->pack_tmp_name,\n+\t\tfixup_pack_header_footer(the_hash_algo, fd, hash, state->pack_tmp_name,\n \t\t\t\t\t state->nr_written, hash,\n \t\t\t\t\t state->offset);\n \t\tclose(fd);\ndiff --git a/pack-write.c b/pack-write.c\nindex 98a8c0e7853d7b46b5ce9a9672e0249ff051b5f9..fc887850dfb9789132b8642733c6472944dbe32d 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -380,7 +380,8 @@ off_t write_pack_header(struct hashfile *f, uint32_t nr_entries)\n  * partial_pack_sha1 can refer to the same buffer if the caller is not\n  * interested in the resulting SHA1 of pack data above partial_pack_offset.\n  */\n-void fixup_pack_header_footer(int pack_fd,\n+void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n+\t\t\t int pack_fd,\n \t\t\t unsigned char *new_pack_hash,\n \t\t\t const char *pack_name,\n \t\t\t uint32_t object_count,\n@@ -393,8 +394,8 @@ void fixup_pack_header_footer(int pack_fd,\n \tchar *buf;\n \tssize_t read_result;\n \n-\tthe_hash_algo->init_fn(&old_hash_ctx);\n-\tthe_hash_algo->init_fn(&new_hash_ctx);\n+\thash_algo->init_fn(&old_hash_ctx);\n+\thash_algo->init_fn(&new_hash_ctx);\n \n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n@@ -406,9 +407,9 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\t  pack_name);\n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n-\tthe_hash_algo->update_fn(&old_hash_ctx, &hdr, sizeof(hdr));\n+\thash_algo->update_fn(&old_hash_ctx, &hdr, sizeof(hdr));\n \thdr.hdr_entries = htonl(object_count);\n-\tthe_hash_algo->update_fn(&new_hash_ctx, &hdr, sizeof(hdr));\n+\thash_algo->update_fn(&new_hash_ctx, &hdr, sizeof(hdr));\n \twrite_or_die(pack_fd, &hdr, sizeof(hdr));\n \tpartial_pack_offset -= sizeof(hdr);\n \n@@ -423,7 +424,7 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\tbreak;\n \t\tif (n < 0)\n \t\t\tdie_errno(\"Failed to checksum '%s'\", pack_name);\n-\t\tthe_hash_algo->update_fn(&new_hash_ctx, buf, n);\n+\t\thash_algo->update_fn(&new_hash_ctx, buf, n);\n \n \t\taligned_sz -= n;\n \t\tif (!aligned_sz)\n@@ -432,13 +433,12 @@ void fixup_pack_header_footer(int pack_fd,\n \t\tif (!partial_pack_hash)\n \t\t\tcontinue;\n \n-\t\tthe_hash_algo->update_fn(&old_hash_ctx, buf, n);\n+\t\thash_algo->update_fn(&old_hash_ctx, buf, n);\n \t\tpartial_pack_offset -= n;\n \t\tif (partial_pack_offset == 0) {\n \t\t\tunsigned char hash[GIT_MAX_RAWSZ];\n-\t\t\tthe_hash_algo->final_fn(hash, &old_hash_ctx);\n-\t\t\tif (!hasheq(hash, partial_pack_hash,\n-\t\t\t\t    the_repository->hash_algo))\n+\t\t\thash_algo->final_fn(hash, &old_hash_ctx);\n+\t\t\tif (!hasheq(hash, partial_pack_hash, hash_algo))\n \t\t\t\tdie(\"Unexpected checksum for %s \"\n \t\t\t\t    \"(disk corruption?)\", pack_name);\n \t\t\t/*\n@@ -446,7 +446,7 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\t * pack, which also means making partial_pack_offset\n \t\t\t * big enough not to matter anymore.\n \t\t\t */\n-\t\t\tthe_hash_algo->init_fn(&old_hash_ctx);\n+\t\t\thash_algo->init_fn(&old_hash_ctx);\n \t\t\tpartial_pack_offset = ~partial_pack_offset;\n \t\t\tpartial_pack_offset -= MSB(partial_pack_offset, 1);\n \t\t}\n@@ -454,9 +454,9 @@ void fixup_pack_header_footer(int pack_fd,\n \tfree(buf);\n \n \tif (partial_pack_hash)\n-\t\tthe_hash_algo->final_fn(partial_pack_hash, &old_hash_ctx);\n-\tthe_hash_algo->final_fn(new_pack_hash, &new_hash_ctx);\n-\twrite_or_die(pack_fd, new_pack_hash, the_hash_algo->rawsz);\n+\t\thash_algo->final_fn(partial_pack_hash, &old_hash_ctx);\n+\thash_algo->final_fn(new_pack_hash, &new_hash_ctx);\n+\twrite_or_die(pack_fd, new_pack_hash, hash_algo->rawsz);\n \tfsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);\n }\n \ndiff --git a/pack.h b/pack.h\nindex a8da0406299bf267205256aaa8efb215f2ff73be..6d9d477adc83e83d9e9175ccf699c100b4c147c6 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -91,7 +91,9 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offs\n int verify_pack_index(struct packed_git *);\n int verify_pack(struct repository *, struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n off_t write_pack_header(struct hashfile *f, uint32_t);\n-void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);\n+void fixup_pack_header_footer(const struct git_hash_algo *, int,\n+\t\t\t      unsigned char *, const char *, uint32_t,\n+\t\t\t      unsigned char *, off_t);\n char *index_pack_lockfile(int fd, int *is_well_formed);\n \n struct ref;\n\n-- \n2.47.0\n\n\n"},{"id":"510687","messageId":"20250116-kn-the-repo-cleanup-v1-2-a2f4c8e1c4c3@gmail.com","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com","subject":"[PATCH 2/5] pack-write: pass repository to `index_pack_lockfile()`","fromName":"Karthik Nayak via B4 Relay","fromEmail":"devnull+karthik.188.gmail.com@kernel.org","sentAt":"2025-01-16T11:35:14Z","receivedAt":"2025-01-16T11:35:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nThe `index_pack_lockfile()` function uses the global `the_repository`\nvariable to access the repository. To avoid global variable usage, pass\nthe repository from the layers above.\n\nAltough the layers above could have access to the hash function\ninternally, simply pass in `the_hash_algo`. This avoids any\ncompatibility issues and bubbles up global variable usage to upper\nlayers which can be eventually resolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/receive-pack.c | 2 +-\n fetch-pack.c           | 4 +++-\n pack-write.c           | 6 +++---\n pack.h                 | 2 +-\n 4 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 56347a79633505efe8dc05acf1583b4c9995eefe..b83abe5d220cefd3707b701409dc5e6b67566599 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2304,7 +2304,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n \t\tif (status)\n \t\t\treturn \"index-pack fork failed\";\n \n-\t\tlockfile = index_pack_lockfile(child.out, NULL);\n+\t\tlockfile = index_pack_lockfile(the_repository, child.out, NULL);\n \t\tif (lockfile) {\n \t\t\tpack_lockfile = register_tempfile(lockfile);\n \t\t\tfree(lockfile);\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 3a227721ed0935d1f9c40584c57f54043354c032..824f56ecbca11cd9e4da6a3e4c450c6b2e7078ab 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1036,7 +1036,9 @@ static int get_pack(struct fetch_pack_args *args,\n \t\tdie(_(\"fetch-pack: unable to fork off %s\"), cmd_name);\n \tif (do_keep && (pack_lockfiles || fsck_objects)) {\n \t\tint is_well_formed;\n-\t\tchar *pack_lockfile = index_pack_lockfile(cmd.out, &is_well_formed);\n+\t\tchar *pack_lockfile = index_pack_lockfile(the_repository,\n+\t\t\t\t\t\t\t  cmd.out,\n+\t\t\t\t\t\t\t  &is_well_formed);\n \n \t\tif (!is_well_formed)\n \t\t\tdie(_(\"fetch-pack: invalid index-pack output\"));\ndiff --git a/pack-write.c b/pack-write.c\nindex fc887850dfb9789132b8642733c6472944dbe32d..0cd75d2e55419362a61cf981fc11117ea7a1d88a 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -460,10 +460,10 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n \tfsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);\n }\n \n-char *index_pack_lockfile(int ip_out, int *is_well_formed)\n+char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)\n {\n \tchar packname[GIT_MAX_HEXSZ + 6];\n-\tconst int len = the_hash_algo->hexsz + 6;\n+\tconst int len = r->hash_algo->hexsz + 6;\n \n \t/*\n \t * The first thing we expect from index-pack's output\n@@ -480,7 +480,7 @@ char *index_pack_lockfile(int ip_out, int *is_well_formed)\n \t\tpackname[len-1] = 0;\n \t\tif (skip_prefix(packname, \"keep\\t\", &name))\n \t\t\treturn xstrfmt(\"%s/pack/pack-%s.keep\",\n-\t\t\t\t       repo_get_object_directory(the_repository), name);\n+\t\t\t\t       repo_get_object_directory(r), name);\n \t\treturn NULL;\n \t}\n \tif (is_well_formed)\ndiff --git a/pack.h b/pack.h\nindex 6d9d477adc83e83d9e9175ccf699c100b4c147c6..46d85e5bec787c90af69700fd4b328b1ebf1d606 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -94,7 +94,7 @@ off_t write_pack_header(struct hashfile *f, uint32_t);\n void fixup_pack_header_footer(const struct git_hash_algo *, int,\n \t\t\t      unsigned char *, const char *, uint32_t,\n \t\t\t      unsigned char *, off_t);\n-char *index_pack_lockfile(int fd, int *is_well_formed);\n+char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed);\n \n struct ref;\n \n\n-- \n2.47.0\n\n\n"},{"id":"510688","messageId":"20250116-kn-the-repo-cleanup-v1-4-a2f4c8e1c4c3@gmail.com","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com","subject":"[PATCH 4/5] pack-write: pass hash_algo to `write_rev_file()`","fromName":"Karthik Nayak via B4 Relay","fromEmail":"devnull+karthik.188.gmail.com@kernel.org","sentAt":"2025-01-16T11:35:16Z","receivedAt":"2025-01-16T11:35:20Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nThe `write_rev_file()` function uses the global `the_hash_algo` variable\nto access the repository's hash function. To avoid global variable\nusage, let's pass the hash function from the layers above.\n\nAltough the layers above could have access to the hash function\ninternally, simply pass in `the_hash_algo`. This avoids any\ncompatibility issues and bubbles up global variable usage to upper\nlayers which can be eventually resolved.\n\nHowever, in `midx-write.c`, since all usage of global variables is\nremoved, don't reintroduce them and instead use the `repo` available in\nthe context.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/index-pack.c |  6 +++---\n midx-write.c         |  4 ++--\n pack-write.c         | 21 ++++++++++++---------\n pack.h               | 14 ++++++++++++--\n 4 files changed, 29 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex d73699653a2227f8ecfee2c0f51cd680093ac764..e803cb2444633f937fafc841848dae85341475f1 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -2099,9 +2099,9 @@ int cmd_index_pack(int argc,\n \tcurr_index = write_idx_file(the_hash_algo, index_name, idx_objects,\n \t\t\t\t    nr_objects, &opts, pack_hash);\n \tif (rev_index)\n-\t\tcurr_rev_index = write_rev_file(rev_index_name, idx_objects,\n-\t\t\t\t\t\tnr_objects, pack_hash,\n-\t\t\t\t\t\topts.flags);\n+\t\tcurr_rev_index = write_rev_file(the_hash_algo, rev_index_name,\n+\t\t\t\t\t\tidx_objects, nr_objects,\n+\t\t\t\t\t\tpack_hash, opts.flags);\n \tfree(idx_objects);\n \n \tif (!verify)\ndiff --git a/midx-write.c b/midx-write.c\nindex b3827b936bdb1df12c73fb7d9b98ff65fc875cc3..61b59d557d3ba46a39db74c62393545cabf50f2c 100644\n--- a/midx-write.c\n+++ b/midx-write.c\n@@ -658,8 +658,8 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,\n \tstrbuf_addf(&buf, \"%s-%s.rev\", midx_name, hash_to_hex_algop(midx_hash,\n \t\t\t\t\t\t\t\t    ctx->repo->hash_algo));\n \n-\ttmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr,\n-\t\t\t\t\tmidx_hash, WRITE_REV);\n+\ttmp_file = write_rev_file_order(ctx->repo->hash_algo, NULL, ctx->pack_order,\n+\t\t\t\t\tctx->entries_nr, midx_hash, WRITE_REV);\n \n \tif (finalize_object_file(tmp_file, buf.buf))\n \t\tdie(_(\"cannot store reverse index file\"));\ndiff --git a/pack-write.c b/pack-write.c\nindex f344e78a9ec20cea9812a5eaffc72ae0b7e7424d..09ecbcdb069cc9b0383295798ceb49cbdc632b64 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -194,11 +194,12 @@ static int pack_order_cmp(const void *va, const void *vb, void *ctx)\n \treturn 0;\n }\n \n-static void write_rev_header(struct hashfile *f)\n+static void write_rev_header(const struct git_hash_algo *hash_algo,\n+\t\t\t     struct hashfile *f)\n {\n \thashwrite_be32(f, RIDX_SIGNATURE);\n \thashwrite_be32(f, RIDX_VERSION);\n-\thashwrite_be32(f, oid_version(the_hash_algo));\n+\thashwrite_be32(f, oid_version(hash_algo));\n }\n \n static void write_rev_index_positions(struct hashfile *f,\n@@ -215,7 +216,8 @@ static void write_rev_trailer(struct hashfile *f, const unsigned char *hash)\n \thashwrite(f, hash, the_hash_algo->rawsz);\n }\n \n-char *write_rev_file(const char *rev_name,\n+char *write_rev_file(const struct git_hash_algo *hash_algo,\n+\t\t     const char *rev_name,\n \t\t     struct pack_idx_entry **objects,\n \t\t     uint32_t nr_objects,\n \t\t     const unsigned char *hash,\n@@ -233,15 +235,16 @@ char *write_rev_file(const char *rev_name,\n \t\tpack_order[i] = i;\n \tQSORT_S(pack_order, nr_objects, pack_order_cmp, objects);\n \n-\tret = write_rev_file_order(rev_name, pack_order, nr_objects, hash,\n-\t\t\t\t   flags);\n+\tret = write_rev_file_order(hash_algo, rev_name, pack_order, nr_objects,\n+\t\t\t\t   hash, flags);\n \n \tfree(pack_order);\n \n \treturn ret;\n }\n \n-char *write_rev_file_order(const char *rev_name,\n+char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *rev_name,\n \t\t\t   uint32_t *pack_order,\n \t\t\t   uint32_t nr_objects,\n \t\t\t   const unsigned char *hash,\n@@ -280,7 +283,7 @@ char *write_rev_file_order(const char *rev_name,\n \t\treturn NULL;\n \t}\n \n-\twrite_rev_header(f);\n+\twrite_rev_header(hash_algo, f);\n \n \twrite_rev_index_positions(f, pack_order, nr_objects);\n \twrite_rev_trailer(f, hash);\n@@ -568,8 +571,8 @@ void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n \tif (adjust_shared_perm(*idx_tmp_name))\n \t\tdie_errno(\"unable to make temporary index file readable\");\n \n-\trev_tmp_name = write_rev_file(NULL, written_list, nr_written, hash,\n-\t\t\t\t      pack_idx_opts->flags);\n+\trev_tmp_name = write_rev_file(hash_algo, NULL, written_list, nr_written,\n+\t\t\t\t      hash, pack_idx_opts->flags);\n \n \tif (pack_idx_opts->flags & WRITE_MTIMES) {\n \t\tmtimes_tmp_name = write_mtimes_file(to_pack, written_list,\ndiff --git a/pack.h b/pack.h\nindex c650fdbe2dcde8055ad0efe55646338cd0f81df5..8a84ea475cda902936ee0b6091c5b4d462606be8 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -105,8 +105,18 @@ struct ref;\n \n void write_promisor_file(const char *promisor_name, struct ref **sought, int nr_sought);\n \n-char *write_rev_file(const char *rev_name, struct pack_idx_entry **objects, uint32_t nr_objects, const unsigned char *hash, unsigned flags);\n-char *write_rev_file_order(const char *rev_name, uint32_t *pack_order, uint32_t nr_objects, const unsigned char *hash, unsigned flags);\n+char *write_rev_file(const struct git_hash_algo *hash_algo,\n+\t\t     const char *rev_name,\n+\t\t     struct pack_idx_entry **objects,\n+\t\t     uint32_t nr_objects,\n+\t\t     const unsigned char *hash,\n+\t\t     unsigned flags);\n+char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *rev_name,\n+\t\t\t   uint32_t *pack_order,\n+\t\t\t   uint32_t nr_objects,\n+\t\t\t   const unsigned char *hash,\n+\t\t\t   unsigned flags);\n \n /*\n  * The \"hdr\" output buffer should be at least this big, which will handle sizes\n\n-- \n2.47.0\n\n\n"},{"id":"510689","messageId":"20250116-kn-the-repo-cleanup-v1-3-a2f4c8e1c4c3@gmail.com","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com","subject":"[PATCH 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Karthik Nayak via B4 Relay","fromEmail":"devnull+karthik.188.gmail.com@kernel.org","sentAt":"2025-01-16T11:35:15Z","receivedAt":"2025-01-16T11:35:20Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nThe `write_idx_file()` function uses the global `the_hash_algo` variable\nto access the repository's hash function. To avoid global variable\nusage, pass the hash function from the layers above.\n\nAltough the layers above could have access to the hash function\ninternally, simply pass in `the_hash_algo`. This avoids any\ncompatibility issues and bubbles up global variable usage to upper\nlayers which can be eventually resolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/fast-import.c  |  4 ++--\n builtin/index-pack.c   |  3 ++-\n builtin/pack-objects.c |  7 ++++---\n bulk-checkin.c         |  5 +++--\n pack-write.c           | 14 ++++++++------\n pack.h                 | 10 ++++++++--\n 6 files changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 6baf2b1b71e2443a7987a41e0cd246076682bf58..c4bc52f93c011f34bd7f98c8e9d74a33cf9783bd 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -798,8 +798,8 @@ static const char *create_index(void)\n \tif (c != last)\n \t\tdie(\"internal consistency error creating the index\");\n \n-\ttmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts,\n-\t\t\t\t pack_data->hash);\n+\ttmpfile = write_idx_file(the_hash_algo, NULL, idx, object_count,\n+\t\t\t\t &pack_idx_opts, pack_data->hash);\n \tfree(idx);\n \treturn tmpfile;\n }\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 6c5e3483f4fe67fb2e26132c55b1f8395d60c11f..d73699653a2227f8ecfee2c0f51cd680093ac764 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -2096,7 +2096,8 @@ int cmd_index_pack(int argc,\n \tALLOC_ARRAY(idx_objects, nr_objects);\n \tfor (i = 0; i < nr_objects; i++)\n \t\tidx_objects[i] = &objects[i].idx;\n-\tcurr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_hash);\n+\tcurr_index = write_idx_file(the_hash_algo, index_name, idx_objects,\n+\t\t\t\t    nr_objects, &opts, pack_hash);\n \tif (rev_index)\n \t\tcurr_rev_index = write_rev_file(rev_index_name, idx_objects,\n \t\t\t\t\t\tnr_objects, pack_hash,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex ffc62930b68c9b4152057572ede216381a4b0991..7b2dacda514c29f5393b604f15d379abb81546c0 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1369,9 +1369,10 @@ static void write_pack_file(void)\n \t\t\tif (cruft)\n \t\t\t\tpack_idx_opts.flags |= WRITE_MTIMES;\n \n-\t\t\tstage_tmp_packfiles(&tmpname, pack_tmp_name,\n-\t\t\t\t\t    written_list, nr_written,\n-\t\t\t\t\t    &to_pack, &pack_idx_opts, hash,\n+\t\t\tstage_tmp_packfiles(the_hash_algo, &tmpname,\n+\t\t\t\t\t    pack_tmp_name, written_list,\n+\t\t\t\t\t    nr_written, &to_pack,\n+\t\t\t\t\t    &pack_idx_opts, hash,\n \t\t\t\t\t    &idx_tmp_name);\n \n \t\t\tif (write_bitmap_index) {\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex c4b085f57f74fb8b998576ac9d84fed9e01ed0ed..0d49889bfbb233d58fc094f5e2c2d2433dad9851 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -44,8 +44,9 @@ static void finish_tmp_packfile(struct strbuf *basename,\n {\n \tchar *idx_tmp_name = NULL;\n \n-\tstage_tmp_packfiles(basename, pack_tmp_name, written_list, nr_written,\n-\t\t\t    NULL, pack_idx_opts, hash, &idx_tmp_name);\n+\tstage_tmp_packfiles(the_hash_algo, basename, pack_tmp_name,\n+\t\t\t    written_list, nr_written, NULL, pack_idx_opts, hash,\n+\t\t\t    &idx_tmp_name);\n \trename_tmp_packfile_idx(basename, &idx_tmp_name);\n \n \tfree(idx_tmp_name);\ndiff --git a/pack-write.c b/pack-write.c\nindex 0cd75d2e55419362a61cf981fc11117ea7a1d88a..f344e78a9ec20cea9812a5eaffc72ae0b7e7424d 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -56,7 +56,8 @@ static int need_large_offset(off_t offset, const struct pack_idx_option *opts)\n  * The *sha1 contains the pack content SHA1 hash.\n  * The objects array passed in will be sorted by SHA1 on exit.\n  */\n-const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects,\n+const char *write_idx_file(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *index_name, struct pack_idx_entry **objects,\n \t\t\t   int nr_objects, const struct pack_idx_option *opts,\n \t\t\t   const unsigned char *sha1)\n {\n@@ -130,7 +131,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec\n \t\tstruct pack_idx_entry *obj = *list++;\n \t\tif (index_version < 2)\n \t\t\thashwrite_be32(f, obj->offset);\n-\t\thashwrite(f, obj->oid.hash, the_hash_algo->rawsz);\n+\t\thashwrite(f, obj->oid.hash, hash_algo->rawsz);\n \t\tif ((opts->flags & WRITE_IDX_STRICT) &&\n \t\t    (i && oideq(&list[-2]->oid, &obj->oid)))\n \t\t\tdie(\"The same object %s appears twice in the pack\",\n@@ -172,7 +173,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec\n \t\t}\n \t}\n \n-\thashwrite(f, sha1, the_hash_algo->rawsz);\n+\thashwrite(f, sha1, hash_algo->rawsz);\n \tfinalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,\n \t\t\t  CSUM_HASH_IN_STREAM | CSUM_CLOSE |\n \t\t\t  ((opts->flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));\n@@ -546,7 +547,8 @@ void rename_tmp_packfile_idx(struct strbuf *name_buffer,\n \trename_tmp_packfile(name_buffer, *idx_tmp_name, \"idx\");\n }\n \n-void stage_tmp_packfiles(struct strbuf *name_buffer,\n+void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n+\t\t\t struct strbuf *name_buffer,\n \t\t\t const char *pack_tmp_name,\n \t\t\t struct pack_idx_entry **written_list,\n \t\t\t uint32_t nr_written,\n@@ -561,8 +563,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,\n \tif (adjust_shared_perm(pack_tmp_name))\n \t\tdie_errno(\"unable to make temporary pack file readable\");\n \n-\t*idx_tmp_name = (char *)write_idx_file(NULL, written_list, nr_written,\n-\t\t\t\t\t       pack_idx_opts, hash);\n+\t*idx_tmp_name = (char *)write_idx_file(hash_algo, NULL, written_list,\n+\t\t\t\t\t       nr_written, pack_idx_opts, hash);\n \tif (adjust_shared_perm(*idx_tmp_name))\n \t\tdie_errno(\"unable to make temporary index file readable\");\n \ndiff --git a/pack.h b/pack.h\nindex 46d85e5bec787c90af69700fd4b328b1ebf1d606..c650fdbe2dcde8055ad0efe55646338cd0f81df5 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -86,7 +86,12 @@ struct progress;\n /* Note, the data argument could be NULL if object type is blob */\n typedef int (*verify_fn)(const struct object_id *, enum object_type, unsigned long, void*, int*);\n \n-const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, const unsigned char *sha1);\n+const char *write_idx_file(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *index_name,\n+\t\t\t   struct pack_idx_entry **objects,\n+\t\t\t   int nr_objects,\n+\t\t\t   const struct pack_idx_option *,\n+\t\t\t   const unsigned char *sha1);\n int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);\n int verify_pack_index(struct packed_git *);\n int verify_pack(struct repository *, struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n@@ -119,7 +124,8 @@ int read_pack_header(int fd, struct pack_header *);\n struct packing_data;\n \n struct hashfile *create_tmp_packfile(char **pack_tmp_name);\n-void stage_tmp_packfiles(struct strbuf *name_buffer,\n+void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n+\t\t\t struct strbuf *name_buffer,\n \t\t\t const char *pack_tmp_name,\n \t\t\t struct pack_idx_entry **written_list,\n \t\t\t uint32_t nr_written,\n\n-- \n2.47.0\n\n\n"},{"id":"510690","messageId":"20250116-kn-the-repo-cleanup-v1-5-a2f4c8e1c4c3@gmail.com","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com","subject":"[PATCH 5/5] pack-write: pass hash_algo to `write_rev_*()`","fromName":"Karthik Nayak via B4 Relay","fromEmail":"devnull+karthik.188.gmail.com@kernel.org","sentAt":"2025-01-16T11:35:17Z","receivedAt":"2025-01-16T11:35:20Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"From: Karthik Nayak <karthik.188@gmail.com>\n\nThe `write_rev_*()` functions use the global `the_hash_algo` variable to\naccess the repository's hash function. Pass the hash from down as we've\nadded made them available in the previous few commits.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n pack-write.c | 30 ++++++++++++++++--------------\n 1 file changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/pack-write.c b/pack-write.c\nindex 09ecbcdb069cc9b0383295798ceb49cbdc632b64..a2faeb1895e41f4c17281380478f1f2cabcc6f24 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -1,5 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n-\n #include \"git-compat-util.h\"\n #include \"environment.h\"\n #include \"gettext.h\"\n@@ -211,9 +209,10 @@ static void write_rev_index_positions(struct hashfile *f,\n \t\thashwrite_be32(f, pack_order[i]);\n }\n \n-static void write_rev_trailer(struct hashfile *f, const unsigned char *hash)\n+static void write_rev_trailer(const struct git_hash_algo *hash_algo,\n+\t\t\t      struct hashfile *f, const unsigned char *hash)\n {\n-\thashwrite(f, hash, the_hash_algo->rawsz);\n+\thashwrite(f, hash, hash_algo->rawsz);\n }\n \n char *write_rev_file(const struct git_hash_algo *hash_algo,\n@@ -286,7 +285,7 @@ char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n \twrite_rev_header(hash_algo, f);\n \n \twrite_rev_index_positions(f, pack_order, nr_objects);\n-\twrite_rev_trailer(f, hash);\n+\twrite_rev_trailer(hash_algo, f, hash);\n \n \tif (adjust_shared_perm(path) < 0)\n \t\tdie(_(\"failed to make %s readable\"), path);\n@@ -298,11 +297,12 @@ char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n \treturn path;\n }\n \n-static void write_mtimes_header(struct hashfile *f)\n+static void write_mtimes_header(const struct git_hash_algo *hash_algo,\n+\t\t\t\tstruct hashfile *f)\n {\n \thashwrite_be32(f, MTIMES_SIGNATURE);\n \thashwrite_be32(f, MTIMES_VERSION);\n-\thashwrite_be32(f, oid_version(the_hash_algo));\n+\thashwrite_be32(f, oid_version(hash_algo));\n }\n \n /*\n@@ -322,12 +322,14 @@ static void write_mtimes_objects(struct hashfile *f,\n \t}\n }\n \n-static void write_mtimes_trailer(struct hashfile *f, const unsigned char *hash)\n+static void write_mtimes_trailer(const struct git_hash_algo *hash_algo,\n+\t\t\t\t struct hashfile *f, const unsigned char *hash)\n {\n-\thashwrite(f, hash, the_hash_algo->rawsz);\n+\thashwrite(f, hash, hash_algo->rawsz);\n }\n \n-static char *write_mtimes_file(struct packing_data *to_pack,\n+static char *write_mtimes_file(const struct git_hash_algo *hash_algo,\n+\t\t\t       struct packing_data *to_pack,\n \t\t\t       struct pack_idx_entry **objects,\n \t\t\t       uint32_t nr_objects,\n \t\t\t       const unsigned char *hash)\n@@ -344,9 +346,9 @@ static char *write_mtimes_file(struct packing_data *to_pack,\n \tmtimes_name = strbuf_detach(&tmp_file, NULL);\n \tf = hashfd(fd, mtimes_name);\n \n-\twrite_mtimes_header(f);\n+\twrite_mtimes_header(hash_algo, f);\n \twrite_mtimes_objects(f, to_pack, objects, nr_objects);\n-\twrite_mtimes_trailer(f, hash);\n+\twrite_mtimes_trailer(hash_algo, f, hash);\n \n \tif (adjust_shared_perm(mtimes_name) < 0)\n \t\tdie(_(\"failed to make %s readable\"), mtimes_name);\n@@ -575,8 +577,8 @@ void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n \t\t\t\t      hash, pack_idx_opts->flags);\n \n \tif (pack_idx_opts->flags & WRITE_MTIMES) {\n-\t\tmtimes_tmp_name = write_mtimes_file(to_pack, written_list,\n-\t\t\t\t\t\t    nr_written,\n+\t\tmtimes_tmp_name = write_mtimes_file(hash_algo, to_pack,\n+\t\t\t\t\t\t    written_list, nr_written,\n \t\t\t\t\t\t    hash);\n \t}\n \n\n-- \n2.47.0\n\n\n"},{"id":"510697","messageId":"Z4kIg8ihbgPPb3C_@pks.im","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-3-a2f4c8e1c4c3@gmail.com","subject":"Re: [PATCH 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-16T13:24:28Z","receivedAt":"2025-01-16T13:24:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 16, 2025 at 12:35:15PM +0100, Karthik Nayak via B4 Relay wrote:\n> @@ -546,7 +547,8 @@ void rename_tmp_packfile_idx(struct strbuf *name_buffer,\n>  \trename_tmp_packfile(name_buffer, *idx_tmp_name, \"idx\");\n>  }\n>  \n> -void stage_tmp_packfiles(struct strbuf *name_buffer,\n> +void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n> +\t\t\t struct strbuf *name_buffer,\n>  \t\t\t const char *pack_tmp_name,\n>  \t\t\t struct pack_idx_entry **written_list,\n>  \t\t\t uint32_t nr_written,\n\nThis change was somewhat unexpected to me as it wasn't mentioned, so it\nmakes you wonder why it's different than `wried_idx_file()`.\n\nPatrick\n"},{"id":"510698","messageId":"Z4kIkY17fFDRrBdV@pks.im","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-4-a2f4c8e1c4c3@gmail.com","subject":"Re: [PATCH 4/5] pack-write: pass hash_algo to `write_rev_file()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-16T13:24:33Z","receivedAt":"2025-01-16T13:24:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Jan 16, 2025 at 12:35:16PM +0100, Karthik Nayak via B4 Relay wrote:\n> From: Karthik Nayak <karthik.188@gmail.com>\n> \n> The `write_rev_file()` function uses the global `the_hash_algo` variable\n> to access the repository's hash function. To avoid global variable\n> usage, let's pass the hash function from the layers above.\n> \n> Altough the layers above could have access to the hash function\n> internally, simply pass in `the_hash_algo`. This avoids any\n> compatibility issues and bubbles up global variable usage to upper\n> layers which can be eventually resolved.\n> \n> However, in `midx-write.c`, since all usage of global variables is\n> removed, don't reintroduce them and instead use the `repo` available in\n> the context.\n\nYeah, this feels quite sensible. We know this file is supposedly\n`the_repository`-clean, and callers expect it to be, so reintroducing it\nwouldn't be sensible.\n\nPatrick\n"},{"id":"510726","messageId":"xmqqo706r34s.fsf@gitster.g","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-2-a2f4c8e1c4c3@gmail.com","subject":"Re: [PATCH 2/5] pack-write: pass repository to `index_pack_lockfile()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-16T19:05:07Z","receivedAt":"2025-01-16T19:05:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak via B4 Relay\n<devnull+karthik.188.gmail.com@kernel.org> writes:\n\n> From: Karthik Nayak <karthik.188@gmail.com>\n>\n> The `index_pack_lockfile()` function uses the global `the_repository`\n> variable to access the repository. To avoid global variable usage, pass\n> the repository from the layers above.\n>\n> Altough the layers above could have access to the hash function\n\nI do not think the choice of the hash algorithm has much to do with\nthis change, though ;-)\n\n> internally, simply pass in `the_hash_algo`. This avoids any\n> compatibility issues and bubbles up global variable usage to upper\n> layers which can be eventually resolved.\n>\n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  builtin/receive-pack.c | 2 +-\n>  fetch-pack.c           | 4 +++-\n>  pack-write.c           | 6 +++---\n>  pack.h                 | 2 +-\n>  4 files changed, 8 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 56347a79633505efe8dc05acf1583b4c9995eefe..b83abe5d220cefd3707b701409dc5e6b67566599 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -2304,7 +2304,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n>  \t\tif (status)\n>  \t\t\treturn \"index-pack fork failed\";\n>  \n> -\t\tlockfile = index_pack_lockfile(child.out, NULL);\n> +\t\tlockfile = index_pack_lockfile(the_repository, child.out, NULL);\n>  \t\tif (lockfile) {\n>  \t\t\tpack_lockfile = register_tempfile(lockfile);\n>  \t\t\tfree(lockfile);\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index 3a227721ed0935d1f9c40584c57f54043354c032..824f56ecbca11cd9e4da6a3e4c450c6b2e7078ab 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -1036,7 +1036,9 @@ static int get_pack(struct fetch_pack_args *args,\n>  \t\tdie(_(\"fetch-pack: unable to fork off %s\"), cmd_name);\n>  \tif (do_keep && (pack_lockfiles || fsck_objects)) {\n>  \t\tint is_well_formed;\n> -\t\tchar *pack_lockfile = index_pack_lockfile(cmd.out, &is_well_formed);\n> +\t\tchar *pack_lockfile = index_pack_lockfile(the_repository,\n> +\t\t\t\t\t\t\t  cmd.out,\n> +\t\t\t\t\t\t\t  &is_well_formed);\n>  \n>  \t\tif (!is_well_formed)\n>  \t\t\tdie(_(\"fetch-pack: invalid index-pack output\"));\n> diff --git a/pack-write.c b/pack-write.c\n> index fc887850dfb9789132b8642733c6472944dbe32d..0cd75d2e55419362a61cf981fc11117ea7a1d88a 100644\n> --- a/pack-write.c\n> +++ b/pack-write.c\n> @@ -460,10 +460,10 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n>  \tfsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);\n>  }\n>  \n> -char *index_pack_lockfile(int ip_out, int *is_well_formed)\n> +char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)\n>  {\n>  \tchar packname[GIT_MAX_HEXSZ + 6];\n> -\tconst int len = the_hash_algo->hexsz + 6;\n> +\tconst int len = r->hash_algo->hexsz + 6;\n>  \n>  \t/*\n>  \t * The first thing we expect from index-pack's output\n> @@ -480,7 +480,7 @@ char *index_pack_lockfile(int ip_out, int *is_well_formed)\n>  \t\tpackname[len-1] = 0;\n>  \t\tif (skip_prefix(packname, \"keep\\t\", &name))\n>  \t\t\treturn xstrfmt(\"%s/pack/pack-%s.keep\",\n> -\t\t\t\t       repo_get_object_directory(the_repository), name);\n> +\t\t\t\t       repo_get_object_directory(r), name);\n>  \t\treturn NULL;\n>  \t}\n>  \tif (is_well_formed)\n> diff --git a/pack.h b/pack.h\n> index 6d9d477adc83e83d9e9175ccf699c100b4c147c6..46d85e5bec787c90af69700fd4b328b1ebf1d606 100644\n> --- a/pack.h\n> +++ b/pack.h\n> @@ -94,7 +94,7 @@ off_t write_pack_header(struct hashfile *f, uint32_t);\n>  void fixup_pack_header_footer(const struct git_hash_algo *, int,\n>  \t\t\t      unsigned char *, const char *, uint32_t,\n>  \t\t\t      unsigned char *, off_t);\n> -char *index_pack_lockfile(int fd, int *is_well_formed);\n> +char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed);\n>  \n>  struct ref;\n"},{"id":"510727","messageId":"xmqqjzaur2ry.fsf@gitster.g","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-3-a2f4c8e1c4c3@gmail.com","subject":"Re: [PATCH 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-16T19:12:49Z","receivedAt":"2025-01-16T19:12:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak via B4 Relay\n<devnull+karthik.188.gmail.com@kernel.org> writes:\n\n> From: Karthik Nayak <karthik.188@gmail.com>\n>\n> The `write_idx_file()` function uses the global `the_hash_algo` variable\n> to access the repository's hash function. To avoid global variable\n> usage, pass the hash function from the layers above.\n>\n> Altough the layers above could have access to the hash function\n> internally, simply pass in `the_hash_algo`. This avoids any\n> compatibility issues and bubbles up global variable usage to upper\n> layers which can be eventually resolved.\n> ...\n> -void stage_tmp_packfiles(struct strbuf *name_buffer,\n> +void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n> +\t\t\t struct strbuf *name_buffer,\n>  \t\t\t const char *pack_tmp_name,\n>  \t\t\t struct pack_idx_entry **written_list,\n>  \t\t\t uint32_t nr_written,\n> @@ -561,8 +563,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,\n>  \tif (adjust_shared_perm(pack_tmp_name))\n>  \t\tdie_errno(\"unable to make temporary pack file readable\");\n>  \n> -\t*idx_tmp_name = (char *)write_idx_file(NULL, written_list, nr_written,\n> -\t\t\t\t\t       pack_idx_opts, hash);\n> +\t*idx_tmp_name = (char *)write_idx_file(hash_algo, NULL, written_list,\n> +\t\t\t\t\t       nr_written, pack_idx_opts, hash);\n\nThe proposed log message should mention the reason why this\nstage_tmp_packfiles() function needs to be singled out among many\nother direct callers of write_idx_file() function.  \n\nIn other words, ...\n\n> @@ -798,8 +798,8 @@ static const char *create_index(void)\n>  \tif (c != last)\n>  \t\tdie(\"internal consistency error creating the index\");\n>  \n> -\ttmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts,\n> -\t\t\t\t pack_data->hash);\n> +\ttmpfile = write_idx_file(the_hash_algo, NULL, idx, object_count,\n> +\t\t\t\t &pack_idx_opts, pack_data->hash);\n>  \tfree(idx);\n>  \treturn tmpfile;\n>  }\n\n... this hunk could have made create_index() to take a git_hash_algo\nobject and pass it down to write_idx_file(), while changing all the\ncallers of create_index() pass the_hash_algo, but we did not do so.\n\nBut stage_tmp_packfiles() got that treatment.  Please tell your\nreaders in the proposed log message what makes it special.\n\nThanks.\n\n"},{"id":"510728","messageId":"xmqq8qrar1qm.fsf@gitster.g","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-1-a2f4c8e1c4c3@gmail.com","subject":"Re: [PATCH 1/5] pack-write: pass hash_algo to `fixup_pack_header_footer()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-16T19:35:13Z","receivedAt":"2025-01-16T19:35:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak via B4 Relay\n<devnull+karthik.188.gmail.com@kernel.org> writes:\n\n> From: Karthik Nayak <karthik.188@gmail.com>\n>\n> The `fixup_pack_header_footer()` function uses the global\n> `the_hash_algo` variable to access the repository's hash function. To\n> avoid global variable usage, pass the hash function from the layers\n> above.\n>\n> Altough the layers above could have access to the hash function\n> internally, simply pass in `the_hash_algo`. This avoids any\n> compatibility issues and bubbles up global variable usage to upper\n> layers which can be eventually resolved.\n\nOK, so the aim is to make a bug-to-bug compatible rewrite at this\nstep.  We used to use the_hash_algo at the lowermost callee, so have\nit take a hash_algo as parameter, but have everybody pass the_hash_algo\nto it, even though a caller might have a more \"suitable\" hash_algo\nfor the data it is feeding the lower layer.\n\nWhich makes sense.\n\nNaturally, as we are producing pack stream, we ought to have an idea\non what hash function is in use to name these objects in the pack\nstream and the hash function used at the csum-file level, so all\ncallers ought to have the information eventually (and if some of\nthem are relying on the_hash_algo currently, they need to be fixed).\nAll of that can be left for some future date and blindly passing\ndown the_hash_algo from all callers in this patch is a good approach\nto establish a solid foundation to build on.\n"},{"id":"510729","messageId":"xmqq34hir1qk.fsf@gitster.g","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-4-a2f4c8e1c4c3@gmail.com","subject":"Re: [PATCH 4/5] pack-write: pass hash_algo to `write_rev_file()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-16T19:35:15Z","receivedAt":"2025-01-16T19:35:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak via B4 Relay\n<devnull+karthik.188.gmail.com@kernel.org> writes:\n\n> From: Karthik Nayak <karthik.188@gmail.com>\n>\n> The `write_rev_file()` function uses the global `the_hash_algo` variable\n> to access the repository's hash function. To avoid global variable\n> usage, let's pass the hash function from the layers above.\n\nThere are a few other functions that got an extra git_hash_algo\nparameter in this patch.  The changes to them all look sensible,\nbut it would be nice to mention them here.\n"},{"id":"510765","messageId":"CAOLa=ZTF_2Gq9sFZqwu3Sw2dcQficBW3FOVeJnBDuwc5n4e-gg@mail.gmail.com","threadId":"62817","inReplyTo":"xmqqo706r34s.fsf@gitster.g","subject":"Re: [PATCH 2/5] pack-write: pass repository to `index_pack_lockfile()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T08:47:42Z","receivedAt":"2025-01-17T08:47:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Karthik Nayak via B4 Relay\n> <devnull+karthik.188.gmail.com@kernel.org> writes:\n>\n>> From: Karthik Nayak <karthik.188@gmail.com>\n>>\n>> The `index_pack_lockfile()` function uses the global `the_repository`\n>> variable to access the repository. To avoid global variable usage, pass\n>> the repository from the layers above.\n>>\n>> Altough the layers above could have access to the hash function\n>\n> I do not think the choice of the hash algorithm has much to do with\n> this change, though ;-)\n>\n\nOops, copy-paste error. Thanks for pointing out, will fix.\n\n[snip]\n"},{"id":"510766","messageId":"CAOLa=ZTGzEwKp7LtXGnHLo1cayryXouy1MCLPrNRCXPzgXk4rw@mail.gmail.com","threadId":"62817","inReplyTo":"Z4kIg8ihbgPPb3C_@pks.im","subject":"Re: [PATCH 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T08:56:55Z","receivedAt":"2025-01-17T08:56:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Thu, Jan 16, 2025 at 12:35:15PM +0100, Karthik Nayak via B4 Relay wrote:\n>> @@ -546,7 +547,8 @@ void rename_tmp_packfile_idx(struct strbuf *name_buffer,\n>>  \trename_tmp_packfile(name_buffer, *idx_tmp_name, \"idx\");\n>>  }\n>>\n>> -void stage_tmp_packfiles(struct strbuf *name_buffer,\n>> +void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n>> +\t\t\t struct strbuf *name_buffer,\n>>  \t\t\t const char *pack_tmp_name,\n>>  \t\t\t struct pack_idx_entry **written_list,\n>>  \t\t\t uint32_t nr_written,\n>\n> This change was somewhat unexpected to me as it wasn't mentioned, so it\n> makes you wonder why it's different than `wried_idx_file()`.\n>\n\nShould've mentioned it. This was also modified since it exists in\n'pack-write.c'. Will amend the commit message.\n\n> Patrick\n"},{"id":"510767","messageId":"CAOLa=ZS8EeC_G56CYq9CT8cwvb5QL3nEgY1Fo1_zz4uCn8jB_A@mail.gmail.com","threadId":"62817","inReplyTo":"xmqqjzaur2ry.fsf@gitster.g","subject":"Re: [PATCH 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T08:58:11Z","receivedAt":"2025-01-17T08:58:13Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Karthik Nayak via B4 Relay\n> <devnull+karthik.188.gmail.com@kernel.org> writes:\n>\n>> From: Karthik Nayak <karthik.188@gmail.com>\n>>\n>> The `write_idx_file()` function uses the global `the_hash_algo` variable\n>> to access the repository's hash function. To avoid global variable\n>> usage, pass the hash function from the layers above.\n>>\n>> Altough the layers above could have access to the hash function\n>> internally, simply pass in `the_hash_algo`. This avoids any\n>> compatibility issues and bubbles up global variable usage to upper\n>> layers which can be eventually resolved.\n>> ...\n>> -void stage_tmp_packfiles(struct strbuf *name_buffer,\n>> +void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n>> +\t\t\t struct strbuf *name_buffer,\n>>  \t\t\t const char *pack_tmp_name,\n>>  \t\t\t struct pack_idx_entry **written_list,\n>>  \t\t\t uint32_t nr_written,\n>> @@ -561,8 +563,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,\n>>  \tif (adjust_shared_perm(pack_tmp_name))\n>>  \t\tdie_errno(\"unable to make temporary pack file readable\");\n>>\n>> -\t*idx_tmp_name = (char *)write_idx_file(NULL, written_list, nr_written,\n>> -\t\t\t\t\t       pack_idx_opts, hash);\n>> +\t*idx_tmp_name = (char *)write_idx_file(hash_algo, NULL, written_list,\n>> +\t\t\t\t\t       nr_written, pack_idx_opts, hash);\n>\n> The proposed log message should mention the reason why this\n> stage_tmp_packfiles() function needs to be singled out among many\n> other direct callers of write_idx_file() function.\n>\n\nAgreed, Patrick also pointed out the same.\n\n> In other words, ...\n>\n>> @@ -798,8 +798,8 @@ static const char *create_index(void)\n>>  \tif (c != last)\n>>  \t\tdie(\"internal consistency error creating the index\");\n>>\n>> -\ttmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts,\n>> -\t\t\t\t pack_data->hash);\n>> +\ttmpfile = write_idx_file(the_hash_algo, NULL, idx, object_count,\n>> +\t\t\t\t &pack_idx_opts, pack_data->hash);\n>>  \tfree(idx);\n>>  \treturn tmpfile;\n>>  }\n>\n> ... this hunk could have made create_index() to take a git_hash_algo\n> object and pass it down to write_idx_file(), while changing all the\n> callers of create_index() pass the_hash_algo, but we did not do so.\n>\n> But stage_tmp_packfiles() got that treatment.  Please tell your\n> readers in the proposed log message what makes it special.\n>\n> Thanks.\n>\n\nWill fix it in the next version, but in short, it was because the\nfunction is also part of 'pack-write.c' and we're focusing only on\ncleanup of 'pack-write.c'.\n"},{"id":"510769","messageId":"20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com","threadId":"62817","inReplyTo":"20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com","subject":"[PATCH v2 0/5] pack-write: cleanup usage of global variables","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T09:20:47Z","receivedAt":"2025-01-17T09:21:16Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This is a small series to remove global variable usage from\n`pack-write.c`. Mostly it bubble's up the usage of global variables to\nupper layers. The only exception is in `write-midx.c`, which was cleaned\nof global variable usage, so there, we use the repo that is in available\nin the context.\n\nThis series is based on fbe8d3079d (Git 2.48, 2025-01-10) with\n'ps/more-sign-compare' and 'ps/the-repository' merged in.\n\nThere are no conflicts with topics in 'next', however there is a\nconflict with 'tb/incremental-midx-part-2' in 'seen', the fix is simple\nbut happy to merge that in too if necessary.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\nChanges in v2:\n- Fixes to the commit messages to:\n  - Fix copy-paste error s/the_hash_algo/the_repository\n  - Mention why certain functions are modified\n  - Small cleanups\n- Link to v1: https://lore.kernel.org/r/20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com\n\n---\nKarthik Nayak (5):\n      pack-write: pass hash_algo to `fixup_pack_header_footer()`\n      pack-write: pass repository to `index_pack_lockfile()`\n      pack-write: pass hash_algo to `write_idx_file()`\n      pack-write: pass hash_algo to `write_rev_file()`\n      pack-write: pass hash_algo to internal functions\n\n builtin/fast-import.c  | 11 +++---\n builtin/index-pack.c   | 11 +++---\n builtin/pack-objects.c | 12 +++---\n builtin/receive-pack.c |  2 +-\n bulk-checkin.c         |  7 ++--\n fetch-pack.c           |  4 +-\n midx-write.c           |  4 +-\n pack-write.c           | 99 +++++++++++++++++++++++++++-----------------------\n pack.h                 | 30 ++++++++++++---\n 9 files changed, 106 insertions(+), 74 deletions(-)\n---\n\nRange-diff versus v1:\n\n1:  4e365523a5 = 1:  ed4ba01c95 pack-write: pass hash_algo to `fixup_pack_header_footer()`\n2:  efa224f83b ! 2:  10aeaa5afc pack-write: pass repository to `index_pack_lockfile()`\n    @@ Commit message\n         variable to access the repository. To avoid global variable usage, pass\n         the repository from the layers above.\n     \n    -    Altough the layers above could have access to the hash function\n    -    internally, simply pass in `the_hash_algo`. This avoids any\n    -    compatibility issues and bubbles up global variable usage to upper\n    -    layers which can be eventually resolved.\n    +    Altough the layers above could have access to the repository internally,\n    +    simply pass in `the_repository`. This avoids any compatibility issues\n    +    and bubbles up global variable usage to upper layers which can be\n    +    eventually resolved.\n     \n         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n     \n3:  6cca689d32 ! 3:  a4dbc3906d pack-write: pass hash_algo to `write_idx_file()`\n    @@ Commit message\n         to access the repository's hash function. To avoid global variable\n         usage, pass the hash function from the layers above.\n     \n    +    Since `stage_tmp_packfiles()` also resides in 'pack-write.c' and calls\n    +    `write_idx_file()`, update it to accept `the_hash_algo` as a parameter\n    +    and pass it through to the callee.\n    +\n         Altough the layers above could have access to the hash function\n         internally, simply pass in `the_hash_algo`. This avoids any\n         compatibility issues and bubbles up global variable usage to upper\n4:  b29eb6a305 ! 4:  d06d8ee962 pack-write: pass hash_algo to `write_rev_file()`\n    @@ Commit message\n     \n         The `write_rev_file()` function uses the global `the_hash_algo` variable\n         to access the repository's hash function. To avoid global variable\n    -    usage, let's pass the hash function from the layers above.\n    +    usage, let's pass the hash function from the layers above. Also modify\n    +    children functions `write_rev_file_order()` and `write_rev_header()` to\n    +    accept 'the_hash_algo'.\n     \n         Altough the layers above could have access to the hash function\n         internally, simply pass in `the_hash_algo`. This avoids any\n5:  438bfec4de ! 5:  1e34f5a70c pack-write: pass hash_algo to `write_rev_*()`\n    @@ Metadata\n     Author: Karthik Nayak <karthik.188@gmail.com>\n     \n      ## Commit message ##\n    -    pack-write: pass hash_algo to `write_rev_*()`\n    +    pack-write: pass hash_algo to internal functions\n     \n    -    The `write_rev_*()` functions use the global `the_hash_algo` variable to\n    -    access the repository's hash function. Pass the hash from down as we've\n    -    added made them available in the previous few commits.\n    +    The internal functions `write_rev_trailer()`, `write_rev_trailer()`,\n    +    `write_mtimes_header()` and write_mtimes_trailer()` use the global\n    +    `the_hash_algo` variable to access the repository's hash function. Pass\n    +    the hash from down as we've added made them available in the previous\n    +    few commits.\n    +\n    +    This removes all global variables from the 'pack-write.c' file, so\n    +    remove the 'USE_THE_REPOSITORY_VARIABLE' macro.\n     \n         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n     \n\n\n---\n\nbase-commit: 8b2efc058aaa3d1437678616bccf7c5f7ce1f92b\nchange-id: 20250110-kn-the-repo-cleanup-44144fa42dc3\n\nThanks\n- Karthik\n\n"},{"id":"510770","messageId":"20250117-kn-the-repo-cleanup-v2-1-a7fdc19688f5@gmail.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com","subject":"[PATCH v2 1/5] pack-write: pass hash_algo to `fixup_pack_header_footer()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T09:20:48Z","receivedAt":"2025-01-17T09:21:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `fixup_pack_header_footer()` function uses the global\n`the_hash_algo` variable to access the repository's hash function. To\navoid global variable usage, pass the hash function from the layers\nabove.\n\nAltough the layers above could have access to the hash function\ninternally, simply pass in `the_hash_algo`. This avoids any\ncompatibility issues and bubbles up global variable usage to upper\nlayers which can be eventually resolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/fast-import.c  |  7 ++++---\n builtin/index-pack.c   |  2 +-\n builtin/pack-objects.c |  5 +++--\n bulk-checkin.c         |  2 +-\n pack-write.c           | 28 ++++++++++++++--------------\n pack.h                 |  4 +++-\n 6 files changed, 26 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa..6baf2b1b71e2443a7987a41e0cd246076682bf58 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -878,9 +878,10 @@ static void end_packfile(void)\n \n \t\tclose_pack_windows(pack_data);\n \t\tfinalize_hashfile(pack_file, cur_pack_oid.hash, FSYNC_COMPONENT_PACK, 0);\n-\t\tfixup_pack_header_footer(pack_data->pack_fd, pack_data->hash,\n-\t\t\t\t\t pack_data->pack_name, object_count,\n-\t\t\t\t\t cur_pack_oid.hash, pack_size);\n+\t\tfixup_pack_header_footer(the_hash_algo, pack_data->pack_fd,\n+\t\t\t\t\t pack_data->hash, pack_data->pack_name,\n+\t\t\t\t\t object_count, cur_pack_oid.hash,\n+\t\t\t\t\t pack_size);\n \n \t\tif (object_count <= unpack_limit) {\n \t\t\tif (!loosen_small_pack(pack_data)) {\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 0bef61c57232e198ba539cd44ec301d26dcb0eb8..6c5e3483f4fe67fb2e26132c55b1f8395d60c11f 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1390,7 +1390,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha\n \t\tstrbuf_release(&msg);\n \t\tfinalize_hashfile(f, tail_hash, FSYNC_COMPONENT_PACK, 0);\n \t\thashcpy(read_hash, pack_hash, the_repository->hash_algo);\n-\t\tfixup_pack_header_footer(output_fd, pack_hash,\n+\t\tfixup_pack_header_footer(the_hash_algo, output_fd, pack_hash,\n \t\t\t\t\t curr_pack, nr_objects,\n \t\t\t\t\t read_hash, consumed_bytes-the_hash_algo->rawsz);\n \t\tif (!hasheq(read_hash, tail_hash, the_repository->hash_algo))\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex d51c021d99d9f470c04b7ec52565ab2f4c1c19ae..ffc62930b68c9b4152057572ede216381a4b0991 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1319,8 +1319,9 @@ static void write_pack_file(void)\n \t\t\t */\n \n \t\t\tint fd = finalize_hashfile(f, hash, FSYNC_COMPONENT_PACK, 0);\n-\t\t\tfixup_pack_header_footer(fd, hash, pack_tmp_name,\n-\t\t\t\t\t\t nr_written, hash, offset);\n+\t\t\tfixup_pack_header_footer(the_hash_algo, fd, hash,\n+\t\t\t\t\t\t pack_tmp_name, nr_written,\n+\t\t\t\t\t\t hash, offset);\n \t\t\tclose(fd);\n \t\t\tif (write_bitmap_index) {\n \t\t\t\tif (write_bitmap_index != WRITE_BITMAP_QUIET)\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex 5044cb7fa083d692a3797e2491c27b61ec44c69c..c4b085f57f74fb8b998576ac9d84fed9e01ed0ed 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -70,7 +70,7 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state)\n \t\t\t\t  CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);\n \t} else {\n \t\tint fd = finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK, 0);\n-\t\tfixup_pack_header_footer(fd, hash, state->pack_tmp_name,\n+\t\tfixup_pack_header_footer(the_hash_algo, fd, hash, state->pack_tmp_name,\n \t\t\t\t\t state->nr_written, hash,\n \t\t\t\t\t state->offset);\n \t\tclose(fd);\ndiff --git a/pack-write.c b/pack-write.c\nindex 98a8c0e7853d7b46b5ce9a9672e0249ff051b5f9..fc887850dfb9789132b8642733c6472944dbe32d 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -380,7 +380,8 @@ off_t write_pack_header(struct hashfile *f, uint32_t nr_entries)\n  * partial_pack_sha1 can refer to the same buffer if the caller is not\n  * interested in the resulting SHA1 of pack data above partial_pack_offset.\n  */\n-void fixup_pack_header_footer(int pack_fd,\n+void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n+\t\t\t int pack_fd,\n \t\t\t unsigned char *new_pack_hash,\n \t\t\t const char *pack_name,\n \t\t\t uint32_t object_count,\n@@ -393,8 +394,8 @@ void fixup_pack_header_footer(int pack_fd,\n \tchar *buf;\n \tssize_t read_result;\n \n-\tthe_hash_algo->init_fn(&old_hash_ctx);\n-\tthe_hash_algo->init_fn(&new_hash_ctx);\n+\thash_algo->init_fn(&old_hash_ctx);\n+\thash_algo->init_fn(&new_hash_ctx);\n \n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n@@ -406,9 +407,9 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\t  pack_name);\n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n-\tthe_hash_algo->update_fn(&old_hash_ctx, &hdr, sizeof(hdr));\n+\thash_algo->update_fn(&old_hash_ctx, &hdr, sizeof(hdr));\n \thdr.hdr_entries = htonl(object_count);\n-\tthe_hash_algo->update_fn(&new_hash_ctx, &hdr, sizeof(hdr));\n+\thash_algo->update_fn(&new_hash_ctx, &hdr, sizeof(hdr));\n \twrite_or_die(pack_fd, &hdr, sizeof(hdr));\n \tpartial_pack_offset -= sizeof(hdr);\n \n@@ -423,7 +424,7 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\tbreak;\n \t\tif (n < 0)\n \t\t\tdie_errno(\"Failed to checksum '%s'\", pack_name);\n-\t\tthe_hash_algo->update_fn(&new_hash_ctx, buf, n);\n+\t\thash_algo->update_fn(&new_hash_ctx, buf, n);\n \n \t\taligned_sz -= n;\n \t\tif (!aligned_sz)\n@@ -432,13 +433,12 @@ void fixup_pack_header_footer(int pack_fd,\n \t\tif (!partial_pack_hash)\n \t\t\tcontinue;\n \n-\t\tthe_hash_algo->update_fn(&old_hash_ctx, buf, n);\n+\t\thash_algo->update_fn(&old_hash_ctx, buf, n);\n \t\tpartial_pack_offset -= n;\n \t\tif (partial_pack_offset == 0) {\n \t\t\tunsigned char hash[GIT_MAX_RAWSZ];\n-\t\t\tthe_hash_algo->final_fn(hash, &old_hash_ctx);\n-\t\t\tif (!hasheq(hash, partial_pack_hash,\n-\t\t\t\t    the_repository->hash_algo))\n+\t\t\thash_algo->final_fn(hash, &old_hash_ctx);\n+\t\t\tif (!hasheq(hash, partial_pack_hash, hash_algo))\n \t\t\t\tdie(\"Unexpected checksum for %s \"\n \t\t\t\t    \"(disk corruption?)\", pack_name);\n \t\t\t/*\n@@ -446,7 +446,7 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\t * pack, which also means making partial_pack_offset\n \t\t\t * big enough not to matter anymore.\n \t\t\t */\n-\t\t\tthe_hash_algo->init_fn(&old_hash_ctx);\n+\t\t\thash_algo->init_fn(&old_hash_ctx);\n \t\t\tpartial_pack_offset = ~partial_pack_offset;\n \t\t\tpartial_pack_offset -= MSB(partial_pack_offset, 1);\n \t\t}\n@@ -454,9 +454,9 @@ void fixup_pack_header_footer(int pack_fd,\n \tfree(buf);\n \n \tif (partial_pack_hash)\n-\t\tthe_hash_algo->final_fn(partial_pack_hash, &old_hash_ctx);\n-\tthe_hash_algo->final_fn(new_pack_hash, &new_hash_ctx);\n-\twrite_or_die(pack_fd, new_pack_hash, the_hash_algo->rawsz);\n+\t\thash_algo->final_fn(partial_pack_hash, &old_hash_ctx);\n+\thash_algo->final_fn(new_pack_hash, &new_hash_ctx);\n+\twrite_or_die(pack_fd, new_pack_hash, hash_algo->rawsz);\n \tfsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);\n }\n \ndiff --git a/pack.h b/pack.h\nindex a8da0406299bf267205256aaa8efb215f2ff73be..6d9d477adc83e83d9e9175ccf699c100b4c147c6 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -91,7 +91,9 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offs\n int verify_pack_index(struct packed_git *);\n int verify_pack(struct repository *, struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n off_t write_pack_header(struct hashfile *f, uint32_t);\n-void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);\n+void fixup_pack_header_footer(const struct git_hash_algo *, int,\n+\t\t\t      unsigned char *, const char *, uint32_t,\n+\t\t\t      unsigned char *, off_t);\n char *index_pack_lockfile(int fd, int *is_well_formed);\n \n struct ref;\n\n-- \n2.47.0\n\n"},{"id":"510771","messageId":"20250117-kn-the-repo-cleanup-v2-2-a7fdc19688f5@gmail.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com","subject":"[PATCH v2 2/5] pack-write: pass repository to `index_pack_lockfile()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T09:20:49Z","receivedAt":"2025-01-17T09:21:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `index_pack_lockfile()` function uses the global `the_repository`\nvariable to access the repository. To avoid global variable usage, pass\nthe repository from the layers above.\n\nAltough the layers above could have access to the repository internally,\nsimply pass in `the_repository`. This avoids any compatibility issues\nand bubbles up global variable usage to upper layers which can be\neventually resolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/receive-pack.c | 2 +-\n fetch-pack.c           | 4 +++-\n pack-write.c           | 6 +++---\n pack.h                 | 2 +-\n 4 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 56347a79633505efe8dc05acf1583b4c9995eefe..b83abe5d220cefd3707b701409dc5e6b67566599 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2304,7 +2304,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n \t\tif (status)\n \t\t\treturn \"index-pack fork failed\";\n \n-\t\tlockfile = index_pack_lockfile(child.out, NULL);\n+\t\tlockfile = index_pack_lockfile(the_repository, child.out, NULL);\n \t\tif (lockfile) {\n \t\t\tpack_lockfile = register_tempfile(lockfile);\n \t\t\tfree(lockfile);\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 3a227721ed0935d1f9c40584c57f54043354c032..824f56ecbca11cd9e4da6a3e4c450c6b2e7078ab 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1036,7 +1036,9 @@ static int get_pack(struct fetch_pack_args *args,\n \t\tdie(_(\"fetch-pack: unable to fork off %s\"), cmd_name);\n \tif (do_keep && (pack_lockfiles || fsck_objects)) {\n \t\tint is_well_formed;\n-\t\tchar *pack_lockfile = index_pack_lockfile(cmd.out, &is_well_formed);\n+\t\tchar *pack_lockfile = index_pack_lockfile(the_repository,\n+\t\t\t\t\t\t\t  cmd.out,\n+\t\t\t\t\t\t\t  &is_well_formed);\n \n \t\tif (!is_well_formed)\n \t\t\tdie(_(\"fetch-pack: invalid index-pack output\"));\ndiff --git a/pack-write.c b/pack-write.c\nindex fc887850dfb9789132b8642733c6472944dbe32d..0cd75d2e55419362a61cf981fc11117ea7a1d88a 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -460,10 +460,10 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n \tfsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);\n }\n \n-char *index_pack_lockfile(int ip_out, int *is_well_formed)\n+char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)\n {\n \tchar packname[GIT_MAX_HEXSZ + 6];\n-\tconst int len = the_hash_algo->hexsz + 6;\n+\tconst int len = r->hash_algo->hexsz + 6;\n \n \t/*\n \t * The first thing we expect from index-pack's output\n@@ -480,7 +480,7 @@ char *index_pack_lockfile(int ip_out, int *is_well_formed)\n \t\tpackname[len-1] = 0;\n \t\tif (skip_prefix(packname, \"keep\\t\", &name))\n \t\t\treturn xstrfmt(\"%s/pack/pack-%s.keep\",\n-\t\t\t\t       repo_get_object_directory(the_repository), name);\n+\t\t\t\t       repo_get_object_directory(r), name);\n \t\treturn NULL;\n \t}\n \tif (is_well_formed)\ndiff --git a/pack.h b/pack.h\nindex 6d9d477adc83e83d9e9175ccf699c100b4c147c6..46d85e5bec787c90af69700fd4b328b1ebf1d606 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -94,7 +94,7 @@ off_t write_pack_header(struct hashfile *f, uint32_t);\n void fixup_pack_header_footer(const struct git_hash_algo *, int,\n \t\t\t      unsigned char *, const char *, uint32_t,\n \t\t\t      unsigned char *, off_t);\n-char *index_pack_lockfile(int fd, int *is_well_formed);\n+char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed);\n \n struct ref;\n \n\n-- \n2.47.0\n\n"},{"id":"510772","messageId":"20250117-kn-the-repo-cleanup-v2-3-a7fdc19688f5@gmail.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com","subject":"[PATCH v2 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T09:20:50Z","receivedAt":"2025-01-17T09:21:24Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `write_idx_file()` function uses the global `the_hash_algo` variable\nto access the repository's hash function. To avoid global variable\nusage, pass the hash function from the layers above.\n\nSince `stage_tmp_packfiles()` also resides in 'pack-write.c' and calls\n`write_idx_file()`, update it to accept `the_hash_algo` as a parameter\nand pass it through to the callee.\n\nAltough the layers above could have access to the hash function\ninternally, simply pass in `the_hash_algo`. This avoids any\ncompatibility issues and bubbles up global variable usage to upper\nlayers which can be eventually resolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/fast-import.c  |  4 ++--\n builtin/index-pack.c   |  3 ++-\n builtin/pack-objects.c |  7 ++++---\n bulk-checkin.c         |  5 +++--\n pack-write.c           | 14 ++++++++------\n pack.h                 | 10 ++++++++--\n 6 files changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 6baf2b1b71e2443a7987a41e0cd246076682bf58..c4bc52f93c011f34bd7f98c8e9d74a33cf9783bd 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -798,8 +798,8 @@ static const char *create_index(void)\n \tif (c != last)\n \t\tdie(\"internal consistency error creating the index\");\n \n-\ttmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts,\n-\t\t\t\t pack_data->hash);\n+\ttmpfile = write_idx_file(the_hash_algo, NULL, idx, object_count,\n+\t\t\t\t &pack_idx_opts, pack_data->hash);\n \tfree(idx);\n \treturn tmpfile;\n }\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 6c5e3483f4fe67fb2e26132c55b1f8395d60c11f..d73699653a2227f8ecfee2c0f51cd680093ac764 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -2096,7 +2096,8 @@ int cmd_index_pack(int argc,\n \tALLOC_ARRAY(idx_objects, nr_objects);\n \tfor (i = 0; i < nr_objects; i++)\n \t\tidx_objects[i] = &objects[i].idx;\n-\tcurr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_hash);\n+\tcurr_index = write_idx_file(the_hash_algo, index_name, idx_objects,\n+\t\t\t\t    nr_objects, &opts, pack_hash);\n \tif (rev_index)\n \t\tcurr_rev_index = write_rev_file(rev_index_name, idx_objects,\n \t\t\t\t\t\tnr_objects, pack_hash,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex ffc62930b68c9b4152057572ede216381a4b0991..7b2dacda514c29f5393b604f15d379abb81546c0 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1369,9 +1369,10 @@ static void write_pack_file(void)\n \t\t\tif (cruft)\n \t\t\t\tpack_idx_opts.flags |= WRITE_MTIMES;\n \n-\t\t\tstage_tmp_packfiles(&tmpname, pack_tmp_name,\n-\t\t\t\t\t    written_list, nr_written,\n-\t\t\t\t\t    &to_pack, &pack_idx_opts, hash,\n+\t\t\tstage_tmp_packfiles(the_hash_algo, &tmpname,\n+\t\t\t\t\t    pack_tmp_name, written_list,\n+\t\t\t\t\t    nr_written, &to_pack,\n+\t\t\t\t\t    &pack_idx_opts, hash,\n \t\t\t\t\t    &idx_tmp_name);\n \n \t\t\tif (write_bitmap_index) {\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex c4b085f57f74fb8b998576ac9d84fed9e01ed0ed..0d49889bfbb233d58fc094f5e2c2d2433dad9851 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -44,8 +44,9 @@ static void finish_tmp_packfile(struct strbuf *basename,\n {\n \tchar *idx_tmp_name = NULL;\n \n-\tstage_tmp_packfiles(basename, pack_tmp_name, written_list, nr_written,\n-\t\t\t    NULL, pack_idx_opts, hash, &idx_tmp_name);\n+\tstage_tmp_packfiles(the_hash_algo, basename, pack_tmp_name,\n+\t\t\t    written_list, nr_written, NULL, pack_idx_opts, hash,\n+\t\t\t    &idx_tmp_name);\n \trename_tmp_packfile_idx(basename, &idx_tmp_name);\n \n \tfree(idx_tmp_name);\ndiff --git a/pack-write.c b/pack-write.c\nindex 0cd75d2e55419362a61cf981fc11117ea7a1d88a..f344e78a9ec20cea9812a5eaffc72ae0b7e7424d 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -56,7 +56,8 @@ static int need_large_offset(off_t offset, const struct pack_idx_option *opts)\n  * The *sha1 contains the pack content SHA1 hash.\n  * The objects array passed in will be sorted by SHA1 on exit.\n  */\n-const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects,\n+const char *write_idx_file(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *index_name, struct pack_idx_entry **objects,\n \t\t\t   int nr_objects, const struct pack_idx_option *opts,\n \t\t\t   const unsigned char *sha1)\n {\n@@ -130,7 +131,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec\n \t\tstruct pack_idx_entry *obj = *list++;\n \t\tif (index_version < 2)\n \t\t\thashwrite_be32(f, obj->offset);\n-\t\thashwrite(f, obj->oid.hash, the_hash_algo->rawsz);\n+\t\thashwrite(f, obj->oid.hash, hash_algo->rawsz);\n \t\tif ((opts->flags & WRITE_IDX_STRICT) &&\n \t\t    (i && oideq(&list[-2]->oid, &obj->oid)))\n \t\t\tdie(\"The same object %s appears twice in the pack\",\n@@ -172,7 +173,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec\n \t\t}\n \t}\n \n-\thashwrite(f, sha1, the_hash_algo->rawsz);\n+\thashwrite(f, sha1, hash_algo->rawsz);\n \tfinalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,\n \t\t\t  CSUM_HASH_IN_STREAM | CSUM_CLOSE |\n \t\t\t  ((opts->flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));\n@@ -546,7 +547,8 @@ void rename_tmp_packfile_idx(struct strbuf *name_buffer,\n \trename_tmp_packfile(name_buffer, *idx_tmp_name, \"idx\");\n }\n \n-void stage_tmp_packfiles(struct strbuf *name_buffer,\n+void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n+\t\t\t struct strbuf *name_buffer,\n \t\t\t const char *pack_tmp_name,\n \t\t\t struct pack_idx_entry **written_list,\n \t\t\t uint32_t nr_written,\n@@ -561,8 +563,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,\n \tif (adjust_shared_perm(pack_tmp_name))\n \t\tdie_errno(\"unable to make temporary pack file readable\");\n \n-\t*idx_tmp_name = (char *)write_idx_file(NULL, written_list, nr_written,\n-\t\t\t\t\t       pack_idx_opts, hash);\n+\t*idx_tmp_name = (char *)write_idx_file(hash_algo, NULL, written_list,\n+\t\t\t\t\t       nr_written, pack_idx_opts, hash);\n \tif (adjust_shared_perm(*idx_tmp_name))\n \t\tdie_errno(\"unable to make temporary index file readable\");\n \ndiff --git a/pack.h b/pack.h\nindex 46d85e5bec787c90af69700fd4b328b1ebf1d606..c650fdbe2dcde8055ad0efe55646338cd0f81df5 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -86,7 +86,12 @@ struct progress;\n /* Note, the data argument could be NULL if object type is blob */\n typedef int (*verify_fn)(const struct object_id *, enum object_type, unsigned long, void*, int*);\n \n-const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, const unsigned char *sha1);\n+const char *write_idx_file(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *index_name,\n+\t\t\t   struct pack_idx_entry **objects,\n+\t\t\t   int nr_objects,\n+\t\t\t   const struct pack_idx_option *,\n+\t\t\t   const unsigned char *sha1);\n int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);\n int verify_pack_index(struct packed_git *);\n int verify_pack(struct repository *, struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n@@ -119,7 +124,8 @@ int read_pack_header(int fd, struct pack_header *);\n struct packing_data;\n \n struct hashfile *create_tmp_packfile(char **pack_tmp_name);\n-void stage_tmp_packfiles(struct strbuf *name_buffer,\n+void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n+\t\t\t struct strbuf *name_buffer,\n \t\t\t const char *pack_tmp_name,\n \t\t\t struct pack_idx_entry **written_list,\n \t\t\t uint32_t nr_written,\n\n-- \n2.47.0\n\n"},{"id":"510773","messageId":"20250117-kn-the-repo-cleanup-v2-4-a7fdc19688f5@gmail.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com","subject":"[PATCH v2 4/5] pack-write: pass hash_algo to `write_rev_file()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T09:20:51Z","receivedAt":"2025-01-17T09:21:28Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `write_rev_file()` function uses the global `the_hash_algo` variable\nto access the repository's hash function. To avoid global variable\nusage, let's pass the hash function from the layers above. Also modify\nchildren functions `write_rev_file_order()` and `write_rev_header()` to\naccept 'the_hash_algo'.\n\nAltough the layers above could have access to the hash function\ninternally, simply pass in `the_hash_algo`. This avoids any\ncompatibility issues and bubbles up global variable usage to upper\nlayers which can be eventually resolved.\n\nHowever, in `midx-write.c`, since all usage of global variables is\nremoved, don't reintroduce them and instead use the `repo` available in\nthe context.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/index-pack.c |  6 +++---\n midx-write.c         |  4 ++--\n pack-write.c         | 21 ++++++++++++---------\n pack.h               | 14 ++++++++++++--\n 4 files changed, 29 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex d73699653a2227f8ecfee2c0f51cd680093ac764..e803cb2444633f937fafc841848dae85341475f1 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -2099,9 +2099,9 @@ int cmd_index_pack(int argc,\n \tcurr_index = write_idx_file(the_hash_algo, index_name, idx_objects,\n \t\t\t\t    nr_objects, &opts, pack_hash);\n \tif (rev_index)\n-\t\tcurr_rev_index = write_rev_file(rev_index_name, idx_objects,\n-\t\t\t\t\t\tnr_objects, pack_hash,\n-\t\t\t\t\t\topts.flags);\n+\t\tcurr_rev_index = write_rev_file(the_hash_algo, rev_index_name,\n+\t\t\t\t\t\tidx_objects, nr_objects,\n+\t\t\t\t\t\tpack_hash, opts.flags);\n \tfree(idx_objects);\n \n \tif (!verify)\ndiff --git a/midx-write.c b/midx-write.c\nindex b3827b936bdb1df12c73fb7d9b98ff65fc875cc3..61b59d557d3ba46a39db74c62393545cabf50f2c 100644\n--- a/midx-write.c\n+++ b/midx-write.c\n@@ -658,8 +658,8 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,\n \tstrbuf_addf(&buf, \"%s-%s.rev\", midx_name, hash_to_hex_algop(midx_hash,\n \t\t\t\t\t\t\t\t    ctx->repo->hash_algo));\n \n-\ttmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr,\n-\t\t\t\t\tmidx_hash, WRITE_REV);\n+\ttmp_file = write_rev_file_order(ctx->repo->hash_algo, NULL, ctx->pack_order,\n+\t\t\t\t\tctx->entries_nr, midx_hash, WRITE_REV);\n \n \tif (finalize_object_file(tmp_file, buf.buf))\n \t\tdie(_(\"cannot store reverse index file\"));\ndiff --git a/pack-write.c b/pack-write.c\nindex f344e78a9ec20cea9812a5eaffc72ae0b7e7424d..09ecbcdb069cc9b0383295798ceb49cbdc632b64 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -194,11 +194,12 @@ static int pack_order_cmp(const void *va, const void *vb, void *ctx)\n \treturn 0;\n }\n \n-static void write_rev_header(struct hashfile *f)\n+static void write_rev_header(const struct git_hash_algo *hash_algo,\n+\t\t\t     struct hashfile *f)\n {\n \thashwrite_be32(f, RIDX_SIGNATURE);\n \thashwrite_be32(f, RIDX_VERSION);\n-\thashwrite_be32(f, oid_version(the_hash_algo));\n+\thashwrite_be32(f, oid_version(hash_algo));\n }\n \n static void write_rev_index_positions(struct hashfile *f,\n@@ -215,7 +216,8 @@ static void write_rev_trailer(struct hashfile *f, const unsigned char *hash)\n \thashwrite(f, hash, the_hash_algo->rawsz);\n }\n \n-char *write_rev_file(const char *rev_name,\n+char *write_rev_file(const struct git_hash_algo *hash_algo,\n+\t\t     const char *rev_name,\n \t\t     struct pack_idx_entry **objects,\n \t\t     uint32_t nr_objects,\n \t\t     const unsigned char *hash,\n@@ -233,15 +235,16 @@ char *write_rev_file(const char *rev_name,\n \t\tpack_order[i] = i;\n \tQSORT_S(pack_order, nr_objects, pack_order_cmp, objects);\n \n-\tret = write_rev_file_order(rev_name, pack_order, nr_objects, hash,\n-\t\t\t\t   flags);\n+\tret = write_rev_file_order(hash_algo, rev_name, pack_order, nr_objects,\n+\t\t\t\t   hash, flags);\n \n \tfree(pack_order);\n \n \treturn ret;\n }\n \n-char *write_rev_file_order(const char *rev_name,\n+char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *rev_name,\n \t\t\t   uint32_t *pack_order,\n \t\t\t   uint32_t nr_objects,\n \t\t\t   const unsigned char *hash,\n@@ -280,7 +283,7 @@ char *write_rev_file_order(const char *rev_name,\n \t\treturn NULL;\n \t}\n \n-\twrite_rev_header(f);\n+\twrite_rev_header(hash_algo, f);\n \n \twrite_rev_index_positions(f, pack_order, nr_objects);\n \twrite_rev_trailer(f, hash);\n@@ -568,8 +571,8 @@ void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n \tif (adjust_shared_perm(*idx_tmp_name))\n \t\tdie_errno(\"unable to make temporary index file readable\");\n \n-\trev_tmp_name = write_rev_file(NULL, written_list, nr_written, hash,\n-\t\t\t\t      pack_idx_opts->flags);\n+\trev_tmp_name = write_rev_file(hash_algo, NULL, written_list, nr_written,\n+\t\t\t\t      hash, pack_idx_opts->flags);\n \n \tif (pack_idx_opts->flags & WRITE_MTIMES) {\n \t\tmtimes_tmp_name = write_mtimes_file(to_pack, written_list,\ndiff --git a/pack.h b/pack.h\nindex c650fdbe2dcde8055ad0efe55646338cd0f81df5..8a84ea475cda902936ee0b6091c5b4d462606be8 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -105,8 +105,18 @@ struct ref;\n \n void write_promisor_file(const char *promisor_name, struct ref **sought, int nr_sought);\n \n-char *write_rev_file(const char *rev_name, struct pack_idx_entry **objects, uint32_t nr_objects, const unsigned char *hash, unsigned flags);\n-char *write_rev_file_order(const char *rev_name, uint32_t *pack_order, uint32_t nr_objects, const unsigned char *hash, unsigned flags);\n+char *write_rev_file(const struct git_hash_algo *hash_algo,\n+\t\t     const char *rev_name,\n+\t\t     struct pack_idx_entry **objects,\n+\t\t     uint32_t nr_objects,\n+\t\t     const unsigned char *hash,\n+\t\t     unsigned flags);\n+char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *rev_name,\n+\t\t\t   uint32_t *pack_order,\n+\t\t\t   uint32_t nr_objects,\n+\t\t\t   const unsigned char *hash,\n+\t\t\t   unsigned flags);\n \n /*\n  * The \"hdr\" output buffer should be at least this big, which will handle sizes\n\n-- \n2.47.0\n\n"},{"id":"510774","messageId":"20250117-kn-the-repo-cleanup-v2-5-a7fdc19688f5@gmail.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com","subject":"[PATCH v2 5/5] pack-write: pass hash_algo to internal functions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T09:20:52Z","receivedAt":"2025-01-17T09:21:31Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The internal functions `write_rev_trailer()`, `write_rev_trailer()`,\n`write_mtimes_header()` and write_mtimes_trailer()` use the global\n`the_hash_algo` variable to access the repository's hash function. Pass\nthe hash from down as we've added made them available in the previous\nfew commits.\n\nThis removes all global variables from the 'pack-write.c' file, so\nremove the 'USE_THE_REPOSITORY_VARIABLE' macro.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n pack-write.c | 30 ++++++++++++++++--------------\n 1 file changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/pack-write.c b/pack-write.c\nindex 09ecbcdb069cc9b0383295798ceb49cbdc632b64..a2faeb1895e41f4c17281380478f1f2cabcc6f24 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -1,5 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n-\n #include \"git-compat-util.h\"\n #include \"environment.h\"\n #include \"gettext.h\"\n@@ -211,9 +209,10 @@ static void write_rev_index_positions(struct hashfile *f,\n \t\thashwrite_be32(f, pack_order[i]);\n }\n \n-static void write_rev_trailer(struct hashfile *f, const unsigned char *hash)\n+static void write_rev_trailer(const struct git_hash_algo *hash_algo,\n+\t\t\t      struct hashfile *f, const unsigned char *hash)\n {\n-\thashwrite(f, hash, the_hash_algo->rawsz);\n+\thashwrite(f, hash, hash_algo->rawsz);\n }\n \n char *write_rev_file(const struct git_hash_algo *hash_algo,\n@@ -286,7 +285,7 @@ char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n \twrite_rev_header(hash_algo, f);\n \n \twrite_rev_index_positions(f, pack_order, nr_objects);\n-\twrite_rev_trailer(f, hash);\n+\twrite_rev_trailer(hash_algo, f, hash);\n \n \tif (adjust_shared_perm(path) < 0)\n \t\tdie(_(\"failed to make %s readable\"), path);\n@@ -298,11 +297,12 @@ char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n \treturn path;\n }\n \n-static void write_mtimes_header(struct hashfile *f)\n+static void write_mtimes_header(const struct git_hash_algo *hash_algo,\n+\t\t\t\tstruct hashfile *f)\n {\n \thashwrite_be32(f, MTIMES_SIGNATURE);\n \thashwrite_be32(f, MTIMES_VERSION);\n-\thashwrite_be32(f, oid_version(the_hash_algo));\n+\thashwrite_be32(f, oid_version(hash_algo));\n }\n \n /*\n@@ -322,12 +322,14 @@ static void write_mtimes_objects(struct hashfile *f,\n \t}\n }\n \n-static void write_mtimes_trailer(struct hashfile *f, const unsigned char *hash)\n+static void write_mtimes_trailer(const struct git_hash_algo *hash_algo,\n+\t\t\t\t struct hashfile *f, const unsigned char *hash)\n {\n-\thashwrite(f, hash, the_hash_algo->rawsz);\n+\thashwrite(f, hash, hash_algo->rawsz);\n }\n \n-static char *write_mtimes_file(struct packing_data *to_pack,\n+static char *write_mtimes_file(const struct git_hash_algo *hash_algo,\n+\t\t\t       struct packing_data *to_pack,\n \t\t\t       struct pack_idx_entry **objects,\n \t\t\t       uint32_t nr_objects,\n \t\t\t       const unsigned char *hash)\n@@ -344,9 +346,9 @@ static char *write_mtimes_file(struct packing_data *to_pack,\n \tmtimes_name = strbuf_detach(&tmp_file, NULL);\n \tf = hashfd(fd, mtimes_name);\n \n-\twrite_mtimes_header(f);\n+\twrite_mtimes_header(hash_algo, f);\n \twrite_mtimes_objects(f, to_pack, objects, nr_objects);\n-\twrite_mtimes_trailer(f, hash);\n+\twrite_mtimes_trailer(hash_algo, f, hash);\n \n \tif (adjust_shared_perm(mtimes_name) < 0)\n \t\tdie(_(\"failed to make %s readable\"), mtimes_name);\n@@ -575,8 +577,8 @@ void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n \t\t\t\t      hash, pack_idx_opts->flags);\n \n \tif (pack_idx_opts->flags & WRITE_MTIMES) {\n-\t\tmtimes_tmp_name = write_mtimes_file(to_pack, written_list,\n-\t\t\t\t\t\t    nr_written,\n+\t\tmtimes_tmp_name = write_mtimes_file(hash_algo, to_pack,\n+\t\t\t\t\t\t    written_list, nr_written,\n \t\t\t\t\t\t    hash);\n \t}\n \n\n-- \n2.47.0\n\n"},{"id":"510779","messageId":"Z4onCYz5zD0L1_Q-@pks.im","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-5-a7fdc19688f5@gmail.com","subject":"Re: [PATCH v2 5/5] pack-write: pass hash_algo to internal functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-17T09:46:49Z","receivedAt":"2025-01-17T09:46:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Jan 17, 2025 at 10:20:52AM +0100, Karthik Nayak wrote:\n> The internal functions `write_rev_trailer()`, `write_rev_trailer()`,\n> `write_mtimes_header()` and write_mtimes_trailer()` use the global\n> `the_hash_algo` variable to access the repository's hash function. Pass\n> the hash from down as we've added made them available in the previous\n\nThis doesn't read quite right -- from where do we want to pass it down?\nOther than that the series looks good to me, thanks!\n\nPatrick\n"},{"id":"510780","messageId":"CAOLa=ZRaJUHCANv9ot6FbtgYnEDVV3itWnF--0g3NikLZkohmA@mail.gmail.com","threadId":"62817","inReplyTo":"Z4onCYz5zD0L1_Q-@pks.im","subject":"Re: [PATCH v2 5/5] pack-write: pass hash_algo to internal functions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-17T09:55:13Z","receivedAt":"2025-01-17T09:55:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Jan 17, 2025 at 10:20:52AM +0100, Karthik Nayak wrote:\n>> The internal functions `write_rev_trailer()`, `write_rev_trailer()`,\n>> `write_mtimes_header()` and write_mtimes_trailer()` use the global\n>> `the_hash_algo` variable to access the repository's hash function. Pass\n>> the hash from down as we've added made them available in the previous\n>\n> This doesn't read quite right -- from where do we want to pass it down?\n> Other than that the series looks good to me, thanks!\n>\n\nI should have s/from//, but let me rephrase it to make it clearer.\n\nThanks for the review.\n\n> Patrick\n"},{"id":"510814","messageId":"87zfjpiefc.fsf@iotcl.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-1-a7fdc19688f5@gmail.com","subject":"Re: [PATCH v2 1/5] pack-write: pass hash_algo to `fixup_pack_header_footer()`","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-01-17T16:38:15Z","receivedAt":"2025-01-17T16:38:31Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> The `fixup_pack_header_footer()` function uses the global\n> `the_hash_algo` variable to access the repository's hash function. To\n> avoid global variable usage, pass the hash function from the layers\n> above.\n\nI'm probably being overly pedantic here, so feel free to ignore me. But\nyou say \"pass the hash function\", technically that's not correct, you're\npassing down the struct that defines several properties of the hashing\nalgorithm. This includes the hash function, but also other properties\nlike the hex size. By using \"pass the hash function\" in the commit\nmessages (and not only this commit message) it sounds to me like you're\nchanging the type of the object that desribes the \"hash algo\". But\nagain, feel free to ignore this comment.\n\n-- \nToon\n"},{"id":"510815","messageId":"87wmetieb6.fsf@iotcl.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-3-a7fdc19688f5@gmail.com","subject":"Re: [PATCH v2 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2025-01-17T16:40:45Z","receivedAt":"2025-01-17T16:40:58Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Karthik Nayak <karthik.188@gmail.com> writes:\n\n> The `write_idx_file()` function uses the global `the_hash_algo` variable\n> to access the repository's hash function. To avoid global variable\n> usage, pass the hash function from the layers above.\n>\n> Since `stage_tmp_packfiles()` also resides in 'pack-write.c' and calls\n> `write_idx_file()`, update it to accept `the_hash_algo` as a parameter\n> and pass it through to the callee.\n\nTechnically speaking you're updating it to accept a `struct hash_algo`.\n\nBesides from this nit, and the other nit I've submitted on the first\npatch, these changes look good to me. I'm doubtful any of the comments\nneeds a reroll.\n\n-- \nToon\n"},{"id":"510818","messageId":"xmqqy0z9l3hg.fsf@gitster.g","threadId":"62817","inReplyTo":"87zfjpiefc.fsf@iotcl.com","subject":"Re: [PATCH v2 1/5] pack-write: pass hash_algo to `fixup_pack_header_footer()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-17T18:06:19Z","receivedAt":"2025-01-17T18:06:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Toon Claes <toon@iotcl.com> writes:\n\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> The `fixup_pack_header_footer()` function uses the global\n>> `the_hash_algo` variable to access the repository's hash function. To\n>> avoid global variable usage, pass the hash function from the layers\n>> above.\n>\n> I'm probably being overly pedantic here, so feel free to ignore me. But\n> you say \"pass the hash function\", technically that's not correct, you're\n> passing down the struct that defines several properties of the hashing\n> algorithm. This includes the hash function, but also other properties\n> like the hex size. By using \"pass the hash function\" in the commit\n> messages (and not only this commit message) it sounds to me like you're\n> changing the type of the object that desribes the \"hash algo\". But\n> again, feel free to ignore this comment.\n\nI think the phrasing in the title that uses hash_algo (instead of\n\"hash function\") is fine, so we can use \"pass a hash_algo from the\nlayers above\" in the body, perhaps?\n"},{"id":"510892","messageId":"CAOLa=ZTziDsW+M-oQvW1cqp=gzjc9ncmHFMR-vVvHK0aHp9ZYg@mail.gmail.com","threadId":"62817","inReplyTo":"xmqqy0z9l3hg.fsf@gitster.g","subject":"Re: [PATCH v2 1/5] pack-write: pass hash_algo to `fixup_pack_header_footer()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:07:39Z","receivedAt":"2025-01-19T11:07:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Toon Claes <toon@iotcl.com> writes:\n>\n>> Karthik Nayak <karthik.188@gmail.com> writes:\n>>\n>>> The `fixup_pack_header_footer()` function uses the global\n>>> `the_hash_algo` variable to access the repository's hash function. To\n>>> avoid global variable usage, pass the hash function from the layers\n>>> above.\n>>\n>> I'm probably being overly pedantic here, so feel free to ignore me. But\n>> you say \"pass the hash function\", technically that's not correct, you're\n>> passing down the struct that defines several properties of the hashing\n>> algorithm. This includes the hash function, but also other properties\n>> like the hex size. By using \"pass the hash function\" in the commit\n>> messages (and not only this commit message) it sounds to me like you're\n>> changing the type of the object that desribes the \"hash algo\". But\n>> again, feel free to ignore this comment.\n>\n> I think the phrasing in the title that uses hash_algo (instead of\n> \"hash function\") is fine, so we can use \"pass a hash_algo from the\n> layers above\" in the body, perhaps?\n\nThis is a good suggestion, I'll amend! Thanks both.\n"},{"id":"510893","messageId":"CAOLa=ZR=wftoUmOFa5BG_RxGX9v-85zeCs33cZT9Y+rrxa3sDA@mail.gmail.com","threadId":"62817","inReplyTo":"87wmetieb6.fsf@iotcl.com","subject":"Re: [PATCH v2 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:10:38Z","receivedAt":"2025-01-19T11:10:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Toon Claes <toon@iotcl.com> writes:\n\n> Karthik Nayak <karthik.188@gmail.com> writes:\n>\n>> The `write_idx_file()` function uses the global `the_hash_algo` variable\n>> to access the repository's hash function. To avoid global variable\n>> usage, pass the hash function from the layers above.\n>>\n>> Since `stage_tmp_packfiles()` also resides in 'pack-write.c' and calls\n>> `write_idx_file()`, update it to accept `the_hash_algo` as a parameter\n>> and pass it through to the callee.\n>\n> Technically speaking you're updating it to accept a `struct hash_algo`.\n>\n\nYes, but also the callers pass in `the_hash_algo`. But let me amend to\nmake it better.\n\n> Besides from this nit, and the other nit I've submitted on the first\n> patch, these changes look good to me. I'm doubtful any of the comments\n> needs a reroll.\n>\n> --\n> Toon\n\nThanks for the review! I'll re-roll with the fixes.\n"},{"id":"510894","messageId":"20250119-kn-the-repo-cleanup-v3-0-a495fce08d71@gmail.com","threadId":"62817","inReplyTo":"20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com","subject":"[PATCH v3 0/5] pack-write: cleanup usage of global variables","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:19:25Z","receivedAt":"2025-01-19T11:19:36Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This is a small series to remove global variable usage from\n`pack-write.c`. Mostly it bubble's up the usage of global variables to\nupper layers. The only exception is in `write-midx.c`, which was cleaned\nof global variable usage, so there, we use the repo that is in available\nin the context.\n\nThis series is based on fbe8d3079d (Git 2.48, 2025-01-10) with\n'ps/more-sign-compare' and 'ps/the-repository' merged in.\n\nThere are no conflicts with topics in 'next', however there is a\nconflict with 'tb/incremental-midx-part-2' in 'seen', the fix is simple\nbut happy to merge that in too if necessary.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\nChanges in v3:\n- Fix the ambiguous commit message in the last commit.\n- Link to v2: https://lore.kernel.org/r/20250117-kn-the-repo-cleanup-v2-0-a7fdc19688f5@gmail.com\n\nChanges in v2:\n- Fixes to the commit messages to:\n  - Fix copy-paste error s/the_hash_algo/the_repository\n  - Mention why certain functions are modified\n  - Small cleanups\n- Link to v1: https://lore.kernel.org/r/20250116-kn-the-repo-cleanup-v1-0-a2f4c8e1c4c3@gmail.com\n\n---\nKarthik Nayak (5):\n      pack-write: pass hash_algo to `fixup_pack_header_footer()`\n      pack-write: pass repository to `index_pack_lockfile()`\n      pack-write: pass hash_algo to `write_idx_file()`\n      pack-write: pass hash_algo to `write_rev_file()`\n      pack-write: pass hash_algo to internal functions\n\n builtin/fast-import.c  | 11 +++---\n builtin/index-pack.c   | 11 +++---\n builtin/pack-objects.c | 12 +++---\n builtin/receive-pack.c |  2 +-\n bulk-checkin.c         |  7 ++--\n fetch-pack.c           |  4 +-\n midx-write.c           |  4 +-\n pack-write.c           | 99 +++++++++++++++++++++++++++-----------------------\n pack.h                 | 30 ++++++++++++---\n 9 files changed, 106 insertions(+), 74 deletions(-)\n---\n\nRange-diff versus v2:\n\n1:  d3812f88e6 ! 1:  5f646111d0 pack-write: pass hash_algo to `fixup_pack_header_footer()`\n    @@ Commit message\n     \n         The `fixup_pack_header_footer()` function uses the global\n         `the_hash_algo` variable to access the repository's hash function. To\n    -    avoid global variable usage, pass the hash function from the layers\n    -    above.\n    +    avoid global variable usage, pass a hash_algo from the layers above.\n     \n    -    Altough the layers above could have access to the hash function\n    -    internally, simply pass in `the_hash_algo`. This avoids any\n    -    compatibility issues and bubbles up global variable usage to upper\n    -    layers which can be eventually resolved.\n    +    Altough the layers above could have access to the hash_algo internally,\n    +    simply pass in `the_hash_algo`. This avoids any compatibility issues and\n    +    bubbles up global variable usage to upper layers which can be eventually\n    +    resolved.\n     \n         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n     \n2:  89255a4822 = 2:  696e991823 pack-write: pass repository to `index_pack_lockfile()`\n3:  5ccf220732 ! 3:  21fefb8279 pack-write: pass hash_algo to `write_idx_file()`\n    @@ Commit message\n         pack-write: pass hash_algo to `write_idx_file()`\n     \n         The `write_idx_file()` function uses the global `the_hash_algo` variable\n    -    to access the repository's hash function. To avoid global variable\n    -    usage, pass the hash function from the layers above.\n    +    to access the repository's hash_algo. To avoid global variable usage,\n    +    pass a hash_algo from the layers above.\n     \n         Since `stage_tmp_packfiles()` also resides in 'pack-write.c' and calls\n    -    `write_idx_file()`, update it to accept `the_hash_algo` as a parameter\n    -    and pass it through to the callee.\n    +    `write_idx_file()`, update it to accept a `struct git_hash_algo` as a\n    +    parameter and pass it through to the callee.\n     \n    -    Altough the layers above could have access to the hash function\n    -    internally, simply pass in `the_hash_algo`. This avoids any\n    -    compatibility issues and bubbles up global variable usage to upper\n    -    layers which can be eventually resolved.\n    +    Altough the layers above could have access to the hash_algo internally,\n    +    simply pass in `the_hash_algo`. This avoids any compatibility issues and\n    +    bubbles up global variable usage to upper layers which can be eventually\n    +    resolved.\n     \n         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n     \n4:  95b1f784e2 ! 4:  3f730b26d1 pack-write: pass hash_algo to `write_rev_file()`\n    @@ Commit message\n         pack-write: pass hash_algo to `write_rev_file()`\n     \n         The `write_rev_file()` function uses the global `the_hash_algo` variable\n    -    to access the repository's hash function. To avoid global variable\n    -    usage, let's pass the hash function from the layers above. Also modify\n    -    children functions `write_rev_file_order()` and `write_rev_header()` to\n    -    accept 'the_hash_algo'.\n    +    to access the repository's hash_algo. To avoid global variable usage,\n    +    pass a hash_algo from the layers above. Also modify children functions\n    +    `write_rev_file_order()` and `write_rev_header()` to accept\n    +    'the_hash_algo'.\n     \n    -    Altough the layers above could have access to the hash function\n    -    internally, simply pass in `the_hash_algo`. This avoids any\n    -    compatibility issues and bubbles up global variable usage to upper\n    -    layers which can be eventually resolved.\n    +    Altough the layers above could have access to the hash_algo internally,\n    +    simply pass in `the_hash_algo`. This avoids any compatibility issues and\n    +    bubbles up global variable usage to upper layers which can be eventually\n    +    resolved.\n     \n         However, in `midx-write.c`, since all usage of global variables is\n         removed, don't reintroduce them and instead use the `repo` available in\n5:  50da9bf405 ! 5:  7d1d42ca67 pack-write: pass hash_algo to internal functions\n    @@ Commit message\n         The internal functions `write_rev_trailer()`, `write_rev_trailer()`,\n         `write_mtimes_header()` and write_mtimes_trailer()` use the global\n         `the_hash_algo` variable to access the repository's hash function. Pass\n    -    the hash from down as we've added made them available in the previous\n    -    few commits.\n    +    the hash_algo down from callers, all of which already have access to the\n    +    variable.\n     \n         This removes all global variables from the 'pack-write.c' file, so\n         remove the 'USE_THE_REPOSITORY_VARIABLE' macro.\n\n\n---\n\nbase-commit: 8b2efc058aaa3d1437678616bccf7c5f7ce1f92b\nchange-id: 20250110-kn-the-repo-cleanup-44144fa42dc3\n\nThanks\n- Karthik\n\n"},{"id":"510895","messageId":"20250119-kn-the-repo-cleanup-v3-1-a495fce08d71@gmail.com","threadId":"62817","inReplyTo":"20250119-kn-the-repo-cleanup-v3-0-a495fce08d71@gmail.com","subject":"[PATCH v3 1/5] pack-write: pass hash_algo to `fixup_pack_header_footer()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:19:26Z","receivedAt":"2025-01-19T11:19:39Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `fixup_pack_header_footer()` function uses the global\n`the_hash_algo` variable to access the repository's hash function. To\navoid global variable usage, pass a hash_algo from the layers above.\n\nAltough the layers above could have access to the hash_algo internally,\nsimply pass in `the_hash_algo`. This avoids any compatibility issues and\nbubbles up global variable usage to upper layers which can be eventually\nresolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/fast-import.c  |  7 ++++---\n builtin/index-pack.c   |  2 +-\n builtin/pack-objects.c |  5 +++--\n bulk-checkin.c         |  2 +-\n pack-write.c           | 28 ++++++++++++++--------------\n pack.h                 |  4 +++-\n 6 files changed, 26 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 0f86392761abbe6acb217fef7f4fe7c3ff5ac1fa..6baf2b1b71e2443a7987a41e0cd246076682bf58 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -878,9 +878,10 @@ static void end_packfile(void)\n \n \t\tclose_pack_windows(pack_data);\n \t\tfinalize_hashfile(pack_file, cur_pack_oid.hash, FSYNC_COMPONENT_PACK, 0);\n-\t\tfixup_pack_header_footer(pack_data->pack_fd, pack_data->hash,\n-\t\t\t\t\t pack_data->pack_name, object_count,\n-\t\t\t\t\t cur_pack_oid.hash, pack_size);\n+\t\tfixup_pack_header_footer(the_hash_algo, pack_data->pack_fd,\n+\t\t\t\t\t pack_data->hash, pack_data->pack_name,\n+\t\t\t\t\t object_count, cur_pack_oid.hash,\n+\t\t\t\t\t pack_size);\n \n \t\tif (object_count <= unpack_limit) {\n \t\t\tif (!loosen_small_pack(pack_data)) {\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 0bef61c57232e198ba539cd44ec301d26dcb0eb8..6c5e3483f4fe67fb2e26132c55b1f8395d60c11f 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1390,7 +1390,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha\n \t\tstrbuf_release(&msg);\n \t\tfinalize_hashfile(f, tail_hash, FSYNC_COMPONENT_PACK, 0);\n \t\thashcpy(read_hash, pack_hash, the_repository->hash_algo);\n-\t\tfixup_pack_header_footer(output_fd, pack_hash,\n+\t\tfixup_pack_header_footer(the_hash_algo, output_fd, pack_hash,\n \t\t\t\t\t curr_pack, nr_objects,\n \t\t\t\t\t read_hash, consumed_bytes-the_hash_algo->rawsz);\n \t\tif (!hasheq(read_hash, tail_hash, the_repository->hash_algo))\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex d51c021d99d9f470c04b7ec52565ab2f4c1c19ae..ffc62930b68c9b4152057572ede216381a4b0991 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1319,8 +1319,9 @@ static void write_pack_file(void)\n \t\t\t */\n \n \t\t\tint fd = finalize_hashfile(f, hash, FSYNC_COMPONENT_PACK, 0);\n-\t\t\tfixup_pack_header_footer(fd, hash, pack_tmp_name,\n-\t\t\t\t\t\t nr_written, hash, offset);\n+\t\t\tfixup_pack_header_footer(the_hash_algo, fd, hash,\n+\t\t\t\t\t\t pack_tmp_name, nr_written,\n+\t\t\t\t\t\t hash, offset);\n \t\t\tclose(fd);\n \t\t\tif (write_bitmap_index) {\n \t\t\t\tif (write_bitmap_index != WRITE_BITMAP_QUIET)\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex 5044cb7fa083d692a3797e2491c27b61ec44c69c..c4b085f57f74fb8b998576ac9d84fed9e01ed0ed 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -70,7 +70,7 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state)\n \t\t\t\t  CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);\n \t} else {\n \t\tint fd = finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK, 0);\n-\t\tfixup_pack_header_footer(fd, hash, state->pack_tmp_name,\n+\t\tfixup_pack_header_footer(the_hash_algo, fd, hash, state->pack_tmp_name,\n \t\t\t\t\t state->nr_written, hash,\n \t\t\t\t\t state->offset);\n \t\tclose(fd);\ndiff --git a/pack-write.c b/pack-write.c\nindex 98a8c0e7853d7b46b5ce9a9672e0249ff051b5f9..fc887850dfb9789132b8642733c6472944dbe32d 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -380,7 +380,8 @@ off_t write_pack_header(struct hashfile *f, uint32_t nr_entries)\n  * partial_pack_sha1 can refer to the same buffer if the caller is not\n  * interested in the resulting SHA1 of pack data above partial_pack_offset.\n  */\n-void fixup_pack_header_footer(int pack_fd,\n+void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n+\t\t\t int pack_fd,\n \t\t\t unsigned char *new_pack_hash,\n \t\t\t const char *pack_name,\n \t\t\t uint32_t object_count,\n@@ -393,8 +394,8 @@ void fixup_pack_header_footer(int pack_fd,\n \tchar *buf;\n \tssize_t read_result;\n \n-\tthe_hash_algo->init_fn(&old_hash_ctx);\n-\tthe_hash_algo->init_fn(&new_hash_ctx);\n+\thash_algo->init_fn(&old_hash_ctx);\n+\thash_algo->init_fn(&new_hash_ctx);\n \n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n@@ -406,9 +407,9 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\t  pack_name);\n \tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n \t\tdie_errno(\"Failed seeking to start of '%s'\", pack_name);\n-\tthe_hash_algo->update_fn(&old_hash_ctx, &hdr, sizeof(hdr));\n+\thash_algo->update_fn(&old_hash_ctx, &hdr, sizeof(hdr));\n \thdr.hdr_entries = htonl(object_count);\n-\tthe_hash_algo->update_fn(&new_hash_ctx, &hdr, sizeof(hdr));\n+\thash_algo->update_fn(&new_hash_ctx, &hdr, sizeof(hdr));\n \twrite_or_die(pack_fd, &hdr, sizeof(hdr));\n \tpartial_pack_offset -= sizeof(hdr);\n \n@@ -423,7 +424,7 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\tbreak;\n \t\tif (n < 0)\n \t\t\tdie_errno(\"Failed to checksum '%s'\", pack_name);\n-\t\tthe_hash_algo->update_fn(&new_hash_ctx, buf, n);\n+\t\thash_algo->update_fn(&new_hash_ctx, buf, n);\n \n \t\taligned_sz -= n;\n \t\tif (!aligned_sz)\n@@ -432,13 +433,12 @@ void fixup_pack_header_footer(int pack_fd,\n \t\tif (!partial_pack_hash)\n \t\t\tcontinue;\n \n-\t\tthe_hash_algo->update_fn(&old_hash_ctx, buf, n);\n+\t\thash_algo->update_fn(&old_hash_ctx, buf, n);\n \t\tpartial_pack_offset -= n;\n \t\tif (partial_pack_offset == 0) {\n \t\t\tunsigned char hash[GIT_MAX_RAWSZ];\n-\t\t\tthe_hash_algo->final_fn(hash, &old_hash_ctx);\n-\t\t\tif (!hasheq(hash, partial_pack_hash,\n-\t\t\t\t    the_repository->hash_algo))\n+\t\t\thash_algo->final_fn(hash, &old_hash_ctx);\n+\t\t\tif (!hasheq(hash, partial_pack_hash, hash_algo))\n \t\t\t\tdie(\"Unexpected checksum for %s \"\n \t\t\t\t    \"(disk corruption?)\", pack_name);\n \t\t\t/*\n@@ -446,7 +446,7 @@ void fixup_pack_header_footer(int pack_fd,\n \t\t\t * pack, which also means making partial_pack_offset\n \t\t\t * big enough not to matter anymore.\n \t\t\t */\n-\t\t\tthe_hash_algo->init_fn(&old_hash_ctx);\n+\t\t\thash_algo->init_fn(&old_hash_ctx);\n \t\t\tpartial_pack_offset = ~partial_pack_offset;\n \t\t\tpartial_pack_offset -= MSB(partial_pack_offset, 1);\n \t\t}\n@@ -454,9 +454,9 @@ void fixup_pack_header_footer(int pack_fd,\n \tfree(buf);\n \n \tif (partial_pack_hash)\n-\t\tthe_hash_algo->final_fn(partial_pack_hash, &old_hash_ctx);\n-\tthe_hash_algo->final_fn(new_pack_hash, &new_hash_ctx);\n-\twrite_or_die(pack_fd, new_pack_hash, the_hash_algo->rawsz);\n+\t\thash_algo->final_fn(partial_pack_hash, &old_hash_ctx);\n+\thash_algo->final_fn(new_pack_hash, &new_hash_ctx);\n+\twrite_or_die(pack_fd, new_pack_hash, hash_algo->rawsz);\n \tfsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);\n }\n \ndiff --git a/pack.h b/pack.h\nindex a8da0406299bf267205256aaa8efb215f2ff73be..6d9d477adc83e83d9e9175ccf699c100b4c147c6 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -91,7 +91,9 @@ int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offs\n int verify_pack_index(struct packed_git *);\n int verify_pack(struct repository *, struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n off_t write_pack_header(struct hashfile *f, uint32_t);\n-void fixup_pack_header_footer(int, unsigned char *, const char *, uint32_t, unsigned char *, off_t);\n+void fixup_pack_header_footer(const struct git_hash_algo *, int,\n+\t\t\t      unsigned char *, const char *, uint32_t,\n+\t\t\t      unsigned char *, off_t);\n char *index_pack_lockfile(int fd, int *is_well_formed);\n \n struct ref;\n\n-- \n2.47.0\n\n"},{"id":"510896","messageId":"20250119-kn-the-repo-cleanup-v3-2-a495fce08d71@gmail.com","threadId":"62817","inReplyTo":"20250119-kn-the-repo-cleanup-v3-0-a495fce08d71@gmail.com","subject":"[PATCH v3 2/5] pack-write: pass repository to `index_pack_lockfile()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:19:27Z","receivedAt":"2025-01-19T11:19:41Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `index_pack_lockfile()` function uses the global `the_repository`\nvariable to access the repository. To avoid global variable usage, pass\nthe repository from the layers above.\n\nAltough the layers above could have access to the repository internally,\nsimply pass in `the_repository`. This avoids any compatibility issues\nand bubbles up global variable usage to upper layers which can be\neventually resolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/receive-pack.c | 2 +-\n fetch-pack.c           | 4 +++-\n pack-write.c           | 6 +++---\n pack.h                 | 2 +-\n 4 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 56347a79633505efe8dc05acf1583b4c9995eefe..b83abe5d220cefd3707b701409dc5e6b67566599 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -2304,7 +2304,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)\n \t\tif (status)\n \t\t\treturn \"index-pack fork failed\";\n \n-\t\tlockfile = index_pack_lockfile(child.out, NULL);\n+\t\tlockfile = index_pack_lockfile(the_repository, child.out, NULL);\n \t\tif (lockfile) {\n \t\t\tpack_lockfile = register_tempfile(lockfile);\n \t\t\tfree(lockfile);\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 3a227721ed0935d1f9c40584c57f54043354c032..824f56ecbca11cd9e4da6a3e4c450c6b2e7078ab 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1036,7 +1036,9 @@ static int get_pack(struct fetch_pack_args *args,\n \t\tdie(_(\"fetch-pack: unable to fork off %s\"), cmd_name);\n \tif (do_keep && (pack_lockfiles || fsck_objects)) {\n \t\tint is_well_formed;\n-\t\tchar *pack_lockfile = index_pack_lockfile(cmd.out, &is_well_formed);\n+\t\tchar *pack_lockfile = index_pack_lockfile(the_repository,\n+\t\t\t\t\t\t\t  cmd.out,\n+\t\t\t\t\t\t\t  &is_well_formed);\n \n \t\tif (!is_well_formed)\n \t\t\tdie(_(\"fetch-pack: invalid index-pack output\"));\ndiff --git a/pack-write.c b/pack-write.c\nindex fc887850dfb9789132b8642733c6472944dbe32d..0cd75d2e55419362a61cf981fc11117ea7a1d88a 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -460,10 +460,10 @@ void fixup_pack_header_footer(const struct git_hash_algo *hash_algo,\n \tfsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);\n }\n \n-char *index_pack_lockfile(int ip_out, int *is_well_formed)\n+char *index_pack_lockfile(struct repository *r, int ip_out, int *is_well_formed)\n {\n \tchar packname[GIT_MAX_HEXSZ + 6];\n-\tconst int len = the_hash_algo->hexsz + 6;\n+\tconst int len = r->hash_algo->hexsz + 6;\n \n \t/*\n \t * The first thing we expect from index-pack's output\n@@ -480,7 +480,7 @@ char *index_pack_lockfile(int ip_out, int *is_well_formed)\n \t\tpackname[len-1] = 0;\n \t\tif (skip_prefix(packname, \"keep\\t\", &name))\n \t\t\treturn xstrfmt(\"%s/pack/pack-%s.keep\",\n-\t\t\t\t       repo_get_object_directory(the_repository), name);\n+\t\t\t\t       repo_get_object_directory(r), name);\n \t\treturn NULL;\n \t}\n \tif (is_well_formed)\ndiff --git a/pack.h b/pack.h\nindex 6d9d477adc83e83d9e9175ccf699c100b4c147c6..46d85e5bec787c90af69700fd4b328b1ebf1d606 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -94,7 +94,7 @@ off_t write_pack_header(struct hashfile *f, uint32_t);\n void fixup_pack_header_footer(const struct git_hash_algo *, int,\n \t\t\t      unsigned char *, const char *, uint32_t,\n \t\t\t      unsigned char *, off_t);\n-char *index_pack_lockfile(int fd, int *is_well_formed);\n+char *index_pack_lockfile(struct repository *r, int fd, int *is_well_formed);\n \n struct ref;\n \n\n-- \n2.47.0\n\n"},{"id":"510897","messageId":"20250119-kn-the-repo-cleanup-v3-3-a495fce08d71@gmail.com","threadId":"62817","inReplyTo":"20250119-kn-the-repo-cleanup-v3-0-a495fce08d71@gmail.com","subject":"[PATCH v3 3/5] pack-write: pass hash_algo to `write_idx_file()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:19:28Z","receivedAt":"2025-01-19T11:19:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `write_idx_file()` function uses the global `the_hash_algo` variable\nto access the repository's hash_algo. To avoid global variable usage,\npass a hash_algo from the layers above.\n\nSince `stage_tmp_packfiles()` also resides in 'pack-write.c' and calls\n`write_idx_file()`, update it to accept a `struct git_hash_algo` as a\nparameter and pass it through to the callee.\n\nAltough the layers above could have access to the hash_algo internally,\nsimply pass in `the_hash_algo`. This avoids any compatibility issues and\nbubbles up global variable usage to upper layers which can be eventually\nresolved.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/fast-import.c  |  4 ++--\n builtin/index-pack.c   |  3 ++-\n builtin/pack-objects.c |  7 ++++---\n bulk-checkin.c         |  5 +++--\n pack-write.c           | 14 ++++++++------\n pack.h                 | 10 ++++++++--\n 6 files changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 6baf2b1b71e2443a7987a41e0cd246076682bf58..c4bc52f93c011f34bd7f98c8e9d74a33cf9783bd 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -798,8 +798,8 @@ static const char *create_index(void)\n \tif (c != last)\n \t\tdie(\"internal consistency error creating the index\");\n \n-\ttmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts,\n-\t\t\t\t pack_data->hash);\n+\ttmpfile = write_idx_file(the_hash_algo, NULL, idx, object_count,\n+\t\t\t\t &pack_idx_opts, pack_data->hash);\n \tfree(idx);\n \treturn tmpfile;\n }\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 6c5e3483f4fe67fb2e26132c55b1f8395d60c11f..d73699653a2227f8ecfee2c0f51cd680093ac764 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -2096,7 +2096,8 @@ int cmd_index_pack(int argc,\n \tALLOC_ARRAY(idx_objects, nr_objects);\n \tfor (i = 0; i < nr_objects; i++)\n \t\tidx_objects[i] = &objects[i].idx;\n-\tcurr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_hash);\n+\tcurr_index = write_idx_file(the_hash_algo, index_name, idx_objects,\n+\t\t\t\t    nr_objects, &opts, pack_hash);\n \tif (rev_index)\n \t\tcurr_rev_index = write_rev_file(rev_index_name, idx_objects,\n \t\t\t\t\t\tnr_objects, pack_hash,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex ffc62930b68c9b4152057572ede216381a4b0991..7b2dacda514c29f5393b604f15d379abb81546c0 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1369,9 +1369,10 @@ static void write_pack_file(void)\n \t\t\tif (cruft)\n \t\t\t\tpack_idx_opts.flags |= WRITE_MTIMES;\n \n-\t\t\tstage_tmp_packfiles(&tmpname, pack_tmp_name,\n-\t\t\t\t\t    written_list, nr_written,\n-\t\t\t\t\t    &to_pack, &pack_idx_opts, hash,\n+\t\t\tstage_tmp_packfiles(the_hash_algo, &tmpname,\n+\t\t\t\t\t    pack_tmp_name, written_list,\n+\t\t\t\t\t    nr_written, &to_pack,\n+\t\t\t\t\t    &pack_idx_opts, hash,\n \t\t\t\t\t    &idx_tmp_name);\n \n \t\t\tif (write_bitmap_index) {\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex c4b085f57f74fb8b998576ac9d84fed9e01ed0ed..0d49889bfbb233d58fc094f5e2c2d2433dad9851 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -44,8 +44,9 @@ static void finish_tmp_packfile(struct strbuf *basename,\n {\n \tchar *idx_tmp_name = NULL;\n \n-\tstage_tmp_packfiles(basename, pack_tmp_name, written_list, nr_written,\n-\t\t\t    NULL, pack_idx_opts, hash, &idx_tmp_name);\n+\tstage_tmp_packfiles(the_hash_algo, basename, pack_tmp_name,\n+\t\t\t    written_list, nr_written, NULL, pack_idx_opts, hash,\n+\t\t\t    &idx_tmp_name);\n \trename_tmp_packfile_idx(basename, &idx_tmp_name);\n \n \tfree(idx_tmp_name);\ndiff --git a/pack-write.c b/pack-write.c\nindex 0cd75d2e55419362a61cf981fc11117ea7a1d88a..f344e78a9ec20cea9812a5eaffc72ae0b7e7424d 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -56,7 +56,8 @@ static int need_large_offset(off_t offset, const struct pack_idx_option *opts)\n  * The *sha1 contains the pack content SHA1 hash.\n  * The objects array passed in will be sorted by SHA1 on exit.\n  */\n-const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects,\n+const char *write_idx_file(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *index_name, struct pack_idx_entry **objects,\n \t\t\t   int nr_objects, const struct pack_idx_option *opts,\n \t\t\t   const unsigned char *sha1)\n {\n@@ -130,7 +131,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec\n \t\tstruct pack_idx_entry *obj = *list++;\n \t\tif (index_version < 2)\n \t\t\thashwrite_be32(f, obj->offset);\n-\t\thashwrite(f, obj->oid.hash, the_hash_algo->rawsz);\n+\t\thashwrite(f, obj->oid.hash, hash_algo->rawsz);\n \t\tif ((opts->flags & WRITE_IDX_STRICT) &&\n \t\t    (i && oideq(&list[-2]->oid, &obj->oid)))\n \t\t\tdie(\"The same object %s appears twice in the pack\",\n@@ -172,7 +173,7 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec\n \t\t}\n \t}\n \n-\thashwrite(f, sha1, the_hash_algo->rawsz);\n+\thashwrite(f, sha1, hash_algo->rawsz);\n \tfinalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,\n \t\t\t  CSUM_HASH_IN_STREAM | CSUM_CLOSE |\n \t\t\t  ((opts->flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));\n@@ -546,7 +547,8 @@ void rename_tmp_packfile_idx(struct strbuf *name_buffer,\n \trename_tmp_packfile(name_buffer, *idx_tmp_name, \"idx\");\n }\n \n-void stage_tmp_packfiles(struct strbuf *name_buffer,\n+void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n+\t\t\t struct strbuf *name_buffer,\n \t\t\t const char *pack_tmp_name,\n \t\t\t struct pack_idx_entry **written_list,\n \t\t\t uint32_t nr_written,\n@@ -561,8 +563,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,\n \tif (adjust_shared_perm(pack_tmp_name))\n \t\tdie_errno(\"unable to make temporary pack file readable\");\n \n-\t*idx_tmp_name = (char *)write_idx_file(NULL, written_list, nr_written,\n-\t\t\t\t\t       pack_idx_opts, hash);\n+\t*idx_tmp_name = (char *)write_idx_file(hash_algo, NULL, written_list,\n+\t\t\t\t\t       nr_written, pack_idx_opts, hash);\n \tif (adjust_shared_perm(*idx_tmp_name))\n \t\tdie_errno(\"unable to make temporary index file readable\");\n \ndiff --git a/pack.h b/pack.h\nindex 46d85e5bec787c90af69700fd4b328b1ebf1d606..c650fdbe2dcde8055ad0efe55646338cd0f81df5 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -86,7 +86,12 @@ struct progress;\n /* Note, the data argument could be NULL if object type is blob */\n typedef int (*verify_fn)(const struct object_id *, enum object_type, unsigned long, void*, int*);\n \n-const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, const unsigned char *sha1);\n+const char *write_idx_file(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *index_name,\n+\t\t\t   struct pack_idx_entry **objects,\n+\t\t\t   int nr_objects,\n+\t\t\t   const struct pack_idx_option *,\n+\t\t\t   const unsigned char *sha1);\n int check_pack_crc(struct packed_git *p, struct pack_window **w_curs, off_t offset, off_t len, unsigned int nr);\n int verify_pack_index(struct packed_git *);\n int verify_pack(struct repository *, struct packed_git *, verify_fn fn, struct progress *, uint32_t);\n@@ -119,7 +124,8 @@ int read_pack_header(int fd, struct pack_header *);\n struct packing_data;\n \n struct hashfile *create_tmp_packfile(char **pack_tmp_name);\n-void stage_tmp_packfiles(struct strbuf *name_buffer,\n+void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n+\t\t\t struct strbuf *name_buffer,\n \t\t\t const char *pack_tmp_name,\n \t\t\t struct pack_idx_entry **written_list,\n \t\t\t uint32_t nr_written,\n\n-- \n2.47.0\n\n"},{"id":"510898","messageId":"20250119-kn-the-repo-cleanup-v3-4-a495fce08d71@gmail.com","threadId":"62817","inReplyTo":"20250119-kn-the-repo-cleanup-v3-0-a495fce08d71@gmail.com","subject":"[PATCH v3 4/5] pack-write: pass hash_algo to `write_rev_file()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:19:29Z","receivedAt":"2025-01-19T11:19:47Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `write_rev_file()` function uses the global `the_hash_algo` variable\nto access the repository's hash_algo. To avoid global variable usage,\npass a hash_algo from the layers above. Also modify children functions\n`write_rev_file_order()` and `write_rev_header()` to accept\n'the_hash_algo'.\n\nAltough the layers above could have access to the hash_algo internally,\nsimply pass in `the_hash_algo`. This avoids any compatibility issues and\nbubbles up global variable usage to upper layers which can be eventually\nresolved.\n\nHowever, in `midx-write.c`, since all usage of global variables is\nremoved, don't reintroduce them and instead use the `repo` available in\nthe context.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n builtin/index-pack.c |  6 +++---\n midx-write.c         |  4 ++--\n pack-write.c         | 21 ++++++++++++---------\n pack.h               | 14 ++++++++++++--\n 4 files changed, 29 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex d73699653a2227f8ecfee2c0f51cd680093ac764..e803cb2444633f937fafc841848dae85341475f1 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -2099,9 +2099,9 @@ int cmd_index_pack(int argc,\n \tcurr_index = write_idx_file(the_hash_algo, index_name, idx_objects,\n \t\t\t\t    nr_objects, &opts, pack_hash);\n \tif (rev_index)\n-\t\tcurr_rev_index = write_rev_file(rev_index_name, idx_objects,\n-\t\t\t\t\t\tnr_objects, pack_hash,\n-\t\t\t\t\t\topts.flags);\n+\t\tcurr_rev_index = write_rev_file(the_hash_algo, rev_index_name,\n+\t\t\t\t\t\tidx_objects, nr_objects,\n+\t\t\t\t\t\tpack_hash, opts.flags);\n \tfree(idx_objects);\n \n \tif (!verify)\ndiff --git a/midx-write.c b/midx-write.c\nindex b3827b936bdb1df12c73fb7d9b98ff65fc875cc3..61b59d557d3ba46a39db74c62393545cabf50f2c 100644\n--- a/midx-write.c\n+++ b/midx-write.c\n@@ -658,8 +658,8 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash,\n \tstrbuf_addf(&buf, \"%s-%s.rev\", midx_name, hash_to_hex_algop(midx_hash,\n \t\t\t\t\t\t\t\t    ctx->repo->hash_algo));\n \n-\ttmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr,\n-\t\t\t\t\tmidx_hash, WRITE_REV);\n+\ttmp_file = write_rev_file_order(ctx->repo->hash_algo, NULL, ctx->pack_order,\n+\t\t\t\t\tctx->entries_nr, midx_hash, WRITE_REV);\n \n \tif (finalize_object_file(tmp_file, buf.buf))\n \t\tdie(_(\"cannot store reverse index file\"));\ndiff --git a/pack-write.c b/pack-write.c\nindex f344e78a9ec20cea9812a5eaffc72ae0b7e7424d..09ecbcdb069cc9b0383295798ceb49cbdc632b64 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -194,11 +194,12 @@ static int pack_order_cmp(const void *va, const void *vb, void *ctx)\n \treturn 0;\n }\n \n-static void write_rev_header(struct hashfile *f)\n+static void write_rev_header(const struct git_hash_algo *hash_algo,\n+\t\t\t     struct hashfile *f)\n {\n \thashwrite_be32(f, RIDX_SIGNATURE);\n \thashwrite_be32(f, RIDX_VERSION);\n-\thashwrite_be32(f, oid_version(the_hash_algo));\n+\thashwrite_be32(f, oid_version(hash_algo));\n }\n \n static void write_rev_index_positions(struct hashfile *f,\n@@ -215,7 +216,8 @@ static void write_rev_trailer(struct hashfile *f, const unsigned char *hash)\n \thashwrite(f, hash, the_hash_algo->rawsz);\n }\n \n-char *write_rev_file(const char *rev_name,\n+char *write_rev_file(const struct git_hash_algo *hash_algo,\n+\t\t     const char *rev_name,\n \t\t     struct pack_idx_entry **objects,\n \t\t     uint32_t nr_objects,\n \t\t     const unsigned char *hash,\n@@ -233,15 +235,16 @@ char *write_rev_file(const char *rev_name,\n \t\tpack_order[i] = i;\n \tQSORT_S(pack_order, nr_objects, pack_order_cmp, objects);\n \n-\tret = write_rev_file_order(rev_name, pack_order, nr_objects, hash,\n-\t\t\t\t   flags);\n+\tret = write_rev_file_order(hash_algo, rev_name, pack_order, nr_objects,\n+\t\t\t\t   hash, flags);\n \n \tfree(pack_order);\n \n \treturn ret;\n }\n \n-char *write_rev_file_order(const char *rev_name,\n+char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *rev_name,\n \t\t\t   uint32_t *pack_order,\n \t\t\t   uint32_t nr_objects,\n \t\t\t   const unsigned char *hash,\n@@ -280,7 +283,7 @@ char *write_rev_file_order(const char *rev_name,\n \t\treturn NULL;\n \t}\n \n-\twrite_rev_header(f);\n+\twrite_rev_header(hash_algo, f);\n \n \twrite_rev_index_positions(f, pack_order, nr_objects);\n \twrite_rev_trailer(f, hash);\n@@ -568,8 +571,8 @@ void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n \tif (adjust_shared_perm(*idx_tmp_name))\n \t\tdie_errno(\"unable to make temporary index file readable\");\n \n-\trev_tmp_name = write_rev_file(NULL, written_list, nr_written, hash,\n-\t\t\t\t      pack_idx_opts->flags);\n+\trev_tmp_name = write_rev_file(hash_algo, NULL, written_list, nr_written,\n+\t\t\t\t      hash, pack_idx_opts->flags);\n \n \tif (pack_idx_opts->flags & WRITE_MTIMES) {\n \t\tmtimes_tmp_name = write_mtimes_file(to_pack, written_list,\ndiff --git a/pack.h b/pack.h\nindex c650fdbe2dcde8055ad0efe55646338cd0f81df5..8a84ea475cda902936ee0b6091c5b4d462606be8 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -105,8 +105,18 @@ struct ref;\n \n void write_promisor_file(const char *promisor_name, struct ref **sought, int nr_sought);\n \n-char *write_rev_file(const char *rev_name, struct pack_idx_entry **objects, uint32_t nr_objects, const unsigned char *hash, unsigned flags);\n-char *write_rev_file_order(const char *rev_name, uint32_t *pack_order, uint32_t nr_objects, const unsigned char *hash, unsigned flags);\n+char *write_rev_file(const struct git_hash_algo *hash_algo,\n+\t\t     const char *rev_name,\n+\t\t     struct pack_idx_entry **objects,\n+\t\t     uint32_t nr_objects,\n+\t\t     const unsigned char *hash,\n+\t\t     unsigned flags);\n+char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n+\t\t\t   const char *rev_name,\n+\t\t\t   uint32_t *pack_order,\n+\t\t\t   uint32_t nr_objects,\n+\t\t\t   const unsigned char *hash,\n+\t\t\t   unsigned flags);\n \n /*\n  * The \"hdr\" output buffer should be at least this big, which will handle sizes\n\n-- \n2.47.0\n\n"},{"id":"510899","messageId":"20250119-kn-the-repo-cleanup-v3-5-a495fce08d71@gmail.com","threadId":"62817","inReplyTo":"20250119-kn-the-repo-cleanup-v3-0-a495fce08d71@gmail.com","subject":"[PATCH v3 5/5] pack-write: pass hash_algo to internal functions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-01-19T11:19:30Z","receivedAt":"2025-01-19T11:19:50Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The internal functions `write_rev_trailer()`, `write_rev_trailer()`,\n`write_mtimes_header()` and write_mtimes_trailer()` use the global\n`the_hash_algo` variable to access the repository's hash function. Pass\nthe hash_algo down from callers, all of which already have access to the\nvariable.\n\nThis removes all global variables from the 'pack-write.c' file, so\nremove the 'USE_THE_REPOSITORY_VARIABLE' macro.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n pack-write.c | 30 ++++++++++++++++--------------\n 1 file changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/pack-write.c b/pack-write.c\nindex 09ecbcdb069cc9b0383295798ceb49cbdc632b64..a2faeb1895e41f4c17281380478f1f2cabcc6f24 100644\n--- a/pack-write.c\n+++ b/pack-write.c\n@@ -1,5 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n-\n #include \"git-compat-util.h\"\n #include \"environment.h\"\n #include \"gettext.h\"\n@@ -211,9 +209,10 @@ static void write_rev_index_positions(struct hashfile *f,\n \t\thashwrite_be32(f, pack_order[i]);\n }\n \n-static void write_rev_trailer(struct hashfile *f, const unsigned char *hash)\n+static void write_rev_trailer(const struct git_hash_algo *hash_algo,\n+\t\t\t      struct hashfile *f, const unsigned char *hash)\n {\n-\thashwrite(f, hash, the_hash_algo->rawsz);\n+\thashwrite(f, hash, hash_algo->rawsz);\n }\n \n char *write_rev_file(const struct git_hash_algo *hash_algo,\n@@ -286,7 +285,7 @@ char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n \twrite_rev_header(hash_algo, f);\n \n \twrite_rev_index_positions(f, pack_order, nr_objects);\n-\twrite_rev_trailer(f, hash);\n+\twrite_rev_trailer(hash_algo, f, hash);\n \n \tif (adjust_shared_perm(path) < 0)\n \t\tdie(_(\"failed to make %s readable\"), path);\n@@ -298,11 +297,12 @@ char *write_rev_file_order(const struct git_hash_algo *hash_algo,\n \treturn path;\n }\n \n-static void write_mtimes_header(struct hashfile *f)\n+static void write_mtimes_header(const struct git_hash_algo *hash_algo,\n+\t\t\t\tstruct hashfile *f)\n {\n \thashwrite_be32(f, MTIMES_SIGNATURE);\n \thashwrite_be32(f, MTIMES_VERSION);\n-\thashwrite_be32(f, oid_version(the_hash_algo));\n+\thashwrite_be32(f, oid_version(hash_algo));\n }\n \n /*\n@@ -322,12 +322,14 @@ static void write_mtimes_objects(struct hashfile *f,\n \t}\n }\n \n-static void write_mtimes_trailer(struct hashfile *f, const unsigned char *hash)\n+static void write_mtimes_trailer(const struct git_hash_algo *hash_algo,\n+\t\t\t\t struct hashfile *f, const unsigned char *hash)\n {\n-\thashwrite(f, hash, the_hash_algo->rawsz);\n+\thashwrite(f, hash, hash_algo->rawsz);\n }\n \n-static char *write_mtimes_file(struct packing_data *to_pack,\n+static char *write_mtimes_file(const struct git_hash_algo *hash_algo,\n+\t\t\t       struct packing_data *to_pack,\n \t\t\t       struct pack_idx_entry **objects,\n \t\t\t       uint32_t nr_objects,\n \t\t\t       const unsigned char *hash)\n@@ -344,9 +346,9 @@ static char *write_mtimes_file(struct packing_data *to_pack,\n \tmtimes_name = strbuf_detach(&tmp_file, NULL);\n \tf = hashfd(fd, mtimes_name);\n \n-\twrite_mtimes_header(f);\n+\twrite_mtimes_header(hash_algo, f);\n \twrite_mtimes_objects(f, to_pack, objects, nr_objects);\n-\twrite_mtimes_trailer(f, hash);\n+\twrite_mtimes_trailer(hash_algo, f, hash);\n \n \tif (adjust_shared_perm(mtimes_name) < 0)\n \t\tdie(_(\"failed to make %s readable\"), mtimes_name);\n@@ -575,8 +577,8 @@ void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,\n \t\t\t\t      hash, pack_idx_opts->flags);\n \n \tif (pack_idx_opts->flags & WRITE_MTIMES) {\n-\t\tmtimes_tmp_name = write_mtimes_file(to_pack, written_list,\n-\t\t\t\t\t\t    nr_written,\n+\t\tmtimes_tmp_name = write_mtimes_file(hash_algo, to_pack,\n+\t\t\t\t\t\t    written_list, nr_written,\n \t\t\t\t\t\t    hash);\n \t}\n \n\n-- \n2.47.0\n\n"},{"id":"511139","messageId":"Z5MpOelecT-4ym7B@pks.im","threadId":"62817","inReplyTo":"20250119-kn-the-repo-cleanup-v3-0-a495fce08d71@gmail.com","subject":"Re: [PATCH v3 0/5] pack-write: cleanup usage of global variables","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-01-24T05:46:33Z","receivedAt":"2025-01-24T05:46:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Jan 19, 2025 at 12:19:25PM +0100, Karthik Nayak wrote:\n> This is a small series to remove global variable usage from\n> `pack-write.c`. Mostly it bubble's up the usage of global variables to\n> upper layers. The only exception is in `write-midx.c`, which was cleaned\n> of global variable usage, so there, we use the repo that is in available\n> in the context.\n> \n> This series is based on fbe8d3079d (Git 2.48, 2025-01-10) with\n> 'ps/more-sign-compare' and 'ps/the-repository' merged in.\n> \n> There are no conflicts with topics in 'next', however there is a\n> conflict with 'tb/incremental-midx-part-2' in 'seen', the fix is simple\n> but happy to merge that in too if necessary.\n\nThanks, this version looks good to me.\n\nPatrick\n"},{"id":"511169","messageId":"xmqq1pwsqkn1.fsf@gitster.g","threadId":"62817","inReplyTo":"Z5MpOelecT-4ym7B@pks.im","subject":"Re: [PATCH v3 0/5] pack-write: cleanup usage of global variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-24T15:47:14Z","receivedAt":"2025-01-24T15:47:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Sun, Jan 19, 2025 at 12:19:25PM +0100, Karthik Nayak wrote:\n>> This is a small series to remove global variable usage from\n>> `pack-write.c`. Mostly it bubble's up the usage of global variables to\n>> upper layers. The only exception is in `write-midx.c`, which was cleaned\n>> of global variable usage, so there, we use the repo that is in available\n>> in the context.\n>> \n>> This series is based on fbe8d3079d (Git 2.48, 2025-01-10) with\n>> 'ps/more-sign-compare' and 'ps/the-repository' merged in.\n>> \n>> There are no conflicts with topics in 'next', however there is a\n>> conflict with 'tb/incremental-midx-part-2' in 'seen', the fix is simple\n>> but happy to merge that in too if necessary.\n>\n> Thanks, this version looks good to me.\n\nThanks, both of you.  Let me mark it for 'next' then.\n"}]}