ligang/vpp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

vppapigen: api crc checker

Browse Source 
crcchecker is a tool for enforcement of the binary API.

1. Production APIs should never change.
2. An API can be deprecated across three release cycles.
   Release 1: API is marked as deprecated.
   option deprecated="vyy.mm";
   option replaced_by="new_api_2";
   Release 2: both old and new APIs are supported
   Release 3: the deprecated API is deleted.
3. APIs that are experimental / not released are not checked.
   An API message can be individually marked as in progress, by:
   option status="in_progress";
   In the API definition.

The definition of a "production API" is if the major version in the API file is > 0.

extras/scripts/crcchecker.py --check-patchset # returns -1 if backwards incompatible
extras/scripts/crcchecker.py --dump-manifest
extras/scripts/crcchecker.py --git-revision v20.01 <files>
extras/scripts/crcchecker.py -- diff <oldfile> <newfile>

This patch integrates the tool in "make checkstyle-api".
A future patch is required to integrate the tool in the verify job.
I.e. this patch does not enable enforcement through Jenkins.

Change-Id: I5f13c0976d8a12a58131b3e270f2dc9c00dc7d8c
Type: feature
Signed-off-by: Ole Troan <ot@cisco.com>
This commit is contained in:
Ole Troan
2020-05-05 12:23:47 +02:00
committed by Andrew Yourtchenko
parent aa55a926fe
commit 5c318c70d0
5 changed files with 415 additions and 23 deletions

5
Makefile

@ -227,6 +227,7 @@ help:
@echo " checkstyle - check coding style"
@echo " checkstyle-commit - check commit message format"
@echo " checkstyle-test - check test framework coding style"
@echo " checkstyle-api - check api for incompatible changes"
@echo " fixstyle - fix coding style"
@echo " doxygen - (re)generate documentation"
@echo " bootstrap-doxygen - setup Doxygen dependencies"
@ -677,6 +678,10 @@ checkstyle-all: checkstyle-commit checkstyle checkstyle-test
fixstyle:
@build-root/scripts/checkstyle.sh --fix
.PHONY: checkstyle-api
checkstyle-api:
@extras/scripts/crcchecker.py --check-patch
# necessary because Bug 1696324 - Update to python3.6 breaks PyYAML dependencies
# Status: CLOSED CANTFIX
# https://bugzilla.redhat.com/show_bug.cgi?id=1696324

203
extras/scripts/crcchecker.py Executable file

