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

[Jiang Xin <worldhello.net@gmail.com>]

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