From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <bruce.richardson@intel.com>, dev@dpdk.org
Cc: thomas@monjalon.net
References: <20210421220358.2597249-1-thomas@monjalon.net>
<20210422090211.320855-1-bruce.richardson@intel.com>
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Message-ID: <b9b6c02e-fb3f-f514-a7fe-bdd35c022512@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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 <bruce.richardson@intel.com>
> ---
> 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