DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Aaron Conole <aconole@redhat.com>, Thomas Monjalon <thomas@monjalon.net>
Cc: "Kinsella, Ray" <ray.kinsella@intel.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	David Marchand <david.marchand@redhat.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Laatz, Kevin" <kevin.laatz@intel.com>,
	"nhorman@tuxdriver.com" <nhorman@tuxdriver.com>,
	Michael Santana <maicolgabriel@hotmail.com>,
	"Mcnamara, John" <john.mcnamara@intel.com>,
	"Kovacevic, Marko" <marko.kovacevic@intel.com>
Subject: Re: [dpdk-dev] [PATCH] add ABI checks
Date: Wed, 15 Jan 2020 13:07:27 +0000	[thread overview]
Message-ID: <017eb858-6388-ecbd-f7da-c6d361c80010@intel.com> (raw)
In-Reply-To: <f7timlog1vt.fsf@dhcp-25.97.bos.redhat.com>

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


-- 
Thanks,
Anatoly

  reply	other threads:[~2020-01-15 13:07 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-20 15:20 David Marchand
2019-12-20 15:32 ` Richardson, Bruce
2019-12-20 16:20   ` Kinsella, Ray
2019-12-20 21:00     ` Thomas Monjalon
2020-01-06 13:17       ` Aaron Conole
2020-01-15 13:07         ` Burakov, Anatoly [this message]
2020-01-14 23:19     ` Thomas Monjalon
2020-01-15 11:33       ` Neil Horman
2020-01-15 12:38         ` Thomas Monjalon
2020-01-16 11:52           ` Neil Horman
2020-01-16 14:20             ` Thomas Monjalon
2020-01-16 18:49               ` Neil Horman
2020-01-16 20:01                 ` Thomas Monjalon
2020-01-17 19:01                   ` Neil Horman
2020-01-17 21:26                     ` David Marchand
2019-12-20 20:25 ` Neil Horman
2020-01-29 17:26 ` [dpdk-dev] [PATCH v2 0/4] " David Marchand
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 1/4] hash: fix meson headers packaging David Marchand
2020-01-30 10:12     ` Luca Boccassi
2020-01-30 10:54       ` David Marchand
2020-01-30 10:56         ` Luca Boccassi
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 2/4] build: split build helper David Marchand
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 3/4] build: test meson installation David Marchand
2020-01-29 17:26   ` [dpdk-dev] [PATCH v2 4/4] add ABI checks David Marchand
2020-01-29 17:42     ` Thomas Monjalon
2020-01-29 18:10       ` Anoob Joseph
2020-01-29 20:03         ` David Marchand
2020-01-29 20:13           ` Akhil Goyal
2020-01-30 16:09             ` Ferruh Yigit
2020-01-30 20:18               ` Thomas Monjalon
2020-01-31  9:03                 ` Ferruh Yigit
2020-01-31 10:26                   ` Ananyev, Konstantin
2020-01-31 14:16                 ` Trahe, Fiona
2020-02-02 13:05                   ` Thomas Monjalon
2020-02-02 14:41                     ` Ananyev, Konstantin
2020-02-03  9:30                       ` Ferruh Yigit
2020-02-03 11:50                         ` Neil Horman
2020-02-03 13:09                           ` Ferruh Yigit
2020-02-03 14:00                             ` Dodji Seketeli
2020-02-03 14:46                               ` Ferruh Yigit
2020-02-03 15:08                             ` Trahe, Fiona
2020-02-03 17:09                         ` Thomas Monjalon
2020-02-03 17:34                           ` Thomas Monjalon
2020-02-03 18:55                             ` Ray Kinsella
2020-02-03 21:07                               ` Thomas Monjalon
2020-02-04  9:46                                 ` Ferruh Yigit
2020-02-04 10:24                                 ` Thomas Monjalon
2020-02-04 12:44                                   ` Trahe, Fiona
2020-02-04 15:52                                     ` Trahe, Fiona
2020-02-04 15:59                                       ` Thomas Monjalon
2020-02-04 17:46                                         ` Trahe, Fiona
2020-02-13 14:51                                           ` Kusztal, ArkadiuszX
2020-03-16 12:57                                             ` Trahe, Fiona
2020-03-16 13:09                                               ` Thomas Monjalon
2020-03-17 13:27                                                 ` Kusztal, ArkadiuszX
2020-03-17 15:10                                                   ` Thomas Monjalon
2020-03-17 19:10                                                     ` Kusztal, ArkadiuszX
2020-02-04 12:57                                   ` Kevin Traynor
2020-02-04 14:44                                   ` Aaron Conole
2020-02-04 19:49                                     ` Neil Horman
2020-02-04  9:51                               ` David Marchand
2020-02-04 10:10                                 ` Trahe, Fiona
2020-02-04 10:38                                   ` Thomas Monjalon
2020-02-05 11:10                                 ` Ray Kinsella
2020-02-03 17:40                           ` Ferruh Yigit
2020-02-03 18:40                             ` Thomas Monjalon
2020-02-04  9:19                               ` Ferruh Yigit
2020-02-04  9:45                                 ` Thomas Monjalon
2020-02-04  9:56                                   ` Ferruh Yigit
2020-02-04 10:08                                     ` Bruce Richardson
2020-02-04 10:17                                     ` Kevin Traynor
2020-02-04 10:16                             ` Akhil Goyal
2020-02-04 10:28                               ` Thomas Monjalon
2020-02-04 10:32                                 ` Akhil Goyal
2020-02-04 11:35                                   ` Bruce Richardson
2020-02-04 22:10                                   ` Neil Horman
2020-02-05  6:16                                     ` [dpdk-dev] [EXT] " Anoob Joseph
2020-02-05 14:33                                       ` Trahe, Fiona
2020-02-04 21:59                               ` [dpdk-dev] " Neil Horman
2020-01-30 13:06         ` Trahe, Fiona
2020-01-30 15:59           ` Thomas Monjalon
2020-01-30 16:42             ` Ferruh Yigit
2020-01-30 23:49             ` Ananyev, Konstantin
2020-01-31  9:07               ` Ferruh Yigit
2020-01-31  9:37                 ` Ananyev, Konstantin
2020-01-30 10:57   ` [dpdk-dev] [PATCH v2 0/4] " Luca Boccassi
2020-01-30 16:00 ` [dpdk-dev] [PATCH v3 " David Marchand
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 1/4] hash: fix meson headers packaging David Marchand
2020-01-30 18:01     ` Wang, Yipeng1
2020-01-30 18:40       ` Honnappa Nagarahalli
2020-02-05 19:51         ` Wang, Yipeng1
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 2/4] build: split build helper David Marchand
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 3/4] build: test meson installation David Marchand
2020-01-30 22:17     ` Thomas Monjalon
2020-01-30 16:00   ` [dpdk-dev] [PATCH v3 4/4] add ABI checks David Marchand
2020-01-30 22:32     ` Thomas Monjalon
2020-02-01 15:29       ` David Marchand
2020-01-30 22:44     ` Thomas Monjalon
2020-02-02 21:08 ` [dpdk-dev] [PATCH v4 0/3] " David Marchand
2020-02-02 21:08   ` [dpdk-dev] [PATCH v4 1/3] hash: fix meson headers packaging David Marchand
2020-02-05 19:53     ` Wang, Yipeng1
2020-02-02 21:08   ` [dpdk-dev] [PATCH v4 2/3] build: split build helper David Marchand
2020-02-02 21:08   ` [dpdk-dev] [PATCH v4 3/3] add ABI checks David Marchand
2020-02-05 14:13   ` [dpdk-dev] [PATCH v4 0/3] " Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=017eb858-6388-ecbd-f7da-c6d361c80010@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=aconole@redhat.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=john.mcnamara@intel.com \
    --cc=kevin.laatz@intel.com \
    --cc=maicolgabriel@hotmail.com \
    --cc=marko.kovacevic@intel.com \
    --cc=nhorman@tuxdriver.com \
    --cc=ray.kinsella@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).