Re: [PATCH 2/2] CI: don't care about SHA256 mismatch on upstream "perforce" package

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> In the obscure (but unlikely to ever happen) that the failure is
> specifically because perforce.com published a bad updated package, and
> it a failure that their testing wouldn't have caught, but whoever's
> updating the homebrew SHA-256 recipe would have caught, we will have a
> failure in our p4 tests that we wouldn't have otherwise had.

Or DNS the CI site consults is tainted and we got a bad package from
a fake perforce.com?

> @@ -37,7 +37,13 @@ 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 &&

I had to read this three times before realizing what you are
"assuming".  You suspect without having a way to verify that SHA-256
mismatch was the reason why the attempt to install failed, and
working it around.  Makes sense.

What does it buy us to do this only as a fallback?  If we munged the
$path to disable sha256 checking before the initial "brew install",
we would install it happily if the package is the correct one, and
if it is not a kosher one, we'd install it anyway.

Is it so that we can tell if we had the checksum mismatch or not?
It is unfortunate that no_check is the only "special" value for the
field (I would have loved to use "warn_only" if it were available).

> +
> +		path=$(brew edit --print-path perforce) &&
> +		sed -i -e 's/\(sha256.\).*/\1:no_check/' "$path" &&

"sed -i" is not POSIX and without macOS box I do not know if it
works there.  FreeBSD sed manual seems to indicate they have

	sed -i <extension>

In our current codebase, "sed -i" appears to be used only in vcxproj
part of config.mak.uname

I would usually have said that "I'd rather see us not to use it
here, to prevent others from copying and pasting it, if it can be
helped", but this is very much macOS specific part of an obscure
corner of the source tree, so as long as we are sure it works there,
and if it is too cumbersome to avoid editing in-place, I'd let it
go.

Ah, no, I'd say we should NOT use "sed -i" here, not in the form in
this patch, after seeing

  https://unix.stackexchange.com/questions/401905/bsd-sed-vs-gnu-sed-and-i

but that is 4-year old information, so...

> +		brew install perforce
> +	}
>  
>  	if test -n "$CC_PACKAGE"
>  	then