Re: [PATCH] gdb/gdbserver: switch to AC_CONFIG_MACRO_DIRS

[Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>]

On 17 Jun 2021 10:43, Simon Marchi via Gdb-patches wrote:
> On 2021-06-17 12:21 a.m., Mike Frysinger wrote:
> > On 16 Jun 2021 22:30, Simon Marchi wrote:
> >> On 2021-06-15 1:44 a.m., Mike Frysinger via Gdb-patches wrote:
> >>> These dirs don't use automake, so use AC_CONFIG_MACRO_DIRS to specify
> >>> ../config as a search dir for m4 macros.  This allows removal of a lot
> >>> of hand-written m4_include's from acinclude.m4 files, and simplifies
> >>> use of `aclocal` or `autoreconf` as manual -I is not needed.
> >>
> >> For some reason, I get this when reconfiguring gdb:
> >>
> >>     configure.ac:531: warning: macro 'AM_ICONV' not found in library
> >>
> >> The result looks ok, the resulting configure looks fine, but if you
> >> happen to know why...
> > 
> > i don't know why automake is able to find some of the ../config/ m4 files
> > just fine, but a few others it barfs on.  i don't see the warning because
> > i have gettext installed and automake finds the system iconv.m4.
> 
> There seems to be a special case for printing that warning for AM_*
> macros:
> 
> https://github.com/autotools-mirror/automake/blob/e7724fb1b7b97f662caf154414e6f71ffcbd966c/bin/aclocal.in#L528-L540
> 
> I debugged aclocal as much as I could, and it seems to find AM_ICONV
> just fine, with --verbose I see:
> 
>   aclocal: found macro AM_ICONV in ../config/iconv.m4: 104
> 
> Also, note that with `aclocal -I ../config` I do not see the warning...
> but aclocal does support AC_CONFIG_MACRO_DIRS, so this difference is
> surprising.
> 
> So I'd be tempted to call that an aclocal bug, although a benign one.

oof, i agree, that smells like an aclocal bug.  i'm not sure i'd call it benign
when it's indistinguishable from actual missing macros.  for example:
$ echo AM_FOO >> configure.ac
$ aclocal
configure.ac:121: warning: macro 'AM_FOO' not found in library
$ echo $?
0

i think autoconf might fail (i hope that's always the case), but it feels a
bit wrong for it to be disconnected as such.  and hopefully people always
check the exit code of their tools ;).

> I'd suggest still just doing the right thing and removing all the
> ../config/* includes, and just ignore the warnings.  I'll try to open an
> automake bug or ask on the mailing list eventually.

this seems to go against our -Werror approach to things.  i also suspect
it'll trip up devs who waste time trying to figure out why there's this
warning (and surely it's their problem because no one else in the project
would allow this to creep in), which means a snowball effect for people.

so all things considered, i think the explicit include to silence the
warning is the right trade-off.  we can add some comments explaining
why they're there so that at least doesn't keep tripping us up.

how about at the top of acinclude.m4:
dnl NB: When possible, try to avoid explicit includes of ../config/ files.
dnl They're normally found by aclocal automatically and recorded in aclocal.m4.
dnl However, some are kept here explicitly to silence harmless warnings from
dnl aclocal when it finds AM_xxx macros via local search paths instead of
dnl system search paths.
-mike