From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C9718A09E4; Thu, 22 Apr 2021 11:40:45 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9BD5B4068E; Thu, 22 Apr 2021 11:40:45 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 46C894003D for ; Thu, 22 Apr 2021 11:40:43 +0200 (CEST) IronPort-SDR: RyX2xYEjODYUcF7BKjmVwZIIwj3cWEVckIKLKhHYp0HqtSHH8v+EyDmvW7ZBmCLM52uUtlJLtq oX4LWtYlMG7g== X-IronPort-AV: E=McAfee;i="6200,9189,9961"; a="259811410" X-IronPort-AV: E=Sophos;i="5.82,242,1613462400"; d="scan'208";a="259811410" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Apr 2021 02:40:42 -0700 IronPort-SDR: yTCopqf0tpi9TLUepAYU+hROvZF/uB8LY5F8kEnRhrdVekAygYQd/8jeg6W9A/3nGcZgJn4vlw e+a2kNKqUkFw== X-IronPort-AV: E=Sophos;i="5.82,242,1613462400"; d="scan'208";a="384717189" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.213.222.135]) ([10.213.222.135]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Apr 2021 02:40:41 -0700 To: Bruce Richardson , dev@dpdk.org Cc: thomas@monjalon.net References: <20210421220358.2597249-1-thomas@monjalon.net> <20210422090211.320855-1-bruce.richardson@intel.com> From: "Burakov, Anatoly" Message-ID: Date: Thu, 22 Apr 2021 10:40:37 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: <20210422090211.320855-1-bruce.richardson@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC PATCH] devtools: script to check meson indentation of lists X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 22-Apr-21 10:02 AM, Bruce Richardson wrote: > This is a draft script developed when I was working on the whitespace rework > changes, since extended a little to attempt to fix some trailing comma issues. > > Signed-off-by: Bruce Richardson > --- > devtools/dpdk_meson_check.py | 106 +++++++++++++++++++++++++++++++++++ > 1 file changed, 106 insertions(+) > create mode 100755 devtools/dpdk_meson_check.py > > diff --git a/devtools/dpdk_meson_check.py b/devtools/dpdk_meson_check.py > new file mode 100755 > index 000000000..dc4c714ad > --- /dev/null > +++ b/devtools/dpdk_meson_check.py > @@ -0,0 +1,106 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2021 Intel Corporation > + > +''' > +A Python script to run some checks on meson.build files in DPDK > +''' > + > +import sys > +import os > +from os.path import relpath, join > +from argparse import ArgumentParser > + > +VERBOSE = False > +FIX = False > + > +def scan_dir(path): > + '''return meson.build files found in path''' > + for root, dirs, files in os.walk(path): > + if 'meson.build' in files: > + yield(relpath(join(root, 'meson.build'))) > + > + > +def check_indentation(filename, contents): > + '''check that a list or files() is correctly indented''' > + infiles = False > + inlist = False > + edit_count = 0 > + for lineno in range(len(contents)): for lineno, line in enumerate(contents) ? > + line = contents[lineno].rstrip() > + if not line: > + continue > + if line.endswith('files('): > + if infiles: > + raise(f'Error parsing {filename}:{lineno}, got "files(" when already parsing files list') > + if inlist: > + print(f'Error parsing {filename}:{lineno}, got "files(" when already parsing array list') > + infiles = True > + indent = 0 > + while line[indent] == ' ': > + indent += 1 Here and in other places, if this is measuring length of indent, maybe do something like: indent = len(line) - len(line.lstrip(' ')) ? > + indent += 8 # double indent required > + elif line.endswith('= ['): > + if infiles: > + raise(f'Error parsing {filename}:{lineno}, got start of array when already parsing files list') > + if inlist: > + print(f'Error parsing {filename}:{lineno}, got start of array when already parsing array list') > + inlist = True > + indent = 0 > + while line[indent] == ' ': > + indent += 1 > + indent += 8 # double indent required > + elif infiles and (line.endswith(')') or line.strip().startswith(')')): It's kinda hard to read with all the endswith/startswith, maybe extract those into a function? e.g. 'elif infiles and is_file_start(line)' > + infiles = False > + continue > + elif inlist and line.endswith(']') or line.strip().startswith(']'): > + inlist = False > + continue > + elif inlist or infiles: > + # skip further subarrays or lists > + if '[' in line or ']' in line: > + continue I guess you could make it recursive instead of giving up? Does this happen with any kind of regularity? > + if not line.startswith(' ' * indent) or line[indent] == ' ': > + print(f'Error: Incorrect indent at {filename}:{lineno + 1}') > + contents[lineno] = (' ' * indent) + line.strip() + '\n' > + line = contents[lineno].rstrip() > + edit_count += 1 > + if not line.endswith(',') and '#' not in line: > + # TODO: support stripping comment and adding ',' > + print(f'Error: Missing trailing "," in list at {filename}:{lineno + 1}') > + contents[lineno] = line + ',\n' > + line = contents[lineno].rstrip() What is the point of setting `line` here? > + edit_count += 1 > + return edit_count > + > + > +def process_file(filename): > + '''run checks on file "filename"''' > + if VERBOSE: > + print(f'Processing {filename}') > + with open(filename) as f: > + contents = f.readlines() I guess meson build files don't get too big so it's OK to read the entire file in memory and then work on it, rather than go line by line... > + > + if check_indentation(filename, contents) > 0 and FIX: > + print(f"Fixing {filename}") > + with open(filename, 'w') as f: > + f.writelines(contents) > + > + > +def main(): > + '''parse arguments and then call other functions to do work''' > + global VERBOSE > + global FIX Seems like globals are unnecessary here when you can just pass them into process_file? > + parser = ArgumentParser(description='Run syntax checks on DPDK meson.build files') > + parser.add_argument('-d', metavar='directory', default='.', help='Directory to process') > + parser.add_argument('--fix', action='store_true', help='Attempt to fix errors') > + parser.add_argument('-v', action='store_true', help='Verbose output') > + args = parser.parse_args() > + > + VERBOSE = args.v > + FIX = args.fix > + for f in scan_dir(args.d): > + process_file(f) > + > +if __name__ == "__main__": > + main() > -- > 2.27.0 > -- Thanks, Anatoly