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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 28, 2020, 19:51 UTC
Message-ID: <xmqq4knhn0he.fsf@gitster.c.googlers.com>
In-Reply-To: <20200928193400.GA88208@mail.clickyotomy.dev>

Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> writes:

Show 28 quoted lines

>> That build runs the test suite with a bunch of GIT_TEST_* knobs
>> enabled, and the last two tests added in this series fail when run as:
>> 
>>   GIT_TEST_COMMIT_GRAPH=1 ./t5533-push-cas.sh
>  
> Thanks for the heads-up. It turns out that "in_merge_bases_many()"
> returns different results depending on "GIT_TEST_COMMIT_GRAPH".
> Initially I thought that it might be related to batching the entries,
> but that is not the case.
>
> One of the tests that is failing is:
>   cd src &&
>   git switch branch &&
>   test_commit I &&
>   git switch master &&
>   test_commit J &&
>   git pull --rebase origin master &&
>   git push --force-if-includes --force-with-lease="master"
>
> Here, we are testing to check if forced updates are allowed after
> the remote changes have been incorporated locally, which is true
> in this case and should pass.
>
> "in_merge_bases_many()" used in the check as follows:
> ...
> Unfortunately, I am unfamiliar with the code, and not sure why this
> happens; I remember Junio mention [2] something about generation
> numbers could it be related to that?

Now it's time to summon the commit-graph folks. I think we should assume that it a bug in the code with commit-graph if it produces a result that is different from the code without, until we prove otherwise (e.g. in a history with clock-skew, traditional traversal of A..B could give a wrong result where commit-graph may produce a correct result. I however think the topology-based merge-base computation does not suffer from the same issue).

Thanks.