Re: [PATCH v9 3/5] cat-file: teach cat-file a '--allow-unknown-type' option

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

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

> Currently 'git cat-file' throws an error while trying to
> print the type or size of a broken/corrupt object. This is
> because these objects are usually of unknown types.
>
> Teach git cat-file a '--allow-unkown-type' option where it prints
> the type or size of a broken/corrupt object without throwing
> an error.

Drop "Currently" from the description of the problem you are
solving.  We know that the problem you have to solve in the code is
current without being told.  This comment applies to all patches.

> Modify '-t' and '-s' options to call sha1_object_info_extended()
> directly to support the '--allow-unkown-type' option.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  builtin/cat-file.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 53b5376..299e2e5 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -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 unkown_type)
>  {
>  	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 (unkown_type)
> +		flags |= LOOKUP_UNKNOWN_OBJECT;
>  
>  	if (get_sha1_with_context(obj_name, 0, sha1, &obj_context))
>  		die("Not a valid object name %s", obj_name);
> @@ -23,20 +30,22 @@ 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.typename = &sb;
> +		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
> +			die("git cat-file: could not get object info");
> +		if (sb.len) {
> +			printf("%s\n", sb.buf);
> +			strbuf_release(&sb);
>  			return 0;
>  		}
>  		break;
>  
>  	case 's':
> -		type = sha1_object_info(sha1, &size);
> -		if (type > 0) {
> -			printf("%lu\n", size);
> -			return 0;
> -		}
> -		break;
> +		oi.sizep = &size;
> +		if (sha1_object_info_extended(sha1, &oi, flags) < 0)
> +			die("git cat-file: could not get object info");
> +		printf("%lu\n", size);
> +		return 0;
>  
>  	case 'e':
>  		return !has_sha1_file(sha1);
> @@ -323,7 +332,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 [--allow-unkown-type]|-s [--allow-unkown-type]|-e|-p|<type>|--textconv) <object>"),
>  	N_("git cat-file (--batch | --batch-check) < <list-of-objects>"),
>  	NULL
>  };
> @@ -359,6 +368,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 unkown_type = 0;
>  
>  	const struct option options[] = {
>  		OPT_GROUP(N_("<type> can be one of: blob, tree, commit, tag")),
> @@ -369,6 +379,8 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
>  		OPT_CMDMODE('p', NULL, &opt, N_("pretty-print object's content"), 'p'),
>  		OPT_CMDMODE(0, "textconv", &opt,
>  			    N_("for blob objects, run textconv on object's content"), 'c'),
> +		OPT_BOOL( 0, "allow-unkown-type", &unkown_type,
> +			  N_("allow -s and -t to work with broken/corrupt objects")),
>  		{ OPTION_CALLBACK, 0, "batch", &batch, "format",
>  			N_("show info and content of objects fed from the standard input"),
>  			PARSE_OPT_OPTARG, batch_option_callback },
> @@ -402,5 +414,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 (unkown_type && opt != 't' && opt != 's')
> +		die("git cat-file --allow-unkown-type: use with -s or -t");
> +	return cat_one_file(opt, exp_type, obj_name, unkown_type);
>  }