Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: May 21, 2013, 21:09 UTC
Message-ID: <CALkWK0np7o0eH8ZsWQSwk1Cdwnnpj5B==gS8kAE+OkTskASOsQ@mail.gmail.com>
In-Reply-To: <7vppwkm682.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 9 quoted lines

> In general, when a command is working in your repository with a
> working tree, we do not make any such promise that it keeps
> operationg normally when you pull the rug under its feet from
> another terminal ("git checkout maint" running at the same time "git
> pull" would not have a chance to work correctly).  Some are safe
> (like "git push" racing with "git checkout maint" that would not
> have to look at what the current branch is) and some are not (like
> "git push github" racing with "git checkout maint && git push
> origin HEAD:preview").

My current set of expectations look like this:

1. Commands that operate on a worktree (merge family) do not lock the
worktree before operating on it.  These are fast synchronous commands
(operating on recent hot-cached stuff), and there is no _utility_ in a
perfectly atomic worktree operation.  Might be an interesting
theoretical exercise to merge-trees in memory, but I have absolutely
no interest in such a problem.

2. Commands that operate on refs, the-index, and intermediate states
(update-ref, update-index, rebase-state family) do so atomically using
the lockfile API.  The atomicity is important here, because we don't
want a higher-level command to half-fail.  And it's trivial to
implement.

3. Commands that operate only on the object store are guaranteed not
to race as the object store itself is read-only (hash-object,
commit-tree family; gc being the exception).  This is very important
for concurrent access in a server implementation.

In 3 out of the 4 push.default states, push is category (3). push.default = current is a special case, and should try not to break end-user expectation even if it involves resolving HEAD.

Show 9 quoted lines

> I view the value of this topic purely as "showing a real branch name
> when that is what we actually pushed is a lot more preferrable than
> showing HEAD, especially because the user may see it in the terminal
> scrollback buffer hours after it happened".  Explaining this patch
> as "we avoid issues from simultaneously flipping HEAD by resolving
> early" gives a false sense of security to the reader, as "early" has
> to happen early enough for the patch to really avoid the issue, but
> you are not in control of when the user does that flipping in the
> other terminal.

Why should I lie in the patch? The terminal flipping was a very big itch I had, and the patch fixes exactly that issue. Showing the real branch name was an unintended side-effect.

I just said "early" and showed a nice end-user example in which it works, not "theoretically impossible to race with". Better wording (while not lying about the motivation behind the patch)?