devtools: add checks for ABI symbol addition

Recently, some additional patches were added to allow for programmatic
marking of C symbols as experimental.  The addition of these markers is
dependent on the manual addition of exported symbols to the EXPERIMENTAL
section of the corresponding libraries version map file.  The consensus
on review is that, in addition to mandating the addition of symbols to
the EXPERIMENTAL version in the map, we need a mechanism to enforce our
documented process of mandating that addition when they are introduced.
To that end, I am proposing this change.  It is an addition to the
checkpatches script, which scan incoming patches for additions and
removals of symbols to the map file, and warns the user appropriately.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
This commit is contained in:
Neil Horman 2018-06-27 14:01:01 -04:00 committed by Thomas Monjalon
parent d9ddc004e6
commit 4bec48184e
3 changed files with 196 additions and 7 deletions

View File

@ -124,6 +124,7 @@ M: Neil Horman <nhorman@tuxdriver.com>
F: lib/librte_compat/
F: doc/guides/rel_notes/deprecation.rst
F: devtools/validate-abi.sh
F: devtools/check-symbol-change.sh
F: buildtools/check-experimental-syms.sh
Driver information

159
devtools/check-symbol-change.sh Executable file
View File

@ -0,0 +1,159 @@
#!/bin/sh
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2018 Neil Horman <nhorman@tuxdriver.com>
build_map_changes()
{
local fname=$1
local mapdb=$2
cat $fname | awk '
# Initialize our variables
BEGIN {map="";sym="";ar="";sec=""; in_sec=0; in_map=0}
# Anything that starts with + or -, followed by an a
# and ends in the string .map is the name of our map file
# This may appear multiple times in a patch if multiple
# map files are altered, and all section/symbol names
# appearing between a triggering of this rule and the
# next trigger of this rule are associated with this file
/[-+] a\/.*\.map/ {map=$2; in_map=1}
# Same pattern as above, only it matches on anything that
# does not end in 'map', indicating we have left the map chunk.
# When we hit this, turn off the in_map variable, which
# supresses the subordonate rules below
/[-+] a\/.*\.^(map)/ {in_map=0}
# Triggering this rule, which starts a line with a + and ends it
# with a { identifies a versioned section. The section name is
# the rest of the line with the + and { symbols remvoed.
# Triggering this rule sets in_sec to 1, which actives the
# symbol rule below
/+.*{/ {gsub("+","");
if (in_map == 1) {
sec=$1; in_sec=1;
}
}
# This rule idenfies the end of a section, and disables the
# symbol rule
/.*}/ {in_sec=0}
# This rule matches on a + followed by any characters except a :
# (which denotes a global vs local segment), and ends with a ;.
# The semicolon is removed and the symbol is printed with its
# association file name and version section, along with an
# indicator that the symbol is a new addition. Note this rule
# only works if we have found a version section in the rule
# above (hence the in_sec check) And found a map file (the
# in_map check). If we are not in a map chunk, do nothing. If
# we are in a map chunk but not a section chunk, record it as
# unknown.
/^+[^}].*[^:*];/ {gsub(";","");sym=$2;
if (in_map == 1) {
if (in_sec == 1) {
print map " " sym " " sec " add"
} else {
print map " " sym " unknown add"
}
}
}
# This is the same rule as above, but the rule matches on a
# leading - rather than a +, denoting that the symbol is being
# removed.
/^-[^}].*[^:*];/ {gsub(";","");sym=$2;
if (in_map == 1) {
if (in_sec == 1) {
print map " " sym " " sec " del"
} else {
print map " " sym " unknown del"
}
}
}' > ./$mapdb
sort -u $mapdb > ./$mapdb.2
mv -f $mapdb.2 $mapdb
}
check_for_rule_violations()
{
local mapdb=$1
local mname
local symname
local secname
local ar
local ret=0
while read mname symname secname ar
do
if [ "$ar" == "add" ]
then
if [ "$secname" == "unknown" ]
then
# Just inform the user of this occurrence, but
# don't flag it as an error
echo -n "INFO: symbol $syname is added but "
echo -n "patch has insuficient context "
echo -n "to determine the section name "
echo -n "please ensure the version is "
echo "EXPERIMENTAL"
continue
fi
if [ "$secname" != "EXPERIMENTAL" ]
then
# Symbols that are getting added in a section
# other than the experimental section
# to be moving from an already supported
# section or its a violation
grep -q \
"$mname $symname [^EXPERIMENTAL] del" $mapdb
if [ $? -ne 0 ]
then
echo -n "ERROR: symbol $symname "
echo -n "is added in a section "
echo -n "other than the EXPERIMENTAL "
echo "section of the version map"
ret=1
fi
fi
else
if [ "$secname" != "EXPERIMENTAL" ]
then
# Just inform users that non-experimenal
# symbols need to go through a deprecation
# process
echo -n "INFO: symbol $symname is being "
echo -n "removed, ensure that it has "
echo "gone through the deprecation process"
fi
fi
done < $mapdb
return $ret
}
trap clean_and_exit_on_sig EXIT
mapfile=`mktemp mapdb.XXXXXX`
patch=$1
exit_code=1
clean_and_exit_on_sig()
{
rm -f $mapfile
exit $exit_code
}
build_map_changes $patch $mapfile
check_for_rule_violations $mapfile
exit_code=$?
rm -f $mapfile
exit $exit_code

