Re: [PATCH] gdb: add ../config/pkg.m4 in acinclude.m4

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-05-09 9:14 p.m., Mike Frysinger wrote:
> On 09 May 2021 20:16, Simon Marchi via Gdb-patches wrote:
>> On 2021-05-09 4:00 p.m., Mike Frysinger wrote:
>>> On 08 May 2021 21:24, Simon Marchi via Gdb-patches wrote:
>>>> Since commit adeab0c5b33f ("config/debuginfod: do not include pkg.m4
>>>> directly"), it no longer works to re-generate the autoconf stuff in GDB
>>>> without passing "-I ../config":
>>>>
>>>>     $ autoreconf -f
>>>>     configure.ac:161: error: possibly undefined macro: AC_MSG_ERROR
>>>>           If this token and others are legitimate, please use m4_pattern_allow.
>>>>           See the Autoconf documentation.
>>>>     configure.ac:329: error: possibly undefined macro: AC_MSG_WARN
>>>>     configure.ac:409: error: possibly undefined macro: AC_DEFINE
>>>>     autoreconf: /opt/autostuff/2.69/bin/autoconf failed with exit status: 1
>>>>
>>>> The reason is: since that commit, the PKG_CHECK_MODULES macro is not
>>>> found.  When passing "-I ../config" to the autoconf tools, then it's
>>>> found.  But if I understand correctly, the reason why we have all these
>>>> includes in acinclude.m4 is to avoid having to specify that, to make the
>>>> default invocation without special arguments "just work".
>>>
>>> how about this instead ?  i can run `aclocal && autoconf` with this.
>>>
>>> --- a/gdb/configure.ac
>>> +++ b/gdb/configure.ac
>>> @@ -18,9 +18,8 @@ dnl along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>>  
>>>  dnl Process this file with autoconf to produce a configure script.
>>>  
>>> -m4_include(../config/debuginfod.m4)
>>> -
>>>  AC_INIT
>>> +AC_CONFIG_MACRO_DIRS([../config])
>>>  AC_CONFIG_SRCDIR([main.c])
>>>  AC_CONFIG_HEADERS(config.h:config.in, [echo > stamp-h])
>>>  AM_MAINTAINER_MODE
>>
>> Ah yes, that sounds much better.  If we have this, could we remove all
>> lines in acinclude.m4 that refer to a file in ../config?
> 
> we can remove most, but not all.  there seems to be some bugs in aclocal where
> it warns about not finding a few of the macros, but it actually does.  probably
> worth leaving the explicit includes so as to not confuse devs.
> 
> updated patch below
> -mike
> 
> --- a/gdb/acinclude.m4
> +++ b/gdb/acinclude.m4
> @@ -21,49 +21,12 @@ m4_include(../gdbsupport/selftest.m4)
>  dnl gdb/configure.in uses BFD_NEED_DECLARATION, so get its definition.
>  m4_include(../bfd/bfd.m4)
>  
> -dnl This gets the standard macros.
> -m4_include(../config/acinclude.m4)
> -
> -dnl This gets GCC_ENABLE.
> -sinclude(../config/enable.m4)
> -
> -dnl This gets AC_PLUGINS, needed by ACX_LARGEFILE.
> -m4_include(../config/plugins.m4)
> -
> -dnl For ACX_LARGEFILE.
> -m4_include(../config/largefile.m4)
> -
> -dnl For AM_SET_LEADING_DOT.
> -m4_include(../config/lead-dot.m4)
> -
> -dnl This gets autoconf bugfixes.
> -m4_include(../config/override.m4)
> -
> -dnl For ZW_GNU_GETTEXT_SISTER_DIR.
> -m4_include(../config/gettext-sister.m4)
> -
> -dnl For AC_LIB_HAVE_LINKFLAGS.
> -m4_include(../config/lib-ld.m4)
> -m4_include(../config/lib-prefix.m4)
> -m4_include(../config/lib-link.m4)
> -
> -dnl For ACX_PKGVERSION and ACX_BUGURL.
> -m4_include(../config/acx.m4)
> -
> -dnl for TCL definitions
> -m4_include(../config/tcl.m4)
> -
> -dnl For dependency tracking macros.
> -m4_include([../config/depstand.m4])
> -
>  dnl For AM_LC_MESSAGES
>  m4_include([../config/lcmessage.m4])
>  
>  dnl For AM_LANGINFO_CODESET.
>  m4_include([../config/codeset.m4])
>  
> -m4_include([../config/iconv.m4])
> -
>  m4_include([../config/zlib.m4])
>  
>  m4_include([../gdbsupport/common.m4])
> @@ -76,8 +39,6 @@ m4_include(../gdbsupport/ptrace.m4)
>  
>  m4_include(ax_cxx_compile_stdcxx.m4)
>  
> -m4_include([../config/ax_pthread.m4])
> -
>  dnl written by Guido Draheim <guidod@gmx.de>, original by Alexandre Oliva 
>  dnl Version 1.3 (2001/03/02)
>  dnl source http://www.gnu.org/software/ac-archive/Miscellaneous/ac_define_dir.html
> --- a/gdb/aclocal.m4
> +++ b/gdb/aclocal.m4
> @@ -199,5 +199,20 @@ AC_DEFUN([_AM_SUBST_NOTMAKE])
>  # Public sister of _AM_SUBST_NOTMAKE.
>  AC_DEFUN([AM_SUBST_NOTMAKE], [_AM_SUBST_NOTMAKE($@)])
>  
> +m4_include([../config/acx.m4])
> +m4_include([../config/ax_pthread.m4])
> +m4_include([../config/debuginfod.m4])
> +m4_include([../config/depstand.m4])
> +m4_include([../config/enable.m4])
> +m4_include([../config/gettext-sister.m4])
> +m4_include([../config/iconv.m4])
> +m4_include([../config/largefile.m4])
> +m4_include([../config/lead-dot.m4])
> +m4_include([../config/lib-ld.m4])
> +m4_include([../config/lib-link.m4])
> +m4_include([../config/lib-prefix.m4])
> +m4_include([../config/override.m4])
>  m4_include([../config/pkg.m4])
> +m4_include([../config/plugins.m4])
> +m4_include([../config/tcl.m4])
>  m4_include([acinclude.m4])
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -2990,6 +2990,7 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
>  
>  
>  
> +
>  ac_config_headers="$ac_config_headers config.h:config.in"
>  
>  
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -18,9 +18,8 @@ dnl along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
>  dnl Process this file with autoconf to produce a configure script.
>  
> -m4_include(../config/debuginfod.m4)
> -
>  AC_INIT
> +AC_CONFIG_MACRO_DIRS([../config])
>  AC_CONFIG_SRCDIR([main.c])
>  AC_CONFIG_HEADERS(config.h:config.in, [echo > stamp-h])
>  AM_MAINTAINER_MODE
> 

Seeing that there is no significant change to the generated configure,
that LGTM.  However, it would be good to maybe wait for Tom Tromey's
approval, he has more experience with this.

Also, the same should be applied to gdbsupport and gdbserver.

Simon