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

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

[Francesco Mazzoli]

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.
 +
 --
 
diff --git a/builtin/push.c b/builtin/push.c
index 03846e8..96fc4b2 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -561,6 +561,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 
 	packet_trace_identity("push");
 	git_config(git_push_config, &flags);
+	if (push_always_force_with_lease) {
+		cas.use_tracking_for_rest = 1;
+	}
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 	set_push_cert_flags(&flags, push_cert);
 
diff --git a/cache.h b/cache.h
index 96055c2..d31c972 100644
--- a/cache.h
+++ b/cache.h
@@ -830,6 +830,7 @@ enum push_default_type {
 extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
+extern int push_always_force_with_lease;
 
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
diff --git a/config.c b/config.c
index 1cd40a5..2a0dbe4 100644
--- a/config.c
+++ b/config.c
@@ -1317,6 +1317,10 @@ static int git_default_push_config(const char *var, const char *value)
 		}
 		return 0;
 	}
+	if (!strcmp(var, "push.alwaysforcewithlease")) {
+		push_always_force_with_lease = git_config_bool(var, value);
+		return 0;
+	}
 
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
diff --git a/environment.c b/environment.c
index d40b21f..79186c7 100644
--- a/environment.c
+++ b/environment.c
@@ -53,6 +53,7 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
 enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
 enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
+int push_always_force_with_lease = 0;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
-- 
2.7.4

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

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 03 2017, Francesco Mazzoli jotted:

> The flag can be overridden with `--no-force-with-lease`, or by
> passing the config via the command line.

Thanks for hacking on this. A couple of things:

