[libvirt] [PATCH v5 11/23] src: rewrite driver impl checker in Python

Daniel P. Berrangé posted 23 patches 5 years ago
There is a newer version of this series
[libvirt] [PATCH v5 11/23] src: rewrite driver impl checker in Python
Posted by Daniel P. Berrangé 5 years ago
As part of an goal to eliminate Perl from libvirt build tools,
rewrite the check-driverimpls.pl tool in Python.

This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 Makefile.am                  |   1 +
 scripts/check-driverimpls.py | 102 +++++++++++++++++++++++++++++++++++
 src/Makefile.am              |   4 +-
 src/check-driverimpls.pl     |  80 ---------------------------
 4 files changed, 105 insertions(+), 82 deletions(-)
 create mode 100755 scripts/check-driverimpls.py
 delete mode 100755 src/check-driverimpls.pl

diff --git a/Makefile.am b/Makefile.am
index 7155ab6ce9..407a664626 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -48,6 +48,7 @@ EXTRA_DIST = \
   scripts/augeas-gentest.py \
   scripts/check-aclperms.py \
   scripts/check-drivername.py \
+  scripts/check-driverimpls.py \
   scripts/check-spacing.py \
   scripts/check-symfile.py \
   scripts/check-symsorting.py \
diff --git a/scripts/check-driverimpls.py b/scripts/check-driverimpls.py
new file mode 100755
index 0000000000..78d53e75a4
--- /dev/null
+++ b/scripts/check-driverimpls.py
@@ -0,0 +1,102 @@
+#!/usr/bin/env python
+#
+# Copyright (C) 2013-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/>.
+#
+
+from __future__ import print_function
+
+import re
+import sys
+
+
+def checkdriverimpls(filename):
+    intable = False
+    mainprefix = None
+
+    errs = False
+    with open(filename, "r") as fh:
+        lineno = 0
+        for line in fh:
+            lineno = lineno + 1
+            if intable:
+                if line.find("}") != -1:
+                    intable = False
+                    mainprefix = None
+                    continue
+
+                m = re.search(r'''\.(\w+)\s*=\s*(\w+),?/''', line)
+                if m is not None:
+                    api = m.group(1)
+                    impl = m.group(2)
+
+                    if api in ["no", "name"]:
+                        continue
+                    if impl in ["NULL"]:
+                        continue
+
+                    suffix = impl
+                    prefix = re.sub(r'''^([a-z]+)(.*?)$''', r'''\1''', impl)
+
+                    if mainprefix is not None:
+                        if mainprefix != prefix:
+                            print("%s:%d Bad prefix '%s' for API '%s', " +
+                                  "expecting '%s'" %
+                                  (filename, lineno, prefix, api, mainprefix),
+                                  file=sys.stderr)
+                            errs = True
+                    else:
+                        mainprefix = prefix
+
+                    if not api.startswith(mainprefix):
+                        suffix = re.sub(r'''^[a-z]+''', "", suffix)
+                        suffix = re.sub(r'''^([A-Z]+)''',
+                                        lambda m: m.group(1).lower(), suffix)
+
+                    if api != suffix:
+                        want = api
+                        if want.startswith("nwf"):
+                            want = "NWF" + want[3:]
+
+                        if not api.startswith(mainprefix):
+                            want = re.sub(r'''^([a-z])''',
+                                          lambda m: m.group(1).upper(), want)
+                            want = mainprefix + want
+
+                        print("%s:%d Bad impl name '%s' for API " +
+                              "'%s', expecting '%s'" %
+                              (filename, lineno, impl, api, want),
+                              file=sys.stderr)
+                        errs = True
+            else:
+                m = re.search(r'''^(?:static\s+)?(vir(?:\w+)?Driver)''' +
+                              r'''\s+(?!.*;)''', line)
+                if m is not None:
+                    drv = m.group(1)
+                    if drv in ["virNWFilterCallbackDriver",
+                               "virNWFilterTechDriver",
+                                "virConnectDriver"]:
+                        continue
+                    intable = True
+
+    return errs
+
+
+status = 0
+for filename in sys.argv[1:]:
+    if checkdriverimpls(filename):
+        status = 1
+sys.exit(status)
diff --git a/src/Makefile.am b/src/Makefile.am
index 55ad51abf1..62f1d55402 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -350,7 +350,7 @@ check-drivername:
 		$(srcdir)/libvirt_lxc.syms
 
 check-driverimpls:
