Skip to contentSkip to conversation

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

PATCH Superseded karthik nayak 2 participants 10 messages Jun 15, 2016 Documentation

v6 of 10 Superseded Latest is v10 →

lore
Share & tools
lore

karthik nayak

author
#0 lore raw

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

The changes made in this version are :

* unpack_sha1_header_to_strbuf() now checks for Z_STREAM_END and also
the return is not changed to status (as per Junios suggestion) as this breaks the check done in sha1_loose_object_info() as the return is sometimes Z_BUF_ERROR (-5).

* In sha1_loose_object_info() if the status is set, then change the type to status. Also check for this in cat-file.c

* Add documentation and tests for the same.

Thanks for your suggestions.

author
#1 lore raw

PATCH v6 1/4 sha1_file.c: support reading from a loose object of unknown type

Update sha1_loose_object_info() to optionally allow it to read from a loose object file of unknown/bogus type; as the function usually returns the type of the object it read in the form of enum for known types, add an optional "typename" field to receive the name of the type in textual form and a flag to indicate the reading of a loose object file of unknown/bogus type.

Add parse_sha1_header_extended() which acts as a wrapper around parse_sha1_header() allowing more information to be obtained.

Add unpack_sha1_header_to_strbuf() to unpack sha1 headers of unknown/corrupt objects which have a unknown sha1 header size to a strbuf structure. This was written by Junio C Hamano but tested by me.

Helped-by Junio C Hamano <[email protected]>
Helped-by Eric Sunshine <[email protected]>
Signed-off-by Karthik Nayak <[email protected]>
---
 cache.h     |   2 ++
 sha1_file.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 91 insertions(+), 22 deletions(-)
cache.h +2 -0
index 4d02efc..949ef4c 100644
@@ -830,6 +830,7 @@ extern int is_ntfs_dotgit(const char *name);
 
 /* object replacement */
 #define LOOKUP_REPLACE_OBJECT 1
+#define LOOKUP_LITERALLY 2
 extern void *read_sha1_file_extended(const unsigned char *sha1, enum object_type *type, unsigned long *size, unsigned flag);
 static inline void *read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
 {

@@ -1296,6 +1297,7 @@ struct object_info {
 	unsigned long *sizep;
 	unsigned long *disk_sizep;
 	unsigned char *delta_base_sha1;
+	struct strbuf *typename;
 
 	/* Response */
 	enum {
sha1_file.c +89 -22

183 lines not on this page · raw message

Signature · 1 line
2.3.1.172.g04a1281
#2 lore raw

Re: [PATCH v6 1/4] sha1_file.c: support reading from a loose object of unknown type

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

Quoted 9 lines · Karthik Nayak
sha1_file.c +9 -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);

I briefly wondered if this can return Z_BUF_ERROR, but it is OK because we do not call inflate with Z_FINISH in unpack_sha1_header() for obvious reasons ;-)

Quoted 7 lines
sha1_file.c +6 -0
+	if (status) {
+		strbuf_add(header, buffer, stream->next_out - buffer);
+		return status;
+	}
 
+	do {
+		status = git_inflate(stream, 0);

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

Quoted 10 lines
sha1_file.c +10 -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;
+		stream->next_out = buffer;
+		stream->avail_out = bufsiz;
+	} while (status != Z_STREAM_END);
+	return -1;
+}

OK.

Quoted 6 lines
sha1_file.c +1 -1
@@ -1614,27 +1642,24 @@ static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s
  * too permissive for what we want to check. So do an anal
  * object header parse by hand.
  */
