Libguestfs - Mar 2020 - Re: [PATCH libnbd 3/3] build: Test new pkgconf file.

For these simple compile tests where we want to ensure that a basic
external program could be compiled using libnbd, we shouldn't include
<config.h>.  This is because we want to test here that <libnbd.h>
can
stand alone, without needing anything defined by the GNU autotools
infrastructure.

Of course we can use <config.h> in other test programs where we aren't
evaluating if <libnbd.h> works standalone.
---
 tests/compile-ansi-c.c | 2 --
 tests/compile-cxx.cpp  | 2 --
 tests/compile.c        | 2 --
 3 files changed, 6 deletions(-)

diff --git a/tests/compile-ansi-c.c b/tests/compile-ansi-c.c
index 0d48618..82e1976 100644
--- a/tests/compile-ansi-c.c
+++ b/tests/compile-ansi-c.c
@@ -20,8 +20,6 @@
  * compiler.
  */
 
-#include <config.h>
-
 #include <stdio.h>
 #include <stdlib.h>
 
diff --git a/tests/compile-cxx.cpp b/tests/compile-cxx.cpp
index 4c8447c..f24d409 100644
--- a/tests/compile-cxx.cpp
+++ b/tests/compile-cxx.cpp
@@ -22,8 +22,6 @@
 #error "this test should be compiled with a C++ compiler"
 #endif
 
-#include <config.h>
-
 #include <iostream>
 #include <cstdlib>
 
diff --git a/tests/compile.c b/tests/compile.c
index d7ef98c..b23cf81 100644
--- a/tests/compile.c
+++ b/tests/compile.c
@@ -18,8 +18,6 @@
 
 /* Compile, open and close a handle. */
 
-#include <config.h>
-
 #include <stdio.h>
 #include <stdlib.h>
 
-- 
2.25.0

We use a similar trick to libvirt and libguestfs to allow external C
programs that use libnbd to be compiled against the built (but not
installed) libnbd with:

  ../libnbd/run ./configure
  make

What actually happens is we have a second pkg-config file
(lib/local/libnbd.pc) which points to the locally built libnbd.  The
./run script sets up PKG_CONFIG_PATH to point to this directory.
Assuming that ./configure is using pkg-config/pkgconf and not some
other half-baked solution it will pick up the libnbd.pc file from here
which will set CFLAGS and LIBS appropriately.

See also this libguestfs commit:
https://github.com/libguestfs/libguestfs/commit/e33b3c83a02c1d7e82018105166e451373e8c218
---
 .gitignore             |  1 +
 configure.ac           |  1 +
 lib/Makefile.am        |  1 +
 lib/local/libnbd.pc.in | 36 ++++++++++++++++++++++++++++++++++++
 run.in                 | 12 +++++++++++-
 5 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index ec54b53..777471b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -90,6 +90,7 @@ Makefile.in
 /lib/api.c
 /lib/libnbd.pc
 /lib/libnbd.syms
+/lib/local/libnbd.pc
 /lib/states-run.c
 /lib/states.c
 /lib/states.h