-	$(AM_V_GEN)$(PERL) $(srcdir)/check-driverimpls.pl \
+	$(AM_V_GEN)$(RUNUTF8) $(PYTHON) $(top_srcdir)/scripts/check-driverimpls.py \
 		$(filter /%,$(DRIVER_SOURCE_FILES)) \
 		$(filter $(srcdir)/%,$(DRIVER_SOURCE_FILES)) \
 		$(addprefix $(srcdir)/,$(filter-out $(srcdir)/%, \
@@ -366,7 +366,7 @@ check-aclperms:
 		$(srcdir)/access/viraccessperm.h \
 		$(srcdir)/access/viraccessperm.c
 
-EXTRA_DIST += check-driverimpls.pl check-aclrules.pl
+EXTRA_DIST += check-aclrules.pl
 
 check-local: check-protocol check-symfile check-symsorting \
 	check-drivername check-driverimpls check-aclrules \
diff --git a/src/check-driverimpls.pl b/src/check-driverimpls.pl
deleted file mode 100755
index 3c0d54724c..0000000000
--- a/src/check-driverimpls.pl
+++ /dev/null
@@ -1,80 +0,0 @@
-#!/usr/bin/env perl
-#
-# Copyright (C) 2013 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/>.
-#
-
-use strict;
-use warnings;
-
-my $intable = 0;
-my $table;
-my $mainprefix;
-
-my $status = 0;
-while (<>) {
-    if ($intable) {
-        if (/}/) {
-            $intable = 0;
-            $table = undef;
-            $mainprefix = undef;
-        } elsif (/\.(\w+)\s*=\s*(\w+),?/) {
-            my $api = $1;
-            my $impl = $2;
-
-            next if $api eq "no";
-            next if $api eq "name";
-            next if $impl eq "NULL";
-
-            my $suffix = $impl;
-            my $prefix = $impl;
-            $prefix =~ s/^([a-z]+)(.*?)$/$1/;
-
-            if (defined $mainprefix) {
-                if ($mainprefix ne $prefix) {
-                    print "$ARGV:$. Bad prefix '$prefix' for API '$api', expecting '$mainprefix'\n";
-                    $status = 1;
-                }
-            } else {
-                $mainprefix = $prefix;
-            }
-
-            if ($api !~ /^$mainprefix/) {
-                $suffix =~ s/^[a-z]+//;
-                $suffix =~ s/^([A-Z]+)/lc $1/e;
-            }
-
-            if ($api ne $suffix) {
-                my $want = $api;
-                $want =~ s/^nwf/NWF/;
-                if ($api !~ /^$mainprefix/) {
-                    $want =~ s/^([a-z])/uc $1/e;
-                    $want = $mainprefix . $want;
-                }
-                print "$ARGV:$. Bad impl name '$impl' for API '$api', expecting '$want'\n";
-                $status = 1;
-            }
-        }
-    } elsif (/^(?:static\s+)?(vir(?:\w+)?Driver)\s+(?!.*;)/) {
-        next if $1 eq "virNWFilterCallbackDriver" ||
-                $1 eq "virNWFilterTechDriver" ||
-                $1 eq "virConnectDriver";
-        $intable = 1;
-        $table = $1;
-    }
-}
-
-exit $status;
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 11/23] src: rewrite driver impl checker in Python
Posted by Cole Robinson 5 years ago
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 check-driverimpls.pl tool in Python.
> 
> This was a straight conversion, manually going line-by-line to
> change the syntax from Perl to Python. Thus the overall structure
> of the file and approach is the same.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  Makefile.am                  |   1 +
>  scripts/check-driverimpls.py | 102 +++++++++++++++++++++++++++++++++++
>  src/Makefile.am              |   4 +-
>  src/check-driverimpls.pl     |  80 ---------------------------
>  4 files changed, 105 insertions(+), 82 deletions(-)
>  create mode 100755 scripts/check-driverimpls.py
>  delete mode 100755 src/check-driverimpls.pl
> 
> diff --git a/Makefile.am b/Makefile.am
> index 7155ab6ce9..407a664626 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -48,6 +48,7 @@ EXTRA_DIST = \
>    scripts/augeas-gentest.py \
>    scripts/check-aclperms.py \
>    scripts/check-drivername.py \
> +  scripts/check-driverimpls.py \
>    scripts/check-spacing.py \
>    scripts/check-symfile.py \
>    scripts/check-symsorting.py \
> diff --git a/scripts/check-driverimpls.py b/scripts/check-driverimpls.py
> new file mode 100755
> index 0000000000..78d53e75a4
> --- /dev/null
> +++ b/scripts/check-driverimpls.py
> @@ -0,0 +1,102 @@
> +#!/usr/bin/env python
> +#
> +# Copyright (C) 2013-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/>.
> +#
> +
> +from __future__ import print_function
> +
> +import re
> +import sys
> +
> +
> +def checkdriverimpls(filename):
> +    intable = False
> +    mainprefix = None
> +
> +    errs = False
> +    with open(filename, "r") as fh:
> +        lineno = 0
> +        for line in fh:
> +            lineno = lineno + 1
> +            if intable:
> +                if line.find("}") != -1:
> +                    intable = False
> +                    mainprefix = None
> +                    continue
> +
> +                m = re.search(r'''\.(\w+)\s*=\s*(\w+),?/''', line)

