cat-file: add a '--literally' option

sha1_file.c +16 -0

+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
+					unsigned long mapsize,
+					struct strbuf *header)
+{
+	unsigned char buffer[32], *cp;
+	unsigned long bufsiz = sizeof(buffer);
+	int status;
+
+	status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
+
+	if (status) {
+		strbuf_add(header, buffer, stream->next_out - buffer);
+		return status;
+	}
+
+	do {

Quoted 22 lines · Karthik Nayak

sha1_file.c +22 -0

+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned char *map,
+					unsigned long mapsize,
+					struct strbuf *header)
+{
+	unsigned char buffer[32], *cp;
+	unsigned long bufsiz = sizeof(buffer);
+	int status;
+
+	status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
+
+	if (status) {
+		strbuf_add(header, buffer, stream->next_out - buffer);
+		return status;
+	}
+
+	do {
+		status = git_inflate(stream, 0);
+		strbuf_add(header, buffer, stream->next_out - buffer);
+		for (cp = buffer; cp < stream->next_out; cp++)
+			if (!*cp)
+				/* Found the NUL at the end of the header */
+				return 0;

sha1_file.c int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma · line 1592

Quoted 4 lines

sha1_file.c +4 -0

+		stream->next_out = buffer;
+		stream->avail_out = bufsiz;
+	} while (status == Z_OK);
+	return -1;

A caller of unpack_sha1_header() gets the return from git_inflate(); I wonder if we should be returning "status" here for consistency, perhaps?

sha1_file.c static int sha1_loose_object_info(const unsigned char *sha1, · line 2626

Quoted 13 lines · Karthik Nayak

sha1_file.c +13 -0

+	if ((flags & LOOKUP_LITERALLY)) {
+		if (unpack_sha1_header_to_strbuf(&stream, map, mapsize, &hdrbuf) < 0)
+			status = error("unable to unpack %s header with --literally",
+				       sha1_to_hex(sha1));
+		else if ((status = parse_sha1_header_extended(hdrbuf.buf, oi, flags)) < 0)
+			status = error("unable to parse %s header", sha1_to_hex(sha1));
+	} else {
+		if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
+			status = error("unable to unpack %s header",
+				       sha1_to_hex(sha1));
+		else if ((status = parse_sha1_header_extended(hdr, oi, flags)) < 0)
+			status = error("unable to parse %s header", sha1_to_hex(sha1));
+	}

I wonder if you can further reduce an unnecessary duplication in the two "else if" clauses in the above, and if the result becomes easier to read and maintain. Perhaps

	if (((flags & LOOKUP_LITERALLY)
              ? unpack_sha1_header_to_strbuf(...)
              : unpack_sha1_header(...)) < 0)
		status = error(...);
	else if ((status = parse_sha1_header_extended(...)) < 0)
         	status = error(...);

or even

	status = 0;
	if (flags & LOOKUP_LITERALLY) {
		if (unpack_sha1_header_to_strbuf(...) < 0)
			status = error(...);
	} else {
		if (unpack_sha1_header(...) < 0)
			status = error(...);
	}
         if (!status) {
         	if (status = parse(...)) < 0)
	        	status = error(...);
	}

although I think the latter might be a bit harder to read.

I hope you meant the former.

Currently 'git cat-file' throws an error while trying to print the type or size of a broken/corrupt object which is created using 'git hash-object --literally'. This is because these objects are usually of unknown types.

Teach git cat-file a '--literally' option where it prints the type or size of a broken/corrupt object without throwing an error.

Modify '-t' and '-s' options to call sha1_object_info_extended() directly to support the '--literally' option.

Helped-by Junio C Hamano <[email protected]>

Helped-by Eric Sunshine <sunshine@sunshineco

Signed-off-by Karthik Nayak <[email protected]>

---

builtin/cat-file.c +0 -0

index df99df4..6fee461 100644

builtin/cat-file.c +0 -1

 }
--

2.3.1.170.g5319d60.dirty

Quoted 21 lines · Karthik Nayak

Currently 'git cat-file' throws an error while trying to print the type or size of a broken/corrupt object which is created using 'git hash-object --literally'. This is because these objects are usually of unknown types.

Teach git cat-file a '--literally' option where it prints the type or size of a broken/corrupt object without throwing an error.

Modify '-t' and '-s' options to call sha1_object_info_extended() directly to support the '--literally' option.

Helped-by Junio C Hamano <[email protected]>

Helped-by Eric Sunshine <sunshine@sunshineco

Signed-off-by Karthik Nayak <[email protected]>

---

builtin/cat-file.c +0 -0

index df99df4..6fee461 100644

builtin/cat-file.c int cmd_cat_file(int argc, const char **argv, const char *prefix) · line 431

Quoted 10 lines

builtin/cat-file.c +6 -1

@@ -405,5 +423,10 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
         if (batch.enabled)
                 return batch_objects(&batch);
 
-       return cat_one_file(opt, exp_type, obj_name);
+       if (literally && (opt == 't' || opt == 's'))
+               return cat_one_file(opt, exp_type, obj_name, literally);
+       else if (literally)
+               die("git cat-file --literally: use with -s or -t");
+
+       return cat_one_file(opt, exp_type, obj_name, literally);

Although I haven't read the patch closely yet, this unnecessarily complex logic caught my eye as I was scanning the changes. If you check for the illegal option combination first, then you can coalesce the two identical cat_one_file() invocations, and this entire hunk reduces to the more readable:

     if (literally && opt != 't' && opt != 's')
         die("git cat-file --literally: use with -s or -t");

     return cat_one_file(opt, exp_type, obj_name, literally);

Quoted 3 lines

builtin/cat-file.c +0 -1

  }
--

2.3.1.170.g5319d60.dirty