[PATCH] cat-file: print tags raw for "cat-file -p"

[Jeff King <peff@peff.net>]

On Wed, Apr 17, 2013 at 09:06:21PM +0200, René Scharfe wrote:

> Am 17.04.2013 20:02, schrieb Jeff King:
> >I think we also need to do something about "git cat-file -p", which does
> >not use the split_ident_line parser (but has its own problems with the
> >home-grown parser).
> 
> Ah, while it prints commit object contents verbatim, it formats the date
> of tags.  And it does it without help from tag.c (or ident.c), which in
> turn does its own parsing as well.  So it looks like we have two more
> candidates for conversion to split_ident_line() here.

I think we should apply the patch below to just drop the date formatting
from cat-file, along with your two patches.  This is the 4/4 from the
series I posted in February:

  http://thread.gmane.org/gmane.comp.version-control.git/216870/focus=217081

but there I claimed that "git tag -v" might be affected. Upon looking
closer, it is not; we accidentally dropped the pretty-printing of the
date from it many years ago (and nobody seemed to care).

The other patches from that series aren't necessary. The 1/4 is replaced
by your patches (which do roughly the same thing, but add nice tests and
seem to refactor a bit more). The 2/4 and 3/4 patches were about adding
new fsck checks for tags, but I think there is some refactoring
necessary there. They can wait for now.

-- >8 --
Subject: [PATCH] cat-file: print tags raw for "cat-file -p"

When "cat-file -p" prints commits, it shows them in their
raw format, since git's format is already human-readable.
For tags, however, we print the whole thing raw except for
one thing: we convert the timestamp on the tagger line into a
human-readable date.

This dates all the way back to a0f15fa (Pretty-print tagger
dates, 2006-03-01). At that time there was no other way to
pretty-print a tag.  These days, however, neither of those
matters much. The normal way to pretty-print a tag is with
"git show", which is much more flexible than "cat-file -p".

Commit a0f15fa also built "verify-tag --verbose" (and
subsequently "tag -v") around the "cat-file -p" output.
However, that behavior was lost in commit 62e09ce (Make git
tag a builtin, 2007-07-20), and we went back to printing
the raw tag contents. Nobody seems to have noticed the bug
since then (and it is arguably a saner behavior anyway, as
it shows the actual bytes for which we verified the
signature).

Let's drop the tagger-date formatting for "cat-file -p". It
makes us more consistent with cat-file's commit
pretty-printer, and as a bonus, we can drop the hand-rolled
tag parsing code in cat-file (which happened to behave
inconsistently with the tag pretty-printing code elsewhere).

This is a change of output format, so it's possible that
some callers could considered this a regression. However,
the original behavior was arguably a bug (due to the
inconsistency with commits), likely nobody was relying on it
(even we do not use it ourselves these days), and anyone
relying on the "-p" pretty-printer should be able to expect
a change in the output format (i.e., while "cat-file" is
plumbing, the output format of "-p" was never guaranteed to
be stable).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c  | 71 -----------------------------------------------------
 t/t1006-cat-file.sh |  5 +---
 2 files changed, 1 insertion(+), 75 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 40f87b4..045cee7 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -16,73 +16,6 @@
 #define BATCH 1
 #define BATCH_CHECK 2
 
-static void pprint_tag(const unsigned char *sha1, const char *buf, unsigned long size)
-{
-	/* the parser in tag.c is useless here. */
-	const char *endp = buf + size;
-	const char *cp = buf;
-
-	while (cp < endp) {
-		char c = *cp++;
-		if (c != '\n')
-			continue;
-		if (7 <= endp - cp && !memcmp("tagger ", cp, 7)) {
-			const char *tagger = cp;
-
-			/* Found the tagger line.  Copy out the contents
-			 * of the buffer so far.
-			 */
-			write_or_die(1, buf, cp - buf);
-
-			/*
-			 * Do something intelligent, like pretty-printing
-			 * the date.
-			 */
-			while (cp < endp) {
-				if (*cp++ == '\n') {
-					/* tagger to cp is a line
-					 * that has ident and time.
-					 */
-					const char *sp = tagger;
-					char *ep;
-					unsigned long date;
-					long tz;
-					while (sp < cp && *sp != '>')
-						sp++;
-					if (sp == cp) {
-						/* give up */
-						write_or_die(1, tagger,
-							     cp - tagger);
-						break;
-					}
-					while (sp < cp &&
-					       !('0' <= *sp && *sp <= '9'))
-						sp++;
-					write_or_die(1, tagger, sp - tagger);
-					date = strtoul(sp, &ep, 10);
-					tz = strtol(ep, NULL, 10);
-					sp = show_date(date, tz, 0);
-					write_or_die(1, sp, strlen(sp));
-					xwrite(1, "\n", 1);
-					break;
-				}
-			}
-			break;
-		}
-		if (cp < endp && *cp == '\n')
-			/* end of header */
-			break;
-	}
-	/* At this point, we have copied out the header up to the end of
-	 * the tagger line and cp points at one past \n.  It could be the
-	 * next header line after the tagger line, or it could be another
-	 * \n that marks the end of the headers.  We need to copy out the
-	 * remainder as is.
-	 */
-	if (cp < endp)
-		write_or_die(1, cp, endp - cp);
-}
-
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
 {
 	unsigned char sha1[20];
@@ -133,10 +66,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)
 		buf = read_sha1_file(sha1, &type, &size);
 		if (!buf)
 			die("Cannot read object %s", obj_name);
-		if (type == OBJ_TAG) {
-			pprint_tag(sha1, buf, size);
-			return 0;
-		}
 
 		/* otherwise just spit out the data */
 		break;
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 9820f70..9cc5c6b 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -135,14 +135,11 @@ tag_size=$(strlen "$tag_content")
 tag_content="$tag_header_without_timestamp 0000000000 +0000
 
 $tag_description"
-tag_pretty_content="$tag_header_without_timestamp Thu Jan 1 00:00:00 1970 +0000
-
-$tag_description"
 
 tag_sha1=$(echo_without_newline "$tag_content" | git mktag)
 tag_size=$(strlen "$tag_content")
 
-run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_pretty_content" 1
+run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_content" 1
 
 test_expect_success \
     "Reach a blob from a tag pointing to it" \
-- 
1.8.2.11.g42401f0