DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/4] abi breakage checks for meson
@ 2020-09-10 14:01 Conor Walsh
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
                   ` (4 more replies)
  0 siblings, 5 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:01 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patchset allows developers to check ABI breakages during build time.
Currently checking that the DPDK ABI has not changed before up-streaming
code is not intuitive. The current method, requires the contributor to
use either the test-build.sh and test-meson-build.sh tools, along side
some environmental variables to test their changes. Contributors in many
cases are either unaware or unable to do this themselves, leading to a
potentially serious situation where they are unknowingly up-streaming
code that breaks the ABI. These breakages are then caught by Travis, but
it would be more efficient if this is caught locally before up-streaming.

Conor Walsh (4):
  devtools: bug fix for gen-abi.sh
  devtools: add generation of compressed abi dump archives
  buildtools: add script to setup abi checks for meson
  build: add abi breakage checks to meson

 buildtools/abi-setup.py     | 104 ++++++++++++++++++++++++++++++
 buildtools/meson.build      |  20 ++++++
 config/meson.build          |   9 +++
 devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
 devtools/gen-abi.sh         |   6 +-
 drivers/meson.build         |  15 +++++
 lib/meson.build             |  15 +++++
 meson_options.txt           |   2 +
 8 files changed, 291 insertions(+), 5 deletions(-)
 create mode 100755 buildtools/abi-setup.py
 create mode 100755 devtools/gen-abi-tarball.py

-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH 1/4] devtools: bug fix for gen-abi.sh
  2020-09-10 14:01 [dpdk-dev] [PATCH 0/4] abi breakage checks for meson Conor Walsh
@ 2020-09-10 14:01 ` Conor Walsh
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:01 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch fixes a bug with the gen-abi.sh script in devtools.
When ran on an install directory the script would try to generate
.dump files from directories as well as the .so files which is
not correct.
Example error: abidw: gcc/lib/librte_net.so.21.0.p is not a regular file
To rectify this the regex that finds the appropriate .so files has been
changed and the file test has been removed.
This change was tested with the ABI_CHECK Travis checks in DPDK 20.08.
Travis build:
https://travis-ci.com/github/conorwalsh-intel/dpdk/jobs/382812849

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/devtools/gen-abi.sh b/devtools/gen-abi.sh
index c44b0e228..da6fe0556 100755
--- a/devtools/gen-abi.sh
+++ b/devtools/gen-abi.sh
@@ -16,11 +16,7 @@ fi
 dumpdir=$installdir/dump
 rm -rf $dumpdir
 mkdir -p $dumpdir
-for f in $(find $installdir -name "*.so.*"); do
-	if test -L $f; then
-		continue
-	fi
-
+for f in $(find $installdir -name "*.so"); do
 	libname=$(basename $f)
 	abidw --out-file $dumpdir/${libname%.so*}.dump $f
 done
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH 2/4] devtools: add generation of compressed abi dump archives
  2020-09-10 14:01 [dpdk-dev] [PATCH 0/4] abi breakage checks for meson Conor Walsh
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
@ 2020-09-10 14:01 ` Conor Walsh
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:01 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds a script that generates a compressed archive
containing .dump files which can be used to perform ABI
breakage checking for the build specified in the parameters.
Invoke using "./gen-abi-tarball.py -t <tag> -a <arch> [-cf <cross-file>]"
 - <tag>: dpdk tag e.g. "v20.11"
 - <arch>: required architecture e.g. "arm" or "x86_64"
 - <cross-file>: configuration file for cross compiling for another
                 system, this flag is not required.
                 e.g. "config/arm/arm64_armv8_linux_gcc"
E.g. "./gen-abi-tarball.py -t latest -a x86_64"
If a compiler is not specified using the CC environmental variable then
the script will default to using gcc.
Using these parameters the script will produce a .tar.gz archive
containing .dump files required to do ABI breakage checking

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100755 devtools/gen-abi-tarball.py

diff --git a/devtools/gen-abi-tarball.py b/devtools/gen-abi-tarball.py
new file mode 100755
index 000000000..ce7a0c6ac
--- /dev/null
+++ b/devtools/gen-abi-tarball.py
@@ -0,0 +1,125 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+import argparse
+
+# Get command line arguments
+parser = argparse.ArgumentParser(usage='\rThis script is intended to generate ABI dump tarballs\n'+
+                                       'Supported enviromental variables\n'+
+                                       '\t- CC: The required compiler will be determined using this environmental variable.\n')
+parser.add_argument('-t', '--tag', type=str, dest='tag', help='DPDK tag e.g. latest or v20.11')
+parser.add_argument('-cf', '--cross-file', type=str, dest='crosscompile', help='Set the location of a cross compile config')
+parser.add_argument('-a', '--arch', type=str, dest='arch', help='Arch arm or x86_64')
+args = parser.parse_args()
+
+# Get the DPDK tag if not supplied set as latest
+if args.tag:
+    user_tag = args.tag
+else:
+    user_tag = 'latest'
+    print('No tag supplied defaulting to latest')
+
+# Get the cross-compile option
+if args.crosscompile:
+    cross_comp = args.crosscompile
+    if not args.arch:
+        print('ERROR Arch must be set using -a when using cross compile')
+        exit()
+    cross_comp = os.path.abspath(cross_comp)
+    cross_comp_meson = '--cross-file '+cross_comp
+else:
+    cross_comp = ''
+    cross_comp_meson = ''
+
+# Get the required system architecture if not supplied set as x86_64
+if args.arch:
+    arch = args.arch
+else:
+    arch = os.popen('uname -m').read().strip()
+    print('No system architecture supplied defaulting to '+arch)
+
+tag = ''
+# If the user did not supply tag or wants latest then get latest tag
+if user_tag == 'latest':
+    # Get latest quarterly build tag from git repo
+    tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk | grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
+else:
+    tag = user_tag
+    # If the user supplied tag is not in the DPDK repo then fail
+    tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk refs/tags/'+tag+' | wc -l').read().strip())
+    if tag_check != 1:
+        print('ERROR supplied tag does not exist in DPDK repo')
+        exit()
+
+# Get the specified compiler from system
+comp_env = 'CC'
+if comp_env in os.environ:
+    comp = os.environ[comp_env]
+    comp_default = ''
+else:
+    print('No compiler specified, defaulting to gcc')
+    comp = 'gcc'
+    comp_default = 'CC=gcc'
+
+# Print the configuration to the user
+print('\nSelected Build: '+tag+', Compiler: '+comp+', Architecture: '+arch+', Cross Compile: '+cross_comp)
+
+# Store the base directory script is working from
+baseDir = os.getcwd()
+# Store devtools dir
+devtoolsDir = os.path.abspath(os.path.dirname(os.path.realpath(sys.argv[0])))
+
+# Create directory for DPDK git repo and build
+try:
+    os.mkdir('dump_dpdk')
+except OSError as error:
+    print('ERROR The dump_dpdk directory could not be created, ensure it does not exist before start')
+    exit()
+os.chdir('dump_dpdk')
+# Clone DPDK and switch to specified tag
+print('Cloning '+tag+' from DPDK git')
+os.popen('git clone --quiet http://dpdk.org/git/dpdk >/dev/null').read()
+os.chdir('dpdk')
+os.popen('git checkout --quiet '+tag+' >/dev/null').read()
+
+# Create build folder with meson and set debug build and cross compile (if needed)
+print('Configuring Meson')
+os.popen(comp_default+' meson dumpbuild '+cross_comp_meson+' >/dev/null').read()
+os.chdir('dumpbuild')
+os.popen('meson configure -Dbuildtype=debug >/dev/null').read()
+print('Building DPDK . . .')
+#Build DPDK with ninja
+os.popen('ninja >/dev/null').read()
+gccDir = os.getcwd()
+
+# Create directory for abi dump files
+dumpDir = os.path.join(baseDir,tag+'-'+comp+'-'+arch+'-abi_dump')
+try:
+    os.mkdir(dumpDir)
+except OSError as error:
+    print('ERROR The '+dumpDir+' directory could not be created')
+    os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
+    exit()
+
+# Create dump files and output to dump directory
+print('Generating ABI dump files')
+genabiout = os.popen(os.path.join(devtoolsDir,'gen-abi.sh')+' '+gccDir).read()
+os.popen('cp dump/* '+dumpDir).read()
+
+# Compress the dump directory
+print('Creating Tarball of dump files')
+os.chdir(baseDir)
+origSize = os.popen('du -sh '+dumpDir+' | sed "s/\s.*$//"').read()
+os.popen('tar -czf '+dumpDir.split('/')[-1]+'.tar.gz '+dumpDir.split('/')[-1]+' >/dev/null').read()
+newSize = os.popen('du -sh '+dumpDir+'.tar.gz | sed "s/\s.*$//"').read()
+
+# Remove all temporary directories
+print('Cleaning up temporary directories')
+os.popen('rm -rf '+dumpDir).read()
+os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
+
+#Print output of the script to the user
+print('\nDump of DPDK ABI '+tag+' is available in '+dumpDir.split('/')[-1]+'.tar.gz (Original Size: '+origSize.strip()+', Compressed Size:'+newSize.strip()+')\n')
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH 3/4] buildtools: add script to setup abi checks for meson
  2020-09-10 14:01 [dpdk-dev] [PATCH 0/4] abi breakage checks for meson Conor Walsh
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-09-10 14:01 ` Conor Walsh
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 4/4] build: add abi breakage checks to meson Conor Walsh
  2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:01 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds a script that is intended to be invoked by meson to
do the required setup for performing ABI breakage checks at build time.
The required ABI dump archives can come from several sources including
being generated at build time or prebuilt archives can be pulled from a
remote http location or local directory.
Invoke using "./abi-setup.py -t <tag> -d <dpdk_source_path>"
 - <tag>: dpdk tag e.g. "v20.11"
 - <dpdk_source_path>: path to dpdk source directory
E.g. "./abi-setup.py -t v20.08 -d /root/dpdk"
As this script is intended to be run by meson during a build
some options can be specified by environmental variables:
 - DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the
   systems dump directories.
 - CC: The required compiler will be determined using this
   environmental variable
 - DPDK_ABI_TAR_URI: Can be used to specify a location that the script
   can pull prebuilt or cached dump archives from. This can be a remote
   http location or a local directory
After the script has setup an appropriate ABI dump directory using one of
the multiple methods available to it, it will print the location of this
directory to the command line.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/abi-setup.py | 104 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100755 buildtools/abi-setup.py

diff --git a/buildtools/abi-setup.py b/buildtools/abi-setup.py
new file mode 100755
index 000000000..a3c34536e
--- /dev/null
+++ b/buildtools/abi-setup.py
@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+import argparse
+
+# Get command line arguments
+parser = argparse.ArgumentParser(usage='\rThis script is intended to setup ABI dumps for meson to perform ABI checks\n'+
+                                       'Supported enviromental variables\n'+
+                                       '\t- DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the systems dump directories.\n'+
+                                       '\t- CC: The required compiler will be determined using this environmental variable.\n'+
+                                       '\t- DPDK_ABI_TAR_URI: Can be used to specify a location that the script can pull prebuilt or cached dump archives from. This can be a remote http location or a local directory.\n')
+parser.add_argument('-t', '--tag', dest='tag', type=str, help='DPDK tag e.g. latest or v20.11')
+parser.add_argument('-d', '--dpdk', dest='dpdk', type=str, help='Path to DPDK source directory')
+args = parser.parse_args()
+
+# Get the DPDK tag if not supplied set as latest
+if args.tag:
+    user_tag = args.tag
+else:
+    user_tag = 'latest'
+
+tag = ''
+# If the user did not supply tag or wants latest then get latest tag
+if user_tag == 'latest':
+    # Get latest quarterly build tag from git repo
+    tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk | grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
+else:
+    tag = user_tag
+    # If the user supplied tag does not exist then fail
+    tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk refs/tags/'+tag+' | wc -l').read().strip())
+    if tag_check != 1:
+        print('ERROR supplied tag does not exist in DPDK repo')
+        exit()
+
+# Get the specified compiler from system
+comp_env = 'CC'
+if comp_env in os.environ:
+    comp = os.environ[comp_env]
+else:
+    comp = 'gcc'
+
+# Get the systems architecture
+arch = os.popen('uname -m').read().strip()
+
+# Get devtools path
+devtools_path = ''
+if args.dpdk:
+    devtools_path = os.path.abspath(os.path.join(args.dpdk,'devtools'))
+else:
+    print('ERROR DPDK source directory must be specified')
+    exit()
+
+# Get the abi dumps folder from args or env fail if none supplied
+abi_folder = ''
+abi_env = 'DPDK_ABI_DUMPS_PATH'
+if abi_env in os.environ:
+    abi_folder = os.path.abspath(os.environ[abi_env])
+else:
+    abi_folder = os.path.abspath(os.path.join(args.dpdk,'abi_dumps'))
+
+# If the directory doesn't exist create it and add a README to explain what it does
+if not os.path.exists(abi_folder):
+    os.makedirs(abi_folder)
+    f=open(abi_folder+'/README','w+')
+    f.write('This directory has been setup to contain the ABI dump folders needed to perform ABI checks\n')
+    f.write('Directories here must be in the format {DPDK Tag}-{Compiler ID}-{Architecture}-abi_dump\n')
+    f.write('e.g. v20.11-gcc-x86_64-abi_dump\n')
+    f.write('Directories that do not use this format will not be picked up by the meson ABI checks\n')
+    f.write('This directory is managed automatically unless desired by the user\n')
+    f.close()
+
+# Move to abi folder
+os.chdir(abi_folder)
+abi_dump=tag+'-'+comp+'-'+arch+'-abi_dump'
+# Download and untar abi dump if not present
+if not os.path.exists(abi_dump):
+    # Check DPDK_ABI_TAR_URI for the location of the tarballs local or web
+    tar_uri_env = 'DPDK_ABI_TAR_URI'
+    if tar_uri_env in os.environ:
+        abi_tar_uri = os.environ[tar_uri_env]
+        if abi_tar_uri.startswith('http'):
+            # Wget the required tarball
+            os.popen('wget '+os.path.join(abi_tar_uri,abi_dump)+'.tar.gz >/dev/null 2>&1').read()
+        else:
+            abi_tar_uri = os.path.abspath(abi_tar_uri)
+            os.popen('cp '+os.path.join(abi_tar_uri,abi_dump)+'.tar.gz . >/dev/null 2>&1').read()
+    # Check tarball was downloaded
+    if os.path.isfile(abi_dump+'.tar.gz'):
+        os.popen('tar -xzf '+abi_dump+'.tar.gz >/dev/null 2>&1').read()
+        os.popen('rm -rf '+abi_dump+'.tar.gz').read()
+    # If the tarball was not found then generate it
+    else:
+        os.popen(os.path.join(devtools_path,'gen-abi-tarball.py')+' -t '+tag+' -a '+arch+' >/dev/null 2>&1').read()
+        if not os.path.isfile(abi_dump+'.tar.gz'):
+            print('ERROR ABI check generation failed '+os.path.join(devtools_path,'gen-abi-tarball.py')+' -t '+tag+' -a '+arch)
+            exit()
+        os.popen('tar -xzf '+abi_dump+'.tar.gz >/dev/null 2>&1').read()
+        os.popen('rm -rf '+abi_dump+'.tar.gz').read()
+
+# Tell user where specified directory is
+print(os.path.abspath(abi_dump))
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH 4/4] build: add abi breakage checks to meson
  2020-09-10 14:01 [dpdk-dev] [PATCH 0/4] abi breakage checks for meson Conor Walsh
                   ` (2 preceding siblings ...)
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
@ 2020-09-10 14:01 ` Conor Walsh
  2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:01 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds the ability to run ABI breakage checks to meson.
To do this the developer needs to set the meson build type to debug and
set the version of DPDK that they want to check the ABI against.
The option "abi_checks" has been added to meson for this, the option
accepts DPDK tags e.g. "latest" or "v20.11".
Example meson command: "meson -Dbuildtype=debug -Dabi_checks=v20.11 build"
When the build is done using ninja the ABI checks will be performed
if any breakages are present the build will fail.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/meson.build | 20 ++++++++++++++++++++
 config/meson.build     |  9 +++++++++
 drivers/meson.build    | 15 +++++++++++++++
 lib/meson.build        | 15 +++++++++++++++
 meson_options.txt      |  2 ++
 5 files changed, 61 insertions(+)

diff --git a/buildtools/meson.build b/buildtools/meson.build
index 04808dabc..63513a273 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -8,6 +8,26 @@ check_symbols = find_program('check-symbols.sh')
 ldflags_ibverbs_static = find_program('options-ibverbs-static.sh')
 binutils_avx512_check = find_program('binutils-avx512-check.sh')
 
+abi_checks = get_option('abi_checks')
+abi_dir = ''
+# If abi checks enabled setup abi dump directory
+if abi_checks!=''
+	message('ABI Checks are being setup, If DPDK_ABI_TAR_URI has not been set these checks may be need to be generated this could take several minutes')
+	setup_check = run_command('abi-setup.py','-t',abi_checks,'-d',meson.source_root()).stdout().strip()
+	# Check if error returned from script
+	if setup_check.startswith('ERROR')
+		error('ABI checks setup failed: '+setup_check)
+	# No error then set abi directory
+	else
+		abi_dir=setup_check
+	endif
+endif
+abidiff = find_program('abidiff', required: abi_checks!='')
+if abidiff.found()==false and abi_checks!=''
+	error('ABI checks require abidiff to to be completed')
+endif
+abignore = files(meson.source_root()+'/devtools/libabigail.abignore')
+
 # set up map-to-win script using python, either built-in or external
 python3 = import('python').find_installation(required: false)
 if python3.found()
diff --git a/config/meson.build b/config/meson.build
index 6996e5cbe..65f8ea4d7 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -47,6 +47,15 @@ else
 	dpdk_conf.set('RTE_VER_RELEASE', 99)
 endif
 
+# abi checks cannot be run on windows
+if is_windows and abi_checks!=''
+	error('ABI checks cannot be run on windows')
+endif
+# abi checks can only be run on a debug build
+if not get_option('debug') and abi_checks!=''
+  error('Build type must have debug symbols when abi_checks=enabled')
+endif
+
 pmd_subdir_opt = get_option('drivers_install_subdir')
 if pmd_subdir_opt.contains('<VERSION>')
 	pmd_subdir_opt = abi_version.join(pmd_subdir_opt.split('<VERSION>'))
diff --git a/drivers/meson.build b/drivers/meson.build
index 5f9526557..c82fbc250 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -208,6 +208,21 @@ foreach subpath:subdirs
 					include_directories: includes,
 					dependencies: static_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/lib' + lib_name + '.dump'
+			if abidiff.found() and abi_checks!='' and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target('lib' + lib_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							files(dump_name),
+							'@INPUT@'],
+					      input: shared_lib,
+					      output: 'lib' + lib_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_drivers += static_lib
 
 			set_variable('shared_@0@'.format(lib_name), shared_dep)
diff --git a/lib/meson.build b/lib/meson.build
index 3852c0156..e864e0440 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -190,6 +190,21 @@ foreach l:libraries
 					include_directories: includes,
 					dependencies: shared_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/' + dir_name + '.dump'
+			if abidiff.found() and abi_checks!='' and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target(dir_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							files(dump_name),
+							'@INPUT@'],
+					      input: shared_lib,
+					      output: dir_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 		endif # sources.length() > 0
diff --git a/meson_options.txt b/meson_options.txt
index 9bf18ab6b..26ac48f45 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('abi_checks', type: 'string', value: '',
+    description: 'Enable abi compatibility checks to run during the build. This requires debug build to be enabled. Input is latest or git tag e.g. v20.11')
 option('armv8_crypto_dir', type: 'string', value: '',
 	description: 'path to the armv8_crypto library installation directory')
 option('disable_drivers', type: 'string', value: '',
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson
  2020-09-10 14:01 [dpdk-dev] [PATCH 0/4] abi breakage checks for meson Conor Walsh
                   ` (3 preceding siblings ...)
  2020-09-10 14:01 ` [dpdk-dev] [PATCH 4/4] build: add abi breakage checks to meson Conor Walsh
@ 2020-09-10 14:21 ` Conor Walsh
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
                     ` (4 more replies)
  4 siblings, 5 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:21 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patchset allows developers to check ABI breakages during build time.
Currently checking that the DPDK ABI has not changed before up-streaming
code is not intuitive. The current method, requires the contributor to
use either the test-build.sh and test-meson-build.sh tools, along side
some environmental variables to test their changes. Contributors in many
cases are either unaware or unable to do this themselves, leading to a
potentially serious situation where they are unknowingly up-streaming
code that breaks the ABI. These breakages are then caught by Travis, but
it is more efficient if this is caught locally before up-streaming.

---
v2: Spelling mistake, corrected spelling of environmental

Conor Walsh (4):
  devtools: bug fix for gen-abi.sh
  devtools: add generation of compressed abi dump archives
  buildtools: add script to setup abi checks for meson
  build: add abi breakage checks to meson

 buildtools/abi-setup.py     | 104 ++++++++++++++++++++++++++++++
 buildtools/meson.build      |  20 ++++++
 config/meson.build          |   9 +++
 devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
 devtools/gen-abi.sh         |   6 +-
 drivers/meson.build         |  15 +++++
 lib/meson.build             |  15 +++++
 meson_options.txt           |   2 +
 8 files changed, 291 insertions(+), 5 deletions(-)
 create mode 100755 buildtools/abi-setup.py
 create mode 100755 devtools/gen-abi-tarball.py

-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v2 1/4] devtools: bug fix for gen-abi.sh
  2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
@ 2020-09-10 14:21   ` Conor Walsh
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:21 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch fixes a bug with the gen-abi.sh script in devtools.
When ran on an install directory the script would try to generate
.dump files from directories as well as the .so files which is
not correct.
Example error: abidw: gcc/lib/librte_net.so.21.0.p is not a regular file
To rectify this the regex that finds the appropriate .so files has been
changed and the file test has been removed.
This change was tested with the ABI_CHECK Travis checks in DPDK 20.08.
Travis build:
https://travis-ci.com/github/conorwalsh-intel/dpdk/jobs/382812849

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/devtools/gen-abi.sh b/devtools/gen-abi.sh
index c44b0e228..da6fe0556 100755
--- a/devtools/gen-abi.sh
+++ b/devtools/gen-abi.sh
@@ -16,11 +16,7 @@ fi
 dumpdir=$installdir/dump
 rm -rf $dumpdir
 mkdir -p $dumpdir
-for f in $(find $installdir -name "*.so.*"); do
-	if test -L $f; then
-		continue
-	fi
-
+for f in $(find $installdir -name "*.so"); do
 	libname=$(basename $f)
 	abidw --out-file $dumpdir/${libname%.so*}.dump $f
 done
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v2 2/4] devtools: add generation of compressed abi dump archives
  2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
@ 2020-09-10 14:21   ` Conor Walsh
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:21 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds a script that generates a compressed archive
containing .dump files which can be used to perform ABI
breakage checking for the build specified in the parameters.
Invoke using "./gen-abi-tarball.py -t <tag> -a <arch> [-cf <cross-file>]"
 - <tag>: dpdk tag e.g. "v20.11"
 - <arch>: required architecture e.g. "arm" or "x86_64"
 - <cross-file>: configuration file for cross compiling for another
                 system, this flag is not required.
                 e.g. "config/arm/arm64_armv8_linux_gcc"
E.g. "./gen-abi-tarball.py -t latest -a x86_64"
If a compiler is not specified using the CC environmental variable then
the script will default to using gcc.
Using these parameters the script will produce a .tar.gz archive
containing .dump files required to do ABI breakage checking

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100755 devtools/gen-abi-tarball.py

