Re: [Xen-devel] [PATCH RFC] build: add autoconf to replace custom checks in tools/check

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-01-09 at 13:51 +0000, Roger Pau Monnà wrote:
> 2012/1/9 Ian Campbell <Ian.Campbell@xxxxxxxxxx>:
> > On Sat, 2012-01-07 at 03:20 +0000, Roger Pau Monne wrote:
> >> # HG changeset patch
> >> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> >> # Date 1325906230 -3600
> >> # Node ID e12ec1071410c946367cb0508cf218a0c3b596ca
> >> # Parent  4086e4811547ddffb9a53fbf2efb6c2fa436b70a
> >> build: add autoconf to replace custom checks in tools/check
> >>
> >> Added autotools magic to replace custom check scripts. The previous
> >> checks have been ported to autoconf, and some additional ones have
> >> been added (plus the suggestions from running autoscan). Two files are
> >> created as a result from executing configure script,
> >> config/Autoconf.mk and config.h.
> >>
> >> Autoconf.mk is included by Config.mk, and contains most of the
> >> options previously defined in .config, that can now be set passing
> >> parameters or defining environment variables when executing configure
> >> script.
> >>
> >> config.h is still not used anywhere, and is automatically created by
> >> autoheader, altough this migh change when we start to include this
> >> file.
> >>
> >> Just a first release, and since Iit's my first autoconf script I guess
> >> there will be many things to polish here... Please review and comment.
> >>
> >> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> >
> > I would have expected to see an update to the clean/distclean rules
> > and .hgignore due to all the new generated files.
> 
> Yes, I was going to do that when we agree on were should things
> reside, that's why I didn't include any of this on the patch.

OK

> >> diff -r 4086e4811547 -r e12ec1071410 README
> >> --- a/README    Thu Jan 05 17:25:23 2012 +0000
> >> +++ b/README    Sat Jan 07 04:17:10 2012 +0100
> >> @@ -87,9 +87,15 @@ 2. cd to xen-unstable (or whatever you s
> >>  3. For the very first build, or if you want to destroy build trees,
> >>     perform the following steps:
> >>
> 
> This will be like:
> 
> # aclocal && automake -a
> 
> >> +    # automake -a
> >
> > We aren't using automake though? Perhaps this explains your problem with
> > things alwyas trying to use Makefile.am?
> 
> We need to run automake, because it generates config.sub and
> config.guess, which is needed for certain autoconf tests. The problem
> is that I don't know how to call automake to only generate those files
> and don't try to parse Makefile.am, because automake tries to parse
> Makefile.am and exits with an error code, that's also preventing me
> from using autoreconf and forget about calling aclocal, automake,
> autoheader... separately. I was hoping that some autotools expert has
> a solution for this (I will keep searching also).

I'm afraid I don't know the answer to this.  It seems like it should be
obvious that we shouldn't be running automake unless we want to use
automake, but with auto* who knows what goes on ;-).

[...]

> >> +    # ./configure
> >>      # make world
> >>      # make install
> >>
> >> +   If you want, you can run ./configure --help to see the list of
> >> +   optins available options when building and installing Xen.
> >
> >      options
> 
> Also Ok when we decide the definitive options.

I was pointing out the typo "optins".

