From: Neil Horman <nhorman@tuxdriver.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: dev@dpdk.org, bruce.richardson@intel.com
Subject: Re: [dpdk-dev] [PATCH v2] devtools: rework abi checker script
Date: Fri, 8 Sep 2017 09:46:49 -0400 [thread overview]
Message-ID: <20170908134649.GA4086@hmswarspite.think-freely.org> (raw)
In-Reply-To: <20170906145101.11537-1-olivier.matz@6wind.com>
On Wed, Sep 06, 2017 at 04:51:01PM +0200, Olivier Matz wrote:
> The intiatial 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>
> ---
>
> v1->v2:
> - use /usr/bin/env to find bash (which is required)
> - fix displayed path to html reports
> - reword help for -f option
>
This still doesn't seem to work. Running the following:
./validate-abi.sh v17.05 v17.08
produces no report, and the end of the abi-check.log file contains:
[nhorman@hmswarspite abi-check]$ tail -n 10 ./abi-check.log
CMD: abi-dumper librte_vhost.so -o /home/nhorman/git/dpdk/devtools/abi-check/222555480/librte_vhost.so.dump -lver 222555480
Reading debug-info
WARNING: incompatible build option detected: -O0 (required -Og for better analysis)
Creating ABI dump
The object ABI has been dumped to:
/home/nhorman/git/dpdk/devtools/abi-check/222555480/librte_vhost.so.dump
CMD: cd ../..
CMD: git clone ./.. /home/nhorman/git/dpdk/devtools/abi-check/02657b4ad
fatal: repository './..' does not exist
The warning I assume should be fixed to, but the second clone failing seems to
be the most salient bit here
Neil
> devtools/validate-abi.sh | 389 ++++++++++++++++++++++++-----------------------
> 1 file changed, 197 insertions(+), 192 deletions(-)
>
> diff --git a/devtools/validate-abi.sh b/devtools/validate-abi.sh
> index 0accc99b1..d747db189 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,240 @@
> # (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
> + 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
> +
> + log "CMD" "$s"
> + "$@"
> +}
>
> -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() {
> + cmd git clone $(dirname $0)/.. ${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 > ${dst}/log 2>&1
> +
> + # 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 -O0" \
> + 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
> + cmd cd ../..
> +}
>
> -#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
>
> -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
> +ret=0
> +list=""
> +for i in ${dst}/${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 $dst/${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 $dst/${hash1}/$name -new $dst/${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" "At least one call to $abicheck returned an error."
> + log "NOTICE" "ABI may be incompatible, please check 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}/${hash2}/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
>
>
next prev parent reply other threads:[~2017-09-08 13:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-30 13:51 [dpdk-dev] [PATCH] " 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 ` [dpdk-dev] [PATCH v2] " Olivier Matz
2017-09-08 13:46 ` Neil Horman [this message]
2017-09-11 8:18 ` Olivier MATZ
2017-09-11 8:46 ` [dpdk-dev] [PATCH v3] " Olivier Matz
2017-09-13 15:00 ` Neil Horman
2017-09-19 9:15 ` Olivier MATZ
2017-09-20 9:12 ` [dpdk-dev] [PATCH v4] " Olivier Matz
2017-09-21 15:40 ` Neil Horman
2017-09-25 9:11 ` Olivier MATZ
2017-09-25 11:21 ` Neil Horman
2017-10-05 7:53 ` [dpdk-dev] [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=20170908134649.GA4086@hmswarspite.think-freely.org \
--to=nhorman@tuxdriver.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=olivier.matz@6wind.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).