mbox series

[v3,0/4] abi breakage checks for meson

Message ID 20200911160332.256343-1-conor.walsh@intel.com (mailing list archive)
Headers
Series abi breakage checks for meson |

Message

Conor Walsh Sept. 11, 2020, 4:03 p.m. UTC
  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
  

Comments

Thomas Monjalon Sept. 14, 2020, 8:08 a.m. UTC | #1
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.
"
  
Ray Kinsella Sept. 14, 2020, 8:30 a.m. UTC | #2
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.
  
Ray Kinsella Sept. 14, 2020, 9:34 a.m. UTC | #3
(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.