git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 7/7] push: document --lockref
Date: Sun, 14 Jul 2013 14:28:00 -0700	[thread overview]
Message-ID: <20130714212800.GA11009@google.com> (raw)
In-Reply-To: <51E31131.3070005@kdbg.org>

Johannes Sixt wrote:
> Am 14.07.2013 22:34, schrieb Jonathan Nieder:

>> Would a '*' that acts like --lockref on a per ref basis address your
>> concerns?
>
> No, because I think that new syntax is not necessary.
>
> But admittedly, I haven't spent any time to think about push.default
> modes other than 'matching'. In particular, I wonder how Junio's last
> example with push.default=simple can work today:
>
>    $ git pull --rebase  # not a merge
>    $ git push
>
> because it is not a fast-forward.

Right, let's examine this example more closely.

If I run:

	(1) git pull --rebase
	(2) git push

then normally that push will be a fast-forward.  My changes are
on top of the new upstream changes, just as though I used format-patch
and send-email to submit the changes to a maintainer who would then
apply them.

However, someone else might have pushed to the same branch between
step (1) and (2), causing the fast-forward-only push to fail.

Usually that means other person made a valuable change and I can
simply repeat steps (1), and (2) and they will succeed.

But maybe that intervening push was a mistake.  To distinguish that
possibility I might do something like

	(3) git fetch origin
	(4) gitk @{u}@{1}..@{u}; # Is the change good?

	(5a) git pull --rebase; git push; # Yes, put my change on top of it
	(5b) git push --force; # No, my change is better!

So far so good.  But what if yet another change is made upstream
between step (3) and (5)?

If following approach (5a), that's fine.  We notice the new
intervening change and react accordingly, again.  There is a
possibility of starvation, but no other harm done.

In case (5b), it may be a serious problem.  I don't know about the
intervening change until I read the "git push" output, and in the
usual case I just won't notice.  The new lockref UI is meant to
address this problem.  So in the new world order, in case (5b) it
sounds like I should have instead used

	(5b') git push --allow-non-ff

Suppose I am writing a script that is meant to set the remote
repository to a known state.  Other contributors are only using
fast-forward updates so once my change goes in they will act
appropriately.  I just need to get my ref update in, without being
blocked by other ref updates.

Then I will use

	(5c) git push --force

which means not to use this new lockref trick that looks at my
remote-tracking branch and instead to just force the ref update.  This
would for example be the right semantics when pushing to a mirror from
a relay that also fetches from a canonical repository.  It avoids
needing to fetch from the target repo before every push.

Of course if ref updates are highly contended, even the current "git
push --force" will sometimes fail, since it internally *does* use a
compare-and-swap against the result of an ls-remote.  That's a (minor)
bug, imho.  Fixing it will require tweaking the protocol to make the
compare-and-swap optional.

  reply	other threads:[~2013-07-14 21:28 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
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 [this message]
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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20130714212800.GA11009@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).