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