Re: [PATCH 1/3] t4216: avoid unnecessary subshell in test_bloom_filters_not_used
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 20, 2020, 15:04 UTC
- Message-ID
- <xmqqr1vewsug.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200520034444.47932-2-carenas@gmail.com>
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
Show 6 quoted lines
> Seems to trigger a bug in at least OpenBSD's 6.7 sh where it is > interpreted as a history lookup and therefore fails 125-126, 128, > 130. > > Remove the subshell and get a space between ! and grep, so tests > pass successfully.
It's strange that somebody thinks of doing history lookup in non-interactive use.
But even more curious is why we have this in a subshell in the first place. I do not see a reason why we need subshell, nor use of the double-quote in the outer layer that forces us to use backslashes.
Show 5 quoted lines
> test_bloom_filters_not_used () {
> log_args=$1
> setup "$log_args" &&
> - !(grep -q "statistics:{\"filter_not_present\":" "$TRASH_DIRECTORY/trace.perf") &&
> + ! grep -q "statistics:{\"filter_not_present\":" "$TRASH_DIRECTORY/trace.perf" &&This is obviously the minimum fix, so I'm willing to take the change as-is, but if we were writing it today, perhaps
! grep 'statistics:{"filter_not_present":' "$TRASH_DIRECTORY/trace.perf" &&is how we write it. I do not see any reason why we want to use the "-q" option either.