Re: disallowing push to currently checked-out branch

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: disallowing push to currently checked-out branch

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:13

Jeff King [off-list ref] writes:
  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.

But is this something even worth considering about in the same context as
the denyCurrentBranch?  The same thing can happen even if you are not
detaching HEAD.

For example, I sometimes end up with an ugly series on a branch, whose
endpoint is a good looking tree.  And a refactoring I would want to do
would be too cumbersome for the interactive rebase (I could do it, but the
machinery does not help as much as it would for a simpler case).  In such
a case, often I would just say:

	$ git branch -f goal
        $ git reset --hard master
        : repeat from here until "diff HEAD goal" becomes empty
        ... cherry-pick $a_commit_in_goal_branch, or
        ... edit "show $a_commit_in_goal_branch" output and apply, or
        ... edit the files in place.
        ... make a commit, perhaps using -c $a_commit_in_goal_branch
	: repeat up to here

I would not push into this repository to update the branch "goal" while I
am doing this, as it will obviously screw up the whole process.  I think
it is the same thing that you would not push from elsewhere to update the
branch you are in the middle of interactively rebasing.  Mucking with the
same repository from two different places at the same time, when you know
there can be only one version of a work tree that is checked out, is
simply insane.

It's just a common sense thing.  What denyCurrentBranch protects you from
is a push from elsewhere *while* you are not there, and then next day,
getting confused by what such a push did in the receiving repository.  In
that scenario, you are not mucking with the receiving repository from two
places at the same time, but still you can get your repository into a
confusing state, and it is worth protecting new people from.

Obviously you can tell receive-pack to refuse pushing into a non-bare
repository, with a "I know what I am doing" configuration, but I think at
that point the whole "you could break things this way, so let's prevent a
new user from making such mistake" goes into the realm of absurdity.

Re: disallowing push to currently checked-out branch

From: Jeff King <hidden>
Date: 2016-06-15 22:46:13

On Mon, Feb 16, 2009 at 03:23:00PM -0800, Junio C Hamano wrote:
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.

So the problem is simply that the repository you're pushing into is not
in the state you think it is (either because you forgot what state you
left it in, didn't realize what state you left it in, or because it is
somebody else's repo).
But is this something even worth considering about in the same context as
the denyCurrentBranch?  The same thing can happen even if you are not
detaching HEAD.
I don't think it's the same, but I think it is a related problem. I
don't think the solutions are related, though.
For example, I sometimes end up with an ugly series on a branch, whose
endpoint is a good looking tree.  And a refactoring I would want to do
would be too cumbersome for the interactive rebase (I could do it, but the
machinery does not help as much as it would for a simpler case).  In such
a case, often I would just say:

	$ git branch -f goal
        $ git reset --hard master
        : repeat from here until "diff HEAD goal" becomes empty
        ... cherry-pick $a_commit_in_goal_branch, or
        ... edit "show $a_commit_in_goal_branch" output and apply, or
        ... edit the files in place.
        ... make a commit, perhaps using -c $a_commit_in_goal_branch
	: repeat up to here

I would not push into this repository to update the branch "goal" while I
am doing this, as it will obviously screw up the whole process.  I think
OK, that is a good example of how this is basically impossible to
protect from fully (and a good argument why pushing into a repo used for
work is probably not a good idea in general). I think the rebase example
is a little worse because:

  - It's subtle. with denyCurrentBranch, we generally protect the user
    from pushing into the current branch and messing things up. But
    during a rebase we don't, and the only way the user would realize
    that is if they understand that rebasing happens on a detached HEAD.

  - the error message is confusing, and there is no clear way out of the
    error case. You can "rebase --abort" which throws away your rebase
    work _and_ the push.  But what you probably want to do, I described
    earlier.
It's just a common sense thing.  What denyCurrentBranch protects you from
is a push from elsewhere *while* you are not there, and then next day,
getting confused by what such a push did in the receiving repository.  In
See above for why I think this can happen while you are not there. It is
about repo state for long-running workflows. I'm not too concerned with
somebody pushing in the exact half second while you are making running
"git commit".
Obviously you can tell receive-pack to refuse pushing into a non-bare
repository, with a "I know what I am doing" configuration, but I think at
that point the whole "you could break things this way, so let's prevent a
new user from making such mistake" goes into the realm of absurdity.
I think that is insane, too. This is not all that likely to happen
compared to the possible benefits of pushing into a non-bare repo. And
as you say, in most cases common sense rules: don't push into something
you are actively working on.

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?

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help