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

[Junio C Hamano <gitster@pobox.com>]

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

> to the updated tip of the remote instead. The condition:
>
> 	if (!oideq(&ref->old_oid, &ref->old_oid_expect))
>
> would evaluate false since we're using `use_tracking`. This essentially
> reduces the behavior of `--force-with-lease=<ref>` to `--force` in this
> scenario.

Yes, that is exactly why I have kept saying that the form without
<expect> is not safe and cannot be made safer especially when
auto-fetching is involved.

> Right, I agree with what is described above. But, in this patch, we are
> looking at the reflog of the _local_ branch that is going to be updated
> on the remote side. The point of going through the reflog is to see if
> the current tip its remote-tracking branch is present in one of the
> reflog entries implying that any new changes (pushes from another user)
> in the meantime aren't ignored and overwritten with our push.
> Would that be an incorrect assumption?

I am afraid it is.  You may have looked at it and even kept it
locally, to be looked at later, without letting it affect what you
are going ot force-push in any way.  You might later come back and
resurrect their change on top of what you force pushed, or more
likely you may simply forget, especially if the forced push goes
through without failing.

> Yes, that sounds like a good way to mitigate this issue; but that being
> said, setups where `--force-with-lease` is being used as an alias for
> `--force` should probably be taken into consideration though.

We cannot help, and it is not our job to dispel, misconfigurations
and misconceptions caused by following bad pieces of advice other
people gave our users, though.