[dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Neil Horman]

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

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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.

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?

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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?

 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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.
> 

Looks like I need to set DPDK_ABI_REF_VERSION=master, not obvious.
Any tips or tricks would be welcome.
 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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...

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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?

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
 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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?

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Ray Kinsella]

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. 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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

Neil

> Thanks,
> 
> Ray K
> 
> 
> 
> Ray K
>  
> 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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)
Neil
 
> 
> 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Ray Kinsella]

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

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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.

Neil

> 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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?

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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?
 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Neil Horman]

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

> 
> 

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[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.

Neil

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCHv3] Remove validate-abi.sh from tree

[Neil Horman]

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

> 
> 

[dpdk-dev] [PATCH v4] devtools: remove old ABI validation script

[Thomas Monjalon]

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