From: Bruce Richardson <bruce.richardson@intel.com> To: David Marchand <david.marchand@redhat.com> Cc: Kevin Laatz <kevin.laatz@intel.com>, dev <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>, "Kinsella, Ray" <ray.kinsella@intel.com>, Tomasz Duszynski <tdu@semihalf.com>, Zyta Szpak <zr@semihalf.com>, Rastislav Cernay <cernay@netcope.com>, Aaron Conole <aconole@redhat.com> Subject: Re: [dpdk-dev] [PATCH v6 00/11] Add ABI compatibility checks to the meson build Date: Fri, 20 Dec 2019 11:04:14 +0000 Message-ID: <20191220110414.GA514@bricha3-MOBL.ger.corp.intel.com> (raw) In-Reply-To: <CAJFAV8xJaQoTYh9Q2rWkEcRU1fS0bLW=m-4SMmYptK0RDk_g8A@mail.gmail.com> On Thu, Dec 19, 2019 at 10:58:35PM +0100, David Marchand wrote: > Hello Kevin, > > On Fri, Dec 13, 2019 at 5:41 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 set. 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 in this set include the ABI dump file generating script, the > > dump files for drivers and libs, the meson option required to > > enable/disable the checks, and the required meson changes to run the > > compatibility checks during the build. > > > > Note: This patch set depends on: http://patches.dpdk.org/patch/63765/. The > > generated .dump files in this patch set are based on the changes in the > > patch "build: fix soname info for 19.11 compatibility". If a decision is > > made to use a different format for the sonames, then a new version of this > > patch set will be required as the .dump files will need to be regenerated. > > > > Note: The following driver dump files are not included in these patches: > > common/mvep: missing dependency, "libmusdk" > > net/mvneta: missing dependency, "libmusdk" > > net/mvpp2: missing dependency, "libmusdk" > > net/nfb: missing dependency, "libnfb" > > crypto/mvsam: missing dependency, "libmusdk" > > > > They have not been included as I do not have access to these dependencies. > > Please feel free to add them if you can! (Maintainers of the above Cc'ed). > > - I asked for the dump files, but I can see that it is impractical. > > The dump files are huge. I did not expect that :-). Yes, they are big alright, but on the other hand, they also don't change very much (we hope!) > The dump files are architecture specific and maintaining multi arch > dumps would be even bigger than just what you sent for x86_64. > (not even considering the changes in ABI if some configuration items > have an impact...). Good point, we missed that when looking at this. > > As you pointed out, people who don't have all dependencies won't > create/update those dump files. Well, the creation should be a once-off, the comparison is what is done regularly and needs the build tools. > Dealing with ABI updates (thinking of bumping the ABI version) is > likely a maintainer job, but it will be a source of issues and we > (maintainers) might miss some updates especially for drivers we can't > build. > > > - Why do we restrict the checks to meson? > The make build framework is going to disappear ok, but we can't leave > it untested. > People still rely on it. > Because as you point out below, checking the ABI is technically orthogonal to building the DPDK, so we didn't see the payoff in adding support to two build systems as being worth the additional effort. > Checking the ABI is orthogonal to building DPDK. > Dumping the ABI and checking it against objects can be done externally. > True, but the advantage of doing so as part of each and every build is that any ABI break is caught by the original developer before he even submits his patch to the CI. As with so many things, the earlier in the process that something can be run the better it is. > > - For those reasons, I have been trying an alternate approach [1]: in > Travis, generate a reference dump based on the first ancestor tag, > then build the proposed patches. > All contributions get picked up by Aaron robot and would have to pass > through this check. Yes, the alternative to having the checks done at build time is to have them done as part of the CI, though I'd personally perfer the former. > > As an exercise, I tried to integrate Eelco patch [2], that moves > symbols from EXPERIMENTAL to a stable ABI. The check passes fine. > I also tried to bump the ABI major version. The check fails, as > expected, but I prepared a way to bypass such failures for the > releases where we will explicitely break ABI. > IMHO: we should not bypass such failures, but instead update our reference ABI dumps. This is one reason why having the ABI spec in the git repo would work well, any patches that change ABI would also include the updates to the dump files. > For maintainers that integrate patches or developers that get a CI > failure and want to fix it, we would need to help them to: > * generate dumps on a reference version, so I would tend to write some > documentation since playing with the current sources would be too > dangerous from my pov, This should be a one-off reference dump archived somewhere. Each maintainer should not have his own copy, I think. > * run the checks, like adding the check in the > devtools/test-*-build.sh scripts that already exist, with a new > configuration item to point at the dumps per target, > Where do we store the dumps then? Do they get stored in a separate git repo? > Those last two points are still to be done. > > WDYT? > Makes sense. I still prefer a solution where we keep the offical ABI in git alongside the source code, but I realise that doing so for multiple archtectures is likely to get to be very big. However, since these are plain text files though, they should compress well when stored in the git repo itself or when being pushed/pulled. [And again, the deltas for these should be pretty tiny, even when we do update the ABI]. /Bruce
next prev parent reply index Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-23 1:07 [dpdk-dev] [RFC 0/6] " 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 2019-12-10 11:07 ` David Marchand 2019-12-10 11:36 ` Laatz, Kevin 2019-12-11 18:21 ` [dpdk-dev] [PATCH v4 0/3] " Kevin Laatz 2019-12-11 18:21 ` [dpdk-dev] [PATCH v4 2/3] build: add abi checks to meson Kevin Laatz 2019-12-11 18:21 ` [dpdk-dev] [PATCH v4 3/3] build: clean up experimental syms check Kevin Laatz [not found] ` <20191211182147.19355-2-kevin.laatz@intel.com> 2019-12-12 8:43 ` [dpdk-dev] [PATCH v4 1/3] build: add dump files for v20.0 ABI David Marchand 2019-12-12 9:36 ` David Marchand 2019-12-12 9:45 ` Laatz, Kevin 2019-12-12 9:45 ` Laatz, Kevin 2019-12-13 14:02 ` [dpdk-dev] [PATCH v5 0/3] Add ABI compatibility checks to the meson build Kevin Laatz 2019-12-13 14:03 ` [dpdk-dev] [PATCH v5 2/3] build: add abi checks to meson Kevin Laatz 2019-12-13 14:03 ` [dpdk-dev] [PATCH v5 3/3] build: clean up experimental syms check Kevin Laatz 2019-12-13 16:40 ` [dpdk-dev] [PATCH v6 00/11] Add ABI compatibility checks to the meson build Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 01/11] lib: add dump files for v20.0 ABI Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 02/11] drivers/bus: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 03/11] drivers/mempool: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 04/11] drivers/common: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 05/11] drivers/raw: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 06/11] drivers/crypto: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 07/11] drivers/compress: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 08/11] drivers/net: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 09/11] drivers/net/intel: " Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 10/11] build: add abi checks to meson Kevin Laatz 2019-12-13 16:41 ` [dpdk-dev] [PATCH v6 11/11] build: clean up experimental syms check Kevin Laatz 2019-12-19 21:58 ` [dpdk-dev] [PATCH v6 00/11] Add ABI compatibility checks to the meson build David Marchand 2019-12-20 10:20 ` Thomas Monjalon 2019-12-20 11:04 ` Bruce Richardson [this message] 2019-12-20 13:19 ` David Marchand 2019-12-20 14:17 ` Bruce Richardson 2020-01-06 13:20 ` Aaron Conole
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191220110414.GA514@bricha3-MOBL.ger.corp.intel.com \ --to=bruce.richardson@intel.com \ --cc=aconole@redhat.com \ --cc=cernay@netcope.com \ --cc=david.marchand@redhat.com \ --cc=dev@dpdk.org \ --cc=kevin.laatz@intel.com \ --cc=ray.kinsella@intel.com \ --cc=tdu@semihalf.com \ --cc=thomas@monjalon.net \ --cc=zr@semihalf.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
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