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

From: Matthias Aßhauer <mha1993@live.de>
Date: Jan 18, 2024, 07:19 UTC
Message-ID: <DB9P250MB0692629640B05593B798E5A0A5712@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM>
In-Reply-To: <a5d725bea7b2cc2b8f66682920dbba016e89da9e.1705318985.git.ps@pks.im>

On Mon, 15 Jan 2024, Patrick Steinhardt wrote:

Show 37 quoted lines

> 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?

Show 22 quoted lines

> +	popd
>
> 	if test -n "$CC_PACKAGE"
> 	then
> diff --git a/ci/lib.sh b/ci/lib.sh
> index c749b21366..f631206a44 100755
> --- a/ci/lib.sh
> +++ b/ci/lib.sh
> @@ -344,6 +344,9 @@ macos-*)
> 	then
> 		MAKEFLAGS="$MAKEFLAGS APPLE_COMMON_CRYPTO_SHA1=Yes"
> 	fi
> +
> +	P4_PATH="$HOME/custom/p4"
> +	export PATH="$P4_PATH:$PATH"
> 	;;
> esac
>
> -- 
> 2.43.GIT
>
>