{"thread":{"id":"61705","subject":"[PATCH] t0613: mark as leak-free","startedAt":"2024-06-30T06:46:43Z","lastAt":"2024-07-24T06:45:36Z","messageCount":11,"participants":["Rubén Justo","Jeff King","Eric Sunshine","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"497852","messageId":"23d41343-54fd-46c6-9d78-369e8009fa0b@gmail.com","threadId":"61705","inReplyTo":null,"subject":"[PATCH] t0613: mark as leak-free","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-06-30T06:46:38Z","receivedAt":"2024-06-30T06:46:43Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"We can mark t0613 as leak-free:\n\n    $ make test SANITIZE=leak GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true T=t0613-reftable-write-options.sh\n    [...]\n    *** t0613-reftable-write-options.sh ***\n    in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true\n    ok 1 - default write options\n    ok 2 - disabled reflog writes no log blocks\n    ok 3 - many refs results in multiple blocks\n    ok 4 - tiny block size leads to error\n    ok 5 - small block size leads to multiple ref blocks\n    ok 6 - small block size fails with large reflog message\n    ok 7 - block size exceeding maximum supported size\n    ok 8 - restart interval at every single record\n    ok 9 - restart interval exceeding maximum supported interval\n    ok 10 - object index gets written by default with ref index\n    ok 11 - object index can be disabled\n    # passed all 11 test(s)\n    1..11\n    # faking up non-zero exit with --invert-exit-code\n    make[2]: *** [Makefile:75: t0613-reftable-write-options.sh] Error 1\n\nDo it.\n\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n\nI'm not sure why this simple change has fallen through the cracks.\nTherefore, it's possible that I'm missing something.\n\nI'd appreciate if someone could double-check.\n\nThanks.\n\n t/t0613-reftable-write-options.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t0613-reftable-write-options.sh b/t/t0613-reftable-write-options.sh\nindex e2708e11d5..b1c6c97524 100755\n--- a/t/t0613-reftable-write-options.sh\n+++ b/t/t0613-reftable-write-options.sh\n@@ -16,6 +16,7 @@ export GIT_TEST_DEFAULT_HASH\n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n test_expect_success 'default write options' '\n-- \n2.45.1\n"},{"id":"497875","messageId":"20240701035759.GF610406@coredump.intra.peff.net","threadId":"61705","inReplyTo":"23d41343-54fd-46c6-9d78-369e8009fa0b@gmail.com","subject":"Re: [PATCH] t0613: mark as leak-free","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-01T03:57:59Z","receivedAt":"2024-07-01T03:58:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 30, 2024 at 08:46:38AM +0200, Rubén Justo wrote:\n\n> We can mark t0613 as leak-free:\n> [...]\n> I'm not sure why this simple change has fallen through the cracks.\n> Therefore, it's possible that I'm missing something.\n> \n> I'd appreciate if someone could double-check.\n\nI'd noticed it, too, while doing recent leak fixes. But since Patrick\nhas been working on leaks and is the go-to person for reftables, I\nassumed he had already seen it and there was something clever going on. ;)\n\nI also get a passing result from t0612 (and I do have JGit available, so\nit actually runs the tests).\n\nI also get funny results from t4255, but I think we can ignore them.\nIt's known breakages vanishing, which I guess is just some sub-program\nreturning failure due to a leak and changing the test results.\n\nSo anyway, this patch looks good to me, but probably we could squash\nt0612 into it, as well.\n\n-Peff\n"},{"id":"497919","messageId":"7ef69875-b18f-4ccb-be83-e994315636bd@gmail.com","threadId":"61705","inReplyTo":"20240701035759.GF610406@coredump.intra.peff.net","subject":"Re: [PATCH] t0613: mark as leak-free","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-01T19:35:56Z","receivedAt":"2024-07-01T19:35:59Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Sun, Jun 30, 2024 at 11:57:59PM -0400, Jeff King wrote:\n> On Sun, Jun 30, 2024 at 08:46:38AM +0200, Rubén Justo wrote:\n> \n> > We can mark t0613 as leak-free:\n> > [...]\n> > I'm not sure why this simple change has fallen through the cracks.\n> > Therefore, it's possible that I'm missing something.\n> > \n> > I'd appreciate if someone could double-check.\n> \n> I'd noticed it, too, while doing recent leak fixes. But since Patrick\n> has been working on leaks and is the go-to person for reftables, I\n> assumed he had already seen it and there was something clever going on. ;)\n> \n> I also get a passing result from t0612 (and I do have JGit available, so\n> it actually runs the tests).\n\nI have no idea how JGit works, and I didn't have it installed either. \nBut after a quick test, I can confirm that t0612 can also be marked as\nleak-free.\n\nI'll respond to this message shortly with a patch to fix that.\n\n> \n> I also get funny results from t4255, but I think we can ignore them.\n> It's known breakages vanishing, which I guess is just some sub-program\n> returning failure due to a leak and changing the test results.\n> \n> So anyway, this patch looks good to me, but probably we could squash\n> t0612 into it, as well.\n> \n> -Peff\n\nThank you!\n"},{"id":"497920","messageId":"d244fb44-0bd2-4416-b24c-0a93835b75a4@gmail.com","threadId":"61705","inReplyTo":"7ef69875-b18f-4ccb-be83-e994315636bd@gmail.com","subject":"t0612: mark as leak-free","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-01T19:38:43Z","receivedAt":"2024-07-01T19:38:46Z","isPatch":false,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"A quick test tell us that t0612 does not trigger any leak:\n\n    $ make SANITIZE=leak test GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true GIT_TEST_OPTS=-i T=t0612-reftable-jgit-compatibility.sh\n    [...]\n    *** t0612-reftable-jgit-compatibility.sh ***\n    in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true\n    ok 1 - CGit repository can be read by JGit\n    ok 2 - JGit repository can be read by CGit\n    ok 3 - mixed writes from JGit and CGit\n    ok 4 - JGit can read multi-level index\n    # passed all 4 test(s)\n    1..4\n    # faking up non-zero exit with --invert-exit-code\n    make[2]: *** [Makefile:75: t0612-reftable-jgit-compatibility.sh] Error 1\n\nLet's mark it as leak-free to silence the machinery activated by\n`GIT_TEST_PASSING_SANITIZE_LEAK=check`.\n\nReported-by: Jeff King <peff@peff.net>\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n t/t0612-reftable-jgit-compatibility.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh\nindex d0d7e80b49..84922153ab 100755\n--- a/t/t0612-reftable-jgit-compatibility.sh\n+++ b/t/t0612-reftable-jgit-compatibility.sh\n@@ -11,6 +11,7 @@ export GIT_TEST_DEFAULT_REF_FORMAT\n GIT_TEST_SPLIT_INDEX=0\n export GIT_TEST_SPLIT_INDEX\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n if ! test_have_prereq JGIT\n-- \n2.45.1\n"},{"id":"497921","messageId":"CAPig+cSG8xqfVcZmejRKKDww-+29fkyRZ=yn6NizcfTm8FzouA@mail.gmail.com","threadId":"61705","inReplyTo":"d244fb44-0bd2-4416-b24c-0a93835b75a4@gmail.com","subject":"Re: t0612: mark as leak-free","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-01T19:40:55Z","receivedAt":"2024-07-01T19:41:07Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jul 1, 2024 at 3:38 PM Rubén Justo <rjusto@gmail.com> wrote:\n> A quick test tell us that t0612 does not trigger any leak:\n\ns/tell/tells/\n\n>     $ make SANITIZE=leak test GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true GIT_TEST_OPTS=-i T=t0612-reftable-jgit-compatibility.sh\n>     [...]\n>\n> Let's mark it as leak-free to silence the machinery activated by\n> `GIT_TEST_PASSING_SANITIZE_LEAK=check`.\n>\n> Reported-by: Jeff King <peff@peff.net>\n> Signed-off-by: Rubén Justo <rjusto@gmail.com>\n"},{"id":"497922","messageId":"86427b9e-9574-4e61-890a-691779a8da82@gmail.com","threadId":"61705","inReplyTo":"7ef69875-b18f-4ccb-be83-e994315636bd@gmail.com","subject":"[PATCH] t0612: mark as leak-free","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-01T19:44:18Z","receivedAt":"2024-07-01T19:44:21Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"A quick test tells us that t0612 does not trigger any leak:\n\n    $ make SANITIZE=leak test GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true GIT_TEST_OPTS=-i T=t0612-reftable-jgit-compatibility.sh\n    [...]\n    *** t0612-reftable-jgit-compatibility.sh ***\n    in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true\n    ok 1 - CGit repository can be read by JGit\n    ok 2 - JGit repository can be read by CGit\n    ok 3 - mixed writes from JGit and CGit\n    ok 4 - JGit can read multi-level index\n    # passed all 4 test(s)\n    1..4\n    # faking up non-zero exit with --invert-exit-code\n    make[2]: *** [Makefile:75: t0612-reftable-jgit-compatibility.sh] Error 1\n\nLet's mark it as leak-free to silence the machinery activated by\n`GIT_TEST_PASSING_SANITIZE_LEAK=check`.\n\nReported-by: Jeff King <peff@peff.net>\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n\nNow with the correct subject and the correction to the error pointed out\nby Eric.\n\nThanks.\n\n\n t/t0612-reftable-jgit-compatibility.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/t0612-reftable-jgit-compatibility.sh b/t/t0612-reftable-jgit-compatibility.sh\nindex d0d7e80b49..84922153ab 100755\n--- a/t/t0612-reftable-jgit-compatibility.sh\n+++ b/t/t0612-reftable-jgit-compatibility.sh\n@@ -11,6 +11,7 @@ export GIT_TEST_DEFAULT_REF_FORMAT\n GIT_TEST_SPLIT_INDEX=0\n export GIT_TEST_SPLIT_INDEX\n \n+TEST_PASSES_SANITIZE_LEAK=true\n . ./test-lib.sh\n \n if ! test_have_prereq JGIT\n-- \n2.45.1\n"},{"id":"499050","messageId":"Zp4gILfskdpc6RUk@tanuki","threadId":"61705","inReplyTo":"20240701035759.GF610406@coredump.intra.peff.net","subject":"Re: [PATCH] t0613: mark as leak-free","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-22T09:02:24Z","receivedAt":"2024-07-22T09:02:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Jun 30, 2024 at 11:57:59PM -0400, Jeff King wrote:\n> On Sun, Jun 30, 2024 at 08:46:38AM +0200, Rubén Justo wrote:\n> \n> > We can mark t0613 as leak-free:\n> > [...]\n> > I'm not sure why this simple change has fallen through the cracks.\n> > Therefore, it's possible that I'm missing something.\n> > \n> > I'd appreciate if someone could double-check.\n> \n> I'd noticed it, too, while doing recent leak fixes. But since Patrick\n> has been working on leaks and is the go-to person for reftables, I\n> assumed he had already seen it and there was something clever going on. ;)\n\nNah, you assumed too much :) I just forgot to mark this as leak-free and\nthe topic crossed with my memory-leak-fix topics, so I didn't yet find\nthe time to fix it.\n\nIt does highlight an issue though: I think memory leak checks should be\nopt-out rather than opt-in by now. Most of our tests run just fine with\nthe memory leak checker enabled, and that's also where we want to be\nheaded. So making tests opt-out would likely raise more eyebrows when\nnew tests are being added that explicitly opt out.\n\nThe only reason I didn't send a patch like this yet is that it would of\ncourse create quite a bit of churn in our tests. I'm not sure whether\nthat churn is really worth it, or whether we should instead just\ncontinue fixing tests until we can get rid of this marking altogether\nbecause all of our tests pass.\n\nPatrick\n"},{"id":"499200","messageId":"20240723210339.GD6779@coredump.intra.peff.net","threadId":"61705","inReplyTo":"Zp4gILfskdpc6RUk@tanuki","subject":"Re: [PATCH] t0613: mark as leak-free","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-23T21:03:39Z","receivedAt":"2024-07-23T21:03:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 22, 2024 at 11:02:24AM +0200, Patrick Steinhardt wrote:\n\n> > I'd noticed it, too, while doing recent leak fixes. But since Patrick\n> > has been working on leaks and is the go-to person for reftables, I\n> > assumed he had already seen it and there was something clever going on. ;)\n> \n> Nah, you assumed too much :) I just forgot to mark this as leak-free and\n> the topic crossed with my memory-leak-fix topics, so I didn't yet find\n> the time to fix it.\n\nAh, OK. :) Then I think we did the right thing in your absence.\n\n> It does highlight an issue though: I think memory leak checks should be\n> opt-out rather than opt-in by now. Most of our tests run just fine with\n> the memory leak checker enabled, and that's also where we want to be\n> headed. So making tests opt-out would likely raise more eyebrows when\n> new tests are being added that explicitly opt out.\n> \n> The only reason I didn't send a patch like this yet is that it would of\n> course create quite a bit of churn in our tests. I'm not sure whether\n> that churn is really worth it, or whether we should instead just\n> continue fixing tests until we can get rid of this marking altogether\n> because all of our tests pass.\n\nI could see arguments in both directions. I'd worry that by switching\nthe default to \"assume leak free\", it may end up with misalignment\nbetween who introduces the bug and who deals with the fallout.\n\nRight now, if I introduce a test that is leak free but don't mark it,\nsomebody working on leaks later runs in check mode and says \"yay, it\npasses. Let's mark it\". It becomes their task to do, but it's an\neasy-ish task.\n\nIf we go the other way, then a new test that _does_ leak means that\neither:\n\n  1. The original author notices the CI leaks job failing.\n\n     a. They introduced the leak, and it was caught early. Yay!\n\n     b. The leak is in some random part of Git that their test happened\n\tto trigger. Now they spend effort proving it was not their fault\n\tbefore they annotate the test with \"does not pass leak\".\n\n  2. The original author does not notice. Somebody notices later when\n     doing leak-checking (or I guess just running their own CI, if we\n     are hitting these by default). Now they are stuck with doing (1a)\n     or (1b) themselves, even though they do not care about the original\n     topic.\n\nSo I dunno. If we think people are paying attention to CI on their\ntopics, and we think that we are close enough to leak-free that (1b)\nwon't come up a lot, it might make sense. I'm not quite sure we're there\nyet on the latter, but it's mostly gut feeling (and I know things have\ngotten a bit better recently, too).\n\nI guess the only way to know is to try it, but as you noted, it is a bit\nof churn to switch between the two states.\n\n-Peff\n"},{"id":"499210","messageId":"4b1391d5-89c2-41b1-b1de-e1bd26b9f10e@gmail.com","threadId":"61705","inReplyTo":"20240723210339.GD6779@coredump.intra.peff.net","subject":"Re* [PATCH] t0613: mark as leak-free","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-23T23:07:23Z","receivedAt":"2024-07-23T23:07:26Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Tue, Jul 23, 2024 at 05:03:39PM -0400, Jeff King wrote:\n> On Mon, Jul 22, 2024 at 11:02:24AM +0200, Patrick Steinhardt wrote:\n> \n> > > I'd noticed it, too, while doing recent leak fixes. But since Patrick\n> > > has been working on leaks and is the go-to person for reftables, I\n> > > assumed he had already seen it and there was something clever going on. ;)\n> > \n> > Nah, you assumed too much :) I just forgot to mark this as leak-free and\n> > the topic crossed with my memory-leak-fix topics, so I didn't yet find\n> > the time to fix it.\n> \n> Ah, OK. :) Then I think we did the right thing in your absence.\n\n:)\n\n> \n> > It does highlight an issue though: I think memory leak checks should be\n> > opt-out rather than opt-in by now. Most of our tests run just fine with\n> > the memory leak checker enabled, and that's also where we want to be\n> > headed. So making tests opt-out would likely raise more eyebrows when\n> > new tests are being added that explicitly opt out.\n> > \n> > The only reason I didn't send a patch like this yet is that it would of\n> > course create quite a bit of churn in our tests. I'm not sure whether\n> > that churn is really worth it, or whether we should instead just\n> > continue fixing tests until we can get rid of this marking altogether\n> > because all of our tests pass.\n> \n> I could see arguments in both directions. I'd worry that by switching\n> the default to \"assume leak free\", it may end up with misalignment\n> between who introduces the bug and who deals with the fallout.\n> \n> Right now, if I introduce a test that is leak free but don't mark it,\n> somebody working on leaks later runs in check mode and says \"yay, it\n> passes. Let's mark it\". It becomes their task to do, but it's an\n> easy-ish task.\n> \n> If we go the other way, then a new test that _does_ leak means that\n> either:\n> \n>   1. The original author notices the CI leaks job failing.\n> \n>      a. They introduced the leak, and it was caught early. Yay!\n> \n>      b. The leak is in some random part of Git that their test happened\n> \tto trigger. Now they spend effort proving it was not their fault\n> \tbefore they annotate the test with \"does not pass leak\".\n> \n>   2. The original author does not notice. Somebody notices later when\n>      doing leak-checking (or I guess just running their own CI, if we\n>      are hitting these by default). Now they are stuck with doing (1a)\n>      or (1b) themselves, even though they do not care about the original\n>      topic.\n> \n> So I dunno. If we think people are paying attention to CI on their\n> topics, and we think that we are close enough to leak-free that (1b)\n> won't come up a lot, it might make sense. I'm not quite sure we're there\n> yet on the latter, but it's mostly gut feeling (and I know things have\n> gotten a bit better recently, too).\n\nI don't know either.  Maybe it seems a bit early still considering the\nnumbers we have: \n\n   $ git grep -l PASSES_SANITIZE_LEAK=true t/t[0-9][0-9][0-9][0-9]-*.sh | wc -l\n   678\n   $ git grep -L PASSES_SANITIZE_LEAK=true t/t[0-9][0-9][0-9][0-9]-*.sh | wc -l\n   329\n\nBTW, perhaps we want to do this:\n\n----- >8 --------- >8 --------- >8 --------- >8 ----\nSubject: [PATCH] t: do not mark tests as no-leak-free\n\nThe mark TEST_PASSES_SANITIZE_LEAK=false and the absence of it mean the\nsame thing: the test triggers leaks.  However, explicitly declaring that\nthe test leak, with TEST_PASSES_SANITIZE_LEAK=false, can suggest there\nis some special consideration for it, which is not the case.\n\nLooking back at the history of the tests we're modifying in this patch,\nwe see that both were marked as \"leak-free\", but later started\ntriggering leaks and had to be re-marked accordingly as \"no-leak-free\".\nInstead of removing the mark entirely, it was changed to \"false\".\n\nIt doesn't seem to have much historical value to keep the mark for this\nreason, and removing it reduces the confusion that it has some special\nmeaning.\n\nDo it so.\n\nSigned-off-by: Rubén Justo <rjusto@gmail.com>\n---\n t/t0210-trace2-normal.sh | 1 -\n t/t0211-trace2-perf.sh   | 1 -\n 2 files changed, 2 deletions(-)\n\ndiff --git a/t/t0210-trace2-normal.sh b/t/t0210-trace2-normal.sh\nindex c312657a12..eff9a59dbd 100755\n--- a/t/t0210-trace2-normal.sh\n+++ b/t/t0210-trace2-normal.sh\n@@ -2,7 +2,6 @@\n \n test_description='test trace2 facility (normal target)'\n \n-TEST_PASSES_SANITIZE_LEAK=false\n . ./test-lib.sh\n \n # Turn off any inherited trace2 settings for this test.\ndiff --git a/t/t0211-trace2-perf.sh b/t/t0211-trace2-perf.sh\nindex 070fe7a5da..b9421a64a7 100755\n--- a/t/t0211-trace2-perf.sh\n+++ b/t/t0211-trace2-perf.sh\n@@ -2,7 +2,6 @@\n \n test_description='test trace2 facility (perf target)'\n \n-TEST_PASSES_SANITIZE_LEAK=false\n . ./test-lib.sh\n \n # Turn off any inherited trace2 settings for this test.\n\n"},{"id":"499220","messageId":"ZqCOEGfTdOSAL60w@tanuki","threadId":"61705","inReplyTo":"4b1391d5-89c2-41b1-b1de-e1bd26b9f10e@gmail.com","subject":"Re: Re* [PATCH] t0613: mark as leak-free","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-07-24T05:16:00Z","receivedAt":"2024-07-24T05:16:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jul 24, 2024 at 01:07:23AM +0200, Rubén Justo wrote:\n> On Tue, Jul 23, 2024 at 05:03:39PM -0400, Jeff King wrote:\n> > On Mon, Jul 22, 2024 at 11:02:24AM +0200, Patrick Steinhardt wrote:\n> > So I dunno. If we think people are paying attention to CI on their\n> > topics, and we think that we are close enough to leak-free that (1b)\n> > won't come up a lot, it might make sense. I'm not quite sure we're there\n> > yet on the latter, but it's mostly gut feeling (and I know things have\n> > gotten a bit better recently, too).\n> \n> I don't know either.  Maybe it seems a bit early still considering the\n> numbers we have: \n> \n>    $ git grep -l PASSES_SANITIZE_LEAK=true t/t[0-9][0-9][0-9][0-9]-*.sh | wc -l\n>    678\n>    $ git grep -L PASSES_SANITIZE_LEAK=true t/t[0-9][0-9][0-9][0-9]-*.sh | wc -l\n>    329\n\nThese numbers aren't quite right -- you have to filter out most of the\ntests that include \"lib-git-svn.sh\", which reverses the schema and makes\nleak checks opt-out (?!). That brings me to the following hacky numbers:\n\n    $ grep -l TEST_PASSES_SANITIZE_LEAK=true t[0-9][0-9][0-9][0-9]-*.sh | grep -v svn | wc -l\n    678\n    $ grep -L TEST_PASSES_SANITIZE_LEAK=true t[0-9][0-9][0-9][0-9]-*.sh | grep -v svn | wc -l\n    261\n\nI've got two local topic branches pending that reduce the number of\nfailing tests even further. One is the Perforce series I've sent out\nyesterday. And then another random set of leak fixes. Which together\nbring us to:\n\n    $ grep -l TEST_PASSES_SANITIZE_LEAK=true t[0-9][0-9][0-9][0-9]-*.sh | grep -v svn | wc -l\n    749\n    $ grep -L TEST_PASSES_SANITIZE_LEAK=true t[0-9][0-9][0-9][0-9]-*.sh | grep -v svn | wc -l\n    190\n\nSo considering that it's currently still rather easy to make progress,\nI'd vote for keeping things as-is and wait for another couple of series\nto land before switching to opt-out.\n\nPatrick\n"},{"id":"499222","messageId":"d3f248de-c424-4fd9-bb54-2314291603e3@gmail.com","threadId":"61705","inReplyTo":"ZqCOEGfTdOSAL60w@tanuki","subject":"Re: Re* [PATCH] t0613: mark as leak-free","fromName":"Rubén Justo","fromEmail":"rjusto@gmail.com","sentAt":"2024-07-24T06:45:33Z","receivedAt":"2024-07-24T06:45:36Z","isPatch":true,"sender":{"key":"rjusto@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5685487?v=4"},"body":"On Wed, Jul 24, 2024 at 07:16:00AM +0200, Patrick Steinhardt wrote:\n> On Wed, Jul 24, 2024 at 01:07:23AM +0200, Rubén Justo wrote:\n> > On Tue, Jul 23, 2024 at 05:03:39PM -0400, Jeff King wrote:\n> > > On Mon, Jul 22, 2024 at 11:02:24AM +0200, Patrick Steinhardt wrote:\n> > > So I dunno. If we think people are paying attention to CI on their\n> > > topics, and we think that we are close enough to leak-free that (1b)\n> > > won't come up a lot, it might make sense. I'm not quite sure we're there\n> > > yet on the latter, but it's mostly gut feeling (and I know things have\n> > > gotten a bit better recently, too).\n> > \n> > I don't know either.  Maybe it seems a bit early still considering the\n> > numbers we have: \n> > \n> >    $ git grep -l PASSES_SANITIZE_LEAK=true t/t[0-9][0-9][0-9][0-9]-*.sh | wc -l\n> >    678\n> >    $ git grep -L PASSES_SANITIZE_LEAK=true t/t[0-9][0-9][0-9][0-9]-*.sh | wc -l\n> >    329\n> \n> These numbers aren't quite right -- you have to filter out most of the\n> tests that include \"lib-git-svn.sh\", which reverses the schema and makes\n> leak checks opt-out (?!).\n\nYou are right.  \n\n> That brings me to the following hacky numbers:\n> \n>     $ grep -l TEST_PASSES_SANITIZE_LEAK=true t[0-9][0-9][0-9][0-9]-*.sh | grep -v svn | wc -l\n>     678\n>     $ grep -L TEST_PASSES_SANITIZE_LEAK=true t[0-9][0-9][0-9][0-9]-*.sh | grep -v svn | wc -l\n>     261\n\nOut of curiosity, I ran this:\n\n    $ echo $((329 - $(git grep -l lib-git-svn.sh t/t[0-9][0-9][0-9][0-9]-*.sh | wc -l)))\n    260\n\nwhich points to t9150-svk-mergetickets.sh.\n"}]}