From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id ABBD7A058A; Fri, 17 Apr 2020 12:20:04 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 321081D711; Fri, 17 Apr 2020 12:20:04 +0200 (CEST) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id D6C3C1D6CF for ; Fri, 17 Apr 2020 12:20:02 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 84A645C0289; Fri, 17 Apr 2020 06:20:02 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Fri, 17 Apr 2020 06:20:02 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=6Gipk3fL4i/GhSoguIk++PhKq/8broGVCqkiV1T0nD0=; b=q946B0mZe/H1 wD4kLAW09OzaejHNWtatHqrk6/x93TnLKoe1I3MYjBC7PJDmbYrjwGFGYQt/z9nw qELOqeoogqDGrBku0USv1u0kLi9w1KmJB8ge24/Wi//6E8wB0BhX/tr/wZcBaBEO vYqlh+WUo62RQ3eeA93KDgq3lyjpQYY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=6Gipk3fL4i/GhSoguIk++PhKq/8broGVCqkiV1T0n D0=; b=atGNsg6brWWZlZnbOPgi2J9nPMNel/vKp72lQzoEIGX+Hm89s8Vsvh8Mj hcV8cajnfPRQQKU7Hh2V7JZZBnb8/q9e4EDkRe7rbN0dZs9PTijmBTrSx1PR4413 0qgcvAqeILXGX3K7uCsmM99M8Osh1cOI233gz5lB9E49/2Tjd3jDa0UvlTnM5XPY o1I9fkvfETQxdgSUBjCZ7w+oQOLyfZrSFhoJ4cEcbEGD5snwaRM8+thbwhG5YGwA 7PMv1DHvEDWkkbRjqjDXJNCeRke66U1gSj8yyM9iXd3KElC2ShzrdtCjZG2OXtlh 1Y21cAigJxCidKQumq1eNksonCdaA== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrfeejgddviecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucffoh hmrghinheplhhinhhugigsrghsvgdrohhrghdpshhouhhrtggvfigrrhgvrdhorhhgpdhk vghrnhgvlhdrohhrghenucfkphepjeejrddufeegrddvtdefrddukeegnecuvehluhhsth gvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhn jhgrlhhonhdrnhgvth X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 91F10306005C; Fri, 17 Apr 2020 06:20:01 -0400 (EDT) From: Thomas Monjalon To: Neil Horman , Ray Kinsella Cc: dev@dpdk.org, david.marchand@redhat.com Date: Fri, 17 Apr 2020 12:20:00 +0200 Message-ID: <10824099.L8ug28u51p@thomas> In-Reply-To: <311553a5-4c65-c312-cd4d-face87245f9a@ashroe.eu> References: <20200416145414.262296-1-nhorman@tuxdriver.com> <311553a5-4c65-c312-cd4d-face87245f9a@ashroe.eu> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 17/04/2020 12:11, Ray Kinsella: > > On 16/04/2020 15:54, Neil Horman wrote: > > Since we've moved away from our initial validate-abi.sh script, in > > favor of check-abi.sh, which uses libabigail, remove the old script from > > the tree, and update the docs accordingly > > > > Signed-off-by: Neil Horman > > CC: thomas@monjalon.net > > CC: david.marchand@redhat.com > > CC: mdr@ashroe.eu > > --- > > MAINTAINERS | 1 - > > devtools/validate-abi.sh | 251 --------------------- > > doc/guides/contributing/abi_versioning.rst | 50 ++-- > > 3 files changed, 18 insertions(+), 284 deletions(-) > > delete mode 100755 devtools/validate-abi.sh > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 4800f6884..84b633431 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -152,7 +152,6 @@ F: devtools/gen-abi.sh > > F: devtools/libabigail.abignore > > F: devtools/update-abi.sh > > F: devtools/update_version_map_abi.py > > -F: devtools/validate-abi.sh > > F: buildtools/check-experimental-syms.sh > > F: buildtools/map-list-symbol.sh > > > > diff --git a/devtools/validate-abi.sh b/devtools/validate-abi.sh > > deleted file mode 100755 > > index f64e19d38..000000000 > > --- a/devtools/validate-abi.sh > > +++ /dev/null > > @@ -1,251 +0,0 @@ > > -#!/usr/bin/env bash > > -# SPDX-License-Identifier: BSD-3-Clause > > -# Copyright(c) 2015 Neil Horman. All rights reserved. > > -# Copyright(c) 2017 6WIND S.A. > > -# All rights reserved > > - > > -set -e > > - > > -abicheck=abi-compliance-checker > > -abidump=abi-dumper > > -default_dst=abi-check > > -default_target=x86_64-native-linuxapp-gcc > > - > > -# trap on error > > -err_report() { > > - echo "$0: error at line $1" > > -} > > -trap 'err_report $LINENO' ERR > > - > > -print_usage () { > > - cat <<- END_OF_HELP > > - $(basename $0) [options] > > - > > - 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. > > - > > - and are git commit id or tags. > > - > > - Options: > > - -h show this help > > - -j enable parallel compilation with threads > > - -v show compilation logs on the console > > - -d change working directory (default is ${default_dst}) > > - -t 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 > > -} > > - > > -# 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 > > -} > > - > > -# 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() { > > - 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 > > -} > > - > > -# 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 > > -} > > - > > -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 [ $# != 2 ]; then > > - print_usage > > - exit 1 > > -fi > > - > > -tag1=$1 > > -tag2=$2 > > - > > -# convert path to absolute > > -case "${dst}" in > > - /*) ;; > > - *) dst=${PWD}/${dst} ;; > > -esac > > -dpdkroot=$(readlink -f $(dirname $0)/..) > > - > > -if [ -e "${dst}" -a "$force" = 0 ]; then > > - echo "The ${dst} directory is not empty. Remove it, use another" > > - echo "one (-d ), or force overriding (-f)" > > - 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" > > - > > -command -v ${abicheck} || log "INFO" "Can't find ${abicheck} utility" > > -command -v ${abidump} || log "INFO" "Can't find ${abidump} utility" > > - > > -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)" > > - > > -if [ "$hash1" = "$hash2" ]; then > > - log "ERROR" "$tag1 and $tag2 are the same revisions" > > - exit 1 > > -fi > > - > > -cmd mkdir -p ${dst} > > - > > -# dump abi for each revision > > -gen_abi ${hash1} > > -gen_abi ${hash2} > > - > > -# compare the abi dumps > > -cmd cd ${dst} > > -ret=0 > > -list="" > > -for i in ${hash2}/*.dump; do > > - name=`basename $i` > > - libname=${name%.dump} > > - > > - if [ ! -f ${hash1}/$name ]; then > > - log "INFO" "$NAME does not exist in $tag1. skipping..." > > - continue > > - fi > > - > > - 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 > > - > > -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 > > - > > -log "INFO" "Logs are in ${dst}/abi-check.log" > > -log "INFO" "HTML reports are in ${dst}/compat_reports directory" > > - > > -exit $ret > > diff --git a/doc/guides/contributing/abi_versioning.rst b/doc/guides/contributing/abi_versioning.rst > > index a21f4e7a4..219823923 100644 > > --- a/doc/guides/contributing/abi_versioning.rst > > +++ b/doc/guides/contributing/abi_versioning.rst > > @@ -482,41 +482,27 @@ Running the ABI Validator > > ------------------------- > > > > The ``devtools`` directory in the DPDK source tree contains a utility program, > > -``validate-abi.sh``, for validating the DPDK ABI based on the Linux `ABI > > -Compliance Checker > > -`_. > > +``check-abi.sh``, for validating the DPDK ABI based on the libabigail `abidiff > > +utility: `_ > > > > -This has a dependency on the ``abi-compliance-checker`` and ``and abi-dumper`` > > -utilities which can be installed via a package manager. For example:: > > +The syntax of the ``check-abi.sh`` utility is:: > > > > - sudo yum install abi-compliance-checker > > - sudo yum install abi-dumper > > + ./devtools/check-abi.sh > > > > -The syntax of the ``validate-abi.sh`` utility is:: > > +Where specifies the directory housing the reference build of dpdk, and > > + specifies the dpdk build directory to check the abi of > > > > - ./devtools/validate-abi.sh > > +Example: > > +To compare your build branch to the ABI of the master branch > > +after you have built your branch > > > > -Where ``REV1`` and ``REV2`` are valid gitrevisions(7) > > -https://www.kernel.org/pub/software/scm/git/docs/gitrevisions.html > > -on the local repo. > > +#. $ cd > > +#. $ mkdir ~/ref > > +#. $ git clone --local --no-hardlinks --single-branch -b master . ~/ref > > +#. $ cd ~/ref > > +#. $ cp /.config ./.config > > +#. $ make defconfig > > +#. $ make > > +#. $ /devtools/check-abi.sh ~/ref > > > > -For example:: > > - > > - # Check between the previous and latest commit: > > - ./devtools/validate-abi.sh HEAD~1 HEAD > > - > > - # Check on a specific compilation target: > > - ./devtools/validate-abi.sh -t x86_64-native-linux-gcc HEAD~1 HEAD > > - > > - # Check between two tags: > > - ./devtools/validate-abi.sh v2.0.0 v2.1.0 > > - > > - # Check between git master and local topic-branch "vhost-hacking": > > - ./devtools/validate-abi.sh master vhost-hacking > > - > > -After the validation script completes (it can take a while since it need to > > -compile both tags) it will create compatibility reports in the > > -``./abi-check/compat_report`` directory. Listed incompatibilities can be found > > -as follows:: > > - > > - grep -lr Incompatible abi-check/compat_reports/ > > + > > > > check-abi.sh appears to be backward step in terms of usability. No, check-abi.sh benefits from a nice integration in build scripts. See below. > With validate-abi.sh I do can do a "validate-abi.sh HEAD~1 HEAD". > And it will do the build, install, dump and comparison for me. > And it picked up my 20.0.2 - > 21.0 changes no problem. > > With check-abi on the other hand, I need to the build and install myself. > check-abi requires dump files, but I see no reference in the documentation to how these are created. > It silently fails when it doesn't find any ... > > Do I run abi-dumper on the so's myself, or how does it work? check-abi.sh is integrated in test-build.sh and test-meson-builds.sh. Probably we should document usage in these scripts.