{"thread":{"id":"17449","subject":"[PATCH] Switch receive.denyCurrentBranch to \"refuse\"","startedAt":"2009-01-30T00:34:28Z","lastAt":"2010-04-13T17:57:50Z","messageCount":43,"participants":["Johannes Schindelin","Jay Soffian","Asheesh Laroia","Junio C Hamano","Miklos Vajna","Jeff King","Johannes Sixt","Nanako Shiraishi","Sam Vilain","Dave Abrahams"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"102531","messageId":"alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"cover.1233275583u.git.johannes.schindelin@gmx.de","subject":"[PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T00:34:28Z","receivedAt":"2009-01-30T00:34:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Many, many users set up non-bare repositories on their server, and are\nconfused that the working directory is not updated.\n\nThe reason may be that they did not read the manual, or that they found\n\"helpful\" walk-throughs via Google, or that they did not understand the\nconcepts behind Git.\n\nOr, the reason could be that we made a design mistake, and that the\nnumber of puzzled new users should tell us something.\n\nGranted, we wanted to have a longer grace period for old-timers, but\nlet's face it:\n\n- old-timers will have to edit their configs at some stage anyway,\n\n- for old-timers, it will be a matter of less than a minute,\n\n- new-timers will not spend frustrated hours, and\n\n- since there are many more new-timers now than old-timers, we should\n  cater for them anyway.\n\nTo make it easier for old-timers, the error message was enhanced to\nsuggest how to allow updating the current branch easily.\n\nFor the tests relying on the old behavior, receive.denyCurrentBranch\nwas set to false, to avoid breaking them.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tLet's be honest here, I have not much respect for users who fail \n\tto read up enough to understand what they are doing.\n\n\tBut hearing from those users constantly is really unnerving.  And \n\tit would be a one-time cost to old-timers.\n\n builtin-receive-pack.c      |    9 ++++++---\n t/t5400-send-pack.sh        |    5 ++++-\n t/t5401-update-hooks.sh     |    1 +\n t/t5405-send-pack-rewind.sh |    1 +\n t/t5516-fetch-push.sh       |    1 +\n t/t5517-push-mirror.sh      |    3 ++-\n t/t5521-pull-symlink.sh     |    1 +\n t/t5701-clone-local.sh      |    2 +-\n 8 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 6564a97..e9510fc 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -19,7 +19,7 @@ enum deny_action {\n \n static int deny_deletes = 0;\n static int deny_non_fast_forwards = 0;\n-static enum deny_action deny_current_branch = DENY_WARN;\n+static enum deny_action deny_current_branch = DENY_REFUSE;\n static int receive_fsck_objects;\n static int receive_unpack_limit = -1;\n static int transfer_unpack_limit = -1;\n@@ -239,9 +239,12 @@ static const char *update(struct command *cmd)\n \t\t\t\" that are now in HEAD.\");\n \t\tbreak;\n \tcase DENY_REFUSE:\n-\t\tif (!is_ref_checked_out(name))\n+\t\tif (is_bare_repository() || !is_ref_checked_out(name))\n \t\t\tbreak;\n-\t\terror(\"refusing to update checked out branch: %s\", name);\n+\t\terror(\"refusing to update checked out branch: %s\\n\"\n+\t\t\t\"if you know what you are doing, you can allow it by \"\n+\t\t\t\"setting\\n\\n\"\n+\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n \t\treturn \"branch is currently checked out\";\n \t}\n \ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex b21317d..240380c 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -33,6 +33,7 @@ test_expect_success setup '\n \tgit update-ref HEAD \"$commit\" &&\n \tgit clone ./. victim &&\n \tcd victim &&\n+\tgit config receive.denyCurrentBranch false &&\n \tgit log &&\n \tcd .. &&\n \tgit update-ref HEAD \"$zero\" &&\n@@ -131,13 +132,15 @@ test_expect_success \\\n \tcd .. &&\n \tgit clone parent child && cd child && git push --all &&\n \tcd ../parent &&\n-\tgit branch -a >branches && ! grep origin/master branches\n+\tgit branch -a >branches && ! grep origin/master branches &&\n+\tcd ..\n '\n \n rewound_push_setup() {\n \trm -rf parent child &&\n \tmkdir parent && cd parent &&\n \tgit init && echo one >file && git add file && git commit -m one &&\n+\tgit config receive.denyCurrentBranch false &&\n \techo two >file && git commit -a -m two &&\n \tcd .. &&\n \tgit clone parent child && cd child && git reset --hard HEAD^\ndiff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh\nindex 64f66c9..7f04b64 100755\n--- a/t/t5401-update-hooks.sh\n+++ b/t/t5401-update-hooks.sh\n@@ -18,6 +18,7 @@ test_expect_success setup '\n \tgit update-ref refs/heads/master $commit0 &&\n \tgit update-ref refs/heads/tofail $commit1 &&\n \tgit clone ./. victim &&\n+\tGIT_DIR=victim/.git git config receive.denyCurrentBranch false &&\n \tGIT_DIR=victim/.git git update-ref refs/heads/tofail $commit1 &&\n \tgit update-ref refs/heads/master $commit1 &&\n \tgit update-ref refs/heads/tofail $commit0\ndiff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh\nindex cb9aacc..37c1f23 100755\n--- a/t/t5405-send-pack-rewind.sh\n+++ b/t/t5405-send-pack-rewind.sh\n@@ -6,6 +6,7 @@ test_description='forced push to replace commit we do not have'\n \n test_expect_success setup '\n \n+\tgit config receive.denyCurrentBranch false &&\n \t>file1 && git add file1 && test_tick &&\n \tgit commit -m Initial &&\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4426df9..9ca2730 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -12,6 +12,7 @@ mk_empty () {\n \t(\n \t\tcd testrepo &&\n \t\tgit init &&\n+\t\tgit config receive.denyCurrentBranch false &&\n \t\tmv .git/hooks .git/hooks-disabled\n \t)\n }\ndiff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh\nindex ea49ded..0a31a4e 100755\n--- a/t/t5517-push-mirror.sh\n+++ b/t/t5517-push-mirror.sh\n@@ -19,7 +19,8 @@ mk_repo_pair () {\n \tmkdir mirror &&\n \t(\n \t\tcd mirror &&\n-\t\tgit init\n+\t\tgit init &&\n+\t\tgit config receive.denyCurrentBranch false\n \t) &&\n \tmkdir master &&\n \t(\ndiff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh\nindex 5672b51..736c24e 100755\n--- a/t/t5521-pull-symlink.sh\n+++ b/t/t5521-pull-symlink.sh\n@@ -14,6 +14,7 @@ test_description='pulling from symlinked subdir'\n #\n # The working directory is subdir-link.\n \n+git config receive.denyCurrentBranch false\n mkdir subdir\n echo file >subdir/file\n git add subdir/file\ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex 3559d17..06b2f13 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -119,7 +119,7 @@ test_expect_success 'bundle clone with nonexistent HEAD' '\n test_expect_success 'clone empty repository' '\n \tcd \"$D\" &&\n \tmkdir empty &&\n-\t(cd empty && git init) &&\n+\t(cd empty && git init && git config receive.denyCurrentBranch false) &&\n \tgit clone empty empty-clone &&\n \ttest_tick &&\n \t(cd empty-clone\n-- \n1.6.1.2.531.g6f52\n"},{"id":"102534","messageId":"76718490901291728y2edb0520ie87de783a43c408d@mail.gmail.com","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-30T01:28:39Z","receivedAt":"2009-01-30T01:28:39Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Jan 29, 2009 at 7:34 PM, Johannes Schindelin\n<johannes.schindelin@gmx.de> wrote:\n> Or, the reason could be that we made a design mistake, and that the\n> number of puzzled new users should tell us something.\n\nI happen to have spent some time looking at Mercurial the other day\nsince I was curious how it's evolved since I last played with it, and\nso w/that perspective, I think that git did make a small design\nmistake. With mercurial, pull and push are symmetric opposites.\nNeither, by default, updates the working copy.\n\n(Confusingly for users of both mercurial and git, the mercurial\nequivalent of \"git pull\" is \"hg fetch\". Doh.)\n\nAnyway, I think that this may be what leads to confusion. git pull\nupdates the working copy, and beginners I think expect that push,\nwhich sounds like the opposite of pull, ought do the same thing.\n\nj.\n"},{"id":"102536","messageId":"alpine.DEB.2.00.0901291729540.22558@vellum.laroia.net","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Asheesh Laroia","fromEmail":"asheesh@asheesh.org","sentAt":"2009-01-30T01:32:27Z","receivedAt":"2009-01-30T01:32:27Z","isPatch":true,"sender":{"key":"asheesh@asheesh.org","avatar":"https://avatars.githubusercontent.com/u/25457?v=4"},"body":"On Fri, 30 Jan 2009, Johannes Schindelin wrote:\n\n> \tcase DENY_REFUSE:\n> -\t\tif (!is_ref_checked_out(name))\n> +\t\tif (is_bare_repository() || !is_ref_checked_out(name))\n> \t\t\tbreak;\n> -\t\terror(\"refusing to update checked out branch: %s\", name);\n> +\t\terror(\"refusing to update checked out branch: %s\\n\"\n> +\t\t\t\"if you know what you are doing, you can allow it by \"\n> +\t\t\t\"setting\\n\\n\"\n> +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n\nIt seems like those new users you're trying to protect could use an \nadditional sentence, like:\n\n \t\"A bare repository would not have this issue.\"\n\nor\n\n \t\"You may prefer to have a bare repository instead.\"\n\nBeing told how to do it right is even better than being told that you're \ndoing it wrong. (-:\n\n-- Asheesh.\n\n-- \nFame is a vapor; popularity an accident; the only earthly certainty is\noblivion.\n \t\t-- Mark Twain\n"},{"id":"102537","messageId":"7vwscdbkpd.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"7v4ozhd1wp.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-30T02:18:22Z","receivedAt":"2009-01-30T02:18:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I do not think this improves anything.\n\n@@ -239,9 +239,12 @@ static const char *update(struct command *cmd)\n \t\t\t\" that are now in HEAD.\");\n \t\tbreak;\n \tcase DENY_REFUSE:\n-\t\tif (!is_ref_checked_out(name))\n+\t\tif (is_bare_repository() || !is_ref_checked_out(name))\n \t\t\tbreak;\n-\t\terror(\"refusing to update checked out branch: %s\", name);\n+\t\terror(\"refusing to update checked out branch: %s\\n\"\n+\t\t\t\"if you know what you are doing, you can allow it by \"\n+\t\t\t\"setting\\n\\n\"\n+\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n \t\treturn \"branch is currently checked out\";\n \t}\n \nAs the message I am currently getting from such a push is:\n\n$ git push ../victim-010 next:master\nTotal 0 (delta 0), reused 0 (delta 0)\nwarning: updating the currently checked out branch; this may cause confusion,\nas the index and working tree do not reflect changes that are now in HEAD.\nTo ../victim-010\n   a34a9db..d79e69c  next -> master\n\nwhich is much better than what you did.  It at least tries to explain why\nit is warning, even though I think it has a huge room for improvement.\n\nSaying \"If you know what you are doing\" never works in practice.  It can\nserve as an excuse for you to later say, \"See, I told you so\", but that is\nthe only usefulness of the expression, and everybody, especially the most\nclueless people, *think* they know what they are doing.\n\n\nYou alluded that we wanted to make grace period much longer, but you want\nto cut it short.  I think it is a huge mistake.  The warning has only been\nthere for the last two months, and only can be seen from v1.6.1-rc1 or\nnewer software.  These new people even haven't a chance to learn from the\nexisting warning.\n\n\nI think what would work much better would be a patch that keeps the\nwarn-but-allow as the default, but clarifies the warning message.  Say\nthese things in separate paragraphs, perhaps in red blinking letters:\n\n (1) what symptoms, that are easily observable by the most novice users,\n     are caused by \"index and work tree going out of sync\" the warning\n     talks about, and why that would not be what they want;\n\n (2) if the user did not mean to do it (and the user can tell by observing\n     the symptom described in the previous point), describe what needs to\n     be done to recover from the fallout this push has caused (we do not\n     need a recipe; pointing at a URL or manpage is fine), and what switch\n     to flip to prevent herself from doing it again in the future;\n\n (3) if the user did mean it, and finds the above two big warning\n     annoying, what switch to flip to squelch the warning for future\n     pushes.\n\nThe goal of the warning should be to *force* people *choose*, either to\nsilently-allow (aka DENY_IGNORE) or refuse (DENY_REFUSE), and give enough\ninformation for them to make an informed decision.  We can afford to be\nannoyingly long, loud and verbose there.\n\nOn the other hand, you cannot make the message for DENY_REFUSE annoyingly\nlong, as people may have already chosen to say \"please refuse my push into\na live branch\".\n\nIf you are making \"refuse\" the default, an annoyingly long message is even\nworse.  \"Yeah, thanks for stopping me, but you do not have to remind me\nevery time that I made a mistake in large red letters.  I perfectly well\nknow what I am doing, I perfectly well know that I did not want to push\ninto that branch, I just made a mistake---you do not have to be so loud\".\n\nI suspect that you cannot even be long enough to be informative, not to\nannoy people.\n"},{"id":"102538","messageId":"20090130023040.GR21473@genesis.frugalware.org","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-01-30T02:30:40Z","receivedAt":"2009-01-30T02:30:40Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Fri, Jan 30, 2009 at 01:34:28AM +0100, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:\n> -\t\terror(\"refusing to update checked out branch: %s\", name);\n> +\t\terror(\"refusing to update checked out branch: %s\\n\"\n> +\t\t\t\"if you know what you are doing, you can allow it by \"\n> +\t\t\t\"setting\\n\\n\"\n> +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n\nShouldn't this be\n\ngit config receive.denyCurrentBranch ignore\n\ninstead of \"true\"?\n"},{"id":"102539","messageId":"20090130025546.GA18257@coredump.intra.peff.net","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-30T02:55:46Z","receivedAt":"2009-01-30T02:55:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 30, 2009 at 01:34:28AM +0100, Johannes Schindelin wrote:\n\n> \tLet's be honest here, I have not much respect for users who fail \n> \tto read up enough to understand what they are doing.\n> \n> \tBut hearing from those users constantly is really unnerving.  And \n> \tit would be a one-time cost to old-timers.\n\nI am not personally opposed to changing this default. I seem to\nrecall some opposition when this was brought up initially, but I don't\nrecall any specific reason besides \"change is bad\". Maybe those who\noppose want to summarize their arguments here.\n\nI was hoping that introducing the warning would cause new users to \"get\nit\". But since this warning was put in place, I think we have still\ngotten a few questions on the list about this. I don't know if it simply\nbecause they are on older versions, or if the warning is insufficient.\nIf the former, then perhaps that argues for leaving it a little longer.\n\n>  \tcase DENY_REFUSE:\n> -\t\tif (!is_ref_checked_out(name))\n> +\t\tif (is_bare_repository() || !is_ref_checked_out(name))\n\nNow what is this change about?\n\n> --- a/t/t5701-clone-local.sh\n> +++ b/t/t5701-clone-local.sh\n> @@ -119,7 +119,7 @@ test_expect_success 'bundle clone with nonexistent HEAD' '\n>  test_expect_success 'clone empty repository' '\n>  \tcd \"$D\" &&\n>  \tmkdir empty &&\n> -\t(cd empty && git init) &&\n> +\t(cd empty && git init && git config receive.denyCurrentBranch false) &&\n>  \tgit clone empty empty-clone &&\n>  \ttest_tick &&\n>  \t(cd empty-clone\n\nPerhaps some of these tests would do better to actually just use a bare\nrepository. That would better match the expected workflow for cloning\nempty, anyway.\n\n-Peff\n"},{"id":"102548","messageId":"4982A99C.6070301@viscovery.net","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-01-30T07:17:48Z","receivedAt":"2009-01-30T07:17:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> +\t\terror(\"refusing to update checked out branch: %s\\n\"\n> +\t\t\t\"if you know what you are doing, you can allow it by \"\n> +\t\t\t\"setting\\n\\n\"\n> +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n\nOh, fscking hell, I should have screamed out loudly when Jeff named this\noption \"denyCurrentBranch\" instead of \"allowCurrentBranch\". It's all too\neasy to fall into the trap, like you here.\n\nSigh.\n\n-- J we-don't-need-no-double-negations 6t\n"},{"id":"102549","messageId":"20090130073415.GA27224@coredump.intra.peff.net","threadId":"17449","inReplyTo":"4982A99C.6070301@viscovery.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-30T07:34:15Z","receivedAt":"2009-01-30T07:34:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 30, 2009 at 08:17:48AM +0100, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > +\t\terror(\"refusing to update checked out branch: %s\\n\"\n> > +\t\t\t\"if you know what you are doing, you can allow it by \"\n> > +\t\t\t\"setting\\n\\n\"\n> > +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n> \n> Oh, fscking hell, I should have screamed out loudly when Jeff named this\n> option \"denyCurrentBranch\" instead of \"allowCurrentBranch\". It's all too\n> easy to fall into the trap, like you here.\n\nSorry. ;P\n\nOn the other hand, you also missed the boat on receive.denyDeletes and\nreceive.denyNonFastForwards.\n\n-Peff\n"},{"id":"102586","messageId":"alpine.DEB.1.00.0901301421170.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"20090130073415.GA27224@coredump.intra.peff.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T13:23:02Z","receivedAt":"2009-01-30T13:23:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 Jan 2009, Jeff King wrote:\n\n> On Fri, Jan 30, 2009 at 08:17:48AM +0100, Johannes Sixt wrote:\n> \n> > Johannes Schindelin schrieb:\n> > > +\t\terror(\"refusing to update checked out branch: %s\\n\"\n> > > +\t\t\t\"if you know what you are doing, you can allow it by \"\n> > > +\t\t\t\"setting\\n\\n\"\n> > > +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n> > \n> > Oh, fscking hell, I should have screamed out loudly when Jeff named this\n> > option \"denyCurrentBranch\" instead of \"allowCurrentBranch\". It's all too\n> > easy to fall into the trap, like you here.\n> \n> Sorry. ;P\n> \n> On the other hand, you also missed the boat on receive.denyDeletes and\n> receive.denyNonFastForwards.\n\nThe idea with these is that they are _booleans_, and therefore\n\n\t[receive]\n\t\tdenyDeletes\n\nis something natural to write, because \"denyDeletes\" is _not_ the default.\n\nHowever, with denyCurrentBranch we wanted to change the default in the \nlong run, so I agree it was a not-so-brilliant choice.\n\nCiao,\nDscho\n"},{"id":"102587","messageId":"alpine.DEB.1.00.0901301423120.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"7vwscdbkpd.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T13:24:57Z","receivedAt":"2009-01-30T13:24:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 29 Jan 2009, Junio C Hamano wrote:\n\n> @@ -239,9 +239,12 @@ static const char *update(struct command *cmd)\n>  \t\t\t\" that are now in HEAD.\");\n>  \t\tbreak;\n>  \tcase DENY_REFUSE:\n> -\t\tif (!is_ref_checked_out(name))\n> +\t\tif (is_bare_repository() || !is_ref_checked_out(name))\n>  \t\t\tbreak;\n> -\t\terror(\"refusing to update checked out branch: %s\", name);\n> +\t\terror(\"refusing to update checked out branch: %s\\n\"\n> +\t\t\t\"if you know what you are doing, you can allow it by \"\n> +\t\t\t\"setting\\n\\n\"\n> +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n>  \t\treturn \"branch is currently checked out\";\n>  \t}\n>  \n> As the message I am currently getting from such a push is:\n> \n> $ git push ../victim-010 next:master\n> Total 0 (delta 0), reused 0 (delta 0)\n> 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.\n> To ../victim-010\n>    a34a9db..d79e69c  next -> master\n> \n> which is much better than what you did.  It at least tries to explain why\n> it is warning, even though I think it has a huge room for improvement.\n\nI do not really care about the output.  You are probably right, it should \nbe different from what I proposed.\n\n> You alluded that we wanted to make grace period much longer, but you \n> want to cut it short.  I think it is a huge mistake.  The warning has \n> only been there for the last two months, and only can be seen from \n> v1.6.1-rc1 or newer software.  These new people even haven't a chance to \n> learn from the existing warning.\n\nIn reality, people will not learn from the warning.  Those that are \nold-timers (and who should be warned in the first place, instead of \nrefuses) will just happily ignore that there was a warning: the command \nthey used so often and the worked all the time just happened to work -- \nagain -- no matter what the output is.\n\nBut we are really hurting new users, and let's face it, the balance of \ntime cost currently is in a huge favor of the old-timers there.\n\nNot only are there many more newbies these days than old timers.\n\nNo, the _time_ spent by an old-timer to read an appropriate message and \nfix the setup would be _substantially_ shorter than the _hours_ of \nfrustration a newbie spends on the issue.\n\nAnd we claim to make decentralized repositories easy.\n\n> I think what would work much better would be a patch that keeps the\n> warn-but-allow as the default, but clarifies the warning message.\n\nAs I said, I am _convinced_ that a warning will do nothing at all.  Just \nlike the warning about the dashed commands, nobody who should be concerned \nwill notice it.\n\n>  (1) what symptoms, that are easily observable by the most novice users,\n>      are caused by \"index and work tree going out of sync\" the warning\n>      talks about, and why that would not be what they want;\n> \n>  (2) if the user did not mean to do it (and the user can tell by observing\n>      the symptom described in the previous point), describe what needs to\n>      be done to recover from the fallout this push has caused (we do not\n>      need a recipe; pointing at a URL or manpage is fine), and what switch\n>      to flip to prevent herself from doing it again in the future;\n> \n>  (3) if the user did mean it, and finds the above two big warning\n>      annoying, what switch to flip to squelch the warning for future\n>      pushes.\n> \n> The goal of the warning should be to *force* people *choose*, either to\n> silently-allow (aka DENY_IGNORE) or refuse (DENY_REFUSE), and give enough\n> information for them to make an informed decision.  We can afford to be\n> annoyingly long, loud and verbose there.\n> \n> On the other hand, you cannot make the message for DENY_REFUSE annoyingly\n> long, as people may have already chosen to say \"please refuse my push into\n> a live branch\".\n> \n> If you are making \"refuse\" the default, an annoyingly long message is even\n> worse.  \"Yeah, thanks for stopping me, but you do not have to remind me\n> every time that I made a mistake in large red letters.  I perfectly well\n> know what I am doing, I perfectly well know that I did not want to push\n> into that branch, I just made a mistake---you do not have to be so loud\".\n> \n> I suspect that you cannot even be long enough to be informative, not to\n> annoy people.\n\nLet's reap all the opinions about this issue, and then I'll do the wrap-up \npatch.\n\nBut this is a serious issue that seriously needs to be coped with.  We are \ngetting another generation of \"Git is difficult\" users that way.\n\nCiao,\nDscho\n"},{"id":"102588","messageId":"alpine.DEB.1.00.0901301426150.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"20090130023040.GR21473@genesis.frugalware.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T13:28:39Z","receivedAt":"2009-01-30T13:28:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 Jan 2009, Miklos Vajna wrote:\n\n> On Fri, Jan 30, 2009 at 01:34:28AM +0100, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:\n> > -\t\terror(\"refusing to update checked out branch: %s\", name);\n> > +\t\terror(\"refusing to update checked out branch: %s\\n\"\n> > +\t\t\t\"if you know what you are doing, you can allow it by \"\n> > +\t\t\t\"setting\\n\\n\"\n> > +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n> \n> Shouldn't this be\n> \n> git config receive.denyCurrentBranch ignore\n> \n> instead of \"true\"?\n\nRight.\n\nHowever, as Junio pointed out, we do not want to give this resolution in \nthe error message.  I am now leaning more to something like\n\n\trefusing to update checked out branch '%s' in non-bare repository\n\nHmm?\n\nOld-timers will know \"oh, what the hell, I did not mark my repository as \nbare!\", and new-timers will no longer be confused.\n\nCiao,\nDscho\n"},{"id":"102596","messageId":"alpine.DEB.1.00.0901301429010.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"20090130025546.GA18257@coredump.intra.peff.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T14:11:14Z","receivedAt":"2009-01-30T14:11:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 29 Jan 2009, Jeff King wrote:\n\n> On Fri, Jan 30, 2009 at 01:34:28AM +0100, Johannes Schindelin wrote:\n> \n> > \tLet's be honest here, I have not much respect for users who fail \n> > \tto read up enough to understand what they are doing.\n> > \n> > \tBut hearing from those users constantly is really unnerving.  And \n> > \tit would be a one-time cost to old-timers.\n> \n> I am not personally opposed to changing this default. I seem to\n> recall some opposition when this was brought up initially, but I don't\n> recall any specific reason besides \"change is bad\". Maybe those who\n> oppose want to summarize their arguments here.\n\nWe like to play it safe when changing behavior that does not meet \nexpectations of old-timers.\n\nFor example, all those early adopters who have forks of the linux-2.6 \nrepository (and probably that repository itself, too) do not have \ncore.bare set.\n\nSo whenever an old-timer would upgrade to a new Git _with_ my patch, they \nwould need to change their setup.\n\nA one-time cost.\n\nAnd far easier to accomodate than the push for non-dashed commands (which \npeople still seem to grumble about, even if they should have realized by \nnow that calling Git through the wrapper exclusively brings so many \nadvantages).\n\n> I was hoping that introducing the warning would cause new users to \"get \n> it\". But since this warning was put in place, I think we have still \n> gotten a few questions on the list about this. I don't know if it simply \n> because they are on older versions, or if the warning is insufficient. \n> If the former, then perhaps that argues for leaving it a little longer.\n\nI would argue it is because users cannot read :-)\n\n> >  \tcase DENY_REFUSE:\n> > -\t\tif (!is_ref_checked_out(name))\n> > +\t\tif (is_bare_repository() || !is_ref_checked_out(name))\n> \n> Now what is this change about?\n\nI missed the fact that is_ref_checked_out() already checked for that.\n\n> > --- a/t/t5701-clone-local.sh\n> > +++ b/t/t5701-clone-local.sh\n> > @@ -119,7 +119,7 @@ test_expect_success 'bundle clone with nonexistent HEAD' '\n> >  test_expect_success 'clone empty repository' '\n> >  \tcd \"$D\" &&\n> >  \tmkdir empty &&\n> > -\t(cd empty && git init) &&\n> > +\t(cd empty && git init && git config receive.denyCurrentBranch false) &&\n> >  \tgit clone empty empty-clone &&\n> >  \ttest_tick &&\n> >  \t(cd empty-clone\n> \n> Perhaps some of these tests would do better to actually just use a bare\n> repository.\n\nRight.  I just ran out of time, but did not want to hide the patch from \nthe community.\n\n> That would better match the expected workflow for cloning empty, anyway.\n\nWell, I did not want to mix up the two of them.  Besides, I have this \npatch in my personal tree for quite some time now, always wanting to clean \nit up enough to send it...)\n\nCiao,\nDscho\n"},{"id":"102599","messageId":"20090130143521.GA31673@coredump.intra.peff.net","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901301421170.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-30T14:35:21Z","receivedAt":"2009-01-30T14:35:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 30, 2009 at 02:23:02PM +0100, Johannes Schindelin wrote:\n\n> > On the other hand, you also missed the boat on receive.denyDeletes and\n> > receive.denyNonFastForwards.\n> \n> The idea with these is that they are _booleans_, and therefore\n> \n> \t[receive]\n> \t\tdenyDeletes\n> \n> is something natural to write, because \"denyDeletes\" is _not_ the default.\n> \n> However, with denyCurrentBranch we wanted to change the default in the \n> long run, so I agree it was a not-so-brilliant choice.\n\nGood point. I do agree that allowCurrentBranch would have been a better\nname, but I don't know that it is worth the pain now of adding new\nconfig plus supporting the old name forever.\n\n-Peff\n"},{"id":"102612","messageId":"76718490901300817x3f31460k59b6fe75d136372d@mail.gmail.com","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-30T16:17:49Z","receivedAt":"2009-01-30T16:17:49Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Jan 29, 2009 at 7:34 PM, Johannes Schindelin\n<johannes.schindelin@gmx.de> wrote:\n> Many, many users set up non-bare repositories on their server, and are\n> confused that the working directory is not updated.\n\nThis comes up on the list from time-to-time and is even in the FAQ. It\nhas even been suggested that HEAD be detached when pushing into a\nnon-bare repository, but I am not suggesting that again.\n\nI wonder if it might be helpful to teach clone to setup a push line in\nthe cloned repo. i.e.:\n\n[remote \"origin\"]\n\turl = ...\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n\tpush = refs/heads/*:refs/remotes/origin/*\n\nThis could be a configurable default behavior when cloning from a\nnon-bare repo (can that be determined?) and/or as a switch\n(--satellite perhaps?).\n\nj.\n"},{"id":"102617","messageId":"20090130162845.GA6963@sigill.intra.peff.net","threadId":"17449","inReplyTo":"76718490901300817x3f31460k59b6fe75d136372d@mail.gmail.com","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-30T16:28:45Z","receivedAt":"2009-01-30T16:28:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 30, 2009 at 11:17:49AM -0500, Jay Soffian wrote:\n\n> I wonder if it might be helpful to teach clone to setup a push line in\n> the cloned repo. i.e.:\n> \n> [remote \"origin\"]\n> \turl = ...\n> \tfetch = +refs/heads/*:refs/remotes/origin/*\n> \tpush = refs/heads/*:refs/remotes/origin/*\n\nThat refspec doesn't make sense, since the downstream is not the\n\"origin\" to the upstream repo. But I don't think this is a good\nsolution; it is fundamentally changing the layout of pushed branches in\nthe upstream repo, which is going to cause a lot of confusion.\n\n> This could be a configurable default behavior when cloning from a\n> non-bare repo (can that be determined?) and/or as a switch\n> (--satellite perhaps?).\n\nI don't think you can tell whether a repo you are cloning is bare.\n\n-Peff\n"},{"id":"102619","messageId":"20090130163317.GB6963@sigill.intra.peff.net","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901301423120.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-01-30T16:33:17Z","receivedAt":"2009-01-30T16:33:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 30, 2009 at 02:24:57PM +0100, Johannes Schindelin wrote:\n\n> Let's reap all the opinions about this issue, and then I'll do the wrap-up \n> patch.\n\nI thought what Junio said was very reasonable (improve the message and\ngive it some more time to work).\n\nBut I honestly do not care that much either way. I probably would have\nmade the original patch default to \"deny\" if not for discussion\nrecommending to be conservative. On the other hand, I don't think we\nhave really given the \"warning\" approach enough time to see whether it\nis working (and I don't necessarily disagree with your gut feeling that\nit won't work; I am undecided, which leads me to want more data).\n\n-Peff\n"},{"id":"102622","messageId":"alpine.DEB.1.00.0901301754110.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"20090130163317.GB6963@sigill.intra.peff.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T16:55:36Z","receivedAt":"2009-01-30T16:55:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 Jan 2009, Jeff King wrote:\n\n> On Fri, Jan 30, 2009 at 02:24:57PM +0100, Johannes Schindelin wrote:\n> \n> > Let's reap all the opinions about this issue, and then I'll do the \n> > wrap-up patch.\n> \n> I thought what Junio said was very reasonable (improve the message and \n> give it some more time to work).\n> \n> But I honestly do not care that much either way. I probably would have \n> made the original patch default to \"deny\" if not for discussion \n> recommending to be conservative. On the other hand, I don't think we \n> have really given the \"warning\" approach enough time to see whether it \n> is working (and I don't necessarily disagree with your gut feeling that \n> it won't work; I am undecided, which leads me to want more data).\n\nIt is not working:\n\nhttp://groups.google.com/group/msysgit/msg/55b1aa03fbbbefba?dmode=source\n\n(I am simply assuming that the mentioned 1.6.1-preview has the \nwarning, since denyCurrentBranch is in v1.6.1-rc1~59^2, and I am too short \non time to check it in detail (which would mean finding a Windows machine \nand running a test)).\n\nCiao,\nDscho\n"},{"id":"102624","messageId":"alpine.DEB.1.00.0901301756560.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"76718490901300817x3f31460k59b6fe75d136372d@mail.gmail.com","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T17:01:15Z","receivedAt":"2009-01-30T17:01:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 Jan 2009, Jay Soffian wrote:\n\n> On Thu, Jan 29, 2009 at 7:34 PM, Johannes Schindelin\n> <johannes.schindelin@gmx.de> wrote:\n> > Many, many users set up non-bare repositories on their server, and are \n> > confused that the working directory is not updated.\n> \n> This comes up on the list from time-to-time and is even in the FAQ.\n\nSo much so that it is high time we admitted that we have a design bug \nthere.\n\n> It has even been suggested that HEAD be detached when pushing into a \n> non-bare repository, but I am not suggesting that again.\n\nNo, because that would be as wrong as trying to update the working \ndirectory in any other way.  (Not only is it possible that you are a \ngit-shell user, in which case you have no business meddling with the \nworking directory -- or the symbolic ref HEAD -- to begin with, but you \nalso run into the problem that you might not know where the working \ndirectory is at all, let alone if there is one.)\n\nSo it is a good thing you are not suggesting it again.\n\n> I wonder if it might be helpful to teach clone to setup a push line in\n> the cloned repo. i.e.:\n> \n> [remote \"origin\"]\n> \turl = ...\n> \tfetch = +refs/heads/*:refs/remotes/origin/*\n> \tpush = refs/heads/*:refs/remotes/origin/*\n> \n> This could be a configurable default behavior when cloning from a\n> non-bare repo (can that be determined?) and/or as a switch\n> (--satellite perhaps?).\n\nAs Peff commented, this would be horribly wrong if the remote has a \ndifferent \"origin\" remote.  Not forcing the push does not help either, it \nis still wrong.\n\nBut I think there is an even more fundamental problem: You do not want \nthat default push.  We have \"push only those refs the remote and the local \nrepository share\" rule for a reason.  It is way too easy to publish \nsomething you did not mean to publish otherwise.\n\nCiao,\nDscho\n"},{"id":"102635","messageId":"76718490901301050h1f0f5b2bq902de384d954d99b@mail.gmail.com","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901301756560.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-01-30T18:50:25Z","receivedAt":"2009-01-30T18:50:25Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Fri, Jan 30, 2009 at 12:01 PM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> As Peff commented, this would be horribly wrong if the remote has a\n> different \"origin\" remote.  Not forcing the push does not help either, it\n> is still wrong.\n\nGot it. Here was my impression of the work-flow we're trying to help\nbeginners with:\n\nmachineA$ mkdir repo\nmachineA$ cd repo\nmachineA$ git init\nmachineA$ add, commit, add, commit...\n\nmachineB$ git clone ssh://machine1/repo\nmachineB$ add, commit, add, commit...\nmachineB$ git push\n\n(And if my impression is wrong, then stop me right here and I'll\nshut-up on this thread.)\n\nIn this case, the clone operation sets up the repo on B to fetch all\nof the branches from the repo on A. But it doesn't do anything to help\nthe user with pushing the repo from B back to machine A. So perhaps:\n\ngit clone --origin machineA --push-as machineB ssh://machineA/repo\n\n[remote \"machineA\"]\n\turl = ...\n\tfetch = +refs/heads/*:refs/remotes/machineA/*\n\tpush = +refs/heads/*:refs/remotes/machineB/*\n\nNow fetch and push are symmetric operations on machineB.\n\n> But I think there is an even more fundamental problem: You do not want\n> that default push.  We have \"push only those refs the remote and the local\n> repository share\" rule for a reason.  It is way too easy to publish\n> something you did not mean to publish otherwise.\n\nI don't have a good answer for that, other than to say that if user is\nsetting up symmetric repositories, user wants to push everything.\n\nj.\n"},{"id":"102637","messageId":"alpine.DEB.1.00.0901301959300.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"76718490901301050h1f0f5b2bq902de384d954d99b@mail.gmail.com","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-30T19:03:22Z","receivedAt":"2009-01-30T19:03:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 30 Jan 2009, Jay Soffian wrote:\n\n> On Fri, Jan 30, 2009 at 12:01 PM, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> > As Peff commented, this would be horribly wrong if the remote has a \n> > different \"origin\" remote.  Not forcing the push does not help either, \n> > it is still wrong.\n> \n> Got it. Here was my impression of the work-flow we're trying to help \n> beginners with:\n> \n> machineA$ mkdir repo\n> machineA$ cd repo\n> machineA$ git init\n> machineA$ add, commit, add, commit...\n> \n> machineB$ git clone ssh://machine1/repo\n> machineB$ add, commit, add, commit...\n> machineB$ git push\n> \n> (And if my impression is wrong, then stop me right here and I'll\n> shut-up on this thread.)\n\nI think your impression is not wrong.\n\nBUT.\n\nYou cannot just cater for one workflow and fsck the other workflows over.\n\nYou'll have to devise a method that helps the workflow you are interested \nin, but leaves the others alone.\n\nExample: the thing I heard most often was \"I want to start this \nrepository, but there is nothing in there yet, yet I want other people to \nclone it already so they'll see something when I do.\"\n\nI admit, it does not strike me sensible, but so does cloning an empty \nrepository.  As I could not understand how people would want to vote for \nBush.  Yet they did, so I guess I'll have to live with it.\n\nCiao,\nDscho\n"},{"id":"102667","messageId":"20090131095622.6117@nanako3.lavabit.com","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901301959300.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-01-31T00:56:22Z","receivedAt":"2009-01-31T00:56:22Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n\n> You cannot just cater for one workflow and fsck the other workflows over.\n>\n> You'll have to devise a method that helps the workflow you are interested \n> in, but leaves the others alone.\n\nI think you'd want to repeat that to yourself when you propose to switch\nthe default for denyCurrentcurrentBranch config to \"true\" too hastily the\nnext time?\n\nI don't think your patch matches the tradition of how defaults are changed\nin git project. You don't introduce a large change just after the maintainer\nhints about going into a freeze for 1.X.Y release when Y isn't zero.\n\nI assume that everybody, including the maintainer who is too heavyweight\nand has too much inertia to accept too sudden a change of the course,\nwants to eventually make the default to deny pushing to the current\nbranch. But I think such a change should come at 1.7.0 release at the\nearliest, and a constructive thing to do is to put in a patch to 1.6.2\nthat helps the users with the eventual transition.\n\nHow about doing these before the 1.7.0 release?\n\n 1. Add some code to git-clone to set the config to \"deny\" if it is\n    not a bare repository. The reason I think this makes sense is\n    because the reason why old-timers want to push into the current\n    branch is because they are used to the old layout that doesn't use\n    separate remotes. If they use today's git-clone and still want to\n    use the old layout, they need to update the config file in the new\n    clone anyway. The \"deny\" is just another thing for them to fix at\n    that point.\n\n    I suspect that Junio will not like this in 1.6.2 because it is an\n    unannounced and unplanned change in behavior, but I think it is a\n    reasonable preparatory step, probably in 1.6.3, before you change\n    the default to deny in release 1.7.0.\n\n 2. Reword the warning message as Junio suggested in his response. I\n    don't know the details of the code very well, but I think you can\n    tell a repository that doesn't have the config at all from a\n    repository that has the config set to \"warn\", and you can use\n    \"annoyingly long\" (in Junio's words) message to force the user set\n    the config to a desired value only when pushing into the former\n    kind, and say that the default will change to deny in release\n    1.7.0. When pushing into the latter, the warning message can be\n    shorter (probably you can say \"warning: updating the current\n    branch in a non-bare repository\" and nothing else).\n\n 3. Reword the error message as you proposed to say \"error: won't\n    update the current branch in a non-bare repository\", without\n    saying anything else. You want to eventually change the default to\n    deny, and there is no point to teach how to allow it to people who\n    set the config to deny themselves, nor to new people who created\n    their repository with updated git-clone.\n\n    I think this makes sense to do in 1.6.2 release, because the only\n    people who will see this message will be the people who set the\n    config to deny themselves, especially if you postpone the change\n    to git-clone for the upcoming release.\n\nWhat do people think?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"102733","messageId":"7vy6wr0wvi.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"20090131095622.6117@nanako3.lavabit.com","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-01T01:27:45Z","receivedAt":"2009-02-01T01:27:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> How about doing these before the 1.7.0 release?\n> ...\n> What do people think?\n\nI haven't manged to convince myself about the \"git init\" change (I have\nthe code and also I've looked at the extent of damage the change causes to\nthe existing test suite), but at least I think it is a sensible suggestion\nto differentiate between unconfigured-wwwand-defaults-to-warn case and\nconfigured-to-warn-so-we-warn case.  Something like this.\n\n-- >8 --\nSubject: [PATCH] receive-pack: explain what to do when push updates the current branch\n\nThis makes \"git push\" issue a more detailed instruction when a user pushes\ninto the current branch of a non-bare repository without having an\nexplicit configuration set to receive.denycurrentbranch.  In such a case,\nit will also tell the user that the default will change to refusal in a\nfuture version of git.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-receive-pack.c |   58 +++++++++++++++++++++++++++++++++++------------\n t/t5516-fetch-push.sh  |    6 ++--\n 2 files changed, 46 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 6564a97..f2c94fc 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -12,6 +12,7 @@\n static const char receive_pack_usage[] = \"git-receive-pack <git-dir>\";\n \n enum deny_action {\n+\tDENY_UNCONFIGURED,\n \tDENY_IGNORE,\n \tDENY_WARN,\n \tDENY_REFUSE,\n@@ -19,7 +20,7 @@ enum deny_action {\n \n static int deny_deletes = 0;\n static int deny_non_fast_forwards = 0;\n-static enum deny_action deny_current_branch = DENY_WARN;\n+static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n static int receive_fsck_objects;\n static int receive_unpack_limit = -1;\n static int transfer_unpack_limit = -1;\n@@ -214,6 +215,35 @@ static int is_ref_checked_out(const char *ref)\n \treturn !strcmp(head, ref);\n }\n \n+static char *warn_unconfigured_deny_msg[] = {\n+\t\"Updating the currently checked out branch may cause confusion,\",\n+\t\"as the index and work tree do not reflect changes that are in HEAD.\"\n+\t\"As a result, you may see the changes you just pushed into it\",\n+\t\"reverted when you run 'git diff' over there, and you may want\",\n+\t\"to run 'git reset --hard' before starting to work to recover.\",\n+\t\"\",\n+\t\"You can set 'receive.denyCurrentBranch' configuration variable to\",\n+\t\"'refuse' in the repository to forbid pushing into the current branch\",\n+\t\"of it.\"\n+\t\"\",\n+\t\"To allow pushing into the current branch, you can set it to 'ignore';\",\n+\t\"but this is not recommended unless you really know what you are doing.\",\n+\t\"\",\n+\t\"To squelch this message, you can set it to 'warn'.\",\n+\t\"\",\n+\t\"Note that the default will change in a future version of git\",\n+\t\"to refuse updating the currentbranch unless you have the\",\n+\t\"configuration variable set to either 'ignore' or 'warn'.\"\n+};\n+\n+static void warn_unconfigured_deny(void)\n+{\n+\tint i;\n+\tfor (i = 0; i < ARRAY_SIZE(warn_unconfigured_deny_msg); i++)\n+\t\twarning(warn_unconfigured_deny_msg[i]);\n+}\n+\n+\n static const char *update(struct command *cmd)\n {\n \tconst char *name = cmd->ref_name;\n@@ -227,22 +257,20 @@ static const char *update(struct command *cmd)\n \t\treturn \"funny refname\";\n \t}\n \n-\tswitch (deny_current_branch) {\n-\tcase DENY_IGNORE:\n-\t\tbreak;\n-\tcase DENY_WARN:\n-\t\tif (!is_ref_checked_out(name))\n+\tif (is_ref_checked_out(name)) {\n+\t\tswitch (deny_current_branch) {\n+\t\tcase DENY_IGNORE:\n \t\t\tbreak;\n-\t\twarning(\"updating the currently checked out branch; this may\"\n-\t\t\t\" cause confusion,\\n\"\n-\t\t\t\"as the index and working tree do not reflect changes\"\n-\t\t\t\" that are now in HEAD.\");\n-\t\tbreak;\n-\tcase DENY_REFUSE:\n-\t\tif (!is_ref_checked_out(name))\n+\t\tcase DENY_UNCONFIGURED:\n+\t\tcase DENY_WARN:\n+\t\t\twarning(\"updating the current branch\");\n+\t\t\tif (deny_current_branch == DENY_UNCONFIGURED)\n+\t\t\t\twarn_unconfigured_deny();\n \t\t\tbreak;\n-\t\terror(\"refusing to update checked out branch: %s\", name);\n-\t\treturn \"branch is currently checked out\";\n+\t\tcase DENY_REFUSE:\n+\t\t\terror(\"refusing to update checked out branch: %s\", name);\n+\t\t\treturn \"branch is currently checked out\";\n+\t\t}\n \t}\n \n \tif (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 4426df9..89649e7 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -492,7 +492,7 @@ test_expect_success 'warn on push to HEAD of non-bare repository' '\n \t\tgit checkout master &&\n \t\tgit config receive.denyCurrentBranch warn) &&\n \tgit push testrepo master 2>stderr &&\n-\tgrep \"warning.*this may cause confusion\" stderr\n+\tgrep \"warning: updating the current branch\" stderr\n '\n \n test_expect_success 'deny push to HEAD of non-bare repository' '\n@@ -510,7 +510,7 @@ test_expect_success 'allow push to HEAD of bare repository (bare)' '\n \t\tgit config receive.denyCurrentBranch true &&\n \t\tgit config core.bare true) &&\n \tgit push testrepo master 2>stderr &&\n-\t! grep \"warning.*this may cause confusion\" stderr\n+\t! grep \"warning: updating the current branch\" stderr\n '\n \n test_expect_success 'allow push to HEAD of non-bare repository (config)' '\n@@ -520,7 +520,7 @@ test_expect_success 'allow push to HEAD of non-bare repository (config)' '\n \t\tgit config receive.denyCurrentBranch false\n \t) &&\n \tgit push testrepo master 2>stderr &&\n-\t! grep \"warning.*this may cause confusion\" stderr\n+\t! grep \"warning: updating the current branch\" stderr\n '\n \n test_expect_success 'fetch with branches' '\n-- \n1.6.1.2.312.g5be3c\n"},{"id":"102741","messageId":"7vk58bylxv.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"7vy6wr0wvi.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-01T01:39:56Z","receivedAt":"2009-02-01T01:39:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I haven't manged to convince myself about the \"git init\" change (I have\n> the code and also I've looked at the extent of damage the change causes to\n> the existing test suite),...\n\nAnd here is such a patch.\n\n-- >8 --\nSubject: [PATCH] Set receive.denyCurrentBranch to true in a new non-bare repository\n\nThis prepares new people to get used to the default planned for 1.7.0;\nnecessary adjustments are done to many tests, as they all assumed the\ntraditional \"only warn but allow updating\" semantics.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-init-db.c           |    2 ++\n t/t5400-send-pack.sh        |    2 ++\n t/t5401-update-hooks.sh     |    1 +\n t/t5405-send-pack-rewind.sh |    1 +\n t/t5516-fetch-push.sh       |    1 +\n t/t5517-push-mirror.sh      |    3 ++-\n t/t5521-pull-symlink.sh     |   20 +++++++++++++-------\n t/t5701-clone-local.sh      |    4 +++-\n 8 files changed, 25 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin-init-db.c b/builtin-init-db.c\nindex ee3911f..26c10cc 100644\n--- a/builtin-init-db.c\n+++ b/builtin-init-db.c\n@@ -250,6 +250,8 @@ static int create_default_files(const char *template_path)\n \t\t    strcmp(git_dir + strlen(work_tree), \"/.git\")) {\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n \t\t}\n+\t\tif (!reinit)\n+\t\t\tgit_config_set(\"receive.denyCurrentBranch\", \"refuse\");\n \t}\n \n \tif (!reinit) {\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex b21317d..5c9c277 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -33,6 +33,7 @@ test_expect_success setup '\n \tgit update-ref HEAD \"$commit\" &&\n \tgit clone ./. victim &&\n \tcd victim &&\n+\tgit config receive.denyCurrentBranch warn &&\n \tgit log &&\n \tcd .. &&\n \tgit update-ref HEAD \"$zero\" &&\n@@ -138,6 +139,7 @@ rewound_push_setup() {\n \trm -rf parent child &&\n \tmkdir parent && cd parent &&\n \tgit init && echo one >file && git add file && git commit -m one &&\n+\tgit config receive.denyCurrentBranch warn &&\n \techo two >file && git commit -a -m two &&\n \tcd .. &&\n \tgit clone parent child && cd child && git reset --hard HEAD^\ndiff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh\nindex 64f66c9..325714e 100755\n--- a/t/t5401-update-hooks.sh\n+++ b/t/t5401-update-hooks.sh\n@@ -18,6 +18,7 @@ test_expect_success setup '\n \tgit update-ref refs/heads/master $commit0 &&\n \tgit update-ref refs/heads/tofail $commit1 &&\n \tgit clone ./. victim &&\n+\tGIT_DIR=victim/.git git config receive.denyCurrentBranch warn &&\n \tGIT_DIR=victim/.git git update-ref refs/heads/tofail $commit1 &&\n \tgit update-ref refs/heads/master $commit1 &&\n \tgit update-ref refs/heads/tofail $commit0\ndiff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh\nindex cb9aacc..4bda18a 100755\n--- a/t/t5405-send-pack-rewind.sh\n+++ b/t/t5405-send-pack-rewind.sh\n@@ -8,6 +8,7 @@ test_expect_success setup '\n \n \t>file1 && git add file1 && test_tick &&\n \tgit commit -m Initial &&\n+\tgit config receive.denyCurrentBranch warn &&\n \n \tmkdir another && (\n \t\tcd another &&\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 89649e7..a67ebd0 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -12,6 +12,7 @@ mk_empty () {\n \t(\n \t\tcd testrepo &&\n \t\tgit init &&\n+\t\tgit config receive.denyCurrentBranch warn &&\n \t\tmv .git/hooks .git/hooks-disabled\n \t)\n }\ndiff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh\nindex ea49ded..e2ad260 100755\n--- a/t/t5517-push-mirror.sh\n+++ b/t/t5517-push-mirror.sh\n@@ -19,7 +19,8 @@ mk_repo_pair () {\n \tmkdir mirror &&\n \t(\n \t\tcd mirror &&\n-\t\tgit init\n+\t\tgit init &&\n+\t\tgit config receive.denyCurrentBranch warn\n \t) &&\n \tmkdir master &&\n \t(\ndiff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh\nindex 5672b51..66b5ac1 100755\n--- a/t/t5521-pull-symlink.sh\n+++ b/t/t5521-pull-symlink.sh\n@@ -14,13 +14,19 @@ test_description='pulling from symlinked subdir'\n #\n # The working directory is subdir-link.\n \n-mkdir subdir\n-echo file >subdir/file\n-git add subdir/file\n-git commit -q -m file\n-git clone -q . clone-repo\n-ln -s clone-repo/subdir/ subdir-link\n-\n+test_expect_success setup '\n+\tmkdir subdir &&\n+\techo file >subdir/file &&\n+\tgit add subdir/file &&\n+\tgit commit -q -m file &&\n+\tgit clone -q . clone-repo &&\n+\tln -s clone-repo/subdir/ subdir-link &&\n+\t(\n+\t\tcd clone-repo &&\n+\t\tgit config receive.denyCurrentBranch warn\n+\t) &&\n+\tgit config receive.denyCurrentBranch warn\n+'\n \n # Demonstrate that things work if we just avoid the symlink\n #\ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex 3559d17..10accc2 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -119,7 +119,9 @@ test_expect_success 'bundle clone with nonexistent HEAD' '\n test_expect_success 'clone empty repository' '\n \tcd \"$D\" &&\n \tmkdir empty &&\n-\t(cd empty && git init) &&\n+\t(cd empty &&\n+\t git init &&\n+\t git config receive.denyCurrentBranch warn) &&\n \tgit clone empty empty-clone &&\n \ttest_tick &&\n \t(cd empty-clone\n-- \n1.6.1.2.312.g5be3c\n"},{"id":"102743","messageId":"7v63juzz9m.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"20090131095622.6117@nanako3.lavabit.com","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-01T02:06:45Z","receivedAt":"2009-02-01T02:06:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> I assume that everybody, including the maintainer who is too heavyweight\n> and has too much inertia to accept too sudden a change of the course,\n> wants to eventually make the default to deny pushing to the current\n> branch. But I think such a change should come at 1.7.0 release at the\n> earliest, and a constructive thing to do is to put in a patch to 1.6.2\n> that helps the users with the eventual transition.\n\nI am not opposed to eventually change the default to refuse at some point,\nbut I have to say that now would not be the best time to do so.  Jeff's\n986e823 (receive-pack: detect push to current branch of non-bare repo,\n2008-11-08) that is v1.6.1-rc1~59^2 was the one we started warning about\nthis, and we only had one major release since then, and I'd love to see a\nsolid rc or even the final release by mid February.\n\nBy the way, I do not appreciate other people who I have never met\nspeculate about my body mass very much.  I am on the skinner end of the\nspectrum, if you need to know.\n"},{"id":"102750","messageId":"1233459475.17688.128.camel@maia.lan","threadId":"17449","inReplyTo":"7v63juzz9m.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-02-01T03:37:55Z","receivedAt":"2009-02-01T03:37:55Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On Sat, 2009-01-31 at 18:06 -0800, Junio C Hamano wrote:\n> Nanako Shiraishi <nanako3@lavabit.com> writes:\n> \n> > I assume that everybody, including the maintainer who is too heavyweight\n> > and has too much inertia to accept too sudden a change of the course,\n> > wants to eventually make the default to deny pushing to the current\n> > branch. But I think such a change should come at 1.7.0 release at the\n> > earliest, and a constructive thing to do is to put in a patch to 1.6.2\n> > that helps the users with the eventual transition.\n> \n> I am not opposed to eventually change the default to refuse at some point,\n> but I have to say that now would not be the best time to do so.  Jeff's\n> 986e823 (receive-pack: detect push to current branch of non-bare repo,\n> 2008-11-08) that is v1.6.1-rc1~59^2 was the one we started warning about\n> this, and we only had one major release since then, and I'd love to see a\n> solid rc or even the final release by mid February.\n\nPersonally I think it's worth fast tracking, because I think very few\npeople are actually using push to a checked out branch whereas many\npeople are confused by the behaviour.  I just can't understand the\nresistance to this safety feature.  People who encounter the bug can\njust change the setting and move on... it seems like an argument based\non \"principles\", usually a sign that one has run out of actual\narguments..\n\n> By the way, I do not appreciate other people who I have never met\n> speculate about my body mass very much.  I am on the skinner end of the\n> spectrum, if you need to know.\n\nlol.  It was a metaphorical use of the term from my reading ;-)\n\nSam.\n"},{"id":"102780","messageId":"7vbptlsuyv.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"1233459475.17688.128.camel@maia.lan","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-01T21:33:44Z","receivedAt":"2009-02-01T21:33:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sam Vilain <sam@vilain.net> writes:\n\n> I just can't understand the\n> resistance to this safety feature.  People who encounter the bug can\n> just change the setting and move on... it seems like an argument based\n> on \"principles\", usually a sign that one has run out of actual\n> arguments..\n\nThere is no resitance to any safety feature.  The resistance is to\nsomething entirely different.\n\nThe change we all share as the desired end result is to introduce a\ndifferent behaviour, even if it is a better one, that will deliberately\nbreak people's working setup.\n\nThe end result being better does not justify breaking people's setup.  You\nneed to help people whose setups will be broken prepare for such a change.\n\nPerhaps you already forgot the fiasco after 1.6.0, which moved tons of\ngit-foo scripts out of the users' way.  It resulted in a better layout in\nthe end, but we knew it would break people's working setup from the\nbeginning.\n\nIt was a change that many people argued for, saying that too many commands\nin the path scared new people, that we thought we planned very carefully,\nand that we thought we gave ample warning to existing users.  Yet it\nresulted in a huge fallout.\n\nYou probably forgot about it already, but that is because you weren't the\none who had to take the flak.  I was, and I haven't forgotten.\n\nThe resistance is to an irresponsible transition strategy that causes pain\nunnecessarily.  We need to improve the situation incrementally, helping\nnew people a bit in one step while not hurting old people along the way,\naiming to help everybody more in the end.\n\nOne of the concluding comments after the 1.6.0 fiasco was that the\nold-timers _heard about_ the upcoming change, but were too busy to stop\nand think to realize that it is any urgent that they need to prepare for\nit, and we should have warned them more actively.  In the end, they didn't\nmind the change itself per-se because we had an escape hatch (i.e. to\nprepend the output from \"git --exec-path\" to the PATH in your script), but\nthe primary pain was having to adjust their setup on _our_ timetable, not\ntheirs.\n\nEven k.org, which is one of the early adopters of new releases among the\nlarger sites (with larger proportion of old timers), has started using the\nversion with the \"updating the branch you have checked out\" warning (which\nhappened in v1.6.1) fairly recently, and during the discussion we noticed\nthat the warning didn't say we will be switching the default to \"refuse\"\nany time soon.  We haven't yet given them enough advance warning telling\nthem they will have to go running around flipping the bit in their\nrepositories.  Dismissing the issue by saying \"old-timers can simply flip\nthe configuration once\" makes an irresponsible argument for repeating the\nsame mistake of 1.6.0.  That is what I am resisting to.\n\nI think the plan outlined in this thread would ease the transition in much\nbetter way, and the patch I sent yesterday uses a deliberately loooooooong\nwarning message whose primary purpose is to be annoyingly obvious that the\nusers need to adjust their configuration now, so that the eventual change\nin the default we will make won't inconvenience them.\n\nI was reluctant to change the default for new repositories to refuse\nduring 1.6.2 timeframe, but I think with the attached patch on top of the\nsecond patch I sent out earlier, it would force people choose before they\ndo any real damange to their repositories, and having it early would make\nthe overall transition plan smoother.\n\n builtin-init-db.c      |    2 +-\n builtin-receive-pack.c |   26 ++++++++++++++++++++++++++\n 2 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git c/builtin-init-db.c w/builtin-init-db.c\nindex 26c10cc..ea2765c 100644\n--- c/builtin-init-db.c\n+++ w/builtin-init-db.c\n@@ -251,7 +251,7 @@ static int create_default_files(const char *template_path)\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n \t\t}\n \t\tif (!reinit)\n-\t\t\tgit_config_set(\"receive.denyCurrentBranch\", \"refuse\");\n+\t\t\tgit_config_set(\"receive.denyCurrentBranch\", \"refuse-with-insn\");\n \t}\n \n \tif (!reinit) {\ndiff --git c/builtin-receive-pack.c w/builtin-receive-pack.c\nindex f2c94fc..82d372f 100644\n--- c/builtin-receive-pack.c\n+++ w/builtin-receive-pack.c\n@@ -16,6 +16,7 @@ enum deny_action {\n \tDENY_IGNORE,\n \tDENY_WARN,\n \tDENY_REFUSE,\n+\tDENY_REFUSE_WITH_INSN,\n };\n \n static int deny_deletes = 0;\n@@ -39,6 +40,8 @@ static enum deny_action parse_deny_action(const char *var, const char *value)\n \t\t\treturn DENY_WARN;\n \t\tif (!strcasecmp(value, \"refuse\"))\n \t\t\treturn DENY_REFUSE;\n+\t\tif (!strcasecmp(value, \"refuse-with-insn\"))\n+\t\t\treturn DENY_REFUSE_WITH_INSN;\n \t}\n \tif (git_config_bool(var, value))\n \t\treturn DENY_REFUSE;\n@@ -236,6 +239,20 @@ static char *warn_unconfigured_deny_msg[] = {\n \t\"configuration variable set to either 'ignore' or 'warn'.\"\n };\n \n+static char *refuse_current_insn_msg[] = {\n+\t\"By default, updating the current branch in a non-bare repository\",\n+\t\"is denied, because it will make the index and work tree inconsistent\",\n+\t\"with what you pushed, and will require 'git reset --hard' to match\",\n+\t\"the work tree to HEAD.\",\n+\t\"\",\n+\t\"You can set 'receive.denyCurrentBranch' configuration variable to\",\n+\t\"'ignore' or 'warn' in the repository to allow pushing into the\",\n+\t\"current branch of it; this is not recommended unless you really know\",\n+\t\"what you are doing.\",\n+\t\"\",\n+\t\"To squelch this message, you can set it to 'refuse'.\",\n+};\n+\n static void warn_unconfigured_deny(void)\n {\n \tint i;\n@@ -243,6 +260,12 @@ static void warn_unconfigured_deny(void)\n \t\twarning(warn_unconfigured_deny_msg[i]);\n }\n \n+static void refuse_current_insn(void)\n+{\n+\tint i;\n+\tfor (i = 0; i < ARRAY_SIZE(refuse_current_insn_msg); i++)\n+\t\terror(refuse_current_insn_msg[i]);\n+}\n \n static const char *update(struct command *cmd)\n {\n@@ -268,7 +291,10 @@ static const char *update(struct command *cmd)\n \t\t\t\twarn_unconfigured_deny();\n \t\t\tbreak;\n \t\tcase DENY_REFUSE:\n+\t\tcase DENY_REFUSE_WITH_INSN:\n \t\t\terror(\"refusing to update checked out branch: %s\", name);\n+\t\t\tif (deny_current_branch == DENY_REFUSE_WITH_INSN)\n+\t\t\t\trefuse_current_insn();\n \t\t\treturn \"branch is currently checked out\";\n \t\t}\n \t}\n"},{"id":"102796","messageId":"alpine.DEB.1.00.0902012349360.3586@pacific.mpi-cbg.de","threadId":"17449","inReplyTo":"20090131095622.6117@nanako3.lavabit.com","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-01T22:59:57Z","receivedAt":"2009-02-01T22:59:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 31 Jan 2009, Nanako Shiraishi wrote:\n\n> Quoting Johannes Schindelin <Johannes.Schindelin@gmx.de>:\n> \n> > You cannot just cater for one workflow and fsck the other workflows \n> > over.\n> >\n> > You'll have to devise a method that helps the workflow you are \n> > interested in, but leaves the others alone.\n> \n> I think you'd want to repeat that to yourself when you propose to switch \n> the default for denyCurrentcurrentBranch config to \"true\" too hastily \n> the next time?\n\nNanako, what exactly do you think I did before writing these lines:\n\n    Granted, we wanted to have a longer grace period for old-timers, but\n    let's face it:\n    [... a discussion on the pros and cons ...]\n\n?  Do you think I did that just on a whim, or do you rather assume that I \nthought long and hard about it?\n\n> I don't think your patch matches the tradition of how defaults are \n> changed in git project. You don't introduce a large change just after \n> the maintainer hints about going into a freeze for 1.X.Y release when Y \n> isn't zero.\n\nIndeed.  That is why I wrote \"Granted, we wanted to have a longer grace \nperiod\"!\n\n> I assume that everybody, including the maintainer who is too heavyweight\n\nI saw Junio.  He is in no way heavyweight.  He is actually rather skinny.\n\n> and has too much inertia to accept too sudden a change of the course,\n> wants to eventually make the default to deny pushing to the current\n> branch. But I think such a change should come at 1.7.0 release at the\n> earliest, and a constructive thing to do is to put in a patch to 1.6.2\n> that helps the users with the eventual transition.\n\nSo what do you want to achieve?  Annoy me?  Annoy Git newbies?  Annoy Git \noldtimers?\n\nEventually, it will boil down to\n\n- who\n- when\n\nto annoy.\n\nAnd I have a strong suspicion that it does not help the reputation of Git \nat all, if we annoy\n\n- new Git users\n- for a long time\n\nRather, I'd like to annoy only\n\n- a few oldtimers who should know better by now\n- just once, when they upgrade to a new minor release and see that they \n  forgot to mark their repository as \"bare\".\n\nIf you would think about it as long and hard as I did, you would see that \nwe have to annoy\n\n- a few oldtimers\n- at some stage\n\nanyway, but in the meantime, we could avoid to annoy\n\n- a lot of new Git users\n- for a long time\n\nat the cost of annoying\n\n- a few oldtimers\n- now, instead of later\n\nwhich cost will come to\n\n- us\n- anyway\n\nFrankly, I am surprised that people do not agree with me on this point.\n\n> What do people think?\n\nSeriously, when it comes to the Git users I interact with, they think \n\"what the bl**dy fsck did the Git people smoke when they made it _so_ hard \non new Git users, I am certainly not the only person bitten by \nthis.\"\n\nI know, because they let me in on their thoughts, but are too shy to \nmention them here on the Git list.\n\nAnd as everybody knows, I am a nice guy, and I listen.\n\nCiao,\nDscho\n"},{"id":"102800","messageId":"7vr62hr9s9.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0902012349360.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-01T23:56:38Z","receivedAt":"2009-02-01T23:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> at the cost of annoying\n>\n> - a few oldtimers\n> - now, instead of later\n\nNobody seems to have realized this, but suddenly changing the default to\nrefuse without giving people enough advance warning to adjust will hurt\nnot just the old-timers (a rough definition is people who are from the\nkernel circle and have been using git since summer of 2005), but people\nwho picked up a recipe from various how-to web pages to push to a live\nrepository and updating the checkout that is otherwise never touched by\nthe humans with its post-update hook running \"reset --hard\".  Old timers\nmay be savvy enough to know what has changed and may be able to grudgingly\nreact, but what is your plans for these recipe following kids?\n\nHow many times do I have to repeat that it is much worse to break a\nworking setup of people without advance warning and sound transition\nguidance than having a known breakage that users can be trained to avoid?\n\nAnd realize that I am not saying we need to keep the known breakage\nforever.\n\nThe only thing I am saying is that you need to have a smooth transition\nplan for changing the default, and a mechanism to guide people in place.\n\nI'll ignore you if you keep repeating \"all it takes is for old timers to\nflip a switch\".  Such an argument shows that you didn't learn a thing\nafter the 1.6.0 fallout.\n"},{"id":"102829","messageId":"1233558035.20131.72.camel@maia.lan","threadId":"17449","inReplyTo":"7vbptlsuyv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-02-02T07:00:35Z","receivedAt":"2009-02-02T07:00:35Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On Sun, 2009-02-01 at 13:33 -0800, Junio C Hamano wrote:\n> > I just can't understand the\n> > resistance to this safety feature.  People who encounter the bug can\n> > just change the setting and move on... it seems like an argument based\n> > on \"principles\", usually a sign that one has run out of actual\n> > arguments..\n> \n> There is no resitance to any safety feature.  The resistance is to\n> something entirely different.\n  [...]\n> Perhaps you already forgot the fiasco after 1.6.0, which moved tons of\n> git-foo scripts out of the users' way.  It resulted in a better layout in\n> the end, but we knew it would break people's working setup from the\n> beginning.\n  [...]\n\nYeah sure but the changes are a bit different aren't they.  One affected\nall users who used the previously documented way to access subcommands\n(and the names that the man pages all still retain).  The other affects\na small number of users who are doing something which is labeled in many\nplaces as a bad thing to want to do.\n\nThat being said, I think I like the copy and design of the patch you\njust posted.  If the path of caution is to be followed for this, then\nthe way you propose seems a good way to do it.\n\nSam\n"},{"id":"102842","messageId":"7v1vuhkzmp.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"1233558035.20131.72.camel@maia.lan","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-02T08:32:30Z","receivedAt":"2009-02-02T08:32:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sam Vilain <sam@vilain.net> writes:\n\n> The other affects\n> a small number of users who are doing something which is labeled in many\n> places as a bad thing to want to do.\n\nSorry, but I do not agree with this.\n\nWhat is bad is to push into a repository that is used for editing.  That\nis labelled as a bad thing to want to do.\n\nIt is often the easiest to push and then run \"reset --hard\" (perhaps from\nthe post-update script) to propagate your change to a repository that is\nnot usually used for editing.  E.g. that has always been the way I update\nmy private repository at k.org that I use for final testing before pushing\nout the results that I built and tested on my personal machine.  People\nwho have live web pages served from a checkout do that, too.  It is not a\nbad thing to do at all, and you can find many instructions with google\nwithout spending a lot of time to do exactly that.\n\n    http://kerneltrap.org/mailarchive/git/2008/7/1/2315924\n    http://utsl.gen.nz/git/post-update\n    http://groups.google.com/group/sl-ugr/browse_thread/thread/04e4c4bd6ce174af\n"},{"id":"102850","messageId":"1233571805.20131.358.camel@maia.lan","threadId":"17449","inReplyTo":"7v1vuhkzmp.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2009-02-02T10:50:05Z","receivedAt":"2009-02-02T10:50:05Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On Mon, 2009-02-02 at 00:32 -0800, Junio C Hamano wrote:\n> > The other affects\n> > a small number of users who are doing something which is labeled in many\n> > places as a bad thing to want to do.\n> \n> Sorry, but I do not agree with this.\n> \n> What is bad is to push into a repository that is used for editing.  That\n> is labelled as a bad thing to want to do.\n> \n> It is often the easiest to push and then run \"reset --hard\" (perhaps from\n> the post-update script) to propagate your change to a repository that is\n> not usually used for editing.  E.g. that has always been the way I update\n> my private repository at k.org that I use for final testing before pushing\n> out the results that I built and tested on my personal machine.  People\n> who have live web pages served from a checkout do that, too.  It is not a\n> bad thing to do at all, and you can find many instructions with google\n> without spending a lot of time to do exactly that.\n> \n>     http://kerneltrap.org/mailarchive/git/2008/7/1/2315924\n>     http://utsl.gen.nz/git/post-update\n\nHeh, thanks for referring me to my own script ;-)\n\nI think a \"repository that is used for editing\" can be practically\ndefined as one which does not have any dirty local files.  Or, if there\nare dirty local files then they are none of the files which would be\nchanged by the push, or none of them would be changed by the push.\nSimilar to the check that 'git merge' does.\n\nWith that definition, if receive.denyCurrentBranch is set to \"update\" it\ncould be pretty much automagic, perhaps even good enough behaviour to\nconsider making it the default.  This kind of behaviour is what my\npost-update hook tries to achieve, but it really needs a corresponding\npiece in the update hook, and I didn't code all of the conditions above\ninto it.\n\nSam.\n"},{"id":"102853","messageId":"20090202124148.GB8325@sigio.peff.net","threadId":"17449","inReplyTo":"7vy6wr0wvi.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-02T12:41:48Z","receivedAt":"2009-02-02T12:41:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 31, 2009 at 05:27:45PM -0800, Junio C Hamano wrote:\n\n> I haven't manged to convince myself about the \"git init\" change (I have\n> the code and also I've looked at the extent of damage the change causes to\n\nI think the \"git init\" change doesn't make sense. In fact, I don't think\nsuch a proposal ever really makes sense (and I have even proposed it in\nthe past, but others arguments have changed my way of thinking).\n\nThe reason is that you are just moving the breakage to a different point\nin their workflow. The claim is that it's not OK for this to break:\n\n  cd foo && git init\n  git push ;# ok\n  ... time passes, git upgrade ...\n  git push ;# broken\n\nbut it somehow _is_ OK for this to break:\n\n  cd foo && git init\n  git push ;# ok\n  ... time passes, git upgrade ...\n  cd bar && git init\n  git push ;# broken\n\nIn both cases, you have a sequence of commands that does one thing with\none git version, and something else with another git version. The only\ndifference is whether your sequence includes git init. So while you\ndon't break people with existing repositories, you are still breaking\nanybody who creates a new one and gets confused when there is new\nbehavior (or even has scripts which involve repository creation).\n\nSo in my opinion either the breakage is serious enough not to allow the\nchange, or minor enough (compared to the benefit) to allow it. But\nchanging default config in git init is:\n\n  - a half-way solution that leaves some workflows broken and some not\n\n  - possibly even _worse_, since now we have sacrificed consistency. So\n    now users wonder why some of their repos show breakage and some\n    don't. Or why a particular behavior goes away when they try to write\n    a test case that involves creating a new test repo.\n\nAnd note that it doesn't matter whether you think the right path is to\nmake the change or not: I am only arguing here against this sort of\nhalf-way technique.\n\n> -- >8 --\n> Subject: [PATCH] receive-pack: explain what to do when push updates the current branch\n> \n> This makes \"git push\" issue a more detailed instruction when a user pushes\n> into the current branch of a non-bare repository without having an\n> explicit configuration set to receive.denycurrentbranch.  In such a case,\n> it will also tell the user that the default will change to refusal in a\n> future version of git.\n\nI think this is a definite improvement over the current behavior. As I\nsaid before, I am not sure what is the right path (though I think I am\nleaning towards leaving the warning longer based on the recent\ndiscussion), but if we are to leave the default to warn and not refuse,\nI think this should definitely be applied.\n\nA few comments on the specific message:\n\n>  }\n>  \n> +static char *warn_unconfigured_deny_msg[] = {\n> +\t\"Updating the currently checked out branch may cause confusion,\",\n> +\t\"as the index and work tree do not reflect changes that are in HEAD.\"\n> +\t\"As a result, you may see the changes you just pushed into it\",\n\nMissing comma between lines 2 and 3, which results in an overly long\nline in the output.\n\n> +\t\"You can set 'receive.denyCurrentBranch' configuration variable to\",\n> +\t\"'refuse' in the repository to forbid pushing into the current branch\",\n> +\t\"of it.\"\n\nMaybe this should specifically say \"remote repository\". If you\nunderstand how the feature works, it is obvious that you must do it that\nway, but for less advanced users it is not even clear that this text is\nbeing generated by the remote end.\n\n> +\t\"To allow pushing into the current branch, you can set it to 'ignore';\",\n> +\t\"but this is not recommended unless you really know what you are doing.\"\n\nI thought somebody (you?) argued against the phrase \"unless you really\nknow what you are doing\". And it is better here in context explaining\nthe general issue. But as a user, now I have to ask: do I know what I am\ndoing, and if not, how do I find out?\n\nThe two obvious solutions for people who \"know what they are doing\" are\nrunning \"git reset --hard\", and installing a hook that does something\nsensible. I don't know if it is worth mentioning them here (the former\nis mentioned earlier in the message, but that doesn't necessarily mean\nthe user understands all the implications). Since there are so many\nsubtleties to explain, maybe it make sense to simply put in a pointer to\nan expanded discussion in the \"git push\" manpage?\n\n-Peff\n"},{"id":"102915","messageId":"7vvdrsdtvr.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"20090202124148.GB8325@sigio.peff.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-03T04:30:48Z","receivedAt":"2009-02-03T04:30:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> In both cases, you have a sequence of commands that does one thing with\n> one git version, and something else with another git version. The only\n> difference is whether your sequence includes git init.\n\nI vaguely remember arguing against different behaviour between a new\nrepository and an existing one in the past on a different topic myself.  I\nam not married to the idea of effectively flipping the default to \"refuse\"\nin a new repository early, and do not mind dropping the \"git init\" change\nat all.  I do not like the inconsistency myself.\n\nThe only reason why I did that \"git init\" patch was because I just thought\nthat it might be a good way to help new people sooner, who will start\nusing git after 1.6.2 gets released but before 1.7.0 flips the default for\neverybody, while explaining people older than 1.6.2 what is happening in\nthe warning/error message during the transition period.  I suspect it\ncould be argued that with an extra line that says \"the default will change\nto 'refuse' in 1.7.0 for all repositories, but we are making the change\nearly for newly created repositories to help new people\", the main idea of\nthe patch may be salvageable, but I do not deeply care either way.\n\nBy the way, I just realized one thing.\n\nWhen we flip the default to \"refuse\" in 1.7.0 for everybody, we will need\nthe explanation and instruction on how to get a non-default behaviour and\nhow to squelch the message when we \"refuse by defaut\", just like my first\npatch did when we \"warn by default\".  It is entirely possible some people\nsimply skip 1.6.2 and directly jump to 1.7.0, and while we cannot help\nthem avoid the surprise caused by the change in behaviour, we cannot be\nsilent in such a situation.\n"},{"id":"102936","messageId":"7vskmwc5js.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"20090202124148.GB8325@sigio.peff.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-03T08:01:43Z","receivedAt":"2009-02-03T08:01:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think this is a definite improvement over the current behavior. As I\n> said before, I am not sure what is the right path (though I think I am\n> leaning towards leaving the warning longer based on the recent\n> discussion), but if we are to leave the default to warn and not refuse,\n> I think this should definitely be applied.\n>\n> A few comments on the specific message:\n\nThanks.\n\nThe commit was only in 'pu', so I'll be amending it instead of applying an\nincremental, but here is the interdiff to incorporate your comments.\n\n builtin-receive-pack.c |    9 +++++----\n 1 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git c/builtin-receive-pack.c w/builtin-receive-pack.c\nindex f2c94fc..09c07d9 100644\n--- c/builtin-receive-pack.c\n+++ w/builtin-receive-pack.c\n@@ -217,17 +217,18 @@ static int is_ref_checked_out(const char *ref)\n \n static char *warn_unconfigured_deny_msg[] = {\n \t\"Updating the currently checked out branch may cause confusion,\",\n-\t\"as the index and work tree do not reflect changes that are in HEAD.\"\n+\t\"as the index and work tree do not reflect changes that are in HEAD.\",\n \t\"As a result, you may see the changes you just pushed into it\",\n \t\"reverted when you run 'git diff' over there, and you may want\",\n \t\"to run 'git reset --hard' before starting to work to recover.\",\n \t\"\",\n \t\"You can set 'receive.denyCurrentBranch' configuration variable to\",\n-\t\"'refuse' in the repository to forbid pushing into the current branch\",\n-\t\"of it.\"\n+\t\"'refuse' in the remote repository to forbid pushing into the\",\n+\t\"current branch of it.\"\n \t\"\",\n \t\"To allow pushing into the current branch, you can set it to 'ignore';\",\n-\t\"but this is not recommended unless you really know what you are doing.\",\n+\t\"but this is not recommended unless you arranged its work tree to get\",\n+\t\"updated to match what you pushed in some other way.\",\n \t\"\",\n \t\"To squelch this message, you can set it to 'warn'.\",\n \t\"\",\n"},{"id":"102937","messageId":"20090203080734.GA27251@sigill.intra.peff.net","threadId":"17449","inReplyTo":"7vskmwc5js.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-03T08:07:34Z","receivedAt":"2009-02-03T08:07:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 03, 2009 at 12:01:43AM -0800, Junio C Hamano wrote:\n\n>  \t\"To allow pushing into the current branch, you can set it to 'ignore';\",\n> -\t\"but this is not recommended unless you really know what you are doing.\",\n> +\t\"but this is not recommended unless you arranged its work tree to get\",\n> +\t\"updated to match what you pushed in some other way.\",\n\nThis is much better, but I believe it needs to be \"...arranged _for_\nits work tree to get updated...\" to be grammatically correct.\n\nAnd as a nit (which I seem to be full of tonight), you can get rid of\nthe passive voice by saying:\n\n but this is not recommended unless you arranged to update its work\n tree to match what you pushed in some other way.\n\nwhich is slightly more clear, IMHO.\n\n-Peff\n"},{"id":"102943","messageId":"7v4ozbdgea.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"20090203080734.GA27251@sigill.intra.peff.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-03T09:22:05Z","receivedAt":"2009-02-03T09:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Feb 03, 2009 at 12:01:43AM -0800, Junio C Hamano wrote:\n>\n>>  \t\"To allow pushing into the current branch, you can set it to 'ignore';\",\n>> -\t\"but this is not recommended unless you really know what you are doing.\",\n>> +\t\"but this is not recommended unless you arranged its work tree to get\",\n>> +\t\"updated to match what you pushed in some other way.\",\n>\n> This is much better, but I believe it needs to be \"...arranged _for_\n> its work tree to get updated...\" to be grammatically correct.\n>\n> And as a nit (which I seem to be full of tonight), you can get rid of\n> the passive voice by saying:\n>\n>  but this is not recommended unless you arranged to update its work\n>  tree to match what you pushed in some other way.\n>\n> which is slightly more clear, IMHO.\n\nMuch more clear.  I overuse the passive voice, I know it is a bad habit I\nsomehow cannot shake off.\n\nThanks.\n"},{"id":"102999","messageId":"7vd4dzbei5.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"7vvdrsdtvr.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-03T17:45:54Z","receivedAt":"2009-02-03T17:45:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> By the way, I just realized one thing.\n>\n> When we flip the default to \"refuse\" in 1.7.0 for everybody, we will need\n> the explanation and instruction on how to get a non-default behaviour and\n> how to squelch the message when we \"refuse by defaut\", just like my first\n> patch did when we \"warn by default\".  It is entirely possible some people\n> simply skip 1.6.2 and directly jump to 1.7.0, and while we cannot help\n> them avoid the surprise caused by the change in behaviour, we cannot be\n> silent in such a situation.\n\nAnd this is meant for 1.7.0, and is queued at the tip of 'pu' just for the\nheck of it.\n\nIt will flip the default to \"refuse\", but explains why the operation that\nused to succeed suddenly started failing, how to get the older behaviour\nback (but with caveats), and how to squelch this message.\n\nAdjustments to the tests were resurrected from the previous \"init-db\"\npatch.  Since this is meant for 1.7.0 that does flip the default for\neverybody, there is no need to touch init-db anymore.\n\n-- >8 --\n[PATCH] Refuse updating the current branch in a non-bare repository via push\n\nThis makes git-push to refuse pushing-push into a non-bare repository to\nupdate its current branch by default.  To help people who are used to\nbe able to do this (and later \"reset --hard\" it in some other way),\na big error message is issued when this refusal is triggered.\n\nHosting sites that do not give the users direct access to customize their\nrepositories (e.g. repo.or.cz, gitorious, github etc.) may further want to\nexplicitly set the configuration variable to \"refuse\" for their customers'\nrepositories.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-receive-pack.c      |   40 +++++++++++++++++-----------------------\n t/t5400-send-pack.sh        |    2 ++\n t/t5401-update-hooks.sh     |    1 +\n t/t5405-send-pack-rewind.sh |    1 +\n t/t5516-fetch-push.sh       |    1 +\n t/t5517-push-mirror.sh      |    3 ++-\n t/t5521-pull-symlink.sh     |   20 +++++++++++++-------\n t/t5701-clone-local.sh      |    4 +++-\n 8 files changed, 40 insertions(+), 32 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 6f61c45..17ab4f5 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -215,33 +215,27 @@ static int is_ref_checked_out(const char *ref)\n \treturn !strcmp(head, ref);\n }\n \n-static char *warn_unconfigured_deny_msg[] = {\n-\t\"Updating the currently checked out branch may cause confusion,\",\n-\t\"as the index and work tree do not reflect changes that are in HEAD.\",\n-\t\"As a result, you may see the changes you just pushed into it\",\n-\t\"reverted when you run 'git diff' over there, and you may want\",\n-\t\"to run 'git reset --hard' before starting to work to recover.\",\n+static char *refuse_unconfigured_deny_msg[] = {\n+\t\"By default, updating the current branch in a non-bare repository\",\n+\t\"is denied, because it will make the index and work tree inconsistent\",\n+\t\"with what you pushed, and will require 'git reset --hard' to match\",\n+\t\"the work tree to HEAD.\",\n \t\"\",\n \t\"You can set 'receive.denyCurrentBranch' configuration variable to\",\n-\t\"'refuse' in the remote repository to forbid pushing into its\",\n-\t\"current branch.\"\n+\t\"'ignore' or 'warn' in the remote repository to allow pushing into\",\n+\t\"its current branch; however, this is not recommended unless you\",\n+\t\"arranged to update its work tree to match what you pushed in some\",\n+\t\"other way.\",\n \t\"\",\n-\t\"To allow pushing into the current branch, you can set it to 'ignore';\",\n-\t\"but this is not recommended unless you arranged to update its work\",\n-\t\"tree to match what you pushed in some other way.\",\n-\t\"\",\n-\t\"To squelch this message, you can set it to 'warn'.\",\n-\t\"\",\n-\t\"Note that the default will change in a future version of git\",\n-\t\"to refuse updating the current branch unless you have the\",\n-\t\"configuration variable set to either 'ignore' or 'warn'.\"\n+\t\"To squelch this message, you can set the configuration variable to\",\n+\t\"'refuse'.\"\n };\n \n-static void warn_unconfigured_deny(void)\n+static void refuse_unconfigured_deny(void)\n {\n \tint i;\n-\tfor (i = 0; i < ARRAY_SIZE(warn_unconfigured_deny_msg); i++)\n-\t\twarning(warn_unconfigured_deny_msg[i]);\n+\tfor (i = 0; i < ARRAY_SIZE(refuse_unconfigured_deny_msg); i++)\n+\t\terror(refuse_unconfigured_deny_msg[i]);\n }\n \n static const char *update(struct command *cmd)\n@@ -261,14 +255,14 @@ static const char *update(struct command *cmd)\n \t\tswitch (deny_current_branch) {\n \t\tcase DENY_IGNORE:\n \t\t\tbreak;\n-\t\tcase DENY_UNCONFIGURED:\n \t\tcase DENY_WARN:\n \t\t\twarning(\"updating the current branch\");\n-\t\t\tif (deny_current_branch == DENY_UNCONFIGURED)\n-\t\t\t\twarn_unconfigured_deny();\n \t\t\tbreak;\n \t\tcase DENY_REFUSE:\n+\t\tcase DENY_UNCONFIGURED:\n \t\t\terror(\"refusing to update checked out branch: %s\", name);\n+\t\t\tif (deny_current_branch == DENY_UNCONFIGURED)\n+\t\t\t\trefuse_unconfigured_deny();\n \t\t\treturn \"branch is currently checked out\";\n \t\t}\n \t}\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex b21317d..5c9c277 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -33,6 +33,7 @@ test_expect_success setup '\n \tgit update-ref HEAD \"$commit\" &&\n \tgit clone ./. victim &&\n \tcd victim &&\n+\tgit config receive.denyCurrentBranch warn &&\n \tgit log &&\n \tcd .. &&\n \tgit update-ref HEAD \"$zero\" &&\n@@ -138,6 +139,7 @@ rewound_push_setup() {\n \trm -rf parent child &&\n \tmkdir parent && cd parent &&\n \tgit init && echo one >file && git add file && git commit -m one &&\n+\tgit config receive.denyCurrentBranch warn &&\n \techo two >file && git commit -a -m two &&\n \tcd .. &&\n \tgit clone parent child && cd child && git reset --hard HEAD^\ndiff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh\nindex 64f66c9..325714e 100755\n--- a/t/t5401-update-hooks.sh\n+++ b/t/t5401-update-hooks.sh\n@@ -18,6 +18,7 @@ test_expect_success setup '\n \tgit update-ref refs/heads/master $commit0 &&\n \tgit update-ref refs/heads/tofail $commit1 &&\n \tgit clone ./. victim &&\n+\tGIT_DIR=victim/.git git config receive.denyCurrentBranch warn &&\n \tGIT_DIR=victim/.git git update-ref refs/heads/tofail $commit1 &&\n \tgit update-ref refs/heads/master $commit1 &&\n \tgit update-ref refs/heads/tofail $commit0\ndiff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh\nindex cb9aacc..4bda18a 100755\n--- a/t/t5405-send-pack-rewind.sh\n+++ b/t/t5405-send-pack-rewind.sh\n@@ -8,6 +8,7 @@ test_expect_success setup '\n \n \t>file1 && git add file1 && test_tick &&\n \tgit commit -m Initial &&\n+\tgit config receive.denyCurrentBranch warn &&\n \n \tmkdir another && (\n \t\tcd another &&\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 89649e7..a67ebd0 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -12,6 +12,7 @@ mk_empty () {\n \t(\n \t\tcd testrepo &&\n \t\tgit init &&\n+\t\tgit config receive.denyCurrentBranch warn &&\n \t\tmv .git/hooks .git/hooks-disabled\n \t)\n }\ndiff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh\nindex ea49ded..e2ad260 100755\n--- a/t/t5517-push-mirror.sh\n+++ b/t/t5517-push-mirror.sh\n@@ -19,7 +19,8 @@ mk_repo_pair () {\n \tmkdir mirror &&\n \t(\n \t\tcd mirror &&\n-\t\tgit init\n+\t\tgit init &&\n+\t\tgit config receive.denyCurrentBranch warn\n \t) &&\n \tmkdir master &&\n \t(\ndiff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh\nindex 5672b51..66b5ac1 100755\n--- a/t/t5521-pull-symlink.sh\n+++ b/t/t5521-pull-symlink.sh\n@@ -14,13 +14,19 @@ test_description='pulling from symlinked subdir'\n #\n # The working directory is subdir-link.\n \n-mkdir subdir\n-echo file >subdir/file\n-git add subdir/file\n-git commit -q -m file\n-git clone -q . clone-repo\n-ln -s clone-repo/subdir/ subdir-link\n-\n+test_expect_success setup '\n+\tmkdir subdir &&\n+\techo file >subdir/file &&\n+\tgit add subdir/file &&\n+\tgit commit -q -m file &&\n+\tgit clone -q . clone-repo &&\n+\tln -s clone-repo/subdir/ subdir-link &&\n+\t(\n+\t\tcd clone-repo &&\n+\t\tgit config receive.denyCurrentBranch warn\n+\t) &&\n+\tgit config receive.denyCurrentBranch warn\n+'\n \n # Demonstrate that things work if we just avoid the symlink\n #\ndiff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh\nindex 3559d17..10accc2 100755\n--- a/t/t5701-clone-local.sh\n+++ b/t/t5701-clone-local.sh\n@@ -119,7 +119,9 @@ test_expect_success 'bundle clone with nonexistent HEAD' '\n test_expect_success 'clone empty repository' '\n \tcd \"$D\" &&\n \tmkdir empty &&\n-\t(cd empty && git init) &&\n+\t(cd empty &&\n+\t git init &&\n+\t git config receive.denyCurrentBranch warn) &&\n \tgit clone empty empty-clone &&\n \ttest_tick &&\n \t(cd empty-clone\n-- \n1.6.1.2.346.g115a3\n"},{"id":"103479","messageId":"20090206140637.GB18364@coredump.intra.peff.net","threadId":"17449","inReplyTo":"7vd4dzbei5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-06T14:06:37Z","receivedAt":"2009-02-06T14:06:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 03, 2009 at 09:45:54AM -0800, Junio C Hamano wrote:\n\n> And this is meant for 1.7.0, and is queued at the tip of 'pu' just for the\n> heck of it.\n\nLooks sane to me for 1.7.0, but it will probably require tweaking of\nwhich tests are fixed at that time (and actually, I think some can just\nbe cleaned up to use bare repositories instead of adding the config --\nbut let's deal with that when the patch is ready to be applied).\n\n> This makes git-push to refuse pushing-push into a non-bare repository to\n\nAs opposed to non-pushing push?\n\n-Peff\n"},{"id":"103596","messageId":"7v1vuaznuf.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"20090206140637.GB18364@coredump.intra.peff.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-07T07:51:36Z","receivedAt":"2009-02-07T07:51:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> This makes git-push to refuse pushing-push into a non-bare repository to\n>\n> As opposed to non-pushing push?\n\nEditor/finger slippage.  Thanks for catching it.\n"},{"id":"104127","messageId":"20090211001138.GU21473@genesis.frugalware.org","threadId":"17449","inReplyTo":"alpine.DEB.1.00.0901301426150.3586@pacific.mpi-cbg.de","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2009-02-11T00:11:38Z","receivedAt":"2009-02-11T00:11:38Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"[ Sorry for the late reply, I did not read my mail recently. ]\n\nOn Fri, Jan 30, 2009 at 02:28:39PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> > Shouldn't this be\n> > \n> > git config receive.denyCurrentBranch ignore\n> > \n> > instead of \"true\"?\n> \n> Right.\n> \n> However, as Junio pointed out, we do not want to give this resolution in \n> the error message.  I am now leaning more to something like\n> \n> \trefusing to update checked out branch '%s' in non-bare repository\n> \n> Hmm?\n> \n> Old-timers will know \"oh, what the hell, I did not mark my repository as \n> bare!\", and new-timers will no longer be confused.\n\nSo in an \"I know what I'm doing\" mode, is \"git config core.bare true\" in\na non-bare repo considered as a better workaround than using \"git config\nreceive.denyCurrentBranch ignore\"?\n"},{"id":"104792","messageId":"7vskmliy2c.fsf@gitster.siamese.dyndns.org","threadId":"17449","inReplyTo":"20090211001138.GU21473@genesis.frugalware.org","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-11T01:04:11Z","receivedAt":"2009-02-11T01:04:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> [ Sorry for the late reply, I did not read my mail recently. ]\n>\n> On Fri, Jan 30, 2009 at 02:28:39PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n>> > Shouldn't this be\n>> > \n>> > git config receive.denyCurrentBranch ignore\n>> > \n>> > instead of \"true\"?\n>> \n>> Right.\n>> \n>> However, as Junio pointed out, we do not want to give this resolution in \n>> the error message.  I am now leaning more to something like\n>> \n>> \trefusing to update checked out branch '%s' in non-bare repository\n>> \n>> Hmm?\n>> \n>> Old-timers will know \"oh, what the hell, I did not mark my repository as \n>> bare!\", and new-timers will no longer be confused.\n>\n> So in an \"I know what I'm doing\" mode, is \"git config core.bare true\" in\n> a non-bare repo considered as a better workaround than using \"git config\n> receive.denyCurrentBranch ignore\"?\n\nIf you have to ask, you do not know what you're doing ;-)\n"},{"id":"139425","messageId":"loom.20100413T183858-696@post.gmane.org","threadId":"17449","inReplyTo":"alpine.DEB.2.00.0901291729540.22558@vellum.laroia.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to &quot;refuse&quot;","fromName":"Dave Abrahams","fromEmail":"dave@boostpro.com","sentAt":"2010-04-13T16:42:45Z","receivedAt":"2010-04-13T16:42:45Z","isPatch":true,"sender":{"key":"dave@boostpro.com","avatar":"https://gravatar.com/avatar/df0921f05114687777894565de21c052fb137ba7c303a399528b43d08833f065?d=mp&s=160"},"body":"Asheesh Laroia <asheesh <at> asheesh.org> writes:\n\n\n> Being told how to do it right is even better than being told that you're \n> doing it wrong. \n\nI would prefer if Git would simply \"not refuse,\" but barring that, a more\nverbose error would be helpful.  I knew what I was doing in\nhttp://github.com/jelmer/dulwich/issues/issue/17, but it took me a\nlong time  to even notice git's message.\n"},{"id":"139428","messageId":"7vtyrfutep.fsf@alter.siamese.dyndns.org","threadId":"17449","inReplyTo":"alpine.DEB.2.00.0901291729540.22558@vellum.laroia.net","subject":"Re: [PATCH] Switch receive.denyCurrentBranch to \"refuse\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-13T17:57:50Z","receivedAt":"2010-04-13T17:57:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Asheesh Laroia <asheesh@asheesh.org> writes:\n\n> On Fri, 30 Jan 2009, Johannes Schindelin wrote:\n>\n>> \tcase DENY_REFUSE:\n>> +\t\tif (is_bare_repository() || !is_ref_checked_out(name))\n>> \t\t\tbreak;\n>> +\t\terror(\"refusing to update checked out branch: %s\\n\"\n>> +\t\t\t\"if you know what you are doing, you can allow it by \"\n>> +\t\t\t\"setting\\n\\n\"\n>> +\t\t\t\"\\tgit config receive.denyCurrentBranch true\\n\", name);\n>\n> Being told how to do it right is even better than being told that\n> you're doing it wrong. (-:\n\nOf course you are correct, but there are two _right ways_ that are\ncompletely different, depending on how the repository you are pushing\ninto is meant to be used:\n\n - If you are using it as a shared central repository, a distribution\n   point, or a back-up location, you don't need a working tree, and\n   as you say, the \"checked out branch\" condition will not trigger, if\n   you made it a bare one.\n\n - People do wish a way to keep a repository with a checkout, and that is\n   often the reason why this codepath is triggered.  They want a checkout\n   in the repository (perhaps they are serving the files in them from a\n   webserver).  For them, \"pushing into it\" is not the ultimate goal, but\n   \"having its working tree and keeping it up-to-date\" is.  For that,\n   pushing into a \"reception branch\" and merging that to the checkout from\n   the post-update hook is probably the right way (Cf. [*1*] especially is\n   \"See also ...\").\n\nAlso I do not think it would help users to suggest \"bare repository\"\neven for the first class of users.\n\n - If the user knows what a \"bare\" repository is, the user would realize\n   \"Hmm, I am not allowed to push to the checked out branch?  Wait, this\n   repository does not even need a working tree, so if I make it a bare\n   one, I wouldn't have any checked out branch by definition and I\n   wouldn't have this issue\" without being told.\n\n - If the user does not know what a \"bare\" repository is, the user may not\n   even realize that the target repository does not have to have a working\n   tree.  In such a case, there won't be a mental \"click\" between \"checked\n   out\" and \"bare\" anyway.  The added message to suggest \"bare\" will be\n   another line of unintelligible gitspeak in the message to them.\n\n\n[Reference]\n\n*1* https://git.wiki.kernel.org/index.php/GitFaq#Why_won.27t_I_see_changes_in_the_remote_repo_after_.22git_push.22.3F\n"}]}