All of lore.kernel.org
 help / color / mirror / Atom feed
From: Olivier Matz <olivier.matz@6wind.com>
To: dev@dpdk.org, nhorman@tuxdriver.com, bruce.richardson@intel.com
Subject: [PATCH v4] devtools: rework abi checker script
Date: Wed, 20 Sep 2017 11:12:53 +0200	[thread overview]
Message-ID: <20170920091253.15794-1-olivier.matz@6wind.com> (raw)
In-Reply-To: <20170911084635.11707-1-olivier.matz@6wind.com>

The initial version of the script had some limitations:
- cannot work on a non-clean workspace
- environment variables are not documented
- no compilation log in case of failure
- return success even it abi is incompatible

This patch addresses these issues and rework the code.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

v3->v4:
- clarify logs on incompatible abi
- log when an error returned an error
- [really] fix the report path
- log the output of make config in the proper file

v2->v3:
- fix when not launched from dpdk root dir
- use "-Og -Wno-error" instead of "-O0"
- fix typo in commit log

v1->v2:
- use /usr/bin/env to find bash (which is required)
- fix displayed path to html reports
- reword help for -f option


 devtools/validate-abi.sh | 397 ++++++++++++++++++++++++-----------------------
 1 file changed, 205 insertions(+), 192 deletions(-)

diff --git a/devtools/validate-abi.sh b/devtools/validate-abi.sh
index 0accc99b1..8caf43e83 100755
--- a/devtools/validate-abi.sh
+++ b/devtools/validate-abi.sh
@@ -1,7 +1,8 @@
-#!/bin/sh
+#!/usr/bin/env bash
 #   BSD LICENSE
 #
 #   Copyright(c) 2015 Neil Horman. All rights reserved.
+#   Copyright(c) 2017 6WIND S.A.
 #   All rights reserved.
 #
 #   Redistribution and use in source and binary forms, with or without
@@ -27,236 +28,248 @@
 #   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 #   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-TAG1=$1
-TAG2=$2
-TARGET=$3
-ABI_DIR=`mktemp -d -p /tmp ABI.XXXXXX`
+set -e
 
-usage() {
-	echo "$0 <REV1> <REV2> <TARGET>"
-}
+abicheck=abi-compliance-checker
+abidump=abi-dumper
+default_dst=abi-check
+default_target=x86_64-native-linuxapp-gcc
 
