Re: [PATCH 1/3] ci: make p4 setup on macOS more robust
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 18, 2024, 09:44 UTC
- Message-ID
- <ZajzDNR1MtbV7zxp@tanuki>
- In-Reply-To
- <DB9P250MB0692629640B05593B798E5A0A5712@DB9P250MB0692.EURP250.PROD.OUTLOOK.COM>
On Thu, Jan 18, 2024 at 08:19:10AM +0100, Matthias Aßhauer wrote:
Show 46 quoted lines
>
>
> 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