[PATCH 0/2] Use fixed github-actions runner image

[PATCH 0/2] Use fixed github-actions runner image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

Today I found lots of CI errors of my private host git repository
on GitHub. It is because the runner image "ubuntu-latest" of the
CI jobs is ubuntu 22.04 (jammy) instead of ubuntu 20.04 (focal).

The upgrade of runner images is in progress, and my public forked
repository still use 20.04 (focal) as "ubuntu-latest".

Two patches in this series try to protect our CI in advance.

New CI instance see below:

 * https://github.com/jiangxin/git/actions/runs/3532978329

--

Jiang Xin (2):
  github-actions: run gcc-8 on ubuntu-20.04 image
  ci: upgrade version of p4

 .github/workflows/main.yml | 16 ++++++++++++----
 ci/install-dependencies.sh | 10 +++++-----
 ci/lib.sh                  |  8 ++++----
 3 files changed, 21 insertions(+), 13 deletions(-)

-- 
2.39.0.rc0

[PATCH 1/2] github-actions: run gcc-8 on ubuntu-20.04 image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

GitHub starts to upgrade its runner image "ubuntu-latest" from version
"ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
install "gcc-8" package on the new runner image.

Change the runner images from "ubuntu-latest" to "ubuntu-20.04" in order
to run with "gcc-8" as a dependency.

Instead of use the environment "$runs_on_pool" as below:

    case "$runs_on_pool" in
    ubuntu-20.04 | ubuntu-latest)
	;;

we can reuse the os field in the matrix, and use a new environment
"$runs_on_os" as below:

    case "$runs_on_os" in
    ubuntu)
        ;;

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 .github/workflows/main.yml | 16 ++++++++++++----
 ci/install-dependencies.sh |  6 +++---
 ci/lib.sh                  |  6 +++---
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 592f9193a8..da0d8ab0bf 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -224,6 +224,7 @@ jobs:
         vector:
           - jobname: linux-clang
             cc: clang
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-sha256
             cc: clang
@@ -232,36 +233,43 @@ jobs:
           - jobname: linux-gcc
             cc: gcc
             cc_package: gcc-8
-            pool: ubuntu-latest
+            os: ubuntu
+            pool: ubuntu-20.04
           - jobname: linux-TEST-vars
             cc: gcc
-            os: ubuntu
             cc_package: gcc-8
-            pool: ubuntu-latest
+            os: ubuntu
+            pool: ubuntu-20.04
           - jobname: osx-clang
             cc: clang
+            os: macos
             pool: macos-latest
           - jobname: osx-gcc
             cc: gcc
             cc_package: gcc-9
+            os: macos
             pool: macos-latest
           - jobname: linux-gcc-default
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-leaks
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-asan
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-ubsan
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
-      runs_on_pool: ${{matrix.vector.pool}}
+      runs_on_os: ${{matrix.vector.os}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v2
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 107757a1fe..f639263a62 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,8 +11,8 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$runs_on_pool" in
-ubuntu-latest)
+case "$runs_on_os" in
+ubuntu)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
 		$UBUNTU_COMMON_PKGS $CC_PACKAGE
@@ -30,7 +30,7 @@ ubuntu-latest)
 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
 	popd
 	;;
-macos-latest)
+macos)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
 	# brew install gnu-time
diff --git a/ci/lib.sh b/ci/lib.sh
index 24d20a5d64..0c0767d354 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -225,8 +225,8 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$runs_on_pool" in
-ubuntu-latest)
+case "$runs_on_os" in
+ubuntu)
 	if test "$jobname" = "linux-gcc-default"
 	then
 		break
@@ -253,7 +253,7 @@ ubuntu-latest)
 	GIT_LFS_PATH="$HOME/custom/git-lfs"
 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
-macos-latest)
+macos)
 	if [ "$jobname" = osx-gcc ]
 	then
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
-- 
2.39.0.rc0

[PATCH 2/2] ci: upgrade version of p4

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

There would be a segmentation fault when running p4 v16.2 on ubuntu
22.04 which is the latest version of ubuntu runner image for github
actions. Upgrade p4 from version 16.2 to 19.2 will fix this issue.

Also add some instructions to show errors of command "p4 -V", so we can
see why the output doesn't match.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 4 ++--
 ci/lib.sh                  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index f639263a62..291e49bdde 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -83,9 +83,9 @@ esac
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
-	p4d -V | grep Rev.
+	p4d -V | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
 	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
-	p4 -V | grep Rev.
+	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
diff --git a/ci/lib.sh b/ci/lib.sh
index 0c0767d354..8474b0f249 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -246,7 +246,7 @@ ubuntu)
 	# were recorded in the Homebrew database upon creating the OS X
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
-	export LINUX_P4_VERSION="16.2"
+	export LINUX_P4_VERSION="19.2"
 	export LINUX_GIT_LFS_VERSION="1.5.2"
 
 	P4_PATH="$HOME/custom/p4"
-- 
2.39.0.rc0

Re: [PATCH 1/2] github-actions: run gcc-8 on ubuntu-20.04 image

[Johannes Schindelin]

Hi,

On Wed, 23 Nov 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> GitHub starts to upgrade its runner image "ubuntu-latest" from version
> "ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
> install "gcc-8" package on the new runner image.
>
> Change the runner images from "ubuntu-latest" to "ubuntu-20.04" in order
> to run with "gcc-8" as a dependency.
>
> Instead of use the environment "$runs_on_pool" as below:
>
>     case "$runs_on_pool" in
>     ubuntu-20.04 | ubuntu-latest)
> 	;;
>
> we can reuse the os field in the matrix, and use a new environment
> "$runs_on_os" as below:
>
>     case "$runs_on_os" in
>     ubuntu)
>         ;;

While this is a bit more refactoring than I usually recommend when trying
to fix an urgent bug, the outcome does look nice and correct.

Here is my

	Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Thank you for working on this!
Dscho

>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  .github/workflows/main.yml | 16 ++++++++++++----
>  ci/install-dependencies.sh |  6 +++---
>  ci/lib.sh                  |  6 +++---
>  3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 592f9193a8..da0d8ab0bf 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -224,6 +224,7 @@ jobs:
>          vector:
>            - jobname: linux-clang
>              cc: clang
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-sha256
>              cc: clang
> @@ -232,36 +233,43 @@ jobs:
>            - jobname: linux-gcc
>              cc: gcc
>              cc_package: gcc-8
> -            pool: ubuntu-latest
> +            os: ubuntu
> +            pool: ubuntu-20.04
>            - jobname: linux-TEST-vars
>              cc: gcc
> -            os: ubuntu
>              cc_package: gcc-8
> -            pool: ubuntu-latest
> +            os: ubuntu
> +            pool: ubuntu-20.04
>            - jobname: osx-clang
>              cc: clang
> +            os: macos
>              pool: macos-latest
>            - jobname: osx-gcc
>              cc: gcc
>              cc_package: gcc-9
> +            os: macos
>              pool: macos-latest
>            - jobname: linux-gcc-default
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-leaks
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-asan
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-ubsan
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>      env:
>        CC: ${{matrix.vector.cc}}
>        CC_PACKAGE: ${{matrix.vector.cc_package}}
>        jobname: ${{matrix.vector.jobname}}
> -      runs_on_pool: ${{matrix.vector.pool}}
> +      runs_on_os: ${{matrix.vector.os}}
>      runs-on: ${{matrix.vector.pool}}
>      steps:
>      - uses: actions/checkout@v2
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 107757a1fe..f639263a62 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -11,8 +11,8 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
>   tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
>   libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
>
> -case "$runs_on_pool" in
> -ubuntu-latest)
> +case "$runs_on_os" in
> +ubuntu)
>  	sudo apt-get -q update
>  	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
>  		$UBUNTU_COMMON_PKGS $CC_PACKAGE
> @@ -30,7 +30,7 @@ ubuntu-latest)
>  		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
>  	popd
>  	;;
> -macos-latest)
> +macos)
>  	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
>  	# Uncomment this if you want to run perf tests:
>  	# brew install gnu-time
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 24d20a5d64..0c0767d354 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -225,8 +225,8 @@ export DEFAULT_TEST_TARGET=prove
>  export GIT_TEST_CLONE_2GB=true
>  export SKIP_DASHED_BUILT_INS=YesPlease
>
> -case "$runs_on_pool" in
> -ubuntu-latest)
> +case "$runs_on_os" in
> +ubuntu)
>  	if test "$jobname" = "linux-gcc-default"
>  	then
>  		break
> @@ -253,7 +253,7 @@ ubuntu-latest)
>  	GIT_LFS_PATH="$HOME/custom/git-lfs"
>  	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
>  	;;
> -macos-latest)
> +macos)
>  	if [ "$jobname" = osx-gcc ]
>  	then
>  		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
> --
> 2.39.0.rc0
>
>

Re: [PATCH 2/2] ci: upgrade version of p4

[Johannes Schindelin]

Hi,

On Wed, 23 Nov 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> There would be a segmentation fault when running p4 v16.2 on ubuntu
> 22.04 which is the latest version of ubuntu runner image for github
> actions. Upgrade p4 from version 16.2 to 19.2 will fix this issue.

I was about to embark on a debugging session when I had the splendid idea
to look at the Git mailing list archives to see whether anybody else had
encountered that particular problem.

However, when I pushed a branch with this fix, it still segfaulted even
when downloading version 19.2 (link:
https://github.com/dscho/git/actions/runs/3538788474/jobs/5939977231#step:3:387):

 + wget --quiet https://cdist2.perforce.com/perforce/r19.2/bin.linux26x86_64/p4d
 + wget --quiet https://cdist2.perforce.com/perforce/r19.2/bin.linux26x86_64/p4
 + [...]
 + echo 'Perforce Server Version'
 Perforce Server Version
 + p4d -V
 + grep Rev.
 + echo 'p4d: bad version'
 p4d: bad version
 + p4d -V
 ci/install-dependencies.sh: line 91:  3051 Segmentation fault      (core
 dumped) p4d -V

I guess I'll embark on that debugging session after all ;-)

> Also add some instructions to show errors of command "p4 -V", so we can
> see why the output doesn't match.

This is an excellent addition. If I read the logs correctly, this change
adds the error message "Segmentation fault" to the output, which is really
helpful.

Thank you for working on this!
Dscho

>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/install-dependencies.sh | 4 ++--
>  ci/lib.sh                  | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index f639263a62..291e49bdde 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -83,9 +83,9 @@ esac
>  if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
>  then
>  	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
> -	p4d -V | grep Rev.
> +	p4d -V | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
>  	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
> -	p4 -V | grep Rev.
> +	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
>  else
>  	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
>  fi
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 0c0767d354..8474b0f249 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -246,7 +246,7 @@ ubuntu)
>  	# were recorded in the Homebrew database upon creating the OS X
>  	# image.
>  	# Keep that in mind when you encounter a broken OS X build!
> -	export LINUX_P4_VERSION="16.2"
> +	export LINUX_P4_VERSION="19.2"
>  	export LINUX_GIT_LFS_VERSION="1.5.2"
>
>  	P4_PATH="$HOME/custom/p4"
> --
> 2.39.0.rc0
>
>

Re: [PATCH 0/2] Use fixed github-actions runner image

[Johannes Schindelin]

Hi,

On Wed, 23 Nov 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> Today I found lots of CI errors of my private host git repository
> on GitHub. It is because the runner image "ubuntu-latest" of the
> CI jobs is ubuntu 22.04 (jammy) instead of ubuntu 20.04 (focal).

Excellent timing! This is indeed what I, too, ran into when adjusting
microsoft/git to v2.39.0-rc0. Here are the relevant links for the failing
and the succeeding runs, for the very same commit, but in different
repositories and with different `ubuntu-*` images:

Failing (ubuntu-22.04):
https://github.com/dscho/git/actions/runs/3533530319/jobs/5939252441#step:1:8

Succeeding (ubuntu-20.04):
https://github.com/microsoft/git/actions/runs/3532815597/jobs/5927646267#step:1:9

Having said that, I applied your fixes on top and pushed out a test
branch, but `p4d` still segfaults for me:

https://github.com/dscho/git/actions/runs/3538788474/jobs/5939977231#step:3:387

I hope that we can get this fixed very soon and am _very much_ in favor of
fast-tracking the patches once we confirm that they fix the CI builds.

Thanks,
Dscho

> The upgrade of runner images is in progress, and my public forked
> repository still use 20.04 (focal) as "ubuntu-latest".
>
> Two patches in this series try to protect our CI in advance.
>
> New CI instance see below:
>
>  * https://github.com/jiangxin/git/actions/runs/3532978329
>
> --
>
> Jiang Xin (2):
>   github-actions: run gcc-8 on ubuntu-20.04 image
>   ci: upgrade version of p4
>
>  .github/workflows/main.yml | 16 ++++++++++++----
>  ci/install-dependencies.sh | 10 +++++-----
>  ci/lib.sh                  |  8 ++++----
>  3 files changed, 21 insertions(+), 13 deletions(-)
>
> --
> 2.39.0.rc0
>
>

