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 1C98B42CB2; Mon, 19 Jun 2023 10:00:21 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AD95F40DF8; Mon, 19 Jun 2023 10:00:20 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id E4F3A40A81 for ; Mon, 19 Jun 2023 10:00:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1687161618; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=eNDDKs/1HASQfoGrsfc0chUPkCDY5lxWNW2dMIwcAfs=; b=eUc6wz00SmPhYzirqS+8kk8Z9vK4LKuB5Xi0gYZO2z12bGv9VYIpV+S/fh/o3sttAxAILn xdPbUZGloh6ka7PTVLthXoCjS+yHylZjaqCfpGua+KAiuZQrTrf4aj5vo23KzdZ6W848J4 n5oiysUbuACPeYwGbILqBJPh7rizEsc= Received: from mail-pj1-f72.google.com (mail-pj1-f72.google.com [209.85.216.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-617-FDtYVAcXOm62ajdLZSQ7bw-1; Mon, 19 Jun 2023 04:00:15 -0400 X-MC-Unique: FDtYVAcXOm62ajdLZSQ7bw-1 Received: by mail-pj1-f72.google.com with SMTP id 98e67ed59e1d1-25eb3db302eso2569292a91.3 for ; Mon, 19 Jun 2023 01:00:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687161614; x=1689753614; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=eNDDKs/1HASQfoGrsfc0chUPkCDY5lxWNW2dMIwcAfs=; b=L/FFodGfXCJ38nboi1L7v9V4+pazR7JkF5CYrRkwnXWV9KNPYV0tg2CDsBUwZpkcLR PMPJYzq/EA+kEE+ydDgM0WuGcpsTZ+juCbhrfGnu0kOXKKeRRfxPhtDJoKTdKxB924UB WDEhrxuTsV9Y7eFHb8Bf+UNPAs50X9N5U6Ppx+eU8Z8JDYaGEf8hJAAvOUAY6n1Z7Hr0 Gno/seLhNLNYkMGVKKivWS4CnxD2P2BCRobNmneAhd9w7lDco8S9QYqMluSnAFeiR/9T Pfn+/gmWns1RgzZ9xq8kPuPs2HXoiDEU8z0f2KDlB3jXtQ2zeEnK4Gwg0k8yjfP4twvA BrAg== X-Gm-Message-State: AC+VfDx+m+Eb8HdrT5eFQ7APGZ7trpiftIWbLrXz1bPvHxcKKIhhlqUf c+ouFcPSm2fvKbM5RtjJJWbt0tE4vLW3qjiP31THko1QARGvO1IkjIJCs07FpxkHgdFrDgk6+Bw +RHg7KltorRCIHWhefwY= X-Received: by 2002:a17:90a:9ae:b0:25e:a643:adeb with SMTP id 43-20020a17090a09ae00b0025ea643adebmr8660202pjo.39.1687161613802; Mon, 19 Jun 2023 01:00:13 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7t9f1q0UtkrTZvBarjg5qQQRfEjK1sr4OegeOVeYkqVC7smtUoAX8h8ZSOaqEYLNe5v9B73n+daHMlasXwViU= X-Received: by 2002:a17:90a:9ae:b0:25e:a643:adeb with SMTP id 43-20020a17090a09ae00b0025ea643adebmr8660181pjo.39.1687161613439; Mon, 19 Jun 2023 01:00:13 -0700 (PDT) MIME-Version: 1.0 References: <20211117112847.7362-6-david.marchand@redhat.com> <20230616071450.3542479-1-david.marchand@redhat.com> In-Reply-To: From: David Marchand Date: Mon, 19 Jun 2023 10:00:02 +0200 Message-ID: Subject: Re: [PATCH v3] build: select optional libraries To: Bruce Richardson Cc: dev@dpdk.org, thomas@monjalon.net, bluca@debian.org, tredaelli@redhat.com, i.maximets@ovn.org, james.r.harris@intel.com, mohammed@hawari.fr, Aaron Conole , Michael Santana X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Fri, Jun 16, 2023 at 11:42=E2=80=AFAM Bruce Richardson wrote: > On Fri, Jun 16, 2023 at 09:14:50AM +0200, 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. > > > > Deprecated libraries are handled via some logic in lib/meson.build > > rather than a default value in meson_options.txt. > > Enabling deprecated libraries is achieved by requesting all > > libraries to be built in the CI. > > > > kni dependency to IOVA configuration is moved to its own meson.build. > > > > Note: the enabled_libs variable is renamed for sake of consistency wrt = to > > drivers and applications similar variables. > > > > Signed-off-by: David Marchand > > --- > > Thanks David. > Some comments inline below just on the usability aspects of this. > > /Bruce > > > The topic was raised again while Stephen was reviewing stale patches in > > patchwork. The idea of this patch is the same as before. > > I simply rebased the change (and dealt with the deprecated libraries th= at > > added some little complication). > > > > Changes since v2: > > - moved the IOVA check for kni build in lib/kni/meson.build, > > - reworked deprecated libraries handling by only considering them when > > no enable_libs option is set. With this, no need for a default value > > in meson_options.txt, yet configurations in the CI must be adjusted, > > - moved mandatory libraries check after enable/disable_libs evaluation, > > > > --- > > .ci/linux-build.sh | 2 +- > > app/test/meson.build | 2 +- > > buildtools/chkincs/meson.build | 2 +- > > devtools/test-meson-builds.sh | 4 ++-- > > lib/kni/meson.build | 5 ++++ > > lib/meson.build | 44 ++++++++++++++++++++-------------- > > meson.build | 3 ++- > > meson_options.txt | 4 +++- > > 8 files changed, 41 insertions(+), 25 deletions(-) > > > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh > > index 9631e342b5..a4e474a900 100755 > > --- a/.ci/linux-build.sh > > +++ b/.ci/linux-build.sh > > @@ -97,7 +97,7 @@ if [ "$MINI" =3D "true" ]; then > > OPTS=3D"$OPTS -Denable_drivers=3Dnet/null" > > OPTS=3D"$OPTS -Ddisable_libs=3D*" > > else > > - OPTS=3D"$OPTS -Ddisable_libs=3D" > > + OPTS=3D"$OPTS -Denable_libs=3D*" > > fi > > > > if [ "$ASAN" =3D "true" ]; then > > diff --git a/app/test/meson.build b/app/test/meson.build > > index d0fabcbb8b..f3217ae577 100644 > > --- a/app/test/meson.build > > +++ b/app/test/meson.build > > @@ -152,7 +152,7 @@ test_sources =3D files( > > 'virtual_pmd.c', > > ) > > > > -test_deps =3D enabled_libs > > +test_deps =3D dpdk_libs_enabled > > # as well as libs, the pci and vdev bus drivers are needed for a lot o= f tests > > test_deps +=3D ['bus_pci', 'bus_vdev'] > > > > diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.= build > > index 378c2f19ef..f2dadcae18 100644 > > --- a/buildtools/chkincs/meson.build > > +++ b/buildtools/chkincs/meson.build > > @@ -22,7 +22,7 @@ sources +=3D gen_c_files.process(dpdk_chkinc_headers) > > # so we always include them in deps list > > deps =3D [get_variable('shared_rte_bus_vdev'), get_variable('shared_rt= e_bus_pci')] > > # add the rest of the libs to the dependencies > > -foreach l:enabled_libs > > +foreach l:dpdk_libs_enabled > > deps +=3D get_variable('shared_rte_' + l) > > endforeach > > > > diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds= .sh > > index 9131088c9d..4f93702e1f 100755 > > --- a/devtools/test-meson-builds.sh > > +++ b/devtools/test-meson-builds.sh > > @@ -122,8 +122,8 @@ config () # > > options=3D > > # deprecated libs may be disabled by default, so for complete bui= lds ensure > > # no libs are disabled > > - if ! echo $* | grep -q -- 'disable_libs' ; then > > - options=3D"$options -Ddisable_libs=3D" > > + if ! echo $* | grep -q -- 'enable_libs' ; then > > + options=3D"$options -Denable_libs=3D*" > > fi > > if echo $* | grep -qw -- '--default-library=3Dshared' ; then > > options=3D"$options -Dexamples=3Dall" > > diff --git a/lib/kni/meson.build b/lib/kni/meson.build > > index 8a71d8ba6f..5ce410f7f2 100644 > > --- a/lib/kni/meson.build > > +++ b/lib/kni/meson.build > > @@ -7,6 +7,11 @@ if is_windows > > subdir_done() > > endif > > > > +if dpdk_conf.get('RTE_IOVA_IN_MBUF') =3D=3D 0 > > + build =3D false > > + reason =3D 'requires IOVA in mbuf (set enable_iova_as_pa option)' > > +endif > > + > > if not is_linux or not dpdk_conf.get('RTE_ARCH_64') > > build =3D false > > reason =3D 'only supported on 64-bit Linux' > > diff --git a/lib/meson.build b/lib/meson.build > > index 9677239236..6018f5230d 100644 > > --- a/lib/meson.build > > +++ b/lib/meson.build > > @@ -92,20 +92,18 @@ dpdk_libs_deprecated +=3D [ > > 'kni', > > ] > > > > -disabled_libs =3D [] > > -opt_disabled_libs =3D run_command(list_dir_globs, get_option('disable_= libs'), > > +disable_libs =3D run_command(list_dir_globs, get_option('disable_libs'= ), > > check: true).stdout().split() > > -if dpdk_conf.get('RTE_IOVA_IN_MBUF') =3D=3D 0 > > - opt_disabled_libs +=3D ['kni'] > > +enable_libs =3D run_command(list_dir_globs, get_option('enable_libs'), > > + check: true).stdout().split() > > +if enable_libs.length() =3D=3D 0 > > + enable_libs =3D [] > > + foreach l:run_command(list_dir_globs, '*', check: true).stdout().s= plit() > > + if l not in dpdk_libs_deprecated > > + enable_libs +=3D [l] > > + endif > > + endforeach > > The enable libraries option is really only used for enabling optional > libraries - mandatory libs should not need to be specified. I'm wondering > about if we should append the mandatory libs to the glob search > automatically. This specifically concerns the error messsage about > mandatory libs being disabled below.... Good point. Let me relook at this. > > > endif > > -foreach l:opt_disabled_libs > > - if not optional_libs.contains(l) > > - warning('Cannot disable mandatory library "@0@"'.format(l)) > > - continue > > - endif > > - disabled_libs +=3D l > > -endforeach > > - > > > > default_cflags =3D machine_args > > default_cflags +=3D ['-DALLOW_EXPERIMENTAL_API'] > > @@ -115,8 +113,6 @@ if cc.has_argument('-Wno-format-truncation') > > default_cflags +=3D '-Wno-format-truncation' > > endif > > > > -enabled_libs =3D [] # used to print summary at the end > > - > > foreach l:libraries > > build =3D true > > reason =3D '' # set if build =3D=3D false to expla= in why > > @@ -141,13 +137,25 @@ foreach l:libraries > > deps +=3D ['eal'] > > endif > > > > - if disabled_libs.contains(l) > > + if not enable_libs.contains(l) > > + build =3D false > > + reason =3D 'not in enabled libraries build config' > > + elif disable_libs.contains(l) > > build =3D false > > reason =3D 'explicitly disabled via build config' > > - if dpdk_libs_deprecated.contains(l) > > + endif > > + > > + if not build > > + if not optional_libs.contains(l) > > + warning('Cannot disable mandatory library "@0@"'.format(l)= ) > > + build =3D true > > + reason =3D '' > > + elif dpdk_libs_deprecated.contains(l) > > This error message seems weird/wrong in the case that the user uses > "enable_libs" option rather than "disable_libs". If the user specified he > wanted to enable the vhost lib, for example, it would be strange getting = an > error about having disabled eal, mbuf etc. etc. Ack. > > > reason +=3D ' (deprecated lib)' > > endif > > - else > > + endif > > + > > + if build > > if dpdk_libs_deprecated.contains(l) > > warning('Enabling deprecated library, "@0@"'.format(l)) > > endif > > @@ -183,7 +191,7 @@ foreach l:libraries > > continue > > endif > > > > - enabled_libs +=3D name > > + dpdk_libs_enabled +=3D name > > dpdk_conf.set('RTE_LIB_' + name.to_upper(), 1) > > install_headers(headers) > > install_headers(indirect_headers) > > diff --git a/meson.build b/meson.build > > index 992ca91e88..39cb73846d 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -44,6 +44,7 @@ dpdk_extra_ldflags =3D [] > > dpdk_libs_deprecated =3D [] > > dpdk_apps_disabled =3D [] > > dpdk_libs_disabled =3D [] > > +dpdk_libs_enabled =3D [] > > dpdk_drvs_disabled =3D [] > > testpmd_drivers_sources =3D [] > > testpmd_drivers_deps =3D [] > > @@ -134,7 +135,7 @@ message(output_message + '\n') > > output_message =3D '\n=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D\nLibraries Enabled\n=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= \n' > > output_message +=3D '\nlibs:\n\t' > > output_count =3D 0 > > -foreach lib:enabled_libs > > +foreach lib:dpdk_libs_enabled > > output_message +=3D lib + ', ' > > output_count +=3D 1 > > if output_count =3D=3D 8 > > diff --git a/meson_options.txt b/meson_options.txt > > index 82c8297065..af54f8b8d4 100644 > > --- a/meson_options.txt > > +++ b/meson_options.txt > > @@ -10,7 +10,7 @@ option('disable_apps', type: 'string', value: '', des= cription: > > 'Comma-separated list of apps to explicitly disable.') > > option('disable_drivers', type: 'string', value: '', description: > > 'Comma-separated list of drivers to explicitly disable.') > > -option('disable_libs', type: 'string', value: 'flow_classify,kni', des= cription: > > +option('disable_libs', type: 'string', value: '', description: > > 'Comma-separated list of libraries to explicitly disable. [NOTE= : not all libs can be disabled]') > > option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-', description: > > 'Subdirectory of libdir where to install PMDs. Defaults to usin= g a versioned subdirectory.') > > @@ -20,6 +20,8 @@ option('enable_apps', type: 'string', value: '', desc= ription: > > 'Comma-separated list of apps to build. If unspecified, build a= ll apps.') > > option('enable_drivers', type: 'string', value: '', description: > > 'Comma-separated list of drivers to build. If unspecified, buil= d all drivers.') > > +option('enable_libs', type: 'string', value: '', description: > > + 'Comma-separated list of libraries to explicitly enable.') > > s/libraries/optional libraries/ ?? > Maybe like with the disable option, add a note that mandatory libs are > always enabled? I'll update this too. > > > option('enable_driver_sdk', type: 'boolean', value: false, description= : > > 'Install headers to build drivers.') > > option('enable_kmods', type: 'boolean', value: false, description: > > -- > > 2.40.1 > > > Thanks Bruce. --=20 David Marchand