As part of an goal to eliminate Perl from libvirt build tools,
rewrite the test-wrap-argv.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 +
 build-aux/syntax-check.mk |   4 +-
 scripts/test-wrap-argv.py | 170 +++++++++++++++++++++++++++++++++++++
 tests/test-wrap-argv.pl   | 174 --------------------------------------
 tests/testutils.c         |  16 ++--
 5 files changed, 181 insertions(+), 184 deletions(-)
 create mode 100755 scripts/test-wrap-argv.py
 delete mode 100755 tests/test-wrap-argv.pl

diff --git a/Makefile.am b/Makefile.am
index 8c9c73c715..6f6cead526 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -61,6 +61,7 @@ EXTRA_DIST = \
   scripts/minimize-po.py \
   scripts/mock-noinline.py \
   scripts/prohibit-duplicate-header.py \
+  scripts/test-wrap-argv.py \
   build-aux/syntax-check.mk \
   build-aux/useless-if-before-free \
   build-aux/vc-list-files \
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index a61818855c..7d54df182a 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -2172,8 +2172,8 @@ header-ifdef:
 	$(PYTHON) $(top_srcdir)/scripts/header-ifdef.py
 
 test-wrap-argv:
-	$(AM_V_GEN)$(VC_LIST) | $(GREP) -E '\.(ldargs|args)' | xargs \
-	$(PERL) $(top_srcdir)/tests/test-wrap-argv.pl --check
+	$(AM_V_GEN)$(VC_LIST) | $(GREP) -E '\.(ldargs|args)' | $(RUNUTF8) xargs \
+	$(PYTHON) $(top_srcdir)/scripts/test-wrap-argv.py --check
 
 group-qemu-caps:
 	$(AM_V_GEN)$(PERL) $(top_srcdir)/tests/group-qemu-caps.pl --check $(top_srcdir)/
