git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Derrick Stolee" <stolee@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Taylor Blau" <me@ttaylorr.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 2/3] CI: don't care about SHA256 mismatch on upstream "perforce" package
Date: Fri, 22 Apr 2022 11:07:38 +0200	[thread overview]
Message-ID: <patch-v2-2.3-2805e89623c-20220422T085958Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v2-0.3-00000000000-20220422T085958Z-avarab@gmail.com>

As can be seen in the commit history[1] of the upstream perforce.rb in
homebrew-cask the upstream perforce package URL and its SHA-256 are
aren't a unique pair. The upstream will start publishing an updated
package at the same URL as the previous version, causing the CI to
routinely fail with errors like:

	==> Downloading https://cdist2.perforce.com/perforce/r21.2/bin.macosx1015x86_64/helix-core-server.tgz
	Error: SHA256 mismatch
	Expected: ffc757b9d4d0629b2594e2777edfb18097825e29c70d8f33a444c7482d622806
	  Actual: 37bc306f0bdfd1d63cfcea113ada132d96f89d53cbb20c282735d51d06223054

Once someone gets around to updating the perforce.rb the failure of
git's CI will be cleared up, but in the meantime all osx-{gcc,clang}
jobs will encounter hard failures.

Let's not be so anal about this and fallback to a "sha256 :no_check"
on failure. For the "ubuntu-latest" jobs just a few lines earlier we
"wget" and chmod binaries from http://filehost.perforce.com (no
https!).

We're already trusting the DNS in the CI to do the right thing here,
and for there not to be any MitM attacks between the CI and
filehost.perforce.com. Even if someone managed to get in the middle
the worst they could do is run arbitrary code in the CI environment.

The only thing we were getting out of the SHA-256 check was to serve
as a canary that the homebrew-cask repository itself was drifting out
of date. It seems sensible to just cut it out as a middle-man
entirely (as the ubuntu-latest jobs do). We want to run the
t/*-git-p4-*.sh tests, not to validate the chain of custody between
perforce.com and the homebrew-cask repository.

See [2] for the "no_check" syntax, and [3] for an example of a failure
in "seen" before this change.

It's been suggested as an alternative [4] to simply disable the p4
tests if we can't install the package from homebrew. While I don't
really care, I think that would be strictly worse. Before this change
we've either run the p4 tests or failed, and we'll still fail now if
we can't run the p4 tests.

Whereas skipping them entirely as [4] does is introducing the caveat
that our test coverage in these jobs today might be different than the
coverage tomorrow. If we do want to introduce such dynamic runs to CI
I think they should use the relevant "needs" or "if" features, so that
the UX can make it obvious what was or wasn't dynamically skipped.

1. https://github.com/Homebrew/homebrew-cask/commits/master/Casks/perforce.rb
2. https://docs.brew.sh/Cask-Cookbook#required-stanzas
3. https://github.com/git/git/runs/6104156856?check_suite_focus=true
4. https://lore.kernel.org/git/20220421225515.6316-1-carenas@gmail.com/
---
 ci/install-dependencies.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
index 82fa87f97af..cab6e04a358 100755
--- a/ci/install-dependencies.sh
+++ b/ci/install-dependencies.sh
@@ -37,7 +37,14 @@ macos-latest)
 	test -z "$BREW_INSTALL_PACKAGES" ||
 	brew install $BREW_INSTALL_PACKAGES
 	brew link --force gettext
-	brew install perforce
+	brew install perforce || {
+		echo Installing perforce failed, assuming and working around SHA256 mismatch >&2 &&
+
+		path=$(brew edit --print-path perforce) &&
+		sed 's/\(sha256.\).*/\1:no_check/' <"$path" >"$path".tmp &&
+		mv "$path".tmp "$path" &&
+		brew install perforce
+	}
 
 	if test -n "$CC_PACKAGE"
 	then
-- 
2.36.0.879.g56a83971f3f


  parent reply	other threads:[~2022-04-22  9:14 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-21 12:53 [PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-match Ævar Arnfjörð Bjarmason
2022-04-21 12:53 ` [PATCH 1/2] CI: run "brew install perforce" without past workarounds Ævar Arnfjörð Bjarmason
2022-04-21 12:53 ` [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream "perforce" package Ævar Arnfjörð Bjarmason
2022-04-21 20:47   ` Junio C Hamano
2022-04-21 21:08     ` Ævar Arnfjörð Bjarmason
2022-04-21 21:29       ` Eric Sunshine
2022-04-21 21:38         ` Junio C Hamano
2022-04-21 21:48           ` Eric Sunshine
2022-04-21 22:41             ` Junio C Hamano
2022-04-22  9:22               ` Ævar Arnfjörð Bjarmason
2022-04-21 21:30 ` [PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-matchy Carlo Marcelo Arenas Belón
2022-04-21 21:39   ` Junio C Hamano
2022-04-22  9:21   ` Ævar Arnfjörð Bjarmason
2022-04-21 21:57 ` [PATCH 0/2] CI: don't fail OSX tests due to brew v.s. perforce.com mis-match Junio C Hamano
2022-04-22  9:07 ` [PATCH v2 0/3] " Ævar Arnfjörð Bjarmason
2022-04-22  9:07   ` [PATCH v2 1/3] CI: run "brew install perforce" without past workarounds Ævar Arnfjörð Bjarmason
2022-04-22  9:52     ` Carlo Arenas
2022-04-22 18:46       ` Ævar Arnfjörð Bjarmason
2022-04-22  9:07   ` Ævar Arnfjörð Bjarmason [this message]
2022-04-22 11:15     ` [PATCH v2 2/3] CI: don't care about SHA256 mismatch on upstream "perforce" package Carlo Arenas
2022-04-22  9:07   ` [PATCH v2 3/3] CI: use https, not http to download binaries from perforce.com Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v2-2.3-2805e89623c-20220422T085958Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=me@ttaylorr.com \
    --cc=stolee@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).