diff --git a/devtools/gen-abi-tarball.py b/devtools/gen-abi-tarball.py
new file mode 100755
index 000000000..2104b4ee6
--- /dev/null
+++ b/devtools/gen-abi-tarball.py
@@ -0,0 +1,125 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+import argparse
+
+# Get command line arguments
+parser = argparse.ArgumentParser(usage='\rThis script is intended to generate ABI dump tarballs\n'+
+                                       'Supported environmental variables\n'+
+                                       '\t- CC: The required compiler will be determined using this environmental variable.\n')
+parser.add_argument('-t', '--tag', type=str, dest='tag', help='DPDK tag e.g. latest or v20.11')
+parser.add_argument('-cf', '--cross-file', type=str, dest='crosscompile', help='Set the location of a cross compile config')
+parser.add_argument('-a', '--arch', type=str, dest='arch', help='Arch arm or x86_64')
+args = parser.parse_args()
+
+# Get the DPDK tag if not supplied set as latest
+if args.tag:
+    user_tag = args.tag
+else:
+    user_tag = 'latest'
+    print('No tag supplied defaulting to latest')
+
+# Get the cross-compile option
+if args.crosscompile:
+    cross_comp = args.crosscompile
+    if not args.arch:
+        print('ERROR Arch must be set using -a when using cross compile')
+        exit()
+    cross_comp = os.path.abspath(cross_comp)
+    cross_comp_meson = '--cross-file '+cross_comp
+else:
+    cross_comp = ''
+    cross_comp_meson = ''
+
+# Get the required system architecture if not supplied set as x86_64
+if args.arch:
+    arch = args.arch
+else:
+    arch = os.popen('uname -m').read().strip()
+    print('No system architecture supplied defaulting to '+arch)
+
+tag = ''
+# If the user did not supply tag or wants latest then get latest tag
+if user_tag == 'latest':
+    # Get latest quarterly build tag from git repo
+    tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk | grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
+else:
+    tag = user_tag
+    # If the user supplied tag is not in the DPDK repo then fail
+    tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk refs/tags/'+tag+' | wc -l').read().strip())
+    if tag_check != 1:
+        print('ERROR supplied tag does not exist in DPDK repo')
+        exit()
+
+# Get the specified compiler from system
+comp_env = 'CC'
+if comp_env in os.environ:
+    comp = os.environ[comp_env]
+    comp_default = ''
+else:
+    print('No compiler specified, defaulting to gcc')
+    comp = 'gcc'
+    comp_default = 'CC=gcc'
+
+# Print the configuration to the user
+print('\nSelected Build: '+tag+', Compiler: '+comp+', Architecture: '+arch+', Cross Compile: '+cross_comp)
+
+# Store the base directory script is working from
+baseDir = os.getcwd()
+# Store devtools dir
+devtoolsDir = os.path.abspath(os.path.dirname(os.path.realpath(sys.argv[0])))
+
+# Create directory for DPDK git repo and build
+try:
+    os.mkdir('dump_dpdk')
+except OSError as error:
+    print('ERROR The dump_dpdk directory could not be created, ensure it does not exist before start')
+    exit()
+os.chdir('dump_dpdk')
+# Clone DPDK and switch to specified tag
+print('Cloning '+tag+' from DPDK git')
+os.popen('git clone --quiet http://dpdk.org/git/dpdk >/dev/null').read()
+os.chdir('dpdk')
+os.popen('git checkout --quiet '+tag+' >/dev/null').read()
+
+# Create build folder with meson and set debug build and cross compile (if needed)
+print('Configuring Meson')
+os.popen(comp_default+' meson dumpbuild '+cross_comp_meson+' >/dev/null').read()
+os.chdir('dumpbuild')
+os.popen('meson configure -Dbuildtype=debug >/dev/null').read()
+print('Building DPDK . . .')
+#Build DPDK with ninja
+os.popen('ninja >/dev/null').read()
+gccDir = os.getcwd()
+
+# Create directory for abi dump files
+dumpDir = os.path.join(baseDir,tag+'-'+comp+'-'+arch+'-abi_dump')
+try:
+    os.mkdir(dumpDir)
+except OSError as error:
+    print('ERROR The '+dumpDir+' directory could not be created')
+    os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
+    exit()
+
+# Create dump files and output to dump directory
+print('Generating ABI dump files')
+genabiout = os.popen(os.path.join(devtoolsDir,'gen-abi.sh')+' '+gccDir).read()
+os.popen('cp dump/* '+dumpDir).read()
+
+# Compress the dump directory
+print('Creating Tarball of dump files')
+os.chdir(baseDir)
+origSize = os.popen('du -sh '+dumpDir+' | sed "s/\s.*$//"').read()
+os.popen('tar -czf '+dumpDir.split('/')[-1]+'.tar.gz '+dumpDir.split('/')[-1]+' >/dev/null').read()
+newSize = os.popen('du -sh '+dumpDir+'.tar.gz | sed "s/\s.*$//"').read()
+
+# Remove all temporary directories
+print('Cleaning up temporary directories')
+os.popen('rm -rf '+dumpDir).read()
+os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
+
+#Print output of the script to the user
+print('\nDump of DPDK ABI '+tag+' is available in '+dumpDir.split('/')[-1]+'.tar.gz (Original Size: '+origSize.strip()+', Compressed Size:'+newSize.strip()+')\n')
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v2 3/4] buildtools: add script to setup abi checks for meson
  2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-09-10 14:21   ` Conor Walsh
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 4/4] build: add abi breakage checks to meson Conor Walsh
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:21 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds a script that is intended to be invoked by meson to
do the required setup for performing ABI breakage checks at build time.
The required ABI dump archives can come from several sources including
being generated at build time or prebuilt archives can be pulled from a
remote http location or local directory.
Invoke using "./abi-setup.py -t <tag> -d <dpdk_source_path>"
 - <tag>: dpdk tag e.g. "v20.11"
 - <dpdk_source_path>: path to dpdk source directory
E.g. "./abi-setup.py -t v20.08 -d /root/dpdk"
As this script is intended to be run by meson during a build
some options can be specified by environmental variables:
 - DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the
   systems dump directories.
 - CC: The required compiler will be determined using this
   environmental variable
 - DPDK_ABI_TAR_URI: Can be used to specify a location that the script
   can pull prebuilt or cached dump archives from. This can be a remote
   http location or a local directory
After the script has setup an appropriate ABI dump directory using one of
the multiple methods available to it, it will print the location of this
directory to the command line.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/abi-setup.py | 104 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100755 buildtools/abi-setup.py

diff --git a/buildtools/abi-setup.py b/buildtools/abi-setup.py
new file mode 100755
index 000000000..c156b3c73
--- /dev/null
+++ b/buildtools/abi-setup.py
@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+import argparse
+
+# Get command line arguments
+parser = argparse.ArgumentParser(usage='\rThis script is intended to setup ABI dumps for meson to perform ABI checks\n'+
+                                       'Supported environmental variables\n'+
+                                       '\t- DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the systems dump directories.\n'+
+                                       '\t- CC: The required compiler will be determined using this environmental variable.\n'+
+                                       '\t- DPDK_ABI_TAR_URI: Can be used to specify a location that the script can pull prebuilt or cached dump archives from. This can be a remote http location or a local directory.\n')
+parser.add_argument('-t', '--tag', dest='tag', type=str, help='DPDK tag e.g. latest or v20.11')
+parser.add_argument('-d', '--dpdk', dest='dpdk', type=str, help='Path to DPDK source directory')
+args = parser.parse_args()
+
+# Get the DPDK tag if not supplied set as latest
+if args.tag:
+    user_tag = args.tag
+else:
+    user_tag = 'latest'
+
+tag = ''
+# If the user did not supply tag or wants latest then get latest tag
+if user_tag == 'latest':
+    # Get latest quarterly build tag from git repo
+    tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk | grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
+else:
+    tag = user_tag
+    # If the user supplied tag does not exist then fail
+    tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk refs/tags/'+tag+' | wc -l').read().strip())
+    if tag_check != 1:
+        print('ERROR supplied tag does not exist in DPDK repo')
+        exit()
+
+# Get the specified compiler from system
+comp_env = 'CC'
+if comp_env in os.environ:
+    comp = os.environ[comp_env]
+else:
+    comp = 'gcc'
+
+# Get the systems architecture
+arch = os.popen('uname -m').read().strip()
+
+# Get devtools path
+devtools_path = ''
+if args.dpdk:
+    devtools_path = os.path.abspath(os.path.join(args.dpdk,'devtools'))
+else:
+    print('ERROR DPDK source directory must be specified')
+    exit()
+
+# Get the abi dumps folder from args or env fail if none supplied
+abi_folder = ''
+abi_env = 'DPDK_ABI_DUMPS_PATH'
+if abi_env in os.environ:
+    abi_folder = os.path.abspath(os.environ[abi_env])
+else:
+    abi_folder = os.path.abspath(os.path.join(args.dpdk,'abi_dumps'))
+
+# If the directory doesn't exist create it and add a README to explain what it does
+if not os.path.exists(abi_folder):
+    os.makedirs(abi_folder)
+    f=open(abi_folder+'/README','w+')
+    f.write('This directory has been setup to contain the ABI dump folders needed to perform ABI checks\n')
+    f.write('Directories here must be in the format {DPDK Tag}-{Compiler ID}-{Architecture}-abi_dump\n')
+    f.write('e.g. v20.11-gcc-x86_64-abi_dump\n')
+    f.write('Directories that do not use this format will not be picked up by the meson ABI checks\n')
+    f.write('This directory is managed automatically unless desired by the user\n')
+    f.close()
+
+# Move to abi folder
+os.chdir(abi_folder)
+abi_dump=tag+'-'+comp+'-'+arch+'-abi_dump'
+# Download and untar abi dump if not present
+if not os.path.exists(abi_dump):
+    # Check DPDK_ABI_TAR_URI for the location of the tarballs local or web
+    tar_uri_env = 'DPDK_ABI_TAR_URI'
+    if tar_uri_env in os.environ:
+        abi_tar_uri = os.environ[tar_uri_env]
+        if abi_tar_uri.startswith('http'):
+            # Wget the required tarball
+            os.popen('wget '+os.path.join(abi_tar_uri,abi_dump)+'.tar.gz >/dev/null 2>&1').read()
+        else:
+            abi_tar_uri = os.path.abspath(abi_tar_uri)
+            os.popen('cp '+os.path.join(abi_tar_uri,abi_dump)+'.tar.gz . >/dev/null 2>&1').read()
+    # Check tarball was downloaded
+    if os.path.isfile(abi_dump+'.tar.gz'):
+        os.popen('tar -xzf '+abi_dump+'.tar.gz >/dev/null 2>&1').read()
+        os.popen('rm -rf '+abi_dump+'.tar.gz').read()
+    # If the tarball was not found then generate it
+    else:
+        os.popen(os.path.join(devtools_path,'gen-abi-tarball.py')+' -t '+tag+' -a '+arch+' >/dev/null 2>&1').read()
+        if not os.path.isfile(abi_dump+'.tar.gz'):
+            print('ERROR ABI check generation failed '+os.path.join(devtools_path,'gen-abi-tarball.py')+' -t '+tag+' -a '+arch)
+            exit()
+        os.popen('tar -xzf '+abi_dump+'.tar.gz >/dev/null 2>&1').read()
+        os.popen('rm -rf '+abi_dump+'.tar.gz').read()
+
+# Tell user where specified directory is
+print(os.path.abspath(abi_dump))
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v2 4/4] build: add abi breakage checks to meson
  2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
                     ` (2 preceding siblings ...)
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
@ 2020-09-10 14:21   ` Conor Walsh
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-10 14:21 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds the ability to run ABI breakage checks to meson.
To do this the developer needs to set the meson build type to debug and
set the version of DPDK that they want to check the ABI against.
The option "abi_checks" has been added to meson for this, the option
accepts DPDK tags e.g. "latest" or "v20.11".
Example meson command: "meson -Dbuildtype=debug -Dabi_checks=v20.11 build"
When the build is done using ninja the ABI checks will be performed
if any breakages are present the build will fail.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/meson.build | 20 ++++++++++++++++++++
 config/meson.build     |  9 +++++++++
 drivers/meson.build    | 15 +++++++++++++++
 lib/meson.build        | 15 +++++++++++++++
 meson_options.txt      |  2 ++
 5 files changed, 61 insertions(+)

diff --git a/buildtools/meson.build b/buildtools/meson.build
index 04808dabc..63513a273 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -8,6 +8,26 @@ check_symbols = find_program('check-symbols.sh')
 ldflags_ibverbs_static = find_program('options-ibverbs-static.sh')
 binutils_avx512_check = find_program('binutils-avx512-check.sh')
 
+abi_checks = get_option('abi_checks')
+abi_dir = ''
+# If abi checks enabled setup abi dump directory
+if abi_checks!=''
+	message('ABI Checks are being setup, If DPDK_ABI_TAR_URI has not been set these checks may be need to be generated this could take several minutes')
+	setup_check = run_command('abi-setup.py','-t',abi_checks,'-d',meson.source_root()).stdout().strip()
+	# Check if error returned from script
+	if setup_check.startswith('ERROR')
+		error('ABI checks setup failed: '+setup_check)
+	# No error then set abi directory
+	else
+		abi_dir=setup_check
+	endif
+endif
+abidiff = find_program('abidiff', required: abi_checks!='')
+if abidiff.found()==false and abi_checks!=''
+	error('ABI checks require abidiff to to be completed')
+endif
+abignore = files(meson.source_root()+'/devtools/libabigail.abignore')
+
 # set up map-to-win script using python, either built-in or external
 python3 = import('python').find_installation(required: false)
 if python3.found()
diff --git a/config/meson.build b/config/meson.build
index 6996e5cbe..65f8ea4d7 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -47,6 +47,15 @@ else
 	dpdk_conf.set('RTE_VER_RELEASE', 99)
 endif
 
+# abi checks cannot be run on windows
+if is_windows and abi_checks!=''
+	error('ABI checks cannot be run on windows')
+endif
+# abi checks can only be run on a debug build
+if not get_option('debug') and abi_checks!=''
+  error('Build type must have debug symbols when abi_checks=enabled')
+endif
+
 pmd_subdir_opt = get_option('drivers_install_subdir')
 if pmd_subdir_opt.contains('<VERSION>')
 	pmd_subdir_opt = abi_version.join(pmd_subdir_opt.split('<VERSION>'))
diff --git a/drivers/meson.build b/drivers/meson.build
index 5f9526557..c82fbc250 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -208,6 +208,21 @@ foreach subpath:subdirs
 					include_directories: includes,
 					dependencies: static_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/lib' + lib_name + '.dump'
+			if abidiff.found() and abi_checks!='' and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target('lib' + lib_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							files(dump_name),
+							'@INPUT@'],
+					      input: shared_lib,
+					      output: 'lib' + lib_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_drivers += static_lib
 
 			set_variable('shared_@0@'.format(lib_name), shared_dep)
diff --git a/lib/meson.build b/lib/meson.build
index 3852c0156..e864e0440 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -190,6 +190,21 @@ foreach l:libraries
 					include_directories: includes,
 					dependencies: shared_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/' + dir_name + '.dump'
+			if abidiff.found() and abi_checks!='' and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target(dir_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							files(dump_name),
+							'@INPUT@'],
+					      input: shared_lib,
+					      output: dir_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 		endif # sources.length() > 0
diff --git a/meson_options.txt b/meson_options.txt
index 9bf18ab6b..26ac48f45 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('abi_checks', type: 'string', value: '',
+    description: 'Enable abi compatibility checks to run during the build. This requires debug build to be enabled. Input is latest or git tag e.g. v20.11')
 option('armv8_crypto_dir', type: 'string', value: '',
 	description: 'path to the armv8_crypto library installation directory')
 option('disable_drivers', type: 'string', value: '',
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson
  2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
                     ` (3 preceding siblings ...)
  2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 4/4] build: add abi breakage checks to meson Conor Walsh
@ 2020-09-11 16:03   ` Conor Walsh
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
                       ` (5 more replies)
  4 siblings, 6 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-11 16:03 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patchset allows developers to check ABI breakages during build
time.
Currently checking that the DPDK ABI has not changed before up-streaming
code is not intuitive. The current method, requires the contributor to
use either the test-build.sh and test-meson-build.sh tools, along side
some environmental variables to test their changes. Contributors in many
cases are either unaware or unable to do this themselves, leading to a
potentially serious situation where they are unknowingly up-streaming
code that breaks the ABI. These breakages are then caught by Travis, but
it is more efficient if this is caught locally before up-streaming.

---
v2: Spelling mistake, corrected spelling of environmental

---
v3:
 - Fix for bug which now allows meson < 0.48.0 to be used
 - Various coding style changes throughout
 - Minor bug fixes to the various meson.build files

Conor Walsh (4):
  devtools: bug fix for gen-abi.sh
  devtools: add generation of compressed abi dump archives
  buildtools: add script to setup abi checks for meson
  build: add abi breakage checks to meson

 buildtools/abi-setup.py     | 104 ++++++++++++++++++++++++++++++
 buildtools/meson.build      |  18 ++++++
 config/meson.build          |  15 +++++
 devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
 devtools/gen-abi.sh         |   6 +-
 drivers/meson.build         |  14 ++++
 lib/meson.build             |  14 ++++
 meson_options.txt           |   2 +
 8 files changed, 293 insertions(+), 5 deletions(-)
 create mode 100755 buildtools/abi-setup.py
 create mode 100755 devtools/gen-abi-tarball.py

-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v3 1/4] devtools: bug fix for gen-abi.sh
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
@ 2020-09-11 16:03     ` Conor Walsh
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
                       ` (4 subsequent siblings)
  5 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-11 16:03 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch fixes a bug with the gen-abi.sh script in devtools.
When ran on an install directory the script would try to generate
.dump files from directories as well as the .so files which is
not correct.
Example error: abidw: gcc/lib/librte_net.so.21.0.p is not a regular file
To rectify this the regex that finds the appropriate .so files has been
changed and the file test has been removed.
This change was tested with the ABI_CHECK Travis checks in DPDK 20.08.
Travis build:
https://travis-ci.com/github/conorwalsh-intel/dpdk/jobs/382812849

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/devtools/gen-abi.sh b/devtools/gen-abi.sh
index c44b0e228..da6fe0556 100755
--- a/devtools/gen-abi.sh
+++ b/devtools/gen-abi.sh
@@ -16,11 +16,7 @@ fi
 dumpdir=$installdir/dump
 rm -rf $dumpdir
 mkdir -p $dumpdir
-for f in $(find $installdir -name "*.so.*"); do
-	if test -L $f; then
-		continue
-	fi
-
+for f in $(find $installdir -name "*.so"); do
 	libname=$(basename $f)
 	abidw --out-file $dumpdir/${libname%.so*}.dump $f
 done
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
@ 2020-09-11 16:03     ` Conor Walsh
  2020-09-14 12:50       ` Burakov, Anatoly
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
                       ` (3 subsequent siblings)
  5 siblings, 1 reply; 53+ messages in thread
From: Conor Walsh @ 2020-09-11 16:03 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds a script that generates a compressed archive
containing .dump files which can be used to perform ABI
breakage checking for the build specified in the parameters.
Invoke using "./gen-abi-tarball.py -t <tag> -a <arch> [-cf <cross-file>]"
 - <tag>: dpdk tag e.g. "v20.11"
 - <arch>: required architecture e.g. "arm" or "x86_64"
 - <cross-file>: configuration file for cross compiling for another
                 system, this flag is not required.
                 e.g. "config/arm/arm64_armv8_linux_gcc"
E.g. "./gen-abi-tarball.py -t latest -a x86_64"
If a compiler is not specified using the CC environmental variable then
the script will default to using gcc.
Using these parameters the script will produce a .tar.gz archive
containing .dump files required to do ABI breakage checking

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100755 devtools/gen-abi-tarball.py

diff --git a/devtools/gen-abi-tarball.py b/devtools/gen-abi-tarball.py
new file mode 100755
index 000000000..06761fca6
--- /dev/null
+++ b/devtools/gen-abi-tarball.py
@@ -0,0 +1,125 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+import argparse
+
+# Get command line arguments
+parser = argparse.ArgumentParser(usage='\rThis script is intended to generate ABI dump tarballs\n'+
+                                       'Supported environmental variables\n'+
+                                       '\t- CC: The required compiler will be determined using this environmental variable.\n')
+parser.add_argument('-t', '--tag', type=str, dest='tag', help='DPDK tag e.g. latest or v20.11')
+parser.add_argument('-cf', '--cross-file', type=str, dest='crosscompile', help='Set the location of a cross compile config')
+parser.add_argument('-a', '--arch', type=str, dest='arch', help='Arch arm or x86_64')
+args = parser.parse_args()
+
+# Get the DPDK tag if not supplied set as latest
+if args.tag:
+    user_tag = args.tag
+else:
+    user_tag = 'latest'
+    print('No tag supplied defaulting to latest')
+
+# Get the cross-compile option
+if args.crosscompile:
+    cross_comp = args.crosscompile
+    if not args.arch:
+        sys.stderr.write('ERROR Arch must be set using -a when using cross compile\n')
+        exit(1)
+    cross_comp = os.path.abspath(cross_comp)
+    cross_comp_meson = '--cross-file '+cross_comp
+else:
+    cross_comp = ''
+    cross_comp_meson = ''
+
+# Get the required system architecture if not supplied set as x86_64
+if args.arch:
+    arch = args.arch
+else:
+    arch = os.popen('uname -m').read().strip()
+    print('No system architecture supplied defaulting to '+arch)
+
+tag = ''
+# If the user did not supply tag or wants latest then get latest tag
+if user_tag == 'latest':
+    # Get latest quarterly build tag from git repo
+    tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk | grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
+else:
+    tag = user_tag
+    # If the user supplied tag is not in the DPDK repo then fail
+    tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk refs/tags/'+tag+' | wc -l').read().strip())
+    if tag_check != 1:
+        sys.stderr.write('ERROR supplied tag does not exist in DPDK repo\n')
+        exit(1)
+
+# Get the specified compiler from system
+comp_env = 'CC'
+if comp_env in os.environ:
+    comp = os.environ[comp_env]
+    comp_default = ''
+else:
+    print('No compiler specified, defaulting to gcc')
+    comp = 'gcc'
+    comp_default = 'CC=gcc'
+
+# Print the configuration to the user
+print('\nSelected Build: '+tag+', Compiler: '+comp+', Architecture: '+arch+', Cross Compile: '+cross_comp)
+
+# Store the base directory script is working from
+baseDir = os.getcwd()
+# Store devtools dir
+devtoolsDir = os.path.abspath(os.path.dirname(os.path.realpath(sys.argv[0])))
+
+# Create directory for DPDK git repo and build
+try:
+    os.mkdir('dump_dpdk')
+except OSError as error:
+    sys.stderr.write('ERROR The dump_dpdk directory could not be created, ensure it does not exist before start\n')
+    exit(1)
+os.chdir('dump_dpdk')
+# Clone DPDK and switch to specified tag
+print('Cloning '+tag+' from DPDK git')
+os.popen('git clone --quiet http://dpdk.org/git/dpdk >/dev/null').read()
+os.chdir('dpdk')
+os.popen('git checkout --quiet '+tag+' >/dev/null').read()
+
+# Create build folder with meson and set debug build and cross compile (if needed)
+print('Configuring Meson')
+os.popen(comp_default+' meson dumpbuild '+cross_comp_meson+' >/dev/null').read()
+os.chdir('dumpbuild')
+os.popen('meson configure -Dbuildtype=debug >/dev/null').read()
+print('Building DPDK . . .')
+#Build DPDK with ninja
+os.popen('ninja >/dev/null').read()
+gccDir = os.getcwd()
+
+# Create directory for abi dump files
+dumpDir = os.path.join(baseDir,tag+'-'+comp+'-'+arch+'-abi_dump')
+try:
+    os.mkdir(dumpDir)
+except OSError as error:
+    sys.stderr.write('ERROR The '+dumpDir+' directory could not be created\n')
+    os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
+    exit(1)
+
+# Create dump files and output to dump directory
+print('Generating ABI dump files')
+genabiout = os.popen(os.path.join(devtoolsDir,'gen-abi.sh')+' '+gccDir).read()
+os.popen('cp dump/* '+dumpDir).read()
+
+# Compress the dump directory
+print('Creating Tarball of dump files')
+os.chdir(baseDir)
+origSize = os.popen('du -sh '+dumpDir+' | sed "s/\s.*$//"').read()
+os.popen('tar -czf '+dumpDir.split('/')[-1]+'.tar.gz '+dumpDir.split('/')[-1]+' >/dev/null').read()
+newSize = os.popen('du -sh '+dumpDir+'.tar.gz | sed "s/\s.*$//"').read()
+
+# Remove all temporary directories
+print('Cleaning up temporary directories')
+os.popen('rm -rf '+dumpDir).read()
+os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
+
+#Print output of the script to the user
+print('\nDump of DPDK ABI '+tag+' is available in '+dumpDir.split('/')[-1]+'.tar.gz (Original Size: '+origSize.strip()+', Compressed Size:'+newSize.strip()+')\n')
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v3 3/4] buildtools: add script to setup abi checks for meson
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-09-11 16:03     ` Conor Walsh
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 4/4] build: add abi breakage checks to meson Conor Walsh
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-11 16:03 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds a script that is intended to be invoked by meson to
do the required setup for performing ABI breakage checks at build time.
The required ABI dump archives can come from several sources including
being generated at build time or prebuilt archives can be pulled from a
remote http location or local directory.
Invoke using "./abi-setup.py -t <tag> -d <dpdk_source_path>"
 - <tag>: dpdk tag e.g. "v20.11"
 - <dpdk_source_path>: path to dpdk source directory
E.g. "./abi-setup.py -t v20.08 -d /root/dpdk"
As this script is intended to be run by meson during a build
some options can be specified by environmental variables:
 - DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the
   systems dump directories.
 - CC: The required compiler will be determined using this
   environmental variable
 - DPDK_ABI_TAR_URI: Can be used to specify a location that the script
   can pull prebuilt or cached dump archives from. This can be a remote
   http location or a local directory
After the script has setup an appropriate ABI dump directory using one of
the multiple methods available to it, it will print the location of this
directory to the command line.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/abi-setup.py | 104 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100755 buildtools/abi-setup.py

diff --git a/buildtools/abi-setup.py b/buildtools/abi-setup.py
new file mode 100755
index 000000000..3bdef4925
--- /dev/null
+++ b/buildtools/abi-setup.py
@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+import sys
+import os
+import argparse
+
+# Get command line arguments
+parser = argparse.ArgumentParser(usage='\rThis script is intended to setup ABI dumps for meson to perform ABI checks\n'+
+                                       'Supported environmental variables\n'+
+                                       '\t- DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the systems dump directories.\n'+
+                                       '\t- CC: The required compiler will be determined using this environmental variable.\n'+
+                                       '\t- DPDK_ABI_TAR_URI: Can be used to specify a location that the script can pull prebuilt or cached dump archives from. This can be a remote http location or a local directory.\n')
+parser.add_argument('-t', '--tag', dest='tag', type=str, help='DPDK tag e.g. latest or v20.11')
+parser.add_argument('-d', '--dpdk', dest='dpdk', type=str, help='Path to DPDK source directory')
+args = parser.parse_args()
+
+# Get the DPDK tag if not supplied set as latest
+if args.tag:
+    user_tag = args.tag
+else:
+    user_tag = 'latest'
+
+tag = ''
+# If the user did not supply tag or wants latest then get latest tag
+if user_tag == 'latest':
+    # Get latest quarterly build tag from git repo
+    tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk | grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
+else:
+    tag = user_tag
+    # If the user supplied tag does not exist then fail
+    tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk refs/tags/'+tag+' | wc -l').read().strip())
+    if tag_check != 1:
+        sys.stderr.write('ERROR supplied tag does not exist in DPDK repo\n')
+        exit(1)
+
+# Get the specified compiler from system
+comp_env = 'CC'
+if comp_env in os.environ:
+    comp = os.environ[comp_env]
+else:
+    comp = 'gcc'
+
+# Get the systems architecture
+arch = os.popen('uname -m').read().strip()
+
+# Get devtools path
+devtools_path = ''
+if args.dpdk:
+    devtools_path = os.path.abspath(os.path.join(args.dpdk,'devtools'))
+else:
+    sys.stderr.write('ERROR DPDK source directory must be specified\n')
+    exit(1)
+
+# Get the abi dumps folder from args or env fail if none supplied
+abi_folder = ''
+abi_env = 'DPDK_ABI_DUMPS_PATH'
+if abi_env in os.environ:
+    abi_folder = os.path.abspath(os.environ[abi_env])
+else:
+    abi_folder = os.path.abspath(os.path.join(args.dpdk,'abi_dumps'))
+
+# If the directory doesn't exist create it and add a README to explain what it does
+if not os.path.exists(abi_folder):
+    os.makedirs(abi_folder)
+    f=open(abi_folder+'/README','w+')
+    f.write('This directory has been setup to contain the ABI dump folders needed to perform ABI checks\n')
+    f.write('Directories here must be in the format {DPDK Tag}-{Compiler ID}-{Architecture}-abi_dump\n')
+    f.write('e.g. v20.11-gcc-x86_64-abi_dump\n')
+    f.write('Directories that do not use this format will not be picked up by the meson ABI checks\n')
+    f.write('This directory is managed automatically unless desired by the user\n')
+    f.close()
+
+# Move to abi folder
+os.chdir(abi_folder)
+abi_dump=tag+'-'+comp+'-'+arch+'-abi_dump'
+# Download and untar abi dump if not present
+if not os.path.exists(abi_dump):
+    # Check DPDK_ABI_TAR_URI for the location of the tarballs local or web
+    tar_uri_env = 'DPDK_ABI_TAR_URI'
+    if tar_uri_env in os.environ:
+        abi_tar_uri = os.environ[tar_uri_env]
+        if abi_tar_uri.startswith('http'):
+            # Wget the required tarball
+            os.popen('wget '+os.path.join(abi_tar_uri,abi_dump)+'.tar.gz >/dev/null 2>&1').read()
+        else:
+            abi_tar_uri = os.path.abspath(abi_tar_uri)
+            os.popen('cp '+os.path.join(abi_tar_uri,abi_dump)+'.tar.gz . >/dev/null 2>&1').read()
+    # Check tarball was downloaded
+    if os.path.isfile(abi_dump+'.tar.gz'):
+        os.popen('tar -xzf '+abi_dump+'.tar.gz >/dev/null 2>&1').read()
+        os.popen('rm -rf '+abi_dump+'.tar.gz').read()
+    # If the tarball was not found then generate it
+    else:
+        os.popen(os.path.join(devtools_path,'gen-abi-tarball.py')+' -t '+tag+' -a '+arch+' >/dev/null').read()
+        if not os.path.isfile(abi_dump+'.tar.gz'):
+            sys.stderr.write('ERROR ABI check generation failed\n')
+            exit(1)
+        os.popen('tar -xzf '+abi_dump+'.tar.gz >/dev/null 2>&1').read()
+        os.popen('rm -rf '+abi_dump+'.tar.gz').read()
+
+# Tell user where specified directory is
+print(os.path.abspath(abi_dump))
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v3 4/4] build: add abi breakage checks to meson
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
                       ` (2 preceding siblings ...)
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
@ 2020-09-11 16:03     ` Conor Walsh
  2020-09-14  8:08     ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Thomas Monjalon
  2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
  5 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-11 16:03 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, Conor Walsh

