{"thread":{"id":"60165","subject":"[PATCH] ci(linux-asan-ubsan): let's save some time","startedAt":"2023-08-29T10:30:03Z","lastAt":"2023-08-30T06:21:09Z","messageCount":6,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"481111","messageId":"pull.1578.git.1693304963963.gitgitgadget@gmail.com","threadId":"60165","inReplyTo":null,"subject":"[PATCH] ci(linux-asan-ubsan): let's save some time","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-29T10:29:23Z","receivedAt":"2023-08-29T10:30:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEvery once in a while, the `git-p4` tests flake for reasons outside of\nour control. It typically fails with \"Connection refused\" e.g. here:\nhttps://github.com/git/git/actions/runs/5969707156/job/16196057724\n\n\t[...]\n\t+ git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git //depot\n\t Initialized empty Git repository in /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git/.git/\n\t Perforce client error:\n\t\tConnect to server failed; check $P4PORT.\n\t\tTCP connect to localhost:9807 failed.\n\t\tconnect: 127.0.0.1:9807: Connection refused\n\t failure accessing depot: could not run p4\n\t Importing from //depot into /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git\n\t [...]\n\nThis happens in other jobs, too, but in the `linux-asan-ubsan` job it\nhurts the most because that job often takes over a full hour to run,\ntherefore re-running a failed `linux-asan-ubsan` job is _very_ costly.\n\nThe purpose of the `linux-asan-ubsan` job is to exercise the C code of\nGit, anyway, and any part of Git's source code that the `git-p4` tests\nrun and that would benefit from the attention of ASAN/UBSAN are run\nbetter in other tests anyway, as debugging C code run via Python scripts\ncan get a bit hairy.\n\nIn fact, it is not even `git-p4` that is the problem (even if it flakes\noften enough to be problematic in the CI builds), but really the part\nabout Python scripts. So let's just skip any Python parts of the tests\nfrom being run in that job.\n\nFor good measure, also skip the Subversion tests because debugging C\ncode run via Perl scripts is as much fun as debugging C code run via\nPython scripts. And it will reduce the time this very expensive job\ntakes, which is a big benefit.\n\nSigned-off-by: Johannes Schindelin \n---\n ci(linux-asan-ubsan): let's save some time\n \n I often look at failed CI runs, and the linux-asan-ubsan job comes up\n frequently, and painfully (because it takes such a long time that\n re-running is often less desirable than getting the CI runs to pass).\n \n This commit is an attempt to reduce the pain and suffering stemming from\n this particular job, simply by deciding that the benefit of running the\n Python/Subversion-related tests in that job is far outweighed by its\n cost.\n \n This commit not only reduces the number of git-p4 flakes in\n linux-asan-ubsan to 0, it also seems to shave off about 10 minutes\n runtime, comparing\n https://github.com/gitgitgadget/git/actions/runs/5929602548/job/16077585391\n to\n https://github.com/gitgitgadget/git/actions/runs/6010305446/job/16301473243?pr=1578\n (which is not quite scientific due to the lack of a controlled\n environment, but it's the best we got for now). Together, those benefits\n form a strong incentive for me to get this merged.\n \n This patch is based on maint.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1578%2Fdscho%2Fskip-p4-from-asan-runs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1578/dscho/skip-p4-from-asan-runs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1578\n\n ci/lib.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 369d462f130..8e4e6713344 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -280,6 +280,8 @@ linux-leaks)\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\n+\texport NO_SVN_TESTS=LetsSaveSomeTimeBack\n+\tMAKEFLAGS=\"$MAKEFLAGS NO_PYTHON=YepItFlakesTooOften\"\n \t;;\n esac\n \n\nbase-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40\n-- \ngitgitgadget\n"},{"id":"481120","messageId":"xmqqzg29equv.fsf@gitster.g","threadId":"60165","inReplyTo":"pull.1578.git.1693304963963.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(linux-asan-ubsan): let's save some time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-29T16:49:12Z","receivedAt":"2023-08-29T16:50:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Every once in a while, the `git-p4` tests flake for reasons outside of\n> our control. It typically fails with \"Connection refused\" e.g. here:\n> https://github.com/git/git/actions/runs/5969707156/job/16196057724\n>\n> \t[...]\n> \t+ git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git //depot\n> \t Initialized empty Git repository in /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git/.git/\n> \t Perforce client error:\n> \t\tConnect to server failed; check $P4PORT.\n> \t\tTCP connect to localhost:9807 failed.\n> \t\tconnect: 127.0.0.1:9807: Connection refused\n> \t failure accessing depot: could not run p4\n> \t Importing from //depot into /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git\n> \t [...]\n>\n> This happens in other jobs, too, but in the `linux-asan-ubsan` job it\n> hurts the most because that job often takes over a full hour to run,\n> therefore re-running a failed `linux-asan-ubsan` job is _very_ costly.\n> ...\n> For good measure, also skip the Subversion tests because debugging C\n> code run via Perl scripts is as much fun as debugging C code run via\n> Python scripts. And it will reduce the time this very expensive job\n> takes, which is a big benefit.\n\nMakes sense to me.\n\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index 369d462f130..8e4e6713344 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -280,6 +280,8 @@ linux-leaks)\n> \t;;\n> linux-asan-ubsan)\n> \texport SANITIZE=address,undefined\n> +\texport NO_SVN_TESTS=LetsSaveSomeTimeBack\n> +\tMAKEFLAGS=\"$MAKEFLAGS NO_PYTHON=YepItFlakesTooOften\"\n> \t;;\n> esac\n\nHmph, we do not have NO_P4_TESTS to match, which lead to the\napparent inconsistency that is a bit of shame, but I think blanket\nexclusion of Python is OK because we are very unlikely to add new\nPython dependencies. s/ItFlakes/P4Flakes/ might be a good protection\nagainst Python enthusiasts complaining, though ;-)\n\nThanks, will queue.\n"},{"id":"481134","messageId":"pull.1578.v2.git.1693342048633.gitgitgadget@gmail.com","threadId":"60165","inReplyTo":"pull.1578.git.1693304963963.gitgitgadget@gmail.com","subject":"[PATCH v2] ci(linux-asan-ubsan): let's save some time","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-08-29T20:47:28Z","receivedAt":"2023-08-29T20:48:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nEvery once in a while, the `git-p4` tests flake for reasons outside of\nour control. It typically fails with \"Connection refused\" e.g. here:\nhttps://github.com/git/git/actions/runs/5969707156/job/16196057724\n\n\t[...]\n\t+ git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git //depot\n\t Initialized empty Git repository in /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git/.git/\n\t Perforce client error:\n\t\tConnect to server failed; check $P4PORT.\n\t\tTCP connect to localhost:9807 failed.\n\t\tconnect: 127.0.0.1:9807: Connection refused\n\t failure accessing depot: could not run p4\n\t Importing from //depot into /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git\n\t [...]\n\nThis happens in other jobs, too, but in the `linux-asan-ubsan` job it\nhurts the most because that job often takes over a full hour to run,\ntherefore re-running a failed `linux-asan-ubsan` job is _very_ costly.\n\nThe purpose of the `linux-asan-ubsan` job is to exercise the C code of\nGit, anyway, and any part of Git's source code that the `git-p4` tests\nrun and that would benefit from the attention of ASAN/UBSAN are run\nbetter in other tests anyway, as debugging C code run via Python scripts\ncan get a bit hairy.\n\nIn fact, it is not even just `git-p4` that is the problem (even if it\nflakes often enough to be problematic in the CI builds), but really the\npart about Python scripts. So let's just skip any Python parts of the\ntests from being run in that job.\n\nFor good measure, also skip the Subversion tests because debugging C\ncode run via Perl scripts is as much fun as debugging C code run via\nPython scripts. And it will reduce the time this very expensive job\ntakes, which is a big benefit.\n\nSigned-off-by: Johannes Schindelin \n---\n ci(linux-asan-ubsan): let's save some time\n \n I often look at failed CI runs, and the linux-asan-ubsan job comes up\n frequently, and painfully (because it takes such a long time that\n re-running is often less desirable than getting the CI runs to pass).\n \n This commit is an attempt to reduce the pain and suffering stemming from\n this particular job, simply by deciding that the benefit of running the\n Python/Subversion-related tests in that job is far outweighed by its\n cost.\n \n This commit not only reduces the number of git-p4 flakes in\n linux-asan-ubsan to 0, it also seems to shave off about 10 minutes\n runtime, comparing\n https://github.com/gitgitgadget/git/actions/runs/5929602548/job/16077585391\n to\n https://github.com/gitgitgadget/git/actions/runs/6010305446/job/16301473243?pr=1578\n (which is not quite scientific due to the lack of a controlled\n environment, but it's the best we got for now). Together, those benefits\n form a strong incentive for me to get this merged.\n \n This patch is based on maint.\n \n Changes since v1:\n \n * Made the rationale clearer (it is not Python that flakes, but\n Perforce).\n * Touched up the commit message.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1578%2Fdscho%2Fskip-p4-from-asan-runs-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1578/dscho/skip-p4-from-asan-runs-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1578\n\nRange-diff vs v1:\n\n 1: ac5b82ea934 ! 1: 1927a97667b ci(linux-asan-ubsan): let's save some time\n @@ Commit message\n better in other tests anyway, as debugging C code run via Python scripts\n can get a bit hairy.\n \n - In fact, it is not even `git-p4` that is the problem (even if it flakes\n - often enough to be problematic in the CI builds), but really the part\n - about Python scripts. So let's just skip any Python parts of the tests\n - from being run in that job.\n + In fact, it is not even just `git-p4` that is the problem (even if it\n + flakes often enough to be problematic in the CI builds), but really the\n + part about Python scripts. So let's just skip any Python parts of the\n + tests from being run in that job.\n \n For good measure, also skip the Subversion tests because debugging C\n code run via Perl scripts is as much fun as debugging C code run via\n @@ ci/lib.sh: linux-leaks)\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\n -+\texport NO_SVN_TESTS=LetsSaveSomeTimeBack\n -+\tMAKEFLAGS=\"$MAKEFLAGS NO_PYTHON=YepItFlakesTooOften\"\n ++\texport NO_SVN_TESTS=LetsSaveSomeTime\n ++\tMAKEFLAGS=\"$MAKEFLAGS NO_PYTHON=YepBecauseP4FlakesTooOften\"\n \t;;\n esac\n \n\n\n ci/lib.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 369d462f130..6fbb5bade12 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -280,6 +280,8 @@ linux-leaks)\n \t;;\n linux-asan-ubsan)\n \texport SANITIZE=address,undefined\n+\texport NO_SVN_TESTS=LetsSaveSomeTime\n+\tMAKEFLAGS=\"$MAKEFLAGS NO_PYTHON=YepBecauseP4FlakesTooOften\"\n \t;;\n esac\n \n\nbase-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40\n-- \ngitgitgadget\n"},{"id":"481135","messageId":"xmqqjztdcya8.fsf@gitster.g","threadId":"60165","inReplyTo":"pull.1578.v2.git.1693342048633.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] ci(linux-asan-ubsan): let's save some time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-29T21:51:43Z","receivedAt":"2023-08-29T21:52:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> ...\n> In fact, it is not even just `git-p4` that is the problem (even if it\n> flakes often enough to be problematic in the CI builds), but really the\n> part about Python scripts. So let's just skip any Python parts of the\n> tests from being run in that job.\n>\n> For good measure, also skip the Subversion tests because debugging C\n> code run via Perl scripts is as much fun as debugging C code run via\n> Python scripts. And it will reduce the time this very expensive job\n> takes, which is a big benefit.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> ...\n> Changes since v1:\n> \n> * Made the rationale clearer (it is not Python that flakes, but\n> Perforce).\n> * Touched up the commit message.\n\nThanks. Merged to 'next'.\n"},{"id":"481166","messageId":"20230830002801.GB227310@coredump.intra.peff.net","threadId":"60165","inReplyTo":"pull.1578.v2.git.1693342048633.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] ci(linux-asan-ubsan): let's save some time","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-08-30T00:28:01Z","receivedAt":"2023-08-30T00:29:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 29, 2023 at 08:47:28PM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> This commit is an attempt to reduce the pain and suffering stemming from\n> this particular job, simply by deciding that the benefit of running the\n> Python/Subversion-related tests in that job is far outweighed by its\n> cost.\n\nFWIW, I am in favor of this patch. I've had the same thought many times\nbut was worried I was being too dismissive of p4/svn (which I personally\ndo not care at all about). Omitting them from the sanitizer job (but\nstill running the basic svn/p4 tests elsewhere) seems like a good\ncompromise.\n\n-Peff\n"},{"id":"481169","messageId":"xmqqsf819hma.fsf@gitster.g","threadId":"60165","inReplyTo":"20230830002801.GB227310@coredump.intra.peff.net","subject":"Re: [PATCH v2] ci(linux-asan-ubsan): let's save some time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-30T06:19:57Z","receivedAt":"2023-08-30T06:21:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Aug 29, 2023 at 08:47:28PM +0000, Johannes Schindelin via GitGitGadget wrote:\n>\n>> This commit is an attempt to reduce the pain and suffering stemming from\n>> this particular job, simply by deciding that the benefit of running the\n>> Python/Subversion-related tests in that job is far outweighed by its\n>> cost.\n>\n> FWIW, I am in favor of this patch. I've had the same thought many times\n> but was worried I was being too dismissive of p4/svn (which I personally\n> do not care at all about). Omitting them from the sanitizer job (but\n> still running the basic svn/p4 tests elsewhere) seems like a good\n> compromise.\n\nExactly my feeling. Thanks, both.\n"}]}