Re: [PATCH 1/3] ci: make p4 setup on macOS more robust

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]

On Thu, Jan 18, 2024 at 08:19:10AM +0100, Matthias Aßhauer wrote:
> 
> 
> On Mon, 15 Jan 2024, Patrick Steinhardt wrote:
> 
> > When setting up Perforce on macOS we put both `p4` and `p4d` into
> > "$HOME/bin". On GitHub CI this directory is indeed contained in the PATH
> > environment variable and thus there is no need for additional setup than
> > to put the binaries there. But GitLab CI does not do this, and thus our
> > Perforce-based tests would be skipped there even though we download the
> > binaries.
> > 
> > Refactor the setup code to become more robust by downloading binaries
> > into a separate directory which we then manually append to our PATH.
> > This matches what we do on Linux-based jobs.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> > ci/install-dependencies.sh | 10 ++++------
> > ci/lib.sh                  |  3 +++
> > 2 files changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> > index 4f407530d3..b4e22de3cb 100755
> > --- a/ci/install-dependencies.sh
> > +++ b/ci/install-dependencies.sh
> > @@ -37,15 +37,13 @@ macos-*)
> > 	test -z "$BREW_INSTALL_PACKAGES" ||
> > 	brew install $BREW_INSTALL_PACKAGES
> > 	brew link --force gettext
> > -	mkdir -p $HOME/bin
> > -	(
> > -		cd $HOME/bin
> > +
> > +	mkdir -p "$P4_PATH"
> > +	pushd "$P4_PATH"
> > 		wget -q "$P4WHENCE/bin.macosx1015x86_64/helix-core-server.tgz" &&
> > 		tar -xf helix-core-server.tgz &&
> > 		sudo xattr -d com.apple.quarantine p4 p4d 2>/dev/null || true
> > -	)
> > -	PATH="$PATH:${HOME}/bin"
> > -	export PATH
> 
> Didn't this put "$HOME/bin" on the PATH? And isn't the main premise of this
> patch that "$HOME/bin" is not on the PATH?
> or is the issue mainly about where we modify and export PATH and was masked
> by GitHub Actions already having "$HOME/bin" on the PATH?

Yes and no. While these lines put it in PATH, this only works inside of
"ci/install-dependencies.sh". When we call "ci/run-build-and-test.sh" we
do not source this script though, which means that "$HOME/bin" will not
be part of PATH during the actual test run unless it was already added
by the CI.

I'll update the commit message to explain this better.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]