{"thread":{"id":"52789","subject":"[PATCH 0/5] object.c: localize global the_repository variable into r","startedAt":"2020-02-12T19:19:15Z","lastAt":"2020-02-15T00:00:28Z","messageCount":17,"participants":["Parth Gala via GitGitGadget","Taylor Blau","Junio C Hamano","Eric Sunshine","parth gala","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"391609","messageId":"pull.545.git.1581535151.gitgitgadget@gmail.com","threadId":"52789","inReplyTo":null,"subject":"[PATCH 0/5] object.c: localize global the_repository variable into r","fromName":"Parth Gala via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-02-12T19:19:06Z","receivedAt":"2020-02-12T19:19:15Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"I have created this commit in response to \nhttps://github.com/gitgitgadget/git/issues/379#issue-503866117. All the\nfunctions in object.c which relied on 'the_repository' have been modified.\nThe functions calling these functions in object.c consisted calls to other\nfunctions using 'the_repository' as well, and although I intended to use 'r'\nat all those instances, I thought it would make more sense when we would\ndeal with their callee functions in another similar patch. What do you think\n?\n\nSigned-off-by: Parth Gala parthpgala@gmail.com [parthpgala@gmail.com]\n\nParth Gala (5):\n  object.c: get_max_object_index and get_indexed_object accept 'r'\n    parameter\n  object.c: lookup_unknown_object() accept 'r' as parameter\n  object.c: parse_object_or_die() accept 'r' as parameter\n  object.c: clear_object_flags() accept 'r' as parameter\n  object.c: clear_commit_marks_all() accept 'r' as parameter\n\n builtin/checkout.c               |  3 ++-\n builtin/fsck.c                   |  8 +++++---\n builtin/grep.c                   |  6 ++++--\n builtin/index-pack.c             |  5 +++--\n builtin/log.c                    |  3 ++-\n builtin/name-rev.c               |  5 +++--\n builtin/pack-objects.c           |  3 ++-\n builtin/prune.c                  |  3 ++-\n bundle.c                         |  8 +++++---\n http-push.c                      |  3 ++-\n object.c                         | 32 ++++++++++++++++----------------\n object.h                         | 12 ++++++------\n pack-bitmap.c                    |  5 +++--\n reachable.c                      |  6 ++++--\n refs.c                           |  3 ++-\n revision.c                       |  3 ++-\n shallow.c                        | 13 ++++++++-----\n t/helper/test-example-decorate.c |  7 ++++---\n upload-pack.c                    | 19 ++++++++++++-------\n walker.c                         |  3 ++-\n 20 files changed, 89 insertions(+), 61 deletions(-)\n\n\nbase-commit: 0ad714499976290d9a0229230cbe4efae930b8dc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-545%2FParthGala2k%2Flocalize-the_repository-variable-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-545/ParthGala2k/localize-the_repository-variable-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/545\n-- \ngitgitgadget\n"},{"id":"391610","messageId":"ef8ec98181ff03d756be80f1d3796eda98c7f475.1581535151.git.gitgitgadget@gmail.com","threadId":"52789","inReplyTo":"pull.545.git.1581535151.gitgitgadget@gmail.com","subject":"[PATCH 3/5] object.c: parse_object_or_die() accept 'r' as parameter","fromName":"Parth Gala via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-02-12T19:19:09Z","receivedAt":"2020-02-12T19:19:18Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"From: Parth Gala <parthpgala@gmail.com>\n\n'parse_object_or_die()' and its callers are modified to enable\npassing 'r' as an argument to 'parse_object_or_die()'.\n\nSigned-off-by: Parth Gala <parthpgala@gmail.com>\n---\n builtin/grep.c  | 6 ++++--\n builtin/prune.c | 3 ++-\n bundle.c        | 8 +++++---\n object.c        | 4 ++--\n object.h        | 2 +-\n pack-bitmap.c   | 5 +++--\n reachable.c     | 6 ++++--\n upload-pack.c   | 4 +++-\n 8 files changed, 24 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 50ce8d9461..c4156b0560 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -406,6 +406,7 @@ static int grep_submodule(struct grep_opt *opt,\n \t\t\t  const char *filename, const char *path, int cached)\n {\n \tstruct repository subrepo;\n+\tstruct repository *r = the_repository;\n \tstruct repository *superproject = opt->repo;\n \tconst struct submodule *sub = submodule_from_path(superproject,\n \t\t\t\t\t\t\t  &null_oid, path);\n@@ -455,7 +456,7 @@ static int grep_submodule(struct grep_opt *opt,\n \t\tunsigned long size;\n \t\tstruct strbuf base = STRBUF_INIT;\n \n-\t\tobject = parse_object_or_die(oid, oid_to_hex(oid));\n+\t\tobject = parse_object_or_die(r, oid, oid_to_hex(oid));\n \n \t\tgrep_read_lock();\n \t\tdata = read_object_with_reference(&subrepo,\n@@ -802,6 +803,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n \tstruct grep_opt opt;\n \tstruct object_array list = OBJECT_ARRAY_INIT;\n+\tstruct repository *r = the_repository;\n \tstruct pathspec pathspec;\n \tstruct string_list path_list = STRING_LIST_INIT_NODUP;\n \tint i;\n@@ -1037,7 +1039,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tbreak;\n \t\t}\n \n-\t\tobject = parse_object_or_die(&oid, arg);\n+\t\tobject = parse_object_or_die(r, &oid, arg);\n \t\tif (!seen_dashdash)\n \t\t\tverify_non_filename(prefix, arg);\n \t\tadd_object_array_with_path(object, arg, &list, oc.mode, oc.path);\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex 2b76872ad2..6d478717ef 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -125,6 +125,7 @@ static void remove_temporary_files(const char *path)\n int cmd_prune(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n+\tstruct repository *r = the_repository;\n \tint exclude_promisor_objects = 0;\n \tconst struct option options[] = {\n \t\tOPT__DRY_RUN(&show_only, N_(\"do not remove, show only\")),\n@@ -154,7 +155,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)\n \t\tconst char *name = *argv++;\n \n \t\tif (!get_oid(name, &oid)) {\n-\t\t\tstruct object *object = parse_object_or_die(&oid,\n+\t\t\tstruct object *object = parse_object_or_die(r, &oid,\n \t\t\t\t\t\t\t\t    name);\n \t\t\tadd_pending_object(&revs, object, \"\");\n \t\t}\ndiff --git a/bundle.c b/bundle.c\nindex 99439e07a1..26231f2a38 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -298,6 +298,7 @@ static int compute_and_write_prerequisites(int bundle_fd,\n {\n \tstruct child_process rls = CHILD_PROCESS_INIT;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct repository *r = the_repository;\n \tFILE *rls_fout;\n \tint i;\n \n@@ -316,13 +317,13 @@ static int compute_and_write_prerequisites(int bundle_fd,\n \t\tif (buf.len > 0 && buf.buf[0] == '-') {\n \t\t\twrite_or_die(bundle_fd, buf.buf, buf.len);\n \t\t\tif (!get_oid_hex(buf.buf + 1, &oid)) {\n-\t\t\t\tstruct object *object = parse_object_or_die(&oid,\n+\t\t\t\tstruct object *object = parse_object_or_die(r, &oid,\n \t\t\t\t\t\t\t\t\t    buf.buf);\n \t\t\t\tobject->flags |= UNINTERESTING;\n \t\t\t\tadd_pending_object(revs, object, buf.buf);\n \t\t\t}\n \t\t} else if (!get_oid_hex(buf.buf, &oid)) {\n-\t\t\tstruct object *object = parse_object_or_die(&oid,\n+\t\t\tstruct object *object = parse_object_or_die(r, &oid,\n \t\t\t\t\t\t\t\t    buf.buf);\n \t\t\tobject->flags |= SHOWN;\n \t\t}\n@@ -347,6 +348,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)\n {\n \tint i;\n \tint ref_count = 0;\n+\tstruct repository *r = the_repository;\n \n \tfor (i = 0; i < revs->pending.nr; i++) {\n \t\tstruct object_array_entry *e = revs->pending.objects + i;\n@@ -407,7 +409,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)\n \t\t\t\t * end up triggering \"empty bundle\"\n \t\t\t\t * error.\n \t\t\t\t */\n-\t\t\t\tobj = parse_object_or_die(&oid, e->name);\n+\t\t\t\tobj = parse_object_or_die(r, &oid, e->name);\n \t\t\t\tobj->flags |= SHOWN;\n \t\t\t\tadd_pending_object(revs, obj, e->name);\n \t\t\t}\ndiff --git a/object.c b/object.c\nindex 90338a509c..0a7a278c88 100644\n--- a/object.c\n+++ b/object.c\n@@ -236,10 +236,10 @@ struct object *parse_object_buffer(struct repository *r, const struct object_id\n \treturn obj;\n }\n \n-struct object *parse_object_or_die(const struct object_id *oid,\n+struct object *parse_object_or_die(struct repository *r, const struct object_id *oid,\n \t\t\t\t   const char *name)\n {\n-\tstruct object *o = parse_object(the_repository, oid);\n+\tstruct object *o = parse_object(r, oid);\n \tif (o)\n \t\treturn o;\n \ndiff --git a/object.h b/object.h\nindex 375236cec3..92af2ead8f 100644\n--- a/object.h\n+++ b/object.h\n@@ -135,7 +135,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid);\n  * \"name\" parameter is not NULL, it is included in the error message\n  * (otherwise, the hex object ID is given).\n  */\n-struct object *parse_object_or_die(const struct object_id *oid, const char *name);\n+struct object *parse_object_or_die(struct repository *, const struct object_id *oid, const char *name);\n \n /* Given the result of read_sha1_file(), returns the object after\n  * parsing it.  eaten_p indicates if the object has a borrowed copy\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex e07c798879..b7f9aebc7b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -682,6 +682,7 @@ static int in_bitmapped_pack(struct bitmap_index *bitmap_git,\n struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs)\n {\n \tunsigned int i;\n+\tstruct repository *r = the_repository;\n \n \tstruct object_list *wants = NULL;\n \tstruct object_list *haves = NULL;\n@@ -699,7 +700,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs)\n \t\tstruct object *object = revs->pending.objects[i].item;\n \n \t\tif (object->type == OBJ_NONE)\n-\t\t\tparse_object_or_die(&object->oid, NULL);\n+\t\t\tparse_object_or_die(r, &object->oid, NULL);\n \n \t\twhile (object->type == OBJ_TAG) {\n \t\t\tstruct tag *tag = (struct tag *) object;\n@@ -709,7 +710,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info *revs)\n \t\t\telse\n \t\t\t\tobject_list_insert(object, &wants);\n \n-\t\t\tobject = parse_object_or_die(get_tagged_oid(tag), NULL);\n+\t\t\tobject = parse_object_or_die(r, get_tagged_oid(tag), NULL);\n \t\t}\n \n \t\tif (object->flags & UNINTERESTING)\ndiff --git a/reachable.c b/reachable.c\nindex 8f50235b28..c661a1c892 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -31,13 +31,14 @@ static int add_one_ref(const char *path, const struct object_id *oid,\n {\n \tstruct rev_info *revs = (struct rev_info *)cb_data;\n \tstruct object *object;\n+\tstruct repository *r = the_repository;\n \n \tif ((flag & REF_ISSYMREF) && (flag & REF_ISBROKEN)) {\n \t\twarning(\"symbolic ref is dangling: %s\", path);\n \t\treturn 0;\n \t}\n \n-\tobject = parse_object_or_die(oid, path);\n+\tobject = parse_object_or_die(r, oid, path);\n \tadd_pending_object(revs, object, \"\");\n \n \treturn 0;\n@@ -68,6 +69,7 @@ static void add_recent_object(const struct object_id *oid,\n {\n \tstruct object *obj;\n \tenum object_type type;\n+\tstruct repository *r = the_repository;\n \n \tif (mtime <= data->timestamp)\n \t\treturn;\n@@ -86,7 +88,7 @@ static void add_recent_object(const struct object_id *oid,\n \tswitch (type) {\n \tcase OBJ_TAG:\n \tcase OBJ_COMMIT:\n-\t\tobj = parse_object_or_die(oid, NULL);\n+\t\tobj = parse_object_or_die(r, oid, NULL);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\tobj = (struct object *)lookup_tree(the_repository, oid);\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 6d196e275b..daea9059f0 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1210,6 +1210,8 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,\n \t\t\t  struct object_array *want_obj)\n {\n \tconst char *arg;\n+\tstruct repository *r = the_repository;\n+\n \tif (skip_prefix(line, \"want-ref \", &arg)) {\n \t\tstruct object_id oid;\n \t\tstruct string_list_item *item;\n@@ -1223,7 +1225,7 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,\n \t\titem = string_list_append(wanted_refs, arg);\n \t\titem->util = oiddup(&oid);\n \n-\t\to = parse_object_or_die(&oid, arg);\n+\t\to = parse_object_or_die(r, &oid, arg);\n \t\tif (!(o->flags & WANTED)) {\n \t\t\to->flags |= WANTED;\n \t\t\tadd_object_array(o, NULL, want_obj);\n-- \ngitgitgadget\n\n"},{"id":"391611","messageId":"d98b7caff56cc839f251ac43c36826ecf800cf2f.1581535151.git.gitgitgadget@gmail.com","threadId":"52789","inReplyTo":"pull.545.git.1581535151.gitgitgadget@gmail.com","subject":"[PATCH 1/5] object.c: get_max_object_index and get_indexed_object accept 'r' parameter","fromName":"Parth Gala via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-02-12T19:19:07Z","receivedAt":"2020-02-12T19:19:19Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"From: Parth Gala <parthpgala@gmail.com>\n\nCurrently the two functions use global variable 'the_repository' to access\nthe values stored in it. This makes 'the_repository' to be existent even\nwhen not required.\n\nThis commit replaces it with 'r' which is passed as a parameter to those\nfunctions.\n\nSigned-off-by: Parth Gala <parthpgala@gmail.com>\n---\n builtin/fsck.c       |  5 +++--\n builtin/index-pack.c |  5 +++--\n builtin/name-rev.c   |  5 +++--\n object.c             |  8 ++++----\n object.h             |  4 ++--\n shallow.c            | 10 ++++++----\n upload-pack.c        | 10 ++++++----\n 7 files changed, 27 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 8d13794b14..d2b4336f7e 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -375,6 +375,7 @@ static void check_object(struct object *obj)\n static void check_connectivity(void)\n {\n \tint i, max;\n+\tstruct repository *r = the_repository;\n \n \t/* Traverse the pending reachable objects */\n \ttraverse_reachable();\n@@ -400,12 +401,12 @@ static void check_connectivity(void)\n \t}\n \n \t/* Look up all the requirements, warn about missing objects.. */\n-\tmax = get_max_object_index();\n+\tmax = get_max_object_index(r);\n \tif (verbose)\n \t\tfprintf_ln(stderr, _(\"Checking connectivity (%d objects)\"), max);\n \n \tfor (i = 0; i < max; i++) {\n-\t\tstruct object *obj = get_indexed_object(i);\n+\t\tstruct object *obj = get_indexed_object(r, i);\n \n \t\tif (obj)\n \t\t\tcheck_object(obj);\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 60a5591039..d2115535bc 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -218,14 +218,15 @@ static unsigned check_object(struct object *obj)\n static unsigned check_objects(void)\n {\n \tunsigned i, max, foreign_nr = 0;\n+\tstruct repository *r = the_repository;\n \n-\tmax = get_max_object_index();\n+\tmax = get_max_object_index(r);\n \n \tif (verbose)\n \t\tprogress = start_delayed_progress(_(\"Checking objects\"), max);\n \n \tfor (i = 0; i < max; i++) {\n-\t\tforeign_nr += check_object(get_indexed_object(i));\n+\t\tforeign_nr += check_object(get_indexed_object(r, i));\n \t\tdisplay_progress(progress, i + 1);\n \t}\n \ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex 6b9e8c850b..afe9f6df01 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -456,6 +456,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)\n int cmd_name_rev(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_array revs = OBJECT_ARRAY_INIT;\n+\tstruct repository *r = the_repository;\n \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n \tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n \tstruct option opts[] = {\n@@ -553,9 +554,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t} else if (all) {\n \t\tint i, max;\n \n-\t\tmax = get_max_object_index();\n+\t\tmax = get_max_object_index(r);\n \t\tfor (i = 0; i < max; i++) {\n-\t\t\tstruct object *obj = get_indexed_object(i);\n+\t\t\tstruct object *obj = get_indexed_object(r, i);\n \t\t\tif (!obj || obj->type != OBJ_COMMIT)\n \t\t\t\tcontinue;\n \t\t\tshow_name(obj, NULL,\ndiff --git a/object.c b/object.c\nindex 142ef69399..549fbe69ca 100644\n--- a/object.c\n+++ b/object.c\n@@ -10,14 +10,14 @@\n #include \"packfile.h\"\n #include \"commit-graph.h\"\n \n-unsigned int get_max_object_index(void)\n+unsigned int get_max_object_index(struct repository *r)\n {\n-\treturn the_repository->parsed_objects->obj_hash_size;\n+\treturn r->parsed_objects->obj_hash_size;\n }\n \n-struct object *get_indexed_object(unsigned int idx)\n+struct object *get_indexed_object(struct repository *r, unsigned int idx)\n {\n-\treturn the_repository->parsed_objects->obj_hash[idx];\n+\treturn r->parsed_objects->obj_hash[idx];\n }\n \n static const char *object_type_strings[] = {\ndiff --git a/object.h b/object.h\nindex 25f5ab3d54..5a8ae274ee 100644\n--- a/object.h\n+++ b/object.h\n@@ -98,12 +98,12 @@ int type_from_string_gently(const char *str, ssize_t, int gentle);\n /*\n  * Return the current number of buckets in the object hashmap.\n  */\n-unsigned int get_max_object_index(void);\n+unsigned int get_max_object_index(struct repository *);\n \n /*\n  * Return the object from the specified bucket in the object hashmap.\n  */\n-struct object *get_indexed_object(unsigned int);\n+struct object *get_indexed_object(struct repository *, unsigned int);\n \n /*\n  * This can be used to see if we have heard of the object before, but\ndiff --git a/shallow.c b/shallow.c\nindex 7fd04afed1..4537d98482 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -510,6 +510,7 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,\n \t\t       unsigned int id)\n {\n \tunsigned int i, nr;\n+\tstruct repository *r = the_repository;\n \tstruct commit_list *head = NULL;\n \tint bitmap_nr = DIV_ROUND_UP(info->nr_bits, 32);\n \tsize_t bitmap_size = st_mult(sizeof(uint32_t), bitmap_nr);\n@@ -563,9 +564,9 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,\n \t\t}\n \t}\n \n-\tnr = get_max_object_index();\n+\tnr = get_max_object_index(r);\n \tfor (i = 0; i < nr; i++) {\n-\t\tstruct object *o = get_indexed_object(i);\n+\t\tstruct object *o = get_indexed_object(r, i);\n \t\tif (o && o->type == OBJ_COMMIT)\n \t\t\to->flags &= ~SEEN;\n \t}\n@@ -608,6 +609,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,\n \tstruct object_id *oid = info->shallow->oid;\n \tstruct oid_array *ref = info->ref;\n \tunsigned int i, nr;\n+\tstruct repository *r = the_repository;\n \tint *shallow, nr_shallow = 0;\n \tstruct paint_info pi;\n \n@@ -622,9 +624,9 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,\n \t * Prepare the commit graph to track what refs can reach what\n \t * (new) shallow commits.\n \t */\n-\tnr = get_max_object_index();\n+\tnr = get_max_object_index(r);\n \tfor (i = 0; i < nr; i++) {\n-\t\tstruct object *o = get_indexed_object(i);\n+\t\tstruct object *o = get_indexed_object(r, i);\n \t\tif (!o || o->type != OBJ_COMMIT)\n \t\t\tcontinue;\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex a00d7ece6b..cb7312268f 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -450,6 +450,7 @@ static int do_reachable_revlist(struct child_process *cmd,\n \t\t\"rev-list\", \"--stdin\", NULL,\n \t};\n \tstruct object *o;\n+\tstruct repository *r = the_repository;\n \tchar namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */\n \tint i;\n \tconst unsigned hexsz = the_hash_algo->hexsz;\n@@ -472,8 +473,8 @@ static int do_reachable_revlist(struct child_process *cmd,\n \n \tnamebuf[0] = '^';\n \tnamebuf[hexsz + 1] = '\\n';\n-\tfor (i = get_max_object_index(); 0 < i; ) {\n-\t\to = get_indexed_object(--i);\n+\tfor (i = get_max_object_index(r); 0 < i; ) {\n+\t\to = get_indexed_object(r, --i);\n \t\tif (!o)\n \t\t\tcontinue;\n \t\tif (reachable && o->type == OBJ_COMMIT)\n@@ -520,6 +521,7 @@ static int get_reachable_list(struct object_array *src,\n \tstruct child_process cmd = CHILD_PROCESS_INIT;\n \tint i;\n \tstruct object *o;\n+\tstruct repository *r = the_repository;\n \tchar namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */\n \tconst unsigned hexsz = the_hash_algo->hexsz;\n \n@@ -538,8 +540,8 @@ static int get_reachable_list(struct object_array *src,\n \t\t\to->flags &= ~TMP_MARK;\n \t\t}\n \t}\n-\tfor (i = get_max_object_index(); 0 < i; i--) {\n-\t\to = get_indexed_object(i - 1);\n+\tfor (i = get_max_object_index(r); 0 < i; i--) {\n+\t\to = get_indexed_object(r, i - 1);\n \t\tif (o && o->type == OBJ_COMMIT &&\n \t\t    (o->flags & TMP_MARK)) {\n \t\t\tadd_object_array(o, NULL, reachable);\n-- \ngitgitgadget\n\n"},{"id":"391612","messageId":"a8202cc5353d274f6278f2350ae1371af0a093d5.1581535151.git.gitgitgadget@gmail.com","threadId":"52789","inReplyTo":"pull.545.git.1581535151.gitgitgadget@gmail.com","subject":"[PATCH 4/5] object.c: clear_object_flags() accept 'r' as parameter","fromName":"Parth Gala via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-02-12T19:19:10Z","receivedAt":"2020-02-12T19:19:20Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"From: Parth Gala <parthpgala@gmail.com>\n\n'clear_object_flags()' and its callers are modified to enable\npassing 'r' as an argument to 'clear_object_flags()'.\n\nSigned-off-by: Parth Gala <parthpgala@gmail.com>\n---\n builtin/log.c | 3 ++-\n object.c      | 6 +++---\n object.h      | 2 +-\n revision.c    | 3 ++-\n shallow.c     | 3 ++-\n upload-pack.c | 2 +-\n 6 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 83a4a6188e..b163d2fbdf 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1592,6 +1592,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit *commit;\n \tstruct commit **list = NULL;\n+\tstruct repository *r = the_repository;\n \tstruct rev_info rev;\n \tstruct setup_revision_opt s_r_opt;\n \tint nr = 0, total, i;\n@@ -1998,7 +1999,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (base_commit) {\n \t\tstruct commit *base = get_base_commit(base_commit, list, nr);\n \t\treset_revision_walk();\n-\t\tclear_object_flags(UNINTERESTING);\n+\t\tclear_object_flags(r, UNINTERESTING);\n \t\tprepare_bases(&bases, base, list, nr);\n \t}\n \ndiff --git a/object.c b/object.c\nindex 0a7a278c88..804488c8dd 100644\n--- a/object.c\n+++ b/object.c\n@@ -432,12 +432,12 @@ void object_array_remove_duplicates(struct object_array *array)\n \t}\n }\n \n-void clear_object_flags(unsigned flags)\n+void clear_object_flags(struct repository *r, unsigned flags)\n {\n \tint i;\n \n-\tfor (i=0; i < the_repository->parsed_objects->obj_hash_size; i++) {\n-\t\tstruct object *obj = the_repository->parsed_objects->obj_hash[i];\n+\tfor (i=0; i < r->parsed_objects->obj_hash_size; i++) {\n+\t\tstruct object *obj = r->parsed_objects->obj_hash[i];\n \t\tif (obj)\n \t\t\tobj->flags &= ~flags;\n \t}\ndiff --git a/object.h b/object.h\nindex 92af2ead8f..bfc95e062a 100644\n--- a/object.h\n+++ b/object.h\n@@ -185,7 +185,7 @@ void object_array_remove_duplicates(struct object_array *array);\n  */\n void object_array_clear(struct object_array *array);\n \n-void clear_object_flags(unsigned flags);\n+void clear_object_flags(struct repository *, unsigned flags);\n \n /*\n  * Clear the specified object flags from all in-core commit objects.\ndiff --git a/revision.c b/revision.c\nindex 8136929e23..ac32ecf2e2 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3086,7 +3086,8 @@ static void set_children(struct rev_info *revs)\n \n void reset_revision_walk(void)\n {\n-\tclear_object_flags(SEEN | ADDED | SHOWN | TOPO_WALK_EXPLORED | TOPO_WALK_INDEGREE);\n+\tstruct repository *r = the_repository;\n+\tclear_object_flags(r, SEEN | ADDED | SHOWN | TOPO_WALK_EXPLORED | TOPO_WALK_INDEGREE);\n }\n \n static int mark_uninteresting(const struct object_id *oid,\ndiff --git a/shallow.c b/shallow.c\nindex 4537d98482..d32adbe088 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -180,6 +180,7 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, const char **av,\n {\n \tstruct commit_list *result = NULL, *p;\n \tstruct commit_list *not_shallow_list = NULL;\n+\tstruct repository *r = the_repository;\n \tstruct rev_info revs;\n \tint both_flags = shallow_flag | not_shallow_flag;\n \n@@ -187,7 +188,7 @@ struct commit_list *get_shallow_commits_by_rev_list(int ac, const char **av,\n \t * SHALLOW (excluded) and NOT_SHALLOW (included) should not be\n \t * set at this point. But better be safe than sorry.\n \t */\n-\tclear_object_flags(both_flags);\n+\tclear_object_flags(r, both_flags);\n \n \tis_repository_shallow(the_repository); /* make sure shallows are read */\n \ndiff --git a/upload-pack.c b/upload-pack.c\nindex daea9059f0..e296f2cfa8 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1472,7 +1472,7 @@ int upload_pack_v2(struct repository *r, struct argv_array *keys,\n \tstruct object_array have_obj = OBJECT_ARRAY_INIT;\n \tstruct object_array want_obj = OBJECT_ARRAY_INIT;\n \n-\tclear_object_flags(ALL_FLAGS);\n+\tclear_object_flags(r, ALL_FLAGS);\n \n \tgit_config(upload_pack_config, NULL);\n \n-- \ngitgitgadget\n\n"},{"id":"391613","messageId":"16dd64618ab6a086727685e9eca3850fabc46437.1581535151.git.gitgitgadget@gmail.com","threadId":"52789","inReplyTo":"pull.545.git.1581535151.gitgitgadget@gmail.com","subject":"[PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter","fromName":"Parth Gala via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-02-12T19:19:08Z","receivedAt":"2020-02-12T19:19:21Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"From: Parth Gala <parthpgala@gmail.com>\n\n'lookup_unknown_object()' and its callers are modified to enable\npassing 'r' as an argument to 'lookup_unknown_object()' in an\neffort to reduce dependence on global 'the_repository' variable.\n\nSigned-off-by: Parth Gala <parthpgala@gmail.com>\n---\n builtin/fsck.c                   | 3 ++-\n builtin/pack-objects.c           | 3 ++-\n http-push.c                      | 3 ++-\n object.c                         | 8 ++++----\n object.h                         | 2 +-\n refs.c                           | 3 ++-\n t/helper/test-example-decorate.c | 7 ++++---\n upload-pack.c                    | 3 ++-\n walker.c                         | 3 ++-\n 9 files changed, 21 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex d2b4336f7e..cd0b67f3bc 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -745,7 +745,8 @@ static int fsck_cache_tree(struct cache_tree *it)\n \n static void mark_object_for_connectivity(const struct object_id *oid)\n {\n-\tstruct object *obj = lookup_unknown_object(oid);\n+\tstruct repository *r = the_repository;\n+\tstruct object *obj = lookup_unknown_object(r, oid);\n \tobj->flags |= HAS_OBJ;\n }\n \ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 393c20a2d7..b03f4378a0 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2891,6 +2891,7 @@ static void add_objects_in_unpacked_packs(void)\n {\n \tstruct packed_git *p;\n \tstruct in_pack in_pack;\n+\tstruct repository *r = the_repository;\n \tuint32_t i;\n \n \tmemset(&in_pack, 0, sizeof(in_pack));\n@@ -2910,7 +2911,7 @@ static void add_objects_in_unpacked_packs(void)\n \n \t\tfor (i = 0; i < p->num_objects; i++) {\n \t\t\tnth_packed_object_oid(&oid, p, i);\n-\t\t\to = lookup_unknown_object(&oid);\n+\t\t\to = lookup_unknown_object(r, &oid);\n \t\t\tif (!(o->flags & OBJECT_ADDED))\n \t\t\t\tmark_in_pack_object(o, p, &in_pack);\n \t\t\to->flags |= OBJECT_ADDED;\ndiff --git a/http-push.c b/http-push.c\nindex 822f326599..c26d03b21b 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1416,6 +1416,7 @@ static void one_remote_ref(const char *refname)\n {\n \tstruct ref *ref;\n \tstruct object *obj;\n+\tstruct repository *r = the_repository;\n \n \tref = alloc_ref(refname);\n \n@@ -1432,7 +1433,7 @@ static void one_remote_ref(const char *refname)\n \t * may be required for updating server info later.\n \t */\n \tif (repo->can_update_info_refs && !has_object_file(&ref->old_oid)) {\n-\t\tobj = lookup_unknown_object(&ref->old_oid);\n+\t\tobj = lookup_unknown_object(r, &ref->old_oid);\n \t\tfprintf(stderr,\t\"  fetch %s for %s\\n\",\n \t\t\toid_to_hex(&ref->old_oid), refname);\n \t\tadd_fetch_request(obj);\ndiff --git a/object.c b/object.c\nindex 549fbe69ca..90338a509c 100644\n--- a/object.c\n+++ b/object.c\n@@ -177,12 +177,12 @@ void *object_as_type(struct repository *r, struct object *obj, enum object_type\n \t}\n }\n \n-struct object *lookup_unknown_object(const struct object_id *oid)\n+struct object *lookup_unknown_object(struct repository *r, const struct object_id *oid)\n {\n-\tstruct object *obj = lookup_object(the_repository, oid);\n+\tstruct object *obj = lookup_object(r, oid);\n \tif (!obj)\n-\t\tobj = create_object(the_repository, oid,\n-\t\t\t\t    alloc_object_node(the_repository));\n+\t\tobj = create_object(r, oid,\n+\t\t\t\t    alloc_object_node(r));\n \treturn obj;\n }\n \ndiff --git a/object.h b/object.h\nindex 5a8ae274ee..375236cec3 100644\n--- a/object.h\n+++ b/object.h\n@@ -144,7 +144,7 @@ struct object *parse_object_or_die(const struct object_id *oid, const char *name\n struct object *parse_object_buffer(struct repository *r, const struct object_id *oid, enum object_type type, unsigned long size, void *buffer, int *eaten_p);\n \n /** Returns the object, with potentially excess memory allocated. **/\n-struct object *lookup_unknown_object(const struct object_id *oid);\n+struct object *lookup_unknown_object(struct repository *, const struct object_id *oid);\n \n struct object_list *object_list_insert(struct object *item,\n \t\t\t\t       struct object_list **list_p);\ndiff --git a/refs.c b/refs.c\nindex 1ab0bb54d3..a630a8c271 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -379,7 +379,8 @@ static int filter_refs(const char *refname, const struct object_id *oid,\n \n enum peel_status peel_object(const struct object_id *name, struct object_id *oid)\n {\n-\tstruct object *o = lookup_unknown_object(name);\n+\tstruct repository *r = the_repository;\n+\tstruct object *o = lookup_unknown_object(r, name);\n \n \tif (o->type == OBJ_NONE) {\n \t\tint type = oid_object_info(the_repository, name, NULL);\ndiff --git a/t/helper/test-example-decorate.c b/t/helper/test-example-decorate.c\nindex c8a1cde7d2..6b3262a9d3 100644\n--- a/t/helper/test-example-decorate.c\n+++ b/t/helper/test-example-decorate.c\n@@ -10,6 +10,7 @@ int cmd__example_decorate(int argc, const char **argv)\n \tstruct object_id two_oid = { {2} };\n \tstruct object_id three_oid = { {3} };\n \tstruct object *one, *two, *three;\n+\tstruct repository *r = the_repository;\n \n \tint decoration_a, decoration_b;\n \n@@ -26,8 +27,8 @@ int cmd__example_decorate(int argc, const char **argv)\n \t * Add 2 objects, one with a non-NULL decoration and one with a NULL\n \t * decoration.\n \t */\n-\tone = lookup_unknown_object(&one_oid);\n-\ttwo = lookup_unknown_object(&two_oid);\n+\tone = lookup_unknown_object(r, &one_oid);\n+\ttwo = lookup_unknown_object(r, &two_oid);\n \tret = add_decoration(&n, one, &decoration_a);\n \tif (ret)\n \t\tBUG(\"when adding a brand-new object, NULL should be returned\");\n@@ -56,7 +57,7 @@ int cmd__example_decorate(int argc, const char **argv)\n \tret = lookup_decoration(&n, two);\n \tif (ret != &decoration_b)\n \t\tBUG(\"lookup should return added declaration\");\n-\tthree = lookup_unknown_object(&three_oid);\n+\tthree = lookup_unknown_object(r, &three_oid);\n \tret = lookup_decoration(&n, three);\n \tif (ret)\n \t\tBUG(\"lookup for unknown object should return NULL\");\ndiff --git a/upload-pack.c b/upload-pack.c\nindex cb7312268f..6d196e275b 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -962,7 +962,8 @@ static void receive_needs(struct packet_reader *reader, struct object_array *wan\n static int mark_our_ref(const char *refname, const char *refname_full,\n \t\t\tconst struct object_id *oid)\n {\n-\tstruct object *o = lookup_unknown_object(oid);\n+\tstruct repository *r = the_repository;\n+\tstruct object *o = lookup_unknown_object(r, oid);\n \n \tif (ref_is_hidden(refname, refname_full)) {\n \t\to->flags |= HIDDEN_REF;\ndiff --git a/walker.c b/walker.c\nindex 06cd2bd569..098c69ebe1 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -258,6 +258,7 @@ void walker_targets_free(int targets, char **target, const char **write_ref)\n int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t const char **write_ref, const char *write_ref_log_details)\n {\n+\tstruct repository *r = the_repository;\n \tstruct strbuf refname = STRBUF_INIT;\n \tstruct strbuf err = STRBUF_INIT;\n \tstruct ref_transaction *transaction = NULL;\n@@ -285,7 +286,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n \t\t\terror(\"Could not interpret response from server '%s' as something to pull\", target[i]);\n \t\t\tgoto done;\n \t\t}\n-\t\tif (process(walker, lookup_unknown_object(&oids[i])))\n+\t\tif (process(walker, lookup_unknown_object(r, &oids[i])))\n \t\t\tgoto done;\n \t}\n \n-- \ngitgitgadget\n\n"},{"id":"391614","messageId":"746d9ff810c384db8568c75472b18f8a9afec5fb.1581535151.git.gitgitgadget@gmail.com","threadId":"52789","inReplyTo":"pull.545.git.1581535151.gitgitgadget@gmail.com","subject":"[PATCH 5/5] object.c: clear_commit_marks_all() accept 'r' as parameter","fromName":"Parth Gala via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-02-12T19:19:11Z","receivedAt":"2020-02-12T19:19:21Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"From: Parth Gala <parthpgala@gmail.com>\n\n'clear_commit_marks_all()' and its callers are modified to enable\npassing 'r' as an argument to 'clear_commit_marks_all()'.\n\nSigned-off-by: Parth Gala <parthpgala@gmail.com>\n---\n builtin/checkout.c | 3 ++-\n object.c           | 6 +++---\n object.h           | 2 +-\n 3 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b52c490c8f..bd2b8de8b7 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -989,6 +989,7 @@ static void suggest_reattach(struct commit *commit, struct rev_info *revs)\n static void orphaned_commit_warning(struct commit *old_commit, struct commit *new_commit)\n {\n \tstruct rev_info revs;\n+\tstruct repository *r = the_repository;\n \tstruct object *object = &old_commit->object;\n \n \trepo_init_revisions(the_repository, &revs, NULL);\n@@ -1011,7 +1012,7 @@ static void orphaned_commit_warning(struct commit *old_commit, struct commit *ne\n \t\tdescribe_detached_head(_(\"Previous HEAD position was\"), old_commit);\n \n \t/* Clean up objects used, as they will be reused. */\n-\tclear_commit_marks_all(ALL_REV_FLAGS);\n+\tclear_commit_marks_all(r, ALL_REV_FLAGS);\n }\n \n static int switch_branches(const struct checkout_opts *opts,\ndiff --git a/object.c b/object.c\nindex 804488c8dd..2bc08d8df7 100644\n--- a/object.c\n+++ b/object.c\n@@ -443,12 +443,12 @@ void clear_object_flags(struct repository *r, unsigned flags)\n \t}\n }\n \n-void clear_commit_marks_all(unsigned int flags)\n+void clear_commit_marks_all(struct repository *r, unsigned int flags)\n {\n \tint i;\n \n-\tfor (i = 0; i < the_repository->parsed_objects->obj_hash_size; i++) {\n-\t\tstruct object *obj = the_repository->parsed_objects->obj_hash[i];\n+\tfor (i = 0; i < r->parsed_objects->obj_hash_size; i++) {\n+\t\tstruct object *obj = r->parsed_objects->obj_hash[i];\n \t\tif (obj && obj->type == OBJ_COMMIT)\n \t\t\tobj->flags &= ~flags;\n \t}\ndiff --git a/object.h b/object.h\nindex bfc95e062a..b0f0bce0a3 100644\n--- a/object.h\n+++ b/object.h\n@@ -190,6 +190,6 @@ void clear_object_flags(struct repository *, unsigned flags);\n /*\n  * Clear the specified object flags from all in-core commit objects.\n  */\n-void clear_commit_marks_all(unsigned int flags);\n+void clear_commit_marks_all(struct repository *, unsigned int flags);\n \n #endif /* OBJECT_H */\n-- \ngitgitgadget\n"},{"id":"391616","messageId":"20200212201810.GB4364@syl.local","threadId":"52789","inReplyTo":"pull.545.git.1581535151.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] object.c: localize global the_repository variable into r","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-02-12T20:18:10Z","receivedAt":"2020-02-12T20:18:15Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Parth,\n\nOn Wed, Feb 12, 2020 at 07:19:06PM +0000, Parth Gala via GitGitGadget wrote:\n> I have created this commit in response to\n> https://github.com/gitgitgadget/git/issues/379#issue-503866117.\n\nFantastic! Thank you for working on this.\n\n> All the functions in object.c which relied on 'the_repository' have\n> been modified.  The functions calling these functions in object.c\n> consisted calls to other functions using 'the_repository' as well, and\n> although I intended to use 'r' at all those instances, I thought it\n> would make more sense when we would deal with their callee functions\n> in another similar patch. What do you think ?\n\nThat makes sense, and follows the conventions that other similar\nrefactorings have done in the past.\n\nAny reason why you decided to start with 'object.c'? Not that any such\nreason is necessary, I'm just curious about how you came to this\ndecision.\n\n> Signed-off-by: Parth Gala parthpgala@gmail.com [parthpgala@gmail.com]\n\nEven though you *do* need a 'Signed-off-by' in each of your patches,\nadding it in the cover letter is not necessary.\n\n> Parth Gala (5):\n>   object.c: get_max_object_index and get_indexed_object accept 'r'\n>     parameter\n>   object.c: lookup_unknown_object() accept 'r' as parameter\n>   object.c: parse_object_or_die() accept 'r' as parameter\n>   object.c: clear_object_flags() accept 'r' as parameter\n>   object.c: clear_commit_marks_all() accept 'r' as parameter\n>\n>  builtin/checkout.c               |  3 ++-\n>  builtin/fsck.c                   |  8 +++++---\n>  builtin/grep.c                   |  6 ++++--\n>  builtin/index-pack.c             |  5 +++--\n>  builtin/log.c                    |  3 ++-\n>  builtin/name-rev.c               |  5 +++--\n>  builtin/pack-objects.c           |  3 ++-\n>  builtin/prune.c                  |  3 ++-\n>  bundle.c                         |  8 +++++---\n>  http-push.c                      |  3 ++-\n>  object.c                         | 32 ++++++++++++++++----------------\n>  object.h                         | 12 ++++++------\n>  pack-bitmap.c                    |  5 +++--\n>  reachable.c                      |  6 ++++--\n>  refs.c                           |  3 ++-\n>  revision.c                       |  3 ++-\n>  shallow.c                        | 13 ++++++++-----\n>  t/helper/test-example-decorate.c |  7 ++++---\n>  upload-pack.c                    | 19 ++++++++++++-------\n>  walker.c                         |  3 ++-\n>  20 files changed, 89 insertions(+), 61 deletions(-)\n>\n>\n> base-commit: 0ad714499976290d9a0229230cbe4efae930b8dc\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-545%2FParthGala2k%2Flocalize-the_repository-variable-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-545/ParthGala2k/localize-the_repository-variable-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/545\n> --\n> gitgitgadget\n\nThanks,\nTaylor\n"},{"id":"391617","messageId":"20200212202210.GC4364@syl.local","threadId":"52789","inReplyTo":"d98b7caff56cc839f251ac43c36826ecf800cf2f.1581535151.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/5] object.c: get_max_object_index and get_indexed_object accept 'r' parameter","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-02-12T20:22:10Z","receivedAt":"2020-02-12T20:22:15Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Feb 12, 2020 at 07:19:07PM +0000, Parth Gala via GitGitGadget wrote:\n> From: Parth Gala <parthpgala@gmail.com>\n>\n> Currently the two functions use global variable 'the_repository' to access\n> the values stored in it. This makes 'the_repository' to be existent even\n> when not required.\n\nNit: please wrap your commit messages at 72 characters instead of 80.\n\n> This commit replaces it with 'r' which is passed as a parameter to those\n> functions\n\nMakes sense.\n\n> Signed-off-by: Parth Gala <parthpgala@gmail.com>\n> ---\n>  builtin/fsck.c       |  5 +++--\n>  builtin/index-pack.c |  5 +++--\n>  builtin/name-rev.c   |  5 +++--\n>  object.c             |  8 ++++----\n>  object.h             |  4 ++--\n>  shallow.c            | 10 ++++++----\n>  upload-pack.c        | 10 ++++++----\n>  7 files changed, 27 insertions(+), 20 deletions(-)\n>\n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index 8d13794b14..d2b4336f7e 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -375,6 +375,7 @@ static void check_object(struct object *obj)\n>  static void check_connectivity(void)\n>  {\n>  \tint i, max;\n> +\tstruct repository *r = the_repository;\n\nIs there a reason that you assign use/assign 'r' here? I can find a few\nother such instances of it in:\n\n  $ git grep -l 'struct repository \\*r = '\n  builtin/merge-tree.c\n  builtin/sparse-checkout.c\n  builtin/update-index.c\n  fetch-pack.c\n  read-cache.c\n  t/helper/test-reach.c\n  tree.c\n\nbut I'm not sure that it's necessary here. Could you instead pass\n'the_repository' directly to the functions that now require it?\n\n>  \t/* Traverse the pending reachable objects */\n>  \ttraverse_reachable();\n> @@ -400,12 +401,12 @@ static void check_connectivity(void)\n>  \t}\n>\n>  \t/* Look up all the requirements, warn about missing objects.. */\n> -\tmax = get_max_object_index();\n> +\tmax = get_max_object_index(r);\n\nFor example, changing this line to:\n\n  max = get_max_object_index(the_repository);\n\n>  \tif (verbose)\n>  \t\tfprintf_ln(stderr, _(\"Checking connectivity (%d objects)\"), max);\n>\n>  \tfor (i = 0; i < max; i++) {\n> -\t\tstruct object *obj = get_indexed_object(i);\n> +\t\tstruct object *obj = get_indexed_object(r, i);\n>\n>  \t\tif (obj)\n>  \t\t\tcheck_object(obj);\n> diff --git a/builtin/index-pack.c b/builtin/index-pack.c\n> index 60a5591039..d2115535bc 100644\n> --- a/builtin/index-pack.c\n> +++ b/builtin/index-pack.c\n> @@ -218,14 +218,15 @@ static unsigned check_object(struct object *obj)\n>  static unsigned check_objects(void)\n>  {\n>  \tunsigned i, max, foreign_nr = 0;\n> +\tstruct repository *r = the_repository;\n>\n> -\tmax = get_max_object_index();\n> +\tmax = get_max_object_index(r);\n>\n>  \tif (verbose)\n>  \t\tprogress = start_delayed_progress(_(\"Checking objects\"), max);\n>\n>  \tfor (i = 0; i < max; i++) {\n> -\t\tforeign_nr += check_object(get_indexed_object(i));\n> +\t\tforeign_nr += check_object(get_indexed_object(r, i));\n>  \t\tdisplay_progress(progress, i + 1);\n>  \t}\n>\n> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n> index 6b9e8c850b..afe9f6df01 100644\n> --- a/builtin/name-rev.c\n> +++ b/builtin/name-rev.c\n> @@ -456,6 +456,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)\n>  int cmd_name_rev(int argc, const char **argv, const char *prefix)\n>  {\n>  \tstruct object_array revs = OBJECT_ARRAY_INIT;\n> +\tstruct repository *r = the_repository;\n>  \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n>  \tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n>  \tstruct option opts[] = {\n> @@ -553,9 +554,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n>  \t} else if (all) {\n>  \t\tint i, max;\n>\n> -\t\tmax = get_max_object_index();\n> +\t\tmax = get_max_object_index(r);\n>  \t\tfor (i = 0; i < max; i++) {\n> -\t\t\tstruct object *obj = get_indexed_object(i);\n> +\t\t\tstruct object *obj = get_indexed_object(r, i);\n>  \t\t\tif (!obj || obj->type != OBJ_COMMIT)\n>  \t\t\t\tcontinue;\n>  \t\t\tshow_name(obj, NULL,\n> diff --git a/object.c b/object.c\n> index 142ef69399..549fbe69ca 100644\n> --- a/object.c\n> +++ b/object.c\n> @@ -10,14 +10,14 @@\n>  #include \"packfile.h\"\n>  #include \"commit-graph.h\"\n>\n> -unsigned int get_max_object_index(void)\n> +unsigned int get_max_object_index(struct repository *r)\n>  {\n> -\treturn the_repository->parsed_objects->obj_hash_size;\n> +\treturn r->parsed_objects->obj_hash_size;\n>  }\n>\n> -struct object *get_indexed_object(unsigned int idx)\n> +struct object *get_indexed_object(struct repository *r, unsigned int idx)\n>  {\n> -\treturn the_repository->parsed_objects->obj_hash[idx];\n> +\treturn r->parsed_objects->obj_hash[idx];\n>  }\n>\n>  static const char *object_type_strings[] = {\n> diff --git a/object.h b/object.h\n> index 25f5ab3d54..5a8ae274ee 100644\n> --- a/object.h\n> +++ b/object.h\n> @@ -98,12 +98,12 @@ int type_from_string_gently(const char *str, ssize_t, int gentle);\n>  /*\n>   * Return the current number of buckets in the object hashmap.\n>   */\n> -unsigned int get_max_object_index(void);\n> +unsigned int get_max_object_index(struct repository *);\n>\n>  /*\n>   * Return the object from the specified bucket in the object hashmap.\n>   */\n> -struct object *get_indexed_object(unsigned int);\n> +struct object *get_indexed_object(struct repository *, unsigned int);\n>\n>  /*\n>   * This can be used to see if we have heard of the object before, but\n> diff --git a/shallow.c b/shallow.c\n> index 7fd04afed1..4537d98482 100644\n> --- a/shallow.c\n> +++ b/shallow.c\n> @@ -510,6 +510,7 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,\n>  \t\t       unsigned int id)\n>  {\n>  \tunsigned int i, nr;\n> +\tstruct repository *r = the_repository;\n>  \tstruct commit_list *head = NULL;\n>  \tint bitmap_nr = DIV_ROUND_UP(info->nr_bits, 32);\n>  \tsize_t bitmap_size = st_mult(sizeof(uint32_t), bitmap_nr);\n> @@ -563,9 +564,9 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,\n>  \t\t}\n>  \t}\n>\n> -\tnr = get_max_object_index();\n> +\tnr = get_max_object_index(r);\n>  \tfor (i = 0; i < nr; i++) {\n> -\t\tstruct object *o = get_indexed_object(i);\n> +\t\tstruct object *o = get_indexed_object(r, i);\n>  \t\tif (o && o->type == OBJ_COMMIT)\n>  \t\t\to->flags &= ~SEEN;\n>  \t}\n> @@ -608,6 +609,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,\n>  \tstruct object_id *oid = info->shallow->oid;\n>  \tstruct oid_array *ref = info->ref;\n>  \tunsigned int i, nr;\n> +\tstruct repository *r = the_repository;\n>  \tint *shallow, nr_shallow = 0;\n>  \tstruct paint_info pi;\n>\n> @@ -622,9 +624,9 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,\n>  \t * Prepare the commit graph to track what refs can reach what\n>  \t * (new) shallow commits.\n>  \t */\n> -\tnr = get_max_object_index();\n> +\tnr = get_max_object_index(r);\n>  \tfor (i = 0; i < nr; i++) {\n> -\t\tstruct object *o = get_indexed_object(i);\n> +\t\tstruct object *o = get_indexed_object(r, i);\n>  \t\tif (!o || o->type != OBJ_COMMIT)\n>  \t\t\tcontinue;\n>\n> diff --git a/upload-pack.c b/upload-pack.c\n> index a00d7ece6b..cb7312268f 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -450,6 +450,7 @@ static int do_reachable_revlist(struct child_process *cmd,\n>  \t\t\"rev-list\", \"--stdin\", NULL,\n>  \t};\n>  \tstruct object *o;\n> +\tstruct repository *r = the_repository;\n>  \tchar namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */\n>  \tint i;\n>  \tconst unsigned hexsz = the_hash_algo->hexsz;\n> @@ -472,8 +473,8 @@ static int do_reachable_revlist(struct child_process *cmd,\n>\n>  \tnamebuf[0] = '^';\n>  \tnamebuf[hexsz + 1] = '\\n';\n> -\tfor (i = get_max_object_index(); 0 < i; ) {\n> -\t\to = get_indexed_object(--i);\n> +\tfor (i = get_max_object_index(r); 0 < i; ) {\n> +\t\to = get_indexed_object(r, --i);\n>  \t\tif (!o)\n>  \t\t\tcontinue;\n>  \t\tif (reachable && o->type == OBJ_COMMIT)\n> @@ -520,6 +521,7 @@ static int get_reachable_list(struct object_array *src,\n>  \tstruct child_process cmd = CHILD_PROCESS_INIT;\n>  \tint i;\n>  \tstruct object *o;\n> +\tstruct repository *r = the_repository;\n>  \tchar namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */\n>  \tconst unsigned hexsz = the_hash_algo->hexsz;\n>\n> @@ -538,8 +540,8 @@ static int get_reachable_list(struct object_array *src,\n>  \t\t\to->flags &= ~TMP_MARK;\n>  \t\t}\n>  \t}\n> -\tfor (i = get_max_object_index(); 0 < i; i--) {\n> -\t\to = get_indexed_object(i - 1);\n> +\tfor (i = get_max_object_index(r); 0 < i; i--) {\n> +\t\to = get_indexed_object(r, i - 1);\n>  \t\tif (o && o->type == OBJ_COMMIT &&\n>  \t\t    (o->flags & TMP_MARK)) {\n>  \t\t\tadd_object_array(o, NULL, reachable);\n> --\n> gitgitgadget\n\nOtherwise this looks pretty good.\n\nThanks,\nTaylor\n"},{"id":"391618","messageId":"20200212202540.GD4364@syl.local","threadId":"52789","inReplyTo":"16dd64618ab6a086727685e9eca3850fabc46437.1581535151.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-02-12T20:25:40Z","receivedAt":"2020-02-12T20:25:44Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Feb 12, 2020 at 07:19:08PM +0000, Parth Gala via GitGitGadget wrote:\n> From: Parth Gala <parthpgala@gmail.com>\n>\n> 'lookup_unknown_object()' and its callers are modified to enable\n> passing 'r' as an argument to 'lookup_unknown_object()' in an\n> effort to reduce dependence on global 'the_repository' variable.\n\nThe changes in 'object.[ch]' look sane to me here, again, but I have the\nsame question about why assigning:\n\n  struct repository *r = the_repository;\n\nand passing 'r' everywhere is preferable to simply passing\n'the_repository' in directly.\n\n> Signed-off-by: Parth Gala <parthpgala@gmail.com>\n> ---\n>  builtin/fsck.c                   | 3 ++-\n>  builtin/pack-objects.c           | 3 ++-\n>  http-push.c                      | 3 ++-\n>  object.c                         | 8 ++++----\n>  object.h                         | 2 +-\n>  refs.c                           | 3 ++-\n>  t/helper/test-example-decorate.c | 7 ++++---\n>  upload-pack.c                    | 3 ++-\n>  walker.c                         | 3 ++-\n>  9 files changed, 21 insertions(+), 14 deletions(-)\n>\n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index d2b4336f7e..cd0b67f3bc 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -745,7 +745,8 @@ static int fsck_cache_tree(struct cache_tree *it)\n>\n>  static void mark_object_for_connectivity(const struct object_id *oid)\n>  {\n> -\tstruct object *obj = lookup_unknown_object(oid);\n> +\tstruct repository *r = the_repository;\n> +\tstruct object *obj = lookup_unknown_object(r, oid);\n>  \tobj->flags |= HAS_OBJ;\n>  }\n>\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 393c20a2d7..b03f4378a0 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -2891,6 +2891,7 @@ static void add_objects_in_unpacked_packs(void)\n>  {\n>  \tstruct packed_git *p;\n>  \tstruct in_pack in_pack;\n> +\tstruct repository *r = the_repository;\n>  \tuint32_t i;\n>\n>  \tmemset(&in_pack, 0, sizeof(in_pack));\n> @@ -2910,7 +2911,7 @@ static void add_objects_in_unpacked_packs(void)\n>\n>  \t\tfor (i = 0; i < p->num_objects; i++) {\n>  \t\t\tnth_packed_object_oid(&oid, p, i);\n> -\t\t\to = lookup_unknown_object(&oid);\n> +\t\t\to = lookup_unknown_object(r, &oid);\n>  \t\t\tif (!(o->flags & OBJECT_ADDED))\n>  \t\t\t\tmark_in_pack_object(o, p, &in_pack);\n>  \t\t\to->flags |= OBJECT_ADDED;\n> diff --git a/http-push.c b/http-push.c\n> index 822f326599..c26d03b21b 100644\n> --- a/http-push.c\n> +++ b/http-push.c\n> @@ -1416,6 +1416,7 @@ static void one_remote_ref(const char *refname)\n>  {\n>  \tstruct ref *ref;\n>  \tstruct object *obj;\n> +\tstruct repository *r = the_repository;\n>\n>  \tref = alloc_ref(refname);\n>\n> @@ -1432,7 +1433,7 @@ static void one_remote_ref(const char *refname)\n>  \t * may be required for updating server info later.\n>  \t */\n>  \tif (repo->can_update_info_refs && !has_object_file(&ref->old_oid)) {\n> -\t\tobj = lookup_unknown_object(&ref->old_oid);\n> +\t\tobj = lookup_unknown_object(r, &ref->old_oid);\n>  \t\tfprintf(stderr,\t\"  fetch %s for %s\\n\",\n>  \t\t\toid_to_hex(&ref->old_oid), refname);\n>  \t\tadd_fetch_request(obj);\n> diff --git a/object.c b/object.c\n> index 549fbe69ca..90338a509c 100644\n> --- a/object.c\n> +++ b/object.c\n> @@ -177,12 +177,12 @@ void *object_as_type(struct repository *r, struct object *obj, enum object_type\n>  \t}\n>  }\n>\n> -struct object *lookup_unknown_object(const struct object_id *oid)\n> +struct object *lookup_unknown_object(struct repository *r, const struct object_id *oid)\n>  {\n> -\tstruct object *obj = lookup_object(the_repository, oid);\n> +\tstruct object *obj = lookup_object(r, oid);\n>  \tif (!obj)\n> -\t\tobj = create_object(the_repository, oid,\n> -\t\t\t\t    alloc_object_node(the_repository));\n> +\t\tobj = create_object(r, oid,\n> +\t\t\t\t    alloc_object_node(r));\n>  \treturn obj;\n>  }\n>\n> diff --git a/object.h b/object.h\n> index 5a8ae274ee..375236cec3 100644\n> --- a/object.h\n> +++ b/object.h\n> @@ -144,7 +144,7 @@ struct object *parse_object_or_die(const struct object_id *oid, const char *name\n>  struct object *parse_object_buffer(struct repository *r, const struct object_id *oid, enum object_type type, unsigned long size, void *buffer, int *eaten_p);\n>\n>  /** Returns the object, with potentially excess memory allocated. **/\n> -struct object *lookup_unknown_object(const struct object_id *oid);\n> +struct object *lookup_unknown_object(struct repository *, const struct object_id *oid);\n>\n>  struct object_list *object_list_insert(struct object *item,\n>  \t\t\t\t       struct object_list **list_p);\n> diff --git a/refs.c b/refs.c\n> index 1ab0bb54d3..a630a8c271 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -379,7 +379,8 @@ static int filter_refs(const char *refname, const struct object_id *oid,\n>\n>  enum peel_status peel_object(const struct object_id *name, struct object_id *oid)\n>  {\n> -\tstruct object *o = lookup_unknown_object(name);\n> +\tstruct repository *r = the_repository;\n> +\tstruct object *o = lookup_unknown_object(r, name);\n>\n>  \tif (o->type == OBJ_NONE) {\n>  \t\tint type = oid_object_info(the_repository, name, NULL);\n> diff --git a/t/helper/test-example-decorate.c b/t/helper/test-example-decorate.c\n> index c8a1cde7d2..6b3262a9d3 100644\n> --- a/t/helper/test-example-decorate.c\n> +++ b/t/helper/test-example-decorate.c\n> @@ -10,6 +10,7 @@ int cmd__example_decorate(int argc, const char **argv)\n>  \tstruct object_id two_oid = { {2} };\n>  \tstruct object_id three_oid = { {3} };\n>  \tstruct object *one, *two, *three;\n> +\tstruct repository *r = the_repository;\n>\n>  \tint decoration_a, decoration_b;\n>\n> @@ -26,8 +27,8 @@ int cmd__example_decorate(int argc, const char **argv)\n>  \t * Add 2 objects, one with a non-NULL decoration and one with a NULL\n>  \t * decoration.\n>  \t */\n> -\tone = lookup_unknown_object(&one_oid);\n> -\ttwo = lookup_unknown_object(&two_oid);\n> +\tone = lookup_unknown_object(r, &one_oid);\n> +\ttwo = lookup_unknown_object(r, &two_oid);\n>  \tret = add_decoration(&n, one, &decoration_a);\n>  \tif (ret)\n>  \t\tBUG(\"when adding a brand-new object, NULL should be returned\");\n> @@ -56,7 +57,7 @@ int cmd__example_decorate(int argc, const char **argv)\n>  \tret = lookup_decoration(&n, two);\n>  \tif (ret != &decoration_b)\n>  \t\tBUG(\"lookup should return added declaration\");\n> -\tthree = lookup_unknown_object(&three_oid);\n> +\tthree = lookup_unknown_object(r, &three_oid);\n>  \tret = lookup_decoration(&n, three);\n>  \tif (ret)\n>  \t\tBUG(\"lookup for unknown object should return NULL\");\n> diff --git a/upload-pack.c b/upload-pack.c\n> index cb7312268f..6d196e275b 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -962,7 +962,8 @@ static void receive_needs(struct packet_reader *reader, struct object_array *wan\n>  static int mark_our_ref(const char *refname, const char *refname_full,\n>  \t\t\tconst struct object_id *oid)\n>  {\n> -\tstruct object *o = lookup_unknown_object(oid);\n> +\tstruct repository *r = the_repository;\n> +\tstruct object *o = lookup_unknown_object(r, oid);\n>\n>  \tif (ref_is_hidden(refname, refname_full)) {\n>  \t\to->flags |= HIDDEN_REF;\n> diff --git a/walker.c b/walker.c\n> index 06cd2bd569..098c69ebe1 100644\n> --- a/walker.c\n> +++ b/walker.c\n> @@ -258,6 +258,7 @@ void walker_targets_free(int targets, char **target, const char **write_ref)\n>  int walker_fetch(struct walker *walker, int targets, char **target,\n>  \t\t const char **write_ref, const char *write_ref_log_details)\n>  {\n> +\tstruct repository *r = the_repository;\n>  \tstruct strbuf refname = STRBUF_INIT;\n>  \tstruct strbuf err = STRBUF_INIT;\n>  \tstruct ref_transaction *transaction = NULL;\n> @@ -285,7 +286,7 @@ int walker_fetch(struct walker *walker, int targets, char **target,\n>  \t\t\terror(\"Could not interpret response from server '%s' as something to pull\", target[i]);\n>  \t\t\tgoto done;\n>  \t\t}\n> -\t\tif (process(walker, lookup_unknown_object(&oids[i])))\n> +\t\tif (process(walker, lookup_unknown_object(r, &oids[i])))\n>  \t\t\tgoto done;\n>  \t}\n>\n> --\n> gitgitgadget\n>\n\nThanks,\nTaylor\n"},{"id":"391621","messageId":"xmqqtv3vtst5.fsf@gitster-ct.c.googlers.com","threadId":"52789","inReplyTo":"20200212202540.GD4364@syl.local","subject":"Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-12T21:11:02Z","receivedAt":"2020-02-12T21:11:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> ... same question about why assigning:\n>\n>   struct repository *r = the_repository;\n>\n> and passing 'r' everywhere is preferable to simply passing\n> 'the_repository' in directly.\n> ...\n>>  static void mark_object_for_connectivity(const struct object_id *oid)\n>>  {\n>> -\tstruct object *obj = lookup_unknown_object(oid);\n>> +\tstruct repository *r = the_repository;\n>> +\tstruct object *obj = lookup_unknown_object(r, oid);\n>>  \tobj->flags |= HAS_OBJ;\n>>  }\n\nI do not claim that it applies to this particular function, and the\nfunction is too small for it to matter, but when a function is large\nenough and it always works on one single repository, it would make\nit easier to later update the function further to set up 'r'\nupfront, making it point at the_repository for now, and to use 'r'\nthroughout the function.  That way, when the time comes to update\nthe function to work on an arbitrary repository, the only change\nrequired will be to turn the local variable 'r' to an incoming\nparameter the caller can supply.\n"},{"id":"391622","messageId":"CAPig+cQhh8pNppSsGGk62-VoHsOUv0aNTxHtDWY+_pousR+KMg@mail.gmail.com","threadId":"52789","inReplyTo":"20200212202210.GC4364@syl.local","subject":"Re: [PATCH 1/5] object.c: get_max_object_index and get_indexed_object accept 'r' parameter","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-02-12T21:13:42Z","receivedAt":"2020-02-12T21:13:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Feb 12, 2020 at 3:22 PM Taylor Blau <me@ttaylorr.com> wrote:\n> On Wed, Feb 12, 2020 at 07:19:07PM +0000, Parth Gala via GitGitGadget wrote:\n> > diff --git a/builtin/fsck.c b/builtin/fsck.c\n> > @@ -375,6 +375,7 @@ static void check_object(struct object *obj)\n> >  static void check_connectivity(void)\n> >  {\n> >       int i, max;\n> > +     struct repository *r = the_repository;\n>\n> Is there a reason that you assign use/assign 'r' here? [...]\n> but I'm not sure that it's necessary here. Could you instead pass\n> 'the_repository' directly to the functions that now require it?\n\nOne benefit of doing it this way is that future patches will be much\nless noisy which convert these callers to also work with any\n'repository' object. For instance, after the current patch, we have:\n\n    static void check_connectivity(void)\n    {\n        struct repository *r = the_repository;\n        max = get_max_object_index(r);\n        ...\n\nA future patch which converts check_connectivity() to work with any\nrepository will then require very little change -- just adding an 'r'\nargument to the function declaration, and dropping the line which\ndeclares 'r':\n\n    static void check_connectivity(struct repository *r)\n    {\n        max = get_max_object_index(r);\n        ...\n\nSo, I'm fine with this patch series' approach of declaring a variable\n'r' like this.\n"},{"id":"391650","messageId":"f1e4da02-9411-8a93-ca62-6d7ae7bf4ae8@gmail.com","threadId":"52789","inReplyTo":"20200212201810.GB4364@syl.local","subject":"Re: [PATCH 0/5] object.c: localize global the_repository variable into r","fromName":"parth gala","fromEmail":"parthpgala@gmail.com","sentAt":"2020-02-13T05:14:49Z","receivedAt":"2020-02-13T05:14:54Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"\nOn 13/02/20 1:48 am, Taylor Blau wrote:\n> Hi Parth,\n>\n> On Wed, Feb 12, 2020 at 07:19:06PM +0000, Parth Gala via GitGitGadget wrote:\n>> I have created this commit in response to\n>> https://github.com/gitgitgadget/git/issues/379#issue-503866117.\n> Fantastic! Thank you for working on this.\n>\n>> All the functions in object.c which relied on 'the_repository' have\n>> been modified.  The functions calling these functions in object.c\n>> consisted calls to other functions using 'the_repository' as well, and\n>> although I intended to use 'r' at all those instances, I thought it\n>> would make more sense when we would deal with their callee functions\n>> in another similar patch. What do you think ?\n> That makes sense, and follows the conventions that other similar\n> refactorings have done in the past.\n>\n> Any reason why you decided to start with 'object.c'? Not that any such\n> reason is necessary, I'm just curious about how you came to this\n> decision.\n\nI am new to the git community and while searching for issues to solve I\nfound the issue linked above. I figured solving it would give me a good\nexperience navigating between functions and exploring the huge repository.\n\nInitially I grepped to find all functions using `the_repository` but\nrandomly chose object.c since the refactoring had to start somewhere.\nSpecial thanks to Johannes Schindelin for this.\n\n>> Signed-off-by: Parth Gala parthpgala@gmail.com [parthpgala@gmail.com]\n> Even though you *do* need a 'Signed-off-by' in each of your patches,\n> adding it in the cover letter is not necessary.\n\nI probably did not check the preview I sent to myself carefully for\nthis. Will make note of it.\n\n>> Parth Gala (5):\n>>    object.c: get_max_object_index and get_indexed_object accept 'r'\n>>      parameter\n>>    object.c: lookup_unknown_object() accept 'r' as parameter\n>>    object.c: parse_object_or_die() accept 'r' as parameter\n>>    object.c: clear_object_flags() accept 'r' as parameter\n>>    object.c: clear_commit_marks_all() accept 'r' as parameter\n>>\n>>   builtin/checkout.c               |  3 ++-\n>>   builtin/fsck.c                   |  8 +++++---\n>>   builtin/grep.c                   |  6 ++++--\n>>   builtin/index-pack.c             |  5 +++--\n>>   builtin/log.c                    |  3 ++-\n>>   builtin/name-rev.c               |  5 +++--\n>>   builtin/pack-objects.c           |  3 ++-\n>>   builtin/prune.c                  |  3 ++-\n>>   bundle.c                         |  8 +++++---\n>>   http-push.c                      |  3 ++-\n>>   object.c                         | 32 ++++++++++++++++----------------\n>>   object.h                         | 12 ++++++------\n>>   pack-bitmap.c                    |  5 +++--\n>>   reachable.c                      |  6 ++++--\n>>   refs.c                           |  3 ++-\n>>   revision.c                       |  3 ++-\n>>   shallow.c                        | 13 ++++++++-----\n>>   t/helper/test-example-decorate.c |  7 ++++---\n>>   upload-pack.c                    | 19 ++++++++++++-------\n>>   walker.c                         |  3 ++-\n>>   20 files changed, 89 insertions(+), 61 deletions(-)\n>>\n>>\n>> base-commit: 0ad714499976290d9a0229230cbe4efae930b8dc\n>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-545%2FParthGala2k%2Flocalize-the_repository-variable-v1\n>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-545/ParthGala2k/localize-the_repository-variable-v1\n>> Pull-Request: https://github.com/gitgitgadget/git/pull/545\n>> --\n>> gitgitgadget\n> Thanks,\n> Taylor\n"},{"id":"391651","messageId":"f153f247-fd5a-7a1f-9cd6-fa65e59e6f00@gmail.com","threadId":"52789","inReplyTo":"20200212202210.GC4364@syl.local","subject":"Re: [PATCH 1/5] object.c: get_max_object_index and get_indexed_object accept 'r' parameter","fromName":"parth gala","fromEmail":"parthpgala@gmail.com","sentAt":"2020-02-13T05:23:19Z","receivedAt":"2020-02-13T05:23:25Z","isPatch":true,"sender":{"key":"parthpgala@gmail.com","avatar":null},"body":"\nOn 13/02/20 1:52 am, Taylor Blau wrote:\n> On Wed, Feb 12, 2020 at 07:19:07PM +0000, Parth Gala via GitGitGadget wrote:\n>> From: Parth Gala <parthpgala@gmail.com>\n>>\n>> Currently the two functions use global variable 'the_repository' to access\n>> the values stored in it. This makes 'the_repository' to be existent even\n>> when not required.\n> Nit: please wrap your commit messages at 72 characters instead of 80.\n\nAlright.\n\n>> This commit replaces it with 'r' which is passed as a parameter to those\n>> functions\n> Makes sense.\n>\n>> Signed-off-by: Parth Gala <parthpgala@gmail.com>\n>> ---\n>>   builtin/fsck.c       |  5 +++--\n>>   builtin/index-pack.c |  5 +++--\n>>   builtin/name-rev.c   |  5 +++--\n>>   object.c             |  8 ++++----\n>>   object.h             |  4 ++--\n>>   shallow.c            | 10 ++++++----\n>>   upload-pack.c        | 10 ++++++----\n>>   7 files changed, 27 insertions(+), 20 deletions(-)\n>>\n>> diff --git a/builtin/fsck.c b/builtin/fsck.c\n>> index 8d13794b14..d2b4336f7e 100644\n>> --- a/builtin/fsck.c\n>> +++ b/builtin/fsck.c\n>> @@ -375,6 +375,7 @@ static void check_object(struct object *obj)\n>>   static void check_connectivity(void)\n>>   {\n>>   \tint i, max;\n>> +\tstruct repository *r = the_repository;\n> Is there a reason that you assign use/assign 'r' here? I can find a few\n> other such instances of it in:\n>\n>    $ git grep -l 'struct repository \\*r = '\n>    builtin/merge-tree.c\n>    builtin/sparse-checkout.c\n>    builtin/update-index.c\n>    fetch-pack.c\n>    read-cache.c\n>    t/helper/test-reach.c\n>    tree.c\n>\n> but I'm not sure that it's necessary here. Could you instead pass\n> 'the_repository' directly to the functions that now require it?\n\nThis was the more convenient thing to do, given that the ultimate\ngoal is to actually use the `r` that is passed to these caller\nfunctions rather than defining them locally, which would require\nanother layer of refactoring.\n\nBesides it is what was decided in the issue.\n\nAlso, it would be easier to differentiate whether or not a\nfunction has been refactored, upon seeing an `r` rather than\n`the_repository`(which is otherwise commonly used in a global\nscope) ie. you would be confused whether `the_repository` used\nhere was passed to the function or not(especially in longer\nfunctions where you'd have to scroll up to the function\ndefinition to check).\n\n>>   \t/* Traverse the pending reachable objects */\n>>   \ttraverse_reachable();\n>> @@ -400,12 +401,12 @@ static void check_connectivity(void)\n>>   \t}\n>>\n>>   \t/* Look up all the requirements, warn about missing objects.. */\n>> -\tmax = get_max_object_index();\n>> +\tmax = get_max_object_index(r);\n> For example, changing this line to:\n>\n>    max = get_max_object_index(the_repository);\n>\n>>   \tif (verbose)\n>>   \t\tfprintf_ln(stderr, _(\"Checking connectivity (%d objects)\"), max);\n>>\n>>   \tfor (i = 0; i < max; i++) {\n>> -\t\tstruct object *obj = get_indexed_object(i);\n>> +\t\tstruct object *obj = get_indexed_object(r, i);\n>>\n>>   \t\tif (obj)\n>>   \t\t\tcheck_object(obj);\n>> diff --git a/builtin/index-pack.c b/builtin/index-pack.c\n>> index 60a5591039..d2115535bc 100644\n>> --- a/builtin/index-pack.c\n>> +++ b/builtin/index-pack.c\n>> @@ -218,14 +218,15 @@ static unsigned check_object(struct object *obj)\n>>   static unsigned check_objects(void)\n>>   {\n>>   \tunsigned i, max, foreign_nr = 0;\n>> +\tstruct repository *r = the_repository;\n>>\n>> -\tmax = get_max_object_index();\n>> +\tmax = get_max_object_index(r);\n>>\n>>   \tif (verbose)\n>>   \t\tprogress = start_delayed_progress(_(\"Checking objects\"), max);\n>>\n>>   \tfor (i = 0; i < max; i++) {\n>> -\t\tforeign_nr += check_object(get_indexed_object(i));\n>> +\t\tforeign_nr += check_object(get_indexed_object(r, i));\n>>   \t\tdisplay_progress(progress, i + 1);\n>>   \t}\n>>\n>> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n>> index 6b9e8c850b..afe9f6df01 100644\n>> --- a/builtin/name-rev.c\n>> +++ b/builtin/name-rev.c\n>> @@ -456,6 +456,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)\n>>   int cmd_name_rev(int argc, const char **argv, const char *prefix)\n>>   {\n>>   \tstruct object_array revs = OBJECT_ARRAY_INIT;\n>> +\tstruct repository *r = the_repository;\n>>   \tint all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n>>   \tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n>>   \tstruct option opts[] = {\n>> @@ -553,9 +554,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n>>   \t} else if (all) {\n>>   \t\tint i, max;\n>>\n>> -\t\tmax = get_max_object_index();\n>> +\t\tmax = get_max_object_index(r);\n>>   \t\tfor (i = 0; i < max; i++) {\n>> -\t\t\tstruct object *obj = get_indexed_object(i);\n>> +\t\t\tstruct object *obj = get_indexed_object(r, i);\n>>   \t\t\tif (!obj || obj->type != OBJ_COMMIT)\n>>   \t\t\t\tcontinue;\n>>   \t\t\tshow_name(obj, NULL,\n>> diff --git a/object.c b/object.c\n>> index 142ef69399..549fbe69ca 100644\n>> --- a/object.c\n>> +++ b/object.c\n>> @@ -10,14 +10,14 @@\n>>   #include \"packfile.h\"\n>>   #include \"commit-graph.h\"\n>>\n>> -unsigned int get_max_object_index(void)\n>> +unsigned int get_max_object_index(struct repository *r)\n>>   {\n>> -\treturn the_repository->parsed_objects->obj_hash_size;\n>> +\treturn r->parsed_objects->obj_hash_size;\n>>   }\n>>\n>> -struct object *get_indexed_object(unsigned int idx)\n>> +struct object *get_indexed_object(struct repository *r, unsigned int idx)\n>>   {\n>> -\treturn the_repository->parsed_objects->obj_hash[idx];\n>> +\treturn r->parsed_objects->obj_hash[idx];\n>>   }\n>>\n>>   static const char *object_type_strings[] = {\n>> diff --git a/object.h b/object.h\n>> index 25f5ab3d54..5a8ae274ee 100644\n>> --- a/object.h\n>> +++ b/object.h\n>> @@ -98,12 +98,12 @@ int type_from_string_gently(const char *str, ssize_t, int gentle);\n>>   /*\n>>    * Return the current number of buckets in the object hashmap.\n>>    */\n>> -unsigned int get_max_object_index(void);\n>> +unsigned int get_max_object_index(struct repository *);\n>>\n>>   /*\n>>    * Return the object from the specified bucket in the object hashmap.\n>>    */\n>> -struct object *get_indexed_object(unsigned int);\n>> +struct object *get_indexed_object(struct repository *, unsigned int);\n>>\n>>   /*\n>>    * This can be used to see if we have heard of the object before, but\n>> diff --git a/shallow.c b/shallow.c\n>> index 7fd04afed1..4537d98482 100644\n>> --- a/shallow.c\n>> +++ b/shallow.c\n>> @@ -510,6 +510,7 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,\n>>   \t\t       unsigned int id)\n>>   {\n>>   \tunsigned int i, nr;\n>> +\tstruct repository *r = the_repository;\n>>   \tstruct commit_list *head = NULL;\n>>   \tint bitmap_nr = DIV_ROUND_UP(info->nr_bits, 32);\n>>   \tsize_t bitmap_size = st_mult(sizeof(uint32_t), bitmap_nr);\n>> @@ -563,9 +564,9 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,\n>>   \t\t}\n>>   \t}\n>>\n>> -\tnr = get_max_object_index();\n>> +\tnr = get_max_object_index(r);\n>>   \tfor (i = 0; i < nr; i++) {\n>> -\t\tstruct object *o = get_indexed_object(i);\n>> +\t\tstruct object *o = get_indexed_object(r, i);\n>>   \t\tif (o && o->type == OBJ_COMMIT)\n>>   \t\t\to->flags &= ~SEEN;\n>>   \t}\n>> @@ -608,6 +609,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,\n>>   \tstruct object_id *oid = info->shallow->oid;\n>>   \tstruct oid_array *ref = info->ref;\n>>   \tunsigned int i, nr;\n>> +\tstruct repository *r = the_repository;\n>>   \tint *shallow, nr_shallow = 0;\n>>   \tstruct paint_info pi;\n>>\n>> @@ -622,9 +624,9 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,\n>>   \t * Prepare the commit graph to track what refs can reach what\n>>   \t * (new) shallow commits.\n>>   \t */\n>> -\tnr = get_max_object_index();\n>> +\tnr = get_max_object_index(r);\n>>   \tfor (i = 0; i < nr; i++) {\n>> -\t\tstruct object *o = get_indexed_object(i);\n>> +\t\tstruct object *o = get_indexed_object(r, i);\n>>   \t\tif (!o || o->type != OBJ_COMMIT)\n>>   \t\t\tcontinue;\n>>\n>> diff --git a/upload-pack.c b/upload-pack.c\n>> index a00d7ece6b..cb7312268f 100644\n>> --- a/upload-pack.c\n>> +++ b/upload-pack.c\n>> @@ -450,6 +450,7 @@ static int do_reachable_revlist(struct child_process *cmd,\n>>   \t\t\"rev-list\", \"--stdin\", NULL,\n>>   \t};\n>>   \tstruct object *o;\n>> +\tstruct repository *r = the_repository;\n>>   \tchar namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */\n>>   \tint i;\n>>   \tconst unsigned hexsz = the_hash_algo->hexsz;\n>> @@ -472,8 +473,8 @@ static int do_reachable_revlist(struct child_process *cmd,\n>>\n>>   \tnamebuf[0] = '^';\n>>   \tnamebuf[hexsz + 1] = '\\n';\n>> -\tfor (i = get_max_object_index(); 0 < i; ) {\n>> -\t\to = get_indexed_object(--i);\n>> +\tfor (i = get_max_object_index(r); 0 < i; ) {\n>> +\t\to = get_indexed_object(r, --i);\n>>   \t\tif (!o)\n>>   \t\t\tcontinue;\n>>   \t\tif (reachable && o->type == OBJ_COMMIT)\n>> @@ -520,6 +521,7 @@ static int get_reachable_list(struct object_array *src,\n>>   \tstruct child_process cmd = CHILD_PROCESS_INIT;\n>>   \tint i;\n>>   \tstruct object *o;\n>> +\tstruct repository *r = the_repository;\n>>   \tchar namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */\n>>   \tconst unsigned hexsz = the_hash_algo->hexsz;\n>>\n>> @@ -538,8 +540,8 @@ static int get_reachable_list(struct object_array *src,\n>>   \t\t\to->flags &= ~TMP_MARK;\n>>   \t\t}\n>>   \t}\n>> -\tfor (i = get_max_object_index(); 0 < i; i--) {\n>> -\t\to = get_indexed_object(i - 1);\n>> +\tfor (i = get_max_object_index(r); 0 < i; i--) {\n>> +\t\to = get_indexed_object(r, i - 1);\n>>   \t\tif (o && o->type == OBJ_COMMIT &&\n>>   \t\t    (o->flags & TMP_MARK)) {\n>>   \t\t\tadd_object_array(o, NULL, reachable);\n>> --\n>> gitgitgadget\n> Otherwise this looks pretty good.\n>\n> Thanks,\n> Taylor\n"},{"id":"391678","messageId":"20200213180030.GF45518@syl.local","threadId":"52789","inReplyTo":"xmqqtv3vtst5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-02-13T18:00:30Z","receivedAt":"2020-02-13T18:00:35Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Feb 12, 2020 at 01:11:02PM -0800, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n>\n> > ... same question about why assigning:\n> >\n> >   struct repository *r = the_repository;\n> >\n> > and passing 'r' everywhere is preferable to simply passing\n> > 'the_repository' in directly.\n> > ...\n> >>  static void mark_object_for_connectivity(const struct object_id *oid)\n> >>  {\n> >> -\tstruct object *obj = lookup_unknown_object(oid);\n> >> +\tstruct repository *r = the_repository;\n> >> +\tstruct object *obj = lookup_unknown_object(r, oid);\n> >>  \tobj->flags |= HAS_OBJ;\n> >>  }\n>\n> I do not claim that it applies to this particular function, and the\n> function is too small for it to matter, but when a function is large\n> enough and it always works on one single repository, it would make\n> it easier to later update the function further to set up 'r'\n> upfront, making it point at the_repository for now, and to use 'r'\n> throughout the function.  That way, when the time comes to update\n> the function to work on an arbitrary repository, the only change\n> required will be to turn the local variable 'r' to an incoming\n> parameter the caller can supply.\n\nRight, but my suggestion was that this advice doesn't apply to this\nparticular instance since I don't expect that we'd ever passing\nsomething other than 'the_repository'.\n\nSpecifically, I was worried that we'd get bitten by re-assigning 'r' in\nthe middle of the function and then end up in some odd broken state.\n\nMaybe I'm worrying too much.\n\n\nThanks,\nTaylor\n"},{"id":"391679","messageId":"xmqqh7zus6hm.fsf@gitster-ct.c.googlers.com","threadId":"52789","inReplyTo":"20200213180030.GF45518@syl.local","subject":"Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-02-13T18:10:45Z","receivedAt":"2020-02-13T18:10:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> On Wed, Feb 12, 2020 at 01:11:02PM -0800, Junio C Hamano wrote:\n>> Taylor Blau <me@ttaylorr.com> writes:\n>>\n>> > ... same question about why assigning:\n>> >\n>> >   struct repository *r = the_repository;\n>> >\n>> > and passing 'r' everywhere is preferable to simply passing\n>> > 'the_repository' in directly.\n>> > ...\n>> >>  static void mark_object_for_connectivity(const struct object_id *oid)\n>> >>  {\n>> >> -\tstruct object *obj = lookup_unknown_object(oid);\n>> >> +\tstruct repository *r = the_repository;\n>> >> +\tstruct object *obj = lookup_unknown_object(r, oid);\n>> >>  \tobj->flags |= HAS_OBJ;\n>> >>  }\n>> ...\n> Right, but my suggestion was that this advice doesn't apply to this\n> particular instance since I don't expect that we'd ever passing\n> something other than 'the_repository'.\n>\n> Specifically, I was worried that we'd get bitten by re-assigning 'r' in\n> the middle of the function and then end up in some odd broken state.\n\n\"git fsck\" works only in a single, \"the\", repository, so I guess you\nare right to be worried about unnecessary complexity here.\n"},{"id":"391687","messageId":"20200213185235.GA15252@coredump.intra.peff.net","threadId":"52789","inReplyTo":"xmqqh7zus6hm.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-13T18:52:35Z","receivedAt":"2020-02-13T18:52:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 13, 2020 at 10:10:45AM -0800, Junio C Hamano wrote:\n\n> > Right, but my suggestion was that this advice doesn't apply to this\n> > particular instance since I don't expect that we'd ever passing\n> > something other than 'the_repository'.\n> >\n> > Specifically, I was worried that we'd get bitten by re-assigning 'r' in\n> > the middle of the function and then end up in some odd broken state.\n> \n> \"git fsck\" works only in a single, \"the\", repository, so I guess you\n> are right to be worried about unnecessary complexity here.\n\nI think the end-game for this whole repository transition would be to\nget rid of the_repository, though. I.e., I'd envision the progression\nsomething like this:\n\n  1. Teach all of the library code to take (and operate on) \"struct\n     repository\".\n\n  2. Teach static local functions like this to pass in the_repository.\n\n  3. Teach top-level commands like cmd_fsck() to pass the_repository to\n     all of those static local helpers.\n\n  4. Teach top-level commands to get a real repository pointer, either\n     from the git.c wrapper (when RUN_SETUP is used) or by calling\n     setup_git_repository() themselves.\n\n  5. Grep for the_repository and drop it everywhere.\n\nHere we're at step 2 now, but declaring \"r\" makes moving to step 3 just\na little easier. And I think the existence of steps 4 and 5 implies that\nit would eventually be worth going through step 3.\n\nOf course I just wrote those steps down for the first time, so maybe\nnobody else shares my vision. ;)\n\n-Peff\n"},{"id":"391810","messageId":"20200215000022.GB11400@syl.local","threadId":"52789","inReplyTo":"20200213185235.GA15252@coredump.intra.peff.net","subject":"Re: [PATCH 2/5] object.c: lookup_unknown_object() accept 'r' as parameter","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-02-15T00:00:22Z","receivedAt":"2020-02-15T00:00:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Feb 13, 2020 at 01:52:35PM -0500, Jeff King wrote:\n> On Thu, Feb 13, 2020 at 10:10:45AM -0800, Junio C Hamano wrote:\n>\n> > > Right, but my suggestion was that this advice doesn't apply to this\n> > > particular instance since I don't expect that we'd ever passing\n> > > something other than 'the_repository'.\n> > >\n> > > Specifically, I was worried that we'd get bitten by re-assigning 'r' in\n> > > the middle of the function and then end up in some odd broken state.\n> >\n> > \"git fsck\" works only in a single, \"the\", repository, so I guess you\n> > are right to be worried about unnecessary complexity here.\n>\n> I think the end-game for this whole repository transition would be to\n> get rid of the_repository, though. I.e., I'd envision the progression\n> something like this:\n>\n>   1. Teach all of the library code to take (and operate on) \"struct\n>      repository\".\n>\n>   2. Teach static local functions like this to pass in the_repository.\n>\n>   3. Teach top-level commands like cmd_fsck() to pass the_repository to\n>      all of those static local helpers.\n>\n>   4. Teach top-level commands to get a real repository pointer, either\n>      from the git.c wrapper (when RUN_SETUP is used) or by calling\n>      setup_git_repository() themselves.\n>\n>   5. Grep for the_repository and drop it everywhere.\n>\n> Here we're at step 2 now, but declaring \"r\" makes moving to step 3 just\n> a little easier. And I think the existence of steps 4 and 5 implies that\n> it would eventually be worth going through step 3.\n\nAh, the transition to step 3 justifies this, I think. I wasn't aware\nthat steps 3+ existed. If they didn't, I'd stand by my original advice,\nbut given that they do, the approach here makes more sense long-term.\n\n> Of course I just wrote those steps down for the first time, so maybe\n> nobody else shares my vision. ;)\n\nThanks for writing it down. I'm sure that it has been loosely discussed\nover a while, but this is the first time that I've seen it all in one\nplace.\n\n> -Peff\n\nThanks,\nTaylor\n"}]}