On 22 Nov 2021, at 12:22, Adrian Moreno wrote:

> In order to minimize the risk of having the python flow parsing code and
> the C flow formatting code divert, add a target that checks if the
> formatting code has been changed since the last revision and warn the
> developer if it has.
>
> The script also makes it easy to update the dependency file so hopefully
> it will not cause too much trouble for a developer that has modifed the
> file without changing the flow string format.
>
> Signed-off-by: Adrian Moreno <[email protected]>
> ---
>  .gitignore                      |   1 +
>  python/automake.mk              |  13 +++-
>  python/build/flow-parse-deps.py | 101 ++++++++++++++++++++++++++++++++
>  python/ovs/flows/deps.py        |   5 ++
>  4 files changed, 118 insertions(+), 2 deletions(-)
>  create mode 100755 python/build/flow-parse-deps.py
>  create mode 100644 python/ovs/flows/deps.py
>
> diff --git a/.gitignore b/.gitignore
> index f1cdcf124..e6bca1cd2 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -79,3 +79,4 @@ testsuite.tmp.orig
>  /Documentation/_build
>  /.venv
>  /cxx-check
> +/flowparse-deps-check
> diff --git a/python/automake.mk b/python/automake.mk
> index 21aa897f2..da6ef08b4 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -50,7 +50,8 @@ ovs_pyfiles = \
>       python/ovs/flows/ofp.py \
>       python/ovs/flows/ofp_act.py \
>       python/ovs/flows/odp.py \
> -     python/ovs/flows/filter.py
> +     python/ovs/flows/filter.py \
> +     python/ovs/flows/deps.py

Add in alphabetical order

>
>  # These python files are used at build time but not runtime,
>  # so they are not installed.
> @@ -58,7 +59,8 @@ EXTRA_DIST += \
>       python/build/__init__.py \
>       python/build/nroff.py \
>       python/build/soutil.py \
> -     python/build/extract_ofp_fields.py
> +     python/build/extract_ofp_fields.py \
> +     python/build/flow-parse-deps.py
>

Add in alphabetical order, and also add it to FLAKE8_PYFILES.

>  # PyPI support.
>  EXTRA_DIST += \
> @@ -135,3 +137,10 @@ $(srcdir)/python/ovs/flows/ofp_fields.py: 
> $(srcdir)/build-aux/gen_ofp_field_deco
>  EXTRA_DIST += python/ovs/flows/ofp_fields.py
>  CLEANFILES += python/ovs/flows/ofp_fields.py
>
> +ALL_LOCAL += flowparse-deps-check
> +DEPS = $(shell $(AM_V_GEN)$(run_python) 
> $(srcdir)/python/build/flow-parse-deps.py list)
> +flowparse-deps-check: $(srcdir)/python/build/flow-parse-deps.py $(DEPS)
> +     echo $(DEPS)
> +     $(AM_V_GEN)$(run_python) $(srcdir)/python/build/flow-parse-deps.py check
> +     touch $@
> +CLEANFILES += flowparse-deps-check
> diff --git a/python/build/flow-parse-deps.py b/python/build/flow-parse-deps.py
> new file mode 100755
> index 000000000..9adb1d89c
> --- /dev/null
> +++ b/python/build/flow-parse-deps.py
> @@ -0,0 +1,101 @@
> +#!/usr/bin/env python3
> +# Copyright (c) 2021 Red Hat, Inc.
> +#
> +# Licensed under the Apache License, Version 2.0 (the "License");
> +# you may not use this file except in compliance with the License.
> +# You may obtain a copy of the License at:
> +#
> +#     http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> +# See the License for the specific language governing permissions and
> +# limitations under the License.
> +
> +# Breaks lines read from stdin into groups using blank lines as
> +# group separators, then sorts lines within the groups for
> +# reproducibility.
> +
> +
> +# ovs-test-ofparse is just a wrapper around ovs-ofctl
> +# that also runs the python flow parsing utility to check that flows are
> +# parseable
> +
> +import hashlib
> +import sys
> +
> +DEPENDENCIES = ["lib/ofp-actions.c", "lib/odp-util.c"]
> +DEPENDENCY_FILE = "python/ovs/flows/deps.py"
> +
> +
> +def usage():
> +    print(
> +        """
> +Usage {cmd} [check | update | list]
> +Tool to verify flow parsing python code is kept in sync with
> +flow printing C code.
> +
> +Commands:
> +  check: check the dependencies are met
> +  update: update the dependencies based on current file content
> +  list: list the dependency files

Nit, as my OCD kicked in. Can you align (if not, that’s fine, I can try to 
suppress my OCD ;)


  check:  check the dependencies are met
  update: update the dependencies based on current file content
  list:   list the dependency files

> +""".format(
> +            cmd=sys.argv[0]
> +        )
> +    )
> +
> +
> +def digest(filename):
> +    with open(filename, "rb") as f:
> +        return hashlib.md5(f.read()).hexdigest()
> +
> +
> +def main():
> +    if len(sys.argv) != 2:
> +        usage()
> +        sys.exit(1)
> +
> +    if sys.argv[1] == "list":
> +        print(" ".join(DEPENDENCIES))
> +    elif sys.argv[1] == "update":
> +        dep_str = list()
> +        for dep in DEPENDENCIES:
> +            dep_str.append(
> +                '    "{dep}": "{digest}"'.format(dep=dep, digest=digest(dep))
> +            )
> +
> +        depends = """# File automatically generated. Do not modify manually

       depends = """# File automatically generated. Do not modify manually!

