| Message ID | 20221124090545.4790-3-worldhello.net@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
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".
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
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"