git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, Jerry Zhang <jerry@skydio.com>
Subject: Re: [PATCH V5 2/2] git-apply: add --allow-empty flag
Date: Wed, 15 Dec 2021 17:40:06 -0800	[thread overview]
Message-ID: <xmqqee6dz5s9.fsf@gitster.g> (raw)
In-Reply-To: <20211213220327.16042-2-jerry@skydio.com> (Jerry Zhang's message of "Mon, 13 Dec 2021 14:03:27 -0800")

Jerry Zhang <jerry@skydio.com> writes:

>  t/t4126-apply-empty.sh      | 22 ++++++++++++++++++----
>  4 files changed, 30 insertions(+), 7 deletions(-)
> ...
> diff --git a/t/t4126-apply-empty.sh b/t/t4126-apply-empty.sh
> index ceb6a79fe0..949e284d14 100755
> --- a/t/t4126-apply-empty.sh
> +++ b/t/t4126-apply-empty.sh
> @@ -7,10 +7,12 @@ test_description='apply empty'
>  test_expect_success setup '
>  	>empty &&
>  	git add empty &&
>  	test_tick &&
>  	git commit -m initial &&
> +	git commit --allow-empty -m "empty commit" &&
> +	git format-patch --always HEAD~ >empty.patch &&
>  	for i in a b c d e

When merged with anything that has ab/mark-leak-free-tests-even-more
topic, this will start breaking the tests, as it is my understanding
that "git log" family hasn't been audited and converted for leak
sanitizer.

This is sort of water under the bridge, as the other topic is
already in 'master', but come to think of it, the strategy we used
with TEST_PASSES_SANITIZE_LEAK variable was misguided.  

If the git subcommands a single test script uses were only the
subcommands that the test script wants to test, the approach to
default to "this subcommand has not been made leak sanitizer clean",
and then to add TEST_PASSES mark as we sanitize the subcommand makes
perfect sense, but most test scripts need to run git subcommands
that are *not* the focus of the test---they run them only to prepare
the scene in which the subcommands being tested are excersized.  In
such a situation (which is exactly what happens here), marking that
"right now, all the tested subcommands and also all the subcommands
that happen to be exercised to prepare fixture are clean" would
force us to flip-flop with "now we use a subcommand we didn't use in
this script before to prepare the scene, and it is not yet sanitizer
clean, so we need to unmark it", which is not quite ideal, but is
much better than forcing the contributor who is *not* working on making
these subcommands leak-sanitizer-clean to worry about such a breakage.

I am tempted to drop the "TEST_PASSES" bit from this script for now,
but I have to say that the "mark leak-free tests" topic took us in
an awkward place.  We probably want to do something a bit more fine
grained about it.

Thanks.

  reply	other threads:[~2021-12-16  1:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13 22:03 [PATCH V3 1/2] git-apply: add --quiet flag Jerry Zhang
2021-12-13 22:03 ` [PATCH V5 2/2] git-apply: add --allow-empty flag Jerry Zhang
2021-12-16  1:40   ` Junio C Hamano [this message]
2021-12-16 23:11     ` [PATCH] t4204 is not sanitizer clean at all Junio C Hamano
2021-12-17  4:39       ` Ævar Arnfjörð Bjarmason
2021-12-17 20:48         ` Junio C Hamano
2021-12-17 22:23           ` Ævar Arnfjörð Bjarmason
2021-12-16 23:37     ` [PATCH] format-patch: mark rev_info with UNLEAK Junio C Hamano
2021-12-17  4:51     ` [PATCH V5 2/2] git-apply: add --allow-empty flag Ævar Arnfjörð Bjarmason
2021-12-17 20:48       ` Junio C Hamano
2021-12-17 22:28         ` Ævar Arnfjörð Bjarmason
2021-12-17 22:32         ` Junio C Hamano
2021-12-13 22:30 ` [PATCH V3 1/2] git-apply: add --quiet flag Junio C Hamano

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqee6dz5s9.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jerry@skydio.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.
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 NNTP newsgroup(s).