patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH 1/1] build: update link args and includes for libarchive
@ 2023-10-20 17:01 Srikanth Yalavarthi
  2023-10-23  9:26 ` Bruce Richardson
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-20 17:01 UTC (permalink / raw)
  To: Bruce Richardson, Aaron Conole, Igor Russkikh, David Marchand
  Cc: dev, syalavarthi, sshankarnara, jerinj, stable

In order to avoid linking with all libraries listed as
Libs.private in libarchive.pc, libarchive is not added
to ext_deps during meson setup.

Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.

This patch updates meson build files to add libarchive's
includedir and libdir to compiler flags and would not require
setting c_args and c_link_args externally.

Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org

Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
 config/meson.build  | 1 +
 lib/eal/meson.build | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..1bacea74ab 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -239,6 +239,7 @@ if libarchive.found()
     # Push libarchive link dependency at the project level to support
     # statically linking dpdk apps. Details at:
     # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
+    add_project_link_arguments('-L' + libarchive.get_variable(pkgconfig: 'libdir'), language: 'c')
     add_project_link_arguments('-larchive', language: 'c')
     dpdk_extra_ldflags += '-larchive'
 endif
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..741a5cd088 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
 if dpdk_conf.has('RTE_USE_LIBBSD')
     ext_deps += libbsd
 endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+    includes += include_directories(libarchive.get_variable(pkgconfig: 'includedir'))
+endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
-- 
2.42.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
@ 2023-10-23  9:26 ` Bruce Richardson
  2023-10-23 11:40   ` [EXT] " Srikanth Yalavarthi
  2023-10-29  8:10 ` [PATCH v2 1/1] build: add libarchive to external deps Srikanth Yalavarthi
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23  9:26 UTC (permalink / raw)
  To: Srikanth Yalavarthi
  Cc: Aaron Conole, Igor Russkikh, David Marchand, dev, sshankarnara,
	jerinj, stable

On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi wrote:
> In order to avoid linking with all libraries listed as
> Libs.private in libarchive.pc, libarchive is not added
> to ext_deps during meson setup.
> 
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
> 
> This patch updates meson build files to add libarchive's
> includedir and libdir to compiler flags and would not require
> setting c_args and c_link_args externally.
> 
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> ---

Checking back through the mail archives I'm still a little unclear as to
what breaks when we try using libarchive as any other package with a
pkg-config file? I would have thought the best solution was just to add
libarchive as an external dependency, found using pkg-config, to EAL. When
we add it as a dependency, rather than using c/ldflags, we should get all
this path fixup for free?
Can you clarify what breaks when we add libarchive as a libeal dependency
only?

/Bruce

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-23  9:26 ` Bruce Richardson
@ 2023-10-23 11:40   ` Srikanth Yalavarthi
  2023-10-23 11:53     ` Bruce Richardson
  0 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-23 11:40 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Aaron Conole, Igor Russkikh, David Marchand, dev,
	Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
	stable, Srikanth Yalavarthi

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 23 October 2023 14:56
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org
> Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi wrote:
> > In order to avoid linking with all libraries listed as Libs.private in
> > libarchive.pc, libarchive is not added to ext_deps during meson setup.
> >
> > Since libarchive is not added to ext_deps, cross-compilation or native
> > compilation with libarchive installed in non-standard location fails
> > with errors related to "cannot find -larchive"
> > or "archive.h: No such file or directory". In order to fix the build
> > failures, user is required to define the 'c_args' and 'c_link_args'
> > with '-I<includedir>' and '-L<libdir>'.
> >
> > This patch updates meson build files to add libarchive's includedir
> > and libdir to compiler flags and would not require setting c_args and
> > c_link_args externally.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > ---
> 
> Checking back through the mail archives I'm still a little unclear as to what
> breaks when we try using libarchive as any other package with a pkg-config
> file? I would have thought the best solution was just to add libarchive as an
> external dependency, found using pkg-config, to EAL. When we add it as a
> dependency, rather than using c/ldflags, we should get all this path fixup for
> free?
> Can you clarify what breaks when we add libarchive as a libeal dependency
> only?

Below is my observation.

In current implementation, we are looking for libarchive's availability through pkg-config.
When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive' to ldflags.

Since, we are not adding libarchive to ext_deps (to avoid linking with deps.private), the
variables provided by pkg-config like 'includedir' and 'libdir' are ignored by meson and
are not used as part of c_args (-I<includedir>) and c_link_args (-L<libdir>).

In this case, build would fail during cross-compilation and native  compilation (when
libarchive is not installed through package managers like yum/apt). And, we need to
manually set c_args and c_link_args as per the build environment to fix the failures.

This patch fixes the issue observed and avoids the requirement to set c_args and c_link_args
manually . Below are the steps to reproduce the issue (cross-compile for aarch64). Tested on
ubuntu:20.04 docker. Same can be reproduced for native build, if libarchive is not installed
through apt-get.


rm -rf /dpdk-buildtest && mkdir -p /dpdk-buildtest

# libarchive
cd /dpdk-buildtest
wget -N https://www.libarchive.org/downloads/libarchive-3.6.1.tar.gz
tar -xzf libarchive-3.6.1.tar.gz
cd libarchive-3.6.1
./configure --prefix /dpdk-buildtest/install-aarch64 --host=aarch64-linux-gnu --without-xml2 --without-lzma --without-zlib
make
make install

# dpdk
cd /dpdk-buildtest
git clone https://dpdk.org/git/dpdk
cd dpdk
git checkout main
export PKG_CONFIG_PATH=/dpdk-buildtest/install-aarch64/lib/pkgconfig
meson setup --prefix /dpdk-buildtest/install-aarch64 --cross-file config/arm/arm64_armv8_linux_gcc build
ninja -C build

### meson log ### libarchive is found by pkg-config
Has header "execinfo.h" : YES
Found pkg-config: /usr/bin/aarch64-linux-gnu-pkg-config (0.29.1)
Run-time dependency libarchive found: YES 3.6.1
Run-time dependency libbsd found: NO (tried pkgconfig)
Run-time dependency jansson found: NO (tried pkgconfig)


### compilation log ### -I<inlcudedir> is not used
[39/2835] Compiling C object 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o'.
FAILED: lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o
aarch64-linux-gnu-gcc -Ilib/76b5a35@@rte_eal@sta -Ilib -I../lib -I. -I../ -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/arm/include -I../lib/eal/arm/include -Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/telemetry/../metrics -I../lib/telemetry/../metrics -Ilib/telemetry -I../lib/telemetry -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=c11 -O3 -include rte_config.h -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-address-of-packed-member -Wno-packed-not-aligned -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=armv8-a+crc -moutline-atomics -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation '-DABI_VERSION="24.0"' -DRTE_LOG_DEFAULT_LOGTYPE=lib.eal -MD -MQ 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o' -MF 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o.d' -o 'lib/76b5a35@@rte_eal@sta/eal_unix_eal_firmware.c.o' -c ../lib/eal/unix/eal_firmware.c
../lib/eal/unix/eal_firmware.c:6:10: fatal error: archive.h: No such file or directory
    6 | #include <archive.h>
      |          ^~~~~~~~~~~
compilation terminated.

> 
> /Bruce

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-23 11:40   ` [EXT] " Srikanth Yalavarthi
@ 2023-10-23 11:53     ` Bruce Richardson
  2023-10-23 12:46       ` Srikanth Yalavarthi
  0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23 11:53 UTC (permalink / raw)
  To: Srikanth Yalavarthi
  Cc: Aaron Conole, Igor Russkikh, David Marchand, dev,
	Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
	stable

