{"thread":{"id":"17629","subject":"Deleting the \"current\" branch in remote bare repositories","startedAt":"2009-02-07T15:27:54Z","lastAt":"2009-02-11T18:42:05Z","messageCount":29,"participants":["Jan Krüger","Felipe Contreras","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"103631","messageId":"20090207162754.5fb8b63f@perceptron","threadId":"17629","inReplyTo":null,"subject":"Deleting the \"current\" branch in remote bare repositories","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2009-02-07T15:27:54Z","receivedAt":"2009-02-07T15:27:54Z","isPatch":false,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"Hi everyone,\n\nrecently on IRC we had a case where someone had accidentally deleted\nthe \"current\" branch (i.e. thing pointed to by HEAD) by using \"git push\norigin :master\". This broke the remote HEAD as well as the local\nrefs/remotes/origin/HEAD. Not good. I think we want to make it harder\nto get into this situation.\n\nPersonally, without being aware of any potential counterindications, I\nthink the best solution from a usability point of view would\nbe to have receive-pack reject deletions of what's currently in HEAD.\nThe question is, of course: how do we go about situations where someone\nactually wants to delete the branch HEAD points at?\n\n1. reject deletion and point out a command to change HEAD first (I\ndon't think we've got a command to do this remotely; do we want one?)\n\n2. automatically change HEAD to something else if there's any other\nbranch (eww)\n\n3. accept the deletion but warn the user that she just broke the\nrepository (especially eww because it also breaks the local tracking\nref)\n\nAny smart ideas?\n-Jan\n"},{"id":"103664","messageId":"94a0d4530902071405m33a0804er8030e14bea205898@mail.gmail.com","threadId":"17629","inReplyTo":"20090207162754.5fb8b63f@perceptron","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-02-07T22:05:05Z","receivedAt":"2009-02-07T22:05:05Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, Feb 7, 2009 at 5:27 PM, Jan Krüger <jk@jk.gs> wrote:\n> Hi everyone,\n>\n> recently on IRC we had a case where someone had accidentally deleted\n> the \"current\" branch (i.e. thing pointed to by HEAD) by using \"git push\n> origin :master\". This broke the remote HEAD as well as the local\n> refs/remotes/origin/HEAD. Not good. I think we want to make it harder\n> to get into this situation.\n>\n> Personally, without being aware of any potential counterindications, I\n> think the best solution from a usability point of view would\n> be to have receive-pack reject deletions of what's currently in HEAD.\n> The question is, of course: how do we go about situations where someone\n> actually wants to delete the branch HEAD points at?\n>\n> 1. reject deletion and point out a command to change HEAD first (I\n> don't think we've got a command to do this remotely; do we want one?)\n>\n> 2. automatically change HEAD to something else if there's any other\n> branch (eww)\n>\n> 3. accept the deletion but warn the user that she just broke the\n> repository (especially eww because it also breaks the local tracking\n> ref)\n>\n> Any smart ideas?\n\nThis was brought up before:\nhttp://marc.info/?l=git&m=123254293910829&w=2\n\nBut I don't think it reached any conclusion.\n\n-- \nFelipe Contreras\n"},{"id":"103674","messageId":"20090208011802.2b7b9e74@perceptron","threadId":"17629","inReplyTo":"94a0d4530902071405m33a0804er8030e14bea205898@mail.gmail.com","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Jan Krüger","fromEmail":"jk@jk.gs","sentAt":"2009-02-08T00:18:02Z","receivedAt":"2009-02-08T00:18:02Z","isPatch":false,"sender":{"key":"jk@jk.gs","avatar":"https://avatars.githubusercontent.com/u/1774?v=4"},"body":"On Sun, 8 Feb 2009 00:05:05 +0200, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> This was brought up before:\n> http://marc.info/?l=git&m=123254293910829&w=2\n> \n> But I don't think it reached any conclusion.\n\nOkay, somehow I missed that. To reiterate the things from that\ndiscussion that I think are most reasonable:\n\n1) a local broken symref should generally be ignored unless we actually\n   need the symref.\n\n2) there should be a more convenient (porcelain) way to change a\n   refs/remotes/foo/HEAD symref, e.g. git remote set-default, possibly\n   with an option to re-sync from the remote head (we could even make\n   that an option for git remote update).\n\nRegarding 2): if we managed to add an option to that to change the\nremote HEAD, we could disallow deleting a remote branch that HEAD\npoints to, and refer to this command. I think the problem is that we\nwould have to add symref updating logic for all types of remote\nprotocols.\n\nIf people agree with these ideas I think I'll write up a couple of\npatches to implement these changes. So, any protests?\n\n-Jan\n"},{"id":"103698","messageId":"20090208084453.GA4392@coredump.intra.peff.net","threadId":"17629","inReplyTo":"20090208011802.2b7b9e74@perceptron","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-08T08:44:53Z","receivedAt":"2009-02-08T08:44:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 08, 2009 at 01:18:02AM +0100, Jan Krüger wrote:\n\n> Okay, somehow I missed that. To reiterate the things from that\n> discussion that I think are most reasonable:\n> \n> 1) a local broken symref should generally be ignored unless we actually\n>    need the symref.\n\nI think this is almost as easy as:\n\ndiff --git a/refs.c b/refs.c\nindex 024211d..9601101 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -276,7 +276,6 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!resolve_ref(ref, sha1, 1, &flag)) {\n-\t\t\t\terror(\"%s points nowhere!\", ref);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tlist = add_ref(ref, sha1, flag, list, NULL);\n\nSince this is just called when enumerating all of the loose refs (via\nget_loose_refs(), which is generally called from for_each_ref).\n\nHowever, there is one other complication. rename_ref uses get_loose_refs\nto check whether the destination space is available:\n\n  if (!is_refname_available(newref, oldref, get_loose_refs(), 0))\n        return 1;\n\nso you can get funny behavior through:\n\n  git branch -m foo bar\n\nwhen \"bar\" is a symref pointing to a non-existent ref. Of course, we are\nnot _changing_ that behavior, since we always just ignored that symref.\nBut we are removing the warning message that might clue the user that\nsomething confusing is about to happen.\n\n> 2) there should be a more convenient (porcelain) way to change a\n>    refs/remotes/foo/HEAD symref, e.g. git remote set-default, possibly\n>    with an option to re-sync from the remote head (we could even make\n>    that an option for git remote update).\n\nYes, I think that is a good idea (optionally with a switch to just\nre-grab the information from the remote).\n\n> Regarding 2): if we managed to add an option to that to change the\n> remote HEAD, we could disallow deleting a remote branch that HEAD\n> points to, and refer to this command. I think the problem is that we\n> would have to add symref updating logic for all types of remote\n> protocols.\n\nYes, the protocol support would make this a much bigger patch (and you\nwould have to handle the case where the remote side didn't support it).\nBut bear in mind that deleting the remote HEAD breaks things not just\nfor you, but for other people who are cloning that remote. Maybe we\nshould refuse such updates unless \"-f\" is given (similar to\nnon-fast-forward updates); I haven't looked to see if we even have the\nremote's HEAD information during push, though.\n\n> If people agree with these ideas I think I'll write up a couple of\n> patches to implement these changes. So, any protests?\n\nI say go for it.\n\n-Peff\n"},{"id":"103705","messageId":"7v1vu91d00.fsf@gitster.siamese.dyndns.org","threadId":"17629","inReplyTo":"20090208011802.2b7b9e74@perceptron","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-08T09:42:07Z","receivedAt":"2009-02-08T09:42:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Jan Krüger\" <jk@jk.gs> writes:\n\n> 1) a local broken symref should generally be ignored unless we actually\n>    need the symref.\n>\n> 2) there should be a more convenient (porcelain) way to change a\n>    refs/remotes/foo/HEAD symref, e.g. git remote set-default, possibly\n>    with an option to re-sync from the remote head (we could even make\n>    that an option for git remote update).\n\nIt would be good to sugarcoat symbolic-ref with \"git remote set-something\",\nalthough I am not sure \"default\" is a good term for it (it feels more\nlike \"primary\" to me).\n\n> Regarding 2): if we managed to add an option to that to change the\n> remote HEAD, we could disallow deleting a remote branch that HEAD\n> points to, and refer to this command.\n\nWith (2), you would have the ability to pick the branch you are the most\ninterested in among the branches that particular remote 'foo' offers,\nwhich is what remotes/foo/HEAD is about.  But the latter part of your\nsentence is talking about disallowing the removal of a branch in the\nremote 'foo'.  How would (2) help you if what you want is to delete one\nparticular branch at the remote 'foo'?  I do not think these two are\nrelated at all.\n\nRather, if you reject deletion of the current branch at the remote, you\nwould not get into the situation where your remote/foo/HEAD points at\nnonexisting tracking branch by your own \"deleting the branch by pushing a\nvoid to the remote\", and you would reduce the need for (2).\n\nI said \"reduce\" because you can get into the same situation by other\nmeans.  For example, somebody else can remove the branch your\nremotes/foo/HEAD points at.  Or the repository owner of the remote can say\n\"my HEAD is not 'master' anymore, it is 'next'\" and delete 'master' from\nthere.  In either case, your next \"git remote prune foo\" will get you into\nthat situation, and you would need (2) to recover (or use symbolic-ref).\n\nForbidding the deletion of the current branch at remote and (2) do not\nhave anything to do with each other.\n\nI think forbidding the removal of the current branch falls into the same\ncategory as forbidding the updating of the current branch.  It is what you\nwould want to avoid in many workflows, and receive.denyDeleteCurrent that\ndefaults to refuse may eventually be a good way to do this, but we need a\ntransition plan for that escape hatch.  Most people may not see why they\nwould want to do such a thing, and many people may perceive that in *their\nown* workflow such an operation can only be a mistake, but that is not a\ngood enough reason to suddenly start forbidding something other people\nwere allowed to do so far.\n\nThe \"refer to this command\" you mention can and should be issued when the\nuser actually uses \"remotes/foo\" (or \"remotes/foo/HEAD\"), expecting it to\nresolve to the branch HEAD used to point at but now missing.  The current\nerrors you see in your issue (1) were *meant to* notify you of the\nsituation as soon as it occurs (i.e. it gives pre-emptive warning, even\nbefore you actually refer to \"remotes/foo/HEAD\"), expecting that you know\nwhat to do (namely, repoint HEAD with symbolic-ref, or remove HEAD), so\ntheoretically you could also issue the \"refer to this command\" there as\nwell.\n\nBut I agree the current error messages are a bit too loud, and it would be\nbetter to squelch them and only give the instruction when the user refers\nto \"remotes/foo\" or \"remotes/foo/HEAD\".\n"},{"id":"103731","messageId":"20090208111838.GD14359@coredump.intra.peff.net","threadId":"17629","inReplyTo":"7v1vu91d00.fsf@gitster.siamese.dyndns.org","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-08T11:18:39Z","receivedAt":"2009-02-08T11:18:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 08, 2009 at 01:42:07AM -0800, Junio C Hamano wrote:\n\n> I think forbidding the removal of the current branch falls into the same\n> category as forbidding the updating of the current branch.  It is what you\n> would want to avoid in many workflows, and receive.denyDeleteCurrent that\n> defaults to refuse may eventually be a good way to do this, but we need a\n> transition plan for that escape hatch.  Most people may not see why they\n> would want to do such a thing, and many people may perceive that in *their\n> own* workflow such an operation can only be a mistake, but that is not a\n> good enough reason to suddenly start forbidding something other people\n> were allowed to do so far.\n\nI thought of that, too, but note that receive.denyDeleteCurrent is about\npreventing mistakes in a _non-bare_ repo. But deleting the HEAD branch\nis also annoying in a bare repo (but not _as_ annoying; the real impact\nis that cloners get a \"could not checkout\" message, but you don't have\nthe weird index-and-HEAD don't sync issue that non-bare repos get).\nShould such a safety valve apply to both?\n\n-Peff\n"},{"id":"103768","messageId":"7v8wogzr3v.fsf@gitster.siamese.dyndns.org","threadId":"17629","inReplyTo":"20090208111838.GD14359@coredump.intra.peff.net","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-08T19:05:40Z","receivedAt":"2009-02-08T19:05:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Feb 08, 2009 at 01:42:07AM -0800, Junio C Hamano wrote:\n>\n>> I think forbidding the removal of the current branch falls into the same\n>> category as forbidding the updating of the current branch.  It is what you\n>> would want to avoid in many workflows, and receive.denyDeleteCurrent that\n>> defaults to refuse may eventually be a good way to do this, but we need a\n>> transition plan for that escape hatch.  Most people may not see why they\n>> would want to do such a thing, and many people may perceive that in *their\n>> own* workflow such an operation can only be a mistake, but that is not a\n>> good enough reason to suddenly start forbidding something other people\n>> were allowed to do so far.\n>\n> I thought of that, too, but note that receive.denyDeleteCurrent is about\n> preventing mistakes in a _non-bare_ repo. But deleting the HEAD branch\n> is also annoying in a bare repo (but not _as_ annoying; the real impact\n> is that cloners get a \"could not checkout\" message, but you don't have\n> the weird index-and-HEAD don't sync issue that non-bare repos get).\n> Should such a safety valve apply to both?\n\nIf you remove the current branch from a repository, the impact that\noperation causes the remote users of the repository would be the same\nwhether the repository has or does not have a work tree.  And in that\nsense, I think it should apply equally.  The criteria is not \"is it bare\",\nbut \"is it used by people remotely\".  IOW, you refuse deletion of the\ncurrent branch if other people may fetch from it.\n\nIn addition, removing the current branch will affect local operations in\nthe repository --- the person who were working in the work tree to prepare\nfor the _next_ commit will now end up creating a root commit.  The effect\nis similar, as you correctly point out, to the issue of the HEAD and the\nwork tree going out of sync that ends up creating a commit with a lot of\nreverts.  They both cause the user on the work tree to create an undesired\ncommit.  Here, the criteria is \"does this have a work tree\".  I think the\ncurrent receive.denyCurrentBranch should also trigger in this case.\n"},{"id":"103820","messageId":"1234170565-6740-1-git-send-email-gitster@pobox.com","threadId":"17629","inReplyTo":"7v8wogzr3v.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/6] Deleting the \"current\" branch in a remote repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T09:09:19Z","receivedAt":"2009-02-09T09:09:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The first two are preparatory clean-up and bugfix patches.\n\nThe third one introduces receive.denyDeleteCurrent configuration that\ndefaults to \"annoyingly loud warning\", which we will flip to \"refusal with\ninsn\" at the end.\n\nThe fourth and fifth one are not about push and are more or less\nindependent.  They deal with what happens when you ended up with a\ndangling symbolic ref in a tracking hierarchy.  I think a check\nand warning similar to the fourth one may be needed in git-push (and\ngit-send-pack) when it pushes a void to remove a branch from a remote, and\nin turn removes the corresponding tracking branch at the local end.\n\nThen finally the last one flips the default for receive.denyDeleteCurrent\nto refuse.\n\nI think the first five ought to be in 1.6.2-rc1 but I lack the energy and\ntime to finish the testing, re-eyeballing and documentation tonight.  It\nwould be very nice to see friends from other timezones to help me with\nthese tasks ;-)\n\n\nJunio C Hamano (6):\n  builtin-receive-pack.c: do not initialize statics to 0\n  t5400: allow individual tests to fail\n  receive-pack: receive.denyDeleteCurrent\n  remote prune: warn dangling symrefs\n  Warn use of \"origin\" when remotes/origin/HEAD is dangling\n  receive-pack: default receive.denyDeleteCurrent to refuse\n\n builtin-receive-pack.c |   70 ++++++++++++++++++++++++++++++++++++++--------\n builtin-remote.c       |    6 ++++\n refs.c                 |   72 ++++++++++++++++++++++++++++++++++++------------\n refs.h                 |    5 +++\n sha1_name.c            |    6 ++-\n t/t5400-send-pack.sh   |   44 ++++++++++++++++++++--------\n 6 files changed, 157 insertions(+), 46 deletions(-)\n"},{"id":"103821","messageId":"1234170565-6740-2-git-send-email-gitster@pobox.com","threadId":"17629","inReplyTo":"1234170565-6740-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/6] builtin-receive-pack.c: do not initialize statics to 0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T09:09:20Z","receivedAt":"2009-02-09T09:09:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-receive-pack.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex 6de186c..b2cb4ba 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -18,8 +18,8 @@ enum deny_action {\n \tDENY_REFUSE,\n };\n \n-static int deny_deletes = 0;\n-static int deny_non_fast_forwards = 0;\n+static int deny_deletes;\n+static int deny_non_fast_forwards;\n static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n static int receive_fsck_objects;\n static int receive_unpack_limit = -1;\n-- \n1.6.2.rc0.28.g2593d\n"},{"id":"103822","messageId":"1234170565-6740-3-git-send-email-gitster@pobox.com","threadId":"17629","inReplyTo":"1234170565-6740-2-git-send-email-gitster@pobox.com","subject":"[PATCH 2/6] t5400: allow individual tests to fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T09:09:21Z","receivedAt":"2009-02-09T09:09:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Each test chdir'ed around and ended up in a random place if any of the\ntest in the sequence failed but the entire test script was allowed to\nrun.  This wrapps each in a subshell as necessary.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5400-send-pack.sh |   37 ++++++++++++++++++++++++-------------\n 1 files changed, 24 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex b21317d..013aced 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -10,6 +10,7 @@ test_description='See why rewinding head breaks send-pack\n \n cnt=64\n test_expect_success setup '\n+    (\n \ttest_tick &&\n \tmkdir mozart mozart/is &&\n \techo \"Commit #0\" >mozart/is/pink &&\n@@ -51,7 +52,9 @@ test_expect_success setup '\n \tdone &&\n \tgit update-ref HEAD \"$commit\" &&\n \techo Rebase &&\n-\tgit log'\n+\tgit log\n+    )\n+'\n \n test_expect_success 'pack the source repository' '\n \tgit repack -a -d &&\n@@ -59,10 +62,12 @@ test_expect_success 'pack the source repository' '\n '\n \n test_expect_success 'pack the destination repository' '\n+    (\n \tcd victim &&\n \tgit repack -a -d &&\n \tgit prune &&\n \tcd ..\n+    )\n '\n \n test_expect_success \\\n@@ -89,49 +94,53 @@ test_expect_success \\\n \tcmp victim/.git/refs/heads/master .git/refs/heads/master\n '\n \n-test_expect_success \\\n-        'push can be used to delete a ref' '\n+test_expect_success 'push can be used to delete a ref' '\n+    (\n \tcd victim &&\n \tgit branch extra master &&\n \tcd .. &&\n \ttest -f victim/.git/refs/heads/extra &&\n \tgit send-pack ./victim/.git/ :extra master &&\n \t! test -f victim/.git/refs/heads/extra\n+    )\n '\n \n unset GIT_CONFIG\n HOME=`pwd`/no-such-directory\n export HOME ;# this way we force the victim/.git/config to be used.\n \n-test_expect_success \\\n-\t'pushing a delete should be denied with denyDeletes' '\n+test_expect_success 'pushing a delete should be denied with denyDeletes' '\n+    (\n \tcd victim &&\n \tgit config receive.denyDeletes true &&\n \tgit branch extra master &&\n \tcd .. &&\n \ttest -f victim/.git/refs/heads/extra &&\n \ttest_must_fail git send-pack ./victim/.git/ :extra master\n+    )\n '\n rm -f victim/.git/refs/heads/extra\n \n-test_expect_success \\\n-        'pushing with --force should be denied with denyNonFastforwards' '\n+test_expect_success 'pushing with --force should be denied with denyNonFastforwards' '\n+    (\n \tcd victim &&\n \tgit config receive.denyNonFastforwards true &&\n \tcd .. &&\n \tgit update-ref refs/heads/master master^ || return 1\n \tgit send-pack --force ./victim/.git/ master && return 1\n \t! test_cmp .git/refs/heads/master victim/.git/refs/heads/master\n+    )\n '\n \n-test_expect_success \\\n-\t'pushing does not include non-head refs' '\n+test_expect_success 'pushing does not include non-head refs' '\n+    (\n \tmkdir parent && cd parent &&\n \tgit init && touch file && git add file && git commit -m add &&\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+    )\n '\n \n rewound_push_setup() {\n@@ -156,8 +165,8 @@ rewound_push_failed() {\n \tfi\n }\n \n-test_expect_success \\\n-\t'pushing explicit refspecs respects forcing' '\n+test_expect_success 'pushing explicit refspecs respects forcing' '\n+    (\n \trewound_push_setup &&\n \tif git send-pack ../parent/.git refs/heads/master:refs/heads/master\n \tthen\n@@ -167,10 +176,11 @@ test_expect_success \\\n \tfi && rewound_push_failed &&\n \tgit send-pack ../parent/.git +refs/heads/master:refs/heads/master &&\n \trewound_push_succeeded\n+    )\n '\n \n-test_expect_success \\\n-\t'pushing wildcard refspecs respects forcing' '\n+test_expect_success 'pushing wildcard refspecs respects forcing' '\n+    (\n \trewound_push_setup &&\n \tif git send-pack ../parent/.git refs/heads/*:refs/heads/*\n \tthen\n@@ -180,6 +190,7 @@ test_expect_success \\\n \tfi && rewound_push_failed &&\n \tgit send-pack ../parent/.git +refs/heads/*:refs/heads/* &&\n \trewound_push_succeeded\n+    )\n '\n \n test_done\n-- \n1.6.2.rc0.28.g2593d\n"},{"id":"103824","messageId":"1234170565-6740-4-git-send-email-gitster@pobox.com","threadId":"17629","inReplyTo":"1234170565-6740-3-git-send-email-gitster@pobox.com","subject":"[PATCH 3/6] receive-pack: receive.denyDeleteCurrent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T09:09:22Z","receivedAt":"2009-02-09T09:09:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is a companion patch to the recent 3d95d92 (receive-pack: explain\nwhat to do when push updates the current branch, 2009-01-31).\n\nDeleting the current branch from a remote will result in the next clone\nfrom it not check out anything, among other things.  It also is one of the\ncause that makes remotes/origin/HEAD a dangling symbolic ref.  This patch\nstill allows the traditional behaviour but with a big warning, and promises\nthat the default will change to 'refuse' in a future release.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-receive-pack.c |   72 ++++++++++++++++++++++++++++++++++++++++-------\n t/t5400-send-pack.sh   |    7 ++++\n 2 files changed, 68 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex b2cb4ba..f7e04c4 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -21,11 +21,13 @@ enum deny_action {\n static int deny_deletes;\n static int deny_non_fast_forwards;\n static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n+static enum deny_action deny_delete_current = DENY_UNCONFIGURED;\n static int receive_fsck_objects;\n static int receive_unpack_limit = -1;\n static int transfer_unpack_limit = -1;\n static int unpack_limit = 100;\n static int report_status;\n+static const char *head_name;\n \n static char capabilities[] = \" report-status delete-refs \";\n static int capabilities_sent;\n@@ -77,6 +79,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (strcmp(var, \"receive.denydeletecurrent\") == 0) {\n+\t\tdeny_delete_current = parse_deny_action(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -203,16 +210,12 @@ static int run_update_hook(struct command *cmd)\n \n static int is_ref_checked_out(const char *ref)\n {\n-\tunsigned char sha1[20];\n-\tconst char *head;\n-\n \tif (is_bare_repository())\n \t\treturn 0;\n \n-\thead = resolve_ref(\"HEAD\", sha1, 0, NULL);\n-\tif (!head)\n+\tif (!head_name)\n \t\treturn 0;\n-\treturn !strcmp(head, ref);\n+\treturn !strcmp(head_name, ref);\n }\n \n static char *warn_unconfigured_deny_msg[] = {\n@@ -244,6 +247,32 @@ static void warn_unconfigured_deny(void)\n \t\twarning(warn_unconfigured_deny_msg[i]);\n }\n \n+static char *warn_unconfigured_deny_delete_current_msg[] = {\n+\t\"Deleting the current branch can cause confusion by making the next\",\n+\t\"'git clone' not check out any file.\",\n+\t\"\",\n+\t\"You can set 'receive.denyDeleteCurrent' configuration variable to\",\n+\t\"'refuse' in the remote repository to disallow deleting the current\",\n+\t\"branch.\",\n+\t\"\",\n+\t\"You can set it to 'ignore' to allow such a delete without a warning.\",\n+\t\"\",\n+\t\"To make this warning message less loud, 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 deleting the current branch unless you have the\",\n+\t\"configuration variable set to either 'ignore' or 'warn'.\"\n+};\n+\n+static void warn_unconfigured_deny_delete_current(void)\n+{\n+\tint i;\n+\tfor (i = 0;\n+\t     i < ARRAY_SIZE(warn_unconfigured_deny_delete_current_msg);\n+\t     i++)\n+\t\twarning(warn_unconfigured_deny_delete_current_msg[i]);\n+}\n+\n static const char *update(struct command *cmd)\n {\n \tconst char *name = cmd->ref_name;\n@@ -278,12 +307,30 @@ static const char *update(struct command *cmd)\n \t\t      \"but I can't find it!\", sha1_to_hex(new_sha1));\n \t\treturn \"bad pack\";\n \t}\n-\tif (deny_deletes && is_null_sha1(new_sha1) &&\n-\t    !is_null_sha1(old_sha1) &&\n-\t    !prefixcmp(name, \"refs/heads/\")) {\n-\t\terror(\"denying ref deletion for %s\", name);\n-\t\treturn \"deletion prohibited\";\n+\n+\tif (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {\n+\t\tif (deny_deletes && !prefixcmp(name, \"refs/heads/\")) {\n+\t\t\terror(\"denying ref deletion for %s\", name);\n+\t\t\treturn \"deletion prohibited\";\n+\t\t}\n+\n+\t\tif (!strcmp(name, head_name)) {\n+\t\t\tswitch (deny_delete_current) {\n+\t\t\tcase DENY_IGNORE:\n+\t\t\t\tbreak;\n+\t\t\tcase DENY_WARN:\n+\t\t\tcase DENY_UNCONFIGURED:\n+\t\t\t\tif (deny_delete_current == DENY_UNCONFIGURED)\n+\t\t\t\t\twarn_unconfigured_deny_delete_current();\n+\t\t\t\twarning(\"deleting the current branch\");\n+\t\t\t\tbreak;\n+\t\t\tcase DENY_REFUSE:\n+\t\t\t\terror(\"refusing to delete the current branch: %s\", name);\n+\t\t\t\treturn \"deletion of the current branch prohibited\";\n+\t\t\t}\n+\t\t}\n \t}\n+\n \tif (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&\n \t    !is_null_sha1(old_sha1) &&\n \t    !prefixcmp(name, \"refs/heads/\")) {\n@@ -377,6 +424,7 @@ static void run_update_post_hook(struct command *cmd)\n static void execute_commands(const char *unpacker_error)\n {\n \tstruct command *cmd = commands;\n+\tunsigned char sha1[20];\n \n \tif (unpacker_error) {\n \t\twhile (cmd) {\n@@ -394,6 +442,8 @@ static void execute_commands(const char *unpacker_error)\n \t\treturn;\n \t}\n \n+\thead_name = resolve_ref(\"HEAD\", sha1, 0, NULL);\n+\n \twhile (cmd) {\n \t\tcmd->error_string = update(cmd);\n \t\tcmd = cmd->next;\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 013aced..7b21f5f 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -193,4 +193,11 @@ test_expect_success 'pushing wildcard refspecs respects forcing' '\n     )\n '\n \n+test_expect_success 'deny pushing to delete current branch' '\n+    (\n+\trewound_push_setup &&\n+\tgit send-pack ../parent/.git :refs/heads/master 2>errs\n+    )\n+'\n+\n test_done\n-- \n1.6.2.rc0.28.g2593d\n"},{"id":"103823","messageId":"1234170565-6740-5-git-send-email-gitster@pobox.com","threadId":"17629","inReplyTo":"1234170565-6740-4-git-send-email-gitster@pobox.com","subject":"[PATCH 4/6] remote prune: warn dangling symrefs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T09:09:23Z","receivedAt":"2009-02-09T09:09:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If you prune from the remote \"frotz\" that deleted the ref your tracking\nbranch remotes/frotz/HEAD points at, the symbolic ref will become\ndangling.  We used to detect this as an error condition and issued a\nmessage every time refs are enumerated.\n\nThis stops the error message, but moves the warning to \"remote prune\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-remote.c |    6 ++++\n refs.c           |   72 ++++++++++++++++++++++++++++++++++++++++-------------\n refs.h           |    5 +++\n 3 files changed, 65 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex db18bcf..ac69d37 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -756,12 +756,17 @@ static int prune(int argc, const char **argv)\n \t\tOPT_END()\n \t};\n \tstruct ref_states states;\n+\tconst char *dangling_msg;\n \n \targc = parse_options(argc, argv, options, builtin_remote_usage, 0);\n \n \tif (argc < 1)\n \t\tusage_with_options(builtin_remote_usage, options);\n \n+\tdangling_msg = (dry_run\n+\t\t\t? \" %s will become dangling!\\n\"\n+\t\t\t: \" %s has become dangling!\\n\");\n+\n \tmemset(&states, 0, sizeof(states));\n \tfor (; argc; argc--, argv++) {\n \t\tint i;\n@@ -784,6 +789,7 @@ static int prune(int argc, const char **argv)\n \n \t\t\tprintf(\" * [%s] %s\\n\", dry_run ? \"would prune\" : \"pruned\",\n \t\t\t       abbrev_ref(refname, \"refs/remotes/\"));\n+\t\t\twarn_dangling_symref(dangling_msg, refname);\n \t\t}\n \n \t\t/* NEEDSWORK: free remote */\ndiff --git a/refs.c b/refs.c\nindex 024211d..6eb5f53 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -275,10 +275,8 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)\n \t\t\t\tlist = get_ref_dir(ref, list);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (!resolve_ref(ref, sha1, 1, &flag)) {\n-\t\t\t\terror(\"%s points nowhere!\", ref);\n-\t\t\t\tcontinue;\n-\t\t\t}\n+\t\t\tif (!resolve_ref(ref, sha1, 1, &flag))\n+\t\t\t\thashclr(sha1);\n \t\t\tlist = add_ref(ref, sha1, flag, list, NULL);\n \t\t}\n \t\tfree(ref);\n@@ -287,6 +285,35 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)\n \treturn sort_ref_list(list);\n }\n \n+struct warn_if_dangling_data {\n+\tconst char *refname;\n+\tconst char *msg_fmt;\n+};\n+\n+static int warn_if_dangling_symref(const char *refname, const unsigned char *sha1,\n+\t\t\t\t   int flags, void *cb_data)\n+{\n+\tstruct warn_if_dangling_data *d = cb_data;\n+\tconst char *resolves_to;\n+\tunsigned char junk[20];\n+\n+\tif (!(flags & REF_ISSYMREF))\n+\t\treturn 0;\n+\n+\tresolves_to = resolve_ref(refname, junk, 0, NULL);\n+\tif (!resolves_to || strcmp(resolves_to, d->refname))\n+\t\treturn 0;\n+\n+\tprintf(d->msg_fmt, refname);\n+\treturn 0;\n+}\n+\n+void warn_dangling_symref(const char *msg_fmt, const char *refname)\n+{\n+\tstruct warn_if_dangling_data data = { refname, msg_fmt };\n+\tfor_each_rawref(warn_if_dangling_symref, &data);\n+}\n+\n static struct ref_list *get_loose_refs(void)\n {\n \tif (!cached_refs.did_loose) {\n@@ -498,16 +525,19 @@ int read_ref(const char *ref, unsigned char *sha1)\n \treturn -1;\n }\n \n+#define DO_FOR_EACH_INCLUDE_BROKEN 01\n static int do_one_ref(const char *base, each_ref_fn fn, int trim,\n-\t\t      void *cb_data, struct ref_list *entry)\n+\t\t      int flags, void *cb_data, struct ref_list *entry)\n {\n \tif (strncmp(base, entry->name, trim))\n \t\treturn 0;\n-\tif (is_null_sha1(entry->sha1))\n-\t\treturn 0;\n-\tif (!has_sha1_file(entry->sha1)) {\n-\t\terror(\"%s does not point to a valid object!\", entry->name);\n-\t\treturn 0;\n+\tif (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) {\n+\t\tif (is_null_sha1(entry->sha1))\n+\t\t\treturn 0;\n+\t\tif (!has_sha1_file(entry->sha1)) {\n+\t\t\terror(\"%s does not point to a valid object!\", entry->name);\n+\t\t\treturn 0;\n+\t\t}\n \t}\n \tcurrent_ref = entry;\n \treturn fn(entry->name + trim, entry->sha1, entry->flag, cb_data);\n@@ -561,7 +591,7 @@ fallback:\n }\n \n static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,\n-\t\t\t   void *cb_data)\n+\t\t\t   int flags, void *cb_data)\n {\n \tint retval = 0;\n \tstruct ref_list *packed = get_packed_refs();\n@@ -570,7 +600,7 @@ static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,\n \tstruct ref_list *extra;\n \n \tfor (extra = extra_refs; extra; extra = extra->next)\n-\t\tretval = do_one_ref(base, fn, trim, cb_data, extra);\n+\t\tretval = do_one_ref(base, fn, trim, flags, cb_data, extra);\n \n \twhile (packed && loose) {\n \t\tstruct ref_list *entry;\n@@ -586,13 +616,13 @@ static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,\n \t\t\tentry = packed;\n \t\t\tpacked = packed->next;\n \t\t}\n-\t\tretval = do_one_ref(base, fn, trim, cb_data, entry);\n+\t\tretval = do_one_ref(base, fn, trim, flags, cb_data, entry);\n \t\tif (retval)\n \t\t\tgoto end_each;\n \t}\n \n \tfor (packed = packed ? packed : loose; packed; packed = packed->next) {\n-\t\tretval = do_one_ref(base, fn, trim, cb_data, packed);\n+\t\tretval = do_one_ref(base, fn, trim, flags, cb_data, packed);\n \t\tif (retval)\n \t\t\tgoto end_each;\n \t}\n@@ -614,22 +644,28 @@ int head_ref(each_ref_fn fn, void *cb_data)\n \n int for_each_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/\", fn, 0, cb_data);\n+\treturn do_for_each_ref(\"refs/\", fn, 0, 0, cb_data);\n }\n \n int for_each_tag_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/tags/\", fn, 10, cb_data);\n+\treturn do_for_each_ref(\"refs/tags/\", fn, 10, 0, cb_data);\n }\n \n int for_each_branch_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/heads/\", fn, 11, cb_data);\n+\treturn do_for_each_ref(\"refs/heads/\", fn, 11, 0, cb_data);\n }\n \n int for_each_remote_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/remotes/\", fn, 13, cb_data);\n+\treturn do_for_each_ref(\"refs/remotes/\", fn, 13, 0, cb_data);\n+}\n+\n+int for_each_rawref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(\"refs/\", fn, 0,\n+\t\t\t       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n }\n \n /*\ndiff --git a/refs.h b/refs.h\nindex 3bb529d..29bdcec 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -24,6 +24,11 @@ extern int for_each_tag_ref(each_ref_fn, void *);\n extern int for_each_branch_ref(each_ref_fn, void *);\n extern int for_each_remote_ref(each_ref_fn, void *);\n \n+/* can be used to learn about broken ref and symref */\n+extern int for_each_rawref(each_ref_fn, void *);\n+\n+extern void warn_dangling_symref(const char *msg_fmt, const char *refname);\n+\n /*\n  * Extra refs will be listed by for_each_ref() before any actual refs\n  * for the duration of this process or until clear_extra_refs() is\n-- \n1.6.2.rc0.28.g2593d\n"},{"id":"103825","messageId":"1234170565-6740-6-git-send-email-gitster@pobox.com","threadId":"17629","inReplyTo":"1234170565-6740-5-git-send-email-gitster@pobox.com","subject":"[PATCH 5/6] Warn use of \"origin\" when remotes/origin/HEAD is dangling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T09:09:24Z","receivedAt":"2009-02-09T09:09:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The previous one squelched the diagnositic message we used to issue every\ntime we enumerated the refs and noticed a dangling ref.  This adds the\nwarning back to the place where the user actually attempts to use it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 5d0ac02..3bd2ef0 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -268,16 +268,18 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)\n \t\tchar fullref[PATH_MAX];\n \t\tunsigned char sha1_from_ref[20];\n \t\tunsigned char *this_result;\n+\t\tint flag;\n \n \t\tthis_result = refs_found ? sha1_from_ref : sha1;\n \t\tmksnpath(fullref, sizeof(fullref), *p, len, str);\n-\t\tr = resolve_ref(fullref, this_result, 1, NULL);\n+\t\tr = resolve_ref(fullref, this_result, 1, &flag);\n \t\tif (r) {\n \t\t\tif (!refs_found++)\n \t\t\t\t*ref = xstrdup(r);\n \t\t\tif (!warn_ambiguous_refs)\n \t\t\t\tbreak;\n-\t\t}\n+\t\t} else if (flag & REF_ISSYMREF)\n+\t\t\twarning(\"ignoring dangling symref %s.\", fullref);\n \t}\n \tfree(last_branch);\n \treturn refs_found;\n-- \n1.6.2.rc0.28.g2593d\n"},{"id":"103826","messageId":"1234170565-6740-7-git-send-email-gitster@pobox.com","threadId":"17629","inReplyTo":"1234170565-6740-6-git-send-email-gitster@pobox.com","subject":"[PATCH 6/6] receive-pack: default receive.denyDeleteCurrent to refuse","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T09:09:25Z","receivedAt":"2009-02-09T09:09:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is for a future release that switches the default to refuse.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-receive-pack.c |   30 ++++++++++++------------------\n t/t5400-send-pack.sh   |    2 +-\n 2 files changed, 13 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin-receive-pack.c b/builtin-receive-pack.c\nindex f7e04c4..44163ac 100644\n--- a/builtin-receive-pack.c\n+++ b/builtin-receive-pack.c\n@@ -247,30 +247,24 @@ static void warn_unconfigured_deny(void)\n \t\twarning(warn_unconfigured_deny_msg[i]);\n }\n \n-static char *warn_unconfigured_deny_delete_current_msg[] = {\n-\t\"Deleting the current branch can cause confusion by making the next\",\n-\t\"'git clone' not check out any file.\",\n+static char *refuse_unconfigured_deny_delete_current_msg[] = {\n+\t\"By default, deleting the current branch is denied, because the next\",\n+\t\"'git clone' won't result in any file checked out, causing confusion.\",\n \t\"\",\n \t\"You can set 'receive.denyDeleteCurrent' configuration variable to\",\n-\t\"'refuse' in the remote repository to disallow deleting the current\",\n-\t\"branch.\",\n+\t\"'warn' or 'ignore' in the remote repository to allow deleting the\",\n+\t\"current branch, with or without a warning message.\",\n \t\"\",\n-\t\"You can set it to 'ignore' to allow such a delete without a warning.\",\n-\t\"\",\n-\t\"To make this warning message less loud, 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 deleting 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 it to 'refuse'.\"\n };\n \n-static void warn_unconfigured_deny_delete_current(void)\n+static void refuse_unconfigured_deny_delete_current(void)\n {\n \tint i;\n \tfor (i = 0;\n-\t     i < ARRAY_SIZE(warn_unconfigured_deny_delete_current_msg);\n+\t     i < ARRAY_SIZE(refuse_unconfigured_deny_delete_current_msg);\n \t     i++)\n-\t\twarning(warn_unconfigured_deny_delete_current_msg[i]);\n+\t\terror(refuse_unconfigured_deny_delete_current_msg[i]);\n }\n \n static const char *update(struct command *cmd)\n@@ -319,12 +313,12 @@ static const char *update(struct command *cmd)\n \t\t\tcase DENY_IGNORE:\n \t\t\t\tbreak;\n \t\t\tcase DENY_WARN:\n-\t\t\tcase DENY_UNCONFIGURED:\n-\t\t\t\tif (deny_delete_current == DENY_UNCONFIGURED)\n-\t\t\t\t\twarn_unconfigured_deny_delete_current();\n \t\t\t\twarning(\"deleting the current branch\");\n \t\t\t\tbreak;\n \t\t\tcase DENY_REFUSE:\n+\t\t\tcase DENY_UNCONFIGURED:\n+\t\t\t\tif (deny_delete_current == DENY_UNCONFIGURED)\n+\t\t\t\t\trefuse_unconfigured_deny_delete_current();\n \t\t\t\terror(\"refusing to delete the current branch: %s\", name);\n \t\t\t\treturn \"deletion of the current branch prohibited\";\n \t\t\t}\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 7b21f5f..6ef1f03 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -196,7 +196,7 @@ test_expect_success 'pushing wildcard refspecs respects forcing' '\n test_expect_success 'deny pushing to delete current branch' '\n     (\n \trewound_push_setup &&\n-\tgit send-pack ../parent/.git :refs/heads/master 2>errs\n+\ttest_must_fail git send-pack ../parent/.git :refs/heads/master 2>errs\n     )\n '\n \n-- \n1.6.2.rc0.28.g2593d\n"},{"id":"103866","messageId":"20090209182843.GA27109@coredump.intra.peff.net","threadId":"17629","inReplyTo":"7v8wogzr3v.fsf@gitster.siamese.dyndns.org","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-09T18:28:43Z","receivedAt":"2009-02-09T18:28:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 08, 2009 at 11:05:40AM -0800, Junio C Hamano wrote:\n\n> If you remove the current branch from a repository, the impact that\n> operation causes the remote users of the repository would be the same\n> whether the repository has or does not have a work tree.  And in that\n> sense, I think it should apply equally.  The criteria is not \"is it bare\",\n> but \"is it used by people remotely\".  IOW, you refuse deletion of the\n> current branch if other people may fetch from it.\n\nHere are two reasons why we might not want to conflate the two safety\nvalves:\n\n 1. I'm concerned about it becoming more confusing for end users. Right\n    now we don't even bother to look at denyCurrentBranch if we are in a\n    bare repo. That is, it is easily explained as \"this prevents a\n    dangerous operation pushing into a non-bare repo, and if you have a\n    bare repo, you don't have to care about or set this at all.\" But\n    having it cover HEAD deletion, too, means that it is now \"prevent\n    some dangerous operations; some things are dangerous if your repo is\n    non-bare, but safe otherwise.  But some of the operations are\n    dangerous all the time\".\n\n    But maybe that is not really a concern. Most users should leave it\n    set to the default unless they are clueful enough to know which\n    operations are safe and which are not.\n\n 2. It may not be fine-grained enough. If I am a clueful user with a\n    post-receive hook, then I may want to set receive.denyCurrentBranch\n    to \"ignore\". But that doesn't necessarily mean my hook gracefully\n    handles _deletion_ of the branch. I may want to keep that set to\n    \"refuse\", since it makes no sense for my workflow.\n\n    Such a user is better served by a separate receive.denyDeleteHEAD\n    option.\n\n-Peff\n"},{"id":"103867","messageId":"20090209183623.GA27255@coredump.intra.peff.net","threadId":"17629","inReplyTo":"20090209182843.GA27109@coredump.intra.peff.net","subject":"Re: Deleting the \"current\" branch in remote bare repositories","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-09T18:36:23Z","receivedAt":"2009-02-09T18:36:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 09, 2009 at 01:28:43PM -0500, Jeff King wrote:\n\n> Here are two reasons why we might not want to conflate the two safety\n> valves:\n\nArgh, sorry, I'm an idiot. For some reason when I saw\n\"receive.denyDeleteCurrent\" I read \"receive.denyCurrentBranch\" (and if\nyou look carefully in the thread, I even _typed_ it wrong, too!).\n\nSo I read your emails as \"we should put this new safety valve under the\nsame option as the other one\" which you are of course not proposing at\nall.\n\nPlease disregard this last email (and the one before it, but you have\nalready responded to that ;) ). And I will ask a doctor to remove the brain\ntumor that is clearly inhibiting my reading comprehension.\n\n-Peff\n"},{"id":"103868","messageId":"20090209184625.GA27037@coredump.intra.peff.net","threadId":"17629","inReplyTo":"1234170565-6740-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/6] t5400: allow individual tests to fail","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-09T18:46:26Z","receivedAt":"2009-02-09T18:46:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 09, 2009 at 01:09:21AM -0800, Junio C Hamano wrote:\n\n> Each test chdir'ed around and ended up in a random place if any of the\n> test in the sequence failed but the entire test script was allowed to\n> run.  This wrapps each in a subshell as necessary.\n\nCertainly a good cleanup, but...\n\n> -test_expect_success \\\n> -        'push can be used to delete a ref' '\n> +test_expect_success 'push can be used to delete a ref' '\n> +    (\n>  \tcd victim &&\n>  \tgit branch extra master &&\n>  \tcd .. &&\n>  \ttest -f victim/.git/refs/heads/extra &&\n>  \tgit send-pack ./victim/.git/ :extra master &&\n>  \t! test -f victim/.git/refs/heads/extra\n> +    )\n>  '\n\nWouldn't this be cleaner as:\n\n  (\n    cd victim &&\n    git branch extra master\n  ) &&\n  ...\n\nThat is, it is not only safer but (IMHO) a bit easier to see which parts\nare happening in which directory.\n\n> +test_expect_success 'pushing a delete should be denied with denyDeletes' '\n> +    (\n>  \tcd victim &&\n>  \tgit config receive.denyDeletes true &&\n>  \tgit branch extra master &&\n>  \tcd .. &&\n>  \ttest -f victim/.git/refs/heads/extra &&\n>  \ttest_must_fail git send-pack ./victim/.git/ :extra master\n> +    )\n\nDitto (and there are more, but I won't quote each one).\n\n> +test_expect_success 'pushing with --force should be denied with denyNonFastforwards' '\n> +    (\n>  \tcd victim &&\n>  \tgit config receive.denyNonFastforwards true &&\n>  \tcd .. &&\n>  \tgit update-ref refs/heads/master master^ || return 1\n>  \tgit send-pack --force ./victim/.git/ master && return 1\n>  \t! test_cmp .git/refs/heads/master victim/.git/refs/heads/master\n> +    )\n\nAnd here I don't know what in the world is going on with those \"return\n1\" lines. Shouldn't this be a chain of &&'s with a test_must_fail?\nI.e.,:\n\n  ( cd victim && git config receive.denyNonFastforwards true ) &&\n  git update-ref refs/heads/master master^ &&\n  test_must_fail git send-pack --force ./victim/.git/ master &&\n  ! test_cmp .git/refs/heads/master victim/.git/refs/heads/master\n\nNot to mention that the final test_cmp would be more robust if written\nto make sure the victim's master ref stayed the same (instead of just\nmaking sure we didn't screw it up in one particular way). And it should\nprobably use a git command rather than looking at the refs files (to be\nfuture-proof against any automatic ref-packing), but that is just\nnit-picking.\n\n\nAll minor things, of course, but while we're cleaning up... :)\n\n-Peff\n"},{"id":"103869","messageId":"20090209185310.GB27037@coredump.intra.peff.net","threadId":"17629","inReplyTo":"1234170565-6740-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 3/6] receive-pack: receive.denyDeleteCurrent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-09T18:53:10Z","receivedAt":"2009-02-09T18:53:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 09, 2009 at 01:09:22AM -0800, Junio C Hamano wrote:\n\n> This is a companion patch to the recent 3d95d92 (receive-pack: explain\n> what to do when push updates the current branch, 2009-01-31).\n> \n> Deleting the current branch from a remote will result in the next clone\n> from it not check out anything, among other things.  It also is one of the\n> cause that makes remotes/origin/HEAD a dangling symbolic ref.  This patch\n> still allows the traditional behaviour but with a big warning, and promises\n> that the default will change to 'refuse' in a future release.\n\nThis patch looks good to me. One comment:\n\n> --- a/t/t5400-send-pack.sh\n> +++ b/t/t5400-send-pack.sh\n> @@ -193,4 +193,11 @@ test_expect_success 'pushing wildcard refspecs respects forcing' '\n>      )\n>  '\n>  \n> +test_expect_success 'deny pushing to delete current branch' '\n> +    (\n> +\trewound_push_setup &&\n> +\tgit send-pack ../parent/.git :refs/heads/master 2>errs\n> +    )\n> +'\n\nIt was not immediately obvious why the subshell is needed here (for\nthose in the audience, it is because rewound_push_setup changes\ndirectory). Perhaps 2/6 should be amended for rewound_push_setup to use\na subshell instead, which makes it less likely to be forgotten by new\ntests.\n\n-Peff\n"},{"id":"103872","messageId":"7v4oz3v36a.fsf@gitster.siamese.dyndns.org","threadId":"17629","inReplyTo":"20090209184625.GA27037@coredump.intra.peff.net","subject":"Re: [PATCH 2/6] t5400: allow individual tests to fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T19:08:29Z","receivedAt":"2009-02-09T19:08:29Z","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 Mon, Feb 09, 2009 at 01:09:21AM -0800, Junio C Hamano wrote:\n>\n>> Each test chdir'ed around and ended up in a random place if any of the\n>> test in the sequence failed but the entire test script was allowed to\n>> run.  This wrapps each in a subshell as necessary.\n>\n> Certainly a good cleanup, but...\n>\n>> -test_expect_success \\\n>> -        'push can be used to delete a ref' '\n>> +test_expect_success 'push can be used to delete a ref' '\n>> +    (\n>>  \tcd victim &&\n>>  \tgit branch extra master &&\n>>  \tcd .. &&\n>>  \ttest -f victim/.git/refs/heads/extra &&\n>>  \tgit send-pack ./victim/.git/ :extra master &&\n>>  \t! test -f victim/.git/refs/heads/extra\n>> +    )\n>>  '\n>\n> Wouldn't this be cleaner as:\n>\n>   (\n>     cd victim &&\n>     git branch extra master\n>   ) &&\n>   ...\n>\n> That is, it is not only safer but (IMHO) a bit easier to see which parts\n> are happening in which directory.\n>\n>> +test_expect_success 'pushing a delete should be denied with denyDeletes' '\n>> +    (\n>>  \tcd victim &&\n>>  \tgit config receive.denyDeletes true &&\n>>  \tgit branch extra master &&\n>>  \tcd .. &&\n>>  \ttest -f victim/.git/refs/heads/extra &&\n>>  \ttest_must_fail git send-pack ./victim/.git/ :extra master\n>> +    )\n>\n> Ditto (and there are more, but I won't quote each one).\n>\n>> +test_expect_success 'pushing with --force should be denied with denyNonFastforwards' '\n>> +    (\n>>  \tcd victim &&\n>>  \tgit config receive.denyNonFastforwards true &&\n>>  \tcd .. &&\n>>  \tgit update-ref refs/heads/master master^ || return 1\n>>  \tgit send-pack --force ./victim/.git/ master && return 1\n>>  \t! test_cmp .git/refs/heads/master victim/.git/refs/heads/master\n>> +    )\n>\n> And here I don't know what in the world is going on with those \"return\n> 1\" lines. Shouldn't this be a chain of &&'s with a test_must_fail?\n> I.e.,:\n>\n>   ( cd victim && git config receive.denyNonFastforwards true ) &&\n>   git update-ref refs/heads/master master^ &&\n>   test_must_fail git send-pack --force ./victim/.git/ master &&\n>   ! test_cmp .git/refs/heads/master victim/.git/refs/heads/master\n>\n> Not to mention that the final test_cmp would be more robust if written\n> to make sure the victim's master ref stayed the same (instead of just\n> making sure we didn't screw it up in one particular way). And it should\n> probably use a git command rather than looking at the refs files (to be\n> future-proof against any automatic ref-packing), but that is just\n> nit-picking.\n>\n>\n> All minor things, of course, but while we're cleaning up... :)\n\nSure.  This was made as a quick-fix to a mess others created, so I did not\nstudy them very deeply.\n\nWill reroll if I have the time but it is likely that I may be tending\nother topics first.\n"},{"id":"103876","messageId":"20090209191526.GC27037@coredump.intra.peff.net","threadId":"17629","inReplyTo":"1234170565-6740-5-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 4/6] remote prune: warn dangling symrefs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-09T19:15:26Z","receivedAt":"2009-02-09T19:15:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 09, 2009 at 01:09:23AM -0800, Junio C Hamano wrote:\n\n> If you prune from the remote \"frotz\" that deleted the ref your tracking\n> branch remotes/frotz/HEAD points at, the symbolic ref will become\n> dangling.  We used to detect this as an error condition and issued a\n> message every time refs are enumerated.\n> \n> This stops the error message, but moves the warning to \"remote prune\".\n\nVery nice. As a bonus, this fixes certain (admittedly unlikely) renames,\ntoo (which don't need to pass the BROKEN flag, since ref_rename uses\nget_loose_refs directly):\n\n  # without this patch\n  $ git symbolic-ref refs/heads/foo/bar refs/heads/nonexistant\n  $ git branch -m master foo\n  error: refs/heads/foo/bar points nowhere!\n  error: there are still refs under 'refs/heads/foo'\n  error: unable to lock refs/heads/foo for update\n  fatal: Branch rename failed\n\n  # with this patch\n  $ git branch -m master foo\n  error: 'refs/heads/foo/bar' exists; cannot create 'refs/heads/foo'\n  fatal: Branch rename failed\n\n-Peff\n"},{"id":"103878","messageId":"20090209192210.GD27037@coredump.intra.peff.net","threadId":"17629","inReplyTo":"20090209185310.GB27037@coredump.intra.peff.net","subject":"Re: [PATCH 3/6] receive-pack: receive.denyDeleteCurrent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-09T19:22:10Z","receivedAt":"2009-02-09T19:22:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 09, 2009 at 01:53:10PM -0500, Jeff King wrote:\n\n> On Mon, Feb 09, 2009 at 01:09:22AM -0800, Junio C Hamano wrote:\n> \n> > This is a companion patch to the recent 3d95d92 (receive-pack: explain\n> > what to do when push updates the current branch, 2009-01-31).\n> > \n> > Deleting the current branch from a remote will result in the next clone\n> > from it not check out anything, among other things.  It also is one of the\n> > cause that makes remotes/origin/HEAD a dangling symbolic ref.  This patch\n> > still allows the traditional behaviour but with a big warning, and promises\n> > that the default will change to 'refuse' in a future release.\n> \n> This patch looks good to me. One comment:\n\nActually, one more comment on this one. If a user tries to delete the\ncurrent branch in a non-bare repository, they actually get _both_\ngigantic warning messages (one for denyCurrentBranch and one for\ndenyDeleteCurrent).\n\nShould the denyCurrentBranch code be triggering at all on a deletion?\nThat is, if I have:\n\n  [receive]\n    denyCurrentBranch = refuse\n    denyDeleteCurrent = ignore\n\nshould such a deletion be refused or allowed?\n\n-Peff\n"},{"id":"103905","messageId":"7veiy7thnv.fsf@gitster.siamese.dyndns.org","threadId":"17629","inReplyTo":"20090209192210.GD27037@coredump.intra.peff.net","subject":"Re: [PATCH 3/6] receive-pack: receive.denyDeleteCurrent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T21:38:28Z","receivedAt":"2009-02-09T21:38:28Z","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> Should the denyCurrentBranch code be triggering at all on a deletion?\n>\n> That is, if I have:\n>\n>   [receive]\n>     denyCurrentBranch = refuse\n>     denyDeleteCurrent = ignore\n>\n> should such a deletion be refused or allowed?\n\nI think denyCurrentBranch means do not touch the currently checked out\nbranch, so 'refuse' there should trump whatever denyDeleteCurrent says as\nlong as the repository has a work tree.\n\nPerhaps the logic needs to be restructured to:\n\n\tif (the push affects the current branch) {\n\t\tif (in a repository with a work tree) {\n                        decide if we want to refuse or allow;\n                \tdecide what message to issue;\n\t\t}\n\t\tif (deletion and we decided not to refuse) {\n                \tdecide if we want to refuse or allow;\n                        decide what message to issue;\n                }\n                give message(s), possibly with a paragraph break in between;\n                if (refuse)\n                \trefuse;\n\t}\n"},{"id":"103906","messageId":"7vab8vthlj.fsf@gitster.siamese.dyndns.org","threadId":"17629","inReplyTo":"7v4oz3v36a.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/6] t5400: allow individual tests to fail","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-09T21:39:52Z","receivedAt":"2009-02-09T21:39:52Z","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> Jeff King <peff@peff.net> writes:\n>\n>> All minor things, of course, but while we're cleaning up... :)\n>\n> Sure.  This was made as a quick-fix to a mess others created, so I did not\n> study them very deeply.\n>\n> Will reroll if I have the time but it is likely that I may be tending\n> other topics first.\n\n-- >8 --\nSubject: [PATCH] Modernize t5400 test script\n\nMany tests checked for failure by hand without using test_must_fail (they\nprobably predate the shell function).\n\nWhen we know the desired outcome, explicitly check for it, instead of\nchecking if the result does not match one possible incorrect outcome.\nE.g. if you expect a push to be refused, you do not test if the result is\ndifferent from what was pushed.  Instead, make sure that the ref did not\nbefore and after the push.\n\nThe test sequence chdir'ed around and any failure at one point could have\nstarted the next test in an unexpected directory.  Fix this problem by\nusing subshells as necessary.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5400-send-pack.sh |  180 ++++++++++++++++++++++++++------------------------\n 1 files changed, 94 insertions(+), 86 deletions(-)\n\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex b21317d..785c2f4 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -32,9 +32,7 @@ test_expect_success setup '\n \tdone &&\n \tgit update-ref HEAD \"$commit\" &&\n \tgit clone ./. victim &&\n-\tcd victim &&\n-\tgit log &&\n-\tcd .. &&\n+\t( cd victim && git log ) &&\n \tgit update-ref HEAD \"$zero\" &&\n \tparent=$zero &&\n \ti=0 &&\n@@ -59,88 +57,84 @@ test_expect_success 'pack the source repository' '\n '\n \n test_expect_success 'pack the destination repository' '\n+    (\n \tcd victim &&\n \tgit repack -a -d &&\n-\tgit prune &&\n-\tcd ..\n+\tgit prune\n+    )\n '\n \n-test_expect_success \\\n-        'pushing rewound head should not barf but require --force' '\n-\t# should not fail but refuse to update.\n-\tif git send-pack ./victim/.git/ master\n-\tthen\n-\t\t# now it should fail with Pasky patch\n-\t\techo >&2 Gaah, it should have failed.\n-\t\tfalse\n-\telse\n-\t\techo >&2 Thanks, it correctly failed.\n-\t\ttrue\n-\tfi &&\n-\tif cmp victim/.git/refs/heads/master .git/refs/heads/master\n-\tthen\n-\t\t# should have been left as it was!\n-\t\tfalse\n-\telse\n-\t\ttrue\n-\tfi &&\n+test_expect_success 'refuse pushing rewound head without --force' '\n+\tpushed_head=$(git rev-parse --verify master) &&\n+\tvictim_orig=$(cd victim && git rev-parse --verify master) &&\n+\ttest_must_fail git send-pack ./victim master &&\n+\tvictim_head=$(cd victim && git rev-parse --verify master) &&\n+\ttest \"$victim_head\" = \"$victim_orig\" &&\n \t# this should update\n-\tgit send-pack --force ./victim/.git/ master &&\n-\tcmp victim/.git/refs/heads/master .git/refs/heads/master\n+\tgit send-pack --force ./victim master &&\n+\tvictim_head=$(cd victim && git rev-parse --verify master) &&\n+\ttest \"$victim_head\" = \"$pushed_head\"\n '\n \n test_expect_success \\\n         'push can be used to delete a ref' '\n-\tcd victim &&\n-\tgit branch extra master &&\n-\tcd .. &&\n-\ttest -f victim/.git/refs/heads/extra &&\n-\tgit send-pack ./victim/.git/ :extra master &&\n-\t! test -f victim/.git/refs/heads/extra\n+\t( cd victim && git branch extra master ) &&\n+\tgit send-pack ./victim :extra master &&\n+\t( cd victim &&\n+\t  test_must_fail git rev-parse --verify extra )\n '\n \n-unset GIT_CONFIG\n-HOME=`pwd`/no-such-directory\n-export HOME ;# this way we force the victim/.git/config to be used.\n-\n-test_expect_success \\\n-\t'pushing a delete should be denied with denyDeletes' '\n-\tcd victim &&\n-\tgit config receive.denyDeletes true &&\n-\tgit branch extra master &&\n-\tcd .. &&\n-\ttest -f victim/.git/refs/heads/extra &&\n-\ttest_must_fail git send-pack ./victim/.git/ :extra master\n+test_expect_success 'refuse deleting push with denyDeletes' '\n+\t(\n+\t    cd victim &&\n+\t    ( git branch -D extra || : ) &&\n+\t    git config receive.denyDeletes true &&\n+\t    git branch extra master\n+\t) &&\n+\ttest_must_fail git send-pack ./victim :extra master\n '\n-rm -f victim/.git/refs/heads/extra\n \n-test_expect_success \\\n-        'pushing with --force should be denied with denyNonFastforwards' '\n-\tcd victim &&\n-\tgit config receive.denyNonFastforwards true &&\n-\tcd .. &&\n-\tgit update-ref refs/heads/master master^ || return 1\n-\tgit send-pack --force ./victim/.git/ master && return 1\n-\t! test_cmp .git/refs/heads/master victim/.git/refs/heads/master\n+test_expect_success 'denyNonFastforwards trumps --force' '\n+\t(\n+\t    cd victim &&\n+\t    ( git branch -D extra || : ) &&\n+\t    git config receive.denyNonFastforwards true\n+\t) &&\n+\tvictim_orig=$(cd victim && git rev-parse --verify master) &&\n+\ttest_must_fail git send-pack --force ./victim master^:master &&\n+\tvictim_head=$(cd victim && git rev-parse --verify master) &&\n+\ttest \"$victim_orig\" = \"$victim_head\"\n '\n \n-test_expect_success \\\n-\t'pushing does not include non-head refs' '\n-\tmkdir parent && cd parent &&\n-\tgit init && touch file && git add file && git commit -m add &&\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+test_expect_success 'push --all excludes remote tracking hierarchy' '\n+\tmkdir parent &&\n+\t(\n+\t    cd parent &&\n+\t    git init && : >file && git add file && git commit -m add\n+\t) &&\n+\tgit clone parent child &&\n+\t(\n+\t    cd child && git push --all\n+\t) &&\n+\t(\n+\t    cd parent &&\n+\t    test -z \"$(git for-each-ref refs/remotes/origin)\"\n+\t)\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-\techo two >file && git commit -a -m two &&\n-\tcd .. &&\n-\tgit clone parent child && cd child && git reset --hard HEAD^\n+\tmkdir parent &&\n+\t(\n+\t    cd parent &&\n+\t    git init &&\n+\t    echo one >file && git add file && git commit -m one &&\n+\t    echo two >file && git commit -a -m two\n+\t) &&\n+\tgit clone parent child &&\n+\t(\n+\t    cd child && git reset --hard HEAD^\n+\t)\n }\n \n rewound_push_succeeded() {\n@@ -156,30 +150,44 @@ rewound_push_failed() {\n \tfi\n }\n \n-test_expect_success \\\n-\t'pushing explicit refspecs respects forcing' '\n+test_expect_success 'pushing explicit refspecs respects forcing' '\n \trewound_push_setup &&\n-\tif git send-pack ../parent/.git refs/heads/master:refs/heads/master\n-\tthen\n-\t\tfalse\n-\telse\n-\t\ttrue\n-\tfi && rewound_push_failed &&\n-\tgit send-pack ../parent/.git +refs/heads/master:refs/heads/master &&\n-\trewound_push_succeeded\n+\tparent_orig=$(cd parent && git rev-parse --verify master) &&\n+\t(\n+\t    cd child &&\n+\t    test_must_fail git send-pack ../parent \\\n+\t\trefs/heads/master:refs/heads/master\n+\t) &&\n+\tparent_head=$(cd parent && git rev-parse --verify master) &&\n+\ttest \"$parent_orig\" = \"$parent_head\"\n+\t(\n+\t    cd child &&\n+\t    git send-pack ../parent \\\n+\t        +refs/heads/master:refs/heads/master\n+\t) &&\n+\tparent_head=$(cd parent && git rev-parse --verify master) &&\n+\tchild_head=$(cd parent && git rev-parse --verify master) &&\n+\ttest \"$parent_head\" = \"$child_head\"\n '\n \n-test_expect_success \\\n-\t'pushing wildcard refspecs respects forcing' '\n+test_expect_success 'pushing wildcard refspecs respects forcing' '\n \trewound_push_setup &&\n-\tif git send-pack ../parent/.git refs/heads/*:refs/heads/*\n-\tthen\n-\t\tfalse\n-\telse\n-\t\ttrue\n-\tfi && rewound_push_failed &&\n-\tgit send-pack ../parent/.git +refs/heads/*:refs/heads/* &&\n-\trewound_push_succeeded\n+\tparent_orig=$(cd parent && git rev-parse --verify master) &&\n+\t(\n+\t    cd child &&\n+\t    test_must_fail git send-pack ../parent \\\n+\t        \"refs/heads/*:refs/heads/*\"\n+\t) &&\n+\tparent_head=$(cd parent && git rev-parse --verify master) &&\n+\ttest \"$parent_orig\" = \"$parent_head\"\n+\t(\n+\t    cd child &&\n+\t    git send-pack ../parent \\\n+\t        \"+refs/heads/*:refs/heads/*\"\n+\t) &&\n+\tparent_head=$(cd parent && git rev-parse --verify master) &&\n+\tchild_head=$(cd parent && git rev-parse --verify master) &&\n+\ttest \"$parent_head\" = \"$child_head\"\n '\n \n test_done\n-- \n1.6.2.rc0.36.g8307\n"},{"id":"103987","messageId":"20090210120107.GA15491@coredump.intra.peff.net","threadId":"17629","inReplyTo":"7vab8vthlj.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/6] t5400: allow individual tests to fail","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T12:01:07Z","receivedAt":"2009-02-10T12:01:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 09, 2009 at 01:39:52PM -0800, Junio C Hamano wrote:\n\n> > Will reroll if I have the time but it is likely that I may be tending\n> > other topics first.\n> \n> -- >8 --\n> Subject: [PATCH] Modernize t5400 test script\n\nMuch nicer, thanks.\n\n-Peff\n"},{"id":"103988","messageId":"20090210120753.GB15491@coredump.intra.peff.net","threadId":"17629","inReplyTo":"7veiy7thnv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/6] receive-pack: receive.denyDeleteCurrent","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T12:07:53Z","receivedAt":"2009-02-10T12:07:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 09, 2009 at 01:38:28PM -0800, Junio C Hamano wrote:\n\n> > Should the denyCurrentBranch code be triggering at all on a deletion?\n> >\n> > That is, if I have:\n> >\n> >   [receive]\n> >     denyCurrentBranch = refuse\n> >     denyDeleteCurrent = ignore\n> >\n> > should such a deletion be refused or allowed?\n> \n> I think denyCurrentBranch means do not touch the currently checked out\n> branch, so 'refuse' there should trump whatever denyDeleteCurrent says as\n> long as the repository has a work tree.\n\nI'm not sure \"trump\" is the right behavior. How would I specify \"it is\nOK to update this branch, but not to delete it, because I have installed\na hook that deals with the former but not the latter\".\n\nIt seems like \"delete\" is a subset of \"touch\", so I think you probably\nwant to refuse if _either_ is refuse. Which I think maybe is what you\nare saying here:\n\n> Perhaps the logic needs to be restructured to:\n> \n> \tif (the push affects the current branch) {\n> \t\tif (in a repository with a work tree) {\n>                         decide if we want to refuse or allow;\n>                 \tdecide what message to issue;\n> \t\t}\n> \t\tif (deletion and we decided not to refuse) {\n>                 \tdecide if we want to refuse or allow;\n>                         decide what message to issue;\n>                 }\n>                 give message(s), possibly with a paragraph break in between;\n>                 if (refuse)\n>                 \trefuse;\n> \t}\n\nbut it was not immediately clear to me.\n\n-Peff\n"},{"id":"104022","messageId":"7vfximpblk.fsf@gitster.siamese.dyndns.org","threadId":"17629","inReplyTo":"20090210120753.GB15491@coredump.intra.peff.net","subject":"Re: [PATCH 3/6] receive-pack: receive.denyDeleteCurrent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T15:15:19Z","receivedAt":"2009-02-10T15:15:19Z","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 denyCurrentBranch means do not touch the currently checked out\n>> branch, so 'refuse' there should trump whatever denyDeleteCurrent says as\n>> long as the repository has a work tree.\n>\n> I'm not sure \"trump\" is the right behavior. How would I specify \"it is\n> OK to update this branch, but not to delete it, because I have installed\n> a hook that deals with the former but not the latter\".\n>\n> It seems like \"delete\" is a subset of \"touch\", so I think you probably\n> want to refuse if _either_ is refuse. Which I think maybe is what you\n> are saying here:\n\nI meant \"'refuse' in denyCurrentBranch trumps\" and not \"whatever is set to\ndenyCurrentBranch trumps\".  IOW, I think we are in agreement.\n"},{"id":"104220","messageId":"7vhc30g9ub.fsf@gitster.siamese.dyndns.org","threadId":"17629","inReplyTo":"20090209191526.GC27037@coredump.intra.peff.net","subject":"Re: [PATCH 4/6] remote prune: warn dangling symrefs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-11T17:30:20Z","receivedAt":"2009-02-11T17:30:20Z","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 Mon, Feb 09, 2009 at 01:09:23AM -0800, Junio C Hamano wrote:\n>\n>> If you prune from the remote \"frotz\" that deleted the ref your tracking\n>> branch remotes/frotz/HEAD points at, the symbolic ref will become\n>> dangling.  We used to detect this as an error condition and issued a\n>> message every time refs are enumerated.\n>> \n>> This stops the error message, but moves the warning to \"remote prune\".\n>\n> Very nice. As a bonus, this fixes certain (admittedly unlikely) renames,\n> too (which don't need to pass the BROKEN flag, since ref_rename uses\n> get_loose_refs directly):\n>\n>   # without this patch\n>   $ git symbolic-ref refs/heads/foo/bar refs/heads/nonexistant\n>   $ git branch -m master foo\n>   error: refs/heads/foo/bar points nowhere!\n>   error: there are still refs under 'refs/heads/foo'\n>   error: unable to lock refs/heads/foo for update\n>   fatal: Branch rename failed\n>\n>   # with this patch\n>   $ git branch -m master foo\n>   error: 'refs/heads/foo/bar' exists; cannot create 'refs/heads/foo'\n>   fatal: Branch rename failed\n\nAs a bonus, this issues unwarranted warning when creating the initial\ncommit in an empty repository.\n\nThe following fixes it.\n\n-- >8 --\nSubject: [PATCH] Squelch overzealous \"ignoring dangling symref\" in an empty repository\n\n057e713 (Warn use of \"origin\" when remotes/origin/HEAD is dangling,\n2009-02-08) tried to warn dangling refs/remotes/origin/HEAD only when\n\"origin\" was used to refer to it.  There was one corner case a symref is\nexpected to be dangling and this warning is unwarranted: HEAD in an empty\nrepository.\n\nThis squelches the warning for this special case.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 3bd2ef0..2f75179 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -278,7 +278,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)\n \t\t\t\t*ref = xstrdup(r);\n \t\t\tif (!warn_ambiguous_refs)\n \t\t\t\tbreak;\n-\t\t} else if (flag & REF_ISSYMREF)\n+\t\t} else if ((flag & REF_ISSYMREF) &&\n+\t\t\t   (len != 4 || strcmp(str, \"HEAD\")))\n \t\t\twarning(\"ignoring dangling symref %s.\", fullref);\n \t}\n \tfree(last_branch);\n-- \n1.6.2.rc0.55.g7a105\n"},{"id":"104245","messageId":"20090211183547.GB21122@coredump.intra.peff.net","threadId":"17629","inReplyTo":"7vhc30g9ub.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/6] remote prune: warn dangling symrefs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-11T18:35:47Z","receivedAt":"2009-02-11T18:35:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 11, 2009 at 09:30:20AM -0800, Junio C Hamano wrote:\n\n> Subject: [PATCH] Squelch overzealous \"ignoring dangling symref\" in an empty repository\n> \n> 057e713 (Warn use of \"origin\" when remotes/origin/HEAD is dangling,\n> 2009-02-08) tried to warn dangling refs/remotes/origin/HEAD only when\n> \"origin\" was used to refer to it.  There was one corner case a symref is\n> expected to be dangling and this warning is unwarranted: HEAD in an empty\n> repository.\n> \n> This squelches the warning for this special case.\n\nIs the special case really about \"this is HEAD\", or is it about writing\nversus reading? For example, in an empty repo, without this patch I now\nget:\n\n  $ git init && git show\n  warning: ignoring dangling symref HEAD.\n  fatal: bad default revision 'HEAD'\n\nwhich makes a lot of sense. But _writing_ to any dangling symref\nshouldn't trigger a warning.\n\nAdmittedly, we have gotten by without this warning until now, and I\ndoubt anyone will want to write to other symrefs that are branches to be\nborn, so I think in practice your patch is fine.\n\n-Peff\n"},{"id":"104263","messageId":"20090211184205.GA27859@coredump.intra.peff.net","threadId":"17629","inReplyTo":"20090211183547.GB21122@coredump.intra.peff.net","subject":"Re: [PATCH 4/6] remote prune: warn dangling symrefs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-11T18:42:05Z","receivedAt":"2009-02-11T18:42:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 11, 2009 at 01:35:47PM -0500, Jeff King wrote:\n\n> Is the special case really about \"this is HEAD\", or is it about writing\n> versus reading? For example, in an empty repo, without this patch I now\n> get:\n> \n>   $ git init && git show\n>   warning: ignoring dangling symref HEAD.\n>   fatal: bad default revision 'HEAD'\n> \n> which makes a lot of sense. But _writing_ to any dangling symref\n> shouldn't trigger a warning.\n> \n> Admittedly, we have gotten by without this warning until now, and I\n> doubt anyone will want to write to other symrefs that are branches to be\n> born, so I think in practice your patch is fine.\n\nI looked into what it would take to fix this, and it's a little hairy.\n\"git commit\" calls:\n\n  if (get_sha1(\"HEAD\", head_sha1))\n          initial_commit = 1;\n\nSo it is fine with HEAD not resolving, but that intent is not passed to\nlower-level code. So we could add a \"flags\" field, but the callstack\nlooks like:\n\n  dwim_ref\n  get_sha1_basic\n  get_sha1_1\n  get_sha1_with_mode\n  get_sha1\n\nall of which would need to pass the flag through, and all of which have\ntons of callers (though we could do the get_sha1_with_flags trick).\n\nI don't know if it's worth it.\n\n-Peff\n"}]}