-log() {
-	local level=$1
-	shift
-	echo "$*"
+# trap on error
+err_report() {
+    echo "$0: error at line $1"
 }
+trap 'err_report $LINENO' ERR
 
-validate_tags() {
+print_usage () {
+	cat <<- END_OF_HELP
+	$(basename $0) [options] <rev1> <rev2>
 
-	if [ -z "$HASH1" ]
-	then
-		echo "invalid revision: $TAG1"
-		return
-	fi
-	if [ -z "$HASH2" ]
-	then
-		echo "invalid revision: $TAG2"
-		return
-	fi
+	This script compares the ABI of 2 git revisions of the current
+	workspace. The output is a html report and a compilation log.
+
+	The objective is to make sure that applications built against
+	DSOs from the first revision can still run when executed using
+	the DSOs built from the second revision.
+
+	<rev1> and <rev2> are git commit id or tags.
+
+	Options:
+	  -h		show this help
+	  -j <num>	enable parallel compilation with <num> threads
+	  -v		show compilation logs on the console
+	  -d <dir>	change working directory (default is ${default_dst})
+	  -t <target>	the dpdk target to use (default is ${default_target})
+	  -f		overwrite existing files in destination directory
+
+	The script returns 0 on success, or the value of last failing
+	call of ${abicheck} (incompatible abi or the tool has run with errors).
+	The errors returned by ${abidump} are ignored.
+
+	END_OF_HELP
 }
 
-validate_args() {
-	if [ -z "$TAG1" ]
-	then
-		echo "Must Specify REV1"
-		return
-	fi
-	if [ -z "$TAG2" ]
-	then
-		echo "Must Specify REV2"
-		return
-	fi
-	if [ -z "$TARGET" ]
-	then
-		echo "Must Specify a build target"
+# log in the file, and on stdout if verbose
+# $1: level string
+# $2: string to be logged
+log() {
+	echo "$1: $2"
+	if [ "${verbose}" != "true" ]; then
+		echo "$1: $2" >&3
 	fi
 }
 
+# launch a command and log it, taking care of surrounding spaces with quotes
+cmd() {
+	local i s whitespace ret
+	s=""
+	whitespace="[[:space:]]"
+	for i in "$@"; do
+		if [[ $i =~ $whitespace ]]; then
+			i=\"$i\"
+		fi
+		if [ -z "$s" ]; then
+			s="$i"
+		else
+			s="$s $i"
+		fi
+	done
+
+	ret=0
+	log "CMD" "$s"
+	"$@" || ret=$?
+	if [ "$ret" != "0" ]; then
+		log "CMD" "previous command returned $ret"
+	fi
+
+	return $ret
+}
 
-cleanup_and_exit() {
-	rm -rf $ABI_DIR
-	git checkout $CURRENT_BRANCH
-	exit $1
+# redirect or copy stderr/stdout to a file
+# the syntax is unfamiliar, but it makes the rest of the
+# code easier to read, avoiding the use of pipes
+set_log_file() {
+	# save original stdout and stderr in fd 3 and 4
+	exec 3>&1
+	exec 4>&2
+	# create a new fd 5 that send to a file
+	exec 5> >(cat > $1)
+	# send stdout and stderr to fd 5
+	if [ "${verbose}" = "true" ]; then
+		exec 1> >(tee /dev/fd/5 >&3)
+		exec 2> >(tee /dev/fd/5 >&4)
+	else
+		exec 1>&5
+		exec 2>&5
+	fi
 }
 
 # Make sure we configure SHARED libraries
 # Also turn off IGB and KNI as those require kernel headers to build
 fixup_config() {
-	sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" config/defconfig_$TARGET
-	sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" config/defconfig_$TARGET
-	sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" config/defconfig_$TARGET
-	sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" config/defconfig_$TARGET
-	sed -i -e"$ a\CONFIG_RTE_KNI_KMOD=n" config/defconfig_$TARGET
+	local conf=config/defconfig_$target
+	cmd sed -i -e"$ a\CONFIG_RTE_BUILD_SHARED_LIB=y" $conf
+	cmd sed -i -e"$ a\CONFIG_RTE_NEXT_ABI=n" $conf
+	cmd sed -i -e"$ a\CONFIG_RTE_EAL_IGB_UIO=n" $conf
+	cmd sed -i -e"$ a\CONFIG_RTE_LIBRTE_KNI=n" $conf
+	cmd sed -i -e"$ a\CONFIG_RTE_KNI_KMOD=n" $conf
 }
 
-###########################################
-#START
-############################################
+# build dpdk for the given tag and dump abi
+# $1: hash of the revision
+gen_abi() {
+	local i
+
+	cmd git clone ${dpdkroot} ${dst}/${1}
+	cmd cd ${dst}/${1}
+
+	log "INFO" "Checking out version ${1} of the dpdk"
+	# Move to the old version of the tree
+	cmd git checkout ${1}
+
+	fixup_config
+
+	# Now configure the build
+	log "INFO" "Configuring DPDK ${1}"
+	cmd make config T=$target O=$target
+
+	# Checking abi compliance relies on using the dwarf information in
+	# the shared objects. Build with -g to include them.
+	log "INFO" "Building DPDK ${1}. This might take a moment"
+	cmd make -j$parallel O=$target V=1 EXTRA_CFLAGS="-g -Og -Wno-error" \
+	    EXTRA_LDFLAGS="-g" || log "INFO" "The build failed"
+
+	# Move to the lib directory
+	cmd cd ${PWD}/$target/lib
+	log "INFO" "Collecting ABI information for ${1}"
+	for i in *.so; do
+		[ -e "$i" ] || break
+		cmd $abidump ${i} -o $dst/${1}/${i}.dump -lver ${1} || true
+		# hack to ignore empty SymbolsInfo section (no public ABI)
+		if grep -q "'SymbolInfo' => {}," $dst/${1}/${i}.dump \
+				2> /dev/null; then
+			log "INFO" "${i} has no public ABI, remove dump file"
+			cmd rm -f $dst/${1}/${i}.dump
+		fi
+	done
+}
 
-#trap on ctrl-c to clean up
-trap cleanup_and_exit SIGINT
+verbose=false
+parallel=1
+dst=${default_dst}
+target=${default_target}
+force=0
+while getopts j:vd:t:fh ARG ; do
+	case $ARG in
+		j ) parallel=$OPTARG ;;
+		v ) verbose=true ;;
+		d ) dst=$OPTARG ;;
+		t ) target=$OPTARG ;;
+		f ) force=1 ;;
+		h ) print_usage ; exit 0 ;;
+		? ) print_usage ; exit 1 ;;
+	esac
+done
+shift $(($OPTIND - 1))
 
