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

From: Junio C Hamano <[email-masked]>
Date: Thu, 01 Oct 2020 11:32:21 -0700

Srinidhi Kaushik <[email-masked]> writes:

> Ah, this will keep a record of the behavior in the tests; nice,
> will update in the next set.

I actually think this kind of "sweep the breakage under the rug"
belongs not in the code that uses the (suspected broken) API but
in the code that is suspected to be broken, i.e. somewhere in
commit.c that is involved in in_merge_bases_many() codepath.

So if you are going to reroll, I would rather see you drop this part
entirely, and add a similar "this code is broken so we won't run in
the real user's environment" to protect all code that call
in_merge_bases_many() from the breakage, not just your new one.

> One of the failing tests in t5533 was modified (for debugging), and
> the following shows the difference in behavior of
> "repo_in_merge_bases_many()". The test checks if the remote tip is
> reachable from the reflog entries of the local branch after
> "git pull --rebase" is run.
> ...
> It looks like G (b995a30) is reachable from J (7c16010), but
> "repo_in_merge_bases_many()" disagrees when "GIT_TEST_COMMIT_GRAPH"
> is enabled. I hope this information helps a little to understand
> why this happens and whether it is a bug or not.

Wonderful.  With this good start, perhaps we may not have to discuss
disabling the commit-graph code in your code or in commit.c for all
callers ;-)