Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"

[Junio C Hamano <gitster@pobox.com>]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> How about doing these before the 1.7.0 release?
> ...
> What do people think?

I haven't manged to convince myself about the "git init" change (I have
the code and also I've looked at the extent of damage the change causes to
the existing test suite), but at least I think it is a sensible suggestion
to differentiate between unconfigured-wwwand-defaults-to-warn case and
configured-to-warn-so-we-warn case.  Something like this.

-- >8 --
Subject: [PATCH] receive-pack: explain what to do when push updates the current branch

This makes "git push" issue a more detailed instruction when a user pushes
into the current branch of a non-bare repository without having an
explicit configuration set to receive.denycurrentbranch.  In such a case,
it will also tell the user that the default will change to refusal in a
future version of git.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-receive-pack.c |   58 +++++++++++++++++++++++++++++++++++------------
 t/t5516-fetch-push.sh  |    6 ++--
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 6564a97..f2c94fc 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -12,6 +12,7 @@
 static const char receive_pack_usage[] = "git-receive-pack <git-dir>";
 
 enum deny_action {
+	DENY_UNCONFIGURED,
 	DENY_IGNORE,
 	DENY_WARN,
 	DENY_REFUSE,
@@ -19,7 +20,7 @@ enum deny_action {
 
 static int deny_deletes = 0;
 static int deny_non_fast_forwards = 0;
-static enum deny_action deny_current_branch = DENY_WARN;
+static enum deny_action deny_current_branch = DENY_UNCONFIGURED;
 static int receive_fsck_objects;
 static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
@@ -214,6 +215,35 @@ static int is_ref_checked_out(const char *ref)
 	return !strcmp(head, ref);
 }
 
+static char *warn_unconfigured_deny_msg[] = {
+	"Updating the currently checked out branch may cause confusion,",
+	"as the index and work tree do not reflect changes that are in HEAD."
+	"As a result, you may see the changes you just pushed into it",
+	"reverted when you run 'git diff' over there, and you may want",
+	"to run 'git reset --hard' before starting to work to recover.",
+	"",
+	"You can set 'receive.denyCurrentBranch' configuration variable to",
+	"'refuse' in the repository to forbid pushing into the current branch",
+	"of it."
+	"",
+	"To allow pushing into the current branch, you can set it to 'ignore';",
+	"but this is not recommended unless you really know what you are doing.",
+	"",
+	"To squelch this message, you can set it to 'warn'.",
+	"",
+	"Note that the default will change in a future version of git",
+	"to refuse updating the currentbranch unless you have the",
+	"configuration variable set to either 'ignore' or 'warn'."
+};
+
+static void warn_unconfigured_deny(void)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(warn_unconfigured_deny_msg); i++)
+		warning(warn_unconfigured_deny_msg[i]);
+}
+
+
 static const char *update(struct command *cmd)
 {
 	const char *name = cmd->ref_name;
@@ -227,22 +257,20 @@ static const char *update(struct command *cmd)
 		return "funny refname";
 	}
 
-	switch (deny_current_branch) {
-	case DENY_IGNORE:
-		break;
-	case DENY_WARN:
-		if (!is_ref_checked_out(name))
+	if (is_ref_checked_out(name)) {
+		switch (deny_current_branch) {
+		case DENY_IGNORE:
 			break;
-		warning("updating the currently checked out branch; this may"
-			" cause confusion,\n"
-			"as the index and working tree do not reflect changes"
-			" that are now in HEAD.");
-		break;
-	case DENY_REFUSE:
-		if (!is_ref_checked_out(name))
+		case DENY_UNCONFIGURED:
+		case DENY_WARN:
+			warning("updating the current branch");
+			if (deny_current_branch == DENY_UNCONFIGURED)
+				warn_unconfigured_deny();
 			break;
-		error("refusing to update checked out branch: %s", name);
-		return "branch is currently checked out";
+		case DENY_REFUSE:
+			error("refusing to update checked out branch: %s", name);
+			return "branch is currently checked out";
+		}
 	}
 
 	if (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 4426df9..89649e7 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -492,7 +492,7 @@ test_expect_success 'warn on push to HEAD of non-bare repository' '
 		git checkout master &&
 		git config receive.denyCurrentBranch warn) &&
 	git push testrepo master 2>stderr &&
-	grep "warning.*this may cause confusion" stderr
+	grep "warning: updating the current branch" stderr
 '
 
 test_expect_success 'deny push to HEAD of non-bare repository' '
@@ -510,7 +510,7 @@ test_expect_success 'allow push to HEAD of bare repository (bare)' '
 		git config receive.denyCurrentBranch true &&
 		git config core.bare true) &&
 	git push testrepo master 2>stderr &&
-	! grep "warning.*this may cause confusion" stderr
+	! grep "warning: updating the current branch" stderr
 '
 
 test_expect_success 'allow push to HEAD of non-bare repository (config)' '
@@ -520,7 +520,7 @@ test_expect_success 'allow push to HEAD of non-bare repository (config)' '
 		git config receive.denyCurrentBranch false
 	) &&
 	git push testrepo master 2>stderr &&
-	! grep "warning.*this may cause confusion" stderr
+	! grep "warning: updating the current branch" stderr
 '
 
 test_expect_success 'fetch with branches' '
-- 
1.6.1.2.312.g5be3c