Re: [PATCH V5 2/2] git-apply: add --allow-empty flag
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 16, 2021, 01:40 UTC
- Message-ID
- <xmqqee6dz5s9.fsf@gitster.g>
- In-Reply-To
- <20211213220327.16042-2-jerry@skydio.com>
Jerry Zhang <jerry@skydio.com> writes:
Show 16 quoted lines
> 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.