On 06/27/2016 10:49 PM, Andreas Tille wrote: > On Mon, Jun 27, 2016 at 07:49:53PM +0200, Christian Seiler wrote: >> - you build tool/ropy, but you never use it? Neither in the test suite >> (dh_auto_test doesn't do anything) nor do you package that - why? > > Since I wanted to get things working somehow first. I've now reated a > libdisorder-tools package (will write a ropy.1 as well). If you could > help with a sensible test suite - I have no idea how to get this working > with autoconf.
Well, autoconf in and by itself doesn't support testing, automake does, which fortunately you're also using. I've added a very trivial test from the way I understand how the program you're using works, and attached an updated patch to this email. (I've also updated the autoconf/automake prerequisites, see my last email.) It's actually quite trivial: write a shell script that returns successfully (exit code 0) or not. (You can run a C program directly if it does return 0 or not depending on the test results, but that doesn't apply to ropy directly.) My test applies ropy to the COPYING file containing the text of the GPL and checks if the result matches the expectations. Feel free to extend/change that to your liking, but as a basic functionality check I think that is already useful. Note that adding it to automake will automatically have "make check" available and then dh_auto_test will call that (unless DEB_BUILD_OPTIONS contains nocheck), so it's automatically integrated into the package build without additional things for you to do. You can also use more complicated test harnesses like TAP or DejaGNU, but that would be overkill here IMHO. Appropriate documentation: https://www.gnu.org/software/automake/manual/html_node/Tests.html (+ subpages of that) Beware though that if "make check" fails, the package build will also fail by default. (I consider that to be a good thing, just something to keep in mind.) I did some test builds on amd64, i386, x32, and arm64, and the simple test I created succeeds on all of these platforms. >> - you could consider installing a pkg-config file for the library, >> as it requires linking against -lm. While this works automatically >> with the shared library, this is not the case for the static >> library, where you explicitly have to add -lm after libdisorder.a >> when linking against it > > I have no specific reason not to use pkg-config. If you recommend this > I'd happily apply a patch. Well, in general I would consider pkg-config to be good thing for libraries, but I'm not sure here, especially if the circle of consumers is a very small circle and they don't appear to be using pkg-config so far anyway, so it's probably a waste of time here. >> - you install everything into the proper multiarch locations, but >> libdisorder0 and libdisorder-dev are not Multi-Arch: same, even >> though they easily could be > > How? Well, simply add 'Multi-Arch: same' to both packages. M-A: same implies that the same package can be installed for multiple architectures at the same time, so you have to take care that - architecture-specific stuff is installed in /usr/lib/$triplet (or /usr/include/$triplet for include files that are different on different architectures) - the rest (e.g. generic includes that are non-architecture dependent and hence in /usr/include directly) is identical regardless of the architecture a package is built for In your package, that's already the case, so you can just add the header to debian/control. (Don't add it to the new -tools package you created though, because that's not M-A: same.) Relevant documentation: https://wiki.ubuntu.com/MultiarchSpec https://wiki.debian.org/Multiarch >> - you don't build the test program (although it doesn't serve as >> a unit test, because it will never fail unless /dev/urandom is >> not accessible, so there's no actual check whether the routine >> works as expected or not - so I don't think that's a problem >> really, just wanted to make you aware of it) > > Well, upstream provides a test and I would like to run it - if I would > know how to do this. Any patch is welcome. I've looked at the upstream test binary btw., and it's an endless loop that processes /dev/urandom - not something you want as a unit test. But see above for something that is a useful unit test. >> - this is more of an upstream issue (i.e. not related to your >> packaging): the library uses global state and is non-reentrant; >> which is actually not a great thing for a shared library to be > > I guess upstream is MIA since a long time. > >> Unfortunately, improving that would require breaking both API >> and ABI, so I'm not suggesting to you to do so, but I would >> maybe talk with upstream about improving that in the future. > > I can forward this but my guess is that upstream will not change > anything. It would also spoil my primary goal for the libraries that > are using libdisorder. Yes, sure. That's why I said you could maybe mention it to upstream, but you should definitely not change this yourself. Another thing I noticed: your configure.ac checks for a C++ compiler, which is unnecessary here, as this is a pure C library. You can simply drop the C++ checks and save computing resources. I've done that in the updated autoconf.patch I've attached (that also contains the unit test). Anyway: in case you want to know more about autoconf/automake etc. I would really recommend <https://autotools.io/index.html> in addition to the official documentation of that ecosystem ([1], [2], [3], [4]). Regards, Christian [1] https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/index.html [2] https://www.gnu.org/software/automake/manual/html_node/index.html [3] https://www.gnu.org/software/libtool/manual/html_node/index.html [4] https://people.freedesktop.org/~dbn/pkg-config-guide.html
Author: Andreas Tille <ti...@debian.org> Last-Update: Wed, 22 Jun 2016 16:27:46 +0200 Description: Add autoconf stuff to enable simple library creation --- /dev/null +++ b/Makefile.am @@ -0,0 +1,16 @@ +lib_LTLIBRARIES = libdisorder.la +libdisorder_la_SOURCES = src/disorder.c +libdisorder_la_LDFLAGS = -version-info @LIB_VERSION@ +libdisorder_la_CPPFLAGS = -Iinclude +libdisorder_la_LIBADD = -lm + +bin_PROGRAMS = ropy +ropy_SOURCES = tool/ropy.c +ropy_LDADD = libdisorder.la + +include_HEADERS = include/disorder.h +man_MANS = man/shannon_H.3 + +TESTS = test/gpl.sh +TEST_EXTENSIONS = .sh +SH_LOG_COMPILER = /bin/sh --- /dev/null +++ b/configure.ac @@ -0,0 +1,47 @@ +# -*- Autoconf -*- +# Process this file with autoconf to produce a configure script. + +AC_INIT(disorder, 0.0.2, mich...@freshdefense.net) +AC_CONFIG_HEADERS([config.h]) + +AC_PREREQ(2.64) + +# Directory that contains install-sh and other auxiliary files +AC_CONFIG_AUX_DIR([config]) + +################################################################################ +# According to (http://www.mail-archive.com/autoconf@gnu.org/msg14232.html) +# this macro should be after AC_INIT but before AM_INIT_AUTOMAKE +################################################################################ +AC_CONFIG_MACRO_DIR(config) + +AM_INIT_AUTOMAKE([1.11 parallel-tests foreign subdir-objects dist-zip tar-ustar filename-length-max=299]) + +LIB_VERSION=0:0 + +AC_SUBST([VERSION]) +AC_SUBST([LIB_VERSION]) + +AC_SUBST([VERSION]) + +# Checks for programs. +AC_PROG_LN_S +AC_PROG_INSTALL + +LT_PREREQ(2.2) +LT_INIT + +CPPFLAGS="-I\$(top_srcdir) $CPPFLAGS" +# Checks for libraries. + +# Checks for header files. +AC_HEADER_STDC +AC_CHECK_HEADERS([stdlib.h]) + +AC_PROG_MAKE_SET + +AC_CONFIG_FILES([ + Makefile + ]) +AC_OUTPUT + --- /dev/null +++ b/test/gpl.sh @@ -0,0 +1,18 @@ +#!/bin/sh + +EXPECTED="tokens: 78 entropy: 4.737054 metric: 0.000259 maxent: 6.285402 ratio: 0.753660" +ACTUAL="$(./ropy COPYING)" + +if [ x"$EXPECTED" = x"$ACTUAL" ] ; then + echo "Entropy of COPYING matches expected value." + exit 0 +else + echo "Test failure:" + echo " expected output:" + echo -n " " + echo "$EXPECTED" + echo " actual output:" + echo -n " " + echo "$ACTUAL" + exit 1 +fi
