# [PATCH] t: remove unexpected SANITIZE_LEAK variables 6 messages from 2025-05-20 to 2025-05-22. Participants: Karthik Nayak, Justin Tobler, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/63484 ## Karthik Nayak, 2025-05-20 14:40 Subject: [PATCH] t: remove unexpected SANITIZE_LEAK variables Message-ID: <20250520-kn-remove-unexpected-exported-v1-1-bb60cec57e84@gmail.com> URL: https://gitlist.dev/e/20250520-kn-remove-unexpected-exported-v1-1-bb60cec57e84%40gmail.com ``` As of 1fc7ddf35b (test-lib: unconditionally enable leak checking, 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the leak checks are enabled by default. However, some newly added tests include them by mistake. Let's clean this up. Signed-off-by: Karthik Nayak --- t/perf/p5313-pack-objects.sh | 3 --- t/perf/p5314-name-hash.sh | 3 --- t/t6601-path-walk.sh | 2 -- 3 files changed, 8 deletions(-) diff --git a/t/perf/p5313-pack-objects.sh b/t/perf/p5313-pack-objects.sh index be5229a0ec..786a2c1c6f 100755 --- a/t/perf/p5313-pack-objects.sh +++ b/t/perf/p5313-pack-objects.sh @@ -3,9 +3,6 @@ test_description='Tests pack performance using bitmaps' . ./perf-lib.sh -GIT_TEST_PASSING_SANITIZE_LEAK=0 -export GIT_TEST_PASSING_SANITIZE_LEAK - test_perf_large_repo test_expect_success 'create rev input' ' diff --git a/t/perf/p5314-name-hash.sh b/t/perf/p5314-name-hash.sh index 4ef0ba7711..235cdfc824 100755 --- a/t/perf/p5314-name-hash.sh +++ b/t/perf/p5314-name-hash.sh @@ -3,9 +3,6 @@ test_description='Tests pack performance using bitmaps' . ./perf-lib.sh -GIT_TEST_PASSING_SANITIZE_LEAK=0 -export GIT_TEST_PASSING_SANITIZE_LEAK - test_perf_large_repo test_size 'paths at head' ' diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh index c89b0f1e19..8d187f7279 100755 --- a/t/t6601-path-walk.sh +++ b/t/t6601-path-walk.sh @@ -1,7 +1,5 @@ #!/bin/sh -TEST_PASSES_SANITIZE_LEAK=true - test_description='direct path-walk API tests' . ./test-lib.sh ``` ## Justin Tobler, 2025-05-20 21:16 Subject: Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables Message-ID: URL: https://gitlist.dev/e/dw57mhszlyk3s4rvitdthpbaoiicjnnraq24e6uxaeu2kze26d%40xpiouvxmhowr In-Reply-To: <20250520-kn-remove-unexpected-exported-v1-1-bb60cec57e84@gmail.com> ``` On 25/05/20 04:40PM, Karthik Nayak wrote: > As of 1fc7ddf35b (test-lib: unconditionally enable leak checking, > 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and > `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the > leak checks are enabled by default. However, some newly added tests > include them by mistake. Let's clean this up. Indeed, both `GIT_TEST_PASSING_SANITIZE_LEAK` and `TEST_PASSES_SANITIZE_LEAK` appear not have any purpose anymore. Removing all remaining instances where they appear makes sense and from a quick search it looks like this patch got them all. So this looks good to me. -Justin > > Signed-off-by: Karthik Nayak > --- > t/perf/p5313-pack-objects.sh | 3 --- > t/perf/p5314-name-hash.sh | 3 --- > t/t6601-path-walk.sh | 2 -- > 3 files changed, 8 deletions(-) > > diff --git a/t/perf/p5313-pack-objects.sh b/t/perf/p5313-pack-objects.sh > index be5229a0ec..786a2c1c6f 100755 > --- a/t/perf/p5313-pack-objects.sh > +++ b/t/perf/p5313-pack-objects.sh > @@ -3,9 +3,6 @@ > test_description='Tests pack performance using bitmaps' > . ./perf-lib.sh > > -GIT_TEST_PASSING_SANITIZE_LEAK=0 > -export GIT_TEST_PASSING_SANITIZE_LEAK > - > test_perf_large_repo > > test_expect_success 'create rev input' ' > diff --git a/t/perf/p5314-name-hash.sh b/t/perf/p5314-name-hash.sh > index 4ef0ba7711..235cdfc824 100755 > --- a/t/perf/p5314-name-hash.sh > +++ b/t/perf/p5314-name-hash.sh > @@ -3,9 +3,6 @@ > test_description='Tests pack performance using bitmaps' > . ./perf-lib.sh > > -GIT_TEST_PASSING_SANITIZE_LEAK=0 > -export GIT_TEST_PASSING_SANITIZE_LEAK > - > test_perf_large_repo > > test_size 'paths at head' ' > diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh > index c89b0f1e19..8d187f7279 100755 > --- a/t/t6601-path-walk.sh > +++ b/t/t6601-path-walk.sh > @@ -1,7 +1,5 @@ > #!/bin/sh > > -TEST_PASSES_SANITIZE_LEAK=true > - > test_description='direct path-walk API tests' > > . ./test-lib.sh > > > > ``` ## Junio C Hamano, 2025-05-20 22:09 Subject: Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables Message-ID: URL: https://gitlist.dev/e/xmqqbjrngci9.fsf%40gitster.g In-Reply-To: ``` Justin Tobler writes: > On 25/05/20 04:40PM, Karthik Nayak wrote: >> As of 1fc7ddf35b (test-lib: unconditionally enable leak checking, >> 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and >> `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the >> leak checks are enabled by default. However, some newly added tests >> include them by mistake. Let's clean this up. > > Indeed, both `GIT_TEST_PASSING_SANITIZE_LEAK` and > `TEST_PASSES_SANITIZE_LEAK` appear not have any purpose anymore. > Removing all remaining instances where they appear makes sense and from > a quick search it looks like this patch got them all. So this looks good > to me. Thanks, both. ``` ## Jeff King, 2025-05-22 06:06 Subject: Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables Message-ID: <20250522060626.GB1137854@coredump.intra.peff.net> URL: https://gitlist.dev/e/20250522060626.GB1137854%40coredump.intra.peff.net In-Reply-To: <20250520-kn-remove-unexpected-exported-v1-1-bb60cec57e84@gmail.com> ``` On Tue, May 20, 2025 at 04:40:12PM +0200, Karthik Nayak wrote: > As of 1fc7ddf35b (test-lib: unconditionally enable leak checking, > 2024-11-20), both the `GIT_TEST_PASSING_SANITIZE_LEAK` and > `TEST_PASSES_SANITIZE_LEAK` variables no longer have any meaning, the > leak checks are enabled by default. However, some newly added tests > include them by mistake. Let's clean this up. Thanks, I saw these recently while looking at another topic and was surprised. I hadn't yet confirmed that they truly are pointless, so I'm glad that you did. :) As a side note, we do still use the SANITIZE_LEAK prereq in a few places, and I believe that it does actually work. It might be nice to clean up any leaks in those few spots, though we probably want to keep the prereq around forever (e.g., if you introduce a test which shows off a leak and then fixes it later). All orthogonal to your patch, though, which looks good to me. -Peff ``` ## Jeff King, 2025-05-22 06:09 Subject: Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables Message-ID: <20250522060933.GA1135565@coredump.intra.peff.net> URL: https://gitlist.dev/e/20250522060933.GA1135565%40coredump.intra.peff.net In-Reply-To: <20250522060626.GB1137854@coredump.intra.peff.net> ``` On Thu, May 22, 2025 at 02:06:26AM -0400, Jeff King wrote: > As a side note, we do still use the SANITIZE_LEAK prereq in a few > places, and I believe that it does actually work. It might be nice to > clean up any leaks in those few spots, though we probably want to keep > the prereq around forever (e.g., if you introduce a test which shows off > a leak and then fixes it later). All orthogonal to your patch, though, > which looks good to me. Ah, nevermind. These are all due to 8415595203 (t5601: work around leak sanitizer issue, 2024-11-20). They are leak-free (and the tests pass on my system), but apparently some upstream bug can cause issues. -Peff ``` ## Karthik Nayak, 2025-05-22 09:06 Subject: Re: [PATCH] t: remove unexpected SANITIZE_LEAK variables Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZTg8_Hs0Ysu4bhtp9VZxnDEkSx3MvD1dN2yYYw7DqKRLw%40mail.gmail.com In-Reply-To: <20250522060933.GA1135565@coredump.intra.peff.net> ``` Jeff King writes: > On Thu, May 22, 2025 at 02:06:26AM -0400, Jeff King wrote: > >> As a side note, we do still use the SANITIZE_LEAK prereq in a few >> places, and I believe that it does actually work. It might be nice to >> clean up any leaks in those few spots, though we probably want to keep >> the prereq around forever (e.g., if you introduce a test which shows off >> a leak and then fixes it later). All orthogonal to your patch, though, >> which looks good to me. > > Ah, nevermind. These are all due to 8415595203 (t5601: work around leak > sanitizer issue, 2024-11-20). They are leak-free (and the tests pass on > my system), but apparently some upstream bug can cause issues. > > -Peff Yup. Someday in the future we can cleanup the whole thing. That would be nice! - Karthik ```