{"thread":{"id":"49720","subject":"[PATCHv2 00/24] Bring more repository handles into our code base]","startedAt":"2018-10-30T22:08:35Z","lastAt":"2018-11-14T04:15:12Z","messageCount":43,"participants":["Stefan Beller","Junio C Hamano","Derrick Stolee","Martin Ågren","Josh Steadmon","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"362015","messageId":"20181030220817.61691-1-sbeller@google.com","threadId":"49720","inReplyTo":null,"subject":"[PATCHv2 00/24] Bring more repository handles into our code base]","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:07:53Z","receivedAt":"2018-10-30T22:08:35Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This resends sb/more-repo-in-api\n\nOn Thu, Oct 25, 2018 at 2:14 AM SZEDER Gábor wrote:\n> On Tue, Oct 16, 2018 at 04:35:49PM -0700, Stefan Beller wrote:\n> > This converts the 'show_submodule_header' function to use\n> > the repository API properly, such that the submodule objects\n> > are not added to the main object store.\n>\n> This patch breaks the test suite with 'GIT_TEST_COMMIT_GRAPH=1', in\n> particular 't4041-diff-submodule-option.sh' fails with:\n> [...]\n\nI debugged into this and the last four patches will fix it.\n\nI also picked up the patch for pending semantic patches, as the\nfirst patch, can I have your sign off, please?\n\nThis also addresses Jonathans feedback in\nhttps://public-inbox.org/git/20181019203750.110741-1-jonathantanmy@google.com/\n\nA range-diff is below.\n\nThanks,\nStefan\n\nSZEDER Gábor (1):\n Makefile: add pending semantic patches\n\nStefan Beller (23):\n sha1_file: allow read_object to read objects in arbitrary repositories\n packfile: allow has_packed_and_bad to handle arbitrary repositories\n object-store: allow read_object_file_extended to read from arbitrary\n repositories\n object-store: prepare read_object_file to deal with arbitrary\n repositories\n object-store: prepare has_{sha1, object}_file[_with_flags] to handle\n arbitrary repositories\n object: parse_object to honor its repository argument\n commit: allow parse_commit* to handle arbitrary repositories\n commit-reach.c: allow paint_down_to_common to handle arbitrary\n repositories\n commit-reach.c: allow merge_bases_many to handle arbitrary\n repositories\n commit-reach.c: allow remove_redundant to handle arbitrary\n repositories\n commit-reach.c: allow get_merge_bases_many_0 to handle arbitrary\n repositories\n commit-reach: prepare get_merge_bases to handle arbitrary repositories\n commit-reach: prepare in_merge_bases[_many] to handle arbitrary\n repositories\n commit: prepare get_commit_buffer to handle arbitrary repositories\n commit: prepare repo_unuse_commit_buffer to handle arbitrary\n repositories\n commit: prepare logmsg_reencode to handle arbitrary repositories\n pretty: prepare format_commit_message to handle arbitrary repositories\n submodule: use submodule repos for object lookup\n submodule: don't add submodule as odb for push\n commit-graph: convert remaining function to handle arbitrary\n repositories\n commit: make free_commit_buffer and release_commit_memory repository\n agnostic\n path.h: make REPO_GIT_PATH_FUNC repository agnostic\n t/helper/test-repository: celebrate independence from the_repository\n\n Makefile | 6 +-\n builtin/fsck.c | 3 +-\n builtin/log.c | 6 +-\n builtin/rev-list.c | 3 +-\n cache.h | 2 +\n commit-graph.c | 40 +++--\n commit-reach.c | 73 +++++----\n commit-reach.h | 38 +++--\n commit.c | 41 ++---\n commit.h | 43 +++++-\n .../coccinelle/the_repository.pending.cocci | 144 ++++++++++++++++++\n object-store.h | 35 ++++-\n object.c | 8 +-\n packfile.c | 5 +-\n packfile.h | 2 +-\n path.h | 2 +-\n pretty.c | 28 ++--\n pretty.h | 7 +-\n sha1-file.c | 34 +++--\n streaming.c | 2 +-\n submodule.c | 79 +++++++---\n t/helper/test-repository.c | 10 ++\n 22 files changed, 459 insertions(+), 152 deletions(-)\n create mode 100644 contrib/coccinelle/the_repository.pending.cocci\n\ngit range-diff origin/sb/more-repo-in-api...\n[...] # rebased to origin/master\n -: ---------- > 116: 4ede3d42df Seventh batch for 2.20\n -: ---------- > 117: b1de196491 Makefile: add pending semantic patches\n 1: 1b9b5c695e = 118: f1be5eb487 sha1_file: allow read_object to read objects in arbitrary repositories\n 2: 33b94066f2 = 119: 9100a5705d packfile: allow has_packed_and_bad to handle arbitrary repositories\n 3: 5217b6b1e1 = 120: a4ad7791da object-store: allow read_object_file_extended to read from arbitrary repositories\n 4: 2b7239b55b ! 121: 5d7b3a6dd9 object-store: prepare read_object_file to deal with arbitrary repositories\n @@ -19,10 +19,10 @@\n Signed-off-by: Stefan Beller \n Signed-off-by: Junio C Hamano \n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n new file mode 100644\n --- /dev/null\n - +++ b/contrib/coccinelle/the_repository.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n +// This file is used for the ongoing refactoring of\n +// bringing the index or repository struct in all of\n @@ -36,6 +36,7 @@\n +- read_object_file(\n ++ repo_read_object_file(the_repository,\n + E, F, G)\n ++\n \n diff --git a/object-store.h b/object-store.h\n --- a/object-store.h\n 5: 24291f4d84 ! 122: 77d406fc51 object-store: prepare has_{sha1, object}_file[_with_flags] to handle arbitrary repositories\n @@ -3,16 +3,14 @@\n object-store: prepare has_{sha1, object}_file[_with_flags] to handle arbitrary repositories\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - - read_object_file(\n + repo_read_object_file(the_repository,\n E, F, G)\n -+\n + \n +@@\n +expression E;\n +@@\n 6: 6aa209978e = 123: 7224b378d8 object: parse_object to honor its repository argument\n 7: 9b2d6aa7c3 ! 124: e4d4ae08ca commit: allow parse_commit* to handle arbitrary repositories\n @@ -93,9 +93,9 @@\n \n struct buffer_slab;\n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - has_object_file_with_flags(\n + repo_has_object_file_with_flags(the_repository,\n 8: 0e7e681118 ! 125: f739ef6078 commit-reach.c: allow paint_down_to_common to handle arbitrary repositories\n @@ -5,7 +5,6 @@\n As the function is file local and not widely used, migrate it all at once.\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit-reach.c b/commit-reach.c\n --- a/commit-reach.c\n 9: e83b26f5b3 ! 126: c054a80564 commit-reach.c: allow merge_bases_many to handle arbitrary repositories\n @@ -3,7 +3,6 @@\n commit-reach.c: allow merge_bases_many to handle arbitrary repositories\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit-reach.c b/commit-reach.c\n --- a/commit-reach.c\n 10: d80b9de832 ! 127: 3434a5a262 commit-reach.c: allow remove_redundant to handle arbitrary repositories\n @@ -3,7 +3,6 @@\n commit-reach.c: allow remove_redundant to handle arbitrary repositories\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit-reach.c b/commit-reach.c\n --- a/commit-reach.c\n 11: 3ec21ad503 ! 128: 82f6e797bf commit-reach.c: allow get_merge_bases_many_0 to handle arbitrary repositories\n @@ -3,7 +3,6 @@\n commit-reach.c: allow get_merge_bases_many_0 to handle arbitrary repositories\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit-reach.c b/commit-reach.c\n --- a/commit-reach.c\n 12: 3f21279f50 ! 129: b66b636a97 commit-reach: prepare get_merge_bases to handle arbitrary repositories\n @@ -9,7 +9,6 @@\n functions behind a wrapper macro.\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit-reach.c b/commit-reach.c\n --- a/commit-reach.c\n @@ -91,9 +90,9 @@\n int is_descendant_of(struct commit *commit, struct commit_list *with_commit);\n int in_merge_bases_many(struct commit *commit, int nr_reference, struct commit **reference);\n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - parse_commit(\n + repo_parse_commit(the_repository,\n @@ -124,3 +123,4 @@\n +- get_merge_bases_many_dirty(\n ++ repo_get_merge_bases_many_dirty(the_repository,\n + E, F, G);\n ++\n 13: 4decc3acc1 ! 130: e68e48ca91 commit-reach: prepare in_merge_bases[_many] to handle arbitrary repositories\n @@ -3,7 +3,6 @@\n commit-reach: prepare in_merge_bases[_many] to handle arbitrary repositories\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit-reach.c b/commit-reach.c\n --- a/commit-reach.c\n @@ -76,14 +75,13 @@\n /*\n * Takes a list of commits and returns a new list where those\n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - - get_merge_bases_many_dirty(\n + repo_get_merge_bases_many_dirty(the_repository,\n E, F, G);\n -+\n + \n +@@\n +expression E;\n +expression F;\n 14: 141b86ea89 ! 131: aac401ca98 commit: prepare get_commit_buffer to handle arbitrary repositories\n @@ -3,7 +3,6 @@\n commit: prepare get_commit_buffer to handle arbitrary repositories\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit.c b/commit.c\n --- a/commit.c\n @@ -46,9 +45,9 @@\n /*\n * Tell the commit subsytem that we are done with a particular commit buffer.\n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - in_merge_bases_many(\n + repo_in_merge_bases_many(the_repository,\n 15: 2cc415c1db ! 132: c45e6d2c0c commit: prepare repo_unuse_commit_buffer to handle arbitrary repositories\n @@ -3,7 +3,6 @@\n commit: prepare repo_unuse_commit_buffer to handle arbitrary repositories\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/commit.c b/commit.c\n --- a/commit.c\n @@ -42,9 +41,9 @@\n /*\n * Free any cached object buffer associated with the commit.\n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - get_commit_buffer(\n + repo_get_commit_buffer(the_repository,\n 16: 554daa8cfc ! 133: 25e8f5dec4 commit: prepare logmsg_reencode to handle arbitrary repositories\n @@ -24,9 +24,9 @@\n \n /** Removes the first commit from a list sorted by date, and adds all\n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - unuse_commit_buffer(\n + repo_unuse_commit_buffer(the_repository,\n 17: bd8737176b ! 134: 2cab6c18b6 pretty: prepare format_commit_message to handle arbitrary repositories\n @@ -5,9 +5,9 @@\n Signed-off-by: Stefan Beller \n Signed-off-by: Junio C Hamano \n \n - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n - --- a/contrib/coccinelle/the_repository.cocci\n - +++ b/contrib/coccinelle/the_repository.cocci\n + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n + --- a/contrib/coccinelle/the_repository.pending.cocci\n + +++ b/contrib/coccinelle/the_repository.pending.cocci\n @@\n - logmsg_reencode(\n + repo_logmsg_reencode(the_repository,\n 18: b303ef65e7 ! 135: 794592c573 submodule: use submodule repos for object lookup\n @@ -7,7 +7,6 @@\n are not added to the main object store.\n \n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/submodule.c b/submodule.c\n --- a/submodule.c\n @@ -46,24 +45,26 @@\n + * in .gitmodules. This function exists only to preserve historical behavior,\n + *\n + * Returns 0 on success, -1 when the submodule is not present.\n -+ */\n -+static int open_submodule(struct repository *out, const char *path)\n + */\n +-static void show_submodule_header(struct diff_options *o, const char *path,\n ++static struct repository *open_submodule(const char *path)\n +{\n +\tstruct strbuf sb = STRBUF_INIT;\n ++\tstruct repository *out = xmalloc(sizeof(*out));\n +\n +\tif (submodule_to_gitdir(&sb, path) || repo_init(out, sb.buf, NULL)) {\n +\t\tstrbuf_release(&sb);\n -+\t\treturn -1;\n ++\t\tfree(out);\n ++\t\treturn NULL;\n +\t}\n +\n -+\tout->submodule_prefix = xstrdup(path);\n +\tout->submodule_prefix = xstrfmt(\"%s%s/\",\n +\t\t\t\t\tthe_repository->submodule_prefix ?\n +\t\t\t\t\tthe_repository->submodule_prefix :\n +\t\t\t\t\t\"\", path);\n +\n +\tstrbuf_release(&sb);\n -+\treturn 0;\n ++\treturn out;\n +}\n +\n +/*\n @@ -73,8 +74,7 @@\n + * If it can locate the submodule git directory it will create a repository\n + * handle for the submodule and lookup both the left and right commits and\n + * put them into the left and right pointers.\n - */\n --static void show_submodule_header(struct diff_options *o, const char *path,\n ++ */\n +static void show_submodule_header(struct diff_options *o,\n +\t\tconst char *path,\n \t\tstruct object_id *one, struct object_id *two,\n @@ -116,11 +116,9 @@\n \tstruct rev_info rev;\n \tstruct commit *left = NULL, *right = NULL;\n \tstruct commit_list *merge_bases = NULL;\n -+\tstruct repository subrepo, *sub = &subrepo;\n -+\n -+\tif (open_submodule(&subrepo, path) < 0)\n -+\t\tsub = NULL;\n ++\tstruct repository *sub;\n \n ++\tsub = open_submodule(path);\n \tshow_submodule_header(o, path, one, two, dirty_submodule,\n -\t\t\t &left, &right, &merge_bases);\n +\t\t\t sub, &left, &right, &merge_bases);\n @@ -147,8 +145,10 @@\n \t\tfree_commit_list(merge_bases);\n \tclear_commit_marks(left, ~0);\n \tclear_commit_marks(right, ~0);\n -+\tif (sub)\n ++\tif (sub) {\n +\t\trepo_clear(sub);\n ++\t\tfree(sub);\n ++\t}\n }\n \n void show_submodule_inline_diff(struct diff_options *o, const char *path,\n @@ -156,11 +156,9 @@\n \tstruct commit_list *merge_bases = NULL;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n -+\tstruct repository subrepo, *sub = &subrepo;\n -+\n -+\tif (open_submodule(&subrepo, path) < 0)\n -+\t\tsub = NULL;\n ++\tstruct repository *sub;\n \n ++\tsub = open_submodule(path);\n \tshow_submodule_header(o, path, one, two, dirty_submodule,\n -\t\t\t &left, &right, &merge_bases);\n +\t\t\t sub, &left, &right, &merge_bases);\n @@ -171,8 +169,10 @@\n \t\tclear_commit_marks(left, ~0);\n \tif (right)\n \t\tclear_commit_marks(right, ~0);\n -+\tif (sub)\n ++\tif (sub) {\n +\t\trepo_clear(sub);\n ++\t\tfree(sub);\n ++\t}\n }\n \n int should_update_submodules(void)\n 19: dbb0dd9322 ! 136: df748a859f submodule: don't add submodule as odb for push\n @@ -8,8 +8,10 @@\n objects, and the actual push is done by spawning another process,\n which handles object access by itself.)\n \n + This code of push_submodule() is exercised in t5531 and continues\n + to work, showing that the submodule odbc is not needed.\n +\n Signed-off-by: Stefan Beller \n - Signed-off-by: Junio C Hamano \n \n diff --git a/submodule.c b/submodule.c\n --- a/submodule.c\n -: ---------- > 137: c9e990afac commit-graph: convert remaining function to handle arbitrary repositories\n -: ---------- > 138: a086f3dd11 commit: make free_commit_buffer and release_commit_memory repository agnostic\n -: ---------- > 139: 878bd34799 path.h: make REPO_GIT_PATH_FUNC repository agnostic\n -: ---------- > 140: 641824bbeb t/helper/test-repository: celebrate independence from the_repository\n"},{"id":"362016","messageId":"20181030220817.61691-2-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 01/24] Makefile: add pending semantic patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:07:54Z","receivedAt":"2018-10-30T22:08:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: SZEDER Gábor \n\nThere are basically two main use cases for semantic patches:\n\n - To avoid undesirable code patterns, e.g. we should not use\n sha1_to_hex(oid.hash) or strbuf_addf(&sb, \"fixed string\"), but\n use oid_to_hex(&oid) or strbuf_addstr(&sb, \"fixed string\")\n instead. Note that in these cases we don't remove the\n functions sha1_to_hex() or strbuf_addf(), because there are\n good reasons to use them in other scenarios.\n\n Our semantic patches under 'contrib/coccinelle/' fall into\n this category, and we have 'make coccicheck' and the static\n analysis build job on Travis CI to catch these undesirable\n code patterns preferably early, and to prevent them from\n entering our codebase.\n\n - To perform one-off code transformations, e.g. to modify a\n function's name and/or signature and convert all its\n callsites; see e.g. commits abef9020e3 (sha1_file: convert\n sha1_object_info* to object_id, 2018-03-12) and b4f5aca40e\n (sha1_file: convert read_sha1_file to struct object_id,\n 2018-03-12).\n\nTo allows semantic patches of the second category, we'll introduce\nthe concept of \"pending\" semantic patches, stored in\n'contrib/coccinelle/.pending.cocci' files, modifying\n'make coccicheck' to skip them, and adding the new 'make\ncoccicheck-pending' target to make it convenient to apply them.\n\n[Missing: SZEDERs sign off, so I also do not sign off]\n---\n Makefile | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b08d5ea258..6ea2bbd7f5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2739,9 +2739,11 @@ endif\n \tthen \\\n \t\techo ' ' SPATCH result: $@; \\\n \tfi\n-coccicheck: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.cocci))\n+coccicheck: $(addsuffix .patch,$(filter-out %.pending.cocci,$(wildcard contrib/coccinelle/*.cocci)))\n \n-.PHONY: coccicheck\n+coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.cocci))\n+\n+.PHONY: coccicheck coccicheck-pending\n \n ### Installation rules\n \n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362017","messageId":"20181030220817.61691-3-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 02/24] sha1_file: allow read_object to read objects in arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:07:55Z","receivedAt":"2018-10-30T22:08:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Allow read_object (a file local functon in sha1_file) to\nhandle arbitrary repositories by passing the repository down\nto oid_object_info_extended.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n sha1-file.c | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/sha1-file.c b/sha1-file.c\nindex dd0b6aa873..b8ce21cbaf 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -1361,7 +1361,9 @@ int oid_object_info(struct repository *r,\n \treturn type;\n }\n \n-static void *read_object(const unsigned char *sha1, enum object_type *type,\n+static void *read_object(struct repository *r,\n+\t\t\t const unsigned char *sha1,\n+\t\t\t enum object_type *type,\n \t\t\t unsigned long *size)\n {\n \tstruct object_id oid;\n@@ -1373,7 +1375,7 @@ static void *read_object(const unsigned char *sha1, enum object_type *type,\n \n \thashcpy(oid.hash, sha1);\n \n-\tif (oid_object_info_extended(the_repository, &oid, &oi, 0) < 0)\n+\tif (oid_object_info_extended(r, &oid, &oi, 0) < 0)\n \t\treturn NULL;\n \treturn content;\n }\n@@ -1414,7 +1416,7 @@ void *read_object_file_extended(const struct object_id *oid,\n \t\tlookup_replace_object(the_repository, oid) : oid;\n \n \terrno = 0;\n-\tdata = read_object(repl->hash, type, size);\n+\tdata = read_object(the_repository, repl->hash, type, size);\n \tif (data)\n \t\treturn data;\n \n@@ -1755,7 +1757,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)\n \n \tif (has_loose_object(oid))\n \t\treturn 0;\n-\tbuf = read_object(oid->hash, &type, &len);\n+\tbuf = read_object(the_repository, oid->hash, &type, &len);\n \tif (!buf)\n \t\treturn error(_(\"cannot read sha1_file for %s\"), oid_to_hex(oid));\n \thdrlen = xsnprintf(hdr, sizeof(hdr), \"%s %lu\", type_name(type), len) + 1;\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362018","messageId":"20181030220817.61691-4-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 03/24] packfile: allow has_packed_and_bad to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:07:56Z","receivedAt":"2018-10-30T22:08:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"has_packed_and_bad is not widely used, so just migrate it all at once.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n packfile.c | 5 +++--\n packfile.h | 2 +-\n sha1-file.c | 2 +-\n 3 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex f2850a00b5..63f8dc7b98 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1138,12 +1138,13 @@ void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1)\n \tp->num_bad_objects++;\n }\n \n-const struct packed_git *has_packed_and_bad(const unsigned char *sha1)\n+const struct packed_git *has_packed_and_bad(struct repository *r,\n+\t\t\t\t\t const unsigned char *sha1)\n {\n \tstruct packed_git *p;\n \tunsigned i;\n \n-\tfor (p = the_repository->objects->packed_git; p; p = p->next)\n+\tfor (p = r->objects->packed_git; p; p = p->next)\n \t\tfor (i = 0; i < p->num_bad_objects; i++)\n \t\t\tif (hasheq(sha1,\n \t\t\t\t p->bad_object_sha1 + the_hash_algo->rawsz * i))\ndiff --git a/packfile.h b/packfile.h\nindex 6c4037605d..d70c6d9afb 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -146,7 +146,7 @@ extern int packed_object_info(struct repository *r,\n \t\t\t off_t offset, struct object_info *);\n \n extern void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1);\n-extern const struct packed_git *has_packed_and_bad(const unsigned char *sha1);\n+extern const struct packed_git *has_packed_and_bad(struct repository *r, const unsigned char *sha1);\n \n /*\n * Iff a pack file in the given repository contains the object named by sha1,\ndiff --git a/sha1-file.c b/sha1-file.c\nindex b8ce21cbaf..856e000ee1 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -1432,7 +1432,7 @@ void *read_object_file_extended(const struct object_id *oid,\n \t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(repl), path);\n \n-\tif ((p = has_packed_and_bad(repl->hash)) != NULL)\n+\tif ((p = has_packed_and_bad(the_repository, repl->hash)) != NULL)\n \t\tdie(_(\"packed object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(repl), p->pack_name);\n \n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362019","messageId":"20181030220817.61691-5-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 04/24] object-store: allow read_object_file_extended to read from arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:07:57Z","receivedAt":"2018-10-30T22:08:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"read_object_file_extended is not widely used, so migrate it all at once.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n object-store.h | 5 +++--\n sha1-file.c | 11 ++++++-----\n streaming.c | 2 +-\n 3 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/object-store.h b/object-store.h\nindex 63b7605a3e..3d98a682b2 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -161,12 +161,13 @@ void sha1_file_name(struct repository *r, struct strbuf *buf, const unsigned cha\n \n void *map_sha1_file(struct repository *r, const unsigned char *sha1, unsigned long *size);\n \n-extern void *read_object_file_extended(const struct object_id *oid,\n+extern void *read_object_file_extended(struct repository *r,\n+\t\t\t\t const struct object_id *oid,\n \t\t\t\t enum object_type *type,\n \t\t\t\t unsigned long *size, int lookup_replace);\n static inline void *read_object_file(const struct object_id *oid, enum object_type *type, unsigned long *size)\n {\n-\treturn read_object_file_extended(oid, type, size, 1);\n+\treturn read_object_file_extended(the_repository, oid, type, size, 1);\n }\n \n /* Read and unpack an object file into memory, write memory to an object file */\ndiff --git a/sha1-file.c b/sha1-file.c\nindex 856e000ee1..c5b704aec5 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -1403,7 +1403,8 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,\n * deal with them should arrange to call read_object() and give error\n * messages themselves.\n */\n-void *read_object_file_extended(const struct object_id *oid,\n+void *read_object_file_extended(struct repository *r,\n+\t\t\t\tconst struct object_id *oid,\n \t\t\t\tenum object_type *type,\n \t\t\t\tunsigned long *size,\n \t\t\t\tint lookup_replace)\n@@ -1413,10 +1414,10 @@ void *read_object_file_extended(const struct object_id *oid,\n \tconst char *path;\n \tstruct stat st;\n \tconst struct object_id *repl = lookup_replace ?\n-\t\tlookup_replace_object(the_repository, oid) : oid;\n+\t\tlookup_replace_object(r, oid) : oid;\n \n \terrno = 0;\n-\tdata = read_object(the_repository, repl->hash, type, size);\n+\tdata = read_object(r, repl->hash, type, size);\n \tif (data)\n \t\treturn data;\n \n@@ -1428,11 +1429,11 @@ void *read_object_file_extended(const struct object_id *oid,\n \t\tdie(_(\"replacement %s not found for %s\"),\n \t\t oid_to_hex(repl), oid_to_hex(oid));\n \n-\tif (!stat_sha1_file(the_repository, repl->hash, &st, &path))\n+\tif (!stat_sha1_file(r, repl->hash, &st, &path))\n \t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(repl), path);\n \n-\tif ((p = has_packed_and_bad(the_repository, repl->hash)) != NULL)\n+\tif ((p = has_packed_and_bad(r, repl->hash)) != NULL)\n \t\tdie(_(\"packed object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(repl), p->pack_name);\n \ndiff --git a/streaming.c b/streaming.c\nindex d1e6b2dce6..c843a1230f 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -490,7 +490,7 @@ static struct stream_vtbl incore_vtbl = {\n \n static open_method_decl(incore)\n {\n-\tst->u.incore.buf = read_object_file_extended(oid, type, &st->size, 0);\n+\tst->u.incore.buf = read_object_file_extended(the_repository, oid, type, &st->size, 0);\n \tst->u.incore.read_ptr = 0;\n \tst->vtbl = &incore_vtbl;\n \n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362020","messageId":"20181030220817.61691-6-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 05/24] object-store: prepare read_object_file to deal with arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:07:58Z","receivedAt":"2018-10-30T22:08:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As read_object_file is a widely used function (which is also regularly used\nin new code in flight between master..pu), changing its signature is painful\nis hard, as other series in flight rely on the original signature. It would\nburden the maintainer if we'd just change the signature.\n\nIntroduce repo_read_object_file which takes the repository argument, and\nhide the original read_object_file as a macro behind\nNO_THE_REPOSITORY_COMPATIBILITY_MACROS, similar to\ne675765235 (diff.c: remove implicit dependency on the_index, 2018-09-21)\n\nAdd a coccinelle patch to convert existing callers, but do not apply\nthe resulting patch from 'make coccicheck' to keep the diff of this\npatch small.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n contrib/coccinelle/the_repository.pending.cocci | 13 +++++++++++++\n object-store.h | 10 ++++++++--\n 2 files changed, 21 insertions(+), 2 deletions(-)\n create mode 100644 contrib/coccinelle/the_repository.pending.cocci\n\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nnew file mode 100644\nindex 0000000000..3c7fa70502\n--- /dev/null\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -0,0 +1,13 @@\n+// This file is used for the ongoing refactoring of\n+// bringing the index or repository struct in all of\n+// our code base.\n+\n+@@\n+expression E;\n+expression F;\n+expression G;\n+@@\n+- read_object_file(\n++ repo_read_object_file(the_repository,\n+ E, F, G)\n+\ndiff --git a/object-store.h b/object-store.h\nindex 3d98a682b2..00a64622e6 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -165,10 +165,16 @@ extern void *read_object_file_extended(struct repository *r,\n \t\t\t\t const struct object_id *oid,\n \t\t\t\t enum object_type *type,\n \t\t\t\t unsigned long *size, int lookup_replace);\n-static inline void *read_object_file(const struct object_id *oid, enum object_type *type, unsigned long *size)\n+static inline void *repo_read_object_file(struct repository *r,\n+\t\t\t\t\t const struct object_id *oid,\n+\t\t\t\t\t enum object_type *type,\n+\t\t\t\t\t unsigned long *size)\n {\n-\treturn read_object_file_extended(the_repository, oid, type, size, 1);\n+\treturn read_object_file_extended(r, oid, type, size, 1);\n }\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define read_object_file(oid, type, size) repo_read_object_file(the_repository, oid, type, size)\n+#endif\n \n /* Read and unpack an object file into memory, write memory to an object file */\n int oid_object_info(struct repository *r, const struct object_id *, unsigned long *);\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362021","messageId":"20181030220817.61691-7-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 06/24] object-store: prepare has_{sha1, object}_file[_with_flags] to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:07:59Z","receivedAt":"2018-10-30T22:08:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n .../coccinelle/the_repository.pending.cocci | 29 +++++++++++++++++++\n object-store.h | 22 ++++++++++----\n sha1-file.c | 15 ++++++----\n 3 files changed, 55 insertions(+), 11 deletions(-)\n\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex 3c7fa70502..46f3a1b23a 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -11,3 +11,32 @@ expression G;\n + repo_read_object_file(the_repository,\n E, F, G)\n \n+@@\n+expression E;\n+@@\n+- has_sha1_file(\n++ repo_has_sha1_file(the_repository,\n+ E)\n+\n+@@\n+expression E;\n+expression F;\n+@@\n+- has_sha1_file_with_flags(\n++ repo_has_sha1_file_with_flags(the_repository,\n+ E)\n+\n+@@\n+expression E;\n+@@\n+- has_object_file(\n++ repo_has_object_file(the_repository,\n+ E)\n+\n+@@\n+expression E;\n+expression F;\n+@@\n+- has_object_file_with_flags(\n++ repo_has_object_file_with_flags(the_repository,\n+ E)\ndiff --git a/object-store.h b/object-store.h\nindex 00a64622e6..2b5e6ff1ed 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -212,15 +212,27 @@ int read_loose_object(const char *path,\n * object_info. OBJECT_INFO_SKIP_CACHED is automatically set; pass\n * nonzero flags to also set other flags.\n */\n-extern int has_sha1_file_with_flags(const unsigned char *sha1, int flags);\n-static inline int has_sha1_file(const unsigned char *sha1)\n+int repo_has_sha1_file_with_flags(struct repository *r,\n+\t\t\t\t const unsigned char *sha1, int flags);\n+static inline int repo_has_sha1_file(struct repository *r,\n+\t\t\t\t const unsigned char *sha1)\n {\n-\treturn has_sha1_file_with_flags(sha1, 0);\n+\treturn repo_has_sha1_file_with_flags(r, sha1, 0);\n }\n \n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define has_sha1_file_with_flags(sha1, flags) repo_has_sha1_file_with_flags(the_repository, sha1, flags)\n+#define has_sha1_file(sha1) repo_has_sha1_file(the_repository, sha1)\n+#endif\n+\n /* Same as the above, except for struct object_id. */\n-extern int has_object_file(const struct object_id *oid);\n-extern int has_object_file_with_flags(const struct object_id *oid, int flags);\n+int repo_has_object_file(struct repository *r, const struct object_id *oid);\n+int repo_has_object_file_with_flags(struct repository *r,\n+\t\t\t\t const struct object_id *oid, int flags);\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define has_object_file(oid) repo_has_object_file(the_repository, oid)\n+#define has_object_file_with_flags(oid, flags) repo_has_object_file_with_flags(the_repository, oid, flags)\n+#endif\n \n /*\n * Return true iff an alternate object database has a loose object\ndiff --git a/sha1-file.c b/sha1-file.c\nindex c5b704aec5..e77273ccfd 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -1768,24 +1768,27 @@ int force_object_loose(const struct object_id *oid, time_t mtime)\n \treturn ret;\n }\n \n-int has_sha1_file_with_flags(const unsigned char *sha1, int flags)\n+int repo_has_sha1_file_with_flags(struct repository *r,\n+\t\t\t\t const unsigned char *sha1, int flags)\n {\n \tstruct object_id oid;\n \tif (!startup_info->have_repository)\n \t\treturn 0;\n \thashcpy(oid.hash, sha1);\n-\treturn oid_object_info_extended(the_repository, &oid, NULL,\n+\treturn oid_object_info_extended(r, &oid, NULL,\n \t\t\t\t\tflags | OBJECT_INFO_SKIP_CACHED) >= 0;\n }\n \n-int has_object_file(const struct object_id *oid)\n+int repo_has_object_file(struct repository *r,\n+\t\t\t const struct object_id *oid)\n {\n-\treturn has_sha1_file(oid->hash);\n+\treturn repo_has_sha1_file(r, oid->hash);\n }\n \n-int has_object_file_with_flags(const struct object_id *oid, int flags)\n+int repo_has_object_file_with_flags(struct repository *r,\n+\t\t\t\t const struct object_id *oid, int flags)\n {\n-\treturn has_sha1_file_with_flags(oid->hash, flags);\n+\treturn repo_has_sha1_file_with_flags(r, oid->hash, flags);\n }\n \n static void check_tree(const void *buf, size_t size)\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362022","messageId":"20181030220817.61691-8-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 07/24] object: parse_object to honor its repository argument","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:00Z","receivedAt":"2018-10-30T22:08:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In 8e4b0b6047 (object.c: allow parse_object to handle\narbitrary repositories, 2018-06-28), we forgot to pass the\nrepository down to the read_object_file.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n object.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/object.c b/object.c\nindex e54160550c..003f870484 100644\n--- a/object.c\n+++ b/object.c\n@@ -259,8 +259,8 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)\n \tif (obj && obj->parsed)\n \t\treturn obj;\n \n-\tif ((obj && obj->type == OBJ_BLOB && has_object_file(oid)) ||\n-\t (!obj && has_object_file(oid) &&\n+\tif ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||\n+\t (!obj && repo_has_object_file(r, oid) &&\n \t oid_object_info(r, oid, NULL) == OBJ_BLOB)) {\n \t\tif (check_object_signature(repl, NULL, 0, NULL) < 0) {\n \t\t\terror(_(\"sha1 mismatch %s\"), oid_to_hex(oid));\n@@ -270,7 +270,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)\n \t\treturn lookup_object(r, oid->hash);\n \t}\n \n-\tbuffer = read_object_file(oid, &type, &size);\n+\tbuffer = repo_read_object_file(r, oid, &type, &size);\n \tif (buffer) {\n \t\tif (check_object_signature(repl, buffer, size, type_name(type)) < 0) {\n \t\t\tfree(buffer);\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362023","messageId":"20181030220817.61691-9-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 08/24] commit: allow parse_commit* to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:01Z","receivedAt":"2018-10-30T22:08:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Just like the previous commit, parse_commit and friends are used a lot\nand are found in new patches, so we cannot change their signature easily.\n\nRe-introduce these function prefixed with 'repo_' that take a repository\nargument and keep the original as a shallow macro.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n commit.c | 18 ++++++++------\n commit.h | 17 +++++++++----\n .../coccinelle/the_repository.pending.cocci | 24 +++++++++++++++++++\n 3 files changed, 48 insertions(+), 11 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex dc8a39d52a..7a931d7fd4 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -443,7 +443,10 @@ int parse_commit_buffer(struct repository *r, struct commit *item, const void *b\n \treturn 0;\n }\n \n-int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_commit_graph)\n+int repo_parse_commit_internal(struct repository *r,\n+\t\t\t struct commit *item,\n+\t\t\t int quiet_on_missing,\n+\t\t\t int use_commit_graph)\n {\n \tenum object_type type;\n \tvoid *buffer;\n@@ -454,9 +457,9 @@ int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_com\n \t\treturn -1;\n \tif (item->object.parsed)\n \t\treturn 0;\n-\tif (use_commit_graph && parse_commit_in_graph(the_repository, item))\n+\tif (use_commit_graph && parse_commit_in_graph(r, item))\n \t\treturn 0;\n-\tbuffer = read_object_file(&item->object.oid, &type, &size);\n+\tbuffer = repo_read_object_file(r, &item->object.oid, &type, &size);\n \tif (!buffer)\n \t\treturn quiet_on_missing ? -1 :\n \t\t\terror(\"Could not read %s\",\n@@ -467,18 +470,19 @@ int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_com\n \t\t\t oid_to_hex(&item->object.oid));\n \t}\n \n-\tret = parse_commit_buffer(the_repository, item, buffer, size, 0);\n+\tret = parse_commit_buffer(r, item, buffer, size, 0);\n \tif (save_commit_buffer && !ret) {\n-\t\tset_commit_buffer(the_repository, item, buffer, size);\n+\t\tset_commit_buffer(r, item, buffer, size);\n \t\treturn 0;\n \t}\n \tfree(buffer);\n \treturn ret;\n }\n \n-int parse_commit_gently(struct commit *item, int quiet_on_missing)\n+int repo_parse_commit_gently(struct repository *r,\n+\t\t\t struct commit *item, int quiet_on_missing)\n {\n-\treturn parse_commit_internal(item, quiet_on_missing, 1);\n+\treturn repo_parse_commit_internal(r, item, quiet_on_missing, 1);\n }\n \n void parse_commit_or_die(struct commit *item)\ndiff --git a/commit.h b/commit.h\nindex 1d260d62f5..08935f9a19 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -79,12 +79,21 @@ struct commit *lookup_commit_reference_by_name(const char *name);\n struct commit *lookup_commit_or_die(const struct object_id *oid, const char *ref_name);\n \n int parse_commit_buffer(struct repository *r, struct commit *item, const void *buffer, unsigned long size, int check_graph);\n-int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_commit_graph);\n-int parse_commit_gently(struct commit *item, int quiet_on_missing);\n-static inline int parse_commit(struct commit *item)\n+int repo_parse_commit_internal(struct repository *r, struct commit *item,\n+\t\t\t int quiet_on_missing, int use_commit_graph);\n+int repo_parse_commit_gently(struct repository *r,\n+\t\t\t struct commit *item,\n+\t\t\t int quiet_on_missing);\n+static inline int repo_parse_commit(struct repository *r, struct commit *item)\n {\n-\treturn parse_commit_gently(item, 0);\n+\treturn repo_parse_commit_gently(r, item, 0);\n }\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define parse_commit_internal(item, quiet, use) repo_parse_commit_internal(the_repository, item, quiet, use)\n+#define parse_commit_gently(item, quiet) repo_parse_commit_gently(the_repository, item, quiet)\n+#define parse_commit(item) repo_parse_commit(the_repository, item)\n+#endif\n+\n void parse_commit_or_die(struct commit *item);\n \n struct buffer_slab;\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex 46f3a1b23a..b185fe0a1d 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -40,3 +40,27 @@ expression F;\n - has_object_file_with_flags(\n + repo_has_object_file_with_flags(the_repository,\n E)\n+\n+@@\n+expression E;\n+expression F;\n+expression G;\n+@@\n+- parse_commit_internal(\n++ repo_parse_commit_internal(the_repository,\n+ E, F, G)\n+\n+@@\n+expression E;\n+expression F;\n+@@\n+- parse_commit_gently(\n++ repo_parse_commit_gently(the_repository,\n+ E, F)\n+\n+@@\n+expression E;\n+@@\n+- parse_commit(\n++ repo_parse_commit(the_repository,\n+ E)\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362024","messageId":"20181030220817.61691-10-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 09/24] commit-reach.c: allow paint_down_to_common to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:02Z","receivedAt":"2018-10-30T22:08:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As the function is file local and not widely used, migrate it all at once.\n\nSigned-off-by: Stefan Beller \n---\n commit-reach.c | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex a9da65c462..080ae0a758 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -30,7 +30,8 @@ static int queue_has_nonstale(struct prio_queue *queue)\n }\n \n /* all input commits in one and twos[] must have been parsed! */\n-static struct commit_list *paint_down_to_common(struct commit *one, int n,\n+static struct commit_list *paint_down_to_common(struct repository *r,\n+\t\t\t\t\t\tstruct commit *one, int n,\n \t\t\t\t\t\tstruct commit **twos,\n \t\t\t\t\t\tint min_generation)\n {\n@@ -83,7 +84,7 @@ static struct commit_list *paint_down_to_common(struct commit *one, int n,\n \t\t\tparents = parents->next;\n \t\t\tif ((p->object.flags & flags) == flags)\n \t\t\t\tcontinue;\n-\t\t\tif (parse_commit(p))\n+\t\t\tif (repo_parse_commit(r, p))\n \t\t\t\treturn NULL;\n \t\t\tp->object.flags |= flags;\n \t\t\tprio_queue_put(&queue, p);\n@@ -116,7 +117,7 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n \t\t\treturn NULL;\n \t}\n \n-\tlist = paint_down_to_common(one, n, twos, 0);\n+\tlist = paint_down_to_common(the_repository, one, n, twos, 0);\n \n \twhile (list) {\n \t\tstruct commit *commit = pop_commit(&list);\n@@ -187,8 +188,8 @@ static int remove_redundant(struct commit **array, int cnt)\n \t\t\tif (array[j]->generation < min_generation)\n \t\t\t\tmin_generation = array[j]->generation;\n \t\t}\n-\t\tcommon = paint_down_to_common(array[i], filled, work,\n-\t\t\t\t\t min_generation);\n+\t\tcommon = paint_down_to_common(the_repository, array[i], filled,\n+\t\t\t\t\t work, min_generation);\n \t\tif (array[i]->object.flags & PARENT2)\n \t\t\tredundant[i] = 1;\n \t\tfor (j = 0; j < filled; j++)\n@@ -322,7 +323,9 @@ int in_merge_bases_many(struct commit *commit, int nr_reference, struct commit *\n \tif (commit->generation > min_generation)\n \t\treturn ret;\n \n-\tbases = paint_down_to_common(commit, nr_reference, reference, commit->generation);\n+\tbases = paint_down_to_common(the_repository, commit,\n+\t\t\t\t nr_reference, reference,\n+\t\t\t\t commit->generation);\n \tif (commit->object.flags & PARENT2)\n \t\tret = 1;\n \tclear_commit_marks(commit, all_flags);\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362025","messageId":"20181030220817.61691-11-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 10/24] commit-reach.c: allow merge_bases_many to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:03Z","receivedAt":"2018-10-30T22:08:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n commit-reach.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex 080ae0a758..4cf471bfaf 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -95,7 +95,9 @@ static struct commit_list *paint_down_to_common(struct repository *r,\n \treturn result;\n }\n \n-static struct commit_list *merge_bases_many(struct commit *one, int n, struct commit **twos)\n+static struct commit_list *merge_bases_many(struct repository *r,\n+\t\t\t\t\t struct commit *one, int n,\n+\t\t\t\t\t struct commit **twos)\n {\n \tstruct commit_list *list = NULL;\n \tstruct commit_list *result = NULL;\n@@ -110,14 +112,14 @@ static struct commit_list *merge_bases_many(struct commit *one, int n, struct co\n \t\t\treturn commit_list_insert(one, &result);\n \t}\n \n-\tif (parse_commit(one))\n+\tif (repo_parse_commit(r, one))\n \t\treturn NULL;\n \tfor (i = 0; i < n; i++) {\n-\t\tif (parse_commit(twos[i]))\n+\t\tif (repo_parse_commit(r, twos[i]))\n \t\t\treturn NULL;\n \t}\n \n-\tlist = paint_down_to_common(the_repository, one, n, twos, 0);\n+\tlist = paint_down_to_common(r, one, n, twos, 0);\n \n \twhile (list) {\n \t\tstruct commit *commit = pop_commit(&list);\n@@ -224,7 +226,7 @@ static struct commit_list *get_merge_bases_many_0(struct commit *one,\n \tstruct commit_list *result;\n \tint cnt, i;\n \n-\tresult = merge_bases_many(one, n, twos);\n+\tresult = merge_bases_many(the_repository, one, n, twos);\n \tfor (i = 0; i < n; i++) {\n \t\tif (one == twos[i])\n \t\t\treturn result;\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362026","messageId":"20181030220817.61691-12-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 11/24] commit-reach.c: allow remove_redundant to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:04Z","receivedAt":"2018-10-30T22:09:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n commit-reach.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex 4cf471bfaf..ab2bb1e5d5 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -156,7 +156,7 @@ struct commit_list *get_octopus_merge_bases(struct commit_list *in)\n \treturn ret;\n }\n \n-static int remove_redundant(struct commit **array, int cnt)\n+static int remove_redundant(struct repository *r, struct commit **array, int cnt)\n {\n \t/*\n \t * Some commit in the array may be an ancestor of\n@@ -174,7 +174,7 @@ static int remove_redundant(struct commit **array, int cnt)\n \tALLOC_ARRAY(filled_index, cnt - 1);\n \n \tfor (i = 0; i < cnt; i++)\n-\t\tparse_commit(array[i]);\n+\t\trepo_parse_commit(r, array[i]);\n \tfor (i = 0; i < cnt; i++) {\n \t\tstruct commit_list *common;\n \t\tuint32_t min_generation = array[i]->generation;\n@@ -190,7 +190,7 @@ static int remove_redundant(struct commit **array, int cnt)\n \t\t\tif (array[j]->generation < min_generation)\n \t\t\t\tmin_generation = array[j]->generation;\n \t\t}\n-\t\tcommon = paint_down_to_common(the_repository, array[i], filled,\n+\t\tcommon = paint_down_to_common(r, array[i], filled,\n \t\t\t\t\t work, min_generation);\n \t\tif (array[i]->object.flags & PARENT2)\n \t\t\tredundant[i] = 1;\n@@ -249,7 +249,7 @@ static struct commit_list *get_merge_bases_many_0(struct commit *one,\n \tclear_commit_marks(one, all_flags);\n \tclear_commit_marks_many(n, twos, all_flags);\n \n-\tcnt = remove_redundant(rslt, cnt);\n+\tcnt = remove_redundant(the_repository, rslt, cnt);\n \tresult = NULL;\n \tfor (i = 0; i < cnt; i++)\n \t\tcommit_list_insert_by_date(rslt[i], &result);\n@@ -370,7 +370,7 @@ struct commit_list *reduce_heads(struct commit_list *heads)\n \t\t\tp->item->object.flags &= ~STALE;\n \t\t}\n \t}\n-\tnum_head = remove_redundant(array, num_head);\n+\tnum_head = remove_redundant(the_repository, array, num_head);\n \tfor (i = 0; i < num_head; i++)\n \t\ttail = &commit_list_insert(array[i], tail)->next;\n \tfree(array);\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362027","messageId":"20181030220817.61691-14-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 13/24] commit-reach: prepare get_merge_bases to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:06Z","receivedAt":"2018-10-30T22:09:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Similarly to previous patches, the get_merge_base functions are used\noften in the code base, which makes migrating them hard.\n\nImplement the new functions, prefixed with 'repo_' and hide the old\nfunctions behind a wrapper macro.\n\nSigned-off-by: Stefan Beller \n---\n commit-reach.c | 24 ++++++++++-------\n commit-reach.h | 26 +++++++++++-------\n .../coccinelle/the_repository.pending.cocci | 27 +++++++++++++++++++\n 3 files changed, 57 insertions(+), 20 deletions(-)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex bf7a513991..3be5526957 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -258,23 +258,27 @@ static struct commit_list *get_merge_bases_many_0(struct repository *r,\n \treturn result;\n }\n \n-struct commit_list *get_merge_bases_many(struct commit *one,\n-\t\t\t\t\t int n,\n-\t\t\t\t\t struct commit **twos)\n+struct commit_list *repo_get_merge_bases_many(struct repository *r,\n+\t\t\t\t\t struct commit *one,\n+\t\t\t\t\t int n,\n+\t\t\t\t\t struct commit **twos)\n {\n-\treturn get_merge_bases_many_0(the_repository, one, n, twos, 1);\n+\treturn get_merge_bases_many_0(r, one, n, twos, 1);\n }\n \n-struct commit_list *get_merge_bases_many_dirty(struct commit *one,\n-\t\t\t\t\t int n,\n-\t\t\t\t\t struct commit **twos)\n+struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,\n+\t\t\t\t\t\t struct commit *one,\n+\t\t\t\t\t\t int n,\n+\t\t\t\t\t\t struct commit **twos)\n {\n-\treturn get_merge_bases_many_0(the_repository, one, n, twos, 0);\n+\treturn get_merge_bases_many_0(r, one, n, twos, 0);\n }\n \n-struct commit_list *get_merge_bases(struct commit *one, struct commit *two)\n+struct commit_list *repo_get_merge_bases(struct repository *r,\n+\t\t\t\t\t struct commit *one,\n+\t\t\t\t\t struct commit *two)\n {\n-\treturn get_merge_bases_many_0(the_repository, one, 1, &two, 1);\n+\treturn get_merge_bases_many_0(r, one, 1, &two, 1);\n }\n \n /*\ndiff --git a/commit-reach.h b/commit-reach.h\nindex 122a23a24d..c647447263 100644\n--- a/commit-reach.h\n+++ b/commit-reach.h\n@@ -8,17 +8,23 @@ struct commit_list;\n struct contains_cache;\n struct ref_filter;\n \n-struct commit_list *get_merge_bases_many(struct commit *one,\n-\t\t\t\t\t int n,\n-\t\t\t\t\t struct commit **twos);\n-struct commit_list *get_merge_bases_many_dirty(struct commit *one,\n-\t\t\t\t\t int n,\n-\t\t\t\t\t struct commit **twos);\n-struct commit_list *get_merge_bases(struct commit *one, struct commit *two);\n-struct commit_list *get_octopus_merge_bases(struct commit_list *in);\n-\n+struct commit_list *repo_get_merge_bases(struct repository *r,\n+\t\t\t\t\t struct commit *rev1,\n+\t\t\t\t\t struct commit *rev2);\n+struct commit_list *repo_get_merge_bases_many(struct repository *r,\n+\t\t\t\t\t struct commit *one, int n,\n+\t\t\t\t\t struct commit **twos);\n /* To be used only when object flags after this call no longer matter */\n-struct commit_list *get_merge_bases_many_dirty(struct commit *one, int n, struct commit **twos);\n+struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,\n+\t\t\t\t\t\t struct commit *one, int n,\n+\t\t\t\t\t\t struct commit **twos);\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define get_merge_bases(r1, r2) repo_get_merge_bases(the_repository, r1, r2)\n+#define get_merge_bases_many(one, n, two) repo_get_merge_bases_many(the_repository, one, n, two)\n+#define get_merge_bases_many_dirty(one, n, twos) repo_get_merge_bases_many_dirty(the_repository, one, n, twos)\n+#endif\n+\n+struct commit_list *get_octopus_merge_bases(struct commit_list *in);\n \n int is_descendant_of(struct commit *commit, struct commit_list *with_commit);\n int in_merge_bases_many(struct commit *commit, int nr_reference, struct commit **reference);\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex b185fe0a1d..5e037fe428 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -64,3 +64,30 @@ expression E;\n - parse_commit(\n + repo_parse_commit(the_repository,\n E)\n+\n+@@\n+expression E;\n+expression F;\n+@@\n+- get_merge_bases(\n++ repo_get_merge_bases(the_repository,\n+ E, F);\n+\n+@@\n+expression E;\n+expression F;\n+expression G;\n+@@\n+- get_merge_bases_many(\n++ repo_get_merge_bases_many(the_repository,\n+ E, F, G);\n+\n+@@\n+expression E;\n+expression F;\n+expression G;\n+@@\n+- get_merge_bases_many_dirty(\n++ repo_get_merge_bases_many_dirty(the_repository,\n+ E, F, G);\n+\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362028","messageId":"20181030220817.61691-15-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 14/24] commit-reach: prepare in_merge_bases[_many] to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:07Z","receivedAt":"2018-10-30T22:09:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n commit-reach.c | 15 +++++++++------\n commit-reach.h | 12 ++++++++++--\n contrib/coccinelle/the_repository.pending.cocci | 16 ++++++++++++++++\n 3 files changed, 35 insertions(+), 8 deletions(-)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex 3be5526957..88a65f854f 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -312,16 +312,17 @@ int is_descendant_of(struct commit *commit, struct commit_list *with_commit)\n /*\n * Is \"commit\" an ancestor of one of the \"references\"?\n */\n-int in_merge_bases_many(struct commit *commit, int nr_reference, struct commit **reference)\n+int repo_in_merge_bases_many(struct repository *r, struct commit *commit,\n+\t\t\t int nr_reference, struct commit **reference)\n {\n \tstruct commit_list *bases;\n \tint ret = 0, i;\n \tuint32_t min_generation = GENERATION_NUMBER_INFINITY;\n \n-\tif (parse_commit(commit))\n+\tif (repo_parse_commit(r, commit))\n \t\treturn ret;\n \tfor (i = 0; i < nr_reference; i++) {\n-\t\tif (parse_commit(reference[i]))\n+\t\tif (repo_parse_commit(r, reference[i]))\n \t\t\treturn ret;\n \t\tif (reference[i]->generation < min_generation)\n \t\t\tmin_generation = reference[i]->generation;\n@@ -330,7 +331,7 @@ int in_merge_bases_many(struct commit *commit, int nr_reference, struct commit *\n \tif (commit->generation > min_generation)\n \t\treturn ret;\n \n-\tbases = paint_down_to_common(the_repository, commit,\n+\tbases = paint_down_to_common(r, commit,\n \t\t\t\t nr_reference, reference,\n \t\t\t\t commit->generation);\n \tif (commit->object.flags & PARENT2)\n@@ -344,9 +345,11 @@ int in_merge_bases_many(struct commit *commit, int nr_reference, struct commit *\n /*\n * Is \"commit\" an ancestor of (i.e. reachable from) the \"reference\"?\n */\n-int in_merge_bases(struct commit *commit, struct commit *reference)\n+int repo_in_merge_bases(struct repository *r,\n+\t\t\tstruct commit *commit,\n+\t\t\tstruct commit *reference)\n {\n-\treturn in_merge_bases_many(commit, 1, &reference);\n+\treturn repo_in_merge_bases_many(r, commit, 1, &reference);\n }\n \n struct commit_list *reduce_heads(struct commit_list *heads)\ndiff --git a/commit-reach.h b/commit-reach.h\nindex c647447263..6b16f12467 100644\n--- a/commit-reach.h\n+++ b/commit-reach.h\n@@ -27,8 +27,16 @@ struct commit_list *repo_get_merge_bases_many_dirty(struct repository *r,\n struct commit_list *get_octopus_merge_bases(struct commit_list *in);\n \n int is_descendant_of(struct commit *commit, struct commit_list *with_commit);\n-int in_merge_bases_many(struct commit *commit, int nr_reference, struct commit **reference);\n-int in_merge_bases(struct commit *commit, struct commit *reference);\n+int repo_in_merge_bases(struct repository *r,\n+\t\t\tstruct commit *commit,\n+\t\t\tstruct commit *reference);\n+int repo_in_merge_bases_many(struct repository *r,\n+\t\t\t struct commit *commit,\n+\t\t\t int nr_reference, struct commit **reference);\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define in_merge_bases(c1, c2) repo_in_merge_bases(the_repository, c1, c2)\n+#define in_merge_bases_many(c1, n, cs) repo_in_merge_bases_many(the_repository, c1, n, cs)\n+#endif\n \n /*\n * Takes a list of commits and returns a new list where those\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex 5e037fe428..8c6a71bf64 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -91,3 +91,19 @@ expression G;\n + repo_get_merge_bases_many_dirty(the_repository,\n E, F, G);\n \n+@@\n+expression E;\n+expression F;\n+@@\n+- in_merge_bases(\n++ repo_in_merge_bases(the_repository,\n+ E, F);\n+\n+@@\n+expression E;\n+expression F;\n+expression G;\n+@@\n+- in_merge_bases_many(\n++ repo_in_merge_bases_many(the_repository,\n+ E, F, G);\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362029","messageId":"20181030220817.61691-16-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 15/24] commit: prepare get_commit_buffer to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:08Z","receivedAt":"2018-10-30T22:09:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n commit.c | 8 +++++---\n commit.h | 7 ++++++-\n contrib/coccinelle/the_repository.pending.cocci | 8 ++++++++\n 3 files changed, 19 insertions(+), 4 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 7a931d7fd4..4034def16c 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -297,13 +297,15 @@ const void *get_cached_commit_buffer(struct repository *r, const struct commit *\n \treturn v->buffer;\n }\n \n-const void *get_commit_buffer(const struct commit *commit, unsigned long *sizep)\n+const void *repo_get_commit_buffer(struct repository *r,\n+\t\t\t\t const struct commit *commit,\n+\t\t\t\t unsigned long *sizep)\n {\n-\tconst void *ret = get_cached_commit_buffer(the_repository, commit, sizep);\n+\tconst void *ret = get_cached_commit_buffer(r, commit, sizep);\n \tif (!ret) {\n \t\tenum object_type type;\n \t\tunsigned long size;\n-\t\tret = read_object_file(&commit->object.oid, &type, &size);\n+\t\tret = repo_read_object_file(r, &commit->object.oid, &type, &size);\n \t\tif (!ret)\n \t\t\tdie(\"cannot read commit object %s\",\n \t\t\t oid_to_hex(&commit->object.oid));\ndiff --git a/commit.h b/commit.h\nindex 08935f9a19..591a77a5bb 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -117,7 +117,12 @@ const void *get_cached_commit_buffer(struct repository *, const struct commit *,\n * from disk. The resulting memory should not be modified, and must be given\n * to unuse_commit_buffer when the caller is done.\n */\n-const void *get_commit_buffer(const struct commit *, unsigned long *size);\n+const void *repo_get_commit_buffer(struct repository *r,\n+\t\t\t\t const struct commit *,\n+\t\t\t\t unsigned long *size);\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define get_commit_buffer(c, s) repo_get_commit_buffer(the_repository, c, s)\n+#endif\n \n /*\n * Tell the commit subsytem that we are done with a particular commit buffer.\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex 8c6a71bf64..4018e6eaf7 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -107,3 +107,11 @@ expression G;\n - in_merge_bases_many(\n + repo_in_merge_bases_many(the_repository,\n E, F, G);\n+\n+@@\n+expression E;\n+expression F;\n+@@\n+- get_commit_buffer(\n++ repo_get_commit_buffer(the_repository,\n+ E, F);\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362030","messageId":"20181030220817.61691-13-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 12/24] commit-reach.c: allow get_merge_bases_many_0 to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:05Z","receivedAt":"2018-10-30T22:09:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n commit-reach.c | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/commit-reach.c b/commit-reach.c\nindex ab2bb1e5d5..bf7a513991 100644\n--- a/commit-reach.c\n+++ b/commit-reach.c\n@@ -216,7 +216,8 @@ static int remove_redundant(struct repository *r, struct commit **array, int cnt\n \treturn filled;\n }\n \n-static struct commit_list *get_merge_bases_many_0(struct commit *one,\n+static struct commit_list *get_merge_bases_many_0(struct repository *r,\n+\t\t\t\t\t\t struct commit *one,\n \t\t\t\t\t\t int n,\n \t\t\t\t\t\t struct commit **twos,\n \t\t\t\t\t\t int cleanup)\n@@ -226,7 +227,7 @@ static struct commit_list *get_merge_bases_many_0(struct commit *one,\n \tstruct commit_list *result;\n \tint cnt, i;\n \n-\tresult = merge_bases_many(the_repository, one, n, twos);\n+\tresult = merge_bases_many(r, one, n, twos);\n \tfor (i = 0; i < n; i++) {\n \t\tif (one == twos[i])\n \t\t\treturn result;\n@@ -249,7 +250,7 @@ static struct commit_list *get_merge_bases_many_0(struct commit *one,\n \tclear_commit_marks(one, all_flags);\n \tclear_commit_marks_many(n, twos, all_flags);\n \n-\tcnt = remove_redundant(the_repository, rslt, cnt);\n+\tcnt = remove_redundant(r, rslt, cnt);\n \tresult = NULL;\n \tfor (i = 0; i < cnt; i++)\n \t\tcommit_list_insert_by_date(rslt[i], &result);\n@@ -261,19 +262,19 @@ struct commit_list *get_merge_bases_many(struct commit *one,\n \t\t\t\t\t int n,\n \t\t\t\t\t struct commit **twos)\n {\n-\treturn get_merge_bases_many_0(one, n, twos, 1);\n+\treturn get_merge_bases_many_0(the_repository, one, n, twos, 1);\n }\n \n struct commit_list *get_merge_bases_many_dirty(struct commit *one,\n \t\t\t\t\t int n,\n \t\t\t\t\t struct commit **twos)\n {\n-\treturn get_merge_bases_many_0(one, n, twos, 0);\n+\treturn get_merge_bases_many_0(the_repository, one, n, twos, 0);\n }\n \n struct commit_list *get_merge_bases(struct commit *one, struct commit *two)\n {\n-\treturn get_merge_bases_many_0(one, 1, &two, 1);\n+\treturn get_merge_bases_many_0(the_repository, one, 1, &two, 1);\n }\n \n /*\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362031","messageId":"20181030220817.61691-17-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 16/24] commit: prepare repo_unuse_commit_buffer to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:09Z","receivedAt":"2018-10-30T22:09:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n commit.c | 6 ++++--\n commit.h | 7 ++++++-\n contrib/coccinelle/the_repository.pending.cocci | 8 ++++++++\n 3 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 4034def16c..7d2f3a9a93 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -318,10 +318,12 @@ const void *repo_get_commit_buffer(struct repository *r,\n \treturn ret;\n }\n \n-void unuse_commit_buffer(const struct commit *commit, const void *buffer)\n+void repo_unuse_commit_buffer(struct repository *r,\n+\t\t\t const struct commit *commit,\n+\t\t\t const void *buffer)\n {\n \tstruct commit_buffer *v = buffer_slab_peek(\n-\t\tthe_repository->parsed_objects->buffer_slab, commit);\n+\t\tr->parsed_objects->buffer_slab, commit);\n \tif (!(v && v->buffer == buffer))\n \t\tfree((void *)buffer);\n }\ndiff --git a/commit.h b/commit.h\nindex 591a77a5bb..57375e3239 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -130,7 +130,12 @@ const void *repo_get_commit_buffer(struct repository *r,\n * from an earlier call to get_commit_buffer. The buffer may or may not be\n * freed by this call; callers should not access the memory afterwards.\n */\n-void unuse_commit_buffer(const struct commit *, const void *buffer);\n+void repo_unuse_commit_buffer(struct repository *r,\n+\t\t\t const struct commit *,\n+\t\t\t const void *buffer);\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define unuse_commit_buffer(c, b) repo_unuse_commit_buffer(the_repository, c, b)\n+#endif\n \n /*\n * Free any cached object buffer associated with the commit.\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex 4018e6eaf7..516f19ffee 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -115,3 +115,11 @@ expression F;\n - get_commit_buffer(\n + repo_get_commit_buffer(the_repository,\n E, F);\n+\n+@@\n+expression E;\n+expression F;\n+@@\n+- unuse_commit_buffer(\n++ repo_unuse_commit_buffer(the_repository,\n+ E, F);\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362032","messageId":"20181030220817.61691-18-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 17/24] commit: prepare logmsg_reencode to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:10Z","receivedAt":"2018-10-30T22:09:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n commit.h | 8 ++++++++\n contrib/coccinelle/the_repository.pending.cocci | 9 +++++++++\n pretty.c | 13 +++++++------\n 3 files changed, 24 insertions(+), 6 deletions(-)\n\ndiff --git a/commit.h b/commit.h\nindex 57375e3239..2e6b799b26 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -180,6 +180,14 @@ extern int has_non_ascii(const char *text);\n extern const char *logmsg_reencode(const struct commit *commit,\n \t\t\t\t char **commit_encoding,\n \t\t\t\t const char *output_encoding);\n+const char *repo_logmsg_reencode(struct repository *r,\n+\t\t\t\t const struct commit *commit,\n+\t\t\t\t char **commit_encoding,\n+\t\t\t\t const char *output_encoding);\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define logmsg_reencode(c, enc, out) repo_logmsg_reencode(the_repository, c, enc, out)\n+#endif\n+\n extern const char *skip_blank_lines(const char *msg);\n \n /** Removes the first commit from a list sorted by date, and adds all\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex 516f19ffee..f5b42cfc62 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -123,3 +123,12 @@ expression F;\n - unuse_commit_buffer(\n + repo_unuse_commit_buffer(the_repository,\n E, F);\n+\n+@@\n+expression E;\n+expression F;\n+expression G;\n+@@\n+- logmsg_reencode(\n++ repo_logmsg_reencode(the_repository,\n+ E, F, G);\ndiff --git a/pretty.c b/pretty.c\nindex 8ca29e9281..b359b68750 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -595,14 +595,15 @@ static char *replace_encoding_header(char *buf, const char *encoding)\n \treturn strbuf_detach(&tmp, NULL);\n }\n \n-const char *logmsg_reencode(const struct commit *commit,\n-\t\t\t char **commit_encoding,\n-\t\t\t const char *output_encoding)\n+const char *repo_logmsg_reencode(struct repository *r,\n+\t\t\t\t const struct commit *commit,\n+\t\t\t\t char **commit_encoding,\n+\t\t\t\t const char *output_encoding)\n {\n \tstatic const char *utf8 = \"UTF-8\";\n \tconst char *use_encoding;\n \tchar *encoding;\n-\tconst char *msg = get_commit_buffer(commit, NULL);\n+\tconst char *msg = repo_get_commit_buffer(r, commit, NULL);\n \tchar *out;\n \n \tif (!output_encoding || !*output_encoding) {\n@@ -630,7 +631,7 @@ const char *logmsg_reencode(const struct commit *commit,\n \t\t * the cached copy from get_commit_buffer, we need to duplicate it\n \t\t * to avoid munging the cached copy.\n \t\t */\n-\t\tif (msg == get_cached_commit_buffer(the_repository, commit, NULL))\n+\t\tif (msg == get_cached_commit_buffer(r, commit, NULL))\n \t\t\tout = xstrdup(msg);\n \t\telse\n \t\t\tout = (char *)msg;\n@@ -644,7 +645,7 @@ const char *logmsg_reencode(const struct commit *commit,\n \t\t */\n \t\tout = reencode_string(msg, output_encoding, use_encoding);\n \t\tif (out)\n-\t\t\tunuse_commit_buffer(commit, msg);\n+\t\t\trepo_unuse_commit_buffer(r, commit, msg);\n \t}\n \n \t/*\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362033","messageId":"20181030220817.61691-19-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 18/24] pretty: prepare format_commit_message to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:11Z","receivedAt":"2018-10-30T22:09:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n contrib/coccinelle/the_repository.pending.cocci | 10 ++++++++++\n pretty.c | 15 ++++++++-------\n pretty.h | 7 ++++++-\n 3 files changed, 24 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\nindex f5b42cfc62..2ee702ecf7 100644\n--- a/contrib/coccinelle/the_repository.pending.cocci\n+++ b/contrib/coccinelle/the_repository.pending.cocci\n@@ -132,3 +132,13 @@ expression G;\n - logmsg_reencode(\n + repo_logmsg_reencode(the_repository,\n E, F, G);\n+\n+@@\n+expression E;\n+expression F;\n+expression G;\n+expression H;\n+@@\n+- format_commit_message(\n++ repo_format_commit_message(the_repository,\n+ E, F, G, H);\ndiff --git a/pretty.c b/pretty.c\nindex b359b68750..3240495308 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1508,9 +1508,10 @@ void userformat_find_requirements(const char *fmt, struct userformat_want *w)\n \tstrbuf_release(&dummy);\n }\n \n-void format_commit_message(const struct commit *commit,\n-\t\t\t const char *format, struct strbuf *sb,\n-\t\t\t const struct pretty_print_context *pretty_ctx)\n+void repo_format_commit_message(struct repository *r,\n+\t\t\t\tconst struct commit *commit,\n+\t\t\t\tconst char *format, struct strbuf *sb,\n+\t\t\t\tconst struct pretty_print_context *pretty_ctx)\n {\n \tstruct format_commit_context context;\n \tconst char *output_enc = pretty_ctx->output_encoding;\n@@ -1524,9 +1525,9 @@ void format_commit_message(const struct commit *commit,\n \t * convert a commit message to UTF-8 first\n \t * as far as 'format_commit_item' assumes it in UTF-8\n \t */\n-\tcontext.message = logmsg_reencode(commit,\n-\t\t\t\t\t &context.commit_encoding,\n-\t\t\t\t\t utf8);\n+\tcontext.message = repo_logmsg_reencode(r, commit,\n+\t\t\t\t\t &context.commit_encoding,\n+\t\t\t\t\t utf8);\n \n \tstrbuf_expand(sb, format, format_commit_item, &context);\n \trewrap_message_tail(sb, &context, 0, 0, 0);\n@@ -1550,7 +1551,7 @@ void format_commit_message(const struct commit *commit,\n \t}\n \n \tfree(context.commit_encoding);\n-\tunuse_commit_buffer(commit, context.message);\n+\trepo_unuse_commit_buffer(r, commit, context.message);\n }\n \n static void pp_header(struct pretty_print_context *pp,\ndiff --git a/pretty.h b/pretty.h\nindex 7359d318a9..e6625269cf 100644\n--- a/pretty.h\n+++ b/pretty.h\n@@ -103,9 +103,14 @@ void pp_remainder(struct pretty_print_context *pp, const char **msg_p,\n * Put the result to \"sb\".\n * Please use this function for custom formats.\n */\n-void format_commit_message(const struct commit *commit,\n+void repo_format_commit_message(struct repository *r,\n+\t\t\tconst struct commit *commit,\n \t\t\tconst char *format, struct strbuf *sb,\n \t\t\tconst struct pretty_print_context *context);\n+#ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS\n+#define format_commit_message(c, f, s, con) \\\n+\trepo_format_commit_message(the_repository, c, f, s, con)\n+#endif\n \n /*\n * Parse given arguments from \"arg\", check it for correctness and\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362034","messageId":"20181030220817.61691-20-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 19/24] submodule: use submodule repos for object lookup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:12Z","receivedAt":"2018-10-30T22:09:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This converts the 'show_submodule_header' function to use\nthe repository API properly, such that the submodule objects\nare not added to the main object store.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 76 ++++++++++++++++++++++++++++++++++++++++++-----------\n 1 file changed, 61 insertions(+), 15 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex d9d3046689..0fdda45252 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -443,7 +443,7 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n \treturn prepare_revision_walk(rev);\n }\n \n-static void print_submodule_summary(struct rev_info *rev, struct diff_options *o)\n+static void print_submodule_summary(struct repository *r, struct rev_info *rev, struct diff_options *o)\n {\n \tstatic const char format[] = \" %m %s\";\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -454,7 +454,8 @@ static void print_submodule_summary(struct rev_info *rev, struct diff_options *o\n \t\tctx.date_mode = rev->date_mode;\n \t\tctx.output_encoding = get_log_output_encoding();\n \t\tstrbuf_setlen(&sb, 0);\n-\t\tformat_commit_message(commit, format, &sb, &ctx);\n+\t\trepo_format_commit_message(r, commit, format, &sb,\n+\t\t\t\t &ctx);\n \t\tstrbuf_addch(&sb, '\\n');\n \t\tif (commit->object.flags & SYMMETRIC_LEFT)\n \t\t\tdiff_emit_submodule_del(o, sb.buf);\n@@ -481,14 +482,47 @@ void prepare_submodule_repo_env(struct argv_array *out)\n \t\t\t DEFAULT_GIT_DIR_ENVIRONMENT);\n }\n \n-/* Helper function to display the submodule header line prior to the full\n- * summary output. If it can locate the submodule objects directory it will\n- * attempt to lookup both the left and right commits and put them into the\n- * left and right pointers.\n+/*\n+ * Initialize 'out' based on the provided submodule path.\n+ *\n+ * Unlike repo_submodule_init, this tolerates submodules not present\n+ * in .gitmodules. This function exists only to preserve historical behavior,\n+ *\n+ * Returns 0 on success, -1 when the submodule is not present.\n */\n-static void show_submodule_header(struct diff_options *o, const char *path,\n+static struct repository *open_submodule(const char *path)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct repository *out = xmalloc(sizeof(*out));\n+\n+\tif (submodule_to_gitdir(&sb, path) || repo_init(out, sb.buf, NULL)) {\n+\t\tstrbuf_release(&sb);\n+\t\tfree(out);\n+\t\treturn NULL;\n+\t}\n+\n+\tout->submodule_prefix = xstrfmt(\"%s%s/\",\n+\t\t\t\t\tthe_repository->submodule_prefix ?\n+\t\t\t\t\tthe_repository->submodule_prefix :\n+\t\t\t\t\t\"\", path);\n+\n+\tstrbuf_release(&sb);\n+\treturn out;\n+}\n+\n+/*\n+ * Helper function to display the submodule header line prior to the full\n+ * summary output.\n+ *\n+ * If it can locate the submodule git directory it will create a repository\n+ * handle for the submodule and lookup both the left and right commits and\n+ * put them into the left and right pointers.\n+ */\n+static void show_submodule_header(struct diff_options *o,\n+\t\tconst char *path,\n \t\tstruct object_id *one, struct object_id *two,\n \t\tunsigned dirty_submodule,\n+\t\tstruct repository *sub,\n \t\tstruct commit **left, struct commit **right,\n \t\tstruct commit_list **merge_bases)\n {\n@@ -507,7 +541,7 @@ static void show_submodule_header(struct diff_options *o, const char *path,\n \telse if (is_null_oid(two))\n \t\tmessage = \"(submodule deleted)\";\n \n-\tif (add_submodule_odb(path)) {\n+\tif (!sub) {\n \t\tif (!message)\n \t\t\tmessage = \"(commits not present)\";\n \t\tgoto output_header;\n@@ -517,8 +551,8 @@ static void show_submodule_header(struct diff_options *o, const char *path,\n \t * Attempt to lookup the commit references, and determine if this is\n \t * a fast forward or fast backwards update.\n \t */\n-\t*left = lookup_commit_reference(the_repository, one);\n-\t*right = lookup_commit_reference(the_repository, two);\n+\t*left = lookup_commit_reference(sub, one);\n+\t*right = lookup_commit_reference(sub, two);\n \n \t/*\n \t * Warn about missing commits in the submodule project, but only if\n@@ -528,7 +562,7 @@ static void show_submodule_header(struct diff_options *o, const char *path,\n \t (!is_null_oid(two) && !*right))\n \t\tmessage = \"(commits not present)\";\n \n-\t*merge_bases = get_merge_bases(*left, *right);\n+\t*merge_bases = repo_get_merge_bases(sub, *left, *right);\n \tif (*merge_bases) {\n \t\tif ((*merge_bases)->item == *left)\n \t\t\tfast_forward = 1;\n@@ -562,16 +596,18 @@ void show_submodule_summary(struct diff_options *o, const char *path,\n \tstruct rev_info rev;\n \tstruct commit *left = NULL, *right = NULL;\n \tstruct commit_list *merge_bases = NULL;\n+\tstruct repository *sub;\n \n+\tsub = open_submodule(path);\n \tshow_submodule_header(o, path, one, two, dirty_submodule,\n-\t\t\t &left, &right, &merge_bases);\n+\t\t\t sub, &left, &right, &merge_bases);\n \n \t/*\n \t * If we don't have both a left and a right pointer, there is no\n \t * reason to try and display a summary. The header line should contain\n \t * all the information the user needs.\n \t */\n-\tif (!left || !right)\n+\tif (!left || !right || !sub)\n \t\tgoto out;\n \n \t/* Treat revision walker failure the same as missing commits */\n@@ -580,13 +616,17 @@ void show_submodule_summary(struct diff_options *o, const char *path,\n \t\tgoto out;\n \t}\n \n-\tprint_submodule_summary(&rev, o);\n+\tprint_submodule_summary(sub, &rev, o);\n \n out:\n \tif (merge_bases)\n \t\tfree_commit_list(merge_bases);\n \tclear_commit_marks(left, ~0);\n \tclear_commit_marks(right, ~0);\n+\tif (sub) {\n+\t\trepo_clear(sub);\n+\t\tfree(sub);\n+\t}\n }\n \n void show_submodule_inline_diff(struct diff_options *o, const char *path,\n@@ -598,9 +638,11 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,\n \tstruct commit_list *merge_bases = NULL;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n+\tstruct repository *sub;\n \n+\tsub = open_submodule(path);\n \tshow_submodule_header(o, path, one, two, dirty_submodule,\n-\t\t\t &left, &right, &merge_bases);\n+\t\t\t sub, &left, &right, &merge_bases);\n \n \t/* We need a valid left and right commit to display a difference */\n \tif (!(left || is_null_oid(one)) ||\n@@ -661,6 +703,10 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,\n \t\tclear_commit_marks(left, ~0);\n \tif (right)\n \t\tclear_commit_marks(right, ~0);\n+\tif (sub) {\n+\t\trepo_clear(sub);\n+\t\tfree(sub);\n+\t}\n }\n \n int should_update_submodules(void)\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362035","messageId":"20181030220817.61691-21-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 20/24] submodule: don't add submodule as odb for push","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:13Z","receivedAt":"2018-10-30T22:09:23Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In push_submodule(), because we do not actually need access to objects\nin the submodule, do not invoke add_submodule_odb().\n(for_each_remote_ref_submodule() does not require access to those\nobjects, and the actual push is done by spawning another process,\nwhich handles object access by itself.)\n\nThis code of push_submodule() is exercised in t5531 and continues\nto work, showing that the submodule odbc is not needed.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 0fdda45252..da1ac1e6f8 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1024,9 +1024,6 @@ static int push_submodule(const char *path,\n \t\t\t const struct string_list *push_options,\n \t\t\t int dry_run)\n {\n-\tif (add_submodule_odb(path))\n-\t\treturn 1;\n-\n \tif (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {\n \t\tstruct child_process cp = CHILD_PROCESS_INIT;\n \t\targv_array_push(&cp.args, \"push\");\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362036","messageId":"20181030220817.61691-22-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 21/24] commit-graph: convert remaining function to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:14Z","receivedAt":"2018-10-30T22:09:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Convert all functions to handle arbitrary repositories in commit-graph.c\nthat are used by functions taking a repository argument already.\n\nNotable exclusion is write_commit_graph and its local functions as that\nonly works on the_repository.\n\nSigned-off-by: Stefan Beller \n---\n commit-graph.c | 40 ++++++++++++++++++++++++----------------\n 1 file changed, 24 insertions(+), 16 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 40c855f185..f78a8e96b5 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -292,7 +292,8 @@ static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t\n \t\t\t g->chunk_oid_lookup, g->hash_len, pos);\n }\n \n-static struct commit_list **insert_parent_or_die(struct commit_graph *g,\n+static struct commit_list **insert_parent_or_die(struct repository *r,\n+\t\t\t\t\t\t struct commit_graph *g,\n \t\t\t\t\t\t uint64_t pos,\n \t\t\t\t\t\t struct commit_list **pptr)\n {\n@@ -303,7 +304,7 @@ static struct commit_list **insert_parent_or_die(struct commit_graph *g,\n \t\tdie(\"invalid parent position %\"PRIu64, pos);\n \n \thashcpy(oid.hash, g->chunk_oid_lookup + g->hash_len * pos);\n-\tc = lookup_commit(the_repository, &oid);\n+\tc = lookup_commit(r, &oid);\n \tif (!c)\n \t\tdie(_(\"could not find commit %s\"), oid_to_hex(&oid));\n \tc->graph_pos = pos;\n@@ -317,7 +318,9 @@ static void fill_commit_graph_info(struct commit *item, struct commit_graph *g,\n \titem->generation = get_be32(commit_data + g->hash_len + 8) >> 2;\n }\n \n-static int fill_commit_in_graph(struct commit *item, struct commit_graph *g, uint32_t pos)\n+static int fill_commit_in_graph(struct repository *r,\n+\t\t\t\tstruct commit *item,\n+\t\t\t\tstruct commit_graph *g, uint32_t pos)\n {\n \tuint32_t edge_value;\n \tuint32_t *parent_data_ptr;\n@@ -341,13 +344,13 @@ static int fill_commit_in_graph(struct commit *item, struct commit_graph *g, uin\n \tedge_value = get_be32(commit_data + g->hash_len);\n \tif (edge_value == GRAPH_PARENT_NONE)\n \t\treturn 1;\n-\tpptr = insert_parent_or_die(g, edge_value, pptr);\n+\tpptr = insert_parent_or_die(r, g, edge_value, pptr);\n \n \tedge_value = get_be32(commit_data + g->hash_len + 4);\n \tif (edge_value == GRAPH_PARENT_NONE)\n \t\treturn 1;\n \tif (!(edge_value & GRAPH_OCTOPUS_EDGES_NEEDED)) {\n-\t\tpptr = insert_parent_or_die(g, edge_value, pptr);\n+\t\tpptr = insert_parent_or_die(r, g, edge_value, pptr);\n \t\treturn 1;\n \t}\n \n@@ -355,7 +358,7 @@ static int fill_commit_in_graph(struct commit *item, struct commit_graph *g, uin\n \t\t\t 4 * (uint64_t)(edge_value & GRAPH_EDGE_LAST_MASK));\n \tdo {\n \t\tedge_value = get_be32(parent_data_ptr);\n-\t\tpptr = insert_parent_or_die(g,\n+\t\tpptr = insert_parent_or_die(r, g,\n \t\t\t\t\t edge_value & GRAPH_EDGE_LAST_MASK,\n \t\t\t\t\t pptr);\n \t\tparent_data_ptr++;\n@@ -374,7 +377,9 @@ static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uin\n \t}\n }\n \n-static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item)\n+static int parse_commit_in_graph_one(struct repository *r,\n+\t\t\t\t struct commit_graph *g,\n+\t\t\t\t struct commit *item)\n {\n \tuint32_t pos;\n \n@@ -382,7 +387,7 @@ static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item\n \t\treturn 1;\n \n \tif (find_commit_in_graph(item, g, &pos))\n-\t\treturn fill_commit_in_graph(item, g, pos);\n+\t\treturn fill_commit_in_graph(r, item, g, pos);\n \n \treturn 0;\n }\n@@ -391,7 +396,7 @@ int parse_commit_in_graph(struct repository *r, struct commit *item)\n {\n \tif (!prepare_commit_graph(r))\n \t\treturn 0;\n-\treturn parse_commit_in_graph_one(r->objects->commit_graph, item);\n+\treturn parse_commit_in_graph_one(r, r->objects->commit_graph, item);\n }\n \n void load_commit_graph_info(struct repository *r, struct commit *item)\n@@ -403,19 +408,22 @@ void load_commit_graph_info(struct repository *r, struct commit *item)\n \t\tfill_commit_graph_info(item, r->objects->commit_graph, pos);\n }\n \n-static struct tree *load_tree_for_commit(struct commit_graph *g, struct commit *c)\n+static struct tree *load_tree_for_commit(struct repository *r,\n+\t\t\t\t\t struct commit_graph *g,\n+\t\t\t\t\t struct commit *c)\n {\n \tstruct object_id oid;\n \tconst unsigned char *commit_data = g->chunk_commit_data +\n \t\t\t\t\t GRAPH_DATA_WIDTH * (c->graph_pos);\n \n \thashcpy(oid.hash, commit_data);\n-\tc->maybe_tree = lookup_tree(the_repository, &oid);\n+\tc->maybe_tree = lookup_tree(r, &oid);\n \n \treturn c->maybe_tree;\n }\n \n-static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g,\n+static struct tree *get_commit_tree_in_graph_one(struct repository *r,\n+\t\t\t\t\t\t struct commit_graph *g,\n \t\t\t\t\t\t const struct commit *c)\n {\n \tif (c->maybe_tree)\n@@ -423,12 +431,12 @@ static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g,\n \tif (c->graph_pos == COMMIT_NOT_FROM_GRAPH)\n \t\tBUG(\"get_commit_tree_in_graph_one called from non-commit-graph commit\");\n \n-\treturn load_tree_for_commit(g, (struct commit *)c);\n+\treturn load_tree_for_commit(r, g, (struct commit *)c);\n }\n \n struct tree *get_commit_tree_in_graph(struct repository *r, const struct commit *c)\n {\n-\treturn get_commit_tree_in_graph_one(r->objects->commit_graph, c);\n+\treturn get_commit_tree_in_graph_one(r, r->objects->commit_graph, c);\n }\n \n static void write_graph_chunk_fanout(struct hashfile *f,\n@@ -1025,7 +1033,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)\n \t\t}\n \n \t\tgraph_commit = lookup_commit(r, &cur_oid);\n-\t\tif (!parse_commit_in_graph_one(g, graph_commit))\n+\t\tif (!parse_commit_in_graph_one(r, g, graph_commit))\n \t\t\tgraph_report(\"failed to parse %s from commit-graph\",\n \t\t\t\t oid_to_hex(&cur_oid));\n \t}\n@@ -1061,7 +1069,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (!oideq(&get_commit_tree_in_graph_one(g, graph_commit)->object.oid,\n+\t\tif (!oideq(&get_commit_tree_in_graph_one(r, g, graph_commit)->object.oid,\n \t\t\t get_commit_tree_oid(odb_commit)))\n \t\t\tgraph_report(\"root tree OID for commit %s in commit-graph is %s != %s\",\n \t\t\t\t oid_to_hex(&cur_oid),\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362037","messageId":"20181030220817.61691-23-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 22/24] commit: make free_commit_buffer and release_commit_memory repository agnostic","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:15Z","receivedAt":"2018-10-30T22:09:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Pass the object pool to free_commit_buffer and release_commit_memory,\nsuch that we can eliminate access to 'the_repository'.\n\nAlso remove the TODO in release_commit_memory, as commit->util was\nremoved in 9d2c97016f (commit.h: delete 'util' field in struct commit,\n2018-05-19)\n\nSigned-off-by: Stefan Beller \n---\n builtin/fsck.c | 3 ++-\n builtin/log.c | 6 ++++--\n builtin/rev-list.c | 3 ++-\n commit.c | 9 ++++-----\n commit.h | 4 ++--\n object.c | 2 +-\n 6 files changed, 15 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 06eb421720..c476ac6983 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -382,7 +382,8 @@ static int fsck_obj(struct object *obj, void *buffer, unsigned long size)\n \tif (obj->type == OBJ_TREE)\n \t\tfree_tree_buffer((struct tree *)obj);\n \tif (obj->type == OBJ_COMMIT)\n-\t\tfree_commit_buffer((struct commit *)obj);\n+\t\tfree_commit_buffer(the_repository->parsed_objects,\n+\t\t\t\t (struct commit *)obj);\n \treturn err;\n }\n \ndiff --git a/builtin/log.c b/builtin/log.c\nindex 061d4fd864..64c2649c7c 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -395,7 +395,8 @@ static int cmd_log_walk(struct rev_info *rev)\n \t\t\t * We may show a given commit multiple times when\n \t\t\t * walking the reflogs.\n \t\t\t */\n-\t\t\tfree_commit_buffer(commit);\n+\t\t\tfree_commit_buffer(the_repository->parsed_objects,\n+\t\t\t\t\t commit);\n \t\t\tfree_commit_list(commit->parents);\n \t\t\tcommit->parents = NULL;\n \t\t}\n@@ -1922,7 +1923,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))\n \t\t\tdie(_(\"Failed to create output files\"));\n \t\tshown = log_tree_commit(&rev, commit);\n-\t\tfree_commit_buffer(commit);\n+\t\tfree_commit_buffer(the_repository->parsed_objects,\n+\t\t\t\t commit);\n \n \t\t/* We put one extra blank line between formatted\n \t\t * patches and this flag is used by log-tree code\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 5064d08e1b..a8867f0c4b 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -197,7 +197,8 @@ static void finish_commit(struct commit *commit, void *data)\n \t\tfree_commit_list(commit->parents);\n \t\tcommit->parents = NULL;\n \t}\n-\tfree_commit_buffer(commit);\n+\tfree_commit_buffer(the_repository->parsed_objects,\n+\t\t\t commit);\n }\n \n static inline void finish_object__ma(struct object *obj)\ndiff --git a/commit.c b/commit.c\nindex 7d2f3a9a93..4fe74aa4bc 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -328,10 +328,10 @@ void repo_unuse_commit_buffer(struct repository *r,\n \t\tfree((void *)buffer);\n }\n \n-void free_commit_buffer(struct commit *commit)\n+void free_commit_buffer(struct parsed_object_pool *pool, struct commit *commit)\n {\n \tstruct commit_buffer *v = buffer_slab_peek(\n-\t\tthe_repository->parsed_objects->buffer_slab, commit);\n+\t\tpool->buffer_slab, commit);\n \tif (v) {\n \t\tFREE_AND_NULL(v->buffer);\n \t\tv->size = 0;\n@@ -354,13 +354,12 @@ struct object_id *get_commit_tree_oid(const struct commit *commit)\n \treturn &get_commit_tree(commit)->object.oid;\n }\n \n-void release_commit_memory(struct commit *c)\n+void release_commit_memory(struct parsed_object_pool *pool, struct commit *c)\n {\n \tc->maybe_tree = NULL;\n \tc->index = 0;\n-\tfree_commit_buffer(c);\n+\tfree_commit_buffer(pool, c);\n \tfree_commit_list(c->parents);\n-\t/* TODO: what about commit->util? */\n \n \tc->object.parsed = 0;\n }\ndiff --git a/commit.h b/commit.h\nindex 2e6b799b26..d2779a23f6 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -140,7 +140,7 @@ void repo_unuse_commit_buffer(struct repository *r,\n /*\n * Free any cached object buffer associated with the commit.\n */\n-void free_commit_buffer(struct commit *);\n+void free_commit_buffer(struct parsed_object_pool *pool, struct commit *);\n \n struct tree *get_commit_tree(const struct commit *);\n struct object_id *get_commit_tree_oid(const struct commit *);\n@@ -149,7 +149,7 @@ struct object_id *get_commit_tree_oid(const struct commit *);\n * Release memory related to a commit, including the parent list and\n * any cached object buffer.\n */\n-void release_commit_memory(struct commit *c);\n+void release_commit_memory(struct parsed_object_pool *pool, struct commit *c);\n \n /*\n * Disassociate any cached object buffer from the commit, but do not free it.\ndiff --git a/object.c b/object.c\nindex 003f870484..c4170d2d0c 100644\n--- a/object.c\n+++ b/object.c\n@@ -540,7 +540,7 @@ void parsed_object_pool_clear(struct parsed_object_pool *o)\n \t\tif (obj->type == OBJ_TREE)\n \t\t\tfree_tree_buffer((struct tree*)obj);\n \t\telse if (obj->type == OBJ_COMMIT)\n-\t\t\trelease_commit_memory((struct commit*)obj);\n+\t\t\trelease_commit_memory(o, (struct commit*)obj);\n \t\telse if (obj->type == OBJ_TAG)\n \t\t\trelease_tag_memory((struct tag*)obj);\n \t}\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362038","messageId":"20181030220817.61691-24-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 23/24] path.h: make REPO_GIT_PATH_FUNC repository agnostic","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:16Z","receivedAt":"2018-10-30T22:09:31Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"git_pathdup uses the_repository internally, but the macro\nREPO_GIT_PATH_FUNC is specifically made for arbitrary repositories.\nSwitch to repo_git_path which works on arbitrary repositories.\n\nSigned-off-by: Stefan Beller \n---\n path.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/path.h b/path.h\nindex b654ea8ff5..651e6157fc 100644\n--- a/path.h\n+++ b/path.h\n@@ -165,7 +165,7 @@ extern void report_linked_checkout_garbage(void);\n \tconst char *git_path_##var(struct repository *r) \\\n \t{ \\\n \t\tif (!r->cached_paths.var) \\\n-\t\t\tr->cached_paths.var = git_pathdup(filename); \\\n+\t\t\tr->cached_paths.var = repo_git_path(r, filename); \\\n \t\treturn r->cached_paths.var; \\\n \t}\n \n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362039","messageId":"20181030220817.61691-25-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"[PATCH 24/24] t/helper/test-repository: celebrate independence from the_repository","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-10-30T22:08:17Z","receivedAt":"2018-10-30T22:09:34Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"dade47c06c (commit-graph: add repo arg to graph readers, 2018-07-11)\nbrought more independence from the_repository to the commit graph, however\nit was not completely independent of the_repository, as the previous\npatches show.\n\nTo ensure we're not accessing the_repository by accident, we'd ideally\nassign NULL to the_repository to trigger a segfault on access.\n\nWe currently have a temporary hack in cache.h, which relies on\nthe_hash_algo (which is a short form of the_repository->hash_algo) to\nbe set, so we cannot do that. The next best thing is to set all fields of\nthe_repository to 0, so any accidental access is more likely to be found.\n\nSigned-off-by: Stefan Beller \n---\n cache.h | 2 ++\n t/helper/test-repository.c | 10 ++++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex f7fabdde8f..9f535040af 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1035,6 +1035,8 @@ static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)\n \t *\n \t * This will need to be extended or ripped out when we learn about\n \t * hashes of different sizes.\n+\t *\n+\t * When ripping this out, see TODO in test-repository.c.\n \t */\n \tif (the_hash_algo->rawsz != 20)\n \t\tBUG(\"hash size not yet supported by hashcmp\");\ndiff --git a/t/helper/test-repository.c b/t/helper/test-repository.c\nindex 6a84a53efb..f7f8618445 100644\n--- a/t/helper/test-repository.c\n+++ b/t/helper/test-repository.c\n@@ -17,6 +17,11 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,\n \n \tsetup_git_env(gitdir);\n \n+\tmemset(the_repository, 0, sizeof(*the_repository));\n+\n+\t/* TODO: Needed for temporary hack in hashcmp, see 183a638b7da. */\n+\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\n \n@@ -43,6 +48,11 @@ static void test_get_commit_tree_in_graph(const char *gitdir,\n \n \tsetup_git_env(gitdir);\n \n+\tmemset(the_repository, 0, sizeof(*the_repository));\n+\n+\t/* TODO: Needed for temporary hack in hashcmp, see 183a638b7da. */\n+\trepo_set_hash_algo(the_repository, GIT_HASH_SHA1);\n+\n \tif (repo_init(&r, gitdir, worktree))\n \t\tdie(\"Couldn't init repo\");\n \n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362079","messageId":"xmqq4ld2wrcq.fsf@gitster-ct.c.googlers.com","threadId":"49720","inReplyTo":"20181030220817.61691-1-sbeller@google.com","subject":"Re: [PATCHv2 00/24] Bring more repository handles into our code base]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-31T06:41:09Z","receivedAt":"2018-10-31T06:41:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> I also picked up the patch for pending semantic patches, as the\n> first patch, can I have your sign off, please?\n\nI find this step quite lacking. \n\nWhat was posted would have been perfectly fine as a \"how about doing\nit this way\" weatherbaloon patch, but as a part of real series, it\nneeds to explain to the developers what the distinctions between two\nclasses are, and who is to use the cocci patch at what point in the\ndevelopment cycle, etc., in an accompanying documentation update.\n\nSo can we have both sign-off and write-up (perhaps cut&paste from\nthe original e-mail discussion)?\n\n> Stefan Beller (23):\n> sha1_file: allow read_object to read objects in arbitrary repositories\n> packfile: allow has_packed_and_bad to handle arbitrary repositories\n> object-store: allow read_object_file_extended to read from arbitrary\n> repositories\n> object-store: prepare read_object_file to deal with arbitrary\n> repositories\n> object-store: prepare has_{sha1, object}_file[_with_flags] to handle\n> arbitrary repositories\n> object: parse_object to honor its repository argument\n> commit: allow parse_commit* to handle arbitrary repositories\n> commit-reach.c: allow paint_down_to_common to handle arbitrary\n> repositories\n> commit-reach.c: allow merge_bases_many to handle arbitrary\n> repositories\n> commit-reach.c: allow remove_redundant to handle arbitrary\n> repositories\n> commit-reach.c: allow get_merge_bases_many_0 to handle arbitrary\n> repositories\n> commit-reach: prepare get_merge_bases to handle arbitrary repositories\n> commit-reach: prepare in_merge_bases[_many] to handle arbitrary\n> repositories\n> commit: prepare get_commit_buffer to handle arbitrary repositories\n> commit: prepare repo_unuse_commit_buffer to handle arbitrary\n> repositories\n> commit: prepare logmsg_reencode to handle arbitrary repositories\n> pretty: prepare format_commit_message to handle arbitrary repositories\n> submodule: use submodule repos for object lookup\n> submodule: don't add submodule as odb for push\n> commit-graph: convert remaining function to handle arbitrary\n> repositories\n> commit: make free_commit_buffer and release_commit_memory repository\n> agnostic\n> path.h: make REPO_GIT_PATH_FUNC repository agnostic\n> t/helper/test-repository: celebrate independence from the_repository\n\nIt seems that this topic is full of commits with overly long title.\n\n>\n> Makefile | 6 +-\n> builtin/fsck.c | 3 +-\n> builtin/log.c | 6 +-\n> builtin/rev-list.c | 3 +-\n> cache.h | 2 +\n> commit-graph.c | 40 +++--\n> commit-reach.c | 73 +++++----\n> commit-reach.h | 38 +++--\n> commit.c | 41 ++---\n> commit.h | 43 +++++-\n> .../coccinelle/the_repository.pending.cocci | 144 ++++++++++++++++++\n> object-store.h | 35 ++++-\n> object.c | 8 +-\n> packfile.c | 5 +-\n> packfile.h | 2 +-\n> path.h | 2 +-\n> pretty.c | 28 ++--\n> pretty.h | 7 +-\n> sha1-file.c | 34 +++--\n> streaming.c | 2 +-\n> submodule.c | 79 +++++++---\n> t/helper/test-repository.c | 10 ++\n> 22 files changed, 459 insertions(+), 152 deletions(-)\n> create mode 100644 contrib/coccinelle/the_repository.pending.cocci\n>\n> git range-diff origin/sb/more-repo-in-api...\n> [...] # rebased to origin/master\n\nI offhand do not recall what happened during these 100+ pacthes.\nDId we acquire something significantly new that would have\nconflicted with this new round of the topic? I do not mind at all\nseeing that a series gets adjusted to the updated codebase, but I do\ndislike seeing it done without explanation---an unexplained rebase\nto a new base is a wasted opportunity to learn what areas of the\ncodebase are so hot that multiple and separate topics would want to\ntouch them.\n\n> -: ---------- > 116: 4ede3d42df Seventh batch for 2.20\n> -: ---------- > 117: b1de196491 Makefile: add pending semantic patches\n> 1: 1b9b5c695e = 118: f1be5eb487 sha1_file: allow read_object to read objects in arbitrary repositories\n> 2: 33b94066f2 = 119: 9100a5705d packfile: allow has_packed_and_bad to handle arbitrary repositories\n> 3: 5217b6b1e1 = 120: a4ad7791da object-store: allow read_object_file_extended to read from arbitrary repositories\n> 4: 2b7239b55b ! 121: 5d7b3a6dd9 object-store: prepare read_object_file to deal with arbitrary repositories\n> @@ -19,10 +19,10 @@\n> Signed-off-by: Stefan Beller \n> Signed-off-by: Junio C Hamano \n> \n> - diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci\n> + diff --git a/contrib/coccinelle/the_repository.pending.cocci b/contrib/coccinelle/the_repository.pending.cocci\n> new file mode 100644\n> --- /dev/null\n> - +++ b/contrib/coccinelle/the_repository.cocci\n> + +++ b/contrib/coccinelle/the_repository.pending.cocci\n> @@\n> +// This file is used for the ongoing refactoring of\n> +// bringing the index or repository struct in all of\n> @@ -36,6 +36,7 @@\n> +- read_object_file(\n> ++ repo_read_object_file(the_repository,\n> + E, F, G)\n> ++\n\nIs it necessary for this file to end with a trailing blank line?\n\n\n\nThanks.\n"},{"id":"362080","messageId":"xmqqzhuuvcpt.fsf@gitster-ct.c.googlers.com","threadId":"49720","inReplyTo":"20181030220817.61691-2-sbeller@google.com","subject":"Re: [PATCH 01/24] Makefile: add pending semantic patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-31T06:42:38Z","receivedAt":"2018-10-31T06:42:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> [Missing: SZEDERs sign off, so I also do not sign off]\n\nAt least to me, based on my reading of DCO in\nDocumentation/SubmittingPatches, this reasoning does not make much\nsense.\n"},{"id":"362210","messageId":"CAGZ79kacJLv7Eimgs82W_tEJR57o5HoDvmYfv4T2U9_gs0-Ujg@mail.gmail.com","threadId":"49720","inReplyTo":"xmqq4ld2wrcq.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCHv2 00/24] Bring more repository handles into our code base]","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-01T19:45:52Z","receivedAt":"2018-11-01T19:46:08Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 30, 2018 at 11:41 PM Junio C Hamano wrote:\n>\n> Stefan Beller writes:\n>\n> > I also picked up the patch for pending semantic patches, as the\n> > first patch, can I have your sign off, please?\n>\n> I find this step quite lacking.\n>\n> What was posted would have been perfectly fine as a \"how about doing\n> it this way\" weatherbaloon patch, but as a part of real series, it\n> needs to explain to the developers what the distinctions between two\n> classes are, and who is to use the cocci patch at what point in the\n> development cycle, etc., in an accompanying documentation update.\n\nif only we had documentation [as found via \"git grep coccicheck\"]\nthat I could update ... I'd totally do that.\nI guess this asks for documentation to begin with, now?\n\n> So can we have both sign-off and write-up (perhaps cut&paste from\n> the original e-mail discussion)?\n\nI'll see where to put the docs; I assumed commit messages are enough.\n63f0a758a0 (add coccicheck make target, 2016-09-15)\nis what I found nice.\n\n\n> > t/helper/test-repository: celebrate independence from the_repository\n>\n> It seems that this topic is full of commits with overly long title.\n\nyep.\n> > git range-diff origin/sb/more-repo-in-api...\n> > [...] # rebased to origin/master\n>\n> I offhand do not recall what happened during these 100+ pacthes.\n> DId we acquire something significantly new that would have\n> conflicted with this new round of the topic? I do not mind at all\n> seeing that a series gets adjusted to the updated codebase, but I do\n> dislike seeing it done without explanation---an unexplained rebase\n> to a new base is a wasted opportunity to learn what areas of the\n> codebase are so hot that multiple and separate topics would want to\n> touch them.\n\nFrom the point of view of these large scale refactorings,\nall of the code is hot, e.g. the patch that was present in the RFC\n\"apply all semantic patches\" would clash with nearly any topic.\n\nAs I do not carry that patch any more, I do not recall any conflicts\nthat needed to be resolved.\n\nThanks.\n"},{"id":"362226","messageId":"xmqqftwks0fe.fsf@gitster-ct.c.googlers.com","threadId":"49720","inReplyTo":"CAGZ79kacJLv7Eimgs82W_tEJR57o5HoDvmYfv4T2U9_gs0-Ujg@mail.gmail.com","subject":"Re: [PATCHv2 00/24] Bring more repository handles into our code base]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-02T02:00:53Z","receivedAt":"2018-11-02T02:01:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>> What was posted would have been perfectly fine as a \"how about doing\n>> it this way\" weatherbaloon patch, but as a part of real series, it\n>> needs to explain to the developers what the distinctions between two\n>> classes are, and who is to use the cocci patch at what point in the\n>> development cycle, etc., in an accompanying documentation update.\n>\n> if only we had documentation [as found via \"git grep coccicheck\"]\n> that I could update ... I'd totally do that.\n> I guess this asks for documentation to begin with, now?\n\nSo far, we didn't even need any, as there was no \"workflow\" to speak\nof. It's just \"any random developer finds a suggested update,\neither by running 'make coccicheck' oneself or by peeking Travis\nlog, and sends in a patch\".\n\nBut the \"pending\" thing has a lot more workflow elements, who is\nresponsible for noticing update suggested by \"pending\" ones, for\nupdating the code, and for promoting \"pending\" to the normal. These\nare all new, and these are all needed as ongoing basis to help\ndevelopers---I'd say the way Documentation/SubmittingPatches helps\ndevelopers is very close to the way this new document would help them.\n"},{"id":"362264","messageId":"127add9b-d58c-ee46-73de-41bfe915589b@gmail.com","threadId":"49720","inReplyTo":"20181030220817.61691-20-sbeller@google.com","subject":"Re: [PATCH 19/24] submodule: use submodule repos for object lookup","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-11-02T13:03:02Z","receivedAt":"2018-11-02T13:03:07Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 10/30/2018 6:08 PM, Stefan Beller wrote:\n> This converts the 'show_submodule_header' function to use\n> the repository API properly, such that the submodule objects\n> are not added to the main object store.\n>\n> Signed-off-by: Stefan Beller \n\nA couple tests are broken in 'pu' when run with GIT_TEST_COMMIT_GRAPH=1, \nincluding t4041-diff-submodule-option.sh. The failure bisects to this patch.\n\nHere is a verbose output of the first failure in that script:;\n\n\nexpecting success:\n         git diff-index -p --submodule=log HEAD >actual &&\n         cat >expected <<-EOF &&\n         Submodule sm1 $head2..$head3 (rewind):\n           < Add foo3 ($added foo3)\n           < Add foo2 ($added foo2)\n         EOF\n         test_cmp expected actual\n\n+ git diff-index -p --submodule=log HEAD\n+ cat\n+ test_cmp expected actual\n+ diff -u expected actual\n--- expected    2018-11-02 12:58:43.429262380 +0000\n+++ actual      2018-11-02 12:58:43.429262380 +0000\n@@ -1,3 +1,5 @@\n-Submodule sm1 30b9670..dafb207 (rewind):\n+Submodule sm1 30b9670...dafb207:\n    < Add foo3 (hinzugefügt foo3)\n    < Add foo2 (hinzugefügt foo2)\n+  > Add foo1 (hinzugefügt foo1)\n+  < Add foo1 (hinzugefügt foo1)\nerror: last command exited with $?=1\nnot ok 9 - modified submodule(backward)\n\nI've been looking into the patch below to see if there is an obvious \nproblem, but the best I can think is that open_submodule() creates an \nalternate 'struct repository' and somehow the commit-graph feature is \ninteracting poorly with that struct.\n\nStefan, do you know what's going on?\n\nThanks,\n\n-Stolee\n\n> ---\n> submodule.c | 76 ++++++++++++++++++++++++++++++++++++++++++-----------\n> 1 file changed, 61 insertions(+), 15 deletions(-)\n>\n> diff --git a/submodule.c b/submodule.c\n> index d9d3046689..0fdda45252 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -443,7 +443,7 @@ static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n> \treturn prepare_revision_walk(rev);\n> }\n> \n> -static void print_submodule_summary(struct rev_info *rev, struct diff_options *o)\n> +static void print_submodule_summary(struct repository *r, struct rev_info *rev, struct diff_options *o)\n> {\n> \tstatic const char format[] = \" %m %s\";\n> \tstruct strbuf sb = STRBUF_INIT;\n> @@ -454,7 +454,8 @@ static void print_submodule_summary(struct rev_info *rev, struct diff_options *o\n> \t\tctx.date_mode = rev->date_mode;\n> \t\tctx.output_encoding = get_log_output_encoding();\n> \t\tstrbuf_setlen(&sb, 0);\n> -\t\tformat_commit_message(commit, format, &sb, &ctx);\n> +\t\trepo_format_commit_message(r, commit, format, &sb,\n> +\t\t\t\t &ctx);\n> \t\tstrbuf_addch(&sb, '\\n');\n> \t\tif (commit->object.flags & SYMMETRIC_LEFT)\n> \t\t\tdiff_emit_submodule_del(o, sb.buf);\n> @@ -481,14 +482,47 @@ void prepare_submodule_repo_env(struct argv_array *out)\n> \t\t\t DEFAULT_GIT_DIR_ENVIRONMENT);\n> }\n> \n> -/* Helper function to display the submodule header line prior to the full\n> - * summary output. If it can locate the submodule objects directory it will\n> - * attempt to lookup both the left and right commits and put them into the\n> - * left and right pointers.\n> +/*\n> + * Initialize 'out' based on the provided submodule path.\n> + *\n> + * Unlike repo_submodule_init, this tolerates submodules not present\n> + * in .gitmodules. This function exists only to preserve historical behavior,\n> + *\n> + * Returns 0 on success, -1 when the submodule is not present.\n> */\n> -static void show_submodule_header(struct diff_options *o, const char *path,\n> +static struct repository *open_submodule(const char *path)\n> +{\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct repository *out = xmalloc(sizeof(*out));\n> +\n> +\tif (submodule_to_gitdir(&sb, path) || repo_init(out, sb.buf, NULL)) {\n> +\t\tstrbuf_release(&sb);\n> +\t\tfree(out);\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\tout->submodule_prefix = xstrfmt(\"%s%s/\",\n> +\t\t\t\t\tthe_repository->submodule_prefix ?\n> +\t\t\t\t\tthe_repository->submodule_prefix :\n> +\t\t\t\t\t\"\", path);\n> +\n> +\tstrbuf_release(&sb);\n> +\treturn out;\n> +}\n> +\n> +/*\n> + * Helper function to display the submodule header line prior to the full\n> + * summary output.\n> + *\n> + * If it can locate the submodule git directory it will create a repository\n> + * handle for the submodule and lookup both the left and right commits and\n> + * put them into the left and right pointers.\n> + */\n> +static void show_submodule_header(struct diff_options *o,\n> +\t\tconst char *path,\n> \t\tstruct object_id *one, struct object_id *two,\n> \t\tunsigned dirty_submodule,\n> +\t\tstruct repository *sub,\n> \t\tstruct commit **left, struct commit **right,\n> \t\tstruct commit_list **merge_bases)\n> {\n> @@ -507,7 +541,7 @@ static void show_submodule_header(struct diff_options *o, const char *path,\n> \telse if (is_null_oid(two))\n> \t\tmessage = \"(submodule deleted)\";\n> \n> -\tif (add_submodule_odb(path)) {\n> +\tif (!sub) {\n> \t\tif (!message)\n> \t\t\tmessage = \"(commits not present)\";\n> \t\tgoto output_header;\n> @@ -517,8 +551,8 @@ static void show_submodule_header(struct diff_options *o, const char *path,\n> \t * Attempt to lookup the commit references, and determine if this is\n> \t * a fast forward or fast backwards update.\n> \t */\n> -\t*left = lookup_commit_reference(the_repository, one);\n> -\t*right = lookup_commit_reference(the_repository, two);\n> +\t*left = lookup_commit_reference(sub, one);\n> +\t*right = lookup_commit_reference(sub, two);\n> \n> \t/*\n> \t * Warn about missing commits in the submodule project, but only if\n> @@ -528,7 +562,7 @@ static void show_submodule_header(struct diff_options *o, const char *path,\n> \t (!is_null_oid(two) && !*right))\n> \t\tmessage = \"(commits not present)\";\n> \n> -\t*merge_bases = get_merge_bases(*left, *right);\n> +\t*merge_bases = repo_get_merge_bases(sub, *left, *right);\n> \tif (*merge_bases) {\n> \t\tif ((*merge_bases)->item == *left)\n> \t\t\tfast_forward = 1;\n> @@ -562,16 +596,18 @@ void show_submodule_summary(struct diff_options *o, const char *path,\n> \tstruct rev_info rev;\n> \tstruct commit *left = NULL, *right = NULL;\n> \tstruct commit_list *merge_bases = NULL;\n> +\tstruct repository *sub;\n> \n> +\tsub = open_submodule(path);\n> \tshow_submodule_header(o, path, one, two, dirty_submodule,\n> -\t\t\t &left, &right, &merge_bases);\n> +\t\t\t sub, &left, &right, &merge_bases);\n> \n> \t/*\n> \t * If we don't have both a left and a right pointer, there is no\n> \t * reason to try and display a summary. The header line should contain\n> \t * all the information the user needs.\n> \t */\n> -\tif (!left || !right)\n> +\tif (!left || !right || !sub)\n> \t\tgoto out;\n> \n> \t/* Treat revision walker failure the same as missing commits */\n> @@ -580,13 +616,17 @@ void show_submodule_summary(struct diff_options *o, const char *path,\n> \t\tgoto out;\n> \t}\n> \n> -\tprint_submodule_summary(&rev, o);\n> +\tprint_submodule_summary(sub, &rev, o);\n> \n> out:\n> \tif (merge_bases)\n> \t\tfree_commit_list(merge_bases);\n> \tclear_commit_marks(left, ~0);\n> \tclear_commit_marks(right, ~0);\n> +\tif (sub) {\n> +\t\trepo_clear(sub);\n> +\t\tfree(sub);\n> +\t}\n> }\n> \n> void show_submodule_inline_diff(struct diff_options *o, const char *path,\n> @@ -598,9 +638,11 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,\n> \tstruct commit_list *merge_bases = NULL;\n> \tstruct child_process cp = CHILD_PROCESS_INIT;\n> \tstruct strbuf sb = STRBUF_INIT;\n> +\tstruct repository *sub;\n> \n> +\tsub = open_submodule(path);\n> \tshow_submodule_header(o, path, one, two, dirty_submodule,\n> -\t\t\t &left, &right, &merge_bases);\n> +\t\t\t sub, &left, &right, &merge_bases);\n> \n> \t/* We need a valid left and right commit to display a difference */\n> \tif (!(left || is_null_oid(one)) ||\n> @@ -661,6 +703,10 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,\n> \t\tclear_commit_marks(left, ~0);\n> \tif (right)\n> \t\tclear_commit_marks(right, ~0);\n> +\tif (sub) {\n> +\t\trepo_clear(sub);\n> +\t\tfree(sub);\n> +\t}\n> }\n> \n> int should_update_submodules(void)\n"},{"id":"362289","messageId":"CAGZ79kZV9AZLHiSz19jx_Z5PnqO1V+-ngL9ACGhr21umn6UQVQ@mail.gmail.com","threadId":"49720","inReplyTo":"127add9b-d58c-ee46-73de-41bfe915589b@gmail.com","subject":"Re: [PATCH 19/24] submodule: use submodule repos for object lookup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-02T17:23:44Z","receivedAt":"2018-11-02T17:23:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Nov 2, 2018 at 6:03 AM Derrick Stolee wrote:\n>\n> On 10/30/2018 6:08 PM, Stefan Beller wrote:\n> > This converts the 'show_submodule_header' function to use\n> > the repository API properly, such that the submodule objects\n> > are not added to the main object store.\n> >\n> > Signed-off-by: Stefan Beller \n>\n> A couple tests are broken in 'pu' when run with GIT_TEST_COMMIT_GRAPH=1,\n> including t4041-diff-submodule-option.sh. The failure bisects to this patch.\n>\n> Here is a verbose output of the first failure in that script:;\n>\n>\n> expecting success:\n> git diff-index -p --submodule=log HEAD >actual &&\n> cat >expected <<-EOF &&\n> Submodule sm1 $head2..$head3 (rewind):\n> < Add foo3 ($added foo3)\n> < Add foo2 ($added foo2)\n> EOF\n> test_cmp expected actual\n>\n> + git diff-index -p --submodule=log HEAD\n> + cat\n> + test_cmp expected actual\n> + diff -u expected actual\n> --- expected 2018-11-02 12:58:43.429262380 +0000\n> +++ actual 2018-11-02 12:58:43.429262380 +0000\n> @@ -1,3 +1,5 @@\n> -Submodule sm1 30b9670..dafb207 (rewind):\n> +Submodule sm1 30b9670...dafb207:\n> < Add foo3 (hinzugefügt foo3)\n> < Add foo2 (hinzugefügt foo2)\n> + > Add foo1 (hinzugefügt foo1)\n> + < Add foo1 (hinzugefügt foo1)\n> error: last command exited with $?=1\n> not ok 9 - modified submodule(backward)\n>\n> I've been looking into the patch below to see if there is an obvious\n> problem, but the best I can think is that open_submodule() creates an\n> alternate 'struct repository' and somehow the commit-graph feature is\n> interacting poorly with that struct.\n>\n> Stefan, do you know what's going on?\n\nSure, see the last four patches of this series\nhttps://public-inbox.org/git/20181030220817.61691-1-sbeller@google.com/\n(to which you also reply to? Junio did not queue this one, yet).\n"},{"id":"362290","messageId":"1db4a22e-466d-3d21-1899-7b2d32363114@gmail.com","threadId":"49720","inReplyTo":"CAGZ79kZV9AZLHiSz19jx_Z5PnqO1V+-ngL9ACGhr21umn6UQVQ@mail.gmail.com","subject":"Re: [PATCH 19/24] submodule: use submodule repos for object lookup","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-11-02T17:27:35Z","receivedAt":"2018-11-02T17:27:41Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/2/2018 1:23 PM, Stefan Beller wrote:\n> On Fri, Nov 2, 2018 at 6:03 AM Derrick Stolee wrote:\n>> On 10/30/2018 6:08 PM, Stefan Beller wrote:\n>>> This converts the 'show_submodule_header' function to use\n>>> the repository API properly, such that the submodule objects\n>>> are not added to the main object store.\n>>>\n>>> Signed-off-by: Stefan Beller \n>> A couple tests are broken in 'pu' when run with GIT_TEST_COMMIT_GRAPH=1,\n>> including t4041-diff-submodule-option.sh. The failure bisects to this patch.\n>>\n>> Here is a verbose output of the first failure in that script:;\n>>\n>>\n>> expecting success:\n>> git diff-index -p --submodule=log HEAD >actual &&\n>> cat >expected <<-EOF &&\n>> Submodule sm1 $head2..$head3 (rewind):\n>> < Add foo3 ($added foo3)\n>> < Add foo2 ($added foo2)\n>> EOF\n>> test_cmp expected actual\n>>\n>> + git diff-index -p --submodule=log HEAD\n>> + cat\n>> + test_cmp expected actual\n>> + diff -u expected actual\n>> --- expected 2018-11-02 12:58:43.429262380 +0000\n>> +++ actual 2018-11-02 12:58:43.429262380 +0000\n>> @@ -1,3 +1,5 @@\n>> -Submodule sm1 30b9670..dafb207 (rewind):\n>> +Submodule sm1 30b9670...dafb207:\n>> < Add foo3 (hinzugefügt foo3)\n>> < Add foo2 (hinzugefügt foo2)\n>> + > Add foo1 (hinzugefügt foo1)\n>> + < Add foo1 (hinzugefügt foo1)\n>> error: last command exited with $?=1\n>> not ok 9 - modified submodule(backward)\n>>\n>> I've been looking into the patch below to see if there is an obvious\n>> problem, but the best I can think is that open_submodule() creates an\n>> alternate 'struct repository' and somehow the commit-graph feature is\n>> interacting poorly with that struct.\n>>\n>> Stefan, do you know what's going on?\n> Sure, see the last four patches of this series\n> https://public-inbox.org/git/20181030220817.61691-1-sbeller@google.com/\n> (to which you also reply to? Junio did not queue this one, yet).\n\nSorry! Got a bit mixed up looking at everything. I didn't realize that \nthe current 'pu' didn't have your latest. Thanks!\n\n"},{"id":"362753","messageId":"20181108205255.193402-1-sbeller@google.com","threadId":"49720","inReplyTo":"20181030220817.61691-2-sbeller@google.com","subject":"[PATCH] Makefile: add pending semantic patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-08T20:52:55Z","receivedAt":"2018-11-08T20:53:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: SZEDER Gábor \n\nAdd a description and place on how to use coccinelle for large refactorings\nthat happen only once.\n\nBased-on-work-by: SZEDER Gábor \nSigned-off-by: Stefan Beller \n---\n\nI consider including this patch in a resend instead.\nIt outlays the basics of such a new workflow, which we can refine later.\n\nThanks,\nStefan\n\n Makefile | 7 +++--\n contrib/coccinelle/README | 60 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 65 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b08d5ea258..e5abfe4cef 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2739,9 +2739,12 @@ endif\n \tthen \\\n \t\techo ' ' SPATCH result: $@; \\\n \tfi\n-coccicheck: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.cocci))\n+coccicheck: $(addsuffix .patch,$(filter-out %.pending.cocci,$(wildcard contrib/coccinelle/*.cocci)))\n \n-.PHONY: coccicheck\n+# See contrib/coccinelle/README\n+coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.cocci))\n+\n+.PHONY: coccicheck coccicheck-pending\n \n ### Installation rules\n \ndiff --git a/contrib/coccinelle/README b/contrib/coccinelle/README\nindex 9c2f8879c2..fa09d1abcc 100644\n--- a/contrib/coccinelle/README\n+++ b/contrib/coccinelle/README\n@@ -1,2 +1,62 @@\n This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)\n semantic patches that might be useful to developers.\n+\n+There are two types of semantic patches:\n+\n+ * Using the semantic transformation to check for bad patterns in the code;\n+ This is what the original target 'make coccicheck' is designed to do and\n+ it is expected that any resulting patch indicates a regression.\n+ The patches resulting from 'make coccicheck' are small and infrequent,\n+ so once they are found, they can be sent to the mailing list as per usual.\n+\n+ Example for introducing new patterns:\n+ 67947c34ae (convert \"hashcmp() != 0\" to \"!hasheq()\", 2018-08-28)\n+ b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24)\n+\n+ Example of fixes using this approach:\n+ 248f66ed8e (run-command: use strbuf_addstr() for adding a string to\n+ a strbuf, 2018-03-25)\n+ f919ffebed (Use MOVE_ARRAY, 2018-01-22)\n+\n+ These types of semantic patches are usually part of testing, c.f.\n+ 0860a7641b (travis-ci: fail if Coccinelle static analysis found something\n+ to transform, 2018-07-23)\n+\n+ * Using semantic transformations in large scale refactorings throughout\n+ the code base.\n+\n+ When applying the semantic patch into a real patch, sending it to the\n+ mailing list in the usual way, such a patch would be expected to have a\n+ lot of textual and semantic conflicts as such large scale refactorings\n+ change function signatures that are used widely in the code base.\n+ A textual conflict would arise if surrounding code near any call of such\n+ function changes. A semantic conflict arises when other patch series in\n+ flight introduce calls to such functions.\n+\n+ So to aid these large scale refactorings, semantic patches can be used,\n+ using the process as follows:\n+\n+ 1) Figure out what kind of large scale refactoring we need\n+ -> This is usually done by another unrelated series.\n+ 2) Create the sematic patch needed for the large scale refactoring\n+ and store it in contrib/coccinelle/*.pending.cocci\n+ -> The suffix containing 'pending' is important to differentiate\n+ this case from the other use case of checking for bad patterns.\n+ 3) Apply the semantic patch only partially, where needed for the patch series\n+ that motivates the large scale refactoring and then build that series\n+ on top of it.\n+ By applying the semantic patch only partially where needed, the series\n+ is less likely to conflict with other series in flight.\n+ To make it possible to apply the semantic patch partially, there needs\n+ to be mechanism for backwards compatibility to keep those places working\n+ where the semantic patch is not applied. This can be done via a\n+ wrapper function that has the exact name and signature as the function\n+ to be changed.\n+\n+ 4) Send the series as usual, including only the needed parts of the\n+ large scale refactoring\n+\n+ Later steps (not necessarily by the original author) are to apply the\n+ semantic patch in a way that do not produce lots of conflicts, for example\n+ by consulting `git diff --numstat origin/master..origin/pu` and the changes\n+ of the semantic patch.\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362775","messageId":"CAN0heSrF7zU=5NGHD9AUcRZgGHoCmrbBwYqJ-6YUM4xg3r+8Rg@mail.gmail.com","threadId":"49720","inReplyTo":"20181108205255.193402-1-sbeller@google.com","subject":"Re: [PATCH] Makefile: add pending semantic patches","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-11-09T04:55:53Z","receivedAt":"2018-11-09T04:56:11Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Thu, 8 Nov 2018 at 21:53, Stefan Beller wrote:\n>\n> From: SZEDER Gábor \n>\n\nI haven't followed the original discussion too carefully, so I'll read\nthis like someone new to the topic probably would.\n\nA nit, perhaps, but I was genuinely confused at first. The subject is\n\"Makefile: add pending semantic patches\", but this patch doesn't add\nany. It adds Makefile-support for such patches though, and it defines\nthe entire concept of pending semantic patches. How about \"coccicheck:\nintroduce 'pending' semantic patches\"?\n\n> Add a description and place on how to use coccinelle for large refactorings\n> that happen only once.\n\nA bit confused about \"and place\". Based on my understanding from reading\nthe remainder of this patch, maybe:\n\n Teach `make coccicheck` to avoid patches named \"*.pending.cocci\" and\n handle them separately in a new `make coccicheck-pending` instead.\n This means that we can separate \"critical\" patches from \"FYI\" patches.\n The former target can continue causing Travis to fail its static\n analysis job, while the latter can let us keep an eye on ongoing\n (pending) transitions without them causing too much fallout.\n\n Document the intended use-cases and processes around these two\n targets.\n\n> This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)\n> semantic patches that might be useful to developers.\n> +\n> +There are two types of semantic patches:\n> +\n> + * Using the semantic transformation to check for bad patterns in the code;\n> + This is what the original target 'make coccicheck' is designed to do and\n\nIs it relevant that this was the \"original\" target? (Genuine question.)\n\n> + it is expected that any resulting patch indicates a regression.\n> + The patches resulting from 'make coccicheck' are small and infrequent,\n> + so once they are found, they can be sent to the mailing list as per usual.\n> +\n> + Example for introducing new patterns:\n> + 67947c34ae (convert \"hashcmp() != 0\" to \"!hasheq()\", 2018-08-28)\n> + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24)\n> +\n> + Example of fixes using this approach:\n> + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to\n> + a strbuf, 2018-03-25)\n> + f919ffebed (Use MOVE_ARRAY, 2018-01-22)\n> +\n> + These types of semantic patches are usually part of testing, c.f.\n> + 0860a7641b (travis-ci: fail if Coccinelle static analysis found something\n> + to transform, 2018-07-23)\n\nVery nicely described, nice with the examples to quickly give a feeling\nabout how/when to use this.\n\n> + * Using semantic transformations in large scale refactorings throughout\n> + the code base.\n> +\n> + When applying the semantic patch into a real patch, sending it to the\n> + mailing list in the usual way, such a patch would be expected to have a\n> + lot of textual and semantic conflicts as such large scale refactorings\n> + change function signatures that are used widely in the code base.\n> + A textual conflict would arise if surrounding code near any call of such\n> + function changes. A semantic conflict arises when other patch series in\n> + flight introduce calls to such functions.\n\nOK, I'm with you.\n\n> + So to aid these large scale refactorings, semantic patches can be used,\n> + using the process as follows:\n> +\n> + 1) Figure out what kind of large scale refactoring we need\n> + -> This is usually done by another unrelated series.\n\n\"This\"? The figuring out, or the refactoring? Also, \"unrelated\"?\n\n> + 2) Create the sematic patch needed for the large scale refactoring\n\ns/sematic/semantic/\n\n> + and store it in contrib/coccinelle/*.pending.cocci\n> + -> The suffix containing 'pending' is important to differentiate\n> + this case from the other use case of checking for bad patterns.\n\nGood.\n\n> + 3) Apply the semantic patch only partially, where needed for the patch series\n> + that motivates the large scale refactoring and then build that series\n> + on top of it.\n> + By applying the semantic patch only partially where needed, the series\n> + is less likely to conflict with other series in flight.\n> + To make it possible to apply the semantic patch partially, there needs\n> + to be mechanism for backwards compatibility to keep those places working\n> + where the semantic patch is not applied. This can be done via a\n> + wrapper function that has the exact name and signature as the function\n> + to be changed.\n> +\n> + 4) Send the series as usual, including only the needed parts of the\n> + large scale refactoring\n\nTrailing period.\n\nOK, I think I get it, but I wonder if this might not work equally well\nor better under certain circumstances:\n\n - introduce new API\n - add pending semantic patch\n - convert quiet areas to use the new API\n\nOn the other hand, listing all possible flows might be needlessly\nlimiting. I guess it boils down to this:\n\n\"Create a pending semantic patch. Make sure the old way of doing things\nstill works. Apply the semantic patch to the quieter areas of the\ncodebase. If in doubt, convert fewer places in the original series --\nremaining spots can always be converted at a later time.\"\n\nMartin\n"},{"id":"362776","messageId":"xmqqzhui4ymh.fsf@gitster-ct.c.googlers.com","threadId":"49720","inReplyTo":"20181108205255.193402-1-sbeller@google.com","subject":"Re: [PATCH] Makefile: add pending semantic patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-09T05:18:46Z","receivedAt":"2018-11-09T05:20:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: SZEDER Gábor \n>\n> Add a description and place on how to use coccinelle for large refactorings\n> that happen only once.\n>\n> Based-on-work-by: SZEDER Gábor \n> Signed-off-by: Stefan Beller \n> ---\n>\n> I consider including this patch in a resend instead.\n> It outlays the basics of such a new workflow, which we can refine later.\n\nThanks for tying loose ends.\n\n> diff --git a/contrib/coccinelle/README b/contrib/coccinelle/README\n> index 9c2f8879c2..fa09d1abcc 100644\n> --- a/contrib/coccinelle/README\n> +++ b/contrib/coccinelle/README\n> @@ -1,2 +1,62 @@\n> This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)\n> semantic patches that might be useful to developers.\n> +\n> +There are two types of semantic patches:\n> +\n> + * Using the semantic transformation to check for bad patterns in the code;\n> + This is what the original target 'make coccicheck' is designed to do and\n> + it is expected that any resulting patch indicates a regression.\n> + The patches resulting from 'make coccicheck' are small and infrequent,\n> + so once they are found, they can be sent to the mailing list as per usual.\n> +\n> + Example for introducing new patterns:\n> + 67947c34ae (convert \"hashcmp() != 0\" to \"!hasheq()\", 2018-08-28)\n> + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24)\n> +\n> + Example of fixes using this approach:\n> + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to\n> + a strbuf, 2018-03-25)\n> + f919ffebed (Use MOVE_ARRAY, 2018-01-22)\n> +\n> + These types of semantic patches are usually part of testing, c.f.\n> + 0860a7641b (travis-ci: fail if Coccinelle static analysis found something\n> + to transform, 2018-07-23)\n\nYup, and I think what we have in 'pu' (including your the_repository\nstuff) falls into this category.\n\nI've been paying attention to \"make coccicheck\" produced patches for\nthe past few weeks, and so far, I found it _much_ _much_ more\npleasant, compared to having to worry about merge conflicts with the\ntopics in flight that changes day to day (not just because we add\nnew topics or update existing topics, but also the order of the\nmerge changes as topics mature at different rates and jumps over\neach other in master..pu history), that \"make coccicheck\" after\ntopics are merged to integration branches fixes these issues up as\nneeded.\n\n> + 3) Apply the semantic patch only partially, where needed for the patch series\n> + that motivates the large scale refactoring and then build that series\n> + on top of it.\n\nIt is not quite clear what \"needed for the patch series\" really\nmeans in the context of this paragraph. What are the changes that\nare not needed, that is still produced if we ran \"make coccicheck\"?\n\nAre they wrong changes (e.g. a carelessly written read_cache() to\nread_index(&the_index) conversion may munge the implementation of\nread_cache(...) { return read_index(&the_index, ...); } and make\ninifinite recursion)? Or are they \"correct but not immediately\nnecessary\" (e.g. because calling read_cache() does not hurt until\nthat function gets removed, so rewriting the callers to call\nread_index() with &the_index may be correct but not immediately\nnecessary)?\n\n> + By applying the semantic patch only partially where needed, the series\n> + is less likely to conflict with other series in flight.\n\nThat is correct.\n\n> + To make it possible to apply the semantic patch partially, there needs\n> + to be mechanism for backwards compatibility to keep those places working\n> + where the semantic patch is not applied. This can be done via a\n> + wrapper function that has the exact name and signature as the function\n> + to be changed.\n\nOK, so this argues for leaving read_cache()-like things to help\nother in-flight topics, while a change to encourage the use of\nread_index() takes place. That also makes sense, and this directly\nrelates to \"less likely to conflict\" benefit you mentioned above.\n\nBut it is still unclear to me then what are \"necessary\".\n\n... goes and thinks ...\n\nOK, so a series that allows a codepath to work on an arbitrary\nin-core istate, for example, may need to update a function to take\nistate and use it to call read_index(istate...), and the old code in\nsuch a call chain must have used read_cache(), always operating on\n&the_index. For the purpose of that series, it does not matter if\nother codepaths that are not involved in the callchain the series\nwants to update are still only working on &the_index, so a change to\nturn read_cache() into read_index(&the_index) is *not* necessary\n(but still would be correct) and should be left out of the series.\nBut any change the series makes to the callchain in question that\nturns read_cache() into read_index() with something call-specific\n(not &the_index) is a necesary one. Is that a reasonable example\nof what these paragraphs wanted to convey with the distinction\nbetween \"needed for the patch series\" and other changes?\n\n"},{"id":"362838","messageId":"CAGZ79kZRYNjjevWqra2e72rUM3Lu9y1tEL5dDGBLhPKe1o_bGA@mail.gmail.com","threadId":"49720","inReplyTo":"CAN0heSrF7zU=5NGHD9AUcRZgGHoCmrbBwYqJ-6YUM4xg3r+8Rg@mail.gmail.com","subject":"Re: [PATCH] Makefile: add pending semantic patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-09T20:50:33Z","receivedAt":"2018-11-09T20:50:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Nov 8, 2018 at 8:56 PM Martin Ågren wrote:\n> I haven't followed the original discussion too carefully, so I'll read\n> this like someone new to the topic probably would.\n\nThanks!\n\n> A nit, perhaps, but I was genuinely confused at first. The subject is\n> \"Makefile: add pending semantic patches\", but this patch doesn't add\n> any. It adds Makefile-support for such patches though, and it defines\n> the entire concept of pending semantic patches. How about \"coccicheck:\n> introduce 'pending' semantic patches\"?\n>\n> > Add a description and place on how to use coccinelle for large refactorings\n> > that happen only once.\n>\n> A bit confused about \"and place\". Based on my understanding from reading\n> the remainder of this patch, maybe:\n>\n> Teach `make coccicheck` to avoid patches named \"*.pending.cocci\" and\n> handle them separately in a new `make coccicheck-pending` instead.\n> This means that we can separate \"critical\" patches from \"FYI\" patches.\n> The former target can continue causing Travis to fail its static\n> analysis job, while the latter can let us keep an eye on ongoing\n> (pending) transitions without them causing too much fallout.\n>\n> Document the intended use-cases and processes around these two\n> targets.\n\nBoth suggested title and new commit message make sense.\n\n>\n> > This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)\n> > semantic patches that might be useful to developers.\n> > +\n> > +There are two types of semantic patches:\n> > +\n> > + * Using the semantic transformation to check for bad patterns in the code;\n> > + This is what the original target 'make coccicheck' is designed to do and\n>\n> Is it relevant that this was the \"original\" target? (Genuine question.)\n\nNo. I can omit that part.\n\n>\n> > + it is expected that any resulting patch indicates a regression.\n> > + The patches resulting from 'make coccicheck' are small and infrequent,\n> > + so once they are found, they can be sent to the mailing list as per usual.\n> > +\n> > + Example for introducing new patterns:\n> > + 67947c34ae (convert \"hashcmp() != 0\" to \"!hasheq()\", 2018-08-28)\n> > + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24)\n> > +\n> > + Example of fixes using this approach:\n> > + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to\n> > + a strbuf, 2018-03-25)\n> > + f919ffebed (Use MOVE_ARRAY, 2018-01-22)\n> > +\n> > + These types of semantic patches are usually part of testing, c.f.\n> > + 0860a7641b (travis-ci: fail if Coccinelle static analysis found something\n> > + to transform, 2018-07-23)\n>\n> Very nicely described, nice with the examples to quickly give a feeling\n> about how/when to use this.\n\nThanks.\n\n\n>\n> > + * Using semantic transformations in large scale refactorings throughout\n> > + the code base.\n> > +\n> > + When applying the semantic patch into a real patch, sending it to the\n> > + mailing list in the usual way, such a patch would be expected to have a\n> > + lot of textual and semantic conflicts as such large scale refactorings\n> > + change function signatures that are used widely in the code base.\n> > + A textual conflict would arise if surrounding code near any call of such\n> > + function changes. A semantic conflict arises when other patch series in\n> > + flight introduce calls to such functions.\n>\n> OK, I'm with you.\n>\n> > + So to aid these large scale refactorings, semantic patches can be used,\n> > + using the process as follows:\n> > +\n> > + 1) Figure out what kind of large scale refactoring we need\n> > + -> This is usually done by another unrelated series.\n>\n> \"This\"? The figuring out, or the refactoring? Also, \"unrelated\"?\n\nThe need and type of what kind of large scale refactoring are\nusually determined by a patch series, that is not refactoring for the\nsake of refactoring, but it wants to achieve a specific goal, unrelated\nto large refactorings per se.\n\nThe large refactoring is just another tool that a developer can use\nto make their original series happen much faster.\n\nSo \"unrelated\" == \"not the large scale refactoring, as that may\ncome as an preparatory series, but to have a preparatory series\nit may be good to showcase why we need the preparatory series\"\n\n>\n> > + 2) Create the sematic patch needed for the large scale refactoring\n>\n> s/sematic/semantic/\n\nyup.\n\n>\n> > + and store it in contrib/coccinelle/*.pending.cocci\n> > + -> The suffix containing 'pending' is important to differentiate\n> > + this case from the other use case of checking for bad patterns.\n>\n> Good.\n>\n> > + 3) Apply the semantic patch only partially, where needed for the patch series\n> > + that motivates the large scale refactoring and then build that series\n> > + on top of it.\n> > + By applying the semantic patch only partially where needed, the series\n> > + is less likely to conflict with other series in flight.\n> > + To make it possible to apply the semantic patch partially, there needs\n> > + to be mechanism for backwards compatibility to keep those places working\n> > + where the semantic patch is not applied. This can be done via a\n> > + wrapper function that has the exact name and signature as the function\n> > + to be changed.\n> > +\n> > + 4) Send the series as usual, including only the needed parts of the\n> > + large scale refactoring\n>\n> Trailing period.\n\nok.\n\n> OK, I think I get it, but I wonder if this might not work equally well\n> or better under certain circumstances:\n>\n> - introduce new API\n\nThe \"new API\" is similar enough to the old API and may even be\na superset.\n\n> - add pending semantic patch\n> - convert quiet areas to use the new API\n>\n> On the other hand, listing all possible flows might be needlessly\n> limiting. I guess it boils down to this:\n>\n> \"Create a pending semantic patch. Make sure the old way of doing things\n> still works. Apply the semantic patch to the quieter areas of the\n> codebase. If in doubt, convert fewer places in the original series --\n> remaining spots can always be converted at a later time.\"\n\nThat's the gist of it. :)\n\nThanks for the review,\nStefan\n"},{"id":"362841","messageId":"CAGZ79kbdewnxe=uC4GScvjJTgsru75O2FzSckjWoin2g1yXyPw@mail.gmail.com","threadId":"49720","inReplyTo":"xmqqzhui4ymh.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] Makefile: add pending semantic patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-09T21:58:01Z","receivedAt":"2018-11-09T21:58:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Nov 8, 2018 at 9:18 PM Junio C Hamano wrote:\n>\n> Stefan Beller writes:\n>\n> > From: SZEDER Gábor \n> >\n> > Add a description and place on how to use coccinelle for large refactorings\n> > that happen only once.\n> >\n> > Based-on-work-by: SZEDER Gábor \n> > Signed-off-by: Stefan Beller \n> > ---\n> >\n> > I consider including this patch in a resend instead.\n> > It outlays the basics of such a new workflow, which we can refine later.\n>\n> Thanks for tying loose ends.\n>\n> > diff --git a/contrib/coccinelle/README b/contrib/coccinelle/README\n> > index 9c2f8879c2..fa09d1abcc 100644\n> > --- a/contrib/coccinelle/README\n> > +++ b/contrib/coccinelle/README\n> > @@ -1,2 +1,62 @@\n> > This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)\n> > semantic patches that might be useful to developers.\n> > +\n> > +There are two types of semantic patches:\n> > +\n> > + * Using the semantic transformation to check for bad patterns in the code;\n> > + This is what the original target 'make coccicheck' is designed to do and\n> > + it is expected that any resulting patch indicates a regression.\n> > + The patches resulting from 'make coccicheck' are small and infrequent,\n> > + so once they are found, they can be sent to the mailing list as per usual.\n> > +\n> > + Example for introducing new patterns:\n> > + 67947c34ae (convert \"hashcmp() != 0\" to \"!hasheq()\", 2018-08-28)\n> > + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24)\n> > +\n> > + Example of fixes using this approach:\n> > + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to\n> > + a strbuf, 2018-03-25)\n> > + f919ffebed (Use MOVE_ARRAY, 2018-01-22)\n> > +\n> > + These types of semantic patches are usually part of testing, c.f.\n> > + 0860a7641b (travis-ci: fail if Coccinelle static analysis found something\n> > + to transform, 2018-07-23)\n>\n> Yup, and I think what we have in 'pu' (including your the_repository\n> stuff) falls into this category.\n\nMy impression was that the_repository is strongly second category\nas the_repository.cocci doesn't fix bad smells of code, but proposes\na refactoring that we agreed on doing.\n\n> I've been paying attention to \"make coccicheck\" produced patches for\n> the past few weeks, and so far, I found it _much_ _much_ more\n> pleasant, compared to having to worry about merge conflicts with the\n> topics in flight that changes day to day (not just because we add\n> new topics or update existing topics, but also the order of the\n> merge changes as topics mature at different rates and jumps over\n> each other in master..pu history), that \"make coccicheck\" after\n> topics are merged to integration branches fixes these issues up as\n> needed.\n\nSo from your end we would not need the \"pending\" category as long\nas the follow ups come in a timely manner?\n\n>\n> > + 3) Apply the semantic patch only partially, where needed for the patch series\n> > + that motivates the large scale refactoring and then build that series\n> > + on top of it.\n>\n> It is not quite clear what \"needed for the patch series\" really\n> means in the context of this paragraph. What are the changes that\n> are not needed, that is still produced if we ran \"make coccicheck\"?\n\nAn example for \"needed\" would be\n3f21279f50..bd8737176b\nwhereas \"not needed\" is what is in\n\"treewide: apply cocci patch\".\n\nThe treewide conversion of e.g. unuse_commit_buffer to\nrepo_unuse_commit_buffer is nice, but \"needed\" only in\nits followup patch that converts logmsg_reencode as that\ncalls into the unuse_commit_buffer.\n\n> Are they wrong changes (e.g. a carelessly written read_cache() to\n> read_index(&the_index) conversion may munge the implementation of\n> read_cache(...) { return read_index(&the_index, ...); } and make\n> inifinite recursion)? Or are they \"correct but not immediately\n> necessary\" (e.g. because calling read_cache() does not hurt until\n> that function gets removed, so rewriting the callers to call\n> read_index() with &the_index may be correct but not immediately\n> necessary)?\n\nthe latter. I assume correctness (of the semantic patch) to be a given,\nbut this is all about timing, i.e. how can I send the series without breaking\nothers in flight.\n\n>\n> > + By applying the semantic patch only partially where needed, the series\n> > + is less likely to conflict with other series in flight.\n>\n> That is correct.\n>\n> > + To make it possible to apply the semantic patch partially, there needs\n> > + to be mechanism for backwards compatibility to keep those places working\n> > + where the semantic patch is not applied. This can be done via a\n> > + wrapper function that has the exact name and signature as the function\n> > + to be changed.\n>\n> OK, so this argues for leaving read_cache()-like things to help\n> other in-flight topics, while a change to encourage the use of\n> read_index() takes place. That also makes sense, and this directly\n> relates to \"less likely to conflict\" benefit you mentioned above.\n\nok.\n\n>\n> But it is still unclear to me then what are \"necessary\".\n>\n> ... goes and thinks ...\n>\n> OK, so a series that allows a codepath to work on an arbitrary\n> in-core istate, for example, may need to update a function to take\n> istate and use it to call read_index(istate...), and the old code in\n> such a call chain must have used read_cache(), always operating on\n> &the_index. For the purpose of that series, it does not matter if\n> other codepaths that are not involved in the callchain the series\n> wants to update are still only working on &the_index, so a change to\n> turn read_cache() into read_index(&the_index) is *not* necessary\n> (but still would be correct) and should be left out of the series.\n> But any change the series makes to the callchain in question that\n> turns read_cache() into read_index() with something call-specific\n> (not &the_index) is a necesary one. Is that a reasonable example\n> of what these paragraphs wanted to convey with the distinction\n> between \"needed for the patch series\" and other changes?\n\nExactly.\n\nMaybe another way to phrase it is to explain the two series\nindependently of each other:\n\n 1) Create the semantic patch series containing\n 1a) - a *.pending.cocci semantic patch\n 1b) - forward/backward compatibility providers\n (wrapper/defines)\n 1c) then send the semantic patch series to the list\n\n 2a) Write the other series as if (1) doesn't exist\n This means there will be some upgrades of\n the call chain from read_cache() to read_index()\n 2b) Coincidentally these upgrades are the same\n as (1) would have produced. That's the whole\n trick.\n 2c) send of this series independently of (1)\n\nThis can be done for read_cache / read_index as\nthey both exist already, but when read_index is\nnew to the code base, we'd need (2) to rely on (1b).\n\nAnd that is why this patch sounded complicated.\n"},{"id":"362842","messageId":"20181110001052.101290-1-sbeller@google.com","threadId":"49720","inReplyTo":"20181108205255.193402-1-sbeller@google.com","subject":"[PATCH] coccicheck: introduce 'pending' semantic patches","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-11-10T00:10:52Z","receivedAt":"2018-11-10T00:10:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: SZEDER Gábor \n\nTeach `make coccicheck` to avoid patches named \"*.pending.cocci\" and\nhandle them separately in a new `make coccicheck-pending` instead.\nThis means that we can separate \"critical\" patches from \"FYI\" patches.\nThe former target can continue causing Travis to fail its static\nanalysis job, while the latter can let us keep an eye on ongoing\n(pending) transitions without them causing too much fallout.\n\nDocument the intended use-cases around these two targets.\nAs the process around the pending patches is not yet fully explored,\nleave that out.\n\nBased-on-work-by: SZEDER Gábor \nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n\nI dialed back on the workflow, as we may want to explore it first\nbefore writing it down.\n\nStefan\n\n Makefile | 7 +++++--\n contrib/coccinelle/README | 41 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 46 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex bbfbb4292d..6bc4654828 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2740,9 +2740,12 @@ endif\n \tthen \\\n \t\techo ' ' SPATCH result: $@; \\\n \tfi\n-coccicheck: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.cocci))\n+coccicheck: $(addsuffix .patch,$(filter-out %.pending.cocci,$(wildcard contrib/coccinelle/*.cocci)))\n \n-.PHONY: coccicheck\n+# See contrib/coccinelle/README\n+coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.cocci))\n+\n+.PHONY: coccicheck coccicheck-pending\n \n ### Installation rules\n \ndiff --git a/contrib/coccinelle/README b/contrib/coccinelle/README\nindex 9c2f8879c2..f0e80bd7f0 100644\n--- a/contrib/coccinelle/README\n+++ b/contrib/coccinelle/README\n@@ -1,2 +1,43 @@\n This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)\n semantic patches that might be useful to developers.\n+\n+There are two types of semantic patches:\n+\n+ * Using the semantic transformation to check for bad patterns in the code;\n+ The target 'make coccicheck' is designed to check for these patterns and\n+ it is expected that any resulting patch indicates a regression.\n+ The patches resulting from 'make coccicheck' are small and infrequent,\n+ so once they are found, they can be sent to the mailing list as per usual.\n+\n+ Example for introducing new patterns:\n+ 67947c34ae (convert \"hashcmp() != 0\" to \"!hasheq()\", 2018-08-28)\n+ b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24)\n+\n+ Example of fixes using this approach:\n+ 248f66ed8e (run-command: use strbuf_addstr() for adding a string to\n+ a strbuf, 2018-03-25)\n+ f919ffebed (Use MOVE_ARRAY, 2018-01-22)\n+\n+ These types of semantic patches are usually part of testing, c.f.\n+ 0860a7641b (travis-ci: fail if Coccinelle static analysis found something\n+ to transform, 2018-07-23)\n+\n+ * Using semantic transformations in large scale refactorings throughout\n+ the code base.\n+\n+ When applying the semantic patch into a real patch, sending it to the\n+ mailing list in the usual way, such a patch would be expected to have a\n+ lot of textual and semantic conflicts as such large scale refactorings\n+ change function signatures that are used widely in the code base.\n+ A textual conflict would arise if surrounding code near any call of such\n+ function changes. A semantic conflict arises when other patch series in\n+ flight introduce calls to such functions.\n+\n+ So to aid these large scale refactorings, semantic patches can be used.\n+ However we do not want to store them in the same place as the checks for\n+ bad patterns, as then automated builds would fail.\n+ That is why semantic patches 'contrib/coccinelle/*.pending.cocci'\n+ are ignored for checks, and can be applied using 'make coccicheck-pending'.\n+\n+ This allows to expose plans of pending large scale refactorings without\n+ impacting the bad pattern checks.\n-- \n2.19.1.930.g4563a0d9d0-goog\n\n"},{"id":"362896","messageId":"CAN0heSqOehMfhpa62MqWtQjOsr4OAgFYU+0J=ds1uZNjP2LeOQ@mail.gmail.com","threadId":"49720","inReplyTo":"20181110001052.101290-1-sbeller@google.com","subject":"Re: [PATCH] coccicheck: introduce 'pending' semantic patches","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-11-10T20:37:13Z","receivedAt":"2018-11-10T20:37:30Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Sat, 10 Nov 2018 at 01:10, Stefan Beller wrote:\n> I dialed back on the workflow, as we may want to explore it first\n> before writing it down.\n\nMakes sense.\n\nFWIW, this iteration looks good to me.\n\nMartin\n"},{"id":"363085","messageId":"20181112211941.GA126896@google.com","threadId":"49720","inReplyTo":"20181110001052.101290-1-sbeller@google.com","subject":"Re: [PATCH] coccicheck: introduce 'pending' semantic patches","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2018-11-12T21:19:41Z","receivedAt":"2018-11-12T21:19:50Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2018.11.09 16:10, Stefan Beller wrote:\n> From: SZEDER Gábor \n> \n> Teach `make coccicheck` to avoid patches named \"*.pending.cocci\" and\n> handle them separately in a new `make coccicheck-pending` instead.\n> This means that we can separate \"critical\" patches from \"FYI\" patches.\n> The former target can continue causing Travis to fail its static\n> analysis job, while the latter can let us keep an eye on ongoing\n> (pending) transitions without them causing too much fallout.\n> \n> Document the intended use-cases around these two targets.\n> As the process around the pending patches is not yet fully explored,\n> leave that out.\n> \n> Based-on-work-by: SZEDER Gábor \n> Signed-off-by: Stefan Beller \n> Signed-off-by: Junio C Hamano \n> ---\n> \n> I dialed back on the workflow, as we may want to explore it first\n> before writing it down.\n> \n> Stefan\n\nLooks good to me.\n\nReviewed-by: Josh Steadmon \n"},{"id":"363172","messageId":"20181113154818.GE30222@szeder.dev","threadId":"49720","inReplyTo":"CAGZ79kbdewnxe=uC4GScvjJTgsru75O2FzSckjWoin2g1yXyPw@mail.gmail.com","subject":"Re: [PATCH] Makefile: add pending semantic patches","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-13T15:48:18Z","receivedAt":"2018-11-13T15:48:24Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Nov 09, 2018 at 01:58:01PM -0800, Stefan Beller wrote:\n> On Thu, Nov 8, 2018 at 9:18 PM Junio C Hamano wrote:\n> > Are they wrong changes (e.g. a carelessly written read_cache() to\n> > read_index(&the_index) conversion may munge the implementation of\n> > read_cache(...) { return read_index(&the_index, ...); } and make\n> > inifinite recursion)? Or are they \"correct but not immediately\n> > necessary\" (e.g. because calling read_cache() does not hurt until\n> > that function gets removed, so rewriting the callers to call\n> > read_index() with &the_index may be correct but not immediately\n> > necessary)?\n> \n> the latter. I assume correctness (of the semantic patch) to be a given,\n\nI'm afraid we can't assume that. As far as correctness is concerned,\nI think semantic patches are not different from any other code we\nwrite, i.e. they are potentially buggy. Perhaps even more so than the\n\"usual\" Git code, because we have long experience writing C and shell\ncode (with all their quirks and error-proneness), but none of us is\nreally an expert in writing semantic patches.\n\nCases in point:\n\n - 6afedba8c9 (object_id.cocci: match only expressions of type\n 'struct object_id', 2018-10-15)\n - 279ffad17d (coccinelle: avoid wrong transformation suggestions\n from commit.cocci, 2018-04-30)\n - cd9a4b6d93 (cocci: use format keyword instead of a literal string,\n 2018-01-19); though this one is probably a bug in Coccinelle\n itself\n - c2bb0c1d1e (cocci: avoid self-references in object_id\n transformations, 2016-11-01)\n\n"},{"id":"363174","messageId":"20181113160252.GF30222@szeder.dev","threadId":"49720","inReplyTo":"20181110001052.101290-1-sbeller@google.com","subject":"Re: [PATCH] coccicheck: introduce 'pending' semantic patches","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-13T16:02:52Z","receivedAt":"2018-11-13T16:02:58Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Nov 09, 2018 at 04:10:52PM -0800, Stefan Beller wrote:\n> From: SZEDER Gábor \n> \n> Teach `make coccicheck` to avoid patches named \"*.pending.cocci\" and\n> handle them separately in a new `make coccicheck-pending` instead.\n> This means that we can separate \"critical\" patches from \"FYI\" patches.\n> The former target can continue causing Travis to fail its static\n> analysis job, while the latter can let us keep an eye on ongoing\n> (pending) transitions without them causing too much fallout.\n> \n> Document the intended use-cases around these two targets.\n> As the process around the pending patches is not yet fully explored,\n> leave that out.\n> \n> Based-on-work-by: SZEDER Gábor \n\nHm, do I need to sign off?\nWell, I sign off now anyway, and then Junio can take it if necessary\nor just ignore it if not.\n\nSigned-off-by: SZEDER Gábor \n\n> Signed-off-by: Stefan Beller \n> Signed-off-by: Junio C Hamano \n> ---\n> \n> I dialed back on the workflow, as we may want to explore it first\n> before writing it down.\n\nYeah, I too think that's better to wait with the workflow details\nuntil we gather some experience about how it works out in practice.\n\nThanks for following it through.\n\n"},{"id":"363306","messageId":"xmqq4lckqoqc.fsf@gitster-ct.c.googlers.com","threadId":"49720","inReplyTo":"20181113154818.GE30222@szeder.dev","subject":"Re: [PATCH] Makefile: add pending semantic patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-14T04:15:07Z","receivedAt":"2018-11-14T04:15:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> > inifinite recursion)? Or are they \"correct but not immediately\n>> > necessary\" (e.g. because calling read_cache() does not hurt until\n>> > that function gets removed, so rewriting the callers to call\n>> > read_index() with &the_index may be correct but not immediately\n>> > necessary)?\n>> \n>> the latter. I assume correctness (of the semantic patch) to be a given,\n>\n> I'm afraid we can't assume that. As far as correctness is concerned,\n> I think semantic patches are not different from any other code we\n> write, i.e. they are potentially buggy. Perhaps even more so than the\n> \"usual\" Git code, because we have long experience writing C and shell\n> code (with all their quirks and error-proneness), but none of us is\n> really an expert in writing semantic patches.\n\nAll correct.\n\nAnd applying semantic patches generated from buggy sources can\nproduce buggy code, just like merging a buggy topic branch does.\n\nThese days, I ran coccicheck at the tip of 'pu' (even though this\ncost me quite a lot a few times every day) and feed its findings\nback to authors of topics that introduce new ones, so that their\ntopics next time do not need the fix-up at the tip of 'pu' in the\nnext integration cycle. That way, the changes mechanically\nsuggested by coccicheck can still be reviewed in small bite-sized\nchunks and hopefully possible problems caused by buggy sources can\neither be found in the review process, or discovered at the tip of\n'pu'.\n\n\n"}]}