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 214AF5A86 for ; Tue, 17 Mar 2015 19:08:37 +0100 (CET) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1YXvun-0000Ec-In; Tue, 17 Mar 2015 14:08:35 -0400 Date: Tue, 17 Mar 2015 14:08:28 -0400 From: Neil Horman To: Thomas Monjalon Message-ID: <20150317180828.GB8509@hmsreliant.think-freely.org> References: <1422652596-12777-1-git-send-email-nhorman@tuxdriver.com> <1426255750-3961-1-git-send-email-nhorman@tuxdriver.com> <8343380.8uC93YKvWO@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8343380.8uC93YKvWO@xps13> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v4] ABI: Add abi checking utility X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Mar 2015 18:08:37 -0000 On Tue, Mar 17, 2015 at 04:42:31PM +0100, Thomas Monjalon wrote: > Hi Neil, > > I tested this tool and I see few small improvements possible. > I'll fix the bug you found, but I'm not going to go chasing every feature that you happen to note. Not saying they're not fine features, but I don't have time to implement features that you happen to note might be nice to have, especially not in time for 2.0. Regarding your question about report tolerance, Its not that kind of tool. The ABI checker simply calls a developers attention to symbols that have inadvertently changed due to code or data structure modifications. It is incumbent on the developer to make a well informed decision about how to handle those changes (via deprecation/versioning/etc), and to defend his/her reasoning. Neil > 2015-03-13 10:09, Neil Horman: > > There was a request for an abi validation utilty for the ongoing ABI stability > utility > > work. As it turns out there is a abi compliance checker in development that > > seems to be under active development and provides fairly detailed ABI compliance > > reports. Its not yet intellegent enough to understand symbol versioning, but it > intelligent > > does provide the ability to identify symbols which have changed between > > releases, along with details of the change, and offers developers the > > opportunity to identify which symbols then need versioning and validation for a > > given update via manual testing. > > > > This script automates the use of the compliance checker between two arbitrarily > > specified tags within the dpdk tree. To execute enter the $RTE_SDK directory > > and run: > > > > ./scripts/validate_abi.sh $GIT_TAG1 $GIT_TAG2 $CONFIG > > > > where $GIT_TAG1 and 2 are git tags and $CONFIG is a config specification > > suitable for passing as the T= variable in the make config command. > > > > Note the upstream source for the abi compliance checker is here: > > http://ispras.linuxbase.org/index.php/ABI_compliance_checker > > > > It generates a report for each DSO built from the requested tags that developers > > can review to find ABI compliance issues. > > > > Signed-off-by: Neil Horman > > > > --- > > > > Change Notes: > > > > v2) Fixed some typos as requested by Thomas > > > > v3) Fixed some additional typos Thomas requested > > Improved script to work from detached state > > Added some documentation to the changelog > > Added some comments to the scripts > > > > v4) Remove duplicate exports. > > Move restoration of starting branch/comit to cleanup_and_exit > > --- > [...] > > +TAG1=$1 > > +TAG2=$2 > > +TARGET=$3 > > +ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX` > > +JOBS=$(grep -c '^processor' /proc/cpuinfo) > > [...] > > +cleanup_and_exit() { > > + rm -rf $ABI_DIR > > + exit $1 > > + git checkout $CURRENT_BRANCH > > Checkout is never done because of previous exit. > > > +} > [...] > > +log "INFO" "Checking out version $TAG1 of the dpdk" > > +# Move to the old version of the tree > > +git checkout $TAG1 > > What about -q for quiet mode? > > [...] > > +log "INFO" "Building DPDK $TAG1. This might take a moment" > > +make O=$TARGET > $VERBOSE 2>&1 > > -j$JOBS would improve building time > > [...] > > +# Move to the new version of the tree > > +log "INFO" "Checking out version $TAG2 of the dpdk" > > +git checkout $TAG2 > > -q ? > > [...] > > +log "INFO" "Building DPDK $TAG2. This might take a moment" > > +make O=$TARGET > $VERBOSE 2>&1 > > -j ? > > [...] > > +# Start comparison of ABI dumps > > +for i in `ls $ABI_DIR/*-1.dump` > > +do > > + NEWNAME=`basename $i` > > + OLDNAME=`basename $i | sed -e"s/1.dump/0.dump/"` > > + LIBNAME=`basename $i | sed -e"s/-ABI-1.dump//"` > > + > > + if [ ! -f $ABI_DIR/$OLDNAME ] > > + then > > + log "INFO" "$OLDNAME DOES NOT EXIST IN $TAG1. SKIPPING..." > > + fi > > + > > + #compare the abi dumps > > + $ABICHECK -l $LIBNAME -old $ABI_DIR/$OLDNAME -new $ABI_DIR/$NEWNAME > > +done > > It would be more convenient to generate an HTML index giving access to every > reports for every DSOs. > > > + > > +git reset --hard > > +log "INFO" "ABI CHECK COMPLETE. REPORTS ARE IN compat_report directory" > > +cleanup_and_exit 0 > > After reading the report, it's not clear what would be tolerated or not. > Should we forbid every defects? > >