Subject: bug#13378: More work on subdir-objects, AC_PROG_CC and AM_PROG_CC_C_O

From: Stefano Lattarini <[email-masked]>
Date: Sat, 11 May 2013 11:57:30 +0200

Reference:
<http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13378#130>

Hello everybody, sorry for the delay.  This follow-up is looong
overdue.

On 01/14/2013 11:23 AM, Stefano Lattarini wrote:
> On 01/13/2013 10:06 PM, Nick Bowler wrote:
>> On 2013-01-13, Stefano Lattarini <[email-masked]> wrote:
>>
>>> Another useful follow-up would be to move the AM_PROG_CC_C_O in a private
>>> macro (to be expanded in AC_CONFIG_COMMANDS_PRE like you did above), and
>>> make AM_PROG_CC_C_O a no-op (without runtime deprecation).  That way, we
>>> could rely on the improved semantic of having the potential '$CC' rewrite
>>> placed near the end of configure, rather than near the beginning.  WDYT?
>>
>> With AC_CONFIG_COMMANDS_PRE, AM_PROG_CC_C_O would still be required if
>> package authors want to make use of the "compile" wrapper in configure
>> tests -- essentially, configure.ac can call AM_PROG_CC_C_O to force the
>> test to happen where it is needed, rather than just before AC_OUTPUT.
>>
>> I think it'd be worthwhile to keep that working.
>>
> Ah, but the $CC rewrite has always been an undocumented hack, and relying
> on it is a bad idea.  Still, your reasoning makes it clear that changing
> that semantics abruptly might cause subtle backward-incompatibilities in
> corner-case but perfectly valid situations.  Hmmm...  I think it's better
> to follow your approach for now, and then, if and when we decide to fix
> our macros not to rewrite $CC, do so with proper NEWS and documentation
> warnings beforehand, and a viable deprecation plan.
> 
The attached patches implement Nick's suggestion on the current code
base (original patches from Nick has unfortunately gotten too much
out-of-sync with the current codebase situation).  But note that the
second (one-liner) is actually identical to the first patch originally
posted by Nick <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=13378#121>,
so it's still in his name.

I plan to push these patches to maint in a couple of days if there is
no objection.

Regards,
  Stefano
>From 32eb770b73903a6b09216709790a093b33afff8d Mon Sep 17 00:00:00 2001
Message-Id: <32eb770b7[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Sat, 11 May 2013 11:03:41 +0200
Subject: [PATCH 1/2] compile: avoid AC_PROG_CC messy rewrite

Instead, add an hook to AC_OUTPUT to have AM_PROG_CC_C_O invoked
automatically.

See also the long-winded discussion about automake bug#13378.

* m4/minuso.m4 (AM_PROG_CC_C_O): Bring back the old implementation,
from commit v1.13.1-55-g1ab8fb6.
* m4/init.m4 (AC_PROG_CC): Remove this horrible, hacky re-write.
* (AM_INIT_AUTOMAKE): Arrange for AM_PROG_CC_C_O to be called if
necessary.
* t/am-prog-cc-c-o.sh: Adjust to avoid spurious failure.
* t/subobj.sh: Likewise.

Suggested-by: Nick Bowler <[email-masked]>
Signed-off-by: Stefano Lattarini <[email-masked]>
---
 m4/init.m4          | 52 ++++++----------------------------------------------
 m4/minuso.m4        | 27 +++++++++++++++++----------
 t/add-missing.tap   |  9 ++++-----
 t/am-prog-cc-c-o.sh |  4 ++--
 t/subobj.sh         |  3 ++-
 5 files changed, 31 insertions(+), 64 deletions(-)

diff --git a/m4/init.m4 b/m4/init.m4
index ce64a6c..a6f2733 100644
--- a/m4/init.m4
+++ b/m4/init.m4
@@ -110,6 +110,12 @@ AC_PROVIDE_IFELSE([AC_PROG_OBJCXX],
 		  [m4_define([AC_PROG_OBJCXX],
 			     m4_defn([AC_PROG_OBJCXX])[_AM_DEPENDENCIES([OBJCXX])])])dnl
 ])
