Re: bug#9928: bug#10237: AM_SILENT_RULES does not work with NonStop make

[Stefano Lattarini]

[adding automake-patches -- which I should have done before]

On 12/22/2011 10:56 PM, Paul Eggert wrote:
> On 12/21/11 04:21, Stefano Lattarini wrote:
>> Hi Paul, thanks for the respin.  My comments and objections are inlined.
> 
> Thanks, I have a patch updated with all those comments in mind.
> One followup:
> 
>> AM_AM_DEFAULT_VERBOSITY ?
> 
> I changed that to AM_DEFAULT_V, as that seems to fit into the naming
> convention better.
>
Thanks.

A couple of typo reports and cosmetic nits that you might fix right
away are inlined.

Apart from that, I still have some further nits and suggestions for
improvements, but I also think the patch is good enough already, and
I don't want to force you to yet another respin.  So here's the deal:
could you apply the patch *not to maint nor to master*, but to a *new*
branch (say `silent-fixes' or `silent-portability') based off of maint,
and push that new branch to the public automake repo?  We can then
address the further nits in the next days by working on that branch,
and then merge it back into maint once we're done (that will obviously
happen before 1.11.3).

> From 939ba315f5e3e5caea0513af20724c35ead86956 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Thu, 22 Dec 2011 13:48:54 -0800
> Subject: [PATCH] silent-rules: fallback for makes without nested vars
> 
> This fixes two problems reported for Automake (Bug#9928, Bug#10237)
> and is in response to a bug report for building coreutils on HP
> NonStop OS (Bug#10234).  The problem is that HP NonStop 'make'
> treats a line like "AM_V_CC = $(am__v_CC_$(V))" as one that
> expands a macro with the funny name am__v_CC_$(V instead of the
> desired name am__v_CC_1 or am__v_CC_0, and since the funny macro
> is not defined the line is equivalent to "AM_V_CC = )"; this
> inserts a stray ")" when $(AM_V_CC) is used, which eventually
> causes 'make' to fail.
> 
> The basic idea is that instead of generating Makefile.in lines like
> "AM_V_CC = $(am__v_CC_$(V))", we generate
> "AM_V_CC = $(address@hidden@)".  We then AC_SUBST $(V) for @AM_V@
> in the usual case where `make' supports nested variables,
> and substitute 1 (or 0) otherwise.  Similarly for usages like
> $(am__v_CC_$(AM_DEFAULT_VERBOSITY)).
> 
> With this change, make implementations that doesn't grasp nested
> variable expansions will still be able to run Makefiles generated
> using the silent-rules option.  They won't allow the user to
> override the make verbosity at runtime through redefinition of
> $(V) (as in "make V=0"); but this is still an improvement over not
> being able to work at all.
> 
> * NEWS: Document this.
> * automake.in (define_verbose_var): When defining the variable,
>
s/variable/variables/

> use @AM_V@ rather than $(V), and use @AM_DEFAULT_V@ rather than
> $(AM_DEFAULT_VERBOSITY).
> * doc/automake.texi (Automake silent-rules Option): Explain new system.
> * m4/silent.m4 (AM_SILENT_RULES): Check whether `make' supports
> nested variables, and substitute AM_V and AM_DEFAULT_V accordingly.
> * tests/silent-nested-vars.test: New test.
> * tests/Makefile.am (TESTS): Add it.
> ---
>  ChangeLog                     |   39 ++++++++
>  NEWS                          |    7 ++
>  automake.in                   |    9 ++-
>  doc/automake.texi             |   22 +++--
>  m4/silent.m4                  |   34 +++++++-
>  tests/Makefile.am             |    1 +
>  tests/silent-nested-vars.test |  193 
> +++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 290 insertions(+), 15 deletions(-)
>  create mode 100755 tests/silent-nested-vars.test
> 
> diff --git a/ChangeLog b/ChangeLog
> index 26e4a2d..461dfac 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,42 @@
> +2011-12-22  Paul Eggert  <address@hidden>
> +
> +     silent-rules: fallback for makes without nested vars
> +
> +     This fixes two problems reported for Automake (Bug#9928, Bug#10237)
> +     and is in response to a bug report for building coreutils on HP
> +     NonStop OS (Bug#10234).  The problem is that HP NonStop 'make'
> +     treats a line like "AM_V_CC = $(am__v_CC_$(V))" as one that
> +     expands a macro with the funny name am__v_CC_$(V instead of the
> +     desired name am__v_CC_1 or am__v_CC_0, and since the funny macro
> +     is not defined the line is equivalent to "AM_V_CC = )"; this
> +     inserts a stray ")" when $(AM_V_CC) is used, which eventually
> +     causes 'make' to fail.
> +
> +     The basic idea is that instead of generating Makefile.in lines like
> +     "AM_V_CC = $(am__v_CC_$(V))", we generate
> +     "AM_V_CC = $(address@hidden@)".  We then AC_SUBST $(V) for @AM_V@
> +     in the usual case where `make' supports nested variables,
> +     and substitute 1 (or 0) otherwise.  Similarly for usages like
> +     $(am__v_CC_$(AM_DEFAULT_VERBOSITY)).
> +
> +     With this change, make implementations that doesn't grasp nested
> +     variable expansions will still be able to run Makefiles generated
> +     using the silent-rules option.  They won't allow the user to
> +     override the make verbosity at runtime through redefinition of
> +     $(V) (as in "make V=0"); but this is still an improvement over not
> +     being able to work at all.
> +
> +     * NEWS: Document this.
> +     * automake.in (define_verbose_var): When defining the variable,
>
s/variable/variables/

> +     use @AM_V@ rather than $(V), and use @AM_DEFAULT_V@ rather than
> +     $(AM_DEFAULT_VERBOSITY).
> +     * doc/automake.texi (Automake silent-rules Option): Explain new system.
> +     * m4/silent.m4 (AM_SILENT_RULES): Check whether `make' supports
> +     nested variables, and substitute AM_V and AM_DEFAULT_V accordingly.
> +     * tests/silent-nested-vars.test: New test.
> +     * tests/Makefile.am (TESTS): Add it.
> +
> +
>  2011-12-22  Stefano Lattarini  <address@hidden>
>  
>       hacking: distribute it, and mention it in the ChangeLog
> diff --git a/NEWS b/NEWS
> index db448a9..80ed91e 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -84,6 +84,13 @@ Bugs fixed in 1.11.0a:
>      not used, `make' output no longer contains spurious backslash-only
>      lines, thus once again matching what Automake did before 1.11.
>  
> +  - The `silent-rules' option now generates working makefiles even for
> +    the uncommon `make' implementations that do not support the
> +    nested-variables extension to POSIX 2008.  For such `make'
> +    implementations, whether a build is silent is determined at
> +    configure time, and cannot be overridden at make time with `make
> +    V=0' or `make V=1'.
> +
Better line wrapping for the last two lines?

  +    configure time, and cannot be overridden at make time with
  +    `make V=0' or `make V=1'.

No more nits from now on; quoting rest of the patch for reference.

>    - The AM_COND_IF macro also works if the shell expression for the 
> conditional
>      is no longer valid for the condition.
>  
> diff --git a/automake.in b/automake.in
> index 309eade..7efb7d5 100644
> --- a/automake.in
> +++ b/automake.in
> @@ -1161,9 +1161,12 @@ sub define_verbose_var ($$)
>      my $silent_var = $pvar . '_0';
>      if (option 'silent-rules')
>        {
> -     # Using `$V' instead of `$(V)' breaks IRIX make.
> -     define_variable ($var, '$(' . $pvar . '_$(V))', INTERNAL);
> -     define_variable ($pvar . '_', '$(' . $pvar . 
> '_$(AM_DEFAULT_VERBOSITY))', INTERNAL);
> +     # For typical `make's, `configure' replaces AM_V (inside @@) with $(V)
> +     # and AM_DEFAULT_V (inside @@) with $(AM_DEFAULT_VERBOSITY).
> +     # For strict POSIX 2008 `make's, it replaces them with 0 or 1 instead.
> +     # See AM_SILENT_RULES in m4/silent.m4.
> +     define_variable ($var, '$(' . $pvar . '_@'.'AM_V'.'@)', INTERNAL);
> +     define_variable ($pvar . '_', '$(' . $pvar . '_@'.'AM_DEFAULT_V'.'@)', 
> INTERNAL);
>       Automake::Variable::define ($silent_var, VAR_AUTOMAKE, '', TRUE, $val,
>                                   '', INTERNAL, VAR_ASIS)
>         if (! vardef ($silent_var, TRUE));
> diff --git a/doc/automake.texi b/doc/automake.texi
> index ced1b72..39884af 100644
> --- a/doc/automake.texi
> +++ b/doc/automake.texi
> @@ -10132,18 +10132,20 @@ For portability to different @command{make} 
> implementations, package authors
>  are advised to not set the variable @code{V} inside the @file{Makefile.am}
>  file, to allow the user to override the value for subdirectories as well.
>  
> -The current implementation of this feature relies on a non-POSIX, but in
> -practice rather widely supported @file{Makefile} construct of nested
> -variable expansion @samp{$(@var{var1}$(V))}.  Do not use the
> address@hidden option if your package needs to build with
> address@hidden implementations that do not support it.  The
> address@hidden option turns off warnings about recursive variable
> -expansion, which are in turn enabled by @option{-Wportability}
> -(@pxref{Invoking Automake}).
> +The current implementation of this feature normally uses nested
> +variable expansion @samp{$(@var{var1}$(V))}, a @file{Makefile} feature
> +that is not required by POSIX 2008 but is widely supported in
> +practice.  On the rare @command{make} implementations that do not
> +support nested variable expansion, whether rules are silent is always
> +determined at configure time, and cannot be overridden at make time.
> +Future versions of POSIX are likely to require nested variable
> +expansion, so this minor limitation should go away with time.
>  
>  @vindex @code{AM_V_GEN}
>  @vindex @code{AM_V_at}
>  @vindex @code{AM_DEFAULT_VERBOSITY}
> address@hidden @code{AM_V}
> address@hidden @code{AM_DEFAULT_V}
>  To extend the silent mode to your own rules, you have two choices:
>  
>  @itemize @bullet
> @@ -10159,8 +10161,8 @@ The following snippet shows how you would define your 
> own equivalent of
>  @code{AM_V_GEN}:
>  
>  @example
> -pkg_verbose = $(pkg_verbose_$(V))
> -pkg_verbose_ = $(pkg_verbose_$(AM_DEFAULT_VERBOSITY))
> +pkg_verbose = $(pkg_verbose_@@AM_V@@)
> +pkg_verbose_ = $(pkg_verbose_@@AM_DEFAULT_V@@)
>  pkg_verbose_0 = @@echo PKG-GEN $@@;
>  
>  foo: foo.in
> diff --git a/m4/silent.m4 b/m4/silent.m4
> index 6d2a1a2..8bd931b 100644
> --- a/m4/silent.m4
> +++ b/m4/silent.m4
> @@ -1,11 +1,11 @@
>  ##                                                          -*- Autoconf -*-
> -# Copyright (C) 2009  Free Software Foundation, Inc.
> +# Copyright (C) 2009, 2011  Free Software Foundation, Inc.
>  #
>  # This file is free software; the Free Software Foundation
>  # gives unlimited permission to copy and/or distribute it,
>  # with or without modifications, as long as this notice is preserved.
>  
> -# serial 1
> +# serial 2
>  
>  # AM_SILENT_RULES([DEFAULT])
>  # --------------------------
> @@ -20,6 +20,36 @@ yes) AM_DEFAULT_VERBOSITY=0;;
>  no)  AM_DEFAULT_VERBOSITY=1;;
>  *)   AM_DEFAULT_VERBOSITY=m4_if([$1], [yes], [0], [1]);;
>  esac
> +dnl
> +dnl A few `make' implementations (e.g., NonStop OS and NextStep)
> +dnl do not support nested variable expansions.
> +dnl See automake bug#9928 and bug#10237.
> +am_make=${MAKE-make}
> +AC_CACHE_CHECK([whether $am_make supports nested variables],
> +   [am_cv_make_support_nested_variables],
> +   [if AS_ECHO([['TRUE=$(BAR$(V))
> +BAR0=false
> +BAR1=true
> +V=1
> +am__doit:
> +     @$(TRUE)
> +.PHONY: am__doit']]) | $am_make -f - >/dev/null 2>&1; then
> +  am_cv_make_support_nested_variables=yes
> +else
> +  am_cv_make_support_nested_variables=no
> +fi])
> +if test $am_cv_make_support_nested_variables = yes; then
> +  dnl Using `$V' instead of `$(V)' breaks IRIX make.
> +  AM_V='$(V)'
> +  AM_DEFAULT_V='$(AM_DEFAULT_VERBOSITY)'
> +else
> +  AM_V=$AM_DEFAULT_VERBOSITY
> +  AM_DEFAULT_V=$AM_DEFAULT_VERBOSITY
> +fi
> +AC_SUBST([AM_V])dnl
> +AM_SUBST_NOTMAKE([AM_V])dnl
> +AC_SUBST([AM_DEFAULT_V])dnl
> +AM_SUBST_NOTMAKE([AM_DEFAULT_V])dnl
>  AC_SUBST([AM_DEFAULT_VERBOSITY])dnl
>  AM_BACKSLASH='\'
>  AC_SUBST([AM_BACKSLASH])dnl
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 5ac0e48..c1c0c83 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -759,6 +759,7 @@ silent-many-gcc.test \
>  silent-many-generic.test \
>  silent-lex-gcc.test \
>  silent-lex-generic.test \
> +silent-nested-vars.test \
>  silent-yacc-gcc.test \
>  silent-yacc-generic.test \
>  silent-configsite.test \
> diff --git a/tests/silent-nested-vars.test b/tests/silent-nested-vars.test
> new file mode 100755
> index 0000000..4b0fa3c
> --- /dev/null
> +++ b/tests/silent-nested-vars.test
> @@ -0,0 +1,193 @@
> +#!/bin/sh
> +# Copyright (C) 2011 Free Software Foundation, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2, or (at your option)
> +# any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Check silent-rules mode, on 'make' implementations that do not
> +# support nested variables (Bug#9928, Bug#10237).
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +mkdir sub
> +
> +cat >>configure.in <<'EOF'
> +AM_SILENT_RULES
> +AM_CONDITIONAL([HAVE_NESTED_VARIABLES],
> +  [expr "x$AM_V" : '.*\$' >/dev/null])
> +AC_CONFIG_FILES([sub/Makefile])
> +AC_PROG_CC
> +AM_PROG_CC_C_O
> +AC_OUTPUT
> +EOF
> +
> +cat > Makefile.am <<'EOF'
> +# Need generic and non-generic rules.
> +bin_PROGRAMS = foo bar
> +bar_CFLAGS = $(AM_CFLAGS)
> +SUBDIRS = sub
> +
> +# Check that AM_V and AM_DEFAULT_V work as advertised.
> +pkg_verbose = $(address@hidden@)
> +pkg_verbose_ = $(address@hidden@)
> +pkg_verbose_0 = @echo PKG-GEN $@;
> +
> +bin_SCRIPTS = oop
> +oop:
> +     $(pkg_verbose)echo $@ >$@
> +
> +mostlyclean-local:
> +     rm -f oop
> +
> +if HAVE_NESTED_VARIABLES
> +# Check that the older form (documented in Automake 1.11) works.
> +older_pkg_verbose = $(older_pkg_verbose_$(V))
> +older_pkg_verbose_ = $(older_pkg_verbose_$(AM_DEFAULT_VERBOSITY))
> +older_pkg_verbose_0 = @echo OLDER-PKG-GEN $@;
> +
> +bin_SCRIPTS += older-oop
> +older-oop:
> +     $(older_pkg_verbose)echo $@ >$@
> +endif
> +EOF
> +
> +cat > sub/Makefile.am <<'EOF'
> +AUTOMAKE_OPTIONS = subdir-objects
> +# Need generic and non-generic rules.
> +bin_PROGRAMS = baz bla
> +bla_CFLAGS = $(AM_CFLAGS)
> +EOF
> +
> +cat > foo.c <<'EOF'
> +int main ()
> +{
> +  return 0;
> +}
> +EOF
> +cp foo.c bar.c
> +cp foo.c sub/baz.c
> +cp foo.c sub/bla.c
> +
> +cat >mymake <<'EOF'
> +#! /bin/sh
> +makerules=
> +LC_ALL=C
> +export LC_ALL
> +
> +case $1 in
> +  -f)
> +    makefile=$2
> +    case $2 in
> +      -) makerules=`cat` || exit ;;
> +    esac ;;
> +  *)
> +    for makefile in makefile Makefile; do
> +      test -f $makefile && break
> +    done ;;
> +esac
> +
> +if
> +  nested_var_pat='^[^#]*\$([a-zA-Z0-9_]*\$'
> +  case $makefile in
> +    -) printf '%s\n' "$makerules" | grep "$nested_var_pat";;
> +    *) grep "$nested_var_pat" $makefile;;
> +  esac
> +then
> +  echo >&2 "mymake: $makefile contains nested variables"
> +  exit 1
> +fi
> +
> +case $makefile in
> +  -) printf '%s\n' "$makerules" | $mymake_MAKE "$@";;
> +  *) exec $mymake_MAKE "$@";;
> +esac
> +EOF
> +chmod a+x mymake
> +mymake_MAKE=${MAKE-make}
> +export mymake_MAKE
> +
> +# As a sanity check, verify that `mymake' rejects Makefiles that
> +# use nested variables.
> +cat > Makefile <<'END'
> +a = $(b$(c))
> +all:
> +     touch bar
> +END
> +./mymake && Exit 99
> +mv -f Makefile foo.mk
> +./mymake -f foo.mk && Exit 99
> +cat foo.mk | ./mymake -f - && Exit 99
> +test -f bar && Exit 99
> +sed '/a =/d' foo.mk > Makefile
> +./mymake && test -f bar || Exit 99
> +
> +$ACLOCAL
> +$AUTOMAKE --add-missing
> +$AUTOCONF
> +
> +for make in ${MAKE-make} ./mymake; do
> +  ./configure --enable-silent-rules MAKE=$make >enable.out 2>&1 ||
> +    { cat enable.out; Exit 1; }
> +  cat enable.out
> +  case $make in
> +    ./mymake)
> +      grep 'AM_V_CC = .*0' Makefile
> +      grep 'checking whether ./mymake supports nested variables... no' \
> +        enable.out
> +      ;;
> +  esac
> +
> +  $make >stdout || { cat stdout; Exit 1; }
> +  cat stdout
> +  $EGREP ' (-c|-o)' stdout && Exit 1
> +  grep 'mv ' stdout && Exit 1
> +  grep 'CC    .*foo\.' stdout
> +  grep 'CC .*bar\.' stdout
> +  grep 'CC .*baz\.' stdout
> +  grep 'CC .*bla\.' stdout
> +  grep 'CCLD .*foo' stdout
> +  grep 'CCLD .*bar' stdout
> +  grep 'CCLD .*baz' stdout
> +  grep 'CCLD .*bla' stdout
> +  grep 'PKG-GEN .*oop' stdout
> +  if test $am_cv_make_support_nested_variables = yes; then
> +    grep 'OLDER-PKG-GEN .*older-oop' stdout
> +  fi
> +  $make clean
> +
> +  ./configure --disable-silent-rules MAKE=$make >disable.out 2>&1 ||
> +    { cat disable.out; Exit 1; }
> +  cat disable.out
> +  case $make in
> +    ./mymake)
> +      grep 'AM_V_CC = .*1' Makefile
> +      grep 'checking whether ./mymake supports nested variables... no' \
> +        disable.out
> +      ;;
> +  esac
> +
> +  $make >stdout || { cat stdout; Exit 1; }
> +  cat stdout
> +  grep ' -c' stdout
> +  grep ' -o foo' stdout
> +  grep 'echo .*oop' stdout
> +  if test $am_cv_make_support_nested_variables = yes; then
> +    grep 'echo .*older-oop' stdout
> +  fi
> +  $EGREP '(CC|LD) ' stdout && Exit 1
> +  $make clean
> +done
> +
> +:

Thanks,
  Stefano