From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 55A481B62D for ; Mon, 5 Feb 2018 18:31:04 +0100 (CET) Received: from cpe-2606-a000-111b-4011-eaa3-4b92-4a68-8f24.dyn6.twc.com ([2606:a000:111b:4011:eaa3:4b92:4a68:8f24] helo=hmswarspite.think-freely.org) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1eikbI-0004ju-IL; Mon, 05 Feb 2018 12:30:58 -0500 Received: from hmswarspite.think-freely.org (localhost [127.0.0.1]) by hmswarspite.think-freely.org (8.15.2/8.15.2) with ESMTP id w15HU7OT021139; Mon, 5 Feb 2018 12:30:07 -0500 Received: (from nhorman@localhost) by hmswarspite.think-freely.org (8.15.2/8.15.2/Submit) id w15HU4ka021075; Mon, 5 Feb 2018 12:30:04 -0500 From: Neil Horman To: dev@dpdk.org Cc: Neil Horman , thomas@monjalon.net, john.mcnamara@intel.com, bruce.richardson@intel.com, Ferruh Yigit , Stephen Hemminger Date: Mon, 5 Feb 2018 12:29:51 -0500 Message-Id: <20180205172951.20683-1-nhorman@tuxdriver.com> X-Mailer: git-send-email 2.14.3 In-Reply-To: <20180115190545.25687-1-nhorman@tuxdriver.com> References: <20180115190545.25687-1-nhorman@tuxdriver.com> X-Spam-Score: -2.9 (--) X-Spam-Status: No Subject: [dpdk-dev] [PATCH v4] checkpatches.sh: Add checks for ABI symbol addition X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Feb 2018 17:31:04 -0000 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 CC: thomas@monjalon.net CC: john.mcnamara@intel.com CC: bruce.richardson@intel.com CC: Ferruh Yigit CC: Stephen Hemminger --- Change notes v2) * Cleaned up and documented awk script (shemminger) * fixed sort/uniq usage (shemminger) * moved checking to new script (tmonjalon) * added maintainer entry (tmonjalon) * added license (tmonjalon) v3) * Changed symbol check script name (tmonjalon) * Trapped exit to clean temp file (tmonjalon) * Honored verbose command (tmonjalon) * Cleaned left over debug bits (tmonjalon) * Updated location in MAINTAINERS file (tmonjalon) v4) * Updated maintainers file (tmonjalon) --- MAINTAINERS | 2 + devtools/check-symbol-change.sh | 146 ++++++++++++++++++++++++++++++++++++++++ devtools/checkpatches.sh | 23 ++++++- 3 files changed, 170 insertions(+), 1 deletion(-) create mode 100755 devtools/check-symbol-change.sh diff --git a/MAINTAINERS b/MAINTAINERS index acd056134..d1ef43479 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -86,9 +86,11 @@ M: Neil Horman 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 +M: Neil Horman F: buildtools/pmdinfogen/ F: usertools/dpdk-pmdinfo.py F: doc/guides/tools/pmdinfo.rst diff --git a/devtools/check-symbol-change.sh b/devtools/check-symbol-change.sh new file mode 100755 index 000000000..22b17e6f2 --- /dev/null +++ b/devtools/check-symbol-change.sh @@ -0,0 +1,146 @@ +#!/bin/sh + +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2018 Neil Horman + +build_map_changes() +{ + local fname=$1 + local mapdb=$2 + + cat $fname | filterdiff -i *.map | awk ' + # Initialize our variables + BEGIN {map="";sym="";ar="";sec=""; in_sec=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} + + # 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("+","");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). Otherwise we flag it as an + # unknown section + /^+[^}].*[^:*];/ {gsub(";","");sym=$2; + 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_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 ithan 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 + + diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh index 7676a6b50..0b2b5f039 100755 --- a/devtools/checkpatches.sh +++ b/devtools/checkpatches.sh @@ -35,6 +35,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 @@ -61,6 +63,7 @@ print_usage () { END_OF_HELP } + number=0 quiet=false verbose=false @@ -86,6 +89,7 @@ total=0 status=0 check () { # + local reta total=$(($total + 1)) ! $verbose || printf '\n### %s\n\n' "$3" if [ -n "$1" ] ; then @@ -96,9 +100,26 @@ check () { # <patch> <commit> <title> else report=$($DPDK_CHECKPATCH_PATH $options - 2>/dev/null) fi - [ $? -ne 0 ] || return 0 + reta=$? + $verbose || printf '\n### %s\n\n' "$3" printf '%s\n' "$report" | sed -n '1,/^total:.*lines checked$/p' + + ! $verbose || echo + ! $verbose || echo "Checking API additions/removals:" + + if [ -n "$1" ] ; then + report=$($VALIDATE_NEW_API $1) + elif [ -n "$2" ] ; then + report=$(git format-patch \ + --find-renames --no-stat --stdout -1 $commit | + $VALIDATE_NEW_API -) + else + report=$($VALIDATE_NEW_API -) + fi + [ $? -ne 0 -o $reta -ne 0 ] || return 0 + printf '%s\n' "$report" | sed -n '1,/^total:.*lines checked$/p' + status=$(($status + 1)) } -- 2.14.3