+dnl Automatically invoke AM_PROG_CC_C_O as necessary.  Since AC_PROG_CC is
+dnl usually called after AM_INIT_AUTOMAKE, we arrange for the test to be
+dnl done later by AC_CONFIG_COMMANDS_PRE.
+AC_CONFIG_COMMANDS_PRE([AC_PROVIDE_IFELSE(
+    [AC_PROG_CC],
+    [AC_LANG_PUSH([C]) AM_PROG_CC_C_O AC_LANG_POP([C])])])dnl
 AC_REQUIRE([AM_SILENT_RULES])dnl
 dnl The testsuite driver may need to know about EXEEXT, so add the
 dnl 'am__EXEEXT' conditional if _AM_COMPILER_EXEEXT was seen.  This
@@ -166,52 +172,6 @@ dnl mangled by Autoconf and run in a shell conditional statement.
 m4_define([_AC_COMPILER_EXEEXT],
 m4_defn([_AC_COMPILER_EXEEXT])[m4_provide([_AM_COMPILER_EXEEXT])])
 
-dnl We have to redefine AC_PROG_CC to allow our compile rules to use
-dnl "-c -o" together also with losing compilers.
-dnl FIXME: Add references to the original discussion and bug report.
-dnl FIXME: Shameless copy & paste from Autoconf internals, since trying to
-dnl        play smart among tangles of AC_REQUIRE, m4_defn, m4_provide and
-dnl        other tricks was proving too difficult, and in the end, likely
-dnl        more brittle too.  And this should anyway be just a temporary
-dnl        band-aid, until Autoconf provides the semantics and/or hooks we
-dnl        need (hint hint, nudge nudge) ...
-AC_DEFUN([AC_PROG_CC],
-m4_defn([AC_PROG_CC])
-[AC_REQUIRE([AM_AUX_DIR_EXPAND])dnl
-AC_REQUIRE_AUX_FILE([compile])dnl
-dnl FIXME The following abomination is expected to disappear in
-dnl       Automake 1.14.
-AC_MSG_CHECKING([whether $CC understands -c and -o together])
-set dummy $CC; am__cc=`AS_ECHO(["$[2]"]) | \
-                       sed 's/[[^a-zA-Z0-9_]]/_/g;s/^[[0-9]]/_/'`
-AC_CACHE_VAL([am_cv_prog_cc_${am__cc}_c_o],
-[AC_LANG_CONFTEST([AC_LANG_PROGRAM([])])
-# Make sure it works both with $CC and with simple cc.
-# We do the test twice because some compilers refuse to overwrite an
-# existing .o file with -o, though they will create one.
-ac_try='$CC -c conftest.$ac_ext -o conftest2.$ac_objext >&AS_MESSAGE_LOG_FD'
-rm -f conftest2.*
-if _AC_DO_VAR(ac_try) && test -f conftest2.$ac_objext
-then
-  eval am_cv_prog_cc_${am__cc}_c_o=yes
-else
-  eval am_cv_prog_cc_${am__cc}_c_o=no
-fi
-rm -f core conftest*
-])dnl
-if eval test \"\$am_cv_prog_cc_${am__cc}_c_o\" = yes; then
-  AC_MSG_RESULT([yes])
-else
-  AC_MSG_RESULT([no])
-  # Losing compiler, so wrap it with the 'compile' script.
-  # FIXME: It is wrong to rewrite CC.
-  # But if we don't then we get into trouble of one sort or another.
-  # A longer-term fix would be to have automake use am__CC in this case,
-  # and then we could set am__CC="\$(top_srcdir)/compile \$(CC)"
-  CC="$am_aux_dir/compile $CC"
-fi
-])
-
 # When config.status generates a header, we must update the stamp-h file.
 # This file resides in the same directory as the config header
 # that is generated.  The stamp files are numbered to have different names.
diff --git a/m4/minuso.m4 b/m4/minuso.m4
index 17fa8c9..984427c 100644
--- a/m4/minuso.m4
+++ b/m4/minuso.m4
@@ -7,19 +7,26 @@
 
 # AM_PROG_CC_C_O
 # --------------
