aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOle Tr�an <otroan@employees.org>2020-12-15 15:11:21 +0000
committerOle Tr�an <otroan@employees.org>2020-12-15 16:04:03 +0000
commit58a6e7725212188dc993c2d6ac9fb149f33ed0db (patch)
treeeaac10586c4bdfdcdcea5f366de24c9ae0ac5183
parent3dcf795cf07885285124eed88fb44fc7d70a28c6 (diff)
api: crchcecker ignore version < 1.0.0 and outside of src directory
This reverts commit 510aaa8911843206f7b9ff48b41e3c7b8c4a99fe. Reason for revert: failed in case of no api file in changeset. Change-Id: I2c6f01b25a35128df870418eef0008766bb590df Type: fix Signed-off-by: Ole Troan <ot@cisco.com>
-rw-r--r--MAINTAINERS1
-rw-r--r--Makefile2
-rwxr-xr-xextras/scripts/crcchecker.py218
-rwxr-xr-xextras/scripts/tests/test_crcchecker.sh46
-rw-r--r--src/tools/vppapigen/vppapigen_crc.py3
-rw-r--r--src/vat2/test/vat2_test.api1
6 files changed, 94 insertions, 177 deletions
diff --git a/MAINTAINERS b/MAINTAINERS
index d9a3006ce52..2ad7c557c66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -582,7 +582,6 @@ Binary API Compiler for Python
I: vppapigen
M: Ole Troan <otroan@employees.org>
F: src/tools/vppapigen/
-F: extras/scripts/crcchecker.py
API trace tool
I: vppapitrace
diff --git a/Makefile b/Makefile
index ed6b3e9092b..ee83d7af358 100644
--- a/Makefile
+++ b/Makefile
@@ -631,7 +631,7 @@ fixstyle:
.PHONY: checkstyle-api
checkstyle-api:
- @extras/scripts/crcchecker.py --check-patchset
+ @extras/scripts/crcchecker.py --check-patch
# necessary because Bug 1696324 - Update to python3.6 breaks PyYAML dependencies
# Status: CLOSED CANTFIX
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
diff --git a/src/tools/vppapigen/vppapigen_crc.py b/src/tools/vppapigen/vppapigen_crc.py
index 791e347292e..6947f12d467 100644
--- a/src/tools/vppapigen/vppapigen_crc.py
+++ b/src/tools/vppapigen/vppapigen_crc.py
@@ -10,12 +10,9 @@ process_imports = True
def run(args, input_filename, s):
j = {}
major = 0
- minor = 0
- patch = 0
if 'version' in s['Option']:
v = s['Option']['version']
(major, minor, patch) = v.split('.')
- j['_version'] = {'major': major, 'minor': minor, 'patch': patch}
for t in s['Define']:
j[t.name] = {'crc': f'{t.crc:#08x}', 'version': major,
'options': t.options}
diff --git a/src/vat2/test/vat2_test.api b/src/vat2/test/vat2_test.api
index 20b483aa1db..6a2c94d182e 100644
--- a/src/vat2/test/vat2_test.api
+++ b/src/vat2/test/vat2_test.api
@@ -13,7 +13,6 @@
* limitations under the License.
*/
-option version="0.0.0";
import "vnet/ip/ip_types.api";
autoreply define test_prefix {