ci(linux-asan-ubsan): let's save some time

[dscho]

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.

cc: Jeff King peff@peff.net

[dscho]

/submit

[gitgitgadget]

Submitted as pull.1578.git.1693304963963.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1578/dscho/skip-p4-from-asan-runs-v1

To fetch this version to local tag pr-1578/dscho/skip-p4-from-asan-runs-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1578/dscho/skip-p4-from-asan-runs-v1

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"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.

[dscho]

/submit

[gitgitgadget]

Submitted as pull.1578.v2.git.1693342048633.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1578/dscho/skip-p4-from-asan-runs-v2

To fetch this version to local tag pr-1578/dscho/skip-p4-from-asan-runs-v2:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1578/dscho/skip-p4-from-asan-runs-v2

[gitgitgadget]

This branch is now known as js/ci-san-skip-p4-and-svn-tests.

[gitgitgadget]

This patch series was integrated into seen via git@a0d1e37.

[gitgitgadget]

This patch series was integrated into next via git@9617f99.

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

"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'.

[gitgitgadget]

This patch series was integrated into seen via git@9210b31.

[gitgitgadget]

On the Git mailing list, Jeff King wrote (reply to this):

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

[gitgitgadget]

User Jeff King <peff@peff.net> has been added to the cc: list.

[gitgitgadget]

On the Git mailing list, Junio C Hamano wrote (reply to this):

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.

[gitgitgadget]

This patch series was integrated into seen via git@cdf70b1.

[gitgitgadget]

This patch series was integrated into seen via git@4e5820a.

[gitgitgadget]

There was a status update in the "New Topics" section about the branch js/ci-san-skip-p4-and-svn-tests on the Git mailing list:

Flakey "git p4" tests, as well as "git svn" tests, are now skipped
in the (rather expensive) sanitizer CI job.

Will merge to 'master'.
source: <pull.1578.v2.git.1693342048633.gitgitgadget@gmail.com>

[gitgitgadget]

This patch series was integrated into seen via git@e5d41be.

[gitgitgadget]

This patch series was integrated into seen via git@3e2b0c2.

[gitgitgadget]

This patch series was integrated into master via git@3e2b0c2.

[gitgitgadget]

This patch series was integrated into next via git@3e2b0c2.

[gitgitgadget]

Closed via 3e2b0c2.