Re: disallowing push to currently checked-out branch

Jeff King [off-list ref] writes:

On Mon, Feb 16, 2009 at 03:23:00PM -0800, Junio C Hamano wrote:

quoted

quoted

  1. How can we improve this situation?

The situation you described is all about "don't allow a push that is NOT
CONTROLLED BY YOU and that can interfere with what you are doing into a
live repository", and you are right, we have operations that deliberately
detach the HEAD and expect nobody mucks with the branch.

I don't agree that it has to be a push not controlled by you. I have
many times left a rebase-in-progress sitting in a repository, either
accidentally because I meant to "--abort" it after a conflict but
forgot, or because I got interrupted during an interactive edit and
needed to come back to it.

That sounds similar to saying "I left my editor open without saving my
changes, and accidentally opened another instance of an editor from a
different terminal and edited the same file, the result is a mess".  The
editors protect users from such a situation by locking the file they are
editing.

Perhaps operations that detaches HEAD (rebase and perhaps sequencer) can
all agree to use a single marker file that says "Do not mess with these
refs via push or fetch" and make receive-pack and fetch honor that?  Then
the issue you raised in your earlier message about receive-pack having to
know random states random set of tools leave will be alleviated.  We need
to make sure that the marker is cleaned up correctly when the command is
done with the lock, of course.

If we were to go that route, I think the same receive.denyCurrentBranch
configuration variable can and should be used to control this, even though
its name originally comes from the most visible operation that can cause
the confusion (i.e. "pushing into the current branch").  It is about
protecting the person who is currently using the work tree, or who will
use the work tree next.

I am really just proposing that the "ref was not what we expected"
message to better indicate what is going on, and how the user might get
out of it. Do you not agree with that?

The recovery recipe you described looked good.

On Mon, Feb 16, 2009 at 04:43:03PM -0800, Junio C Hamano wrote:

That sounds similar to saying "I left my editor open without saving my
changes, and accidentally opened another instance of an editor from a
different terminal and edited the same file, the result is a mess".  The
editors protect users from such a situation by locking the file they are
editing.

It is definitely similar.

Perhaps operations that detaches HEAD (rebase and perhaps sequencer) can
all agree to use a single marker file that says "Do not mess with these
refs via push or fetch" and make receive-pack and fetch honor that?  Then
the issue you raised in your earlier message about receive-pack having to
know random states random set of tools leave will be alleviated.  We need
to make sure that the marker is cleaned up correctly when the command is
done with the lock, of course.

I think such a marker is a fine idea in general, because it would be
nice to be able to say "what is all state in the repo that I might care
about" (which I think has been talked about several times). In fact, I
have had a similar problem _without_ pushing just by leaving and coming
back in the middle of operations, especially failed ones (e.g., "git
am", realize the patch doesn't apply, forget to --abort, then make more
commits, realize only when you try to "git am" something else, but now
aborting will intermediate work).

I'm not sure that supporting it in receive-pack is necessary. The
current rebase code already detects the situation; it just doesn't
handle it as gracefully as it might. And it doesn't close _all_
possibility for danger, as the example you gave previously shows; the
user can still be surprised by the ref changing.

Whereas improving the local tool support for "rebase --abort" and "am
--abort" to help a user recover from such a situation means that we help
not only the situation of somebody pushing, but also local "I forgot
and changed the repo" situations.

quoted

I am really just proposing that the "ref was not what we expected"
message to better indicate what is going on, and how the user might get
out of it. Do you not agree with that?

The recovery recipe you described looked good.

OK. I'll look at working up a patch.

-Peff