On Mon, Sep 26 2022, Siddharth Asthana wrote:
> Even though the cat-file command with `-s` option does not complain when
> `--use-mailmap` option is given, the latter option is ignored. Compute
> the size of the object after replacing the idents and report it instead.
>
> In order to make `-s` option honour the mailmap mechanism we have to
> read the contents of the commit/tag object. Make use of the call to
> `oid_object_info_extended()` to get the contents of the object and store
> in `buf`. `buf` is later freed in the function.
>
> Mentored-by: Christian Couder <christian.couder@xxxxxxxxx>
> Mentored-by: John Cai <johncai86@xxxxxxxxx>
> Signed-off-by: Siddharth Asthana <siddharthasthana31@xxxxxxxxx>
> ---
> Documentation/git-cat-file.txt | 4 +++-
> builtin/cat-file.c | 13 +++++++++++++
> t/t4203-mailmap.sh | 10 ++++++++++
> 3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
> index ec30b5c574..594b6f2dfd 100644
> --- a/Documentation/git-cat-file.txt
> +++ b/Documentation/git-cat-file.txt
> @@ -45,7 +45,9 @@ OPTIONS
>
> -s::
> Instead of the content, show the object size identified by
> - `<object>`.
> + `<object>`. If used with `--use-mailmap` option, will show the
> + size of updated object after replacing idents using the mailmap
> + mechanism.
>
> -e::
> Exit with zero status if `<object>` exists and is a valid
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index 989eee0bb4..9942b93867 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
>
> case 's':
> oi.sizep = &size;
> +
> + if (use_mailmap) {
> + oi.typep = &type;
> + oi.contentp = (void**)&buf;
> + }
> +
> if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
> die("git cat-file: could not get object info");
> +
> + if (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {
Just following along here: We want to handle both tag printing and size
computations. I.e. we happily search-replace the author in tag objects:
$ git -P diff -- .mailmap
diff --git a/.mailmap b/.mailmap
index 07db36a9bb9..cace49e462b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -125,7 +125,7 @@ Jonathan del Strother <jon.delStrother@xxxxxxxxxxxxx> <maillist@xxxxxxxxxxxxxx>
Josh Triplett <josh@xxxxxxxxxxxxxxxx> <josh@xxxxxxxxxxxxxxx>
Josh Triplett <josh@xxxxxxxxxxxxxxxx> <josht@xxxxxxxxxx>
Julian Phillips <julian@xxxxxxxxxxxxxxxxx> <jp3@xxxxxxxxxxxxxxxxx>
-Junio C Hamano <gitster@xxxxxxxxx> <gitster@xxxxxxxxx>
+Foo <bar@xxxxxxxx> Junio C Hamano <gitster@xxxxxxxxx>
Junio C Hamano <gitster@xxxxxxxxx> <junio@xxxxxxxxxxxxxxx>
Junio C Hamano <gitster@xxxxxxxxx> <junio@xxxxxxxxxx>
Junio C Hamano <gitster@xxxxxxxxx> <junio@xxxxxxxxx>
$ ./git cat-file --use-mailmap tag v2.37.0 | head -n 4
object e4a4b31577c7419497ac30cebe30d755b97752c5
type commit
tag v2.37.0
tagger Foo <bar@xxxxxxxx> 1656346695 -0700
And we want the "-s" to match, okey, but... (continued below)
> + size_t s = size;
> + buf = replace_idents_using_mailmap(buf, &s);
> + size = cast_size_t_to_ulong(s);
> + }
> +
> printf("%"PRIuMAX"\n", (uintmax_t)size);
> ret = 0;
> goto cleanup;
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
> index cd1cab3e54..59513e7c57 100755
> --- a/t/t4203-mailmap.sh
> +++ b/t/t4203-mailmap.sh
> @@ -1022,4 +1022,14 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj
> test_cmp expect actual
> '
>
> +test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '
> + test_when_finished "rm .mailmap" &&
> + cat >.mailmap <<-EOF &&
nit: use \ before EOF, no variables here.
> + C O Mitter <committer@xxxxxxxxxxx> Orig <orig@xxxxxxxxxxx>
> + EOF
> + echo "220" >expect &&
nit: no need for "" quotes.
> + git cat-file --use-mailmap -s HEAD >actual &&
I'd find this a bit easier to follow if acter setting up .mailmap we did
something like (I didn't look up what the actual "234" value is):
>actual &&
git cat-file -s HEAD >actual &&
git cat-file -s --use-mailmap HEAD >>actual &&
cat >expect <<-\EOF
234
220
EOF
We surely test that somewhere else, but it would be a bit more
self-documenting, as the difference in sizes would correspond to the
size of the address (or a multiple thereof, if it's used replaced N
times).
> + test_cmp expect actual
> +'
...our test only checks the commit handling. Let's be a bit more
defensive here & test both potential paths through that new "if".
