On 03/12/2019 11:03, David Marchand wrote:
On Fri, Nov 29, 2019 at 10:09 PM Kevin Laatz <kevin.la...@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
