Subject: Re: [PATCH] push: disable lazy --force-with-lease by default

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 07 Jul 2017 11:39:34 +0200

On Thu, Jul 06 2017, Junio C. Hamano jotted:

> "git push --force-with-lease=<branch>:<expect>" makes sure that
> there is no unexpected changes to the branch at the remote while you
> prepare a rewrite based on the old state of the branch.  This
> feature came with an experimental option that allows :<expect> part
> to be omitted by using the tip of remote-tracking branch that
> corresponds to the <branch>.
>
> It turns out that some people use third-party tools that fetch from
> remote and update the remote-tracking branches behind users' back,
> defeating the safety relying on the stability of the remote-tracking
> branches.  We have some warning text that was meant to be scary
> sounding in our documentation, but nevertheless people seem to be
> bitten.  cf. https://public-inbox.org/git/[email-masked]/
> for a recent example.
>
> Let's disable the form that relies on the stability of remote-tracking
> branches by default, and allow users who _know_ their remote-tracking
> branches are stable to enable it with a configuration variable.
>
> This problem was predicted from the very beginning; see 28f5d176
> (remote.c: add command line option parser for "--force-with-lease",
> 2013-07-08).
>
> Signed-off-by: Junio C Hamano <[email-masked]>
> ---
>
>  * This is a bit overdue safety fix that we should have done long
>    time ago.  If we had this, I do not think it makes it riskier to
>    forbid --force and tell people to use --force-with-lease.
>
>  Documentation/config.txt   |  5 +++++
>  Documentation/git-push.txt |  5 +++--
>  builtin/send-pack.c        |  5 +++++
>  remote.c                   | 16 ++++++++++++----
>  remote.h                   |  2 ++
>  send-pack.c                |  1 +
>  t/t5533-push-cas.sh        | 19 +++++++++++++++++--
>  transport-helper.c         |  5 +++++
>  transport.c                |  5 +++++
>  9 files changed, 55 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 06898a7498..2f929315a2 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2588,6 +2588,11 @@ new default).
>
>  --
>
> +push.allowLazyForceWithLease::
> +	If set to true, allow the `--force-with-lease` option
> +	without the expected object name (i.e. expecting the objects
> +	at the tip of corresponding remote-tracking branches).
> +

Just a note on the implementation. Re what I mentioned in
[email-masked] it would be more consistent to add a
--lazy-force-with-lease option, and have a corresponding
push.LazyForceWithLease config, which we'd turn off by default.

Then if/when I polish the patch to make CLI options configurable this
doesn't have to be handled by a special case, either by code or in the
mind of users.

But perhaps adding new CLI options is a bit too much of a hassle to
maintain such consistency.