[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.

>> 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).

>> + Â Â# autoheader && autoconf
>
> What does autoheader do? This should be clearly noted as being only
> necessary if building from hg and not if you are building from tarball.
> I expect we also need some makefile runes or process updates to make
> sure the generated stuff gets included in the tarball.

autoheader creates a generic config.h.in which contains defines for
all the exported variables. We could create our own config.h.in and
don't run autoheader, but I think the generated file is fine.

>
> You need to add the relevant packages to the list of build requirements.

Ok, will make a list

>> + Â Â# ./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.

>> +
>> Â Â This will create and install onto the local machine. It will build
>> Â Â the xen binary (xen.gz), the tools and the documentation.
>>
>> diff -r 4086e4811547 -r e12ec1071410 config/Autoconf.mk.in
>> --- /dev/null  Thu Jan 01 00:00:00 1970 +0000
>> +++ b/config/Autoconf.mk.in   Sat Jan 07 04:17:10 2012 +0100
>> @@ -0,0 +1,49 @@
>> +# Prefix and install folder
>> +PREFIX Â Â Â Â Â Â Â:= @prefix@
>> +LIBLEAFDIR_x86_64 Â := @LIB_PATH@
>> +
>> +# A debug build of Xen and tools?
>> +debug        := @debug@
>> +
>> +# Tools path
>> +BISON Â Â Â Â Â Â Â := @BISON@
>> +FLEX Â Â Â Â Â Â Â Â:= @FLEX@
>> +PYTHON Â Â Â Â Â Â Â:= @PYTHON@
>> +PERL Â Â Â Â Â Â Â Â:= @PERL@
>> +BRCTL Â Â Â Â Â Â Â := @BRCTL@
>> +IP Â Â Â Â Â Â Â Â Â:= @IP@
>> +CURL-CONFIG Â Â Â Â := @CURL@
>
> Are hyphens ok in make variables?

Looks like, but I will replace them with underscores (_) to be on the safe side.

>
>> +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.

>> +AC_INIT([Xen Hypervisor], [4.2], [xen-devel@xxxxxxxxxxxxxxxxxxx])
>
> Ideally the 4.2 would not be duplicated both here and in xen/Makefile.
> I'm not sure how this can be resolved without conflicting with Jan's
> (quite reasonable) desire not to need ./configure for the hypervisor.
>
> I suppose it must be possible via the magic of m4 to pull out the
> version from the Makefile and include it here.

Probably with some m4 and shell scripting

> 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.

> 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).

>
>> +AC_CONFIG_SRCDIR([tools/libxl/libxl.c])
>> +AC_CONFIG_HEADERS([config.h])
>> +AC_CONFIG_FILES([config/Autoconf.mk])
>> +AC_PREFIX_DEFAULT([/usr])
>> +
>> +# Check if CFLAGS, LDFLAGS, LIBS, CPPFLAGS or CPP is set and print a warning
>> +
>> +AS_IF([test -n "$CC$CFLAGS$LDFLAGS$LIBS$CPPFLAGS$CPP"], [
>> + Â ÂAC_MSG_WARN(
>> +[Setting CC, CFLAGS, LDFLAGS, LIBS, CPPFLAGS or CPP is not \
>> +recommended, use PREPEND_INCLUDES, PREPEND_LIB, \
>> +APPEND_INCLUDES and APPEND_LIB instead when possible.])
>> +])
>> +
>> +AC_USE_SYSTEM_EXTENSIONS
>> +AC_CANONICAL_HOST
>> +
>> +# M4 Macro includes
>> +m4_include([m4/enable_feature.m4])
>> +m4_include([m4/disable_feature.m4])
>> +m4_include([m4/path_or_fail.m4])
>> +m4_include([m4/python_xml.m4])
>> +m4_include([m4/python_version.m4])
>> +m4_include([m4/python_devel.m4])
>> +m4_include([m4/udev.m4])
>> +m4_include([m4/ocaml.m4])
>> +m4_include([m4/default_lib.m4])
>> +m4_include([m4/set_cflags_ldflags.m4])
>> +
>> +# Enable/disable options
>> +AX_ARG_ENABLE_AND_EXPORT([xsm],
>> + Â Â[Enable XSM security module (by default, Flask)])
>
> Does this macro include support for reading from config.cache as well as
> the cmdline option?

They weren't but I've added support and they are cached now (provided
that you run configure with --config-cache).

>
> [...]
>> +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.

