Would a config var for --force-with-lease be useful?

Would a config var for --force-with-lease be useful?

[Scott Johnson]

Hello Everyone:

I'm considering writing a patch that adds a configuration variable
that will allow the user to default the command:

git push --force

to:

git push --force-with-lease

As discussed here:

https://stackoverflow.com/questions/30542491/push-force-with-lease-by-default

Now, I understand that there are downsides to having this enabled,
namely that a user who has this enabled might forget that they have it
enabled, and, as such, on a machine that _doesn't_ have it enabled (of
which they are unfamiliar) might then run the more consequential
command "git push --force", but my thinking is that adding this as a
feature to the git codebase as an _optional_ (i.e. not enabled by
default) configuration variable would then save some of us who use a
"rebase-then-force-push for pull request" workflow some time and
headaches.

Of course, I don't want to submit a patch if this is a feature that
isn't likely to be accepted, so I wanted to get some thoughts from the
mailing list regarding this idea.

Thank you,

~Scott Johnson

Re: Would a config var for --force-with-lease be useful?

[Constantin Weißer]

I think there are two aspects to using "force with lease".

Firstly, you, a person aware of the option, using it. In this case I
think an alias is very fitting, because you get quickly used to just
typing `git pf` or so. Plus, you don't have the disadvantage you
described: if you’re working on a machine without your alias, you’ll
just notice immediately and type the full option.

The other aspect is working in a team. The problem there is, that most
(at least in my surroundings) use plain --force and you have to make
them aware of --force-with-lease. But with an option or an alias, you
depend on them using force with lease instead of plain force, so again I
don't really see the advantage of such an option.

And lastly, a question: say you are using your proposed option and it is
turned on. Now, git refuses to push, you clarify the situation and
actually mean to push --force now. How would you do this? 1) turn off 2)
push 3) turn option on again?

Regards,
Constantin

Quoting Scott Johnson (2018-08-24 18:39:27)
> Hello Everyone:
> 
> I'm considering writing a patch that adds a configuration variable
> that will allow the user to default the command:
> 
> git push --force
> 
> to:
> 
> git push --force-with-lease
> 
> As discussed here:
> 
> https://stackoverflow.com/questions/30542491/push-force-with-lease-by-default
> 
> Now, I understand that there are downsides to having this enabled,
> namely that a user who has this enabled might forget that they have it
> enabled, and, as such, on a machine that _doesn't_ have it enabled (of
> which they are unfamiliar) might then run the more consequential
> command "git push --force", but my thinking is that adding this as a
> feature to the git codebase as an _optional_ (i.e. not enabled by
> default) configuration variable would then save some of us who use a
> "rebase-then-force-push for pull request" workflow some time and
> headaches.
> 
> Of course, I don't want to submit a patch if this is a feature that
> isn't likely to be accepted, so I wanted to get some thoughts from the
> mailing list regarding this idea.
> 
> Thank you,
> 
> ~Scott Johnson

Re: Would a config var for --force-with-lease be useful?

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 5263 bytes --]

Hi,

On Sat, 25 Aug 2018, Constantin Weißer wrote:

> I think there are two aspects to using "force with lease".

There is a third, very, very important aspect.

When you use --force-with-lease (and I, for one, do, all the time), keep
in mind that it assumes that you are at least aware of the latest fetched
ref.

Let's add a little color to this, to make it less theoretical, and more
relateable. Let's assume that you work on a branch, say,
`make-this-thing-work`, and you have to work on this branch on two
different machines, because it is a finicky thing you need to make work.
Let's add even more color by saying that one of your machines is a windows
machine, and the other one an old IRIX machine. We will call them
`windoze` and `irixxx`.

The common use case for `--force-with-lease`, at least as far as I know,
is that this branch is pushed from one of these setups first, say,
`windoze`, and fetched on the other, then a fixup is necessary and you
rebase the branch with `--autosquash` to make it compile on IRIX, and then
you push with `--force-with-lease` a day later, just in case that you
forgot to re-fetch something that you did on the `windoze` machine.

(Something like this happened to me recently, where one of my branches did
not compile in a Linux VM with an older cURL, and I had to fix a
Windows-targeting branch to keep it cross-platform)