* Most things (but not all) that configure `git whatevs --some-option`
  are configurable via whatevs.someOption, I think this should follow
  that convention. I.e. be push.forceWithLease not
  push.alwaysForceWithLease.

  See my
  https://public-inbox.org/git/20170324231013.23346-1-avarab@gmail.com/
  patch series for something that went through many of these cases
  (although I see I didn't send it all to list). Anyway, something like
  8/10 of our config variables for switches follow that
  convention. Let's use it for new config.

* It makes sense to also document this the git-push manpage. See
  e.g. how we document --follow-tags:

      Push all the refs that [...] This can also be specified with
      configuration variable push.followTags. For more information, see
      push.followTags in git-config(1).

   You should add something like that to --force-with-lease.

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

Semantically this makes sense, and is exactly how most of these switch
config variables work (and would be how it worked once refactored with
my CLI arg config patch).

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

[Francesco Mazzoli]

On 3 July 2017 at 23:47, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
> On Mon, Jul 03 2017, Francesco Mazzoli jotted:
>
> > The flag can be overridden with `--no-force-with-lease`, or by
> > passing the config via the command line.
>
> Thanks for hacking on this. A couple of things:
>
> * Most things (but not all) that configure `git whatevs --some-option`
>   are configurable via whatevs.someOption, I think this should follow
>   that convention. I.e. be push.forceWithLease not
>   push.alwaysForceWithLease.
>
>   See my
>   https://public-inbox.org/git/20170324231013.23346-1-avarab@gmail.com/
>   patch series for something that went through many of these cases
>   (although I see I didn't send it all to list). Anyway, something like
>   8/10 of our config variables for switches follow that
>   convention. Let's use it for new config.

OK, my worry was about the fact that if I call it like the option I'd better
support the full range of options available to `--force-with-lease`. Anyway,
I'll change the name.

> * It makes sense to also document this the git-push manpage. See
>   e.g. how we document --follow-tags:
>
>       Push all the refs that [...] This can also be specified with
>       configuration variable push.followTags. For more information, see
>       push.followTags in git-config(1).
>
>    You should add something like that to --force-with-lease.

OK, will do.

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

[Francesco Mazzoli]

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 +++++
 Documentation/git-push.txt | 4 +++-
 builtin/push.c             | 3 +++
 cache.h                    | 1 +
 config.c                   | 4 ++++
 environment.c              | 1 +
 6 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 06898a7..835b34b 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.forceWithLease::
+	When true, `--force-with-lease` is the default behavior when
+	using `push --force`. Explicit invocations of `--force-with-lease`
+	or `--no-force-with-lease` take precedence.
 +
 --
 
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 0a63966..5182656 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -195,7 +195,9 @@ only if the "lease" is still valid.
 `--force-with-lease` alone, without specifying the details, will protect
 all remote refs that are going to be updated by requiring their
 current value to be the same as the remote-tracking branch we have
-for them.
+for them. This behavior can be made default using the `push.forceWithLease`
+configuration option. Consult linkgit:git-config[1]. For more
+information.
 +
 `--force-with-lease=<refname>`, without specifying the expected value, will
 protect the named ref (alone), if it is going to be updated, by
diff --git a/builtin/push.c b/builtin/push.c
index 03846e8..d8fa826 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -561,6 +561,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 
 	packet_trace_identity("push");
 	git_config(git_push_config, &flags);
+	if (push_force_with_lease) {
+		cas.use_tracking_for_rest = 1;
+	}
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
 	set_push_cert_flags(&flags, push_cert);
 
diff --git a/cache.h b/cache.h
index 96055c2..5d29e8e 100644
--- a/cache.h
+++ b/cache.h
@@ -830,6 +830,7 @@ enum push_default_type {
 extern enum branch_track git_branch_track;
 extern enum rebase_setup_type autorebase;
 extern enum push_default_type push_default;
+extern int push_force_with_lease;
 
 enum object_creation_mode {
 	OBJECT_CREATION_USES_HARDLINKS = 0,
diff --git a/config.c b/config.c
index 1cd40a5..f1b1e28 100644
--- a/config.c
+++ b/config.c
@@ -1317,6 +1317,10 @@ static int git_default_push_config(const char *var, const char *value)
 		}
 		return 0;
 	}
+	if (!strcmp(var, "push.forcewithlease")) {
+		push_force_with_lease = git_config_bool(var, value);
+		return 0;
+	}
 
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
diff --git a/environment.c b/environment.c
index d40b21f..fb36c1b 100644
--- a/environment.c
+++ b/environment.c
@@ -53,6 +53,7 @@ unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
 enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
 enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
 enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
+int push_force_with_lease = 0;
 #ifndef OBJECT_CREATION_MODE
 #define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
 #endif
-- 
2.7.4

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

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 03 2017, Francesco Mazzoli jotted:

A couple of things I didn't notice at first:

>  	git_config(git_push_config, &flags);
> +	if (push_always_force_with_lease) {
> +		cas.use_tracking_for_rest = 1;
> +	}

This should go in git_push_config.

> +	if (!strcmp(var, "push.alwaysforcewithlease")) {
> +		push_always_force_with_lease = git_config_bool(var, value);
> +		return 0;
> +	}

[As you noted on IRC] --force-with-lease takes args, but yours
doesn't. Arguably this makes no sense whatsoever to have in the config,
but something worth pointing out in the commit message.

We could make the config accept args, you'd call
parse_push_cas_option(), but should we? I don't know.

Should this also apply to send-pack's --force-with-lease? Under the same
option name or send-pack.forceWithLease? I also don't know...

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

[Ævar Arnfjörð Bjarmason]

On Mon, Jul 03 2017, Ævar Arnfjörð Bjarmason jotted:

> On Mon, Jul 03 2017, Francesco Mazzoli jotted:
>
> A couple of things I didn't notice at first:

Oh and also, makes sense to add tests for this, see
t/t5533-push-cas.sh. I.e. just make sure the cases where the option
works work with the config, and that the option overrides the config
etc.

>>  	git_config(git_push_config, &flags);
>> +	if (push_always_force_with_lease) {
>> +		cas.use_tracking_for_rest = 1;
>> +	}
>
> This should go in git_push_config.
>
>> +	if (!strcmp(var, "push.alwaysforcewithlease")) {
>> +		push_always_force_with_lease = git_config_bool(var, value);
>> +		return 0;
>> +	}
>
> [As you noted on IRC] --force-with-lease takes args, but yours
> doesn't. Arguably this makes no sense whatsoever to have in the config,
> but something worth pointing out in the commit message.
>
> We could make the config accept args, you'd call
> parse_push_cas_option(), but should we? I don't know.
>
> Should this also apply to send-pack's --force-with-lease? Under the same
> option name or send-pack.forceWithLease? I also don't know...

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

[Junio C Hamano]

Francesco Mazzoli <f@mazzo.li> writes:

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

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

[Francesco Mazzoli]

On 4 July 2017 at 19:51, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

Could you clarify the danger you're referring to? E.g. give an example
of surprising --force-with-lease behavior that we do not want to
encourage?

Thanks,
Francesco

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

[Junio C Hamano]

On Tue, Jul 4, 2017 at 11:34 PM, Francesco Mazzoli <f@mazzo.li> wrote:
>
> Could you clarify the danger you're referring to? E.g. give an example
> of surprising --force-with-lease behavior that we do not want to
> encourage?

https://public-inbox.org/git/1491617750.2149.10.camel@mattmccutchen.net/

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

[Francesco Mazzoli]

On 5 July 2017 at 09:43, Junio C Hamano <gitster@pobox.com> wrote:
> On Tue, Jul 4, 2017 at 11:34 PM, Francesco Mazzoli <f@mazzo.li> wrote:
>>
>> Could you clarify the danger you're referring to? E.g. give an example
>> of surprising --force-with-lease behavior that we do not want to
>> encourage?
>
> https://public-inbox.org/git/1491617750.2149.10.camel@mattmccutchen.net/

Thanks for clarifying, I had not encountered this because of my git workflow.
I'd also be happy with a config item  that simply disables `--force`
and suggests
`--force-with-lease`, but then we'd need a flag to override that config item
when the user definitely wants to force push.

So we would have something like

* `push.disableForce`: config flag that disables `--force` and suggests
    `--force-with-lease` instead;
* `--disable-force` and `--no-disable-force`, config flags to tune the above
    config parameter at will.

What do you think?

Thanks,
Francesco

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

[Junio C Hamano]

Francesco Mazzoli <f@mazzo.li> writes:

> So we would have something like
>
> * `push.disableForce`: config flag that disables `--force` and suggests
>     `--force-with-lease` instead;
> * `--disable-force` and `--no-disable-force`, config flags to tune the above
>     config parameter at will.
>
> What do you think?

The take-away lesson that the earlier thread gave me was that the
order in which the three options are ranked by their desirebility
in the UI (and the order we would like to encourage users to use)
is, from the most to the least preferrable:

 - "--force-with-lease=<ref>:<expect>" that is safer than "--force";

 - "--force" that is known to be dangerous, and does not pretend to
   be anything but;

 - "--force-with-lease" that pretends to be safer but is not.

The last form should eventually be eliminated, as there is no way to
correctly intuit what the expected object should be.

To me, a disableForce configuration that encourages use of either
the best one or the worst one alone does not look like a step
forward, unless we also have a change to disable the last form.

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

[Francesco Mazzoli]

> On 5 Jul 2017, at 17:17, Junio C Hamano <gitster@pobox.com> wrote:
> 
> The take-away lesson that the earlier thread gave me was that the
> order in which the three options are ranked by their desirebility
> in the UI (and the order we would like to encourage users to use)
> is, from the most to the least preferrable:
> 
> - "--force-with-lease=<ref>:<expect>" that is safer than "--force";
> 
> - "--force" that is known to be dangerous, and does not pretend to
>   be anything but;
> 
> - "--force-with-lease" that pretends to be safer but is not.
> 
> The last form should eventually be eliminated, as there is no way to
> correctly intuit what the expected object should be.

What's not clear to me is what the intended workflow using
`--force-with-lease=<ref>:<expect>` is. Intuitively it seems extremely
cumbersome to manually pluck a revision each time, especially when
dealing with commits that all have the same description.

On the other hand for my workflow `--force-with-lease` works quite well
because I tend to use it in cases where me and a colleague are working
on the same PR, and thus I'm not doing anything else (including fetching).

Moreover, it seems to me that the problem `--force-with-lease` is
just one of marketing. `--force-with-lease` is strictly more "safe"
than `--force` in the sense that it'll reject some pushes that `--force`
will let through. I think that if we advertise it better including its
drawbacks it can still be better than no checks at all.

Francesco

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

[Mike Rappazzo]

On Wed, Jul 5, 2017 at 12:43 PM, Francesco Mazzoli <f@mazzo.li> wrote:
>> On 5 Jul 2017, at 17:17, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> The take-away lesson that the earlier thread gave me was that the
>> order in which the three options are ranked by their desirebility
>> in the UI (and the order we would like to encourage users to use)
>> is, from the most to the least preferrable:
>>
>> - "--force-with-lease=<ref>:<expect>" that is safer than "--force";
>>
>> - "--force" that is known to be dangerous, and does not pretend to
>>   be anything but;
>>
>> - "--force-with-lease" that pretends to be safer but is not.
>>
>> The last form should eventually be eliminated, as there is no way to
>> correctly intuit what the expected object should be.
>
> What's not clear to me is what the intended workflow using
> `--force-with-lease=<ref>:<expect>` is. Intuitively it seems extremely
> cumbersome to manually pluck a revision each time, especially when
> dealing with commits that all have the same description.
>
> On the other hand for my workflow `--force-with-lease` works quite well
> because I tend to use it in cases where me and a colleague are working
> on the same PR, and thus I'm not doing anything else (including fetching).
>
> Moreover, it seems to me that the problem `--force-with-lease` is
> just one of marketing. `--force-with-lease` is strictly more "safe"
> than `--force` in the sense that it'll reject some pushes that `--force`
> will let through. I think that if we advertise it better including its
> drawbacks it can still be better than no checks at all.
>
> Francesco

I am in your camp on this, and I will also only ever explicitly fetch.
I would hate for --force-with-lease to disappear.

However, I believe that the problem is that there are many third party
tools which do a fetch behind the scenes (for example Atlassian
SourceTree).  This can update the local refs without a user
necessarily thinking about it.  This can lead to a force-with-lease
being used unsafely (without the stated lease).

_Mike

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

[Junio C Hamano]

Francesco Mazzoli <f@mazzo.li> writes:

> Moreover, it seems to me that the problem `--force-with-lease` is
> just one of marketing. `--force-with-lease` is strictly more "safe"
> than `--force` in the sense that it'll reject some pushes that `--force`
> will let through.

By that logic, a hypothetical update to `--force` that makes 1/3 of
the attempted forced push randomly would make it safer than the
current `--force`, wouldn't it?

When third-party tools fetch and update remote-tracking branches
behind the users' back, the safety based on the stability of
remote-tracking branches are defeated.  And the biggest problem
is that the way `--force-with-lease` misbehaves---it is not like
it randomly and mistakenly stops the push that could go through;
it lets through what shouldn't.

See the other patch I sent just now---with something like that patch
that lets those like you, who know their remote-tracking branches
are reliable, use the lazy form, while disabling it by default for
others (until they examine their situation and perhaps disable the
problematic auto-fetching) in place, I do not think it is a bad idea
to advertise --force-with-lease a safer option than --force (because
those for whom it is not safer will not be able to use it).

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

[Francesco Mazzoli]

On 6 July 2017 at 21:13, Junio C Hamano <gitster@pobox.com> wrote:
> By that logic, a hypothetical update to `--force` that makes 1/3 of
> the attempted forced push randomly would make it safer than the
> current `--force`, wouldn't it?

It would. However, this additional safety is not really meaningful to any
workflow, while the one added by `--force-with-lease` is.

> When third-party tools fetch and update remote-tracking branches
> behind the users' back, the safety based on the stability of
> remote-tracking branches are defeated.  And the biggest problem
> is that the way `--force-with-lease` misbehaves---it is not like
> it randomly and mistakenly stops the push that could go through;
> it lets through what shouldn't.
>
> See the other patch I sent just now---with something like that patch
> that lets those like you, who know their remote-tracking branches
> are reliable, use the lazy form, while disabling it by default for
> others (until they examine their situation and perhaps disable the
> problematic auto-fetching) in place, I do not think it is a bad idea
> to advertise --force-with-lease a safer option than --force (because
> those for whom it is not safer will not be able to use it).

Fair enough, I'm OK with enabling it with some config. I'd still like a
way to enable it by default if I want though.

Thanks,
Francesco

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

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 05 2017, Junio C. Hamano jotted:

> On Tue, Jul 4, 2017 at 11:34 PM, Francesco Mazzoli <f@mazzo.li> wrote:
>>
>> Could you clarify the danger you're referring to? E.g. give an example
>> of surprising --force-with-lease behavior that we do not want to
>> encourage?
>
> https://public-inbox.org/git/1491617750.2149.10.camel@mattmccutchen.net/

In the context of this patch I don't understand why you're concerned
that making --force mean --force-with-lease makes things worse.

See my
https://public-inbox.org/git/CACBZZX48RanjHsv1UsnxkbxRtqKRGgMcgmtVqQmR84H5j8awqQ@mail.gmail.com/
(follow-up to the E-Mail you posted):

    To me the *main* feature of --force-with-lease is that it's less
    shitty than --force, without imposing too much UI overhead. We have to
    be really careful not to make --force-with-lease so complex by default
    that people just give u and go back to using --force, which would be
    worse than either whatever current problems there are with the
    current --force-with-lease behavior, or anything we replace it with.

I.e. yes there are workflows with some background auto-update that will
make it less safe, which I documented in f17d642d3b ("push: document &
test --force-with-lease with multiple remotes", 2017-04-19).

But it is still the case that --force-with-lease is categorically a more
safer option than simply --force, which has none of the safety
--force-with-lease has. It would still wipe away history in this
scenario you're pointing out *and others*.

Surely the point of having an option like this is to have a net
reduction in complexity.

I think it can be argued that it's bad UI design though to have --force
mean different things depending on the config, and we'd be better off
with a patch that disables --force.

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

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 5, 2017 at 1:26 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Wed, Jul 05 2017, Junio C. Hamano jotted:
>
>> On Tue, Jul 4, 2017 at 11:34 PM, Francesco Mazzoli <f@mazzo.li> wrote:
>>>
>>> Could you clarify the danger you're referring to? E.g. give an example
>>> of surprising --force-with-lease behavior that we do not want to
>>> encourage?
>>
>> https://public-inbox.org/git/1491617750.2149.10.camel@mattmccutchen.net/
>
> In the context of this patch I don't understand why you're concerned
> that making --force mean --force-with-lease makes things worse.
>
> See my
> https://public-inbox.org/git/CACBZZX48RanjHsv1UsnxkbxRtqKRGgMcgmtVqQmR84H5j8awqQ@mail.gmail.com/
> (follow-up to the E-Mail you posted):
>
>     To me the *main* feature of --force-with-lease is that it's less
>     shitty than --force, without imposing too much UI overhead. We have to
>     be really careful not to make --force-with-lease so complex by default
>     that people just give u and go back to using --force, which would be
>     worse than either whatever current problems there are with the
>     current --force-with-lease behavior, or anything we replace it with.
>
> I.e. yes there are workflows with some background auto-update that will
> make it less safe, which I documented in f17d642d3b ("push: document &
> test --force-with-lease with multiple remotes", 2017-04-19).
>
> But it is still the case that --force-with-lease is categorically a more
> safer option than simply --force, which has none of the safety
> --force-with-lease has. It would still wipe away history in this
> scenario you're pointing out *and others*.
>
> Surely the point of having an option like this is to have a net
> reduction in complexity.

I mean reduction in risk...

> I think it can be argued that it's bad UI design though to have --force
> mean different things depending on the config, and we'd be better off
> with a patch that disables --force.