View File

@ -7,6 +7,8 @@
# - DPDK_CHECKPATCH_LINE_LENGTH
. $(dirname $(readlink -e $0))/load-devel-config
VALIDATE_NEW_API=$(dirname $(readlink -e $0))/check-symbol-change.sh
length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}
# override default Linux options
@ -21,6 +23,14 @@ SPLIT_STRING,LONG_LINE_STRING,\
LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
NEW_TYPEDEFS,COMPARISON_TO_NULL"
clean_tmp_files() {
if echo $tmpinput | grep -q '^checkpatches\.' ; then
rm -f $tmpinput
fi
}
trap "clean_tmp_files" SIGINT
print_usage () {
cat <<- END_OF_HELP
usage: $(basename $0) [-q] [-v] [-nX|patch1 [patch2] ...]]
@ -58,19 +68,38 @@ total=0
status=0
check () { # <patch> <commit> <title>
local ret=0
total=$(($total + 1))
! $verbose || printf '\n### %s\n\n' "$3"
if [ -n "$1" ] ; then
report=$($DPDK_CHECKPATCH_PATH $options "$1" 2>/dev/null)
tmpinput=$1
elif [ -n "$2" ] ; then
report=$(git format-patch --find-renames --no-stat --stdout -1 $commit |
$DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
tmpinput=$(mktemp checkpatches.XXXXXX)
git format-patch --find-renames \
--no-stat --stdout -1 $commit > $tmpinput
else
report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null)
tmpinput=$(mktemp checkpatches.XXXXXX)
cat > $tmpinput
fi
[ $? -ne 0 ] || return 0
$verbose || printf '\n### %s\n\n' "$3"
printf '%s\n' "$report" | sed -n '1,/^total:.*lines checked$/p'
report=$($DPDK_CHECKPATCH_PATH $options $tmpinput 2>/dev/null)
if [ $? -ne 0 ] ; then
$verbose || printf '\n### %s\n\n' "$3"
printf '%s\n' "$report" | sed -n '1,/^total:.*lines checked$/p'
ret=1
fi
! $verbose || printf '\nChecking API additions/removals:\n'
report=$($VALIDATE_NEW_API "$tmpinput")
if [ $? -ne 0 ] ; then
printf '%s\n' "$report"
ret=1
fi
clean_tmp_files
[ $ret -eq 0 ] && return 0
status=$(($status + 1))
}