{"thread":{"id":"38968","subject":"Trying to push into empty repo, get fatal: bad revision 'HEAD'","startedAt":"2015-03-31T23:29:20Z","lastAt":"2015-04-02T05:07:19Z","messageCount":7,"participants":["Samuel Williams","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"258800","messageId":"CAHkN8V8cpZUmCi6=MUcsCOCBe6H_G6btr6WaV2vVrO1Nm_r4tg@mail.gmail.com","threadId":"38968","inReplyTo":null,"subject":"Trying to push into empty repo, get fatal: bad revision 'HEAD'","fromName":"Samuel Williams","fromEmail":"space.ship.traveller@gmail.com","sentAt":"2015-03-31T23:29:20Z","receivedAt":"2015-03-31T23:29:20Z","isPatch":false,"sender":{"key":"space.ship.traveller@gmail.com","avatar":null},"body":"I have set up a remote repository like so:\n\nremote $ cat .git/config\n[core]\nrepositoryformatversion = 0\nfilemode = true\nbare = false\nlogallrefupdates = true\nsharedrepository = 1\nworktree = /srv/http/www.site.com\n[receive]\ndenyNonFastforwards = true\ndenyCurrentBranch = updateInstead\n\nThis repo is empty by default.\n\nWhen I try to push into it, I get the following:\n\nlocal $ git push --set-upstream production master\nCounting objects: 29, done.\nDelta compression using up to 8 threads.\nCompressing objects: 100% (27/27), done.\nWriting objects: 100% (29/29), 1.13 MiB | 0 bytes/s, done.\nTotal 29 (delta 3), reused 17 (delta 0)\nfatal: bad revision 'HEAD'\nTo ssh://remote.net/srv/http/www.site.com\n ! [remote rejected] master -> master (Working directory has staged changes)\nerror: failed to push some refs to 'ssh://remote.net/srv/http/www.site.com'\n\nIs this expected? Am I doing something wrong? Or, is this a bug in Git?\n\nI would expect if you push to an empty repo, it would update it\n(because denyCurrentBranch = updateInstead).\n\nThanks\n"},{"id":"258803","messageId":"xmqqego4ogxw.fsf@gitster.dls.corp.google.com","threadId":"38968","inReplyTo":"CAHkN8V8cpZUmCi6=MUcsCOCBe6H_G6btr6WaV2vVrO1Nm_r4tg@mail.gmail.com","subject":"Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-01T00:00:59Z","receivedAt":"2015-04-01T00:00:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Williams <space.ship.traveller@gmail.com> writes:\n\n> I would expect if you push to an empty repo, it would update it\n> (because denyCurrentBranch = updateInstead).\n\nGood finding.\n\nI think the current implementation of updateInstead is set up to\nbootstrap from an empty repository but only supports incremental\nupdates once the receiving repository and its working tree gets set\nup.  But I do not think it was a conscious design decison to forbid\nbootstrapping an empty repository, but was a mere gap in the\nimplementation.  At least, I do not think of a reason why we should\nforbid it (and I am Cc'ing Dscho to confirm).\n\nFixing it should not be too hard, but I am on a bus right now so...\n"},{"id":"258806","messageId":"xmqq8uecocjg.fsf@gitster.dls.corp.google.com","threadId":"38968","inReplyTo":"xmqqego4ogxw.fsf@gitster.dls.corp.google.com","subject":"Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-01T01:36:03Z","receivedAt":"2015-04-01T01:36:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Samuel Williams <space.ship.traveller@gmail.com> writes:\n>\n>> I would expect if you push to an empty repo, it would update it\n>> (because denyCurrentBranch = updateInstead).\n>\n> Good finding.\n>\n> I think the current implementation of updateInstead is set up to\n> bootstrap from an empty repository but only supports incremental\n> updates once the receiving repository and its working tree gets set\n> up.  But I do not think it was a conscious design decison to forbid\n> bootstrapping an empty repository, but was a mere gap in the\n> implementation.  At least, I do not think of a reason why we should\n> forbid it (and I am Cc'ing Dscho to confirm).\n>\n> Fixing it should not be too hard, but I am on a bus right now so...\n\nA fix (or is it an enhancement) would probably look like this.\n\nThis is a tangent but I think we should unify the \"do we already\nhave history behind HEAD, or is the current branch still unborn\"\ntest done by various commands and tighten it.  As a quick and dirty\nhack, I just mimicked what builtin/merge.c seems to do, but this\nwould tell a detached HEAD that points at a nonsense object name\n(i.e. \"abcde\" not a full 40-hex) as \"unborn\", where we would be\nbetter off stopping the operation instead of making the repository\nbreakage worse by doing further damage.\n\nI originally suspected I'd need to fix the push_to_checkout()\ncodepath, too, but it turns out that the detection of unborn-ness\nof the current branch is also outsourced to the push-to-checkout\nhook, so I do not have to do anything special ;-)\n\n builtin/receive-pack.c | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex fc8ec9c..758b0b3 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -733,6 +733,13 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)\n \treturn 0;\n }\n \n+static int head_has_history(void)\n+{\n+\tunsigned char sha1[20];\n+\n+\treturn !get_sha1(\"HEAD\", sha1);\n+}\n+\n static const char *push_to_deploy(unsigned char *sha1,\n \t\t\t\t  struct argv_array *env,\n \t\t\t\t  const char *work_tree)\n@@ -745,13 +752,15 @@ static const char *push_to_deploy(unsigned char *sha1,\n \t};\n \tconst char *diff_index[] = {\n \t\t\"diff-index\", \"--quiet\", \"--cached\", \"--ignore-submodules\",\n-\t\t\"HEAD\", \"--\", NULL\n+\t\tNULL, \"--\", NULL\n \t};\n \tconst char *read_tree[] = {\n \t\t\"read-tree\", \"-u\", \"-m\", NULL, NULL\n \t};\n \tstruct child_process child = CHILD_PROCESS_INIT;\n \n+\tint empty = !head_has_history();\n+\n \tchild.argv = update_refresh;\n \tchild.env = env->argv;\n \tchild.dir = work_tree;\n@@ -772,6 +781,9 @@ static const char *push_to_deploy(unsigned char *sha1,\n \tif (run_command(&child))\n \t\treturn \"Working directory has unstaged changes\";\n \n+\t/* diff-index with either HEAD or an empty tree */\n+\tdiff_index[4] = empty ? EMPTY_TREE_SHA1_HEX : \"HEAD\";\n+\n \tchild_process_init(&child);\n \tchild.argv = diff_index;\n \tchild.env = env->argv;\n"},{"id":"258809","messageId":"xmqq4mp0nzla.fsf@gitster.dls.corp.google.com","threadId":"38968","inReplyTo":"xmqq8uecocjg.fsf@gitster.dls.corp.google.com","subject":"Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-01T06:15:45Z","receivedAt":"2015-04-01T06:15:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> A fix (or is it an enhancement) would probably look like this.\n> ...\n\nAnd this time with a pair of tests.  It probably should be extended\nto make sure it fails when the pushed HEAD records paths that are\nfloating in the working tree (as that would mean overwrting them),\nbut it is getting late and I am lazy, so ... ;-)\n\n-- >8 --\nSubject: [PATCH] push-to-deploy: allow pushing into an unborn branch and updating it\n\nSetting receive.denycurrentbranch to updateinstead and pushing into\nthe current branch, when the working tree and the index is truly\nclean, is supposed to reset the working tree and the index to match\nthe tree of the pushed commit.  This did not work when pushing into\nan unborn branch.\n\nThe code that drives push-to-checkout hook needs no change, as the\ninterface is defined so that hook can decide what to do when the\npush is coming to an unborn branch and take an appropriate action\nsince the beginning.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/receive-pack.c | 21 ++++++++++++++++++-\n t/t5516-fetch-push.sh  | 55 +++++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 74 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex fc8ec9c..0c0a261 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -733,6 +733,22 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)\n \treturn 0;\n }\n \n+/*\n+ * NEEDSWORK: we should consolidate various implementions of \"are we\n+ * on an unborn branch?\" test into one, and make the unified one more\n+ * robust. !get_sha1() based check used here and elsewhere would not\n+ * allow us to tell an unborn branch from corrupt ref, for example.\n+ * For the purpose of fixing \"deploy-to-update does not work when\n+ * pushing into an empty repository\" issue, this should suffice for\n+ * now.\n+ */\n+static int head_has_history(void)\n+{\n+\tunsigned char sha1[20];\n+\n+\treturn !get_sha1(\"HEAD\", sha1);\n+}\n+\n static const char *push_to_deploy(unsigned char *sha1,\n \t\t\t\t  struct argv_array *env,\n \t\t\t\t  const char *work_tree)\n@@ -745,7 +761,7 @@ static const char *push_to_deploy(unsigned char *sha1,\n \t};\n \tconst char *diff_index[] = {\n \t\t\"diff-index\", \"--quiet\", \"--cached\", \"--ignore-submodules\",\n-\t\t\"HEAD\", \"--\", NULL\n+\t\tNULL, \"--\", NULL\n \t};\n \tconst char *read_tree[] = {\n \t\t\"read-tree\", \"-u\", \"-m\", NULL, NULL\n@@ -772,6 +788,9 @@ static const char *push_to_deploy(unsigned char *sha1,\n \tif (run_command(&child))\n \t\treturn \"Working directory has unstaged changes\";\n \n+\t/* diff-index with either HEAD or an empty tree */\n+\tdiff_index[4] = head_has_history() ? \"HEAD\" : EMPTY_TREE_SHA1_HEX;\n+\n \tchild_process_init(&child);\n \tchild.argv = diff_index;\n \tchild.env = env->argv;\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex e4436c1..329d7d4 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -1430,8 +1430,22 @@ test_expect_success 'receive.denyCurrentBranch = updateInstead' '\n \t\ttest $(git -C .. rev-parse HEAD^^) = $(git rev-parse HEAD) &&\n \t\tgit diff --quiet &&\n \t\ttest fifth = \"$(cat path3)\"\n-\t)\n+\t) &&\n \n+\t# (5) push into void\n+\trm -fr void &&\n+\tgit init void &&\n+\t(\n+\t\tcd void &&\n+\t\tgit config receive.denyCurrentBranch updateInstead\n+\t) &&\n+\tgit push void master &&\n+\t(\n+\t\tcd void &&\n+\t\ttest $(git -C .. rev-parse master) = $(git rev-parse HEAD) &&\n+\t\tgit diff --quiet &&\n+\t\tgit diff --cached --quiet\n+\t)\n '\n \n test_expect_success 'updateInstead with push-to-checkout hook' '\n@@ -1494,6 +1508,45 @@ test_expect_success 'updateInstead with push-to-checkout hook' '\n \t\ttest \"$(cat path5)\" = irrelevant &&\n \t\ttest \"$(git diff --name-only --cached HEAD)\" = path5 &&\n \t\ttest $(git -C .. rev-parse HEAD) = $(git rev-parse HEAD)\n+\t) &&\n+\n+\t# push into void\n+\trm -fr void &&\n+\tgit init void &&\n+\t(\n+\t\tcd void &&\n+\t\tgit config receive.denyCurrentBranch updateInstead &&\n+\t\twrite_script .git/hooks/push-to-checkout <<-\\EOF\n+\t\tif git rev-parse --quiet --verify HEAD\n+\t\tthen\n+\t\t\thas_head=yes\n+\t\t\techo >&2 updating from $(git rev-parse HEAD)\n+\t\telse\n+\t\t\thas_head=no\n+\t\t\techo >&2 pushing into void\n+\t\tfi\n+\t\techo >&2 updating to \"$1\"\n+\n+\t\tgit update-index -q --refresh &&\n+\t\tcase \"$has_head\" in\n+\t\tyes)\n+\t\t\tgit read-tree -u -m HEAD \"$1\" ;;\n+\t\tno)\n+\t\t\tgit read-tree -u -m \"$1\" ;;\n+\t\tesac || {\n+\t\t\tstatus=$?\n+\t\t\techo >&2 read-tree failed\n+\t\t\texit $status\n+\t\t}\n+\t\tEOF\n+\t) &&\n+\n+\tgit push void master &&\n+\t(\n+\t\tcd void &&\n+\t\tgit diff --quiet &&\n+\t\tgit diff --cached --quiet &&\n+\t\ttest $(git -C .. rev-parse HEAD) = $(git rev-parse HEAD)\n \t)\n '\n \n-- \n2.4.0-rc0-179-ge750d7b\n"},{"id":"258814","messageId":"cb4098391d46cc25b3d4d059e35dc806@www.dscho.org","threadId":"38968","inReplyTo":"xmqq8uecocjg.fsf@gitster.dls.corp.google.com","subject":"Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-04-01T08:51:34Z","receivedAt":"2015-04-01T08:51:34Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Samuel & Junio,\n\nOn 2015-04-01 03:36, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> Samuel Williams <space.ship.traveller@gmail.com> writes:\n>>\n>>> I would expect if you push to an empty repo, it would update it\n>>> (because denyCurrentBranch = updateInstead).\n>>\n>> Good finding.\n>>\n>> I think the current implementation of updateInstead is set up to\n>> bootstrap from an empty repository but only supports incremental\n>> updates once the receiving repository and its working tree gets set\n>> up.  But I do not think it was a conscious design decison to forbid\n>> bootstrapping an empty repository, but was a mere gap in the\n>> implementation.  At least, I do not think of a reason why we should\n>> forbid it (and I am Cc'ing Dscho to confirm).\n> \n> This is a tangent but I think we should unify the \"do we already\n> have history behind HEAD, or is the current branch still unborn\"\n> test done by various commands and tighten it.\n\nRight, I did not think about that use case at all.\n\n> As a quick and dirty hack, I just mimicked what builtin/merge.c seems to do, but this would tell a detached HEAD that points at a nonsense object name (i.e. \"abcde\" not a full 40-hex) as \"unborn\", where we would be better off stopping the operation instead of making the repository breakage worse by doing further damage.\n\nYeah, and we could refactor that into a global function, too. But for the moment, I think your proposed patch is good enough.\n\nCiao,\nDscho\n"},{"id":"258833","messageId":"xmqqd23nn2ym.fsf@gitster.dls.corp.google.com","threadId":"38968","inReplyTo":"cb4098391d46cc25b3d4d059e35dc806@www.dscho.org","subject":"Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-01T18:00:33Z","receivedAt":"2015-04-01T18:00:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Yeah, and we could refactor that into a global function, too. But for\n> the moment, I think your proposed patch is good enough.\n\nOK, so can I forge your Acked-by?\n\nThanks for double checking.\n"},{"id":"258847","messageId":"79faa1f88bcfa26d2ce8dd65e4e944ae@www.dscho.org","threadId":"38968","inReplyTo":"xmqqd23nn2ym.fsf@gitster.dls.corp.google.com","subject":"Re: Trying to push into empty repo, get fatal: bad revision 'HEAD'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-04-02T05:07:19Z","receivedAt":"2015-04-02T05:07:19Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn 2015-04-01 20:00, Junio C Hamano wrote:\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n>> Yeah, and we could refactor that into a global function, too. But for\n>> the moment, I think your proposed patch is good enough.\n> \n> OK, so can I forge your Acked-by?\n\nYou read my mind.\n\nCiao,\nDscho\n"}]}