bug: update hook failure doesn't prevent local deletion of a branch

Hello,

I wanted to delete a branch (let's call it "deleted-branch") from my public repository. I ran this:

$ git push origin :deleted-branch
deleting 'refs/heads/deleted-branch'
 Also local refs/remotes/up/deleted-branch
*** Update hook: aborting
error: hooks/update exited with error code 1
error: hook declined to update refs/heads/deleted-branch
ng refs/heads/deleted-branch hook declined
error: failed to push to '/path/to/git/repo.git'

Coincidentally, my update hook happened to have a bug in it that prevented me from running the operation.

However, when I run the operation again...

$ git push origin :deleted-branch
deleting 'refs/heads/deleted-branch'
 Also local refs/remotes/up/deleted-branch
error: unlink(.git/refs/remotes/up/deleted-branch) failed: No such file or 
directory
error: Failed to delete

If the remote didn't get deleted, then it seems wrong that the local copy does get deleted.

Summary: when using git-push to delete a remote branch, and that deletion is 
disallowed by the update hook, the local tracking branch _is_ deleted.

Obviously, this isn't _that_ serious because it could be recovered again with a git-fetch; but it does make some scary looking errors.

Andy

Andy Parkins <andyparkins@gmail.com> writes:

Yes, a few months ago with b516968f, "send-pack" started to pretend that it turned around and fetched immediately after it attempted to push.

There are two problems with it. One is this exact problem Andy reported. Newer receive-pack reports the status (if an update of the ref was successful), and the sender _could_ try to detect a failure and refrain from making the local side update (but if the receive-pack running on the other end is old enough there is no indication of an error); the code does not bother to do this correctly. I think this is correctable without major changes. If the remote end refused to update only one of the refs, and send-pack as a whole fails, we could refrain from updating anything local.

But another more fundamental issue is that post-update hook on the receiving end could do anything it pleases (for example, it could try to rebase what was pushed), and at the protocol level there is no way to say "you tried to push this SHA-1 but we replaced it with this other SHA-1 instead".

Junio C Hamano <gitster@pobox.com> wrote:

...

Reasonable.  Probably pretty simple too.

You keep raising this argument against the "git-push updates local refs" feature. Its not just the post-update hook that could alter the ref. Another user could just push behind you (and I mean right behind you) and either force an update, delete the ref entirely, or just fast-forward it. (Its entirely possible that the other user does have the commit you just pushed, because you had previously supplied it to them.) So its actually possible for the ref to have changed again before you even see the successful update message on your console.

I don't think it really matters. Based on the output you see from git-push on screen you have good reason to believe that the ref you just pushed to is now at the commit SHA-1 displayed. For that reason alone I think it is correct to update the corresponding tracking branch, because an immediate git-fetch just wastes the user's time and will (usually) fetch the SHA-1 just pushed.

We should make our repository state reflect the user's internal mental view of what just happened, especially here, because the user's mental view is probably correct.

"Shawn O. Pearce" <spearce@spearce.org> writes:

Fair enough. At least if we correct git-push so that when it exits with failure it would not touch the local refs, I would think that would make what happens to match user's mental model much better. The user would be _told_ that it failed, and then can fetch back if he cares where the remote heads are too deeply.