All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Herland <johan@herland.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [RFD] Making "git push [--force/--delete]" safer?
Date: Wed, 3 Jul 2013 00:55:34 +0200	[thread overview]
Message-ID: <CALKQrgenpqKUxOZ+p79NsaQD9M2-q4h93ZqN0oencVo-QZF=zg@mail.gmail.com> (raw)
In-Reply-To: <7vfvvwk7ce.fsf@alter.siamese.dyndns.org>

On Tue, Jul 2, 2013 at 10:57 PM, Junio C Hamano <gitster@pobox.com> wrote:

[...]

>   (2) Add --compare-and-swap=dst:expect parameters, e.g.
>
>       $ git push --cas=master:deadbabecafe --cas=next:cafebabe ":"
>
>       This removes the "reservation" I expressed against (1) above
>       (i.e. we are doing a "matching" push in this example, but we
>       will fail if 'master' and 'next' are not pointing at the
>       expected objects).

I still think this is too long/verbose for the average user to
remember, and type out. Also, I don't like the name, as it is too
'technical', and describes the nature of the implementation (i.e. the
"how") rather than the purpose of using it (i.e. the "why" or "what").

>   (3) Add a mechanism to call a custom validation script after "git
>       push" reads the list of <current object name, refname> tuples,
>       but before responding with the proposed update.  The script
>       would be fed a list of <current object name, new object
>       name, refname> tuples (i.e. what the sender _would_ tell the
>       receiving end if there weren't this mechanism), and can tell
>       "git push" to fail with its exit status.
>
>       This would be the most flexible in that the validation does
>       not have to be limited to "the ref must be still pointing at
>       the object we expect" (aka compare-and-swap); the script could
>       implement other semantics (e.g. "the ref must be pointing at
>       the object or its ancestor").

With this, I guess --dry-run could be reformulated as a trivial
validation script that always returns a non-zero exit code (although
it should still cause 'push' to return zero).

[...]

> I am inclined to say, if we were to do this, we should do (2) among
> the above three.
>
> But of course, others may have better ideas ;-).

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

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.


...Johan

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

  reply	other threads:[~2013-07-02 22:55 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-02 20:57 [RFD] Making "git push [--force/--delete]" safer? Junio C Hamano
2013-07-02 22:55 ` Johan Herland [this message]
2013-07-03  6:34   ` Johan Herland
2013-07-03  8:49     ` Junio C Hamano
2013-07-03 10:00       ` Johan Herland
2013-07-03 10:06         ` Jonathan del Strother
2013-07-03 10:11           ` Johan Herland
2013-07-03 10:50             ` Michael Haggerty
2013-07-03 12:06               ` Johannes Sixt
2013-07-03 19:53                 ` Junio C Hamano
2013-07-04  5:37                   ` Johannes Sixt
2013-07-04  5:46                     ` Junio C Hamano
2013-07-03 19:50           ` Junio C Hamano
2013-07-03 20:18             ` Junio C Hamano
2013-07-03 19:48         ` Junio C Hamano
2013-07-09 19:53 ` [PATCH 0/7] safer "push --force" with compare-and-swap Junio C Hamano
2013-07-09 19:53   ` [PATCH 1/7] cache.h: move remote/connect API out of it Junio C Hamano
2013-07-09 19:53   ` [PATCH 2/7] builtin/push.c: use OPT_BOOL, not OPT_BOOLEAN Junio C Hamano
2013-07-09 19:53   ` [PATCH 3/7] push: beginning of compare-and-swap "force/delete safety" Junio C Hamano
2013-07-09 19:53   ` [PATCH 4/7] remote.c: add command line option parser for --lockref Junio C Hamano
2013-07-16 22:13     ` John Keeping
2013-07-17 17:06       ` Junio C Hamano
2013-07-17 17:09       ` Junio C Hamano
2013-07-09 19:53   ` [PATCH 5/7] push --lockref: implement logic to populate old_sha1_expect[] Junio C Hamano
2013-07-09 19:53   ` [PATCH 6/7] t5533: test "push --lockref" Junio C Hamano
2013-07-09 19:53   ` [PATCH 7/7] push: document --lockref Junio C Hamano
2013-07-09 20:17     ` Aaron Schrab
2013-07-09 20:39       ` Junio C Hamano
2013-07-09 20:24     ` Johannes Sixt
2013-07-09 20:37       ` Junio C Hamano
2013-07-09 20:55         ` Johannes Sixt
2013-07-09 22:09           ` Junio C Hamano
2013-07-09 23:08             ` Junio C Hamano
2013-07-11 21:10               ` Johannes Sixt
2013-07-11 21:57                 ` Junio C Hamano
2013-07-11 22:14                 ` Junio C Hamano
2013-07-12 17:21                   ` Johannes Sixt
2013-07-12 17:40                     ` Junio C Hamano
2013-07-12 20:00                       ` Johannes Sixt
2013-07-12 21:19                         ` Junio C Hamano
2013-07-13  6:52                           ` Johannes Sixt
2013-07-13 18:14                             ` Junio C Hamano
2013-07-13 20:08                               ` Junio C Hamano
2013-07-13 21:11                                 ` Johannes Sixt
2013-07-14 14:28                                 ` John Keeping
2013-07-13 20:17                               ` Johannes Sixt
2013-07-14 19:17                                 ` Junio C Hamano
2013-07-14 20:21                                   ` Johannes Sixt
2013-07-14 20:34                                     ` Jonathan Nieder
2013-07-14 20:49                                       ` Jonathan Nieder
2013-07-14 20:59                                       ` Johannes Sixt
2013-07-14 21:28                                         ` Jonathan Nieder
2013-07-15  4:10                                           ` Junio C Hamano
2013-07-15  4:44                                             ` Jonathan Nieder
2013-07-15 15:37                                               ` Junio C Hamano
2013-07-15 20:30                                         ` Johannes Sixt
2013-07-15  3:50                                     ` Junio C Hamano
2013-07-15 15:47                                       ` Default expectation of --lockref Junio C Hamano
2013-07-15 20:27                                       ` [PATCH 7/7] push: document --lockref Johannes Sixt
2013-07-09 21:37         ` Marc Branchaud
2013-07-09 20:27     ` Michael Haggerty
2013-07-09 20:42       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALKQrgenpqKUxOZ+p79NsaQD9M2-q4h93ZqN0oencVo-QZF=zg@mail.gmail.com' \
    --to=johan@herland.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.