-# Basically a no-op now, completely superseded by the AC_PROG_CC
-# adjusted by Automake.  Kept for backward-compatibility.
+# Like AC_PROG_CC_C_O, but changed for automake.
 AC_DEFUN([AM_PROG_CC_C_O],
-[AC_REQUIRE([AC_PROG_CC])dnl
+[AC_REQUIRE([AC_PROG_CC_C_O])dnl
+AC_REQUIRE([AM_AUX_DIR_EXPAND])dnl
+AC_REQUIRE_AUX_FILE([compile])dnl
+# FIXME: we rely on the cache variable name because
+# there is no other way.
+set dummy $CC
+am_cc=`echo $[2] | sed ['s/[^a-zA-Z0-9_]/_/g;s/^[0-9]/_/']`
+eval am_t=\$ac_cv_prog_cc_${am_cc}_c_o
+if test "$am_t" != yes; then
+   # Losing compiler, so override with the script.
+   # FIXME: It is wrong to rewrite CC.
+   # But if we don't then we get into trouble of one sort or another.
+   # A longer-term fix would be to have automake use am__CC in this case,
+   # and then we could set am__CC="\$(top_srcdir)/compile \$(CC)"
+   CC="$am_aux_dir/compile $CC"
+fi
 dnl Make sure AC_PROG_CC is never called again, or it will override our
 dnl setting of CC.
 m4_define([AC_PROG_CC],
           [m4_fatal([AC_PROG_CC cannot be called after AM_PROG_CC_C_O])])
-# For better backward-compatibility.  Users are advised to stop
-# relying on this cache variable and C preprocessor symbol ASAP.
-eval ac_cv_prog_cc_${am__cc}_c_o=\$am_cv_prog_cc_${am__cc}_c_o
-if eval test \"\$ac_cv_prog_cc_${am__cc}_c_o\" != yes; then
-  AC_DEFINE([NO_MINUS_C_MINUS_O], [1],
-            [Define to 1 if your C compiler doesn't accept -c and -o together.])
-fi
 ])
diff --git a/t/add-missing.tap b/t/add-missing.tap
index 9c4b774..053b9a1 100755
--- a/t/add-missing.tap
+++ b/t/add-missing.tap
@@ -62,6 +62,7 @@ AC_CANONICAL_TARGET
 AC_CANONICAL_SYSTEM
 AM_PATH_LISPDIR
 AM_PATH_PYTHON
+AC_OUTPUT
 END
 $ACLOCAL || framework_failure_ "cannot pre-compute aclocal.m4"
 
@@ -247,7 +248,6 @@ check_ <<'END'
 depcomp/C
 == Files ==
 depcomp
-compile
 == configure.ac ==
 AC_PROG_CC
 == Makefile.am ==
@@ -272,9 +272,10 @@ compile
 == Files ==
 compile
 == configure.ac ==
-# Using AC_PROG_CC in configure.ac should be enough.  No
-# need to also define, say, xxx_PROGRAMS in Makefile.am.
+# Using AC_PROG_CC and AC_OUTPUT in configure.ac should be enough.
+# No need to also define, say, xxx_PROGRAMS in Makefile.am.
 AC_PROG_CC
+AC_OUTPUT
 END
 
 # For config.guess and config.sub.
@@ -295,7 +296,6 @@ check_ <<'END'
 == Name ==
 ylwrap/Lex
 == Files ==
-compile
 ylwrap
 == configure.ac ==
 AC_PROG_CC
@@ -310,7 +310,6 @@ check_ <<'END'
 == Name ==
 ylwrap/Yacc
 == Files ==
-compile
 ylwrap
 == configure.ac ==
 AC_PROG_CC
diff --git a/t/am-prog-cc-c-o.sh b/t/am-prog-cc-c-o.sh
index da6a3a4..549cdcc 100755
--- a/t/am-prog-cc-c-o.sh
+++ b/t/am-prog-cc-c-o.sh
@@ -56,7 +56,7 @@ $AUTOMAKE --add-missing
 
 ./configure >stdout || { cat stdout; exit 1; }
 cat stdout
-grep 'understands -c and -o together.* yes$' stdout
+$EGREP 'understands? -c and -o together.* yes$' stdout
 # No repeated checks please.
 test $(grep -c ".*-c['\" ].*-o['\" ]" stdout) -eq 1
 $MAKE
@@ -83,7 +83,7 @@ CC=$am_testaux_builddir/cc-no-c-o; export CC
 
 ./configure >stdout || { cat stdout; exit 1; }
 cat stdout
-grep 'understands -c and -o together.* no$' stdout
+$EGREP 'understands? -c and -o together.* no$' stdout
 # No repeated checks please.
 test $(grep -c ".*-c['\" ].*-o['\" ]" stdout) -eq 1
 $MAKE
diff --git a/t/subobj.sh b/t/subobj.sh
index 22ab2d3..f595e68 100755
--- a/t/subobj.sh
+++ b/t/subobj.sh
@@ -23,6 +23,7 @@ AC_PROG_CC
 AC_PROG_CXX
 AC_PROG_YACC
 AC_CONFIG_FILES([sub/Makefile])
+AC_OUTPUT
 END
 
 $ACLOCAL
@@ -75,7 +76,7 @@ rm -f compile
 $AUTOMAKE --add-missing 2>stderr || { cat stderr >&2; exit 1; }
 cat stderr >&2
 # Make sure compile is installed, and that Automake says so.
-grep '^configure\.ac:4:.*install.*compile' stderr
+grep '^configure\.ac:[48]:.*install.*compile' stderr
 test -f compile
 
 grep '^generic/a\.\$(OBJEXT):' Makefile.in
-- 
1.8.3.rc0.19.g7e6a0cc

>From c148dc73a92c1df5e70a61e9495e62c010090bd4 Mon Sep 17 00:00:00 2001
Message-Id: <c148dc73a[email-masked]>
In-Reply-To: <32eb770b7[email-masked]>
References: <32eb770b7[email-masked]>
From: Nick Bowler <[email-masked]>
Date: Sat, 11 May 2013 11:45:16 +0200
Subject: [PATCH 2/2] Use AC_DEFUN_ONCE to define AM_PROG_CC_C_O

If AM_PROG_CC_C_O is expanded multiple times, and the compiler does not
support -c and -o together, each expansion of the macro will prepend
the compile script to CC.  This can result in the compile script
invoking the compile script, which at best pointless and silly.
Fortunately, there does not appear to be any serious problems as the
first compile invocation strips out -o options, causing subsequent
invocations of the script to merely exec their arguments.

Other than fixing the above, this should not normally cause any changes
to the resulting configure script, except in the (hopefully rare) case
where AM_PROG_CC_C_O is directly expanded (i.e., *not* using AC_REQUIRE)
in the body of a macro defined with AC_DEFUN.  In that case, the use of
AC_DEFUN_ONCE may cause the expansion of AM_PROG_CC_C_O to appear
earlier in the configure script.

* m4/minuso.m4: Change the definition of AM_PROG_CC_C_O to use
AC_DEFUN_ONCE, avoiding problems caused by multiple expansions.

Copyright-paperwork-exempt: yes
Signed-off-by: Stefano Lattarini <[email-masked]>
---
 m4/minuso.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/m4/minuso.m4 b/m4/minuso.m4
index 984427c..06f74c9 100644
--- a/m4/minuso.m4
+++ b/m4/minuso.m4
@@ -8,7 +8,7 @@
 # AM_PROG_CC_C_O
 # --------------
 # Like AC_PROG_CC_C_O, but changed for automake.
-AC_DEFUN([AM_PROG_CC_C_O],
+AC_DEFUN_ONCE([AM_PROG_CC_C_O],
 [AC_REQUIRE([AC_PROG_CC_C_O])dnl
 AC_REQUIRE([AM_AUX_DIR_EXPAND])dnl
 AC_REQUIRE_AUX_FILE([compile])dnl
-- 
1.8.3.rc0.19.g7e6a0cc