[PATCH v3] Accept tags in HEAD or MERGE_HEAD

["Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>]

HEAD and MERGE_HEAD (among other branch tips) should never hold a
tag. That can only be caused by broken tools and is cumbersome to fix
by an end user with:

  $ git update-ref HEAD $(git rev-parse HEAD^{commit})

which may look like a magic to a new person.

Be easy, warn users (so broken tools can be fixed) and move on.

Be robust, if the given SHA-1 cannot be resolved to a commit object,
die (therefore return value is always valid).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c        |   25 ++++++++++++-------------
 builtin/fmt-merge-msg.c |    2 +-
 builtin/merge.c         |   19 +++++++++++--------
 commit.c                |   12 ++++++++++++
 commit.h                |    1 +
 http-push.c             |    8 ++++----
 revision.c              |    6 ++++--
 7 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index cb73857..f78b449 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -63,6 +63,7 @@ N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\
 "Otherwise, please use 'git reset'\n");
 
 static unsigned char head_sha1[20];
+static struct commit *head_commit;
 
 static const char *use_message_buffer;
 static const char commit_editmsg[] = "COMMIT_EDITMSG";
@@ -518,11 +519,8 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	return s->commitable;
 }
 
-static int is_a_merge(const unsigned char *sha1)
+static int is_a_merge(struct commit *commit)
 {
-	struct commit *commit = lookup_commit(sha1);
-	if (!commit || parse_commit(commit))
-		die(_("could not parse HEAD commit"));
 	return !!(commit->parents && commit->parents->next);
 }
 
