[1/1] build: update link args and includes for libarchive

Message ID 20231020170135.18319-1-syalavarthi@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [1/1] build: update link args and includes for libarchive |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-compile-amd64-testing fail Testing issues
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS

Commit Message

Srikanth Yalavarthi Oct. 20, 2023, 5:01 p.m. UTC
  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(+)
  

Comments

Bruce Richardson Oct. 23, 2023, 9:26 a.m. UTC | #1
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
  
Srikanth Yalavarthi Oct. 23, 2023, 11:40 a.m. UTC | #2
> -----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
  
Bruce Richardson Oct. 23, 2023, 11:53 a.m. UTC | #3
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
  
Srikanth Yalavarthi Oct. 23, 2023, 12:46 p.m. UTC | #4
> -----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
  
Bruce Richardson Oct. 23, 2023, 1 p.m. UTC | #5
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
  
Bruce Richardson Oct. 23, 2023, 1:06 p.m. UTC | #6
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
  
Srikanth Yalavarthi Oct. 23, 2023, 2:01 p.m. UTC | #7
> -----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.
  
Srikanth Yalavarthi Oct. 26, 2023, 12:53 p.m. UTC | #8
> -----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.
  

Patch

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