[PoC] ci: Homebrew strikes again, let's try some workarounds on curre…

[szeder]

…nt master

This is just a small experiment.

[dscho]

@szeder how about doing something like this instead (see git-for-windows@2aaa68e)?

commit 2aaa68e3a232a1353d6d796e9466428b22aaea4b
Author: Johannes Schindelin <[email protected]>
Date:   Fri Oct 11 14:54:07 2019 +0200

    ci(osx): use new location of the `perforce` cask
    
    ... falling back to the old one.
    
    Signed-off-by: Johannes Schindelin <[email protected]>

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 85a9d6b15cd..489b5dcf173 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -40,6 +40,7 @@ osx-clang|osx-gcc)
 	test -z "$BREW_INSTALL_PACKAGES" ||
 	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
+	brew cask install perforce ||
 	brew install caskroom/cask/perforce
 	case "$jobname" in
 	osx-gcc)

That way, it works no matter how old/new the image happens to be on the build agent...

[dscho]

how about doing something like this instead (see git-for-windows@2aaa68e)?

Just for the record: this passes the CI...

[szeder]

On Fri, Oct 11, 2019 at 02:47:45PM -0700, Johannes Schindelin wrote: @szeder how about doing something like this instead (see git-for-windows@2aaa68e)?

Heh, I take it that you stumbled upon that ``` +brew install caskroom/cask/perforce Error: caskroom/cask was moved. Tap homebrew/cask-cask instead. ``` error as well. Good.

```diff + brew cask install perforce || brew install caskroom/cask/perforce ``` That way, it works no matter how old/new the image happens to be on the build agent...

It's reassuring to see that you came up with essentially the same solution. I tried to make sense of that "Tap homebrew/cask-cask instead" suggestion, but couldn't get anything working out of it. However, I don't think it's worth keeping that old install command around. As you know, Homebrew tends to keep us entertained by breaking overnight every couple of months, and sometimes we don't have to do anything just wait until the issue sorts itself out. Back when there was some quarantine-related breakage I sent a patch to work it around, but there were some open questions with that approach, so the patch wasn't picked up, and the issue eventually went away. https://public-inbox.org/git/[email protected]/ The point is that the command `brew cask install perforce` apparently worked already over a year ago. I think that for our intents and purposes (building cooking topics and PRs on Travis CI and Azure Pipelines) that's "old enough".

[dscho]

I don't think it's worth keeping that old install command around.

It's not an old "install command", but an "old install" command.

In other words, I do operate Azure Pipelines in places (e.g. git/git) where the macOS agent does have an older Homebrew, and since we no longer brew update, you would break that.

I am not a fan of breaking existing setups, even if it requires slightly ugly workarounds. Especially when those workarounds save me time (I look at every failed CI build in git/git and gitgitgadget/git and git-for-windows/git).

The point is that the command brew cask install perforce apparently worked already over a year ago. I think that for our intents and purposes (building cooking topics and PRs on Travis CI and Azure Pipelines) that's "old enough".

Not sure about that.

I, for one, often go directly into the Azure Pipelines/AppVeyor/Travis configuration when I need to build a project that I do not know yet. They typically have much more up to date information about building than any README.md and/or INSTALL.

Does it really hurt to keep that workaround, just to please some sort of OCD? ;-)

[szeder]

On Sat, Oct 12, 2019 at 10:20:46AM -0700, Johannes Schindelin wrote: Does it _really_ hurt to keep that workaround, just to please some sort of OCD? ;-)

Well, it's not just about pleasing my OCD... Keeping the old command as a fallback does actually hurt on the oldest images (IMO), but, as it turns out, it does help in one of the more recent images. Therefore now I'm inclined to agree with keeping that old command as well. Once upon a time Homebrew offered Perforce as a regular package, but around mid January 2017 it was moved to a "cask"; see 672f51c (travis-ci: fix Perforce install on macOS, 2017-01-22). This means that neither of those two commands can install Perforce in any image created before that. Travis CI currently has 16 macOS images [1] ranging from Xcode 6.4 to 11.2, with 9.4 being the default. I couldn't find any info about when any of those images were created, but three of them (6.4, 7.3 and 8) seem to be older than Jan 2017. Xcode 8.3 is the first version that was released after Jan 2017 [2]. And indeed, both commands work fine in most of the xcode8.3 and later images [3], but neither of those two commands work in the oldest three images. However, there is a significant difference: - `brew cask install perforce` fails with "Error: No available Cask for perforce" [4] in the oldest three images, thus breaking the build, as expected. - `brew install caskroom/cask/perforce` is simply buggy in the oldest three images: it doesn't fail, but prints, among other things, "Error: Tap caskroom/cask already tapped" (whatever it might mean) with a stack trace, and doesn't install what it was supposed to... yet it still exits with success! [5] Consequently, a build could eventually succeed without running all the tests that we want to run. In my book this qualifies for broken. Of course you could argue, if you want, that silently not installing Perforce only results in skipping the tests of an ancillary component (git p4), so you don't care at all... but please don't waste any time on it and read on instead. There is an issue with the xcode10.3 image: it seems that it was created while the Homebrew database contained a weird checksum mismatch for Perforce. I wrote "weird", because `brew cask install perforce` chokes on it [6] and thus would break the build, but `brew install caskroom/cask/perforce` somehow doesn't, although it seems to verify the checksum as well [7]. I won't even try to make sense of it :) Anyway, I don't know when other CI systems created their macOS images. If they are lucky, they might have just avoided that (presumably short) time window when the Homebrew database contained this bogus checksum. OTOH, this might not be the only time when there was such a checksum mismatch, and other CI systems might have happened to hit those other cases, and created an image with a similar issue, just with other Xcode and/or Perforce versions. Dunno. So, given that this issue is in a fairly recent image (newer than Travis CI's current default macOS image), I, too, think that we should keep the old command as a fallback. I also think that this warrants a comment above those two ||-chained commands. Feel free to use anything from what I wrote above when updating the log message of that GfW commit. [1] https://docs.travis-ci.com/user/reference/osx/#macos-version but the oldest one showed me a warning that it's "deprecated and will be removed in January 2019" (yes, 10 months ago :) [2] https://en.wikipedia.org/wiki/Xcode#8.x_series [3] https://travis-ci.org/szeder/git/builds/597248288 https://travis-ci.org/szeder/git/builds/597248368 A stripped-down build config, does nothing more than tries to install P4 and checks that it was successfully installed. [4] https://travis-ci.org/szeder/git/jobs/597248371#L52 [5] https://travis-ci.org/szeder/git/jobs/597248291#L53 [6] https://travis-ci.org/szeder/git/jobs/597248381#L93 [7] https://travis-ci.org/szeder/git/jobs/597248301#L101