On Thu, Nov 24, 2022 at 7:01 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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
