[libvirt] [PATCH v3 04/22] build-aux: rewrite duplicate header checker in Python

[Erik Skultety]

On Tue, Sep 24, 2019 at 03:58:45PM +0100, Daniel P. Berrangé wrote:
> As part of an goal to eliminate Perl from libvirt build tools,
> rewrite the prohibit-duplicate-header.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 at redhat.com>
> ---
>  Makefile.am                            |  2 +-
>  build-aux/prohibit-duplicate-header.pl | 26 ------------
>  build-aux/prohibit-duplicate-header.py | 57 ++++++++++++++++++++++++++
>  cfg.mk                                 |  4 +-
>  4 files changed, 60 insertions(+), 29 deletions(-)
>  delete mode 100644 build-aux/prohibit-duplicate-header.pl
>  create mode 100644 build-aux/prohibit-duplicate-header.py
>
> diff --git a/Makefile.am b/Makefile.am
> index 8f688d40d0..a7d8da7146 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -47,7 +47,7 @@ EXTRA_DIST = \
>    build-aux/header-ifdef.pl \
>    build-aux/minimize-po.py \
>    build-aux/mock-noinline.pl \
> -  build-aux/prohibit-duplicate-header.pl \
> +  build-aux/prohibit-duplicate-header.py \
>    build-aux/useless-if-before-free \
>    build-aux/vc-list-files \
>    ci/Makefile \
> diff --git a/build-aux/prohibit-duplicate-header.pl b/build-aux/prohibit-duplicate-header.pl
> deleted file mode 100644
> index 4a2ea65665..0000000000
> --- a/build-aux/prohibit-duplicate-header.pl
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -#!/usr/bin/env perl
> -
> -use strict;
> -
> -my $file = " ";
> -my $ret = 0;
> -my %includes = ( );
> -my $lineno = 0;
> -
> -while (<>) {
> -    if (not $file eq $ARGV) {
> -        %includes = ( );
> -        $file = $ARGV;
> -        $lineno = 0;
> -    }
> -    $lineno++;
> -    if (/^# *include *[<"]([^>"]*\.h)[">]/) {
> -        $includes{$1}++;
> -        if ($includes{$1} == 2) {
> -            $ret = 1;
> -            print STDERR "$ARGV:$lineno: $_";
> -            print STDERR "Do not include a header more than once per file\n";
> -        }
> -    }
> -}
> -exit $ret;
> diff --git a/build-aux/prohibit-duplicate-header.py b/build-aux/prohibit-duplicate-header.py
> new file mode 100644
> index 0000000000..5ed3f82ba7
> --- /dev/null
> +++ b/build-aux/prohibit-duplicate-header.py
> @@ -0,0 +1,57 @@
> +#!/usr/bin/env python
> +#
> +# Copyright (C) 2016-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 check_file(filename):
> +    includes = {}
> +    lineno = 0
> +    errs = False
> +    with open(filename, "r") as fh:
> +        headerprog = re.compile(r'''^# *include *[<"]([^>"]*\.h)[">]\s*$''')
> +        for line in fh:
> +            lineno = lineno + 1
> +
> +            headermatch = headerprog.match(line)
> +            if headermatch is not None:
> +                inc = headermatch.group(1)
> +
> +                if inc in includes:
> +                    print("%s:%d: %s" % (filename, lineno, inc),
> +                          file=sys.stderr)
> +                    errs = True
> +                else:
> +                    includes[inc] = True
> +
> +    return errs
> +
> +
> +ret = 0
> +
> +for filename in sys.argv[1:]:
> +    if check_file(filename):
> +        ret = 1

so, how about instead of using @ret at all we make @errs global and then simply
do:

if err:
    print("Do not include a header...")

It's just an alternative, maybe a tiny bit more intuitive.

Other than that, same question about the re.compile as in the previous patch,
but those are just nitpicks:

Reviewed-by: Erik Skultety <eskultet at redhat.com>

> +
> +if ret == 1:
> +    print("Do not include a header more than once per file", file=sys.stderr)
> +
> +sys.exit(ret)