On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: 23 October 2023 14:56
> > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; stable@dpdk.org
> > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> > libarchive
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi wrote:
> > > In order to avoid linking with all libraries listed as Libs.private in
> > > libarchive.pc, libarchive is not added to ext_deps during meson setup.
> > >
> > > Since libarchive is not added to ext_deps, cross-compilation or native
> > > compilation with libarchive installed in non-standard location fails
> > > with errors related to "cannot find -larchive"
> > > or "archive.h: No such file or directory". In order to fix the build
> > > failures, user is required to define the 'c_args' and 'c_link_args'
> > > with '-I<includedir>' and '-L<libdir>'.
> > >
> > > This patch updates meson build files to add libarchive's includedir
> > > and libdir to compiler flags and would not require setting c_args and
> > > c_link_args externally.
> > >
> > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > ---
> > 
> > Checking back through the mail archives I'm still a little unclear as to what
> > breaks when we try using libarchive as any other package with a pkg-config
> > file? I would have thought the best solution was just to add libarchive as an
> > external dependency, found using pkg-config, to EAL. When we add it as a
> > dependency, rather than using c/ldflags, we should get all this path fixup for
> > free?
> > Can you clarify what breaks when we add libarchive as a libeal dependency
> > only?
> 
> Below is my observation.
> 
> In current implementation, we are looking for libarchive's availability through pkg-config.
> When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive' to ldflags.
> 
> Since, we are not adding libarchive to ext_deps (to avoid linking with deps.private), the

This is the bit I'm a bit stuck on. What is the issues with adding
libarchive to ext_deps? For other libs, when doing static builds we have to
link with deps.private and it's the correct behaviour AFAIK. Not doing so
would surely lead to problematic builds, no?

/Bruce

^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-23 11:53     ` Bruce Richardson
@ 2023-10-23 12:46       ` Srikanth Yalavarthi
  2023-10-23 13:00         ` Bruce Richardson
  0 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-23 12:46 UTC (permalink / raw)
  To: Bruce Richardson, dmitry.kozliuk
  Cc: Aaron Conole, Igor Russkikh, David Marchand, dev,
	Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
	stable, Srikanth Yalavarthi

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 23 October 2023 17:24
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org
> Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
> 
> On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: 23 October 2023 14:56
> > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > <irusskikh@marvell.com>; David Marchand
> <david.marchand@redhat.com>;
> > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > <jerinj@marvell.com>; stable@dpdk.org
> > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes
> > > for libarchive
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi
> > > wrote:
> > > > In order to avoid linking with all libraries listed as
> > > > Libs.private in libarchive.pc, libarchive is not added to ext_deps during
> meson setup.
> > > >
> > > > Since libarchive is not added to ext_deps, cross-compilation or
> > > > native compilation with libarchive installed in non-standard
> > > > location fails with errors related to "cannot find -larchive"
> > > > or "archive.h: No such file or directory". In order to fix the
> > > > build failures, user is required to define the 'c_args' and 'c_link_args'
> > > > with '-I<includedir>' and '-L<libdir>'.
> > > >
> > > > This patch updates meson build files to add libarchive's
> > > > includedir and libdir to compiler flags and would not require
> > > > setting c_args and c_link_args externally.
> > > >
> > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > ---
> > >
> > > Checking back through the mail archives I'm still a little unclear
> > > as to what breaks when we try using libarchive as any other package
> > > with a pkg-config file? I would have thought the best solution was
> > > just to add libarchive as an external dependency, found using
> > > pkg-config, to EAL. When we add it as a dependency, rather than
> > > using c/ldflags, we should get all this path fixup for free?
> > > Can you clarify what breaks when we add libarchive as a libeal
> > > dependency only?
> >
> > Below is my observation.
> >
> > In current implementation, we are looking for libarchive's availability
> through pkg-config.
> > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive'
> to ldflags.
> >
> > Since, we are not adding libarchive to ext_deps (to avoid linking with
> > deps.private), the
> 
> This is the bit I'm a bit stuck on. What is the issues with adding libarchive to
> ext_deps? For other libs, when doing static builds we have to link with
> deps.private and it's the correct behaviour AFAIK. Not doing so would surely
> lead to problematic builds, no?

I agree on adding to ext_deps as it's the correct behaviour.

However, there was a discussion in mail archives regarding this.
https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/

Adding Dmitry Kozlyuk for comments.

> 
> /Bruce

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-23 12:46       ` Srikanth Yalavarthi
@ 2023-10-23 13:00         ` Bruce Richardson
  2023-10-23 13:06           ` Bruce Richardson
  0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23 13:00 UTC (permalink / raw)
  To: Srikanth Yalavarthi
  Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
	Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
	stable

On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: 23 October 2023 17:24
> > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; stable@dpdk.org
> > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> > libarchive
> > 
> > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > -----Original Message-----
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > Sent: 23 October 2023 14:56
> > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > <irusskikh@marvell.com>; David Marchand
> > <david.marchand@redhat.com>;
> > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes
> > > > for libarchive
> > > >
> > > > External Email
> > > >
> > > > --------------------------------------------------------------------
> > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi
> > > > wrote:
> > > > > In order to avoid linking with all libraries listed as
> > > > > Libs.private in libarchive.pc, libarchive is not added to ext_deps during
> > meson setup.
> > > > >
> > > > > Since libarchive is not added to ext_deps, cross-compilation or
> > > > > native compilation with libarchive installed in non-standard
> > > > > location fails with errors related to "cannot find -larchive"
> > > > > or "archive.h: No such file or directory". In order to fix the
> > > > > build failures, user is required to define the 'c_args' and 'c_link_args'
> > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > >
> > > > > This patch updates meson build files to add libarchive's
> > > > > includedir and libdir to compiler flags and would not require
> > > > > setting c_args and c_link_args externally.
> > > > >
> > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > ---
> > > >
> > > > Checking back through the mail archives I'm still a little unclear
> > > > as to what breaks when we try using libarchive as any other package
> > > > with a pkg-config file? I would have thought the best solution was
> > > > just to add libarchive as an external dependency, found using
> > > > pkg-config, to EAL. When we add it as a dependency, rather than
> > > > using c/ldflags, we should get all this path fixup for free?
> > > > Can you clarify what breaks when we add libarchive as a libeal
> > > > dependency only?
> > >
> > > Below is my observation.
> > >
> > > In current implementation, we are looking for libarchive's availability
> > through pkg-config.
> > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive'
> > to ldflags.
> > >
> > > Since, we are not adding libarchive to ext_deps (to avoid linking with
> > > deps.private), the
> > 
> > This is the bit I'm a bit stuck on. What is the issues with adding libarchive to
> > ext_deps? For other libs, when doing static builds we have to link with
> > deps.private and it's the correct behaviour AFAIK. Not doing so would surely
> > lead to problematic builds, no?
> 
> I agree on adding to ext_deps as it's the correct behaviour.
> 
> However, there was a discussion in mail archives regarding this.
> https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
> 
> Adding Dmitry Kozlyuk for comments.
> 
Testing it out myself, the sample applications don't build statically due
to missing dependencies. The libarchive-dev package on Ubuntu, doesn't seem
to install all dependent packages for static builds.
I had to manually install liblz4-dev and libacl1-dev packages, and then
test-meson-builds ran successfully.

Personally, it looks to me like a packaging issue, in that I would expect
the -dev package to install all required dependent dev packages. I also
think using the pkgconfig as a regular dependency is the way we should look
to go, and if necessary, document the list of extra dependencies needed for
libarchive in our docs.

