DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build
@ 2019-10-23  1:07 Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 1/6] build: enable debug info by default in meson builds Kevin Laatz
                   ` (7 more replies)
  0 siblings, 8 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-10-23  1:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, ray.kinsella, Kevin Laatz

With the recent changes made to stabilize ABI versioning in DPDK, it will
become increasingly important to check patches for ABI compatibility. We
propose adding the ABI compatibility checking to be done as part of the
build.

The advantages to adding the ABI compatibility checking to the build are
two-fold. Firstly, developers can easily check their patches to make sure
they don’t break the ABI without adding any extra steps. Secondly, it
makes the integration into existing CI seamless since there are no extra
scripts to make the CI run. The build will run as usual and if an
incompatibility is detected in the ABI, the build will fail and show the
incompatibility. As an added bonus, enabling the ABI compatibility checks
does not impact the build speed.

The proposed solution works as follows:
1. Generate the ABI dump of the baseline. This can be done with the new
   script added in this RFC. This step will only need to be done when the
   ABI version changes (so once a year) and can be added to master so it
   exists by default. This step can be skipped if the dump files for the
   baseline already exist.
2. Build with meson. If there is an ABI incompatibility, the build will
   fail and print the incompatibility information.

The patches accompanying this RFC add the ABI dump file generating script,
the meson option required to enable/disable the checks, and the required
meson changes to run the compatibility checks during the build.

This patchset depends on:
 - The "Implement the new ABI policy and add helper scripts" patchset
   (http://patches.dpdk.org/project/dpdk/list/?series=6913).
 - The "Add scanning for experimental symbols to meson" patchset
   (http://patches.dpdk.org/project/dpdk/list/?series=6744).
 - "build: enable extra warnings for meson build" patch
   (http://patches.dpdk.org/patch/60622/).

Bruce Richardson (2):
  build: enable debug info by default in meson builds
  build: use meson warning levels

Kevin Laatz (4):
  devtools: add abi dump generation script
  build: add meson option for abi related checks
  build: add lib abi checks to meson
  build: add drivers abi checks to meson

 buildtools/meson.build   |  4 ++++
 config/meson.build       | 40 +++++++++++++++++++++-------------------
 devtools/gen-abi-dump.sh | 24 ++++++++++++++++++++++++
 drivers/meson.build      | 17 ++++++++++++++++-
 lib/meson.build          | 17 ++++++++++++++++-
 meson.build              |  9 ++++++++-
 meson_options.txt        |  2 ++
 7 files changed, 91 insertions(+), 22 deletions(-)
 create mode 100755 devtools/gen-abi-dump.sh

-- 
2.17.1


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

* [dpdk-dev] [RFC 1/6] build: enable debug info by default in meson builds
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
@ 2019-10-23  1:07 ` Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 2/6] build: use meson warning levels Kevin Laatz
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-10-23  1:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, ray.kinsella

From: Bruce Richardson <bruce.richardson@intel.com>

We can turn on debug info by default in meson builds, since it has no
performance penalty. This is done by changing the default build type from
"release" to "debugoptimized". Since the latter using O2, we can using
extra cflags to override that back to O3, which will make little real
difference for actual debugging.

For real debug builds, the user can still do "meson --buildtype=debug ..."
and to remove the debug info "meson --buildtype=release ..." can be used.
These are all standard meson options.

The advantage of having debug builds by default using meson settings is
that we can then add checks for ABI compatibility into each build, and
disable them if we detect that the user has turned off the debug info.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 meson.build | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index c5a3dda26..b77ccd6ef 100644
--- a/meson.build
+++ b/meson.build
@@ -7,10 +7,16 @@ project('DPDK', 'C',
 	version: run_command(find_program('cat', 'more'),
 		files('VERSION')).stdout().strip(),
 	license: 'BSD',
-	default_options: ['buildtype=release', 'default_library=static'],
+	default_options: ['buildtype=debugoptimized',
+			'default_library=static'],
 	meson_version: '>= 0.47.1'
 )
 
+# for default "debugoptimized" builds override optimization level 2 with 3
+if get_option('buildtype') == 'debugoptimized'
+	add_project_arguments('-O3', language: 'c')
+endif
+
 # set up some global vars for compiler, platform, configuration, etc.
 cc = meson.get_compiler('c')
 dpdk_conf = configuration_data()
-- 
2.17.1


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

* [dpdk-dev] [RFC 2/6] build: use meson warning levels
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 1/6] build: enable debug info by default in meson builds Kevin Laatz
@ 2019-10-23  1:07 ` Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 3/6] devtools: add abi dump generation script Kevin Laatz
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-10-23  1:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, ray.kinsella

From: Bruce Richardson <bruce.richardson@intel.com>

Rather than trying to manage all the cflags ourselves, we can use meson
warning levels to give the user more control. We remove the Wextra flag and
rely on meson to add it, by bumping up our default warning level.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 config/meson.build | 40 +++++++++++++++++++++-------------------
 meson.build        |  3 ++-
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index b383e79c0..3bcfe0668 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -141,31 +141,33 @@ endif
 # add -include rte_config to cflags
 add_project_arguments('-include', 'rte_config.h', language: 'c')
 
-# enable extra warnings and disable any unwanted warnings
+# enable extra warnings and disable any unwanted warnings. "-Wall" is added
+# by meson at warning level 1, and "-Wextra" at level 2, so we can omit
+# those. Add extra warnings at level 2 or above. (2 is default level).
 warning_flags = [
-	# -Wall is added by meson by default, so add -Wextra only
-	'-Wextra',
-
-	# additional warnings in alphabetical order
-	'-Wcast-qual',
-	'-Wdeprecated',
-	'-Wformat-nonliteral',
-	'-Wformat-security',
-	'-Wmissing-declarations',
-	'-Wmissing-prototypes',
-	'-Wnested-externs',
-	'-Wold-style-definition',
-	'-Wpointer-arith',
-	'-Wsign-compare',
-	'-Wstrict-prototypes',
-	'-Wundef',
-	'-Wwrite-strings',
-
 	# globally disabled warnings
 	'-Wno-address-of-packed-member',
 	'-Wno-packed-not-aligned',
 	'-Wno-missing-field-initializers'
 ]
+if get_option('warning_level').to_int() >= 2
+	warning_flags += [
+		# additional warnings in alphabetical order
+		'-Wcast-qual',
+		'-Wdeprecated',
+		'-Wformat-nonliteral',
+		'-Wformat-security',
+		'-Wmissing-declarations',
+		'-Wmissing-prototypes',
+		'-Wnested-externs',
+		'-Wold-style-definition',
+		'-Wpointer-arith',
+		'-Wsign-compare',
+		'-Wstrict-prototypes',
+		'-Wundef',
+		'-Wwrite-strings',
+	]
+endif
 if not dpdk_conf.get('RTE_ARCH_64')
 # for 32-bit, don't warn about casting a 32-bit pointer to 64-bit int - it's fine!!
 	warning_flags += '-Wno-pointer-to-int-cast'
diff --git a/meson.build b/meson.build
index b77ccd6ef..dd83b0bcb 100644
--- a/meson.build
+++ b/meson.build
@@ -8,7 +8,8 @@ project('DPDK', 'C',
 		files('VERSION')).stdout().strip(),
 	license: 'BSD',
 	default_options: ['buildtype=debugoptimized',
-			'default_library=static'],
+			'default_library=static',
+			'warning_level=2'],
 	meson_version: '>= 0.47.1'
 )
 
-- 
2.17.1


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

* [dpdk-dev] [RFC 3/6] devtools: add abi dump generation script
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 1/6] build: enable debug info by default in meson builds Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 2/6] build: use meson warning levels Kevin Laatz
@ 2019-10-23  1:07 ` Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 4/6] build: add meson option for abi related checks Kevin Laatz
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-10-23  1:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, ray.kinsella, Kevin Laatz

This patch adds a script to generate ABI dump files. These files will be
required to perform ABI compatibility checks during the build later in the
patchset. This script should be run on a DPDK version with a stable ABI.

Since this is a tool designed for human use, we simplify it to just work
off a whole build directory, taking the parameter of the builddir and
generating the lib|drivers/abi dir. This is hardcoded into the script since
the meson build expects the .dump files in these directories.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 devtools/gen-abi-dump.sh | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100755 devtools/gen-abi-dump.sh

diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh
new file mode 100755
index 000000000..ffedef10c
--- /dev/null
+++ b/devtools/gen-abi-dump.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+builddir=$1
+
+if [ -z "$builddir" ] ; then
+	echo "Usage: $(basename $0) build_dir"
+	exit 1
+fi
+
+if [ ! -d "$builddir" ] ; then
+	echo "Error: build directory, '$builddir', doesn't exist"
+	exit 1
+fi
+
+for d in lib drivers ; do
+	mkdir -p $d/abi
+
+	for f in $builddir/$d/*.so* ; do
+		test -L "$f" && continue
+
+		libname=$(basename $f)
+		abidw --out-file $d/abi/${libname%.so*}.dump $f || exit 1
+	done
+done
-- 
2.17.1


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

* [dpdk-dev] [RFC 4/6] build: add meson option for abi related checks
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
                   ` (2 preceding siblings ...)
  2019-10-23  1:07 ` [dpdk-dev] [RFC 3/6] devtools: add abi dump generation script Kevin Laatz
@ 2019-10-23  1:07 ` Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 5/6] build: add lib abi checks to meson Kevin Laatz
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-10-23  1:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, ray.kinsella, Kevin Laatz

This patch adds a new meson option for running ABI compatibility checks
during the build. If enabled, the lib and drivers .so files will be
compared against any existing ABI dump files in lib|drivers/abi of the
source directory. If there are any incompatibilities, the build will fail
and display the incompatibility.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 meson_options.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson_options.txt b/meson_options.txt
index 000e38fd9..aefab391a 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('abi_compat_checks', type: 'boolean', value: true,
+	description: 'enable abi compatibility checks to run during the build')
 option('allow_invalid_socket_id', type: 'boolean', value: false,
 	description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
-- 
2.17.1


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

* [dpdk-dev] [RFC 5/6] build: add lib abi checks to meson
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
                   ` (3 preceding siblings ...)
  2019-10-23  1:07 ` [dpdk-dev] [RFC 4/6] build: add meson option for abi related checks Kevin Laatz
@ 2019-10-23  1:07 ` Kevin Laatz
  2019-10-23  1:07 ` [dpdk-dev] [RFC 6/6] build: add drivers " Kevin Laatz
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-10-23  1:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, ray.kinsella, Kevin Laatz

This patch adds the ABI compatibility check for the lib directory to the
meson build. If enabled, the ABI compatibility checks will run for all
.so's in the lib directory (provided a matching dump file exists). The
build will fail if an ABI incompatibility is detected.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/meson.build |  4 ++++
 lib/meson.build        | 17 ++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/buildtools/meson.build b/buildtools/meson.build
index 1ec2c2f95..a895c791c 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -7,6 +7,10 @@ pmdinfo = find_program('gen-pmdinfo-cfile.sh')
 
 check_experimental_syms = find_program('check-experimental-syms.sh')
 
+if get_option('abi_compat_checks')
+	abidiff = find_program('abidiff')
+endif
+
 # set up map-to-def script using python, either built-in or external
 python3 = import('python').find_installation(required: false)
 if python3.found()
diff --git a/lib/meson.build b/lib/meson.build
index 7849ac9f7..da180fb37 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -146,7 +146,9 @@ foreach l:libraries
 						version_map, '@INPUT@'],
 					capture: true,
 					input: static_lib,
-					output: name + '.exp_chk')
+					output: name + '.exp_chk',
+					install: false,
+					build_by_default: get_option('abi_compat_checks'))
 			endif
 
 			shared_lib = shared_library(libname,
@@ -164,6 +166,19 @@ foreach l:libraries
 					include_directories: includes,
 					dependencies: shared_deps)
 
+			if is_experimental == 0
+				custom_target(dir_name + '.abi_chk',
+					command: [abidiff,
+						meson.source_root() + '/lib/abi/'
+						+ dir_name + '.dump',
+						'@INPUT@'],
+					input: shared_lib,
+					output: dir_name + '.abi_chk',
+					capture: true,
+					install: false,
+					build_by_default: get_option('abi_compat_checks'))
+			endif
+
 			dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 		endif # sources.length() > 0
-- 
2.17.1


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

* [dpdk-dev] [RFC 6/6] build: add drivers abi checks to meson
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
                   ` (4 preceding siblings ...)
  2019-10-23  1:07 ` [dpdk-dev] [RFC 5/6] build: add lib abi checks to meson Kevin Laatz
@ 2019-10-23  1:07 ` " Kevin Laatz
  2019-11-29 12:13 ` [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build David Marchand
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-10-23  1:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson, thomas, ray.kinsella, Kevin Laatz

This patch adds the ABI compatibility check for the drivers directory to
the meson build. If enabled, the ABI compatibility checks will run for all
.so's in the lib directory (provided a matching dump file exists). The
build will fail if an ABI incompatibility is detected.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 drivers/meson.build | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/meson.build b/drivers/meson.build
index 3202ba00d..0fda5a9e0 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -158,7 +158,9 @@ foreach class:dpdk_driver_classes
 						version_map, '@INPUT@'],
 					capture: true,
 					input: static_lib,
-					output: lib_name + '.exp_chk')
+					output: lib_name + '.exp_chk'
+					install: false,
+					build_by_default: get_option('abi_compat_checks'))
 			endif
 
 			shared_lib = shared_library(lib_name,
@@ -183,6 +185,19 @@ foreach class:dpdk_driver_classes
 					include_directories: includes,
 					dependencies: static_objs)
 
+			if is_experimental == 0
+				custom_target('lib' + lib_name + '.abi_chk',
+					command: [abidiff,
+						meson.source_root() + '/drivers/abi/lib'
+						+ lib_name + '.dump',
+						'@INPUT@'],
+					input: shared_lib,
+					output: 'lib' + lib_name + '.abi_chk',
+					capture: true,
+					install: false,
+					build_by_default: get_option('abi_compat_checks'))
+			endif
+
 			dpdk_drivers += static_lib
 
 			set_variable('shared_@0@'.format(lib_name), shared_dep)
-- 
2.17.1


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

* Re: [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
                   ` (5 preceding siblings ...)
  2019-10-23  1:07 ` [dpdk-dev] [RFC 6/6] build: add drivers " Kevin Laatz
@ 2019-11-29 12:13 ` David Marchand
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
  7 siblings, 0 replies; 30+ messages in thread
From: David Marchand @ 2019-11-29 12:13 UTC (permalink / raw)
  To: Kevin Laatz; +Cc: dev, Bruce Richardson, Thomas Monjalon, Kinsella, Ray

Hello Kevin,

On Wed, Oct 23, 2019 at 11:26 AM Kevin Laatz <kevin.laatz@intel.com> wrote:
>
> With the recent changes made to stabilize ABI versioning in DPDK, it will
> become increasingly important to check patches for ABI compatibility. We
> propose adding the ABI compatibility checking to be done as part of the
> build.
>
> The advantages to adding the ABI compatibility checking to the build are
> two-fold. Firstly, developers can easily check their patches to make sure
> they don’t break the ABI without adding any extra steps. Secondly, it
> makes the integration into existing CI seamless since there are no extra
> scripts to make the CI run. The build will run as usual and if an
> incompatibility is detected in the ABI, the build will fail and show the
> incompatibility. As an added bonus, enabling the ABI compatibility checks
> does not impact the build speed.
>
> The proposed solution works as follows:
> 1. Generate the ABI dump of the baseline. This can be done with the new
>    script added in this RFC. This step will only need to be done when the
>    ABI version changes (so once a year) and can be added to master so it
>    exists by default. This step can be skipped if the dump files for the
>    baseline already exist.
> 2. Build with meson. If there is an ABI incompatibility, the build will
>    fail and print the incompatibility information.
>
> The patches accompanying this RFC add the ABI dump file generating script,
> the meson option required to enable/disable the checks, and the required
> meson changes to run the compatibility checks during the build.

Could you rebase this series on master?
Thanks.


-- 
David Marchand


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

* [dpdk-dev] [PATCH v2 0/7] Add ABI compatibility checks to the meson build
  2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
                   ` (6 preceding siblings ...)
  2019-11-29 12:13 ` [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build David Marchand
@ 2019-11-29 17:10 ` " Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 1/7] build: enable debug info by default in meson builds Kevin Laatz
                     ` (7 more replies)
  7 siblings, 8 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

With the recent changes made to stabilize ABI versioning in DPDK, it will
become increasingly important to check patches for ABI compatibility. We
propose adding the ABI compatibility checking to be done as part of the
build.

The advantages to adding the ABI compatibility checking to the build are
two-fold. Firstly, developers can easily check their patches to make sure
they don’t break the ABI without adding any extra steps. Secondly, it
makes the integration into existing CI seamless since there are no extra
scripts to make the CI run. The build will run as usual and if an
incompatibility is detected in the ABI, the build will fail and show the
incompatibility. As an added bonus, enabling the ABI compatibility checks
does not impact the build speed.

The proposed solution works as follows:
1. Generate the ABI dump of the baseline. This can be done with the new
   script added in this RFC. This step will only need to be done when the
   ABI version changes (so once a year) and can be added to master so it
   exists by default. This step can be skipped if the dump files for the
   baseline already exist.
2. Build with meson. If there is an ABI incompatibility, the build will
   fail and print the incompatibility information.

The patches accompanying this RFC add the ABI dump file generating script,
the meson option required to enable/disable the checks, and the required
meson changes to run the compatibility checks during the build.

---
v2:
  - Rebased on master for 19.11.
  - Moved the experimental syms checks next to the abi checks. This also
    removed the dependency on the experimental checks from the shared
    build.
  - General cleanup.

Bruce Richardson (2):
  build: enable debug info by default in meson builds
  build: use meson warning levels

Kevin Laatz (5):
  devtools: add abi dump generation script
  build: add meson option for abi related checks
  build: add lib abi checks to meson
  build: add drivers abi checks to meson
  build: clean up experimental syms check

 buildtools/meson.build   |  4 ++++
 config/meson.build       | 40 +++++++++++++++++++++-------------------
 devtools/gen-abi-dump.sh | 24 ++++++++++++++++++++++++
 drivers/meson.build      | 34 ++++++++++++++++++++++++----------
 lib/meson.build          | 34 ++++++++++++++++++++++++----------
 meson.build              |  9 ++++++++-
 meson_options.txt        |  2 ++
 7 files changed, 107 insertions(+), 40 deletions(-)
 create mode 100755 devtools/gen-abi-dump.sh

-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 1/7] build: enable debug info by default in meson builds
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
@ 2019-11-29 17:10   ` Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 2/7] build: use meson warning levels Kevin Laatz
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella

From: Bruce Richardson <bruce.richardson@intel.com>

We can turn on debug info by default in meson builds, since it has no
performance penalty. This is done by changing the default build type from
"release" to "debugoptimized". Since the latter using O2, we can using
extra cflags to override that back to O3, which will make little real
difference for actual debugging.

For real debug builds, the user can still do "meson --buildtype=debug ..."
and to remove the debug info "meson --buildtype=release ..." can be used.
These are all standard meson options.

The advantage of having debug builds by default using meson settings is
that we can then add checks for ABI compatibility into each build, and
disable them if we detect that the user has turned off the debug info.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 meson.build | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index b7ae9c8d9..3b7a2e7de 100644
--- a/meson.build
+++ b/meson.build
@@ -7,10 +7,16 @@ project('DPDK', 'C',
 	version: run_command(find_program('cat', 'more'),
 		files('VERSION')).stdout().strip(),
 	license: 'BSD',
-	default_options: ['buildtype=release', 'default_library=static'],
+	default_options: ['buildtype=debugoptimized',
+			'default_library=static'],
 	meson_version: '>= 0.47.1'
 )
 
+# for default "debugoptimized" builds override optimization level 2 with 3
+if get_option('buildtype') == 'debugoptimized'
+	add_project_arguments('-O3', language: 'c')
+endif
+
 # set up some global vars for compiler, platform, configuration, etc.
 cc = meson.get_compiler('c')
 dpdk_conf = configuration_data()
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 2/7] build: use meson warning levels
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 1/7] build: enable debug info by default in meson builds Kevin Laatz
@ 2019-11-29 17:10   ` Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 3/7] devtools: add abi dump generation script Kevin Laatz
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella

From: Bruce Richardson <bruce.richardson@intel.com>

Rather than trying to manage all the cflags ourselves, we can use meson
warning levels to give the user more control. We remove the Wextra flag and
rely on meson to add it, by bumping up our default warning level.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 config/meson.build | 40 +++++++++++++++++++++-------------------
 meson.build        |  3 ++-
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 364a8d739..36a594970 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -158,31 +158,33 @@ endif
 # add -include rte_config to cflags
 add_project_arguments('-include', 'rte_config.h', language: 'c')
 
-# enable extra warnings and disable any unwanted warnings
+# enable extra warnings and disable any unwanted warnings. "-Wall" is added
+# by meson at warning level 1, and "-Wextra" at level 2, so we can omit
+# those. Add extra warnings at level 2 or above. (2 is default level).
 warning_flags = [
-	# -Wall is added by meson by default, so add -Wextra only
-	'-Wextra',
-
-	# additional warnings in alphabetical order
-	'-Wcast-qual',
-	'-Wdeprecated',
-	'-Wformat-nonliteral',
-	'-Wformat-security',
-	'-Wmissing-declarations',
-	'-Wmissing-prototypes',
-	'-Wnested-externs',
-	'-Wold-style-definition',
-	'-Wpointer-arith',
-	'-Wsign-compare',
-	'-Wstrict-prototypes',
-	'-Wundef',
-	'-Wwrite-strings',
-
 	# globally disabled warnings
 	'-Wno-address-of-packed-member',
 	'-Wno-packed-not-aligned',
 	'-Wno-missing-field-initializers'
 ]
+if get_option('warning_level').to_int() >= 2
+	warning_flags += [
+		# additional warnings in alphabetical order
+		'-Wcast-qual',
+		'-Wdeprecated',
+		'-Wformat-nonliteral',
+		'-Wformat-security',
+		'-Wmissing-declarations',
+		'-Wmissing-prototypes',
+		'-Wnested-externs',
+		'-Wold-style-definition',
+		'-Wpointer-arith',
+		'-Wsign-compare',
+		'-Wstrict-prototypes',
+		'-Wundef',
+		'-Wwrite-strings',
+	]
+endif
 if not dpdk_conf.get('RTE_ARCH_64')
 # for 32-bit, don't warn about casting a 32-bit pointer to 64-bit int - it's fine!!
 	warning_flags += '-Wno-pointer-to-int-cast'
diff --git a/meson.build b/meson.build
index 3b7a2e7de..7a8f97ad6 100644
--- a/meson.build
+++ b/meson.build
@@ -8,7 +8,8 @@ project('DPDK', 'C',
 		files('VERSION')).stdout().strip(),
 	license: 'BSD',
 	default_options: ['buildtype=debugoptimized',
-			'default_library=static'],
+			'default_library=static',
+			'warning_level=2'],
 	meson_version: '>= 0.47.1'
 )
 
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 3/7] devtools: add abi dump generation script
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 1/7] build: enable debug info by default in meson builds Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 2/7] build: use meson warning levels Kevin Laatz
@ 2019-11-29 17:10   ` Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 4/7] build: add meson option for abi related checks Kevin Laatz
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds a script to generate ABI dump files. These files will be
required to perform ABI compatibility checks during the build later in the
patchset. This script should be run on a DPDK version with a stable ABI.

Since this is a tool designed for human use, we simplify it to just work
off a whole build directory, taking the parameter of the builddir and
generating the lib|drivers/abi dir. This is hardcoded into the script since
the meson build expects the .dump files in these directories.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 devtools/gen-abi-dump.sh | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100755 devtools/gen-abi-dump.sh

diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh
new file mode 100755
index 000000000..ffedef10c
--- /dev/null
+++ b/devtools/gen-abi-dump.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+builddir=$1
+
+if [ -z "$builddir" ] ; then
+	echo "Usage: $(basename $0) build_dir"
+	exit 1
+fi
+
+if [ ! -d "$builddir" ] ; then
+	echo "Error: build directory, '$builddir', doesn't exist"
+	exit 1
+fi
+
+for d in lib drivers ; do
+	mkdir -p $d/abi
+
+	for f in $builddir/$d/*.so* ; do
+		test -L "$f" && continue
+
+		libname=$(basename $f)
+		abidw --out-file $d/abi/${libname%.so*}.dump $f || exit 1
+	done
+done
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 4/7] build: add meson option for abi related checks
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
                     ` (2 preceding siblings ...)
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 3/7] devtools: add abi dump generation script Kevin Laatz
@ 2019-11-29 17:10   ` Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 5/7] build: add lib abi checks to meson Kevin Laatz
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds a new meson option for running ABI compatibility checks
during the build. If enabled, the lib and drivers .so files will be
compared against any existing ABI dump files in lib|drivers/abi of the
source directory. If there are any incompatibilities, the build will fail
and display the incompatibility.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 meson_options.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson_options.txt b/meson_options.txt
index bc369d06c..5f42def1d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('compat_checks', type: 'boolean', value: true,
+	description: 'enable abi compatibility checks and experimental syms checks to run during the build')
 option('disable_drivers', type: 'string', value: '',
 	description: 'Comma-separated list of drivers to explicitly disable.')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 5/7] build: add lib abi checks to meson
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
                     ` (3 preceding siblings ...)
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 4/7] build: add meson option for abi related checks Kevin Laatz
@ 2019-11-29 17:10   ` Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 6/7] build: add drivers " Kevin Laatz
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds the ABI compatibility check for the lib directory to the
meson build. If enabled, the ABI compatibility checks will run for all
.so's in the lib directory (provided a matching dump file exists). The
build will fail if an ABI incompatibility is detected.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

---
v2:
  - fixed conditional around abi check custom target
---
 buildtools/meson.build |  4 ++++
 lib/meson.build        | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/buildtools/meson.build b/buildtools/meson.build
index 6ef2c5721..56a1e3dee 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -7,6 +7,10 @@ pmdinfo = find_program('gen-pmdinfo-cfile.sh')
 
 check_experimental_syms = find_program('check-experimental-syms.sh')
 
+if get_option('abi_compat_checks')
+	abidiff = find_program('abidiff')
+endif
+
 # set up map-to-def script using python, either built-in or external
 python3 = import('python').find_installation(required: false)
 if python3.found()
diff --git a/lib/meson.build b/lib/meson.build
index 6ceb5e756..69ea3a2b0 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -180,6 +180,19 @@ foreach l:libraries
 					include_directories: includes,
 					dependencies: shared_deps)
 
+			if not is_windows and get_option('compat_checks')
+				custom_target(dir_name + '.abi_chk',
+					command: [abidiff,
+						meson.source_root() + '/lib/abi/'
+						+ dir_name + '.dump',
+						'@INPUT@'],
+					input: shared_lib,
+					output: dir_name + '.abi_chk',
+					capture: true,
+					install: false,
+					build_by_default: is_experimental == 0)
+			endif
+
 			dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 		endif # sources.length() > 0
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 6/7] build: add drivers abi checks to meson
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
                     ` (4 preceding siblings ...)
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 5/7] build: add lib abi checks to meson Kevin Laatz
@ 2019-11-29 17:10   ` " Kevin Laatz
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 7/7] build: clean up experimental syms check Kevin Laatz
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds the ABI compatibility check for the drivers directory to
the meson build. If enabled, the ABI compatibility checks will run for all
.so's in the lib directory (provided a matching dump file exists). The
build will fail if an ABI incompatibility is detected.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>

---
v2:
  - fixed conditional around abi check custom target
---
 drivers/meson.build | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/meson.build b/drivers/meson.build
index 72eec4608..e19eed419 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -196,6 +196,19 @@ foreach class:dpdk_driver_classes
 					include_directories: includes,
 					dependencies: static_deps)
 
+			if not is_windows and get_option('compat_checks')
+				custom_target('lib' + lib_name + '.abi_chk',
+					command: [abidiff,
+						meson.source_root() + '/drivers/abi/lib'
+						+ lib_name + '.dump',
+						'@INPUT@'],
+					input: shared_lib,
+					output: 'lib' + lib_name + '.abi_chk',
+					capture: true,
+					install: false,
+					build_by_default: is_experimental == 0)
+			endif
+
 			dpdk_drivers += static_lib
 
 			set_variable('shared_@0@'.format(lib_name), shared_dep)
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 7/7] build: clean up experimental syms check
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
                     ` (5 preceding siblings ...)
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 6/7] build: add drivers " Kevin Laatz
@ 2019-11-29 17:10   ` Kevin Laatz
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 17:10 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch cleans up the meson build files in lib and drivers by moving the
custom target for checking the experimental syms next to the abi compat
checks. This also removes the dependency on the check for the shared build,
which was not required by anything, but was only added to force the
experimental syms check run.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 drivers/meson.build | 21 +++++++++++----------
 lib/meson.build     | 21 +++++++++++----------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/meson.build b/drivers/meson.build
index e19eed419..9b0955722 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -163,15 +163,6 @@ foreach class:dpdk_driver_classes
 					'-Wl,/implib:lib\\' + implib]
 			else
 				lk_args = ['-Wl,--version-script=' + version_map]
-				# on unix systems check the output of the
-				# experimental syms script, using it as a
-				# dependency of the .so build
-				lk_deps += custom_target(lib_name + '.exp_chk',
-					command: [check_experimental_syms,
-						version_map, '@INPUT@'],
-					capture: true,
-					input: static_lib,
-					output: lib_name + '.exp_chk')
 			endif
 
 			shared_lib = shared_library(lib_name,
@@ -181,7 +172,6 @@ foreach class:dpdk_driver_classes
 				dependencies: shared_deps,
 				c_args: cflags,
 				link_args: lk_args,
-				link_depends: lk_deps,
 				version: lib_version,
 				soversion: so_version,
 				install: true,
@@ -197,6 +187,17 @@ foreach class:dpdk_driver_classes
 					dependencies: static_deps)
 
 			if not is_windows and get_option('compat_checks')
+				# on unix systems check the output of the
+				# experimental syms script
+				custom_target(lib_name + '.exp_chk',
+					command: [check_experimental_syms,
+						version_map, '@INPUT@'],
+					capture: true,
+					input: static_lib,
+					output: lib_name + '.exp_chk',
+					install: false,
+					build_by_default: true)
+
 				custom_target('lib' + lib_name + '.abi_chk',
 					command: [abidiff,
 						meson.source_root() + '/drivers/abi/lib'
diff --git a/lib/meson.build b/lib/meson.build
index 69ea3a2b0..c448d9dff 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -154,15 +154,6 @@ foreach l:libraries
 					'-Wl,/implib:lib\\' + implib]
 			else
 				lk_args = ['-Wl,--version-script=' + version_map]
-				# on unix systems check the output of the
-				# experimental syms script, using it as a
-				# dependency of the .so build
-				lk_deps += custom_target(name + '.exp_chk',
-					command: [check_experimental_syms,
-						version_map, '@INPUT@'],
-					capture: true,
-					input: static_lib,
-					output: name + '.exp_chk')
 			endif
 
 			shared_lib = shared_library(libname,
@@ -172,7 +163,6 @@ foreach l:libraries
 					dependencies: shared_deps,
 					include_directories: includes,
 					link_args: lk_args,
-					link_depends: lk_deps,
 					version: lib_version,
 					soversion: so_version,
 					install: true)
@@ -181,6 +171,17 @@ foreach l:libraries
 					dependencies: shared_deps)
 
 			if not is_windows and get_option('compat_checks')
+				# on unix systems check the output of the
+				# experimental syms script
+				custom_target(name + '.exp_chk',
+					command: [check_experimental_syms,
+						version_map, '@INPUT@'],
+					capture: true,
+					input: static_lib,
+					output: name + '.exp_chk',
+					install: false,
+					build_by_default: true)
+
 				custom_target(dir_name + '.abi_chk',
 					command: [abidiff,
 						meson.source_root() + '/lib/abi/'
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build
  2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
                     ` (6 preceding siblings ...)
  2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 7/7] build: clean up experimental syms check Kevin Laatz
@ 2019-11-29 21:08   ` Kevin Laatz
  2019-11-29 21:08     ` [dpdk-dev] [PATCH v3 1/7] build: enable debug info by default in meson builds Kevin Laatz
                       ` (7 more replies)
  7 siblings, 8 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:08 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

With the recent changes made to stabilize ABI versioning in DPDK, it will
become increasingly important to check patches for ABI compatibility. We
propose adding the ABI compatibility checking to be done as part of the
build.

The advantages to adding the ABI compatibility checking to the build are
two-fold. Firstly, developers can easily check their patches to make sure
they don’t break the ABI without adding any extra steps. Secondly, it
makes the integration into existing CI seamless since there are no extra
scripts to make the CI run. The build will run as usual and if an
incompatibility is detected in the ABI, the build will fail and show the
incompatibility. As an added bonus, enabling the ABI compatibility checks
does not impact the build speed.

The proposed solution works as follows:
1. Generate the ABI dump of the baseline. This can be done with the new
   script added in this RFC. This step will only need to be done when the
   ABI version changes (so once a year) and can be added to master so it
   exists by default. This step can be skipped if the dump files for the
   baseline already exist.
2. Build with meson. If there is an ABI incompatibility, the build will
   fail and print the incompatibility information.

The patches accompanying this RFC add the ABI dump file generating script,
the meson option required to enable/disable the checks, and the required
meson changes to run the compatibility checks during the build.

---
v2:
  - Rebased on master for 19.11.
  - Moved the experimental syms checks next to the abi checks. This also
    removed the dependency on the experimental checks from the shared
    build.
  - General cleanup.

v3:
  - Fixed typo in meson option name in buildtools/meson.build.

Bruce Richardson (2):
  build: enable debug info by default in meson builds
  build: use meson warning levels

Kevin Laatz (5):
  devtools: add abi dump generation script
  build: add meson option for abi related checks
  build: add lib abi checks to meson
  build: add drivers abi checks to meson
  build: clean up experimental syms check

 buildtools/meson.build   |  4 ++++
 config/meson.build       | 40 +++++++++++++++++++++-------------------
 devtools/gen-abi-dump.sh | 24 ++++++++++++++++++++++++
 drivers/meson.build      | 34 ++++++++++++++++++++++++----------
 lib/meson.build          | 34 ++++++++++++++++++++++++----------
 meson.build              |  9 ++++++++-
 meson_options.txt        |  2 ++
 7 files changed, 107 insertions(+), 40 deletions(-)
 create mode 100755 devtools/gen-abi-dump.sh

-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 1/7] build: enable debug info by default in meson builds
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
@ 2019-11-29 21:08     ` Kevin Laatz
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 2/7] build: use meson warning levels Kevin Laatz
                       ` (6 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:08 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella

From: Bruce Richardson <bruce.richardson@intel.com>

We can turn on debug info by default in meson builds, since it has no
performance penalty. This is done by changing the default build type from
"release" to "debugoptimized". Since the latter using O2, we can using
extra cflags to override that back to O3, which will make little real
difference for actual debugging.

For real debug builds, the user can still do "meson --buildtype=debug ..."
and to remove the debug info "meson --buildtype=release ..." can be used.
These are all standard meson options.

The advantage of having debug builds by default using meson settings is
that we can then add checks for ABI compatibility into each build, and
disable them if we detect that the user has turned off the debug info.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 meson.build | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index b7ae9c8d9..3b7a2e7de 100644
--- a/meson.build
+++ b/meson.build
@@ -7,10 +7,16 @@ project('DPDK', 'C',
 	version: run_command(find_program('cat', 'more'),
 		files('VERSION')).stdout().strip(),
 	license: 'BSD',
-	default_options: ['buildtype=release', 'default_library=static'],
+	default_options: ['buildtype=debugoptimized',
+			'default_library=static'],
 	meson_version: '>= 0.47.1'
 )
 
+# for default "debugoptimized" builds override optimization level 2 with 3
+if get_option('buildtype') == 'debugoptimized'
+	add_project_arguments('-O3', language: 'c')
+endif
+
 # set up some global vars for compiler, platform, configuration, etc.
 cc = meson.get_compiler('c')
 dpdk_conf = configuration_data()
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 2/7] build: use meson warning levels
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
  2019-11-29 21:08     ` [dpdk-dev] [PATCH v3 1/7] build: enable debug info by default in meson builds Kevin Laatz
@ 2019-11-29 21:09     ` Kevin Laatz
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 3/7] devtools: add abi dump generation script Kevin Laatz
                       ` (5 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:09 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella

From: Bruce Richardson <bruce.richardson@intel.com>

Rather than trying to manage all the cflags ourselves, we can use meson
warning levels to give the user more control. We remove the Wextra flag and
rely on meson to add it, by bumping up our default warning level.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 config/meson.build | 40 +++++++++++++++++++++-------------------
 meson.build        |  3 ++-
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 364a8d739..36a594970 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -158,31 +158,33 @@ endif
 # add -include rte_config to cflags
 add_project_arguments('-include', 'rte_config.h', language: 'c')
 
-# enable extra warnings and disable any unwanted warnings
+# enable extra warnings and disable any unwanted warnings. "-Wall" is added
+# by meson at warning level 1, and "-Wextra" at level 2, so we can omit
+# those. Add extra warnings at level 2 or above. (2 is default level).
 warning_flags = [
-	# -Wall is added by meson by default, so add -Wextra only
-	'-Wextra',
-
-	# additional warnings in alphabetical order
-	'-Wcast-qual',
-	'-Wdeprecated',
-	'-Wformat-nonliteral',
-	'-Wformat-security',
-	'-Wmissing-declarations',
-	'-Wmissing-prototypes',
-	'-Wnested-externs',
-	'-Wold-style-definition',
-	'-Wpointer-arith',
-	'-Wsign-compare',
-	'-Wstrict-prototypes',
-	'-Wundef',
-	'-Wwrite-strings',
-
 	# globally disabled warnings
 	'-Wno-address-of-packed-member',
 	'-Wno-packed-not-aligned',
 	'-Wno-missing-field-initializers'
 ]
+if get_option('warning_level').to_int() >= 2
+	warning_flags += [
+		# additional warnings in alphabetical order
+		'-Wcast-qual',
+		'-Wdeprecated',
+		'-Wformat-nonliteral',
+		'-Wformat-security',
+		'-Wmissing-declarations',
+		'-Wmissing-prototypes',
+		'-Wnested-externs',
+		'-Wold-style-definition',
+		'-Wpointer-arith',
+		'-Wsign-compare',
+		'-Wstrict-prototypes',
+		'-Wundef',
+		'-Wwrite-strings',
+	]
+endif
 if not dpdk_conf.get('RTE_ARCH_64')
 # for 32-bit, don't warn about casting a 32-bit pointer to 64-bit int - it's fine!!
 	warning_flags += '-Wno-pointer-to-int-cast'
diff --git a/meson.build b/meson.build
index 3b7a2e7de..7a8f97ad6 100644
--- a/meson.build
+++ b/meson.build
@@ -8,7 +8,8 @@ project('DPDK', 'C',
 		files('VERSION')).stdout().strip(),
 	license: 'BSD',
 	default_options: ['buildtype=debugoptimized',
-			'default_library=static'],
+			'default_library=static',
+			'warning_level=2'],
 	meson_version: '>= 0.47.1'
 )
 
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 3/7] devtools: add abi dump generation script
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
  2019-11-29 21:08     ` [dpdk-dev] [PATCH v3 1/7] build: enable debug info by default in meson builds Kevin Laatz
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 2/7] build: use meson warning levels Kevin Laatz
@ 2019-11-29 21:09     ` Kevin Laatz
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 4/7] build: add meson option for abi related checks Kevin Laatz
                       ` (4 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:09 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds a script to generate ABI dump files. These files will be
required to perform ABI compatibility checks during the build later in the
patchset. This script should be run on a DPDK version with a stable ABI.

Since this is a tool designed for human use, we simplify it to just work
off a whole build directory, taking the parameter of the builddir and
generating the lib|drivers/abi dir. This is hardcoded into the script since
the meson build expects the .dump files in these directories.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 devtools/gen-abi-dump.sh | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100755 devtools/gen-abi-dump.sh

diff --git a/devtools/gen-abi-dump.sh b/devtools/gen-abi-dump.sh
new file mode 100755
index 000000000..ffedef10c
--- /dev/null
+++ b/devtools/gen-abi-dump.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+builddir=$1
+
+if [ -z "$builddir" ] ; then
+	echo "Usage: $(basename $0) build_dir"
+	exit 1
+fi
+
+if [ ! -d "$builddir" ] ; then
+	echo "Error: build directory, '$builddir', doesn't exist"
+	exit 1
+fi
+
+for d in lib drivers ; do
+	mkdir -p $d/abi
+
+	for f in $builddir/$d/*.so* ; do
+		test -L "$f" && continue
+
+		libname=$(basename $f)
+		abidw --out-file $d/abi/${libname%.so*}.dump $f || exit 1
+	done
+done
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 4/7] build: add meson option for abi related checks
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
                       ` (2 preceding siblings ...)
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 3/7] devtools: add abi dump generation script Kevin Laatz
@ 2019-11-29 21:09     ` Kevin Laatz
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 5/7] build: add lib abi checks to meson Kevin Laatz
                       ` (3 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:09 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds a new meson option for running ABI compatibility checks
during the build. If enabled, the lib and drivers .so files will be
compared against any existing ABI dump files in lib|drivers/abi of the
source directory. If there are any incompatibilities, the build will fail
and display the incompatibility.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 meson_options.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson_options.txt b/meson_options.txt
index bc369d06c..5f42def1d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('compat_checks', type: 'boolean', value: true,
+	description: 'enable abi compatibility checks and experimental syms checks to run during the build')
 option('disable_drivers', type: 'string', value: '',
 	description: 'Comma-separated list of drivers to explicitly disable.')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 5/7] build: add lib abi checks to meson
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
                       ` (3 preceding siblings ...)
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 4/7] build: add meson option for abi related checks Kevin Laatz
@ 2019-11-29 21:09     ` Kevin Laatz
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 6/7] build: add drivers " Kevin Laatz
                       ` (2 subsequent siblings)
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:09 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds the ABI compatibility check for the lib directory to the
meson build. If enabled, the ABI compatibility checks will run for all
.so's in the lib directory (provided a matching dump file exists). The
build will fail if an ABI incompatibility is detected.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

---
v2:
  - fixed conditional around abi check custom target
v3:
  - fix typo in meson option name
---
 buildtools/meson.build |  4 ++++
 lib/meson.build        | 13 +++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/buildtools/meson.build b/buildtools/meson.build
index 6ef2c5721..1d6915708 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -7,6 +7,10 @@ pmdinfo = find_program('gen-pmdinfo-cfile.sh')
 
 check_experimental_syms = find_program('check-experimental-syms.sh')
 
+if get_option('compat_checks')
+	abidiff = find_program('abidiff')
+endif
+
 # set up map-to-def script using python, either built-in or external
 python3 = import('python').find_installation(required: false)
 if python3.found()
diff --git a/lib/meson.build b/lib/meson.build
index 6ceb5e756..69ea3a2b0 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -180,6 +180,19 @@ foreach l:libraries
 					include_directories: includes,
 					dependencies: shared_deps)
 
+			if not is_windows and get_option('compat_checks')
+				custom_target(dir_name + '.abi_chk',
+					command: [abidiff,
+						meson.source_root() + '/lib/abi/'
+						+ dir_name + '.dump',
+						'@INPUT@'],
+					input: shared_lib,
+					output: dir_name + '.abi_chk',
+					capture: true,
+					install: false,
+					build_by_default: is_experimental == 0)
+			endif
+
 			dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 		endif # sources.length() > 0
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 6/7] build: add drivers abi checks to meson
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
                       ` (4 preceding siblings ...)
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 5/7] build: add lib abi checks to meson Kevin Laatz
@ 2019-11-29 21:09     ` " Kevin Laatz
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 7/7] build: clean up experimental syms check Kevin Laatz
  2019-12-03 11:03     ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build David Marchand
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:09 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch adds the ABI compatibility check for the drivers directory to
the meson build. If enabled, the ABI compatibility checks will run for all
.so's in the lib directory (provided a matching dump file exists). The
build will fail if an ABI incompatibility is detected.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>

---
v2:
  - fixed conditional around abi check custom target
---
 drivers/meson.build | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/meson.build b/drivers/meson.build
index 72eec4608..e19eed419 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -196,6 +196,19 @@ foreach class:dpdk_driver_classes
 					include_directories: includes,
 					dependencies: static_deps)
 
+			if not is_windows and get_option('compat_checks')
+				custom_target('lib' + lib_name + '.abi_chk',
+					command: [abidiff,
+						meson.source_root() + '/drivers/abi/lib'
+						+ lib_name + '.dump',
+						'@INPUT@'],
+					input: shared_lib,
+					output: 'lib' + lib_name + '.abi_chk',
+					capture: true,
+					install: false,
+					build_by_default: is_experimental == 0)
+			endif
+
 			dpdk_drivers += static_lib
 
 			set_variable('shared_@0@'.format(lib_name), shared_dep)
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 7/7] build: clean up experimental syms check
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
                       ` (5 preceding siblings ...)
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 6/7] build: add drivers " Kevin Laatz
@ 2019-11-29 21:09     ` Kevin Laatz
  2019-12-03 11:03     ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build David Marchand
  7 siblings, 0 replies; 30+ messages in thread
From: Kevin Laatz @ 2019-11-29 21:09 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas, bruce.richardson, ray.kinsella, Kevin Laatz

This patch cleans up the meson build files in lib and drivers by moving the
custom target for checking the experimental syms next to the abi compat
checks. This also removes the dependency on the check for the shared build,
which was not required by anything, but was only added to force the
experimental syms check run.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 drivers/meson.build | 21 +++++++++++----------
 lib/meson.build     | 21 +++++++++++----------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/meson.build b/drivers/meson.build
index e19eed419..9b0955722 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -163,15 +163,6 @@ foreach class:dpdk_driver_classes
 					'-Wl,/implib:lib\\' + implib]
 			else
 				lk_args = ['-Wl,--version-script=' + version_map]
-				# on unix systems check the output of the
-				# experimental syms script, using it as a
-				# dependency of the .so build
-				lk_deps += custom_target(lib_name + '.exp_chk',
-					command: [check_experimental_syms,
-						version_map, '@INPUT@'],
-					capture: true,
-					input: static_lib,
-					output: lib_name + '.exp_chk')
 			endif
 
 			shared_lib = shared_library(lib_name,
@@ -181,7 +172,6 @@ foreach class:dpdk_driver_classes
 				dependencies: shared_deps,
 				c_args: cflags,
 				link_args: lk_args,
-				link_depends: lk_deps,
 				version: lib_version,
 				soversion: so_version,
 				install: true,
@@ -197,6 +187,17 @@ foreach class:dpdk_driver_classes
 					dependencies: static_deps)
 
 			if not is_windows and get_option('compat_checks')
+				# on unix systems check the output of the
+				# experimental syms script
+				custom_target(lib_name + '.exp_chk',
+					command: [check_experimental_syms,
+						version_map, '@INPUT@'],
+					capture: true,
+					input: static_lib,
+					output: lib_name + '.exp_chk',
+					install: false,
+					build_by_default: true)
+
 				custom_target('lib' + lib_name + '.abi_chk',
 					command: [abidiff,
 						meson.source_root() + '/drivers/abi/lib'
diff --git a/lib/meson.build b/lib/meson.build
index 69ea3a2b0..c448d9dff 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -154,15 +154,6 @@ foreach l:libraries
 					'-Wl,/implib:lib\\' + implib]
 			else
 				lk_args = ['-Wl,--version-script=' + version_map]
-				# on unix systems check the output of the
-				# experimental syms script, using it as a
-				# dependency of the .so build
-				lk_deps += custom_target(name + '.exp_chk',
-					command: [check_experimental_syms,
-						version_map, '@INPUT@'],
-					capture: true,
-					input: static_lib,
-					output: name + '.exp_chk')
 			endif
 
 			shared_lib = shared_library(libname,
@@ -172,7 +163,6 @@ foreach l:libraries
 					dependencies: shared_deps,
 					include_directories: includes,
 					link_args: lk_args,
-					link_depends: lk_deps,
 					version: lib_version,
 					soversion: so_version,
 					install: true)
@@ -181,6 +171,17 @@ foreach l:libraries
 					dependencies: shared_deps)
 
 			if not is_windows and get_option('compat_checks')
+				# on unix systems check the output of the
+				# experimental syms script
+				custom_target(name + '.exp_chk',
+					command: [check_experimental_syms,
+						version_map, '@INPUT@'],
+					capture: true,
+					input: static_lib,
+					output: name + '.exp_chk',
+					install: false,
+					build_by_default: true)
+
 				custom_target(dir_name + '.abi_chk',
 					command: [abidiff,
 						meson.source_root() + '/lib/abi/'
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build
  2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
                       ` (6 preceding siblings ...)
  2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 7/7] build: clean up experimental syms check Kevin Laatz
@ 2019-12-03 11:03     ` David Marchand
  2019-12-03 15:27       ` Laatz, Kevin
  7 siblings, 1 reply; 30+ messages in thread
From: David Marchand @ 2019-12-03 11:03 UTC (permalink / raw)
  To: Kevin Laatz; +Cc: dev, Thomas Monjalon, Bruce Richardson, Kinsella, Ray

On Fri, Nov 29, 2019 at 10:09 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
>
> With the recent changes made to stabilize ABI versioning in DPDK, it will
> become increasingly important to check patches for ABI compatibility. We
> propose adding the ABI compatibility checking to be done as part of the
> build.
>
> The advantages to adding the ABI compatibility checking to the build are
> two-fold. Firstly, developers can easily check their patches to make sure
> they don’t break the ABI without adding any extra steps. Secondly, it
> makes the integration into existing CI seamless since there are no extra
> scripts to make the CI run. The build will run as usual and if an
> incompatibility is detected in the ABI, the build will fail and show the
> incompatibility. As an added bonus, enabling the ABI compatibility checks
> does not impact the build speed.
>
> The proposed solution works as follows:
> 1. Generate the ABI dump of the baseline. This can be done with the new
>    script added in this RFC. This step will only need to be done when the
>    ABI version changes (so once a year) and can be added to master so it
>    exists by default. This step can be skipped if the dump files for the
>    baseline already exist.
> 2. Build with meson. If there is an ABI incompatibility, the build will
>    fail and print the incompatibility information.
>
> The patches accompanying this RFC add the ABI dump file generating script,
> the meson option required to enable/disable the checks, and the required
> meson changes to run the compatibility checks during the build.

Global comments:
- why are patch 1 and 2 in this series, is this really needed?
- please squash patches 3, 4, 5 and 6, reading them separately is a
pain and they are quite small,
- if Windows does not support the ABI check, enforce this earlier in
meson and refuse enabling it rather than silently ignoring it,
- I would not enable this check by default
  - this is a developer option, people just building the dpdk don't
care about it,
  - can we have something like a tristate "auto" (default, triggers
the check if abidiff is installed), "disabled" and "enabled" (not
having abidiff installed triggers an error) ?
- please update the travis packages so that we can run those checks
for developers submitting patches
- I don't think you tested this series with
devtools/test-meson-builds.sh, please do. It fails with a custom build
directory and in the dpdk tree too:

Build targets in project: 1019
WARNING: Project specifies a minimum meson_version '>= 0.47.1' but
uses features which were added in newer versions:
 * 0.48.0: {'console arg in custom_target'}
 * 0.50.0: {'install arg in configure_file'}
Found ninja-1.9.0 at /usr/bin/ninja
ninja -C /home/dmarchan/builds/build-gcc-static
ninja: Entering directory `/home/dmarchan/builds/build-gcc-static'
[48/2291] Generating librte_kvargs.abi_chk with a meson_exe.py custom command.
FAILED: lib/librte_kvargs.abi_chk
/usr/bin/meson --internal exe
/home/dmarchan/builds/build-gcc-static/meson-private/meson_exe_abidiff_6511538ddd95d9672028017110fa45c67f01f7be.dat
file /home/dmarchan/dpdk/lib/abi/librte_kvargs.dump does not exist
[77/2291] Compiling C object
'lib/76b5a35@@rte_mbuf@sta/librte_mbuf_rte_mbuf.c.o'.
ninja: build stopped: subcommand failed.


--
David Marchand


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

* Re: [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build
  2019-12-03 11:03     ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build David Marchand
@ 2019-12-03 15:27       ` Laatz, Kevin
  2019-12-04  8:47         ` David Marchand
  0 siblings, 1 reply; 30+ messages in thread
From: Laatz, Kevin @ 2019-12-03 15:27 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Thomas Monjalon, Bruce Richardson, Kinsella, Ray

On 03/12/2019 11:03, David Marchand wrote:
> On Fri, Nov 29, 2019 at 10:09 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
>> With the recent changes made to stabilize ABI versioning in DPDK, it will
>> become increasingly important to check patches for ABI compatibility. We
>> propose adding the ABI compatibility checking to be done as part of the
>> build.
>>
>> The advantages to adding the ABI compatibility checking to the build are
>> two-fold. Firstly, developers can easily check their patches to make sure
>> they don’t break the ABI without adding any extra steps. Secondly, it
>> makes the integration into existing CI seamless since there are no extra
>> scripts to make the CI run. The build will run as usual and if an
>> incompatibility is detected in the ABI, the build will fail and show the
>> incompatibility. As an added bonus, enabling the ABI compatibility checks
>> does not impact the build speed.
>>
>> The proposed solution works as follows:
>> 1. Generate the ABI dump of the baseline. This can be done with the new
>>     script added in this RFC. This step will only need to be done when the
>>     ABI version changes (so once a year) and can be added to master so it
>>     exists by default. This step can be skipped if the dump files for the
>>     baseline already exist.
>> 2. Build with meson. If there is an ABI incompatibility, the build will
>>     fail and print the incompatibility information.
>>
>> The patches accompanying this RFC add the ABI dump file generating script,
>> the meson option required to enable/disable the checks, and the required
>> meson changes to run the compatibility checks during the build.
> Global comments:
> - why are patch 1 and 2 in this series, is this really needed?
Not if we make this an 'auto' option. It would have needed debug info to 
do the ABI check.
> - please squash patches 3, 4, 5 and 6, reading them separately is a
> pain and they are quite small,
Will do
> - if Windows does not support the ABI check, enforce this earlier in
> meson and refuse enabling it rather than silently ignoring it,
Makes sense, will look into this.
> - I would not enable this check by default
>    - this is a developer option, people just building the dpdk don't
> care about it,
>    - can we have something like a tristate "auto" (default, triggers
> the check if abidiff is installed), "disabled" and "enabled" (not
> having abidiff installed triggers an error) ?
Seems reasonable, will change.
> - please update the travis packages so that we can run those checks
> for developers submitting patches
Will do.
> - I don't think you tested this series with
> devtools/test-meson-builds.sh, please do. It fails with a custom build
> directory and in the dpdk tree too:
>
> Build targets in project: 1019
> WARNING: Project specifies a minimum meson_version '>= 0.47.1' but
> uses features which were added in newer versions:
>   * 0.48.0: {'console arg in custom_target'}
>   * 0.50.0: {'install arg in configure_file'}
> Found ninja-1.9.0 at /usr/bin/ninja
> ninja -C /home/dmarchan/builds/build-gcc-static
> ninja: Entering directory `/home/dmarchan/builds/build-gcc-static'
> [48/2291] Generating librte_kvargs.abi_chk with a meson_exe.py custom command.
> FAILED: lib/librte_kvargs.abi_chk
> /usr/bin/meson --internal exe
> /home/dmarchan/builds/build-gcc-static/meson-private/meson_exe_abidiff_6511538ddd95d9672028017110fa45c67f01f7be.dat
> file /home/dmarchan/dpdk/lib/abi/librte_kvargs.dump does not exist
> [77/2291] Compiling C object
> 'lib/76b5a35@@rte_mbuf@sta/librte_mbuf_rte_mbuf.c.o'.
> ninja: build stopped: subcommand failed.

This is failing as the .dump files have not been created yet. They can 
be generated with devtools/gen-abi-dump.sh <build_dir>. This will 
generate a .dump file for each shared object in the builddir drivers and 
lib folders.

/Kevin



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

* Re: [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build
  2019-12-03 15:27       ` Laatz, Kevin
@ 2019-12-04  8:47         ` David Marchand
  2019-12-04 10:46           ` Bruce Richardson
  2019-12-04 11:56           ` Neil Horman
  0 siblings, 2 replies; 30+ messages in thread
From: David Marchand @ 2019-12-04  8:47 UTC (permalink / raw)
  To: Laatz, Kevin
  Cc: dev, Thomas Monjalon, Bruce Richardson, Kinsella, Ray, Neil Horman

On Tue, Dec 3, 2019 at 4:27 PM Laatz, Kevin <kevin.laatz@intel.com> wrote:
> > Build targets in project: 1019
> > WARNING: Project specifies a minimum meson_version '>= 0.47.1' but
> > uses features which were added in newer versions:
> >   * 0.48.0: {'console arg in custom_target'}
> >   * 0.50.0: {'install arg in configure_file'}
> > Found ninja-1.9.0 at /usr/bin/ninja
> > ninja -C /home/dmarchan/builds/build-gcc-static
> > ninja: Entering directory `/home/dmarchan/builds/build-gcc-static'
> > [48/2291] Generating librte_kvargs.abi_chk with a meson_exe.py custom command.
> > FAILED: lib/librte_kvargs.abi_chk
> > /usr/bin/meson --internal exe
> > /home/dmarchan/builds/build-gcc-static/meson-private/meson_exe_abidiff_6511538ddd95d9672028017110fa45c67f01f7be.dat
> > file /home/dmarchan/dpdk/lib/abi/librte_kvargs.dump does not exist
> > [77/2291] Compiling C object
> > 'lib/76b5a35@@rte_mbuf@sta/librte_mbuf_rte_mbuf.c.o'.
> > ninja: build stopped: subcommand failed.
>
> This is failing as the .dump files have not been created yet. They can
> be generated with devtools/gen-abi-dump.sh <build_dir>. This will
> generate a .dump file for each shared object in the builddir drivers and
> lib folders.

Throwing an idea, I did not investigate.

Could we refactor our tools/checks on the .map files to use the dump files?
We would then only maintain one file about symbols versioning.


-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build
  2019-12-04  8:47         ` David Marchand
@ 2019-12-04 10:46           ` Bruce Richardson
  2019-12-04 11:56           ` Neil Horman
  1 sibling, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2019-12-04 10:46 UTC (permalink / raw)
  To: David Marchand
  Cc: Laatz, Kevin, dev, Thomas Monjalon, Kinsella, Ray, Neil Horman

On Wed, Dec 04, 2019 at 09:47:31AM +0100, David Marchand wrote:
> On Tue, Dec 3, 2019 at 4:27 PM Laatz, Kevin <kevin.laatz@intel.com> wrote:
> > > Build targets in project: 1019
> > > WARNING: Project specifies a minimum meson_version '>= 0.47.1' but
> > > uses features which were added in newer versions:
> > >   * 0.48.0: {'console arg in custom_target'}
> > >   * 0.50.0: {'install arg in configure_file'}
> > > Found ninja-1.9.0 at /usr/bin/ninja
> > > ninja -C /home/dmarchan/builds/build-gcc-static
> > > ninja: Entering directory `/home/dmarchan/builds/build-gcc-static'
> > > [48/2291] Generating librte_kvargs.abi_chk with a meson_exe.py custom command.
> > > FAILED: lib/librte_kvargs.abi_chk
> > > /usr/bin/meson --internal exe
> > > /home/dmarchan/builds/build-gcc-static/meson-private/meson_exe_abidiff_6511538ddd95d9672028017110fa45c67f01f7be.dat
> > > file /home/dmarchan/dpdk/lib/abi/librte_kvargs.dump does not exist
> > > [77/2291] Compiling C object
> > > 'lib/76b5a35@@rte_mbuf@sta/librte_mbuf_rte_mbuf.c.o'.
> > > ninja: build stopped: subcommand failed.
> >
> > This is failing as the .dump files have not been created yet. They can
> > be generated with devtools/gen-abi-dump.sh <build_dir>. This will
> > generate a .dump file for each shared object in the builddir drivers and
> > lib folders.
> 
> Throwing an idea, I did not investigate.
> 
> Could we refactor our tools/checks on the .map files to use the dump files?
> We would then only maintain one file about symbols versioning.
> 
Could be looked into, but I'd worry about the complexity of the dump files
holding the whole API compared to the simpler map files which just have
a simple function listing.

/Bruce

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

* Re: [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build
  2019-12-04  8:47         ` David Marchand
  2019-12-04 10:46           ` Bruce Richardson
@ 2019-12-04 11:56           ` Neil Horman
  2019-12-04 12:00             ` David Marchand
  1 sibling, 1 reply; 30+ messages in thread
From: Neil Horman @ 2019-12-04 11:56 UTC (permalink / raw)
  To: David Marchand
  Cc: Laatz, Kevin, dev, Thomas Monjalon, Bruce Richardson, Kinsella, Ray

On Wed, Dec 04, 2019 at 09:47:31AM +0100, David Marchand wrote:
> On Tue, Dec 3, 2019 at 4:27 PM Laatz, Kevin <kevin.laatz@intel.com> wrote:
> > > Build targets in project: 1019
> > > WARNING: Project specifies a minimum meson_version '>= 0.47.1' but
> > > uses features which were added in newer versions:
> > >   * 0.48.0: {'console arg in custom_target'}
> > >   * 0.50.0: {'install arg in configure_file'}
> > > Found ninja-1.9.0 at /usr/bin/ninja
> > > ninja -C /home/dmarchan/builds/build-gcc-static
> > > ninja: Entering directory `/home/dmarchan/builds/build-gcc-static'
> > > [48/2291] Generating librte_kvargs.abi_chk with a meson_exe.py custom command.
> > > FAILED: lib/librte_kvargs.abi_chk
> > > /usr/bin/meson --internal exe
> > > /home/dmarchan/builds/build-gcc-static/meson-private/meson_exe_abidiff_6511538ddd95d9672028017110fa45c67f01f7be.dat
> > > file /home/dmarchan/dpdk/lib/abi/librte_kvargs.dump does not exist
> > > [77/2291] Compiling C object
> > > 'lib/76b5a35@@rte_mbuf@sta/librte_mbuf_rte_mbuf.c.o'.
> > > ninja: build stopped: subcommand failed.
> >
> > This is failing as the .dump files have not been created yet. They can
> > be generated with devtools/gen-abi-dump.sh <build_dir>. This will
> > generate a .dump file for each shared object in the builddir drivers and
> > lib folders.
> 
> Throwing an idea, I did not investigate.
> 
> Could we refactor our tools/checks on the .map files to use the dump files?
> We would then only maintain one file about symbols versioning.
> 
Thats a chicken and egg problem.  The map file is the canonical source for
versioning information. Any information that is encoded in the output of objdump
regarding versioning is sourced from the map files, so while you could use the
objdump output to check versioning, you can't remove the need for map files to
encode it.

If you want to remove the map files, we would need to look at the creation of
macros to encode versioning information for each symbol you want to export, and
use that to dynamically build a version map file (simmilar to what the linux
kernel EXPORT_SYMBOL does).  But that just moves version tracking around, it
doesn't really eliminate it.

Neil

> 
> -- 
> David Marchand
> 
> 

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

* Re: [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build
  2019-12-04 11:56           ` Neil Horman
@ 2019-12-04 12:00             ` David Marchand
  0 siblings, 0 replies; 30+ messages in thread
From: David Marchand @ 2019-12-04 12:00 UTC (permalink / raw)
  To: Neil Horman
  Cc: Laatz, Kevin, dev, Thomas Monjalon, Bruce Richardson, Kinsella, Ray

On Wed, Dec 4, 2019 at 12:56 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Wed, Dec 04, 2019 at 09:47:31AM +0100, David Marchand wrote:
> > On Tue, Dec 3, 2019 at 4:27 PM Laatz, Kevin <kevin.laatz@intel.com> wrote:
> > > > Build targets in project: 1019
> > > > WARNING: Project specifies a minimum meson_version '>= 0.47.1' but
> > > > uses features which were added in newer versions:
> > > >   * 0.48.0: {'console arg in custom_target'}
> > > >   * 0.50.0: {'install arg in configure_file'}
> > > > Found ninja-1.9.0 at /usr/bin/ninja
> > > > ninja -C /home/dmarchan/builds/build-gcc-static
> > > > ninja: Entering directory `/home/dmarchan/builds/build-gcc-static'
> > > > [48/2291] Generating librte_kvargs.abi_chk with a meson_exe.py custom command.
> > > > FAILED: lib/librte_kvargs.abi_chk
> > > > /usr/bin/meson --internal exe
> > > > /home/dmarchan/builds/build-gcc-static/meson-private/meson_exe_abidiff_6511538ddd95d9672028017110fa45c67f01f7be.dat
> > > > file /home/dmarchan/dpdk/lib/abi/librte_kvargs.dump does not exist
> > > > [77/2291] Compiling C object
> > > > 'lib/76b5a35@@rte_mbuf@sta/librte_mbuf_rte_mbuf.c.o'.
> > > > ninja: build stopped: subcommand failed.
> > >
> > > This is failing as the .dump files have not been created yet. They can
> > > be generated with devtools/gen-abi-dump.sh <build_dir>. This will
> > > generate a .dump file for each shared object in the builddir drivers and
> > > lib folders.
> >
> > Throwing an idea, I did not investigate.
> >
> > Could we refactor our tools/checks on the .map files to use the dump files?
> > We would then only maintain one file about symbols versioning.
> >
> Thats a chicken and egg problem.  The map file is the canonical source for
> versioning information. Any information that is encoded in the output of objdump
> regarding versioning is sourced from the map files, so while you could use the
> objdump output to check versioning, you can't remove the need for map files to
> encode it.

Ah ah.. yes.
^U on this idea.


-- 
David Marchand


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

end of thread, back to index

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23  1:07 [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build Kevin Laatz
2019-10-23  1:07 ` [dpdk-dev] [RFC 1/6] build: enable debug info by default in meson builds Kevin Laatz
2019-10-23  1:07 ` [dpdk-dev] [RFC 2/6] build: use meson warning levels Kevin Laatz
2019-10-23  1:07 ` [dpdk-dev] [RFC 3/6] devtools: add abi dump generation script Kevin Laatz
2019-10-23  1:07 ` [dpdk-dev] [RFC 4/6] build: add meson option for abi related checks Kevin Laatz
2019-10-23  1:07 ` [dpdk-dev] [RFC 5/6] build: add lib abi checks to meson Kevin Laatz
2019-10-23  1:07 ` [dpdk-dev] [RFC 6/6] build: add drivers " Kevin Laatz
2019-11-29 12:13 ` [dpdk-dev] [RFC 0/6] Add ABI compatibility checks to the meson build David Marchand
2019-11-29 17:10 ` [dpdk-dev] [PATCH v2 0/7] " Kevin Laatz
2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 1/7] build: enable debug info by default in meson builds Kevin Laatz
2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 2/7] build: use meson warning levels Kevin Laatz
2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 3/7] devtools: add abi dump generation script Kevin Laatz
2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 4/7] build: add meson option for abi related checks Kevin Laatz
2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 5/7] build: add lib abi checks to meson Kevin Laatz
2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 6/7] build: add drivers " Kevin Laatz
2019-11-29 17:10   ` [dpdk-dev] [PATCH v2 7/7] build: clean up experimental syms check Kevin Laatz
2019-11-29 21:08   ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build Kevin Laatz
2019-11-29 21:08     ` [dpdk-dev] [PATCH v3 1/7] build: enable debug info by default in meson builds Kevin Laatz
2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 2/7] build: use meson warning levels Kevin Laatz
2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 3/7] devtools: add abi dump generation script Kevin Laatz
2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 4/7] build: add meson option for abi related checks Kevin Laatz
2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 5/7] build: add lib abi checks to meson Kevin Laatz
2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 6/7] build: add drivers " Kevin Laatz
2019-11-29 21:09     ` [dpdk-dev] [PATCH v3 7/7] build: clean up experimental syms check Kevin Laatz
2019-12-03 11:03     ` [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build David Marchand
2019-12-03 15:27       ` Laatz, Kevin
2019-12-04  8:47         ` David Marchand
2019-12-04 10:46           ` Bruce Richardson
2019-12-04 11:56           ` Neil Horman
2019-12-04 12:00             ` David Marchand

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git