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 1E2F6A058A; Fri, 17 Apr 2020 13:47:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7B1841E4B2; Fri, 17 Apr 2020 13:47:00 +0200 (CEST) Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 3EF7B1D6E3 for ; Fri, 17 Apr 2020 13:46:59 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 8CCDA5C00AB; Fri, 17 Apr 2020 07:46:58 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Fri, 17 Apr 2020 07:46:58 -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=zDPQEs0jtmUqLnr9pmNY/bWGyB4nTajKnAUzwBK93Tw=; b=BKT70pL6igYi wYTK65ymZtCSxyfylCY+NSbSilVHNhA6UWN00aVWQrLRilWJ6qOfbpQwYdDFwFoL EnjPehe+nVX1OStExAKRr6r8hSHYqpylJJIyAsTFdYpHMoH9BVSENafqe26Fokwt Rjv8yWnoUkFo3sOiR/lF3A8oLz8V4GY= 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=zDPQEs0jtmUqLnr9pmNY/bWGyB4nTajKnAUzwBK93 Tw=; b=pRB8lwKkxamChk8ScgOCGSBzjW+OgGS98KaCZdzzJxzXyigtxpLV50D2j hqB6jEkrZ3wp8xscyxPbnSVtkYsa43rM5MF7g8bsByflwin+yabQzLBjf+LvGc6H pdJe5ltAgZEtBMz+7efnv7NcdUNvE9jqarVyBrhjnOL9ztO0gUH3Uq2gROoe8oa3 E3XuwajUdgiAP/EEtfspl9MEfttQz8nGguzaDFO7ohyI6szxAkE1SP/5SNpWPvxl zs+/RTxqHDh5L+C0jukVjMra2Xyb5p9K3BD/4hbUpkeqN1LwPOMnNyLRUMt+YO7q 5QEanFSwZ+wzpL1fP5I1P7GuC6RvQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrfeejgdegfecutefuodetggdotefrodftvf 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 1EAD6328005A; Fri, 17 Apr 2020 07:46:56 -0400 (EDT) From: Thomas Monjalon To: Neil Horman , Ray Kinsella Cc: dev@dpdk.org, david.marchand@redhat.com Date: Fri, 17 Apr 2020 13:46:53 +0200 Message-ID: <8424599.A5hrfCrGMc@thomas> In-Reply-To: <993eefb3-2529-aaeb-d2ac-a3ca48d52157@ashroe.eu> References: <20200416145414.262296-1-nhorman@tuxdriver.com> <10824099.L8ug28u51p@thomas> <993eefb3-2529-aaeb-d2ac-a3ca48d52157@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:35, Ray Kinsella: > On 17/04/2020 11:20, Thomas Monjalon wrote: > > 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. > > > > ok in that case, this documentation should reference those scripts instead? Maybe both? No strong opinion.