[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



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.

> 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:
> 
> +    # automake -a

We aren't using automake though? Perhaps this explains your problem with
things alwyas trying to use Makefile.am?

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

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

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

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

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

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

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

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

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

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

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

Isn't the management of these snippets the sort of thing aclocal does
for you?

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

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

[...]

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