Re: [PATCH 2/2] ci: upgrade version of p4

[Johannes Schindelin]

Hi,

On Thu, 24 Nov 2022, Johannes Schindelin wrote:

> On Wed, 23 Nov 2022, Jiang Xin wrote:
>
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > There would be a segmentation fault when running p4 v16.2 on ubuntu
> > 22.04 which is the latest version of ubuntu runner image for github
> > actions. Upgrade p4 from version 16.2 to 19.2 will fix this issue.
>
> I was about to embark on a debugging session when I had the splendid idea
> to look at the Git mailing list archives to see whether anybody else had
> encountered that particular problem.
>
> However, when I pushed a branch with this fix, it still segfaulted even
> when downloading version 19.2 (link:
> https://github.com/dscho/git/actions/runs/3538788474/jobs/5939977231#step:3:387):
>
>  + wget --quiet https://cdist2.perforce.com/perforce/r19.2/bin.linux26x86_64/p4d
>  + wget --quiet https://cdist2.perforce.com/perforce/r19.2/bin.linux26x86_64/p4
>  + [...]
>  + echo 'Perforce Server Version'
>  Perforce Server Version
>  + p4d -V
>  + grep Rev.
>  + echo 'p4d: bad version'
>  p4d: bad version
>  + p4d -V
>  ci/install-dependencies.sh: line 91:  3051 Segmentation fault      (core
>  dumped) p4d -V
>
> I guess I'll embark on that debugging session after all ;-)

And I did. It turns out that r22.2 works both on ubuntu-20.04 and
ubuntu-22.04 (I locally verified for the former, proof for the latter:
https://github.com/dscho/git/actions/runs/3538941550/jobs/5940295721#step:3:384).
So I propose this fixup:

-- snip --
From 81e2d9a4f6ad2d2d9df27edfb666fe2112bdef57 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 24 Nov 2022 09:31:41 +0100
Subject: [PATCH] amend! ci: upgrade version of p4

ci: upgrade version of p4

There would be a segmentation fault when running p4 v16.2 on ubuntu
22.04 which is the latest version of ubuntu runner image for github
actions. Upgrade p4 from version 16.2 to 22.2 will fix this issue.

Also add some instructions to show errors of command "p4 -V", so we can
see why the output doesn't match.

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/lib.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ci/lib.sh b/ci/lib.sh
index 6c658fa21227..eaa75ab3c07d 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -252,7 +252,7 @@ ubuntu)
 	# were recorded in the Homebrew database upon creating the OS X
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
-	export LINUX_P4_VERSION="19.2"
+	export LINUX_P4_VERSION="22.2"
 	export LINUX_GIT_LFS_VERSION="1.5.2"

 	P4_PATH="$HOME/custom/p4"
-- snap --

If you squash this in, please feel free to also use the updated commit
message.

Thank you,
Dscho

Re: [PATCH 2/2] ci: upgrade version of p4

[Johannes Schindelin]

Me again,

On Thu, 24 Nov 2022, Johannes Schindelin wrote:

> On Thu, 24 Nov 2022, Johannes Schindelin wrote:
>
> > On Wed, 23 Nov 2022, Jiang Xin wrote:
> >
> > > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> > >
> > > There would be a segmentation fault when running p4 v16.2 on ubuntu
> > > 22.04 which is the latest version of ubuntu runner image for github
> > > actions. Upgrade p4 from version 16.2 to 19.2 will fix this issue.
> >
> > I was about to embark on a debugging session when I had the splendid idea
> > to look at the Git mailing list archives to see whether anybody else had
> > encountered that particular problem.
> >
> > However, when I pushed a branch with this fix, it still segfaulted even
> > when downloading version 19.2 (link:
> > https://github.com/dscho/git/actions/runs/3538788474/jobs/5939977231#step:3:387):
> >
> >  + wget --quiet https://cdist2.perforce.com/perforce/r19.2/bin.linux26x86_64/p4d
> >  + wget --quiet https://cdist2.perforce.com/perforce/r19.2/bin.linux26x86_64/p4
> >  + [...]
> >  + echo 'Perforce Server Version'
> >  Perforce Server Version
> >  + p4d -V
> >  + grep Rev.
> >  + echo 'p4d: bad version'
> >  p4d: bad version
> >  + p4d -V
> >  ci/install-dependencies.sh: line 91:  3051 Segmentation fault      (core
> >  dumped) p4d -V
> >
> > I guess I'll embark on that debugging session after all ;-)
>
> And I did. It turns out that r22.2 works both on ubuntu-20.04 and
> ubuntu-22.04 (I locally verified for the former, proof for the latter:
> https://github.com/dscho/git/actions/runs/3538941550/jobs/5940295721#step:3:384).
> So I propose this fixup:
>
> -- snip --
> From 81e2d9a4f6ad2d2d9df27edfb666fe2112bdef57 Mon Sep 17 00:00:00 2001
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Thu, 24 Nov 2022 09:31:41 +0100
> Subject: [PATCH] amend! ci: upgrade version of p4
>
> ci: upgrade version of p4
>
> There would be a segmentation fault when running p4 v16.2 on ubuntu
> 22.04 which is the latest version of ubuntu runner image for github
> actions. Upgrade p4 from version 16.2 to 22.2 will fix this issue.
>
> Also add some instructions to show errors of command "p4 -V", so we can
> see why the output doesn't match.
>
> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/lib.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 6c658fa21227..eaa75ab3c07d 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -252,7 +252,7 @@ ubuntu)
>  	# were recorded in the Homebrew database upon creating the OS X
>  	# image.
>  	# Keep that in mind when you encounter a broken OS X build!
> -	export LINUX_P4_VERSION="19.2"
> +	export LINUX_P4_VERSION="22.2"
>  	export LINUX_GIT_LFS_VERSION="1.5.2"
>
>  	P4_PATH="$HOME/custom/p4"
> -- snap --
>
> If you squash this in, please feel free to also use the updated commit
> message.

Hmm. Hold on. While `p4d` now no longer segfaults, it looks as if `git p4`
is completely broken (see
https://github.com/dscho/git/actions/runs/3538941550/jobs/5940295721#step:4:2005):

  failure: t9800.3 basic git p4 clone
  	git p4 clone --dest="$git" //depot &&
  	test_when_finished cleanup_git &&
  	(
  		cd "$git" &&
  		git log --oneline >lines &&
  		test_line_count = 1 lines
  	)

  + git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git //depot
  Perforce db files in '.' will be created if missing...
  fatal: 'p4' appears to be a git command, but we were not
  able to execute it. Maybe git-p4 is broken?
  error: last command exited with $?=128
  not ok 3 - basic git p4 clone
  #
  #		git p4 clone --dest="$git" //depot &&
  #		test_when_finished cleanup_git &&
  #		(
  #			cd "$git" &&
  #			git log --oneline >lines &&
  #			test_line_count = 1 lines
  #		)
  #

I guess I will keep digging,
Dscho

[PATCH v2 0/3] Fix broken CI on newer github-actions runner image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

GitHub CI runner image "ubuntu-latest" will upgrade to "ubuntu-22.04"
soon, and the CI runner image of my private host repository has
already been upgraded.

Our CI will break on new runner image because there is not "gcc-8"
package in ubuntu-22.04. See log of CI#2:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3537628107/jobs/5937769957

This issue is fixed in patch 1/3 by downgrade the version of the runner
image for jobs which need "gcc-8". But there are still some CI errors.
This is because p4/p4d version 16.2 cannot run on ubuntu-22.04. See this
log of CI#3:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3537650146/jobs/5937813922

This issue is fixed in patch 2/3 by upgrade p4 and p4d. But all p4
related test cases failed becasue python was missing on ubuntu-22.04.
See the log of CI#4:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3537695959/jobs/5937929695

This issue is fixed in patch 3/3 by install python2/python3 on ubutnu.

If install p4 version 22.2, will break several p4 related test cases,
see the log of CI#7:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3538795233/jobs/5939989823

So we choose p4 21.2, and the final successful log of CI#8 is below:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3538946849

--
Jiang Xin (3):
  github-actions: run gcc-8 on ubuntu-20.04 image
  ci: upgrade version of p4 to 21.2
  ci: install python on ubuntu

 .github/workflows/main.yml | 16 ++++++++++++----
 ci/install-dependencies.sh | 12 ++++++------
 ci/lib.sh                  | 10 ++++++----
 3 files changed, 24 insertions(+), 14 deletions(-)

-- 
2.39.0.rc0

[PATCH v2 1/3] github-actions: run gcc-8 on ubuntu-20.04 image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

GitHub starts to upgrade its runner image "ubuntu-latest" from version
"ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
install "gcc-8" package on the new runner image.

Change the runner images from "ubuntu-latest" to "ubuntu-20.04" in order
to run with "gcc-8" as a dependency.

Instead of use the environment "$runs_on_pool" as below:

    case "$runs_on_pool" in
    ubuntu-20.04 | ubuntu-latest)
	;;

we can reuse the os field in the matrix, and use a new environment
"$runs_on_os" as below:

    case "$runs_on_os" in
    ubuntu)
        ;;

In this way, we can change the "ubuntu-latest" runner image to any
version such as "ubuntu-22.04" to test CI behavior in advance.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 .github/workflows/main.yml | 16 ++++++++++++----
 ci/install-dependencies.sh |  6 +++---
 ci/lib.sh                  |  6 +++---
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 592f9193a8..da0d8ab0bf 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -224,6 +224,7 @@ jobs:
         vector:
           - jobname: linux-clang
             cc: clang
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-sha256
             cc: clang
@@ -232,36 +233,43 @@ jobs:
           - jobname: linux-gcc
             cc: gcc
             cc_package: gcc-8
-            pool: ubuntu-latest
+            os: ubuntu
+            pool: ubuntu-20.04
           - jobname: linux-TEST-vars
             cc: gcc
-            os: ubuntu
             cc_package: gcc-8
-            pool: ubuntu-latest
+            os: ubuntu
+            pool: ubuntu-20.04
           - jobname: osx-clang
             cc: clang
+            os: macos
             pool: macos-latest
           - jobname: osx-gcc
             cc: gcc
             cc_package: gcc-9
+            os: macos
             pool: macos-latest
           - jobname: linux-gcc-default
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-leaks
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-asan
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-ubsan
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
-      runs_on_pool: ${{matrix.vector.pool}}
+      runs_on_os: ${{matrix.vector.os}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v2
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 107757a1fe..f639263a62 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,8 +11,8 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$runs_on_pool" in
-ubuntu-latest)
+case "$runs_on_os" in
+ubuntu)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
 		$UBUNTU_COMMON_PKGS $CC_PACKAGE
@@ -30,7 +30,7 @@ ubuntu-latest)
 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
 	popd
 	;;
-macos-latest)
+macos)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
 	# brew install gnu-time
diff --git a/ci/lib.sh b/ci/lib.sh
index 24d20a5d64..0c0767d354 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -225,8 +225,8 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$runs_on_pool" in
-ubuntu-latest)
+case "$runs_on_os" in
+ubuntu)
 	if test "$jobname" = "linux-gcc-default"
 	then
 		break
@@ -253,7 +253,7 @@ ubuntu-latest)
 	GIT_LFS_PATH="$HOME/custom/git-lfs"
 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
-macos-latest)
+macos)
 	if [ "$jobname" = osx-gcc ]
 	then
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
-- 
2.39.0.rc0

[PATCH v2 2/3] ci: upgrade version of p4 to 21.2

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

There would be a segmentation fault when running p4 v16.2 on ubuntu
22.04 which is the latest version of ubuntu runner image for github
actions.

By checking each version from [1], p4d version 21.1 and above can work
properly on ubuntu 22.04. But version 22.x will break some p4 test
cases. So p4 version 21.x is exactly the version we can use.

In addition to upgrade p4 from version 16.2 to 21.2, also add some
instructions to show errors of command "p4 -V", so we can see why the
command output doesn't match.

[1]: https://cdist2.perforce.com/perforce/

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 4 ++--
 ci/lib.sh                  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index f639263a62..291e49bdde 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -83,9 +83,9 @@ esac
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
-	p4d -V | grep Rev.
+	p4d -V | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
 	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
-	p4 -V | grep Rev.
+	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
diff --git a/ci/lib.sh b/ci/lib.sh
index 0c0767d354..a618d66b81 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -246,7 +246,7 @@ ubuntu)
 	# were recorded in the Homebrew database upon creating the OS X
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
-	export LINUX_P4_VERSION="16.2"
+	export LINUX_P4_VERSION="21.2"
 	export LINUX_GIT_LFS_VERSION="1.5.2"
 
 	P4_PATH="$HOME/custom/p4"
-- 
2.39.0.rc0

[PATCH v2 3/3] ci: install python on ubuntu

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

