Re: Broken makefile given Autoconf version mismatch

[Ralf Wildenhues]

Hi Stepan,

Thanks for the review!

* Stepan Kasal wrote on Thu, May 25, 2006 at 08:28:03PM CEST:
> On Thu, May 25, 2006 at 03:43:11PM +0200, Ralf Wildenhues wrote:

> > +  AC_MSG_WARN([ $ac_file seems to ignore the --datarootdir setting])
> 
> I'd delete the space before $ac_file.  (The value of ac_file_inputs
> starts by a space, but that is not intentional, it just is not worth
> the hassle to fix it.)

Sure.

> I don't like that the two messages are similar: the former one
> indicates a problem, which is most probably worked around, while the
> latter one indicates a bug which will most probably hit.
> 
> I think the latter one should be much longer and should speak about
> ``referenece to undefined variable'' or some such.

Yes, I agree.

> And the check
*snip*

> comes too late, $ac_file can be `-'.  You should check $tmp/out as
> soon as it is created, a few lines above.

Yep.

> > The Debian switch to 2.59c has already uncovered another (wrong) usage
> > case: in an AC_DEFINE*.
> 
> I have no idea what workaround or warning could be added to fix this.

Me neither.

> But I think we can let it be, this is less common than the AC_SUBST
> problem.

I'm not so sure about this, unfortunately.  That is, I'm not sure it's
less common, however I do think it will lead to rather obvious failures.
(Remember silent failures are the ones we should really be afraid of.)

> > (It's on purpose that I'm putting warning signs everywhere near the
> > wrong AC_SUBST ... too many people picking up stuff from anywhere.)
> 
> I wasn't able to understand this, sorry.

Well, everytime we mention
  AC_SUBST([mydatadir], [$datadir/my])   dnl This is wrong!!!

we should also mention that it is wrong.  So that the random google hit
the messages we're currently writing eventually end up as, don't make
for bad examples.  ;-)

Updated patch below.

Cheers,
Ralf

2005-05-25  Stepan Kasal  <address@hidden>
        and Ralf Wildenhues <address@hidden>

        * lib/autoconf/status.m4 (_AC_OUTPUT_FILE): If we have not seen
        mention of `datarootdir' in the input file(s), but literal
        `${datarootdir}' in the output file, and we haven't warned yet,
        then warn as well: the user may have (erroneously) used
        `AC_SUBST([mydatadir], [$datadir/my])' instead of the correct
        `AC_SUBST([mydatadir], ['${datadir}/my'])'.
        * tests/torture.at (datarootdir workaround): Extend this test.
        * NEWS: Update.

Index: NEWS
===================================================================
RCS file: /cvsroot/autoconf/autoconf/NEWS,v
retrieving revision 1.372
diff -u -r1.372 NEWS
--- NEWS        22 May 2006 15:54:09 -0000      1.372
+++ NEWS        25 May 2006 20:16:28 -0000
@@ -1,5 +1,8 @@
 * Major changes in Autoconf 2.59d
 
+** Even more safety checks for the new Directory variables:
+  Warn about suspicious `${datarootdir}' found in config files output.
+
 ** ac_config_guess, ac_config_sub, ac_configure
   These never-documented variables have been marked with a comment
   saying that we intend to remove them in a future release.
Index: lib/autoconf/status.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autoconf/status.m4,v
retrieving revision 1.108
diff -u -r1.108 status.m4
--- lib/autoconf/status.m4      23 May 2006 23:30:57 -0000      1.108
+++ lib/autoconf/status.m4      25 May 2006 20:16:40 -0000
@@ -506,7 +506,7 @@
 cat >>$CONFIG_STATUS <<\_ACEOF
 # If the template does not know about datarootdir, expand it.
 # FIXME: This hack should be removed a few years after 2.60.
-ac_datarootdir_hack=
+ac_datarootdir_hack=; ac_datarootdir_seen=
 m4_define([_AC_datarootdir_vars],
           [datadir, docdir, infodir, localedir, mandir])
 case `sed -n '/datarootdir/ {
@@ -516,7 +516,7 @@
 m4_foreach([_AC_Var], m4_defn([_AC_datarootdir_vars]),
            [/@_AC_Var@/p
 ])' $ac_file_inputs` in
-*datarootdir*) ;;
+*datarootdir*) ac_datarootdir_seen=yes;;
 address@hidden(address@hidden|address@hidden, _AC_datarootdir_vars)@*)
   AC_MSG_WARN([$ac_file_inputs seems to ignore the --datarootdir setting])
 _ACEOF
@@ -551,6 +551,11 @@
 $ac_datarootdir_hack
 " $ac_file_inputs m4_defn([_AC_SED_CMDS])>$tmp/out
 
+test -z "$ac_datarootdir_hack$ac_datarootdir_seen" &&
+  grep '\${datarootdir}' "$tmp/out" &&
+  AC_MSG_WARN([$ac_file contains a reference to the variable `datarootdir'
+which seems to be undefined.  Please make sure it is defined.])
+
   rm -f "$tmp/stdin"
   case $ac_file in
   -) cat "$tmp/out"; rm -f "$tmp/out";;
Index: tests/torture.at
===================================================================
RCS file: /cvsroot/autoconf/autoconf/tests/torture.at,v
retrieving revision 1.62
diff -u -r1.62 torture.at
--- tests/torture.at    17 May 2006 02:20:15 -0000      1.62
+++ tests/torture.at    25 May 2006 20:16:40 -0000
@@ -667,16 +667,27 @@
 @mandir@
 ])
 
+AT_DATA([Bar.in],
address@hidden@
+])
+
 AT_DATA([configure.ac],
 [[AC_INIT
 AC_CONFIG_AUX_DIR($top_srcdir/config)
-AC_CONFIG_FILES([Foo])
+
+# This substitution is wrong and bogus!  Don't use it in your own code!
+# Read `info Autoconf "Defining Directories"'!
+AC_SUBST([mydatadir], [${datadir}/my])
+
+AC_CONFIG_FILES([Foo Bar])
 AC_OUTPUT
 ]])
 
 AT_CHECK_AUTOCONF
 AT_CHECK_CONFIGURE([], [], [],
   [config.status: WARNING:  Foo.in seems to ignore the --datarootdir setting
+config.status: WARNING: Bar contains a reference to the variable `datarootdir'
+which seems to be undefined.  Please make sure it is defined.
 ])
 AT_CHECK([grep datarootdir Foo], 1, [])
 AT_CLEANUP