/Bruce

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-23 13:00         ` Bruce Richardson
@ 2023-10-23 13:06           ` Bruce Richardson
  2023-10-23 14:01             ` Srikanth Yalavarthi
  0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-10-23 13:06 UTC (permalink / raw)
  To: Srikanth Yalavarthi
  Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
	Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
	stable

On Mon, Oct 23, 2023 at 02:00:33PM +0100, Bruce Richardson wrote:
> On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: 23 October 2023 17:24
> > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > <irusskikh@marvell.com>; David Marchand <david.marchand@redhat.com>;
> > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > <jerinj@marvell.com>; stable@dpdk.org
> > > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> > > libarchive
> > > 
> > > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > > -----Original Message-----
> > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Sent: 23 October 2023 14:56
> > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > <irusskikh@marvell.com>; David Marchand
> > > <david.marchand@redhat.com>;
> > > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and includes
> > > > > for libarchive
> > > > >
> > > > > External Email
> > > > >
> > > > > --------------------------------------------------------------------
> > > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth Yalavarthi
> > > > > wrote:
> > > > > > In order to avoid linking with all libraries listed as
> > > > > > Libs.private in libarchive.pc, libarchive is not added to ext_deps during
> > > meson setup.
> > > > > >
> > > > > > Since libarchive is not added to ext_deps, cross-compilation or
> > > > > > native compilation with libarchive installed in non-standard
> > > > > > location fails with errors related to "cannot find -larchive"
> > > > > > or "archive.h: No such file or directory". In order to fix the
> > > > > > build failures, user is required to define the 'c_args' and 'c_link_args'
> > > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > > >
> > > > > > This patch updates meson build files to add libarchive's
> > > > > > includedir and libdir to compiler flags and would not require
> > > > > > setting c_args and c_link_args externally.
> > > > > >
> > > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > > Cc: stable@dpdk.org
> > > > > >
> > > > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > ---
> > > > >
> > > > > Checking back through the mail archives I'm still a little unclear
> > > > > as to what breaks when we try using libarchive as any other package
> > > > > with a pkg-config file? I would have thought the best solution was
> > > > > just to add libarchive as an external dependency, found using
> > > > > pkg-config, to EAL. When we add it as a dependency, rather than
> > > > > using c/ldflags, we should get all this path fixup for free?
> > > > > Can you clarify what breaks when we add libarchive as a libeal
> > > > > dependency only?
> > > >
> > > > Below is my observation.
> > > >
> > > > In current implementation, we are looking for libarchive's availability
> > > through pkg-config.
> > > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-larchive'
> > > to ldflags.
> > > >
> > > > Since, we are not adding libarchive to ext_deps (to avoid linking with
> > > > deps.private), the
> > > 
> > > This is the bit I'm a bit stuck on. What is the issues with adding libarchive to
> > > ext_deps? For other libs, when doing static builds we have to link with
> > > deps.private and it's the correct behaviour AFAIK. Not doing so would surely
> > > lead to problematic builds, no?
> > 
> > I agree on adding to ext_deps as it's the correct behaviour.
> > 
> > However, there was a discussion in mail archives regarding this.
> > https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
> > 
> > Adding Dmitry Kozlyuk for comments.
> > 
> Testing it out myself, the sample applications don't build statically due
> to missing dependencies. The libarchive-dev package on Ubuntu, doesn't seem
> to install all dependent packages for static builds.
> I had to manually install liblz4-dev and libacl1-dev packages, and then
> test-meson-builds ran successfully.
> 
> Personally, it looks to me like a packaging issue, in that I would expect
> the -dev package to install all required dependent dev packages. I also
> think using the pkgconfig as a regular dependency is the way we should look
> to go, and if necessary, document the list of extra dependencies needed for
> libarchive in our docs.
> 
For reference, here is the diff I tested with on Ubuntu 23.04:

diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..4ff101767e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
 libarchive = dependency('libarchive', required: false, method: 'pkg-config')
 if libarchive.found()
     dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
-    # Push libarchive link dependency at the project level to support
-    # statically linking dpdk apps. Details at:
-    # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
-    add_project_link_arguments('-larchive', language: 'c')
-    dpdk_extra_ldflags += '-larchive'
 endif
 
 # check for libbsd
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
 if dpdk_conf.has('RTE_USE_LIBBSD')
     ext_deps += libbsd
 endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+    ext_deps += libarchive
+endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-23 13:06           ` Bruce Richardson
@ 2023-10-23 14:01             ` Srikanth Yalavarthi
  2023-10-26 12:53               ` Srikanth Yalavarthi
  0 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-23 14:01 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
	Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
	stable, Srikanth Yalavarthi

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 23 October 2023 18:36
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: dmitry.kozliuk@gmail.com; Aaron Conole <aconole@redhat.com>; Igor
> Russkikh <irusskikh@marvell.com>; David Marchand
> <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar Shankar
> Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org
> Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
> 
> On Mon, Oct 23, 2023 at 02:00:33PM +0100, Bruce Richardson wrote:
> > On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > > > -----Original Message-----
> > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > Sent: 23 October 2023 17:24
> > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > <irusskikh@marvell.com>; David Marchand
> > > > <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar
> Shankar
> > > > Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > includes for libarchive
> > > >
> > > > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > > > -----Original Message-----
> > > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > Sent: 23 October 2023 14:56
> > > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > > <irusskikh@marvell.com>; David Marchand
> > > > <david.marchand@redhat.com>;
> > > > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > > > includes for libarchive
> > > > > >
> > > > > > External Email
> > > > > >
> > > > > > --------------------------------------------------------------
> > > > > > ------
> > > > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth
> > > > > > Yalavarthi
> > > > > > wrote:
> > > > > > > In order to avoid linking with all libraries listed as
> > > > > > > Libs.private in libarchive.pc, libarchive is not added to
> > > > > > > ext_deps during
> > > > meson setup.
> > > > > > >
> > > > > > > Since libarchive is not added to ext_deps, cross-compilation
> > > > > > > or native compilation with libarchive installed in
> > > > > > > non-standard location fails with errors related to "cannot find -
> larchive"
> > > > > > > or "archive.h: No such file or directory". In order to fix
> > > > > > > the build failures, user is required to define the 'c_args' and
> 'c_link_args'
> > > > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > > > >
> > > > > > > This patch updates meson build files to add libarchive's
> > > > > > > includedir and libdir to compiler flags and would not
> > > > > > > require setting c_args and c_link_args externally.
> > > > > > >
> > > > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > > > Cc: stable@dpdk.org
> > > > > > >
> > > > > > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > > ---
> > > > > >
> > > > > > Checking back through the mail archives I'm still a little
> > > > > > unclear as to what breaks when we try using libarchive as any
> > > > > > other package with a pkg-config file? I would have thought the
> > > > > > best solution was just to add libarchive as an external
> > > > > > dependency, found using pkg-config, to EAL. When we add it as
> > > > > > a dependency, rather than using c/ldflags, we should get all this
> path fixup for free?
> > > > > > Can you clarify what breaks when we add libarchive as a libeal
> > > > > > dependency only?
> > > > >
> > > > > Below is my observation.
> > > > >
> > > > > In current implementation, we are looking for libarchive's
> > > > > availability
> > > > through pkg-config.
> > > > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-
> larchive'
> > > > to ldflags.
> > > > >
> > > > > Since, we are not adding libarchive to ext_deps (to avoid
> > > > > linking with deps.private), the
> > > >
> > > > This is the bit I'm a bit stuck on. What is the issues with adding
> > > > libarchive to ext_deps? For other libs, when doing static builds
> > > > we have to link with deps.private and it's the correct behaviour
> > > > AFAIK. Not doing so would surely lead to problematic builds, no?
> > >
> > > I agree on adding to ext_deps as it's the correct behaviour.
> > >
> > > However, there was a discussion in mail archives regarding this.
> > > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__inbox.dpdk.org_
> > > dev_20210605004024.660267a1-
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPa
> > > z7xtfQ&r=SNPqUkGl0n_Ms1iJa_6wD6LBwX8efL_NOyXvAX-
> iCMI&m=HAX8pzasBDOgY
> > > 4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SPMpIPssi23l&s=wKtxZ-
> fddu7b0b
> > > ADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> > >
> > > Adding Dmitry Kozlyuk for comments.
> > >
> > Testing it out myself, the sample applications don't build statically
> > due to missing dependencies. The libarchive-dev package on Ubuntu,
> > doesn't seem to install all dependent packages for static builds.
> > I had to manually install liblz4-dev and libacl1-dev packages, and
> > then test-meson-builds ran successfully.
> >
> > Personally, it looks to me like a packaging issue, in that I would
> > expect the -dev package to install all required dependent dev
> > packages. I also think using the pkgconfig as a regular dependency is
> > the way we should look to go, and if necessary, document the list of
> > extra dependencies needed for libarchive in our docs.
> >
> For reference, here is the diff I tested with on Ubuntu 23.04:
> 
> diff --git a/config/meson.build b/config/meson.build index
> d56b0f9bce..4ff101767e 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE',
> cc.has_header('execinfo.h') or is_windows)  libarchive =
> dependency('libarchive', required: false, method: 'pkg-config')  if
> libarchive.found()
>      dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> -    # Push libarchive link dependency at the project level to support
> -    # statically linking dpdk apps. Details at:
> -    # https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__inbox.dpdk.org_dev_20210605004024.660267a1-
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=SNPqUkGl0n_M
> s1iJa_6wD6LBwX8efL_NOyXvAX-
> iCMI&m=HAX8pzasBDOgY4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SP
> MpIPssi23l&s=wKtxZ-fddu7b0bADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> -    add_project_link_arguments('-larchive', language: 'c')
> -    dpdk_extra_ldflags += '-larchive'
>  endif
> 
>  # check for libbsd
> diff --git a/lib/eal/meson.build b/lib/eal/meson.build index
> 9942104386..e1d6c4cf17 100644
> --- a/lib/eal/meson.build
> +++ b/lib/eal/meson.build
> @@ -21,6 +21,9 @@ endif
>  if dpdk_conf.has('RTE_USE_LIBBSD')
>      ext_deps += libbsd
>  endif
> +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> +    ext_deps += libarchive
> +endif
>  if cc.has_function('getentropy', prefix : '#include <unistd.h>')
>      cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
>  endif

I have tried this approach earlier and it work as expected. But, based on the
discussion in mail archives, I have pushed an alternate one.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for libarchive
  2023-10-23 14:01             ` Srikanth Yalavarthi
@ 2023-10-26 12:53               ` Srikanth Yalavarthi
  0 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-26 12:53 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: dmitry.kozliuk, Aaron Conole, Igor Russkikh, David Marchand, dev,
	Shivah Shankar Shankar Narayan Rao, Jerin Jacob Kollanukkaran,
	stable, Srikanth Yalavarthi

> -----Original Message-----
> From: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Sent: 23 October 2023 19:32
> To: Bruce Richardson <bruce.richardson@intel.com>
> Cc: dmitry.kozliuk@gmail.com; Aaron Conole <aconole@redhat.com>; Igor
> Russkikh <irusskikh@marvell.com>; David Marchand
> <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar Shankar
> Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; stable@dpdk.org; Srikanth Yalavarthi
> <syalavarthi@marvell.com>; Srikanth Yalavarthi <syalavarthi@marvell.com>
> Subject: RE: [EXT] Re: [PATCH 1/1] build: update link args and includes for
> libarchive
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: 23 October 2023 18:36
> > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Cc: dmitry.kozliuk@gmail.com; Aaron Conole <aconole@redhat.com>; Igor
> > Russkikh <irusskikh@marvell.com>; David Marchand
> > <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar Shankar
> > Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; stable@dpdk.org
> > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and
> > includes for libarchive
> >
> > On Mon, Oct 23, 2023 at 02:00:33PM +0100, Bruce Richardson wrote:
> > > On Mon, Oct 23, 2023 at 12:46:59PM +0000, Srikanth Yalavarthi wrote:
> > > > > -----Original Message-----
> > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Sent: 23 October 2023 17:24
> > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > <irusskikh@marvell.com>; David Marchand
> > > > > <david.marchand@redhat.com>; dev@dpdk.org; Shivah Shankar
> > Shankar
> > > > > Narayan Rao <sshankarnara@marvell.com>; Jerin Jacob
> > > > > Kollanukkaran <jerinj@marvell.com>; stable@dpdk.org
> > > > > Subject: Re: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > > includes for libarchive
> > > > >
> > > > > On Mon, Oct 23, 2023 at 11:40:14AM +0000, Srikanth Yalavarthi wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > > > Sent: 23 October 2023 14:56
> > > > > > > To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > > > > > > Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> > > > > > > <irusskikh@marvell.com>; David Marchand
> > > > > <david.marchand@redhat.com>;
> > > > > > > dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> > > > > > > <sshankarnara@marvell.com>; Jerin Jacob Kollanukkaran
> > > > > > > <jerinj@marvell.com>; stable@dpdk.org
> > > > > > > Subject: [EXT] Re: [PATCH 1/1] build: update link args and
> > > > > > > includes for libarchive
> > > > > > >
> > > > > > > External Email
> > > > > > >
> > > > > > > ------------------------------------------------------------
> > > > > > > --
> > > > > > > ------
> > > > > > > -- On Fri, Oct 20, 2023 at 10:01:35AM -0700, Srikanth
> > > > > > > Yalavarthi
> > > > > > > wrote:
> > > > > > > > In order to avoid linking with all libraries listed as
> > > > > > > > Libs.private in libarchive.pc, libarchive is not added to
> > > > > > > > ext_deps during
> > > > > meson setup.
> > > > > > > >
> > > > > > > > Since libarchive is not added to ext_deps,
> > > > > > > > cross-compilation or native compilation with libarchive
> > > > > > > > installed in non-standard location fails with errors
> > > > > > > > related to "cannot find -
> > larchive"
> > > > > > > > or "archive.h: No such file or directory". In order to fix
> > > > > > > > the build failures, user is required to define the
> > > > > > > > 'c_args' and
> > 'c_link_args'
> > > > > > > > with '-I<includedir>' and '-L<libdir>'.
> > > > > > > >
> > > > > > > > This patch updates meson build files to add libarchive's
> > > > > > > > includedir and libdir to compiler flags and would not
> > > > > > > > require setting c_args and c_link_args externally.
> > > > > > > >
> > > > > > > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > > > > > > Cc: stable@dpdk.org
> > > > > > > >
> > > > > > > > Signed-off-by: Srikanth Yalavarthi
> > > > > > > > <syalavarthi@marvell.com>
> > > > > > > > ---
> > > > > > >
> > > > > > > Checking back through the mail archives I'm still a little
> > > > > > > unclear as to what breaks when we try using libarchive as
> > > > > > > any other package with a pkg-config file? I would have
> > > > > > > thought the best solution was just to add libarchive as an
> > > > > > > external dependency, found using pkg-config, to EAL. When we
> > > > > > > add it as a dependency, rather than using c/ldflags, we
> > > > > > > should get all this
> > path fixup for free?
> > > > > > > Can you clarify what breaks when we add libarchive as a
> > > > > > > libeal dependency only?
> > > > > >
> > > > > > Below is my observation.
> > > > > >
> > > > > > In current implementation, we are looking for libarchive's
> > > > > > availability
> > > > > through pkg-config.
> > > > > > When found, we are setting RTE_HAS_LIBARCHIVE=1 and adding '-
> > larchive'
> > > > > to ldflags.
> > > > > >
> > > > > > Since, we are not adding libarchive to ext_deps (to avoid
> > > > > > linking with deps.private), the
> > > > >
> > > > > This is the bit I'm a bit stuck on. What is the issues with
> > > > > adding libarchive to ext_deps? For other libs, when doing static
> > > > > builds we have to link with deps.private and it's the correct
> > > > > behaviour AFAIK. Not doing so would surely lead to problematic
> builds, no?
> > > >
> > > > I agree on adding to ext_deps as it's the correct behaviour.
> > > >
> > > > However, there was a discussion in mail archives regarding this.
> > > > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__inbox.dpdk.org_
> > > > dev_20210605004024.660267a1-
> > 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPa
> > > > z7xtfQ&r=SNPqUkGl0n_Ms1iJa_6wD6LBwX8efL_NOyXvAX-
> > iCMI&m=HAX8pzasBDOgY
> > > >
> 4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SPMpIPssi23l&s=wKtxZ-
> > fddu7b0b
> > > > ADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> > > >
> > > > Adding Dmitry Kozlyuk for comments.
> > > >
> > > Testing it out myself, the sample applications don't build
> > > statically due to missing dependencies. The libarchive-dev package
> > > on Ubuntu, doesn't seem to install all dependent packages for static
> builds.
> > > I had to manually install liblz4-dev and libacl1-dev packages, and
> > > then test-meson-builds ran successfully.
> > >
> > > Personally, it looks to me like a packaging issue, in that I would
> > > expect the -dev package to install all required dependent dev
> > > packages. I also think using the pkgconfig as a regular dependency
> > > is the way we should look to go, and if necessary, document the list
> > > of extra dependencies needed for libarchive in our docs.
> > >
> > For reference, here is the diff I tested with on Ubuntu 23.04:
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > d56b0f9bce..4ff101767e 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE',
> > cc.has_header('execinfo.h') or is_windows)  libarchive =
> > dependency('libarchive', required: false, method: 'pkg-config')  if
> > libarchive.found()
> >      dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> > -    # Push libarchive link dependency at the project level to support
> > -    # statically linking dpdk apps. Details at:
> > -    # https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__inbox.dpdk.org_dev_20210605004024.660267a1-
> >
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=SNPqUkGl0n_M
> > s1iJa_6wD6LBwX8efL_NOyXvAX-
> >
> iCMI&m=HAX8pzasBDOgY4zxEIMFHaTzH02VdRpw5xfHxjYz1CorbIA3_Gw9SP
> > MpIPssi23l&s=wKtxZ-fddu7b0bADmwT_nOeJ_UjbuFNea3pcTuPKAGQ&e=
> > -    add_project_link_arguments('-larchive', language: 'c')
> > -    dpdk_extra_ldflags += '-larchive'
> >  endif
> >
> >  # check for libbsd
> > diff --git a/lib/eal/meson.build b/lib/eal/meson.build index
> > 9942104386..e1d6c4cf17 100644
> > --- a/lib/eal/meson.build
> > +++ b/lib/eal/meson.build
> > @@ -21,6 +21,9 @@ endif
> >  if dpdk_conf.has('RTE_USE_LIBBSD')
> >      ext_deps += libbsd
> >  endif
> > +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> > +    ext_deps += libarchive
> > +endif
> >  if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> >      cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> >  endif
> 
> I have tried this approach earlier and it work as expected. But, based on the
> discussion in mail archives, I have pushed an alternate one.