The ending / here breaks things. If the intent is to match lines like:

.connectSupportsFeature = qemuConnectSupportsFeature, /* 0.5.0 */

Then there needs to be something between '?' and '/'. But this script
could still be useful for sanitizing virStateDriver structs too, which
don't have those ending comments, so maybe just drop the / entirely.

With that and the pep8 issues fixed, all the errors trigger as expected

Tested-by: Cole Robinson <crobinso@redhat.com>

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 11/23] src: rewrite driver impl checker in Python
Posted by Ján Tomko 5 years ago
On Mon, Nov 11, 2019 at 02:38:14PM +0000, Daniel P. Berrangé wrote:
>As part of an goal to eliminate Perl from libvirt build tools,
>rewrite the check-driverimpls.pl tool in Python.
>
>This was a straight conversion, manually going line-by-line to
>change the syntax from Perl to Python. Thus the overall structure
>of the file and approach is the same.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> Makefile.am                  |   1 +
> scripts/check-driverimpls.py | 102 +++++++++++++++++++++++++++++++++++
> src/Makefile.am              |   4 +-
> src/check-driverimpls.pl     |  80 ---------------------------
> 4 files changed, 105 insertions(+), 82 deletions(-)
> create mode 100755 scripts/check-driverimpls.py
> delete mode 100755 src/check-driverimpls.pl
>
>diff --git a/Makefile.am b/Makefile.am
>index 7155ab6ce9..407a664626 100644
>--- a/Makefile.am
>+++ b/Makefile.am
>@@ -48,6 +48,7 @@ EXTRA_DIST = \
>   scripts/augeas-gentest.py \
>   scripts/check-aclperms.py \
>   scripts/check-drivername.py \
>+  scripts/check-driverimpls.py \
>   scripts/check-spacing.py \
>   scripts/check-symfile.py \
>   scripts/check-symsorting.py \
>diff --git a/scripts/check-driverimpls.py b/scripts/check-driverimpls.py
>new file mode 100755
>index 0000000000..78d53e75a4
>--- /dev/null
>+++ b/scripts/check-driverimpls.py
>@@ -0,0 +1,102 @@
>+#!/usr/bin/env python
>+#
>+# Copyright (C) 2013-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/>.
>+#
>+
>+from __future__ import print_function
>+
>+import re
>+import sys
>+
>+
>+def checkdriverimpls(filename):
>+    intable = False
>+    mainprefix = None
>+
>+    errs = False
>+    with open(filename, "r") as fh:
>+        lineno = 0
>+        for line in fh:
>+            lineno = lineno + 1
>+            if intable:
>+                if line.find("}") != -1:
>+                    intable = False
>+                    mainprefix = None
>+                    continue
>+
>+                m = re.search(r'''\.(\w+)\s*=\s*(\w+),?/''', line)

With the leftover '/' removed:

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

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