surprising error message in parse_opt_with_commit

When I typed 'git branch --contains efabdfb' on a machine today, I was
surprised to receive this error message: "error: malformed object name
efabdfb"

I would have expected instead to receive the message: "no such commit: efabdfb".

I went hunting through the source code and found the origination point
of the error:

/parse-options.c
 610 int parse_opt_with_commit(const struct option *opt, const char
*arg, int unset)
 611 {
 612 	unsigned char sha1[20];
 613 	struct commit *commit;
 614
 615 	if (!arg)
 616 		return -1;
 617 	if (get_sha1(arg, sha1))
 618 		return error("malformed object name %s", arg);
 619 	commit = lookup_commit_reference(sha1);
 620 	if (!commit)
 621 		return error("no such commit %s", arg);
 622 	commit_list_insert(commit, opt->value);
 623 	return 0;
 624 }

It appears the get_sha1 call is returning true, causing the 'malformed
object name' error to be returned.  However, it seems that ideally
since efabdfb is not malformed (it would be a valid ref if it
existed), the execution path should continue to line 619, receive no
commit, and fail on 621.

Am I off base here?

Tim Harper [off-list ref] wrote:

 610 int parse_opt_with_commit(const struct option *opt, const char
*arg, int unset)
 611 {
 612 	unsigned char sha1[20];
 613 	struct commit *commit;
 614
 615 	if (!arg)
 616 		return -1;
 617 	if (get_sha1(arg, sha1))
 618 		return error("malformed object name %s", arg);
 619 	commit = lookup_commit_reference(sha1);
 620 	if (!commit)
 621 		return error("no such commit %s", arg);
 622 	commit_list_insert(commit, opt->value);
 623 	return 0;
 624 }

It appears the get_sha1 call is returning true, causing the 'malformed
object name' error to be returned.  However, it seems that ideally
since efabdfb is not malformed (it would be a valid ref if it
existed), the execution path should continue to line 619, receive no
commit, and fail on 621.

get_sha1 is responsible for expanding an abbreviated ID to the
full ID.  If it can't do the expansion, it errors out.  The code
is correct as-is, though the error message on 618 is a bit odd.

-- 
Shawn.

When running the command 'git branch --contains efabdfb' on a repository that doesn't yet have efabdfb, git reports: "malformed object name efabdfb". To the uninitiated, this makes little sense (as far as they are concerned, efabdfb is perfectly formed).

This commit changes the message to "malformed object name or no such commit: efabdfb"
---
 parse-options.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 3b71fbb..95eb1c4 100644
--- a/parse-options.c
+++ b/parse-options.c

@@ -615,7 +615,7 @@ int parse_opt_with_commit(const struct option *opt, const char *arg, int unset)
 	if (!arg)
 		return -1;
 	if (get_sha1(arg, sha1))
-		return error("malformed object name %s", arg);
+		return error("malformed object name or no such commit: %s", arg);
 	commit = lookup_commit_reference(sha1);
 	if (!commit)
 		return error("no such commit %s", arg);

-- 
1.6.4

On Thu, Aug 6, 2009 at 1:53 PM, Tim Harper[off-list ref] wrote:

Does nobody think this is a good idea?