Re: [PATCH v2 02/15] user-manual: Update for receive.denyCurrentBranch=refuse

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2013, 21:24 UTC
Message-ID: <7vd2w7uajk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <ba62167be6df7a6ba666373913bfd235bcc29c9d.1360508415.git.wking@tremily.us>

"W. Trevor King" <wking@tremily.us> writes:

Show 29 quoted lines

> From: "W. Trevor King" <wking@tremily.us>
>
> acd2a45 (Refuse updating the current branch in a non-bare repository
> via push, 2009-02-11) changed the default to refuse such a push, but
> it forgot to update the docs.
>
> 7d182f5 (Documentation: receive.denyCurrentBranch defaults to
> 'refuse', 2010-03-17) updated Documentation/config.txt, but forgot to
> update the user manual.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
>  Documentation/user-manual.txt | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index 222545b..8524c08 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -1988,8 +1988,10 @@ handling this case.
>  Note that the target of a "push" is normally a
>  <<def_bare_repository,bare>> repository.  You can also push to a
>  repository that has a checked-out working tree, but the working tree
> -will not be updated by the push.  This may lead to unexpected results if
> -the branch you push to is the currently checked-out branch!
> +will not be updated by the push.  To protect against this, pushes to
> +the currently checked-out branch of a repository are denied by
> +default.  See the description of the receive.denyCurrentBranch option
> +in linkgit:git-config[1] for details.

Nobody else felt the same logic gap between "will not be updated" and "to protect against this" I sensed while reading this?

I would not be surprised if some readers felt as if "then why not update it instead of rejecting?" were a valid question, without a bit more explanation.

	You can also push to a repository that has a working tree,
	but a push to the currently checked out branch is denied for
	two reasons:

        (1) if the push were allowed to update the working tree to
            match the pushed commit, it will overwrite and lose the
            work in progress in the working tree;

	(2) the push does _not_ update the working tree for the
	    above reason, but then if the push were allowed to
	    update the tip of the branch, the next commit made based
	    on the work done in the working tree will be based on
	    the state of the tree _before_ the push but will be
	    recorded as a descendant of the pushed commit, reverting
	    changes to the contents made by the pushed commit.
	
	If you want to handle the above two risks yourself (e.g. you
	can promise that you will never make changes to the files in
	the working tree or make commit from the working tree, and
	install a post-push hook that does "git reset --hard" to
	match the working tree state to the commit that was pushed),
	you can override this by setting receive.denyCurrentBranch.

It almost makes me wonder if it also makes sense to supersede the denyCurrentBranch setting with a new receive.currentBranch variable (setting it to "deny" is equivalent to setting denyCurrentBranch to true), and make "receive.currentBranch = reset" to allow a push and always run "git reset --hard" afterwards.

If we were to do so, I would strongly be against adding "checkout" mode that does "git reset --keep" instead of "--hard". Those who can accept the unconditional ovewriting with "reset --hard" are the only ones that are safe if "push" updated the tip of the branch and the working tree. Updating the working tree with a "reset --keep" may appear safe because it will catch the case where a file that needs updating by "push" is modified, but that is an incorrect assumption. It does not mean nobody has the contents in the editor buffer with changes yet to be written out. An automated update to the working tree condition has to be allowed only to a repository with nobody sitting at the keyboard editing files in there.