> +dependencies = {{
> +{dependencies_dict}
> +}}""".format(
> +            dependencies_dict=",\n".join(dep_str)
> +        )
> +        with open(DEPENDENCY_FILE, "w") as f:
> +            print(depends, file=f)
> +
> +    elif sys.argv[1] == "check":
> +        from ovs.flows.deps import dependencies
> +
> +        for dep in DEPENDENCIES:
> +            expected = dependencies.get(dep)
> +            if not expected or expected != digest(dep):
> +                print(
> +                    """
> +Dependency file {dep} has changed.
> +Please verify the flow output format has not changed or modify the python 
> flow parsing code accordingly.

Please verify the flow output format has not changed.
If it has changed, modify the python flow parsing code accordingly.

> +
> +Once you're done, update the dependencies by running '{cmd} update' and 
> check in the new dependency file.

Please fix line length here to <= 79, so flake8 is not complaining.

> +""".format(
> +                        dep=dep,
> +                        cmd=sys.argv[0],
> +                    )
> +                )
> +                return 2
> +    else:
> +        usage()
> +        sys.exit(1)
> +
> +
> +if __name__ == "__main__":
> +    sys.exit(main())
> diff --git a/python/ovs/flows/deps.py b/python/ovs/flows/deps.py
> new file mode 100644
> index 000000000..aaf428917
> --- /dev/null
> +++ b/python/ovs/flows/deps.py
> @@ -0,0 +1,5 @@
> +# File automatically generated. Do not modify manually
> +dependencies = {
> +    "lib/ofp-actions.c": "f108b3e119f03b3373064589aecdeaf0",
> +    "lib/odp-util.c": "c946c21d8f644869ce778842167f9129"
> +}
> -- 

Running check from from the root does not work, maybe we can fix the ovs path 
to make it work from here?

$ ./python/build/flow-parse-deps.py check
Traceback (most recent call last):
  File 
"/home/echaudron/Documents/review/ovs_amorenoz_python/./python/build/flow-parse-deps.py",
 line 101, in <module>
    sys.exit(main())
  File 
"/home/echaudron/Documents/review/ovs_amorenoz_python/./python/build/flow-parse-deps.py",
 line 78, in main
    from ovs.flows.deps import dependencies
ModuleNotFoundError: No module named 'ovs'

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to