Xen devel - Mar 2012 - [PATCH] tools/configure: correct enable/disable-feature option

# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1331748796 -3600
# Node ID 36690251f6bfe153f390ef200ff286e64f2582d7
# Parent  cd473b1fb313bb107cf1c32ce224f265a5de097e
tools/configure: correct enable/disable-feature option

If --disable-feature is not specified then feature should default to n.
But with the current code disable with set feature to y, and enable will
set feature to n. Reverse the logic in the two .m4 files to use correct
default values.
The failure currently is that default := is y while it should be n.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

diff -r cd473b1fb313 -r 36690251f6bf tools/m4/disable_feature.m4
--- a/tools/m4/disable_feature.m4
+++ b/tools/m4/disable_feature.m4
@@ -7,7 +7,7 @@ AS_IF([test "x$enable_$1" = "xno"], [
 ], [test "x$enable_$1" = "xyes"], [
     ax_cv_$1="y"
 ], [test -z $ax_cv_$1], [
-    ax_cv_$1="y"
+    ax_cv_$1="n"
 ])
 $1=$ax_cv_$1
 AC_SUBST($1)])
diff -r cd473b1fb313 -r 36690251f6bf tools/m4/enable_feature.m4
--- a/tools/m4/enable_feature.m4
+++ b/tools/m4/enable_feature.m4
@@ -7,7 +7,7 @@ AS_IF([test "x$enable_$1" = "xyes"], [
 ], [test "x$enable_$1" = "xno"], [
     ax_cv_$1="n"
 ], [test -z $ax_cv_$1], [
-    ax_cv_$1="n"
+    ax_cv_$1="y"
 ])
 $1=$ax_cv_$1
 AC_SUBST($1)])

On Wed, 2012-03-14 at 18:13 +0000, Olaf Hering wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1331748796 -3600
> # Node ID 36690251f6bfe153f390ef200ff286e64f2582d7
> # Parent  cd473b1fb313bb107cf1c32ce224f265a5de097e
> tools/configure: correct enable/disable-feature option
> 
> If --disable-feature is not specified then feature should default to n.
Maybe I''m misunderstanding what you are saying. I''d have
though that it
would depend on the feature whether it was on by default or not and in
other cases it will depend on whether the prerequisites are met.
> But with the current code disable with set feature to y, and enable will
> set feature to n. Reverse the logic in the two .m4 files to use correct
> default values.
> The failure currently is that default := is y while it should be n.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r cd473b1fb313 -r 36690251f6bf tools/m4/disable_feature.m4
> --- a/tools/m4/disable_feature.m4
> +++ b/tools/m4/disable_feature.m4
> @@ -7,7 +7,7 @@ AS_IF([test "x$enable_$1" = "xno"], [
>  ], [test "x$enable_$1" = "xyes"], [
>      ax_cv_$1="y"
>  ], [test -z $ax_cv_$1], [
> -    ax_cv_$1="y"
> +    ax_cv_$1="n"
>  ])
>  $1=$ax_cv_$1
>  AC_SUBST($1)])
> diff -r cd473b1fb313 -r 36690251f6bf tools/m4/enable_feature.m4
> --- a/tools/m4/enable_feature.m4
> +++ b/tools/m4/enable_feature.m4
> @@ -7,7 +7,7 @@ AS_IF([test "x$enable_$1" = "xyes"], [
>  ], [test "x$enable_$1" = "xno"], [
>      ax_cv_$1="n"
>  ], [test -z $ax_cv_$1], [
> -    ax_cv_$1="n"
> +    ax_cv_$1="y"
>  ])
>  $1=$ax_cv_$1
>  AC_SUBST($1)])
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On Wed, Mar 14, Ian Campbell wrote:
> On Wed, 2012-03-14 at 18:13 +0000, Olaf Hering wrote:
> > # HG changeset patch
> > # User Olaf Hering <olaf@aepfle.de>
> > # Date 1331748796 -3600
> > # Node ID 36690251f6bfe153f390ef200ff286e64f2582d7
> > # Parent  cd473b1fb313bb107cf1c32ce224f265a5de097e
> > tools/configure: correct enable/disable-feature option
> > 
> > If --disable-feature is not specified then feature should default to
n.
> 
> Maybe I''m misunderstanding what you are saying. I''d have
though that it
> would depend on the feature whether it was on by default or not and in
> other cases it will depend on whether the prerequisites are met.
For example, --disable-debug is supposed to be the default, so that
debug becomes n. But with this code from configure it will become y, at
least for me (enable_debug is unset, becomes "set" so
ax_cv_debug="y":

# Check whether --enable-debug was given.
if test "${enable_debug+set}" = set; then :
  enableval=$enable_debug;
fi


if test "x$enable_debug" = "xno"; then :

    ax_cv_debug="n"

elif test "x$enable_debug" = "xyes"; then :

    ax_cv_debug="y"

elif test -z $ax_cv_debug; then :

    ax_cv_debug="y"

fi
debug=$ax_cv_debug


> > But with the current code disable with set feature to y, and enable
will
> > set feature to n. Reverse the logic in the two .m4 files to use
correct
> > default values.
> > The failure currently is that default := is y while it should be n.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > 
> > diff -r cd473b1fb313 -r 36690251f6bf tools/m4/disable_feature.m4
> > --- a/tools/m4/disable_feature.m4
> > +++ b/tools/m4/disable_feature.m4
> > @@ -7,7 +7,7 @@ AS_IF([test "x$enable_$1" =
"xno"], [
> >  ], [test "x$enable_$1" = "xyes"], [
> >      ax_cv_$1="y"
> >  ], [test -z $ax_cv_$1], [
> > -    ax_cv_$1="y"
> > +    ax_cv_$1="n"
> >  ])
> >  $1=$ax_cv_$1
> >  AC_SUBST($1)])
> > diff -r cd473b1fb313 -r 36690251f6bf tools/m4/enable_feature.m4
> > --- a/tools/m4/enable_feature.m4
> > +++ b/tools/m4/enable_feature.m4
> > @@ -7,7 +7,7 @@ AS_IF([test "x$enable_$1" =
"xyes"], [
> >  ], [test "x$enable_$1" = "xno"], [
> >      ax_cv_$1="n"
> >  ], [test -z $ax_cv_$1], [
> > -    ax_cv_$1="n"
> > +    ax_cv_$1="y"
> >  ])
> >  $1=$ax_cv_$1
> >  AC_SUBST($1)])
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 
>