The `--force-with-lease` option helps here, and quite a bit.

It would totally lose its usefulness, though, if you use a tool that
auto-fetches those remote branches. For example, "synchronizing" in
<name-your-favorite-git-gui-here>.

In that case, you did not see what was fetched, and you might have missed
updates, and you will overwrite them, even if you tried to be careful by
using `--for-ce-with-lease`.

I proposed, a couple of months ago, to either fix `--force-with-lease`, or
to deprecate it in favor of a new option, with a new behavior. The new
behavior would look at the *reflog*, much as the `--fork-point` option of
`git rebase`: in addition to the regular `--force-with-lease` server-side
checks, a client-side check *first* verifies that the remote-tracking
branch is reachable at least from *one* of the items in the reflog of the
branch we are about to push.

That is, it would ensure that even if we rebased locally, we did
incorporate the tip commit of the remote-tracking branch, at some stage.

Granted, there are probably cases where you would fetch, look at the
remote-tracking branch, and reject those changes without integrating those
into the local branch. In that case, you would want to relax to the
current behavior of `--force-with-lease`. But I would expect that to
happen only rarely.

The safety by the proposed behavior would make it a lot easier to accept a
config setting that makes this the default.

I guess that is the reason why that config setting does not exist yet: we
would want to have that new behavior in place first...

Ciao,
Johannes

> 
> Firstly, you, a person aware of the option, using it. In this case I
> think an alias is very fitting, because you get quickly used to just
> typing `git pf` or so. Plus, you don't have the disadvantage you
> described: if you’re working on a machine without your alias, you’ll
> just notice immediately and type the full option.
> 
> The other aspect is working in a team. The problem there is, that most
> (at least in my surroundings) use plain --force and you have to make
> them aware of --force-with-lease. But with an option or an alias, you
> depend on them using force with lease instead of plain force, so again I
> don't really see the advantage of such an option.
> 
> And lastly, a question: say you are using your proposed option and it is
> turned on. Now, git refuses to push, you clarify the situation and
> actually mean to push --force now. How would you do this? 1) turn off 2)
> push 3) turn option on again?
> 
> Regards,
> Constantin
> 
> Quoting Scott Johnson (2018-08-24 18:39:27)
> > Hello Everyone:
> > 
> > I'm considering writing a patch that adds a configuration variable
> > that will allow the user to default the command:
> > 
> > git push --force
> > 
> > to:
> > 
> > git push --force-with-lease
> > 
> > As discussed here:
> > 
> > https://stackoverflow.com/questions/30542491/push-force-with-lease-by-default
> > 
> > Now, I understand that there are downsides to having this enabled,
> > namely that a user who has this enabled might forget that they have it
> > enabled, and, as such, on a machine that _doesn't_ have it enabled (of
> > which they are unfamiliar) might then run the more consequential
> > command "git push --force", but my thinking is that adding this as a
> > feature to the git codebase as an _optional_ (i.e. not enabled by
> > default) configuration variable would then save some of us who use a
> > "rebase-then-force-push for pull request" workflow some time and
> > headaches.
> > 
> > Of course, I don't want to submit a patch if this is a feature that
> > isn't likely to be accepted, so I wanted to get some thoughts from the
> > mailing list regarding this idea.
> > 
> > Thank you,
> > 
> > ~Scott Johnson
> 

Re: Would a config var for --force-with-lease be useful?

[Phillip Wood]

Hi Johannes

On 27/08/18 22:21, Johannes Schindelin wrote:
> Hi,
> 
> On Sat, 25 Aug 2018, Constantin Weißer wrote:
> 
>> I think there are two aspects to using "force with lease".
> 
> There is a third, very, very important aspect.
> 
> When you use --force-with-lease (and I, for one, do, all the time), keep
> in mind that it assumes that you are at least aware of the latest fetched
> ref.
> 
> Let's add a little color to this, to make it less theoretical, and more
> relateable. Let's assume that you work on a branch, say,
> `make-this-thing-work`, and you have to work on this branch on two
> different machines, because it is a finicky thing you need to make work.
> Let's add even more color by saying that one of your machines is a windows
> machine, and the other one an old IRIX machine. We will call them
> `windoze` and `irixxx`.
> 
> The common use case for `--force-with-lease`, at least as far as I know,
> is that this branch is pushed from one of these setups first, say,
> `windoze`, and fetched on the other, then a fixup is necessary and you
> rebase the branch with `--autosquash` to make it compile on IRIX, and then
> you push with `--force-with-lease` a day later, just in case that you
> forgot to re-fetch something that you did on the `windoze` machine.

