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)?