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 if they bother to report) 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@xxxxxxxxx> --- builtin/commit.c | 6 ++++-- builtin/fmt-merge-msg.c | 2 +- builtin/merge.c | 7 ++----- commit.c | 12 ++++++++++++ commit.h | 7 +++++++ http-push.c | 8 ++++---- revision.c | 6 ++++-- 7 files changed, 34 insertions(+), 14 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 1a65319..402eb5a 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1396,7 +1396,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) if (get_sha1("HEAD", sha1)) current_head = NULL; else { - current_head = lookup_commit(sha1); + current_head = lookup_commit_or_die(sha1, "HEAD"); if (!current_head || parse_commit(current_head)) die(_("could not parse HEAD commit")); } @@ -1431,6 +1431,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) 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) @@ -1444,7 +1445,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_commit_or_die(sha1, "MERGE_HEAD"); + pptr = &commit_list_insert(commit, pptr)->next; } fclose(fp); strbuf_release(&m); diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 7581632..7e2f225 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_commit_or_die(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 f5eb3f5..9567d60 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1036,11 +1036,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix) branch += 11; if (!branch || is_null_sha1(head_sha1)) head_commit = NULL; - else { - head_commit = lookup_commit(head_sha1); - if (!head_commit) - die(_("could not parse HEAD")); - } + else + head_commit = lookup_commit_or_die(head_sha1, "HEAD"); git_config(git_merge_config, NULL); diff --git a/commit.c b/commit.c index ac337c7..50fcf96 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_commit_or_die(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..044134a 100644 --- a/commit.h +++ b/commit.h @@ -37,6 +37,13 @@ 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); +/* + * Look sha1 up for a commit, defer if needed. If dereference occurs, + * update "sha1" for consistency with retval->object.sha1. Also warn + * users this case because it is expected that sha1 points directly to + * a commit. + */ +struct commit *lookup_commit_or_die(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 28bfe76..d432b30 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_commit_or_die(head_sha1, "HEAD"); + struct commit *branch = lookup_commit_or_die(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..5e057a0 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_commit_or_die(sha1, "HEAD"); + if (get_sha1("MERGE_HEAD", sha1)) die("--merge without MERGE_HEAD?"); + other = lookup_commit_or_die(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.3.1.256.g2539c.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html