{"thread":{"id":"36110","subject":"[PATCH 00/26] Clean up update-refs --stdin and implement ref_transaction","startedAt":"2014-03-10T12:46:17Z","lastAt":"2014-03-20T17:01:05Z","messageCount":38,"participants":["Michael Haggerty","Johan Herland","Brad King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":26},"messages":[{"id":"236375","messageId":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":null,"subject":"[PATCH 00/26] Clean up update-refs --stdin and implement ref_transaction","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:17Z","receivedAt":"2014-03-10T12:46:17Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"I just sent an email to the list [1] describing how I want to\ndecouple reference-handling code from the rest of Git, and implement\npluggable reference storage backends. This patch series is the first\nmovement in that direction.\n\nupdate_refs() and \"update-ref --stdin\" implement the beginning of\ntransactions for git references, by allowing a group of reference\nchanges to be done in an all-or-nothing fashion. The main point of\nthis patch series is to increase the abstraction level of the API for\ndealing with reference transactions, by moving the handling of the\ntransaction to refs.c. The new API for dealing with reference\ntransactions is\n\n ref_transaction *transaction = create_ref_transaction();\n queue_create_ref(transaction, refname, new_sha1, ...);\n queue_update_ref(transaction, refname, new_sha1, old_sha1, ...);\n queue_delete_ref(transaction, refname, old_sha1, ...);\n ...\n if (commit_ref_transaction(transaction, msg, ...))\n die(...);\n\nWhen implementing this I found a number of minor problems in the\nimplementation of \"git update-ref --stdin\", not to mention that it\nused \"struct ref_update\" all the way up and down its parser call\nstack. So most of the commits in this series are actually cleanups in\nbuiltin/update-ref.c. I also spend some time making the error\nmessages emitted by that command more uniform.\n\nThen, in just a couple of commits, the ref_transaction abstraction is\nintroduced, update-ref is changed to use it, and update_refs() is\nremoved from the refs API (it was only used by this one caller).\n\nFinally, now that refs.c owns the data structures for dealing with\ntransactions, it is possible to make a few simplifications. More\nchanges in this neighborhood will be coming in future patches.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/243726\n\nMichael Haggerty (26):\n t1400: Fix name and expected result of one test\n t1400: Provide sensible input to the command\n t1400: Pass a legitimate to update command\n parse_arg(): Really test that argument is properly terminated\n t1400: Add some more tests involving quoted arguments\n refs.h: Rename the action_on_err constants\n update_refs(): Fix constness\n update-ref --stdin: Read the whole input at once\n parse_cmd_verify(): Copy old_sha1 instead of evaluating \n twice\n update-ref.c: Extract a new function, parse_refname()\n update-ref --stdin: Improve error messages for invalid values\n update-ref --stdin: Make error messages more consistent\n update-ref --stdin: Simplify error messages for missing oldvalues\n update-ref.c: Extract a new function, parse_next_sha1()\n update-ref --stdin: Improve the error message for unexpected EOF\n update-ref --stdin: Harmonize error messages\n refs: Add a concept of a reference transaction\n update-ref --stdin: Reimplement using reference transactions\n refs: Remove API function update_refs()\n struct ref_update: Rename field \"ref_name\" to \"refname\"\n struct ref_update: Store refname as a FLEX_ARRAY.\n commit_ref_transaction(): Introduce temporary variables\n struct ref_update: Add a lock member\n struct ref_update: Add type field\n commit_ref_transaction(): Also free the ref_transaction\n commit_ref_transaction(): Work with transaction->updates in place\n\n builtin/checkout.c | 2 +-\n builtin/clone.c | 9 +-\n builtin/merge.c | 6 +-\n builtin/notes.c | 6 +-\n builtin/reset.c | 6 +-\n builtin/update-ref.c | 402 +++++++++++++++++++--------------\n contrib/examples/builtin-fetch--tool.c | 3 +-\n notes-cache.c | 2 +-\n notes-utils.c | 3 +-\n refs.c | 184 +++++++++++----\n refs.h | 93 ++++++--\n t/t1400-update-ref.sh | 86 ++++---\n 12 files changed, 524 insertions(+), 278 deletions(-)\n\n-- \n1.9.0\n"},{"id":"236376","messageId":"1394455603-2968-2-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 01/26] t1400: Fix name and expected result of one test","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:18Z","receivedAt":"2014-03-10T12:46:18Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The test\n\n stdin -z create ref fails with zero new value\n\nactually passes an empty new value, not a zero new value. So rename\nthe test s/zero/empty/, and change the expected error from\n\n fatal: create $c given zero new value\n\nto\n\n fatal: create $c missing \n\nOf course, this makes the test fail now, so mark it\ntest_expect_failure. The failure will be fixed later in this patch\nseries.\n\nSigned-off-by: Michael Haggerty \n---\n t/t1400-update-ref.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 6ffd82f..fa927d2 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -827,10 +827,10 @@ test_expect_success 'stdin -z create ref fails with bad new value' '\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n-test_expect_success 'stdin -z create ref fails with zero new value' '\n+test_expect_failure 'stdin -z create ref fails with empty new value' '\n \tprintf $F \"create $c\" \"\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: create $c given zero new value\" err &&\n+\tgrep \"fatal: create $c missing \" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n-- \n1.9.0\n"},{"id":"236377","messageId":"1394455603-2968-3-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 02/26] t1400: Provide sensible input to the command","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:19Z","receivedAt":"2014-03-10T12:46:19Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The old version was passing (among other things)\n\n update SP refs/heads/c NUL NUL 0{40} NUL\n\nto \"git update-ref -z --stdin\" to test whether the old-value check for\nc is working. But the is empty, which is not allowed for\nthe \"update\" command.\n\nSo, to be sure that we are testing what we want to test, provide a\nlegitimate on the \"update\" line.\n\nSigned-off-by: Michael Haggerty \n---\n t/t1400-update-ref.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex fa927d2..29391c6 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -912,7 +912,7 @@ test_expect_success 'stdin -z update refs works with identity updates' '\n \n test_expect_success 'stdin -z update refs fails with wrong old value' '\n \tgit update-ref $c $m &&\n-\tprintf $F \"update $a\" \"$m\" \"$m\" \"update $b\" \"$m\" \"$m\" \"update $c\" \"\" \"$Z\" >stdin &&\n+\tprintf $F \"update $a\" \"$m\" \"$m\" \"update $b\" \"$m\" \"$m\" \"update $c\" \"$m\" \"$Z\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n \tgrep \"fatal: Cannot lock the ref '\"'\"'$c'\"'\"'\" err &&\n \tgit rev-parse $m >expect &&\n-- \n1.9.0\n"},{"id":"236398","messageId":"1394455603-2968-4-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 03/26] t1400: Pass a legitimate to update command","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:20Z","receivedAt":"2014-03-10T12:46:20Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This test is trying to test a few ways to delete references using \"git\nupdate-ref -z --stdin\". The third line passed in is\n\n update SP /refs/heads/c NUL NUL NUL\n\n, which is not a correct way to delete a reference according to the\ndocumentation (the new value should be zeros, not empty). Pass zeros\ninstead as the new value to test the code correctly.\n\nSigned-off-by: Michael Haggerty \n---\n t/t1400-update-ref.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 29391c6..e2f1dfa 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -927,7 +927,7 @@ test_expect_success 'stdin -z update refs fails with wrong old value' '\n test_expect_success 'stdin -z delete refs works with packed and loose refs' '\n \tgit pack-refs --all &&\n \tgit update-ref $c $m~1 &&\n-\tprintf $F \"delete $a\" \"$m\" \"update $b\" \"$Z\" \"$m\" \"update $c\" \"\" \"$m~1\" >stdin &&\n+\tprintf $F \"delete $a\" \"$m\" \"update $b\" \"$Z\" \"$m\" \"update $c\" \"$Z\" \"$m~1\" >stdin &&\n \tgit update-ref -z --stdin \n---\n builtin/update-ref.c | 20 +++++++++++++++-----\n t/t1400-update-ref.sh | 4 ++--\n 2 files changed, 17 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 1292cfe..02b5f95 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -62,16 +62,26 @@ static void update_store_old_sha1(struct ref_update *update,\n \tupdate->have_old = *oldvalue || line_termination;\n }\n \n+/*\n+ * Parse one whitespace- or NUL-terminated, possibly C-quoted argument\n+ * and append the result to arg. Return a pointer to the terminator.\n+ * Die if there is an error in how the argument is C-quoted. This\n+ * function is only used if not -z.\n+ */\n static const char *parse_arg(const char *next, struct strbuf *arg)\n {\n-\t/* Parse SP-terminated, possibly C-quoted argument */\n-\tif (*next != '\"')\n+\tif (*next == '\"') {\n+\t\tconst char *orig = next;\n+\n+\t\tif (unquote_c_style(arg, next, &next))\n+\t\t\tdie(\"badly quoted argument: %s\", orig);\n+\t\tif (*next && !isspace(*next))\n+\t\t\tdie(\"unexpected character after quoted argument: %s\", orig);\n+\t} else {\n \t\twhile (*next && !isspace(*next))\n \t\t\tstrbuf_addch(arg, *next++);\n-\telse if (unquote_c_style(arg, next, &next))\n-\t\tdie(\"badly quoted argument: %s\", next);\n+\t}\n \n-\t/* Return position after the argument */\n \treturn next;\n }\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex e2f1dfa..5836842 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -356,10 +356,10 @@ test_expect_success 'stdin fails on badly quoted input' '\n \tgrep \"fatal: badly quoted argument: \\\\\\\"master\" err\n '\n \n-test_expect_success 'stdin fails on arguments not separated by space' '\n+test_expect_success 'stdin fails on junk after quoted argument' '\n \techo \"create \\\"$a\\\"master\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: expected SP but got: master\" err\n+\tgrep \"fatal: unexpected character after quoted argument: \\\\\\\"$a\\\\\\\"master\" err\n '\n \n test_expect_success 'stdin fails create with no ref' '\n-- \n1.9.0\n"},{"id":"236399","messageId":"1394455603-2968-6-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 05/26] t1400: Add some more tests involving quoted arguments","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:22Z","receivedAt":"2014-03-10T12:46:22Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Previously there were no good tests of C-quoted arguments.\n\nSigned-off-by: Michael Haggerty \n---\n t/t1400-update-ref.sh | 26 +++++++++++++++++++++++++-\n 1 file changed, 25 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 5836842..627aaaf 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -350,12 +350,18 @@ test_expect_success 'stdin fails on unknown command' '\n \tgrep \"fatal: unknown command: unknown $a\" err\n '\n \n-test_expect_success 'stdin fails on badly quoted input' '\n+test_expect_success 'stdin fails on unbalanced quotes' '\n \techo \"create $a \\\"master\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n \tgrep \"fatal: badly quoted argument: \\\\\\\"master\" err\n '\n \n+test_expect_success 'stdin fails on invalid escape' '\n+\techo \"create $a \\\"ma\\zter\\\"\" >stdin &&\n+\ttest_must_fail git update-ref --stdin err &&\n+\tgrep \"fatal: badly quoted argument: \\\\\\\"ma\\\\\\\\zter\\\\\\\"\" err\n+'\n+\n test_expect_success 'stdin fails on junk after quoted argument' '\n \techo \"create \\\"$a\\\"master\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n@@ -458,6 +464,24 @@ test_expect_success 'stdin create ref works' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'stdin succeeds with quoted argument' '\n+\tgit update-ref -d $a &&\n+\techo \"create $a \\\"$m\\\"\" >stdin &&\n+\tgit update-ref --stdin expect &&\n+\tgit rev-parse $a >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'stdin succeeds with escaped character' '\n+\tgit update-ref -d $a &&\n+\techo \"create $a \\\"ma\\\\163ter\\\"\" >stdin &&\n+\tgit update-ref --stdin expect &&\n+\tgit rev-parse $a >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'stdin update ref creates with zero old value' '\n \techo \"update $b $m $Z\" >stdin &&\n \tgit update-ref --stdin \n---\n builtin/checkout.c | 2 +-\n builtin/clone.c | 9 +++++----\n builtin/merge.c | 6 +++---\n builtin/notes.c | 6 +++---\n builtin/reset.c | 6 ++++--\n builtin/update-ref.c | 5 +++--\n contrib/examples/builtin-fetch--tool.c | 3 ++-\n notes-cache.c | 2 +-\n notes-utils.c | 3 ++-\n refs.c | 18 +++++++++---------\n refs.h | 9 +++++++--\n 11 files changed, 40 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex ada51fa..f79b222 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -624,7 +624,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,\n \t\t/* Nothing to do. */\n \t} else if (opts->force_detach || !new->path) {\t/* No longer on any branch. */\n \t\tupdate_ref(msg.buf, \"HEAD\", new->commit->object.sha1, NULL,\n-\t\t\t REF_NODEREF, DIE_ON_ERR);\n+\t\t\t REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n \t\tif (!opts->quiet) {\n \t\t\tif (old->path && advice_detached_head)\n \t\t\t\tdetach_advice(new->name);\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 43e772c..af3b86f 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -521,7 +521,7 @@ static void write_followtags(const struct ref *refs, const char *msg)\n \t\tif (!has_sha1_file(ref->old_sha1))\n \t\t\tcontinue;\n \t\tupdate_ref(msg, ref->name, ref->old_sha1,\n-\t\t\t NULL, 0, DIE_ON_ERR);\n+\t\t\t NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n \t}\n }\n \n@@ -589,14 +589,15 @@ static void update_head(const struct ref *our, const struct ref *remote,\n \t\tcreate_symref(\"HEAD\", our->name, NULL);\n \t\tif (!option_bare) {\n \t\t\tconst char *head = skip_prefix(our->name, \"refs/heads/\");\n-\t\t\tupdate_ref(msg, \"HEAD\", our->old_sha1, NULL, 0, DIE_ON_ERR);\n+\t\t\tupdate_ref(msg, \"HEAD\", our->old_sha1, NULL, 0,\n+\t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n \t\t\tinstall_branch_config(0, head, option_origin, our->name);\n \t\t}\n \t} else if (our) {\n \t\tstruct commit *c = lookup_commit_reference(our->old_sha1);\n \t\t/* --branch specifies a non-branch (i.e. tags), detach HEAD */\n \t\tupdate_ref(msg, \"HEAD\", c->object.sha1,\n-\t\t\t NULL, REF_NODEREF, DIE_ON_ERR);\n+\t\t\t NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n \t} else if (remote) {\n \t\t/*\n \t\t * We know remote HEAD points to a non-branch, or\n@@ -604,7 +605,7 @@ static void update_head(const struct ref *our, const struct ref *remote,\n \t\t * Detach HEAD in all these cases.\n \t\t */\n \t\tupdate_ref(msg, \"HEAD\", remote->old_sha1,\n-\t\t\t NULL, REF_NODEREF, DIE_ON_ERR);\n+\t\t\t NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);\n \t}\n }\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex f0cf120..a4c3b17 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -398,7 +398,7 @@ static void finish(struct commit *head_commit,\n \t\t\tconst char *argv_gc_auto[] = { \"gc\", \"--auto\", NULL };\n \t\t\tupdate_ref(reflog_message.buf, \"HEAD\",\n \t\t\t\tnew_head, head, 0,\n-\t\t\t\tDIE_ON_ERR);\n+\t\t\t\tUPDATE_REFS_DIE_ON_ERR);\n \t\t\t/*\n \t\t\t * We ignore errors in 'gc --auto', since the\n \t\t\t * user should see them.\n@@ -1222,7 +1222,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"%s - not something we can merge\"), argv[0]);\n \t\tread_empty(remote_head->object.sha1, 0);\n \t\tupdate_ref(\"initial pull\", \"HEAD\", remote_head->object.sha1,\n-\t\t\t NULL, 0, DIE_ON_ERR);\n+\t\t\t NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n \t\tgoto done;\n \t} else {\n \t\tstruct strbuf merge_names = STRBUF_INIT;\n@@ -1339,7 +1339,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t}\n \n \tupdate_ref(\"updating ORIG_HEAD\", \"ORIG_HEAD\", head_commit->object.sha1,\n-\t\t NULL, 0, DIE_ON_ERR);\n+\t\t NULL, 0, UPDATE_REFS_DIE_ON_ERR);\n \n \tif (remoteheads && !common)\n \t\t; /* No common ancestors found. We need a real merge. */\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 2b24d05..5e11a3e 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -713,7 +713,7 @@ static int merge_commit(struct notes_merge_options *o)\n \tstrbuf_insert(&msg, 0, \"notes: \", 7);\n \tupdate_ref(msg.buf, o->local_ref, sha1,\n \t\t is_null_sha1(parent_sha1) ? NULL : parent_sha1,\n-\t\t 0, DIE_ON_ERR);\n+\t\t 0, UPDATE_REFS_DIE_ON_ERR);\n \n \tfree_notes(t);\n \tstrbuf_release(&msg);\n@@ -808,11 +808,11 @@ static int merge(int argc, const char **argv, const char *prefix)\n \tif (result >= 0) /* Merge resulted (trivially) in result_sha1 */\n \t\t/* Update default notes ref with new commit */\n \t\tupdate_ref(msg.buf, default_notes_ref(), result_sha1, NULL,\n-\t\t\t 0, DIE_ON_ERR);\n+\t\t\t 0, UPDATE_REFS_DIE_ON_ERR);\n \telse { /* Merge has unresolved conflicts */\n \t\t/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */\n \t\tupdate_ref(msg.buf, \"NOTES_MERGE_PARTIAL\", result_sha1, NULL,\n-\t\t\t 0, DIE_ON_ERR);\n+\t\t\t 0, UPDATE_REFS_DIE_ON_ERR);\n \t\t/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */\n \t\tif (create_symref(\"NOTES_MERGE_REF\", default_notes_ref(), NULL))\n \t\t\tdie(\"Failed to store link to current notes ref (%s)\",\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4fd1c6c..15a96aa 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -252,11 +252,13 @@ static int reset_refs(const char *rev, const unsigned char *sha1)\n \tif (!get_sha1(\"HEAD\", sha1_orig)) {\n \t\torig = sha1_orig;\n \t\tset_reflog_message(&msg, \"updating ORIG_HEAD\", NULL);\n-\t\tupdate_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0, MSG_ON_ERR);\n+\t\tupdate_ref(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0,\n+\t\t\t UPDATE_REFS_MSG_ON_ERR);\n \t} else if (old_orig)\n \t\tdelete_ref(\"ORIG_HEAD\", old_orig, 0);\n \tset_reflog_message(&msg, \"updating HEAD\", rev);\n-\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0, MSG_ON_ERR);\n+\tupdate_ref_status = update_ref(msg.buf, \"HEAD\", sha1, orig, 0,\n+\t\t\t\t UPDATE_REFS_MSG_ON_ERR);\n \tstrbuf_release(&msg);\n \treturn update_ref_status;\n }\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 02b5f95..f6345e5 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -282,7 +282,8 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\treturn update_refs(msg, updates, updates_count, DIE_ON_ERR);\n+\t\treturn update_refs(msg, updates, updates_count,\n+\t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n \t}\n \n \tif (end_null)\n@@ -314,5 +315,5 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\treturn delete_ref(refname, oldval ? oldsha1 : NULL, flags);\n \telse\n \t\treturn update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n-\t\t\t\t flags, DIE_ON_ERR);\n+\t\t\t\t flags, UPDATE_REFS_DIE_ON_ERR);\n }\ndiff --git a/contrib/examples/builtin-fetch--tool.c b/contrib/examples/builtin-fetch--tool.c\nindex 8bc8c75..ee19166 100644\n--- a/contrib/examples/builtin-fetch--tool.c\n+++ b/contrib/examples/builtin-fetch--tool.c\n@@ -31,7 +31,8 @@ static int update_ref_env(const char *action,\n \t\trla = \"(reflog update)\";\n \tif (snprintf(msg, sizeof(msg), \"%s: %s\", rla, action) >= sizeof(msg))\n \t\twarning(\"reflog message too long: %.*s...\", 50, msg);\n-\treturn update_ref(msg, refname, sha1, oldval, 0, QUIET_ON_ERR);\n+\treturn update_ref(msg, refname, sha1, oldval, 0,\n+\t\t\t UPDATE_REFS_QUIET_ON_ERR);\n }\n \n static int update_local_ref(const char *name,\ndiff --git a/notes-cache.c b/notes-cache.c\nindex eabe4a0..97dfd63 100644\n--- a/notes-cache.c\n+++ b/notes-cache.c\n@@ -62,7 +62,7 @@ int notes_cache_write(struct notes_cache *c)\n \tif (commit_tree(&msg, tree_sha1, NULL, commit_sha1, NULL, NULL) < 0)\n \t\treturn -1;\n \tif (update_ref(\"update notes cache\", c->tree.ref, commit_sha1, NULL,\n-\t\t 0, QUIET_ON_ERR) < 0)\n+\t\t 0, UPDATE_REFS_QUIET_ON_ERR) < 0)\n \t\treturn -1;\n \n \treturn 0;\ndiff --git a/notes-utils.c b/notes-utils.c\nindex 2975dcd..e559642 100644\n--- a/notes-utils.c\n+++ b/notes-utils.c\n@@ -48,7 +48,8 @@ void commit_notes(struct notes_tree *t, const char *msg)\n \n \tcreate_notes_commit(t, NULL, &buf, commit_sha1);\n \tstrbuf_insert(&buf, 0, \"notes: \", 7); /* commit message starts at index 7 */\n-\tupdate_ref(buf.buf, t->ref, commit_sha1, NULL, 0, DIE_ON_ERR);\n+\tupdate_ref(buf.buf, t->ref, commit_sha1, NULL, 0,\n+\t\t UPDATE_REFS_DIE_ON_ERR);\n \n \tstrbuf_release(&buf);\n }\ndiff --git a/refs.c b/refs.c\nindex 89228e2..58faf95 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3243,9 +3243,9 @@ static struct ref_lock *update_ref_lock(const char *refname,\n \tif (!lock) {\n \t\tconst char *str = \"Cannot lock the ref '%s'.\";\n \t\tswitch (onerr) {\n-\t\tcase MSG_ON_ERR: error(str, refname); break;\n-\t\tcase DIE_ON_ERR: die(str, refname); break;\n-\t\tcase QUIET_ON_ERR: break;\n+\t\tcase UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;\n+\t\tcase UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;\n+\t\tcase UPDATE_REFS_QUIET_ON_ERR: break;\n \t\t}\n \t}\n \treturn lock;\n@@ -3258,9 +3258,9 @@ static int update_ref_write(const char *action, const char *refname,\n \tif (write_ref_sha1(lock, sha1, action) < 0) {\n \t\tconst char *str = \"Cannot update the ref '%s'.\";\n \t\tswitch (onerr) {\n-\t\tcase MSG_ON_ERR: error(str, refname); break;\n-\t\tcase DIE_ON_ERR: die(str, refname); break;\n-\t\tcase QUIET_ON_ERR: break;\n+\t\tcase UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;\n+\t\tcase UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;\n+\t\tcase UPDATE_REFS_QUIET_ON_ERR: break;\n \t\t}\n \t\treturn 1;\n \t}\n@@ -3294,11 +3294,11 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \t\t\tconst char *str =\n \t\t\t\t\"Multiple updates for ref '%s' not allowed.\";\n \t\t\tswitch (onerr) {\n-\t\t\tcase MSG_ON_ERR:\n+\t\t\tcase UPDATE_REFS_MSG_ON_ERR:\n \t\t\t\terror(str, updates[i]->ref_name); break;\n-\t\t\tcase DIE_ON_ERR:\n+\t\t\tcase UPDATE_REFS_DIE_ON_ERR:\n \t\t\t\tdie(str, updates[i]->ref_name); break;\n-\t\t\tcase QUIET_ON_ERR:\n+\t\t\tcase UPDATE_REFS_QUIET_ON_ERR:\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\treturn 1;\ndiff --git a/refs.h b/refs.h\nindex 87a1a79..a713b34 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -214,8 +214,13 @@ extern int rename_ref(const char *oldref, const char *newref, const char *logmsg\n */\n extern int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sha1);\n \n-/** lock a ref and then write its file */\n-enum action_on_err { MSG_ON_ERR, DIE_ON_ERR, QUIET_ON_ERR };\n+enum action_on_err {\n+\tUPDATE_REFS_MSG_ON_ERR,\n+\tUPDATE_REFS_DIE_ON_ERR,\n+\tUPDATE_REFS_QUIET_ON_ERR\n+};\n+\n+/** Lock a ref and then write its file */\n int update_ref(const char *action, const char *refname,\n \t\tconst unsigned char *sha1, const unsigned char *oldval,\n \t\tint flags, enum action_on_err onerr);\n-- \n1.9.0\n"},{"id":"236397","messageId":"1394455603-2968-8-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 07/26] update_refs(): Fix constness","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:24Z","receivedAt":"2014-03-10T12:46:24Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Since full const correctness is beyond the ability of C's type system,\njust put the const where it doesn't do any harm. A (struct ref_update\n**) can be passed to a (struct ref_update * const *) argument, but not\nto a (const struct ref_update **) argument.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 2 +-\n refs.c | 2 +-\n refs.h | 2 +-\n 3 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex f6345e5..a8a68e8 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -14,7 +14,7 @@ static const char * const git_update_ref_usage[] = {\n \n static int updates_alloc;\n static int updates_count;\n-static const struct ref_update **updates;\n+static struct ref_update **updates;\n \n static char line_termination = '\\n';\n static int update_flags;\ndiff --git a/refs.c b/refs.c\nindex 58faf95..0963077 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3306,7 +3306,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \treturn 0;\n }\n \n-int update_refs(const char *action, const struct ref_update **updates_orig,\n+int update_refs(const char *action, struct ref_update * const *updates_orig,\n \t\tint n, enum action_on_err onerr)\n {\n \tint ret = 0, delnum = 0, i;\ndiff --git a/refs.h b/refs.h\nindex a713b34..08e60ac 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -228,7 +228,7 @@ int update_ref(const char *action, const char *refname,\n /**\n * Lock all refs and then perform all modifications.\n */\n-int update_refs(const char *action, const struct ref_update **updates,\n+int update_refs(const char *action, struct ref_update * const *updates,\n \t\tint n, enum action_on_err onerr);\n \n extern int parse_hide_refs_config(const char *var, const char *value, const char *);\n-- \n1.9.0\n"},{"id":"236379","messageId":"1394455603-2968-9-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 08/26] update-ref --stdin: Read the whole input at once","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:25Z","receivedAt":"2014-03-10T12:46:25Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Decouple the parsing code from the input source (the old parsing code\nhad to read new data even in the middle of commands). This might also\nbe a tad faster, but that is inconsequential. Add docstrings for the\nparsing functions.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 170 ++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 108 insertions(+), 62 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex a8a68e8..5f197fe 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -85,44 +85,70 @@ static const char *parse_arg(const char *next, struct strbuf *arg)\n \treturn next;\n }\n \n-static const char *parse_first_arg(const char *next, struct strbuf *arg)\n+/*\n+ * Parse the argument immediately after \"command SP\". If not -z, then\n+ * handle C-quoting. Write the argument to arg. Set *next to point\n+ * at the character that terminates the argument. Die if C-quoting is\n+ * malformed.\n+ */\n+static void parse_first_arg(struct strbuf *input, const char **next,\n+\t\t\t struct strbuf *arg)\n {\n-\t/* Parse argument immediately after \"command SP\" */\n \tstrbuf_reset(arg);\n \tif (line_termination) {\n \t\t/* Without -z, use the next argument */\n-\t\tnext = parse_arg(next, arg);\n+\t\t*next = parse_arg(*next, arg);\n \t} else {\n-\t\t/* With -z, use rest of first NUL-terminated line */\n-\t\tstrbuf_addstr(arg, next);\n-\t\tnext = next + arg->len;\n+\t\t/* With -z, use everything up to the next NUL */\n+\t\tstrbuf_addstr(arg, *next);\n+\t\t*next += arg->len;\n \t}\n-\treturn next;\n }\n \n-static const char *parse_next_arg(const char *next, struct strbuf *arg)\n+/*\n+ * Parse a SP/NUL separator followed by the next SP- or NUL-terminated\n+ * argument, if any. If there is an argument, write it to arg, set\n+ * *next to point at the character terminating the argument, and\n+ * return 0. If there is no argument at all (not even the empty\n+ * string), return a non-zero result and leave *next unchanged.\n+ */\n+static int parse_next_arg(struct strbuf *input, const char **next,\n+\t\t\t struct strbuf *arg)\n {\n-\t/* Parse next SP-terminated or NUL-terminated argument, if any */\n \tstrbuf_reset(arg);\n \tif (line_termination) {\n \t\t/* Without -z, consume SP and use next argument */\n-\t\tif (!*next)\n-\t\t\treturn NULL;\n-\t\tif (*next != ' ')\n-\t\t\tdie(\"expected SP but got: %s\", next);\n-\t\tnext = parse_arg(next + 1, arg);\n+\t\tif (!**next || **next == line_termination)\n+\t\t\treturn -1;\n+\t\tif (**next != ' ')\n+\t\t\tdie(\"expected SP but got: %s\", *next);\n+\t\t(*next)++;\n+\t\t*next = parse_arg(*next, arg);\n \t} else {\n \t\t/* With -z, read the next NUL-terminated line */\n-\t\tif (*next)\n-\t\t\tdie(\"expected NUL but got: %s\", next);\n-\t\tif (strbuf_getline(arg, stdin, '\\0') == EOF)\n-\t\t\treturn NULL;\n-\t\tnext = arg->buf + arg->len;\n+\t\tif (**next)\n+\t\t\tdie(\"expected NUL but got: %s\", *next);\n+\t\t(*next)++;\n+\t\tif (*next == input->buf + input->len)\n+\t\t\treturn -1;\n+\t\tstrbuf_addstr(arg, *next);\n+\t\t*next += arg->len;\n \t}\n-\treturn next;\n+\treturn 0;\n }\n \n-static void parse_cmd_update(const char *next)\n+\n+/*\n+ * The following five parse_cmd_*() functions parse the corresponding\n+ * command. In each case, next points at the character following the\n+ * command name and the following space. They each return a pointer\n+ * to the character terminating the command, and die with an\n+ * explanatory message if there are any parsing problems. All of\n+ * these functions handle either text or binary format input,\n+ * depending on how line_termination is set.\n+ */\n+\n+static const char *parse_cmd_update(struct strbuf *input, const char *next)\n {\n \tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf newvalue = STRBUF_INIT;\n@@ -131,26 +157,28 @@ static void parse_cmd_update(const char *next)\n \n \tupdate = update_alloc();\n \n-\tif ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])\n+\tparse_first_arg(input, &next, &ref);\n+\tif (ref.buf[0])\n \t\tupdate_store_ref_name(update, ref.buf);\n \telse\n \t\tdie(\"update line missing \");\n \n-\tif ((next = parse_next_arg(next, &newvalue)) != NULL)\n+\tif (!parse_next_arg(input, &next, &newvalue))\n \t\tupdate_store_new_sha1(update, newvalue.buf);\n \telse\n \t\tdie(\"update %s missing \", ref.buf);\n \n-\tif ((next = parse_next_arg(next, &oldvalue)) != NULL)\n+\tif (!parse_next_arg(input, &next, &oldvalue)) {\n \t\tupdate_store_old_sha1(update, oldvalue.buf);\n-\telse if(!line_termination)\n+\t\tif (*next != line_termination)\n+\t\t\tdie(\"update %s has extra input: %s\", ref.buf, next);\n+\t} else if (!line_termination)\n \t\tdie(\"update %s missing [] NUL\", ref.buf);\n \n-\tif (next && *next)\n-\t\tdie(\"update %s has extra input: %s\", ref.buf, next);\n+\treturn next;\n }\n \n-static void parse_cmd_create(const char *next)\n+static const char *parse_cmd_create(struct strbuf *input, const char *next)\n {\n \tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf newvalue = STRBUF_INIT;\n@@ -158,23 +186,27 @@ static void parse_cmd_create(const char *next)\n \n \tupdate = update_alloc();\n \n-\tif ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])\n+\tparse_first_arg(input, &next, &ref);\n+\tif (ref.buf[0])\n \t\tupdate_store_ref_name(update, ref.buf);\n \telse\n \t\tdie(\"create line missing \");\n \n-\tif ((next = parse_next_arg(next, &newvalue)) != NULL)\n+\tif (!parse_next_arg(input, &next, &newvalue))\n \t\tupdate_store_new_sha1(update, newvalue.buf);\n \telse\n \t\tdie(\"create %s missing \", ref.buf);\n+\n \tif (is_null_sha1(update->new_sha1))\n \t\tdie(\"create %s given zero new value\", ref.buf);\n \n-\tif (next && *next)\n+\tif (*next != line_termination)\n \t\tdie(\"create %s has extra input: %s\", ref.buf, next);\n+\n+\treturn next;\n }\n \n-static void parse_cmd_delete(const char *next)\n+static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n {\n \tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf oldvalue = STRBUF_INIT;\n@@ -182,23 +214,26 @@ static void parse_cmd_delete(const char *next)\n \n \tupdate = update_alloc();\n \n-\tif ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])\n+\tparse_first_arg(input, &next, &ref);\n+\tif (ref.buf[0])\n \t\tupdate_store_ref_name(update, ref.buf);\n \telse\n \t\tdie(\"delete line missing \");\n \n-\tif ((next = parse_next_arg(next, &oldvalue)) != NULL)\n+\tif (!parse_next_arg(input, &next, &oldvalue)) {\n \t\tupdate_store_old_sha1(update, oldvalue.buf);\n-\telse if(!line_termination)\n+\t\tif (update->have_old && is_null_sha1(update->old_sha1))\n+\t\t\tdie(\"delete %s given zero old value\", ref.buf);\n+\t} else if (!line_termination)\n \t\tdie(\"delete %s missing [] NUL\", ref.buf);\n-\tif (update->have_old && is_null_sha1(update->old_sha1))\n-\t\tdie(\"delete %s given zero old value\", ref.buf);\n \n-\tif (next && *next)\n+\tif (*next != line_termination)\n \t\tdie(\"delete %s has extra input: %s\", ref.buf, next);\n+\n+\treturn next;\n }\n \n-static void parse_cmd_verify(const char *next)\n+static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n {\n \tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf value = STRBUF_INIT;\n@@ -206,53 +241,64 @@ static void parse_cmd_verify(const char *next)\n \n \tupdate = update_alloc();\n \n-\tif ((next = parse_first_arg(next, &ref)) != NULL && ref.buf[0])\n+\tparse_first_arg(input, &next, &ref);\n+\tif (ref.buf[0])\n \t\tupdate_store_ref_name(update, ref.buf);\n \telse\n \t\tdie(\"verify line missing \");\n \n-\tif ((next = parse_next_arg(next, &value)) != NULL) {\n+\tif (!parse_next_arg(input, &next, &value)) {\n \t\tupdate_store_old_sha1(update, value.buf);\n \t\tupdate_store_new_sha1(update, value.buf);\n-\t} else if(!line_termination)\n+\t} else if (!line_termination)\n \t\tdie(\"verify %s missing [] NUL\", ref.buf);\n \n-\tif (next && *next)\n+\tif (*next != line_termination)\n \t\tdie(\"verify %s has extra input: %s\", ref.buf, next);\n+\n+\treturn next;\n }\n \n-static void parse_cmd_option(const char *next)\n+static const char *parse_cmd_option(struct strbuf *input, const char *next)\n {\n-\tif (!strcmp(next, \"no-deref\"))\n+\tif (!strncmp(next, \"no-deref\", 8) && next[8] == line_termination)\n \t\tupdate_flags |= REF_NODEREF;\n \telse\n \t\tdie(\"option unknown: %s\", next);\n+\treturn next + 8;\n }\n \n static void update_refs_stdin(void)\n {\n-\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct strbuf input = STRBUF_INIT;\n+\tconst char *next;\n \n+\tif (strbuf_read(&input, 0, 1000) < 0)\n+\t\tdie_errno(\"could not read from stdin\");\n+\tnext = input.buf;\n \t/* Read each line dispatch its command */\n-\twhile (strbuf_getline(&cmd, stdin, line_termination) != EOF)\n-\t\tif (!cmd.buf[0])\n+\twhile (next < input.buf + input.len) {\n+\t\tif (*next == line_termination)\n \t\t\tdie(\"empty command in input\");\n-\t\telse if (isspace(*cmd.buf))\n-\t\t\tdie(\"whitespace before command: %s\", cmd.buf);\n-\t\telse if (starts_with(cmd.buf, \"update \"))\n-\t\t\tparse_cmd_update(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"create \"))\n-\t\t\tparse_cmd_create(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"delete \"))\n-\t\t\tparse_cmd_delete(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"verify \"))\n-\t\t\tparse_cmd_verify(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"option \"))\n-\t\t\tparse_cmd_option(cmd.buf + 7);\n+\t\telse if (isspace(*next))\n+\t\t\tdie(\"whitespace before command: %s\", next);\n+\t\telse if (starts_with(next, \"update \"))\n+\t\t\tnext = parse_cmd_update(&input, next + 7);\n+\t\telse if (starts_with(next, \"create \"))\n+\t\t\tnext = parse_cmd_create(&input, next + 7);\n+\t\telse if (starts_with(next, \"delete \"))\n+\t\t\tnext = parse_cmd_delete(&input, next + 7);\n+\t\telse if (starts_with(next, \"verify \"))\n+\t\t\tnext = parse_cmd_verify(&input, next + 7);\n+\t\telse if (starts_with(next, \"option \"))\n+\t\t\tnext = parse_cmd_option(&input, next + 7);\n \t\telse\n-\t\t\tdie(\"unknown command: %s\", cmd.buf);\n+\t\t\tdie(\"unknown command: %s\", next);\n+\n+\t\tnext++;\n+\t}\n \n-\tstrbuf_release(&cmd);\n+\tstrbuf_release(&input);\n }\n \n int cmd_update_ref(int argc, const char **argv, const char *prefix)\n-- \n1.9.0\n"},{"id":"236395","messageId":"1394455603-2968-10-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 09/26] parse_cmd_verify(): Copy old_sha1 instead of evaluating twice","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:26Z","receivedAt":"2014-03-10T12:46:26Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Aside from avoiding work, this makes it transparently obvious that\nold_sha1 and new_sha1 are identical.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 5f197fe..51adf2d 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -249,7 +249,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \n \tif (!parse_next_arg(input, &next, &value)) {\n \t\tupdate_store_old_sha1(update, value.buf);\n-\t\tupdate_store_new_sha1(update, value.buf);\n+\t\thashcpy(update->new_sha1, update->old_sha1);\n \t} else if (!line_termination)\n \t\tdie(\"verify %s missing [] NUL\", ref.buf);\n \n-- \n1.9.0\n"},{"id":"236394","messageId":"1394455603-2968-11-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 10/26] update-ref.c: Extract a new function, parse_refname()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:27Z","receivedAt":"2014-03-10T12:46:27Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"There is no reason to obscure the fact that parse_first_arg() always\nparses refnames. Form the new function by combining parse_first_arg()\nand update_store_ref_name().\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 90 ++++++++++++++++++++++++----------------------------\n 1 file changed, 41 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 51adf2d..0dc2061 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -35,14 +35,6 @@ static struct ref_update *update_alloc(void)\n \treturn update;\n }\n \n-static void update_store_ref_name(struct ref_update *update,\n-\t\t\t\t const char *ref_name)\n-{\n-\tif (check_refname_format(ref_name, REFNAME_ALLOW_ONELEVEL))\n-\t\tdie(\"invalid ref format: %s\", ref_name);\n-\tupdate->ref_name = xstrdup(ref_name);\n-}\n-\n static void update_store_new_sha1(struct ref_update *update,\n \t\t\t\t const char *newvalue)\n {\n@@ -86,23 +78,35 @@ static const char *parse_arg(const char *next, struct strbuf *arg)\n }\n \n /*\n- * Parse the argument immediately after \"command SP\". If not -z, then\n- * handle C-quoting. Write the argument to arg. Set *next to point\n- * at the character that terminates the argument. Die if C-quoting is\n- * malformed.\n+ * Parse the reference name immediately after \"command SP\". If not\n+ * -z, then handle C-quoting. Return a pointer to a newly allocated\n+ * string containing the name of the reference, or NULL if there was\n+ * an error. Update *next to point at the character that terminates\n+ * the argument. Die if C-quoting is malformed or the reference name\n+ * is invalid.\n */\n-static void parse_first_arg(struct strbuf *input, const char **next,\n-\t\t\t struct strbuf *arg)\n+static char *parse_refname(struct strbuf *input, const char **next)\n {\n-\tstrbuf_reset(arg);\n+\tstruct strbuf ref = STRBUF_INIT;\n+\n \tif (line_termination) {\n \t\t/* Without -z, use the next argument */\n-\t\t*next = parse_arg(*next, arg);\n+\t\t*next = parse_arg(*next, &ref);\n \t} else {\n \t\t/* With -z, use everything up to the next NUL */\n-\t\tstrbuf_addstr(arg, *next);\n-\t\t*next += arg->len;\n+\t\tstrbuf_addstr(&ref, *next);\n+\t\t*next += ref.len;\n+\t}\n+\n+\tif (!ref.len) {\n+\t\tstrbuf_release(&ref);\n+\t\treturn NULL;\n \t}\n+\n+\tif (check_refname_format(ref.buf, REFNAME_ALLOW_ONELEVEL))\n+\t\tdie(\"invalid ref format: %s\", ref.buf);\n+\n+\treturn strbuf_detach(&ref, NULL);\n }\n \n /*\n@@ -150,111 +154,99 @@ static int parse_next_arg(struct strbuf *input, const char **next,\n \n static const char *parse_cmd_update(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf newvalue = STRBUF_INIT;\n \tstruct strbuf oldvalue = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n \n-\tparse_first_arg(input, &next, &ref);\n-\tif (ref.buf[0])\n-\t\tupdate_store_ref_name(update, ref.buf);\n-\telse\n+\tupdate->ref_name = parse_refname(input, &next);\n+\tif (!update->ref_name)\n \t\tdie(\"update line missing \");\n \n \tif (!parse_next_arg(input, &next, &newvalue))\n \t\tupdate_store_new_sha1(update, newvalue.buf);\n \telse\n-\t\tdie(\"update %s missing \", ref.buf);\n+\t\tdie(\"update %s missing \", update->ref_name);\n \n \tif (!parse_next_arg(input, &next, &oldvalue)) {\n \t\tupdate_store_old_sha1(update, oldvalue.buf);\n \t\tif (*next != line_termination)\n-\t\t\tdie(\"update %s has extra input: %s\", ref.buf, next);\n+\t\t\tdie(\"update %s has extra input: %s\", update->ref_name, next);\n \t} else if (!line_termination)\n-\t\tdie(\"update %s missing [] NUL\", ref.buf);\n+\t\tdie(\"update %s missing [] NUL\", update->ref_name);\n \n \treturn next;\n }\n \n static const char *parse_cmd_create(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf newvalue = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n \n-\tparse_first_arg(input, &next, &ref);\n-\tif (ref.buf[0])\n-\t\tupdate_store_ref_name(update, ref.buf);\n-\telse\n+\tupdate->ref_name = parse_refname(input, &next);\n+\tif (!update->ref_name)\n \t\tdie(\"create line missing \");\n \n \tif (!parse_next_arg(input, &next, &newvalue))\n \t\tupdate_store_new_sha1(update, newvalue.buf);\n \telse\n-\t\tdie(\"create %s missing \", ref.buf);\n+\t\tdie(\"create %s missing \", update->ref_name);\n \n \tif (is_null_sha1(update->new_sha1))\n-\t\tdie(\"create %s given zero new value\", ref.buf);\n+\t\tdie(\"create %s given zero new value\", update->ref_name);\n \n \tif (*next != line_termination)\n-\t\tdie(\"create %s has extra input: %s\", ref.buf, next);\n+\t\tdie(\"create %s has extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\n \n static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf oldvalue = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n \n-\tparse_first_arg(input, &next, &ref);\n-\tif (ref.buf[0])\n-\t\tupdate_store_ref_name(update, ref.buf);\n-\telse\n+\tupdate->ref_name = parse_refname(input, &next);\n+\tif (!update->ref_name)\n \t\tdie(\"delete line missing \");\n \n \tif (!parse_next_arg(input, &next, &oldvalue)) {\n \t\tupdate_store_old_sha1(update, oldvalue.buf);\n \t\tif (update->have_old && is_null_sha1(update->old_sha1))\n-\t\t\tdie(\"delete %s given zero old value\", ref.buf);\n+\t\t\tdie(\"delete %s given zero old value\", update->ref_name);\n \t} else if (!line_termination)\n-\t\tdie(\"delete %s missing [] NUL\", ref.buf);\n+\t\tdie(\"delete %s missing [] NUL\", update->ref_name);\n \n \tif (*next != line_termination)\n-\t\tdie(\"delete %s has extra input: %s\", ref.buf, next);\n+\t\tdie(\"delete %s has extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\n \n static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf ref = STRBUF_INIT;\n \tstruct strbuf value = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n \n-\tparse_first_arg(input, &next, &ref);\n-\tif (ref.buf[0])\n-\t\tupdate_store_ref_name(update, ref.buf);\n-\telse\n+\tupdate->ref_name = parse_refname(input, &next);\n+\tif (!update->ref_name)\n \t\tdie(\"verify line missing \");\n \n \tif (!parse_next_arg(input, &next, &value)) {\n \t\tupdate_store_old_sha1(update, value.buf);\n \t\thashcpy(update->new_sha1, update->old_sha1);\n \t} else if (!line_termination)\n-\t\tdie(\"verify %s missing [] NUL\", ref.buf);\n+\t\tdie(\"verify %s missing [] NUL\", update->ref_name);\n \n \tif (*next != line_termination)\n-\t\tdie(\"verify %s has extra input: %s\", ref.buf, next);\n+\t\tdie(\"verify %s has extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\n-- \n1.9.0\n"},{"id":"236381","messageId":"1394455603-2968-12-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 11/26] update-ref --stdin: Improve error messages for invalid values","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:28Z","receivedAt":"2014-03-10T12:46:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"If an invalid value is passed to \"update-ref --stdin\" as or\n, include the command and the name of the reference at the\nbeginning of the error message. Update the tests accordingly.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 24 +++++++++++++-----------\n t/t1400-update-ref.sh | 8 ++++----\n 2 files changed, 17 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 0dc2061..13a884a 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -35,20 +35,22 @@ static struct ref_update *update_alloc(void)\n \treturn update;\n }\n \n-static void update_store_new_sha1(struct ref_update *update,\n+static void update_store_new_sha1(const char *command,\n+\t\t\t\t struct ref_update *update,\n \t\t\t\t const char *newvalue)\n {\n \tif (*newvalue && get_sha1(newvalue, update->new_sha1))\n-\t\tdie(\"invalid new value for ref %s: %s\",\n-\t\t update->ref_name, newvalue);\n+\t\tdie(\"%s %s: invalid new value: %s\",\n+\t\t command, update->ref_name, newvalue);\n }\n \n-static void update_store_old_sha1(struct ref_update *update,\n+static void update_store_old_sha1(const char *command,\n+\t\t\t\t struct ref_update *update,\n \t\t\t\t const char *oldvalue)\n {\n \tif (*oldvalue && get_sha1(oldvalue, update->old_sha1))\n-\t\tdie(\"invalid old value for ref %s: %s\",\n-\t\t update->ref_name, oldvalue);\n+\t\tdie(\"%s %s: invalid old value: %s\",\n+\t\t command, update->ref_name, oldvalue);\n \n \t/* We have an old value if non-empty, or if empty without -z */\n \tupdate->have_old = *oldvalue || line_termination;\n@@ -165,12 +167,12 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \t\tdie(\"update line missing \");\n \n \tif (!parse_next_arg(input, &next, &newvalue))\n-\t\tupdate_store_new_sha1(update, newvalue.buf);\n+\t\tupdate_store_new_sha1(\"update\", update, newvalue.buf);\n \telse\n \t\tdie(\"update %s missing \", update->ref_name);\n \n \tif (!parse_next_arg(input, &next, &oldvalue)) {\n-\t\tupdate_store_old_sha1(update, oldvalue.buf);\n+\t\tupdate_store_old_sha1(\"update\", update, oldvalue.buf);\n \t\tif (*next != line_termination)\n \t\t\tdie(\"update %s has extra input: %s\", update->ref_name, next);\n \t} else if (!line_termination)\n@@ -191,7 +193,7 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n \t\tdie(\"create line missing \");\n \n \tif (!parse_next_arg(input, &next, &newvalue))\n-\t\tupdate_store_new_sha1(update, newvalue.buf);\n+\t\tupdate_store_new_sha1(\"create\", update, newvalue.buf);\n \telse\n \t\tdie(\"create %s missing \", update->ref_name);\n \n@@ -216,7 +218,7 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \t\tdie(\"delete line missing \");\n \n \tif (!parse_next_arg(input, &next, &oldvalue)) {\n-\t\tupdate_store_old_sha1(update, oldvalue.buf);\n+\t\tupdate_store_old_sha1(\"delete\", update, oldvalue.buf);\n \t\tif (update->have_old && is_null_sha1(update->old_sha1))\n \t\t\tdie(\"delete %s given zero old value\", update->ref_name);\n \t} else if (!line_termination)\n@@ -240,7 +242,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \t\tdie(\"verify line missing \");\n \n \tif (!parse_next_arg(input, &next, &value)) {\n-\t\tupdate_store_old_sha1(update, value.buf);\n+\t\tupdate_store_old_sha1(\"verify\", update, value.buf);\n \t\thashcpy(update->new_sha1, update->old_sha1);\n \t} else if (!line_termination)\n \t\tdie(\"verify %s missing [] NUL\", update->ref_name);\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 627aaaf..c5be870 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -518,14 +518,14 @@ test_expect_success 'stdin update ref fails with wrong old value' '\n test_expect_success 'stdin update ref fails with bad old value' '\n \techo \"update $c $m does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: invalid old value for ref $c: does-not-exist\" err &&\n+\tgrep \"fatal: update $c: invalid old value: does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n test_expect_success 'stdin create ref fails with bad new value' '\n \techo \"create $c does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: invalid new value for ref $c: does-not-exist\" err &&\n+\tgrep \"fatal: create $c: invalid new value: does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n@@ -840,14 +840,14 @@ test_expect_success 'stdin -z update ref fails with wrong old value' '\n test_expect_success 'stdin -z update ref fails with bad old value' '\n \tprintf $F \"update $c\" \"$m\" \"does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: invalid old value for ref $c: does-not-exist\" err &&\n+\tgrep \"fatal: update $c: invalid old value: does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n test_expect_success 'stdin -z create ref fails with bad new value' '\n \tprintf $F \"create $c\" \"does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: invalid new value for ref $c: does-not-exist\" err &&\n+\tgrep \"fatal: create $c: invalid new value: does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n-- \n1.9.0\n"},{"id":"236396","messageId":"1394455603-2968-13-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 12/26] update-ref --stdin: Make error messages more consistent","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:29Z","receivedAt":"2014-03-10T12:46:29Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The old error messages emitted for invalid input sometimes said\n\"\"/\"\" and sometimes said \"old value\"/\"new value\".\nConvert them all to the former. Update the tests accordingly.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 8 ++++----\n t/t1400-update-ref.sh | 14 +++++++-------\n 2 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 13a884a..e4c0854 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -40,7 +40,7 @@ static void update_store_new_sha1(const char *command,\n \t\t\t\t const char *newvalue)\n {\n \tif (*newvalue && get_sha1(newvalue, update->new_sha1))\n-\t\tdie(\"%s %s: invalid new value: %s\",\n+\t\tdie(\"%s %s: invalid : %s\",\n \t\t command, update->ref_name, newvalue);\n }\n \n@@ -49,7 +49,7 @@ static void update_store_old_sha1(const char *command,\n \t\t\t\t const char *oldvalue)\n {\n \tif (*oldvalue && get_sha1(oldvalue, update->old_sha1))\n-\t\tdie(\"%s %s: invalid old value: %s\",\n+\t\tdie(\"%s %s: invalid : %s\",\n \t\t command, update->ref_name, oldvalue);\n \n \t/* We have an old value if non-empty, or if empty without -z */\n@@ -198,7 +198,7 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n \t\tdie(\"create %s missing \", update->ref_name);\n \n \tif (is_null_sha1(update->new_sha1))\n-\t\tdie(\"create %s given zero new value\", update->ref_name);\n+\t\tdie(\"create %s given zero \", update->ref_name);\n \n \tif (*next != line_termination)\n \t\tdie(\"create %s has extra input: %s\", update->ref_name, next);\n@@ -220,7 +220,7 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \tif (!parse_next_arg(input, &next, &oldvalue)) {\n \t\tupdate_store_old_sha1(\"delete\", update, oldvalue.buf);\n \t\tif (update->have_old && is_null_sha1(update->old_sha1))\n-\t\t\tdie(\"delete %s given zero old value\", update->ref_name);\n+\t\t\tdie(\"delete %s given zero \", update->ref_name);\n \t} else if (!line_termination)\n \t\tdie(\"delete %s missing [] NUL\", update->ref_name);\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex c5be870..3045ae7 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -518,21 +518,21 @@ test_expect_success 'stdin update ref fails with wrong old value' '\n test_expect_success 'stdin update ref fails with bad old value' '\n \techo \"update $c $m does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: update $c: invalid old value: does-not-exist\" err &&\n+\tgrep \"fatal: update $c: invalid : does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n test_expect_success 'stdin create ref fails with bad new value' '\n \techo \"create $c does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: create $c: invalid new value: does-not-exist\" err &&\n+\tgrep \"fatal: create $c: invalid : does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n test_expect_success 'stdin create ref fails with zero new value' '\n \techo \"create $c \" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: create $c given zero new value\" err &&\n+\tgrep \"fatal: create $c given zero \" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n@@ -556,7 +556,7 @@ test_expect_success 'stdin delete ref fails with wrong old value' '\n test_expect_success 'stdin delete ref fails with zero old value' '\n \techo \"delete $a \" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: delete $a given zero old value\" err &&\n+\tgrep \"fatal: delete $a given zero \" err &&\n \tgit rev-parse $m >expect &&\n \tgit rev-parse $a >actual &&\n \ttest_cmp expect actual\n@@ -840,14 +840,14 @@ test_expect_success 'stdin -z update ref fails with wrong old value' '\n test_expect_success 'stdin -z update ref fails with bad old value' '\n \tprintf $F \"update $c\" \"$m\" \"does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: update $c: invalid old value: does-not-exist\" err &&\n+\tgrep \"fatal: update $c: invalid : does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n test_expect_success 'stdin -z create ref fails with bad new value' '\n \tprintf $F \"create $c\" \"does-not-exist\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: create $c: invalid new value: does-not-exist\" err &&\n+\tgrep \"fatal: create $c: invalid : does-not-exist\" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n@@ -878,7 +878,7 @@ test_expect_success 'stdin -z delete ref fails with wrong old value' '\n test_expect_success 'stdin -z delete ref fails with zero old value' '\n \tprintf $F \"delete $a\" \"$Z\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: delete $a given zero old value\" err &&\n+\tgrep \"fatal: delete $a given zero \" err &&\n \tgit rev-parse $m >expect &&\n \tgit rev-parse $a >actual &&\n \ttest_cmp expect actual\n-- \n1.9.0\n"},{"id":"236382","messageId":"1394455603-2968-14-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 13/26] update-ref --stdin: Simplify error messages for missing oldvalues","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:30Z","receivedAt":"2014-03-10T12:46:30Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Instead of, for example,\n\n fatal: update refs/heads/master missing [] NUL\n\nemit\n\n fatal: update refs/heads/master missing \n\nUpdate the tests accordingly.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 6 +++---\n t/t1400-update-ref.sh | 6 +++---\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex e4c0854..a9eb5fe 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -176,7 +176,7 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \t\tif (*next != line_termination)\n \t\t\tdie(\"update %s has extra input: %s\", update->ref_name, next);\n \t} else if (!line_termination)\n-\t\tdie(\"update %s missing [] NUL\", update->ref_name);\n+\t\tdie(\"update %s missing \", update->ref_name);\n \n \treturn next;\n }\n@@ -222,7 +222,7 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \t\tif (update->have_old && is_null_sha1(update->old_sha1))\n \t\t\tdie(\"delete %s given zero \", update->ref_name);\n \t} else if (!line_termination)\n-\t\tdie(\"delete %s missing [] NUL\", update->ref_name);\n+\t\tdie(\"delete %s missing \", update->ref_name);\n \n \tif (*next != line_termination)\n \t\tdie(\"delete %s has extra input: %s\", update->ref_name, next);\n@@ -245,7 +245,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \t\tupdate_store_old_sha1(\"verify\", update, value.buf);\n \t\thashcpy(update->new_sha1, update->old_sha1);\n \t} else if (!line_termination)\n-\t\tdie(\"verify %s missing [] NUL\", update->ref_name);\n+\t\tdie(\"verify %s missing \", update->ref_name);\n \n \tif (*next != line_termination)\n \t\tdie(\"verify %s has extra input: %s\", update->ref_name, next);\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 3045ae7..42fec4e 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -739,7 +739,7 @@ test_expect_success 'stdin -z fails update with no new value' '\n test_expect_success 'stdin -z fails update with no old value' '\n \tprintf $F \"update $a\" \"$m\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: update $a missing \\\\[\\\\] NUL\" err\n+\tgrep \"fatal: update $a missing \" err\n '\n \n test_expect_success 'stdin -z fails update with too many arguments' '\n@@ -763,7 +763,7 @@ test_expect_success 'stdin -z fails delete with bad ref name' '\n test_expect_success 'stdin -z fails delete with no old value' '\n \tprintf $F \"delete $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: delete $a missing \\\\[\\\\] NUL\" err\n+\tgrep \"fatal: delete $a missing \" err\n '\n \n test_expect_success 'stdin -z fails delete with too many arguments' '\n@@ -781,7 +781,7 @@ test_expect_success 'stdin -z fails verify with too many arguments' '\n test_expect_success 'stdin -z fails verify with no old value' '\n \tprintf $F \"verify $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: verify $a missing \\\\[\\\\] NUL\" err\n+\tgrep \"fatal: verify $a missing \" err\n '\n \n test_expect_success 'stdin -z fails option with unknown name' '\n-- \n1.9.0\n"},{"id":"236393","messageId":"1394455603-2968-15-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 14/26] update-ref.c: Extract a new function, parse_next_sha1()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:31Z","receivedAt":"2014-03-10T12:46:31Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Replace three functions, update_store_new_sha1(),\nupdate_store_old_sha1(), and parse_next_arg(), with a single function,\nparse_next_sha1(). The new function takes care of a whole argument,\nincluding checking whether it is there, converting it to an SHA-1, and\nemitting errors on EOF or for invalid values. The return value\nindicates whether the argument was present or absent, which requires\na bit of intelligence because absent values are represented\ndifferently depending on whether \"-z\" was used.\n\nThe new interface means that the calling functions, parse_cmd_*(),\ndon't have to interpret the result differently based on the\nline_termination mode that is in effect. It also means that\nparse_cmd_create() can distinguish unambiguously between an empty new\nvalue and a zeros new value, which fixes a failure in t1400.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 138 +++++++++++++++++++++++++++-----------------------\n t/t1400-update-ref.sh | 2 +-\n 2 files changed, 77 insertions(+), 63 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex a9eb5fe..5937291 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -35,27 +35,6 @@ static struct ref_update *update_alloc(void)\n \treturn update;\n }\n \n-static void update_store_new_sha1(const char *command,\n-\t\t\t\t struct ref_update *update,\n-\t\t\t\t const char *newvalue)\n-{\n-\tif (*newvalue && get_sha1(newvalue, update->new_sha1))\n-\t\tdie(\"%s %s: invalid : %s\",\n-\t\t command, update->ref_name, newvalue);\n-}\n-\n-static void update_store_old_sha1(const char *command,\n-\t\t\t\t struct ref_update *update,\n-\t\t\t\t const char *oldvalue)\n-{\n-\tif (*oldvalue && get_sha1(oldvalue, update->old_sha1))\n-\t\tdie(\"%s %s: invalid : %s\",\n-\t\t command, update->ref_name, oldvalue);\n-\n-\t/* We have an old value if non-empty, or if empty without -z */\n-\tupdate->have_old = *oldvalue || line_termination;\n-}\n-\n /*\n * Parse one whitespace- or NUL-terminated, possibly C-quoted argument\n * and append the result to arg. Return a pointer to the terminator.\n@@ -112,35 +91,74 @@ static char *parse_refname(struct strbuf *input, const char **next)\n }\n \n /*\n- * Parse a SP/NUL separator followed by the next SP- or NUL-terminated\n- * argument, if any. If there is an argument, write it to arg, set\n- * *next to point at the character terminating the argument, and\n+ * Parse an argument separator followed by the next argument, if any.\n+ * If there is an argument, convert it to a SHA-1, write it to sha1,\n+ * set *next to point at the character terminating the argument, and\n * return 0. If there is no argument at all (not even the empty\n- * string), return a non-zero result and leave *next unchanged.\n+ * string), return 1 and leave *next unchanged. If the value is\n+ * provided but cannot be converted to a SHA-1, die.\n */\n-static int parse_next_arg(struct strbuf *input, const char **next,\n-\t\t\t struct strbuf *arg)\n+static int parse_next_sha1(struct strbuf *input, const char **next,\n+\t\t\t unsigned char *sha1,\n+\t\t\t const char *command, const char *refname, int old)\n {\n-\tstrbuf_reset(arg);\n+\tstruct strbuf arg = STRBUF_INIT;\n+\tint ret = 0;\n+\n+\tif (*next == input->buf + input->len)\n+\t\tgoto eof;\n+\n \tif (line_termination) {\n \t\t/* Without -z, consume SP and use next argument */\n \t\tif (!**next || **next == line_termination)\n-\t\t\treturn -1;\n+\t\t\treturn 1;\n \t\tif (**next != ' ')\n-\t\t\tdie(\"expected SP but got: %s\", *next);\n+\t\t\tdie(\"%s %s: expected SP but got: %s\",\n+\t\t\t command, refname, *next);\n \t\t(*next)++;\n-\t\t*next = parse_arg(*next, arg);\n+\t\t*next = parse_arg(*next, &arg);\n+\t\tif (arg.len) {\n+\t\t\tif (get_sha1(arg.buf, sha1))\n+\t\t\t\tgoto invalid;\n+\t\t} else {\n+\t\t\t/* Without -z, an empty value means all zeros: */\n+\t\t\thashclr(sha1);\n+\t\t}\n \t} else {\n \t\t/* With -z, read the next NUL-terminated line */\n \t\tif (**next)\n-\t\t\tdie(\"expected NUL but got: %s\", *next);\n+\t\t\tdie(\"%s %s: expected NUL but got: %s\",\n+\t\t\t command, refname, *next);\n \t\t(*next)++;\n \t\tif (*next == input->buf + input->len)\n-\t\t\treturn -1;\n-\t\tstrbuf_addstr(arg, *next);\n-\t\t*next += arg->len;\n+\t\t\tgoto eof;\n+\t\tstrbuf_addstr(&arg, *next);\n+\t\t*next += arg.len;\n+\n+\t\tif (arg.len) {\n+\t\t\tif (get_sha1(arg.buf, sha1))\n+\t\t\t\tgoto invalid;\n+\t\t} else {\n+\t\t\t/* With -z, an empty value means unspecified: */\n+\t\t\tret = 1;\n+\t\t}\n \t}\n-\treturn 0;\n+\n+\tstrbuf_release(&arg);\n+\n+\treturn ret;\n+\n+ invalid:\n+\tdie(old ?\n+\t \"%s %s: invalid : %s\" :\n+\t \"%s %s: invalid : %s\",\n+\t command, refname, arg.buf);\n+\n+ eof:\n+\tdie(old ?\n+\t \"%s %s missing \" :\n+\t \"%s %s missing \",\n+\t command, refname);\n }\n \n \n@@ -156,8 +174,6 @@ static int parse_next_arg(struct strbuf *input, const char **next,\n \n static const char *parse_cmd_update(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf newvalue = STRBUF_INIT;\n-\tstruct strbuf oldvalue = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n@@ -166,24 +182,21 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \tif (!update->ref_name)\n \t\tdie(\"update line missing \");\n \n-\tif (!parse_next_arg(input, &next, &newvalue))\n-\t\tupdate_store_new_sha1(\"update\", update, newvalue.buf);\n-\telse\n+\tif (parse_next_sha1(input, &next, update->new_sha1,\n+\t\t\t \"update\", update->ref_name, 0))\n \t\tdie(\"update %s missing \", update->ref_name);\n \n-\tif (!parse_next_arg(input, &next, &oldvalue)) {\n-\t\tupdate_store_old_sha1(\"update\", update, oldvalue.buf);\n-\t\tif (*next != line_termination)\n-\t\t\tdie(\"update %s has extra input: %s\", update->ref_name, next);\n-\t} else if (!line_termination)\n-\t\tdie(\"update %s missing \", update->ref_name);\n+\tupdate->have_old = !parse_next_sha1(input, &next, update->old_sha1,\n+\t\t\t\t\t \"update\", update->ref_name, 1);\n+\n+\tif (*next != line_termination)\n+\t\tdie(\"update %s has extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\n \n static const char *parse_cmd_create(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf newvalue = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n@@ -192,9 +205,8 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n \tif (!update->ref_name)\n \t\tdie(\"create line missing \");\n \n-\tif (!parse_next_arg(input, &next, &newvalue))\n-\t\tupdate_store_new_sha1(\"create\", update, newvalue.buf);\n-\telse\n+\tif (parse_next_sha1(input, &next, update->new_sha1,\n+\t\t\t \"create\", update->ref_name, 0))\n \t\tdie(\"create %s missing \", update->ref_name);\n \n \tif (is_null_sha1(update->new_sha1))\n@@ -208,7 +220,6 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n \n static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf oldvalue = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n@@ -217,12 +228,14 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \tif (!update->ref_name)\n \t\tdie(\"delete line missing \");\n \n-\tif (!parse_next_arg(input, &next, &oldvalue)) {\n-\t\tupdate_store_old_sha1(\"delete\", update, oldvalue.buf);\n-\t\tif (update->have_old && is_null_sha1(update->old_sha1))\n+\tif (parse_next_sha1(input, &next, update->old_sha1,\n+\t\t\t \"delete\", update->ref_name, 1)) {\n+\t\tupdate->have_old = 0;\n+\t} else {\n+\t\tif (is_null_sha1(update->old_sha1))\n \t\t\tdie(\"delete %s given zero \", update->ref_name);\n-\t} else if (!line_termination)\n-\t\tdie(\"delete %s missing \", update->ref_name);\n+\t\tupdate->have_old = 1;\n+\t}\n \n \tif (*next != line_termination)\n \t\tdie(\"delete %s has extra input: %s\", update->ref_name, next);\n@@ -232,7 +245,6 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \n static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n {\n-\tstruct strbuf value = STRBUF_INIT;\n \tstruct ref_update *update;\n \n \tupdate = update_alloc();\n@@ -241,11 +253,13 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \tif (!update->ref_name)\n \t\tdie(\"verify line missing \");\n \n-\tif (!parse_next_arg(input, &next, &value)) {\n-\t\tupdate_store_old_sha1(\"verify\", update, value.buf);\n+\tif (parse_next_sha1(input, &next, update->old_sha1,\n+\t\t\t \"verify\", update->ref_name, 1)) {\n+\t\tupdate->have_old = 0;\n+\t} else {\n \t\thashcpy(update->new_sha1, update->old_sha1);\n-\t} else if (!line_termination)\n-\t\tdie(\"verify %s missing \", update->ref_name);\n+\t\tupdate->have_old = 1;\n+\t}\n \n \tif (*next != line_termination)\n \t\tdie(\"verify %s has extra input: %s\", update->ref_name, next);\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 42fec4e..7332753 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -851,7 +851,7 @@ test_expect_success 'stdin -z create ref fails with bad new value' '\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n-test_expect_failure 'stdin -z create ref fails with empty new value' '\n+test_expect_success 'stdin -z create ref fails with empty new value' '\n \tprintf $F \"create $c\" \"\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n \tgrep \"fatal: create $c missing \" err &&\n-- \n1.9.0\n"},{"id":"236383","messageId":"1394455603-2968-16-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 15/26] update-ref --stdin: Improve the error message for unexpected EOF","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:32Z","receivedAt":"2014-03-10T12:46:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Distinguish this error from the error that an argument is missing for\nanother reason. Update the tests accordingly.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 4 ++--\n t/t1400-update-ref.sh | 10 +++++-----\n 2 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 5937291..0a81a11 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -156,8 +156,8 @@ static int parse_next_sha1(struct strbuf *input, const char **next,\n \n eof:\n \tdie(old ?\n-\t \"%s %s missing \" :\n-\t \"%s %s missing \",\n+\t \"%s %s: unexpected end of input when reading \" :\n+\t \"%s %s: unexpected end of input when reading \",\n \t command, refname);\n }\n \ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 7332753..e9a0103 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -709,7 +709,7 @@ test_expect_success 'stdin -z fails create with bad ref name' '\n test_expect_success 'stdin -z fails create with no new value' '\n \tprintf $F \"create $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: create $a missing \" err\n+\tgrep \"fatal: create $a: unexpected end of input when reading \" err\n '\n \n test_expect_success 'stdin -z fails create with too many arguments' '\n@@ -733,13 +733,13 @@ test_expect_success 'stdin -z fails update with bad ref name' '\n test_expect_success 'stdin -z fails update with no new value' '\n \tprintf $F \"update $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: update $a missing \" err\n+\tgrep \"fatal: update $a: unexpected end of input when reading \" err\n '\n \n test_expect_success 'stdin -z fails update with no old value' '\n \tprintf $F \"update $a\" \"$m\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: update $a missing \" err\n+\tgrep \"fatal: update $a: unexpected end of input when reading \" err\n '\n \n test_expect_success 'stdin -z fails update with too many arguments' '\n@@ -763,7 +763,7 @@ test_expect_success 'stdin -z fails delete with bad ref name' '\n test_expect_success 'stdin -z fails delete with no old value' '\n \tprintf $F \"delete $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: delete $a missing \" err\n+\tgrep \"fatal: delete $a: unexpected end of input when reading \" err\n '\n \n test_expect_success 'stdin -z fails delete with too many arguments' '\n@@ -781,7 +781,7 @@ test_expect_success 'stdin -z fails verify with too many arguments' '\n test_expect_success 'stdin -z fails verify with no old value' '\n \tprintf $F \"verify $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: verify $a missing \" err\n+\tgrep \"fatal: verify $a: unexpected end of input when reading \" err\n '\n \n test_expect_success 'stdin -z fails option with unknown name' '\n-- \n1.9.0\n"},{"id":"236391","messageId":"1394455603-2968-17-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 16/26] update-ref --stdin: Harmonize error messages","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:33Z","receivedAt":"2014-03-10T12:46:33Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Make (most of) the error messages for invalid input have the same\nformat [1]:\n\n $COMMAND [SP $REFNAME]: $MESSAGE\n\nUpdate the tests accordingly.\n\n[1] A few error messages still have their old form, because $COMMAND\nand $REFNAME aren't passed all the way down the call stack.\n\nSigned-off-by: Michael Haggerty \n---\nMaking more error messages conform to the new pattern is an exercise\nleft to the reader (or maybe the writer if I find time to get back to\nit).\n\n builtin/update-ref.c | 24 ++++++++++++------------\n t/t1400-update-ref.sh | 32 ++++++++++++++++----------------\n 2 files changed, 28 insertions(+), 28 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 0a81a11..ac41635 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -180,17 +180,17 @@ static const char *parse_cmd_update(struct strbuf *input, const char *next)\n \n \tupdate->ref_name = parse_refname(input, &next);\n \tif (!update->ref_name)\n-\t\tdie(\"update line missing \");\n+\t\tdie(\"update: missing \");\n \n \tif (parse_next_sha1(input, &next, update->new_sha1,\n \t\t\t \"update\", update->ref_name, 0))\n-\t\tdie(\"update %s missing \", update->ref_name);\n+\t\tdie(\"update %s: missing \", update->ref_name);\n \n \tupdate->have_old = !parse_next_sha1(input, &next, update->old_sha1,\n \t\t\t\t\t \"update\", update->ref_name, 1);\n \n \tif (*next != line_termination)\n-\t\tdie(\"update %s has extra input: %s\", update->ref_name, next);\n+\t\tdie(\"update %s: extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\n@@ -203,17 +203,17 @@ static const char *parse_cmd_create(struct strbuf *input, const char *next)\n \n \tupdate->ref_name = parse_refname(input, &next);\n \tif (!update->ref_name)\n-\t\tdie(\"create line missing \");\n+\t\tdie(\"create: missing \");\n \n \tif (parse_next_sha1(input, &next, update->new_sha1,\n \t\t\t \"create\", update->ref_name, 0))\n-\t\tdie(\"create %s missing \", update->ref_name);\n+\t\tdie(\"create %s: missing \", update->ref_name);\n \n \tif (is_null_sha1(update->new_sha1))\n-\t\tdie(\"create %s given zero \", update->ref_name);\n+\t\tdie(\"create %s: zero \", update->ref_name);\n \n \tif (*next != line_termination)\n-\t\tdie(\"create %s has extra input: %s\", update->ref_name, next);\n+\t\tdie(\"create %s: extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\n@@ -226,19 +226,19 @@ static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n \n \tupdate->ref_name = parse_refname(input, &next);\n \tif (!update->ref_name)\n-\t\tdie(\"delete line missing \");\n+\t\tdie(\"delete: missing \");\n \n \tif (parse_next_sha1(input, &next, update->old_sha1,\n \t\t\t \"delete\", update->ref_name, 1)) {\n \t\tupdate->have_old = 0;\n \t} else {\n \t\tif (is_null_sha1(update->old_sha1))\n-\t\t\tdie(\"delete %s given zero \", update->ref_name);\n+\t\t\tdie(\"delete %s: zero \", update->ref_name);\n \t\tupdate->have_old = 1;\n \t}\n \n \tif (*next != line_termination)\n-\t\tdie(\"delete %s has extra input: %s\", update->ref_name, next);\n+\t\tdie(\"delete %s: extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\n@@ -251,7 +251,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \n \tupdate->ref_name = parse_refname(input, &next);\n \tif (!update->ref_name)\n-\t\tdie(\"verify line missing \");\n+\t\tdie(\"verify: missing \");\n \n \tif (parse_next_sha1(input, &next, update->old_sha1,\n \t\t\t \"verify\", update->ref_name, 1)) {\n@@ -262,7 +262,7 @@ static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n \t}\n \n \tif (*next != line_termination)\n-\t\tdie(\"verify %s has extra input: %s\", update->ref_name, next);\n+\t\tdie(\"verify %s: extra input: %s\", update->ref_name, next);\n \n \treturn next;\n }\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex e9a0103..3cc5c66 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -371,7 +371,7 @@ test_expect_success 'stdin fails on junk after quoted argument' '\n test_expect_success 'stdin fails create with no ref' '\n \techo \"create \" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: create line missing \" err\n+\tgrep \"fatal: create: missing \" err\n '\n \n test_expect_success 'stdin fails create with bad ref name' '\n@@ -383,19 +383,19 @@ test_expect_success 'stdin fails create with bad ref name' '\n test_expect_success 'stdin fails create with no new value' '\n \techo \"create $a\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: create $a missing \" err\n+\tgrep \"fatal: create $a: missing \" err\n '\n \n test_expect_success 'stdin fails create with too many arguments' '\n \techo \"create $a $m $m\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: create $a has extra input: $m\" err\n+\tgrep \"fatal: create $a: extra input: $m\" err\n '\n \n test_expect_success 'stdin fails update with no ref' '\n \techo \"update \" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: update line missing \" err\n+\tgrep \"fatal: update: missing \" err\n '\n \n test_expect_success 'stdin fails update with bad ref name' '\n@@ -407,19 +407,19 @@ test_expect_success 'stdin fails update with bad ref name' '\n test_expect_success 'stdin fails update with no new value' '\n \techo \"update $a\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: update $a missing \" err\n+\tgrep \"fatal: update $a: missing \" err\n '\n \n test_expect_success 'stdin fails update with too many arguments' '\n \techo \"update $a $m $m $m\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: update $a has extra input: $m\" err\n+\tgrep \"fatal: update $a: extra input: $m\" err\n '\n \n test_expect_success 'stdin fails delete with no ref' '\n \techo \"delete \" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: delete line missing \" err\n+\tgrep \"fatal: delete: missing \" err\n '\n \n test_expect_success 'stdin fails delete with bad ref name' '\n@@ -431,13 +431,13 @@ test_expect_success 'stdin fails delete with bad ref name' '\n test_expect_success 'stdin fails delete with too many arguments' '\n \techo \"delete $a $m $m\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: delete $a has extra input: $m\" err\n+\tgrep \"fatal: delete $a: extra input: $m\" err\n '\n \n test_expect_success 'stdin fails verify with too many arguments' '\n \techo \"verify $a $m $m\" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: verify $a has extra input: $m\" err\n+\tgrep \"fatal: verify $a: extra input: $m\" err\n '\n \n test_expect_success 'stdin fails option with unknown name' '\n@@ -532,7 +532,7 @@ test_expect_success 'stdin create ref fails with bad new value' '\n test_expect_success 'stdin create ref fails with zero new value' '\n \techo \"create $c \" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: create $c given zero \" err &&\n+\tgrep \"fatal: create $c: zero \" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n@@ -556,7 +556,7 @@ test_expect_success 'stdin delete ref fails with wrong old value' '\n test_expect_success 'stdin delete ref fails with zero old value' '\n \techo \"delete $a \" >stdin &&\n \ttest_must_fail git update-ref --stdin err &&\n-\tgrep \"fatal: delete $a given zero \" err &&\n+\tgrep \"fatal: delete $a: zero \" err &&\n \tgit rev-parse $m >expect &&\n \tgit rev-parse $a >actual &&\n \ttest_cmp expect actual\n@@ -697,7 +697,7 @@ test_expect_success 'stdin -z fails on unknown command' '\n test_expect_success 'stdin -z fails create with no ref' '\n \tprintf $F \"create \" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: create line missing \" err\n+\tgrep \"fatal: create: missing \" err\n '\n \n test_expect_success 'stdin -z fails create with bad ref name' '\n@@ -721,7 +721,7 @@ test_expect_success 'stdin -z fails create with too many arguments' '\n test_expect_success 'stdin -z fails update with no ref' '\n \tprintf $F \"update \" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: update line missing \" err\n+\tgrep \"fatal: update: missing \" err\n '\n \n test_expect_success 'stdin -z fails update with bad ref name' '\n@@ -751,7 +751,7 @@ test_expect_success 'stdin -z fails update with too many arguments' '\n test_expect_success 'stdin -z fails delete with no ref' '\n \tprintf $F \"delete \" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: delete line missing \" err\n+\tgrep \"fatal: delete: missing \" err\n '\n \n test_expect_success 'stdin -z fails delete with bad ref name' '\n@@ -854,7 +854,7 @@ test_expect_success 'stdin -z create ref fails with bad new value' '\n test_expect_success 'stdin -z create ref fails with empty new value' '\n \tprintf $F \"create $c\" \"\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: create $c missing \" err &&\n+\tgrep \"fatal: create $c: missing \" err &&\n \ttest_must_fail git rev-parse --verify -q $c\n '\n \n@@ -878,7 +878,7 @@ test_expect_success 'stdin -z delete ref fails with wrong old value' '\n test_expect_success 'stdin -z delete ref fails with zero old value' '\n \tprintf $F \"delete $a\" \"$Z\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-\tgrep \"fatal: delete $a given zero \" err &&\n+\tgrep \"fatal: delete $a: zero \" err &&\n \tgit rev-parse $m >expect &&\n \tgit rev-parse $a >actual &&\n \ttest_cmp expect actual\n-- \n1.9.0\n"},{"id":"236392","messageId":"1394455603-2968-18-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 17/26] refs: Add a concept of a reference transaction","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:34Z","receivedAt":"2014-03-10T12:46:34Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Build out the API for dealing with a bunch of reference checks and\nchanges within a transaction. Define an opaque ref_transaction type\nthat is managed entirely within refs.c. Introduce functions for\nstarting a transaction, adding updates to a transaction, and\ncommitting a transaction.\n\nThis API will soon replace update_refs().\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n refs.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 148 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 0963077..54260ce 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3267,6 +3267,85 @@ static int update_ref_write(const char *action, const char *refname,\n \treturn 0;\n }\n \n+/*\n+ * Data structure for holding a reference transaction, which can\n+ * consist of checks and updates to multiple references, carried out\n+ * as atomically as possible. This structure is opaque to callers.\n+ */\n+struct ref_transaction {\n+\tstruct ref_update **updates;\n+\tsize_t alloc;\n+\tsize_t nr;\n+};\n+\n+struct ref_transaction *create_ref_transaction(void)\n+{\n+\treturn xcalloc(1, sizeof(struct ref_transaction));\n+}\n+\n+void free_ref_transaction(struct ref_transaction *transaction)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < transaction->nr; i++) {\n+\t\tstruct ref_update *update = transaction->updates[i];\n+\n+\t\tfree((char *)update->ref_name);\n+\t\tfree(update);\n+\t}\n+\n+\tfree(transaction->updates);\n+\tfree(transaction);\n+}\n+\n+static struct ref_update *add_update(struct ref_transaction *transaction,\n+\t\t\t\t const char *refname)\n+{\n+\tstruct ref_update *update = xcalloc(1, sizeof(*update));\n+\n+\tupdate->ref_name = xstrdup(refname);\n+\tALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);\n+\ttransaction->updates[transaction->nr++] = update;\n+\treturn update;\n+}\n+\n+void queue_update_ref(struct ref_transaction *transaction, const char *refname,\n+\t\t unsigned char *new_sha1, unsigned char *old_sha1,\n+\t\t int flags, int have_old)\n+{\n+\tstruct ref_update *update = add_update(transaction, refname);\n+\n+\thashcpy(update->new_sha1, new_sha1);\n+\tupdate->flags = flags;\n+\tupdate->have_old = have_old;\n+\tif (have_old)\n+\t\thashcpy(update->old_sha1, old_sha1);\n+}\n+\n+void queue_create_ref(struct ref_transaction *transaction, const char *refname,\n+\t\t unsigned char *new_sha1,\n+\t\t int flags)\n+{\n+\tstruct ref_update *update = add_update(transaction, refname);\n+\n+\thashcpy(update->new_sha1, new_sha1);\n+\thashclr(update->old_sha1);\n+\tupdate->flags = flags;\n+\tupdate->have_old = 1;\n+}\n+\n+void queue_delete_ref(struct ref_transaction *transaction, const char *refname,\n+\t\t unsigned char *old_sha1,\n+\t\t int flags, int have_old)\n+{\n+\tstruct ref_update *update = add_update(transaction, refname);\n+\n+\tupdate->flags = flags;\n+\tupdate->have_old = have_old;\n+\tif (have_old)\n+\t\thashcpy(update->old_sha1, old_sha1);\n+}\n+\n int update_ref(const char *action, const char *refname,\n \t const unsigned char *sha1, const unsigned char *oldval,\n \t int flags, enum action_on_err onerr)\n@@ -3378,6 +3457,12 @@ cleanup:\n \treturn ret;\n }\n \n+int commit_ref_transaction(struct ref_transaction *transaction,\n+\t\t\t const char *msg, enum action_on_err onerr)\n+{\n+\treturn update_refs(msg, transaction->updates, transaction->nr, onerr);\n+}\n+\n char *shorten_unambiguous_ref(const char *refname, int strict)\n {\n \tint i;\ndiff --git a/refs.h b/refs.h\nindex 08e60ac..2848fb7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -24,6 +24,8 @@ struct ref_update {\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n };\n \n+struct ref_transaction;\n+\n /*\n * Bit values set in the flags argument passed to each_ref_fn():\n */\n@@ -220,6 +222,67 @@ enum action_on_err {\n \tUPDATE_REFS_QUIET_ON_ERR\n };\n \n+/*\n+ * Allocate and initialize a ref_transaction object. The object must\n+ * be freed by calling free_ref_transaction().\n+ */\n+struct ref_transaction *create_ref_transaction(void);\n+\n+/*\n+ * Free a ref_transaction and all associated data. This function does\n+ * not commit the transaction; that must be done first (if desired) by\n+ * calling commit_ref_transaction().\n+ */\n+void free_ref_transaction(struct ref_transaction *transaction);\n+\n+\n+/*\n+ * The following functions add a reference check or update to a\n+ * ref_transaction. In all of them, refname is the name of the\n+ * reference to be affected. The functions make internal copies of\n+ * refname, so the caller retains ownership of the parameter. flags\n+ * can be REF_NODEREF; it is passed to update_ref_lock().\n+ */\n+\n+\n+/*\n+ * Add a reference update to transaction. new_sha1 is the value that\n+ * the reference should have after the update, or zeros if it should\n+ * be deleted. If have_old is true, then old_sha1 holds the value\n+ * that the reference should have had before the update, or zeros if\n+ * it must not have existed beforehand.\n+ */\n+void queue_update_ref(struct ref_transaction *transaction, const char *refname,\n+\t\t unsigned char *new_sha1, unsigned char *old_sha1,\n+\t\t int flags, int have_old);\n+\n+/*\n+ * Add a reference creation to transaction. new_sha1 is the value\n+ * that the reference should have after the update, or zeros if it\n+ * should be deleted. It is verified that the reference does not\n+ * exist already.\n+ */\n+void queue_create_ref(struct ref_transaction *transaction, const char *refname,\n+\t\t unsigned char *new_sha1,\n+\t\t int flags);\n+\n+/*\n+ * Add a reference deletion to transaction. If have_old is true, then\n+ * old_sha1 holds the value that the reference should have had before\n+ * the update.\n+ */\n+void queue_delete_ref(struct ref_transaction *transaction, const char *refname,\n+\t\t unsigned char *old_sha1,\n+\t\t int flags, int have_old);\n+\n+/*\n+ * Commit all of the changes that have been queued in transaction, as\n+ * atomically as possible. Return a nonzero value if there is a\n+ * problem. The transaction is unmodified by this function.\n+ */\n+int commit_ref_transaction(struct ref_transaction *transaction,\n+\t\t\t const char *msg, enum action_on_err onerr);\n+\n /** Lock a ref and then write its file */\n int update_ref(const char *action, const char *refname,\n \t\tconst unsigned char *sha1, const unsigned char *oldval,\n-- \n1.9.0\n"},{"id":"236384","messageId":"1394455603-2968-19-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 18/26] update-ref --stdin: Reimplement using reference transactions","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:35Z","receivedAt":"2014-03-10T12:46:35Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This change is mostly clerical: the parse_cmd_*() functions need to\nuse local variables rather than a struct ref_update to collect the\narguments needed for each update, and then call queue_*_ref() to queue\nthe change rather than building up the list of changes at the caller\nside.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 142 +++++++++++++++++++++++++++------------------------\n 1 file changed, 76 insertions(+), 66 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex ac41635..ffed061 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -12,29 +12,11 @@ static const char * const git_update_ref_usage[] = {\n \tNULL\n };\n \n-static int updates_alloc;\n-static int updates_count;\n-static struct ref_update **updates;\n+static struct ref_transaction *transaction;\n \n static char line_termination = '\\n';\n static int update_flags;\n \n-static struct ref_update *update_alloc(void)\n-{\n-\tstruct ref_update *update;\n-\n-\t/* Allocate and zero-init a struct ref_update */\n-\tupdate = xcalloc(1, sizeof(*update));\n-\tALLOC_GROW(updates, updates_count + 1, updates_alloc);\n-\tupdates[updates_count++] = update;\n-\n-\t/* Store and reset accumulated options */\n-\tupdate->flags = update_flags;\n-\tupdate_flags = 0;\n-\n-\treturn update;\n-}\n-\n /*\n * Parse one whitespace- or NUL-terminated, possibly C-quoted argument\n * and append the result to arg. Return a pointer to the terminator.\n@@ -174,95 +156,118 @@ static int parse_next_sha1(struct strbuf *input, const char **next,\n \n static const char *parse_cmd_update(struct strbuf *input, const char *next)\n {\n-\tstruct ref_update *update;\n-\n-\tupdate = update_alloc();\n+\tchar *refname;\n+\tunsigned char new_sha1[20];\n+\tunsigned char old_sha1[20];\n+\tint have_old;\n \n-\tupdate->ref_name = parse_refname(input, &next);\n-\tif (!update->ref_name)\n+\trefname = parse_refname(input, &next);\n+\tif (!refname)\n \t\tdie(\"update: missing \");\n \n-\tif (parse_next_sha1(input, &next, update->new_sha1,\n-\t\t\t \"update\", update->ref_name, 0))\n-\t\tdie(\"update %s: missing \", update->ref_name);\n+\tif (parse_next_sha1(input, &next, new_sha1,\n+\t\t\t \"update\", refname, 0))\n+\t\tdie(\"update %s: missing \", refname);\n \n-\tupdate->have_old = !parse_next_sha1(input, &next, update->old_sha1,\n-\t\t\t\t\t \"update\", update->ref_name, 1);\n+\thave_old = !parse_next_sha1(input, &next, old_sha1,\n+\t\t\t\t \"update\", refname, 1);\n \n \tif (*next != line_termination)\n-\t\tdie(\"update %s: extra input: %s\", update->ref_name, next);\n+\t\tdie(\"update %s: extra input: %s\", refname, next);\n+\n+\tqueue_update_ref(transaction, refname, new_sha1, old_sha1,\n+\t\t\t update_flags, have_old);\n+\n+\tupdate_flags = 0;\n+\tfree(refname);\n \n \treturn next;\n }\n \n static const char *parse_cmd_create(struct strbuf *input, const char *next)\n {\n-\tstruct ref_update *update;\n-\n-\tupdate = update_alloc();\n+\tchar *refname;\n+\tunsigned char new_sha1[20];\n \n-\tupdate->ref_name = parse_refname(input, &next);\n-\tif (!update->ref_name)\n+\trefname = parse_refname(input, &next);\n+\tif (!refname)\n \t\tdie(\"create: missing \");\n \n-\tif (parse_next_sha1(input, &next, update->new_sha1,\n-\t\t\t \"create\", update->ref_name, 0))\n-\t\tdie(\"create %s: missing \", update->ref_name);\n+\tif (parse_next_sha1(input, &next, new_sha1,\n+\t\t\t \"create\", refname, 0))\n+\t\tdie(\"create %s: missing \", refname);\n \n-\tif (is_null_sha1(update->new_sha1))\n-\t\tdie(\"create %s: zero \", update->ref_name);\n+\tif (is_null_sha1(new_sha1))\n+\t\tdie(\"create %s: zero \", refname);\n \n \tif (*next != line_termination)\n-\t\tdie(\"create %s: extra input: %s\", update->ref_name, next);\n+\t\tdie(\"create %s: extra input: %s\", refname, next);\n+\n+\tqueue_create_ref(transaction, refname, new_sha1, update_flags);\n+\n+\tupdate_flags = 0;\n+\tfree(refname);\n \n \treturn next;\n }\n \n static const char *parse_cmd_delete(struct strbuf *input, const char *next)\n {\n-\tstruct ref_update *update;\n+\tchar *refname;\n+\tunsigned char old_sha1[20];\n+\tint have_old;\n \n-\tupdate = update_alloc();\n-\n-\tupdate->ref_name = parse_refname(input, &next);\n-\tif (!update->ref_name)\n+\trefname = parse_refname(input, &next);\n+\tif (!refname)\n \t\tdie(\"delete: missing \");\n \n-\tif (parse_next_sha1(input, &next, update->old_sha1,\n-\t\t\t \"delete\", update->ref_name, 1)) {\n-\t\tupdate->have_old = 0;\n+\tif (parse_next_sha1(input, &next, old_sha1, \"delete\", refname, 1)) {\n+\t\thave_old = 0;\n \t} else {\n-\t\tif (is_null_sha1(update->old_sha1))\n-\t\t\tdie(\"delete %s: zero \", update->ref_name);\n-\t\tupdate->have_old = 1;\n+\t\tif (is_null_sha1(old_sha1))\n+\t\t\tdie(\"delete %s: zero \", refname);\n+\t\thave_old = 1;\n \t}\n \n \tif (*next != line_termination)\n-\t\tdie(\"delete %s: extra input: %s\", update->ref_name, next);\n+\t\tdie(\"delete %s: extra input: %s\", refname, next);\n+\n+\tqueue_delete_ref(transaction, refname, old_sha1, update_flags, have_old);\n+\n+\tupdate_flags = 0;\n+\tfree(refname);\n \n \treturn next;\n }\n \n static const char *parse_cmd_verify(struct strbuf *input, const char *next)\n {\n-\tstruct ref_update *update;\n-\n-\tupdate = update_alloc();\n+\tchar *refname;\n+\tunsigned char new_sha1[20];\n+\tunsigned char old_sha1[20];\n+\tint have_old;\n \n-\tupdate->ref_name = parse_refname(input, &next);\n-\tif (!update->ref_name)\n+\trefname = parse_refname(input, &next);\n+\tif (!refname)\n \t\tdie(\"verify: missing \");\n \n-\tif (parse_next_sha1(input, &next, update->old_sha1,\n-\t\t\t \"verify\", update->ref_name, 1)) {\n-\t\tupdate->have_old = 0;\n+\tif (parse_next_sha1(input, &next, old_sha1,\n+\t\t\t \"verify\", refname, 1)) {\n+\t\thashclr(new_sha1);\n+\t\thave_old = 0;\n \t} else {\n-\t\thashcpy(update->new_sha1, update->old_sha1);\n-\t\tupdate->have_old = 1;\n+\t\thashcpy(new_sha1, old_sha1);\n+\t\thave_old = 1;\n \t}\n \n \tif (*next != line_termination)\n-\t\tdie(\"verify %s: extra input: %s\", update->ref_name, next);\n+\t\tdie(\"verify %s: extra input: %s\", refname, next);\n+\n+\tqueue_update_ref(transaction, refname, new_sha1, old_sha1,\n+\t\t\t update_flags, have_old);\n+\n+\tupdate_flags = 0;\n+\tfree(refname);\n \n \treturn next;\n }\n@@ -331,13 +336,18 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Refusing to perform update with empty message.\");\n \n \tif (read_stdin) {\n+\t\tint ret;\n+\t\ttransaction = create_ref_transaction();\n+\n \t\tif (delete || no_deref || argc > 0)\n \t\t\tusage_with_options(git_update_ref_usage, options);\n \t\tif (end_null)\n \t\t\tline_termination = '\\0';\n \t\tupdate_refs_stdin();\n-\t\treturn update_refs(msg, updates, updates_count,\n-\t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n+\t\tret = commit_ref_transaction(transaction, msg,\n+\t\t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n+\t\tfree_ref_transaction(transaction);\n+\t\treturn ret;\n \t}\n \n \tif (end_null)\n-- \n1.9.0\n"},{"id":"236385","messageId":"1394455603-2968-20-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 19/26] refs: Remove API function update_refs()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:36Z","receivedAt":"2014-03-10T12:46:36Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This should be done via reference transactions now. This also means\nthat struct ref_update can become private.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 31 ++++++++++++++++++++-----------\n refs.h | 20 --------------------\n 2 files changed, 20 insertions(+), 31 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 54260ce..91af0a0 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3267,6 +3267,20 @@ static int update_ref_write(const char *action, const char *refname,\n \treturn 0;\n }\n \n+/**\n+ * Information needed for a single ref update. Set new_sha1 to the\n+ * new value or to zero to delete the ref. To check the old value\n+ * while locking the ref, set have_old to 1 and set old_sha1 to the\n+ * value or to zero to ensure the ref does not exist before update.\n+ */\n+struct ref_update {\n+\tconst char *ref_name;\n+\tunsigned char new_sha1[20];\n+\tunsigned char old_sha1[20];\n+\tint flags; /* REF_NODEREF? */\n+\tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n+};\n+\n /*\n * Data structure for holding a reference transaction, which can\n * consist of checks and updates to multiple references, carried out\n@@ -3385,16 +3399,17 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n \treturn 0;\n }\n \n-int update_refs(const char *action, struct ref_update * const *updates_orig,\n-\t\tint n, enum action_on_err onerr)\n+int commit_ref_transaction(struct ref_transaction *transaction,\n+\t\t\t const char *msg, enum action_on_err onerr)\n {\n \tint ret = 0, delnum = 0, i;\n \tstruct ref_update **updates;\n \tint *types;\n \tstruct ref_lock **locks;\n \tconst char **delnames;\n+\tint n = transaction->nr;\n \n-\tif (!updates_orig || !n)\n+\tif (!n)\n \t\treturn 0;\n \n \t/* Allocate work space */\n@@ -3404,7 +3419,7 @@ int update_refs(const char *action, struct ref_update * const *updates_orig,\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n \n \t/* Copy, sort, and reject duplicate refs */\n-\tmemcpy(updates, updates_orig, sizeof(*updates) * n);\n+\tmemcpy(updates, transaction->updates, sizeof(*updates) * n);\n \tqsort(updates, n, sizeof(*updates), ref_update_compare);\n \tret = ref_update_reject_duplicates(updates, n, onerr);\n \tif (ret)\n@@ -3426,7 +3441,7 @@ int update_refs(const char *action, struct ref_update * const *updates_orig,\n \t/* Perform updates first so live commits remain referenced */\n \tfor (i = 0; i < n; i++)\n \t\tif (!is_null_sha1(updates[i]->new_sha1)) {\n-\t\t\tret = update_ref_write(action,\n+\t\t\tret = update_ref_write(msg,\n \t\t\t\t\t updates[i]->ref_name,\n \t\t\t\t\t updates[i]->new_sha1,\n \t\t\t\t\t locks[i], onerr);\n@@ -3457,12 +3472,6 @@ cleanup:\n \treturn ret;\n }\n \n-int commit_ref_transaction(struct ref_transaction *transaction,\n-\t\t\t const char *msg, enum action_on_err onerr)\n-{\n-\treturn update_refs(msg, transaction->updates, transaction->nr, onerr);\n-}\n-\n char *shorten_unambiguous_ref(const char *refname, int strict)\n {\n \tint i;\ndiff --git a/refs.h b/refs.h\nindex 2848fb7..b1f8b74 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -10,20 +10,6 @@ struct ref_lock {\n \tint force_write;\n };\n \n-/**\n- * Information needed for a single ref update. Set new_sha1 to the\n- * new value or to zero to delete the ref. To check the old value\n- * while locking the ref, set have_old to 1 and set old_sha1 to the\n- * value or to zero to ensure the ref does not exist before update.\n- */\n-struct ref_update {\n-\tconst char *ref_name;\n-\tunsigned char new_sha1[20];\n-\tunsigned char old_sha1[20];\n-\tint flags; /* REF_NODEREF? */\n-\tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n-};\n-\n struct ref_transaction;\n \n /*\n@@ -288,12 +274,6 @@ int update_ref(const char *action, const char *refname,\n \t\tconst unsigned char *sha1, const unsigned char *oldval,\n \t\tint flags, enum action_on_err onerr);\n \n-/**\n- * Lock all refs and then perform all modifications.\n- */\n-int update_refs(const char *action, struct ref_update * const *updates,\n-\t\tint n, enum action_on_err onerr);\n-\n extern int parse_hide_refs_config(const char *var, const char *value, const char *);\n extern int ref_is_hidden(const char *);\n \n-- \n1.9.0\n"},{"id":"236386","messageId":"1394455603-2968-21-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 20/26] struct ref_update: Rename field \"ref_name\" to \"refname\"","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:37Z","receivedAt":"2014-03-10T12:46:37Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is consistent with the usual nomenclature.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 18 +++++++++---------\n refs.h | 2 +-\n 2 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 91af0a0..5d08cdf 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3274,7 +3274,7 @@ static int update_ref_write(const char *action, const char *refname,\n * value or to zero to ensure the ref does not exist before update.\n */\n struct ref_update {\n-\tconst char *ref_name;\n+\tconst char *refname;\n \tunsigned char new_sha1[20];\n \tunsigned char old_sha1[20];\n \tint flags; /* REF_NODEREF? */\n@@ -3304,7 +3304,7 @@ void free_ref_transaction(struct ref_transaction *transaction)\n \tfor (i = 0; i < transaction->nr; i++) {\n \t\tstruct ref_update *update = transaction->updates[i];\n \n-\t\tfree((char *)update->ref_name);\n+\t\tfree((char *)update->refname);\n \t\tfree(update);\n \t}\n \n@@ -3317,7 +3317,7 @@ static struct ref_update *add_update(struct ref_transaction *transaction,\n {\n \tstruct ref_update *update = xcalloc(1, sizeof(*update));\n \n-\tupdate->ref_name = xstrdup(refname);\n+\tupdate->refname = xstrdup(refname);\n \tALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);\n \ttransaction->updates[transaction->nr++] = update;\n \treturn update;\n@@ -3375,7 +3375,7 @@ static int ref_update_compare(const void *r1, const void *r2)\n {\n \tconst struct ref_update * const *u1 = r1;\n \tconst struct ref_update * const *u2 = r2;\n-\treturn strcmp((*u1)->ref_name, (*u2)->ref_name);\n+\treturn strcmp((*u1)->refname, (*u2)->refname);\n }\n \n static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n@@ -3383,14 +3383,14 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n,\n {\n \tint i;\n \tfor (i = 1; i < n; i++)\n-\t\tif (!strcmp(updates[i - 1]->ref_name, updates[i]->ref_name)) {\n+\t\tif (!strcmp(updates[i - 1]->refname, updates[i]->refname)) {\n \t\t\tconst char *str =\n \t\t\t\t\"Multiple updates for ref '%s' not allowed.\";\n \t\t\tswitch (onerr) {\n \t\t\tcase UPDATE_REFS_MSG_ON_ERR:\n-\t\t\t\terror(str, updates[i]->ref_name); break;\n+\t\t\t\terror(str, updates[i]->refname); break;\n \t\t\tcase UPDATE_REFS_DIE_ON_ERR:\n-\t\t\t\tdie(str, updates[i]->ref_name); break;\n+\t\t\t\tdie(str, updates[i]->refname); break;\n \t\t\tcase UPDATE_REFS_QUIET_ON_ERR:\n \t\t\t\tbreak;\n \t\t\t}\n@@ -3427,7 +3427,7 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \n \t/* Acquire all locks while verifying old values */\n \tfor (i = 0; i < n; i++) {\n-\t\tlocks[i] = update_ref_lock(updates[i]->ref_name,\n+\t\tlocks[i] = update_ref_lock(updates[i]->refname,\n \t\t\t\t\t (updates[i]->have_old ?\n \t\t\t\t\t updates[i]->old_sha1 : NULL),\n \t\t\t\t\t updates[i]->flags,\n@@ -3442,7 +3442,7 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \tfor (i = 0; i < n; i++)\n \t\tif (!is_null_sha1(updates[i]->new_sha1)) {\n \t\t\tret = update_ref_write(msg,\n-\t\t\t\t\t updates[i]->ref_name,\n+\t\t\t\t\t updates[i]->refname,\n \t\t\t\t\t updates[i]->new_sha1,\n \t\t\t\t\t locks[i], onerr);\n \t\t\tlocks[i] = NULL; /* freed by update_ref_write */\ndiff --git a/refs.h b/refs.h\nindex b1f8b74..cc24213 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -154,7 +154,7 @@ extern void unlock_ref(struct ref_lock *lock);\n extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);\n \n /** Setup reflog before using. **/\n-int log_ref_setup(const char *ref_name, char *logfile, int bufsize);\n+int log_ref_setup(const char *refname, char *logfile, int bufsize);\n \n /** Reads log for the value of ref during at_time. **/\n extern int read_ref_at(const char *refname, unsigned long at_time, int cnt,\n-- \n1.9.0\n"},{"id":"236402","messageId":"1394455603-2968-22-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 21/26] struct ref_update: Store refname as a FLEX_ARRAY.","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:38Z","receivedAt":"2014-03-10T12:46:38Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Signed-off-by: Michael Haggerty \n---\n refs.c | 15 ++++++---------\n 1 file changed, 6 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 5d08cdf..335d0e2 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3274,11 +3274,11 @@ static int update_ref_write(const char *action, const char *refname,\n * value or to zero to ensure the ref does not exist before update.\n */\n struct ref_update {\n-\tconst char *refname;\n \tunsigned char new_sha1[20];\n \tunsigned char old_sha1[20];\n \tint flags; /* REF_NODEREF? */\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n+\tconst char refname[FLEX_ARRAY];\n };\n \n /*\n@@ -3301,12 +3301,8 @@ void free_ref_transaction(struct ref_transaction *transaction)\n {\n \tint i;\n \n-\tfor (i = 0; i < transaction->nr; i++) {\n-\t\tstruct ref_update *update = transaction->updates[i];\n-\n-\t\tfree((char *)update->refname);\n-\t\tfree(update);\n-\t}\n+\tfor (i = 0; i < transaction->nr; i++)\n+\t\tfree(transaction->updates[i]);\n \n \tfree(transaction->updates);\n \tfree(transaction);\n@@ -3315,9 +3311,10 @@ void free_ref_transaction(struct ref_transaction *transaction)\n static struct ref_update *add_update(struct ref_transaction *transaction,\n \t\t\t\t const char *refname)\n {\n-\tstruct ref_update *update = xcalloc(1, sizeof(*update));\n+\tsize_t len = strlen(refname);\n+\tstruct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);\n \n-\tupdate->refname = xstrdup(refname);\n+\tstrcpy((char *)update->refname, refname);\n \tALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);\n \ttransaction->updates[transaction->nr++] = update;\n \treturn update;\n-- \n1.9.0\n"},{"id":"236390","messageId":"1394455603-2968-23-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 22/26] commit_ref_transaction(): Introduce temporary variables","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:39Z","receivedAt":"2014-03-10T12:46:39Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Use temporary variables in the for-loop blocks to simplify expressions\nin the rest of the loop.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 25 ++++++++++++++++---------\n 1 file changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 335d0e2..ec638e9 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3424,10 +3424,12 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \n \t/* Acquire all locks while verifying old values */\n \tfor (i = 0; i < n; i++) {\n-\t\tlocks[i] = update_ref_lock(updates[i]->refname,\n-\t\t\t\t\t (updates[i]->have_old ?\n-\t\t\t\t\t updates[i]->old_sha1 : NULL),\n-\t\t\t\t\t updates[i]->flags,\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tlocks[i] = update_ref_lock(update->refname,\n+\t\t\t\t\t (update->have_old ?\n+\t\t\t\t\t update->old_sha1 : NULL),\n+\t\t\t\t\t update->flags,\n \t\t\t\t\t &types[i], onerr);\n \t\tif (!locks[i]) {\n \t\t\tret = 1;\n@@ -3436,23 +3438,28 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \t}\n \n \t/* Perform updates first so live commits remain referenced */\n-\tfor (i = 0; i < n; i++)\n-\t\tif (!is_null_sha1(updates[i]->new_sha1)) {\n+\tfor (i = 0; i < n; i++) {\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (!is_null_sha1(update->new_sha1)) {\n \t\t\tret = update_ref_write(msg,\n-\t\t\t\t\t updates[i]->refname,\n-\t\t\t\t\t updates[i]->new_sha1,\n+\t\t\t\t\t update->refname,\n+\t\t\t\t\t update->new_sha1,\n \t\t\t\t\t locks[i], onerr);\n \t\t\tlocks[i] = NULL; /* freed by update_ref_write */\n \t\t\tif (ret)\n \t\t\t\tgoto cleanup;\n \t\t}\n+\t}\n \n \t/* Perform deletes now that updates are safely completed */\n-\tfor (i = 0; i < n; i++)\n+\tfor (i = 0; i < n; i++) {\n \t\tif (locks[i]) {\n \t\t\tdelnames[delnum++] = locks[i]->ref_name;\n \t\t\tret |= delete_ref_loose(locks[i], types[i]);\n \t\t}\n+\t}\n+\n \tret |= repack_without_refs(delnames, delnum);\n \tfor (i = 0; i < delnum; i++)\n \t\tunlink_or_warn(git_path(\"logs/%s\", delnames[i]));\n-- \n1.9.0\n"},{"id":"236403","messageId":"1394455603-2968-24-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 23/26] struct ref_update: Add a lock member","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:40Z","receivedAt":"2014-03-10T12:46:40Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Now that we manage ref_update objects internally, we can use them to\nhold some of the scratch space we need when actually carrying out the\nupdates. Store the (struct ref_lock *) there.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 32 ++++++++++++++++----------------\n 1 file changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex ec638e9..73aec88 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3278,6 +3278,7 @@ struct ref_update {\n \tunsigned char old_sha1[20];\n \tint flags; /* REF_NODEREF? */\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n+\tstruct ref_lock *lock;\n \tconst char refname[FLEX_ARRAY];\n };\n \n@@ -3402,7 +3403,6 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \tint ret = 0, delnum = 0, i;\n \tstruct ref_update **updates;\n \tint *types;\n-\tstruct ref_lock **locks;\n \tconst char **delnames;\n \tint n = transaction->nr;\n \n@@ -3412,7 +3412,6 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \t/* Allocate work space */\n \tupdates = xmalloc(sizeof(*updates) * n);\n \ttypes = xmalloc(sizeof(*types) * n);\n-\tlocks = xcalloc(n, sizeof(*locks));\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n \n \t/* Copy, sort, and reject duplicate refs */\n@@ -3426,12 +3425,12 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \tfor (i = 0; i < n; i++) {\n \t\tstruct ref_update *update = updates[i];\n \n-\t\tlocks[i] = update_ref_lock(update->refname,\n-\t\t\t\t\t (update->have_old ?\n-\t\t\t\t\t update->old_sha1 : NULL),\n-\t\t\t\t\t update->flags,\n-\t\t\t\t\t &types[i], onerr);\n-\t\tif (!locks[i]) {\n+\t\tupdate->lock = update_ref_lock(update->refname,\n+\t\t\t\t\t (update->have_old ?\n+\t\t\t\t\t\tupdate->old_sha1 : NULL),\n+\t\t\t\t\t update->flags,\n+\t\t\t\t\t &types[i], onerr);\n+\t\tif (!update->lock) {\n \t\t\tret = 1;\n \t\t\tgoto cleanup;\n \t\t}\n@@ -3445,8 +3444,8 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \t\t\tret = update_ref_write(msg,\n \t\t\t\t\t update->refname,\n \t\t\t\t\t update->new_sha1,\n-\t\t\t\t\t locks[i], onerr);\n-\t\t\tlocks[i] = NULL; /* freed by update_ref_write */\n+\t\t\t\t\t update->lock, onerr);\n+\t\t\tupdate->lock = NULL; /* freed by update_ref_write */\n \t\t\tif (ret)\n \t\t\t\tgoto cleanup;\n \t\t}\n@@ -3454,9 +3453,11 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \n \t/* Perform deletes now that updates are safely completed */\n \tfor (i = 0; i < n; i++) {\n-\t\tif (locks[i]) {\n-\t\t\tdelnames[delnum++] = locks[i]->ref_name;\n-\t\t\tret |= delete_ref_loose(locks[i], types[i]);\n+\t\tstruct ref_update *update = updates[i];\n+\n+\t\tif (update->lock) {\n+\t\t\tdelnames[delnum++] = update->lock->ref_name;\n+\t\t\tret |= delete_ref_loose(update->lock, types[i]);\n \t\t}\n \t}\n \n@@ -3467,11 +3468,10 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \n cleanup:\n \tfor (i = 0; i < n; i++)\n-\t\tif (locks[i])\n-\t\t\tunlock_ref(locks[i]);\n+\t\tif (updates[i]->lock)\n+\t\t\tunlock_ref(updates[i]->lock);\n \tfree(updates);\n \tfree(types);\n-\tfree(locks);\n \tfree(delnames);\n \treturn ret;\n }\n-- \n1.9.0\n"},{"id":"236387","messageId":"1394455603-2968-25-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 24/26] struct ref_update: Add type field","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:41Z","receivedAt":"2014-03-10T12:46:41Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This is temporary space for commit_ref_transaction()\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 8 +++-----\n 1 file changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 73aec88..1fd38b0 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3279,6 +3279,7 @@ struct ref_update {\n \tint flags; /* REF_NODEREF? */\n \tint have_old; /* 1 if old_sha1 is valid, 0 otherwise */\n \tstruct ref_lock *lock;\n+\tint type;\n \tconst char refname[FLEX_ARRAY];\n };\n \n@@ -3402,7 +3403,6 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n {\n \tint ret = 0, delnum = 0, i;\n \tstruct ref_update **updates;\n-\tint *types;\n \tconst char **delnames;\n \tint n = transaction->nr;\n \n@@ -3411,7 +3411,6 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \n \t/* Allocate work space */\n \tupdates = xmalloc(sizeof(*updates) * n);\n-\ttypes = xmalloc(sizeof(*types) * n);\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n \n \t/* Copy, sort, and reject duplicate refs */\n@@ -3429,7 +3428,7 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \t\t\t\t\t (update->have_old ?\n \t\t\t\t\t\tupdate->old_sha1 : NULL),\n \t\t\t\t\t update->flags,\n-\t\t\t\t\t &types[i], onerr);\n+\t\t\t\t\t &update->type, onerr);\n \t\tif (!update->lock) {\n \t\t\tret = 1;\n \t\t\tgoto cleanup;\n@@ -3457,7 +3456,7 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \n \t\tif (update->lock) {\n \t\t\tdelnames[delnum++] = update->lock->ref_name;\n-\t\t\tret |= delete_ref_loose(update->lock, types[i]);\n+\t\t\tret |= delete_ref_loose(update->lock, update->type);\n \t\t}\n \t}\n \n@@ -3471,7 +3470,6 @@ cleanup:\n \t\tif (updates[i]->lock)\n \t\t\tunlock_ref(updates[i]->lock);\n \tfree(updates);\n-\tfree(types);\n \tfree(delnames);\n \treturn ret;\n }\n-- \n1.9.0\n"},{"id":"236388","messageId":"1394455603-2968-26-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 25/26] commit_ref_transaction(): Also free the ref_transaction","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:42Z","receivedAt":"2014-03-10T12:46:42Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change commit_ref_transaction() to also free the associated data, to\nabsolve the caller from having to do it.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/update-ref.c | 1 -\n refs.c | 1 +\n refs.h | 11 ++++++-----\n 3 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex ffed061..b33288c 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -346,7 +346,6 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\tupdate_refs_stdin();\n \t\tret = commit_ref_transaction(transaction, msg,\n \t\t\t\t\t UPDATE_REFS_DIE_ON_ERR);\n-\t\tfree_ref_transaction(transaction);\n \t\treturn ret;\n \t}\n \ndiff --git a/refs.c b/refs.c\nindex 1fd38b0..d83fc7b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3471,6 +3471,7 @@ cleanup:\n \t\t\tunlock_ref(updates[i]->lock);\n \tfree(updates);\n \tfree(delnames);\n+\tfree_ref_transaction(transaction);\n \treturn ret;\n }\n \ndiff --git a/refs.h b/refs.h\nindex cc24213..754894b 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -210,14 +210,15 @@ enum action_on_err {\n \n /*\n * Allocate and initialize a ref_transaction object. The object must\n- * be freed by calling free_ref_transaction().\n+ * be freed by calling commit_ref_transaction() or\n+ * free_ref_transaction().\n */\n struct ref_transaction *create_ref_transaction(void);\n \n /*\n- * Free a ref_transaction and all associated data. This function does\n- * not commit the transaction; that must be done first (if desired) by\n- * calling commit_ref_transaction().\n+ * Free a ref_transaction and all associated data. This function\n+ * should be called to free a ref_transaction that will not be\n+ * committed.\n */\n void free_ref_transaction(struct ref_transaction *transaction);\n \n@@ -264,7 +265,7 @@ void queue_delete_ref(struct ref_transaction *transaction, const char *refname,\n /*\n * Commit all of the changes that have been queued in transaction, as\n * atomically as possible. Return a nonzero value if there is a\n- * problem. The transaction is unmodified by this function.\n+ * problem. The transaction is freed by this function.\n */\n int commit_ref_transaction(struct ref_transaction *transaction,\n \t\t\t const char *msg, enum action_on_err onerr);\n-- \n1.9.0\n"},{"id":"236389","messageId":"1394455603-2968-27-git-send-email-mhagger@alum.mit.edu","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 26/26] commit_ref_transaction(): Work with transaction->updates in place","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T12:46:43Z","receivedAt":"2014-03-10T12:46:43Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Now that we free the transaction when we are done, there is no need to\nmake a copy of transaction->updates before working with it.\n\nSigned-off-by: Michael Haggerty \n---\n refs.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d83fc7b..ea33adc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -3402,19 +3402,17 @@ int commit_ref_transaction(struct ref_transaction *transaction,\n \t\t\t const char *msg, enum action_on_err onerr)\n {\n \tint ret = 0, delnum = 0, i;\n-\tstruct ref_update **updates;\n \tconst char **delnames;\n \tint n = transaction->nr;\n+\tstruct ref_update **updates = transaction->updates;\n \n \tif (!n)\n \t\treturn 0;\n \n \t/* Allocate work space */\n-\tupdates = xmalloc(sizeof(*updates) * n);\n \tdelnames = xmalloc(sizeof(*delnames) * n);\n \n \t/* Copy, sort, and reject duplicate refs */\n-\tmemcpy(updates, transaction->updates, sizeof(*updates) * n);\n \tqsort(updates, n, sizeof(*updates), ref_update_compare);\n \tret = ref_update_reject_duplicates(updates, n, onerr);\n \tif (ret)\n@@ -3469,7 +3467,6 @@ cleanup:\n \tfor (i = 0; i < n; i++)\n \t\tif (updates[i]->lock)\n \t\t\tunlock_ref(updates[i]->lock);\n-\tfree(updates);\n \tfree(delnames);\n \tfree_ref_transaction(transaction);\n \treturn ret;\n-- \n1.9.0\n"},{"id":"236404","messageId":"CALKQrgd+Jp4WK6EV1m8RJ5atT6J27yhg=nepxneNN0QEJmjPRQ@mail.gmail.com","threadId":"36110","inReplyTo":"1394455603-2968-6-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 05/26] t1400: Add some more tests involving quoted arguments","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2014-03-10T13:53:39Z","receivedAt":"2014-03-10T13:53:39Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Mon, Mar 10, 2014 at 1:46 PM, Michael Haggerty wrote:\n> Previously there were no good tests of C-quoted arguments.\n>\n> Signed-off-by: Michael Haggerty \n\nFWIW, the first 5 patches seem trivially correct to me. Feel free to add:\n\nReviewed-by: Johan Herland \n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"236417","messageId":"531DF079.9050909@kitware.com","threadId":"36110","inReplyTo":"1394455603-2968-4-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 03/26] t1400: Pass a legitimate to update command","fromName":"Brad King","fromEmail":"brad.king@kitware.com","sentAt":"2014-03-10T17:03:53Z","receivedAt":"2014-03-10T17:03:53Z","isPatch":true,"sender":{"key":"brad.king@kitware.com","avatar":"https://avatars.githubusercontent.com/u/87268?v=4"},"body":"On 03/10/2014 08:46 AM, Michael Haggerty wrote:\n> This test is trying to test a few ways to delete references using \"git\n> update-ref -z --stdin\". The third line passed in is\n> \n> update SP /refs/heads/c NUL NUL NUL\n> \n> , which is not a correct way to delete a reference according to the\n> documentation (the new value should be zeros, not empty). Pass zeros\n> instead as the new value to test the code correctly.\n\nIn my original work on this feature, an empty is allowed.\nSince newvalue is not optional an empty value can be treated as zero.\nThe relevant documentation is:\n\n update::\n Set to after verifying , if given.\n Specify a zero to ensure the ref does not exist\n\n ...\n\n Use 40 \"0\" or the empty string to specify a zero value, except that\n with `-z` an empty is considered missing.\n\nThe two together say that can be the empty string instead\nof a literal zero.\n\n-Brad\n"},{"id":"236419","messageId":"531DF195.7020304@kitware.com","threadId":"36110","inReplyTo":"1394455603-2968-14-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 13/26] update-ref --stdin: Simplify error messages for missing oldvalues","fromName":"Brad King","fromEmail":"brad.king@kitware.com","sentAt":"2014-03-10T17:08:37Z","receivedAt":"2014-03-10T17:08:37Z","isPatch":true,"sender":{"key":"brad.king@kitware.com","avatar":"https://avatars.githubusercontent.com/u/87268?v=4"},"body":"On 03/10/2014 08:46 AM, Michael Haggerty wrote:\n> Instead of, for example,\n> \n> fatal: update refs/heads/master missing [] NUL\n> \n> emit\n> \n> fatal: update refs/heads/master missing \n[snip]\n> -\t\tdie(\"update %s missing [] NUL\", update->ref_name);\n> +\t\tdie(\"update %s missing \", update->ref_name);\n\nThe reason for the original wording is that the is indeed\noptional. This can only occur at end-of-input, and it is actually the\n*NUL* that is missing because an empty old value can be specified to\nmean that it it intentionally missing.\n\n-Brad\n"},{"id":"236418","messageId":"531DF26E.50806@kitware.com","threadId":"36110","inReplyTo":"531DF195.7020304@kitware.com","subject":"Re: [PATCH 13/26] update-ref --stdin: Simplify error messages for missing oldvalues","fromName":"Brad King","fromEmail":"brad.king@kitware.com","sentAt":"2014-03-10T17:12:14Z","receivedAt":"2014-03-10T17:12:14Z","isPatch":true,"sender":{"key":"brad.king@kitware.com","avatar":"https://avatars.githubusercontent.com/u/87268?v=4"},"body":"On 03/10/2014 01:08 PM, Brad King wrote:\n>> -\t\tdie(\"update %s missing [] NUL\", update->ref_name);\n>> +\t\tdie(\"update %s missing \", update->ref_name);\n> \n> The reason for the original wording is that the is indeed\n> optional. This can only occur at end-of-input, and it is actually the\n> *NUL* that is missing because an empty old value can be specified to\n> mean that it it intentionally missing.\n\nI see a following patch makes the wording even clearer about\nunexpected end of input, so ignore my previous review.\n\n-Brad\n"},{"id":"236423","messageId":"531DF9FC.4070707@kitware.com","threadId":"36110","inReplyTo":"1394455603-2968-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH 00/26] Clean up update-refs --stdin and implement ref_transaction","fromName":"Brad King","fromEmail":"brad.king@kitware.com","sentAt":"2014-03-10T17:44:28Z","receivedAt":"2014-03-10T17:44:28Z","isPatch":true,"sender":{"key":"brad.king@kitware.com","avatar":"https://avatars.githubusercontent.com/u/87268?v=4"},"body":"Hi Michael,\n\nThis is excellent work.\n\nI haven't reviewed every line of logic in detail but the changes\nlook correct at a high level. The only exception is that the empty\n is supposed to be accepted and treated as zero even in\n\"--stdin -z\" mode. See my response to that individual change.\n\nOn 03/10/2014 08:46 AM, Michael Haggerty wrote:\n> The new API for dealing with reference transactions is\n> \n> ref_transaction *transaction = create_ref_transaction();\n> queue_create_ref(transaction, refname, new_sha1, ...);\n> queue_update_ref(transaction, refname, new_sha1, old_sha1, ...);\n> queue_delete_ref(transaction, refname, old_sha1, ...);\n> ...\n> if (commit_ref_transaction(transaction, msg, ...))\n> die(...);\n\nThe layout of this API looks good.\n\nThe name \"queue\" is not fully representative of the current behavior.\nIt implies that the order is meaningful but we currently allow at most\none update to a ref and sort them by refname. Does your follow-up work\ndefine behavior for multiple updates to one ref? Can it collapse them\ninto a single update after checking internal consistency of the sequence?\n\n> So most of the commits in this series are actually cleanups in\n> builtin/update-ref.c. I also spend some time making the error\n> messages emitted by that command more uniform.\n\nAll good cleanups, thanks.\n\n> Finally, now that refs.c owns the data structures for dealing with\n> transactions, it is possible to make a few simplifications.\n\nYes, it is much nicer to keep the data structures private, especially\nas it avoids the copy of the transaction made before sorting.\n\nThanks,\n-Brad\n"},{"id":"236464","messageId":"531E30D7.40208@alum.mit.edu","threadId":"36110","inReplyTo":"531DF079.9050909@kitware.com","subject":"Re: [PATCH 03/26] t1400: Pass a legitimate to update command","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T21:38:31Z","receivedAt":"2014-03-10T21:38:31Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Brad,\n\nThanks for your feedback.\n\nOn 03/10/2014 06:03 PM, Brad King wrote:\n> On 03/10/2014 08:46 AM, Michael Haggerty wrote:\n>> This test is trying to test a few ways to delete references using \"git\n>> update-ref -z --stdin\". The third line passed in is\n>>\n>> update SP /refs/heads/c NUL NUL NUL\n>>\n>> , which is not a correct way to delete a reference according to the\n>> documentation (the new value should be zeros, not empty). Pass zeros\n>> instead as the new value to test the code correctly.\n> \n> In my original work on this feature, an empty is allowed.\n> Since newvalue is not optional an empty value can be treated as zero.\n> The relevant documentation is:\n> \n> update::\n> Set to after verifying , if given.\n> Specify a zero to ensure the ref does not exist\n> \n> ...\n> \n> Use 40 \"0\" or the empty string to specify a zero value, except that\n> with `-z` an empty is considered missing.\n> \n> The two together say that can be the empty string instead\n> of a literal zero.\n\nOK, with your explanation and after reading the docs a couple more\ntimes, I can see your reading. Your rules as I now understand them:\n\n* Without -z\n * 0{40} or the empty string represents zeros\n * No preceding SP delimiter indicates that the value is missing\n (as are any following values)\n\n* With -z\n * For \n * 0{40} or the empty string represents zeros (the value is\n not allowed to be missing)\n * For \n * 0{40} represents zeros\n * The empty string indicates that the value is missing\n\nI implemented the slightly simpler rules\n\n* Without -z\n * 0{40} or the empty string represents zeros\n * No preceding delimiter indicates that the value is missing (as\n are any following values)\n\n* With -z\n * 0{40} represents zeros\n * The empty string indicates that the value is missing\n\nIt seems to me that \"-z\" input will nearly always be machine-generated,\nso there is not much reason to accept the empty string as shorthand for\nzeros. So I think that my version of the rules, being simpler to\nexplain, is a slight improvement. But your version is already out in\nthe wild, so backwards-compatibility is also a consideration, even\nthough it is rather a fine point in a rather unlikely usage (why use\nupdate rather than delete to delete a reference?).\n\nI don't know. I'm willing to rewrite the code to go back to your rules,\nor rewrite the documentation to describe my rules.\n\nNeutral bystanders *cough*Junio*cough*, what do you prefer?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"236465","messageId":"531E32CE.9040509@alum.mit.edu","threadId":"36110","inReplyTo":"531DF9FC.4070707@kitware.com","subject":"Re: [PATCH 00/26] Clean up update-refs --stdin and implement ref_transaction","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-10T21:46:54Z","receivedAt":"2014-03-10T21:46:54Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/10/2014 06:44 PM, Brad King wrote:\n> [...]\n\nThanks for your kind words.\n\n> On 03/10/2014 08:46 AM, Michael Haggerty wrote:\n>> The new API for dealing with reference transactions is\n>>\n>> ref_transaction *transaction = create_ref_transaction();\n>> queue_create_ref(transaction, refname, new_sha1, ...);\n>> queue_update_ref(transaction, refname, new_sha1, old_sha1, ...);\n>> queue_delete_ref(transaction, refname, old_sha1, ...);\n>> ...\n>> if (commit_ref_transaction(transaction, msg, ...))\n>> die(...);\n> \n> The layout of this API looks good.\n> \n> The name \"queue\" is not fully representative of the current behavior.\n> It implies that the order is meaningful but we currently allow at most\n> one update to a ref and sort them by refname. Does your follow-up work\n> define behavior for multiple updates to one ref? Can it collapse them\n> into a single update after checking internal consistency of the sequence?\n\nI don't really like the word \"queue\" in these function names, but I\ncouldn't think of a better alternative. I wanted a word that conveys\nthat the change is being collected for later application as opposed to\nbeing applied immediately. Other suggestions are welcome.\n\nIn the future I *do* want to define the behavior for multiple updates to\na single ref. Even now, although order is not preserved when carrying\nout the updates, the facts that (1) at most one update is allowed to\neach reference and (2) the changes are made (approximately) atomically\ntogether mean that the effect of committing a transaction is\n(approximately) indistinguishable from the effect it would have if order\nwere preserved.\n\n> [...]\n\nCheers,\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"236501","messageId":"531F0658.4000106@kitware.com","threadId":"36110","inReplyTo":"531E30D7.40208@alum.mit.edu","subject":"Re: [PATCH 03/26] t1400: Pass a legitimate to update command","fromName":"Brad King","fromEmail":"brad.king@kitware.com","sentAt":"2014-03-11T12:49:28Z","receivedAt":"2014-03-11T12:49:28Z","isPatch":true,"sender":{"key":"brad.king@kitware.com","avatar":"https://avatars.githubusercontent.com/u/87268?v=4"},"body":"On 03/10/2014 05:38 PM, Michael Haggerty wrote:\n> It seems to me that \"-z\" input will nearly always be machine-generated,\n> so there is not much reason to accept the empty string as shorthand for\n> zeros. So I think that my version of the rules, being simpler to\n> explain, is a slight improvement.\n\nI agree.\n\n> But your version is already out in the wild, so backwards-compatibility\n> is also a consideration, even though it is rather a fine point in a\n> rather unlikely usage (why use update rather than delete to delete a\n> reference?).\n\nI'm not using empty==zero with -z in any deployment. Since the feature\nis quite new, the behavior change is not silent, and it is easy to\nconstruct input that works with both versions, I do not think we need\nto worry about compatibility.\n\n> or rewrite the documentation to describe my rules.\n\nI prefer this approach.\n\nThanks,\n-Brad\n"},{"id":"236528","messageId":"xmqqa9cwpkiw.fsf@gitster.dls.corp.google.com","threadId":"36110","inReplyTo":"531E30D7.40208@alum.mit.edu","subject":"Re: [PATCH 03/26] t1400: Pass a legitimate to update command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-11T20:06:47Z","receivedAt":"2014-03-11T20:06:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> It seems to me that \"-z\" input will nearly always be machine-generated,\n> so there is not much reason to accept the empty string as shorthand for\n> zeros. So I think that my version of the rules, being simpler to\n> explain, is a slight improvement. But your version is already out in\n> the wild, so backwards-compatibility is also a consideration, even\n> though it is rather a fine point in a rather unlikely usage (why use\n> update rather than delete to delete a reference?).\n>\n> I don't know. I'm willing to rewrite the code to go back to your rules,\n> or rewrite the documentation to describe my rules.\n>\n> Neutral bystanders *cough*Junio*cough*, what do you prefer?\n\nI may be misremembering things, but your first sentence quoted above\nwas exactly my reaction while reviewing the original change, and I\nmight have even raised that as an issue myself, saying something\nlike \"consistency across values is more important than type-saving\nin a machine format\".\n\nSince nobody else were raising the issue back then, however, we are\nstuck with the interface. I am not against deprecating and removing\nthe support for it in the longer term, though.\n"},{"id":"236541","messageId":"531F82FE.9030305@kitware.com","threadId":"36110","inReplyTo":"xmqqa9cwpkiw.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/26] t1400: Pass a legitimate to update command","fromName":"Brad King","fromEmail":"brad.king@kitware.com","sentAt":"2014-03-11T21:41:18Z","receivedAt":"2014-03-11T21:41:18Z","isPatch":true,"sender":{"key":"brad.king@kitware.com","avatar":"https://avatars.githubusercontent.com/u/87268?v=4"},"body":"On Tue, Mar 11, 2014 at 4:06 PM, Junio C Hamano wrote:\n> I may be misremembering things, but your first sentence quoted above\n> was exactly my reaction while reviewing the original change, and I\n> might have even raised that as an issue myself, saying something\n> like \"consistency across values is more important than type-saving\n> in a machine format\".\n\nFor reference, the original design discussion of the format was here:\n\n http://thread.gmane.org/gmane.comp.version-control.git/233842\n\nI do not recall this issue being raised before, but now that it has\nbeen raised I fully agree:\n\n http://thread.gmane.org/gmane.comp.version-control.git/243754/focus=243862\n\nIn -z mode an empty should be treated as missing just as\nit is for . This is obvious now in hindsight and I wish I\nhad realized this at the time. Back then I went through a lot of\niterations on the format and missed this simplification in the final\nversion :(\n\nMoving forward:\n\nThe \"create\" command rejects a zero so the change in\nquestion for that command is merely the wording of the error message\nand there is no compatibility issue.\n\nThe \"update\" command supports a zero so that it can\nbe used for all operations (create, update, delete, verify) with\nthe proper combination of old and new values. The change in question\nmakes an empty an error where it was previously treated\nas zero. (BTW, Michael, I do not see a test case for the new error\nin your series. Something like the patch below should work.)\n\n> I am not against deprecating and removing\n> the support for it in the longer term, though.\n\nAs I reported in my above-linked response, I'm not depending on\nthe old behavior myself. Also if one were to start seeing this\nerror then generated input needs only trivial changes to avoid it.\nIf we do want to preserve compatibility for others then perhaps an\nempty with -z should produce:\n\n warning: update $ref: missing , treating as zero\n\nThen after a few releases it can be switched to an error.\n\nThanks,\n-Brad\n\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex 3cc5c66..1e9fe7c 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -730,6 +730,12 @@ test_expect_success 'stdin -z fails update with bad ref name' '\n \tgrep \"fatal: invalid ref format: ~a\" err\n '\n\n+test_expect_success 'stdin -z fails update with empty new value' '\n+\tprintf $F \"update $a\" \"\" >stdin &&\n+\ttest_must_fail git update-ref -z --stdin err &&\n+\tgrep \"fatal: update $a: missing \" err\n+'\n+\n test_expect_success 'stdin -z fails update with no new value' '\n \tprintf $F \"update $a\" >stdin &&\n \ttest_must_fail git update-ref -z --stdin err &&\n-- \n1.8.5.2\n"},{"id":"237181","messageId":"532B1ED1.3090208@alum.mit.edu","threadId":"36110","inReplyTo":"531F82FE.9030305@kitware.com","subject":"Re: [PATCH 03/26] t1400: Pass a legitimate to update command","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-20T17:01:05Z","receivedAt":"2014-03-20T17:01:05Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/11/2014 10:41 PM, Brad King wrote:\n> On Tue, Mar 11, 2014 at 4:06 PM, Junio C Hamano wrote:\n>> I may be misremembering things, but your first sentence quoted above\n>> was exactly my reaction while reviewing the original change, and I\n>> might have even raised that as an issue myself, saying something\n>> like \"consistency across values is more important than type-saving\n>> in a machine format\".\n> \n> For reference, the original design discussion of the format was here:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/233842\n> \n> I do not recall this issue being raised before, but now that it has\n> been raised I fully agree:\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/243754/focus=243862\n> \n> In -z mode an empty should be treated as missing just as\n> it is for . This is obvious now in hindsight and I wish I\n> had realized this at the time. Back then I went through a lot of\n> iterations on the format and missed this simplification in the final\n> version :(\n\nIt's not your fault; anybody could have reviewed your code at the time\n(I most of all, because I have been so active in this area of the code).\n\n> Moving forward:\n> \n> The \"create\" command rejects a zero so the change in\n> question for that command is merely the wording of the error message\n> and there is no compatibility issue.\n> \n> The \"update\" command supports a zero so that it can\n> be used for all operations (create, update, delete, verify) with\n> the proper combination of old and new values. The change in question\n> makes an empty an error where it was previously treated\n> as zero. (BTW, Michael, I do not see a test case for the new error\n> in your series. Something like the patch below should work.)\n> \n>> I am not against deprecating and removing\n>> the support for it in the longer term, though.\n> \n> As I reported in my above-linked response, I'm not depending on\n> the old behavior myself. Also if one were to start seeing this\n> error then generated input needs only trivial changes to avoid it.\n> If we do want to preserve compatibility for others then perhaps an\n> empty with -z should produce:\n> \n> warning: update $ref: missing , treating as zero\n\nThis last suggestion is what I am implementing for the re-roll (coming\nshortly). Thanks for the discussion.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}