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

From: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
Date: Sep 10, 2020, 14:44 UTC
Message-ID: <20200910144456.GA25125@mail.clickyotomy.dev>
In-Reply-To: <nycvar.QRO.7.76.6.2009101220400.56@tvgsbejvaqbjf.bet>

Hello,

On 09/10/2020 12:22, Johannes Schindelin wrote:

> Hi
>> [...] 
> Maybe `--force-if-incorporated`? Originally, I had in mind to call it
> `--safe-force`, but that might be too vague.

That's nice. I haven't been able to come up with a good name for this
option. So far, I have: `--check-updated-remote-refs` which is really
long and is probably confusing.

Show 6 quoted lines

> 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
> revisions.

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?

Thanks.

-- >8 --
+ static int oid_in_reflog_ent(struct object_id *ooid, struct object_id *noid,
+			     const char *ident, timestamp_t timestamp, int tz,
+			     const char *message, void *cb_data)
+ {
+	struct object_id *remote_oid = cb_data;
+	struct commit *a = lookup_commit_reference(the_repository, noid);
+	struct commit *b = lookup_commit_reference(the_repository, remote_oid);
+	return oideq(noid, remote_oid) || in_merge_bases(b, a);
+ }