[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] autogen.sh: tell user the correct make command



On Tue, 2017-07-04 at 16:22 +0100, Daniel P. Berrange wrote:
> When autogen.sh finishes it helpfully prints
> 
>   "Now type 'make' to compile libvirt."
> 
> which is fine if on a host with GNU make, but on *BSD running
> 'make' will end in tears. We should tell users to run 'gmake'
> on these platforms. If 'gmake' doesn't exist then we should
> report an error too
> 
>   "GNU make is required to build libvirt"
> 
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  autogen.sh | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/autogen.sh b/autogen.sh
> index d5d836a..1e99ce8 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -193,4 +193,21 @@ else
>  fi
>  
>  echo

I'd move this 'echo' to the bottom as well.

> -echo "Now type 'make' to compile libvirt."
> +
> +# Make sure we can find GNU make and tell the user
> +# the right command to run
> +make -v | grep "GNU Make" 1>/dev/null 2>&1

This doesn't catch stderr for the make invocation, and
FreeBSD's make doesn't support the -v flag so you'll
end up with a bunch of spurious output. You can use

  make -v 2>&1 | grep -q "GNU Make"

instead.

> +if test $? = 0

You're not using $? for anything else, so you can just
have the command above as condition for 'if'.

> +then

The rest of the file puts 'then' on the same line as
'if', please keep it consistent.

> +    MAKE=make
> +else
> +    which gmake 1>/dev/null 2>&1
> +    if test $? = 0
> +    then

Same comments as above. Additionally, you don't need
to mention fd 1 explicitly when redirecting stdout,
just '>/dev/null' is enough and looks less weird.

-- 
Andrea Bolognani / Red Hat / Virtualization


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]