Re: [Xen-devel] [PATCH v5 11/14] autoconf: xen: move standard variables to a generic place

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-05-20 at 05:31 -0700, Luis R. Rodriguez wrote:
> diff --git a/README b/README
> index 9bbe734..079e6a9 100644
> --- a/README
> +++ b/README
> @@ -129,6 +129,22 @@ performed with root privileges.]
>     versions of those scripts, so that you can copy the dist directory
>     to another machine and install from that distribution.
>  
> +Xen system configuration
> +========================
> +
> +Xen uses a set of variables for system configuration and upon build time,

s/upon/at/

> +because of this these variables are defined on a top level general input
> +source and are generated after running ./configure. There are two versions
> +of the input files, one for scripts (bash, python) and another for C / header
> +files:
> +
> +  * config/xen-environment-header.in - used to generate 
> config/xen-environment-header 
> +  * config/xen-environment-scripts.in - used to generate 
> config/xen-environment-scripts
> +
> +Further documentation can be found on the wiki:

This all seems very developer focused (why would users care about
this?). I think most of this would be better as a comment in the
relevant files.

> +
> +http://wiki.xen.org/wiki/Category:Host_Configuration#System_wide_xen_configuration

This might be useful if it were more user focused in particular if it
referred to the installed locations of things and not the source which
users don't care about.

> +
>  Python Runtime Libraries
>  ========================
>  
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index c6439f6..273f1e8 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -1,3 +1,17 @@
> +# These are standard defaults which you can use to avoid having
> +# to run ./configure -- you can use this to compile the hypervisor
> +# and the mini os:
> +#
> +# make xen
> +# sudo make -C xen install
> +#
> +# make -C extras/mini-os
> +include $(XEN_ROOT)/config/defaults.mk
> +
> +# This comes from running configure and will override
> +# the defaults.
> +-include $(XEN_ROOT)/config/Toplevel.mk

As I said in the reply to the subthread started by Jan I don't think any
of this belongs here.

> new file mode 100644
> index 0000000..7dd7a53
> --- /dev/null
> +++ b/config/xen-environment-header.in
> @@ -0,0 +1,13 @@
> +#define SBINDIR "@SBINDIR@"
> +#define BINDIR "@BINDIR@"
> +#define LIBEXEC "@LIBEXEC@"
> +#define LIBDIR "@LIBDIR@"
> +#define SHAREDIR "@SHAREDIR@"
> +
> +#define PRIVATE_BINDIR "@PRIVATE_BINDIR@"
> +#define XENFIRMWAREDIR "@XENFIRMWAREDIR@"
> +#define XEN_CONFIG_DIR "@XEN_CONFIG_DIR@"
> +#define XEN_SCRIPT_DIR "@XEN_SCRIPT_DIR@"
> +#define XEN_LOCK_DIR "@XEN_LOCK_DIR@"
> +#define XEN_RUN_DIR "@XEN_RUN_DIR@"
> +#define XEN_PAGING_DIR "@XEN_PAGING_DIR@"
> diff --git a/config/xen-environment-scripts.in 
> b/config/xen-environment-scripts.in
> new file mode 100644
> index 0000000..9623231
> --- /dev/null
> +++ b/config/xen-environment-scripts.in
> @@ -0,0 +1,15 @@
> +SBINDIR="@SBINDIR@"
> +BINDIR="@BINDIR@"
> +LIBEXEC="@LIBEXEC@"
> +LIBDIR="@LIBDIR@"
> +SHAREDIR="@SHAREDIR@"
> +
> +PRIVATE_BINDIR="@PRIVATE_BINDIR@"
> +XENFIRMWAREDIR="@XENFIRMWAREDIR@"
> +XEN_CONFIG_DIR="@XEN_CONFIG_DIR@"
> +XEN_SCRIPT_DIR="@XEN_SCRIPT_DIR@"
> +XEN_LOCK_DIR="@XEN_LOCK_DIR@"
> +XEN_LOG_DIR="@XEN_LOG_DIR@"
> +XEN_LIB_STORED="@XEN_LIB_STORED@"
> +XEN_RUN_DIR="@XEN_RUN_DIR@"
> +XEN_PAGING_DIR="@XEN_PAGING_DIR@"

The downside of these is that we now need to keep both in sync. I think
a reasonable compromise would be to generate the scripts version of this
via autoconf and create the C version using a mechanism similar to the
existing buildmakevars2file etc stuff.

> diff --git a/m4/expand_config.m4 b/m4/expand_config.m4

expand_config is a strange name for this, paths.m4 would be more
descriptive, wouldn't it?

Or this sort of thing wouldn't be out of place in confgure.ac I think.

> new file mode 100644
> index 0000000..717fcd1
> --- /dev/null
> +++ b/m4/expand_config.m4
> @@ -0,0 +1,61 @@
> +AC_DEFUN([AX_XEN_EXPAND_CONFIG], [
> +dnl expand these early so we can use this for substitutions
> +test "x$prefix" = "xNONE" && prefix=$ac_default_prefix
> +test "x$exec_prefix" = "xNONE" && exec_prefix=$ac_default_prefix
> +
> +BINDIR=$prefix/bin
> +AC_SUBST(BINDIR)
> +
> +SBINDIR=$prefix/sbin
> +AC_SUBST(SBINDIR)

TBH I thought that after going through all this fuss --bindir and
--sbindir might worl.


> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index c41de27..f0f624a 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -48,18 +48,19 @@ TARGET_LDFLAGS += -nostdlib 
> -L$(CROSS_PREFIX)/$(GNU_TARGET_ARCH)-xen-elf/lib
>  
>  TARGETS=$(STUBDOM_TARGETS)
>  
> +STUBDOMPATH="stubdompath.sh"
> +
> +$(STUBDOMPATH): $(XEN_ROOT)/config/xen-environment-scripts
> +     @cp $(XEN_ROOT)/config/xen-environment-scripts $@

        cp $< $@

Not sure it needs hiding from the user either.

(there are a few of these)

> diff --git a/stubdom/configure.ac b/stubdom/configure.ac
> index 6468203..d6b0fbf 100644
> --- a/stubdom/configure.ac
> +++ b/stubdom/configure.ac
> @@ -16,6 +16,9 @@ m4_include([../m4/features.m4])
>  m4_include([../m4/path_or_fail.m4])
>  m4_include([../m4/depends.m4])
>  m4_include([../m4/fetcher.m4])
> +m4_include([../m4/expand_config.m4])
> +
> +AX_XEN_EXPAND_CONFIG()

Doesn't only the toplevel configure.ac need this, because you've put the
things into config/Toplevel.mk (soon to be config/Paths.mk).

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel