Checks
Commit Message
Starting from Kevin and Bruce idea of using libabigail, here is an
alternate approach to implement ABI checks.
By default, those checks are disabled and enabling them requires a
manual step that generates the ABI dumps on a reference version for a
set of configurations.
Those checks are enabled in the CI by default for the default meson
options on x86 and aarch64 so that proposed patches are validated.
A cache of the ABI is stored in travis jobs.
Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
breaking the ABI in a future release.
For advanced developers and maintainers, the contributing guide details
the higher level scripts that are quite close to the existing devtools
scripts.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
.ci/linux-build.sh | 43 +++++++++++++++++++++++++++
.travis.yml | 20 +++++++++++--
devtools/check-abi-dump.sh | 46 +++++++++++++++++++++++++++++
devtools/check-abi-reference.sh | 27 +++++++++++++++++
devtools/dpdk.abignore | 2 ++
devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
devtools/gen-abi-reference.sh | 24 +++++++++++++++
devtools/test-build.sh | 13 ++++++--
devtools/test-meson-builds.sh | 6 ++++
doc/guides/contributing/patches.rst | 25 ++++++++++++++++
10 files changed, 230 insertions(+), 5 deletions(-)
create mode 100755 devtools/check-abi-dump.sh
create mode 100755 devtools/check-abi-reference.sh
create mode 100644 devtools/dpdk.abignore
create mode 100755 devtools/gen-abi-dump.sh
create mode 100755 devtools/gen-abi-reference.sh
Comments
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, December 20, 2019 3:21 PM
> To: dev@dpdk.org
> Cc: thomas@monjalon.net; Richardson, Bruce <bruce.richardson@intel.com>;
> Laatz, Kevin <kevin.laatz@intel.com>; aconole@redhat.com;
> nhorman@tuxdriver.com; Michael Santana <maicolgabriel@hotmail.com>;
> Mcnamara, John <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Subject: [PATCH] add ABI checks
>
> Starting from Kevin and Bruce idea of using libabigail, here is an
> alternate approach to implement ABI checks.
>
> By default, those checks are disabled and enabling them requires a manual
> step that generates the ABI dumps on a reference version for a set of
> configurations.
>
> Those checks are enabled in the CI by default for the default meson
> options on x86 and aarch64 so that proposed patches are validated.
> A cache of the ABI is stored in travis jobs.
> Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
> breaking the ABI in a future release.
>
> For advanced developers and maintainers, the contributing guide details
> the higher level scripts that are quite close to the existing devtools
> scripts.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> .ci/linux-build.sh | 43 +++++++++++++++++++++++++++
> .travis.yml | 20 +++++++++++--
> devtools/check-abi-dump.sh | 46 +++++++++++++++++++++++++++++
> devtools/check-abi-reference.sh | 27 +++++++++++++++++
> devtools/dpdk.abignore | 2 ++
> devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
> devtools/gen-abi-reference.sh | 24 +++++++++++++++
> devtools/test-build.sh | 13 ++++++--
> devtools/test-meson-builds.sh | 6 ++++
> doc/guides/contributing/patches.rst | 25 ++++++++++++++++
> 10 files changed, 230 insertions(+), 5 deletions(-) create mode 100755
> devtools/check-abi-dump.sh create mode 100755 devtools/check-abi-
> reference.sh create mode 100644 devtools/dpdk.abignore create mode
> 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
> reference.sh
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> ccc3a7ccd..345dba264 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -30,8 +30,51 @@ fi
>
> OPTS="$OPTS --default-library=$DEF_LIB"
> meson build --werror -Dexamples=all $OPTS
> +
> +if [ "$ABI_CHECKS" = "1" ]; then
> + git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
> + git fetch --tags ref ${REF_GIT_BRANCH:-master}
> +
> + head=$(git describe --all)
> + tag=$(git describe --abbrev=0)
> +
> + if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
> + rm -rf reference
> + fi
> +
> + if [ ! -d reference ]; then
> + gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
> + cp -a devtools/gen-abi-dump.sh $gen_abi_dump
> +
> + git checkout -qf $tag
> + ninja -C build
> + $gen_abi_dump build reference
> +
> + if [ "$AARCH64" != "1" ]; then
> + mkdir -p reference/app
> + cp -a build/app/dpdk-testpmd reference/app/
> +
> + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> + devtools/test-null.sh reference/app/dpdk-testpmd
> + unset LD_LIBRARY_PATH
> + fi
> + echo $tag > reference/VERSION
> +
> + git checkout -qf $head
> + fi
> +fi
> +
> ninja -C build
>
> +if [ "$ABI_CHECKS" = "1" ]; then
> + devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-}
> + if [ "$AARCH64" != "1" ]; then
> + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> + devtools/test-null.sh reference/app/dpdk-testpmd
> + unset LD_LIBRARY_PATH
> + fi
> +fi
> +
> if [ "$AARCH64" != "1" ]; then
> devtools/test-null.sh
> fi
> diff --git a/.travis.yml b/.travis.yml
> index 8f90d06f2..bbb060fa2 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,5 +1,8 @@
> language: c
> -cache: ccache
> +cache:
> + ccache: true
> + directories:
> + - reference
> compiler:
> - gcc
> - clang
> @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
>
> extra_packages: &extra_packages
> - *required_packages
> - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4,
> + abigail-tools]
>
> build_32b_packages: &build_32b_packages
> - *required_packages
> @@ -59,6 +62,13 @@ matrix:
> apt:
> packages:
> - *aarch64_packages
> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
> + compiler: gcc
> + addons:
> + apt:
> + packages:
> + - *aarch64_packages
> + - *extra_packages
> - env: DEF_LIB="static" EXTRA_PACKAGES=1
> compiler: gcc
> addons:
> @@ -72,6 +82,12 @@ matrix:
> packages:
> - *extra_packages
> - *doc_packages
> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
> + compiler: gcc
> + addons:
> + apt:
> + packages:
> + - *extra_packages
> - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
> compiler: gcc
> addons:
> diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh new
> file mode 100755 index 000000000..f48a2ae7e
> --- /dev/null
> +++ b/devtools/check-abi-dump.sh
> @@ -0,0 +1,46 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat,
> +Inc.
> +
> +if [ $# != 2 ] && [ $# != 3 ]; then
> + echo "Usage: $0 builddir dumpdir [warnonly]"
> + exit 1
> +fi
> +
> +builddir=$1
> +dumpdir=$2
> +warnonly=${3:-}
> +if [ ! -d $builddir ]; then
> + echo "Error: build directory '$builddir' does not exist."
> + exit 1
> +fi
> +if [ ! -d $dumpdir ]; then
> + echo "Error: dump directory '$dumpdir' does not exist."
> + exit 1
> +fi
> +
> +ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
> +for dump in $(find $dumpdir -name "*.dump"); do
> + libname=$(basename $dump)
> + libname=${libname%.dump}
> + result=
> + for f in $(find $builddir -name "$libname.so.*"); do
> + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> + continue
> + fi
> + result=found
> +
> + if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
> + if [ -z "$warnonly" ]; then
> + echo "Error: ABI issue reported for $dump, $f"
> + exit 1
> + fi
> + echo "Warning: ABI issue reported for $dump, $f"
> + fi
> + break
> + done
> + if [ "$result" != "found" ]; then
> + echo "Error: can't find a library for dump file $dump"
> + exit 1
> + fi
> +done
> diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-
> reference.sh new file mode 100755 index 000000000..7addb094e
> --- /dev/null
> +++ b/devtools/check-abi-reference.sh
> @@ -0,0 +1,27 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat,
> +Inc.
> +
> +devtools_dir=$(dirname $(readlink -f $0)) .
> +$devtools_dir/load-devel-config
> +
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> +builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> +
> +for dir in $abi_ref_build_dir/*; do
> + if [ "$dir" = "$abi_ref_build_dir" ]; then
> + exit 1
> + fi
> + if [ ! -d $dir/dump ]; then
> + echo "Skipping $dir"
> + continue
> + fi
> + target=$(basename $dir)
> + if [ -d $builds_dir/$target/install ]; then
> + libdir=$builds_dir/$target/install
> + else
> + libdir=$builds_dir/$target
> + fi
> + echo "Checking ABI between $libdir and $dir/dump"
> + $devtools_dir/check-abi-dump.sh $libdir $dir/dump done
> diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore new file mode
> 100644 index 000000000..b866b7f26
> --- /dev/null
> +++ b/devtools/dpdk.abignore
> @@ -0,0 +1,2 @@
> +[suppress_function]
> + symbol_version = EXPERIMENTAL
> diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh new file
> mode 100755 index 000000000..4e38d751f
> --- /dev/null
> +++ b/devtools/gen-abi-dump.sh
> @@ -0,0 +1,29 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat,
> +Inc.
> +
> +if [ $# != 2 ]; then
> + echo "Usage: $0 builddir dumpdir"
> + exit 1
> +fi
> +
> +builddir=$1
> +dumpdir=$2
> +if [ ! -d $builddir ]; then
> + echo "Error: build directory '$builddir' does not exist."
> + exit 1
> +fi
> +if [ -d $dumpdir ]; then
> + echo "Error: dump directory '$dumpdir' already exists."
> + exit 1
> +fi
> +
> +mkdir -p $dumpdir
> +for f in $(find $builddir -name "*.so.*"); do
> + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> + continue
> + fi
> +
> + libname=$(basename $f)
> + abidw --out-file $dumpdir/${libname%.so.*}.dump $f done
> diff --git a/devtools/gen-abi-reference.sh b/devtools/gen-abi-reference.sh
> new file mode 100755 index 000000000..f41d7fadc
> --- /dev/null
> +++ b/devtools/gen-abi-reference.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red Hat,
> +Inc.
> +
> +devtools_dir=$(dirname $(readlink -f $0)) .
> +$devtools_dir/load-devel-config
> +
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> +for dir in $abi_ref_build_dir/*; do
> + if [ "$dir" = "$abi_ref_build_dir" ]; then
> + exit 1
> + fi
> + if [ -d $dir/dump ]; then
> + echo "Skipping $dir"
> + continue
> + fi
> + if [ -d $dir/install ]; then
> + libdir=$dir/install
> + else
> + libdir=$dir
> + fi
> + echo "Dumping libraries from $libdir in $dir/dump"
> + $devtools_dir/gen-abi-dump.sh $libdir $dir/dump done
> diff --git a/devtools/test-build.sh b/devtools/test-build.sh index
> 52305fbb8..8cb5b56fb 100755
> --- a/devtools/test-build.sh
> +++ b/devtools/test-build.sh
> @@ -30,7 +30,8 @@ default_path=$PATH
> # - LIBSSO_SNOW3G_PATH
> # - LIBSSO_KASUMI_PATH
> # - LIBSSO_ZUC_PATH
> -. $(dirname $(readlink -f $0))/load-devel-config
> +devtools_dir=$(dirname $(readlink -f $0)) .
> +$devtools_dir/load-devel-config
>
> print_usage () {
> echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2]
> ...]]"
> @@ -64,6 +65,7 @@ print_help () {
> [ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
>
> J=$DPDK_MAKE_JOBS
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> short=false
> unset verbose
> @@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT #
> notify result on exit trap on_exit EXIT
>
> -cd $(dirname $(readlink -f $0))/..
> +cd $devtools_dir/..
>
> reset_env ()
> {
> @@ -233,7 +235,7 @@ for conf in $configs ; do
> # reload config with DPDK_TARGET set
> DPDK_TARGET=$target
> reset_env
> - . $(dirname $(readlink -f $0))/load-devel-config
> + . $devtools_dir/load-devel-config
>
> options=$(echo $conf | sed 's,[^~+]*,,')
> dir=$builds_dir/$conf
> @@ -246,6 +248,11 @@ for conf in $configs ; do
> export RTE_TARGET=$target
> rm -rf $dir/install
> ${MAKE} install O=$dir DESTDIR=$dir/install prefix=
> + if [ -d $abi_ref_build_dir/$conf/dump ]; then
> + echo "================== Check ABI $conf"
> + $devtools_dir/check-abi-dump.sh $dir/install \
> + $abi_ref_build_dir/$conf/dump
> + fi
> echo "================== Build examples for $conf"
> export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
> ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 688567714..aaefa38a2 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
>
> MESON=${MESON:-meson}
> use_shared="--default-library=shared"
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>
> if command -v gmake >/dev/null 2>&1 ; then @@ -88,6 +89,11 @@ build () #
> <directory> <target compiler> <meson options>
> echo "$ninja_cmd -C $builddir"
> $ninja_cmd -C $builddir
> fi
> +
> + if [ -d $abi_ref_build_dir/$1/dump ]; then
> + $srcdir/devtools/check-abi-dump.sh $builddir
> + $abi_ref_build_dir/$1/dump
> + fi
> }
>
> if [ "$1" = "-vv" ] ; then
> diff --git a/doc/guides/contributing/patches.rst
> b/doc/guides/contributing/patches.rst
> index 0686450e4..de3dff145 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -513,6 +513,31 @@ 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.
>
>
> +Checking ABI compatibility
> +--------------------------
> +
> +The first thing is to build reference binaries for the latest release
> +your patches are built on top of.
> +
> +Either you are in a git tree and an easy way to identify this is to run::
> +
> + git checkout $(git describe --abbrev=0)
> +
> +Or you use a tarball and you extract the sources in a director of your
> choice.
> +
> +Next is building those sources, refer to the previous paragraph.
> +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds occur
> +in this directory.
> +
> +Finally, the ABI dump files are generated with the
> +``devtools/gen-abi-reference.sh`` script. This script will look for
> +builds in the current sub directory ``reference``. But you can set the
> +environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different location.
> +
> +Once done, you can check your current binaries ABI with this reference
> +with the ``devtools/check-abi-reference.sh`` script.
> +
> +
> Sending Patches
> ---------------
>
> --
> 2.23.0
I still very much dislike forcing the user to generate his own reference version
to compare the ABI against. These should be archived and the user should just
be able to pull them down via git or http or otherwise. Two reasons for this:
1. Less error prone, since there is no chance of the user having an incorrect
build for whatever reason.
2. Less effort for the user than asking them to do extra builds. The more steps
the user has to follow, the less likely they are to attempt the process.
Regards,
/Bruce
> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Friday 20 December 2019 15:32
> To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; Laatz, Kevin <kevin.laatz@intel.com>;
> aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
> <maicolgabriel@hotmail.com>; Mcnamara, John <john.mcnamara@intel.com>;
> Kovacevic, Marko <marko.kovacevic@intel.com>; Kinsella, Ray
> <ray.kinsella@intel.com>
> Subject: RE: [PATCH] add ABI checks
>
>
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Friday, December 20, 2019 3:21 PM
> > To: dev@dpdk.org
> > Cc: thomas@monjalon.net; Richardson, Bruce
> > <bruce.richardson@intel.com>; Laatz, Kevin <kevin.laatz@intel.com>;
> > aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
> > <maicolgabriel@hotmail.com>; Mcnamara, John
> <john.mcnamara@intel.com>;
> > Kovacevic, Marko <marko.kovacevic@intel.com>
> > Subject: [PATCH] add ABI checks
> >
> > Starting from Kevin and Bruce idea of using libabigail, here is an
> > alternate approach to implement ABI checks.
> >
> > By default, those checks are disabled and enabling them requires a
> > manual step that generates the ABI dumps on a reference version for a
> > set of configurations.
> >
> > Those checks are enabled in the CI by default for the default meson
> > options on x86 and aarch64 so that proposed patches are validated.
> > A cache of the ABI is stored in travis jobs.
> > Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
> > breaking the ABI in a future release.
> >
> > For advanced developers and maintainers, the contributing guide
> > details the higher level scripts that are quite close to the existing
> > devtools scripts.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> > .ci/linux-build.sh | 43 +++++++++++++++++++++++++++
> > .travis.yml | 20 +++++++++++--
> > devtools/check-abi-dump.sh | 46
> +++++++++++++++++++++++++++++
> > devtools/check-abi-reference.sh | 27 +++++++++++++++++
> > devtools/dpdk.abignore | 2 ++
> > devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
> > devtools/gen-abi-reference.sh | 24 +++++++++++++++
> > devtools/test-build.sh | 13 ++++++--
> > devtools/test-meson-builds.sh | 6 ++++
> > doc/guides/contributing/patches.rst | 25 ++++++++++++++++
> > 10 files changed, 230 insertions(+), 5 deletions(-) create mode
> > 100755 devtools/check-abi-dump.sh create mode 100755
> > devtools/check-abi- reference.sh create mode 100644
> > devtools/dpdk.abignore create mode
> > 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
> > reference.sh
> >
> > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > ccc3a7ccd..345dba264 100755
> > --- a/.ci/linux-build.sh
> > +++ b/.ci/linux-build.sh
> > @@ -30,8 +30,51 @@ fi
> >
> > OPTS="$OPTS --default-library=$DEF_LIB"
> > meson build --werror -Dexamples=all $OPTS
> > +
> > +if [ "$ABI_CHECKS" = "1" ]; then
> > + git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
> > + git fetch --tags ref ${REF_GIT_BRANCH:-master}
> > +
> > + head=$(git describe --all)
> > + tag=$(git describe --abbrev=0)
> > +
> > + if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
> > + rm -rf reference
> > + fi
> > +
> > + if [ ! -d reference ]; then
> > + gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
> > + cp -a devtools/gen-abi-dump.sh $gen_abi_dump
> > +
> > + git checkout -qf $tag
> > + ninja -C build
> > + $gen_abi_dump build reference
> > +
> > + if [ "$AARCH64" != "1" ]; then
> > + mkdir -p reference/app
> > + cp -a build/app/dpdk-testpmd reference/app/
> > +
> > + export
> LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> > + devtools/test-null.sh reference/app/dpdk-testpmd
> > + unset LD_LIBRARY_PATH
> > + fi
> > + echo $tag > reference/VERSION
> > +
> > + git checkout -qf $head
> > + fi
> > +fi
> > +
> > ninja -C build
> >
> > +if [ "$ABI_CHECKS" = "1" ]; then
> > + devtools/check-abi-dump.sh build reference
> ${ABI_CHECKS_WARN_ONLY:-}
> > + if [ "$AARCH64" != "1" ]; then
> > + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> > + devtools/test-null.sh reference/app/dpdk-testpmd
> > + unset LD_LIBRARY_PATH
> > + fi
> > +fi
> > +
> > if [ "$AARCH64" != "1" ]; then
> > devtools/test-null.sh
> > fi
> > diff --git a/.travis.yml b/.travis.yml index 8f90d06f2..bbb060fa2
> > 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -1,5 +1,8 @@
> > language: c
> > -cache: ccache
> > +cache:
> > + ccache: true
> > + directories:
> > + - reference
> > compiler:
> > - gcc
> > - clang
> > @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
> >
> > extra_packages: &extra_packages
> > - *required_packages
> > - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> > + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4,
> > + abigail-tools]
> >
> > build_32b_packages: &build_32b_packages
> > - *required_packages
> > @@ -59,6 +62,13 @@ matrix:
> > apt:
> > packages:
> > - *aarch64_packages
> > + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
> > + compiler: gcc
> > + addons:
> > + apt:
> > + packages:
> > + - *aarch64_packages
> > + - *extra_packages
> > - env: DEF_LIB="static" EXTRA_PACKAGES=1
> > compiler: gcc
> > addons:
> > @@ -72,6 +82,12 @@ matrix:
> > packages:
> > - *extra_packages
> > - *doc_packages
> > + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
> > + compiler: gcc
> > + addons:
> > + apt:
> > + packages:
> > + - *extra_packages
> > - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> EXTRA_PACKAGES=1
> > compiler: gcc
> > addons:
> > diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
> > new file mode 100755 index 000000000..f48a2ae7e
> > --- /dev/null
> > +++ b/devtools/check-abi-dump.sh
> > @@ -0,0 +1,46 @@
> > +#!/bin/sh -e
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> Hat,
> > +Inc.
> > +
> > +if [ $# != 2 ] && [ $# != 3 ]; then
> > + echo "Usage: $0 builddir dumpdir [warnonly]"
> > + exit 1
> > +fi
> > +
> > +builddir=$1
> > +dumpdir=$2
> > +warnonly=${3:-}
> > +if [ ! -d $builddir ]; then
> > + echo "Error: build directory '$builddir' does not exist."
> > + exit 1
> > +fi
> > +if [ ! -d $dumpdir ]; then
> > + echo "Error: dump directory '$dumpdir' does not exist."
> > + exit 1
> > +fi
> > +
> > +ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
> > +for dump in $(find $dumpdir -name "*.dump"); do
> > + libname=$(basename $dump)
> > + libname=${libname%.dump}
> > + result=
> > + for f in $(find $builddir -name "$libname.so.*"); do
> > + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> > + continue
> > + fi
> > + result=found
> > +
> > + if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
> > + if [ -z "$warnonly" ]; then
> > + echo "Error: ABI issue reported for $dump, $f"
> > + exit 1
> > + fi
> > + echo "Warning: ABI issue reported for $dump, $f"
> > + fi
> > + break
> > + done
> > + if [ "$result" != "found" ]; then
> > + echo "Error: can't find a library for dump file $dump"
> > + exit 1
> > + fi
> > +done
> > diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-
> > reference.sh new file mode 100755 index 000000000..7addb094e
> > --- /dev/null
> > +++ b/devtools/check-abi-reference.sh
> > @@ -0,0 +1,27 @@
> > +#!/bin/sh -e
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> Hat,
> > +Inc.
> > +
> > +devtools_dir=$(dirname $(readlink -f $0)) .
> > +$devtools_dir/load-devel-config
> > +
> > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > +builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> > +
> > +for dir in $abi_ref_build_dir/*; do
> > + if [ "$dir" = "$abi_ref_build_dir" ]; then
> > + exit 1
> > + fi
> > + if [ ! -d $dir/dump ]; then
> > + echo "Skipping $dir"
> > + continue
> > + fi
> > + target=$(basename $dir)
> > + if [ -d $builds_dir/$target/install ]; then
> > + libdir=$builds_dir/$target/install
> > + else
> > + libdir=$builds_dir/$target
> > + fi
> > + echo "Checking ABI between $libdir and $dir/dump"
> > + $devtools_dir/check-abi-dump.sh $libdir $dir/dump done
> > diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore new file
> > mode
> > 100644 index 000000000..b866b7f26
> > --- /dev/null
> > +++ b/devtools/dpdk.abignore
> > @@ -0,0 +1,2 @@
> > +[suppress_function]
> > + symbol_version = EXPERIMENTAL
> > diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh new
> > file mode 100755 index 000000000..4e38d751f
> > --- /dev/null
> > +++ b/devtools/gen-abi-dump.sh
> > @@ -0,0 +1,29 @@
> > +#!/bin/sh -e
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> Hat,
> > +Inc.
> > +
> > +if [ $# != 2 ]; then
> > + echo "Usage: $0 builddir dumpdir"
> > + exit 1
> > +fi
> > +
> > +builddir=$1
> > +dumpdir=$2
> > +if [ ! -d $builddir ]; then
> > + echo "Error: build directory '$builddir' does not exist."
> > + exit 1
> > +fi
> > +if [ -d $dumpdir ]; then
> > + echo "Error: dump directory '$dumpdir' already exists."
> > + exit 1
> > +fi
> > +
> > +mkdir -p $dumpdir
> > +for f in $(find $builddir -name "*.so.*"); do
> > + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> > + continue
> > + fi
> > +
> > + libname=$(basename $f)
> > + abidw --out-file $dumpdir/${libname%.so.*}.dump $f done
> > diff --git a/devtools/gen-abi-reference.sh
> > b/devtools/gen-abi-reference.sh new file mode 100755 index
> > 000000000..f41d7fadc
> > --- /dev/null
> > +++ b/devtools/gen-abi-reference.sh
> > @@ -0,0 +1,24 @@
> > +#!/bin/sh -e
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> Hat,
> > +Inc.
> > +
> > +devtools_dir=$(dirname $(readlink -f $0)) .
> > +$devtools_dir/load-devel-config
> > +
> > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > +for dir in $abi_ref_build_dir/*; do
> > + if [ "$dir" = "$abi_ref_build_dir" ]; then
> > + exit 1
> > + fi
> > + if [ -d $dir/dump ]; then
> > + echo "Skipping $dir"
> > + continue
> > + fi
> > + if [ -d $dir/install ]; then
> > + libdir=$dir/install
> > + else
> > + libdir=$dir
> > + fi
> > + echo "Dumping libraries from $libdir in $dir/dump"
> > + $devtools_dir/gen-abi-dump.sh $libdir $dir/dump done
> > diff --git a/devtools/test-build.sh b/devtools/test-build.sh index
> > 52305fbb8..8cb5b56fb 100755
> > --- a/devtools/test-build.sh
> > +++ b/devtools/test-build.sh
> > @@ -30,7 +30,8 @@ default_path=$PATH
> > # - LIBSSO_SNOW3G_PATH
> > # - LIBSSO_KASUMI_PATH
> > # - LIBSSO_ZUC_PATH
> > -. $(dirname $(readlink -f $0))/load-devel-config
> > +devtools_dir=$(dirname $(readlink -f $0)) .
> > +$devtools_dir/load-devel-config
> >
> > print_usage () {
> > echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2]
> > ...]]"
> > @@ -64,6 +65,7 @@ print_help () {
> > [ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
> >
> > J=$DPDK_MAKE_JOBS
> > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> > short=false
> > unset verbose
> > @@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT
> #
> > notify result on exit trap on_exit EXIT
> >
> > -cd $(dirname $(readlink -f $0))/..
> > +cd $devtools_dir/..
> >
> > reset_env ()
> > {
> > @@ -233,7 +235,7 @@ for conf in $configs ; do
> > # reload config with DPDK_TARGET set
> > DPDK_TARGET=$target
> > reset_env
> > - . $(dirname $(readlink -f $0))/load-devel-config
> > + . $devtools_dir/load-devel-config
> >
> > options=$(echo $conf | sed 's,[^~+]*,,')
> > dir=$builds_dir/$conf
> > @@ -246,6 +248,11 @@ for conf in $configs ; do
> > export RTE_TARGET=$target
> > rm -rf $dir/install
> > ${MAKE} install O=$dir DESTDIR=$dir/install prefix=
> > + if [ -d $abi_ref_build_dir/$conf/dump ]; then
> > + echo "================== Check ABI $conf"
> > + $devtools_dir/check-abi-dump.sh $dir/install \
> > + $abi_ref_build_dir/$conf/dump
> > + fi
> > echo "================== Build examples for $conf"
> > export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
> > ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
> > diff --git a/devtools/test-meson-builds.sh
> > b/devtools/test-meson-builds.sh index 688567714..aaefa38a2 100755
> > --- a/devtools/test-meson-builds.sh
> > +++ b/devtools/test-meson-builds.sh
> > @@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
> >
> > MESON=${MESON:-meson}
> > use_shared="--default-library=shared"
> > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> >
> > if command -v gmake >/dev/null 2>&1 ; then @@ -88,6 +89,11 @@ build
> > () # <directory> <target compiler> <meson options>
> > echo "$ninja_cmd -C $builddir"
> > $ninja_cmd -C $builddir
> > fi
> > +
> > + if [ -d $abi_ref_build_dir/$1/dump ]; then
> > + $srcdir/devtools/check-abi-dump.sh $builddir
> > + $abi_ref_build_dir/$1/dump
> > + fi
> > }
> >
> > if [ "$1" = "-vv" ] ; then
> > diff --git a/doc/guides/contributing/patches.rst
> > b/doc/guides/contributing/patches.rst
> > index 0686450e4..de3dff145 100644
> > --- a/doc/guides/contributing/patches.rst
> > +++ b/doc/guides/contributing/patches.rst
> > @@ -513,6 +513,31 @@ 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.
> >
> >
> > +Checking ABI compatibility
> > +--------------------------
> > +
> > +The first thing is to build reference binaries for the latest
> release
> > +your patches are built on top of.
> > +
> > +Either you are in a git tree and an easy way to identify this is to
> run::
> > +
> > + git checkout $(git describe --abbrev=0)
> > +
> > +Or you use a tarball and you extract the sources in a director of
> > +your
> > choice.
> > +
> > +Next is building those sources, refer to the previous paragraph.
> > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > +occur in this directory.
> > +
> > +Finally, the ABI dump files are generated with the
> > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > +builds in the current sub directory ``reference``. But you can set
> > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> location.
> > +
> > +Once done, you can check your current binaries ABI with this
> > +reference with the ``devtools/check-abi-reference.sh`` script.
> > +
> > +
> > Sending Patches
> > ---------------
> >
> > --
> > 2.23.0
>
> I still very much dislike forcing the user to generate his own
> reference version to compare the ABI against. These should be archived
> and the user should just be able to pull them down via git or http or
> otherwise. Two reasons for this:
>
> 1. Less error prone, since there is no chance of the user having an
> incorrect build for whatever reason.
>
> 2. Less effort for the user than asking them to do extra builds. The
> more steps the user has to follow, the less likely they are to attempt
> the process.
>
> Regards,
> /Bruce
>
+1 ... 100% agree with this.
Many people won't know or understand what the reference is,
or why they to generate it.
Ray K
On Fri, Dec 20, 2019 at 04:20:58PM +0100, David Marchand wrote:
> Starting from Kevin and Bruce idea of using libabigail, here is an
> alternate approach to implement ABI checks.
>
> By default, those checks are disabled and enabling them requires a
> manual step that generates the ABI dumps on a reference version for a
> set of configurations.
>
> Those checks are enabled in the CI by default for the default meson
> options on x86 and aarch64 so that proposed patches are validated.
> A cache of the ABI is stored in travis jobs.
Are they? I'm looking here:
https://travis-ci.com/DPDK/dpdk
And can't for the life of me see where those cached ABI files are pulled from,
or where the checks are encoded at all. Am I missing something (which is the
far greater likelyhood).
assuming they are there, and I'm just not seeing them, do we have external
access to them? Could we download them as part of the abi check process on
local trees?
Neil
> Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
> breaking the ABI in a future release.
>
> For advanced developers and maintainers, the contributing guide details
> the higher level scripts that are quite close to the existing devtools
> scripts.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> .ci/linux-build.sh | 43 +++++++++++++++++++++++++++
> .travis.yml | 20 +++++++++++--
> devtools/check-abi-dump.sh | 46 +++++++++++++++++++++++++++++
> devtools/check-abi-reference.sh | 27 +++++++++++++++++
> devtools/dpdk.abignore | 2 ++
> devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
> devtools/gen-abi-reference.sh | 24 +++++++++++++++
> devtools/test-build.sh | 13 ++++++--
> devtools/test-meson-builds.sh | 6 ++++
> doc/guides/contributing/patches.rst | 25 ++++++++++++++++
> 10 files changed, 230 insertions(+), 5 deletions(-)
> create mode 100755 devtools/check-abi-dump.sh
> create mode 100755 devtools/check-abi-reference.sh
> create mode 100644 devtools/dpdk.abignore
> create mode 100755 devtools/gen-abi-dump.sh
> create mode 100755 devtools/gen-abi-reference.sh
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index ccc3a7ccd..345dba264 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -30,8 +30,51 @@ fi
>
> OPTS="$OPTS --default-library=$DEF_LIB"
> meson build --werror -Dexamples=all $OPTS
> +
> +if [ "$ABI_CHECKS" = "1" ]; then
> + git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
> + git fetch --tags ref ${REF_GIT_BRANCH:-master}
> +
> + head=$(git describe --all)
> + tag=$(git describe --abbrev=0)
> +
> + if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
> + rm -rf reference
> + fi
> +
> + if [ ! -d reference ]; then
> + gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
> + cp -a devtools/gen-abi-dump.sh $gen_abi_dump
> +
> + git checkout -qf $tag
> + ninja -C build
> + $gen_abi_dump build reference
> +
> + if [ "$AARCH64" != "1" ]; then
> + mkdir -p reference/app
> + cp -a build/app/dpdk-testpmd reference/app/
> +
> + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> + devtools/test-null.sh reference/app/dpdk-testpmd
> + unset LD_LIBRARY_PATH
> + fi
> + echo $tag > reference/VERSION
> +
> + git checkout -qf $head
> + fi
> +fi
> +
> ninja -C build
>
> +if [ "$ABI_CHECKS" = "1" ]; then
> + devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-}
> + if [ "$AARCH64" != "1" ]; then
> + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> + devtools/test-null.sh reference/app/dpdk-testpmd
> + unset LD_LIBRARY_PATH
> + fi
> +fi
> +
> if [ "$AARCH64" != "1" ]; then
> devtools/test-null.sh
> fi
> diff --git a/.travis.yml b/.travis.yml
> index 8f90d06f2..bbb060fa2 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,5 +1,8 @@
> language: c
> -cache: ccache
> +cache:
> + ccache: true
> + directories:
> + - reference
> compiler:
> - gcc
> - clang
> @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
>
> extra_packages: &extra_packages
> - *required_packages
> - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4, abigail-tools]
>
> build_32b_packages: &build_32b_packages
> - *required_packages
> @@ -59,6 +62,13 @@ matrix:
> apt:
> packages:
> - *aarch64_packages
> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
> + compiler: gcc
> + addons:
> + apt:
> + packages:
> + - *aarch64_packages
> + - *extra_packages
> - env: DEF_LIB="static" EXTRA_PACKAGES=1
> compiler: gcc
> addons:
> @@ -72,6 +82,12 @@ matrix:
> packages:
> - *extra_packages
> - *doc_packages
> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
> + compiler: gcc
> + addons:
> + apt:
> + packages:
> + - *extra_packages
> - env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
> compiler: gcc
> addons:
> diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
> new file mode 100755
> index 000000000..f48a2ae7e
> --- /dev/null
> +++ b/devtools/check-abi-dump.sh
> @@ -0,0 +1,46 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2019 Red Hat, Inc.
> +
> +if [ $# != 2 ] && [ $# != 3 ]; then
> + echo "Usage: $0 builddir dumpdir [warnonly]"
> + exit 1
> +fi
> +
> +builddir=$1
> +dumpdir=$2
> +warnonly=${3:-}
> +if [ ! -d $builddir ]; then
> + echo "Error: build directory '$builddir' does not exist."
> + exit 1
> +fi
> +if [ ! -d $dumpdir ]; then
> + echo "Error: dump directory '$dumpdir' does not exist."
> + exit 1
> +fi
> +
> +ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
> +for dump in $(find $dumpdir -name "*.dump"); do
> + libname=$(basename $dump)
> + libname=${libname%.dump}
> + result=
> + for f in $(find $builddir -name "$libname.so.*"); do
> + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> + continue
> + fi
> + result=found
> +
> + if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
> + if [ -z "$warnonly" ]; then
> + echo "Error: ABI issue reported for $dump, $f"
> + exit 1
> + fi
> + echo "Warning: ABI issue reported for $dump, $f"
> + fi
> + break
> + done
> + if [ "$result" != "found" ]; then
> + echo "Error: can't find a library for dump file $dump"
> + exit 1
> + fi
> +done
> diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-reference.sh
> new file mode 100755
> index 000000000..7addb094e
> --- /dev/null
> +++ b/devtools/check-abi-reference.sh
> @@ -0,0 +1,27 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2019 Red Hat, Inc.
> +
> +devtools_dir=$(dirname $(readlink -f $0))
> +. $devtools_dir/load-devel-config
> +
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> +builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> +
> +for dir in $abi_ref_build_dir/*; do
> + if [ "$dir" = "$abi_ref_build_dir" ]; then
> + exit 1
> + fi
> + if [ ! -d $dir/dump ]; then
> + echo "Skipping $dir"
> + continue
> + fi
> + target=$(basename $dir)
> + if [ -d $builds_dir/$target/install ]; then
> + libdir=$builds_dir/$target/install
> + else
> + libdir=$builds_dir/$target
> + fi
> + echo "Checking ABI between $libdir and $dir/dump"
> + $devtools_dir/check-abi-dump.sh $libdir $dir/dump
> +done
> diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore
> new file mode 100644
> index 000000000..b866b7f26
> --- /dev/null
> +++ b/devtools/dpdk.abignore
> @@ -0,0 +1,2 @@
> +[suppress_function]
> + symbol_version = EXPERIMENTAL
> diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh
> new file mode 100755
> index 000000000..4e38d751f
> --- /dev/null
> +++ b/devtools/gen-abi-dump.sh
> @@ -0,0 +1,29 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2019 Red Hat, Inc.
> +
> +if [ $# != 2 ]; then
> + echo "Usage: $0 builddir dumpdir"
> + exit 1
> +fi
> +
> +builddir=$1
> +dumpdir=$2
> +if [ ! -d $builddir ]; then
> + echo "Error: build directory '$builddir' does not exist."
> + exit 1
> +fi
> +if [ -d $dumpdir ]; then
> + echo "Error: dump directory '$dumpdir' already exists."
> + exit 1
> +fi
> +
> +mkdir -p $dumpdir
> +for f in $(find $builddir -name "*.so.*"); do
> + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> + continue
> + fi
> +
> + libname=$(basename $f)
> + abidw --out-file $dumpdir/${libname%.so.*}.dump $f
> +done
> diff --git a/devtools/gen-abi-reference.sh b/devtools/gen-abi-reference.sh
> new file mode 100755
> index 000000000..f41d7fadc
> --- /dev/null
> +++ b/devtools/gen-abi-reference.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright (c) 2019 Red Hat, Inc.
> +
> +devtools_dir=$(dirname $(readlink -f $0))
> +. $devtools_dir/load-devel-config
> +
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> +for dir in $abi_ref_build_dir/*; do
> + if [ "$dir" = "$abi_ref_build_dir" ]; then
> + exit 1
> + fi
> + if [ -d $dir/dump ]; then
> + echo "Skipping $dir"
> + continue
> + fi
> + if [ -d $dir/install ]; then
> + libdir=$dir/install
> + else
> + libdir=$dir
> + fi
> + echo "Dumping libraries from $libdir in $dir/dump"
> + $devtools_dir/gen-abi-dump.sh $libdir $dir/dump
> +done
> diff --git a/devtools/test-build.sh b/devtools/test-build.sh
> index 52305fbb8..8cb5b56fb 100755
> --- a/devtools/test-build.sh
> +++ b/devtools/test-build.sh
> @@ -30,7 +30,8 @@ default_path=$PATH
> # - LIBSSO_SNOW3G_PATH
> # - LIBSSO_KASUMI_PATH
> # - LIBSSO_ZUC_PATH
> -. $(dirname $(readlink -f $0))/load-devel-config
> +devtools_dir=$(dirname $(readlink -f $0))
> +. $devtools_dir/load-devel-config
>
> print_usage () {
> echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2] ...]]"
> @@ -64,6 +65,7 @@ print_help () {
> [ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
>
> J=$DPDK_MAKE_JOBS
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> short=false
> unset verbose
> @@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT
> # notify result on exit
> trap on_exit EXIT
>
> -cd $(dirname $(readlink -f $0))/..
> +cd $devtools_dir/..
>
> reset_env ()
> {
> @@ -233,7 +235,7 @@ for conf in $configs ; do
> # reload config with DPDK_TARGET set
> DPDK_TARGET=$target
> reset_env
> - . $(dirname $(readlink -f $0))/load-devel-config
> + . $devtools_dir/load-devel-config
>
> options=$(echo $conf | sed 's,[^~+]*,,')
> dir=$builds_dir/$conf
> @@ -246,6 +248,11 @@ for conf in $configs ; do
> export RTE_TARGET=$target
> rm -rf $dir/install
> ${MAKE} install O=$dir DESTDIR=$dir/install prefix=
> + if [ -d $abi_ref_build_dir/$conf/dump ]; then
> + echo "================== Check ABI $conf"
> + $devtools_dir/check-abi-dump.sh $dir/install \
> + $abi_ref_build_dir/$conf/dump
> + fi
> echo "================== Build examples for $conf"
> export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
> ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 688567714..aaefa38a2 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
>
> MESON=${MESON:-meson}
> use_shared="--default-library=shared"
> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>
> if command -v gmake >/dev/null 2>&1 ; then
> @@ -88,6 +89,11 @@ build () # <directory> <target compiler> <meson options>
> echo "$ninja_cmd -C $builddir"
> $ninja_cmd -C $builddir
> fi
> +
> + if [ -d $abi_ref_build_dir/$1/dump ]; then
> + $srcdir/devtools/check-abi-dump.sh $builddir
> + $abi_ref_build_dir/$1/dump
> + fi
> }
>
> if [ "$1" = "-vv" ] ; then
> diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
> index 0686450e4..de3dff145 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -513,6 +513,31 @@ 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.
>
>
> +Checking ABI compatibility
> +--------------------------
> +
> +The first thing is to build reference binaries for the latest release your
> +patches are built on top of.
> +
> +Either you are in a git tree and an easy way to identify this is to run::
> +
> + git checkout $(git describe --abbrev=0)
> +
> +Or you use a tarball and you extract the sources in a director of your choice.
> +
> +Next is building those sources, refer to the previous paragraph.
> +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds occur in this
> +directory.
> +
> +Finally, the ABI dump files are generated with the
> +``devtools/gen-abi-reference.sh`` script. This script will look for builds in
> +the current sub directory ``reference``. But you can set the environment
> +variable ``DPDK_ABI_REF_BUILD_DIR`` to a different location.
> +
> +Once done, you can check your current binaries ABI with this reference with the
> +``devtools/check-abi-reference.sh`` script.
> +
> +
> Sending Patches
> ---------------
>
> --
> 2.23.0
>
>
20/12/2019 17:20, Kinsella, Ray:
> > -----Original Message-----
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > Sent: Friday 20 December 2019 15:32
> > To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
> > Cc: thomas@monjalon.net; Laatz, Kevin <kevin.laatz@intel.com>;
> > aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
> > <maicolgabriel@hotmail.com>; Mcnamara, John <john.mcnamara@intel.com>;
> > Kovacevic, Marko <marko.kovacevic@intel.com>; Kinsella, Ray
> > <ray.kinsella@intel.com>
> > Subject: RE: [PATCH] add ABI checks
> >
> >
> >
> > > -----Original Message-----
> > > From: David Marchand <david.marchand@redhat.com>
> > > Sent: Friday, December 20, 2019 3:21 PM
> > > To: dev@dpdk.org
> > > Cc: thomas@monjalon.net; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Laatz, Kevin <kevin.laatz@intel.com>;
> > > aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
> > > <maicolgabriel@hotmail.com>; Mcnamara, John
> > <john.mcnamara@intel.com>;
> > > Kovacevic, Marko <marko.kovacevic@intel.com>
> > > Subject: [PATCH] add ABI checks
> > >
> > > Starting from Kevin and Bruce idea of using libabigail, here is an
> > > alternate approach to implement ABI checks.
> > >
> > > By default, those checks are disabled and enabling them requires a
> > > manual step that generates the ABI dumps on a reference version for a
> > > set of configurations.
> > >
> > > Those checks are enabled in the CI by default for the default meson
> > > options on x86 and aarch64 so that proposed patches are validated.
> > > A cache of the ABI is stored in travis jobs.
> > > Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
> > > breaking the ABI in a future release.
> > >
> > > For advanced developers and maintainers, the contributing guide
> > > details the higher level scripts that are quite close to the existing
> > > devtools scripts.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > > .ci/linux-build.sh | 43 +++++++++++++++++++++++++++
> > > .travis.yml | 20 +++++++++++--
> > > devtools/check-abi-dump.sh | 46
> > +++++++++++++++++++++++++++++
> > > devtools/check-abi-reference.sh | 27 +++++++++++++++++
> > > devtools/dpdk.abignore | 2 ++
> > > devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
> > > devtools/gen-abi-reference.sh | 24 +++++++++++++++
> > > devtools/test-build.sh | 13 ++++++--
> > > devtools/test-meson-builds.sh | 6 ++++
> > > doc/guides/contributing/patches.rst | 25 ++++++++++++++++
> > > 10 files changed, 230 insertions(+), 5 deletions(-) create mode
> > > 100755 devtools/check-abi-dump.sh create mode 100755
> > > devtools/check-abi- reference.sh create mode 100644
> > > devtools/dpdk.abignore create mode
> > > 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
> > > reference.sh
> > >
> > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
> > > ccc3a7ccd..345dba264 100755
> > > --- a/.ci/linux-build.sh
> > > +++ b/.ci/linux-build.sh
> > > @@ -30,8 +30,51 @@ fi
> > >
> > > OPTS="$OPTS --default-library=$DEF_LIB"
> > > meson build --werror -Dexamples=all $OPTS
> > > +
> > > +if [ "$ABI_CHECKS" = "1" ]; then
> > > + git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
> > > + git fetch --tags ref ${REF_GIT_BRANCH:-master}
> > > +
> > > + head=$(git describe --all)
> > > + tag=$(git describe --abbrev=0)
> > > +
> > > + if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
> > > + rm -rf reference
> > > + fi
> > > +
> > > + if [ ! -d reference ]; then
> > > + gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
> > > + cp -a devtools/gen-abi-dump.sh $gen_abi_dump
> > > +
> > > + git checkout -qf $tag
> > > + ninja -C build
> > > + $gen_abi_dump build reference
> > > +
> > > + if [ "$AARCH64" != "1" ]; then
> > > + mkdir -p reference/app
> > > + cp -a build/app/dpdk-testpmd reference/app/
> > > +
> > > + export
> > LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> > > + devtools/test-null.sh reference/app/dpdk-testpmd
> > > + unset LD_LIBRARY_PATH
> > > + fi
> > > + echo $tag > reference/VERSION
> > > +
> > > + git checkout -qf $head
> > > + fi
> > > +fi
> > > +
> > > ninja -C build
> > >
> > > +if [ "$ABI_CHECKS" = "1" ]; then
> > > + devtools/check-abi-dump.sh build reference
> > ${ABI_CHECKS_WARN_ONLY:-}
> > > + if [ "$AARCH64" != "1" ]; then
> > > + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
> > > + devtools/test-null.sh reference/app/dpdk-testpmd
> > > + unset LD_LIBRARY_PATH
> > > + fi
> > > +fi
> > > +
> > > if [ "$AARCH64" != "1" ]; then
> > > devtools/test-null.sh
> > > fi
> > > diff --git a/.travis.yml b/.travis.yml index 8f90d06f2..bbb060fa2
> > > 100644
> > > --- a/.travis.yml
> > > +++ b/.travis.yml
> > > @@ -1,5 +1,8 @@
> > > language: c
> > > -cache: ccache
> > > +cache:
> > > + ccache: true
> > > + directories:
> > > + - reference
> > > compiler:
> > > - gcc
> > > - clang
> > > @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
> > >
> > > extra_packages: &extra_packages
> > > - *required_packages
> > > - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
> > > + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4,
> > > + abigail-tools]
> > >
> > > build_32b_packages: &build_32b_packages
> > > - *required_packages
> > > @@ -59,6 +62,13 @@ matrix:
> > > apt:
> > > packages:
> > > - *aarch64_packages
> > > + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
> > > + compiler: gcc
> > > + addons:
> > > + apt:
> > > + packages:
> > > + - *aarch64_packages
> > > + - *extra_packages
> > > - env: DEF_LIB="static" EXTRA_PACKAGES=1
> > > compiler: gcc
> > > addons:
> > > @@ -72,6 +82,12 @@ matrix:
> > > packages:
> > > - *extra_packages
> > > - *doc_packages
> > > + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
> > > + compiler: gcc
> > > + addons:
> > > + apt:
> > > + packages:
> > > + - *extra_packages
> > > - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
> > EXTRA_PACKAGES=1
> > > compiler: gcc
> > > addons:
> > > diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
> > > new file mode 100755 index 000000000..f48a2ae7e
> > > --- /dev/null
> > > +++ b/devtools/check-abi-dump.sh
> > > @@ -0,0 +1,46 @@
> > > +#!/bin/sh -e
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> > Hat,
> > > +Inc.
> > > +
> > > +if [ $# != 2 ] && [ $# != 3 ]; then
> > > + echo "Usage: $0 builddir dumpdir [warnonly]"
> > > + exit 1
> > > +fi
> > > +
> > > +builddir=$1
> > > +dumpdir=$2
> > > +warnonly=${3:-}
> > > +if [ ! -d $builddir ]; then
> > > + echo "Error: build directory '$builddir' does not exist."
> > > + exit 1
> > > +fi
> > > +if [ ! -d $dumpdir ]; then
> > > + echo "Error: dump directory '$dumpdir' does not exist."
> > > + exit 1
> > > +fi
> > > +
> > > +ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
> > > +for dump in $(find $dumpdir -name "*.dump"); do
> > > + libname=$(basename $dump)
> > > + libname=${libname%.dump}
> > > + result=
> > > + for f in $(find $builddir -name "$libname.so.*"); do
> > > + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> > > + continue
> > > + fi
> > > + result=found
> > > +
> > > + if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
> > > + if [ -z "$warnonly" ]; then
> > > + echo "Error: ABI issue reported for $dump, $f"
> > > + exit 1
> > > + fi
> > > + echo "Warning: ABI issue reported for $dump, $f"
> > > + fi
> > > + break
> > > + done
> > > + if [ "$result" != "found" ]; then
> > > + echo "Error: can't find a library for dump file $dump"
> > > + exit 1
> > > + fi
> > > +done
> > > diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-
> > > reference.sh new file mode 100755 index 000000000..7addb094e
> > > --- /dev/null
> > > +++ b/devtools/check-abi-reference.sh
> > > @@ -0,0 +1,27 @@
> > > +#!/bin/sh -e
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> > Hat,
> > > +Inc.
> > > +
> > > +devtools_dir=$(dirname $(readlink -f $0)) .
> > > +$devtools_dir/load-devel-config
> > > +
> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > > +builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> > > +
> > > +for dir in $abi_ref_build_dir/*; do
> > > + if [ "$dir" = "$abi_ref_build_dir" ]; then
> > > + exit 1
> > > + fi
> > > + if [ ! -d $dir/dump ]; then
> > > + echo "Skipping $dir"
> > > + continue
> > > + fi
> > > + target=$(basename $dir)
> > > + if [ -d $builds_dir/$target/install ]; then
> > > + libdir=$builds_dir/$target/install
> > > + else
> > > + libdir=$builds_dir/$target
> > > + fi
> > > + echo "Checking ABI between $libdir and $dir/dump"
> > > + $devtools_dir/check-abi-dump.sh $libdir $dir/dump done
> > > diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore new file
> > > mode
> > > 100644 index 000000000..b866b7f26
> > > --- /dev/null
> > > +++ b/devtools/dpdk.abignore
> > > @@ -0,0 +1,2 @@
> > > +[suppress_function]
> > > + symbol_version = EXPERIMENTAL
> > > diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh new
> > > file mode 100755 index 000000000..4e38d751f
> > > --- /dev/null
> > > +++ b/devtools/gen-abi-dump.sh
> > > @@ -0,0 +1,29 @@
> > > +#!/bin/sh -e
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> > Hat,
> > > +Inc.
> > > +
> > > +if [ $# != 2 ]; then
> > > + echo "Usage: $0 builddir dumpdir"
> > > + exit 1
> > > +fi
> > > +
> > > +builddir=$1
> > > +dumpdir=$2
> > > +if [ ! -d $builddir ]; then
> > > + echo "Error: build directory '$builddir' does not exist."
> > > + exit 1
> > > +fi
> > > +if [ -d $dumpdir ]; then
> > > + echo "Error: dump directory '$dumpdir' already exists."
> > > + exit 1
> > > +fi
> > > +
> > > +mkdir -p $dumpdir
> > > +for f in $(find $builddir -name "*.so.*"); do
> > > + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
> > > + continue
> > > + fi
> > > +
> > > + libname=$(basename $f)
> > > + abidw --out-file $dumpdir/${libname%.so.*}.dump $f done
> > > diff --git a/devtools/gen-abi-reference.sh
> > > b/devtools/gen-abi-reference.sh new file mode 100755 index
> > > 000000000..f41d7fadc
> > > --- /dev/null
> > > +++ b/devtools/gen-abi-reference.sh
> > > @@ -0,0 +1,24 @@
> > > +#!/bin/sh -e
> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
> > Hat,
> > > +Inc.
> > > +
> > > +devtools_dir=$(dirname $(readlink -f $0)) .
> > > +$devtools_dir/load-devel-config
> > > +
> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > > +for dir in $abi_ref_build_dir/*; do
> > > + if [ "$dir" = "$abi_ref_build_dir" ]; then
> > > + exit 1
> > > + fi
> > > + if [ -d $dir/dump ]; then
> > > + echo "Skipping $dir"
> > > + continue
> > > + fi
> > > + if [ -d $dir/install ]; then
> > > + libdir=$dir/install
> > > + else
> > > + libdir=$dir
> > > + fi
> > > + echo "Dumping libraries from $libdir in $dir/dump"
> > > + $devtools_dir/gen-abi-dump.sh $libdir $dir/dump done
> > > diff --git a/devtools/test-build.sh b/devtools/test-build.sh index
> > > 52305fbb8..8cb5b56fb 100755
> > > --- a/devtools/test-build.sh
> > > +++ b/devtools/test-build.sh
> > > @@ -30,7 +30,8 @@ default_path=$PATH
> > > # - LIBSSO_SNOW3G_PATH
> > > # - LIBSSO_KASUMI_PATH
> > > # - LIBSSO_ZUC_PATH
> > > -. $(dirname $(readlink -f $0))/load-devel-config
> > > +devtools_dir=$(dirname $(readlink -f $0)) .
> > > +$devtools_dir/load-devel-config
> > >
> > > print_usage () {
> > > echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2]
> > > ...]]"
> > > @@ -64,6 +65,7 @@ print_help () {
> > > [ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
> > >
> > > J=$DPDK_MAKE_JOBS
> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > > builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> > > short=false
> > > unset verbose
> > > @@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT
> > #
> > > notify result on exit trap on_exit EXIT
> > >
> > > -cd $(dirname $(readlink -f $0))/..
> > > +cd $devtools_dir/..
> > >
> > > reset_env ()
> > > {
> > > @@ -233,7 +235,7 @@ for conf in $configs ; do
> > > # reload config with DPDK_TARGET set
> > > DPDK_TARGET=$target
> > > reset_env
> > > - . $(dirname $(readlink -f $0))/load-devel-config
> > > + . $devtools_dir/load-devel-config
> > >
> > > options=$(echo $conf | sed 's,[^~+]*,,')
> > > dir=$builds_dir/$conf
> > > @@ -246,6 +248,11 @@ for conf in $configs ; do
> > > export RTE_TARGET=$target
> > > rm -rf $dir/install
> > > ${MAKE} install O=$dir DESTDIR=$dir/install prefix=
> > > + if [ -d $abi_ref_build_dir/$conf/dump ]; then
> > > + echo "================== Check ABI $conf"
> > > + $devtools_dir/check-abi-dump.sh $dir/install \
> > > + $abi_ref_build_dir/$conf/dump
> > > + fi
> > > echo "================== Build examples for $conf"
> > > export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
> > > ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
> > > diff --git a/devtools/test-meson-builds.sh
> > > b/devtools/test-meson-builds.sh index 688567714..aaefa38a2 100755
> > > --- a/devtools/test-meson-builds.sh
> > > +++ b/devtools/test-meson-builds.sh
> > > @@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
> > >
> > > MESON=${MESON:-meson}
> > > use_shared="--default-library=shared"
> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
> > > builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> > >
> > > if command -v gmake >/dev/null 2>&1 ; then @@ -88,6 +89,11 @@ build
> > > () # <directory> <target compiler> <meson options>
> > > echo "$ninja_cmd -C $builddir"
> > > $ninja_cmd -C $builddir
> > > fi
> > > +
> > > + if [ -d $abi_ref_build_dir/$1/dump ]; then
> > > + $srcdir/devtools/check-abi-dump.sh $builddir
> > > + $abi_ref_build_dir/$1/dump
> > > + fi
> > > }
> > >
> > > if [ "$1" = "-vv" ] ; then
> > > diff --git a/doc/guides/contributing/patches.rst
> > > b/doc/guides/contributing/patches.rst
> > > index 0686450e4..de3dff145 100644
> > > --- a/doc/guides/contributing/patches.rst
> > > +++ b/doc/guides/contributing/patches.rst
> > > @@ -513,6 +513,31 @@ 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.
> > >
> > >
> > > +Checking ABI compatibility
> > > +--------------------------
> > > +
> > > +The first thing is to build reference binaries for the latest
> > release
> > > +your patches are built on top of.
> > > +
> > > +Either you are in a git tree and an easy way to identify this is to
> > run::
> > > +
> > > + git checkout $(git describe --abbrev=0)
> > > +
> > > +Or you use a tarball and you extract the sources in a director of
> > > +your
> > > choice.
> > > +
> > > +Next is building those sources, refer to the previous paragraph.
> > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > +occur in this directory.
> > > +
> > > +Finally, the ABI dump files are generated with the
> > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > +builds in the current sub directory ``reference``. But you can set
> > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > location.
> > > +
> > > +Once done, you can check your current binaries ABI with this
> > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > +
> > > +
> > > Sending Patches
> > > ---------------
> > >
> > > --
> > > 2.23.0
> >
> > I still very much dislike forcing the user to generate his own
> > reference version to compare the ABI against. These should be archived
> > and the user should just be able to pull them down via git or http or
> > otherwise. Two reasons for this:
> >
> > 1. Less error prone, since there is no chance of the user having an
> > incorrect build for whatever reason.
> >
> > 2. Less effort for the user than asking them to do extra builds. The
> > more steps the user has to follow, the less likely they are to attempt
> > the process.
> >
> > Regards,
> > /Bruce
> >
>
> +1 ... 100% agree with this.
>
> Many people won't know or understand what the reference is,
> or why they to generate it.
Many people won't run the test at all and will rely on the automatic CI tests.
Thomas Monjalon <thomas@monjalon.net> writes:
> 20/12/2019 17:20, Kinsella, Ray:
>> > -----Original Message-----
>> > From: Richardson, Bruce <bruce.richardson@intel.com>
>> > Sent: Friday 20 December 2019 15:32
>> > To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
>> > Cc: thomas@monjalon.net; Laatz, Kevin <kevin.laatz@intel.com>;
>> > aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
>> > <maicolgabriel@hotmail.com>; Mcnamara, John <john.mcnamara@intel.com>;
>> > Kovacevic, Marko <marko.kovacevic@intel.com>; Kinsella, Ray
>> > <ray.kinsella@intel.com>
>> > Subject: RE: [PATCH] add ABI checks
>> >
>> >
>> >
>> > > -----Original Message-----
>> > > From: David Marchand <david.marchand@redhat.com>
>> > > Sent: Friday, December 20, 2019 3:21 PM
>> > > To: dev@dpdk.org
>> > > Cc: thomas@monjalon.net; Richardson, Bruce
>> > > <bruce.richardson@intel.com>; Laatz, Kevin <kevin.laatz@intel.com>;
>> > > aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
>> > > <maicolgabriel@hotmail.com>; Mcnamara, John
>> > <john.mcnamara@intel.com>;
>> > > Kovacevic, Marko <marko.kovacevic@intel.com>
>> > > Subject: [PATCH] add ABI checks
>> > >
>> > > Starting from Kevin and Bruce idea of using libabigail, here is an
>> > > alternate approach to implement ABI checks.
>> > >
>> > > By default, those checks are disabled and enabling them requires a
>> > > manual step that generates the ABI dumps on a reference version for a
>> > > set of configurations.
>> > >
>> > > Those checks are enabled in the CI by default for the default meson
>> > > options on x86 and aarch64 so that proposed patches are validated.
>> > > A cache of the ABI is stored in travis jobs.
>> > > Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
>> > > breaking the ABI in a future release.
>> > >
>> > > For advanced developers and maintainers, the contributing guide
>> > > details the higher level scripts that are quite close to the existing
>> > > devtools scripts.
>> > >
>> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
>> > > ---
>> > > .ci/linux-build.sh | 43 +++++++++++++++++++++++++++
>> > > .travis.yml | 20 +++++++++++--
>> > > devtools/check-abi-dump.sh | 46
>> > +++++++++++++++++++++++++++++
>> > > devtools/check-abi-reference.sh | 27 +++++++++++++++++
>> > > devtools/dpdk.abignore | 2 ++
>> > > devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
>> > > devtools/gen-abi-reference.sh | 24 +++++++++++++++
>> > > devtools/test-build.sh | 13 ++++++--
>> > > devtools/test-meson-builds.sh | 6 ++++
>> > > doc/guides/contributing/patches.rst | 25 ++++++++++++++++
>> > > 10 files changed, 230 insertions(+), 5 deletions(-) create mode
>> > > 100755 devtools/check-abi-dump.sh create mode 100755
>> > > devtools/check-abi- reference.sh create mode 100644
>> > > devtools/dpdk.abignore create mode
>> > > 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
>> > > reference.sh
>> > >
>> > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
>> > > ccc3a7ccd..345dba264 100755
>> > > --- a/.ci/linux-build.sh
>> > > +++ b/.ci/linux-build.sh
>> > > @@ -30,8 +30,51 @@ fi
>> > >
>> > > OPTS="$OPTS --default-library=$DEF_LIB"
>> > > meson build --werror -Dexamples=all $OPTS
>> > > +
>> > > +if [ "$ABI_CHECKS" = "1" ]; then
>> > > + git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
>> > > + git fetch --tags ref ${REF_GIT_BRANCH:-master}
>> > > +
>> > > + head=$(git describe --all)
>> > > + tag=$(git describe --abbrev=0)
>> > > +
>> > > + if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
>> > > + rm -rf reference
>> > > + fi
>> > > +
>> > > + if [ ! -d reference ]; then
>> > > + gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
>> > > + cp -a devtools/gen-abi-dump.sh $gen_abi_dump
>> > > +
>> > > + git checkout -qf $tag
>> > > + ninja -C build
>> > > + $gen_abi_dump build reference
>> > > +
>> > > + if [ "$AARCH64" != "1" ]; then
>> > > + mkdir -p reference/app
>> > > + cp -a build/app/dpdk-testpmd reference/app/
>> > > +
>> > > + export
>> > LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
>> > > + devtools/test-null.sh reference/app/dpdk-testpmd
>> > > + unset LD_LIBRARY_PATH
>> > > + fi
>> > > + echo $tag > reference/VERSION
>> > > +
>> > > + git checkout -qf $head
>> > > + fi
>> > > +fi
>> > > +
>> > > ninja -C build
>> > >
>> > > +if [ "$ABI_CHECKS" = "1" ]; then
>> > > + devtools/check-abi-dump.sh build reference
>> > ${ABI_CHECKS_WARN_ONLY:-}
>> > > + if [ "$AARCH64" != "1" ]; then
>> > > + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
>> > > + devtools/test-null.sh reference/app/dpdk-testpmd
>> > > + unset LD_LIBRARY_PATH
>> > > + fi
>> > > +fi
>> > > +
>> > > if [ "$AARCH64" != "1" ]; then
>> > > devtools/test-null.sh
>> > > fi
>> > > diff --git a/.travis.yml b/.travis.yml index 8f90d06f2..bbb060fa2
>> > > 100644
>> > > --- a/.travis.yml
>> > > +++ b/.travis.yml
>> > > @@ -1,5 +1,8 @@
>> > > language: c
>> > > -cache: ccache
>> > > +cache:
>> > > + ccache: true
>> > > + directories:
>> > > + - reference
>> > > compiler:
>> > > - gcc
>> > > - clang
>> > > @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
>> > >
>> > > extra_packages: &extra_packages
>> > > - *required_packages
>> > > - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
>> > > + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4,
>> > > + abigail-tools]
>> > >
>> > > build_32b_packages: &build_32b_packages
>> > > - *required_packages
>> > > @@ -59,6 +62,13 @@ matrix:
>> > > apt:
>> > > packages:
>> > > - *aarch64_packages
>> > > + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
>> > > + compiler: gcc
>> > > + addons:
>> > > + apt:
>> > > + packages:
>> > > + - *aarch64_packages
>> > > + - *extra_packages
>> > > - env: DEF_LIB="static" EXTRA_PACKAGES=1
>> > > compiler: gcc
>> > > addons:
>> > > @@ -72,6 +82,12 @@ matrix:
>> > > packages:
>> > > - *extra_packages
>> > > - *doc_packages
>> > > + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
>> > > + compiler: gcc
>> > > + addons:
>> > > + apt:
>> > > + packages:
>> > > + - *extra_packages
>> > > - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
>> > EXTRA_PACKAGES=1
>> > > compiler: gcc
>> > > addons:
>> > > diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
>> > > new file mode 100755 index 000000000..f48a2ae7e
>> > > --- /dev/null
>> > > +++ b/devtools/check-abi-dump.sh
>> > > @@ -0,0 +1,46 @@
>> > > +#!/bin/sh -e
>> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>> > Hat,
>> > > +Inc.
>> > > +
>> > > +if [ $# != 2 ] && [ $# != 3 ]; then
>> > > + echo "Usage: $0 builddir dumpdir [warnonly]"
>> > > + exit 1
>> > > +fi
>> > > +
>> > > +builddir=$1
>> > > +dumpdir=$2
>> > > +warnonly=${3:-}
>> > > +if [ ! -d $builddir ]; then
>> > > + echo "Error: build directory '$builddir' does not exist."
>> > > + exit 1
>> > > +fi
>> > > +if [ ! -d $dumpdir ]; then
>> > > + echo "Error: dump directory '$dumpdir' does not exist."
>> > > + exit 1
>> > > +fi
>> > > +
>> > > +ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
>> > > +for dump in $(find $dumpdir -name "*.dump"); do
>> > > + libname=$(basename $dump)
>> > > + libname=${libname%.dump}
>> > > + result=
>> > > + for f in $(find $builddir -name "$libname.so.*"); do
>> > > + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
>> > > + continue
>> > > + fi
>> > > + result=found
>> > > +
>> > > + if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
>> > > + if [ -z "$warnonly" ]; then
>> > > + echo "Error: ABI issue reported for $dump, $f"
>> > > + exit 1
>> > > + fi
>> > > + echo "Warning: ABI issue reported for $dump, $f"
>> > > + fi
>> > > + break
>> > > + done
>> > > + if [ "$result" != "found" ]; then
>> > > + echo "Error: can't find a library for dump file $dump"
>> > > + exit 1
>> > > + fi
>> > > +done
>> > > diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-
>> > > reference.sh new file mode 100755 index 000000000..7addb094e
>> > > --- /dev/null
>> > > +++ b/devtools/check-abi-reference.sh
>> > > @@ -0,0 +1,27 @@
>> > > +#!/bin/sh -e
>> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>> > Hat,
>> > > +Inc.
>> > > +
>> > > +devtools_dir=$(dirname $(readlink -f $0)) .
>> > > +$devtools_dir/load-devel-config
>> > > +
>> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>> > > +builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>> > > +
>> > > +for dir in $abi_ref_build_dir/*; do
>> > > + if [ "$dir" = "$abi_ref_build_dir" ]; then
>> > > + exit 1
>> > > + fi
>> > > + if [ ! -d $dir/dump ]; then
>> > > + echo "Skipping $dir"
>> > > + continue
>> > > + fi
>> > > + target=$(basename $dir)
>> > > + if [ -d $builds_dir/$target/install ]; then
>> > > + libdir=$builds_dir/$target/install
>> > > + else
>> > > + libdir=$builds_dir/$target
>> > > + fi
>> > > + echo "Checking ABI between $libdir and $dir/dump"
>> > > + $devtools_dir/check-abi-dump.sh $libdir $dir/dump done
>> > > diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore new file
>> > > mode
>> > > 100644 index 000000000..b866b7f26
>> > > --- /dev/null
>> > > +++ b/devtools/dpdk.abignore
>> > > @@ -0,0 +1,2 @@
>> > > +[suppress_function]
>> > > + symbol_version = EXPERIMENTAL
>> > > diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh new
>> > > file mode 100755 index 000000000..4e38d751f
>> > > --- /dev/null
>> > > +++ b/devtools/gen-abi-dump.sh
>> > > @@ -0,0 +1,29 @@
>> > > +#!/bin/sh -e
>> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>> > Hat,
>> > > +Inc.
>> > > +
>> > > +if [ $# != 2 ]; then
>> > > + echo "Usage: $0 builddir dumpdir"
>> > > + exit 1
>> > > +fi
>> > > +
>> > > +builddir=$1
>> > > +dumpdir=$2
>> > > +if [ ! -d $builddir ]; then
>> > > + echo "Error: build directory '$builddir' does not exist."
>> > > + exit 1
>> > > +fi
>> > > +if [ -d $dumpdir ]; then
>> > > + echo "Error: dump directory '$dumpdir' already exists."
>> > > + exit 1
>> > > +fi
>> > > +
>> > > +mkdir -p $dumpdir
>> > > +for f in $(find $builddir -name "*.so.*"); do
>> > > + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
>> > > + continue
>> > > + fi
>> > > +
>> > > + libname=$(basename $f)
>> > > + abidw --out-file $dumpdir/${libname%.so.*}.dump $f done
>> > > diff --git a/devtools/gen-abi-reference.sh
>> > > b/devtools/gen-abi-reference.sh new file mode 100755 index
>> > > 000000000..f41d7fadc
>> > > --- /dev/null
>> > > +++ b/devtools/gen-abi-reference.sh
>> > > @@ -0,0 +1,24 @@
>> > > +#!/bin/sh -e
>> > > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>> > Hat,
>> > > +Inc.
>> > > +
>> > > +devtools_dir=$(dirname $(readlink -f $0)) .
>> > > +$devtools_dir/load-devel-config
>> > > +
>> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>> > > +for dir in $abi_ref_build_dir/*; do
>> > > + if [ "$dir" = "$abi_ref_build_dir" ]; then
>> > > + exit 1
>> > > + fi
>> > > + if [ -d $dir/dump ]; then
>> > > + echo "Skipping $dir"
>> > > + continue
>> > > + fi
>> > > + if [ -d $dir/install ]; then
>> > > + libdir=$dir/install
>> > > + else
>> > > + libdir=$dir
>> > > + fi
>> > > + echo "Dumping libraries from $libdir in $dir/dump"
>> > > + $devtools_dir/gen-abi-dump.sh $libdir $dir/dump done
>> > > diff --git a/devtools/test-build.sh b/devtools/test-build.sh index
>> > > 52305fbb8..8cb5b56fb 100755
>> > > --- a/devtools/test-build.sh
>> > > +++ b/devtools/test-build.sh
>> > > @@ -30,7 +30,8 @@ default_path=$PATH
>> > > # - LIBSSO_SNOW3G_PATH
>> > > # - LIBSSO_KASUMI_PATH
>> > > # - LIBSSO_ZUC_PATH
>> > > -. $(dirname $(readlink -f $0))/load-devel-config
>> > > +devtools_dir=$(dirname $(readlink -f $0)) .
>> > > +$devtools_dir/load-devel-config
>> > >
>> > > print_usage () {
>> > > echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2]
>> > > ...]]"
>> > > @@ -64,6 +65,7 @@ print_help () {
>> > > [ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
>> > >
>> > > J=$DPDK_MAKE_JOBS
>> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>> > > builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>> > > short=false
>> > > unset verbose
>> > > @@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT
>> > #
>> > > notify result on exit trap on_exit EXIT
>> > >
>> > > -cd $(dirname $(readlink -f $0))/..
>> > > +cd $devtools_dir/..
>> > >
>> > > reset_env ()
>> > > {
>> > > @@ -233,7 +235,7 @@ for conf in $configs ; do
>> > > # reload config with DPDK_TARGET set
>> > > DPDK_TARGET=$target
>> > > reset_env
>> > > - . $(dirname $(readlink -f $0))/load-devel-config
>> > > + . $devtools_dir/load-devel-config
>> > >
>> > > options=$(echo $conf | sed 's,[^~+]*,,')
>> > > dir=$builds_dir/$conf
>> > > @@ -246,6 +248,11 @@ for conf in $configs ; do
>> > > export RTE_TARGET=$target
>> > > rm -rf $dir/install
>> > > ${MAKE} install O=$dir DESTDIR=$dir/install prefix=
>> > > + if [ -d $abi_ref_build_dir/$conf/dump ]; then
>> > > + echo "================== Check ABI $conf"
>> > > + $devtools_dir/check-abi-dump.sh $dir/install \
>> > > + $abi_ref_build_dir/$conf/dump
>> > > + fi
>> > > echo "================== Build examples for $conf"
>> > > export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
>> > > ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
>> > > diff --git a/devtools/test-meson-builds.sh
>> > > b/devtools/test-meson-builds.sh index 688567714..aaefa38a2 100755
>> > > --- a/devtools/test-meson-builds.sh
>> > > +++ b/devtools/test-meson-builds.sh
>> > > @@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
>> > >
>> > > MESON=${MESON:-meson}
>> > > use_shared="--default-library=shared"
>> > > +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>> > > builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>> > >
>> > > if command -v gmake >/dev/null 2>&1 ; then @@ -88,6 +89,11 @@ build
>> > > () # <directory> <target compiler> <meson options>
>> > > echo "$ninja_cmd -C $builddir"
>> > > $ninja_cmd -C $builddir
>> > > fi
>> > > +
>> > > + if [ -d $abi_ref_build_dir/$1/dump ]; then
>> > > + $srcdir/devtools/check-abi-dump.sh $builddir
>> > > + $abi_ref_build_dir/$1/dump
>> > > + fi
>> > > }
>> > >
>> > > if [ "$1" = "-vv" ] ; then
>> > > diff --git a/doc/guides/contributing/patches.rst
>> > > b/doc/guides/contributing/patches.rst
>> > > index 0686450e4..de3dff145 100644
>> > > --- a/doc/guides/contributing/patches.rst
>> > > +++ b/doc/guides/contributing/patches.rst
>> > > @@ -513,6 +513,31 @@ 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.
>> > >
>> > >
>> > > +Checking ABI compatibility
>> > > +--------------------------
>> > > +
>> > > +The first thing is to build reference binaries for the latest
>> > release
>> > > +your patches are built on top of.
>> > > +
>> > > +Either you are in a git tree and an easy way to identify this is to
>> > run::
>> > > +
>> > > + git checkout $(git describe --abbrev=0)
>> > > +
>> > > +Or you use a tarball and you extract the sources in a director of
>> > > +your
>> > > choice.
>> > > +
>> > > +Next is building those sources, refer to the previous paragraph.
>> > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
>> > > +occur in this directory.
>> > > +
>> > > +Finally, the ABI dump files are generated with the
>> > > +``devtools/gen-abi-reference.sh`` script. This script will look for
>> > > +builds in the current sub directory ``reference``. But you can set
>> > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
>> > location.
>> > > +
>> > > +Once done, you can check your current binaries ABI with this
>> > > +reference with the ``devtools/check-abi-reference.sh`` script.
>> > > +
>> > > +
>> > > Sending Patches
>> > > ---------------
>> > >
>> > > --
>> > > 2.23.0
>> >
>> > I still very much dislike forcing the user to generate his own
>> > reference version to compare the ABI against. These should be archived
>> > and the user should just be able to pull them down via git or http or
>> > otherwise. Two reasons for this:
>> >
>> > 1. Less error prone, since there is no chance of the user having an
>> > incorrect build for whatever reason.
It's more error prone I think. There are issues with archived versions
of ABI dumps, or bad content delivery networks. Working from the git
repository is *guaranteed* to generate a correct ABI dump. Anything
else allows your CDN / other to break.
>> > 2. Less effort for the user than asking them to do extra builds. The
>> > more steps the user has to follow, the less likely they are to attempt
>> > the process.
It's important to remember that (as far as I'm aware) the vast majority
of DPDK users don't make many code changes or even compile it. And
those who do typically don't even run the unit tests. I think it's good
to document a process for people to follow, but such process shouldn't
assume that developers can't run basic git commands or execute scripts.
>> > Regards,
>> > /Bruce
>> >
>>
>> +1 ... 100% agree with this.
>>
>> Many people won't know or understand what the reference is,
>> or why they to generate it.
>
> Many people won't run the test at all and will rely on the automatic CI tests.
+1 to this. It's good for the robot's travis build to validate the ABI for
patches, since it will alert us anyway.
20/12/2019 17:20, Kinsella, Ray:
> From: Richardson, Bruce <bruce.richardson@intel.com>
> > From: David Marchand <david.marchand@redhat.com>
> > > +Checking ABI compatibility
> > > +--------------------------
> > > +
> > > +The first thing is to build reference binaries for the latest
> > release
> > > +your patches are built on top of.
> > > +
> > > +Either you are in a git tree and an easy way to identify this is to
> > run::
> > > +
> > > + git checkout $(git describe --abbrev=0)
> > > +
> > > +Or you use a tarball and you extract the sources in a director of
> > > +your
> > > choice.
> > > +
> > > +Next is building those sources, refer to the previous paragraph.
> > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > +occur in this directory.
> > > +
> > > +Finally, the ABI dump files are generated with the
> > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > +builds in the current sub directory ``reference``. But you can set
> > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > location.
> > > +
> > > +Once done, you can check your current binaries ABI with this
> > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > >
> >
> > I still very much dislike forcing the user to generate his own
> > reference version to compare the ABI against. These should be archived
> > and the user should just be able to pull them down via git or http or
> > otherwise. Two reasons for this:
> >
> > 1. Less error prone, since there is no chance of the user having an
> > incorrect build for whatever reason.
> >
> > 2. Less effort for the user than asking them to do extra builds. The
> > more steps the user has to follow, the less likely they are to attempt
> > the process.
>
> +1 ... 100% agree with this.
>
> Many people won't know or understand what the reference is,
> or why they to generate it.
I don't want to generate and save the reference in git for each arch.
We can make reference build more automatic with a command like this:
git clone --branch v19.11 . $DPDK_BUILD_TEST_DIR/abiref-19.11
Also I don't like mixing build and check steps.
I believe the compilation should be simple and right to the point.
This approach, from David, does not prevent from saving the dumps later
if we really feel a strong need.
That's why I suggest going with this patch.
On Wed, Jan 15, 2020 at 12:19:30AM +0100, Thomas Monjalon wrote:
> 20/12/2019 17:20, Kinsella, Ray:
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > From: David Marchand <david.marchand@redhat.com>
> > > > +Checking ABI compatibility
> > > > +--------------------------
> > > > +
> > > > +The first thing is to build reference binaries for the latest
> > > release
> > > > +your patches are built on top of.
> > > > +
> > > > +Either you are in a git tree and an easy way to identify this is to
> > > run::
> > > > +
> > > > + git checkout $(git describe --abbrev=0)
> > > > +
> > > > +Or you use a tarball and you extract the sources in a director of
> > > > +your
> > > > choice.
> > > > +
> > > > +Next is building those sources, refer to the previous paragraph.
> > > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > > +occur in this directory.
> > > > +
> > > > +Finally, the ABI dump files are generated with the
> > > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > > +builds in the current sub directory ``reference``. But you can set
> > > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > > location.
> > > > +
> > > > +Once done, you can check your current binaries ABI with this
> > > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > >
> > >
> > > I still very much dislike forcing the user to generate his own
> > > reference version to compare the ABI against. These should be archived
> > > and the user should just be able to pull them down via git or http or
> > > otherwise. Two reasons for this:
> > >
> > > 1. Less error prone, since there is no chance of the user having an
> > > incorrect build for whatever reason.
> > >
> > > 2. Less effort for the user than asking them to do extra builds. The
> > > more steps the user has to follow, the less likely they are to attempt
> > > the process.
> >
> > +1 ... 100% agree with this.
> >
> > Many people won't know or understand what the reference is,
> > or why they to generate it.
>
> I don't want to generate and save the reference in git for each arch.
>
Can I ask what your reluctance is? Is it related to not wanting to have to save
all this information that is otherwise not used for building purposes?
If so I might suggest saving the dumps in a separate git tree and pulling them
in as a git submodule when the check is performed
I really like the idea of caching the results so everyone is working from a
known ABI baseline.
Neil
> We can make reference build more automatic with a command like this:
> git clone --branch v19.11 . $DPDK_BUILD_TEST_DIR/abiref-19.11
>
> Also I don't like mixing build and check steps.
> I believe the compilation should be simple and right to the point.
>
> This approach, from David, does not prevent from saving the dumps later
> if we really feel a strong need.
>
> That's why I suggest going with this patch.
>
>
>
15/01/2020 12:33, Neil Horman:
> On Wed, Jan 15, 2020 at 12:19:30AM +0100, Thomas Monjalon wrote:
> > 20/12/2019 17:20, Kinsella, Ray:
> > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > From: David Marchand <david.marchand@redhat.com>
> > > > > +Checking ABI compatibility
> > > > > +--------------------------
> > > > > +
> > > > > +The first thing is to build reference binaries for the latest
> > > > release
> > > > > +your patches are built on top of.
> > > > > +
> > > > > +Either you are in a git tree and an easy way to identify this is to
> > > > run::
> > > > > +
> > > > > + git checkout $(git describe --abbrev=0)
> > > > > +
> > > > > +Or you use a tarball and you extract the sources in a director of
> > > > > +your
> > > > > choice.
> > > > > +
> > > > > +Next is building those sources, refer to the previous paragraph.
> > > > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > > > +occur in this directory.
> > > > > +
> > > > > +Finally, the ABI dump files are generated with the
> > > > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > > > +builds in the current sub directory ``reference``. But you can set
> > > > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > > > location.
> > > > > +
> > > > > +Once done, you can check your current binaries ABI with this
> > > > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > > >
> > > >
> > > > I still very much dislike forcing the user to generate his own
> > > > reference version to compare the ABI against. These should be archived
> > > > and the user should just be able to pull them down via git or http or
> > > > otherwise. Two reasons for this:
> > > >
> > > > 1. Less error prone, since there is no chance of the user having an
> > > > incorrect build for whatever reason.
> > > >
> > > > 2. Less effort for the user than asking them to do extra builds. The
> > > > more steps the user has to follow, the less likely they are to attempt
> > > > the process.
> > >
> > > +1 ... 100% agree with this.
> > >
> > > Many people won't know or understand what the reference is,
> > > or why they to generate it.
> >
> > I don't want to generate and save the reference in git for each arch.
> >
> Can I ask what your reluctance is? Is it related to not wanting to have to save
> all this information that is otherwise not used for building purposes?
Yes I prefer keeping only the sources in the repository.
And these dumps are big.
And last but not the least, there is no ready-to-use environment to build
and dump all libs for all archs.
> If so I might suggest saving the dumps in a separate git tree and pulling them
> in as a git submodule when the check is performed
>
> I really like the idea of caching the results so everyone is working from a
> known ABI baseline.
You don't trust the result of the build made from tagged sources?
> > We can make reference build more automatic with a command like this:
> > git clone --branch v19.11 . $DPDK_BUILD_TEST_DIR/abiref-19.11
> >
> > Also I don't like mixing build and check steps.
> > I believe the compilation should be simple and right to the point.
> >
> > This approach, from David, does not prevent from saving the dumps later
> > if we really feel a strong need.
> >
> > That's why I suggest going with this patch.
On 06-Jan-20 1:17 PM, Aaron Conole wrote:
> Thomas Monjalon <thomas@monjalon.net> writes:
>
>> 20/12/2019 17:20, Kinsella, Ray:
>>>> -----Original Message-----
>>>> From: Richardson, Bruce <bruce.richardson@intel.com>
>>>> Sent: Friday 20 December 2019 15:32
>>>> To: David Marchand <david.marchand@redhat.com>; dev@dpdk.org
>>>> Cc: thomas@monjalon.net; Laatz, Kevin <kevin.laatz@intel.com>;
>>>> aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
>>>> <maicolgabriel@hotmail.com>; Mcnamara, John <john.mcnamara@intel.com>;
>>>> Kovacevic, Marko <marko.kovacevic@intel.com>; Kinsella, Ray
>>>> <ray.kinsella@intel.com>
>>>> Subject: RE: [PATCH] add ABI checks
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: David Marchand <david.marchand@redhat.com>
>>>>> Sent: Friday, December 20, 2019 3:21 PM
>>>>> To: dev@dpdk.org
>>>>> Cc: thomas@monjalon.net; Richardson, Bruce
>>>>> <bruce.richardson@intel.com>; Laatz, Kevin <kevin.laatz@intel.com>;
>>>>> aconole@redhat.com; nhorman@tuxdriver.com; Michael Santana
>>>>> <maicolgabriel@hotmail.com>; Mcnamara, John
>>>> <john.mcnamara@intel.com>;
>>>>> Kovacevic, Marko <marko.kovacevic@intel.com>
>>>>> Subject: [PATCH] add ABI checks
>>>>>
>>>>> Starting from Kevin and Bruce idea of using libabigail, here is an
>>>>> alternate approach to implement ABI checks.
>>>>>
>>>>> By default, those checks are disabled and enabling them requires a
>>>>> manual step that generates the ABI dumps on a reference version for a
>>>>> set of configurations.
>>>>>
>>>>> Those checks are enabled in the CI by default for the default meson
>>>>> options on x86 and aarch64 so that proposed patches are validated.
>>>>> A cache of the ABI is stored in travis jobs.
>>>>> Checks can be only informational by setting ABI_CHECKS_WARN_ONLY when
>>>>> breaking the ABI in a future release.
>>>>>
>>>>> For advanced developers and maintainers, the contributing guide
>>>>> details the higher level scripts that are quite close to the existing
>>>>> devtools scripts.
>>>>>
>>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>>> ---
>>>>> .ci/linux-build.sh | 43 +++++++++++++++++++++++++++
>>>>> .travis.yml | 20 +++++++++++--
>>>>> devtools/check-abi-dump.sh | 46
>>>> +++++++++++++++++++++++++++++
>>>>> devtools/check-abi-reference.sh | 27 +++++++++++++++++
>>>>> devtools/dpdk.abignore | 2 ++
>>>>> devtools/gen-abi-dump.sh | 29 ++++++++++++++++++
>>>>> devtools/gen-abi-reference.sh | 24 +++++++++++++++
>>>>> devtools/test-build.sh | 13 ++++++--
>>>>> devtools/test-meson-builds.sh | 6 ++++
>>>>> doc/guides/contributing/patches.rst | 25 ++++++++++++++++
>>>>> 10 files changed, 230 insertions(+), 5 deletions(-) create mode
>>>>> 100755 devtools/check-abi-dump.sh create mode 100755
>>>>> devtools/check-abi- reference.sh create mode 100644
>>>>> devtools/dpdk.abignore create mode
>>>>> 100755 devtools/gen-abi-dump.sh create mode 100755 devtools/gen-abi-
>>>>> reference.sh
>>>>>
>>>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh index
>>>>> ccc3a7ccd..345dba264 100755
>>>>> --- a/.ci/linux-build.sh
>>>>> +++ b/.ci/linux-build.sh
>>>>> @@ -30,8 +30,51 @@ fi
>>>>>
>>>>> OPTS="$OPTS --default-library=$DEF_LIB"
>>>>> meson build --werror -Dexamples=all $OPTS
>>>>> +
>>>>> +if [ "$ABI_CHECKS" = "1" ]; then
>>>>> + git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
>>>>> + git fetch --tags ref ${REF_GIT_BRANCH:-master}
>>>>> +
>>>>> + head=$(git describe --all)
>>>>> + tag=$(git describe --abbrev=0)
>>>>> +
>>>>> + if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
>>>>> + rm -rf reference
>>>>> + fi
>>>>> +
>>>>> + if [ ! -d reference ]; then
>>>>> + gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
>>>>> + cp -a devtools/gen-abi-dump.sh $gen_abi_dump
>>>>> +
>>>>> + git checkout -qf $tag
>>>>> + ninja -C build
>>>>> + $gen_abi_dump build reference
>>>>> +
>>>>> + if [ "$AARCH64" != "1" ]; then
>>>>> + mkdir -p reference/app
>>>>> + cp -a build/app/dpdk-testpmd reference/app/
>>>>> +
>>>>> + export
>>>> LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
>>>>> + devtools/test-null.sh reference/app/dpdk-testpmd
>>>>> + unset LD_LIBRARY_PATH
>>>>> + fi
>>>>> + echo $tag > reference/VERSION
>>>>> +
>>>>> + git checkout -qf $head
>>>>> + fi
>>>>> +fi
>>>>> +
>>>>> ninja -C build
>>>>>
>>>>> +if [ "$ABI_CHECKS" = "1" ]; then
>>>>> + devtools/check-abi-dump.sh build reference
>>>> ${ABI_CHECKS_WARN_ONLY:-}
>>>>> + if [ "$AARCH64" != "1" ]; then
>>>>> + export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
>>>>> + devtools/test-null.sh reference/app/dpdk-testpmd
>>>>> + unset LD_LIBRARY_PATH
>>>>> + fi
>>>>> +fi
>>>>> +
>>>>> if [ "$AARCH64" != "1" ]; then
>>>>> devtools/test-null.sh
>>>>> fi
>>>>> diff --git a/.travis.yml b/.travis.yml index 8f90d06f2..bbb060fa2
>>>>> 100644
>>>>> --- a/.travis.yml
>>>>> +++ b/.travis.yml
>>>>> @@ -1,5 +1,8 @@
>>>>> language: c
>>>>> -cache: ccache
>>>>> +cache:
>>>>> + ccache: true
>>>>> + directories:
>>>>> + - reference
>>>>> compiler:
>>>>> - gcc
>>>>> - clang
>>>>> @@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
>>>>>
>>>>> extra_packages: &extra_packages
>>>>> - *required_packages
>>>>> - - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
>>>>> + - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4,
>>>>> + abigail-tools]
>>>>>
>>>>> build_32b_packages: &build_32b_packages
>>>>> - *required_packages
>>>>> @@ -59,6 +62,13 @@ matrix:
>>>>> apt:
>>>>> packages:
>>>>> - *aarch64_packages
>>>>> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
>>>>> + compiler: gcc
>>>>> + addons:
>>>>> + apt:
>>>>> + packages:
>>>>> + - *aarch64_packages
>>>>> + - *extra_packages
>>>>> - env: DEF_LIB="static" EXTRA_PACKAGES=1
>>>>> compiler: gcc
>>>>> addons:
>>>>> @@ -72,6 +82,12 @@ matrix:
>>>>> packages:
>>>>> - *extra_packages
>>>>> - *doc_packages
>>>>> + - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
>>>>> + compiler: gcc
>>>>> + addons:
>>>>> + apt:
>>>>> + packages:
>>>>> + - *extra_packages
>>>>> - env: DEF_LIB="static" OPTS="-Denable_kmods=false"
>>>> EXTRA_PACKAGES=1
>>>>> compiler: gcc
>>>>> addons:
>>>>> diff --git a/devtools/check-abi-dump.sh b/devtools/check-abi-dump.sh
>>>>> new file mode 100755 index 000000000..f48a2ae7e
>>>>> --- /dev/null
>>>>> +++ b/devtools/check-abi-dump.sh
>>>>> @@ -0,0 +1,46 @@
>>>>> +#!/bin/sh -e
>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>>>> Hat,
>>>>> +Inc.
>>>>> +
>>>>> +if [ $# != 2 ] && [ $# != 3 ]; then
>>>>> + echo "Usage: $0 builddir dumpdir [warnonly]"
>>>>> + exit 1
>>>>> +fi
>>>>> +
>>>>> +builddir=$1
>>>>> +dumpdir=$2
>>>>> +warnonly=${3:-}
>>>>> +if [ ! -d $builddir ]; then
>>>>> + echo "Error: build directory '$builddir' does not exist."
>>>>> + exit 1
>>>>> +fi
>>>>> +if [ ! -d $dumpdir ]; then
>>>>> + echo "Error: dump directory '$dumpdir' does not exist."
>>>>> + exit 1
>>>>> +fi
>>>>> +
>>>>> +ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
>>>>> +for dump in $(find $dumpdir -name "*.dump"); do
>>>>> + libname=$(basename $dump)
>>>>> + libname=${libname%.dump}
>>>>> + result=
>>>>> + for f in $(find $builddir -name "$libname.so.*"); do
>>>>> + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
>>>>> + continue
>>>>> + fi
>>>>> + result=found
>>>>> +
>>>>> + if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
>>>>> + if [ -z "$warnonly" ]; then
>>>>> + echo "Error: ABI issue reported for $dump, $f"
>>>>> + exit 1
>>>>> + fi
>>>>> + echo "Warning: ABI issue reported for $dump, $f"
>>>>> + fi
>>>>> + break
>>>>> + done
>>>>> + if [ "$result" != "found" ]; then
>>>>> + echo "Error: can't find a library for dump file $dump"
>>>>> + exit 1
>>>>> + fi
>>>>> +done
>>>>> diff --git a/devtools/check-abi-reference.sh b/devtools/check-abi-
>>>>> reference.sh new file mode 100755 index 000000000..7addb094e
>>>>> --- /dev/null
>>>>> +++ b/devtools/check-abi-reference.sh
>>>>> @@ -0,0 +1,27 @@
>>>>> +#!/bin/sh -e
>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>>>> Hat,
>>>>> +Inc.
>>>>> +
>>>>> +devtools_dir=$(dirname $(readlink -f $0)) .
>>>>> +$devtools_dir/load-devel-config
>>>>> +
>>>>> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>>>>> +builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>>>>> +
>>>>> +for dir in $abi_ref_build_dir/*; do
>>>>> + if [ "$dir" = "$abi_ref_build_dir" ]; then
>>>>> + exit 1
>>>>> + fi
>>>>> + if [ ! -d $dir/dump ]; then
>>>>> + echo "Skipping $dir"
>>>>> + continue
>>>>> + fi
>>>>> + target=$(basename $dir)
>>>>> + if [ -d $builds_dir/$target/install ]; then
>>>>> + libdir=$builds_dir/$target/install
>>>>> + else
>>>>> + libdir=$builds_dir/$target
>>>>> + fi
>>>>> + echo "Checking ABI between $libdir and $dir/dump"
>>>>> + $devtools_dir/check-abi-dump.sh $libdir $dir/dump done
>>>>> diff --git a/devtools/dpdk.abignore b/devtools/dpdk.abignore new file
>>>>> mode
>>>>> 100644 index 000000000..b866b7f26
>>>>> --- /dev/null
>>>>> +++ b/devtools/dpdk.abignore
>>>>> @@ -0,0 +1,2 @@
>>>>> +[suppress_function]
>>>>> + symbol_version = EXPERIMENTAL
>>>>> diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh new
>>>>> file mode 100755 index 000000000..4e38d751f
>>>>> --- /dev/null
>>>>> +++ b/devtools/gen-abi-dump.sh
>>>>> @@ -0,0 +1,29 @@
>>>>> +#!/bin/sh -e
>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>>>> Hat,
>>>>> +Inc.
>>>>> +
>>>>> +if [ $# != 2 ]; then
>>>>> + echo "Usage: $0 builddir dumpdir"
>>>>> + exit 1
>>>>> +fi
>>>>> +
>>>>> +builddir=$1
>>>>> +dumpdir=$2
>>>>> +if [ ! -d $builddir ]; then
>>>>> + echo "Error: build directory '$builddir' does not exist."
>>>>> + exit 1
>>>>> +fi
>>>>> +if [ -d $dumpdir ]; then
>>>>> + echo "Error: dump directory '$dumpdir' already exists."
>>>>> + exit 1
>>>>> +fi
>>>>> +
>>>>> +mkdir -p $dumpdir
>>>>> +for f in $(find $builddir -name "*.so.*"); do
>>>>> + if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
>>>>> + continue
>>>>> + fi
>>>>> +
>>>>> + libname=$(basename $f)
>>>>> + abidw --out-file $dumpdir/${libname%.so.*}.dump $f done
>>>>> diff --git a/devtools/gen-abi-reference.sh
>>>>> b/devtools/gen-abi-reference.sh new file mode 100755 index
>>>>> 000000000..f41d7fadc
>>>>> --- /dev/null
>>>>> +++ b/devtools/gen-abi-reference.sh
>>>>> @@ -0,0 +1,24 @@
>>>>> +#!/bin/sh -e
>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2019 Red
>>>> Hat,
>>>>> +Inc.
>>>>> +
>>>>> +devtools_dir=$(dirname $(readlink -f $0)) .
>>>>> +$devtools_dir/load-devel-config
>>>>> +
>>>>> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>>>>> +for dir in $abi_ref_build_dir/*; do
>>>>> + if [ "$dir" = "$abi_ref_build_dir" ]; then
>>>>> + exit 1
>>>>> + fi
>>>>> + if [ -d $dir/dump ]; then
>>>>> + echo "Skipping $dir"
>>>>> + continue
>>>>> + fi
>>>>> + if [ -d $dir/install ]; then
>>>>> + libdir=$dir/install
>>>>> + else
>>>>> + libdir=$dir
>>>>> + fi
>>>>> + echo "Dumping libraries from $libdir in $dir/dump"
>>>>> + $devtools_dir/gen-abi-dump.sh $libdir $dir/dump done
>>>>> diff --git a/devtools/test-build.sh b/devtools/test-build.sh index
>>>>> 52305fbb8..8cb5b56fb 100755
>>>>> --- a/devtools/test-build.sh
>>>>> +++ b/devtools/test-build.sh
>>>>> @@ -30,7 +30,8 @@ default_path=$PATH
>>>>> # - LIBSSO_SNOW3G_PATH
>>>>> # - LIBSSO_KASUMI_PATH
>>>>> # - LIBSSO_ZUC_PATH
>>>>> -. $(dirname $(readlink -f $0))/load-devel-config
>>>>> +devtools_dir=$(dirname $(readlink -f $0)) .
>>>>> +$devtools_dir/load-devel-config
>>>>>
>>>>> print_usage () {
>>>>> echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2]
>>>>> ...]]"
>>>>> @@ -64,6 +65,7 @@ print_help () {
>>>>> [ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
>>>>>
>>>>> J=$DPDK_MAKE_JOBS
>>>>> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>>>>> builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>>>>> short=false
>>>>> unset verbose
>>>>> @@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT
>>>> #
>>>>> notify result on exit trap on_exit EXIT
>>>>>
>>>>> -cd $(dirname $(readlink -f $0))/..
>>>>> +cd $devtools_dir/..
>>>>>
>>>>> reset_env ()
>>>>> {
>>>>> @@ -233,7 +235,7 @@ for conf in $configs ; do
>>>>> # reload config with DPDK_TARGET set
>>>>> DPDK_TARGET=$target
>>>>> reset_env
>>>>> - . $(dirname $(readlink -f $0))/load-devel-config
>>>>> + . $devtools_dir/load-devel-config
>>>>>
>>>>> options=$(echo $conf | sed 's,[^~+]*,,')
>>>>> dir=$builds_dir/$conf
>>>>> @@ -246,6 +248,11 @@ for conf in $configs ; do
>>>>> export RTE_TARGET=$target
>>>>> rm -rf $dir/install
>>>>> ${MAKE} install O=$dir DESTDIR=$dir/install prefix=
>>>>> + if [ -d $abi_ref_build_dir/$conf/dump ]; then
>>>>> + echo "================== Check ABI $conf"
>>>>> + $devtools_dir/check-abi-dump.sh $dir/install \
>>>>> + $abi_ref_build_dir/$conf/dump
>>>>> + fi
>>>>> echo "================== Build examples for $conf"
>>>>> export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
>>>>> ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
>>>>> diff --git a/devtools/test-meson-builds.sh
>>>>> b/devtools/test-meson-builds.sh index 688567714..aaefa38a2 100755
>>>>> --- a/devtools/test-meson-builds.sh
>>>>> +++ b/devtools/test-meson-builds.sh
>>>>> @@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
>>>>>
>>>>> MESON=${MESON:-meson}
>>>>> use_shared="--default-library=shared"
>>>>> +abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
>>>>> builds_dir=${DPDK_BUILD_TEST_DIR:-.}
>>>>>
>>>>> if command -v gmake >/dev/null 2>&1 ; then @@ -88,6 +89,11 @@ build
>>>>> () # <directory> <target compiler> <meson options>
>>>>> echo "$ninja_cmd -C $builddir"
>>>>> $ninja_cmd -C $builddir
>>>>> fi
>>>>> +
>>>>> + if [ -d $abi_ref_build_dir/$1/dump ]; then
>>>>> + $srcdir/devtools/check-abi-dump.sh $builddir
>>>>> + $abi_ref_build_dir/$1/dump
>>>>> + fi
>>>>> }
>>>>>
>>>>> if [ "$1" = "-vv" ] ; then
>>>>> diff --git a/doc/guides/contributing/patches.rst
>>>>> b/doc/guides/contributing/patches.rst
>>>>> index 0686450e4..de3dff145 100644
>>>>> --- a/doc/guides/contributing/patches.rst
>>>>> +++ b/doc/guides/contributing/patches.rst
>>>>> @@ -513,6 +513,31 @@ 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.
>>>>>
>>>>>
>>>>> +Checking ABI compatibility
>>>>> +--------------------------
>>>>> +
>>>>> +The first thing is to build reference binaries for the latest
>>>> release
>>>>> +your patches are built on top of.
>>>>> +
>>>>> +Either you are in a git tree and an easy way to identify this is to
>>>> run::
>>>>> +
>>>>> + git checkout $(git describe --abbrev=0)
>>>>> +
>>>>> +Or you use a tarball and you extract the sources in a director of
>>>>> +your
>>>>> choice.
>>>>> +
>>>>> +Next is building those sources, refer to the previous paragraph.
>>>>> +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
>>>>> +occur in this directory.
>>>>> +
>>>>> +Finally, the ABI dump files are generated with the
>>>>> +``devtools/gen-abi-reference.sh`` script. This script will look for
>>>>> +builds in the current sub directory ``reference``. But you can set
>>>>> +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
>>>> location.
>>>>> +
>>>>> +Once done, you can check your current binaries ABI with this
>>>>> +reference with the ``devtools/check-abi-reference.sh`` script.
>>>>> +
>>>>> +
>>>>> Sending Patches
>>>>> ---------------
>>>>>
>>>>> --
>>>>> 2.23.0
>>>>
>>>> I still very much dislike forcing the user to generate his own
>>>> reference version to compare the ABI against. These should be archived
>>>> and the user should just be able to pull them down via git or http or
>>>> otherwise. Two reasons for this:
>>>>
>>>> 1. Less error prone, since there is no chance of the user having an
>>>> incorrect build for whatever reason.
>
> It's more error prone I think. There are issues with archived versions
> of ABI dumps, or bad content delivery networks. Working from the git
> repository is *guaranteed* to generate a correct ABI dump. Anything
> else allows your CDN / other to break.
I can't speak for everyone else, but normally when i'm working on a DPDK
codebase, there's a whole bunch of stuff that isn't built by default,
and i normally never bother. So, while working from git repo is
guaranteed to generate a correct ABI dump, it is *not* guaranteed to
cover everything, unless the developer takes steps to do otherwise.
As far as i understand, the plan is to make ABI checks part of the
automation - so the effort of *having* these reference ABI dumps is
already spent anyway, might as well make them accessible to whoever
develops DPDK too. And if CDN/connectivity fails - well, so be it,
either fall back to manual process, or let the automation handle it.
>
>>>> 2. Less effort for the user than asking them to do extra builds. The
>>>> more steps the user has to follow, the less likely they are to attempt
>>>> the process.
>
> It's important to remember that (as far as I'm aware) the vast majority
> of DPDK users don't make many code changes or even compile it. And
> those who do typically don't even run the unit tests. I think it's good
> to document a process for people to follow, but such process shouldn't
> assume that developers can't run basic git commands or execute scripts.
>
>>>> Regards,
>>>> /Bruce
>>>>
>>>
>>> +1 ... 100% agree with this.
>>>
>>> Many people won't know or understand what the reference is,
>>> or why they to generate it.
>>
>> Many people won't run the test at all and will rely on the automatic CI tests.
>
> +1 to this. It's good for the robot's travis build to validate the ABI for
> patches, since it will alert us anyway.
>
>
On Wed, Jan 15, 2020 at 01:38:17PM +0100, Thomas Monjalon wrote:
> 15/01/2020 12:33, Neil Horman:
> > On Wed, Jan 15, 2020 at 12:19:30AM +0100, Thomas Monjalon wrote:
> > > 20/12/2019 17:20, Kinsella, Ray:
> > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > > +Checking ABI compatibility
> > > > > > +--------------------------
> > > > > > +
> > > > > > +The first thing is to build reference binaries for the latest
> > > > > release
> > > > > > +your patches are built on top of.
> > > > > > +
> > > > > > +Either you are in a git tree and an easy way to identify this is to
> > > > > run::
> > > > > > +
> > > > > > + git checkout $(git describe --abbrev=0)
> > > > > > +
> > > > > > +Or you use a tarball and you extract the sources in a director of
> > > > > > +your
> > > > > > choice.
> > > > > > +
> > > > > > +Next is building those sources, refer to the previous paragraph.
> > > > > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > > > > +occur in this directory.
> > > > > > +
> > > > > > +Finally, the ABI dump files are generated with the
> > > > > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > > > > +builds in the current sub directory ``reference``. But you can set
> > > > > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > > > > location.
> > > > > > +
> > > > > > +Once done, you can check your current binaries ABI with this
> > > > > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > > > >
> > > > >
> > > > > I still very much dislike forcing the user to generate his own
> > > > > reference version to compare the ABI against. These should be archived
> > > > > and the user should just be able to pull them down via git or http or
> > > > > otherwise. Two reasons for this:
> > > > >
> > > > > 1. Less error prone, since there is no chance of the user having an
> > > > > incorrect build for whatever reason.
> > > > >
> > > > > 2. Less effort for the user than asking them to do extra builds. The
> > > > > more steps the user has to follow, the less likely they are to attempt
> > > > > the process.
> > > >
> > > > +1 ... 100% agree with this.
> > > >
> > > > Many people won't know or understand what the reference is,
> > > > or why they to generate it.
> > >
> > > I don't want to generate and save the reference in git for each arch.
> > >
> > Can I ask what your reluctance is? Is it related to not wanting to have to save
> > all this information that is otherwise not used for building purposes?
>
> Yes I prefer keeping only the sources in the repository.
> And these dumps are big.
> And last but not the least, there is no ready-to-use environment to build
> and dump all libs for all archs.
>
> > If so I might suggest saving the dumps in a separate git tree and pulling them
> > in as a git submodule when the check is performed
> >
> > I really like the idea of caching the results so everyone is working from a
> > known ABI baseline.
>
> You don't trust the result of the build made from tagged sources?
>
I trust the result from the tools, sure, its trusting that people will take the
extra time to build a version from a prior tag that I'm less sure of.
Consistent use in my mind is predicated on ease and timeliness of use.
I get not wanting to store large dumps in the source tree, but storage is cheap,
and I don't see the issue with storing the xml dump in a separate git tree to be
referenced through a git submodule that gets pulled in when the check is run.
Neil
> > > We can make reference build more automatic with a command like this:
> > > git clone --branch v19.11 . $DPDK_BUILD_TEST_DIR/abiref-19.11
> > >
> > > Also I don't like mixing build and check steps.
> > > I believe the compilation should be simple and right to the point.
> > >
> > > This approach, from David, does not prevent from saving the dumps later
> > > if we really feel a strong need.
> > >
> > > That's why I suggest going with this patch.
>
>
>
>
16/01/2020 12:52, Neil Horman:
> On Wed, Jan 15, 2020 at 01:38:17PM +0100, Thomas Monjalon wrote:
> > 15/01/2020 12:33, Neil Horman:
> > > On Wed, Jan 15, 2020 at 12:19:30AM +0100, Thomas Monjalon wrote:
> > > > 20/12/2019 17:20, Kinsella, Ray:
> > > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > > > +Checking ABI compatibility
> > > > > > > +--------------------------
> > > > > > > +
> > > > > > > +The first thing is to build reference binaries for the latest
> > > > > > release
> > > > > > > +your patches are built on top of.
> > > > > > > +
> > > > > > > +Either you are in a git tree and an easy way to identify this is to
> > > > > > run::
> > > > > > > +
> > > > > > > + git checkout $(git describe --abbrev=0)
> > > > > > > +
> > > > > > > +Or you use a tarball and you extract the sources in a director of
> > > > > > > +your
> > > > > > > choice.
> > > > > > > +
> > > > > > > +Next is building those sources, refer to the previous paragraph.
> > > > > > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > > > > > +occur in this directory.
> > > > > > > +
> > > > > > > +Finally, the ABI dump files are generated with the
> > > > > > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > > > > > +builds in the current sub directory ``reference``. But you can set
> > > > > > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > > > > > location.
> > > > > > > +
> > > > > > > +Once done, you can check your current binaries ABI with this
> > > > > > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > > > > >
> > > > > >
> > > > > > I still very much dislike forcing the user to generate his own
> > > > > > reference version to compare the ABI against. These should be archived
> > > > > > and the user should just be able to pull them down via git or http or
> > > > > > otherwise. Two reasons for this:
> > > > > >
> > > > > > 1. Less error prone, since there is no chance of the user having an
> > > > > > incorrect build for whatever reason.
> > > > > >
> > > > > > 2. Less effort for the user than asking them to do extra builds. The
> > > > > > more steps the user has to follow, the less likely they are to attempt
> > > > > > the process.
> > > > >
> > > > > +1 ... 100% agree with this.
> > > > >
> > > > > Many people won't know or understand what the reference is,
> > > > > or why they to generate it.
> > > >
> > > > I don't want to generate and save the reference in git for each arch.
> > > >
> > > Can I ask what your reluctance is? Is it related to not wanting to have to save
> > > all this information that is otherwise not used for building purposes?
> >
> > Yes I prefer keeping only the sources in the repository.
> > And these dumps are big.
> > And last but not the least, there is no ready-to-use environment to build
> > and dump all libs for all archs.
> >
> > > If so I might suggest saving the dumps in a separate git tree and pulling them
> > > in as a git submodule when the check is performed
> > >
> > > I really like the idea of caching the results so everyone is working from a
> > > known ABI baseline.
> >
> > You don't trust the result of the build made from tagged sources?
> >
> I trust the result from the tools, sure, its trusting that people will take the
> extra time to build a version from a prior tag that I'm less sure of.
> Consistent use in my mind is predicated on ease and timeliness of use.
>
> I get not wanting to store large dumps in the source tree, but storage is cheap,
> and I don't see the issue with storing the xml dump in a separate git tree to be
> referenced through a git submodule that gets pulled in when the check is run.
Yes this is an option.
My fear is that this reference database will not be complete
if we don't build it for all libraries/drivers on all archs,
managing setups and dependencies.
On Thu, Jan 16, 2020 at 03:20:48PM +0100, Thomas Monjalon wrote:
> 16/01/2020 12:52, Neil Horman:
> > On Wed, Jan 15, 2020 at 01:38:17PM +0100, Thomas Monjalon wrote:
> > > 15/01/2020 12:33, Neil Horman:
> > > > On Wed, Jan 15, 2020 at 12:19:30AM +0100, Thomas Monjalon wrote:
> > > > > 20/12/2019 17:20, Kinsella, Ray:
> > > > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > > > > +Checking ABI compatibility
> > > > > > > > +--------------------------
> > > > > > > > +
> > > > > > > > +The first thing is to build reference binaries for the latest
> > > > > > > release
> > > > > > > > +your patches are built on top of.
> > > > > > > > +
> > > > > > > > +Either you are in a git tree and an easy way to identify this is to
> > > > > > > run::
> > > > > > > > +
> > > > > > > > + git checkout $(git describe --abbrev=0)
> > > > > > > > +
> > > > > > > > +Or you use a tarball and you extract the sources in a director of
> > > > > > > > +your
> > > > > > > > choice.
> > > > > > > > +
> > > > > > > > +Next is building those sources, refer to the previous paragraph.
> > > > > > > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > > > > > > +occur in this directory.
> > > > > > > > +
> > > > > > > > +Finally, the ABI dump files are generated with the
> > > > > > > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > > > > > > +builds in the current sub directory ``reference``. But you can set
> > > > > > > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > > > > > > location.
> > > > > > > > +
> > > > > > > > +Once done, you can check your current binaries ABI with this
> > > > > > > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > > > > > >
> > > > > > >
> > > > > > > I still very much dislike forcing the user to generate his own
> > > > > > > reference version to compare the ABI against. These should be archived
> > > > > > > and the user should just be able to pull them down via git or http or
> > > > > > > otherwise. Two reasons for this:
> > > > > > >
> > > > > > > 1. Less error prone, since there is no chance of the user having an
> > > > > > > incorrect build for whatever reason.
> > > > > > >
> > > > > > > 2. Less effort for the user than asking them to do extra builds. The
> > > > > > > more steps the user has to follow, the less likely they are to attempt
> > > > > > > the process.
> > > > > >
> > > > > > +1 ... 100% agree with this.
> > > > > >
> > > > > > Many people won't know or understand what the reference is,
> > > > > > or why they to generate it.
> > > > >
> > > > > I don't want to generate and save the reference in git for each arch.
> > > > >
> > > > Can I ask what your reluctance is? Is it related to not wanting to have to save
> > > > all this information that is otherwise not used for building purposes?
> > >
> > > Yes I prefer keeping only the sources in the repository.
> > > And these dumps are big.
> > > And last but not the least, there is no ready-to-use environment to build
> > > and dump all libs for all archs.
> > >
> > > > If so I might suggest saving the dumps in a separate git tree and pulling them
> > > > in as a git submodule when the check is performed
> > > >
> > > > I really like the idea of caching the results so everyone is working from a
> > > > known ABI baseline.
> > >
> > > You don't trust the result of the build made from tagged sources?
> > >
> > I trust the result from the tools, sure, its trusting that people will take the
> > extra time to build a version from a prior tag that I'm less sure of.
> > Consistent use in my mind is predicated on ease and timeliness of use.
> >
> > I get not wanting to store large dumps in the source tree, but storage is cheap,
> > and I don't see the issue with storing the xml dump in a separate git tree to be
> > referenced through a git submodule that gets pulled in when the check is run.
>
> Yes this is an option.
> My fear is that this reference database will not be complete
> if we don't build it for all libraries/drivers on all archs,
> managing setups and dependencies.
>
I can understand that, but I would have assumed that we would have done all
config build for all supported arches as part of the CI for a release, from
which we could archive the results. Is that not the case?
Neil
>
>
16/01/2020 19:49, Neil Horman:
> On Thu, Jan 16, 2020 at 03:20:48PM +0100, Thomas Monjalon wrote:
> > 16/01/2020 12:52, Neil Horman:
> > > On Wed, Jan 15, 2020 at 01:38:17PM +0100, Thomas Monjalon wrote:
> > > > 15/01/2020 12:33, Neil Horman:
> > > > > On Wed, Jan 15, 2020 at 12:19:30AM +0100, Thomas Monjalon wrote:
> > > > > > 20/12/2019 17:20, Kinsella, Ray:
> > > > > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > > > > > +Checking ABI compatibility
> > > > > > > > > +--------------------------
> > > > > > > > > +
> > > > > > > > > +The first thing is to build reference binaries for the latest
> > > > > > > > release
> > > > > > > > > +your patches are built on top of.
> > > > > > > > > +
> > > > > > > > > +Either you are in a git tree and an easy way to identify this is to
> > > > > > > > run::
> > > > > > > > > +
> > > > > > > > > + git checkout $(git describe --abbrev=0)
> > > > > > > > > +
> > > > > > > > > +Or you use a tarball and you extract the sources in a director of
> > > > > > > > > +your
> > > > > > > > > choice.
> > > > > > > > > +
> > > > > > > > > +Next is building those sources, refer to the previous paragraph.
> > > > > > > > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > > > > > > > +occur in this directory.
> > > > > > > > > +
> > > > > > > > > +Finally, the ABI dump files are generated with the
> > > > > > > > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > > > > > > > +builds in the current sub directory ``reference``. But you can set
> > > > > > > > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > > > > > > > location.
> > > > > > > > > +
> > > > > > > > > +Once done, you can check your current binaries ABI with this
> > > > > > > > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I still very much dislike forcing the user to generate his own
> > > > > > > > reference version to compare the ABI against. These should be archived
> > > > > > > > and the user should just be able to pull them down via git or http or
> > > > > > > > otherwise. Two reasons for this:
> > > > > > > >
> > > > > > > > 1. Less error prone, since there is no chance of the user having an
> > > > > > > > incorrect build for whatever reason.
> > > > > > > >
> > > > > > > > 2. Less effort for the user than asking them to do extra builds. The
> > > > > > > > more steps the user has to follow, the less likely they are to attempt
> > > > > > > > the process.
> > > > > > >
> > > > > > > +1 ... 100% agree with this.
> > > > > > >
> > > > > > > Many people won't know or understand what the reference is,
> > > > > > > or why they to generate it.
> > > > > >
> > > > > > I don't want to generate and save the reference in git for each arch.
> > > > > >
> > > > > Can I ask what your reluctance is? Is it related to not wanting to have to save
> > > > > all this information that is otherwise not used for building purposes?
> > > >
> > > > Yes I prefer keeping only the sources in the repository.
> > > > And these dumps are big.
> > > > And last but not the least, there is no ready-to-use environment to build
> > > > and dump all libs for all archs.
> > > >
> > > > > If so I might suggest saving the dumps in a separate git tree and pulling them
> > > > > in as a git submodule when the check is performed
> > > > >
> > > > > I really like the idea of caching the results so everyone is working from a
> > > > > known ABI baseline.
> > > >
> > > > You don't trust the result of the build made from tagged sources?
> > > >
> > > I trust the result from the tools, sure, its trusting that people will take the
> > > extra time to build a version from a prior tag that I'm less sure of.
> > > Consistent use in my mind is predicated on ease and timeliness of use.
> > >
> > > I get not wanting to store large dumps in the source tree, but storage is cheap,
> > > and I don't see the issue with storing the xml dump in a separate git tree to be
> > > referenced through a git submodule that gets pulled in when the check is run.
> >
> > Yes this is an option.
> > My fear is that this reference database will not be complete
> > if we don't build it for all libraries/drivers on all archs,
> > managing setups and dependencies.
> >
> I can understand that, but I would have assumed that we would have done all
> config build for all supported arches as part of the CI for a release, from
> which we could archive the results. Is that not the case?
No, we don't have a fully complete setup covering all dependencies and archs.
On Thu, Jan 16, 2020 at 09:01:52PM +0100, Thomas Monjalon wrote:
> 16/01/2020 19:49, Neil Horman:
> > On Thu, Jan 16, 2020 at 03:20:48PM +0100, Thomas Monjalon wrote:
> > > 16/01/2020 12:52, Neil Horman:
> > > > On Wed, Jan 15, 2020 at 01:38:17PM +0100, Thomas Monjalon wrote:
> > > > > 15/01/2020 12:33, Neil Horman:
> > > > > > On Wed, Jan 15, 2020 at 12:19:30AM +0100, Thomas Monjalon wrote:
> > > > > > > 20/12/2019 17:20, Kinsella, Ray:
> > > > > > > > From: Richardson, Bruce <bruce.richardson@intel.com>
> > > > > > > > > From: David Marchand <david.marchand@redhat.com>
> > > > > > > > > > +Checking ABI compatibility
> > > > > > > > > > +--------------------------
> > > > > > > > > > +
> > > > > > > > > > +The first thing is to build reference binaries for the latest
> > > > > > > > > release
> > > > > > > > > > +your patches are built on top of.
> > > > > > > > > > +
> > > > > > > > > > +Either you are in a git tree and an easy way to identify this is to
> > > > > > > > > run::
> > > > > > > > > > +
> > > > > > > > > > + git checkout $(git describe --abbrev=0)
> > > > > > > > > > +
> > > > > > > > > > +Or you use a tarball and you extract the sources in a director of
> > > > > > > > > > +your
> > > > > > > > > > choice.
> > > > > > > > > > +
> > > > > > > > > > +Next is building those sources, refer to the previous paragraph.
> > > > > > > > > > +You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds
> > > > > > > > > > +occur in this directory.
> > > > > > > > > > +
> > > > > > > > > > +Finally, the ABI dump files are generated with the
> > > > > > > > > > +``devtools/gen-abi-reference.sh`` script. This script will look for
> > > > > > > > > > +builds in the current sub directory ``reference``. But you can set
> > > > > > > > > > +the environment variable ``DPDK_ABI_REF_BUILD_DIR`` to a different
> > > > > > > > > location.
> > > > > > > > > > +
> > > > > > > > > > +Once done, you can check your current binaries ABI with this
> > > > > > > > > > +reference with the ``devtools/check-abi-reference.sh`` script.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I still very much dislike forcing the user to generate his own
> > > > > > > > > reference version to compare the ABI against. These should be archived
> > > > > > > > > and the user should just be able to pull them down via git or http or
> > > > > > > > > otherwise. Two reasons for this:
> > > > > > > > >
> > > > > > > > > 1. Less error prone, since there is no chance of the user having an
> > > > > > > > > incorrect build for whatever reason.
> > > > > > > > >
> > > > > > > > > 2. Less effort for the user than asking them to do extra builds. The
> > > > > > > > > more steps the user has to follow, the less likely they are to attempt
> > > > > > > > > the process.
> > > > > > > >
> > > > > > > > +1 ... 100% agree with this.
> > > > > > > >
> > > > > > > > Many people won't know or understand what the reference is,
> > > > > > > > or why they to generate it.
> > > > > > >
> > > > > > > I don't want to generate and save the reference in git for each arch.
> > > > > > >
> > > > > > Can I ask what your reluctance is? Is it related to not wanting to have to save
> > > > > > all this information that is otherwise not used for building purposes?
> > > > >
> > > > > Yes I prefer keeping only the sources in the repository.
> > > > > And these dumps are big.
> > > > > And last but not the least, there is no ready-to-use environment to build
> > > > > and dump all libs for all archs.
> > > > >
> > > > > > If so I might suggest saving the dumps in a separate git tree and pulling them
> > > > > > in as a git submodule when the check is performed
> > > > > >
> > > > > > I really like the idea of caching the results so everyone is working from a
> > > > > > known ABI baseline.
> > > > >
> > > > > You don't trust the result of the build made from tagged sources?
> > > > >
> > > > I trust the result from the tools, sure, its trusting that people will take the
> > > > extra time to build a version from a prior tag that I'm less sure of.
> > > > Consistent use in my mind is predicated on ease and timeliness of use.
> > > >
> > > > I get not wanting to store large dumps in the source tree, but storage is cheap,
> > > > and I don't see the issue with storing the xml dump in a separate git tree to be
> > > > referenced through a git submodule that gets pulled in when the check is run.
> > >
> > > Yes this is an option.
> > > My fear is that this reference database will not be complete
> > > if we don't build it for all libraries/drivers on all archs,
> > > managing setups and dependencies.
> > >
> > I can understand that, but I would have assumed that we would have done all
> > config build for all supported arches as part of the CI for a release, from
> > which we could archive the results. Is that not the case?
>
> No, we don't have a fully complete setup covering all dependencies and archs.
>
Hmmm....I wonder if its worth orchestrating the build system to use a git
submodule storing whatever our CI system can produce and using it as a cache,
and falling back to a local build if the appropriate arch isn't found? That
might offer some incentive to alternate arch maintainers to contribute hardware
or compute time to populating it in pursuit of better build times?
Neil
>
>
On Fri, Jan 17, 2020 at 8:02 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> Hmmm....I wonder if its worth orchestrating the build system to use a git
> submodule storing whatever our CI system can produce and using it as a cache,
> and falling back to a local build if the appropriate arch isn't found? That
> might offer some incentive to alternate arch maintainers to contribute hardware
> or compute time to populating it in pursuit of better build times?
Thomas, Neil,
I would prefer we consider this in a second step.
Just want to give a little update on this patch.
Building dpdk with debuginfo seems necessary, if we want to catch
internal structures changes.
I suppose Bruce patch can be merged already:
https://patchwork.dpdk.org/patch/63400/
libabigail in Ubuntu 16.04 (Xenial) is a bit old (version 1.0-rc3) and
exhibits a false positive on the linkage name of the default symbol if
we do function versioning: we caught this with Olivier when he asked
for help on his patch for mempool.
This issue disappears with version 1.2 that is in Ubuntu 18.04 (Bionic).
After running the check in my environment (fc30 with libabigail 1.5),
I caught issues with abidiff when comparing a .so and an abi dump.
I reported this upstream, and Dodji fixed this right away.
https://sourceware.org/bugzilla/show_bug.cgi?id=25409
The fix should be part of version 1.7.
This could be ignored if we only do the checks with binaries generated with gcc.
I just got a false positive on an ABI change for the "custom element
size" ring series, but on a type that is not user visible (thanks
Honnappa).
So this needs more work: I hope the --headers-dirX options will help
but it means we must store the public headers with the ABI dumps.
I can prepare a v2 and have the CI only warns about changes without
reporting a failure.
@@ -30,8 +30,51 @@ fi
OPTS="$OPTS --default-library=$DEF_LIB"
meson build --werror -Dexamples=all $OPTS
+
+if [ "$ABI_CHECKS" = "1" ]; then
+ git remote add ref ${REF_GIT_REPO:-https://dpdk.org/git/dpdk}
+ git fetch --tags ref ${REF_GIT_BRANCH:-master}
+
+ head=$(git describe --all)
+ tag=$(git describe --abbrev=0)
+
+ if [ "$(cat reference/VERSION 2>/dev/null)" != "$tag" ]; then
+ rm -rf reference
+ fi
+
+ if [ ! -d reference ]; then
+ gen_abi_dump=$(mktemp -t gen-abi-dump-XXX.sh)
+ cp -a devtools/gen-abi-dump.sh $gen_abi_dump
+
+ git checkout -qf $tag
+ ninja -C build
+ $gen_abi_dump build reference
+
+ if [ "$AARCH64" != "1" ]; then
+ mkdir -p reference/app
+ cp -a build/app/dpdk-testpmd reference/app/
+
+ export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+ devtools/test-null.sh reference/app/dpdk-testpmd
+ unset LD_LIBRARY_PATH
+ fi
+ echo $tag > reference/VERSION
+
+ git checkout -qf $head
+ fi
+fi
+
ninja -C build
+if [ "$ABI_CHECKS" = "1" ]; then
+ devtools/check-abi-dump.sh build reference ${ABI_CHECKS_WARN_ONLY:-}
+ if [ "$AARCH64" != "1" ]; then
+ export LD_LIBRARY_PATH=$(pwd)/build/lib:$(pwd)/build/drivers
+ devtools/test-null.sh reference/app/dpdk-testpmd
+ unset LD_LIBRARY_PATH
+ fi
+fi
+
if [ "$AARCH64" != "1" ]; then
devtools/test-null.sh
fi
@@ -1,5 +1,8 @@
language: c
-cache: ccache
+cache:
+ ccache: true
+ directories:
+ - reference
compiler:
- gcc
- clang
@@ -21,7 +24,7 @@ aarch64_packages: &aarch64_packages
extra_packages: &extra_packages
- *required_packages
- - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4]
+ - [libbsd-dev, libpcap-dev, libcrypto++-dev, libjansson4, abigail-tools]
build_32b_packages: &build_32b_packages
- *required_packages
@@ -59,6 +62,13 @@ matrix:
apt:
packages:
- *aarch64_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1 AARCH64=1
+ compiler: gcc
+ addons:
+ apt:
+ packages:
+ - *aarch64_packages
+ - *extra_packages
- env: DEF_LIB="static" EXTRA_PACKAGES=1
compiler: gcc
addons:
@@ -72,6 +82,12 @@ matrix:
packages:
- *extra_packages
- *doc_packages
+ - env: DEF_LIB="shared" EXTRA_PACKAGES=1 ABI_CHECKS=1
+ compiler: gcc
+ addons:
+ apt:
+ packages:
+ - *extra_packages
- env: DEF_LIB="static" OPTS="-Denable_kmods=false" EXTRA_PACKAGES=1
compiler: gcc
addons:
new file mode 100755
@@ -0,0 +1,46 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+if [ $# != 2 ] && [ $# != 3 ]; then
+ echo "Usage: $0 builddir dumpdir [warnonly]"
+ exit 1
+fi
+
+builddir=$1
+dumpdir=$2
+warnonly=${3:-}
+if [ ! -d $builddir ]; then
+ echo "Error: build directory '$builddir' does not exist."
+ exit 1
+fi
+if [ ! -d $dumpdir ]; then
+ echo "Error: dump directory '$dumpdir' does not exist."
+ exit 1
+fi
+
+ABIDIFF_OPTIONS="--suppr $(dirname $0)/dpdk.abignore"
+for dump in $(find $dumpdir -name "*.dump"); do
+ libname=$(basename $dump)
+ libname=${libname%.dump}
+ result=
+ for f in $(find $builddir -name "$libname.so.*"); do
+ if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
+ continue
+ fi
+ result=found
+
+ if ! abidiff $ABIDIFF_OPTIONS $dump $f; then
+ if [ -z "$warnonly" ]; then
+ echo "Error: ABI issue reported for $dump, $f"
+ exit 1
+ fi
+ echo "Warning: ABI issue reported for $dump, $f"
+ fi
+ break
+ done
+ if [ "$result" != "found" ]; then
+ echo "Error: can't find a library for dump file $dump"
+ exit 1
+ fi
+done
new file mode 100755
@@ -0,0 +1,27 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+devtools_dir=$(dirname $(readlink -f $0))
+. $devtools_dir/load-devel-config
+
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
+builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+
+for dir in $abi_ref_build_dir/*; do
+ if [ "$dir" = "$abi_ref_build_dir" ]; then
+ exit 1
+ fi
+ if [ ! -d $dir/dump ]; then
+ echo "Skipping $dir"
+ continue
+ fi
+ target=$(basename $dir)
+ if [ -d $builds_dir/$target/install ]; then
+ libdir=$builds_dir/$target/install
+ else
+ libdir=$builds_dir/$target
+ fi
+ echo "Checking ABI between $libdir and $dir/dump"
+ $devtools_dir/check-abi-dump.sh $libdir $dir/dump
+done
new file mode 100644
@@ -0,0 +1,2 @@
+[suppress_function]
+ symbol_version = EXPERIMENTAL
new file mode 100755
@@ -0,0 +1,29 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+if [ $# != 2 ]; then
+ echo "Usage: $0 builddir dumpdir"
+ exit 1
+fi
+
+builddir=$1
+dumpdir=$2
+if [ ! -d $builddir ]; then
+ echo "Error: build directory '$builddir' does not exist."
+ exit 1
+fi
+if [ -d $dumpdir ]; then
+ echo "Error: dump directory '$dumpdir' already exists."
+ exit 1
+fi
+
+mkdir -p $dumpdir
+for f in $(find $builddir -name "*.so.*"); do
+ if test -L $f || [ "$f" != "${f%%.symbols}" ]; then
+ continue
+ fi
+
+ libname=$(basename $f)
+ abidw --out-file $dumpdir/${libname%.so.*}.dump $f
+done
new file mode 100755
@@ -0,0 +1,24 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2019 Red Hat, Inc.
+
+devtools_dir=$(dirname $(readlink -f $0))
+. $devtools_dir/load-devel-config
+
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
+for dir in $abi_ref_build_dir/*; do
+ if [ "$dir" = "$abi_ref_build_dir" ]; then
+ exit 1
+ fi
+ if [ -d $dir/dump ]; then
+ echo "Skipping $dir"
+ continue
+ fi
+ if [ -d $dir/install ]; then
+ libdir=$dir/install
+ else
+ libdir=$dir
+ fi
+ echo "Dumping libraries from $libdir in $dir/dump"
+ $devtools_dir/gen-abi-dump.sh $libdir $dir/dump
+done
@@ -30,7 +30,8 @@ default_path=$PATH
# - LIBSSO_SNOW3G_PATH
# - LIBSSO_KASUMI_PATH
# - LIBSSO_ZUC_PATH
-. $(dirname $(readlink -f $0))/load-devel-config
+devtools_dir=$(dirname $(readlink -f $0))
+. $devtools_dir/load-devel-config
print_usage () {
echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2] ...]]"
@@ -64,6 +65,7 @@ print_help () {
[ -z $MAKE ] && echo "Cannot find make or gmake" && exit 1
J=$DPDK_MAKE_JOBS
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
builds_dir=${DPDK_BUILD_TEST_DIR:-.}
short=false
unset verbose
@@ -97,7 +99,7 @@ trap "signal=INT ; trap - INT ; kill -INT $$" INT
# notify result on exit
trap on_exit EXIT
-cd $(dirname $(readlink -f $0))/..
+cd $devtools_dir/..
reset_env ()
{
@@ -233,7 +235,7 @@ for conf in $configs ; do
# reload config with DPDK_TARGET set
DPDK_TARGET=$target
reset_env
- . $(dirname $(readlink -f $0))/load-devel-config
+ . $devtools_dir/load-devel-config
options=$(echo $conf | sed 's,[^~+]*,,')
dir=$builds_dir/$conf
@@ -246,6 +248,11 @@ for conf in $configs ; do
export RTE_TARGET=$target
rm -rf $dir/install
${MAKE} install O=$dir DESTDIR=$dir/install prefix=
+ if [ -d $abi_ref_build_dir/$conf/dump ]; then
+ echo "================== Check ABI $conf"
+ $devtools_dir/check-abi-dump.sh $dir/install \
+ $abi_ref_build_dir/$conf/dump
+ fi
echo "================== Build examples for $conf"
export RTE_SDK=$(readlink -f $dir)/install/share/dpdk
ln -sTf $(pwd)/lib $RTE_SDK/lib # workaround for vm_power_manager
@@ -16,6 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
MESON=${MESON:-meson}
use_shared="--default-library=shared"
+abi_ref_build_dir=${DPDK_ABI_REF_BUILD_DIR:-reference}
builds_dir=${DPDK_BUILD_TEST_DIR:-.}
if command -v gmake >/dev/null 2>&1 ; then
@@ -88,6 +89,11 @@ build () # <directory> <target compiler> <meson options>
echo "$ninja_cmd -C $builddir"
$ninja_cmd -C $builddir
fi
+
+ if [ -d $abi_ref_build_dir/$1/dump ]; then
+ $srcdir/devtools/check-abi-dump.sh $builddir
+ $abi_ref_build_dir/$1/dump
+ fi
}
if [ "$1" = "-vv" ] ; then
@@ -513,6 +513,31 @@ 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.
+Checking ABI compatibility
+--------------------------
+
+The first thing is to build reference binaries for the latest release your
+patches are built on top of.
+
+Either you are in a git tree and an easy way to identify this is to run::
+
+ git checkout $(git describe --abbrev=0)
+
+Or you use a tarball and you extract the sources in a director of your choice.
+
+Next is building those sources, refer to the previous paragraph.
+You can set ``DPDK_BUILD_TEST_DIR=reference``, so that the builds occur in this
+directory.
+
+Finally, the ABI dump files are generated with the
+``devtools/gen-abi-reference.sh`` script. This script will look for builds in
+the current sub directory ``reference``. But you can set the environment
+variable ``DPDK_ABI_REF_BUILD_DIR`` to a different location.
+
+Once done, you can check your current binaries ABI with this reference with the
+``devtools/check-abi-reference.sh`` script.
+
+
Sending Patches
---------------