From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean-Jacques Hiblot Date: Thu, 18 Oct 2018 13:38:11 +0200 Subject: [U-Boot] [RFC PATCH v2 3/3] tools: Add a tool to get an overview of the usage of CONFIG options In-Reply-To: References: <1538574832-21910-1-git-send-email-jjhiblot@ti.com> <1538574832-21910-4-git-send-email-jjhiblot@ti.com> Message-ID: <5b44c009-9a9d-2506-9707-6030e2840b97@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Simon, On 09/10/2018 18:20, Simon Glass wrote: > Hi Jean-Jacques, > > On 3 October 2018 at 07:53, Jean-Jacques Hiblot wrote: >> configs2csv.py is tool that allow to check how some options are used for a >> particular subset of platforms. >> The purpose is to identify the targets that are actually using one or more >> options of interest. >> For example, it can tell what targets are still using CONFIG_DM_I2_COMPAT. >> It relies on the config database produced by tools/moveconfig.py. >> If the database doesn't exist, it will build it for the restricted set of >> the selected platforms. Once the database is built, it is much faster than >> greping the configs directory and more accurate as it relies on the >> information found in u-boot.cfg instead of defconfigs. >> It possible to look for options in the u-boot, the SPL or the TPL >> configurations. It can also perform diffs between those configurations. >> >> usage: configs2csv.py [-h] [-X] [--u-boot] [--spl] [--tpl] [--diff] >> [--rebuild-db] [-j JOBS] [-o OUTPUT] [--no-header] >> [--discard-empty] [-i] [--soc SOC] [--vendor VENDOR] >> [--arch ARCH] [--cpu CPU] [--board BOARD] >> [--target TARGET] >> OPTION [OPTION ...] >> >> all filtering parameters (OPTION, vendor, arch, ...) accept regexp. >> ex: configs2csv.py .*DM_I2C.* --soc 'omap[2345]|k3' will match >> CONFIG_DM_I2C and CONFIG_DM_I2C_COMPAT and look for it only for targets >> using the omap2, omap3, omap4, omap5 or k3 SOCs. >> >> Signed-off-by: Jean-Jacques Hiblot >> >> --- >> >> Changes in v2: >> - basically rewrote the whole thing >> - use tools/moveconfig.py to generate the database of configs >> - use tools/find_defconfigs.py to get the list of defconfigs off interest >> - removed diff with .config. tools/moveconfig.py does a better job >> >> tools/configs2csv.py | 387 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 387 insertions(+) >> create mode 100755 tools/configs2csv.py > This looks like a useful tool and a big improvement on the movconfig > starting point. Style comments below. Thanks for the review. I'll send a v3 taking care of the comments in a couple of days. > >> diff --git a/tools/configs2csv.py b/tools/configs2csv.py >> new file mode 100755 >> index 0000000..70b6602 >> --- /dev/null >> +++ b/tools/configs2csv.py >> @@ -0,0 +1,387 @@ >> +#!/usr/bin/env python >> +# SPDX-License-Identifier: GPL-2.0+ >> +# >> +# Author: JJ Hiblot >> +# >> + >> +""" >> +scan the configuration of specified targets (ie defconfigs) and outputs a >> +summary in a csv file. >> +Useful tool to check what platform is using a particular set of options. >> + >> + >> +How does it work? >> +----------------- >> + >> +This tools uses the config database produced by tools/moveconfig.py (called >> +with option -B to get all the configs: SPl, TPL and u-boot). If the database >> +is not present, it will build it. A rebuild can be forced with the option >> +'--rebuild-db'. >> + >> +The list of the targets of interest can be specified by a set of filter (soc, >> +vendor, defconfig name, ..). All those filters are actually regexp, allowing >> +for complex selection. The selection process is done by >> +tools/find_defconfigs.py >> +ex: --soc omap[23] --vendor 'ti|compulab' will inspect the omap2 and omap3 >> +platforms from TI and compulab >> + >> + >> +examples: >> +--------- >> + >> + >> +1) Get an overview of the usage of CONFIG_DM, CONFIG_SPL_DM, and DM/I2C related >> + options for platforms with omap5 or k3 SOC in u-boot and in SPL >> + >> +$ tools/configs2csv.py CONFIG_SPL_DM CONFIG_DM CONFIG_DM_I2C.* --vendor ti \ >> + --soc 'omap5|k3' -X --u-boot --spl -o dummy.csv >> + > Is this output below showing the contents of the ,csv file in a non-CVS format? Yes. This represents what we would see in a spreadsheet viewer. The csv output is not easily readable. >> +vendor soc defconfig type CONFIG_DM CONFIG_DM_I2C CONFIG_DM_I2C_COMPAT CONFIG_SPL_DM >> +ti omap5 am57xx_evm_defconfig SPL X X >> +ti omap5 am57xx_evm_defconfig u-boot X X X X >> +ti omap5 am57xx_hs_evm_defconfig SPL X X >> +ti omap5 am57xx_hs_evm_defconfig u-boot X X X X >> +ti k3 am65x_evm_a53_defconfig SPL X X >> +ti k3 am65x_evm_a53_defconfig u-boot X X >> +ti omap5 dra7xx_evm_defconfig SPL X X >> +ti omap5 dra7xx_evm_defconfig u-boot X X X X >> +ti omap5 dra7xx_hs_evm_defconfig SPL X X >> +ti omap5 dra7xx_hs_evm_defconfig u-boot X X X X >> +ti omap5 omap5_uevm_defconfig SPL >> +ti omap5 omap5_uevm_defconfig u-boot >> + >> + >> +This shows quickly that DM is not supported at all for omap5_uevm, that >> +only am65x_evm_a53 in not using DM_I2C in u-boot, and finally that DM_I2C is >> +not enabled in the SPL for any platform although SPL_DM is. >> +Also all the other platforms that enabled DM_I2C, also enabled >> +CONFIG_DM_I2C_COMPAT. >> + >> + >> +2) Check differences in config between SPL, TPL and u-boot (--diff option) >> + >> +Some platforms may disable/enable stuff in the configuration header files if >> +in SPl. This makes it hard to know the usage of a variable by just looking at >> +the .config. This is specially true for DM stuff. >> + >> +$ tools/configs2csv.py CONFIG\(_SPL\)?_DM_.* --vendor ti \ >> + --soc 'omap5|k3' --diff --spl --u-boot > dummy.csv >> + >> +vendor soc defconfig CONFIG_DM_I2C CONFIG_DM_I2C_COMPAT CONFIG_DM_STDIO CONFIG_DM_WARN >> +ti omap5 am57xx_evm_defconfig u-boot u-boot u-boot u-boot >> +ti omap5 am57xx_hs_evm_defconfig u-boot u-boot u-boot u-boot >> +ti k3 am65x_evm_a53_defconfig u-boot u-boot >> +ti omap5 dra7xx_evm_defconfig u-boot u-boot u-boot u-boot >> +ti omap5 dra7xx_hs_evm_defconfig u-boot u-boot u-boot u-boot >> + >> +This shows that k3 has no real config diff between SPl and u-boot. whereas am57 >> +and dra7 have different settings for DM_I2C and DM_I2C_COMPAT >> + >> +""" >> + >> +import argparse >> +import csv >> +import os >> +import re >> +import sys >> +from collections import namedtuple >> +from itertools import combinations >> + >> +import find_defconfigs >> + >> +CONFIG_DATABASE = 'moveconfig.db' >> +target = namedtuple("target", ["defconfig", "binary_type"]) >> + >> + >> +class db: > Please capitalise the class name. Also how about ConfigDb or something > a little longer than db? Below you actually use db as a variable name. > >> + >> + """ db is an object that store a collection of targets >> + a target is identified buy its defconfig and its binary type. ex: >> + (omap3_evm_defconfig,SPL) >> + The main purpose of this object is to output a CSV file that describes all >> + the targets. >> + There is also the possibility to create a "diff" db from a db. This new db >> + contains a summary of the differences between target of same defconfig. >> + """ >> + >> + def __init__(self): >> + self.targets = dict() >> + >> + def add_target(self, target): >> + self.targets[target] = dict() >> + >> + def add_option(self, target, option, value): >> + self.targets[target][option] = value >> + >> + def add_options(self, target, dic): >> + self.targets[target].update(dic) >> + >> + def output_csv( >> + self, output, show_X=False, header=True, left_columns=None, discard_empty_rows=False): > Please add function comments with Args and Returns (if you have > argument and return value) along with what the function does. > >> + all_options = set() >> + if len(self.targets) == 0: >> + return >> + >> + if discard_empty_rows: >> + dic = {k: self.targets[k] for k in self.targets if self.targets[k]} >> + else: >> + dic = self.targets.copy() >> + for target in dic.keys(): >> + for option in dic[target].keys(): >> + all_options.add(option) >> + if show_X: >> + dic[target][option] = "X" >> + if left_columns: >> + left_columns(target, dic, header=False) >> + >> + columns = [] >> + if left_columns: >> + columns.extend(left_columns(None, header=True)) >> + columns.extend(sorted(all_options)) >> + >> + writer = csv.DictWriter(output, fieldnames=columns, >> + lineterminator='\n') >> + if header: >> + writer.writeheader() >> + for target in sorted(dic.keys()): >> + writer.writerow(dic[target]) >> + >> + def diff_one_defconfig(self, defconfig): >> + """ This function creates a dictionary of the differences between the >> + binaries os a single target. >> + For example, for "dra7xx_evm_defconfig" it will compute the diffence >> + between the options used to compile u-boot and the SPL (not the TPL >> + because this platform doesn't have it). >> + The return value looks as follow: { 'CONFIG_DM_I2C: "u-boot", >> + CONFIG_SPL_BUILD:"SPL", CONFIG_DUMMY_SPI_FREQ: "diff" }. >> + >> + The algorithm can probably be optimized, but I didn't care enough. >> + algo is: >> + - return immediately is there is only one binary type (u-boot) >> + - create a dic that is merge of the dic for all the binary types >> + - for each binary type, compare its dic to the merged_dic. If it is >> + different then break. It means that at least one option is different. >> + - if no difference has been found, then return >> + - at this point, we know that there is at least one diff. For each >> + binary types and for all options used for this binary type, check if it >> + is in the merged dic and, if so, if its value is the same. update our >> + return dic with the proper description. >> + > Drop blank line before """ > > Returns: > dict: > key: ... > value: ... > > See buildman,, etc. for examples. > >> + """ >> + >> + diffs = dict() >> + diff_found = False >> + >> + # get all binary types (spl, TPL, u-boot) generated by this defconfig >> + all_binary_types = sorted( >> + set([t.binary_type for t in self.targets.keys() if t.defconfig == defconfig])) >> + >> + # If there is only one type of binary, no need to do a diff >> + if len(all_binary_types) <= 1: >> + return None >> + >> + # create a dict with all options:values used by all binaries >> + merged_dic = dict() >> + for bin_type in all_binary_types: >> + merged_dic.update(self.targets[target(defconfig, bin_type)]) >> + >> + # check if all binaries have the same options (should be the case for >> + # most of the defconfigs) >> + for bin_type in all_binary_types: >> + if self.targets[target(defconfig, bin_type)] != merged_dic: >> + diff_found = True >> + break >> + if not diff_found: >> + return None >> + >> + # at this point, we know that there are some options that differ >> + # between binaries (either not present or different) >> + >> + # Get a list (actually a set) of the options that are different >> + differing_keys = set() >> + for bin_type in all_binary_types: >> + dic = self.targets[target(defconfig, bin_type)] >> + for opt, value in merged_dic.items(): >> + if dic.get(opt, None) != value: >> + differing_keys.add(opt) >> + >> + # create a dictionary that summarize the differences >> + for bin_type in all_binary_types: >> + dic = self.targets[target(defconfig, bin_type)] >> + for opt in differing_keys: >> + dic_value = dic.get(opt, None) >> + merged_value = merged_dic.get(opt, None) >> + previous = diffs.get(opt, None) >> + if dic_value: >> + if dic_value != merged_value: >> + diffs[opt] = "diff" >> + elif previous != "diff": >> + diffs[opt] = ' / '.join( >> + [previous, bin_type]) if previous else bin_type >> + >> + return diffs >> + >> + def diff(self): >> + """ create a new db that contains the differences between the binaries >> + for all the targets in the db """ > """ goes on its own line. Looks like this needs Returns comment. > >> + diff_db = db() >> + # get a list of all the targets >> + all_defconfigs = set([t.defconfig for t in self.targets.keys()]) >> + # for every target of the list, get a dictionary of the difference. >> + # if the dictionary is not empty, add it the new db >> + for defconfig in all_defconfigs: >> + diff_dic = self.diff_one_defconfig(defconfig) >> + if diff_dic: >> + diff_db.add_target(target(defconfig, None)) >> + diff_db.add_options(target(defconfig, None), diff_dic) >> + return diff_db >> + >> + >> +def read_db(boards, option_filter, binary_types): > Needs function comment again. > >> + defconfig = "" >> + _db = db() >> + >> + # Read in the database >> + with open(CONFIG_DATABASE) as fd: >> + for line in fd.readlines(): >> + line = line.rstrip() >> + if not line: # Separator between defconfigs. >> + # We do not really care. We detect a new config by the absence >> + # of ' 'at the beginning of the line >> + pass >> + elif line[0] == ' ': # CONFIG_xxx line >> + if t and option_filter(line): >> + config, value = line.strip().split('=', 1) >> + _db.add_option(t, config, value) >> + else: # New defconfig >> + infos = line.split() >> + defconfig = infos[0] >> + try: >> + binary_type = infos[1] >> + except: >> + binary_type = "u-boot" >> + if binary_type in binary_types and defconfig in boards: >> + t = target(defconfig, binary_type) >> + _db.add_target(t) >> + else: >> + t = None >> + return _db >> + >> + >> +def main(): >> + parser = argparse.ArgumentParser(description="Show CONFIG options usage") >> + parser.add_argument("options", metavar='OPTION', type=str, nargs='+', >> + help="regexp to filter on options.\ >> + ex: CONFIG_DM_I2C_COMPAT or '.*DM_MMC.*'") >> + parser.add_argument( >> + "-X", help="show a X instead of the value of the option", > Please capitalise the help, e.g. 'Show a X' > >> + action="store_true") >> + parser.add_argument("--u-boot", help="parse the u-boot configs", >> + action="store_true") >> + parser.add_argument("--spl", help="parse the SPL configs", >> + action="store_true") >> + parser.add_argument("--tpl", help="parse the TPL configs", >> + action="store_true") >> + parser.add_argument("--diff", >> + help="show only the options that differs between the selected configs (SPL, TPL, u-boot)", >> + action="store_true") >> + parser.add_argument("--rebuild-db", >> + help="Force a rebuild of the config database", >> + action="store_true") >> + parser.add_argument('-j', '--jobs', >> + help='the number of jobs to run simultaneously') >> + parser.add_argument('-o', '--output', >> + help='The output CSV filename. uses stdout if not specified') >> + parser.add_argument('--no-header', help='Do not put the header at the top', >> + action="store_true") >> + parser.add_argument('--discard-empty', action="store_true", >> + help='Discard the empty rows (defconfigs that do not enable at least one option)') >> + >> + find_defconfigs.update_parser_with_default_options(parser) >> + args = parser.parse_args() >> + >> + # generate db file if needed or requested >> + # The job of generating the db is actually done by tools/moveconfig.py >> + # (called with -B) >> + if args.rebuild_db or not os.path.isfile(CONFIG_DATABASE): >> + find_defconfig_args = ["--{} '{}'".format(f, getattr(args, f)) >> + for f in find_defconfigs.get_default_options() >> + if getattr(args, f)] >> + if args.jobs: >> + jobs_option = "-j {}".format(args.jobs) >> + else: >> + jobs_option = "" >> + >> + rc = os.system( >> + "tools/find_defconfigs.py {} | tools/moveconfig.py -B {} -d - 1>&2 " >> + .format(" ".join(find_defconfig_args), jobs_option)) >> + if rc: >> + sys.exit(1) > This is fine, but I wonder why you don't import this module and call > it directly? It could return a dict, perhaps? I tried at first but ended up with a lot of changes in moveconfig.py. moveconfig.py has evolved and grown in complexity overtime. It could be split into small independent programs using a generic core to build and parse the configs. I am not sure that it is worth it though. >> + >> + # get a list of defconfigs matching the rules >> + targets = [t for t in find_defconfigs.get_matching_boards(args)] >> + defconfigs = [t.defconfig for t in targets] >> + >> + # create a list of binary types we are interested in >> + binary_types = [] >> + if args.spl: >> + binary_types.append("SPL") >> + if args.tpl: >> + binary_types.append("TPL") >> + if args.u_boot or not binary_types: >> + binary_types.append("u-boot") >> + >> + # define a function used to filter on the options >> + rules = [re.compile(" {}=".format(cfg_opt)) >> + for cfg_opt in args.options] >> + >> + def match_any_rule(line): >> + for r in rules: >> + if r.match(line): >> + return True >> + return False >> + >> + # read the database >> + db = read_db(defconfigs, match_any_rule, binary_types) >> + >> + target_dict = {} >> + for t in targets: >> + target_dict[t.defconfig] = t >> + >> + def populate_left_columns(target=None, dic=None, header=True): >> + if header: >> + return ["vendor", "soc", "defconfig", "type"] >> + else: >> + dic[target]["vendor"] = target_dict[target.defconfig].vendor >> + dic[target]["soc"] = target_dict[target.defconfig].soc >> + dic[target]["defconfig"] = target.defconfig >> + dic[target]["type"] = target.binary_type >> + >> + def populate_left_columns_diff(target=None, dic=None, header=True): >> + if header: >> + return ["vendor", "soc", "defconfig"] >> + else: >> + dic[target]["vendor"] = target_dict[target.defconfig].vendor >> + dic[target]["soc"] = target_dict[target.defconfig].soc >> + dic[target]["defconfig"] = target.defconfig >> + >> + if args.output: >> + out = open(args.output, "w") >> + else: >> + out = sys.stdout >> + >> + if args.diff: >> + db.diff().output_csv(output=out, show_X=False, >> + header=not args.no_header, >> + discard_empty_rows=args.discard_empty, >> + left_columns=populate_left_columns_diff) >> + else: >> + db.output_csv(output=out, show_X=args.X, header=not args.no_header, >> + discard_empty_rows=args.discard_empty, >> + left_columns=populate_left_columns) >> + >> + if out != sys.stdout: >> + out.close() >> + >> +if __name__ == '__main__': >> + main() >> -- >> 2.7.4 >> > Regards, > Simon >