[16/16] configure: do_compiler: Dump some extra info under bash

This makes it much easier to find a particular thing in config.log.

The information may be lacking in other shells, resulting in harmless
empty output.  (This is why we don't use the proper ${FUNCNAME[*]}
array syntax - other shells will choke on that.)

The extra output is only printed if configure is run with bash.  On
systems where /bin/sh is not bash, it is necessary to say bash
./configure to get the extra debug info in the log.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Kent R. Spillner <kspillner@acm.org>
CC: Janosch Frank <frankja@linux.vnet.ibm.com>
CC: Thomas Huth <thuth@redhat.com>
CC: Peter Maydell <peter.maydell@linaro.org>
CC: Paolo Bonzini <pbonzini@redhat.com>
---
v6: Fix commit message wording.
v4: No longer tag this patch RFC.
---
 configure | 4 ++++
 1 file changed, 4 insertions(+)

On Thu, Apr 19, 2018 at 05:45:19PM +0100, Ian Jackson wrote:
> This makes it much easier to find a particular thing in config.log.
> 
> The information may be lacking in other shells, resulting in harmless
> empty output.  (This is why we don't use the proper ${FUNCNAME[*]}
> array syntax - other shells will choke on that.)
> 
> The extra output is only printed if configure is run with bash.  On
> systems where /bin/sh is not bash, it is necessary to say bash
> ./configure to get the extra debug info in the log.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  configure | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/configure b/configure
> index d5435ff..a4c5292 100755
> --- a/configure
> +++ b/configure
> @@ -60,6 +60,10 @@ do_compiler() {
>      # is compiler binary to execute.
>      local compiler="$1"
>      shift
> +    echo >>config.log "
> +funcs: ${FUNCNAME}
> +lines: ${BASH_LINENO}
> +files: ${BASH_SOURCE}"
>      echo $compiler "$@" >> config.log
>      $compiler "$@" >> config.log 2>&1 || return $?
>      # Test passed. If this is an --enable-werror build, rerun

How is this usefull? All I have in my config.log is a lot of:
  funcs: do_compiler
  lines: 91
  files: ./configure

And one:
  funcs: do_compiler
  lines: 95
  files: ./configure

It still don't tell me which test had runned.

Regards,

On Mon, Apr 23, 2018 at 05:21:42PM +0100, Anthony PERARD wrote:
> On Thu, Apr 19, 2018 at 05:45:19PM +0100, Ian Jackson wrote:
> > This makes it much easier to find a particular thing in config.log.
> > 
> > The information may be lacking in other shells, resulting in harmless
> > empty output.  (This is why we don't use the proper ${FUNCNAME[*]}
> > array syntax - other shells will choke on that.)
> > 
> > The extra output is only printed if configure is run with bash.  On
> > systems where /bin/sh is not bash, it is necessary to say bash
> > ./configure to get the extra debug info in the log.
> > 
> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > ---
> >  configure | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/configure b/configure
> > index d5435ff..a4c5292 100755
> > --- a/configure
> > +++ b/configure
> > @@ -60,6 +60,10 @@ do_compiler() {
> >      # is compiler binary to execute.
> >      local compiler="$1"
> >      shift
> > +    echo >>config.log "
> > +funcs: ${FUNCNAME}
> > +lines: ${BASH_LINENO}
> > +files: ${BASH_SOURCE}"
> >      echo $compiler "$@" >> config.log
> >      $compiler "$@" >> config.log 2>&1 || return $?
> >      # Test passed. If this is an --enable-werror build, rerun
> 
> How is this usefull? All I have in my config.log is a lot of:
>   funcs: do_compiler
>   lines: 91
>   files: ./configure
> 
> And one:
>   funcs: do_compiler
>   lines: 95
>   files: ./configure
> 
> It still don't tell me which test had runned.

In autoconf, you would generally have a line output to stdout for every
test being run, as it is done so you can see immediately which test it
stopped on. QEMU's configure by comparison is completely silent, except
for the fnal summary at the end which is fine if everything works perfectly,
but not great when it doesn't.

Personally I'd suggest we add informative messages throughout the
configure script for each check being run. If people really hate the
idea of a verbose output from configure, we could leave it silent by
default and add a '--verbose' option to turn it on.

Regards,
Daniel

Anthony PERARD writes ("Re: [PATCH 16/16] configure: do_compiler: Dump some extra info under bash"):
> On Thu, Apr 19, 2018 at 05:45:19PM +0100, Ian Jackson wrote:
> > +funcs: ${FUNCNAME}
> > +lines: ${BASH_LINENO}
> > +files: ${BASH_SOURCE}"
> >      echo $compiler "$@" >> config.log
> >      $compiler "$@" >> config.log 2>&1 || return $?
> >      # Test passed. If this is an --enable-werror build, rerun
> 
> How is this usefull? All I have in my config.log is a lot of:
>   funcs: do_compiler
>   lines: 91
>   files: ./configure
> 
> And one:
>   funcs: do_compiler
>   lines: 95
>   files: ./configure
> 
> It still don't tell me which test had runned.

You are right.  Perhaps my testing was inadequate.  I wrote this a
long while ago, and if there was a syntax along these lines that DTRT
in both bash and dash in my tests it is long gone.  Starting de novo,
the following code works for me:

    (echo >>config.log "
 funcs: ${FUNCNAME[*]}
 lines: ${BASH_LINENO[*]}
 files: ${BASH_SOURCE[*]}") 2>/dev/null ||:

With bash I get the expected information in config.log, which looks
like this:

 funcs: do_compiler do_cc compile_prog cc_has_warning_flag main
 lines: 91 124 1720 1724 0
 files: ./configure ./configure ./configure ./configure ./configure

With dash the script runs but there is nothing from this segment in
the log.  Without the 2>/dev/null, it prints
  ./configure: 63: ./configure: Bad substitution
so the syntax error is indeed being suprresed and ignored.

The ( ) is necessary because syntax errors are not like set -e errors:
they cause the shell process to exit.

I will update the patch and put some of this info in the commit
message.

Ian.

Daniel P. Berrangé writes ("Re: [Qemu-devel] [PATCH 16/16] configure: do_compiler: Dump some extra info under bash"):
> Personally I'd suggest we add informative messages throughout the
> configure script for each check being run. If people really hate the
> idea of a verbose output from configure, we could leave it silent by
> default and add a '--verbose' option to turn it on.

Doing that would be a lot of work.  I am not opposed to it, but I am
not going to do it.

My suggestion is a very small change which instantly makes configure
much easier to debug, at least if you have bash available (and the bug
you are fixing doesn't go away with bash).

Please don't let the best be the enemy of an improvement.  If
configure is ever improved (without being simply replaced), the
logging code can easily be removed.

Thanks,
Ian.

On 04/23/2018 11:38 AM, Ian Jackson wrote:

> You are right.  Perhaps my testing was inadequate.  I wrote this a
> long while ago, and if there was a syntax along these lines that DTRT
> in both bash and dash in my tests it is long gone.  Starting de novo,
> the following code works for me:
> 
>     (echo >>config.log "
>  funcs: ${FUNCNAME[*]}
>  lines: ${BASH_LINENO[*]}
>  files: ${BASH_SOURCE[*]}") 2>/dev/null ||:

That's still fork-heavy.  You could do:

test -n "$BASH_VERSION" && eval '
echo >>config.log "
funcs: ${FUNCNAME[*]}
lines: ${BASH_LINENO[*]}
files: ${BASH_SOURCE[*]}"'

which avoids the fork, but remains silent on dash.

> 
> With bash I get the expected information in config.log, which looks
> like this:
> 
>  funcs: do_compiler do_cc compile_prog cc_has_warning_flag main
>  lines: 91 124 1720 1724 0
>  files: ./configure ./configure ./configure ./configure ./configure

Is files: really useful information?  The other two are (as it gives a
full stack trace), but if we aren't actively sourcing lots of other
files, seeing a bunch of ./configure doesn't add much.

> 
> With dash the script runs but there is nothing from this segment in
> the log.  Without the 2>/dev/null, it prints
>   ./configure: 63: ./configure: Bad substitution
> so the syntax error is indeed being suprresed and ignored.
> 
> The ( ) is necessary because syntax errors are not like set -e errors:
> they cause the shell process to exit.

See above - a well-quoted eval is sufficient to avoid a subshell.

Eric Blake writes ("Re: [Qemu-devel] [PATCH 16/16] configure: do_compiler: Dump some extra info under bash"):
> That's still fork-heavy.  You could do:
> 
> test -n "$BASH_VERSION" && eval '
> echo >>config.log "
> funcs: ${FUNCNAME[*]}
> lines: ${BASH_LINENO[*]}
> files: ${BASH_SOURCE[*]}"'
> 
> which avoids the fork, but remains silent on dash.

Thanks.  I will adopt this.  Although I will use if ... then as that
seems to be the usual style.  (&& would break with set -e which
configure does not use...)

Ian.