Python is missing from the default ubuntu-22.04 runner image, which
prevent git-p4 from working. To install python on ubuntu, we need to
provide correct package name:

 * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the
   "python" package, and "/usr/bin/python3" is provided by the "python3"
   package.

 * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by
   the "python2" package which has a different name from bionic, and
   "/usr/bin/python3" is provided by "python3".

Since the "ubuntu-latest" runner image has a higher version, so its safe
to use "python2" or "python3" package name.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 291e49bdde..e28d93a154 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -15,7 +15,7 @@ case "$runs_on_os" in
 ubuntu)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS $CC_PACKAGE
+		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
diff --git a/ci/lib.sh b/ci/lib.sh
index a618d66b81..ebe702e0ea 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -235,8 +235,10 @@ ubuntu)
 	if [ "$jobname" = linux-gcc ]
 	then
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
+		PYTHON_PACKAGE=python3
 	else
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
+		PYTHON_PACKAGE=python2
 	fi
 
 	export GIT_TEST_HTTPD=true
-- 
2.39.0.rc0

Re: [PATCH 2/2] ci: upgrade version of p4

[Jiang Xin]

On Thu, Nov 24, 2022 at 4:16 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > There would be a segmentation fault when running p4 v16.2 on ubuntu
> > 22.04 which is the latest version of ubuntu runner image for github
> > actions. Upgrade p4 from version 16.2 to 19.2 will fix this issue.
>
> I was about to embark on a debugging session when I had the splendid idea
> to look at the Git mailing list archives to see whether anybody else had
> encountered that particular problem.
>
> However, when I pushed a branch with this fix, it still segfaulted even
> when downloading version 19.2 (link:
> https://github.com/dscho/git/actions/runs/3538788474/jobs/5939977231#step:3:387):

Yes, p4d 19.2 also has segfault errors, and p4 version 21.x and above
is OK.  Maybe yesterday I only check p4, and forgot to check p4d by
hands.

It is fixed in v2 by upgrade p4 to 21.2.

--
Jiang Xin

Re: [PATCH 2/2] ci: upgrade version of p4

[Jiang Xin]

On Thu, Nov 24, 2022 at 4:54 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Me again,
> Hmm. Hold on. While `p4d` now no longer segfaults, it looks as if `git p4`
> is completely broken (see
> https://github.com/dscho/git/actions/runs/3538941550/jobs/5940295721#step:4:2005):
>
>   failure: t9800.3 basic git p4 clone
>         git p4 clone --dest="$git" //depot &&
>         test_when_finished cleanup_git &&
>         (
>                 cd "$git" &&
>                 git log --oneline >lines &&
>                 test_line_count = 1 lines
>         )
>
>   + git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git //depot
>   Perforce db files in '.' will be created if missing...
>   fatal: 'p4' appears to be a git command, but we were not
>   able to execute it. Maybe git-p4 is broken?
>   error: last command exited with $?=128
>   not ok 3 - basic git p4 clone
>   #
>   #             git p4 clone --dest="$git" //depot &&
>   #             test_when_finished cleanup_git &&
>   #             (
>   #                     cd "$git" &&
>   #                     git log --oneline >lines &&
>   #                     test_line_count = 1 lines
>   #             )
>   #
>
> I guess I will keep digging,
> Dscho

I found this is because python is not installed by default on
ubuntu-22.04. This issue is fix in patch 3/3 of reroll v2.

--
Jiang Xin

Re: [PATCH 2/2] ci: upgrade version of p4

[Johannes Schindelin]

Hi,

On Thu, 24 Nov 2022, Jiang Xin wrote:

> On Thu, Nov 24, 2022 at 4:54 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Me again,
> > Hmm. Hold on. While `p4d` now no longer segfaults, it looks as if `git p4`
> > is completely broken (see
> > https://github.com/dscho/git/actions/runs/3538941550/jobs/5940295721#step:4:2005):
> >
> >   failure: t9800.3 basic git p4 clone
> >         git p4 clone --dest="$git" //depot &&
> >         test_when_finished cleanup_git &&
> >         (
> >                 cd "$git" &&
> >                 git log --oneline >lines &&
> >                 test_line_count = 1 lines
> >         )
> >
> >   + git p4 clone --dest=/home/runner/work/git/git/t/trash directory.t9800-git-p4-basic/git //depot
> >   Perforce db files in '.' will be created if missing...
> >   fatal: 'p4' appears to be a git command, but we were not
> >   able to execute it. Maybe git-p4 is broken?
> >   error: last command exited with $?=128
> >   not ok 3 - basic git p4 clone
> >   #
> >   #             git p4 clone --dest="$git" //depot &&
> >   #             test_when_finished cleanup_git &&
> >   #             (
> >   #                     cd "$git" &&
> >   #                     git log --oneline >lines &&
> >   #                     test_line_count = 1 lines
> >   #             )
> >   #
> >
> > I guess I will keep digging,
> > Dscho
>
> I found this is because python is not installed by default on
> ubuntu-22.04. This issue is fix in patch 3/3 of reroll v2.

Excellent find!

I was pulling out my hair because I could not reproduce the issue, but
then I did not override `PYTHON_PATH` in my build and therefore the
already-installed python3 was used.

FWIW I confirm that this fixes the build. I restricted the build to the
necessary minimum (and threw in an `action-tmate` step for more efficient
debugging) and it passed the p4 tests:
https://github.com/dscho/git/actions/runs/3539338333/jobs/5941122554

Thank you so much!
Dscho

Re: [PATCH v2 0/3] Fix broken CI on newer github-actions runner image

[Johannes Schindelin]

Hi,

On Thu, 24 Nov 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> GitHub CI runner image "ubuntu-latest" will upgrade to "ubuntu-22.04"
> soon, and the CI runner image of my private host repository has
> already been upgraded.
>
> Our CI will break on new runner image because there is not "gcc-8"
> package in ubuntu-22.04. See log of CI#2:
>
>  * https://github.com/jiangxin/git-ci-test/actions/runs/3537628107/jobs/5937769957
>
> This issue is fixed in patch 1/3 by downgrade the version of the runner
> image for jobs which need "gcc-8". But there are still some CI errors.
> This is because p4/p4d version 16.2 cannot run on ubuntu-22.04. See this
> log of CI#3:
>
>  * https://github.com/jiangxin/git-ci-test/actions/runs/3537650146/jobs/5937813922
>
> This issue is fixed in patch 2/3 by upgrade p4 and p4d. But all p4
> related test cases failed becasue python was missing on ubuntu-22.04.
> See the log of CI#4:
>
>  * https://github.com/jiangxin/git-ci-test/actions/runs/3537695959/jobs/5937929695
>
> This issue is fixed in patch 3/3 by install python2/python3 on ubutnu.
>
> If install p4 version 22.2, will break several p4 related test cases,
> see the log of CI#7:
>
>  * https://github.com/jiangxin/git-ci-test/actions/runs/3538795233/jobs/5939989823
>
> So we choose p4 21.2, and the final successful log of CI#8 is below:
>
>  * https://github.com/jiangxin/git-ci-test/actions/runs/3538946849

ACK!

I verified that this patch series does the job, by applying it on top of
`microsoft/git`'s tentative rebase to v2.39.0-rc0:
https://github.com/dscho/git/actions/runs/3539338333/jobs/5941122554

This run was restricted to `linux-clang` and the p4 tests. I care a lot
about not using more resources than one's fair share, therefore I wanted
to avoid using build minutes unnecessarily during my debuging.

To build even more confidence in the patch series, I will now start a full
run (which will take *a lot* of build minutes, unfortunately).

Thank you so much!
Dscho

Re: [PATCH v2 1/3] github-actions: run gcc-8 on ubuntu-20.04 image

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 24 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> GitHub starts to upgrade its runner image "ubuntu-latest" from version
> "ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
> install "gcc-8" package on the new runner image.
>
> Change the runner images from "ubuntu-latest" to "ubuntu-20.04" in order
> to run with "gcc-8" as a dependency.

I very much like this direction, as we shouldn't have to scramble to
update our CI every time GitHub switches defaults, and by using "latest"
in general we also break e.g. re-pushes or re-rolls of outstanding topics/PRs.

I.e. you'll push today, it'll work, tomorrow the "latest" image has
changed and your CI breaks, but through no fault of the code you're
testing.

But per previous discussion on this exact topic at least Junio & Derrick
(CC'd) disagreed with going in that direction, see
https://lore.kernel.org/git/220825.865yig4bd7.gmgdl@evledraar.gmail.com/
(and the E-Mail it links to).

Now, I think that if we have some notification so we'll update the
"pinned" image sooner than later it'll address the concerns they had,
but do we have that? I've seen notices from GitHub CI about e.g. some
things in the main.yml file being deprecated, will we get those for
these "pinned" images too in a timely manner?

Because while I think a non-pinned "latest" sucks because it forces us
to scramble with updates like this, having a "pinned" image go away
entirely (because nobody noticed we should update it sooner-than-later)
would suck more.

> Instead of use the environment "$runs_on_pool" as below:
>
>     case "$runs_on_pool" in
>     ubuntu-20.04 | ubuntu-latest)
> 	;;
>
> we can reuse the os field in the matrix, and use a new environment
> "$runs_on_os" as below:
>
>     case "$runs_on_os" in
>     ubuntu)
>         ;;

I think this is probably a good change, as we won't need to
search-replace the image name in as many places in the future.

But let's have this as a seperate change. Here you're just refactoring
the selection behavior of the main.yml + CI lib, it doesn't need to be
bundled up with the change to change the target image.

Re: [PATCH v2 0/3] Fix broken CI on newer github-actions runner image

[Johannes Schindelin]

Hi,

On Thu, 24 Nov 2022, Johannes Schindelin wrote:

> To build even more confidence in the patch series, I will now start a full
> run (which will take *a lot* of build minutes, unfortunately).

And it passed: https://github.com/dscho/git/actions/runs/3539451056

I also had a look at the range-diff between v1 and v2:

-- snip --
1:  ef80c39de1e5 ! 1:  6d4607a4ee46 github-actions: run gcc-8 on ubuntu-20.04 image
    @@ Commit message
             ubuntu)
                 ;;

    +    In this way, we can change the "ubuntu-latest" runner image to any
    +    version such as "ubuntu-22.04" to test CI behavior in advance.
    +
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>

2:  1d0903c8b2f9 ! 2:  eba96648368a ci: upgrade version of p4
    @@ Metadata
     Author: Jiang Xin <zhiyou.jx@alibaba-inc.com>

      ## Commit message ##
    -    ci: upgrade version of p4
    +    ci: upgrade version of p4 to 21.2

         There would be a segmentation fault when running p4 v16.2 on ubuntu
         22.04 which is the latest version of ubuntu runner image for github
    -    actions. Upgrade p4 from version 16.2 to 19.2 will fix this issue.
    +    actions.

    -    Also add some instructions to show errors of command "p4 -V", so we can
    -    see why the output doesn't match.
    +    By checking each version from [1], p4d version 21.1 and above can work
    +    properly on ubuntu 22.04. But version 22.x will break some p4 test
    +    cases. So p4 version 21.x is exactly the version we can use.
    +
    +    In addition to upgrade p4 from version 16.2 to 21.2, also add some
    +    instructions to show errors of command "p4 -V", so we can see why the
    +    command output doesn't match.
    +
    +    [1]: https://cdist2.perforce.com/perforce/

         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
    @@ ci/lib.sh: ubuntu)
      	# image.
      	# Keep that in mind when you encounter a broken OS X build!
     -	export LINUX_P4_VERSION="16.2"
    -+	export LINUX_P4_VERSION="19.2"
    ++	export LINUX_P4_VERSION="21.2"
      	export LINUX_GIT_LFS_VERSION="1.5.2"

      	P4_PATH="$HOME/custom/p4"
-:  ------------ > 3:  8e432f13bef8 ci: install python on ubuntu
-- snap --

The changes look good!

One alternative I considered about 8e432f13bef8 (ci: install python on
ubuntu, 2022-11-24) was to drop testing Python v2.x (it's years past end
of life after all, see https://endoflife.date/python).

However, as we see frequently demonstrated on the Git mailing list:
losing focus, trying to force patch series to address more than a single
concern, gets us nowhere, and not even fast.

So I agree that the best idea in this patch series is the stop-gap
solution to install `python2` on `ubuntu-22.04`, and deal with deprecating
Python v2.x support separately, later, or never, whichever comes first ;-)

With this, here goes my (probably final, because I think this patch series
is ready):

	Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Thank you so much for working on this, it was a thoroughly enjoyable
experience for me!

Ciao,
Dscho

Re: [PATCH v2 2/3] ci: upgrade version of p4 to 21.2

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 24 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> There would be a segmentation fault when running p4 v16.2 on ubuntu
> 22.04 which is the latest version of ubuntu runner image for github
> actions.
>
> By checking each version from [1], p4d version 21.1 and above can work
> properly on ubuntu 22.04. But version 22.x will break some p4 test
> cases. So p4 version 21.x is exactly the version we can use.
>
> In addition to upgrade p4 from version 16.2 to 21.2, also add some
> instructions to show errors of command "p4 -V", so we can see why the
> command output doesn't match.
> [...]
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 0c0767d354..a618d66b81 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -246,7 +246,7 @@ ubuntu)