Will resubmit the patch with adding libarchive to ext_deps.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/1] build: add libarchive to external deps
  2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
  2023-10-23  9:26 ` Bruce Richardson
@ 2023-10-29  8:10 ` Srikanth Yalavarthi
  2023-10-29  8:20 ` [PATCH v3 " Srikanth Yalavarthi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-29  8:10 UTC (permalink / raw)
  To: Bruce Richardson, Srikanth Yalavarthi, Aaron Conole,
	Igor Russkikh, David Marchand
  Cc: dev, sshankarnara, jerinj, stable

In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.

Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.

This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.

Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org

Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
 config/meson.build          | 5 -----
 drivers/ml/cnxk/meson.build | 1 +
 lib/eal/meson.build         | 3 +++
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..4ff101767e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
 libarchive = dependency('libarchive', required: false, method: 'pkg-config')
 if libarchive.found()
     dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
-    # Push libarchive link dependency at the project level to support
-    # statically linking dpdk apps. Details at:
-    # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
-    add_project_link_arguments('-larchive', language: 'c')
-    dpdk_extra_ldflags += '-larchive'
 endif
 
 # check for libbsd
diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
index 0680a0faa5..921dc7e89b 100644
--- a/drivers/ml/cnxk/meson.build
+++ b/drivers/ml/cnxk/meson.build
@@ -67,6 +67,7 @@ sources += files(
         'mvtvm_ml_model.c',
 )
 
+ext_deps += libarchive
 ext_deps += jansson_dep
 ext_deps += dlpack_dep
 ext_deps += dmlc_dep
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
 if dpdk_conf.has('RTE_USE_LIBBSD')
     ext_deps += libbsd
 endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+    ext_deps += libarchive
+endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
-- 
2.42.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/1] build: add libarchive to external deps
  2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
  2023-10-23  9:26 ` Bruce Richardson
  2023-10-29  8:10 ` [PATCH v2 1/1] build: add libarchive to external deps Srikanth Yalavarthi
@ 2023-10-29  8:20 ` Srikanth Yalavarthi
  2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
  2023-11-06  4:12 ` [PATCH v5 " Srikanth Yalavarthi
  4 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-10-29  8:20 UTC (permalink / raw)
  To: Bruce Richardson, Srikanth Yalavarthi, David Marchand,
	Aaron Conole, Igor Russkikh
  Cc: dev, sshankarnara, aprabhu, ptakkar, stable

In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.

Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.

This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.

Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org

Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
Depends-on: series-30002 ("Implementation of revised ml/cnxk driver")

v3:
  - Updated depends on series information

v2:
  - Updated patch to include libarchive to ext_deps

v1:
  - Initial changes

 config/meson.build          | 5 -----
 drivers/ml/cnxk/meson.build | 1 +
 lib/eal/meson.build         | 3 +++
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index d56b0f9bce..4ff101767e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -236,11 +236,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
 libarchive = dependency('libarchive', required: false, method: 'pkg-config')
 if libarchive.found()
     dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
-    # Push libarchive link dependency at the project level to support
-    # statically linking dpdk apps. Details at:
-    # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
-    add_project_link_arguments('-larchive', language: 'c')
-    dpdk_extra_ldflags += '-larchive'
 endif
 
 # check for libbsd
diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
index 0680a0faa5..921dc7e89b 100644
--- a/drivers/ml/cnxk/meson.build
+++ b/drivers/ml/cnxk/meson.build
@@ -67,6 +67,7 @@ sources += files(
         'mvtvm_ml_model.c',
 )
 
+ext_deps += libarchive
 ext_deps += jansson_dep
 ext_deps += dlpack_dep
 ext_deps += dmlc_dep
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
 if dpdk_conf.has('RTE_USE_LIBBSD')
     ext_deps += libbsd
 endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+    ext_deps += libarchive
+endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
-- 
2.42.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/1] build: add libarchive to external deps
  2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
                   ` (2 preceding siblings ...)
  2023-10-29  8:20 ` [PATCH v3 " Srikanth Yalavarthi
@ 2023-11-03 16:38 ` Srikanth Yalavarthi
  2023-11-03 16:50   ` Bruce Richardson
  2023-11-06  4:12 ` [PATCH v5 " Srikanth Yalavarthi
  4 siblings, 1 reply; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-11-03 16:38 UTC (permalink / raw)
  To: Bruce Richardson, Srikanth Yalavarthi, David Marchand,
	Aaron Conole, Igor Russkikh
  Cc: dev, sshankarnara, aprabhu, ptakkar, stable

In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.

Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.

This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.

Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org

Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
v4:
  - Rebase over latest main
v3:
  - Add to libarchive ext_deps
v2:
  - Update ml/cnxk meson config
v1:
  - Initial patch

 config/meson.build          | 5 -----
 drivers/ml/cnxk/meson.build | 1 +
 lib/eal/meson.build         | 3 +++
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 0968351740..250833d0a4 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
 libarchive = dependency('libarchive', required: false, method: 'pkg-config')
 if libarchive.found()
     dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
-    # Push libarchive link dependency at the project level to support
-    # statically linking dpdk apps. Details at:
-    # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
-    add_project_link_arguments('-larchive', language: 'c')
-    dpdk_extra_ldflags += '-larchive'
 endif
 
 # check for libbsd
diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
index 0680a0faa5..921dc7e89b 100644
--- a/drivers/ml/cnxk/meson.build
+++ b/drivers/ml/cnxk/meson.build
@@ -67,6 +67,7 @@ sources += files(
         'mvtvm_ml_model.c',
 )
 
+ext_deps += libarchive
 ext_deps += jansson_dep
 ext_deps += dlpack_dep
 ext_deps += dmlc_dep
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
 if dpdk_conf.has('RTE_USE_LIBBSD')
     ext_deps += libbsd
 endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+    ext_deps += libarchive
+endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
-- 
2.42.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/1] build: add libarchive to external deps
  2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
