[master] c8c016d8a Add VMOD version information to Panic output, debug.vmod and ELF

Nils Goroll nils.goroll at uplex.de
Mon Jan 20 14:46:05 UTC 2025


commit c8c016d8abdc9144279fefa2b411e7ebda8af118
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Thu Mar 7 14:54:19 2024 +0100

    Add VMOD version information to Panic output, debug.vmod and ELF
    
    This commit adds a facility which I had wanted for a very long time, and should
    have added much earlier: Analyzing bug reports with VMODs involved used to be
    complicated by the fact that they typically did not contain reliable version
    information.
    
    We add to vmodtool.py the generic code to create version information from
    generate.py with minor modifications. It adds the version set for autotools and,
    if possible, the git revision to the vmodtool-generated interface code. We copy
    that to struct vrt and output it for panics and the debug.vmod CLI command. For
    builds from distributions, save the git revision to a file.
    
    Being at it, we polish the panic output slightly for readability.
    
    There are two elements to the version information:
    
    - "version", which is intended to be user-friendly and
    
    - "vcs" (for version control system) which is intended to represent a commit id
      from the scm.
    
    The VCC file Stanza $Version overrides "version".
    
    As an example, the output from a panic now might look like this:
    
    vmods = {
      debug = {p=0x7f5b19c2c600, abi=\"Varnish trunk 172e7b43a49bdf43d82cc6b10ab08362939fc8a5\", vrt=19.1,
        vcs=\"172e7b43a49bdf43d82cc6b10ab08362939fc8a5\", version=\"Varnish trunk\"},
    },
    
    The debug.vmod CLI outout is, for example:
    
        1 vtc (path="/home/slink/Devel/varnish-git/varnish-cache/vmod/.libs/libvmod_vtc.so", version="Varnish trunk", vcs="172e7b43a49bdf43d82cc6b10ab08362939fc8a5")
    
    And, finally, a readelf example to extract the vcs string:
    
     $ readelf -p.vmod_vcs ./vmod/.libs/libvmod_std.so
    
     String dump of section '.vmod_vcs':
       [     0]  172e7b43a49bdf43d82cc6b10ab08362939fc8a5

diff --git a/.gitignore b/.gitignore
index a6eb65683..fe61bb233 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ cscope.*out
 /vmod/vcc_*_if.c
 /vmod/vcc_*_if.h
 /vmod/vmod_*.rst
+/vmod/vmod_vcs_version.txt
 
 # Man-files and binaries
 /man/*.1
diff --git a/bin/varnishd/cache/cache_vrt_vmod.c b/bin/varnishd/cache/cache_vrt_vmod.c
index c1fa22070..3b5e7c564 100644
--- a/bin/varnishd/cache/cache_vrt_vmod.c
+++ b/bin/varnishd/cache/cache_vrt_vmod.c
@@ -65,6 +65,8 @@ struct vmod {
 	const char		*abi;
 	unsigned		vrt_major;
 	unsigned		vrt_minor;
+	const char		*vcs;
+	const char		*version;
 };
 
 static VTAILQ_HEAD(,vmod)	vmods = VTAILQ_HEAD_INITIALIZER(vmods);
@@ -147,6 +149,8 @@ VPI_Vmod_Init(VRT_CTX, struct vmod **hdl, unsigned nbr, void *ptr, int len,
 		v->abi = d->abi;
 		v->vrt_major = d->vrt_major;
 		v->vrt_minor = d->vrt_minor;
+		v->vcs = d->vcs;
+		v->version = d->version;
 
 		REPLACE(v->nm, nm);
 		REPLACE(v->path, path);
@@ -201,9 +205,19 @@ VMOD_Panic(struct vsb *vsb)
 
 	VSB_cat(vsb, "vmods = {\n");
 	VSB_indent(vsb, 2);
-	VTAILQ_FOREACH(v, &vmods, list)
-		VSB_printf(vsb, "%s = {%p, %s, %u.%u},\n",
-		    v->nm, v, v->abi, v->vrt_major, v->vrt_minor);
+	VTAILQ_FOREACH(v, &vmods, list) {
+		VSB_printf(vsb, "%s = {", v->nm);
+		VSB_indent(vsb, 2);
+		VSB_printf(vsb, "p=%p, abi=\"%s\", vrt=%u.%u,\n",
+			   v, v->abi, v->vrt_major, v->vrt_minor);
+		VSB_bcat(vsb, "vcs=", 4);
+		VSB_quote(vsb, v->vcs, -1, VSB_QUOTE_CSTR);
+		VSB_bcat(vsb, ", version=", 10);
+		VSB_quote(vsb, v->version, -1, VSB_QUOTE_CSTR);
+		VSB_indent(vsb, -2);
+		VSB_bcat(vsb, "},\n", 3);
+	}
+
 	VSB_indent(vsb, -2);
 	VSB_cat(vsb, "},\n");
 }
@@ -218,8 +232,11 @@ ccf_debug_vmod(struct cli *cli, const char * const *av, void *priv)
 	(void)av;
 	(void)priv;
 	ASSERT_CLI();
-	VTAILQ_FOREACH(v, &vmods, list)
-		VCLI_Out(cli, "%5d %s (%s)\n", v->ref, v->nm, v->path);
+	VTAILQ_FOREACH(v, &vmods, list) {
+		VCLI_Out(cli, "%5d %s (path=\"%s\", version=\"%s\","
+		    " vcs=\"%s\")\n", v->ref, v->nm, v->path, v->version,
+		    v->vcs);
+	}
 }
 
 static struct cli_proto vcl_cmds[] = {
diff --git a/doc/changes.rst b/doc/changes.rst
index e465cd0ed..a14b6ef66 100644
--- a/doc/changes.rst
+++ b/doc/changes.rst
@@ -41,6 +41,42 @@ Varnish Cache NEXT (2025-03-15)
 .. PLEASE keep this roughly in commit order as shown by git-log / tig
    (new to old)
 
+* Two fields have been added to the VMOD data registered with varnish-cache:
+
+  - ``vcs`` for Version Control System is intended as an identifier from the
+    source code management system, e.g. the git revision, to identify the exact
+    source code which was used to build a VMOD binary.
+
+  - ``version`` is intended as a more user friendly identifier as to which
+    version of a vmod a binary represents.
+
+  Panics and the ``debug.vmod`` CLI command output now contain these
+  identifiers.
+
+  Where supported by the compiler and linker, the ``vcs`` identifier is also
+  reachable via the ``.vmod_vcs`` section of the vmod shared object ELF file and
+  can be extracted, for example, using ``readelf -p.vmod_vcs <file>``
+
+* ``vmodtool.py`` now creates a file ``vmod_vcs_version.txt`` in the current
+  working directory when called from a git tree. This file is intended to
+  transport version control system information to builds from distribution
+  bundles.
+
+  vmod authors should add it to the distribution and otherwise ignore it for
+  SCM.
+
+  Where git and automake are used, this can be accomplished by adding
+  ``vmod_vcs_version.txt`` to the ``.gitignore`` file and to the ``EXTRA_DIST``
+  and ``DISTCLEANFILES`` variables in ``Makefile.am``.
+
+  If neither git is used nor ``vmod_vcs_version.txt`` present, ``vmodtool.py``
+  will add ``NOGIT`` to the vmod as the vcs identifier.
+
+* ``vmodtool.py`` now accepts a ``$Version`` stanza in vmod vcc files to set the
+  vmod version as registered with Varnish-Cache. If ``$Version`` is not present,
+  an attempt is made to extract ``PACKAGE_STRING`` from an automake
+  ``Makefile``, otherwise ``NOVERSION`` is used as the version identifier.
+
 * The scope of VCL variables ``req.is_hitmiss`` and ``req.is_hitpass`` is now
   restricted to ``vcl_miss, vcl_deliver, vcl_pass, vcl_synth`` and ``vcl_pass,
   vcl_deliver, vcl_synth`` respectively.
diff --git a/doc/sphinx/reference/vmod.rst b/doc/sphinx/reference/vmod.rst
index e8de061ec..cf28deac0 100644
--- a/doc/sphinx/reference/vmod.rst
+++ b/doc/sphinx/reference/vmod.rst
@@ -56,6 +56,7 @@ data structures that do all the hard work.
 The std VMODs vmod.vcc file looks somewhat like this::
 
 	$ABI strict
+	$Version my.version
 	$Module std 3 "Varnish Standard Module"
 	$Event event_function
 	$Function STRING toupper(STRANDS s)
@@ -73,6 +74,11 @@ version, use ``strict``.  If it complies to the VRT and only needs
 to be rebuilt when breaking changes are introduced to the VRT API,
 use ``vrt``.
 
+The ``$Version`` line is also optional. It specifies the version identifier
+compiled into the VMOD binary for later identification. If omitted,
+``PACKAGE_STRING`` from an automake ``Makefile`` will be used, or ``NOVERSION``
+otherwise.
+
 The ``$Module`` line gives the name of the module, the manual section
 where the documentation will reside, and the description.
 
diff --git a/include/vrt.h b/include/vrt.h
index 1b4be285e..23cb77776 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -493,6 +493,8 @@ struct vmod_data {
 	const char			*proto;
 	const char			*json;
 	const char			*abi;
+	const char			*vcs;
+	const char			*version;
 };
 
 /***********************************************************************
diff --git a/lib/libvcc/vmodtool.py b/lib/libvcc/vmodtool.py
index 21d91a7b4..f554ce8a8 100755
--- a/lib/libvcc/vmodtool.py
+++ b/lib/libvcc/vmodtool.py
@@ -45,6 +45,7 @@ import json
 import optparse
 import os
 import re
+import subprocess
 import sys
 import time
 
@@ -128,6 +129,7 @@ CTYPES.update(PRIVS)
 
 DEPRECATED = {}
 
+
 #######################################################################
 
 def deprecated(key, txt):
@@ -930,6 +932,14 @@ class AliasStanza(Stanza):
     def json(self, jl):
         jl.append(["$ALIAS", self.sym_alias, self.sym_name])
 
+class VersionStanza(Stanza):
+
+    ''' $Version version ... '''
+
+    def parse(self):
+        if len(self.toks) < 2:
+            self.syntax()
+        self.vcc.pkgstr = " ".join(self.toks[1:])
 
 #######################################################################
 
@@ -944,6 +954,7 @@ DISPATCH = {
     "Synopsis": SynopsisStanza,
     "Alias":    AliasStanza,
     "Restrict": RestrictStanza,
+    "Version":  VersionStanza
 }
 
 
@@ -966,6 +977,7 @@ class vcc():
         self.auto_synopsis = True
         self.modname = None
         self.csn = None
+        self.pkgstr = None
 
     def openfile(self, fn):
         self.commit_files.append(fn)
@@ -1180,10 +1192,60 @@ class vcc():
         fo.write('\t\"\\n\\x03\"\n};\n')
         fo.write('#undef STRINGIFY\n')
 
+    # parts from varnish-cache include/generate.py
+    def version(self):
+        srcdir = os.path.dirname(self.inputfile)
+
+        pkgstr = "NOVERSION"
+
+        if self.pkgstr is not None:
+            pkgstr = self.pkgstr
+        else:
+            for d in [srcdir, "."]:
+                f = os.path.join(d, "Makefile")
+                if not os.path.exists(f):
+                    continue
+                for pkgstr in open(f):
+                    if pkgstr[:14] == "PACKAGE_STRING":
+                        pkgstr = pkgstr.split("=")[1].strip()
+                        break
+                break
+        return pkgstr
+
+    # parts from varnish-cache include/generate.py
+    def vcs(self):
+        srcdir = os.path.dirname(self.inputfile)
+
+        gitver = subprocess.check_output([
+            "git -C %s rev-parse HEAD 2>/dev/null || echo NOGIT" %
+            srcdir], shell=True, universal_newlines=True).strip()
+        gitfile = "vmod_vcs_version.txt"
+
+        if gitver == "NOGIT":
+            for d in [".", srcdir]:
+                f = os.path.join(d, gitfile)
+                if not os.path.exists(f):
+                    continue
+                fh = open(f, "r")
+                if not fh:
+                    continue
+                gitver = fh.read()
+                fh.close()
+                break;
+        else:
+            fh = open(gitfile, "w")
+            fh.write(gitver)
+            fh.close()
+
+        return gitver
 
     def vmod_data(self, fo):
+        version = json.dumps(self.version())
+        vcs = json.dumps(self.vcs())
         vmd = "Vmod_%s_Data" % self.modname
         fo.write('\n')
+        fo.write('__attribute__((section(".vmod_vcs"), used))\n')
+        fo.write('const char vmod_vcs[] = %s;' % vcs)
         for i in (714, 759, 765):
             fo.write("/*lint -esym(%d, %s) */\n" % (i, vmd))
         fo.write("\nextern const struct vmod_data %s;\n" % vmd)
@@ -1197,6 +1259,8 @@ class vcc():
         fo.write('\t.func_len =\tsizeof(%s),\n' % self.csn)
         fo.write('\t.json =\t\tVmod_Json,\n')
         fo.write('\t.abi =\t\tVMOD_ABI_Version,\n')
+        fo.write('\t.version =\t%s,\n' % version)
+        fo.write('\t.vcs =\tvmod_vcs,\n')
         fo.write("};\n")
 
     def mkcfile(self):
diff --git a/vmod/Makefile.am b/vmod/Makefile.am
index 1d2df6317..0449eaf4b 100644
--- a/vmod/Makefile.am
+++ b/vmod/Makefile.am
@@ -5,7 +5,9 @@ TESTS = @VMOD_TESTS@
 include $(top_srcdir)/vsc.am
 include $(top_srcdir)/vtc.am
 
-EXTRA_DIST = $(TESTS)
+EXTRA_DIST = $(TESTS) vmod_vcs_version.txt
+
+DISTCLEANFILES = vmod_vcs_version.txt
 
 AM_LDFLAGS  = $(AM_LT_LDFLAGS)
 


More information about the varnish-commit mailing list