From: Siddharth Asthana <siddharthasthana31@gmail.com>
To: git@vger.kernel.org
Cc: christian.couder@gmail.com, gitster@pobox.com,
johncai86@gmail.com, Johannes.Schindelin@gmx.de,
avarab@gmail.com, me@ttaylorr.com,
Siddharth Asthana <siddharthasthana31@gmail.com>
Subject: [PATCH v5 0/2] Add mailmap mechanism in cat-file options
Date: Sun, 20 Nov 2022 13:18:50 +0530 [thread overview]
Message-ID: <20221120074852.121346-1-siddharthasthana31@gmail.com> (raw)
In-Reply-To: <20220916205946.178925-1-siddharthasthana31@gmail.com>
Thanks a lot Christian, Taylor and Junio for review :) I have made the
suggested in this patch series.
= Description
At present, `git-cat-file` command with `--batch-check` and `-s` options
does not complain when `--use-mailmap` option is given. The latter
option is just ignored. Instead, for commit/tag objects, the command
should compute the size of the object after replacing the idents and
report it. So, this patch series makes `-s` and `--batch-check` options
of `git-cat-file` honor mailmap when used with `--use-mailmap` option.
In this patch series we didn't want to change that '%(objectsize)'
always shows the size of the original object even when `--use-mailmap`
is set because first we have the long term plan to unify how the formats
for `git cat-file` and other commands works. And second existing formats
like the "pretty formats" used by `git log` have different options for
fields respecting mailmap or not respecting it (%an is for author name
while %aN for author name respecting mailmap).
I would like to thank my mentors, Christian Couder and John Cai, for all
of their help!
Looking forward to the reviews!
= Patch Organization
- The first patch makes `-s` option to return updated size of the
<commit/tag> object, when combined with `--use-mailmap` option, after
replacing the idents using the mailmap mechanism.
- The second patch makes `--batch-check` option to return updated size of
the <commit/tag> object, when combined with `--use-mailmap` option,
after replacing the idents using the mailmap mechanism.
= Changes in v5
- The patch series which improves the documentation for `-s`, `--batch`,
`--batch-check` and `--batch-command` is again part of this patch
series with patch 3/3 squashed into patches 1/3 and 2/3 as suggested
by Junio, Taylor and Christian. The doc patch series was perviously
sent independently for improving the documentation of git cat-file
options:
https://lore.kernel.org/git/20221029092513.73982-1-siddharthasthana31@gmail.com/
- Improve the tests according to run under SHA-256 mode.
Siddharth Asthana (2):
cat-file: add mailmap support to -s option
cat-file: add mailmap support to --batch-check option
Documentation/git-cat-file.txt | 53 ++++++++++++++++++++++---------
builtin/cat-file.c | 27 ++++++++++++++++
t/t4203-mailmap.sh | 57 ++++++++++++++++++++++++++++++++++
3 files changed, 123 insertions(+), 14 deletions(-)
Range-diff against v4:
1: 4ae3af37d2 ! 1: 0db3583535 cat-file: add mailmap support to -s option
@@ Commit message
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>
+ ## Documentation/git-cat-file.txt ##
+@@ Documentation/git-cat-file.txt: OPTIONS
+
+ -s::
+ Instead of the content, show the object size identified by
+- `<object>`.
++ `<object>`. If used with `--use-mailmap` option, will show
++ the size of updated object after replacing idents using the
++ mailmap mechanism.
+
+ -e::
+ Exit with zero status if `<object>` exists and is a valid
+
## builtin/cat-file.c ##
@@ builtin/cat-file.c: static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
@@ t/t4203-mailmap.sh: test_expect_success '--mailmap enables mailmap in cat-file f
+ cat >.mailmap <<-\EOF &&
+ C O Mitter <committer@example.com> Orig <orig@example.com>
+ EOF
-+ cat >expect <<-\EOF &&
-+ 209
-+ 220
-+ EOF
++ git cat-file commit HEAD | wc -c >expect &&
++ git cat-file --use-mailmap commit HEAD | wc -c >>expect &&
+ git cat-file -s HEAD >actual &&
+ git cat-file --use-mailmap -s HEAD >>actual &&
+ test_cmp expect actual
@@ t/t4203-mailmap.sh: test_expect_success '--mailmap enables mailmap in cat-file f
+ Orig <orig@example.com> C O Mitter <committer@example.com>
+ EOF
+ git tag -a -m "annotated tag" v3 &&
-+ cat >expect <<-\EOF &&
-+ 141
-+ 130
-+ EOF
++ git cat-file tag v3 | wc -c >expect &&
++ git cat-file --use-mailmap tag v3 | wc -c >>expect &&
+ git cat-file -s v3 >actual &&
+ git cat-file --use-mailmap -s v3 >>actual &&
+ test_cmp expect actual
2: a692646228 < -: ---------- cat-file: add mailmap support to --batch-check option
3: 41b4650b24 ! 2: b8205ede7d doc/cat-file: allow --use-mailmap for --batch options
@@ Metadata
Author: Siddharth Asthana <siddharthasthana31@gmail.com>
## Commit message ##
- doc/cat-file: allow --use-mailmap for --batch options
+ cat-file: add mailmap support to --batch-check option
+
+ Even though the cat-file command with `--batch-check` option does not
+ complain when `--use-mailmap` option is given, the latter option is
+ ignored. Compute the size of the object after replacing the idents and
+ report it instead.
+
+ In order to make `--batch-check` option honour the mailmap mechanism we
+ have to read the contents of the commit/tag object.
+
+ There were two ways to do it:
+
+ 1. Make two calls to `oid_object_info_extended()`. If `--use-mailmap`
+ option is given, the first call will get us the type of the object
+ and second call will only be made if the object type is either a
+ commit or tag to get the contents of the object.
+
+ 2. Make one call to `oid_object_info_extended()` to get the type of the
+ object. Then, if the object type is either of commit or tag, make a
+ call to `read_object_file()` to read the contents of the object.
+
+ I benchmarked the following command with both the above approaches and
+ compared against the current implementation where `--use-mailmap`
+ option is ignored:
+
+ `git cat-file --use-mailmap --batch-all-objects --batch-check --buffer
+ --unordered`
+
+ The results can be summarized as follows:
+ Time (mean ± σ)
+ default 827.7 ms ± 104.8 ms
+ first approach 6.197 s ± 0.093 s
+ second approach 1.975 s ± 0.217 s
+
+ Since, the second approach is faster than the first one, I implemented
+ it in this patch.
The command git cat-file can now use the mailmap mechanism to replace
- idents with their canonical versions for commit and tag objects. There
- are several options like `-s`, `--batch`, `--batch-check` and
- `--batch-command` that can be combined with `--use-mailmap`. But the
- documentation for `-s`, `--batch`, `--batch-check` and `--batch-command`
- doesn't say so. This patch fixes that documentation.
+ idents with canonical versions for commit and tag objects. There are
+ several options like `--batch`, `--batch-check` and `--batch-command`
+ that can be combined with `--use-mailmap`. But the documentation for
+ `--batch`, `--batch-check` and `--batch-command` doesn't say so. This
+ patch fixes that documentation.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: John Cai <johncai86@gmail.com>
Helped-by: Taylor Blau <me@ttaylorr.com>
+ Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Siddharth Asthana <siddharthasthana31@gmail.com>
## Documentation/git-cat-file.txt ##
@@ Documentation/git-cat-file.txt: OPTIONS
-
- -s::
- Instead of the content, show the object size identified by
-- `<object>`.
-+ `<object>`. If used with `--use-mailmap` option, will show
-+ the size of updated object after replacing idents using the
-+ mailmap mechanism.
-
- -e::
- Exit with zero status if `<object>` exists and is a valid
-@@ Documentation/git-cat-file.txt: OPTIONS
--batch::
--batch=<format>::
Print object information and contents for each object provided
@@ Documentation/git-cat-file.txt: OPTIONS
+
`--batch-command` recognizes the following commands:
+
+
+ ## builtin/cat-file.c ##
+@@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,
+ if (!data->skip_object_info) {
+ int ret;
+
++ if (use_mailmap)
++ data->info.typep = &data->type;
++
+ if (pack)
+ ret = packed_object_info(the_repository, pack, offset,
+ &data->info);
+@@ builtin/cat-file.c: static void batch_object_write(const char *obj_name,
+ fflush(stdout);
+ return;
+ }
++
++ if (use_mailmap && (data->type == OBJ_COMMIT || data->type == OBJ_TAG)) {
++ size_t s = data->size;
++ char *buf = NULL;
++
++ buf = read_object_file(&data->oid, &data->type, &data->size);
++ buf = replace_idents_using_mailmap(buf, &s);
++ data->size = cast_size_t_to_ulong(s);
++
++ free(buf);
++ }
+ }
+
+ strbuf_reset(scratch);
+
+ ## t/t4203-mailmap.sh ##
+@@ t/t4203-mailmap.sh: test_expect_success 'git cat-file -s returns correct size with --use-mailmap for
+ test_cmp expect actual
+ '
+
++test_expect_success 'git cat-file --batch-check returns correct size with --use-mailmap' '
++ test_when_finished "rm .mailmap" &&
++ cat >.mailmap <<-\EOF &&
++ C O Mitter <committer@example.com> Orig <orig@example.com>
++ EOF
++ commit_size=`git cat-file commit HEAD | wc -c` &&
++ commit_sha=`git log --pretty=format:'%H' -n 1` &&
++ echo "$commit_sha commit $commit_size" >expect &&
++ commit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&
++ echo "$commit_sha commit $commit_size" >>expect &&
++ echo "HEAD" >in &&
++ git cat-file --batch-check <in >actual &&
++ git cat-file --use-mailmap --batch-check <in >>actual &&
++ test_cmp expect actual
++'
++
++test_expect_success 'git cat-file --batch-command returns correct size with --use-mailmap' '
++ test_when_finished "rm .mailmap" &&
++ cat >.mailmap <<-\EOF &&
++ C O Mitter <committer@example.com> Orig <orig@example.com>
++ EOF
++ commit_size=`git cat-file commit HEAD | wc -c` &&
++ commit_sha=`git log --pretty=format:'%H' -n 1` &&
++ echo "$commit_sha commit $commit_size" >expect &&
++ commit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&
++ echo "$commit_sha commit $commit_size" >>expect &&
++ echo "info HEAD" >in &&
++ git cat-file --batch-command <in >actual &&
++ git cat-file --use-mailmap --batch-command <in >>actual &&
++ test_cmp expect actual
++'
++
+ test_done
--
2.38.1.438.gd2340c8913
next prev parent reply other threads:[~2022-11-20 7:49 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-16 20:59 [PATCH 0/3] Add mailmap mechanism in --batch-check options Siddharth Asthana
2022-09-16 20:59 ` [PATCH 1/3] doc/cat-file: allow --use-mailmap for --batch options Siddharth Asthana
2022-09-16 22:02 ` Junio C Hamano
2022-09-16 20:59 ` [PATCH 2/3] cat-file: add mailmap support to -s option Siddharth Asthana
2022-09-16 22:22 ` Junio C Hamano
2022-09-16 20:59 ` [PATCH 3/3] cat-file: add mailmap support to --batch-check option Siddharth Asthana
2022-09-16 22:35 ` Junio C Hamano
2022-09-26 10:53 ` [PATCH v2 0/2] Add mailmap mechanism in cat-file options Siddharth Asthana
2022-09-26 10:53 ` [PATCH v2 1/2] cat-file: add mailmap support to -s option Siddharth Asthana
2022-09-26 13:16 ` Ævar Arnfjörð Bjarmason
2022-09-26 13:25 ` Ævar Arnfjörð Bjarmason
2022-09-26 10:53 ` [PATCH v2 2/2] cat-file: add mailmap support to --batch-check option Siddharth Asthana
2022-10-29 10:24 ` [PATCH v3 0/2] Add mailmap mechanism in cat-file options Siddharth Asthana
2022-10-29 10:24 ` [PATCH v3 1/2] cat-file: add mailmap support to -s option Siddharth Asthana
2022-10-31 11:49 ` Christian Couder
2022-10-29 10:24 ` [PATCH v3 2/2] cat-file: add mailmap support to --batch-check option Siddharth Asthana
2022-10-31 11:43 ` Christian Couder
2022-10-29 18:00 ` [PATCH v3 0/2] Add mailmap mechanism in cat-file options Taylor Blau
2022-11-13 21:28 ` [PATCH v4 0/3] " Siddharth Asthana
2022-11-13 21:28 ` [PATCH v4 1/3] cat-file: add mailmap support to -s option Siddharth Asthana
2022-11-13 21:28 ` [PATCH v4 2/3] cat-file: add mailmap support to --batch-check option Siddharth Asthana
2022-11-15 21:40 ` Taylor Blau
2022-11-13 21:28 ` [PATCH v4 3/3] doc/cat-file: allow --use-mailmap for --batch options Siddharth Asthana
2022-11-14 17:48 ` [PATCH v4 0/3] Add mailmap mechanism in cat-file options Christian Couder
2022-11-14 22:30 ` Taylor Blau
2022-11-20 7:42 ` Siddharth Asthana
2022-11-20 7:48 ` Siddharth Asthana [this message]
2022-11-20 7:48 ` [PATCH v5 1/2] cat-file: add mailmap support to -s option Siddharth Asthana
2022-11-21 7:27 ` Junio C Hamano
2022-11-21 9:40 ` Christian Couder
2022-11-21 9:45 ` Junio C Hamano
2022-11-21 11:27 ` Ævar Arnfjörð Bjarmason
2022-11-20 7:48 ` [PATCH v5 2/2] cat-file: add mailmap support to --batch-check option Siddharth Asthana
2022-11-21 7:38 ` Junio C Hamano
2022-11-30 9:19 ` Junio C Hamano
2022-12-01 15:55 ` [PATCH v6 0/2] Add mailmap mechanism in cat-file options Siddharth Asthana
2022-12-01 15:55 ` [PATCH v6 1/2] cat-file: add mailmap support to -s option Siddharth Asthana
2022-12-01 15:55 ` [PATCH v6 2/2] cat-file: add mailmap support to --batch-check option Siddharth Asthana
2022-12-14 11:27 ` Ævar Arnfjörð Bjarmason
2022-12-14 14:04 ` Christian Couder
2022-12-20 6:01 ` [PATCH v7 0/2] Add mailmap mechanism in cat-file options Siddharth Asthana
2022-12-20 6:01 ` [PATCH v7 1/2] cat-file: add mailmap support to -s option Siddharth Asthana
2022-12-20 6:01 ` [PATCH v7 2/2] cat-file: add mailmap support to --batch-check option Siddharth Asthana
2022-12-20 13:02 ` [PATCH v7 0/2] Add mailmap mechanism in cat-file options Ævar Arnfjörð Bjarmason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221120074852.121346-1-siddharthasthana31@gmail.com \
--to=siddharthasthana31@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=avarab@gmail.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johncai86@gmail.com \
--cc=me@ttaylorr.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).