{"thread":{"id":"53745","subject":"[PATCH 1/2] revision: use repository from rev_info when parsing commits","startedAt":"2020-06-23T21:02:24Z","lastAt":"2020-09-04T12:19:47Z","messageCount":10,"participants":["Michael Forney","Eric Sunshine","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"400437","messageId":"20200623205659.14297-1-mforney@mforney.org","threadId":"53745","inReplyTo":null,"subject":"[PATCH 1/2] revision: use repository from rev_info when parsing commits","fromName":"Michael Forney","fromEmail":"mforney@mforney.org","sentAt":"2020-06-23T20:56:58Z","receivedAt":"2020-06-23T21:02:24Z","isPatch":true,"sender":{"key":"mforney@mforney.org","avatar":"https://avatars.githubusercontent.com/u/52851?v=4"},"body":"This is needed when repo_init_revisions() is called with a repository\nthat is not the_repository to ensure appropriate repository is used\nin repo_parse_commit_internal(). If the wrong repository is used,\na fatal error is the commit-graph machinery occurs:\n\n  fatal: invalid commit position. commit-graph is likely corrupt\n\nSince revision.c was the only user of the parse_commit_gently\ncompatibility define, remove it from commit.h.\n\nSigned-off-by: Michael Forney <mforney@mforney.org>\n---\n commit.h   |  1 -\n revision.c | 18 +++++++++---------\n 2 files changed, 9 insertions(+), 10 deletions(-)\n\ndiff --git a/commit.h b/commit.h\nindex 1b2dea5d85..a2e8ca99a2 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -97,7 +97,6 @@ static inline int parse_commit_no_graph(struct commit *commit)\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 \ndiff --git a/revision.c b/revision.c\nindex ebb4d2a0f2..2b6bf47c81 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -439,7 +439,7 @@ static struct commit *handle_commit(struct rev_info *revs,\n \tif (object->type == OBJ_COMMIT) {\n \t\tstruct commit *commit = (struct commit *)object;\n \n-\t\tif (parse_commit(commit) < 0)\n+\t\tif (repo_parse_commit(revs->repo, commit) < 0)\n \t\t\tdie(\"unable to parse commit %s\", name);\n \t\tif (flags & UNINTERESTING) {\n \t\t\tmark_parents_uninteresting(commit);\n@@ -992,7 +992,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t\t\tts->treesame[0] = 1;\n \t\t\t}\n \t\t}\n-\t\tif (parse_commit(p) < 0)\n+\t\tif (repo_parse_commit(revs->repo, p) < 0)\n \t\t\tdie(\"cannot simplify commit %s (because of %s)\",\n \t\t\t    oid_to_hex(&commit->object.oid),\n \t\t\t    oid_to_hex(&p->object.oid));\n@@ -1037,7 +1037,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t\t * IOW, we pretend this parent is a\n \t\t\t\t * \"root\" commit.\n \t\t\t\t */\n-\t\t\t\tif (parse_commit(p) < 0)\n+\t\t\t\tif (repo_parse_commit(revs->repo, p) < 0)\n \t\t\t\t\tdie(\"cannot simplify commit %s (invalid %s)\",\n \t\t\t\t\t    oid_to_hex(&commit->object.oid),\n \t\t\t\t\t    oid_to_hex(&p->object.oid));\n@@ -1105,7 +1105,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,\n \t\t\tparent = parent->next;\n \t\t\tif (p)\n \t\t\t\tp->object.flags |= UNINTERESTING;\n-\t\t\tif (parse_commit_gently(p, 1) < 0)\n+\t\t\tif (repo_parse_commit_gently(revs->repo, p, 1) < 0)\n \t\t\t\tcontinue;\n \t\t\tif (p->parents)\n \t\t\t\tmark_parents_uninteresting(p);\n@@ -1136,7 +1136,7 @@ static int process_parents(struct rev_info *revs, struct commit *commit,\n \t\tstruct commit *p = parent->item;\n \t\tint gently = revs->ignore_missing_links ||\n \t\t\t     revs->exclude_promisor_objects;\n-\t\tif (parse_commit_gently(p, gently) < 0) {\n+\t\tif (repo_parse_commit_gently(revs->repo, p, gently) < 0) {\n \t\t\tif (revs->exclude_promisor_objects &&\n \t\t\t    is_promisor_object(&p->object.oid)) {\n \t\t\t\tif (revs->first_parent_only)\n@@ -3295,7 +3295,7 @@ static void explore_walk_step(struct rev_info *revs)\n \tif (!c)\n \t\treturn;\n \n-\tif (parse_commit_gently(c, 1) < 0)\n+\tif (repo_parse_commit_gently(revs->repo, c, 1) < 0)\n \t\treturn;\n \n \tif (revs->sort_order == REV_SORT_BY_AUTHOR_DATE)\n@@ -3333,7 +3333,7 @@ static void indegree_walk_step(struct rev_info *revs)\n \tif (!c)\n \t\treturn;\n \n-\tif (parse_commit_gently(c, 1) < 0)\n+\tif (repo_parse_commit_gently(revs->repo, c, 1) < 0)\n \t\treturn;\n \n \texplore_to_depth(revs, c->generation);\n@@ -3414,7 +3414,7 @@ static void init_topo_walk(struct rev_info *revs)\n \tfor (list = revs->commits; list; list = list->next) {\n \t\tstruct commit *c = list->item;\n \n-\t\tif (parse_commit_gently(c, 1))\n+\t\tif (repo_parse_commit_gently(revs->repo, c, 1))\n \t\t\tcontinue;\n \n \t\ttest_flag_and_insert(&info->explore_queue, c, TOPO_WALK_EXPLORED);\n@@ -3476,7 +3476,7 @@ static void expand_topo_walk(struct rev_info *revs, struct commit *commit)\n \t\tif (parent->object.flags & UNINTERESTING)\n \t\t\tcontinue;\n \n-\t\tif (parse_commit_gently(parent, 1) < 0)\n+\t\tif (repo_parse_commit_gently(revs->repo, parent, 1) < 0)\n \t\t\tcontinue;\n \n \t\tif (parent->generation < info->min_generation) {\n-- \n2.27.0\n\n"},{"id":"400438","messageId":"20200623205659.14297-2-mforney@mforney.org","threadId":"53745","inReplyTo":"20200623205659.14297-1-mforney@mforney.org","subject":"[PATCH 2/2] submodule: use submodule repository when preparing summary","fromName":"Michael Forney","fromEmail":"mforney@mforney.org","sentAt":"2020-06-23T20:56:59Z","receivedAt":"2020-06-23T21:02:26Z","isPatch":true,"sender":{"key":"mforney@mforney.org","avatar":"https://avatars.githubusercontent.com/u/52851?v=4"},"body":"In show_submodule_header(), we gather the left and right commits\nof the submodule repository, as well as the merge bases. However,\nprepare_submodule_summary() initializes the rev_info with the_repository,\nso we end up parsing the commit in the wrong repository.\n\nThis results in a fatal error in parse_commit_in_graph(), since the\npassed item does not belong to the repository's commit graph.\n\nSigned-off-by: Michael Forney <mforney@mforney.org>\n---\n submodule.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex e2ef5698c8..785ab47629 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -438,13 +438,13 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t */\n }\n \n-static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n-\t\tstruct commit *left, struct commit *right,\n+static int prepare_submodule_summary(struct repository *r, struct rev_info *rev,\n+\t\tconst char *path, struct commit *left, struct commit *right,\n \t\tstruct commit_list *merge_bases)\n {\n \tstruct commit_list *list;\n \n-\trepo_init_revisions(the_repository, rev, NULL);\n+\trepo_init_revisions(r, rev, NULL);\n \tsetup_revisions(0, NULL, rev, NULL);\n \trev->left_right = 1;\n \trev->first_parent_only = 1;\n@@ -632,7 +632,7 @@ void show_submodule_summary(struct diff_options *o, const char *path,\n \t\tgoto out;\n \n \t/* Treat revision walker failure the same as missing commits */\n-\tif (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {\n+\tif (prepare_submodule_summary(sub, &rev, path, left, right, merge_bases)) {\n \t\tdiff_emit_submodule_error(o, \"(revision walker failed)\\n\");\n \t\tgoto out;\n \t}\n-- \n2.27.0\n\n"},{"id":"400451","messageId":"CAPig+cSDxTjLbFWGMZuGuCDcZ05fnaJR0q-TA9DpKTki2M7MyQ@mail.gmail.com","threadId":"53745","inReplyTo":"20200623205659.14297-1-mforney@mforney.org","subject":"Re: [PATCH 1/2] revision: use repository from rev_info when parsing commits","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-23T21:24:34Z","receivedAt":"2020-06-23T21:26:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 23, 2020 at 5:02 PM Michael Forney <mforney@mforney.org> wrote:\n> This is needed when repo_init_revisions() is called with a repository\n> that is not the_repository to ensure appropriate repository is used\n> in repo_parse_commit_internal(). If the wrong repository is used,\n> a fatal error is the commit-graph machinery occurs:\n\ns/is/in/\n\n>   fatal: invalid commit position. commit-graph is likely corrupt\n>\n> Since revision.c was the only user of the parse_commit_gently\n> compatibility define, remove it from commit.h.\n>\n> Signed-off-by: Michael Forney <mforney@mforney.org>\n"},{"id":"400504","messageId":"88d8b24c-a0ae-bbbf-dd1f-5adb7a36ee95@gmail.com","threadId":"53745","inReplyTo":"20200623205659.14297-1-mforney@mforney.org","subject":"Re: [PATCH 1/2] revision: use repository from rev_info when parsing commits","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-06-24T14:29:41Z","receivedAt":"2020-06-24T14:29:48Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/23/2020 4:56 PM, Michael Forney wrote:\n> This is needed when repo_init_revisions() is called with a repository\n> that is not the_repository to ensure appropriate repository is used\n> in repo_parse_commit_internal(). If the wrong repository is used,\n> a fatal error is the commit-graph machinery occurs:\n> \n>   fatal: invalid commit position. commit-graph is likely corrupt\n> \n> Since revision.c was the only user of the parse_commit_gently\n> compatibility define, remove it from commit.h.\n\nIs this demonstrable in a test case, to prevent regressions?\n\nNotably, you are _not_ dropping parse_commit(), and it would be\neasy for another call to that shim to slip into revision.c.\n\n> Signed-off-by: Michael Forney <mforney@mforney.org>\n> ---\n>  commit.h   |  1 -\n>  revision.c | 18 +++++++++---------\n>  2 files changed, 9 insertions(+), 10 deletions(-)\n> \n> diff --git a/commit.h b/commit.h\n> -#define parse_commit_gently(item, quiet) repo_parse_commit_gently(the_repository, item, quiet)\n\nI'm happy we can drop this shim!\n\n> diff --git a/revision.c b/revision.c\n> index ebb4d2a0f2..2b6bf47c81 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -439,7 +439,7 @@ static struct commit *handle_commit(struct rev_info *revs,\n>  \tif (object->type == OBJ_COMMIT) {\n>  \t\tstruct commit *commit = (struct commit *)object;\n>  \n> -\t\tif (parse_commit(commit) < 0)\n> +\t\tif (repo_parse_commit(revs->repo, commit) < 0)\n\nI counted 9 copies of parse_commit[_gently]() in my version\nof revision.c, so it looks like you caught them all.\n\nThanks!\n-Stolee\n\n"},{"id":"400505","messageId":"33de1078-5f19-e76c-2a30-1754494d1e31@gmail.com","threadId":"53745","inReplyTo":"20200623205659.14297-2-mforney@mforney.org","subject":"Re: [PATCH 2/2] submodule: use submodule repository when preparing summary","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-06-24T14:35:30Z","receivedAt":"2020-06-24T14:35:34Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/23/2020 4:56 PM, Michael Forney wrote:\n> In show_submodule_header(), we gather the left and right commits\n> of the submodule repository, as well as the merge bases. However,\n> prepare_submodule_summary() initializes the rev_info with the_repository,\n> so we end up parsing the commit in the wrong repository.\n> \n> This results in a fatal error in parse_commit_in_graph(), since the\n> passed item does not belong to the repository's commit graph.\n> \n> Signed-off-by: Michael Forney <mforney@mforney.org>\n> ---\n>  submodule.c | 8 ++++----\n>  1 file changed, 4 insertions(+), 4 deletions(-)\n> \n> diff --git a/submodule.c b/submodule.c\n> index e2ef5698c8..785ab47629 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -438,13 +438,13 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt,\n>  \t */\n>  }\n>  \n> -static int prepare_submodule_summary(struct rev_info *rev, const char *path,\n> -\t\tstruct commit *left, struct commit *right,\n> +static int prepare_submodule_summary(struct repository *r, struct rev_info *rev,\n> +\t\tconst char *path, struct commit *left, struct commit *right,\n>  \t\tstruct commit_list *merge_bases)\n>  {\n>  \tstruct commit_list *list;\n>  \n> -\trepo_init_revisions(the_repository, rev, NULL);\n> +\trepo_init_revisions(r, rev, NULL);\n\nThis is how we properly initialize the repository in the rev_info.\nIt's unfortunate that this use of the_repository was pretty clearly\nincorrect. This is submodule.c, so every instance of the_repository\nshould be examined carefully. Taking a brief look right now, the\nrest seem to be correct in that they are finding submodules within\nthe super-repo. The only issue will arise when recursing into\nsubmodules, which is known to be broken in-process and are handled\nwith subprocesses instead.\n\n>  \tsetup_revisions(0, NULL, rev, NULL);\n>  \trev->left_right = 1;\n>  \trev->first_parent_only = 1;\n> @@ -632,7 +632,7 @@ void show_submodule_summary(struct diff_options *o, const char *path,\n>  \t\tgoto out;\n>  \n>  \t/* Treat revision walker failure the same as missing commits */\n> -\tif (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {\n> +\tif (prepare_submodule_summary(sub, &rev, path, left, right, merge_bases)) {\n>  \t\tdiff_emit_submodule_error(o, \"(revision walker failed)\\n\");\n>  \t\tgoto out;\n>  \t}\n\nPerhaps the test I requested in patch 1 is only appropriate\nhere? Or, maybe the test should be test_expect_failure in the\nfirst patch and switched to test_expect_success here?\n\nThanks,\n-Stolee\n\n\n"},{"id":"400540","messageId":"xmqq4kr08mtw.fsf@gitster.c.googlers.com","threadId":"53745","inReplyTo":"33de1078-5f19-e76c-2a30-1754494d1e31@gmail.com","subject":"Re: [PATCH 2/2] submodule: use submodule repository when preparing summary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-24T16:12:27Z","receivedAt":"2020-06-24T16:12:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> Perhaps the test I requested in patch 1 is only appropriate\n> here? Or, maybe the test should be test_expect_failure in the\n> first patch and switched to test_expect_success here?\n\nEither is OK, but it is probably easier to read to have just one\naddition in 2/2 to expect succeses.  Temporarily revierting with\n\"git show ':!t' | git apply -R\" before running test when you want\nto reallly see how the original crashed is easy and simple enough.\n\nThanks.\n\n\n"},{"id":"400799","messageId":"CAGw6cBsctN0-BP6k7p71-edsHR4BxJWai4Qz5m5gi4J6pYh=Kw@mail.gmail.com","threadId":"53745","inReplyTo":"33de1078-5f19-e76c-2a30-1754494d1e31@gmail.com","subject":"Re: [PATCH 2/2] submodule: use submodule repository when preparing summary","fromName":"Michael Forney","fromEmail":"mforney@mforney.org","sentAt":"2020-06-30T11:04:15Z","receivedAt":"2020-06-30T11:04:22Z","isPatch":true,"sender":{"key":"mforney@mforney.org","avatar":"https://avatars.githubusercontent.com/u/52851?v=4"},"body":"On 2020-06-24, Derrick Stolee <stolee@gmail.com> wrote:\n> Perhaps the test I requested in patch 1 is only appropriate\n> here? Or, maybe the test should be test_expect_failure in the\n> first patch and switched to test_expect_success here?\n\nI made a good effort to write a test, but I am still unable to\nreliably trigger the offending codepath, which is:\n\nsubmodule.c:prepare_submodule_summary\nrevision.c:prepare_revision_walk\nrevision.c:limit_list\nrevision.c:process_parents\ncommit.c:repo_parse_commit_gently\ncommit.c:repo_parse_commit_internal (needs !item->object.parsed and\nuse_commit_graph)\ncommit-graph.c:parse_commit_in_graph\ncommit-graph.c:parse_commit_in_graph_one\ncommit-graph.c:fill_commit_in_graph (needs pos >= number of commits in\ncommit-graph in parent repository)\n\nThe trick seems to be ensuring that the parent commit of the first\ncommit in the range of commits changed in a submodule does not get\nparsed during show_submodule_header, is not a loose object in the\nrepository, and has an index in the commit-graph that is larger than\nthe size of the commit-graph in the parent repository. This seems to\ndepend on the order of commits in the commit-graph, which seems to be\nrandom (perhaps based on commit hashes?).\n\nI attached my best attempt at a test to trigger the error. The\nprobability of the test failing correctly (without the fix applied)\nseems to depend on how many commits are present in submodule before\nthe first commit in the range of changed commits. This can be\ncontrolled by adjusting the `seq 1 X` in the for loop. The lowest\nnumber of commits with which I have been able to reproduce the bug is\n3, where it occurs around 1% of the time, and if I set it to 200, I\ncan reproduce the bug around 99% of the time.\n\nI don't really want to spend more time on this than I already have.\nCan the bug fix be applied without a test? If not, hopefully someone\ncan volunteer to craft a reliable test (assuming that this is even\npossible).\n\n-Michael\n"},{"id":"403593","messageId":"CAGw6cBs2O4eLGu=CWNM4G3aL0hjOtxxOuy2wMBadd9o5Wb9iNQ@mail.gmail.com","threadId":"53745","inReplyTo":"CAGw6cBsctN0-BP6k7p71-edsHR4BxJWai4Qz5m5gi4J6pYh=Kw@mail.gmail.com","subject":"Re: [PATCH 2/2] submodule: use submodule repository when preparing summary","fromName":"Michael Forney","fromEmail":"mforney@mforney.org","sentAt":"2020-08-13T21:16:42Z","receivedAt":"2020-08-13T21:16:45Z","isPatch":true,"sender":{"key":"mforney@mforney.org","avatar":"https://avatars.githubusercontent.com/u/52851?v=4"},"body":"On 2020-06-30, Michael Forney <mforney@mforney.org> wrote:\n> I attached my best attempt at a test to trigger the error. The\n> probability of the test failing correctly (without the fix applied)\n> seems to depend on how many commits are present in submodule before\n> the first commit in the range of changed commits. This can be\n> controlled by adjusting the `seq 1 X` in the for loop. The lowest\n> number of commits with which I have been able to reproduce the bug is\n> 3, where it occurs around 1% of the time, and if I set it to 200, I\n> can reproduce the bug around 99% of the time.\n>\n> Can the bug fix be applied without a test? If not, hopefully someone\n> can volunteer to craft a reliable test (assuming that this is even\n> possible).\n\nStill looking for any help with this. It seems pretty clear that this\nis a bug (I am not the only one who has hit this), and I'd really like\nto see the issue fixed. I gave my best shot at a test, but I don't\nthink it's acceptable to commit a test that gives a false positive\nsome percentage of the time.\n\nI see that my patch is still blocked as \"Needs tests\" in the what's\ncooking summary, but I really don't know how to proceed from here.\n"},{"id":"404995","messageId":"xmqqzh667ca4.fsf@gitster.c.googlers.com","threadId":"53745","inReplyTo":"88d8b24c-a0ae-bbbf-dd1f-5adb7a36ee95@gmail.com","subject":"Re: [PATCH 1/2] revision: use repository from rev_info when parsing commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-03T21:58:43Z","receivedAt":"2020-09-03T21:58:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 6/23/2020 4:56 PM, Michael Forney wrote:\n>> This is needed when repo_init_revisions() is called with a repository\n>> that is not the_repository to ensure appropriate repository is used\n>> in repo_parse_commit_internal(). If the wrong repository is used,\n>> a fatal error is the commit-graph machinery occurs:\n>> \n>>   fatal: invalid commit position. commit-graph is likely corrupt\n>> \n>> Since revision.c was the only user of the parse_commit_gently\n>> compatibility define, remove it from commit.h.\n>\n> Is this demonstrable in a test case, to prevent regressions?\n\nIt appears that Michael tried and failed.  Even if we do not\ncurrently have a caller that asks these functions in revision.c to\nwork on a repository that is not the primary one (i.e. in a\nsubmodule), in which case these patches may not be fixing any bug\nthat can be triggered in the current code, it is quite obvious that\nthese functions misbehave once a caller starts asking them to work\non a repository other than the primary one.\n\nSo, given that ... \n\n>\n> I counted 9 copies of parse_commit[_gently]() in my version\n> of revision.c, so it looks like you caught them all.\n\n... we should be able to proceed with the code as-is, I guess.\n\nThanks.\n"},{"id":"405022","messageId":"181e95c7-8b43-e548-7fc9-36fca140645d@gmail.com","threadId":"53745","inReplyTo":"xmqqzh667ca4.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/2] revision: use repository from rev_info when parsing commits","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2020-09-04T12:19:35Z","receivedAt":"2020-09-04T12:19:47Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/3/2020 5:58 PM, Junio C Hamano wrote:\n> Derrick Stolee <stolee@gmail.com> writes:\n> \n>> On 6/23/2020 4:56 PM, Michael Forney wrote:\n>>> This is needed when repo_init_revisions() is called with a repository\n>>> that is not the_repository to ensure appropriate repository is used\n>>> in repo_parse_commit_internal(). If the wrong repository is used,\n>>> a fatal error is the commit-graph machinery occurs:\n>>>\n>>>   fatal: invalid commit position. commit-graph is likely corrupt\n>>>\n>>> Since revision.c was the only user of the parse_commit_gently\n>>> compatibility define, remove it from commit.h.\n>>\n>> Is this demonstrable in a test case, to prevent regressions?\n> \n> It appears that Michael tried and failed.  Even if we do not\n> currently have a caller that asks these functions in revision.c to\n> work on a repository that is not the primary one (i.e. in a\n> submodule), in which case these patches may not be fixing any bug\n> that can be triggered in the current code, it is quite obvious that\n> these functions misbehave once a caller starts asking them to work\n> on a repository other than the primary one.\n> \n> So, given that ... \n> \n>>\n>> I counted 9 copies of parse_commit[_gently]() in my version\n>> of revision.c, so it looks like you caught them all.\n> \n> ... we should be able to proceed with the code as-is, I guess\nYes, I think this is an improvement regardless.\n\nThanks, for the reminder.\n\n-Stolee\n"}]}