From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1C7B2A0C41; Wed, 17 Nov 2021 11:47:23 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D65EA4068C; Wed, 17 Nov 2021 11:47:22 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id E293B40040 for ; Wed, 17 Nov 2021 11:47:20 +0100 (CET) X-IronPort-AV: E=McAfee;i="6200,9189,10170"; a="231398120" X-IronPort-AV: E=Sophos;i="5.87,241,1631602800"; d="scan'208";a="231398120" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Nov 2021 02:47:20 -0800 X-IronPort-AV: E=Sophos;i="5.87,241,1631602800"; d="scan'208";a="604678664" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.19.204]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 17 Nov 2021 02:47:17 -0800 Date: Wed, 17 Nov 2021 10:47:14 +0000 From: Bruce Richardson To: Thomas Monjalon Cc: David Marchand , dev@dpdk.org, bluca@debian.org, tredaelli@redhat.com, i.maximets@ovn.org, james.r.harris@intel.com, mohammed@hawari.fr Subject: Re: [PATCH 5/5] build: select optional libraries Message-ID: References: <20211110164814.5231-1-david.marchand@redhat.com> <20211110164814.5231-6-david.marchand@redhat.com> <4536860.m2YTRCI3sh@thomas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4536860.m2YTRCI3sh@thomas> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Nov 16, 2021 at 06:25:28PM +0100, Thomas Monjalon wrote: > 10/11/2021 18:34, Bruce Richardson: > > On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote: > > > There is currently no way to know which libraries are optional. > > > Introduce a enable_libs option (close to what we have for drivers) so > > > that packagers or projects consuming DPDK can more easily select the > > > optional libraries that matter to them and disable other optional > > > libraries. > > > > > > Note: the enabled_libs variable is renamed for sake of consistency. > > > > > > Signed-off-by: David Marchand > > > --- > > This is the only patch of this set I would have some concerns about. I'm > > just not sure that it makes sense to have this option for libraries > > compared to drivers. > > > > Specifically: > > * We have over 200 drivers in DPDK (rough count using find), of which 2 are > > mandatory, and therefore specifying just 1 or 2 that you want can make > > sense. > > * On the other hand, we have 53 libraries, of which only 7 or so (after > > this patchset) are optional. This means that use of the term > > "enable_libs" is misleading - at least to me - in that it's only a very > > small proportion of the libs which would be affected by that flag > > (compared to 99% of the drivers) > > The options are described like this: > > option('disable_libs', type: 'string', value: '', description: > 'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]') > +option('enable_libs', type: 'string', value: '', description: > + 'Comma-separated list of libraries to explicitly enable.') > > I feel we should mention it is enabling optional libraries, > and the default is to enable all. > > > * Also, while the number of mandatory drivers is unlikely to change much > > (since there are only 2), it should be fairly safe to do builds using > > "--enable-drivers". On the other hand, the list of libraries affected by > > "--enable-libs" is likely to change, so short of each user naming each > > and every lib they use (and each library those depend on), to the list, > > it's quite possible that any --enable-libs use could lead to a broken > > build in future if a library changes from mandatory to optional. > > In order to be safe, the user can list all required libs, > including non-optional ones. > Yes, they can, but that is the part I'm concerned about. It should not be expected for users to know what all libraries should be needed and dependencies between them. The list of mandatory libraries is quite long. > > Overall, I'm just concerned that this flag is premature, and would prefer > > to keep it just to the disable option until we are confident that out > > "optional library" list is relatively settled. > > I see it the opposite way: > Someone who does not wish to deliver extra libs could use this option > to list the required libs, so not-required libs will disappear from > the build once they are declared optional in future releases. > Yes, though as-above, I'm concerned about the difficulty of building up such a list. However, if this option is only for "expert" and distro-packaging use, then I suppose it's reasonable. Perhaps we should add a warning to the doc line too, noting that it's only recommended for advanced users. /Bruce