Ensure programs that link Xapi_version honor the configured version

[shindere]

The problem

./configure --xapi_version=v1912.6.23 # Turing's birth date
make
make install DESTDIR=/tmp/xapi
/tmp/xapi/usr/sbin/xenopsd-xc --version

When git describe finds a tag, the last command prints its output instead of the configured version:

26.20.0-29-g0af2b37-dirty

When it finds none, git describe returns a bare commit hash. Xapi_version.parse_xapi_version cannot parse it, and since this function is called when the Xapi_version module is initialised, xenopsd-xc aborts at startup, whatever its arguments:

Fatal error: exception Failure("Couldn't determine xapi version from string: '0af2b37-dirty'")

This happened in XCP-ng builds, for instance: xcp-ng-rpms/xapi#145

Eight other programs take their version from git describe in the same way, among them xcp-networkd, xcp-rrdd and squeezed. More generally, of the 19 installed programs that link Xapi_version, the module that holds the configured version, only one reports 1912.6.23 when invoked with --version.

Its causes

There are three causes, and they overlap: one cause can hide another. A program that aborts at startup because it gets its version from git describe (1) never gets a chance to show that it has no --version option (2), or that it reports a hard-coded version (3). Once they get the configured version instead, squeezed, varstored-guard and pvs-proxy-ovs-setup turn out to have no --version option (2), and sm-cli turns out to report a hard-coded version (3).

1. dune-build-info chooses the version it writes into a program as follows:

   • if the program belongs to a package that has a version, it writes that version, which here is the configured one;
   • otherwise, it writes the output of git describe when the program is installed, ignoring the version of the project.

   The nine programs mentioned above belong to no package: they are installed through an (install) stanza, and their (executable) stanza names none.

2. Many programs have no --version option: xapi, squeezed, varstored-guard, pvs-proxy-ovs-setup, mpathalert, event_listen, vncproxy, quicktestbin, gen_lifecycle, alert-certificate-check and daily-license-check. Some of them, xapi for instance, already use the configured version internally, but cannot show it.

3. Some programs report a hard-coded version: rrd2csv reports 0.1.3, and sm-cli, xapi-nbd and xenops-cli report 1.0.0.

What this PR does

This situation went unnoticed because nothing checked the version that installed programs report. The PR has two parts: it first adds the missing check, then removes the three causes described above.

The first part (commits 1 to 3) sets up the check. The script check-versions.sh lists every installed program that links Xapi_version, together with what the program does when invoked with --version, and checks that it does exactly that. The CI runs it after make install. The check has three goals:

1. Document how each program behaves before any change.
2. Track the progress made by the fixes (commits 4 to 10). Since the causes overlap, fixing one of them sometimes changes how a program fails instead of making it succeed: once commit 4 removes the first cause, squeezed, varstored-guard and pvs-proxy-ovs-setup stop aborting and turn out not to recognise --version, and sm-cli turns out to report 1.0.0. The script makes such changes visible: a program that does not behave as listed makes the check fail, even when it does better, so each fix updates the script, and each commit message shows how the summary of check-versions.sh changes.
3. Keep the problem from coming back, as far as possible. An installed program that links Xapi_version but is not listed in the script makes the check fail. A program that does not link Xapi_version, however, goes unnoticed, although it may still need the configured version: xapi-nbd and xenops-cli, fixed by commit 10, were such programs. For such programs, it is up to the programmer to use Xapi_version.

