Re: [PATCH] push: make `--force-with-lease[=<ref>]` safer

From: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>, git@vger.kernel.org
Subject: Re: [PATCH] push: make `--force-with-lease[=<ref>]` safer
Date: Wed, 16 Sep 2020 11:01:41 +0530	[thread overview]
Message-ID: <20200916053141.GA29901@mail.clickyotomy.dev> (raw)
In-Reply-To: <xmqqk0wwjf4p.fsf@gitster.c.googlers.com>

Hi Junio,

On 09/14/2020 13:08, Junio C Hamano wrote:
>>> [...]
>>>
>>> Good catch. Would adding in_merge_bases() along with checking if OIDs
>>> are equal for each reflog entry in oid_in_reflog_ent() address the
>>> problem? That way, we would check if remote ref is reachable from
>>> one of the entries?
> 
> That sounds very expensive.  

Yes, you're right about that.

> If we switched to check the reflog of HEAD (and not any particular
> local branch's reflog), then "has this ever been checked out", tests
> would suffice, no?  We detach at the tip of the remote-tracking
> branch and then reapply our work one by one in such a case, so we
> should have the record of having their tip in the working tree at
> some point (i.e. at the beginning of the "rebase" phase of the "pull
> --rebase" operation).

Interesting, I think that might work! Since HEAD moves around and
records all the movements, if the remote was ever checked out there
should be an entry in the reflog. I guess we could stick to "oideq()"
for each entry if we're going this way. However, I think we should
keep that one "in_merge_bases()" call at the beginning to check if
we can can skip checking the reflog if the remote ref is an ancestor
of the local ref.

>>>> On 09/10/2020 12:22, Johannes Schindelin wrote:
>>>>
>>>> BTW I think the patch needs to cover a bit more, still: after I run `git
>>>> pull --rebase`, the local branch will never have been at the same revision
>>>> as the fetched one: `git rebase` moves to an unnamed branch before
>>>> replaying the patches. So I think we need to see whether the remote tip
>>>> was _reachable_ from (not necessarily identical to) any of the reflog's
>>>> revision

Also, there would be an entry for what Johannes describes above -- when
a `git pull --rebase` is run. Will change to this in the next series.

Thanks.
-- 
Srinidhi Kaushik