{"thread":{"id":"28414","subject":"[PATCH v5 1/4] merge: keep stash[] a local variable","startedAt":"2011-09-17T11:57:42Z","lastAt":"2011-09-18T21:51:31Z","messageCount":5,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":5,"patchTotal":4},"messages":[{"id":"175687","messageId":"1316260665-1728-1-git-send-email-pclouds@gmail.com","threadId":"28414","inReplyTo":null,"subject":"[PATCH v5 1/4] merge: keep stash[] a local variable","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-09-17T11:57:42Z","receivedAt":"2011-09-17T11:57:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"A stash is created by save_state() and used by restore_state(). Pass\nSHA-1 explicitly for clarity and keep stash[] to cmd_merge().\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n v4 was http://thread.gmane.org/gmane.comp.version-control.git/179375/focus=179706\n\n builtin/merge.c |   33 ++++++++++++++++-----------------\n 1 files changed, 16 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 325891e..a068660 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -50,7 +50,7 @@ static int fast_forward_only;\n static int allow_trivial = 1, have_message;\n static struct strbuf merge_msg;\n static struct commit_list *remoteheads;\n-static unsigned char head[20], stash[20];\n+static unsigned char head[20];\n static struct strategy **use_strategies;\n static size_t use_strategies_nr, use_strategies_alloc;\n static const char **xopts;\n@@ -217,7 +217,7 @@ static void drop_save(void)\n \tunlink(git_path(\"MERGE_MODE\"));\n }\n \n-static void save_state(void)\n+static int save_state(unsigned char *stash)\n {\n \tint len;\n \tstruct child_process cp;\n@@ -236,11 +236,12 @@ static void save_state(void)\n \n \tif (finish_command(&cp) || len < 0)\n \t\tdie(_(\"stash failed\"));\n-\telse if (!len)\n-\t\treturn;\n+\telse if (!len)\t\t/* no changes */\n+\t\treturn -1;\n \tstrbuf_setlen(&buffer, buffer.len-1);\n \tif (get_sha1(buffer.buf, stash))\n \t\tdie(_(\"not a valid object: %s\"), buffer.buf);\n+\treturn 0;\n }\n \n static void read_empty(unsigned const char *sha1, int verbose)\n@@ -278,7 +279,7 @@ static void reset_hard(unsigned const char *sha1, int verbose)\n \t\tdie(_(\"read-tree failed\"));\n }\n \n-static void restore_state(void)\n+static void restore_state(const unsigned char *stash)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *args[] = { \"stash\", \"apply\", NULL, NULL };\n@@ -1010,6 +1011,7 @@ static int setup_with_upstream(const char ***argv)\n int cmd_merge(int argc, const char **argv, const char *prefix)\n {\n \tunsigned char result_tree[20];\n+\tunsigned char stash[20];\n \tstruct strbuf buf = STRBUF_INIT;\n \tconst char *head_arg;\n \tint flag, head_invalid = 0, i;\n@@ -1320,21 +1322,18 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * sync with the head commit.  The strategies are responsible\n \t * to ensure this.\n \t */\n-\tif (use_strategies_nr != 1) {\n-\t\t/*\n-\t\t * Stash away the local changes so that we can try more\n-\t\t * than one.\n-\t\t */\n-\t\tsave_state();\n-\t} else {\n-\t\tmemcpy(stash, null_sha1, 20);\n-\t}\n+\tif (use_strategies_nr == 1 ||\n+\t    /*\n+\t     * Stash away the local changes so that we can try more than one.\n+\t     */\n+\t    save_state(stash))\n+\t\thashcpy(stash, null_sha1);\n \n \tfor (i = 0; i < use_strategies_nr; i++) {\n \t\tint ret;\n \t\tif (i) {\n \t\t\tprintf(_(\"Rewinding the tree to pristine...\\n\"));\n-\t\t\trestore_state();\n+\t\t\trestore_state(stash);\n \t\t}\n \t\tif (use_strategies_nr != 1)\n \t\t\tprintf(_(\"Trying merge strategy %s...\\n\"),\n@@ -1395,7 +1394,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * it up.\n \t */\n \tif (!best_strategy) {\n-\t\trestore_state();\n+\t\trestore_state(stash);\n \t\tif (use_strategies_nr > 1)\n \t\t\tfprintf(stderr,\n \t\t\t\t_(\"No merge strategy handled the merge.\\n\"));\n@@ -1407,7 +1406,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t; /* We already have its result in the working tree. */\n \telse {\n \t\tprintf(_(\"Rewinding the tree to pristine...\\n\"));\n-\t\trestore_state();\n+\t\trestore_state(stash);\n \t\tprintf(_(\"Using the %s to prepare resolving by hand.\\n\"),\n \t\t\tbest_strategy);\n \t\ttry_merge_strategy(best_strategy, common, head_arg);\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"175688","messageId":"1316260665-1728-2-git-send-email-pclouds@gmail.com","threadId":"28414","inReplyTo":"1316260665-1728-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v5 2/4] merge: use return value of resolve_ref() to determine if HEAD is invalid","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-09-17T11:57:43Z","receivedAt":"2011-09-17T11:57:43Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"resolve_ref() only updates \"head\" when it returns non NULL value (it\nmay update \"head\" even when returning NULL, but not in all cases).\n\nBecause \"head\" is not initialized before the call, is_null_sha1() is\nnot enough. Check also resolve_ref() return value.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n > Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n >> -     if (is_null_sha1(head))\n >> -             head_invalid = 1;\n >> +     if (!is_null_sha1(head)) {\n >> +             head_commit = lookup_commit(head);\n >> +             if (!head_commit)\n >> +                     die(_(\"could not parse HEAD\"));\n >> +     }\n >\n > Is this is_null_sha1() valid without first clearing head[]?\n\n No it's not.\n\n builtin/merge.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a068660..c371484 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1030,7 +1030,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tbranch = resolve_ref(\"HEAD\", head, 0, &flag);\n \tif (branch && !prefixcmp(branch, \"refs/heads/\"))\n \t\tbranch += 11;\n-\tif (is_null_sha1(head))\n+\tif (!branch || is_null_sha1(head))\n \t\thead_invalid = 1;\n \n \tgit_config(git_merge_config, NULL);\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"175689","messageId":"1316260665-1728-3-git-send-email-pclouds@gmail.com","threadId":"28414","inReplyTo":"1316260665-1728-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v5 3/4] merge: remove global variable head[]","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-09-17T11:57:44Z","receivedAt":"2011-09-17T11:57:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Also kill head_invalid in favor of \"head_commit == NULL\".\n\nLocal variable \"head\" in cmd_merge() is renamed to \"head_sha1\" to make\nsure I don't miss any access because this variable should not be used\nafter head_commit is set (use head_commit->object.sha1 instead).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/merge.c |   97 ++++++++++++++++++++++++++++++-------------------------\n 1 files changed, 53 insertions(+), 44 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex c371484..f5eb3f5 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -50,7 +50,6 @@ static int fast_forward_only;\n static int allow_trivial = 1, have_message;\n static struct strbuf merge_msg;\n static struct commit_list *remoteheads;\n-static unsigned char head[20];\n static struct strategy **use_strategies;\n static size_t use_strategies_nr, use_strategies_alloc;\n static const char **xopts;\n@@ -279,7 +278,8 @@ static void reset_hard(unsigned const char *sha1, int verbose)\n \t\tdie(_(\"read-tree failed\"));\n }\n \n-static void restore_state(const unsigned char *stash)\n+static void restore_state(const unsigned char *head,\n+\t\t\t  const unsigned char *stash)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *args[] = { \"stash\", \"apply\", NULL, NULL };\n@@ -309,10 +309,9 @@ static void finish_up_to_date(const char *msg)\n \tdrop_save();\n }\n \n-static void squash_message(void)\n+static void squash_message(struct commit *commit)\n {\n \tstruct rev_info rev;\n-\tstruct commit *commit;\n \tstruct strbuf out = STRBUF_INIT;\n \tstruct commit_list *j;\n \tint fd;\n@@ -327,7 +326,6 @@ static void squash_message(void)\n \trev.ignore_merges = 1;\n \trev.commit_format = CMIT_FMT_MEDIUM;\n \n-\tcommit = lookup_commit(head);\n \tcommit->object.flags |= UNINTERESTING;\n \tadd_pending_object(&rev, &commit->object, NULL);\n \n@@ -356,9 +354,11 @@ static void squash_message(void)\n \tstrbuf_release(&out);\n }\n \n-static void finish(const unsigned char *new_head, const char *msg)\n+static void finish(struct commit *head_commit,\n+\t\t   const unsigned char *new_head, const char *msg)\n {\n \tstruct strbuf reflog_message = STRBUF_INIT;\n+\tconst unsigned char *head = head_commit->object.sha1;\n \n \tif (!msg)\n \t\tstrbuf_addstr(&reflog_message, getenv(\"GIT_REFLOG_ACTION\"));\n@@ -369,7 +369,7 @@ static void finish(const unsigned char *new_head, const char *msg)\n \t\t\tgetenv(\"GIT_REFLOG_ACTION\"), msg);\n \t}\n \tif (squash) {\n-\t\tsquash_message();\n+\t\tsquash_message(head_commit);\n \t} else {\n \t\tif (verbosity >= 0 && !merge_msg.len)\n \t\t\tprintf(_(\"No merge message -- not updating HEAD\\n\"));\n@@ -664,7 +664,7 @@ int try_merge_command(const char *strategy, size_t xopts_nr,\n }\n \n static int try_merge_strategy(const char *strategy, struct commit_list *common,\n-\t\t\t      const char *head_arg)\n+\t\t\t      struct commit *head, const char *head_arg)\n {\n \tint index_fd;\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n@@ -710,7 +710,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \t\t\tcommit_list_insert(j->item, &reversed);\n \n \t\tindex_fd = hold_locked_index(lock, 1);\n-\t\tclean = merge_recursive(&o, lookup_commit(head),\n+\t\tclean = merge_recursive(&o, head,\n \t\t\t\tremoteheads->item, reversed, &result);\n \t\tif (active_cache_changed &&\n \t\t\t\t(write_cache(index_fd, active_cache, active_nr) ||\n@@ -861,25 +861,26 @@ static void run_prepare_commit_msg(void)\n \tread_merge_msg();\n }\n \n-static int merge_trivial(void)\n+static int merge_trivial(struct commit *head)\n {\n \tunsigned char result_tree[20], result_commit[20];\n \tstruct commit_list *parent = xmalloc(sizeof(*parent));\n \n \twrite_tree_trivial(result_tree);\n \tprintf(_(\"Wonderful.\\n\"));\n-\tparent->item = lookup_commit(head);\n+\tparent->item = head;\n \tparent->next = xmalloc(sizeof(*parent->next));\n \tparent->next->item = remoteheads->item;\n \tparent->next->next = NULL;\n \trun_prepare_commit_msg();\n \tcommit_tree(merge_msg.buf, result_tree, parent, result_commit, NULL);\n-\tfinish(result_commit, \"In-index merge\");\n+\tfinish(head, result_commit, \"In-index merge\");\n \tdrop_save();\n \treturn 0;\n }\n \n-static int finish_automerge(struct commit_list *common,\n+static int finish_automerge(struct commit *head,\n+\t\t\t    struct commit_list *common,\n \t\t\t    unsigned char *result_tree,\n \t\t\t    const char *wt_strategy)\n {\n@@ -890,12 +891,12 @@ static int finish_automerge(struct commit_list *common,\n \tfree_commit_list(common);\n \tif (allow_fast_forward) {\n \t\tparents = remoteheads;\n-\t\tcommit_list_insert(lookup_commit(head), &parents);\n+\t\tcommit_list_insert(head, &parents);\n \t\tparents = reduce_heads(parents);\n \t} else {\n \t\tstruct commit_list **pptr = &parents;\n \n-\t\tpptr = &commit_list_insert(lookup_commit(head),\n+\t\tpptr = &commit_list_insert(head,\n \t\t\t\tpptr)->next;\n \t\tfor (j = remoteheads; j; j = j->next)\n \t\t\tpptr = &commit_list_insert(j->item, pptr)->next;\n@@ -905,7 +906,7 @@ static int finish_automerge(struct commit_list *common,\n \trun_prepare_commit_msg();\n \tcommit_tree(merge_msg.buf, result_tree, parents, result_commit, NULL);\n \tstrbuf_addf(&buf, \"Merge made by %s.\", wt_strategy);\n-\tfinish(result_commit, buf.buf);\n+\tfinish(head, result_commit, buf.buf);\n \tstrbuf_release(&buf);\n \tdrop_save();\n \treturn 0;\n@@ -939,7 +940,8 @@ static int suggest_conflicts(int renormalizing)\n \treturn 1;\n }\n \n-static struct commit *is_old_style_invocation(int argc, const char **argv)\n+static struct commit *is_old_style_invocation(int argc, const char **argv,\n+\t\t\t\t\t      const unsigned char *head)\n {\n \tstruct commit *second_token = NULL;\n \tif (argc > 2) {\n@@ -1012,9 +1014,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n {\n \tunsigned char result_tree[20];\n \tunsigned char stash[20];\n+\tunsigned char head_sha1[20];\n+\tstruct commit *head_commit;\n \tstruct strbuf buf = STRBUF_INIT;\n \tconst char *head_arg;\n-\tint flag, head_invalid = 0, i;\n+\tint flag, i;\n \tint best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;\n \tstruct commit_list *common = NULL;\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n@@ -1027,11 +1031,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\n \t * current branch.\n \t */\n-\tbranch = resolve_ref(\"HEAD\", head, 0, &flag);\n+\tbranch = resolve_ref(\"HEAD\", head_sha1, 0, &flag);\n \tif (branch && !prefixcmp(branch, \"refs/heads/\"))\n \t\tbranch += 11;\n-\tif (!branch || is_null_sha1(head))\n-\t\thead_invalid = 1;\n+\tif (!branch || is_null_sha1(head_sha1))\n+\t\thead_commit = NULL;\n+\telse {\n+\t\thead_commit = lookup_commit(head_sha1);\n+\t\tif (!head_commit)\n+\t\t\tdie(_(\"could not parse HEAD\"));\n+\t}\n \n \tgit_config(git_merge_config, NULL);\n \n@@ -1112,12 +1121,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * additional safety measure to check for it.\n \t */\n \n-\tif (!have_message && is_old_style_invocation(argc, argv)) {\n+\tif (!have_message && head_commit &&\n+\t    is_old_style_invocation(argc, argv, head_commit->object.sha1)) {\n \t\tstrbuf_addstr(&merge_msg, argv[0]);\n \t\thead_arg = argv[1];\n \t\targv += 2;\n \t\targc -= 2;\n-\t} else if (head_invalid) {\n+\t} else if (!head_commit) {\n \t\tstruct object *remote_head;\n \t\t/*\n \t\t * If the merged head is a valid one there is no reason\n@@ -1164,7 +1174,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\tif (head_invalid || !argc)\n+\tif (!head_commit || !argc)\n \t\tusage_with_options(builtin_merge_usage,\n \t\t\tbuiltin_merge_options);\n \n@@ -1205,17 +1215,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (!remoteheads->next)\n-\t\tcommon = get_merge_bases(lookup_commit(head),\n-\t\t\t\tremoteheads->item, 1);\n+\t\tcommon = get_merge_bases(head_commit, remoteheads->item, 1);\n \telse {\n \t\tstruct commit_list *list = remoteheads;\n-\t\tcommit_list_insert(lookup_commit(head), &list);\n+\t\tcommit_list_insert(head_commit, &list);\n \t\tcommon = get_octopus_merge_bases(list);\n \t\tfree(list);\n \t}\n \n-\tupdate_ref(\"updating ORIG_HEAD\", \"ORIG_HEAD\", head, NULL, 0,\n-\t\tDIE_ON_ERR);\n+\tupdate_ref(\"updating ORIG_HEAD\", \"ORIG_HEAD\", head_commit->object.sha1,\n+\t\t   NULL, 0, DIE_ON_ERR);\n \n \tif (!common)\n \t\t; /* No common ancestors found. We need a real merge. */\n@@ -1229,13 +1238,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\treturn 0;\n \t} else if (allow_fast_forward && !remoteheads->next &&\n \t\t\t!common->next &&\n-\t\t\t!hashcmp(common->item->object.sha1, head)) {\n+\t\t\t!hashcmp(common->item->object.sha1, head_commit->object.sha1)) {\n \t\t/* Again the most common case of merging one remote. */\n \t\tstruct strbuf msg = STRBUF_INIT;\n \t\tstruct object *o;\n \t\tchar hex[41];\n \n-\t\tstrcpy(hex, find_unique_abbrev(head, DEFAULT_ABBREV));\n+\t\tstrcpy(hex, find_unique_abbrev(head_commit->object.sha1, DEFAULT_ABBREV));\n \n \t\tif (verbosity >= 0)\n \t\t\tprintf(_(\"Updating %s..%s\\n\"),\n@@ -1251,10 +1260,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tif (!o)\n \t\t\treturn 1;\n \n-\t\tif (checkout_fast_forward(head, remoteheads->item->object.sha1))\n+\t\tif (checkout_fast_forward(head_commit->object.sha1, remoteheads->item->object.sha1))\n \t\t\treturn 1;\n \n-\t\tfinish(o->sha1, msg.buf);\n+\t\tfinish(head_commit, o->sha1, msg.buf);\n \t\tdrop_save();\n \t\treturn 0;\n \t} else if (!remoteheads->next && common->next)\n@@ -1274,8 +1283,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t\tgit_committer_info(IDENT_ERROR_ON_NO_NAME);\n \t\t\tprintf(_(\"Trying really trivial in-index merge...\\n\"));\n \t\t\tif (!read_tree_trivial(common->item->object.sha1,\n-\t\t\t\t\thead, remoteheads->item->object.sha1))\n-\t\t\t\treturn merge_trivial();\n+\t\t\t\t\thead_commit->object.sha1, remoteheads->item->object.sha1))\n+\t\t\t\treturn merge_trivial(head_commit);\n \t\t\tprintf(_(\"Nope.\\n\"));\n \t\t}\n \t} else {\n@@ -1294,8 +1303,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t\t * merge_bases again, otherwise \"git merge HEAD^\n \t\t\t * HEAD^^\" would be missed.\n \t\t\t */\n-\t\t\tcommon_one = get_merge_bases(lookup_commit(head),\n-\t\t\t\tj->item, 1);\n+\t\t\tcommon_one = get_merge_bases(head_commit, j->item, 1);\n \t\t\tif (hashcmp(common_one->item->object.sha1,\n \t\t\t\tj->item->object.sha1)) {\n \t\t\t\tup_to_date = 0;\n@@ -1333,7 +1341,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tint ret;\n \t\tif (i) {\n \t\t\tprintf(_(\"Rewinding the tree to pristine...\\n\"));\n-\t\t\trestore_state(stash);\n+\t\t\trestore_state(head_commit->object.sha1, stash);\n \t\t}\n \t\tif (use_strategies_nr != 1)\n \t\t\tprintf(_(\"Trying merge strategy %s...\\n\"),\n@@ -1345,7 +1353,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\twt_strategy = use_strategies[i]->name;\n \n \t\tret = try_merge_strategy(use_strategies[i]->name,\n-\t\t\tcommon, head_arg);\n+\t\t\t\t\t common, head_commit, head_arg);\n \t\tif (!option_commit && !ret) {\n \t\t\tmerge_was_ok = 1;\n \t\t\t/*\n@@ -1387,14 +1395,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * auto resolved the merge cleanly.\n \t */\n \tif (automerge_was_ok)\n-\t\treturn finish_automerge(common, result_tree, wt_strategy);\n+\t\treturn finish_automerge(head_commit, common, result_tree,\n+\t\t\t\t\twt_strategy);\n \n \t/*\n \t * Pick the result from the best strategy and have the user fix\n \t * it up.\n \t */\n \tif (!best_strategy) {\n-\t\trestore_state(stash);\n+\t\trestore_state(head_commit->object.sha1, stash);\n \t\tif (use_strategies_nr > 1)\n \t\t\tfprintf(stderr,\n \t\t\t\t_(\"No merge strategy handled the merge.\\n\"));\n@@ -1406,14 +1415,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t; /* We already have its result in the working tree. */\n \telse {\n \t\tprintf(_(\"Rewinding the tree to pristine...\\n\"));\n-\t\trestore_state(stash);\n+\t\trestore_state(head_commit->object.sha1, stash);\n \t\tprintf(_(\"Using the %s to prepare resolving by hand.\\n\"),\n \t\t\tbest_strategy);\n-\t\ttry_merge_strategy(best_strategy, common, head_arg);\n+\t\ttry_merge_strategy(best_strategy, common, head_commit, head_arg);\n \t}\n \n \tif (squash)\n-\t\tfinish(NULL, NULL);\n+\t\tfinish(head_commit, NULL, NULL);\n \telse {\n \t\tint fd;\n \t\tstruct commit_list *j;\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"175690","messageId":"1316260665-1728-4-git-send-email-pclouds@gmail.com","threadId":"28414","inReplyTo":"1316260665-1728-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v5 4/4] Accept tags in HEAD or MERGE_HEAD","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-09-17T11:57:45Z","receivedAt":"2011-09-17T11:57:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"HEAD and MERGE_HEAD (among other branch tips) should never hold a\ntag. That can only be caused by broken tools and is cumbersome to fix\nby an end user with:\n\n  $ git update-ref HEAD $(git rev-parse HEAD^{commit})\n\nwhich may look like a magic to a new person.\n\nBe easy, warn users (so broken tools can be fixed if they bother to\nreport) and move on.\n\nBe robust, if the given SHA-1 cannot be resolved to a commit object,\ndie (therefore return value is always valid).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/commit.c        |    6 ++++--\n builtin/fmt-merge-msg.c |    2 +-\n builtin/merge.c         |    7 ++-----\n commit.c                |   12 ++++++++++++\n commit.h                |    7 +++++++\n http-push.c             |    8 ++++----\n revision.c              |    6 ++++--\n 7 files changed, 34 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 1a65319..402eb5a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1396,7 +1396,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tif (get_sha1(\"HEAD\", sha1))\n \t\tcurrent_head = NULL;\n \telse {\n-\t\tcurrent_head = lookup_commit(sha1);\n+\t\tcurrent_head = lookup_commit_or_die(sha1, \"HEAD\");\n \t\tif (!current_head || parse_commit(current_head))\n \t\t\tdie(_(\"could not parse HEAD commit\"));\n \t}\n@@ -1431,6 +1431,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t\tpptr = &commit_list_insert(c->item, pptr)->next;\n \t} else if (whence == FROM_MERGE) {\n \t\tstruct strbuf m = STRBUF_INIT;\n+\t\tstruct commit *commit;\n \t\tFILE *fp;\n \n \t\tif (!reflog_msg)\n@@ -1444,7 +1445,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t\tunsigned char sha1[20];\n \t\t\tif (get_sha1_hex(m.buf, sha1) < 0)\n \t\t\t\tdie(_(\"Corrupt MERGE_HEAD file (%s)\"), m.buf);\n-\t\t\tpptr = &commit_list_insert(lookup_commit(sha1), pptr)->next;\n+\t\t\tcommit = lookup_commit_or_die(sha1, \"MERGE_HEAD\");\n+\t\t\tpptr = &commit_list_insert(commit, pptr)->next;\n \t\t}\n \t\tfclose(fp);\n \t\tstrbuf_release(&m);\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 7581632..7e2f225 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -293,7 +293,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,\n \t\tstruct commit *head;\n \t\tstruct rev_info rev;\n \n-\t\thead = lookup_commit(head_sha1);\n+\t\thead = lookup_commit_or_die(head_sha1, \"HEAD\");\n \t\tinit_revisions(&rev, NULL);\n \t\trev.commit_format = CMIT_FMT_ONELINE;\n \t\trev.ignore_merges = 1;\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex f5eb3f5..9567d60 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1036,11 +1036,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tbranch += 11;\n \tif (!branch || is_null_sha1(head_sha1))\n \t\thead_commit = NULL;\n-\telse {\n-\t\thead_commit = lookup_commit(head_sha1);\n-\t\tif (!head_commit)\n-\t\t\tdie(_(\"could not parse HEAD\"));\n-\t}\n+\telse\n+\t\thead_commit = lookup_commit_or_die(head_sha1, \"HEAD\");\n \n \tgit_config(git_merge_config, NULL);\n \ndiff --git a/commit.c b/commit.c\nindex ac337c7..50fcf96 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -39,6 +39,18 @@ struct commit *lookup_commit_reference(const unsigned char *sha1)\n \treturn lookup_commit_reference_gently(sha1, 0);\n }\n \n+struct commit *lookup_commit_or_die(const unsigned char *sha1, const char *ref_name)\n+{\n+\tstruct commit *c = lookup_commit_reference(sha1);\n+\tif (!c)\n+\t\tdie(_(\"could not parse %s\"), ref_name);\n+\tif (hashcmp(sha1, c->object.sha1)) {\n+\t\twarning(_(\"%s %s is not a commit!\"),\n+\t\t\tref_name, sha1_to_hex(sha1));\n+\t}\n+\treturn c;\n+}\n+\n struct commit *lookup_commit(const unsigned char *sha1)\n {\n \tstruct object *obj = lookup_object(sha1);\ndiff --git a/commit.h b/commit.h\nindex a2d571b..044134a 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -37,6 +37,13 @@ struct commit *lookup_commit_reference(const unsigned char *sha1);\n struct commit *lookup_commit_reference_gently(const unsigned char *sha1,\n \t\t\t\t\t      int quiet);\n struct commit *lookup_commit_reference_by_name(const char *name);\n+/*\n+ * Look sha1 up for a commit, defer if needed. If dereference occurs,\n+ * update \"sha1\" for consistency with retval->object.sha1. Also warn\n+ * users this case because it is expected that sha1 points directly to\n+ * a commit.\n+ */\n+struct commit *lookup_commit_or_die(const unsigned char *sha1, const char *ref_name);\n \n int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long size);\n int parse_commit(struct commit *item);\ndiff --git a/http-push.c b/http-push.c\nindex 28bfe76..d432b30 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1606,10 +1606,10 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)\n \tstrbuf_release(&buffer);\n }\n \n-static int verify_merge_base(unsigned char *head_sha1, unsigned char *branch_sha1)\n+static int verify_merge_base(unsigned char *head_sha1, struct ref *remote)\n {\n-\tstruct commit *head = lookup_commit(head_sha1);\n-\tstruct commit *branch = lookup_commit(branch_sha1);\n+\tstruct commit *head = lookup_commit_or_die(head_sha1, \"HEAD\");\n+\tstruct commit *branch = lookup_commit_or_die(remote->old_sha1, remote->name);\n \tstruct commit_list *merge_bases = get_merge_bases(head, branch, 1);\n \n \treturn (merge_bases && !merge_bases->next && merge_bases->item == branch);\n@@ -1680,7 +1680,7 @@ static int delete_remote_branch(const char *pattern, int force)\n \t\t\treturn error(\"Remote branch %s resolves to object %s\\nwhich does not exist locally, perhaps you need to fetch?\", remote_ref->name, sha1_to_hex(remote_ref->old_sha1));\n \n \t\t/* Remote branch must be an ancestor of remote HEAD */\n-\t\tif (!verify_merge_base(head_sha1, remote_ref->old_sha1)) {\n+\t\tif (!verify_merge_base(head_sha1, remote_ref)) {\n \t\t\treturn error(\"The branch '%s' is not an ancestor \"\n \t\t\t\t     \"of your current HEAD.\\n\"\n \t\t\t\t     \"If you are sure you want to delete it,\"\ndiff --git a/revision.c b/revision.c\nindex c46cfaa..5e057a0 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -986,10 +986,12 @@ static void prepare_show_merge(struct rev_info *revs)\n \tconst char **prune = NULL;\n \tint i, prune_num = 1; /* counting terminating NULL */\n \n-\tif (get_sha1(\"HEAD\", sha1) || !(head = lookup_commit(sha1)))\n+\tif (get_sha1(\"HEAD\", sha1))\n \t\tdie(\"--merge without HEAD?\");\n-\tif (get_sha1(\"MERGE_HEAD\", sha1) || !(other = lookup_commit(sha1)))\n+\thead = lookup_commit_or_die(sha1, \"HEAD\");\n+\tif (get_sha1(\"MERGE_HEAD\", sha1))\n \t\tdie(\"--merge without MERGE_HEAD?\");\n+\tother = lookup_commit_or_die(sha1, \"MERGE_HEAD\");\n \tadd_pending_object(revs, &head->object, \"HEAD\");\n \tadd_pending_object(revs, &other->object, \"MERGE_HEAD\");\n \tbases = get_merge_bases(head, other, 1);\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"175758","messageId":"7vsjntbjto.fsf@alter.siamese.dyndns.org","threadId":"28414","inReplyTo":"1316260665-1728-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v5 4/4] Accept tags in HEAD or MERGE_HEAD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-18T21:51:31Z","receivedAt":"2011-09-18T21:51:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n\n> HEAD and MERGE_HEAD (among other branch tips) should never hold a\n> tag. That can only be caused by broken tools and is cumbersome to fix\n> by an end user with:\n>\n>   $ git update-ref HEAD $(git rev-parse HEAD^{commit})\n>\n> which may look like a magic to a new person.\n> ...\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n\nNicely done.\n\nJust a few micronits.\n\n> diff --git a/commit.h b/commit.h\n> ...\n> +/*\n> + * Look sha1 up for a commit, defer if needed. If dereference occurs,\n\nThat's s/defer/deref/;\n\n> + * update \"sha1\" for consistency with retval->object.sha1. Also warn\n> + * users this case because it is expected that sha1 points directly to\n> + * a commit.\n> + */\n> +struct commit *lookup_commit_or_die(const unsigned char *sha1, const char *ref_name);\n\nThis no longer updates sha1 now it is a pointer to a const arena.\nI'll update this part as follows, and queue the result.\n\n/*\n * Look up object named by \"sha1\", dereference tag as necessary, \n * get a commit and return it. If \"sha1\" does not dereference to\n * a commit, use ref_name to report an error and die.\n */\n\nThanks.\n"}]}