cat-file: add "--literally" option
v2 cover cat-file: add "--literally" option
Second version of the patch submitted to add "-literlly" option to "cat-file"
http://article.gmane.org/gmane.comp.version-control.git/264383
Thanks to Eric, Junio and David for suggesting changes on my first version.
PATCH v2 1/3 cache: modify for "cat-file --literally -t"
Add a "struct strbuf *typename" to object_info to hold the typename when the literally option is used. Add a flag to notify functions when literally is used.
--- cache.h | 2 ++ 1 file changed, 2 insertions(+)
cache.h +2 -0
@@ -830,6 +830,7 @@ extern int is_ntfs_dotgit(const char *name);/* object replacement */ #define LOOKUP_REPLACE_OBJECT 1 +#define LOOKUP_LITERALLY 2extern 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 {
Signature · 1 line
2.3.1.169.ga243085.dirty
PATCH v2 2/3 sha1_file: implement changes for "cat-file --literally -t"
In reply to karthik nayakadd "sha1_object_info_literally()" which is to be used when the "literally" option is given to get the type of object and print it, using "sha1_object_info_extended()".
add "unpack_sha1_header_literally()" to unpack sha headers which may be greater than 32 bytes, which is the threshold for a regular object header.
modify "sha1_loose_object_info()" to include a flag argument and also include changes to call "unpack_sha1_header_literally()" when the literally flag is passed. Also copies the obtained type to the typename field of object_info.
modify "sha1_object_info_extended()" to call the function "sha1_loose_object_info()" with flags.
--- sha1_file.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 7 deletions(-)
sha1_file.c +77 -7
@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mareturn git_inflate (stream,0 ); } +static int unpack_sha1_header_literally (git_zstream *stream,unsigned char *map, +unsigned long mapsize, +struct strbuf *header) +{ +unsigned char buffer[32 ], *cp; +unsigned long bufsiz =sizeof (buffer); +int status; + +/* Get the data stream */ +memset (stream,0 ,sizeof (*stream)); + stream->next_in = map; + stream->avail_in = mapsize; + stream->next_out = buffer; + stream->avail_out = bufsiz; + +git_inflate_init (stream); + +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 ; + stream->next_out = buffer; + stream->avail_out = bufsiz; + }while (status ==Z_OK ); +return -1 ; +} +static void *unpack_sha1_rest (git_zstream *stream,void *buffer,unsigned long size,const unsigned char *sha1) {int bytes =strlen (buffer) +1 ;
@@ -2524,13 +2554,16 @@ struct packed_git *find_sha1_pack(const unsigned char *sha1, }static int sha1_loose_object_info (const unsigned char *sha1, -struct object_info *oi) +struct object_info *oi, +int flags) { -int status; +int status =0 ;unsigned long mapsize, size;void *map;git_zstream stream;char hdr[32 ]; +struct strbuf hdrbuf =STRBUF_INIT ; +char *hdrp;if (oi->delta_base_sha1)hashclr (oi->delta_base_sha1);
@@ -2557,10 +2590,25 @@ static int sha1_loose_object_info(const unsigned char *sha1,return -1 ;if (oi->disk_sizep) *oi->disk_sizep = mapsize; -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 (hdr, &size)) <0 ) +if ((flags &LOOKUP_LITERALLY )) { +if (unpack_sha1_header_literally (&stream, map, mapsize, &hdrbuf) <0 ) + status =error ("unable to unpack %s header with --literally" , +sha1_to_hex (sha1)); + hdrp = hdrbuf.buf; + }else { +if (unpack_sha1_header (&stream, map, mapsize, hdr,sizeof (hdr)) <0 ) { + status =error ("unable to unpack %s header" , +sha1_to_hex (sha1)); + } + hdrp = hdr; + } +if (status) + ;/* We're already checking for errors */ +else if ((flags &LOOKUP_LITERALLY )) { +size_t typelen =strcspn (hdrbuf.buf," " ); +strbuf_add (oi->typename, hdrbuf.buf, typelen); + } +else if ((status =parse_sha1_header (hdrp, &size)) <0 ) status =error ("unable to parse %s header" ,sha1_to_hex (sha1));else if (oi->sizep) *oi->sizep = size;
@@ -2568,6 +2616,10 @@ 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_addstr (oi->typename,typename (status)); +if (hdrp == hdrbuf.buf) +strbuf_release (&hdrbuf);return 0 ; }
@@ -2594,7 +2646,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,if (!find_pack_entry (real, &e)) {/* Most likely it's a loose object. */ -if (!sha1_loose_object_info (real, oi)) { +if (!sha1_loose_object_info (real, oi, flags)) { oi->whence =OI_LOOSE ;return 0 ; }
@@ -2635,6 +2687,24 @@ int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)return type; } +int sha1_object_info_literally (const unsigned char *sha1) +{ +enum object_type type; +struct strbuf sb =STRBUF_INIT ; +struct object_info oi = {NULL }; + + oi.typename = &sb; + oi.typep = &type; +if (sha1_object_info_extended (sha1, &oi,LOOKUP_LITERALLY ) <0 ) +return -1 ; +if (*oi.typep >0 ) +printf ("%s\n" ,typename (*oi.typep)); +else +printf ("%s\n" , oi.typename->buf); +strbuf_release (oi.typename); +return 0 ; +} +static void *read_packed_sha1 (const unsigned char *sha1,enum object_type *type,unsigned long *size) {
Signature · 1 line
2.3.1.169.ga243085.dirty
Junio C Hamano
NAKRe: [PATCH v2 2/3] sha1_file: implement changes for "cat-file --literally -t"
Quoted 26 lines · Karthik Nayak
add "
sha1_object_info_literally()" which is to be used when the "literally" option is given to get the type of object and print it, using "sha1_object_info_extended()".add "
unpack_sha1_header_literally()" to unpack sha headers which may be greater than 32 bytes, which is the threshold for a regular object header.modify "
sha1_loose_object_info()" to include a flag argument and also include changes to call "unpack_sha1_header_literally()" when the literally flag is passed. Also copies the obtained type to the typename field ofobject_info.modify "
sha1_object_info_extended()" to call the function "sha1_loose_object_info()" with flags.Signed-off-by Karthik Nayak <[email protected]>--- sha1_file.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 7 deletions(-)sha1_file.c +0
-0
sha1_file.c int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma · line 1572
Quoted 9 lines
sha1_file.c +6
-0@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mareturn git_inflate (stream,0 ); } +static int unpack_sha1_header_literally (git_zstream *stream,unsigned char *map, +unsigned long mapsize, +struct strbuf *header) +{ +... +}
Looks suspiciously familiar...
Quoted 8 lines
sha1_file.c +8
-0+int sha1_object_info_literally (const unsigned char *sha1) +{ +enum object_type type; +struct strbuf sb =STRBUF_INIT ; +struct object_info oi = {NULL }; + + oi.typename = &sb; + oi.typep = &type;
sha1_file.c int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) · line 2707
Quoted 10 lines
sha1_file.c +10
-0+if (sha1_object_info_extended (sha1, &oi,LOOKUP_LITERALLY ) <0 ) +return -1 ; +if (*oi.typep >0 ) +printf ("%s\n" ,typename (*oi.typep)); +else +printf ("%s\n" , oi.typename->buf); +strbuf_release (oi.typename); +return 0 ; +} +
NAK.
Please don't add end-user facing final output to sha1_file.c; instead have the caller use a helper function like this one to extract necessary information and perform the end-user interaction there.
Re: [PATCH v2 2/3] sha1_file: implement changes for "cat-file --literally -t"
Quoted 31 lines · Junio C Hamano
Quoted 29 lines · Karthik Nayak
add "
sha1_object_info_literally()" which is to be used when the "literally" option is given to get the type of object and print it, using "sha1_object_info_extended()".add "
unpack_sha1_header_literally()" to unpack sha headers which may be greater than 32 bytes, which is the threshold for a regular object header.modify "
sha1_loose_object_info()" to include a flag argument and also include changes to call "unpack_sha1_header_literally()" when the literally flag is passed. Also copies the obtained type to the typename field ofobject_info.modify "
sha1_object_info_extended()" to call the function "sha1_loose_object_info()" with flags.Signed-off-by Karthik Nayak <[email protected]>--- sha1_file.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 7 deletions(-)sha1_file.c +0
-0@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mareturn git_inflate (stream,0 ); }
Quoted 8 lines
sha1_file.c int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long ma · line 1595
Quoted 6 lines
sha1_file.c +6
-0+static int unpack_sha1_header_literally (git_zstream *stream,unsigned char *map, +unsigned long mapsize, +struct strbuf *header) +{ +... +}Looks suspiciously familiar...
Yes, you suggested it.
It has a similar structure to unpack_sha1_header(). Anything wrong with it?
Quoted 22 lines
Quoted 9 lines
sha1_file.c +8
-0+int sha1_object_info_literally (const unsigned char *sha1) +{ +enum object_type type; +struct strbuf sb =STRBUF_INIT ; +struct object_info oi = {NULL }; + + oi.typename = &sb; + oi.typep = &type;sha1_file.c int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) · line 2707
Quoted 10 lines
sha1_file.c +10
-0+if (sha1_object_info_extended (sha1, &oi,LOOKUP_LITERALLY ) <0 ) +return -1 ; +if (*oi.typep >0 ) +printf ("%s\n" ,typename (*oi.typep)); +else +printf ("%s\n" , oi.typename->buf); +strbuf_release (oi.typename); +return 0 ; +} +NAK.
Quoted 5 lines
Please don't add end-user facing final output to
sha1_file.c; instead have the caller use a helper function like this one to extract necessary information and perform the end-user interaction there.
Ok, will do that.
Re: [PATCH v2 2/3] sha1_file: implement changes for "cat-file --literally -t"
Quoted 4 lines · karthik nayak
Quoted 1 line
Looks suspiciously familiar...
Yes, you suggested it.
It has a similar structure tounpack_sha1_header(). Anything wrong with it?
I don't know if there is something wrong with the code, or not, but it wasn't mentioned in the log message at all that it is not your code, and I was mostly reacting to that.
It is fairly common around here to take somebody else's code and run with it, but people say things like "This is based on suggestion from X", "This function was stolen from Y", etc. and then conclude with "but I adjusted it to match the caller I wrote, so bugs are mine." when they do so.
PATCH v2 3/3 cat-file: add "--literally" option
In reply to karthik nayakmade changes to "cat-file" to include a "--literally" option which prints the type of the object without any complaints.
--- builtin/cat-file.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
builtin/cat-file.c +15 -4
@@ -9,7 +9,8 @@#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;
@@ -23,6 +24,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name) buf =NULL ;switch (opt) {case 't' : +if (literally) +return sha1_object_info_literally (sha1); type =sha1_object_info (sha1,NULL );if (type >0 ) {printf ("%s\n" ,typename (type));
@@ -323,7 +326,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|-s|-e|-p|<type>|--textconv|-t --literally) <object>" ),N_ ("git cat-file (--batch | --batch-check) < <list-of-objects>" ),NULL };
@@ -359,6 +362,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 +373,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_ ("show the type of the given loose object, use for debugging" )), { 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 +386,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 !=3 && argc !=2 && argc !=4 )usage_with_options (cat_file_usage, options); argc =parse_options (argc, argv, prefix, options, cat_file_usage,0 );
@@ -405,5 +411,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' ) +return cat_one_file (opt, exp_type, obj_name, literally); +else if (literally) +usage_with_options (cat_file_usage, options); + +return cat_one_file (opt, exp_type, obj_name, literally); }
Signature · 1 line
2.3.1.169.ga243085.dirty