On Tue, Mar 7, 2017 at 12:17 AM, Ondrej Zajicek <santi...@crfreenet.org> wrote: > On Sat, Mar 04, 2017 at 06:13:16PM +0100, Ruben Kerkhof wrote: >> The goal of this series is to make it possible to run autoreconf on a git >> checkout. >> >> It does this by moving configure.in to configure.ac and fixes deprecation >> warnings >> and errors generated by autoheader. > > Hi
Hi Ondrej, > > Thanks for the cleanup patches, our configure script is old and not much > kept up-to-date. I have some questions w.r.t. your patches: Thanks for looking at my patches. > > It seems that (by removing sysdep/autoconf.h.in) the patchset not only > make it possible to run autoreconf, but make it necessary. I am not > familiar with that tool, could you briefly describe what are advantages > of using it? Of course. You're right, I should have mentioned this in the commit message. autoreconf ships with autoconf, and it has been the standard tool to bootstrap a development checkout for a few years. Note, it is only needed if you want to develop from a git checkout, not for an end user building from a tarball. There's more documentation at https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/autoreconf-Invocation.html I also believe there's strong interest, at least in Debian and in Fedora too, to run autoreconf by default, to make sure config.sub is always up to date: https://wiki.debian.org/Autoreconf > > >> rename aclocal.m4 => m4/bird.m4 (100%) > > Is this necessary? I have an aversion to boilerplate directories > containing just one file. It's not strictly necessary since bird doesn't use automake, but m4/ is somewhat of a canonical location. I have plans to split this file up in later patches, and to see which ones of these macros are still needed and possibly redo them differently, in configure.ac itself. > > >> +AH_TEMPLATE(CONFIG_BABEL) >> +AH_TEMPLATE(CONFIG_BFD) >> ... > > Shouldn't quotes be also there? Esp. when other patches added plenty of > quotes to other macro expressions. You're right, I missed that. > > >> -AC_INIT(conf/confbase.Y) >> +AC_INIT > > According to autoconf doc, AC_INIT should have at least arguments > 'package, version'? Well, they were missing in the old code too ... It works fine without these until you start using automake. I didn't want to define the package version in there yet, since there's BIRD_VERSION too. I can add this and replace BIRD_VERSION with PACKAGE_VERSION if you want in a followup patch. > > > -- > Elen sila lumenn' omentielvo > > Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org) > OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) > "To err is human -- to blame it on a computer is even more so." Kind regards, Ruben Kerkhof
