git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH V5 2/2] git-apply: add --allow-empty flag
Date: Fri, 17 Dec 2021 14:32:07 -0800	[thread overview]
Message-ID: <xmqqee6a3lso.fsf@gitster.g> (raw)
In-Reply-To: <xmqqr1ab2c0v.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 17 Dec 2021 12:48:32 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> Surely, I am sympathetic to the intent.  If you are updating "git
> frotz" that is sanitizer-clean, and if you write a new test in a
> test script that happens to be sanitizer-clean, if you introduced a
> new leak to "git frotz", you would appreciate if the CI notices it
> and blocks you.
> ...
> The only time we can sensibly do the "now these are leak-free, and
> we will catch and yell at you when you add a new leak" is when we
> know _all_ git commands are sanitize clean...

There is another scenario where the TEST_PASSES_SANITIZE_LEAK=true
may make sense, actually.  If we declare that from the time we
commit to the approach, until we can mark all the test scripts with
the mark, we will put it the sole priority to squash any and all
leaks, without doing anything else so that we can finish it the
soonest possible.

Then it is probably OK to start at 230 and cover all 940 as fast as
we can.  Because we are effectively closing the tree for anything
but plug-leak changes and adding TEST_PASSES_SANITIZE_LEAK=true line
to more tests, we wouldn't have to worry about introducing new leaks
to existing tests that are marked as already clean---because of the
tree closure, they are more likely to stay clean.  t4126 wouldn't
have gained a new use of format-patch to break it.

But of course, such an approach is not feasible in this project,
where people do not work in lock-step.  That leads to the question I
asked at the end of my previous message.

> Having said that, what would be the next step to help developers to
> avoid introducing new leaks while yelling at them for existing leaks
> they did not introduce and not forbidding them to use git subccommands
> with existing leaks in their tests?
>
> I would prefer an approach that does not force the project to make
> it the highest priority to plug leaks over everything else.

Thanks.

  parent reply	other threads:[~2021-12-17 22:32 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
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 [this message]
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

  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=xmqqee6a3lso.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    /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).