Re: [PATCH 1/2] test-lib: show missing prereq summary

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 17, 2021, 08:19 UTC
Message-ID: <xmqqpmqz42kj.fsf@gitster.g>
In-Reply-To: <20211116141938.gbaufny5o2boptvh@fs>

Fabian Stelzer <fs@gigacodes.de> writes:

Show 27 quoted lines

> On 15.11.2021 18:44, Ævar Arnfjörð Bjarmason wrote:
>>
>>On Mon, Nov 15 2021, Fabian Stelzer wrote:
>>
>>> +if test -n "$missing_prereq"
>>> +then
>>> +	unique_missing_prereq=$(
>>> +		echo $missing_prereq |
>>> +		tr -s "," "\n" |
>>> +		grep -v '^$' |
>>> +		sort -u |
>>> +		paste -s -d ',')
>>
>>What is paste? Some out-of-tree debugging utility?
>>
>>I think you might find a better way to do this shown in my
>>"ab/generate-command-list" topic, currently in seen. It removed most of
>>the same sort of tr|grep|sort etc. chain in generate-cmdlist.sh.
>
> I've looked at the generate-command-list code and TBH i still think this
> is a better solution. If I read your change correctly you've removed the
> sort and unique completely since it was not necessary for the use-case.
> In this case i think it is. Since we call tr with `-s` the grep -v might
> not be strictly necessary though. Also in this case these commands are
> only called once at the end of the test run and not in any kind of loop
> like in the cmdlist code so i think this variant is much easier to read
> and debug with a negligible performance impact.

I tend to agree with you. The snippet we see above is quite straight-forward not over-engineered.

> I tried writing a sh only variant and this is what i came up with. Not
> sure if this could be much more simplified. It looses the sort though.

Fun, but I'd rather not go there, unless this is a performance critical bit, which it is not.

Thanks.

Show 30 quoted lines

>
> input="PCRE,JGIT2,JGIT2,,PCRE,JGIT2,PCRE,PCRE2,!PCRE,!WINDOWS,GPG,GPGSSH,PCRE,!GPG,GPG,JGIT2"
>
> unique=
> save_IFS=$IFS
> IFS=,
> for prereq in $input
> do
> 	case "$prereq" in
> 	'')
> 		# Skip empty entries
> 		;;
> 	*)
> 		case ",$unique," in
> 		*,$prereq,*)
> 			# Skip over duplicates
> 			;;
> 		*)
> 			if test -z "$unique"
> 			then
> 				unique="$prereq"
> 			else
> 				unique="$unique,$prereq"
> 			fi
> 			;;
> 		esac
> 	esac
> done
> IFS=$save_IFS
> echo $unique