@ 2023-11-03 16:50   ` Bruce Richardson
  2023-11-06  4:27     ` [EXT] " Srikanth Yalavarthi
  0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-11-03 16:50 UTC (permalink / raw)
  To: Srikanth Yalavarthi
  Cc: David Marchand, Aaron Conole, Igor Russkikh, dev, sshankarnara,
	aprabhu, ptakkar, stable

On Fri, Nov 03, 2023 at 09:38:53AM -0700, Srikanth Yalavarthi wrote:
> In order to avoid linking with Libs.private, libarchive
> is not added to ext_deps during the meson setup stage.
> 
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
> 
> This patch adds libarchive to ext_deps and further would not
> require setting c_args and c_link_args externally.
> 
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>

I think this is the cleanest solution to the problem you were having.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One minor comment below. Patch is ok without taking it on board if you
like.

> ---
> v4:
>   - Rebase over latest main
> v3:
>   - Add to libarchive ext_deps
> v2:
>   - Update ml/cnxk meson config
> v1:
>   - Initial patch
> 
>  config/meson.build          | 5 -----
>  drivers/ml/cnxk/meson.build | 1 +
>  lib/eal/meson.build         | 3 +++
>  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 0968351740..250833d0a4 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
>  libarchive = dependency('libarchive', required: false, method: 'pkg-config')
>  if libarchive.found()
>      dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> -    # Push libarchive link dependency at the project level to support
> -    # statically linking dpdk apps. Details at:
> -    # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
> -    add_project_link_arguments('-larchive', language: 'c')
> -    dpdk_extra_ldflags += '-larchive'
>  endif
>  
>  # check for libbsd
> diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
> index 0680a0faa5..921dc7e89b 100644
> --- a/drivers/ml/cnxk/meson.build
> +++ b/drivers/ml/cnxk/meson.build
> @@ -67,6 +67,7 @@ sources += files(
>          'mvtvm_ml_model.c',
>  )
>  
> +ext_deps += libarchive

minor nit - I don't think this is necessary. If libarchive is found, then
DPDK eal will be linked against it, and so all other drivers should simply
have that as a transitive dependency.

Same probably applies to jansson.

>  ext_deps += jansson_dep
>  ext_deps += dlpack_dep
>  ext_deps += dmlc_dep
> diff --git a/lib/eal/meson.build b/lib/eal/meson.build
> index 9942104386..e1d6c4cf17 100644
> --- a/lib/eal/meson.build
> +++ b/lib/eal/meson.build
> @@ -21,6 +21,9 @@ endif
>  if dpdk_conf.has('RTE_USE_LIBBSD')
>      ext_deps += libbsd
>  endif
> +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> +    ext_deps += libarchive
> +endif
>  if cc.has_function('getentropy', prefix : '#include <unistd.h>')
>      cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
>  endif
> -- 
> 2.42.0
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5 1/1] build: add libarchive to external deps
  2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
                   ` (3 preceding siblings ...)
  2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
@ 2023-11-06  4:12 ` Srikanth Yalavarthi
  2023-11-06  8:53   ` Bruce Richardson
  2023-11-06 16:03   ` David Marchand
  4 siblings, 2 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-11-06  4:12 UTC (permalink / raw)
  To: Bruce Richardson, Aaron Conole, Igor Russkikh, David Marchand
  Cc: dev, syalavarthi, sshankarnara, aprabhu, ptakkar, stable

In order to avoid linking with Libs.private, libarchive
is not added to ext_deps during the meson setup stage.

Since libarchive is not added to ext_deps, cross-compilation
or native compilation with libarchive installed in non-standard
location fails with errors related to "cannot find -larchive"
or "archive.h: No such file or directory". In order to fix the
build failures, user is required to define the 'c_args' and
'c_link_args' with '-I<includedir>' and '-L<libdir>'.

This patch adds libarchive to ext_deps and further would not
require setting c_args and c_link_args externally.

Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org

Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
v5:
  - Updated as per review comments
v4:
  - Rebase over latest main
v3:
  - Add to libarchive ext_deps
v2:
  - Update ml/cnxk meson config
v1:
  - Initial patch

 config/meson.build  | 5 -----
 lib/eal/meson.build | 3 +++
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/config/meson.build b/config/meson.build
index 0968351740..250833d0a4 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE', cc.has_header('execinfo.h') or is_windows)
 libarchive = dependency('libarchive', required: false, method: 'pkg-config')
 if libarchive.found()
     dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
-    # Push libarchive link dependency at the project level to support
-    # statically linking dpdk apps. Details at:
-    # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
-    add_project_link_arguments('-larchive', language: 'c')
-    dpdk_extra_ldflags += '-larchive'
 endif
 
 # check for libbsd
diff --git a/lib/eal/meson.build b/lib/eal/meson.build
index 9942104386..e1d6c4cf17 100644
--- a/lib/eal/meson.build
+++ b/lib/eal/meson.build
@@ -21,6 +21,9 @@ endif
 if dpdk_conf.has('RTE_USE_LIBBSD')
     ext_deps += libbsd
 endif
+if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
+    ext_deps += libarchive
+endif
 if cc.has_function('getentropy', prefix : '#include <unistd.h>')
     cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
 endif
-- 
2.42.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* RE: [EXT] Re: [PATCH v4 1/1] build: add libarchive to external deps
  2023-11-03 16:50   ` Bruce Richardson
@ 2023-11-06  4:27     ` Srikanth Yalavarthi
  0 siblings, 0 replies; 21+ messages in thread
