Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors

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

Jiang Xin <worldhello.net@gmail.com> writes:

> Subject: Re: [PATCH v4 2/4] ci: remove the pipe after "p4 -V" to cache errors

"cache" -> "catch", I think.

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> When installing p4 as a dependency, we used to pipe output of "p4 -V"
> and "p4d -V" to validate the installation and output a condensed version
> information. But this would hide potential errors of p4 and would stop
> with an empty output. E.g.: p4d version 16.2 running on ubuntu 22.04
> causes sigfaults.

... before it produces any output.

> By removing the pipe after "p4 -V" and "p4d -V", we may get a verbose
> output, and stop immediately on errors becuase we have "set -e" in

"because".

> "ci/lib.sh". Since we won't look at these trace logs unless something
> fails, so just including the raw output seems most sensible.

You started the sentence with "Since", so "so" should be dropped.

> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
>  ci/install-dependencies.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Well reasoned.  Will queue.  Thanks.

> diff --git a/ci/install-dependencies.sh b/ci/install-dependencies.sh
> index feefd6e9bb..97a1a1f574 100755
> --- a/ci/install-dependencies.sh
> +++ b/ci/install-dependencies.sh
> @@ -83,9 +83,9 @@ 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 | grep Rev.
> +	p4d -V
>  	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
> -	p4 -V | grep Rev.
> +	p4 -V
>  else
>  	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
>  fi