diff --git a/scripts/test-wrap-argv.py b/scripts/test-wrap-argv.py
new file mode 100755
index 0000000000..dd9e7de824
--- /dev/null
+++ b/scripts/test-wrap-argv.py
@@ -0,0 +1,170 @@
+#!/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 script is intended to be passed a list of .args files, used
+# to store command line ARGV for the test suites. It will reformat
+# them such that there is at most one '-param value' on each line
+# of the file. Parameter values that are longer than 80 chars will
+# also be split.
+#
+# If --in-place is supplied as the first parameter of this script,
+# the files will be changed in place.
+# If --check is the first parameter, the script will return
+# a non-zero value if a file is not wrapped correctly.
+# Otherwise the rewrapped files are printed to the standard output.
+
+from __future__ import print_function
+
+import argparse
+import subprocess
+import sys
+
+
+def rewrap_line(line):
+    bits = line.split(" ")
+
+    # bits contains env vars, then the command line
+    # and then the arguments
+    env = []
+    cmd = None
+    args = []
+
+    if bits[0].find("=") == -1:
+        cmd = bits[0]
+        bits = bits[1:]
+
+    for bit in bits:
+        # If no command is defined yet, we must still
+        # have env vars
+        if cmd is None:
+            # Look for leading / to indicate command name
+            if bit.startswith("/"):
+                cmd = bit
+            else:
+                env.append(bit)
+        else:
+            # If there's a leading '-' then this is a new
+            # parameter, otherwise its a value for the prev
+            # parameter.
+            if bit.startswith("-"):
+                args.append(bit)
+            else:
+                args[-1] = args[-1] + " " + bit
+
+    # We might have to split line argument values...
+    args = [rewrap_arg(arg) for arg in args]
+
+    # Print env + command first
+    return " \\\n".join(env + [cmd] + args) + "\n"
+
+
+def rewrap_arg(arg):
+    ret = []
+    max_len = 78
+
+    while len(arg) > max_len:
+        split = arg.rfind(",", 0, max_len + 1)
+        if split == -1:
+            split = arg.rfind(":", 0, max_len + 1)
+        if split == -1:
+            split = arg.rfind(" ", 0, max_len + 1)
+        if split == -1:
+            print("cannot find nice place to split '%s' below 80 chars" %
+                  arg, file=sys.stderr)
+            split = max_len - 1
+
+        split = split + 1
+
+        ret.append(arg[0:split])
+        arg = arg[split:]
+
+    ret.append(arg)
+    return "\\\n".join(ret)
+
+
+def rewrap(filename, in_place, check):
+    # Read the original file
+    with open(filename, 'r') as fh:
+        orig_lines = []
+        for line in fh:
+            orig_lines.append(line)
+
+    if len(orig_lines) == 0:
+        return
+
+    lines = []
+    for line in orig_lines:
+        if line.endswith("\\\n"):
+            line = line[:-2]
+        lines.append(line)
+
+    # Kill the last new line in the file
+    lines[-1] = lines[-1].rstrip("\n")
+
+    # Reconstruct the master data by joining all lines
+    # and then split again based on the real desired
+    # newlines
+    lines = "".join(lines).split("\n")
+
+    # Now each 'lines' entry represents a single command, we
+    # can process them
+    new_lines = []
+    for line in lines:
+        new_lines.append(rewrap_line(line))
+
+    if in_place:
+        with open(filename, "w") as fh:
+            for line in new_lines:
+                print(line, file=fh)
+    elif check:
+        orig = "".join(orig_lines)
+        new = "".join(new_lines)
+        if new != orig:
+            diff = subprocess.Popen(["diff", "-u", filename, "-"],
+                                    stdin=subprocess.PIPE)
+            diff.communicate(input=new.encode('utf-8'))
+
+            print("Incorrect line wrapping in $file",
+                  file=sys.stderr)
+            print("Use test-wrap-argv.py to wrap test data files",
+                  file=sys.stderr)
+            return False
+    else:
+        for line in new_lines:
+            print(line)
+
+    return True
+
+
+parser = argparse.ArgumentParser(description='Test arg line wrapper')
+parser.add_argument('--in-place', '-i', action="store_true",
+                    help='modify files in-place')
+parser.add_argument('--check', action="store_true",
+                    help='check existing files only')
+parser.add_argument('files', nargs="+",
+                    help="filenames to check")
+args = parser.parse_args()
+
+errs = False
+for filename in args.files:
+    if not rewrap(filename, args.in_place, args.check):
+        errs = True
+
+if errs:
+    sys.exit(1)
+sys.exit(0)
diff --git a/tests/test-wrap-argv.pl b/tests/test-wrap-argv.pl
deleted file mode 100755
index 7867e9d719..0000000000
--- a/tests/test-wrap-argv.pl
+++ /dev/null
@@ -1,174 +0,0 @@
-#!/usr/bin/env perl
-#
-# Copyright (C) 2015 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 script is intended to be passed a list of .args files, used
-# to store command line ARGV for the test suites. It will reformat
-# them such that there is at most one '-param value' on each line
-# of the file. Parameter values that are longer than 80 chars will
-# also be split.
-#
-# If --in-place is supplied as the first parameter of this script,
-# the files will be changed in place.
-# If --check is the first parameter, the script will return
-# a non-zero value if a file is not wrapped correctly.
-# Otherwise the rewrapped files are printed to the standard output.
-
-$in_place = 0;
-$check = 0;
-
-if (@ARGV[0] eq "--in-place" or @ARGV[0] eq "-i") {
-    $in_place = 1;
-    shift @ARGV;
-} elsif (@ARGV[0] eq "--check") {
-    $check = 1;
-    shift @ARGV;
-}
-
-$ret = 0;
-foreach my $file (@ARGV) {
-    if (&rewrap($file) < 0) {
-        $ret = 1;
-    }
-}
-
-exit $ret;
-
-sub rewrap {
-    my $file = shift;
-
-    # Read the original file
-    open FILE, "<", $file or die "cannot read $file: $!";
-    my @orig_lines = <FILE>;
-    close FILE;
-    my @lines = @orig_lines;
-    foreach (@lines) {
-        # If there is a trailing '\' then kill the new line
-        if (/\\$/) {
-            chomp;
-            $_ =~ s/\\$//;
-        }
-    }
-
-    # Skip empty files
-    return unless @lines;
-
-    # Kill the last new line in the file
-    chomp @lines[$#lines];
-
-    # Reconstruct the master data by joining all lines
-    # and then split again based on the real desired
-    # newlines
-    @lines = split /\n/, join('', @lines);
-
-    # Now each @lines represents a single command, we
-    # can process them
-    @lines = map { &rewrap_line($_) } @lines;
-
-    if ($in_place) {
-        open FILE, ">", $file or die "cannot write $file: $!";
-        foreach my $line (@lines) {
-            print FILE $line;
-        }
-        close FILE;
-    } elsif ($check) {
-        my $nl = join('', @lines);
-        my $ol = join('', @orig_lines);
-        unless ($nl eq $ol) {
-            open DIFF, "| diff -u $file -" or die "cannot run diff: $!";
-            print DIFF $nl;
-            close DIFF;
-
-            print STDERR "Incorrect line wrapping in $file\n";
-            print STDERR "Use test-wrap-argv.pl to wrap test data files\n";
-            return -1;
-        }
-    } else {
-        foreach my $line (@lines) {
-            print $line;
-        }
-    }
-    return 0;
-}
-
-sub rewrap_line {
-    my $line = shift;
-    my @bits = split / /, join('', $line);
-
-    # @bits contains env vars, then the command line
-    # and then the arguments
-    my @env;
-    my $cmd;
-    my @args;
-
-    if ($bits[0] !~ /=/) {
-        $cmd = shift @bits;
-    }
-
-    foreach my $bit (@bits) {
-        # If no command is defined yet, we must still
-        # have env vars
-        if (!defined $cmd) {
-            # Look for leading / to indicate command name
-            if ($bit =~ m,^/,) {
-                $cmd = $bit;
-            } else {
-                push @env, $bit;
-            }
-        } else {
-            # If there's a leading '-' then this is a new
-            # parameter, otherwise its a value for the prev
-            # parameter.
-            if ($bit =~ m,^-,) {
-                push @args, $bit;
-            } else {
-                $args[$#args] .= " " . $bit;
-            }
-        }
-    }
-
-    # We might have to split line argument values...
-    @args = map { &rewrap_arg($_) } @args;
-    # Print env + command first
-    return join(" \\\n", @env, $cmd, @args), "\n";
-}
-
-sub rewrap_arg {
-    my $arg = shift;
-    my @ret;
-    my $max_len = 78;
-
-    while (length($arg) > $max_len) {
-        my $split = rindex $arg, ",", $max_len;
-        if ($split == -1) {
-            $split = rindex $arg, ":", $max_len;
-        }
-        if ($split == -1) {
-            $split = rindex $arg, " ", $max_len;
-        }
-        if ($split == -1) {
-            warn "cannot find nice place to split '$arg' below 80 chars\n";
-            $split = $max_len - 1;
-        }
-        $split++;
-
-        push @ret, substr $arg, 0, $split;
-        $arg = substr $arg, $split;
-    }
-    push @ret, $arg;
-    return join("\\\n", @ret);
-}
diff --git a/tests/testutils.c b/tests/testutils.c
index da236c74a1..1acdc71fca 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -59,7 +59,7 @@ static size_t testCounter;
 static virBitmapPtr testBitmap;
 
 char *progname;
-static char *perl;
+static char *python;
 
 static int virTestUseTerminalColors(void)
 {
@@ -399,15 +399,15 @@ virTestRewrapFile(const char *filename)
           virStringHasSuffix(filename, ".ldargs")))
         return 0;
 
-    if (!perl) {
-        fprintf(stderr, "cannot rewrap %s: unable to find perl in path", filename);
+    if (!python) {
+        fprintf(stderr, "cannot rewrap %s: unable to find python in path", filename);
         return -1;
     }
 
-    if (virAsprintf(&script, "%s/test-wrap-argv.pl", abs_srcdir) < 0)
+    if (virAsprintf(&script, "%s/scripts/test-wrap-argv.py", abs_top_srcdir) < 0)
         goto cleanup;
 
-    cmd = virCommandNewArgList(perl, script, "--in-place", filename, NULL);
+    cmd = virCommandNewArgList(python, script, "--in-place", filename, NULL);
     if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
 
@@ -895,8 +895,8 @@ int virTestMain(int argc,
         }
     }
 
-    /* Find perl early because some tests override PATH */
-    perl = virFindFileInPath("perl");
+    /* Find python early because some tests override PATH */
+    python = virFindFileInPath("python");
 
     ret = (func)();
 
@@ -907,7 +907,7 @@ int virTestMain(int argc,
         fprintf(stderr, " %-3zu %s\n", testCounter, ret == 0 ? "OK" : "FAIL");
     }
     virLogReset();
-    VIR_FREE(perl);
+    VIR_FREE(python);
     return ret;
 }
 