From: Srikanth Yalavarthi @ 2023-11-06  4:27 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: David Marchand, Aaron Conole, Igor Russkikh, dev,
	Shivah Shankar Shankar Narayan Rao, Anup Prabhu, Prince Takkar,
	stable, Srikanth Yalavarthi

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: 03 November 2023 22:21
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: David Marchand <david.marchand@redhat.com>; Aaron Conole
> <aconole@redhat.com>; Igor Russkikh <irusskikh@marvell.com>;
> dev@dpdk.org; Shivah Shankar Shankar Narayan Rao
> <sshankarnara@marvell.com>; Anup Prabhu <aprabhu@marvell.com>;
> Prince Takkar <ptakkar@marvell.com>; stable@dpdk.org
> Subject: [EXT] Re: [PATCH v4 1/1] build: add libarchive to external deps
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Fri, Nov 03, 2023 at 09:38:53AM -0700, Srikanth Yalavarthi wrote:
> > In order to avoid linking with Libs.private, libarchive is not added
> > to ext_deps during the meson setup stage.
> >
> > Since libarchive is not added to ext_deps, cross-compilation or native
> > compilation with libarchive installed in non-standard location fails
> > with errors related to "cannot find -larchive"
> > or "archive.h: No such file or directory". In order to fix the build
> > failures, user is required to define the 'c_args' and 'c_link_args'
> > with '-I<includedir>' and '-L<libdir>'.
> >
> > This patch adds libarchive to ext_deps and further would not require
> > setting c_args and c_link_args externally.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> 
> I think this is the cleanest solution to the problem you were having.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> One minor comment below. Patch is ok without taking it on board if you like.
> 
> > ---
> > v4:
> >   - Rebase over latest main
> > v3:
> >   - Add to libarchive ext_deps
> > v2:
> >   - Update ml/cnxk meson config
> > v1:
> >   - Initial patch
> >
> >  config/meson.build          | 5 -----
> >  drivers/ml/cnxk/meson.build | 1 +
> >  lib/eal/meson.build         | 3 +++
> >  3 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > 0968351740..250833d0a4 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -241,11 +241,6 @@ dpdk_conf.set('RTE_BACKTRACE',
> > cc.has_header('execinfo.h') or is_windows)  libarchive =
> > dependency('libarchive', required: false, method: 'pkg-config')  if
> libarchive.found()
> >      dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> > -    # Push libarchive link dependency at the project level to support
> > -    # statically linking dpdk apps. Details at:
> > -    # https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__inbox.dpdk.org_dev_20210605004024.660267a1-
> 40sovereign_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=SNPqUkGl0n_M
> s1iJa_6wD6LBwX8efL_NOyXvAX-iCMI&m=6Lg7j9wRNcl4gjqh1r9DHzydY-
> ufK1K63u-
> HpinJ3wSa4B3z1AUzWiaAsg3C5Cwp&s=PFchwx1MTzUrXGTLrJiy1bWDf4M7Y
> cSM8t3BNbGxLUg&e=
> > -    add_project_link_arguments('-larchive', language: 'c')
> > -    dpdk_extra_ldflags += '-larchive'
> >  endif
> >
> >  # check for libbsd
> > diff --git a/drivers/ml/cnxk/meson.build b/drivers/ml/cnxk/meson.build
> > index 0680a0faa5..921dc7e89b 100644
> > --- a/drivers/ml/cnxk/meson.build
> > +++ b/drivers/ml/cnxk/meson.build
> > @@ -67,6 +67,7 @@ sources += files(
> >          'mvtvm_ml_model.c',
> >  )
> >
> > +ext_deps += libarchive
> 
> minor nit - I don't think this is necessary. If libarchive is found, then DPDK eal
> will be linked against it, and so all other drivers should simply have that as a
> transitive dependency.

Agreed, this would not be necessary for libarchive. Removed in v5.
> 
> Same probably applies to jansson.

Jansson is added to ext_deps by lib/metrics. However, 'lib/metrics' is optional and can be disabled through -Ddisable_libs'
I think it would be good to add jansson_dep to ext_deps here to handle the case when 'lib/metrics' is disabled.

Also, I am working on change that would remove Jansson as a dependency for ml/cnxk. We will submit a separate patch for this.

> 
> >  ext_deps += jansson_dep
> >  ext_deps += dlpack_dep
> >  ext_deps += dmlc_dep
> > diff --git a/lib/eal/meson.build b/lib/eal/meson.build index
> > 9942104386..e1d6c4cf17 100644
> > --- a/lib/eal/meson.build
> > +++ b/lib/eal/meson.build
> > @@ -21,6 +21,9 @@ endif
> >  if dpdk_conf.has('RTE_USE_LIBBSD')
> >      ext_deps += libbsd
> >  endif
> > +if dpdk_conf.has('RTE_HAS_LIBARCHIVE')
> > +    ext_deps += libarchive
> > +endif
> >  if cc.has_function('getentropy', prefix : '#include <unistd.h>')
> >      cflags += '-DRTE_LIBEAL_USE_GETENTROPY'
> >  endif
> > --
> > 2.42.0
> >

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/1] build: add libarchive to external deps
  2023-11-06  4:12 ` [PATCH v5 " Srikanth Yalavarthi
@ 2023-11-06  8:53   ` Bruce Richardson
  2023-11-06 15:24     ` Thomas Monjalon
  2023-11-06 16:03   ` David Marchand
  1 sibling, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-11-06  8:53 UTC (permalink / raw)
  To: Srikanth Yalavarthi
  Cc: Aaron Conole, Igor Russkikh, David Marchand, dev, sshankarnara,
	aprabhu, ptakkar, stable

On Sun, Nov 05, 2023 at 08:12:43PM -0800, Srikanth Yalavarthi wrote:
> In order to avoid linking with Libs.private, libarchive
> is not added to ext_deps during the meson setup stage.
> 
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
> 
> This patch adds libarchive to ext_deps and further would not
> require setting c_args and c_link_args externally.
> 
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/1] build: add libarchive to external deps
  2023-11-06  8:53   ` Bruce Richardson
@ 2023-11-06 15:24     ` Thomas Monjalon
  2023-11-06 15:32       ` Thomas Monjalon
  0 siblings, 1 reply; 21+ messages in thread
From: Thomas Monjalon @ 2023-11-06 15:24 UTC (permalink / raw)
  To: Srikanth Yalavarthi, Bruce Richardson
  Cc: dev, Aaron Conole, Igor Russkikh, David Marchand, dev,
	sshankarnara, aprabhu, ptakkar, stable

06/11/2023 09:53, Bruce Richardson:
> On Sun, Nov 05, 2023 at 08:12:43PM -0800, Srikanth Yalavarthi wrote:
> > In order to avoid linking with Libs.private, libarchive
> > is not added to ext_deps during the meson setup stage.
> > 
> > Since libarchive is not added to ext_deps, cross-compilation
> > or native compilation with libarchive installed in non-standard
> > location fails with errors related to "cannot find -larchive"
> > or "archive.h: No such file or directory". In order to fix the
> > build failures, user is required to define the 'c_args' and
> > 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
> > 
> > This patch adds libarchive to ext_deps and further would not
> > require setting c_args and c_link_args externally.
> > 
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

I'm not sure to understand which new failure will happen.
Was there a problem solved in new libarchive packages?



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/1] build: add libarchive to external deps
  2023-11-06 15:24     ` Thomas Monjalon
