Plain --force-with-lease considered dangerous on the Git list

[gergelypolonkai]

I was thinking that since Magit’s default is to use --force-with-lease when i tell it to P -f P, can i set it as the default in the git CLI?

As it turns, i’m not the first to think of that, XonqNopp already asked this on the mailing list (emphasis mine in all quotes):

I have discovered some months ago that it is more safe to force a push with
git push --force-with-lease

But since then I keep wondering: why is this not the default?

And Junio C Hamano replied, among other things:

Because the form "--force-with-lease=" is a
strict improvement over "--force", but the lazy "guess where the
other end should be" form, i.e. "--force-with-lease" alone, is worse
than "--force" in that it gives users false sense of security
without offering the safety the "lease" mechanism gives.

Johannes Schindelin added that

In this instance, --force-with-lease is very dangerous.

We did introduce the --force-if-includes option, but it seems that we
made this option not imply --force-with-lease, which makes it an
unwieldy git push --force-with-lease --force-if-includes <refspec>,
which admittedly not even I type, and I really would benefit from this
option.

Reading through the man page of git i strongly agree with Johannes here; what do you think of adding force-if-includes to the -f switch of Magit? Would it be more safe? Would it even be feasible?

[tarsius]

It all depends on the workflow:

A general note on safety: supplying this option without an expected
value, i.e. as --force-with-lease or --force-with-lease=<refname>
interacts very badly with anything that implicitly runs git fetch
on the remote to be pushed to in the background, e.g.  git fetch
origin on your repository in a cronjob.

The protection it offers over --force is ensuring that subsequent
changes your work wasn’t based on aren’t clobbered, but this is
trivially defeated if some background process is updating refs in
the background. We don’t have anything except the remote tracking
info to go by as a heuristic for refs you’re expected to have seen
& are willing to clobber.

If you don't do that, then --force-with-lease isn't dangerous. Magit doesn't do that, but of course we cannot rule out that some Magit users use additional tools, which do that.

So I think Magit should enable --force-if-included by default. All it does is make --force-with-lease safer. If --force-with-lease isn't used then --force-if-included has no effect:

--[no-]force-if-includes
Force an update only if the tip of the remote-tracking ref has been
integrated locally.

This option enables a check that verifies if the tip of the
remote-tracking ref is reachable from one of the "reflog" entries
of the local branch based in it for a rewrite. The check ensures
that any updates from the remote have been incorporated locally by
rejecting the forced update if that is not the case.

If the option is passed without specifying --force-with-lease, or
specified along with --force-with-lease=<refname>:<expect>, it is a
"no-op".

The question is how do we enable it by default. We could add it to the push popup and enable it by default. Most users would be confused by that and that is unfortunate because I expect most users would not actually be affected by this at all (because they don't do such a silly thing as pushing while fetching). So that would be a net negative.

So I think we should just silently and unconditionally add that argument. Some users will notice and wonder about it, but I hope that doing it like this increases the odds that they will look up the documentation before asking others to explain it to them.

IMO --force-with-lease should by default behave as if --force-if-includeswas given, i.e. users would have to use --no-force-if-includes (or set some option) to opt-in to the strange middle-ground that --force-with-lease currently offers by default.

[kyleam]

So I think Magit should enable --force-if-included by default

Just a note: --force-if-includes isn't available until Git v2.30.

So I think Magit should enable --force-if-included by default. All it does is make --force-with-lease safer.
[...]
So I think we should just silently and unconditionally add that argument.

--force-if-included can block certain workflows. In some situations, I use --force-with-lease when I've inspected the remote-tracking branch, perhaps incorporating some of the changes in a way that does not bring the remote-tracking branch's tip into the local branch's reflog (e.g. non-ff cherry-picking). At that point, I know how the two branches differ, and I'm happy with forcing the push if I have the assurance that the ref-to-be-updated matches what I have locally as a remote-tracking branch. So, as a deliberate/manual fetcher, --force-with-lease is sufficient for me, but if --force-if-included is added automatically and unconditionally, that push would now fail.

Perhaps a gentler first step would be to suggest somewhere in the documentation that users consider setting push.useForceIfIncludes=true.

[tarsius]

Perhaps a gentler first step would be to suggest somewhere in the documentation that users consider setting push.useForceIfIncludes=true.

I overlooked that. Yes that sounds very reasonable.

[kyleam]

I've pushed km/manual-force-with-lease-note, which adds a note to the push section of the manual. I'll wait a day or two for any comments and then bring that into master.