{"thread":{"id":"64821","subject":"[PATCH] ci(*-leaks): skip the git-svn tests to save time","startedAt":"2026-01-16T17:31:19Z","lastAt":"2026-01-26T16:06:06Z","messageCount":13,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Phillip Wood","Kristoffer Haugsbakk"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"534063","messageId":"pull.2031.git.1768584676520.gitgitgadget@gmail.com","threadId":"64821","inReplyTo":null,"subject":"[PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-01-16T17:31:16Z","receivedAt":"2026-01-16T17:31:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nI noticed recently that the leak-checking jobs still take a lot of time,\nand upon analysis, the git-svn tests contribute significantly to this.\n\nAnalyzing a recent CI run, I saw that the Git test suite contains\n1,017 tests, running for approximately 5¼ hours total. Of these, 65\ngit-svn-related tests (~6% of test count) took 42.24 minutes combined,\naccounting for ~13.% of the total runtime. This implies that the git-svn\ntests are roughly twice as expernsive compared to the other tests.\n\nHowever, testing git-svn in the leak-checking jobs provides minimal\nvalue: git-svn is implemented as a Perl script, and leak checking only\nhandles C code. While git-svn does call into Git's built-in commands\nthat are implemented in C, these are standard Git operations that are\nalready thoroughly exercised elsewhere in the test suite. Therefore,\nrunning the git-svn tests in the leak-checking jobs only adds to the\noverall run time with little value in return.\n\nGiven that the leak-checking jobs are particularly time-intensive and\nthese 42+ minutes of SVN tests per job provide no additional leak\ndetection value, skip them in the *-leaks jobs to reduce CI runtime.\n\nAssisted-by: Claude Sonnet 4.5\nSigned-off-by: Johannes Schindelin \n---\n ci(*-leaks): skip the git-svn tests to save time\n \n I leaned heavily on AI to implement this patch, in particular when\n analyzing the logs. That's why I added that trailer talking about Claude\n Sonnet. If this is undesirable, please let me know.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2031%2Fdscho%2Fskip-svn-and-leak-tests-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2031/dscho/skip-svn-and-leak-tests-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2031\n\n ci/lib.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex f561884d40..a165c7f268 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -356,6 +356,7 @@ linux-musl-meson)\n \t;;\n linux-leaks|linux-reftable-leaks)\n \texport SANITIZE=leak\n+\texport NO_SVN_TESTS=LetsSaveSomeTime\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\n\nbase-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75\n-- \ngitgitgadget\n"},{"id":"534076","messageId":"xmqqms2d74b7.fsf@gitster.g","threadId":"64821","inReplyTo":"pull.2031.git.1768584676520.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-16T19:20:12Z","receivedAt":"2026-01-16T19:20:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> However, testing git-svn in the leak-checking jobs provides minimal\n> value: git-svn is implemented as a Perl script, and leak checking only\n> handles C code. While git-svn does call into Git's built-in commands\n> that are implemented in C, these are standard Git operations that are\n> already thoroughly exercised elsewhere in the test suite. Therefore,\n> running the git-svn tests in the leak-checking jobs only adds to the\n> overall run time with little value in return.\n\nVery nicely reasoned. And the implementation of this idea is ...\n\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index f561884d40..a165c7f268 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -356,6 +356,7 @@ linux-musl-meson)\n> \t;;\n> linux-leaks|linux-reftable-leaks)\n> \texport SANITIZE=leak\n> +\texport NO_SVN_TESTS=LetsSaveSomeTime\n> \t;;\n\n... surprisingly simple. I very much like it.\n\nThanks. Will queue.\n"},{"id":"534113","messageId":"b649f137-7140-4fb6-848a-b3408e54bbe5@gmail.com","threadId":"64821","inReplyTo":"pull.2031.git.1768584676520.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-17T15:04:48Z","receivedAt":"2026-01-17T15:04:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 16/01/2026 17:31, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> I noticed recently that the leak-checking jobs still take a lot of time,\n> and upon analysis, the git-svn tests contribute significantly to this.\n> \n> Analyzing a recent CI run, I saw that the Git test suite contains\n> 1,017 tests, running for approximately 5¼ hours total. Of these, 65\n> git-svn-related tests (~6% of test count) took 42.24 minutes combined,\n> accounting for ~13.% of the total runtime. This implies that the git-svn\n> tests are roughly twice as expernsive compared to the other tests.\n\nLooking at the CI logs for this PR the p4 and cvs tests account for \nanother 24 minutes of test time and I suspect they also offer little in \nthe way of extra coverage. Unfortunately there is no equivalent of \nNO_SVN_TESTS to disable them - I wonder if building with NO_PYTHON and \nNO_PERL would make sense for the leak test job?\n\nEither way I like the direction of this patch\n\nThanks\n\nPhillip\n\n> However, testing git-svn in the leak-checking jobs provides minimal\n> value: git-svn is implemented as a Perl script, and leak checking only\n> handles C code. While git-svn does call into Git's built-in commands\n> that are implemented in C, these are standard Git operations that are\n> already thoroughly exercised elsewhere in the test suite. Therefore,\n> running the git-svn tests in the leak-checking jobs only adds to the\n> overall run time with little value in return.\n> \n> Given that the leak-checking jobs are particularly time-intensive and\n> these 42+ minutes of SVN tests per job provide no additional leak\n> detection value, skip them in the *-leaks jobs to reduce CI runtime.\n> \n> Assisted-by: Claude Sonnet 4.5\n> Signed-off-by: Johannes Schindelin \n> ---\n> ci(*-leaks): skip the git-svn tests to save time\n> \n> I leaned heavily on AI to implement this patch, in particular when\n> analyzing the logs. That's why I added that trailer talking about Claude\n> Sonnet. If this is undesirable, please let me know.\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2031%2Fdscho%2Fskip-svn-and-leak-tests-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2031/dscho/skip-svn-and-leak-tests-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/2031\n> \n> ci/lib.sh | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index f561884d40..a165c7f268 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -356,6 +356,7 @@ linux-musl-meson)\n> \t;;\n> linux-leaks|linux-reftable-leaks)\n> \texport SANITIZE=leak\n> +\texport NO_SVN_TESTS=LetsSaveSomeTime\n> \t;;\n> linux-asan-ubsan)\n> \texport SANITIZE=address,undefined\n> \n> base-commit: 7264e61d87e58b9d0f5e6424c47c11e9657dfb75\n\n"},{"id":"534124","messageId":"xmqqa4yc5brq.fsf@gitster.g","threadId":"64821","inReplyTo":"b649f137-7140-4fb6-848a-b3408e54bbe5@gmail.com","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-17T18:34:17Z","receivedAt":"2026-01-17T18:34:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Looking at the CI logs for this PR the p4 and cvs tests account for \n> another 24 minutes of test time and I suspect they also offer little in \n> the way of extra coverage. Unfortunately there is no equivalent of \n> NO_SVN_TESTS to disable them - I wonder if building with NO_PYTHON and \n> NO_PERL would make sense for the leak test job?\n>\n> Either way I like the direction of this patch\n>\n> Thanks\n>\n> Phillip\n\nYup, I generally like this direction, and introducing NO_P4_TESTS\nand NO_CVS_TESTS would not be so bad. Here is how it looks on top\nof Dscho's patch.\n\n--- >8 ---\nSubject: [PATCH] ci: skip CVS and P4 tests in leaks job, too\n\nLooking at the CI logs, the p4 and cvs tests account for another 24\nminutes of test time and they offer minimal value for quite a\nsimilar reason as the previous step.\n\nLet's introduce and use a mechanism to skip these tests to save\nsome resources.\n\nSuggested-by: Phillip Wood \nSigned-off-by: Junio C Hamano \n---\n ci/lib.sh | 2 ++\n t/lib-cvs.sh | 6 ++++++\n t/lib-git-p4.sh | 5 +++++\n 3 files changed, 13 insertions(+)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex a165c7f268..3ecbf147db 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -356,7 +356,9 @@ linux-musl-meson)\n \t;;\n linux-leaks|linux-reftable-leaks)\n \texport SANITIZE=leak\n+\texport NO_CVS_TESTS=LetsSaveSomeTime\n \texport NO_SVN_TESTS=LetsSaveSomeTime\n+\texport NO_P4_TESTS=LetsSaveSomeTime\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\ndiff --git a/t/lib-cvs.sh b/t/lib-cvs.sh\nindex 57b9b2db9b..c8b4404888 100644\n--- a/t/lib-cvs.sh\n+++ b/t/lib-cvs.sh\n@@ -2,6 +2,12 @@\n \n . ./test-lib.sh\n \n+if test -n \"$NO_CVS_TESTS\"\n+then\n+\tskip_all='skipping git cvs tests, NO_CVS_TESTS defined'\n+\ttest_done\n+fi\n+\n unset CVS_SERVER\n \n if ! type cvs >/dev/null 2>&1\ndiff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\nindex 2a5b8738ea..d22e9c684a 100644\n--- a/t/lib-git-p4.sh\n+++ b/t/lib-git-p4.sh\n@@ -16,6 +16,11 @@ P4D_TIMEOUT=300\n \n . ./test-lib.sh\n \n+if test -n \"$NO_P4_TESTS\"\n+then\n+\tskip_all='skipping git p4 tests, NO_P4_TESTS defined'\n+\ttest_done\n+fi\n if ! test_have_prereq PYTHON\n then\n \tskip_all='skipping git p4 tests; python not available'\n-- \n2.53.0-rc0-217-gd590ba4684\n\n"},{"id":"534125","messageId":"de592bff-d7ac-41b5-bb24-4ced285a52e0@app.fastmail.com","threadId":"64821","inReplyTo":"xmqqa4yc5brq.fsf@gitster.g","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-01-17T19:02:16Z","receivedAt":"2026-01-17T19:02:37Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Sat, Jan 17, 2026, at 19:34, Junio C Hamano wrote:\n>>[snip]\n> Yup, I generally like this direction, and introducing NO_P4_TESTS\n> and NO_CVS_TESTS would not be so bad. Here is how it looks on top\n> of Dscho's patch.\n>\n> --- >8 ---\n> Subject: [PATCH] ci: skip CVS and P4 tests in leaks job, too\n>\n> Looking at the CI logs, the p4 and cvs tests account for another 24\n> minutes of test time and they offer minimal value for quite a\n> similar reason as the previous step.\n>\n> Let's introduce and use a mechanism to skip these tests to save\n> some resources.\n>\n> Suggested-by: Phillip Wood \n\nNitpick: Using the commit ident\n\n Phillip Wood \n\nmight be slightly better?\n\n> Signed-off-by: Junio C Hamano \n> ---\n>[snip]\n"},{"id":"534131","messageId":"xmqq5x8z69mq.fsf@gitster.g","threadId":"64821","inReplyTo":"de592bff-d7ac-41b5-bb24-4ced285a52e0@app.fastmail.com","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-18T00:35:09Z","receivedAt":"2026-01-18T00:35:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristoffer Haugsbakk\" writes:\n\n> On Sat, Jan 17, 2026, at 19:34, Junio C Hamano wrote:\n>>>[snip]\n>> Yup, I generally like this direction, and introducing NO_P4_TESTS\n>> and NO_CVS_TESTS would not be so bad. Here is how it looks on top\n>> of Dscho's patch.\n>>\n>> --- >8 ---\n>> Subject: [PATCH] ci: skip CVS and P4 tests in leaks job, too\n>>\n>> Looking at the CI logs, the p4 and cvs tests account for another 24\n>> minutes of test time and they offer minimal value for quite a\n>> similar reason as the previous step.\n>>\n>> Let's introduce and use a mechanism to skip these tests to save\n>> some resources.\n>>\n>> Suggested-by: Phillip Wood \n>\n> Nitpick: Using the commit ident\n>\n> Phillip Wood \n>\n> might be slightly better?\n\nI didn't even realize there are multiple addresses in play,\nactually. I just took it from the e-mail header's Cc: field,\nwhich my MUA copied from From: field of the message I was responding\nto, which was the identity of the person who suggested the change\nafter all ;-).\n\nSo, I dunno.\n"},{"id":"534226","messageId":"b1ea2c89-890d-492d-9bb0-2f0f67599b2c@gmail.com","threadId":"64821","inReplyTo":"xmqq5x8z69mq.fsf@gitster.g","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-20T10:31:27Z","receivedAt":"2026-01-20T10:31:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 18/01/2026 00:35, Junio C Hamano wrote:\n> \"Kristoffer Haugsbakk\" writes:\n> \n>> On Sat, Jan 17, 2026, at 19:34, Junio C Hamano wrote:\n>>>> [snip]\n>>> Yup, I generally like this direction, and introducing NO_P4_TESTS\n>>> and NO_CVS_TESTS would not be so bad. Here is how it looks on top\n>>> of Dscho's patch.\n>>>\n>>> --- >8 ---\n>>> Subject: [PATCH] ci: skip CVS and P4 tests in leaks job, too\n>>>\n>>> Looking at the CI logs, the p4 and cvs tests account for another 24\n>>> minutes of test time and they offer minimal value for quite a\n>>> similar reason as the previous step.\n>>>\n>>> Let's introduce and use a mechanism to skip these tests to save\n>>> some resources.\n>>>\n>>> Suggested-by: Phillip Wood \n>>\n>> Nitpick: Using the commit ident\n>>\n>> Phillip Wood \n>>\n>> might be slightly better?\n> \n> I didn't even realize there are multiple addresses in play,\n> actually. I just took it from the e-mail header's Cc: field,\n> which my MUA copied from From: field of the message I was responding\n> to, which was the identity of the person who suggested the change\n> after all ;-).\n\nThe dunelm address is a forwarding address that should keep working if I \nchange my email provider. I keep meaning send a patch with a mailmap \nentry but never get round to actually doing it.\n\nThanks\n\nPhillip\n"},{"id":"534227","messageId":"79c29032-dc32-4030-95ad-dfbd50d9b6ab@gmail.com","threadId":"64821","inReplyTo":"xmqqa4yc5brq.fsf@gitster.g","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-20T10:34:39Z","receivedAt":"2026-01-20T10:34:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/01/2026 18:34, Junio C Hamano wrote:\n> \n> Yup, I generally like this direction, and introducing NO_P4_TESTS\n> and NO_CVS_TESTS would not be so bad. Here is how it looks on top\n> of Dscho's patch.\n> \n> --- >8 ---\n> Subject: [PATCH] ci: skip CVS and P4 tests in leaks job, too\n> \n> Looking at the CI logs, the p4 and cvs tests account for another 24\n> minutes of test time and they offer minimal value for quite a\n> similar reason as the previous step.\n> \n> Let's introduce and use a mechanism to skip these tests to save\n> some resources.\n\nThe patch looks good to me, it is very convenient that we can put the \ntest in the library files rather than each test file. Should we drop \nthese tests from the ASan job as well?\n\nThanks\n\nPhillip\n\n> Suggested-by: Phillip Wood \n> Signed-off-by: Junio C Hamano \n> ---\n> ci/lib.sh | 2 ++\n> t/lib-cvs.sh | 6 ++++++\n> t/lib-git-p4.sh | 5 +++++\n> 3 files changed, 13 insertions(+)\n> \n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index a165c7f268..3ecbf147db 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -356,7 +356,9 @@ linux-musl-meson)\n> \t;;\n> linux-leaks|linux-reftable-leaks)\n> \texport SANITIZE=leak\n> +\texport NO_CVS_TESTS=LetsSaveSomeTime\n> \texport NO_SVN_TESTS=LetsSaveSomeTime\n> +\texport NO_P4_TESTS=LetsSaveSomeTime\n> \t;;\n> linux-asan-ubsan)\n> \texport SANITIZE=address,undefined\n> diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh\n> index 57b9b2db9b..c8b4404888 100644\n> --- a/t/lib-cvs.sh\n> +++ b/t/lib-cvs.sh\n> @@ -2,6 +2,12 @@\n> \n> . ./test-lib.sh\n> \n> +if test -n \"$NO_CVS_TESTS\"\n> +then\n> +\tskip_all='skipping git cvs tests, NO_CVS_TESTS defined'\n> +\ttest_done\n> +fi\n> +\n> unset CVS_SERVER\n> \n> if ! type cvs >/dev/null 2>&1\n> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n> index 2a5b8738ea..d22e9c684a 100644\n> --- a/t/lib-git-p4.sh\n> +++ b/t/lib-git-p4.sh\n> @@ -16,6 +16,11 @@ P4D_TIMEOUT=300\n> \n> . ./test-lib.sh\n> \n> +if test -n \"$NO_P4_TESTS\"\n> +then\n> +\tskip_all='skipping git p4 tests, NO_P4_TESTS defined'\n> +\ttest_done\n> +fi\n> if ! test_have_prereq PYTHON\n> then\n> \tskip_all='skipping git p4 tests; python not available'\n\n"},{"id":"534266","messageId":"xmqq1pjk2sv2.fsf@gitster.g","threadId":"64821","inReplyTo":"79c29032-dc32-4030-95ad-dfbd50d9b6ab@gmail.com","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-20T15:42:25Z","receivedAt":"2026-01-20T15:42:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> The patch looks good to me, it is very convenient that we can put the \n> test in the library files rather than each test file. Should we drop \n> these tests from the ASan job as well?\n\nThat's probably a good idea.\n\nI also was wondering if we want a blanket NO_FOO_TESTS that we can\nuse instead listing all. That FOO should not be SCM, though, as the\nreason why we exclude the tests is not because they are test about\nforeign SCM. We exclude them as low value because testing them\nexercises little code of ours that we may make mistakes these checks\nare trying to uncover and that we can fix when they do.\nNO_FOREIGN_CODE_TESTS? I dunno.\n\nThanks.\n\n> Thanks\n>\n> Phillip\n>\n>> Suggested-by: Phillip Wood \n>> Signed-off-by: Junio C Hamano \n>> ---\n>> ci/lib.sh | 2 ++\n>> t/lib-cvs.sh | 6 ++++++\n>> t/lib-git-p4.sh | 5 +++++\n>> 3 files changed, 13 insertions(+)\n>> \n>> diff --git a/ci/lib.sh b/ci/lib.sh\n>> index a165c7f268..3ecbf147db 100755\n>> --- a/ci/lib.sh\n>> +++ b/ci/lib.sh\n>> @@ -356,7 +356,9 @@ linux-musl-meson)\n>> \t;;\n>> linux-leaks|linux-reftable-leaks)\n>> \texport SANITIZE=leak\n>> +\texport NO_CVS_TESTS=LetsSaveSomeTime\n>> \texport NO_SVN_TESTS=LetsSaveSomeTime\n>> +\texport NO_P4_TESTS=LetsSaveSomeTime\n>> \t;;\n>> linux-asan-ubsan)\n>> \texport SANITIZE=address,undefined\n>> diff --git a/t/lib-cvs.sh b/t/lib-cvs.sh\n>> index 57b9b2db9b..c8b4404888 100644\n>> --- a/t/lib-cvs.sh\n>> +++ b/t/lib-cvs.sh\n>> @@ -2,6 +2,12 @@\n>> \n>> . ./test-lib.sh\n>> \n>> +if test -n \"$NO_CVS_TESTS\"\n>> +then\n>> +\tskip_all='skipping git cvs tests, NO_CVS_TESTS defined'\n>> +\ttest_done\n>> +fi\n>> +\n>> unset CVS_SERVER\n>> \n>> if ! type cvs >/dev/null 2>&1\n>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh\n>> index 2a5b8738ea..d22e9c684a 100644\n>> --- a/t/lib-git-p4.sh\n>> +++ b/t/lib-git-p4.sh\n>> @@ -16,6 +16,11 @@ P4D_TIMEOUT=300\n>> \n>> . ./test-lib.sh\n>> \n>> +if test -n \"$NO_P4_TESTS\"\n>> +then\n>> +\tskip_all='skipping git p4 tests, NO_P4_TESTS defined'\n>> +\ttest_done\n>> +fi\n>> if ! test_have_prereq PYTHON\n>> then\n>> \tskip_all='skipping git p4 tests; python not available'\n"},{"id":"534550","messageId":"a29ce07a-9585-475e-bb3d-4742f2ec400a@gmail.com","threadId":"64821","inReplyTo":"xmqq1pjk2sv2.fsf@gitster.g","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-23T14:47:11Z","receivedAt":"2026-01-23T14:47:16Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/01/2026 15:42, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> The patch looks good to me, it is very convenient that we can put the\n>> test in the library files rather than each test file. Should we drop\n>> these tests from the ASan job as well?\n> \n> That's probably a good idea.\n> \n> I also was wondering if we want a blanket NO_FOO_TESTS that we can\n> use instead listing all. That FOO should not be SCM, though, as the\n> reason why we exclude the tests is not because they are test about\n> foreign SCM. We exclude them as low value because testing them\n> exercises little code of ours that we may make mistakes these checks\n> are trying to uncover and that we can fix when they do.\n> NO_FOREIGN_CODE_TESTS? I dunno.\n\nHaving a single Makefile knob is tempting, the naming as tricky though. \nWe're skipping these tests because they're scripted and we already have \nleak coverage for the git commands that they call, the fact that they're \ncalling foreign programs is incidental to that. If \"git svn\" was \nimplemented in C then we probably would want to check it for leaks even \nthough it called a foreign program. That's a long winded way of saying I \ndon't have any better suggestions!\n\nThanks\n\nPhillip\n\n"},{"id":"534571","messageId":"xmqqqzrggr39.fsf@gitster.g","threadId":"64821","inReplyTo":"a29ce07a-9585-475e-bb3d-4742f2ec400a@gmail.com","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-23T17:46:02Z","receivedAt":"2026-01-23T17:46:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> ... If \"git svn\" was \n> implemented in C then we probably would want to check it for leaks even \n> though it called a foreign program. That's a long winded way of saying I \n> don't have any better suggestions!\n\nI am not sure if I agree. If Perl interpreter used to run the Perl\nversion of \"git svn\" were found leaky, are we willing to go in and\nplug leaks there? Not likely, particularly since it is not what we\nship and we do not have control over which version of Perl the users\nhave on their systems. So we say \"Perl is foreign and we are not\nequipped to plug leaks in various versions of it on users' systems,\nso it is not worth spending cycles to test for leaks in it\".\n\nIf \"git svn\" were in C, linked with libsvn without using the perl\nbinding, and libsvn were found leaky, the story is the same. We do\nnot control the version of libsvn the users have on their systems,\nwe are not equipped to plug leaks in there, so it is not our job to\nspend cycles to test for leaks in it.\n\n\n"},{"id":"534639","messageId":"82b656a5-e5c8-4056-8ec5-4bdab9ef7128@gmail.com","threadId":"64821","inReplyTo":"xmqqqzrggr39.fsf@gitster.g","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-01-26T09:47:46Z","receivedAt":"2026-01-26T09:47:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 23/01/2026 17:46, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> ... If \"git svn\" was\n>> implemented in C then we probably would want to check it for leaks even\n>> though it called a foreign program. That's a long winded way of saying I\n>> don't have any better suggestions!\n> \n> I am not sure if I agree. If Perl interpreter used to run the Perl\n> version of \"git svn\" were found leaky, are we willing to go in and\n> plug leaks there? Not likely, particularly since it is not what we\n> ship and we do not have control over which version of Perl the users\n> have on their systems. So we say \"Perl is foreign and we are not\n> equipped to plug leaks in various versions of it on users' systems,\n> so it is not worth spending cycles to test for leaks in it\".\n>\n> If \"git svn\" were in C, linked with libsvn without using the perl\n> binding, and libsvn were found leaky, the story is the same. We do\n> not control the version of libsvn the users have on their systems,\n> we are not equipped to plug leaks in there, so it is not our job to\n> spend cycles to test for leaks in it.\n\nI think that unless the libsvn that linked against was built with \n-fsanitize=leak we wouldn't find any leaks in it anyway. When I wrote my \noriginal mail I was imagining C implementation that forked \"svn\" but \nreplaced the perl code with C that called the appropriate functions in \nlibgit rather than forking git. In that case I think there's an argument \nfor checking that our code does not leak. Anyway this is all rather \nhypothetical as we're not likely to rewrite these scripts in C.\n\nThanks\n\nPhillip\n"},{"id":"534672","messageId":"xmqqikcojr4k.fsf@gitster.g","threadId":"64821","inReplyTo":"82b656a5-e5c8-4056-8ec5-4bdab9ef7128@gmail.com","subject":"Re: [PATCH] ci(*-leaks): skip the git-svn tests to save time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-26T16:06:03Z","receivedAt":"2026-01-26T16:06:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> I think that unless the libsvn that linked against was built with \n> -fsanitize=leak we wouldn't find any leaks in it anyway. When I wrote my \n> original mail I was imagining C implementation that forked \"svn\" but \n> replaced the perl code with C that called the appropriate functions in \n> libgit rather than forking git.\n\nIt was the scenario I was assuming as well, but I simply forgot to\nconsider that we want to catch leaks in our \"client\" code (client\nfrom the point of view of the libsvn library).\n\nAnd you are right. It can be done to check our leaks without being\nable to touch libsvn to fix their leaks, even though we may have to\nfilter out noises from the leak checker if there are their leaks we\ncannot plug.\n\n> In that case I think there's an argument \n> for checking that our code does not leak. Anyway this is all rather \n> hypothetical as we're not likely to rewrite these scripts in C.\n\n;-).\n"}]}