-int parse_sha1_header(const char *hdr, unsigned long *sizep)
+int parse_sha1_header_extended(const char *hdr, struct object_info *oi,

sha1_file.c static void *unpack_sha1_rest(git_zstream *stream, void *buffer, unsigned long s · line 1653

Quoted 10 lines
sha1_file.c +3 -2
+			       unsigned int flags)
 {
-	char type[10];
-	int i;
+	struct strbuf typename = STRBUF_INIT;
 	unsigned long size;
+	int type;
 
 	/*
 	 * The type can be at most ten bytes (including the

Is this still a valid comment?

Quoted 17 lines
sha1_file.c +1 -5
 	 * terminating '\0' that we add), and is followed by
 	 * a space.
 	 */
-	i = 0;
 	for (;;) {
 		char c = *hdr++;
 		if (c == ' ')
 			break;
-		type[i++] = c;
-		if (i >= sizeof(type))
-			return -1;
+		strbuf_addch(&typename, c);
 	}
-	type[i] = 0;
 
 	/*
 	 * The length must follow immediately, and be in canonical
author
#3 lore raw

PATCH v6 2/4 cat-file: teach cat-file a '--literally' option

In reply to 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 | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)
builtin/cat-file.c +29 -9
index df99df4..91ceae0 100644
@@ -9,13 +9,20 @@
 #include "userdiff.h"
 #include "streaming.h"
 
-static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
+static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
+			int literally)
 {
 	unsigned char sha1[20];
 	enum object_type type;
 	char *buf;
 	unsigned long size;
 	struct object_context obj_context;
+	struct object_info oi = {NULL};
+	struct strbuf sb = STRBUF_INIT;
+	unsigned flags = LOOKUP_REPLACE_OBJECT;
+
+	if (literally)
+		flags |= LOOKUP_LITERALLY;
 
 	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
 		die("Not a valid object name %s", obj_name);

@@ -23,16 +30,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
 	buf = NULL;
 	switch (opt) {
 	case 't':
-		type = sha1_object_info(sha1, NULL);
-		if (type > 0) {
-			printf("%s\n", typename(type));
+		oi.typep = &type;
+		oi.typename = &sb;
+		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+			die("git cat-file: could not get object info");
+		if (type >= 0 && sb.len) {
+			printf("%s\n", sb.buf);
+			strbuf_release(&sb);
 			return 0;
 		}
 		break;
 
 	case 's':
-		type = sha1_object_info(sha1, &size);
-		if (type > 0) {
+		oi.typep = &type;
+		oi.typename = &sb;
+		oi.sizep = &size;
+		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+			die("git cat-file: could not get object info");
+		if (type >= 0 && sb.len) {
 			printf("%lu\n", size);
 			return 0;
 		}

@@ -323,7 +338,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t | -s | -e | -p | <type> | --textconv) <object>"),
+	N_("git cat-file (-t [--literally]|-s [--literally]|-e|-p|<type>|--textconv) <object>"),
 	N_("git cat-file (--batch | --batch-check) < <list-of-objects>"),
 	NULL
 };

@@ -359,6 +374,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 	int opt = 0;
 	const char *exp_type = NULL, *obj_name = NULL;
 	struct batch_options batch = {0};
+	int literally = 0;
 
 	const struct option options[] = {
 		OPT_GROUP(N_("<type> can be one of: blob, tree, commit, tag")),

@@ -369,6 +385,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
 		OPT_SET_INT(0, "textconv", &opt,
 			    N_("for blob objects, run textconv on object's content"), 'c'),
+		OPT_BOOL( 0, "literally", &literally,
+			  N_("get information about corrupt objects for debugging Git")),
 		{ OPTION_CALLBACK, 0, "batch", &batch, "format",
 			N_("show info and content of objects fed from the standard input"),
 			PARSE_OPT_OPTARG, batch_option_callback },

@@ -380,7 +398,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 
 	git_config(git_cat_file_config, NULL);
 
-	if (argc != 3 && argc != 2)
+	if (argc < 2 || argc > 4)
 		usage_with_options(cat_file_usage, options);
 
 	argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);

@@ -405,5 +423,7 @@ 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')
+		die("git cat-file --literally: use with -s or -t");
+	return cat_one_file(opt, exp_type, obj_name, literally);
 }
Signature · 1 line
2.3.1.172.g04a1281
#4 lore raw

Re: [PATCH v6 2/4] cat-file: teach cat-file a '--literally' option

Quoted 9 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.

Quoted 5 lines

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

s/$/.com/ perhaps?

Quoted 105 lines
Signed-off-by Karthik Nayak <[email protected]>
---
 builtin/cat-file.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)
builtin/cat-file.c +29 -9
index df99df4..91ceae0 100644
@@ -9,13 +9,20 @@
 #include "userdiff.h"
 #include "streaming.h"
 
-static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
+static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
+			int literally)
 {
 	unsigned char sha1[20];
 	enum object_type type;
 	char *buf;
 	unsigned long size;
 	struct object_context obj_context;
+	struct object_info oi = {NULL};
+	struct strbuf sb = STRBUF_INIT;
+	unsigned flags = LOOKUP_REPLACE_OBJECT;
+
+	if (literally)
+		flags |= LOOKUP_LITERALLY;
 
 	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
 		die("Not a valid object name %s", obj_name);

@@ -23,16 +30,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
 	buf = NULL;
 	switch (opt) {
 	case 't':
-		type = sha1_object_info(sha1, NULL);
-		if (type > 0) {
-			printf("%s\n", typename(type));
+		oi.typep = &type;
+		oi.typename = &sb;
+		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+			die("git cat-file: could not get object info");
+		if (type >= 0 && sb.len) {
+			printf("%s\n", sb.buf);
+			strbuf_release(&sb);
 			return 0;
 		}
 		break;
 
 	case 's':
-		type = sha1_object_info(sha1, &size);
-		if (type > 0) {
+		oi.typep = &type;
+		oi.typename = &sb;
+		oi.sizep = &size;
+		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+			die("git cat-file: could not get object info");
+		if (type >= 0 && sb.len) {
 			printf("%lu\n", size);
 			return 0;
 		}

@@ -323,7 +338,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t | -s | -e | -p | <type> | --textconv) <object>"),
+	N_("git cat-file (-t [--literally]|-s [--literally]|-e|-p|<type>|--textconv) <object>"),
 	N_("git cat-file (--batch | --batch-check) < <list-of-objects>"),
 	NULL
 };

@@ -359,6 +374,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 	int opt = 0;
 	const char *exp_type = NULL, *obj_name = NULL;
 	struct batch_options batch = {0};
+	int literally = 0;
 
 	const struct option options[] = {
 		OPT_GROUP(N_("<type> can be one of: blob, tree, commit, tag")),

@@ -369,6 +385,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 		OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
 		OPT_SET_INT(0, "textconv", &opt,
 			    N_("for blob objects, run textconv on object's content"), 'c'),
+		OPT_BOOL( 0, "literally", &literally,
+			  N_("get information about corrupt objects for debugging Git")),
 		{ OPTION_CALLBACK, 0, "batch", &batch, "format",
 			N_("show info and content of objects fed from the standard input"),
 			PARSE_OPT_OPTARG, batch_option_callback },

@@ -380,7 +398,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 
 	git_config(git_cat_file_config, NULL);
 
-	if (argc != 3 && argc != 2)
+	if (argc < 2 || argc > 4)
 		usage_with_options(cat_file_usage, options);
 
 	argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);

@@ -405,5 +423,7 @@ 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')
+		die("git cat-file --literally: use with -s or -t");
+	return cat_one_file(opt, exp_type, obj_name, literally);
 }
author
#5 lore raw

Re: [PATCH v6 2/4] cat-file: teach cat-file a '--literally' option

Quoted 11 lines · Junio C Hamano
Quoted 9 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.

Quoted 7 lines
Quoted 5 lines

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

s/$/.com/ perhaps?

Definitely!

Quoted 106 lines
Quoted 106 lines
Signed-off-by Karthik Nayak <[email protected]>
---
  builtin/cat-file.c | 38 +++++++++++++++++++++++++++++---------
  1 file changed, 29 insertions(+), 9 deletions(-)
builtin/cat-file.c +29 -9
index df99df4..91ceae0 100644
@@ -9,13 +9,20 @@
  #include "userdiff.h"
  #include "streaming.h"
 
-static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
+static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
+			int literally)
  {
  	unsigned char sha1[20];
  	enum object_type type;
  	char *buf;
  	unsigned long size;
  	struct object_context obj_context;
+	struct object_info oi = {NULL};
+	struct strbuf sb = STRBUF_INIT;
+	unsigned flags = LOOKUP_REPLACE_OBJECT;
+
+	if (literally)
+		flags |= LOOKUP_LITERALLY;
 
  	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
  		die("Not a valid object name %s", obj_name);

@@ -23,16 +30,24 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
  	buf = NULL;
  	switch (opt) {
  	case 't':
-		type = sha1_object_info(sha1, NULL);
-		if (type > 0) {
-			printf("%s\n", typename(type));
+		oi.typep = &type;
+		oi.typename = &sb;
+		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+			die("git cat-file: could not get object info");
+		if (type >= 0 && sb.len) {
+			printf("%s\n", sb.buf);
+			strbuf_release(&sb);
  			return 0;
  		}
  		break;
 
  	case 's':
-		type = sha1_object_info(sha1, &size);
-		if (type > 0) {
+		oi.typep = &type;
+		oi.typename = &sb;
+		oi.sizep = &size;
+		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
+			die("git cat-file: could not get object info");
+		if (type >= 0 && sb.len) {
  			printf("%lu\n", size);
  			return 0;
  		}

@@ -323,7 +338,7 @@ static int batch_objects(struct batch_options *opt)
  }
 
  static const char * const cat_file_usage[] = {
-	N_("git cat-file (-t | -s | -e | -p | <type> | --textconv) <object>"),
+	N_("git cat-file (-t [--literally]|-s [--literally]|-e|-p|<type>|--textconv) <object>"),
  	N_("git cat-file (--batch | --batch-check) < <list-of-objects>"),
  	NULL
  };

@@ -359,6 +374,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
  	int opt = 0;
  	const char *exp_type = NULL, *obj_name = NULL;
  	struct batch_options batch = {0};
+	int literally = 0;
 
  	const struct option options[] = {
  		OPT_GROUP(N_("<type> can be one of: blob, tree, commit, tag")),

@@ -369,6 +385,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
  		OPT_SET_INT('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
  		OPT_SET_INT(0, "textconv", &opt,
  			    N_("for blob objects, run textconv on object's content"), 'c'),
+		OPT_BOOL( 0, "literally", &literally,
+			  N_("get information about corrupt objects for debugging Git")),
  		{ OPTION_CALLBACK, 0, "batch", &batch, "format",
  			N_("show info and content of objects fed from the standard input"),
  			PARSE_OPT_OPTARG, batch_option_callback },

@@ -380,7 +398,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
 
  	git_config(git_cat_file_config, NULL);
 
-	if (argc != 3 && argc != 2)
+	if (argc < 2 || argc > 4)
  		usage_with_options(cat_file_usage, options);
 
  	argc = parse_options(argc, argv, prefix, options, cat_file_usage, 0);

@@ -405,5 +423,7 @@ 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')
+		die("git cat-file --literally: use with -s or -t");
+	return cat_one_file(opt, exp_type, obj_name, literally);
  }
author
#6 lore raw

PATCH v6 3/4 cat-file: add documentation for '--literally' option.

In reply to karthik nayak
Signed-off-by Karthik Nayak <[email protected]>
---
 Documentation/git-cat-file.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Documentation/git-cat-file.txt +5 -1
index f6a16f4..8bac7bd 100644
@@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for repository objec
 SYNOPSIS
 --------
 [verse]
-'git cat-file' (-t | -s | -e | -p | <type> | --textconv ) <object>
+'git cat-file' (-t [--literally]| -s [--literally]| -e | -p | <type> | --textconv ) <object>
 'git cat-file' (--batch | --batch-check) < <list-of-objects>
 
 DESCRIPTION

@@ -69,6 +69,10 @@ OPTIONS
 	not be combined with any other options or arguments.  See the
 	section `BATCH OUTPUT` below for details.
 
+--literally::
+	Print information of broken/corrupt objects of unknown type without
+	throwing an error. To be used combined with '-s' or '-t' option.
+
 OUTPUT
 ------
 If '-t' is specified, one of the <type>.
Signature · 1 line
2.3.1.172.g04a1281
author
#7 lore raw

PATCH v6 4/4 t1006: add tests for git cat-file --literally

In reply to karthik nayak
Signed-off-by Karthik Nayak <[email protected]>
---
 t/t1006-cat-file.sh | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
t/t1006-cat-file.sh +27 -0
index a72e700..3015062 100755
@@ -47,6 +47,18 @@ $content"
 	test_cmp expect actual
     '
 
+    test_expect_success "Type of $type is correct using --literally" '
+	echo $type >expect &&
+	git cat-file -t --literally $sha1 >actual &&
+	test_cmp expect actual
+    '
+
+    test_expect_success "Size of $type is correct using --literally" '
+	echo $size >expect &&
+	git cat-file -s --literally $sha1 >actual &&
+	test_cmp expect actual
+    '
+
     test -z "$content" ||
     test_expect_success "Content of $type is correct" '
 	maybe_remove_timestamp "$content" $no_ts >expect &&

@@ -296,4 +308,19 @@ test_expect_success '%(deltabase) reports packed delta bases' '
 	}
 '
 
+bogus_type="bogus"
+bogus_sha1=$(git hash-object -t $bogus_type --literally -w --stdin </dev/null)
+
+test_expect_success "Type of broken object is correct" '
+	echo $bogus_type >expect &&
+	git cat-file -t --literally $bogus_sha1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "Size of broken object is correct" '
+	echo "0" >expect &&
+	git cat-file -s --literally $bogus_sha1 >actual &&
+	test_cmp expect actual
+'
+
 test_done
Signature · 1 line
2.3.1.172.g04a1281
author
#9 lore raw
Quoted 3 lines · Junio C Hamano

When merged to 'pu', this seems to break at least 5309 and 5300 tests (there might be others, but I didn't check).

There was a problem with packed object types. I have it fixed, will send a re-roll.