[libvirt PATCH 0/4] meson: Make syntax-check work on FreeBSD and macOS

Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/275868429 Note that https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/101 needs to be merged and the data in ci/cirrus/ regenerated accordingly before this can be pushed without introducing CI failures. Andrea Bolognani (4): meson: Print custom message when GNU grep is not installed meson: Reorganize looking for programs meson: Check GNU sed's availability meson: Look for GNU tools on macOS too build-aux/Makefile.in | 1 + build-aux/meson.build | 22 ++++++++++++++++------ build-aux/syntax-check.mk | 9 --------- 3 files changed, 17 insertions(+), 15 deletions(-) -- 2.26.3

Currently, if GNU grep is not installed on a FreeBSD system the configuration step will fail with Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO ERROR: Program '/usr/local/bin/grep' not found which is confusing and not very useful; after this change, the message will be Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO ERROR: Problem encountered: GNU grep not found instead, which should do a better job helping the user figure out that they need to install GNU grep from ports to proceed. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/meson.build | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/build-aux/meson.build b/build-aux/meson.build index c44ed6821c..06b7a7c01f 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -18,7 +18,10 @@ if host_machine.system() == 'freebsd' grep_cmd = run_command(grep_prog, '--version') if grep_cmd.stdout().startswith('grep (BSD grep') - grep_prog = find_program('/usr/local/bin/grep') + grep_prog = find_program('/usr/local/bin/grep', required: false) + if not grep_prog.found() + error('GNU grep not found') + endif grep_cmd = run_command(grep_prog, '--version') if grep_cmd.stdout().startswith('grep (BSD grep') error('GNU grep not found') -- 2.26.3

On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
Currently, if GNU grep is not installed on a FreeBSD system the configuration step will fail with
Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO
ERROR: Program '/usr/local/bin/grep' not found
Stupid question, but how does one need to invoke meson to actually hit ^this. On my local FreeBSD 12: $ ls -l /usr/local/bin/grep ls: /usr/local/bin/grep: No such file or directory $ pkg info | grep gnu gnupg-2.2.26 gnutls-3.6.15 $ pwd /home/test/libvirt $ git describe v7.1.0-328-g5f9330e724 $ meson --version 0.56.0 $ meson build -Dsystem=true | grep grep Program grep found: YES (/usr/bin/grep) Erik

On Fri, 2021-03-26 at 11:15 +0100, Erik Skultety wrote:
On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
Currently, if GNU grep is not installed on a FreeBSD system the configuration step will fail with
Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO
ERROR: Program '/usr/local/bin/grep' not found
Stupid question, but how does one need to invoke meson to actually hit ^this. On my local FreeBSD 12:
$ ls -l /usr/local/bin/grep ls: /usr/local/bin/grep: No such file or directory
$ pkg info | grep gnu gnupg-2.2.26 gnutls-3.6.15
$ pwd /home/test/libvirt
$ git describe v7.1.0-328-g5f9330e724
$ meson --version 0.56.0
$ meson build -Dsystem=true | grep grep Program grep found: YES (/usr/bin/grep)
This doesn't affect FreeBSD 12, but it can be hit without any particular setup on -CURRENT, plus of course FreeBSD 13 once that's released. See commit 7dd7ddac500cd3f453cd19606904ae406f5ffe03 Author: Roman Bogorodskiy <bogorodskiy@gmail.com> Date: Tue Mar 2 18:31:36 2021 +0400 build-aux: require GNU grep on FreeBSD FreeBSD 13.x and newer ship BSD grep which apparently has some performance issues causing certain syntax check tests to run longer than the default 30 seconds timeout used by meson. However, GNU grep is still available through the textproc/gnugrep port, so require it on FreeBSD if /usr/bin/grep is a BSD grep to make checks pass in a reasonable time. Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> for more information. -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
Currently, if GNU grep is not installed on a FreeBSD system the configuration step will fail with
Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO
ERROR: Program '/usr/local/bin/grep' not found
which is confusing and not very useful; after this change, the message will be
Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO
ERROR: Problem encountered: GNU grep not found
instead, which should do a better job helping the user figure out that they need to install GNU grep from ports to proceed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>

