From dc405758304f871810853e691857fb684a1909c2 Mon Sep 17 00:00:00 2001 From: Konrad Weihmann Date: Sat, 6 Jan 2024 13:12:22 +0100 Subject: [PATCH] rules: fix poor performance on larger files - avoid multiple calls to stash.GetItemsFor - pre-filter items as much as possible - call parser functions only once for static items (e.g. get_valid_package_names) - use sets if appropriate Closes #476 Signed-off-by: Konrad Weihmann --- oelint_adv/rule_base/rule_var_misspell.py | 6 +- .../rule_base/rule_vars_filessetting.py | 63 ++++++++++--------- .../rule_base/rule_vars_pathhardcode.py | 13 ++-- oelint_adv/rule_base/rule_vars_suggested.py | 15 +++-- .../rule_base/rule_vars_variable_override.py | 8 +-- 5 files changed, 52 insertions(+), 53 deletions(-) diff --git a/oelint_adv/rule_base/rule_var_misspell.py b/oelint_adv/rule_base/rule_var_misspell.py index 758374fd..28df4171 100644 --- a/oelint_adv/rule_base/rule_var_misspell.py +++ b/oelint_adv/rule_base/rule_var_misspell.py @@ -22,13 +22,13 @@ def get_best_match(self, item, _list, minconfidence=0.5): def check(self, _file, stash): res = [] - items = stash.GetItemsFor(filename=_file, classifier=Variable.CLASSIFIER, - attribute=Variable.ATTR_VAR) + items = stash.GetItemsFor(filename=_file, classifier=Variable.CLASSIFIER) _all = stash.GetItemsFor(filename=_file) _extras = [f'SRCREV_{x}' for x in get_valid_named_resources(stash, _file)] + _pkgs = get_valid_package_names(stash, _file, strippn=True) for i in items: _cleanvarname = i.VarName - for pkg in get_valid_package_names(stash, _file, strippn=True): + for pkg in _pkgs: if not pkg: continue # pragma: no cover if _cleanvarname.endswith(pkg): diff --git a/oelint_adv/rule_base/rule_vars_filessetting.py b/oelint_adv/rule_base/rule_vars_filessetting.py index 1382723b..40c3367a 100644 --- a/oelint_adv/rule_base/rule_vars_filessetting.py +++ b/oelint_adv/rule_base/rule_vars_filessetting.py @@ -9,11 +9,9 @@ def __init__(self): message='Check for improvable FILES settings', appendix=['hidden', 'double']) - def __find_match_from_stash(self, _file, stash, variable_, needle, msg, appendix, onappendonly=False): + def __find_match_from_stash(self, _files, variable_, needle, msg, appendix, onappendonly=False): res = [] - items = stash.GetItemsFor(filename=_file, classifier=Variable.CLASSIFIER, - attribute=Variable.ATTR_VAR, attributeValue='FILES') - for i in items: + for i in _files: if variable_ in i.SubItems and 'remove' not in i.SubItems and needle in i.VarValue: # pragma: no cover if (onappendonly and i.IsAppend()) or (not onappendonly): res += self.finding(i.Origin, i.InFileLine, @@ -22,31 +20,34 @@ def __find_match_from_stash(self, _file, stash, variable_, needle, msg, appendix def check(self, _file, stash): res = [] - _expanded = stash.ExpandVar( - filename=_file, attribute=Variable.ATTR_VAR) - for ov in ["_", ":"]: - _seenpath = {} - for p in _expanded['PACKAGES']: - _files = 'FILES{o}{a}'.format(o=ov, a=p) - _convfiles = _files.replace(_expanded['PN'][0], '${PN}') - if _files in _expanded: - _pattern = _expanded[_files] - for _p in _pattern: - # double setting in FILES - if len([x for x in _pattern if x == _p]) > 1: - # try to find with both unexpanded and expanded values - res += self.__find_match_from_stash(_file, stash, p.replace(_expanded['PN'][0], '${PN}'), _p, - '{a} is already set by default or in this recipe'.format(a=_p), 'double', True) - res += self.__find_match_from_stash(_file, stash, p, _p, - '{a} is already set by default or in this recipe'.format(a=_p), 'double', True) - # useless as hidden by previous package - if _p in _seenpath.keys() and _seenpath[_p] != _convfiles: - # try to find with both unexpanded and expanded values - res += self.__find_match_from_stash(_file, stash, p.replace(_expanded['PN'][0], '${PN}'), _p, - '{a} is already covered by {b}'.format(a=_p, b=_seenpath[_p]), - 'hidden') - res += self.__find_match_from_stash(_file, stash, p, _p, - '{a} is already covered by {b}'.format(a=_p, b=_seenpath[_p]), - 'hidden') - _seenpath[_p] = _convfiles + _expanded = stash.ExpandVar(filename=_file, attribute=Variable.ATTR_VAR, attributeValue='PACKAGES') + _files = stash.GetItemsFor(filename=_file, classifier=Variable.CLASSIFIER, + attribute=Variable.ATTR_VAR, attributeValue='FILES') + if not any(_files): + return res + override_delimiter = _files[-1].OverrideDelimiter + _seenpath = {} + for p in _expanded['PACKAGES']: + _files = 'FILES{o}{a}'.format(o=override_delimiter, a=p) + _convfiles = _files.replace(_expanded['PN'][0], '${PN}') + if _files in _expanded: + _pattern = _expanded[_files] + for _p in _pattern: + # double setting in FILES + if len([x for x in _pattern if x == _p]) > 1: + # try to find with both unexpanded and expanded values + res += self.__find_match_from_stash(_file, _files, p.replace(_expanded['PN'][0], '${PN}'), _p, + '{a} is already set by default or in this recipe'.format(a=_p), 'double', True) + res += self.__find_match_from_stash(_file, _files, p, _p, + '{a} is already set by default or in this recipe'.format(a=_p), 'double', True) + # unless as hidden by previous package + if _p in _seenpath.keys() and _seenpath[_p] != _convfiles: + # try to find with both unexpanded and expanded values + res += self.__find_match_from_stash(_file, _files, p.replace(_expanded['PN'][0], '${PN}'), _p, + '{a} is already covered by {b}'.format(a=_p, b=_seenpath[_p]), + 'hidden') + res += self.__find_match_from_stash(_file, _files, p, _p, + '{a} is already covered by {b}'.format(a=_p, b=_seenpath[_p]), + 'hidden') + _seenpath[_p] = _convfiles return res diff --git a/oelint_adv/rule_base/rule_vars_pathhardcode.py b/oelint_adv/rule_base/rule_vars_pathhardcode.py index 53d5f08b..6a2392c7 100644 --- a/oelint_adv/rule_base/rule_vars_pathhardcode.py +++ b/oelint_adv/rule_base/rule_vars_pathhardcode.py @@ -1,8 +1,7 @@ from collections import OrderedDict from oelint_adv.cls_rule import Rule -from oelint_parser.cls_item import Comment -from oelint_parser.cls_item import Variable +from oelint_parser.cls_item import Variable, Export, Function, FunctionExports, PythonBlock from oelint_parser.rpl_regex import RegexRpl @@ -34,12 +33,14 @@ def __init__(self): def check(self, _file, stash): res = [] - items = stash.GetItemsFor(filename=_file) + items = stash.GetItemsFor(filename=_file, + classifier=[Variable.CLASSIFIER, Export.CLASSIFIER, + Function.CLASSIFIER, FunctionExports.CLASSIFIER, + PythonBlock.CLASSIFIER]) for i in items: - if isinstance(i, Variable) and i.VarName in ['SUMMARY', 'DESCRIPTION', 'HOMEPAGE', 'AUTHOR', 'BUGTRACKER', 'FILES']: - continue - if isinstance(i, Comment): + if isinstance(i, Variable) and i.VarName in ['SUMMARY', 'DESCRIPTION', 'HOMEPAGE', 'AUTHOR', + 'BUGTRACKER', 'FILES', 'CVE_STATUS']: continue _matches = [] for k, v in self._map.items(): diff --git a/oelint_adv/rule_base/rule_vars_suggested.py b/oelint_adv/rule_base/rule_vars_suggested.py index 24133d23..79d35358 100644 --- a/oelint_adv/rule_base/rule_vars_suggested.py +++ b/oelint_adv/rule_base/rule_vars_suggested.py @@ -1,6 +1,7 @@ from oelint_adv.cls_rule import Rule from oelint_parser.cls_item import Variable from oelint_parser.constants import CONSTANTS +from oelint_parser.helper_files import is_packagegroup class VarSuggestedExists(Rule): @@ -13,20 +14,18 @@ def __init__(self): def check(self, _file, stash): res = [] - _is_pkg_group = False - for i in stash.GetItemsFor(filename=_file, classifier=Variable.CLASSIFIER, - attribute=Variable.ATTR_VAR, attributeValue='inherit'): - if any(x == 'packagegroup' for x in i.get_items()): - _is_pkg_group = True - break + _is_pkg_group = is_packagegroup(stash, _file) + all_items = stash.GetItemsFor(filename=_file, + classifier=Variable.CLASSIFIER, + attribute=Variable.ATTR_VAR, + attributeValue=CONSTANTS.VariablesSuggested) for var_ in CONSTANTS.VariablesSuggested: if var_ == 'BBCLASSEXTEND': # this is better covered by oelint_adv/rule_base/rule_vars_bbclassextends.py continue if _is_pkg_group and var_ in ['LICENSE', 'CVE_PRODUCT']: continue - items = stash.GetItemsFor( - filename=_file, classifier=Variable.CLASSIFIER, attribute=Variable.ATTR_VAR, attributeValue=var_) + items = [x for x in all_items if x.VarName == var_] if not any(items): res += self.finding(_file, 0, 'Variable \'{var}\' should be set'.format(var=var_), appendix=var_) return res diff --git a/oelint_adv/rule_base/rule_vars_variable_override.py b/oelint_adv/rule_base/rule_vars_variable_override.py index b5b207d6..a2ba95ba 100644 --- a/oelint_adv/rule_base/rule_vars_variable_override.py +++ b/oelint_adv/rule_base/rule_vars_variable_override.py @@ -12,14 +12,12 @@ def __init__(self): def check(self, _file, stash): res = [] - __varnames = [x.VarName for x in stash.GetItemsFor( - filename=_file, classifier=Variable.CLASSIFIER)] - for v in __varnames: + _all = [x for x in stash.GetItemsFor(filename=_file, classifier=Variable.CLASSIFIER)] + for v in set(x.VarName for x in _all): if v == 'inherit' or not v: # This will be done by another rule continue # pragma: no cover - items = stash.GetItemsFor( - filename=_file, classifier=Variable.CLASSIFIER, attribute=Variable.ATTR_VAR, attributeValue=v) + items = [x for x in _all if x.VarName == v] items = sorted(items, key=lambda x: x.Line, reverse=False) for sub in [x.SubItem for x in items]: # Get all entries but not the only that do immediate expansion,