Re: [PATCH 1/2] sha1_file: Add sha1_object_type_literally and export it.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 25, 2015, 21:32 UTC
- Message-ID
- <xmqq61apoewr.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1424862460-13514-1-git-send-email-karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 12 quoted lines
> +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;