Re: [RFD] Making "git push [--force/--delete]" safer?

From: Johan Herland <johan@herland.net>
Date: Jul 3, 2013, 06:34 UTC
Message-ID: <CALKQrgdovWTd50LVDnNR+BhurWgSCKkhr88wCo01VZF3sd5PNg@mail.gmail.com>
In-Reply-To: <CALKQrgenpqKUxOZ+p79NsaQD9M2-q4h93ZqN0oencVo-QZF=zg@mail.gmail.com>

On Wed, Jul 3, 2013 at 12:55 AM, Johan Herland <johan@herland.net> wrote:

Show 10 quoted lines

> I assume that in most cases the expected value of the remote ref would
> equal the current value of the corresponding remote-tracking ref in
> the user's repo, so why not use that as the default expected value?
> E.g.:
>
>   $ git config push.default simple
>   $ git checkout -b foo -t origin/foo
>   # prepare non-ff update
>   $ git push --force-if-expected
>   # the above validates foo @ origin != origin/foo before pushing

Oops, typo: s/!=/==/

Show 10 quoted lines

>
> And if the users expects a different value, (s)he can pass that to the
> same option:
>
>   $ git push --force-if-expected=refs/original/foo my_remote HEAD:foo
>   # the above fails if foo @ origin != refs/original/foo
>
> The option name probably needs a little work, but as long as it
> properly communicates the user's _intent_ I'm fine with whatever we
> call it.

Overnight, it occured to me that --force-if-expected could be simplified by leveraging the existing --force option; for the above two examples, respectively:

  $ git push --force --expect
  # validate foo @ origin == @{upstream} before pushing

and

  $ git push --force --expect=refs/original/foo my_remote HEAD:foo
  # validate foo @ my_remote == refs/original/foo before pushing

In other words, the --expect option becomes a modifier on the --force behaviour: If --expect is given, and the remote ref is not as expected, then the push will still fail, even when --force is given. Furthermore, this could be fleshed out by allowing the user to configure push.expect = True, in which case --expect will be assumed whenever --force is used, and the user can override with --no-expect.

If push.expect == True (or if --expect is given on command-line without a parameter), we default to using @{upstream} as the expected value, and we complain to the user if the current branch has no upstream. This way, you can still enable push.expect even when you do not configure @{upstream}, but it compels you to always supply --expect=$something (or --no-expect) when you use --force.

...Johan

PS: I'm still unsure about the option naming. Maybe --validate would
be better than --expect, but I feel it should convey more strongly
that we're doing _pre_-validation, as opposed to (post-)validating the
_result_ of the push, whatever that would look like.

-- 
Johan Herland, <johan@herland.net>
www.herland.net