|
| From: | John Snow |
| Subject: | Re: [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking |
| Date: | Thu, 25 Mar 2021 13:48:34 -0400 |
| User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 3/25/21 2:18 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:On 3/24/21 1:57 AM, Markus Armbruster wrote:John Snow <jsnow@redhat.com> writes:On 3/23/21 5:40 AM, Markus Armbruster wrote:Naming rules differ for the various kinds of names. To prepare enforcing them, define functions to check them: check_name_upper(), check_name_lower(), and check_name_camel(). For now, these merely wrap around check_name_str(), but that will change shortly. Replace the other uses of check_name_str() by appropriate uses of the wrappers. No change in behavior just yet. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi/expr.py | 51 +++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index e00467636c..30285fe334 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -21,11 +21,12 @@ from .error import QAPISemError -# Names must be letters, numbers, -, and _. They must start with letter, -# except for downstream extensions which must start with __RFQDN_. -# Dots are only valid in the downstream extension prefix. -valid_name = re.compile(r'^(__[a-zA-Z0-9.-]+_)?' - '[a-zA-Z][a-zA-Z0-9_-]*$') +# Names consist of letters, digits, -, and _, starting with a letter. +# An experimental name is prefixed with x-. A name of a downstream +# extension is prefixed with __RFQDN_. The latter prefix goes first. +valid_name = re.compile(r'(__[a-z0-9.-]+_)?' + r'(x-)?' + r'([a-z][a-z0-9_-]*)$', re.IGNORECASE) def check_name_is_str(name, info, source): @@ -37,16 +38,38 @@ def check_name_str(name, info, source, permit_upper=False): # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty' # and 'q_obj_*' implicit type names. - if not valid_name.match(name) or \ - c_name(name, False).startswith('q_'): + match = valid_name.match(name) + if not match or c_name(name, False).startswith('q_'): raise QAPISemError(info, "%s has an invalid name" % source) if not permit_upper and name.lower() != name: raise QAPISemError( info, "%s uses uppercase in name" % source) + return match.group(3) + + +def check_name_upper(name, info, source): + stem = check_name_str(name, info, source, permit_upper=True) + # TODO reject '[a-z-]' in @stem +Creates (presumably) temporary errors in flake8 for the dead assignment here and below.All gone by the end of the series. "make check" and checkpatch were content. Anything else you'd like me to run?Eventually it'll be part of CI, with targets to run locally. I never expected the process to take this long, so I did not invest my time in developing an interim solution. I use a hastily written script to do my own testing, which I run for every commit that touches QAPI: #!/usr/bin/env bash set -e if [[ -f qapi/.flake8 ]]; then echo "flake8 --config=qapi/.flake8 qapi/" flake8 --config=qapi/.flake8 qapi/ fi if [[ -f qapi/pylintrc ]]; then echo "pylint --rcfile=qapi/pylintrc qapi/" pylint --rcfile=qapi/pylintrc qapi/ fi if [[ -f qapi/mypy.ini ]]; then echo "mypy --config-file=qapi/mypy.ini qapi/" mypy --config-file=qapi/mypy.ini qapi/ fi if [[ -f qapi/.isort.cfg ]]; then pushd qapi echo "isort -c ." isort -c . popd fi pushd ../bin/git make -j9 make check-qapi-schema popdThanks for sharing this!
My intent, eventually, was to get scripts/qapi under python/qemu/qapi; under which there is a testing framework thing I have been debating on and off with Cleber for a while that does (basically) the same thing as what my hasty script does, but more integrated with Python ecosystem.
It'll do a few things: (1) Gets these checks on CI (2) Allows developers to manually run the checks locally(3) Allows developers to manually run the checks locally using a pre-determined set of pinned linter versions to guarantee synchronicity with CI
cd python/qemu && "make check" would do more or less the same as the hacky script above, "make venv-check" would create a virtual environment with precisely the right packages and then run the same.
I have been stalled there for a while, and I missed freeze deadline from illness. Anguish. For now, the dumb script in my scripts directory does the lifting I need it to, using packages I have selected in my local environment that just-so-happen to pass.
Apropos qapi-gen testing scripts. I have scripts to show me how the generated code changes along the way in a branch. They evolved over a long time, and try to cope with changes in the tree that are hardly relevant anymore. By now, they could quite possibly make Frankenstein recoil in horror.
Are they in the tree? Largely if the generated code changes it's invisible to me, but I rely heavily on the unit tests. I guess maybe if they are not in a state to upstream it might not be worth the hassle to clean them, but I don't know.
As a secondary purpose, the scripts show me how output of pycodestyle-3 and pylint change. This would be uninteresting if the code in master was clean against a useful configuration of these tools. Your work has been making it less interesting.
--js
| [Prev in Thread] | Current Thread | [Next in Thread] |