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

2019-11-20 Thread Daniel P . Berrangé 
On Wed, Nov 20, 2019 at 02:13:37PM +0100, Ján Tomko wrote:
> On Mon, Nov 11, 2019 at 02:38:17PM +, 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é 
> > ---
> > 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?

I honestly don't know what configure option it might have been
referring to in the first place.

My guess was perhaps related to --disable-shared to force a static
library build. Libvirt fails to link when doing this and we never
test it, so I considerd static build out of scope, and in any case
when i try it a .libs dir still appears

In absence of a clear need I figured just ignore this comment and
lets see if anyone reports breakage that our CI systems don't
catch.


> > -# 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

Same note as above.

> 
> > - 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 '  }' \
> > -

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

2019-11-20 Thread Ján Tomko 

On Mon, Nov 11, 2019 at 02:38:17PM +, 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é 
---
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

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

2019-11-18 Thread Cole Robinson 
On 11/11/19 9:38 AM, 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é 
> ---
>  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

I verified the script detected a difference in src/qemu_protocol-structs

Tested-by: Cole Robinson 

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2019-11-11 Thread Daniel P . Berrangé 
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é 
---
 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

diff --git a/Makefile.am b/Makefile.am
index e7ebe7281a..8c9c73c715 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/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 26eb5b94d9..a61818855c 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -412,6 +412,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)
 
@@ -2216,7 +2217,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$$)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py
new file mode 100644
index 00..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
+# .
+#
+# 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)