Message ID | 20200911160332.256343-1-conor.walsh@intel.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BE209A04C0; Fri, 11 Sep 2020 18:04:12 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 443C01C10B; Fri, 11 Sep 2020 18:03:56 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id EAC151B9B7 for <dev@dpdk.org>; Fri, 11 Sep 2020 18:03:49 +0200 (CEST) IronPort-SDR: l2uZTQ2s33Ra+1ROvq43UT2HyUtcQCo7g4h3TUog/Zv7CJK/WI5XWZAkb0P/rozmwlhSNC/x28 EERFsyew07UQ== X-IronPort-AV: E=McAfee;i="6000,8403,9741"; a="220343445" X-IronPort-AV: E=Sophos;i="5.76,416,1592895600"; d="scan'208";a="220343445" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Sep 2020 09:03:43 -0700 IronPort-SDR: 78UPN6aEo4GevCvzK/GPa+srRl9niCTN6UybQaS88SK2lxRmcDaxjZF9jRPy6Ud+0/1amAhkzB FUPyBbfulboA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.76,416,1592895600"; d="scan'208";a="318321516" Received: from silpixa00400466.ir.intel.com ([10.237.213.195]) by orsmga002.jf.intel.com with ESMTP; 11 Sep 2020 09:03:40 -0700 From: Conor Walsh <conor.walsh@intel.com> To: dev@dpdk.org Cc: david.marchand@redhat.com, ray.kinsella@intel.com, nhorman@tuxdriver.com, aconole@redhat.com, maicolgabriel@hotmail.com, thomas@monjalon.net, bruce.richardson@intel.com, Conor Walsh <conor.walsh@intel.com> Date: Fri, 11 Sep 2020 16:03:28 +0000 Message-Id: <20200911160332.256343-1-conor.walsh@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200910142121.3995680-1-conor.walsh@intel.com> References: <20200910142121.3995680-1-conor.walsh@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v3 0/4] abi breakage checks for meson X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
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
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. "
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.
(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.