git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] commit: accept tag objects in HEAD/MERGE_HEAD

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Aug 17, 2011, 01:42 UTC
Message-ID
<1313545369-7096-1-git-send-email-pclouds@gmail.com>
In-Reply-To
<1313422716-26432-1-git-send-email-pclouds@gmail.com>

HEAD, MERGE_HEAD (or any other branches) should only have SHA-1 of a commit object. However broken tools can put a tag object there. While it's wrong, it'd be better to tolerate the situation and move on ("commit" is an often used operation, unable to commit could be bad).

In worse cases, where we cannot even resolve HEAD to a commit, die() instead of segfault.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/commit.c |   15 +++++++++------
 commit.c         |   12 ++++++++++++
 commit.h         |    1 +
 3 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 2088b6b..f327595 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1387,6 +1387,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	unsigned char commit_sha1[20];
 	struct ref_lock *ref_lock;
 	struct commit_list *parents = NULL, **pptr = &parents;
+	struct commit *commit;
 	struct stat statbuf;
 	int allow_fast_forward = 1;
 	struct wt_status s;
@@ -1423,12 +1424,11 @@ 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))
+		commit = lookup_expect_commit(head_sha1, "HEAD");
+		if (parse_commit(commit))
 			die(_("could not parse HEAD commit"));
 
 		for (c = commit->parents; c; c = c->next)
@@ -1439,7 +1439,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 		if (!reflog_msg)
 			reflog_msg = "commit (merge)";
-		pptr = &commit_list_insert(lookup_commit(head_sha1), pptr)->next;
+		commit = lookup_expect_commit(head_sha1, "HEAD");
+		pptr = &commit_list_insert(commit, pptr)->next;
 		fp = fopen(git_path("MERGE_HEAD"), "r");
 		if (fp == NULL)
 			die_errno(_("could not open '%s' for reading"),
@@ -1448,7 +1449,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);
@@ -1465,7 +1467,8 @@ 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;
+		commit = lookup_expect_commit(head_sha1, "HEAD");
+		pptr = &commit_list_insert(commit, pptr)->next;
 	}
 
 	/* Finally, get the commit message */
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 feb809f..0e36fd0 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);
-- 
1.7.4.74.g639db
Previous: Nguyen Thai Ngoc DuyNext: Junio C Hamano
Message 6 of 21 in “commit: check return value of lookup_commit()”
  1. commit: check return value of lookup_commit()Nguyễn Thái Ngọc Duy, Aug 15, 2011
  2. Junio C HamanoAug 15, 2011
  3. Nguyen Thai Ngoc DuyAug 16, 2011
  4. Junio C HamanoAug 16, 2011
  5. Nguyen Thai Ngoc DuyAug 17, 2011
  6. commit: accept tag objects in HEAD/MERGE_HEADNguyễn Thái Ngọc Duy, Aug 17, 2011
  7. Junio C HamanoAug 17, 2011
  8. Nguyen Thai Ngoc DuyAug 18, 2011
  9. Accept tags in HEAD or MERGE_HEADNguyễn Thái Ngọc Duy, Aug 18, 2011
  10. Junio C HamanoAug 18, 2011
  11. Nguyen Thai Ngoc DuyAug 19, 2011
  12. 1/4 commit: remove global variable head_sha1[]Nguyễn Thái Ngọc Duy, Aug 19, 2011
  13. 2/4 merge: keep stash[] a local variableNguyễn Thái Ngọc Duy, Aug 19, 2011
  14. Junio C HamanoAug 19, 2011
  15. 3/4 merge: remove global variable head[]Nguyễn Thái Ngọc Duy, Aug 19, 2011
  16. Junio C HamanoAug 23, 2011
  17. 4/4 Accept tags in HEAD or MERGE_HEADNguyễn Thái Ngọc Duy, Aug 19, 2011
  18. Junio C HamanoAug 19, 2011
  19. Nguyen Thai Ngoc DuyAug 20, 2011
  20. Junio C HamanoAug 19, 2011
  21. Nguyen Thai Ngoc DuyAug 20, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.