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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 15, 2017, 10:45 UTC
Message-ID: <87k23a2d28.fsf@gmail.com>
In-Reply-To: <xmqqlgo0cm7f.fsf@gitster.mtv.corp.google.com>

On Fri, Jul 07 2017, Junio C. Hamano jotted:

[Re-flowing & re-quoting some of this for clarity]

Show 7 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Which is why I think we should take Francesco's patch (with fixes from
>> feedback), instead of Junio's.
>
> The patch in this discussion is not meant as a replacement for the
> one from Francesco.  It was meant as a companion patch.

Okey, so per the table I laid out in 8760f4bmig.fsf@gmail.com:

Show 10 quoted lines

>> The concern I have with Junio's patch above (but I like Francesco
>> Mazzoli's approach better) is that the safety of the various --force
>> options, from least safe to most safe, is:
>>
>>  1. --force: You blow away the remote history, no idea what's there, or
>>     if your local ref mirrors what you just wiped.
>>
>>  2. --force-with-lease: Even if you have a `git fetch` in the
>>      background, at least if you wipe a remote ref you have a copy in a
>>      local reflog to restore it.

So with your patch you won't get #2 at all unless you set push.allowLazyForceWithLease=true.

Once Francesco's patch is also applied (or some version thereof) you can then set push.AlwaysForceWithElease to make --force mean --force-with-lease, which is disabled by default.

I think this is really crappy UX design. Now in some future version of Git I need to set a config option *and* type a longer option name to get behavior that's an improvement over --force.

To get the --force-with-lease behavior by default on --force I'll need to set two options, one to alias it, one to allow the option it'll be aliased to.

Show 7 quoted lines

> As I view the form of the option that relies on the stability of
> remote-tracking branches strictly worse than the honest "--force"
> that loudly advertises itself as dangerous (as opposed to being
> advertised as a safer option, when it isn't), I consider the change
> to require users to opt into relying on remote-tracking branches as
> a prerequisite before we can recommend the form as a safer version
> of "--force".

How is it being advertised as strictly safer without explaining the caveats after my f17d642d3b ("push: document & test --force-with-lease with multiple remotes", 2017-04-19)? I think we now do a very good job of describing the caveats involved, but maybe I missed something.

If you think the documentation is now over-promising can you point out what parts, so I can fix them.

I think we're really losing the forest for the trees here.

I've had to help a lot of people (mainly inexperienced people @ work) after they --force pushed something.

It would be a *huge* improvement if git shipped with a default such that I *knew* that whatever they just wiped away could be found in their local reflog, right now you need to ssh to the git server and dig around there to see what they wiped away.

Most of these people are not running some auto-fetch editor thingy the presence of which is is your motivation for removing the lazy --force-with-lease, and even if they were these people would also benefit from being able to run e.g. `git reflog refs/remotes/origin/topic` to see what they just nuked once someone more experienced shows up and tries to recover from their mistake.

So I wish we could make --force eventually mean --force-with-lease, but it sounds as though you'd like to hide that behind two config options.