This patch adds the ability to run ABI breakage checks to meson.
To do this the developer needs to set the meson build type to debug and
set the version of DPDK that they want to check the ABI against.
The option "abi_checks" has been added to meson for this, the option
accepts DPDK tags e.g. "latest" or "v20.11".
Example meson command: "meson -Dbuildtype=debug -Dabi_checks=v20.11 build"
When the build is done using ninja the ABI checks will be performed
if any breakages are present the build will fail.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/meson.build | 18 ++++++++++++++++++
 config/meson.build     | 15 +++++++++++++++
 drivers/meson.build    | 14 ++++++++++++++
 lib/meson.build        | 14 ++++++++++++++
 meson_options.txt      |  2 ++
 5 files changed, 63 insertions(+)

diff --git a/buildtools/meson.build b/buildtools/meson.build
index 04808dabc..c3ee69a44 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -17,3 +17,21 @@ else
 endif
 map_to_win_cmd = py3 + files('map_to_win.py')
 sphinx_wrapper = py3 + files('call-sphinx-build.py')
+
+abi_check_version = get_option('abi_checks')
+check_abi = (abi_check_version != '')
+
+abi_dir = ''
+# If abi checks enabled setup abi dump directory
+if check_abi
+	message('ABI checks are being setup. This could take several minutes')
+	setup_run = run_command('abi-setup.py', '-t', abi_check_version, '-d', meson.source_root())
+	# Check if error returned from script
+	if setup_run.returncode() != 0
+		setup_err = setup_run.stderr().strip()
+		error('ABI checks setup script returned an error: ' + setup_err)
+	endif
+	abi_dir = setup_run.stdout().strip()
+endif
+abidiff = find_program('abidiff', required: check_abi)
+abignore = files('../devtools/libabigail.abignore')
diff --git a/config/meson.build b/config/meson.build
index 6996e5cbe..79b90f2cc 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -47,6 +47,21 @@ else
 	dpdk_conf.set('RTE_VER_RELEASE', 99)
 endif
 
+# abi checks cannot be run on windows
+if is_windows and check_abi
+	error('ABI checks cannot be run on windows')
+endif
+# abi checks can only be run on a debug build
+# meson <0.48 does not support get_option('debug')
+if meson.version().version_compare('>=0.48')
+	debug_enabled = get_option('debug')
+else
+	debug_enabled = get_option('buildtype').startswith('debug')
+endif
+if check_abi and not debug_enabled
+	error('Build type must have debug symbols when abi_checks are enabled')
+endif
+
 pmd_subdir_opt = get_option('drivers_install_subdir')
 if pmd_subdir_opt.contains('<VERSION>')
 	pmd_subdir_opt = abi_version.join(pmd_subdir_opt.split('<VERSION>'))
diff --git a/drivers/meson.build b/drivers/meson.build
index 5f9526557..b24a875da 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -208,6 +208,20 @@ foreach subpath:subdirs
 					include_directories: includes,
 					dependencies: static_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/lib' + lib_name + '.dump'
+			if check_abi and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target('lib' + lib_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							'@INPUT@'],
+					      input: [dump_name, shared_lib],
+					      output: 'lib' + lib_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_drivers += static_lib
 
 			set_variable('shared_@0@'.format(lib_name), shared_dep)
diff --git a/lib/meson.build b/lib/meson.build
index 3852c0156..0d9325b90 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -190,6 +190,20 @@ foreach l:libraries
 					include_directories: includes,
 					dependencies: shared_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/' + dir_name + '.dump'
+			if check_abi and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target(dir_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							'@INPUT@'],
+					      input: [dump_name, shared_lib],
+					      output: dir_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 		endif # sources.length() > 0
diff --git a/meson_options.txt b/meson_options.txt
index 9bf18ab6b..26ac48f45 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('abi_checks', type: 'string', value: '',
+    description: 'Enable abi compatibility checks to run during the build. This requires debug build to be enabled. Input is latest or git tag e.g. v20.11')
 option('armv8_crypto_dir', type: 'string', value: '',
 	description: 'path to the armv8_crypto library installation directory')
 option('disable_drivers', type: 'string', value: '',
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
                       ` (3 preceding siblings ...)
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 4/4] build: add abi breakage checks to meson Conor Walsh
@ 2020-09-14  8:08     ` Thomas Monjalon
  2020-09-14  8:30       ` Kinsella, Ray
  2020-09-14  9:34       ` Kinsella, Ray
  2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
  5 siblings, 2 replies; 53+ messages in thread
From: Thomas Monjalon @ 2020-09-14  8:08 UTC (permalink / raw)
  To: Conor Walsh
  Cc: dev, david.marchand, ray.kinsella, nhorman, aconole,
	maicolgabriel, bruce.richardson

Hi,

> This patchset allows developers to check ABI breakages during build
> time.
> Currently checking that the DPDK ABI has not changed before up-streaming
> code is not intuitive. The current method, requires the contributor to
> use either the test-build.sh and test-meson-build.sh tools, along side

The contributor *MUST* test compilation with test-meson-build.sh in any case.

> some environmental variables to test their changes. Contributors in many

It is just one variable to add to a file:
	export DPDK_ABI_REF_VERSION=v20.11
in ~/.config/dpdk/devel.config or dpdk/.develconfig

> cases are either unaware or unable to do this themselves, leading to a
> potentially serious situation where they are unknowingly up-streaming
> code that breaks the ABI. These breakages are then caught by Travis, but
> it is more efficient if this is caught locally before up-streaming.

I think you are proposing a complex solution to a non-issue.

And I don't understand how your method is more straight-forward
for the user, given this statement in your last patch:
"
This patch adds the ability to run ABI breakage checks to meson.
To do this the developer needs to set the meson build type to debug and
set the version of DPDK that they want to check the ABI against.
"



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson
  2020-09-14  8:08     ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Thomas Monjalon
@ 2020-09-14  8:30       ` Kinsella, Ray
  2020-09-14  9:34       ` Kinsella, Ray
  1 sibling, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-09-14  8:30 UTC (permalink / raw)
  To: dev

Predictably perhaps, I disagree.

On 14/09/2020 09:08, Thomas Monjalon wrote:
> Hi,
> 
>> This patchset allows developers to check ABI breakages during build
>> time.
>> Currently checking that the DPDK ABI has not changed before up-streaming
>> code is not intuitive. The current method, requires the contributor to
>> use either the test-build.sh and test-meson-build.sh tools, along side
> 
> The contributor *MUST* test compilation with test-meson-build.sh in any case.

So I guess, you need to ask what the "user interface" is for DPDK.
The reality is that is meson/ninja (or previously make) tooling is that interface.
as it is most other places, dev's expect a familiar build system.

> 
>> some environmental variables to test their changes. Contributors in many
> 
> It is just one variable to add to a file:
> 	export DPDK_ABI_REF_VERSION=v20.11
> in ~/.config/dpdk/devel.config or dpdk/.develconfig

It's a documentation heavy, non-obvious process.
We need to consider UX, to make it as _easy_ as possible, 
integrated with the build system and automated. 

> 
>> cases are either unaware or unable to do this themselves, leading to a
>> potentially serious situation where they are unknowingly up-streaming
>> code that breaks the ABI. These breakages are then caught by Travis, but
>> it is more efficient if this is caught locally before up-streaming.
> 
> I think you are proposing a complex solution to a non-issue.

This is puzzling, complex compared to running a separate script to meson/ninja, 
and requiring that esoteric environment variables are set?

> 
> And I don't understand how your method is more straight-forward
> for the user, given this statement in your last patch:
> "
> This patch adds the ability to run ABI breakage checks to meson.
> To do this the developer needs to set the meson build type to debug and
> set the version of DPDK that they want to check the ABI against.
> "

The developer can set the abi-check option to latest, and the build system
will take care of the rest. The system even caches the debug symbols, so they 
don't need to be generated locally. 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson
  2020-09-14  8:08     ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Thomas Monjalon
  2020-09-14  8:30       ` Kinsella, Ray
@ 2020-09-14  9:34       ` Kinsella, Ray
  1 sibling, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-09-14  9:34 UTC (permalink / raw)
  To: Thomas Monjalon, Conor Walsh
  Cc: dev, david.marchand, ray.kinsella, nhorman, aconole,
	maicolgabriel, bruce.richardson

(apologies for re-sending, I missed the CC list the 1st time). 

Predictably perhaps, I disagree.

On 14/09/2020 09:08, Thomas Monjalon wrote:
> Hi,
> 
>> This patchset allows developers to check ABI breakages during build
>> time.
>> Currently checking that the DPDK ABI has not changed before up-streaming
>> code is not intuitive. The current method, requires the contributor to
>> use either the test-build.sh and test-meson-build.sh tools, along side
> 
> The contributor *MUST* test compilation with test-meson-build.sh in any case.
> 

So I guess, you need to ask what the "user interface" is for DPDK.
The reality is that is meson/ninja (or previously make) tooling is that interface.
as it is most other places, dev's expect a familiar build system.

>> some environmental variables to test their changes. Contributors in many
> 
> It is just one variable to add to a file:
> 	export DPDK_ABI_REF_VERSION=v20.11
> in ~/.config/dpdk/devel.config or dpdk/.develconfig

It's a documentation heavy, non-obvious process.
We need to consider UX, to make it as _easy_ as possible, 
integrated with the build system and automated. 

> 
>> cases are either unaware or unable to do this themselves, leading to a
>> potentially serious situation where they are unknowingly up-streaming
>> code that breaks the ABI. These breakages are then caught by Travis, but
>> it is more efficient if this is caught locally before up-streaming.

This is puzzling, complex compared to running a separate script to meson/ninja, 
and requiring that esoteric environment variables are set?

> I think you are proposing a complex solution to a non-issue.
> 
> And I don't understand how your method is more straight-forward
> for the user, given this statement in your last patch:
> "
> This patch adds the ability to run ABI breakage checks to meson.
> To do this the developer needs to set the meson build type to debug and
> set the version of DPDK that they want to check the ABI against.
> "
> 

The developer can set the abi-check option to latest, and the build system
will take care of the rest. The system even caches the debug symbols, so they 
don't need to be generated locally. 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives
  2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-09-14 12:50       ` Burakov, Anatoly
  2020-09-15 14:35         ` Aaron Conole
  0 siblings, 1 reply; 53+ messages in thread
From: Burakov, Anatoly @ 2020-09-14 12:50 UTC (permalink / raw)
  To: Conor Walsh, dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson

On 11-Sep-20 5:03 PM, Conor Walsh wrote:
> This patch adds a script that generates a compressed archive
> containing .dump files which can be used to perform ABI
> breakage checking for the build specified in the parameters.
> Invoke using "./gen-abi-tarball.py -t <tag> -a <arch> [-cf <cross-file>]"
>   - <tag>: dpdk tag e.g. "v20.11"
>   - <arch>: required architecture e.g. "arm" or "x86_64"
>   - <cross-file>: configuration file for cross compiling for another
>                   system, this flag is not required.
>                   e.g. "config/arm/arm64_armv8_linux_gcc"
> E.g. "./gen-abi-tarball.py -t latest -a x86_64"
> If a compiler is not specified using the CC environmental variable then
> the script will default to using gcc.
> Using these parameters the script will produce a .tar.gz archive
> containing .dump files required to do ABI breakage checking
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> ---

Just a general comment: this script looks like it's bash translated to 
python. If you're going to do that, you might as well just write it in bash?

>   devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 125 insertions(+)
>   create mode 100755 devtools/gen-abi-tarball.py
> 
> diff --git a/devtools/gen-abi-tarball.py b/devtools/gen-abi-tarball.py
> new file mode 100755
> index 000000000..06761fca6
> --- /dev/null
> +++ b/devtools/gen-abi-tarball.py
> @@ -0,0 +1,125 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Intel Corporation
> +
> +import sys
> +import os
> +import argparse
> +

It is preferred to wrap executable code in "if __name__ == 
"__main__"..., helps with importing code while avoiding executing it on 
import.

> +# Get command line arguments
> +parser = argparse.ArgumentParser(usage='\rThis script is intended to generate ABI dump tarballs\n'+
> +                                       'Supported environmental variables\n'+
> +                                       '\t- CC: The required compiler will be determined using this environmental variable.\n')
> +parser.add_argument('-t', '--tag', type=str, dest='tag', help='DPDK tag e.g. latest or v20.11')
> +parser.add_argument('-cf', '--cross-file', type=str, dest='crosscompile', help='Set the location of a cross compile config')
> +parser.add_argument('-a', '--arch', type=str, dest='arch', help='Arch arm or x86_64')
> +args = parser.parse_args()
> +
> +# Get the DPDK tag if not supplied set as latest
> +if args.tag:
> +    user_tag = args.tag
> +else:
> +    user_tag = 'latest'
> +    print('No tag supplied defaulting to latest')

There's a "default" option for arguments.

> +
> +# Get the cross-compile option
> +if args.crosscompile:
> +    cross_comp = args.crosscompile
> +    if not args.arch:
> +        sys.stderr.write('ERROR Arch must be set using -a when using cross compile\n')
> +        exit(1)
> +    cross_comp = os.path.abspath(cross_comp)
> +    cross_comp_meson = '--cross-file '+cross_comp
> +else:
> +    cross_comp = ''
> +    cross_comp_meson = ''
> +
> +# Get the required system architecture if not supplied set as x86_64
> +if args.arch:
> +    arch = args.arch
> +else:
> +    arch = os.popen('uname -m').read().strip()

There's a built-in python library for this, i think it's called "platform".

> +    print('No system architecture supplied defaulting to '+arch)
> +
> +tag = ''
> +# If the user did not supply tag or wants latest then get latest tag
> +if user_tag == 'latest':
> +    # Get latest quarterly build tag from git repo
> +    tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk | grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
> +else:
> +    tag = user_tag
> +    # If the user supplied tag is not in the DPDK repo then fail
> +    tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk refs/tags/'+tag+' | wc -l').read().strip())
> +    if tag_check != 1:
> +        sys.stderr.write('ERROR supplied tag does not exist in DPDK repo\n')
> +        exit(1)
> +
> +# Get the specified compiler from system
> +comp_env = 'CC'
> +if comp_env in os.environ:
> +    comp = os.environ[comp_env]
> +    comp_default = ''
> +else:
> +    print('No compiler specified, defaulting to gcc')
> +    comp = 'gcc'
> +    comp_default = 'CC=gcc'
> +
> +# Print the configuration to the user
> +print('\nSelected Build: '+tag+', Compiler: '+comp+', Architecture: '+arch+', Cross Compile: '+cross_comp)

Here and in similar places: please don't use string concatenation, use 
string formatting instead.

> +
> +# Store the base directory script is working from
> +baseDir = os.getcwd()
> +# Store devtools dir
> +devtoolsDir = os.path.abspath(os.path.dirname(os.path.realpath(sys.argv[0])))
> +
> +# Create directory for DPDK git repo and build
> +try:
> +    os.mkdir('dump_dpdk')
> +except OSError as error:
> +    sys.stderr.write('ERROR The dump_dpdk directory could not be created, ensure it does not exist before start\n')
> +    exit(1)
> +os.chdir('dump_dpdk')
> +# Clone DPDK and switch to specified tag
> +print('Cloning '+tag+' from DPDK git')
> +os.popen('git clone --quiet http://dpdk.org/git/dpdk >/dev/null').read()
> +os.chdir('dpdk')
> +os.popen('git checkout --quiet '+tag+' >/dev/null').read()
> +
> +# Create build folder with meson and set debug build and cross compile (if needed)
> +print('Configuring Meson')
> +os.popen(comp_default+' meson dumpbuild '+cross_comp_meson+' >/dev/null').read()

You do this os.popen(something > /dev/bull).read() quite a lot, maybe 
put it into a function? Also, since you're discarding the data anyway - 
why os.popen().read() instead of os.system()?

> +os.chdir('dumpbuild')
> +os.popen('meson configure -Dbuildtype=debug >/dev/null').read()
> +print('Building DPDK . . .')
> +#Build DPDK with ninja
> +os.popen('ninja >/dev/null').read()
> +gccDir = os.getcwd()
> +
> +# Create directory for abi dump files
> +dumpDir = os.path.join(baseDir,tag+'-'+comp+'-'+arch+'-abi_dump')
> +try:
> +    os.mkdir(dumpDir)
> +except OSError as error:
> +    sys.stderr.write('ERROR The '+dumpDir+' directory could not be created\n')
> +    os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
> +    exit(1)
> +
> +# Create dump files and output to dump directory
> +print('Generating ABI dump files')
> +genabiout = os.popen(os.path.join(devtoolsDir,'gen-abi.sh')+' '+gccDir).read()
> +os.popen('cp dump/* '+dumpDir).read()
> +
> +# Compress the dump directory
> +print('Creating Tarball of dump files')
> +os.chdir(baseDir)
> +origSize = os.popen('du -sh '+dumpDir+' | sed "s/\s.*$//"').read()
> +os.popen('tar -czf '+dumpDir.split('/')[-1]+'.tar.gz '+dumpDir.split('/')[-1]+' >/dev/null').read()
> +newSize = os.popen('du -sh '+dumpDir+'.tar.gz | sed "s/\s.*$//"').read()
> +
> +# Remove all temporary directories
> +print('Cleaning up temporary directories')
> +os.popen('rm -rf '+dumpDir).read()
> +os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
> +
> +#Print output of the script to the user
> +print('\nDump of DPDK ABI '+tag+' is available in '+dumpDir.split('/')[-1]+'.tar.gz (Original Size: '+origSize.strip()+', Compressed Size:'+newSize.strip()+')\n')
> 


-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives
  2020-09-14 12:50       ` Burakov, Anatoly
@ 2020-09-15 14:35         ` Aaron Conole
  2020-09-15 14:49           ` Walsh, Conor
  0 siblings, 1 reply; 53+ messages in thread
From: Aaron Conole @ 2020-09-15 14:35 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Conor Walsh, dev, david.marchand, ray.kinsella, nhorman,
	maicolgabriel, thomas, bruce.richardson

"Burakov, Anatoly" <anatoly.burakov@intel.com> writes:

> On 11-Sep-20 5:03 PM, Conor Walsh wrote:
>> This patch adds a script that generates a compressed archive
>> containing .dump files which can be used to perform ABI
>> breakage checking for the build specified in the parameters.
>> Invoke using "./gen-abi-tarball.py -t <tag> -a <arch> [-cf <cross-file>]"
>>   - <tag>: dpdk tag e.g. "v20.11"
>>   - <arch>: required architecture e.g. "arm" or "x86_64"
>>   - <cross-file>: configuration file for cross compiling for another
>>                   system, this flag is not required.
>>                   e.g. "config/arm/arm64_armv8_linux_gcc"
>> E.g. "./gen-abi-tarball.py -t latest -a x86_64"
>> If a compiler is not specified using the CC environmental variable then
>> the script will default to using gcc.
>> Using these parameters the script will produce a .tar.gz archive
>> containing .dump files required to do ABI breakage checking
>>
>> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
>> ---
>
> Just a general comment: this script looks like it's bash translated to
> python. If you're going to do that, you might as well just write it in
> bash?
>
>>   devtools/gen-abi-tarball.py | 125 ++++++++++++++++++++++++++++++++++++
>>   1 file changed, 125 insertions(+)
>>   create mode 100755 devtools/gen-abi-tarball.py
>>
>> diff --git a/devtools/gen-abi-tarball.py b/devtools/gen-abi-tarball.py
>> new file mode 100755
>> index 000000000..06761fca6
>> --- /dev/null
>> +++ b/devtools/gen-abi-tarball.py
>> @@ -0,0 +1,125 @@
>> +#!/usr/bin/env python3
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright(c) 2020 Intel Corporation
>> +
>> +import sys
>> +import os
>> +import argparse
>> +
>
> It is preferred to wrap executable code in "if __name__ ==
> "__main__"..., helps with importing code while avoiding executing it
> on import.

I wonder, since DPDK is supporting windows, we should prefer python to
bash?  I do prefer to use bash because I don't use windows, but maybe
that is a good reason?

>> +# Get command line arguments
>> +parser = argparse.ArgumentParser(usage='\rThis script is intended to generate ABI dump tarballs\n'+
>> +                                       'Supported environmental variables\n'+
>> + '\t- CC: The required compiler will be determined using this
>> environmental variable.\n')
>> +parser.add_argument('-t', '--tag', type=str, dest='tag', help='DPDK tag e.g. latest or v20.11')
>> +parser.add_argument('-cf', '--cross-file', type=str,
>> dest='crosscompile', help='Set the location of a cross compile
>> config')
>> +parser.add_argument('-a', '--arch', type=str, dest='arch', help='Arch arm or x86_64')
>> +args = parser.parse_args()
>> +
>> +# Get the DPDK tag if not supplied set as latest
>> +if args.tag:
>> +    user_tag = args.tag
>> +else:
>> +    user_tag = 'latest'
>> +    print('No tag supplied defaulting to latest')
>
> There's a "default" option for arguments.
>
>> +
>> +# Get the cross-compile option
>> +if args.crosscompile:
>> +    cross_comp = args.crosscompile
>> +    if not args.arch:
>> +        sys.stderr.write('ERROR Arch must be set using -a when using cross compile\n')
>> +        exit(1)
>> +    cross_comp = os.path.abspath(cross_comp)
>> +    cross_comp_meson = '--cross-file '+cross_comp
>> +else:
>> +    cross_comp = ''
>> +    cross_comp_meson = ''
>> +
>> +# Get the required system architecture if not supplied set as x86_64
>> +if args.arch:
>> +    arch = args.arch
>> +else:
>> +    arch = os.popen('uname -m').read().strip()
>
> There's a built-in python library for this, i think it's called "platform".
>
>> +    print('No system architecture supplied defaulting to '+arch)
>> +
>> +tag = ''
>> +# If the user did not supply tag or wants latest then get latest tag
>> +if user_tag == 'latest':
>> +    # Get latest quarterly build tag from git repo
>> + tag = os.popen('git ls-remote --tags http://dpdk.org/git/dpdk |
>> grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
>> +else:
>> +    tag = user_tag
>> +    # If the user supplied tag is not in the DPDK repo then fail
>> + tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk
>> refs/tags/'+tag+' | wc -l').read().strip())
>> +    if tag_check != 1:
>> +        sys.stderr.write('ERROR supplied tag does not exist in DPDK repo\n')
>> +        exit(1)
>> +
>> +# Get the specified compiler from system
>> +comp_env = 'CC'
>> +if comp_env in os.environ:
>> +    comp = os.environ[comp_env]
>> +    comp_default = ''
>> +else:
>> +    print('No compiler specified, defaulting to gcc')
>> +    comp = 'gcc'
>> +    comp_default = 'CC=gcc'
>> +
>> +# Print the configuration to the user
>> +print('\nSelected Build: '+tag+', Compiler: '+comp+', Architecture:
>> '+arch+', Cross Compile: '+cross_comp)
>
> Here and in similar places: please don't use string concatenation, use
> string formatting instead.
>
>> +
>> +# Store the base directory script is working from
>> +baseDir = os.getcwd()
>> +# Store devtools dir
>> +devtoolsDir = os.path.abspath(os.path.dirname(os.path.realpath(sys.argv[0])))
>> +
>> +# Create directory for DPDK git repo and build
>> +try:
>> +    os.mkdir('dump_dpdk')
>> +except OSError as error:
>> + sys.stderr.write('ERROR The dump_dpdk directory could not be
>> created, ensure it does not exist before start\n')
>> +    exit(1)
>> +os.chdir('dump_dpdk')
>> +# Clone DPDK and switch to specified tag
>> +print('Cloning '+tag+' from DPDK git')
>> +os.popen('git clone --quiet http://dpdk.org/git/dpdk >/dev/null').read()
>> +os.chdir('dpdk')
>> +os.popen('git checkout --quiet '+tag+' >/dev/null').read()
>> +
>> +# Create build folder with meson and set debug build and cross compile (if needed)
>> +print('Configuring Meson')
>> +os.popen(comp_default+' meson dumpbuild '+cross_comp_meson+' >/dev/null').read()
>
> You do this os.popen(something > /dev/bull).read() quite a lot, maybe
> put it into a function? Also, since you're discarding the data anyway
> - 
> why os.popen().read() instead of os.system()?
>
>> +os.chdir('dumpbuild')
>> +os.popen('meson configure -Dbuildtype=debug >/dev/null').read()
>> +print('Building DPDK . . .')
>> +#Build DPDK with ninja
>> +os.popen('ninja >/dev/null').read()
>> +gccDir = os.getcwd()
>> +
>> +# Create directory for abi dump files
>> +dumpDir = os.path.join(baseDir,tag+'-'+comp+'-'+arch+'-abi_dump')
>> +try:
>> +    os.mkdir(dumpDir)
>> +except OSError as error:
>> +    sys.stderr.write('ERROR The '+dumpDir+' directory could not be created\n')
>> +    os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
>> +    exit(1)
>> +
>> +# Create dump files and output to dump directory
>> +print('Generating ABI dump files')
>> +genabiout = os.popen(os.path.join(devtoolsDir,'gen-abi.sh')+' '+gccDir).read()
>> +os.popen('cp dump/* '+dumpDir).read()
>> +
>> +# Compress the dump directory
>> +print('Creating Tarball of dump files')
>> +os.chdir(baseDir)
>> +origSize = os.popen('du -sh '+dumpDir+' | sed "s/\s.*$//"').read()
>> +os.popen('tar -czf '+dumpDir.split('/')[-1]+'.tar.gz '+dumpDir.split('/')[-1]+' >/dev/null').read()
>> +newSize = os.popen('du -sh '+dumpDir+'.tar.gz | sed "s/\s.*$//"').read()
>> +
>> +# Remove all temporary directories
>> +print('Cleaning up temporary directories')
>> +os.popen('rm -rf '+dumpDir).read()
>> +os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
>> +
>> +#Print output of the script to the user
>> +print('\nDump of DPDK ABI '+tag+' is available in
>> '+dumpDir.split('/')[-1]+'.tar.gz (Original Size:
>> '+origSize.strip()+', Compressed Size:'+newSize.strip()+')\n')
>>


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives
  2020-09-15 14:35         ` Aaron Conole
@ 2020-09-15 14:49           ` Walsh, Conor
  0 siblings, 0 replies; 53+ messages in thread
From: Walsh, Conor @ 2020-09-15 14:49 UTC (permalink / raw)
  To: Aaron Conole, Burakov, Anatoly
  Cc: dev, david.marchand, Kinsella, Ray, nhorman, maicolgabriel,
	thomas, Richardson, Bruce

> >> This patch adds a script that generates a compressed archive
> >> containing .dump files which can be used to perform ABI breakage
> >> checking for the build specified in the parameters.
> >> Invoke using "./gen-abi-tarball.py -t <tag> -a <arch> [-cf <cross-file>]"
> >>   - <tag>: dpdk tag e.g. "v20.11"
> >>   - <arch>: required architecture e.g. "arm" or "x86_64"
> >>   - <cross-file>: configuration file for cross compiling for another
> >>                   system, this flag is not required.
> >>                   e.g. "config/arm/arm64_armv8_linux_gcc"
> >> E.g. "./gen-abi-tarball.py -t latest -a x86_64"
> >> If a compiler is not specified using the CC environmental variable
> >> then the script will default to using gcc.
> >> Using these parameters the script will produce a .tar.gz archive
> >> containing .dump files required to do ABI breakage checking
> >>
> >> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> >> ---
> >
> > Just a general comment: this script looks like it's bash translated to
> > python. If you're going to do that, you might as well just write it in
> > bash?

I am currently reworking this script to make it less BASH like and use more Python native functions and modules.
Thank you for the feedback Anatoly, it was very helpful.

> >
> >>   devtools/gen-abi-tarball.py | 125
> ++++++++++++++++++++++++++++++++++++
> >>   1 file changed, 125 insertions(+)
> >>   create mode 100755 devtools/gen-abi-tarball.py
> >>
> >> diff --git a/devtools/gen-abi-tarball.py
> >> b/devtools/gen-abi-tarball.py new file mode 100755 index
> >> 000000000..06761fca6
> >> --- /dev/null
> >> +++ b/devtools/gen-abi-tarball.py
> >> @@ -0,0 +1,125 @@
> >> +#!/usr/bin/env python3
> >> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Intel
> >> +Corporation
> >> +
> >> +import sys
> >> +import os
> >> +import argparse
> >> +
> >
> > It is preferred to wrap executable code in "if __name__ ==
> > "__main__"..., helps with importing code while avoiding executing it
> > on import.
> 
> I wonder, since DPDK is supporting windows, we should prefer python to
> bash?  I do prefer to use bash because I don't use windows, but maybe that
> is a good reason?

This patchset does not support Windows currently but if it did in the future having the scripts written in Python would probably make this easier to implement.

> 
> >> +# Get command line arguments
> >> +parser = argparse.ArgumentParser(usage='\rThis script is intended to
> generate ABI dump tarballs\n'+
> >> +                                       'Supported environmental
> >> +variables\n'+
> >> + '\t- CC: The required compiler will be determined using this
> >> environmental variable.\n')
> >> +parser.add_argument('-t', '--tag', type=str, dest='tag', help='DPDK
> >> +tag e.g. latest or v20.11') parser.add_argument('-cf',
> >> +'--cross-file', type=str,
> >> dest='crosscompile', help='Set the location of a cross compile
> >> config')
> >> +parser.add_argument('-a', '--arch', type=str, dest='arch',
> >> +help='Arch arm or x86_64') args = parser.parse_args()
> >> +
> >> +# Get the DPDK tag if not supplied set as latest if args.tag:
> >> +    user_tag = args.tag
> >> +else:
> >> +    user_tag = 'latest'
> >> +    print('No tag supplied defaulting to latest')
> >
> > There's a "default" option for arguments.
> >
> >> +
> >> +# Get the cross-compile option
> >> +if args.crosscompile:
> >> +    cross_comp = args.crosscompile
> >> +    if not args.arch:
> >> +        sys.stderr.write('ERROR Arch must be set using -a when using cross
> compile\n')
> >> +        exit(1)
> >> +    cross_comp = os.path.abspath(cross_comp)
> >> +    cross_comp_meson = '--cross-file '+cross_comp
> >> +else:
> >> +    cross_comp = ''
> >> +    cross_comp_meson = ''
> >> +
> >> +# Get the required system architecture if not supplied set as x86_64
> >> +if args.arch:
> >> +    arch = args.arch
> >> +else:
> >> +    arch = os.popen('uname -m').read().strip()
> >
> > There's a built-in python library for this, i think it's called "platform".
> >
> >> +    print('No system architecture supplied defaulting to '+arch)
> >> +
> >> +tag = ''
> >> +# If the user did not supply tag or wants latest then get latest tag
> >> +if user_tag == 'latest':
> >> +    # Get latest quarterly build tag from git repo  tag =
> >> +os.popen('git ls-remote --tags http://dpdk.org/git/dpdk |
> >> grep -v "rc\|{}" | tail -n 1 | sed "s/.*\///"').read().strip()
> >> +else:
> >> +    tag = user_tag
> >> +    # If the user supplied tag is not in the DPDK repo then fail
> >> +tag_check = int(os.popen('git ls-remote http://dpdk.org/git/dpdk
> >> refs/tags/'+tag+' | wc -l').read().strip())
> >> +    if tag_check != 1:
> >> +        sys.stderr.write('ERROR supplied tag does not exist in DPDK repo\n')
> >> +        exit(1)
> >> +
> >> +# Get the specified compiler from system comp_env = 'CC'
> >> +if comp_env in os.environ:
> >> +    comp = os.environ[comp_env]
> >> +    comp_default = ''
> >> +else:
> >> +    print('No compiler specified, defaulting to gcc')
> >> +    comp = 'gcc'
> >> +    comp_default = 'CC=gcc'
> >> +
> >> +# Print the configuration to the user print('\nSelected Build:
> >> +'+tag+', Compiler: '+comp+', Architecture:
> >> '+arch+', Cross Compile: '+cross_comp)
> >
> > Here and in similar places: please don't use string concatenation, use
> > string formatting instead.
> >
> >> +
> >> +# Store the base directory script is working from baseDir =
> >> +os.getcwd() # Store devtools dir devtoolsDir =
> >> +os.path.abspath(os.path.dirname(os.path.realpath(sys.argv[0])))
> >> +
> >> +# Create directory for DPDK git repo and build
> >> +try:
> >> +    os.mkdir('dump_dpdk')
> >> +except OSError as error:
> >> + sys.stderr.write('ERROR The dump_dpdk directory could not be
> >> created, ensure it does not exist before start\n')
> >> +    exit(1)
> >> +os.chdir('dump_dpdk')
> >> +# Clone DPDK and switch to specified tag print('Cloning '+tag+' from
> >> +DPDK git') os.popen('git clone --quiet http://dpdk.org/git/dpdk
> >> +>/dev/null').read()
> >> +os.chdir('dpdk')
> >> +os.popen('git checkout --quiet '+tag+' >/dev/null').read()
> >> +
> >> +# Create build folder with meson and set debug build and cross
> >> +compile (if needed) print('Configuring Meson')
> >> +os.popen(comp_default+' meson dumpbuild '+cross_comp_meson+'
> >> +>/dev/null').read()
> >
> > You do this os.popen(something > /dev/bull).read() quite a lot, maybe
> > put it into a function? Also, since you're discarding the data anyway
> > -
> > why os.popen().read() instead of os.system()?
> >
> >> +os.chdir('dumpbuild')
> >> +os.popen('meson configure -Dbuildtype=debug >/dev/null').read()
> >> +print('Building DPDK . . .') #Build DPDK with ninja os.popen('ninja
> >> +>/dev/null').read() gccDir = os.getcwd()
> >> +
> >> +# Create directory for abi dump files dumpDir =
> >> +os.path.join(baseDir,tag+'-'+comp+'-'+arch+'-abi_dump')
> >> +try:
> >> +    os.mkdir(dumpDir)
> >> +except OSError as error:
> >> +    sys.stderr.write('ERROR The '+dumpDir+' directory could not be
> created\n')
> >> +    os.popen('rm -rf '+os.path.join(baseDir,'dump_dpdk')).read()
> >> +    exit(1)
> >> +
> >> +# Create dump files and output to dump directory print('Generating
> >> +ABI dump files') genabiout =
> >> +os.popen(os.path.join(devtoolsDir,'gen-abi.sh')+' '+gccDir).read()
> >> +os.popen('cp dump/* '+dumpDir).read()
> >> +
> >> +# Compress the dump directory
> >> +print('Creating Tarball of dump files')
> >> +os.chdir(baseDir)
> >> +origSize = os.popen('du -sh '+dumpDir+' | sed "s/\s.*$//"').read()
> >> +os.popen('tar -czf '+dumpDir.split('/')[-1]+'.tar.gz
> >> +'+dumpDir.split('/')[-1]+' >/dev/null').read() newSize =
> >> +os.popen('du -sh '+dumpDir+'.tar.gz | sed "s/\s.*$//"').read()
> >> +
> >> +# Remove all temporary directories
> >> +print('Cleaning up temporary directories') os.popen('rm -rf
> >> +'+dumpDir).read() os.popen('rm -rf
> >> +'+os.path.join(baseDir,'dump_dpdk')).read()
> >> +
> >> +#Print output of the script to the user print('\nDump of DPDK ABI
> >> +'+tag+' is available in
> >> '+dumpDir.split('/')[-1]+'.tar.gz (Original Size:
> >> '+origSize.strip()+', Compressed Size:'+newSize.strip()+')\n')
> >>

Thanks,
Conor.


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v4 0/4] abi breakage checks for meson
  2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
                       ` (4 preceding siblings ...)
  2020-09-14  8:08     ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Thomas Monjalon
@ 2020-09-18 12:11     ` Conor Walsh
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
                         ` (4 more replies)
  5 siblings, 5 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-18 12:11 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, anatoly.burakov, Conor Walsh

This patchset allows developers to check ABI breakages during build time.
Currently checking that the DPDK ABI has not changed before up-streaming
code is not intuitive. The current method, requires the contributor to
use either the test-build.sh and test-meson-build.sh tools, along side
some environmental variables to test their changes. Contributors in many
cases are either unaware or unable to do this themselves, leading to a
potentially serious situation where they are unknowingly up-streaming
code that breaks the ABI. These breakages are then caught by Travis, but
it is more efficient if this is caught locally before up-streaming.

---
v4:
 - Reworked both Python scripts to use more native Python functions
   and modules.
 - Python scripts are now in line with how other Python scripts in
   DPDK are structured.

v3:
 - Fix for bug which now allows meson < 0.48.0 to be used
 - Various coding style changes throughout
 - Minor bug fixes to the various meson.build files

v2: Spelling mistake, corrected spelling of environmental

Conor Walsh (4):
  devtools: bug fix for gen-abi.sh
  devtools: add generation of compressed abi dump archives
  buildtools: add script to setup abi checks for meson
  build: add abi breakage checks to meson

 buildtools/abi-setup.py     | 141 +++++++++++++++++++++++++++++++++++
 buildtools/meson.build      |  18 +++++
 config/meson.build          |  15 ++++
 devtools/gen-abi-tarball.py | 142 ++++++++++++++++++++++++++++++++++++
 devtools/gen-abi.sh         |   6 +-
 drivers/meson.build         |  14 ++++
 lib/meson.build             |  14 ++++
 meson_options.txt           |   2 +
 8 files changed, 347 insertions(+), 5 deletions(-)
 create mode 100755 buildtools/abi-setup.py
 create mode 100755 devtools/gen-abi-tarball.py

-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v4 1/4] devtools: bug fix for gen-abi.sh
  2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
