Libguestfs - May 2015 - [PATCH 1/2] mllib: remove spurious check_SCRIPTS from Makefile.am

Not actually useful, as TESTS defines the tests, and it breaks when
oUnit is not available (as it tries to build an oUnit-based unit test).
---
 mllib/Makefile.am | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/mllib/Makefile.am b/mllib/Makefile.am
index 0b43684..e363f27 100644
--- a/mllib/Makefile.am
+++ b/mllib/Makefile.am
@@ -132,9 +132,6 @@ libdir.ml: Makefile
 
 # Tests.
 
-check_SCRIPTS = \
-	common_utils_tests
-
 if HAVE_OCAMLOPT
 common_utils_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS)
 common_utils_tests: config.cmx common_gettext.cmx common_utils.cmx
common_utils_tests.cmx
-- 
2.1.0

Instead of parsing the version, try to look for the OUnit2 module within
the oUnit package.

Followup of commit 46bc79109bb51cfb656863da3635f1c8da5f6412.
---
 configure.ac | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5ff151b..7a609cb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1135,6 +1135,7 @@ AS_IF([test "x$OCAMLC" != "xno"],[
 OCAML_PKG_gettext=no
 OCAML_PKG_libvirt=no
 OCAML_PKG_oUnit=no
+ounit_is_v2=no
 AS_IF([test "x$OCAMLC" != "xno"],[
     # Create mllib/common_gettext.ml, gettext functions or stubs.
 
@@ -1147,17 +1148,9 @@ AS_IF([test "x$OCAMLC" != "xno"],[
     AC_CHECK_OCAML_PKG(libvirt)
     AC_CHECK_OCAML_PKG(oUnit)
 
-    # oUnit >= 2 is required.  If it's not this version, discard.
+    # oUnit >= 2 is required, so check that it has OUnit2.
     if test "x$OCAML_PKG_oUnit" != "xno"; then
-        AC_MSG_CHECKING([for oUnit version 2 or above])
-        ounit_version=`$OCAMLFIND query oUnit -format '%v'`
-        ounit_version_major=`echo $ounit_version | $AWK -F. '{print
$1}'`
-        if test $ounit_version_major -ge 2; then
-            AC_MSG_RESULT([yes])
-        else
-            AC_MSG_RESULT([no])
-            OCAML_PKG_oUnit=no
-        fi
+        AC_CHECK_OCAML_MODULE(ounit_is_v2,[OUnit.OUnit2],OUnit2,[+oUnit])
     fi
 ])
 AM_CONDITIONAL([HAVE_OCAML_PKG_GETTEXT],
@@ -1165,7 +1158,7 @@ AM_CONDITIONAL([HAVE_OCAML_PKG_GETTEXT],
 AM_CONDITIONAL([HAVE_OCAML_PKG_LIBVIRT],
     [test "x$OCAMLC" != "xno" && test
"x$OCAMLFIND" != "xno" && test
"x$OCAML_PKG_libvirt" != "xno"])
 AM_CONDITIONAL([HAVE_OCAML_PKG_OUNIT],
-    [test "x$OCAMLC" != "xno" && test
"x$OCAMLFIND" != "xno" && test
"x$OCAML_PKG_oUnit" != "xno"])
+    [test "x$OCAMLC" != "xno" && test
"x$OCAMLFIND" != "xno" && test
"x$OCAML_PKG_oUnit" != "xno" && test
"x$ounit_is_v2" != "xno"])
 
 AC_CHECK_PROG([OCAML_GETTEXT],[ocaml-gettext],[ocaml-gettext],[no])
 AM_CONDITIONAL([HAVE_OCAML_GETTEXT],
-- 
2.1.0

On Tue, May 12, 2015 at 03:20:56PM +0200, Pino Toscano
wrote:
> Not actually useful, as TESTS defines the tests, and it breaks when
> oUnit is not available (as it tries to build an oUnit-based unit test).
> ---
>  mllib/Makefile.am | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/mllib/Makefile.am b/mllib/Makefile.am
> index 0b43684..e363f27 100644
> --- a/mllib/Makefile.am
> +++ b/mllib/Makefile.am
> @@ -132,9 +132,6 @@ libdir.ml: Makefile
>  
>  # Tests.
>  
> -check_SCRIPTS = \
> -	common_utils_tests
> -
>  if HAVE_OCAMLOPT
>  common_utils_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS)
>  common_utils_tests: config.cmx common_gettext.cmx common_utils.cmx
common_utils_tests.cmx
> -- 
I don't think this is spurious (although it doesn't do very much).
Automake wants us to list all built scripts in a check_SCRIPTS rule,
of which this is one.  See section 9.1 Executable Scripts in the
automake info file.

It probably does need to be changed to:

check_SCRIPTS if HAVE_OCAML_PKG_OUNIT
check_SCRIPTS += common_utils_tests
endif

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Tue, May 12, 2015 at 03:20:57PM +0200, Pino Toscano
wrote:
> Instead of parsing the version, try to look for the OUnit2 module within
> the oUnit package.
> 
> Followup of commit 46bc79109bb51cfb656863da3635f1c8da5f6412.
> ---
>  configure.ac | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 5ff151b..7a609cb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1135,6 +1135,7 @@ AS_IF([test "x$OCAMLC" !=
"xno"],[
>  OCAML_PKG_gettext=no
>  OCAML_PKG_libvirt=no
>  OCAML_PKG_oUnit=no
> +ounit_is_v2=no
>  AS_IF([test "x$OCAMLC" != "xno"],[
>      # Create mllib/common_gettext.ml, gettext functions or stubs.
>  
> @@ -1147,17 +1148,9 @@ AS_IF([test "x$OCAMLC" !=
"xno"],[
>      AC_CHECK_OCAML_PKG(libvirt)
>      AC_CHECK_OCAML_PKG(oUnit)
>  
> -    # oUnit >= 2 is required.  If it's not this version, discard.
> +    # oUnit >= 2 is required, so check that it has OUnit2.
>      if test "x$OCAML_PKG_oUnit" != "xno"; then
> -        AC_MSG_CHECKING([for oUnit version 2 or above])
> -        ounit_version=`$OCAMLFIND query oUnit -format '%v'`
> -        ounit_version_major=`echo $ounit_version | $AWK -F. '{print
$1}'`
> -        if test $ounit_version_major -ge 2; then
> -            AC_MSG_RESULT([yes])
> -        else
> -            AC_MSG_RESULT([no])
> -            OCAML_PKG_oUnit=no
> -        fi
> +        AC_CHECK_OCAML_MODULE(ounit_is_v2,[OUnit.OUnit2],OUnit2,[+oUnit])
>      fi
>  ])
>  AM_CONDITIONAL([HAVE_OCAML_PKG_GETTEXT],
> @@ -1165,7 +1158,7 @@ AM_CONDITIONAL([HAVE_OCAML_PKG_GETTEXT],
>  AM_CONDITIONAL([HAVE_OCAML_PKG_LIBVIRT],
>      [test "x$OCAMLC" != "xno" && test
"x$OCAMLFIND" != "xno" && test
"x$OCAML_PKG_libvirt" != "xno"])
>  AM_CONDITIONAL([HAVE_OCAML_PKG_OUNIT],
> -    [test "x$OCAMLC" != "xno" && test
"x$OCAMLFIND" != "xno" && test
"x$OCAML_PKG_oUnit" != "xno"])
> +    [test "x$OCAMLC" != "xno" && test
"x$OCAMLFIND" != "xno" && test
"x$OCAML_PKG_oUnit" != "xno" && test
"x$ounit_is_v2" != "xno"])
>  
>  AC_CHECK_PROG([OCAML_GETTEXT],[ocaml-gettext],[ocaml-gettext],[no])
>  AM_CONDITIONAL([HAVE_OCAML_GETTEXT],
This one is OK, ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Tuesday 12 May 2015 19:54:32 Richard W.M. Jones
wrote:
> On Tue, May 12, 2015 at 03:20:56PM +0200, Pino Toscano wrote:
> > Not actually useful, as TESTS defines the tests, and it breaks when
> > oUnit is not available (as it tries to build an oUnit-based unit
test).
> > ---
> >  mllib/Makefile.am | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/mllib/Makefile.am b/mllib/Makefile.am
> > index 0b43684..e363f27 100644
> > --- a/mllib/Makefile.am
> > +++ b/mllib/Makefile.am
> > @@ -132,9 +132,6 @@ libdir.ml: Makefile
> >  
> >  # Tests.
> >  
> > -check_SCRIPTS = \
> > -	common_utils_tests
> > -
> >  if HAVE_OCAMLOPT
> >  common_utils_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS)
> >  common_utils_tests: config.cmx common_gettext.cmx common_utils.cmx
common_utils_tests.cmx
> 
> I don't think this is spurious (although it doesn't do very much).
> Automake wants us to list all built scripts in a check_SCRIPTS rule,
> of which this is one.  See section 9.1 Executable Scripts in the
> automake info file.
This is somehow confusing, as check_SCRIPTS and check_PROGRAMS seem to
be defined as the scripts/programs needed by the tests (those in TESTS),
although it says they can be added to TESTS...
Add to that that common_utils_tests is not really a script (like a
shell/Python/etc would be), and it's somehow that due to not being built
with automake.

I've just sent a couple of patches which, in the end, link the two tests
in mllib using automake.
> It probably does need to be changed to:
> 
> check_SCRIPTS > if HAVE_OCAML_PKG_OUNIT
> check_SCRIPTS += common_utils_tests
> endif
Right, although with my patches this is now check_PROGRAMS.

Thanks,
-- 
Pino Toscano