From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 25 Nov 2022 04:48:55 GMT
Subject: Re: [PATCH v3 2/4] ci: show error message of "p4 -V"
Message-ID: <xmqq8rjzhc5k.fsf@gitster.g>
In-Reply-To: <221124.86wn7knx1x.gmgdl@evledraar.gmail.com>

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

>>  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 | grep Rev. || { echo >&2 "p4d: bad version"; p4d -V; exit 1; }
>>  	echo "$(tput setaf 6)Perforce Client Version$(tput sgr0)"
>> -	p4 -V | grep Rev.
>> +	p4 -V | grep Rev. || { echo >&2 "p4: bad version"; p4 -V; exit 1; }
>>  else
>>  	echo >&2 "WARNING: perforce wasn't installed, see above for clues why"
>>  fi
>
> I think it makes sense to detect this, but the specific remedy makes no
> sense to me.
> ...
> So we want to show that it segfaults, but how it is useful once that
> command fails to pretend that it's a "bad version?" The issue is that
> the command can't show the version at all.
>
> 	$ { echo >&2 "p4d: bad version"; ./p4d -V; exit 1; }
> 	p4d: bad version
> 	Segmentation fault
> 	exit
> ...
> But I don't see the point of that effort, we won't look at these trace
> logs unless something fails, so just including the raw output seems most
> sensible.

Yup, makes sense.

Thanks, both, for working on this topic.