diff --git a/configure.ac b/configure.ac
index 5e9bac4..9fd284b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -427,6 +427,7 @@ AC_CONFIG_FILES([Makefile
                  interop/Makefile
                  lib/Makefile
                  lib/libnbd.pc
+                 lib/local/libnbd.pc
                  ocaml/Makefile
                  ocaml/META
                  ocaml/examples/Makefile
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 1c46c54..4f5360d 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -28,6 +28,7 @@ generator_built = \
 EXTRA_DIST = \
 	$(generator_built) \
 	libnbd.syms \
+	local/libnbd.pc.in \
 	$(NULL)
 
 lib_LTLIBRARIES = libnbd.la
diff --git a/lib/local/libnbd.pc.in b/lib/local/libnbd.pc.in
new file mode 100644
index 0000000..6d0d568
--- /dev/null
+++ b/lib/local/libnbd.pc.in
@@ -0,0 +1,36 @@
+# nbd client library in userspace
+# @configure_input@
+# Copyright (C) 2013-2020 Red Hat Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# Dummy pkg-config file which is used to allow out of tree packages to
+# be configured against the libnbd tree without libnbd needing to be
+# installed.
+#
+# Note if you are using the ./run script then you don't need to worry
+# about this because the script sets PKG_CONFIG_PATH correctly.
+
+prefix=@abs_top_builddir@
+exec_prefix=@abs_top_builddir@
+libdir=@abs_top_builddir@/lib/.libs
+includedir=@abs_top_srcdir@/include
+
+Name: @PACKAGE_NAME@
+Version: @PACKAGE_VERSION@
+Description: NBD client library in userspace
+Requires:
+Cflags: -I${includedir}
+Libs: -L${libdir} -lnbd
diff --git a/run.in b/run.in
index 599752d..a7a2850 100755
--- a/run.in
+++ b/run.in
@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 # libnbd 'run' programs locally script
-# Copyright (C) 2011-2019 Red Hat Inc.
+# Copyright (C) 2011-2020 Red Hat Inc.
 #
 # @configure_input@
 #
@@ -35,6 +35,12 @@
 #   >>> import nbd    # locally-compiled nbd module
 #
 # This works for any C program and most non-C bindings.
+#
+# You can also compile other programs against this uninstalled libnbd
+# tree if those programs are using pkgconf/pkg-config:
+#
+#   ../libnbd/run ./configure
+#   make
 
 #----------------------------------------------------------------------
 
@@ -78,6 +84,10 @@ export MALLOC_CHECK_=1
 random_val="$(@AWK@ 'BEGIN{srand(); print 1+int(255*rand())}' <
/dev/null)"
 export MALLOC_PERTURB_=$random_val
 
+# Allow dependent packages to be compiled against local libnbd.
+prepend PKG_CONFIG_PATH "$b/lib/local"
+export PKG_CONFIG_PATH
+
 # Do we have libtool?  If we have it then we can use it to make
 # running valgrind simpler.  However don't depend on it.
 if libtool --help >/dev/null 2>&1; then
-- 
2.25.0

Unfortunately only tests the ./run version rather than the real one.
---
 .gitignore          |  1 +
 configure.ac        |  2 ++
 tests/Makefile.am   |  4 +++-
 tests/pkgconf.sh.in | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/.gitignore b/.gitignore
index 777471b..dea9274 100644
--- a/.gitignore
+++ b/.gitignore
@@ -159,6 +159,7 @@ Makefile.in
 /tests/keys.psk
 /tests/meta-base-allocation
 /tests/oldstyle
+/tests/pkgconf.sh
 /tests/pki/
 /tests/read-only-flag
 /tests/read-write-flag
diff --git a/configure.ac b/configure.ac
index 9fd284b..0200394 100644
--- a/configure.ac
+++ b/configure.ac
@@ -414,6 +414,8 @@ AC_CONFIG_FILES([run],
                 [chmod +x,-w run])
 AC_CONFIG_FILES([sh/nbdsh],
                 [chmod +x,-w sh/nbdsh])
+AC_CONFIG_FILES([tests/pkgconf.sh],
+                [chmod +x,-w tests/pkgconf.sh])
 
 AC_CONFIG_FILES([Makefile
                  bash/Makefile
diff --git a/tests/Makefile.am b/tests/Makefile.am
index c720988..f0cbc2e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1,5 +1,5 @@
 # nbd client library in userspace
-# Copyright (C) 2013-2019 Red Hat Inc.
+# Copyright (C) 2013-2020 Red Hat Inc.
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -36,6 +36,7 @@ EXTRA_DIST = \
 	functions.sh.in \
 	make-pki.sh \
 	meta-base-allocation.sh \
+	pkgconf.sh.in \
 	synch-parallel.sh \
 	synch-parallel-tls.sh \
 	$(NULL)
@@ -73,6 +74,7 @@ TESTS += \
 	debug-environment \
 	version \
 	export-name \
+	pkgconf.sh \
 	$(NULL)
 
 # Even though we have a compile.c, we do not want make to create a
'compile'
diff --git a/tests/pkgconf.sh.in b/tests/pkgconf.sh.in
new file mode 100755
index 0000000..14b513b
--- /dev/null
+++ b/tests/pkgconf.sh.in
@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# @configure_input@
+# Copyright (C) 2019-2020 Red Hat Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# This is unfortunately only testing the local .pc file (in lib/local)
+# used by the ./run script, rather than the real .pc file (in lib).
+# It's hard to test the latter. XXX
+
+source ./functions.sh
+
+set -x
+set -e
+
+requires pkgconf --version
+
+aout=compile-pkgconf
+cleanup_fn rm -f $aout
+
+@CC@ @CFLAGS@ $(pkgconf libnbd --cflags) \
+    @srcdir@/compile.c \
+    -o $aout $(pkgconf libnbd --libs)
+
+./$aout
-- 
2.25.0

On Thursday, 12 March 2020 23:46:57 CET Richard W.M. Jones
wrote:
> Unfortunately only tests the ./run version rather than the real one.
> ---
Is it worth it though? The new pkg-config file won't be deployed to
users, and it should (hopefully) not break if the locations of the
sources are not changed.

Otherwise a small hint: rename it to e.g. local-pkg-config.sh to
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index c720988..f0cbc2e 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -1,5 +1,5 @@
>  # nbd client library in userspace
> -# Copyright (C) 2013-2019 Red Hat Inc.
> +# Copyright (C) 2013-2020 Red Hat Inc.
>  #
>  # This library is free software; you can redistribute it and/or
>  # modify it under the terms of the GNU Lesser General Public
> @@ -36,6 +36,7 @@ EXTRA_DIST = \
>  	functions.sh.in \
>  	make-pki.sh \
>  	meta-base-allocation.sh \
> +	pkgconf.sh.in \
As AC_CONFIG_FILES has tests/pkgconf.sh, the .in file is
included automatically by autotools in the dist tarballs.
Hence, this line is extra.
> diff --git a/tests/pkgconf.sh.in b/tests/pkgconf.sh.in
> new file mode 100755
> index 0000000..14b513b
> --- /dev/null
> +++ b/tests/pkgconf.sh.in
> @@ -0,0 +1,38 @@
> +#!/usr/bin/env bash
> +# nbd client library in userspace
> +# @configure_input@
> +# Copyright (C) 2019-2020 Red Hat Inc.
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2 of the License, or (at your option) any later version.
> +#
> +# This library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
> +
> +# This is unfortunately only testing the local .pc file (in lib/local)
> +# used by the ./run script, rather than the real .pc file (in lib).
> +# It's hard to test the latter. XXX
> +
> +source ./functions.sh
> +
> +set -x
> +set -e
> +
> +requires pkgconf --version
Better use pkg-config, so it works with both pkg-config & pkgconf.
> +
> +aout=compile-pkgconf
> +cleanup_fn rm -f $aout
> +
> +@CC@ @CFLAGS@ $(pkgconf libnbd --cflags) \
@CFLAGS@ are the ones you use to build libnbd with, not the ones
applications using libnbd need to use (otherwise they would be in
Cflags & Libs in the pkg-config file).
> +    @srcdir@/compile.c \
> +    -o $aout $(pkgconf libnbd --libs)
> +
> +./$aout
> 
Should this test test all the tests/compile*.{c,cpp} sources?

-- 
Pino Toscano

On Thursday, 12 March 2020 23:46:56 CET Richard W.M. Jones
wrote:
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 1c46c54..4f5360d 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -28,6 +28,7 @@ generator_built = \
>  EXTRA_DIST = \
>  	$(generator_built) \
>  	libnbd.syms \
> +	local/libnbd.pc.in \
As AC_CONFIG_FILES has lib/local/libnbd.pc, the .in file is
included automatically by autotools in the dist tarballs.
Hence, this line is extra.

The rest LGTM.

Thanks,
-- 
Pino Toscano

On Thursday, 12 March 2020 23:46:55 CET Richard W.M. Jones
wrote:
> For these simple compile tests where we want to ensure that a basic
> external program could be compiled using libnbd, we shouldn't include
> <config.h>.  This is because we want to test here that
<libnbd.h> can
> stand alone, without needing anything defined by the GNU autotools
> infrastructure.
> 
> Of course we can use <config.h> in other test programs where we
aren't
> evaluating if <libnbd.h> works standalone.
> ---
LGTM.

-- 
Pino Toscano