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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 7, 2017, 09:54 UTC
Message-ID: <8760f4bmig.fsf@gmail.com>
In-Reply-To: <1n8sh3u.1lsabkd1pislrwM%lists@haller-berlin.de>

On Fri, Jul 07 2017, Stefan Haller jotted:

Show 19 quoted lines

> Junio C Hamano <gitster@pobox.com> wrote:
>
>> 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.
>
> Third-party tools are not the only problem. They may make the problem
> more likely to occur, but it can also happen without them. (See below.)
>
>> 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.
>
> I'm wondering if people who claim they know they are safe really do.
> Elsewhere in the other thread somebody said "I only ever explicitly
> fetch, so I know I'm safe". Are you sure?
>
> Consider this example:

Both of your examples explicitly fetch. Yes this could be confusing to someone who doesn't understand that "git fetch" doesn't just fetch the current remote branch, but all branches.

> What I'm getting at is that there's a lot of things that you have to
> remember to not do in order to make --force-with-lease without parameter
> a useful tool.

Fully agreed, it's confusing, but it's less shitty than --force.

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.

 3. --force-with-lease=master:origin/master: More explicit, but still
     subject to the caveat with background fetching.

 4. --force-with-lease=master:<manually copied sha1>: You know exactly
     what you're wiping, and have likely reviewed that exact commit.

Yes, #4 is the safest, #2 & #3 are similar but subject to various caveats with background fetching / users not realizing "git pull" fetches everything etc.

But I think we have to keep our eye on the ball here. Which is to enact a net increase in user safety.

Right now most users who want to force a remote branch just use --force. E.g. Stack Overflow shows >100k results for git + --force, but just 500 for git + --force-with-lease.

You and others are rightly pointing out that --force-with-lease has lots of caveats, but that as an argument-less flag is something we could (with Francesco patch) turn on by default as a --force replacement.

This would leave users better off than they were before, because now when they accidentally wipe something they at least have a local copy if they did the wrong thing.

Moving everyone from #1 to #2 would be a net increase in user safety without more complex UX. Not having #2 would, for a lot of users who'd otherwise be happy to use #2, mean they'll just use #1 (the least safe option!) instead of the more ideal #4.

Which is why I think we should take Francesco's patch (with fixes from feedback), instead of Junio's.