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.
