Re: [dpdk-dev] [PATCH v3 0/7] Add ABI compatibility checks to the meson build

["Laatz, Kevin" <kevin.laatz@intel.com>]

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