src: rewrite remote protocol checker in Python

As part of a 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. Tested-by: Cole Robinson <crobinso@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2024-12-22 13:45:38 +00:00 · 2019-08-30 13:22:54 +01:00 · 2019-08-30 13:22:54 +01:00 · e0314b6855
commit e0314b6855
parent f0b3840fb6
4 changed files with 153 additions and 83 deletions
--- a/Makefile.am
+++ b/Makefile.am
@ -52,6 +52,7 @@ EXTRA_DIST = \
  scripts/check-aclrules.py \
  scripts/check-drivername.py \
  scripts/check-driverimpls.py \
+  scripts/check-remote-protocol.py \
  scripts/check-symfile.py \
  scripts/check-symsorting.py \
  scripts/dtrace2systemtap.py \
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@ -419,6 +419,7 @@ sc_prohibit_mkdtemp:
 # 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)

@ -2234,7 +2235,7 @@ exclude_file_name_regexp--sc_prohibit_PATH_MAX = \
 	^build-aux/syntax-check\.mk$$

 exclude_file_name_regexp--sc_prohibit_access_xok = \
-	^(build-aux/syntax-check\.mk|src/util/virutil\.c)$$
+	^(src/util/virutil\.c)$$

 exclude_file_name_regexp--sc_prohibit_asprintf = \
  ^(build-aux/syntax-check\.mk|bootstrap.conf$$|examples/|src/util/virstring\.[ch]$$|tests/vircgroupmock\.c|tools/virt-login-shell\.c|tools/nss/libvirt_nss\.c$$)
--- a/scripts/check-remote-protocol.py
+++ b/scripts/check-remote-protocol.py
@ -0,0 +1,134 @@
+#!/usr/bin/env python3
+#
+# 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
+
+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)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@ -196,83 +196,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
@ -301,27 +224,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 check-remote-protocol.py script 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