* [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree @ 2020-04-16 14:54 Neil Horman 2020-04-17 10:11 ` Ray Kinsella 2020-05-24 20:34 ` [dpdk-dev] [PATCH v4] devtools: remove old ABI validation script Thomas Monjalon 0 siblings, 2 replies; 24+ messages in thread From: Neil Horman @ 2020-04-16 14:54 UTC (permalink / raw) To: dev; +Cc: Neil Horman, thomas, david.marchand, mdr 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 <nhorman@tuxdriver.com> 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] <rev1> <rev2> - - 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 -} - -# 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 <dir>), 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 -<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_. +``check-abi.sh``, for validating the DPDK ABI based on the libabigail `abidiff +utility: <https://sourceware.org/libabigail/manual/abidiff.html>`_ -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 <refdir> <newdir> -The syntax of the ``validate-abi.sh`` utility is:: +Where <refdir> specifies the directory housing the reference build of dpdk, and +<newdir> specifies the dpdk build directory to check the abi of - ./devtools/validate-abi.sh <REV1> <REV2> +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 <path to dpdk src tree> +#. $ mkdir ~/ref +#. $ git clone --local --no-hardlinks --single-branch -b master . ~/ref +#. $ cd ~/ref +#. $ cp <path to dpdk src tree from above>/.config ./.config +#. $ make defconfig +#. $ make +#. $ <path to dpdk src tree>/devtools/check-abi.sh ~/ref <path to dpdk src tree> -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/ + -- 2.25.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-16 14:54 [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree Neil Horman @ 2020-04-17 10:11 ` Ray Kinsella 2020-04-17 10:20 ` Thomas Monjalon 2020-05-24 20:34 ` [dpdk-dev] [PATCH v4] devtools: remove old ABI validation script Thomas Monjalon 1 sibling, 1 reply; 24+ messages in thread From: Ray Kinsella @ 2020-04-17 10:11 UTC (permalink / raw) To: Neil Horman, dev; +Cc: thomas, david.marchand 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 <nhorman@tuxdriver.com> > 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] <rev1> <rev2> > - > - 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 > -} > - > -# 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 <dir>), 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 > -<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_. > +``check-abi.sh``, for validating the DPDK ABI based on the libabigail `abidiff > +utility: <https://sourceware.org/libabigail/manual/abidiff.html>`_ > > -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 <refdir> <newdir> > > -The syntax of the ``validate-abi.sh`` utility is:: > +Where <refdir> specifies the directory housing the reference build of dpdk, and > +<newdir> specifies the dpdk build directory to check the abi of > > - ./devtools/validate-abi.sh <REV1> <REV2> > +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 <path to dpdk src tree> > +#. $ mkdir ~/ref > +#. $ git clone --local --no-hardlinks --single-branch -b master . ~/ref > +#. $ cd ~/ref > +#. $ cp <path to dpdk src tree from above>/.config ./.config > +#. $ make defconfig > +#. $ make > +#. $ <path to dpdk src tree>/devtools/check-abi.sh ~/ref <path to dpdk src tree> > > -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. 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? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 10:11 ` Ray Kinsella @ 2020-04-17 10:20 ` Thomas Monjalon 2020-04-17 10:35 ` Ray Kinsella 2020-04-17 11:47 ` Ray Kinsella 0 siblings, 2 replies; 24+ messages in thread From: Thomas Monjalon @ 2020-04-17 10:20 UTC (permalink / raw) To: Neil Horman, Ray Kinsella; +Cc: dev, david.marchand 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 <nhorman@tuxdriver.com> > > 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] <rev1> <rev2> > > - > > - 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 > > -} > > - > > -# 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 <dir>), 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 > > -<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_. > > +``check-abi.sh``, for validating the DPDK ABI based on the libabigail `abidiff > > +utility: <https://sourceware.org/libabigail/manual/abidiff.html>`_ > > > > -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 <refdir> <newdir> > > > > -The syntax of the ``validate-abi.sh`` utility is:: > > +Where <refdir> specifies the directory housing the reference build of dpdk, and > > +<newdir> specifies the dpdk build directory to check the abi of > > > > - ./devtools/validate-abi.sh <REV1> <REV2> > > +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 <path to dpdk src tree> > > +#. $ mkdir ~/ref > > +#. $ git clone --local --no-hardlinks --single-branch -b master . ~/ref > > +#. $ cd ~/ref > > +#. $ cp <path to dpdk src tree from above>/.config ./.config > > +#. $ make defconfig > > +#. $ make > > +#. $ <path to dpdk src tree>/devtools/check-abi.sh ~/ref <path to dpdk src tree> > > > > -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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 10:20 ` Thomas Monjalon @ 2020-04-17 10:35 ` Ray Kinsella 2020-04-17 11:46 ` Thomas Monjalon 2020-04-17 11:47 ` Ray Kinsella 1 sibling, 1 reply; 24+ messages in thread From: Ray Kinsella @ 2020-04-17 10:35 UTC (permalink / raw) To: Thomas Monjalon, Neil Horman; +Cc: dev, david.marchand 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 <nhorman@tuxdriver.com> >>> 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] <rev1> <rev2> >>> - >>> - 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 >>> -} >>> - >>> -# 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 <dir>), 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 >>> -<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_. >>> +``check-abi.sh``, for validating the DPDK ABI based on the libabigail `abidiff >>> +utility: <https://sourceware.org/libabigail/manual/abidiff.html>`_ >>> >>> -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 <refdir> <newdir> >>> >>> -The syntax of the ``validate-abi.sh`` utility is:: >>> +Where <refdir> specifies the directory housing the reference build of dpdk, and >>> +<newdir> specifies the dpdk build directory to check the abi of >>> >>> - ./devtools/validate-abi.sh <REV1> <REV2> >>> +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 <path to dpdk src tree> >>> +#. $ mkdir ~/ref >>> +#. $ git clone --local --no-hardlinks --single-branch -b master . ~/ref >>> +#. $ cd ~/ref >>> +#. $ cp <path to dpdk src tree from above>/.config ./.config >>> +#. $ make defconfig >>> +#. $ make >>> +#. $ <path to dpdk src tree>/devtools/check-abi.sh ~/ref <path to dpdk src tree> >>> >>> -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? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 10:35 ` Ray Kinsella @ 2020-04-17 11:46 ` Thomas Monjalon 0 siblings, 0 replies; 24+ messages in thread From: Thomas Monjalon @ 2020-04-17 11:46 UTC (permalink / raw) To: Neil Horman, Ray Kinsella; +Cc: dev, david.marchand 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 <nhorman@tuxdriver.com> > >>> 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] <rev1> <rev2> > >>> - > >>> - 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 > >>> -} > >>> - > >>> -# 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 <dir>), 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 > >>> -<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_. > >>> +``check-abi.sh``, for validating the DPDK ABI based on the libabigail `abidiff > >>> +utility: <https://sourceware.org/libabigail/manual/abidiff.html>`_ > >>> > >>> -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 <refdir> <newdir> > >>> > >>> -The syntax of the ``validate-abi.sh`` utility is:: > >>> +Where <refdir> specifies the directory housing the reference build of dpdk, and > >>> +<newdir> specifies the dpdk build directory to check the abi of > >>> > >>> - ./devtools/validate-abi.sh <REV1> <REV2> > >>> +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 <path to dpdk src tree> > >>> +#. $ mkdir ~/ref > >>> +#. $ git clone --local --no-hardlinks --single-branch -b master . ~/ref > >>> +#. $ cd ~/ref > >>> +#. $ cp <path to dpdk src tree from above>/.config ./.config > >>> +#. $ make defconfig > >>> +#. $ make > >>> +#. $ <path to dpdk src tree>/devtools/check-abi.sh ~/ref <path to dpdk src tree> > >>> > >>> -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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 10:20 ` Thomas Monjalon 2020-04-17 10:35 ` Ray Kinsella @ 2020-04-17 11:47 ` Ray Kinsella 2020-04-17 12:10 ` Thomas Monjalon 1 sibling, 1 reply; 24+ messages in thread From: Ray Kinsella @ 2020-04-17 11:47 UTC (permalink / raw) To: Thomas Monjalon, Neil Horman; +Cc: dev, david.marchand 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 <nhorman@tuxdriver.com> >>> 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] <rev1> <rev2> >>> - >>> - 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 >>> -} >>> - >>> -# 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 <dir>), 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 >>> -<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_. >>> +``check-abi.sh``, for validating the DPDK ABI based on the libabigail `abidiff >>> +utility: <https://sourceware.org/libabigail/manual/abidiff.html>`_ >>> >>> -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 <refdir> <newdir> >>> >>> -The syntax of the ``validate-abi.sh`` utility is:: >>> +Where <refdir> specifies the directory housing the reference build of dpdk, and >>> +<newdir> specifies the dpdk build directory to check the abi of >>> >>> - ./devtools/validate-abi.sh <REV1> <REV2> >>> +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 <path to dpdk src tree> >>> +#. $ mkdir ~/ref >>> +#. $ git clone --local --no-hardlinks --single-branch -b master . ~/ref >>> +#. $ cd ~/ref >>> +#. $ cp <path to dpdk src tree from above>/.config ./.config >>> +#. $ make defconfig >>> +#. $ make >>> +#. $ <path to dpdk src tree>/devtools/check-abi.sh ~/ref <path to dpdk src tree> >>> >>> -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. > Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. Any tips or tricks would be welcome. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 11:47 ` Ray Kinsella @ 2020-04-17 12:10 ` Thomas Monjalon 2020-04-17 15:42 ` Ray Kinsella 0 siblings, 1 reply; 24+ messages in thread From: Thomas Monjalon @ 2020-04-17 12:10 UTC (permalink / raw) To: Neil Horman, Ray Kinsella; +Cc: dev, david.marchand 17/04/2020 13:47, Ray Kinsella: > On 17/04/2020 11:20, Thomas Monjalon wrote: > > 17/04/2020 12:11, Ray Kinsella: > >> 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. > > Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > Any tips or tricks would be welcome. export DPDK_ABI_REF_VERSION=v20.02 or export DPDK_ABI_REF_VERSION=v19.11 Depends on which compatibility you want to test... ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 12:10 ` Thomas Monjalon @ 2020-04-17 15:42 ` Ray Kinsella 2020-04-17 16:10 ` Thomas Monjalon 2020-04-21 11:12 ` Neil Horman 0 siblings, 2 replies; 24+ messages in thread From: Ray Kinsella @ 2020-04-17 15:42 UTC (permalink / raw) To: Thomas Monjalon, Neil Horman; +Cc: dev, david.marchand On 17/04/2020 13:10, Thomas Monjalon wrote: > 17/04/2020 13:47, Ray Kinsella: >> On 17/04/2020 11:20, Thomas Monjalon wrote: >>> 17/04/2020 12:11, Ray Kinsella: >>>> 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. >> >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. >> Any tips or tricks would be welcome. > > export DPDK_ABI_REF_VERSION=v20.02 > or > export DPDK_ABI_REF_VERSION=v19.11 > > Depends on which compatibility you want to test... > Few things ... 1. test-meson-build.sh keep barfing complaining about reference paths. ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app Under the hood, ninja install is failing complaining that it needs an absolute path. I fixed this in test_meson_build.sh and will send a patch in a minute. Though it's strange no-one else has seen it? 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` Thanks, Ray K Ray K ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 15:42 ` Ray Kinsella @ 2020-04-17 16:10 ` Thomas Monjalon 2020-04-20 8:43 ` Ray Kinsella 2020-04-21 11:12 ` Neil Horman 1 sibling, 1 reply; 24+ messages in thread From: Thomas Monjalon @ 2020-04-17 16:10 UTC (permalink / raw) To: Neil Horman, Ray Kinsella; +Cc: dev, david.marchand 17/04/2020 17:42, Ray Kinsella: > On 17/04/2020 13:10, Thomas Monjalon wrote: > > 17/04/2020 13:47, Ray Kinsella: > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > >>> 17/04/2020 12:11, Ray Kinsella: > >>>> 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. > >> > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > >> Any tips or tricks would be welcome. > > > > export DPDK_ABI_REF_VERSION=v20.02 > > or > > export DPDK_ABI_REF_VERSION=v19.11 > > > > Depends on which compatibility you want to test... > > > > Few things ... > > 1. test-meson-build.sh keep barfing complaining about reference paths. > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > Under the hood, ninja install is failing complaining that it needs an absolute path. > I fixed this in test_meson_build.sh and will send a patch in a minute. > Though it's strange no-one else has seen it? I set an absolute path in DPDK_ABI_REF_DIR. Not sure you can really fix it. What would be the root dir? > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. Yes > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` Why is it a common case? You want to compare with a tag. Why something else? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 16:10 ` Thomas Monjalon @ 2020-04-20 8:43 ` Ray Kinsella 0 siblings, 0 replies; 24+ messages in thread From: Ray Kinsella @ 2020-04-20 8:43 UTC (permalink / raw) To: Thomas Monjalon, Neil Horman; +Cc: dev, david.marchand On 17/04/2020 17:10, Thomas Monjalon wrote: > 17/04/2020 17:42, Ray Kinsella: >> On 17/04/2020 13:10, Thomas Monjalon wrote: >>> 17/04/2020 13:47, Ray Kinsella: >>>> On 17/04/2020 11:20, Thomas Monjalon wrote: >>>>> 17/04/2020 12:11, Ray Kinsella: >>>>>> 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. >>>> >>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. >>>> Any tips or tricks would be welcome. >>> >>> export DPDK_ABI_REF_VERSION=v20.02 >>> or >>> export DPDK_ABI_REF_VERSION=v19.11 >>> >>> Depends on which compatibility you want to test... >>> >> >> Few things ... >> >> 1. test-meson-build.sh keep barfing complaining about reference paths. >> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app >> >> Under the hood, ninja install is failing complaining that it needs an absolute path. >> I fixed this in test_meson_build.sh and will send a patch in a minute. >> Though it's strange no-one else has seen it? > > I set an absolute path in DPDK_ABI_REF_DIR. > Not sure you can really fix it. What would be the root dir? Figure it out at runtime, check with realpath, if DPDK_ABI_REF_DIR is not set? Or at the very least test_meson_build.sh should barf, if DPDK_ABI_REF_DIR is not set when $DPDK_ABI_REF_VERSION is set. root@silpixa00395806:/build/dpdk# git diff devtools/test-meson-builds.sh diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh index c1ff2bb50..f24cc104d 100755 --- a/devtools/test-meson-builds.sh +++ b/devtools/test-meson-builds.sh @@ -103,13 +103,15 @@ compile () # <builddir> install_target () # <builddir> <installdir> { + destdir_fp=$(realpath $2) + rm -rf $2 if [ -n "$TEST_MESON_BUILD_VERY_VERBOSE$TEST_MESON_BUILD_VERBOSE" ]; then echo "DESTDIR=$2 $ninja_cmd -C $1 install" - DESTDIR=$2 $ninja_cmd -C $1 install + DESTDIR=$destdir_fp $ninja_cmd -C $1 install else echo "DESTDIR=$2 $ninja_cmd -C $1 install >/dev/null" - DESTDIR=$2 $ninja_cmd -C $1 install >/dev/null + DESTDIR=$destdir_fp $ninja_cmd -C $1 install >/dev/null fi } @@ -147,7 +149,7 @@ build () # <directory> <target compiler> <meson options> $srcdir/devtools/gen-abi.sh \ $(readlink -f $builds_dir/$targetdir/install) $srcdir/devtools/check-abi.sh $abirefdir/$targetdir \ - $(readlink -f $builds_dir/$targetdir/install) + $(readlink -f $builds_dir/$targetdir/install) || : fi } > >> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > Yes > >> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. >> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. >> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > Why is it a common case? You want to compare with a tag. Why something else? > I disagree, perhaps it's just me as a contributor. My first action is to check that _my_ submission has done no harm. Assuming that the origin's HEAD is more than likely good, I check my changes against it. I assume that anything else that has changed between the origin's HEAD and v19.11, someone somewhere else has approved. So why would I compare against v19.11, when I only want to check that the changes I have made have caused no harm. ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-17 15:42 ` Ray Kinsella 2020-04-17 16:10 ` Thomas Monjalon @ 2020-04-21 11:12 ` Neil Horman 2020-04-21 11:46 ` Thomas Monjalon 1 sibling, 1 reply; 24+ messages in thread From: Neil Horman @ 2020-04-21 11:12 UTC (permalink / raw) To: Ray Kinsella; +Cc: Thomas Monjalon, dev, david.marchand On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > > On 17/04/2020 13:10, Thomas Monjalon wrote: > > 17/04/2020 13:47, Ray Kinsella: > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > >>> 17/04/2020 12:11, Ray Kinsella: > >>>> 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. > >> > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > >> Any tips or tricks would be welcome. > > > > export DPDK_ABI_REF_VERSION=v20.02 > > or > > export DPDK_ABI_REF_VERSION=v19.11 > > > > Depends on which compatibility you want to test... > > > > Few things ... > > 1. test-meson-build.sh keep barfing complaining about reference paths. > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > Under the hood, ninja install is failing complaining that it needs an absolute path. > I fixed this in test_meson_build.sh and will send a patch in a minute. > Though it's strange no-one else has seen it? > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > I think this code in test-meson-build.sh should probably be fixed: if [ ! -d $abirefdir/src ]; then git clone --local --no-hardlinks \ --single-branch \ -b $DPDK_ABI_REF_VERSION \ $srcdir $abirefdir/src fi Like you noted, using -b allows us to checkout a tag/branch in the cloned repository but requires that it exist locally. We should probably prefix the checkout with a git fetch --tags Neil > Thanks, > > Ray K > > > > Ray K > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-21 11:12 ` Neil Horman @ 2020-04-21 11:46 ` Thomas Monjalon 2020-04-21 18:56 ` Neil Horman 0 siblings, 1 reply; 24+ messages in thread From: Thomas Monjalon @ 2020-04-21 11:46 UTC (permalink / raw) To: Ray Kinsella, Neil Horman; +Cc: dev, david.marchand 21/04/2020 13:12, Neil Horman: > On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > On 17/04/2020 13:10, Thomas Monjalon wrote: > > > 17/04/2020 13:47, Ray Kinsella: > > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > > >>> 17/04/2020 12:11, Ray Kinsella: > > >>>> 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. > > >> > > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > >> Any tips or tricks would be welcome. > > > > > > export DPDK_ABI_REF_VERSION=v20.02 > > > or > > > export DPDK_ABI_REF_VERSION=v19.11 > > > > > > Depends on which compatibility you want to test... > > > > > > > Few things ... > > > > 1. test-meson-build.sh keep barfing complaining about reference paths. > > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > > > Under the hood, ninja install is failing complaining that it needs an absolute path. > > I fixed this in test_meson_build.sh and will send a patch in a minute. > > Though it's strange no-one else has seen it? > > > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > > I think this code in test-meson-build.sh should probably be fixed: > > if [ ! -d $abirefdir/src ]; then > git clone --local --no-hardlinks \ > --single-branch \ > -b $DPDK_ABI_REF_VERSION \ > $srcdir $abirefdir/src > fi > > Like you noted, using -b allows us to checkout a tag/branch in the cloned > repository but requires that it exist locally. We should probably prefix the > checkout with a git fetch --tags I don't understand your concern. A reference is an older version, so it should be in the git tree. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-21 11:46 ` Thomas Monjalon @ 2020-04-21 18:56 ` Neil Horman 2020-04-21 21:42 ` Thomas Monjalon 0 siblings, 1 reply; 24+ messages in thread From: Neil Horman @ 2020-04-21 18:56 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Ray Kinsella, dev, david.marchand On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > 21/04/2020 13:12, Neil Horman: > > On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > > On 17/04/2020 13:10, Thomas Monjalon wrote: > > > > 17/04/2020 13:47, Ray Kinsella: > > > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > > > >>> 17/04/2020 12:11, Ray Kinsella: > > > >>>> 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. > > > >> > > > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > > >> Any tips or tricks would be welcome. > > > > > > > > export DPDK_ABI_REF_VERSION=v20.02 > > > > or > > > > export DPDK_ABI_REF_VERSION=v19.11 > > > > > > > > Depends on which compatibility you want to test... > > > > > > > > > > Few things ... > > > > > > 1. test-meson-build.sh keep barfing complaining about reference paths. > > > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > > > > > Under the hood, ninja install is failing complaining that it needs an absolute path. > > > I fixed this in test_meson_build.sh and will send a patch in a minute. > > > Though it's strange no-one else has seen it? > > > > > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > > > > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > > > > I think this code in test-meson-build.sh should probably be fixed: > > > > if [ ! -d $abirefdir/src ]; then > > git clone --local --no-hardlinks \ > > --single-branch \ > > -b $DPDK_ABI_REF_VERSION \ > > $srcdir $abirefdir/src > > fi > > > > Like you noted, using -b allows us to checkout a tag/branch in the cloned > > repository but requires that it exist locally. We should probably prefix the > > checkout with a git fetch --tags > > I don't understand your concern. > A reference is an older version, so it should be in the git tree. > yes, but not unless you've done a recent pull or fetch. If you set DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you updated the tree, it won't be there (which it sounds like what is being encountered here). You can fix that by doing a git pull or git fetch prior to running this script (or internal to the script) Neil > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-21 18:56 ` Neil Horman @ 2020-04-21 21:42 ` Thomas Monjalon 2020-04-22 11:43 ` Ray Kinsella 2020-04-22 12:01 ` Neil Horman 0 siblings, 2 replies; 24+ messages in thread From: Thomas Monjalon @ 2020-04-21 21:42 UTC (permalink / raw) To: Neil Horman; +Cc: Ray Kinsella, dev, david.marchand 21/04/2020 20:56, Neil Horman: > On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > > 21/04/2020 13:12, Neil Horman: > > > On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > > > On 17/04/2020 13:10, Thomas Monjalon wrote: > > > > > 17/04/2020 13:47, Ray Kinsella: > > > > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > > > > >>> 17/04/2020 12:11, Ray Kinsella: > > > > >>>> 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. > > > > >> > > > > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > > > >> Any tips or tricks would be welcome. > > > > > > > > > > export DPDK_ABI_REF_VERSION=v20.02 > > > > > or > > > > > export DPDK_ABI_REF_VERSION=v19.11 > > > > > > > > > > Depends on which compatibility you want to test... > > > > > > > > > > > > > Few things ... > > > > > > > > 1. test-meson-build.sh keep barfing complaining about reference paths. > > > > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > > > > > > > Under the hood, ninja install is failing complaining that it needs an absolute path. > > > > I fixed this in test_meson_build.sh and will send a patch in a minute. > > > > Though it's strange no-one else has seen it? > > > > > > > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > > > > > > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > > > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > > > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > > > > > > I think this code in test-meson-build.sh should probably be fixed: > > > > > > if [ ! -d $abirefdir/src ]; then > > > git clone --local --no-hardlinks \ > > > --single-branch \ > > > -b $DPDK_ABI_REF_VERSION \ > > > $srcdir $abirefdir/src > > > fi > > > > > > Like you noted, using -b allows us to checkout a tag/branch in the cloned > > > repository but requires that it exist locally. We should probably prefix the > > > checkout with a git fetch --tags > > > > I don't understand your concern. > > A reference is an older version, so it should be in the git tree. > > > yes, but not unless you've done a recent pull or fetch. If you set > DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > updated the tree, it won't be there (which it sounds like what is being > encountered here). You can fix that by doing a git pull or git fetch prior to > running this script (or internal to the script) Sorry I still don't understand the case. We want to compare the current version C with a reference R which is older. If the reference R is not in the tree, it means the version C is not in the tree. But C is the current version, so it is in the tree by definition. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-21 21:42 ` Thomas Monjalon @ 2020-04-22 11:43 ` Ray Kinsella 2020-04-22 12:07 ` Neil Horman 2020-04-22 12:01 ` Neil Horman 1 sibling, 1 reply; 24+ messages in thread From: Ray Kinsella @ 2020-04-22 11:43 UTC (permalink / raw) To: Thomas Monjalon, Neil Horman; +Cc: dev, david.marchand On 21/04/2020 22:42, Thomas Monjalon wrote: > 21/04/2020 20:56, Neil Horman: >> On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: >>> 21/04/2020 13:12, Neil Horman: >>>> On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: >>>>> On 17/04/2020 13:10, Thomas Monjalon wrote: >>>>>> 17/04/2020 13:47, Ray Kinsella: >>>>>>> On 17/04/2020 11:20, Thomas Monjalon wrote: >>>>>>>> 17/04/2020 12:11, Ray Kinsella: >>>>>>>>> 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. >>>>>>> >>>>>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. >>>>>>> Any tips or tricks would be welcome. >>>>>> >>>>>> export DPDK_ABI_REF_VERSION=v20.02 >>>>>> or >>>>>> export DPDK_ABI_REF_VERSION=v19.11 >>>>>> >>>>>> Depends on which compatibility you want to test... >>>>>> >>>>> >>>>> Few things ... >>>>> >>>>> 1. test-meson-build.sh keep barfing complaining about reference paths. >>>>> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app >>>>> >>>>> Under the hood, ninja install is failing complaining that it needs an absolute path. >>>>> I fixed this in test_meson_build.sh and will send a patch in a minute. >>>>> Though it's strange no-one else has seen it? >>>>> >>>>> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. >>>>> >>>>> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. >>>>> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. >>>>> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` >>>>> >>>> I think this code in test-meson-build.sh should probably be fixed: >>>> >>>> if [ ! -d $abirefdir/src ]; then >>>> git clone --local --no-hardlinks \ >>>> --single-branch \ >>>> -b $DPDK_ABI_REF_VERSION \ >>>> $srcdir $abirefdir/src >>>> fi >>>> >>>> Like you noted, using -b allows us to checkout a tag/branch in the cloned >>>> repository but requires that it exist locally. We should probably prefix the >>>> checkout with a git fetch --tags >>> >>> I don't understand your concern. >>> A reference is an older version, so it should be in the git tree. >>> >> yes, but not unless you've done a recent pull or fetch. If you set >> DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you >> updated the tree, it won't be there (which it sounds like what is being >> encountered here). You can fix that by doing a git pull or git fetch prior to >> running this script (or internal to the script) > > Sorry I still don't understand the case. > We want to compare the current version C with a reference R which is older. > If the reference R is not in the tree, it means the version C is not in the tree. > But C is the current version, so it is in the tree by definition. > So I can just relate my experience .... root@silpixa00395806:/build/dpdk# DPDK_ABI_REF_VERSION=HEAD~1 ./devtools/test-meson-builds.sh ninja -C ./build-gcc-static ninja: Entering directory `./build-gcc-static' [1766/2204] Compiling C object 'examples/c590b3c@@dpdk-vm_power_manager@exe/vm_power_manager_channel_monitor.c.o'. ../examples/vm_power_manager/channel_monitor.c:22:9: note: #pragma message: Jansson dev libs unavailable, not including JSON parsing #pragma message "Jansson dev libs unavailable, not including JSON parsing" ^~~~~~~ [2204/2204] Linking target drivers/librte_pmd_softnic.so.20.0.2. Cloning into 'reference/HEAD~1/src'... warning: Could not find remote branch HEAD~1 to clone. fatal: Remote branch HEAD~1 not found in upstream origin fatal: The remote end hung up unexpectedly ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-22 11:43 ` Ray Kinsella @ 2020-04-22 12:07 ` Neil Horman 2020-04-22 12:18 ` Thomas Monjalon 0 siblings, 1 reply; 24+ messages in thread From: Neil Horman @ 2020-04-22 12:07 UTC (permalink / raw) To: Ray Kinsella; +Cc: Thomas Monjalon, dev, david.marchand On Wed, Apr 22, 2020 at 12:43:44PM +0100, Ray Kinsella wrote: > > > On 21/04/2020 22:42, Thomas Monjalon wrote: > > 21/04/2020 20:56, Neil Horman: > >> On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > >>> 21/04/2020 13:12, Neil Horman: > >>>> On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > >>>>> On 17/04/2020 13:10, Thomas Monjalon wrote: > >>>>>> 17/04/2020 13:47, Ray Kinsella: > >>>>>>> On 17/04/2020 11:20, Thomas Monjalon wrote: > >>>>>>>> 17/04/2020 12:11, Ray Kinsella: > >>>>>>>>> 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. > >>>>>>> > >>>>>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > >>>>>>> Any tips or tricks would be welcome. > >>>>>> > >>>>>> export DPDK_ABI_REF_VERSION=v20.02 > >>>>>> or > >>>>>> export DPDK_ABI_REF_VERSION=v19.11 > >>>>>> > >>>>>> Depends on which compatibility you want to test... > >>>>>> > >>>>> > >>>>> Few things ... > >>>>> > >>>>> 1. test-meson-build.sh keep barfing complaining about reference paths. > >>>>> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > >>>>> > >>>>> Under the hood, ninja install is failing complaining that it needs an absolute path. > >>>>> I fixed this in test_meson_build.sh and will send a patch in a minute. > >>>>> Though it's strange no-one else has seen it? > >>>>> > >>>>> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > >>>>> > >>>>> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > >>>>> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > >>>>> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > >>>>> > >>>> I think this code in test-meson-build.sh should probably be fixed: > >>>> > >>>> if [ ! -d $abirefdir/src ]; then > >>>> git clone --local --no-hardlinks \ > >>>> --single-branch \ > >>>> -b $DPDK_ABI_REF_VERSION \ > >>>> $srcdir $abirefdir/src > >>>> fi > >>>> > >>>> Like you noted, using -b allows us to checkout a tag/branch in the cloned > >>>> repository but requires that it exist locally. We should probably prefix the > >>>> checkout with a git fetch --tags > >>> > >>> I don't understand your concern. > >>> A reference is an older version, so it should be in the git tree. > >>> > >> yes, but not unless you've done a recent pull or fetch. If you set > >> DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > >> updated the tree, it won't be there (which it sounds like what is being > >> encountered here). You can fix that by doing a git pull or git fetch prior to > >> running this script (or internal to the script) > > > > Sorry I still don't understand the case. > > We want to compare the current version C with a reference R which is older. > > If the reference R is not in the tree, it means the version C is not in the tree. > > But C is the current version, so it is in the tree by definition. > > > > So I can just relate my experience .... > > root@silpixa00395806:/build/dpdk# DPDK_ABI_REF_VERSION=HEAD~1 ./devtools/test-meson-builds.sh > ninja -C ./build-gcc-static > ninja: Entering directory `./build-gcc-static' > [1766/2204] Compiling C object 'examples/c590b3c@@dpdk-vm_power_manager@exe/vm_power_manager_channel_monitor.c.o'. > ../examples/vm_power_manager/channel_monitor.c:22:9: note: #pragma message: Jansson dev libs unavailable, not including JSON parsing > #pragma message "Jansson dev libs unavailable, not including JSON parsing" > ^~~~~~~ > [2204/2204] Linking target drivers/librte_pmd_softnic.so.20.0.2. > Cloning into 'reference/HEAD~1/src'... > warning: Could not find remote branch HEAD~1 to clone. > fatal: Remote branch HEAD~1 not found in upstream origin > fatal: The remote end hung up unexpectedly > Ah, So its not the problem i was describing, I think the problem you are seeing is that the -b option only operates on branches and tags, not arbitrary git revisions. To fix that, what we probably need to do is alter test-build.sh and test-meson-build.sh such that the git clone operation is preceded by something like this: git tag ABI_CHECK_TAG $DPDK_ABI_REF_VERSION git clone .... git tag -d ABI_CHECK_TAG Doing so will guarantee that the source tree has a tag reference that the git clone operation can use to do a checkout with a -b option on. Neil > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-22 12:07 ` Neil Horman @ 2020-04-22 12:18 ` Thomas Monjalon 2020-04-22 13:19 ` Ray Kinsella 2020-04-23 11:03 ` Neil Horman 0 siblings, 2 replies; 24+ messages in thread From: Thomas Monjalon @ 2020-04-22 12:18 UTC (permalink / raw) To: Ray Kinsella, Neil Horman; +Cc: dev, david.marchand 22/04/2020 14:07, Neil Horman: > On Wed, Apr 22, 2020 at 12:43:44PM +0100, Ray Kinsella wrote: > > On 21/04/2020 22:42, Thomas Monjalon wrote: > > > 21/04/2020 20:56, Neil Horman: > > >> On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > > >>> 21/04/2020 13:12, Neil Horman: > > >>>> On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > >>>>> On 17/04/2020 13:10, Thomas Monjalon wrote: > > >>>>>> 17/04/2020 13:47, Ray Kinsella: > > >>>>>>> On 17/04/2020 11:20, Thomas Monjalon wrote: > > >>>>>>>> 17/04/2020 12:11, Ray Kinsella: > > >>>>>>>>> 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. > > >>>>>>> > > >>>>>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > >>>>>>> Any tips or tricks would be welcome. > > >>>>>> > > >>>>>> export DPDK_ABI_REF_VERSION=v20.02 > > >>>>>> or > > >>>>>> export DPDK_ABI_REF_VERSION=v19.11 > > >>>>>> > > >>>>>> Depends on which compatibility you want to test... > > >>>>>> > > >>>>> > > >>>>> Few things ... > > >>>>> > > >>>>> 1. test-meson-build.sh keep barfing complaining about reference paths. > > >>>>> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > >>>>> > > >>>>> Under the hood, ninja install is failing complaining that it needs an absolute path. > > >>>>> I fixed this in test_meson_build.sh and will send a patch in a minute. > > >>>>> Though it's strange no-one else has seen it? > > >>>>> > > >>>>> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > >>>>> > > >>>>> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > >>>>> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > >>>>> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > >>>>> > > >>>> I think this code in test-meson-build.sh should probably be fixed: > > >>>> > > >>>> if [ ! -d $abirefdir/src ]; then > > >>>> git clone --local --no-hardlinks \ > > >>>> --single-branch \ > > >>>> -b $DPDK_ABI_REF_VERSION \ > > >>>> $srcdir $abirefdir/src > > >>>> fi > > >>>> > > >>>> Like you noted, using -b allows us to checkout a tag/branch in the cloned > > >>>> repository but requires that it exist locally. We should probably prefix the > > >>>> checkout with a git fetch --tags > > >>> > > >>> I don't understand your concern. > > >>> A reference is an older version, so it should be in the git tree. > > >>> > > >> yes, but not unless you've done a recent pull or fetch. If you set > > >> DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > > >> updated the tree, it won't be there (which it sounds like what is being > > >> encountered here). You can fix that by doing a git pull or git fetch prior to > > >> running this script (or internal to the script) > > > > > > Sorry I still don't understand the case. > > > We want to compare the current version C with a reference R which is older. > > > If the reference R is not in the tree, it means the version C is not in the tree. > > > But C is the current version, so it is in the tree by definition. > > > > > > > So I can just relate my experience .... > > > > root@silpixa00395806:/build/dpdk# DPDK_ABI_REF_VERSION=HEAD~1 ./devtools/test-meson-builds.sh > > ninja -C ./build-gcc-static > > ninja: Entering directory `./build-gcc-static' > > [1766/2204] Compiling C object 'examples/c590b3c@@dpdk-vm_power_manager@exe/vm_power_manager_channel_monitor.c.o'. > > ../examples/vm_power_manager/channel_monitor.c:22:9: note: #pragma message: Jansson dev libs unavailable, not including JSON parsing > > #pragma message "Jansson dev libs unavailable, not including JSON parsing" > > ^~~~~~~ > > [2204/2204] Linking target drivers/librte_pmd_softnic.so.20.0.2. > > Cloning into 'reference/HEAD~1/src'... > > warning: Could not find remote branch HEAD~1 to clone. > > fatal: Remote branch HEAD~1 not found in upstream origin > > fatal: The remote end hung up unexpectedly > > > Ah, So its not the problem i was describing, I think the problem you are seeing > is that the -b option only operates on branches and tags, not arbitrary git > revisions. > > To fix that, what we probably need to do is alter test-build.sh and > test-meson-build.sh such that the git clone operation is preceded by something > like this: > git tag ABI_CHECK_TAG $DPDK_ABI_REF_VERSION > > git clone .... > > git tag -d ABI_CHECK_TAG > > Doing so will guarantee that the source tree has a tag reference that the git > clone operation can use to do a checkout with a -b option on. I don't see the benefit of such test. Can we just document that the reference must be an existing tag? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-22 12:18 ` Thomas Monjalon @ 2020-04-22 13:19 ` Ray Kinsella 2020-04-22 13:30 ` Thomas Monjalon 2020-04-23 11:03 ` Neil Horman 1 sibling, 1 reply; 24+ messages in thread From: Ray Kinsella @ 2020-04-22 13:19 UTC (permalink / raw) To: Thomas Monjalon, Neil Horman; +Cc: dev, david.marchand On 22/04/2020 13:18, Thomas Monjalon wrote: > 22/04/2020 14:07, Neil Horman: >> On Wed, Apr 22, 2020 at 12:43:44PM +0100, Ray Kinsella wrote: >>> On 21/04/2020 22:42, Thomas Monjalon wrote: >>>> 21/04/2020 20:56, Neil Horman: >>>>> On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: >>>>>> 21/04/2020 13:12, Neil Horman: >>>>>>> On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: >>>>>>>> On 17/04/2020 13:10, Thomas Monjalon wrote: >>>>>>>>> 17/04/2020 13:47, Ray Kinsella: >>>>>>>>>> On 17/04/2020 11:20, Thomas Monjalon wrote: >>>>>>>>>>> 17/04/2020 12:11, Ray Kinsella: >>>>>>>>>>>> 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. >>>>>>>>>> >>>>>>>>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. >>>>>>>>>> Any tips or tricks would be welcome. >>>>>>>>> >>>>>>>>> export DPDK_ABI_REF_VERSION=v20.02 >>>>>>>>> or >>>>>>>>> export DPDK_ABI_REF_VERSION=v19.11 >>>>>>>>> >>>>>>>>> Depends on which compatibility you want to test... >>>>>>>>> >>>>>>>> >>>>>>>> Few things ... >>>>>>>> >>>>>>>> 1. test-meson-build.sh keep barfing complaining about reference paths. >>>>>>>> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app >>>>>>>> >>>>>>>> Under the hood, ninja install is failing complaining that it needs an absolute path. >>>>>>>> I fixed this in test_meson_build.sh and will send a patch in a minute. >>>>>>>> Though it's strange no-one else has seen it? >>>>>>>> >>>>>>>> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. >>>>>>>> >>>>>>>> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. >>>>>>>> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. >>>>>>>> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` >>>>>>>> >>>>>>> I think this code in test-meson-build.sh should probably be fixed: >>>>>>> >>>>>>> if [ ! -d $abirefdir/src ]; then >>>>>>> git clone --local --no-hardlinks \ >>>>>>> --single-branch \ >>>>>>> -b $DPDK_ABI_REF_VERSION \ >>>>>>> $srcdir $abirefdir/src >>>>>>> fi >>>>>>> >>>>>>> Like you noted, using -b allows us to checkout a tag/branch in the cloned >>>>>>> repository but requires that it exist locally. We should probably prefix the >>>>>>> checkout with a git fetch --tags >>>>>> >>>>>> I don't understand your concern. >>>>>> A reference is an older version, so it should be in the git tree. >>>>>> >>>>> yes, but not unless you've done a recent pull or fetch. If you set >>>>> DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you >>>>> updated the tree, it won't be there (which it sounds like what is being >>>>> encountered here). You can fix that by doing a git pull or git fetch prior to >>>>> running this script (or internal to the script) >>>> >>>> Sorry I still don't understand the case. >>>> We want to compare the current version C with a reference R which is older. >>>> If the reference R is not in the tree, it means the version C is not in the tree. >>>> But C is the current version, so it is in the tree by definition. >>>> >>> >>> So I can just relate my experience .... >>> >>> root@silpixa00395806:/build/dpdk# DPDK_ABI_REF_VERSION=HEAD~1 ./devtools/test-meson-builds.sh >>> ninja -C ./build-gcc-static >>> ninja: Entering directory `./build-gcc-static' >>> [1766/2204] Compiling C object 'examples/c590b3c@@dpdk-vm_power_manager@exe/vm_power_manager_channel_monitor.c.o'. >>> ../examples/vm_power_manager/channel_monitor.c:22:9: note: #pragma message: Jansson dev libs unavailable, not including JSON parsing >>> #pragma message "Jansson dev libs unavailable, not including JSON parsing" >>> ^~~~~~~ >>> [2204/2204] Linking target drivers/librte_pmd_softnic.so.20.0.2. >>> Cloning into 'reference/HEAD~1/src'... >>> warning: Could not find remote branch HEAD~1 to clone. >>> fatal: Remote branch HEAD~1 not found in upstream origin >>> fatal: The remote end hung up unexpectedly >>> >> Ah, So its not the problem i was describing, I think the problem you are seeing >> is that the -b option only operates on branches and tags, not arbitrary git >> revisions. >> >> To fix that, what we probably need to do is alter test-build.sh and >> test-meson-build.sh such that the git clone operation is preceded by something >> like this: >> git tag ABI_CHECK_TAG $DPDK_ABI_REF_VERSION >> >> git clone .... >> >> git tag -d ABI_CHECK_TAG >> >> Doing so will guarantee that the source tree has a tag reference that the git >> clone operation can use to do a checkout with a -b option on. > > I don't see the benefit of such test. > Can we just document that the reference must be an existing tag? > You want people to use this thing right? ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-22 13:19 ` Ray Kinsella @ 2020-04-22 13:30 ` Thomas Monjalon 0 siblings, 0 replies; 24+ messages in thread From: Thomas Monjalon @ 2020-04-22 13:30 UTC (permalink / raw) To: Neil Horman, Ray Kinsella; +Cc: dev, david.marchand 22/04/2020 15:19, Ray Kinsella: > On 22/04/2020 13:18, Thomas Monjalon wrote: > > 22/04/2020 14:07, Neil Horman: > >> On Wed, Apr 22, 2020 at 12:43:44PM +0100, Ray Kinsella wrote: > >>> On 21/04/2020 22:42, Thomas Monjalon wrote: > >>>> 21/04/2020 20:56, Neil Horman: > >>>>> On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > >>>>>> 21/04/2020 13:12, Neil Horman: > >>>>>>> On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > >>>>>>>> On 17/04/2020 13:10, Thomas Monjalon wrote: > >>>>>>>>> 17/04/2020 13:47, Ray Kinsella: > >>>>>>>>>> On 17/04/2020 11:20, Thomas Monjalon wrote: > >>>>>>>>>>> 17/04/2020 12:11, Ray Kinsella: > >>>>>>>>>>>> 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. > >>>>>>>>>> > >>>>>>>>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > >>>>>>>>>> Any tips or tricks would be welcome. > >>>>>>>>> > >>>>>>>>> export DPDK_ABI_REF_VERSION=v20.02 > >>>>>>>>> or > >>>>>>>>> export DPDK_ABI_REF_VERSION=v19.11 > >>>>>>>>> > >>>>>>>>> Depends on which compatibility you want to test... > >>>>>>>>> > >>>>>>>> > >>>>>>>> Few things ... > >>>>>>>> > >>>>>>>> 1. test-meson-build.sh keep barfing complaining about reference paths. > >>>>>>>> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > >>>>>>>> > >>>>>>>> Under the hood, ninja install is failing complaining that it needs an absolute path. > >>>>>>>> I fixed this in test_meson_build.sh and will send a patch in a minute. > >>>>>>>> Though it's strange no-one else has seen it? > >>>>>>>> > >>>>>>>> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > >>>>>>>> > >>>>>>>> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > >>>>>>>> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > >>>>>>>> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > >>>>>>>> > >>>>>>> I think this code in test-meson-build.sh should probably be fixed: > >>>>>>> > >>>>>>> if [ ! -d $abirefdir/src ]; then > >>>>>>> git clone --local --no-hardlinks \ > >>>>>>> --single-branch \ > >>>>>>> -b $DPDK_ABI_REF_VERSION \ > >>>>>>> $srcdir $abirefdir/src > >>>>>>> fi > >>>>>>> > >>>>>>> Like you noted, using -b allows us to checkout a tag/branch in the cloned > >>>>>>> repository but requires that it exist locally. We should probably prefix the > >>>>>>> checkout with a git fetch --tags > >>>>>> > >>>>>> I don't understand your concern. > >>>>>> A reference is an older version, so it should be in the git tree. > >>>>>> > >>>>> yes, but not unless you've done a recent pull or fetch. If you set > >>>>> DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > >>>>> updated the tree, it won't be there (which it sounds like what is being > >>>>> encountered here). You can fix that by doing a git pull or git fetch prior to > >>>>> running this script (or internal to the script) > >>>> > >>>> Sorry I still don't understand the case. > >>>> We want to compare the current version C with a reference R which is older. > >>>> If the reference R is not in the tree, it means the version C is not in the tree. > >>>> But C is the current version, so it is in the tree by definition. > >>>> > >>> > >>> So I can just relate my experience .... > >>> > >>> root@silpixa00395806:/build/dpdk# DPDK_ABI_REF_VERSION=HEAD~1 ./devtools/test-meson-builds.sh > >>> ninja -C ./build-gcc-static > >>> ninja: Entering directory `./build-gcc-static' > >>> [1766/2204] Compiling C object 'examples/c590b3c@@dpdk-vm_power_manager@exe/vm_power_manager_channel_monitor.c.o'. > >>> ../examples/vm_power_manager/channel_monitor.c:22:9: note: #pragma message: Jansson dev libs unavailable, not including JSON parsing > >>> #pragma message "Jansson dev libs unavailable, not including JSON parsing" > >>> ^~~~~~~ > >>> [2204/2204] Linking target drivers/librte_pmd_softnic.so.20.0.2. > >>> Cloning into 'reference/HEAD~1/src'... > >>> warning: Could not find remote branch HEAD~1 to clone. > >>> fatal: Remote branch HEAD~1 not found in upstream origin > >>> fatal: The remote end hung up unexpectedly > >>> > >> Ah, So its not the problem i was describing, I think the problem you are seeing > >> is that the -b option only operates on branches and tags, not arbitrary git > >> revisions. > >> > >> To fix that, what we probably need to do is alter test-build.sh and > >> test-meson-build.sh such that the git clone operation is preceded by something > >> like this: > >> git tag ABI_CHECK_TAG $DPDK_ABI_REF_VERSION > >> > >> git clone .... > >> > >> git tag -d ABI_CHECK_TAG > >> > >> Doing so will guarantee that the source tree has a tag reference that the git > >> clone operation can use to do a checkout with a -b option on. > > > > I don't see the benefit of such test. > > Can we just document that the reference must be an existing tag? > > > You want people to use this thing right? Yes, compare their own code with a well known tag. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-22 12:18 ` Thomas Monjalon 2020-04-22 13:19 ` Ray Kinsella @ 2020-04-23 11:03 ` Neil Horman 1 sibling, 0 replies; 24+ messages in thread From: Neil Horman @ 2020-04-23 11:03 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Ray Kinsella, dev, david.marchand On Wed, Apr 22, 2020 at 02:18:05PM +0200, Thomas Monjalon wrote: > 22/04/2020 14:07, Neil Horman: > > On Wed, Apr 22, 2020 at 12:43:44PM +0100, Ray Kinsella wrote: > > > On 21/04/2020 22:42, Thomas Monjalon wrote: > > > > 21/04/2020 20:56, Neil Horman: > > > >> On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > > > >>> 21/04/2020 13:12, Neil Horman: > > > >>>> On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > > >>>>> On 17/04/2020 13:10, Thomas Monjalon wrote: > > > >>>>>> 17/04/2020 13:47, Ray Kinsella: > > > >>>>>>> On 17/04/2020 11:20, Thomas Monjalon wrote: > > > >>>>>>>> 17/04/2020 12:11, Ray Kinsella: > > > >>>>>>>>> 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. > > > >>>>>>> > > > >>>>>>> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > > >>>>>>> Any tips or tricks would be welcome. > > > >>>>>> > > > >>>>>> export DPDK_ABI_REF_VERSION=v20.02 > > > >>>>>> or > > > >>>>>> export DPDK_ABI_REF_VERSION=v19.11 > > > >>>>>> > > > >>>>>> Depends on which compatibility you want to test... > > > >>>>>> > > > >>>>> > > > >>>>> Few things ... > > > >>>>> > > > >>>>> 1. test-meson-build.sh keep barfing complaining about reference paths. > > > >>>>> ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > > >>>>> > > > >>>>> Under the hood, ninja install is failing complaining that it needs an absolute path. > > > >>>>> I fixed this in test_meson_build.sh and will send a patch in a minute. > > > >>>>> Though it's strange no-one else has seen it? > > > >>>>> > > > >>>>> 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > > >>>>> > > > >>>>> 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > > >>>>> In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > > >>>>> I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > > >>>>> > > > >>>> I think this code in test-meson-build.sh should probably be fixed: > > > >>>> > > > >>>> if [ ! -d $abirefdir/src ]; then > > > >>>> git clone --local --no-hardlinks \ > > > >>>> --single-branch \ > > > >>>> -b $DPDK_ABI_REF_VERSION \ > > > >>>> $srcdir $abirefdir/src > > > >>>> fi > > > >>>> > > > >>>> Like you noted, using -b allows us to checkout a tag/branch in the cloned > > > >>>> repository but requires that it exist locally. We should probably prefix the > > > >>>> checkout with a git fetch --tags > > > >>> > > > >>> I don't understand your concern. > > > >>> A reference is an older version, so it should be in the git tree. > > > >>> > > > >> yes, but not unless you've done a recent pull or fetch. If you set > > > >> DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > > > >> updated the tree, it won't be there (which it sounds like what is being > > > >> encountered here). You can fix that by doing a git pull or git fetch prior to > > > >> running this script (or internal to the script) > > > > > > > > Sorry I still don't understand the case. > > > > We want to compare the current version C with a reference R which is older. > > > > If the reference R is not in the tree, it means the version C is not in the tree. > > > > But C is the current version, so it is in the tree by definition. > > > > > > > > > > So I can just relate my experience .... > > > > > > root@silpixa00395806:/build/dpdk# DPDK_ABI_REF_VERSION=HEAD~1 ./devtools/test-meson-builds.sh > > > ninja -C ./build-gcc-static > > > ninja: Entering directory `./build-gcc-static' > > > [1766/2204] Compiling C object 'examples/c590b3c@@dpdk-vm_power_manager@exe/vm_power_manager_channel_monitor.c.o'. > > > ../examples/vm_power_manager/channel_monitor.c:22:9: note: #pragma message: Jansson dev libs unavailable, not including JSON parsing > > > #pragma message "Jansson dev libs unavailable, not including JSON parsing" > > > ^~~~~~~ > > > [2204/2204] Linking target drivers/librte_pmd_softnic.so.20.0.2. > > > Cloning into 'reference/HEAD~1/src'... > > > warning: Could not find remote branch HEAD~1 to clone. > > > fatal: Remote branch HEAD~1 not found in upstream origin > > > fatal: The remote end hung up unexpectedly > > > > > Ah, So its not the problem i was describing, I think the problem you are seeing > > is that the -b option only operates on branches and tags, not arbitrary git > > revisions. > > > > To fix that, what we probably need to do is alter test-build.sh and > > test-meson-build.sh such that the git clone operation is preceded by something > > like this: > > git tag ABI_CHECK_TAG $DPDK_ABI_REF_VERSION > > > > git clone .... > > > > git tag -d ABI_CHECK_TAG > > > > Doing so will guarantee that the source tree has a tag reference that the git > > clone operation can use to do a checkout with a -b option on. > > I don't see the benefit of such test. > Can we just document that the reference must be an existing tag? > We could, but like Ray said, if you want people to use this, you'll want to make it easy for them. Many people will want to test the ABI against a well known tag, but some are going to want to test against an arbitrary version (i.e. against the merge base of their feature branch and the point they branched from), which means they will attempt to check against an arbitrary hash value or non-tag symbolic reference. Theres no reason we can't enable that, the only reason it doesn't work is because of an ideosyncracy of the git branch command, and the above fixes that. Neil > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-21 21:42 ` Thomas Monjalon 2020-04-22 11:43 ` Ray Kinsella @ 2020-04-22 12:01 ` Neil Horman 2020-04-22 12:16 ` Thomas Monjalon 1 sibling, 1 reply; 24+ messages in thread From: Neil Horman @ 2020-04-22 12:01 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Ray Kinsella, dev, david.marchand On Tue, Apr 21, 2020 at 11:42:42PM +0200, Thomas Monjalon wrote: > 21/04/2020 20:56, Neil Horman: > > On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > > > 21/04/2020 13:12, Neil Horman: > > > > On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > > > > On 17/04/2020 13:10, Thomas Monjalon wrote: > > > > > > 17/04/2020 13:47, Ray Kinsella: > > > > > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > > > > > >>> 17/04/2020 12:11, Ray Kinsella: > > > > > >>>> 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. > > > > > >> > > > > > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > > > > >> Any tips or tricks would be welcome. > > > > > > > > > > > > export DPDK_ABI_REF_VERSION=v20.02 > > > > > > or > > > > > > export DPDK_ABI_REF_VERSION=v19.11 > > > > > > > > > > > > Depends on which compatibility you want to test... > > > > > > > > > > > > > > > > Few things ... > > > > > > > > > > 1. test-meson-build.sh keep barfing complaining about reference paths. > > > > > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > > > > > > > > > Under the hood, ninja install is failing complaining that it needs an absolute path. > > > > > I fixed this in test_meson_build.sh and will send a patch in a minute. > > > > > Though it's strange no-one else has seen it? > > > > > > > > > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > > > > > > > > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > > > > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > > > > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > > > > > > > > I think this code in test-meson-build.sh should probably be fixed: > > > > > > > > if [ ! -d $abirefdir/src ]; then > > > > git clone --local --no-hardlinks \ > > > > --single-branch \ > > > > -b $DPDK_ABI_REF_VERSION \ > > > > $srcdir $abirefdir/src > > > > fi > > > > > > > > Like you noted, using -b allows us to checkout a tag/branch in the cloned > > > > repository but requires that it exist locally. We should probably prefix the > > > > checkout with a git fetch --tags > > > > > > I don't understand your concern. > > > A reference is an older version, so it should be in the git tree. > > > > > yes, but not unless you've done a recent pull or fetch. If you set > > DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > > updated the tree, it won't be there (which it sounds like what is being > > encountered here). You can fix that by doing a git pull or git fetch prior to > > running this script (or internal to the script) > > Sorry I still don't understand the case. > We want to compare the current version C with a reference R which is older. > If the reference R is not in the tree, it means the version C is not in the tree. > But C is the current version, so it is in the tree by definition. > +---+ | | +---+ | | DPDK 20.11 | |Feature Branch commit +-+-+ | | | +-+-+ +---+ +-v-+ | | | master HEAD | | master HEAD | +-+-+ +-+-+ | | | | | | | +-v-+ +-v-+ +---->+ | DPDK 19.11 | | DPDK 19.11 +---+ +---+ X X X X XXXXXXX XXXXXXXXX XXXXXXX XXXXXXXXX XXX XX XXX XX XXXX XXXX Local GIT copy DPDK.ORG If a user clones from dpdk.org when dpdk 19.11 is tagged in the tree, or any time before dpdk 20.11 is tagged, then creates a feature branch, they may want to compare their abi to the latest stable version. If they check dpdk.org and see that dpdk 20.11 is the latest tag in the tree, they can run check-abi.sh with the reference tag specified as 20.11, which exists in the dpdk.org tree, but may not have been pulled into their local copy yet. I'm not saying this is definately what happened, but it would explain the reported observations. Neil ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-22 12:01 ` Neil Horman @ 2020-04-22 12:16 ` Thomas Monjalon 2020-04-23 10:57 ` Neil Horman 0 siblings, 1 reply; 24+ messages in thread From: Thomas Monjalon @ 2020-04-22 12:16 UTC (permalink / raw) To: Neil Horman; +Cc: Ray Kinsella, dev, david.marchand 22/04/2020 14:01, Neil Horman: > On Tue, Apr 21, 2020 at 11:42:42PM +0200, Thomas Monjalon wrote: > > 21/04/2020 20:56, Neil Horman: > > > On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > > > > 21/04/2020 13:12, Neil Horman: > > > > > On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > > > > > On 17/04/2020 13:10, Thomas Monjalon wrote: > > > > > > > 17/04/2020 13:47, Ray Kinsella: > > > > > > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > > > > > > >>> 17/04/2020 12:11, Ray Kinsella: > > > > > > >>>> 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. > > > > > > >> > > > > > > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > > > > > >> Any tips or tricks would be welcome. > > > > > > > > > > > > > > export DPDK_ABI_REF_VERSION=v20.02 > > > > > > > or > > > > > > > export DPDK_ABI_REF_VERSION=v19.11 > > > > > > > > > > > > > > Depends on which compatibility you want to test... > > > > > > > > > > > > > > > > > > > Few things ... > > > > > > > > > > > > 1. test-meson-build.sh keep barfing complaining about reference paths. > > > > > > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > > > > > > > > > > > Under the hood, ninja install is failing complaining that it needs an absolute path. > > > > > > I fixed this in test_meson_build.sh and will send a patch in a minute. > > > > > > Though it's strange no-one else has seen it? > > > > > > > > > > > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > > > > > > > > > > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > > > > > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > > > > > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > > > > > > > > > > I think this code in test-meson-build.sh should probably be fixed: > > > > > > > > > > if [ ! -d $abirefdir/src ]; then > > > > > git clone --local --no-hardlinks \ > > > > > --single-branch \ > > > > > -b $DPDK_ABI_REF_VERSION \ > > > > > $srcdir $abirefdir/src > > > > > fi > > > > > > > > > > Like you noted, using -b allows us to checkout a tag/branch in the cloned > > > > > repository but requires that it exist locally. We should probably prefix the > > > > > checkout with a git fetch --tags > > > > > > > > I don't understand your concern. > > > > A reference is an older version, so it should be in the git tree. > > > > > > > yes, but not unless you've done a recent pull or fetch. If you set > > > DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > > > updated the tree, it won't be there (which it sounds like what is being > > > encountered here). You can fix that by doing a git pull or git fetch prior to > > > running this script (or internal to the script) > > > > Sorry I still don't understand the case. > > We want to compare the current version C with a reference R which is older. > > If the reference R is not in the tree, it means the version C is not in the tree. > > But C is the current version, so it is in the tree by definition. > > > > > +---+ > | | > +---+ | | DPDK 20.11 > | |Feature Branch commit +-+-+ > | | | > +-+-+ +---+ +-v-+ > | | | master HEAD | | master HEAD > | +-+-+ +-+-+ > | | | > | | | > | +-v-+ +-v-+ > +---->+ | DPDK 19.11 | | DPDK 19.11 > +---+ +---+ > X X X X > XXXXXXX XXXXXXXXX XXXXXXX XXXXXXXXX > XXX XX XXX XX > XXXX XXXX > > Local GIT copy DPDK.ORG > > > > If a user clones from dpdk.org when dpdk 19.11 is tagged in the tree, or any > time before dpdk 20.11 is tagged, then creates a feature branch, they may want > to compare their abi to the latest stable version. If they check dpdk.org and > see that dpdk 20.11 is the latest tag in the tree, they can run check-abi.sh > with the reference tag specified as 20.11, which exists in the dpdk.org tree, > but may not have been pulled into their local copy yet. > > I'm not saying this is definately what happened, but it would explain the > reported observations. OK now I understand. The user specified a reference which is not in his local tree. I vote for considering this case as an user mistake, and document it as a limitation of the tool integration in our build testing scripts. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree 2020-04-22 12:16 ` Thomas Monjalon @ 2020-04-23 10:57 ` Neil Horman 0 siblings, 0 replies; 24+ messages in thread From: Neil Horman @ 2020-04-23 10:57 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Ray Kinsella, dev, david.marchand On Wed, Apr 22, 2020 at 02:16:57PM +0200, Thomas Monjalon wrote: > 22/04/2020 14:01, Neil Horman: > > On Tue, Apr 21, 2020 at 11:42:42PM +0200, Thomas Monjalon wrote: > > > 21/04/2020 20:56, Neil Horman: > > > > On Tue, Apr 21, 2020 at 01:46:43PM +0200, Thomas Monjalon wrote: > > > > > 21/04/2020 13:12, Neil Horman: > > > > > > On Fri, Apr 17, 2020 at 04:42:38PM +0100, Ray Kinsella wrote: > > > > > > > On 17/04/2020 13:10, Thomas Monjalon wrote: > > > > > > > > 17/04/2020 13:47, Ray Kinsella: > > > > > > > >> On 17/04/2020 11:20, Thomas Monjalon wrote: > > > > > > > >>> 17/04/2020 12:11, Ray Kinsella: > > > > > > > >>>> 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. > > > > > > > >> > > > > > > > >> Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious. > > > > > > > >> Any tips or tricks would be welcome. > > > > > > > > > > > > > > > > export DPDK_ABI_REF_VERSION=v20.02 > > > > > > > > or > > > > > > > > export DPDK_ABI_REF_VERSION=v19.11 > > > > > > > > > > > > > > > > Depends on which compatibility you want to test... > > > > > > > > > > > > > > > > > > > > > > Few things ... > > > > > > > > > > > > > > 1. test-meson-build.sh keep barfing complaining about reference paths. > > > > > > > ValueError: dst_dir must be absolute, got reference/v19.11/build-gcc-static/usr/local/share/dpdk/examples/bbdev_app > > > > > > > > > > > > > > Under the hood, ninja install is failing complaining that it needs an absolute path. > > > > > > > I fixed this in test_meson_build.sh and will send a patch in a minute. > > > > > > > Though it's strange no-one else has seen it? > > > > > > > > > > > > > > 2. test-meson-build.sh compares the abi for the static builds, which doesn't make any sense. > > > > > > > > > > > > > > 3. test-meson-build.sh will only take a branch in DPDK_ABI_REF_VERSION that exists locally. > > > > > > > In order to get it to compare HEAD against HEAD~1, which you would imagine is a pretty common case. > > > > > > > I had a create a branch for HEAD~1, in validate-abi this a pretty simple `validate-abi HEAD~1 HEAD` > > > > > > > > > > > > > I think this code in test-meson-build.sh should probably be fixed: > > > > > > > > > > > > if [ ! -d $abirefdir/src ]; then > > > > > > git clone --local --no-hardlinks \ > > > > > > --single-branch \ > > > > > > -b $DPDK_ABI_REF_VERSION \ > > > > > > $srcdir $abirefdir/src > > > > > > fi > > > > > > > > > > > > Like you noted, using -b allows us to checkout a tag/branch in the cloned > > > > > > repository but requires that it exist locally. We should probably prefix the > > > > > > checkout with a git fetch --tags > > > > > > > > > > I don't understand your concern. > > > > > A reference is an older version, so it should be in the git tree. > > > > > > > > > yes, but not unless you've done a recent pull or fetch. If you set > > > > DPDK_ABI_REF_VERSION to a tag/branch that didn't exist as of the last time you > > > > updated the tree, it won't be there (which it sounds like what is being > > > > encountered here). You can fix that by doing a git pull or git fetch prior to > > > > running this script (or internal to the script) > > > > > > Sorry I still don't understand the case. > > > We want to compare the current version C with a reference R which is older. > > > If the reference R is not in the tree, it means the version C is not in the tree. > > > But C is the current version, so it is in the tree by definition. > > > > > > > > > +---+ > > | | > > +---+ | | DPDK 20.11 > > | |Feature Branch commit +-+-+ > > | | | > > +-+-+ +---+ +-v-+ > > | | | master HEAD | | master HEAD > > | +-+-+ +-+-+ > > | | | > > | | | > > | +-v-+ +-v-+ > > +---->+ | DPDK 19.11 | | DPDK 19.11 > > +---+ +---+ > > X X X X > > XXXXXXX XXXXXXXXX XXXXXXX XXXXXXXXX > > XXX XX XXX XX > > XXXX XXXX > > > > Local GIT copy DPDK.ORG > > > > > > > > If a user clones from dpdk.org when dpdk 19.11 is tagged in the tree, or any > > time before dpdk 20.11 is tagged, then creates a feature branch, they may want > > to compare their abi to the latest stable version. If they check dpdk.org and > > see that dpdk 20.11 is the latest tag in the tree, they can run check-abi.sh > > with the reference tag specified as 20.11, which exists in the dpdk.org tree, > > but may not have been pulled into their local copy yet. > > > > I'm not saying this is definately what happened, but it would explain the > > reported observations. > > OK now I understand. > The user specified a reference which is not in his local tree. > I vote for considering this case as an user mistake, > and document it as a limitation of the tool integration in our build > testing scripts. > I'd be fine with that, given that this turned out to not be rays issue. Neil > > ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v4] devtools: remove old ABI validation script 2020-04-16 14:54 [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree Neil Horman 2020-04-17 10:11 ` Ray Kinsella @ 2020-05-24 20:34 ` Thomas Monjalon 1 sibling, 0 replies; 24+ messages in thread From: Thomas Monjalon @ 2020-05-24 20:34 UTC (permalink / raw) To: dev Cc: david.marchand, Neil Horman, Ray Kinsella, John McNamara, Marko Kovacevic From: Neil Horman <nhorman@tuxdriver.com> 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 <nhorman@tuxdriver.com> Signed-off-by: Thomas Monjalon <thomas@monjalon.net> --- No progress was done during a month after discussions about the usage example. This v4 fixes punctuation and replaces the usage example based on make with a link to the recommendation in doc/guides/contributing/patches.rst. Applied quickly for 20.05. --- MAINTAINERS | 1 - devtools/validate-abi.sh | 251 --------------------- doc/guides/contributing/abi_versioning.rst | 43 +--- doc/guides/contributing/patches.rst | 2 + 4 files changed, 11 insertions(+), 286 deletions(-) delete mode 100755 devtools/validate-abi.sh diff --git a/MAINTAINERS b/MAINTAINERS index 1616951d7f..d2b286701d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -150,7 +150,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-symbols.sh F: buildtools/map-list-symbol.sh F: drivers/*/*/*.map diff --git a/devtools/validate-abi.sh b/devtools/validate-abi.sh deleted file mode 100755 index f64e19d38f..0000000000 --- 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] <rev1> <rev2> - - 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 -} - -# 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 <dir>), 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 f4a9273afc..e96fde340f 100644 --- a/doc/guides/contributing/abi_versioning.rst +++ b/doc/guides/contributing/abi_versioning.rst @@ -682,41 +682,16 @@ 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 -<http://ispras.linuxbase.org/index.php/ABI_compliance_checker>`_. +``check-abi.sh``, for validating the DPDK ABI based on the libabigail +`abidiff utility <https://sourceware.org/libabigail/manual/abidiff.html>`_. -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 <refdir> <newdir> -The syntax of the ``validate-abi.sh`` utility is:: +Where <refdir> specifies the directory housing the reference build of DPDK, +and <newdir> specifies the DPDK build directory to check the ABI of. - ./devtools/validate-abi.sh <REV1> <REV2> - -Where ``REV1`` and ``REV2`` are valid gitrevisions(7) -https://www.kernel.org/pub/software/scm/git/docs/gitrevisions.html -on the local repo. - -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/ +The ABI compatibility is automatically verified when using a build script +from ``devtools``, if the variable ``DPDK_ABI_REF_VERSION`` is set with a tag, +as described in :ref:`ABI check recommendations<integrated_abi_check>`. diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst index 59442824a1..e6a934846e 100644 --- a/doc/guides/contributing/patches.rst +++ b/doc/guides/contributing/patches.rst @@ -513,6 +513,8 @@ in a single subfolder called "__builds" created in the current directory. Setting ``DPDK_BUILD_TEST_DIR`` to an absolute directory path e.g. ``/tmp`` is also supported. +.. _integrated_abi_check: + Checking ABI compatibility -------------------------- -- 2.26.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-05-24 20:34 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-16 14:54 [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree Neil Horman 2020-04-17 10:11 ` Ray Kinsella 2020-04-17 10:20 ` Thomas Monjalon 2020-04-17 10:35 ` Ray Kinsella 2020-04-17 11:46 ` Thomas Monjalon 2020-04-17 11:47 ` Ray Kinsella 2020-04-17 12:10 ` Thomas Monjalon 2020-04-17 15:42 ` Ray Kinsella 2020-04-17 16:10 ` Thomas Monjalon 2020-04-20 8:43 ` Ray Kinsella 2020-04-21 11:12 ` Neil Horman 2020-04-21 11:46 ` Thomas Monjalon 2020-04-21 18:56 ` Neil Horman 2020-04-21 21:42 ` Thomas Monjalon 2020-04-22 11:43 ` Ray Kinsella 2020-04-22 12:07 ` Neil Horman 2020-04-22 12:18 ` Thomas Monjalon 2020-04-22 13:19 ` Ray Kinsella 2020-04-22 13:30 ` Thomas Monjalon 2020-04-23 11:03 ` Neil Horman 2020-04-22 12:01 ` Neil Horman 2020-04-22 12:16 ` Thomas Monjalon 2020-04-23 10:57 ` Neil Horman 2020-05-24 20:34 ` [dpdk-dev] [PATCH v4] devtools: remove old ABI validation script Thomas Monjalon
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.