diff options
Diffstat (limited to 'extras')
| -rwxr-xr-x | extras/scripts/crcchecker.py | 218 | ||||
| -rwxr-xr-x | extras/scripts/tests/test_crcchecker.sh | 46 |
2 files changed, 93 insertions, 171 deletions
diff --git a/extras/scripts/crcchecker.py b/extras/scripts/crcchecker.py index 5060d9f2e0e..2b026338129 100755 --- a/extras/scripts/crcchecker.py +++ b/extras/scripts/crcchecker.py @@ -1,176 +1,134 @@ #!/usr/bin/env python3 -''' -crcchecker is a tool to used to enforce that .api messages do not change. -API files with a semantic version < 1.0.0 are ignored. -''' - import sys import os import json import argparse -import re from subprocess import run, PIPE, check_output, CalledProcessError -# pylint: disable=subprocess-run-check - -ROOTDIR = os.path.dirname(os.path.realpath(__file__)) + '/../..' -APIGENBIN = f'{ROOTDIR}/src/tools/vppapigen/vppapigen.py' - +rootdir = os.path.dirname(os.path.realpath(__file__)) + '/../..' def crc_from_apigen(revision, filename): - '''Runs vppapigen with crc plugin returning a JSON object with CRCs for - all APIs in filename''' if not revision and not os.path.isfile(filename): print(f'skipping: {filename}', file=sys.stderr) return {} - + apigen_bin = f'{rootdir}/src/tools/vppapigen/vppapigen.py' if revision: - apigen = (f'{APIGENBIN} --git-revision {revision} --includedir src ' + apigen = (f'{apigen_bin} --git-revision {revision} --includedir src ' f'--input {filename} CRC') else: - apigen = (f'{APIGENBIN} --includedir src --input {filename} CRC') - returncode = run(apigen.split(), stdout=PIPE, stderr=PIPE) - if returncode.returncode == 2: # No such file - print(f'skipping: {revision}:{filename} {returncode}', file=sys.stderr) + apigen = (f'{apigen_bin} --includedir src --input {filename} CRC') + rv = run(apigen.split(), stdout=PIPE, stderr=PIPE) + if rv.returncode == 2: # No such file + print(f'skipping: {revision}:{filename} {rv}', file=sys.stderr) return {} - if returncode.returncode != 0: - print(f'vppapigen failed for {revision}:{filename} with ' - 'command\n {apigen}\n error: {rv}', - returncode.stderr.decode('ascii'), file=sys.stderr) + if rv.returncode != 0: + print(f'vppapigen failed for {revision}:{filename} with command\n {apigen}\n error: {rv}', + rv.stderr.decode('ascii'), file=sys.stderr) sys.exit(-2) - return json.loads(returncode.stdout) + return json.loads(rv.stdout) -def dict_compare(dict1, dict2): - '''Compare two dictionaries returning added, removed, modified - and equal entries''' - d1_keys = set(dict1.keys()) - d2_keys = set(dict2.keys()) +def dict_compare(d1, d2): + d1_keys = set(d1.keys()) + d2_keys = set(d2.keys()) intersect_keys = d1_keys.intersection(d2_keys) added = d1_keys - d2_keys removed = d2_keys - d1_keys - modified = {o: (dict1[o], dict2[o]) for o in intersect_keys - if dict1[o]['crc'] != dict2[o]['crc']} - same = set(o for o in intersect_keys if dict1[o] == dict2[o]) + modified = {o: (d1[o], d2[o]) for o in intersect_keys if d1[o]['crc'] != d2[o]['crc']} + same = set(o for o in intersect_keys if d1[o] == d2[o]) return added, removed, modified, same def filelist_from_git_ls(): - '''Returns a list of all api files in the git repository''' filelist = [] git_ls = 'git ls-files *.api' - returncode = run(git_ls.split(), stdout=PIPE, stderr=PIPE) - if returncode.returncode != 0: - sys.exit(returncode.returncode) + rv = run(git_ls.split(), stdout=PIPE, stderr=PIPE) + if rv.returncode != 0: + sys.exit(rv.returncode) - for line in returncode.stdout.decode('ascii').split('\n'): - if line: - filelist.append(line) + for l in rv.stdout.decode('ascii').split('\n'): + if len(l): + filelist.append(l) return filelist def is_uncommitted_changes(): - '''Returns true if there are uncommitted changes in the repo''' git_status = 'git status --porcelain -uno' - returncode = run(git_status.split(), stdout=PIPE, stderr=PIPE) - if returncode.returncode != 0: - sys.exit(returncode.returncode) + rv = run(git_status.split(), stdout=PIPE, stderr=PIPE) + if rv.returncode != 0: + sys.exit(rv.returncode) - if returncode.stdout: + if len(rv.stdout): return True return False def filelist_from_git_grep(filename): - '''Returns a list of api files that this <filename> api files imports.''' filelist = [] try: - returncode = check_output(f'git grep -e "import .*{filename}"' - ' -- *.api', - shell=True) - except CalledProcessError: + rv = check_output(f'git grep -e "import .*{filename}" -- *.api', shell=True) + except CalledProcessError as err: return [] - for line in returncode.decode('ascii').split('\n'): - if line: - filename, _ = line.split(':') - filelist.append(filename) + print('RV', err.returncode) + for l in rv.decode('ascii').split('\n'): + if l: + f, p = l.split(':') + filelist.append(f) return filelist -def filelist_from_patchset(pattern): - '''Returns list of api files in changeset and the list of api - files they import.''' +def filelist_from_patchset(): filelist = [] - git_cmd = ('((git diff HEAD~1.. --name-only;git ls-files -m) | ' - 'sort -u | grep "\\.api$")') - returncode = check_output(git_cmd, shell=True) + git_cmd = '((git diff HEAD~1.. --name-only;git ls-files -m) | sort -u)' + rv = check_output(git_cmd, shell=True) + for l in rv.decode('ascii').split('\n'): + if len(l) and os.path.splitext(l)[1] == '.api': + filelist.append(l) # Check for dependencies (imports) imported_files = [] - for line in returncode.decode('ascii').split('\n'): - if not line: - continue - if not re.search(pattern, line): - continue - filelist.append(line) - imported_files.extend(filelist_from_git_grep(os.path.basename(line))) + for f in filelist: + imported_files.extend(filelist_from_git_grep(os.path.basename(f))) filelist.extend(imported_files) return set(filelist) - -def is_deprecated(message): - '''Given a message, return True if message is deprecated''' - if 'options' in message: - if 'deprecated' in message['options']: +def is_deprecated(d, k): + if 'options' in d[k]: + if 'deprecated' in d[k]['options']: return True # recognize the deprecated format - if 'status' in message['options'] and \ - message['options']['status'] == 'deprecated': + if 'status' in d[k]['options'] and d[k]['options']['status'] == 'deprecated': print("WARNING: please use 'option deprecated;'") return True return False - -def is_in_progress(message): - '''Given a message, return True if message is marked as in_progress''' - if 'options' in message: - if 'in_progress' in message['options']: +def is_in_progress(d, k): + if 'options' in d[k]: + if 'in_progress' in d[k]['options']: return True # recognize the deprecated format - if 'status' in message['options'] and \ - message['options']['status'] == 'in_progress': + if 'status' in d[k]['options'] and d[k]['options']['status'] == 'in_progress': print("WARNING: please use 'option in_progress;'") return True return False - def report(new, old): - '''Given a dictionary of new crcs and old crcs, print all the - added, removed, modified, in-progress, deprecated messages. - Return the number of backwards incompatible changes made.''' - - # pylint: disable=too-many-branches - - new.pop('_version', None) - old.pop('_version', None) - added, removed, modified, _ = dict_compare(new, old) + added, removed, modified, same = dict_compare(new, old) backwards_incompatible = 0 - # print the full list of in-progress messages # they should eventually either disappear of become supported for k in new.keys(): newversion = int(new[k]['version']) - if newversion == 0 or is_in_progress(new[k]): + if newversion == 0 or is_in_progress(new, k): print(f'in-progress: {k}') for k in added: print(f'added: {k}') for k in removed: oldversion = int(old[k]['version']) - if oldversion > 0 and not is_deprecated(old[k]) and not \ - is_in_progress(old[k]): + if oldversion > 0 and not is_deprecated(old, k) and not is_in_progress(old, k): backwards_incompatible += 1 print(f'removed: ** {k}') else: @@ -178,7 +136,7 @@ def report(new, old): for k in modified.keys(): oldversion = int(old[k]['version']) newversion = int(new[k]['version']) - if oldversion > 0 and not is_in_progress(old[k]): + if oldversion > 0 and not is_in_progress(old, k): backwards_incompatible += 1 print(f'modified: ** {k}') else: @@ -187,9 +145,9 @@ def report(new, old): # check which messages are still there but were marked for deprecation for k in new.keys(): newversion = int(new[k]['version']) - if newversion > 0 and is_deprecated(new[k]): + if newversion > 0 and is_deprecated(new, k): if k in old: - if not is_deprecated(old[k]): + if not is_deprecated(old, k): print(f'deprecated: {k}') else: print(f'added+deprecated: {k}') @@ -197,49 +155,16 @@ def report(new, old): return backwards_incompatible -def check_patchset(): - '''Compare the changes to API messages in this changeset. - Ignores API files with version < 1.0.0. - Only considers API files located under the src directory in the repo. - ''' - files = filelist_from_patchset('^src/') - revision = 'HEAD~1' - - oldcrcs = {} - newcrcs = {} - for filename in files: - # Ignore files that have version < 1.0.0 - _ = crc_from_apigen(None, filename) - if _['_version']['major'] == '0': - continue - - newcrcs.update(_) - oldcrcs.update(crc_from_apigen(revision, filename)) - - backwards_incompatible = report(newcrcs, oldcrcs) - if backwards_incompatible: - # alert on changing production API - print("crcchecker: Changing production APIs in an incompatible way", - file=sys.stderr) - sys.exit(-1) - else: - print('*' * 67) - print('* VPP CHECKAPI SUCCESSFULLY COMPLETED') - print('*' * 67) - - def main(): - '''Main entry point.''' parser = argparse.ArgumentParser(description='VPP CRC checker.') parser.add_argument('--git-revision', help='Git revision to compare against') parser.add_argument('--dump-manifest', action='store_true', help='Dump CRC for all messages') parser.add_argument('--check-patchset', action='store_true', - help='Check patchset for backwards incompatbile changes') + help='Dump CRC for all messages') parser.add_argument('files', nargs='*') - parser.add_argument('--diff', help='Files to compare (on filesystem)', - nargs=2) + parser.add_argument('--diff', help='Files to compare (on filesystem)', nargs=2) args = parser.parse_args() @@ -258,10 +183,10 @@ def main(): if args.dump_manifest: files = args.files if args.files else filelist_from_git_ls() crcs = {} - for filename in files: - crcs.update(crc_from_apigen(args.git_revision, filename)) - for k, value in crcs.items(): - print(f'{k}: {value}') + for f in files: + crcs.update(crc_from_apigen(args.git_revision, f)) + for k, v in crcs.items(): + print(f'{k}: {v}') sys.exit(0) # Find changes between current patchset and given revision (previous) @@ -270,23 +195,21 @@ def main(): print('Argument git-revision ignored', file=sys.stderr) # Check there are no uncomitted changes if is_uncommitted_changes(): - print('Please stash or commit changes in workspace', - file=sys.stderr) + print('Please stash or commit changes in workspace', file=sys.stderr) sys.exit(-1) - check_patchset() - sys.exit(0) - - # Find changes between current workspace and revision - # Find changes between a given file and a revision - files = args.files if args.files else filelist_from_git_ls() + files = filelist_from_patchset() + else: + # Find changes between current workspace and revision + # Find changes between a given file and a revision + files = args.files if args.files else filelist_from_git_ls() revision = args.git_revision if args.git_revision else 'HEAD~1' oldcrcs = {} newcrcs = {} - for file in files: - newcrcs.update(crc_from_apigen(None, file)) - oldcrcs.update(crc_from_apigen(revision, file)) + for f in files: + newcrcs.update(crc_from_apigen(None, f)) + oldcrcs.update(crc_from_apigen(revision, f)) backwards_incompatible = report(newcrcs, oldcrcs) @@ -300,6 +223,5 @@ def main(): print('* VPP CHECKAPI SUCCESSFULLY COMPLETED') print('*' * 67) - if __name__ == '__main__': main() diff --git a/extras/scripts/tests/test_crcchecker.sh b/extras/scripts/tests/test_crcchecker.sh index 4caffe28e0d..9cfc66ae523 100755 --- a/extras/scripts/tests/test_crcchecker.sh +++ b/extras/scripts/tests/test_crcchecker.sh @@ -55,7 +55,7 @@ verify_check_patchset_fails echo "TEST 7: Verify we can delete deprecated message" git commit -a -m "reset" -cat >src/crccheck.api <<EOL +cat >crccheck.api <<EOL option version="1.0.0"; autoreply define crccheck { @@ -63,22 +63,22 @@ autoreply define crccheck bool foo; }; EOL -git add src/crccheck.api +git add crccheck.api git commit -m "deprecated api"; # delete API -cat >src/crccheck.api <<EOL +cat >crccheck.api <<EOL option version="1.0.0"; autoreply define crccheck_2 { bool foo; }; EOL -git add src/crccheck.api +git add crccheck.api git commit -m "deprecated api"; extras/scripts/crcchecker.py --check-patchset echo "TEST 7.1: Verify we can delete deprecated message (old/confused style)" -cat >src/crccheck_dep.api <<EOL +cat >crccheck_dep.api <<EOL option version="1.0.0"; autoreply define crccheck { @@ -86,31 +86,31 @@ autoreply define crccheck bool foo; }; EOL -git add src/crccheck_dep.api +git add crccheck_dep.api git commit -m "deprecated api"; # delete API -cat >src/crccheck_dep.api <<EOL +cat >crccheck_dep.api <<EOL option version="1.0.0"; autoreply define crccheck_2 { bool foo; }; EOL -git add src/crccheck_dep.api +git add crccheck_dep.api git commit -m "deprecated api"; extras/scripts/crcchecker.py --check-patchset echo "TEST 8: Verify that we can not rename a non-deprecated message" -sed -i -e 's/crccheck_2/crccheck_3/g' src/crccheck.api -git add src/crccheck.api +sed -i -e 's/crccheck_2/crccheck_3/g' crccheck.api +git add crccheck.api git commit -m "renamed api"; verify_check_patchset_fails # fix it. -sed -i -e 's/crccheck_3/crccheck_2/g' src/crccheck.api +sed -i -e 's/crccheck_3/crccheck_2/g' crccheck.api git commit -a --amend -m "empty commit after we renamed api back" --allow-empty echo "TEST 9: Verify that the check fails if the changes are not committed" -cat >>src/crccheck.api <<EOL +cat >>crccheck.api <<EOL autoreply define crc_new_check_in_progress { option status="in_progress"; @@ -120,31 +120,31 @@ EOL verify_check_patchset_fails echo "TEST10: Verify that the in-progress message can be added" -git add src/crccheck.api +git add crccheck.api git commit -m "added a new in-progress api"; extras/scripts/crcchecker.py --check-patchset echo "TEST11: Verify we can rename an in-progress API" -sed -i -e 's/crc_new_check_in_progress/crc_new_check_in_progress_2/g' src/crccheck.api -git add src/crccheck.api +sed -i -e 's/crc_new_check_in_progress/crc_new_check_in_progress_2/g' crccheck.api +git add crccheck.api git commit -m "renamed in-progress api"; extras/scripts/crcchecker.py --check-patchset echo "TEST11.1: Switch to new designation of in-progress API" -sed -i -e 's/status="in_progress"/in_progress/g' src/crccheck.api -git add src/crccheck.api +sed -i -e 's/status="in_progress"/in_progress/g' crccheck.api +git add crccheck.api git commit -m "new designation of in-progress api"; extras/scripts/crcchecker.py --check-patchset echo "TEST12: Verify we can add a field to an in-progress API" -sed -i -e 's/foobar;/foobar; bool new_baz;/g' src/crccheck.api -git add src/crccheck.api +sed -i -e 's/foobar;/foobar; bool new_baz;/g' crccheck.api +git add crccheck.api git commit -m "new field added in in-progress api"; extras/scripts/crcchecker.py --check-patchset echo "TEST13: Verify we fail the check if the file can not be compiled" -cat >src/crccheck2.api <<EOL +cat >crccheck2.api <<EOL option version="0.0.1"; autoreply define spot_the_error { @@ -152,7 +152,7 @@ autoreply define spot_the_error bool something_important; }; EOL -git add src/crccheck2.api +git add crccheck2.api git commit -m "a new message with a syntax error"; verify_check_patchset_fails @@ -160,13 +160,13 @@ verify_check_patchset_fails git reset --hard HEAD~1 echo "TEST14: Verify we handle new .api file" -cat >src/crccheck3.api <<EOL +cat >crccheck3.api <<EOL autoreply define foo { bool bar; }; EOL -git add src/crccheck3.api +git add crccheck3.api git commit -m "a new message in new file"; extras/scripts/crcchecker.py --check-patchset |