{"thread":{"id":"28521","subject":"[PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","startedAt":"2011-09-28T15:39:35Z","lastAt":"2011-09-30T18:19:24Z","messageCount":8,"participants":["Pang Yan Han","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"176426","messageId":"20110928153935.GA7800@myhost","threadId":"28521","inReplyTo":null,"subject":"[PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Pang Yan Han","fromEmail":"pangyanhan@gmail.com","sentAt":"2011-09-28T15:39:35Z","receivedAt":"2011-09-28T15:39:35Z","isPatch":true,"sender":{"key":"pangyanhan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/539739?v=4"},"body":"When a push specifies deletion of non-existent refs, the post post-receive and\npost-update hooks receive as input/arguments the non-existent refs.\n\nFor instance, for the following push, where refs/heads/nonexistent is a ref\nwhich does not exist on the remote side:\n\n\tgit push origin :refs/heads/nonexistent\n\nthe post-receive hook receives from standard input:\n\n\t<null-sha1> SP <null-sha1> SP refs/heads/nonexistent\n\nand the post-update hook receives as arguments:\n\n\trefs/heads/nonexistent\n\nwhich does not make sense since it is a no-op.\n\nTeach receive-pack to not pass non-existent refs as input / arguments to the\npost-receive and post-update hooks in the event of a push involving\nnon-existent ref deletion.\n\nSigned-off-by: Pang Yan Han <pangyanhan@gmail.com>\n---\n builtin/receive-pack.c |   31 ++++++++-\n t/t5516-fetch-push.sh  |  170 ++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 197 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ae164da..8a0a9d2 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -153,6 +153,26 @@ struct command {\n \tchar ref_name[FLEX_ARRAY]; /* more */\n };\n \n+/* For invalid refs */\n+static struct command **invalid_delete;\n+static size_t invalid_delete_nr;\n+static size_t invalid_delete_alloc;\n+\n+static void invalid_delete_append(struct command *cmd)\n+{\n+\tALLOC_GROW(invalid_delete, invalid_delete_nr + 1, invalid_delete_alloc);\n+\tinvalid_delete[invalid_delete_nr++] = cmd;\n+}\n+\n+static int is_invalid_delete(struct command *cmd)\n+{\n+\tsize_t i;\n+\tfor (i = 0; i < invalid_delete_nr; i++)\n+\t\tif (invalid_delete[i] == cmd)\n+\t\t\treturn 1;\n+\treturn 0;\n+}\n+\n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n@@ -215,7 +235,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)\n \tint have_input = 0, code;\n \n \tfor (cmd = commands; !have_input && cmd; cmd = cmd->next) {\n-\t\tif (!cmd->error_string)\n+\t\tif (!cmd->error_string && !is_invalid_delete(cmd))\n \t\t\thave_input = 1;\n \t}\n \n@@ -248,7 +268,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)\n \t}\n \n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n-\t\tif (!cmd->error_string) {\n+\t\tif (!cmd->error_string && !is_invalid_delete(cmd)) {\n \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n \t\t\t\tsha1_to_hex(cmd->old_sha1),\n \t\t\t\tsha1_to_hex(cmd->new_sha1),\n@@ -447,6 +467,8 @@ static const char *update(struct command *cmd)\n \t\tif (!parse_object(old_sha1)) {\n \t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n \t\t\told_sha1 = NULL;\n+\t\t\tif (!ref_exists((char *) name))\n+\t\t\t\tinvalid_delete_append(cmd);\n \t\t}\n \t\tif (delete_ref(namespaced_name, old_sha1, 0)) {\n \t\t\trp_error(\"failed to delete %s\", name);\n@@ -477,7 +499,7 @@ static void run_update_post_hook(struct command *commands)\n \tstruct child_process proc;\n \n \tfor (argc = 0, cmd = commands; cmd; cmd = cmd->next) {\n-\t\tif (cmd->error_string)\n+\t\tif (cmd->error_string || is_invalid_delete(cmd))\n \t\t\tcontinue;\n \t\targc++;\n \t}\n@@ -488,7 +510,7 @@ static void run_update_post_hook(struct command *commands)\n \n \tfor (argc = 1, cmd = commands; cmd; cmd = cmd->next) {\n \t\tchar *p;\n-\t\tif (cmd->error_string)\n+\t\tif (cmd->error_string || is_invalid_delete(cmd))\n \t\t\tcontinue;\n \t\tp = xmalloc(strlen(cmd->ref_name) + 1);\n \t\tstrcpy(p, cmd->ref_name);\n@@ -866,5 +888,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t}\n \tif (use_sideband)\n \t\tpacket_flush(1);\n+\tfree(invalid_delete);\n \treturn 0;\n }\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 3abb290..c0d8a0e 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -40,6 +40,39 @@ mk_test () {\n \t)\n }\n \n+mk_test_with_hooks() {\n+\tmk_test \"$@\" &&\n+\t(\n+\tcd testrepo &&\n+\tmkdir .git/hooks &&\n+\tcd .git/hooks &&\n+\n+\tcat >pre-receive <<'EOF' &&\n+#!/bin/sh\n+cat - >>pre-receive.actual\n+EOF\n+\n+\tcat >update <<'EOF' &&\n+#!/bin/sh\n+printf \"%s %s %s\\n\" \"$@\" >>update.actual\n+EOF\n+\tcat >post-receive <<'EOF' &&\n+#!/bin/sh\n+cat - >>post-receive.actual\n+EOF\n+\n+\tcat >post-update <<'EOF' &&\n+#!/bin/sh\n+for ref in \"$@\"\n+do\n+\tprintf \"%s\\n\" \"$ref\" >>post-update.actual\n+done\n+EOF\n+\n+\tchmod u+x pre-receive update post-receive post-update\n+\t)\n+}\n+\n mk_child() {\n \trm -rf \"$1\" &&\n \tgit clone testrepo \"$1\"\n@@ -559,6 +592,143 @@ test_expect_success 'allow deleting an invalid remote ref' '\n \n '\n \n+test_expect_success 'pushing valid refs triggers post-receive and post-update hooks' '\n+\tmk_test_with_hooks heads/master heads/next &&\n+\torgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&\n+\tnewmaster=$(git show-ref -s --verify refs/heads/master) &&\n+\torgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&\n+\tnewnext=$_z40 &&\n+\tgit push testrepo refs/heads/master:refs/heads/master :refs/heads/next &&\n+\t(cd testrepo/.git &&\n+\tcat >pre-receive.expect <<'EOF' &&\n+$orgmaster $newmaster refs/heads/master\n+$orgnext $newnext refs/heads/next\n+EOF\n+\n+\tcat >update.expect <<'EOF' &&\n+refs/heads/master $orgmaster $newmaster\n+refs/heads/next $orgnext $newnext\n+EOF\n+\n+\tcat >post-receive.expect <<'EOF' &&\n+$orgmaster $newmaster refs/heads/master\n+$orgnext $newnext refs/heads/next\n+EOF\n+\n+\tcat >post-update.expect <<'EOF' &&\n+refs/heads/master\n+refs/heads/next\n+EOF\n+\n+\ttest_cmp pre-receive.expect pre-receive.actual &&\n+\ttest_cmp update.expect update.actual &&\n+\ttest_cmp post-receive.expect post-receive.actual &&\n+\ttest_cmp post-update.expect post-update.actual\n+\t)\n+'\n+\n+test_expect_success 'deleting dangling ref triggers hooks with correct args' '\n+\tmk_test_with_hooks heads/master &&\n+\trm -f testrepo/.git/objects/??/* &&\n+\tgit push testrepo :refs/heads/master &&\n+\t(cd testrepo/.git &&\n+\tcat >pre-receive.expect <<'EOF' &&\n+$_z40 $_z40 refs/heads/master\n+EOF\n+\n+\tcat >update.expect <<'EOF' &&\n+refs/heads/master $_z40 $_z40\n+EOF\n+\n+\tcat >post-receive.expect <<'EOF' &&\n+$_z40 $_z40 refs/heads/master\n+EOF\n+\n+\tcat >post-update.expect <<'EOF' &&\n+refs/heads/master\n+EOF\n+\n+\ttest_cmp pre-receive.expect pre-receive.actual &&\n+\ttest_cmp update.expect update.actual &&\n+\ttest_cmp post-receive.expect post-receive.actual &&\n+\ttest_cmp post-update.expect post-update.actual\n+\t)\n+'\n+\n+test_expect_success 'deleting non-existent ref does not trigger post-receive and post-update hooks' '\n+\tmk_test_with_hooks heads/master &&\n+\torgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&\n+\tnewmaster=$(git show-ref -s --verify refs/heads/master) &&\n+\tgit push testrepo master :refs/heads/nonexistent &&\n+\t(cd testrepo/.git &&\n+\tcat >pre-receive.expect <<'EOF' &&\n+$orgmaster $newmaster refs/heads/master\n+$_z40 $_z40 refs/heads/nonexistent\n+EOF\n+\n+\tcat >update.expect <<'EOF' &&\n+refs/heads/master $orgmaster $newmaster\n+refs/heads/nonexistent $_z40 $_z40\n+EOF\n+\n+\tcat >post-receive.expect <<'EOF' &&\n+$orgmaster $newmaster refs/heads/master\n+EOF\n+\n+\tcat >post-update.expect <<'EOF' &&\n+refs/heads/master\n+EOF\n+\n+\ttest_cmp pre-receive.expect pre-receive.actual &&\n+\ttest_cmp update.expect update.actual &&\n+\ttest_cmp post-receive.expect post-receive.actual &&\n+\ttest_cmp post-update.expect post-update.actual\n+\t)\n+'\n+\n+test_expect_success 'mixed ref updates, deletes, invalid deletes trigger hooks with correct input' '\n+\tmk_test_with_hooks heads/master heads/next heads/pu &&\n+\torgmaster=$(cd testrepo && git show-ref -s --verify refs/heads/master) &&\n+\tnewmaster=$(git show-ref -s --verify refs/heads/master) &&\n+\torgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&\n+\tnewnext=$_z40 &&\n+\torgpu=$(cd testrepo && git show-ref -s --verify refs/heads/pu) &&\n+\tnewpu=$(git show-ref -s --verify refs/heads/master) &&\n+\tgit push testrepo refs/heads/master:refs/heads/master refs/heads/master:refs/heads/pu :refs/heads/next :refs/heads/nonexistent &&\n+\t(cd testrepo/.git &&\n+\tcat >pre-receive.expect <<'EOF' &&\n+$orgmaster $newmaster refs/heads/master\n+$orgnext $newnext refs/heads/next\n+$orgpu $newpu refs/heads/pu\n+$_z40 $_z40 refs/heads/nonexistent\n+EOF\n+\n+\tcat >update.expect <<'EOF' &&\n+refs/heads/master $orgmaster $newmaster\n+refs/heads/next $orgnext $newnext\n+refs/heads/pu $orgpu $newpu\n+refs/heads/nonexistent $_z40 $_z40\n+EOF\n+\n+\tcat >post-receive.expect <<'EOF' &&\n+$orgmaster $newmaster refs/heads/master\n+$orgnext $newnext refs/heads/next\n+$orgpu $newpu refs/heads/pu\n+EOF\n+\n+\tcat >post-update.expect <<'EOF' &&\n+refs/heads/master\n+refs/heads/next\n+refs/heads/pu\n+EOF\n+\n+\ttest_cmp pre-receive.expect pre-receive.actual &&\n+\ttest_cmp update.expect update.actual &&\n+\ttest_cmp post-receive.expect post-receive.actual &&\n+\ttest_cmp post-update.expect post-update.actual\n+\t)\n+'\n+\n test_expect_success 'allow deleting a ref using --delete' '\n \tmk_test heads/master &&\n \t(cd testrepo && git config receive.denyDeleteCurrent warn) &&\n-- \n1.7.7.rc3.2.g6bf07\n"},{"id":"176463","messageId":"7vsjngxphv.fsf@alter.siamese.dyndns.org","threadId":"28521","inReplyTo":"20110928153935.GA7800@myhost","subject":"Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-28T22:37:32Z","receivedAt":"2011-09-28T22:37:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pang Yan Han <pangyanhan@gmail.com> writes:\n\n> +/* For invalid refs */\n> +static struct command **invalid_delete;\n> +static size_t invalid_delete_nr;\n> +static size_t invalid_delete_alloc;\n\nDo you have to have these separately only to keep track of the corner case\nerrors?  I would have expected that it would be more natural to mark them\nby adding a single bitfield to \"struct command\".\n\n> @@ -447,6 +467,8 @@ static const char *update(struct command *cmd)\n>  \t\tif (!parse_object(old_sha1)) {\n>  \t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n>  \t\t\told_sha1 = NULL;\n> +\t\t\tif (!ref_exists((char *) name))\n> +\t\t\t\tinvalid_delete_append(cmd);\n\nThis is not an \"invalid\" delete but deleting a non-existing ref.  Perhaps\nyou would want to move the warning and optionally reword it as well, e.g.\n\n\tif (!parse_object(old_sha1)) {\n        \told_sha1 = NULL;\n                if (ref_exists(name)) {\n\t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n\t\t} else {\n\t\t\trp_warning(\"Deleting a ref that does not exist.\");\n\t\t\tcmd->did_not_exist = 1;\n\t\t}\n\t\t...\n"},{"id":"176459","messageId":"20110928230809.GA1798@myhost","threadId":"28521","inReplyTo":"7vsjngxphv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Pang Yan Han","fromEmail":"pangyanhan@gmail.com","sentAt":"2011-09-28T23:08:09Z","receivedAt":"2011-09-28T23:08:09Z","isPatch":true,"sender":{"key":"pangyanhan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/539739?v=4"},"body":"On Wed, Sep 28, 2011 at 03:37:32PM -0700, Junio C Hamano wrote:\n> Pang Yan Han <pangyanhan@gmail.com> writes:\n> \n> > +/* For invalid refs */\n> > +static struct command **invalid_delete;\n> > +static size_t invalid_delete_nr;\n> > +static size_t invalid_delete_alloc;\n> \n> Do you have to have these separately only to keep track of the corner case\n> errors?  I would have expected that it would be more natural to mark them\n> by adding a single bitfield to \"struct command\".\n\nYes they are purely for keeping track of deleting non-existent refs.\nOk I will add the bitfield to struct command.\n\n> \n> > @@ -447,6 +467,8 @@ static const char *update(struct command *cmd)\n> >  \t\tif (!parse_object(old_sha1)) {\n> >  \t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n> >  \t\t\told_sha1 = NULL;\n> > +\t\t\tif (!ref_exists((char *) name))\n> > +\t\t\t\tinvalid_delete_append(cmd);\n> \n> This is not an \"invalid\" delete but deleting a non-existing ref.  Perhaps\n> you would want to move the warning and optionally reword it as well, e.g.\n> \n> \tif (!parse_object(old_sha1)) {\n>         \told_sha1 = NULL;\n>                 if (ref_exists(name)) {\n> \t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n> \t\t} else {\n> \t\t\trp_warning(\"Deleting a ref that does not exist.\");\n> \t\t\tcmd->did_not_exist = 1;\n> \t\t}\n> \t\t...\n\nSure thing.\n\nI am unable to reply this until much later, but are the tests in this patch ok?\nIt's the first time I'm writing test cases for git.\n\nThanks for the feedback!\n"},{"id":"176458","messageId":"7voby4xo18.fsf@alter.siamese.dyndns.org","threadId":"28521","inReplyTo":"20110928153935.GA7800@myhost","subject":"Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-28T23:09:07Z","receivedAt":"2011-09-28T23:09:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is a quick fix-up on top.\n\nPoints of interest:\n\n - Get rid of an unnecessary list; just a single bitfield is sufficient;\n   while at it, make skip_update also a bitfield.\n\n - ref_exists(refname) calls resolve_ref() which wants \"const char *\",\n   and does not touch refname itself. Make it to take \"const char *\",\n   instead of casting const away in the new caller.\n\n - Properly indent test script.  Also:\n\n   - Use \"cmd <<-EOF\" to allow indenting the here text with leading tabs;\n\n   - Strip useless use of single quote around \"cmd <<'EOF'\" inside\n     test_expect_success; they were only stepping out of the sq context\n     while the third argument to test_expect_success was being parsed. A\n     construct like this:\n\n\tcat >pre-receive.expect <<'EOF'\n        $orgmaster $newmaster refs/heads/master\n        EOF\n\n     only misleads the readers as if the file is getting literal variable\n     names without substitution, which is not what is going on.\n\nI didn't fix this in this fix-up patch, but you also need to test the case\nwhere _ONLY_ deletion of a non-existent ref is requested, in which case,\npre-receive and update should be told about it, but post-receive and\npost-update should not be even run (i.e. test the absense of these\npost-*.actual files).\n\n builtin/receive-pack.c |   41 ++++++---------------\n refs.c                 |    2 +-\n refs.h                 |    2 +-\n t/t5516-fetch-push.sh  |   90 ++++++++++++++++++++++++-----------------------\n 4 files changed, 60 insertions(+), 75 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 8a0a9d2..b73f18a 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -147,32 +147,13 @@ static void write_head_info(void)\n struct command {\n \tstruct command *next;\n \tconst char *error_string;\n-\tunsigned int skip_update;\n+\tunsigned int skip_update:1,\n+\t\t     did_not_exist:1;\n \tunsigned char old_sha1[20];\n \tunsigned char new_sha1[20];\n \tchar ref_name[FLEX_ARRAY]; /* more */\n };\n \n-/* For invalid refs */\n-static struct command **invalid_delete;\n-static size_t invalid_delete_nr;\n-static size_t invalid_delete_alloc;\n-\n-static void invalid_delete_append(struct command *cmd)\n-{\n-\tALLOC_GROW(invalid_delete, invalid_delete_nr + 1, invalid_delete_alloc);\n-\tinvalid_delete[invalid_delete_nr++] = cmd;\n-}\n-\n-static int is_invalid_delete(struct command *cmd)\n-{\n-\tsize_t i;\n-\tfor (i = 0; i < invalid_delete_nr; i++)\n-\t\tif (invalid_delete[i] == cmd)\n-\t\t\treturn 1;\n-\treturn 0;\n-}\n-\n static const char pre_receive_hook[] = \"hooks/pre-receive\";\n static const char post_receive_hook[] = \"hooks/post-receive\";\n \n@@ -235,7 +216,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)\n \tint have_input = 0, code;\n \n \tfor (cmd = commands; !have_input && cmd; cmd = cmd->next) {\n-\t\tif (!cmd->error_string && !is_invalid_delete(cmd))\n+\t\tif (!cmd->error_string && !cmd->did_not_exist)\n \t\t\thave_input = 1;\n \t}\n \n@@ -268,7 +249,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)\n \t}\n \n \tfor (cmd = commands; cmd; cmd = cmd->next) {\n-\t\tif (!cmd->error_string && !is_invalid_delete(cmd)) {\n+\t\tif (!cmd->error_string && !cmd->did_not_exist) {\n \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n \t\t\t\tsha1_to_hex(cmd->old_sha1),\n \t\t\t\tsha1_to_hex(cmd->new_sha1),\n@@ -465,10 +446,13 @@ static const char *update(struct command *cmd)\n \n \tif (is_null_sha1(new_sha1)) {\n \t\tif (!parse_object(old_sha1)) {\n-\t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n \t\t\told_sha1 = NULL;\n-\t\t\tif (!ref_exists((char *) name))\n-\t\t\t\tinvalid_delete_append(cmd);\n+\t\t\tif (ref_exists(name)) {\n+\t\t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n+\t\t\t} else {\n+\t\t\t\trp_warning(\"Deleting a non-existent ref.\");\n+\t\t\t\tcmd->did_not_exist = 1;\n+\t\t\t}\n \t\t}\n \t\tif (delete_ref(namespaced_name, old_sha1, 0)) {\n \t\t\trp_error(\"failed to delete %s\", name);\n@@ -499,7 +483,7 @@ static void run_update_post_hook(struct command *commands)\n \tstruct child_process proc;\n \n \tfor (argc = 0, cmd = commands; cmd; cmd = cmd->next) {\n-\t\tif (cmd->error_string || is_invalid_delete(cmd))\n+\t\tif (cmd->error_string || cmd->did_not_exist)\n \t\t\tcontinue;\n \t\targc++;\n \t}\n@@ -510,7 +494,7 @@ static void run_update_post_hook(struct command *commands)\n \n \tfor (argc = 1, cmd = commands; cmd; cmd = cmd->next) {\n \t\tchar *p;\n-\t\tif (cmd->error_string || is_invalid_delete(cmd))\n+\t\tif (cmd->error_string || cmd->did_not_exist)\n \t\t\tcontinue;\n \t\tp = xmalloc(strlen(cmd->ref_name) + 1);\n \t\tstrcpy(p, cmd->ref_name);\n@@ -888,6 +872,5 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t}\n \tif (use_sideband)\n \t\tpacket_flush(1);\n-\tfree(invalid_delete);\n \treturn 0;\n }\ndiff --git a/refs.c b/refs.c\nindex a615043..43e9225 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1851,7 +1851,7 @@ int update_ref(const char *action, const char *refname,\n \treturn 0;\n }\n \n-int ref_exists(char *refname)\n+int ref_exists(const char *refname)\n {\n \tunsigned char sha1[20];\n \treturn !!resolve_ref(refname, sha1, 1, NULL);\ndiff --git a/refs.h b/refs.h\nindex dfb086e..4431205 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -57,7 +57,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn\n  */\n extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);\n extern void clear_extra_refs(void);\n-extern int ref_exists(char *);\n+extern int ref_exists(const char *);\n \n extern int peel_ref(const char *, unsigned char *);\n \ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex c0d8a0e..47db4b1 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -43,33 +43,34 @@ mk_test () {\n mk_test_with_hooks() {\n \tmk_test \"$@\" &&\n \t(\n-\tcd testrepo &&\n-\tmkdir .git/hooks &&\n-\tcd .git/hooks &&\n+\t\tcd testrepo &&\n+\t\tmkdir .git/hooks &&\n+\t\tcd .git/hooks &&\n \n-\tcat >pre-receive <<'EOF' &&\n-#!/bin/sh\n-cat - >>pre-receive.actual\n-EOF\n+\t\tcat >pre-receive <<-'EOF' &&\n+\t\t#!/bin/sh\n+\t\tcat - >>pre-receive.actual\n+\t\tEOF\n \n-\tcat >update <<'EOF' &&\n-#!/bin/sh\n-printf \"%s %s %s\\n\" \"$@\" >>update.actual\n-EOF\n-\tcat >post-receive <<'EOF' &&\n-#!/bin/sh\n-cat - >>post-receive.actual\n-EOF\n+\t\tcat >update <<-'EOF' &&\n+\t\t#!/bin/sh\n+\t\tprintf \"%s %s %s\\n\" \"$@\" >>update.actual\n+\t\tEOF\n \n-\tcat >post-update <<'EOF' &&\n-#!/bin/sh\n-for ref in \"$@\"\n-do\n-\tprintf \"%s\\n\" \"$ref\" >>post-update.actual\n-done\n-EOF\n+\t\tcat >post-receive <<-'EOF' &&\n+\t\t#!/bin/sh\n+\t\tcat - >>post-receive.actual\n+\t\tEOF\n \n-\tchmod u+x pre-receive update post-receive post-update\n+\t\tcat >post-update <<-'EOF' &&\n+\t\t#!/bin/sh\n+\t\tfor ref in \"$@\"\n+\t\tdo\n+\t\t\tprintf \"%s\\n\" \"$ref\" >>post-update.actual\n+\t\tdone\n+\t\tEOF\n+\n+\t\tchmod +x pre-receive update post-receive post-update\n \t)\n }\n \n@@ -599,31 +600,32 @@ test_expect_success 'pushing valid refs triggers post-receive and post-update ho\n \torgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&\n \tnewnext=$_z40 &&\n \tgit push testrepo refs/heads/master:refs/heads/master :refs/heads/next &&\n-\t(cd testrepo/.git &&\n-\tcat >pre-receive.expect <<'EOF' &&\n-$orgmaster $newmaster refs/heads/master\n-$orgnext $newnext refs/heads/next\n-EOF\n+\t(\n+\t\tcd testrepo/.git &&\n+\t\tcat >pre-receive.expect <<-EOF &&\n+\t\t$orgmaster $newmaster refs/heads/master\n+\t\t$orgnext $newnext refs/heads/next\n+\t\tEOF\n \n-\tcat >update.expect <<'EOF' &&\n-refs/heads/master $orgmaster $newmaster\n-refs/heads/next $orgnext $newnext\n-EOF\n+\t\tcat >update.expect <<-EOF &&\n+\t\trefs/heads/master $orgmaster $newmaster\n+\t\trefs/heads/next $orgnext $newnext\n+\t\tEOF\n \n-\tcat >post-receive.expect <<'EOF' &&\n-$orgmaster $newmaster refs/heads/master\n-$orgnext $newnext refs/heads/next\n-EOF\n+\t\tcat >post-receive.expect <<-EOF &&\n+\t\t$orgmaster $newmaster refs/heads/master\n+\t\t$orgnext $newnext refs/heads/next\n+\t\tEOF\n \n-\tcat >post-update.expect <<'EOF' &&\n-refs/heads/master\n-refs/heads/next\n-EOF\n+\t\tcat >post-update.expect <<-EOF &&\n+\t\trefs/heads/master\n+\t\trefs/heads/next\n+\t\tEOF\n \n-\ttest_cmp pre-receive.expect pre-receive.actual &&\n-\ttest_cmp update.expect update.actual &&\n-\ttest_cmp post-receive.expect post-receive.actual &&\n-\ttest_cmp post-update.expect post-update.actual\n+\t\ttest_cmp pre-receive.expect pre-receive.actual &&\n+\t\ttest_cmp update.expect update.actual &&\n+\t\ttest_cmp post-receive.expect post-receive.actual &&\n+\t\ttest_cmp post-update.expect post-update.actual\n \t)\n '\n \n-- \n1.7.7.rc3.4.g8d714\n"},{"id":"176460","messageId":"20110928231147.GA1932@myhost","threadId":"28521","inReplyTo":"7voby4xo18.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Pang Yan Han","fromEmail":"pangyanhan@gmail.com","sentAt":"2011-09-28T23:11:47Z","receivedAt":"2011-09-28T23:11:47Z","isPatch":true,"sender":{"key":"pangyanhan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/539739?v=4"},"body":"Sorry Junio, disregard my earlier message. I didn't see this email.\n\nOn Wed, Sep 28, 2011 at 04:09:07PM -0700, Junio C Hamano wrote:\n> Here is a quick fix-up on top.\n> \n> Points of interest:\n> \n>  - Get rid of an unnecessary list; just a single bitfield is sufficient;\n>    while at it, make skip_update also a bitfield.\n> \n>  - ref_exists(refname) calls resolve_ref() which wants \"const char *\",\n>    and does not touch refname itself. Make it to take \"const char *\",\n>    instead of casting const away in the new caller.\n> \n>  - Properly indent test script.  Also:\n> \n>    - Use \"cmd <<-EOF\" to allow indenting the here text with leading tabs;\n> \n>    - Strip useless use of single quote around \"cmd <<'EOF'\" inside\n>      test_expect_success; they were only stepping out of the sq context\n>      while the third argument to test_expect_success was being parsed. A\n>      construct like this:\n> \n> \tcat >pre-receive.expect <<'EOF'\n>         $orgmaster $newmaster refs/heads/master\n>         EOF\n> \n>      only misleads the readers as if the file is getting literal variable\n>      names without substitution, which is not what is going on.\n> \n> I didn't fix this in this fix-up patch, but you also need to test the case\n> where _ONLY_ deletion of a non-existent ref is requested, in which case,\n> pre-receive and update should be told about it, but post-receive and\n> post-update should not be even run (i.e. test the absense of these\n> post-*.actual files).\n> \n>  builtin/receive-pack.c |   41 ++++++---------------\n>  refs.c                 |    2 +-\n>  refs.h                 |    2 +-\n>  t/t5516-fetch-push.sh  |   90 ++++++++++++++++++++++++-----------------------\n>  4 files changed, 60 insertions(+), 75 deletions(-)\n> \n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index 8a0a9d2..b73f18a 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -147,32 +147,13 @@ static void write_head_info(void)\n>  struct command {\n>  \tstruct command *next;\n>  \tconst char *error_string;\n> -\tunsigned int skip_update;\n> +\tunsigned int skip_update:1,\n> +\t\t     did_not_exist:1;\n>  \tunsigned char old_sha1[20];\n>  \tunsigned char new_sha1[20];\n>  \tchar ref_name[FLEX_ARRAY]; /* more */\n>  };\n>  \n> -/* For invalid refs */\n> -static struct command **invalid_delete;\n> -static size_t invalid_delete_nr;\n> -static size_t invalid_delete_alloc;\n> -\n> -static void invalid_delete_append(struct command *cmd)\n> -{\n> -\tALLOC_GROW(invalid_delete, invalid_delete_nr + 1, invalid_delete_alloc);\n> -\tinvalid_delete[invalid_delete_nr++] = cmd;\n> -}\n> -\n> -static int is_invalid_delete(struct command *cmd)\n> -{\n> -\tsize_t i;\n> -\tfor (i = 0; i < invalid_delete_nr; i++)\n> -\t\tif (invalid_delete[i] == cmd)\n> -\t\t\treturn 1;\n> -\treturn 0;\n> -}\n> -\n>  static const char pre_receive_hook[] = \"hooks/pre-receive\";\n>  static const char post_receive_hook[] = \"hooks/post-receive\";\n>  \n> @@ -235,7 +216,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)\n>  \tint have_input = 0, code;\n>  \n>  \tfor (cmd = commands; !have_input && cmd; cmd = cmd->next) {\n> -\t\tif (!cmd->error_string && !is_invalid_delete(cmd))\n> +\t\tif (!cmd->error_string && !cmd->did_not_exist)\n>  \t\t\thave_input = 1;\n>  \t}\n>  \n> @@ -268,7 +249,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name)\n>  \t}\n>  \n>  \tfor (cmd = commands; cmd; cmd = cmd->next) {\n> -\t\tif (!cmd->error_string && !is_invalid_delete(cmd)) {\n> +\t\tif (!cmd->error_string && !cmd->did_not_exist) {\n>  \t\t\tsize_t n = snprintf(buf, sizeof(buf), \"%s %s %s\\n\",\n>  \t\t\t\tsha1_to_hex(cmd->old_sha1),\n>  \t\t\t\tsha1_to_hex(cmd->new_sha1),\n> @@ -465,10 +446,13 @@ static const char *update(struct command *cmd)\n>  \n>  \tif (is_null_sha1(new_sha1)) {\n>  \t\tif (!parse_object(old_sha1)) {\n> -\t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n>  \t\t\told_sha1 = NULL;\n> -\t\t\tif (!ref_exists((char *) name))\n> -\t\t\t\tinvalid_delete_append(cmd);\n> +\t\t\tif (ref_exists(name)) {\n> +\t\t\t\trp_warning(\"Allowing deletion of corrupt ref.\");\n> +\t\t\t} else {\n> +\t\t\t\trp_warning(\"Deleting a non-existent ref.\");\n> +\t\t\t\tcmd->did_not_exist = 1;\n> +\t\t\t}\n>  \t\t}\n>  \t\tif (delete_ref(namespaced_name, old_sha1, 0)) {\n>  \t\t\trp_error(\"failed to delete %s\", name);\n> @@ -499,7 +483,7 @@ static void run_update_post_hook(struct command *commands)\n>  \tstruct child_process proc;\n>  \n>  \tfor (argc = 0, cmd = commands; cmd; cmd = cmd->next) {\n> -\t\tif (cmd->error_string || is_invalid_delete(cmd))\n> +\t\tif (cmd->error_string || cmd->did_not_exist)\n>  \t\t\tcontinue;\n>  \t\targc++;\n>  \t}\n> @@ -510,7 +494,7 @@ static void run_update_post_hook(struct command *commands)\n>  \n>  \tfor (argc = 1, cmd = commands; cmd; cmd = cmd->next) {\n>  \t\tchar *p;\n> -\t\tif (cmd->error_string || is_invalid_delete(cmd))\n> +\t\tif (cmd->error_string || cmd->did_not_exist)\n>  \t\t\tcontinue;\n>  \t\tp = xmalloc(strlen(cmd->ref_name) + 1);\n>  \t\tstrcpy(p, cmd->ref_name);\n> @@ -888,6 +872,5 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n>  \t}\n>  \tif (use_sideband)\n>  \t\tpacket_flush(1);\n> -\tfree(invalid_delete);\n>  \treturn 0;\n>  }\n> diff --git a/refs.c b/refs.c\n> index a615043..43e9225 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1851,7 +1851,7 @@ int update_ref(const char *action, const char *refname,\n>  \treturn 0;\n>  }\n>  \n> -int ref_exists(char *refname)\n> +int ref_exists(const char *refname)\n>  {\n>  \tunsigned char sha1[20];\n>  \treturn !!resolve_ref(refname, sha1, 1, NULL);\n> diff --git a/refs.h b/refs.h\n> index dfb086e..4431205 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -57,7 +57,7 @@ extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refn\n>   */\n>  extern void add_extra_ref(const char *refname, const unsigned char *sha1, int flags);\n>  extern void clear_extra_refs(void);\n> -extern int ref_exists(char *);\n> +extern int ref_exists(const char *);\n>  \n>  extern int peel_ref(const char *, unsigned char *);\n>  \n> diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\n> index c0d8a0e..47db4b1 100755\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -43,33 +43,34 @@ mk_test () {\n>  mk_test_with_hooks() {\n>  \tmk_test \"$@\" &&\n>  \t(\n> -\tcd testrepo &&\n> -\tmkdir .git/hooks &&\n> -\tcd .git/hooks &&\n> +\t\tcd testrepo &&\n> +\t\tmkdir .git/hooks &&\n> +\t\tcd .git/hooks &&\n>  \n> -\tcat >pre-receive <<'EOF' &&\n> -#!/bin/sh\n> -cat - >>pre-receive.actual\n> -EOF\n> +\t\tcat >pre-receive <<-'EOF' &&\n> +\t\t#!/bin/sh\n> +\t\tcat - >>pre-receive.actual\n> +\t\tEOF\n>  \n> -\tcat >update <<'EOF' &&\n> -#!/bin/sh\n> -printf \"%s %s %s\\n\" \"$@\" >>update.actual\n> -EOF\n> -\tcat >post-receive <<'EOF' &&\n> -#!/bin/sh\n> -cat - >>post-receive.actual\n> -EOF\n> +\t\tcat >update <<-'EOF' &&\n> +\t\t#!/bin/sh\n> +\t\tprintf \"%s %s %s\\n\" \"$@\" >>update.actual\n> +\t\tEOF\n>  \n> -\tcat >post-update <<'EOF' &&\n> -#!/bin/sh\n> -for ref in \"$@\"\n> -do\n> -\tprintf \"%s\\n\" \"$ref\" >>post-update.actual\n> -done\n> -EOF\n> +\t\tcat >post-receive <<-'EOF' &&\n> +\t\t#!/bin/sh\n> +\t\tcat - >>post-receive.actual\n> +\t\tEOF\n>  \n> -\tchmod u+x pre-receive update post-receive post-update\n> +\t\tcat >post-update <<-'EOF' &&\n> +\t\t#!/bin/sh\n> +\t\tfor ref in \"$@\"\n> +\t\tdo\n> +\t\t\tprintf \"%s\\n\" \"$ref\" >>post-update.actual\n> +\t\tdone\n> +\t\tEOF\n> +\n> +\t\tchmod +x pre-receive update post-receive post-update\n>  \t)\n>  }\n>  \n> @@ -599,31 +600,32 @@ test_expect_success 'pushing valid refs triggers post-receive and post-update ho\n>  \torgnext=$(cd testrepo && git show-ref -s --verify refs/heads/next) &&\n>  \tnewnext=$_z40 &&\n>  \tgit push testrepo refs/heads/master:refs/heads/master :refs/heads/next &&\n> -\t(cd testrepo/.git &&\n> -\tcat >pre-receive.expect <<'EOF' &&\n> -$orgmaster $newmaster refs/heads/master\n> -$orgnext $newnext refs/heads/next\n> -EOF\n> +\t(\n> +\t\tcd testrepo/.git &&\n> +\t\tcat >pre-receive.expect <<-EOF &&\n> +\t\t$orgmaster $newmaster refs/heads/master\n> +\t\t$orgnext $newnext refs/heads/next\n> +\t\tEOF\n>  \n> -\tcat >update.expect <<'EOF' &&\n> -refs/heads/master $orgmaster $newmaster\n> -refs/heads/next $orgnext $newnext\n> -EOF\n> +\t\tcat >update.expect <<-EOF &&\n> +\t\trefs/heads/master $orgmaster $newmaster\n> +\t\trefs/heads/next $orgnext $newnext\n> +\t\tEOF\n>  \n> -\tcat >post-receive.expect <<'EOF' &&\n> -$orgmaster $newmaster refs/heads/master\n> -$orgnext $newnext refs/heads/next\n> -EOF\n> +\t\tcat >post-receive.expect <<-EOF &&\n> +\t\t$orgmaster $newmaster refs/heads/master\n> +\t\t$orgnext $newnext refs/heads/next\n> +\t\tEOF\n>  \n> -\tcat >post-update.expect <<'EOF' &&\n> -refs/heads/master\n> -refs/heads/next\n> -EOF\n> +\t\tcat >post-update.expect <<-EOF &&\n> +\t\trefs/heads/master\n> +\t\trefs/heads/next\n> +\t\tEOF\n>  \n> -\ttest_cmp pre-receive.expect pre-receive.actual &&\n> -\ttest_cmp update.expect update.actual &&\n> -\ttest_cmp post-receive.expect post-receive.actual &&\n> -\ttest_cmp post-update.expect post-update.actual\n> +\t\ttest_cmp pre-receive.expect pre-receive.actual &&\n> +\t\ttest_cmp update.expect update.actual &&\n> +\t\ttest_cmp post-receive.expect post-receive.actual &&\n> +\t\ttest_cmp post-update.expect post-update.actual\n>  \t)\n>  '\n>  \n> -- \n> 1.7.7.rc3.4.g8d714\n> \n"},{"id":"176462","messageId":"7vk48sxn5g.fsf@alter.siamese.dyndns.org","threadId":"28521","inReplyTo":"20110928230809.GA1798@myhost","subject":"Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-28T23:28:11Z","receivedAt":"2011-09-28T23:28:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pang Yan Han <pangyanhan@gmail.com> writes:\n\n> I am unable to reply this until much later, but are the tests in this patch ok?\n> It's the first time I'm writing test cases for git.\n\nI'll squash in a bit more updates and later publish the result.\nThanks.\n"},{"id":"176570","messageId":"20110930132903.GA1622@myhost","threadId":"28521","inReplyTo":"7vk48sxn5g.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Pang Yan Han","fromEmail":"pangyanhan@gmail.com","sentAt":"2011-09-30T13:29:03Z","receivedAt":"2011-09-30T13:29:03Z","isPatch":true,"sender":{"key":"pangyanhan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/539739?v=4"},"body":"On Wed, Sep 28, 2011 at 04:28:11PM -0700, Junio C Hamano wrote:\n> Pang Yan Han <pangyanhan@gmail.com> writes:\n> \n> > I am unable to reply this until much later, but are the tests in this patch ok?\n> > It's the first time I'm writing test cases for git.\n> \n> I'll squash in a bit more updates and later publish the result.\n> Thanks.\n\nHi Junio,\n\nThank you for correcting my many amateur mistakes!\n\nSorry for asking, but do I need to reroll this with the fixup in\norigin/ph/push-to-delete-nothing ? Is the commit message fine especially\nin light of the changes in the fixup?\n\nThanks.\n"},{"id":"176585","messageId":"7v1uuxq4er.fsf@alter.siamese.dyndns.org","threadId":"28521","inReplyTo":"20110930132903.GA1622@myhost","subject":"Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-30T18:19:24Z","receivedAt":"2011-09-30T18:19:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pang Yan Han <pangyanhan@gmail.com> writes:\n\n> Sorry for asking, but do I need to reroll this with the fixup in\n> origin/ph/push-to-delete-nothing ? Is the commit message fine especially\n> in light of the changes in the fixup?\n\nIf you think that the result of squashing the fix-up commit into your\npatch looks OK, I do not think there is a need to reroll. The patch title\nmay need to be shortened, but other than that I do not see anything\nglaringly wrong in the commit log message that I cannot amend out.\n\nThanks.\n"}]}