Thread (4 messages) flat view 4 messages, 2 authors, 2016-06-15

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

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:57:23

Junio C Hamano wrote:
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.
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)?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help