Commit 3 changes the version the CI configures from 0.0.0 to 1912.6.23 (Alan Turing's birth date). 0.0.0 is the default of Xapi_version.platform_version, so a program could report it because it is a default, not because it was configured, and still pass the check. Nobody would type 1912.6.23 by accident: a program that reports it can only have got it from the configuration. The value is also easy to spot in a log. The check itself, however, does not depend on the configured version, which it reads from the configuration: it passes both before commit 3, with 0.0.0, and after, with 1912.6.23.

The second part (commits 4 to 10) removes the causes one at a time. Commit 4 attaches the installed programs to their package, which removes cause (1). Commits 5, 6, 8 and 9 add a --version option where it was missing (2). Commits 6, 8 and 10 replace hard-coded versions with the configured one (3). Commit 7 prepares commit 8: it makes sure quicktestbin does nothing before parsing its arguments. Commits 4 to 9 deal with programs that already link Xapi_version. Commit 10 deals with xapi-nbd and xenops-cli: they do not link Xapi_version and report a hard-coded 1.0.0, but since they are part of the toolstack, it seemed relevant to have them report the configured version too.

Commit 4 attaches an executable to its package without installing it with (public_name -) and (package ...). The documentation of the executable stanza is ambiguous about this combination, but it matches what it documents for the executables stanza, where - in public_names marks an executable that is not installed, and dune does attach the executable to the package. If that changed, check-versions.sh would detect it.

The series is meant to be reviewed commit by commit. Each commit builds and passes the CI on its own. The message of commit 1 shows the summary of check-versions.sh before any fix, and the message of each later commit says how that summary changes.

Visible changes

• All 21 programs that now link Xapi_version report the configured version when invoked with --version.
• rrd2csv used to print "rrd2csv version 0.1.3" followed by "(C) Citrix 2012" for both --version and -v. Both now print the configured version only. The copyright notice is no longer printed, but remains in the header of the source file. Is that acceptable?
• quicktestbin no longer prints a qcheck seed, nor runs tput, before parsing its arguments. As a consequence, the message "tput: No value for $TERM", which appeared in the logs of the CI, is gone.
• The opam packages xapi-tools, xapi-storage-cli, varstored-guard, xapi-debug and xapi-nbd now declare their dependency on xapi-consts, which provides Xapi_version. They already depended on it indirectly, so this changes nothing in what gets installed: the dependency is only made explicit. I will update xs-opam accordingly once this PR is merged.

What this PR does not do

• vhd-tool, message-cli and qcow-stream-tool still report a hard-coded version, 1.0.0. They are standalone tools that do not link Xapi_version, and the packages of message-cli and qcow-stream-tool do not even depend on xapi-consts. Should they report the configured version too?
• This PR makes sure that programs honor a configured version. What happens when no version is configured is out of its scope, but deserves to be addressed separately: dune-build-info then gives every program the output of git describe. When that output is a bare commit hash, Xapi_version.parse_xapi_version fails to parse it while the Xapi_version module is initialised, so every program that links this module aborts before its main module even runs: such a program cannot do anything, not even report its version.

[psafont]

vhd-tool, message-cli and qcow-stream-tool still report a hard-coded version, 1.0.0. They are standalone tools that do not link Xapi_version, and the packages of message-cli and qcow-stream-tool do not even depend on xapi-consts. Should they report the configured version too?

I think so, yes. The hardcoded versions come from a time when all executables where stored in a different repo and no automated tooling for branding existed in the ocaml ecosystem.

This PR makes sure that programs honor a configured version. What happens when no version is configured is out of its scope, but deserves to be addressed separately: dune-build-info then gives every program the output of git describe. When that output is a bare commit hash, Xapi_version.parse_xapi_version fails to parse it while the Xapi_version module is initialised, so every program that links this module aborts before its main module even runs: such a program cannot do anything, not even report its version.

The versioning system was meant to interrupt the startup of the processes when neither of their 2 configuration methods were available, or contained a good version scheme. This makes it so the issue is detected at test time by producing a catastrophic error.

I would say it's working as intended. For reference the two configuration methods are git describe while running make and ./configure --xapi_version=... before running make.

[psafont]

I would have thought that the installs belonging to the package would have done the promotion. It would be interesting to know whether dune maintainers would be interested in adding the behaviour

[psafont]

The configure function is quite crappy and all the daemons should be made to use configure2.
This makes all daemons to use cmdliners which has several benefits, like adding --version, automagic man creation and bash completion.

The changes added by the branch can be found at master...psafont:xen-api:dev/pau/tweaks

I had another commit to change configure2 to normalise printing names for tools: fb64c0b

Unfortunately I didn't have time to test the, so I didn't end up submitting them. Hopefully they can give a few pointers on how to make this part betterin other ways