diff mbox series

[v2,1/3] CI: run "brew install perforce" without past workarounds

Message ID patch-v2-1.3-dcedf03c2d7-20220422T085958Z-avarab@gmail.com (mailing list archive)
State New
Headers show
Series CI: don't fail OSX tests due to brew v.s. perforce.com mis-match | expand

Commit Message

Ævar Arnfjörð Bjarmason April 22, 2022, 9:07 a.m. UTC
Remove the alternating between --no-quarantine, --cask and fallback
"git pull" updating of the "perforce" package.

As can be seen in [1], [2] and [3] these were workarounds for various
past CI issues. Running "brew install perforce" works now in GitHub
CI, so there's no need to alternate between package names, and the
"git pull" method was a workaround for some staleness issue on the
Azure pipelines removed in [4].

We do have a really common issue with this failing, but that's
unrelated to any of those past fixes, and removing these old
workarounds makes dealing with that a lot easier.

1. 0eb3671ed96 (ci(osx): use new location of the `perforce` cask, 2019-10-23)
2. 5ed9fc3fc86 (ci: prevent `perforce` from being quarantined, 2020-02-27)
3. 3831132ace6 (ci/install-depends: attempt to fix "brew cask" stuff, 2021-01-14)
4. 6081d3898fe (ci: retire the Azure Pipelines definition, 2020-04-11)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 ci/install-dependencies.sh | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Comments

Carlo Marcelo Arenas Belón April 22, 2022, 9:52 a.m. UTC | #1
On Fri, Apr 22, 2022 at 2:07 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index dbcebad2fb2..82fa87f97af 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -37,13 +37,7 @@ macos-latest)
>         test -z "$BREW_INSTALL_PACKAGES" ||
>         brew install $BREW_INSTALL_PACKAGES
>         brew link --force gettext
> -       brew install --cask --no-quarantine perforce || {
> -               # Update the definitions and try again
> -               cask_repo="$(brew --repository)"/Library/Taps/homebrew/homebrew-cask &&
> -               git -C "$cask_repo" pull --no-stat --ff-only &&
> -               brew install --cask --no-quarantine perforce
> -       } ||
> -       brew install homebrew/cask/perforce
> +       brew install perforce

While this might work under the current VM configuration used by
github actions, is definitely not the usual configuration in macOS
installations and therefore likely to break if run locally (as some
other on the fly changes attempt to suggest)

keeping the "--no-quarantine" makes for a less likely to fail option
(since SIP is enabled by default), and therefore I am also concerned
that by removing all these other (learned the hard way) workarounds we
might be making this more fragile for the future as well.

instead of this rewrite of the brew interface logic, removing brew as
you suggested would be probably better.

Carlo
Ævar Arnfjörð Bjarmason April 22, 2022, 6:46 p.m. UTC | #2
On Fri, Apr 22 2022, Carlo Arenas wrote:

> On Fri, Apr 22, 2022 at 2:07 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
>> index dbcebad2fb2..82fa87f97af 100755
>> --- a/ci/install-dependencies.sh
>> +++ b/ci/install-dependencies.sh
>> @@ -37,13 +37,7 @@ macos-latest)
>>         test -z "$BREW_INSTALL_PACKAGES" ||
>>         brew install $BREW_INSTALL_PACKAGES
>>         brew link --force gettext
>> -       brew install --cask --no-quarantine perforce || {
>> -               # Update the definitions and try again
>> -               cask_repo="$(brew --repository)"/Library/Taps/homebrew/homebrew-cask &&
>> -               git -C "$cask_repo" pull --no-stat --ff-only &&
>> -               brew install --cask --no-quarantine perforce
>> -       } ||
>> -       brew install homebrew/cask/perforce
>> +       brew install perforce
>
> While this might work under the current VM configuration used by
> github actions, is definitely not the usual configuration in macOS
> installations and therefore likely to break if run locally (as some
> other on the fly changes attempt to suggest)
>
> keeping the "--no-quarantine" makes for a less likely to fail option
> (since SIP is enabled by default), and therefore I am also concerned
> that by removing all these other (learned the hard way) workarounds we
> might be making this more fragile for the future as well.

It works with the current CI, and keeping those fallbacks would have
meant turning this into some for-loop where we track which command
variant failed exactly, then retrying that with the SHA-256 munging.

> instead of this rewrite of the brew interface logic, removing brew as
> you suggested would be probably better.

I won't have time to pursue that in the near future, sorry. Especially
as I've got no OSX box, so any changes to this series mean long painful
bouncing around against the GitHub CI.

Anyway. I'd be happy for you to pick this up, whether that means
re-rolling my series with your suggested changes or taking yours over
mine I really don't care, I just want to not see those OSX CI errors
anymore.

As noted I have a mild preference out of general principle of having the
CI at least somewhat deterministic, i.e. to do this series where if we
can't get p4 at all we'll fail.

But I'd be fine either way, i.e. your series is also fine by
me. Whatever stops these errors from happening whenever perforce.com
updates the packages & the brew recipes haven't been bumped yet...

Thanks!
diff mbox series

Patch

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index dbcebad2fb2..82fa87f97af 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -37,13 +37,7 @@  macos-latest)
 	test -z "$BREW_INSTALL_PACKAGES" ||
 	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
-	brew install --cask --no-quarantine perforce || {
-		# Update the definitions and try again
-		cask_repo="$(brew --repository)"/Library/Taps/homebrew/homebrew-cask &&
-		git -C "$cask_repo" pull --no-stat --ff-only &&
-		brew install --cask --no-quarantine perforce
-	} ||
-	brew install homebrew/cask/perforce
+	brew install perforce
 
 	if test -n "$CC_PACKAGE"
 	then