Re: [PATCH] commit-reach: fix in_merge_bases_many bug

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> The fix itself is to swap min_generation with a max_generation in
> repo_in_merge_bases_many().
>
> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Reported-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>

The order of these looked iffy so I rearranged them in chrono order,
Srinidhi's series found it broken, and with Dscho's help, fix was
produced and you sent it with your sign off.

>  commit-reach.c        |  8 ++++----
>  t/helper/test-reach.c |  2 ++
>  t/t6600-test-reach.sh | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 4 deletions(-)

I've applied this and then rebased Srinidhi's latest on top, with
the following to re-enable the commit-graph in the codepath.  t5533
and t6600 passes and when the fix in the message I am responding to
is reverted, t5533 again fails.

Thanks.

    SQUASH??? drop "hide the breakage under the rug" hack
    
    Derrick and Dscho spotted and fixed the incorrect optimization in
    in_merge_bases_many() when the commit-graph is in use, so there is
    no longer a reason why we would want to disable commit-graph locally
    in this codepath.

diff --git a/remote.c b/remote.c
index 98a578f5dc..8c91188677 100644
--- a/remote.c
+++ b/remote.c
@@ -2405,34 +2405,18 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
 	return ret;
 }
 
-/* Toggle the "commit-graph" feature; return the previously set state. */
-static int toggle_commit_graph(struct repository *repo, int disable) {
-	int prev = repo->commit_graph_disabled;
-	repo->commit_graph_disabled = disable;
-	return prev;
-}
-
 /*
  * Check for reachability of a remote-tracking
  * ref in the reflog entries of its local ref.
  */
 static void check_if_includes_upstream(struct ref *remote)
 {
-	int prev;
 	struct ref *local = get_local_ref(remote->name);
 	if (!local)
 		return;
 
-	/*
-	 * TODO: Remove "toggle_commit_graph()" calls around the check.
-	 * Depending on whether "commit-graph" enabled or not,
-	 * "in_merge_bases_many()" returns different results;
-	 * disable it temporarily when the check runs.
-	 */
-	prev = toggle_commit_graph(the_repository, 1);
 	if (is_reachable_in_reflog(local->name, remote) <= 0)
 		remote->unreachable = 1;
-	toggle_commit_graph(the_repository, prev);
 }
 
 static void apply_cas(struct push_cas_option *cas,