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

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

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: fail the build if perforce, git-lfs or JGit fail to install
    
    Fail the build if perforce, git-lfs or JGit fail to install on the
    platforms that install them.
    
    Changes in v2:
    
     * Perforce, git-lfs, and JGit install failures now fail the build
       instead of being silently swallowed. Removed the presence checks and
       warnings at the end of the script, dead code once install failure is
       fatal.
     * Version output (p4d -V, git-lfs version, jgit version) moved inline
       right after each successful install.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2403%2FHaraldNordgren%2Fci-scope-optional-tool-warnings-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2403/HaraldNordgren/ci-scope-optional-tool-warnings-v2
Pull-Request: https://github.com/git/git/pull/2403

Range-diff vs v1:

 1:  3ce51dc322 ! 1:  dde1b38567 ci: only warn about perforce/git-lfs/JGit on platforms that need them
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    ci: only warn about perforce/git-lfs/JGit on platforms that need them
     +    ci: fail the build if perforce, git-lfs, or JGit fail to install
      
     -    perforce, git-lfs, and JGit test git's own interop code, not anything
     -    platform-specific, so installing them once on ubuntu-* (all three)
     -    and macos-* (perforce) is enough coverage. debian, i386/ubuntu,
     -    alpine, fedora and almalinux never install them, yet the presence
     -    check at the end of the script warned on all of them anyway.
     +    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.
      
     -    Scope each check to the platforms that attempt the install, so a
     -    warning means one actually failed.
     +    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 ##
     +@@ ci/install-dependencies.sh: 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
     + 		}
     ++		echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
     ++		p4d -V
     ++		echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
     ++		p4 -V
     + 
     + 		wget --quiet \
     + 		     "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" &&
     + 		tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
     + 			-C "$CUSTOM_PATH" --strip-components=1 \
     + 			"git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs" &&
     +-		rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" ||
     +-		rm -f "$CUSTOM_PATH/git-lfs"
     ++		rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" || {
     ++			echo >&2 "error: could not install git-lfs"
     ++			exit 1
     ++		}
     ++		echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
     ++		git-lfs version
     + 
     + 		wget --quiet "$JGITWHENCE" --output-document="$CUSTOM_PATH/jgit" &&
     +-		chmod a+x "$CUSTOM_PATH/jgit" ||
     +-		rm -f "$CUSTOM_PATH/jgit"
     ++		chmod a+x "$CUSTOM_PATH/jgit" || {
     ++			echo >&2 "error: could not install JGit"
     ++			exit 1
     ++		}
     ++		echo "$(tput setaf 6)JGit Version$(tput sgr0)"
     ++		jgit version
     + 		;;
     + 	esac
     + 	;;
     +@@ ci/install-dependencies.sh: 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
     ++	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)
      @@ ci/install-dependencies.sh: Documentation)
       	;;
       esac
     @@ ci/install-dependencies.sh: Documentation)
      -else
      -	echo >&2 "::warning:: perforce wasn't installed, see above for clues why"
      -fi
     -+case "$distro" in
     -+ubuntu-*|macos-*)
     -+	if type p4d >/dev/null 2>&1 && type p4 >/dev/null 2>&1
     -+	then
     -+		echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
     -+		p4d -V
     -+		echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
     -+		p4 -V
     -+	else
     -+		echo >&2 "::warning:: perforce wasn't installed, see above for clues why"
     -+	fi
     -+	;;
     -+esac
     - 
     +-
      -if type git-lfs >/dev/null 2>&1
      -then
      -	echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
     @@ ci/install-dependencies.sh: Documentation)
      -else
      -	echo >&2 "::warning:: git-lfs wasn't installed, see above for clues why"
      -fi
     -+case "$distro" in
     -+ubuntu-*)
     -+	if type git-lfs >/dev/null 2>&1
     -+	then
     -+		echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
     -+		git-lfs version
     -+	else
     -+		echo >&2 "::warning:: git-lfs wasn't installed, see above for clues why"
     -+	fi
     - 
     +-
      -if type jgit >/dev/null 2>&1
      -then
      -	echo "$(tput setaf 6)JGit Version$(tput sgr0)"
     @@ ci/install-dependencies.sh: Documentation)
      -else
      -	echo >&2 "::warning:: JGit wasn't installed, see above for clues why"
      -fi
     -+	if type jgit >/dev/null 2>&1
     -+	then
     -+		echo "$(tput setaf 6)JGit Version$(tput sgr0)"
     -+		jgit version
     -+	else
     -+		echo >&2 "::warning:: JGit wasn't installed, see above for clues why"
     -+	fi
     -+	;;
     -+esac
     - 
     +-
       end_group "Install dependencies"


 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
 		}
+		echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
+		p4d -V
+		echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
+		p4 -V
 
 		wget --quiet \
 		     "$LFSWHENCE/git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" &&
 		tar -xzf "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" \
 			-C "$CUSTOM_PATH" --strip-components=1 \
 			"git-lfs-$LINUX_GIT_LFS_VERSION/git-lfs" &&
-		rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" ||
-		rm -f "$CUSTOM_PATH/git-lfs"
+		rm "git-lfs-linux-amd64-$LINUX_GIT_LFS_VERSION.tar.gz" || {
+			echo >&2 "error: could not install git-lfs"
+			exit 1
+		}
+		echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
+		git-lfs version
 
 		wget --quiet "$JGITWHENCE" --output-document="$CUSTOM_PATH/jgit" &&
-		chmod a+x "$CUSTOM_PATH/jgit" ||
-		rm -f "$CUSTOM_PATH/jgit"
+		chmod a+x "$CUSTOM_PATH/jgit" || {
+			echo >&2 "error: could not install JGit"
+			exit 1
+		}
+		echo "$(tput setaf 6)JGit Version$(tput sgr0)"
+		jgit version
 		;;
 	esac
 	;;
@@ -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
+	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
-	echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)"
-	p4d -V
-	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
-	p4 -V
-else
-	echo >&2 "::warning:: perforce wasn't installed, see above for clues why"
-fi
-
-if type git-lfs >/dev/null 2>&1
-then
-	echo "$(tput setaf 6)Git-LFS Version$(tput sgr0)"
-	git-lfs version
-else
-	echo >&2 "::warning:: git-lfs wasn't installed, see above for clues why"
-fi
-
-if type jgit >/dev/null 2>&1
-then
-	echo "$(tput setaf 6)JGit Version$(tput sgr0)"
-	jgit version
-else
-	echo >&2 "::warning:: JGit wasn't installed, see above for clues why"
-fi
-
 end_group "Install dependencies"

base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e
-- 
gitgitgadget