-- 
2.21.0

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

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 test-wrap-argv.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 +
>  build-aux/syntax-check.mk |   4 +-
>  scripts/test-wrap-argv.py | 170 +++++++++++++++++++++++++++++++++++++
>  tests/test-wrap-argv.pl   | 174 --------------------------------------
>  tests/testutils.c         |  16 ++--
>  5 files changed, 181 insertions(+), 184 deletions(-)
>  create mode 100755 scripts/test-wrap-argv.py
>  delete mode 100755 tests/test-wrap-argv.pl
> 

./scripts/test-wrap-argv.py --in-place tests/qemuxml2argvdata/*.args

isn't idempotent, it adds trailing whitespace, but I didn't investigate.
The perl script doesn't alter those files

- Cole

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

On Mon, Nov 11, 2019 at 02:38:18PM +0000, Daniel P. Berrangé wrote:
>As part of an goal to eliminate Perl from libvirt build tools,

a goal? the goal?

>rewrite the test-wrap-argv.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 +
> build-aux/syntax-check.mk |   4 +-
> scripts/test-wrap-argv.py | 170 +++++++++++++++++++++++++++++++++++++
> tests/test-wrap-argv.pl   | 174 --------------------------------------
> tests/testutils.c         |  16 ++--
> 5 files changed, 181 insertions(+), 184 deletions(-)
> create mode 100755 scripts/test-wrap-argv.py
> delete mode 100755 tests/test-wrap-argv.pl
>

>+def rewrap_line(line):
>+    bits = line.split(" ")
>+
>+    # bits contains env vars, then the command line
>+    # and then the arguments
>+    env = []
>+    cmd = None
>+    args = []
>+
>+    if bits[0].find("=") == -1:

if "=" not in bits[0]:

>+        cmd = bits[0]
>+        bits = bits[1:]
>+

[...]

>+
>+    # Now each 'lines' entry represents a single command, we
>+    # can process them
>+    new_lines = []
>+    for line in lines:
>+        new_lines.append(rewrap_line(line))
>+
>+    if in_place:
>+        with open(filename, "w") as fh:
>+            for line in new_lines:
>+                print(line, file=fh)

This print needs an end='' to match the perl script behavior.

>+    elif check:
>+        orig = "".join(orig_lines)
>+        new = "".join(new_lines)
>+        if new != orig:
>+            diff = subprocess.Popen(["diff", "-u", filename, "-"],
>+                                    stdin=subprocess.PIPE)
>+            diff.communicate(input=new.encode('utf-8'))
>+
>+            print("Incorrect line wrapping in $file",
>+                  file=sys.stderr)
>+            print("Use test-wrap-argv.py to wrap test data files",
>+                  file=sys.stderr)
>+            return False
>+    else:
>+        for line in new_lines:
>+            print(line)

Same here.

>+
>+    return True
>+
>+

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