{"thread":{"id":"58447","subject":"[PATCH 0/3] Add mailmap mechanism in --batch-check options","startedAt":"2022-09-16T21:00:07Z","lastAt":"2022-12-20T13:14:48Z","messageCount":44,"participants":["Siddharth Asthana","Junio C Hamano","Ævar Arnfjörð Bjarmason","Taylor Blau","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"463111","messageId":"20220916205946.178925-1-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":null,"subject":"[PATCH 0/3] Add mailmap mechanism in --batch-check options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-09-16T20:59:43Z","receivedAt":"2022-09-16T21:00:07Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Hi Everyone!\n\nI am working as a GSoC mentee with GitLab organization. My projects aims\nto add mailmap support in GitLab. The patch for adding mailmap support\nin git cat-file is already merged. So, using git cat-file\n`--use-mailmap` with either `-s` and `--batch-check option`, like the\nfollowing is allowed:\n\n* git cat-file --use-mailmap -s \n* git cat-file --use-mailmap --batch-check\n\nThe current implementation will return the same object size irrespective\nof the mailmap option, which is not as useful as it could be. When we\nuse the mailmap mechanism to replace idents, the size of the object can\nchange `-s` and `--batch-check` options would be more useful if they\nshow the size of the changed object. This patch series implemets that.\n\nIn this patch series we didn't want to change that how '%(objectsize)`\nalways show the size of the original object even when `--use-mailmap` is\nset because first we are going to unify how the formats for git cat-file\nand other commands work. And second existing formats like the \"pretty\nformats\" used by `git log` have different options for fields respecting\nmailmap or not respecting it (%an is for author name while %aN is for\nauthor name respecting mailmap).\n\nI would like to thank my mentors, Christian Couder and John Cai, for all\nof their help!\nLooking forward to the reviews!\n\n= Patch Organization\n\n- The first patch improves the documentation `--batch`, `--batch-check` and\n `--batch-command` options by adding they can be combined with\n `--use-mailmap` options.\n- The second patch makes -s option to return updated size of the \n object, when combined with `--use-mailmap` options, after replacing the idents\n using the mailmap mechanism.\n- The third patch makes --batch-check option to return updated size of\n the object, when combined with `--use-mailmap` options,\n after replacing the idents using the mailmap mechanism.\n\n\nSiddharth Asthana (3):\n doc/cat-file: allow --use-mailmap for --batch options\n cat-file: add mailmap support to -s option\n cat-file: add mailmap support to --batch-check option\n\n Documentation/git-cat-file.txt | 26 ++++++++++++++------------\n builtin/cat-file.c | 26 ++++++++++++++++++++++++++\n t/t4203-mailmap.sh | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+), 12 deletions(-)\n\n-- \n2.38.0.rc0.3.g53c2677cac\n\n"},{"id":"463112","messageId":"20220916205946.178925-3-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH 2/3] cat-file: add mailmap support to -s option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-09-16T20:59:45Z","receivedAt":"2022-09-16T21:00:16Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Using `git cat-file --use-mailmap` with `-s` option, like the following is\nallowed:\n\n git cat-file --use-mailmap -s \n\nThe current implementation will return the same object size irrespective\nof the mailmap option, which is not as useful as it could be. When we\nuse the mailmap mechanism to replace the idents, the size of the object\ncan change and `-s` option would be more useful if it shows the size of\nthe changed object. This patch implements that.\n\nMentored-by: Christian Couder's avatarChristian Couder \nMentored-by: John Cai's avatarJohn Cai \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 4 +++-\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 10 ++++++++++\n 3 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex 5792f21a72..708d094db4 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -45,7 +45,9 @@ OPTIONS\n \n -s::\n \tInstead of the content, show the object size identified by\n-\t``.\n+\t``. If used with `--use-mailmap` option, will show the\n+\tsize of updated object after replacing idents using the mailmap\n+\tmechanism.\n \n -e::\n \tExit with zero status if `` exists and is a valid\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 989eee0bb4..9942b93867 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \tcase 's':\n \t\toi.sizep = &size;\n+\n+\t\tif (use_mailmap) {\n+\t\t\toi.typep = &type;\n+\t\t\toi.contentp = (void**)&buf;\n+\t\t}\n+\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n+\n+\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n+\t\t\tsize_t s = size;\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tsize = cast_size_t_to_ulong(s);\n+\t\t}\n+\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex cd1cab3e54..59513e7c57 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1022,4 +1022,14 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\techo \"220\" >expect &&\n+\tgit cat-file --use-mailmap -s HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.0.rc0.3.g53c2677cac\n\n"},{"id":"463113","messageId":"20220916205946.178925-2-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH 1/3] doc/cat-file: allow --use-mailmap for --batch options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-09-16T20:59:44Z","receivedAt":"2022-09-16T21:00:16Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"The command git cat-file can now use the mailmap mechanism to replace\nidents with their canonical versions for commit and tag objects. There\nare several options like `--batch`, `--batch-check` and\n`--batch-command` that can be combined with `--use-mailmap`. But, the\ndocumentation for `--batch`, `--batch-check` and `--batch-command`\ndoesn't say so. This patch fixes that documentation.\n\nMentored-by: Christian Couder's avatarChristian Couder \nMentored-by: John Cai's avatarJohn Cai \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 20 +++++++++-----------\n 1 file changed, 9 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex ec30b5c574..5792f21a72 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -89,24 +89,22 @@ OPTIONS\n --batch::\n --batch=::\n \tPrint object information and contents for each object provided\n-\ton stdin. May not be combined with any other options or arguments\n-\texcept `--textconv` or `--filters`, in which case the input lines\n-\talso need to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\ton stdin. May only be combined with `--use-mailmap`, `--textconv` or `--filters`.\n+\tIn the case of `--textconv` or `--filters` the input lines also need to specify\n+\tthe path, separated by whitespace. See the `BATCH OUTPUT` section below for details.\n \n --batch-check::\n --batch-check=::\n-\tPrint object information for each object provided on stdin. May\n-\tnot be combined with any other options or arguments except\n-\t`--textconv` or `--filters`, in which case the input lines also\n-\tneed to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\tPrint object information for each object provided on stdin. May only be combined\n+\twith `--use-mailmap`, `--textconv` or `--filters`. In the case of `--textconv` or\n+\t`--filters` the input lines also need to specify the path, separated by whitespace.\n+\tSee the `BATCH OUTPUT` section below for details.\n \n --batch-command::\n --batch-command=::\n \tEnter a command mode that reads commands and arguments from stdin. May\n-\tonly be combined with `--buffer`, `--textconv` or `--filters`. In the\n-\tcase of `--textconv` or `--filters`, the input lines also need to specify\n+\tonly be combined with `--buffer`, `--textconv`, `--filters` or `--use-mailmap`.\n+\tIn the case of `--textconv` or `--filters`, the input lines also need to specify\n \tthe path, separated by whitespace. See the section `BATCH OUTPUT` below\n \tfor details.\n +\n-- \n2.38.0.rc0.3.g53c2677cac\n\n"},{"id":"463114","messageId":"20220916205946.178925-4-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH 3/3] cat-file: add mailmap support to --batch-check option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-09-16T20:59:46Z","receivedAt":"2022-09-16T21:00:42Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Using `git cat-file --use-mailmap` with --batch-check option, like the\nfollowing is allowed:\n\n git cat-file --use-mailmap -batch-check\n\nThe current implementation will return the same object size irrespective\nof the mailmap option, which is not as useful as it could be. When we\nuse the mailmap mechanism to replace the idents, the size of the object\ncan change and --batch-check option would be more useful if it shows the\nsize of the changed object. This patch implements that.\n\nMentored-by: Christian Couder's avatarChristian Couder \nMentored-by: John Cai's avatarJohn Cai \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 2 ++\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 22 ++++++++++++++++++++++\n 3 files changed, 37 insertions(+)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex 708d094db4..0d5cc9335f 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -101,6 +101,8 @@ OPTIONS\n \twith `--use-mailmap`, `--textconv` or `--filters`. In the case of `--textconv` or\n \t`--filters` the input lines also need to specify the path, separated by whitespace.\n \tSee the `BATCH OUTPUT` section below for details.\n+\tIf used with `--use-mailmap` option, will show the size of updated object after\n+\treplacing idents using the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 9942b93867..93d127d687 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -424,6 +424,12 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d\n \n static void print_default_format(struct strbuf *scratch, struct expand_data *data)\n {\n+\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n+\t\tsize_t s = data->size;\n+\t\t*data->info.contentp = replace_idents_using_mailmap((char*)*data->info.contentp, &s);\n+\t\tdata->size = cast_size_t_to_ulong(s);\n+\t}\n+\n \tstrbuf_addf(scratch, \"%s %s %\"PRIuMAX\"\\n\", oid_to_hex(&data->oid),\n \t\t type_name(data->type),\n \t\t (uintmax_t)data->size);\n@@ -441,9 +447,14 @@ static void batch_object_write(const char *obj_name,\n \t\t\t struct packed_git *pack,\n \t\t\t off_t offset)\n {\n+\tvoid *buf = NULL;\n+\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap)\n+\t\t\tdata->info.contentp = &buf;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -474,6 +485,8 @@ static void batch_object_write(const char *obj_name,\n \t\tprint_object_or_die(opt, data);\n \t\tbatch_write(opt, \"\\n\", 1);\n \t}\n+\n+\tfree(buf);\n }\n \n static void batch_one_object(const char *obj_name,\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 59513e7c57..4b236c68aa 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1032,4 +1032,26 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\techo \"92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\" >expect &&\n+\techo \"HEAD\" >in &&\n+\tgit cat-file --use-mailmap --batch-check actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\techo \"92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\" >expect &&\n+\techo \"info HEAD\" >in &&\n+\tgit cat-file --use-mailmap --batch-command actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.0.rc0.3.g53c2677cac\n\n"},{"id":"463116","messageId":"xmqqk063f056.fsf@gitster.g","threadId":"58447","inReplyTo":"20220916205946.178925-2-siddharthasthana31@gmail.com","subject":"Re: [PATCH 1/3] doc/cat-file: allow --use-mailmap for --batch options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-16T22:02:45Z","receivedAt":"2022-09-16T22:02:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Asthana writes:\n\n> The command git cat-file can now use the mailmap mechanism to replace\n> idents with their canonical versions for commit and tag objects. There\n> are several options like `--batch`, `--batch-check` and\n> `--batch-command` that can be combined with `--use-mailmap`. But, the\n> documentation for `--batch`, `--batch-check` and `--batch-command`\n> doesn't say so. This patch fixes that documentation.\n>\n> Mentored-by: Christian Couder's avatarChristian Couder \n> Mentored-by: John Cai's avatarJohn Cai \n> Signed-off-by: Siddharth Asthana \n> ---\n> Documentation/git-cat-file.txt | 20 +++++++++-----------\n> 1 file changed, 9 insertions(+), 11 deletions(-)\n>\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index ec30b5c574..5792f21a72 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -89,24 +89,22 @@ OPTIONS\n> --batch::\n> --batch=::\n> \tPrint object information and contents for each object provided\n> -\ton stdin. May not be combined with any other options or arguments\n> -\texcept `--textconv` or `--filters`, in which case the input lines\n> -\talso need to specify the path, separated by whitespace. See the\n> -\tsection `BATCH OUTPUT` below for details.\n> +\ton stdin. May only be combined with `--use-mailmap`, `--textconv` or `--filters`.\n> +\tIn the case of `--textconv` or `--filters` the input lines also need to specify\n> +\tthe path, separated by whitespace. See the `BATCH OUTPUT` section below for details.\n\nThe above is not wrong per-se, but I suspect that phrasing it like\nso\n\n * When used with `--textconv` or `--filters`, the input lines must\n specify the path, separated by whitespace.\n\n * When used with `--use-mailmap`, THIS HAPPENS.\n\n Cannot be used with any other options.\n\nwould be easier to extend. The same comment applies to the other\ntwo changes in this patch.\n\nAs you do not have any code that implements the behaviour of\n`--use-mailmap` at this point in the series yet, it might be nicer\nto restructure the existing text for existing options in a\npreliminary preparation patch, and then add the explanation and the\nimplementation of `--use-mailmap` option in a separate patch.\n\nThanks.\n"},{"id":"463117","messageId":"xmqq8rmjez7h.fsf@gitster.g","threadId":"58447","inReplyTo":"20220916205946.178925-3-siddharthasthana31@gmail.com","subject":"Re: [PATCH 2/3] cat-file: add mailmap support to -s option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-16T22:22:58Z","receivedAt":"2022-09-16T22:23:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Asthana writes:\n\n> Using `git cat-file --use-mailmap` with `-s` option, like the following is\n> allowed:\n>\n> git cat-file --use-mailmap -s \n>\n> The current implementation will return the same object size irrespective\n> of the mailmap option, which is not as useful as it could be.\n> When we\n> use the mailmap mechanism to replace the idents, the size of the object\n> can change and `-s` option would be more useful if it shows the size of\n> the changed object. This patch implements that.\n\nSimply put, the current implementation is BUGGY, in other words, no?\n\nThe above sounds like a quite roundabout way to say that. \"the\nfollowing is allowed\" (but it does not work, really). \"is not as\nuseful as it could be\" (it is pretty much useless, in fact).\n\nIf I were doing this step, I'd summarize it into a single three-line\nparagraph, like so:\n\n Even though the cat-file command with -s option does not complain\n when --use-mailmap option is given, it is ignored. Compute the size\n of the object after replacing the idents and report it instead.\n\n> -s::\n> \tInstead of the content, show the object size identified by\n> -\t``.\n> +\t``. If used with `--use-mailmap` option, will show the\n> +\tsize of updated object after replacing idents using the mailmap\n> +\tmechanism.\n\nWe are not modifying the object in the object database, so it would\nbe preferrable to avoid a misleading phrasing \"updated object\", if\nwe can. The size that the object would have had, if the idents\nrecorded in it were the ones \"corrected\" by the mailmap, is what we\nreport.\n\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 989eee0bb4..9942b93867 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n> \n> \tcase 's':\n> \t\toi.sizep = &size;\n> +\n> +\t\tif (use_mailmap) {\n> +\t\t\toi.typep = &type;\n> +\t\t\toi.contentp = (void**)&buf;\n> +\t\t}\n> +\n> \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n> \t\t\tdie(\"git cat-file: could not get object info\");\n> +\n> +\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n> +\t\t\tsize_t s = size;\n> +\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n> +\t\t\tsize = cast_size_t_to_ulong(s);\n> +\t\t}\n> +\n> \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n> \t\tret = 0;\n> \t\tgoto cleanup;\n\nWe did not grab the object contents but just asked the API to fill oi.sizep\nbut now we grab the contents in oi.contentp, and possibly munge it\nvia mailmap.\n\nAre we merely borrowing the object contents from somebody so we do\nnot have to release anything before jumping to \"cleanup\" here?\nWhat's the memory ownership around replace_idents_using_mailmap()?\nYou pass \"buf\" to it, and overwrite \"buf\" with what it returns. If\nyou were responsible for releasing the original \"buf\" you obtained\nfrom oid_object_info_extended(), have you lost the only pointer to\nit that you may have wanted to use to release it by doing so?\n\n ... goes and looks ...\n\nOK, replace_idents_using_mailmap() assumes that object_buf is owned\nby the caller and the caller is willing to let it go. That is why\nit attaches it to a strbuf, calls apply_mailmap_to_header() to munge\nthe strbuf, and detaches. The net effect is that the original \"buf\"\nwe took from oid_object_info_extended() may be freed inside it, and\nthe returned value may be a newly allocated one to replace it, so\noverwriting buf is OK.\n\nWe still need to free \"buf\", but the function assumes that all cases\n\"buf\" would be populated (or left to NULL) and it is OK to free it\nat cleanup: label, so we are not leaking anything here.\n\nGood.\n\n> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\n> index cd1cab3e54..59513e7c57 100755\n> --- a/t/t4203-mailmap.sh\n> +++ b/t/t4203-mailmap.sh\n> @@ -1022,4 +1022,14 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-EOF &&\n> +\tC O Mitter Orig \n> +\tEOF\n> +\techo \"220\" >expect &&\n> +\tgit cat-file --use-mailmap -s HEAD >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n"},{"id":"463118","messageId":"xmqq35creymz.fsf@gitster.g","threadId":"58447","inReplyTo":"20220916205946.178925-4-siddharthasthana31@gmail.com","subject":"Re: [PATCH 3/3] cat-file: add mailmap support to --batch-check option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-09-16T22:35:16Z","receivedAt":"2022-09-16T22:35:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Asthana writes:\n\n> Using `git cat-file --use-mailmap` with --batch-check option, like the\n> following is allowed:\n>\n> git cat-file --use-mailmap -batch-check\n>\n> The current implementation will return the same object size irrespective\n> of the mailmap option, which is not as useful as it could be. When we\n> use the mailmap mechanism to replace the idents, the size of the object\n> can change and --batch-check option would be more useful if it shows the\n> size of the changed object. This patch implements that.\n\nAlmost the same comment on the proposed log message as [2/3].\n\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 9942b93867..93d127d687 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -424,6 +424,12 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d\n> \n> static void print_default_format(struct strbuf *scratch, struct expand_data *data)\n> {\n> +\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n> +\t\tsize_t s = data->size;\n> +\t\t*data->info.contentp = replace_idents_using_mailmap((char*)*data->info.contentp, &s);\n> +\t\tdata->size = cast_size_t_to_ulong(s);\n> +\t}\n> +\n> \tstrbuf_addf(scratch, \"%s %s %\"PRIuMAX\"\\n\", oid_to_hex(&data->oid),\n> \t\t type_name(data->type),\n> \t\t (uintmax_t)data->size);\n> @@ -441,9 +447,14 @@ static void batch_object_write(const char *obj_name,\n> \t\t\t struct packed_git *pack,\n> \t\t\t off_t offset)\n> {\n> +\tvoid *buf = NULL;\n> +\n> \tif (!data->skip_object_info) {\n> \t\tint ret;\n> \n> +\t\tif (use_mailmap)\n> +\t\t\tdata->info.contentp = &buf;\n> +\n> \t\tif (pack)\n> \t\t\tret = packed_object_info(the_repository, pack, offset,\n> \t\t\t\t\t\t &data->info);\n> @@ -474,6 +485,8 @@ static void batch_object_write(const char *obj_name,\n> \t\tprint_object_or_die(opt, data);\n> \t\tbatch_write(opt, \"\\n\", 1);\n> \t}\n> +\n> +\tfree(buf);\n\nOK. Do we have _any_ idea what kind of object this is upon entry to\nthis function so that we can avoid populating .contentp for say a\nhuge blob object? Of course, we could probe for type without\nloading the contents, something like the attached sketch. Usually\nthe blobs and trees are far larger than commits and tags and more\nexpensive to materialize in core (especially because trees delta so\nwell), so avoiding the cost to do so may worth it. I dunno.\n\ndiff --git i/builtin/cat-file.c w/builtin/cat-file.c\nindex 989eee0bb4..562691eb1e 100644\n--- i/builtin/cat-file.c\n+++ w/builtin/cat-file.c\n@@ -431,6 +431,9 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap && !data->info.typep)\n+\t\t\tdata->info.typep = &data.type;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -444,8 +447,14 @@ static void batch_object_write(const char *obj_name,\n \t\t\tfflush(stdout);\n \t\t\treturn;\n \t\t}\n-\t}\n \n+\t\tif (use_mailmap && \n+\t\t (*(data->info.typep) == OBJ_COMMIT ||\n+\t\t (*data->info.typep) == OBJ_TAG)) {\n+\t\t\t... load the contents here ...;\n+\t\t\t... replace idents with mailmap ...;\n+\t\t}\n+\t}\n \tstrbuf_reset(scratch);\n \n \tif (!opt->format) {\n\n"},{"id":"463617","messageId":"20220926105343.233296-1-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH v2 0/2] Add mailmap mechanism in cat-file options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-09-26T10:53:41Z","receivedAt":"2022-09-26T12:06:32Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Thanks a lot Junio for the review :) I have made the suggested changes.\n\n= Description\n\nAt present, `git-cat-file` command with `--batch-check` and `-s` options\ndoes not complain when `--use-mailmap` option is given. The latter\noption is just ignored. Instead, for commit/tag objects, the command\nshould compute the size of the object after replacing the idents and\nreport it. So, this patch series makes `-s` and `--batch-check` options\nof `git-cat-file` honor mailmap when used with `--use-mailmap` option.\n\nIn this patch series we didn't want to change that '%(objectsize)'\nalways shows the size of the original object even when `--use-mailmap`\nis set because first we have the long term plan to unify how the formats\nfor `git cat-file` and other commands works. And second existing formats\nlike the \"pretty formats\" used bt `git log` have different options for\nfields respecting mailmap or not respecting it (%an is for author name\nwhile %aN for author name respecting mailmap).\n\nI would like to thank my mentors, Christian Couder and John Cai, for all\nof their help!\nLooking forward to the reviews!\n\n= Patch Organization\n\n- The first patch makes `-s` option to return updated size of the\n object, when combined with `--use-mailmap` option, after\n replacing the idents using the mailmap mechanism.\n- The second patch makes `--batch-check` option to return updated size of\n the object, when combined with `--use-mailmap` option,\n after replacing the idents using the mailmap mechanism.\n\n= Changes in v2:\n\n- The commit messages of both the patches have been improved.\n- In the second patch, we were populating the `contentp` field of the\n `object_info` structure when `--batch-check` was combined with\n `--use-mailmap`. Which made us read the contents of tree and blob\n object types as well, which affected the performance. We should only\n be reading the contents for commit or tag object types. The second\n patch has been updated to do just that.\n\nSiddharth Asthana (2):\n cat-file: add mailmap support to -s option\n cat-file: add mailmap support to --batch-check option\n\n Documentation/git-cat-file.txt | 6 +++++-\n builtin/cat-file.c | 27 +++++++++++++++++++++++++++\n t/t4203-mailmap.sh | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 64 insertions(+), 1 deletion(-)\n\nRange-diff against v1:\n1: 513ad3b5f7 < -: ---------- doc/cat-file: allow --use-mailmap for --batch options\n2: 6f3dcce9e3 ! 1: 60cf7bc28c cat-file: add mailmap support to -s option\n @@ Metadata\n ## Commit message ##\n cat-file: add mailmap support to -s option\n \n - Using `git cat-file --use-mailmap` with `-s` option, like the following is\n - allowed:\n + Even though the cat-file command with `-s` option does not complain when\n + `--use-mailmap` option is given, the latter option is ignored. Compute\n + the size of the object after replacing the idents and report it instead.\n \n - git cat-file --use-mailmap -s \n + In order to make `-s` option honour the mailmap mechanism we have to\n + read the contents of the commit/tag object. Make use of the call to\n + `oid_object_info_extended()` to get the contents of the object and store\n + in `buf`. `buf` is later freed in the function.\n \n - The current implementation will return the same object size irrespective\n - of the mailmap option, which is not as useful as it could be. When we\n - use the mailmap mechanism to replace the idents, the size of the object\n - can change and `-s` option would be more useful if it shows the size of\n - the changed object. This patch implements that.\n -\n - Mentored-by: Christian Couder's avatarChristian Couder \n - Mentored-by: John Cai's avatarJohn Cai \n + Mentored-by: Christian Couder \n + Mentored-by: John Cai \n Signed-off-by: Siddharth Asthana \n \n ## Documentation/git-cat-file.txt ##\n3: af90241d32 ! 2: 06c74dd017 cat-file: add mailmap support to --batch-check option\n @@ Metadata\n ## Commit message ##\n cat-file: add mailmap support to --batch-check option\n \n - Using `git cat-file --use-mailmap` with --batch-check option, like the\n - following is allowed:\n + Even though the cat-file command with `--batch-check` option does not\n + complain when `--use-mailmap` option is given, the latter option is\n + ignored. Compute the size of the object after replacing the idents and\n + report it instead.\n \n - git cat-file --use-mailmap -batch-check\n + In order to make `--batch-check` option honour the mailmap mechanism we\n + have to read the contents of the commit/tag object.\n \n - The current implementation will return the same object size irrespective\n - of the mailmap option, which is not as useful as it could be. When we\n - use the mailmap mechanism to replace the idents, the size of the object\n - can change and --batch-check option would be more useful if it shows the\n - size of the changed object. This patch implements that.\n + There were two ways to do it:\n \n - Mentored-by: Christian Couder's avatarChristian Couder \n - Mentored-by: John Cai's avatarJohn Cai \n + 1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n + option is given, the first call will get us the type of the object\n + and second call will only be made if the object type is either a\n + commit or tag to get the contents of the object.\n +\n + 2. Make one call to `oid_object_info_extended()` to get the type of the\n + object. Then, if the object type is either of commit or tag, make a\n + call to `read_object_file()` to read the contents of the object.\n +\n + I benchmarked the following command with both the above approaches and\n + compared against the current implementation where `--use-mailmap`\n + option is ignored:\n +\n + `git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n + --unordered`\n +\n + The results can be summarized as follows:\n + Time (mean ± σ)\n + default 827.7 ms ± 104.8 ms\n + first approach 6.197 s ± 0.093 s\n + second approach 1.975 s ± 0.217 s\n +\n + Since, the second approach is faster than the first one, I implemented\n + it in this patch.\n +\n + Mentored-by: Christian Couder \n + Mentored-by: John Cai \n Signed-off-by: Siddharth Asthana \n \n ## Documentation/git-cat-file.txt ##\n @@ Documentation/git-cat-file.txt: OPTIONS\n - \twith `--use-mailmap`, `--textconv` or `--filters`. In the case of `--textconv` or\n - \t`--filters` the input lines also need to specify the path, separated by whitespace.\n - \tSee the `BATCH OUTPUT` section below for details.\n -+\tIf used with `--use-mailmap` option, will show the size of updated object after\n -+\treplacing idents using the mailmap mechanism.\n + \t`--textconv` or `--filters`, in which case the input lines also\n + \tneed to specify the path, separated by whitespace. See the\n + \tsection `BATCH OUTPUT` below for details.\n ++\tIf used with `--use-mailmap` option, will show the size of\n ++\tupdated object after replacing idents using the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\n \n ## builtin/cat-file.c ##\n -@@ builtin/cat-file.c: static void print_object_or_die(struct batch_options *opt, struct expand_data *d\n - \n - static void print_default_format(struct strbuf *scratch, struct expand_data *data)\n - {\n -+\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n -+\t\tsize_t s = data->size;\n -+\t\t*data->info.contentp = replace_idents_using_mailmap((char*)*data->info.contentp, &s);\n -+\t\tdata->size = cast_size_t_to_ulong(s);\n -+\t}\n -+\n - \tstrbuf_addf(scratch, \"%s %s %\"PRIuMAX\"\\n\", oid_to_hex(&data->oid),\n - \t\t type_name(data->type),\n - \t\t (uintmax_t)data->size);\n @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n - \t\t\t struct packed_git *pack,\n - \t\t\t off_t offset)\n - {\n -+\tvoid *buf = NULL;\n -+\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n +\t\tif (use_mailmap)\n -+\t\t\tdata->info.contentp = &buf;\n ++\t\t\tdata->info.typep = &data->type;\n +\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n - \t\tprint_object_or_die(opt, data);\n - \t\tbatch_write(opt, \"\\n\", 1);\n - \t}\n + \t\t\tfflush(stdout);\n + \t\t\treturn;\n + \t\t}\n +\n -+\tfree(buf);\n - }\n ++\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n ++\t\t\tsize_t s = data->size;\n ++\t\t\tchar *buf = NULL;\n ++\n ++\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n ++\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n ++\t\t\tdata->size = cast_size_t_to_ulong(s);\n ++\n ++\t\t\tfree(buf);\n ++\t\t}\n + \t}\n \n - static void batch_one_object(const char *obj_name,\n + \tstrbuf_reset(scratch);\n \n ## t/t4203-mailmap.sh ##\n @@ t/t4203-mailmap.sh: test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n-- \n2.38.0.rc1.8.g9592ff2ba4\n\n"},{"id":"463618","messageId":"20220926105343.233296-2-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220926105343.233296-1-siddharthasthana31@gmail.com","subject":"[PATCH v2 1/2] cat-file: add mailmap support to -s option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-09-26T10:53:42Z","receivedAt":"2022-09-26T12:06:34Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `-s` option does not complain when\n`--use-mailmap` option is given, the latter option is ignored. Compute\nthe size of the object after replacing the idents and report it instead.\n\nIn order to make `-s` option honour the mailmap mechanism we have to\nread the contents of the commit/tag object. Make use of the call to\n`oid_object_info_extended()` to get the contents of the object and store\nin `buf`. `buf` is later freed in the function.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 4 +++-\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 10 ++++++++++\n 3 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex ec30b5c574..594b6f2dfd 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -45,7 +45,9 @@ OPTIONS\n \n -s::\n \tInstead of the content, show the object size identified by\n-\t``.\n+\t``. If used with `--use-mailmap` option, will show the\n+\tsize of updated object after replacing idents using the mailmap\n+\tmechanism.\n \n -e::\n \tExit with zero status if `` exists and is a valid\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 989eee0bb4..9942b93867 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \tcase 's':\n \t\toi.sizep = &size;\n+\n+\t\tif (use_mailmap) {\n+\t\t\toi.typep = &type;\n+\t\t\toi.contentp = (void**)&buf;\n+\t\t}\n+\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n+\n+\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n+\t\t\tsize_t s = size;\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tsize = cast_size_t_to_ulong(s);\n+\t\t}\n+\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex cd1cab3e54..59513e7c57 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1022,4 +1022,14 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\techo \"220\" >expect &&\n+\tgit cat-file --use-mailmap -s HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.0.rc1.8.g9592ff2ba4\n\n"},{"id":"463619","messageId":"20220926105343.233296-3-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220926105343.233296-1-siddharthasthana31@gmail.com","subject":"[PATCH v2 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-09-26T10:53:43Z","receivedAt":"2022-09-26T12:06:37Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `--batch-check` option does not\ncomplain when `--use-mailmap` option is given, the latter option is\nignored. Compute the size of the object after replacing the idents and\nreport it instead.\n\nIn order to make `--batch-check` option honour the mailmap mechanism we\nhave to read the contents of the commit/tag object.\n\nThere were two ways to do it:\n\n1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n option is given, the first call will get us the type of the object\n and second call will only be made if the object type is either a\n commit or tag to get the contents of the object.\n\n2. Make one call to `oid_object_info_extended()` to get the type of the\n object. Then, if the object type is either of commit or tag, make a\n call to `read_object_file()` to read the contents of the object.\n\nI benchmarked the following command with both the above approaches and\ncompared against the current implementation where `--use-mailmap`\noption is ignored:\n\n`git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n--unordered`\n\nThe results can be summarized as follows:\n Time (mean ± σ)\ndefault 827.7 ms ± 104.8 ms\nfirst approach 6.197 s ± 0.093 s\nsecond approach 1.975 s ± 0.217 s\n\nSince, the second approach is faster than the first one, I implemented\nit in this patch.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 2 ++\n builtin/cat-file.c | 14 ++++++++++++++\n t/t4203-mailmap.sh | 22 ++++++++++++++++++++++\n 3 files changed, 38 insertions(+)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex 594b6f2dfd..a8e7906b3d 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -103,6 +103,8 @@ OPTIONS\n \t`--textconv` or `--filters`, in which case the input lines also\n \tneed to specify the path, separated by whitespace. See the\n \tsection `BATCH OUTPUT` below for details.\n+\tIf used with `--use-mailmap` option, will show the size of\n+\tupdated object after replacing idents using the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 9942b93867..7ee8cb453f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -444,6 +444,9 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap)\n+\t\t\tdata->info.typep = &data->type;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -457,6 +460,17 @@ static void batch_object_write(const char *obj_name,\n \t\t\tfflush(stdout);\n \t\t\treturn;\n \t\t}\n+\n+\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n+\t\t\tsize_t s = data->size;\n+\t\t\tchar *buf = NULL;\n+\n+\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tdata->size = cast_size_t_to_ulong(s);\n+\n+\t\t\tfree(buf);\n+\t\t}\n \t}\n \n \tstrbuf_reset(scratch);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 59513e7c57..4b236c68aa 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1032,4 +1032,26 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\techo \"92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\" >expect &&\n+\techo \"HEAD\" >in &&\n+\tgit cat-file --use-mailmap --batch-check actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\techo \"92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\" >expect &&\n+\techo \"info HEAD\" >in &&\n+\tgit cat-file --use-mailmap --batch-command actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.0.rc1.8.g9592ff2ba4\n\n"},{"id":"463625","messageId":"220926.86leq61d85.gmgdl@evledraar.gmail.com","threadId":"58447","inReplyTo":"20220926105343.233296-2-siddharthasthana31@gmail.com","subject":"Re: [PATCH v2 1/2] cat-file: add mailmap support to -s option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-09-26T13:16:57Z","receivedAt":"2022-09-26T14:55:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Sep 26 2022, Siddharth Asthana wrote:\n\n> Even though the cat-file command with `-s` option does not complain when\n> `--use-mailmap` option is given, the latter option is ignored. Compute\n> the size of the object after replacing the idents and report it instead.\n>\n> In order to make `-s` option honour the mailmap mechanism we have to\n> read the contents of the commit/tag object. Make use of the call to\n> `oid_object_info_extended()` to get the contents of the object and store\n> in `buf`. `buf` is later freed in the function.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: John Cai \n> Signed-off-by: Siddharth Asthana \n> ---\n> Documentation/git-cat-file.txt | 4 +++-\n> builtin/cat-file.c | 13 +++++++++++++\n> t/t4203-mailmap.sh | 10 ++++++++++\n> 3 files changed, 26 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index ec30b5c574..594b6f2dfd 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -45,7 +45,9 @@ OPTIONS\n> \n> -s::\n> \tInstead of the content, show the object size identified by\n> -\t``.\n> +\t``. If used with `--use-mailmap` option, will show the\n> +\tsize of updated object after replacing idents using the mailmap\n> +\tmechanism.\n> \n> -e::\n> \tExit with zero status if `` exists and is a valid\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 989eee0bb4..9942b93867 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n> \n> \tcase 's':\n> \t\toi.sizep = &size;\n> +\n> +\t\tif (use_mailmap) {\n> +\t\t\toi.typep = &type;\n> +\t\t\toi.contentp = (void**)&buf;\n> +\t\t}\n> +\n> \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n> \t\t\tdie(\"git cat-file: could not get object info\");\n> +\n> +\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n> +\t\t\tsize_t s = size;\n> +\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n\nThis is partially commentary on your already-landed series for cat-file\n--mailmap support. I wondered why we needed this temporary variable, and\nwhy we needed the cast_size_t_to_ulong() at all. On \"master\" we have a\nsize, but e.g. for cat_one_file()'s *current* codpaths we just pass it\nto write_or_die().\n\nSo the net effect is that we refuse to use write_or_die() if the number\nin size_t doesn't fit an unsigned long, even though we never need an\nunsigned long in that case.\n\nWe have *other* things in the object code that need unsigned long, so it\nprobably amounts to no practical limitation, but it's confusing & I\nthink per [1] below we could do away with it.\n\nThere's also a subtle gotcha on \"master\", we\nreplace_idents_using_mailmap() with a possibly NULL \"contents\", which is\na misuse of the strbuf API (the \"buf\" member should never be NULL), but\nwe're about to die anyway...\n\n> +\t\t\tsize = cast_size_t_to_ulong(s);\n> +\t\t}\n> +\n> \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n\n...but expanding on \"master\", here we have seemingly the first use of\ncast_size_t_to_ulong() thaht's actually needed in this file. I.e. we are\nabout to use PRIuMAX.\n\nBut why not skip the cast(s) and make this more obvious by having the\nprintf() argument be cast_size_t_to_ulong(size)?\n\nIn your 2/2 you then have another use of cast_size_t_to_ulong() which\n*is* needed in that case (we're about to stick it in a \"unsigned long\"\nmember, and the \"size_t s\" temporary variable is also needed in that\ncase.\n\n\n1.\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 989eee0bb4c..676c34cba4b 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -178,11 +178,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tif (!buf)\n \t\t\tdie(\"Cannot read object %s\", obj_name);\n \n-\t\tif (use_mailmap) {\n-\t\t\tsize_t s = size;\n-\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n-\t\t\tsize = cast_size_t_to_ulong(s);\n-\t\t}\n+\t\tif (use_mailmap)\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &size);\n \n \t\t/* otherwise just spit out the data */\n \t\tbreak;\n@@ -218,11 +215,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \t\tbuf = read_object_with_reference(the_repository, &oid,\n \t\t\t\t\t\t exp_type_id, &size, NULL);\n \n-\t\tif (use_mailmap) {\n-\t\t\tsize_t s = size;\n-\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n-\t\t\tsize = cast_size_t_to_ulong(s);\n-\t\t}\n+\t\tif (use_mailmap)\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &size);\n \t\tbreak;\n \t}\n \tdefault:\n@@ -391,12 +385,8 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d\n \n \t\tcontents = read_object_file(oid, &type, &size);\n \n-\t\tif (use_mailmap) {\n-\t\t\tsize_t s = size;\n-\t\t\tcontents = replace_idents_using_mailmap(contents, &s);\n-\t\t\tsize = cast_size_t_to_ulong(s);\n-\t\t}\n-\n+\t\tif (use_mailmap)\n+\t\t\tcontents = replace_idents_using_mailmap(contents, &size);\n \t\tif (!contents)\n \t\t\tdie(\"object %s disappeared\", oid_to_hex(oid));\n \t\tif (type != data->type)\n"},{"id":"463628","messageId":"220926.86h70u1ct3.gmgdl@evledraar.gmail.com","threadId":"58447","inReplyTo":"20220926105343.233296-2-siddharthasthana31@gmail.com","subject":"Re: [PATCH v2 1/2] cat-file: add mailmap support to -s option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-09-26T13:25:44Z","receivedAt":"2022-09-26T15:02:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Sep 26 2022, Siddharth Asthana wrote:\n\n> Even though the cat-file command with `-s` option does not complain when\n> `--use-mailmap` option is given, the latter option is ignored. Compute\n> the size of the object after replacing the idents and report it instead.\n>\n> In order to make `-s` option honour the mailmap mechanism we have to\n> read the contents of the commit/tag object. Make use of the call to\n> `oid_object_info_extended()` to get the contents of the object and store\n> in `buf`. `buf` is later freed in the function.\n>\n> Mentored-by: Christian Couder \n> Mentored-by: John Cai \n> Signed-off-by: Siddharth Asthana \n> ---\n> Documentation/git-cat-file.txt | 4 +++-\n> builtin/cat-file.c | 13 +++++++++++++\n> t/t4203-mailmap.sh | 10 ++++++++++\n> 3 files changed, 26 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index ec30b5c574..594b6f2dfd 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -45,7 +45,9 @@ OPTIONS\n> \n> -s::\n> \tInstead of the content, show the object size identified by\n> -\t``.\n> +\t``. If used with `--use-mailmap` option, will show the\n> +\tsize of updated object after replacing idents using the mailmap\n> +\tmechanism.\n> \n> -e::\n> \tExit with zero status if `` exists and is a valid\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 989eee0bb4..9942b93867 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n> \n> \tcase 's':\n> \t\toi.sizep = &size;\n> +\n> +\t\tif (use_mailmap) {\n> +\t\t\toi.typep = &type;\n> +\t\t\toi.contentp = (void**)&buf;\n> +\t\t}\n> +\n> \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n> \t\t\tdie(\"git cat-file: could not get object info\");\n> +\n> +\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n\nJust following along here: We want to handle both tag printing and size\ncomputations. I.e. we happily search-replace the author in tag objects:\n\t\n\t$ git -P diff -- .mailmap\n\tdiff --git a/.mailmap b/.mailmap\n\tindex 07db36a9bb9..cace49e462b 100644\n\t--- a/.mailmap\n\t+++ b/.mailmap\n\t@@ -125,7 +125,7 @@ Jonathan del Strother \n\t Josh Triplett \n\t Josh Triplett \n\t Julian Phillips \n\t-Junio C Hamano \n\t+Foo Junio C Hamano \n\t Junio C Hamano \n\t Junio C Hamano \n\t Junio C Hamano \n\t$ ./git cat-file --use-mailmap tag v2.37.0 | head -n 4\n\tobject e4a4b31577c7419497ac30cebe30d755b97752c5\n\ttype commit\n\ttag v2.37.0\n\ttagger Foo 1656346695 -0700\n\nAnd we want the \"-s\" to match, okey, but... (continued below)\n\n> +\t\t\tsize_t s = size;\n> +\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n> +\t\t\tsize = cast_size_t_to_ulong(s);\n> +\t\t}\n> +\n> \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n> \t\tret = 0;\n> \t\tgoto cleanup;\n> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\n> index cd1cab3e54..59513e7c57 100755\n> --- a/t/t4203-mailmap.sh\n> +++ b/t/t4203-mailmap.sh\n> @@ -1022,4 +1022,14 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-EOF &&\n\nnit: use \\ before EOF, no variables here.\n\n> +\tC O Mitter Orig \n> +\tEOF\n> +\techo \"220\" >expect &&\n\nnit: no need for \"\" quotes.\n\n> +\tgit cat-file --use-mailmap -s HEAD >actual &&\n\nI'd find this a bit easier to follow if acter setting up .mailmap we did\nsomething like (I didn't look up what the actual \"234\" value is):\n\n\t>actual &&\n\tgit cat-file -s HEAD >actual &&\n\tgit cat-file -s --use-mailmap HEAD >>actual &&\n\tcat >expect <<-\\EOF\n 234\n 220\n\tEOF\n\nWe surely test that somewhere else, but it would be a bit more\nself-documenting, as the difference in sizes would correspond to the\nsize of the address (or a multiple thereof, if it's used replaced N\ntimes).\n\n> +\ttest_cmp expect actual\n> +'\n\n...our test only checks the commit handling. Let's be a bit more\ndefensive here & test both potential paths through that new \"if\".\n"},{"id":"466011","messageId":"20221029102459.82428-2-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221029102459.82428-1-siddharthasthana31@gmail.com","subject":"[PATCH v3 1/2] cat-file: add mailmap support to -s option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-10-29T10:24:58Z","receivedAt":"2022-10-29T10:25:22Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `-s` option does not complain when\n`--use-mailmap` option is given, the latter option is ignored. Compute\nthe size of the object after replacing the idents and report it instead.\n\nIn order to make `-s` option honour the mailmap mechanism we have to\nread the contents of the commit/tag object. Make use of the call to\n`oid_object_info_extended()` to get the contents of the object and store\nin `buf`. `buf` is later freed in the function.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 4 +++-\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 29 +++++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex ec30b5c574..594b6f2dfd 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -45,7 +45,9 @@ OPTIONS\n \n -s::\n \tInstead of the content, show the object size identified by\n-\t``.\n+\t``. If used with `--use-mailmap` option, will show the\n+\tsize of updated object after replacing idents using the mailmap\n+\tmechanism.\n \n -e::\n \tExit with zero status if `` exists and is a valid\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex fa7bd89169..8a6e2343ec 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \tcase 's':\n \t\toi.sizep = &size;\n+\n+\t\tif (use_mailmap) {\n+\t\t\toi.typep = &type;\n+\t\t\toi.contentp = (void**)&buf;\n+\t\t}\n+\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n+\n+\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n+\t\t\tsize_t s = size;\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tsize = cast_size_t_to_ulong(s);\n+\t\t}\n+\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex cd1cab3e54..b500b31c92 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1022,4 +1022,33 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcat >expect <<-\\EOF &&\n+\t209\n+\t220\n+\tEOF\n+\tgit cat-file -s HEAD >actual &&\n+\tgit cat-file --use-mailmap -s HEAD >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap for tag objects' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tOrig C O Mitter \n+\tEOF\n+\tgit tag -a -m \"annotated tag\" v3 &&\n+\tcat >expect <<-\\EOF &&\n+\t141\n+\t130\n+\tEOF\n+\tgit cat-file -s v3 >actual &&\n+\tgit cat-file --use-mailmap -s v3 >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.1.282.g2e87897fbb\n\n"},{"id":"466012","messageId":"20221029102459.82428-1-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH v3 0/2] Add mailmap mechanism in cat-file options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-10-29T10:24:57Z","receivedAt":"2022-10-29T10:25:22Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Thanks a lot Avar for the review :) I have made the suggested changes.\n\n= Description\n\nAt present, `git-cat-file` command with `--batch-check` and `-s` options\ndoes not complain when `--use-mailmap` option is given. The latter\noption is just ignored. Instead, for commit/tag objects, the command\nshould compute the size of the object after replacing the idents and\nreport it. So, this patch series makes `-s` and `--batch-check` options\nof `git-cat-file` honor mailmap when used with `--use-mailmap` option.\n\nIn this patch series we didn't want to change that '%(objectsize)'\nalways shows the size of the original object even when `--use-mailmap`\nis set because first we have the long term plan to unify how the formats\nfor `git cat-file` and other commands works. And second existing formats\nlike the \"pretty formats\" used bt `git log` have different options for\nfields respecting mailmap or not respecting it (%an is for author name\nwhile %aN for author name respecting mailmap).\n\nI would like to thank my mentors, Christian Couder and John Cai, for all\nof their help!\nLooking forward to the reviews!\n\n= Patch Organization\n\n- The first patch makes `-s` option to return updated size of the\n object, when combined with `--use-mailmap` option, after\n replacing the idents using the mailmap mechanism.\n- The second patch makes `--batch-check` option to return updated size of\n the object, when combined with `--use-mailmap` option,\n after replacing the idents using the mailmap mechanism.\n\n= Changes in v2:\n\n- The commit messages of both the patches have been improved.\n- In the second patch, we were populating the `contentp` field of the\n `object_info` structure when `--batch-check` was combined with\n `--use-mailmap`. Which made us read the contents of tree and blob\n object types as well, which affected the performance. We should only\n be reading the contents for commit or tag object types. The second\n patch has been updated to do just that.\n\n= Changes in v3:\n\n- Make the tests a bit more self documenting by running commands in test\n without using mailmap flag.\n- In the first patch added the test for tag objects as well.\n\nSiddharth Asthana (2):\n cat-file: add mailmap support to -s option\n cat-file: add mailmap support to --batch-check option\n\n Documentation/git-cat-file.txt | 6 +++-\n builtin/cat-file.c | 27 ++++++++++++++++\n t/t4203-mailmap.sh | 59 ++++++++++++++++++++++++++++++++++\n 3 files changed, 91 insertions(+), 1 deletion(-)\n\nRange-diff against v2:\n-: ---------- > 1: 4b0231504b cat-file: add mailmap support to -s option\n1: 0e142beb2b ! 2: 2e87897fbb cat-file: add mailmap support to --batch-check option\n @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n \tstrbuf_reset(scratch);\n \n ## t/t4203-mailmap.sh ##\n -@@ t/t4203-mailmap.sh: test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n +@@ t/t4203-mailmap.sh: test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n \ttest_cmp expect actual\n '\n \n +test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n +\ttest_when_finished \"rm .mailmap\" &&\n -+\tcat >.mailmap <<-EOF &&\n ++\tcat >.mailmap <<-\\EOF &&\n +\tC O Mitter Orig \n +\tEOF\n -+\techo \"92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\" >expect &&\n ++\tcat >expect <<-\\EOF &&\n ++\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n ++\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n ++\tEOF\n +\techo \"HEAD\" >in &&\n -+\tgit cat-file --use-mailmap --batch-check actual &&\n ++\tgit cat-file --batch-check actual &&\n ++\tgit cat-file --use-mailmap --batch-check >actual &&\n +\ttest_cmp expect actual\n +'\n +\n +test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n +\ttest_when_finished \"rm .mailmap\" &&\n -+\tcat >.mailmap <<-EOF &&\n ++\tcat >.mailmap <<-\\EOF &&\n +\tC O Mitter Orig \n +\tEOF\n -+\techo \"92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\" >expect &&\n ++\tcat >expect <<-\\EOF &&\n ++\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n ++\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n ++\tEOF\n +\techo \"info HEAD\" >in &&\n -+\tgit cat-file --use-mailmap --batch-command actual &&\n ++\tgit cat-file --batch-command actual &&\n ++\tgit cat-file --use-mailmap --batch-command >actual &&\n +\ttest_cmp expect actual\n +'\n +\n2: e95cbd7932 < -: ---------- Add Test\n-- \n2.38.1.282.g2e87897fbb\n\n"},{"id":"466013","messageId":"20221029102459.82428-3-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221029102459.82428-1-siddharthasthana31@gmail.com","subject":"[PATCH v3 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-10-29T10:24:59Z","receivedAt":"2022-10-29T10:25:29Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `--batch-check` option does not\ncomplain when `--use-mailmap` option is given, the latter option is\nignored. Compute the size of the object after replacing the idents and\nreport it instead.\n\nIn order to make `--batch-check` option honour the mailmap mechanism we\nhave to read the contents of the commit/tag object.\n\nThere were two ways to do it:\n\n1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n option is given, the first call will get us the type of the object\n and second call will only be made if the object type is either a\n commit or tag to get the contents of the object.\n\n2. Make one call to `oid_object_info_extended()` to get the type of the\n object. Then, if the object type is either of commit or tag, make a\n call to `read_object_file()` to read the contents of the object.\n\nI benchmarked the following command with both the above approaches and\ncompared against the current implementation where `--use-mailmap`\noption is ignored:\n\n`git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n--unordered`\n\nThe results can be summarized as follows:\n Time (mean ± σ)\ndefault 827.7 ms ± 104.8 ms\nfirst approach 6.197 s ± 0.093 s\nsecond approach 1.975 s ± 0.217 s\n\nSince, the second approach is faster than the first one, I implemented\nit in this patch.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 2 ++\n builtin/cat-file.c | 14 ++++++++++++++\n t/t4203-mailmap.sh | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 46 insertions(+)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex 594b6f2dfd..a8e7906b3d 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -103,6 +103,8 @@ OPTIONS\n \t`--textconv` or `--filters`, in which case the input lines also\n \tneed to specify the path, separated by whitespace. See the\n \tsection `BATCH OUTPUT` below for details.\n+\tIf used with `--use-mailmap` option, will show the size of\n+\tupdated object after replacing idents using the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 8a6e2343ec..39f2a2483f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -444,6 +444,9 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap)\n+\t\t\tdata->info.typep = &data->type;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -457,6 +460,17 @@ static void batch_object_write(const char *obj_name,\n \t\t\tfflush(stdout);\n \t\t\treturn;\n \t\t}\n+\n+\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n+\t\t\tsize_t s = data->size;\n+\t\t\tchar *buf = NULL;\n+\n+\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tdata->size = cast_size_t_to_ulong(s);\n+\n+\t\t\tfree(buf);\n+\t\t}\n \t}\n \n \tstrbuf_reset(scratch);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex b500b31c92..b4355c3e51 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1051,4 +1051,34 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcat >expect <<-\\EOF &&\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n+\tEOF\n+\techo \"HEAD\" >in &&\n+\tgit cat-file --batch-check actual &&\n+\tgit cat-file --use-mailmap --batch-check >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcat >expect <<-\\EOF &&\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n+\tEOF\n+\techo \"info HEAD\" >in &&\n+\tgit cat-file --batch-command actual &&\n+\tgit cat-file --use-mailmap --batch-command >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.1.282.g2e87897fbb\n\n"},{"id":"466026","messageId":"Y11qKQCWvdH5zDYk@nand.local","threadId":"58447","inReplyTo":"20221029102459.82428-1-siddharthasthana31@gmail.com","subject":"Re: [PATCH v3 0/2] Add mailmap mechanism in cat-file options","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-10-29T18:00:09Z","receivedAt":"2022-10-29T18:00:17Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sat, Oct 29, 2022 at 03:54:57PM +0530, Siddharth Asthana wrote:\n> Siddharth Asthana (2):\n> cat-file: add mailmap support to -s option\n> cat-file: add mailmap support to --batch-check option\n>\n> Documentation/git-cat-file.txt | 6 +++-\n> builtin/cat-file.c | 27 ++++++++++++++++\n> t/t4203-mailmap.sh | 59 ++++++++++++++++++++++++++++++++++\n> 3 files changed, 91 insertions(+), 1 deletion(-)\n\nThis approach in this round looks OK to my eyes. Would some of the\nother reviewers (perhaps Ævar or Christian) chime in and see if they\nagree?\n\nThanks,\nTaylor\n"},{"id":"466081","messageId":"CAP8UFD3tE6cF7y_zF=-eaFAr3Pw_GcTr7C-zyb2ujBBLDG-ojg@mail.gmail.com","threadId":"58447","inReplyTo":"20221029102459.82428-3-siddharthasthana31@gmail.com","subject":"Re: [PATCH v3 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-10-31T11:43:13Z","receivedAt":"2022-10-31T11:43:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Oct 29, 2022 at 12:25 PM Siddharth Asthana\n wrote:\n\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index 594b6f2dfd..a8e7906b3d 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -103,6 +103,8 @@ OPTIONS\n> `--textconv` or `--filters`, in which case the input lines also\n> need to specify the path, separated by whitespace. See the\n> section `BATCH OUTPUT` below for details.\n> + If used with `--use-mailmap` option, will show the size of\n> + updated object after replacing idents using the mailmap mechanism.\n\nI think all documentation changes should probably go to the\ndocumentation patch that was split off this series, instead of in this\nseries.\n\nWhether the other patch gets merged before or after this series, I\nthink it will simplify things.\n\nOtherwise, this LGTM, thanks!\n"},{"id":"466082","messageId":"CAP8UFD3VRooL7mxYJGs4BAfYTgZjj+GJPN86o-x57LbOVVmCkg@mail.gmail.com","threadId":"58447","inReplyTo":"20221029102459.82428-2-siddharthasthana31@gmail.com","subject":"Re: [PATCH v3 1/2] cat-file: add mailmap support to -s option","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-10-31T11:49:20Z","receivedAt":"2022-10-31T11:49:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Oct 29, 2022 at 12:25 PM Siddharth Asthana\n wrote:\n\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index ec30b5c574..594b6f2dfd 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -45,7 +45,9 @@ OPTIONS\n>\n> -s::\n> Instead of the content, show the object size identified by\n> - ``.\n> + ``. If used with `--use-mailmap` option, will show the\n> + size of updated object after replacing idents using the mailmap\n> + mechanism.\n\nI think this should also go in the other documentation patch that was\nsplit off from this series, but it is less clear cut here as there are\nprobably no conflicts with the other patch.\n\nOtherwise this patch also LGTM, thanks!\n"},{"id":"467225","messageId":"20221113212830.92609-1-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH v4 0/3] Add mailmap mechanism in cat-file options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-13T21:28:27Z","receivedAt":"2022-11-13T21:28:47Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Thanks a lot Junio, Taylor and Christian for the review :) I have made\nthe suggested changes.\n\n= Description\n\nAt present, `git-cat-file` command with `--batch-check` and `-s` options\ndoes not complain when `--use-mailmap` option is given. The latter\noption is just ignored. Instead, for commit/tag objects, the command\nshould compute the size of the object after replacing the idents and\nreport it. So, this patch series makes `-s` and `--batch-check` options\nof `git-cat-file` honor mailmap when used with `--use-mailmap` option.\n\nIn this patch series we didn't want to change that '%(objectsize)'\nalways shows the size of the original object even when `--use-mailmap`\nis set because first we have the long term plan to unify how the formats\nfor `git cat-file` and other commands works. And second existing formats\nlike the \"pretty formats\" used bt `git log` have different options for\nfields respecting mailmap or not respecting it (%an is for author name\nwhile %aN for author name respecting mailmap).\n\nI would like to thank my mentors, Christian Couder and John Cai, for all\nof their help!\nLooking forward to the reviews!\n\n= Patch Organization\n\n- The first patch makes `-s` option to return updated size of the\n object, when combined with `--use-mailmap` option, after\n replacing the idents using the mailmap mechanism.\n- The second patch makes `--batch-check` option to return updated size of\n the object, when combined with `--use-mailmap` option,\n after replacing the idents using the mailmap mechanism.\n- The third patch improves the documentation of `-s`, `--batch`,\n `--batch-check` and `--batch-command` options by adding they can be\n combined with `--use-mailmap` options.\n\n= Changes in v4\n\n- Improve the documentation patch to clearly state that the `-s`,\n `--batch-check`, `--batch-command` and `--batch` options can be only\n be used with `--textconv`, `--filters` or `--use-mailmap`.\n\nSiddharth Asthana (3):\n cat-file: add mailmap support to -s option\n cat-file: add mailmap support to --batch-check option\n doc/cat-file: allow --use-mailmap for --batch options\n\n Documentation/git-cat-file.txt | 53 ++++++++++++++++++++++--------\n builtin/cat-file.c | 27 ++++++++++++++++\n t/t4203-mailmap.sh | 59 ++++++++++++++++++++++++++++++++++\n 3 files changed, 125 insertions(+), 14 deletions(-)\n\nRange-diff against v3:\n1: 38bb89d350 ! 1: 4ae3af37d2 cat-file: add mailmap support to -s option\n @@ Commit message\n \n Mentored-by: Christian Couder \n Mentored-by: John Cai \n + Helped-by: Taylor Blau \n + Helped-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Siddharth Asthana \n \n - ## Documentation/git-cat-file.txt ##\n -@@ Documentation/git-cat-file.txt: OPTIONS\n - \n - -s::\n - \tInstead of the content, show the object size identified by\n --\t``.\n -+\t``. If used with `--use-mailmap` option, will show the\n -+\tsize of updated object after replacing idents using the mailmap\n -+\tmechanism.\n - \n - -e::\n - \tExit with zero status if `` exists and is a valid\n -\n ## builtin/cat-file.c ##\n @@ builtin/cat-file.c: static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n2: 4d49cfde73 ! 2: a692646228 cat-file: add mailmap support to --batch-check option\n @@ Commit message\n \n Mentored-by: Christian Couder \n Mentored-by: John Cai \n + Helped-by: Taylor Blau \n + Helped-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Siddharth Asthana \n \n - ## Documentation/git-cat-file.txt ##\n -@@ Documentation/git-cat-file.txt: OPTIONS\n - \texcept `--textconv` or `--filters`, in which case the input lines\n - \talso need to specify the path, separated by whitespace. See the\n - \tsection `BATCH OUTPUT` below for details.\n -+\tIf used with `--use-mailmap` option, will show the size of\n -+\tupdated object after replacing idents using the mailmap mechanism.\n - \n - --batch-check::\n - --batch-check=::\n -\n ## builtin/cat-file.c ##\n @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n-: ---------- > 3: 41b4650b24 doc/cat-file: allow --use-mailmap for --batch options\n-- \n2.38.1.420.g319605f8f0\n\n"},{"id":"467226","messageId":"20221113212830.92609-2-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221113212830.92609-1-siddharthasthana31@gmail.com","subject":"[PATCH v4 1/3] cat-file: add mailmap support to -s option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-13T21:28:28Z","receivedAt":"2022-11-13T21:28:56Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `-s` option does not complain when\n`--use-mailmap` option is given, the latter option is ignored. Compute\nthe size of the object after replacing the idents and report it instead.\n\nIn order to make `-s` option honour the mailmap mechanism we have to\nread the contents of the commit/tag object. Make use of the call to\n`oid_object_info_extended()` to get the contents of the object and store\nin `buf`. `buf` is later freed in the function.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 29 +++++++++++++++++++++++++++++\n 2 files changed, 42 insertions(+)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex fa7bd89169..8a6e2343ec 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \tcase 's':\n \t\toi.sizep = &size;\n+\n+\t\tif (use_mailmap) {\n+\t\t\toi.typep = &type;\n+\t\t\toi.contentp = (void**)&buf;\n+\t\t}\n+\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n+\n+\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n+\t\t\tsize_t s = size;\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tsize = cast_size_t_to_ulong(s);\n+\t\t}\n+\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex cd1cab3e54..b500b31c92 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1022,4 +1022,33 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcat >expect <<-\\EOF &&\n+\t209\n+\t220\n+\tEOF\n+\tgit cat-file -s HEAD >actual &&\n+\tgit cat-file --use-mailmap -s HEAD >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap for tag objects' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tOrig C O Mitter \n+\tEOF\n+\tgit tag -a -m \"annotated tag\" v3 &&\n+\tcat >expect <<-\\EOF &&\n+\t141\n+\t130\n+\tEOF\n+\tgit cat-file -s v3 >actual &&\n+\tgit cat-file --use-mailmap -s v3 >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.1.420.g319605f8f0\n\n"},{"id":"467227","messageId":"20221113212830.92609-3-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221113212830.92609-1-siddharthasthana31@gmail.com","subject":"[PATCH v4 2/3] cat-file: add mailmap support to --batch-check option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-13T21:28:29Z","receivedAt":"2022-11-13T21:29:09Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `--batch-check` option does not\ncomplain when `--use-mailmap` option is given, the latter option is\nignored. Compute the size of the object after replacing the idents and\nreport it instead.\n\nIn order to make `--batch-check` option honour the mailmap mechanism we\nhave to read the contents of the commit/tag object.\n\nThere were two ways to do it:\n\n1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n option is given, the first call will get us the type of the object\n and second call will only be made if the object type is either a\n commit or tag to get the contents of the object.\n\n2. Make one call to `oid_object_info_extended()` to get the type of the\n object. Then, if the object type is either of commit or tag, make a\n call to `read_object_file()` to read the contents of the object.\n\nI benchmarked the following command with both the above approaches and\ncompared against the current implementation where `--use-mailmap`\noption is ignored:\n\n`git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n--unordered`\n\nThe results can be summarized as follows:\n Time (mean ± σ)\ndefault 827.7 ms ± 104.8 ms\nfirst approach 6.197 s ± 0.093 s\nsecond approach 1.975 s ± 0.217 s\n\nSince, the second approach is faster than the first one, I implemented\nit in this patch.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n builtin/cat-file.c | 14 ++++++++++++++\n t/t4203-mailmap.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 8a6e2343ec..39f2a2483f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -444,6 +444,9 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap)\n+\t\t\tdata->info.typep = &data->type;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -457,6 +460,17 @@ static void batch_object_write(const char *obj_name,\n \t\t\tfflush(stdout);\n \t\t\treturn;\n \t\t}\n+\n+\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n+\t\t\tsize_t s = data->size;\n+\t\t\tchar *buf = NULL;\n+\n+\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tdata->size = cast_size_t_to_ulong(s);\n+\n+\t\t\tfree(buf);\n+\t\t}\n \t}\n \n \tstrbuf_reset(scratch);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex b500b31c92..b4355c3e51 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1051,4 +1051,34 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcat >expect <<-\\EOF &&\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n+\tEOF\n+\techo \"HEAD\" >in &&\n+\tgit cat-file --batch-check actual &&\n+\tgit cat-file --use-mailmap --batch-check >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcat >expect <<-\\EOF &&\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n+\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n+\tEOF\n+\techo \"info HEAD\" >in &&\n+\tgit cat-file --batch-command actual &&\n+\tgit cat-file --use-mailmap --batch-command >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.1.420.g319605f8f0\n\n"},{"id":"467228","messageId":"20221113212830.92609-4-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221113212830.92609-1-siddharthasthana31@gmail.com","subject":"[PATCH v4 3/3] doc/cat-file: allow --use-mailmap for --batch options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-13T21:28:30Z","receivedAt":"2022-11-13T21:29:19Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"The command git cat-file can now use the mailmap mechanism to replace\nidents with their canonical versions for commit and tag objects. There\nare several options like `-s`, `--batch`, `--batch-check` and\n`--batch-command` that can be combined with `--use-mailmap`. But the\ndocumentation for `-s`, `--batch`, `--batch-check` and `--batch-command`\ndoesn't say so. This patch fixes that documentation.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 53 +++++++++++++++++++++++++---------\n 1 file changed, 39 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex ec30b5c574..81235c60a3 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -45,7 +45,9 @@ OPTIONS\n \n -s::\n \tInstead of the content, show the object size identified by\n-\t``.\n+\t``. If used with `--use-mailmap` option, will show\n+\tthe size of updated object after replacing idents using the\n+\tmailmap mechanism.\n \n -e::\n \tExit with zero status if `` exists and is a valid\n@@ -89,26 +91,49 @@ OPTIONS\n --batch::\n --batch=::\n \tPrint object information and contents for each object provided\n-\ton stdin. May not be combined with any other options or arguments\n-\texcept `--textconv` or `--filters`, in which case the input lines\n-\talso need to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\ton stdin. May not be combined with any other options or arguments\n+\texcept --textconv, --filters, or --use-mailmap.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines\n+\t must specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t contents part of the output shows the identities replaced using the\n+\t mailmap mechanism, while the information part of the output shows\n+\t the size of the object as if it actually recorded the replacement\n+\t identities.\n \n --batch-check::\n --batch-check=::\n-\tPrint object information for each object provided on stdin. May\n-\tnot be combined with any other options or arguments except\n-\t`--textconv` or `--filters`, in which case the input lines also\n-\tneed to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\tPrint object information for each object provided on stdin. May not be\n+\tcombined with any other options or arguments except --textconv, --filters\n+\tor --use-mailmap.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t printed object information shows the size of the object as if the\n+\t identities recorded in it were replaced by the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\n \tEnter a command mode that reads commands and arguments from stdin. May\n-\tonly be combined with `--buffer`, `--textconv` or `--filters`. In the\n-\tcase of `--textconv` or `--filters`, the input lines also need to specify\n-\tthe path, separated by whitespace. See the section `BATCH OUTPUT` below\n-\tfor details.\n+\tonly be combined with `--buffer`, `--textconv`, `--use-mailmap` or\n+\t`--filters`.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t `contents` command shows the identities replaced using the\n+\t mailmap mechanism, while the `info` command shows the size\n+\t of the object as if it actually recorded the replacement\n+\t identities.\n+\n +\n `--batch-command` recognizes the following commands:\n +\n-- \n2.38.1.420.g319605f8f0\n\n"},{"id":"467257","messageId":"CAP8UFD1nhXnYUj9zsXwvf2tjeK1yimY3AwomU30wor1Vf-QPbA@mail.gmail.com","threadId":"58447","inReplyTo":"20221113212830.92609-1-siddharthasthana31@gmail.com","subject":"Re: [PATCH v4 0/3] Add mailmap mechanism in cat-file options","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-11-14T17:48:54Z","receivedAt":"2022-11-14T17:50:01Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sun, Nov 13, 2022 at 10:28 PM Siddharth Asthana\n wrote:\n\n> In this patch series we didn't want to change that '%(objectsize)'\n> always shows the size of the original object even when `--use-mailmap`\n> is set because first we have the long term plan to unify how the formats\n> for `git cat-file` and other commands works. And second existing formats\n> like the \"pretty formats\" used bt `git log` have different options for\n\ns/used bt/used by/\n\n> fields respecting mailmap or not respecting it (%an is for author name\n> while %aN for author name respecting mailmap).\n\n[...]\n\n> = Patch Organization\n>\n> - The first patch makes `-s` option to return updated size of the\n> object, when combined with `--use-mailmap` option, after\n> replacing the idents using the mailmap mechanism.\n> - The second patch makes `--batch-check` option to return updated size of\n> the object, when combined with `--use-mailmap` option,\n> after replacing the idents using the mailmap mechanism.\n> - The third patch improves the documentation of `-s`, `--batch`,\n> `--batch-check` and `--batch-command` options by adding they can be\n> combined with `--use-mailmap` options.\n\nSo the documentation patch is now part of this small series again.\n\nEven if this documentation patch is a bug fix, it might be better at\nthis point to squash this patch into the patches 1/3 and 2/3. At least\nI think that would better follow Junio's last comments about this. If\nyou go this way, you might want to squash the documentation parts\nabout -s from patch 3/3 into patch 1/3 and the rest of patch 3/3 into\npatch 2/3.\n\n> = Changes in v4\n>\n> - Improve the documentation patch to clearly state that the `-s`,\n> `--batch-check`, `--batch-command` and `--batch` options can be only\n> be used with `--textconv`, `--filters` or `--use-mailmap`.\n\nHere I think you should say that the documentation patch is part of\nthis series again and explain a bit why.\n\nAnyway I took a look at the actual patches in this series and they\nlook good to me now. So I would be Ok to merge it either as is or with\npatch 3/3 squashed into patches 1/3 and 2/3 as discussed above.\n\nThanks!\n"},{"id":"467273","messageId":"Y3LBkZI6x9Iz8lgo@nand.local","threadId":"58447","inReplyTo":"CAP8UFD1nhXnYUj9zsXwvf2tjeK1yimY3AwomU30wor1Vf-QPbA@mail.gmail.com","subject":"Re: [PATCH v4 0/3] Add mailmap mechanism in cat-file options","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-14T22:30:41Z","receivedAt":"2022-11-14T22:30:53Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Nov 14, 2022 at 06:48:54PM +0100, Christian Couder wrote:\n> > = Patch Organization\n> >\n> > - The first patch makes `-s` option to return updated size of the\n> > object, when combined with `--use-mailmap` option, after\n> > replacing the idents using the mailmap mechanism.\n> > - The second patch makes `--batch-check` option to return updated size of\n> > the object, when combined with `--use-mailmap` option,\n> > after replacing the idents using the mailmap mechanism.\n> > - The third patch improves the documentation of `-s`, `--batch`,\n> > `--batch-check` and `--batch-command` options by adding they can be\n> > combined with `--use-mailmap` options.\n>\n> So the documentation patch is now part of this small series again.\n\nFor what it's worth, I'm fine to include the documentation updates in\nthis series. It makes it vaguely easier to queue, but I think if we're\nclose to merging this one down, then there's no reason to separate the\ntwo.\n\n> Even if this documentation patch is a bug fix, it might be better at\n> this point to squash this patch into the patches 1/3 and 2/3. At least\n> I think that would better follow Junio's last comments about this. If\n> you go this way, you might want to squash the documentation parts\n> about -s from patch 3/3 into patch 1/3 and the rest of patch 3/3 into\n> patch 2/3.\n\nThanks. I agree with you that following Junio's advice in this instance\nis good to do. Let's wait for one hopefully-final reroll and then start\nmerging this down.\n\nThanks,\nTaylor\n"},{"id":"467344","messageId":"Y3QHPYiZl7PbyFhP@nand.local","threadId":"58447","inReplyTo":"20221113212830.92609-3-siddharthasthana31@gmail.com","subject":"Re: [PATCH v4 2/3] cat-file: add mailmap support to --batch-check option","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-15T21:40:13Z","receivedAt":"2022-11-15T21:40:21Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Nov 14, 2022 at 02:58:29AM +0530, Siddharth Asthana wrote:\n> @@ -1051,4 +1051,34 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n> \ttest_cmp expect actual\n> '\n>\n> +test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-\\EOF &&\n> +\tC O Mitter Orig \n> +\tEOF\n> +\tcat >expect <<-\\EOF &&\n> +\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n> +\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n> +\tEOF\n> +\techo \"HEAD\" >in &&\n> +\tgit cat-file --batch-check actual &&\n> +\tgit cat-file --use-mailmap --batch-check >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-\\EOF &&\n> +\tC O Mitter Orig \n> +\tEOF\n> +\tcat >expect <<-\\EOF &&\n> +\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 209\n> +\t92d99959b011b1cd69fe7be5315d6732fe302575 commit 220\n> +\tEOF\n> +\techo \"info HEAD\" >in &&\n> +\tgit cat-file --batch-command actual &&\n> +\tgit cat-file --use-mailmap --batch-command >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n\nThese two tests (and the -s ones above it) are not going to work when\nrun under SHA-256 mode, i.e., with 'GIT_TEST_DEFAULT_HASH=sha256' in the\nenvironment:\n\n not ok 66 - git cat-file -s returns correct size with --use-mailmap\n not ok 67 - git cat-file -s returns correct size with --use-mailmap for tag objects\n not ok 68 - git cat-file --batch-check returns correct size with --use-mailmap\n not ok 69 - git cat-file --batch-command returns correct size with --use-mailmap\n\nThanks,\nTaylor\n"},{"id":"467605","messageId":"42aa5823-36d7-8860-90ab-953eb6d2f2b4@gmail.com","threadId":"58447","inReplyTo":"CAP8UFD1nhXnYUj9zsXwvf2tjeK1yimY3AwomU30wor1Vf-QPbA@mail.gmail.com","subject":"Re: [PATCH v4 0/3] Add mailmap mechanism in cat-file options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-20T07:42:06Z","receivedAt":"2022-11-20T07:42:17Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"\n\nOn 14/11/22 23:18, Christian Couder wrote:\n> On Sun, Nov 13, 2022 at 10:28 PM Siddharth Asthana\n> wrote:\n> \n>> In this patch series we didn't want to change that '%(objectsize)'\n>> always shows the size of the original object even when `--use-mailmap`\n>> is set because first we have the long term plan to unify how the formats\n>> for `git cat-file` and other commands works. And second existing formats\n>> like the \"pretty formats\" used bt `git log` have different options for\n> \n> s/used bt/used by/\n> \n>> fields respecting mailmap or not respecting it (%an is for author name\n>> while %aN for author name respecting mailmap).\n> \n> [...]\n> \n>> = Patch Organization\n>>\n>> - The first patch makes `-s` option to return updated size of the\n>> object, when combined with `--use-mailmap` option, after\n>> replacing the idents using the mailmap mechanism.\n>> - The second patch makes `--batch-check` option to return updated size of\n>> the object, when combined with `--use-mailmap` option,\n>> after replacing the idents using the mailmap mechanism.\n>> - The third patch improves the documentation of `-s`, `--batch`,\n>> `--batch-check` and `--batch-command` options by adding they can be\n>> combined with `--use-mailmap` options.\n> \n> So the documentation patch is now part of this small series again.\n> \n> Even if this documentation patch is a bug fix, it might be better at\n> this point to squash this patch into the patches 1/3 and 2/3. At least\n> I think that would better follow Junio's last comments about this. If\n> you go this way, you might want to squash the documentation parts\n> about -s from patch 3/3 into patch 1/3 and the rest of patch 3/3 into\n> patch 2/3.\n> \n>> = Changes in v4\n>>\n>> - Improve the documentation patch to clearly state that the `-s`,\n>> `--batch-check`, `--batch-command` and `--batch` options can be only\n>> be used with `--textconv`, `--filters` or `--use-mailmap`.\n> \n> Here I think you should say that the documentation patch is part of\n> this series again and explain a bit why.\n> \n> Anyway I took a look at the actual patches in this series and they\n> look good to me now. So I would be Ok to merge it either as is or with\n> patch 3/3 squashed into patches 1/3 and 2/3 as discussed above.\n> \n> Thanks!\n\nThanks a lot for the review Christian :) I have made the suggested \nchanges in v5.\n"},{"id":"467606","messageId":"20221120074852.121346-1-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH v5 0/2] Add mailmap mechanism in cat-file options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-20T07:48:50Z","receivedAt":"2022-11-20T07:49:09Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Thanks a lot Christian, Taylor and Junio for review :) I have made the\nsuggested in this patch series.\n\n= Description\n\nAt present, `git-cat-file` command with `--batch-check` and `-s` options\ndoes not complain when `--use-mailmap` option is given. The latter\noption is just ignored. Instead, for commit/tag objects, the command\nshould compute the size of the object after replacing the idents and\nreport it. So, this patch series makes `-s` and `--batch-check` options\nof `git-cat-file` honor mailmap when used with `--use-mailmap` option.\n\nIn this patch series we didn't want to change that '%(objectsize)'\nalways shows the size of the original object even when `--use-mailmap`\nis set because first we have the long term plan to unify how the formats\nfor `git cat-file` and other commands works. And second existing formats\nlike the \"pretty formats\" used by `git log` have different options for\nfields respecting mailmap or not respecting it (%an is for author name\nwhile %aN for author name respecting mailmap).\n\nI would like to thank my mentors, Christian Couder and John Cai, for all\nof their help!\nLooking forward to the reviews!\n\n= Patch Organization\n\n- The first patch makes `-s` option to return updated size of the\n object, when combined with `--use-mailmap` option, after\n replacing the idents using the mailmap mechanism.\n- The second patch makes `--batch-check` option to return updated size of\n the object, when combined with `--use-mailmap` option,\n after replacing the idents using the mailmap mechanism.\n\n= Changes in v5\n\n- The patch series which improves the documentation for `-s`, `--batch`,\n `--batch-check` and `--batch-command` is again part of this patch\n series with patch 3/3 squashed into patches 1/3 and 2/3 as suggested\n by Junio, Taylor and Christian. The doc patch series was perviously\n sent independently for improving the documentation of git cat-file\n options:\n https://lore.kernel.org/git/20221029092513.73982-1-siddharthasthana31@gmail.com/\n\n- Improve the tests according to run under SHA-256 mode.\n\nSiddharth Asthana (2):\n cat-file: add mailmap support to -s option\n cat-file: add mailmap support to --batch-check option\n\n Documentation/git-cat-file.txt | 53 ++++++++++++++++++++++---------\n builtin/cat-file.c | 27 ++++++++++++++++\n t/t4203-mailmap.sh | 57 ++++++++++++++++++++++++++++++++++\n 3 files changed, 123 insertions(+), 14 deletions(-)\n\nRange-diff against v4:\n1: 4ae3af37d2 ! 1: 0db3583535 cat-file: add mailmap support to -s option\n @@ Commit message\n Helped-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Siddharth Asthana \n \n + ## Documentation/git-cat-file.txt ##\n +@@ Documentation/git-cat-file.txt: OPTIONS\n + \n + -s::\n + \tInstead of the content, show the object size identified by\n +-\t``.\n ++\t``. If used with `--use-mailmap` option, will show\n ++\tthe size of updated object after replacing idents using the\n ++\tmailmap mechanism.\n + \n + -e::\n + \tExit with zero status if `` exists and is a valid\n +\n ## builtin/cat-file.c ##\n @@ builtin/cat-file.c: static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n @@ t/t4203-mailmap.sh: test_expect_success '--mailmap enables mailmap in cat-file f\n +\tcat >.mailmap <<-\\EOF &&\n +\tC O Mitter Orig \n +\tEOF\n -+\tcat >expect <<-\\EOF &&\n -+\t209\n -+\t220\n -+\tEOF\n ++\tgit cat-file commit HEAD | wc -c >expect &&\n ++\tgit cat-file --use-mailmap commit HEAD | wc -c >>expect &&\n +\tgit cat-file -s HEAD >actual &&\n +\tgit cat-file --use-mailmap -s HEAD >>actual &&\n +\ttest_cmp expect actual\n @@ t/t4203-mailmap.sh: test_expect_success '--mailmap enables mailmap in cat-file f\n +\tOrig C O Mitter \n +\tEOF\n +\tgit tag -a -m \"annotated tag\" v3 &&\n -+\tcat >expect <<-\\EOF &&\n -+\t141\n -+\t130\n -+\tEOF\n ++\tgit cat-file tag v3 | wc -c >expect &&\n ++\tgit cat-file --use-mailmap tag v3 | wc -c >>expect &&\n +\tgit cat-file -s v3 >actual &&\n +\tgit cat-file --use-mailmap -s v3 >>actual &&\n +\ttest_cmp expect actual\n2: a692646228 < -: ---------- cat-file: add mailmap support to --batch-check option\n3: 41b4650b24 ! 2: b8205ede7d doc/cat-file: allow --use-mailmap for --batch options\n @@ Metadata\n Author: Siddharth Asthana \n \n ## Commit message ##\n - doc/cat-file: allow --use-mailmap for --batch options\n + cat-file: add mailmap support to --batch-check option\n +\n + Even though the cat-file command with `--batch-check` option does not\n + complain when `--use-mailmap` option is given, the latter option is\n + ignored. Compute the size of the object after replacing the idents and\n + report it instead.\n +\n + In order to make `--batch-check` option honour the mailmap mechanism we\n + have to read the contents of the commit/tag object.\n +\n + There were two ways to do it:\n +\n + 1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n + option is given, the first call will get us the type of the object\n + and second call will only be made if the object type is either a\n + commit or tag to get the contents of the object.\n +\n + 2. Make one call to `oid_object_info_extended()` to get the type of the\n + object. Then, if the object type is either of commit or tag, make a\n + call to `read_object_file()` to read the contents of the object.\n +\n + I benchmarked the following command with both the above approaches and\n + compared against the current implementation where `--use-mailmap`\n + option is ignored:\n +\n + `git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n + --unordered`\n +\n + The results can be summarized as follows:\n + Time (mean ± σ)\n + default 827.7 ms ± 104.8 ms\n + first approach 6.197 s ± 0.093 s\n + second approach 1.975 s ± 0.217 s\n +\n + Since, the second approach is faster than the first one, I implemented\n + it in this patch.\n \n The command git cat-file can now use the mailmap mechanism to replace\n - idents with their canonical versions for commit and tag objects. There\n - are several options like `-s`, `--batch`, `--batch-check` and\n - `--batch-command` that can be combined with `--use-mailmap`. But the\n - documentation for `-s`, `--batch`, `--batch-check` and `--batch-command`\n - doesn't say so. This patch fixes that documentation.\n + idents with canonical versions for commit and tag objects. There are\n + several options like `--batch`, `--batch-check` and `--batch-command`\n + that can be combined with `--use-mailmap`. But the documentation for\n + `--batch`, `--batch-check` and `--batch-command` doesn't say so. This\n + patch fixes that documentation.\n \n Mentored-by: Christian Couder \n Mentored-by: John Cai \n Helped-by: Taylor Blau \n + Helped-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Siddharth Asthana \n \n ## Documentation/git-cat-file.txt ##\n @@ Documentation/git-cat-file.txt: OPTIONS\n - \n - -s::\n - \tInstead of the content, show the object size identified by\n --\t``.\n -+\t``. If used with `--use-mailmap` option, will show\n -+\tthe size of updated object after replacing idents using the\n -+\tmailmap mechanism.\n - \n - -e::\n - \tExit with zero status if `` exists and is a valid\n -@@ Documentation/git-cat-file.txt: OPTIONS\n --batch::\n --batch=::\n \tPrint object information and contents for each object provided\n @@ Documentation/git-cat-file.txt: OPTIONS\n +\n `--batch-command` recognizes the following commands:\n +\n +\n + ## builtin/cat-file.c ##\n +@@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n + \tif (!data->skip_object_info) {\n + \t\tint ret;\n + \n ++\t\tif (use_mailmap)\n ++\t\t\tdata->info.typep = &data->type;\n ++\n + \t\tif (pack)\n + \t\t\tret = packed_object_info(the_repository, pack, offset,\n + \t\t\t\t\t\t &data->info);\n +@@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n + \t\t\tfflush(stdout);\n + \t\t\treturn;\n + \t\t}\n ++\n ++\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n ++\t\t\tsize_t s = data->size;\n ++\t\t\tchar *buf = NULL;\n ++\n ++\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n ++\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n ++\t\t\tdata->size = cast_size_t_to_ulong(s);\n ++\n ++\t\t\tfree(buf);\n ++\t\t}\n + \t}\n + \n + \tstrbuf_reset(scratch);\n +\n + ## t/t4203-mailmap.sh ##\n +@@ t/t4203-mailmap.sh: test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n + \ttest_cmp expect actual\n + '\n + \n ++test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n ++\ttest_when_finished \"rm .mailmap\" &&\n ++\tcat >.mailmap <<-\\EOF &&\n ++\tC O Mitter Orig \n ++\tEOF\n ++\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n ++\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n ++\techo \"$commit_sha commit $commit_size\" >expect &&\n ++\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n ++\techo \"$commit_sha commit $commit_size\" >>expect &&\n ++\techo \"HEAD\" >in &&\n ++\tgit cat-file --batch-check actual &&\n ++\tgit cat-file --use-mailmap --batch-check >actual &&\n ++\ttest_cmp expect actual\n ++'\n ++\n ++test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n ++\ttest_when_finished \"rm .mailmap\" &&\n ++\tcat >.mailmap <<-\\EOF &&\n ++\tC O Mitter Orig \n ++\tEOF\n ++\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n ++\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n ++\techo \"$commit_sha commit $commit_size\" >expect &&\n ++\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n ++\techo \"$commit_sha commit $commit_size\" >>expect &&\n ++\techo \"info HEAD\" >in &&\n ++\tgit cat-file --batch-command actual &&\n ++\tgit cat-file --use-mailmap --batch-command >actual &&\n ++\ttest_cmp expect actual\n ++'\n ++\n + test_done\n-- \n2.38.1.438.gd2340c8913\n\n"},{"id":"467607","messageId":"20221120074852.121346-2-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221120074852.121346-1-siddharthasthana31@gmail.com","subject":"[PATCH v5 1/2] cat-file: add mailmap support to -s option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-20T07:48:51Z","receivedAt":"2022-11-20T07:49:16Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `-s` option does not complain when\n`--use-mailmap` option is given, the latter option is ignored. Compute\nthe size of the object after replacing the idents and report it instead.\n\nIn order to make `-s` option honour the mailmap mechanism we have to\nread the contents of the commit/tag object. Make use of the call to\n`oid_object_info_extended()` to get the contents of the object and store\nin `buf`. `buf` is later freed in the function.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 4 +++-\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 25 +++++++++++++++++++++++++\n 3 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex ec30b5c574..f82d702d6b 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -45,7 +45,9 @@ OPTIONS\n \n -s::\n \tInstead of the content, show the object size identified by\n-\t``.\n+\t``. If used with `--use-mailmap` option, will show\n+\tthe size of updated object after replacing idents using the\n+\tmailmap mechanism.\n \n -e::\n \tExit with zero status if `` exists and is a valid\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex fa7bd89169..8a6e2343ec 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \tcase 's':\n \t\toi.sizep = &size;\n+\n+\t\tif (use_mailmap) {\n+\t\t\toi.typep = &type;\n+\t\t\toi.contentp = (void**)&buf;\n+\t\t}\n+\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n+\n+\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n+\t\t\tsize_t s = size;\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tsize = cast_size_t_to_ulong(s);\n+\t\t}\n+\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex cd1cab3e54..87b77fc5c9 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1022,4 +1022,29 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tgit cat-file commit HEAD | wc -c >expect &&\n+\tgit cat-file --use-mailmap commit HEAD | wc -c >>expect &&\n+\tgit cat-file -s HEAD >actual &&\n+\tgit cat-file --use-mailmap -s HEAD >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap for tag objects' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tOrig C O Mitter \n+\tEOF\n+\tgit tag -a -m \"annotated tag\" v3 &&\n+\tgit cat-file tag v3 | wc -c >expect &&\n+\tgit cat-file --use-mailmap tag v3 | wc -c >>expect &&\n+\tgit cat-file -s v3 >actual &&\n+\tgit cat-file --use-mailmap -s v3 >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.1.438.gd2340c8913\n\n"},{"id":"467608","messageId":"20221120074852.121346-3-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221120074852.121346-1-siddharthasthana31@gmail.com","subject":"[PATCH v5 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-11-20T07:48:52Z","receivedAt":"2022-11-20T07:49:22Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `--batch-check` option does not\ncomplain when `--use-mailmap` option is given, the latter option is\nignored. Compute the size of the object after replacing the idents and\nreport it instead.\n\nIn order to make `--batch-check` option honour the mailmap mechanism we\nhave to read the contents of the commit/tag object.\n\nThere were two ways to do it:\n\n1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n option is given, the first call will get us the type of the object\n and second call will only be made if the object type is either a\n commit or tag to get the contents of the object.\n\n2. Make one call to `oid_object_info_extended()` to get the type of the\n object. Then, if the object type is either of commit or tag, make a\n call to `read_object_file()` to read the contents of the object.\n\nI benchmarked the following command with both the above approaches and\ncompared against the current implementation where `--use-mailmap`\noption is ignored:\n\n`git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n--unordered`\n\nThe results can be summarized as follows:\n Time (mean ± σ)\ndefault 827.7 ms ± 104.8 ms\nfirst approach 6.197 s ± 0.093 s\nsecond approach 1.975 s ± 0.217 s\n\nSince, the second approach is faster than the first one, I implemented\nit in this patch.\n\nThe command git cat-file can now use the mailmap mechanism to replace\nidents with canonical versions for commit and tag objects. There are\nseveral options like `--batch`, `--batch-check` and `--batch-command`\nthat can be combined with `--use-mailmap`. But the documentation for\n`--batch`, `--batch-check` and `--batch-command` doesn't say so. This\npatch also fixes that documentation.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 49 +++++++++++++++++++++++++---------\n builtin/cat-file.c | 14 ++++++++++\n t/t4203-mailmap.sh | 32 ++++++++++++++++++++++\n 3 files changed, 82 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex f82d702d6b..81235c60a3 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -91,26 +91,49 @@ OPTIONS\n --batch::\n --batch=::\n \tPrint object information and contents for each object provided\n-\ton stdin. May not be combined with any other options or arguments\n-\texcept `--textconv` or `--filters`, in which case the input lines\n-\talso need to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\ton stdin. May not be combined with any other options or arguments\n+\texcept --textconv, --filters, or --use-mailmap.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines\n+\t must specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t contents part of the output shows the identities replaced using the\n+\t mailmap mechanism, while the information part of the output shows\n+\t the size of the object as if it actually recorded the replacement\n+\t identities.\n \n --batch-check::\n --batch-check=::\n-\tPrint object information for each object provided on stdin. May\n-\tnot be combined with any other options or arguments except\n-\t`--textconv` or `--filters`, in which case the input lines also\n-\tneed to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\tPrint object information for each object provided on stdin. May not be\n+\tcombined with any other options or arguments except --textconv, --filters\n+\tor --use-mailmap.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t printed object information shows the size of the object as if the\n+\t identities recorded in it were replaced by the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\n \tEnter a command mode that reads commands and arguments from stdin. May\n-\tonly be combined with `--buffer`, `--textconv` or `--filters`. In the\n-\tcase of `--textconv` or `--filters`, the input lines also need to specify\n-\tthe path, separated by whitespace. See the section `BATCH OUTPUT` below\n-\tfor details.\n+\tonly be combined with `--buffer`, `--textconv`, `--use-mailmap` or\n+\t`--filters`.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t `contents` command shows the identities replaced using the\n+\t mailmap mechanism, while the `info` command shows the size\n+\t of the object as if it actually recorded the replacement\n+\t identities.\n+\n +\n `--batch-command` recognizes the following commands:\n +\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 8a6e2343ec..39f2a2483f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -444,6 +444,9 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap)\n+\t\t\tdata->info.typep = &data->type;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -457,6 +460,17 @@ static void batch_object_write(const char *obj_name,\n \t\t\tfflush(stdout);\n \t\t\treturn;\n \t\t}\n+\n+\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n+\t\t\tsize_t s = data->size;\n+\t\t\tchar *buf = NULL;\n+\n+\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tdata->size = cast_size_t_to_ulong(s);\n+\n+\t\t\tfree(buf);\n+\t\t}\n \t}\n \n \tstrbuf_reset(scratch);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex 87b77fc5c9..21ba6bc278 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1047,4 +1047,36 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n+\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n+\techo \"$commit_sha commit $commit_size\" >expect &&\n+\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n+\techo \"$commit_sha commit $commit_size\" >>expect &&\n+\techo \"HEAD\" >in &&\n+\tgit cat-file --batch-check actual &&\n+\tgit cat-file --use-mailmap --batch-check >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n+\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n+\techo \"$commit_sha commit $commit_size\" >expect &&\n+\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n+\techo \"$commit_sha commit $commit_size\" >>expect &&\n+\techo \"info HEAD\" >in &&\n+\tgit cat-file --batch-command actual &&\n+\tgit cat-file --use-mailmap --batch-command >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.38.1.438.gd2340c8913\n\n"},{"id":"467661","messageId":"xmqqy1s4wyvf.fsf@gitster.g","threadId":"58447","inReplyTo":"20221120074852.121346-2-siddharthasthana31@gmail.com","subject":"Re: [PATCH v5 1/2] cat-file: add mailmap support to -s option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-21T07:27:48Z","receivedAt":"2022-11-21T07:28:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Siddharth Asthana writes:\n\n> +test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-\\EOF &&\n> +\tC O Mitter Orig \n> +\tEOF\n> +\tgit cat-file commit HEAD | wc -c >expect &&\n> +\tgit cat-file --use-mailmap commit HEAD | wc -c >>expect &&\n> +\tgit cat-file -s HEAD >actual &&\n> +\tgit cat-file --use-mailmap -s HEAD >>actual &&\n\nDoesn't this break under macOS where wc output tends to be padded\nwith SP on the right? We used to often see test breakage when a\ncarelessly written test like\n\n\ttest \"$(wc -l writes:\n\n> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\n> index 87b77fc5c9..21ba6bc278 100755\n> --- a/t/t4203-mailmap.sh\n> +++ b/t/t4203-mailmap.sh\n> @@ -1047,4 +1047,36 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-\\EOF &&\n> +\tC O Mitter Orig \n> +\tEOF\n> +\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n\nWe prefer $(command substitution) over `command substitution`. \n\nWhen \"cat-file\" segfaults and dumps core, having it upstream of a\npipe would mean its crashing will be hidden.\n\nNote that some implementations of \"wc\" pads its output with SP. The\nimplication will be seen in a few paragraphs below.\n\n> +\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n\nThese single quotes are not doing what you think they are doing.\nThe body of the test is inside a pair of single quotes, and the one\nafter format: makes you step outside the single quote, take two\nbytes %H literally, and the other single quote opens a new singly\nquoted string segment. Which is not wrong per-se, because there is\nno special meaning attached to the sequence %H in the shell language,\nbut then you'd be better off writing format:%H without any quotes,\nas that is more direct way to write what you are writing.\n\nAlso, --pretty=format: is almost always a mistake.\nUnless you have a good reason to use it, you'd most likely want to\nuse --format= instead.\n\nIn any case, don't abuse \"log\" when you mean\n\n commit_object_name=$(git rev-parse HEAD) &&\n\n> +\techo \"$commit_sha commit $commit_size\" >expect &&\n\nAs $commit_size here may have extra and unwanted SP before it, this\nmay break with the implementation of \"wc\" on certain platforms. In\nthis particular instance, losing quoting, i.e.\n\n\techo $commit_sha commit $commit_size >expect\n\nmay be a good workaround.\n\n> +\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n\nExactly the same set of comments as above apply to this side, too.\n\n> +\techo \"$commit_sha commit $commit_size\" >>expect &&\n> +\techo \"HEAD\" >in &&\n> +\tgit cat-file --batch-check actual &&\n> +\tgit cat-file --use-mailmap --batch-check >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n> +\ttest_when_finished \"rm .mailmap\" &&\n> +\tcat >.mailmap <<-\\EOF &&\n> +\tC O Mitter Orig \n> +\tEOF\n> +\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n> +\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n> +\techo \"$commit_sha commit $commit_size\" >expect &&\n> +\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n> +\techo \"$commit_sha commit $commit_size\" >>expect &&\n> +\techo \"info HEAD\" >in &&\n> +\tgit cat-file --batch-command actual &&\n> +\tgit cat-file --use-mailmap --batch-command >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n"},{"id":"467664","messageId":"CAP8UFD1CB90eoWpQmGJbfxK7uHX0-u4BuSE-v=mD1yuW+nnAxA@mail.gmail.com","threadId":"58447","inReplyTo":"xmqqy1s4wyvf.fsf@gitster.g","subject":"Re: [PATCH v5 1/2] cat-file: add mailmap support to -s option","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-11-21T09:40:31Z","receivedAt":"2022-11-21T09:40:47Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Nov 21, 2022 at 8:27 AM Junio C Hamano wrote:\n>\n> Siddharth Asthana writes:\n>\n> > +test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n> > + test_when_finished \"rm .mailmap\" &&\n> > + cat >.mailmap <<-\\EOF &&\n> > + C O Mitter Orig \n> > + EOF\n> > + git cat-file commit HEAD | wc -c >expect &&\n> > + git cat-file --use-mailmap commit HEAD | wc -c >>expect &&\n> > + git cat-file -s HEAD >actual &&\n> > + git cat-file --use-mailmap -s HEAD >>actual &&\n>\n> Doesn't this break under macOS where wc output tends to be padded\n> with SP on the right? We used to often see test breakage when a\n> carelessly written test like\n>\n> test \"$(wc -l \n> which expects the output file to have exactly two files (the\n> solution in this sample case is to lose the double quotes around the\n> command substitution).\n\nI guess that's the reason why `wc -c | sed -e 's/^ *//'` is used in\nthe strlen() function in t1006-cat-file.sh. There are a number of\nplaces in the tests where wc -c or wc -l are used without piping the\nresult into sed -e 's/^ *//' though. So it's not easy to understand\nwhy it's sometimes needed.\n\n> Besides, having \"cat-file\" on the upstream side of a pipe is a bad\n> practice.\n\nYeah, right. So I would suggest the following:\n\n git cat-file commit HEAD >commit.out &&\n wc -c expect &&\n git cat-file --use-mailmap commit HEAD >commit.out &&\n wc -c >expect &&\n\nThanks!\n"},{"id":"467665","messageId":"xmqqtu2svdy2.fsf@gitster.g","threadId":"58447","inReplyTo":"CAP8UFD1CB90eoWpQmGJbfxK7uHX0-u4BuSE-v=mD1yuW+nnAxA@mail.gmail.com","subject":"Re: [PATCH v5 1/2] cat-file: add mailmap support to -s option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-21T09:45:09Z","receivedAt":"2022-11-21T09:45:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Yeah, right. So I would suggest the following:\n>\n> git cat-file commit HEAD >commit.out &&\n> wc -c expect &&\n\nYou should not use sed for things like that.\n\n\techo $(wc -c On Mon, Nov 21, 2022 at 8:27 AM Junio C Hamano wrote:\n>>\n>> Siddharth Asthana writes:\n>>\n>> > +test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n>> > + test_when_finished \"rm .mailmap\" &&\n>> > + cat >.mailmap <<-\\EOF &&\n>> > + C O Mitter Orig \n>> > + EOF\n>> > + git cat-file commit HEAD | wc -c >expect &&\n>> > + git cat-file --use-mailmap commit HEAD | wc -c >>expect &&\n>> > + git cat-file -s HEAD >actual &&\n>> > + git cat-file --use-mailmap -s HEAD >>actual &&\n>>\n>> Doesn't this break under macOS where wc output tends to be padded\n>> with SP on the right? We used to often see test breakage when a\n>> carelessly written test like\n>>\n>> test \"$(wc -l >\n>> which expects the output file to have exactly two files (the\n>> solution in this sample case is to lose the double quotes around the\n>> command substitution).\n>\n> I guess that's the reason why `wc -c | sed -e 's/^ *//'` is used in\n> the strlen() function in t1006-cat-file.sh. There are a number of\n> places in the tests where wc -c or wc -l are used without piping the\n> result into sed -e 's/^ *//' though. So it's not easy to understand\n> why it's sometimes needed.\n\nIt's because in \"t1006-cat-file.sh\" we're assigning the \"wc -c\" to a\nvariable, because it's used to \"test_cmp\" the number of bytes in some\nfree-form text.\n\nIt would be nicer to split \"test_line_count\" into some utility function\nthat knew how to parse out \"wc -l\", \"wc -c\" etc. for a given input file,\nand return that as a string.\n\nIn that case the \"sed\" isn't needed, and we're just (ab)using it to do\nthings we can do with whitespace managent + shell built-ins. E.g. this\nworks too (the \"echo; echo; echo\" showing that we're stripping out\nwhitespace \"wc -c\" might emit:\n\t\n\tdiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n\tindex 23b8942edba..9ae4b534421 100755\n\t--- a/t/t1006-cat-file.sh\n\t+++ b/t/t1006-cat-file.sh\n\t@@ -106,7 +106,7 @@ echo_without_newline_nul () {\n\t }\n\t \n\t strlen () {\n\t- echo_without_newline \"$1\" | wc -c | sed -e 's/^ *//'\n\t+ printf \"%s\" $(printf \"%s\" \"$1\" | (echo ; echo ; echo ; wc -c))\n\t }\n\t \n\t maybe_remove_timestamp () {\n"},{"id":"468234","messageId":"xmqqa648ztn8.fsf@gitster.g","threadId":"58447","inReplyTo":"xmqqbkp0wyd8.fsf@gitster.g","subject":"Re: [PATCH v5 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-30T09:19:39Z","receivedAt":"2022-11-30T09:19:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Note that some implementations of \"wc\" pads its output with SP. The\n> implication will be seen in a few paragraphs below.\n> ...\n> In any case, don't abuse \"log\" when you mean\n>\n> commit_object_name=$(git rev-parse HEAD) &&\n>\n>> +\techo \"$commit_sha commit $commit_size\" >expect &&\n>\n> As $commit_size here may have extra and unwanted SP before it, this\n> may break with the implementation of \"wc\" on certain platforms. In\n> this particular instance, losing quoting, i.e.\n>\n> \techo $commit_sha commit $commit_size >expect\n>\n> may be a good workaround.\n\nAnd this indeed does break GitHub CI osx jobs ...\n\n https://github.com/git/git/actions/runs/3581605069/jobs/6024866010#step:4:1860\n\n... in the way exactly I predicted to break in the message I am\nresponding to.\n\n"},{"id":"468327","messageId":"20221201155504.320461-1-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH v6 0/2] Add mailmap mechanism in cat-file options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-12-01T15:55:02Z","receivedAt":"2022-12-01T15:55:25Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Thanks a ton Christian, Ævar and Junio for review :) I have made the\nsuggested changes in this patch series.\n\n= Description\n\nAt present, `git-cat-file` command with `--batch-check` and `-s` options\ndoes not complain when `--use-mailmap` option is given. The latter\noption is just ignored. Instead, for commit/tag objects, the command\nshould compute the size of the object after replacing the idents and\nreport it. So, this patch series makes `-s` and `--batch-check` options\nof `git-cat-file` honor mailmap when used with `--use-mailmap` option.\n\nIn this patch series we didn't want to change that '%(objectsize)'\nalways shows the size of the original object even when `--use-mailmap`\nis set because first we have the long term plan to unify how the formats\nfor `git cat-file` and other commands works. And second existing formats\nlike the \"pretty formats\" used by `git log` have different options for\nfields respecting mailmap or not respecting it (%an is for author name\nwhile %aN for author name respecting mailmap).\n\nI would like to thank my mentors, Christian Couder and John Cai, for all\nof their help!\nLooking forward to the reviews!\n\n= Patch Organization\n\n- The first patch makes `-s` option to return updated size of the\n object, when combined with `--use-mailmap` option, after\n replacing the idents using the mailmap mechanism.\n- The second patch makes `--batch-check` option to return updated size of\n the object, when combined with `--use-mailmap` option,\n after replacing the idents using the mailmap mechanism.\n\n= Changes in v5\n\n- The patch series which improves the documentation for `-s`, `--batch`,\n `--batch-check` and `--batch-command` is again part of this patch\n series with patch 3/3 squashed into patches 1/3 and 2/3 as suggested\n by Junio, Taylor and Christian. The doc patch series was perviously\n sent independently for improving the documentation of git cat-file\n options:\n https://lore.kernel.org/git/20221029092513.73982-1-siddharthasthana31@gmail.com/\n\n- Improve the tests according to run under SHA-256 mode.\n\n= Changes in v6\n\n- Improve the tests so it doesn't break under macOS.\n\nSiddharth Asthana (2):\n cat-file: add mailmap support to -s option\n cat-file: add mailmap support to --batch-check option\n\n Documentation/git-cat-file.txt | 53 +++++++++++++++++++--------\n builtin/cat-file.c | 27 ++++++++++++++\n t/t4203-mailmap.sh | 65 ++++++++++++++++++++++++++++++++++\n 3 files changed, 131 insertions(+), 14 deletions(-)\n\nRange-diff against v5:\n1: 0db3583535 ! 1: 15366d872e cat-file: add mailmap support to -s option\n @@ t/t4203-mailmap.sh: test_expect_success '--mailmap enables mailmap in cat-file f\n +\tcat >.mailmap <<-\\EOF &&\n +\tC O Mitter Orig \n +\tEOF\n -+\tgit cat-file commit HEAD | wc -c >expect &&\n -+\tgit cat-file --use-mailmap commit HEAD | wc -c >>expect &&\n ++\tgit cat-file commit HEAD >commit.out &&\n ++\techo $(wc -c expect &&\n ++\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n ++\techo $(wc -c >expect &&\n +\tgit cat-file -s HEAD >actual &&\n +\tgit cat-file --use-mailmap -s HEAD >>actual &&\n +\ttest_cmp expect actual\n @@ t/t4203-mailmap.sh: test_expect_success '--mailmap enables mailmap in cat-file f\n +\tOrig C O Mitter \n +\tEOF\n +\tgit tag -a -m \"annotated tag\" v3 &&\n -+\tgit cat-file tag v3 | wc -c >expect &&\n -+\tgit cat-file --use-mailmap tag v3 | wc -c >>expect &&\n ++\tgit cat-file tag v3 >tag.out &&\n ++\techo $(wc -c expect &&\n ++\tgit cat-file --use-mailmap tag v3 >tag.out &&\n ++\techo $(wc -c >expect &&\n +\tgit cat-file -s v3 >actual &&\n +\tgit cat-file --use-mailmap -s v3 >>actual &&\n +\ttest_cmp expect actual\n2: b8205ede7d ! 2: 86692db720 cat-file: add mailmap support to --batch-check option\n @@ t/t4203-mailmap.sh: test_expect_success 'git cat-file -s returns correct size wi\n +\tcat >.mailmap <<-\\EOF &&\n +\tC O Mitter Orig \n +\tEOF\n -+\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n -+\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n -+\techo \"$commit_sha commit $commit_size\" >expect &&\n -+\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n -+\techo \"$commit_sha commit $commit_size\" >>expect &&\n ++\tgit cat-file commit HEAD >commit.out &&\n ++\tcommit_size=$(wc -c expect &&\n ++\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n ++\tcommit_size=$(wc -c >expect &&\n +\techo \"HEAD\" >in &&\n +\tgit cat-file --batch-check actual &&\n +\tgit cat-file --use-mailmap --batch-check >actual &&\n @@ t/t4203-mailmap.sh: test_expect_success 'git cat-file -s returns correct size wi\n +\tcat >.mailmap <<-\\EOF &&\n +\tC O Mitter Orig \n +\tEOF\n -+\tcommit_size=`git cat-file commit HEAD | wc -c` &&\n -+\tcommit_sha=`git log --pretty=format:'%H' -n 1` &&\n -+\techo \"$commit_sha commit $commit_size\" >expect &&\n -+\tcommit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&\n -+\techo \"$commit_sha commit $commit_size\" >>expect &&\n ++\tgit cat-file commit HEAD >commit.out &&\n ++\tcommit_size=$(wc -c expect &&\n ++\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n ++\tcommit_size=$(wc -c >expect &&\n +\techo \"info HEAD\" >in &&\n +\tgit cat-file --batch-command actual &&\n +\tgit cat-file --use-mailmap --batch-command >actual &&\n-- \n2.39.0.rc1.6.g86692db720\n\n"},{"id":"468328","messageId":"20221201155504.320461-2-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221201155504.320461-1-siddharthasthana31@gmail.com","subject":"[PATCH v6 1/2] cat-file: add mailmap support to -s option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-12-01T15:55:03Z","receivedAt":"2022-12-01T15:55:33Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `-s` option does not complain when\n`--use-mailmap` option is given, the latter option is ignored. Compute\nthe size of the object after replacing the idents and report it instead.\n\nIn order to make `-s` option honour the mailmap mechanism we have to\nread the contents of the commit/tag object. Make use of the call to\n`oid_object_info_extended()` to get the contents of the object and store\nin `buf`. `buf` is later freed in the function.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 4 +++-\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 29 +++++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex ec30b5c574..f82d702d6b 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -45,7 +45,9 @@ OPTIONS\n \n -s::\n \tInstead of the content, show the object size identified by\n-\t``.\n+\t``. If used with `--use-mailmap` option, will show\n+\tthe size of updated object after replacing idents using the\n+\tmailmap mechanism.\n \n -e::\n \tExit with zero status if `` exists and is a valid\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex b3be58b1fb..dde8dbeacd 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \tcase 's':\n \t\toi.sizep = &size;\n+\n+\t\tif (use_mailmap) {\n+\t\t\toi.typep = &type;\n+\t\t\toi.contentp = (void**)&buf;\n+\t\t}\n+\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n+\n+\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n+\t\t\tsize_t s = size;\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tsize = cast_size_t_to_ulong(s);\n+\t\t}\n+\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex cd1cab3e54..b8ec5e0959 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1022,4 +1022,33 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tgit cat-file commit HEAD >commit.out &&\n+\techo $(wc -c expect &&\n+\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n+\techo $(wc -c >expect &&\n+\tgit cat-file -s HEAD >actual &&\n+\tgit cat-file --use-mailmap -s HEAD >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap for tag objects' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tOrig C O Mitter \n+\tEOF\n+\tgit tag -a -m \"annotated tag\" v3 &&\n+\tgit cat-file tag v3 >tag.out &&\n+\techo $(wc -c expect &&\n+\tgit cat-file --use-mailmap tag v3 >tag.out &&\n+\techo $(wc -c >expect &&\n+\tgit cat-file -s v3 >actual &&\n+\tgit cat-file --use-mailmap -s v3 >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.39.0.rc1.6.g86692db720\n\n"},{"id":"468329","messageId":"20221201155504.320461-3-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221201155504.320461-1-siddharthasthana31@gmail.com","subject":"[PATCH v6 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-12-01T15:55:04Z","receivedAt":"2022-12-01T15:55:38Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `--batch-check` option does not\ncomplain when `--use-mailmap` option is given, the latter option is\nignored. Compute the size of the object after replacing the idents and\nreport it instead.\n\nIn order to make `--batch-check` option honour the mailmap mechanism we\nhave to read the contents of the commit/tag object.\n\nThere were two ways to do it:\n\n1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n option is given, the first call will get us the type of the object\n and second call will only be made if the object type is either a\n commit or tag to get the contents of the object.\n\n2. Make one call to `oid_object_info_extended()` to get the type of the\n object. Then, if the object type is either of commit or tag, make a\n call to `read_object_file()` to read the contents of the object.\n\nI benchmarked the following command with both the above approaches and\ncompared against the current implementation where `--use-mailmap`\noption is ignored:\n\n`git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n--unordered`\n\nThe results can be summarized as follows:\n Time (mean ± σ)\ndefault 827.7 ms ± 104.8 ms\nfirst approach 6.197 s ± 0.093 s\nsecond approach 1.975 s ± 0.217 s\n\nSince, the second approach is faster than the first one, I implemented\nit in this patch.\n\nThe command git cat-file can now use the mailmap mechanism to replace\nidents with canonical versions for commit and tag objects. There are\nseveral options like `--batch`, `--batch-check` and `--batch-command`\nthat can be combined with `--use-mailmap`. But the documentation for\n`--batch`, `--batch-check` and `--batch-command` doesn't say so. This\npatch fixes that documentation.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 49 +++++++++++++++++++++++++---------\n builtin/cat-file.c | 14 ++++++++++\n t/t4203-mailmap.sh | 36 +++++++++++++++++++++++++\n 3 files changed, 86 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex f82d702d6b..81235c60a3 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -91,26 +91,49 @@ OPTIONS\n --batch::\n --batch=::\n \tPrint object information and contents for each object provided\n-\ton stdin. May not be combined with any other options or arguments\n-\texcept `--textconv` or `--filters`, in which case the input lines\n-\talso need to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\ton stdin. May not be combined with any other options or arguments\n+\texcept --textconv, --filters, or --use-mailmap.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines\n+\t must specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t contents part of the output shows the identities replaced using the\n+\t mailmap mechanism, while the information part of the output shows\n+\t the size of the object as if it actually recorded the replacement\n+\t identities.\n \n --batch-check::\n --batch-check=::\n-\tPrint object information for each object provided on stdin. May\n-\tnot be combined with any other options or arguments except\n-\t`--textconv` or `--filters`, in which case the input lines also\n-\tneed to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\tPrint object information for each object provided on stdin. May not be\n+\tcombined with any other options or arguments except --textconv, --filters\n+\tor --use-mailmap.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t printed object information shows the size of the object as if the\n+\t identities recorded in it were replaced by the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\n \tEnter a command mode that reads commands and arguments from stdin. May\n-\tonly be combined with `--buffer`, `--textconv` or `--filters`. In the\n-\tcase of `--textconv` or `--filters`, the input lines also need to specify\n-\tthe path, separated by whitespace. See the section `BATCH OUTPUT` below\n-\tfor details.\n+\tonly be combined with `--buffer`, `--textconv`, `--use-mailmap` or\n+\t`--filters`.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t `contents` command shows the identities replaced using the\n+\t mailmap mechanism, while the `info` command shows the size\n+\t of the object as if it actually recorded the replacement\n+\t identities.\n+\n +\n `--batch-command` recognizes the following commands:\n +\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex dde8dbeacd..7811016ae8 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -444,6 +444,9 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap)\n+\t\t\tdata->info.typep = &data->type;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -457,6 +460,17 @@ static void batch_object_write(const char *obj_name,\n \t\t\tfflush(stdout);\n \t\t\treturn;\n \t\t}\n+\n+\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n+\t\t\tsize_t s = data->size;\n+\t\t\tchar *buf = NULL;\n+\n+\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tdata->size = cast_size_t_to_ulong(s);\n+\n+\t\t\tfree(buf);\n+\t\t}\n \t}\n \n \tstrbuf_reset(scratch);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex b8ec5e0959..fa7f987284 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1051,4 +1051,40 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tgit cat-file commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c expect &&\n+\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c >expect &&\n+\techo \"HEAD\" >in &&\n+\tgit cat-file --batch-check actual &&\n+\tgit cat-file --use-mailmap --batch-check >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tgit cat-file commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c expect &&\n+\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c >expect &&\n+\techo \"info HEAD\" >in &&\n+\tgit cat-file --batch-command actual &&\n+\tgit cat-file --use-mailmap --batch-command >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.39.0.rc1.6.g86692db720\n\n"},{"id":"469011","messageId":"221214.86a63q441k.gmgdl@evledraar.gmail.com","threadId":"58447","inReplyTo":"20221201155504.320461-3-siddharthasthana31@gmail.com","subject":"Re: [PATCH v6 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-14T11:27:20Z","receivedAt":"2022-12-14T11:29:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 01 2022, Siddharth Asthana wrote:\n\n> 2. Make one call to `oid_object_info_extended()` to get the type of the\n> object. Then, if the object type is either of commit or tag, make a\n> call to `read_object_file()` to read the contents of the object.\n> [...]\n> +\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n\nCould you please change this to:\n\n\tbuf = repo_read_object_file(the_repository, &data->oid, &data->type,\n\t\t\t\t &data->size);\n\nI.e. just use the repo_*() variant. We have been trying for a long time\nto migrate away from these, and new code should use the non-macro\nvariants.\n\nI noticed this because I have a local topic to do that migration, which\nhas a semantic conflict with this.\n"},{"id":"469013","messageId":"CAP8UFD0_86GiwNXmCZzVfDJTE5C6KWz=8F9S4hVEA5Xi8XhgDw@mail.gmail.com","threadId":"58447","inReplyTo":"20221201155504.320461-3-siddharthasthana31@gmail.com","subject":"Re: [PATCH v6 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-12-14T14:04:17Z","receivedAt":"2022-12-14T14:05:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Dec 1, 2022 at 4:55 PM Siddharth Asthana\n wrote:\n\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index f82d702d6b..81235c60a3 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -91,26 +91,49 @@ OPTIONS\n> --batch::\n> --batch=::\n> Print object information and contents for each object provided\n> - on stdin. May not be combined with any other options or arguments\n> - except `--textconv` or `--filters`, in which case the input lines\n\nNit: here there were backticks around --textconv and --filters ...\n\n> - also need to specify the path, separated by whitespace. See the\n> - section `BATCH OUTPUT` below for details.\n> + on stdin. May not be combined with any other options or arguments\n> + except --textconv, --filters, or --use-mailmap.\n\n... but here there are no backticks anymore.\n\nIt would be better if backticks were used.\n\n> + +\n> + * When used with `--textconv` or `--filters`, the input lines\n\nHere and below backticks are used which is good.\n\n> + must specify the path, separated by whitespace. See the section\n> + `BATCH OUTPUT` below for details.\n> + +\n> + * When used with `--use-mailmap`, for commit and tag objects, the\n> + contents part of the output shows the identities replaced using the\n> + mailmap mechanism, while the information part of the output shows\n> + the size of the object as if it actually recorded the replacement\n> + identities.\n>\n> --batch-check::\n> --batch-check=::\n> - Print object information for each object provided on stdin. May\n> - not be combined with any other options or arguments except\n> - `--textconv` or `--filters`, in which case the input lines also\n> - need to specify the path, separated by whitespace. See the\n> - section `BATCH OUTPUT` below for details.\n> + Print object information for each object provided on stdin. May not be\n> + combined with any other options or arguments except --textconv, --filters\n> + or --use-mailmap.\n\nHere backticks are also missing.\n\n> + +\n> + * When used with `--textconv` or `--filters`, the input lines must\n> + specify the path, separated by whitespace. See the section\n> + `BATCH OUTPUT` below for details.\n> + +\n> + * When used with `--use-mailmap`, for commit and tag objects, the\n> + printed object information shows the size of the object as if the\n> + identities recorded in it were replaced by the mailmap mechanism.\n>\n> --batch-command::\n> --batch-command=::\n> Enter a command mode that reads commands and arguments from stdin. May\n> - only be combined with `--buffer`, `--textconv` or `--filters`. In the\n> - case of `--textconv` or `--filters`, the input lines also need to specify\n> - the path, separated by whitespace. See the section `BATCH OUTPUT` below\n> - for details.\n> + only be combined with `--buffer`, `--textconv`, `--use-mailmap` or\n> + `--filters`.\n\nHere they are used which is good.\n\n> + +\n> + * When used with `--textconv` or `--filters`, the input lines must\n> + specify the path, separated by whitespace. See the section\n> + `BATCH OUTPUT` below for details.\n> + +\n> + * When used with `--use-mailmap`, for commit and tag objects, the\n> + `contents` command shows the identities replaced using the\n> + mailmap mechanism, while the `info` command shows the size\n> + of the object as if it actually recorded the replacement\n> + identities.\n\nThanks!\n"},{"id":"469351","messageId":"20221220060113.51010-1-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20220916205946.178925-1-siddharthasthana31@gmail.com","subject":"[PATCH v7 0/2] Add mailmap mechanism in cat-file options","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-12-20T06:01:11Z","receivedAt":"2022-12-20T06:01:29Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Thanks a ton Christian and Ævar for the review :) I have made the\nsuggested changes in v7.\n\n= Description\n\nAt present, `git-cat-file` command with `--batch-check` and `-s` options\ndoes not complain when `--use-mailmap` option is given. The latter\noption is just ignored. Instead, for commit/tag objects, the command\nshould compute the size of the object after replacing the idents and\nreport it. So, this patch series makes `-s` and `--batch-check` options\nof `git-cat-file` honor mailmap when used with `--use-mailmap` option.\n\nIn this patch series we didn't want to change that '%(objectsize)'\nalways shows the size of the original object even when `--use-mailmap`\nis set because first we have the long term plan to unify how the formats\nfor `git cat-file` and other commands works. And second existing formats\nlike the \"pretty formats\" used by `git log` have different options for\nfields respecting mailmap or not respecting it (%an is for author name\nwhile %aN for author name respecting mailmap).\n\nI would like to thank my mentors, Christian Couder and John Cai, for all\nof their help!\nLooking forward to the reviews!\n\n= Patch Organization\n\n- The first patch makes `-s` option to return updated size of the\n object, when combined with `--use-mailmap` option, after\n replacing the idents using the mailmap mechanism.\n- The second patch makes `--batch-check` option to return updated size of\n the object, when combined with `--use-mailmap` option,\n after replacing the idents using the mailmap mechanism.\n\n= Changes in v5\n\n- The patch series which improves the documentation for `-s`, `--batch`,\n `--batch-check` and `--batch-command` is again part of this patch\n series with patch 3/3 squashed into patches 1/3 and 2/3 as suggested\n by Junio, Taylor and Christian. The doc patch series was perviously\n sent independently for improving the documentation of git cat-file\n options:\n https://lore.kernel.org/git/20221029092513.73982-1-siddharthasthana31@gmail.com/\n\n- Improve the tests according to run under SHA-256 mode.\n\n= Changes in v6\n\n- Improve the tests so it doesn't break under macOS.\n\n= Changes in v7\n\n- Used `repo_read_object_file` instead of `read_object_file` because we\n have been trying to migrate away from these, and new code should use\n the non-macro variants.\n- Improve the documentation of patch series.\n\nSiddharth Asthana (2):\n cat-file: add mailmap support to -s option\n cat-file: add mailmap support to --batch-check option\n\n Documentation/git-cat-file.txt | 53 +++++++++++++++++++--------\n builtin/cat-file.c | 28 +++++++++++++++\n t/t4203-mailmap.sh | 65 ++++++++++++++++++++++++++++++++++\n 3 files changed, 132 insertions(+), 14 deletions(-)\n\nRange-diff against v6:\n1: 7f20f45183 = 1: 2097544b2d cat-file: add mailmap support to -s option\n2: 971f38e064 ! 2: 8305148718 cat-file: add mailmap support to --batch-check option\n @@ Commit message\n \n 2. Make one call to `oid_object_info_extended()` to get the type of the\n object. Then, if the object type is either of commit or tag, make a\n - call to `read_object_file()` to read the contents of the object.\n + call to `repo_read_object_file()` to read the contents of the object.\n \n I benchmarked the following command with both the above approaches and\n compared against the current implementation where `--use-mailmap`\n @@ Documentation/git-cat-file.txt: OPTIONS\n -\talso need to specify the path, separated by whitespace. See the\n -\tsection `BATCH OUTPUT` below for details.\n +\ton stdin. May not be combined with any other options or arguments\n -+\texcept --textconv, --filters, or --use-mailmap.\n ++\texcept `--textconv`, `--filters`, or `--use-mailmap`.\n +\t+\n +\t* When used with `--textconv` or `--filters`, the input lines\n +\t must specify the path, separated by whitespace. See the section\n @@ Documentation/git-cat-file.txt: OPTIONS\n -\tneed to specify the path, separated by whitespace. See the\n -\tsection `BATCH OUTPUT` below for details.\n +\tPrint object information for each object provided on stdin. May not be\n -+\tcombined with any other options or arguments except --textconv, --filters\n -+\tor --use-mailmap.\n ++\tcombined with any other options or arguments except `--textconv`, `--filters`\n ++\tor `--use-mailmap`.\n +\t+\n +\t* When used with `--textconv` or `--filters`, the input lines must\n +\t specify the path, separated by whitespace. See the section\n @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n +\t\t\tsize_t s = data->size;\n +\t\t\tchar *buf = NULL;\n +\n -+\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n ++\t\t\tbuf = repo_read_object_file(the_repository, &data->oid, &data->type,\n ++\t\t\t\t\t\t &data->size);\n +\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n +\t\t\tdata->size = cast_size_t_to_ulong(s);\n +\n-- \n2.39.0.97.g8305148718\n\n"},{"id":"469352","messageId":"20221220060113.51010-2-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221220060113.51010-1-siddharthasthana31@gmail.com","subject":"[PATCH v7 1/2] cat-file: add mailmap support to -s option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-12-20T06:01:12Z","receivedAt":"2022-12-20T06:01:45Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `-s` option does not complain when\n`--use-mailmap` option is given, the latter option is ignored. Compute\nthe size of the object after replacing the idents and report it instead.\n\nIn order to make `-s` option honour the mailmap mechanism we have to\nread the contents of the commit/tag object. Make use of the call to\n`oid_object_info_extended()` to get the contents of the object and store\nin `buf`. `buf` is later freed in the function.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 4 +++-\n builtin/cat-file.c | 13 +++++++++++++\n t/t4203-mailmap.sh | 29 +++++++++++++++++++++++++++++\n 3 files changed, 45 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex ec30b5c574..f82d702d6b 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -45,7 +45,9 @@ OPTIONS\n \n -s::\n \tInstead of the content, show the object size identified by\n-\t``.\n+\t``. If used with `--use-mailmap` option, will show\n+\tthe size of updated object after replacing idents using the\n+\tmailmap mechanism.\n \n -e::\n \tExit with zero status if `` exists and is a valid\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex b3be58b1fb..dde8dbeacd 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -132,8 +132,21 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \tcase 's':\n \t\toi.sizep = &size;\n+\n+\t\tif (use_mailmap) {\n+\t\t\toi.typep = &type;\n+\t\t\toi.contentp = (void**)&buf;\n+\t\t}\n+\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n+\n+\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n+\t\t\tsize_t s = size;\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tsize = cast_size_t_to_ulong(s);\n+\t\t}\n+\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex cd1cab3e54..b8ec5e0959 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1022,4 +1022,33 @@ test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tgit cat-file commit HEAD >commit.out &&\n+\techo $(wc -c expect &&\n+\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n+\techo $(wc -c >expect &&\n+\tgit cat-file -s HEAD >actual &&\n+\tgit cat-file --use-mailmap -s HEAD >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file -s returns correct size with --use-mailmap for tag objects' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tOrig C O Mitter \n+\tEOF\n+\tgit tag -a -m \"annotated tag\" v3 &&\n+\tgit cat-file tag v3 >tag.out &&\n+\techo $(wc -c expect &&\n+\tgit cat-file --use-mailmap tag v3 >tag.out &&\n+\techo $(wc -c >expect &&\n+\tgit cat-file -s v3 >actual &&\n+\tgit cat-file --use-mailmap -s v3 >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.39.0.97.g8305148718\n\n"},{"id":"469353","messageId":"20221220060113.51010-3-siddharthasthana31@gmail.com","threadId":"58447","inReplyTo":"20221220060113.51010-1-siddharthasthana31@gmail.com","subject":"[PATCH v7 2/2] cat-file: add mailmap support to --batch-check option","fromName":"Siddharth Asthana","fromEmail":"siddharthasthana31@gmail.com","sentAt":"2022-12-20T06:01:13Z","receivedAt":"2022-12-20T06:01:55Z","isPatch":true,"sender":{"key":"siddharthasthana31@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53316982?v=4"},"body":"Even though the cat-file command with `--batch-check` option does not\ncomplain when `--use-mailmap` option is given, the latter option is\nignored. Compute the size of the object after replacing the idents and\nreport it instead.\n\nIn order to make `--batch-check` option honour the mailmap mechanism we\nhave to read the contents of the commit/tag object.\n\nThere were two ways to do it:\n\n1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`\n option is given, the first call will get us the type of the object\n and second call will only be made if the object type is either a\n commit or tag to get the contents of the object.\n\n2. Make one call to `oid_object_info_extended()` to get the type of the\n object. Then, if the object type is either of commit or tag, make a\n call to `repo_read_object_file()` to read the contents of the object.\n\nI benchmarked the following command with both the above approaches and\ncompared against the current implementation where `--use-mailmap`\noption is ignored:\n\n`git cat-file --use-mailmap --batch-all-objects --batch-check --buffer\n--unordered`\n\nThe results can be summarized as follows:\n Time (mean ± σ)\ndefault 827.7 ms ± 104.8 ms\nfirst approach 6.197 s ± 0.093 s\nsecond approach 1.975 s ± 0.217 s\n\nSince, the second approach is faster than the first one, I implemented\nit in this patch.\n\nThe command git cat-file can now use the mailmap mechanism to replace\nidents with canonical versions for commit and tag objects. There are\nseveral options like `--batch`, `--batch-check` and `--batch-command`\nthat can be combined with `--use-mailmap`. But the documentation for\n`--batch`, `--batch-check` and `--batch-command` doesn't say so. This\npatch fixes that documentation.\n\nMentored-by: Christian Couder \nMentored-by: John Cai \nHelped-by: Taylor Blau \nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Siddharth Asthana \n---\n Documentation/git-cat-file.txt | 49 +++++++++++++++++++++++++---------\n builtin/cat-file.c | 15 +++++++++++\n t/t4203-mailmap.sh | 36 +++++++++++++++++++++++++\n 3 files changed, 87 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex f82d702d6b..830f0a2eff 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -91,26 +91,49 @@ OPTIONS\n --batch::\n --batch=::\n \tPrint object information and contents for each object provided\n-\ton stdin. May not be combined with any other options or arguments\n-\texcept `--textconv` or `--filters`, in which case the input lines\n-\talso need to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\ton stdin. May not be combined with any other options or arguments\n+\texcept `--textconv`, `--filters`, or `--use-mailmap`.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines\n+\t must specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t contents part of the output shows the identities replaced using the\n+\t mailmap mechanism, while the information part of the output shows\n+\t the size of the object as if it actually recorded the replacement\n+\t identities.\n \n --batch-check::\n --batch-check=::\n-\tPrint object information for each object provided on stdin. May\n-\tnot be combined with any other options or arguments except\n-\t`--textconv` or `--filters`, in which case the input lines also\n-\tneed to specify the path, separated by whitespace. See the\n-\tsection `BATCH OUTPUT` below for details.\n+\tPrint object information for each object provided on stdin. May not be\n+\tcombined with any other options or arguments except `--textconv`, `--filters`\n+\tor `--use-mailmap`.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t printed object information shows the size of the object as if the\n+\t identities recorded in it were replaced by the mailmap mechanism.\n \n --batch-command::\n --batch-command=::\n \tEnter a command mode that reads commands and arguments from stdin. May\n-\tonly be combined with `--buffer`, `--textconv` or `--filters`. In the\n-\tcase of `--textconv` or `--filters`, the input lines also need to specify\n-\tthe path, separated by whitespace. See the section `BATCH OUTPUT` below\n-\tfor details.\n+\tonly be combined with `--buffer`, `--textconv`, `--use-mailmap` or\n+\t`--filters`.\n+\t+\n+\t* When used with `--textconv` or `--filters`, the input lines must\n+\t specify the path, separated by whitespace. See the section\n+\t `BATCH OUTPUT` below for details.\n+\t+\n+\t* When used with `--use-mailmap`, for commit and tag objects, the\n+\t `contents` command shows the identities replaced using the\n+\t mailmap mechanism, while the `info` command shows the size\n+\t of the object as if it actually recorded the replacement\n+\t identities.\n+\n +\n `--batch-command` recognizes the following commands:\n +\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex dde8dbeacd..cc17635e76 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -444,6 +444,9 @@ static void batch_object_write(const char *obj_name,\n \tif (!data->skip_object_info) {\n \t\tint ret;\n \n+\t\tif (use_mailmap)\n+\t\t\tdata->info.typep = &data->type;\n+\n \t\tif (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n@@ -457,6 +460,18 @@ static void batch_object_write(const char *obj_name,\n \t\t\tfflush(stdout);\n \t\t\treturn;\n \t\t}\n+\n+\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n+\t\t\tsize_t s = data->size;\n+\t\t\tchar *buf = NULL;\n+\n+\t\t\tbuf = repo_read_object_file(the_repository, &data->oid, &data->type,\n+\t\t\t\t\t\t &data->size);\n+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n+\t\t\tdata->size = cast_size_t_to_ulong(s);\n+\n+\t\t\tfree(buf);\n+\t\t}\n \t}\n \n \tstrbuf_reset(scratch);\ndiff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh\nindex b8ec5e0959..fa7f987284 100755\n--- a/t/t4203-mailmap.sh\n+++ b/t/t4203-mailmap.sh\n@@ -1051,4 +1051,40 @@ test_expect_success 'git cat-file -s returns correct size with --use-mailmap for\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tgit cat-file commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c expect &&\n+\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c >expect &&\n+\techo \"HEAD\" >in &&\n+\tgit cat-file --batch-check actual &&\n+\tgit cat-file --use-mailmap --batch-check >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '\n+\ttest_when_finished \"rm .mailmap\" &&\n+\tcat >.mailmap <<-\\EOF &&\n+\tC O Mitter Orig \n+\tEOF\n+\tgit cat-file commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c expect &&\n+\tgit cat-file --use-mailmap commit HEAD >commit.out &&\n+\tcommit_size=$(wc -c >expect &&\n+\techo \"info HEAD\" >in &&\n+\tgit cat-file --batch-command actual &&\n+\tgit cat-file --use-mailmap --batch-command >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.39.0.97.g8305148718\n\n"},{"id":"469373","messageId":"221220.865ye6xlmo.gmgdl@evledraar.gmail.com","threadId":"58447","inReplyTo":"20221220060113.51010-1-siddharthasthana31@gmail.com","subject":"Re: [PATCH v7 0/2] Add mailmap mechanism in cat-file options","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-20T13:02:07Z","receivedAt":"2022-12-20T13:14:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Dec 20 2022, Siddharth Asthana wrote:\n\n> 2. Make one call to `oid_object_info_extended()` to get the type of the\n> object. Then, if the object type is either of commit or tag, make a\n> - call to `read_object_file()` to read the contents of the object.\n> + call to `repo_read_object_file()` to read the contents of the object.\n> \n> I benchmarked the following command with both the above approaches and\n> compared against the current implementation where `--use-mailmap`\n> @@ Documentation/git-cat-file.txt: OPTIONS\n> -\talso need to specify the path, separated by whitespace. See the\n> -\tsection `BATCH OUTPUT` below for details.\n> +\ton stdin. May not be combined with any other options or arguments\n> -+\texcept --textconv, --filters, or --use-mailmap.\n> ++\texcept `--textconv`, `--filters`, or `--use-mailmap`.\n> +\t+\n> +\t* When used with `--textconv` or `--filters`, the input lines\n> +\t must specify the path, separated by whitespace. See the section\n> @@ Documentation/git-cat-file.txt: OPTIONS\n> -\tneed to specify the path, separated by whitespace. See the\n> -\tsection `BATCH OUTPUT` below for details.\n> +\tPrint object information for each object provided on stdin. May not be\n> -+\tcombined with any other options or arguments except --textconv, --filters\n> -+\tor --use-mailmap.\n> ++\tcombined with any other options or arguments except `--textconv`, `--filters`\n> ++\tor `--use-mailmap`.\n> +\t+\n> +\t* When used with `--textconv` or `--filters`, the input lines must\n> +\t specify the path, separated by whitespace. See the section\n> @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n> +\t\t\tsize_t s = data->size;\n> +\t\t\tchar *buf = NULL;\n> +\n> -+\t\t\tbuf = read_object_file(&data->oid, &data->type, &data->size);\n> ++\t\t\tbuf = repo_read_object_file(the_repository, &data->oid, &data->type,\n> ++\t\t\t\t\t\t &data->size);\n> +\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n> +\t\t\tdata->size = cast_size_t_to_ulong(s);\n> +\n\n\nThis series looks good to me at this point, thanks in particular for the\nrepo_*() change (will make something I plan to submit soon easier).\n\nThese[1][2] are nits I came up with while reviewing this, not worth a\nre-roll, but tweaked some things I found a bit odd, namely:\n\n * Let's not cast to void **, instead we can just declare a variable for\n the 's' case\n * If we don't need the \"buf\" return value we can skip the assignment\n (although I guess technically this breaks the encapsulation, so maybe\n we shouldn't skip it)\n * We can skip the NULL assignment in 2/2, and instead just assign to\n the variable as we declare it, and also do the replace/free on one\n line (to make it clear that we immediately don't care about it, and\n only want the size).\n\nI don't think any of it's worth a re-roll, just notes to show you I've\nlooked at it carefully.\n\nThe only unresolved question I had while reading this is if we're sure\nthat a repo_read_object_file() following the a successful\noid_object_info_extended() is guaranteed to succeed? If not the code in\n2/2 has a segfault we might trigger (as buf will be NULL), but maybe\nwe're guaranteed to always get the already-retrieved object from the\nobject cache.\n\n1: fe3cc3715b2 ! 1: 31701b3e55d cat-file: add mailmap support to -s option\n @@ Documentation/git-cat-file.txt: OPTIONS\n \n ## builtin/cat-file.c ##\n @@ builtin/cat-file.c: static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n + \t\tbreak;\n \n \tcase 's':\n ++\t{\n ++\t\tvoid *obuf = NULL;\n ++\n \t\toi.sizep = &size;\n +\n +\t\tif (use_mailmap) {\n +\t\t\toi.typep = &type;\n -+\t\t\toi.contentp = (void**)&buf;\n ++\t\t\toi.contentp = &obuf;\n +\t\t}\n +\n \t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n @@ builtin/cat-file.c: static int cat_one_file(int opt, const char *exp_type, const\n +\n +\t\tif (use_mailmap && (type == OBJ_COMMIT || type == OBJ_TAG)) {\n +\t\t\tsize_t s = size;\n -+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n ++\n ++\t\t\treplace_idents_using_mailmap(obuf, &s);\n +\t\t\tsize = cast_size_t_to_ulong(s);\n +\t\t}\n ++\t\tfree(obuf);\n +\n \t\tprintf(\"%\"PRIuMAX\"\\n\", (uintmax_t)size);\n \t\tret = 0;\n \t\tgoto cleanup;\n +-\n ++\t}\n + \tcase 'e':\n + \t\treturn !has_object_file(&oid);\n + \n \n ## t/t4203-mailmap.sh ##\n @@ t/t4203-mailmap.sh: test_expect_success '--mailmap enables mailmap in cat-file for annotated tag obj\n2: d6c621820d2 ! 2: 14d95db69e9 cat-file: add mailmap support to --batch-check option\n @@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,\n +\n +\t\tif (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {\n +\t\t\tsize_t s = data->size;\n -+\t\t\tchar *buf = NULL;\n ++\t\t\tchar *buf = repo_read_object_file(the_repository,\n ++\t\t\t\t\t\t\t &data->oid,\n ++\t\t\t\t\t\t\t &data->type,\n ++\t\t\t\t\t\t\t &data->size);\n +\n -+\t\t\tbuf = repo_read_object_file(the_repository, &data->oid, &data->type,\n -+\t\t\t\t\t\t &data->size);\n -+\t\t\tbuf = replace_idents_using_mailmap(buf, &s);\n ++\t\t\tfree(replace_idents_using_mailmap(buf, &s));\n +\t\t\tdata->size = cast_size_t_to_ulong(s);\n -+\n -+\t\t\tfree(buf);\n +\t\t}\n \t}\n \n"}]}