@ -0,0 +1,203 @@
#!/usr/bin/env python3
import sys
import os
import json
import argparse
from subprocess import run, PIPE, check_output, CalledProcessError
rootdir = os.path.dirname(os.path.realpath(__file__)) + '/../..'
def crc_from_apigen(revision, 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'{apigen_bin} --git-revision {revision} --includedir src '
f'--input {filename} CRC')
else:
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 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(rv.stdout)
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: (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():
filelist = []
git_ls = 'git ls-files *.api'
rv = run(git_ls.split(), stdout=PIPE, stderr=PIPE)
if rv.returncode != 0:
sys.exit(rv.returncode)
for l in rv.stdout.decode('ascii').split('\n'):
if len(l):
filelist.append(l)
return filelist
def is_uncommitted_changes():
git_status = 'git status --porcelain -uno'
rv = run(git_status.split(), stdout=PIPE, stderr=PIPE)
if rv.returncode != 0:
sys.exit(rv.returncode)
if len(rv.stdout):
return True
return False
def filelist_from_git_grep(filename):
filelist = []
try:
rv = check_output(f'git grep -e "import .*{filename}" -- *.api', shell=True)
except CalledProcessError as err:
return []
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():
filelist = []
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 f in filelist:
imported_files.extend(filelist_from_git_grep(os.path.basename(f)))
filelist.extend(imported_files)
return set(filelist)
def is_deprecated(d, k):
if 'options' in d[k] and 'deprecated' in d[k]['options']:
return True
return False
def is_in_progress(d, k):
try:
if d[k]['options']['status'] == 'in_progress':
return True
except:
return False
def report(old, new, added, removed, modified, same):
backwards_incompatible = 0
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):
backwards_incompatible += 1
print(f'removed: ** {k}')
else:
print(f'removed: {k}')
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):
backwards_incompatible += 1
print(f'modified: ** {k}')
else:
print(f'modified: {k}')
return backwards_incompatible
def main():
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='Dump CRC for all messages')
parser.add_argument('files', nargs='*')
parser.add_argument('--diff', help='Files to compare (on filesystem)', nargs=2)
args = parser.parse_args()
if args.diff and args.files:
parser.print_help()
sys.exit(-1)
# Diff two files
if args.diff:
oldcrcs = crc_from_apigen(None, args.diff[0])
newcrcs = crc_from_apigen(None, args.diff[1])
added, removed, modified, same = dict_compare(newcrcs, oldcrcs)
backwards_incompatible = report(oldcrcs, newcrcs, added, removed, modified, same)
sys.exit(0)
# Dump CRC for messages in given files / revision
if args.dump_manifest:
files = args.files if args.files else filelist_from_git_ls()
crcs = {}
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)
if args.check_patchset:
if args.git_revision:
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)
sys.exit(-1)
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 f in files:
newcrcs.update(crc_from_apigen(None, f))
oldcrcs.update(crc_from_apigen(revision, f))
added, removed, modified, same = dict_compare(newcrcs, oldcrcs)
backwards_incompatible = report(oldcrcs, newcrcs, added, removed, modified, same)
if args.check_patchset:
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)
if __name__ == '__main__':
main()

140
extras/scripts/tests/test_crcchecker.sh Executable file

@ -0,0 +1,140 @@
#!/bin/sh
set -eux
TMPDIR=$(mktemp -d /tmp/vpp-crccheck-test-XXXXX)
CURR_ROOT=$(git rev-parse --show-toplevel)
CURR_DIR=$(pwd)
verify_check_patchset_fails() {
if (extras/scripts/crcchecker.py --check-patchset); then
echo "ERROR - check succeeded, it should have failed!"
exit 1;
fi
}
finish() {
if [ -e "$TMPDIR" ]; then
echo "Temporary directory is: $TMPDIR"
fi
}
trap finish EXIT
# make a copy of the current repo that we can play with
cd ${TMPDIR}
mkdir misc-files
git clone ${CURR_ROOT} vpp-uut
cd vpp-uut
# maybe grab the CRC checker
# git fetch "https://gerrit.fd.io/r/vpp" refs/changes/81/26881/14 && git cherry-pick FETCH_HEAD || echo "Already there"
echo "TEST 1: Check the current patchset..."
extras/scripts/crcchecker.py --check-patchset
echo "TEST 2: Dumping the current manifest..."
extras/scripts/crcchecker.py --dump-manifest >${TMPDIR}/misc-files/manifest.txt
echo "TEST 3: Checking the 20.01 version of acl.api...."
extras/scripts/crcchecker.py --git-revision v20.01 src/plugins/acl/acl.api
echo "TEST 4: Add a new field into a message in acl.api, and check patchset - must fail..."
sed -i -e 's#vpe_pid;#vpe_pid; u32 sneaky_new_field;#' src/plugins/acl/acl.api
verify_check_patchset_fails
echo "TEST 5: Rename the changed acl.api file and not add it to git... must fail (due to deletion of the APIs)..."
mv src/plugins/acl/acl.api src/plugins/acl/acl_new.api
verify_check_patchset_fails
echo "TEST 6: Add the renamed file to git commit... must fail (due to addition of the fields)..."
git add src/plugins/acl/acl_new.api
git commit -m "added acl_new.api"
verify_check_patchset_fails
echo "TEST 7: Verify we can delete deprecated message"
git commit -a -m "reset"
cat >crccheck.api <<EOL
option version="1.0.0";
autoreply define crccheck
{
option deprecated="v20.11";
bool foo;
};
EOL
git add crccheck.api
git commit -m "deprecated api";
# delete API
cat >crccheck.api <<EOL
option version="1.0.0";
autoreply define crccheck_2
{
bool foo;
};
EOL
git add crccheck.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' 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' 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 >>crccheck.api <<EOL
autoreply define crc_new_check_in_progress
{
option status="in_progress";
bool foobar;
};
EOL
verify_check_patchset_fails
echo "TEST10: Verify that the in-progress message can be added"
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' crccheck.api
git add crccheck.api
git commit -m "renamed 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' 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 >crccheck2.api <<EOL
option version="0.0.1";
autoreply define spot_the_error
{
option status="in_progress"
bool something_important;
};
EOL
git add crccheck2.api
git commit -m "a new message with a syntax error";
verify_check_patchset_fails
# get rid of the "erroneous" commit in the previous test
git reset --hard HEAD~1
echo "TEST: All tests got the expected result, cleaning up."
# done with all the tests - clean up
cd ${CURR_DIR}
# beware of empty variables, careful with deletion
rm -rf ${TMPDIR}/vpp-uut
rm -rf ${TMPDIR}/misc-files
rmdir ${TMPDIR}

