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

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt;