Re: [PATCH v9 0/3] push: add "--[no-]force-if-includes"

From

Junio C Hamano <gitster@pobox.com>

Date

Oct 1, 2020, 17:12 UTC

Message-ID

<xmqq4knddg5v.fsf@gitster.c.googlers.com>

In-Reply-To

<xmqqlfgqc5kq.fsf@gitster.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

> Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> writes:
>
>> Changes since v8:
>>   - Disable "commit-graph" when "in_merge_bases_many()" is called
>>     for this check, because it returns different results depending
>>     on whether "commit-graph" is enabled [1].
>
> Is that a wise move, though?  If the "different results" is
> expected, then it is a different story, but I would think it is a
> bug in commit-graph codepath if it produces a result different from
> what the callers expect, and disabling from the caller's end would
> mean that we lose one opportunity to help commit-graph folks to go
> and fix their bugs, no?
>
> Other than that, I think the topic is in quite a good shape.  Thanks
> for working on polishing it.

In other words, how about doing it like so.

 remote.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git i/remote.c w/remote.c
index 98a578f5dc..361b8f1c0e 100644
--- i/remote.c
+++ w/remote.c
@@ -2408,7 +2408,20 @@ static int is_reachable_in_reflog(const char *local, const struct ref *remote)
 /* 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;
+	static int should_toggle = -1;
+
+	if (should_toggle < 0) {
+		/*
+		 * The in_merge_bases_many() seems to misbehave when
+		 * the commit-graph feature is in use.  Disable it for
+		 * normal users, but keep it enabled when specifically
+		 * testing the feature.
+		 */
+		should_toggle = !git_env_bool("GIT_TEST_COMMIT_GRAPH", 0);
+	}
+
+	if (should_toggle)
+		repo->commit_graph_disabled = disable;
 	return prev;
 }