Re: [PATCH v5 2/2] cat-file: add mailmap support to --batch-check option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 21, 2022, 07:38 UTC
- Message-ID
- <xmqqbkp0wyd8.fsf@gitster.g>
- In-Reply-To
- <20221120074852.121346-3-siddharthasthana31@gmail.com>
Siddharth Asthana <siddharthasthana31@gmail.com> writes:
Show 14 quoted lines
> diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh > index 87b77fc5c9..21ba6bc278 100755 > --- a/t/t4203-mailmap.sh > +++ b/t/t4203-mailmap.sh > @@ -1047,4 +1047,36 @@ 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` &&
We prefer $(command substitution) over `command substitution`.
When "cat-file" segfaults and dumps core, having it upstream of a pipe would mean its crashing will be hidden.
Note that some implementations of "wc" pads its output with SP. The implication will be seen in a few paragraphs below.
> + commit_sha=`git log --pretty=format:'%H' -n 1` &&
These single quotes are not doing what you think they are doing. The body of the test is inside a pair of single quotes, and the one after format: makes you step outside the single quote, take two bytes %H literally, and the other single quote opens a new singly quoted string segment. Which is not wrong per-se, because there is no special meaning attached to the sequence %H in the shell language, but then you'd be better off writing format:%H without any quotes, as that is more direct way to write what you are writing.
Also, --pretty=format:<something> is almost always a mistake. Unless you have a good reason to use it, you'd most likely want to use --format=<something> instead.
In any case, don't abuse "log" when you mean
commit_object_name=$(git rev-parse HEAD) &&
> + echo "$commit_sha commit $commit_size" >expect &&
As $commit_size here may have extra and unwanted SP before it, this may break with the implementation of "wc" on certain platforms. In this particular instance, losing quoting, i.e.
echo $commit_sha commit $commit_size >expect
may be a good workaround.
> + commit_size=`git cat-file --use-mailmap commit HEAD | wc -c` &&
Exactly the same set of comments as above apply to this side, too.
Show 24 quoted lines
> + 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