On Thu, Nov 24, 2022 at 7:01 PM Ævar Arnfjörð Bjarmason
[off-list ref] 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.

quoted

      # 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