Skip to content
This repository was archived by the owner on Feb 16, 2026. It is now read-only.

support for externally built vtest with libvtc_varnish.so and libvtc_builtwith.so - #4406

Open
nigoroll wants to merge 578 commits into
varnishcache:masterfrom
nigoroll:vtest_ext_varnish
Open

nigoroll wants to merge 578 commits into
varnishcache:masterfrom
nigoroll:vtest_ext_varnish

Conversation

@nigoroll

@nigoroll nigoroll commented Oct 15, 2025

Copy link
Copy Markdown
Member

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.

@nigoroll
nigoroll force-pushed the vtest_ext_varnish branch 2 times, most recently from 95f62b3 to b155963 Compare October 16, 2025 06:10
@nigoroll nigoroll changed the title WIP support for externally built vtest with vtc_varnish as a shared object support for externally built vtest with vtc_varnish as a shared object Oct 16, 2025
@nigoroll nigoroll changed the title support for externally built vtest with vtc_varnish as a shared object support for externally built vtest with libvtc_varnish.so and libvtc_builtwith.so Oct 16, 2025
@nigoroll
nigoroll marked this pull request as ready for review October 16, 2025 06:46
@nigoroll

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member Author

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

nigoroll commented Oct 21, 2025

Copy link
Copy Markdown
Member Author

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:

$ /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 and others added 21 commits June 2, 2026 11:21
Ref 5f2e844

I should have noticed this earlier
To avoid duplication, inverse mode is revisited. Capturing the exit
status of vinyltest conflicts with 'set -e', so it is disabled in
run mode.

Better diff with the --ignore-all-space option.

Committer edit: Vinylized

 Conflicts:
	tools/vtc-bisect.sh
It is possible to start a git-bisect with a dirty working copy, and in
the absence of a conflict git will generally do a good job of preserving
the uncommitted changes across checkouts. An alternative could be to
apply a patch at each step, but the chances of a patch that does not
always apply are higher.
To ensure that an mktemp failure will interrupt the script.
On BSD, the reliance on the limited recv buffer seems to not work

**** v1    vsl|       1001 ReqAcct         c 52 0 52 173 116736 116909
While, technically, a close in v1f_read() is correct as long as we always use
blocking I/O, having the close in this place caused irritation as to the
correct error handling:

For the case of errors unrelated to I/O (for example, parse errors), we need to
properly prevent backend connection reuse anyway, so why would we treat a zero
length read differently?

Calling code should check for error cases (or, in the case of v1f_eof_pull(),
normal EOF) and prevent backend connection recycling.

Except for the case of v1f_eof_pull(), htc->doclose was actually overridden
later.

Ref https://code.vinyl-cache.org/vinyl-cache/vinyl-cache/pulls/4508#issuecomment-61603

I checked all VFP_Suck() code paths and convinced myself that we do in fact
properly close the connection, either directly by setting bo->htc->doclose in
vbf_stp_fetchbody(), or indirectly by setting req->req_body_status = BS_ERROR in
vrb_pull(). The other VFP_Suck() invocations are recursive.
found by Flexelint
This is extremely useful for testing new features with all of the test cases.

Usage example:

	VTEST_VINYL_VCL_PREPEND='import debug;' vinyltest ....
VTEST_VINYL_VCL_PREPEND='import debug;' on b00045.vtc exposed that we let
multiple managers modify the working directory even if vinyld is already
running. Being racy, the symptoms differed slightly across runs (usually one of
the two AZ(unlink(...)) cases hitting), but generally looked like this:

***  v1    debug|Assert error in mgt_vcl_del(), mgt/mgt_vcl.c line 278:
***  v1    debug|  Condition((unlink(vf->fname)) == 0) not true.
***  v1    debug|  errno = 2 (No such file or directory)

We now move the create_pid_file() calls right after creation of the workdir,
such that the "vinyld is already running" case is hit before any modifications
are done on the working directory.

C_flag support *remains* incomplete in that regard, in that it will still modify
the working directory if -n is given explicitly. This probably calls for a
warning.
Now that we moved the pidfile check earlier, it became apparent that this test
case was racy in that new instances on the same work directory were started when
the old ones were still shutting down.

https://code.vinyl-cache.org/vinyl-cache/vinyl-cache/actions/runs/521/jobs/0/attempt/1

**** top   shell_out|Error: vinyld is already running (pid=2346) (pidfile=/tmp/vtc.2331.5bba46af/v1/_.pid)
**** top   shell_out|(-? gives usage)
**** top   shell_status = 0x00ff
---- top   shell_expect not found: ("Error: -sdebug conflicting options")
**** dT    1.087
*    top   RESETTING after ../../../../bin/vinyltest/tests/c00030.vtc
*    top   TEST ../../../../bin/vinyltest/tests/c00030.vtc FAILED

Ref 88c13af
nigoroll and others added 29 commits September 7, 2026 10:52
It served no purpose.

Fixes #4566
Restructured so that:

* 'Upgrading' is limited to work that has to be done to upgrade from
  a current deployment to the new version.

* 'Changes' is a comprehensive, user-level description of changes and
  new features.
pkglibdir == $(libdir)/vinyl-cache is used as the base for vmoddir, but
otherwise we do not install anything there any more since we switched our
internal libraries to static (see 932035b, for
example). vmoddir has its own pkg-config variable. Our libraries go to libdir
directly

To be sure, I installed 6dc1d5b when
VARNISH_LIBRARY_PATH got introduced, and at the time we _did_ install libraries
in pkglibdir. But even then did we install the api.so directly in libdir.

So this is really just something we overlooked to remove when we went static.
We are going to need control over when exactly it runs
It allows to set the name of the project to build for (== api implementation
package name)

Use --with-vcache=vanilla to seach for vanillaapi.pc
…e flavors

For example, if a VMOD prefers Vinyl Cache with a version of 9.0.0 or greater,
but also supports Foo Cache with a version between 1.0.0 and 2.0.0 (inclusive),
it can use this in configure.ac:

VCACHE_REQUIRE(
        [[vinyl], [9.0.0]],
        [[foo], [1.0.0], [2.0.0]],
)

Users can override the preference with --with-vcache. For example, if both vinyl
and foo are installed, but the user prefers foo, they can use --with-vcache=foo.

Users can also override to build against a vcache flavor which is not declared
by the vmod by using --with-vcache=iknowbetter (iknowbetter obivously being the
name of the flavor, which has iknowbetterapi.pc installed).
Except for VINYL_PREREQ, which is replaced by VCACHE_REQUIRE.

Also no aliases for the _VINYL macros, which are not for use outside our tree
Ideally, we would go all the way and make vtest external, stop building
vinyltest etc, but I only want to interfere with what phk might still have in
the works to the extent absolutely necessary.

Ref https://code.vinyl-cache.org/vtest/VTest2/pulls/13

Note that because I did not want to build another binary (vtest), if our
(vinyltest) program is used, we will overwrite the logexpect, vsm and vinyl
commands with those from the library. The code is the same, so this should not
make a difference, but I still wanted to mention it just in case.
This would constitute a conflicting file between alternative flavors, which I
wanted to avoid.

Add VTESTEXT to .pc and vinyl.m4

VMODs should use

	VTC_LOG_COMPILER = vtest -E@VTESTEXT@ ...
Ref a1f53a0

I have not understood this uninstalled.pc thing.
(see also Vtest2 d5b49e00a06c16d807170e9ba8e0617ed765246a)

It would have been nice to maintain them in a repo shared by multiple projects,
but a) are they tightly bound to to vinyl code base and b) do they need to be
linked to vinyl private libraries, and if we no longer want the VTest2 codebase
in the vinyl tree.

Unfortunately, recreating their commit history would also make the commit
history of Vinyl Cache look really weird, so please refer to the VTest2 repo for
the history up to the given commit.

Note: Keeping VTEST_WITH_* for now to be able to still build a vinyltest with
vinyl support not in an extension for the coming release. But these should go
away as soon as we have established the vtest + extension way.
... which vtest now gained in order to allow version checks with an external
vtest
this is in preparation of upcoming changes:

- move vinyltest specific CPPFLAGS to vinyltest_CPPFLAGS
- move common CFLAGS to AM_CFLAGS
- define addlibs variable to be used in _LDADD and _LIBADD
- split common EXTRA_DIST and additions for vinyltest
- introduce VTEST_CPPFLAGS (this is going to reappear) for
  CPPFLAGS needed by extensions
- move LOCs around to more clearly seperate vinyltest from the
  extension
If it looks like the git submodule for VTest2 was not available, check an
external vtest's version and, if it is ok, use it.

We now have two vinyltest modes:

If the submodule is present, we build it as before.

If the submodule is not present, we only build the vtest vinyl extension, but
also install a vinyltest wrapper. This is to avoid breaking VMODs which use
vinyltest and have not yet switched to vtest + extention (see #4537)

There are two follow-up steps to complete the transition:

- Stop building vtest and remove the submodule
- Remove the wrapper and use vtest -Elibvtest_ext_vinyl.so

The vinyltest wrapper is build for libtool mechanics: If it just passed the
installed path of libvtest_ext_vinyl.so to -E (ending up in a fixed path
dlopen()), even "make check" would expect the installed library. So instead, we
pass just the base name to -E and rely on dlopen() LD_LIBRARY_PATH mechanics: If
called via the libtool wrapper shell script, vtest will find the extension in
the source tree first. Otherwise it will find it in the LD_LIBRARY_PATH which we
set to the libdir of our install.
... in the vtest_vinyl extension

As we no longer compile vtest with every build, we can no longer use code in
vtest core to inform tests about compile time settings.

Ref varnishcache#4398
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants