* [PATCH] ci(linux-asan-ubsan): let's save some time
@ 2023-08-29 10:29 Johannes Schindelin via GitGitGadget
2023-08-29 16:49 ` Junio C Hamano
2023-08-29 20:47 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
0 siblings, 2 replies; 6+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2023-08-29 10:29 UTC (permalink / raw
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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(+)
diff --git a/ci/lib.sh b/ci/lib.sh
index 369d462f130..8e4e6713344 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ci(linux-asan-ubsan): let's save some time
2023-08-29 10:29 [PATCH] ci(linux-asan-ubsan): let's save some time Johannes Schindelin via GitGitGadget
@ 2023-08-29 16:49 ` Junio C Hamano
2023-08-29 20:47 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2023-08-29 16:49 UTC (permalink / raw
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> 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.
> ...
> 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.
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 369d462f130..8e4e6713344 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] ci(linux-asan-ubsan): let's save some time
2023-08-29 10:29 [PATCH] ci(linux-asan-ubsan): let's save some time Johannes Schindelin via GitGitGadget
2023-08-29 16:49 ` Junio C Hamano
@ 2023-08-29 20:47 ` Johannes Schindelin via GitGitGadget
2023-08-29 21:51 ` Junio C Hamano
2023-08-30 0:28 ` Jeff King
1 sibling, 2 replies; 6+ messages in thread
From: Johannes Schindelin via GitGitGadget @ 2023-08-29 20:47 UTC (permalink / raw
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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(+)
diff --git a/ci/lib.sh b/ci/lib.sh
index 369d462f130..6fbb5bade12 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -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
--
gitgitgadget
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ci(linux-asan-ubsan): let's save some time
2023-08-29 20:47 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
@ 2023-08-29 21:51 ` Junio C Hamano
2023-08-30 0:28 ` Jeff King
1 sibling, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2023-08-29 21:51 UTC (permalink / raw
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> ...
> 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.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> ...
> Changes since v1:
>
> * Made the rationale clearer (it is not Python that flakes, but
> Perforce).
> * Touched up the commit message.
Thanks. Merged to 'next'.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ci(linux-asan-ubsan): let's save some time
2023-08-29 20:47 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2023-08-29 21:51 ` Junio C Hamano
@ 2023-08-30 0:28 ` Jeff King
2023-08-30 6:19 ` Junio C Hamano
1 sibling, 1 reply; 6+ messages in thread
From: Jeff King @ 2023-08-30 0:28 UTC (permalink / raw
To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin
On Tue, Aug 29, 2023 at 08:47:28PM +0000, Johannes Schindelin via GitGitGadget wrote:
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] ci(linux-asan-ubsan): let's save some time
2023-08-30 0:28 ` Jeff King
@ 2023-08-30 6:19 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2023-08-30 6:19 UTC (permalink / raw
To: Jeff King; +Cc: Johannes Schindelin via GitGitGadget, git, Johannes Schindelin
Jeff King <peff@peff.net> writes:
> On Tue, Aug 29, 2023 at 08:47:28PM +0000, Johannes Schindelin via GitGitGadget wrote:
>
>> 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.
Exactly my feeling. Thanks, both.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-30 6:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-29 10:29 [PATCH] ci(linux-asan-ubsan): let's save some time Johannes Schindelin via GitGitGadget
2023-08-29 16:49 ` Junio C Hamano
2023-08-29 20:47 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2023-08-29 21:51 ` Junio C Hamano
2023-08-30 0:28 ` Jeff King
2023-08-30 6:19 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).