| Message ID | 20170725124825.1936-2-paul.kocialkowski@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I like this patch, however there's a mistake in it: On Tue, 2017-07-25 at 15:48 +0300, Paul Kocialkowski wrote: > Since the chamelium is not a very usual piece of hardware and > requires > pulling-in lots of specific dependencies, it makes sense to keep it > disabled by default. > > An explicit --enable-chamelium argument is provided to enable it when > necessary. This also leads to more predictable results than > automatically enabling it when its dependencies are met. > > Finally, whether chamelium support is enabled is printed by the > information summary at end of the configure run. > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com> > --- > configure.ac | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/configure.ac b/configure.ac > index a6ab9e4a..17a226de 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -185,23 +185,23 @@ PKG_CHECK_MODULES(GSL, [gsl], [gsl=yes], > [gsl=no]) > AM_CONDITIONAL(HAVE_GSL, [test "x$gsl" = xyes]) > > # for chamelium > -AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium], > - [Enable building of chamelium libraries and tests > (default: yes)]), > - [enable_chamelium=no], [enable_chamelium=yes]) > +AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--enable-chamelium], > + [Enable building of chamelium libraries and tests > (default: no)]), > + [enable_chamelium=yes], [enable_chamelium=no]) It appears you repeated my original mistake of switching the yes and no parameters by accident :P. This causes passing --disable-chamelium to result in enabling the chamelium tests instead of disabling them. > AM_CONDITIONAL(HAVE_CHAMELIUM, [test "x$enable_chamelium" = xyes]) > if test "x$enable_chamelium" = xyes; then > PKG_CHECK_MODULES(XMLRPC, xmlrpc xmlrpc_util xmlrpc_client, > [], > - [AC_MSG_ERROR([Failed to find xmlrpc, > required by chamelium. Use --disable-chamelium to disable chamelium > support.])]) > + [AC_MSG_ERROR([Failed to find xmlrpc, > required by chamelium.])]) > PKG_CHECK_MODULES(PIXMAN, pixman-1, [], > - [AC_MSG_ERROR([Failed to find pixman, > required by chamelium. Use --disable-chamelium to disable chamelium > support.])]) > + [AC_MSG_ERROR([Failed to find pixman, > required by chamelium.])]) > if test x"$udev" != xyes; then > - AC_MSG_ERROR([Failed to find udev, required by > chamelium. Use --disable-chamelium to disable chamelium support.]) > + AC_MSG_ERROR([Failed to find udev, required by > chamelium.]) > fi > if test x"$glib" != xyes; then > - AC_MSG_ERROR([Failed to find glib, required by > chamelium. Use --disable-chamelium to disable chamelium support.]) > + AC_MSG_ERROR([Failed to find glib, required by > chamelium.]) > fi > if test x"$gsl" != xyes; then > - AC_MSG_ERROR([Failed to find gsl, required by > chamelium. Use --disable-chamelium to disable chamelium support.]) > + AC_MSG_ERROR([Failed to find gsl, required by > chamelium.]) > fi > > AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support]) > @@ -404,6 +404,7 @@ echo "Intel GPU tools" > echo "" > echo " • Tests:" > echo " Build tests : ${BUILD_TESTS}" > +echo " Chamelium tests : ${enable_chamelium}" > echo " Compile prime tests: ${NOUVEAU}" > echo " Print stack traces : ${with_libunwind}" > echo " Debug flags : ${DEBUG_CFLAGS}"
On Tue, 2017-07-25 at 13:07 -0400, Lyude Paul wrote: > I like this patch, however there's a mistake in it: > > On Tue, 2017-07-25 at 15:48 +0300, Paul Kocialkowski wrote: > > Since the chamelium is not a very usual piece of hardware and > > requires > > pulling-in lots of specific dependencies, it makes sense to keep it > > disabled by default. > > > > An explicit --enable-chamelium argument is provided to enable it > > when > > necessary. This also leads to more predictable results than > > automatically enabling it when its dependencies are met. > > > > Finally, whether chamelium support is enabled is printed by the > > information summary at end of the configure run. > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com> > > --- > > configure.ac | 17 +++++++++-------- > > 1 file changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/configure.ac b/configure.ac > > index a6ab9e4a..17a226de 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -185,23 +185,23 @@ PKG_CHECK_MODULES(GSL, [gsl], [gsl=yes], > > [gsl=no]) > > AM_CONDITIONAL(HAVE_GSL, [test "x$gsl" = xyes]) > > > > # for chamelium > > -AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium], > > - [Enable building of chamelium libraries and tests > > (default: yes)]), > > - [enable_chamelium=no], [enable_chamelium=yes]) > > +AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--enable-chamelium], > > + [Enable building of chamelium libraries and tests > > (default: no)]), > > + [enable_chamelium=yes], [enable_chamelium=no]) > > It appears you repeated my original mistake of switching the yes and > no > parameters by accident :P. This causes passing --disable-chamelium to > result in enabling the chamelium tests instead of disabling them. Well, the order is correct since the option is --enable-chamelium and not --disable-chamelium anymore. However, that made me realize that it will still enable it if --disable-chamelium is passed because I'm not checking enableval. Will fix this in v2! > > AM_CONDITIONAL(HAVE_CHAMELIUM, [test "x$enable_chamelium" = xyes]) > > if test "x$enable_chamelium" = xyes; then > > PKG_CHECK_MODULES(XMLRPC, xmlrpc xmlrpc_util xmlrpc_client, > > [], > > - [AC_MSG_ERROR([Failed to find xmlrpc, > > required by chamelium. Use --disable-chamelium to disable chamelium > > support.])]) > > + [AC_MSG_ERROR([Failed to find xmlrpc, > > required by chamelium.])]) > > PKG_CHECK_MODULES(PIXMAN, pixman-1, [], > > - [AC_MSG_ERROR([Failed to find pixman, > > required by chamelium. Use --disable-chamelium to disable chamelium > > support.])]) > > + [AC_MSG_ERROR([Failed to find pixman, > > required by chamelium.])]) > > if test x"$udev" != xyes; then > > - AC_MSG_ERROR([Failed to find udev, required by > > chamelium. Use --disable-chamelium to disable chamelium support.]) > > + AC_MSG_ERROR([Failed to find udev, required by > > chamelium.]) > > fi > > if test x"$glib" != xyes; then > > - AC_MSG_ERROR([Failed to find glib, required by > > chamelium. Use --disable-chamelium to disable chamelium support.]) > > + AC_MSG_ERROR([Failed to find glib, required by > > chamelium.]) > > fi > > if test x"$gsl" != xyes; then > > - AC_MSG_ERROR([Failed to find gsl, required by > > chamelium. Use --disable-chamelium to disable chamelium support.]) > > + AC_MSG_ERROR([Failed to find gsl, required by > > chamelium.]) > > fi > > > > AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support]) > > @@ -404,6 +404,7 @@ echo "Intel GPU tools" > > echo "" > > echo " • Tests:" > > echo " Build tests : ${BUILD_TESTS}" > > +echo " Chamelium tests : ${enable_chamelium}" > > echo " Compile prime tests: ${NOUVEAU}" > > echo " Print stack traces : ${with_libunwind}" > > echo " Debug flags : ${DEBUG_CFLAGS}"
diff --git a/configure.ac b/configure.ac index a6ab9e4a..17a226de 100644 --- a/configure.ac +++ b/configure.ac @@ -185,23 +185,23 @@ PKG_CHECK_MODULES(GSL, [gsl], [gsl=yes], [gsl=no]) AM_CONDITIONAL(HAVE_GSL, [test "x$gsl" = xyes]) # for chamelium -AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--disable-chamelium], - [Enable building of chamelium libraries and tests (default: yes)]), - [enable_chamelium=no], [enable_chamelium=yes]) +AC_ARG_ENABLE(chamelium, AS_HELP_STRING([--enable-chamelium], + [Enable building of chamelium libraries and tests (default: no)]), + [enable_chamelium=yes], [enable_chamelium=no]) AM_CONDITIONAL(HAVE_CHAMELIUM, [test "x$enable_chamelium" = xyes]) if test "x$enable_chamelium" = xyes; then PKG_CHECK_MODULES(XMLRPC, xmlrpc xmlrpc_util xmlrpc_client, [], - [AC_MSG_ERROR([Failed to find xmlrpc, required by chamelium. Use --disable-chamelium to disable chamelium support.])]) + [AC_MSG_ERROR([Failed to find xmlrpc, required by chamelium.])]) PKG_CHECK_MODULES(PIXMAN, pixman-1, [], - [AC_MSG_ERROR([Failed to find pixman, required by chamelium. Use --disable-chamelium to disable chamelium support.])]) + [AC_MSG_ERROR([Failed to find pixman, required by chamelium.])]) if test x"$udev" != xyes; then - AC_MSG_ERROR([Failed to find udev, required by chamelium. Use --disable-chamelium to disable chamelium support.]) + AC_MSG_ERROR([Failed to find udev, required by chamelium.]) fi if test x"$glib" != xyes; then - AC_MSG_ERROR([Failed to find glib, required by chamelium. Use --disable-chamelium to disable chamelium support.]) + AC_MSG_ERROR([Failed to find glib, required by chamelium.]) fi if test x"$gsl" != xyes; then - AC_MSG_ERROR([Failed to find gsl, required by chamelium. Use --disable-chamelium to disable chamelium support.]) + AC_MSG_ERROR([Failed to find gsl, required by chamelium.]) fi AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support]) @@ -404,6 +404,7 @@ echo "Intel GPU tools" echo "" echo " • Tests:" echo " Build tests : ${BUILD_TESTS}" +echo " Chamelium tests : ${enable_chamelium}" echo " Compile prime tests: ${NOUVEAU}" echo " Print stack traces : ${with_libunwind}" echo " Debug flags : ${DEBUG_CFLAGS}"
Since the chamelium is not a very usual piece of hardware and requires pulling-in lots of specific dependencies, it makes sense to keep it disabled by default. An explicit --enable-chamelium argument is provided to enable it when necessary. This also leads to more predictable results than automatically enabling it when its dependencies are met. Finally, whether chamelium support is enabled is printed by the information summary at end of the configure run. Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com> --- configure.ac | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)