>
> [...]
>> +
>> +# Checks for header files.
>> +AC_FUNC_ALLOCA
>> +AC_CHECK_HEADERS([ \
>> + Â Â Â Â Â Â Â Âarpa/inet.h fcntl.h inttypes.h libintl.h limits.h malloc.h \
>> + Â Â Â Â Â Â Â Ânetdb.h netinet/in.h stddef.h stdint.h stdlib.h string.h \
>> + Â Â Â Â Â Â Â Âstrings.h sys/file.h sys/ioctl.h sys/mount.h sys/param.h \
>> + Â Â Â Â Â Â Â Âsys/socket.h sys/statvfs.h sys/time.h syslog.h termios.h \
>> + Â Â Â Â Â Â Â Âunistd.h yajl/yajl_version.h \
>> + Â Â Â Â Â Â Â Â])
>> +
>> +# Checks for typedefs, structures, and compiler characteristics.
>> +AC_HEADER_STDBOOL
>> +AC_TYPE_UID_T
>> +AC_C_INLINE
>> +AC_TYPE_INT16_T
>> +AC_TYPE_INT32_T
>> +AC_TYPE_INT64_T
>> +AC_TYPE_INT8_T
>
> 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.

>
>> +AC_TYPE_MODE_T
>> +AC_TYPE_OFF_T
>> +AC_TYPE_PID_T
>> +AC_C_RESTRICT
>> +AC_TYPE_SIZE_T
>> +AC_TYPE_SSIZE_T
>
> 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.

>
>> +AC_CHECK_MEMBERS([struct stat.st_blksize])
>> +AC_STRUCT_ST_BLOCKS
>> +AC_CHECK_MEMBERS([struct stat.st_rdev])
>> +AC_TYPE_UINT16_T
>> +AC_TYPE_UINT32_T
>> +AC_TYPE_UINT64_T
>> +AC_TYPE_UINT8_T
>> +AC_CHECK_TYPES([ptrdiff_t])
>> +
>> +# Checks for library functions.
>> +AC_FUNC_ERROR_AT_LINE
>> +AC_FUNC_FORK
>> +AC_FUNC_FSEEKO
>> +AC_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK
>> +AC_HEADER_MAJOR
>> +AC_FUNC_MALLOC
>> +AC_FUNC_MKTIME
>> +AC_FUNC_MMAP
>> +AC_FUNC_REALLOC
>> +AC_FUNC_STRNLEN
>> +AC_FUNC_STRTOD
>> +AC_CHECK_FUNCS([ \
>> + Â Â Â Â Â Â Â Âalarm atexit bzero clock_gettime dup2 fdatasync ftruncate \
>> + Â Â Â Â Â Â Â Âgetcwd gethostbyname gethostname getpagesize gettimeofday \
>> + Â Â Â Â Â Â Â Âinet_ntoa isascii localtime_r memchr memmove memset mkdir \
>> + Â Â Â Â Â Â Â Âmkfifo munmap pathconf realpath regcomp rmdir select setenv 
>> \
>> + Â Â Â Â Â Â Â Âsocket strcasecmp strchr strcspn strdup strerror strndup \
>> + Â Â Â Â Â Â Â Âstrpbrk strrchr strspn strstr strtol strtoul strtoull tzset 
>> \
>> + Â Â Â Â Â Â Â Âuname \
>> + Â Â Â Â Â Â Â Â])
>> +
>> +AC_OUTPUT()
>> diff -r 4086e4811547 -r e12ec1071410 m4/default_lib.m4
>> --- /dev/null  Thu Jan 01 00:00:00 1970 +0000
>> +++ b/m4/default_lib.m4 Sat Jan 07 04:17:10 2012 +0100
>> @@ -0,0 +1,7 @@
>> +AC_DEFUN([AX_DEFAULT_LIB],
>> +[AS_IF([test -d "$prefix/lib64"], [
>> + Â ÂLIB_PATH="lib64"
>> +],[
>> + Â ÂLIB_PATH="lib"
>> +])
>> +AC_SUBST(LIB_PATH)])
>
> Does this get exposed via config.cache and/or the command line? It
> should be.
>
> Did you write m4/* from scratch or did they come from some snippet
> library?

The only one that comes from a library is ocaml.m4, the other ones are
hand-written. Most are translations of tools/check/check_*, and the
rest are a couple of handful macros that avoid repeating code blocks
all over.

>
> 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.

>
>> \ No newline at end of file
>
> Might this newline be significant in a macro expansion library? I think
> it's good practice to include it anyway (here and elsewhere).

Yup, I've forgot.

>
> [...]
>> 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/

>
> [...]
>
> Ian.
>

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.