git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Jiang Xin <worldhello.net@gmail.com>
Cc: Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Jiang Xin <zhiyou.jx@alibaba-inc.com>
Subject: Re: [PATCH v2 2/3] ci: upgrade version of p4 to 21.2
Date: Thu, 24 Nov 2022 11:55:20 +0100	[thread overview]
Message-ID: <221124.86ilj4pqfn.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20221124090545.4790-3-worldhello.net@gmail.com>


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


  reply	other threads:[~2022-11-24 11:02 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-23 15:02 [PATCH 0/2] Use fixed github-actions runner image Jiang Xin
2022-11-23 15:02 ` [PATCH 1/2] github-actions: run gcc-8 on ubuntu-20.04 image Jiang Xin
2022-11-24  8:11   ` Johannes Schindelin
2022-11-23 15:02 ` [PATCH 2/2] ci: upgrade version of p4 Jiang Xin
2022-11-24  8:16   ` Johannes Schindelin
2022-11-24  8:41     ` Johannes Schindelin
2022-11-24  8:54       ` Johannes Schindelin
2022-11-24  9:17         ` Jiang Xin
2022-11-24  9:41           ` Johannes Schindelin
2022-11-24  9:15     ` Jiang Xin
2022-11-24  8:18 ` [PATCH 0/2] Use fixed github-actions runner image Johannes Schindelin
2022-11-24  9:05 ` [PATCH v2 0/3] Fix broken CI on newer " Jiang Xin
2022-11-24  9:44   ` Johannes Schindelin
2022-11-24 10:48     ` Johannes Schindelin
2022-11-24 11:23       ` Jiang Xin
2022-11-24 12:28       ` python 2 EOL (was: [PATCH v2 0/3] Fix broken CI on newer github-actions runner image) Ævar Arnfjörð Bjarmason
2022-11-25  7:11         ` python 2 EOL Junio C Hamano
2022-11-24 15:39   ` [PATCH v3 0/4] Fix broken CI on newer github-actions runner image Jiang Xin
2022-11-25  9:59     ` [PATCH v4 " Jiang Xin
2022-11-25  9:59     ` [PATCH v4 1/4] github-actions: run gcc-8 on ubuntu-20.04 image Jiang Xin
2022-11-27  0:24       ` Junio C Hamano
2022-11-25  9:59     ` [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors Jiang Xin
2022-11-27  0:24       ` Junio C Hamano
2022-11-27  9:14         ` Jiang Xin
2022-11-25  9:59     ` [PATCH v4 3/4] ci: p4 on Linux has the same version as on macOS Jiang Xin
2022-11-27  0:28       ` Junio C Hamano
2022-11-25  9:59     ` [PATCH v4 4/4] ci: install python on ubuntu Jiang Xin
2022-11-27  0:30       ` Junio C Hamano
2022-11-27  9:01         ` Jiang Xin
2022-11-27 23:36           ` Junio C Hamano
2022-11-24 15:39   ` [PATCH v3 1/4] github-actions: run gcc-8 on ubuntu-20.04 image Jiang Xin
2022-11-24 16:29     ` Ævar Arnfjörð Bjarmason
2022-11-24 15:39   ` [PATCH v3 2/4] ci: show error message of "p4 -V" Jiang Xin
2022-11-24 16:10     ` Ævar Arnfjörð Bjarmason
2022-11-25  4:48       ` Junio C Hamano
2022-11-24 15:39   ` [PATCH v3 3/4] ci: p4 on Linux has the same version as on macOS Jiang Xin
2022-11-24 15:39   ` [PATCH v3 4/4] ci: install python on ubuntu Jiang Xin
2022-11-24  9:05 ` [PATCH v2 1/3] github-actions: run gcc-8 on ubuntu-20.04 image Jiang Xin
2022-11-24 10:46   ` Ævar Arnfjörð Bjarmason
2022-11-25  7:21     ` Junio C Hamano
2022-11-24  9:05 ` [PATCH v2 2/3] ci: upgrade version of p4 to 21.2 Jiang Xin
2022-11-24 10:55   ` Ævar Arnfjörð Bjarmason [this message]
2022-11-24 12:56     ` Jiang Xin
2022-11-24  9:05 ` [PATCH v2 3/3] ci: install python on ubuntu Jiang Xin
2022-11-24 11:02   ` Ævar Arnfjörð Bjarmason
2022-11-24 11:37     ` Jiang Xin
2022-11-24 12:23       ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=221124.86ilj4pqfn.gmgdl@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=worldhello.net@gmail.com \
    --cc=zhiyou.jx@alibaba-inc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).