{"thread":{"id":"63484","subject":"[PATCH] t: remove unexpected SANITIZE_LEAK variables","startedAt":"2025-05-20T14:40:22Z","lastAt":"2025-05-22T09:06:56Z","messageCount":6,"participants":["Karthik Nayak","Justin Tobler","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"518492","messageId":"20250520-kn-remove-unexpected-exported-v1-1-bb60cec57e84@gmail.com","threadId":"63484","inReplyTo":null,"subject":"[PATCH] t: remove unexpected SANITIZE_LEAK variables","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-05-20T14:40:12Z","receivedAt":"2025-05-20T14:40:22Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,\n2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and\n`TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the\nleak checks are enabled by default. However, some newly added tests\ninclude them by mistake. Let's clean this up.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n t/perf/p5313-pack-objects.sh | 3 ---\n t/perf/p5314-name-hash.sh    | 3 ---\n t/t6601-path-walk.sh         | 2 --\n 3 files changed, 8 deletions(-)\n\ndiff --git a/t/perf/p5313-pack-objects.sh b/t/perf/p5313-pack-objects.sh\nindex be5229a0ec..786a2c1c6f 100755\n--- a/t/perf/p5313-pack-objects.sh\n+++ b/t/perf/p5313-pack-objects.sh\n@@ -3,9 +3,6 @@\n test_description='Tests pack performance using bitmaps'\n . ./perf-lib.sh\n \n-GIT_TEST_PASSING_SANITIZE_LEAK=0\n-export GIT_TEST_PASSING_SANITIZE_LEAK\n-\n test_perf_large_repo\n \n test_expect_success 'create rev input' '\ndiff --git a/t/perf/p5314-name-hash.sh b/t/perf/p5314-name-hash.sh\nindex 4ef0ba7711..235cdfc824 100755\n--- a/t/perf/p5314-name-hash.sh\n+++ b/t/perf/p5314-name-hash.sh\n@@ -3,9 +3,6 @@\n test_description='Tests pack performance using bitmaps'\n . ./perf-lib.sh\n \n-GIT_TEST_PASSING_SANITIZE_LEAK=0\n-export GIT_TEST_PASSING_SANITIZE_LEAK\n-\n test_perf_large_repo\n \n test_size 'paths at head' '\ndiff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh\nindex c89b0f1e19..8d187f7279 100755\n--- a/t/t6601-path-walk.sh\n+++ b/t/t6601-path-walk.sh\n@@ -1,7 +1,5 @@\n #!/bin/sh\n \n-TEST_PASSES_SANITIZE_LEAK=true\n-\n test_description='direct path-walk API tests'\n \n . ./test-lib.sh\n\n\n\n"},{"id":"518534","messageId":"dw57mhszlyk3s4rvitdthpbaoiicjnnraq24e6uxaeu2kze26d@xpiouvxmhowr","threadId":"63484","inReplyTo":"20250520-kn-remove-unexpected-exported-v1-1-bb60cec57e84@gmail.com","subject":"Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-05-20T21:16:02Z","receivedAt":"2025-05-20T21:20:41Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/05/20 04:40PM, Karthik Nayak wrote:\n> As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,\n> 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and\n> `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the\n> leak checks are enabled by default. However, some newly added tests\n> include them by mistake. Let's clean this up.\n\nIndeed, both `GIT_TEST_PASSING_SANITIZE_LEAK` and\n`TEST_PASSES_SANITIZE_LEAK` appear not have any purpose anymore.\nRemoving all remaining instances where they appear makes sense and from\na quick search it looks like this patch got them all. So this looks good\nto me.\n\n-Justin\n\n> \n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  t/perf/p5313-pack-objects.sh | 3 ---\n>  t/perf/p5314-name-hash.sh    | 3 ---\n>  t/t6601-path-walk.sh         | 2 --\n>  3 files changed, 8 deletions(-)\n> \n> diff --git a/t/perf/p5313-pack-objects.sh b/t/perf/p5313-pack-objects.sh\n> index be5229a0ec..786a2c1c6f 100755\n> --- a/t/perf/p5313-pack-objects.sh\n> +++ b/t/perf/p5313-pack-objects.sh\n> @@ -3,9 +3,6 @@\n>  test_description='Tests pack performance using bitmaps'\n>  . ./perf-lib.sh\n>  \n> -GIT_TEST_PASSING_SANITIZE_LEAK=0\n> -export GIT_TEST_PASSING_SANITIZE_LEAK\n> -\n>  test_perf_large_repo\n>  \n>  test_expect_success 'create rev input' '\n> diff --git a/t/perf/p5314-name-hash.sh b/t/perf/p5314-name-hash.sh\n> index 4ef0ba7711..235cdfc824 100755\n> --- a/t/perf/p5314-name-hash.sh\n> +++ b/t/perf/p5314-name-hash.sh\n> @@ -3,9 +3,6 @@\n>  test_description='Tests pack performance using bitmaps'\n>  . ./perf-lib.sh\n>  \n> -GIT_TEST_PASSING_SANITIZE_LEAK=0\n> -export GIT_TEST_PASSING_SANITIZE_LEAK\n> -\n>  test_perf_large_repo\n>  \n>  test_size 'paths at head' '\n> diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh\n> index c89b0f1e19..8d187f7279 100755\n> --- a/t/t6601-path-walk.sh\n> +++ b/t/t6601-path-walk.sh\n> @@ -1,7 +1,5 @@\n>  #!/bin/sh\n>  \n> -TEST_PASSES_SANITIZE_LEAK=true\n> -\n>  test_description='direct path-walk API tests'\n>  \n>  . ./test-lib.sh\n> \n> \n> \n> \n"},{"id":"518536","messageId":"xmqqbjrngci9.fsf@gitster.g","threadId":"63484","inReplyTo":"dw57mhszlyk3s4rvitdthpbaoiicjnnraq24e6uxaeu2kze26d@xpiouvxmhowr","subject":"Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-20T22:09:50Z","receivedAt":"2025-05-20T22:09:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> On 25/05/20 04:40PM, Karthik Nayak wrote:\n>> As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,\n>> 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and\n>> `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the\n>> leak checks are enabled by default. However, some newly added tests\n>> include them by mistake. Let's clean this up.\n>\n> Indeed, both `GIT_TEST_PASSING_SANITIZE_LEAK` and\n> `TEST_PASSES_SANITIZE_LEAK` appear not have any purpose anymore.\n> Removing all remaining instances where they appear makes sense and from\n> a quick search it looks like this patch got them all. So this looks good\n> to me.\n\nThanks, both.\n"},{"id":"518646","messageId":"20250522060626.GB1137854@coredump.intra.peff.net","threadId":"63484","inReplyTo":"20250520-kn-remove-unexpected-exported-v1-1-bb60cec57e84@gmail.com","subject":"Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-22T06:06:26Z","receivedAt":"2025-05-22T06:06:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 20, 2025 at 04:40:12PM +0200, Karthik Nayak wrote:\n\n> As of 1fc7ddf35b (test-lib: unconditionally enable leak checking,\n> 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and\n> `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the\n> leak checks are enabled by default. However, some newly added tests\n> include them by mistake. Let's clean this up.\n\nThanks, I saw these recently while looking at another topic and was\nsurprised. I hadn't yet confirmed that they truly are pointless, so I'm\nglad that you did. :)\n\nAs a side note, we do still use the SANITIZE_LEAK prereq in a few\nplaces, and I believe that it does actually work. It might be nice to\nclean up any leaks in those few spots, though we probably want to keep\nthe prereq around forever (e.g., if you introduce a test which shows off\na leak and then fixes it later). All orthogonal to your patch, though,\nwhich looks good to me.\n\n-Peff\n"},{"id":"518647","messageId":"20250522060933.GA1135565@coredump.intra.peff.net","threadId":"63484","inReplyTo":"20250522060626.GB1137854@coredump.intra.peff.net","subject":"Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-22T06:09:33Z","receivedAt":"2025-05-22T06:09:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 22, 2025 at 02:06:26AM -0400, Jeff King wrote:\n\n> As a side note, we do still use the SANITIZE_LEAK prereq in a few\n> places, and I believe that it does actually work. It might be nice to\n> clean up any leaks in those few spots, though we probably want to keep\n> the prereq around forever (e.g., if you introduce a test which shows off\n> a leak and then fixes it later). All orthogonal to your patch, though,\n> which looks good to me.\n\nAh, nevermind. These are all due to 8415595203 (t5601: work around leak\nsanitizer issue, 2024-11-20). They are leak-free (and the tests pass on\nmy system), but apparently some upstream bug can cause issues.\n\n-Peff\n"},{"id":"518652","messageId":"CAOLa=ZTg8_Hs0Ysu4bhtp9VZxnDEkSx3MvD1dN2yYYw7DqKRLw@mail.gmail.com","threadId":"63484","inReplyTo":"20250522060933.GA1135565@coredump.intra.peff.net","subject":"Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-05-22T09:06:54Z","receivedAt":"2025-05-22T09:06:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, May 22, 2025 at 02:06:26AM -0400, Jeff King wrote:\n>\n>> As a side note, we do still use the SANITIZE_LEAK prereq in a few\n>> places, and I believe that it does actually work. It might be nice to\n>> clean up any leaks in those few spots, though we probably want to keep\n>> the prereq around forever (e.g., if you introduce a test which shows off\n>> a leak and then fixes it later). All orthogonal to your patch, though,\n>> which looks good to me.\n>\n> Ah, nevermind. These are all due to 8415595203 (t5601: work around leak\n> sanitizer issue, 2024-11-20). They are leak-free (and the tests pass on\n> my system), but apparently some upstream bug can cause issues.\n>\n> -Peff\n\nYup. Someday in the future we can cleanup the whole thing. That would be\nnice!\n\n- Karthik\n"}]}