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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 1, 2020, 18:32 UTC
Message-ID: <xmqqtuvdbxvu.fsf@gitster.c.googlers.com>
In-Reply-To: <20201001175443.GA28444@mail.clickyotomy.dev>

Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> 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.

Show 10 quoted lines

> 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 ;-)