I have this workflow scripted to sync between my desktop and laptop and 
use it a lot.

> (Something like this happened to me recently, where one of my branches did
> not compile in a Linux VM with an older cURL, and I had to fix a
> Windows-targeting branch to keep it cross-platform)
> 
> The `--force-with-lease` option helps here, and quite a bit.
> 
> It would totally lose its usefulness, though, if you use a tool that
> auto-fetches those remote branches. For example, "synchronizing" in
> <name-your-favorite-git-gui-here>.
> 
> In that case, you did not see what was fetched, and you might have missed
> updates, and you will overwrite them, even if you tried to be careful by
> using `--for-ce-with-lease`.
> 
> I proposed, a couple of months ago, to either fix `--force-with-lease`, or
> to deprecate it in favor of a new option, with a new behavior. The new
> behavior would look at the *reflog*, much as the `--fork-point` option of
> `git rebase`: in addition to the regular `--force-with-lease` server-side
> checks, a client-side check *first* verifies that the remote-tracking
> branch is reachable at least from *one* of the items in the reflog of the
> branch we are about to push.

I think to be completely safe you only want to check the reflog items 
added since the last push, you can get that time by walking the reflog 
of the remote branch. (In my script I have to settle for using the hash 
of the last push rather than the time as there are no pretty format 
specifiers for the reflog dates, just commit and author dates). The 
script also implements a pull command that will rebase any local changes 
since the last push on top of the remote changes so long as the local 
changes are descendants of the last push (i.e rebase --onto $new_remote 
$last_push - I've never been sure if this is the same as using 
--fork-point or not).

> That is, it would ensure that even if we rebased locally, we did
> incorporate the tip commit of the remote-tracking branch, at some stage.
> 
> Granted, there are probably cases where you would fetch, look at the
> remote-tracking branch, and reject those changes without integrating those
> into the local branch. In that case, you would want to relax to the
> current behavior of `--force-with-lease`. But I would expect that to
> happen only rarely.
> 
> The safety by the proposed behavior would make it a lot easier to accept a
> config setting that makes this the default.

I agree having this as an option to push would be very useful for this 
sort of work flow.

Best Wishes

Phillip

> 
> I guess that is the reason why that config setting does not exist yet: we
> would want to have that new behavior in place first...
> 
> Ciao,
> Johannes
> 
>>
>> Firstly, you, a person aware of the option, using it. In this case I
>> think an alias is very fitting, because you get quickly used to just
>> typing `git pf` or so. Plus, you don't have the disadvantage you
>> described: if you’re working on a machine without your alias, you’ll
>> just notice immediately and type the full option.
>>
>> The other aspect is working in a team. The problem there is, that most
>> (at least in my surroundings) use plain --force and you have to make
>> them aware of --force-with-lease. But with an option or an alias, you
>> depend on them using force with lease instead of plain force, so again I
>> don't really see the advantage of such an option.
>>
>> And lastly, a question: say you are using your proposed option and it is
>> turned on. Now, git refuses to push, you clarify the situation and
>> actually mean to push --force now. How would you do this? 1) turn off 2)
>> push 3) turn option on again?
>>
>> Regards,
>> Constantin
>>
>> Quoting Scott Johnson (2018-08-24 18:39:27)
>>> Hello Everyone:
>>>
>>> I'm considering writing a patch that adds a configuration variable
>>> that will allow the user to default the command:
>>>
>>> git push --force
>>>
>>> to:
>>>
>>> git push --force-with-lease
>>>
>>> As discussed here:
>>>
>>> https://stackoverflow.com/questions/30542491/push-force-with-lease-by-default
>>>
>>> Now, I understand that there are downsides to having this enabled,
>>> namely that a user who has this enabled might forget that they have it
>>> enabled, and, as such, on a machine that _doesn't_ have it enabled (of
>>> which they are unfamiliar) might then run the more consequential
>>> command "git push --force", but my thinking is that adding this as a
>>> feature to the git codebase as an _optional_ (i.e. not enabled by
>>> default) configuration variable would then save some of us who use a
>>> "rebase-then-force-push for pull request" workflow some time and
>>> headaches.
>>>
>>> Of course, I don't want to submit a patch if this is a feature that
>>> isn't likely to be accepted, so I wanted to get some thoughts from the
>>> mailing list regarding this idea.
>>>
>>> Thank you,
>>>
>>> ~Scott Johnson

