* [dtschema PATCH] tools: Add tool to compare schemas for ABI changes
@ 2023-11-09 23:12 Rob Herring
2023-12-02 18:33 ` Simon Glass
0 siblings, 1 reply; 2+ messages in thread
From: Rob Herring @ 2023-11-09 23:12 UTC (permalink / raw)
To: devicetree-spec
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@kernel.org>
---
pyproject.toml | 1 +
tools/dt-cmp-schema | 135 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 136 insertions(+)
create mode 100755 tools/dt-cmp-schema
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)
+
+
+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)
+
+ 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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [dtschema PATCH] tools: Add tool to compare schemas for ABI changes
2023-11-09 23:12 [dtschema PATCH] tools: Add tool to compare schemas for ABI changes Rob Herring
@ 2023-12-02 18:33 ` Simon Glass
0 siblings, 0 replies; 2+ messages in thread
From: Simon Glass @ 2023-12-02 18:33 UTC (permalink / raw)
To: Rob Herring; +Cc: devicetree-spec
Hi Rob,
On Thu, 9 Nov 2023 at 16:12, Rob Herring <robh@kernel.org> 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@kernel.org>
> ---
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-12-02 18:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-09 23:12 [dtschema PATCH] tools: Add tool to compare schemas for ABI changes Rob Herring
2023-12-02 18:33 ` Simon Glass
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).