@ 2020-09-18 12:11       ` Conor Walsh
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
                         ` (3 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-18 12:11 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, anatoly.burakov, Conor Walsh

This patch fixes a bug with the gen-abi.sh script in devtools.
When ran on an install directory the script would try to generate
.dump files from directories as well as the .so files which is
not correct.
Example error: abidw: gcc/lib/librte_net.so.21.0.p is not a regular file
To rectify this the regex that finds the appropriate .so files has been
changed and the file test has been removed.
This change was tested with the ABI_CHECK Travis checks in DPDK 20.08.
Travis build:
https://travis-ci.com/github/conorwalsh-intel/dpdk/jobs/382812849

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/devtools/gen-abi.sh b/devtools/gen-abi.sh
index c44b0e228..da6fe0556 100755
--- a/devtools/gen-abi.sh
+++ b/devtools/gen-abi.sh
@@ -16,11 +16,7 @@ fi
 dumpdir=$installdir/dump
 rm -rf $dumpdir
 mkdir -p $dumpdir
-for f in $(find $installdir -name "*.so.*"); do
-	if test -L $f; then
-		continue
-	fi
-
+for f in $(find $installdir -name "*.so"); do
 	libname=$(basename $f)
 	abidw --out-file $dumpdir/${libname%.so*}.dump $f
 done
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v4 2/4] devtools: add generation of compressed abi dump archives
  2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
@ 2020-09-18 12:11       ` Conor Walsh
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
                         ` (2 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-18 12:11 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, anatoly.burakov, Conor Walsh

This patch adds a script that generates a compressed archive
containing .dump files which can be used to perform ABI
breakage checking for the build specified in the parameters.
Invoke using "./gen-abi-tarball.py [-t <tag>] [-a <arch>]
              [-cf <cross-file>]"
 - <tag>: dpdk tag e.g. "v20.11", default: latest
 - <arch>: required architecture e.g. "arm" or "x86_64",
           default: current system's architecture
 - <cross-file>: configuration file for cross compiling for another
                 system, this flag is not required.
                 e.g. "config/arm/arm64_armv8_linux_gcc", default: None
E.g. "./gen-abi-tarball.py -t latest -a x86_64"
If a compiler is not specified using the CC environmental variable then
the script will default to using gcc.
Using these parameters the script will produce a .tar.gz archive
containing .dump files required to do ABI breakage checking

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi-tarball.py | 142 ++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100755 devtools/gen-abi-tarball.py

diff --git a/devtools/gen-abi-tarball.py b/devtools/gen-abi-tarball.py
new file mode 100755
index 000000000..6c337104e
--- /dev/null
+++ b/devtools/gen-abi-tarball.py
@@ -0,0 +1,142 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+"""
+This Python script generates a compressed archive containing .dump
+files which can be used to perform ABI breakage checking for the
+build specified in the parameters.
+"""
+
+import os
+from os.path import abspath, realpath, dirname, basename, join, getsize
+import sys
+import argparse
+import platform
+import tarfile
+import subprocess
+import shutil
+import tempfile
+
+# Get command line options
+def args_parse():
+    parser = argparse.ArgumentParser(
+            description='This script is intended to generate ABI dump tarballs\n\n'+
+                        'Supported environmental variables:\n'+
+                        '\t- CC: The required compiler will be determined using this environmental variable.\n',
+            formatter_class=argparse.RawTextHelpFormatter)
+    parser.add_argument(
+            '-t', '--tag', type=str, dest='tag',
+            help='DPDK tag e.g. latest or v20.11', default='latest')
+    parser.add_argument(
+            '-cf', '--cross-file', type=str, dest='crosscompile',
+            help='Set the location of a cross compile config')
+    parser.add_argument(
+            '-a', '--arch', type=str, dest='arch',
+            help='Architecture arm or x86_64', default=platform.machine())
+    args = parser.parse_args()
+    return args
+
+# Function to execute git commands
+def call_git(args):
+    args = list(filter(None, args))
+    git_call = subprocess.run(['git'] + args, capture_output=True)
+    if git_call.returncode != 0:
+        print('ERROR Git returned an error', file=sys.stderr)
+        exit(1)
+    return git_call.stdout.decode('utf-8').strip()
+
+# Function to execute commands
+def call_exec(args):
+    args = list(filter(None, args))
+    exec_call = subprocess.run(args, stdout=subprocess.DEVNULL)
+    if exec_call.returncode != 0:
+        print('ERROR Script returned an error', file=sys.stderr)
+        exit(1)
+
+# Get the required git tag
+def get_tag(tag):
+    tags = call_git(['ls-remote', '--tags', 'http://dpdk.org/git/dpdk']).split('\n')
+    tags = [t.split('/')[-1].strip() for t in tags if 'rc' not in t and not t.endswith('{}')]
+    if tag == 'latest':
+        tag = tags[-1]
+    if tag not in tags:
+        print('ERROR supplied tag does not exist in DPDK repo', file=sys.stderr)
+        exit(1)
+    return tag
+
+def main():
+    args = args_parse()
+
+    # Get the cross-compile option
+    cross_comp_meson = [None, None]
+    if args.crosscompile:
+        cross_comp_meson = ['--cross-file', abspath(args.crosscompile)]
+
+    tag = get_tag(args.tag)
+
+    # Get the specified compiler from system
+    if 'CC' in os.environ:
+        comp = os.environ['CC']
+    else:
+        print('No compiler specified in environmental varibles, setting CC=gcc')
+        comp = 'gcc'
+        os.environ['CC'] = 'gcc'
+
+    # Print the configuration to the user
+    print('\nSelected Build: {}, Compiler: {}, Architecture: {}, Cross Compile: {}'.format(tag,comp,args.arch,cross_comp_meson[1]))
+
+    # Store the users working directory
+    baseDir = os.getcwd()
+    # Store devtools dir
+    devtoolsDir = abspath(dirname(realpath(sys.argv[0])))
+
+    # Create directory for DPDK git repo and build
+    tmpDir = tempfile.TemporaryDirectory(dir = "/tmp")
+
+    os.chdir(tmpDir.name)
+    # Clone DPDK and switch to specified tag
+    print('Cloning {} from DPDK git'.format(tag))
+    call_git(['clone', '--quiet', 'http://dpdk.org/git/dpdk'])
+    os.chdir('dpdk')
+    call_git(['checkout', '--quiet', tag])
+
+    # Create build folder with meson and set debug build and cross compile (if needed)
+    print('Configuring Meson')
+    call_exec(['meson', '-Dbuildtype=debug', 'dumpbuild'] + cross_comp_meson)
+    #os.system('meson -Dbuildtype=debug dumpbuild {} >/dev/null'.format(cross_comp_meson))
+    print('Building DPDK . . .')
+    #Build DPDK with ninja
+    call_exec(['ninja', '-C', 'dumpbuild'])
+
+    # Create dump files and output to dump directory
+    dumpDir = join(baseDir,'{}-{}-{}-abi_dump'.format(tag,comp,args.arch))
+    print('Generating ABI dump files')
+    call_exec([join(devtoolsDir,'gen-abi.sh'), 'dumpbuild'])
+    try:
+        shutil.copytree('dumpbuild/dump', dumpDir)
+    except FileExistsError as error:
+        print('ERROR The {} directory already exists, ensure it is not present before running script'.format(dumpDir), file=sys.stderr)
+        tmpDir.cleanup()
+        exit(1)
+
+    # Compress the dump directory
+    print('Creating Tarball of dump files')
+    os.chdir(baseDir)
+    origSize = 0
+    for f in os.scandir(dumpDir):
+        origSize += getsize(f)
+    with tarfile.open('{}.tar.gz'.format(dumpDir), "w:gz") as tar:
+        tar.add(dumpDir, arcname=basename(dumpDir))
+    newSize = getsize('{}.tar.gz'.format(dumpDir))
+
+    # Remove all temporary directories
+    print('Cleaning up temporary directories')
+    shutil.rmtree(dumpDir)
+    tmpDir.cleanup()
+
+    #Print output of the script to the user
+    print('\nDump of DPDK ABI {} is available in {}.tar.gz (Original Size: {:.1f}MB, Compressed Size: {:.1f}MB)\n'.format(tag,dumpDir.split('/')[-1],float(origSize)*1e-6,float(newSize)*1e-6))
+
+if __name__ == "__main__":
+    main()
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v4 3/4] buildtools: add script to setup abi checks for meson
  2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-09-18 12:11       ` Conor Walsh
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 4/4] build: add abi breakage checks to meson Conor Walsh
  2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-18 12:11 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, anatoly.burakov, Conor Walsh

This patch adds a script that is intended to be invoked by meson to
do the required setup for performing ABI breakage checks at build time.
The required ABI dump archives can come from several sources including
being generated at build time or prebuilt archives can be pulled from a
remote http location or local directory.
Invoke using "./abi-setup.py [-t <tag>] -d <dpdk_source_path>"
 - <tag>: dpdk tag e.g. "v20.11", default: latest
 - <dpdk_source_path>: path to dpdk source directory, required
E.g. "./abi-setup.py -t v20.08 -d /root/dpdk"
As this script is intended to be run by meson during a build
some options can be specified by environmental variables:
 - DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the
   systems dump directories.
 - CC: The required compiler will be determined using this
   environmental variable
 - DPDK_ABI_TAR_URI: Can be used to specify a location that the script
   can pull prebuilt or cached dump archives from. This can be a remote
   http location or a local directory
After the script has setup an appropriate ABI dump directory using one of
the multiple methods available to it, it will print the location of this
directory to the command line.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/abi-setup.py | 141 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 141 insertions(+)
 create mode 100755 buildtools/abi-setup.py

diff --git a/buildtools/abi-setup.py b/buildtools/abi-setup.py
new file mode 100755
index 000000000..39616d21f
--- /dev/null
+++ b/buildtools/abi-setup.py
@@ -0,0 +1,141 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+"""
+This script is intended to be invoked by meson to do the required setup
+for performing ABI breakage checks at build time.
+The required ABI dump archives can come from several sources including
+being generated at build time or prebuilt archives can be pulled from a
+remote http location or local directory.
+"""
+
+import sys
+import os
+from os.path import abspath, join, exists, isfile
+import argparse
+import platform
+import subprocess
+import requests
+import tarfile
+import shutil
+
+# Get command line options
+def args_parse():
+    # Get command line arguments
+    parser = argparse.ArgumentParser(
+            description='This script is intended to setup ABI dumps for meson to perform ABI checks\n'+
+                        'Supported environmental variables\n'+
+                        '\t- DPDK_ABI_DUMPS_PATH: Can be used to specify a custom directory for the systems dump directories.\n'+
+                        '\t- CC: The required compiler will be determined using this environmental variable.\n'+
+                        '\t- DPDK_ABI_TAR_URI: Can be used to specify a location that the script can pull prebuilt or cached dump archives from. This can be a remote http location or a local directory.\n',
+            formatter_class=argparse.RawTextHelpFormatter)
+    parser.add_argument(
+            '-t', '--tag', dest='tag', type=str,
+            help='DPDK tag e.g. latest or v20.11', default='latest')
+    parser.add_argument(
+            '-d', '--dpdk', dest='dpdk', type=str,
+            help='Path to DPDK source directory', required=True)
+    args = parser.parse_args()
+    return args
+
+# Function to execute git commands
+def call_git(args):
+    args = list(filter(None, args))
+    git_call = subprocess.run(['git'] + args, capture_output=True)
+    if git_call.returncode != 0:
+        print('ERROR Git returned an error', file=sys.stderr)
+        exit(1)
+    return git_call.stdout.decode('utf-8')
+
+# Function to execute commands
+def call_exec(args):
+    args = list(filter(None, args))
+    exec_call = subprocess.run(args, stdout=subprocess.DEVNULL)
+    if exec_call.returncode != 0:
+        print('ERROR Script returned an error', file=sys.stderr)
+        exit(1)
+
+# Get required git tag
+def get_tag(tag):
+    tags = call_git(['ls-remote', '--tags', 'http://dpdk.org/git/dpdk']).split('\n')
+    tags = [t.split('/')[-1].strip() for t in tags if 'rc' not in t and not t.endswith('{}') and t != '']
+    if tag == 'latest':
+        tag = tags[-1]
+    if tag not in tags:
+        print('ERROR supplied tag does not exist in DPDK repo', file=sys.stderr)
+        exit(1)
+    return tag
+
+def main():
+    args = args_parse()
+
+    tag = get_tag(args.tag)
+
+    # Get the specified compiler from system
+    if 'CC' in os.environ:
+        comp = os.environ['CC']
+    else:
+        comp = 'gcc'
+
+    # Get the systems architecture
+    arch = platform.machine()
+
+    # Get devtools path
+    devtools_path = abspath(join(args.dpdk,'devtools'))
+
+    # Get the abi dumps folder from args or env fail if none supplied
+    abi_folder = ''
+    abi_env = 'DPDK_ABI_DUMPS_PATH'
+    if abi_env in os.environ:
+        abi_folder = abspath(os.environ[abi_env])
+    else:
+        abi_folder = abspath(join(args.dpdk,'abi_dumps'))
+
+    # If the directory doesn't exist create it and add a README to explain what it does
+    if not exists(abi_folder):
+        os.makedirs(abi_folder)
+        f=open(abi_folder+'/README','w+')
+        f.write('This directory has been setup to contain the ABI dump folders needed to perform ABI checks\n')
+        f.write('Directories here must be in the format {DPDK Tag}-{Compiler ID}-{Architecture}-abi_dump\n')
+        f.write('e.g. v20.11-gcc-x86_64-abi_dump\n')
+        f.write('Directories that do not use this format will not be picked up by the meson ABI checks\n')
+        f.write('This directory is managed automatically unless desired by the user\n')
+        f.close()
+
+    # Move to abi folder
+    os.chdir(abi_folder)
+    abi_dump=tag+'-'+comp+'-'+arch+'-abi_dump'
+    # Download and untar abi dump if not present
+    if not exists(abi_dump):
+        # Check DPDK_ABI_TAR_URI for the location of the tarballs local or web
+        tar_uri_env = 'DPDK_ABI_TAR_URI'
+        if tar_uri_env in os.environ:
+            abi_tar_uri = os.environ[tar_uri_env]
+            if abi_tar_uri.startswith('http'):
+                # Download the required tarball
+                tar_loc = '{}.tar.gz'.format(join(abi_tar_uri,abi_dump))
+                r = requests.get(tar_loc)
+                if r.status_code == 200:
+                    with open('{}.tar.gz'.format(abi_dump), 'wb') as f:
+                        f.write(r.content)
+            else:
+                abi_tar_uri = abspath(abi_tar_uri)
+                try:
+                    shutil.copy('{}.tar.gz'.format(join(abi_tar_uri,abi_dump)), '.')
+                except FileNotFoundError as error:
+                    pass
+        if not isfile(abi_dump+'.tar.gz'):
+            call_exec([join(devtools_path,'gen-abi-tarball.py'), '-t', tag, '-a', arch])
+            if not isfile(abi_dump+'.tar.gz'):
+                print('ERROR ABI check generation failed', file=sys.stderr)
+                exit(1)
+        f = tarfile.open('{}.tar.gz'.format(abi_dump))
+        f.extractall()
+        os.remove('{}.tar.gz'.format(abi_dump))
+
+    # Tell user where specified directory is
+    print(abspath(abi_dump))
+
+if __name__ == "__main__":
+    main()
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v4 4/4] build: add abi breakage checks to meson
  2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
                         ` (2 preceding siblings ...)
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
@ 2020-09-18 12:11       ` Conor Walsh
  2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-09-18 12:11 UTC (permalink / raw)
  To: dev
  Cc: david.marchand, ray.kinsella, nhorman, aconole, maicolgabriel,
	thomas, bruce.richardson, anatoly.burakov, Conor Walsh

This patch adds the ability to run ABI breakage checks to meson.
To do this the developer needs to set the meson build type to debug and
set the version of DPDK that they want to check the ABI against.
The option "abi_checks" has been added to meson for this, the option
accepts DPDK tags e.g. "latest" or "v20.11".
Example meson command: "meson -Dbuildtype=debug -Dabi_checks=v20.08 build"
When the build is done using ninja the ABI checks will be performed
if any breakages are present the build will fail.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 buildtools/meson.build | 18 ++++++++++++++++++
 config/meson.build     | 15 +++++++++++++++
 drivers/meson.build    | 14 ++++++++++++++
 lib/meson.build        | 14 ++++++++++++++
 meson_options.txt      |  2 ++
 5 files changed, 63 insertions(+)

diff --git a/buildtools/meson.build b/buildtools/meson.build
index 04808dabc..c3ee69a44 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -17,3 +17,21 @@ else
 endif
 map_to_win_cmd = py3 + files('map_to_win.py')
 sphinx_wrapper = py3 + files('call-sphinx-build.py')
+
+abi_check_version = get_option('abi_checks')
+check_abi = (abi_check_version != '')
+
+abi_dir = ''
+# If abi checks enabled setup abi dump directory
+if check_abi
+	message('ABI checks are being setup. This could take several minutes')
+	setup_run = run_command('abi-setup.py', '-t', abi_check_version, '-d', meson.source_root())
+	# Check if error returned from script
+	if setup_run.returncode() != 0
+		setup_err = setup_run.stderr().strip()
+		error('ABI checks setup script returned an error: ' + setup_err)
+	endif
+	abi_dir = setup_run.stdout().strip()
+endif
+abidiff = find_program('abidiff', required: check_abi)
+abignore = files('../devtools/libabigail.abignore')
diff --git a/config/meson.build b/config/meson.build
index 6996e5cbe..79b90f2cc 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -47,6 +47,21 @@ else
 	dpdk_conf.set('RTE_VER_RELEASE', 99)
 endif
 
+# abi checks cannot be run on windows
+if is_windows and check_abi
+	error('ABI checks cannot be run on windows')
+endif
+# abi checks can only be run on a debug build
+# meson <0.48 does not support get_option('debug')
+if meson.version().version_compare('>=0.48')
+	debug_enabled = get_option('debug')
+else
+	debug_enabled = get_option('buildtype').startswith('debug')
+endif
+if check_abi and not debug_enabled
+	error('Build type must have debug symbols when abi_checks are enabled')
+endif
+
 pmd_subdir_opt = get_option('drivers_install_subdir')
 if pmd_subdir_opt.contains('<VERSION>')
 	pmd_subdir_opt = abi_version.join(pmd_subdir_opt.split('<VERSION>'))
diff --git a/drivers/meson.build b/drivers/meson.build
index 5f9526557..b24a875da 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -208,6 +208,20 @@ foreach subpath:subdirs
 					include_directories: includes,
 					dependencies: static_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/lib' + lib_name + '.dump'
+			if check_abi and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target('lib' + lib_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							'@INPUT@'],
+					      input: [dump_name, shared_lib],
+					      output: 'lib' + lib_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_drivers += static_lib
 
 			set_variable('shared_@0@'.format(lib_name), shared_dep)
diff --git a/lib/meson.build b/lib/meson.build
index 3852c0156..0d9325b90 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -190,6 +190,20 @@ foreach l:libraries
 					include_directories: includes,
 					dependencies: shared_deps)
 
+			# If abidiff found, abi checks are enabled and the abi dump files for the library are available run abi check
+			dump_name = abi_dir+'/' + dir_name + '.dump'
+			if check_abi and run_command('[', '-f', dump_name, ']').returncode() == 0
+				custom_target(dir_name + '.abi_chk',
+					      command: [abidiff, '--no-added-syms',
+							'--suppr', abignore,
+							'@INPUT@'],
+					      input: [dump_name, shared_lib],
+					      output: dir_name + '.abi_chk',
+					      capture: true,
+					      install: false,
+					      build_by_default: true)
+			endif
+
 			dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 		endif # sources.length() > 0
diff --git a/meson_options.txt b/meson_options.txt
index 9bf18ab6b..26ac48f45 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('abi_checks', type: 'string', value: '',
+    description: 'Enable abi compatibility checks to run during the build. This requires debug build to be enabled. Input is latest or git tag e.g. v20.11')
 option('armv8_crypto_dir', type: 'string', value: '',
 	description: 'path to the armv8_crypto library installation directory')
 option('disable_drivers', type: 'string', value: '',
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks
  2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
                         ` (3 preceding siblings ...)
  2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 4/4] build: add abi breakage checks to meson Conor Walsh