Re: Would a config var for --force-with-lease be useful?

[Junio C Hamano]

Scott Johnson <jaywir3@gmail.com> writes:

> Hello Everyone:
>
> I'm considering writing a patch that adds a configuration variable
> that will allow the user to default the command:
>
> git push --force
>
> to:
>
> git push --force-with-lease

I actually consider "--force-with-lease" that does not say "this is
what exactly I am expecting to replace with my version" a fairly
dangerous form to recommend to the general public, unless their use
of "git fetch" (or "git pull") is disciplined.  In the extreme case,
if you habitually do "git fetch origin" only to update the remote
tracking branches (so that you can do things like "git log ..origin"
to see what others have been doing while you were offline), using
"--force-with-lease" offers no value over "--force", as you're
likely to find your remote-tracking ref to be up-to-date, but it no
longer is what you based on your decision that replacing the tip
with your version is safe.

So, from that point of view, I would recommend thinking twice before
considering to add such a configuration variable.

Re: Would a config var for --force-with-lease be useful?

[Ævar Arnfjörð Bjarmason]

On Mon, Aug 27 2018, Junio C Hamano wrote:

> Scott Johnson <jaywir3@gmail.com> writes:
>
>> Hello Everyone:
>>
>> I'm considering writing a patch that adds a configuration variable
>> that will allow the user to default the command:
>>
>> git push --force
>>
>> to:
>>
>> git push --force-with-lease
>
> I actually consider "--force-with-lease" that does not say "this is
> what exactly I am expecting to replace with my version" a fairly
> dangerous form to recommend to the general public, unless their use
> of "git fetch" (or "git pull") is disciplined.  In the extreme case,
> if you habitually do "git fetch origin" only to update the remote
> tracking branches (so that you can do things like "git log ..origin"
> to see what others have been doing while you were offline), using
> "--force-with-lease" offers no value over "--force", as you're
> likely to find your remote-tracking ref to be up-to-date, but it no
> longer is what you based on your decision that replacing the tip
> with your version is safe.
>
> So, from that point of view, I would recommend thinking twice before
> considering to add such a configuration variable.

Last year there was a proposal for such a patch in:
https://public-inbox.org/git/1499116727-757-1-git-send-email-f@mazzo.li/

This was after/during a long discussion starting with:
https://public-inbox.org/git/CACBZZX7MeX-6RHgh2Fa9+YL03mjxs8xmyE86HnVxBxjMYizcig@mail.gmail.com/

