Skip to contentSkip to conversation

git cat-file -e behavior

DISCUSSION Michal Čihař 2 participants 5 messages Jun 15, 2016

lore
Share & tools
lore

Michal Čihař

author
#0 lore raw

Hi

the documentation for "git cat-file -e" says:

Quoted 1 line

Suppress all output; instead exit with zero status if <object> exists

and is a valid object.

However running it on non existing object will complain "fatal: Not a valid object name".

$ git cat-file -e master:README.rst
$ echo $?
0
$ git cat-file -e master:foo
fatal: Not a valid object name master:foo
$ echo $?
128

Is the output in this case expected?

I'm currently running 2.7.0.

Signature · 1 line
	Michal Čihař | http://cihar.com | http://blog.cihar.com
#1 lore raw
Quoted 16 lines · Michal Čihař

the documentation for "git cat-file -e" says:

Quoted 1 line

Suppress all output; instead exit with zero status if <object> exists

and is a valid object.

However running it on non existing object will complain "fatal: Not a valid object name".

$ git cat-file -e master:README.rst
$ echo $?
0
$ git cat-file -e master:foo
fatal: Not a valid object name master:foo
$ echo $?
128
Quoted 3 lines

Is the output in this case expected?

I'm currently running 2.7.0.

It looks like it has been this way forever. The first thing we do with the object is resolve its name to a sha1, and that's where the error you see comes from. And then we actually check whether we have the object.

I think the intended use was to feed it a sha1 to see if it exists. Then the name-resolution step is a noop.

I'm not sure if the behavior you are seeing is all that bad (the documentation could be read as suppressing the normal stdout output, but error messages remain), but it would be easy to change:

builtin/cat-file.c +3 -3
index 54db118..afde169 100644
@@ -35,6 +35,9 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	if (unknown_type)
 		flags |= LOOKUP_UNKNOWN_OBJECT;
 
+	if (opt == 'e')
+		return !has_sha1_file(sha1);
+
 	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
 		die("Not a valid object name %s", obj_name);
 

@@ -58,9 +61,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		printf("%lu\n", size);
 		return 0;
 
-	case 'e':
-		return !has_sha1_file(sha1);
-
 	case 'c':
 		if (!obj_context.path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
#2 lore raw
Quoted 17 lines · Jeff King

[...]but it would be easy to change:

builtin/cat-file.c +3 -0
index 54db118..afde169 100644
@@ -35,6 +35,9 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	if (unknown_type)
 		flags |= LOOKUP_UNKNOWN_OBJECT;
 
+	if (opt == 'e')
+		return !has_sha1_file(sha1);
+
 	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
 		die("Not a valid object name %s", obj_name);

builtin/cat-file.c static int cat_one_file(int opt, const char *exp_type, const char *obj_name, · line 66

Quoted 9 lines
builtin/cat-file.c +0 -3
@@ -58,9 +61,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		printf("%lu\n", size);
 		return 0;
 
-	case 'e':
-		return !has_sha1_file(sha1);
-
 	case 'c':
 		if (!obj_context.path[0])
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",

This is wrong, of course. We still need to call get_sha1, but just need to not die. So it's more like this (totally untested, naturally):

builtin/cat-file.c +4 -1
index 54db118..44add8c 100644
@@ -35,8 +35,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 	if (unknown_type)
 		flags |= LOOKUP_UNKNOWN_OBJECT;
 
-	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
+	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context)) {
+		if (opt == 'e')
+			return 1;
 		die("Not a valid object name %s", obj_name);
+	}
 
 	buf = NULL;
 	switch (opt) {

-Peff

author
#3 lore raw
In reply to Jeff King

Hi

Dne 29.2.2016 v 12:44 Jeff King napsal(a):

Quoted 6 lines

It looks like it has been this way forever. The first thing we do with the object is resolve its name to a sha1, and that's where the error you see comes from. And then we actually check whether we have the object.

I think the intended use was to feed it a sha1 to see if it exists. Then the name-resolution step is a noop.

I found this as best way to check whether file exists in branch. Checking git ls-tree output seems less error prone than checking return value of git cat-file -e...

Quoted 3 lines

I'm not sure if the behavior you are seeing is all that bad (the documentation could be read as suppressing the normal stdout output, but error messages remain),

I understand this, that's why I'm asking whether it's expected output or not :-).

Signature · 1 line
	Michal Čihař | http://cihar.com | http://blog.cihar.com
#4 lore raw
Quoted 11 lines · Michal Čihař

Dne 29.2.2016 v 12:44 Jeff King napsal(a):

Quoted 6 lines

It looks like it has been this way forever. The first thing we do with the object is resolve its name to a sha1, and that's where the error you see comes from. And then we actually check whether we have the object.

I think the intended use was to feed it a sha1 to see if it exists. Then the name-resolution step is a noop.

I found this as best way to check whether file exists in branch. Checking git ls-tree output seems less error prone than checking return value of git cat-file -e...

I think the usual way to do that would be:

  git rev-parse --verify $branch:$path >/dev/null

which just resolves the name (not that what you are doing is wrong, I just think rev-parse is the way we usually do it in our scripts).

That _will_ write a message to stderr when the name doesn't resolve.

-Peff