On Thu, Apr 01, 2021 at 11:10:05AM +0200, Erik Skultety wrote:
On Wed, Mar 24, 2021 at 07:37:57PM +0100, Andrea Bolognani wrote:
Currently, if GNU grep is not installed on a FreeBSD system the configuration step will fail with
Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO
ERROR: Program '/usr/local/bin/grep' not found
which is confusing and not very useful; after this change, the message will be
Program grep found: YES (/usr/bin/grep) Program /usr/local/bin/grep found: NO
ERROR: Problem encountered: GNU grep not found
instead, which should do a better job helping the user figure out that they need to install GNU grep from ports to proceed.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
Please squash this in before merging: diff --git a/build-aux/meson.build b/build-aux/meson.build index 1095982397..e491bdeebc 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd' if not grep_prog.found() error('GNU grep not found') endif - grep_cmd = run_command(grep_prog, '--version') - if grep_cmd.stdout().startswith('grep (BSD grep') - error('GNU grep not found') - endif endif elif host_machine.system() == 'darwin' grep_prog = find_program('ggrep')

On Thu, 2021-04-01 at 11:23 +0200, Erik Skultety wrote:
Please squash this in before merging:
diff --git a/build-aux/meson.build b/build-aux/meson.build index 1095982397..e491bdeebc 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd' if not grep_prog.found() error('GNU grep not found') endif - grep_cmd = run_command(grep_prog, '--version') - if grep_cmd.stdout().startswith('grep (BSD grep') - error('GNU grep not found') - endif
Mh, so what you're saying is that we can assume that /usr/local/bin/grep is *always* going to be GNU grep? That's probably a fair assumption. I disagree that the hunk should be squashed in, though, since this patch simply changes the error message and not what's been checked. It should go in separately, either after this series or before it. Do you want me to post it in that form, or would you rather do that yourself since you have already done the work? Whatever is more convenient for you :) -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Apr 01, 2021 at 11:51:15AM +0200, Andrea Bolognani wrote:
On Thu, 2021-04-01 at 11:23 +0200, Erik Skultety wrote:
Please squash this in before merging:
diff --git a/build-aux/meson.build b/build-aux/meson.build index 1095982397..e491bdeebc 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd' if not grep_prog.found() error('GNU grep not found') endif - grep_cmd = run_command(grep_prog, '--version') - if grep_cmd.stdout().startswith('grep (BSD grep') - error('GNU grep not found') - endif
Mh, so what you're saying is that we can assume that /usr/local/bin/grep is *always* going to be GNU grep? That's probably a fair assumption.
I'd be surprised if a symlink existed in /usr/local/bin/grep pointing to the distro's BSD grep, doesn't sound like FreeBSD hierarchy standards :).
I disagree that the hunk should be squashed in, though, since this patch simply changes the error message and not what's been checked. It should go in separately, either after this series or before it.
Do you want me to post it in that form, or would you rather do that yourself since you have already done the work? Whatever is more convenient for you :)
I honestly don't care, so if you squash it or append another patch to your series before merging - up to you. Erik

On Thu, 2021-04-01 at 12:03 +0200, Erik Skultety wrote:
On Thu, Apr 01, 2021 at 11:51:15AM +0200, Andrea Bolognani wrote:
I disagree that the hunk should be squashed in, though, since this patch simply changes the error message and not what's been checked. It should go in separately, either after this series or before it.
Do you want me to post it in that form, or would you rather do that yourself since you have already done the work? Whatever is more convenient for you :)
I honestly don't care, so if you squash it or append another patch to your series before merging - up to you.
Okay, posted as a separate patch here: https://listman.redhat.com/archives/libvir-list/2021-April/msg00024.html -- Andrea Bolognani / Red Hat / Virtualization

While this change doesn't look like it would improve things and actually introduces a tiny bit of duplication, it's necessary in order to prepares the stage for further changes. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/meson.build | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/build-aux/meson.build b/build-aux/meson.build index 06b7a7c01f..c56a348946 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -10,12 +10,14 @@ syntax_check_conf.set('flake8_path', flake8_path) syntax_check_conf.set('runutf8', ' '.join(runutf8)) syntax_check_conf.set('PYTHON3', python3_prog.path()) - -grep_prog = find_program('grep') - if host_machine.system() == 'freebsd' make_prog = find_program('gmake') +else + make_prog = find_program('make') +endif +if host_machine.system() == 'freebsd' + grep_prog = find_program('grep') grep_cmd = run_command(grep_prog, '--version') if grep_cmd.stdout().startswith('grep (BSD grep') grep_prog = find_program('/usr/local/bin/grep', required: false) @@ -28,7 +30,7 @@ if host_machine.system() == 'freebsd' endif endif else - make_prog = find_program('make') + grep_prog = find_program('grep') endif syntax_check_conf.set('GREP', grep_prog.path()) -- 2.26.3

As explained in the comment in build-aux/Makefile.in, the version of sed included in the FreeBSD base system is not GNU sed, which our syntax-check rules expect; as a result, many checks will fail with gmake: gsed: No such file or directory /bin/sh: gsed: not found Similarly to what we're already doing with GNU make and GNU grep, look for GNU sed during the configuration step and fail early if it's not available. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/Makefile.in | 1 + build-aux/meson.build | 3 +++ build-aux/syntax-check.mk | 9 --------- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/build-aux/Makefile.in b/build-aux/Makefile.in index 415a8df305..9ccbec7b1b 100644 --- a/build-aux/Makefile.in +++ b/build-aux/Makefile.in @@ -6,6 +6,7 @@ FLAKE8 = @flake8_path@ RUNUTF8 = @runutf8@ PYTHON = @PYTHON3@ GREP = @GREP@ +SED = @SED@ # include syntax-check.mk file include $(top_srcdir)/build-aux/syntax-check.mk diff --git a/build-aux/meson.build b/build-aux/meson.build index c56a348946..fe88d6b736 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -12,8 +12,10 @@ syntax_check_conf.set('PYTHON3', python3_prog.path()) if host_machine.system() == 'freebsd' make_prog = find_program('gmake') + sed_prog = find_program('gsed') else make_prog = find_program('make') + sed_prog = find_program('sed') endif if host_machine.system() == 'freebsd' @@ -34,6 +36,7 @@ else endif syntax_check_conf.set('GREP', grep_prog.path()) +syntax_check_conf.set('SED', sed_prog.path()) configure_file( input: 'Makefile.in', diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 51a498a897..7f4a23c048 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -27,15 +27,6 @@ ME := build-aux/syntax-check.mk # of the module description. But some packages import this file directly, # ignoring the module description. AWK ?= awk -# FreeBSD (and probably some other OSes too) ships own version of sed(1), not -# compatible with the GNU sed. GNU sed is available as gsed(1), so use this -# instead -UNAME := $(shell uname) -ifeq ($(UNAME),FreeBSD) -SED ?= gsed -else -SED ?= sed -endif # Helper variables. _empty = -- 2.26.3

macOS is similar to FreeBSD in that it ships non-GNU versions of several base utilities that we need in the base system. macOS actually includes GNU make already, but unfortunately due to licensing reasons the tool is permanently stuck in 2006, so even in that case users are better off installing a recent version from Homebrew along with the dozens of other libvirt dependencies that already need to be obtained that way. Note that, unlike FreeBSD ports, Homebrew is fully consistent in adding the 'g' prefix to the name of the GNU tools, so we can detect GNU grep without additional hacks. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/meson.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build-aux/meson.build b/build-aux/meson.build index fe88d6b736..1095982397 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -10,7 +10,7 @@ syntax_check_conf.set('flake8_path', flake8_path) syntax_check_conf.set('runutf8', ' '.join(runutf8)) syntax_check_conf.set('PYTHON3', python3_prog.path()) -if host_machine.system() == 'freebsd' +if host_machine.system() == 'freebsd' or host_machine.system() == 'darwin' make_prog = find_program('gmake') sed_prog = find_program('gsed') else @@ -31,6 +31,8 @@ if host_machine.system() == 'freebsd' error('GNU grep not found') endif endif +elif host_machine.system() == 'darwin' + grep_prog = find_program('ggrep') else grep_prog = find_program('grep') endif -- 2.26.3

On Wed, Mar 24, 2021 at 07:37:56PM +0100, Andrea Bolognani wrote:
Test pipeline:
https://gitlab.com/abologna/libvirt/-/pipelines/275868429
Note that
https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/101
needs to be merged and the data in ci/cirrus/ regenerated accordingly before this can be pushed without introducing CI failures.
Reviewed-by: Erik Skultety <eskultet@redhat.com>

Since /usr/local is where ports live, it's reasonable to assume that a grep binary found in there will have been installed via ports and will thus be GNU grep. Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/meson.build | 4 ---- 1 file changed, 4 deletions(-) diff --git a/build-aux/meson.build b/build-aux/meson.build index 1095982397..e491bdeebc 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -26,10 +26,6 @@ if host_machine.system() == 'freebsd' if not grep_prog.found() error('GNU grep not found') endif - grep_cmd = run_command(grep_prog, '--version') - if grep_cmd.stdout().startswith('grep (BSD grep') - error('GNU grep not found') - endif endif elif host_machine.system() == 'darwin' grep_prog = find_program('ggrep') -- 2.26.3

On Thu, Apr 01, 2021 at 02:17:14PM +0200, Andrea Bolognani wrote:
Since /usr/local is where ports live, it's reasonable to assume that a grep binary found in there will have been installed via ports and will thus be GNU grep.
Suggested-by: Erik Skultety <eskultet@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> ---
Feels weird ACKing this, but FWIW: Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Andrea Bolognani
-
Erik Skultety