On Wed, 2012-03-14 at 19:33 +0000, Olaf Hering wrote:
> On Wed, Mar 14, Ian Campbell wrote:
> 
> > On Wed, 2012-03-14 at 18:13 +0000, Olaf Hering wrote:
> > > # HG changeset patch
> > > # User Olaf Hering <olaf@aepfle.de>
> > > # Date 1331748796 -3600
> > > # Node ID 36690251f6bfe153f390ef200ff286e64f2582d7
> > > # Parent  cd473b1fb313bb107cf1c32ce224f265a5de097e
> > > tools/configure: correct enable/disable-feature option
> > > 
> > > If --disable-feature is not specified then feature should default
to n.
> > 
> > Maybe I''m misunderstanding what you are saying. I''d
have though that it
> > would depend on the feature whether it was on by default or not and in
> > other cases it will depend on whether the prerequisites are met.
> 
> For example, --disable-debug is supposed to be the default,
Right, what was missing from my understanding is that you were patching
a pair of macros one of which enables by default and supplies
--disable-foo and the other vice versa. I presume the right macro is
used depending on the type of the argument.

The original description would have been clearer if it has been:
        If AX_ARG_DISABLE_AND_EXPORT is used and --disable-feature is
        not specified then feature should default to n.
or so. Although now that I read it like that it seems that the macro
names are pretty confusing, since AX_ARG_DISABLE_AND_EXPORT appears to
mean "option is enabled by default so provide --disable-feature".

On that basis I think your change may be wrong -- it looks like the
intention was to enable debug by default and AX_ARG_DISABLE_AND_EXPORT
does that. I''ve CC''d Roger for his input.

I think those macros are very confusingly named.
AX_ARG_DEFAULT_{ENABLE,DISABLE} (or AX_ARG_DEFAULT(name,{yes|no}) might
be clearer? The _AND_EXPORT seems a bit unnecessary to me.

Ian.

On Wed, Mar 14, Ian Campbell wrote:
> On that basis I think your change may be wrong -- it looks like the
> intention was to enable debug by default and AX_ARG_DISABLE_AND_EXPORT
> does that. I''ve CC''d Roger for his input.
I think you are right about that one.

Olaf