From d1fdd6996a6ee94534905fd993f3472ad318611d Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Sat, 26 Oct 2024 18:05:50 +0200 Subject: [PATCH 01/34] packfile: add repository to struct `packed_git` The struct `packed_git` holds information regarding a packed object file. Let's add the repository variable to this object, to represent the repository that this packfile belongs to. This helps remove dependency on the global `the_repository` object in `packfile.c` by simply using repository information now readily available in the struct. We do need to consider that a pack file could be part of the alternates of a repository, but considering that we only have one repository struct and also that we currently anyways use 'the_repository'. We should be OK with this change. We also modify `alloc_packed_git` to ensure that the repository is added to newly created `packed_git` structs. This requires modifying the function and all its callee to pass the repository object down the levels. Helped-by: Taylor Blau Signed-off-by: Karthik Nayak --- builtin/fast-import.c | 3 ++- builtin/index-pack.c | 6 ++++-- commit-graph.c | 2 +- connected.c | 3 ++- http.c | 2 +- midx-write.c | 2 +- midx.c | 2 +- object-store-ll.h | 5 +++++ packfile.c | 15 +++++++++------ packfile.h | 6 ++++-- 10 files changed, 30 insertions(+), 16 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 76d5c20f141..da7e2d613b7 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -765,6 +765,7 @@ static void start_packfile(void) p->pack_fd = pack_fd; p->do_not_close = 1; + p->repo = the_repository; pack_file = hashfd(pack_fd, p->pack_name); pack_data = p; @@ -888,7 +889,7 @@ static void end_packfile(void) idx_name = keep_pack(create_index()); /* Register the packfile with core git's machinery. */ - new_p = add_packed_git(idx_name, strlen(idx_name), 1); + new_p = add_packed_git(pack_data->repo, idx_name, strlen(idx_name), 1); if (!new_p) die("core git rejected index %s", idx_name); all_packs[pack_id] = new_p; diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 9d23b41b3a2..be2f99625ea 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1552,7 +1552,8 @@ static void final(const char *final_pack_name, const char *curr_pack_name, if (do_fsck_object) { struct packed_git *p; - p = add_packed_git(final_index_name, strlen(final_index_name), 0); + p = add_packed_git(the_repository, final_index_name, + strlen(final_index_name), 0); if (p) install_packed_git(the_repository, p); } @@ -1650,7 +1651,8 @@ static void read_v2_anomalous_offsets(struct packed_git *p, static void read_idx_option(struct pack_idx_option *opts, const char *pack_name) { - struct packed_git *p = add_packed_git(pack_name, strlen(pack_name), 1); + struct packed_git *p = add_packed_git(the_repository, pack_name, + strlen(pack_name), 1); if (!p) die(_("Cannot open existing pack file '%s'"), pack_name); diff --git a/commit-graph.c b/commit-graph.c index 5bd89c0acdd..83dd69bfebc 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1914,7 +1914,7 @@ static int fill_oids_from_packs(struct write_commit_graph_context *ctx, struct packed_git *p; strbuf_setlen(&packname, dirlen); strbuf_addstr(&packname, pack_indexes->items[i].string); - p = add_packed_git(packname.buf, packname.len, 1); + p = add_packed_git(ctx->r, packname.buf, packname.len, 1); if (!p) { ret = error(_("error adding pack %s"), packname.buf); goto cleanup; diff --git a/connected.c b/connected.c index a9e2e139956..3099da84f33 100644 --- a/connected.c +++ b/connected.c @@ -54,7 +54,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data, strbuf_add(&idx_file, transport->pack_lockfiles.items[0].string, base_len); strbuf_addstr(&idx_file, ".idx"); - new_pack = add_packed_git(idx_file.buf, idx_file.len, 1); + new_pack = add_packed_git(the_repository, idx_file.buf, + idx_file.len, 1); strbuf_release(&idx_file); } diff --git a/http.c b/http.c index 58242b9d2dd..6744e184092 100644 --- a/http.c +++ b/http.c @@ -2439,7 +2439,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head, if (!tmp_idx) return -1; - new_pack = parse_pack_index(sha1, tmp_idx); + new_pack = parse_pack_index(the_repository, sha1, tmp_idx); if (!new_pack) { unlink(tmp_idx); free(tmp_idx); diff --git a/midx-write.c b/midx-write.c index b3a5f6c5166..c57726ef947 100644 --- a/midx-write.c +++ b/midx-write.c @@ -154,7 +154,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len, return; ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc); - p = add_packed_git(full_path, full_path_len, 0); + p = add_packed_git(the_repository, full_path, full_path_len, 0); if (!p) { warning(_("failed to add packfile '%s'"), full_path); diff --git a/midx.c b/midx.c index e82d4f2e654..8edb75f51dc 100644 --- a/midx.c +++ b/midx.c @@ -464,7 +464,7 @@ int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, strhash(key.buf), key.buf, struct packed_git, packmap_ent); if (!p) { - p = add_packed_git(pack_name.buf, pack_name.len, m->local); + p = add_packed_git(r, pack_name.buf, pack_name.len, m->local); if (p) { install_packed_git(r, p); list_add_tail(&p->mru, &r->objects->packed_git_mru); diff --git a/object-store-ll.h b/object-store-ll.h index 53b8e693b1b..d46cd0e6545 100644 --- a/object-store-ll.h +++ b/object-store-ll.h @@ -10,6 +10,7 @@ struct oidmap; struct oidtree; struct strbuf; +struct repository; struct object_directory { struct object_directory *next; @@ -135,6 +136,10 @@ struct packed_git { */ const uint32_t *mtimes_map; size_t mtimes_size; + + /* repo denotes the repository this packfile belongs to */ + struct repository *repo; + /* something like ".git/objects/pack/xxxxx.pack" */ char pack_name[FLEX_ARRAY]; /* more */ }; diff --git a/packfile.c b/packfile.c index 9560f0a33c6..6058eddf358 100644 --- a/packfile.c +++ b/packfile.c @@ -217,11 +217,12 @@ uint32_t get_pack_fanout(struct packed_git *p, uint32_t value) return ntohl(level1_ofs[value]); } -static struct packed_git *alloc_packed_git(int extra) +static struct packed_git *alloc_packed_git(struct repository *r, int extra) { struct packed_git *p = xmalloc(st_add(sizeof(*p), extra)); memset(p, 0, sizeof(*p)); p->pack_fd = -1; + p->repo = r; return p; } @@ -233,11 +234,12 @@ static char *pack_path_from_idx(const char *idx_path) return xstrfmt("%.*s.pack", (int)len, idx_path); } -struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path) +struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1, + const char *idx_path) { char *path = pack_path_from_idx(idx_path); size_t alloc = st_add(strlen(path), 1); - struct packed_git *p = alloc_packed_git(alloc); + struct packed_git *p = alloc_packed_git(r, alloc); memcpy(p->pack_name, path, alloc); /* includes NUL */ free(path); @@ -703,7 +705,8 @@ void unuse_pack(struct pack_window **w_cursor) } } -struct packed_git *add_packed_git(const char *path, size_t path_len, int local) +struct packed_git *add_packed_git(struct repository *r, const char *path, + size_t path_len, int local) { struct stat st; size_t alloc; @@ -721,7 +724,7 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local) * the use xsnprintf double-checks that) */ alloc = st_add3(path_len, strlen(".promisor"), 1); - p = alloc_packed_git(alloc); + p = alloc_packed_git(r, alloc); memcpy(p->pack_name, path, path_len); xsnprintf(p->pack_name + path_len, alloc - path_len, ".keep"); @@ -877,7 +880,7 @@ static void prepare_pack(const char *full_name, size_t full_name_len, /* Don't reopen a pack we already have. */ if (!hashmap_get(&data->r->objects->pack_map, &hent, pack_name)) { - p = add_packed_git(full_name, full_name_len, data->local); + p = add_packed_git(data->r, full_name, full_name_len, data->local); if (p) install_packed_git(data->r, p); } diff --git a/packfile.h b/packfile.h index 08f88a7ff52..aee69d1a0b6 100644 --- a/packfile.h +++ b/packfile.h @@ -46,7 +46,8 @@ const char *pack_basename(struct packed_git *p); * and does not add the resulting packed_git struct to the internal list of * packs. You probably want add_packed_git() instead. */ -struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path); +struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1, + const char *idx_path); typedef void each_file_in_pack_dir_fn(const char *full_path, size_t full_path_len, const char *file_name, void *data); @@ -113,7 +114,8 @@ void close_pack(struct packed_git *); void close_object_store(struct raw_object_store *o); void unuse_pack(struct pack_window **); void clear_delta_base_cache(void); -struct packed_git *add_packed_git(const char *path, size_t path_len, int local); +struct packed_git *add_packed_git(struct repository *r, const char *path, + size_t path_len, int local); /* * Unlink the .pack and associated extension files. -- GitLab From 65c09858ceffb8a530ae66c6dd460200cd24c3ca Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Sat, 26 Oct 2024 18:33:44 +0200 Subject: [PATCH 02/34] packfile: use `repository` from `packed_git` directly In the previous commit, we introduced the `repository` structure inside `packed_git`. This provides an alternative route instead of using the global `the_repository` variable. Let's modify `packfile.c` now to use this field wherever possible instead of relying on the global state. There are still a few instances of `the_repository` usage in the file, where there is no struct `packed_git` locally available, which will be fixed in the following commits. Helped-by: Taylor Blau Signed-off-by: Karthik Nayak --- packfile.c | 50 +++++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/packfile.c b/packfile.c index 6058eddf358..5bfa1e17c2e 100644 --- a/packfile.c +++ b/packfile.c @@ -79,7 +79,7 @@ static int check_packed_git_idx(const char *path, struct packed_git *p) size_t idx_size; int fd = git_open(path), ret; struct stat st; - const unsigned int hashsz = the_hash_algo->rawsz; + const unsigned int hashsz = p->repo->hash_algo->rawsz; if (fd < 0) return -1; @@ -243,7 +243,7 @@ struct packed_git *parse_pack_index(struct repository *r, unsigned char *sha1, memcpy(p->pack_name, path, alloc); /* includes NUL */ free(path); - hashcpy(p->hash, sha1, the_repository->hash_algo); + hashcpy(p->hash, sha1, p->repo->hash_algo); if (check_packed_git_idx(idx_path, p)) { free(p); return NULL; @@ -278,7 +278,7 @@ static int unuse_one_window(struct packed_git *current) if (current) scan_windows(current, &lru_p, &lru_w, &lru_l); - for (p = the_repository->objects->packed_git; p; p = p->next) + for (p = current->repo->objects->packed_git; p; p = p->next) scan_windows(p, &lru_p, &lru_w, &lru_l); if (lru_p) { munmap(lru_w->base, lru_w->len); @@ -540,7 +540,7 @@ static int open_packed_git_1(struct packed_git *p) unsigned char hash[GIT_MAX_RAWSZ]; unsigned char *idx_hash; ssize_t read_result; - const unsigned hashsz = the_hash_algo->rawsz; + const unsigned hashsz = p->repo->hash_algo->rawsz; if (open_pack_index(p)) return error("packfile %s index unavailable", p->pack_name); @@ -597,7 +597,7 @@ static int open_packed_git_1(struct packed_git *p) if (read_result != hashsz) return error("packfile %s signature is unavailable", p->pack_name); idx_hash = ((unsigned char *)p->index_data) + p->index_size - hashsz * 2; - if (!hasheq(hash, idx_hash, the_repository->hash_algo)) + if (!hasheq(hash, idx_hash, p->repo->hash_algo)) return error("packfile %s does not match index", p->pack_name); return 0; } @@ -637,7 +637,7 @@ unsigned char *use_pack(struct packed_git *p, */ if (!p->pack_size && p->pack_fd == -1 && open_packed_git(p)) die("packfile %s cannot be accessed", p->pack_name); - if (offset > (p->pack_size - the_hash_algo->rawsz)) + if (offset > (p->pack_size - p->repo->hash_algo->rawsz)) die("offset beyond end of packfile (truncated pack?)"); if (offset < 0) die(_("offset before end of packfile (broken .idx?)")); @@ -711,6 +711,7 @@ struct packed_git *add_packed_git(struct repository *r, const char *path, struct stat st; size_t alloc; struct packed_git *p; + struct object_id oid; /* * Make sure a corresponding .pack file exists and that @@ -751,9 +752,13 @@ struct packed_git *add_packed_git(struct repository *r, const char *path, p->pack_size = st.st_size; p->pack_local = local; p->mtime = st.st_mtime; - if (path_len < the_hash_algo->hexsz || - get_hash_hex(path + path_len - the_hash_algo->hexsz, p->hash)) - hashclr(p->hash, the_repository->hash_algo); + if (path_len < r->hash_algo->hexsz || + get_oid_hex_algop(path + path_len - r->hash_algo->hexsz, &oid, + r->hash_algo)) + hashclr(p->hash, r->hash_algo); + else + hashcpy(p->hash, oid.hash, r->hash_algo); + return p; } @@ -1243,9 +1248,9 @@ off_t get_delta_base(struct packed_git *p, } else if (type == OBJ_REF_DELTA) { /* The base entry _must_ be in the same pack */ struct object_id oid; - oidread(&oid, base_info, the_repository->hash_algo); + oidread(&oid, base_info, p->repo->hash_algo); base_offset = find_pack_entry_one(&oid, p); - *curpos += the_hash_algo->rawsz; + *curpos += p->repo->hash_algo->rawsz; } else die("I am totally screwed"); return base_offset; @@ -1266,7 +1271,7 @@ static int get_delta_base_oid(struct packed_git *p, { if (type == OBJ_REF_DELTA) { unsigned char *base = use_pack(p, w_curs, curpos, NULL); - oidread(oid, base, the_repository->hash_algo); + oidread(oid, base, p->repo->hash_algo); return 0; } else if (type == OBJ_OFS_DELTA) { uint32_t base_pos; @@ -1608,7 +1613,7 @@ int packed_object_info(struct repository *r, struct packed_git *p, goto out; } } else - oidclr(oi->delta_base_oid, the_repository->hash_algo); + oidclr(oi->delta_base_oid, p->repo->hash_algo); } oi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED : @@ -1897,7 +1902,7 @@ int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32 { const unsigned char *index_fanout = p->index_data; const unsigned char *index_lookup; - const unsigned int hashsz = the_hash_algo->rawsz; + const unsigned int hashsz = p->repo->hash_algo->rawsz; int index_lookup_width; if (!index_fanout) @@ -1922,7 +1927,7 @@ int nth_packed_object_id(struct object_id *oid, uint32_t n) { const unsigned char *index = p->index_data; - const unsigned int hashsz = the_hash_algo->rawsz; + const unsigned int hashsz = p->repo->hash_algo->rawsz; if (!index) { if (open_pack_index(p)) return -1; @@ -1933,11 +1938,10 @@ int nth_packed_object_id(struct object_id *oid, index += 4 * 256; if (p->index_version == 1) { oidread(oid, index + st_add(st_mult(hashsz + 4, n), 4), - the_repository->hash_algo); + p->repo->hash_algo); } else { index += 8; - oidread(oid, index + st_mult(hashsz, n), - the_repository->hash_algo); + oidread(oid, index + st_mult(hashsz, n), p->repo->hash_algo); } return 0; } @@ -1959,7 +1963,7 @@ void check_pack_index_ptr(const struct packed_git *p, const void *vptr) off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n) { const unsigned char *index = p->index_data; - const unsigned int hashsz = the_hash_algo->rawsz; + const unsigned int hashsz = p->repo->hash_algo->rawsz; index += 4 * 256; if (p->index_version == 1) { return ntohl(*((uint32_t *)(index + st_mult(hashsz + 4, n)))); @@ -2159,7 +2163,7 @@ int for_each_object_in_pack(struct packed_git *p, int r = 0; if (flags & FOR_EACH_OBJECT_PACK_ORDER) { - if (load_pack_revindex(the_repository, p)) + if (load_pack_revindex(p->repo, p)) return -1; } @@ -2227,7 +2231,7 @@ int for_each_packed_object(each_packed_object_fn cb, void *data, } static int add_promisor_object(const struct object_id *oid, - struct packed_git *pack UNUSED, + struct packed_git *pack, uint32_t pos UNUSED, void *set_) { @@ -2235,12 +2239,12 @@ static int add_promisor_object(const struct object_id *oid, struct object *obj; int we_parsed_object; - obj = lookup_object(the_repository, oid); + obj = lookup_object(pack->repo, oid); if (obj && obj->parsed) { we_parsed_object = 0; } else { we_parsed_object = 1; - obj = parse_object(the_repository, oid); + obj = parse_object(pack->repo, oid); } if (!obj) -- GitLab From 80632934d135bc1a5cc2fca1a69f398b692a38ce Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Sat, 26 Oct 2024 18:50:56 +0200 Subject: [PATCH 03/34] packfile: pass `repository` to static function in the file Some of the static functions in the `packfile.c` access global variables, which can simply be avoiding by passing the `repository` struct down to them. Let's do that. Signed-off-by: Karthik Nayak --- packfile.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packfile.c b/packfile.c index 5bfa1e17c2e..c96ebc4c694 100644 --- a/packfile.c +++ b/packfile.c @@ -460,13 +460,13 @@ static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, struc *accept_windows_inuse = has_windows_inuse; } -static int close_one_pack(void) +static int close_one_pack(struct repository *r) { struct packed_git *p, *lru_p = NULL; struct pack_window *mru_w = NULL; int accept_windows_inuse = 1; - for (p = the_repository->objects->packed_git; p; p = p->next) { + for (p = r->objects->packed_git; p; p = p->next) { if (p->pack_fd == -1) continue; find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse); @@ -555,7 +555,7 @@ static int open_packed_git_1(struct packed_git *p) pack_max_fds = 1; } - while (pack_max_fds <= pack_open_fds && close_one_pack()) + while (pack_max_fds <= pack_open_fds && close_one_pack(p->repo)) ; /* nothing */ p->pack_fd = git_open(p->pack_name); @@ -610,7 +610,8 @@ static int open_packed_git(struct packed_git *p) return -1; } -static int in_window(struct pack_window *win, off_t offset) +static int in_window(struct repository *r, struct pack_window *win, + off_t offset) { /* We must promise at least one full hash after the * offset is available from this window, otherwise the offset @@ -620,7 +621,7 @@ static int in_window(struct pack_window *win, off_t offset) */ off_t win_off = win->offset; return win_off <= offset - && (offset + the_hash_algo->rawsz) <= (win_off + win->len); + && (offset + r->hash_algo->rawsz) <= (win_off + win->len); } unsigned char *use_pack(struct packed_git *p, @@ -642,11 +643,11 @@ unsigned char *use_pack(struct packed_git *p, if (offset < 0) die(_("offset before end of packfile (broken .idx?)")); - if (!win || !in_window(win, offset)) { + if (!win || !in_window(p->repo, win, offset)) { if (win) win->inuse_cnt--; for (win = p->windows; win; win = win->next) { - if (in_window(win, offset)) + if (in_window(p->repo, win, offset)) break; } if (!win) { -- GitLab From 67d71eab836ea5fd27a5af3394b51fbd593b4355 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Sat, 26 Oct 2024 18:44:58 +0200 Subject: [PATCH 04/34] packfile: pass down repository to `odb_pack_name` The function `odb_pack_name` currently relies on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Signed-off-by: Karthik Nayak --- builtin/fast-import.c | 8 ++++---- builtin/index-pack.c | 4 ++-- builtin/pack-redundant.c | 2 +- http.c | 2 +- packfile.c | 9 ++++----- packfile.h | 3 ++- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index da7e2d613b7..3ccc4c57227 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -806,7 +806,7 @@ static char *keep_pack(const char *curr_index_name) struct strbuf name = STRBUF_INIT; int keep_fd; - odb_pack_name(&name, pack_data->hash, "keep"); + odb_pack_name(pack_data->repo, &name, pack_data->hash, "keep"); keep_fd = odb_pack_keep(name.buf); if (keep_fd < 0) die_errno("cannot create keep file"); @@ -814,11 +814,11 @@ static char *keep_pack(const char *curr_index_name) if (close(keep_fd)) die_errno("failed to write keep file"); - odb_pack_name(&name, pack_data->hash, "pack"); + odb_pack_name(pack_data->repo, &name, pack_data->hash, "pack"); if (finalize_object_file(pack_data->pack_name, name.buf)) die("cannot store pack file"); - odb_pack_name(&name, pack_data->hash, "idx"); + odb_pack_name(pack_data->repo, &name, pack_data->hash, "idx"); if (finalize_object_file(curr_index_name, name.buf)) die("cannot store index file"); free((void *)curr_index_name); @@ -832,7 +832,7 @@ static void unkeep_all_packs(void) for (k = 0; k < pack_id; k++) { struct packed_git *p = all_packs[k]; - odb_pack_name(&name, p->hash, "keep"); + odb_pack_name(p->repo, &name, p->hash, "keep"); unlink_or_warn(name.buf); } strbuf_release(&name); diff --git a/builtin/index-pack.c b/builtin/index-pack.c index be2f99625ea..eaefb41761e 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1479,7 +1479,7 @@ static void write_special_file(const char *suffix, const char *msg, if (pack_name) filename = derive_filename(pack_name, "pack", suffix, &name_buf); else - filename = odb_pack_name(&name_buf, hash, suffix); + filename = odb_pack_name(the_repository, &name_buf, hash, suffix); fd = odb_pack_keep(filename); if (fd < 0) { @@ -1507,7 +1507,7 @@ static void rename_tmp_packfile(const char **final_name, { if (!*final_name || strcmp(*final_name, curr_name)) { if (!*final_name) - *final_name = odb_pack_name(name, hash, ext); + *final_name = odb_pack_name(the_repository, name, hash, ext); if (finalize_object_file(curr_name, *final_name)) die(_("unable to rename temporary '*.%s' file to '%s'"), ext, *final_name); diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c index d2c1c4e5ec1..bc61990a933 100644 --- a/builtin/pack-redundant.c +++ b/builtin/pack-redundant.c @@ -690,7 +690,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s pl = red = pack_list_difference(local_packs, min); while (pl) { printf("%s\n%s\n", - odb_pack_name(&idx_name, pl->pack->hash, "idx"), + odb_pack_name(pl->pack->repo, &idx_name, pl->pack->hash, "idx"), pl->pack->pack_name); pl = pl->next; } diff --git a/http.c b/http.c index 6744e184092..420f1566f00 100644 --- a/http.c +++ b/http.c @@ -2581,7 +2581,7 @@ struct http_pack_request *new_direct_http_pack_request( preq->url = url; - odb_pack_name(&preq->tmpfile, packed_git_hash, "pack"); + odb_pack_name(the_repository, &preq->tmpfile, packed_git_hash, "pack"); strbuf_addstr(&preq->tmpfile, ".temp"); preq->packfile = fopen(preq->tmpfile.buf, "a"); if (!preq->packfile) { diff --git a/packfile.c b/packfile.c index c96ebc4c694..1015dac6dbc 100644 --- a/packfile.c +++ b/packfile.c @@ -25,13 +25,12 @@ #include "pack-revindex.h" #include "promisor-remote.h" -char *odb_pack_name(struct strbuf *buf, - const unsigned char *hash, - const char *ext) +char *odb_pack_name(struct repository *r, struct strbuf *buf, + const unsigned char *hash, const char *ext) { strbuf_reset(buf); - strbuf_addf(buf, "%s/pack/pack-%s.%s", repo_get_object_directory(the_repository), - hash_to_hex(hash), ext); + strbuf_addf(buf, "%s/pack/pack-%s.%s", repo_get_object_directory(r), + hash_to_hex_algop(hash, r->hash_algo), ext); return buf->buf; } diff --git a/packfile.h b/packfile.h index aee69d1a0b6..51187f2393f 100644 --- a/packfile.h +++ b/packfile.h @@ -29,7 +29,8 @@ struct pack_entry { * * Example: odb_pack_name(out, sha1, "idx") => ".git/objects/pack/pack-1234..idx" */ -char *odb_pack_name(struct strbuf *buf, const unsigned char *sha1, const char *ext); +char *odb_pack_name(struct repository *r, struct strbuf *buf, + const unsigned char *hash, const char *ext); /* * Return the basename of the packfile, omitting any containing directory -- GitLab From ee210fa1537bf6de6e03713ac6362298ce121a92 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 14 Oct 2024 22:09:11 +0200 Subject: [PATCH 05/34] packfile: pass down repository to `has_object[_kept]_pack` The functions `has_object[_kept]_pack` currently rely on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Let's remove its usage from these functions and any related ones. Signed-off-by: Karthik Nayak --- builtin/count-objects.c | 2 +- builtin/fsck.c | 2 +- builtin/pack-objects.c | 4 ++-- diff.c | 3 ++- list-objects.c | 3 ++- pack-bitmap.c | 2 +- packfile.c | 9 +++++---- packfile.h | 5 +++-- prune-packed.c | 2 +- reachable.c | 2 +- revision.c | 4 ++-- 11 files changed, 21 insertions(+), 17 deletions(-) diff --git a/builtin/count-objects.c b/builtin/count-objects.c index 04d80887e0e..1e89148ed74 100644 --- a/builtin/count-objects.c +++ b/builtin/count-objects.c @@ -67,7 +67,7 @@ static int count_loose(const struct object_id *oid, const char *path, else { loose_size += on_disk_bytes(st); loose++; - if (verbose && has_object_pack(oid)) + if (verbose && has_object_pack(the_repository, oid)) packed_loose++; } return 0; diff --git a/builtin/fsck.c b/builtin/fsck.c index 7f4e2f04143..bb56eb98acc 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -272,7 +272,7 @@ static void check_reachable_object(struct object *obj) if (!(obj->flags & HAS_OBJ)) { if (is_promisor_object(&obj->oid)) return; - if (has_object_pack(&obj->oid)) + if (has_object_pack(the_repository, &obj->oid)) return; /* it is in pack - forget about it */ printf_ln(_("missing %s %s"), printable_type(&obj->oid, obj->type), diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 08007142671..0f32e92a3a3 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1529,7 +1529,7 @@ static int want_found_object(const struct object_id *oid, int exclude, return 0; if (ignore_packed_keep_in_core && p->pack_keep_in_core) return 0; - if (has_object_kept_pack(oid, flags)) + if (has_object_kept_pack(p->repo, oid, flags)) return 0; } @@ -3627,7 +3627,7 @@ static void show_cruft_commit(struct commit *commit, void *data) static int cruft_include_check_obj(struct object *obj, void *data UNUSED) { - return !has_object_kept_pack(&obj->oid, IN_CORE_KEEP_PACKS); + return !has_object_kept_pack(to_pack.repo, &obj->oid, IN_CORE_KEEP_PACKS); } static int cruft_include_check(struct commit *commit, void *data) diff --git a/diff.c b/diff.c index dceac20d18f..266ddf18e73 100644 --- a/diff.c +++ b/diff.c @@ -4041,7 +4041,8 @@ static int reuse_worktree_file(struct index_state *istate, * objects however would tend to be slower as they need * to be individually opened and inflated. */ - if (!FAST_WORKING_DIRECTORY && !want_file && has_object_pack(oid)) + if (!FAST_WORKING_DIRECTORY && !want_file && + has_object_pack(istate->repo, oid)) return 0; /* diff --git a/list-objects.c b/list-objects.c index 985d008799d..31236a8dc91 100644 --- a/list-objects.c +++ b/list-objects.c @@ -41,7 +41,8 @@ static void show_object(struct traversal_context *ctx, { if (!ctx->show_object) return; - if (ctx->revs->unpacked && has_object_pack(&object->oid)) + if (ctx->revs->unpacked && has_object_pack(ctx->revs->repo, + &object->oid)) return; ctx->show_object(object, name, ctx->show_data); diff --git a/pack-bitmap.c b/pack-bitmap.c index 4fa9dfc771a..d34ba9909af 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -1889,7 +1889,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git, bitmap_unset(result, i); for (i = 0; i < eindex->count; ++i) { - if (has_object_pack(&eindex->objects[i]->oid)) + if (has_object_pack(the_repository, &eindex->objects[i]->oid)) bitmap_unset(result, objects_nr + i); } } diff --git a/packfile.c b/packfile.c index 1015dac6dbc..e7dd2702174 100644 --- a/packfile.c +++ b/packfile.c @@ -2143,16 +2143,17 @@ int find_kept_pack_entry(struct repository *r, return 0; } -int has_object_pack(const struct object_id *oid) +int has_object_pack(struct repository *r, const struct object_id *oid) { struct pack_entry e; - return find_pack_entry(the_repository, oid, &e); + return find_pack_entry(r, oid, &e); } -int has_object_kept_pack(const struct object_id *oid, unsigned flags) +int has_object_kept_pack(struct repository *r, const struct object_id *oid, + unsigned flags) { struct pack_entry e; - return find_kept_pack_entry(the_repository, oid, flags, &e); + return find_kept_pack_entry(r, oid, flags, &e); } int for_each_object_in_pack(struct packed_git *p, diff --git a/packfile.h b/packfile.h index 51187f2393f..b09fb2c530a 100644 --- a/packfile.h +++ b/packfile.h @@ -193,8 +193,9 @@ const struct packed_git *has_packed_and_bad(struct repository *, const struct ob int find_pack_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e); int find_kept_pack_entry(struct repository *r, const struct object_id *oid, unsigned flags, struct pack_entry *e); -int has_object_pack(const struct object_id *oid); -int has_object_kept_pack(const struct object_id *oid, unsigned flags); +int has_object_pack(struct repository *r, const struct object_id *oid); +int has_object_kept_pack(struct repository *r, const struct object_id *oid, + unsigned flags); /* * Return 1 if an object in a promisor packfile is or refers to the given diff --git a/prune-packed.c b/prune-packed.c index 2bb99c29dfb..d1c65ab10ed 100644 --- a/prune-packed.c +++ b/prune-packed.c @@ -24,7 +24,7 @@ static int prune_object(const struct object_id *oid, const char *path, { int *opts = data; - if (!has_object_pack(oid)) + if (!has_object_pack(the_repository, oid)) return 0; if (*opts & PRUNE_PACKED_DRY_RUN) diff --git a/reachable.c b/reachable.c index 3e9b3dd0a46..09d2c500799 100644 --- a/reachable.c +++ b/reachable.c @@ -239,7 +239,7 @@ static int want_recent_object(struct recent_data *data, const struct object_id *oid) { if (data->ignore_in_core_kept_packs && - has_object_kept_pack(oid, IN_CORE_KEEP_PACKS)) + has_object_kept_pack(data->revs->repo, oid, IN_CORE_KEEP_PACKS)) return 0; return 1; } diff --git a/revision.c b/revision.c index f5f5b84f2b0..d1d152a67b8 100644 --- a/revision.c +++ b/revision.c @@ -4103,10 +4103,10 @@ enum commit_action get_commit_action(struct rev_info *revs, struct commit *commi { if (commit->object.flags & SHOWN) return commit_ignore; - if (revs->unpacked && has_object_pack(&commit->object.oid)) + if (revs->unpacked && has_object_pack(revs->repo, &commit->object.oid)) return commit_ignore; if (revs->no_kept_objects) { - if (has_object_kept_pack(&commit->object.oid, + if (has_object_kept_pack(revs->repo, &commit->object.oid, revs->keep_pack_cache_flags)) return commit_ignore; } -- GitLab From 8db7094f4eeea89caab9444b197a60427526a032 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Sat, 26 Oct 2024 22:11:15 +0200 Subject: [PATCH 06/34] packfile: pass down repository to `for_each_packed_object` The function `for_each_packed_object` currently relies on the global variable `the_repository`. To eliminate global variable usage in `packfile.c`, we should progressively shift the dependency on the_repository to higher layers. Let's remove its usage from this function and closely related function `is_promisor_object`. Signed-off-by: Karthik Nayak --- builtin/cat-file.c | 7 ++++--- builtin/fsck.c | 18 +++++++++++------- builtin/pack-objects.c | 7 +++++-- builtin/repack.c | 2 +- builtin/rev-list.c | 2 +- commit-graph.c | 2 +- fsck.c | 2 +- list-objects.c | 4 ++-- object-store-ll.h | 4 ++-- packfile.c | 14 +++++++------- packfile.h | 2 +- promisor-remote.c | 2 +- reachable.c | 2 +- revision.c | 9 +++++---- tag.c | 2 +- 15 files changed, 44 insertions(+), 35 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index bfdfb51c7cb..d67b101c20c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -827,15 +827,16 @@ static int batch_objects(struct batch_options *opt) cb.seen = &seen; for_each_loose_object(batch_unordered_loose, &cb, 0); - for_each_packed_object(batch_unordered_packed, &cb, - FOR_EACH_OBJECT_PACK_ORDER); + for_each_packed_object(the_repository, batch_unordered_packed, + &cb, FOR_EACH_OBJECT_PACK_ORDER); oidset_clear(&seen); } else { struct oid_array sa = OID_ARRAY_INIT; for_each_loose_object(collect_loose_object, &sa, 0); - for_each_packed_object(collect_packed_object, &sa, 0); + for_each_packed_object(the_repository, collect_packed_object, + &sa, 0); oid_array_for_each_unique(&sa, batch_object_cb, &cb); diff --git a/builtin/fsck.c b/builtin/fsck.c index bb56eb98acc..0196c54eb68 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -150,7 +150,7 @@ static int mark_object(struct object *obj, enum object_type type, return 0; obj->flags |= REACHABLE; - if (is_promisor_object(&obj->oid)) + if (is_promisor_object(the_repository, &obj->oid)) /* * Further recursion does not need to be performed on this * object since it is a promisor object (so it does not need to @@ -270,7 +270,7 @@ static void check_reachable_object(struct object *obj) * do a full fsck */ if (!(obj->flags & HAS_OBJ)) { - if (is_promisor_object(&obj->oid)) + if (is_promisor_object(the_repository, &obj->oid)) return; if (has_object_pack(the_repository, &obj->oid)) return; /* it is in pack - forget about it */ @@ -391,7 +391,10 @@ static void check_connectivity(void) * traversal. */ for_each_loose_object(mark_loose_unreachable_referents, NULL, 0); - for_each_packed_object(mark_packed_unreachable_referents, NULL, 0); + for_each_packed_object(the_repository, + mark_packed_unreachable_referents, + NULL, + 0); } /* Look up all the requirements, warn about missing objects.. */ @@ -488,7 +491,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid, refname, timestamp); obj->flags |= USED; mark_object_reachable(obj); - } else if (!is_promisor_object(oid)) { + } else if (!is_promisor_object(the_repository, oid)) { error(_("%s: invalid reflog entry %s"), refname, oid_to_hex(oid)); errors_found |= ERROR_REACHABLE; @@ -531,7 +534,7 @@ static int fsck_handle_ref(const char *refname, const char *referent UNUSED, con obj = parse_object(the_repository, oid); if (!obj) { - if (is_promisor_object(oid)) { + if (is_promisor_object(the_repository, oid)) { /* * Increment default_refs anyway, because this is a * valid ref. @@ -966,7 +969,8 @@ int cmd_fsck(int argc, if (connectivity_only) { for_each_loose_object(mark_loose_for_connectivity, NULL, 0); - for_each_packed_object(mark_packed_for_connectivity, NULL, 0); + for_each_packed_object(the_repository, + mark_packed_for_connectivity, NULL, 0); } else { prepare_alt_odb(the_repository); for (odb = the_repository->objects->odb; odb; odb = odb->next) @@ -1011,7 +1015,7 @@ int cmd_fsck(int argc, &oid); if (!obj || !(obj->flags & HAS_OBJ)) { - if (is_promisor_object(&oid)) + if (is_promisor_object(the_repository, &oid)) continue; error(_("%s: object missing"), oid_to_hex(&oid)); errors_found |= ERROR_OBJECT; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 0f32e92a3a3..db20f0cf51a 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -3858,7 +3858,8 @@ static void show_object__ma_allow_promisor(struct object *obj, const char *name, * Quietly ignore EXPECTED missing objects. This avoids problems with * staging them now and getting an odd error later. */ - if (!has_object(the_repository, &obj->oid, 0) && is_promisor_object(&obj->oid)) + if (!has_object(the_repository, &obj->oid, 0) && + is_promisor_object(to_pack.repo, &obj->oid)) return; show_object(obj, name, data); @@ -3927,7 +3928,9 @@ static int add_object_in_unpacked_pack(const struct object_id *oid, static void add_objects_in_unpacked_packs(void) { - if (for_each_packed_object(add_object_in_unpacked_pack, NULL, + if (for_each_packed_object(to_pack.repo, + add_object_in_unpacked_pack, + NULL, FOR_EACH_OBJECT_PACK_ORDER | FOR_EACH_OBJECT_LOCAL_ONLY | FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS | diff --git a/builtin/repack.c b/builtin/repack.c index d6bb37e84ae..96a4fa234bd 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -404,7 +404,7 @@ static void repack_promisor_objects(const struct pack_objects_args *args, * {type -> existing pack order} ordering when computing deltas instead * of a {type -> size} ordering, which may produce better deltas. */ - for_each_packed_object(write_oid, &cmd, + for_each_packed_object(the_repository, write_oid, &cmd, FOR_EACH_OBJECT_PROMISOR_ONLY); if (cmd.in == -1) { diff --git a/builtin/rev-list.c b/builtin/rev-list.c index f62bcbf2b14..43c42621e3d 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -121,7 +121,7 @@ static inline void finish_object__ma(struct object *obj) return; case MA_ALLOW_PROMISOR: - if (is_promisor_object(&obj->oid)) + if (is_promisor_object(the_repository, &obj->oid)) return; die("unexpected missing %s object '%s'", type_name(obj->type), oid_to_hex(&obj->oid)); diff --git a/commit-graph.c b/commit-graph.c index 83dd69bfebc..e2e2083951c 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1960,7 +1960,7 @@ static void fill_oids_from_all_packs(struct write_commit_graph_context *ctx) ctx->progress = start_delayed_progress( _("Finding commits for commit graph among packed objects"), ctx->approx_nr_objects); - for_each_packed_object(add_packed_commits, ctx, + for_each_packed_object(ctx->r, add_packed_commits, ctx, FOR_EACH_OBJECT_PACK_ORDER); if (ctx->progress_done < ctx->approx_nr_objects) display_progress(ctx->progress, ctx->approx_nr_objects); diff --git a/fsck.c b/fsck.c index 3756f52459e..87ce999a49c 100644 --- a/fsck.c +++ b/fsck.c @@ -1295,7 +1295,7 @@ static int fsck_blobs(struct oidset *blobs_found, struct oidset *blobs_done, buf = repo_read_object_file(the_repository, oid, &type, &size); if (!buf) { - if (is_promisor_object(oid)) + if (is_promisor_object(the_repository, oid)) continue; ret |= report(options, oid, OBJ_BLOB, msg_missing, diff --git a/list-objects.c b/list-objects.c index 31236a8dc91..d11a389b3a0 100644 --- a/list-objects.c +++ b/list-objects.c @@ -75,7 +75,7 @@ static void process_blob(struct traversal_context *ctx, */ if (ctx->revs->exclude_promisor_objects && !repo_has_object_file(the_repository, &obj->oid) && - is_promisor_object(&obj->oid)) + is_promisor_object(ctx->revs->repo, &obj->oid)) return; pathlen = path->len; @@ -180,7 +180,7 @@ static void process_tree(struct traversal_context *ctx, * an incomplete list of missing objects. */ if (revs->exclude_promisor_objects && - is_promisor_object(&obj->oid)) + is_promisor_object(revs->repo, &obj->oid)) return; if (!revs->do_not_die_on_missing_objects) diff --git a/object-store-ll.h b/object-store-ll.h index d46cd0e6545..cd3bd5bd99f 100644 --- a/object-store-ll.h +++ b/object-store-ll.h @@ -550,7 +550,7 @@ typedef int each_packed_object_fn(const struct object_id *oid, int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn, void *data, enum for_each_object_flags flags); -int for_each_packed_object(each_packed_object_fn, void *, - enum for_each_object_flags flags); +int for_each_packed_object(struct repository *repo, each_packed_object_fn cb, + void *data, enum for_each_object_flags flags); #endif /* OBJECT_STORE_LL_H */ diff --git a/packfile.c b/packfile.c index e7dd2702174..5e8019b1fe4 100644 --- a/packfile.c +++ b/packfile.c @@ -2200,15 +2200,15 @@ int for_each_object_in_pack(struct packed_git *p, return r; } -int for_each_packed_object(each_packed_object_fn cb, void *data, - enum for_each_object_flags flags) +int for_each_packed_object(struct repository *repo, each_packed_object_fn cb, + void *data, enum for_each_object_flags flags) { struct packed_git *p; int r = 0; int pack_errors = 0; - prepare_packed_git(the_repository); - for (p = get_all_packs(the_repository); p; p = p->next) { + prepare_packed_git(repo); + for (p = get_all_packs(repo); p; p = p->next) { if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local) continue; if ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) && @@ -2286,14 +2286,14 @@ static int add_promisor_object(const struct object_id *oid, return 0; } -int is_promisor_object(const struct object_id *oid) +int is_promisor_object(struct repository *r, const struct object_id *oid) { static struct oidset promisor_objects; static int promisor_objects_prepared; if (!promisor_objects_prepared) { - if (repo_has_promisor_remote(the_repository)) { - for_each_packed_object(add_promisor_object, + if (repo_has_promisor_remote(r)) { + for_each_packed_object(r, add_promisor_object, &promisor_objects, FOR_EACH_OBJECT_PROMISOR_ONLY | FOR_EACH_OBJECT_PACK_ORDER); diff --git a/packfile.h b/packfile.h index b09fb2c530a..addb95b0c44 100644 --- a/packfile.h +++ b/packfile.h @@ -201,7 +201,7 @@ int has_object_kept_pack(struct repository *r, const struct object_id *oid, * Return 1 if an object in a promisor packfile is or refers to the given * object, 0 otherwise. */ -int is_promisor_object(const struct object_id *oid); +int is_promisor_object(struct repository *r, const struct object_id *oid); /* * Expose a function for fuzz testing. diff --git a/promisor-remote.c b/promisor-remote.c index 9345ae3db23..c714f4f0072 100644 --- a/promisor-remote.c +++ b/promisor-remote.c @@ -283,7 +283,7 @@ void promisor_remote_get_direct(struct repository *repo, } for (i = 0; i < remaining_nr; i++) { - if (is_promisor_object(&remaining_oids[i])) + if (is_promisor_object(repo, &remaining_oids[i])) die(_("could not fetch %s from promisor remote"), oid_to_hex(&remaining_oids[i])); } diff --git a/reachable.c b/reachable.c index 09d2c500799..ecf7ccf5041 100644 --- a/reachable.c +++ b/reachable.c @@ -324,7 +324,7 @@ int add_unseen_recent_objects_to_traversal(struct rev_info *revs, if (ignore_in_core_kept_packs) flags |= FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS; - r = for_each_packed_object(add_recent_packed, &data, flags); + r = for_each_packed_object(revs->repo, add_recent_packed, &data, flags); done: oidset_clear(&data.extra_recent_oids); diff --git a/revision.c b/revision.c index d1d152a67b8..45dc6d2819a 100644 --- a/revision.c +++ b/revision.c @@ -390,7 +390,8 @@ static struct object *get_reference(struct rev_info *revs, const char *name, if (!object) { if (revs->ignore_missing) return NULL; - if (revs->exclude_promisor_objects && is_promisor_object(oid)) + if (revs->exclude_promisor_objects && + is_promisor_object(revs->repo, oid)) return NULL; if (revs->do_not_die_on_missing_objects) { oidset_insert(&revs->missing_commits, oid); @@ -432,7 +433,7 @@ static struct commit *handle_commit(struct rev_info *revs, if (revs->ignore_missing_links || (flags & UNINTERESTING)) return NULL; if (revs->exclude_promisor_objects && - is_promisor_object(&tag->tagged->oid)) + is_promisor_object(revs->repo, &tag->tagged->oid)) return NULL; if (revs->do_not_die_on_missing_objects && oid) { oidset_insert(&revs->missing_commits, oid); @@ -1211,7 +1212,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit, revs->do_not_die_on_missing_objects; if (repo_parse_commit_gently(revs->repo, p, gently) < 0) { if (revs->exclude_promisor_objects && - is_promisor_object(&p->object.oid)) { + is_promisor_object(revs->repo, &p->object.oid)) { if (revs->first_parent_only) break; continue; @@ -3915,7 +3916,7 @@ int prepare_revision_walk(struct rev_info *revs) revs->treesame.name = "treesame"; if (revs->exclude_promisor_objects) { - for_each_packed_object(mark_uninteresting, revs, + for_each_packed_object(revs->repo, mark_uninteresting, revs, FOR_EACH_OBJECT_PROMISOR_ONLY); } diff --git a/tag.c b/tag.c index d24170e3406..beef9571b5c 100644 --- a/tag.c +++ b/tag.c @@ -84,7 +84,7 @@ struct object *deref_tag(struct repository *r, struct object *o, const char *war o = NULL; } if (!o && warn) { - if (last_oid && is_promisor_object(last_oid)) + if (last_oid && is_promisor_object(r, last_oid)) return NULL; if (!warnlen) warnlen = strlen(warn); -- GitLab From d1b6e8801bb7edc9ce55bb50a5d46f9779906ad1 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Fri, 18 Oct 2024 23:43:20 +0200 Subject: [PATCH 07/34] config: make `delta_base_cache_limit` a non-global variable The `delta_base_cache_limit` variable is a global config variable used by multiple subsystems. Let's make this non-global, by adding this variable independently to the subsystems where it is used. First, add the setting to the `repo_settings` struct, this provides access to the config in places where the repository is available. Use this in `packfile.c`. In `index-pack.c` we add it to the `pack_idx_option` struct and its constructor. While the repository struct is available here, it may not be set because `git index-pack` can be used without a repository. In `gc.c` add it to the `gc_config` struct and also the constructor function. The gc functions currently do not have direct access to a repository struct. These changes are made to remove the usage of `delta_base_cache_limit` as a global variable in `packfile.c`. This brings us one step closer to removing the `USE_THE_REPOSITORY_VARIABLE` definition in `packfile.c` which we complete in the next patch. Signed-off-by: Karthik Nayak --- builtin/gc.c | 8 +++++++- builtin/index-pack.c | 10 +++++++--- config.c | 5 ----- environment.c | 1 - environment.h | 1 - pack-objects.h | 3 ++- pack-write.c | 1 + pack.h | 2 ++ packfile.c | 10 ++++++++-- repo-settings.c | 5 +++++ repo-settings.h | 3 +++ 11 files changed, 35 insertions(+), 14 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index d52735354c9..09802eb9894 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -138,6 +138,7 @@ struct gc_config { char *repack_filter_to; unsigned long big_pack_threshold; unsigned long max_delta_cache_size; + size_t delta_base_cache_limit; }; #define GC_CONFIG_INIT { \ @@ -153,6 +154,7 @@ struct gc_config { .prune_expire = xstrdup("2.weeks.ago"), \ .prune_worktrees_expire = xstrdup("3.months.ago"), \ .max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \ + .delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \ } static void gc_config_release(struct gc_config *cfg) @@ -168,6 +170,7 @@ static void gc_config(struct gc_config *cfg) { const char *value; char *owned = NULL; + unsigned long ulongval; if (!git_config_get_value("gc.packrefs", &value)) { if (value && !strcmp(value, "notbare")) @@ -206,6 +209,9 @@ static void gc_config(struct gc_config *cfg) git_config_get_ulong("gc.bigpackthreshold", &cfg->big_pack_threshold); git_config_get_ulong("pack.deltacachesize", &cfg->max_delta_cache_size); + if (!git_config_get_ulong("core.deltabasecachelimit", &ulongval)) + cfg->delta_base_cache_limit = ulongval; + if (!git_config_get_string("gc.repackfilter", &owned)) { free(cfg->repack_filter); cfg->repack_filter = owned; @@ -416,7 +422,7 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg, * read_sha1_file() (either at delta calculation phase, or * writing phase) also fills up the delta base cache */ - heap += delta_base_cache_limit; + heap += cfg->delta_base_cache_limit; /* and of course pack-objects has its own delta cache */ heap += cfg->max_delta_cache_size; diff --git a/builtin/index-pack.c b/builtin/index-pack.c index eaefb41761e..23bfa454031 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1238,7 +1238,7 @@ static void parse_pack_objects(unsigned char *hash) * recursively checking if the resulting object is used as a base * for some more deltas. */ -static void resolve_deltas(void) +static void resolve_deltas(struct pack_idx_option *opts) { int i; @@ -1254,7 +1254,7 @@ static void resolve_deltas(void) nr_ref_deltas + nr_ofs_deltas); nr_dispatched = 0; - base_cache_limit = delta_base_cache_limit * nr_threads; + base_cache_limit = opts->delta_base_cache_limit * nr_threads; if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) { init_thread(); work_lock(); @@ -1604,6 +1604,10 @@ static int git_index_pack_config(const char *k, const char *v, else opts->flags &= ~WRITE_REV; } + if (!strcmp(k, "core.deltabasecachelimit")) { + opts->delta_base_cache_limit = git_config_ulong(k, v, ctx->kvi); + return 0; + } return git_default_config(k, v, ctx, cb); } @@ -1930,7 +1934,7 @@ int cmd_index_pack(int argc, parse_pack_objects(pack_hash); if (report_end_of_input) write_in_full(2, "\0", 1); - resolve_deltas(); + resolve_deltas(&opts); conclude_pack(fix_thin_pack, curr_pack, pack_hash); free(ofs_deltas); free(ref_deltas); diff --git a/config.c b/config.c index a11bb85da30..728ef98e427 100644 --- a/config.c +++ b/config.c @@ -1515,11 +1515,6 @@ static int git_default_core_config(const char *var, const char *value, return 0; } - if (!strcmp(var, "core.deltabasecachelimit")) { - delta_base_cache_limit = git_config_ulong(var, value, ctx->kvi); - return 0; - } - if (!strcmp(var, "core.autocrlf")) { if (value && !strcasecmp(value, "input")) { auto_crlf = AUTO_CRLF_INPUT; diff --git a/environment.c b/environment.c index a2ce9980818..8e5022c282c 100644 --- a/environment.c +++ b/environment.c @@ -51,7 +51,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT; enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT; size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE; size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT; -size_t delta_base_cache_limit = 96 * 1024 * 1024; unsigned long big_file_threshold = 512 * 1024 * 1024; char *editor_program; char *askpass_program; diff --git a/environment.h b/environment.h index 923e12661e1..2f43340f0b5 100644 --- a/environment.h +++ b/environment.h @@ -165,7 +165,6 @@ extern int zlib_compression_level; extern int pack_compression_level; extern size_t packed_git_window_size; extern size_t packed_git_limit; -extern size_t delta_base_cache_limit; extern unsigned long big_file_threshold; extern unsigned long pack_size_limit_cfg; extern int max_allowed_tree_depth; diff --git a/pack-objects.h b/pack-objects.h index b9898a4e64b..3f6f5042030 100644 --- a/pack-objects.h +++ b/pack-objects.h @@ -7,7 +7,8 @@ struct repository; -#define DEFAULT_DELTA_CACHE_SIZE (256 * 1024 * 1024) +#define DEFAULT_DELTA_CACHE_SIZE (256 * 1024 * 1024) +#define DEFAULT_DELTA_BASE_CACHE_LIMIT (96 * 1024 * 1024) #define OE_DFS_STATE_BITS 2 #define OE_DEPTH_BITS 12 diff --git a/pack-write.c b/pack-write.c index 8c7dfddc5aa..98a8c0e7853 100644 --- a/pack-write.c +++ b/pack-write.c @@ -21,6 +21,7 @@ void reset_pack_idx_option(struct pack_idx_option *opts) memset(opts, 0, sizeof(*opts)); opts->version = 2; opts->off32_limit = 0x7fffffff; + opts->delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT; } static int sha1_compare(const void *_a, const void *_b) diff --git a/pack.h b/pack.h index 02bbdfb19cc..a8da0406299 100644 --- a/pack.h +++ b/pack.h @@ -58,6 +58,8 @@ struct pack_idx_option { */ int anomaly_alloc, anomaly_nr; uint32_t *anomaly; + + size_t delta_base_cache_limit; }; void reset_pack_idx_option(struct pack_idx_option *); diff --git a/packfile.c b/packfile.c index 5e8019b1fe4..64248ca664a 100644 --- a/packfile.c +++ b/packfile.c @@ -1496,7 +1496,9 @@ void clear_delta_base_cache(void) } static void add_delta_base_cache(struct packed_git *p, off_t base_offset, - void *base, unsigned long base_size, enum object_type type) + void *base, unsigned long base_size, + unsigned long delta_base_cache_limit, + enum object_type type) { struct delta_base_cache_entry *ent; struct list_head *lru, *tmp; @@ -1698,6 +1700,8 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset, int delta_stack_nr = 0, delta_stack_alloc = UNPACK_ENTRY_STACK_PREALLOC; int base_from_cache = 0; + prepare_repo_settings(p->repo); + write_pack_access_log(p, obj_offset); /* PHASE 1: drill down to the innermost base object */ @@ -1878,7 +1882,9 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset, * before we are done using it. */ if (!external_base) - add_delta_base_cache(p, base_obj_offset, base, base_size, type); + add_delta_base_cache(p, base_obj_offset, base, base_size, + p->repo->settings.delta_base_cache_limit, + type); free(delta_data); free(external_base); diff --git a/repo-settings.c b/repo-settings.c index 4699b4b3650..acc27eb8fe7 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -3,6 +3,7 @@ #include "repo-settings.h" #include "repository.h" #include "midx.h" +#include "pack-objects.h" static void repo_cfg_bool(struct repository *r, const char *key, int *dest, int def) @@ -26,6 +27,7 @@ void prepare_repo_settings(struct repository *r) const char *strval; int manyfiles; int read_changed_paths; + unsigned long ulongval; if (!r->gitdir) BUG("Cannot add settings for uninitialized repository"); @@ -123,6 +125,9 @@ void prepare_repo_settings(struct repository *r) * removed. */ r->settings.command_requires_full_index = 1; + + if (!repo_config_get_ulong(r, "core.deltabasecachelimit", &ulongval)) + r->settings.delta_base_cache_limit = ulongval; } enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo) diff --git a/repo-settings.h b/repo-settings.h index 51d6156a117..10a6f7ed64d 100644 --- a/repo-settings.h +++ b/repo-settings.h @@ -57,12 +57,15 @@ struct repo_settings { int core_multi_pack_index; int warn_ambiguous_refs; /* lazily loaded via accessor */ + + size_t delta_base_cache_limit; }; #define REPO_SETTINGS_INIT { \ .index_version = -1, \ .core_untracked_cache = UNTRACKED_CACHE_KEEP, \ .fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE, \ .warn_ambiguous_refs = -1, \ + .delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \ } void prepare_repo_settings(struct repository *r); -- GitLab From 30a52f192f07fcc45d692ed1a42d21571c5c53b9 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Sat, 19 Oct 2024 23:09:04 +0200 Subject: [PATCH 08/34] config: make `packed_git_(limit|window_size)` non-global variables The variables `packed_git_window_size` and `packed_git_limit` are global config variables used in the `packfile.c` file. Since it is only used in this file, let's change it from being a global config variable to a local variable for the subsystem. With this, we rid `packfile.c` from all global variable usage and this means we can also remove the `USE_THE_REPOSITORY_VARIABLE` guard from the file. Helped-by: Taylor Blau Signed-off-by: Karthik Nayak --- builtin/fast-import.c | 4 ++-- config.c | 17 ----------------- environment.c | 2 -- packfile.c | 23 +++++++++++++++-------- packfile.h | 2 +- repo-settings.c | 13 +++++++++++++ repo-settings.h | 4 ++++ 7 files changed, 35 insertions(+), 30 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 3ccc4c57227..0ece0702604 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3539,7 +3539,7 @@ static void parse_argv(void) int cmd_fast_import(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { unsigned int i; @@ -3660,7 +3660,7 @@ int cmd_fast_import(int argc, fprintf(stderr, " pools: %10lu KiB\n", (unsigned long)((tree_entry_allocd + fi_mem_pool.pool_alloc) /1024)); fprintf(stderr, " objects: %10" PRIuMAX " KiB\n", (alloc_count*sizeof(struct object_entry))/1024); fprintf(stderr, "---------------------------------------------------------------------\n"); - pack_report(); + pack_report(repo); fprintf(stderr, "---------------------------------------------------------------------\n"); fprintf(stderr, "\n"); } diff --git a/config.c b/config.c index 728ef98e427..2c295f74308 100644 --- a/config.c +++ b/config.c @@ -1493,28 +1493,11 @@ static int git_default_core_config(const char *var, const char *value, return 0; } - if (!strcmp(var, "core.packedgitwindowsize")) { - int pgsz_x2 = getpagesize() * 2; - packed_git_window_size = git_config_ulong(var, value, ctx->kvi); - - /* This value must be multiple of (pagesize * 2) */ - packed_git_window_size /= pgsz_x2; - if (packed_git_window_size < 1) - packed_git_window_size = 1; - packed_git_window_size *= pgsz_x2; - return 0; - } - if (!strcmp(var, "core.bigfilethreshold")) { big_file_threshold = git_config_ulong(var, value, ctx->kvi); return 0; } - if (!strcmp(var, "core.packedgitlimit")) { - packed_git_limit = git_config_ulong(var, value, ctx->kvi); - return 0; - } - if (!strcmp(var, "core.autocrlf")) { if (value && !strcasecmp(value, "input")) { auto_crlf = AUTO_CRLF_INPUT; diff --git a/environment.c b/environment.c index 8e5022c282c..8389a272700 100644 --- a/environment.c +++ b/environment.c @@ -49,8 +49,6 @@ int fsync_object_files = -1; int use_fsync = -1; enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT; enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT; -size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE; -size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT; unsigned long big_file_threshold = 512 * 1024 * 1024; char *editor_program; char *askpass_program; diff --git a/packfile.c b/packfile.c index 64248ca664a..2e0e28c7de8 100644 --- a/packfile.c +++ b/packfile.c @@ -1,4 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE #include "git-compat-util.h" #include "environment.h" @@ -46,15 +45,15 @@ static size_t pack_mapped; #define SZ_FMT PRIuMAX static inline uintmax_t sz_fmt(size_t s) { return s; } -void pack_report(void) +void pack_report(struct repository *repo) { fprintf(stderr, "pack_report: getpagesize() = %10" SZ_FMT "\n" "pack_report: core.packedGitWindowSize = %10" SZ_FMT "\n" "pack_report: core.packedGitLimit = %10" SZ_FMT "\n", sz_fmt(getpagesize()), - sz_fmt(packed_git_window_size), - sz_fmt(packed_git_limit)); + sz_fmt(repo->settings.packed_git_window_size), + sz_fmt(repo->settings.packed_git_limit)); fprintf(stderr, "pack_report: pack_used_ctr = %10u\n" "pack_report: pack_mmap_calls = %10u\n" @@ -650,8 +649,15 @@ unsigned char *use_pack(struct packed_git *p, break; } if (!win) { - size_t window_align = packed_git_window_size / 2; + size_t window_align; off_t len; + struct repo_settings *settings; + + /* lazy load the settings in case it hasn't been setup */ + prepare_repo_settings(p->repo); + settings = &p->repo->settings; + + window_align = settings->packed_git_window_size / 2; if (p->pack_fd == -1 && open_packed_git(p)) die("packfile %s cannot be accessed", p->pack_name); @@ -659,11 +665,12 @@ unsigned char *use_pack(struct packed_git *p, CALLOC_ARRAY(win, 1); win->offset = (offset / window_align) * window_align; len = p->pack_size - win->offset; - if (len > packed_git_window_size) - len = packed_git_window_size; + if (len > settings->packed_git_window_size) + len = settings->packed_git_window_size; win->len = (size_t)len; pack_mapped += win->len; - while (packed_git_limit < pack_mapped + + while (settings->packed_git_limit < pack_mapped && unuse_one_window(p)) ; /* nothing */ win->base = xmmap_gently(NULL, win->len, diff --git a/packfile.h b/packfile.h index addb95b0c44..58104fa009d 100644 --- a/packfile.h +++ b/packfile.h @@ -89,7 +89,7 @@ unsigned long repo_approximate_object_count(struct repository *r); struct packed_git *find_oid_pack(const struct object_id *oid, struct packed_git *packs); -void pack_report(void); +void pack_report(struct repository *repo); /* * mmap the index file for the specified packfile (if it is not diff --git a/repo-settings.c b/repo-settings.c index acc27eb8fe7..9d16d5399e3 100644 --- a/repo-settings.c +++ b/repo-settings.c @@ -128,6 +128,19 @@ void prepare_repo_settings(struct repository *r) if (!repo_config_get_ulong(r, "core.deltabasecachelimit", &ulongval)) r->settings.delta_base_cache_limit = ulongval; + + if (!repo_config_get_ulong(r, "core.packedgitwindowsize", &ulongval)) { + int pgsz_x2 = getpagesize() * 2; + + /* This value must be multiple of (pagesize * 2) */ + ulongval /= pgsz_x2; + if (ulongval < 1) + ulongval = 1; + r->settings.packed_git_window_size = ulongval * pgsz_x2; + } + + if (!repo_config_get_ulong(r, "core.packedgitlimit", &ulongval)) + r->settings.packed_git_limit = ulongval; } enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo) diff --git a/repo-settings.h b/repo-settings.h index 10a6f7ed64d..93ea0c32741 100644 --- a/repo-settings.h +++ b/repo-settings.h @@ -59,6 +59,8 @@ struct repo_settings { int warn_ambiguous_refs; /* lazily loaded via accessor */ size_t delta_base_cache_limit; + size_t packed_git_window_size; + size_t packed_git_limit; }; #define REPO_SETTINGS_INIT { \ .index_version = -1, \ @@ -66,6 +68,8 @@ struct repo_settings { .fetch_negotiation_algorithm = FETCH_NEGOTIATION_CONSECUTIVE, \ .warn_ambiguous_refs = -1, \ .delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \ + .packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE, \ + .packed_git_limit = DEFAULT_PACKED_GIT_LIMIT, \ } void prepare_repo_settings(struct repository *r); -- GitLab From 2fe5d2506f72841d3fa9a4cde5430d6aed7b0da2 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 30 Oct 2024 14:43:22 +0100 Subject: [PATCH 09/34] midx: add repository to `multi_pack_index` struct The `multi_pack_index` struct represents the MIDX for a repository. Here, we add a pointer to the repository in this struct, allowing direct use of the repository variable without relying on the global `the_repository` struct. With this addition, we can determine the repository associated with a `bitmap_index` struct. A `bitmap_index` points to either a `packed_git` or a `multi_pack_index`, both of which have direct repository references. To support this, we introduce a static helper function, `bitmap_repo`, in `pack-bitmap.c`, which retrieves a repository given a `bitmap_index`. With this, we clear up all usages of `the_repository` within `pack-bitmap.c` and also remove the `USE_THE_REPOSITORY_VARIABLE` definition. Bringing us another step closer to remove all global variable usage. Although this change also opens up the potential to clean up `midx.c`, doing so would require additional refactoring to pass the repository struct to functions where the MIDX struct is created: a task better suited for future patches. Signed-off-by: Karthik Nayak --- midx.c | 1 + midx.h | 3 ++ pack-bitmap.c | 90 +++++++++++++++++++++++++++++++-------------------- 3 files changed, 59 insertions(+), 35 deletions(-) diff --git a/midx.c b/midx.c index 8edb75f51dc..079c45a1aaf 100644 --- a/midx.c +++ b/midx.c @@ -131,6 +131,7 @@ static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir m->data = midx_map; m->data_len = midx_size; m->local = local; + m->repo = the_repository; m->signature = get_be32(m->data); if (m->signature != MIDX_SIGNATURE) diff --git a/midx.h b/midx.h index 42d4f8d149e..3b0ac4d8788 100644 --- a/midx.h +++ b/midx.h @@ -71,6 +71,9 @@ struct multi_pack_index { const char **pack_names; struct packed_git **packs; + + struct repository *repo; + char object_dir[FLEX_ARRAY]; }; diff --git a/pack-bitmap.c b/pack-bitmap.c index d34ba9909af..0cb1b56c9d5 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -1,5 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE - #include "git-compat-util.h" #include "commit.h" #include "gettext.h" @@ -177,12 +175,21 @@ static uint32_t bitmap_num_objects(struct bitmap_index *index) return index->pack->num_objects; } +static struct repository *bitmap_repo(struct bitmap_index *bitmap_git) +{ + if (bitmap_is_midx(bitmap_git)) + return bitmap_git->midx->repo; + return bitmap_git->pack->repo; +} + static int load_bitmap_header(struct bitmap_index *index) { struct bitmap_disk_header *header = (void *)index->map; - size_t header_size = sizeof(*header) - GIT_MAX_RAWSZ + the_hash_algo->rawsz; + const struct git_hash_algo *hash_algo = bitmap_repo(index)->hash_algo; + + size_t header_size = sizeof(*header) - GIT_MAX_RAWSZ + hash_algo->rawsz; - if (index->map_size < header_size + the_hash_algo->rawsz) + if (index->map_size < header_size + hash_algo->rawsz) return error(_("corrupted bitmap index (too small)")); if (memcmp(header->magic, BITMAP_IDX_SIGNATURE, sizeof(BITMAP_IDX_SIGNATURE)) != 0) @@ -196,7 +203,7 @@ static int load_bitmap_header(struct bitmap_index *index) { uint32_t flags = ntohs(header->options); size_t cache_size = st_mult(bitmap_num_objects(index), sizeof(uint32_t)); - unsigned char *index_end = index->map + index->map_size - the_hash_algo->rawsz; + unsigned char *index_end = index->map + index->map_size - hash_algo->rawsz; if ((flags & BITMAP_OPT_FULL_DAG) == 0) BUG("unsupported options for bitmap index file " @@ -409,7 +416,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, if (bitmap_git->pack || bitmap_git->midx) { struct strbuf buf = STRBUF_INIT; get_midx_filename(&buf, midx->object_dir); - trace2_data_string("bitmap", the_repository, + trace2_data_string("bitmap", bitmap_repo(bitmap_git), "ignoring extra midx bitmap file", buf.buf); close(fd); strbuf_release(&buf); @@ -427,7 +434,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, goto cleanup; if (!hasheq(get_midx_checksum(bitmap_git->midx), bitmap_git->checksum, - the_repository->hash_algo)) { + bitmap_repo(bitmap_git)->hash_algo)) { error(_("checksum doesn't match in MIDX and bitmap")); goto cleanup; } @@ -438,7 +445,9 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, } for (i = 0; i < bitmap_git->midx->num_packs; i++) { - if (prepare_midx_pack(the_repository, bitmap_git->midx, i)) { + if (prepare_midx_pack(bitmap_repo(bitmap_git), + bitmap_git->midx, + i)) { warning(_("could not open pack %s"), bitmap_git->midx->pack_names[i]); goto cleanup; @@ -492,8 +501,9 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git } if (bitmap_git->pack || bitmap_git->midx) { - trace2_data_string("bitmap", the_repository, - "ignoring extra bitmap file", packfile->pack_name); + trace2_data_string("bitmap", bitmap_repo(bitmap_git), + "ignoring extra bitmap file", + packfile->pack_name); close(fd); return -1; } @@ -518,8 +528,8 @@ static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git return -1; } - trace2_data_string("bitmap", the_repository, "opened bitmap file", - packfile->pack_name); + trace2_data_string("bitmap", bitmap_repo(bitmap_git), + "opened bitmap file", packfile->pack_name); return 0; } @@ -649,7 +659,7 @@ struct bitmap_index *prepare_bitmap_git(struct repository *r) struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx) { - struct repository *r = the_repository; + struct repository *r = midx->repo; struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git)); if (!open_midx_bitmap_1(bitmap_git, midx) && !load_bitmap(r, bitmap_git)) @@ -1213,6 +1223,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git, { struct bitmap_boundary_cb cb; struct object_list *root; + struct repository *repo; unsigned int i; unsigned int tmp_blobs, tmp_trees, tmp_tags; int any_missing = 0; @@ -1222,6 +1233,8 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git, cb.base = bitmap_new(); object_array_init(&cb.boundary); + repo = bitmap_repo(bitmap_git); + revs->ignore_missing_links = 1; if (bitmap_git->pseudo_merges.nr) { @@ -1280,19 +1293,19 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git, * revision walk to (a) OR in any bitmaps that are UNINTERESTING * between the tips and boundary, and (b) record the boundary. */ - trace2_region_enter("pack-bitmap", "boundary-prepare", the_repository); + trace2_region_enter("pack-bitmap", "boundary-prepare", repo); if (prepare_revision_walk(revs)) die("revision walk setup failed"); - trace2_region_leave("pack-bitmap", "boundary-prepare", the_repository); + trace2_region_leave("pack-bitmap", "boundary-prepare", repo); - trace2_region_enter("pack-bitmap", "boundary-traverse", the_repository); + trace2_region_enter("pack-bitmap", "boundary-traverse", repo); revs->boundary = 1; traverse_commit_list_filtered(revs, show_boundary_commit, show_boundary_object, &cb, NULL); revs->boundary = 0; - trace2_region_leave("pack-bitmap", "boundary-traverse", the_repository); + trace2_region_leave("pack-bitmap", "boundary-traverse", repo); revs->blob_objects = tmp_blobs; revs->tree_objects = tmp_trees; @@ -1304,7 +1317,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git, /* * Then add the boundary commit(s) as fill-in traversal tips. */ - trace2_region_enter("pack-bitmap", "boundary-fill-in", the_repository); + trace2_region_enter("pack-bitmap", "boundary-fill-in", repo); for (i = 0; i < cb.boundary.nr; i++) { struct object *obj = cb.boundary.objects[i].item; if (bitmap_walk_contains(bitmap_git, cb.base, &obj->oid)) @@ -1314,7 +1327,7 @@ static struct bitmap *find_boundary_objects(struct bitmap_index *bitmap_git, } if (revs->pending.nr) cb.base = fill_in_bitmap(bitmap_git, revs, cb.base, NULL); - trace2_region_leave("pack-bitmap", "boundary-fill-in", the_repository); + trace2_region_leave("pack-bitmap", "boundary-fill-in", repo); cleanup: object_array_clear(&cb.boundary); @@ -1718,7 +1731,8 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git, ofs = pack_pos_to_offset(pack, pos); } - if (packed_object_info(the_repository, pack, ofs, &oi) < 0) { + if (packed_object_info(bitmap_repo(bitmap_git), pack, ofs, + &oi) < 0) { struct object_id oid; nth_bitmap_object_oid(bitmap_git, &oid, pack_pos_to_index(pack, pos)); @@ -1727,7 +1741,8 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git, } else { struct eindex *eindex = &bitmap_git->ext_index; struct object *obj = eindex->objects[pos - bitmap_num_objects(bitmap_git)]; - if (oid_object_info_extended(the_repository, &obj->oid, &oi, 0) < 0) + if (oid_object_info_extended(bitmap_repo(bitmap_git), &obj->oid, + &oi, 0) < 0) die(_("unable to get size of %s"), oid_to_hex(&obj->oid)); } @@ -1889,7 +1904,8 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git, bitmap_unset(result, i); for (i = 0; i < eindex->count; ++i) { - if (has_object_pack(the_repository, &eindex->objects[i]->oid)) + if (has_object_pack(bitmap_repo(bitmap_git), + &eindex->objects[i]->oid)) bitmap_unset(result, objects_nr + i); } } @@ -1907,6 +1923,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs, struct bitmap *haves_bitmap = NULL; struct bitmap_index *bitmap_git; + struct repository *repo; /* * We can't do pathspec limiting with bitmaps, because we don't know @@ -1980,18 +1997,20 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs, if (!use_boundary_traversal) object_array_clear(&revs->pending); + repo = bitmap_repo(bitmap_git); + if (haves) { if (use_boundary_traversal) { - trace2_region_enter("pack-bitmap", "haves/boundary", the_repository); + trace2_region_enter("pack-bitmap", "haves/boundary", repo); haves_bitmap = find_boundary_objects(bitmap_git, revs, haves); - trace2_region_leave("pack-bitmap", "haves/boundary", the_repository); + trace2_region_leave("pack-bitmap", "haves/boundary", repo); } else { - trace2_region_enter("pack-bitmap", "haves/classic", the_repository); + trace2_region_enter("pack-bitmap", "haves/classic", repo); revs->ignore_missing_links = 1; haves_bitmap = find_objects(bitmap_git, revs, haves, NULL); reset_revision_walk(); revs->ignore_missing_links = 0; - trace2_region_leave("pack-bitmap", "haves/classic", the_repository); + trace2_region_leave("pack-bitmap", "haves/classic", repo); } if (!haves_bitmap) @@ -2025,17 +2044,17 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs, object_list_free(&wants); object_list_free(&haves); - trace2_data_intmax("bitmap", the_repository, "pseudo_merges_satisfied", + trace2_data_intmax("bitmap", repo, "pseudo_merges_satisfied", pseudo_merges_satisfied_nr); - trace2_data_intmax("bitmap", the_repository, "pseudo_merges_cascades", + trace2_data_intmax("bitmap", repo, "pseudo_merges_cascades", pseudo_merges_cascades_nr); - trace2_data_intmax("bitmap", the_repository, "bitmap/hits", + trace2_data_intmax("bitmap", repo, "bitmap/hits", existing_bitmaps_hits_nr); - trace2_data_intmax("bitmap", the_repository, "bitmap/misses", + trace2_data_intmax("bitmap", repo, "bitmap/misses", existing_bitmaps_misses_nr); - trace2_data_intmax("bitmap", the_repository, "bitmap/roots_with_bitmap", + trace2_data_intmax("bitmap", repo, "bitmap/roots_with_bitmap", roots_with_bitmaps_nr); - trace2_data_intmax("bitmap", the_repository, "bitmap/roots_without_bitmap", + trace2_data_intmax("bitmap", repo, "bitmap/roots_without_bitmap", roots_without_bitmaps_nr); return bitmap_git; @@ -2256,7 +2275,7 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git, struct bitmap **reuse_out, int multi_pack_reuse) { - struct repository *r = the_repository; + struct repository *r = bitmap_repo(bitmap_git); struct bitmapped_pack *packs = NULL; struct bitmap *result = bitmap_git->result; struct bitmap *reuse; @@ -2792,7 +2811,7 @@ int rebuild_bitmap(const uint32_t *reposition, uint32_t *create_bitmap_mapping(struct bitmap_index *bitmap_git, struct packing_data *mapping) { - struct repository *r = the_repository; + struct repository *r = bitmap_repo(bitmap_git); uint32_t i, num_objects; uint32_t *reposition; @@ -2948,7 +2967,8 @@ static off_t get_disk_usage_for_extended(struct bitmap_index *bitmap_git) st_add(bitmap_num_objects(bitmap_git), i))) continue; - if (oid_object_info_extended(the_repository, &obj->oid, &oi, 0) < 0) + if (oid_object_info_extended(bitmap_repo(bitmap_git), &obj->oid, + &oi, 0) < 0) die(_("unable to get disk usage of '%s'"), oid_to_hex(&obj->oid)); -- GitLab From 05989c2e2702226d470047c58fb0e0522df8cdba Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 20 Nov 2024 17:48:51 -0500 Subject: [PATCH 10/34] packfile.c: remove unnecessary prepare_packed_git() call In 454ea2e4d7 (treewide: use get_all_packs, 2018-08-20) we converted existing calls to both: - get_packed_git(), as well as - the_repository->objects->packed_git , to instead use the new get_all_packs() function. In the instance that this commit addresses, there was a preceding call to prepare_packed_git(), which dates all the way back to 660c889e46 (sha1_file: add for_each iterators for loose and packed objects, 2014-10-15) when its caller (for_each_packed_object()) was first introduced. This call could have been removed in 454ea2e4d7, since get_all_packs() itself calls prepare_packed_git(). But the translation in 454ea2e4d7 was (to the best of my knowledge) a find-and-replace rather than inspecting each individual caller. Having an extra prepare_packed_git() call here is harmless, since it will notice that we have already set the 'packed_git_initialized' field and the call will be a noop. So we're only talking about a few dozen CPU cycles to set up and tear down the stack frame. But having a lone prepare_packed_git() call immediately before a call to get_all_packs() confused me, so let's remove it as redundant to avoid more confusion in the future. Signed-off-by: Taylor Blau --- packfile.c | 1 - 1 file changed, 1 deletion(-) diff --git a/packfile.c b/packfile.c index 2e0e28c7de8..9c4bd81a8c7 100644 --- a/packfile.c +++ b/packfile.c @@ -2220,7 +2220,6 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb, int r = 0; int pack_errors = 0; - prepare_packed_git(repo); for (p = get_all_packs(repo); p; p = p->next) { if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local) continue; -- GitLab From b795c9d4b10f740fc591ef14c1fc2d93a36879a2 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Thu, 24 Oct 2024 12:53:57 +0200 Subject: [PATCH 11/34] CodingGuidelines: discourage arbitrary suffixes in function names We often name functions with arbitrary suffixes like `_1` as an extension of another existing function. This creates confusion and doesn't provide good clarity into the functions purpose. Let's document good function naming etiquette in our CodingGuidelines. Signed-off-by: Karthik Nayak Signed-off-by: Taylor Blau --- Documentation/CodingGuidelines | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 30fda4142ca..87904791cbc 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -621,6 +621,20 @@ For C programs: - `S_free()` releases a structure's contents and frees the structure. + - Function names should be clear and descriptive, accurately reflecting + their purpose or behavior. Arbitrary suffixes that do not add meaningful + context can lead to confusion, particularly for newcomers to the codebase. + + Historically, the '_1' suffix has been used in situations where: + + - A function handles one element among a group that requires similar + processing. + - A recursive function has been separated from its setup phase. + + The '_1' suffix can be used as a concise way to indicate these specific + cases. However, it is recommended to find a more descriptive name wherever + possible to improve the readability and maintainability of the code. + For Perl programs: - Most of the C guidelines above apply. -- GitLab From 165834c4f97824f64c14887eb9831392fc332cdb Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Fri, 18 Oct 2024 10:46:45 +0200 Subject: [PATCH 12/34] clang-format: re-adjust line break penalties In 42efde4c29 (clang-format: adjust line break penalties, 2017-09-29) we adjusted the line break penalties to really fine tune what we care about while doing line breaks. Modify some of those to be more inline with what we care about in the Git project now. We need to understand that the values set to penalties in '.clang-format' are relative to each other and do not hold any absolute value. The penalty arguments take an 'Unsigned' value, so we have some liberty over the values we can set. First, in that commit, we decided, that under no circumstances do we want to exceed 80 characters. This seems a bit too strict. We do overshoot this limit from time to time to prioritize readability. So let's reduce the value for 'PenaltyExcessCharacter' to 10. This means we that we add a penalty of 10 for each character that exceeds the column limit. By itself this is enough to restrict to column limit. Tuning other penalties in relation to this is what is important. The penalty `PenaltyBreakAssignment` talks about the penalty for breaking an assignment operator on to the next line. In our project, we are okay with this, so giving a value of 5, which is below the value for 'PenaltyExcessCharacter' ensures that in the end, even 1 character over the column limit is not worth keeping an assignment on the same line. Similarly set the penalty for breaking before the first call parameter 'PenaltyBreakBeforeFirstCallParameter' and the penalty for breaking comments 'PenaltyBreakComment' and the penalty for breaking string literals 'PenaltyBreakString' also to 5. Finally, we really care about not breaking the return type into its own line and we really care about not breaking before an open parenthesis. This avoids weird formatting like: static const struct strbuf * a_really_really_large_function_name(struct strbuf resolved, const char *path, int flags) or static const struct strbuf *a_really_really_large_function_name( struct strbuf resolved, const char *path, int flags) to instead have something more readable like: static const struct strbuf *a_really_really_large_function_name(struct strbuf resolved, const char *path, int flags) (note: the tabs here have been replaced by spaces for easier reading) This is done by bumping the values of 'PenaltyReturnTypeOnItsOwnLine' and 'PenaltyBreakOpenParenthesis' to 300. This is so that we can allow a few characters above the 80 column limit to make code more readable. Signed-off-by: Karthik Nayak Signed-off-by: Taylor Blau --- .clang-format | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/.clang-format b/.clang-format index 41969eca4b7..66a2360ae57 100644 --- a/.clang-format +++ b/.clang-format @@ -209,13 +209,14 @@ KeepEmptyLinesAtTheStartOfBlocks: false # Penalties # This decides what order things should be done if a line is too long -PenaltyBreakAssignment: 10 -PenaltyBreakBeforeFirstCallParameter: 30 -PenaltyBreakComment: 10 +PenaltyBreakAssignment: 5 +PenaltyBreakBeforeFirstCallParameter: 5 +PenaltyBreakComment: 5 PenaltyBreakFirstLessLess: 0 -PenaltyBreakString: 10 -PenaltyExcessCharacter: 100 -PenaltyReturnTypeOnItsOwnLine: 60 +PenaltyBreakOpenParenthesis: 300 +PenaltyBreakString: 5 +PenaltyExcessCharacter: 10 +PenaltyReturnTypeOnItsOwnLine: 300 # Don't sort #include's SortIncludes: false -- GitLab From dbf8d5e81d3093c030619e6d49b00346fadb2001 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Fri, 18 Oct 2024 10:46:46 +0200 Subject: [PATCH 13/34] clang-format: align consecutive macro definitions We generally align consecutive macro definitions for better readability: #define OUTPUT_ANNOTATE_COMPAT (1U<<0) #define OUTPUT_LONG_OBJECT_NAME (1U<<1) #define OUTPUT_RAW_TIMESTAMP (1U<<2) #define OUTPUT_PORCELAIN (1U<<3) over #define OUTPUT_ANNOTATE_COMPAT (1U<<0) #define OUTPUT_LONG_OBJECT_NAME (1U<<1) #define OUTPUT_RAW_TIMESTAMP (1U<<2) #define OUTPUT_PORCELAIN (1U<<3) So let's add the rule in clang-format to follow this. Signed-off-by: Karthik Nayak Signed-off-by: Taylor Blau --- .clang-format | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.clang-format b/.clang-format index 66a2360ae57..9547fe1b77c 100644 --- a/.clang-format +++ b/.clang-format @@ -32,6 +32,9 @@ AlignConsecutiveAssignments: false # double b = 3.14; AlignConsecutiveDeclarations: false +# Align consecutive macro definitions. +AlignConsecutiveMacros: true + # Align escaped newlines as far left as possible # #define A \ # int aaaa; \ -- GitLab From 07e968ff575e7b90389daa91fed2fb649299faad Mon Sep 17 00:00:00 2001 From: Caleb White Date: Tue, 22 Oct 2024 00:08:49 +0000 Subject: [PATCH 14/34] doc: consolidate extensions in git-config documentation The `technical/repository-version.txt` document originally served as the master list for extensions, requiring that any new extensions be defined there. However, the `config/extensions.txt` file was introduced later and has since become the de facto location for describing extensions, with several extensions listed there but missing from `repository-version.txt`. This consolidates all extension definitions into `config/extensions.txt`, making it the authoritative source for extensions. The references in `repository-version.txt` are updated to point to `config/extensions.txt`, and cross-references to related documentation such as `gitrepository-layout[5]` and `git-config[1]` are added. Suggested-by: Junio C Hamano Signed-off-by: Caleb White Signed-off-by: Taylor Blau --- Documentation/config/core.txt | 2 +- Documentation/config/extensions.txt | 73 ++++++++++++++----- Documentation/gitrepository-layout.txt | 1 + .../technical/hash-function-transition.txt | 4 +- Documentation/technical/partial-clone.txt | 2 +- .../technical/repository-version.txt | 44 +---------- 6 files changed, 62 insertions(+), 64 deletions(-) diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index 60ca9f2b686..8f6d8e77541 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -366,7 +366,7 @@ default in a bare repository. core.repositoryFormatVersion:: Internal variable identifying the repository format and layout - version. + version. See linkgit:gitrepository-layout[5]. core.sharedRepository:: When 'group' (or 'true'), the repository is made shareable between diff --git a/Documentation/config/extensions.txt b/Documentation/config/extensions.txt index f0a784447db..5dc569d1c9c 100644 --- a/Documentation/config/extensions.txt +++ b/Documentation/config/extensions.txt @@ -1,17 +1,13 @@ -extensions.objectFormat:: - Specify the hash algorithm to use. The acceptable values are `sha1` and - `sha256`. If not specified, `sha1` is assumed. It is an error to specify - this key unless `core.repositoryFormatVersion` is 1. +extensions.*:: + Unless otherwise stated, is an error to specify an extension if + `core.repositoryFormatVersion` is not `1`. See + linkgit:gitrepository-layout[5]. + -Note that this setting should only be set by linkgit:git-init[1] or -linkgit:git-clone[1]. Trying to change it after initialization will not -work and will produce hard-to-diagnose issues. - -extensions.compatObjectFormat:: - +-- +compatObjectFormat:: Specify a compatibility hash algorithm to use. The acceptable values are `sha1` and `sha256`. The value specified must be different from the - value of extensions.objectFormat. This allows client level + value of `extensions.objectFormat`. This allows client level interoperability between git repositories whose objectFormat matches this compatObjectFormat. In particular when fully implemented the pushes and pulls from a repository in whose objectFormat matches @@ -19,18 +15,55 @@ extensions.compatObjectFormat:: compatObjectFormat in addition to oids encoded with objectFormat to locally specify objects. -extensions.refStorage:: +noop:: + This extension does not change git's behavior at all. It is useful only + for testing format-1 compatibility. ++ +For historical reasons, this extension is respected regardless of the +`core.repositoryFormatVersion` setting. + +noop-v1:: + This extension does not change git's behavior at all. It is useful only + for testing format-1 compatibility. + +objectFormat:: + Specify the hash algorithm to use. The acceptable values are `sha1` and + `sha256`. If not specified, `sha1` is assumed. ++ +Note that this setting should only be set by linkgit:git-init[1] or +linkgit:git-clone[1]. Trying to change it after initialization will not +work and will produce hard-to-diagnose issues. + +partialClone:: + When enabled, indicates that the repo was created with a partial clone + (or later performed a partial fetch) and that the remote may have + omitted sending certain unwanted objects. Such a remote is called a + "promisor remote" and it promises that all such omitted objects can + be fetched from it in the future. ++ +The value of this key is the name of the promisor remote. ++ +For historical reasons, this extension is respected regardless of the +`core.repositoryFormatVersion` setting. + +preciousObjects:: + If enabled, indicates that objects in the repository MUST NOT be deleted + (e.g., by `git-prune` or `git repack -d`). ++ +For historical reasons, this extension is respected regardless of the +`core.repositoryFormatVersion` setting. + +refStorage:: Specify the ref storage format to use. The acceptable values are: + include::../ref-storage-format.txt[] -+ -It is an error to specify this key unless `core.repositoryFormatVersion` is 1. + + Note that this setting should only be set by linkgit:git-init[1] or linkgit:git-clone[1]. Trying to change it after initialization will not work and will produce hard-to-diagnose issues. -extensions.worktreeConfig:: +worktreeConfig:: If enabled, then worktrees will load config settings from the `$GIT_DIR/config.worktree` file in addition to the `$GIT_COMMON_DIR/config` file. Note that `$GIT_COMMON_DIR` and @@ -40,7 +73,7 @@ extensions.worktreeConfig:: `config.worktree` file will override settings from any other config files. + -When enabling `extensions.worktreeConfig`, you must be careful to move +When enabling this extension, you must be careful to move certain values from the common config file to the main working tree's `config.worktree` file, if present: + @@ -48,15 +81,17 @@ certain values from the common config file to the main working tree's `$GIT_COMMON_DIR/config.worktree`. * If `core.bare` is true, then it must be moved from `$GIT_COMMON_DIR/config` to `$GIT_COMMON_DIR/config.worktree`. + + It may also be beneficial to adjust the locations of `core.sparseCheckout` and `core.sparseCheckoutCone` depending on your desire for customizable sparse-checkout settings for each worktree. By default, the `git -sparse-checkout` builtin enables `extensions.worktreeConfig`, assigns +sparse-checkout` builtin enables this extension, assigns these config values on a per-worktree basis, and uses the `$GIT_DIR/info/sparse-checkout` file to specify the sparsity for each worktree independently. See linkgit:git-sparse-checkout[1] for more details. + -For historical reasons, `extensions.worktreeConfig` is respected -regardless of the `core.repositoryFormatVersion` setting. +For historical reasons, this extension is respected regardless of the +`core.repositoryFormatVersion` setting. +-- diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt index 949cd8a31e9..fa8b51daf08 100644 --- a/Documentation/gitrepository-layout.txt +++ b/Documentation/gitrepository-layout.txt @@ -298,6 +298,7 @@ SEE ALSO -------- linkgit:git-init[1], linkgit:git-clone[1], +linkgit:git-config[1], linkgit:git-fetch[1], linkgit:git-pack-refs[1], linkgit:git-gc[1], diff --git a/Documentation/technical/hash-function-transition.txt b/Documentation/technical/hash-function-transition.txt index ed574810891..7102c7c8f5d 100644 --- a/Documentation/technical/hash-function-transition.txt +++ b/Documentation/technical/hash-function-transition.txt @@ -148,8 +148,8 @@ Detailed Design Repository format extension ~~~~~~~~~~~~~~~~~~~~~~~~~~~ A SHA-256 repository uses repository format version `1` (see -Documentation/technical/repository-version.txt) with extensions -`objectFormat` and `compatObjectFormat`: +linkgit:gitrepository-layout[5]) with `extensions.objectFormat` and +`extensions.compatObjectFormat` (see linkgit:git-config[1]) set to: [core] repositoryFormatVersion = 1 diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt index cd948b00722..bf5ec5c82d9 100644 --- a/Documentation/technical/partial-clone.txt +++ b/Documentation/technical/partial-clone.txt @@ -102,7 +102,7 @@ or commits that reference missing trees. - On the client a repository extension is added to the local config to prevent older versions of git from failing mid-operation because of missing objects that they cannot handle. - See "extensions.partialClone" in Documentation/technical/repository-version.txt" + See `extensions.partialClone` in linkgit:git-config[1]. Handling Missing Objects diff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt index 47281420fc4..b9bb81a81f9 100644 --- a/Documentation/technical/repository-version.txt +++ b/Documentation/technical/repository-version.txt @@ -65,44 +65,6 @@ Note that if no extensions are specified in the config file, then provides no benefit, and makes the repository incompatible with older implementations of git). -This document will serve as the master list for extensions. Any -implementation wishing to define a new extension should make a note of -it here, in order to claim the name. - -The defined extensions are: - -==== `noop` - -This extension does not change git's behavior at all. It is useful only -for testing format-1 compatibility. - -==== `preciousObjects` - -When the config key `extensions.preciousObjects` is set to `true`, -objects in the repository MUST NOT be deleted (e.g., by `git-prune` or -`git repack -d`). - -==== `partialClone` - -When the config key `extensions.partialClone` is set, it indicates -that the repo was created with a partial clone (or later performed -a partial fetch) and that the remote may have omitted sending -certain unwanted objects. Such a remote is called a "promisor remote" -and it promises that all such omitted objects can be fetched from it -in the future. - -The value of this key is the name of the promisor remote. - -==== `worktreeConfig` - -If set, by default "git config" reads from both "config" and -"config.worktree" files from GIT_DIR in that order. In -multiple working directory mode, "config" file is shared while -"config.worktree" is per-working directory (i.e., it's in -GIT_COMMON_DIR/worktrees//config.worktree) - -==== `refStorage` - -Specifies the file format for the ref database. The valid values are -`files` (loose references with a packed-refs file) and `reftable` (see -Documentation/technical/reftable.txt). +The defined extensions are given in the `extensions.*` section of +linkgit:git-config[1]. Any implementation wishing to define a new +extension should make a note of it there, in order to claim the name. -- GitLab From bc0ff7da396d4828443233823f1039efb0337c66 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 21 Oct 2024 12:56:33 +0200 Subject: [PATCH 15/34] t/unit-tests: update clar to 206accb Update clar from: - 1516124 (Merge pull request #97 from pks-t/pks-whitespace-fixes, 2024-08-15). To: - 206accb (Merge pull request #108 from pks-t/pks-uclibc-without-wchar, 2024-10-21) This update includes a bunch of fixes and improvements that we have discussed in Git when initial support for clar was merged: - There is a ".editorconfig" file now. - Compatibility with Windows has been improved so that the clar compiles on this platform without an issue. This has been tested with Cygwin, MinGW and Microsoft Visual Studio. - clar now uses CMake. This does not impact us at all as we wire up the clar into our own build infrastructure anyway. This conversion was done such that we can easily run CI jobs against Windows. - Allocation failures are now checked for consistently. - We now define feature test macros in "clar.c", which fixes compilation on some platforms that didn't previously pull in non-standard functions like lstat(3p) or strdup(3p). This was reported by a user of OpenSUSE Leap. - We stop using `struct timezone`, which is undefined behaviour nowadays and results in a compilation error on some platforms. - We now use the combination of mktemp(3) and mkdir(3) on SunOS, same as we do on NonStop. - We now support uClibc without support for . The most important bits here are the improved platform compatibility with Windows, OpenSUSE, SunOS and uClibc. Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau --- t/unit-tests/clar/.editorconfig | 13 +++ t/unit-tests/clar/.github/workflows/ci.yml | 20 +++- t/unit-tests/clar/.gitignore | 1 + t/unit-tests/clar/CMakeLists.txt | 28 +++++ t/unit-tests/clar/clar.c | 127 ++++++++++++--------- t/unit-tests/clar/clar/print.h | 11 +- t/unit-tests/clar/clar/sandbox.h | 17 ++- t/unit-tests/clar/clar/summary.h | 14 +-- t/unit-tests/clar/test/.gitignore | 4 - t/unit-tests/clar/test/CMakeLists.txt | 39 +++++++ t/unit-tests/clar/test/Makefile | 39 ------- 11 files changed, 189 insertions(+), 124 deletions(-) create mode 100644 t/unit-tests/clar/.editorconfig create mode 100644 t/unit-tests/clar/.gitignore create mode 100644 t/unit-tests/clar/CMakeLists.txt delete mode 100644 t/unit-tests/clar/test/.gitignore create mode 100644 t/unit-tests/clar/test/CMakeLists.txt delete mode 100644 t/unit-tests/clar/test/Makefile diff --git a/t/unit-tests/clar/.editorconfig b/t/unit-tests/clar/.editorconfig new file mode 100644 index 00000000000..aa343a42885 --- /dev/null +++ b/t/unit-tests/clar/.editorconfig @@ -0,0 +1,13 @@ +root = true + +[*] +charset = utf-8 +insert_final_newline = true + +[*.{c,h}] +indent_style = tab +tab_width = 8 + +[CMakeLists.txt] +indent_style = tab +tab_width = 8 diff --git a/t/unit-tests/clar/.github/workflows/ci.yml b/t/unit-tests/clar/.github/workflows/ci.yml index b1ac2de460a..0065843d17a 100644 --- a/t/unit-tests/clar/.github/workflows/ci.yml +++ b/t/unit-tests/clar/.github/workflows/ci.yml @@ -10,14 +10,26 @@ jobs: build: strategy: matrix: - os: [ ubuntu-latest, macos-latest ] + platform: + - os: ubuntu-latest + generator: Unix Makefiles + - os: macos-latest + generator: Unix Makefiles + - os: windows-latest + generator: Visual Studio 17 2022 + - os: windows-latest + generator: MSYS Makefiles + - os: windows-latest + generator: MinGW Makefiles - runs-on: ${{ matrix.os }} + runs-on: ${{ matrix.platform.os }} steps: - name: Check out uses: actions/checkout@v2 - name: Build run: | - cd test - make + mkdir build + cd build + cmake .. -G "${{matrix.platform.generator}}" + cmake --build . diff --git a/t/unit-tests/clar/.gitignore b/t/unit-tests/clar/.gitignore new file mode 100644 index 00000000000..84c048a73cc --- /dev/null +++ b/t/unit-tests/clar/.gitignore @@ -0,0 +1 @@ +/build/ diff --git a/t/unit-tests/clar/CMakeLists.txt b/t/unit-tests/clar/CMakeLists.txt new file mode 100644 index 00000000000..12d4af114fe --- /dev/null +++ b/t/unit-tests/clar/CMakeLists.txt @@ -0,0 +1,28 @@ +cmake_minimum_required(VERSION 3.16..3.29) + +project(clar LANGUAGES C) + +option(BUILD_TESTS "Build test executable" ON) + +add_library(clar INTERFACE) +target_sources(clar INTERFACE + clar.c + clar.h + clar/fixtures.h + clar/fs.h + clar/print.h + clar/sandbox.h + clar/summary.h +) +set_target_properties(clar PROPERTIES + C_STANDARD 90 + C_STANDARD_REQUIRED ON + C_EXTENSIONS OFF +) + +if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) + include(CTest) + if(BUILD_TESTING) + add_subdirectory(test) + endif() +endif() diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c index cef0f023c24..d54e4553674 100644 --- a/t/unit-tests/clar/clar.c +++ b/t/unit-tests/clar/clar.c @@ -4,7 +4,12 @@ * This file is part of clar, distributed under the ISC license. * For full terms see the included COPYING file. */ -#include + +#define _BSD_SOURCE +#define _DARWIN_C_SOURCE +#define _DEFAULT_SOURCE + +#include #include #include #include @@ -13,11 +18,22 @@ #include #include #include +#include /* required for sandboxing */ #include #include +#if defined(__UCLIBC__) && ! defined(__UCLIBC_HAS_WCHAR__) + /* + * uClibc can optionally be built without wchar support, in which case + * the installed is a stub that only defines the `whar_t` + * type but none of the functions typically declared by it. + */ +#else +# define CLAR_HAVE_WCHAR +#endif + #ifdef _WIN32 # define WIN32_LEAN_AND_MEAN # include @@ -28,6 +44,9 @@ # ifndef stat # define stat(path, st) _stat(path, st) + typedef struct _stat STAT_T; +# else + typedef struct stat STAT_T; # endif # ifndef mkdir # define mkdir(path, mode) _mkdir(path) @@ -60,30 +79,11 @@ # else # define p_snprintf snprintf # endif - -# ifndef PRIuZ -# define PRIuZ "Iu" -# endif -# ifndef PRIxZ -# define PRIxZ "Ix" -# endif - -# if defined(_MSC_VER) || (defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR)) - typedef struct stat STAT_T; -# else - typedef struct _stat STAT_T; -# endif #else # include /* waitpid(2) */ # include # define _MAIN_CC # define p_snprintf snprintf -# ifndef PRIuZ -# define PRIuZ "zu" -# endif -# ifndef PRIxZ -# define PRIxZ "zx" -# endif typedef struct stat STAT_T; #endif @@ -102,7 +102,7 @@ fixture_path(const char *base, const char *fixture_name); struct clar_error { const char *file; const char *function; - size_t line_number; + uintmax_t line_number; const char *error_msg; char *description; @@ -195,11 +195,12 @@ static void clar_print_shutdown(int test_count, int suite_count, int error_count static void clar_print_error(int num, const struct clar_report *report, const struct clar_error *error); static void clar_print_ontest(const char *suite_name, const char *test_name, int test_number, enum cl_test_status failed); static void clar_print_onsuite(const char *suite_name, int suite_index); +static void clar_print_onabortv(const char *msg, va_list argp); static void clar_print_onabort(const char *msg, ...); /* From clar_sandbox.c */ static void clar_unsandbox(void); -static int clar_sandbox(void); +static void clar_sandbox(void); /* From summary.h */ static struct clar_summary *clar_summary_init(const char *filename); @@ -218,6 +219,15 @@ static int clar_summary_shutdown(struct clar_summary *fp); _clar.trace_payload); \ } while (0) +static void clar_abort(const char *msg, ...) +{ + va_list argp; + va_start(argp, msg); + clar_print_onabortv(msg, argp); + va_end(argp); + exit(-1); +} + void cl_trace_register(cl_trace_cb *cb, void *payload) { _clar.pfn_trace_cb = cb; @@ -271,9 +281,7 @@ static double clar_time_diff(clar_time *start, clar_time *end) static void clar_time_now(clar_time *out) { - struct timezone tz; - - gettimeofday(out, &tz); + gettimeofday(out, NULL); } static double clar_time_diff(clar_time *start, clar_time *end) @@ -386,7 +394,8 @@ clar_run_suite(const struct clar_suite *suite, const char *filter) _clar.active_test = test[i].name; - report = calloc(1, sizeof(struct clar_report)); + if ((report = calloc(1, sizeof(*report))) == NULL) + clar_abort("Failed to allocate report.\n"); report->suite = _clar.active_suite; report->test = _clar.active_test; report->test_number = _clar.tests_ran; @@ -479,9 +488,10 @@ clar_parse_args(int argc, char **argv) switch (action) { case 's': { - struct clar_explicit *explicit = - calloc(1, sizeof(struct clar_explicit)); - assert(explicit); + struct clar_explicit *explicit; + + if ((explicit = calloc(1, sizeof(*explicit))) == NULL) + clar_abort("Failed to allocate explicit test.\n"); explicit->suite_idx = j; explicit->filter = argument; @@ -505,10 +515,8 @@ clar_parse_args(int argc, char **argv) } } - if (!found) { - clar_print_onabort("No suite matching '%s' found.\n", argument); - exit(-1); - } + if (!found) + clar_abort("No suite matching '%s' found.\n", argument); break; } @@ -540,11 +548,17 @@ clar_parse_args(int argc, char **argv) case 'r': _clar.write_summary = 1; free(_clar.summary_filename); - _clar.summary_filename = *(argument + 2) ? strdup(argument + 2) : NULL; + if (*(argument + 2)) { + if ((_clar.summary_filename = strdup(argument + 2)) == NULL) + clar_abort("Failed to allocate summary filename.\n"); + } else { + _clar.summary_filename = NULL; + } break; default: - assert(!"Unexpected commandline argument!"); + clar_abort("Unexpected commandline argument '%s'.\n", + argument[1]); } } } @@ -566,22 +580,18 @@ clar_test_init(int argc, char **argv) if (!_clar.summary_filename && (summary_env = getenv("CLAR_SUMMARY")) != NULL) { _clar.write_summary = 1; - _clar.summary_filename = strdup(summary_env); + if ((_clar.summary_filename = strdup(summary_env)) == NULL) + clar_abort("Failed to allocate summary filename.\n"); } if (_clar.write_summary && !_clar.summary_filename) - _clar.summary_filename = strdup("summary.xml"); + if ((_clar.summary_filename = strdup("summary.xml")) == NULL) + clar_abort("Failed to allocate summary filename.\n"); - if (_clar.write_summary && - !(_clar.summary = clar_summary_init(_clar.summary_filename))) { - clar_print_onabort("Failed to open the summary file\n"); - exit(-1); - } + if (_clar.write_summary) + _clar.summary = clar_summary_init(_clar.summary_filename); - if (clar_sandbox() < 0) { - clar_print_onabort("Failed to sandbox the test runner.\n"); - exit(-1); - } + clar_sandbox(); } int @@ -615,10 +625,9 @@ clar_test_shutdown(void) clar_unsandbox(); - if (_clar.write_summary && clar_summary_shutdown(_clar.summary) < 0) { - clar_print_onabort("Failed to write the summary file\n"); - exit(-1); - } + if (_clar.write_summary && clar_summary_shutdown(_clar.summary) < 0) + clar_abort("Failed to write the summary file '%s: %s.\n", + _clar.summary_filename, strerror(errno)); for (explicit = _clar.explicit; explicit; explicit = explicit_next) { explicit_next = explicit->next; @@ -649,7 +658,7 @@ static void abort_test(void) { if (!_clar.trampoline_enabled) { clar_print_onabort( - "Fatal error: a cleanup method raised an exception."); + "Fatal error: a cleanup method raised an exception.\n"); clar_report_errors(_clar.last_report); exit(-1); } @@ -673,7 +682,10 @@ void clar__fail( const char *description, int should_abort) { - struct clar_error *error = calloc(1, sizeof(struct clar_error)); + struct clar_error *error; + + if ((error = calloc(1, sizeof(*error))) == NULL) + clar_abort("Failed to allocate error.\n"); if (_clar.last_report->errors == NULL) _clar.last_report->errors = error; @@ -688,8 +700,9 @@ void clar__fail( error->line_number = line; error->error_msg = error_msg; - if (description != NULL) - error->description = strdup(description); + if (description != NULL && + (error->description = strdup(description)) == NULL) + clar_abort("Failed to allocate description.\n"); _clar.total_errors++; _clar.last_report->status = CL_TEST_FAILURE; @@ -763,6 +776,7 @@ void clar__assert_equal( } } } +#ifdef CLAR_HAVE_WCHAR else if (!strcmp("%ls", fmt)) { const wchar_t *wcs1 = va_arg(args, const wchar_t *); const wchar_t *wcs2 = va_arg(args, const wchar_t *); @@ -798,8 +812,9 @@ void clar__assert_equal( } } } - else if (!strcmp("%"PRIuZ, fmt) || !strcmp("%"PRIxZ, fmt)) { - size_t sz1 = va_arg(args, size_t), sz2 = va_arg(args, size_t); +#endif /* CLAR_HAVE_WCHAR */ + else if (!strcmp("%"PRIuMAX, fmt) || !strcmp("%"PRIxMAX, fmt)) { + uintmax_t sz1 = va_arg(args, uintmax_t), sz2 = va_arg(args, uintmax_t); is_equal = (sz1 == sz2); if (!is_equal) { int offset = p_snprintf(buf, sizeof(buf), fmt, sz1); diff --git a/t/unit-tests/clar/clar/print.h b/t/unit-tests/clar/clar/print.h index c17e2f693bd..69d0ee967e7 100644 --- a/t/unit-tests/clar/clar/print.h +++ b/t/unit-tests/clar/clar/print.h @@ -21,7 +21,7 @@ static void clar_print_clap_error(int num, const struct clar_report *report, con { printf(" %d) Failure:\n", num); - printf("%s::%s [%s:%"PRIuZ"]\n", + printf("%s::%s [%s:%"PRIuMAX"]\n", report->suite, report->test, error->file, @@ -136,7 +136,7 @@ static void clar_print_tap_ontest(const char *suite_name, const char *test_name, printf(" at:\n"); printf(" file: '"); print_escaped(error->file); printf("'\n"); - printf(" line: %" PRIuZ "\n", error->line_number); + printf(" line: %" PRIuMAX "\n", error->line_number); printf(" function: '%s'\n", error->function); printf(" ---\n"); @@ -202,10 +202,15 @@ static void clar_print_onsuite(const char *suite_name, int suite_index) PRINT(onsuite, suite_name, suite_index); } +static void clar_print_onabortv(const char *msg, va_list argp) +{ + PRINT(onabort, msg, argp); +} + static void clar_print_onabort(const char *msg, ...) { va_list argp; va_start(argp, msg); - PRINT(onabort, msg, argp); + clar_print_onabortv(msg, argp); va_end(argp); } diff --git a/t/unit-tests/clar/clar/sandbox.h b/t/unit-tests/clar/clar/sandbox.h index e25057b7c49..bc960f50e0f 100644 --- a/t/unit-tests/clar/clar/sandbox.h +++ b/t/unit-tests/clar/clar/sandbox.h @@ -122,14 +122,14 @@ static int build_sandbox_path(void) if (mkdir(_clar_path, 0700) != 0) return -1; -#elif defined(__TANDEM) - if (mktemp(_clar_path) == NULL) +#elif defined(_WIN32) + if (_mktemp_s(_clar_path, sizeof(_clar_path)) != 0) return -1; if (mkdir(_clar_path, 0700) != 0) return -1; -#elif defined(_WIN32) - if (_mktemp_s(_clar_path, sizeof(_clar_path)) != 0) +#elif defined(__sun) || defined(__TANDEM) + if (mktemp(_clar_path) == NULL) return -1; if (mkdir(_clar_path, 0700) != 0) @@ -142,15 +142,14 @@ static int build_sandbox_path(void) return 0; } -static int clar_sandbox(void) +static void clar_sandbox(void) { if (_clar_path[0] == '\0' && build_sandbox_path() < 0) - return -1; + clar_abort("Failed to build sandbox path.\n"); if (chdir(_clar_path) != 0) - return -1; - - return 0; + clar_abort("Failed to change into sandbox directory '%s': %s.\n", + _clar_path, strerror(errno)); } const char *clar_sandbox_path(void) diff --git a/t/unit-tests/clar/clar/summary.h b/t/unit-tests/clar/clar/summary.h index 4dd352e28b8..0d0b646fe75 100644 --- a/t/unit-tests/clar/clar/summary.h +++ b/t/unit-tests/clar/clar/summary.h @@ -66,16 +66,12 @@ struct clar_summary *clar_summary_init(const char *filename) struct clar_summary *summary; FILE *fp; - if ((fp = fopen(filename, "w")) == NULL) { - perror("fopen"); - return NULL; - } + if ((fp = fopen(filename, "w")) == NULL) + clar_abort("Failed to open the summary file '%s': %s.\n", + filename, strerror(errno)); - if ((summary = malloc(sizeof(struct clar_summary))) == NULL) { - perror("malloc"); - fclose(fp); - return NULL; - } + if ((summary = malloc(sizeof(struct clar_summary))) == NULL) + clar_abort("Failed to allocate summary.\n"); summary->filename = filename; summary->fp = fp; diff --git a/t/unit-tests/clar/test/.gitignore b/t/unit-tests/clar/test/.gitignore deleted file mode 100644 index a477d0c40ca..00000000000 --- a/t/unit-tests/clar/test/.gitignore +++ /dev/null @@ -1,4 +0,0 @@ -clar.suite -.clarcache -clar_test -*.o diff --git a/t/unit-tests/clar/test/CMakeLists.txt b/t/unit-tests/clar/test/CMakeLists.txt new file mode 100644 index 00000000000..7f2c1dc17a9 --- /dev/null +++ b/t/unit-tests/clar/test/CMakeLists.txt @@ -0,0 +1,39 @@ +find_package(Python COMPONENTS Interpreter REQUIRED) + +add_custom_command(OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/clar.suite" + COMMAND "${Python_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/generate.py" --output "${CMAKE_CURRENT_BINARY_DIR}" + DEPENDS main.c sample.c clar_test.h + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" +) + +add_executable(clar_test) +set_target_properties(clar_test PROPERTIES + C_STANDARD 90 + C_STANDARD_REQUIRED ON + C_EXTENSIONS OFF +) + +# MSVC generates all kinds of warnings. We may want to fix these in the future +# and then unconditionally treat warnings as errors. +if(NOT MSVC) + set_target_properties(clar_test PROPERTIES + COMPILE_WARNING_AS_ERROR ON + ) +endif() + +target_sources(clar_test PRIVATE + main.c + sample.c + "${CMAKE_CURRENT_BINARY_DIR}/clar.suite" +) +target_compile_definitions(clar_test PRIVATE + CLAR_FIXTURE_PATH="${CMAKE_CURRENT_SOURCE_DIR}/resources/" +) +target_compile_options(clar_test PRIVATE + $,/W4,-Wall> +) +target_include_directories(clar_test PRIVATE + "${CMAKE_SOURCE_DIR}" + "${CMAKE_CURRENT_BINARY_DIR}" +) +target_link_libraries(clar_test clar) diff --git a/t/unit-tests/clar/test/Makefile b/t/unit-tests/clar/test/Makefile deleted file mode 100644 index 93c6b2ad32c..00000000000 --- a/t/unit-tests/clar/test/Makefile +++ /dev/null @@ -1,39 +0,0 @@ -# -# Copyright (c) Vicent Marti. All rights reserved. -# -# This file is part of clar, distributed under the ISC license. -# For full terms see the included COPYING file. -# - -# -# Set up the path to the clar sources and to the fixtures directory -# -# The fixture path needs to be an absolute path so it can be used -# even after we have chdir'ed into the test directory while testing. -# -CURRENT_MAKEFILE := $(word $(words $(MAKEFILE_LIST)),$(MAKEFILE_LIST)) -TEST_DIRECTORY := $(abspath $(dir $(CURRENT_MAKEFILE))) -CLAR_PATH := $(dir $(TEST_DIRECTORY)) -CLAR_FIXTURE_PATH := $(TEST_DIRECTORY)/resources/ - -CFLAGS=-g -I.. -I. -Wall -DCLAR_FIXTURE_PATH=\"$(CLAR_FIXTURE_PATH)\" - -.PHONY: clean - -# list the objects that go into our test -objects = main.o sample.o - -# build the test executable itself -clar_test: $(objects) clar_test.h clar.suite $(CLAR_PATH)clar.c - $(CC) $(CFLAGS) -o $@ "$(CLAR_PATH)clar.c" $(objects) - -# test object files depend on clar macros -$(objects) : $(CLAR_PATH)clar.h - -# build the clar.suite file of test metadata -clar.suite: - python "$(CLAR_PATH)generate.py" . - -# remove all generated files -clean: - $(RM) -rf *.o clar.suite .clarcache clar_test clar_test.dSYM -- GitLab From 29c37756cd08ecb921c344f64344af144b7ef292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20R=2E=20Sede=C3=B1o?= Date: Mon, 21 Oct 2024 12:56:35 +0200 Subject: [PATCH 16/34] Makefile: adjust sed command for generating "clar-decls.h" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This moves the end-of-line marker out of the captured group, matching the start-of-line marker and for some reason fixing generation of "clar-decls.h" on some older, more esoteric platforms. Signed-off-by: Alejandro R. Sedeño Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6f5986b66ea..6f2ed87bfe4 100644 --- a/Makefile +++ b/Makefile @@ -3906,7 +3906,7 @@ GIT-TEST-SUITES: FORCE $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES $(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \ - sed -ne "s/^\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)$$\)/extern \1;/p" $(UNIT_TEST_DIR)/$$suite.c; \ + sed -ne "s/^\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$$/extern \1;/p" $(UNIT_TEST_DIR)/$$suite.c; \ done >$@ $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite -- GitLab From e1f9a325135dee3e6edffc8b05fb87388827b552 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 21 Oct 2024 12:56:38 +0200 Subject: [PATCH 17/34] Makefile: extract script to generate clar declarations Extract the script to generate function declarations for the clar unit testing framework into a standalone script. This is done such that we can reuse it in other build systems. Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau --- Makefile | 4 +--- t/unit-tests/generate-clar-decls.sh | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) create mode 100755 t/unit-tests/generate-clar-decls.sh diff --git a/Makefile b/Makefile index 6f2ed87bfe4..d06c9a8ffa7 100644 --- a/Makefile +++ b/Makefile @@ -3905,9 +3905,7 @@ GIT-TEST-SUITES: FORCE fi $(UNIT_TEST_DIR)/clar-decls.h: $(patsubst %,$(UNIT_TEST_DIR)/%.c,$(CLAR_TEST_SUITES)) GIT-TEST-SUITES - $(QUIET_GEN)for suite in $(CLAR_TEST_SUITES); do \ - sed -ne "s/^\(void test_$${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$$/extern \1;/p" $(UNIT_TEST_DIR)/$$suite.c; \ - done >$@ + $(QUIET_GEN)$(SHELL_PATH) $(UNIT_TEST_DIR)/generate-clar-decls.sh "$@" $(filter %.c,$^) $(UNIT_TEST_DIR)/clar.suite: $(UNIT_TEST_DIR)/clar-decls.h $(QUIET_GEN)awk -f $(UNIT_TEST_DIR)/clar-generate.awk $< >$(UNIT_TEST_DIR)/clar.suite $(UNIT_TEST_DIR)/clar/clar.o: $(UNIT_TEST_DIR)/clar.suite diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh new file mode 100755 index 00000000000..688e0885f4f --- /dev/null +++ b/t/unit-tests/generate-clar-decls.sh @@ -0,0 +1,16 @@ +#!/bin/sh + +if test $# -lt 2 +then + echo "USAGE: $0 ..." 2>&1 + exit 1 +fi + +OUTPUT="$1" +shift + +for suite in "$@" +do + sed -ne "s/^\(void test_$(basename "${suite%.c}")__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" || + exit 1 +done >"$OUTPUT" -- GitLab From 08e03fc1a3ea891c2e43d8e375e31a1572a5de8f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 21 Oct 2024 12:56:41 +0200 Subject: [PATCH 18/34] cmake: fix compilation of clar-based unit tests The compilation of clar-based unit tests is broken because we do not add the binary directory into which we generate the "clar-decls.h" and "clar.suite" files as include directories. Instead, we accidentally set up the source directory as include directory. Fix this by including the binary directory instead of the source directory. Furthermore, set up the include directories as PUBLIC instead of PRIVATE such that they propagate from "unit-tests.lib" to the "unit-tests" executable, which needs to include the same directory. Reported-by: Ed Reel Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau --- contrib/buildsystems/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 1384c0eb6d3..ce9281b8357 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1042,7 +1042,7 @@ file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/") list(TRANSFORM clar_test_SUITES APPEND ".c") add_library(unit-tests-lib ${clar_test_SUITES} "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c") -target_include_directories(unit-tests-lib PRIVATE "${CMAKE_SOURCE_DIR}/t/unit-tests") +target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests") add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c") target_link_libraries(unit-tests unit-tests-lib common-main) set_target_properties(unit-tests -- GitLab From d21e754d03765cbd4c8dc139f7aaf3b618a9076c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 21 Oct 2024 12:56:44 +0200 Subject: [PATCH 19/34] cmake: set up proper dependencies for generated clar headers The auto-generated headers used by clar are written at configure time and thus do not get regenerated automatically. Refactor the build recipes such that we use custom commands instead, which also has the benefit that we can reuse the same infrastructure as our Makefile. Signed-off-by: Patrick Steinhardt Signed-off-by: Taylor Blau --- contrib/buildsystems/CMakeLists.txt | 50 +++++++---------------------- 1 file changed, 12 insertions(+), 38 deletions(-) diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index ce9281b8357..8974bb9fa20 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -1002,46 +1002,20 @@ foreach(unit_test ${unit_test_PROGRAMS}) endforeach() parse_makefile_for_scripts(clar_test_SUITES "CLAR_TEST_SUITES" "") - -set(clar_decls "") -set(clar_cbs "") -set(clar_cbs_count 0) -set(clar_suites "static struct clar_suite _clar_suites[] = {\n") -list(LENGTH clar_test_SUITES clar_suites_count) -foreach(suite ${clar_test_SUITES}) - file(STRINGS "${CMAKE_SOURCE_DIR}/t/unit-tests/${suite}.c" decls - REGEX "^void test_${suite}__[a-zA-Z_0-9][a-zA-Z_0-9]*\\(void\\)$") - - list(LENGTH decls decls_count) - string(REGEX REPLACE "void (test_${suite}__([a-zA-Z_0-9]*))\\(void\\)" " { \"\\2\", &\\1 },\n" cbs ${decls}) - string(JOIN "" cbs ${cbs}) - list(TRANSFORM decls PREPEND "extern ") - string(JOIN ";\n" decls ${decls}) - - string(APPEND clar_decls "${decls};\n") - string(APPEND clar_cbs - "static const struct clar_func _clar_cb_${suite}[] = {\n" - ${cbs} - "};\n") - string(APPEND clar_suites - " {\n" - " \"${suite}\",\n" - " { NULL, NULL },\n" - " { NULL, NULL },\n" - " _clar_cb_${suite}, ${decls_count}, 1\n" - " },\n") - math(EXPR clar_cbs_count "${clar_cbs_count}+${decls_count}") -endforeach() -string(APPEND clar_suites - "};\n" - "static const size_t _clar_suite_count = ${clar_suites_count};\n" - "static const size_t _clar_callback_count = ${clar_cbs_count};\n") -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" "${clar_decls}") -file(WRITE "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" "${clar_decls}" "${clar_cbs}" "${clar_suites}") - list(TRANSFORM clar_test_SUITES PREPEND "${CMAKE_SOURCE_DIR}/t/unit-tests/") list(TRANSFORM clar_test_SUITES APPEND ".c") -add_library(unit-tests-lib ${clar_test_SUITES} "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c") +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" + COMMAND ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" ${clar_test_SUITES} + DEPENDS ${CMAKE_SOURCE_DIR}/t/unit-tests/generate-clar-decls.sh ${clar_test_SUITES}) +add_custom_command(OUTPUT "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" + COMMAND awk -f "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" > "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" + DEPENDS "${CMAKE_SOURCE_DIR}/t/unit-tests/clar-generate.awk" "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h") + +add_library(unit-tests-lib ${clar_test_SUITES} + "${CMAKE_SOURCE_DIR}/t/unit-tests/clar/clar.c" + "${CMAKE_BINARY_DIR}/t/unit-tests/clar-decls.h" + "${CMAKE_BINARY_DIR}/t/unit-tests/clar.suite" +) target_include_directories(unit-tests-lib PUBLIC "${CMAKE_BINARY_DIR}/t/unit-tests") add_executable(unit-tests "${CMAKE_SOURCE_DIR}/t/unit-tests/unit-test.c") target_link_libraries(unit-tests unit-tests-lib common-main) -- GitLab From 0c93147caa6477488b89137e82551fc64690eba5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 1 Nov 2024 08:16:06 -0400 Subject: [PATCH 20/34] rev-list: skip bitmap traversal for --left-right Running: git rev-list --left-right --use-bitmap-index one...two will produce output without any left-right markers, since the bitmap traversal returns only a single set of reachable commits. Instead we should refuse to use bitmaps here and produce the correct output using a traditional traversal. This is probably not the only remaining option that misbehaves with bitmaps, but it's particularly egregious in that it feels like it _could_ work. Doing two separate traversals for the left/right sides and then taking the symmetric set differences should yield the correct answer, but our traversal code doesn't know how to do that. It's not clear if naively doing two separate traversals would always be a performance win. A traditional traversal only needs to walk down to the merge base, but bitmaps always fill out the full reachability set. So depending on your bitmap coverage, we could end up walking old bits of history twice to fill out the same uninteresting bits on both sides. We'd also of course end up with a very large --boundary set, if the user asked for that. So this might or might not be something worth implementing later. But for now, let's make sure we don't produce the wrong answer if somebody tries it. The test covers this, but also the same thing with "--count" (which is what I originally tried in a real-world case). Ironically the try_bitmap_count() code already realizes that "--left-right" won't work there. But that just causes us to fall back to the regular bitmap traversal code, which itself doesn't handle counting (we produce a list of objects rather than a count). Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- builtin/rev-list.c | 7 +++++++ t/t5310-pack-bitmaps.sh | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 43c42621e3d..8fe83893fad 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -485,6 +485,13 @@ static int try_bitmap_traversal(struct rev_info *revs, if (revs->max_count >= 0) return -1; + /* + * We can't know which commits were left/right in a single traversal, + * and we don't yet know how to traverse them separately. + */ + if (revs->left_right) + return -1; + bitmap_git = prepare_bitmap_walk(revs, filter_provided_objects); if (!bitmap_git) return -1; diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index 7044c7d7c6d..6bcbea64cc3 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -503,6 +503,18 @@ test_expect_success 'boundary-based traversal is used when requested' ' done ' +test_expect_success 'left-right not confused by bitmap index' ' + git rev-list --left-right other...HEAD >expect && + git rev-list --use-bitmap-index --left-right other...HEAD >actual && + test_cmp expect actual +' + +test_expect_success 'left-right count not confused by bitmap-index' ' + git rev-list --left-right --count other...HEAD >expect && + git rev-list --use-bitmap-index --left-right --count other...HEAD >actual && + test_cmp expect actual +' + test_bitmap_cases "pack.writeBitmapLookupTable" test_expect_success 'verify writing bitmap lookup table when enabled' ' -- GitLab From 3852493cf463c7d2cb9f19ca60c3153634a693b0 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 8 Nov 2024 12:55:48 +0900 Subject: [PATCH 21/34] The eighth batch Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.48.0.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/RelNotes/2.48.0.txt b/Documentation/RelNotes/2.48.0.txt index 75698862c7e..9e882c83528 100644 --- a/Documentation/RelNotes/2.48.0.txt +++ b/Documentation/RelNotes/2.48.0.txt @@ -16,6 +16,7 @@ UI, Workflows & Features * Teach 'git notes add' and 'git notes append' a new '-e' flag, instructing them to open the note in $GIT_EDITOR before saving. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -68,6 +69,16 @@ Performance, Internal Implementation, Development Support etc. * Documentation updates to 'git-update-ref(1)'. + * Update the project's CodingGuidelines to discourage naming functions + with a "_1()" suffix. + + * Updates the '.clang-format' to match project conventions. + + * Centralize documentation for repository extensions into a single place. + + * Buildfix and upgrade of Clar to a newer version. + + Fixes since v2.47 ----------------- @@ -114,6 +125,10 @@ Fixes since v2.47 relying on: the *.idx file we got from the remote. (merge 863f2459a2 jk/dumb-http-finalize later to maint). + * When called with '--left-right' and '--use-bitmap-index', 'rev-list' + will produce output without any left/right markers, which has been + corrected. + * Other code cleanup, docfix, build fix, etc. (merge 66893a14d0 ps/leakfixes-part-8 later to maint). (merge 1164e270b5 jk/output-prefix-cleanup later to maint). -- GitLab From 582626834e3c77329ebcb669189b7a2530f2ba1b Mon Sep 17 00:00:00 2001 From: Tobias Boesch Date: Thu, 12 Sep 2024 10:17:57 +0000 Subject: [PATCH 22/34] git gui: add directly calling merge tool from configuration git gui can open a merge tool when conflicts are detected (Right click in the diff of the file with conflicts). The merge tools that are allowed to use are hard coded into git gui. If one wants to add a new merge tool it has to be added to git gui through a source code change. This is not convenient in comparison to how it works in git (without gui). git itself has configuration options for a merge tools path and command in the git configuration. New merge tools can be set up there without a source code change. Those options are used only by pure git in contrast to git gui. git calls the configured merge tools directly from the configuration while git Gui doesn't. With this change git gui can call merge tools configured in the configuration directly without a change in git gui source code. It needs a configured "merge.tool" and a configured "mergetool..cmd" configuration entry as shown in the git-config manual page. Configuration example: [merge] tool = vscode [mergetool "vscode"] cmd = \"the/path/to/Code.exe\" --wait --merge \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$MERGED\" Without the "mergetool..cmd" entry and an unsupported "merge.tool" entry, git gui behaves mainly as before this change and informs the user about an unsupported merge tool. In addtition, it also shows a hint to add a configuration entry to use the tool as an unsupported tool with degraded support. If a wrong "mergetool..cmd" is configured by accident, it gets handled by git gui already. In this case git gui informs the user that the merge tool couldn't be opened. This behavior is preserved by this change and should not change. "Beyond Compare 3" and "Visual Studio Code" were tested as manually configured merge tools. Signed-off-by: Tobias Boesch Signed-off-by: Johannes Sixt --- git-gui/lib/mergetool.tcl | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl index e688b016ef6..8b8c16b1d61 100644 --- a/git-gui/lib/mergetool.tcl +++ b/git-gui/lib/mergetool.tcl @@ -272,8 +272,25 @@ proc merge_resolve_tool2 {} { } } default { - error_popup [mc "Unsupported merge tool '%s'" $tool] - return + set tool_cmd [get_config mergetool.$tool.cmd] + if {$tool_cmd ne {}} { + if {([string first {[} $tool_cmd] != -1) || ([string first {]} $tool_cmd] != -1)} { + error_popup [mc "Unable to process square brackets in \"mergetool.%s.cmd\" configuration option. + +Please remove the square brackets." $tool] + return + } else { + set cmdline {} + foreach command_part $tool_cmd { + lappend cmdline [subst -nobackslashes -nocommands $command_part] + } + } + } else { + error_popup [mc "Unsupported merge tool '%s'. + +To use this tool, configure \"mergetool.%s.cmd\" as shown in the git-config manual page." $tool $tool] + return + } } } -- GitLab From ddb29146643c2deaebc855472704a9bc8da52c5a Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Tue, 13 Aug 2024 11:06:30 +0200 Subject: [PATCH 23/34] git-gui: strip comments and consecutive empty lines from commit messages This is also known as "washing". This is consistent with the behavior of interactive git commit, which we should emulate as closely as possible to avoid usability problems. This way commit message templates and prepare hooks can be used properly, and comments from conflicted rebases and merges are cleaned up without having to introduce special handling for them. Signed-off-by: Oswald Buddenhagen Signed-off-by: Johannes Sixt --- git-gui/lib/commit.tcl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl index 11379f8ad35..f00a6346242 100644 --- a/git-gui/lib/commit.tcl +++ b/git-gui/lib/commit.tcl @@ -209,6 +209,10 @@ You must stage at least 1 file before you can commit. # set msg [string trim [$ui_comm get 1.0 end]] regsub -all -line {[ \t\r]+$} $msg {} msg + # Strip comment lines + regsub -all {(^|\n)#[^\n]*} $msg {\1} msg + # Compress consecutive empty lines + regsub -all {\n{3,}} $msg "\n\n" msg if {$msg eq {}} { error_popup [mc "Please supply a commit message. -- GitLab From aaaf18565ec780ddcc5552ac6aa66f0ea2bdffb6 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Tue, 13 Aug 2024 11:06:31 +0200 Subject: [PATCH 24/34] git-gui: strip commit messages less aggressively We would strip all leading and trailing whitespace, which git commit does not. Let's be consistent here. Signed-off-by: Oswald Buddenhagen Signed-off-by: Johannes Sixt --- git-gui/lib/commit.tcl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl index f00a6346242..208dc2817ca 100644 --- a/git-gui/lib/commit.tcl +++ b/git-gui/lib/commit.tcl @@ -207,12 +207,17 @@ You must stage at least 1 file before you can commit. # -- A message is required. # - set msg [string trim [$ui_comm get 1.0 end]] + set msg [$ui_comm get 1.0 end] + # Strip trailing whitespace regsub -all -line {[ \t\r]+$} $msg {} msg # Strip comment lines regsub -all {(^|\n)#[^\n]*} $msg {\1} msg + # Strip leading empty lines + regsub {^\n*} $msg {} msg # Compress consecutive empty lines regsub -all {\n{3,}} $msg "\n\n" msg + # Strip trailing empty line + regsub {\n\n$} $msg "\n" msg if {$msg eq {}} { error_popup [mc "Please supply a commit message. -- GitLab From 102dcc939af452e00f828aed4a571da0b0ffa9e7 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 25 Nov 2024 15:55:30 +0100 Subject: [PATCH 25/34] builtin: pass repository to sub commands In 9b1cb5070f (builtin: add a repository parameter for builtin functions, 2024-09-13) the repository was passed down to all builtin commands. This allowed the repository to be passed down to lower layers without depending on the global `the_repository` variable. Continue this work by also passing down the repository parameter from the command to sub-commands. This will help pass down the repository to other subsystems and cleanup usage of global variables like 'the_repository' and 'the_hash_algo'. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- builtin/bisect.c | 32 +++++++++++++++--------- builtin/bundle.c | 16 +++++++----- builtin/commit-graph.c | 10 +++++--- builtin/config.c | 25 ++++++++++++------- builtin/gc.c | 21 ++++++++++------ builtin/hook.c | 7 +++--- builtin/multi-pack-index.c | 16 +++++++----- builtin/notes.c | 36 +++++++++++++++++---------- builtin/reflog.c | 17 ++++++++----- builtin/refs.c | 10 +++++--- builtin/remote.c | 34 +++++++++++++++++--------- builtin/sparse-checkout.c | 25 ++++++++++++------- builtin/stash.c | 39 ++++++++++++++++++----------- builtin/submodule--helper.c | 46 +++++++++++++++++++++++------------ builtin/worktree.c | 28 +++++++++++++-------- parse-options.h | 4 ++- t/helper/test-parse-options.c | 8 +++--- 17 files changed, 239 insertions(+), 135 deletions(-) diff --git a/builtin/bisect.c b/builtin/bisect.c index 21d17a6c1a8..8166d4abf53 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -1312,7 +1312,8 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv) return res; } -static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNUSED) +static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { if (argc > 1) return error(_("'%s' requires either no argument or a commit"), @@ -1320,7 +1321,8 @@ static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNU return bisect_reset(argc ? argv[0] : NULL); } -static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNUSED) +static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { int res; struct bisect_terms terms = { 0 }; @@ -1333,7 +1335,8 @@ static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNU return res; } -static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNUSED) +static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { int res; struct bisect_terms terms = { 0 }; @@ -1344,7 +1347,8 @@ static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNU return res; } -static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *prefix) +static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *prefix, + struct repository *repo UNUSED) { int res; struct bisect_terms terms = { 0 }; @@ -1358,12 +1362,15 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref return res; } -static int cmd_bisect__log(int argc UNUSED, const char **argv UNUSED, const char *prefix UNUSED) +static int cmd_bisect__log(int argc UNUSED, const char **argv UNUSED, + const char *prefix UNUSED, + struct repository *repo UNUSED) { return bisect_log(); } -static int cmd_bisect__replay(int argc, const char **argv, const char *prefix UNUSED) +static int cmd_bisect__replay(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { int res; struct bisect_terms terms = { 0 }; @@ -1376,7 +1383,8 @@ static int cmd_bisect__replay(int argc, const char **argv, const char *prefix UN return res; } -static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUSED) +static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { int res; struct bisect_terms terms = { 0 }; @@ -1388,7 +1396,8 @@ static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUS return res; } -static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix UNUSED) +static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { int res; struct bisect_terms terms = { 0 }; @@ -1399,7 +1408,8 @@ static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix return res; } -static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSED) +static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { int res; struct bisect_terms terms = { 0 }; @@ -1415,7 +1425,7 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE int cmd_bisect(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { int res = 0; parse_opt_subcommand_fn *fn = NULL; @@ -1451,7 +1461,7 @@ int cmd_bisect(int argc, } else { argc--; argv++; - res = fn(argc, argv, prefix); + res = fn(argc, argv, prefix, repo); } return is_bisect_success(res) ? 0 : -res; diff --git a/builtin/bundle.c b/builtin/bundle.c index 127518c2a8d..3f14754197c 100644 --- a/builtin/bundle.c +++ b/builtin/bundle.c @@ -67,7 +67,8 @@ static int parse_options_cmd_bundle(int argc, return argc; } -static int cmd_bundle_create(int argc, const char **argv, const char *prefix) { +static int cmd_bundle_create(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct strvec pack_opts = STRVEC_INIT; int version = -1; int ret; @@ -123,7 +124,8 @@ static int open_bundle(const char *path, struct bundle_header *header, return read_bundle_header(path, header); } -static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) { +static int cmd_bundle_verify(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct bundle_header header = BUNDLE_HEADER_INIT; int bundle_fd = -1; int quiet = 0; @@ -164,7 +166,8 @@ static int cmd_bundle_verify(int argc, const char **argv, const char *prefix) { return ret; } -static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix) { +static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct bundle_header header = BUNDLE_HEADER_INIT; int bundle_fd = -1; int ret; @@ -189,7 +192,8 @@ static int cmd_bundle_list_heads(int argc, const char **argv, const char *prefix return ret; } -static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) { +static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct bundle_header header = BUNDLE_HEADER_INIT; int bundle_fd = -1; int ret; @@ -231,7 +235,7 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) int cmd_bundle(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option options[] = { @@ -247,5 +251,5 @@ int cmd_bundle(int argc, packet_trace_identity("bundle"); - return !!fn(argc, argv, prefix); + return !!fn(argc, argv, prefix, repo); } diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 7c991db6eb4..bd70d052e70 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -62,7 +62,8 @@ static struct option *add_common_options(struct option *to) return parse_options_concat(common_opts, to); } -static int graph_verify(int argc, const char **argv, const char *prefix) +static int graph_verify(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct commit_graph *graph = NULL; struct object_directory *odb = NULL; @@ -214,7 +215,8 @@ static int git_commit_graph_write_config(const char *var, const char *value, return 0; } -static int graph_write(int argc, const char **argv, const char *prefix) +static int graph_write(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct string_list pack_indexes = STRING_LIST_INIT_DUP; struct strbuf buf = STRBUF_INIT; @@ -333,7 +335,7 @@ static int graph_write(int argc, const char **argv, const char *prefix) int cmd_commit_graph(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option builtin_commit_graph_options[] = { @@ -352,5 +354,5 @@ int cmd_commit_graph(int argc, builtin_commit_graph_usage, 0); FREE_AND_NULL(options); - return fn(argc, argv, prefix); + return fn(argc, argv, prefix, repo); } diff --git a/builtin/config.c b/builtin/config.c index cba70221081..16e6e305559 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -826,7 +826,8 @@ static void display_options_init(struct config_display_options *opts) } } -static int cmd_config_list(int argc, const char **argv, const char *prefix) +static int cmd_config_list(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct config_location_options location_opts = CONFIG_LOCATION_OPTIONS_INIT; struct config_display_options display_opts = CONFIG_DISPLAY_OPTIONS_INIT; @@ -861,7 +862,8 @@ static int cmd_config_list(int argc, const char **argv, const char *prefix) return 0; } -static int cmd_config_get(int argc, const char **argv, const char *prefix) +static int cmd_config_get(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct config_location_options location_opts = CONFIG_LOCATION_OPTIONS_INIT; struct config_display_options display_opts = CONFIG_DISPLAY_OPTIONS_INIT; @@ -915,7 +917,8 @@ static int cmd_config_get(int argc, const char **argv, const char *prefix) return ret; } -static int cmd_config_set(int argc, const char **argv, const char *prefix) +static int cmd_config_set(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct config_location_options location_opts = CONFIG_LOCATION_OPTIONS_INIT; const char *value_pattern = NULL, *comment_arg = NULL; @@ -973,7 +976,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix) return ret; } -static int cmd_config_unset(int argc, const char **argv, const char *prefix) +static int cmd_config_unset(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct config_location_options location_opts = CONFIG_LOCATION_OPTIONS_INIT; const char *value_pattern = NULL; @@ -1010,7 +1014,8 @@ static int cmd_config_unset(int argc, const char **argv, const char *prefix) return ret; } -static int cmd_config_rename_section(int argc, const char **argv, const char *prefix) +static int cmd_config_rename_section(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct config_location_options location_opts = CONFIG_LOCATION_OPTIONS_INIT; struct option opts[] = { @@ -1039,7 +1044,8 @@ static int cmd_config_rename_section(int argc, const char **argv, const char *pr return ret; } -static int cmd_config_remove_section(int argc, const char **argv, const char *prefix) +static int cmd_config_remove_section(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct config_location_options location_opts = CONFIG_LOCATION_OPTIONS_INIT; struct option opts[] = { @@ -1099,7 +1105,8 @@ static int show_editor(struct config_location_options *opts) return 0; } -static int cmd_config_edit(int argc, const char **argv, const char *prefix) +static int cmd_config_edit(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct config_location_options location_opts = CONFIG_LOCATION_OPTIONS_INIT; struct option opts[] = { @@ -1395,7 +1402,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix) int cmd_config(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *subcommand = NULL; struct option subcommand_opts[] = { @@ -1422,7 +1429,7 @@ int cmd_config(int argc, if (subcommand) { argc = parse_options(argc, argv, prefix, subcommand_opts, builtin_config_usage, PARSE_OPT_SUBCOMMAND_OPTIONAL|PARSE_OPT_KEEP_UNKNOWN_OPT); - return subcommand(argc, argv, prefix); + return subcommand(argc, argv, prefix, repo); } return cmd_config_actions(argc, argv, prefix); diff --git a/builtin/gc.c b/builtin/gc.c index 09802eb9894..50ccab17236 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1567,7 +1567,8 @@ static int task_option_parse(const struct option *opt UNUSED, return 0; } -static int maintenance_run(int argc, const char **argv, const char *prefix) +static int maintenance_run(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i; struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; @@ -1629,7 +1630,8 @@ static char const * const builtin_maintenance_register_usage[] = { NULL }; -static int maintenance_register(int argc, const char **argv, const char *prefix) +static int maintenance_register(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { char *config_file = NULL; struct option options[] = { @@ -1693,7 +1695,8 @@ static char const * const builtin_maintenance_unregister_usage[] = { NULL }; -static int maintenance_unregister(int argc, const char **argv, const char *prefix) +static int maintenance_unregister(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int force = 0; char *config_file = NULL; @@ -2923,7 +2926,8 @@ static const char *const builtin_maintenance_start_usage[] = { NULL }; -static int maintenance_start(int argc, const char **argv, const char *prefix) +static int maintenance_start(int argc, const char **argv, const char *prefix, + struct repository *repo) { struct maintenance_start_opts opts = { 0 }; struct option options[] = { @@ -2946,7 +2950,7 @@ static int maintenance_start(int argc, const char **argv, const char *prefix) if (update_background_schedule(&opts, 1)) die(_("failed to set up maintenance schedule")); - if (maintenance_register(ARRAY_SIZE(register_args)-1, register_args, NULL)) + if (maintenance_register(ARRAY_SIZE(register_args)-1, register_args, NULL, repo)) warning(_("failed to add repo to global config")); return 0; } @@ -2956,7 +2960,8 @@ static const char *const builtin_maintenance_stop_usage[] = { NULL }; -static int maintenance_stop(int argc, const char **argv, const char *prefix) +static int maintenance_stop(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() @@ -2976,7 +2981,7 @@ static const char * const builtin_maintenance_usage[] = { int cmd_maintenance(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option builtin_maintenance_options[] = { @@ -2990,5 +2995,5 @@ int cmd_maintenance(int argc, argc = parse_options(argc, argv, prefix, builtin_maintenance_options, builtin_maintenance_usage, 0); - return fn(argc, argv, prefix); + return fn(argc, argv, prefix, repo); } diff --git a/builtin/hook.c b/builtin/hook.c index 367ef3e0b89..672d2e37e84 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -19,7 +19,8 @@ static const char * const builtin_hook_run_usage[] = { NULL }; -static int run(int argc, const char **argv, const char *prefix) +static int run(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i; struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; @@ -70,7 +71,7 @@ static int run(int argc, const char **argv, const char *prefix) int cmd_hook(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option builtin_hook_options[] = { @@ -81,5 +82,5 @@ int cmd_hook(int argc, argc = parse_options(argc, argv, NULL, builtin_hook_options, builtin_hook_usage, 0); - return fn(argc, argv, prefix); + return fn(argc, argv, prefix, repo); } diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c index d159ed1314d..85e40a4b6d3 100644 --- a/builtin/multi-pack-index.c +++ b/builtin/multi-pack-index.c @@ -119,7 +119,8 @@ static void read_packs_from_stdin(struct string_list *to) } static int cmd_multi_pack_index_write(int argc, const char **argv, - const char *prefix) + const char *prefix, + struct repository *repo UNUSED) { struct option *options; static struct option builtin_multi_pack_index_write_options[] = { @@ -183,7 +184,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv, } static int cmd_multi_pack_index_verify(int argc, const char **argv, - const char *prefix) + const char *prefix, + struct repository *repo UNUSED) { struct option *options; static struct option builtin_multi_pack_index_verify_options[] = { @@ -210,7 +212,8 @@ static int cmd_multi_pack_index_verify(int argc, const char **argv, } static int cmd_multi_pack_index_expire(int argc, const char **argv, - const char *prefix) + const char *prefix, + struct repository *repo UNUSED) { struct option *options; static struct option builtin_multi_pack_index_expire_options[] = { @@ -237,7 +240,8 @@ static int cmd_multi_pack_index_expire(int argc, const char **argv, } static int cmd_multi_pack_index_repack(int argc, const char **argv, - const char *prefix) + const char *prefix, + struct repository *repo UNUSED) { struct option *options; static struct option builtin_multi_pack_index_repack_options[] = { @@ -271,7 +275,7 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv, int cmd_multi_pack_index(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { int res; parse_opt_subcommand_fn *fn = NULL; @@ -297,7 +301,7 @@ int cmd_multi_pack_index(int argc, builtin_multi_pack_index_usage, 0); FREE_AND_NULL(options); - res = fn(argc, argv, prefix); + res = fn(argc, argv, prefix, repo); free(opts.object_dir); return res; diff --git a/builtin/notes.c b/builtin/notes.c index 72c8a51cfac..d051abf6dff 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -431,7 +431,8 @@ static struct notes_tree *init_notes_check(const char *subcommand, return t; } -static int list(int argc, const char **argv, const char *prefix) +static int list(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct notes_tree *t; struct object_id object; @@ -468,9 +469,11 @@ static int list(int argc, const char **argv, const char *prefix) return retval; } -static int append_edit(int argc, const char **argv, const char *prefix); +static int append_edit(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED); -static int add(int argc, const char **argv, const char *prefix) +static int add(int argc, const char **argv, const char *prefix, + struct repository *repo) { int force = 0, allow_empty = 0; const char *object_ref; @@ -543,7 +546,7 @@ static int add(int argc, const char **argv, const char *prefix) * argv[0-1]. */ argv[0] = "edit"; - return append_edit(argc, argv, prefix); + return append_edit(argc, argv, prefix, repo); } fprintf(stderr, _("Overwriting existing notes for object %s\n"), oid_to_hex(&object)); @@ -569,7 +572,8 @@ static int add(int argc, const char **argv, const char *prefix) return 0; } -static int copy(int argc, const char **argv, const char *prefix) +static int copy(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int retval = 0, force = 0, from_stdin = 0; const struct object_id *from_note, *note; @@ -646,7 +650,8 @@ static int copy(int argc, const char **argv, const char *prefix) return retval; } -static int append_edit(int argc, const char **argv, const char *prefix) +static int append_edit(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int allow_empty = 0; const char *object_ref; @@ -749,7 +754,8 @@ static int append_edit(int argc, const char **argv, const char *prefix) return 0; } -static int show(int argc, const char **argv, const char *prefix) +static int show(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { const char *object_ref; struct notes_tree *t; @@ -875,7 +881,8 @@ static int git_config_get_notes_strategy(const char *key, return 0; } -static int merge(int argc, const char **argv, const char *prefix) +static int merge(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT; struct object_id result_oid; @@ -1016,7 +1023,8 @@ static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag return (flag & IGNORE_MISSING) ? 0 : status; } -static int remove_cmd(int argc, const char **argv, const char *prefix) +static int remove_cmd(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { unsigned flag = 0; int from_stdin = 0; @@ -1059,7 +1067,8 @@ static int remove_cmd(int argc, const char **argv, const char *prefix) return retval; } -static int prune(int argc, const char **argv, const char *prefix) +static int prune(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct notes_tree *t; int show_only = 0, verbose = 0; @@ -1088,7 +1097,8 @@ static int prune(int argc, const char **argv, const char *prefix) return 0; } -static int get_ref(int argc, const char **argv, const char *prefix) +static int get_ref(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() }; char *notes_ref; @@ -1109,7 +1119,7 @@ static int get_ref(int argc, const char **argv, const char *prefix) int cmd_notes(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { const char *override_notes_ref = NULL; parse_opt_subcommand_fn *fn = NULL; @@ -1148,5 +1158,5 @@ int cmd_notes(int argc, strbuf_release(&sb); } - return !!fn(argc, argv, prefix); + return !!fn(argc, argv, prefix, repo); } diff --git a/builtin/reflog.c b/builtin/reflog.c index 22df6834f71..5a0c22f2f7e 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -234,7 +234,8 @@ static int expire_total_callback(const struct option *opt, return 0; } -static int cmd_reflog_show(int argc, const char **argv, const char *prefix) +static int cmd_reflog_show(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() @@ -253,7 +254,8 @@ static int show_reflog(const char *refname, void *cb_data UNUSED) return 0; } -static int cmd_reflog_list(int argc, const char **argv, const char *prefix) +static int cmd_reflog_list(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() @@ -270,7 +272,8 @@ static int cmd_reflog_list(int argc, const char **argv, const char *prefix) return refs_for_each_reflog(ref_store, show_reflog, NULL); } -static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) +static int cmd_reflog_expire(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct cmd_reflog_expire_cb cmd = { 0 }; timestamp_t now = time(NULL); @@ -394,7 +397,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) return status; } -static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) +static int cmd_reflog_delete(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i, status = 0; unsigned int flags = 0; @@ -424,7 +428,8 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) return status; } -static int cmd_reflog_exists(int argc, const char **argv, const char *prefix) +static int cmd_reflog_exists(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() @@ -467,7 +472,7 @@ int cmd_reflog(int argc, PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN_OPT); if (fn) - return fn(argc - 1, argv + 1, prefix); + return fn(argc - 1, argv + 1, prefix, repository); else return cmd_log_reflog(argc, argv, prefix, repository); } diff --git a/builtin/refs.c b/builtin/refs.c index 24978a7b7b0..3502980d21d 100644 --- a/builtin/refs.c +++ b/builtin/refs.c @@ -12,7 +12,8 @@ #define REFS_VERIFY_USAGE \ N_("git refs verify [--strict] [--verbose]") -static int cmd_refs_migrate(int argc, const char **argv, const char *prefix) +static int cmd_refs_migrate(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { const char * const migrate_usage[] = { REFS_MIGRATE_USAGE, @@ -63,7 +64,8 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix) return err; } -static int cmd_refs_verify(int argc, const char **argv, const char *prefix) +static int cmd_refs_verify(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct fsck_options fsck_refs_options = FSCK_REFS_OPTIONS_DEFAULT; const char * const verify_usage[] = { @@ -93,7 +95,7 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix) int cmd_refs(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { const char * const refs_usage[] = { REFS_MIGRATE_USAGE, @@ -108,5 +110,5 @@ int cmd_refs(int argc, }; argc = parse_options(argc, argv, prefix, opts, refs_usage, 0); - return fn(argc, argv, prefix); + return fn(argc, argv, prefix, repo); } diff --git a/builtin/remote.c b/builtin/remote.c index 76670ddd8b4..56214775df4 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -155,7 +155,8 @@ static int parse_mirror_opt(const struct option *opt, const char *arg, int not) return 0; } -static int add(int argc, const char **argv, const char *prefix) +static int add(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int fetch = 0, fetch_tags = TAGS_DEFAULT; unsigned mirror = MIRROR_NONE; @@ -706,7 +707,8 @@ static void handle_push_default(const char* old_name, const char* new_name) } -static int mv(int argc, const char **argv, const char *prefix) +static int mv(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int show_progress = isatty(2); struct option options[] = { @@ -881,7 +883,8 @@ static int mv(int argc, const char **argv, const char *prefix) return result; } -static int rm(int argc, const char **argv, const char *prefix) +static int rm(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() @@ -1303,7 +1306,8 @@ static int show_all(void) return result; } -static int show(int argc, const char **argv, const char *prefix) +static int show(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int no_query = 0, result = 0, query_flag = 0; struct option options[] = { @@ -1399,7 +1403,8 @@ static int show(int argc, const char **argv, const char *prefix) return result; } -static int set_head(int argc, const char **argv, const char *prefix) +static int set_head(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i, opt_a = 0, opt_d = 0, result = 0; struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT; @@ -1503,7 +1508,8 @@ static int prune_remote(const char *remote, int dry_run) return result; } -static int prune(int argc, const char **argv, const char *prefix) +static int prune(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int dry_run = 0, result = 0; struct option options[] = { @@ -1534,7 +1540,8 @@ static int get_remote_default(const char *key, const char *value UNUSED, return 0; } -static int update(int argc, const char **argv, const char *prefix) +static int update(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i, prune = -1; struct option options[] = { @@ -1616,7 +1623,8 @@ static int set_remote_branches(const char *remotename, const char **branches, return 0; } -static int set_branches(int argc, const char **argv, const char *prefix) +static int set_branches(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int add_mode = 0; struct option options[] = { @@ -1635,7 +1643,8 @@ static int set_branches(int argc, const char **argv, const char *prefix) return set_remote_branches(argv[0], argv + 1, add_mode); } -static int get_url(int argc, const char **argv, const char *prefix) +static int get_url(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i, push_mode = 0, all_mode = 0; const char *remotename = NULL; @@ -1674,7 +1683,8 @@ static int get_url(int argc, const char **argv, const char *prefix) return 0; } -static int set_url(int argc, const char **argv, const char *prefix) +static int set_url(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i, push_mode = 0, add_mode = 0, delete_mode = 0; int matches = 0, negative_matches = 0; @@ -1765,7 +1775,7 @@ static int set_url(int argc, const char **argv, const char *prefix) int cmd_remote(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option options[] = { @@ -1788,7 +1798,7 @@ int cmd_remote(int argc, PARSE_OPT_SUBCOMMAND_OPTIONAL); if (fn) { - return !!fn(argc, argv, prefix); + return !!fn(argc, argv, prefix, repo); } else { if (argc) { error(_("unknown subcommand: `%s'"), argv[0]); diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 49aedc1de81..6f5fa5893b8 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -48,7 +48,8 @@ static char const * const builtin_sparse_checkout_list_usage[] = { NULL }; -static int sparse_checkout_list(int argc, const char **argv, const char *prefix) +static int sparse_checkout_list(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { static struct option builtin_sparse_checkout_list_options[] = { OPT_END(), @@ -443,7 +444,8 @@ static struct sparse_checkout_init_opts { int sparse_index; } init_opts; -static int sparse_checkout_init(int argc, const char **argv, const char *prefix) +static int sparse_checkout_init(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct pattern_list pl; char *sparse_filename; @@ -770,7 +772,8 @@ static struct sparse_checkout_add_opts { int use_stdin; } add_opts; -static int sparse_checkout_add(int argc, const char **argv, const char *prefix) +static int sparse_checkout_add(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { static struct option builtin_sparse_checkout_add_options[] = { OPT_BOOL_F(0, "skip-checks", &add_opts.skip_checks, @@ -808,7 +811,8 @@ static struct sparse_checkout_set_opts { int use_stdin; } set_opts; -static int sparse_checkout_set(int argc, const char **argv, const char *prefix) +static int sparse_checkout_set(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int default_patterns_nr = 2; const char *default_patterns[] = {"/*", "!/*/", NULL}; @@ -866,7 +870,8 @@ static struct sparse_checkout_reapply_opts { } reapply_opts; static int sparse_checkout_reapply(int argc, const char **argv, - const char *prefix) + const char *prefix, + struct repository *repo UNUSED) { static struct option builtin_sparse_checkout_reapply_options[] = { OPT_BOOL(0, "cone", &reapply_opts.cone_mode, @@ -901,7 +906,8 @@ static char const * const builtin_sparse_checkout_disable_usage[] = { }; static int sparse_checkout_disable(int argc, const char **argv, - const char *prefix) + const char *prefix, + struct repository *repo UNUSED) { static struct option builtin_sparse_checkout_disable_options[] = { OPT_END(), @@ -989,7 +995,8 @@ static int check_rules(struct pattern_list *pl, int null_terminated) { return 0; } -static int sparse_checkout_check_rules(int argc, const char **argv, const char *prefix) +static int sparse_checkout_check_rules(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { static struct option builtin_sparse_checkout_check_rules_options[] = { OPT_BOOL('z', NULL, &check_rules_opts.null_termination, @@ -1037,7 +1044,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char * int cmd_sparse_checkout(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option builtin_sparse_checkout_options[] = { @@ -1060,5 +1067,5 @@ int cmd_sparse_checkout(int argc, prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0; - return fn(argc, argv, prefix); + return fn(argc, argv, prefix, repo); } diff --git a/builtin/stash.c b/builtin/stash.c index 1399a1bbe2c..c212b1c0b2c 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -249,7 +249,8 @@ static int do_clear_stash(void) ref_stash, &obj, 0); } -static int clear_stash(int argc, const char **argv, const char *prefix) +static int clear_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() @@ -652,7 +653,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, return ret; } -static int apply_stash(int argc, const char **argv, const char *prefix) +static int apply_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int ret = -1; int quiet = 0; @@ -726,7 +728,8 @@ static int get_stash_info_assert(struct stash_info *info, int argc, return 0; } -static int drop_stash(int argc, const char **argv, const char *prefix) +static int drop_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int ret = -1; int quiet = 0; @@ -748,7 +751,8 @@ static int drop_stash(int argc, const char **argv, const char *prefix) return ret; } -static int pop_stash(int argc, const char **argv, const char *prefix) +static int pop_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int ret = -1; int index = 0; @@ -778,7 +782,8 @@ static int pop_stash(int argc, const char **argv, const char *prefix) return ret; } -static int branch_stash(int argc, const char **argv, const char *prefix) +static int branch_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int ret = -1; const char *branch = NULL; @@ -816,7 +821,8 @@ static int branch_stash(int argc, const char **argv, const char *prefix) return ret; } -static int list_stash(int argc, const char **argv, const char *prefix) +static int list_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct child_process cp = CHILD_PROCESS_INIT; struct option options[] = { @@ -889,7 +895,8 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op do_diff_cache(&info->b_commit, diff_opt); } -static int show_stash(int argc, const char **argv, const char *prefix) +static int show_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i; int ret = -1; @@ -1017,7 +1024,8 @@ static int do_store_stash(const struct object_id *w_commit, const char *stash_ms return 0; } -static int store_stash(int argc, const char **argv, const char *prefix) +static int store_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int quiet = 0; const char *stash_msg = NULL; @@ -1491,7 +1499,8 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b return ret; } -static int create_stash(int argc, const char **argv, const char *prefix UNUSED) +static int create_stash(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { int ret; struct strbuf stash_msg_buf = STRBUF_INIT; @@ -1827,12 +1836,14 @@ static int push_stash(int argc, const char **argv, const char *prefix, return ret; } -static int push_stash_unassumed(int argc, const char **argv, const char *prefix) +static int push_stash_unassumed(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { return push_stash(argc, argv, prefix, 0); } -static int save_stash(int argc, const char **argv, const char *prefix) +static int save_stash(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int keep_index = -1; int only_staged = 0; @@ -1878,7 +1889,7 @@ static int save_stash(int argc, const char **argv, const char *prefix) int cmd_stash(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { pid_t pid = getpid(); const char *index_file; @@ -1916,9 +1927,9 @@ int cmd_stash(int argc, (uintmax_t)pid); if (fn) - return !!fn(argc, argv, prefix); + return !!fn(argc, argv, prefix, repo); else if (!argc) - return !!push_stash_unassumed(0, NULL, prefix); + return !!push_stash_unassumed(0, NULL, prefix, repo); /* Assume 'stash push' */ strvec_push(&args, "push"); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b6b5f1ebde7..19e58783813 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -399,7 +399,8 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item, free(displaypath); } -static int module_foreach(int argc, const char **argv, const char *prefix) +static int module_foreach(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct foreach_cb info = FOREACH_CB_INIT; struct pathspec pathspec = { 0 }; @@ -544,7 +545,8 @@ static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data info->flags); } -static int module_init(int argc, const char **argv, const char *prefix) +static int module_init(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct init_cb info = INIT_CB_INIT; struct pathspec pathspec = { 0 }; @@ -738,7 +740,8 @@ static void status_submodule_cb(const struct cache_entry *list_item, info->prefix, info->super_prefix, info->flags); } -static int module_status(int argc, const char **argv, const char *prefix) +static int module_status(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct status_cb info = STATUS_CB_INIT; struct pathspec pathspec = { 0 }; @@ -1163,7 +1166,8 @@ static int compute_summary_module_list(struct object_id *head_oid, return ret; } -static int module_summary(int argc, const char **argv, const char *prefix) +static int module_summary(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct summary_cb info = SUMMARY_CB_INIT; int cached = 0; @@ -1339,7 +1343,8 @@ static void sync_submodule_cb(const struct cache_entry *list_item, void *cb_data info->flags); } -static int module_sync(int argc, const char **argv, const char *prefix) +static int module_sync(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct sync_cb info = SYNC_CB_INIT; struct pathspec pathspec = { 0 }; @@ -1485,7 +1490,8 @@ static void deinit_submodule_cb(const struct cache_entry *list_item, deinit_submodule(list_item->name, info->prefix, info->flags); } -static int module_deinit(int argc, const char **argv, const char *prefix) +static int module_deinit(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct deinit_cb info = DEINIT_CB_INIT; struct pathspec pathspec = { 0 }; @@ -1842,7 +1848,8 @@ static int clone_submodule(const struct module_clone_data *clone_data, return 0; } -static int module_clone(int argc, const char **argv, const char *prefix) +static int module_clone(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int dissociate = 0, quiet = 0, progress = 0, require_init = 0; struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT; @@ -2779,7 +2786,8 @@ static int update_submodules(struct update_data *update_data) return ret; } -static int module_update(int argc, const char **argv, const char *prefix) +static int module_update(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { struct pathspec pathspec = { 0 }; struct pathspec pathspec2 = { 0 }; @@ -2911,7 +2919,8 @@ static int module_update(int argc, const char **argv, const char *prefix) return ret; } -static int push_check(int argc, const char **argv, const char *prefix UNUSED) +static int push_check(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { struct remote *remote; const char *superproject_head; @@ -2991,7 +3000,8 @@ static int push_check(int argc, const char **argv, const char *prefix UNUSED) return 0; } -static int absorb_git_dirs(int argc, const char **argv, const char *prefix) +static int absorb_git_dirs(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int i; struct pathspec pathspec = { 0 }; @@ -3024,7 +3034,8 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) return ret; } -static int module_set_url(int argc, const char **argv, const char *prefix) +static int module_set_url(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int quiet = 0, ret; const char *newurl; @@ -3063,7 +3074,8 @@ static int module_set_url(int argc, const char **argv, const char *prefix) return !!ret; } -static int module_set_branch(int argc, const char **argv, const char *prefix) +static int module_set_branch(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int opt_default = 0, ret; const char *opt_branch = NULL; @@ -3113,7 +3125,8 @@ static int module_set_branch(int argc, const char **argv, const char *prefix) return !!ret; } -static int module_create_branch(int argc, const char **argv, const char *prefix) +static int module_create_branch(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { enum branch_track track; int quiet = 0, force = 0, reflog = 0, dry_run = 0; @@ -3424,7 +3437,8 @@ static void die_on_repo_without_commits(const char *path) strbuf_release(&sb); } -static int module_add(int argc, const char **argv, const char *prefix) +static int module_add(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) { int force = 0, quiet = 0, progress = 0, dissociate = 0; struct add_data add_data = ADD_DATA_INIT; @@ -3557,7 +3571,7 @@ static int module_add(int argc, const char **argv, const char *prefix) int cmd_submodule__helper(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; const char *const usage[] = { @@ -3583,5 +3597,5 @@ int cmd_submodule__helper(int argc, }; argc = parse_options(argc, argv, prefix, options, usage, 0); - return fn(argc, argv, prefix); + return fn(argc, argv, prefix, repo); } diff --git a/builtin/worktree.c b/builtin/worktree.c index dae63dedf4c..824dd71d643 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -231,7 +231,8 @@ static void prune_worktrees(void) strbuf_release(&reason); } -static int prune(int ac, const char **av, const char *prefix) +static int prune(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT__DRY_RUN(&show_only, N_("do not remove, show only")), @@ -763,7 +764,8 @@ static char *dwim_branch(const char *path, char **new_branch) return NULL; } -static int add(int ac, const char **av, const char *prefix) +static int add(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { struct add_opts opts; const char *new_branch_force = NULL; @@ -1039,7 +1041,8 @@ static void pathsort(struct worktree **wt) QSORT(wt, n, pathcmp); } -static int list(int ac, const char **av, const char *prefix) +static int list(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { int porcelain = 0; int line_terminator = '\n'; @@ -1084,7 +1087,8 @@ static int list(int ac, const char **av, const char *prefix) return 0; } -static int lock_worktree(int ac, const char **av, const char *prefix) +static int lock_worktree(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { const char *reason = "", *old_reason; struct option options[] = { @@ -1119,7 +1123,8 @@ static int lock_worktree(int ac, const char **av, const char *prefix) return 0; } -static int unlock_worktree(int ac, const char **av, const char *prefix) +static int unlock_worktree(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { struct option options[] = { OPT_END() @@ -1182,7 +1187,8 @@ static void validate_no_submodules(const struct worktree *wt) die(_("working trees containing submodules cannot be moved or removed")); } -static int move_worktree(int ac, const char **av, const char *prefix) +static int move_worktree(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { int force = 0; struct option options[] = { @@ -1312,7 +1318,8 @@ static int delete_git_work_tree(struct worktree *wt) return ret; } -static int remove_worktree(int ac, const char **av, const char *prefix) +static int remove_worktree(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { int force = 0; struct option options[] = { @@ -1377,7 +1384,8 @@ static void report_repair(int iserr, const char *path, const char *msg, void *cb } } -static int repair(int ac, const char **av, const char *prefix) +static int repair(int ac, const char **av, const char *prefix, + struct repository *repo UNUSED) { const char **p; const char *self[] = { ".", NULL }; @@ -1397,7 +1405,7 @@ static int repair(int ac, const char **av, const char *prefix) int cmd_worktree(int ac, const char **av, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option options[] = { @@ -1422,5 +1430,5 @@ int cmd_worktree(int ac, prepare_repo_settings(the_repository); the_repository->settings.command_requires_full_index = 0; - return fn(ac, av, prefix); + return fn(ac, av, prefix, repo); } diff --git a/parse-options.h b/parse-options.h index ae153423908..f0801d4532a 100644 --- a/parse-options.h +++ b/parse-options.h @@ -3,6 +3,8 @@ #include "gettext.h" +struct repository; + /** * Refer to Documentation/technical/api-parse-options.txt for the API doc. */ @@ -73,7 +75,7 @@ typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx, const char *arg, int unset); typedef int parse_opt_subcommand_fn(int argc, const char **argv, - const char *prefix); + const char *prefix, struct repository *repo); /* * `type`:: diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 5250913d99e..5da359486cd 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -282,14 +282,16 @@ int cmd__parse_options_flags(int argc, const char **argv) return parse_options_flags__cmd(argc, argv, test_flags); } -static int subcmd_one(int argc, const char **argv, const char *prefix UNUSED) +static int subcmd_one(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { printf("fn: subcmd_one\n"); print_args(argc, argv); return 0; } -static int subcmd_two(int argc, const char **argv, const char *prefix UNUSED) +static int subcmd_two(int argc, const char **argv, const char *prefix UNUSED, + struct repository *repo UNUSED) { printf("fn: subcmd_two\n"); print_args(argc, argv); @@ -319,7 +321,7 @@ static int parse_subcommand__cmd(int argc, const char **argv, printf("opt: %d\n", opt); - return fn(argc, argv, NULL); + return fn(argc, argv, NULL, NULL); } int cmd__parse_subcommand(int argc, const char **argv) -- GitLab From 30993e0aa2a481f35511bc3d5ecad19e8cde53f1 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 11 Nov 2024 18:17:15 +0100 Subject: [PATCH 26/34] Change midx.c and midx-write.c to not use global variables Similar to the earlier patch series on cleaning up packfile.c and removing usage of global variables [1], we change the midx.c and midx-write.c files to no longer use global variables. This is done by the following: - Usage of repository variable already available in existing structs. - Passing down repository variable from other subsystems. - Modifying all subcommands to obtain repository variable from the command in `builtins/` and passing down the variable from there. The biggest change is in the first commit, wherein we modify all subcommands to add the repository variable. Since the subcommand definition are not often changed, it shouldn't cause too many conflicts with in flight topics. Since the `packfile.c` cleanup is still in flight, this series is based on top of master: b31fb630c0 (Merge https://github.com/j6t/git-gui, 2024-11-11) with those patches merged in. Since v3 this series also depends on 'kn/pass-repo-to-builtin-sub-sub-commands', which was split out from this series. [1]: https://lore.kernel.org/git/cover.1729504640.git.karthik.188@gmail.com/ To: git@vger.kernel.org Signed-off-by: Karthik Nayak --- Changes in v3: - Split out the first commit into a separate series [1]. - Improved some of the commit messages to be more descriptive. - Merged the 8th and 9th commits together, since they were similar. - v2: https://lore.kernel.org/r/20241119-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-v2-0-e2f607174efc@gmail.com Changes in v2: - Split commit modifying static functions in `midx-write.c` to multiple commits, which makes it easier to review. - Remove usage of `the_repository` in `test-parse-options.c` and use NULL instead. - Fix the commit messages to be imperative. - Fix error in commit sign off. - v1: https://lore.kernel.org/r/20241115-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-v1-0-761f8a2c7775@gmail.com [1]: https://lore.kernel.org/git/xmqq34jdyey3.fsf@gitster.g/T/#t --- b4-submit-tracking --- { "series": { "revision": 3, "change-id": "20241111-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-a88498c2590f", "prefixes": [], "prerequisites": [ "message-id: ", "base-commit: b31fb630c0" ], "history": { "v1": [ "20241115-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-v1-0-761f8a2c7775@gmail.com" ], "v2": [ "20241119-374-refactor-midx-c-and-midx-write-c-to-not-depend-on-global-state-v2-0-e2f607174efc@gmail.com" ] } } } -- GitLab From d7c1056ebdf374f9fb09561cec5bdead3315fc57 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 18 Nov 2024 16:25:43 +0100 Subject: [PATCH 27/34] midx-write: pass down repository to static functions In 'midx-write.c' there are a lot of static functions which use global variables `the_repository` or `the_hash_algo`. In a follow up commit, the repository variable will be added to `write_midx_context`, which some of the functions can use. But for functions which do not have access to this struct, pass down the required information from non-static functions `write_midx_file` and `write_midx_file_only`. This requires that the function `hash_to_hex` is also replaced with `hash_to_hex_algop` since the former internally accesses the `the_hash_algo` global variable. This ensures that the usage of global variables is limited to these non-static functions, which will be cleaned up in a follow up commit. Signed-off-by: Karthik Nayak --- midx-write.c | 57 ++++++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/midx-write.c b/midx-write.c index c57726ef947..22b5233f51e 100644 --- a/midx-write.c +++ b/midx-write.c @@ -35,13 +35,13 @@ extern void clear_incremental_midx_files_ext(const char *object_dir, extern int cmp_idx_or_pack_name(const char *idx_or_pack_name, const char *idx_name); -static size_t write_midx_header(struct hashfile *f, - unsigned char num_chunks, +static size_t write_midx_header(const struct git_hash_algo *hash_algo, + struct hashfile *f, unsigned char num_chunks, uint32_t num_packs) { hashwrite_be32(f, MIDX_SIGNATURE); hashwrite_u8(f, MIDX_VERSION); - hashwrite_u8(f, oid_version(the_hash_algo)); + hashwrite_u8(f, oid_version(hash_algo)); hashwrite_u8(f, num_chunks); hashwrite_u8(f, 0); /* unused */ hashwrite_be32(f, num_packs); @@ -702,7 +702,7 @@ static int add_ref_to_pending(const char *refname, const char *referent UNUSED, return 0; } - if (!peel_iterated_oid(the_repository, oid, &peeled)) + if (!peel_iterated_oid(revs->repo, oid, &peeled)) oid = &peeled; object = parse_object_or_die(oid, refname); @@ -827,7 +827,7 @@ static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr return cb.commits; } -static int write_midx_bitmap(const char *midx_name, +static int write_midx_bitmap(struct repository *r, const char *midx_name, const unsigned char *midx_hash, struct packing_data *pdata, struct commit **commits, @@ -840,9 +840,9 @@ static int write_midx_bitmap(const char *midx_name, struct bitmap_writer writer; struct pack_idx_entry **index; char *bitmap_name = xstrfmt("%s-%s.bitmap", midx_name, - hash_to_hex(midx_hash)); + hash_to_hex_algop(midx_hash, r->hash_algo)); - trace2_region_enter("midx", "write_midx_bitmap", the_repository); + trace2_region_enter("midx", "write_midx_bitmap", r); if (flags & MIDX_WRITE_BITMAP_HASH_CACHE) options |= BITMAP_OPT_HASH_CACHE; @@ -859,7 +859,7 @@ static int write_midx_bitmap(const char *midx_name, for (i = 0; i < pdata->nr_objects; i++) index[i] = &pdata->objects[i].idx; - bitmap_writer_init(&writer, the_repository, pdata); + bitmap_writer_init(&writer, r, pdata); bitmap_writer_show_progress(&writer, flags & MIDX_PROGRESS); bitmap_writer_build_type_index(&writer, index); @@ -892,7 +892,7 @@ static int write_midx_bitmap(const char *midx_name, free(bitmap_name); bitmap_writer_free(&writer); - trace2_region_leave("midx", "write_midx_bitmap", the_repository); + trace2_region_leave("midx", "write_midx_bitmap", r); return ret; } @@ -1049,7 +1049,7 @@ static void clear_midx_files(const char *object_dir, strbuf_release(&buf); } -static int write_midx_internal(const char *object_dir, +static int write_midx_internal(struct repository *r, const char *object_dir, struct string_list *packs_to_include, struct string_list *packs_to_drop, const char *preferred_pack_name, @@ -1070,7 +1070,8 @@ static int write_midx_internal(const char *object_dir, const char **keep_hashes = NULL; struct chunkfile *cf; - trace2_region_enter("midx", "write_midx_internal", the_repository); + trace2_region_enter("midx", "write_midx_internal", r); + ctx.incremental = !!(flags & MIDX_WRITE_INCREMENTAL); if (ctx.incremental && (flags & MIDX_WRITE_BITMAP)) @@ -1087,8 +1088,7 @@ static int write_midx_internal(const char *object_dir, midx_name.buf); if (!packs_to_include || ctx.incremental) { - struct multi_pack_index *m = lookup_multi_pack_index(the_repository, - object_dir); + struct multi_pack_index *m = lookup_multi_pack_index(r, object_dir); if (m && !midx_checksum_valid(m)) { warning(_("ignoring existing multi-pack-index; checksum mismatch")); m = NULL; @@ -1351,7 +1351,7 @@ static int write_midx_internal(const char *object_dir, add_chunk(cf, MIDX_CHUNKID_OIDFANOUT, MIDX_CHUNK_FANOUT_SIZE, write_midx_oid_fanout); add_chunk(cf, MIDX_CHUNKID_OIDLOOKUP, - st_mult(ctx.entries_nr, the_hash_algo->rawsz), + st_mult(ctx.entries_nr, r->hash_algo->rawsz), write_midx_oid_lookup); add_chunk(cf, MIDX_CHUNKID_OBJECTOFFSETS, st_mult(ctx.entries_nr, MIDX_CHUNK_OFFSET_WIDTH), @@ -1373,7 +1373,8 @@ static int write_midx_internal(const char *object_dir, write_midx_bitmapped_packs); } - write_midx_header(f, get_num_chunks(cf), ctx.nr - dropped_packs); + write_midx_header(r->hash_algo, f, get_num_chunks(cf), + ctx.nr - dropped_packs); write_chunkfile(cf, &ctx); finalize_hashfile(f, midx_hash, FSYNC_COMPONENT_PACK_METADATA, @@ -1405,7 +1406,7 @@ static int write_midx_internal(const char *object_dir, FREE_AND_NULL(ctx.entries); ctx.entries_nr = 0; - if (write_midx_bitmap(midx_name.buf, midx_hash, &pdata, + if (write_midx_bitmap(r, midx_name.buf, midx_hash, &pdata, commits, commits_nr, ctx.pack_order, flags) < 0) { error(_("could not write multi-pack bitmap")); @@ -1449,12 +1450,13 @@ static int write_midx_internal(const char *object_dir, strbuf_release(&final_midx_name); keep_hashes[ctx.num_multi_pack_indexes_before] = - xstrdup(hash_to_hex(midx_hash)); + xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo)); for (i = 0; i < ctx.num_multi_pack_indexes_before; i++) { uint32_t j = ctx.num_multi_pack_indexes_before - i - 1; - keep_hashes[j] = xstrdup(hash_to_hex(get_midx_checksum(m))); + keep_hashes[j] = xstrdup(hash_to_hex_algop(get_midx_checksum(m), + r->hash_algo)); m = m->base_midx; } @@ -1462,7 +1464,7 @@ static int write_midx_internal(const char *object_dir, fprintf(get_lock_file_fp(&lk), "%s\n", keep_hashes[i]); } else { keep_hashes[ctx.num_multi_pack_indexes_before] = - xstrdup(hash_to_hex(midx_hash)); + xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo)); } if (ctx.m || ctx.base_midx) @@ -1495,7 +1497,7 @@ static int write_midx_internal(const char *object_dir, } strbuf_release(&midx_name); - trace2_region_leave("midx", "write_midx_internal", the_repository); + trace2_region_leave("midx", "write_midx_internal", r); return result; } @@ -1505,8 +1507,8 @@ int write_midx_file(const char *object_dir, const char *refs_snapshot, unsigned flags) { - return write_midx_internal(object_dir, NULL, NULL, preferred_pack_name, - refs_snapshot, flags); + return write_midx_internal(the_repository, object_dir, NULL, NULL, + preferred_pack_name, refs_snapshot, flags); } int write_midx_file_only(const char *object_dir, @@ -1515,8 +1517,9 @@ int write_midx_file_only(const char *object_dir, const char *refs_snapshot, unsigned flags) { - return write_midx_internal(object_dir, packs_to_include, NULL, - preferred_pack_name, refs_snapshot, flags); + return write_midx_internal(the_repository, object_dir, packs_to_include, + NULL, preferred_pack_name, refs_snapshot, + flags); } int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags) @@ -1572,7 +1575,8 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla free(count); if (packs_to_drop.nr) - result = write_midx_internal(object_dir, NULL, &packs_to_drop, NULL, NULL, flags); + result = write_midx_internal(r, object_dir, NULL, + &packs_to_drop, NULL, NULL, flags); string_list_clear(&packs_to_drop, 0); @@ -1769,7 +1773,8 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, goto cleanup; } - result = write_midx_internal(object_dir, NULL, NULL, NULL, NULL, flags); + result = write_midx_internal(r, object_dir, NULL, NULL, NULL, NULL, + flags); cleanup: free(include_pack); -- GitLab From 11cd6c6bec6a80dfbff6f7a1e72fc8ffca8e574d Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 18 Nov 2024 16:33:24 +0100 Subject: [PATCH 28/34] midx-write: use `revs->repo` inside `read_refs_snapshot` The function `read_refs_snapshot()` uses `parse_oid_hex()`, which relies on the global `the_hash_algo` variable. Let's instead use `parse_oid_hex_algop()` and provide the hash algo via `revs->repo`. Also, while here, fix a missing newline after the function's definition. Signed-off-by: Karthik Nayak --- midx-write.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/midx-write.c b/midx-write.c index 22b5233f51e..564438f616f 100644 --- a/midx-write.c +++ b/midx-write.c @@ -760,7 +760,7 @@ static int read_refs_snapshot(const char *refs_snapshot, hex = &buf.buf[1]; } - if (parse_oid_hex(hex, &oid, &end) < 0) + if (parse_oid_hex_algop(hex, &oid, &end, revs->repo->hash_algo) < 0) die(_("could not parse line: %s"), buf.buf); if (*end) die(_("malformed line: %s"), buf.buf); @@ -776,6 +776,7 @@ static int read_refs_snapshot(const char *refs_snapshot, strbuf_release(&buf); return 0; } + static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr_p, const char *refs_snapshot, struct write_midx_context *ctx) -- GitLab From d74bf1fa511591a11c60cf356ea546717e08cda3 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 18 Nov 2024 16:35:21 +0100 Subject: [PATCH 29/34] write-midx: add repository field to `write_midx_context` The struct `write_midx_context` is used to pass context for creating MIDX files. Add the repository field here to ensure that most functions within `midx-write.c` have access to the field and can use that instead of the global `the_repository` variable. Signed-off-by: Karthik Nayak --- midx-write.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/midx-write.c b/midx-write.c index 564438f616f..1c355cdf8db 100644 --- a/midx-write.c +++ b/midx-write.c @@ -110,6 +110,8 @@ struct write_midx_context { uint32_t num_multi_pack_indexes_before; struct string_list *to_include; + + struct repository *repo; }; static int should_include_pack(const struct write_midx_context *ctx, @@ -154,7 +156,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len, return; ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc); - p = add_packed_git(the_repository, full_path, full_path_len, 0); + p = add_packed_git(ctx->repo, full_path, full_path_len, 0); if (!p) { warning(_("failed to add packfile '%s'"), full_path); @@ -480,7 +482,7 @@ static int write_midx_oid_lookup(struct hashfile *f, void *data) { struct write_midx_context *ctx = data; - unsigned char hash_len = the_hash_algo->rawsz; + unsigned char hash_len = ctx->repo->hash_algo->rawsz; struct pack_midx_entry *list = ctx->entries; uint32_t i; @@ -605,7 +607,7 @@ static uint32_t *midx_pack_order(struct write_midx_context *ctx) uint32_t *pack_order, base_objects = 0; uint32_t i; - trace2_region_enter("midx", "midx_pack_order", the_repository); + trace2_region_enter("midx", "midx_pack_order", ctx->repo); if (ctx->incremental && ctx->base_midx) base_objects = ctx->base_midx->num_objects + @@ -640,7 +642,7 @@ static uint32_t *midx_pack_order(struct write_midx_context *ctx) } free(data); - trace2_region_leave("midx", "midx_pack_order", the_repository); + trace2_region_leave("midx", "midx_pack_order", ctx->repo); return pack_order; } @@ -651,9 +653,10 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash, struct strbuf buf = STRBUF_INIT; char *tmp_file; - trace2_region_enter("midx", "write_midx_reverse_index", the_repository); + trace2_region_enter("midx", "write_midx_reverse_index", ctx->repo); - strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex(midx_hash)); + strbuf_addf(&buf, "%s-%s.rev", midx_name, hash_to_hex_algop(midx_hash, + ctx->repo->hash_algo)); tmp_file = write_rev_file_order(NULL, ctx->pack_order, ctx->entries_nr, midx_hash, WRITE_REV); @@ -664,7 +667,7 @@ static void write_midx_reverse_index(char *midx_name, unsigned char *midx_hash, strbuf_release(&buf); free(tmp_file); - trace2_region_leave("midx", "write_midx_reverse_index", the_repository); + trace2_region_leave("midx", "write_midx_reverse_index", ctx->repo); } static void prepare_midx_packing_data(struct packing_data *pdata, @@ -672,10 +675,10 @@ static void prepare_midx_packing_data(struct packing_data *pdata, { uint32_t i; - trace2_region_enter("midx", "prepare_midx_packing_data", the_repository); + trace2_region_enter("midx", "prepare_midx_packing_data", ctx->repo); memset(pdata, 0, sizeof(struct packing_data)); - prepare_packing_data(the_repository, pdata); + prepare_packing_data(ctx->repo, pdata); for (i = 0; i < ctx->entries_nr; i++) { uint32_t pos = ctx->pack_order[i]; @@ -686,7 +689,7 @@ static void prepare_midx_packing_data(struct packing_data *pdata, ctx->info[ctx->pack_perm[from->pack_int_id]].p); } - trace2_region_leave("midx", "prepare_midx_packing_data", the_repository); + trace2_region_leave("midx", "prepare_midx_packing_data", ctx->repo); } static int add_ref_to_pending(const char *refname, const char *referent UNUSED, @@ -784,17 +787,16 @@ static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr struct rev_info revs; struct bitmap_commit_cb cb = {0}; - trace2_region_enter("midx", "find_commits_for_midx_bitmap", - the_repository); + trace2_region_enter("midx", "find_commits_for_midx_bitmap", ctx->repo); cb.ctx = ctx; - repo_init_revisions(the_repository, &revs, NULL); + repo_init_revisions(ctx->repo, &revs, NULL); if (refs_snapshot) { read_refs_snapshot(refs_snapshot, &revs); } else { setup_revisions(0, NULL, &revs, NULL); - refs_for_each_ref(get_main_ref_store(the_repository), + refs_for_each_ref(get_main_ref_store(ctx->repo), add_ref_to_pending, &revs); } @@ -822,8 +824,7 @@ static struct commit **find_commits_for_midx_bitmap(uint32_t *indexed_commits_nr release_revisions(&revs); - trace2_region_leave("midx", "find_commits_for_midx_bitmap", - the_repository); + trace2_region_leave("midx", "find_commits_for_midx_bitmap", ctx->repo); return cb.commits; } @@ -945,7 +946,7 @@ static int fill_packs_from_midx(struct write_midx_context *ctx, */ if (flags & MIDX_WRITE_REV_INDEX || preferred_pack_name) { - if (prepare_midx_pack(the_repository, m, + if (prepare_midx_pack(ctx->repo, m, m->num_packs_in_base + i)) { error(_("could not load pack")); return 1; @@ -1073,6 +1074,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir, trace2_region_enter("midx", "write_midx_internal", r); + ctx.repo = r; ctx.incremental = !!(flags & MIDX_WRITE_INCREMENTAL); if (ctx.incremental && (flags & MIDX_WRITE_BITMAP)) @@ -1469,7 +1471,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir, } if (ctx.m || ctx.base_midx) - close_object_store(the_repository->objects); + close_object_store(ctx.repo->objects); if (commit_lock_file(&lk) < 0) die_errno(_("could not write multi-pack-index")); -- GitLab From 630bffe661cec943fac8f4a0d26a20d6298a7748 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 13 Nov 2024 12:42:06 +0100 Subject: [PATCH 30/34] midx-write: pass down repository to `write_midx_file[_only]` In a previous commit, we passed the repository field to all subcommands in the `builtin/` directory. Utilize this to pass the repository field down to the `write_midx_file[_only]` functions to remove the usage of `the_repository` global variables. With this, all usage of global variables in `midx-write.c` is removed, hence, remove the `USE_THE_REPOSITORY_VARIABLE` guard from the file. Signed-off-by: Karthik Nayak --- builtin/multi-pack-index.c | 6 +++--- builtin/repack.c | 2 +- midx-write.c | 22 +++++++++------------- midx.h | 10 ++++------ 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c index 85e40a4b6d3..2a938466f53 100644 --- a/builtin/multi-pack-index.c +++ b/builtin/multi-pack-index.c @@ -120,7 +120,7 @@ static void read_packs_from_stdin(struct string_list *to) static int cmd_multi_pack_index_write(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { struct option *options; static struct option builtin_multi_pack_index_write_options[] = { @@ -165,7 +165,7 @@ static int cmd_multi_pack_index_write(int argc, const char **argv, read_packs_from_stdin(&packs); - ret = write_midx_file_only(opts.object_dir, &packs, + ret = write_midx_file_only(repo, opts.object_dir, &packs, opts.preferred_pack, opts.refs_snapshot, opts.flags); @@ -176,7 +176,7 @@ static int cmd_multi_pack_index_write(int argc, const char **argv, } - ret = write_midx_file(opts.object_dir, opts.preferred_pack, + ret = write_midx_file(repo, opts.object_dir, opts.preferred_pack, opts.refs_snapshot, opts.flags); free(opts.refs_snapshot); diff --git a/builtin/repack.c b/builtin/repack.c index 96a4fa234bd..9c21fc482df 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -1569,7 +1569,7 @@ int cmd_repack(int argc, unsigned flags = 0; if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL, 0)) flags |= MIDX_WRITE_INCREMENTAL; - write_midx_file(repo_get_object_directory(the_repository), + write_midx_file(the_repository, repo_get_object_directory(the_repository), NULL, NULL, flags); } diff --git a/midx-write.c b/midx-write.c index 1c355cdf8db..1bc2f525691 100644 --- a/midx-write.c +++ b/midx-write.c @@ -1,5 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE - #include "git-compat-util.h" #include "abspath.h" #include "config.h" @@ -1505,24 +1503,22 @@ static int write_midx_internal(struct repository *r, const char *object_dir, return result; } -int write_midx_file(const char *object_dir, +int write_midx_file(struct repository *r, const char *object_dir, const char *preferred_pack_name, - const char *refs_snapshot, - unsigned flags) + const char *refs_snapshot, unsigned flags) { - return write_midx_internal(the_repository, object_dir, NULL, NULL, - preferred_pack_name, refs_snapshot, flags); + return write_midx_internal(r, object_dir, NULL, NULL, + preferred_pack_name, refs_snapshot, + flags); } -int write_midx_file_only(const char *object_dir, +int write_midx_file_only(struct repository *r, const char *object_dir, struct string_list *packs_to_include, const char *preferred_pack_name, - const char *refs_snapshot, - unsigned flags) + const char *refs_snapshot, unsigned flags) { - return write_midx_internal(the_repository, object_dir, packs_to_include, - NULL, preferred_pack_name, refs_snapshot, - flags); + return write_midx_internal(r, object_dir, packs_to_include, NULL, + preferred_pack_name, refs_snapshot, flags); } int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags) diff --git a/midx.h b/midx.h index 3b0ac4d8788..c37ad5b5242 100644 --- a/midx.h +++ b/midx.h @@ -123,15 +123,13 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i * Variant of write_midx_file which writes a MIDX containing only the packs * specified in packs_to_include. */ -int write_midx_file(const char *object_dir, - const char *preferred_pack_name, - const char *refs_snapshot, +int write_midx_file(struct repository *r, const char *object_dir, + const char *preferred_pack_name, const char *refs_snapshot, unsigned flags); -int write_midx_file_only(const char *object_dir, +int write_midx_file_only(struct repository *r, const char *object_dir, struct string_list *packs_to_include, const char *preferred_pack_name, - const char *refs_snapshot, - unsigned flags); + const char *refs_snapshot, unsigned flags); void clear_midx_file(struct repository *r); int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags); int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags); -- GitLab From 32d001bdd6d25b9859e378b6927656d1352f2325 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 12 Nov 2024 12:51:37 +0100 Subject: [PATCH 31/34] midx: cleanup internal usage of `the_repository` and `the_hash_algo` In the `midx.c` file, there are multiple usages of `the_repository` and `the_hash_algo` within static functions of the file. Some of the usages can be simply swapped out with the available `repository` struct. While some of them can be swapped out by passing the repository to the required functions. This leaves out only some other usages of `the_repository` and `the_hash_algo` in the file in non-static functions, which we'll tackle in upcoming commits. Signed-off-by: Karthik Nayak --- midx.c | 49 +++++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/midx.c b/midx.c index 079c45a1aaf..6f0fb8285af 100644 --- a/midx.c +++ b/midx.c @@ -25,7 +25,7 @@ int cmp_idx_or_pack_name(const char *idx_or_pack_name, const unsigned char *get_midx_checksum(struct multi_pack_index *m) { - return m->data + m->data_len - the_hash_algo->rawsz; + return m->data + m->data_len - m->repo->hash_algo->rawsz; } void get_midx_filename(struct strbuf *out, const char *object_dir) @@ -94,7 +94,8 @@ static int midx_read_object_offsets(const unsigned char *chunk_start, #define MIDX_MIN_SIZE (MIDX_HEADER_SIZE + the_hash_algo->rawsz) -static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir, +static struct multi_pack_index *load_multi_pack_index_one(struct repository *r, + const char *object_dir, const char *midx_name, int local) { @@ -131,7 +132,7 @@ static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir m->data = midx_map; m->data_len = midx_size; m->local = local; - m->repo = the_repository; + m->repo = r; m->signature = get_be32(m->data); if (m->signature != MIDX_SIGNATURE) @@ -144,12 +145,12 @@ static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir m->version); hash_version = m->data[MIDX_BYTE_HASH_VERSION]; - if (hash_version != oid_version(the_hash_algo)) { + if (hash_version != oid_version(r->hash_algo)) { error(_("multi-pack-index hash version %u does not match version %u"), - hash_version, oid_version(the_hash_algo)); + hash_version, oid_version(r->hash_algo)); goto cleanup_fail; } - m->hash_len = the_hash_algo->rawsz; + m->hash_len = r->hash_algo->rawsz; m->num_chunks = m->data[MIDX_BYTE_NUM_CHUNKS]; @@ -206,8 +207,8 @@ static struct multi_pack_index *load_multi_pack_index_one(const char *object_dir m->pack_names[i]); } - trace2_data_intmax("midx", the_repository, "load/num_packs", m->num_packs); - trace2_data_intmax("midx", the_repository, "load/num_objects", m->num_objects); + trace2_data_intmax("midx", r, "load/num_packs", m->num_packs); + trace2_data_intmax("midx", r, "load/num_objects", m->num_objects); free_chunkfile(cf); return m; @@ -240,8 +241,9 @@ void get_split_midx_filename_ext(struct strbuf *buf, const char *object_dir, strbuf_addf(buf, "/multi-pack-index-%s.%s", hash_to_hex(hash), ext); } -static int open_multi_pack_index_chain(const char *chain_file, - int *fd, struct stat *st) +static int open_multi_pack_index_chain(const struct git_hash_algo *hash_algo, + const char *chain_file, int *fd, + struct stat *st) { *fd = git_open(chain_file); if (*fd < 0) @@ -250,7 +252,7 @@ static int open_multi_pack_index_chain(const char *chain_file, close(*fd); return 0; } - if (st->st_size < the_hash_algo->hexsz) { + if (st->st_size < hash_algo->hexsz) { close(*fd); if (!st->st_size) { /* treat empty files the same as missing */ @@ -292,7 +294,8 @@ static int add_midx_to_chain(struct multi_pack_index *midx, return 1; } -static struct multi_pack_index *load_midx_chain_fd_st(const char *object_dir, +static struct multi_pack_index *load_midx_chain_fd_st(struct repository *r, + const char *object_dir, int local, int fd, struct stat *st, int *incomplete_chain) @@ -303,7 +306,7 @@ static struct multi_pack_index *load_midx_chain_fd_st(const char *object_dir, uint32_t i, count; FILE *fp = xfdopen(fd, "r"); - count = st->st_size / (the_hash_algo->hexsz + 1); + count = st->st_size / (r->hash_algo->hexsz + 1); for (i = 0; i < count; i++) { struct multi_pack_index *m; @@ -312,7 +315,7 @@ static struct multi_pack_index *load_midx_chain_fd_st(const char *object_dir, if (strbuf_getline_lf(&buf, fp) == EOF) break; - if (get_oid_hex(buf.buf, &layer)) { + if (get_oid_hex_algop(buf.buf, &layer, r->hash_algo)) { warning(_("invalid multi-pack-index chain: line '%s' " "not a hash"), buf.buf); @@ -325,7 +328,7 @@ static struct multi_pack_index *load_midx_chain_fd_st(const char *object_dir, strbuf_reset(&buf); get_split_midx_filename_ext(&buf, object_dir, layer.hash, MIDX_EXT_MIDX); - m = load_multi_pack_index_one(object_dir, buf.buf, local); + m = load_multi_pack_index_one(r, object_dir, buf.buf, local); if (m) { if (add_midx_to_chain(m, midx_chain)) { @@ -348,7 +351,8 @@ static struct multi_pack_index *load_midx_chain_fd_st(const char *object_dir, return midx_chain; } -static struct multi_pack_index *load_multi_pack_index_chain(const char *object_dir, +static struct multi_pack_index *load_multi_pack_index_chain(struct repository *r, + const char *object_dir, int local) { struct strbuf chain_file = STRBUF_INIT; @@ -357,10 +361,10 @@ static struct multi_pack_index *load_multi_pack_index_chain(const char *object_d struct multi_pack_index *m = NULL; get_midx_chain_filename(&chain_file, object_dir); - if (open_multi_pack_index_chain(chain_file.buf, &fd, &st)) { + if (open_multi_pack_index_chain(r->hash_algo, chain_file.buf, &fd, &st)) { int incomplete; /* ownership of fd is taken over by load function */ - m = load_midx_chain_fd_st(object_dir, local, fd, &st, + m = load_midx_chain_fd_st(r, object_dir, local, fd, &st, &incomplete); } @@ -376,9 +380,10 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, get_midx_filename(&midx_name, object_dir); - m = load_multi_pack_index_one(object_dir, midx_name.buf, local); + m = load_multi_pack_index_one(the_repository, object_dir, + midx_name.buf, local); if (!m) - m = load_multi_pack_index_chain(object_dir, local); + m = load_multi_pack_index_chain(the_repository, object_dir, local); strbuf_release(&midx_name); @@ -520,7 +525,7 @@ int bsearch_one_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result) { int ret = bsearch_hash(oid->hash, m->chunk_oid_fanout, - m->chunk_oid_lookup, the_hash_algo->rawsz, + m->chunk_oid_lookup, m->repo->hash_algo->rawsz, result); if (result) *result += m->num_objects_in_base; @@ -551,7 +556,7 @@ struct object_id *nth_midxed_object_oid(struct object_id *oid, n = midx_for_object(&m, n); oidread(oid, m->chunk_oid_lookup + st_mult(m->hash_len, n), - the_repository->hash_algo); + m->repo->hash_algo); return oid; } -- GitLab From 392a96de6a5e5ef36ad0e67c8f56de869a422358 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 12 Nov 2024 12:57:47 +0100 Subject: [PATCH 32/34] midx: pass `repository` to `load_multi_pack_index` The `load_multi_pack_index` function in midx uses `the_repository` variable to access the `repository` struct. Modify the function and its callee's to send the `repository` field. This moves usage of `the_repository` to the `test-read-midx.c` file. While that is not optimal, it is okay, since the upcoming commits will slowly move the usage of `the_repository` up the layers and remove it eventually. Signed-off-by: Karthik Nayak --- midx.c | 11 ++++++----- midx.h | 4 +++- t/helper/test-read-midx.c | 8 ++++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/midx.c b/midx.c index 6f0fb8285af..98ee84d4a8b 100644 --- a/midx.c +++ b/midx.c @@ -372,7 +372,8 @@ static struct multi_pack_index *load_multi_pack_index_chain(struct repository *r return m; } -struct multi_pack_index *load_multi_pack_index(const char *object_dir, +struct multi_pack_index *load_multi_pack_index(struct repository *r, + const char *object_dir, int local) { struct strbuf midx_name = STRBUF_INIT; @@ -380,10 +381,10 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, get_midx_filename(&midx_name, object_dir); - m = load_multi_pack_index_one(the_repository, object_dir, + m = load_multi_pack_index_one(r, object_dir, midx_name.buf, local); if (!m) - m = load_multi_pack_index_chain(the_repository, object_dir, local); + m = load_multi_pack_index_chain(r, object_dir, local); strbuf_release(&midx_name); @@ -727,7 +728,7 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i if (!strcmp(object_dir, m_search->object_dir)) return 1; - m = load_multi_pack_index(object_dir, local); + m = load_multi_pack_index(r, object_dir, local); if (m) { struct multi_pack_index *mp = r->objects->multi_pack_index; @@ -881,7 +882,7 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag struct pair_pos_vs_id *pairs = NULL; uint32_t i; struct progress *progress = NULL; - struct multi_pack_index *m = load_multi_pack_index(object_dir, 1); + struct multi_pack_index *m = load_multi_pack_index(r, object_dir, 1); struct multi_pack_index *curr; verify_midx_error = 0; diff --git a/midx.h b/midx.h index c37ad5b5242..78efa28d353 100644 --- a/midx.h +++ b/midx.h @@ -97,7 +97,9 @@ void get_midx_chain_filename(struct strbuf *buf, const char *object_dir); void get_split_midx_filename_ext(struct strbuf *buf, const char *object_dir, const unsigned char *hash, const char *ext); -struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local); +struct multi_pack_index *load_multi_pack_index(struct repository *r, + const char *object_dir, + int local); int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id); struct packed_git *nth_midxed_pack(struct multi_pack_index *m, uint32_t pack_int_id); diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c index 438fb9fc619..fc632369618 100644 --- a/t/helper/test-read-midx.c +++ b/t/helper/test-read-midx.c @@ -18,7 +18,7 @@ static int read_midx_file(const char *object_dir, const char *checksum, struct multi_pack_index *m; setup_git_directory(); - m = load_multi_pack_index(object_dir, 1); + m = load_multi_pack_index(the_repository, object_dir, 1); if (!m) return 1; @@ -82,7 +82,7 @@ static int read_midx_checksum(const char *object_dir) struct multi_pack_index *m; setup_git_directory(); - m = load_multi_pack_index(object_dir, 1); + m = load_multi_pack_index(the_repository, object_dir, 1); if (!m) return 1; printf("%s\n", hash_to_hex(get_midx_checksum(m))); @@ -98,7 +98,7 @@ static int read_midx_preferred_pack(const char *object_dir) setup_git_directory(); - midx = load_multi_pack_index(object_dir, 1); + midx = load_multi_pack_index(the_repository, object_dir, 1); if (!midx) return 1; @@ -121,7 +121,7 @@ static int read_midx_bitmapped_packs(const char *object_dir) setup_git_directory(); - midx = load_multi_pack_index(object_dir, 1); + midx = load_multi_pack_index(the_repository, object_dir, 1); if (!midx) return 1; -- GitLab From 35bdc6d6b1bd30cef38ec75f6536b49f0a6ccc07 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 12 Nov 2024 13:09:13 +0100 Subject: [PATCH 33/34] midx: pass down `hash_algo` to functions using global variables The functions `get_split_midx_filename_ext()`, `get_midx_filename()` and `get_midx_filename_ext()` use `hash_to_hex()` which internally uses the `the_hash_algo` global variable. Remove this dependency on global variables by passing down the `hash_algo` through to the functions mentioned and instead calling `hash_to_hex_algop()` along with the obtained `hash_algo`. Signed-off-by: Karthik Nayak --- midx-write.c | 22 +++++++++++----------- midx.c | 26 +++++++++++++++----------- midx.h | 10 +++++++--- pack-bitmap.c | 6 +++--- pack-revindex.c | 2 +- 5 files changed, 37 insertions(+), 29 deletions(-) diff --git a/midx-write.c b/midx-write.c index 1bc2f525691..bcd1d50eb0f 100644 --- a/midx-write.c +++ b/midx-write.c @@ -991,9 +991,10 @@ static int link_midx_to_chain(struct multi_pack_index *m) for (i = 0; i < ARRAY_SIZE(midx_exts); i++) { const unsigned char *hash = get_midx_checksum(m); - get_midx_filename_ext(&from, m->object_dir, hash, - midx_exts[i].non_split); - get_split_midx_filename_ext(&to, m->object_dir, hash, + get_midx_filename_ext(m->repo->hash_algo, &from, m->object_dir, + hash, midx_exts[i].non_split); + get_split_midx_filename_ext(m->repo->hash_algo, &to, + m->object_dir, hash, midx_exts[i].split); if (link(from.buf, to.buf) < 0 && errno != ENOENT) { @@ -1012,9 +1013,8 @@ static int link_midx_to_chain(struct multi_pack_index *m) return ret; } -static void clear_midx_files(const char *object_dir, - const char **hashes, - uint32_t hashes_nr, +static void clear_midx_files(struct repository *r, const char *object_dir, + const char **hashes, uint32_t hashes_nr, unsigned incremental) { /* @@ -1039,7 +1039,7 @@ static void clear_midx_files(const char *object_dir, } if (incremental) - get_midx_filename(&buf, object_dir); + get_midx_filename(r->hash_algo, &buf, object_dir); else get_midx_chain_filename(&buf, object_dir); @@ -1083,7 +1083,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir, "%s/pack/multi-pack-index.d/tmp_midx_XXXXXX", object_dir); else - get_midx_filename(&midx_name, object_dir); + get_midx_filename(r->hash_algo, &midx_name, object_dir); if (safe_create_leading_directories(midx_name.buf)) die_errno(_("unable to create leading directories of %s"), midx_name.buf); @@ -1440,8 +1440,8 @@ static int write_midx_internal(struct repository *r, const char *object_dir, if (link_midx_to_chain(ctx.base_midx) < 0) return -1; - get_split_midx_filename_ext(&final_midx_name, object_dir, - midx_hash, MIDX_EXT_MIDX); + get_split_midx_filename_ext(r->hash_algo, &final_midx_name, + object_dir, midx_hash, MIDX_EXT_MIDX); if (rename_tempfile(&incr, final_midx_name.buf) < 0) { error_errno(_("unable to rename new multi-pack-index layer")); @@ -1474,7 +1474,7 @@ static int write_midx_internal(struct repository *r, const char *object_dir, if (commit_lock_file(&lk) < 0) die_errno(_("could not write multi-pack-index")); - clear_midx_files(object_dir, keep_hashes, + clear_midx_files(r, object_dir, keep_hashes, ctx.num_multi_pack_indexes_before + 1, ctx.incremental); diff --git a/midx.c b/midx.c index 98ee84d4a8b..f45ea842cd6 100644 --- a/midx.c +++ b/midx.c @@ -28,17 +28,19 @@ const unsigned char *get_midx_checksum(struct multi_pack_index *m) return m->data + m->data_len - m->repo->hash_algo->rawsz; } -void get_midx_filename(struct strbuf *out, const char *object_dir) +void get_midx_filename(const struct git_hash_algo *hash_algo, + struct strbuf *out, const char *object_dir) { - get_midx_filename_ext(out, object_dir, NULL, NULL); + get_midx_filename_ext(hash_algo, out, object_dir, NULL, NULL); } -void get_midx_filename_ext(struct strbuf *out, const char *object_dir, +void get_midx_filename_ext(const struct git_hash_algo *hash_algo, + struct strbuf *out, const char *object_dir, const unsigned char *hash, const char *ext) { strbuf_addf(out, "%s/pack/multi-pack-index", object_dir); if (ext) - strbuf_addf(out, "-%s.%s", hash_to_hex(hash), ext); + strbuf_addf(out, "-%s.%s", hash_to_hex_algop(hash, hash_algo), ext); } static int midx_read_oid_fanout(const unsigned char *chunk_start, @@ -234,11 +236,13 @@ void get_midx_chain_filename(struct strbuf *buf, const char *object_dir) strbuf_addstr(buf, "/multi-pack-index-chain"); } -void get_split_midx_filename_ext(struct strbuf *buf, const char *object_dir, +void get_split_midx_filename_ext(const struct git_hash_algo *hash_algo, + struct strbuf *buf, const char *object_dir, const unsigned char *hash, const char *ext) { get_midx_chain_dirname(buf, object_dir); - strbuf_addf(buf, "/multi-pack-index-%s.%s", hash_to_hex(hash), ext); + strbuf_addf(buf, "/multi-pack-index-%s.%s", + hash_to_hex_algop(hash, hash_algo), ext); } static int open_multi_pack_index_chain(const struct git_hash_algo *hash_algo, @@ -326,8 +330,8 @@ static struct multi_pack_index *load_midx_chain_fd_st(struct repository *r, valid = 0; strbuf_reset(&buf); - get_split_midx_filename_ext(&buf, object_dir, layer.hash, - MIDX_EXT_MIDX); + get_split_midx_filename_ext(r->hash_algo, &buf, object_dir, + layer.hash, MIDX_EXT_MIDX); m = load_multi_pack_index_one(r, object_dir, buf.buf, local); if (m) { @@ -379,7 +383,7 @@ struct multi_pack_index *load_multi_pack_index(struct repository *r, struct strbuf midx_name = STRBUF_INIT; struct multi_pack_index *m; - get_midx_filename(&midx_name, object_dir); + get_midx_filename(r->hash_algo, &midx_name, object_dir); m = load_multi_pack_index_one(r, object_dir, midx_name.buf, local); @@ -822,7 +826,7 @@ void clear_midx_file(struct repository *r) { struct strbuf midx = STRBUF_INIT; - get_midx_filename(&midx, r->objects->odb->path); + get_midx_filename(r->hash_algo, &midx, r->objects->odb->path); if (r->objects && r->objects->multi_pack_index) { close_midx(r->objects->multi_pack_index); @@ -891,7 +895,7 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag struct stat sb; struct strbuf filename = STRBUF_INIT; - get_midx_filename(&filename, object_dir); + get_midx_filename(r->hash_algo, &filename, object_dir); if (!stat(filename.buf, &sb)) { error(_("multi-pack-index file exists, but failed to parse")); diff --git a/midx.h b/midx.h index 78efa28d353..9d1374cbd58 100644 --- a/midx.h +++ b/midx.h @@ -7,6 +7,7 @@ struct object_id; struct pack_entry; struct repository; struct bitmapped_pack; +struct git_hash_algo; #define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */ #define MIDX_VERSION 1 @@ -89,12 +90,15 @@ struct multi_pack_index { #define MIDX_EXT_MIDX "midx" const unsigned char *get_midx_checksum(struct multi_pack_index *m); -void get_midx_filename(struct strbuf *out, const char *object_dir); -void get_midx_filename_ext(struct strbuf *out, const char *object_dir, +void get_midx_filename(const struct git_hash_algo *hash_algo, + struct strbuf *out, const char *object_dir); +void get_midx_filename_ext(const struct git_hash_algo *hash_algo, + struct strbuf *out, const char *object_dir, const unsigned char *hash, const char *ext); void get_midx_chain_dirname(struct strbuf *buf, const char *object_dir); void get_midx_chain_filename(struct strbuf *buf, const char *object_dir); -void get_split_midx_filename_ext(struct strbuf *buf, const char *object_dir, +void get_split_midx_filename_ext(const struct git_hash_algo *hash_algo, + struct strbuf *buf, const char *object_dir, const unsigned char *hash, const char *ext); struct multi_pack_index *load_multi_pack_index(struct repository *r, diff --git a/pack-bitmap.c b/pack-bitmap.c index 0cb1b56c9d5..7b62d099cab 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -375,8 +375,8 @@ static int load_bitmap_entries_v1(struct bitmap_index *index) char *midx_bitmap_filename(struct multi_pack_index *midx) { struct strbuf buf = STRBUF_INIT; - get_midx_filename_ext(&buf, midx->object_dir, get_midx_checksum(midx), - MIDX_EXT_BITMAP); + get_midx_filename_ext(midx->repo->hash_algo, &buf, midx->object_dir, + get_midx_checksum(midx), MIDX_EXT_BITMAP); return strbuf_detach(&buf, NULL); } @@ -415,7 +415,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, if (bitmap_git->pack || bitmap_git->midx) { struct strbuf buf = STRBUF_INIT; - get_midx_filename(&buf, midx->object_dir); + get_midx_filename(midx->repo->hash_algo, &buf, midx->object_dir); trace2_data_string("bitmap", bitmap_repo(bitmap_git), "ignoring extra midx bitmap file", buf.buf); close(fd); diff --git a/pack-revindex.c b/pack-revindex.c index 22d3c234648..d3832478d99 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -383,7 +383,7 @@ int load_midx_revindex(struct multi_pack_index *m) trace2_data_string("load_midx_revindex", the_repository, "source", "rev"); - get_midx_filename_ext(&revindex_name, m->object_dir, + get_midx_filename_ext(m->repo->hash_algo, &revindex_name, m->object_dir, get_midx_checksum(m), MIDX_EXT_REV); ret = load_revindex_from_disk(revindex_name.buf, -- GitLab From 128f4f08b0b798be2bb9f0b549173f015d65d7a1 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 12 Nov 2024 14:00:51 +0100 Subject: [PATCH 34/34] midx: inline the `MIDX_MIN_SIZE` definition The `MIDX_MIN_SIZE` definition is used to check the midx_size in `local_multi_pack_index_one`. This definition relies on the `the_hash_algo` global variable. Inline this and remove the global variable usage. With this, remove `USE_THE_REPOSITORY_VARIABLE` usage from `midx.c`. Signed-off-by: Karthik Nayak --- midx.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/midx.c b/midx.c index f45ea842cd6..e0eae1c25ec 100644 --- a/midx.c +++ b/midx.c @@ -1,5 +1,3 @@ -#define USE_THE_REPOSITORY_VARIABLE - #include "git-compat-util.h" #include "config.h" #include "dir.h" @@ -94,8 +92,6 @@ static int midx_read_object_offsets(const unsigned char *chunk_start, return 0; } -#define MIDX_MIN_SIZE (MIDX_HEADER_SIZE + the_hash_algo->rawsz) - static struct multi_pack_index *load_multi_pack_index_one(struct repository *r, const char *object_dir, const char *midx_name, @@ -122,7 +118,7 @@ static struct multi_pack_index *load_multi_pack_index_one(struct repository *r, midx_size = xsize_t(st.st_size); - if (midx_size < MIDX_MIN_SIZE) { + if (midx_size < (MIDX_HEADER_SIZE + r->hash_algo->rawsz)) { error(_("multi-pack-index file %s is too small"), midx_name); goto cleanup_fail; } -- GitLab