t4126: this test does not pass SANITIZE_LEAK; quit claiming it does
PATCH 1/1 t4126: this test does not pass SANITIZE_LEAK; quit claiming it does
From: Elijah Newren <[email protected]>
---
t4126: this test does not pass SANITIZE_LEAK; quit claiming it does
My builds in several different topics keep running into this, and I'm sure it's a false positive -- I didn't change anything affecting this test. I've just been ignoring it and submitting anyway, and I suspect others are too.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1177%2Fnewren%2Ft4126-apply-empty-is-not-leak-free-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1177/newren/t4126-apply-empty-is-not-leak-free-v1
Pull-Request: https://github.com/git/git/pull/1177
t/t4126-apply-empty.sh | 1 - 1 file changed, 1 deletion(-)
t/t4126-apply-empty.sh +0 -1
@@ -3,7 +3,6 @@ test_description='apply empty' -TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success setup '
base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
Signature · 1 line
gitgitgadget
Hi Elijah,
Quoted 10 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
Signed-off-by Elijah Newren <[email protected]>---
t4126: this test does not passSANITIZE_LEAK; quit claiming it doesMy builds in several different topics keep running into this, and I'm sure it's a false positive -- I didn't change anything affecting this test. I've just been ignoring it and submitting anyway, and I suspect others are too.
So that's what is causing this. Thank you for chasing it down, it has been on my TO-DO list e.g. due to
https://github.com/git-for-windows/git/runs/4622790431?check_suite_focus=true#step:5:146
Given that it points to a leak in cmd_format_patch() (oh no, we're _LEAKING_ some _MEMORY_ in a built-in, how did that ever happen, we must free it before... *checks notes* ... wait, we're quitting immediately after this function anyway? </sarcasm>) I wonder whether the disruption caused by linux-leaks should really be considered worth the benefit.
Anyway. Thank you for the patch!
Ciao,
Dscho
Quoted 5 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
Signed-off-by Elijah Newren <[email protected]>---
t4126: this test does not passSANITIZE_LEAK; quit claiming it does
Is this continuation of [*1*]?
I think it will go away when dee839a2 (format-patch: mark rev_info with UNLEAK, 2021-12-16) is merged.
[Reference]
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Elijah Newren via GitGitGadget
From: Elijah Newren <[email protected]>
Signed-off-by Elijah Newren <[email protected]>---
t4126: this test does not passSANITIZE_LEAK; quit claiming it doesIs this continuation of [*1*]?
Ah, yes it is.
Quoted 2 lines
I think it will go away when
dee839a2(format-patch: markrev_infowith UNLEAK, 2021-12-16) is merged.
Can we fast track that patch? The patch looks good to me. In general, though, I would think the simpler fix of just removing the TEST_PASSES_SANITIZE_LEAK=true would be safer and could be fast tracked.
I'm sorry I missed these potential problems cropping up when I reviewed the earlier series (which added these flags to a bunch of scripts). I just didn't foresee these consequences.