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

Daniel P. Berrange posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170704152203.16958-1-berrange@redhat.com
There is a newer version of this series
autogen.sh | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
[libvirt] [PATCH] autogen.sh: tell user the correct make command
Posted by Daniel P. Berrange 6 years, 8 months ago
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
-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
+if test $? = 0
+then
+    MAKE=make
+else
+    which gmake 1>/dev/null 2>&1
+    if test $? = 0
+    then
+        MAKE=gmake
+    else
+        die "GNU make is required to build libvirt"
+    fi
+fi
+
+echo "Now type '$MAKE' to compile libvirt."
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] autogen.sh: tell user the correct make command
Posted by Andrea Bolognani 6 years, 8 months ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] autogen.sh: tell user the correct make command
Posted by Eric Blake 6 years, 8 months ago
On 07/05/2017 11:32 AM, Andrea Bolognani wrote:
> 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>
>> ---

>> +
>> +# 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

In fact, gnulib's bootstrap script is already able to enforce
prerequisites, look for buildreq= in bootstrap.conf.  But I don't know
if it is smart enough to make it easy to state that a project depends on
GNU make (not all projects have that dependency, but we do) - so maybe
it's worth hoisting the detection of an adequate (GNU) make into
gnulib's bootstrap script.

> 
> 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"

Yes, that is indeed important, if you like scraping help output.  But it
may be better to test some other GNU-only functionality instead of just
scraping output (features, not versions, is always better when it comes
to probing for sufficient support) - the question is whether that can be
done in an equally compact manner.

Is it worth cc'ing the gnulib list for thoughts on incorporating the
notion of "this project requires GNU make" into the $buildreq handling
of bootstrap?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] autogen.sh: tell user the correct make command
Posted by Andrea Bolognani 6 years, 8 months ago
On Wed, 2017-07-05 at 13:06 -0500, Eric Blake wrote:
> > 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"
> 
> Yes, that is indeed important, if you like scraping help output.  But it
> may be better to test some other GNU-only functionality instead of just
> scraping output (features, not versions, is always better when it comes
> to probing for sufficient support) - the question is whether that can be
> done in an equally compact manner.

I think we're using a bunch of construct that can only be
found in GNU make, so while having a more fine-grained
capability check would future-proof us in case (Free)BSD
make decides to implement the same features down the line
it's quite possibly more trouble than it's worth.

> Is it worth cc'ing the gnulib list for thoughts on incorporating the
> notion of "this project requires GNU make" into the $buildreq handling
> of bootstrap?

That sounds good. Not sure whether we should wait until
that's implemented or merge this (fixed) patch right away
and rewrite it in terms of gnulib's make detection once
that's available, though.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] autogen.sh: tell user the correct make command
Posted by Martin Kletzander 6 years, 8 months ago
On Wed, Jul 05, 2017 at 06:32:07PM +0200, Andrea Bolognani wrote:
>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

also I believe `which` is not POSIX-guaranteed, you should use `type` instead.

>> +    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
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list