[libvirt] [PATCH v4 17/26] src: rewrite remote protocol checker in Python

As part of an goal to eliminate Perl from libvirt build tools, rewrite the pdwtags processing script in Python. The original inline shell and perl code was completely unintelligible. The new python code is a manual conversion that attempts todo basically the same thing. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- Makefile.am | 1 + cfg.mk | 3 +- scripts/check-remote-protocol.py | 136 +++++++++++++++++++++++++++++++ src/Makefile.am | 98 ++++------------------ 4 files changed, 155 insertions(+), 83 deletions(-) create mode 100644 scripts/check-remote-protocol.py diff --git a/Makefile.am b/Makefile.am index dea051ae77..a028bee219 100644 --- a/Makefile.am +++ b/Makefile.am @@ -50,6 +50,7 @@ EXTRA_DIST = \ scripts/check-aclrules.py \ scripts/check-drivername.py \ scripts/check-driverimpls.py \ + scripts/check-remote-protocol.py \ scripts/check-spacing.py \ scripts/check-symfile.py \ scripts/check-symsorting.py \ diff --git a/cfg.mk b/cfg.mk index 39b97c54f6..09b0bbbb3d 100644 --- a/cfg.mk +++ b/cfg.mk @@ -353,6 +353,7 @@ sc_prohibit_mkstemp: # access with F_OK or R_OK is okay, though. sc_prohibit_access_xok: @prohibit='access(at)? *\(.*X_OK' \ + in_vc_files='\.[ch]$$' \ halt='use virFileIsExecutable instead of access(,X_OK)' \ $(_sc_search_regexp) @@ -1227,7 +1228,7 @@ exclude_file_name_regexp--sc_prohibit_PATH_MAX = \ ^cfg\.mk$$ exclude_file_name_regexp--sc_prohibit_access_xok = \ - ^(cfg\.mk|src/util/virutil\.c)$$ + ^(src/util/virutil\.c)$$ exclude_file_name_regexp--sc_prohibit_asprintf = \ ^(cfg\.mk|bootstrap.conf$$|examples/|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c|tools/virt-login-shell\.c|tools/nss/libvirt_nss\.c$$) diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py new file mode 100644 index 0000000000..074f8c0ae1 --- /dev/null +++ b/scripts/check-remote-protocol.py @@ -0,0 +1,136 @@ +#!/usr/bin/env python +# +# Copyright (C) 2019 Red Hat, Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see +# <http://www.gnu.org/licenses/>;. +# +# This uses pdwtags to check remote protocol defs +# +# * the "split" splits on the /* DD */ comments, so that $p iterates +# through the struct definitions. +# * process only "struct remote_..." entries +# * remove comments and preceding TAB throughout +# * remove empty lines throughout +# * remove white space at end of buffer + +from __future__ import print_function + +import os +import os.path +import re +import subprocess +import sys + +cc = sys.argv[1] +objext = sys.argv[2] +proto_lo = sys.argv[3] +expected = sys.argv[4] + +proto_lo = proto_lo.replace("/", "/.libs/") + +ccargv = cc.split(" ") +ccargv.append("-v") +ccproc = subprocess.Popen(ccargv, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) +out, err = ccproc.communicate() +out = out.decode("utf-8") +if out.find("clang") != -1: + print("WARNING: skipping pdwtags test with Clang", file=sys.stderr) + sys.exit(0) + + +def which(program): + def is_exe(fpath): + return (os.path.isfile(fpath) and + os.access(fpath, os.X_OK)) + + fpath, fname = os.path.split(program) + if fpath: + if is_exe(program): + return program + else: + for path in os.environ["PATH"].split(os.pathsep): + exe_file = os.path.join(path, program) + if is_exe(exe_file): + return exe_file + + return None + + +pdwtags = which("pdwtags") +if pdwtags is None: + print("WARNING: you lack pdwtags; skipping the protocol test", + file=sys.stderr) + print("WARNING: install the dwarves package to get pdwtags", + file=sys.stderr) + sys.exit(0) + +proto_o = proto_lo.replace(".lo", ".o") + +if not os.path.exists(proto_o): + raise Exception("Missing %s", proto_o) + +pdwtagsproc = subprocess.Popen(["pdwtags", "--verbose", proto_o], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) +out, err = pdwtagsproc.communicate() +out = out.decode("utf-8") +err = err.decode("utf-8") + +if out == "" and err != "": + print("WARNING: no output, pdwtags appears broken:", file=sys.stderr) + for l in err.strip().split("\n"): + print("WARNING: %s" % l, file=sys.stderr) + print("WARNING: skipping the remote protocol test", file=sys.stderr) + sys.exit(0) + +# With pdwtags 1.8, --verbose output includes separators like these: +# /* 93 */ +# /* <0> (null):0 */ +# with the second line omitted for intrinsic types. +# Whereas with pdwtags 1.3, they look like this: +# /* <2d2> /usr/include/libio.h:180 */ +# The alternation of the following regexps matches both cases. +r1 = r'''/\* \d+ \*/''' +r2 = r'''/\* <[0-9a-fA-F]+> \S+:\d+ \*/''' + +libs_prefix = "remote_|qemu_|lxc_|admin_" +other_prefix = "keepalive|vir(Net|LockSpace|LXCMonitor)" +struct_prefix = "(" + libs_prefix + "|" + other_prefix + ")" + +n = 0 +bits = re.split(r'''\n*(?:%s|%s)\n''' % (r1, r2), out) +actual = ["/* -*- c -*- */"] + +for bit in bits: + if re.search(r'''^(struct|enum)\s+''' + struct_prefix, bit): + bit = re.sub(r'''\t*/\*.*?\*/''', "", bit) + bit = re.sub(r'''\s+\n''', '''\n''', bit) + bit = re.sub(r'''\s+$''', "", bit) + bit = re.sub(r'''\t''', " ", bit) + actual.append(bit) + n = n + 1 + +if n < 1: + print("WARNING: No structs/enums matched. Your", file=sys.stderr) + print("WARNING: pdwtags program is probably too old", file=sys.stderr) + print("WARNING: skipping the remote protocol test", file=sys.stderr) + print("WARNING: install dwarves-1.3 or newer", file=sys.stderr) + sys.exit(8) + +diff = subprocess.Popen(["diff", "-u", expected, "-"], stdin=subprocess.PIPE) +actualstr = "\n".join(actual) + "\n" +diff.communicate(input=actualstr.encode("utf-8")) + +sys.exit(diff.returncode) diff --git a/src/Makefile.am b/src/Makefile.am index 148982e33a..6a39afc666 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -192,83 +192,6 @@ DRIVER_SOURCES += \ -# Ensure that we don't change the struct or member names or member ordering -# in remote_protocol.x The embedded perl below needs a few comments, and -# presumes you know what pdwtags output looks like: -# * use -0777 -n to slurp the entire file into $_. -# * the "split" splits on the /* DD */ comments, so that $p iterates -# through the struct definitions. -# * process only "struct remote_..." entries -# * remove comments and preceding TAB throughout -# * remove empty lines throughout -# * remove white space at end of buffer - -# With pdwtags 1.8, --verbose output includes separators like these: -# /* 93 */ -# /* <0> (null):0 */ -# with the second line omitted for intrinsic types. -# Whereas with pdwtags 1.3, they look like this: -# /* <2d2> /usr/include/libio.h:180 */ -# The alternation of the following regexps matches both cases. -r1 = /\* \d+ \*/ -r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/ -libs_prefix = remote_|qemu_|lxc_|admin_ -other_prefix = keepalive|vir(Net|LockSpace|LXCMonitor) -struct_prefix = ($(libs_prefix)|$(other_prefix)) - -# Depending on configure options, libtool creates one or both of -# remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want -# the newest of the two, in case configure options changed and a stale -# file is left around from an earlier build. -# The pdwtags output is completely different when building with clang -# which causes the comparison against expected output to fail, so skip -# if using clang as CC. -PDWTAGS = \ - $(AM_V_GEN)if $(CC) -v 2>&1 | grep -q clang; then \ - echo 'WARNING: skipping pdwtags test with Clang' >&2; \ - exit 0; \ - fi; \ - if (pdwtags --help) > /dev/null 2>&1; then \ - o=`ls -t $(<:.lo=.$(OBJEXT)) \ - $(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \ - 2>/dev/null | sed -n 1p`; \ - test -f "$$o" || { echo ".o for $< not found" >&2; exit 1; }; \ - pdwtags --verbose $$o > $(@F)-t1 2> $(@F)-t2; \ - if test ! -s $(@F)-t1 && test -s $(@F)-t2; then \ - rm -rf $(@F)-t?; \ - echo 'WARNING: pdwtags appears broken; skipping the $@ test' >&2;\ - else \ - $(PERL) -0777 -n \ - -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {' \ - -e ' if ($$p =~ /^(struct|enum) $(struct_prefix)/) {' \ - -e ' $$p =~ s!\t*/\*.*?\*/!!sg;' \ - -e ' $$p =~ s!\s+\n!\n!sg;' \ - -e ' $$p =~ s!\s+$$!!;' \ - -e ' $$p =~ s!\t! !g;' \ - -e ' print "$$p\n";' \ - -e ' $$n++;' \ - -e ' }' \ - -e '}' \ - -e 'BEGIN {' \ - -e ' print "/* -*- c -*- */\n";' \ - -e '}' \ - -e 'END {' \ - -e ' if ($$n < 1) {' \ - -e ' warn "WARNING: your pdwtags program is too old\n";' \ - -e ' warn "WARNING: skipping the $@ test\n";' \ - -e ' warn "WARNING: install dwarves-1.3 or newer\n";' \ - -e ' exit 8;' \ - -e ' }' \ - -e '}' \ - < $(@F)-t1 > $(@F)-t3; \ - case $$? in 8) rm -f $(@F)-t?; exit 0;; 0) ;; *) exit 1;; esac;\ - diff -u $(@)s $(@F)-t3; st=$$?; rm -f $(@F)-t?; exit $$st; \ - fi; \ - else \ - echo 'WARNING: you lack pdwtags; skipping the $@ test' >&2; \ - echo 'WARNING: install the dwarves package to get pdwtags' >&2; \ - fi - # .libs/libvirt.so is built by libtool as a side-effect of the Makefile # rule for libvirt.la. However, checking symbols relies on Linux ELF layout if WITH_LINUX @@ -304,27 +227,38 @@ PROTOCOL_STRUCTS = \ if WITH_REMOTE check-protocol: $(PROTOCOL_STRUCTS) $(PROTOCOL_STRUCTS:structs=struct) +# Ensure that we don't change the struct or member names or member ordering +# in remote_protocol.x The process-pdwtags.py post-processes output to +# extract the bits we want. + +CHECK_REMOTE_PROTOCOL = $(top_srcdir)/scripts/check-remote-protocol.py + # The .o file that pdwtags parses is created as a side effect of running # libtool; but from make's perspective we depend on the .lo file. $(srcdir)/remote_protocol-struct \ $(srcdir)/qemu_protocol-struct \ $(srcdir)/lxc_protocol-struct: \ $(srcdir)/%-struct: remote/libvirt_driver_remote_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(CHECK_REMOTE_PROTOCOL) \ + "$(CC)" "$(OBJEXT)" $< $(@)s $(srcdir)/virnetprotocol-struct $(srcdir)/virkeepaliveprotocol-struct: \ $(srcdir)/%-struct: rpc/libvirt_net_rpc_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(CHECK_REMOTE_PROTOCOL) \ + "$(CC)" "$(OBJEXT)" $< $(@)s if WITH_LXC $(srcdir)/lxc_monitor_protocol-struct: \ $(srcdir)/%-struct: lxc/libvirt_driver_lxc_impl_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(CHECK_REMOTE_PROTOCOL) \ + "$(CC)" "$(OBJEXT)" $< $(@)s endif WITH_LXC $(srcdir)/lock_protocol-struct: \ $(srcdir)/%-struct: locking/lockd_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(CHECK_REMOTE_PROTOCOL) \ + "$(CC)" "$(OBJEXT)" $< $(@)s $(srcdir)/admin_protocol-struct: \ $(srcdir)/%-struct: admin/libvirt_admin_la-%.lo - $(PDWTAGS) + $(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(CHECK_REMOTE_PROTOCOL) \ + "$(CC)" "$(OBJEXT)" $< $(@)s else !WITH_REMOTE # The $(PROTOCOL_STRUCTS) files must live in git, because they cannot be -- 2.21.0