Daniel P. Berrangé wrote:
On Sun, Feb 28, 2021 at 05:00:01PM +0400, Roman Bogorodskiy wrote:
> Peter Krempa wrote:
>
> > On Sun, Feb 28, 2021 at 08:24:58 +0400, Roman Bogorodskiy wrote:
> > > Meson default timeout for test() is 30 seconds. This may be not enough
> > > for some tests like sc_prohibit_nonreentrant or
> > > sc_libvirt_unmarked_diagnostics, so set it to 60 seconds.
> >
> > Recently [1] we've established that we'll not be raising the timeout
> > arbitrarily to compensate for a possibly slow hardware unless it's a
> > widespread problem.
> >
> > The tests you are complaining about are pretty fast on my system:
> >
> > 203/334 libvirt:syntax-check / sc_prohibit_nonreentrant
OK 0.23s
> > 315/334 libvirt:syntax-check / sc_unmarked_diagnostics
OK 0.63s
> >
> > On a laptop:
> > 204/335 libvirt:syntax-check / sc_prohibit_nonreentrant
OK 0.44s
> > 316/335 libvirt:syntax-check / sc_unmarked_diagnostics
OK 0.78s
> >
> > And on a random sample from our (linux) CI runs:
> > 53/158 libvirt:syntax-check / sc_libvirt_unmarked_diagnostics OK
0.6185753345489502 s
> > 27/158 libvirt:syntax-check / sc_prohibit_nonreentrant OK
0.2680661678314209 s
> >
> > Given the almost 2 orders of magnitude difference, I think something is
> > broken on your system and should be investigated first before attempting
> > to increase the timeout.
>
> I *think* the reason it's slow on my system is because BSD grep is
> slower than GNU grep.
>
> I don't have a solid evidence of that though, except that 10 years old
> post [1] and a basic test:
>
> $ time gmake -C /usr/home/novel/code/libvirt/build/build-aux
sc_prohibit_nonreentrant
> gmake: Entering directory '/usr/home/novel/code/libvirt/build/build-aux'
> prohibit_nonreentrant
> gmake: Leaving directory '/usr/home/novel/code/libvirt/build/build-aux'
> gmake -C /usr/home/novel/code/libvirt/build/build-aux sc_prohibit_nonreentran
48,21s user 0,06s system 100% cpu 48,199 total
> $ time PATH="/usr/local/bin:$PATH" gmake -C
/usr/home/novel/code/libvirt/build/build-aux sc_prohibit_nonreentrant
> gmake: Entering directory '/usr/home/novel/code/libvirt/build/build-aux'
> prohibit_nonreentrant
> gmake: Leaving directory '/usr/home/novel/code/libvirt/build/build-aux'
> PATH="/usr/local/bin:$PATH" gmake -C sc_prohibit_nonreentrant 0,23s user
0,02s system 119% cpu 0,215 total
> $
>
> Here, the PATH override is used because on FreeBSD the original BSD grep
> is installed in /usr/bin/grep, and GNU grep is installed in
> /usr/local/bin/grep from the gnugrep package (or textproc/gnugrep port).
We already special case sed for BSD in syntax-check.mk, so how about we
make it also try the GNU grep explicitly too.
Yes, I tried that and it works.
What I don't quite like about this approach is that unlike sed, where
GNU sed is installed as 'gsed' which is easy to detect, GNU grep is
installed as 'grep', but to a different prefix.
We can hardcode GREP ?= /usr/local/bin/grep, but there are some cases
when it won't work:
- If a user installs software to a different prefix (not /usr/local),
- If a user has some other grep installed in /usr/local instead of GNU
grep. For example, there's a textproc/bsdgrep port which installs
grep to the same place. Moreover, it looks like FreeBSD 12.x and
older use GNU grep by default, so with this hardcode we'll be using a
wrong grep on older FreeBSD versions.
A general solution would be to make sure we're dealing with GNU grep by
checking `grep --version` and looking for some specific bits to GNU
grep, maybe "Free Software Foundation" or something like that, as BSD
grep mentions it's GNU grep compatible, so cannot search for just "GNU
grep". This doesn't seem to be very convenient with meson...
More simple, however less general solution would be use
/usr/local/bin/grep only for FreeBSD >= 13.0.
PS In addition to that, for quite some time it was possible to build
FreeBSD with GNU grep even after BSD grep became a default. This switch
was removed only in Dec 2020 and BSD grep became the only option. I
guess it's safe to ignore this case, but that's overall is just feels
too messy.
Roman Bogorodskiy