varnish-cache icon indicating copy to clipboard operation
varnish-cache copied to clipboard

support for externally built vtest with libvtc_varnish.so and libvtc_builtwith.so

Open nigoroll opened this issue 4 months ago • 3 comments

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 https://github.com/vtest/VTest2/pull/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

https://github.com/vtest/VTest2/pull/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.

nigoroll avatar Oct 15 '25 13:10 nigoroll

I noticed that I was making the life of reviewers unnecessarily hard by not providing at least a draft of a version without the submodule. So here it is:

https://github.com/nigoroll/varnish-cache/tree/vtest_ext_varnish_nosubmodule

nigoroll avatar Oct 16 '25 07:10 nigoroll

bugwash feedback: avoid the extra vtc_builtwith.so. Two options: move cmd_builtwith to vtc_varnish.so as suggested as an alternative above, or move into varnishd for use with -x builtwith .... phk wants to look into the latter

nigoroll avatar Oct 20 '25 13:10 nigoroll

It seems that my explanation of this PR was not good enough, or too long, or whatever, but apparently it is not clear what this PR does and doesn't do:

  • https://github.com/vtest/VTest2/pull/7 provides an extension mechanism to VTest2. The vtest binary coming out of it knows nothing about varnish:
$ /usr/bin/vtest tests/b00000.vtc | grep -- ----
---- top   Unknown command: "varnish"
  • In "external vtest mode" (if no submodule is detected via the probe for vtc_main.c), this PR builds libvtc_varnish.so, which provides the varnish specific commands:
$ /usr/bin/vtest -i -E ./.libs/libvtc_varnish.so tests/b00000.vtc 
#    top  TEST tests/b00000.vtc passed (2.231)

The varnishtest provided with this PR is just an exec wrapper.

edit: I wrongly wrote "fork/exec" wrapper. varnishtest is just an exec wrapper.

nigoroll avatar Oct 21 '25 08:10 nigoroll