ci(linux-asan-ubsan): let's save some time
PATCH 1/1 ci(linux-asan-ubsan): let's save some time
From: Johannes Schindelin <[email protected]>
Every once in a while, the git-p4 tests flake for reasons outside of our control. It typically fails with "Connection refused" e.g. here:
https://github.com/git/git/actions/runs/5969707156/job/16196057724
[...]
+ git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git //depot Initialized empty Git repository in /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git/.git/ Perforce client error:
Connect to server failed; check $P4PORT.
TCP connect to localhost:9807 failed.
connect: 127.0.0.1:9807: Connection refused
failure accessing depot: could not run p4
Importing from //depot into /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git
[...]
This happens in other jobs, too, but in the linux-asan-ubsan job it hurts the most because that job often takes over a full hour to run, therefore re-running a failed linux-asan-ubsan job is _very_ costly.
The purpose of the linux-asan-ubsan job is to exercise the C code of Git, anyway, and any part of Git's source code that the git-p4 tests run and that would benefit from the attention of ASAN/UBSAN are run better in other tests anyway, as debugging C code run via Python scripts can get a bit hairy.
In fact, it is not even git-p4 that is the problem (even if it flakes often enough to be problematic in the CI builds), but really the part about Python scripts. So let's just skip any Python parts of the tests from being run in that job.
For good measure, also skip the Subversion tests because debugging C code run via Perl scripts is as much fun as debugging C code run via Python scripts. And it will reduce the time this very expensive job takes, which is a big benefit.
---
ci(linux-asan-ubsan): let's save some time
I often look at failed CI runs, and the linux-asan-ubsan job comes up frequently, and painfully (because it takes such a long time that re-running is often less desirable than getting the CI runs to pass).
This commit is an attempt to reduce the pain and suffering stemming from this particular job, simply by deciding that the benefit of running the Python/Subversion-related tests in that job is far outweighed by its cost.
This commit not only reduces the number of git-p4 flakes in linux-asan-ubsan to 0, it also seems to shave off about 10 minutes runtime, comparing
https://github.com/gitgitgadget/git/actions/runs/5929602548/job/16077585391
to
https://github.com/gitgitgadget/git/actions/runs/6010305446/job/16301473243?pr=1578
(which is not quite scientific due to the lack of a controlled environment, but it's the best we got for now). Together, those benefits form a strong incentive for me to get this merged.
This patch is based on maint.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1578%2Fdscho%2Fskip-p4-from-asan-runs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1578/dscho/skip-p4-from-asan-runs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1578
ci/lib.sh | 2 ++ 1 file changed, 2 insertions(+)
ci/lib.sh +2 -0
@@ -280,6 +280,8 @@ linux-leaks) ;; linux-asan-ubsan) export SANITIZE=address,undefined + export NO_SVN_TESTS=LetsSaveSomeTimeBack + MAKEFLAGS="$MAKEFLAGS NO_PYTHON=YepItFlakesTooOften" ;; esac
base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 15 lines · writes:
Every once in a while, the
git-p4tests flake for reasons outside of our control. It typically fails with "Connection refused" e.g. here:
https://github.com/git/git/actions/runs/5969707156/job/16196057724[...]
+ git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git //depot Initialized empty Git repository in/home/runner/work/git/git/t/trashdirectory.t9807-git-p4-submit/git/.git/ Perforce client error:
Connect to server failed; check $P4PORT.
TCP connect to localhost:9807 failed.
connect: 127.0.0.1:9807: Connection refused
failure accessing depot: could not run p4
Importing from //depot into/home/runner/work/git/git/t/trashdirectory.t9807-git-p4-submit/git
[...]
Quoted 8 lines
This happens in other jobs, too, but in the
linux-asan-ubsanjob it hurts the most because that job often takes over a full hour to run, therefore re-running a failedlinux-asan-ubsanjob is _very_ costly.
...
For good measure, also skip the Subversion tests because debugging C code run via Perl scripts is as much fun as debugging C code run via Python scripts. And it will reduce the time this very expensive job takes, which is a big benefit.
Makes sense to me.
ci/lib.sh linux-leaks) · line 286
Quoted 12 lines
ci/lib.sh +2
-0@@ -280,6 +280,8 @@ linux-leaks) ;; linux-asan-ubsan) export SANITIZE=address,undefined + export NO_SVN_TESTS=LetsSaveSomeTimeBack + MAKEFLAGS="$MAKEFLAGS NO_PYTHON=YepItFlakesTooOften" ;; esac
Hmph, we do not have NO_P4_TESTS to match, which lead to the apparent inconsistency that is a bit of shame, but I think blanket exclusion of Python is OK because we are very unlikely to add new Python dependencies. s/ItFlakes/P4Flakes/ might be a good protection against Python enthusiasts complaining, though ;-)
Thanks, will queue.
PATCH v2 1/1 ci(linux-asan-ubsan): let's save some time
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Every once in a while, the git-p4 tests flake for reasons outside of our control. It typically fails with "Connection refused" e.g. here:
https://github.com/git/git/actions/runs/5969707156/job/16196057724
[...]
+ git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git //depot Initialized empty Git repository in /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git/.git/ Perforce client error:
Connect to server failed; check $P4PORT.
TCP connect to localhost:9807 failed.
connect: 127.0.0.1:9807: Connection refused
failure accessing depot: could not run p4
Importing from //depot into /home/runner/work/git/git/t/trash directory.t9807-git-p4-submit/git
[...]
This happens in other jobs, too, but in the linux-asan-ubsan job it hurts the most because that job often takes over a full hour to run, therefore re-running a failed linux-asan-ubsan job is _very_ costly.
The purpose of the linux-asan-ubsan job is to exercise the C code of Git, anyway, and any part of Git's source code that the git-p4 tests run and that would benefit from the attention of ASAN/UBSAN are run better in other tests anyway, as debugging C code run via Python scripts can get a bit hairy.
In fact, it is not even just git-p4 that is the problem (even if it flakes often enough to be problematic in the CI builds), but really the part about Python scripts. So let's just skip any Python parts of the tests from being run in that job.
For good measure, also skip the Subversion tests because debugging C code run via Perl scripts is as much fun as debugging C code run via Python scripts. And it will reduce the time this very expensive job takes, which is a big benefit.
---
ci(linux-asan-ubsan): let's save some time
I often look at failed CI runs, and the linux-asan-ubsan job comes up frequently, and painfully (because it takes such a long time that re-running is often less desirable than getting the CI runs to pass).
This commit is an attempt to reduce the pain and suffering stemming from this particular job, simply by deciding that the benefit of running the Python/Subversion-related tests in that job is far outweighed by its cost.
This commit not only reduces the number of git-p4 flakes in linux-asan-ubsan to 0, it also seems to shave off about 10 minutes runtime, comparing
https://github.com/gitgitgadget/git/actions/runs/5929602548/job/16077585391
to
https://github.com/gitgitgadget/git/actions/runs/6010305446/job/16301473243?pr=1578
(which is not quite scientific due to the lack of a controlled environment, but it's the best we got for now). Together, those benefits form a strong incentive for me to get this merged.
This patch is based on maint.
Changes since v1:
* Made the rationale clearer (it is not Python that flakes, but Perforce).
* Touched up the commit message.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1578%2Fdscho%2Fskip-p4-from-asan-runs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1578/dscho/skip-p4-from-asan-runs-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1578
Range-diff vs v1:
1: ac5b82ea934 ! 1: 1927a97667b ci(linux-asan-ubsan): let's save some time
@@ Commit message
better in other tests anyway, as debugging C code run via Python scripts can get a bit hairy.
- In fact, it is not even git-p4 that is the problem (even if it flakes
- often enough to be problematic in the CI builds), but really the part
- about Python scripts. So let's just skip any Python parts of the tests
- from being run in that job.
+ In fact, it is not even just git-p4 that is the problem (even if it
+ flakes often enough to be problematic in the CI builds), but really the
+ part about Python scripts. So let's just skip any Python parts of the
+ tests from being run in that job.
For good measure, also skip the Subversion tests because debugging C code run via Perl scripts is as much fun as debugging C code run via
@@ ci/lib.sh: linux-leaks)
;;
linux-asan-ubsan)
export SANITIZE=address,undefined
-+ export NO_SVN_TESTS=LetsSaveSomeTimeBack
-+ MAKEFLAGS="$MAKEFLAGS NO_PYTHON=YepItFlakesTooOften"
++ export NO_SVN_TESTS=LetsSaveSomeTime
++ MAKEFLAGS="$MAKEFLAGS NO_PYTHON=YepBecauseP4FlakesTooOften"
;;
esac
ci/lib.sh | 2 ++ 1 file changed, 2 insertions(+)
ci/lib.sh +2 -0
@@ -280,6 +280,8 @@ linux-leaks) ;; linux-asan-ubsan) export SANITIZE=address,undefined + export NO_SVN_TESTS=LetsSaveSomeTime + MAKEFLAGS="$MAKEFLAGS NO_PYTHON=YepBecauseP4FlakesTooOften" ;; esac
base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 11 lines · writes:
...
In fact, it is not even justgit-p4that is the problem (even if it flakes often enough to be problematic in the CI builds), but really the part about Python scripts. So let's just skip any Python parts of the tests from being run in that job.For good measure, also skip the Subversion tests because debugging C code run via Perl scripts is as much fun as debugging C code run via Python scripts. And it will reduce the time this very expensive job takes, which is a big benefit.
Quoted 8 lines
Signed-off-by Johannes Schindelin <[email protected]>---
...
Changes since v1:* Made the rationale clearer (it is not Python that flakes, but Perforce).
* Touched up the commit message.
Thanks. Merged to 'next'.
Quoted 4 lines · Johannes Schindelin via GitGitGadget
This commit is an attempt to reduce the pain and suffering stemming from this particular job, simply by deciding that the benefit of running the Python/Subversion-related tests in that job is far outweighed by its cost.
FWIW, I am in favor of this patch. I've had the same thought many times but was worried I was being too dismissive of p4/svn (which I personally do not care at all about). Omitting them from the sanitizer job (but still running the basic svn/p4 tests elsewhere) seems like a good compromise.
-Peff
Quoted 7 lines · Jeff King
Quoted 4 lines · Johannes Schindelin via GitGitGadget
This commit is an attempt to reduce the pain and suffering stemming from this particular job, simply by deciding that the benefit of running the Python/Subversion-related tests in that job is far outweighed by its cost.
Quoted 5 lines
FWIW, I am in favor of this patch. I've had the same thought many times but was worried I was being too dismissive of p4/svn (which I personally do not care at all about). Omitting them from the sanitizer job (but still running the basic svn/p4 tests elsewhere) seems like a good compromise.
Exactly my feeling. Thanks, both.