-if [ -z "$DPDK_MAKE_JOBS" ]
-then
-	# This counts the number of cpus on the system
-	if [ -e /usr/bin/lscpu ]
-	then
-		DPDK_MAKE_JOBS=`lscpu -p=cpu | grep -v "#" | wc -l`
-	else
-		DPDK_MAKE_JOBS=1
-	fi
+if [ $# != 2 ]; then
+	print_usage
+	exit 1
 fi
 
-#Save the current branch
-CURRENT_BRANCH=`git branch | grep \* | cut -d' ' -f2`
+tag1=$1
+tag2=$2
 
-if [ -z "$CURRENT_BRANCH" ]
-then
-	CURRENT_BRANCH=`git log --pretty=format:%H HEAD~1..HEAD`
-fi
+# convert path to absolute
+case "${dst}" in
+	/*) ;;
+	*) dst=${PWD}/${dst} ;;
+esac
+dpdkroot=$(readlink -e $(dirname $0)/..)
 
-if [ -n "$VERBOSE" ]
-then
-	export VERBOSE=/dev/stdout
-else
-	export VERBOSE=/dev/null
+if [ -e "${dst}" -a "$force" = 0 ]; then
+	echo "The ${dst} directory is not empty. Remove it, use another"
+	echo "one (-d <dir>), or force overriding (-f)"
+	exit 1
 fi
 
-# Validate that we have all the arguments we need
-res=$(validate_args)
-if [ -n "$res" ]
-then
-	echo $res
-	usage
-	cleanup_and_exit 1
-fi
+rm -rf ${dst}
+mkdir -p ${dst}
+set_log_file ${dst}/abi-check.log
+log "INFO" "Logs available in ${dst}/abi-check.log"
 
-HASH1=$(git show -s --format=%H "$TAG1" -- 2> /dev/null | tail -1)
-HASH2=$(git show -s --format=%H "$TAG2" -- 2> /dev/null | tail -1)
+command -v ${abicheck} || log "INFO" "Can't find ${abicheck} utility"
+command -v ${abidump} || log "INFO" "Can't find ${abidump} utility"
 
-# Make sure our tags exist
-res=$(validate_tags)
-if [ -n "$res" ]
-then
-	echo $res
-	cleanup_and_exit 1
-fi
+hash1=$(git show -s --format=%h "$tag1" -- 2> /dev/null | tail -1)
+hash2=$(git show -s --format=%h "$tag2" -- 2> /dev/null | tail -1)
 
 # Make hashes available in output for non-local reference
-TAG1="$TAG1 ($HASH1)"
-TAG2="$TAG2 ($HASH2)"
-
-ABICHECK=`which abi-compliance-checker 2>/dev/null`
-if [ $? -ne 0 ]
-then
-	log "INFO" "Can't find abi-compliance-checker utility"
-	cleanup_and_exit 1
-fi
-
-ABIDUMP=`which abi-dumper 2>/dev/null`
-if [ $? -ne 0 ]
-then
-	log "INFO" "Can't find abi-dumper utility"
-	cleanup_and_exit 1
-fi
+tag1="$tag1 ($hash1)"
+tag2="$tag2 ($hash2)"
 
-log "INFO" "We're going to check and make sure that applications built"
-log "INFO" "against DPDK DSOs from version $TAG1 will still run when executed"
-log "INFO" "against DPDK DSOs built from version $TAG2."
-log "INFO" ""
-
-# Check to make sure we have a clean tree
-git status | grep -q clean
-if [ $? -ne 0 ]
-then
-	log "WARN" "Working directory not clean, aborting"
-	cleanup_and_exit 1
+if [ "$hash1" = "$hash2" ]; then
+	log "ERROR" "$tag1 and $tag2 are the same revisions"
+	exit 1
 fi
 
-# Move to the root of the git tree
-cd $(dirname $0)/..
+cmd mkdir -p ${dst}
 
-log "INFO" "Checking out version $TAG1 of the dpdk"
-# Move to the old version of the tree
-git checkout $HASH1
+# dump abi for each revision
+gen_abi ${hash1}
+gen_abi ${hash2}
 
-fixup_config
+# compare the abi dumps
+cmd cd ${dst}
+ret=0
+list=""
+for i in ${hash2}/*.dump; do
+	name=`basename $i`
+	libname=${name%.dump}
 
-# Checking abi compliance relies on using the dwarf information in
-# The shared objects.  Thats only included in the DSO's if we build
-# with -g
-export EXTRA_CFLAGS="$EXTRA_CFLAGS -g -O0"
-export EXTRA_LDFLAGS="$EXTRA_LDFLAGS -g"
-
-# Now configure the build
-log "INFO" "Configuring DPDK $TAG1"
-make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
-
-log "INFO" "Building DPDK $TAG1. This might take a moment"
-make -j$DPDK_MAKE_JOBS O=$TARGET > $VERBOSE 2>&1
-
-if [ $? -ne 0 ]
-then
-	log "INFO" "THE BUILD FAILED.  ABORTING"
-	cleanup_and_exit 1
-fi
+	if [ ! -f ${hash1}/$name ]; then
+		log "INFO" "$NAME does not exist in $tag1. skipping..."
+		continue
+	fi
 
-# Move to the lib directory
-cd $TARGET/lib
-log "INFO" "COLLECTING ABI INFORMATION FOR $TAG1"
-for i in `ls *.so`
-do
-	$ABIDUMP $i -o $ABI_DIR/$i-ABI-0.dump -lver $HASH1
+	local_ret=0
+	cmd $abicheck -l $libname \
+	    -old ${hash1}/$name -new ${hash2}/$name || local_ret=$?
+	if [ $local_ret != 0 ]; then
+		log "NOTICE" "$abicheck returned $local_ret"
+		ret=$local_ret
+		list="$list $libname"
+	fi
 done
-cd ../..
-
-# Now clean the tree, checkout the second tag, and rebuild
-git clean -f -d
-git reset --hard
-# Move to the new version of the tree
-log "INFO" "Checking out version $TAG2 of the dpdk"
-git checkout $HASH2
-
-fixup_config
-
-# Now configure the build
-log "INFO" "Configuring DPDK $TAG2"
-make config T=$TARGET O=$TARGET > $VERBOSE 2>&1
-
-log "INFO" "Building DPDK $TAG2. This might take a moment"
-make -j$DPDK_MAKE_JOBS O=$TARGET > $VERBOSE 2>&1
 
-if [ $? -ne 0 ]
-then
-	log "INFO" "THE BUILD FAILED.  ABORTING"
-	cleanup_and_exit 1
+if [ $ret != 0 ]; then
+	log "NOTICE" "ABI may be incompatible, check reports/logs for details."
+	log "NOTICE" "Incompatible list: $list"
+else
+	log "NOTICE" "No error detected, ABI is compatible."
 fi
 
-cd $TARGET/lib
-log "INFO" "COLLECTING ABI INFORMATION FOR $TAG2"
-for i in `ls *.so`
-do
-	$ABIDUMP $i -o $ABI_DIR/$i-ABI-1.dump -lver $HASH2
-done
-cd ../..
-
-# 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
+log "INFO" "Logs are in ${dst}/abi-check.log"
+log "INFO" "HTML reports are in ${dst}/compat_reports directory"
 
-git reset --hard
-log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
-cleanup_and_exit 0
+exit $ret
-- 
2.11.0

  parent reply	other threads:[~2017-09-20  9:13 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-30 13:51 [PATCH] devtools: rework abi checker script Olivier Matz
2017-09-04 16:00 ` Bruce Richardson
2017-09-04 16:03   ` Bruce Richardson
2017-09-04 16:13     ` Bruce Richardson
2017-09-06 14:51 ` [PATCH v2] " Olivier Matz
2017-09-08 13:46   ` Neil Horman
2017-09-11  8:18     ` Olivier MATZ
2017-09-11  8:46       ` [PATCH v3] " Olivier Matz
2017-09-13 15:00         ` Neil Horman
2017-09-19  9:15           ` Olivier MATZ
2017-09-20  9:12         ` Olivier Matz [this message]
2017-09-21 15:40           ` [PATCH v4] " Neil Horman
2017-09-25  9:11             ` Olivier MATZ
2017-09-25 11:21               ` Neil Horman
2017-10-05  7:53           ` [PATCH v5] " Olivier Matz
2017-10-05 13:15             ` Neil Horman
2017-11-07 23:24             ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170920091253.15794-1-olivier.matz@6wind.com \
    --to=olivier.matz@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=nhorman@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.