This PR adds the option to use an externally built vtest binary as agreed in #4398.

Demo

Install VTest2 with extension support

• Build and install the ext branch from Add extension support vtest/VTest2#7 such that vtest is found in $PATH (the example here is intended to show a "canonical" way as with external packaging, I would not normally install unter /usr):

slink@haggis21:~/Devel/varnish-git/VTest2 (ext)$ sudo make install PREFIX=/usr
umask 022 && \
mkdir -p /usr/bin /usr/include/vtest /usr/lib/pkgconfig && \
cp vtest /usr/bin && chmod 755 /usr/bin/vtest && \
cp src/vtc.h src/cmds.h lib/vdef.h lib/miniobj.h lib/vas.h lib/vsb.h /usr/include/vtest && \
cp vtest.pc /usr/lib/pkgconfig && \
chmod 644 /usr/include/vtest/*.h /usr/lib/pkgconfig/vtest.pc

quick check:

slink@haggis21:~/Devel/varnish-git/VTest2 (ext)$ type vtest
vtest is hashed (/usr/bin/vtest)
slink@haggis21:~/Devel/varnish-git/VTest2 (ext)$ vtest -h 2>&1 | grep exten
    -E extension.so              # Load extenstion
slink@haggis21:~/Devel/varnish-git/VTest2 (ext)$ pkg-config --cflags vtest
-I/usr/include/vtest 

Build vinyl-cache in out-of-tree vtest mode"

This can be later be done by not cloning the submodule (because the TODO items are not complete, see below), but for now we simulate a missing submodule by deleting the file which we look for to determine if the submodule is present:

slink@haggis21:~/Devel/varnish-git/varnish-cache (vtest_ext_varnish)$ rm bin/varnishtest/vtest2/src/vtc_main.c 

Now build as always, e.g.

./autogen.sh && ./configure && make -j40 check

All tests should succeed

Implementation / Explanation

vtest/VTest2#7 adds to VTest the option to load a shared object at runtime, which can then extend vtest functionality by registering additional commands.

Moving vinyl test functionality to libvtc_varnish.so

The first commit changes the build to create such a shared object with the vinyl-specific test mechanics in vtc_varnish.c, vtc_vsm.c and vtc_logexp.c.

With this, calling vtest -E libvtc_varnish.so gives us a the same functionality we had before with the static build.

To keep the rest of the build simple, a wrapper program called ... (you guessed it) ... varnishtest is added which simply calls vtest with the extension argument. This is used in favor of the simple shell script 2-liner which could also easily fulfill the task in order to be able to get the libtool wrapper script for uninstalled libraries. This in turn requires linking to an empty dummy library.

Moving compile-time flag checks / macros to libvtc_builtwith

varnishtest already works for the most part with just the above, but we have just broken much of the feature functionality and the pkg_version and pkg_branch macros: These can only work if the respective code is built together with the code base it is supposed to reflect on - otherwise we get behavior based on however vtest was compiled, but not how vinyl-cache was compiled.

So we apply the same idea again and move these checks to a vtest extension with we do compile each time. The outcome is a working varnishtest as demonstrated.

Alternatives considered

We could do without the extra libvtc_builtwith and just move the respective code into libvtc_varnish, but I think this functionality might also be of general interest to other projects and I would add this as an example to VTest2 itself if we decide that we want to go this route.

The builtwith checks could also be replaced with compile time configuration of the tests to execute. I actually have working code which moves all the -spersistent checks to a subdirectory of test and includes these tests only if --with-persistent-storage is given to configure.

But in particular with negated tests I think this quickly becomes really tedious, and the information which tests to run is no longer nicely contained in the tests itself (unless we add something like magic comments which we grep for...). All in all, the "feature test" way is much clearer and easier to maintain.

In general, another alternative would be to invert the sense of what is an executable and what is a library: Create a libvtest.so in the VTest2 project and then an executable in vinyl which only has minimal code to call the library and extend it. In my mind this simply seemed like the wrong way around and I found the extension model very clear, but opinions might vary and at this point, without having written the actual code, I would think that the alternative should also work.

TODOs

A main TODO item is that that switch to the new model is not complete, and deliberately so:

For one, I think that keeping vtest bundled as a submodule will make the life for most of the developers easier. The problem to solve is to enable a build from git with an external vtest, and I see no strong reason to destroy the existing workflow.

Secondly, the vinyl specific code still needs to be checked into the vinyl-cache repo again and removed from Vtest2.

Thirdly, the feature tests which have now been added to builtwith need to be removed from the former.

But all of this only makes sense after the changes are accepted.

Notes

make dist still requires the submodule, and rightly so: Again, this addition is to support builds from git only, not to destroy previous workflows.