Re: [PATCH 1/2] sha1_file: Add sha1_object_type_literally and export it.

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> +int sha1_object_type_literally(const unsigned char *sha1, char *type)
> +{
> +	int status = 0;
> +	unsigned long mapsize;
> +	void *map;
> +	git_zstream stream;
> +	char hdr[32];
> +	int i;
> +
> +	map = map_sha1_file(sha1, &mapsize);
> +	if (!map)
> +		return -1;

I am not sure if it is a good idea to introduce a function with the
above signature and the semantics in the first place.  It is OK to
assume that objects with funny typenames only appear as loose
objects, but it is not OK for "cat-file --literally" to fail to work
on objects that are of kosher types.

What should "git cat-file --literally -t HEAD~2" do?  I think it
should say "commit" as long as my current history is at least 3
commits deep, even when my repository is fully packed.  But I
suspect the above code does not do that.  

Looking at how we collect information on normal objects, it may make
more sense to model this after sha1_loose_object_info(), with a
tweak to struct object_info datatype, and integrate it into
sha1_object_info_extended() may make more sense, perhaps along the
lines of the attached patch.

The new helper would mimick what sha1_loose_object_info() is doing,
in that it may be used to learn on-disk size, object size, typename
string (returned in oi->typename strbuf that is optional).  There is
no sensible value to stuff in oi->typep if the incoming object name
refers to the experimental invalid object, so perhaps you will store
OBJ_NONE or something there and the "cat-file --literally" would use
the oi->typename to learn the name of the "type".


 cache.h     | 1 +
 sha1_file.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/cache.h b/cache.h
index 4d02efc..72b7cfa 100644
--- a/cache.h
+++ b/cache.h
@@ -1296,6 +1296,7 @@ struct object_info {
 	unsigned long *sizep;
 	unsigned long *disk_sizep;
 	unsigned char *delta_base_sha1;
+	struct strbuf *typename;
 
 	/* Response */
 	enum {
diff --git a/sha1_file.c b/sha1_file.c
index 69a60ec..2c0e83a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2568,6 +2568,8 @@ static int sha1_loose_object_info(const unsigned char *sha1,
 	munmap(map, mapsize);
 	if (oi->typep)
 		*oi->typep = status;
+	if (oi->typename && 0 < status && typename(status))
+		strbuf_add(oi->typename, typename(status));
 	return 0;
 }
 
@@ -2593,6 +2595,13 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
 	}
 
 	if (!find_pack_entry(real, &e)) {
+		/* Have we been asked to check possibly invalid ones? */
+		if ((flags & LOOKUP_LITERALLY) &&
+		    !sha1_object_info_literally(real, oi)) {
+			oi->whence = OI_LOOSE;
+			return 0;
+		}
+
 		/* Most likely it's a loose object. */
 		if (!sha1_loose_object_info(real, oi)) {
 			oi->whence = OI_LOOSE;