It appears the only patch that got in from that discussion was my
f17d642d3b ("push: document & test --force-with-lease with multiple
remotes", 2017-04-19) (https://github.com/git/git/commit/f17d642d3b)

I still think something like such a config variable would be useful, as
noted in https://public-inbox.org/git/8760f4bmig.fsf@gmail.com/ Junio
voiced similar objections at the time.

It would be great to have some patch like this for consideration, but
give that thread a read first to see what some of the objections were /
various points raised for/against doing that.

Re: Would a config var for --force-with-lease be useful?

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> This was after/during a long discussion starting with:
> https://public-inbox.org/git/CACBZZX7MeX-6RHgh2Fa9+YL03mjxs8xmyE86HnVxBxjMYizcig@mail.gmail.com/
>
> It appears the only patch that got in from that discussion was my
> f17d642d3b ("push: document & test --force-with-lease with multiple
> remotes", 2017-04-19) (https://github.com/git/git/commit/f17d642d3b)

Thanks for pointing at the old thread.

As far as our documentation is concerned, the invitation to improve
the situation, offered in "git push --help", is still valid:

    Note that all forms other than `--force-with-lease=<refname>:<expect>`
    that specifies the expected current value of the ref explicitly are
    still experimental and their semantics may change as we gain experience
    with this feature.

But I do not think (and I did not think back then) there is a magic
bullet to make the lazy force-with-lease automatically safe for
everybody, so it may be time to declare that the lazy force-with-lease
was a failed experiment and move on, with a patch like the one
suggested last year in the message:

  https://public-inbox.org/git/xmqq37a9fl8a.fsf_-_@gitster.mtv.corp.google.com/

Re: Would a config var for --force-with-lease be useful?

[Ævar Arnfjörð Bjarmason]

On Mon, Aug 27 2018, Junio C Hamano wrote:

[Scott, I hope you're still with us despite your recent attempt to
unsubscribe from git@ :)]

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> This was after/during a long discussion starting with:
>> https://public-inbox.org/git/CACBZZX7MeX-6RHgh2Fa9+YL03mjxs8xmyE86HnVxBxjMYizcig@mail.gmail.com/
>>
>> It appears the only patch that got in from that discussion was my
>> f17d642d3b ("push: document & test --force-with-lease with multiple
>> remotes", 2017-04-19) (https://github.com/git/git/commit/f17d642d3b)
>
> Thanks for pointing at the old thread.
>
> As far as our documentation is concerned, the invitation to improve
> the situation, offered in "git push --help", is still valid:
>
>     Note that all forms other than `--force-with-lease=<refname>:<expect>`
>     that specifies the expected current value of the ref explicitly are
>     still experimental and their semantics may change as we gain experience
>     with this feature.
>
> But I do not think (and I did not think back then) there is a magic
> bullet to make the lazy force-with-lease automatically safe for
> everybody, so it may be time to declare that the lazy force-with-lease
> was a failed experiment and move on, with a patch like the one
> suggested last year in the message:
>
>    https://public-inbox.org/git/xmqq37a9fl8a.fsf_-_@gitster.mtv.corp.google.com/

With the benefit of hindsight I still agree with my counter-argument to
that in https://public-inbox.org/git/8760f4bmig.fsf@gmail.com/

I.e. making plain --force-with-lease harder to use by hiding it behind a
config option gives the user fewer options than with --force to recover.

So I think we should still recommend the longer and even safer variants
of --force-with-lease, but being guaranteed to have the SHA-1 you just
clobbered locally is *better*, and allows us to e.g. do this:

    $ git push --force-with-lease
    hint: You just clobbered <X> on <remote with <Y>. If you regret
    hint: this you can (until the object gets pruned) do:
    hint:     git push <remote> --force-with-lease=<refname>:<Y>

Or, doing the same with --force with some config option to use the
marginally safer (because at least you have a local copy)
--force-with-lease automatically.

Re: Would a config var for --force-with-lease be useful?

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> I.e. making plain --force-with-lease harder to use by hiding it behind a
> config option gives the user fewer options than with --force to recover.

I agree with that.  But I would consider it a good thing, if done
properly (i.e. suggest --force-with-lease that is not a lzzy form).

> So I think we should still recommend the longer and even safer variants
> of --force-with-lease, but being guaranteed to have the SHA-1 you just
> clobbered locally is *better*, and allows us to e.g. do this:
>
>     $ git push --force-with-lease
>     hint: You just clobbered <X> on <remote with <Y>. If you regret
>     hint: this you can (until the object gets pruned) do:
>     hint:     git push <remote> --force-with-lease=<refname>:<Y>

Until the object gets pruned, and until somebody else pushes there
to make the damage bigger, you can recover from a mistake with that
information.  It probably is a bug that the lazy force-with-lease
does not report what the remote tip you just overwrote was, and this
would help great deal.  It would be a good place to start.

One thing that I am still not clear how this line of thought truly
would help users is how to help them decide their answer to "If you
regret this".  An unthinking naïve user would say "of course I meant
it when I gave the option---why do you think I regret it?" without a
bit more hint, namely, "<Y> was taken from the remote tracking
branch, are you sure that is still what the newly prepared contents
you built to replace?  Did somebody clobber it while you are not
watching?"

These three lines however will be felt too loud by those who would
most benefit from them (i.e. those who do not know why they need
protection); I do not think advise.* to allow them to be squelched
would be appropriate.