@ 2020-10-12  8:08       ` Conor Walsh
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
                           ` (4 more replies)
  4 siblings, 5 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-12  8:08 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patchset will help developers discover abi breakages more easily
before upstreaming their code. Currently checking that the DPDK ABI
has not changed before up-streaming code is not intuitive and the
process is time consuming. Currently contributors must use the
test-meson-builds.sh tool, alongside some environmental variables to
test their changes. Contributors in many cases are either unaware or
unable to do this themselves, leading to a potentially serious situation
where they are unknowingly up-streaming code that breaks the ABI. These
breakages are caught by Travis, but it would be more efficient if they
were caught locally before up-streaming. This patchset introduces changes
to test-meson-builds.sh, check-abi.sh and adds a new script
gen-abi-tarballs.sh. The changes to test-meson-builds.sh include UX
changes such as adding command line arguments and allowing the use of
relative paths. Reduced the number of abi checks to just two, one for both
x86_64 and ARM, the references for these tests can now be prebuilt and
downloaded by test-meson-builds.sh, these changes will allow the tests to
run much faster. check-abi.sh is updated to use the prebuilt references.
gen-abi-tarballs.sh is a new script to generate the prebuilt abi
references used by test-meson-builds.sh, these compressed archives can be
retrieved from either a local directory or a remote http location.

---
v5:
 - Patchset has been completely reworked following feedback
 - Patchset is now part of test-meson-builds.sh not the meson build system

v4:
 - Reworked both Python scripts to use more native Python functions
   and modules.
 - Python scripts are now in line with how other Python scripts in
   DPDK are structured.

v3:
 - Fix for bug which now allows meson < 0.48.0 to be used
 - Various coding style changes throughout
 - Minor bug fixes to the various meson.build files

v2: Spelling mistake, corrected spelling of environmental

Conor Walsh (4):
  devtools: add generation of compressed abi dump archives
  devtools: abi and UX changes for test-meson-builds.sh
  devtools: change dump file not found to warning in check-abi.sh
  doc: test-meson-builds.sh doc updates

 devtools/check-abi.sh               |   3 +-
 devtools/gen-abi-tarballs.sh        |  48 ++++++++
 devtools/test-meson-builds.sh       | 170 ++++++++++++++++++++++------
 doc/guides/contributing/patches.rst |  43 +++++--
 4 files changed, 220 insertions(+), 44 deletions(-)
 create mode 100755 devtools/gen-abi-tarballs.sh

-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v5 1/4] devtools: add generation of compressed abi dump archives
  2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
