From ca51699961664890fdaabd276af539e6b3514053 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 6 Feb 2012 03:13:12 -0500 Subject: [PATCH 1/3] tag: fix output of "tag -n" when errors occur When "git tag" is instructed to print lines from annotated tags via "-n", it first prints the tag name, then attempts to parse and print the lines of the tag object, and then finally adds a trailing newline. If an error occurs, we return early from the function and never print the newline, screwing up the output for the next tag. Let's factor the line-printing into its own function so we can manage the early returns better, and make sure that we always terminate the line. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/tag.c | 66 ++++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 667515e527..391160cf83 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -84,18 +84,45 @@ static int contains(struct commit *candidate, const struct commit_list *want) return contains_recurse(candidate, want); } +static void show_tag_lines(const unsigned char *sha1, int lines) +{ + int i; + unsigned long size; + enum object_type type; + char *buf, *sp, *eol; + size_t len; + + buf = read_sha1_file(sha1, &type, &size); + if (!buf || !size) + return; + + /* skip header */ + sp = strstr(buf, "\n\n"); + if (!sp) { + free(buf); + return; + } + /* only take up to "lines" lines, and strip the signature */ + size = parse_signature(buf, size); + for (i = 0, sp += 2; i < lines && sp < buf + size; i++) { + if (i) + printf("\n "); + eol = memchr(sp, '\n', size - (sp - buf)); + len = eol ? eol - sp : size - (sp - buf); + fwrite(sp, len, 1, stdout); + if (!eol) + break; + sp = eol + 1; + } + free(buf); +} + static int show_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data) { struct tag_filter *filter = cb_data; if (match_pattern(filter->patterns, refname)) { - int i; - unsigned long size; - enum object_type type; - char *buf, *sp, *eol; - size_t len; - if (filter->with_commit) { struct commit *commit; @@ -111,33 +138,8 @@ static int show_reference(const char *refname, const unsigned char *sha1, return 0; } printf("%-15s ", refname); - - buf = read_sha1_file(sha1, &type, &size); - if (!buf || !size) - return 0; - - /* skip header */ - sp = strstr(buf, "\n\n"); - if (!sp) { - free(buf); - return 0; - } - /* only take up to "lines" lines, and strip the signature */ - size = parse_signature(buf, size); - for (i = 0, sp += 2; - i < filter->lines && sp < buf + size; - i++) { - if (i) - printf("\n "); - eol = memchr(sp, '\n', size - (sp - buf)); - len = eol ? eol - sp : size - (sp - buf); - fwrite(sp, len, 1, stdout); - if (!eol) - break; - sp = eol + 1; - } + show_tag_lines(sha1, filter->lines); putchar('\n'); - free(buf); } return 0; From fb630e048c6efabe4d46b1b125fe7348062d38e6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 6 Feb 2012 03:13:42 -0500 Subject: [PATCH 2/3] tag: die when listing missing or corrupt objects We don't usually bother looking at tagged objects at all when listing. However, if "-n" is specified, we open the objects to read the annotations of the tags. If we fail to read an object, or if the object has zero length, we simply silently return. The first case is an indication of a broken or corrupt repo, and we should notify the user of the error. The second case is OK to silently ignore; however, the existing code leaked the buffer returned by read_sha1_file. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/tag.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builtin/tag.c b/builtin/tag.c index 391160cf83..1e27f5c3d6 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -93,8 +93,12 @@ static void show_tag_lines(const unsigned char *sha1, int lines) size_t len; buf = read_sha1_file(sha1, &type, &size); - if (!buf || !size) + if (!buf) + die_errno("unable to read object %s", sha1_to_hex(sha1)); + if (!size) { + free(buf); return; + } /* skip header */ sp = strstr(buf, "\n\n"); From 31fd8d72f2eb82c2b6d8d450ee4e13a3925291c3 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 6 Feb 2012 10:13:27 -0800 Subject: [PATCH 3/3] tag: do not show non-tag contents with "-n" "git tag -n" did not check the type of the object it is reading the top n lines from. At least, avoid showing the beginning of trees and blobs when dealing with lightweight tags that point at them. As the payload of a tag and a commit look similar in that they both start with a header block, which is skipped for the purpose of "-n" output, followed by human readable text, allow the message of commit objects to be shown just like the contents of tag objects. This avoids regression for people who have been using "tag -n" to show the log messages of commits that are pointed at by lightweight tags. Test script is from Jeff King. Signed-off-by: Junio C Hamano --- builtin/tag.c | 22 ++++++++++++---------- t/t7004-tag.sh | 13 +++++++++++++ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/builtin/tag.c b/builtin/tag.c index 1e27f5c3d6..6ca53e3310 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -95,19 +95,20 @@ static void show_tag_lines(const unsigned char *sha1, int lines) buf = read_sha1_file(sha1, &type, &size); if (!buf) die_errno("unable to read object %s", sha1_to_hex(sha1)); - if (!size) { - free(buf); - return; - } + if (type != OBJ_COMMIT && type != OBJ_TAG) + goto free_return; + if (!size) + die("an empty %s object %s?", + typename(type), sha1_to_hex(sha1)); /* skip header */ sp = strstr(buf, "\n\n"); - if (!sp) { - free(buf); - return; - } - /* only take up to "lines" lines, and strip the signature */ - size = parse_signature(buf, size); + if (!sp) + goto free_return; + + /* only take up to "lines" lines, and strip the signature from a tag */ + if (type == OBJ_TAG) + size = parse_signature(buf, size); for (i = 0, sp += 2; i < lines && sp < buf + size; i++) { if (i) printf("\n "); @@ -118,6 +119,7 @@ static void show_tag_lines(const unsigned char *sha1, int lines) break; sp = eol + 1; } +free_return: free(buf); } diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 097ce2bc83..7687e62cc5 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -585,6 +585,19 @@ test_expect_success \ test_cmp expect actual ' +test_expect_success 'annotations for blobs are empty' ' + blob=$(git hash-object -w --stdin <<-\EOF + Blob paragraph 1. + + Blob paragraph 2. + EOF + ) && + git tag tag-blob $blob && + echo "tag-blob " >expect && + git tag -n1 -l tag-blob >actual && + test_cmp expect actual +' + # subsequent tests require gpg; check if it is available gpg --version >/dev/null 2>/dev/null if [ $? -eq 127 ]; then