[libvirt] [PATCH v5 14/23] src: rewrite remote protocol checker in Python

[Ján Tomko]

On Mon, Nov 11, 2019 at 02:38:17PM +0000, Daniel P. Berrangé wrote:
>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 at redhat.com>
>---
> Makefile.am                      |   1 +
> build-aux/syntax-check.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
>
>+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 bb63e2486c..c40e61e7ee 100644
>--- 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.

So I assume the configure option that gives you the path without .libs
is gone?

>-# 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`; \

And that OBJEXT is always .o

>-	  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 process-pdwtags.py post-processes output to

Here you refer to process-pdwtags.py but the script is called check-remote-protocol.py

>+# extract the bits we want.
>+
>+CHECK_REMOTE_PROTOCOL = $(top_srcdir)/scripts/check-remote-protocol.py
>+

With the comment fixed or the script renamed:

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191120/ab07dfbe/attachment-0001.sig>