@ 2023-11-06 15:32       ` Thomas Monjalon
  0 siblings, 0 replies; 21+ messages in thread
From: Thomas Monjalon @ 2023-11-06 15:32 UTC (permalink / raw)
  To: Srikanth Yalavarthi, Bruce Richardson
  Cc: dev, Aaron Conole, Igor Russkikh, David Marchand, dev,
	sshankarnara, aprabhu, ptakkar, stable

06/11/2023 16:24, Thomas Monjalon:
> 06/11/2023 09:53, Bruce Richardson:
> > On Sun, Nov 05, 2023 at 08:12:43PM -0800, Srikanth Yalavarthi wrote:
> > > In order to avoid linking with Libs.private, libarchive
> > > is not added to ext_deps during the meson setup stage.
> > > 
> > > Since libarchive is not added to ext_deps, cross-compilation
> > > or native compilation with libarchive installed in non-standard
> > > location fails with errors related to "cannot find -larchive"
> > > or "archive.h: No such file or directory". In order to fix the
> > > build failures, user is required to define the 'c_args' and
> > > 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
> > > 
> > > This patch adds libarchive to ext_deps and further would not
> > > require setting c_args and c_link_args externally.
> > > 
> > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> I'm not sure to understand which new failure will happen.
> Was there a problem solved in new libarchive packages?

BTW applied as it fixes an obvious problem.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/1] build: add libarchive to external deps
  2023-11-06  4:12 ` [PATCH v5 " Srikanth Yalavarthi
  2023-11-06  8:53   ` Bruce Richardson
@ 2023-11-06 16:03   ` David Marchand
  2023-11-06 16:23     ` Bruce Richardson
  1 sibling, 1 reply; 21+ messages in thread
From: David Marchand @ 2023-11-06 16:03 UTC (permalink / raw)
  To: Srikanth Yalavarthi, Bruce Richardson
  Cc: Aaron Conole, Igor Russkikh, dev, sshankarnara, aprabhu, ptakkar, stable

On Mon, Nov 6, 2023 at 5:12 AM Srikanth Yalavarthi
<syalavarthi@marvell.com> wrote:
>
> In order to avoid linking with Libs.private, libarchive
> is not added to ext_deps during the meson setup stage.
>
> Since libarchive is not added to ext_deps, cross-compilation
> or native compilation with libarchive installed in non-standard
> location fails with errors related to "cannot find -larchive"
> or "archive.h: No such file or directory". In order to fix the
> build failures, user is required to define the 'c_args' and
> 'c_link_args' with '-I<includedir>' and '-L<libdir>'.
>
> This patch adds libarchive to ext_deps and further would not
> require setting c_args and c_link_args externally.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>

This breaks static compilation of applications.
This can be reproduced with test-meson-builds.sh and in GHA (which was
not linking examples statically, I added a patch in my github repo):
https://github.com/david-marchand/dpdk/actions/runs/6772879600/job/18406442129#step:19:19572


-- 
David Marchand


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/1] build: add libarchive to external deps
  2023-11-06 16:03   ` David Marchand
@ 2023-11-06 16:23     ` Bruce Richardson
  2023-11-06 18:17       ` David Marchand
  0 siblings, 1 reply; 21+ messages in thread
From: Bruce Richardson @ 2023-11-06 16:23 UTC (permalink / raw)
  To: David Marchand
  Cc: Srikanth Yalavarthi, Aaron Conole, Igor Russkikh, dev,
	sshankarnara, aprabhu, ptakkar, stable

On Mon, Nov 06, 2023 at 05:03:10PM +0100, David Marchand wrote:
> On Mon, Nov 6, 2023 at 5:12 AM Srikanth Yalavarthi
> <syalavarthi@marvell.com> wrote:
> >
> > In order to avoid linking with Libs.private, libarchive is not added to
> > ext_deps during the meson setup stage.
> >
> > Since libarchive is not added to ext_deps, cross-compilation or native
> > compilation with libarchive installed in non-standard location fails
> > with errors related to "cannot find -larchive" or "archive.h: No such
> > file or directory". In order to fix the build failures, user is
> > required to define the 'c_args' and 'c_link_args' with '-I<includedir>'
> > and '-L<libdir>'.
> >
> > This patch adds libarchive to ext_deps and further would not require
> > setting c_args and c_link_args externally.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware") Cc:
> > stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> 
> This breaks static compilation of applications.  This can be reproduced
> with test-meson-builds.sh and in GHA (which was not linking examples
> statically, I added a patch in my github repo):
> https://github.com/david-marchand/dpdk/actions/runs/6772879600/job/18406442129#step:19:19572
> 
The libarchive-dev Ubuntu package does not install all its needed
dependencies for static linking. The errors can be resolved by manually
installing the 3 missing -dev packages.

It's less than ideal, but to my mind, DPDK is behaving correctly with this
fix - it is marking that it requires libarchive as a dependency. The fact
that the libarchive.pc file lists static libraries that aren't installed is
outside of our control. The previous implementation hacked around this by
just passing -larchive in all cases, rather than using pkg-config
information. This then caused other issues that the patch submitter hit.

/Bruce

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/1] build: add libarchive to external deps
  2023-11-06 16:23     ` Bruce Richardson
@ 2023-11-06 18:17       ` David Marchand
  0 siblings, 0 replies; 21+ messages in thread
From: David Marchand @ 2023-11-06 18:17 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Srikanth Yalavarthi, Aaron Conole, Igor Russkikh, dev,
	sshankarnara, aprabhu, ptakkar, stable

On Mon, Nov 6, 2023 at 5:24 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Mon, Nov 06, 2023 at 05:03:10PM +0100, David Marchand wrote:
> > On Mon, Nov 6, 2023 at 5:12 AM Srikanth Yalavarthi
> > <syalavarthi@marvell.com> wrote:
> > >
> > > In order to avoid linking with Libs.private, libarchive is not added to
> > > ext_deps during the meson setup stage.
> > >
> > > Since libarchive is not added to ext_deps, cross-compilation or native
> > > compilation with libarchive installed in non-standard location fails
> > > with errors related to "cannot find -larchive" or "archive.h: No such
> > > file or directory". In order to fix the build failures, user is
> > > required to define the 'c_args' and 'c_link_args' with '-I<includedir>'
> > > and '-L<libdir>'.
> > >
> > > This patch adds libarchive to ext_deps and further would not require
> > > setting c_args and c_link_args externally.
> > >
> > > Fixes: 40edb9c0d36b ("eal: handle compressed firmware") Cc:
> > > stable@dpdk.org
> > >
> > > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> >
> > This breaks static compilation of applications.  This can be reproduced
> > with test-meson-builds.sh and in GHA (which was not linking examples
> > statically, I added a patch in my github repo):
> > https://github.com/david-marchand/dpdk/actions/runs/6772879600/job/18406442129#step:19:19572
> >
> The libarchive-dev Ubuntu package does not install all its needed
> dependencies for static linking. The errors can be resolved by manually
> installing the 3 missing -dev packages.

Same with fedora.

>
> It's less than ideal, but to my mind, DPDK is behaving correctly with this
> fix - it is marking that it requires libarchive as a dependency. The fact
> that the libarchive.pc file lists static libraries that aren't installed is
> outside of our control. The previous implementation hacked around this by
> just passing -larchive in all cases, rather than using pkg-config
> information. This then caused other issues that the patch submitter hit.

Ok, I'll see how to workaround this on my side.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2023-11-06 18:17 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-20 17:01 [PATCH 1/1] build: update link args and includes for libarchive Srikanth Yalavarthi
2023-10-23  9:26 ` Bruce Richardson
2023-10-23 11:40   ` [EXT] " Srikanth Yalavarthi
2023-10-23 11:53     ` Bruce Richardson
2023-10-23 12:46       ` Srikanth Yalavarthi
2023-10-23 13:00         ` Bruce Richardson
2023-10-23 13:06           ` Bruce Richardson
2023-10-23 14:01             ` Srikanth Yalavarthi
2023-10-26 12:53               ` Srikanth Yalavarthi
2023-10-29  8:10 ` [PATCH v2 1/1] build: add libarchive to external deps Srikanth Yalavarthi
2023-10-29  8:20 ` [PATCH v3 " Srikanth Yalavarthi
2023-11-03 16:38 ` [PATCH v4 " Srikanth Yalavarthi
2023-11-03 16:50   ` Bruce Richardson
2023-11-06  4:27     ` [EXT] " Srikanth Yalavarthi
2023-11-06  4:12 ` [PATCH v5 " Srikanth Yalavarthi
2023-11-06  8:53   ` Bruce Richardson
2023-11-06 15:24     ` Thomas Monjalon
2023-11-06 15:32       ` Thomas Monjalon
2023-11-06 16:03   ` David Marchand
2023-11-06 16:23     ` Bruce Richardson
2023-11-06 18:17       ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).