From 370c9268d13ef54a05ed84b92baa81b75930e58f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 12 Dec 2013 07:01:42 +0800 Subject: cat-file: pass expand_data to print_object_or_die We currently individually pass the sha1, type, and size fields calculated by sha1_object_info. However, if we pass the whole struct, the called function can make more intelligent decisions about which fields were actually filled by sha1_object_info. This patch takes that first refactoring step, passing the whole struct, so further patches can make those decisions with less noise in their diffs. There should be no functional change to this patch (aside from a minor typo fix in the error message). As a side effect, we can rename the local variables in the function to "type" and "size", since the names are no longer taken. Reviewed-by: Jonathan Nieder Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index b2ca775a80..1434afbd4d 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -193,25 +193,26 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data) return end - start + 1; } -static void print_object_or_die(int fd, const unsigned char *sha1, - enum object_type type, unsigned long size) +static void print_object_or_die(int fd, struct expand_data *data) { - if (type == OBJ_BLOB) { + const unsigned char *sha1 = data->sha1; + + if (data->type == OBJ_BLOB) { if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0) die("unable to stream %s to stdout", sha1_to_hex(sha1)); } else { - enum object_type rtype; - unsigned long rsize; + enum object_type type; + unsigned long size; void *contents; - contents = read_sha1_file(sha1, &rtype, &rsize); + contents = read_sha1_file(sha1, &type, &size); if (!contents) die("object %s disappeared", sha1_to_hex(sha1)); - if (rtype != type) + if (type != data->type) die("object %s changed type!?", sha1_to_hex(sha1)); - if (rsize != size) - die("object %s change size!?", sha1_to_hex(sha1)); + if (size != data->size) + die("object %s changed size!?", sha1_to_hex(sha1)); write_or_die(fd, contents, size); free(contents); @@ -250,7 +251,7 @@ static int batch_one_object(const char *obj_name, struct batch_options *opt, strbuf_release(&buf); if (opt->print_contents) { - print_object_or_die(1, data->sha1, data->type, data->size); + print_object_or_die(1, data); write_or_die(1, "\n", 1); } return 0; -- cgit v1.2.3 From 6554dfa97af92a159231d94e60403bfe0d6e8e61 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 12 Dec 2013 07:15:50 +0800 Subject: cat-file: handle --batch format with missing type/size Commit 98e2092 taught cat-file to stream blobs with --batch, which requires that we look up the object type before loading it into memory. As a result, we now print the object header from information in sha1_object_info, and the actual contents from the read_sha1_file. We double-check that the information we printed in the header matches the content we are about to show. Later, commit 93d2a60 allowed custom header lines for --batch, and commit 5b08640 made type lookups optional. As a result, specifying a header line without the type or size means that we will not look up those items at all. This causes our double-checking to erroneously die with an error; we think the type or size has changed, when in fact it was simply left at "0". For the size, we can fix this by only doing the consistency double-check when we have retrieved the size via sha1_object_info. In the case that we have not retrieved the value, that means we also did not print it, so there is nothing for us to check that we are consistent with. We could do the same for the type. However, besides our consistency check, we also care about the type in deciding whether to stream or not. So instead of handling the case where we do not know the type, this patch instead makes sure that we always trigger a type lookup when we are printing, so that even a format without the type will stream as we would in the normal case. Reviewed-by: Jonathan Nieder Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 11 ++++++++++- t/t1006-cat-file.sh | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 1434afbd4d..f8288c830c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -197,6 +197,8 @@ static void print_object_or_die(int fd, struct expand_data *data) { const unsigned char *sha1 = data->sha1; + assert(data->info.typep); + if (data->type == OBJ_BLOB) { if (stream_blob_to_fd(fd, sha1, NULL, 0) < 0) die("unable to stream %s to stdout", sha1_to_hex(sha1)); @@ -211,7 +213,7 @@ static void print_object_or_die(int fd, struct expand_data *data) die("object %s disappeared", sha1_to_hex(sha1)); if (type != data->type) die("object %s changed type!?", sha1_to_hex(sha1)); - if (size != data->size) + if (data->info.sizep && size != data->size) die("object %s changed size!?", sha1_to_hex(sha1)); write_or_die(fd, contents, size); @@ -275,6 +277,13 @@ static int batch_objects(struct batch_options *opt) strbuf_expand(&buf, opt->format, expand_format, &data); data.mark_query = 0; + /* + * If we are printing out the object, then always fill in the type, + * since we will want to decide whether or not to stream. + */ + if (opt->print_contents) + data.info.typep = &data.type; + /* * We are going to call get_sha1 on a potentially very large number of * objects. In most large cases, these will be actual object sha1s. The diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index a420742494..df2ae67189 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -85,6 +85,28 @@ $content" git cat-file --batch-check="%(objecttype) %(rest)" >actual && test_cmp expect actual ' + + test -z "$content" || + test_expect_success "--batch without type ($type)" ' + { + echo "$size" && + maybe_remove_timestamp "$content" $no_ts + } >expect && + echo $sha1 | git cat-file --batch="%(objectsize)" >actual.full && + maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual && + test_cmp expect actual + ' + + test -z "$content" || + test_expect_success "--batch without size ($type)" ' + { + echo "$type" && + maybe_remove_timestamp "$content" $no_ts + } >expect && + echo $sha1 | git cat-file --batch="%(objecttype)" >actual.full && + maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual && + test_cmp expect actual + ' } hello_content="Hello World" -- cgit v1.2.3