74
src/tools/vppapigen/vppapigen.py

@ -9,6 +9,7 @@ import logging
import binascii
import os
import sys
from subprocess import Popen, PIPE
log = logging.getLogger('vppapigen')
@ -275,14 +276,17 @@ class Define():
elif f == 'autoreply':
self.autoreply = True
remove = []
for b in block:
if isinstance(b, Option):
if b[1] == 'singular' and b[2] == 'true':
self.singular = True
else:
self.options[b.option] = b.value
block.remove(b)
remove.append(b)
block = [x for x in block if not x in remove]
self.block = block
self.vla = vla_is_last_check(name, block)
self.crc = str(block).encode()
@ -322,25 +326,20 @@ class Import():
return seen_imports[args[0]]
def __init__(self, filename):
def __init__(self, filename, revision):
if self._initialized:
return
else:
self.filename = filename
# Deal with imports
parser = VPPAPI(filename=filename)
parser = VPPAPI(filename=filename, revision=revision)
dirlist = dirlist_get()
f = filename
for dir in dirlist:
f = os.path.join(dir, filename)
if os.path.exists(f):
break
if sys.version[0] == '2':
with open(f) as fd:
self.result = parser.parse_file(fd, None)
else:
with open(f, encoding='utf-8') as fd:
self.result = parser.parse_file(fd, None)
self.result = parser.parse_filename(f, None)
self._initialized = True
def __repr__(self):
@ -430,10 +429,11 @@ class ParseError(Exception):
class VPPAPIParser(object):
tokens = VPPAPILexer.tokens
def __init__(self, filename, logger):
def __init__(self, filename, logger, revision=None):
self.filename = filename
self.logger = logger
self.fields = []
self.revision = revision
def _parse_error(self, msg, coord):
raise ParseError("%s: %s" % (coord, msg))
@ -478,7 +478,7 @@ class VPPAPIParser(object):
def p_import(self, p):
'''import : IMPORT STRING_LITERAL ';' '''
p[0] = Import(p[2])
p[0] = Import(p[2], revision=self.revision)
def p_service(self, p):
'''service : SERVICE '{' service_statements '}' ';' '''
@ -731,23 +731,42 @@ class VPPAPIParser(object):
class VPPAPI(object):
def __init__(self, debug=False, filename='', logger=None):
def __init__(self, debug=False, filename='', logger=None, revision=None):
self.lexer = lex.lex(module=VPPAPILexer(filename), debug=debug)
self.parser = yacc.yacc(module=VPPAPIParser(filename, logger),
self.parser = yacc.yacc(module=VPPAPIParser(filename, logger,
revision=revision),
write_tables=False, debug=debug)
self.logger = logger
self.revision = revision
self.filename = filename
def parse_string(self, code, debug=0, lineno=1):
self.lexer.lineno = lineno
return self.parser.parse(code, lexer=self.lexer, debug=debug)
def parse_file(self, fd, debug=0):
def parse_fd(self, fd, debug=0):
data = fd.read()
return self.parse_string(data, debug=debug)
def autoreply_block(self, name):
def parse_filename(self, filename, debug=0):
if self.revision:
git_show = f'git show {self.revision}:{filename}'
with Popen(git_show.split(), stdout=PIPE, encoding='utf-8') as git:
return self.parse_fd(git.stdout, None)
else:
try:
with open(filename, encoding='utf-8') as fd:
return self.parse_fd(fd, None)
except FileNotFoundError:
print(f'File not found: {filename}', file=sys.stderr)
sys.exit(2)
def autoreply_block(self, name, parent):
block = [Field('u32', 'context'),
Field('i32', 'retval')]
# inherhit the parent's options
for k,v in parent.options.items():
block.append(Option(k, v))
return Define(name + '_reply', [], block)
def process(self, objs):
@ -767,7 +786,7 @@ class VPPAPI(object):
if isinstance(o, Define):
s[tname].append(o)
if o.autoreply:
s[tname].append(self.autoreply_block(o.name))
s[tname].append(self.autoreply_block(o.name, o))
elif isinstance(o, Option):
s[tname][o.option] = o.value
elif type(o) is list:
@ -925,9 +944,7 @@ def main():
cliparser.add_argument('--pluginpath', default=""),
cliparser.add_argument('--includedir', action='append'),
cliparser.add_argument('--outputdir', action='store'),
cliparser.add_argument('--input',
type=argparse.FileType('r', encoding='UTF-8'),
default=sys.stdin)
cliparser.add_argument('--input')
cliparser.add_argument('--output', nargs='?',
type=argparse.FileType('w', encoding='UTF-8'),
default=sys.stdout)
@ -935,6 +952,8 @@ def main():
cliparser.add_argument('output_module', nargs='?', default='C')
cliparser.add_argument('--debug', action='store_true')
cliparser.add_argument('--show-name', nargs=1)
cliparser.add_argument('--git-revision',
help="Git revision to use for opening files")
args = cliparser.parse_args()
dirlist_add(args.includedir)
@ -944,8 +963,8 @@ def main():
# Filename
if args.show_name:
filename = args.show_name[0]
elif args.input != sys.stdin:
filename = args.input.name
elif args.input:
filename = args.input
else:
filename = ''
@ -954,8 +973,17 @@ def main():
else:
logging.basicConfig()
parser = VPPAPI(debug=args.debug, filename=filename, logger=log)
parsed_objects = parser.parse_file(args.input, log)
parser = VPPAPI(debug=args.debug, filename=filename, logger=log,
revision=args.git_revision)
try:
if not args.input:
parsed_objects = parser.parse_fd(sys.stdin, log)
else:
parsed_objects = parser.parse_filename(args.input, log)
except ParseError as e:
print('Parse error: ', e, file=sys.stderr)
sys.exit(1)
# Build a list of objects. Hash of lists.
result = []

16
src/tools/vppapigen/vppapigen_crc.py Normal file

@ -0,0 +1,16 @@
# CRC generation
import json
#
# Plugin entry point
#
def run(args, input_filename, s):
j = {}
major = 0
if 'version' in s['Option']:
v = s['Option']['version']
(major, minor, patch) = v.split('.')
for t in s['Define']:
j[t.name] = {'crc': f'{t.crc:#08x}', 'version': major,
'options': t.options}
return json.dumps(j, indent=4, separators=(',', ': '))