From: Junio C Hamano <gitster@pobox.com>
To: Fabian Stelzer <fs@gigacodes.de>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
git@vger.kernel.org, "Adam Dinwoodie" <adam@dinwoodie.org>,
"Jeff King" <peff@peff.net>
Subject: Re: [PATCH 1/2] test-lib: show missing prereq summary
Date: Wed, 17 Nov 2021 00:19:08 -0800 [thread overview]
Message-ID: <xmqqpmqz42kj.fsf@gitster.g> (raw)
In-Reply-To: 20211116141938.gbaufny5o2boptvh@fs
Fabian Stelzer <fs@gigacodes.de> writes:
> 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.
>
> 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
next prev parent reply other threads:[~2021-11-17 8:19 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-15 16:07 [PATCH 0/2] test-lib: improve missing prereq handling Fabian Stelzer
2021-11-15 16:07 ` [PATCH 1/2] test-lib: show missing prereq summary Fabian Stelzer
2021-11-15 17:44 ` Ævar Arnfjörð Bjarmason
2021-11-15 17:53 ` Junio C Hamano
2021-11-15 19:56 ` Fabian Stelzer
2021-11-15 22:10 ` Junio C Hamano
2021-11-16 14:19 ` Fabian Stelzer
2021-11-17 8:19 ` Junio C Hamano [this message]
2021-11-17 9:05 ` Fabian Stelzer
2021-11-15 16:07 ` [PATCH 2/2] test-lib: introduce required prereq for test runs Fabian Stelzer
2021-11-16 6:09 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqpmqz42kj.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=adam@dinwoodie.org \
--cc=avarab@gmail.com \
--cc=fs@gigacodes.de \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).