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>
---
Junio's point, that if HEAD holds a tag, then head_sha1 and head->object.sha1
in statement [1] are different, is entirely correct. However, favoring
head->object.sha1 over head_sha1 is not enough. The variable head_sha1 is
still there. Somewhere, some time, people may misuse it.
[1] head = lookup_commit_or_die(head_sha1, "HEAD");
Better update head_sha1 to new value in this case, which is the new change
in lookup_commit_or_die().
Or maybe a better approach is
int get_commit_sha1(const char *ref, unsigned char *sha1);
where it only returns zero if it can resolve to SHA-1 of a commit.
builtin/commit.c | 11 +++++------
builtin/fmt-merge-msg.c | 2 +-
builtin/merge.c | 7 ++-----
commit.c | 19 +++++++++++++++++++
commit.h | 1 +
http-push.c | 8 ++++----
revision.c | 6 ++++--
7 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index c9c4ea5..72e2cc5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1393,11 +1393,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
if (get_sha1("HEAD", head_sha1))
initial_commit = 1;
- else {
- head_commit = lookup_commit(head_sha1);
- if (!head_commit || parse_commit(head_commit))
- die(_("could not parse HEAD commit"));
- }
+ else
+ head_commit = lookup_commit_or_die(head_sha1, "HEAD");
if (s.use_color == -1)
s.use_color = git_use_color_default;
@@ -1433,6 +1430,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)
@@ -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_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 b7260f5..39d9ac8 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1034,11 +1034,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
branch = resolve_ref("HEAD", head, 0, &flag);
if (branch && !prefixcmp(branch, "refs/heads/"))
branch += 11;
- if (!is_null_sha1(head)) {
- head_commit = lookup_commit(head);
- if (!head_commit)
- die(_("could not parse HEAD"));
- }
+ if (!is_null_sha1(head))
+ head_commit = lookup_commit_or_die(head, "HEAD");
git_config(git_merge_config, NULL);
diff --git a/commit.c b/commit.c
index ac337c7..9e7f7ef 100644
--- a/commit.c
+++ b/commit.c
@@ -39,6 +39,25 @@ struct commit *lookup_commit_reference(const unsigned char *sha1)
return lookup_commit_reference_gently(sha1, 0);
}
+/*
+ * Look sha1 up for a commit, defer if needed. If deference 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(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));
+ hashcpy(sha1, c->object.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..a098b4c 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_commit_or_die(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..7ccff8f 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.4.74.g639db
--
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