Re: [PATCH] push: add config option to --force-with-lease by default.

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 4, 2017, 17:51 UTC
Message-ID: <xmqqvan8jdje.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1499116727-757-1-git-send-email-f@mazzo.li>

Francesco Mazzoli <f@mazzo.li> writes:

Show 27 quoted lines

> The flag can be overridden with `--no-force-with-lease`, or by
> passing the config via the command line.
>
> Signed-off-by: Francesco Mazzoli <f@mazzo.li>
> ---
>  Documentation/config.txt | 5 +++++
>  builtin/push.c           | 3 +++
>  cache.h                  | 1 +
>  config.c                 | 4 ++++
>  environment.c            | 1 +
>  5 files changed, 14 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 06898a7..36fe882 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2537,6 +2537,11 @@ push.default::
>  	specific workflows; for instance, in a purely central workflow
>  	(i.e. the fetch source is equal to the push destination),
>  	`upstream` is probably what you want.  Possible values are:
> +
> +push.alwaysforcewithlease::
> +	When true, `--force-with-lease` is the default behavior when
> +	using `push --force`. Explicit invocations of `--force-with-lease`
> +	or `--no-force-with-lease` if present, take precedence.
>  +
>  --

I suspect this may be going in a wrong direction.

People have been burned by the lazy "--force-with-lease" that does not say what object to expect there and forces the command to DWIM incorrectly what the remote's ref ought to be pointing at. This change encourages its use without the user being painfully aware of that danger. Whenever you say "push --force", you'd be using the dangerous "--force-with-lease" that does not specify what the expected current state of the remote is. The end result gives an illusion of being safer than a simple "--force", without being not really safer.

I'd understand more if there were two new (and orthogonal) options, though:

 - disable the use of "--force" option, telling the user to use
   "--force-with-lease=<object>" instead.

 - disable the DWIM based on the remote-tracking branches when
   "--force-with-lease[=<refname>]" is used, i.e. error out when the
   option is used without a specific object to expect.