Omitted from this context is:

	# The Linux build installs the defined dependency versions below.
	# The OS X build installs much more recent versions,

That part should be updated here, as it's now out-of-date, they're now
installing the same version: 21.2.

>  	# were recorded in the Homebrew database upon creating the OS X
>  	# image.
>  	# Keep that in mind when you encounter a broken OS X build!
> -	export LINUX_P4_VERSION="16.2"
> +	export LINUX_P4_VERSION="21.2"
>  	export LINUX_GIT_LFS_VERSION="1.5.2"
>  
>  	P4_PATH="$HOME/custom/p4"

This is a welcome change, but it would be even more welcome if you
followed-up and unified the linux and osx p4 logic as a follow-up.

I.e. after this we'll install 21.2 on both osx and linux, so the
versions are no longer different.

I think we probably won't need to install different versions for the two
ever, we just drifted on the linux version, or maybe (per the comment
we'll need to adjust) there was some problem before with upgrading the
linux version, but no longer?

I didn't dig, but covering some of that in the commit message would be
most welcome.

So can we just s/LINUX_P4_VERSION/P4_VERSION/ or something, and then
change this in the "macos-latest";

	wget -q "https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz"

To:

	wget -q "https://cdist2.perforce.com/perforce/r${P4_VERSION}/bin.macosx1015x86_64/helix-core-server.tgz"

While doing that we can just move the "LINUX_P4_VERSION" (or whatever we
rename it to) from lib.sh to "install-dependencies.sh".

Re: [PATCH v2 3/3] ci: install python on ubuntu

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 24 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> Python is missing from the default ubuntu-22.04 runner image, which
> prevent git-p4 from working. To install python on ubuntu, we need to
> provide correct package name:
>
>  * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the
>    "python" package, and "/usr/bin/python3" is provided by the "python3"
>    package.
>
>  * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by
>    the "python2" package which has a different name from bionic, and
>    "/usr/bin/python3" is provided by "python3".
>
> Since the "ubuntu-latest" runner image has a higher version, so its safe
> to use "python2" or "python3" package name.
>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/install-dependencies.sh | 2 +-
>  ci/lib.sh                  | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 291e49bdde..e28d93a154 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -15,7 +15,7 @@ case "$runs_on_os" in
>  ubuntu)
>  	sudo apt-get -q update
>  	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
> -		$UBUNTU_COMMON_PKGS $CC_PACKAGE
> +		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
>  	mkdir --parents "$P4_PATH"
>  	pushd "$P4_PATH"
>  		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
> diff --git a/ci/lib.sh b/ci/lib.sh
> index a618d66b81..ebe702e0ea 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -235,8 +235,10 @@ ubuntu)
>  	if [ "$jobname" = linux-gcc ]
>  	then
>  		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
> +		PYTHON_PACKAGE=python3
>  	else
>  		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
> +		PYTHON_PACKAGE=python2
>  	fi

Let's not copy/paste and repeat ourselves here for no reason. Part of
this is pre-existing, but if you just re-arrange these variable decls
you can do this instead:

	PYTHON_PACKAGE=python2
	if test "$jobname" = linux-gcc
	then
		PYTHON_PACKAGE=python3
	fi
	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/${PYTHON_PACKAGE}"

Even if you don't factor out the "else" like that (which I think would
be OK to do while-we'er-at-it) this should be changed so that the
"PYTHON_PACKAGE" comes before "MAKEFLAGS" in the two if/else branches
here, so we can then use the variable.

Re: [PATCH v2 0/3] Fix broken CI on newer github-actions runner image

[Jiang Xin]

On Thu, Nov 24, 2022 at 6:48 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi,
>
> On Thu, 24 Nov 2022, Johannes Schindelin wrote:
>
> > To build even more confidence in the patch series, I will now start a full
> > run (which will take *a lot* of build minutes, unfortunately).
>
> And it passed: https://github.com/dscho/git/actions/runs/3539451056
>
> I also had a look at the range-diff between v1 and v2:
>
> -- snip --
> 1:  ef80c39de1e5 ! 1:  6d4607a4ee46 github-actions: run gcc-8 on ubuntu-20.04 image
>     @@ Commit message
>              ubuntu)
>                  ;;
>
>     +    In this way, we can change the "ubuntu-latest" runner image to any
>     +    version such as "ubuntu-22.04" to test CI behavior in advance.
>     +
>          Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> 2:  1d0903c8b2f9 ! 2:  eba96648368a ci: upgrade version of p4
>     @@ Metadata
>      Author: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
>       ## Commit message ##
>     -    ci: upgrade version of p4
>     +    ci: upgrade version of p4 to 21.2
>
>          There would be a segmentation fault when running p4 v16.2 on ubuntu
>          22.04 which is the latest version of ubuntu runner image for github
>     -    actions. Upgrade p4 from version 16.2 to 19.2 will fix this issue.
>     +    actions.
>
>     -    Also add some instructions to show errors of command "p4 -V", so we can
>     -    see why the output doesn't match.
>     +    By checking each version from [1], p4d version 21.1 and above can work
>     +    properly on ubuntu 22.04. But version 22.x will break some p4 test
>     +    cases. So p4 version 21.x is exactly the version we can use.
>     +
>     +    In addition to upgrade p4 from version 16.2 to 21.2, also add some
>     +    instructions to show errors of command "p4 -V", so we can see why the
>     +    command output doesn't match.
>     +
>     +    [1]: https://cdist2.perforce.com/perforce/
>
>          Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>     @@ ci/lib.sh: ubuntu)
>         # image.
>         # Keep that in mind when you encounter a broken OS X build!
>      -  export LINUX_P4_VERSION="16.2"
>     -+  export LINUX_P4_VERSION="19.2"
>     ++  export LINUX_P4_VERSION="21.2"
>         export LINUX_GIT_LFS_VERSION="1.5.2"
>
>         P4_PATH="$HOME/custom/p4"
> -:  ------------ > 3:  8e432f13bef8 ci: install python on ubuntu
> -- snap --
>
> The changes look good!

Thank you for providing this range-diff.

--
Jiang Xin

Re: [PATCH v2 3/3] ci: install python on ubuntu

[Jiang Xin]

On Thu, Nov 24, 2022 at 7:06 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
>
> On Thu, Nov 24 2022, Jiang Xin wrote:
>
> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> >
> > Python is missing from the default ubuntu-22.04 runner image, which
> > prevent git-p4 from working. To install python on ubuntu, we need to
> > provide correct package name:
> >
> >  * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the
> >    "python" package, and "/usr/bin/python3" is provided by the "python3"
> >    package.
> >
> >  * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by
> >    the "python2" package which has a different name from bionic, and
> >    "/usr/bin/python3" is provided by "python3".
> >
> > Since the "ubuntu-latest" runner image has a higher version, so its safe
> > to use "python2" or "python3" package name.
> >
> > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> > ---
> >  ci/install-dependencies.sh | 2 +-
> >  ci/lib.sh                  | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> > index 291e49bdde..e28d93a154 100755
> > --- a/ci/install-dependencies.sh
> > +++ b/ci/install-dependencies.sh
> > @@ -15,7 +15,7 @@ case "$runs_on_os" in
> >  ubuntu)
> >       sudo apt-get -q update
> >       sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
> > -             $UBUNTU_COMMON_PKGS $CC_PACKAGE
> > +             $UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
> >       mkdir --parents "$P4_PATH"
> >       pushd "$P4_PATH"
> >               wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
> > diff --git a/ci/lib.sh b/ci/lib.sh
> > index a618d66b81..ebe702e0ea 100755
> > --- a/ci/lib.sh
> > +++ b/ci/lib.sh
> > @@ -235,8 +235,10 @@ ubuntu)
> >       if [ "$jobname" = linux-gcc ]
> >       then
> >               MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
> > +             PYTHON_PACKAGE=python3
> >       else
> >               MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
> > +             PYTHON_PACKAGE=python2
> >       fi
>
> Let's not copy/paste and repeat ourselves here for no reason. Part of
> this is pre-existing, but if you just re-arrange these variable decls
> you can do this instead:
>
>         PYTHON_PACKAGE=python2
>         if test "$jobname" = linux-gcc
>         then
>                 PYTHON_PACKAGE=python3
>         fi
>         MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/${PYTHON_PACKAGE}"

That was exactly my first edition, but I thought it was weird to write
as "/usr/bin/${PYTHON_PACKAGE}". But if use two variables like
PYTHON_BINARY and PYTHON_PACKAGE, looks even more silly. So I choose
current solution.

--
Jiang Xin

Re: [PATCH v2 3/3] ci: install python on ubuntu

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 24 2022, Jiang Xin wrote:

> On Thu, Nov 24, 2022 at 7:06 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>>
>> On Thu, Nov 24 2022, Jiang Xin wrote:
>>
>> > From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>> >
>> > Python is missing from the default ubuntu-22.04 runner image, which
>> > prevent git-p4 from working. To install python on ubuntu, we need to
>> > provide correct package name:
>> >
>> >  * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the
>> >    "python" package, and "/usr/bin/python3" is provided by the "python3"
>> >    package.
>> >
>> >  * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by
>> >    the "python2" package which has a different name from bionic, and
>> >    "/usr/bin/python3" is provided by "python3".
>> >
>> > Since the "ubuntu-latest" runner image has a higher version, so its safe
>> > to use "python2" or "python3" package name.
>> >
>> > Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>> > ---
>> >  ci/install-dependencies.sh | 2 +-
>> >  ci/lib.sh                  | 2 ++
>> >  2 files changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
>> > index 291e49bdde..e28d93a154 100755
>> > --- a/ci/install-dependencies.sh
>> > +++ b/ci/install-dependencies.sh
>> > @@ -15,7 +15,7 @@ case "$runs_on_os" in
>> >  ubuntu)
>> >       sudo apt-get -q update
>> >       sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
>> > -             $UBUNTU_COMMON_PKGS $CC_PACKAGE
>> > +             $UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
>> >       mkdir --parents "$P4_PATH"
>> >       pushd "$P4_PATH"
>> >               wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
>> > diff --git a/ci/lib.sh b/ci/lib.sh
>> > index a618d66b81..ebe702e0ea 100755
>> > --- a/ci/lib.sh
>> > +++ b/ci/lib.sh
>> > @@ -235,8 +235,10 @@ ubuntu)
>> >       if [ "$jobname" = linux-gcc ]
>> >       then
>> >               MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
>> > +             PYTHON_PACKAGE=python3
>> >       else
>> >               MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
>> > +             PYTHON_PACKAGE=python2
>> >       fi
>>
>> Let's not copy/paste and repeat ourselves here for no reason. Part of
>> this is pre-existing, but if you just re-arrange these variable decls
>> you can do this instead:
>>
>>         PYTHON_PACKAGE=python2
>>         if test "$jobname" = linux-gcc
>>         then
>>                 PYTHON_PACKAGE=python3
>>         fi
>>         MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/${PYTHON_PACKAGE}"
>
> That was exactly my first edition, but I thought it was weird to write
> as "/usr/bin/${PYTHON_PACKAGE}". But if use two variables like
> PYTHON_BINARY and PYTHON_PACKAGE, looks even more silly. So I choose
> current solution.

I don't mind if you go for your inital version, it's not much
duplication, but why does it look silly? I don't think we need to worry
that the <package-name> on Ubuntu (and Debian) won't have a 1=1 mapping
to the /usr/bin/<package-name>. So defining the path in terms of the
package name seems like an obvious thing to do.

python 2 EOL (was: [PATCH v2 0/3] Fix broken CI on newer github-actions runner image)

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 24 2022, Johannes Schindelin wrote:

> On Thu, 24 Nov 2022, Johannes Schindelin wrote:
> [...]
> The changes look good!
>
> One alternative I considered about 8e432f13bef8 (ci: install python on
> ubuntu, 2022-11-24) was to drop testing Python v2.x (it's years past end
> of life after all, see https://endoflife.date/python).

Just 2 years? :)

We're still pinning "perl" to a supported 5.8, which is so out-of-life
that I couldn't find a good reference to when exactly it went EOL. My
guess based on [1] and "perldoc perlhist" is sometime around 2009-2010.

> So I agree that the best idea in this patch series is the stop-gap
> solution to install `python2` on `ubuntu-22.04`, and deal with deprecating
> Python v2.x support separately, later, or never, whichever comes first ;-)

Yes, let's address that later. We had a recent discussion relating to
EOL-ing it in-tree. See the ML discussion around[2].

I would like to note that you seem to be assuming that upstream's EOL
for something like this should match our EOL for supporting the
software.

