Re: [PATCH 3/3] cat-file: add mailmap support to --batch-check option

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

Siddharth Asthana <siddharthasthana31@gmail.com> writes:

> Using `git cat-file --use-mailmap` with --batch-check option, like the
> following is allowed:
>
>  git cat-file --use-mailmap -batch-check
>
> The current implementation will return the same object size irrespective
> of the mailmap option, which is not as useful as it could be. When we
> use the mailmap mechanism to replace the idents, the size of the object
> can change and --batch-check option would be more useful if it shows the
> size of the changed object. This patch implements that.

Almost the same comment on the proposed log message as [2/3].

> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 9942b93867..93d127d687 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -424,6 +424,12 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d
>  
>  static void print_default_format(struct strbuf *scratch, struct expand_data *data)
>  {
> +	if (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {
> +		size_t s = data->size;
> +		*data->info.contentp = replace_idents_using_mailmap((char*)*data->info.contentp, &s);
> +		data->size = cast_size_t_to_ulong(s);
> +	}
> +
>  	strbuf_addf(scratch, "%s %s %"PRIuMAX"\n", oid_to_hex(&data->oid),
>  		    type_name(data->type),
>  		    (uintmax_t)data->size);
> @@ -441,9 +447,14 @@ static void batch_object_write(const char *obj_name,
>  			       struct packed_git *pack,
>  			       off_t offset)
>  {
> +	void *buf = NULL;
> +
>  	if (!data->skip_object_info) {
>  		int ret;
>  
> +		if (use_mailmap)
> +			data->info.contentp = &buf;
> +
>  		if (pack)
>  			ret = packed_object_info(the_repository, pack, offset,
>  						 &data->info);
> @@ -474,6 +485,8 @@ static void batch_object_write(const char *obj_name,
>  		print_object_or_die(opt, data);
>  		batch_write(opt, "\n", 1);
>  	}
> +
> +	free(buf);

OK.  Do we have _any_ idea what kind of object this is upon entry to
this function so that we can avoid populating .contentp for say a
huge blob object?  Of course, we could probe for type without
loading the contents, something like the attached sketch.  Usually
the blobs and trees are far larger than commits and tags and more
expensive to materialize in core (especially because trees delta so
well), so avoiding the cost to do so may worth it.  I dunno.

diff --git i/builtin/cat-file.c w/builtin/cat-file.c
index 989eee0bb4..562691eb1e 100644
--- i/builtin/cat-file.c
+++ w/builtin/cat-file.c
@@ -431,6 +431,9 @@ static void batch_object_write(const char *obj_name,
 	if (!data->skip_object_info) {
 		int ret;
 
+		if (use_mailmap && !data->info.typep)
+			data->info.typep = &data.type;
+
 		if (pack)
 			ret = packed_object_info(the_repository, pack, offset,
 						 &data->info);
@@ -444,8 +447,14 @@ static void batch_object_write(const char *obj_name,
 			fflush(stdout);
 			return;
 		}
-	}
 
+		if (use_mailmap && 
+		    (*(data->info.typep) == OBJ_COMMIT ||
+		    (*data->info.typep) == OBJ_TAG)) {
+			... load the contents here ...;
+			... replace idents with mailmap ...;
+		}
+	}
 	strbuf_reset(scratch);
 
 	if (!opt->format) {