From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id D83CD3858D37 for ; Tue, 16 Apr 2024 14:47:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D83CD3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D83CD3858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::52c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713278844; cv=none; b=a5lFzlv23yRS8w+mHg8pfzg8Tzm47eMgz6a0u7NMYbvPTmuzwlBsvhklmEsW6IxGKTrKS6D4uf3aEMKvUSgCRhrygvGFEUOBENkuqmIkV+rwqcXNsBG4e6T2iIDd3qDSpNIdif50/vVlSuCLh0E3fXgpIU6vAc4PeEGGRLvIZ7g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713278844; c=relaxed/simple; bh=EusR87DjUYSE4WeqYfQZIh5wwjdlMl4/i58HcO3haRA=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=CTDp/ryPQ1n/vkjqo1M00giDI7bw1MMVr+EJ3E+Jby5HCzXkMdlnUbklR77ZlbwI08t33OnM8rg8PHV+1YjXA6jLl9k8csKigZC7Eo5zl1BQpP60TLtwyQB7Lsb+4+zkTDML89sb+2NIKlvBzkWbm9sdVS/0XQq96aOi8aB2LLA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ed1-x52c.google.com with SMTP id 4fb4d7f45d1cf-56e346224bdso3849553a12.1 for ; Tue, 16 Apr 2024 07:47:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1713278839; x=1713883639; darn=sourceware.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=eKEP1Dm46768a5xiXHAEfpoF1H5I4yxxoS8tIQfZIrM=; b=Nqj0fz6cPXzn/ZFX9kReT5R24S/CCbPat+cwnagTCtWbSPFz5ol/JHbFQ/L8LZR4S9 82SSQ6W3Ahs8oSSusqUrZVJAn2kKrqbe/fAhRvGRNbOGtIFB+roS8qB1T6yTWVufSaP7 ipHNYF+FgP3Ht8G6Av8npGkVXNb+9hi32brFTKgUGbMEr3b6E14AIxbZ4oSF+nqJGBlW FAnQKkumXJG0hmT0bbvfs3YnyyftBIe6hAvy1hz/lQradxJO/X4S07La6nkgSS/o/zmE NAYMo4OdttR1ifM5h/Xpbmb2fab0c6jBIwqIZHTb4W2SmV63Sw6++PGSZzkp1UGMtpU+ wHfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713278839; x=1713883639; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eKEP1Dm46768a5xiXHAEfpoF1H5I4yxxoS8tIQfZIrM=; b=O4iJcmCBjVkgLMBTeujZD2oTp3eXGqdKnL5GdkobehHOdDEO2OkQsfAq947mD7EzcX 1W0X/VB5xZ5iMRcYZvz8yrQ/DWOAk5dIrK1p8HbKQ/zZGPDATZJrOzQZpM3rxrKbH+YB hGQgoBmFiX32IPuqdMRUjbj7JH1qFscFqjyLiMdoQq4Fd1GLUl4qi7qXJxRBlPrr+DCG eIlr7yESYacSLVTmp1aefY2UkijZOXFepcIGPbtm6kOFRx9Pw+6+aQx6vSLzZrHM0nLf NmVRFDgihs3aek7vPHq8dRUM/qEbF55Z7gXZjuwtRTW9b2qqxaHs8yfodpjrs6n8wu0F KVUw== X-Gm-Message-State: AOJu0YyLJmgTZpmp4dGBmjfbkOp/ANwh1trTeuuYk1H8HF94vIUAu2q4 yvmps1QCIE7yl9wgSR+Dqm0HsURDXLmGMwRLoWZqInsnuhzvg4oAevfLoOSbeFpKj8MLpsVPiKj THIjObwF/4dnAi2OGNscb6MwHn48zO3XjgAeA6QmikLCzI5A/lf9YIA== X-Google-Smtp-Source: AGHT+IGFxoTn7tte1NK1KBo4SiolTWniNxY6DTF7PCQXjJX8IJnBJhSdlHhFM4HLojxZ8U5lJ7ucocF12mxwhHC77oE= X-Received: by 2002:a17:907:7e85:b0:a4e:4771:6267 with SMTP id qb5-20020a1709077e8500b00a4e47716267mr11127340ejc.4.1713278839213; Tue, 16 Apr 2024 07:47:19 -0700 (PDT) MIME-Version: 1.0 References: <20240412200559.1649050-1-christophe.lyon@linaro.org> <20240412200559.1649050-5-christophe.lyon@linaro.org> In-Reply-To: From: Christophe Lyon Date: Tue, 16 Apr 2024 16:47:13 +0200 Message-ID: Subject: Re: [PATCH 4/6] autoregen.py: Use autoreconf in most GCC directories To: Simon Marchi Cc: buildbot@sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 15 Apr 2024 at 16:48, Simon Marchi wrote: > > On 4/15/24 8:52 AM, Christophe Lyon wrote: > > On Mon, 15 Apr 2024 at 05:07, Simon Marchi wrote: > >> > >> > >> > >> On 2024-04-12 16:05, Christophe Lyon wrote: > >>> Add most of GCC's subdirectories to AUTORECONF_DIRS, since 'autorecon= f > >>> -f' works for them. > >>> > >>> A few subdirs still need to be regenerated "manually", because they d= o > >>> not have Makefile.am which autoreconf uses to determine which aclocal > >>> flags to use. > >> > >> gdb, for instance, doesn't use Makefile.am, and yet autoreconf works. = It > >> finds the aclocal include paths from the AC_CONFIG_MACRO_DIRS macro in > >> configure.ac. If the remaining subdirs that still need to be manually > >> handled used AC_CONFIG_MACRO_DIRS, would autoreconf work in them? > > > > that's because gdb's aclocal.m4 is generated with 'aclocal' without any= flag. > > > > If you look at gcc/gcc/Makefile.in, you'll see: > > ACLOCAL_AMFLAGS =3D -I ../config -I .. > > So to regenerate gcc/gcc/aclocal.m4, we need to call > > aclocal -I ../config -I .. > > but autoreconf looks for this info in Makefile.am which does not exist > > in this case. > > (see line 410 and below in autoreconf 2.69) > > I don't think aclocal gets the macro include paths from Makefile.am. It That's not what I am saying: it's autoreconf that gets aclocal_flags from Makefile.am > gets it from configure.ac's AC_CONFIG_MACRO_DIRS. See the doc for > AC_CONFIG_MACRO_DIRS: > > https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/Inpu= t.html#index-AC_005fCONFIG_005fMACRO_005fDIRS-1 > > Specify the given directories as the location of additional local > Autoconf macros. These macros are intended for use by commands like > autoreconf or aclocal that trace macro calls; they should be called > directly from configure.ac so that tools that install macros for > aclocal can find the macros=E2=80=99 declarations. Indeed, but that's "slightly" different from what I meant :-) > If I call `aclocal` without args in gcc/gcc, it re-generates aclocal.m4 > fine, because it got the include path info from configure.ac. If I > remove the `AC_CONFIG_MACRO_DIRS` line from gcc/gcc/configure.ac and run > aclocal.m4, then I am missing a bunch of m4 files in the resulting > aclocal.m4. If I remove the `ACLOCAL_AMFLAGS` variable from > gcc/gcc/Makefile.am, it has no effect on `aclocal`. That variable only > exists for the benefit of the Makefile rule that regenerates aclocal.m4, > lower in Makefile.am. But I would argue that it's unnecessary, since > that info is encoded in configure.ac, so the rule could call aclocal > without any -I args. Of course: aclocal does NOT read Makefile.am, but autoreconf does. So what I noticed for the 'gcc' subdir is that autoreconf will generate the same contents but also print a few warnings. This is because when calling aclocal with some -I flags, the contents of those included .m4 files is taken into account before scanning configure.ac, so macros like AM_PATH_PROG_WITH_TEST are defined before being used, while when using autoreconf (thus calling aclocal without any -I flag), those .m4 files are processed later, leading to warnings that AM_PATH_PROG_WITH_TEST is "not found in library". IIUC, there's an iterative process which means that things are re-assessed later. So.... it seems it's not a real problem to use autoreconf as you suggest, but we'll see such warnings in the buildbot logs (not causing errors IIUC). Maybe using explicit m4_include(....) would avoid this discrepancy but I'm not sure that's desirable. > > > I don't know how AC_CONFIG_MACRO_DIRS would help, maybe it would... > > > >> Out of the directories you commented out from AUTORECONF_DIRS, the > >> following re-generated cleanly with autoreconf for me: > >> > >> - libdecnumber > > So -I ../config is not needed by aclocal? > > > >> - gcc > > as mentioned above, here we have ACLOCAL_AMFLAGS =3D -I ../config -I .. > > so it's surprising it works without these flags? > > > >> - libiberty > >> - libobjc > > same as for gcc. > > Same answer as above, aclocal reads AC_CONFIG_MACRO_DIRS. > > > > >> So I would suggest adding them to AUTORECONF_DIRS. > > > > Well, maybe :-) > > I must confess the doc for autoconf is not clear enough for me :-) > > https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/In= put.html > > does not mention AC_CONFIG_MACRO_DIRS, only AC_CONFIG_MACRO_DIR > > > > And the "Note that if you use aclocal from Automake to generate aclocal= .m4[...]" > > does not clearly say that AC_CONFIG_MACRO_DIR[S] is enough. > > (and the code I saw in autoreconf only checks /^ACLOCAL_[A-Z_]*FLAGS\s*= =3D\s*(.*) > > in Makefile.am. > > > > https://www.gnu.org/software/autoconf/manual/autoconf-2.70/html_node/In= put.html > > is more verbose and does mention AC_CONFIG_MACRO_DIRS, but we still use= 2.69 > > so I prefered to stay on the "safe" side. > > > > Maybe worth asking on the autoconf/automake list? > > I had to dig a bit to understand, because autoconf 2.69 indeed does not > mention AC_CONFIG_MACRO_DIRS. The answer is that automake 1.15.1 ships > with this for "older" autoconfs - like 2.69 - that don't provide > AC_CONFIG_MACRO_DIRS: > > https://git.savannah.gnu.org/cgit/automake.git/tree/m4/internal/ac-config= -macro-dirs.m4?h=3Dv1.15.1 > > The macro doesn't actually do anything, it's just some kind of marker > that aclocal looks for to get the paths. If we use AC_CONFIG_MACRO_DIRS > throughout the binutils-gdb and gcc repositories, it must be because it > works with autoconf 2.69 somehow, because we never used an autoconf more > recent than 2.69. > > > > >>> Most use our default aclocal -I../config, but a few need special > >>> flags, which I copied from the corresponding Makefile.in: > >>> * fixincludes: -I.. -I../config > >>> * gcc, libiberty, libobjc: -I../config -I.. > >>> * libgm2: -I../config -I.. although Makefile.in says otherwise. Using > >>> what Makefile.in defines results in generating aclocal.m4 with a > >>> different contents than what it is in the repo. The repo is > >>> presumably wrong, but use this until this is fixed. > >>> > >>> Note that we do not regenerate > >>> libvtv/testsuite/other-tests/Makefile.in, because > >>> libvtv/testsuite/other-tests/ has no configure.ac. > >>> > >>> Running this script over GCC's repository generates quite a few > >>> warnings, which are the same as before this patch. Namely, these > >>> subdirs seem to have incorrect autotools files (at least partially): > >>> * libgfortran > >>> * libgomp > >>> * libsanitizer > >>> > >>> This patch does not support regenerating gcc/m2/gm2-libs/config-host > >>> and gcc/m2/gm2-libs/gm2-libs-host.h.in because I didn't manage to > >>> regenerate them with the exact same content. FTR I tried: > >>> autoconf -f config-host.in > config-host > >>> autoheader config-host.in > >>> as described in gcc/m2/Make-maintainer.in > >>> > >>> Similarly, we skip libcpp because the files we regenerate have small > >>> differences with the current versions. > >> > >> libcpp's files appear to be in an invalid state in the repo itself. > >> According to libcpp/Makefile.in line 123, aclocal.m4 should be > >> regenerated with `aclocal -I ../config`. When I use that, I get the > >> same results as what autoreconf would give me. For that one, I think > >> you could send a patch to gcc to fix it in the repo, after which we ca= n > >> switch it to use autoreconf. > > > > Yes I noticed a discrepancy about override.m4, but I thought this is > > too late in GCC stage4 to fix it. > > Maybe I'm wrong on this assumption though :-) > > IMO it's worth asking, it could be considered a bug to not have the > files correctly re-generated. > > Simon