| Message ID | 20170501194443.10029-2-aconole@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, May 01, 2017 at 03:44:37PM -0400, Aaron Conole wrote: > Developers wishing to add checks to checkpatch sift through an adhoc mess, > currently. The process goes something like: > 1. Figure out what to test in the patch > 2. Write some code, quickly, that checks for that condition > 3. Look through the statemachine to find where the check should go > 4. ignore parts of the above and just throw something together > > That worked fine for the initial development, but as interesting new tests > are developed, it is important to have a more flexible framework that lets > a developer just plug in a new test, easily. > > This commit brings in a new framework that allows plugging in checks very > quickly. Hook up the line-length test as an initial demonstration. > > Signed-off-by: Aaron Conole <aconole@redhat.com> With this, I get: ../utilities/checkpatch.py:180:12: E999 SyntaxError: invalid syntax where the cited line is: lambda(x): print_warning("Line is greater than 79-characters long", x)} I'm using: $ flake8 --version 3.2.1 (mccabe: 0.5.3, pycodestyle: 2.2.0, pyflakes: 1.3.0) CPython 3.5.1+ on Linux Thanks, Ben.
Ben Pfaff <blp@ovn.org> writes: > On Mon, May 01, 2017 at 03:44:37PM -0400, Aaron Conole wrote: >> Developers wishing to add checks to checkpatch sift through an adhoc mess, >> currently. The process goes something like: >> 1. Figure out what to test in the patch >> 2. Write some code, quickly, that checks for that condition >> 3. Look through the statemachine to find where the check should go >> 4. ignore parts of the above and just throw something together >> >> That worked fine for the initial development, but as interesting new tests >> are developed, it is important to have a more flexible framework that lets >> a developer just plug in a new test, easily. >> >> This commit brings in a new framework that allows plugging in checks very >> quickly. Hook up the line-length test as an initial demonstration. >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> > > With this, I get: > > ../utilities/checkpatch.py:180:12: E999 SyntaxError: invalid syntax > > where the cited line is: > > lambda(x): print_warning("Line is greater than 79-characters long", x)} d'oh! Trying it with python3, I see: File "./utilities/checkpatch.py", line 180 lambda(x): print_warning("Line is greater than 79-characters long", x)} ^ SyntaxError: invalid syntax So, I guess that is it. I'll submit a fix. I think I broke my python setup while doing some hacking on a different project, so I'll have to be more careful. Maybe now is the time to learn virtualenv. > I'm using: > > $ flake8 --version > 3.2.1 (mccabe: 0.5.3, pycodestyle: 2.2.0, pyflakes: 1.3.0) CPython 3.5.1+ on Linux Yep, that's different from my version: $ flake8 --version 2.5.5 (pep8: 1.6.2, mccabe: 0.2.1, pyflakes: 1.2.3, hacking.core: 0.0.1, ProxyChecker: 0.0.1) CPython 2.7.13 on Linux And I don't get that error flagged: 03:53:13 aconole {checkpatch_2} ~/git/ovs$ git show HEAD | grep checkpatch | head -n1 checkpatch: introduce a flexible framework 03:54:04 aconole {checkpatch_2} ~/git/ovs$ touch utilities/checkpatch.py 03:54:20 aconole {checkpatch_2} ~/git/ovs$ make flake8-check \ src='Documentation/conf.py ofproto/ipfix-gen-entities utilities/ovs-pcap.in utilities/checkpatch.py utilities/ovs-dev.py utilities/ovs-tcpdump.in utilities/bugtool/ovs-bugtool.in tests/appctl.py tests/test-daemon.py tests/test-json.py tests/test-jsonrpc.py tests/test-l7.py tests/test-ovsdb.py tests/test-reconnect.py tests/MockXenAPI.py tests/test-unix-socket.py tests/test-unixctl.py tests/test-vlog.py xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync python/ovs/__init__.py python/ovs/daemon.py python/ovs/fcntl_win.py python/ovs/db/__init__.py python/ovs/db/data.py python/ovs/db/error.py python/ovs/db/idl.py python/ovs/db/parser.py python/ovs/db/schema.py python/ovs/db/types.py python/ovs/fatal_signal.py python/ovs/json.py python/ovs/jsonrpc.py python/ovs/ovsuuid.py python/ovs/poller.py python/ovs/process.py python/ovs/reconnect.py python/ovs/socket_util.py python/ovs/stream.py python/ovs/timeval.py python/ovs/unixctl/__init__.py python/ovs/unixctl/client.py python/o vs/unixctl/server.py python/ovs/util.py python/ovs/version.py python/ovs/vlog.py python/ovs/winutils.py python/ovstest/__init__.py python/ovstest/args.py python/ovstest/rpcserver.py python/ovstest/tcp.py python/ovstest/tests.py python/ovstest/udp.py python/ovstest/util.py python/ovstest/vswitch.py python/setup.py python/build/__init__.py python/build/nroff.py python/ovs/dirs.py.template vtep/ovs-vtep build-aux/xml2nroff' && \ flake8 $src --select=H231,H232,H233,H238 && \ flake8 $src --ignore=E121,E123,E125,E126,E127,E128,E129,E131,W503,F811,D,H,I && \ touch flake8-check 03:54:24 aconole {checkpatch_2} ~/git/ovs$ I'll look into fixing up my setup, but for now apologies for the back-and-forth. > Thanks, > > Ben.
On Mon, May 01, 2017 at 04:01:12PM -0400, Aaron Conole wrote: > Ben Pfaff <blp@ovn.org> writes: > > > On Mon, May 01, 2017 at 03:44:37PM -0400, Aaron Conole wrote: > >> Developers wishing to add checks to checkpatch sift through an adhoc mess, > >> currently. The process goes something like: > >> 1. Figure out what to test in the patch > >> 2. Write some code, quickly, that checks for that condition > >> 3. Look through the statemachine to find where the check should go > >> 4. ignore parts of the above and just throw something together > >> > >> That worked fine for the initial development, but as interesting new tests > >> are developed, it is important to have a more flexible framework that lets > >> a developer just plug in a new test, easily. > >> > >> This commit brings in a new framework that allows plugging in checks very > >> quickly. Hook up the line-length test as an initial demonstration. > >> > >> Signed-off-by: Aaron Conole <aconole@redhat.com> > > > > With this, I get: > > > > ../utilities/checkpatch.py:180:12: E999 SyntaxError: invalid syntax > > > > where the cited line is: > > > > lambda(x): print_warning("Line is greater than 79-characters long", x)} > > d'oh! Trying it with python3, I see: > > File "./utilities/checkpatch.py", line 180 > lambda(x): print_warning("Line is greater than 79-characters long", x)} > ^ > SyntaxError: invalid syntax > > So, I guess that is it. I'll submit a fix. I think I broke my python > setup while doing some hacking on a different project, so I'll have to > be more careful. Maybe now is the time to learn virtualenv. > > > I'm using: > > > > $ flake8 --version > > 3.2.1 (mccabe: 0.5.3, pycodestyle: 2.2.0, pyflakes: 1.3.0) CPython 3.5.1+ on Linux > > Yep, that's different from my version: Diversity is strength, I guess. > $ flake8 --version > 2.5.5 (pep8: 1.6.2, mccabe: 0.2.1, pyflakes: 1.2.3, hacking.core: 0.0.1, ProxyChecker: 0.0.1) CPython 2.7.13 on Linux Thanks, I'll look at v4.
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 638ac97..4ee5670 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -164,6 +164,47 @@ def pointer_whitespace_check(line): return __regex_ptr_declaration_missing_whitespace.search(line) is not None +def line_length_check(line): + """Return TRUE if the line length is too long""" + if len(line) > 79: + return True + return False + + +checks = [ + {'regex': None, + 'match_name': + lambda x: not any([fmt in x for fmt in line_length_blacklist]), + 'check': lambda x: line_length_check(x), + 'print': + lambda(x): print_warning("Line is greater than 79-characters long", x)} +] + + +def get_file_type_checks(filename): + """Returns the list of checks for a file based on matching the filename + against regex.""" + global checks + checkList = [] + for check in checks: + if check['regex'] is None and check['match_name'] is None: + checkList.append(check) + if check['regex'] is not None and \ + re.compile(check['regex']).search(filename) is not None: + checkList.append(check) + elif check['match_name'] is not None and check['match_name'](filename): + checkList.append(check) + return checkList + + +def run_checks(current_file, line, lineno): + """Runs the various checks for the particular line. This will take + filename into account.""" + for check in get_file_type_checks(current_file): + if check['check'](line): + check['print'](lineno) + + def ovs_checkpatch_parse(text): global print_file_name lineno = 0 @@ -180,15 +221,10 @@ def ovs_checkpatch_parse(text): re.I | re.M | re.S) is_co_author = re.compile(r'(\s*(Co-authored-by: )(.*))$', re.I | re.M | re.S) - skip_line_length_check = False for line in text.decode().split('\n'): if current_file != previous_file: previous_file = current_file - if any([fmt in current_file for fmt in line_length_blacklist]): - skip_line_length_check = True - else: - skip_line_length_check = False lineno = lineno + 1 if len(line) <= 0: @@ -250,13 +286,10 @@ def ovs_checkpatch_parse(text): print_line = True print_warning("Line has non-spaces leading whitespace", lineno) + run_checks(current_file, cmp_line, lineno) if trailing_whitespace_or_crlf(cmp_line): print_line = True print_warning("Line has trailing whitespace", lineno) - if len(cmp_line) > 79 and not skip_line_length_check: - print_line = True - print_warning("Line is greater than 79-characters long", - lineno) if not if_and_for_whitespace_checks(cmp_line): print_line = True print_error("Improper whitespace around control block",
Developers wishing to add checks to checkpatch sift through an adhoc mess, currently. The process goes something like: 1. Figure out what to test in the patch 2. Write some code, quickly, that checks for that condition 3. Look through the statemachine to find where the check should go 4. ignore parts of the above and just throw something together That worked fine for the initial development, but as interesting new tests are developed, it is important to have a more flexible framework that lets a developer just plug in a new test, easily. This commit brings in a new framework that allows plugging in checks very quickly. Hook up the line-length test as an initial demonstration. Signed-off-by: Aaron Conole <aconole@redhat.com> --- utilities/checkpatch.py | 51 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 9 deletions(-)