> >
> >> +XML2-CONFIG         := @XML@
> >> +BASH                := @BASH@
> >> +XGETTTEXT           := @XGETTEXT@
> >> +
> >> +# Extra folder for libs/includes
> >> +PREPEND_INCLUDES    := @PREPEND_INCLUDES@
> >> +PREPEND_LIB         := @PREPEND_LIB@
> >> +APPEND_INCLUDES     := @APPEND_INCLUDES@
> >> +APPEND_LIB          := @APPEND_LIB@
> >> +
> >> +# Enable XSM security module (by default, Flask).
> >> +XSM_ENABLE          := @xsm@
> >> +FLASK_ENABLE        := @xsm@
> >> +
> >> +# Download GIT repositories via HTTP or GIT's own protocol?
> >> +# GIT's protocol is faster and more robust, when it works at all 
> >> (firewalls
> >> +# may block it). We make it the default, but if your GIT repository 
> >> downloads
> >> +# fail or hang, please specify GIT_HTTP=y in your environment.
> >> +GIT_HTTP            := @githttp@
> >> +
> >> +# Optional components
> >> +XENSTAT_XENTOP      := @monitors@
> >> +VTPM_TOOLS          := @vtpm@
> >> +LIBXENAPI_BINDINGS  := @xapi@
> >> +PYTHON_TOOLS        := @pythontools@
> >> +OCAML_TOOLS         := @ocamltools@
> >> +CONFIG_MINITERM     := @miniterm@
> >> +CONFIG_LOMOUNT      := @lomount@
> >> +
> >> +#System options
> >> +CONFIG_SYSTEM_LIBAIO:= @system_aio@
> >> +CONFIG_LIBICONV     := @libiconv@
> >> +CONFIG_GCRYPT       := @libgcrypt@
> >> +CONFIG_EXT2FS       := @libext2fs@
> >> diff -r 4086e4811547 -r e12ec1071410 configure.ac
> >> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> >> +++ b/configure.ac      Sat Jan 07 04:17:10 2012 +0100
> >> @@ -0,0 +1,179 @@
> >> +#                                               -*- Autoconf -*-
> >> +# Process this file with autoconf to produce a configure script.
> >> +
> >> +AC_PREREQ([2.67])
> >
> > This is the version in Debian stable which is my rule of thumb for how
> > far back to support things as a minimum.
> >
> > However do we rely on features of 2.67 or could we go older if someone
> > has an older version?
> 
> AC_PREREQ is automatically set by autoscan based on the used version.
> I've generated this with 2.68, and changed the AC_PREREQ to 2.67
> because I knew that you where going to check debian stable version.
> I've tested this under 2.67 with Debian and it works fine, but I don't
> know about older versions.

I think we can fault them in as testers report issues.

[...]

> > I didn't review all the tests in detail, I presume we'll end up having
> > to find a bunch of those errors via testing anywhere.
> >
> > Does the set of tests here precisely equal those currently done by
> > tools/check (or elsewhere) or did you add more as you went?
> 
> I've moved all tools/check, and added a test for libiconv and
> yajl/yajl_version.h.

There seems to be a lot more tests in the configure.ac than there ever
were under tools/check though? All sorts of headers and function checks
etc. Did they come from autoscan? (autoscan is new to me...)

> > It might
> > have been better for review to present this as a basic infrastructure
> > patch, the conversion of each test individually as a patch each and
> > lastly any new tests etc so we could easily review. But I think it's a
> > bit late now and there's no point splitting this patch up now you've got
> > it.
> 
> Each test has it's own m4 file inside m4/, if it's really difficult to
> understand (m4 is not easy for the eye, although I tried to follow
> libxl CODING_STYLE), I will split it up in several patches (one for
> each m4/ and one for the rest of the autotools infrastructure).

I guess most of the stuff was from autoscan can't logically be split up,
so don't worry.

> > [...]
> >> +AS_IF([test "x$ocamltools" = "xy"], [
> >> +    AC_PROG_OCAML
> >> +    AS_IF([test "x$OCAMLC" = "xno"], [
> >> +      AC_MSG_ERROR([You must install the OCaml compiler])
> >
> > ocaml was previously optional, I think.
> 
> Ok, I will set to disabled by default then.

By default it should be enabled if the necessary bits are present and
disabled otherwise.

[...]
> > There's no AC_TYPE_STDINT or similar?
> 
> This was automatically done by autoscan, and I've tried searching for
> something like AC_TYPE_STDINT, but I haven't been able to find
> anything. If you want I can bundle the INT checks inside a macro
> called AX_TYPE_STDINT.
[...]
> > Is there not some umbrella test for these sorts of very standard things,
> > e.g. AC_POSIX or something like that?
> 
> Again, the same as above.

Lets just leave it then.

> > Isn't the management of these snippets the sort of thing aclocal does
> > for you?
> 
> aclocal just scans configure.ac and concatenates all necessary macros
> into aclocal.m4, I've tried to use all the predefined macros I could,
> and most of the files inside m4/* are just wrappers to make
> configure.ac cleaner and easier to understand.

OK.

> > [...]
> >> diff -r 4086e4811547 -r e12ec1071410 m4/ocaml.m4
> >> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
> >> +++ b/m4/ocaml.m4       Sat Jan 07 04:17:10 2012 +0100
> >> @@ -0,0 +1,240 @@
> >> +dnl autoconf macros for OCaml
> >
> > Please add a comment describing where this came from so we know how to
> > update in the future.
> >
> > Shouldn't/couldn't this be provided by the ocaml packages and just
> > included by us?
> 
> This comes from http://forge.ocamlcore.org/, and the manpage states that:
> 
> "To begin using these macros, you will need to copy the ocaml.m4 file
> (usually located at /usr/share/aclocal/ocaml.m4) to the autoconf
> macros directory in your project. Normally this is the m4/ directory
> in your project, but the directory can be changed using the
> AC_CONFIG_MACRO_DIR(DIR) directive."
> 
> That's why I copied it to m4/

Fair enough!

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel