Install VUT headers, plus cleanups

[Dridi]

I just realized that the headers were deemed private, despite the fact that they were added in version 1.5 of libvarnishapi. While at it I had a quick look and polished things here and there in the VUT area.

[fgsch]

Use argv[0] as the VUT.progname

Can you name some tools that will output the full path in the usage?

[Dridi]

httpd

edit: aka "apache 2"

On my system:

$ httpd -h
Usage: httpd [-D name] [-d directory] [-f file]
             [-C "directive"] [-c "directive"]
             [-k start|restart|graceful|graceful-stop|stop]
             [-v] [-V] [-h] [-l] [-L] [-t] [-T] [-S] [-X]
[...snip..]
$ /usr/sbin/httpd -h
Usage: /usr/sbin/httpd [-D name] [-d directory] [-f file]
                       [-C "directive"] [-c "directive"]
                       [-k start|restart|graceful|graceful-stop|stop]
                       [-v] [-V] [-h] [-l] [-L] [-t] [-T] [-S] [-X]
[...snip...]

[fgsch]

Any others? I don't really like this change tbh.

[Dridi]

I figured you didn't like it, and I hadn't thought of other tools doing that prior to doing this change. httpd was my top-of-mind try, here are more:

$ /usr/bin/cat --help | grep -i usage
Usage: /usr/bin/cat [OPTION]... [FILE]...
$ /usr/bin/head --help | grep -i usage
Usage: /usr/bin/head [OPTION]... [FILE]...
$ /usr/bin/cut --help | grep -i usage
Usage: /usr/bin/cut OPTION... [FILE]...
$ /usr/bin/dash --help | grep -i usage
/usr/bin/dash: 0: Illegal option --
$ /usr/bin/zsh --help | grep -i usage
Usage: /usr/bin/zsh [<options>] [<argument> ...]
$ /usr/bin/emacs --help | grep -i usage
Usage: /usr/bin/emacs [OPTION-OR-FILENAME]...

[fgsch]

Hmm, might be a linux-only thing. I cannot see this in any of the BSDs I have access to.

Most likely due to lacking __progname.

EDIT: Aha: http://stackoverflow.com/questions/273691/using-progname-instead-of-argv0

[fgsch]

FWIW, my preference wrt using argv[0] as the VUT.progname is to keep the existing behaviour, if anything because I don't see the benefit.

[Dridi]

Bugwash summary:

• @mbgrydeland agrees in general with exposing VUT symbols
• regarding argv[0] nobody objected among attendees (it seemed to be more about using a pristine argv[0] or its basename).
• @bsdphk wants to hold the merge back for at least a couple weeks

At this point we want to make sure it's safe to expose this API, look at all the bells and whistles (coverage, coverity, flexelint...) and avoid shipping an inappropriate interface to libvarnishapiconsumers. It also means that if we break the VUT ABI before exposing it we will need a 2.0 bump of the soname.

[bsdphk]

This is the kind of stuff I was thinking of, when I talked about making sure VUT was properly polished before we expose it:

During Specific Walk:
  File varnishhist.c line 590: VUT_Arg(?, 0) #2
../../lib/libvarnishapi/vut.c  160  Warning 418: Passing null pointer to
    function 'strtol(const char *, char **, int)', arg. no. 1 [Reference: file
    varnishhist.c: line 590]
varnishhist.c  590  Info 831: Reference cited in prior message

[bsdphk]

Another observation:

Functions like VUT_Error() which sometimes return (status == 0) and sometimes don't (status != 0) are generally a bad idea.

Split it into VUT_Warning() which always return and VUT_Error() or VUT_Fatal() which never does.

[Dridi]

Well flexelint was mistaken so I helped it in 978e82a. And because in another scenario it could be right I pushed 63fcdd6 too.

I had a quick look and I think getopt handling for -V is not OK yet because it prints Varnish's version. This is easy to circumvent.

[Dridi]

FYI this evening I started a tutorial in the form of a repository similar to libvmod-example:

https://github.com/dridi/varnish-template

It's making use of the current VUT headers from Varnish 5.1.2 (for now).

[bsdphk]

There were only one VUT_Error() use outside vut.c with status zero (varnishstat), so I have made VUT_Error() always fatal.

I'm a bit torn on the fprintf's about acquiring SHM, maybe they should only happen under some kind of verbose flag ?

[Dridi]

How about a FILE *log member in struct VUT? Those messages are fprintf'd only if it's non NULL and as far as in-tree VUTs are concerned we set it to stderr.

[Dridi]

And we could wrap the NULL check+fprintf in a VUT_Printf function.

[Dridi]

Updated to current master (started 328 commits ago!) and ready to resume the discussion any time.

[gquintard]

+1 for getting this in the september release

[Dridi]

New elements from IRC discussions and changes since the 328 commits ago:

• libvarnishapi already breaks, so we can safely break the VUT API
• it is currently a global API not fit for concurrent VSM accesses

It wouldn't be hard to make it usable on multiple varnishd instances, or even multiple times on a single varnishd instance. This is a trivial change, we may want to make the struct opaque, and interactions with the API could look (error handling omitted) like this:

struct VUT *vut = VUT_Init(argc, argv, &vopt_spec);

while ((opt = getopt(argc, argv, vopt_spec.vopt_optstring)) != -1) {
        switch (opt) {
        [...]
        default:
                VUT_Arg(vut, opt, optarg);
        }
}

VUT_Setup(vut, dispatch_f, dispatch_priv);
VUT_Main(vut);
VUT_Fini(vut);

The main problem is that getopt is MT-Unsafe, so we need to document that clearly since the VUT heavily relies on it. Or we could wrap the getopt handling in the VUT API and pass a callback+priv pair to VUT_Init do enforce the locking/reset of the getopt state if that's possible (and still document it to prevent users from running getopt concurrently).

[Dridi]

I'd like to take a step back after further IRC discussions.

First, I'm no longer in favor of making this API concurrent-able, because getopt gets in the way and we have no control over it. Also because this is clearly a convenience for single-purpose VSL consumers like varnishlog and varnishncsa.

Not only is the API single-process oriented as shown by the use of getopt, it is also meant to generate RST bits of the manual via the -x long options: the synopsis and the list of options.

On the other hand if someone needs to build something more involved than a single-purpose VSL consumer, they have access to the rest of libvarnishapi and full control over all things VUT can't do by design.

This would make it a realistic target for the September release again.

[fgsch]

Does not build in OSX.
From https://travis-ci.org/varnishcache/varnish-cache/jobs/274525706:

In file included from vut.c:55:
../../include/vut.h:80:17: error: a parameter list without types is only allowed in a function definition
void VUT_Signal(sighandler_t);
                ^
vut.c:224:12: error: unknown type name 'sighandler_t'
VUT_Signal(sighandler_t sig_cb)

[Dridi]

That's on me, I didn't notice in the manual that sighandler_t is a GNU extension, thanks! I updated the patch and I will press the merge button unless the next Travis build fails.

[Dridi]

Hopefully for the last time I updated this pull request (rebased against latest master).

The new commits are: 7dcb762 (autoconf macro) and 704e186 (release notes)

I ported my template project to 5.1.2 [1] and by adding the macro to configure.ac [2] all I need to generate the synopsis and list of options for the manual is a one-liner [3] for building and another one [4] for cleaning.

[1] Dridi/varnish-template@267f0e5
[2] https://github.com/Dridi/varnish-template/blob/41df3e8/configure.ac#L45
[3] https://github.com/Dridi/varnish-template/blob/41df3e8/src/Makefile.am#L63
[4] https://github.com/Dridi/varnish-template/blob/41df3e8/src/Makefile.am#L76