I think one could argue that, but that's not at all the stance we've
taken in the past, as the "perl" example shows.

I've personally wanted to bump the "perl" dependency more aggressively
for purely selfish reasons in the past (being able to use some newer
feature), but the reality is that people "back-port" newer git versions
onto various older platforms.

But in terms of the cost-benefit of the disruption that would incur I
also don't think it's worth it (although a bit past 5.8 is probably
justifiable at this point).

Someone using "perl" on an older system for git's tests and git-svn
etc. really doesn't need to worry much about the full security surface
that "perl" might provide, which includes e.g. all the CPAN libraries it
ships with, I expect that the same would go for "python".

The one potential security issue I can think of that we've ever had
because of it is that you could trick "gitweb" and the like into
ever-growing memory use if you had a perl version older than the "hash
randomization" security/DoS fix.

But other than that we're not exposing perl, python etc. directly over
the Internet, so I think we don't need to be too paranoid about it.

1. https://endoflife.date/perl
2. f7b5ff607fa (git-p4: improve encoding handling to support
   inconsistent encodings, 2022-04-30)

Re: [PATCH v2 2/3] ci: upgrade version of p4 to 21.2

[Jiang Xin]

On Thu, Nov 24, 2022 at 7:01 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> Omitted from this context is:
>
>         # The Linux build installs the defined dependency versions below.
>         # The OS X build installs much more recent versions,
>
> That part should be updated here, as it's now out-of-date, they're now
> installing the same version: 21.2.

Yes, the versions of p4 for linux and macos happen to be the same. We
can use one variable to define p4 version for both linux and macos.

>
> >       # were recorded in the Homebrew database upon creating the OS X
> >       # image.
> >       # Keep that in mind when you encounter a broken OS X build!
> > -     export LINUX_P4_VERSION="16.2"
> > +     export LINUX_P4_VERSION="21.2"
> >       export LINUX_GIT_LFS_VERSION="1.5.2"
> >
> >       P4_PATH="$HOME/custom/p4"
>
> This is a welcome change, but it would be even more welcome if you
> followed-up and unified the linux and osx p4 logic as a follow-up.
> I.e. after this we'll install 21.2 on both osx and linux, so the
> versions are no longer different.
>
> I think we probably won't need to install different versions for the two
> ever, we just drifted on the linux version, or maybe (per the comment
> we'll need to adjust) there was some problem before with upgrading the
> linux version, but no longer?
>
> I didn't dig, but covering some of that in the commit message would be
> most welcome.

It is commit d1c9195116 (ci: avoid brew for installing perforce,
2022-05-12), which changed the installation method of p4 on macOS.
There is also a note about the obsolete comment in ci/lib.sh in this
commit.

> So can we just s/LINUX_P4_VERSION/P4_VERSION/ or something, and then
> change this in the "macos-latest";
>
>         wget -q "https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz"
>
> To:
>
>         wget -q "https://cdist2.perforce.com/perforce/r${P4_VERSION}/bin.macosx1015x86_64/helix-core-server.tgz"

> While doing that we can just move the "LINUX_P4_VERSION" (or whatever we
> rename it to) from lib.sh to "install-dependencies.sh".
>

Will do.

--
Jiang Xin

[PATCH v3 0/4] Fix broken CI on newer github-actions runner image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

# Changes since v2

1. Split patch 2/3 of v2 into two patches:

   patch 2/4: ci: show error message of "p4 -V"
   patch 3/4: ci: p4 on Linux has the same version as on macOS

2. In patch 3/4, remove variable "LINUX_P4_VERSION" and use
   a fixed version "21.2" for p4 on both Linux and macOS.

3. For patch 4/4, follow Ævar's suggestion to make the code simpler.


# Range-diff v2..v3

1:  79c851529c ! 1:  1c0d639487 github-actions: run gcc-8 on ubuntu-20.04 image
    @@ Commit message
         In this way, we can change the "ubuntu-latest" runner image to any
         version such as "ubuntu-22.04" to test CI behavior in advance.
     
    +    Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## .github/workflows/main.yml ##
2:  faa5076fe0 ! 2:  fed880b8bd ci: upgrade version of p4 to 21.2
    @@ Metadata
     Author: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## Commit message ##
    -    ci: upgrade version of p4 to 21.2
    +    ci: show error message of "p4 -V"
     
    -    There would be a segmentation fault when running p4 v16.2 on ubuntu
    -    22.04 which is the latest version of ubuntu runner image for github
    -    actions.
    +    When installing p4 as a dependency, we used to pipe output of "p4 -V" to
    +    validate the installation, but this would hide potential errors of p4.
    +    E.g.: A broken p4 installation fails to run.
     
    -    By checking each version from [1], p4d version 21.1 and above can work
    -    properly on ubuntu 22.04. But version 22.x will break some p4 test
    -    cases. So p4 version 21.x is exactly the version we can use.
    -
    -    In addition to upgrade p4 from version 16.2 to 21.2, also add some
    -    instructions to show errors of command "p4 -V", so we can see why the
    -    command output doesn't match.
    -
    -    [1]: https://cdist2.perforce.com/perforce/
    +    Add some instructions to show errors of command "p4 -V", so we can see
    +    why the command output doesn't match.
     
    +    Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## ci/install-dependencies.sh ##
    @@ ci/install-dependencies.sh: esac
      else
      	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
      fi
    -
    - ## ci/lib.sh ##
    -@@ ci/lib.sh: ubuntu)
    - 	# were recorded in the Homebrew database upon creating the OS X
    - 	# image.
    - 	# Keep that in mind when you encounter a broken OS X build!
    --	export LINUX_P4_VERSION="16.2"
    -+	export LINUX_P4_VERSION="21.2"
    - 	export LINUX_GIT_LFS_VERSION="1.5.2"
    - 
    - 	P4_PATH="$HOME/custom/p4"
-:  ---------- > 3:  da2f516fc9 ci: p4 on Linux has the same version as on macOS
3:  f080b2fdcd ! 4:  bd1850cc93 ci: install python on ubuntu
    @@ Commit message
         Since the "ubuntu-latest" runner image has a higher version, so its safe
         to use "python2" or "python3" package name.
     
    +    Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## ci/install-dependencies.sh ##
    @@ ci/install-dependencies.sh: case "$runs_on_os" in
     
      ## ci/lib.sh ##
     @@ ci/lib.sh: ubuntu)
    - 	if [ "$jobname" = linux-gcc ]
    + 		break
    + 	fi
    + 
    +-	if [ "$jobname" = linux-gcc ]
    ++	PYTHON_PACKAGE=python2
    ++	if test "$jobname" = linux-gcc
      	then
    - 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
    +-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
    +-	else
    +-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
     +		PYTHON_PACKAGE=python3
    - 	else
    - 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
    -+		PYTHON_PACKAGE=python2
      	fi
    ++	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
      
      	export GIT_TEST_HTTPD=true
    + 

--

Jiang Xin (4):
  github-actions: run gcc-8 on ubuntu-20.04 image
  ci: show error message of "p4 -V"
  ci: p4 on Linux has the same version as on macOS
  ci: install python on ubuntu

 .github/workflows/main.yml | 16 ++++++++++++----
 ci/install-dependencies.sh | 16 ++++++++--------
 ci/lib.sh                  | 15 +++++++--------
 3 files changed, 27 insertions(+), 20 deletions(-)

-- 
2.39.0.rc0

[PATCH v3 1/4] github-actions: run gcc-8 on ubuntu-20.04 image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

GitHub starts to upgrade its runner image "ubuntu-latest" from version
"ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
install "gcc-8" package on the new runner image.

Change the runner images from "ubuntu-latest" to "ubuntu-20.04" in order
to run with "gcc-8" as a dependency.

Instead of use the environment "$runs_on_pool" as below:

    case "$runs_on_pool" in
    ubuntu-20.04 | ubuntu-latest)
	;;

we can reuse the os field in the matrix, and use a new environment
"$runs_on_os" as below:

    case "$runs_on_os" in
    ubuntu)
        ;;

In this way, we can change the "ubuntu-latest" runner image to any
version such as "ubuntu-22.04" to test CI behavior in advance.

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 .github/workflows/main.yml | 16 ++++++++++++----
 ci/install-dependencies.sh |  6 +++---
 ci/lib.sh                  |  6 +++---
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 592f9193a8..da0d8ab0bf 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -224,6 +224,7 @@ jobs:
         vector:
           - jobname: linux-clang
             cc: clang
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-sha256
             cc: clang
@@ -232,36 +233,43 @@ jobs:
           - jobname: linux-gcc
             cc: gcc
             cc_package: gcc-8
-            pool: ubuntu-latest
+            os: ubuntu
+            pool: ubuntu-20.04
           - jobname: linux-TEST-vars
             cc: gcc
-            os: ubuntu
             cc_package: gcc-8
-            pool: ubuntu-latest
+            os: ubuntu
+            pool: ubuntu-20.04
           - jobname: osx-clang
             cc: clang
+            os: macos
             pool: macos-latest
           - jobname: osx-gcc
             cc: gcc
             cc_package: gcc-9
+            os: macos
             pool: macos-latest
           - jobname: linux-gcc-default
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-leaks
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-asan
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-ubsan
             cc: gcc
+            os: ubuntu
             pool: ubuntu-latest
     env:
       CC: ${{matrix.vector.cc}}
       CC_PACKAGE: ${{matrix.vector.cc_package}}
       jobname: ${{matrix.vector.jobname}}
-      runs_on_pool: ${{matrix.vector.pool}}
+      runs_on_os: ${{matrix.vector.os}}
     runs-on: ${{matrix.vector.pool}}
     steps:
     - uses: actions/checkout@v2
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 107757a1fe..f639263a62 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -11,8 +11,8 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
-case "$runs_on_pool" in
-ubuntu-latest)
+case "$runs_on_os" in
+ubuntu)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
 		$UBUNTU_COMMON_PKGS $CC_PACKAGE
@@ -30,7 +30,7 @@ ubuntu-latest)
 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
 	popd
 	;;
-macos-latest)
+macos)
 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
 	# Uncomment this if you want to run perf tests:
 	# brew install gnu-time
diff --git a/ci/lib.sh b/ci/lib.sh
index 24d20a5d64..0c0767d354 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -225,8 +225,8 @@ export DEFAULT_TEST_TARGET=prove
 export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
-case "$runs_on_pool" in
-ubuntu-latest)
+case "$runs_on_os" in
+ubuntu)
 	if test "$jobname" = "linux-gcc-default"
 	then
 		break
@@ -253,7 +253,7 @@ ubuntu-latest)
 	GIT_LFS_PATH="$HOME/custom/git-lfs"
 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
 	;;
-macos-latest)
+macos)
 	if [ "$jobname" = osx-gcc ]
 	then
 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
-- 
2.39.0.rc0

[PATCH v3 2/4] ci: show error message of "p4 -V"

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

When installing p4 as a dependency, we used to pipe output of "p4 -V" to
validate the installation, but this would hide potential errors of p4.
E.g.: A broken p4 installation fails to run.

Add some instructions to show errors of command "p4 -V", so we can see
why the command output doesn't match.

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index f639263a62..291e49bdde 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -83,9 +83,9 @@ esac
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
-	p4d -V | grep Rev.
+	p4d -V | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
 	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
-	p4 -V | grep Rev.
+	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
-- 
2.39.0.rc0

[PATCH v3 3/4] ci: p4 on Linux has the same version as on macOS

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

There would be a segmentation fault when running p4 v16.2 on ubuntu
22.04 which is the latest version of ubuntu runner image for github
actions.

By checking each version from [1], p4d version 21.1 and above can work
properly on ubuntu 22.04. But version 22.x will break some p4 test
cases. So p4 version 21.x is exactly the version we can use.

With this update, the versions of p4 for Linux and macOS happen to be
the same. So we can add the version number directly into the "P4WHENCE"
variable, and reuse it in p4 installation for macOS.

By removing the "LINUX_P4_VERSION" variable from "ci/lib.sh", the
comment left above has nothing to do with p4, but still applies to
git-lfs. Since we have a fixed version of git-lfs installed on Linux,
we may have a different version on macOS.

[1]: https://cdist2.perforce.com/perforce/

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 4 ++--
 ci/lib.sh                  | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 291e49bdde..f860c35c75 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -5,7 +5,7 @@
 
 . ${0%/*}/lib.sh
 
-P4WHENCE=https://cdist2.perforce.com/perforce/r$LINUX_P4_VERSION
+P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
@@ -40,7 +40,7 @@ macos)
 	mkdir -p $HOME/bin
 	(
 		cd $HOME/bin
-		wget -q "https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz" &&
+		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
 		tar -xf helix-core-server.tgz &&
 		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
 	)
diff --git a/ci/lib.sh b/ci/lib.sh
index 0c0767d354..991ea4f8a6 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -246,7 +246,6 @@ ubuntu)
 	# were recorded in the Homebrew database upon creating the OS X
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
-	export LINUX_P4_VERSION="16.2"
 	export LINUX_GIT_LFS_VERSION="1.5.2"
 
 	P4_PATH="$HOME/custom/p4"
-- 
2.39.0.rc0

[PATCH v3 4/4] ci: install python on ubuntu

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

Python is missing from the default ubuntu-22.04 runner image, which
prevent git-p4 from working. To install python on ubuntu, we need to
provide correct package name:

 * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the
   "python" package, and "/usr/bin/python3" is provided by the "python3"
   package.

 * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by
   the "python2" package which has a different name from bionic, and
   "/usr/bin/python3" is provided by "python3".

Since the "ubuntu-latest" runner image has a higher version, so its safe
to use "python2" or "python3" package name.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index f860c35c75..e804b1935e 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -15,7 +15,7 @@ case "$runs_on_os" in
 ubuntu)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS $CC_PACKAGE
+		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
diff --git a/ci/lib.sh b/ci/lib.sh
index 991ea4f8a6..fbebad4b9c 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -232,12 +232,12 @@ ubuntu)
 		break
 	fi
 
-	if [ "$jobname" = linux-gcc ]
+	PYTHON_PACKAGE=python2
+	if test "$jobname" = linux-gcc
 	then
-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
-	else
-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
+		PYTHON_PACKAGE=python3
 	fi
+	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
 
 	export GIT_TEST_HTTPD=true
 
-- 
2.39.0.rc0

Re: [PATCH v3 2/4] ci: show error message of "p4 -V"

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 24 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> When installing p4 as a dependency, we used to pipe output of "p4 -V" to
> validate the installation, but this would hide potential errors of p4.
> E.g.: A broken p4 installation fails to run.
>
> Add some instructions to show errors of command "p4 -V", so we can see
> why the command output doesn't match.
>
> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/install-dependencies.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index f639263a62..291e49bdde 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -83,9 +83,9 @@ esac
>  if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
>  then
>  	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
> -	p4d -V | grep Rev.
> +	p4d -V | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
>  	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
> -	p4 -V | grep Rev.
> +	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
>  else
>  	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
>  fi

I think it makes sense to detect this, but the specific remedy makes no
sense to me.

I.e. as your CL's CI output shows the reason we want this is because you
had a p4d (and p4?) that segfaulted. I can reproduce that locally as:

	wget --quiet https://cdist2.perforce.com/perforce/r16.2/bin.linux26x86_64/p4d -O p4d; chmod +x p4d; ./p4d
	Segmentation fault

But we had the initial command piped into Rev, so we wouldn't show that,
just

	$ wget --quiet https://cdist2.perforce.com/perforce/r16.2/bin.linux26x86_64/p4d -O p4d; chmod +x p4d; ./p4d | grep Rev; echo $?
	1

So we want to show that it segfaults, but how it is useful once that
command fails to pretend that it's a "bad version?" The issue is that
the command can't show the version at all.

	$ { echo >&2 "p4d: bad version"; ./p4d -V; exit 1; }
	p4d: bad version
	Segmentation fault
	exit

I think this should just be a plain:

	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
        p4d -V
        echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
	p4 -V

As we're already under "set -e" (in ci/lib.sh).

I.e. I don't see why we need to work at making the output less verbose,
and then if we fail run the command again.

If we really want to make it less verbose and avoid hiding segfault
messages perhaps we could filter out the stuff we don't care about?:
	
	$ ./p4d -V | sed -ne '/[Cc]opyright/d; /[Ll]icense/d; /http/d; /product/d; p'
	Perforce - The Fast Software Configuration Management System.
	Version of OpenSSL Libraries: OpenSSL 1.1.1q  5 Jul 2022
	Version of OpenLDAP Libraries: 2.4.59
	Version of Cyrus SASL Libraries: 2.1.27
	Using the 'SmartHeap' memory manager.
	Rev. P4D/LINUX26X86_64/2021.1/2313999 (2022/07/15).
	
But I don't see the point of that effort, we won't look at these trace
logs unless something fails, so just including the raw output seems most
sensible.

Re: [PATCH v3 1/4] github-actions: run gcc-8 on ubuntu-20.04 image

[Ævar Arnfjörð Bjarmason]

On Thu, Nov 24 2022, Jiang Xin wrote:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> GitHub starts to upgrade its runner image "ubuntu-latest" from version
> "ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
> install "gcc-8" package on the new runner image.
>
> Change the runner images from "ubuntu-latest" to "ubuntu-20.04" in order
> to run with "gcc-8" as a dependency.
>
> Instead of use the environment "$runs_on_pool" as below:
>
>     case "$runs_on_pool" in
>     ubuntu-20.04 | ubuntu-latest)
> 	;;
>
> we can reuse the os field in the matrix, and use a new environment
> "$runs_on_os" as below:
>
>     case "$runs_on_os" in
>     ubuntu)
>         ;;
>
> In this way, we can change the "ubuntu-latest" runner image to any
> version such as "ubuntu-22.04" to test CI behavior in advance.
>
> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  .github/workflows/main.yml | 16 ++++++++++++----
>  ci/install-dependencies.sh |  6 +++---
>  ci/lib.sh                  |  6 +++---
>  3 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 592f9193a8..da0d8ab0bf 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -224,6 +224,7 @@ jobs:
>          vector:
>            - jobname: linux-clang
>              cc: clang
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-sha256
>              cc: clang
> @@ -232,36 +233,43 @@ jobs:
>            - jobname: linux-gcc
>              cc: gcc
>              cc_package: gcc-8
> -            pool: ubuntu-latest
> +            os: ubuntu
> +            pool: ubuntu-20.04
>            - jobname: linux-TEST-vars
>              cc: gcc
> -            os: ubuntu
>              cc_package: gcc-8
> -            pool: ubuntu-latest
> +            os: ubuntu
> +            pool: ubuntu-20.04
>            - jobname: osx-clang
>              cc: clang
> +            os: macos
>              pool: macos-latest
>            - jobname: osx-gcc
>              cc: gcc
>              cc_package: gcc-9
> +            os: macos
>              pool: macos-latest
>            - jobname: linux-gcc-default
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-leaks
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-asan
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>            - jobname: linux-ubsan
>              cc: gcc
> +            os: ubuntu
>              pool: ubuntu-latest
>      env:
>        CC: ${{matrix.vector.cc}}
>        CC_PACKAGE: ${{matrix.vector.cc_package}}
>        jobname: ${{matrix.vector.jobname}}
> -      runs_on_pool: ${{matrix.vector.pool}}
> +      runs_on_os: ${{matrix.vector.os}}
>      runs-on: ${{matrix.vector.pool}}
>      steps:
>      - uses: actions/checkout@v2
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 107757a1fe..f639263a62 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -11,8 +11,8 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
>   tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
>   libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
>  
> -case "$runs_on_pool" in
> -ubuntu-latest)
> +case "$runs_on_os" in
> +ubuntu)
>  	sudo apt-get -q update
>  	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
>  		$UBUNTU_COMMON_PKGS $CC_PACKAGE
> @@ -30,7 +30,7 @@ ubuntu-latest)
>  		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
>  	popd
>  	;;
> -macos-latest)
> +macos)
>  	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
>  	# Uncomment this if you want to run perf tests:
>  	# brew install gnu-time
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 24d20a5d64..0c0767d354 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -225,8 +225,8 @@ export DEFAULT_TEST_TARGET=prove
>  export GIT_TEST_CLONE_2GB=true
>  export SKIP_DASHED_BUILT_INS=YesPlease
>  
> -case "$runs_on_pool" in
> -ubuntu-latest)
> +case "$runs_on_os" in
> +ubuntu)
>  	if test "$jobname" = "linux-gcc-default"
>  	then
>  		break
> @@ -253,7 +253,7 @@ ubuntu-latest)
>  	GIT_LFS_PATH="$HOME/custom/git-lfs"
>  	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
>  	;;
> -macos-latest)
> +macos)
>  	if [ "$jobname" = osx-gcc ]
>  	then
>  		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"

At the end of [1] I suggested splitting up the refactoring part of
this. I still think that would be a good thing to do, but just to
comment on the *actual* change (which I was hoping a split-out would
make clearer).

This is proposing to refactor
"runs-on-pool={ubuntu-latest,macos-latest}" to "os={ubuntu,macos}". I
think it's worth cleaning up how we match main.yml and ci/*.sh, but
think it's a bit of a digression in this otherwise narrowly focused
series.

Now, this seems to somewhat come back to being all my fault. AFAICT I
added "os" in c08bb260105 (CI: rename the "Linux32" job to lower-case
"linux32", 2021-11-23) as part of some WIP (or from an earlier version?)
that ende dup doing nothing.

But looking at it again, I don't see how an "os" here makes much
sense. For "macos-latest" if we're going to refactor it let's just stop
selecting stuff based on this "pool name", and instead fold it into the
adjacent "jobname" selection:
	
	diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
	index 107757a1fea..b5a373060c1 100755
	--- a/ci/install-dependencies.sh
	+++ b/ci/install-dependencies.sh
	@@ -30,7 +30,9 @@ ubuntu-latest)
	 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
	 	popd
	 	;;
	-macos-latest)
	+esac
	+case "$jobname" in
	+osx-*)
	 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
	 	# Uncomment this if you want to run perf tests:
	 	# brew install gnu-time
	diff --git a/ci/lib.sh b/ci/lib.sh
	index 24d20a5d648..fef60a507e9 100755
	--- a/ci/lib.sh
	+++ b/ci/lib.sh
	@@ -253,19 +253,17 @@ ubuntu-latest)
	 	GIT_LFS_PATH="$HOME/custom/git-lfs"
	 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
	 	;;
	-macos-latest)
	-	if [ "$jobname" = osx-gcc ]
	-	then
	-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
	-	else
	-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
	-		MAKEFLAGS="$MAKEFLAGS NO_APPLE_COMMON_CRYPTO=NoThanks"
	-		MAKEFLAGS="$MAKEFLAGS NO_OPENSSL=NoThanks"
	-	fi
	-	;;
	 esac
	 
	 case "$jobname" in
	+osx-gcc)
	+	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
	+	;;
	+osx-clang)
	+	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python2)"
	+	MAKEFLAGS="$MAKEFLAGS NO_APPLE_COMMON_CRYPTO=NoThanks"
	+	MAKEFLAGS="$MAKEFLAGS NO_OPENSSL=NoThanks"
	+	;;
	 linux32)
	 	CC=gcc
	 	;;

That nicely sidesteps the need to refactor it here, and IMO makes it
more future-proof.

For the "ubuntu" case I think it's even worse, because it now *looks*
like a case that should be equivalent to just:

	test "$(lsb_release -si 2>&1 | grep -x Debian)" = "Debian"

But it's not, because we're not setting this "os=ubuntu" in all the
places where the OS is Ubuntu. It really just means "the stuff that used
to set env.runs_on_pool=ubuntu-latest, which e.g. doesn't cover the
"documentation" job, which runs on "ubuntu-latest".

So I think *if* we're going to refactor this while-at-it, let's not make
it "os=ubuntu", but e.g. "extra_ubuntu_packages: 1", which is really
what it means. I.e. we're intercepting it in ci/lib.sh to set the "p4"
variables etc., which we only need if we're doing the optional "apt
install" on the "ubuntu-latest".

1. https://lore.kernel.org/git/221124.86mt8gpqra.gmgdl@evledraar.gmail.com/

Re: [PATCH v3 2/4] ci: show error message of "p4 -V"

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>>  if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
>>  then
>>  	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
>> -	p4d -V | grep Rev.
>> +	p4d -V | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
>>  	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
>> -	p4 -V | grep Rev.
>> +	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
>>  else
>>  	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
>>  fi
>
> I think it makes sense to detect this, but the specific remedy makes no
> sense to me.
> ...
> So we want to show that it segfaults, but how it is useful once that
> command fails to pretend that it's a "bad version?" The issue is that
> the command can't show the version at all.
>
> 	$ { echo >&2 "p4d: bad version"; ./p4d -V; exit 1; }
> 	p4d: bad version
> 	Segmentation fault
> 	exit
> ...
> But I don't see the point of that effort, we won't look at these trace
> logs unless something fails, so just including the raw output seems most
> sensible.

Yup, makes sense.

Thanks, both, for working on this topic.

Re: python 2 EOL

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>> So I agree that the best idea in this patch series is the stop-gap
>> solution to install `python2` on `ubuntu-22.04`, and deal with deprecating
>> Python v2.x support separately, later, or never, whichever comes first ;-)
>
> Yes, let's address that later. We had a recent discussion relating to
> EOL-ing it in-tree. See the ML discussion around[2].
> ...
> 2. f7b5ff607fa (git-p4: improve encoding handling to support
>    inconsistent encodings, 2022-04-30)

I agree with two of you that a good first step is the posted patch.

I do not seem to find any message in the thread that led to the
commit about dropping Python 2 support, though.

Thanks.

Re: [PATCH v2 1/3] github-actions: run gcc-8 on ubuntu-20.04 image

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Because while I think a non-pinned "latest" sucks because it forces us
> to scramble with updates like this, having a "pinned" image go away
> entirely (because nobody noticed we should update it sooner-than-later)
> would suck more.

Yeah, that matches my preference, assuming that we do not have a
pre-warning to help us avoid scrambling either way.

If we have such a pre-warning, I would say it may be nicer to pin a
version, even if the only upside is that we can say we test with
"this exact version".

[PATCH v4 0/4] Fix broken CI on newer github-actions runner image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

GitHub CI runner image "ubuntu-latest" will upgrade to "ubuntu-22.04"
soon. Our CI will break on the new runner image because there is not
"gcc-8" package in ubuntu-22.04. See log of CI#10:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3546701459/jobs/5956020103#step:3:106

Another issue in CI#10 is "pd4 -V” failed without an error output. See:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3546701459/jobs/5956019973#step:3:315

This first issue is fixed in patch 1/4 by downgrading the version of
the runner image for jobs which need "gcc-8". The second issue is
fixed in patch 2/4 by removing the pipe from "p4d -V".  Then, we can
see there was a sigfault when running "p4d -V". See log of CI#11:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3546723251/jobs/5956063536#step:3:311

This issue is fixed in patch 3/4 by upgrading version of p4 on Linux
to the same version as on macOS. But all p4 related test cases failed
becasue python was missing on ubuntu-22.04.  See the log of CI#12:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3546734918/jobs/5956087614#step:4:22733

This issue is fixed in patch 4/4 by installing python2/python3 on
Linux.  All test cases passed. See log of CI#13:

 * https://github.com/jiangxin/git-ci-test/actions/runs/3546744257

# range-diff v3..v4

1:  1c0d639487 ! 1:  28588dad56 github-actions: run gcc-8 on ubuntu-20.04 image
    @@ Commit message
         "ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
         install "gcc-8" package on the new runner image.
     
    -    Change the runner images from "ubuntu-latest" to "ubuntu-20.04" in order
    -    to run with "gcc-8" as a dependency.
    +    Change some of the runner images from "ubuntu-latest" to "ubuntu-20.04"
    +    in order to install "gcc-8" as a dependency.
     
    -    Instead of use the environment "$runs_on_pool" as below:
    +    The first revision of this patch tried to replace "$runs_on_pool" in
    +    "ci/*.sh" with a new "$runs_on_os" environment variable based on the
    +    "os" filed in the matrix strategy. But these "os" fields in matrix
    +    strategies are obsolete legacies from commit [1] and commit [2], and
    +    are no longer useful. So remove these unused "os" fields.
     
    -        case "$runs_on_pool" in
    -        ubuntu-20.04 | ubuntu-latest)
    -            ;;
    -
    -    we can reuse the os field in the matrix, and use a new environment
    -    "$runs_on_os" as below:
    -
    -        case "$runs_on_os" in
    -        ubuntu)
    -            ;;
    -
    -    In this way, we can change the "ubuntu-latest" runner image to any
    -    version such as "ubuntu-22.04" to test CI behavior in advance.
    +    [1]: c08bb26010 (CI: rename the "Linux32" job to lower-case "linux32",
    +                     2021-11-23)
    +    [2]: 25715419bf (CI: don't run "make test" twice in one job, 2021-11-23)
     
         Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    +    Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## .github/workflows/main.yml ##
     @@ .github/workflows/main.yml: jobs:
    -         vector:
    -           - jobname: linux-clang
    -             cc: clang
    -+            os: ubuntu
                  pool: ubuntu-latest
                - jobname: linux-sha256
                  cc: clang
    -@@ .github/workflows/main.yml: jobs:
    +-            os: ubuntu
    +             pool: ubuntu-latest
                - jobname: linux-gcc
                  cc: gcc
                  cc_package: gcc-8
     -            pool: ubuntu-latest
    -+            os: ubuntu
     +            pool: ubuntu-20.04
                - jobname: linux-TEST-vars
                  cc: gcc
     -            os: ubuntu
                  cc_package: gcc-8
     -            pool: ubuntu-latest
    -+            os: ubuntu
     +            pool: ubuntu-20.04
                - jobname: osx-clang
                  cc: clang
    -+            os: macos
                  pool: macos-latest
    -           - jobname: osx-gcc
    -             cc: gcc
    -             cc_package: gcc-9
    -+            os: macos
    -             pool: macos-latest
    -           - jobname: linux-gcc-default
    -             cc: gcc
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -           - jobname: linux-leaks
    -             cc: gcc
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -           - jobname: linux-asan
    -             cc: gcc
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -           - jobname: linux-ubsan
    -             cc: gcc
    -+            os: ubuntu
    -             pool: ubuntu-latest
    -     env:
    -       CC: ${{matrix.vector.cc}}
    -       CC_PACKAGE: ${{matrix.vector.cc_package}}
    -       jobname: ${{matrix.vector.jobname}}
    --      runs_on_pool: ${{matrix.vector.pool}}
    -+      runs_on_os: ${{matrix.vector.os}}
    -     runs-on: ${{matrix.vector.pool}}
    -     steps:
    -     - uses: actions/checkout@v2
    +@@ .github/workflows/main.yml: jobs:
    +         - jobname: linux-musl
    +           image: alpine
    +         - jobname: linux32
    +-          os: ubuntu32
    +           image: daald/ubuntu32:xenial
    +         - jobname: pedantic
    +           image: fedora
     
      ## ci/install-dependencies.sh ##
     @@ ci/install-dependencies.sh: UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
    -  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
       libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
      
    --case "$runs_on_pool" in
    + case "$runs_on_pool" in
     -ubuntu-latest)
    -+case "$runs_on_os" in
    -+ubuntu)
    ++ubuntu-*)
      	sudo apt-get -q update
      	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
      		$UBUNTU_COMMON_PKGS $CC_PACKAGE
    -@@ ci/install-dependencies.sh: ubuntu-latest)
    - 		cp git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs .
    - 	popd
    - 	;;
    --macos-latest)
    -+macos)
    - 	export HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1
    - 	# Uncomment this if you want to run perf tests:
    - 	# brew install gnu-time
     
      ## ci/lib.sh ##
    -@@ ci/lib.sh: export DEFAULT_TEST_TARGET=prove
    - export GIT_TEST_CLONE_2GB=true
    +@@ ci/lib.sh: export GIT_TEST_CLONE_2GB=true
      export SKIP_DASHED_BUILT_INS=YesPlease
      
    --case "$runs_on_pool" in
    + case "$runs_on_pool" in
     -ubuntu-latest)
    -+case "$runs_on_os" in
    -+ubuntu)
    ++ubuntu-*)
      	if test "$jobname" = "linux-gcc-default"
      	then
      		break
    -@@ ci/lib.sh: ubuntu-latest)
    - 	GIT_LFS_PATH="$HOME/custom/git-lfs"
    - 	export PATH="$GIT_LFS_PATH:$P4_PATH:$PATH"
    - 	;;
    --macos-latest)
    -+macos)
    - 	if [ "$jobname" = osx-gcc ]
    - 	then
    - 		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=$(which python3)"
2:  fed880b8bd ! 2:  b914f1d2fc ci: show error message of "p4 -V"
    @@ Metadata
     Author: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## Commit message ##
    -    ci: show error message of "p4 -V"
    +    ci: remove the pipe after "p4 -V" to cache errors
     
    -    When installing p4 as a dependency, we used to pipe output of "p4 -V" to
    -    validate the installation, but this would hide potential errors of p4.
    -    E.g.: A broken p4 installation fails to run.
    +    When installing p4 as a dependency, we used to pipe output of "p4 -V"
    +    and "p4d -V" to validate the installation and output a condensed version
    +    information. But this would hide potential errors of p4 and would stop
    +    with an empty output. E.g.: p4d version 16.2 running on ubuntu 22.04
    +    causes sigfaults.
     
    -    Add some instructions to show errors of command "p4 -V", so we can see
    -    why the command output doesn't match.
    +    By removing the pipe after "p4 -V" and "p4d -V", we may get a verbose
    +    output, and stop immediately on errors becuase we have "set -e" in
    +    "ci/lib.sh". Since we won't look at these trace logs unless something
    +    fails, so just including the raw output seems most sensible.
     
         Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    +    Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
         Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
     
      ## ci/install-dependencies.sh ##
    @@ ci/install-dependencies.sh: esac
      then
      	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
     -	p4d -V | grep Rev.
    -+	p4d -V | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
    ++	p4d -V
      	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
     -	p4 -V | grep Rev.
    -+	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
    ++	p4 -V
      else
      	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
      fi
3:  da2f516fc9 = 3:  4aa2b30c36 ci: p4 on Linux has the same version as on macOS
4:  bd1850cc93 = 4:  35909fd459 ci: install python on ubuntu
      
--

Jiang Xin (4):
  github-actions: run gcc-8 on ubuntu-20.04 image
  ci: remove the pipe after "p4 -V" to cache errors
  ci: p4 on Linux has the same version as on macOS
  ci: install python on ubuntu

 .github/workflows/main.yml |  7 ++-----
 ci/install-dependencies.sh | 12 ++++++------
 ci/lib.sh                  | 11 +++++------
 3 files changed, 13 insertions(+), 17 deletions(-)

-- 
2.39.0.rc0

[PATCH v4 1/4] github-actions: run gcc-8 on ubuntu-20.04 image

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

GitHub starts to upgrade its runner image "ubuntu-latest" from version
"ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
install "gcc-8" package on the new runner image.

Change some of the runner images from "ubuntu-latest" to "ubuntu-20.04"
in order to install "gcc-8" as a dependency.

The first revision of this patch tried to replace "$runs_on_pool" in
"ci/*.sh" with a new "$runs_on_os" environment variable based on the
"os" filed in the matrix strategy. But these "os" fields in matrix
strategies are obsolete legacies from commit [1] and commit [2], and
are no longer useful. So remove these unused "os" fields.

[1]: c08bb26010 (CI: rename the "Linux32" job to lower-case "linux32",
                 2021-11-23)
[2]: 25715419bf (CI: don't run "make test" twice in one job, 2021-11-23)

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 .github/workflows/main.yml | 7 ++-----
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 592f9193a8..9afacfa0b3 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -227,17 +227,15 @@ jobs:
             pool: ubuntu-latest
           - jobname: linux-sha256
             cc: clang
-            os: ubuntu
             pool: ubuntu-latest
           - jobname: linux-gcc
             cc: gcc
             cc_package: gcc-8
-            pool: ubuntu-latest
+            pool: ubuntu-20.04
           - jobname: linux-TEST-vars
             cc: gcc
-            os: ubuntu
             cc_package: gcc-8
-            pool: ubuntu-latest
+            pool: ubuntu-20.04
           - jobname: osx-clang
             cc: clang
             pool: macos-latest
@@ -288,7 +286,6 @@ jobs:
         - jobname: linux-musl
           image: alpine
         - jobname: linux32
-          os: ubuntu32
           image: daald/ubuntu32:xenial
         - jobname: pedantic
           image: fedora
diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 107757a1fe..feefd6e9bb 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -12,7 +12,7 @@ UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  libemail-valid-perl libio-socket-ssl-perl libnet-smtp-ssl-perl"
 
 case "$runs_on_pool" in
-ubuntu-latest)
+ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
 		$UBUNTU_COMMON_PKGS $CC_PACKAGE
diff --git a/ci/lib.sh b/ci/lib.sh
index 24d20a5d64..eb203662c5 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -226,7 +226,7 @@ export GIT_TEST_CLONE_2GB=true
 export SKIP_DASHED_BUILT_INS=YesPlease
 
 case "$runs_on_pool" in
-ubuntu-latest)
+ubuntu-*)
 	if test "$jobname" = "linux-gcc-default"
 	then
 		break
-- 
2.39.0.rc0

[PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

When installing p4 as a dependency, we used to pipe output of "p4 -V"
and "p4d -V" to validate the installation and output a condensed version
information. But this would hide potential errors of p4 and would stop
with an empty output. E.g.: p4d version 16.2 running on ubuntu 22.04
causes sigfaults.

By removing the pipe after "p4 -V" and "p4d -V", we may get a verbose
output, and stop immediately on errors becuase we have "set -e" in
"ci/lib.sh". Since we won't look at these trace logs unless something
fails, so just including the raw output seems most sensible.

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index feefd6e9bb..97a1a1f574 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -83,9 +83,9 @@ esac
 if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
 then
 	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
-	p4d -V | grep Rev.
+	p4d -V
 	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
-	p4 -V | grep Rev.
+	p4 -V
 else
 	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
 fi
-- 
2.39.0.rc0

[PATCH v4 3/4] ci: p4 on Linux has the same version as on macOS

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

There would be a segmentation fault when running p4 v16.2 on ubuntu
22.04 which is the latest version of ubuntu runner image for github
actions.

By checking each version from [1], p4d version 21.1 and above can work
properly on ubuntu 22.04. But version 22.x will break some p4 test
cases. So p4 version 21.x is exactly the version we can use.

With this update, the versions of p4 for Linux and macOS happen to be
the same. So we can add the version number directly into the "P4WHENCE"
variable, and reuse it in p4 installation for macOS.

By removing the "LINUX_P4_VERSION" variable from "ci/lib.sh", the
comment left above has nothing to do with p4, but still applies to
git-lfs. Since we have a fixed version of git-lfs installed on Linux,
we may have a different version on macOS.

[1]: https://cdist2.perforce.com/perforce/

Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 4 ++--
 ci/lib.sh                  | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 97a1a1f574..b569893b38 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -5,7 +5,7 @@
 
 . ${0%/*}/lib.sh
 
-P4WHENCE=https://cdist2.perforce.com/perforce/r$LINUX_P4_VERSION
+P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
 LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
 UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
  tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
@@ -40,7 +40,7 @@ macos-latest)
 	mkdir -p $HOME/bin
 	(
 		cd $HOME/bin
-		wget -q "https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz" &&
+		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
 		tar -xf helix-core-server.tgz &&
 		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
 	)
diff --git a/ci/lib.sh b/ci/lib.sh
index eb203662c5..927b4529b0 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -246,7 +246,6 @@ ubuntu-*)
 	# were recorded in the Homebrew database upon creating the OS X
 	# image.
 	# Keep that in mind when you encounter a broken OS X build!
-	export LINUX_P4_VERSION="16.2"
 	export LINUX_GIT_LFS_VERSION="1.5.2"
 
 	P4_PATH="$HOME/custom/p4"
-- 
2.39.0.rc0

[PATCH v4 4/4] ci: install python on ubuntu

[Jiang Xin]

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

Python is missing from the default ubuntu-22.04 runner image, which
prevent git-p4 from working. To install python on ubuntu, we need to
provide correct package name:

 * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the
   "python" package, and "/usr/bin/python3" is provided by the "python3"
   package.

 * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by
   the "python2" package which has a different name from bionic, and
   "/usr/bin/python3" is provided by "python3".

Since the "ubuntu-latest" runner image has a higher version, so its safe
to use "python2" or "python3" package name.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 ci/install-dependencies.sh | 2 +-
 ci/lib.sh                  | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index b569893b38..d8fafc8ed2 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -15,7 +15,7 @@ case "$runs_on_pool" in
 ubuntu-*)
 	sudo apt-get -q update
 	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
-		$UBUNTU_COMMON_PKGS $CC_PACKAGE
+		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
 	mkdir --parents "$P4_PATH"
 	pushd "$P4_PATH"
 		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
diff --git a/ci/lib.sh b/ci/lib.sh
index 927b4529b0..cb30699f34 100755
--- a/ci/lib.sh
+++ b/ci/lib.sh
@@ -232,12 +232,12 @@ ubuntu-*)
 		break
 	fi
 
-	if [ "$jobname" = linux-gcc ]
+	PYTHON_PACKAGE=python2
+	if test "$jobname" = linux-gcc
 	then
-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
-	else
-		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
+		PYTHON_PACKAGE=python3
 	fi
+	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
 
 	export GIT_TEST_HTTPD=true
 
-- 
2.39.0.rc0

Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> Subject: Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors

"cache" -> "catch", I think.

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> When installing p4 as a dependency, we used to pipe output of "p4 -V"
> and "p4d -V" to validate the installation and output a condensed version
> information. But this would hide potential errors of p4 and would stop
> with an empty output. E.g.: p4d version 16.2 running on ubuntu 22.04
> causes sigfaults.

... before it produces any output.

> By removing the pipe after "p4 -V" and "p4d -V", we may get a verbose
> output, and stop immediately on errors becuase we have "set -e" in

"because".

> "ci/lib.sh". Since we won't look at these trace logs unless something
> fails, so just including the raw output seems most sensible.

You started the sentence with "Since", so "so" should be dropped.

> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/install-dependencies.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Well reasoned.  Will queue.  Thanks.

> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index feefd6e9bb..97a1a1f574 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -83,9 +83,9 @@ esac
>  if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
>  then
>  	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
> -	p4d -V | grep Rev.
> +	p4d -V
>  	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
> -	p4 -V | grep Rev.
> +	p4 -V
>  else
>  	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
>  fi

Re: [PATCH v4 1/4] github-actions: run gcc-8 on ubuntu-20.04 image

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> GitHub starts to upgrade its runner image "ubuntu-latest" from version
> "ubuntu-20.04" to version "ubuntu-22.04". It will fail to find and
> install "gcc-8" package on the new runner image.
>
> Change some of the runner images from "ubuntu-latest" to "ubuntu-20.04"
> in order to install "gcc-8" as a dependency.
>
> The first revision of this patch tried to replace "$runs_on_pool" in
> "ci/*.sh" with a new "$runs_on_os" environment variable based on the
> "os" filed in the matrix strategy. But these "os" fields in matrix

"filed" -> "field".

> strategies are obsolete legacies from commit [1] and commit [2], and
> are no longer useful. So remove these unused "os" fields.
>
> [1]: c08bb26010 (CI: rename the "Linux32" job to lower-case "linux32",
>                  2021-11-23)
> [2]: 25715419bf (CI: don't run "make test" twice in one job, 2021-11-23)
>
> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---

Very well explained.  With the patch text alone, readers would have
been puzzled about the removal of "os" and the description certainly
helps.

I also like the changes to ci/*.sh to match any future versions of
"ubuntu-*" used in the pool name.  It is a sensible way to future
proofing.

Will queue.  Thanks, all.

Re: [PATCH v4 3/4] ci: p4 on Linux has the same version as on macOS

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> There would be a segmentation fault when running p4 v16.2 on ubuntu
> 22.04 which is the latest version of ubuntu runner image for github
> actions.
>
> By checking each version from [1], p4d version 21.1 and above can work
> properly on ubuntu 22.04. But version 22.x will break some p4 test
> cases. So p4 version 21.x is exactly the version we can use.
>
> With this update, the versions of p4 for Linux and macOS happen to be
> the same. So we can add the version number directly into the "P4WHENCE"
> variable, and reuse it in p4 installation for macOS.

Makes sense.  This needs to be retitled from a statement of the fact ...

> Subject: Re: [PATCH v4 3/4] ci: p4 on Linux has the same version as on macOS

... to description of a choice _we_ made, e.g.

    ci: use the same version of p4 on both Linux and macOS

or something like that.

Other than that, looks good.

Thanks.

> By removing the "LINUX_P4_VERSION" variable from "ci/lib.sh", the
> comment left above has nothing to do with p4, but still applies to
> git-lfs. Since we have a fixed version of git-lfs installed on Linux,
> we may have a different version on macOS.
>
> [1]: https://cdist2.perforce.com/perforce/
>
> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/install-dependencies.sh | 4 ++--
>  ci/lib.sh                  | 1 -
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 97a1a1f574..b569893b38 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -5,7 +5,7 @@
>  
>  . ${0%/*}/lib.sh
>  
> -P4WHENCE=https://cdist2.perforce.com/perforce/r$LINUX_P4_VERSION
> +P4WHENCE=https://cdist2.perforce.com/perforce/r21.2
>  LFSWHENCE=https://github.com/github/git-lfs/releases/download/v$LINUX_GIT_LFS_VERSION
>  UBUNTU_COMMON_PKGS="make libssl-dev libcurl4-openssl-dev libexpat-dev
>   tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl
> @@ -40,7 +40,7 @@ macos-latest)
>  	mkdir -p $HOME/bin
>  	(
>  		cd $HOME/bin
> -		wget -q "https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz" &&
> +		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
>  		tar -xf helix-core-server.tgz &&
>  		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
>  	)
> diff --git a/ci/lib.sh b/ci/lib.sh
> index eb203662c5..927b4529b0 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -246,7 +246,6 @@ ubuntu-*)
>  	# were recorded in the Homebrew database upon creating the OS X
>  	# image.
>  	# Keep that in mind when you encounter a broken OS X build!
> -	export LINUX_P4_VERSION="16.2"
>  	export LINUX_GIT_LFS_VERSION="1.5.2"
>  
>  	P4_PATH="$HOME/custom/p4"

Re: [PATCH v4 4/4] ci: install python on ubuntu

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> Python is missing from the default ubuntu-22.04 runner image, which
> prevent git-p4 from working. To install python on ubuntu, we need to
> provide correct package name:
>
>  * On Ubuntu 18.04 (bionic), "/usr/bin/python2" is provided by the
>    "python" package, and "/usr/bin/python3" is provided by the "python3"
>    package.
>
>  * On Ubuntu 20.04 (focal) and above, "/usr/bin/python2" is provided by
>    the "python2" package which has a different name from bionic, and
>    "/usr/bin/python3" is provided by "python3".
>
> Since the "ubuntu-latest" runner image has a higher version, so its safe
> to use "python2" or "python3" package name.

Makes sense.  Just out of curiousity (read: not a suggestion to
change anything), what happens if you say "apt install python" on a
recent system?

Will queue.  Thanks.

> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/install-dependencies.sh | 2 +-
>  ci/lib.sh                  | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index b569893b38..d8fafc8ed2 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -15,7 +15,7 @@ case "$runs_on_pool" in
>  ubuntu-*)
>  	sudo apt-get -q update
>  	sudo apt-get -q -y install language-pack-is libsvn-perl apache2 \
> -		$UBUNTU_COMMON_PKGS $CC_PACKAGE
> +		$UBUNTU_COMMON_PKGS $CC_PACKAGE $PYTHON_PACKAGE
>  	mkdir --parents "$P4_PATH"
>  	pushd "$P4_PATH"
>  		wget --quiet "$P4WHENCE/bin.linux26x86_64/p4d"
> diff --git a/ci/lib.sh b/ci/lib.sh
> index 927b4529b0..cb30699f34 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -232,12 +232,12 @@ ubuntu-*)
>  		break
>  	fi
>  
> -	if [ "$jobname" = linux-gcc ]
> +	PYTHON_PACKAGE=python2
> +	if test "$jobname" = linux-gcc
>  	then
> -		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python3"
> -	else
> -		MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/python2"
> +		PYTHON_PACKAGE=python3
>  	fi
> +	MAKEFLAGS="$MAKEFLAGS PYTHON_PATH=/usr/bin/$PYTHON_PACKAGE"
>  
>  	export GIT_TEST_HTTPD=true

Re: [PATCH v4 4/4] ci: install python on ubuntu

[Jiang Xin]

On Sun, Nov 27, 2022 at 8:30 AM Junio C Hamano <gitster@pobox.com> wrote:
> > Since the "ubuntu-latest" runner image has a higher version, so its safe
> > to use "python2" or "python3" package name.
>
> Makes sense.  Just out of curiousity (read: not a suggestion to
> change anything), what happens if you say "apt install python" on a
> recent system?

In order to reproduce, I start a docker container like  this:

    $ docker run -it --rm ubuntu:22.04 /bin/bash

Fetch the latest apt source list, by running:

    $ apt-get update

Then install python with the following errors:

    $ apt-get install python
    Reading package lists... Done
    Building dependency tree... Done
    Reading state information... Done
    Package python is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
is only available from another source
However the following packages replace it:
  2to3 python2-minimal python2 dh-python python-is-python3

E: Package 'python' has no installation candidate

We can also find python package name from packages.ubuntu.com:

1. form the link below, we can say Ubuntu 18.04 is the last version
(LTS?) which has python package:

    https://packages.ubuntu.com/search?keywords=python&searchon=names&suite=all&section=all

2. Ubuntu 20.04 is the first version (LTS) which rename its python
pacakge to python2 package:

    https://packages.ubuntu.com/search?keywords=python2&searchon=names&suite=all&section=all

> Will queue.  Thanks.

Thanks.

--
Jiang Xin

Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors

[Jiang Xin]

On Sun, Nov 27, 2022 at 8:24 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > Subject: Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors
>
> "cache" -> "catch", I think.

oops, it's a typo. I used to copy and paste the commit log to MS word
or Apple pages to check for typos, but this step is very cumbersome,
and I always forget it.

I see this has been fixed in your tree, so there is no need to send v5.

--
Jiang Xin

Re: [PATCH v4 4/4] ci: install python on ubuntu

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

>     $ apt-get install python
>     Reading package lists... Done
>     Building dependency tree... Done
>     Reading state information... Done
>     Package python is not available, but is referred to by another package.
> This may mean that the package is missing, has been obsoleted, or
> is only available from another source
> However the following packages replace it:
>   2to3 python2-minimal python2 dh-python python-is-python3
>
> E: Package 'python' has no installation candidate

OK, I see.  Thanks.  I wondered if "python" redirects to either one
of these versions.  Not that I would suggest another approach based
on such a redirection, even if it existed--as I said, the question
was purely out of curiosity.