[PATCH libnbd 1/3] tests: Don't use <config.h> in simple compile tests.
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/e33b3c83a02c1d7e82018105166e... --- .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
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
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 Fri, Mar 13, 2020 at 12:57:52PM +0100, Pino Toscano wrote:
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
I think I'll drop this patch. I was kind of hoping that by posting it someone would think of a way to test the real .pc file :-) Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On 3/14/20 9:44 AM, Richard W.M. Jones wrote:
On Fri, Mar 13, 2020 at 12:57:52PM +0100, Pino Toscano wrote:
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
I think I'll drop this patch. I was kind of hoping that by posting it someone would think of a way to test the real .pc file :-)
Automake produces 'make check' (test the just-built uninstalled binaries) and 'make installcheck' (test the installed binaries). If there is an easy way to set up the test environment so that we use ./run during 'make check' but avoid it during 'make installcheck', then anything you do to test the local-pkg-config during 'make check' will immediately turn into a test of the real .pc file during 'make installcheck'. However, the way we currently do it, by globally setting LOG_COMPILER in tests/Makefile.am, does not distinguish between the two setups, so it would need a tweak, and I don't know offhand what that tweak would look like. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
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
participants (3)
-
Eric Blake -
Pino Toscano -
Richard W.M. Jones