@ 2020-10-12  8:08         ` Conor Walsh
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
                           ` (3 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-12  8:08 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patch adds a script that generates compressed archives
containing .dump files which can be used to perform abi
breakage checking in test-meson-build.sh.
Invoke using "./gen-abi-tarballs.sh [-v <dpdk tag>]"
 - <dpdk tag>: dpdk tag e.g. "v20.11" or "latest"
e.g. "./gen-abi-tarballs.sh -v latest"
If no tag is specified, the script will default to "latest"
Using these parameters the script will produce several *.tar.gz
archives containing .dump files required to do abi breakage checking

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi-tarballs.sh | 48 ++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100755 devtools/gen-abi-tarballs.sh

diff --git a/devtools/gen-abi-tarballs.sh b/devtools/gen-abi-tarballs.sh
new file mode 100755
index 000000000..bcc1beac5
--- /dev/null
+++ b/devtools/gen-abi-tarballs.sh
@@ -0,0 +1,48 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+# Generate the required prebuilt ABI references for test-meson-build.sh
+
+# Get arguments
+usage() { echo "Usage: $0 [-v <dpdk tag or latest>]" 1>&2; exit 1; }
+abi_tag=
+while getopts "v:h" arg; do
+	case $arg in
+	v)
+		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+			echo "DPDK_ABI_REF_VERSION and -v cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_VERSION=${OPTARG} ;;
+	h)
+		usage ;;
+	*)
+		usage ;;
+	esac
+done
+
+if [ -z $DPDK_ABI_REF_VERSION ] ; then
+	DPDK_ABI_REF_VERSION="latest"
+fi
+
+srcdir=$(dirname $(readlink -f $0))/..
+
+DPDK_ABI_GEN_REF=-20
+DPDK_ABI_REF_DIR=$srcdir/__abitarballs
+
+. $srcdir/devtools/test-meson-builds.sh
+
+abirefdir=$DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION
+
+rm -rf $abirefdir/build-*.tar.gz
+cd $abirefdir
+for f in build-* ; do
+	tar -czf $f.tar.gz $f
+done
+cp *.tar.gz ../
+rm -rf *
+mv ../*.tar.gz .
+rm -rf build-x86-default.tar.gz
+
+echo "The references for $DPDK_ABI_REF_VERSION are now available in $abirefdir"
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v5 2/4] devtools: abi and UX changes for test-meson-builds.sh
  2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-10-12  8:08         ` Conor Walsh
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 3/4] devtools: change dump file not found to warning in check-abi.sh Conor Walsh
                           ` (2 subsequent siblings)
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-12  8:08 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patch adds new features to test-meson-builds.sh that help to make
the process of using the script easier, the patch also includes
changes to make the abi breakage checks more performant.
Changes/Additions:
 - Command line arguments added, the changes are fully backwards
   compatible and all previous environmental variables are still supported
 - All paths supplied by user are converted to absolute paths if they
   are relative as meson has a bug that can sometimes error if a
   relative path is supplied to it.
 - abi check/generation code moved to function to improve readability
 - Only 2 abi checks will now be completed:
    - 1 x86_64 gcc or clang check
    - 1 ARM gcc or clang check
   It is not necessary to check abi breakages in every build
 - abi checks can now make use of prebuilt abi references from a http
   or local source, it is hoped these would be hosted on dpdk.org in
   the future.
Invoke using "./test-meson-builds.sh [-b <build directory>]
   [-a <dpdk tag or latest for abi check>] [-u <uri for abi references>]
   [-d <directory for abi references>]"
 - <build directory>: directory to store builds (relative or absolute)
 - <dpdk tag or latest for abi check>: dpdk tag e.g. "v20.11" or "latest"
 - <uri for abi references>: http location or directory to get prebuilt
   abi references from
 - <directory for abi references>: directory to store abi references
   (relative or absolute)
e.g. "./test-meson-builds.sh -a latest"
If no flags are specified test-meson-builds.sh will run the standard
meson tests with default options unless environmental variables are
specified.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/test-meson-builds.sh | 170 +++++++++++++++++++++++++++-------
 1 file changed, 138 insertions(+), 32 deletions(-)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index a87de635a..b45506fb0 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -1,12 +1,73 @@
 #! /bin/sh -e
 # SPDX-License-Identifier: BSD-3-Clause
-# Copyright(c) 2018 Intel Corporation
+# Copyright(c) 2018-2020 Intel Corporation
 
 # Run meson to auto-configure the various builds.
 # * all builds get put in a directory whose name starts with "build-"
 # * if a build-directory already exists we assume it was properly configured
 # Run ninja after configuration is done.
 
+# Get arguments
+usage()
+{
+	echo "Usage: $0
+	      [-b <build directory>]
+	      [-a <dpdk tag or latest for abi check>]
+	      [-u <uri for abi references>]
+	      [-d <directory for abi references>]" 1>&2; exit 1;
+}
+
+DPDK_ABI_DEFAULT_URI="http://dpdk.org/abi-refs"
+
+while getopts "a:u:d:b:h" arg; do
+	case $arg in
+	a)
+		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+			echo "DPDK_ABI_REF_VERSION and -a cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_VERSION=${OPTARG} ;;
+	u)
+		if [ -n "$DPDK_ABI_TAR_URI" ]; then
+			echo "DPDK_ABI_TAR_URI and -u cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_TAR_URI=${OPTARG} ;;
+	d)
+		if [ -n "$DPDK_ABI_REF_DIR" ]; then
+			echo "DPDK_ABI_REF_DIR and -d cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_DIR=${OPTARG} ;;
+	b)
+		if [ -n "$DPDK_BUILD_TEST_DIR" ]; then
+			echo "DPDK_BUILD_TEST_DIR and -a cannot both be set"
+			exit 1
+		fi
+		DPDK_BUILD_TEST_DIR=${OPTARG} ;;
+	h)
+		usage ;;
+	*)
+		usage ;;
+	esac
+done
+
+if [ -n "$DPDK_ABI_REF_VERSION" ] ; then
+	if [ "$DPDK_ABI_REF_VERSION" = "latest" ] ; then
+		DPDK_ABI_REF_VERSION=$(git ls-remote --tags http://dpdk.org/git/dpdk |
+	        	sed "s/.*\///" | grep -v "r\|{}" |
+			grep '^[^.]*.[^.]*$' | tail -n 1)
+	elif [ -z "$(git ls-remote http://dpdk.org/git/dpdk refs/tags/$DPDK_ABI_REF_VERSION)" ] ; then
+		echo "$DPDK_ABI_REF_VERSION is not a valid DPDK tag"
+		exit 1
+	fi
+fi
+if [ -z $DPDK_ABI_TAR_URI ] ; then
+	DPDK_ABI_TAR_URI=$DPDK_ABI_DEFAULT_URI
+fi
+# allow the generation script to override value with env var
+abi_checks_done=${DPDK_ABI_GEN_REF:-0}
+
 # set pipefail option if possible
 PIPEFAIL=""
 set -o | grep -q pipefail && set -o pipefail && PIPEFAIL=1
@@ -16,7 +77,11 @@ srcdir=$(dirname $(readlink -f $0))/..
 
 MESON=${MESON:-meson}
 use_shared="--default-library=shared"
-builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+builds_dir=${DPDK_BUILD_TEST_DIR:-$srcdir/builds}
+# ensure path is absolute meson returns error when some paths are relative
+if echo "$builds_dir" | grep -qv '^/'; then
+        builds_dir=$srcdir/$builds_dir
+fi
 
 if command -v gmake >/dev/null 2>&1 ; then
 	MAKE=gmake
@@ -123,39 +188,49 @@ install_target () # <builddir> <installdir>
 	fi
 }
 
-build () # <directory> <target compiler | cross file> <meson options>
+abi_gen_check () # no options
 {
-	targetdir=$1
-	shift
-	crossfile=
-	[ -r $1 ] && crossfile=$1 || targetcc=$1
-	shift
-	# skip build if compiler not available
-	command -v ${CC##* } >/dev/null 2>&1 || return 0
-	if [ -n "$crossfile" ] ; then
-		cross="--cross-file $crossfile"
-		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
-			$crossfile | tr -d "'" | tr -d '"')
-	else
-		cross=
+	abirefdir=${DPDK_ABI_REF_DIR:-$builds_dir/__reference}/$DPDK_ABI_REF_VERSION
+	mkdir -p $abirefdir
+	# ensure path is absolute meson returns error when some are relative
+	if echo "$abirefdir" | grep -qv '^/'; then
+		abirefdir=$srcdir/$abirefdir
 	fi
-	load_env $targetcc || return 0
-	config $srcdir $builds_dir/$targetdir $cross --werror $*
-	compile $builds_dir/$targetdir
-	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
-		abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION
-		if [ ! -d $abirefdir/$targetdir ]; then
+	if [ ! -d $abirefdir/$targetdir ]; then
+
+		# try to get abi reference
+		if echo "$DPDK_ABI_TAR_URI" | grep -q '^http'; then
+			if [ $abi_checks_done -gt -1 ]; then
+				if curl --head --fail --silent \
+					"$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz" \
+					>/dev/null; then
+					curl -o $abirefdir/$targetdir.tar.gz \
+					$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz
+				fi
+			fi
+		elif [ $abi_checks_done -gt -1 ]; then
+			if [ -f "$DPDK_ABI_TAR_URI/$targetdir.tar.gz" ]; then
+				cp $DPDK_ABI_TAR_URI/$targetdir.tar.gz \
+					$abirefdir/
+			fi
+		fi
+		if [ -f "$abirefdir/$targetdir.tar.gz" ]; then
+			tar -xf $abirefdir/$targetdir.tar.gz \
+				-C $abirefdir >/dev/null
+			rm -rf $abirefdir/$targetdir.tar.gz
+		# if no reference can be found then generate one
+		else
 			# clone current sources
 			if [ ! -d $abirefdir/src ]; then
 				git clone --local --no-hardlinks \
-					--single-branch \
-					-b $DPDK_ABI_REF_VERSION \
-					$srcdir $abirefdir/src
+					  --single-branch \
+					  -b $DPDK_ABI_REF_VERSION \
+					  $srcdir $abirefdir/src
 			fi
 
 			rm -rf $abirefdir/build
 			config $abirefdir/src $abirefdir/build $cross \
-				-Dexamples= $*
+			       -Dexamples= $*
 			compile $abirefdir/build
 			install_target $abirefdir/build $abirefdir/$targetdir
 			$srcdir/devtools/gen-abi.sh $abirefdir/$targetdir
@@ -164,17 +239,46 @@ build () # <directory> <target compiler | cross file> <meson options>
 			find $abirefdir/$targetdir/usr/local -name '*.a' -delete
 			rm -rf $abirefdir/$targetdir/usr/local/bin
 			rm -rf $abirefdir/$targetdir/usr/local/share
+			rm -rf $abirefdir/$targetdir/usr/local/lib
 		fi
+	fi
 
-		install_target $builds_dir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install)
-		$srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install)
+	install_target $builds_dir/$targetdir \
+		$(readlink -f $builds_dir/$targetdir/install)
+	$srcdir/devtools/gen-abi.sh \
+		$(readlink -f $builds_dir/$targetdir/install)
+	# check abi if not generating references
+	if [ -z $DPDK_ABI_GEN_REF ] ; then
 		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
 			$(readlink -f $builds_dir/$targetdir/install)
 	fi
 }
 
+build () # <directory> <target compiler | cross file> <meson options>
+{
+	targetdir=$1
+	shift
+	crossfile=
+	[ -r $1 ] && crossfile=$1 || targetcc=$1
+	shift
+	# skip build if compiler not available
+	command -v ${CC##* } >/dev/null 2>&1 || return 0
+	if [ -n "$crossfile" ] ; then
+		cross="--cross-file $crossfile"
+		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
+			$crossfile | tr -d "'" | tr -d '"')
+	else
+		cross=
+	fi
+	load_env $targetcc || return 0
+	config $srcdir $builds_dir/$targetdir $cross --werror $*
+	compile $builds_dir/$targetdir
+	if [ -n "$DPDK_ABI_REF_VERSION" ] && [ $abi_checks_done -lt 1 ] ; then
+		abi_gen_check
+		abi_checks_done=$((abi_checks_done+1))
+	fi
+}
+
 if [ "$1" = "-vv" ] ; then
 	TEST_MESON_BUILD_VERY_VERBOSE=1
 elif [ "$1" = "-v" ] ; then
@@ -189,7 +293,7 @@ fi
 # shared and static linked builds with gcc and clang
 for c in gcc clang ; do
 	command -v $c >/dev/null 2>&1 || continue
-	for s in static shared ; do
+	for s in shared static ; do
 		export CC="$CCACHE $c"
 		build build-$c-$s $c --default-library=$s
 		unset CC
@@ -211,6 +315,8 @@ build build-x86-mingw $srcdir/config/x86/cross-mingw -Dexamples=helloworld
 
 # generic armv8a with clang as host compiler
 f=$srcdir/config/arm/arm64_armv8_linux_gcc
+# run abi checks with 1 arm build
+abi_checks_done=$((abi_checks_done-1))
 export CC="clang"
 build build-arm64-host-clang $f $use_shared
 unset CC
@@ -231,7 +337,7 @@ done
 build_path=$(readlink -f $builds_dir/build-x86-default)
 export DESTDIR=$build_path/install
 # No need to reinstall if ABI checks are enabled
-if [ -z "$DPDK_ABI_REF_VERSION" ]; then
+if [ -z "$DPDK_ABI_REF_VERSION" ] ; then
 	install_target $build_path $DESTDIR
 fi
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v5 3/4] devtools: change dump file not found to warning in check-abi.sh
  2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
@ 2020-10-12  8:08         ` Conor Walsh
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-12  8:08 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

Change dump file not found from an error to a warning to make check-abi.sh
compatible with the changes to test-meson-builds.sh needed to use
prebuilt references.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/check-abi.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
index ab6748cfb..60d88777e 100755
--- a/devtools/check-abi.sh
+++ b/devtools/check-abi.sh
@@ -46,8 +46,7 @@ for dump in $(find $refdir -name "*.dump"); do
 	fi
 	dump2=$(find $newdir -name $name)
 	if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then
-		echo "Error: can't find $name in $newdir"
-		error=1
+		echo "WARNING: can't find $name in $newdir, are you building with all dependencies?"
 		continue
 	fi
 	abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v5 4/4] doc: test-meson-builds.sh doc updates
  2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
                           ` (2 preceding siblings ...)
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 3/4] devtools: change dump file not found to warning in check-abi.sh Conor Walsh
@ 2020-10-12  8:08         ` Conor Walsh
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-12  8:08 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

Updates to the Checking Compilation and Checking ABI compatibility
sections of the patches part of the contribution guide

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 doc/guides/contributing/patches.rst | 43 ++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 9ff60944c..d45bb5ce1 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -470,10 +470,9 @@ The script internally checks for dependencies, then builds for several
 combinations of compilation configuration.
 By default, each build will be put in a subfolder of the current working directory.
 However, if it is preferred to place the builds in a different location,
-the environment variable ``DPDK_BUILD_TEST_DIR`` can be set to that desired location.
-For example, setting ``DPDK_BUILD_TEST_DIR=__builds`` will put all builds
-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.
+the environment variable ``DPDK_BUILD_TEST_DIR`` or the command line argument ``-b``
+can be set to that desired location.
+Environmental variables can also be specified in ``.config/dpdk/devel.config``.
 
 
 .. _integrated_abi_check:
@@ -483,14 +482,38 @@ Checking ABI compatibility
 
 By default, ABI compatibility checks are disabled.
 
-To enable them, a reference version must be selected via the environment
-variable ``DPDK_ABI_REF_VERSION``.
+To enable ABI checks the required reference version must be set using either the
+environment variable ``DPDK_ABI_REF_VERSION`` or the command line argument ``-a``.
+The tag ``latest`` is supported, which will select the latest quarterly release.
+e.g. ``./devtools/test-meson-builds.sh -a latest``.
 
-The ``devtools/test-build.sh`` and ``devtools/test-meson-builds.sh`` scripts
-then build this reference version in a temporary directory and store the
+The ``devtools/test-meson-builds.sh`` script will then either build this reference version
+or download a cached version when available in a temporary directory and store the results
+in a subfolder of the current working directory.
+The environment variable ``DPDK_ABI_REF_DIR`` or the argument ``-d`` can be set so that
+the results go to a different location.
+Environmental variables can also be specified in ``.config/dpdk/devel.config``.
+
+
+.. _integrated_abi_check:
+
+Checking ABI compatibility
+--------------------------
+
+By default, ABI compatibility checks are disabled.
+
+To enable ABI checks the required reference version must be set using either
+the environment variable ``DPDK_ABI_REF_VERSION`` or the argument ``-a``.
+The tag ``latest`` is supported, which will select the latest quarterly release.
+e.g. ``./devtools/test-meson-builds.sh -a latest``.
+
+The ``devtools/test-meson-builds.sh`` script will either build this reference version
+or download a cached version if available in a temporary directory and store the
 results in a subfolder of the current working directory.
-The environment variable ``DPDK_ABI_REF_DIR`` can be set so that the results go
-to a different location.
+The environment variable ``DPDK_ABI_REF_DIR`` or the argument ``-d`` can be set so that
+the results go to a different location.
+The environment variable ``DPDK_ABI_TAR_URI`` or the argument ``-u`` can be set to select
+either a remote http location or local directory to download prebuilt ABI references from.
 
 
 Sending Patches
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks
  2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
                           ` (3 preceding siblings ...)
  2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
@ 2020-10-12 13:03         ` Conor Walsh
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
                             ` (5 more replies)
  4 siblings, 6 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-12 13:03 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patchset will help developers discover abi breakages more easily
before upstreaming their code. Currently checking that the DPDK ABI
has not changed before up-streaming code is not intuitive and the
process is time consuming. Currently contributors must use the
test-meson-builds.sh tool, alongside some environmental variables to
test their changes. Contributors in many cases are either unaware or
unable to do this themselves, leading to a potentially serious situation
where they are unknowingly up-streaming code that breaks the ABI. These
breakages are caught by Travis, but it would be more efficient if they
were caught locally before up-streaming. This patchset introduces changes
to test-meson-builds.sh, check-abi.sh and adds a new script
gen-abi-tarballs.sh. The changes to test-meson-builds.sh include UX
changes such as adding command line arguments and allowing the use of
relative paths. Reduced the number of abi checks to just two, one for both
x86_64 and ARM, the references for these tests can now be prebuilt and
downloaded by test-meson-builds.sh, these changes will allow the tests to
run much faster. check-abi.sh is updated to use the prebuilt references.
gen-abi-tarballs.sh is a new script to generate the prebuilt abi
references used by test-meson-builds.sh, these compressed archives can be
retrieved from either a local directory or a remote http location.

---
v6: Corrected a mistake in the doc patch

v5:
 - Patchset has been completely reworked following feedback
 - Patchset is now part of test-meson-builds.sh not the meson build system

v4:
 - Reworked both Python scripts to use more native Python functions
   and modules.
 - Python scripts are now in line with how other Python scripts in
   DPDK are structured.

v3:
 - Fix for bug which now allows meson < 0.48.0 to be used
 - Various coding style changes throughout
 - Minor bug fixes to the various meson.build files

v2: Spelling mistake, corrected spelling of environmental

Conor Walsh (4):
  devtools: add generation of compressed abi dump archives
  devtools: abi and UX changes for test-meson-builds.sh
  devtools: change dump file not found to warning in check-abi.sh
  doc: test-meson-builds.sh doc updates

 devtools/check-abi.sh               |   3 +-
 devtools/gen-abi-tarballs.sh        |  48 ++++++++
 devtools/test-meson-builds.sh       | 170 ++++++++++++++++++++++------
 doc/guides/contributing/patches.rst |  26 +++--
 4 files changed, 201 insertions(+), 46 deletions(-)
 create mode 100755 devtools/gen-abi-tarballs.sh

-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v6 1/4] devtools: add generation of compressed abi dump archives
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
@ 2020-10-12 13:03           ` Conor Walsh
  2020-10-14  9:38             ` Kinsella, Ray
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
                             ` (4 subsequent siblings)
  5 siblings, 1 reply; 53+ messages in thread
From: Conor Walsh @ 2020-10-12 13:03 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patch adds a script that generates compressed archives
containing .dump files which can be used to perform abi
breakage checking in test-meson-build.sh.
Invoke using "./gen-abi-tarballs.sh [-v <dpdk tag>]"
 - <dpdk tag>: dpdk tag e.g. "v20.11" or "latest"
e.g. "./gen-abi-tarballs.sh -v latest"
If no tag is specified, the script will default to "latest"
Using these parameters the script will produce several *.tar.gz
archives containing .dump files required to do abi breakage checking

Signed-off-by: Conor Walsh <conor.walsh@intel.com>

---
 devtools/gen-abi-tarballs.sh | 48 ++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100755 devtools/gen-abi-tarballs.sh

diff --git a/devtools/gen-abi-tarballs.sh b/devtools/gen-abi-tarballs.sh
new file mode 100755
index 000000000..bcc1beac5
--- /dev/null
+++ b/devtools/gen-abi-tarballs.sh
@@ -0,0 +1,48 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+# Generate the required prebuilt ABI references for test-meson-build.sh
+
+# Get arguments
+usage() { echo "Usage: $0 [-v <dpdk tag or latest>]" 1>&2; exit 1; }
+abi_tag=
+while getopts "v:h" arg; do
+	case $arg in
+	v)
+		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+			echo "DPDK_ABI_REF_VERSION and -v cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_VERSION=${OPTARG} ;;
+	h)
+		usage ;;
+	*)
+		usage ;;
+	esac
+done
+
+if [ -z $DPDK_ABI_REF_VERSION ] ; then
+	DPDK_ABI_REF_VERSION="latest"
+fi
+
+srcdir=$(dirname $(readlink -f $0))/..
+
+DPDK_ABI_GEN_REF=-20
+DPDK_ABI_REF_DIR=$srcdir/__abitarballs
+
+. $srcdir/devtools/test-meson-builds.sh
+
+abirefdir=$DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION
+
+rm -rf $abirefdir/build-*.tar.gz
+cd $abirefdir
+for f in build-* ; do
+	tar -czf $f.tar.gz $f
+done
+cp *.tar.gz ../
+rm -rf *
+mv ../*.tar.gz .
+rm -rf build-x86-default.tar.gz
+
+echo "The references for $DPDK_ABI_REF_VERSION are now available in $abirefdir"
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v6 2/4] devtools: abi and UX changes for test-meson-builds.sh
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-10-12 13:03           ` Conor Walsh
  2020-10-14  9:43             ` Kinsella, Ray
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 3/4] devtools: change dump file not found to warning in check-abi.sh Conor Walsh
                             ` (3 subsequent siblings)
  5 siblings, 1 reply; 53+ messages in thread
From: Conor Walsh @ 2020-10-12 13:03 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patch adds new features to test-meson-builds.sh that help to make
the process of using the script easier, the patch also includes
changes to make the abi breakage checks more performant.
Changes/Additions:
 - Command line arguments added, the changes are fully backwards
   compatible and all previous environmental variables are still supported
 - All paths supplied by user are converted to absolute paths if they
   are relative as meson has a bug that can sometimes error if a
   relative path is supplied to it.
 - abi check/generation code moved to function to improve readability
 - Only 2 abi checks will now be completed:
    - 1 x86_64 gcc or clang check
    - 1 ARM gcc or clang check
   It is not necessary to check abi breakages in every build
 - abi checks can now make use of prebuilt abi references from a http
   or local source, it is hoped these would be hosted on dpdk.org in
   the future.
Invoke using "./test-meson-builds.sh [-b <build directory>]
   [-a <dpdk tag or latest for abi check>] [-u <uri for abi references>]
   [-d <directory for abi references>]"
 - <build directory>: directory to store builds (relative or absolute)
 - <dpdk tag or latest for abi check>: dpdk tag e.g. "v20.11" or "latest"
 - <uri for abi references>: http location or directory to get prebuilt
   abi references from
 - <directory for abi references>: directory to store abi references
   (relative or absolute)
e.g. "./test-meson-builds.sh -a latest"
If no flags are specified test-meson-builds.sh will run the standard
meson tests with default options unless environmental variables are
specified.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>

---
 devtools/test-meson-builds.sh | 170 +++++++++++++++++++++++++++-------
 1 file changed, 138 insertions(+), 32 deletions(-)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index a87de635a..b45506fb0 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -1,12 +1,73 @@
 #! /bin/sh -e
 # SPDX-License-Identifier: BSD-3-Clause
-# Copyright(c) 2018 Intel Corporation
+# Copyright(c) 2018-2020 Intel Corporation
 
 # Run meson to auto-configure the various builds.
 # * all builds get put in a directory whose name starts with "build-"
 # * if a build-directory already exists we assume it was properly configured
 # Run ninja after configuration is done.
 
+# Get arguments
+usage()
+{
+	echo "Usage: $0
+	      [-b <build directory>]
+	      [-a <dpdk tag or latest for abi check>]
+	      [-u <uri for abi references>]
+	      [-d <directory for abi references>]" 1>&2; exit 1;
+}
+
+DPDK_ABI_DEFAULT_URI="http://dpdk.org/abi-refs"
+
+while getopts "a:u:d:b:h" arg; do
+	case $arg in
+	a)
+		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+			echo "DPDK_ABI_REF_VERSION and -a cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_VERSION=${OPTARG} ;;
+	u)
+		if [ -n "$DPDK_ABI_TAR_URI" ]; then
+			echo "DPDK_ABI_TAR_URI and -u cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_TAR_URI=${OPTARG} ;;
+	d)
+		if [ -n "$DPDK_ABI_REF_DIR" ]; then
+			echo "DPDK_ABI_REF_DIR and -d cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_DIR=${OPTARG} ;;
+	b)
+		if [ -n "$DPDK_BUILD_TEST_DIR" ]; then
+			echo "DPDK_BUILD_TEST_DIR and -a cannot both be set"
+			exit 1
+		fi
+		DPDK_BUILD_TEST_DIR=${OPTARG} ;;
+	h)
+		usage ;;
+	*)
+		usage ;;
+	esac
+done
+
+if [ -n "$DPDK_ABI_REF_VERSION" ] ; then
+	if [ "$DPDK_ABI_REF_VERSION" = "latest" ] ; then
+		DPDK_ABI_REF_VERSION=$(git ls-remote --tags http://dpdk.org/git/dpdk |
+	        	sed "s/.*\///" | grep -v "r\|{}" |
+			grep '^[^.]*.[^.]*$' | tail -n 1)
+	elif [ -z "$(git ls-remote http://dpdk.org/git/dpdk refs/tags/$DPDK_ABI_REF_VERSION)" ] ; then
+		echo "$DPDK_ABI_REF_VERSION is not a valid DPDK tag"
+		exit 1
+	fi
+fi
+if [ -z $DPDK_ABI_TAR_URI ] ; then
+	DPDK_ABI_TAR_URI=$DPDK_ABI_DEFAULT_URI
+fi
+# allow the generation script to override value with env var
+abi_checks_done=${DPDK_ABI_GEN_REF:-0}
+
 # set pipefail option if possible
 PIPEFAIL=""
 set -o | grep -q pipefail && set -o pipefail && PIPEFAIL=1
@@ -16,7 +77,11 @@ srcdir=$(dirname $(readlink -f $0))/..
 
 MESON=${MESON:-meson}
 use_shared="--default-library=shared"
-builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+builds_dir=${DPDK_BUILD_TEST_DIR:-$srcdir/builds}
+# ensure path is absolute meson returns error when some paths are relative
+if echo "$builds_dir" | grep -qv '^/'; then
+        builds_dir=$srcdir/$builds_dir
+fi
 
 if command -v gmake >/dev/null 2>&1 ; then
 	MAKE=gmake
@@ -123,39 +188,49 @@ install_target () # <builddir> <installdir>
 	fi
 }
 
-build () # <directory> <target compiler | cross file> <meson options>
+abi_gen_check () # no options
 {
-	targetdir=$1
-	shift
-	crossfile=
-	[ -r $1 ] && crossfile=$1 || targetcc=$1
-	shift
-	# skip build if compiler not available
-	command -v ${CC##* } >/dev/null 2>&1 || return 0
-	if [ -n "$crossfile" ] ; then
-		cross="--cross-file $crossfile"
-		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
-			$crossfile | tr -d "'" | tr -d '"')
-	else
-		cross=
+	abirefdir=${DPDK_ABI_REF_DIR:-$builds_dir/__reference}/$DPDK_ABI_REF_VERSION
+	mkdir -p $abirefdir
+	# ensure path is absolute meson returns error when some are relative
+	if echo "$abirefdir" | grep -qv '^/'; then
+		abirefdir=$srcdir/$abirefdir
 	fi
-	load_env $targetcc || return 0
-	config $srcdir $builds_dir/$targetdir $cross --werror $*
-	compile $builds_dir/$targetdir
-	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
-		abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION
-		if [ ! -d $abirefdir/$targetdir ]; then
+	if [ ! -d $abirefdir/$targetdir ]; then
+
+		# try to get abi reference
+		if echo "$DPDK_ABI_TAR_URI" | grep -q '^http'; then
+			if [ $abi_checks_done -gt -1 ]; then
+				if curl --head --fail --silent \
+					"$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz" \
+					>/dev/null; then
+					curl -o $abirefdir/$targetdir.tar.gz \
+					$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz
+				fi
+			fi
+		elif [ $abi_checks_done -gt -1 ]; then
+			if [ -f "$DPDK_ABI_TAR_URI/$targetdir.tar.gz" ]; then
+				cp $DPDK_ABI_TAR_URI/$targetdir.tar.gz \
+					$abirefdir/
+			fi
+		fi
+		if [ -f "$abirefdir/$targetdir.tar.gz" ]; then
+			tar -xf $abirefdir/$targetdir.tar.gz \
+				-C $abirefdir >/dev/null
+			rm -rf $abirefdir/$targetdir.tar.gz
+		# if no reference can be found then generate one
+		else
 			# clone current sources
 			if [ ! -d $abirefdir/src ]; then
 				git clone --local --no-hardlinks \
-					--single-branch \
-					-b $DPDK_ABI_REF_VERSION \
-					$srcdir $abirefdir/src
+					  --single-branch \
+					  -b $DPDK_ABI_REF_VERSION \
+					  $srcdir $abirefdir/src
 			fi
 
 			rm -rf $abirefdir/build
 			config $abirefdir/src $abirefdir/build $cross \
-				-Dexamples= $*
+			       -Dexamples= $*
 			compile $abirefdir/build
 			install_target $abirefdir/build $abirefdir/$targetdir
 			$srcdir/devtools/gen-abi.sh $abirefdir/$targetdir
@@ -164,17 +239,46 @@ build () # <directory> <target compiler | cross file> <meson options>
 			find $abirefdir/$targetdir/usr/local -name '*.a' -delete
 			rm -rf $abirefdir/$targetdir/usr/local/bin
 			rm -rf $abirefdir/$targetdir/usr/local/share
+			rm -rf $abirefdir/$targetdir/usr/local/lib
 		fi
+	fi
 
-		install_target $builds_dir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install)
-		$srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install)
+	install_target $builds_dir/$targetdir \
+		$(readlink -f $builds_dir/$targetdir/install)
+	$srcdir/devtools/gen-abi.sh \
+		$(readlink -f $builds_dir/$targetdir/install)
+	# check abi if not generating references
+	if [ -z $DPDK_ABI_GEN_REF ] ; then
 		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
 			$(readlink -f $builds_dir/$targetdir/install)
 	fi
 }
 
+build () # <directory> <target compiler | cross file> <meson options>
+{
+	targetdir=$1
+	shift
+	crossfile=
+	[ -r $1 ] && crossfile=$1 || targetcc=$1
+	shift
+	# skip build if compiler not available
+	command -v ${CC##* } >/dev/null 2>&1 || return 0
+	if [ -n "$crossfile" ] ; then
+		cross="--cross-file $crossfile"
+		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
+			$crossfile | tr -d "'" | tr -d '"')
+	else
+		cross=
+	fi
+	load_env $targetcc || return 0
+	config $srcdir $builds_dir/$targetdir $cross --werror $*
+	compile $builds_dir/$targetdir
+	if [ -n "$DPDK_ABI_REF_VERSION" ] && [ $abi_checks_done -lt 1 ] ; then
+		abi_gen_check
+		abi_checks_done=$((abi_checks_done+1))
+	fi
+}
+
 if [ "$1" = "-vv" ] ; then
 	TEST_MESON_BUILD_VERY_VERBOSE=1
 elif [ "$1" = "-v" ] ; then
@@ -189,7 +293,7 @@ fi
 # shared and static linked builds with gcc and clang
 for c in gcc clang ; do
 	command -v $c >/dev/null 2>&1 || continue
-	for s in static shared ; do
+	for s in shared static ; do
 		export CC="$CCACHE $c"
 		build build-$c-$s $c --default-library=$s
 		unset CC
@@ -211,6 +315,8 @@ build build-x86-mingw $srcdir/config/x86/cross-mingw -Dexamples=helloworld
 
 # generic armv8a with clang as host compiler
 f=$srcdir/config/arm/arm64_armv8_linux_gcc
+# run abi checks with 1 arm build
+abi_checks_done=$((abi_checks_done-1))
 export CC="clang"
 build build-arm64-host-clang $f $use_shared
 unset CC
@@ -231,7 +337,7 @@ done
 build_path=$(readlink -f $builds_dir/build-x86-default)
 export DESTDIR=$build_path/install
 # No need to reinstall if ABI checks are enabled
-if [ -z "$DPDK_ABI_REF_VERSION" ]; then
+if [ -z "$DPDK_ABI_REF_VERSION" ] ; then
 	install_target $build_path $DESTDIR
 fi
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v6 3/4] devtools: change dump file not found to warning in check-abi.sh
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
@ 2020-10-12 13:03           ` Conor Walsh
  2020-10-14  9:44             ` Kinsella, Ray
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
                             ` (2 subsequent siblings)
  5 siblings, 1 reply; 53+ messages in thread
From: Conor Walsh @ 2020-10-12 13:03 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

Change dump file not found from an error to a warning to make check-abi.sh
compatible with the changes to test-meson-builds.sh needed to use
prebuilt references.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>

---
 devtools/check-abi.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
index ab6748cfb..60d88777e 100755
--- a/devtools/check-abi.sh
+++ b/devtools/check-abi.sh
@@ -46,8 +46,7 @@ for dump in $(find $refdir -name "*.dump"); do
 	fi
 	dump2=$(find $newdir -name $name)
 	if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then
-		echo "Error: can't find $name in $newdir"
-		error=1
+		echo "WARNING: can't find $name in $newdir, are you building with all dependencies?"
 		continue
 	fi
 	abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v6 4/4] doc: test-meson-builds.sh doc updates
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
                             ` (2 preceding siblings ...)
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 3/4] devtools: change dump file not found to warning in check-abi.sh Conor Walsh
@ 2020-10-12 13:03           ` Conor Walsh
  2020-10-14  9:46             ` Kinsella, Ray
  2020-10-14  9:37           ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Kinsella, Ray
  2020-10-14 10:41           ` [dpdk-dev] [PATCH v7 " Conor Walsh
  5 siblings, 1 reply; 53+ messages in thread
From: Conor Walsh @ 2020-10-12 13:03 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

Updates to the Checking Compilation and Checking ABI compatibility
sections of the patches part of the contribution guide

Signed-off-by: Conor Walsh <conor.walsh@intel.com>

---
 doc/guides/contributing/patches.rst | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 9ff60944c..e11d63bb0 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -470,10 +470,9 @@ The script internally checks for dependencies, then builds for several
 combinations of compilation configuration.
 By default, each build will be put in a subfolder of the current working directory.
 However, if it is preferred to place the builds in a different location,
-the environment variable ``DPDK_BUILD_TEST_DIR`` can be set to that desired location.
-For example, setting ``DPDK_BUILD_TEST_DIR=__builds`` will put all builds
-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.
+the environment variable ``DPDK_BUILD_TEST_DIR`` or the command line argument ``-b``
+can be set to that desired location.
+Environmental variables can also be specified in ``.config/dpdk/devel.config``.
 
 
 .. _integrated_abi_check:
@@ -483,14 +482,17 @@ Checking ABI compatibility
 
 By default, ABI compatibility checks are disabled.
 
-To enable them, a reference version must be selected via the environment
-variable ``DPDK_ABI_REF_VERSION``.
-
-The ``devtools/test-build.sh`` and ``devtools/test-meson-builds.sh`` scripts
-then build this reference version in a temporary directory and store the
-results in a subfolder of the current working directory.
-The environment variable ``DPDK_ABI_REF_DIR`` can be set so that the results go
-to a different location.
+To enable ABI checks the required reference version must be set using either the
+environment variable ``DPDK_ABI_REF_VERSION`` or the command line argument ``-a``.
+The tag ``latest`` is supported, which will select the latest quarterly release.
+e.g. ``./devtools/test-meson-builds.sh -a latest``.
+
+The ``devtools/test-meson-builds.sh`` script will then either build this reference version
+or download a cached version when available in a temporary directory and store the results
+in a subfolder of the current working directory.
+The environment variable ``DPDK_ABI_REF_DIR`` or the argument ``-d`` can be set so that
+the results go to a different location.
+Environmental variables can also be specified in ``.config/dpdk/devel.config``.
 
 
 Sending Patches
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
                             ` (3 preceding siblings ...)
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
@ 2020-10-14  9:37           ` Kinsella, Ray
  2020-10-14 10:33             ` Walsh, Conor
  2020-10-14 10:41           ` [dpdk-dev] [PATCH v7 " Conor Walsh
  5 siblings, 1 reply; 53+ messages in thread
From: Kinsella, Ray @ 2020-10-14  9:37 UTC (permalink / raw)
  To: Conor Walsh, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev



On 12/10/2020 14:03, Conor Walsh wrote:
> This patchset will help developers discover abi breakages more easily
> before upstreaming their code. Currently checking that the DPDK ABI
> has not changed before up-streaming code is not intuitive and the
> process is time consuming. Currently contributors must use the
> test-meson-builds.sh tool, alongside some environmental variables to
> test their changes. Contributors in many cases are either unaware or
> unable to do this themselves, leading to a potentially serious situation
> where they are unknowingly up-streaming code that breaks the ABI. These
> breakages are caught by Travis, but it would be more efficient if they
> were caught locally before up-streaming. 

I would remove everything in the git log text before this line... 

> This patchset introduces changes
> to test-meson-builds.sh, check-abi.sh and adds a new script
> gen-abi-tarballs.sh. The changes to test-meson-builds.sh include UX

UX changes = improvements

> changes such as adding command line arguments and allowing the use of
> relative paths. Reduced the number of abi checks to just two, one for both
> x86_64 and ARM, the references for these tests can now be prebuilt and
> downloaded by test-meson-builds.sh, these changes will allow the tests to
> run much faster. check-abi.sh is updated to use the prebuilt references.
> gen-abi-tarballs.sh is a new script to generate the prebuilt abi
> references used by test-meson-builds.sh, these compressed archives can be
> retrieved from either a local directory or a remote http location.
> 
> ---
> v6: Corrected a mistake in the doc patch
> 
> v5:
>  - Patchset has been completely reworked following feedback
>  - Patchset is now part of test-meson-builds.sh not the meson build system
> 
> v4:
>  - Reworked both Python scripts to use more native Python functions
>    and modules.
>  - Python scripts are now in line with how other Python scripts in
>    DPDK are structured.
> 
> v3:
>  - Fix for bug which now allows meson < 0.48.0 to be used
>  - Various coding style changes throughout
>  - Minor bug fixes to the various meson.build files
> 
> v2: Spelling mistake, corrected spelling of environmental
> 
> Conor Walsh (4):
>   devtools: add generation of compressed abi dump archives
>   devtools: abi and UX changes for test-meson-builds.sh
>   devtools: change dump file not found to warning in check-abi.sh
>   doc: test-meson-builds.sh doc updates
> 
>  devtools/check-abi.sh               |   3 +-
>  devtools/gen-abi-tarballs.sh        |  48 ++++++++
>  devtools/test-meson-builds.sh       | 170 ++++++++++++++++++++++------
>  doc/guides/contributing/patches.rst |  26 +++--
>  4 files changed, 201 insertions(+), 46 deletions(-)
>  create mode 100755 devtools/gen-abi-tarballs.sh
> 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v6 1/4] devtools: add generation of compressed abi dump archives
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-10-14  9:38             ` Kinsella, Ray
  0 siblings, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-10-14  9:38 UTC (permalink / raw)
  To: Conor Walsh, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev



On 12/10/2020 14:03, Conor Walsh wrote:
> This patch adds a script that generates compressed archives
> containing .dump files which can be used to perform abi
> breakage checking in test-meson-build.sh.

<new line to aid reading>

> Invoke using "./gen-abi-tarballs.sh [-v <dpdk tag>]"
>  - <dpdk tag>: dpdk tag e.g. "v20.11" or "latest"
> e.g. "./gen-abi-tarballs.sh -v latest"

<new line to aid reading>

> If no tag is specified, the script will default to "latest"
> Using these parameters the script will produce several *.tar.gz
> archives containing .dump files required to do abi breakage checking
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> 
> ---
>  devtools/gen-abi-tarballs.sh | 48 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100755 devtools/gen-abi-tarballs.sh
> 
> diff --git a/devtools/gen-abi-tarballs.sh b/devtools/gen-abi-tarballs.sh
> new file mode 100755
> index 000000000..bcc1beac5
> --- /dev/null
> +++ b/devtools/gen-abi-tarballs.sh
> @@ -0,0 +1,48 @@
> +#! /bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Intel Corporation
> +
> +# Generate the required prebuilt ABI references for test-meson-build.sh
> +
> +# Get arguments
> +usage() { echo "Usage: $0 [-v <dpdk tag or latest>]" 1>&2; exit 1; }
> +abi_tag=
> +while getopts "v:h" arg; do
> +	case $arg in
> +	v)
> +		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> +			echo "DPDK_ABI_REF_VERSION and -v cannot both be set"
> +			exit 1
> +		fi
> +		DPDK_ABI_REF_VERSION=${OPTARG} ;;
> +	h)
> +		usage ;;
> +	*)
> +		usage ;;
> +	esac
> +done
> +
> +if [ -z $DPDK_ABI_REF_VERSION ] ; then
> +	DPDK_ABI_REF_VERSION="latest"
> +fi
> +
> +srcdir=$(dirname $(readlink -f $0))/..
> +
> +DPDK_ABI_GEN_REF=-20
> +DPDK_ABI_REF_DIR=$srcdir/__abitarballs
> +
> +. $srcdir/devtools/test-meson-builds.sh
> +
> +abirefdir=$DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION
> +
> +rm -rf $abirefdir/build-*.tar.gz
> +cd $abirefdir
> +for f in build-* ; do
> +	tar -czf $f.tar.gz $f
> +done
> +cp *.tar.gz ../
> +rm -rf *
> +mv ../*.tar.gz .
> +rm -rf build-x86-default.tar.gz
> +
> +echo "The references for $DPDK_ABI_REF_VERSION are now available in $abirefdir"
> 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v6 2/4] devtools: abi and UX changes for test-meson-builds.sh
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
@ 2020-10-14  9:43             ` Kinsella, Ray
  0 siblings, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-10-14  9:43 UTC (permalink / raw)
  To: Conor Walsh, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev



On 12/10/2020 14:03, Conor Walsh wrote:
> This patch adds new features to test-meson-builds.sh that help to make
> the process of using the script easier, the patch also includes
> changes to make the abi breakage checks more performant.

Avoid commentary such as the above. 

I reduce the following list of bullets to a single paragraph describing the change. 
The core of this change is to improve build times.
So describe reducing the number of build to 2 and using the pre-build references, and thats it. 

> Changes/Additions:
>  - Command line arguments added, the changes are fully backwards
>    compatible and all previous environmental variables are still supported
>  - All paths supplied by user are converted to absolute paths if they
>    are relative as meson has a bug that can sometimes error if a
>    relative path is supplied to it.
>  - abi check/generation code moved to function to improve readability
>  - Only 2 abi checks will now be completed:
>     - 1 x86_64 gcc or clang check
>     - 1 ARM gcc or clang check
>    It is not necessary to check abi breakages in every build
>  - abi checks can now make use of prebuilt abi references from a http
>    or local source, it is hoped these would be hosted on dpdk.org in
>    the future.

<new line to aid reading>

> Invoke using "./test-meson-builds.sh [-b <build directory>]
>    [-a <dpdk tag or latest for abi check>] [-u <uri for abi references>]
>    [-d <directory for abi references>]"
>  - <build directory>: directory to store builds (relative or absolute)
>  - <dpdk tag or latest for abi check>: dpdk tag e.g. "v20.11" or "latest"
>  - <uri for abi references>: http location or directory to get prebuilt
>    abi references from
>  - <directory for abi references>: directory to store abi references
>    (relative or absolute)
> e.g. "./test-meson-builds.sh -a latest"
> If no flags are specified test-meson-builds.sh will run the standard
> meson tests with default options unless environmental variables are
> specified.
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> 
> ---
>  devtools/test-meson-builds.sh | 170 +++++++++++++++++++++++++++-------
>  1 file changed, 138 insertions(+), 32 deletions(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index a87de635a..b45506fb0 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -1,12 +1,73 @@
>  #! /bin/sh -e
>  # SPDX-License-Identifier: BSD-3-Clause
> -# Copyright(c) 2018 Intel Corporation
> +# Copyright(c) 2018-2020 Intel Corporation
>  
>  # Run meson to auto-configure the various builds.
>  # * all builds get put in a directory whose name starts with "build-"
>  # * if a build-directory already exists we assume it was properly configured
>  # Run ninja after configuration is done.
>  
> +# Get arguments
> +usage()
> +{
> +	echo "Usage: $0
> +	      [-b <build directory>]
> +	      [-a <dpdk tag or latest for abi check>]
> +	      [-u <uri for abi references>]
> +	      [-d <directory for abi references>]" 1>&2; exit 1;
> +}
> +
> +DPDK_ABI_DEFAULT_URI="http://dpdk.org/abi-refs"
> +
> +while getopts "a:u:d:b:h" arg; do
> +	case $arg in
> +	a)
> +		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> +			echo "DPDK_ABI_REF_VERSION and -a cannot both be set"
> +			exit 1
> +		fi
> +		DPDK_ABI_REF_VERSION=${OPTARG} ;;
> +	u)
> +		if [ -n "$DPDK_ABI_TAR_URI" ]; then
> +			echo "DPDK_ABI_TAR_URI and -u cannot both be set"
> +			exit 1
> +		fi
> +		DPDK_ABI_TAR_URI=${OPTARG} ;;
> +	d)
> +		if [ -n "$DPDK_ABI_REF_DIR" ]; then
> +			echo "DPDK_ABI_REF_DIR and -d cannot both be set"
> +			exit 1
> +		fi
> +		DPDK_ABI_REF_DIR=${OPTARG} ;;
> +	b)
> +		if [ -n "$DPDK_BUILD_TEST_DIR" ]; then
> +			echo "DPDK_BUILD_TEST_DIR and -a cannot both be set"
> +			exit 1
> +		fi
> +		DPDK_BUILD_TEST_DIR=${OPTARG} ;;
> +	h)
> +		usage ;;
> +	*)
> +		usage ;;
> +	esac
> +done
> +
> +if [ -n "$DPDK_ABI_REF_VERSION" ] ; then
> +	if [ "$DPDK_ABI_REF_VERSION" = "latest" ] ; then
> +		DPDK_ABI_REF_VERSION=$(git ls-remote --tags http://dpdk.org/git/dpdk |
> +	        	sed "s/.*\///" | grep -v "r\|{}" |
> +			grep '^[^.]*.[^.]*$' | tail -n 1)
> +	elif [ -z "$(git ls-remote http://dpdk.org/git/dpdk refs/tags/$DPDK_ABI_REF_VERSION)" ] ; then
> +		echo "$DPDK_ABI_REF_VERSION is not a valid DPDK tag"
> +		exit 1
> +	fi
> +fi
> +if [ -z $DPDK_ABI_TAR_URI ] ; then
> +	DPDK_ABI_TAR_URI=$DPDK_ABI_DEFAULT_URI
> +fi
> +# allow the generation script to override value with env var
> +abi_checks_done=${DPDK_ABI_GEN_REF:-0}
> +
>  # set pipefail option if possible
>  PIPEFAIL=""
>  set -o | grep -q pipefail && set -o pipefail && PIPEFAIL=1
> @@ -16,7 +77,11 @@ srcdir=$(dirname $(readlink -f $0))/..
>  
>  MESON=${MESON:-meson}
>  use_shared="--default-library=shared"
> -builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> +builds_dir=${DPDK_BUILD_TEST_DIR:-$srcdir/builds}
> +# ensure path is absolute meson returns error when some paths are relative
> +if echo "$builds_dir" | grep -qv '^/'; then
> +        builds_dir=$srcdir/$builds_dir
> +fi
>  
>  if command -v gmake >/dev/null 2>&1 ; then
>  	MAKE=gmake
> @@ -123,39 +188,49 @@ install_target () # <builddir> <installdir>
>  	fi
>  }
>  
> -build () # <directory> <target compiler | cross file> <meson options>
> +abi_gen_check () # no options
>  {
> -	targetdir=$1
> -	shift
> -	crossfile=
> -	[ -r $1 ] && crossfile=$1 || targetcc=$1
> -	shift
> -	# skip build if compiler not available
> -	command -v ${CC##* } >/dev/null 2>&1 || return 0
> -	if [ -n "$crossfile" ] ; then
> -		cross="--cross-file $crossfile"
> -		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
> -			$crossfile | tr -d "'" | tr -d '"')
> -	else
> -		cross=
> +	abirefdir=${DPDK_ABI_REF_DIR:-$builds_dir/__reference}/$DPDK_ABI_REF_VERSION
> +	mkdir -p $abirefdir
> +	# ensure path is absolute meson returns error when some are relative
> +	if echo "$abirefdir" | grep -qv '^/'; then
> +		abirefdir=$srcdir/$abirefdir
>  	fi
> -	load_env $targetcc || return 0
> -	config $srcdir $builds_dir/$targetdir $cross --werror $*
> -	compile $builds_dir/$targetdir
> -	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> -		abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION
> -		if [ ! -d $abirefdir/$targetdir ]; then
> +	if [ ! -d $abirefdir/$targetdir ]; then
> +
> +		# try to get abi reference
> +		if echo "$DPDK_ABI_TAR_URI" | grep -q '^http'; then
> +			if [ $abi_checks_done -gt -1 ]; then
> +				if curl --head --fail --silent \
> +					"$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz" \
> +					>/dev/null; then
> +					curl -o $abirefdir/$targetdir.tar.gz \
> +					$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz
> +				fi
> +			fi
> +		elif [ $abi_checks_done -gt -1 ]; then
> +			if [ -f "$DPDK_ABI_TAR_URI/$targetdir.tar.gz" ]; then
> +				cp $DPDK_ABI_TAR_URI/$targetdir.tar.gz \
> +					$abirefdir/
> +			fi
> +		fi
> +		if [ -f "$abirefdir/$targetdir.tar.gz" ]; then
> +			tar -xf $abirefdir/$targetdir.tar.gz \
> +				-C $abirefdir >/dev/null
> +			rm -rf $abirefdir/$targetdir.tar.gz
> +		# if no reference can be found then generate one
> +		else
>  			# clone current sources
>  			if [ ! -d $abirefdir/src ]; then
>  				git clone --local --no-hardlinks \
> -					--single-branch \
> -					-b $DPDK_ABI_REF_VERSION \
> -					$srcdir $abirefdir/src
> +					  --single-branch \
> +					  -b $DPDK_ABI_REF_VERSION \
> +					  $srcdir $abirefdir/src
>  			fi
>  
>  			rm -rf $abirefdir/build
>  			config $abirefdir/src $abirefdir/build $cross \
> -				-Dexamples= $*
> +			       -Dexamples= $*
>  			compile $abirefdir/build
>  			install_target $abirefdir/build $abirefdir/$targetdir
>  			$srcdir/devtools/gen-abi.sh $abirefdir/$targetdir
> @@ -164,17 +239,46 @@ build () # <directory> <target compiler | cross file> <meson options>
>  			find $abirefdir/$targetdir/usr/local -name '*.a' -delete
>  			rm -rf $abirefdir/$targetdir/usr/local/bin
>  			rm -rf $abirefdir/$targetdir/usr/local/share
> +			rm -rf $abirefdir/$targetdir/usr/local/lib
>  		fi
> +	fi
>  
> -		install_target $builds_dir/$targetdir \
> -			$(readlink -f $builds_dir/$targetdir/install)
> -		$srcdir/devtools/gen-abi.sh \
> -			$(readlink -f $builds_dir/$targetdir/install)
> +	install_target $builds_dir/$targetdir \
> +		$(readlink -f $builds_dir/$targetdir/install)
> +	$srcdir/devtools/gen-abi.sh \
> +		$(readlink -f $builds_dir/$targetdir/install)
> +	# check abi if not generating references
> +	if [ -z $DPDK_ABI_GEN_REF ] ; then
>  		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
>  			$(readlink -f $builds_dir/$targetdir/install)
>  	fi
>  }
>  
> +build () # <directory> <target compiler | cross file> <meson options>
> +{
> +	targetdir=$1
> +	shift
> +	crossfile=
> +	[ -r $1 ] && crossfile=$1 || targetcc=$1
> +	shift
> +	# skip build if compiler not available
> +	command -v ${CC##* } >/dev/null 2>&1 || return 0
> +	if [ -n "$crossfile" ] ; then
> +		cross="--cross-file $crossfile"
> +		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
> +			$crossfile | tr -d "'" | tr -d '"')
> +	else
> +		cross=
> +	fi
> +	load_env $targetcc || return 0
> +	config $srcdir $builds_dir/$targetdir $cross --werror $*
> +	compile $builds_dir/$targetdir
> +	if [ -n "$DPDK_ABI_REF_VERSION" ] && [ $abi_checks_done -lt 1 ] ; then
> +		abi_gen_check
> +		abi_checks_done=$((abi_checks_done+1))
> +	fi
> +}
> +
>  if [ "$1" = "-vv" ] ; then
>  	TEST_MESON_BUILD_VERY_VERBOSE=1
>  elif [ "$1" = "-v" ] ; then
> @@ -189,7 +293,7 @@ fi
>  # shared and static linked builds with gcc and clang
>  for c in gcc clang ; do
>  	command -v $c >/dev/null 2>&1 || continue
> -	for s in static shared ; do
> +	for s in shared static ; do
>  		export CC="$CCACHE $c"
>  		build build-$c-$s $c --default-library=$s
>  		unset CC
> @@ -211,6 +315,8 @@ build build-x86-mingw $srcdir/config/x86/cross-mingw -Dexamples=helloworld
>  
>  # generic armv8a with clang as host compiler
>  f=$srcdir/config/arm/arm64_armv8_linux_gcc
> +# run abi checks with 1 arm build
> +abi_checks_done=$((abi_checks_done-1))
>  export CC="clang"
>  build build-arm64-host-clang $f $use_shared
>  unset CC
> @@ -231,7 +337,7 @@ done
>  build_path=$(readlink -f $builds_dir/build-x86-default)
>  export DESTDIR=$build_path/install
>  # No need to reinstall if ABI checks are enabled
> -if [ -z "$DPDK_ABI_REF_VERSION" ]; then
> +if [ -z "$DPDK_ABI_REF_VERSION" ] ; then
>  	install_target $build_path $DESTDIR
>  fi
>  
> 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v6 3/4] devtools: change dump file not found to warning in check-abi.sh
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 3/4] devtools: change dump file not found to warning in check-abi.sh Conor Walsh
@ 2020-10-14  9:44             ` Kinsella, Ray
  0 siblings, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-10-14  9:44 UTC (permalink / raw)
  To: Conor Walsh, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev



On 12/10/2020 14:03, Conor Walsh wrote:
> Change dump file not found from an error to a warning to make check-abi.sh
> compatible with the changes to test-meson-builds.sh needed to use
> prebuilt references.
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> 
> ---
>  devtools/check-abi.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
> index ab6748cfb..60d88777e 100755
> --- a/devtools/check-abi.sh
> +++ b/devtools/check-abi.sh
> @@ -46,8 +46,7 @@ for dump in $(find $refdir -name "*.dump"); do
>  	fi
>  	dump2=$(find $newdir -name $name)
>  	if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then
> -		echo "Error: can't find $name in $newdir"
> -		error=1
> +		echo "WARNING: can't find $name in $newdir, are you building with all dependencies?"
>  		continue
>  	fi
>  	abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
> 

Acked-by: Ray Kinsella <mdr@ashroe.eu>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v6 4/4] doc: test-meson-builds.sh doc updates
  2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
@ 2020-10-14  9:46             ` Kinsella, Ray
  0 siblings, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-10-14  9:46 UTC (permalink / raw)
  To: Conor Walsh, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev



On 12/10/2020 14:03, Conor Walsh wrote:
> Updates to the Checking Compilation and Checking ABI compatibility
> sections of the patches part of the contribution guide
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> 
> ---
>  doc/guides/contributing/patches.rst | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
> index 9ff60944c..e11d63bb0 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -470,10 +470,9 @@ The script internally checks for dependencies, then builds for several
>  combinations of compilation configuration.
>  By default, each build will be put in a subfolder of the current working directory.
>  However, if it is preferred to place the builds in a different location,
> -the environment variable ``DPDK_BUILD_TEST_DIR`` can be set to that desired location.
> -For example, setting ``DPDK_BUILD_TEST_DIR=__builds`` will put all builds
> -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.
> +the environment variable ``DPDK_BUILD_TEST_DIR`` or the command line argument ``-b``
> +can be set to that desired location.
> +Environmental variables can also be specified in ``.config/dpdk/devel.config``.
>  
>  
>  .. _integrated_abi_check:
> @@ -483,14 +482,17 @@ Checking ABI compatibility
>  
>  By default, ABI compatibility checks are disabled.
>  
> -To enable them, a reference version must be selected via the environment
> -variable ``DPDK_ABI_REF_VERSION``.
> -
> -The ``devtools/test-build.sh`` and ``devtools/test-meson-builds.sh`` scripts
> -then build this reference version in a temporary directory and store the
> -results in a subfolder of the current working directory.
> -The environment variable ``DPDK_ABI_REF_DIR`` can be set so that the results go
> -to a different location.
> +To enable ABI checks the required reference version must be set using either the
> +environment variable ``DPDK_ABI_REF_VERSION`` or the command line argument ``-a``.
> +The tag ``latest`` is supported, which will select the latest quarterly release.
> +e.g. ``./devtools/test-meson-builds.sh -a latest``.
> +
> +The ``devtools/test-meson-builds.sh`` script will then either build this reference version
> +or download a cached version when available in a temporary directory and store the results
> +in a subfolder of the current working directory.
> +The environment variable ``DPDK_ABI_REF_DIR`` or the argument ``-d`` can be set so that
> +the results go to a different location.
> +Environmental variables can also be specified in ``.config/dpdk/devel.config``.
>  
>  
>  Sending Patches
> 
Acked-by: Ray Kinsella <mdr@ashroe.eu>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks
  2020-10-14  9:37           ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Kinsella, Ray
@ 2020-10-14 10:33             ` Walsh, Conor
  0 siblings, 0 replies; 53+ messages in thread
From: Walsh, Conor @ 2020-10-14 10:33 UTC (permalink / raw)
  To: Kinsella, Ray, nhorman, Richardson, Bruce, thomas, david.marchand; +Cc: dev

Thanks for your feedback Ray,

V7 with your suggested changes for the patchset is on its way.

/Conor

> -----Original Message-----
> From: Kinsella, Ray <mdr@ashroe.eu>
> Sent: Wednesday 14 October 2020 10:37
> To: Walsh, Conor <conor.walsh@intel.com>; nhorman@tuxdriver.com;
> Richardson, Bruce <bruce.richardson@intel.com>; thomas@monjalon.net;
> david.marchand@redhat.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v6 0/4] devtools: abi breakage checks
> 
> 
> 
> On 12/10/2020 14:03, Conor Walsh wrote:
> > This patchset will help developers discover abi breakages more easily
> > before upstreaming their code. Currently checking that the DPDK ABI
> > has not changed before up-streaming code is not intuitive and the
> > process is time consuming. Currently contributors must use the
> > test-meson-builds.sh tool, alongside some environmental variables to
> > test their changes. Contributors in many cases are either unaware or
> > unable to do this themselves, leading to a potentially serious situation
> > where they are unknowingly up-streaming code that breaks the ABI. These
> > breakages are caught by Travis, but it would be more efficient if they
> > were caught locally before up-streaming.
> 
> I would remove everything in the git log text before this line...
> 
> > This patchset introduces changes
> > to test-meson-builds.sh, check-abi.sh and adds a new script
> > gen-abi-tarballs.sh. The changes to test-meson-builds.sh include UX
> 
> UX changes = improvements
> 
> > changes such as adding command line arguments and allowing the use of
> > relative paths. Reduced the number of abi checks to just two, one for both
> > x86_64 and ARM, the references for these tests can now be prebuilt and
> > downloaded by test-meson-builds.sh, these changes will allow the tests to
> > run much faster. check-abi.sh is updated to use the prebuilt references.
> > gen-abi-tarballs.sh is a new script to generate the prebuilt abi
> > references used by test-meson-builds.sh, these compressed archives can
> be
> > retrieved from either a local directory or a remote http location.
> >
> > ---
> > v6: Corrected a mistake in the doc patch
> >
> > v5:
> >  - Patchset has been completely reworked following feedback
> >  - Patchset is now part of test-meson-builds.sh not the meson build system
> >
> > v4:
> >  - Reworked both Python scripts to use more native Python functions
> >    and modules.
> >  - Python scripts are now in line with how other Python scripts in
> >    DPDK are structured.
> >
> > v3:
> >  - Fix for bug which now allows meson < 0.48.0 to be used
> >  - Various coding style changes throughout
> >  - Minor bug fixes to the various meson.build files
> >
> > v2: Spelling mistake, corrected spelling of environmental
> >
> > Conor Walsh (4):
> >   devtools: add generation of compressed abi dump archives
> >   devtools: abi and UX changes for test-meson-builds.sh
> >   devtools: change dump file not found to warning in check-abi.sh
> >   doc: test-meson-builds.sh doc updates
> >
> >  devtools/check-abi.sh               |   3 +-
> >  devtools/gen-abi-tarballs.sh        |  48 ++++++++
> >  devtools/test-meson-builds.sh       | 170 ++++++++++++++++++++++------
> >  doc/guides/contributing/patches.rst |  26 +++--
> >  4 files changed, 201 insertions(+), 46 deletions(-)
> >  create mode 100755 devtools/gen-abi-tarballs.sh
> >

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v7 0/4] devtools: abi breakage checks
  2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
                             ` (4 preceding siblings ...)
  2020-10-14  9:37           ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Kinsella, Ray
@ 2020-10-14 10:41           ` Conor Walsh
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
                               ` (4 more replies)
  5 siblings, 5 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-14 10:41 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patchset introduces changes to test-meson-builds.sh, check-abi.sh and
adds a new script gen-abi-tarballs.sh. The changes to test-meson-builds.sh
include UX improvements such as adding command line arguments and allowing
the use of relative paths. Reduced the number of abi checks to just two,
one for both x86_64 and ARM, the references for these tests can now be
prebuilt and downloaded by test-meson-builds.sh, these changes will allow
the tests to run much faster. check-abi.sh is updated to use the prebuilt
references. gen-abi-tarballs.sh is a new script to generate the prebuilt
abi references used by test-meson-builds.sh, these compressed archives can
be retrieved from either a local directory or a remote http location.

---
v7: Changes resulting from list feedback

v6: Corrected a mistake in the doc patch

v5:
 - Patchset has been completely reworked following feedback
 - Patchset is now part of test-meson-builds.sh not the meson build
   system

v4:
 - Reworked both Python scripts to use more native Python functions
   and modules.
 - Python scripts are now in line with how other Python scripts in
   DPDK are structured.

v3:
 - Fix for bug which now allows meson < 0.48.0 to be used
 - Various coding style changes throughout
 - Minor bug fixes to the various meson.build files

v2: Spelling mistake, corrected spelling of environmental

Conor Walsh (4):
  devtools: add generation of compressed abi dump archives
  devtools: abi and UX changes for test-meson-builds.sh
  devtools: change dump file not found to warning in check-abi.sh
  doc: test-meson-builds.sh doc updates

 devtools/check-abi.sh               |   3 +-
 devtools/gen-abi-tarballs.sh        |  48 ++++++++
 devtools/test-meson-builds.sh       | 171 ++++++++++++++++++++++------
 doc/guides/contributing/patches.rst |  26 +++--
 4 files changed, 202 insertions(+), 46 deletions(-)
 create mode 100755 devtools/gen-abi-tarballs.sh

-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v7 1/4] devtools: add generation of compressed abi dump archives
  2020-10-14 10:41           ` [dpdk-dev] [PATCH v7 " Conor Walsh
@ 2020-10-14 10:41             ` Conor Walsh
  2020-10-15 10:15               ` Kinsella, Ray
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
                               ` (3 subsequent siblings)
  4 siblings, 1 reply; 53+ messages in thread
From: Conor Walsh @ 2020-10-14 10:41 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

This patch adds a script that generates compressed archives
containing .dump files which can be used to perform abi
breakage checking in test-meson-build.sh.

Invoke using "./gen-abi-tarballs.sh [-v <dpdk tag>]"
 - <dpdk tag>: dpdk tag e.g. "v20.11" or "latest"
e.g. "./gen-abi-tarballs.sh -v latest"

If no tag is specified, the script will default to "latest"
Using these parameters the script will produce several *.tar.gz
archives containing .dump files required to do abi breakage checking

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/gen-abi-tarballs.sh | 48 ++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100755 devtools/gen-abi-tarballs.sh

diff --git a/devtools/gen-abi-tarballs.sh b/devtools/gen-abi-tarballs.sh
new file mode 100755
index 000000000..bcc1beac5
--- /dev/null
+++ b/devtools/gen-abi-tarballs.sh
@@ -0,0 +1,48 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Intel Corporation
+
+# Generate the required prebuilt ABI references for test-meson-build.sh
+
+# Get arguments
+usage() { echo "Usage: $0 [-v <dpdk tag or latest>]" 1>&2; exit 1; }
+abi_tag=
+while getopts "v:h" arg; do
+	case $arg in
+	v)
+		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+			echo "DPDK_ABI_REF_VERSION and -v cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_VERSION=${OPTARG} ;;
+	h)
+		usage ;;
+	*)
+		usage ;;
+	esac
+done
+
+if [ -z $DPDK_ABI_REF_VERSION ] ; then
+	DPDK_ABI_REF_VERSION="latest"
+fi
+
+srcdir=$(dirname $(readlink -f $0))/..
+
+DPDK_ABI_GEN_REF=-20
+DPDK_ABI_REF_DIR=$srcdir/__abitarballs
+
+. $srcdir/devtools/test-meson-builds.sh
+
+abirefdir=$DPDK_ABI_REF_DIR/$DPDK_ABI_REF_VERSION
+
+rm -rf $abirefdir/build-*.tar.gz
+cd $abirefdir
+for f in build-* ; do
+	tar -czf $f.tar.gz $f
+done
+cp *.tar.gz ../
+rm -rf *
+mv ../*.tar.gz .
+rm -rf build-x86-default.tar.gz
+
+echo "The references for $DPDK_ABI_REF_VERSION are now available in $abirefdir"
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v7 2/4] devtools: abi and UX changes for test-meson-builds.sh
  2020-10-14 10:41           ` [dpdk-dev] [PATCH v7 " Conor Walsh
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-10-14 10:41             ` Conor Walsh
  2020-10-15 10:16               ` Kinsella, Ray
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 3/4] devtools: change not found to warning check-abi.sh Conor Walsh
                               ` (2 subsequent siblings)
  4 siblings, 1 reply; 53+ messages in thread
From: Conor Walsh @ 2020-10-14 10:41 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

The core reason for this patch is to reduce the amount of time needed to
run abi checks. The number of abi checks being run has been reduced to
only 2 (1 x86_64 and 1 arm). The script can now also take adavtage of
prebuilt abi references.

Invoke using "./test-meson-builds.sh [-b <build directory>]
   [-a <dpdk tag or latest for abi check>] [-u <uri for abi references>]
   [-d <directory for abi references>]"
 - <build directory>: directory to store builds (relative or absolute)
 - <dpdk tag or latest for abi check>: dpdk tag e.g. "v20.11" or "latest"
 - <uri for abi references>: http location or directory to get prebuilt
   abi references from
 - <directory for abi references>: directory to store abi references
   (relative or absolute)
e.g. "./test-meson-builds.sh -a latest"
If no flags are specified test-meson-builds.sh will run the standard
meson tests with default options unless environmental variables are
specified.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
---
 devtools/test-meson-builds.sh | 171 +++++++++++++++++++++++++++-------
 1 file changed, 139 insertions(+), 32 deletions(-)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index a87de635a..6b959eb63 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -1,12 +1,74 @@
 #! /bin/sh -e
 # SPDX-License-Identifier: BSD-3-Clause
-# Copyright(c) 2018 Intel Corporation
+# Copyright(c) 2018-2020 Intel Corporation
 
 # Run meson to auto-configure the various builds.
 # * all builds get put in a directory whose name starts with "build-"
 # * if a build-directory already exists we assume it was properly configured
 # Run ninja after configuration is done.
 
+# Get arguments
+usage()
+{
+	echo "Usage: $0
+	      [-b <build directory>]
+	      [-a <dpdk tag or latest for abi check>]
+	      [-u <uri for abi references>]
+	      [-d <directory for abi references>]" 1>&2; exit 1;
+}
+
+# Placeholder default uri
+DPDK_ABI_DEFAULT_URI="http://abi-ref.dpdk.org"
+
+while getopts "a:u:d:b:h" arg; do
+	case $arg in
+	a)
+		if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+			echo "DPDK_ABI_REF_VERSION and -a cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_VERSION=${OPTARG} ;;
+	u)
+		if [ -n "$DPDK_ABI_TAR_URI" ]; then
+			echo "DPDK_ABI_TAR_URI and -u cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_TAR_URI=${OPTARG} ;;
+	d)
+		if [ -n "$DPDK_ABI_REF_DIR" ]; then
+			echo "DPDK_ABI_REF_DIR and -d cannot both be set"
+			exit 1
+		fi
+		DPDK_ABI_REF_DIR=${OPTARG} ;;
+	b)
+		if [ -n "$DPDK_BUILD_TEST_DIR" ]; then
+			echo "DPDK_BUILD_TEST_DIR and -a cannot both be set"
+			exit 1
+		fi
+		DPDK_BUILD_TEST_DIR=${OPTARG} ;;
+	h)
+		usage ;;
+	*)
+		usage ;;
+	esac
+done
+
+if [ -n "$DPDK_ABI_REF_VERSION" ] ; then
+	if [ "$DPDK_ABI_REF_VERSION" = "latest" ] ; then
+		DPDK_ABI_REF_VERSION=$(git ls-remote --tags http://dpdk.org/git/dpdk |
+	        	sed "s/.*\///" | grep -v "r\|{}" |
+			grep '^[^.]*.[^.]*$' | tail -n 1)
+	elif [ -z "$(git ls-remote http://dpdk.org/git/dpdk refs/tags/$DPDK_ABI_REF_VERSION)" ] ; then
+		echo "$DPDK_ABI_REF_VERSION is not a valid DPDK tag"
+		exit 1
+	fi
+fi
+if [ -z $DPDK_ABI_TAR_URI ] ; then
+	DPDK_ABI_TAR_URI=$DPDK_ABI_DEFAULT_URI
+fi
+# allow the generation script to override value with env var
+abi_checks_done=${DPDK_ABI_GEN_REF:-0}
+
 # set pipefail option if possible
 PIPEFAIL=""
 set -o | grep -q pipefail && set -o pipefail && PIPEFAIL=1
@@ -16,7 +78,11 @@ srcdir=$(dirname $(readlink -f $0))/..
 
 MESON=${MESON:-meson}
 use_shared="--default-library=shared"
-builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+builds_dir=${DPDK_BUILD_TEST_DIR:-$srcdir/builds}
+# ensure path is absolute meson returns error when some paths are relative
+if echo "$builds_dir" | grep -qv '^/'; then
+        builds_dir=$srcdir/$builds_dir
+fi
 
 if command -v gmake >/dev/null 2>&1 ; then
 	MAKE=gmake
@@ -123,39 +189,49 @@ install_target () # <builddir> <installdir>
 	fi
 }
 
-build () # <directory> <target compiler | cross file> <meson options>
+abi_gen_check () # no options
 {
-	targetdir=$1
-	shift
-	crossfile=
-	[ -r $1 ] && crossfile=$1 || targetcc=$1
-	shift
-	# skip build if compiler not available
-	command -v ${CC##* } >/dev/null 2>&1 || return 0
-	if [ -n "$crossfile" ] ; then
-		cross="--cross-file $crossfile"
-		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
-			$crossfile | tr -d "'" | tr -d '"')
-	else
-		cross=
+	abirefdir=${DPDK_ABI_REF_DIR:-$builds_dir/__reference}/$DPDK_ABI_REF_VERSION
+	mkdir -p $abirefdir
+	# ensure path is absolute meson returns error when some are relative
+	if echo "$abirefdir" | grep -qv '^/'; then
+		abirefdir=$srcdir/$abirefdir
 	fi
-	load_env $targetcc || return 0
-	config $srcdir $builds_dir/$targetdir $cross --werror $*
-	compile $builds_dir/$targetdir
-	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
-		abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION
-		if [ ! -d $abirefdir/$targetdir ]; then
+	if [ ! -d $abirefdir/$targetdir ]; then
+
+		# try to get abi reference
+		if echo "$DPDK_ABI_TAR_URI" | grep -q '^http'; then
+			if [ $abi_checks_done -gt -1 ]; then
+				if curl --head --fail --silent \
+					"$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz" \
+					>/dev/null; then
+					curl -o $abirefdir/$targetdir.tar.gz \
+					$DPDK_ABI_TAR_URI/$DPDK_ABI_REF_VERSION/$targetdir.tar.gz
+				fi
+			fi
+		elif [ $abi_checks_done -gt -1 ]; then
+			if [ -f "$DPDK_ABI_TAR_URI/$targetdir.tar.gz" ]; then
+				cp $DPDK_ABI_TAR_URI/$targetdir.tar.gz \
+					$abirefdir/
+			fi
+		fi
+		if [ -f "$abirefdir/$targetdir.tar.gz" ]; then
+			tar -xf $abirefdir/$targetdir.tar.gz \
+				-C $abirefdir >/dev/null
+			rm -rf $abirefdir/$targetdir.tar.gz
+		# if no reference can be found then generate one
+		else
 			# clone current sources
 			if [ ! -d $abirefdir/src ]; then
 				git clone --local --no-hardlinks \
-					--single-branch \
-					-b $DPDK_ABI_REF_VERSION \
-					$srcdir $abirefdir/src
+					  --single-branch \
+					  -b $DPDK_ABI_REF_VERSION \
+					  $srcdir $abirefdir/src
 			fi
 
 			rm -rf $abirefdir/build
 			config $abirefdir/src $abirefdir/build $cross \
-				-Dexamples= $*
+			       -Dexamples= $*
 			compile $abirefdir/build
 			install_target $abirefdir/build $abirefdir/$targetdir
 			$srcdir/devtools/gen-abi.sh $abirefdir/$targetdir
@@ -164,17 +240,46 @@ build () # <directory> <target compiler | cross file> <meson options>
 			find $abirefdir/$targetdir/usr/local -name '*.a' -delete
 			rm -rf $abirefdir/$targetdir/usr/local/bin
 			rm -rf $abirefdir/$targetdir/usr/local/share
+			rm -rf $abirefdir/$targetdir/usr/local/lib
 		fi
+	fi
 
-		install_target $builds_dir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install)
-		$srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install)
+	install_target $builds_dir/$targetdir \
+		$(readlink -f $builds_dir/$targetdir/install)
+	$srcdir/devtools/gen-abi.sh \
+		$(readlink -f $builds_dir/$targetdir/install)
+	# check abi if not generating references
+	if [ -z $DPDK_ABI_GEN_REF ] ; then
 		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
 			$(readlink -f $builds_dir/$targetdir/install)
 	fi
 }
 
+build () # <directory> <target compiler | cross file> <meson options>
+{
+	targetdir=$1
+	shift
+	crossfile=
+	[ -r $1 ] && crossfile=$1 || targetcc=$1
+	shift
+	# skip build if compiler not available
+	command -v ${CC##* } >/dev/null 2>&1 || return 0
+	if [ -n "$crossfile" ] ; then
+		cross="--cross-file $crossfile"
+		targetcc=$(sed -n 's,^c[[:space:]]*=[[:space:]]*,,p' \
+			$crossfile | tr -d "'" | tr -d '"')
+	else
+		cross=
+	fi
+	load_env $targetcc || return 0
+	config $srcdir $builds_dir/$targetdir $cross --werror $*
+	compile $builds_dir/$targetdir
+	if [ -n "$DPDK_ABI_REF_VERSION" ] && [ $abi_checks_done -lt 1 ] ; then
+		abi_gen_check
+		abi_checks_done=$((abi_checks_done+1))
+	fi
+}
+
 if [ "$1" = "-vv" ] ; then
 	TEST_MESON_BUILD_VERY_VERBOSE=1
 elif [ "$1" = "-v" ] ; then
@@ -189,7 +294,7 @@ fi
 # shared and static linked builds with gcc and clang
 for c in gcc clang ; do
 	command -v $c >/dev/null 2>&1 || continue
-	for s in static shared ; do
+	for s in shared static ; do
 		export CC="$CCACHE $c"
 		build build-$c-$s $c --default-library=$s
 		unset CC
@@ -211,6 +316,8 @@ build build-x86-mingw $srcdir/config/x86/cross-mingw -Dexamples=helloworld
 
 # generic armv8a with clang as host compiler
 f=$srcdir/config/arm/arm64_armv8_linux_gcc
+# run abi checks with 1 arm build
+abi_checks_done=$((abi_checks_done-1))
 export CC="clang"
 build build-arm64-host-clang $f $use_shared
 unset CC
@@ -231,7 +338,7 @@ done
 build_path=$(readlink -f $builds_dir/build-x86-default)
 export DESTDIR=$build_path/install
 # No need to reinstall if ABI checks are enabled
-if [ -z "$DPDK_ABI_REF_VERSION" ]; then
+if [ -z "$DPDK_ABI_REF_VERSION" ] ; then
 	install_target $build_path $DESTDIR
 fi
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v7 3/4] devtools: change not found to warning check-abi.sh
  2020-10-14 10:41           ` [dpdk-dev] [PATCH v7 " Conor Walsh
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
@ 2020-10-14 10:41             ` Conor Walsh
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
       [not found]             ` <7206c209-ed4a-2aeb-12d8-ee162ef92596@ashroe.eu>
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-14 10:41 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

Change dump file not found from an error to a warning to make check-abi.sh
compatible with the changes to test-meson-builds.sh needed to use
prebuilt references.

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
---
 devtools/check-abi.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/devtools/check-abi.sh b/devtools/check-abi.sh
index ab6748cfb..60d88777e 100755
--- a/devtools/check-abi.sh
+++ b/devtools/check-abi.sh
@@ -46,8 +46,7 @@ for dump in $(find $refdir -name "*.dump"); do
 	fi
 	dump2=$(find $newdir -name $name)
 	if [ -z "$dump2" ] || [ ! -e "$dump2" ]; then
-		echo "Error: can't find $name in $newdir"
-		error=1
+		echo "WARNING: can't find $name in $newdir, are you building with all dependencies?"
 		continue
 	fi
 	abidiff $ABIDIFF_OPTIONS $dump $dump2 || {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* [dpdk-dev] [PATCH v7 4/4] doc: test-meson-builds.sh doc updates
  2020-10-14 10:41           ` [dpdk-dev] [PATCH v7 " Conor Walsh
                               ` (2 preceding siblings ...)
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 3/4] devtools: change not found to warning check-abi.sh Conor Walsh
@ 2020-10-14 10:41             ` Conor Walsh
       [not found]             ` <7206c209-ed4a-2aeb-12d8-ee162ef92596@ashroe.eu>
  4 siblings, 0 replies; 53+ messages in thread
From: Conor Walsh @ 2020-10-14 10:41 UTC (permalink / raw)
  To: mdr, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev, Conor Walsh

Updates to the Checking Compilation and Checking ABI compatibility
sections of the patches part of the contribution guide

Signed-off-by: Conor Walsh <conor.walsh@intel.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
---
 doc/guides/contributing/patches.rst | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 9ff60944c..e11d63bb0 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -470,10 +470,9 @@ The script internally checks for dependencies, then builds for several
 combinations of compilation configuration.
 By default, each build will be put in a subfolder of the current working directory.
 However, if it is preferred to place the builds in a different location,
-the environment variable ``DPDK_BUILD_TEST_DIR`` can be set to that desired location.
-For example, setting ``DPDK_BUILD_TEST_DIR=__builds`` will put all builds
-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.
+the environment variable ``DPDK_BUILD_TEST_DIR`` or the command line argument ``-b``
+can be set to that desired location.
+Environmental variables can also be specified in ``.config/dpdk/devel.config``.
 
 
 .. _integrated_abi_check:
@@ -483,14 +482,17 @@ Checking ABI compatibility
 
 By default, ABI compatibility checks are disabled.
 
-To enable them, a reference version must be selected via the environment
-variable ``DPDK_ABI_REF_VERSION``.
-
-The ``devtools/test-build.sh`` and ``devtools/test-meson-builds.sh`` scripts
-then build this reference version in a temporary directory and store the
-results in a subfolder of the current working directory.
-The environment variable ``DPDK_ABI_REF_DIR`` can be set so that the results go
-to a different location.
+To enable ABI checks the required reference version must be set using either the
+environment variable ``DPDK_ABI_REF_VERSION`` or the command line argument ``-a``.
+The tag ``latest`` is supported, which will select the latest quarterly release.
+e.g. ``./devtools/test-meson-builds.sh -a latest``.
+
+The ``devtools/test-meson-builds.sh`` script will then either build this reference version
+or download a cached version when available in a temporary directory and store the results
+in a subfolder of the current working directory.
+The environment variable ``DPDK_ABI_REF_DIR`` or the argument ``-d`` can be set so that
+the results go to a different location.
+Environmental variables can also be specified in ``.config/dpdk/devel.config``.
 
 
 Sending Patches
-- 
2.25.1


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v7 1/4] devtools: add generation of compressed abi dump archives
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
@ 2020-10-15 10:15               ` Kinsella, Ray
  0 siblings, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-10-15 10:15 UTC (permalink / raw)
  To: Conor Walsh, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev



On 14/10/2020 11:41, Conor Walsh wrote:
> This patch adds a script that generates compressed archives
> containing .dump files which can be used to perform abi
> breakage checking in test-meson-build.sh.
> 
> Invoke using "./gen-abi-tarballs.sh [-v <dpdk tag>]"
>  - <dpdk tag>: dpdk tag e.g. "v20.11" or "latest"
> e.g. "./gen-abi-tarballs.sh -v latest"
> 
> If no tag is specified, the script will default to "latest"
> Using these parameters the script will produce several *.tar.gz
> archives containing .dump files required to do abi breakage checking
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v7 2/4] devtools: abi and UX changes for test-meson-builds.sh
  2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
@ 2020-10-15 10:16               ` Kinsella, Ray
  0 siblings, 0 replies; 53+ messages in thread
From: Kinsella, Ray @ 2020-10-15 10:16 UTC (permalink / raw)
  To: Conor Walsh, nhorman, bruce.richardson, thomas, david.marchand; +Cc: dev



On 14/10/2020 11:41, Conor Walsh wrote:
> The core reason for this patch is to reduce the amount of time needed to
> run abi checks. The number of abi checks being run has been reduced to
> only 2 (1 x86_64 and 1 arm). The script can now also take adavtage of
> prebuilt abi references.
> 
> Invoke using "./test-meson-builds.sh [-b <build directory>]
>    [-a <dpdk tag or latest for abi check>] [-u <uri for abi references>]
>    [-d <directory for abi references>]"
>  - <build directory>: directory to store builds (relative or absolute)
>  - <dpdk tag or latest for abi check>: dpdk tag e.g. "v20.11" or "latest"
>  - <uri for abi references>: http location or directory to get prebuilt
>    abi references from
>  - <directory for abi references>: directory to store abi references
>    (relative or absolute)
> e.g. "./test-meson-builds.sh -a latest"
> If no flags are specified test-meson-builds.sh will run the standard
> meson tests with default options unless environmental variables are
> specified.
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v7 0/4] devtools: abi breakage checks
       [not found]               ` <CAJFAV8wmpft6XLRg1RAL+d4ibbJVrR9C0ghkE-kqyig_q_Meeg@mail.gmail.com>
@ 2020-11-03 10:07                 ` Kinsella, Ray
  2020-11-10 12:53                   ` David Marchand
  0 siblings, 1 reply; 53+ messages in thread
From: Kinsella, Ray @ 2020-11-03 10:07 UTC (permalink / raw)
  To: David Marchand
  Cc: Walsh, Conor, dpdk-dev, Luca Boccassi, Dodji Seketeli, Mcnamara, John

Hi David,

Came across an issue with this.

Essentially what is happening is that an ABI dump file generated with a newer versions of libabigail
is not guaranteed to be 100% compatible with a older versions.

That then adds a wrinkle that we need may need to look at maintaining abi dump archives per distro release,
or libabigail version depending on how you look at it. 

An alter approach suggested by Dodi would be to just archive the binaries somewhere instead, 
and regenerate the dumps at build time. That _may_ be feasible, 
but you lose some of the benefit (build time saving) compared to archiving the abi dumps. 

The most sensible approach to archiving the binaries.
is to use DPDK release os packaging for this, installed to a fs sandbox. 

So the next steps are figuring out, which is the better option between 
maintaining multiple abi dump archives, one per supported os distro. 
or looking at what needs to happen with DPDK os packaging. 

So some work still to do here. 

Thanks,

Ray K

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v7 0/4] devtools: abi breakage checks
  2020-11-03 10:07                 ` [dpdk-dev] [PATCH v7 0/4] devtools: abi breakage checks Kinsella, Ray
@ 2020-11-10 12:53                   ` David Marchand
  2020-11-10 13:54                     ` Kinsella, Ray
  0 siblings, 1 reply; 53+ messages in thread
From: David Marchand @ 2020-11-10 12:53 UTC (permalink / raw)
  To: Kinsella, Ray
  Cc: Walsh, Conor, dpdk-dev, Luca Boccassi, Dodji Seketeli, Mcnamara, John

On Tue, Nov 3, 2020 at 11:07 AM Kinsella, Ray <mdr@ashroe.eu> wrote:
> Came across an issue with this.
>
> Essentially what is happening is that an ABI dump file generated with a newer versions of libabigail
> is not guaranteed to be 100% compatible with a older versions.
>
> That then adds a wrinkle that we need may need to look at maintaining abi dump archives per distro release,
> or libabigail version depending on how you look at it.

This is something I had encountered.

The Travis script flushes the ABI cache on a libabigail version change.
When using the test-meson-builds.sh integration, the gen-abi.sh
devtools script can be called to regenerate the dump files from the
existing local binaries.


>
> An alter approach suggested by Dodi would be to just archive the binaries somewhere instead,
> and regenerate the dumps at build time. That _may_ be feasible,
> but you lose some of the benefit (build time saving) compared to archiving the abi dumps.
>
> The most sensible approach to archiving the binaries.
> is to use DPDK release os packaging for this, installed to a fs sandbox.
>
> So the next steps are figuring out, which is the better option between
> maintaining multiple abi dump archives, one per supported os distro.
> or looking at what needs to happen with DPDK os packaging.
>
> So some work still to do here.

I am still unconvinced about the approach, but I'll wait for your next proposal.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v7 0/4] devtools: abi breakage checks
  2020-11-10 12:53                   ` David Marchand
@ 2020-11-10 13:54                     ` Kinsella, Ray
  2020-11-10 13:57                       ` David Marchand
  0 siblings, 1 reply; 53+ messages in thread
From: Kinsella, Ray @ 2020-11-10 13:54 UTC (permalink / raw)
  To: David Marchand
  Cc: Walsh, Conor, dpdk-dev, Luca Boccassi, Dodji Seketeli, Mcnamara, John



On 10/11/2020 12:53, David Marchand wrote:
> On Tue, Nov 3, 2020 at 11:07 AM Kinsella, Ray <mdr@ashroe.eu> wrote:
>> Came across an issue with this.
>>
>> Essentially what is happening is that an ABI dump file generated with a newer versions of libabigail
>> is not guaranteed to be 100% compatible with a older versions.
>>
>> That then adds a wrinkle that we need may need to look at maintaining abi dump archives per distro release,
>> or libabigail version depending on how you look at it.
> 
> This is something I had encountered.
> 
> The Travis script flushes the ABI cache on a libabigail version change.

Why would the libabigail version change in Travis - due do an OS update or the like?

> When using the test-meson-builds.sh integration, the gen-abi.sh
> devtools script can be called to regenerate the dump files from the
> existing local binaries.
> 
> 
>>
>> An alter approach suggested by Dodi would be to just archive the binaries somewhere instead,
>> and regenerate the dumps at build time. That _may_ be feasible,
>> but you lose some of the benefit (build time saving) compared to archiving the abi dumps.
>>
>> The most sensible approach to archiving the binaries.
>> is to use DPDK release os packaging for this, installed to a fs sandbox.
>>
>> So the next steps are figuring out, which is the better option between
>> maintaining multiple abi dump archives, one per supported os distro.
>> or looking at what needs to happen with DPDK os packaging.
>>
>> So some work still to do here.
> 
> I am still unconvinced about the approach, but I'll wait for your next proposal.
> 
> 

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [dpdk-dev] [PATCH v7 0/4] devtools: abi breakage checks
  2020-11-10 13:54                     ` Kinsella, Ray
@ 2020-11-10 13:57                       ` David Marchand
  0 siblings, 0 replies; 53+ messages in thread
From: David Marchand @ 2020-11-10 13:57 UTC (permalink / raw)
  To: Kinsella, Ray
  Cc: Walsh, Conor, dpdk-dev, Luca Boccassi, Dodji Seketeli, Mcnamara, John

On Tue, Nov 10, 2020 at 2:54 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
> > The Travis script flushes the ABI cache on a libabigail version change.
>
> Why would the libabigail version change in Travis - due do an OS update or the like?

Because in Travis, we compiled our own version of libabigail, the one
in ubuntu 18.04 being buggy (opened some bug, never got any feedback).
I had left automatic flush to test different versions.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2020-11-10 13:57 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-10 14:01 [dpdk-dev] [PATCH 0/4] abi breakage checks for meson Conor Walsh
2020-09-10 14:01 ` [dpdk-dev] [PATCH 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
2020-09-10 14:01 ` [dpdk-dev] [PATCH 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
2020-09-10 14:01 ` [dpdk-dev] [PATCH 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
2020-09-10 14:01 ` [dpdk-dev] [PATCH 4/4] build: add abi breakage checks to meson Conor Walsh
2020-09-10 14:21 ` [dpdk-dev] [PATCH v2 0/4] abi breakage checks for meson Conor Walsh
2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
2020-09-10 14:21   ` [dpdk-dev] [PATCH v2 4/4] build: add abi breakage checks to meson Conor Walsh
2020-09-11 16:03   ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Conor Walsh
2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
2020-09-14 12:50       ` Burakov, Anatoly
2020-09-15 14:35         ` Aaron Conole
2020-09-15 14:49           ` Walsh, Conor
2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
2020-09-11 16:03     ` [dpdk-dev] [PATCH v3 4/4] build: add abi breakage checks to meson Conor Walsh
2020-09-14  8:08     ` [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson Thomas Monjalon
2020-09-14  8:30       ` Kinsella, Ray
2020-09-14  9:34       ` Kinsella, Ray
2020-09-18 12:11     ` [dpdk-dev] [PATCH v4 " Conor Walsh
2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 1/4] devtools: bug fix for gen-abi.sh Conor Walsh
2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 2/4] devtools: add generation of compressed abi dump archives Conor Walsh
2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 3/4] buildtools: add script to setup abi checks for meson Conor Walsh
2020-09-18 12:11       ` [dpdk-dev] [PATCH v4 4/4] build: add abi breakage checks to meson Conor Walsh
2020-10-12  8:08       ` [dpdk-dev] [PATCH v5 0/4] devtools: abi breakage checks Conor Walsh
2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 3/4] devtools: change dump file not found to warning in check-abi.sh Conor Walsh
2020-10-12  8:08         ` [dpdk-dev] [PATCH v5 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
2020-10-12 13:03         ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Conor Walsh
2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
2020-10-14  9:38             ` Kinsella, Ray
2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
2020-10-14  9:43             ` Kinsella, Ray
2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 3/4] devtools: change dump file not found to warning in check-abi.sh Conor Walsh
2020-10-14  9:44             ` Kinsella, Ray
2020-10-12 13:03           ` [dpdk-dev] [PATCH v6 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
2020-10-14  9:46             ` Kinsella, Ray
2020-10-14  9:37           ` [dpdk-dev] [PATCH v6 0/4] devtools: abi breakage checks Kinsella, Ray
2020-10-14 10:33             ` Walsh, Conor
2020-10-14 10:41           ` [dpdk-dev] [PATCH v7 " Conor Walsh
2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 1/4] devtools: add generation of compressed abi dump archives Conor Walsh
2020-10-15 10:15               ` Kinsella, Ray
2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 2/4] devtools: abi and UX changes for test-meson-builds.sh Conor Walsh
2020-10-15 10:16               ` Kinsella, Ray
2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 3/4] devtools: change not found to warning check-abi.sh Conor Walsh
2020-10-14 10:41             ` [dpdk-dev] [PATCH v7 4/4] doc: test-meson-builds.sh doc updates Conor Walsh
     [not found]             ` <7206c209-ed4a-2aeb-12d8-ee162ef92596@ashroe.eu>
     [not found]               ` <CAJFAV8wmpft6XLRg1RAL+d4ibbJVrR9C0ghkE-kqyig_q_Meeg@mail.gmail.com>
2020-11-03 10:07                 ` [dpdk-dev] [PATCH v7 0/4] devtools: abi breakage checks Kinsella, Ray
2020-11-10 12:53                   ` David Marchand
2020-11-10 13:54                     ` Kinsella, Ray
2020-11-10 13:57                       ` David Marchand

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