Hi Rob, On Thu, 9 Nov 2023 at 16:12, Rob Herring <robh@xxxxxxxxxx> wrote: > > Add a new tool which compares 2 sets of schemas for possible ABI changes. > It's not complete nor 100% accurate, but it's a start. > > This checks for the following kinds of changes: > > - New required properties > - Minimum number of entries required increased > - Removed properties > > Limitations: > > Restructuring of schemas may result in false positives or missed ABI > changes. There's some support if a property moves from a schema to a > referenced schema. > > Schemas underneath logic keywords (allOf, oneOf, anyOf) or if/then/else schemas > are not handled. > > Signed-off-by: Rob Herring <robh@xxxxxxxxxx> > --- > pyproject.toml | 1 + > tools/dt-cmp-schema | 135 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 136 insertions(+) > create mode 100755 tools/dt-cmp-schema > Would it be possible to add some comments, a test and some documentation updates? > diff --git a/pyproject.toml b/pyproject.toml > index 6fc9bb7ae4b9..da5fcca421ce 100644 > --- a/pyproject.toml > +++ b/pyproject.toml > @@ -36,6 +36,7 @@ Source="https://github.com/devicetree-org/dt-schema"; > > [tool.setuptools] > script-files = [ > + 'tools/dt-cmp-schema', > 'tools/dt-check-compatible', > 'tools/dt-validate', > 'tools/dt-doc-validate', > diff --git a/tools/dt-cmp-schema b/tools/dt-cmp-schema > new file mode 100755 > index 000000000000..25ac3dd05274 > --- /dev/null > +++ b/tools/dt-cmp-schema > @@ -0,0 +1,135 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: BSD-2-Clause > +# Copyright 2023 Arm Ltd. > + > +import sys > +import argparse > +import signal > +import urllib > + > +import dtschema > + > + > +def _sigint_handler(signum, frame): > + sys.exit(-2) Shouldn't this be 2 ? > + > + > +signal.signal(signal.SIGINT, _sigint_handler) > + > + > +def path_list_to_str(path): > + return '/' + '/'.join(path) > + > + > +def prop_generator(schema, path=[]): > + if not isinstance(schema, dict): > + return > + for prop_key in ['properties', 'patternProperties']: > + if prop_key in schema: > + for p, sch in schema[prop_key].items(): > + yield path + [prop_key, p], sch > + yield from prop_generator(sch, path=path + [prop_key, p]) > + > + > +def _ref_to_id(id, ref): > + ref = urllib.parse.urljoin(id, ref) > + if '#/' not in ref: > + ref += '#' > + return ref > + > + > +def _prop_in_schema(prop, schema, schemas): > + for p, sch in prop_generator(schema): > + if p[1] == prop: > + return True > + > + if 'allOf' in schema: > + for e in schema['allOf']: > + if '$ref' in e: > + ref_id = _ref_to_id(schema['$id'], e['$ref']) > + if ref_id in schemas: > + if _prop_in_schema(prop, schemas[ref_id], schemas): > + return True > + > + if '$ref' in schema: > + ref_id = _ref_to_id(schema['$id'], schema['$ref']) > + if ref_id in schemas and _prop_in_schema(prop, schemas[ref_id], schemas): > + return True > + > + return False > + > + > +def check_removed_property(id, base, schemas): > + for p, sch in prop_generator(base): > + if not _prop_in_schema(p[1], schemas[id], schemas): > + print(f'{id}{path_list_to_str(p)}: existing property removed\n', file=sys.stderr) > + > + > +def schema_get_from_path(sch, path): > + for p in path: > + try: > + sch = sch[p] > + except: > + return None > + return sch > + > + > +def check_new_items(id, base, new, path=[]): > + for p, sch in prop_generator(new): > + if not isinstance(sch, dict) or 'minItems' not in sch: > + continue > + > + min = sch['minItems'] > + base_min = schema_get_from_path(base, p + ['minItems']) > + > + if base_min and min > base_min: > + print(f'{id}{path_list_to_str(p)}: new required entry added\n', file=sys.stderr) > + > + > +def _check_required(id, base, new, path=[]): > + if not isinstance(base, dict) or not isinstance(new, dict): > + return > + > + if 'required' not in new: > + return > + > + if 'required' not in base: > + print(f'{id}{path_list_to_str(path)}: new required properties added: {", ".join(new["required"])}\n', file=sys.stderr) > + return > + > + diff = set(new['required']) - set(base['required']) > + if diff: > + print(f'{id}{path_list_to_str(path)}: new required properties added: {", ".join(diff)}\n', file=sys.stderr) > + return > + > + > +def check_required(id, base, new): > + _check_required(id, base, new) > + > + for p, sch in prop_generator(new): > + _check_required(id, schema_get_from_path(base, p), sch, path=p) > + > + > +if __name__ == "__main__": > + ap = argparse.ArgumentParser(description="Compare 2 sets of schemas for possible ABI differences") > + ap.add_argument("baseline", type=str, > + help="Baseline schema directory or preprocessed schema file") > + ap.add_argument("new", type=str, > + help="New schema directory or preprocessed schema file") > + ap.add_argument('-V', '--version', help="Print version number", > + action="version", version=dtschema.__version__) > + args = ap.parse_args() > + > + base_schemas = dtschema.DTValidator([args.baseline]).schemas > + schemas = dtschema.DTValidator([args.new]).schemas > + > + if not schemas or not base_schemas: > + sys.exit(-1) You can use required=True in add_argument(). Otherwise I suggest raise() or adding a message so the user can see what is wrong. > + > + for id, sch in schemas.items(): > + if id not in base_schemas or 'generated' in id: > + continue > + > + check_required(id, base_schemas[id], sch) > + check_removed_property(id, base_schemas[id], schemas) > + check_new_items(id, base_schemas[id], sch) > -- > 2.42.0 > > Regards, Simon
