Re: [PATCH] ci: only warn about perforce/git-lfs/JGit on platforms that need them

[Patrick Steinhardt <ps@pks.im>]

On Sat, Sep 12, 2026 at 02:38:02PM +0000, Harald Nordgren via GitGitGadget wrote:
> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index 2f61fbb07c..a68cec64b4 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -171,30 +171,38 @@ 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
> +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)"
> -	git-lfs version
> -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)"
> -	jgit version
> -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

I wonder whether it makes sense to have these warnings in the first
place.

Part of the reason why we have these checks is that we allow the
installation of these tools to fail, and if so we know to gracefully
continue anyway. Tests will be skipped, and the pipeline will be green
in such a case. But is that even a safe thing to do? I strongly doubt
that we'd start to notice such failures anytime soon, so it very much
gives us a false sense of confidence.

So I'd suggest that instead of warning, we should make the whole build
fail outright if we fail to install any of those tools. And once we do,
these warnings here become quite useless, because we know that the build
would fail on platforms where we expect the tools to be present. And on
platforms where we don't, the warning is pointless anyway.

Thanks!

Patrick