Re: [PATCH 3/3] cat-file: add mailmap support to --batch-check option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 16, 2022, 22:35 UTC
- Message-ID
- <xmqq35creymz.fsf@gitster.g>
- In-Reply-To
- <20220916205946.178925-4-siddharthasthana31@gmail.com>
Siddharth Asthana <siddharthasthana31@gmail.com> writes:
Show 10 quoted lines
> 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].
Show 38 quoted lines
> 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) {