Re: [PATCH v2] ci: fail the build if perforce, git-lfs, or JGit fail to install

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

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> A failed install of perforce, git-lfs, or JGit was silently
> tolerated. The corresponding tests would just skip and the build
> stayed green, so a broken download could go unnoticed indefinitely.
>
> Fail the build immediately instead on the platforms that install
> these tools. The presence checks and warnings at the end of the
> script are gone too, since we already know by then that everything
> installed or the build wouldn't have gotten this far.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> ...
>  ci/install-dependencies.sh | 59 +++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 33 deletions(-)
>
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index d57dce5663..dad35db78b 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -95,21 +95,33 @@ ubuntu-*|i386/ubuntu-*|debian-*)
>  			"$P4WHENCE/bin.linux26x86_64/p4d" \
>  			"$P4WHENCE/bin.linux26x86_64/p4" &&
>  		chmod a+x "$CUSTOM_PATH/p4d" "$CUSTOM_PATH/p4" || {
> -			rm -f "$CUSTOM_PATH/p4"
> -			rm -f "$CUSTOM_PATH/p4d"
> +			echo >&2 "error: could not install perforce"
> +			exit 1
>  		}

We used to attempt cleaning after ourselves before giving up (and
continuing), but now we explicitly die and there is no need to
remove the half-installed stuff, which makes sense.  The same
comment for lfs and jgit (omitted from quotes).

> @@ -121,9 +133,16 @@ macos-*)
>  
>  	mkdir -p "$CUSTOM_PATH"
>  	wget -q "$P4WHENCE/bin.macosx12arm64/helix-core-server.tgz" &&
> -	tar -xf helix-core-server.tgz -C "$CUSTOM_PATH" p4 p4d &&
> +	tar -xf helix-core-server.tgz -C "$CUSTOM_PATH" p4 p4d || {
> +		echo >&2 "error: could not install perforce"
> +		exit 1
> +	}
>  	sudo xattr -d com.apple.quarantine "$CUSTOM_PATH/p4" "$CUSTOM_PATH/p4d" 2>/dev/null || true
>  	rm helix-core-server.tgz

The earlier one tried to make them executable, and failed when it
cannot. I wonder you want to move "|| { ... fail ... }" below the
xattr line for consistency, but the command line says "we do not
care if we cannot remove the attribute and will keep going", so it
cannot be done without dropping "|| true" there.  Hmm...

> +	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
> +	p4d -V
> +	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
> +	p4 -V
>  
>  	case "$jobname" in
>  	osx-meson)
> @@ -171,30 +190,4 @@ Documentation)
>  	;;
>  esac
>  
> -if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
> -then
> ...
> -	jgit version
> -else
> -	echo >&2 "::warning:: JGit wasn't installed, see above for clues why"
> -fi
> -

And it is very nice to see these all go, as we have done the
reporting all very close to where they are installed.

>  end_group "Install dependencies"
>
> base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e