@@ -848,7 +846,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	 * empty due to conflict resolution, which the user should okay.
 	 */
 	if (!commitable && whence != FROM_MERGE && !allow_empty &&
-	    !(amend && is_a_merge(head_sha1))) {
+	    !(amend && is_a_merge(head_commit))) {
 		run_status(stdout, index_file, prefix, 0, s);
 		if (amend)
 			fputs(_(empty_amend_advice), stderr);
@@ -1028,6 +1026,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (get_sha1("HEAD", head_sha1))
 		initial_commit = 1;
+	else
+		head_commit = lookup_expect_commit(head_sha1, "HEAD");
+
 
 	/* Sanity check options */
 	if (amend && initial_commit)
@@ -1421,23 +1422,20 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			reflog_msg = "commit (initial)";
 	} else if (amend) {
 		struct commit_list *c;
-		struct commit *commit;
 
 		if (!reflog_msg)
 			reflog_msg = "commit (amend)";
-		commit = lookup_commit(head_sha1);
-		if (!commit || parse_commit(commit))
-			die(_("could not parse HEAD commit"));
 
-		for (c = commit->parents; c; c = c->next)
+		for (c = head_commit->parents; c; c = c->next)
 			pptr = &commit_list_insert(c->item, pptr)->next;
 	} else if (whence == FROM_MERGE) {
 		struct strbuf m = STRBUF_INIT;
+		struct commit *commit;
 		FILE *fp;
 
 		if (!reflog_msg)
 			reflog_msg = "commit (merge)";
-		pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
+		pptr = &commit_list_insert(head_commit, pptr)->next;
 		fp = fopen(git_path("MERGE_HEAD"), "r");
 		if (fp == NULL)
 			die_errno(_("could not open '%s' for reading"),
@@ -1446,7 +1444,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			unsigned char sha1[20];
 			if (get_sha1_hex(m.buf, sha1) < 0)
 				die(_("Corrupt MERGE_HEAD file (%s)"), m.buf);
-			pptr = &commit_list_insert(lookup_commit(sha1), pptr)->next;
+			commit = lookup_expect_commit(sha1, "MERGE_HEAD");
+			pptr = &commit_list_insert(commit, pptr)->next;
 		}
 		fclose(fp);
 		strbuf_release(&m);
@@ -1463,7 +1462,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 			reflog_msg = (whence == FROM_CHERRY_PICK)
 					? "commit (cherry-pick)"
 					: "commit";
-		pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
+		pptr = &commit_list_insert(head_commit, pptr)->next;
 	}
 
 	/* Finally, get the commit message */
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 7581632..1a31b64 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -293,7 +293,7 @@ static int do_fmt_merge_msg(int merge_title, struct strbuf *in,
 		struct commit *head;
 		struct rev_info rev;
 
-		head = lookup_commit(head_sha1);
+		head = lookup_expect_commit(head_sha1, "HEAD");
 		init_revisions(&rev, NULL);
 		rev.commit_format = CMIT_FMT_ONELINE;
 		rev.ignore_merges = 1;
diff --git a/builtin/merge.c b/builtin/merge.c
index 325891e..22e98e8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -51,6 +51,7 @@ static int allow_trivial = 1, have_message;
 static struct strbuf merge_msg;
 static struct commit_list *remoteheads;
 static unsigned char head[20], stash[20];
+static struct commit* head_commit;
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char **xopts;
@@ -326,7 +327,7 @@ static void squash_message(void)
 	rev.ignore_merges = 1;
 	rev.commit_format = CMIT_FMT_MEDIUM;
 
-	commit = lookup_commit(head);
+	commit = head_commit;
 	commit->object.flags |= UNINTERESTING;
 	add_pending_object(&rev, &commit->object, NULL);
 
@@ -709,7 +710,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 			commit_list_insert(j->item, &reversed);
 
 		index_fd = hold_locked_index(lock, 1);
-		clean = merge_recursive(&o, lookup_commit(head),
+		clean = merge_recursive(&o, head_commit,
 				remoteheads->item, reversed, &result);
 		if (active_cache_changed &&
 				(write_cache(index_fd, active_cache, active_nr) ||
@@ -867,7 +868,7 @@ static int merge_trivial(void)
 
 	write_tree_trivial(result_tree);
 	printf(_("Wonderful.\n"));
-	parent->item = lookup_commit(head);
+	parent->item = head_commit;
 	parent->next = xmalloc(sizeof(*parent->next));
 	parent->next->item = remoteheads->item;
 	parent->next->next = NULL;
@@ -889,12 +890,12 @@ static int finish_automerge(struct commit_list *common,
 	free_commit_list(common);
 	if (allow_fast_forward) {
 		parents = remoteheads;
-		commit_list_insert(lookup_commit(head), &parents);
+		commit_list_insert(head_commit, &parents);
 		parents = reduce_heads(parents);
 	} else {
 		struct commit_list **pptr = &parents;
 
-		pptr = &commit_list_insert(lookup_commit(head),
+		pptr = &commit_list_insert(head_commit,
 				pptr)->next;
 		for (j = remoteheads; j; j = j->next)
 			pptr = &commit_list_insert(j->item, pptr)->next;
@@ -1030,6 +1031,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		branch += 11;
 	if (is_null_sha1(head))
 		head_invalid = 1;
+	else
+		head_commit = lookup_expect_commit(head, "HEAD");
 
 	git_config(git_merge_config, NULL);
 
@@ -1203,11 +1206,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	}
 
 	if (!remoteheads->next)
-		common = get_merge_bases(lookup_commit(head),
+		common = get_merge_bases(head_commit,
 				remoteheads->item, 1);
 	else {
 		struct commit_list *list = remoteheads;
-		commit_list_insert(lookup_commit(head), &list);
+		commit_list_insert(head_commit, &list);
 		common = get_octopus_merge_bases(list);
 		free(list);
 	}
@@ -1292,7 +1295,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			 * merge_bases again, otherwise "git merge HEAD^
 			 * HEAD^^" would be missed.
 			 */
-			common_one = get_merge_bases(lookup_commit(head),
+			common_one = get_merge_bases(head_commit,
 				j->item, 1);
 			if (hashcmp(common_one->item->object.sha1,
 				j->item->object.sha1)) {
diff --git a/commit.c b/commit.c
index ac337c7..dc22695 100644
--- a/commit.c
+++ b/commit.c
@@ -39,6 +39,18 @@ struct commit *lookup_commit_reference(const unsigned char *sha1)
 	return lookup_commit_reference_gently(sha1, 0);
 }
 
+struct commit *lookup_expect_commit(const unsigned char *sha1,
+				    const char *ref_name)
+{
+	struct commit *c = lookup_commit_reference(sha1);
+	if (!c)
+		die(_("could not parse %s"), ref_name);
+	if (hashcmp(sha1, c->object.sha1))
+		warning(_("%s %s is not a commit!"),
+			ref_name, sha1_to_hex(sha1));
+	return c;
+}
+
 struct commit *lookup_commit(const unsigned char *sha1)
 {
 	struct object *obj = lookup_object(sha1);
diff --git a/commit.h b/commit.h
index a2d571b..f36c913 100644
--- a/commit.h
+++ b/commit.h
@@ -37,6 +37,7 @@ struct commit *lookup_commit_reference(const unsigned char *sha1);
 struct commit *lookup_commit_reference_gently(const unsigned char *sha1,
 					      int quiet);
 struct commit *lookup_commit_reference_by_name(const char *name);
+struct commit *lookup_expect_commit(const unsigned char *sha1, const char *ref_name);
 
 int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long size);
 int parse_commit(struct commit *item);
diff --git a/http-push.c b/http-push.c
index 6e8f6d0..31297af 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1606,10 +1606,10 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 	strbuf_release(&buffer);
 }
 
-static int verify_merge_base(unsigned char *head_sha1, unsigned char *branch_sha1)
+static int verify_merge_base(unsigned char *head_sha1, struct ref *remote)
 {
-	struct commit *head = lookup_commit(head_sha1);
-	struct commit *branch = lookup_commit(branch_sha1);
+	struct commit *head = lookup_expect_commit(head_sha1, "HEAD");
+	struct commit *branch = lookup_expect_commit(remote->old_sha1, remote->name);
 	struct commit_list *merge_bases = get_merge_bases(head, branch, 1);
 
 	return (merge_bases && !merge_bases->next && merge_bases->item == branch);
@@ -1680,7 +1680,7 @@ static int delete_remote_branch(const char *pattern, int force)
 			return 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));
 
 		/* Remote branch must be an ancestor of remote HEAD */
-		if (!verify_merge_base(head_sha1, remote_ref->old_sha1)) {
+		if (!verify_merge_base(head_sha1, remote_ref)) {
 			return error("The branch '%s' is not an ancestor "
 				     "of your current HEAD.\n"
 				     "If you are sure you want to delete it,"
diff --git a/revision.c b/revision.c
index c46cfaa..f926412 100644
--- a/revision.c
+++ b/revision.c
@@ -986,10 +986,12 @@ static void prepare_show_merge(struct rev_info *revs)
 	const char **prune = NULL;
 	int i, prune_num = 1; /* counting terminating NULL */
 
-	if (get_sha1("HEAD", sha1) || !(head = lookup_commit(sha1)))
+	if (get_sha1("HEAD", sha1))
 		die("--merge without HEAD?");
-	if (get_sha1("MERGE_HEAD", sha1) || !(other = lookup_commit(sha1)))
+	head = lookup_expect_commit(sha1, "HEAD");
+	if (get_sha1("MERGE_HEAD", sha1))
 		die("--merge without MERGE_HEAD?");
+	other = lookup_expect_commit(sha1, "MERGE_HEAD");
 	add_pending_object(revs, &head->object, "HEAD");
 	add_pending_object(revs, &other->object, "MERGE_HEAD");
 	bases = get_merge_bases(head, other, 1);
-- 
1.7.4.74.g639db